[Mesa-dev] [PATCH] i965/gen7_urb: Re-emit PUSH_CONSTANT_ALLOC on GLK

[Ilia Mirkin]

On Fri, Aug 24, 2018 at 8:46 PM, Nanley Chery <nanleychery at gmail.com> wrote:
> According to internal docs, some gen9 platforms have a pixel shader push
> constant synchronization issue. Although not listed among said
> platforms, this issue seems to be present on the GeminiLake 2x6's we've
> tested.
>
> We consider the available workarounds to be too detrimental on
> performance. Instead, we mitigate the issue by applying part of one of
> the workarounds. Re-emit PUSH_CONSTANT_ALLOC at the top of every batch
> (as suggested by Ken).
>
> Fixes ext_framebuffer_multisample-accuracy piglit test failures with the
> following options:
> * 6 depth_draw small depthstencil
> * 8 stencil_draw small depthstencil
> * 6 stencil_draw small depthstencil
> * 8 depth_resolve small
> * 6 stencil_resolve small depthstencil
> * 4 stencil_draw small depthstencil
> * 16 stencil_draw small depthstencil
> * 16 depth_draw small depthstencil
> * 2 stencil_resolve small depthstencil
> * 6 stencil_draw small
> * all_samples stencil_draw small
> * 2 depth_draw small depthstencil
> * all_samples depth_draw small depthstencil
> * all_samples stencil_resolve small
> * 4 depth_draw small depthstencil
> * all_samples depth_draw small
> * all_samples stencil_draw small depthstencil
> * 4 stencil_resolve small depthstencil
> * 4 depth_resolve small depthstencil
> * all_samples stencil_resolve small depthstencil
>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=106865
> Cc: <mesa-stable at lists.freedesktop.org>
> ---
>  src/mesa/drivers/dri/i965/gen7_urb.c | 23 +++++++++++++++++++++++
>  1 file changed, 23 insertions(+)
>
> diff --git a/src/mesa/drivers/dri/i965/gen7_urb.c b/src/mesa/drivers/dri/i965/gen7_urb.c
> index 2e5f8e60ba9..cb045251236 100644
> --- a/src/mesa/drivers/dri/i965/gen7_urb.c
> +++ b/src/mesa/drivers/dri/i965/gen7_urb.c
> @@ -118,6 +118,28 @@ gen7_emit_push_constant_state(struct brw_context *brw, unsigned vs_size,
>     const struct gen_device_info *devinfo = &brw->screen->devinfo;
>     unsigned offset = 0;
>
> +   /* From the SKL PRM, Workarounds section (#878):
> +    *
> +    *    Push constant buffer corruption possible. WA: Insert 2 zero-length
> +    *    PushConst_PS before every intended PushConst_PS update, issue a
> +    *    NULLPRIM after each of the zero len PC update to make sure CS commits
> +    *    them.
> +    *
> +    * This workaround is attempting to solve a pixel shader push constant
> +    * synchronization issue.
> +    *
> +    * There's an unpublished WA that involves re-emitting
> +    * 3DSTATE_PUSH_CONSTANT_ALLOC_PS for every 500-ish 3DSTATE_CONSTANT_PS
> +    * packets. Since our counting methods may not be reliable due to
> +    * context-switching and pre-emption, we instead choose to approximate this
> +    * behavior by re-emitting the packet at the top of the batch.
> +    */
> +   if (brw->ctx.NewDriverState == BRW_NEW_BATCH) {

Did you want & here?

> +       /* Only GLK 2x6 has demonstrated this issue thus far. */
> +      if (!devinfo->is_geminilake || devinfo->num_subslices[0] != 2)
> +         return;
> +   }
> +
>     BEGIN_BATCH(10);
>     OUT_BATCH(_3DSTATE_PUSH_CONSTANT_ALLOC_VS << 16 | (2 - 2));
>     OUT_BATCH(vs_size | offset << GEN7_PUSH_CONSTANT_BUFFER_OFFSET_SHIFT);
> @@ -154,6 +176,7 @@ const struct brw_tracked_state gen7_push_constant_space = {
>     .dirty = {
>        .mesa = 0,
>        .brw = BRW_NEW_CONTEXT |
> +             BRW_NEW_BATCH | /* GLK workaround */
>               BRW_NEW_GEOMETRY_PROGRAM |
>               BRW_NEW_TESS_PROGRAMS,
>     },
> --
> 2.18.0
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev