[Mesa-dev] [PATCH 2/9] i965: Totally switch around how we handle nonzero baselevel-first_level.

[Chad Versace]

On 09/18/2013 12:59 PM, Eric Anholt wrote:
> This has no effect currently, because intel_finalize_mipmap_tree() always
> makes mt->first_level == tObj->BaseLevel.
>
> The change I made before to handle it
> (b1080cfbdb0a084122fcd662cd27b4748c5598fd) got very close to working, but
> after fixing some unrelated bugs in the series, it still left
> tex-miplevel-selection producing errors when testing textureLod().  The
> problem is that for explicit LODs, the sampler's LOD clamping is ignored,
> and only the surface's MIP clamping is respected.  So we need to use
> surface mip clamping, which applies on top of the sampler's mip clamping,
> so the sampler change gets backed out.
>
> Now actually tested with a non-regressing series producing a non-zero
> computed baselevel.
> ---
>   src/mesa/drivers/dri/i965/brw_wm_sampler_state.c  | 11 +++--------
>   src/mesa/drivers/dri/i965/brw_wm_surface_state.c  |  5 +++--
>   src/mesa/drivers/dri/i965/gen7_sampler_state.c    | 11 +++--------
>   src/mesa/drivers/dri/i965/gen7_wm_surface_state.c |  4 +++-
>   4 files changed, 12 insertions(+), 19 deletions(-)




> 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 531a1a2..115b296 100644
> --- a/src/mesa/drivers/dri/i965/gen7_wm_surface_state.c
> +++ b/src/mesa/drivers/dri/i965/gen7_wm_surface_state.c
> @@ -325,8 +325,10 @@ gen7_update_texture_surface(struct gl_context *ctx,
>      surf[4] = gen7_surface_msaa_bits(mt->num_samples, mt->msaa_layout);
>
>      surf[5] = (SET_FIELD(GEN7_MOCS_L3, GEN7_SURFACE_MOCS) |
> +              SET_FIELD(tObj->BaseLevel - mt->first_level,
> +                        GEN7_SURFACE_MIN_LOD) |
>                 /* mip count */
> -              (intelObj->_MaxLevel - mt->first_level));
> +              (intelObj->_MaxLevel - tObj->BaseLevel));

>
>      if (brw->is_haswell) {
>         /* Handling GL_ALPHA as a surface format override breaks 1.30+ style


I initially was confused as to whether the MipCount field should be the absolute
count of miplevels or the count relative to MinLod. But then I found this piece
of doc that indirectly settled my confusion:

   SAMPLER_STATE
     MipCount/Lod
       For Render Target Surfaces:
          [...] This is the absolute MIP level on the surface and is not relative
          to the Surface Min LOD field, which is ignored for render target surfaces.

I interpret the opposition in tone as indirect confirmation that **for Sampler Surfaces** the MipCount field
*is* relative to the Surface Min LOD field. So, your change there looks good to me.

More important than my flaky interpretation of opaque documentation, though, is that
this patch doesn't regress Piglit.

Patch 2 is
Reviewed-by: Chad Versace <chad.versace at linux.intel.com>