[PATCH 1/6] DownloadFromScreenCS: download via a scratch BO if pixmap domain is unknown

[Michel Dänzer]

On Mit, 2010-09-01 at 10:39 +1200, Karl Tomlinson wrote: 
> Michel Dänzer writes:
> 
> > On Mon, 2010-08-23 at 18:27 +1200, Karl Tomlinson wrote: 
> >> 
> >> radeon_bo_is_busy() may return without setting the domain out-parameter.
> >> If this happens, then download via a scratch GTT BO to avoid CPU VRAM read.
> >
> > I'm on the fence about this one. I can see how it could help *if*
> > radeon_bo_is_busy() doesn't set the domain[0] and the BO happens to be
> > in VRAM, but OTOH it might also hurt if the BO is not in VRAM? Can you
> > elaborate a bit on how often you're encountering this condition and what
> > if any measurements you've made about the impact on performance of this
> > patch and possibly how often the BO is actually in VRAM or not in those
> > cases.
> >
> > [0] AFAICT that can only happen if messages like
> >
> >         radeon 0000:00:10.0: ee7e7400 reserve failed for wait
> >
> > appear in dmesg, which only happens very rarely here and might be
> > fixable in the kernel anyway.
> 
> I guess ERESTARTSYS is not likely with no wait, so yes, I guess I
> would have seen this message, should this have happened.
> I haven't seen this message, so maybe it hasn't happened.
> 
> I changed this because I knew the disaster that resulted from CPU
> VRAM read and didn't want that to happen.
> 
> Is it not possible for another process to have reserved the BO
> (through DRI perhaps)?
> 
> Even if this is unlikely/uncommon, I think it is still worth
> choosing the right behavior here.  Even if it only happens rarely,
> a 2 second glitch gets noticed.

Fair enough I guess.


> Again, the consequences of a CPU read from VRAM are much worse, I
> expect/hope, than the consequences of GPU blit from GTT.  Maybe a
> GPU blit when the BO is in CPU domain would be bad, but is it
> likely to be busy in that situation?
> 
> I expect that the BO is usually in VRAM.  Pixmaps are created in
> VRAM, UTS moves them to VRAM often enough, and most (or all) write
> operations move them to VRAM.
> 
> I suspect the most likely time that the BO would not be in VRAM is
> if it is has been evicted because it is not in use, and so the
> choice doesn't matter (because it is not busy).

FWIW, whether the BO is busy or not doesn't really matter for this. The
code just uses radeon_bo_is_busy() for guessing where the BO might end
up if it isn't referenced by the CS already.


-- 
Earthling Michel Dänzer           |                http://www.vmware.com
Libre software enthusiast         |          Debian, X and DRI developer