[PATCH v3 0/5] drm/i915/bw: Split bandwidth params into platform- and display-IP-specific structs

[PATCH v3 0/5] drm/i915/bw: Split bandwidth params into platform- and display-IP-specific structs

[Gustavo Sousa]

Some of the parameters of used in display bandwidth calculations are
tied to the platform and are orthogonal to the display IP.  After talking
with the hardware team, we now have the information (and Bspec has been
updated) that the members deprogbwlimit and derating of struct
intel_sa_info are such platform-specific ones.

With that, we are now able to make the driver code more aligned with the
hardware by splitting structs intel_sa_info into two different structs:
one that is platform-specific and another that is display-IP-specific.

That change also allows us to simplify how we select the parameters for
the calculation.

Signed-off-by: Gustavo Sousa <gustavo.sousa@intel.com>
---
Changes in v3:
- Incorporated review feedback; see each individual patch for details.
- Link to v2: https://patch.msgid.link/20260511-separate-platform-from-diplay-ip-specific-bw-params-v2-0-e762cb8662da@intel.com

Changes in v2:
- Incorporated review feedback; see each individual patch for details.
- Link to v1: https://patch.msgid.link/20260408-separate-platform-from-diplay-ip-specific-bw-params-v1-0-23c53afa7db0@intel.com

---
Gustavo Sousa (5):
      drm/i915/bw: Don't call intel_dram_info() too early
      drm/i915/bw: Extract platform-specific parameters
      drm/i915/bw: Deduplicate intel_sa_info instances
      drm/i915/bw: Rename struct intel_sa_info to intel_display_bw_params
      drm/i915/bw: Extract get_display_bw_params()

 drivers/gpu/drm/i915/display/intel_bw.c | 218 +++++++++++++++++++++-----------
 1 file changed, 141 insertions(+), 77 deletions(-)
---
base-commit: dee34cfbffbfe2196a9332f966b006cd2e54e976
change-id: 20260408-separate-platform-from-diplay-ip-specific-bw-params-65bfba0603be

Best regards,
--  
Gustavo Sousa <gustavo.sousa@intel.com>

[PATCH v3 1/5] drm/i915/bw: Don't call intel_dram_info() too early

[Gustavo Sousa]

If we end-up bailing early from intel_bw_init_hw() due to
!HAS_DISPLAY(display), the call to intel_dram_info() to initialize
dram_info will be meaningless.  Move the call to be done after that
check.

Cc: Jani Nikula <jani.nikula@intel.com>
Signed-off-by: Gustavo Sousa <gustavo.sousa@intel.com>
---
 drivers/gpu/drm/i915/display/intel_bw.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/display/intel_bw.c b/drivers/gpu/drm/i915/display/intel_bw.c
