> -----Original Message-----
> From: Jadav, Raag <raag.jadav@xxxxxxxxx>
> Sent: Wednesday, October 23, 2024 12:40 PM
> To: Nilawar, Badal <badal.nilawar@xxxxxxxxx>
> Cc: jani.nikula@xxxxxxxxxxxxxxx; joonas.lahtinen@xxxxxxxxxxxxxxx; Vivi, Rodrigo
> <rodrigo.vivi@xxxxxxxxx>; Roper, Matthew D <matthew.d.roper@xxxxxxxxx>;
> andi.shyti@xxxxxxxxxxxxxxx; intel-gfx@xxxxxxxxxxxxxxxxxxxxx; Gupta, Anshuman
> <anshuman.gupta@xxxxxxxxx>; Tauro, Riana <riana.tauro@xxxxxxxxx>
> 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@xxxxxxxxx>
> > > ---
> > > 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(>->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
