On Wed, Oct 15, 2025 at 01:13:33PM -0300, Gustavo Sousa wrote:
> Quoting Jani Nikula (2025-10-15 11:46:52-03:00)
> >On Wed, 15 Oct 2025, Gustavo Sousa <[email protected]> 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 to struct dram_info to
> >> contain that information and set it appropriately when probing for
> >> memory info.  We will use that field when updating bandwidth parameters
> >> for Xe3p_LPD.
> >
> >Could the field name be more accurate than "ecc impacting de"? It sounds
> >quite handwavy to me.
> 
> Well, perhaps the innacurate part would be the generic "de" instead of
> something that refers to the bandwidth?
> 
> If so, would you be fine with ecc_impacting_bandwidth?
If we're going to elaborate I'd prefer it to be ecc_impacting_de_bw
1. It matches the name in the first part
2. The additional context talks about what changes

MattA
> 
> --
> Gustavo Sousa
> 
> >
> >BR,
> >Jani.
> >
> >>
> >> Bspec: 69131
> >> Signed-off-by: Gustavo Sousa <[email protected]>
> >> ---
> >>  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 +
> >>  3 files changed, 6 insertions(+)
> >>
> >> 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 8841cfe1cac8..bf9f8e38d6ba 100644
> >> --- a/drivers/gpu/drm/i915/soc/intel_dram.c
> >> +++ b/drivers/gpu/drm/i915/soc/intel_dram.c
> >> @@ -685,6 +685,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)) {
> >> @@ -723,6 +724,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 = 
> >> REG_FIELD_GET(XE3P_ECC_IMPACTING_DE, val);
> >> +
> >>          return 0;
> >>  }
> >>  
> >> diff --git a/drivers/gpu/drm/i915/soc/intel_dram.h 
> >> b/drivers/gpu/drm/i915/soc/intel_dram.h
> >> index 03a973f1c941..ac77f1ab409f 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; /* Only valid from Xe3p_LPD onward. */
> >>          bool symmetric_memory;
> >>          bool has_16gb_dimms;
> >>  };
> >
> >-- 
> >Jani Nikula, Intel

Reply via email to