| 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
| ||
|
Message-ID: <tozhsnjg34ob4xdhzs2tzga3cghjtkpfy4672ubdyazbqvne2e@kzdhjuwmrtor>
Date: Fri, 25 Jul 2025 16:45:08 +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 Thu, Jul 24, 2025 at 09:56:21AM +0800, Jun Nie wrote:
> Dmitry Baryshkov <dmitry.baryshkov@....qualcomm.com> 于2025年7月22日周二 20:04写道：
> >
> > 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.
> >
>
> I see. So the prev_adjacent_plane_state in dpu_assign_plane_resources()
> shall be converted into an array, and map to prev_adjacent_pstate[N] in
> dpu_plane_virtual_assign_resources(). Then check new single pipe plane
> vs every member in the prev_adjacent_pstate[N] to confirm stage index is
> aligned before sharing SSPP. Right?
>
> If so, that is the optimization for the dual stage case. It does not introduce
> regression to the existing single stage case with current implementation.
> Can we just merge this patch first, then add the optimization for the
> dual stages case in new patch set? As this patch set focus on quad-pipe
> with 2 stages, without hurting existing usage cases. And it changes lots
> of code and involve rebase effort from time to time. While the optimization
> for the dual stage case will limit the change in plane, though several review
> cycle may be needed.
It's not an optimization, it's a normal flow: you have two different
stages, there should be no intersection between them. So, no, please fix
that. It will also change some bits and pieces of the logic that you
have here, hopefully making it easier to follow. For example, I don't
think you'd need dpu_plane_get_single_pipe() anymore, etc.
>
> Regards,
> 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
--
With best wishes
Dmitry
Powered by blists - more mailing lists