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

[PATCH v2 0/4] 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 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 (4):
      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 | 208 ++++++++++++++++++++------------
 1 file changed, 132 insertions(+), 76 deletions(-)
---
base-commit: 0a6c4b1478138dae3ff979b9e95879c6a4def444
change-id: 20260408-separate-platform-from-diplay-ip-specific-bw-params-65bfba0603be

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

[PATCH v2 1/4] 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)

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 | 161 ++++++++++++++++++++++----------
 1 file changed, 113 insertions(+), 48 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_bw.c b/drivers/gpu/drm/i915/display/intel_bw.c
index 9c3a9bbb49f6..cf6756b8ae52 100644
--- a/drivers/gpu/drm/i915/display/intel_bw.c
+++ b/drivers/gpu/drm/i915/display/intel_bw.c
@@ -372,81 +372,147 @@ 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) {
+			return &adl_s_bw_params;
+		} else if (display->platform.alderlake_p) {
+			return &adl_p_bw_params;
+		} else if (display->platform.meteorlake ||
+			   display->platform.lunarlake) {
+			return &adl_s_bw_params;
+		} else if (display->platform.pantherlake ||
+			   display->platform.novalake) {
+			if (display->platform.pantherlake_wildcatlake)
+				return &wcl_bw_params;
+			else
+				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 +532,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 +562,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 +584,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 +621,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 +666,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 +728,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 +743,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,6 +859,7 @@ 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 intel_soc_bw_params *soc_bw_params = get_soc_bw_params(display);
 
 	if (!HAS_DISPLAY(display))
 		return;
@@ -807,28 +875,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 v2 1/4] drm/i915/bw: Extract platform-specific parameters

[Matt Roper]

On Mon, May 11, 2026 at 01:30:56PM -0300, Gustavo Sousa wrote:
> 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)
> 
> 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 | 161 ++++++++++++++++++++++----------
>  1 file changed, 113 insertions(+), 48 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_bw.c b/drivers/gpu/drm/i915/display/intel_bw.c
> index 9c3a9bbb49f6..cf6756b8ae52 100644
> --- a/drivers/gpu/drm/i915/display/intel_bw.c
> +++ b/drivers/gpu/drm/i915/display/intel_bw.c
> @@ -372,81 +372,147 @@ 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) {
> +			return &adl_s_bw_params;
> +		} else if (display->platform.alderlake_p) {
> +			return &adl_p_bw_params;
> +		} else if (display->platform.meteorlake ||
> +			   display->platform.lunarlake) {
> +			return &adl_s_bw_params;

Any reason not to combine this with the ADL-S branch of the if/else
ladder?

> +		} else if (display->platform.pantherlake ||
> +			   display->platform.novalake) {
> +			if (display->platform.pantherlake_wildcatlake)
> +				return &wcl_bw_params;

Can we just flatten this out rather than nesting?

        } else if (display->platform.pantherlake_wildcatlake) {
                return &wcl_bw_params;
        } else if (display->platform.pantherlake ||
                   display->platform.novalake) {
                return &ptl_bw_params;
        }


> +			else
> +				return &ptl_bw_params;
> +		}
> +	}
> +
> +	drm_WARN(display->drm, 1, "Platform-specific bandwidth parameters not found!\n");

I think 

  i915_driver_hw_probe -> intel_bw_init_hw -> get_soc_bw_params

is called unconditionally on all platforms for i915, not just the recent
ones where we started caring about memory bandwidth, so I'm not sure if
this WARN is appropriate since we'll always hit it on the pre-gen11
stuff.

Since the values populated here only get used when paired with display
IP version 11 or later, we should probably add that as a condition since
those are the only cases where it matters that we found a set of SoC
parameters.


Matt

> +
> +	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 +532,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 +562,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 +584,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 +621,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 +666,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 +728,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 +743,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,6 +859,7 @@ 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 intel_soc_bw_params *soc_bw_params = get_soc_bw_params(display);
>  
>  	if (!HAS_DISPLAY(display))
>  		return;
> @@ -807,28 +875,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
> 

