[Mesa-dev] [PATCH] intel: Apply Geminilake "Barrier Mode" workaround.

Mon Jan 8 23:00:30 UTC 2018

On Thu, Jan 04, 2018 at 11:36:48AM -0800, Kenneth Graunke wrote:
> Apparently, Geminilake requires you to whack a chicken bit to select
> either compute or tessellation mode for barriers.  The recommendation
> is to switch between them at PIPELINE_SELECT time.
> 
> We may not need to do this all the time, but I don't know that it hurts
> either.  PIPELINE_SELECT is already a pretty giant stall.
> 
> This appears to fix hangs in tessellation control shaders with barriers
> on Geminilake.  Note that this requires a corresponding kernel change,
> 
>     drm/i915: Whitelist SLICE_COMMON_ECO_CHICKEN1 on Geminilake.
> 
> in order for the register write to actually happen.  Without an updated
> kernel, this register write will be noop'd and the fix will not work.
> ---
>  src/intel/genxml/gen9.xml                  |  8 ++++++++
>  src/intel/vulkan/genX_cmd_buffer.c         | 21 +++++++++++++++++++++
>  src/mesa/drivers/dri/i965/brw_defines.h    |  5 +++++
>  src/mesa/drivers/dri/i965/brw_misc_state.c | 15 +++++++++++++++
>  4 files changed, 49 insertions(+)
> 
> diff --git a/src/intel/genxml/gen9.xml b/src/intel/genxml/gen9.xml
> index 1422463693d..019d264fb70 100644
> --- a/src/intel/genxml/gen9.xml
> +++ b/src/intel/genxml/gen9.xml
> @@ -3710,6 +3710,14 @@
>      <field name="Color Compression Disable Mask" start="31" end="31" type="bool"/>
>    </register>
>  
> +  <register name="SLICE_COMMON_ECO_CHICKEN1" length="1" num="0x731c">
> +    <field name="GLK Barrier Mode" start="7" end="7" type="bool">

Kind of nitpicking, but this field means more than a simple
enable/disable kind of boolean. In other similar places we used "uint"
instead of "bool" to represent that, specially since you are assigning
value names to it. For instance, Floating Point Mode is like that, but
there are other examples. Maybe we should decide one or another way and
making it more consistent.

Regardless of that, this patch is

Reviewed-by: Rafael Antognolli <rafael.antognolli at intel.com>

> +      <value name="GLK_BARRIER_MODE_GPGPU" value="0"/>
> +      <value name="GLK_BARRIER_MODE_3D_HULL" value="1"/>
> +    </field>
> +    <field name="GLK Barrier Mode Mask" start="23" end="23" type="bool"/>
> +  </register>
> +
>    <register name="GFX_ARB_ERROR_RPT" length="1" num="0x40a0">
>      <field name="TLB Page Fault Error" start="0" end="0" type="bool"/>
>      <field name="RSTRM PAVP Read Invalid" start="1" end="1" type="bool"/>
> diff --git a/src/intel/vulkan/genX_cmd_buffer.c b/src/intel/vulkan/genX_cmd_buffer.c
> index 0bd3874db73..25fcce9d01a 100644
> --- a/src/intel/vulkan/genX_cmd_buffer.c
> +++ b/src/intel/vulkan/genX_cmd_buffer.c
> @@ -2714,6 +2714,8 @@ static void
>  genX(flush_pipeline_select)(struct anv_cmd_buffer *cmd_buffer,
>                              uint32_t pipeline)
>  {
> +   UNUSED const struct gen_device_info *devinfo = &cmd_buffer->device->info;
> +
>     if (cmd_buffer->state.current_pipeline == pipeline)
>        return;
>  
> @@ -2764,6 +2766,25 @@ genX(flush_pipeline_select)(struct anv_cmd_buffer *cmd_buffer,
>        ps.PipelineSelection = pipeline;
>     }
>  
> +#if GEN_GEN == 9
> +   if (devinfo->is_geminilake) {
> +      /* Project: DevGLK
> +       *
> +       * "This chicken bit works around a hardware issue with barrier logic
> +       *  encountered when switching between GPGPU and 3D pipelines.  To
> +       *  workaround the issue, this mode bit should be set after a pipeline
> +       *  is selected."
> +       */
> +      uint32_t scec;
> +      anv_pack_struct(&scec, GENX(SLICE_COMMON_ECO_CHICKEN1),
> +                      .GLKBarrierMode =
> +                          pipeline == GPGPU ? GLK_BARRIER_MODE_GPGPU
> +                                            : GLK_BARRIER_MODE_3D_HULL,
> +                      .GLKBarrierModeMask = 1);
> +      emit_lri(&cmd_buffer->batch, GENX(SLICE_COMMON_ECO_CHICKEN1_num), scec);
> +   }
> +#endif
> +
>     cmd_buffer->state.current_pipeline = pipeline;
>  }
>  
> diff --git a/src/mesa/drivers/dri/i965/brw_defines.h b/src/mesa/drivers/dri/i965/brw_defines.h
> index 99d41cf1a56..8bf6f68b67c 100644
> --- a/src/mesa/drivers/dri/i965/brw_defines.h
> +++ b/src/mesa/drivers/dri/i965/brw_defines.h
> @@ -1656,4 +1656,9 @@ enum brw_pixel_shader_coverage_mask_mode {
>  #define CS_DEBUG_MODE2                     0x20d8 /* Gen9+ */
>  # define CSDBG2_CONSTANT_BUFFER_ADDRESS_OFFSET_DISABLE (1 << 4)
>  
> +#define SLICE_COMMON_ECO_CHICKEN1          0x731c /* Gen9+ */
> +# define GLK_SCEC_BARRIER_MODE_GPGPU       (0 << 7)
> +# define GLK_SCEC_BARRIER_MODE_3D_HULL     (1 << 7)
> +# define GLK_SCEC_BARRIER_MODE_MASK        REG_MASK(1 << 7)
> +
>  #endif
> diff --git a/src/mesa/drivers/dri/i965/brw_misc_state.c b/src/mesa/drivers/dri/i965/brw_misc_state.c
> index a1ac0abe285..c4ef6812bff 100644
> --- a/src/mesa/drivers/dri/i965/brw_misc_state.c
> +++ b/src/mesa/drivers/dri/i965/brw_misc_state.c
> @@ -516,6 +516,21 @@ brw_emit_select_pipeline(struct brw_context *brw, enum brw_pipeline pipeline)
>        OUT_BATCH(0);
>        ADVANCE_BATCH();
>     }
> +
> +   if (devinfo->is_geminilake) {
> +      /* Project: DevGLK
> +       *
> +       * "This chicken bit works around a hardware issue with barrier logic
> +       *  encountered when switching between GPGPU and 3D pipelines.  To
> +       *  workaround the issue, this mode bit should be set after a pipeline
> +       *  is selected."
> +       */
> +      const unsigned barrier_mode =
> +         pipeline == BRW_RENDER_PIPELINE ? GLK_SCEC_BARRIER_MODE_3D_HULL
> +                                         : GLK_SCEC_BARRIER_MODE_GPGPU;
> +      brw_load_register_imm32(brw, SLICE_COMMON_ECO_CHICKEN1,
> +                              barrier_mode | GLK_SCEC_BARRIER_MODE_MASK);
> +   }
>  }
>  
>  /**
> -- 
> 2.15.1
> 
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev