[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <d86495d1-4d18-4809-a0a2-53ff26406e9f@quicinc.com>
Date: Thu, 8 May 2025 11:38:23 -0700
From: Jessica Zhang <quic_jesszhan@...cinc.com>
To: Jun Nie <jun.nie@...aro.org>, Rob Clark <robdclark@...il.com>,
"Abhinav
Kumar" <quic_abhinavk@...cinc.com>,
Sean Paul <sean@...rly.run>,
"Marijn
Suijten" <marijn.suijten@...ainline.org>,
David Airlie <airlied@...il.com>, Simona Vetter <simona@...ll.ch>,
Maarten Lankhorst
<maarten.lankhorst@...ux.intel.com>,
Maxime Ripard <mripard@...nel.org>,
Thomas Zimmermann <tzimmermann@...e.de>,
Dmitry Baryshkov <lumag@...nel.org>
CC: <linux-arm-msm@...r.kernel.org>, <dri-devel@...ts.freedesktop.org>,
<freedreno@...ts.freedesktop.org>, <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v9 02/14] drm/msm/dpu: check every pipe per capability
On 5/6/2025 8:47 AM, Jun Nie wrote:
> The capability stored in sblk and pipe_hw_caps is checked only for
> SSPP of the first pipe in the pair with current implementation. That
> of the 2nd pipe, r_pipe, is not checked and may violate hardware
> capability. Move requirement check to dpu_plane_atomic_check_pipe()
> for the check of every pipe.
>
> Fixes: ("dbbf57dfd04e6 drm/msm/dpu: split dpu_plane_atomic_check()")
> Signed-off-by: Jun Nie <jun.nie@...aro.org>
> Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@...aro.org>
Hi Jun,
Just wanted to note that this has already been merged in msm-next [1] so
I think you can drop this in the next revision.
But, FWIW
Reviewed-by: Jessica Zhang <quic_jesszhan@...cinc.com>
Thanks,
Jessica Zhang
[1]
https://gitlab.freedesktop.org/drm/msm/-/commit/bcaa391e177c06a58c5f3cd18484a317d40239aa
> ---
> drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 71 ++++++++++++++++---------------
> 1 file changed, 36 insertions(+), 35 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
> index af3e541f60c303eb5212524e877129359b5ca98c..aeb90c287245d6aaa18b9f280d1e628ee6ed74f5 100644
> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
> @@ -729,12 +729,40 @@ static int dpu_plane_check_inline_rotation(struct dpu_plane *pdpu,
> static int dpu_plane_atomic_check_pipe(struct dpu_plane *pdpu,
> struct dpu_sw_pipe *pipe,
> struct dpu_sw_pipe_cfg *pipe_cfg,
> - const struct msm_format *fmt,
> - const struct drm_display_mode *mode)
> + const struct drm_display_mode *mode,
> + struct drm_plane_state *new_plane_state)
> {
> uint32_t min_src_size;
> struct dpu_kms *kms = _dpu_plane_get_kms(&pdpu->base);
> int ret;
> + const struct msm_format *fmt;
> + uint32_t supported_rotations;
> + const struct dpu_sspp_cfg *pipe_hw_caps;
> + const struct dpu_sspp_sub_blks *sblk;
> +
> + pipe_hw_caps = pipe->sspp->cap;
> + sblk = pipe->sspp->cap->sblk;
> +
> + /*
> + * We already have verified scaling against platform limitations.
> + * Now check if the SSPP supports scaling at all.
> + */
> + if (!sblk->scaler_blk.len &&
> + ((drm_rect_width(&new_plane_state->src) >> 16 !=
> + drm_rect_width(&new_plane_state->dst)) ||
> + (drm_rect_height(&new_plane_state->src) >> 16 !=
> + drm_rect_height(&new_plane_state->dst))))
> + return -ERANGE;
> +
> + fmt = msm_framebuffer_format(new_plane_state->fb);
> +
> + supported_rotations = DRM_MODE_REFLECT_MASK | DRM_MODE_ROTATE_0;
> +
> + if (pipe_hw_caps->features & BIT(DPU_SSPP_INLINE_ROTATION))
> + supported_rotations |= DRM_MODE_ROTATE_90;
> +
> + pipe_cfg->rotation = drm_rotation_simplify(new_plane_state->rotation,
> + supported_rotations);
>
> min_src_size = MSM_FORMAT_IS_YUV(fmt) ? 2 : 1;
>
> @@ -923,47 +951,20 @@ static int dpu_plane_atomic_check_sspp(struct drm_plane *plane,
> struct dpu_plane_state *pstate = to_dpu_plane_state(new_plane_state);
> struct dpu_sw_pipe *pipe = &pstate->pipe;
> struct dpu_sw_pipe *r_pipe = &pstate->r_pipe;
> - const struct msm_format *fmt;
> struct dpu_sw_pipe_cfg *pipe_cfg = &pstate->pipe_cfg;
> struct dpu_sw_pipe_cfg *r_pipe_cfg = &pstate->r_pipe_cfg;
> - uint32_t supported_rotations;
> - const struct dpu_sspp_cfg *pipe_hw_caps;
> - const struct dpu_sspp_sub_blks *sblk;
> int ret = 0;
>
> - pipe_hw_caps = pipe->sspp->cap;
> - sblk = pipe->sspp->cap->sblk;
> -
> - /*
> - * We already have verified scaling against platform limitations.
> - * Now check if the SSPP supports scaling at all.
> - */
> - if (!sblk->scaler_blk.len &&
> - ((drm_rect_width(&new_plane_state->src) >> 16 !=
> - drm_rect_width(&new_plane_state->dst)) ||
> - (drm_rect_height(&new_plane_state->src) >> 16 !=
> - drm_rect_height(&new_plane_state->dst))))
> - return -ERANGE;
> -
> - fmt = msm_framebuffer_format(new_plane_state->fb);
> -
> - supported_rotations = DRM_MODE_REFLECT_MASK | DRM_MODE_ROTATE_0;
> -
> - if (pipe_hw_caps->features & BIT(DPU_SSPP_INLINE_ROTATION))
> - supported_rotations |= DRM_MODE_ROTATE_90;
> -
> - pipe_cfg->rotation = drm_rotation_simplify(new_plane_state->rotation,
> - supported_rotations);
> - r_pipe_cfg->rotation = pipe_cfg->rotation;
> -
> - ret = dpu_plane_atomic_check_pipe(pdpu, pipe, pipe_cfg, fmt,
> - &crtc_state->adjusted_mode);
> + ret = dpu_plane_atomic_check_pipe(pdpu, pipe, pipe_cfg,
> + &crtc_state->adjusted_mode,
> + new_plane_state);
> if (ret)
> return ret;
>
> if (drm_rect_width(&r_pipe_cfg->src_rect) != 0) {
> - ret = dpu_plane_atomic_check_pipe(pdpu, r_pipe, r_pipe_cfg, fmt,
> - &crtc_state->adjusted_mode);
> + ret = dpu_plane_atomic_check_pipe(pdpu, r_pipe, r_pipe_cfg,
> + &crtc_state->adjusted_mode,
> + new_plane_state);
> if (ret)
> return ret;
> }
>
Powered by blists - more mailing lists