[PATCH v2] drm/msm/dpu: Filter modes based on adjusted mode clock

[PATCH v2] drm/msm/dpu: Filter modes based on adjusted mode clock

[Jessica Zhang]

Filter out modes that have a clock rate greater than the max core clock
rate when adjusted for the perf clock factor

This is especially important for chipsets such as QCS615 that have lower
limits for the MDP max core clock.

Since the core CRTC clock is at least the mode clock (adjusted for the
perf clock factor) [1], the modes supported by the driver should be less
than the max core clock rate.

[1] https://elixir.bootlin.com/linux/v6.12.4/source/drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c#L83

Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
Signed-off-by: Jessica Zhang <jessica.zhang@oss.qualcomm.com>
---
Changes in v2:
- *crtc_clock -> *mode_clock (Dmitry)
- Changed adjusted_mode_clk check to use multiplication (Dmitry)
- Switch from quic_* email to OSS email
- Link to v1: https://lore.kernel.org/lkml/20241212-filter-mode-clock-v1-1-f4441988d6aa@quicinc.com/
---
 drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c | 35 ++++++++++++++++++---------
 drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.h |  3 +++
 drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c      | 12 +++++++++
 3 files changed, 39 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c
index 0fb5789c60d0..13cc658065c5 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c
@@ -31,6 +31,26 @@ enum dpu_perf_mode {
 	DPU_PERF_MODE_MAX
 };
 
+/**
+ * dpu_core_perf_adjusted_mode_clk - Adjust given mode clock rate according to
+ *   the perf clock factor.
+ * @crtc_clk_rate - Unadjusted mode clock rate
+ * @perf_cfg: performance configuration
+ */
+u64 dpu_core_perf_adjusted_mode_clk(u64 mode_clk_rate,
+				    const struct dpu_perf_cfg *perf_cfg)
+{
+	u32 clk_factor;
+
+	clk_factor = perf_cfg->clk_inefficiency_factor;
+	if (clk_factor) {
+		mode_clk_rate *= clk_factor;
+		do_div(mode_clk_rate, 100);
+	}
+
+	return mode_clk_rate;
+}
+
 /**
  * _dpu_core_perf_calc_bw() - to calculate BW per crtc
  * @perf_cfg: performance configuration
@@ -75,28 +95,21 @@ static u64 _dpu_core_perf_calc_clk(const struct dpu_perf_cfg *perf_cfg,
 	struct drm_plane *plane;
 	struct dpu_plane_state *pstate;
 	struct drm_display_mode *mode;
-	u64 crtc_clk;
-	u32 clk_factor;
+	u64 mode_clk;
 
 	mode = &state->adjusted_mode;
 
-	crtc_clk = (u64)mode->vtotal * mode->hdisplay * drm_mode_vrefresh(mode);
+	mode_clk = (u64)mode->vtotal * mode->hdisplay * drm_mode_vrefresh(mode);
 
 	drm_atomic_crtc_for_each_plane(plane, crtc) {
 		pstate = to_dpu_plane_state(plane->state);
 		if (!pstate)
 			continue;
 
-		crtc_clk = max(pstate->plane_clk, crtc_clk);
-	}
-
-	clk_factor = perf_cfg->clk_inefficiency_factor;
-	if (clk_factor) {
-		crtc_clk *= clk_factor;
-		do_div(crtc_clk, 100);
+		mode_clk = max(pstate->plane_clk, mode_clk);
 	}
 
-	return crtc_clk;
+	return dpu_core_perf_adjusted_mode_clk(mode_clk, perf_cfg);
 }
 
 static struct dpu_kms *_dpu_crtc_get_kms(struct drm_crtc *crtc)
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.h
index d2f21d34e501..3740bc97422c 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.h
@@ -54,6 +54,9 @@ struct dpu_core_perf {
 	u32 fix_core_ab_vote;
 };
 
+u64 dpu_core_perf_adjusted_mode_clk(u64 clk_rate,
+				    const struct dpu_perf_cfg *perf_cfg);
+
 int dpu_core_perf_crtc_check(struct drm_crtc *crtc,
 		struct drm_crtc_state *state);
 
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
index 0714936d8835..5e3c34fed63b 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
@@ -1501,6 +1501,7 @@ static enum drm_mode_status dpu_crtc_mode_valid(struct drm_crtc *crtc,
 						const struct drm_display_mode *mode)
 {
 	struct dpu_kms *dpu_kms = _dpu_crtc_get_kms(crtc);
+	u64 adjusted_mode_clk;
 
 	/* if there is no 3d_mux block we cannot merge LMs so we cannot
 	 * split the large layer into 2 LMs, filter out such modes
@@ -1508,6 +1509,17 @@ static enum drm_mode_status dpu_crtc_mode_valid(struct drm_crtc *crtc,
 	if (!dpu_kms->catalog->caps->has_3d_merge &&
 	    mode->hdisplay > dpu_kms->catalog->caps->max_mixer_width)
 		return MODE_BAD_HVALUE;
+
+	adjusted_mode_clk = dpu_core_perf_adjusted_mode_clk(mode->clock,
+							    dpu_kms->perf.perf_cfg);
+
+	/*
+	 * The given mode, adjusted for the perf clock factor, should not exceed
+	 * the max core clock rate
+	 */
+	if (dpu_kms->perf.max_core_clk_rate < adjusted_mode_clk * 1000)
+		return MODE_CLOCK_HIGH;
+
 	/*
 	 * max crtc width is equal to the max mixer width * 2 and max height is 4K
 	 */

---
base-commit: db76003ade5953d4a83c2bdc6e15c2d1c33e7350
change-id: 20250506-filter-modes-c60b4332769f

Best regards,
-- 
Jessica Zhang <jessica.zhang@oss.qualcomm.com>

Re: [PATCH v2] drm/msm/dpu: Filter modes based on adjusted mode clock

[neil.armstrong]

Hi,

On 5/7/25 03:38, Jessica Zhang wrote:
> Filter out modes that have a clock rate greater than the max core clock
> rate when adjusted for the perf clock factor
> 
> This is especially important for chipsets such as QCS615 that have lower
> limits for the MDP max core clock.
> 
> Since the core CRTC clock is at least the mode clock (adjusted for the
> perf clock factor) [1], the modes supported by the driver should be less
> than the max core clock rate.
> 
> [1] https://elixir.bootlin.com/linux/v6.12.4/source/drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c#L83
> 
> Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> Signed-off-by: Jessica Zhang <jessica.zhang@oss.qualcomm.com>

This change breaks the T14s OLED display, no modes are reported on the eDP connector.
msm_dpu ae01000.display-controller: [drm:update_display_info.part.0 [drm]] [CONNECTOR:41:eDP-1] Assigning EDID-1.4 digital sink color depth as 10 bpc.
msm_dpu ae01000.display-controller: [drm:update_display_info.part.0 [drm]] [CONNECTOR:41:eDP-1] ELD monitor
msm_dpu ae01000.display-controller: [drm:update_display_info.part.0 [drm]] [CONNECTOR:41:eDP-1] ELD size 20, SAD count 0
[drm:drm_mode_object_put.part.0 [drm]] OBJ ID: 113 (1)
msm_dpu ae01000.display-controller: [drm:drm_mode_prune_invalid [drm]] Rejected mode: "2880x1800": 120 652260 2880 2912 2920 2980 1800 1808 1816 1824 0x48 0x9 (CLOCK_HIGH)
msm_dpu ae01000.display-controller: [drm:drm_mode_prune_invalid [drm]] Rejected mode: "2880x1800": 60 652260 2880 2888 2920 2980 1800 1808 1816 3648 0x40 0x9 (CLOCK_HIGH)

