[Mesa-dev] [PATCH] [v2] i965/skl: Fix SBE state upload code.

[Kenneth Graunke]

On Saturday, November 22, 2014 03:22:15 PM Ben Widawsky wrote:
> The state upload code was incorrectly shifting the attribute swizzles. The
> effect of this is we're likely to get the default swizzle values, which disables
> the component.
> 
> This doesn't technically fix any bugs since Skylake support is still disabled by
> default (no PCI IDs).
> 
> While here, since VARYING_SLOT_MAX can be greater than the number of attributes
> we have available, add a warning to the code to make sure we never do the wrong
> thing (and hopefully prevent further static analysis from finding this).
> Admittedly I am a bit confused. It seems to me like the moment a user has
> greater than 8 varyings we will hit this condition. CC Ken to clarify.
> 
> v2: Forgot to git add the warning message in v1
> 
> Cc: Kenneth Graunke <kenneth at whitecape.org>
> Reported-by: Ilia Mirkin <imirkin at alum.mit.edu> (via Coverity)
> Signed-off-by: Ben Widawsky <ben at bwidawsk.net>
> ---
>  src/mesa/drivers/dri/i965/gen8_sf_state.c | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/src/mesa/drivers/dri/i965/gen8_sf_state.c b/src/mesa/drivers/dri/i965/gen8_sf_state.c
> index 6995a6a..f8a0428 100644
> --- a/src/mesa/drivers/dri/i965/gen8_sf_state.c
> +++ b/src/mesa/drivers/dri/i965/gen8_sf_state.c
> @@ -93,10 +93,15 @@ upload_sbe(struct brw_context *brw)
>           if (!(brw->fragment_program->Base.InputsRead & BITFIELD64_BIT(attr)))
>              continue;
>  
> +         if (unlikely(input_index > 31)) {
> +            _mesa_warning(ctx, "Too many varyings for the SBE.\n");
> +            break;
> +         }

I believe that the GLSL linker ensures that the number of active varyings is
within the required limits.  So, I would just change this to:

         assert(input_index < 32);

With that changed, this is:
Reviewed-by: Kenneth Graunke <kenneth at whitecape.org>

> +
>           if (input_index < 16)
>              dw4 |= (GEN9_SBE_ACTIVE_COMPONENT_XYZW << (input_index << 1));
>           else
> -            dw5 |= (GEN9_SBE_ACTIVE_COMPONENT_XYZW << (input_index << 1));
> +            dw5 |= (GEN9_SBE_ACTIVE_COMPONENT_XYZW << ((input_index - 16) << 1));
>  
>           ++input_index;
>        }
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: This is a digitally signed message part.
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20141130/e4d7f8d4/attachment.sig>