[Mesa-dev,v2] i965: Allow intel_try_pbo_upload for 3D and array textures

Submitted by Neil Roberts on Dec. 23, 2014, 12:32 a.m.

Details

Message ID 1419294739-10029-1-git-send-email-neil@linux.intel.com
State New
Headers show

Not browsing as part of any series.

Commit Message

Neil Roberts Dec. 23, 2014, 12:32 a.m.
I just realised I made regular cube map textures stop working via the
blit path with this patch. Here is a v2 which just adds
GL_TEXTURE_CUBE_MAP to the switch in intel_try_pbo_upload. I've tested
that it still works with a hacky tweak to the piglit test case.

------- >8 --------------- (use git am --scissors to automatically chop here)

intel_try_pbo_upload now iterates over each slice of the uploaded data and
and does a separate blit for each image. This copies in some fiddly details
store_texsubimage in order to handle the image stride correctly for 1D array
textures.
---
 src/mesa/drivers/dri/i965/intel_tex.h          |   4 +-
 src/mesa/drivers/dri/i965/intel_tex_image.c    | 116 ++++++++++++++++++-------
 src/mesa/drivers/dri/i965/intel_tex_subimage.c |   7 +-
 3 files changed, 88 insertions(+), 39 deletions(-)

Patch hide | download patch | download mbox

