On Sat, May 13, 2017 at 12:18 PM, Kenneth Graunke <kenn...@whitecape.org>
wrote:
> On Friday, May 12, 2017 4:38:25 PM PDT Anuj Phogat wrote:
> > v1: By Ben Widawsky <benjamin.widaw...@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.pho...@gmail.com>
> > Cc: Ben Widawsky <benjamin.widaw...@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) |
> >
>
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
