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

[Raag Jadav]

On Tue, Dec 10, 2024 at 01:52:21PM +0100, Andi Shyti wrote:
> Hi Raag,
> 
> > +/* Wa_14022698537:dg2 */
> > +static void i915_enable_g8(struct drm_i915_private *i915)
> > +{
> > +	if (IS_DG2(i915)) {
> > +		if (IS_DG2_D(i915) && !intel_match_g8_cpu())
> > +			return;
> > +
> > +		snb_pcode_write_p(&i915->uncore, PCODE_POWER_SETUP,
> > +				  POWER_SETUP_SUBCOMMAND_G8_ENABLE, 0, 0);
> > +	}
> 
> In the workaround description there an "else if" which I am not
> understanding. I it suggesting to do nothing or is it suggesting
> to do the same thing?

We do the same thing (apply WA) except when the _D IDs are not paired
with whitelisted CPUs. What I did here is added a return call with
inverted CPU logic and got rid of the else part. I know it looks quirky
but does the job.

> > +}
> > +
> >  static int i915_pcode_init(struct drm_i915_private *i915)
> >  {
> >  	struct intel_gt *gt;
> > @@ -428,6 +442,7 @@ static int i915_pcode_init(struct drm_i915_private *i915)
> >  		}
> >  	}
> >  
> > +	i915_enable_g8(i915);
> >  	return 0;
> >  }
> >  
> > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> > index 89e4381f8baa..d400c77423a5 100644
> > --- a/drivers/gpu/drm/i915/i915_reg.h
> > +++ b/drivers/gpu/drm/i915/i915_reg.h
> > @@ -3617,6 +3617,7 @@
> >  #define	    POWER_SETUP_I1_WATTS		REG_BIT(31)
> >  #define	    POWER_SETUP_I1_SHIFT		6	/* 10.6 fixed point format */
> >  #define	    POWER_SETUP_I1_DATA_MASK		REG_GENMASK(15, 0)
> > +#define     POWER_SETUP_SUBCOMMAND_G8_ENABLE	0x6
> 
> for aesthetics 0x06 would look better, but this should be
> changed everywhere in the file because single digit hex values
> are used. It's better to keep uniformity in the style.

I agree about uniformity but perhaps a separate filewide patch would
be much cleaner IMHO. Let's keep it as it is for now.

> >  #define GEN12_PCODE_READ_SAGV_BLOCK_TIME_US	0x23
> >  #define   XEHP_PCODE_FREQUENCY_CONFIG		0x6e	/* pvc */
> >  /* XEHP_PCODE_FREQUENCY_CONFIG sub-commands (param1) */
> > -- 
> > 2.34.1