[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CABymUCMf8LxX6VWUuzNJP+G1y3Xi5-CVYhaqLR7F=kU6ZgdcgA@mail.gmail.com>
Date: Thu, 22 Jan 2026 14:03:25 +0800
From: Jun Nie <jun.nie@...aro.org>
To: Dmitry Baryshkov <dmitry.baryshkov@....qualcomm.com>
Cc: Abhinav Kumar <abhinav.kumar@...ux.dev>, 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>,
Rob Clark <robin.clark@....qualcomm.com>, Neil Armstrong <neil.armstrong@...aro.org>,
linux-arm-msm@...r.kernel.org, dri-devel@...ts.freedesktop.org,
freedreno@...ts.freedesktop.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v17 2/4] drm/msm/dpu: Defer SSPP allocation until CRTC check
Dmitry Baryshkov <dmitry.baryshkov@....qualcomm.com> 于2026年1月21日周三 17:30写道:
>
> On Wed, Jan 21, 2026 at 04:01:51PM +0800, Jun Nie wrote:
> > Currently, plane splitting and SSPP allocation occur during the plane
> > check phase. Defer these operations until dpu_assign_plane_resources()
> > is called from the CRTC side to ensure the topology information from
> > the CRTC check is available.
>
> Why is it important? What is broken otherwise?
I see. Thanks! Will add below lines in next version.
By default, the plane check occurs before the CRTC check.
Without topology information from the CRTC, plane splitting
cannot be properly executed. Consequently, the SSPP
engine starts without a valid memory address, which triggers
an IOMMU warning.
>
> >
> > Signed-off-by: Jun Nie <jun.nie@...aro.org>
> > ---
> > drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 3 +-
> > drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 112 +++++++++++++++++++-----------
> > 2 files changed, 71 insertions(+), 44 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> > index 2d06c950e8143..debdbbe6160dd 100644
> > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> > @@ -1484,8 +1484,7 @@ static int dpu_crtc_atomic_check(struct drm_crtc *crtc,
> > return rc;
> > }
> >
> > - if (dpu_use_virtual_planes &&
> > - (crtc_state->planes_changed || crtc_state->zpos_changed)) {
> > + if (crtc_state->planes_changed || crtc_state->zpos_changed) {
> > rc = dpu_crtc_reassign_planes(crtc, crtc_state);
> > if (rc < 0)
> > return rc;
> > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
> > index 66f240ce29d07..3c629f4df461d 100644
> > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
> > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
> > @@ -1119,49 +1119,25 @@ static int dpu_plane_atomic_check(struct drm_plane *plane,
> > struct drm_plane_state *new_plane_state = drm_atomic_get_new_plane_state(state,
> > plane);
> > int ret = 0;
> > - struct dpu_plane *pdpu = to_dpu_plane(plane);
> > - struct dpu_plane_state *pstate = to_dpu_plane_state(new_plane_state);
> > - struct dpu_kms *dpu_kms = _dpu_plane_get_kms(plane);
> > - struct dpu_sw_pipe *pipe = &pstate->pipe[0];
> > - struct dpu_sw_pipe *r_pipe = &pstate->pipe[1];
> > - struct dpu_sw_pipe_cfg *pipe_cfg = &pstate->pipe_cfg[0];
> > - struct dpu_sw_pipe_cfg *r_pipe_cfg = &pstate->pipe_cfg[1];
> > - const struct drm_crtc_state *crtc_state = NULL;
> > - uint32_t max_linewidth = dpu_kms->catalog->caps->max_linewidth;
> > + struct drm_crtc_state *crtc_state = NULL;
> >
> > if (new_plane_state->crtc)
> > crtc_state = drm_atomic_get_new_crtc_state(state,
> > new_plane_state->crtc);
> >
> > - pipe->sspp = dpu_rm_get_sspp(&dpu_kms->rm, pdpu->pipe);
> > -
> > - if (!pipe->sspp)
> > - return -EINVAL;
> > -
> > ret = dpu_plane_atomic_check_nosspp(plane, new_plane_state, crtc_state);
> > if (ret)
> > return ret;
> >
> > - ret = dpu_plane_split(plane, new_plane_state, crtc_state);
> > - if (ret)
> > - return ret;
> > -
> > if (!new_plane_state->visible)
> > return 0;
> >
> > - if (!dpu_plane_try_multirect_parallel(pipe, pipe_cfg, r_pipe, r_pipe_cfg,
> > - pipe->sspp,
> > - msm_framebuffer_format(new_plane_state->fb),
> > - max_linewidth)) {
> > - DPU_DEBUG_PLANE(pdpu, "invalid " DRM_RECT_FMT " /" DRM_RECT_FMT
> > - " max_line:%u, can't use split source\n",
> > - DRM_RECT_ARG(&pipe_cfg->src_rect),
> > - DRM_RECT_ARG(&r_pipe_cfg->src_rect),
> > - max_linewidth);
> > - return -E2BIG;
> > - }
> > -
> > - return dpu_plane_atomic_check_sspp(plane, state, crtc_state);
> > + /*
> > + * To trigger the callback of dpu_assign_plane_resources() to
> > + * finish the deferred sspp check
> > + */
> > + crtc_state->planes_changed = true;
> > + return 0;
> > }
> >
> > static int dpu_plane_virtual_atomic_check(struct drm_plane *plane,
> > @@ -1186,10 +1162,6 @@ static int dpu_plane_virtual_atomic_check(struct drm_plane *plane,
> > if (ret)
> > return ret;
> >
> > - ret = dpu_plane_split(plane, plane_state, crtc_state);
> > - if (ret)
> > - return ret;
> > -
> > if (!plane_state->visible) {
> > /*
> > * resources are freed by dpu_crtc_assign_plane_resources(),
> > @@ -1261,9 +1233,9 @@ static int dpu_plane_virtual_assign_resources(struct drm_crtc *crtc,
> > struct dpu_global_state *global_state,
> > struct drm_atomic_state *state,
> > struct drm_plane_state *plane_state,
> > + const struct drm_crtc_state *crtc_state,
> > struct drm_plane_state **prev_adjacent_plane_state)
> > {
> > - const struct drm_crtc_state *crtc_state = NULL;
> > struct drm_plane *plane = plane_state->plane;
> > struct dpu_kms *dpu_kms = _dpu_plane_get_kms(plane);
> > struct dpu_rm_sspp_requirements reqs;
> > @@ -1273,10 +1245,6 @@ static int dpu_plane_virtual_assign_resources(struct drm_crtc *crtc,
> > const struct msm_format *fmt;
> > int i, ret;
> >
> > - if (plane_state->crtc)
> > - crtc_state = drm_atomic_get_new_crtc_state(state,
> > - plane_state->crtc);
> > -
> > pstate = to_dpu_plane_state(plane_state);
> > for (i = 0; i < STAGES_PER_PLANE; i++)
> > prev_adjacent_pstate[i] = prev_adjacent_plane_state[i] ?
> > @@ -1288,6 +1256,10 @@ static int dpu_plane_virtual_assign_resources(struct drm_crtc *crtc,
> > if (!plane_state->fb)
> > return -EINVAL;
> >
> > + ret = dpu_plane_split(plane, plane_state, crtc_state);
> > + if (ret)
> > + return ret;
> > +
> > fmt = msm_framebuffer_format(plane_state->fb);
> > reqs.yuv = MSM_FORMAT_IS_YUV(fmt);
> > reqs.scale = (plane_state->src_w >> 16 != plane_state->crtc_w) ||
> > @@ -1318,14 +1290,59 @@ static int dpu_plane_virtual_assign_resources(struct drm_crtc *crtc,
> > return dpu_plane_atomic_check_sspp(plane, state, crtc_state);
> > }
> >
> > +static int dpu_plane_assign_resources(struct drm_crtc *crtc,
> > + struct dpu_global_state *global_state,
> > + struct drm_atomic_state *state,
> > + struct drm_plane_state *plane_state,
> > + const struct drm_crtc_state *crtc_state,
> > + struct drm_plane_state **prev_adjacent_plane_state)
> > +{
> > + struct drm_plane *plane = plane_state->plane;
> > + struct dpu_kms *dpu_kms = _dpu_plane_get_kms(plane);
> > + struct dpu_plane_state *pstate = to_dpu_plane_state(plane_state);
> > + struct dpu_sw_pipe *pipe = &pstate->pipe[0];
> > + struct dpu_sw_pipe *r_pipe = &pstate->pipe[1];
> > + struct dpu_sw_pipe_cfg *pipe_cfg = &pstate->pipe_cfg[0];
> > + struct dpu_sw_pipe_cfg *r_pipe_cfg = &pstate->pipe_cfg[1];
> > + struct dpu_plane *pdpu = to_dpu_plane(plane);
> > + int ret;
> > +
> > + if (!plane_state->visible)
> > + return 0;
> > +
> > + pipe->sspp = dpu_rm_get_sspp(&dpu_kms->rm, pdpu->pipe);
> > + if (!pipe->sspp)
> > + return -EINVAL;
> > +
> > + ret = dpu_plane_split(plane, plane_state, crtc_state);
> > + if (ret)
> > + return ret;
> > +
> > + if (!dpu_plane_try_multirect_parallel(pipe, pipe_cfg, r_pipe, r_pipe_cfg,
> > + pipe->sspp,
> > + msm_framebuffer_format(plane_state->fb),
> > + dpu_kms->catalog->caps->max_linewidth)) {
> > + DPU_DEBUG_PLANE(pdpu, "invalid " DRM_RECT_FMT " /" DRM_RECT_FMT
> > + " max_line:%u, can't use split source\n",
> > + DRM_RECT_ARG(&pipe_cfg->src_rect),
> > + DRM_RECT_ARG(&r_pipe_cfg->src_rect),
> > + dpu_kms->catalog->caps->max_linewidth);
> > + return -E2BIG;
> > + }
> > +
> > + return dpu_plane_atomic_check_sspp(plane, state, crtc_state);
> > +}
> > +
> > int dpu_assign_plane_resources(struct dpu_global_state *global_state,
> > struct drm_atomic_state *state,
> > struct drm_crtc *crtc,
> > struct drm_plane_state **states,
> > unsigned int num_planes)
> > {
> > - unsigned int i;
> > struct drm_plane_state *prev_adjacent_plane_state[STAGES_PER_PLANE] = { NULL };
> > + const struct drm_crtc_state *crtc_state = NULL;
> > + unsigned int i;
> > + int ret;
> >
> > for (i = 0; i < num_planes; i++) {
> > struct drm_plane_state *plane_state = states[i];
> > @@ -1334,8 +1351,19 @@ int dpu_assign_plane_resources(struct dpu_global_state *global_state,
> > !plane_state->visible)
> > continue;
> >
> > - int ret = dpu_plane_virtual_assign_resources(crtc, global_state,
> > + if (plane_state->crtc)
> > + crtc_state = drm_atomic_get_new_crtc_state(state,
> > + plane_state->crtc);
> > +
> > + if (!dpu_use_virtual_planes)
> > + ret = dpu_plane_assign_resources(crtc, global_state,
> > + state, plane_state,
> > + crtc_state,
> > + prev_adjacent_plane_state);
> > + else
> > + ret = dpu_plane_virtual_assign_resources(crtc, global_state,
> > state, plane_state,
> > + crtc_state,
> > prev_adjacent_plane_state);
> > if (ret)
> > return ret;
> >
> > --
> > 2.43.0
> >
>
> --
> With best wishes
> Dmitry
Powered by blists - more mailing lists