With this reverted on v6.18-rc, display works again.

Neil

> ---
> Changes in v2:
> - *crtc_clock -> *mode_clock (Dmitry)
> - Changed adjusted_mode_clk check to use multiplication (Dmitry)
> - Switch from quic_* email to OSS email
> - Link to v1: https://lore.kernel.org/lkml/20241212-filter-mode-clock-v1-1-f4441988d6aa@quicinc.com/
> ---
>   drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c | 35 ++++++++++++++++++---------
>   drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.h |  3 +++
>   drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c      | 12 +++++++++
>   3 files changed, 39 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c
> index 0fb5789c60d0..13cc658065c5 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c
> @@ -31,6 +31,26 @@ enum dpu_perf_mode {
>   	DPU_PERF_MODE_MAX
>   };
>   
> +/**
> + * dpu_core_perf_adjusted_mode_clk - Adjust given mode clock rate according to
> + *   the perf clock factor.
> + * @crtc_clk_rate - Unadjusted mode clock rate
> + * @perf_cfg: performance configuration
> + */
> +u64 dpu_core_perf_adjusted_mode_clk(u64 mode_clk_rate,
> +				    const struct dpu_perf_cfg *perf_cfg)
> +{
> +	u32 clk_factor;
> +
> +	clk_factor = perf_cfg->clk_inefficiency_factor;
> +	if (clk_factor) {
> +		mode_clk_rate *= clk_factor;
> +		do_div(mode_clk_rate, 100);
> +	}
> +
> +	return mode_clk_rate;
> +}
> +
>   /**
>    * _dpu_core_perf_calc_bw() - to calculate BW per crtc
>    * @perf_cfg: performance configuration
> @@ -75,28 +95,21 @@ static u64 _dpu_core_perf_calc_clk(const struct dpu_perf_cfg *perf_cfg,
>   	struct drm_plane *plane;
>   	struct dpu_plane_state *pstate;
>   	struct drm_display_mode *mode;
> -	u64 crtc_clk;
> -	u32 clk_factor;
> +	u64 mode_clk;
>   
>   	mode = &state->adjusted_mode;
>   
> -	crtc_clk = (u64)mode->vtotal * mode->hdisplay * drm_mode_vrefresh(mode);
> +	mode_clk = (u64)mode->vtotal * mode->hdisplay * drm_mode_vrefresh(mode);
>   
>   	drm_atomic_crtc_for_each_plane(plane, crtc) {
>   		pstate = to_dpu_plane_state(plane->state);
>   		if (!pstate)
>   			continue;
>   
> -		crtc_clk = max(pstate->plane_clk, crtc_clk);
> -	}
> -
> -	clk_factor = perf_cfg->clk_inefficiency_factor;
> -	if (clk_factor) {
> -		crtc_clk *= clk_factor;
> -		do_div(crtc_clk, 100);
> +		mode_clk = max(pstate->plane_clk, mode_clk);
>   	}
>   
> -	return crtc_clk;
> +	return dpu_core_perf_adjusted_mode_clk(mode_clk, perf_cfg);
>   }
>   
>   static struct dpu_kms *_dpu_crtc_get_kms(struct drm_crtc *crtc)
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.h
> index d2f21d34e501..3740bc97422c 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.h
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.h
> @@ -54,6 +54,9 @@ struct dpu_core_perf {
>   	u32 fix_core_ab_vote;
>   };
>   
> +u64 dpu_core_perf_adjusted_mode_clk(u64 clk_rate,
> +				    const struct dpu_perf_cfg *perf_cfg);
> +
>   int dpu_core_perf_crtc_check(struct drm_crtc *crtc,
>   		struct drm_crtc_state *state);
>   
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> index 0714936d8835..5e3c34fed63b 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> @@ -1501,6 +1501,7 @@ static enum drm_mode_status dpu_crtc_mode_valid(struct drm_crtc *crtc,
>   						const struct drm_display_mode *mode)
>   {
>   	struct dpu_kms *dpu_kms = _dpu_crtc_get_kms(crtc);
> +	u64 adjusted_mode_clk;
>   
>   	/* if there is no 3d_mux block we cannot merge LMs so we cannot
>   	 * split the large layer into 2 LMs, filter out such modes
> @@ -1508,6 +1509,17 @@ static enum drm_mode_status dpu_crtc_mode_valid(struct drm_crtc *crtc,
>   	if (!dpu_kms->catalog->caps->has_3d_merge &&
>   	    mode->hdisplay > dpu_kms->catalog->caps->max_mixer_width)
>   		return MODE_BAD_HVALUE;
> +
> +	adjusted_mode_clk = dpu_core_perf_adjusted_mode_clk(mode->clock,
> +							    dpu_kms->perf.perf_cfg);
> +
> +	/*
> +	 * The given mode, adjusted for the perf clock factor, should not exceed
> +	 * the max core clock rate
> +	 */
> +	if (dpu_kms->perf.max_core_clk_rate < adjusted_mode_clk * 1000)
> +		return MODE_CLOCK_HIGH;
> +
>   	/*
>   	 * max crtc width is equal to the max mixer width * 2 and max height is 4K
>   	 */
> 
> ---
> base-commit: db76003ade5953d4a83c2bdc6e15c2d1c33e7350
> change-id: 20250506-filter-modes-c60b4332769f
> 
> Best regards,

Re: [PATCH v2] drm/msm/dpu: Filter modes based on adjusted mode clock

[Konrad Dybcio]

On 10/20/25 4:05 PM, neil.armstrong@linaro.org wrote:
> Hi,
> 
> On 5/7/25 03:38, Jessica Zhang wrote:
>> Filter out modes that have a clock rate greater than the max core clock
>> rate when adjusted for the perf clock factor
>>
>> This is especially important for chipsets such as QCS615 that have lower
>> limits for the MDP max core clock.
>>
>> Since the core CRTC clock is at least the mode clock (adjusted for the
>> perf clock factor) [1], the modes supported by the driver should be less
>> than the max core clock rate.
>>
>> [1] https://elixir.bootlin.com/linux/v6.12.4/source/drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c#L83
>>
>> Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
>> Signed-off-by: Jessica Zhang <jessica.zhang@oss.qualcomm.com>
> 
> This change breaks the T14s OLED display, no modes are reported on the eDP connector.
> msm_dpu ae01000.display-controller: [drm:update_display_info.part.0 [drm]] [CONNECTOR:41:eDP-1] Assigning EDID-1.4 digital sink color depth as 10 bpc.
> msm_dpu ae01000.display-controller: [drm:update_display_info.part.0 [drm]] [CONNECTOR:41:eDP-1] ELD monitor
> msm_dpu ae01000.display-controller: [drm:update_display_info.part.0 [drm]] [CONNECTOR:41:eDP-1] ELD size 20, SAD count 0
> [drm:drm_mode_object_put.part.0 [drm]] OBJ ID: 113 (1)
> msm_dpu ae01000.display-controller: [drm:drm_mode_prune_invalid [drm]] Rejected mode: "2880x1800": 120 652260 2880 2912 2920 2980 1800 1808 1816 1824 0x48 0x9 (CLOCK_HIGH)
> msm_dpu ae01000.display-controller: [drm:drm_mode_prune_invalid [drm]] Rejected mode: "2880x1800": 60 652260 2880 2888 2920 2980 1800 1808 1816 3648 0x40 0x9 (CLOCK_HIGH)
> 
> With this reverted on v6.18-rc, display works again.

