| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <6381550a-4c1a-429d-b6c1-8c7ae77bf325@linaro.org>
Date: Tue, 28 Oct 2025 09:42:57 +0100
From: neil.armstrong@...aro.org
To: Jessica Zhang <jessica.zhang@....qualcomm.com>,
Rob Clark <robdclark@...il.com>, Dmitry Baryshkov <lumag@...nel.org>,
Sean Paul <sean@...rly.run>, Marijn Suijten <marijn.suijten@...ainline.org>,
David Airlie <airlied@...il.com>, Simona Vetter <simona@...ll.ch>
Cc: Abhinav Kumar <quic_abhinavk@...cinc.com>, linux-arm-msm@...r.kernel.org,
dri-devel@...ts.freedesktop.org, freedreno@...ts.freedesktop.org,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2] drm/msm/dpu: Filter modes based on adjusted mode clock
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@...aro.org>
> Signed-off-by: Jessica Zhang <jessica.zhang@....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,
Powered by blists - more mailing lists