On Mon, Nov 03, 2025 at 02:18:06PM -0300, Gustavo Sousa wrote: > Starting with Xe3p_LPD, we now have a new field in MEM_SS_INFO_GLOBAL > that indicates whether the memory has enabled ECC that limits display > bandwidth. Add the field ecc_impacting_de_bw to struct dram_info to > contain that information and set it appropriately when probing for > memory info. > > Currently there are no instructions in Bspec on how to handle that case, > so let's throw a warning if we ever find such a scenario. > > v2: > - s/ecc_impacting_de/ecc_impacting_de_bw/ to be more specific. (Matt > Atwood) > - Add warning if ecc_impacting_de_bw is true, since we currently do > not have instructions on how to handle it. (Matt Roper) > v3: > - Check on ecc_impacting_de_bw for the warning only for Xe3p_LPD and > beyond. > - Change warning macro from drm_WARN_ON_ONCE() to drm_WARN_ON(). > > Bspec: 69131 > Cc: Jani Nikula <[email protected]> > Cc: Matt Atwood <[email protected]> > Cc: Matt Roper <[email protected]> > Signed-off-by: Gustavo Sousa <[email protected]> > --- > drivers/gpu/drm/i915/display/intel_bw.c | 9 +++++++++ > drivers/gpu/drm/i915/i915_reg.h | 1 + > drivers/gpu/drm/i915/soc/intel_dram.c | 4 ++++ > drivers/gpu/drm/i915/soc/intel_dram.h | 1 + > 4 files changed, 15 insertions(+) > > diff --git a/drivers/gpu/drm/i915/display/intel_bw.c > b/drivers/gpu/drm/i915/display/intel_bw.c > index 919b25a5fbac..1f6461be50ef 100644 > --- a/drivers/gpu/drm/i915/display/intel_bw.c > +++ b/drivers/gpu/drm/i915/display/intel_bw.c > @@ -805,6 +805,15 @@ void intel_bw_init_hw(struct intel_display *display) > if (!HAS_DISPLAY(display)) > return; > > + /* > + * Starting with Xe3p_LPD, the hardware tells us whether memory has ECC > + * enabled that would impact display bandwidth. However, so far there > + * are no instructions in Bspec on how to handle that case. Let's > + * complain if we ever find such a scenario. > + */ > + if (DISPLAY_VER(display) >= 35) > + drm_WARN_ON(display->drm, dram_info->ecc_impacting_de_bw); > + > if (DISPLAY_VER(display) >= 30) { > if (DISPLAY_VERx100(display) == 3002) > tgl_get_bw_info(display, dram_info, > &xe3lpd_3002_sa_info); > diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h > index 354ef75ef6a5..5bf3b4ab2baa 100644 > --- a/drivers/gpu/drm/i915/i915_reg.h > +++ b/drivers/gpu/drm/i915/i915_reg.h > @@ -1233,6 +1233,7 @@ > #define OROM_OFFSET_MASK REG_GENMASK(20, 16) > > #define MTL_MEM_SS_INFO_GLOBAL _MMIO(0x45700) > +#define XE3P_ECC_IMPACTING_DE REG_BIT(12) > #define MTL_N_OF_ENABLED_QGV_POINTS_MASK REG_GENMASK(11, 8) > #define MTL_N_OF_POPULATED_CH_MASK REG_GENMASK(7, 4) > #define MTL_DDR_TYPE_MASK REG_GENMASK(3, 0) > diff --git a/drivers/gpu/drm/i915/soc/intel_dram.c > b/drivers/gpu/drm/i915/soc/intel_dram.c > index 2e16346a6cc0..3e588762709a 100644 > --- a/drivers/gpu/drm/i915/soc/intel_dram.c > +++ b/drivers/gpu/drm/i915/soc/intel_dram.c > @@ -686,6 +686,7 @@ static int gen12_get_dram_info(struct drm_i915_private > *i915, struct dram_info * > > static int xelpdp_get_dram_info(struct drm_i915_private *i915, struct > dram_info *dram_info) > { > + struct intel_display *display = i915->display; > u32 val = intel_uncore_read(&i915->uncore, MTL_MEM_SS_INFO_GLOBAL); > > switch (REG_FIELD_GET(MTL_DDR_TYPE_MASK, val)) { > @@ -724,6 +725,9 @@ static int xelpdp_get_dram_info(struct drm_i915_private > *i915, struct dram_info > dram_info->num_qgv_points = > REG_FIELD_GET(MTL_N_OF_ENABLED_QGV_POINTS_MASK, val); > /* PSF GV points not supported in D14+ */ > > + if (DISPLAY_VER(display) >= 35) > + dram_info->ecc_impacting_de_bw = > REG_FIELD_GET(XE3P_ECC_IMPACTING_DE, val);
I don't think we really need REG_FIELD_GET() when checking a single bit to set a bool. A simple '&' would work as well. But it doesn't really hurt anything either so, Reviewed-by: Matt Roper <[email protected]> Matt > + > return 0; > } > > diff --git a/drivers/gpu/drm/i915/soc/intel_dram.h > b/drivers/gpu/drm/i915/soc/intel_dram.h > index 03a973f1c941..8475ee379daa 100644 > --- a/drivers/gpu/drm/i915/soc/intel_dram.h > +++ b/drivers/gpu/drm/i915/soc/intel_dram.h > @@ -30,6 +30,7 @@ struct dram_info { > u8 num_channels; > u8 num_qgv_points; > u8 num_psf_gv_points; > + bool ecc_impacting_de_bw; /* Only valid from Xe3p_LPD onward. */ > bool symmetric_memory; > bool has_16gb_dimms; > }; > > -- > 2.51.0 > -- Matt Roper Graphics Software Engineer Linux GPU Platform Enablement Intel Corporation