https://lore.kernel.org/linux-arm-msm/20250923-modeclk-fix-v2-1-01fcd0b2465a@oss.qualcomm.com/

Konrad

Re: [PATCH v2] drm/msm/dpu: Filter modes based on adjusted mode clock

[Neil Armstrong]

Hi,

On 10/20/25 16:15, Konrad Dybcio wrote:
> On 10/20/25 4:05 PM, neil.armstrong@linaro.org wrote:
>> Hi,
>>
>> On 5/7/25 03:38, Jessica Zhang wrote:
>>> Filter out modes that have a clock rate greater than the max core clock
>>> rate when adjusted for the perf clock factor
>>>
>>> This is especially important for chipsets such as QCS615 that have lower
>>> limits for the MDP max core clock.
>>>
>>> Since the core CRTC clock is at least the mode clock (adjusted for the
>>> perf clock factor) [1], the modes supported by the driver should be less
>>> than the max core clock rate.
>>>
>>> [1] https://elixir.bootlin.com/linux/v6.12.4/source/drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c#L83
>>>
>>> Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
>>> Signed-off-by: Jessica Zhang <jessica.zhang@oss.qualcomm.com>
>>
>> This change breaks the T14s OLED display, no modes are reported on the eDP connector.
>> msm_dpu ae01000.display-controller: [drm:update_display_info.part.0 [drm]] [CONNECTOR:41:eDP-1] Assigning EDID-1.4 digital sink color depth as 10 bpc.
>> msm_dpu ae01000.display-controller: [drm:update_display_info.part.0 [drm]] [CONNECTOR:41:eDP-1] ELD monitor
>> msm_dpu ae01000.display-controller: [drm:update_display_info.part.0 [drm]] [CONNECTOR:41:eDP-1] ELD size 20, SAD count 0
>> [drm:drm_mode_object_put.part.0 [drm]] OBJ ID: 113 (1)
>> msm_dpu ae01000.display-controller: [drm:drm_mode_prune_invalid [drm]] Rejected mode: "2880x1800": 120 652260 2880 2912 2920 2980 1800 1808 1816 1824 0x48 0x9 (CLOCK_HIGH)
>> msm_dpu ae01000.display-controller: [drm:drm_mode_prune_invalid [drm]] Rejected mode: "2880x1800": 60 652260 2880 2888 2920 2980 1800 1808 1816 3648 0x40 0x9 (CLOCK_HIGH)
>>
>> With this reverted on v6.18-rc, display works again.
> 
> https://lore.kernel.org/linux-arm-msm/20250923-modeclk-fix-v2-1-01fcd0b2465a@oss.qualcomm.com/

Thanks,
Neil

> 
> Konrad

Re: [PATCH v2] drm/msm/dpu: Filter modes based on adjusted mode clock

[neil.armstrong]

On 5/7/25 03:38, Jessica Zhang wrote:
> Filter out modes that have a clock rate greater than the max core clock
> rate when adjusted for the perf clock factor
> 
> This is especially important for chipsets such as QCS615 that have lower
> limits for the MDP max core clock.
> 
> Since the core CRTC clock is at least the mode clock (adjusted for the
> perf clock factor) [1], the modes supported by the driver should be less
> than the max core clock rate.
> 
> [1] https://elixir.bootlin.com/linux/v6.12.4/source/drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c#L83
> 
> Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> Signed-off-by: Jessica Zhang <jessica.zhang@oss.qualcomm.com>
> ---
> Changes in v2:
> - *crtc_clock -> *mode_clock (Dmitry)
> - Changed adjusted_mode_clk check to use multiplication (Dmitry)
> - Switch from quic_* email to OSS email
> - Link to v1: https://lore.kernel.org/lkml/20241212-filter-mode-clock-v1-1-f4441988d6aa@quicinc.com/
> ---
>   drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c | 35 ++++++++++++++++++---------
>   drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.h |  3 +++
>   drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c      | 12 +++++++++
>   3 files changed, 39 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c
> index 0fb5789c60d0..13cc658065c5 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c
> @@ -31,6 +31,26 @@ enum dpu_perf_mode {
>   	DPU_PERF_MODE_MAX
>   };
>   
> +/**
> + * dpu_core_perf_adjusted_mode_clk - Adjust given mode clock rate according to
> + *   the perf clock factor.
> + * @crtc_clk_rate - Unadjusted mode clock rate
> + * @perf_cfg: performance configuration
> + */
> +u64 dpu_core_perf_adjusted_mode_clk(u64 mode_clk_rate,
> +				    const struct dpu_perf_cfg *perf_cfg)
> +{
> +	u32 clk_factor;
> +
> +	clk_factor = perf_cfg->clk_inefficiency_factor;
> +	if (clk_factor) {
> +		mode_clk_rate *= clk_factor;
> +		do_div(mode_clk_rate, 100);
> +	}
> +
> +	return mode_clk_rate;
> +}
> +
>   /**
>    * _dpu_core_perf_calc_bw() - to calculate BW per crtc
>    * @perf_cfg: performance configuration
> @@ -75,28 +95,21 @@ static u64 _dpu_core_perf_calc_clk(const struct dpu_perf_cfg *perf_cfg,
>   	struct drm_plane *plane;
>   	struct dpu_plane_state *pstate;
>   	struct drm_display_mode *mode;
> -	u64 crtc_clk;
> -	u32 clk_factor;
> +	u64 mode_clk;
>   
>   	mode = &state->adjusted_mode;
>   
> -	crtc_clk = (u64)mode->vtotal * mode->hdisplay * drm_mode_vrefresh(mode);
> +	mode_clk = (u64)mode->vtotal * mode->hdisplay * drm_mode_vrefresh(mode);
>   
>   	drm_atomic_crtc_for_each_plane(plane, crtc) {
>   		pstate = to_dpu_plane_state(plane->state);
>   		if (!pstate)
>   			continue;
>   
> -		crtc_clk = max(pstate->plane_clk, crtc_clk);
> -	}
> -
> -	clk_factor = perf_cfg->clk_inefficiency_factor;
> -	if (clk_factor) {
> -		crtc_clk *= clk_factor;
> -		do_div(crtc_clk, 100);
> +		mode_clk = max(pstate->plane_clk, mode_clk);
>   	}
>   
> -	return crtc_clk;
> +	return dpu_core_perf_adjusted_mode_clk(mode_clk, perf_cfg);
>   }
>   
>   static struct dpu_kms *_dpu_crtc_get_kms(struct drm_crtc *crtc)
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.h
> index d2f21d34e501..3740bc97422c 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.h
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.h
> @@ -54,6 +54,9 @@ struct dpu_core_perf {
>   	u32 fix_core_ab_vote;
>   };
>   
> +u64 dpu_core_perf_adjusted_mode_clk(u64 clk_rate,
> +				    const struct dpu_perf_cfg *perf_cfg);
> +
>   int dpu_core_perf_crtc_check(struct drm_crtc *crtc,
>   		struct drm_crtc_state *state);
>   
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> index 0714936d8835..5e3c34fed63b 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> @@ -1501,6 +1501,7 @@ static enum drm_mode_status dpu_crtc_mode_valid(struct drm_crtc *crtc,
>   						const struct drm_display_mode *mode)
>   {
>   	struct dpu_kms *dpu_kms = _dpu_crtc_get_kms(crtc);
> +	u64 adjusted_mode_clk;
>   
>   	/* if there is no 3d_mux block we cannot merge LMs so we cannot
>   	 * split the large layer into 2 LMs, filter out such modes
> @@ -1508,6 +1509,17 @@ static enum drm_mode_status dpu_crtc_mode_valid(struct drm_crtc *crtc,
>   	if (!dpu_kms->catalog->caps->has_3d_merge &&
>   	    mode->hdisplay > dpu_kms->catalog->caps->max_mixer_width)
>   		return MODE_BAD_HVALUE;
> +
> +	adjusted_mode_clk = dpu_core_perf_adjusted_mode_clk(mode->clock,
> +							    dpu_kms->perf.perf_cfg);
> +
> +	/*
> +	 * The given mode, adjusted for the perf clock factor, should not exceed
> +	 * the max core clock rate
> +	 */
> +	if (dpu_kms->perf.max_core_clk_rate < adjusted_mode_clk * 1000)
> +		return MODE_CLOCK_HIGH;