-- 
Matt Roper
Graphics Software Engineer
Linux GPU Platform Enablement
Intel Corporation

Re: [PATCH v2 1/4] drm/i915/bw: Extract platform-specific parameters

[Jani Nikula]

On Mon, 11 May 2026, Matt Roper <matthew.d.roper@intel.com> wrote:
> On Mon, May 11, 2026 at 01:30:56PM -0300, Gustavo Sousa wrote:
>> 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)
>> 
>> 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 | 161 ++++++++++++++++++++++----------
>>  1 file changed, 113 insertions(+), 48 deletions(-)
>> 
>> diff --git a/drivers/gpu/drm/i915/display/intel_bw.c b/drivers/gpu/drm/i915/display/intel_bw.c
>> index 9c3a9bbb49f6..cf6756b8ae52 100644
>> --- a/drivers/gpu/drm/i915/display/intel_bw.c
>> +++ b/drivers/gpu/drm/i915/display/intel_bw.c
>> @@ -372,81 +372,147 @@ 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) {
>> +			return &adl_s_bw_params;
>> +		} else if (display->platform.alderlake_p) {
>> +			return &adl_p_bw_params;
>> +		} else if (display->platform.meteorlake ||
>> +			   display->platform.lunarlake) {
>> +			return &adl_s_bw_params;
>
> Any reason not to combine this with the ADL-S branch of the if/else
> ladder?
>
>> +		} else if (display->platform.pantherlake ||
>> +			   display->platform.novalake) {
>> +			if (display->platform.pantherlake_wildcatlake)
>> +				return &wcl_bw_params;
>
> Can we just flatten this out rather than nesting?
>
>         } else if (display->platform.pantherlake_wildcatlake) {
>                 return &wcl_bw_params;
>         } else if (display->platform.pantherlake ||
>                    display->platform.novalake) {
>                 return &ptl_bw_params;
>         }
>
>
>> +			else
>> +				return &ptl_bw_params;
>> +		}
>> +	}
>> +
>> +	drm_WARN(display->drm, 1, "Platform-specific bandwidth parameters not found!\n");
>
> I think 
>
>   i915_driver_hw_probe -> intel_bw_init_hw -> get_soc_bw_params
>
> is called unconditionally on all platforms for i915, not just the recent
> ones where we started caring about memory bandwidth, so I'm not sure if
> this WARN is appropriate since we'll always hit it on the pre-gen11
> stuff.
>
> Since the values populated here only get used when paired with display
> IP version 11 or later, we should probably add that as a condition since
> those are the only cases where it matters that we found a set of SoC
> parameters.

In general, we should stop calling low level display init functions from
i915 core code. Can we move the init call to some display init function?

That said, it'll still get called on all sorts of platforms.

BR,
Jani.


>
>
> Matt
>
>> +
>> +	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 +532,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 +562,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 +584,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 +621,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 +666,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 +728,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 +743,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,6 +859,7 @@ 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 intel_soc_bw_params *soc_bw_params = get_soc_bw_params(display);
>>  
>>  	if (!HAS_DISPLAY(display))
>>  		return;
>> @@ -807,28 +875,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 v2 1/4] drm/i915/bw: Extract platform-specific parameters

[Gustavo Sousa]

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

