[Mesa-dev] [PATCH 1/7] i965: Move texture buffer dispatch into single location

Francisco Jerez currojerez at riseup.net
Thu May 7 07:55:48 PDT 2015


"Pohjolainen, Topi" <topi.pohjolainen at intel.com> writes:

> On Thu, May 07, 2015 at 05:15:35PM +0300, Francisco Jerez wrote:
>> From: Topi Pohjolainen <topi.pohjolainen at intel.com>
>> 
>> All generations do the same exact dispatch and it could be
>> therefore done in the hardware independent stage.
>> 
>> Reviewed-by: Matt Turner <mattst88 at gmail.com>
>> Reviewed-by: Kenneth Graunke <kenneth at whitecape.org>
>> Signed-off-by: Topi Pohjolainen <topi.pohjolainen at intel.com>
>> [ Francisco Jerez: Non-trivial rebase. ]
>> Reviewed-by: Francisco Jerez <currojerez at riseup.net>
>> ---
>>  src/mesa/drivers/dri/i965/brw_context.h           |  3 -
>>  src/mesa/drivers/dri/i965/brw_wm_surface_state.c  | 31 ++++++----
>>  src/mesa/drivers/dri/i965/gen7_wm_surface_state.c | 69 +++++++++++------------
>>  src/mesa/drivers/dri/i965/gen8_surface_state.c    | 67 ++++++++++------------
>>  4 files changed, 83 insertions(+), 87 deletions(-)
>> 
>> diff --git a/src/mesa/drivers/dri/i965/brw_context.h b/src/mesa/drivers/dri/i965/brw_context.h
>> index 2fcdcfa..a6282f4 100644
>> --- a/src/mesa/drivers/dri/i965/brw_context.h
>> +++ b/src/mesa/drivers/dri/i965/brw_context.h
>> @@ -1698,9 +1698,6 @@ void brw_create_constant_surface(struct brw_context *brw,
>>                                   uint32_t size,
>>                                   uint32_t *out_offset,
>>                                   bool dword_pitch);
>> -void brw_update_buffer_texture_surface(struct gl_context *ctx,
>> -                                       unsigned unit,
>> -                                       uint32_t *surf_offset);
>>  void
>>  brw_update_sol_surface(struct brw_context *brw,
>>                         struct gl_buffer_object *buffer_obj,
>> 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 160dd2f..2b8040c 100644
>> --- a/src/mesa/drivers/dri/i965/brw_wm_surface_state.c
>> +++ b/src/mesa/drivers/dri/i965/brw_wm_surface_state.c
>> @@ -274,10 +274,10 @@ gen4_emit_buffer_surface_state(struct brw_context *brw,
>>     }
>>  }
>>  
>> -void
>> -brw_update_buffer_texture_surface(struct gl_context *ctx,
>> -                                  unsigned unit,
>> -                                  uint32_t *surf_offset)
>> +static void
>> +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;
>> @@ -320,12 +320,6 @@ brw_update_texture_surface(struct gl_context *ctx,
>>     struct gl_sampler_object *sampler = _mesa_get_samplerobj(ctx, unit);
>>     uint32_t *surf;
>>  
>> -   /* BRW_NEW_TEXTURE_BUFFER */
>> -   if (tObj->Target == GL_TEXTURE_BUFFER) {
>> -      brw_update_buffer_texture_surface(ctx, unit, surf_offset);
>> -      return;
>> -   }
>> -
>>     surf = brw_state_batch(brw, AUB_TRACE_SURFACE_STATE,
>>  			  6 * 4, 32, surf_offset);
>>  
>> @@ -795,6 +789,21 @@ const struct brw_tracked_state gen6_renderbuffer_surfaces = {
>>     .emit = update_renderbuffer_surfaces,
>>  };
>>  
>> +static void
>> +update_texture_surface(struct gl_context *ctx,
>> +                       unsigned unit,
>> +                       uint32_t *surf_offset,
>> +                       bool for_gather)
>> +{
>> +   struct brw_context *brw = brw_context(ctx);
>> +   struct gl_texture_object *obj = ctx->Texture.Unit[unit]._Current;
>> +
>> +   if (obj->Target == GL_TEXTURE_BUFFER) {
>> +      update_buffer_texture_surface(ctx, unit, surf_offset);
>
> In order to avoid extra level of indentation I used the following. I would
> have preferred it here also.
>
>       if (obj->Target == GL_TEXTURE_BUFFER) {
>          update_buffer_texture_surface(ctx, unit, surf_offset);
>          return;
>       }
>
I kept this as an indented block because it's harmless IMHO and it
seemed a somewhat lesser evil than:
1/ Define all texture-specific variables (i.e. things that are not
   applicable to buffer textures, including some pointer dereferences)
   at the top level, which is what you did, but it seemed a bit dodgy.
2/ Mix statements and declarations. (Granted, this file is likely
   already relying on other C99 features, so it wouldn't matter in
   practice, it's just a codestyle itch)
3/ Declare stuff and leave it uninitialized until later.

That said, the reason was largely subjective, and I don't really have a
strong preference.  As you are still the author of this commit you're
free to format it as you wish, you can keep my R-b if you simply
reindent this function.

>> +   } else {
>> +      brw->vtbl.update_texture_surface(ctx, unit, surf_offset, for_gather);
>> +   }
>> +}
>>  
>>  static void
>>  update_stage_texture_surfaces(struct brw_context *brw,
>> @@ -824,7 +833,7 @@ update_stage_texture_surfaces(struct brw_context *brw,
>>  
>>           /* _NEW_TEXTURE */
>>           if (ctx->Texture.Unit[unit]._Current) {
>> -            brw->vtbl.update_texture_surface(ctx, unit, surf_offset + s, for_gather);
>> +            update_texture_surface(ctx, unit, surf_offset + s, for_gather);
>>           }
>>        }
>>     }
>> diff --git a/src/mesa/drivers/dri/i965/gen7_wm_surface_state.c b/src/mesa/drivers/dri/i965/gen7_wm_surface_state.c
>> index 15ab2b0..098b5c8 100644
>> --- a/src/mesa/drivers/dri/i965/gen7_wm_surface_state.c
>> +++ b/src/mesa/drivers/dri/i965/gen7_wm_surface_state.c
>> @@ -356,43 +356,38 @@ gen7_update_texture_surface(struct gl_context *ctx,
>>     struct brw_context *brw = brw_context(ctx);
>>     struct gl_texture_object *obj = ctx->Texture.Unit[unit]._Current;
>>  
>> -   if (obj->Target == GL_TEXTURE_BUFFER) {
>> -      brw_update_buffer_texture_surface(ctx, unit, surf_offset);
>> -
>> -   } else {
>> -      struct intel_texture_object *intel_obj = intel_texture_object(obj);
>> -      struct intel_mipmap_tree *mt = intel_obj->mt;
>> -      struct gl_sampler_object *sampler = _mesa_get_samplerobj(ctx, unit);
>> -      /* If this is a view with restricted NumLayers, then our effective depth
>> -       * is not just the miptree depth.
>> -       */
>> -      const unsigned depth = (obj->Immutable && obj->Target != GL_TEXTURE_3D ?
>> -                              obj->NumLayers : mt->logical_depth0);
>> -
>> -      /* Handling GL_ALPHA as a surface format override breaks 1.30+ style
>> -       * texturing functions that return a float, as our code generation always
>> -       * selects the .x channel (which would always be 0).
>> -       */
>> -      struct gl_texture_image *firstImage = obj->Image[0][obj->BaseLevel];
>> -      const bool alpha_depth = obj->DepthMode == GL_ALPHA &&
>> -         (firstImage->_BaseFormat == GL_DEPTH_COMPONENT ||
>> -          firstImage->_BaseFormat == GL_DEPTH_STENCIL);
>> -      const unsigned swizzle = (unlikely(alpha_depth) ? SWIZZLE_XYZW :
>> -                                brw_get_texture_swizzle(&brw->ctx, obj));
>> -
>> -      unsigned format = translate_tex_format(
>> -         brw, intel_obj->_Format, sampler->sRGBDecode);
>> -
>> -      if (for_gather && format == BRW_SURFACEFORMAT_R32G32_FLOAT)
>> -         format = BRW_SURFACEFORMAT_R32G32_FLOAT_LD;
>> -
>> -      gen7_emit_texture_surface_state(brw, mt, obj->Target,
>> -                                      obj->MinLayer, obj->MinLayer + depth,
>> -                                      obj->MinLevel + obj->BaseLevel,
>> -                                      obj->MinLevel + intel_obj->_MaxLevel + 1,
>> -                                      format, swizzle,
>> -                                      surf_offset, false, for_gather);
>> -   }
>> +   struct intel_texture_object *intel_obj = intel_texture_object(obj);
>> +   struct intel_mipmap_tree *mt = intel_obj->mt;
>> +   struct gl_sampler_object *sampler = _mesa_get_samplerobj(ctx, unit);
>> +   /* If this is a view with restricted NumLayers, then our effective depth
>> +    * is not just the miptree depth.
>> +    */
>> +   const unsigned depth = (obj->Immutable && obj->Target != GL_TEXTURE_3D ?
>> +                           obj->NumLayers : mt->logical_depth0);
>> +
>> +   /* Handling GL_ALPHA as a surface format override breaks 1.30+ style
>> +    * texturing functions that return a float, as our code generation always
>> +    * selects the .x channel (which would always be 0).
>> +    */
>> +   struct gl_texture_image *firstImage = obj->Image[0][obj->BaseLevel];
>> +   const bool alpha_depth = obj->DepthMode == GL_ALPHA &&
>> +      (firstImage->_BaseFormat == GL_DEPTH_COMPONENT ||
>> +       firstImage->_BaseFormat == GL_DEPTH_STENCIL);
>> +   const unsigned swizzle = (unlikely(alpha_depth) ? SWIZZLE_XYZW :
>> +                             brw_get_texture_swizzle(&brw->ctx, obj));
>> +
>> +   unsigned format = translate_tex_format(
>> +      brw, intel_obj->_Format, sampler->sRGBDecode);
>> +
>> +   if (for_gather && format == BRW_SURFACEFORMAT_R32G32_FLOAT)
>> +      format = BRW_SURFACEFORMAT_R32G32_FLOAT_LD;
>> +
>> +   gen7_emit_texture_surface_state(brw, mt, obj->Target,
>> +                                   obj->MinLayer, obj->MinLayer + depth,
>> +                                   obj->MinLevel + obj->BaseLevel,
>> +                                   obj->MinLevel + intel_obj->_MaxLevel + 1,
>> +                                   format, swizzle,
>> +                                   surf_offset, false, for_gather);
>>  }
>>  
>>  /**
>> diff --git a/src/mesa/drivers/dri/i965/gen8_surface_state.c b/src/mesa/drivers/dri/i965/gen8_surface_state.c
>> index d0c2d80..2f9b98e 100644
>> --- a/src/mesa/drivers/dri/i965/gen8_surface_state.c
>> +++ b/src/mesa/drivers/dri/i965/gen8_surface_state.c
>> @@ -255,44 +255,39 @@ gen8_update_texture_surface(struct gl_context *ctx,
>>     struct brw_context *brw = brw_context(ctx);
>>     struct gl_texture_object *obj = ctx->Texture.Unit[unit]._Current;
>>  
>> -   if (obj->Target == GL_TEXTURE_BUFFER) {
>> -      brw_update_buffer_texture_surface(ctx, unit, surf_offset);
>> +   struct gl_texture_image *firstImage = obj->Image[0][obj->BaseLevel];
>> +   struct intel_texture_object *intel_obj = intel_texture_object(obj);
>> +   struct intel_mipmap_tree *mt = intel_obj->mt;
>> +   struct gl_sampler_object *sampler = _mesa_get_samplerobj(ctx, unit);
>> +   /* If this is a view with restricted NumLayers, then our effective depth
>> +    * is not just the miptree depth.
>> +    */
>> +   const unsigned depth = (obj->Immutable && obj->Target != GL_TEXTURE_3D ?
>> +                           obj->NumLayers : mt->logical_depth0);
>>  
>> -   } else {
>> -      struct gl_texture_image *firstImage = obj->Image[0][obj->BaseLevel];
>> -      struct intel_texture_object *intel_obj = intel_texture_object(obj);
>> -      struct intel_mipmap_tree *mt = intel_obj->mt;
>> -      struct gl_sampler_object *sampler = _mesa_get_samplerobj(ctx, unit);
>> -      /* If this is a view with restricted NumLayers, then our effective depth
>> -       * is not just the miptree depth.
>> -       */
>> -      const unsigned depth = (obj->Immutable && obj->Target != GL_TEXTURE_3D ?
>> -                              obj->NumLayers : mt->logical_depth0);
>> -
>> -      /* Handling GL_ALPHA as a surface format override breaks 1.30+ style
>> -       * texturing functions that return a float, as our code generation always
>> -       * selects the .x channel (which would always be 0).
>> -       */
>> -      const bool alpha_depth = obj->DepthMode == GL_ALPHA &&
>> -         (firstImage->_BaseFormat == GL_DEPTH_COMPONENT ||
>> -          firstImage->_BaseFormat == GL_DEPTH_STENCIL);
>> -      const unsigned swizzle = (unlikely(alpha_depth) ? SWIZZLE_XYZW :
>> -                                brw_get_texture_swizzle(&brw->ctx, obj));
>> -
>> -      unsigned format = translate_tex_format(brw, intel_obj->_Format,
>> -                                             sampler->sRGBDecode);
>> -      if (obj->StencilSampling && firstImage->_BaseFormat == GL_DEPTH_STENCIL) {
>> -         mt = mt->stencil_mt;
>> -         format = BRW_SURFACEFORMAT_R8_UINT;
>> -      }
>> -
>> -      gen8_emit_texture_surface_state(brw, mt, obj->Target,
>> -                                      obj->MinLayer, obj->MinLayer + depth,
>> -                                      obj->MinLevel + obj->BaseLevel,
>> -                                      obj->MinLevel + intel_obj->_MaxLevel + 1,
>> -                                      format, swizzle, surf_offset,
>> -                                      false, for_gather);
>> +   /* Handling GL_ALPHA as a surface format override breaks 1.30+ style
>> +    * texturing functions that return a float, as our code generation always
>> +    * selects the .x channel (which would always be 0).
>> +    */
>> +   const bool alpha_depth = obj->DepthMode == GL_ALPHA &&
>> +      (firstImage->_BaseFormat == GL_DEPTH_COMPONENT ||
>> +       firstImage->_BaseFormat == GL_DEPTH_STENCIL);
>> +   const unsigned swizzle = (unlikely(alpha_depth) ? SWIZZLE_XYZW :
>> +                             brw_get_texture_swizzle(&brw->ctx, obj));
>> +
>> +   unsigned format = translate_tex_format(brw, intel_obj->_Format,
>> +                                          sampler->sRGBDecode);
>> +   if (obj->StencilSampling && firstImage->_BaseFormat == GL_DEPTH_STENCIL) {
>> +      mt = mt->stencil_mt;
>> +      format = BRW_SURFACEFORMAT_R8_UINT;
>>     }
>> +
>> +   gen8_emit_texture_surface_state(brw, mt, obj->Target,
>> +                                   obj->MinLayer, obj->MinLayer + depth,
>> +                                   obj->MinLevel + obj->BaseLevel,
>> +                                   obj->MinLevel + intel_obj->_MaxLevel + 1,
>> +                                   format, swizzle, surf_offset,
>> +                                   false, for_gather);
>>  }
>>  
>>  /**
>> -- 
>> 2.3.5
>> 
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 212 bytes
Desc: not available
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20150507/e42e0339/attachment-0001.sig>


More information about the mesa-dev mailing list