This test doesn't take in account if the mode is for a bonded DSI mode, which
is the same mode on 2 interfaces doubled, but it's valid since we could literally
set both modes separately. In bonded DSI this mode_clk must be again divided bv 2
in addition to the fix:
https://lore.kernel.org/linux-arm-msm/20250923-modeclk-fix-v2-1-01fcd0b2465a@oss.qualcomm.com/

I'm trying to find a correct way to handle that, I have tried that:
===========================><========================================
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
index 48c3aef1cfc2..6aa5db1996e3 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
@@ -1684,8 +1684,10 @@ static int dpu_crtc_atomic_check(struct drm_crtc *crtc,
  static enum drm_mode_status dpu_crtc_mode_valid(struct drm_crtc *crtc,
  						const struct drm_display_mode *mode)
  {
+	struct drm_encoder *encoder = get_encoder_from_crtc(crtc);
  	struct dpu_kms *dpu_kms = _dpu_crtc_get_kms(crtc);
  	u64 adjusted_mode_clk;
+	unsigned int intfs;

  	/* if there is no 3d_mux block we cannot merge LMs so we cannot
  	 * split the large layer into 2 LMs, filter out such modes
@@ -1700,12 +1702,18 @@ static enum drm_mode_status dpu_crtc_mode_valid(struct drm_crtc *crtc,
  	if (dpu_kms->catalog->caps->has_3d_merge)
  		adjusted_mode_clk /= 2;

+	intfs = dpu_encoder_get_intf_count(encoder);
+	if (intfs)
+		adjusted_mode_clk /= intfs;
+
  	/*
  	 * The given mode, adjusted for the perf clock factor, should not exceed
  	 * the max core clock rate
  	 */
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
index 3dd202e0ce94..862239b7d4bc 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
@@ -2892,6 +2892,23 @@ enum dpu_intf_mode dpu_encoder_get_intf_mode(struct drm_encoder *encoder)
  	return INTF_MODE_NONE;
  }

+/**
+ * dpu_encoder_get_intf_count - get interface count of the given encoder
+ * @encoder: Pointer to drm encoder object
+ */
+unsigned int dpu_encoder_get_intf_count(struct drm_encoder *encoder)
+{
+	struct dpu_encoder_virt *dpu_enc = NULL;
+
+	if (!encoder) {
+		DPU_ERROR("invalid encoder\n");
+		return 0;
+	}
+	dpu_enc = to_dpu_encoder_virt(encoder);
+
+	return dpu_enc->num_phys_encs;
+}
+
  /**
   * dpu_encoder_helper_get_cwb_mask - get CWB blocks mask for the DPU encoder
   * @phys_enc: Pointer to physical encoder structure
diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h
index ca1ca2e51d7e..f10ad297b379 100644
--- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h
+++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.h
@@ -58,6 +58,8 @@ int dpu_encoder_wait_for_tx_complete(struct drm_encoder *drm_encoder);

  enum dpu_intf_mode dpu_encoder_get_intf_mode(struct drm_encoder *encoder);

+unsigned int dpu_encoder_get_intf_count(struct drm_encoder *encoder);
+
  void dpu_encoder_virt_runtime_resume(struct drm_encoder *encoder);

  uint32_t dpu_encoder_get_clones(struct drm_encoder *drm_enc);
====================================><========================================

But this doesn't work since the crtc hasn't been associated to the encoder yet....

Neil

> +
>   	/*
>   	 * max crtc width is equal to the max mixer width * 2 and max height is 4K
>   	 */
> 
> ---
> base-commit: db76003ade5953d4a83c2bdc6e15c2d1c33e7350
> change-id: 20250506-filter-modes-c60b4332769f
> 
> Best regards,

Re: [PATCH v2] drm/msm/dpu: Filter modes based on adjusted mode clock

[Dmitry Baryshkov]

On Tue, Oct 28, 2025 at 09:42:57AM +0100, neil.armstrong@linaro.org wrote:
> On 5/7/25 03:38, Jessica Zhang wrote:
> > Filter out modes that have a clock rate greater than the max core clock
> > rate when adjusted for the perf clock factor
> > 
> > This is especially important for chipsets such as QCS615 that have lower
> > limits for the MDP max core clock.
> > 
> > Since the core CRTC clock is at least the mode clock (adjusted for the
> > perf clock factor) [1], the modes supported by the driver should be less
> > than the max core clock rate.
> > 
> > [1] https://elixir.bootlin.com/linux/v6.12.4/source/drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c#L83
> > 
> > Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> > Signed-off-by: Jessica Zhang <jessica.zhang@oss.qualcomm.com>
> > ---
> > Changes in v2:
> > - *crtc_clock -> *mode_clock (Dmitry)
> > - Changed adjusted_mode_clk check to use multiplication (Dmitry)
> > - Switch from quic_* email to OSS email
> > - Link to v1: https://lore.kernel.org/lkml/20241212-filter-mode-clock-v1-1-f4441988d6aa@quicinc.com/
> > ---
> >   drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c | 35 ++++++++++++++++++---------
> >   drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.h |  3 +++
> >   drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c      | 12 +++++++++
> >   3 files changed, 39 insertions(+), 11 deletions(-)
> > 
> 
> This test doesn't take in account if the mode is for a bonded DSI mode, which
> is the same mode on 2 interfaces doubled, but it's valid since we could literally
> set both modes separately. In bonded DSI this mode_clk must be again divided bv 2
> in addition to the fix:
> https://lore.kernel.org/linux-arm-msm/20250923-modeclk-fix-v2-1-01fcd0b2465a@oss.qualcomm.com/

From the docs:

         * Since this function is both called from the check phase of an atomic
         * commit, and the mode validation in the probe paths it is not allowed
         * to look at anything else but the passed-in mode, and validate it
         * against configuration-invariant hardware constraints. Any further
         * limits which depend upon the configuration can only be checked in
         * @mode_fixup or @atomic_check.

Additionally, I don't think it is correct to divide mode_clk by two. In
the end, the DPU processes the mode in a single pass, so the perf
constrains applies as is, without additional dividers.


> I'm trying to find a correct way to handle that, I have tried that:
> ===========================><========================================
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> index 48c3aef1cfc2..6aa5db1996e3 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c

-- 
With best wishes
Dmitry

Re: [PATCH v2] drm/msm/dpu: Filter modes based on adjusted mode clock

[Neil Armstrong]

On 10/28/25 20:52, Dmitry Baryshkov wrote:
> On Tue, Oct 28, 2025 at 09:42:57AM +0100, neil.armstrong@linaro.org wrote:
>> On 5/7/25 03:38, Jessica Zhang wrote:
>>> Filter out modes that have a clock rate greater than the max core clock
>>> rate when adjusted for the perf clock factor
>>>
>>> This is especially important for chipsets such as QCS615 that have lower
>>> limits for the MDP max core clock.
>>>
>>> Since the core CRTC clock is at least the mode clock (adjusted for the
>>> perf clock factor) [1], the modes supported by the driver should be less
>>> than the max core clock rate.
>>>
>>> [1] https://elixir.bootlin.com/linux/v6.12.4/source/drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c#L83
>>>
>>> Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
>>> Signed-off-by: Jessica Zhang <jessica.zhang@oss.qualcomm.com>
>>> ---
>>> Changes in v2:
>>> - *crtc_clock -> *mode_clock (Dmitry)
>>> - Changed adjusted_mode_clk check to use multiplication (Dmitry)
>>> - Switch from quic_* email to OSS email
>>> - Link to v1: https://lore.kernel.org/lkml/20241212-filter-mode-clock-v1-1-f4441988d6aa@quicinc.com/
>>> ---
>>>    drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c | 35 ++++++++++++++++++---------
>>>    drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.h |  3 +++
>>>    drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c      | 12 +++++++++
>>>    3 files changed, 39 insertions(+), 11 deletions(-)
>>>
>>
>> This test doesn't take in account if the mode is for a bonded DSI mode, which
>> is the same mode on 2 interfaces doubled, but it's valid since we could literally
>> set both modes separately. In bonded DSI this mode_clk must be again divided bv 2
>> in addition to the fix:
>> https://lore.kernel.org/linux-arm-msm/20250923-modeclk-fix-v2-1-01fcd0b2465a@oss.qualcomm.com/
> 
>  From the docs:
> 
>           * Since this function is both called from the check phase of an atomic
>           * commit, and the mode validation in the probe paths it is not allowed
>           * to look at anything else but the passed-in mode, and validate it
>           * against configuration-invariant hardware constraints. Any further
>           * limits which depend upon the configuration can only be checked in
>           * @mode_fixup or @atomic_check.
> 
> Additionally, I don't think it is correct to divide mode_clk by two. In
> the end, the DPU processes the mode in a single pass, so the perf
> constrains applies as is, without additional dividers.

Sorry but this is not correct, the current check means nothing. If you
enable 2 separate DSI outputs or enable then in bonded mode, the DPU
processes it the same so the bonded doubled mode should be valid.

The difference between separate or bonded DSI DPU-wise is only the
synchronisation of vsyncs between interfaces.

So this check against the max frequency means nothing and should be
removed, but we should solely rely on the bandwidth calculation instead.

Neil

> 
> 
>> I'm trying to find a correct way to handle that, I have tried that:
>> ===========================><========================================
>> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
>> index 48c3aef1cfc2..6aa5db1996e3 100644
>> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
>> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> 

Re: [PATCH v2] drm/msm/dpu: Filter modes based on adjusted mode clock

[Dmitry Baryshkov]

On Wed, Oct 29, 2025 at 10:40:25AM +0100, Neil Armstrong wrote:
> On 10/28/25 20:52, Dmitry Baryshkov wrote:
> > On Tue, Oct 28, 2025 at 09:42:57AM +0100, neil.armstrong@linaro.org wrote:
> > > On 5/7/25 03:38, Jessica Zhang wrote:
> > > > Filter out modes that have a clock rate greater than the max core clock
> > > > rate when adjusted for the perf clock factor
> > > > 
> > > > This is especially important for chipsets such as QCS615 that have lower
> > > > limits for the MDP max core clock.
> > > > 
> > > > Since the core CRTC clock is at least the mode clock (adjusted for the
> > > > perf clock factor) [1], the modes supported by the driver should be less
> > > > than the max core clock rate.
> > > > 
> > > > [1] https://elixir.bootlin.com/linux/v6.12.4/source/drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c#L83
> > > > 
> > > > Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> > > > Signed-off-by: Jessica Zhang <jessica.zhang@oss.qualcomm.com>
> > > > ---
> > > > Changes in v2:
> > > > - *crtc_clock -> *mode_clock (Dmitry)
> > > > - Changed adjusted_mode_clk check to use multiplication (Dmitry)
> > > > - Switch from quic_* email to OSS email
> > > > - Link to v1: https://lore.kernel.org/lkml/20241212-filter-mode-clock-v1-1-f4441988d6aa@quicinc.com/
> > > > ---
> > > >    drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c | 35 ++++++++++++++++++---------
> > > >    drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.h |  3 +++
> > > >    drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c      | 12 +++++++++
> > > >    3 files changed, 39 insertions(+), 11 deletions(-)
> > > > 
> > > 
> > > This test doesn't take in account if the mode is for a bonded DSI mode, which
> > > is the same mode on 2 interfaces doubled, but it's valid since we could literally
> > > set both modes separately. In bonded DSI this mode_clk must be again divided bv 2
> > > in addition to the fix:
> > > https://lore.kernel.org/linux-arm-msm/20250923-modeclk-fix-v2-1-01fcd0b2465a@oss.qualcomm.com/
> > 
> >  From the docs:
> > 
> >           * Since this function is both called from the check phase of an atomic
> >           * commit, and the mode validation in the probe paths it is not allowed
> >           * to look at anything else but the passed-in mode, and validate it
> >           * against configuration-invariant hardware constraints. Any further
> >           * limits which depend upon the configuration can only be checked in
> >           * @mode_fixup or @atomic_check.
> > 
> > Additionally, I don't think it is correct to divide mode_clk by two. In
> > the end, the DPU processes the mode in a single pass, so the perf
> > constrains applies as is, without additional dividers.
> 
> Sorry but this is not correct, the current check means nothing. If you
> enable 2 separate DSI outputs or enable then in bonded mode, the DPU
> processes it the same so the bonded doubled mode should be valid.
> 
> The difference between separate or bonded DSI DPU-wise is only the
> synchronisation of vsyncs between interfaces.

I think there is some sort of confusion. It might be on my side. Please
correct me if I'm wrong.

Each CRTC requires certain MDP clock rate to function: to process pixel
data, for scanout, etc. This is captured in dpu_core_perf.c. The patch
in question verifies that the mode can actually be set, that MDP can
function at the required clock rate. Otherwise we end up in a situation
when the driver lists a particular mode, but then the atomic_check
rejects that mode.

With that in mind, there is a difference between independent and bonded
DSI panels: bonded panels use single CRTC, while independent panels use
two different CRTCs. As such (again, please correct me if I'm wrong),
we need 2x MDP clock for a single CRTC.

> So this check against the max frequency means nothing and should be
> removed, but we should solely rely on the bandwidth calculation instead.

We need both. If you have a particular usecase which fails, lets discuss
it:

- 2 DSI panels, resolution WxH, N Hz, the mode uses l LMs, m DSC units
  and foo bar baz to output.

- The dpu_crtc_mode_valid() calculates the clock ABC, which is more than
  the max value of DEF

- The actual modesetting results in a clock GHI, which is less than DEF

> 
> Neil
> 
> > 
> > 
> > > I'm trying to find a correct way to handle that, I have tried that:
> > > ===========================><========================================
> > > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> > > index 48c3aef1cfc2..6aa5db1996e3 100644
> > > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> > > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> > 
> 

-- 
With best wishes
Dmitry

Re: [PATCH v2] drm/msm/dpu: Filter modes based on adjusted mode clock

[Neil Armstrong]

On 10/29/25 13:30, Dmitry Baryshkov wrote:
> On Wed, Oct 29, 2025 at 10:40:25AM +0100, Neil Armstrong wrote:
>> On 10/28/25 20:52, Dmitry Baryshkov wrote:
>>> On Tue, Oct 28, 2025 at 09:42:57AM +0100, neil.armstrong@linaro.org wrote:
>>>> On 5/7/25 03:38, Jessica Zhang wrote:
>>>>> Filter out modes that have a clock rate greater than the max core clock
>>>>> rate when adjusted for the perf clock factor
>>>>>
>>>>> This is especially important for chipsets such as QCS615 that have lower
>>>>> limits for the MDP max core clock.
>>>>>
>>>>> Since the core CRTC clock is at least the mode clock (adjusted for the
>>>>> perf clock factor) [1], the modes supported by the driver should be less
>>>>> than the max core clock rate.
>>>>>
>>>>> [1] https://elixir.bootlin.com/linux/v6.12.4/source/drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c#L83
>>>>>
>>>>> Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
>>>>> Signed-off-by: Jessica Zhang <jessica.zhang@oss.qualcomm.com>
>>>>> ---
>>>>> Changes in v2:
>>>>> - *crtc_clock -> *mode_clock (Dmitry)
>>>>> - Changed adjusted_mode_clk check to use multiplication (Dmitry)
>>>>> - Switch from quic_* email to OSS email
>>>>> - Link to v1: https://lore.kernel.org/lkml/20241212-filter-mode-clock-v1-1-f4441988d6aa@quicinc.com/
>>>>> ---
>>>>>     drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c | 35 ++++++++++++++++++---------
>>>>>     drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.h |  3 +++
>>>>>     drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c      | 12 +++++++++
>>>>>     3 files changed, 39 insertions(+), 11 deletions(-)
>>>>>
>>>>
>>>> This test doesn't take in account if the mode is for a bonded DSI mode, which
>>>> is the same mode on 2 interfaces doubled, but it's valid since we could literally
>>>> set both modes separately. In bonded DSI this mode_clk must be again divided bv 2
>>>> in addition to the fix:
>>>> https://lore.kernel.org/linux-arm-msm/20250923-modeclk-fix-v2-1-01fcd0b2465a@oss.qualcomm.com/
>>>
>>>   From the docs:
>>>
>>>            * Since this function is both called from the check phase of an atomic
>>>            * commit, and the mode validation in the probe paths it is not allowed
>>>            * to look at anything else but the passed-in mode, and validate it
>>>            * against configuration-invariant hardware constraints. Any further
>>>            * limits which depend upon the configuration can only be checked in
>>>            * @mode_fixup or @atomic_check.
>>>
>>> Additionally, I don't think it is correct to divide mode_clk by two. In
>>> the end, the DPU processes the mode in a single pass, so the perf
>>> constrains applies as is, without additional dividers.
>>
>> Sorry but this is not correct, the current check means nothing. If you
>> enable 2 separate DSI outputs or enable then in bonded mode, the DPU
>> processes it the same so the bonded doubled mode should be valid.
>>
>> The difference between separate or bonded DSI DPU-wise is only the
>> synchronisation of vsyncs between interfaces.
> 
> I think there is some sort of confusion. It might be on my side. Please
> correct me if I'm wrong.
> 
> Each CRTC requires certain MDP clock rate to function: to process pixel
> data, for scanout, etc. This is captured in dpu_core_perf.c. The patch
> in question verifies that the mode can actually be set, that MDP can
> function at the required clock rate. Otherwise we end up in a situation
> when the driver lists a particular mode, but then the atomic_check
> rejects that mode.

A CRTC will be associated to 1 or multiple LMs, the LM is the actual block
you want to check for frequency. Speaking of CRTC means nothing for the DPU.

We should basically run a lightweight version of dpu_rm_reserve() in mode_valid,
and check against all the assigned blocks to see if we can handle the mode.

But is it worth it ? What did the original patch solve exactly ?

Do we have formal proof about which max clock frequency a complete HW setup is able to support ?

> 
> With that in mind, there is a difference between independent and bonded
> DSI panels: bonded panels use single CRTC, while independent panels use
> two different CRTCs. As such (again, please correct me if I'm wrong),
> we need 2x MDP clock for a single CRTC.

Any mode can use 1 or multiple LMs, in independent or bonded DSI. As I
said the bonded DSI with a 2x mode will lead to __exactly__ the same setup
as 2 independed DSI displays. And in bonded mode, you'll always have 2 LMs.

> 
>> So this check against the max frequency means nothing and should be
>> removed, but we should solely rely on the bandwidth calculation instead.
> 
> We need both. If you have a particular usecase which fails, lets discuss
> it:
> 
> - 2 DSI panels, resolution WxH, N Hz, the mode uses l LMs, m DSC units
>    and foo bar baz to output.
> 
> - The dpu_crtc_mode_valid() calculates the clock ABC, which is more than
>    the max value of DEF
> 
> - The actual modesetting results in a clock GHI, which is less than DEF

I don't understand what you need, in the current form the mode_valid will
accept the 2 DSI displays as independent, while using them as bonded will
use the exact same HW setup (resolution WxH, N Hz, the mode uses l LMs, m DSC units)
but the mode_valid with rejects it.

Neil

> 
>>
>> Neil
>>
>>>
>>>
>>>> I'm trying to find a correct way to handle that, I have tried that:
>>>> ===========================><========================================
>>>> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
>>>> index 48c3aef1cfc2..6aa5db1996e3 100644
>>>> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
>>>> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
>>>
>>
> 

Re: [PATCH v2] drm/msm/dpu: Filter modes based on adjusted mode clock

[Dmitry Baryshkov]

On 29/10/2025 14:49, Neil Armstrong wrote:
> On 10/29/25 13:30, Dmitry Baryshkov wrote:
>> On Wed, Oct 29, 2025 at 10:40:25AM +0100, Neil Armstrong wrote:
>>> On 10/28/25 20:52, Dmitry Baryshkov wrote:
>>>> On Tue, Oct 28, 2025 at 09:42:57AM +0100, neil.armstrong@linaro.org 
>>>> wrote:
>>>>> On 5/7/25 03:38, Jessica Zhang wrote:
>>>>>> Filter out modes that have a clock rate greater than the max core 
>>>>>> clock
>>>>>> rate when adjusted for the perf clock factor
>>>>>>
>>>>>> This is especially important for chipsets such as QCS615 that have 
>>>>>> lower
>>>>>> limits for the MDP max core clock.
>>>>>>
>>>>>> Since the core CRTC clock is at least the mode clock (adjusted for 
>>>>>> the
>>>>>> perf clock factor) [1], the modes supported by the driver should 
>>>>>> be less
>>>>>> than the max core clock rate.
>>>>>>
>>>>>> [1] https://elixir.bootlin.com/linux/v6.12.4/source/drivers/gpu/ 
>>>>>> drm/msm/disp/dpu1/dpu_core_perf.c#L83
>>>>>>
>>>>>> Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
>>>>>> Signed-off-by: Jessica Zhang <jessica.zhang@oss.qualcomm.com>
>>>>>> ---
>>>>>> Changes in v2:
>>>>>> - *crtc_clock -> *mode_clock (Dmitry)
>>>>>> - Changed adjusted_mode_clk check to use multiplication (Dmitry)
>>>>>> - Switch from quic_* email to OSS email
>>>>>> - Link to v1: https://lore.kernel.org/lkml/20241212-filter-mode- 
>>>>>> clock-v1-1-f4441988d6aa@quicinc.com/
>>>>>> ---
>>>>>>     drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c | 35 +++++++++++ 
>>>>>> +++++++---------
>>>>>>     drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.h |  3 +++
>>>>>>     drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c      | 12 +++++++++
>>>>>>     3 files changed, 39 insertions(+), 11 deletions(-)
>>>>>>
>>>>>
>>>>> This test doesn't take in account if the mode is for a bonded DSI 
>>>>> mode, which
>>>>> is the same mode on 2 interfaces doubled, but it's valid since we 
>>>>> could literally
>>>>> set both modes separately. In bonded DSI this mode_clk must be 
>>>>> again divided bv 2
>>>>> in addition to the fix:
>>>>> https://lore.kernel.org/linux-arm-msm/20250923-modeclk-fix- 
>>>>> v2-1-01fcd0b2465a@oss.qualcomm.com/
>>>>
>>>>   From the docs:
>>>>
>>>>            * Since this function is both called from the check phase 
>>>> of an atomic
>>>>            * commit, and the mode validation in the probe paths it 
>>>> is not allowed
>>>>            * to look at anything else but the passed-in mode, and 
>>>> validate it
>>>>            * against configuration-invariant hardware constraints. 
>>>> Any further
>>>>            * limits which depend upon the configuration can only be 
>>>> checked in
>>>>            * @mode_fixup or @atomic_check.
>>>>
>>>> Additionally, I don't think it is correct to divide mode_clk by two. In
>>>> the end, the DPU processes the mode in a single pass, so the perf
>>>> constrains applies as is, without additional dividers.
>>>
>>> Sorry but this is not correct, the current check means nothing. If you
>>> enable 2 separate DSI outputs or enable then in bonded mode, the DPU
>>> processes it the same so the bonded doubled mode should be valid.
>>>
>>> The difference between separate or bonded DSI DPU-wise is only the
>>> synchronisation of vsyncs between interfaces.
>>
>> I think there is some sort of confusion. It might be on my side. Please
>> correct me if I'm wrong.
>>
>> Each CRTC requires certain MDP clock rate to function: to process pixel
>> data, for scanout, etc. This is captured in dpu_core_perf.c. The patch
>> in question verifies that the mode can actually be set, that MDP can
>> function at the required clock rate. Otherwise we end up in a situation
>> when the driver lists a particular mode, but then the atomic_check
>> rejects that mode.
> 
> A CRTC will be associated to 1 or multiple LMs, the LM is the actual block
> you want to check for frequency. Speaking of CRTC means nothing for the 
> DPU.
> 
> We should basically run a lightweight version of dpu_rm_reserve() in 
> mode_valid,
> and check against all the assigned blocks to see if we can handle the mode.
> 
> But is it worth it ? What did the original patch solve exactly ?
> 
> Do we have formal proof about which max clock frequency a complete HW 
> setup is able to support ?
> 
>>
>> With that in mind, there is a difference between independent and bonded
>> DSI panels: bonded panels use single CRTC, while independent panels use
>> two different CRTCs. As such (again, please correct me if I'm wrong),
>> we need 2x MDP clock for a single CRTC.
> 
> Any mode can use 1 or multiple LMs, in independent or bonded DSI. As I
> said the bonded DSI with a 2x mode will lead to __exactly__ the same setup
> as 2 independed DSI displays. And in bonded mode, you'll always have 2 LMs.
> 
>>
>>> So this check against the max frequency means nothing and should be
>>> removed, but we should solely rely on the bandwidth calculation instead.
>>
>> We need both. If you have a particular usecase which fails, lets discuss
>> it:
>>
>> - 2 DSI panels, resolution WxH, N Hz, the mode uses l LMs, m DSC units
>>    and foo bar baz to output.
>>
>> - The dpu_crtc_mode_valid() calculates the clock ABC, which is more than
>>    the max value of DEF
>>
>> - The actual modesetting results in a clock GHI, which is less than DEF
> 
> I don't understand what you need,

I have been asking for exact W, H, N, l, m, etc. numbers.

> in the current form the mode_valid will
> accept the 2 DSI displays as independent, while using them as bonded will
> use the exact same HW setup (resolution WxH, N Hz, the mode uses l LMs, 
> m DSC units)
> but the mode_valid with rejects it.

Which clock rate is being returned by _dpu_core_perf_calc_clk() while 
setting up two independent outputs and when setting up a single bonded 
output?
Which clock rate is being used by dpu_crtc_mode_valid() to reject the mode?

> 
> Neil
> 
>>
>>>
>>> Neil
>>>
>>>>
>>>>
>>>>> I'm trying to find a correct way to handle that, I have tried that:
>>>>> ===========================><========================================
>>>>> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c b/drivers/ 
>>>>> gpu/drm/msm/disp/dpu1/dpu_crtc.c
>>>>> index 48c3aef1cfc2..6aa5db1996e3 100644
>>>>> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
>>>>> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
>>>>
>>>
>>
> 


-- 
With best wishes
Dmitry

Re: [PATCH v2] drm/msm/dpu: Filter modes based on adjusted mode clock

[Neil Armstrong]

On 10/29/25 13:55, Dmitry Baryshkov wrote:
> On 29/10/2025 14:49, Neil Armstrong wrote:
>> On 10/29/25 13:30, Dmitry Baryshkov wrote:
>>> On Wed, Oct 29, 2025 at 10:40:25AM +0100, Neil Armstrong wrote:
>>>> On 10/28/25 20:52, Dmitry Baryshkov wrote:
>>>>> On Tue, Oct 28, 2025 at 09:42:57AM +0100, neil.armstrong@linaro.org wrote:
>>>>>> On 5/7/25 03:38, Jessica Zhang wrote:
>>>>>>> Filter out modes that have a clock rate greater than the max core clock rate when adjusted for the perf clock factor
>>>>>>> 
>>>>>>> This is especially important for chipsets such as QCS615 that have lower limits for the MDP max core clock.
>>>>>>> 
>>>>>>> Since the core CRTC clock is at least the mode clock (adjusted for the perf clock factor) [1], the modes supported by the driver should be less than the max core clock rate.
>>>>>>> 
>>>>>>> [1] https://elixir.bootlin.com/linux/v6.12.4/source/drivers/gpu/ drm/msm/disp/dpu1/dpu_core_perf.c#L83
>>>>>>> 
>>>>>>> Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org> Signed-off-by: Jessica Zhang <jessica.zhang@oss.qualcomm.com> --- Changes in v2: - *crtc_clock -> *mode_clock (Dmitry) - Changed adjusted_mode_clk check to use multiplication (Dmitry) - Switch from quic_* email to OSS email - Link to v1: https://lore.kernel.org/lkml/20241212-filter-mode- clock-v1-1-f4441988d6aa@quicinc.com/ --- drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.c | 35 +++++++++++ +++++++--------- drivers/gpu/drm/msm/disp/dpu1/dpu_core_perf.h |  3 +++ drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c      | 12 +++++++++ 3 files changed, 39 insertions(+), 11 deletions(-)
>>>>>>> 
>>>>>> 
>>>>>> This test doesn't take in account if the mode is for a bonded DSI mode, which is the same mode on 2 interfaces doubled, but it's valid since we could literally set both modes separately. In bonded DSI this mode_clk must be again divided bv 2 in addition to the fix: https://lore.kernel.org/linux-arm-msm/20250923-modeclk-fix- v2-1-01fcd0b2465a@oss.qualcomm.com/
>>>>> 
>>>>> From the docs:
>>>>> 
>>>>> * Since this function is both called from the check phase of an atomic * commit, and the mode validation in the probe paths it is not allowed * to look at anything else but the passed-in mode, and validate it * against configuration-invariant hardware constraints. Any further * limits which depend upon the configuration can only be checked in * @mode_fixup or @atomic_check.
>>>>> 
>>>>> Additionally, I don't think it is correct to divide mode_clk by two. In the end, the DPU processes the mode in a single pass, so the perf constrains applies as is, without additional dividers.
>>>> 
>>>> Sorry but this is not correct, the current check means nothing. If you enable 2 separate DSI outputs or enable then in bonded mode, the DPU processes it the same so the bonded doubled mode should be valid.
>>>> 
>>>> The difference between separate or bonded DSI DPU-wise is only the synchronisation of vsyncs between interfaces.
>>> 
>>> I think there is some sort of confusion. It might be on my side. Please correct me if I'm wrong.
>>> 
>>> Each CRTC requires certain MDP clock rate to function: to process pixel data, for scanout, etc. This is captured in dpu_core_perf.c. The patch in question verifies that the mode can actually be set, that MDP can function at the required clock rate. Otherwise we end up in a situation when the driver lists a particular mode, but then the atomic_check rejects that mode.
>> 
>> A CRTC will be associated to 1 or multiple LMs, the LM is the actual block you want to check for frequency. Speaking of CRTC means nothing for the DPU.
>> 
>> We should basically run a lightweight version of dpu_rm_reserve() in mode_valid, and check against all the assigned blocks to see if we can handle the mode.
>> 
>> But is it worth it ? What did the original patch solve exactly ?
>> 
>> Do we have formal proof about which max clock frequency a complete HW setup is able to support ?
>> 
>>> 
>>> With that in mind, there is a difference between independent and bonded DSI panels: bonded panels use single CRTC, while independent panels use two different CRTCs. As such (again, please correct me if I'm wrong), we need 2x MDP clock for a single CRTC.
>> 
>> Any mode can use 1 or multiple LMs, in independent or bonded DSI. As I said the bonded DSI with a 2x mode will lead to __exactly__ the same setup as 2 independed DSI displays. And in bonded mode, you'll always have 2 LMs.
>> 
>>> 
>>>> So this check against the max frequency means nothing and should be removed, but we should solely rely on the bandwidth calculation instead.
>>> 
>>> We need both. If you have a particular usecase which fails, lets discuss it:
>>> 
>>> - 2 DSI panels, resolution WxH, N Hz, the mode uses l LMs, m DSC units and foo bar baz to output.
>>> 
>>> - The dpu_crtc_mode_valid() calculates the clock ABC, which is more than the max value of DEF
>>> 
>>> - The actual modesetting results in a clock GHI, which is less than DEF
>> 
>> I don't understand what you need,
> 
> I have been asking for exact W, H, N, l, m, etc. numbers.

This is irrelevant, checking a frequency for a "CRTC" which doesn't always
maps 1:1 to an actual hardware is buggy.

Dividing by 2 if there's a has_3d_merge is already a hack since 2 LMs will
be associated to a CRTC. I don't see why the bonded DSI cannot have the same handling
since 2 LMs will be associated to the CRTC.

Neil

> 
>> in the current form the mode_valid will accept the 2 DSI displays as independent, while using them as bonded will use the exact same HW setup (resolution WxH, N Hz, the mode uses l LMs, m DSC units) but the mode_valid with rejects it.
> 
> Which clock rate is being returned by _dpu_core_perf_calc_clk() while setting up two independent outputs and when setting up a single bonded output? Which clock rate is being used by dpu_crtc_mode_valid() to reject the mode?
> 
>> 
>> Neil
>> 
>>> 
>>>> 
>>>> Neil
>>>> 
>>>>> 
>>>>> 
>>>>>> I'm trying to find a correct way to handle that, I have tried that: ===========================><======================================== diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c b/drivers/ gpu/drm/msm/disp/dpu1/dpu_crtc.c index 48c3aef1cfc2..6aa5db1996e3 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
>>>>> 
>>>> 
>>> 
>> 
> 
>