> On Mon, 11 May 2026, Matt Roper <matthew.d.roper@intel.com> wrote:
>> On Mon, May 11, 2026 at 01:30:56PM -0300, Gustavo Sousa wrote:
>>> 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)
>>> 
>>> 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 | 161 ++++++++++++++++++++++----------
>>>  1 file changed, 113 insertions(+), 48 deletions(-)
>>> 
>>> diff --git a/drivers/gpu/drm/i915/display/intel_bw.c b/drivers/gpu/drm/i915/display/intel_bw.c
>>> index 9c3a9bbb49f6..cf6756b8ae52 100644
>>> --- a/drivers/gpu/drm/i915/display/intel_bw.c
>>> +++ b/drivers/gpu/drm/i915/display/intel_bw.c
>>> @@ -372,81 +372,147 @@ 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) {
>>> +			return &adl_s_bw_params;
>>> +		} else if (display->platform.alderlake_p) {
>>> +			return &adl_p_bw_params;
>>> +		} else if (display->platform.meteorlake ||
>>> +			   display->platform.lunarlake) {
>>> +			return &adl_s_bw_params;
>>
>> Any reason not to combine this with the ADL-S branch of the if/else
>> ladder?
>>
>>> +		} else if (display->platform.pantherlake ||
>>> +			   display->platform.novalake) {
>>> +			if (display->platform.pantherlake_wildcatlake)
>>> +				return &wcl_bw_params;
>>
>> Can we just flatten this out rather than nesting?
>>
>>         } else if (display->platform.pantherlake_wildcatlake) {
>>                 return &wcl_bw_params;
>>         } else if (display->platform.pantherlake ||
>>                    display->platform.novalake) {
>>                 return &ptl_bw_params;
>>         }
>>
>>
>>> +			else
>>> +				return &ptl_bw_params;
>>> +		}
>>> +	}
>>> +
>>> +	drm_WARN(display->drm, 1, "Platform-specific bandwidth parameters not found!\n");
>>
>> I think 
>>
>>   i915_driver_hw_probe -> intel_bw_init_hw -> get_soc_bw_params
>>
>> is called unconditionally on all platforms for i915, not just the recent
>> ones where we started caring about memory bandwidth, so I'm not sure if
>> this WARN is appropriate since we'll always hit it on the pre-gen11
>> stuff.
>>
>> Since the values populated here only get used when paired with display
>> IP version 11 or later, we should probably add that as a condition since
>> those are the only cases where it matters that we found a set of SoC
>> parameters.
>
> In general, we should stop calling low level display init functions from
> i915 core code. Can we move the init call to some display init function?

I guess we could try to move the calls to intel_dram_detect() and
intel_bw_init_hw() into intel_display_driver_probe_noirq()?

--
Gustavo Sousa

>
> That said, it'll still get called on all sorts of platforms.
>
> BR,
> Jani.
>
>
>>
>>
>> Matt
>>
>>> +
>>> +	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 +532,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 +562,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 +584,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 +621,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 +666,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 +728,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 +743,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,6 +859,7 @@ 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 intel_soc_bw_params *soc_bw_params = get_soc_bw_params(display);
>>>  
>>>  	if (!HAS_DISPLAY(display))
>>>  		return;
>>> @@ -807,28 +875,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 v2 1/4] drm/i915/bw: Extract platform-specific parameters

[Gustavo Sousa]

Matt Roper <matthew.d.roper@intel.com> writes:

> On Mon, May 11, 2026 at 01:30:56PM -0300, Gustavo Sousa wrote:
>> 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)
>> 
>> 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 | 161 ++++++++++++++++++++++----------
>>  1 file changed, 113 insertions(+), 48 deletions(-)
>> 
>> diff --git a/drivers/gpu/drm/i915/display/intel_bw.c b/drivers/gpu/drm/i915/display/intel_bw.c
>> index 9c3a9bbb49f6..cf6756b8ae52 100644
>> --- a/drivers/gpu/drm/i915/display/intel_bw.c
>> +++ b/drivers/gpu/drm/i915/display/intel_bw.c
>> @@ -372,81 +372,147 @@ 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) {
>> +			return &adl_s_bw_params;
>> +		} else if (display->platform.alderlake_p) {
>> +			return &adl_p_bw_params;
>> +		} else if (display->platform.meteorlake ||
>> +			   display->platform.lunarlake) {
>> +			return &adl_s_bw_params;
>
> Any reason not to combine this with the ADL-S branch of the if/else
> ladder?

I was trying to follow platforms in chronological order (not sure if I
got it completely right, though).  But, yeah, maybe just better to group
by bw params instances -- differently from IP version checks, there
isn't much benefit in sorting by platforms anyway, at least not in this
case.

>
>> +		} else if (display->platform.pantherlake ||
>> +			   display->platform.novalake) {
>> +			if (display->platform.pantherlake_wildcatlake)
>> +				return &wcl_bw_params;
>
> Can we just flatten this out rather than nesting?
>
>         } else if (display->platform.pantherlake_wildcatlake) {
>                 return &wcl_bw_params;
>         } else if (display->platform.pantherlake ||
>                    display->platform.novalake) {
>                 return &ptl_bw_params;
>         }

