[PATCH 0/5] drm/i915 and drm/xe: remove fsb/mem freq cleanups

[PATCH 0/5] drm/i915 and drm/xe: remove fsb/mem freq cleanups

[Jani Nikula]

Main goal here is to remove the fsb_freq, mem_freq, and is_ddr3 members
from drm_i915_private and xe_device.

Jani Nikula (5):
  drm/i915/dram: add intel_fsb_freq() and use it
  drm/i915/dram: add intel_mem_freq()
  drm/i915/rps: use intel_fsb_freq() and intel_mem_freq()
  drm/i915/dram: bypass fsb/mem freq detection on dg2 and no display
  drm/i915/dram: move fsb_freq and mem_freq to dram info

 drivers/gpu/drm/i915/display/i9xx_wm.c        | 13 +++--
 drivers/gpu/drm/i915/display/intel_cdclk.c    |  2 +-
 .../gpu/drm/i915/gt/intel_gt_clock_utils.c    |  2 +-
 drivers/gpu/drm/i915/gt/intel_rps.c           | 11 ++--
 drivers/gpu/drm/i915/i915_drv.h               |  2 -
 drivers/gpu/drm/i915/soc/intel_dram.c         | 54 +++++++++++--------
 drivers/gpu/drm/i915/soc/intel_dram.h         |  5 +-
 drivers/gpu/drm/xe/xe_device_types.h          |  1 -
 8 files changed, 54 insertions(+), 36 deletions(-)

-- 
2.39.5

[PATCH 1/5] drm/i915/dram: add intel_fsb_freq() and use it

[Jani Nikula]

Add a more generic intel_fsb_freq() function instead of platform
specific ones.

Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
 drivers/gpu/drm/i915/display/intel_cdclk.c     |  2 +-
 drivers/gpu/drm/i915/gt/intel_gt_clock_utils.c |  2 +-
 drivers/gpu/drm/i915/soc/intel_dram.c          | 14 ++++++++++----
 drivers/gpu/drm/i915/soc/intel_dram.h          |  2 +-
 4 files changed, 13 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.c b/drivers/gpu/drm/i915/display/intel_cdclk.c
index 228aa64c1349..50f8e8e7b2a2 100644
--- a/drivers/gpu/drm/i915/display/intel_cdclk.c
+++ b/drivers/gpu/drm/i915/display/intel_cdclk.c
@@ -3569,7 +3569,7 @@ static int i9xx_hrawclk(struct intel_display *display)
 	struct drm_i915_private *i915 = to_i915(display->drm);
 
 	/* hrawclock is 1/4 the FSB frequency */
-	return DIV_ROUND_CLOSEST(i9xx_fsb_freq(i915), 4);
+	return DIV_ROUND_CLOSEST(intel_fsb_freq(i915), 4);
 }
 
 /**
diff --git a/drivers/gpu/drm/i915/gt/intel_gt_clock_utils.c b/drivers/gpu/drm/i915/gt/intel_gt_clock_utils.c
index 6c499692d61e..88b147fa5cb1 100644
--- a/drivers/gpu/drm/i915/gt/intel_gt_clock_utils.c
+++ b/drivers/gpu/drm/i915/gt/intel_gt_clock_utils.c
@@ -148,7 +148,7 @@ static u32 gen4_read_clock_frequency(struct intel_uncore *uncore)
 	 *
 	 * Testing on actual hardware has shown there is no /16.
 	 */
-	return DIV_ROUND_CLOSEST(i9xx_fsb_freq(uncore->i915), 4) * 1000;
+	return DIV_ROUND_CLOSEST(intel_fsb_freq(uncore->i915), 4) * 1000;
 }
 
 static u32 read_clock_frequency(struct intel_uncore *uncore)
diff --git a/drivers/gpu/drm/i915/soc/intel_dram.c b/drivers/gpu/drm/i915/soc/intel_dram.c
index deb159548a09..6be3618d4885 100644
--- a/drivers/gpu/drm/i915/soc/intel_dram.c
+++ b/drivers/gpu/drm/i915/soc/intel_dram.c
@@ -153,7 +153,7 @@ static void detect_mem_freq(struct drm_i915_private *i915)
 		drm_dbg(&i915->drm, "DDR speed: %d kHz\n", i915->mem_freq);
 }
 
