[PATCH v2 4/4] drm/i915/dg2: Implement Wa_14022698537

[Gupta, Anshuman]

> -----Original Message-----
> From: Jadav, Raag <raag.jadav at intel.com>
> Sent: Wednesday, October 23, 2024 12:40 PM
> To: Nilawar, Badal <badal.nilawar at intel.com>
> Cc: jani.nikula at linux.intel.com; joonas.lahtinen at linux.intel.com; Vivi, Rodrigo
> <rodrigo.vivi at intel.com>; Roper, Matthew D <matthew.d.roper at intel.com>;
> andi.shyti at linux.intel.com; intel-gfx at lists.freedesktop.org; Gupta, Anshuman
> <anshuman.gupta at intel.com>; Tauro, Riana <riana.tauro at intel.com>
> Subject: Re: [PATCH v2 4/4] drm/i915/dg2: Implement Wa_14022698537
> 
> On Tue, Oct 22, 2024 at 06:41:57PM +0530, Nilawar, Badal wrote:
> > On 11-10-2024 16:02, Raag Jadav wrote:
> > > G8 power state entry is disabled due to a limitation on DG2, so we
> > > enable it from driver with Wa_14022698537. Fow now we enable it for
> > > all DG2 devices with the exception of a few, for which, we enable
> > > only when paired with whitelisted CPU models.
> > >
> > > v2: Fix Wa_ID and include it in subject (Badal)
> > >      Rephrase commit message (Jani)
> > >
> > > Signed-off-by: Raag Jadav <raag.jadav at intel.com>
> > > ---
> > >   drivers/gpu/drm/i915/gt/intel_workarounds.c | 18 ++++++++++++++++++
> > >   drivers/gpu/drm/i915/i915_reg.h             |  1 +
> > >   2 files changed, 19 insertions(+)
> > >
> > > diff --git a/drivers/gpu/drm/i915/gt/intel_workarounds.c
> > > b/drivers/gpu/drm/i915/gt/intel_workarounds.c
> > > index e539a656cfc3..bcd7630c1631 100644
> > > --- a/drivers/gpu/drm/i915/gt/intel_workarounds.c
> > > +++ b/drivers/gpu/drm/i915/gt/intel_workarounds.c
> > > @@ -14,6 +14,7 @@
> > >   #include "intel_gt_mcr.h"
> > >   #include "intel_gt_print.h"
> > >   #include "intel_gt_regs.h"
> > > +#include "intel_pcode.h"
> > >   #include "intel_ring.h"
> > >   #include "intel_workarounds.h"
> > > @@ -1770,9 +1771,26 @@ static void wa_list_apply(const struct
> i915_wa_list *wal)
> > >   	intel_gt_mcr_unlock(gt, flags);
> > >   }
> > > +/* Wa_14022698537:dg2 */
> > > +static void intel_enable_g8(struct intel_uncore *uncore) {
> > > +	struct drm_i915_private *i915 = uncore->i915;
> > > +
> > > +	if (IS_DG2(i915)) {
> > > +		if (IS_DG2_WA(i915) && !intel_match_wa_cpu())
> > > +			return;
> > > +
> > > +		snb_pcode_write_p(uncore, PCODE_POWER_SETUP,
> > > +				  POWER_SETUP_SUBCOMMAND_G8_ENABLE,
> 0, 0);
> > > +	}
> > > +}
> > > +
> > >   void intel_gt_apply_workarounds(struct intel_gt *gt)
> > >   {
> > >   	wa_list_apply(&gt->wa_list);
> > > +
> > > +	/* Special case for pcode mailbox which can't be on wa_list */
> > > +	intel_enable_g8(gt->uncore);
My earlier comment was to include this WA in intel_gt_apply_workarounds with  an intention to
Add to wa_list_apply() but seems it is not feasible to keep in that list.
Therefore it is better to keep call this WA function from i915_pcode_init to wrap all pcode related
Functionality.

Thanks,
Anshuman Gupta.  
> >
> > This workaround is not specific to GT; G8 is a state specific to the SoC.
> > Therefore, move this workaround above the GT layer and pass argument
> > i915->uncore instead of gt->uncore
> 
> Since this WA needs to be applied on suspend/resume/reset cycles, I found it to
> be more suitable here, atleast according to the documentation.
> 
>  * - GT workarounds: the list of these WAs is applied whenever these registers
>  *   revert to their default values: on GPU reset, suspend/resume [1]_, etc.
> 
> We can either limit this to primary gt (using gt->info.id) here or move this to
> i915_pcode_init() instead, whichever is the better option.
> 
> Raag