[PATCH 3/6] RADEONDownloadFromScreenCS: flush CS writes before mapping BO for read

[Michel Dänzer]

On Mit, 2010-09-01 at 10:32 +1200, Karl Tomlinson wrote: 
> Michel Dänzer writes:
> 
> >> @@ -593,6 +596,10 @@ RADEONDownloadFromScreenCS(PixmapPtr pSrc, int x, int y, int w,
> >>  	if ((src_domain & (RADEON_GEM_DOMAIN_GTT | RADEON_GEM_DOMAIN_VRAM)) ==
> >>  	    (RADEON_GEM_DOMAIN_GTT | RADEON_GEM_DOMAIN_VRAM))
> >>  	    src_domain = 0;
> >> +#if X_BYTE_ORDER == X_BIG_ENDIAN
> >> +	else /* A write may be scheduled */
> >> +	    flush = TRUE;
> >> +#endif
> >>      }
> 
> (Is there a way to change the default number of lines of context
> for git format-patch?)
> 
> >
> > It should be possible to determine with certainty if the unflushed CS
> > writes to the BO or not, e.g. by looking at the destination domain? (I'm
> > wondering if we shouldn't take that into account for guessing where the
> > BO ends up as well...)
> 
> Note that radeon_bo_get_src_domain() returns the write domain if
> there is one.

AFAICT it's kind of the other way around, it only returns the write
domain if no read domain is specified.

> If only the local code is considered, there could theoretically be
> a read(-only) domain that is not
> (RADEON_GEM_DOMAIN_GTT | RADEON_GEM_DOMAIN_VRAM).
> 
> However, the driver seems consistent in using
> (RADEON_GEM_DOMAIN_GTT | RADEON_GEM_DOMAIN_VRAM) for read domains.
> If it wasn't consistent then it looks like radeon_cs_setup_bo
> would be spitting out "READ DOMAIN RELOC FAILURE" warnings.
> 
> So here we know that the BO is referenced by the CS and we are
> pretty sure that it is not referenced for a read.
> IIUC that means it is referenced for a write.

Maybe you're right though, I'm having trouble wrapping my head around
all of this logic right now. :}


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