[PATCH v2 00/11] drm/i915/display: reduce the pm demand peak bw based on display data rate

[PATCH v2 00/11] drm/i915/display: reduce the pm demand peak bw based on display data rate

[Vinod Govindapillai]

In xe3+, soc can lower the fabric frequency when the display
needs less bandwidth than the minimum GV point.

v1: pmdemand peak bw is updated to 20GB/s is the required data rate
    is low and less than 20GB/s even though there are no QGV point
    with 20GB/s. Only the peak_ww is changed based on the conditions 

v2: At the time of BW info calculations, a new row is inserted with
    peakbw as 20GB/s and deratebw also to 20GB/s and the required
    pmdemand peakbw is automatically calculated based on the old
    logic.    

Vinod Govindapillai (11):
  drm/i915/wm: clear the plane ddb_y entries on plane disable
  drm/i915/pm_demand: introduce HAS_PMDEMAND macro
  drm/i915/display: sagv pre/post plane calls to check pmdemand support
  drm/i915/bw: Extract icl_init_qgv_info()
  drm/i915/bw: sort the qgv points based on the dclk
  drm/i915/bw: update the routine to get max dclk from qgv points
  drm/i915/bw: update the routine to find the peakbw in MTL
  drm/i915/bw: update the tile-y dependency based on the display version
  drm/i915/bw: consolidate the debug info of bw/dgv/psf data
  drm/i915/bw: extract update_sagv_status()
  drm/i915/bw: insert a sw bw info entry to cater low data rate usecases

 drivers/gpu/drm/i915/display/intel_bw.c       | 215 +++++++++++++-----
 .../drm/i915/display/intel_display_device.h   |   2 +
 .../gpu/drm/i915/display/intel_display_irq.c  |   2 +-
 .../drm/i915/display/intel_display_power.c    |   4 +-
 drivers/gpu/drm/i915/display/intel_pmdemand.c |  12 +-
 drivers/gpu/drm/i915/display/skl_watermark.c  |   8 +-
 6 files changed, 178 insertions(+), 65 deletions(-)

-- 
2.43.0

[PATCH v2 01/11] drm/i915/wm: clear the plane ddb_y entries on plane disable

[Vinod Govindapillai]

The UV/Y plane DDB entriess are never cleared on
sk_wm_plane_disable_noatomic() and can leave stale DDB state
for NV12 planes on pre-Gen11 devices

Fixes: d34b59d5ba41 ("drm/i915: Add skl_wm_plane_disable_noatomic()")
Assisted-by: Copilot:claude-sonnet-4.6
Signed-off-by: Vinod Govindapillai <vinod.govindapillai@intel.com>
---
 drivers/gpu/drm/i915/display/skl_watermark.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/display/skl_watermark.c b/drivers/gpu/drm/i915/display/skl_watermark.c
