On Tue, 05 Aug 2025, Rodrigo Vivi <rodrigo.v...@intel.com> wrote:
> On Thu, Jul 31, 2025 at 05:21:25PM +0300, Jani Nikula wrote:
>> Store fsb_freq and mem_freq in dram info the same way we do for other
>> memory info on later platforms for a slightly more unified approach.
>>
>> This allows us to remove fsb_freq, mem_freq and is_ddr3 members from
>> struct drm_i915_private and struct xe_device.
>>
>> Signed-off-by: Jani Nikula <jani.nik...@intel.com>
>> ---
>> drivers/gpu/drm/i915/display/i9xx_wm.c | 13 +++++----
>> drivers/gpu/drm/i915/i915_drv.h | 2 --
>> drivers/gpu/drm/i915/soc/intel_dram.c | 38 +++++++++++---------------
>> drivers/gpu/drm/i915/soc/intel_dram.h | 2 ++
>> drivers/gpu/drm/xe/xe_device_types.h | 1 -
>> 5 files changed, 26 insertions(+), 30 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/display/i9xx_wm.c
>> b/drivers/gpu/drm/i915/display/i9xx_wm.c
>> index 1f9db5118777..591acce2a4b1 100644
>> --- a/drivers/gpu/drm/i915/display/i9xx_wm.c
>> +++ b/drivers/gpu/drm/i915/display/i9xx_wm.c
>> @@ -3,6 +3,8 @@
>> * Copyright © 2023 Intel Corporation
>> */
>>
>> +#include "soc/intel_dram.h"
>> +
>> #include "i915_drv.h"
>> #include "i915_reg.h"
>> #include "i9xx_wm.h"
>> @@ -85,7 +87,8 @@ static const struct cxsr_latency cxsr_latency_table[] = {
>>
>> static const struct cxsr_latency *pnv_get_cxsr_latency(struct intel_display
>> *display)
>> {
>> - struct drm_i915_private *i915 = to_i915(display->drm);
>> + const struct dram_info *dram_info = intel_dram_info(display->drm);
>> + bool is_ddr3 = dram_info->type == INTEL_DRAM_DDR3;
>
> does this deserves a separate patch? I'm not sure if I followed here...
The current check in the loop below is
i915->is_ddr3 == latency->is_ddr3
and with i915->is_ddr3 being replaced by dram_info->type, I thought it's
simpler to have that variable.
The alternative is to convert the cxsr_latency_table to use enum
intel_dram_type and INTEL_DRAM_DDR3, but I felt that's a bit much.
>
>> int i;
>>
>> for (i = 0; i < ARRAY_SIZE(cxsr_latency_table); i++) {
>> @@ -93,15 +96,15 @@ static const struct cxsr_latency
>> *pnv_get_cxsr_latency(struct intel_display *dis
>> bool is_desktop = !display->platform.mobile;
>>
>> if (is_desktop == latency->is_desktop &&
>> - i915->is_ddr3 == latency->is_ddr3 &&
>> - DIV_ROUND_CLOSEST(i915->fsb_freq, 1000) ==
>> latency->fsb_freq &&
>> - DIV_ROUND_CLOSEST(i915->mem_freq, 1000) ==
>> latency->mem_freq)
>> + is_ddr3 == latency->is_ddr3 &&
>> + DIV_ROUND_CLOSEST(dram_info->fsb_freq, 1000) ==
>> latency->fsb_freq &&
>> + DIV_ROUND_CLOSEST(dram_info->mem_freq, 1000) ==
>> latency->mem_freq)
>> return latency;
>> }
>>
>> drm_dbg_kms(display->drm,
>> "Could not find CxSR latency for DDR%s, FSB %u kHz, MEM %u
>> kHz\n",
>> - i915->is_ddr3 ? "3" : "2", i915->fsb_freq, i915->mem_freq);
>> + is_ddr3 ? "3" : "2", dram_info->fsb_freq,
>> dram_info->mem_freq);
>>
>> return NULL;
>> }
>> diff --git a/drivers/gpu/drm/i915/i915_drv.h
>> b/drivers/gpu/drm/i915/i915_drv.h
>> index 4e4e89746aa6..2f3965feada1 100644
>> --- a/drivers/gpu/drm/i915/i915_drv.h
>> +++ b/drivers/gpu/drm/i915/i915_drv.h
>> @@ -237,8 +237,6 @@ struct drm_i915_private {
>>
>> bool preserve_bios_swizzle;
>>
>> - unsigned int fsb_freq, mem_freq, is_ddr3;
>> -
>> unsigned int hpll_freq;
>> unsigned int czclk_freq;
>>
>> diff --git a/drivers/gpu/drm/i915/soc/intel_dram.c
>> b/drivers/gpu/drm/i915/soc/intel_dram.c
>> index d896fb67270f..6405a3d0b930 100644
>> --- a/drivers/gpu/drm/i915/soc/intel_dram.c
>> +++ b/drivers/gpu/drm/i915/soc/intel_dram.c
>> @@ -149,17 +149,6 @@ unsigned int intel_mem_freq(struct drm_i915_private
>> *i915)
>> return 0;
>> }
>>
>> -static void detect_mem_freq(struct drm_i915_private *i915)
>> -{
>> - i915->mem_freq = intel_mem_freq(i915);
>> -
>> - if (IS_PINEVIEW(i915))
>> - i915->is_ddr3 = pnv_is_ddr3(i915);
>> -
>> - if (i915->mem_freq)
>> - drm_dbg(&i915->drm, "DDR speed: %d kHz\n", i915->mem_freq);
>> -}
>> -
>> static unsigned int i9xx_fsb_freq(struct drm_i915_private *i915)
>> {
>> u32 fsb;
>> @@ -252,11 +241,20 @@ unsigned int intel_fsb_freq(struct drm_i915_private
>> *i915)
>> return 0;
>> }
>>
>> -static void detect_fsb_freq(struct drm_i915_private *i915)
>> +static int i915_get_dram_info(struct drm_i915_private *i915, struct
>> dram_info *dram_info)
>> {
>> - i915->fsb_freq = intel_fsb_freq(i915);
>> - if (i915->fsb_freq)
>> - drm_dbg(&i915->drm, "FSB frequency: %d kHz\n", i915->fsb_freq);
>> + dram_info->fsb_freq = intel_fsb_freq(i915);
>> + if (dram_info->fsb_freq)
>> + drm_dbg(&i915->drm, "FSB frequency: %d kHz\n",
>> dram_info->fsb_freq);
>> +
>> + dram_info->mem_freq = intel_mem_freq(i915);
>> + if (dram_info->mem_freq)
>> + drm_dbg(&i915->drm, "DDR speed: %d kHz\n", dram_info->mem_freq);
>> +
>> + if (IS_PINEVIEW(i915) && pnv_is_ddr3(i915))
>> + dram_info->type = INTEL_DRAM_DDR3;
>> +
>> + return 0;
>> }
>>
>> static int intel_dimm_num_devices(const struct dram_dimm_info *dimm)
>> @@ -728,12 +726,6 @@ int intel_dram_detect(struct drm_i915_private *i915)
>> if (IS_DG2(i915) || !HAS_DISPLAY(i915))
>> return 0;
>>
>> - detect_fsb_freq(i915);
>> - detect_mem_freq(i915);
>> -
>> - if (GRAPHICS_VER(i915) < 9)
>> - return 0;
>
> oh! this responds my last question in the previous patch...
Yeah, I could've referred to later changes there!
>
>> -
>> dram_info = drmm_kzalloc(&i915->drm, sizeof(*dram_info), GFP_KERNEL);
>> if (!dram_info)
>> return -ENOMEM;
>> @@ -754,8 +746,10 @@ int intel_dram_detect(struct drm_i915_private *i915)
>> ret = gen11_get_dram_info(i915, dram_info);
>> else if (IS_BROXTON(i915) || IS_GEMINILAKE(i915))
>> ret = bxt_get_dram_info(i915, dram_info);
>> - else
>> + else if (GRAPHICS_VER(i915) >= 9)
>> ret = skl_get_dram_info(i915, dram_info);
>> + else
>> + ret = i915_get_dram_info(i915, dram_info);
>>
>> drm_dbg_kms(&i915->drm, "DRAM type: %s\n",
>> intel_dram_type_str(dram_info->type));
>> diff --git a/drivers/gpu/drm/i915/soc/intel_dram.h
>> b/drivers/gpu/drm/i915/soc/intel_dram.h
>> index 5ba75e279e84..97d21894abdc 100644
>> --- a/drivers/gpu/drm/i915/soc/intel_dram.h
>> +++ b/drivers/gpu/drm/i915/soc/intel_dram.h
>> @@ -29,6 +29,8 @@ struct dram_info {
>> } type;
>> u8 num_qgv_points;
>> u8 num_psf_gv_points;
>> + unsigned int fsb_freq;
>> + unsigned int mem_freq;
>> };
>>
>> void intel_dram_edram_detect(struct drm_i915_private *i915);
>> diff --git a/drivers/gpu/drm/xe/xe_device_types.h
>> b/drivers/gpu/drm/xe/xe_device_types.h
>> index 38c8329b4d2c..e2206e867b33 100644
>> --- a/drivers/gpu/drm/xe/xe_device_types.h
>> +++ b/drivers/gpu/drm/xe/xe_device_types.h
>> @@ -609,7 +609,6 @@ struct xe_device {
>> struct {
>> unsigned int hpll_freq;
>> unsigned int czclk_freq;
>> - unsigned int fsb_freq, mem_freq, is_ddr3;
>> };
>> #endif
>> };
>> --
>> 2.39.5
>>
--
Jani Nikula, Intel
