On Nov 20, 2016 22:42, "Jordan Justen" <<a href="mailto:jordan.l.justen@intel.com">jordan.l.justen@intel.com</a>> wrote: > > On 2016-11-20 21:15:24, Jason Ekstrand wrote: > > On Sun, Nov 20, 2016 at 9:12 PM, Jason Ekstrand <<a href="mailto:jason@jlekstrand.net">jason@jlekstrand.net</a>> > > wrote: > > > > On Sun, Nov 20, 2016 at 8:03 PM, Jordan Justen > > <<a href="mailto:jordan.l.justen@intel.com">jordan.l.justen@intel.com</a>> 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 <<a href="mailto:jordan.l.justen@intel.com">jordan.l.justen@intel.com</a>> > > --- > > 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 <<a href="mailto:jason@jlekstrand.net">jason@jlekstrand.net</a>> > -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 > > <a href="mailto:mesa-dev@lists.freedesktop.org">mesa-dev@lists.freedesktop.org</a> > > <a href="https://lists.freedesktop.org/mailman/listinfo/mesa-dev">https://lists.freedesktop.org/mailman/listinfo/mesa-dev</a>