index 4bffa27ce02c..c942ccfe6897 100644
--- a/drivers/gpu/drm/i915/display/skl_watermark.c
+++ b/drivers/gpu/drm/i915/display/skl_watermark.c
@@ -3858,7 +3858,7 @@ void skl_wm_plane_disable_noatomic(struct intel_crtc *crtc,
 		return;
 
 	skl_ddb_entry_init(&crtc_state->wm.skl.plane_ddb[plane->id], 0, 0);
-	skl_ddb_entry_init(&crtc_state->wm.skl.plane_ddb[plane->id], 0, 0);
+	skl_ddb_entry_init(&crtc_state->wm.skl.plane_ddb_y[plane->id], 0, 0);
 
 	crtc_state->wm.skl.plane_min_ddb[plane->id] = 0;
 	crtc_state->wm.skl.plane_interim_ddb[plane->id] = 0;
-- 
2.43.0

[PATCH v2 02/11] drm/i915/pm_demand: introduce HAS_PMDEMAND macro

[Vinod Govindapillai]

PM demand feature introduces a new way to set bw, power and
performance requirements to pcode from display version 14 onwards.
Use an identifiable name as a macro to distinguish the pm demand
specific changes in the code.

Signed-off-by: Vinod Govindapillai <vinod.govindapillai@intel.com>
---
 drivers/gpu/drm/i915/display/intel_bw.c             |  4 ++--
 drivers/gpu/drm/i915/display/intel_display_device.h |  1 +
 drivers/gpu/drm/i915/display/intel_display_irq.c    |  2 +-
 drivers/gpu/drm/i915/display/intel_display_power.c  |  4 ++--
 drivers/gpu/drm/i915/display/intel_pmdemand.c       | 12 ++++++------
 5 files changed, 12 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_bw.c b/drivers/gpu/drm/i915/display/intel_bw.c
index 9c3a9bbb49f6..d99e921df0b9 100644
--- a/drivers/gpu/drm/i915/display/intel_bw.c
+++ b/drivers/gpu/drm/i915/display/intel_bw.c
@@ -181,7 +181,7 @@ static int icl_pcode_restrict_qgv_points(struct intel_display *display,
 {
 	int ret;
 
-	if (DISPLAY_VER(display) >= 14)
+	if (HAS_PMDEMAND(display))
 		return 0;
 
 	/* bspec says to keep retrying for at least 1 ms */
@@ -1206,7 +1206,7 @@ static int intel_bw_check_qgv_points(struct intel_display *display,
 
 	data_rate = DIV_ROUND_UP(data_rate, 1000);
 
-	if (DISPLAY_VER(display) >= 14)
+	if (HAS_PMDEMAND(display))
 		return mtl_find_qgv_points(display, data_rate, num_active_planes,
 					   new_bw_state);
 	else
diff --git a/drivers/gpu/drm/i915/display/intel_display_device.h b/drivers/gpu/drm/i915/display/intel_display_device.h
index 074e3ba8fb77..65283286771a 100644
--- a/drivers/gpu/drm/i915/display/intel_display_device.h
+++ b/drivers/gpu/drm/i915/display/intel_display_device.h
@@ -194,6 +194,7 @@ struct intel_display_platforms {
 #define HAS_OVERLAY(__display)		(DISPLAY_INFO(__display)->has_overlay)
 #define HAS_PIPEDMC(__display)		(DISPLAY_VER(__display) >= 12)
 #define HAS_PIXEL_NORMALIZER(__display)	(DISPLAY_VER(__display) >= 35)
+#define HAS_PMDEMAND(__display)		(DISPLAY_VER(__display) >= 14)
 #define HAS_PSR(__display)		(DISPLAY_INFO(__display)->has_psr)
 #define HAS_PSR_HW_TRACKING(__display)	(DISPLAY_INFO(__display)->has_psr_hw_tracking)
 #define HAS_PSR2_SEL_FETCH(__display)	(DISPLAY_VER(__display) >= 12)
diff --git a/drivers/gpu/drm/i915/display/intel_display_irq.c b/drivers/gpu/drm/i915/display/intel_display_irq.c
index 70c1bba7c0a8..69bc9101d21d 100644
--- a/drivers/gpu/drm/i915/display/intel_display_irq.c
+++ b/drivers/gpu/drm/i915/display/intel_display_irq.c
@@ -1260,7 +1260,7 @@ gen8_de_misc_irq_handler(struct intel_display *display, u32 iir)
 		}
 	}
 
-	if (DISPLAY_VER(display) >= 14) {
+	if (HAS_PMDEMAND(display)) {
 		if (iir & (XELPDP_PMDEMAND_RSP |
 			   XELPDP_PMDEMAND_RSPTOUT_ERR)) {
 			if (iir & XELPDP_PMDEMAND_RSPTOUT_ERR)
diff --git a/drivers/gpu/drm/i915/display/intel_display_power.c b/drivers/gpu/drm/i915/display/intel_display_power.c
index 80ecf373fb19..94e025e231d5 100644
--- a/drivers/gpu/drm/i915/display/intel_display_power.c
+++ b/drivers/gpu/drm/i915/display/intel_display_power.c
@@ -1122,7 +1122,7 @@ static void gen9_dbuf_enable(struct intel_display *display)
 
 	slices_mask = BIT(DBUF_S1) | display->dbuf.enabled_slices;
 
-	if (DISPLAY_VER(display) >= 14)
+	if (HAS_PMDEMAND(display))
 		intel_pmdemand_program_dbuf(display, slices_mask);
 
 	/*
@@ -1136,7 +1136,7 @@ static void gen9_dbuf_disable(struct intel_display *display)
 {
 	gen9_dbuf_slices_update(display, 0);
 
-	if (DISPLAY_VER(display) >= 14)
+	if (HAS_PMDEMAND(display))
 		intel_pmdemand_program_dbuf(display, 0);
 }
 
diff --git a/drivers/gpu/drm/i915/display/intel_pmdemand.c b/drivers/gpu/drm/i915/display/intel_pmdemand.c
index 7819b724795b..3a2e7825eb52 100644
--- a/drivers/gpu/drm/i915/display/intel_pmdemand.c
+++ b/drivers/gpu/drm/i915/display/intel_pmdemand.c
@@ -152,7 +152,7 @@ intel_pmdemand_update_phys_mask(struct intel_display *display,
 {
 	enum phy phy;
 
-	if (DISPLAY_VER(display) < 14)
+	if (!HAS_PMDEMAND(display))
 		return;
 
 	if (!encoder)
@@ -174,7 +174,7 @@ intel_pmdemand_update_port_clock(struct intel_display *display,
 				 struct intel_pmdemand_state *pmdemand_state,
 				 enum pipe pipe, int port_clock)
 {
-	if (DISPLAY_VER(display) < 14)
+	if (!HAS_PMDEMAND(display))
 		return;
 
 	pmdemand_state->ddi_clocks[pipe] = port_clock;
@@ -326,7 +326,7 @@ int intel_pmdemand_atomic_check(struct intel_atomic_state *state)
 	const struct intel_dbuf_state *new_dbuf_state;
 	struct intel_pmdemand_state *new_pmdemand_state;
 
-	if (DISPLAY_VER(display) < 14)
+	if (!HAS_PMDEMAND(display))
 		return 0;
 
 	if (!intel_pmdemand_needs_update(state))
@@ -406,7 +406,7 @@ intel_pmdemand_init_pmdemand_params(struct intel_display *display,
 {
 	u32 reg1, reg2;
 
-	if (DISPLAY_VER(display) < 14)
+	if (!HAS_PMDEMAND(display))
 		return;
 
 	mutex_lock(&display->pmdemand.lock);
@@ -639,7 +639,7 @@ void intel_pmdemand_pre_plane_update(struct intel_atomic_state *state)
 	const struct intel_pmdemand_state *old_pmdemand_state =
 		intel_atomic_get_old_pmdemand_state(state);
 
-	if (DISPLAY_VER(display) < 14)
+	if (!HAS_PMDEMAND(display))
 		return;
 
 	if (!new_pmdemand_state ||
@@ -662,7 +662,7 @@ void intel_pmdemand_post_plane_update(struct intel_atomic_state *state)
 	const struct intel_pmdemand_state *old_pmdemand_state =
 		intel_atomic_get_old_pmdemand_state(state);
 
-	if (DISPLAY_VER(display) < 14)
+	if (!HAS_PMDEMAND(display))
 		return;
 
 	if (!new_pmdemand_state ||
-- 
2.43.0

[PATCH v2 03/11] drm/i915/display: sagv pre/post plane calls to check pmdemand support

[Vinod Govindapillai]

For pmdemand cases, no need to even calculate the masks based
on the qgv points index. Though the current logic avoids setting
the registers based on the pmdemand support, some qgv point masks
are compared in vain and do nothing. So leave early if pmdemand
is supported.

Signed-off-by: Vinod Govindapillai <vinod.govindapillai@intel.com>
---
 drivers/gpu/drm/i915/display/skl_watermark.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/gpu/drm/i915/display/skl_watermark.c b/drivers/gpu/drm/i915/display/skl_watermark.c
index c942ccfe6897..30a2031aa613 100644
--- a/drivers/gpu/drm/i915/display/skl_watermark.c
+++ b/drivers/gpu/drm/i915/display/skl_watermark.c
@@ -275,6 +275,9 @@ void intel_sagv_pre_plane_update(struct intel_atomic_state *state)
 	if (!intel_has_sagv(display))
 		return;
 
+	if (HAS_PMDEMAND(display))
+		return;
+
 	if (DISPLAY_VER(display) >= 11)
 		icl_sagv_pre_plane_update(state);
 	else
@@ -295,6 +298,9 @@ void intel_sagv_post_plane_update(struct intel_atomic_state *state)
 	if (!intel_has_sagv(display))
 		return;
 
+	if (HAS_PMDEMAND(display))
+		return;
+
 	if (DISPLAY_VER(display) >= 11)
 		icl_sagv_post_plane_update(state);
 	else
-- 
2.43.0

[PATCH v2 04/11] drm/i915/bw: Extract icl_init_qgv_info()

[Vinod Govindapillai]

Simplify the initialization of qgv points info by extracting
the code to initialize the qgv points info from dram info based
on the memory type.

Signed-off-by: Vinod Govindapillai <vinod.govindapillai@intel.com>
---
 drivers/gpu/drm/i915/display/intel_bw.c | 23 +++++++++++++++++------
 1 file changed, 17 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_bw.c b/drivers/gpu/drm/i915/display/intel_bw.c
index d99e921df0b9..e9cfa3edd09e 100644
--- a/drivers/gpu/drm/i915/display/intel_bw.c
+++ b/drivers/gpu/drm/i915/display/intel_bw.c
@@ -238,13 +238,11 @@ intel_read_qgv_point_info(struct intel_display *display,
 		return icl_pcode_read_qgv_point_info(display, sp, point);
 }
 
-static int icl_get_qgv_points(struct intel_display *display,
-			      const struct dram_info *dram_info,
-			      struct intel_qgv_info *qi,
-			      bool is_y_tile)
+static int icl_init_qgv_info(struct intel_display *display,
+			     const struct dram_info *dram_info,
+			     struct intel_qgv_info *qi,
+			     bool is_y_tile)
 {
-	int i, ret;
-
 	qi->num_points = dram_info->num_qgv_points;
 	qi->num_psf_points = dram_info->num_psf_gv_points;
 
@@ -316,6 +314,19 @@ static int icl_get_qgv_points(struct intel_display *display,
 		qi->max_numchannels = 1;
 	}
 
+	return 0;
+}
+
+static int icl_get_qgv_points(struct intel_display *display,
+			      const struct dram_info *dram_info,
+			      struct intel_qgv_info *qi,
+			      bool is_y_tile)
+{
+	int i, ret;
+
+	if (icl_init_qgv_info(display, dram_info, qi, is_y_tile))
+		return -EINVAL;
+
 	if (drm_WARN_ON(display->drm,
 			qi->num_points > ARRAY_SIZE(qi->points)))
 		qi->num_points = ARRAY_SIZE(qi->points);
-- 
2.43.0

[PATCH v2 05/11] drm/i915/bw: sort the qgv points based on the dclk

[Vinod Govindapillai]

QGV points reported from BIOS are not in any sorted order. But the
this order cannot be modified as the pcode was expecting the qgv
point index on pre display 14 versions. But after the introduction
of pmdemand, pcode is expecting the peak bw of the selected qgv
point. But because this point is not sorted, we would have to go
through all of these points to find any exact match. So sort the
qgv point based on the dclk and this sorted order will benefit
in all calculations which need to find appropriate qgv point.

Assisted-by: Copilot:claude-sonnet-4.6
Signed-off-by: Vinod Govindapillai <vinod.govindapillai@intel.com>
---
 drivers/gpu/drm/i915/display/intel_bw.c | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/drivers/gpu/drm/i915/display/intel_bw.c b/drivers/gpu/drm/i915/display/intel_bw.c
index e9cfa3edd09e..7b8801a88cb2 100644
--- a/drivers/gpu/drm/i915/display/intel_bw.c
+++ b/drivers/gpu/drm/i915/display/intel_bw.c
@@ -7,6 +7,8 @@
 #include <drm/drm_print.h>
 #include <drm/intel/intel_pcode_regs.h>
 
+#include <linux/sort.h>
+
 #include "intel_bw.h"
 #include "intel_crtc.h"
 #include "intel_de.h"
@@ -317,6 +319,20 @@ static int icl_init_qgv_info(struct intel_display *display,
 	return 0;
 }
 
+static int qgv_point_cmp(const void *a, const void *b)
+{
+	const struct intel_qgv_point *pa = a;
+	const struct intel_qgv_point *pb = b;
+
+	return pa->dclk - pb->dclk;
+}
+
+static void intel_sort_qgv_points(struct intel_qgv_info *qi)
+{
+	sort(qi->points, qi->num_points, sizeof(*qi->points),
+	     qgv_point_cmp, NULL);
+}
+
 static int icl_get_qgv_points(struct intel_display *display,
 			      const struct dram_info *dram_info,
 			      struct intel_qgv_info *qi,
@@ -346,6 +362,9 @@ static int icl_get_qgv_points(struct intel_display *display,
 			    sp->t_rcd, sp->t_rc);
 	}
 
+	if (HAS_PMDEMAND(display))
+		intel_sort_qgv_points(qi);
+
 	if (qi->num_psf_points > 0) {
 		ret = adls_pcode_read_psf_gv_point_info(display, qi->psf_points);
 		if (ret) {
-- 
2.43.0

[PATCH v2 06/11] drm/i915/bw: update the routine to get max dclk from qgv points

[Vinod Govindapillai]

As the qgv points are in the sorted order in pmdemand supported
versions, get the dclk from last qgv point.

Signed-off-by: Vinod Govindapillai <vinod.govindapillai@intel.com>
---
 drivers/gpu/drm/i915/display/intel_bw.c | 16 ++++++++++++----
 1 file changed, 12 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_bw.c b/drivers/gpu/drm/i915/display/intel_bw.c
index 7b8801a88cb2..f05fdb5f24c9 100644
--- a/drivers/gpu/drm/i915/display/intel_bw.c
+++ b/drivers/gpu/drm/i915/display/intel_bw.c
@@ -391,11 +391,19 @@ static int adl_calc_psf_bw(int clk)
 	return DIV_ROUND_CLOSEST(64 * clk * 100, 6);
 }
 
-static int icl_sagv_max_dclk(const struct intel_qgv_info *qi)
+static int icl_sagv_max_dclk(struct intel_display *display,
+			     const struct intel_qgv_info *qi)
 {
 	u16 dclk = 0;
 	int i;
 
+	/* QGV points are in sorted order in pmdemand supported versions */
+	if (HAS_PMDEMAND(display)) {
+		int max_point = max(0, qi->num_points - 1);
+
+		return qi->points[max_point].dclk;
+	}
+
 	for (i = 0; i < qi->num_points; i++)
 		dclk = max(dclk, qi->points[i].dclk);
 
@@ -495,7 +503,7 @@ static int icl_get_bw_info(struct intel_display *display,
 		return ret;
 	}
 
-	dclk_max = icl_sagv_max_dclk(&qi);
+	dclk_max = icl_sagv_max_dclk(display, &qi);
 	maxdebw = min(sa->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);
@@ -581,7 +589,7 @@ static int tgl_get_bw_info(struct intel_display *display,
 	if (qi.max_numchannels != 0)
 		num_channels = min_t(u8, num_channels, qi.max_numchannels);
 
-	dclk_max = icl_sagv_max_dclk(&qi);
+	dclk_max = icl_sagv_max_dclk(display, &qi);
 
 	peakbw = num_channels * DIV_ROUND_UP(qi.channel_width, 8) * dclk_max;
 	maxdebw = min(sa->deprogbwlimit * 1000, peakbw * DEPROGBWPCLIMIT / 100);
@@ -705,7 +713,7 @@ static int xe2_hpd_get_bw_info(struct intel_display *display,
 		return ret;
 	}
 
-	peakbw = num_channels * qi.channel_width / 8 * icl_sagv_max_dclk(&qi);
+	peakbw = num_channels * qi.channel_width / 8 * icl_sagv_max_dclk(display, &qi);
 	maxdebw = min(sa->deprogbwlimit * 1000, peakbw * DEPROGBWPCLIMIT / 10);
 
 	for (i = 0; i < qi.num_points; i++) {
-- 
2.43.0

[PATCH v2 07/11] drm/i915/bw: update the routine to find the peakbw in MTL

[Vinod Govindapillai]

As the QGV points are sorted based on dclk for pmdemand cases,
there is no need to go through the entire QGV points to find
out the acceptable peak bw. We could break the loop as soon as
we find out the first QGV point with the acceptable deratedbw
for the current display required bw.

Signed-off-by: Vinod Govindapillai <vinod.govindapillai@intel.com>
---
 drivers/gpu/drm/i915/display/intel_bw.c | 13 +++----------
 1 file changed, 3 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_bw.c b/drivers/gpu/drm/i915/display/intel_bw.c
index f05fdb5f24c9..177c3fd8f74a 100644
--- a/drivers/gpu/drm/i915/display/intel_bw.c
+++ b/drivers/gpu/drm/i915/display/intel_bw.c
@@ -1080,10 +1080,8 @@ static int mtl_find_qgv_points(struct intel_display *display,
 			       unsigned int num_active_planes,
 			       struct intel_bw_state *new_bw_state)
 {
-	unsigned int best_rate = UINT_MAX;
 	unsigned int num_qgv_points = display->bw.max[0].num_qgv_points;
 	unsigned int qgv_peak_bw  = 0;
-	int i;
 	int ret;
 
 	ret = intel_atomic_lock_global_state(&new_bw_state->base);
@@ -1105,7 +1103,7 @@ static int mtl_find_qgv_points(struct intel_display *display,
 	 * Find the best QGV point by comparing the data_rate with max data rate
 	 * offered per plane group
 	 */
-	for (i = 0; i < num_qgv_points; i++) {
+	for (int i = 0; i < num_qgv_points; i++) {
 		unsigned int bw_index =
 			tgl_max_bw_index(display, num_active_planes, i);
 		unsigned int max_data_rate;
@@ -1118,13 +1116,8 @@ static int mtl_find_qgv_points(struct intel_display *display,
 		if (max_data_rate < data_rate)
 			continue;
 
-		if (max_data_rate - data_rate < best_rate) {
-			best_rate = max_data_rate - data_rate;
-			qgv_peak_bw = display->bw.max[bw_index].peakbw[i];
-		}
-
-		drm_dbg_kms(display->drm, "QGV point %d: max bw %d required %d qgv_peak_bw: %d\n",
-			    i, max_data_rate, data_rate, qgv_peak_bw);
+		qgv_peak_bw = display->bw.max[bw_index].peakbw[i];
+		break;
 	}
 
 	drm_dbg_kms(display->drm, "Matching peaks QGV bw: %d for required data rate: %d\n",
-- 
2.43.0

[PATCH v2 08/11] drm/i915/bw: update the tile-y dependency based on the display version

[Vinod Govindapillai]

Consolidate the tile-y dependency into a separate function and
use it to determine whether tile-y need to take into account for
the bw calculations. Especially for post MTL cases, as per the
bspec, no need to take the tile_y into account for the
de interleave values.

Bspec: 64631, 68859
Signed-off-by: Vinod Govindapillai <vinod.govindapillai@intel.com>
---
 drivers/gpu/drm/i915/display/intel_bw.c | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_bw.c b/drivers/gpu/drm/i915/display/intel_bw.c
index 177c3fd8f74a..858da1710a61 100644
--- a/drivers/gpu/drm/i915/display/intel_bw.c
+++ b/drivers/gpu/drm/i915/display/intel_bw.c
@@ -483,12 +483,21 @@ static const struct intel_sa_info xe3lpd_3002_sa_info = {
 	.derating = 10,
 };
 
+static bool is_tile_y_factored(struct intel_display *display)
+{
+	/* TGL supports Y-tile for LPDDR4/5, but not for DDR4 */
+	if (DISPLAY_VER(display) >= 14)
+		return false;
+
+	return true;
+}
+
 static int icl_get_bw_info(struct intel_display *display,
 			   const struct dram_info *dram_info,
 			   const struct intel_sa_info *sa)
 {
 	struct intel_qgv_info qi = {};
-	bool is_y_tile = true; /* assume y tile may be used */
+	bool is_y_tile = is_tile_y_factored(display);
 	int num_channels = max_t(u8, 1, dram_info->num_channels);
 	int ipqdepth, ipqdepthpch = 16;
 	int dclk_max;
@@ -559,7 +568,7 @@ static int tgl_get_bw_info(struct intel_display *display,
 			   const struct intel_sa_info *sa)
 {
 	struct intel_qgv_info qi = {};
-	bool is_y_tile = true; /* assume y tile may be used */
+	bool is_y_tile = is_tile_y_factored(display);
 	int num_channels = max_t(u8, 1, dram_info->num_channels);
 	int ipqdepth, ipqdepthpch = 16;
 	int dclk_max;
-- 
2.43.0

[PATCH v2 09/11] drm/i915/bw: consolidate the debug info of bw/dgv/psf data

[Vinod Govindapillai]

Consolidate all the detaills about the bw, gqv and psf gv into
a common function and present it in an organised format

Signed-off-by: Vinod Govindapillai <vinod.govindapillai@intel.com>
---
 drivers/gpu/drm/i915/display/intel_bw.c | 40 +++++++++++++++++--------
 1 file changed, 27 insertions(+), 13 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_bw.c b/drivers/gpu/drm/i915/display/intel_bw.c
index 858da1710a61..0bda13a3e31b 100644
--- a/drivers/gpu/drm/i915/display/intel_bw.c
+++ b/drivers/gpu/drm/i915/display/intel_bw.c
@@ -483,6 +483,28 @@ static const struct intel_sa_info xe3lpd_3002_sa_info = {
 	.derating = 10,
 };
 
+static void debug_print_bw_info(struct intel_display *display)
+{
+	int num_groups = ARRAY_SIZE(display->bw.max);
+	int i;
+
+	for (i = 0; i < num_groups; i++) {
+		struct intel_bw_info *bi = &display->bw.max[i];
+		int j;
+
+		drm_dbg_kms(display->drm, "BW%d: num_planes=%d num_qgv_points:%d\n",
+			    i, bi->num_planes, bi->num_qgv_points);
+
+		for (j = 0; j < bi->num_qgv_points; j++)
+			drm_dbg_kms(display->drm, "\tQGV %d: deratedbw=%u peakbw=%u\n",
+				    j,  bi->deratedbw[j], bi->peakbw[j]);
+
+		for (j = 0; j < bi->num_psf_gv_points; j++)
+			drm_dbg_kms(display->drm, "\tPSF GV %d bw=%u\n",
+				    j, bi->psf_bw[j]);
+	}
+}
+
 static bool is_tile_y_factored(struct intel_display *display)
 {
 	/* TGL supports Y-tile for LPDDR4/5, but not for DDR4 */
@@ -544,12 +566,11 @@ static int icl_get_bw_info(struct intel_display *display,
 
 			bi->deratedbw[j] = min(maxdebw,
 					       bw * (100 - sa->derating) / 100);
-
-			drm_dbg_kms(display->drm,
-				    "BW%d / QGV %d: num_planes=%d deratedbw=%u\n",
-				    i, j, bi->num_planes, bi->deratedbw[j]);
 		}
 	}
+
+	debug_print_bw_info(display);
+
 	/*
 	 * In case if SAGV is disabled in BIOS, we always get 1
 	 * SAGV point, but we can't send PCode commands to restrict it
@@ -650,24 +671,17 @@ static int tgl_get_bw_info(struct intel_display *display,
 			bi->peakbw[j] = DIV_ROUND_CLOSEST(sp->dclk *
 							  num_channels *
 							  qi.channel_width, 8);
-
-			drm_dbg_kms(display->drm,
-				    "BW%d / QGV %d: num_planes=%d deratedbw=%u peakbw: %u\n",
-				    i, j, bi->num_planes, bi->deratedbw[j],
-				    bi->peakbw[j]);
 		}
 
 		for (j = 0; j < qi.num_psf_points; j++) {
 			const struct intel_psf_gv_point *sp = &qi.psf_points[j];
 
 			bi->psf_bw[j] = adl_calc_psf_bw(sp->clk);
-
-			drm_dbg_kms(display->drm,
-				    "BW%d / PSF GV %d: num_planes=%d bw=%u\n",
-				    i, j, bi->num_planes, bi->psf_bw[j]);
 		}
 	}
 
+	debug_print_bw_info(display);
+
 	/*
 	 * In case if SAGV is disabled in BIOS, we always get 1
 	 * SAGV point, but we can't send PCode commands to restrict it
-- 
2.43.0

Re: [PATCH v2 09/11] drm/i915/bw: consolidate the debug info of bw/dgv/psf data

[Jani Nikula]

On Mon, 11 May 2026, Vinod Govindapillai <vinod.govindapillai@intel.com> wrote:
> Consolidate all the detaills about the bw, gqv and psf gv into
> a common function and present it in an organised format
>
> Signed-off-by: Vinod Govindapillai <vinod.govindapillai@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_bw.c | 40 +++++++++++++++++--------
>  1 file changed, 27 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_bw.c b/drivers/gpu/drm/i915/display/intel_bw.c
> index 858da1710a61..0bda13a3e31b 100644
> --- a/drivers/gpu/drm/i915/display/intel_bw.c
> +++ b/drivers/gpu/drm/i915/display/intel_bw.c
> @@ -483,6 +483,28 @@ static const struct intel_sa_info xe3lpd_3002_sa_info = {
>  	.derating = 10,
>  };
>  
> +static void debug_print_bw_info(struct intel_display *display)
> +{
> +	int num_groups = ARRAY_SIZE(display->bw.max);
> +	int i;
> +
> +	for (i = 0; i < num_groups; i++) {
> +		struct intel_bw_info *bi = &display->bw.max[i];
> +		int j;
> +
> +		drm_dbg_kms(display->drm, "BW%d: num_planes=%d num_qgv_points:%d\n",
> +			    i, bi->num_planes, bi->num_qgv_points);
> +
> +		for (j = 0; j < bi->num_qgv_points; j++)
> +			drm_dbg_kms(display->drm, "\tQGV %d: deratedbw=%u peakbw=%u\n",

Please use a drm_printer, and drm_printf_indent().

> +				    j,  bi->deratedbw[j], bi->peakbw[j]);
> +
> +		for (j = 0; j < bi->num_psf_gv_points; j++)
> +			drm_dbg_kms(display->drm, "\tPSF GV %d bw=%u\n",
> +				    j, bi->psf_bw[j]);
> +	}
> +}
> +
>  static bool is_tile_y_factored(struct intel_display *display)
>  {
>  	/* TGL supports Y-tile for LPDDR4/5, but not for DDR4 */
> @@ -544,12 +566,11 @@ static int icl_get_bw_info(struct intel_display *display,
>  
>  			bi->deratedbw[j] = min(maxdebw,
>  					       bw * (100 - sa->derating) / 100);
> -
> -			drm_dbg_kms(display->drm,
> -				    "BW%d / QGV %d: num_planes=%d deratedbw=%u\n",
> -				    i, j, bi->num_planes, bi->deratedbw[j]);
>  		}
>  	}
> +
> +	debug_print_bw_info(display);
> +
>  	/*
>  	 * In case if SAGV is disabled in BIOS, we always get 1
>  	 * SAGV point, but we can't send PCode commands to restrict it
> @@ -650,24 +671,17 @@ static int tgl_get_bw_info(struct intel_display *display,
>  			bi->peakbw[j] = DIV_ROUND_CLOSEST(sp->dclk *
>  							  num_channels *
>  							  qi.channel_width, 8);
> -
> -			drm_dbg_kms(display->drm,
> -				    "BW%d / QGV %d: num_planes=%d deratedbw=%u peakbw: %u\n",
> -				    i, j, bi->num_planes, bi->deratedbw[j],
> -				    bi->peakbw[j]);
>  		}
>  
>  		for (j = 0; j < qi.num_psf_points; j++) {
>  			const struct intel_psf_gv_point *sp = &qi.psf_points[j];
>  
>  			bi->psf_bw[j] = adl_calc_psf_bw(sp->clk);
> -
> -			drm_dbg_kms(display->drm,
> -				    "BW%d / PSF GV %d: num_planes=%d bw=%u\n",
> -				    i, j, bi->num_planes, bi->psf_bw[j]);
>  		}
>  	}
>  
> +	debug_print_bw_info(display);
> +
>  	/*
>  	 * In case if SAGV is disabled in BIOS, we always get 1
>  	 * SAGV point, but we can't send PCode commands to restrict it

-- 
Jani Nikula, Intel

[PATCH v2 10/11] drm/i915/bw: extract update_sagv_status()

[Vinod Govindapillai]

Extract the code to update the sagv status based on the number of
QGV points into a separate function and use it.

Signed-off-by: Vinod Govindapillai <vinod.govindapillai@intel.com>
---
 drivers/gpu/drm/i915/display/intel_bw.c | 33 +++++++++++--------------
 1 file changed, 15 insertions(+), 18 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_bw.c b/drivers/gpu/drm/i915/display/intel_bw.c
index 0bda13a3e31b..938c0294c251 100644
--- a/drivers/gpu/drm/i915/display/intel_bw.c
+++ b/drivers/gpu/drm/i915/display/intel_bw.c
@@ -505,6 +505,19 @@ static void debug_print_bw_info(struct intel_display *display)
 	}
 }
 
+static void update_sagv_status(struct intel_display *display, int qgv_points)
+{
+	/*
+	 * In case if SAGV is disabled in BIOS, we always get 1
+	 * SAGV point, but we can't send PCode commands to restrict it
+	 * as it will fail and pointless anyway.
+	 */
+	if (qgv_points == 1)
+		display->sagv.status = I915_SAGV_NOT_CONTROLLED;
+	else
+		display->sagv.status = I915_SAGV_ENABLED;
+}
+
 static bool is_tile_y_factored(struct intel_display *display)
 {
 	/* TGL supports Y-tile for LPDDR4/5, but not for DDR4 */
@@ -571,15 +584,7 @@ static int icl_get_bw_info(struct intel_display *display,
 
 	debug_print_bw_info(display);
 
-	/*
-	 * In case if SAGV is disabled in BIOS, we always get 1
-	 * SAGV point, but we can't send PCode commands to restrict it
-	 * as it will fail and pointless anyway.
-	 */
-	if (qi.num_points == 1)
-		display->sagv.status = I915_SAGV_NOT_CONTROLLED;
-	else
-		display->sagv.status = I915_SAGV_ENABLED;
+	update_sagv_status(display, qi.num_points);
 
 	return 0;
 }
@@ -682,15 +687,7 @@ static int tgl_get_bw_info(struct intel_display *display,
 
 	debug_print_bw_info(display);
 
-	/*
-	 * In case if SAGV is disabled in BIOS, we always get 1
-	 * SAGV point, but we can't send PCode commands to restrict it
-	 * as it will fail and pointless anyway.
-	 */
-	if (qi.num_points == 1)
-		display->sagv.status = I915_SAGV_NOT_CONTROLLED;
-	else
-		display->sagv.status = I915_SAGV_ENABLED;
+	update_sagv_status(display, qi.num_points);
 
 	return 0;
 }
-- 
2.43.0

[PATCH v2 11/11] drm/i915/bw: insert a sw bw info entry to cater low data rate usecases

[Vinod Govindapillai]

In xe3+, soc can lower the fabric frequency when the display
needs less bandwidth than the minimum GV point. This threshold
has been defined as 20GB/s. To enable this,

Add a new low bw info point with this peakbw threshold of 20GB/s
based on the following conditions:
1. Only for xe3+ versions
2. There is at least one QGV point
3. Number QGV points is less than 8
4. Lowest peak bw across all the QGV point is less than 20 GB/s
5. And the derated bw is in the lowest peak bw qgv point is also
   less than this threshold of 20GB/s

This will make the driver to send this new threshold of 20GB/s
as the pmdemand request whenever the bw required for a usecase
is less than 20GB/s. The current pcode can handle this lower
peakbw value and adjust the fabric frequency accordingly.

Bspec: 68880
Assisted-by: Copilot:claude-sonnet-4.6
Signed-off-by: Vinod Govindapillai <vinod.govindapillai@intel.com>
---
 drivers/gpu/drm/i915/display/intel_bw.c       | 54 +++++++++++++++++++
 .../drm/i915/display/intel_display_device.h   |  1 +
 2 files changed, 55 insertions(+)

diff --git a/drivers/gpu/drm/i915/display/intel_bw.c b/drivers/gpu/drm/i915/display/intel_bw.c
index 938c0294c251..747279075e29 100644
--- a/drivers/gpu/drm/i915/display/intel_bw.c
+++ b/drivers/gpu/drm/i915/display/intel_bw.c
@@ -54,6 +54,8 @@ struct intel_qgv_point {
 
 #define DEPROGBWPCLIMIT		60
 
+#define PEAK_BW_THRESHOLD	20000
+
 struct intel_psf_gv_point {
 	u8 clk; /* clock in multiples of 16.6666 MHz */
 };
@@ -589,6 +591,50 @@ static int icl_get_bw_info(struct intel_display *display,
 	return 0;
 }
 
+static bool xe3_check_lower_peakbw(struct intel_display *display,
+				   const struct intel_qgv_info *qi,
+				   int num_channels)
+{
+	unsigned int lowest_peakbw;
+
+	if (!HAS_PEAK_BW_THRESHOLD(display))
+		return false;
+
+	if (qi->num_points >= I915_NUM_QGV_POINTS) {
+		drm_warn(display->drm, "Cannot insert lowest QGV point, not enough space\n");
+		return false;
+	}
+
+	lowest_peakbw = DIV_ROUND_CLOSEST(qi->points[0].dclk *
+					  qi->channel_width * num_channels, 8);
+	if (lowest_peakbw <= PEAK_BW_THRESHOLD) {
+		drm_dbg_kms(display->drm,
+			    "Lowest QGV point has peak BW %u MB/s, no need to insert lower point\n",
+			    lowest_peakbw);
+		return false;
+	}
+
+	return true;
+}
+
+static void xe3_insert_lowest_qgv_point(struct intel_display *display,
+					struct intel_bw_info *bi)
+{
+	if (bi->num_qgv_points >= ARRAY_SIZE(bi->deratedbw))
+		return;
+
+	memmove(&bi->deratedbw[1], &bi->deratedbw[0],
+		bi->num_qgv_points * sizeof(*bi->deratedbw));
+
+	memmove(&bi->peakbw[1], &bi->peakbw[0],
+		bi->num_qgv_points * sizeof(*bi->peakbw));
+
+	/* Keep the derated bandwidth as the threshold*/
+	bi->deratedbw[0] = PEAK_BW_THRESHOLD;
+	bi->peakbw[0] = PEAK_BW_THRESHOLD;
+	bi->num_qgv_points++;
+}
+
 static int tgl_get_bw_info(struct intel_display *display,
 			   const struct dram_info *dram_info,
 			   const struct intel_sa_info *sa)
@@ -598,6 +644,7 @@ static int tgl_get_bw_info(struct intel_display *display,
 	int num_channels = max_t(u8, 1, dram_info->num_channels);
 	int ipqdepth, ipqdepthpch = 16;
 	int dclk_max;
+	bool insert_low_peakbw;
 	int maxdebw, peakbw;
 	int clperchgroup;
 	int num_groups = ARRAY_SIZE(display->bw.max);
@@ -636,6 +683,10 @@ static int tgl_get_bw_info(struct intel_display *display,
 	 */
 	clperchgroup = 4 * DIV_ROUND_UP(8, num_channels) * qi.deinterleave;
 
+	insert_low_peakbw = xe3_check_lower_peakbw(display, &qi, num_channels);
+
+	display->bw.max[i].num_planes = 0;
+
 	for (i = 0; i < num_groups; i++) {
 		struct intel_bw_info *bi = &display->bw.max[i];
 		struct intel_bw_info *bi_next;
@@ -678,6 +729,9 @@ static int tgl_get_bw_info(struct intel_display *display,
 							  qi.channel_width, 8);
 		}
 
+		if (insert_low_peakbw)
+			xe3_insert_lowest_qgv_point(display, bi);
+
 		for (j = 0; j < qi.num_psf_points; j++) {
 			const struct intel_psf_gv_point *sp = &qi.psf_points[j];
 
diff --git a/drivers/gpu/drm/i915/display/intel_display_device.h b/drivers/gpu/drm/i915/display/intel_display_device.h
index 65283286771a..b31ec42c3248 100644
--- a/drivers/gpu/drm/i915/display/intel_display_device.h
+++ b/drivers/gpu/drm/i915/display/intel_display_device.h
@@ -192,6 +192,7 @@ struct intel_display_platforms {
 #define HAS_MBUS_JOINING(__display)	((__display)->platform.alderlake_p || DISPLAY_VER(__display) >= 14)
 #define HAS_MSO(__display)		(DISPLAY_VER(__display) >= 12)
 #define HAS_OVERLAY(__display)		(DISPLAY_INFO(__display)->has_overlay)
+#define HAS_PEAK_BW_THRESHOLD(__display)	(DISPLAY_VER(__display) >= 30)
 #define HAS_PIPEDMC(__display)		(DISPLAY_VER(__display) >= 12)
 #define HAS_PIXEL_NORMALIZER(__display)	(DISPLAY_VER(__display) >= 35)
 #define HAS_PMDEMAND(__display)		(DISPLAY_VER(__display) >= 14)
-- 
2.43.0

Re: [PATCH v2 11/11] drm/i915/bw: insert a sw bw info entry to cater low data rate usecases

[Jani Nikula]

On Mon, 11 May 2026, Vinod Govindapillai <vinod.govindapillai@intel.com> wrote:
> In xe3+, soc can lower the fabric frequency when the display
> needs less bandwidth than the minimum GV point. This threshold
> has been defined as 20GB/s. To enable this,
>
> Add a new low bw info point with this peakbw threshold of 20GB/s
> based on the following conditions:
> 1. Only for xe3+ versions
> 2. There is at least one QGV point
> 3. Number QGV points is less than 8
> 4. Lowest peak bw across all the QGV point is less than 20 GB/s
> 5. And the derated bw is in the lowest peak bw qgv point is also
>    less than this threshold of 20GB/s
>
> This will make the driver to send this new threshold of 20GB/s
> as the pmdemand request whenever the bw required for a usecase
> is less than 20GB/s. The current pcode can handle this lower
> peakbw value and adjust the fabric frequency accordingly.
>
> Bspec: 68880
> Assisted-by: Copilot:claude-sonnet-4.6
> Signed-off-by: Vinod Govindapillai <vinod.govindapillai@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_bw.c       | 54 +++++++++++++++++++
>  .../drm/i915/display/intel_display_device.h   |  1 +
>  2 files changed, 55 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_bw.c b/drivers/gpu/drm/i915/display/intel_bw.c
> index 938c0294c251..747279075e29 100644
> --- a/drivers/gpu/drm/i915/display/intel_bw.c
> +++ b/drivers/gpu/drm/i915/display/intel_bw.c
> @@ -54,6 +54,8 @@ struct intel_qgv_point {
>  
>  #define DEPROGBWPCLIMIT		60
>  
> +#define PEAK_BW_THRESHOLD	20000
> +
>  struct intel_psf_gv_point {
>  	u8 clk; /* clock in multiples of 16.6666 MHz */
>  };
> @@ -589,6 +591,50 @@ static int icl_get_bw_info(struct intel_display *display,
>  	return 0;
>  }
>  
> +static bool xe3_check_lower_peakbw(struct intel_display *display,
> +				   const struct intel_qgv_info *qi,
> +				   int num_channels)

My pet peeve is naming functions with "check". Is it an assert? What
does it do? What does the return value mean?

BR,
Jani.

> +{
> +	unsigned int lowest_peakbw;
> +
> +	if (!HAS_PEAK_BW_THRESHOLD(display))
> +		return false;
> +
> +	if (qi->num_points >= I915_NUM_QGV_POINTS) {
> +		drm_warn(display->drm, "Cannot insert lowest QGV point, not enough space\n");
> +		return false;
> +	}
> +
> +	lowest_peakbw = DIV_ROUND_CLOSEST(qi->points[0].dclk *
> +					  qi->channel_width * num_channels, 8);
> +	if (lowest_peakbw <= PEAK_BW_THRESHOLD) {
> +		drm_dbg_kms(display->drm,
> +			    "Lowest QGV point has peak BW %u MB/s, no need to insert lower point\n",
> +			    lowest_peakbw);
> +		return false;
> +	}
> +
> +	return true;
> +}
> +
> +static void xe3_insert_lowest_qgv_point(struct intel_display *display,
> +					struct intel_bw_info *bi)
> +{
> +	if (bi->num_qgv_points >= ARRAY_SIZE(bi->deratedbw))
> +		return;
> +
> +	memmove(&bi->deratedbw[1], &bi->deratedbw[0],
> +		bi->num_qgv_points * sizeof(*bi->deratedbw));
> +
> +	memmove(&bi->peakbw[1], &bi->peakbw[0],
> +		bi->num_qgv_points * sizeof(*bi->peakbw));
> +
> +	/* Keep the derated bandwidth as the threshold*/
> +	bi->deratedbw[0] = PEAK_BW_THRESHOLD;
> +	bi->peakbw[0] = PEAK_BW_THRESHOLD;
> +	bi->num_qgv_points++;
> +}
> +
>  static int tgl_get_bw_info(struct intel_display *display,
>  			   const struct dram_info *dram_info,
>  			   const struct intel_sa_info *sa)
> @@ -598,6 +644,7 @@ static int tgl_get_bw_info(struct intel_display *display,
>  	int num_channels = max_t(u8, 1, dram_info->num_channels);
>  	int ipqdepth, ipqdepthpch = 16;
>  	int dclk_max;
> +	bool insert_low_peakbw;
>  	int maxdebw, peakbw;
>  	int clperchgroup;
>  	int num_groups = ARRAY_SIZE(display->bw.max);
> @@ -636,6 +683,10 @@ static int tgl_get_bw_info(struct intel_display *display,
>  	 */
>  	clperchgroup = 4 * DIV_ROUND_UP(8, num_channels) * qi.deinterleave;
>  
> +	insert_low_peakbw = xe3_check_lower_peakbw(display, &qi, num_channels);
> +
> +	display->bw.max[i].num_planes = 0;
> +
>  	for (i = 0; i < num_groups; i++) {
>  		struct intel_bw_info *bi = &display->bw.max[i];
>  		struct intel_bw_info *bi_next;
> @@ -678,6 +729,9 @@ static int tgl_get_bw_info(struct intel_display *display,
>  							  qi.channel_width, 8);
>  		}
>  
> +		if (insert_low_peakbw)
> +			xe3_insert_lowest_qgv_point(display, bi);
> +
>  		for (j = 0; j < qi.num_psf_points; j++) {
>  			const struct intel_psf_gv_point *sp = &qi.psf_points[j];
>  
> diff --git a/drivers/gpu/drm/i915/display/intel_display_device.h b/drivers/gpu/drm/i915/display/intel_display_device.h
> index 65283286771a..b31ec42c3248 100644
> --- a/drivers/gpu/drm/i915/display/intel_display_device.h
> +++ b/drivers/gpu/drm/i915/display/intel_display_device.h
> @@ -192,6 +192,7 @@ struct intel_display_platforms {
>  #define HAS_MBUS_JOINING(__display)	((__display)->platform.alderlake_p || DISPLAY_VER(__display) >= 14)
>  #define HAS_MSO(__display)		(DISPLAY_VER(__display) >= 12)
>  #define HAS_OVERLAY(__display)		(DISPLAY_INFO(__display)->has_overlay)
> +#define HAS_PEAK_BW_THRESHOLD(__display)	(DISPLAY_VER(__display) >= 30)
>  #define HAS_PIPEDMC(__display)		(DISPLAY_VER(__display) >= 12)
>  #define HAS_PIXEL_NORMALIZER(__display)	(DISPLAY_VER(__display) >= 35)
>  #define HAS_PMDEMAND(__display)		(DISPLAY_VER(__display) >= 14)

-- 
Jani Nikula, Intel

Re: [PATCH v2 11/11] drm/i915/bw: insert a sw bw info entry to cater low data rate usecases

[Govindapillai, Vinod]

On Tue, 2026-05-12 at 14:51 +0300, Jani Nikula wrote:
> On Mon, 11 May 2026, Vinod Govindapillai
> <vinod.govindapillai@intel.com> wrote:
> > In xe3+, soc can lower the fabric frequency when the display
> > needs less bandwidth than the minimum GV point. This threshold
> > has been defined as 20GB/s. To enable this,
> > 
> > Add a new low bw info point with this peakbw threshold of 20GB/s
> > based on the following conditions:
> > 1. Only for xe3+ versions
> > 2. There is at least one QGV point
> > 3. Number QGV points is less than 8
> > 4. Lowest peak bw across all the QGV point is less than 20 GB/s
> > 5. And the derated bw is in the lowest peak bw qgv point is also
> >    less than this threshold of 20GB/s
> > 
> > This will make the driver to send this new threshold of 20GB/s
> > as the pmdemand request whenever the bw required for a usecase
> > is less than 20GB/s. The current pcode can handle this lower
> > peakbw value and adjust the fabric frequency accordingly.
> > 
> > Bspec: 68880
> > Assisted-by: Copilot:claude-sonnet-4.6
> > Signed-off-by: Vinod Govindapillai <vinod.govindapillai@intel.com>
> > ---
> >  drivers/gpu/drm/i915/display/intel_bw.c       | 54
> > +++++++++++++++++++
> >  .../drm/i915/display/intel_display_device.h   |  1 +
> >  2 files changed, 55 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/i915/display/intel_bw.c
> > b/drivers/gpu/drm/i915/display/intel_bw.c
> > index 938c0294c251..747279075e29 100644
> > --- a/drivers/gpu/drm/i915/display/intel_bw.c
> > +++ b/drivers/gpu/drm/i915/display/intel_bw.c
> > @@ -54,6 +54,8 @@ struct intel_qgv_point {
> >  
> >  #define DEPROGBWPCLIMIT		60
> >  
> > +#define PEAK_BW_THRESHOLD	20000
> > +
> >  struct intel_psf_gv_point {
> >  	u8 clk; /* clock in multiples of 16.6666 MHz */
> >  };
> > @@ -589,6 +591,50 @@ static int icl_get_bw_info(struct
> > intel_display *display,
> >  	return 0;
> >  }
> >  
> > +static bool xe3_check_lower_peakbw(struct intel_display *display,
> > +				   const struct intel_qgv_info
> > *qi,
> > +				   int num_channels)
> 
> My pet peeve is naming functions with "check". Is it an assert? What
> does it do? What does the return value mean?
> 
> BR,
> Jani.
> 

hmm.. Intention is that, it should check the conditions for inserting
the low peak bw.. 

May be xe3_needs_lower_peakbw_point() ?

BR
Vinod

> > +{
> > +	unsigned int lowest_peakbw;
> > +
> > +	if (!HAS_PEAK_BW_THRESHOLD(display))
> > +		return false;
> > +
> > +	if (qi->num_points >= I915_NUM_QGV_POINTS) {
> > +		drm_warn(display->drm, "Cannot insert lowest QGV
> > point, not enough space\n");
> > +		return false;
> > +	}
> > +
> > +	lowest_peakbw = DIV_ROUND_CLOSEST(qi->points[0].dclk *
> > +					  qi->channel_width *
> > num_channels, 8);
> > +	if (lowest_peakbw <= PEAK_BW_THRESHOLD) {
> > +		drm_dbg_kms(display->drm,
> > +			    "Lowest QGV point has peak BW %u MB/s,
> > no need to insert lower point\n",
> > +			    lowest_peakbw);
> > +		return false;
> > +	}
> > +
> > +	return true;
> > +}
> > +
> > +static void xe3_insert_lowest_qgv_point(struct intel_display
> > *display,
> > +					struct intel_bw_info *bi)
> > +{
> > +	if (bi->num_qgv_points >= ARRAY_SIZE(bi->deratedbw))
> > +		return;
> > +
> > +	memmove(&bi->deratedbw[1], &bi->deratedbw[0],
> > +		bi->num_qgv_points * sizeof(*bi->deratedbw));
> > +
> > +	memmove(&bi->peakbw[1], &bi->peakbw[0],
> > +		bi->num_qgv_points * sizeof(*bi->peakbw));
> > +
> > +	/* Keep the derated bandwidth as the threshold*/
> > +	bi->deratedbw[0] = PEAK_BW_THRESHOLD;
> > +	bi->peakbw[0] = PEAK_BW_THRESHOLD;
> > +	bi->num_qgv_points++;
> > +}
> > +
> >  static int tgl_get_bw_info(struct intel_display *display,
> >  			   const struct dram_info *dram_info,
> >  			   const struct intel_sa_info *sa)
> > @@ -598,6 +644,7 @@ static int tgl_get_bw_info(struct intel_display
> > *display,
> >  	int num_channels = max_t(u8, 1, dram_info->num_channels);
> >  	int ipqdepth, ipqdepthpch = 16;
> >  	int dclk_max;
> > +	bool insert_low_peakbw;
> >  	int maxdebw, peakbw;
> >  	int clperchgroup;
> >  	int num_groups = ARRAY_SIZE(display->bw.max);
> > @@ -636,6 +683,10 @@ static int tgl_get_bw_info(struct
> > intel_display *display,
> >  	 */
> >  	clperchgroup = 4 * DIV_ROUND_UP(8, num_channels) *
> > qi.deinterleave;
> >  
> > +	insert_low_peakbw = xe3_check_lower_peakbw(display, &qi,
> > num_channels);
> > +
> > +	display->bw.max[i].num_planes = 0;
> > +
> >  	for (i = 0; i < num_groups; i++) {
> >  		struct intel_bw_info *bi = &display->bw.max[i];
> >  		struct intel_bw_info *bi_next;
> > @@ -678,6 +729,9 @@ static int tgl_get_bw_info(struct intel_display
> > *display,
> >  							 
> > qi.channel_width, 8);
> >  		}
> >  
> > +		if (insert_low_peakbw)
> > +			xe3_insert_lowest_qgv_point(display, bi);
> > +
> >  		for (j = 0; j < qi.num_psf_points; j++) {
> >  			const struct intel_psf_gv_point *sp =
> > &qi.psf_points[j];
> >  
> > diff --git a/drivers/gpu/drm/i915/display/intel_display_device.h
> > b/drivers/gpu/drm/i915/display/intel_display_device.h
> > index 65283286771a..b31ec42c3248 100644
> > --- a/drivers/gpu/drm/i915/display/intel_display_device.h
> > +++ b/drivers/gpu/drm/i915/display/intel_display_device.h
> > @@ -192,6 +192,7 @@ struct intel_display_platforms {
> >  #define HAS_MBUS_JOINING(__display)	((__display)-
> > >platform.alderlake_p || DISPLAY_VER(__display) >= 14)
> >  #define HAS_MSO(__display)		(DISPLAY_VER(__display) >=
> > 12)
> >  #define HAS_OVERLAY(__display)		(DISPLAY_INFO(__display)-
> > >has_overlay)
> > +#define
> > HAS_PEAK_BW_THRESHOLD(__display)	(DISPLAY_VER(__display) >= 30)
> >  #define HAS_PIPEDMC(__display)		(DISPLAY_VER(__display) >=
> > 12)
> >  #define
> > HAS_PIXEL_NORMALIZER(__display)	(DISPLAY_VER(__display) >= 35)
> >  #define
> > HAS_PMDEMAND(__display)		(DISPLAY_VER(__display) >= 14)
> 

✗ i915.CI.BAT: failure for drm/i915/display: reduce the pm demand peak bw based on display data rate

[Patchwork]

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

== Series Details ==

Series: drm/i915/display: reduce the pm demand peak bw based on display data rate
URL   : https://patchwork.freedesktop.org/series/166315/
State : failure

== Summary ==

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

Summary
-------

  **FAILURE**

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

### IGT changes ###

#### Possible regressions ####

  * igt@i915_selftest@live@hangcheck:
    - bat-atsm-1:         [PASS][1] -> [INCOMPLETE][2]
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_18467/bat-atsm-1/igt@i915_selftest@live@hangcheck.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_166315v1/bat-atsm-1/igt@i915_selftest@live@hangcheck.html

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

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

### IGT changes ###

#### Issues hit ####

  * igt@i915_selftest@live@workarounds:
    - bat-mtlp-9:         [PASS][3] -> [DMESG-FAIL][4] ([i915#12061]) +1 other test dmesg-fail
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_18467/bat-mtlp-9/igt@i915_selftest@live@workarounds.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_166315v1/bat-mtlp-9/igt@i915_selftest@live@workarounds.html

  
#### Warnings ####

  * igt@i915_selftest@live:
    - bat-atsm-1:         [DMESG-FAIL][5] ([i915#12061]) -> [INCOMPLETE][6] ([i915#12061])
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_18467/bat-atsm-1/igt@i915_selftest@live.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_166315v1/bat-atsm-1/igt@i915_selftest@live.html

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


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

  * Linux: CI_DRM_18467 -> Patchwork_166315v1

  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_166315v1: f8ee23694aa6be213355905a78f79bb1b0861565 @ git://anongit.freedesktop.org/gfx-ci/linux

== Logs ==

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

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