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

[Raag Jadav <raag.jadav@intel.com>]

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@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);
> 
> 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