Re: [PATCH 5/8] drm/i915/dram: Move 16Gb DIMM detection fully to the skl/icl codepaths

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

On Tue, Sep 02, 2025 at 04:56:18PM +0300, Jani Nikula wrote:
> On Tue, 02 Sep 2025, Ville Syrjala <ville.syrjala@linux.intel.com> wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >
> > Only skl (and derivatives) and icl need the 16Gb DIMM w/as, and we
> > do the real detection only on those platforms anyway. Move the initial
> > has_16gb_dimms assignment into those codepaths as well.
> >
> > Currently we are incorrectly always applying the 1 usec wm latency bump
> > on all mtl+ platforms due to this (the tgl/adl codepath did remember to
> > undo the assignment, but the mtl+ codepath didn't).
> 
> The commit message could have more clarity that we're fixing this here.
> 
> With that,
> 
> Reviewed-by: Jani Nikula <jani.nikula@intel.com>
> 
> 
> I think after this we could also drop the "!display->platform.dg2 && "
> part in adjust_wm_latency():
> 
> 	if (!display->platform.dg2 && dram_info->has_16gb_dimms)
> 
> Can be a follow-up.

Currently we don't seem to allocate dram_info at all for dg2. So we
need some protection there to avoid oopsing. But I think I'll be
changing that to explictily check for the skl/icl platforms instead
since those are the only ones that need the w/a.

> 
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> >  drivers/gpu/drm/i915/soc/intel_dram.c | 17 ++++++-----------
> >  1 file changed, 6 insertions(+), 11 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/soc/intel_dram.c b/drivers/gpu/drm/i915/soc/intel_dram.c
> > index efb72e137748..bf3ba874f8c5 100644
> > --- a/drivers/gpu/drm/i915/soc/intel_dram.c
> > +++ b/drivers/gpu/drm/i915/soc/intel_dram.c
> > @@ -406,6 +406,9 @@ skl_dram_get_channels_info(struct drm_i915_private *i915, struct dram_info *dram
> >  	u32 val;
> >  	int ret;
> >  
> > +	/* Assume 16Gb DIMMs are present until proven otherwise */
> > +	dram_info->has_16gb_dimms = true;
> > +
> >  	val = intel_uncore_read(&i915->uncore,
> >  				SKL_MAD_DIMM_CH0_0_0_0_MCHBAR_MCMAIN);
> >  	ret = skl_dram_get_channel_info(i915, &ch0, 0, val);
> > @@ -435,6 +438,9 @@ skl_dram_get_channels_info(struct drm_i915_private *i915, struct dram_info *dram
> >  	drm_dbg_kms(&i915->drm, "Memory configuration is symmetric? %s\n",
> >  		    str_yes_no(dram_info->symmetric_memory));
> >  
> > +	drm_dbg_kms(&i915->drm, "16Gb DIMMs: %s\n",
> > +		    str_yes_no(dram_info->has_16gb_dimms));
> > +
> >  	return 0;
> >  }
> >  
> > @@ -673,8 +679,6 @@ static int gen11_get_dram_info(struct drm_i915_private *i915, struct dram_info *
> >  
> >  static int gen12_get_dram_info(struct drm_i915_private *i915, struct dram_info *dram_info)
> >  {
> > -	dram_info->has_16gb_dimms = false;
> > -
> >  	return icl_pcode_read_mem_global_info(i915, dram_info);
> >  }
> >  
> > @@ -736,12 +740,6 @@ int intel_dram_detect(struct drm_i915_private *i915)
> >  
> >  	i915->dram_info = dram_info;
> >  
> > -	/*
> > -	 * Assume 16Gb DIMMs are present until proven
> > -	 * otherwise, this w/a is not needed by bxt/glk.
> > -	 */
> > -	dram_info->has_16gb_dimms = !IS_BROXTON(i915) && !IS_GEMINILAKE(i915);
> > -
> >  	if (DISPLAY_VER(display) >= 14)
> >  		ret = xelpdp_get_dram_info(i915, dram_info);
> >  	else if (GRAPHICS_VER(i915) >= 12)
> > @@ -766,9 +764,6 @@ int intel_dram_detect(struct drm_i915_private *i915)
> >  
> >  	drm_dbg_kms(&i915->drm, "DRAM channels: %u\n", dram_info->num_channels);
> >  
> > -	drm_dbg_kms(&i915->drm, "16Gb DIMMs: %s\n",
> > -		    str_yes_no(dram_info->has_16gb_dimms));
> > -
> >  	return 0;
> >  }
> 
> -- 
> Jani Nikula, Intel

-- 
Ville Syrjälä
Intel