We can.

>
>
>> +			else
>> +				return &ptl_bw_params;
>> +		}
>> +	}
>> +
>> +	drm_WARN(display->drm, 1, "Platform-specific bandwidth parameters not found!\n");
>
> I think 
>
>   i915_driver_hw_probe -> intel_bw_init_hw -> get_soc_bw_params
>
> is called unconditionally on all platforms for i915, not just the recent
> ones where we started caring about memory bandwidth, so I'm not sure if
> this WARN is appropriate since we'll always hit it on the pre-gen11
> stuff.

Yep. My bad: I totally missed that this will end up getting called for
pre-gen11; and CI results made that very clear! :-)

>
> Since the values populated here only get used when paired with display
> IP version 11 or later, we should probably add that as a condition since
> those are the only cases where it matters that we found a set of SoC
> parameters.

Maybe we should just bail out from intel_bw_init_hw() if display version
is less than 11? I think that's probably cleaner and we can keep the
current get_soc_bw_params() as is.

Thoughts?

--
Gustavo Sousa

>
>
> Matt
>
>> +
>> +	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 +532,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 +562,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 +584,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 +621,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 +666,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 +728,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 +743,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,6 +859,7 @@ 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 intel_soc_bw_params *soc_bw_params = get_soc_bw_params(display);
>>  
>>  	if (!HAS_DISPLAY(display))
>>  		return;
>> @@ -807,28 +875,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
>> 
>
> -- 
> Matt Roper
> Graphics Software Engineer
> Linux GPU Platform Enablement
> Intel Corporation

[PATCH v2 2/4] 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.

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 | 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 cf6756b8ae52..2e580c1a3fab 100644
--- a/drivers/gpu/drm/i915/display/intel_bw.c
+++ b/drivers/gpu/drm/i915/display/intel_bw.c
@@ -480,36 +480,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,
@@ -873,25 +848,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

Re: [PATCH v2 2/4] drm/i915/bw: Deduplicate intel_sa_info instances

[Matt Roper]

On Mon, May 11, 2026 at 01:30:57PM -0300, Gustavo Sousa wrote:
> 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.
> 
> Cc: Matt Roper <matthew.d.roper@intel.com>
> Signed-off-by: Gustavo Sousa <gustavo.sousa@intel.com>

Reviewed-by: Matt Roper <matthew.d.roper@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 cf6756b8ae52..2e580c1a3fab 100644
> --- a/drivers/gpu/drm/i915/display/intel_bw.c
> +++ b/drivers/gpu/drm/i915/display/intel_bw.c
> @@ -480,36 +480,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,
> @@ -873,25 +848,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
> 

-- 
Matt Roper
Graphics Software Engineer
Linux GPU Platform Enablement
Intel Corporation

[PATCH v2 3/4] 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>
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 | 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 2e580c1a3fab..c01356d38e64 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;
@@ -465,22 +469,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,
 };
@@ -488,7 +496,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 */
@@ -508,7 +516,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++) {
@@ -516,7 +524,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;
@@ -560,7 +568,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 */
@@ -598,7 +606,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
@@ -611,7 +619,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];
@@ -851,7 +859,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) {
@@ -862,11 +870,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

Re: [PATCH v2 3/4] drm/i915/bw: Rename struct intel_sa_info to intel_display_bw_params

[Matt Roper]

On Mon, May 11, 2026 at 01:30:58PM -0300, Gustavo Sousa wrote:
> 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>
> Cc: Matt Roper <matthew.d.roper@intel.com>
> Signed-off-by: Gustavo Sousa <gustavo.sousa@intel.com>

Reviewed-by: Matt Roper <matthew.d.roper@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 2e580c1a3fab..c01356d38e64 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;
> @@ -465,22 +469,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,
>  };
> @@ -488,7 +496,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 */
> @@ -508,7 +516,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++) {
> @@ -516,7 +524,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;
> @@ -560,7 +568,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 */
> @@ -598,7 +606,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
> @@ -611,7 +619,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];
> @@ -851,7 +859,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) {
> @@ -862,11 +870,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
> 

-- 
Matt Roper
Graphics Software Engineer
Linux GPU Platform Enablement
Intel Corporation

[PATCH v2 4/4] 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)

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

