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.

OK, will duplicate the function to customize non-virtual-plane version.
>
> >               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.

It is just for safety to ensure derferring the SSPP handling into CRTC
check stage.
The planes_changed is set in drm_atomic_helper_plane_changed() as below
call tree for every frame in kmscube test. So this enforcement does
not introduce
redundant calling. I can use another variable to trigger the deferred
SSPP handling.
But we still need to optimize drm framework or msm_atomic_check to
avoid redundant
asserting crtc_state->planes_changed later.

drm_atomic_check_only
  -> msm_atomic_check
     -> drm_atomic_helper_check
        -> drm_atomic_helper_check_planes
            -> drm_atomic_helper_plane_changed  =>
crtc_state->planes_changed is asserted.
>
> >       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.

dpu_plane_assign_resources just include the SSPP assignment, which
is moved from dpu_plane_atomic_check(). The adjancent state is handled
in dpu_plane_virtual_assign_resources()
>
> > +             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

Reply via email to