lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <bxual3dbzffgbe6vziwumaqpyxrnjbvnljy2pg2o5mtgixufbm@mjvg5em2xklf>
Date: Tue, 22 Jul 2025 15:04:16 +0300
From: Dmitry Baryshkov <dmitry.baryshkov@....qualcomm.com>
To: Jun Nie <jun.nie@...aro.org>
Cc: Rob Clark <robdclark@...il.com>, Abhinav Kumar <quic_abhinavk@...cinc.com>,
        Dmitry Baryshkov <lumag@...nel.org>, Sean Paul <sean@...rly.run>,
        Marijn Suijten <marijn.suijten@...ainline.org>,
        David Airlie <airlied@...il.com>, Simona Vetter <simona@...ll.ch>,
        Jessica Zhang <quic_jesszhan@...cinc.com>,
        linux-arm-msm@...r.kernel.org, dri-devel@...ts.freedesktop.org,
        freedreno@...ts.freedesktop.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v12 10/12] drm/msm/dpu: support SSPP assignment for
 quad-pipe case

On Mon, Jul 21, 2025 at 04:06:13PM +0800, Jun Nie wrote:
> Dmitry Baryshkov <dmitry.baryshkov@....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.nie@...aro.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

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