Quoting Matt Roper (2025-10-15 14:48:30-03:00) >On Wed, Oct 15, 2025 at 12:15:06AM -0300, Gustavo Sousa wrote: >> From: Matt Atwood <[email protected]> >> >> Bandwidth parameters for Xe3p_LPD are basically the same as for Xe3_LPD. >> However, now Xe3p_LPD has the ecc_impacting_de field, which could impact >> how the derating is defined. >> >> For the cases where that field is true, we use xe3p_lpd_ecc_sa_info, >> similarly to what was done for Xe2_HPD. Note, however, that Bspec >> specifies the ECC derating value only for GDDR memory. For now, we just >> re-use the value that was defined for Xe2_HPD, namely 45. We need to >> confirm with the hardware team what would be the correct value(s) to use >> for the ECC case. > >I think we need to use .derating = 10. This specific block of the bspec >is a shared block that applies to lots of IPs/platforms. GDDR isn't >relevant to an LPD platform and .derating = 10 is the documented value >for all other types of memory.
In that case, do mean we should drop the patch adding the field ecc_impacting_de and unconditionally use xe3lpd_sa_info? In the meantime I'll try to get clarifications from HW team on this. -- Gustavo Sousa > > >Matt > >> >> Bspec: 68859, 69131 >> Signed-off-by: Matt Atwood <[email protected]> >> Signed-off-by: Gustavo Sousa <[email protected]> >> --- >> drivers/gpu/drm/i915/display/intel_bw.c | 21 ++++++++++++++++++++- >> 1 file changed, 20 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/gpu/drm/i915/display/intel_bw.c >> b/drivers/gpu/drm/i915/display/intel_bw.c >> index 8f5b86cd91b6..f0940ff9d19b 100644 >> --- a/drivers/gpu/drm/i915/display/intel_bw.c >> +++ b/drivers/gpu/drm/i915/display/intel_bw.c >> @@ -461,6 +461,20 @@ static const struct intel_sa_info xe3lpd_3002_sa_info = >> { >> .derating = 10, >> }; >> >> +static const struct intel_sa_info xe3p_lpd_ecc_sa_info = { >> + .deburst = 32, >> + .deprogbwlimit = 65, /* GB/s */ >> + .displayrtids = 256, >> + /* >> + * FIXME: The Bspec only shows that derating for ECC should be 45 >> for >> + * GDDR memory and does not mention other types of memory. For >> now, we >> + * just re-use that value, but we need to confirm whether that is >> + * correct or if there are different values depending on the memory >> + * type. >> + */ >> + .derating = 45, >> +}; >> + >> static int icl_get_bw_info(struct intel_display *display, >> const struct dram_info *dram_info, >> const struct intel_sa_info *sa) >> @@ -812,7 +826,12 @@ void intel_bw_init_hw(struct intel_display *display) >> if (!HAS_DISPLAY(display)) >> return; >> >> - if (DISPLAY_VERx100(display) >= 3002) { >> + if (DISPLAY_VER(display) >= 35) { >> + if (dram_info->ecc_impacting_de) >> + tgl_get_bw_info(display, dram_info, >> &xe3p_lpd_ecc_sa_info); >> + else >> + tgl_get_bw_info(display, dram_info, >> &xe3lpd_sa_info); >> + } else if (DISPLAY_VERx100(display) >= 3002) { >> tgl_get_bw_info(display, dram_info, &xe3lpd_3002_sa_info); >> } else if (DISPLAY_VER(display) >= 30) { >> tgl_get_bw_info(display, dram_info, &xe3lpd_sa_info); >> >> -- >> 2.51.0 >> > >-- >Matt Roper >Graphics Software Engineer >Linux GPU Platform Enablement >Intel Corporation
