linux-kernel - Re: [PATCH v4 16/16] drm/msm/dpu: Enable quad-pipe for DSC and dual-DSI 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: <CABymUCMnYy-L2Bd6dgYzPTB+qzpFLbU-LyCLDKxCSWy19x_A5g@mail.gmail.com>
Date: Fri, 17 Jan 2025 15:32:44 +0800
From: Jun Nie <jun.nie@...aro.org>
To: Dmitry Baryshkov <dmitry.baryshkov@...aro.org>
Cc: Rob Clark <robdclark@...il.com>, Abhinav Kumar <quic_abhinavk@...cinc.com>, 
	Sean Paul <sean@...rly.run>, Marijn Suijten <marijn.suijten@...ainline.org>, 
	David Airlie <airlied@...il.com>, Simona Vetter <simona@...ll.ch>, linux-arm-msm@...r.kernel.org, 
	dri-devel@...ts.freedesktop.org, freedreno@...ts.freedesktop.org, 
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH v4 16/16] drm/msm/dpu: Enable quad-pipe for DSC and
 dual-DSI case

Dmitry Baryshkov <dmitry.baryshkov@...aro.org> 于2025年1月16日周四 16:32写道：
>
> On Thu, Jan 16, 2025 at 03:26:05PM +0800, Jun Nie wrote:
> > Request 4 mixers and 4 DSC for the case that both dual-DSI and DSC are
> > enabled.
>
> Why? What is the issue that you are solving?

    To support high-resolution cases that exceed the width limitation of
    a pair of SSPPs, or scenarios that surpass the maximum MDP clock rate,
    additional pipes are necessary to enable parallel data processing
    within the SSPP width constraints and MDP clock rate.

    Request 4 mixers and 4 DSCs for high-resolution cases where both DSC
    and dual interfaces are enabled. More use cases can be incorporated
    later if quad-pipe capabilities are required.

>
> > 4 pipes are preferred for dual DSI case for it is power optimal
> > for DSC.
> >
> > Signed-off-by: Jun Nie <jun.nie@...aro.org>
> > ---
> >  drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.c         |  2 +-
> >  drivers/gpu/drm/msm/disp/dpu1/dpu_crtc.h         |  6 ++---
> >  drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c      | 29 ++++++++++++++++++------
> >  drivers/gpu/drm/msm/disp/dpu1/dpu_encoder_phys.h |  2 +-
> >  drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h   |  2 +-
> >  drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h      |  2 +-
> >  6 files changed, 29 insertions(+), 14 deletions(-)
> >
>
> > @@ -664,15 +664,20 @@ static struct msm_display_topology dpu_encoder_get_topology(
> >
> >       /* Datapath topology selection
> >        *
> > -      * Dual display
> > +      * Dual display without DSC
> >        * 2 LM, 2 INTF ( Split display using 2 interfaces)
> >        *
> > +      * Dual display with DSC
> > +      * 2 LM, 2 INTF ( Split display using 2 interfaces)
> > +      * 4 LM, 2 INTF ( Split display using 2 interfaces)
> > +      *
> >        * Single display
> >        * 1 LM, 1 INTF
> >        * 2 LM, 1 INTF (stream merge to support high resolution interfaces)
> >        *
> >        * Add dspps to the reservation requirements if ctm is requested
> >        */
> > +
>
> irrlevant extra line, please drop.
>
> >       if (intf_count == 2)
> >               topology.num_lm = 2;
> >       else if (!dpu_kms->catalog->caps->has_3d_merge)
> > @@ -691,10 +696,20 @@ static struct msm_display_topology dpu_encoder_get_topology(
> >                * 2 DSC encoders, 2 layer mixers and 1 interface
> >                * this is power optimal and can drive up to (including) 4k
> >                * screens
> > +              * But for dual display case, we prefer 4 layer mixers. Because
> > +              * the resolution is always high in the case and 4 DSCs are more
> > +              * power optimal.
>
> I think this part is thought about in a wrong way. If it was just about
> power efficiency, we wouldn't have to add quad pipe support.
> Please correct me if I'm wrong, but I think it is about the maximum
> width supported by a particular topology being too low for a requested
> resolution. So, if there is a DSC and mode width is higher than 5120
> (8.x+) / 4096 ( <= 7.x), then we have to use quad pipe. Likewise if
> there is no DSC in play, the limitation should be 2 * max_mixer_width.

Both width limitation and clock rate contribute to pipe number decision.
To support high resolution, the MDP clock may be required to overclock
to a higher rate than the safe rate. Current implementation does not
support checking clock rate when deciding topology. We can add the
clock rate check later with a new patch.
>
> >                */
> > -             topology.num_dsc = 2;
> > -             topology.num_lm = 2;
> > -             topology.num_intf = 1;
> > +
> > +             if (intf_count == 2) {
> > +                     topology.num_dsc = dpu_kms->catalog->dsc_count >= 4 ? 4 : 2;
>
> This assumes that the driver can support 2:2:2. Is it the case?

The code falls back to 2:2:2 case here. But I guess 2:2:2 does not work yet.
How about below code for DSC case?

                 if (intf_count == 2 && dpu_kms->catalog->dsc_count >= 4) {
                         topology.num_dsc = 4;
                         topology.num_lm = 4;
                         topology.num_intf = 2;
                 } else {
                         topology.num_dsc = 2;
                         topology.num_lm = 2;
                         topology.num_intf = 1;
                 }

>
> > +                     topology.num_lm = topology.num_dsc;
> > +                     topology.num_intf = 2;
> > +             } else {
> > +                     topology.num_dsc = 2;
> > +                     topology.num_lm = 2;
> > +                     topology.num_intf = 1;
> > +             }
> >       }
> >
> >       return topology;
>
> --
> With best wishes
> Dmitry