-unsigned int i9xx_fsb_freq(struct drm_i915_private *i915)
+static unsigned int i9xx_fsb_freq(struct drm_i915_private *i915)
 {
 	u32 fsb;
 
@@ -235,13 +235,19 @@ static unsigned int ilk_fsb_freq(struct drm_i915_private *dev_priv)
 	}
 }
 
-static void detect_fsb_freq(struct drm_i915_private *i915)
+unsigned int intel_fsb_freq(struct drm_i915_private *i915)
 {
 	if (GRAPHICS_VER(i915) == 5)
-		i915->fsb_freq = ilk_fsb_freq(i915);
+		return ilk_fsb_freq(i915);
 	else if (GRAPHICS_VER(i915) == 3 || GRAPHICS_VER(i915) == 4)
-		i915->fsb_freq = i9xx_fsb_freq(i915);
+		return i9xx_fsb_freq(i915);
+	else
+		return 0;
+}
 
+static void detect_fsb_freq(struct drm_i915_private *i915)
+{
+	i915->fsb_freq = intel_fsb_freq(i915);
 	if (i915->fsb_freq)
 		drm_dbg(&i915->drm, "FSB frequency: %d kHz\n", i915->fsb_freq);
 }
diff --git a/drivers/gpu/drm/i915/soc/intel_dram.h b/drivers/gpu/drm/i915/soc/intel_dram.h
index 2a696e03aad4..09a7a581d949 100644
--- a/drivers/gpu/drm/i915/soc/intel_dram.h
+++ b/drivers/gpu/drm/i915/soc/intel_dram.h
@@ -33,7 +33,7 @@ struct dram_info {
 
 void intel_dram_edram_detect(struct drm_i915_private *i915);
 int intel_dram_detect(struct drm_i915_private *i915);
-unsigned int i9xx_fsb_freq(struct drm_i915_private *i915);
+unsigned int intel_fsb_freq(struct drm_i915_private *i915);
 const struct dram_info *intel_dram_info(struct drm_device *drm);
 
 #endif /* __INTEL_DRAM_H__ */
-- 
2.39.5

[PATCH 2/5] drm/i915/dram: add intel_mem_freq()

[Jani Nikula]

Add a more generic intel_mem_freq() function instead of platform
specific ones. Expose it for future use outside of intel_dram.c.

Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
 drivers/gpu/drm/i915/soc/intel_dram.c | 17 ++++++++++++-----
 drivers/gpu/drm/i915/soc/intel_dram.h |  1 +
 2 files changed, 13 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/i915/soc/intel_dram.c b/drivers/gpu/drm/i915/soc/intel_dram.c
index 6be3618d4885..193e7f71a356 100644
--- a/drivers/gpu/drm/i915/soc/intel_dram.c
+++ b/drivers/gpu/drm/i915/soc/intel_dram.c
@@ -135,16 +135,23 @@ static unsigned int vlv_mem_freq(struct drm_i915_private *i915)
 	return 0;
 }
 
-static void detect_mem_freq(struct drm_i915_private *i915)
+unsigned int intel_mem_freq(struct drm_i915_private *i915)
 {
 	if (IS_PINEVIEW(i915))
-		i915->mem_freq = pnv_mem_freq(i915);
+		return pnv_mem_freq(i915);
 	else if (GRAPHICS_VER(i915) == 5)
-		i915->mem_freq = ilk_mem_freq(i915);
+		return ilk_mem_freq(i915);
 	else if (IS_CHERRYVIEW(i915))
-		i915->mem_freq = chv_mem_freq(i915);
+		return chv_mem_freq(i915);
 	else if (IS_VALLEYVIEW(i915))
-		i915->mem_freq = vlv_mem_freq(i915);
+		return vlv_mem_freq(i915);
+	else
+		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);
diff --git a/drivers/gpu/drm/i915/soc/intel_dram.h b/drivers/gpu/drm/i915/soc/intel_dram.h
index 09a7a581d949..5ba75e279e84 100644
--- a/drivers/gpu/drm/i915/soc/intel_dram.h
+++ b/drivers/gpu/drm/i915/soc/intel_dram.h
@@ -34,6 +34,7 @@ struct dram_info {
 void intel_dram_edram_detect(struct drm_i915_private *i915);
 int intel_dram_detect(struct drm_i915_private *i915);
 unsigned int intel_fsb_freq(struct drm_i915_private *i915);
+unsigned int intel_mem_freq(struct drm_i915_private *i915);
 const struct dram_info *intel_dram_info(struct drm_device *drm);
 
 #endif /* __INTEL_DRAM_H__ */
-- 
2.39.5

[PATCH 3/5] drm/i915/rps: use intel_fsb_freq() and intel_mem_freq()

[Jani Nikula]

The rps init only happens once, so it's not important to use the cached
versions, and we can drop the dependency on them.

Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
 drivers/gpu/drm/i915/gt/intel_rps.c | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/gt/intel_rps.c b/drivers/gpu/drm/i915/gt/intel_rps.c
index 0b35fdd461d4..006042e0b229 100644
--- a/drivers/gpu/drm/i915/gt/intel_rps.c
+++ b/drivers/gpu/drm/i915/gt/intel_rps.c
@@ -9,6 +9,7 @@
 
 #include "display/intel_display.h"
 #include "display/intel_display_rps.h"
+#include "soc/intel_dram.h"
 #include "i915_drv.h"
 #include "i915_irq.h"
 #include "i915_reg.h"
@@ -276,20 +277,24 @@ static void gen5_rps_init(struct intel_rps *rps)
 {
 	struct drm_i915_private *i915 = rps_to_i915(rps);
 	struct intel_uncore *uncore = rps_to_uncore(rps);
+	unsigned int fsb_freq, mem_freq;
 	u8 fmax, fmin, fstart;
 	u32 rgvmodectl;
 	int c_m, i;
 
-	if (i915->fsb_freq <= 3200000)
+	fsb_freq = intel_fsb_freq(i915);
+	mem_freq = intel_mem_freq(i915);
+
+	if (fsb_freq <= 3200000)
 		c_m = 0;
-	else if (i915->fsb_freq <= 4800000)
+	else if (fsb_freq <= 4800000)
 		c_m = 1;
 	else
 		c_m = 2;
 
 	for (i = 0; i < ARRAY_SIZE(cparams); i++) {
 		if (cparams[i].i == c_m &&
-		    cparams[i].t == DIV_ROUND_CLOSEST(i915->mem_freq, 1000)) {
+		    cparams[i].t == DIV_ROUND_CLOSEST(mem_freq, 1000)) {
 			rps->ips.m = cparams[i].m;
 			rps->ips.c = cparams[i].c;
 			break;
-- 
2.39.5

[PATCH 4/5] drm/i915/dram: bypass fsb/mem freq detection on dg2 and no display

[Jani Nikula]

Non-display now calls the intel_fsb_freq() and intel_mem_freq()
functions, so we don't have to have the frequencies initialized for dg2
or non-display cases.

Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
 drivers/gpu/drm/i915/soc/intel_dram.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/soc/intel_dram.c b/drivers/gpu/drm/i915/soc/intel_dram.c
index 193e7f71a356..d896fb67270f 100644
--- a/drivers/gpu/drm/i915/soc/intel_dram.c
+++ b/drivers/gpu/drm/i915/soc/intel_dram.c
@@ -725,10 +725,13 @@ int intel_dram_detect(struct drm_i915_private *i915)
 	struct dram_info *dram_info;
 	int ret;
 
+	if (IS_DG2(i915) || !HAS_DISPLAY(i915))
+		return 0;
+
 	detect_fsb_freq(i915);
 	detect_mem_freq(i915);
 
-	if (GRAPHICS_VER(i915) < 9 || IS_DG2(i915) || !HAS_DISPLAY(i915))
+	if (GRAPHICS_VER(i915) < 9)
 		return 0;
 
 	dram_info = drmm_kzalloc(&i915->drm, sizeof(*dram_info), GFP_KERNEL);
-- 
2.39.5

[PATCH 5/5] drm/i915/dram: move fsb_freq and mem_freq to dram info

[Jani Nikula]

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.nikula@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;
 	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;
-
 	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

✗ i915.CI.BAT: failure for drm/i915 and drm/xe: remove fsb/mem freq cleanups

[Patchwork]

[-- Attachment #1: Type: text/plain, Size: 3749 bytes --]

== Series Details ==

Series: drm/i915 and drm/xe: remove fsb/mem freq cleanups
URL   : https://patchwork.freedesktop.org/series/152363/
State : failure

== Summary ==

CI Bug Log - changes from CI_DRM_16945 -> Patchwork_152363v1
====================================================

Summary
-------

  **FAILURE**

  Serious unknown changes coming with Patchwork_152363v1 absolutely need to be
  verified manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in Patchwork_152363v1, please notify your bug team (I915-ci-infra@lists.freedesktop.org) to allow them
  to document this new failure mode, which will reduce false positives in CI.

  External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_152363v1/index.html

Participating hosts (45 -> 44)
------------------------------

  Missing    (1): fi-snb-2520m 

Possible new issues
-------------------

  Here are the unknown changes that may have been introduced in Patchwork_152363v1:

### IGT changes ###

#### Possible regressions ####

  * igt@i915_selftest@live@guc_multi_lrc:
    - fi-bsw-n3050:       [PASS][1] -> [ABORT][2] +1 other test abort
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_16945/fi-bsw-n3050/igt@i915_selftest@live@guc_multi_lrc.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_152363v1/fi-bsw-n3050/igt@i915_selftest@live@guc_multi_lrc.html

  
Known issues
------------

  Here are the changes found in Patchwork_152363v1 that come from known issues:

### IGT changes ###

#### Possible fixes ####

  * igt@i915_selftest@live@gt_pm:
    - bat-jsl-1:          [DMESG-FAIL][3] ([i915#13774]) -> [PASS][4]
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_16945/bat-jsl-1/igt@i915_selftest@live@gt_pm.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_152363v1/bat-jsl-1/igt@i915_selftest@live@gt_pm.html

  
#### Warnings ####

  * igt@i915_selftest@live:
    - bat-jsl-1:          [DMESG-FAIL][5] ([i915#13774] / [i915#13827]) -> [DMESG-WARN][6] ([i915#13827])
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_16945/bat-jsl-1/igt@i915_selftest@live.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_152363v1/bat-jsl-1/igt@i915_selftest@live.html
    - bat-atsm-1:         [DMESG-FAIL][7] ([i915#12061] / [i915#13929]) -> [DMESG-FAIL][8] ([i915#12061] / [i915#14204])
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_16945/bat-atsm-1/igt@i915_selftest@live.html
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_152363v1/bat-atsm-1/igt@i915_selftest@live.html

  * igt@i915_selftest@live@mman:
    - bat-atsm-1:         [DMESG-FAIL][9] ([i915#13929]) -> [DMESG-FAIL][10] ([i915#14204])
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_16945/bat-atsm-1/igt@i915_selftest@live@mman.html
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_152363v1/bat-atsm-1/igt@i915_selftest@live@mman.html

  
  [i915#12061]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/12061
  [i915#13774]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/13774
  [i915#13827]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/13827
  [i915#13929]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/13929
  [i915#14204]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/14204


Build changes
-------------

  * Linux: CI_DRM_16945 -> Patchwork_152363v1

  CI-20190529: 20190529
  CI_DRM_16945: 1ecbc700ab547ec8354e03e45629e576b861993a @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_8482: 8482
  Patchwork_152363v1: 1ecbc700ab547ec8354e03e45629e576b861993a @ git://anongit.freedesktop.org/gfx-ci/linux

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_152363v1/index.html

[-- Attachment #2: Type: text/html, Size: 4805 bytes --]

Re: [PATCH 1/5] drm/i915/dram: add intel_fsb_freq() and use it

[Rodrigo Vivi]

On Thu, Jul 31, 2025 at 05:21:21PM +0300, Jani Nikula wrote:
> Add a more generic intel_fsb_freq() function instead of platform
> specific ones.
> 
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>

Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>

> ---
>  drivers/gpu/drm/i915/display/intel_cdclk.c     |  2 +-
>  drivers/gpu/drm/i915/gt/intel_gt_clock_utils.c |  2 +-
>  drivers/gpu/drm/i915/soc/intel_dram.c          | 14 ++++++++++----
>  drivers/gpu/drm/i915/soc/intel_dram.h          |  2 +-
>  4 files changed, 13 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_cdclk.c b/drivers/gpu/drm/i915/display/intel_cdclk.c
> index 228aa64c1349..50f8e8e7b2a2 100644
> --- a/drivers/gpu/drm/i915/display/intel_cdclk.c
> +++ b/drivers/gpu/drm/i915/display/intel_cdclk.c
> @@ -3569,7 +3569,7 @@ static int i9xx_hrawclk(struct intel_display *display)
>  	struct drm_i915_private *i915 = to_i915(display->drm);
>  
>  	/* hrawclock is 1/4 the FSB frequency */
> -	return DIV_ROUND_CLOSEST(i9xx_fsb_freq(i915), 4);
> +	return DIV_ROUND_CLOSEST(intel_fsb_freq(i915), 4);
>  }
>  
>  /**
> diff --git a/drivers/gpu/drm/i915/gt/intel_gt_clock_utils.c b/drivers/gpu/drm/i915/gt/intel_gt_clock_utils.c
> index 6c499692d61e..88b147fa5cb1 100644
> --- a/drivers/gpu/drm/i915/gt/intel_gt_clock_utils.c
> +++ b/drivers/gpu/drm/i915/gt/intel_gt_clock_utils.c
> @@ -148,7 +148,7 @@ static u32 gen4_read_clock_frequency(struct intel_uncore *uncore)
>  	 *
>  	 * Testing on actual hardware has shown there is no /16.
>  	 */
> -	return DIV_ROUND_CLOSEST(i9xx_fsb_freq(uncore->i915), 4) * 1000;
> +	return DIV_ROUND_CLOSEST(intel_fsb_freq(uncore->i915), 4) * 1000;
>  }
>  
>  static u32 read_clock_frequency(struct intel_uncore *uncore)
> diff --git a/drivers/gpu/drm/i915/soc/intel_dram.c b/drivers/gpu/drm/i915/soc/intel_dram.c
> index deb159548a09..6be3618d4885 100644
> --- a/drivers/gpu/drm/i915/soc/intel_dram.c
> +++ b/drivers/gpu/drm/i915/soc/intel_dram.c
> @@ -153,7 +153,7 @@ static void detect_mem_freq(struct drm_i915_private *i915)
>  		drm_dbg(&i915->drm, "DDR speed: %d kHz\n", i915->mem_freq);
>  }
>  
> -unsigned int i9xx_fsb_freq(struct drm_i915_private *i915)
> +static unsigned int i9xx_fsb_freq(struct drm_i915_private *i915)
>  {
>  	u32 fsb;
>  
> @@ -235,13 +235,19 @@ static unsigned int ilk_fsb_freq(struct drm_i915_private *dev_priv)
>  	}
>  }
>  
> -static void detect_fsb_freq(struct drm_i915_private *i915)
> +unsigned int intel_fsb_freq(struct drm_i915_private *i915)
>  {
>  	if (GRAPHICS_VER(i915) == 5)
> -		i915->fsb_freq = ilk_fsb_freq(i915);
> +		return ilk_fsb_freq(i915);
>  	else if (GRAPHICS_VER(i915) == 3 || GRAPHICS_VER(i915) == 4)
> -		i915->fsb_freq = i9xx_fsb_freq(i915);
> +		return i9xx_fsb_freq(i915);
> +	else
> +		return 0;
> +}
>  
> +static void detect_fsb_freq(struct drm_i915_private *i915)
> +{
> +	i915->fsb_freq = intel_fsb_freq(i915);
>  	if (i915->fsb_freq)
>  		drm_dbg(&i915->drm, "FSB frequency: %d kHz\n", i915->fsb_freq);
>  }
> diff --git a/drivers/gpu/drm/i915/soc/intel_dram.h b/drivers/gpu/drm/i915/soc/intel_dram.h
> index 2a696e03aad4..09a7a581d949 100644
> --- a/drivers/gpu/drm/i915/soc/intel_dram.h
> +++ b/drivers/gpu/drm/i915/soc/intel_dram.h
> @@ -33,7 +33,7 @@ struct dram_info {
>  
>  void intel_dram_edram_detect(struct drm_i915_private *i915);
>  int intel_dram_detect(struct drm_i915_private *i915);
> -unsigned int i9xx_fsb_freq(struct drm_i915_private *i915);
> +unsigned int intel_fsb_freq(struct drm_i915_private *i915);
>  const struct dram_info *intel_dram_info(struct drm_device *drm);
>  
>  #endif /* __INTEL_DRAM_H__ */
> -- 
> 2.39.5
> 

Re: [PATCH 2/5] drm/i915/dram: add intel_mem_freq()

[Rodrigo Vivi]

On Thu, Jul 31, 2025 at 05:21:22PM +0300, Jani Nikula wrote:
> Add a more generic intel_mem_freq() function instead of platform
> specific ones. Expose it for future use outside of intel_dram.c.
> 

Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>

> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> ---
>  drivers/gpu/drm/i915/soc/intel_dram.c | 17 ++++++++++++-----
>  drivers/gpu/drm/i915/soc/intel_dram.h |  1 +
>  2 files changed, 13 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/soc/intel_dram.c b/drivers/gpu/drm/i915/soc/intel_dram.c
> index 6be3618d4885..193e7f71a356 100644
> --- a/drivers/gpu/drm/i915/soc/intel_dram.c
> +++ b/drivers/gpu/drm/i915/soc/intel_dram.c
> @@ -135,16 +135,23 @@ static unsigned int vlv_mem_freq(struct drm_i915_private *i915)
>  	return 0;
>  }
>  
> -static void detect_mem_freq(struct drm_i915_private *i915)
> +unsigned int intel_mem_freq(struct drm_i915_private *i915)
>  {
>  	if (IS_PINEVIEW(i915))
> -		i915->mem_freq = pnv_mem_freq(i915);
> +		return pnv_mem_freq(i915);
>  	else if (GRAPHICS_VER(i915) == 5)
> -		i915->mem_freq = ilk_mem_freq(i915);
> +		return ilk_mem_freq(i915);
>  	else if (IS_CHERRYVIEW(i915))
> -		i915->mem_freq = chv_mem_freq(i915);
> +		return chv_mem_freq(i915);
>  	else if (IS_VALLEYVIEW(i915))
> -		i915->mem_freq = vlv_mem_freq(i915);
> +		return vlv_mem_freq(i915);
> +	else
> +		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);
> diff --git a/drivers/gpu/drm/i915/soc/intel_dram.h b/drivers/gpu/drm/i915/soc/intel_dram.h
> index 09a7a581d949..5ba75e279e84 100644
> --- a/drivers/gpu/drm/i915/soc/intel_dram.h
> +++ b/drivers/gpu/drm/i915/soc/intel_dram.h
> @@ -34,6 +34,7 @@ struct dram_info {
>  void intel_dram_edram_detect(struct drm_i915_private *i915);
>  int intel_dram_detect(struct drm_i915_private *i915);
>  unsigned int intel_fsb_freq(struct drm_i915_private *i915);
> +unsigned int intel_mem_freq(struct drm_i915_private *i915);
>  const struct dram_info *intel_dram_info(struct drm_device *drm);
>  
>  #endif /* __INTEL_DRAM_H__ */
> -- 
> 2.39.5
> 

Re: [PATCH 3/5] drm/i915/rps: use intel_fsb_freq() and intel_mem_freq()

[Rodrigo Vivi]

On Thu, Jul 31, 2025 at 05:21:23PM +0300, Jani Nikula wrote:
> The rps init only happens once, so it's not important to use the cached
> versions, and we can drop the dependency on them.

Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>

> 
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> ---
>  drivers/gpu/drm/i915/gt/intel_rps.c | 11 ++++++++---
>  1 file changed, 8 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/gt/intel_rps.c b/drivers/gpu/drm/i915/gt/intel_rps.c
> index 0b35fdd461d4..006042e0b229 100644
> --- a/drivers/gpu/drm/i915/gt/intel_rps.c
> +++ b/drivers/gpu/drm/i915/gt/intel_rps.c
> @@ -9,6 +9,7 @@
>  
>  #include "display/intel_display.h"
>  #include "display/intel_display_rps.h"
> +#include "soc/intel_dram.h"
>  #include "i915_drv.h"
>  #include "i915_irq.h"
>  #include "i915_reg.h"
> @@ -276,20 +277,24 @@ static void gen5_rps_init(struct intel_rps *rps)
>  {
>  	struct drm_i915_private *i915 = rps_to_i915(rps);
>  	struct intel_uncore *uncore = rps_to_uncore(rps);
> +	unsigned int fsb_freq, mem_freq;
>  	u8 fmax, fmin, fstart;
>  	u32 rgvmodectl;
>  	int c_m, i;
>  
> -	if (i915->fsb_freq <= 3200000)
> +	fsb_freq = intel_fsb_freq(i915);
> +	mem_freq = intel_mem_freq(i915);
> +
> +	if (fsb_freq <= 3200000)
>  		c_m = 0;
> -	else if (i915->fsb_freq <= 4800000)
> +	else if (fsb_freq <= 4800000)
>  		c_m = 1;
>  	else
>  		c_m = 2;
>  
>  	for (i = 0; i < ARRAY_SIZE(cparams); i++) {
>  		if (cparams[i].i == c_m &&
> -		    cparams[i].t == DIV_ROUND_CLOSEST(i915->mem_freq, 1000)) {
> +		    cparams[i].t == DIV_ROUND_CLOSEST(mem_freq, 1000)) {
>  			rps->ips.m = cparams[i].m;
>  			rps->ips.c = cparams[i].c;
>  			break;
> -- 
> 2.39.5
> 

Re: [PATCH 4/5] drm/i915/dram: bypass fsb/mem freq detection on dg2 and no display

[Rodrigo Vivi]

On Thu, Jul 31, 2025 at 05:21:24PM +0300, Jani Nikula wrote:
> Non-display now calls the intel_fsb_freq() and intel_mem_freq()
> functions, so we don't have to have the frequencies initialized for dg2
> or non-display cases.
> 
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> ---
>  drivers/gpu/drm/i915/soc/intel_dram.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/soc/intel_dram.c b/drivers/gpu/drm/i915/soc/intel_dram.c
> index 193e7f71a356..d896fb67270f 100644
> --- a/drivers/gpu/drm/i915/soc/intel_dram.c
> +++ b/drivers/gpu/drm/i915/soc/intel_dram.c
> @@ -725,10 +725,13 @@ int intel_dram_detect(struct drm_i915_private *i915)
>  	struct dram_info *dram_info;
>  	int ret;
>  
> +	if (IS_DG2(i915) || !HAS_DISPLAY(i915))
> +		return 0;
> +
>  	detect_fsb_freq(i915);
>  	detect_mem_freq(i915);

but they will only be set to zero no? do we really need to avoid it?
if so, perhaps make this change earlier?

Also I wonder what's special in DG2, but not in BMG...

>  
> -	if (GRAPHICS_VER(i915) < 9 || IS_DG2(i915) || !HAS_DISPLAY(i915))
> +	if (GRAPHICS_VER(i915) < 9)

what about the old gen remaining here? at least deserves a comment on why
it needs the upper calls, but not the call bellow? or should the upper
calls be in another function/flow?

>  		return 0;
>  
>  	dram_info = drmm_kzalloc(&i915->drm, sizeof(*dram_info), GFP_KERNEL);
> -- 
> 2.39.5
> 

Re: [PATCH 5/5] drm/i915/dram: move fsb_freq and mem_freq to dram info

[Rodrigo Vivi]

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.nikula@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...

>  	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...

> -
>  	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
> 

Re: [PATCH 4/5] drm/i915/dram: bypass fsb/mem freq detection on dg2 and no display

[Jani Nikula]

On Tue, 05 Aug 2025, Rodrigo Vivi <rodrigo.vivi@intel.com> wrote:
> On Thu, Jul 31, 2025 at 05:21:24PM +0300, Jani Nikula wrote:
>> Non-display now calls the intel_fsb_freq() and intel_mem_freq()
>> functions, so we don't have to have the frequencies initialized for dg2
>> or non-display cases.

Is this enough amendmend for the commit message:

"""
This is in preparation for unifying the pre-gen9 handling in dram info.

DG2 remains a special case as described in commit 5eb6bf0b44e7
("drm/i915/dg2: Don't read DRAM info").
"""

Or do you want more?

BR,
Jani.

>> 
>> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
>> ---
>>  drivers/gpu/drm/i915/soc/intel_dram.c | 5 ++++-
>>  1 file changed, 4 insertions(+), 1 deletion(-)
>> 
>> diff --git a/drivers/gpu/drm/i915/soc/intel_dram.c b/drivers/gpu/drm/i915/soc/intel_dram.c
>> index 193e7f71a356..d896fb67270f 100644
>> --- a/drivers/gpu/drm/i915/soc/intel_dram.c
>> +++ b/drivers/gpu/drm/i915/soc/intel_dram.c
>> @@ -725,10 +725,13 @@ int intel_dram_detect(struct drm_i915_private *i915)
>>  	struct dram_info *dram_info;
>>  	int ret;
>>  
>> +	if (IS_DG2(i915) || !HAS_DISPLAY(i915))
>> +		return 0;
>> +
>>  	detect_fsb_freq(i915);
>>  	detect_mem_freq(i915);
>
> but they will only be set to zero no? do we really need to avoid it?
> if so, perhaps make this change earlier?
>
> Also I wonder what's special in DG2, but not in BMG...
>
>>  
>> -	if (GRAPHICS_VER(i915) < 9 || IS_DG2(i915) || !HAS_DISPLAY(i915))
>> +	if (GRAPHICS_VER(i915) < 9)
>
> what about the old gen remaining here? at least deserves a comment on why
> it needs the upper calls, but not the call bellow? or should the upper
> calls be in another function/flow?
>
>>  		return 0;
>>  
>>  	dram_info = drmm_kzalloc(&i915->drm, sizeof(*dram_info), GFP_KERNEL);
>> -- 
>> 2.39.5
>> 

-- 
Jani Nikula, Intel

Re: [PATCH 5/5] drm/i915/dram: move fsb_freq and mem_freq to dram info

[Jani Nikula]

On Tue, 05 Aug 2025, Rodrigo Vivi <rodrigo.vivi@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.nikula@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

Re: [PATCH 4/5] drm/i915/dram: bypass fsb/mem freq detection on dg2 and no display

[Rodrigo Vivi]

On Wed, Aug 06, 2025 at 04:52:06PM +0300, Jani Nikula wrote:
> On Tue, 05 Aug 2025, Rodrigo Vivi <rodrigo.vivi@intel.com> wrote:
> > On Thu, Jul 31, 2025 at 05:21:24PM +0300, Jani Nikula wrote:
> >> Non-display now calls the intel_fsb_freq() and intel_mem_freq()
> >> functions, so we don't have to have the frequencies initialized for dg2
> >> or non-display cases.
> 
> Is this enough amendmend for the commit message:
> 
> """
> This is in preparation for unifying the pre-gen9 handling in dram info.
> 
> DG2 remains a special case as described in commit 5eb6bf0b44e7
> ("drm/i915/dg2: Don't read DRAM info").
> """
> 
> Or do you want more?

this is great, with that

Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>

> 
> BR,
> Jani.
> 
> >> 
> >> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> >> ---
> >>  drivers/gpu/drm/i915/soc/intel_dram.c | 5 ++++-
> >>  1 file changed, 4 insertions(+), 1 deletion(-)
> >> 
> >> diff --git a/drivers/gpu/drm/i915/soc/intel_dram.c b/drivers/gpu/drm/i915/soc/intel_dram.c
> >> index 193e7f71a356..d896fb67270f 100644
> >> --- a/drivers/gpu/drm/i915/soc/intel_dram.c
> >> +++ b/drivers/gpu/drm/i915/soc/intel_dram.c
> >> @@ -725,10 +725,13 @@ int intel_dram_detect(struct drm_i915_private *i915)
> >>  	struct dram_info *dram_info;
> >>  	int ret;
> >>  
> >> +	if (IS_DG2(i915) || !HAS_DISPLAY(i915))
> >> +		return 0;
> >> +
> >>  	detect_fsb_freq(i915);
> >>  	detect_mem_freq(i915);
> >
> > but they will only be set to zero no? do we really need to avoid it?
> > if so, perhaps make this change earlier?
> >
> > Also I wonder what's special in DG2, but not in BMG...
> >
> >>  
> >> -	if (GRAPHICS_VER(i915) < 9 || IS_DG2(i915) || !HAS_DISPLAY(i915))
> >> +	if (GRAPHICS_VER(i915) < 9)
> >
> > what about the old gen remaining here? at least deserves a comment on why
> > it needs the upper calls, but not the call bellow? or should the upper
> > calls be in another function/flow?
> >
> >>  		return 0;
> >>  
> >>  	dram_info = drmm_kzalloc(&i915->drm, sizeof(*dram_info), GFP_KERNEL);
> >> -- 
> >> 2.39.5
> >> 
> 
> -- 
> Jani Nikula, Intel

Re: [PATCH 5/5] drm/i915/dram: move fsb_freq and mem_freq to dram info

[Rodrigo Vivi]

On Wed, Aug 06, 2025 at 04:57:53PM +0300, Jani Nikula wrote:
> On Tue, 05 Aug 2025, Rodrigo Vivi <rodrigo.vivi@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.nikula@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.

oh, it makes sense now... what I was missing was the part where
that was getting set in the pnv block.

> 
> 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.

no need for that indeed.

Thanks for the explanation and patience

Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>

> 
> >
> >>  	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