[Mesa-dev] [PATCH 21/24] i965/cnl: Make URB {VS, GS, HS, DS} sizes non multiple of 3

[Kenneth Graunke]

On Friday, May 12, 2017 4:38:25 PM PDT Anuj Phogat wrote:
> v1: By Ben Widawsky <benjamin.widawsky at intel.com>
> v2: Add the restriction for GS, HS and DS and make sure
>     the allocated sizes are not multiple of 3.
> 
> Signed-off-by: Anuj Phogat <anuj.phogat at gmail.com>
> Cc: Ben Widawsky <benjamin.widawsky at intel.com>
> ---
>  src/mesa/drivers/dri/i965/gen7_urb.c | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
> 
> diff --git a/src/mesa/drivers/dri/i965/gen7_urb.c b/src/mesa/drivers/dri/i965/gen7_urb.c
> index 028161d..dc6826a 100644
> --- a/src/mesa/drivers/dri/i965/gen7_urb.c
> +++ b/src/mesa/drivers/dri/i965/gen7_urb.c
> @@ -194,6 +194,17 @@ gen7_upload_urb(struct brw_context *brw, unsigned vs_size,
>        entry_size[i] = prog_data[i] ? prog_data[i]->urb_entry_size : 1;
>     }
>  
> +   /* For Cannonlake:
> +    * Software shall not program an allocation size that specifies a size
> +    * that is a multiple of 3 64B (512-bit) cachelines.
> +    */
> +   if (brw->gen == 10) {
> +      for (int i = MESA_SHADER_VERTEX; i <= MESA_SHADER_GEOMETRY; i++) {
> +         if (entry_size[i] % 3 == 0)
> +            entry_size[i]++;
> +      }
> +   }
> +

This is wrong - you shouldn't be changing this in the state upload code.

You should change it in the compiler.  In src/intel/compiler, git grep
for urb_entry_size.  You'll find it in brw_compile_{vs,tcs,tes,gs}.
Just increment prog_data->base.urb_entry_size, like you're doing here.

Your commit message says that the restriction applies to GS, HS, and DS,
but your code applies it to VS as well.  Is that intentional?

>     /* If we're just switching between programs with the same URB requirements,
>      * skip the rest of the logic.
>      */
> @@ -224,6 +235,7 @@ gen7_upload_urb(struct brw_context *brw, unsigned vs_size,
>  
>     BEGIN_BATCH(8);
>     for (int i = MESA_SHADER_VERTEX; i <= MESA_SHADER_GEOMETRY; i++) {
> +      assert(brw->gen != 10 || entry_size[i] % 3);

The assert is good.

>        OUT_BATCH((_3DSTATE_URB_VS + i) << 16 | (2 - 2));
>        OUT_BATCH(entries[i] |
>                  ((entry_size[i] - 1) << GEN7_URB_ENTRY_SIZE_SHIFT) |
> 
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: This is a digitally signed message part.
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20170513/2a7a658b/attachment.sig>