[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