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
CRTC check stage to unify the 2 cases, and ease later revisement for
quad-pipe change.
Signed-off-by: Jun Nie <jun.nie@linaro.org>
---
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);
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;
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);
+ 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.0On 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 <jun.nie@linaro.org>
> ---
> 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
Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com> 于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 <jun.nie@linaro.org>
> > ---
> > 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?
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
On Wed, Mar 11, 2026 at 06:13:19PM +0800, Jun Nie wrote:
> Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com> 于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 <jun.nie@linaro.org>
> > > ---
> > > 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
Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com> 于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 <jun.nie@linaro.org>
> > ---
> > 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
© 2016 - 2026 Red Hat, Inc.