Re: [PATCH/RFC] PVR2 DMA fix

[Adrian M. <ad...@ne...>]

On Sun, 2007-09-23 at 16:21 +0900, Paul Mundt wrote:
> On Sat, Sep 22, 2007 at 02:16:30PM +0100, Adrian McMenamin wrote:
> > The code for PVR2 DMA in the kernel is currently completely broken at
> > both hardware handling and algorithmic levels and more than likely to
> > cash a fatal crash if used.
> > 
> "Completely" is a bit of an overstatement. I had writes working with it
> without using the TA and had the PVR2 DMAC completion IRQ firing in the
> ->write() path without any difficulty. The main issue is that it was very
> sensitive, and would immediately stop working if something on the SH DMAC
> cascade side was misconfigured.


I'm at a loss to see how it worked at all! It was true that the first
transfer would work if it was less than a page long, but no susbsequent
transfer would and the contiguity check ought to have blown up every
time because of an out-by-one fault in checking the number of pages.

Also the code slept in the interrupt context but I know you knew that
because it was a debugging message :)

> 
> > This patch fixes the code and it seems to work well. I know it is not up
> > to the standards required to go to mainline yet, but any commentary on
> > it would be helpful.
> > 
> It's certainly a good start, there's obviously more information to go on
> these days then there was back when this was initially written, and it's
> good to see that someone is actually carrying it forward :-)
> 
> I suppose the main thing to note is that the ->write() path is really a
> useless place to do DMA. Most applications mmap() the framebuffer and
> dirty pages all over the place, which this simply won't catch. The idea
> behind the ->write() path was just to have something we could trivially
> test via things like cat /dev/urandom > /dev/fb0 and so on.
> 
> Ideally this should be plugged in via the fb deferred I/O, now that we
> can build scatterlists of dirtied pages in the mmap() path and submit 
> those back in one shot. hecubafb currently uses this, but not for DMA.
> The DMA work should be submitted by the ->deferred_io() callback, which
> you can probably just rip out of the write path for better utilization.
> This is really the way we want to go for fb DMA in general.


I have got a patch now that handles all this using the SH DMA API but...

What I now understand (or think I do) is that "texture" (which is what
you are really transferring) isn't the same as a bitmap and you have to
give the TA commands to handle how it is rendered. Not really versed in
the ways of 3D graphics so this is slow going.

I am away for a few days now but if anybody has a burning desire to mess
with the code they can look here:
http://newgolddream.dyndns.info/cgi-bin/cvsweb/aica/pvr2/