On Tue, Sep 02, 2025 at 04:56:18PM +0300, Jani Nikula wrote:
> On Tue, 02 Sep 2025, Ville Syrjala <ville.syrj...@linux.intel.com> wrote:
> > From: Ville Syrjälä <ville.syrj...@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.nik...@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.syrj...@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
