[2/4] i965: Move buffer texture size calculation into a common helper function.

Submitted by Francisco Jerez on March 19, 2018, 6:26 p.m.

Details

Message ID 20180319182659.22905-2-currojerez@riseup.net
State New
Headers show
Series "Series without cover letter" ( rev: 2 1 ) in Mesa

Not browsing as part of any series.

Commit Message

Francisco Jerez March 19, 2018, 6:26 p.m.
The buffer texture size calculations (should be easy enough, right?)
are repeated in three different places, each of them subtly broken in
a different way.  E.g. the image load/store path was never fixed to
clamp to MaxTextureBufferSize, and none of them are taking into
account the buffer offset correctly.  It's easier to fix it all in one
place.

Cc: mesa-stable@lists.freedesktop.org
---
 src/mesa/drivers/dri/i965/brw_wm_surface_state.c | 54 ++++++++++++++----------
 1 file changed, 32 insertions(+), 22 deletions(-)

Patch hide | download patch | download mbox

diff --git a/src/mesa/drivers/dri/i965/brw_wm_surface_state.c b/src/mesa/drivers/dri/i965/brw_wm_surface_state.c
index caa92d7d878..ed4def9046e 100644
--- a/src/mesa/drivers/dri/i965/brw_wm_surface_state.c
+++ b/src/mesa/drivers/dri/i965/brw_wm_surface_state.c
@@ -629,26 +629,14 @@  brw_emit_buffer_surface_state(struct brw_context *brw,
                          .mocs = brw_get_bo_mocs(devinfo, bo));
 }
 