diff --git a/drivers/gpu/drm/i915/display/intel_bw.c b/drivers/gpu/drm/i915/display/intel_bw.c
index c01356d38e64..acd1b6901b46 100644
--- a/drivers/gpu/drm/i915/display/intel_bw.c
+++ b/drivers/gpu/drm/i915/display/intel_bw.c
@@ -493,6 +493,26 @@ 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 {
+		return &gen11_bw_params;
+	}
+}
+
 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,
@@ -843,6 +863,7 @@ void intel_bw_init_hw(struct intel_display *display)
 {
 	const struct dram_info *dram_info = intel_dram_info(display);
 	const struct intel_soc_bw_params *soc_bw_params = get_soc_bw_params(display);
+	const struct intel_display_bw_params *display_bw_params = get_display_bw_params(display);
 
 	if (!HAS_DISPLAY(display))
 		return;
@@ -858,23 +879,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

Re: [PATCH v2 4/4] drm/i915/bw: Extract get_display_bw_params()

[Matt Roper]

On Mon, May 11, 2026 at 01:30:59PM -0300, Gustavo Sousa wrote:
> 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)
> 
> Cc: Jani Nikula <jani.nikula@intel.com>
> Signed-off-by: Gustavo Sousa <gustavo.sousa@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_bw.c | 36 +++++++++++++++++++++------------
>  1 file changed, 23 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_bw.c b/drivers/gpu/drm/i915/display/intel_bw.c
> index c01356d38e64..acd1b6901b46 100644
> --- a/drivers/gpu/drm/i915/display/intel_bw.c
> +++ b/drivers/gpu/drm/i915/display/intel_bw.c
> @@ -493,6 +493,26 @@ 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 {
> +		return &gen11_bw_params;

It doesn't really matter, but this is technically going to assign gen11
parameters for all the pre-gen11 platforms that call through here on
i915.  If we never use the values it probably doesn't hurt anything, but
it might be best to make this a condition on gen11 rather than an 'else'
just to avoid any confusion.


Matt

> +	}
> +}
> +
>  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,
> @@ -843,6 +863,7 @@ void intel_bw_init_hw(struct intel_display *display)
>  {
>  	const struct dram_info *dram_info = intel_dram_info(display);
>  	const struct intel_soc_bw_params *soc_bw_params = get_soc_bw_params(display);
> +	const struct intel_display_bw_params *display_bw_params = get_display_bw_params(display);
>  
>  	if (!HAS_DISPLAY(display))
>  		return;
> @@ -858,23 +879,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
> 

-- 
Matt Roper
Graphics Software Engineer
Linux GPU Platform Enablement
Intel Corporation

Re: [PATCH v2 4/4] drm/i915/bw: Extract get_display_bw_params()

[Jani Nikula]

On Mon, 11 May 2026, Matt Roper <matthew.d.roper@intel.com> wrote:
> On Mon, May 11, 2026 at 01:30:59PM -0300, Gustavo Sousa wrote:
>> 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)
>> 
>> Cc: Jani Nikula <jani.nikula@intel.com>
>> Signed-off-by: Gustavo Sousa <gustavo.sousa@intel.com>
>> ---
>>  drivers/gpu/drm/i915/display/intel_bw.c | 36 +++++++++++++++++++++------------
>>  1 file changed, 23 insertions(+), 13 deletions(-)
>> 
>> diff --git a/drivers/gpu/drm/i915/display/intel_bw.c b/drivers/gpu/drm/i915/display/intel_bw.c
>> index c01356d38e64..acd1b6901b46 100644
>> --- a/drivers/gpu/drm/i915/display/intel_bw.c
>> +++ b/drivers/gpu/drm/i915/display/intel_bw.c
>> @@ -493,6 +493,26 @@ 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 {
>> +		return &gen11_bw_params;
>
> It doesn't really matter, but this is technically going to assign gen11
> parameters for all the pre-gen11 platforms that call through here on
> i915.  If we never use the values it probably doesn't hurt anything, but
> it might be best to make this a condition on gen11 rather than an 'else'
> just to avoid any confusion.
>
>
> Matt
>
>> +	}
>> +}
>> +
>>  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,
>> @@ -843,6 +863,7 @@ void intel_bw_init_hw(struct intel_display *display)
>>  {
>>  	const struct dram_info *dram_info = intel_dram_info(display);
>>  	const struct intel_soc_bw_params *soc_bw_params = get_soc_bw_params(display);
>> +	const struct intel_display_bw_params *display_bw_params = get_display_bw_params(display);

Feels like it gets increasingly weird to call all these functions
unconditionally when we bail out for !display right below.

BR,
Jani.

>>  
>>  	if (!HAS_DISPLAY(display))
>>  		return;
>> @@ -858,23 +879,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
>> 

-- 
Jani Nikula, Intel

Re: [PATCH v2 4/4] drm/i915/bw: Extract get_display_bw_params()

[Gustavo Sousa]

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

> On Mon, 11 May 2026, Matt Roper <matthew.d.roper@intel.com> wrote:
>> On Mon, May 11, 2026 at 01:30:59PM -0300, Gustavo Sousa wrote:
>>> 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)
>>> 
>>> Cc: Jani Nikula <jani.nikula@intel.com>
>>> Signed-off-by: Gustavo Sousa <gustavo.sousa@intel.com>
>>> ---
>>>  drivers/gpu/drm/i915/display/intel_bw.c | 36 +++++++++++++++++++++------------
>>>  1 file changed, 23 insertions(+), 13 deletions(-)
>>> 
>>> diff --git a/drivers/gpu/drm/i915/display/intel_bw.c b/drivers/gpu/drm/i915/display/intel_bw.c
>>> index c01356d38e64..acd1b6901b46 100644
>>> --- a/drivers/gpu/drm/i915/display/intel_bw.c
>>> +++ b/drivers/gpu/drm/i915/display/intel_bw.c
>>> @@ -493,6 +493,26 @@ 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 {
>>> +		return &gen11_bw_params;
>>
>> It doesn't really matter, but this is technically going to assign gen11
>> parameters for all the pre-gen11 platforms that call through here on
>> i915.  If we never use the values it probably doesn't hurt anything, but
>> it might be best to make this a condition on gen11 rather than an 'else'
>> just to avoid any confusion.
>>
>>
>> Matt
>>
>>> +	}
>>> +}
>>> +
>>>  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,
>>> @@ -843,6 +863,7 @@ void intel_bw_init_hw(struct intel_display *display)
>>>  {
>>>  	const struct dram_info *dram_info = intel_dram_info(display);
>>>  	const struct intel_soc_bw_params *soc_bw_params = get_soc_bw_params(display);
>>> +	const struct intel_display_bw_params *display_bw_params = get_display_bw_params(display);
>
> Feels like it gets increasingly weird to call all these functions
> unconditionally when we bail out for !display right below.

Yeah.  Let's fix that. I'll have a separate patch for moving
intel_dram_info() down and then fix the two patches that add the other
calls.

--
Gustavo Sousa

>
> BR,
> Jani.
>
>>>  
>>>  	if (!HAS_DISPLAY(display))
>>>  		return;
>>> @@ -858,23 +879,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
>>> 
>
> -- 
> Jani Nikula, Intel

Re: [PATCH v2 4/4] drm/i915/bw: Extract get_display_bw_params()

[Gustavo Sousa]

Matt Roper <matthew.d.roper@intel.com> writes:

> On Mon, May 11, 2026 at 01:30:59PM -0300, Gustavo Sousa wrote:
>> 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)
>> 
>> Cc: Jani Nikula <jani.nikula@intel.com>
>> Signed-off-by: Gustavo Sousa <gustavo.sousa@intel.com>
>> ---
>>  drivers/gpu/drm/i915/display/intel_bw.c | 36 +++++++++++++++++++++------------
>>  1 file changed, 23 insertions(+), 13 deletions(-)
>> 
>> diff --git a/drivers/gpu/drm/i915/display/intel_bw.c b/drivers/gpu/drm/i915/display/intel_bw.c
>> index c01356d38e64..acd1b6901b46 100644
>> --- a/drivers/gpu/drm/i915/display/intel_bw.c
>> +++ b/drivers/gpu/drm/i915/display/intel_bw.c
>> @@ -493,6 +493,26 @@ 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 {
>> +		return &gen11_bw_params;
>
> It doesn't really matter, but this is technically going to assign gen11
> parameters for all the pre-gen11 platforms that call through here on
> i915.  If we never use the values it probably doesn't hurt anything, but
> it might be best to make this a condition on gen11 rather than an 'else'
> just to avoid any confusion.

Sounds good.  I'll add a check for display version == 11 and then take
the same approach as for get_soc_bw_params(), by returning NULL with a
warning.

--
Gustavo Sousa

>
>
> Matt
>
>> +	}
>> +}
>> +
>>  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,
>> @@ -843,6 +863,7 @@ void intel_bw_init_hw(struct intel_display *display)
>>  {
>>  	const struct dram_info *dram_info = intel_dram_info(display);
>>  	const struct intel_soc_bw_params *soc_bw_params = get_soc_bw_params(display);
>> +	const struct intel_display_bw_params *display_bw_params = get_display_bw_params(display);
>>  
>>  	if (!HAS_DISPLAY(display))
>>  		return;
>> @@ -858,23 +879,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
>> 
>
> -- 
> Matt Roper
> Graphics Software Engineer
> Linux GPU Platform Enablement
> Intel Corporation

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

[Patchwork]

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

== Series Details ==

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

== Summary ==

CI Bug Log - changes from CI_DRM_18467 -> Patchwork_164567v2
====================================================

Summary
-------

  **FAILURE**

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

### IGT changes ###

#### Possible regressions ####

  * igt@i915_module_load@load:
    - fi-ilk-650:         [PASS][1] -> [ABORT][2]
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_18467/fi-ilk-650/igt@i915_module_load@load.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_164567v2/fi-ilk-650/igt@i915_module_load@load.html
    - fi-bsw-n3050:       [PASS][3] -> [ABORT][4]
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_18467/fi-bsw-n3050/igt@i915_module_load@load.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_164567v2/fi-bsw-n3050/igt@i915_module_load@load.html
    - fi-skl-6600u:       [PASS][5] -> [ABORT][6]
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_18467/fi-skl-6600u/igt@i915_module_load@load.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_164567v2/fi-skl-6600u/igt@i915_module_load@load.html
    - fi-pnv-d510:        [PASS][7] -> [ABORT][8]
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_18467/fi-pnv-d510/igt@i915_module_load@load.html
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_164567v2/fi-pnv-d510/igt@i915_module_load@load.html
    - fi-glk-j4005:       [PASS][9] -> [ABORT][10]
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_18467/fi-glk-j4005/igt@i915_module_load@load.html
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_164567v2/fi-glk-j4005/igt@i915_module_load@load.html
    - fi-kbl-7567u:       [PASS][11] -> [ABORT][12]
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_18467/fi-kbl-7567u/igt@i915_module_load@load.html
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_164567v2/fi-kbl-7567u/igt@i915_module_load@load.html
    - fi-cfl-8700k:       [PASS][13] -> [ABORT][14]
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_18467/fi-cfl-8700k/igt@i915_module_load@load.html
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_164567v2/fi-cfl-8700k/igt@i915_module_load@load.html
    - fi-kbl-8809g:       [PASS][15] -> [ABORT][16]
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_18467/fi-kbl-8809g/igt@i915_module_load@load.html
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_164567v2/fi-kbl-8809g/igt@i915_module_load@load.html
    - bat-apl-1:          [PASS][17] -> [ABORT][18]
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_18467/bat-apl-1/igt@i915_module_load@load.html
   [18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_164567v2/bat-apl-1/igt@i915_module_load@load.html
    - bat-dg2-14:         [PASS][19] -> [ABORT][20]
   [19]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_18467/bat-dg2-14/igt@i915_module_load@load.html
   [20]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_164567v2/bat-dg2-14/igt@i915_module_load@load.html
    - fi-elk-e7500:       [PASS][21] -> [ABORT][22]
   [21]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_18467/fi-elk-e7500/igt@i915_module_load@load.html
   [22]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_164567v2/fi-elk-e7500/igt@i915_module_load@load.html
    - fi-bsw-nick:        [PASS][23] -> [ABORT][24]
   [23]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_18467/fi-bsw-nick/igt@i915_module_load@load.html
   [24]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_164567v2/fi-bsw-nick/igt@i915_module_load@load.html
    - bat-kbl-2:          [PASS][25] -> [ABORT][26]
   [25]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_18467/bat-kbl-2/igt@i915_module_load@load.html
   [26]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_164567v2/bat-kbl-2/igt@i915_module_load@load.html
    - bat-atsm-1:         [PASS][27] -> [ABORT][28]
   [27]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_18467/bat-atsm-1/igt@i915_module_load@load.html
   [28]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_164567v2/bat-atsm-1/igt@i915_module_load@load.html
    - fi-cfl-guc:         [PASS][29] -> [ABORT][30]
   [29]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_18467/fi-cfl-guc/igt@i915_module_load@load.html
   [30]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_164567v2/fi-cfl-guc/igt@i915_module_load@load.html
    - bat-dg2-9:          [PASS][31] -> [ABORT][32]
   [31]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_18467/bat-dg2-9/igt@i915_module_load@load.html
   [32]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_164567v2/bat-dg2-9/igt@i915_module_load@load.html
    - fi-kbl-x1275:       [PASS][33] -> [ABORT][34]
   [33]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_18467/fi-kbl-x1275/igt@i915_module_load@load.html
   [34]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_164567v2/fi-kbl-x1275/igt@i915_module_load@load.html
    - fi-hsw-4770:        [PASS][35] -> [ABORT][36]
   [35]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_18467/fi-hsw-4770/igt@i915_module_load@load.html
   [36]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_164567v2/fi-hsw-4770/igt@i915_module_load@load.html
    - fi-cfl-8109u:       [PASS][37] -> [ABORT][38]
   [37]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_18467/fi-cfl-8109u/igt@i915_module_load@load.html
   [38]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_164567v2/fi-cfl-8109u/igt@i915_module_load@load.html
    - fi-ivb-3770:        [PASS][39] -> [ABORT][40]
   [39]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_18467/fi-ivb-3770/igt@i915_module_load@load.html
   [40]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_164567v2/fi-ivb-3770/igt@i915_module_load@load.html
    - bat-dg2-8:          [PASS][41] -> [ABORT][42]
   [41]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_18467/bat-dg2-8/igt@i915_module_load@load.html
   [42]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_164567v2/bat-dg2-8/igt@i915_module_load@load.html

  


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

  * Linux: CI_DRM_18467 -> Patchwork_164567v2

  CI-20190529: 20190529
  CI_DRM_18467: f8ee23694aa6be213355905a78f79bb1b0861565 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_8902: d28bd0b9e0347c58ca9b012c02de7e2ad5ffe847 @ https://gitlab.freedesktop.org/drm/igt-gpu-tools.git
  Patchwork_164567v2: f8ee23694aa6be213355905a78f79bb1b0861565 @ git://anongit.freedesktop.org/gfx-ci/linux

== Logs ==

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

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