Re: [PATCH v2] drm/i915: Added Wa_18022495364

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Fri, Sep 08, 2023 at 03:11:42PM +0530, Dnyaneshwar Bhadane wrote:
> This workaround has two different implementations,
> one for gen 12 to DG2 and another for DG2 and later.
> BSpec: 11354, 56551

Side note:  it's generally not worth adding bspec references for simple
register pages these days.  Anyone who has internal bspec access can
look up the page just as easily using the register offset or name as
they can using this number, so this doesn't help anyone.  And in this
case it looks like the page numbers you gave are wrong for the platforms
this workaround is supposed to apply to:  11354 is for the pre-gen12
version of the register and 56551 is for the Xe2 version of the
instruction.

Bspec references are still good when there are narrative pages
describing general software flows, because those can often be difficult
to locate in the large table of contents.

> 
> v2:
> - Removed extra parentheses from the condition (Lucas)
> - Fixed spacing and new line (Lucas)
> 
> Signed-off-by: Dnyaneshwar Bhadane <dnyaneshwar.bhadane@xxxxxxxxx>
> Cc: Lucas De Marchi <lucas.demarchi@xxxxxxxxx>
> Reviewed-by: Garg, Nemesa <nemesa.garg@xxxxxxxxx>
> ---
>  drivers/gpu/drm/i915/gt/gen8_engine_cs.c    | 5 +++++
>  drivers/gpu/drm/i915/gt/intel_gt_regs.h     | 2 ++
>  drivers/gpu/drm/i915/gt/intel_workarounds.c | 4 ++++
>  3 files changed, 11 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/gt/gen8_engine_cs.c b/drivers/gpu/drm/i915/gt/gen8_engine_cs.c
> index 0143445dba83..e260defdfc23 100644
> --- a/drivers/gpu/drm/i915/gt/gen8_engine_cs.c
> +++ b/drivers/gpu/drm/i915/gt/gen8_engine_cs.c
> @@ -271,6 +271,11 @@ int gen12_emit_flush_rcs(struct i915_request *rq, u32 mode)
>  		if (GRAPHICS_VER_FULL(rq->i915) >= IP_VER(12, 70))
>  			bit_group_0 |= PIPE_CONTROL_CCS_FLUSH;
>  
> +		/* Wa_18022495364 */
> +		if (GRAPHICS_VER_FULL(rq->i915) >= IP_VER(12, 70) ||
> +		    IS_DG2(rq->i915))

This is going to apply the workaround not only to DG2, but also to *all*
platforms MTL and later forever.  Generally we should never have
workarounds marked this way...the expectation is that any hardware
workaround is going to go away eventually, and workaround conditions in
the code should only match the specific platforms listed as being
applicable in the workaround database.

Also, when I look in the workaround database, it doesn't appear that
this workaround is listed as applying to DG2, MTL (Xe_LPG), or LNL
(Xe2_LPG).  Is there some other workaround that requires the same
programming (but has a different workaround lineage #)?  If not, then
this part of the patch should just go away and only the gen12lp change
below should remain..


Matt

> +			bit_group_1 |= PIPE_CONTROL_CONST_CACHE_INVALIDATE;
> +
>  		bit_group_1 |= PIPE_CONTROL_TILE_CACHE_FLUSH;
>  		bit_group_1 |= PIPE_CONTROL_FLUSH_L3;
>  		bit_group_1 |= PIPE_CONTROL_RENDER_TARGET_CACHE_FLUSH;
> diff --git a/drivers/gpu/drm/i915/gt/intel_gt_regs.h b/drivers/gpu/drm/i915/gt/intel_gt_regs.h
> index 0e4c638fcbbf..4c0cb3a3d0aa 100644
> --- a/drivers/gpu/drm/i915/gt/intel_gt_regs.h
> +++ b/drivers/gpu/drm/i915/gt/intel_gt_regs.h
> @@ -164,6 +164,8 @@
>  #define GEN9_CSFE_CHICKEN1_RCS			_MMIO(0x20d4)
>  #define   GEN9_PREEMPT_GPGPU_SYNC_SWITCH_DISABLE	(1 << 2)
>  #define   GEN11_ENABLE_32_PLANE_MODE		(1 << 7)
> +#define GEN12_CS_DEBUG_MODE2			_MMIO(0x20d8)
> +#define   GEN12_GLOBAL_DEBUG_ENABLE			BIT(6)
>  
>  #define GEN7_FF_SLICE_CS_CHICKEN1		_MMIO(0x20e0)
>  #define   GEN9_FFSC_PERCTX_PREEMPT_CTRL		(1 << 14)
> diff --git a/drivers/gpu/drm/i915/gt/intel_workarounds.c b/drivers/gpu/drm/i915/gt/intel_workarounds.c
> index 864d41bcf6bb..efdb4bbf8083 100644
> --- a/drivers/gpu/drm/i915/gt/intel_workarounds.c
> +++ b/drivers/gpu/drm/i915/gt/intel_workarounds.c
> @@ -712,6 +712,10 @@ static void gen12_ctx_workarounds_init(struct intel_engine_cs *engine,
>  			    GEN9_PREEMPT_GPGPU_LEVEL_MASK,
>  			    GEN9_PREEMPT_GPGPU_THREAD_GROUP_LEVEL);
>  
> +	/* Wa_18022495364 */
> +	wa_masked_en(wal, GEN12_CS_DEBUG_MODE2,
> +		     GEN12_GLOBAL_DEBUG_ENABLE);
> +
>  	/*
>  	 * Wa_16011163337 - GS_TIMER
>  	 *
> -- 
> 2.34.1
> 

-- 
Matt Roper
Graphics Software Engineer
Linux GPU Platform Enablement
Intel Corporation



[Index of Archives]     [AMD Graphics]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux