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. > + 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
