[Mesa-dev] [PATCH] i965/hsw: Set integer mode in sampling state for stencil texturing
Jordan Justen
jordan.l.justen at intel.com
Mon Nov 21 06:42:11 UTC 2016
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.)
-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
More information about the mesa-dev
mailing list