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

[Anuj Phogat]

On Sat, May 13, 2017 at 12:18 PM, Kenneth Graunke <kenneth at whitecape.org>
wrote:

> 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.
>
​This might explain a hang I was seeing on CNL. I'll send out a v3 with
suggested
change. Thanks.

>
> 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?
>
​Ben had the restriction added only for VS. Later I added the restrictions
in v2
for GS, HS​ and DS as well. I'll reword the cimmit message to make it clear.

>
> >     /* 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 --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20170515/c89906e4/attachment.html>