[Mesa-stable] [Mesa-dev] [PATCH 3/3] i965: Fix integer border color on Haswell.
Jason Ekstrand
jason at jlekstrand.net
Fri Feb 6 10:11:22 PST 2015
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.
Reviewed-by: Jason Ekstrand <jason.ekstrand at intel.com>
On Fri, Feb 6, 2015 at 4:32 AM, Kenneth Graunke <kenneth at whitecape.org>
wrote:
> +82 Piglits - 100% of border color tests now pass on Haswell.
>
> Signed-off-by: Kenneth Graunke <kenneth at whitecape.org>
> Cc: Chris Forbes <chrisf at ijw.co.nz>
> Cc: mesa-stable at lists.freedesktop.org
> ---
> 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
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/mesa-stable/attachments/20150206/e7c1991b/attachment.html>
More information about the mesa-stable
mailing list