<div dir="ltr"><div class="gmail_default" style="font-family:verdana,sans-serif;font-size:small"><br></div><div class="gmail_extra"><br><div class="gmail_quote">On Sat, May 13, 2017 at 12:18 PM, Kenneth Graunke <span dir="ltr"><<a href="mailto:kenneth@whitecape.org" target="_blank">kenneth@whitecape.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div class="m_5269025737558939268HOEnZb"><div class="m_5269025737558939268h5">On Friday, May 12, 2017 4:38:25 PM PDT Anuj Phogat wrote:<br>
> v1: By Ben Widawsky <<a href="mailto:benjamin.widawsky@intel.com" target="_blank">benjamin.widawsky@intel.com</a>><br>
> v2: Add the restriction for GS, HS and DS and make sure<br>
>     the allocated sizes are not multiple of 3.<br>
><br>
> Signed-off-by: Anuj Phogat <<a href="mailto:anuj.phogat@gmail.com" target="_blank">anuj.phogat@gmail.com</a>><br>
> Cc: Ben Widawsky <<a href="mailto:benjamin.widawsky@intel.com" target="_blank">benjamin.widawsky@intel.com</a>><br>
> ---<br>
>  src/mesa/drivers/dri/i965/gen7<wbr>_urb.c | 12 ++++++++++++<br>
>  1 file changed, 12 insertions(+)<br>
><br>
> diff --git a/src/mesa/drivers/dri/i965/ge<wbr>n7_urb.c b/src/mesa/drivers/dri/i965/ge<wbr>n7_urb.c<br>
> index 028161d..dc6826a 100644<br>
> --- a/src/mesa/drivers/dri/i965/ge<wbr>n7_urb.c<br>
> +++ b/src/mesa/drivers/dri/i965/ge<wbr>n7_urb.c<br>
> @@ -194,6 +194,17 @@ gen7_upload_urb(struct brw_context *brw, unsigned vs_size,<br>
>        entry_size[i] = prog_data[i] ? prog_data[i]->urb_entry_size : 1;<br>
>     }<br>
><br>
> +   /* For Cannonlake:<br>
> +    * Software shall not program an allocation size that specifies a size<br>
> +    * that is a multiple of 3 64B (512-bit) cachelines.<br>
> +    */<br>
> +   if (brw->gen == 10) {<br>
> +      for (int i = MESA_SHADER_VERTEX; i <= MESA_SHADER_GEOMETRY; i++) {<br>
> +         if (entry_size[i] % 3 == 0)<br>
> +            entry_size[i]++;<br>
> +      }<br>
> +   }<br>
> +<br>
<br>
</div></div>This is wrong - you shouldn't be changing this in the state upload code.<br>
<br>
You should change it in the compiler.  In src/intel/compiler, git grep<br>
for urb_entry_size.  You'll find it in brw_compile_{vs,tcs,tes,gs}.<br>
Just increment prog_data->base.urb_entry_size<wbr>, like you're doing here.<br></blockquote><div class="gmail_default" style="font-family:verdana,sans-serif;font-size:small">​This might explain a hang I was seeing on CNL. I'll send out a v3 with suggested</div><div class="gmail_default" style="font-family:verdana,sans-serif;font-size:small">change. Thanks.</div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
Your commit message says that the restriction applies to GS, HS, and DS,<br>
but your code applies it to VS as well.  Is that intentional?<br></blockquote><div class="gmail_default" style="font-family:verdana,sans-serif;font-size:small">​Ben had the restriction added only for VS. Later I added the restrictions in v2</div><div class="gmail_default" style="font-family:verdana,sans-serif;font-size:small">for GS, HS​ and DS as well. I'll reword the cimmit message to make it clear.</div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<span><br>
>     /* If we're just switching between programs with the same URB requirements,<br>
>      * skip the rest of the logic.<br>
>      */<br>
> @@ -224,6 +235,7 @@ gen7_upload_urb(struct brw_context *brw, unsigned vs_size,<br>
><br>
>     BEGIN_BATCH(8);<br>
>     for (int i = MESA_SHADER_VERTEX; i <= MESA_SHADER_GEOMETRY; i++) {<br>
> +      assert(brw->gen != 10 || entry_size[i] % 3);<br>
<br>
</span>The assert is good.<br>
<div class="m_5269025737558939268HOEnZb"><div class="m_5269025737558939268h5"><br>
>        OUT_BATCH((_3DSTATE_URB_VS + i) << 16 | (2 - 2));<br>
>        OUT_BATCH(entries[i] |<br>
>                  ((entry_size[i] - 1) << GEN7_URB_ENTRY_SIZE_SHIFT) |<br>
><br>
</div></div></blockquote></div><br></div></div>