[Mesa-dev] [PATCH 2/2] intel: make sure hardware choose the right filter mode

[Yuanhan Liu]

On Thu, Nov 03, 2011 at 10:16:06AM +0800, Yuanhan Liu wrote:
> On Wed, Nov 02, 2011 at 02:18:46PM -0700, Eric Anholt wrote:
> > On Wed, 2 Nov 2011 11:12:07 +0800, Yuanhan Liu <yuanhan.liu at linux.intel.com> wrote:
> > > On Tue, Nov 01, 2011 at 05:57:36PM +0800, Yuanhan Liu wrote:
> > > > According to bspec, MIPCnt(was set to intelObj->_MaxLevel) was used for
> > > > min/mag filter mode determination. For a normal case with no mipmap like
> > > > this:
> > > > 
> > > >   glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MIN_FILTER, GL_NEAREST);
> > > >   glTexParameteri(GL_TEXTURE_2D, GL_TEXTURE_MAG_FILTER, GL_LINEAR);
> > > > 
> > > > hardware would always choose mag filter(GL_LINEAR) here since MIPCnt was
> > > > set to 0 zero, then would make LOD be zero. Then according the formula:
> > > >   MagMode = (LOD - Base <= 0)
> > > 
> > > Here are some more comments about this: LOD is a pre-computed value. if
> > > preClamp is enabled, then LOD would be:
> > >   LOD = min(min(MIPCnt, MAX_LOD), LOD);
> > > 
> > > So, if MIPCnt was set to 0, and preClamp was enabled, then the _final_
> > > LOD would be 0, and hardware choose mag filter.
> > > 
> > > Thoughts?
> > 
> > It took me a long time to understand what you were trying to fix here.
> 
> Sorry for that :(
> 
> > The story I've worked out is:
> > 
> > "We can't optimize out uploading the other mipmap levels for
> > non-mipmapped filtering modes if the min filter is not the same as the
> > mag filter.  This is because the min/mag decision is based on the
> > computed LOD of the sample, and if we clamp the LOD to BaseLevel by not
> > including the other levels, it will always choose the mag filter even if
> > minification should have occurred. 
> 
> Yes.
> 
> > See page FINISHME: citation of PRM."
> 
> Sorry, what does this mean?
> 
> > 
> > Particularly what was unclear was that the patch was just skipping an
> > optimization, to get behavior back in line with what the user did through
> > the API: Specify multiple levels in their texture.
> 
> Yeah, I saw that. While thinking about this issue, I thought another
> patch to fix this issue. It's somehow against with the Bspec: preclamp
> should be enabled for OpenGL driver.
> 
> Anyway, here is the patch, and I'd like to know what's your thoughts.

Hi Eric,

Thoughts on the following patch?

Thanks,
Yuanhan Liu
> 
> ---
> 
> >From 052b646f5db1da338244325750fb50d28f77d575 Mon Sep 17 00:00:00 2001
> From: Yuanhan Liu <yuanhan.liu at linux.intel.com>
> Date: Thu, 3 Nov 2011 10:01:25 +0800
> Subject: [PATCH] i965: make sure hardware choose the right filter mode
> 
> Here I quoted Eric's words:
>   the min/mag decision is based on the computed LOD of the sample,
>   and if we clamp the LOD to BaseLevel by not including the other
>   levels, it will always choose the mag filter even if minification
>   should have occurred
> 
> And here is how the hardware do the min/mag decision in a formula way:
> 
>   MagMode = (LOD - Base <= 0)
> 
> where LOD is computed in a way like the following if preClamp is enabled:
>   if (preClamp is enabled)
>     LOD = min(min(MIPCnt, MAX_LOD), pre-computed_LOD);
>   else
>     LOD = per-computed_LOD;
> 
> So, if have just one level(the baselevel), just don't enable preclamp to
> make hardware choose the right filter mode.
> 
> This would fix all oglc filtercubemin subcase fail and introduce no oglc
> regression.
> 
> Signed-off-by: Yuanhan Liu <yuanhan.liu at linux.intel.com>
> ---
>  src/mesa/drivers/dri/i965/brw_wm_sampler_state.c |    9 +++++++++
>  1 files changed, 9 insertions(+), 0 deletions(-)
> 
> diff --git a/src/mesa/drivers/dri/i965/brw_wm_sampler_state.c b/src/mesa/drivers/dri/i965/brw_wm_sampler_state.c
> index 6104afc..f102573 100644
> --- a/src/mesa/drivers/dri/i965/brw_wm_sampler_state.c
> +++ b/src/mesa/drivers/dri/i965/brw_wm_sampler_state.c
> @@ -164,8 +164,10 @@ static void brw_update_sampler_state(struct brw_context *brw,
>     struct gl_context *ctx = &intel->ctx;
>     struct gl_texture_unit *texUnit = &ctx->Texture.Unit[unit];
>     struct gl_texture_object *texObj = texUnit->_Current;
> +   struct intel_texture_object *intelObj = intel_texture_object(texObj);
>     struct gl_sampler_object *gl_sampler = _mesa_get_samplerobj(ctx, unit);
>     bool using_nearest = false;
> +   int mip_cnt = intelObj->_MaxLevel - texObj->BaseLevel;
>  
>     switch (gl_sampler->MinFilter) {
>     case GL_NEAREST:
> @@ -275,6 +277,13 @@ static void brw_update_sampler_state(struct brw_context *brw,
>  					 gl_sampler->LodBias, -16, 15), 6);
>  
>     sampler->ss0.lod_preclamp = 1; /* OpenGL mode */
> +   /*
> +    * Workaround: if mip count is set to 0 and preclamp is enabled,
> +    * this would clamp * the final lod to 0 and then hardware would
> +    * always choose mag filter.
> +    */
> +   if (gl_sampler->MinFilter != gl_sampler->MagFilter && mip_cnt == 0)
> +      sampler->ss0.lod_preclamp = 0;
>     sampler->ss0.default_color_mode = 0; /* OpenGL/DX10 mode */
>  
>     /* Set BaseMipLevel, MaxLOD, MinLOD: 
> -- 
> 1.7.4.4
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev