linux-kernel - Re: [PATCH v13 10/12] drm/msm/dpu: support SSPP assignment for quad-pipe case

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: <akxp2lpikvslacz72gvp3ctzdgsrbfnth4ugqfhbxvflmr6ssu@f4ebi3tyz3am>
Date: Fri, 1 Aug 2025 14:06:31 +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 v13 10/12] drm/msm/dpu: support SSPP assignment for
 quad-pipe case

On Fri, Aug 01, 2025 at 09:19:41AM +0800, Jun Nie wrote:
> Dmitry Baryshkov <dmitry.baryshkov@....qualcomm.com> 于2025年8月1日周五 01:08写道：
> >
> > On 31/07/2025 18:37, Jun Nie wrote:
> > > Dmitry Baryshkov <dmitry.baryshkov@....qualcomm.com> 于2025年7月31日周四 22:22写道：
> > >>
> > >> On 31/07/2025 13:52, Jun Nie wrote:
> > >>> Dmitry Baryshkov <dmitry.baryshkov@....qualcomm.com> 于2025年7月31日周四 02:52写道：
> > >>>>
> > >>>> On Mon, Jul 28, 2025 at 09:14:34PM +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 | 160 ++++++++++++++++++------------
> > >>>>>    1 file changed, 99 insertions(+), 61 deletions(-)
> > >>>>>
> > >>>>> diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
> > >>>>> index 55429f29a4b95594771d930efe42aaa4126f6f07..e1e16a8d5ac55ba52a0f460d62901dced65e3a9e 100644
> > >>>>> --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
> > >>>>> +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c
> > >>>>> @@ -959,6 +959,30 @@ static int dpu_plane_is_multirect_parallel_capable(struct dpu_hw_sspp *sspp,
> > >>>>>    }
> > >>>>>
> > >>>>>
> > >>>>> +static bool dpu_plane_get_single_pipe_in_stage(struct dpu_plane_state *pstate,
> > >>>>> +                                            struct dpu_sw_pipe **single_pipe,
> > >>>>> +                                            struct dpu_sw_pipe_cfg **single_pipe_cfg,
> > >>>>> +                                            int stage_index)
> > >>>>> +{
> > >>>>> +     int pipe_idx, i, valid_pipe = 0;
> > >>>>> +
> > >>>>> +     for (i = 0; i < PIPES_PER_STAGE; i++) {
> > >>>>
> > >>>> Why do you need to loop here? Is there a case when pipe 0 is not
> > >>>> assigned, but pipe 1 is?
> > >>>
> > >>> Loop the pipe in a stage to count the valid pipes. If there are 2 valid
> > >>> pipes in stage of the current plane, this function will return false.
> > >>> Or you prefer the below coding?
> > >>>
> > >>> 1029         pipe_idx = stage_index * PIPES_PER_STAGE;
> > >>> 1030         if (drm_rect_width(&pstate->pipe_cfg[pipe_idx].src_rect) != 0 &&
> > >>> 1031             drm_rect_width(&pstate->pipe_cfg[pipe_idx +
> > >>> 1].src_rect) == 0) {
> > >>
> > >> Yes, this is better from my POV. But the bigger question is why do you
> > >> need it at all? What is wrong with the existing check? Can it be that
> > >> pipe0 is not used, but pipe1 is used?
> > >
> > > There is no case that pipe0 is not used, but pipe1 is used. Existing check
> > > does not filter the plane which contains single pipe in a stage, which can
> > > be a candidate for SSPP sharing. If the stage contains 2 valid pipes or
> > > no valid pipes, it is skipped in dpu_plane_try_multirect_shared(), or it is
> > > skipped to be stored in prev_adjacent_plane_state[].
> >
> > Oh, I see, you need to check both pipes because you might need to skip
> > it completely. I'd really prefer to have more explicit code:
> >
> > - check for pipe0, skip this part of the plane if there is none
> > - check for pipe1, if there is none, it's a candidate for sharing.
> 
> Yeah, this is the logic of the current implementation. So the your only
> concern is the loop. Will remove the loop as above code.

Ok.


-- 
With best wishes
Dmitry