Attachment 114014 Details for Bug 89416 – patch

[patch] patch

0001-i965-Tell-intel_get_memcpy-which-direction-the-memcp.patch (text/plain), 16.19 KB, created by Matt Turner on 2015-03-05 00:20:47 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Matt Turner

Created: 2015-03-05 00:20:47 UTC

Size: 16.19 KB

patch

obsolete

>From a48d5f289218fa5af07dc78f5fae52e5cb6875be Mon Sep 17 00:00:00 2001
>From: Matt Turner <mattst88@gmail.com>
>Date: Wed, 4 Mar 2015 15:21:53 -0800
>Subject: [PATCH] i965: Tell intel_get_memcpy() which direction the memcpy() is
> going.
>
>The SSSE3 swizzling code was written for fast uploads to the GPU and
>assumed the destination was always 16-byte aligned. When we began using
>this code for fast downloads as well we didn't do anything to account
>for the fact that the destination pointer given by glReadPixels() or
>glGetTexImage() is not guaranteed to be suitably aligned.
>
>With SSSE3 enabled (at compile-time), some applications would crash when
>an SSE aligned-store instruction tried to store to an unaligned
>destination (or an assertion that the destination is aligned would
>trigger).
>
>To remedy this, tell intel_get_memcpy() whether we're uploading or
>downloading so that it can select whether to assume the destination or
>source is aligned, respectively.
>
>Cc: 10.5 <mesa-stable@lists.freedesktop.org>
>Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=89416
>---
> src/mesa/drivers/dri/i965/intel_pixel_read.c   |   3 +-
> src/mesa/drivers/dri/i965/intel_tex_image.c    |   3 +-
> src/mesa/drivers/dri/i965/intel_tex_subimage.c |   3 +-
> src/mesa/drivers/dri/i965/intel_tiled_memcpy.c | 156 +++++++++++++++++++------
> src/mesa/drivers/dri/i965/intel_tiled_memcpy.h |  15 ++-
> 5 files changed, 138 insertions(+), 42 deletions(-)
>
>diff --git a/src/mesa/drivers/dri/i965/intel_pixel_read.c b/src/mesa/drivers/dri/i965/intel_pixel_read.c
>index df22a63..0972121 100644
>--- a/src/mesa/drivers/dri/i965/intel_pixel_read.c
>+++ b/src/mesa/drivers/dri/i965/intel_pixel_read.c
>@@ -139,7 +139,8 @@ intel_readpixels_tiled_memcpy(struct gl_context * ctx,
>        rb->Format == MESA_FORMAT_R8G8B8X8_UNORM)
>       return false;
> 
>-   if (!intel_get_memcpy(rb->Format, format, type, &mem_copy, &cpp))
>+   if (!intel_get_memcpy(rb->Format, format, type, &mem_copy, &cpp,
>+                         INTEL_DOWNLOAD))
>       return false;
> 
>    if (!irb->mt ||
>diff --git a/src/mesa/drivers/dri/i965/intel_tex_image.c b/src/mesa/drivers/dri/i965/intel_tex_image.c
>index da42fdd..a3312d2 100644
>--- a/src/mesa/drivers/dri/i965/intel_tex_image.c
>+++ b/src/mesa/drivers/dri/i965/intel_tex_image.c
>@@ -408,7 +408,8 @@ intel_gettexsubimage_tiled_memcpy(struct gl_context *ctx,
>        texImage->TexFormat == MESA_FORMAT_R8G8B8X8_UNORM)
>       return false;
> 
>-   if (!intel_get_memcpy(texImage->TexFormat, format, type, &mem_copy, &cpp))
>+   if (!intel_get_memcpy(texImage->TexFormat, format, type, &mem_copy, &cpp,
>+                         INTEL_UPLOAD))
>       return false;
> 
>    /* If this is a nontrivial texture view, let another path handle it instead. */
>diff --git a/src/mesa/drivers/dri/i965/intel_tex_subimage.c b/src/mesa/drivers/dri/i965/intel_tex_subimage.c
>index 4262f71..909ff25 100644
>--- a/src/mesa/drivers/dri/i965/intel_tex_subimage.c
>+++ b/src/mesa/drivers/dri/i965/intel_tex_subimage.c
>@@ -118,7 +118,8 @@ intel_texsubimage_tiled_memcpy(struct gl_context * ctx,
>        packing->Invert)
>       return false;
> 
>-   if (!intel_get_memcpy(texImage->TexFormat, format, type, &mem_copy, &cpp))
>+   if (!intel_get_memcpy(texImage->TexFormat, format, type, &mem_copy, &cpp,
>+                         INTEL_UPLOAD))
>       return false;
> 
>    /* If this is a nontrivial texture view, let another path handle it instead. */
>diff --git a/src/mesa/drivers/dri/i965/intel_tiled_memcpy.c b/src/mesa/drivers/dri/i965/intel_tiled_memcpy.c
>index f2b35cb..c43383b 100644
>--- a/src/mesa/drivers/dri/i965/intel_tiled_memcpy.c
>+++ b/src/mesa/drivers/dri/i965/intel_tiled_memcpy.c
>@@ -60,42 +60,79 @@ static const uint32_t ytile_span = 16;
> static const uint8_t rgba8_permutation[16] =
>    { 2,1,0,3, 6,5,4,7, 10,9,8,11, 14,13,12,15 };
> 
>-/* NOTE: dst must be 16 byte aligned */
>-#define rgba8_copy_16(dst, src)                     \
>-   *(__m128i *)(dst) = _mm_shuffle_epi8(            \
>-      (__m128i) _mm_loadu_ps((float *)(src)),       \
>-      *(__m128i *) rgba8_permutation                \
>-   )
>+/* NOTE: dst must be 16-byte aligned. src may be unaligned. */
>+#define rgba8_copy_16_aligned_dst(dst, src)                            \
>+   _mm_store_si128((__m128i *)(dst),                                   \
>+                   _mm_shuffle_epi8(_mm_loadu_si128((__m128i *)(src)), \
>+                                    *(__m128i *) rgba8_permutation))
>+
>+/* NOTE: src must be 16-byte aligned. dst may be unaligned. */
>+#define rgba8_copy_16_aligned_src(dst, src)                            \
>+   _mm_storeu_si128((__m128i *)(dst),                                  \
>+                    _mm_shuffle_epi8(_mm_load_si128((__m128i *)(src)), \
>+                                     *(__m128i *) rgba8_permutation))
> #endif
> 
> /**
>- * Copy RGBA to BGRA - swap R and B.
>+ * Copy RGBA to BGRA - swap R and B, with the destination 16-byte aligned.
>  */
> static inline void *
>-rgba8_copy(void *dst, const void *src, size_t bytes)
>+rgba8_copy_aligned_dst(void *dst, const void *src, size_t bytes)
> {
>    uint8_t *d = dst;
>    uint8_t const *s = src;
> 
> #ifdef __SSSE3__
>-   /* Fast copying for tile spans.
>-    *
>-    * As long as the destination texture is 16 aligned,
>-    * any 16 or 64 spans we get here should also be 16 aligned.
>-    */
>-
>    if (bytes == 16) {
>       assert(!(((uintptr_t)dst) & 0xf));
>-      rgba8_copy_16(d+ 0, s+ 0);
>+      rgba8_copy_16_aligned_dst(d+ 0, s+ 0);
>       return dst;
>    }
> 
>    if (bytes == 64) {
>       assert(!(((uintptr_t)dst) & 0xf));
>-      rgba8_copy_16(d+ 0, s+ 0);
>-      rgba8_copy_16(d+16, s+16);
>-      rgba8_copy_16(d+32, s+32);
>-      rgba8_copy_16(d+48, s+48);
>+      rgba8_copy_16_aligned_dst(d+ 0, s+ 0);
>+      rgba8_copy_16_aligned_dst(d+16, s+16);
>+      rgba8_copy_16_aligned_dst(d+32, s+32);
>+      rgba8_copy_16_aligned_dst(d+48, s+48);
>+      return dst;
>+   }
>+#endif
>+
>+   while (bytes >= 4) {
>+      d[0] = s[2];
>+      d[1] = s[1];
>+      d[2] = s[0];
>+      d[3] = s[3];
>+      d += 4;
>+      s += 4;
>+      bytes -= 4;
>+   }
>+   return dst;
>+}
>+
>+/**
>+ * Copy RGBA to BGRA - swap R and B, with the source 16-byte aligned.
>+ */
>+static inline void *
>+rgba8_copy_aligned_src(void *dst, const void *src, size_t bytes)
>+{
>+   uint8_t *d = dst;
>+   uint8_t const *s = src;
>+
>+#ifdef __SSSE3__
>+   if (bytes == 16) {
>+      assert(!(((uintptr_t)src) & 0xf));
>+      rgba8_copy_16_aligned_src(d+ 0, s+ 0);
>+      return dst;
>+   }
>+
>+   if (bytes == 64) {
>+      assert(!(((uintptr_t)src) & 0xf));
>+      rgba8_copy_16_aligned_src(d+ 0, s+ 0);
>+      rgba8_copy_16_aligned_src(d+16, s+16);
>+      rgba8_copy_16_aligned_src(d+32, s+32);
>+      rgba8_copy_16_aligned_src(d+48, s+48);
>       return dst;
>    }
> #endif
>@@ -357,16 +394,26 @@ linear_to_xtiled_faster(uint32_t x0, uint32_t x1, uint32_t x2, uint32_t x3,
>       if (mem_copy == memcpy)
>          return linear_to_xtiled(0, 0, xtile_width, xtile_width, 0, xtile_height,
>                                  dst, src, src_pitch, swizzle_bit, memcpy);
>-      else if (mem_copy == rgba8_copy)
>+      else if (mem_copy == rgba8_copy_aligned_dst)
>+         return linear_to_xtiled(0, 0, xtile_width, xtile_width, 0, xtile_height,
>+                                 dst, src, src_pitch, swizzle_bit,
>+                                 rgba8_copy_aligned_dst);
>+      else if (mem_copy == rgba8_copy_aligned_src)
>          return linear_to_xtiled(0, 0, xtile_width, xtile_width, 0, xtile_height,
>-                                 dst, src, src_pitch, swizzle_bit, rgba8_copy);
>+                                 dst, src, src_pitch, swizzle_bit,
>+                                 rgba8_copy_aligned_src);
>    } else {
>       if (mem_copy == memcpy)
>          return linear_to_xtiled(x0, x1, x2, x3, y0, y1,
>                                  dst, src, src_pitch, swizzle_bit, memcpy);
>-      else if (mem_copy == rgba8_copy)
>+      else if (mem_copy == rgba8_copy_aligned_dst)
>          return linear_to_xtiled(x0, x1, x2, x3, y0, y1,
>-                                 dst, src, src_pitch, swizzle_bit, rgba8_copy);
>+                                 dst, src, src_pitch, swizzle_bit,
>+                                 rgba8_copy_aligned_dst);
>+      else if (mem_copy == rgba8_copy_aligned_src)
>+         return linear_to_xtiled(x0, x1, x2, x3, y0, y1,
>+                                 dst, src, src_pitch, swizzle_bit,
>+                                 rgba8_copy_aligned_src);
>    }
>    linear_to_xtiled(x0, x1, x2, x3, y0, y1,
>                     dst, src, src_pitch, swizzle_bit, mem_copy);
>@@ -393,16 +440,26 @@ linear_to_ytiled_faster(uint32_t x0, uint32_t x1, uint32_t x2, uint32_t x3,
>       if (mem_copy == memcpy)
>          return linear_to_ytiled(0, 0, ytile_width, ytile_width, 0, ytile_height,
>                                  dst, src, src_pitch, swizzle_bit, memcpy);
>-      else if (mem_copy == rgba8_copy)
>+      else if (mem_copy == rgba8_copy_aligned_dst)
>+         return linear_to_ytiled(0, 0, ytile_width, ytile_width, 0, ytile_height,
>+                                 dst, src, src_pitch, swizzle_bit,
>+                                 rgba8_copy_aligned_dst);
>+      else if (mem_copy == rgba8_copy_aligned_src)
>          return linear_to_ytiled(0, 0, ytile_width, ytile_width, 0, ytile_height,
>-                                 dst, src, src_pitch, swizzle_bit, rgba8_copy);
>+                                 dst, src, src_pitch, swizzle_bit,
>+                                 rgba8_copy_aligned_src);
>    } else {
>       if (mem_copy == memcpy)
>          return linear_to_ytiled(x0, x1, x2, x3, y0, y1,
>                                  dst, src, src_pitch, swizzle_bit, memcpy);
>-      else if (mem_copy == rgba8_copy)
>+      else if (mem_copy == rgba8_copy_aligned_dst)
>          return linear_to_ytiled(x0, x1, x2, x3, y0, y1,
>-                                 dst, src, src_pitch, swizzle_bit, rgba8_copy);
>+                                 dst, src, src_pitch, swizzle_bit,
>+                                 rgba8_copy_aligned_dst);
>+      else if (mem_copy == rgba8_copy_aligned_src)
>+         return linear_to_ytiled(x0, x1, x2, x3, y0, y1,
>+                                 dst, src, src_pitch, swizzle_bit,
>+                                 rgba8_copy_aligned_src);
>    }
>    linear_to_ytiled(x0, x1, x2, x3, y0, y1,
>                     dst, src, src_pitch, swizzle_bit, mem_copy);
>@@ -429,16 +486,26 @@ xtiled_to_linear_faster(uint32_t x0, uint32_t x1, uint32_t x2, uint32_t x3,
>       if (mem_copy == memcpy)
>          return xtiled_to_linear(0, 0, xtile_width, xtile_width, 0, xtile_height,
>                                  dst, src, dst_pitch, swizzle_bit, memcpy);
>-      else if (mem_copy == rgba8_copy)
>+      else if (mem_copy == rgba8_copy_aligned_dst)
>+         return xtiled_to_linear(0, 0, xtile_width, xtile_width, 0, xtile_height,
>+                                 dst, src, dst_pitch, swizzle_bit,
>+                                 rgba8_copy_aligned_dst);
>+      else if (mem_copy == rgba8_copy_aligned_src)
>          return xtiled_to_linear(0, 0, xtile_width, xtile_width, 0, xtile_height,
>-                                 dst, src, dst_pitch, swizzle_bit, rgba8_copy);
>+                                 dst, src, dst_pitch, swizzle_bit,
>+                                 rgba8_copy_aligned_src);
>    } else {
>       if (mem_copy == memcpy)
>          return xtiled_to_linear(x0, x1, x2, x3, y0, y1,
>                                  dst, src, dst_pitch, swizzle_bit, memcpy);
>-      else if (mem_copy == rgba8_copy)
>+      else if (mem_copy == rgba8_copy_aligned_dst)
>+         return xtiled_to_linear(x0, x1, x2, x3, y0, y1,
>+                                 dst, src, dst_pitch, swizzle_bit,
>+                                 rgba8_copy_aligned_dst);
>+      else if (mem_copy == rgba8_copy_aligned_src)
>          return xtiled_to_linear(x0, x1, x2, x3, y0, y1,
>-                                 dst, src, dst_pitch, swizzle_bit, rgba8_copy);
>+                                 dst, src, dst_pitch, swizzle_bit,
>+                                 rgba8_copy_aligned_src);
>    }
>    xtiled_to_linear(x0, x1, x2, x3, y0, y1,
>                     dst, src, dst_pitch, swizzle_bit, mem_copy);
>@@ -465,16 +532,26 @@ ytiled_to_linear_faster(uint32_t x0, uint32_t x1, uint32_t x2, uint32_t x3,
>       if (mem_copy == memcpy)
>          return ytiled_to_linear(0, 0, ytile_width, ytile_width, 0, ytile_height,
>                                  dst, src, dst_pitch, swizzle_bit, memcpy);
>-      else if (mem_copy == rgba8_copy)
>+      else if (mem_copy == rgba8_copy_aligned_dst)
>          return ytiled_to_linear(0, 0, ytile_width, ytile_width, 0, ytile_height,
>-                                 dst, src, dst_pitch, swizzle_bit, rgba8_copy);
>+                                 dst, src, dst_pitch, swizzle_bit,
>+                                 rgba8_copy_aligned_dst);
>+      else if (mem_copy == rgba8_copy_aligned_src)
>+         return ytiled_to_linear(0, 0, ytile_width, ytile_width, 0, ytile_height,
>+                                 dst, src, dst_pitch, swizzle_bit,
>+                                 rgba8_copy_aligned_src);
>    } else {
>       if (mem_copy == memcpy)
>          return ytiled_to_linear(x0, x1, x2, x3, y0, y1,
>                                  dst, src, dst_pitch, swizzle_bit, memcpy);
>-      else if (mem_copy == rgba8_copy)
>+      else if (mem_copy == rgba8_copy_aligned_dst)
>+         return ytiled_to_linear(x0, x1, x2, x3, y0, y1,
>+                                 dst, src, dst_pitch, swizzle_bit,
>+                                 rgba8_copy_aligned_dst);
>+      else if (mem_copy == rgba8_copy_aligned_src)
>          return ytiled_to_linear(x0, x1, x2, x3, y0, y1,
>-                                 dst, src, dst_pitch, swizzle_bit, rgba8_copy);
>+                                 dst, src, dst_pitch, swizzle_bit,
>+                                 rgba8_copy_aligned_src);
>    }
>    ytiled_to_linear(x0, x1, x2, x3, y0, y1,
>                     dst, src, dst_pitch, swizzle_bit, mem_copy);
>@@ -684,7 +761,8 @@ tiled_to_linear(uint32_t xt1, uint32_t xt2,
>  * \return true if the format and type combination are valid
>  */
> bool intel_get_memcpy(mesa_format tiledFormat, GLenum format,
>-                      GLenum type, mem_copy_fn* mem_copy, uint32_t* cpp)
>+                      GLenum type, mem_copy_fn *mem_copy, uint32_t *cpp,
>+                      enum intel_memcpy_direction direction)
> {
>    if (type == GL_UNSIGNED_INT_8_8_8_8_REV &&
>        !(format == GL_RGBA || format == GL_BGRA))
>@@ -700,7 +778,8 @@ bool intel_get_memcpy(mesa_format tiledFormat, GLenum format,
>       if (format == GL_BGRA) {
>          *mem_copy = memcpy;
>       } else if (format == GL_RGBA) {
>-         *mem_copy = rgba8_copy;
>+         *mem_copy = direction == INTEL_UPLOAD ? rgba8_copy_aligned_dst
>+                                               : rgba8_copy_aligned_src;
>       }
>    } else if ((tiledFormat == MESA_FORMAT_R8G8B8A8_UNORM) ||
>               (tiledFormat == MESA_FORMAT_R8G8B8X8_UNORM)) {
>@@ -709,7 +788,8 @@ bool intel_get_memcpy(mesa_format tiledFormat, GLenum format,
>          /* Copying from RGBA to BGRA is the same as BGRA to RGBA so we can
>           * use the same function.
>           */
>-         *mem_copy = rgba8_copy;
>+         *mem_copy = direction == INTEL_UPLOAD ? rgba8_copy_aligned_dst
>+                                               : rgba8_copy_aligned_src;
>       } else if (format == GL_RGBA) {
>          *mem_copy = memcpy;
>       }
>diff --git a/src/mesa/drivers/dri/i965/intel_tiled_memcpy.h b/src/mesa/drivers/dri/i965/intel_tiled_memcpy.h
>index 3ff0d71..9dc1088 100644
>--- a/src/mesa/drivers/dri/i965/intel_tiled_memcpy.h
>+++ b/src/mesa/drivers/dri/i965/intel_tiled_memcpy.h
>@@ -55,7 +55,20 @@ tiled_to_linear(uint32_t xt1, uint32_t xt2,
>                 uint32_t tiling,
>                 mem_copy_fn mem_copy);
> 
>+/* Tells intel_get_memcpy() whether the memcpy() is
>+ *
>+ *  - an upload to the GPU with an aligned destination and a potentially
>+ *    unaligned source; or
>+ *  - a download from the GPU with an aligned source and a potentially
>+ *    unaligned destination.
>+ */
>+enum intel_memcpy_direction {
>+   INTEL_UPLOAD,
>+   INTEL_DOWNLOAD
>+};
>+
> bool intel_get_memcpy(mesa_format tiledFormat, GLenum format,
>-                      GLenum type, mem_copy_fn* mem_copy, uint32_t* cpp);
>+                      GLenum type, mem_copy_fn *mem_copy, uint32_t *cpp,
>+                      enum intel_memcpy_direction direction);
> 
> #endif /* INTEL_TILED_MEMCPY */
>-- 
>2.0.5
>

Actions: View | Splinter Review

Attachments on bug 89416: 113962 | 113963 | 114014

Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.