index 9c3a9bbb49f6..7eef693b51ad 100644
--- a/drivers/gpu/drm/i915/display/intel_bw.c
+++ b/drivers/gpu/drm/i915/display/intel_bw.c
@@ -791,11 +791,13 @@ static unsigned int icl_qgv_bw(struct intel_display *display,
 
 void intel_bw_init_hw(struct intel_display *display)
 {
-	const struct dram_info *dram_info = intel_dram_info(display);
+	const struct dram_info *dram_info;
 
 	if (!HAS_DISPLAY(display))
 		return;
 
+	dram_info = intel_dram_info(display);
+
 	/*
 	 * Starting with Xe3p_LPD, the hardware tells us whether memory has ECC
 	 * enabled that would impact display bandwidth.  However, so far there

-- 
2.53.0

[PATCH v3 2/5] drm/i915/bw: Extract platform-specific parameters

[Gustavo Sousa]

We got confirmation from the hardware team that the bandwidth parameters
deprogbwlimit and derating are platform-specific and not tied to the
display IP.  As such, let's make sure that we use platform checks for
those.

The rest of the members of struct intel_sa_info are tied to the display
IP and we will deal with them as a follow-up.

v2:
  - Use good old if-ladder instead of weird-looking pattern "assign ret,
    check platform, then return ret". (Jani, Matt)
  - Have a single call site for get_platform_bw_params() and pass the
    result as parameter to the *_get_bw_info() functions. (Jani)
  - Avoid using "plat" as abbreviation for "platform". (Jani)
  - s/_plat_bw_params/_bw_params/, since all of the instances are
    prefixed with platform names. (Jani)
  - s/struct intel_platform_bw_params/struct intel_soc_bw_params/.
    (Matt)
  - Do not return a default value; prefer to return NULL and
    intentionally cause a NULL pointer dereference if a platform is
    missing. (Gustavo)

v3:
  - Call get_soc_bw_params() only after the check on
    HAS_DISPLAY(display). (Jani)
  - Combine if-ladder branches for adl_s_bw_params into a single one.
    (Matt)
  - Flatten if-ladder by checking for WCL before PTL (as opposed to
    checking for WCL inside the brace for PTL). (Matt)
  - Bail out of intel_bw_init_hw() if display version is below 11.
    (Gustavo)

Cc: Jani Nikula <jani.nikula@intel.com>
Cc: Matt Roper <matthew.d.roper@intel.com>
Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
Signed-off-by: Gustavo Sousa <gustavo.sousa@intel.com>
---
 drivers/gpu/drm/i915/display/intel_bw.c | 162 ++++++++++++++++++++++----------
 1 file changed, 114 insertions(+), 48 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_bw.c b/drivers/gpu/drm/i915/display/intel_bw.c
index 7eef693b51ad..351ecf741b54 100644
--- a/drivers/gpu/drm/i915/display/intel_bw.c
+++ b/drivers/gpu/drm/i915/display/intel_bw.c
@@ -372,81 +372,144 @@ static int icl_sagv_max_dclk(const struct intel_qgv_info *qi)
 	return dclk;
 }
 
+struct intel_soc_bw_params {
+	u8 deprogbwlimit;
+	u8 derating;
+};
+
+static const struct intel_soc_bw_params icl_bw_params = {
+	.deprogbwlimit = 25,
+	.derating = 10,
+};
+
+static const struct intel_soc_bw_params tgl_bw_params = {
+	.deprogbwlimit = 34,
+	.derating = 10,
+};
+
+static const struct intel_soc_bw_params rkl_bw_params = {
+	.deprogbwlimit = 20,
+	.derating = 10,
+};
+
+static const struct intel_soc_bw_params adl_s_bw_params = {
+	.deprogbwlimit = 38,
+	.derating = 10,
+};
+
+static const struct intel_soc_bw_params adl_p_bw_params = {
+	.deprogbwlimit = 38,
+	.derating = 20,
+};
+
+static const struct intel_soc_bw_params bmg_bw_params = {
+	.deprogbwlimit = 53,
+	.derating = 30,
+};
+
+static const struct intel_soc_bw_params bmg_ecc_bw_params = {
+	.deprogbwlimit = 53,
+	.derating = 45,
+};
+
+static const struct intel_soc_bw_params ptl_bw_params = {
+	.deprogbwlimit = 65,
+	.derating = 10,
+};
+
+static const struct intel_soc_bw_params wcl_bw_params = {
+	.deprogbwlimit = 22,
+	.derating = 10,
+};
+
+static const struct intel_soc_bw_params *get_soc_bw_params(struct intel_display *display)
+{
+	if (display->platform.dgfx) {
+		if (display->platform.dg1) {
+			return &tgl_bw_params;
+		} else if (display->platform.battlemage) {
+			const struct dram_info *dram_info = intel_dram_info(display);
+
+			if (dram_info->type == INTEL_DRAM_GDDR_ECC)
+				return &bmg_ecc_bw_params;
+			else
+				return &bmg_bw_params;
+		}
+	} else {
+		if (display->platform.icelake ||
+		    display->platform.jasperlake ||
+		    display->platform.elkhartlake)
+			return &icl_bw_params;
+		else if (display->platform.tigerlake)
+			return &tgl_bw_params;
+		else if (display->platform.rocketlake)
+			return &rkl_bw_params;
+		else if (display->platform.alderlake_s ||
+			 display->platform.meteorlake ||
+			 display->platform.lunarlake)
+			return &adl_s_bw_params;
+		else if (display->platform.alderlake_p)
+			return &adl_p_bw_params;
+		else if (display->platform.pantherlake_wildcatlake)
+			return &wcl_bw_params;
+		else if (display->platform.pantherlake ||
+			 display->platform.novalake)
+			return &ptl_bw_params;
+	}
+
+	drm_WARN(display->drm, 1, "Platform-specific bandwidth parameters not found!\n");
+
+	return NULL;
+}
+
 struct intel_sa_info {
 	u16 displayrtids;
-	u8 deburst, deprogbwlimit, derating;
+	u8 deburst;
 };
 
 static const struct intel_sa_info icl_sa_info = {
 	.deburst = 8,
-	.deprogbwlimit = 25, /* GB/s */
 	.displayrtids = 128,
-	.derating = 10,
 };
 
 static const struct intel_sa_info tgl_sa_info = {
 	.deburst = 16,
-	.deprogbwlimit = 34, /* GB/s */
 	.displayrtids = 256,
-	.derating = 10,
 };
 
 static const struct intel_sa_info rkl_sa_info = {
 	.deburst = 8,
-	.deprogbwlimit = 20, /* GB/s */
 	.displayrtids = 128,
-	.derating = 10,
 };
 
 static const struct intel_sa_info adls_sa_info = {
 	.deburst = 16,
-	.deprogbwlimit = 38, /* GB/s */
 	.displayrtids = 256,
-	.derating = 10,
 };
 
 static const struct intel_sa_info adlp_sa_info = {
 	.deburst = 16,
-	.deprogbwlimit = 38, /* GB/s */
 	.displayrtids = 256,
-	.derating = 20,
 };
 
 static const struct intel_sa_info mtl_sa_info = {
 	.deburst = 32,
-	.deprogbwlimit = 38, /* GB/s */
 	.displayrtids = 256,
-	.derating = 10,
-};
-
-static const struct intel_sa_info xe2_hpd_sa_info = {
-	.derating = 30,
-	.deprogbwlimit = 53,
-	/* Other values not used by simplified algorithm */
-};
-
-static const struct intel_sa_info xe2_hpd_ecc_sa_info = {
-	.derating = 45,
-	.deprogbwlimit = 53,
-	/* Other values not used by simplified algorithm */
 };
 
 static const struct intel_sa_info xe3lpd_sa_info = {
 	.deburst = 32,
-	.deprogbwlimit = 65, /* GB/s */
 	.displayrtids = 256,
-	.derating = 10,
 };
 
 static const struct intel_sa_info xe3lpd_3002_sa_info = {
 	.deburst = 32,
-	.deprogbwlimit = 22, /* GB/s */
 	.displayrtids = 256,
-	.derating = 10,
 };
 
 static int icl_get_bw_info(struct intel_display *display,
 			   const struct dram_info *dram_info,
+			   const struct intel_soc_bw_params *soc_bw_params,
 			   const struct intel_sa_info *sa)
 {
 	struct intel_qgv_info qi = {};
@@ -466,7 +529,7 @@ static int icl_get_bw_info(struct intel_display *display,
 	}
 
 	dclk_max = icl_sagv_max_dclk(&qi);
-	maxdebw = min(sa->deprogbwlimit * 1000, dclk_max * 16 * 6 / 10);
+	maxdebw = min(soc_bw_params->deprogbwlimit * 1000, dclk_max * 16 * 6 / 10);
 	ipqdepth = min(ipqdepthpch, sa->displayrtids / num_channels);
 	qi.deinterleave = DIV_ROUND_UP(num_channels, is_y_tile ? 4 : 2);
 
@@ -496,7 +559,7 @@ static int icl_get_bw_info(struct intel_display *display,
 			bw = DIV_ROUND_UP(sp->dclk * clpchgroup * 32 * num_channels, ct);
 
 			bi->deratedbw[j] = min(maxdebw,
-					       bw * (100 - sa->derating) / 100);
+					       bw * (100 - soc_bw_params->derating) / 100);
 
 			drm_dbg_kms(display->drm,
 				    "BW%d / QGV %d: num_planes=%d deratedbw=%u\n",
@@ -518,6 +581,7 @@ static int icl_get_bw_info(struct intel_display *display,
 
 static int tgl_get_bw_info(struct intel_display *display,
 			   const struct dram_info *dram_info,
+			   const struct intel_soc_bw_params *soc_bw_params,
 			   const struct intel_sa_info *sa)
 {
 	struct intel_qgv_info qi = {};
@@ -554,7 +618,7 @@ static int tgl_get_bw_info(struct intel_display *display,
 	dclk_max = icl_sagv_max_dclk(&qi);
 
 	peakbw = num_channels * DIV_ROUND_UP(qi.channel_width, 8) * dclk_max;
-	maxdebw = min(sa->deprogbwlimit * 1000, peakbw * DEPROGBWPCLIMIT / 100);
+	maxdebw = min(soc_bw_params->deprogbwlimit * 1000, peakbw * DEPROGBWPCLIMIT / 100);
 
 	ipqdepth = min(ipqdepthpch, sa->displayrtids / num_channels);
 	/*
@@ -599,7 +663,7 @@ static int tgl_get_bw_info(struct intel_display *display,
 			bw = DIV_ROUND_UP(sp->dclk * clpchgroup * 32 * num_channels, ct);
 
 			bi->deratedbw[j] = min(maxdebw,
-					       bw * (100 - sa->derating) / 100);
+					       bw * (100 - soc_bw_params->derating) / 100);
 			bi->peakbw[j] = DIV_ROUND_CLOSEST(sp->dclk *
 							  num_channels *
 							  qi.channel_width, 8);
@@ -661,7 +725,7 @@ static void dg2_get_bw_info(struct intel_display *display)
 
 static int xe2_hpd_get_bw_info(struct intel_display *display,
 			       const struct dram_info *dram_info,
-			       const struct intel_sa_info *sa)
+			       const struct intel_soc_bw_params *soc_bw_params)
 {
 	struct intel_qgv_info qi = {};
 	int num_channels = dram_info->num_channels;
@@ -676,14 +740,14 @@ static int xe2_hpd_get_bw_info(struct intel_display *display,
 	}
 
 	peakbw = num_channels * qi.channel_width / 8 * icl_sagv_max_dclk(&qi);
-	maxdebw = min(sa->deprogbwlimit * 1000, peakbw * DEPROGBWPCLIMIT / 10);
+	maxdebw = min(soc_bw_params->deprogbwlimit * 1000, peakbw * DEPROGBWPCLIMIT / 10);
 
 	for (i = 0; i < qi.num_points; i++) {
 		const struct intel_qgv_point *point = &qi.points[i];
 		int bw = num_channels * (qi.channel_width / 8) * point->dclk;
 
 		display->bw.max[0].deratedbw[i] =
-			min(maxdebw, (100 - sa->derating) * bw / 100);
+			min(maxdebw, (100 - soc_bw_params->derating) * bw / 100);
 		display->bw.max[0].peakbw[i] = bw;
 
 		drm_dbg_kms(display->drm, "QGV %d: deratedbw=%u peakbw: %u\n",
@@ -792,11 +856,16 @@ static unsigned int icl_qgv_bw(struct intel_display *display,
 void intel_bw_init_hw(struct intel_display *display)
 {
 	const struct dram_info *dram_info;
+	const struct intel_soc_bw_params *soc_bw_params;
 
 	if (!HAS_DISPLAY(display))
 		return;
 
+	if (DISPLAY_VER(display) < 11)
+		return;
+
 	dram_info = intel_dram_info(display);
+	soc_bw_params = get_soc_bw_params(display);
 
 	/*
 	 * Starting with Xe3p_LPD, the hardware tells us whether memory has ECC
@@ -809,28 +878,25 @@ void intel_bw_init_hw(struct intel_display *display)
 
 	if (DISPLAY_VER(display) >= 30) {
 		if (DISPLAY_VERx100(display) == 3002)
-			tgl_get_bw_info(display, dram_info, &xe3lpd_3002_sa_info);
+			tgl_get_bw_info(display, dram_info, soc_bw_params, &xe3lpd_3002_sa_info);
 		else
-			tgl_get_bw_info(display, dram_info, &xe3lpd_sa_info);
+			tgl_get_bw_info(display, dram_info, soc_bw_params, &xe3lpd_sa_info);
 	} else if (DISPLAY_VERx100(display) >= 1401 && display->platform.dgfx) {
-		if (dram_info->type == INTEL_DRAM_GDDR_ECC)
-			xe2_hpd_get_bw_info(display, dram_info, &xe2_hpd_ecc_sa_info);
-		else
-			xe2_hpd_get_bw_info(display, dram_info, &xe2_hpd_sa_info);
+		xe2_hpd_get_bw_info(display, dram_info, soc_bw_params);
 	} else if (DISPLAY_VER(display) >= 14) {
-		tgl_get_bw_info(display, dram_info, &mtl_sa_info);
+		tgl_get_bw_info(display, dram_info, soc_bw_params, &mtl_sa_info);
 	} else if (display->platform.dg2) {
 		dg2_get_bw_info(display);
 	} else if (display->platform.alderlake_p) {
-		tgl_get_bw_info(display, dram_info, &adlp_sa_info);
+		tgl_get_bw_info(display, dram_info, soc_bw_params, &adlp_sa_info);
 	} else if (display->platform.alderlake_s) {
-		tgl_get_bw_info(display, dram_info, &adls_sa_info);
+		tgl_get_bw_info(display, dram_info, soc_bw_params, &adls_sa_info);
 	} else if (display->platform.rocketlake) {
-		tgl_get_bw_info(display, dram_info, &rkl_sa_info);
+		tgl_get_bw_info(display, dram_info, soc_bw_params, &rkl_sa_info);
 	} else if (DISPLAY_VER(display) == 12) {
-		tgl_get_bw_info(display, dram_info, &tgl_sa_info);
+		tgl_get_bw_info(display, dram_info, soc_bw_params, &tgl_sa_info);
 	} else if (DISPLAY_VER(display) == 11) {
-		icl_get_bw_info(display, dram_info, &icl_sa_info);
+		icl_get_bw_info(display, dram_info, soc_bw_params, &icl_sa_info);
 	}
 }
 

-- 
2.53.0

[PATCH v3 3/5] drm/i915/bw: Deduplicate intel_sa_info instances

[Gustavo Sousa]

Now that intel_sa_info contains bandwidth parameters specific to the
display IP, we can drop many duplicates and reuse from previous
releases.

Let's do that and also simplify intel_bw_init_hw() while at it.

v2:
  - Drop rkl_sa_info and reuse icl_sa_info. (Matt)
  - Add comment explaining RKL's display's peculiarity on using ICL's
    parameters. (Matt)
  - Don't rename xelpdp_sa_info to mtl_sa_info.  Renaming of instances
    to use IP names will be done in upcoming changes.

Reviewed-by: Matt Roper <matthew.d.roper@intel.com>
Signed-off-by: Gustavo Sousa <gustavo.sousa@intel.com>
---
 drivers/gpu/drm/i915/display/intel_bw.c | 51 ++++++++-------------------------
 1 file changed, 12 insertions(+), 39 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_bw.c b/drivers/gpu/drm/i915/display/intel_bw.c
index 351ecf741b54..a956b09028f3 100644
--- a/drivers/gpu/drm/i915/display/intel_bw.c
+++ b/drivers/gpu/drm/i915/display/intel_bw.c
@@ -477,36 +477,11 @@ static const struct intel_sa_info tgl_sa_info = {
 	.displayrtids = 256,
 };
 
-static const struct intel_sa_info rkl_sa_info = {
-	.deburst = 8,
-	.displayrtids = 128,
-};
-
-static const struct intel_sa_info adls_sa_info = {
-	.deburst = 16,
-	.displayrtids = 256,
-};
-
-static const struct intel_sa_info adlp_sa_info = {
-	.deburst = 16,
-	.displayrtids = 256,
-};
-
 static const struct intel_sa_info mtl_sa_info = {
 	.deburst = 32,
 	.displayrtids = 256,
 };
 
-static const struct intel_sa_info xe3lpd_sa_info = {
-	.deburst = 32,
-	.displayrtids = 256,
-};
-
-static const struct intel_sa_info xe3lpd_3002_sa_info = {
-	.deburst = 32,
-	.displayrtids = 256,
-};
-
 static int icl_get_bw_info(struct intel_display *display,
 			   const struct dram_info *dram_info,
 			   const struct intel_soc_bw_params *soc_bw_params,
@@ -876,25 +851,23 @@ void intel_bw_init_hw(struct intel_display *display)
 	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, soc_bw_params, &xe3lpd_3002_sa_info);
-		else
-			tgl_get_bw_info(display, dram_info, soc_bw_params, &xe3lpd_sa_info);
-	} else if (DISPLAY_VERx100(display) >= 1401 && display->platform.dgfx) {
+	if (DISPLAY_VERx100(display) >= 1401 && display->platform.dgfx) {
 		xe2_hpd_get_bw_info(display, dram_info, soc_bw_params);
 	} else if (DISPLAY_VER(display) >= 14) {
 		tgl_get_bw_info(display, dram_info, soc_bw_params, &mtl_sa_info);
 	} else if (display->platform.dg2) {
 		dg2_get_bw_info(display);
-	} else if (display->platform.alderlake_p) {
-		tgl_get_bw_info(display, dram_info, soc_bw_params, &adlp_sa_info);
-	} else if (display->platform.alderlake_s) {
-		tgl_get_bw_info(display, dram_info, soc_bw_params, &adls_sa_info);
-	} else if (display->platform.rocketlake) {
-		tgl_get_bw_info(display, dram_info, soc_bw_params, &rkl_sa_info);
-	} else if (DISPLAY_VER(display) == 12) {
-		tgl_get_bw_info(display, dram_info, soc_bw_params, &tgl_sa_info);
+	} else if (DISPLAY_VER(display) >= 12) {
+		/*
+		 * RKL's SoC was based on ICL and the display, even though being
+		 * gen12, had changes to the memory interface to match gen11's,
+		 * consequently inheriting gen11's display-specific bandwidth
+		 * parameters.
+		 */
+		if (display->platform.rocketlake)
+			tgl_get_bw_info(display, dram_info, soc_bw_params, &icl_sa_info);
+		else
+			tgl_get_bw_info(display, dram_info, soc_bw_params, &tgl_sa_info);
 	} else if (DISPLAY_VER(display) == 11) {
 		icl_get_bw_info(display, dram_info, soc_bw_params, &icl_sa_info);
 	}

-- 
2.53.0

[PATCH v3 4/5] drm/i915/bw: Rename struct intel_sa_info to intel_display_bw_params

[Gustavo Sousa]

To align with struct intel_platform_bw_params, rename struct
intel_sa_info to intel_display_bw_params.  Also add comments to contrast
their purposes.

v2:
  - Use gen11 and gen12 as prefixes for ICL's and TGL's display-specific
    parameters variables. (Matt)
  - Prefer to use "display" instead of "disp" in variable names. (Jani)
  - Drop the redundant "disp" from the variable names.

Cc: Jani Nikula <jani.nikula@intel.com>
Reviewed-by: Matt Roper <matthew.d.roper@intel.com>
Signed-off-by: Gustavo Sousa <gustavo.sousa@intel.com>
---
 drivers/gpu/drm/i915/display/intel_bw.c | 36 ++++++++++++++++++++-------------
 1 file changed, 22 insertions(+), 14 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_bw.c b/drivers/gpu/drm/i915/display/intel_bw.c
index a956b09028f3..5821397dc27f 100644
--- a/drivers/gpu/drm/i915/display/intel_bw.c
+++ b/drivers/gpu/drm/i915/display/intel_bw.c
@@ -372,6 +372,10 @@ static int icl_sagv_max_dclk(const struct intel_qgv_info *qi)
 	return dclk;
 }
 
+/*
+ * Bandwidth parameters that are tied to the SoC (as opposed to struct
+ * intel_display_bw_params).
+ */
 struct intel_soc_bw_params {
 	u8 deprogbwlimit;
 	u8 derating;
@@ -462,22 +466,26 @@ static const struct intel_soc_bw_params *get_soc_bw_params(struct intel_display
 	return NULL;
 }
 
-struct intel_sa_info {
+/*
+ * Bandwidth parameters that are tied to the display IP (as opposed to struct
+ * intel_soc_bw_params).
+ */
+struct intel_display_bw_params {
 	u16 displayrtids;
 	u8 deburst;
 };
 
-static const struct intel_sa_info icl_sa_info = {
+static const struct intel_display_bw_params gen11_bw_params = {
 	.deburst = 8,
 	.displayrtids = 128,
 };
 
-static const struct intel_sa_info tgl_sa_info = {
+static const struct intel_display_bw_params gen12_bw_params = {
 	.deburst = 16,
 	.displayrtids = 256,
 };
 
-static const struct intel_sa_info mtl_sa_info = {
+static const struct intel_display_bw_params xelpdp_bw_params = {
 	.deburst = 32,
 	.displayrtids = 256,
 };
@@ -485,7 +493,7 @@ static const struct intel_sa_info mtl_sa_info = {
 static int icl_get_bw_info(struct intel_display *display,
 			   const struct dram_info *dram_info,
 			   const struct intel_soc_bw_params *soc_bw_params,
-			   const struct intel_sa_info *sa)
+			   const struct intel_display_bw_params *display_bw_params)
 {
 	struct intel_qgv_info qi = {};
 	bool is_y_tile = true; /* assume y tile may be used */
@@ -505,7 +513,7 @@ static int icl_get_bw_info(struct intel_display *display,
 
 	dclk_max = icl_sagv_max_dclk(&qi);
 	maxdebw = min(soc_bw_params->deprogbwlimit * 1000, dclk_max * 16 * 6 / 10);
-	ipqdepth = min(ipqdepthpch, sa->displayrtids / num_channels);
+	ipqdepth = min(ipqdepthpch, display_bw_params->displayrtids / num_channels);
 	qi.deinterleave = DIV_ROUND_UP(num_channels, is_y_tile ? 4 : 2);
 
 	for (i = 0; i < num_groups; i++) {
@@ -513,7 +521,7 @@ static int icl_get_bw_info(struct intel_display *display,
 		int clpchgroup;
 		int j;
 
-		clpchgroup = (sa->deburst * qi.deinterleave / num_channels) << i;
+		clpchgroup = (display_bw_params->deburst * qi.deinterleave / num_channels) << i;
 		bi->num_planes = (ipqdepth - clpchgroup) / clpchgroup + 1;
 
 		bi->num_qgv_points = qi.num_points;
@@ -557,7 +565,7 @@ static int icl_get_bw_info(struct intel_display *display,
 static int tgl_get_bw_info(struct intel_display *display,
 			   const struct dram_info *dram_info,
 			   const struct intel_soc_bw_params *soc_bw_params,
-			   const struct intel_sa_info *sa)
+			   const struct intel_display_bw_params *display_bw_params)
 {
 	struct intel_qgv_info qi = {};
 	bool is_y_tile = true; /* assume y tile may be used */
@@ -595,7 +603,7 @@ static int tgl_get_bw_info(struct intel_display *display,
 	peakbw = num_channels * DIV_ROUND_UP(qi.channel_width, 8) * dclk_max;
 	maxdebw = min(soc_bw_params->deprogbwlimit * 1000, peakbw * DEPROGBWPCLIMIT / 100);
 
-	ipqdepth = min(ipqdepthpch, sa->displayrtids / num_channels);
+	ipqdepth = min(ipqdepthpch, display_bw_params->displayrtids / num_channels);
 	/*
 	 * clperchgroup = 4kpagespermempage * clperchperblock,
 	 * clperchperblock = 8 / num_channels * interleave
@@ -608,7 +616,7 @@ static int tgl_get_bw_info(struct intel_display *display,
 		int clpchgroup;
 		int j;
 
-		clpchgroup = (sa->deburst * qi.deinterleave / num_channels) << i;
+		clpchgroup = (display_bw_params->deburst * qi.deinterleave / num_channels) << i;
 
 		if (i < num_groups - 1) {
 			bi_next = &display->bw.max[i + 1];
@@ -854,7 +862,7 @@ void intel_bw_init_hw(struct intel_display *display)
 	if (DISPLAY_VERx100(display) >= 1401 && display->platform.dgfx) {
 		xe2_hpd_get_bw_info(display, dram_info, soc_bw_params);
 	} else if (DISPLAY_VER(display) >= 14) {
-		tgl_get_bw_info(display, dram_info, soc_bw_params, &mtl_sa_info);
+		tgl_get_bw_info(display, dram_info, soc_bw_params, &xelpdp_bw_params);
 	} else if (display->platform.dg2) {
 		dg2_get_bw_info(display);
 	} else if (DISPLAY_VER(display) >= 12) {
@@ -865,11 +873,11 @@ void intel_bw_init_hw(struct intel_display *display)
 		 * parameters.
 		 */
 		if (display->platform.rocketlake)
-			tgl_get_bw_info(display, dram_info, soc_bw_params, &icl_sa_info);
+			tgl_get_bw_info(display, dram_info, soc_bw_params, &gen11_bw_params);
 		else
-			tgl_get_bw_info(display, dram_info, soc_bw_params, &tgl_sa_info);
+			tgl_get_bw_info(display, dram_info, soc_bw_params, &gen12_bw_params);
 	} else if (DISPLAY_VER(display) == 11) {
-		icl_get_bw_info(display, dram_info, soc_bw_params, &icl_sa_info);
+		icl_get_bw_info(display, dram_info, soc_bw_params, &gen11_bw_params);
 	}
 }
 

-- 
2.53.0

[PATCH v3 5/5] drm/i915/bw: Extract get_display_bw_params()

[Gustavo Sousa]

Just like it is done for the platform-specific bandwidth parameters, use
a separate function named get_display_bw_params() to return the display
IP-specific parameters.  This simplifies intel_bw_init_hw() by having
just one call for each of the *_get_bw_info() functions.

v2:
  - Prefer to call get_display_bw_params() only once in
    intel_bw_init_hw() instead of having multiple calls in each of the
    affected *_get_bw_info() functions. (Jani)

v3:
  - Call get_display_bw_params() only after the check on
    HAS_DISPLAY(display). (Jani)
  - Return &gen11_bw_params only if display version is 11. (Matt)

Cc: Jani Nikula <jani.nikula@intel.com>
Cc: Matt Roper <matthew.d.roper@intel.com>
Signed-off-by: Gustavo Sousa <gustavo.sousa@intel.com>
---
 drivers/gpu/drm/i915/display/intel_bw.c | 41 ++++++++++++++++++++++-----------
 1 file changed, 28 insertions(+), 13 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_bw.c b/drivers/gpu/drm/i915/display/intel_bw.c
index 5821397dc27f..07407384e261 100644
--- a/drivers/gpu/drm/i915/display/intel_bw.c
+++ b/drivers/gpu/drm/i915/display/intel_bw.c
@@ -490,6 +490,30 @@ static const struct intel_display_bw_params xelpdp_bw_params = {
 	.displayrtids = 256,
 };
 
+static const struct intel_display_bw_params *get_display_bw_params(struct intel_display *display)
+{
+	if (DISPLAY_VER(display) >= 14) {
+		return &xelpdp_bw_params;
+	} else if (DISPLAY_VER(display) >= 12) {
+		/*
+		 * RKL's SoC was based on ICL and the display, even though being
+		 * gen12, had changes to the memory interface to match gen11's,
+		 * consequently inheriting gen11's display-specific bandwidth
+		 * parameters.
+		 */
+		if (display->platform.rocketlake)
+			return &gen11_bw_params;
+		else
+			return &gen12_bw_params;
+	} else if (DISPLAY_VER(display) == 11) {
+		return &gen11_bw_params;
+	}
+
+	drm_WARN(display->drm, 1, "Display-specific bandwidth parameters not found!\n");
+
+	return NULL;
+}
+
 static int icl_get_bw_info(struct intel_display *display,
 			   const struct dram_info *dram_info,
 			   const struct intel_soc_bw_params *soc_bw_params,
@@ -840,6 +864,7 @@ void intel_bw_init_hw(struct intel_display *display)
 {
 	const struct dram_info *dram_info;
 	const struct intel_soc_bw_params *soc_bw_params;
+	const struct intel_display_bw_params *display_bw_params;
 
 	if (!HAS_DISPLAY(display))
 		return;
@@ -849,6 +874,7 @@ void intel_bw_init_hw(struct intel_display *display)
 
 	dram_info = intel_dram_info(display);
 	soc_bw_params = get_soc_bw_params(display);
+	display_bw_params = get_display_bw_params(display);
 
 	/*
 	 * Starting with Xe3p_LPD, the hardware tells us whether memory has ECC
@@ -861,23 +887,12 @@ void intel_bw_init_hw(struct intel_display *display)
 
 	if (DISPLAY_VERx100(display) >= 1401 && display->platform.dgfx) {
 		xe2_hpd_get_bw_info(display, dram_info, soc_bw_params);
-	} else if (DISPLAY_VER(display) >= 14) {
-		tgl_get_bw_info(display, dram_info, soc_bw_params, &xelpdp_bw_params);
 	} else if (display->platform.dg2) {
 		dg2_get_bw_info(display);
 	} else if (DISPLAY_VER(display) >= 12) {
-		/*
-		 * RKL's SoC was based on ICL and the display, even though being
-		 * gen12, had changes to the memory interface to match gen11's,
-		 * consequently inheriting gen11's display-specific bandwidth
-		 * parameters.
-		 */
-		if (display->platform.rocketlake)
-			tgl_get_bw_info(display, dram_info, soc_bw_params, &gen11_bw_params);
-		else
-			tgl_get_bw_info(display, dram_info, soc_bw_params, &gen12_bw_params);
+		tgl_get_bw_info(display, dram_info, soc_bw_params, display_bw_params);
 	} else if (DISPLAY_VER(display) == 11) {
-		icl_get_bw_info(display, dram_info, soc_bw_params, &gen11_bw_params);
+		icl_get_bw_info(display, dram_info, soc_bw_params, display_bw_params);
 	}
 }
 

-- 
2.53.0

✗ i915.CI.BAT: failure for drm/i915/bw: Split bandwidth params into platform- and display-IP-specific structs (rev3)

[Patchwork]

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

== Series Details ==

Series: drm/i915/bw: Split bandwidth params into platform- and display-IP-specific structs (rev3)
URL   : https://patchwork.freedesktop.org/series/164567/
State : failure

== Summary ==

CI Bug Log - changes from CI_DRM_18491 -> Patchwork_164567v3
====================================================

Summary
-------

  **FAILURE**

  Serious unknown changes coming with Patchwork_164567v3 absolutely need to be
  verified manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in Patchwork_164567v3, 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_164567v3/index.html

Participating hosts (42 -> 40)
------------------------------

  Missing    (2): bat-dg2-13 fi-snb-2520m 

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

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

### IGT changes ###

#### Possible regressions ####

  * igt@i915_module_load@load:
    - bat-dg2-9:          [PASS][1] -> [ABORT][2]
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_18491/bat-dg2-9/igt@i915_module_load@load.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_164567v3/bat-dg2-9/igt@i915_module_load@load.html
    - bat-dg2-14:         [PASS][3] -> [ABORT][4]
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_18491/bat-dg2-14/igt@i915_module_load@load.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_164567v3/bat-dg2-14/igt@i915_module_load@load.html
    - bat-dg2-8:          [PASS][5] -> [ABORT][6]
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_18491/bat-dg2-8/igt@i915_module_load@load.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_164567v3/bat-dg2-8/igt@i915_module_load@load.html

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

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

### IGT changes ###

#### Issues hit ####

  * igt@i915_selftest@live@workarounds:
    - bat-arls-5:         [PASS][7] -> [DMESG-FAIL][8] ([i915#12061]) +1 other test dmesg-fail
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_18491/bat-arls-5/igt@i915_selftest@live@workarounds.html
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_164567v3/bat-arls-5/igt@i915_selftest@live@workarounds.html
    - bat-mtlp-9:         [PASS][9] -> [DMESG-FAIL][10] ([i915#12061]) +1 other test dmesg-fail
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_18491/bat-mtlp-9/igt@i915_selftest@live@workarounds.html
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_164567v3/bat-mtlp-9/igt@i915_selftest@live@workarounds.html

  
#### Possible fixes ####

  * igt@i915_selftest@live@workarounds:
    - bat-arls-6:         [DMESG-FAIL][11] ([i915#12061]) -> [PASS][12] +1 other test pass
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_18491/bat-arls-6/igt@i915_selftest@live@workarounds.html
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_164567v3/bat-arls-6/igt@i915_selftest@live@workarounds.html

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


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

  * Linux: CI_DRM_18491 -> Patchwork_164567v3

  CI-20190529: 20190529
  CI_DRM_18491: 66af2225d31aa8315b0b9f3c735dba4a53e8d3bf @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_8912: 22222b7d987067331459c2a866e6387a669b7f70 @ https://gitlab.freedesktop.org/drm/igt-gpu-tools.git
  Patchwork_164567v3: 66af2225d31aa8315b0b9f3c735dba4a53e8d3bf @ git://anongit.freedesktop.org/gfx-ci/linux

== Logs ==

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

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

Re: ✗ i915.CI.BAT: failure for drm/i915/bw: Split bandwidth params into platform- and display-IP-specific structs (rev3)

[Gustavo Sousa]

Patchwork <patchwork@emeril.freedesktop.org> writes:

> == Series Details ==
>
> Series: drm/i915/bw: Split bandwidth params into platform- and display-IP-specific structs (rev3)
> URL   : https://patchwork.freedesktop.org/series/164567/
> State : failure
>
> == Summary ==
>
> CI Bug Log - changes from CI_DRM_18491 -> Patchwork_164567v3
> ====================================================
>
> Summary
> -------
>
>   **FAILURE**
>
>   Serious unknown changes coming with Patchwork_164567v3 absolutely need to be
>   verified manually.
>   
>   If you think the reported changes have nothing to do with the changes
>   introduced in Patchwork_164567v3, 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_164567v3/index.html
>
> Participating hosts (42 -> 40)
> ------------------------------
>
>   Missing    (2): bat-dg2-13 fi-snb-2520m 
>
> Possible new issues
> -------------------
>
>   Here are the unknown changes that may have been introduced in Patchwork_164567v3:
>
> ### IGT changes ###
>
> #### Possible regressions ####
>
>   * igt@i915_module_load@load:
>     - bat-dg2-9:          [PASS][1] -> [ABORT][2]
>    [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_18491/bat-dg2-9/igt@i915_module_load@load.html
>    [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_164567v3/bat-dg2-9/igt@i915_module_load@load.html
>     - bat-dg2-14:         [PASS][3] -> [ABORT][4]
>    [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_18491/bat-dg2-14/igt@i915_module_load@load.html
>    [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_164567v3/bat-dg2-14/igt@i915_module_load@load.html
>     - bat-dg2-8:          [PASS][5] -> [ABORT][6]
>    [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_18491/bat-dg2-8/igt@i915_module_load@load.html
>    [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_164567v3/bat-dg2-8/igt@i915_module_load@load.html

Hm... This shows the warning because DG2 does not use bandwidth
parameters and get_soc_bw_params() rightfully does not cover DG2.

We could just get rid of the warnings in get_soc_bw_params() and
get_display_bw_params() all together.  The idea of the warning was to
serve as an aid to the developer, but I guess tracing the null pointer
dereference back to those functions shouldn't be too hard?

Another idea is to make sure that only the functions that use those
parameters make the call to get_{soc,display}_bw_params(). Jani, I know
you preferred the other way around, but maybe this is a compelling
reason for moving the call to the direct users?

--
Gustavo Sousa

Re: [PATCH v3 1/5] drm/i915/bw: Don't call intel_dram_info() too early

[Jani Nikula]

On Thu, 14 May 2026, Gustavo Sousa <gustavo.sousa@intel.com> wrote:
> If we end-up bailing early from intel_bw_init_hw() due to
> !HAS_DISPLAY(display), the call to intel_dram_info() to initialize
> dram_info will be meaningless.  Move the call to be done after that
> check.
>
> Cc: Jani Nikula <jani.nikula@intel.com>
> Signed-off-by: Gustavo Sousa <gustavo.sousa@intel.com>

Reviewed-by: Jani Nikula <jani.nikula@intel.com>

> ---
>  drivers/gpu/drm/i915/display/intel_bw.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_bw.c b/drivers/gpu/drm/i915/display/intel_bw.c
> index 9c3a9bbb49f6..7eef693b51ad 100644
> --- a/drivers/gpu/drm/i915/display/intel_bw.c
> +++ b/drivers/gpu/drm/i915/display/intel_bw.c
> @@ -791,11 +791,13 @@ static unsigned int icl_qgv_bw(struct intel_display *display,
>  
>  void intel_bw_init_hw(struct intel_display *display)
>  {
> -	const struct dram_info *dram_info = intel_dram_info(display);
> +	const struct dram_info *dram_info;
>  
>  	if (!HAS_DISPLAY(display))
>  		return;
>  
> +	dram_info = intel_dram_info(display);
> +
>  	/*
>  	 * Starting with Xe3p_LPD, the hardware tells us whether memory has ECC
>  	 * enabled that would impact display bandwidth.  However, so far there

-- 
Jani Nikula, Intel

Re: [PATCH v3 2/5] drm/i915/bw: Extract platform-specific parameters

[Gustavo Sousa]

Gustavo Sousa <gustavo.sousa@intel.com> writes:

> We got confirmation from the hardware team that the bandwidth parameters
> deprogbwlimit and derating are platform-specific and not tied to the
> display IP.  As such, let's make sure that we use platform checks for
> those.
>
> The rest of the members of struct intel_sa_info are tied to the display
> IP and we will deal with them as a follow-up.
>
> v2:
>   - Use good old if-ladder instead of weird-looking pattern "assign ret,
>     check platform, then return ret". (Jani, Matt)
>   - Have a single call site for get_platform_bw_params() and pass the
>     result as parameter to the *_get_bw_info() functions. (Jani)
>   - Avoid using "plat" as abbreviation for "platform". (Jani)
>   - s/_plat_bw_params/_bw_params/, since all of the instances are
>     prefixed with platform names. (Jani)
>   - s/struct intel_platform_bw_params/struct intel_soc_bw_params/.
>     (Matt)
>   - Do not return a default value; prefer to return NULL and
>     intentionally cause a NULL pointer dereference if a platform is
>     missing. (Gustavo)
>
> v3:
>   - Call get_soc_bw_params() only after the check on
>     HAS_DISPLAY(display). (Jani)
>   - Combine if-ladder branches for adl_s_bw_params into a single one.
>     (Matt)
>   - Flatten if-ladder by checking for WCL before PTL (as opposed to
>     checking for WCL inside the brace for PTL). (Matt)
>   - Bail out of intel_bw_init_hw() if display version is below 11.
>     (Gustavo)
>
> Cc: Jani Nikula <jani.nikula@intel.com>
> Cc: Matt Roper <matthew.d.roper@intel.com>
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Signed-off-by: Gustavo Sousa <gustavo.sousa@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_bw.c | 162 ++++++++++++++++++++++----------
>  1 file changed, 114 insertions(+), 48 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_bw.c b/drivers/gpu/drm/i915/display/intel_bw.c
> index 7eef693b51ad..351ecf741b54 100644
> --- a/drivers/gpu/drm/i915/display/intel_bw.c
> +++ b/drivers/gpu/drm/i915/display/intel_bw.c
> @@ -372,81 +372,144 @@ static int icl_sagv_max_dclk(const struct intel_qgv_info *qi)
>  	return dclk;
>  }
>  
> +struct intel_soc_bw_params {
> +	u8 deprogbwlimit;
> +	u8 derating;
> +};
> +
> +static const struct intel_soc_bw_params icl_bw_params = {
> +	.deprogbwlimit = 25,
> +	.derating = 10,
> +};
> +
> +static const struct intel_soc_bw_params tgl_bw_params = {
> +	.deprogbwlimit = 34,
> +	.derating = 10,
> +};
> +
> +static const struct intel_soc_bw_params rkl_bw_params = {
> +	.deprogbwlimit = 20,
> +	.derating = 10,
> +};
> +
> +static const struct intel_soc_bw_params adl_s_bw_params = {
> +	.deprogbwlimit = 38,
> +	.derating = 10,
> +};
> +
> +static const struct intel_soc_bw_params adl_p_bw_params = {
> +	.deprogbwlimit = 38,
> +	.derating = 20,
> +};
> +
> +static const struct intel_soc_bw_params bmg_bw_params = {
> +	.deprogbwlimit = 53,
> +	.derating = 30,
> +};
> +
> +static const struct intel_soc_bw_params bmg_ecc_bw_params = {
> +	.deprogbwlimit = 53,
> +	.derating = 45,
> +};
> +
> +static const struct intel_soc_bw_params ptl_bw_params = {
> +	.deprogbwlimit = 65,
> +	.derating = 10,
> +};
> +
> +static const struct intel_soc_bw_params wcl_bw_params = {
> +	.deprogbwlimit = 22,
> +	.derating = 10,
> +};
> +
> +static const struct intel_soc_bw_params *get_soc_bw_params(struct intel_display *display)
> +{
> +	if (display->platform.dgfx) {
> +		if (display->platform.dg1) {
> +			return &tgl_bw_params;
> +		} else if (display->platform.battlemage) {
> +			const struct dram_info *dram_info = intel_dram_info(display);
> +
> +			if (dram_info->type == INTEL_DRAM_GDDR_ECC)
> +				return &bmg_ecc_bw_params;
> +			else
> +				return &bmg_bw_params;
> +		}
> +	} else {
> +		if (display->platform.icelake ||
> +		    display->platform.jasperlake ||
> +		    display->platform.elkhartlake)
> +			return &icl_bw_params;
> +		else if (display->platform.tigerlake)
> +			return &tgl_bw_params;
> +		else if (display->platform.rocketlake)
> +			return &rkl_bw_params;
> +		else if (display->platform.alderlake_s ||
> +			 display->platform.meteorlake ||
> +			 display->platform.lunarlake)
> +			return &adl_s_bw_params;
> +		else if (display->platform.alderlake_p)
> +			return &adl_p_bw_params;
> +		else if (display->platform.pantherlake_wildcatlake)
> +			return &wcl_bw_params;
> +		else if (display->platform.pantherlake ||
> +			 display->platform.novalake)
> +			return &ptl_bw_params;
> +	}
> +
> +	drm_WARN(display->drm, 1, "Platform-specific bandwidth parameters not found!\n");

CI shows the warning because DG2 does not use bandwidth
parameters and get_soc_bw_params() rightfully does not cover DG2.

We could just get rid of the warnings in get_soc_bw_params() and
get_display_bw_params() all together.  The idea of the warning was to
serve as an aid to the developer, but I guess tracing the null pointer
dereference back to those functions shouldn't be too hard?

Another idea is to make sure that only the functions that use those
parameters make the call to get_{soc,display}_bw_params().

Jani, I know you preferred the other way around, but maybe this is a
compelling reason for moving the call to the direct users?

--
Gustavo Sousa

> +
> +	return NULL;
> +}
> +
>  struct intel_sa_info {
>  	u16 displayrtids;
> -	u8 deburst, deprogbwlimit, derating;
> +	u8 deburst;
>  };
>  
>  static const struct intel_sa_info icl_sa_info = {
>  	.deburst = 8,
> -	.deprogbwlimit = 25, /* GB/s */
>  	.displayrtids = 128,
> -	.derating = 10,
>  };
>  
>  static const struct intel_sa_info tgl_sa_info = {
>  	.deburst = 16,
> -	.deprogbwlimit = 34, /* GB/s */
>  	.displayrtids = 256,
> -	.derating = 10,
>  };
>  
>  static const struct intel_sa_info rkl_sa_info = {
>  	.deburst = 8,
> -	.deprogbwlimit = 20, /* GB/s */
>  	.displayrtids = 128,
> -	.derating = 10,
>  };
>  
>  static const struct intel_sa_info adls_sa_info = {
>  	.deburst = 16,
> -	.deprogbwlimit = 38, /* GB/s */
>  	.displayrtids = 256,
> -	.derating = 10,
>  };
>  
>  static const struct intel_sa_info adlp_sa_info = {
>  	.deburst = 16,
> -	.deprogbwlimit = 38, /* GB/s */
>  	.displayrtids = 256,
> -	.derating = 20,
>  };
>  
>  static const struct intel_sa_info mtl_sa_info = {
>  	.deburst = 32,
> -	.deprogbwlimit = 38, /* GB/s */
>  	.displayrtids = 256,
> -	.derating = 10,
> -};
> -
> -static const struct intel_sa_info xe2_hpd_sa_info = {
> -	.derating = 30,
> -	.deprogbwlimit = 53,
> -	/* Other values not used by simplified algorithm */
> -};
> -
> -static const struct intel_sa_info xe2_hpd_ecc_sa_info = {
> -	.derating = 45,
> -	.deprogbwlimit = 53,
> -	/* Other values not used by simplified algorithm */
>  };
>  
>  static const struct intel_sa_info xe3lpd_sa_info = {
>  	.deburst = 32,
> -	.deprogbwlimit = 65, /* GB/s */
>  	.displayrtids = 256,
> -	.derating = 10,
>  };
>  
>  static const struct intel_sa_info xe3lpd_3002_sa_info = {
>  	.deburst = 32,
> -	.deprogbwlimit = 22, /* GB/s */
>  	.displayrtids = 256,
> -	.derating = 10,
>  };
>  
>  static int icl_get_bw_info(struct intel_display *display,
>  			   const struct dram_info *dram_info,
> +			   const struct intel_soc_bw_params *soc_bw_params,
>  			   const struct intel_sa_info *sa)
>  {
>  	struct intel_qgv_info qi = {};
> @@ -466,7 +529,7 @@ static int icl_get_bw_info(struct intel_display *display,
>  	}
>  
>  	dclk_max = icl_sagv_max_dclk(&qi);
> -	maxdebw = min(sa->deprogbwlimit * 1000, dclk_max * 16 * 6 / 10);
> +	maxdebw = min(soc_bw_params->deprogbwlimit * 1000, dclk_max * 16 * 6 / 10);
>  	ipqdepth = min(ipqdepthpch, sa->displayrtids / num_channels);
>  	qi.deinterleave = DIV_ROUND_UP(num_channels, is_y_tile ? 4 : 2);
>  
> @@ -496,7 +559,7 @@ static int icl_get_bw_info(struct intel_display *display,
>  			bw = DIV_ROUND_UP(sp->dclk * clpchgroup * 32 * num_channels, ct);
>  
>  			bi->deratedbw[j] = min(maxdebw,
> -					       bw * (100 - sa->derating) / 100);
> +					       bw * (100 - soc_bw_params->derating) / 100);
>  
>  			drm_dbg_kms(display->drm,
>  				    "BW%d / QGV %d: num_planes=%d deratedbw=%u\n",
> @@ -518,6 +581,7 @@ static int icl_get_bw_info(struct intel_display *display,
>  
>  static int tgl_get_bw_info(struct intel_display *display,
>  			   const struct dram_info *dram_info,
> +			   const struct intel_soc_bw_params *soc_bw_params,
>  			   const struct intel_sa_info *sa)
>  {
>  	struct intel_qgv_info qi = {};
> @@ -554,7 +618,7 @@ static int tgl_get_bw_info(struct intel_display *display,
>  	dclk_max = icl_sagv_max_dclk(&qi);
>  
>  	peakbw = num_channels * DIV_ROUND_UP(qi.channel_width, 8) * dclk_max;
> -	maxdebw = min(sa->deprogbwlimit * 1000, peakbw * DEPROGBWPCLIMIT / 100);
> +	maxdebw = min(soc_bw_params->deprogbwlimit * 1000, peakbw * DEPROGBWPCLIMIT / 100);
>  
>  	ipqdepth = min(ipqdepthpch, sa->displayrtids / num_channels);
>  	/*
> @@ -599,7 +663,7 @@ static int tgl_get_bw_info(struct intel_display *display,
>  			bw = DIV_ROUND_UP(sp->dclk * clpchgroup * 32 * num_channels, ct);
>  
>  			bi->deratedbw[j] = min(maxdebw,
> -					       bw * (100 - sa->derating) / 100);
> +					       bw * (100 - soc_bw_params->derating) / 100);
>  			bi->peakbw[j] = DIV_ROUND_CLOSEST(sp->dclk *
>  							  num_channels *
>  							  qi.channel_width, 8);
> @@ -661,7 +725,7 @@ static void dg2_get_bw_info(struct intel_display *display)
>  
>  static int xe2_hpd_get_bw_info(struct intel_display *display,
>  			       const struct dram_info *dram_info,
> -			       const struct intel_sa_info *sa)
> +			       const struct intel_soc_bw_params *soc_bw_params)
>  {
>  	struct intel_qgv_info qi = {};
>  	int num_channels = dram_info->num_channels;
> @@ -676,14 +740,14 @@ static int xe2_hpd_get_bw_info(struct intel_display *display,
>  	}
>  
>  	peakbw = num_channels * qi.channel_width / 8 * icl_sagv_max_dclk(&qi);
> -	maxdebw = min(sa->deprogbwlimit * 1000, peakbw * DEPROGBWPCLIMIT / 10);
> +	maxdebw = min(soc_bw_params->deprogbwlimit * 1000, peakbw * DEPROGBWPCLIMIT / 10);
>  
>  	for (i = 0; i < qi.num_points; i++) {
>  		const struct intel_qgv_point *point = &qi.points[i];
>  		int bw = num_channels * (qi.channel_width / 8) * point->dclk;
>  
>  		display->bw.max[0].deratedbw[i] =
> -			min(maxdebw, (100 - sa->derating) * bw / 100);
> +			min(maxdebw, (100 - soc_bw_params->derating) * bw / 100);
>  		display->bw.max[0].peakbw[i] = bw;
>  
>  		drm_dbg_kms(display->drm, "QGV %d: deratedbw=%u peakbw: %u\n",
> @@ -792,11 +856,16 @@ static unsigned int icl_qgv_bw(struct intel_display *display,
>  void intel_bw_init_hw(struct intel_display *display)
>  {
>  	const struct dram_info *dram_info;
> +	const struct intel_soc_bw_params *soc_bw_params;
>  
>  	if (!HAS_DISPLAY(display))
>  		return;
>  
> +	if (DISPLAY_VER(display) < 11)
> +		return;
> +
>  	dram_info = intel_dram_info(display);
> +	soc_bw_params = get_soc_bw_params(display);
>  
>  	/*
>  	 * Starting with Xe3p_LPD, the hardware tells us whether memory has ECC
> @@ -809,28 +878,25 @@ void intel_bw_init_hw(struct intel_display *display)
>  
>  	if (DISPLAY_VER(display) >= 30) {
>  		if (DISPLAY_VERx100(display) == 3002)
> -			tgl_get_bw_info(display, dram_info, &xe3lpd_3002_sa_info);
> +			tgl_get_bw_info(display, dram_info, soc_bw_params, &xe3lpd_3002_sa_info);
>  		else
> -			tgl_get_bw_info(display, dram_info, &xe3lpd_sa_info);
> +			tgl_get_bw_info(display, dram_info, soc_bw_params, &xe3lpd_sa_info);
>  	} else if (DISPLAY_VERx100(display) >= 1401 && display->platform.dgfx) {
> -		if (dram_info->type == INTEL_DRAM_GDDR_ECC)
> -			xe2_hpd_get_bw_info(display, dram_info, &xe2_hpd_ecc_sa_info);
> -		else
> -			xe2_hpd_get_bw_info(display, dram_info, &xe2_hpd_sa_info);
> +		xe2_hpd_get_bw_info(display, dram_info, soc_bw_params);
>  	} else if (DISPLAY_VER(display) >= 14) {
> -		tgl_get_bw_info(display, dram_info, &mtl_sa_info);
> +		tgl_get_bw_info(display, dram_info, soc_bw_params, &mtl_sa_info);
>  	} else if (display->platform.dg2) {
>  		dg2_get_bw_info(display);
>  	} else if (display->platform.alderlake_p) {
> -		tgl_get_bw_info(display, dram_info, &adlp_sa_info);
> +		tgl_get_bw_info(display, dram_info, soc_bw_params, &adlp_sa_info);
>  	} else if (display->platform.alderlake_s) {
> -		tgl_get_bw_info(display, dram_info, &adls_sa_info);
> +		tgl_get_bw_info(display, dram_info, soc_bw_params, &adls_sa_info);
>  	} else if (display->platform.rocketlake) {
> -		tgl_get_bw_info(display, dram_info, &rkl_sa_info);
> +		tgl_get_bw_info(display, dram_info, soc_bw_params, &rkl_sa_info);
>  	} else if (DISPLAY_VER(display) == 12) {
> -		tgl_get_bw_info(display, dram_info, &tgl_sa_info);
> +		tgl_get_bw_info(display, dram_info, soc_bw_params, &tgl_sa_info);
>  	} else if (DISPLAY_VER(display) == 11) {
> -		icl_get_bw_info(display, dram_info, &icl_sa_info);
> +		icl_get_bw_info(display, dram_info, soc_bw_params, &icl_sa_info);
>  	}
>  }
>  
>
> -- 
> 2.53.0

Re: [PATCH v3 2/5] drm/i915/bw: Extract platform-specific parameters

[Jani Nikula]

On Fri, 15 May 2026, Gustavo Sousa <gustavo.sousa@intel.com> wrote:
> Gustavo Sousa <gustavo.sousa@intel.com> writes:
>
>> We got confirmation from the hardware team that the bandwidth parameters
>> deprogbwlimit and derating are platform-specific and not tied to the
>> display IP.  As such, let's make sure that we use platform checks for
>> those.
>>
>> The rest of the members of struct intel_sa_info are tied to the display
>> IP and we will deal with them as a follow-up.
>>
>> v2:
>>   - Use good old if-ladder instead of weird-looking pattern "assign ret,
>>     check platform, then return ret". (Jani, Matt)
>>   - Have a single call site for get_platform_bw_params() and pass the
>>     result as parameter to the *_get_bw_info() functions. (Jani)
>>   - Avoid using "plat" as abbreviation for "platform". (Jani)
>>   - s/_plat_bw_params/_bw_params/, since all of the instances are
>>     prefixed with platform names. (Jani)
>>   - s/struct intel_platform_bw_params/struct intel_soc_bw_params/.
>>     (Matt)
>>   - Do not return a default value; prefer to return NULL and
>>     intentionally cause a NULL pointer dereference if a platform is
>>     missing. (Gustavo)
>>
>> v3:
>>   - Call get_soc_bw_params() only after the check on
>>     HAS_DISPLAY(display). (Jani)
>>   - Combine if-ladder branches for adl_s_bw_params into a single one.
>>     (Matt)
>>   - Flatten if-ladder by checking for WCL before PTL (as opposed to
>>     checking for WCL inside the brace for PTL). (Matt)
>>   - Bail out of intel_bw_init_hw() if display version is below 11.
>>     (Gustavo)
>>
>> Cc: Jani Nikula <jani.nikula@intel.com>
>> Cc: Matt Roper <matthew.d.roper@intel.com>
>> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
>> Signed-off-by: Gustavo Sousa <gustavo.sousa@intel.com>
>> ---
>>  drivers/gpu/drm/i915/display/intel_bw.c | 162 ++++++++++++++++++++++----------
>>  1 file changed, 114 insertions(+), 48 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/display/intel_bw.c b/drivers/gpu/drm/i915/display/intel_bw.c
>> index 7eef693b51ad..351ecf741b54 100644
>> --- a/drivers/gpu/drm/i915/display/intel_bw.c
>> +++ b/drivers/gpu/drm/i915/display/intel_bw.c
>> @@ -372,81 +372,144 @@ static int icl_sagv_max_dclk(const struct intel_qgv_info *qi)
>>  	return dclk;
>>  }
>>  
>> +struct intel_soc_bw_params {
>> +	u8 deprogbwlimit;
>> +	u8 derating;
>> +};
>> +
>> +static const struct intel_soc_bw_params icl_bw_params = {
>> +	.deprogbwlimit = 25,
>> +	.derating = 10,
>> +};
>> +
>> +static const struct intel_soc_bw_params tgl_bw_params = {
>> +	.deprogbwlimit = 34,
>> +	.derating = 10,
>> +};
>> +
>> +static const struct intel_soc_bw_params rkl_bw_params = {
>> +	.deprogbwlimit = 20,
>> +	.derating = 10,
>> +};
>> +
>> +static const struct intel_soc_bw_params adl_s_bw_params = {
>> +	.deprogbwlimit = 38,
>> +	.derating = 10,
>> +};
>> +
>> +static const struct intel_soc_bw_params adl_p_bw_params = {
>> +	.deprogbwlimit = 38,
>> +	.derating = 20,
>> +};
>> +
>> +static const struct intel_soc_bw_params bmg_bw_params = {
>> +	.deprogbwlimit = 53,
>> +	.derating = 30,
>> +};
>> +
>> +static const struct intel_soc_bw_params bmg_ecc_bw_params = {
>> +	.deprogbwlimit = 53,
>> +	.derating = 45,
>> +};
>> +
>> +static const struct intel_soc_bw_params ptl_bw_params = {
>> +	.deprogbwlimit = 65,
>> +	.derating = 10,
>> +};
>> +
>> +static const struct intel_soc_bw_params wcl_bw_params = {
>> +	.deprogbwlimit = 22,
>> +	.derating = 10,
>> +};
>> +
>> +static const struct intel_soc_bw_params *get_soc_bw_params(struct intel_display *display)
>> +{
>> +	if (display->platform.dgfx) {
>> +		if (display->platform.dg1) {
>> +			return &tgl_bw_params;
>> +		} else if (display->platform.battlemage) {
>> +			const struct dram_info *dram_info = intel_dram_info(display);
>> +
>> +			if (dram_info->type == INTEL_DRAM_GDDR_ECC)
>> +				return &bmg_ecc_bw_params;
>> +			else
>> +				return &bmg_bw_params;
>> +		}
>> +	} else {
>> +		if (display->platform.icelake ||
>> +		    display->platform.jasperlake ||
>> +		    display->platform.elkhartlake)
>> +			return &icl_bw_params;
>> +		else if (display->platform.tigerlake)
>> +			return &tgl_bw_params;
>> +		else if (display->platform.rocketlake)
>> +			return &rkl_bw_params;
>> +		else if (display->platform.alderlake_s ||
>> +			 display->platform.meteorlake ||
>> +			 display->platform.lunarlake)
>> +			return &adl_s_bw_params;
>> +		else if (display->platform.alderlake_p)
>> +			return &adl_p_bw_params;
>> +		else if (display->platform.pantherlake_wildcatlake)
>> +			return &wcl_bw_params;
>> +		else if (display->platform.pantherlake ||
>> +			 display->platform.novalake)
>> +			return &ptl_bw_params;
>> +	}
>> +
>> +	drm_WARN(display->drm, 1, "Platform-specific bandwidth parameters not found!\n");
>
> CI shows the warning because DG2 does not use bandwidth
> parameters and get_soc_bw_params() rightfully does not cover DG2.
>
> We could just get rid of the warnings in get_soc_bw_params() and
> get_display_bw_params() all together.  The idea of the warning was to
> serve as an aid to the developer, but I guess tracing the null pointer
> dereference back to those functions shouldn't be too hard?
>
> Another idea is to make sure that only the functions that use those
> parameters make the call to get_{soc,display}_bw_params().
>
> Jani, I know you preferred the other way around, but maybe this is a
> compelling reason for moving the call to the direct users?

I still prefer figuring the pointer out early, and passing it around to
the places that need it. Actually, ditto for dram_info above. I
refactored this stuff once to call intel_dram_info() once, and pass the
pointer around.

I think the alternatives are:

- Remove the drm_WARN() altogether.

- Return NULL directly for platforms where the parameters don't make
  sense, e.g. DG2.

BR,
Jani.

>
> --
> Gustavo Sousa
>
>> +
>> +	return NULL;
>> +}
>> +
>>  struct intel_sa_info {
>>  	u16 displayrtids;
>> -	u8 deburst, deprogbwlimit, derating;
>> +	u8 deburst;
>>  };
>>  
>>  static const struct intel_sa_info icl_sa_info = {
>>  	.deburst = 8,
>> -	.deprogbwlimit = 25, /* GB/s */
>>  	.displayrtids = 128,
>> -	.derating = 10,
>>  };
>>  
>>  static const struct intel_sa_info tgl_sa_info = {
>>  	.deburst = 16,
>> -	.deprogbwlimit = 34, /* GB/s */
>>  	.displayrtids = 256,
>> -	.derating = 10,
>>  };
>>  
>>  static const struct intel_sa_info rkl_sa_info = {
>>  	.deburst = 8,
>> -	.deprogbwlimit = 20, /* GB/s */
>>  	.displayrtids = 128,
>> -	.derating = 10,
>>  };
>>  
>>  static const struct intel_sa_info adls_sa_info = {
>>  	.deburst = 16,
>> -	.deprogbwlimit = 38, /* GB/s */
>>  	.displayrtids = 256,
>> -	.derating = 10,
>>  };
>>  
>>  static const struct intel_sa_info adlp_sa_info = {
>>  	.deburst = 16,
>> -	.deprogbwlimit = 38, /* GB/s */
>>  	.displayrtids = 256,
>> -	.derating = 20,
>>  };
>>  
>>  static const struct intel_sa_info mtl_sa_info = {
>>  	.deburst = 32,
>> -	.deprogbwlimit = 38, /* GB/s */
>>  	.displayrtids = 256,
>> -	.derating = 10,
>> -};
>> -
>> -static const struct intel_sa_info xe2_hpd_sa_info = {
>> -	.derating = 30,
>> -	.deprogbwlimit = 53,
>> -	/* Other values not used by simplified algorithm */
>> -};
>> -
>> -static const struct intel_sa_info xe2_hpd_ecc_sa_info = {
>> -	.derating = 45,
>> -	.deprogbwlimit = 53,
>> -	/* Other values not used by simplified algorithm */
>>  };
>>  
>>  static const struct intel_sa_info xe3lpd_sa_info = {
>>  	.deburst = 32,
>> -	.deprogbwlimit = 65, /* GB/s */
>>  	.displayrtids = 256,
>> -	.derating = 10,
>>  };
>>  
>>  static const struct intel_sa_info xe3lpd_3002_sa_info = {
>>  	.deburst = 32,
>> -	.deprogbwlimit = 22, /* GB/s */
>>  	.displayrtids = 256,
>> -	.derating = 10,
>>  };
>>  
>>  static int icl_get_bw_info(struct intel_display *display,
>>  			   const struct dram_info *dram_info,
>> +			   const struct intel_soc_bw_params *soc_bw_params,
>>  			   const struct intel_sa_info *sa)
>>  {
>>  	struct intel_qgv_info qi = {};
>> @@ -466,7 +529,7 @@ static int icl_get_bw_info(struct intel_display *display,
>>  	}
>>  
>>  	dclk_max = icl_sagv_max_dclk(&qi);
>> -	maxdebw = min(sa->deprogbwlimit * 1000, dclk_max * 16 * 6 / 10);
>> +	maxdebw = min(soc_bw_params->deprogbwlimit * 1000, dclk_max * 16 * 6 / 10);
>>  	ipqdepth = min(ipqdepthpch, sa->displayrtids / num_channels);
>>  	qi.deinterleave = DIV_ROUND_UP(num_channels, is_y_tile ? 4 : 2);
>>  
>> @@ -496,7 +559,7 @@ static int icl_get_bw_info(struct intel_display *display,
>>  			bw = DIV_ROUND_UP(sp->dclk * clpchgroup * 32 * num_channels, ct);
>>  
>>  			bi->deratedbw[j] = min(maxdebw,
>> -					       bw * (100 - sa->derating) / 100);
>> +					       bw * (100 - soc_bw_params->derating) / 100);
>>  
>>  			drm_dbg_kms(display->drm,
>>  				    "BW%d / QGV %d: num_planes=%d deratedbw=%u\n",
>> @@ -518,6 +581,7 @@ static int icl_get_bw_info(struct intel_display *display,
>>  
>>  static int tgl_get_bw_info(struct intel_display *display,
>>  			   const struct dram_info *dram_info,
>> +			   const struct intel_soc_bw_params *soc_bw_params,
>>  			   const struct intel_sa_info *sa)
>>  {
>>  	struct intel_qgv_info qi = {};
>> @@ -554,7 +618,7 @@ static int tgl_get_bw_info(struct intel_display *display,
>>  	dclk_max = icl_sagv_max_dclk(&qi);
>>  
>>  	peakbw = num_channels * DIV_ROUND_UP(qi.channel_width, 8) * dclk_max;
>> -	maxdebw = min(sa->deprogbwlimit * 1000, peakbw * DEPROGBWPCLIMIT / 100);
>> +	maxdebw = min(soc_bw_params->deprogbwlimit * 1000, peakbw * DEPROGBWPCLIMIT / 100);
>>  
>>  	ipqdepth = min(ipqdepthpch, sa->displayrtids / num_channels);
>>  	/*
>> @@ -599,7 +663,7 @@ static int tgl_get_bw_info(struct intel_display *display,
>>  			bw = DIV_ROUND_UP(sp->dclk * clpchgroup * 32 * num_channels, ct);
>>  
>>  			bi->deratedbw[j] = min(maxdebw,
>> -					       bw * (100 - sa->derating) / 100);
>> +					       bw * (100 - soc_bw_params->derating) / 100);
>>  			bi->peakbw[j] = DIV_ROUND_CLOSEST(sp->dclk *
>>  							  num_channels *
>>  							  qi.channel_width, 8);
>> @@ -661,7 +725,7 @@ static void dg2_get_bw_info(struct intel_display *display)
>>  
>>  static int xe2_hpd_get_bw_info(struct intel_display *display,
>>  			       const struct dram_info *dram_info,
>> -			       const struct intel_sa_info *sa)
>> +			       const struct intel_soc_bw_params *soc_bw_params)
>>  {
>>  	struct intel_qgv_info qi = {};
>>  	int num_channels = dram_info->num_channels;
>> @@ -676,14 +740,14 @@ static int xe2_hpd_get_bw_info(struct intel_display *display,
>>  	}
>>  
>>  	peakbw = num_channels * qi.channel_width / 8 * icl_sagv_max_dclk(&qi);
>> -	maxdebw = min(sa->deprogbwlimit * 1000, peakbw * DEPROGBWPCLIMIT / 10);
>> +	maxdebw = min(soc_bw_params->deprogbwlimit * 1000, peakbw * DEPROGBWPCLIMIT / 10);
>>  
>>  	for (i = 0; i < qi.num_points; i++) {
>>  		const struct intel_qgv_point *point = &qi.points[i];
>>  		int bw = num_channels * (qi.channel_width / 8) * point->dclk;
>>  
>>  		display->bw.max[0].deratedbw[i] =
>> -			min(maxdebw, (100 - sa->derating) * bw / 100);
>> +			min(maxdebw, (100 - soc_bw_params->derating) * bw / 100);
>>  		display->bw.max[0].peakbw[i] = bw;
>>  
>>  		drm_dbg_kms(display->drm, "QGV %d: deratedbw=%u peakbw: %u\n",
>> @@ -792,11 +856,16 @@ static unsigned int icl_qgv_bw(struct intel_display *display,
>>  void intel_bw_init_hw(struct intel_display *display)
>>  {
>>  	const struct dram_info *dram_info;
>> +	const struct intel_soc_bw_params *soc_bw_params;
>>  
>>  	if (!HAS_DISPLAY(display))
>>  		return;
>>  
>> +	if (DISPLAY_VER(display) < 11)
>> +		return;
>> +
>>  	dram_info = intel_dram_info(display);
>> +	soc_bw_params = get_soc_bw_params(display);
>>  
>>  	/*
>>  	 * Starting with Xe3p_LPD, the hardware tells us whether memory has ECC
>> @@ -809,28 +878,25 @@ void intel_bw_init_hw(struct intel_display *display)
>>  
>>  	if (DISPLAY_VER(display) >= 30) {
>>  		if (DISPLAY_VERx100(display) == 3002)
>> -			tgl_get_bw_info(display, dram_info, &xe3lpd_3002_sa_info);
>> +			tgl_get_bw_info(display, dram_info, soc_bw_params, &xe3lpd_3002_sa_info);
>>  		else
>> -			tgl_get_bw_info(display, dram_info, &xe3lpd_sa_info);
>> +			tgl_get_bw_info(display, dram_info, soc_bw_params, &xe3lpd_sa_info);
>>  	} else if (DISPLAY_VERx100(display) >= 1401 && display->platform.dgfx) {
>> -		if (dram_info->type == INTEL_DRAM_GDDR_ECC)
>> -			xe2_hpd_get_bw_info(display, dram_info, &xe2_hpd_ecc_sa_info);
>> -		else
>> -			xe2_hpd_get_bw_info(display, dram_info, &xe2_hpd_sa_info);
>> +		xe2_hpd_get_bw_info(display, dram_info, soc_bw_params);
>>  	} else if (DISPLAY_VER(display) >= 14) {
>> -		tgl_get_bw_info(display, dram_info, &mtl_sa_info);
>> +		tgl_get_bw_info(display, dram_info, soc_bw_params, &mtl_sa_info);
>>  	} else if (display->platform.dg2) {
>>  		dg2_get_bw_info(display);
>>  	} else if (display->platform.alderlake_p) {
>> -		tgl_get_bw_info(display, dram_info, &adlp_sa_info);
>> +		tgl_get_bw_info(display, dram_info, soc_bw_params, &adlp_sa_info);
>>  	} else if (display->platform.alderlake_s) {
>> -		tgl_get_bw_info(display, dram_info, &adls_sa_info);
>> +		tgl_get_bw_info(display, dram_info, soc_bw_params, &adls_sa_info);
>>  	} else if (display->platform.rocketlake) {
>> -		tgl_get_bw_info(display, dram_info, &rkl_sa_info);
>> +		tgl_get_bw_info(display, dram_info, soc_bw_params, &rkl_sa_info);
>>  	} else if (DISPLAY_VER(display) == 12) {
>> -		tgl_get_bw_info(display, dram_info, &tgl_sa_info);
>> +		tgl_get_bw_info(display, dram_info, soc_bw_params, &tgl_sa_info);
>>  	} else if (DISPLAY_VER(display) == 11) {
>> -		icl_get_bw_info(display, dram_info, &icl_sa_info);
>> +		icl_get_bw_info(display, dram_info, soc_bw_params, &icl_sa_info);
>>  	}
>>  }
>>  
>>
>> -- 
>> 2.53.0

-- 
Jani Nikula, Intel

Re: [PATCH v3 2/5] drm/i915/bw: Extract platform-specific parameters

[Gustavo Sousa]

Jani Nikula <jani.nikula@intel.com> writes:

> On Fri, 15 May 2026, Gustavo Sousa <gustavo.sousa@intel.com> wrote:
>> Gustavo Sousa <gustavo.sousa@intel.com> writes:
>>
>>> We got confirmation from the hardware team that the bandwidth parameters
>>> deprogbwlimit and derating are platform-specific and not tied to the
>>> display IP.  As such, let's make sure that we use platform checks for
>>> those.
>>>
>>> The rest of the members of struct intel_sa_info are tied to the display
>>> IP and we will deal with them as a follow-up.
>>>
>>> v2:
>>>   - Use good old if-ladder instead of weird-looking pattern "assign ret,
>>>     check platform, then return ret". (Jani, Matt)
>>>   - Have a single call site for get_platform_bw_params() and pass the
>>>     result as parameter to the *_get_bw_info() functions. (Jani)
>>>   - Avoid using "plat" as abbreviation for "platform". (Jani)
>>>   - s/_plat_bw_params/_bw_params/, since all of the instances are
>>>     prefixed with platform names. (Jani)
>>>   - s/struct intel_platform_bw_params/struct intel_soc_bw_params/.
>>>     (Matt)
>>>   - Do not return a default value; prefer to return NULL and
>>>     intentionally cause a NULL pointer dereference if a platform is
>>>     missing. (Gustavo)
>>>
>>> v3:
>>>   - Call get_soc_bw_params() only after the check on
>>>     HAS_DISPLAY(display). (Jani)
>>>   - Combine if-ladder branches for adl_s_bw_params into a single one.
>>>     (Matt)
>>>   - Flatten if-ladder by checking for WCL before PTL (as opposed to
>>>     checking for WCL inside the brace for PTL). (Matt)
>>>   - Bail out of intel_bw_init_hw() if display version is below 11.
>>>     (Gustavo)
>>>
>>> Cc: Jani Nikula <jani.nikula@intel.com>
>>> Cc: Matt Roper <matthew.d.roper@intel.com>
>>> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
>>> Signed-off-by: Gustavo Sousa <gustavo.sousa@intel.com>
>>> ---
>>>  drivers/gpu/drm/i915/display/intel_bw.c | 162 ++++++++++++++++++++++----------
>>>  1 file changed, 114 insertions(+), 48 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/i915/display/intel_bw.c b/drivers/gpu/drm/i915/display/intel_bw.c
>>> index 7eef693b51ad..351ecf741b54 100644
>>> --- a/drivers/gpu/drm/i915/display/intel_bw.c
>>> +++ b/drivers/gpu/drm/i915/display/intel_bw.c
>>> @@ -372,81 +372,144 @@ static int icl_sagv_max_dclk(const struct intel_qgv_info *qi)
>>>  	return dclk;
>>>  }
>>>  
>>> +struct intel_soc_bw_params {
>>> +	u8 deprogbwlimit;
>>> +	u8 derating;
>>> +};
>>> +
>>> +static const struct intel_soc_bw_params icl_bw_params = {
>>> +	.deprogbwlimit = 25,
>>> +	.derating = 10,
>>> +};
>>> +
>>> +static const struct intel_soc_bw_params tgl_bw_params = {
>>> +	.deprogbwlimit = 34,
>>> +	.derating = 10,
>>> +};
>>> +
>>> +static const struct intel_soc_bw_params rkl_bw_params = {
>>> +	.deprogbwlimit = 20,
>>> +	.derating = 10,
>>> +};
>>> +
>>> +static const struct intel_soc_bw_params adl_s_bw_params = {
>>> +	.deprogbwlimit = 38,
>>> +	.derating = 10,
>>> +};
>>> +
>>> +static const struct intel_soc_bw_params adl_p_bw_params = {
>>> +	.deprogbwlimit = 38,
>>> +	.derating = 20,
>>> +};
>>> +
>>> +static const struct intel_soc_bw_params bmg_bw_params = {
>>> +	.deprogbwlimit = 53,
>>> +	.derating = 30,
>>> +};
>>> +
>>> +static const struct intel_soc_bw_params bmg_ecc_bw_params = {
>>> +	.deprogbwlimit = 53,
>>> +	.derating = 45,
>>> +};
>>> +
>>> +static const struct intel_soc_bw_params ptl_bw_params = {
>>> +	.deprogbwlimit = 65,
>>> +	.derating = 10,
>>> +};
>>> +
>>> +static const struct intel_soc_bw_params wcl_bw_params = {
>>> +	.deprogbwlimit = 22,
>>> +	.derating = 10,
>>> +};
>>> +
>>> +static const struct intel_soc_bw_params *get_soc_bw_params(struct intel_display *display)
>>> +{
>>> +	if (display->platform.dgfx) {
>>> +		if (display->platform.dg1) {
>>> +			return &tgl_bw_params;
>>> +		} else if (display->platform.battlemage) {
>>> +			const struct dram_info *dram_info = intel_dram_info(display);
>>> +
>>> +			if (dram_info->type == INTEL_DRAM_GDDR_ECC)
>>> +				return &bmg_ecc_bw_params;
>>> +			else
>>> +				return &bmg_bw_params;
>>> +		}
>>> +	} else {
>>> +		if (display->platform.icelake ||
>>> +		    display->platform.jasperlake ||
>>> +		    display->platform.elkhartlake)
>>> +			return &icl_bw_params;
>>> +		else if (display->platform.tigerlake)
>>> +			return &tgl_bw_params;
>>> +		else if (display->platform.rocketlake)
>>> +			return &rkl_bw_params;
>>> +		else if (display->platform.alderlake_s ||
>>> +			 display->platform.meteorlake ||
>>> +			 display->platform.lunarlake)
>>> +			return &adl_s_bw_params;
>>> +		else if (display->platform.alderlake_p)
>>> +			return &adl_p_bw_params;
>>> +		else if (display->platform.pantherlake_wildcatlake)
>>> +			return &wcl_bw_params;
>>> +		else if (display->platform.pantherlake ||
>>> +			 display->platform.novalake)
>>> +			return &ptl_bw_params;
>>> +	}
>>> +
>>> +	drm_WARN(display->drm, 1, "Platform-specific bandwidth parameters not found!\n");
>>
>> CI shows the warning because DG2 does not use bandwidth
>> parameters and get_soc_bw_params() rightfully does not cover DG2.
>>
>> We could just get rid of the warnings in get_soc_bw_params() and
>> get_display_bw_params() all together.  The idea of the warning was to
>> serve as an aid to the developer, but I guess tracing the null pointer
>> dereference back to those functions shouldn't be too hard?
>>
>> Another idea is to make sure that only the functions that use those
>> parameters make the call to get_{soc,display}_bw_params().
>>
>> Jani, I know you preferred the other way around, but maybe this is a
>> compelling reason for moving the call to the direct users?
>
> I still prefer figuring the pointer out early, and passing it around to
> the places that need it. Actually, ditto for dram_info above. I

Ack.

> refactored this stuff once to call intel_dram_info() once, and pass the
> pointer around.
>
> I think the alternatives are:
>
> - Remove the drm_WARN() altogether.

Okay.  I think I'll just take the easy way out by dropping them in the
next iteration.

--
Gustavo Sousa

>
> - Return NULL directly for platforms where the parameters don't make
>   sense, e.g. DG2.
>
> BR,
> Jani.
>
>>
>> --
>> Gustavo Sousa
>>
>>> +
>>> +	return NULL;
>>> +}
>>> +
>>>  struct intel_sa_info {
>>>  	u16 displayrtids;
>>> -	u8 deburst, deprogbwlimit, derating;
>>> +	u8 deburst;
>>>  };
>>>  
>>>  static const struct intel_sa_info icl_sa_info = {
>>>  	.deburst = 8,
>>> -	.deprogbwlimit = 25, /* GB/s */
>>>  	.displayrtids = 128,
>>> -	.derating = 10,
>>>  };
>>>  
>>>  static const struct intel_sa_info tgl_sa_info = {
>>>  	.deburst = 16,
>>> -	.deprogbwlimit = 34, /* GB/s */
>>>  	.displayrtids = 256,
>>> -	.derating = 10,
>>>  };
>>>  
>>>  static const struct intel_sa_info rkl_sa_info = {
>>>  	.deburst = 8,
>>> -	.deprogbwlimit = 20, /* GB/s */
>>>  	.displayrtids = 128,
>>> -	.derating = 10,
>>>  };
>>>  
>>>  static const struct intel_sa_info adls_sa_info = {
>>>  	.deburst = 16,
>>> -	.deprogbwlimit = 38, /* GB/s */
>>>  	.displayrtids = 256,
>>> -	.derating = 10,
>>>  };
>>>  
>>>  static const struct intel_sa_info adlp_sa_info = {
>>>  	.deburst = 16,
>>> -	.deprogbwlimit = 38, /* GB/s */
>>>  	.displayrtids = 256,
>>> -	.derating = 20,
>>>  };
>>>  
>>>  static const struct intel_sa_info mtl_sa_info = {
>>>  	.deburst = 32,
>>> -	.deprogbwlimit = 38, /* GB/s */
>>>  	.displayrtids = 256,
>>> -	.derating = 10,
>>> -};
>>> -
>>> -static const struct intel_sa_info xe2_hpd_sa_info = {
>>> -	.derating = 30,
>>> -	.deprogbwlimit = 53,
>>> -	/* Other values not used by simplified algorithm */
>>> -};
>>> -
>>> -static const struct intel_sa_info xe2_hpd_ecc_sa_info = {
>>> -	.derating = 45,
>>> -	.deprogbwlimit = 53,
>>> -	/* Other values not used by simplified algorithm */
>>>  };
>>>  
>>>  static const struct intel_sa_info xe3lpd_sa_info = {
>>>  	.deburst = 32,
>>> -	.deprogbwlimit = 65, /* GB/s */
>>>  	.displayrtids = 256,
>>> -	.derating = 10,
>>>  };
>>>  
>>>  static const struct intel_sa_info xe3lpd_3002_sa_info = {
>>>  	.deburst = 32,
>>> -	.deprogbwlimit = 22, /* GB/s */
>>>  	.displayrtids = 256,
>>> -	.derating = 10,
>>>  };
>>>  
>>>  static int icl_get_bw_info(struct intel_display *display,
>>>  			   const struct dram_info *dram_info,
>>> +			   const struct intel_soc_bw_params *soc_bw_params,
>>>  			   const struct intel_sa_info *sa)
>>>  {
>>>  	struct intel_qgv_info qi = {};
>>> @@ -466,7 +529,7 @@ static int icl_get_bw_info(struct intel_display *display,
>>>  	}
>>>  
>>>  	dclk_max = icl_sagv_max_dclk(&qi);
>>> -	maxdebw = min(sa->deprogbwlimit * 1000, dclk_max * 16 * 6 / 10);
>>> +	maxdebw = min(soc_bw_params->deprogbwlimit * 1000, dclk_max * 16 * 6 / 10);
>>>  	ipqdepth = min(ipqdepthpch, sa->displayrtids / num_channels);
>>>  	qi.deinterleave = DIV_ROUND_UP(num_channels, is_y_tile ? 4 : 2);
>>>  
>>> @@ -496,7 +559,7 @@ static int icl_get_bw_info(struct intel_display *display,
>>>  			bw = DIV_ROUND_UP(sp->dclk * clpchgroup * 32 * num_channels, ct);
>>>  
>>>  			bi->deratedbw[j] = min(maxdebw,
>>> -					       bw * (100 - sa->derating) / 100);
>>> +					       bw * (100 - soc_bw_params->derating) / 100);
>>>  
>>>  			drm_dbg_kms(display->drm,
>>>  				    "BW%d / QGV %d: num_planes=%d deratedbw=%u\n",
>>> @@ -518,6 +581,7 @@ static int icl_get_bw_info(struct intel_display *display,
>>>  
>>>  static int tgl_get_bw_info(struct intel_display *display,
>>>  			   const struct dram_info *dram_info,
>>> +			   const struct intel_soc_bw_params *soc_bw_params,
>>>  			   const struct intel_sa_info *sa)
>>>  {
>>>  	struct intel_qgv_info qi = {};
>>> @@ -554,7 +618,7 @@ static int tgl_get_bw_info(struct intel_display *display,
>>>  	dclk_max = icl_sagv_max_dclk(&qi);
>>>  
>>>  	peakbw = num_channels * DIV_ROUND_UP(qi.channel_width, 8) * dclk_max;
>>> -	maxdebw = min(sa->deprogbwlimit * 1000, peakbw * DEPROGBWPCLIMIT / 100);
>>> +	maxdebw = min(soc_bw_params->deprogbwlimit * 1000, peakbw * DEPROGBWPCLIMIT / 100);
>>>  
>>>  	ipqdepth = min(ipqdepthpch, sa->displayrtids / num_channels);
>>>  	/*
>>> @@ -599,7 +663,7 @@ static int tgl_get_bw_info(struct intel_display *display,
>>>  			bw = DIV_ROUND_UP(sp->dclk * clpchgroup * 32 * num_channels, ct);
>>>  
>>>  			bi->deratedbw[j] = min(maxdebw,
>>> -					       bw * (100 - sa->derating) / 100);
>>> +					       bw * (100 - soc_bw_params->derating) / 100);
>>>  			bi->peakbw[j] = DIV_ROUND_CLOSEST(sp->dclk *
>>>  							  num_channels *
>>>  							  qi.channel_width, 8);
>>> @@ -661,7 +725,7 @@ static void dg2_get_bw_info(struct intel_display *display)
>>>  
>>>  static int xe2_hpd_get_bw_info(struct intel_display *display,
>>>  			       const struct dram_info *dram_info,
>>> -			       const struct intel_sa_info *sa)
>>> +			       const struct intel_soc_bw_params *soc_bw_params)
>>>  {
>>>  	struct intel_qgv_info qi = {};
>>>  	int num_channels = dram_info->num_channels;
>>> @@ -676,14 +740,14 @@ static int xe2_hpd_get_bw_info(struct intel_display *display,
>>>  	}
>>>  
>>>  	peakbw = num_channels * qi.channel_width / 8 * icl_sagv_max_dclk(&qi);
>>> -	maxdebw = min(sa->deprogbwlimit * 1000, peakbw * DEPROGBWPCLIMIT / 10);
>>> +	maxdebw = min(soc_bw_params->deprogbwlimit * 1000, peakbw * DEPROGBWPCLIMIT / 10);
>>>  
>>>  	for (i = 0; i < qi.num_points; i++) {
>>>  		const struct intel_qgv_point *point = &qi.points[i];
>>>  		int bw = num_channels * (qi.channel_width / 8) * point->dclk;
>>>  
>>>  		display->bw.max[0].deratedbw[i] =
>>> -			min(maxdebw, (100 - sa->derating) * bw / 100);
>>> +			min(maxdebw, (100 - soc_bw_params->derating) * bw / 100);
>>>  		display->bw.max[0].peakbw[i] = bw;
>>>  
>>>  		drm_dbg_kms(display->drm, "QGV %d: deratedbw=%u peakbw: %u\n",
>>> @@ -792,11 +856,16 @@ static unsigned int icl_qgv_bw(struct intel_display *display,
>>>  void intel_bw_init_hw(struct intel_display *display)
>>>  {
>>>  	const struct dram_info *dram_info;
>>> +	const struct intel_soc_bw_params *soc_bw_params;
>>>  
>>>  	if (!HAS_DISPLAY(display))
>>>  		return;
>>>  
>>> +	if (DISPLAY_VER(display) < 11)
>>> +		return;
>>> +
>>>  	dram_info = intel_dram_info(display);
>>> +	soc_bw_params = get_soc_bw_params(display);
>>>  
>>>  	/*
>>>  	 * Starting with Xe3p_LPD, the hardware tells us whether memory has ECC
>>> @@ -809,28 +878,25 @@ void intel_bw_init_hw(struct intel_display *display)
>>>  
>>>  	if (DISPLAY_VER(display) >= 30) {
>>>  		if (DISPLAY_VERx100(display) == 3002)
>>> -			tgl_get_bw_info(display, dram_info, &xe3lpd_3002_sa_info);
>>> +			tgl_get_bw_info(display, dram_info, soc_bw_params, &xe3lpd_3002_sa_info);
>>>  		else
>>> -			tgl_get_bw_info(display, dram_info, &xe3lpd_sa_info);
>>> +			tgl_get_bw_info(display, dram_info, soc_bw_params, &xe3lpd_sa_info);
>>>  	} else if (DISPLAY_VERx100(display) >= 1401 && display->platform.dgfx) {
>>> -		if (dram_info->type == INTEL_DRAM_GDDR_ECC)
>>> -			xe2_hpd_get_bw_info(display, dram_info, &xe2_hpd_ecc_sa_info);
>>> -		else
>>> -			xe2_hpd_get_bw_info(display, dram_info, &xe2_hpd_sa_info);
>>> +		xe2_hpd_get_bw_info(display, dram_info, soc_bw_params);
>>>  	} else if (DISPLAY_VER(display) >= 14) {
>>> -		tgl_get_bw_info(display, dram_info, &mtl_sa_info);
>>> +		tgl_get_bw_info(display, dram_info, soc_bw_params, &mtl_sa_info);
>>>  	} else if (display->platform.dg2) {
>>>  		dg2_get_bw_info(display);
>>>  	} else if (display->platform.alderlake_p) {
>>> -		tgl_get_bw_info(display, dram_info, &adlp_sa_info);
>>> +		tgl_get_bw_info(display, dram_info, soc_bw_params, &adlp_sa_info);
>>>  	} else if (display->platform.alderlake_s) {
>>> -		tgl_get_bw_info(display, dram_info, &adls_sa_info);
>>> +		tgl_get_bw_info(display, dram_info, soc_bw_params, &adls_sa_info);
>>>  	} else if (display->platform.rocketlake) {
>>> -		tgl_get_bw_info(display, dram_info, &rkl_sa_info);
>>> +		tgl_get_bw_info(display, dram_info, soc_bw_params, &rkl_sa_info);
>>>  	} else if (DISPLAY_VER(display) == 12) {
>>> -		tgl_get_bw_info(display, dram_info, &tgl_sa_info);
>>> +		tgl_get_bw_info(display, dram_info, soc_bw_params, &tgl_sa_info);
>>>  	} else if (DISPLAY_VER(display) == 11) {
>>> -		icl_get_bw_info(display, dram_info, &icl_sa_info);
>>> +		icl_get_bw_info(display, dram_info, soc_bw_params, &icl_sa_info);
>>>  	}
>>>  }
>>>  
>>>
>>> -- 
>>> 2.53.0
>
> -- 
> Jani Nikula, Intel