On Wed, Mar 11, 2026 at 06:13:19PM +0800, Jun Nie wrote:
> Dmitry Baryshkov <[email protected]> 于2026年2月14日周六 01:03写道：
> >
> > On Fri, Feb 13, 2026 at 10:54:26PM +0800, Jun Nie wrote:
> > > Currently, mapping plane to SSPP occurs during the plane check phase for
> > > non-virtual plane case. The SSPP allocation and plane mapping occurs
> > > during
> > > crtc check phase for virtual plane case. Defer these SSPP operations until
> >
> > Nit: CRTC
> >
> > > CRTC check stage to unify the 2 cases, and ease later revisement for
> > > quad-pipe change.
> > >
> > > Signed-off-by: Jun Nie <[email protected]>
> > > ---
> > > drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c | 3 +-
> > > drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 155
> > > +++++++++++++-----------------
> > > 2 files changed, 66 insertions(+), 92 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> > > b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> > > index 6bf7c46379aed..797296b14264e 100644
> > > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> > > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c
> > > @@ -1534,8 +1534,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);
> >
> > dpu_crtc_reassing_planes() starts by freeing all SSPPs. It should not be
> > used in a non-virtual-plane case. I'd suggest duplicating the function
> > and stripping out all code and data related to virtual planes.
> >
> > > 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..be1a7fcf11b81 100644
> > > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
> > > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
> > > @@ -1119,102 +1119,24 @@ 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);
> > > -}
> > > -
> > > -static int dpu_plane_virtual_atomic_check(struct drm_plane *plane,
> > > - struct drm_atomic_state *state)
> > > -{
> > > - struct drm_plane_state *plane_state =
> > > - drm_atomic_get_plane_state(state, plane);
> > > - struct drm_plane_state *old_plane_state =
> > > - drm_atomic_get_old_plane_state(state, plane);
> > > - struct dpu_plane_state *pstate = to_dpu_plane_state(plane_state);
> > > - struct drm_crtc_state *crtc_state = NULL;
> > > - int ret, i;
> > > -
> > > - if (IS_ERR(plane_state))
> > > - return PTR_ERR(plane_state);
> > > -
> > > - if (plane_state->crtc)
> > > - crtc_state = drm_atomic_get_new_crtc_state(state,
> > > -
> > > plane_state->crtc);
> > > -
> > > - ret = dpu_plane_atomic_check_nosspp(plane, plane_state, crtc_state);
> > > - 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(),
> > > - * but clean them here.
> > > - */
> > > - for (i = 0; i < PIPES_PER_PLANE; i++)
> > > - pstate->pipe[i].sspp = NULL;
> > > -
> > > - return 0;
> > > - }
> > > -
> > > /*
> > > - * Force resource reallocation if the format of FB or src/dst have
> > > - * changed. We might need to allocate different SSPP or SSPPs for
> > > this
> > > - * plane than the one used previously.
> > > + * To trigger the callback of dpu_assign_plane_resources() to
> > > + * finish the sspp assignment or allocation and check.
> > > */
> > > - if (!old_plane_state || !old_plane_state->fb ||
> > > - old_plane_state->src_w != plane_state->src_w ||
> > > - old_plane_state->src_h != plane_state->src_h ||
> > > - old_plane_state->crtc_w != plane_state->crtc_w ||
> > > - old_plane_state->crtc_h != plane_state->crtc_h ||
> > > - msm_framebuffer_format(old_plane_state->fb) !=
> > > - msm_framebuffer_format(plane_state->fb))
> > > - crtc_state->planes_changed = true;
> > > -
> > > + crtc_state->planes_changed = true;
> >
> > Why do we need to enforce this? Previously it was limited to the cases
> > when the plane has actually changed and required revalidation.
> >
> > > return 0;
> > > }
> > >
> > > @@ -1261,9 +1183,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 +1195,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 +1206,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 +1240,56 @@ 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;
> > > +
> > > + 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 +1298,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);
> >
> > This is an overkill for the non-virtual case. We don't need adjancent
> > states, we don't need the array of plane state pointers, etc.
>
> Hi Dmitry,
>
> For the array of plane state pointers, I find we have to use it in the
> non-virtual
> case too. Because the logic is moved from plane atomic_check.
> The goal is to assign SSPP for every plane and atomic_check is called for
> every plane before this patch. dpu_assign_plane_resources here is called
> from crtc side just once so all SSPP assignments should be done in one call
> with plane_state in an array.
> Or I misunderstand your comments?
I'm stating that some of the args that you pass to
dpu_plane_assign_resources() are unused and asking to remove them.
>
> Regards,
> Jun
>
> >
> > > + else
> > > + ret = dpu_plane_virtual_assign_resources(crtc,
> > > global_state,
> > > state,
> > > plane_state,
> > > + crtc_state,
> > >
> > > prev_adjacent_plane_state);
> > > if (ret)
> > > return ret;
> > > @@ -1772,7 +1747,7 @@ static const struct drm_plane_helper_funcs
> > > dpu_plane_helper_funcs = {
> > > static const struct drm_plane_helper_funcs
> > > dpu_plane_virtual_helper_funcs = {
> > > .prepare_fb = dpu_plane_prepare_fb,
> > > .cleanup_fb = dpu_plane_cleanup_fb,
> > > - .atomic_check = dpu_plane_virtual_atomic_check,
> > > + .atomic_check = dpu_plane_atomic_check,
> > > .atomic_update = dpu_plane_atomic_update,
> > > };
> > >
> > >
> > > --
> > > 2.43.0
> > >
> >
> > --
> > With best wishes
> > Dmitry
--
With best wishes
Dmitry
