[Mesa-dev] [PATCH 14/23] i965: Make brw_update_sampler_state() use brw_emit_sampler_state().

Kenneth Graunke kenneth at whitecape.org
Wed Jul 30 11:45:07 PDT 2014


On Wednesday, July 30, 2014 11:38:28 AM Pohjolainen, Topi wrote:
> On Tue, Jul 29, 2014 at 04:29:19PM -0700, Kenneth Graunke wrote:
> > Instead of stuffing bits directly into the brw_sampler_state structure,
> > we now store them in local variables, then use brw_emit_sampler_state()
> > to assemble the packet.  This separates the decision about what values
> > to use from the actual packet emission, which makes the code more
> > reusable across generations.
> > 
> > Signed-off-by: Kenneth Graunke <kenneth at whitecape.org>
> 
> Apart from the few small nits looks good to me.
> 
> > ---
> >  src/mesa/drivers/dri/i965/brw_sampler_state.c | 206 +++++++++++---------------
> >  1 file changed, 90 insertions(+), 116 deletions(-)
> > 
> > diff --git a/src/mesa/drivers/dri/i965/brw_sampler_state.c b/src/mesa/drivers/dri/i965/brw_sampler_state.c
> > index c631b76..ae4694b 100644
> > --- a/src/mesa/drivers/dri/i965/brw_sampler_state.c
> > +++ b/src/mesa/drivers/dri/i965/brw_sampler_state.c
> > @@ -286,100 +286,107 @@ upload_default_color(struct brw_context *brw,
> >  static void
> >  brw_update_sampler_state(struct brw_context *brw,
> >                           int unit,
> > -                         struct brw_sampler_state *sampler,
> > +                         uint32_t *sampler_state,
> >                           uint32_t batch_offset_for_sampler_state)
> >  {
> >     struct gl_context *ctx = &brw->ctx;
> >     struct gl_texture_unit *texUnit = &ctx->Texture.Unit[unit];
> >     struct gl_texture_object *texObj = texUnit->_Current;
> > -   struct gl_sampler_object *gl_sampler = _mesa_get_samplerobj(ctx, unit);
> > -   bool using_nearest = false;
> > +   struct gl_sampler_object *sampler = _mesa_get_samplerobj(ctx, unit);
> 
> This is only used for reading, right? I guess we could declare it constant
> to make that clearer.

Done.  This required changing upload_default_color's signature to take a const struct gl_sampler_object * as well, but that's fine.  I did that in a separate patch just before this.

I made all the other changes as well.

> 
> > +
> > +   unsigned min_filter, mag_filter, mip_filter;
> > +   unsigned max_anisotropy = BRW_ANISORATIO_2;
> > +   unsigned address_rounding = 0;
> > +   unsigned wrap_s, wrap_t, wrap_r;
> > +   unsigned min_lod, max_lod, base_level;
> > +   unsigned shadow_function = 0;
> > +   int lod_bias;
> > +   bool non_normalized_coordinates = texObj->Target == GL_TEXTURE_RECTANGLE;
> 
> I would make this constant and also move it just before the final call.
> 
> >  
> >     /* These don't use samplers at all. */
> >     if (texObj->Target == GL_TEXTURE_BUFFER)
> >        return;
> >  
> > -   switch (gl_sampler->MinFilter) {
> > +   /* Select min and mip filters. */
> > +   switch (sampler->MinFilter) {
> >     case GL_NEAREST:
> > -      sampler->ss0.min_filter = BRW_MAPFILTER_NEAREST;
> > -      sampler->ss0.mip_filter = BRW_MIPFILTER_NONE;
> > -      using_nearest = true;
> > +      min_filter = BRW_MAPFILTER_NEAREST;
> > +      mip_filter = BRW_MIPFILTER_NONE;
> >        break;
> >     case GL_LINEAR:
> > -      sampler->ss0.min_filter = BRW_MAPFILTER_LINEAR;
> > -      sampler->ss0.mip_filter = BRW_MIPFILTER_NONE;
> > +      min_filter = BRW_MAPFILTER_LINEAR;
> > +      mip_filter = BRW_MIPFILTER_NONE;
> >        break;
> >     case GL_NEAREST_MIPMAP_NEAREST:
> > -      sampler->ss0.min_filter = BRW_MAPFILTER_NEAREST;
> > -      sampler->ss0.mip_filter = BRW_MIPFILTER_NEAREST;
> > +      min_filter = BRW_MAPFILTER_NEAREST;
> > +      mip_filter = BRW_MIPFILTER_NEAREST;
> >        break;
> >     case GL_LINEAR_MIPMAP_NEAREST:
> > -      sampler->ss0.min_filter = BRW_MAPFILTER_LINEAR;
> > -      sampler->ss0.mip_filter = BRW_MIPFILTER_NEAREST;
> > +      min_filter = BRW_MAPFILTER_LINEAR;
> > +      mip_filter = BRW_MIPFILTER_NEAREST;
> >        break;
> >     case GL_NEAREST_MIPMAP_LINEAR:
> > -      sampler->ss0.min_filter = BRW_MAPFILTER_NEAREST;
> > -      sampler->ss0.mip_filter = BRW_MIPFILTER_LINEAR;
> > +      min_filter = BRW_MAPFILTER_NEAREST;
> > +      mip_filter = BRW_MIPFILTER_LINEAR;
> >        break;
> >     case GL_LINEAR_MIPMAP_LINEAR:
> > -      sampler->ss0.min_filter = BRW_MAPFILTER_LINEAR;
> > -      sampler->ss0.mip_filter = BRW_MIPFILTER_LINEAR;
> > +      min_filter = BRW_MAPFILTER_LINEAR;
> > +      mip_filter = BRW_MIPFILTER_LINEAR;
> >        break;
> >     default:
> >        break;
> >     }
> >  
> > -   /* Set Anisotropy:
> > -    */
> > -   if (gl_sampler->MaxAnisotropy > 1.0) {
> > -      sampler->ss0.min_filter = BRW_MAPFILTER_ANISOTROPIC;
> > -      sampler->ss0.mag_filter = BRW_MAPFILTER_ANISOTROPIC;
> > +   /* Select mag filter. */
> > +   if (sampler->MagFilter == GL_LINEAR)
> > +      mag_filter = BRW_MAPFILTER_LINEAR;
> > +   else
> > +      mag_filter = BRW_MAPFILTER_NEAREST;
> >  
> > -      if (gl_sampler->MaxAnisotropy > 2.0) {
> > -	 sampler->ss3.max_aniso = MIN2((gl_sampler->MaxAnisotropy - 2) / 2,
> > -				       BRW_ANISORATIO_16);
> > -      }
> > -   }
> > -   else {
> > -      switch (gl_sampler->MagFilter) {
> > -      case GL_NEAREST:
> > -	 sampler->ss0.mag_filter = BRW_MAPFILTER_NEAREST;
> > -	 using_nearest = true;
> > -	 break;
> > -      case GL_LINEAR:
> > -	 sampler->ss0.mag_filter = BRW_MAPFILTER_LINEAR;
> > -	 break;
> > -      default:
> > -	 break;
> > +   /* Enable anisotropic filtering if desired. */
> > +   if (sampler->MaxAnisotropy > 1.0) {
> > +      min_filter = BRW_MAPFILTER_ANISOTROPIC;
> > +      mag_filter = BRW_MAPFILTER_ANISOTROPIC;
> > +
> > +      if (sampler->MaxAnisotropy > 2.0) {
> > +	 max_anisotropy =
> > +            MIN2((sampler->MaxAnisotropy - 2) / 2, BRW_ANISORATIO_16);
> >        }
> >     }
> >  
> > -   sampler->ss1.r_wrap_mode = translate_wrap_mode(brw, gl_sampler->WrapR,
> > -						  using_nearest);
> > -   sampler->ss1.s_wrap_mode = translate_wrap_mode(brw, gl_sampler->WrapS,
> > -						  using_nearest);
> > -   sampler->ss1.t_wrap_mode = translate_wrap_mode(brw, gl_sampler->WrapT,
> > -						  using_nearest);
> > +   /* Set address rounding bits if not using nearest filtering. */
> > +   if (min_filter != BRW_MAPFILTER_NEAREST) {
> > +      address_rounding |= BRW_ADDRESS_ROUNDING_ENABLE_U_MIN |
> > +                          BRW_ADDRESS_ROUNDING_ENABLE_V_MIN |
> > +                          BRW_ADDRESS_ROUNDING_ENABLE_R_MIN;
> > +   }
> > +   if (mag_filter != BRW_MAPFILTER_NEAREST) {
> > +      address_rounding |= BRW_ADDRESS_ROUNDING_ENABLE_U_MAG |
> > +                          BRW_ADDRESS_ROUNDING_ENABLE_V_MAG |
> > +                          BRW_ADDRESS_ROUNDING_ENABLE_R_MAG;
> > +   }
> >  
> > -   if (brw->gen >= 6 &&
> > -       sampler->ss0.min_filter != sampler->ss0.mag_filter)
> > -	sampler->ss0.min_mag_neq = 1;
> > +   bool either_nearest =
> > +      sampler->MinFilter == GL_NEAREST || sampler->MagFilter == GL_NEAREST;
> 
> Could be constant as well.
> 
> > +   wrap_s = translate_wrap_mode(brw, sampler->WrapS, either_nearest);
> > +   wrap_t = translate_wrap_mode(brw, sampler->WrapT, either_nearest);
> > +   wrap_r = translate_wrap_mode(brw, sampler->WrapR, either_nearest);
> >  
> > -   /* Cube-maps on 965 and later must use the same wrap mode for all 3
> > -    * coordinate dimensions.  Futher, only CUBE and CLAMP are valid.
> > -    */
> >     if (texObj->Target == GL_TEXTURE_CUBE_MAP ||
> >         texObj->Target == GL_TEXTURE_CUBE_MAP_ARRAY) {
> > -      if ((ctx->Texture.CubeMapSeamless || gl_sampler->CubeMapSeamless) &&
> > -	  (gl_sampler->MinFilter != GL_NEAREST ||
> > -	   gl_sampler->MagFilter != GL_NEAREST)) {
> > -	 sampler->ss1.r_wrap_mode = BRW_TEXCOORDMODE_CUBE;
> > -	 sampler->ss1.s_wrap_mode = BRW_TEXCOORDMODE_CUBE;
> > -	 sampler->ss1.t_wrap_mode = BRW_TEXCOORDMODE_CUBE;
> > +      /* Cube maps must use the same wrap mode for all three coordinate
> > +       * dimensions.  Prior to Haswell, only CUBE and CLAMP are valid.
> > +       */
> > +      if ((ctx->Texture.CubeMapSeamless || sampler->CubeMapSeamless) &&
> > +         (sampler->MinFilter != GL_NEAREST ||
> > +          sampler->MagFilter != GL_NEAREST)) {
> > +	 wrap_s = BRW_TEXCOORDMODE_CUBE;
> > +	 wrap_t = BRW_TEXCOORDMODE_CUBE;
> > +	 wrap_r = BRW_TEXCOORDMODE_CUBE;
> >        } else {
> > -	 sampler->ss1.r_wrap_mode = BRW_TEXCOORDMODE_CLAMP;
> > -	 sampler->ss1.s_wrap_mode = BRW_TEXCOORDMODE_CLAMP;
> > -	 sampler->ss1.t_wrap_mode = BRW_TEXCOORDMODE_CLAMP;
> > +	 wrap_s = BRW_TEXCOORDMODE_CLAMP;
> > +	 wrap_t = BRW_TEXCOORDMODE_CLAMP;
> > +	 wrap_r = BRW_TEXCOORDMODE_CLAMP;
> >        }
> >     } else if (texObj->Target == GL_TEXTURE_1D) {
> >        /* There's a bug in 1D texture sampling - it actually pays
> > @@ -387,66 +394,34 @@ brw_update_sampler_state(struct brw_context *brw,
> >         * Override the wrap_t value here to GL_REPEAT to keep
> >         * any nonexistent border pixels from floating in.
> >         */
> > -      sampler->ss1.t_wrap_mode = BRW_TEXCOORDMODE_WRAP;
> > -   }
> > -
> > -
> > -   /* Set shadow function:
> > -    */
> > -   if (gl_sampler->CompareMode == GL_COMPARE_R_TO_TEXTURE_ARB) {
> > -      /* Shadowing is "enabled" by emitting a particular sampler
> > -       * message (sample_c).  So need to recompile WM program when
> > -       * shadow comparison is enabled on each/any texture unit.
> > -       */
> > -      sampler->ss0.shadow_function =
> > -	 intel_translate_shadow_compare_func(gl_sampler->CompareFunc);
> > +      wrap_t = BRW_TEXCOORDMODE_WRAP;
> >     }
> >  
> > -   /* Set LOD bias:
> > -    */
> > -   sampler->ss0.lod_bias = S_FIXED(CLAMP(texUnit->LodBias +
> > -					 gl_sampler->LodBias, -16, 15), 6);
> > -
> > -   sampler->ss0.lod_preclamp = 1; /* OpenGL mode */
> > -   sampler->ss0.default_color_mode = 0; /* OpenGL/DX10 mode */
> > -
> > -   sampler->ss0.base_level = U_FIXED(0, 1);
> > -
> > -   sampler->ss1.max_lod = U_FIXED(CLAMP(gl_sampler->MaxLod, 0, 13), 6);
> > -   sampler->ss1.min_lod = U_FIXED(CLAMP(gl_sampler->MinLod, 0, 13), 6);
> > -
> > -   /* On Gen6+, the sampler can handle non-normalized texture
> > -    * rectangle coordinates natively
> > -    */
> > -   if (brw->gen >= 6 && texObj->Target == GL_TEXTURE_RECTANGLE) {
> > -      sampler->ss3.non_normalized_coord = 1;
> > -   }
> > -
> > -   uint32_t sdc_offset;
> > -   upload_default_color(brw, gl_sampler, unit, &sdc_offset);
> > -
> > -   if (brw->gen >= 6) {
> > -      sampler->ss2.default_color_pointer = sdc_offset >> 5;
> > -   } else {
> > -      /* reloc */
> > -      sampler->ss2.default_color_pointer =
> > -         (brw->batch.bo->offset64 + sdc_offset) >> 5;
> > -
> > -      drm_intel_bo_emit_reloc(brw->batch.bo,
> > -			      batch_offset_for_sampler_state +
> > -			      offsetof(struct brw_sampler_state, ss2),
> > -			      brw->batch.bo, sdc_offset,
> > -			      I915_GEM_DOMAIN_SAMPLER, 0);
> > +   /* Set shadow function. */
> > +   if (sampler->CompareMode == GL_COMPARE_R_TO_TEXTURE_ARB) {
> > +      shadow_function =
> > +	 intel_translate_shadow_compare_func(sampler->CompareFunc);
> >     }
> >  
> > -   if (sampler->ss0.min_filter != BRW_MAPFILTER_NEAREST)
> > -      sampler->ss3.address_round |= BRW_ADDRESS_ROUNDING_ENABLE_U_MIN |
> > -                                    BRW_ADDRESS_ROUNDING_ENABLE_V_MIN |
> > -                                    BRW_ADDRESS_ROUNDING_ENABLE_R_MIN;
> > -   if (sampler->ss0.mag_filter != BRW_MAPFILTER_NEAREST)
> > -      sampler->ss3.address_round |= BRW_ADDRESS_ROUNDING_ENABLE_U_MAG |
> > -                                    BRW_ADDRESS_ROUNDING_ENABLE_V_MAG |
> > -                                    BRW_ADDRESS_ROUNDING_ENABLE_R_MAG;
> > +   min_lod = U_FIXED(CLAMP(sampler->MinLod, 0, 13), 6);
> > +   max_lod = U_FIXED(CLAMP(sampler->MaxLod, 0, 13), 6);
> > +   lod_bias = S_FIXED(CLAMP(texUnit->LodBias + sampler->LodBias, -16, 15), 6);
> > +   base_level = U_FIXED(0, 1);
> 
> Declarations for these four could be moved here and made constant.
> 
> > +
> > +   uint32_t border_color_offset;
> > +   upload_default_color(brw, sampler, unit, &border_color_offset);
> > +
> > +   brw_emit_sampler_state(brw,
> > +                          sampler_state,
> > +                          batch_offset_for_sampler_state,
> > +                          min_filter, mag_filter, mip_filter,
> > +                          max_anisotropy,
> > +                          address_rounding,
> > +                          wrap_s, wrap_t, wrap_r,
> > +                          min_lod, max_lod, lod_bias, base_level,
> > +                          shadow_function,
> > +                          non_normalized_coordinates,
> > +                          border_color_offset);
> >  }
> >  
> >  
> > @@ -484,7 +459,6 @@ brw_upload_sampler_state_table(struct brw_context *brw,
> >                                           sampler_state);
> >              } else {
> >                 brw_update_sampler_state(brw, unit,
> > -                                        (struct brw_sampler_state *)
> >                                          sampler_state,
> >                                          batch_offset_for_sampler_state);
> >              }
> 
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: This is a digitally signed message part.
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20140730/9b884e1e/attachment.sig>


More information about the mesa-dev mailing list