Bug 89416

Created attachment 113963 [details]
Some logs from UE4Editor

In case you're dying to work around this, you can compile with CFLAGS that do not enable SSSE3 and it will avoid this code path.

You right with -mno-ssse3 is work well. Why we can't use _mm_shuffle_epi8 if first 4 bit of addr dst is 0? And how could this happen?

Sorry for my questions.

Ok, alignment... maybe I can fix it.

(In reply to Uriy Zhuravlev from comment #4)
> Ok, alignment... maybe I can fix it.

Uriy,
The problem here is that the code originally assumed that the destination would always be nicely aligned but the source may not.  When rgba8_copy function started getting used for texture *download* via ReadPixels and GetTexImage the assumption now fails and it is the source that is aligned and not the destination.  To fix this we need to do one of two things.  Either a) make the rgba8_copy function use unaligned reads (which would b slower) or b) make two versions of the rgba8_copy function: one for upload and one for download.

I think option b) is best but if we're going to do that then we need to add a flag to intel_get_memcpy to make sure it returns the correct one and then thread it through the tiled_to_linear_faster functions as well.

(In reply to Jason Ekstrand from comment #5)
> (In reply to Uriy Zhuravlev from comment #4)
> > Ok, alignment... maybe I can fix it.
> 
> Uriy,
> The problem here is that the code originally assumed that the destination
> would always be nicely aligned but the source may not.  When rgba8_copy
> function started getting used for texture *download* via ReadPixels and
> GetTexImage the assumption now fails and it is the source that is aligned
> and not the destination.  To fix this we need to do one of two things. 
> Either a) make the rgba8_copy function use unaligned reads (which would b
> slower) or b) make two versions of the rgba8_copy function: one for upload
> and one for download.

Basically, this all stems from the fact that there's no guarantee that the pointer given to glReadPixels/glGetTexImage is suitably aligned?

Do you have time to fix this before the release on Friday? Trying to figure if I need to work on this.

(In reply to Matt Turner from comment #6)
> (In reply to Jason Ekstrand from comment #5)
> > (In reply to Uriy Zhuravlev from comment #4)
> > > Ok, alignment... maybe I can fix it.
> > 
> > Uriy,
> > The problem here is that the code originally assumed that the destination
> > would always be nicely aligned but the source may not.  When rgba8_copy
> > function started getting used for texture *download* via ReadPixels and
> > GetTexImage the assumption now fails and it is the source that is aligned
> > and not the destination.  To fix this we need to do one of two things. 
> > Either a) make the rgba8_copy function use unaligned reads (which would b
> > slower) or b) make two versions of the rgba8_copy function: one for upload
> > and one for download.
> 
> Basically, this all stems from the fact that there's no guarantee that the
> pointer given to glReadPixels/glGetTexImage is suitably aligned?
> 
> Do you have time to fix this before the release on Friday? Trying to figure
> if I need to work on this.

I think it would probably be more efficient if you did.  The above should be enough of a brain dump on what needs to happen in the tiled_memcpy paths.  The rest is writing suitable mmx code and you probably know more about that than I do.  TBH: I'd probably be asking you on IRC what instructions to use anyway.  If you don't want to bother with threading it through the tiled_memcpy code, I can do that part quick enough.

Yeah, sounds like (b) is a good option. I don't think it should take me too much time.

I will always be happy to help at least testing.

Created attachment 114014 [details] [review]
patch

Please test this patch. I sent it to the mailing list and Cc'd you as well.

This patch works perfectly. Thank U.

Fixed by

commit 2e4c95dfe2cb205c327ceaa12b44a9273bdb20dc
Author: Matt Turner <mattst88@gmail.com>
Date:   Wed Mar 4 15:21:53 2015 -0800

    i965: Tell intel_get_memcpy() which direction the memcpy() is going.