[Mesa-dev] [PATCH 2/3] anv/gen10: Ignore push constant packets during context restore.

[Rafael Antognolli]

On Wed, Jan 24, 2018 at 05:08:54PM -0800, Jason Ekstrand wrote:
> On Wed, Jan 24, 2018 at 4:33 PM, Rafael Antognolli <rafael.antognolli at intel.com
> > wrote:
> 
>     Similar to the GL driver, ignore 3DSTATE_CONSTANT_* packets when doing a
>     context restore.
> 
>     Signed-off-by: Rafael Antognolli <rafael.antognolli at intel.com>
>     Cc: Jason Ekstrand <jason at jlekstrand.net>
>     Cc: "18.0" <mesa-stable at lists.freedesktop.org>
>     ---
>      src/intel/vulkan/anv_private.h     |  1 +
>      src/intel/vulkan/genX_cmd_buffer.c | 47 ++++++++++++++++++++++++++++++
>     ++++++++
>      2 files changed, 48 insertions(+)
> 
>     diff --git a/src/intel/vulkan/anv_private.h b/src/intel/vulkan/anv_
>     private.h
>     index b351c6f63b3..a4c84d2c295 100644
>     --- a/src/intel/vulkan/anv_private.h
>     +++ b/src/intel/vulkan/anv_private.h
>     @@ -1458,6 +1458,7 @@ enum anv_pipe_bits {
>         ANV_PIPE_CONSTANT_CACHE_INVALIDATE_BIT    = (1 << 3),
>         ANV_PIPE_VF_CACHE_INVALIDATE_BIT          = (1 << 4),
>         ANV_PIPE_DATA_CACHE_FLUSH_BIT             = (1 << 5),
>     +   ANV_PIPE_ISP_DISABLE_BIT                  = (1 << 9),
> 
> 
> Let's drop this if we're not going to use it.

OK.

> 
>         ANV_PIPE_TEXTURE_CACHE_INVALIDATE_BIT     = (1 << 10),
>         ANV_PIPE_INSTRUCTION_CACHE_INVALIDATE_BIT = (1 << 11),
>         ANV_PIPE_RENDER_TARGET_CACHE_FLUSH_BIT    = (1 << 12),
>     diff --git a/src/intel/vulkan/genX_cmd_buffer.c b/src/intel/vulkan/
>     genX_cmd_buffer.c
>     index c23a54fb7b9..7028c1ce9df 100644
>     --- a/src/intel/vulkan/genX_cmd_buffer.c
>     +++ b/src/intel/vulkan/genX_cmd_buffer.c
>     @@ -1008,6 +1008,50 @@ genX(BeginCommandBuffer)(
>         return result;
>      }
> 
>     +/**
>     + * From the PRM, Volume 2a:
>     + *
>     + *    "Indirect State Pointers Disable
>     + *
>     + *    At the completion of the post-sync operation associated with this
>     pipe
>     + *    control packet, the indirect state pointers in the hardware are
>     + *    considered invalid; the indirect pointers are not saved in the
>     context.
>     + *    If any new indirect state commands are executed in the command
>     stream
>     + *    while the pipe control is pending, the new indirect state commands
>     are
>     + *    preserved.
>     + *
>     + *    [DevIVB+]: Using Invalidate State Pointer (ISP) only inhibits
>     context
>     + *    restoring of Push Constant (3DSTATE_CONSTANT_*) commands. Push
>     Constant
>     + *    commands are only considered as Indirect State Pointers. Once ISP is
>     + *    issued in a context, SW must initialize by programming push constant
>     + *    commands for all the shaders (at least to zero length) before
>     attempting
>     + *    any rendering operation for the same context."
>     + *
>     + * 3DSTATE_CONSTANT_* packets are restored during a context restore,
>     + * even though they point to a BO that has been already unreferenced at
>     + * the end of the previous batch buffer. This has been fine so far since
>     + * we are protected by these scratch page (every address not covered by
>     + * a BO should be pointing to the scratch page). But on CNL, it is
>     + * causing a GPU hang during context restore at the 3DSTATE_CONSTANT_*
>     + * instruction.
>     + *
>     + * The flag "Indirect State Pointers Disable" in PIPE_CONTROL tells the
>     + * hardware to ignore previous 3DSTATE_CONSTANT_* packets during a
>     + * context restore, so the mentioned hang doesn't happen. However,
>     + * software must program push constant commands for all stages prior to
>     + * rendering anything, so we flag them as dirty.
> 
> 
> And... The next command buffer won't.  I just looked at it and we won't set up
> push constants again until we use them.  We could either set 3DSTATE_CONSTANT_*
> instead or we can make sure that push constants are flagged as dirty in
> BeginCommandBuffer.

Oh, I understood that anv was always sending them at every command
buffer. OK, will check this again.

>     + */
>     +static void
>     +emit_isp_disable(struct anv_cmd_buffer *cmd_buffer)
>     +{
>     +   anv_batch_emit(&cmd_buffer->batch, GENX(PIPE_CONTROL), pc) {
>     +         pc.IndirectStatePointersDisable = true;
>     +         pc.PostSyncOperation = WriteImmediateData;
>     +         pc.Address           =
>     +            (struct anv_address) { &cmd_buffer->device->workaround_bo, 0
>     };'
> 
> 
> Is the W/A BO write needed?

That's what I understood from

"At the completion of the post-sync operation associated with this pipe
control packet..."

> 
>     +   }
>     +}
>     +
>      VkResult
>      genX(EndCommandBuffer)(
>          VkCommandBuffer                             commandBuffer)
>     @@ -1024,6 +1068,9 @@ genX(EndCommandBuffer)(
> 
>         genX(cmd_buffer_apply_pipe_flushes)(cmd_buffer);
> 
>     +   if (GEN_GEN == 10)
>     +      emit_isp_disable(cmd_buffer);
>     +
>         anv_cmd_buffer_end_batch_buffer(cmd_buffer);
> 
>         return VK_SUCCESS;
>     --
>     2.14.3
> 
> 
>