[Mesa-dev] [PATCH] i965/hsw: Set integer mode in sampling state for stencil texturing

Jason Ekstrand jason at jlekstrand.net
Mon Nov 21 07:14:55 UTC 2016


On Nov 20, 2016 22:42, "Jordan Justen" <jordan.l.justen at intel.com> wrote:
>
> On 2016-11-20 21:15:24, Jason Ekstrand wrote:
> >    On Sun, Nov 20, 2016 at 9:12 PM, Jason Ekstrand <jason at jlekstrand.net
>
> >    wrote:
> >
> >      On Sun, Nov 20, 2016 at 8:03 PM, Jordan Justen
> >      <jordan.l.justen at intel.com> wrote:
> >
> >        Fixes:
> >
> >
ES31-CTS.functional.texture.border_clamp.formats.depth24_stencil8_sample_stencil.nearest_size_pot
> >
ES31-CTS.functional.texture.border_clamp.formats.depth24_stencil8_sample_stencil.nearest_size_npot
> >
ES31-CTS.functional.texture.border_clamp.formats.depth32f_stencil8_sample_stencil.nearest_size_pot
> >
ES31-CTS.functional.texture.border_clamp.formats.depth32f_stencil8_sample_stencil.nearest_size_npot
> >
ES31-CTS.functional.texture.border_clamp.unused_channels.depth24_stencil8_sample_stencil
> >
ES31-CTS.functional.texture.border_clamp.unused_channels.depth32f_stencil8_sample_stencil
> >
> >        Signed-off-by: Jordan Justen <jordan.l.justen at intel.com>
> >        ---
> >         src/mesa/drivers/dri/i965/brw_sampler_state.c | 18
+++++++++---------
> >         src/mesa/drivers/dri/i965/brw_state.h         |  9 ---------
> >         2 files changed, 9 insertions(+), 18 deletions(-)
> >
> >        diff --git a/src/mesa/drivers/dri/i965/brw_sampler_state.c
> >        b/src/mesa/drivers/dri/i965/brw_sampler_state.c
> >        index 7df2c55..412efb9 100644
> >        --- a/src/mesa/drivers/dri/i965/brw_sampler_state.c
> >        +++ b/src/mesa/drivers/dri/i965/brw_sampler_state.c
> >        @@ -213,7 +213,7 @@ static void
> >         upload_default_color(struct brw_context *brw,
> >                              const struct gl_sampler_object *sampler,
> >                              mesa_format format, GLenum base_format,
> >        -                     bool is_integer_format,
> >        +                     bool is_integer_format, bool
> >        is_stencil_sampling,
> >                              uint32_t *sdc_offset)
> >         {
> >            union gl_color_union color;
> >        @@ -277,7 +277,7 @@ upload_default_color(struct brw_context *brw,
> >               uint32_t *sdc = brw_state_batch(brw,
> >        AUB_TRACE_SAMPLER_DEFAULT_COLOR,
> >                                               4 * 4, 64, sdc_offset);
> >               memcpy(sdc, color.ui, 4 * 4);
> >        -   } else if (brw->is_haswell && is_integer_format) {
> >        +   } else if (brw->is_haswell && (is_integer_format ||
> >        is_stencil_sampling)) {
> >               /* Haswell's integer border color support is completely
insane:
> >                * SAMPLER_BORDER_COLOR_STATE is 20 DWords.  The first
four are
> >                * for float colors.  The next 12 DWords are MBZ and only
exist
> >        to
> >        @@ -291,10 +291,9 @@ upload_default_color(struct brw_context
*brw,
> >               memset(sdc, 0, 20 * 4);
> >               sdc = &sdc[16];
> >
> >        +      bool stencil = format == MESA_FORMAT_S_UINT8 ||
> >        is_stencil_sampling;
> >               const int bits_per_channel =
> >        -         _mesa_get_format_bits(format,
> >        -                               format == MESA_FORMAT_S_UINT8 ?
> >        -                               GL_STENCIL_BITS : GL_RED_BITS);
> >        +         _mesa_get_format_bits(format, stencil ?
GL_STENCIL_BITS :
> >        GL_RED_BITS);
> >
> >    I guess my suggestion could cause problems here.  When we're stencil
> >    sampling, does the format come in as anything other than S_UINT8?
Do we
> >    ever get combined dept-stencil formats in here?
> >
>
> Yeah, I think that was what was happening. Note that the affected
> tests (see commit message), have depth+stencil. So, I think
> is_integer_format is false due to the depth aspect of the format.
>
> My first attempt at a fix was to alter is_integer_format to || in
> texObj->StencilSampling when calling brw_update_sampler_state, but
> that hit an assert. (I think it was trying to call
> _mesa_get_format_bits with GL_RED_BITS on a depth/stencil format.)

Yeah, that's what I was afraid of on my second reading of the patch.  In
that case, this is probably the correct solution.

Reviewed-by: Jason Ekstrand <jason at jlekstrand.net>

> -Jordan
>
> >
> >               /* From the Haswell PRM, "Command Reference: Structures",
Page
> >        36:
> >                * "If any color channel is missing from the surface
format,
> >        @@ -389,12 +388,13 @@ upload_default_color(struct brw_context
*brw,
> >          * Sets the sampler state for a single unit based off of the
sampler
> >        key
> >          * entry.
> >          */
> >        -void
> >        +static void
> >         brw_update_sampler_state(struct brw_context *brw,
> >                                  GLenum target, bool
tex_cube_map_seamless,
> >                                  GLfloat tex_unit_lod_bias,
> >                                  mesa_format format, GLenum base_format,
> >                                  bool is_integer_format,
> >        +                         bool is_stencil_sampling,
> >
> >      Doesn't is_stencil_sampling imply is_integer_format?  I mean,
stencil is
> >      always R8_UINT, right?
> >
> >
> >                                  const struct gl_sampler_object
*sampler,
> >                                  uint32_t *sampler_state,
> >                                  uint32_t
batch_offset_for_sampler_state)
> >        @@ -516,8 +516,8 @@ brw_update_sampler_state(struct brw_context
*brw,
> >            if (wrap_mode_needs_border_color(wrap_s) ||
> >                wrap_mode_needs_border_color(wrap_t) ||
> >                wrap_mode_needs_border_color(wrap_r)) {
> >        -      upload_default_color(brw, sampler,
> >        -                           format, base_format,
is_integer_format,
> >        +      upload_default_color(brw, sampler, format, base_format,
> >        +                           is_integer_format,
is_stencil_sampling,
> >                                    &border_color_offset);
> >            }
> >
> >        @@ -555,7 +555,7 @@ update_sampler_state(struct brw_context *brw,
> >            brw_update_sampler_state(brw, texObj->Target,
> >        ctx->Texture.CubeMapSeamless,
> >                                     texUnit->LodBias,
> >                                     firstImage->TexFormat,
> >        firstImage->_BaseFormat,
> >        -                            texObj->_IsIntegerFormat,
> >        +                            texObj->_IsIntegerFormat,
> >        texObj->StencilSampling,
> >
> >      In other words, you could just make that comma a pipe and call it
a day.
> >
> >
> >                                     sampler,
> >                                     sampler_state,
> >        batch_offset_for_sampler_state);
> >         }
> >        diff --git a/src/mesa/drivers/dri/i965/brw_state.h
> >        b/src/mesa/drivers/dri/i965/brw_state.h
> >        index 07126e8..176557b 100644
> >        --- a/src/mesa/drivers/dri/i965/brw_state.h
> >        +++ b/src/mesa/drivers/dri/i965/brw_state.h
> >        @@ -335,15 +335,6 @@ void brw_emit_sampler_state(struct
brw_context
> >        *brw,
> >                                     bool non_normalized_coordinates,
> >                                     uint32_t border_color_offset);
> >
> >        -void brw_update_sampler_state(struct brw_context *brw,
> >        -                              GLenum target, bool
> >        tex_cube_map_seamless,
> >        -                              GLfloat tex_unit_lod_bias,
> >        -                              mesa_format format, GLenum
base_format,
> >        -                              bool is_integer_format,
> >        -                              const struct gl_sampler_object
> >        *sampler,
> >        -                              uint32_t *sampler_state,
> >        -                              uint32_t
> >        batch_offset_for_sampler_state);
> >        -
> >         /* gen6_wm_state.c */
> >         void
> >         gen6_upload_wm_state(struct brw_context *brw,
> >        --
> >        2.10.2
> >
> >        _______________________________________________
> >        mesa-dev mailing list
> >        mesa-dev at lists.freedesktop.org
> >        https://lists.freedesktop.org/mailman/listinfo/mesa-dev
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20161120/f6c6a5a4/attachment-0001.html>


More information about the mesa-dev mailing list