-void
-brw_update_buffer_texture_surface(struct gl_context *ctx,
-                                  unsigned unit,
-                                  uint32_t *surf_offset)
+static unsigned
+buffer_texture_range_size(struct brw_context *brw,
+                          struct gl_texture_object *obj)
 {
-   struct brw_context *brw = brw_context(ctx);
-   struct gl_texture_object *tObj = ctx->Texture.Unit[unit]._Current;
-   struct intel_buffer_object *intel_obj =
-      intel_buffer_object(tObj->BufferObject);
-   uint32_t size = tObj->BufferSize;
-   struct brw_bo *bo = NULL;
-   mesa_format format = tObj->_BufferObjectFormat;
-   const enum isl_format isl_format = brw_isl_format_for_mesa_format(format);
-   int texel_size = _mesa_get_format_bytes(format);
-
-   if (intel_obj) {
-      size = MIN2(size, intel_obj->Base.Size);
-      bo = intel_bufferobj_buffer(brw, intel_obj, tObj->BufferOffset, size,
-                                  false);
-   }
+   assert(obj->Target == GL_TEXTURE_BUFFER);
+   const unsigned texel_size = _mesa_get_format_bytes(obj->_BufferObjectFormat);
+   const unsigned buffer_size = (!obj->BufferObject ? 0 :
+                                 obj->BufferObject->Size);
 
    /* The ARB_texture_buffer_specification says:
     *
@@ -666,7 +654,28 @@  brw_update_buffer_texture_surface(struct gl_context *ctx,
     * so that when ISL divides by stride to obtain the number of texels, that
     * texel count is clamped to MAX_TEXTURE_BUFFER_SIZE.
     */
-   size = MIN2(size, ctx->Const.MaxTextureBufferSize * (unsigned) texel_size);
+   return MIN3((unsigned)obj->BufferSize, buffer_size,
+               brw->ctx.Const.MaxTextureBufferSize * texel_size);
+}
+
+void
+brw_update_buffer_texture_surface(struct gl_context *ctx,
+                                  unsigned unit,
+                                  uint32_t *surf_offset)
+{
+   struct brw_context *brw = brw_context(ctx);
+   struct gl_texture_object *tObj = ctx->Texture.Unit[unit]._Current;
+   struct intel_buffer_object *intel_obj =
+      intel_buffer_object(tObj->BufferObject);
+   const unsigned size = buffer_texture_range_size(brw, tObj);
+   struct brw_bo *bo = NULL;
+   mesa_format format = tObj->_BufferObjectFormat;
+   const enum isl_format isl_format = brw_isl_format_for_mesa_format(format);
+   int texel_size = _mesa_get_format_bytes(format);
+
+   if (intel_obj)
+      bo = intel_bufferobj_buffer(brw, intel_obj, tObj->BufferOffset, size,
+                                  false);
 
    if (isl_format == ISL_FORMAT_UNSUPPORTED) {
       _mesa_problem(NULL, "bad format %s for texture buffer\n",
@@ -1468,7 +1477,7 @@  update_buffer_image_param(struct brw_context *brw,
                           struct brw_image_param *param)
 {
    struct gl_buffer_object *obj = u->TexObj->BufferObject;
-   const uint32_t size = MIN2((uint32_t)u->TexObj->BufferSize, obj->Size);
+   const unsigned size = buffer_texture_range_size(brw, u->TexObj);
    update_default_image_param(brw, u, surface_idx, param);
 
    param->size[0] = size / _mesa_get_format_bytes(u->_ActualFormat);
@@ -1504,10 +1513,11 @@  update_image_surface(struct brw_context *brw,
             intel_buffer_object(obj->BufferObject);
          const unsigned texel_size = (format == ISL_FORMAT_RAW ? 1 :
                                       _mesa_get_format_bytes(u->_ActualFormat));
+         const unsigned buffer_size = buffer_texture_range_size(brw, obj);
 
          brw_emit_buffer_surface_state(
             brw, surf_offset, intel_obj->buffer, obj->BufferOffset,
-            format, intel_obj->Base.Size, texel_size,
+            format, buffer_size, texel_size,
             access != GL_READ_ONLY ? RELOC_WRITE : 0);
 
          update_buffer_image_param(brw, u, surface_idx, param);

Comments

On Mon, Mar 19, 2018 at 11:26:57AM -0700, Francisco Jerez wrote:
> The buffer texture size calculations (should be easy enough, right?)
> are repeated in three different places, each of them subtly broken in
> a different way.  E.g. the image load/store path was never fixed to
> clamp to MaxTextureBufferSize, and none of them are taking into
> account the buffer offset correctly.  It's easier to fix it all in one
> place.
> 
> Cc: mesa-stable@lists.freedesktop.org

I created a bug so that we remember to add test coverage for this issue:
https://bugs.freedesktop.org/show_bug.cgi?id=106481

> ---
>  src/mesa/drivers/dri/i965/brw_wm_surface_state.c | 54 ++++++++++++++----------
>  1 file changed, 32 insertions(+), 22 deletions(-)

This patch is
Reviewed-by: Nanley Chery <nanley.g.chery@intel.com>

> 
> diff --git a/src/mesa/drivers/dri/i965/brw_wm_surface_state.c b/src/mesa/drivers/dri/i965/brw_wm_surface_state.c
> index caa92d7d878..ed4def9046e 100644
> --- a/src/mesa/drivers/dri/i965/brw_wm_surface_state.c
> +++ b/src/mesa/drivers/dri/i965/brw_wm_surface_state.c
> @@ -629,26 +629,14 @@ brw_emit_buffer_surface_state(struct brw_context *brw,
>                           .mocs = brw_get_bo_mocs(devinfo, bo));
>  }
>  
> -void
> -brw_update_buffer_texture_surface(struct gl_context *ctx,
> -                                  unsigned unit,
> -                                  uint32_t *surf_offset)
> +static unsigned
> +buffer_texture_range_size(struct brw_context *brw,
> +                          struct gl_texture_object *obj)
>  {
> -   struct brw_context *brw = brw_context(ctx);
> -   struct gl_texture_object *tObj = ctx->Texture.Unit[unit]._Current;
> -   struct intel_buffer_object *intel_obj =
> -      intel_buffer_object(tObj->BufferObject);
> -   uint32_t size = tObj->BufferSize;
> -   struct brw_bo *bo = NULL;
> -   mesa_format format = tObj->_BufferObjectFormat;
> -   const enum isl_format isl_format = brw_isl_format_for_mesa_format(format);
> -   int texel_size = _mesa_get_format_bytes(format);
> -
> -   if (intel_obj) {
> -      size = MIN2(size, intel_obj->Base.Size);
> -      bo = intel_bufferobj_buffer(brw, intel_obj, tObj->BufferOffset, size,
> -                                  false);
> -   }
> +   assert(obj->Target == GL_TEXTURE_BUFFER);
> +   const unsigned texel_size = _mesa_get_format_bytes(obj->_BufferObjectFormat);
> +   const unsigned buffer_size = (!obj->BufferObject ? 0 :
> +                                 obj->BufferObject->Size);
>  
>     /* The ARB_texture_buffer_specification says:
>      *
> @@ -666,7 +654,28 @@ brw_update_buffer_texture_surface(struct gl_context *ctx,
>      * so that when ISL divides by stride to obtain the number of texels, that
>      * texel count is clamped to MAX_TEXTURE_BUFFER_SIZE.
>      */
> -   size = MIN2(size, ctx->Const.MaxTextureBufferSize * (unsigned) texel_size);
> +   return MIN3((unsigned)obj->BufferSize, buffer_size,
> +               brw->ctx.Const.MaxTextureBufferSize * texel_size);
> +}
> +
> +void
> +brw_update_buffer_texture_surface(struct gl_context *ctx,
> +                                  unsigned unit,
> +                                  uint32_t *surf_offset)
> +{
> +   struct brw_context *brw = brw_context(ctx);
> +   struct gl_texture_object *tObj = ctx->Texture.Unit[unit]._Current;
> +   struct intel_buffer_object *intel_obj =
> +      intel_buffer_object(tObj->BufferObject);
> +   const unsigned size = buffer_texture_range_size(brw, tObj);
> +   struct brw_bo *bo = NULL;
> +   mesa_format format = tObj->_BufferObjectFormat;
> +   const enum isl_format isl_format = brw_isl_format_for_mesa_format(format);
> +   int texel_size = _mesa_get_format_bytes(format);
> +
> +   if (intel_obj)
> +      bo = intel_bufferobj_buffer(brw, intel_obj, tObj->BufferOffset, size,
> +                                  false);
>  
>     if (isl_format == ISL_FORMAT_UNSUPPORTED) {
>        _mesa_problem(NULL, "bad format %s for texture buffer\n",
> @@ -1468,7 +1477,7 @@ update_buffer_image_param(struct brw_context *brw,
>                            struct brw_image_param *param)
>  {
>     struct gl_buffer_object *obj = u->TexObj->BufferObject;
> -   const uint32_t size = MIN2((uint32_t)u->TexObj->BufferSize, obj->Size);
> +   const unsigned size = buffer_texture_range_size(brw, u->TexObj);
>     update_default_image_param(brw, u, surface_idx, param);
>  
>     param->size[0] = size / _mesa_get_format_bytes(u->_ActualFormat);
> @@ -1504,10 +1513,11 @@ update_image_surface(struct brw_context *brw,
>              intel_buffer_object(obj->BufferObject);
>           const unsigned texel_size = (format == ISL_FORMAT_RAW ? 1 :
>                                        _mesa_get_format_bytes(u->_ActualFormat));
> +         const unsigned buffer_size = buffer_texture_range_size(brw, obj);
>  
>           brw_emit_buffer_surface_state(
>              brw, surf_offset, intel_obj->buffer, obj->BufferOffset,
> -            format, intel_obj->Base.Size, texel_size,
> +            format, buffer_size, texel_size,
>              access != GL_READ_ONLY ? RELOC_WRITE : 0);
>  
>           update_buffer_image_param(brw, u, surface_idx, param);
> -- 
> 2.16.1
>