diff --git a/src/mesa/drivers/dri/i965/intel_tex.h b/src/mesa/drivers/dri/i965/intel_tex.h
index dfe1b03..f941d26 100644
--- a/src/mesa/drivers/dri/i965/intel_tex.h
+++ b/src/mesa/drivers/dri/i965/intel_tex.h
@@ -71,8 +71,8 @@  intel_texsubimage_tiled_memcpy(struct gl_context *ctx,
 bool
 intel_try_pbo_upload(struct gl_context *ctx,
                      struct gl_texture_image *image,
-                     GLint xoffset, GLint yoffset,
-                     GLsizei width, GLsizei height,
+                     GLint xoffset, GLint yoffset, GLint zoffset,
+                     GLsizei width, GLsizei height, GLsizei depth,
                      const struct gl_pixelstore_attrib *unpack,
                      GLenum format, GLenum type, const void *pixels,
                      bool for_glTexImage);
diff --git a/src/mesa/drivers/dri/i965/intel_tex_image.c b/src/mesa/drivers/dri/i965/intel_tex_image.c
index b1b49b9..6b8b953 100644
--- a/src/mesa/drivers/dri/i965/intel_tex_image.c
+++ b/src/mesa/drivers/dri/i965/intel_tex_image.c
@@ -88,8 +88,8 @@  intel_miptree_create_for_teximage(struct brw_context *brw,
 bool
 intel_try_pbo_upload(struct gl_context *ctx,
                      struct gl_texture_image *image,
-                     GLint xoffset, GLint yoffset,
-                     GLsizei width, GLsizei height,
+                     GLint xoffset, GLint yoffset, GLint zoffset,
+                     GLsizei width, GLsizei height, GLsizei depth,
                      const struct gl_pixelstore_attrib *unpack,
                      GLenum format, GLenum type, const void *pixels,
                      bool for_glTexImage)
@@ -99,6 +99,7 @@  intel_try_pbo_upload(struct gl_context *ctx,
    struct intel_buffer_object *pbo = intel_buffer_object(unpack->BufferObj);
    GLuint src_offset;
    drm_intel_bo *src_buffer;
+   int i;
 
    if (!_mesa_is_bufferobj(unpack->BufferObj))
       return false;
@@ -126,47 +127,96 @@  intel_try_pbo_upload(struct gl_context *ctx,
       return false;
    }
 
-   if (image->TexObject->Target == GL_TEXTURE_1D_ARRAY ||
-       image->TexObject->Target == GL_TEXTURE_2D_ARRAY) {
-      DBG("%s: no support for array textures\n", __FUNCTION__);
-      return false;
-   }
-
    int src_stride =
       _mesa_image_row_stride(unpack, width, format, type);
+   int dims, num_slices = 1, slice_offset = 0;
+   int src_image_stride = 0;
+
+   switch (image->TexObject->Target) {
+   case GL_TEXTURE_2D:
+   case GL_TEXTURE_RECTANGLE:
+   case GL_TEXTURE_CUBE_MAP:
+      /* one image slice, nothing special needs to be done */
+      dims = 2;
+      src_image_stride = src_stride * height;
+      break;
+   case GL_TEXTURE_1D:
+      dims = 1;
+      src_image_stride = src_stride * height;
+      break;
+   case GL_TEXTURE_1D_ARRAY:
+      num_slices = height;
+      slice_offset = yoffset;
+      height = 1;
+      yoffset = 0;
+      dims = 2;
+      src_image_stride = src_stride;
+      break;
+   case GL_TEXTURE_2D_ARRAY:
+      num_slices = depth;
+      slice_offset = zoffset;
+      depth = 1;
+      zoffset = 0;
+      dims = 3;
+      src_image_stride = _mesa_image_image_stride(unpack, width, height,
+                                                  format, type);
+      break;
+   case GL_TEXTURE_3D:
+      num_slices = depth;
+      slice_offset = zoffset;
+      dims = 3;
+      src_image_stride = _mesa_image_image_stride(unpack, width, height,
+                                                  format, type);
+      break;
+   case GL_TEXTURE_CUBE_MAP_ARRAY:
+      num_slices = depth;
+      slice_offset = zoffset;
+      dims = 3;
+      src_image_stride = _mesa_image_image_stride(unpack, width, height,
+                                                  format, type);
+      break;
+   default:
+      return false;
+   }
 
    /* note: potential 64-bit ptr to 32-bit int cast */
    src_offset = (GLuint) (unsigned long) pixels;
-   src_offset += _mesa_image_offset(2,
+   src_offset += _mesa_image_offset(dims,
                                     unpack,
                                     width, height,
                                     format, type,
                                     0, 0, 0 /* img/row/column */);
-   src_buffer = intel_bufferobj_buffer(brw, pbo,
-                                       src_offset, src_stride * height);
 
-   struct intel_mipmap_tree *pbo_mt =
-      intel_miptree_create_for_bo(brw,
-                                  src_buffer,
-                                  intelImage->mt->format,
-                                  src_offset,
-                                  width, height,
-                                  src_stride);
-   if (!pbo_mt)
-      return false;
+   for (i = 0; i < num_slices; i++) {
+      src_buffer = intel_bufferobj_buffer(brw, pbo,
+                                          src_offset, src_stride * height);
+
+      struct intel_mipmap_tree *pbo_mt =
+         intel_miptree_create_for_bo(brw,
+                                     src_buffer,
+                                     intelImage->mt->format,
+                                     src_offset,
+                                     width, height,
+                                     src_stride);
+      if (!pbo_mt)
+         return false;
+
+      if (!intel_miptree_blit(brw,
+                              pbo_mt, 0, 0,
+                              0, 0, false,
+                              intelImage->mt, image->Level,
+                              slice_offset + i + image->Face,
+                              xoffset, yoffset, false,
+                              width, height, GL_COPY)) {
+         DBG("%s: blit failed\n", __FUNCTION__);
+         intel_miptree_release(&pbo_mt);
+         return false;
+      }
 
-   if (!intel_miptree_blit(brw,
-                           pbo_mt, 0, 0,
-                           0, 0, false,
-                           intelImage->mt, image->Level, image->Face,
-                           xoffset, yoffset, false,
-                           width, height, GL_COPY)) {
-      DBG("%s: blit failed\n", __FUNCTION__);
       intel_miptree_release(&pbo_mt);
-      return false;
-   }
 
-   intel_miptree_release(&pbo_mt);
+      src_offset += src_image_stride;
+   }
 
    DBG("%s: success\n", __FUNCTION__);
    return true;
@@ -199,11 +249,11 @@  intelTexImage(struct gl_context * ctx,
 
    /* Attempt to use the blitter for PBO image uploads.
     */
-   if (dims <= 2 &&
-       intel_try_pbo_upload(ctx, texImage,
-                            0, 0, /* x,y offsets */
+   if (intel_try_pbo_upload(ctx, texImage,
+                            0, 0, 0, /* x,y,z offsets */
                             texImage->Width,
                             texImage->Height,
+                            texImage->Depth,
                             unpack, format, type, pixels,
                             true /*for_glTexImage*/)) {
       return;
diff --git a/src/mesa/drivers/dri/i965/intel_tex_subimage.c b/src/mesa/drivers/dri/i965/intel_tex_subimage.c
index dde1591..692e3ec 100644
--- a/src/mesa/drivers/dri/i965/intel_tex_subimage.c
+++ b/src/mesa/drivers/dri/i965/intel_tex_subimage.c
@@ -693,10 +693,9 @@  intelTexSubImage(struct gl_context * ctx,
 
    /* Attempt to use the blitter for PBO image uploads.
     */
-   if (dims <= 2 &&
-       intel_try_pbo_upload(ctx, texImage,
-                            xoffset, yoffset,
-                            width, height,
+   if (intel_try_pbo_upload(ctx, texImage,
+                            xoffset, yoffset, zoffset,
+                            width, height, depth,
                             packing, format, type, pixels,
                             false /*for_glTexImage*/)) {
       return;

Comments

On Mon, Dec 22, 2014 at 4:32 PM, Neil Roberts <neil@linux.intel.com> wrote:

> I just realised I made regular cube map textures stop working via the
> blit path with this patch. Here is a v2 which just adds
> GL_TEXTURE_CUBE_MAP to the switch in intel_try_pbo_upload. I've tested
> that it still works with a hacky tweak to the piglit test case.
>
> ------- >8 --------------- (use git am --scissors to automatically chop
> here)
>
> intel_try_pbo_upload now iterates over each slice of the uploaded data and
> and does a separate blit for each image. This copies in some fiddly details
> store_texsubimage in order to handle the image stride correctly for 1D
> array
> textures.
> ---
>  src/mesa/drivers/dri/i965/intel_tex.h          |   4 +-
>  src/mesa/drivers/dri/i965/intel_tex_image.c    | 116
> ++++++++++++++++++-------
>  src/mesa/drivers/dri/i965/intel_tex_subimage.c |   7 +-
>  3 files changed, 88 insertions(+), 39 deletions(-)
>
> diff --git a/src/mesa/drivers/dri/i965/intel_tex.h
> b/src/mesa/drivers/dri/i965/intel_tex.h
> index dfe1b03..f941d26 100644
> --- a/src/mesa/drivers/dri/i965/intel_tex.h
> +++ b/src/mesa/drivers/dri/i965/intel_tex.h
> @@ -71,8 +71,8 @@ intel_texsubimage_tiled_memcpy(struct gl_context *ctx,
>  bool
>  intel_try_pbo_upload(struct gl_context *ctx,
>                       struct gl_texture_image *image,
> -                     GLint xoffset, GLint yoffset,
> -                     GLsizei width, GLsizei height,
> +                     GLint xoffset, GLint yoffset, GLint zoffset,
> +                     GLsizei width, GLsizei height, GLsizei depth,
>                       const struct gl_pixelstore_attrib *unpack,
>                       GLenum format, GLenum type, const void *pixels,
>                       bool for_glTexImage);
> diff --git a/src/mesa/drivers/dri/i965/intel_tex_image.c
> b/src/mesa/drivers/dri/i965/intel_tex_image.c
> index b1b49b9..6b8b953 100644
> --- a/src/mesa/drivers/dri/i965/intel_tex_image.c
> +++ b/src/mesa/drivers/dri/i965/intel_tex_image.c
> @@ -88,8 +88,8 @@ intel_miptree_create_for_teximage(struct brw_context
> *brw,
>  bool
>  intel_try_pbo_upload(struct gl_context *ctx,
>                       struct gl_texture_image *image,
> -                     GLint xoffset, GLint yoffset,
> -                     GLsizei width, GLsizei height,
> +                     GLint xoffset, GLint yoffset, GLint zoffset,
> +                     GLsizei width, GLsizei height, GLsizei depth,
>                       const struct gl_pixelstore_attrib *unpack,
>                       GLenum format, GLenum type, const void *pixels,
>                       bool for_glTexImage)
> @@ -99,6 +99,7 @@ intel_try_pbo_upload(struct gl_context *ctx,
>     struct intel_buffer_object *pbo =
> intel_buffer_object(unpack->BufferObj);
>     GLuint src_offset;
>     drm_intel_bo *src_buffer;
> +   int i;
>
>     if (!_mesa_is_bufferobj(unpack->BufferObj))
>        return false;
> @@ -126,47 +127,96 @@ intel_try_pbo_upload(struct gl_context *ctx,
>        return false;
>     }
>
> -   if (image->TexObject->Target == GL_TEXTURE_1D_ARRAY ||
> -       image->TexObject->Target == GL_TEXTURE_2D_ARRAY) {
> -      DBG("%s: no support for array textures\n", __FUNCTION__);
> -      return false;
> -   }
> -
>     int src_stride =
>        _mesa_image_row_stride(unpack, width, format, type);
> +   int dims, num_slices = 1, slice_offset = 0;
> +   int src_image_stride = 0;
> +
> +   switch (image->TexObject->Target) {
> +   case GL_TEXTURE_2D:
> +   case GL_TEXTURE_RECTANGLE:
> +   case GL_TEXTURE_CUBE_MAP:
> +      /* one image slice, nothing special needs to be done */
> +      dims = 2;
> +      src_image_stride = src_stride * height;
>

Any reason you're not using _mesa_image_image_stride here?


> +      break;
> +   case GL_TEXTURE_1D:
> +      dims = 1;
> +      src_image_stride = src_stride * height;
>

or here?


> +      break;
> +   case GL_TEXTURE_1D_ARRAY:
> +      num_slices = height;
> +      slice_offset = yoffset;
> +      height = 1;
> +      yoffset = 0;
> +      dims = 2;
> +      src_image_stride = src_stride;
>

or here?


> +      break;
> +   case GL_TEXTURE_2D_ARRAY:
> +      num_slices = depth;
> +      slice_offset = zoffset;
> +      depth = 1;
> +      zoffset = 0;
> +      dims = 3;
> +      src_image_stride = _mesa_image_image_stride(unpack, width, height,
> +                                                  format, type);
> +      break;
> +   case GL_TEXTURE_3D:
> +      num_slices = depth;
> +      slice_offset = zoffset;
> +      dims = 3;
> +      src_image_stride = _mesa_image_image_stride(unpack, width, height,
> +                                                  format, type);
> +      break;
> +   case GL_TEXTURE_CUBE_MAP_ARRAY:
> +      num_slices = depth;
> +      slice_offset = zoffset;
> +      dims = 3;
> +      src_image_stride = _mesa_image_image_stride(unpack, width, height,
> +                                                  format, type);
> +      break;
> +   default:
> +      return false;
> +   }
>

In general, I find this switch statement a big awkward.  You could just
have dims passed in and slice_offset and num_slices seem to be the same as
zoffset and depth.  The only exception here being 1-D array textures which
are a bit silly and you would have to move the yoffset and height into
zoffset and depth, but I think I'd prefer that to the giant switch.


>
>     /* note: potential 64-bit ptr to 32-bit int cast */
>     src_offset = (GLuint) (unsigned long) pixels;
> -   src_offset += _mesa_image_offset(2,
> +   src_offset += _mesa_image_offset(dims,
>                                      unpack,
>                                      width, height,
>                                      format, type,
>                                      0, 0, 0 /* img/row/column */);
> -   src_buffer = intel_bufferobj_buffer(brw, pbo,
> -                                       src_offset, src_stride * height);
>
> -   struct intel_mipmap_tree *pbo_mt =
> -      intel_miptree_create_for_bo(brw,
> -                                  src_buffer,
> -                                  intelImage->mt->format,
> -                                  src_offset,
> -                                  width, height,
> -                                  src_stride);
> -   if (!pbo_mt)
> -      return false;
> +   for (i = 0; i < num_slices; i++) {
> +      src_buffer = intel_bufferobj_buffer(brw, pbo,
> +                                          src_offset, src_stride *
> height);
> +
> +      struct intel_mipmap_tree *pbo_mt =
> +         intel_miptree_create_for_bo(brw,
> +                                     src_buffer,
> +                                     intelImage->mt->format,
> +                                     src_offset,
> +                                     width, height,
> +                                     src_stride);
> +      if (!pbo_mt)
> +         return false;
> +
> +      if (!intel_miptree_blit(brw,
> +                              pbo_mt, 0, 0,
> +                              0, 0, false,
> +                              intelImage->mt, image->Level,
> +                              slice_offset + i + image->Face,
> +                              xoffset, yoffset, false,
> +                              width, height, GL_COPY)) {
> +         DBG("%s: blit failed\n", __FUNCTION__);
> +         intel_miptree_release(&pbo_mt);
> +         return false;
> +      }
>
> -   if (!intel_miptree_blit(brw,
> -                           pbo_mt, 0, 0,
> -                           0, 0, false,
> -                           intelImage->mt, image->Level, image->Face,
> -                           xoffset, yoffset, false,
> -                           width, height, GL_COPY)) {
> -      DBG("%s: blit failed\n", __FUNCTION__);
>        intel_miptree_release(&pbo_mt);
> -      return false;
> -   }
>
> -   intel_miptree_release(&pbo_mt);
> +      src_offset += src_image_stride;
> +   }
>
>     DBG("%s: success\n", __FUNCTION__);
>     return true;
> @@ -199,11 +249,11 @@ intelTexImage(struct gl_context * ctx,
>
>     /* Attempt to use the blitter for PBO image uploads.
>      */
> -   if (dims <= 2 &&
> -       intel_try_pbo_upload(ctx, texImage,
> -                            0, 0, /* x,y offsets */
> +   if (intel_try_pbo_upload(ctx, texImage,
> +                            0, 0, 0, /* x,y,z offsets */
>                              texImage->Width,
>                              texImage->Height,
> +                            texImage->Depth,
>                              unpack, format, type, pixels,
>                              true /*for_glTexImage*/)) {
>        return;
> diff --git a/src/mesa/drivers/dri/i965/intel_tex_subimage.c
> b/src/mesa/drivers/dri/i965/intel_tex_subimage.c
> index dde1591..692e3ec 100644
> --- a/src/mesa/drivers/dri/i965/intel_tex_subimage.c
> +++ b/src/mesa/drivers/dri/i965/intel_tex_subimage.c
> @@ -693,10 +693,9 @@ intelTexSubImage(struct gl_context * ctx,
>
>     /* Attempt to use the blitter for PBO image uploads.
>      */
> -   if (dims <= 2 &&
> -       intel_try_pbo_upload(ctx, texImage,
> -                            xoffset, yoffset,
> -                            width, height,
> +   if (intel_try_pbo_upload(ctx, texImage,
> +                            xoffset, yoffset, zoffset,
> +                            width, height, depth,
>                              packing, format, type, pixels,
>                              false /*for_glTexImage*/)) {
>        return;
> --
> 1.9.3
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
>