<div dir="ltr"><div>You seem to be right about 2 things: Your implementation and the fact that it's entirely INSANE! I checked and double-checked the bspec and this looks correct. I trust you with the rest of the state bits. </div>Reviewed-by: Jason Ekstrand <<a href="mailto:jason.ekstrand@intel.com">jason.ekstrand@intel.com</a>> </div><div class="gmail_extra"> <div class="gmail_quote">On Fri, Feb 6, 2015 at 4:32 AM, Kenneth Graunke <<a href="mailto:kenneth@whitecape.org" target="_blank">kenneth@whitecape.org</a>> wrote: <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">+82 Piglits - 100% of border color tests now pass on Haswell. Signed-off-by: Kenneth Graunke <<a href="mailto:kenneth@whitecape.org">kenneth@whitecape.org</a>> Cc: Chris Forbes <<a href="mailto:chrisf@ijw.co.nz">chrisf@ijw.co.nz</a>> Cc: <a href="mailto:mesa-stable@lists.freedesktop.org">mesa-stable@lists.freedesktop.org</a> --- src/mesa/drivers/dri/i965/brw_defines.h | 1 + src/mesa/drivers/dri/i965/brw_sampler_state.c | 62 +++++++++++++++++++++++ src/mesa/drivers/dri/i965/gen7_wm_surface_state.c | 3 ++ 3 files changed, 66 insertions(+) I finally figured out what the documentation was trying to say! Buried within thirteen pages of tables were a few extra bizarre bits - RG is broken and treated as RB (like it is for textureGather), and 16-bit skip a DWord for no obvious reason. With all the strange corner cases in place, everything seems to work. diff --git a/src/mesa/drivers/dri/i965/brw_defines.h b/src/mesa/drivers/dri/i965/brw_defines.h index f02a0b8..a597d6b 100644 --- a/src/mesa/drivers/dri/i965/brw_defines.h +++ b/src/mesa/drivers/dri/i965/brw_defines.h @@ -551,6 +551,7 @@ #define BRW_SURFACE_PITCH_MASK INTEL_MASK(19, 3) #define BRW_SURFACE_TILED (1 << 1) #define BRW_SURFACE_TILED_Y (1 << 0) +#define HSW_SURFACE_IS_INTEGER_FORMAT (1 << 18) /* Surface state DW4 */ #define BRW_SURFACE_MIN_LOD_SHIFT 28 diff --git a/src/mesa/drivers/dri/i965/brw_sampler_state.c b/src/mesa/drivers/dri/i965/brw_sampler_state.c index c6a8ab1..be85660 100644 --- a/src/mesa/drivers/dri/i965/brw_sampler_state.c +++ b/src/mesa/drivers/dri/i965/brw_sampler_state.c @@ -271,6 +271,68 @@ 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 && texObj->_IsIntegerFormat) { + /* 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 + * pad it out to a 64 byte cacheline boundary. DWords 16-19 then + * contain integer colors; these are only used if SURFACE_STATE + * has the "Integer Surface Format" bit set. Even then, the + * arrangement of the RGBA data devolves into madness. + */ + uint32_t *sdc = brw_state_batch(brw, AUB_TRACE_SAMPLER_DEFAULT_COLOR, + 20 * 4, 512, sdc_offset); + memset(sdc, 0, 20 * 4); + sdc = &sdc[16]; + + mesa_format format = firstImage->TexFormat; + int bits_per_channel = _mesa_get_format_bits(format, GL_RED_BITS); + + /* From the Haswell PRM, "Command Reference: Structures", Page 36: + * "If any color channel is missing from the surface format, + * corresponding border color should be programmed as zero and if + * alpha channel is missing, corresponding Alpha border color should + * be programmed as 1." + */ + unsigned c[4] = { 0, 0, 0, 1 }; + for (int i = 0; i < 4; i++) { + if (_mesa_format_has_color_component(format, i)) + c[i] = color.ui[i]; + } + + switch (bits_per_channel) { + case 8: + /* Copy RGBA in order. */ + for (int i = 0; i < 4; i++) + ((uint8_t *) sdc)[i] = c[i]; + break; + case 10: + /* R10G10B10A2_UINT is treated like a 16-bit format. */ + case 16: + ((uint16_t *) sdc)[0] = c[0]; /* R -> DWord 0, bits 15:0 */ + ((uint16_t *) sdc)[1] = c[1]; /* G -> DWord 0, bits 31:16 */ + /* DWord 1 is Reserved/MBZ! */ + ((uint16_t *) sdc)[4] = c[2]; /* B -> DWord 2, bits 15:0 */ + ((uint16_t *) sdc)[5] = c[3]; /* A -> DWord 3, bits 31:16 */ + break; + case 32: + if (firstImage->_BaseFormat == GL_RG) { + /* Careful inspection of the tables reveals that for RG32 formats, + * the green channel needs to go where blue normally belongs. + */ + sdc[0] = c[0]; + sdc[2] = c[1]; + sdc[3] = 1; + } else { + /* Copy RGBA in order. */ + for (int i = 0; i < 4; i++) + sdc[i] = c[i]; + } + break; + default: + assert(!"Invalid number of bits per channel in integer format."); + break; + } } else if (brw->gen == 5 || brw->gen == 6) { struct gen5_sampler_default_color *sdc; 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 07db678..29553cd 100644 --- a/src/mesa/drivers/dri/i965/gen7_wm_surface_state.c +++ b/src/mesa/drivers/dri/i965/gen7_wm_surface_state.c @@ -321,6 +321,9 @@ gen7_update_texture_surface(struct gl_context *ctx, surf[3] = SET_FIELD(effective_depth - 1, BRW_SURFACE_DEPTH) | (mt->pitch - 1); + if (brw->is_haswell && tObj->_IsIntegerFormat) + surf[3] |= HSW_SURFACE_IS_INTEGER_FORMAT; + surf[4] = gen7_surface_msaa_bits(mt->num_samples, mt->msaa_layout) | SET_FIELD(tObj->MinLayer, GEN7_SURFACE_MIN_ARRAY_ELEMENT) | SET_FIELD((effective_depth - 1), -- 2.2.2 _______________________________________________ mesa-dev mailing list <a href="mailto:mesa-dev@lists.freedesktop.org">mesa-dev@lists.freedesktop.org</a> <a href="http://lists.freedesktop.org/mailman/listinfo/mesa-dev" target="_blank">http://lists.freedesktop.org/mailman/listinfo/mesa-dev</a> </blockquote></div> </div>