On Mon, Jul 21, 2025 at 04:06:13PM +0800, Jun Nie wrote:
> Dmitry Baryshkov <dmitry.barysh...@oss.qualcomm.com> 于2025年7月19日周六 18:09写道：
> >
> > On Mon, Jul 07, 2025 at 02:18:05PM +0800, Jun Nie wrote:
> > > Currently, SSPPs are assigned to a maximum of two pipes. However,
> > > quad-pipe usage scenarios require four pipes and involve configuring
> > > two stages. In quad-pipe case, the first two pipes share a set of
> > > mixer configurations and enable multi-rect mode when certain
> > > conditions are met. The same applies to the subsequent two pipes.
> > >
> > > Assign SSPPs to the pipes in each stage using a unified method and
> > > to loop the stages accordingly.
> > >
> > > Signed-off-by: Jun Nie <jun....@linaro.org>
> > > ---
> > > drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 143
> > > +++++++++++++++++++-----------
> > > 1 file changed, 89 insertions(+), 54 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
> > > b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
> > > index
> > > 149e7066480b07f9f6d422748d89ffd6f9416f33..ecfebf7a2406d65930075cc2a4b8a8a7d40b3d3c
> > > 100644
> > > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
> > > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
> > > @@ -954,6 +954,30 @@ static int
> > > dpu_plane_is_multirect_parallel_capable(struct dpu_hw_sspp *sspp,
> > > dpu_plane_is_parallel_capable(pipe_cfg, fmt, max_linewidth);
> > > }
> > >
> > > +static bool dpu_plane_get_single_pipe(struct dpu_plane_state *pstate,
> > > + struct dpu_sw_pipe **single_pipe,
> > > + struct dpu_sw_pipe_cfg
> > > **single_pipe_cfg,
> > > + int *stage_index)
> > > +{
> > > + int stage_idx, pipe_idx, i, valid_pipe = 0;
> > > +
> > > + for (stage_idx = 0; stage_idx < STAGES_PER_PLANE; stage_idx++) {
> > > + for (i = 0; i < PIPES_PER_STAGE; i++) {
> > > + pipe_idx = stage_idx * PIPES_PER_STAGE + i;
> > > + if
> > > (drm_rect_width(&pstate->pipe_cfg[pipe_idx].src_rect) != 0) {
> > > + valid_pipe++;
> > > + if (valid_pipe > 1)
> > > + return false;
> > > +
> > > + *single_pipe = &pstate->pipe[pipe_idx];
> > > + *single_pipe_cfg =
> > > &pstate->pipe_cfg[pipe_idx];
> > > + *stage_index = stage_idx;
> > > + }
> > > + }
> > > + }
> > > +
> > > + return valid_pipe == 1;
> > > +}
> > >
> > > static int dpu_plane_atomic_check_sspp(struct drm_plane *plane,
> > > struct drm_atomic_state *state,
> > > @@ -1021,18 +1045,23 @@ static int dpu_plane_try_multirect_shared(struct
> > > dpu_plane_state *pstate,
> > > const struct msm_format *fmt,
> > > uint32_t max_linewidth)
> > > {
> > > - 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 *prev_pipe = &prev_adjacent_pstate->pipe[0];
> > > - struct dpu_sw_pipe_cfg *prev_pipe_cfg =
> > > &prev_adjacent_pstate->pipe_cfg[0];
> > > + struct dpu_sw_pipe *pipe, *prev_pipe;
> > > + struct dpu_sw_pipe_cfg *pipe_cfg, *prev_pipe_cfg;
> > > const struct msm_format *prev_fmt =
> > > msm_framebuffer_format(prev_adjacent_pstate->base.fb);
> > > + int stage_index, prev_stage_index;
> > > u16 max_tile_height = 1;
> > >
> > > - if (prev_adjacent_pstate->pipe[1].sspp != NULL ||
> > > + if (!dpu_plane_get_single_pipe(pstate, &pipe, &pipe_cfg,
> > > &stage_index))
> > > + return false;
> > > +
> > > + if (!dpu_plane_get_single_pipe(prev_adjacent_pstate, &prev_pipe,
> > > + &prev_pipe_cfg, &prev_stage_index) ||
> > > prev_pipe->multirect_mode != DPU_SSPP_MULTIRECT_NONE)
> > > return false;
> > >
> > > + if (stage_index != prev_stage_index)
> > > + return false;
> >
> > This should be handled other way around: save N pstates and then loop
> > over stage indices. If there is no rect at the corresponding stage for a
> > plane, skip assignment (and use prev_adjacent_pstate for the next plane).
>
> You mean dpu_plane_virtual_assign_resources() shall notify its caller
> dpu_assign_plane_resources() to skip updating prev_adjacent_plane_state
> if dpu_plane_try_multirect_shared() return false? If so, we can add an
> argument "bool pipe_shared" in dpu_plane_virtual_assign_resources() to
> get the status. But that is an optimization to share pipes across multiple
> planes. Can we add new patches based on current patch set later?
>
> Or my understanding is not aligned with your thoughts?
Not quite. I think we need to store all NUM_STAGES 'prev_adjancent' states and
update them as the driver loops through the stages for each plane.
>
> - Jun
> >
> > > +
> > > if (!dpu_plane_is_multirect_capable(pipe->sspp, pipe_cfg, fmt) ||
> > > !dpu_plane_is_multirect_capable(prev_pipe->sspp, prev_pipe_cfg,
> > > prev_fmt))
> > > return false;
> > > @@ -1043,11 +1072,6 @@ static int dpu_plane_try_multirect_shared(struct
> > > dpu_plane_state *pstate,
> > > if (MSM_FORMAT_IS_UBWC(prev_fmt))
> > > max_tile_height = max(max_tile_height,
> > > prev_fmt->tile_height);
> > >
> > > - r_pipe->multirect_index = DPU_SSPP_RECT_SOLO;
> > > - r_pipe->multirect_mode = DPU_SSPP_MULTIRECT_NONE;
> > > -
> > > - r_pipe->sspp = NULL;
> > > -
> > > if (dpu_plane_is_parallel_capable(pipe_cfg, fmt, max_linewidth) &&
> > > dpu_plane_is_parallel_capable(prev_pipe_cfg, prev_fmt,
> > > max_linewidth) &&
> > > (pipe_cfg->dst_rect.x1 >= prev_pipe_cfg->dst_rect.x2 ||
> > > @@ -1176,6 +1200,44 @@ static int dpu_plane_virtual_atomic_check(struct
> > > drm_plane *plane,
> > > return 0;
> > > }
> > >
> > > +static int dpu_plane_assign_resource_in_stage(struct dpu_sw_pipe *pipe,
> > > + struct dpu_sw_pipe_cfg
> > > *pipe_cfg,
> > > + struct drm_plane_state
> > > *plane_state,
> > > + struct dpu_global_state
> > > *global_state,
> > > + struct drm_crtc *crtc,
> > > + struct
> > > dpu_rm_sspp_requirements *reqs)
> > > +{
> > > + struct drm_plane *plane = plane_state->plane;
> > > + struct dpu_kms *dpu_kms = _dpu_plane_get_kms(plane);
> > > + struct dpu_sw_pipe *r_pipe = pipe + 1;
> > > + struct dpu_sw_pipe_cfg *r_pipe_cfg = pipe_cfg + 1;
> > > +
> > > + if (drm_rect_width(&pipe_cfg->src_rect) != 0) {
> > > + pipe->sspp = dpu_rm_reserve_sspp(&dpu_kms->rm,
> > > global_state, crtc, reqs);
> > > + if (!pipe->sspp)
> > > + return -ENODEV;
> > > + }
> > > +
> > > + if (drm_rect_width(&r_pipe_cfg->src_rect) != 0 &&
> > > + 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)) {
> > > + goto stage_assinged;
> > > + }
> > > +
> > > + if (drm_rect_width(&r_pipe_cfg->src_rect) != 0) {
> > > + r_pipe->sspp = dpu_rm_reserve_sspp(&dpu_kms->rm,
> > > global_state, crtc, reqs);
> > > + if (!r_pipe->sspp)
> > > + return -ENODEV;
> > > + r_pipe->multirect_index = DPU_SSPP_RECT_SOLO;
> > > + r_pipe->multirect_mode = DPU_SSPP_MULTIRECT_NONE;
> > > + }
> > > +
> > > +stage_assinged:
> > > + return 0;
> > > +}
> > > +
> > > static int dpu_plane_virtual_assign_resources(struct drm_crtc *crtc,
> > > struct dpu_global_state
> > > *global_state,
> > > struct drm_atomic_state
> > > *state,
> > > @@ -1188,11 +1250,9 @@ static int
> > > dpu_plane_virtual_assign_resources(struct drm_crtc *crtc,
> > > struct dpu_rm_sspp_requirements reqs;
> > > struct dpu_plane_state *pstate, *prev_adjacent_pstate;
> > > struct dpu_sw_pipe *pipe;
> > > - struct dpu_sw_pipe *r_pipe;
> > > struct dpu_sw_pipe_cfg *pipe_cfg;
> > > - struct dpu_sw_pipe_cfg *r_pipe_cfg;
> > > const struct msm_format *fmt;
> > > - int i;
> > > + int i, stage_id, ret;
> > >
> > > if (plane_state->crtc)
> > > crtc_state = drm_atomic_get_new_crtc_state(state,
> > > @@ -1202,11 +1262,6 @@ static int
> > > dpu_plane_virtual_assign_resources(struct drm_crtc *crtc,
> > > prev_adjacent_pstate = prev_adjacent_plane_state ?
> > > to_dpu_plane_state(prev_adjacent_plane_state) : NULL;
> > >
> > > - pipe = &pstate->pipe[0];
> > > - r_pipe = &pstate->pipe[1];
> > > - pipe_cfg = &pstate->pipe_cfg[0];
> > > - r_pipe_cfg = &pstate->pipe_cfg[1];
> > > -
> > > for (i = 0; i < PIPES_PER_PLANE; i++)
> > > pstate->pipe[i].sspp = NULL;
> > >
> > > @@ -1220,44 +1275,24 @@ static int
> > > dpu_plane_virtual_assign_resources(struct drm_crtc *crtc,
> > >
> > > reqs.rot90 = drm_rotation_90_or_270(plane_state->rotation);
> > >
> > > - if (drm_rect_width(&r_pipe_cfg->src_rect) == 0) {
> > > - if (!prev_adjacent_pstate ||
> > > - !dpu_plane_try_multirect_shared(pstate,
> > > prev_adjacent_pstate, fmt,
> > > -
> > > dpu_kms->catalog->caps->max_linewidth)) {
> > > - pipe->sspp = dpu_rm_reserve_sspp(&dpu_kms->rm,
> > > global_state, crtc, &reqs);
> > > - if (!pipe->sspp)
> > > - return -ENODEV;
> > > -
> > > - r_pipe->sspp = NULL;
> > > -
> > > - pipe->multirect_index = DPU_SSPP_RECT_SOLO;
> > > - pipe->multirect_mode = DPU_SSPP_MULTIRECT_NONE;
> > > -
> > > - r_pipe->multirect_index = DPU_SSPP_RECT_SOLO;
> > > - r_pipe->multirect_mode = DPU_SSPP_MULTIRECT_NONE;
> > > - }
> > > - } else {
> > > - pipe->sspp = dpu_rm_reserve_sspp(&dpu_kms->rm,
> > > global_state, crtc, &reqs);
> > > - if (!pipe->sspp)
> > > - return -ENODEV;
> > > -
> > > - 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)) {
> > > - /* multirect is not possible, use two SSPP blocks */
> > > - r_pipe->sspp = dpu_rm_reserve_sspp(&dpu_kms->rm,
> > > global_state, crtc, &reqs);
> > > - if (!r_pipe->sspp)
> > > - return -ENODEV;
> > > -
> > > - pipe->multirect_index = DPU_SSPP_RECT_SOLO;
> > > - pipe->multirect_mode = DPU_SSPP_MULTIRECT_NONE;
> > > + if (prev_adjacent_pstate &&
> > > + dpu_plane_try_multirect_shared(pstate, prev_adjacent_pstate,
> > > fmt,
> > > +
> > > dpu_kms->catalog->caps->max_linewidth)) {
> > > + goto assigned;
> > > + }
> > >
> > > - r_pipe->multirect_index = DPU_SSPP_RECT_SOLO;
> > > - r_pipe->multirect_mode = DPU_SSPP_MULTIRECT_NONE;
> > > - }
> > > + for (stage_id = 0; stage_id < STAGES_PER_PLANE; stage_id++) {
> > > + pipe = &pstate->pipe[stage_id * PIPES_PER_STAGE];
> > > + pipe_cfg = &pstate->pipe_cfg[stage_id * PIPES_PER_STAGE];
> > > + ret = dpu_plane_assign_resource_in_stage(pipe, pipe_cfg,
> > > + plane_state,
> > > + global_state,
> > > + crtc, &reqs);
> > > + if (ret)
> > > + return ret;
> > > }
> > >
> > > +assigned:
> > > return dpu_plane_atomic_check_sspp(plane, state, crtc_state);
> > > }
> > >
> > >
> > > --
> > > 2.34.1
> > >
> >
> > --
> > With best wishes
> > Dmitry
--
With best wishes
Dmitry
