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

[Nanley Chery]

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 at 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 at 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
>