* [PATCH v2 0/4] drm/i915/bw: Split bandwidth params into platform- and display-IP-specific structs
@ 2026-05-11 16:30 Gustavo Sousa
2026-05-11 16:30 ` [PATCH v2 1/4] drm/i915/bw: Extract platform-specific parameters Gustavo Sousa
` (4 more replies)
0 siblings, 5 replies; 16+ messages in thread
From: Gustavo Sousa @ 2026-05-11 16:30 UTC (permalink / raw)
To: intel-gfx, intel-xe; +Cc: Gustavo Sousa, Jani Nikula, Matt Roper, Rodrigo Vivi
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>
^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH v2 1/4] drm/i915/bw: Extract platform-specific parameters
2026-05-11 16:30 [PATCH v2 0/4] drm/i915/bw: Split bandwidth params into platform- and display-IP-specific structs Gustavo Sousa
@ 2026-05-11 16:30 ` Gustavo Sousa
2026-05-11 22:38 ` Matt Roper
2026-05-11 16:30 ` [PATCH v2 2/4] drm/i915/bw: Deduplicate intel_sa_info instances Gustavo Sousa
` (3 subsequent siblings)
4 siblings, 1 reply; 16+ messages in thread
From: Gustavo Sousa @ 2026-05-11 16:30 UTC (permalink / raw)
To: intel-gfx, intel-xe; +Cc: Gustavo Sousa, Jani Nikula, Matt Roper, Rodrigo Vivi
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
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH v2 2/4] drm/i915/bw: Deduplicate intel_sa_info instances
2026-05-11 16:30 [PATCH v2 0/4] drm/i915/bw: Split bandwidth params into platform- and display-IP-specific structs Gustavo Sousa
2026-05-11 16:30 ` [PATCH v2 1/4] drm/i915/bw: Extract platform-specific parameters Gustavo Sousa
@ 2026-05-11 16:30 ` Gustavo Sousa
2026-05-11 22:43 ` Matt Roper
2026-05-11 16:30 ` [PATCH v2 3/4] drm/i915/bw: Rename struct intel_sa_info to intel_display_bw_params Gustavo Sousa
` (2 subsequent siblings)
4 siblings, 1 reply; 16+ messages in thread
From: Gustavo Sousa @ 2026-05-11 16:30 UTC (permalink / raw)
To: intel-gfx, intel-xe; +Cc: Gustavo Sousa, Matt Roper
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
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH v2 3/4] drm/i915/bw: Rename struct intel_sa_info to intel_display_bw_params
2026-05-11 16:30 [PATCH v2 0/4] drm/i915/bw: Split bandwidth params into platform- and display-IP-specific structs Gustavo Sousa
2026-05-11 16:30 ` [PATCH v2 1/4] drm/i915/bw: Extract platform-specific parameters Gustavo Sousa
2026-05-11 16:30 ` [PATCH v2 2/4] drm/i915/bw: Deduplicate intel_sa_info instances Gustavo Sousa
@ 2026-05-11 16:30 ` Gustavo Sousa
2026-05-11 22:44 ` Matt Roper
2026-05-11 16:30 ` [PATCH v2 4/4] drm/i915/bw: Extract get_display_bw_params() Gustavo Sousa
2026-05-11 20:35 ` ✗ i915.CI.BAT: failure for drm/i915/bw: Split bandwidth params into platform- and display-IP-specific structs (rev2) Patchwork
4 siblings, 1 reply; 16+ messages in thread
From: Gustavo Sousa @ 2026-05-11 16:30 UTC (permalink / raw)
To: intel-gfx, intel-xe; +Cc: Gustavo Sousa, Jani Nikula, Matt Roper
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
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH v2 4/4] drm/i915/bw: Extract get_display_bw_params()
2026-05-11 16:30 [PATCH v2 0/4] drm/i915/bw: Split bandwidth params into platform- and display-IP-specific structs Gustavo Sousa
` (2 preceding siblings ...)
2026-05-11 16:30 ` [PATCH v2 3/4] drm/i915/bw: Rename struct intel_sa_info to intel_display_bw_params Gustavo Sousa
@ 2026-05-11 16:30 ` Gustavo Sousa
2026-05-11 22:49 ` Matt Roper
2026-05-11 20:35 ` ✗ i915.CI.BAT: failure for drm/i915/bw: Split bandwidth params into platform- and display-IP-specific structs (rev2) Patchwork
4 siblings, 1 reply; 16+ messages in thread
From: Gustavo Sousa @ 2026-05-11 16:30 UTC (permalink / raw)
To: intel-gfx, intel-xe; +Cc: Gustavo Sousa, Jani Nikula
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
^ permalink raw reply related [flat|nested] 16+ messages in thread
* ✗ i915.CI.BAT: failure for drm/i915/bw: Split bandwidth params into platform- and display-IP-specific structs (rev2)
2026-05-11 16:30 [PATCH v2 0/4] drm/i915/bw: Split bandwidth params into platform- and display-IP-specific structs Gustavo Sousa
` (3 preceding siblings ...)
2026-05-11 16:30 ` [PATCH v2 4/4] drm/i915/bw: Extract get_display_bw_params() Gustavo Sousa
@ 2026-05-11 20:35 ` Patchwork
4 siblings, 0 replies; 16+ messages in thread
From: Patchwork @ 2026-05-11 20:35 UTC (permalink / raw)
To: Gustavo Sousa; +Cc: intel-gfx
[-- 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 --]
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v2 1/4] drm/i915/bw: Extract platform-specific parameters
2026-05-11 16:30 ` [PATCH v2 1/4] drm/i915/bw: Extract platform-specific parameters Gustavo Sousa
@ 2026-05-11 22:38 ` Matt Roper
2026-05-12 8:18 ` Jani Nikula
2026-05-12 13:05 ` Gustavo Sousa
0 siblings, 2 replies; 16+ messages in thread
From: Matt Roper @ 2026-05-11 22:38 UTC (permalink / raw)
To: Gustavo Sousa; +Cc: intel-gfx, intel-xe, Jani Nikula, Rodrigo Vivi
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
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v2 2/4] drm/i915/bw: Deduplicate intel_sa_info instances
2026-05-11 16:30 ` [PATCH v2 2/4] drm/i915/bw: Deduplicate intel_sa_info instances Gustavo Sousa
@ 2026-05-11 22:43 ` Matt Roper
0 siblings, 0 replies; 16+ messages in thread
From: Matt Roper @ 2026-05-11 22:43 UTC (permalink / raw)
To: Gustavo Sousa; +Cc: intel-gfx, intel-xe
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
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v2 3/4] drm/i915/bw: Rename struct intel_sa_info to intel_display_bw_params
2026-05-11 16:30 ` [PATCH v2 3/4] drm/i915/bw: Rename struct intel_sa_info to intel_display_bw_params Gustavo Sousa
@ 2026-05-11 22:44 ` Matt Roper
0 siblings, 0 replies; 16+ messages in thread
From: Matt Roper @ 2026-05-11 22:44 UTC (permalink / raw)
To: Gustavo Sousa; +Cc: intel-gfx, intel-xe, Jani Nikula
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
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v2 4/4] drm/i915/bw: Extract get_display_bw_params()
2026-05-11 16:30 ` [PATCH v2 4/4] drm/i915/bw: Extract get_display_bw_params() Gustavo Sousa
@ 2026-05-11 22:49 ` Matt Roper
2026-05-12 8:22 ` Jani Nikula
2026-05-12 13:30 ` Gustavo Sousa
0 siblings, 2 replies; 16+ messages in thread
From: Matt Roper @ 2026-05-11 22:49 UTC (permalink / raw)
To: Gustavo Sousa; +Cc: intel-gfx, intel-xe, Jani Nikula
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
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v2 1/4] drm/i915/bw: Extract platform-specific parameters
2026-05-11 22:38 ` Matt Roper
@ 2026-05-12 8:18 ` Jani Nikula
2026-05-12 13:14 ` Gustavo Sousa
2026-05-12 13:05 ` Gustavo Sousa
1 sibling, 1 reply; 16+ messages in thread
From: Jani Nikula @ 2026-05-12 8:18 UTC (permalink / raw)
To: Matt Roper, Gustavo Sousa; +Cc: intel-gfx, intel-xe, Rodrigo Vivi
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
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v2 4/4] drm/i915/bw: Extract get_display_bw_params()
2026-05-11 22:49 ` Matt Roper
@ 2026-05-12 8:22 ` Jani Nikula
2026-05-12 13:33 ` Gustavo Sousa
2026-05-12 13:30 ` Gustavo Sousa
1 sibling, 1 reply; 16+ messages in thread
From: Jani Nikula @ 2026-05-12 8:22 UTC (permalink / raw)
To: Matt Roper, Gustavo Sousa; +Cc: intel-gfx, intel-xe
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
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v2 1/4] drm/i915/bw: Extract platform-specific parameters
2026-05-11 22:38 ` Matt Roper
2026-05-12 8:18 ` Jani Nikula
@ 2026-05-12 13:05 ` Gustavo Sousa
1 sibling, 0 replies; 16+ messages in thread
From: Gustavo Sousa @ 2026-05-12 13:05 UTC (permalink / raw)
To: Matt Roper; +Cc: intel-gfx, intel-xe, Jani Nikula, Rodrigo Vivi
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
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v2 1/4] drm/i915/bw: Extract platform-specific parameters
2026-05-12 8:18 ` Jani Nikula
@ 2026-05-12 13:14 ` Gustavo Sousa
0 siblings, 0 replies; 16+ messages in thread
From: Gustavo Sousa @ 2026-05-12 13:14 UTC (permalink / raw)
To: Jani Nikula, Matt Roper; +Cc: intel-gfx, intel-xe, Rodrigo Vivi
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
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v2 4/4] drm/i915/bw: Extract get_display_bw_params()
2026-05-11 22:49 ` Matt Roper
2026-05-12 8:22 ` Jani Nikula
@ 2026-05-12 13:30 ` Gustavo Sousa
1 sibling, 0 replies; 16+ messages in thread
From: Gustavo Sousa @ 2026-05-12 13:30 UTC (permalink / raw)
To: Matt Roper; +Cc: intel-gfx, intel-xe, Jani Nikula
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
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v2 4/4] drm/i915/bw: Extract get_display_bw_params()
2026-05-12 8:22 ` Jani Nikula
@ 2026-05-12 13:33 ` Gustavo Sousa
0 siblings, 0 replies; 16+ messages in thread
From: Gustavo Sousa @ 2026-05-12 13:33 UTC (permalink / raw)
To: Jani Nikula, Matt Roper; +Cc: intel-gfx, intel-xe
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
^ permalink raw reply [flat|nested] 16+ messages in thread
end of thread, other threads:[~2026-05-12 13:33 UTC | newest]
Thread overview: 16+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-05-11 16:30 [PATCH v2 0/4] drm/i915/bw: Split bandwidth params into platform- and display-IP-specific structs Gustavo Sousa
2026-05-11 16:30 ` [PATCH v2 1/4] drm/i915/bw: Extract platform-specific parameters Gustavo Sousa
2026-05-11 22:38 ` Matt Roper
2026-05-12 8:18 ` Jani Nikula
2026-05-12 13:14 ` Gustavo Sousa
2026-05-12 13:05 ` Gustavo Sousa
2026-05-11 16:30 ` [PATCH v2 2/4] drm/i915/bw: Deduplicate intel_sa_info instances Gustavo Sousa
2026-05-11 22:43 ` Matt Roper
2026-05-11 16:30 ` [PATCH v2 3/4] drm/i915/bw: Rename struct intel_sa_info to intel_display_bw_params Gustavo Sousa
2026-05-11 22:44 ` Matt Roper
2026-05-11 16:30 ` [PATCH v2 4/4] drm/i915/bw: Extract get_display_bw_params() Gustavo Sousa
2026-05-11 22:49 ` Matt Roper
2026-05-12 8:22 ` Jani Nikula
2026-05-12 13:33 ` Gustavo Sousa
2026-05-12 13:30 ` Gustavo Sousa
2026-05-11 20:35 ` ✗ i915.CI.BAT: failure for drm/i915/bw: Split bandwidth params into platform- and display-IP-specific structs (rev2) Patchwork
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox