On 2025-01-17 15:32:44, Jun Nie wrote:
> Dmitry Baryshkov <dmitry.baryshkov@xxxxxxxxxx> 于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
How high is high? Some Sony phones use a bonded 2:2:2 setup.
> 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@xxxxxxxxxx>
> > > ---
> > > 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)
Are these always bonded (i.e. single display with dual-DSI link), or can it be
two independent panels?
> > > + *
> > > * 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;
What if there are other encoders that have already reserved DSC blocks, reducing
the current available number of DSC blocks? This patch seems to cover multiple
panels/interfaces on a single virtual encoder, how does one get to such a
scenario? Bonded display? If one or more DP panel is connected, do they use
their own virtual encoder, and hence miss out on logic that properly divides
available DSC blocks?
That idea is what's been holding back a quick hack to support 1:1:1
topology for sc7280 / FairPhone 5 to perform a similar workaround:
if (catalog->dsc_count < 2)
num_dsc = num_lm = 1;.
> >
> > 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?
I've been working on 2:2:2 support [1] but have stalled it to see how your
series here pans out. Doesn't look like it's heading in a compatible direction
though, going for quick wins rather than redesigning how DSC blocks are
allocated (to name one of the many topics).
[1]: https://lore.kernel.org/linux-arm-msm/20240417-drm-msm-initial-dualpipe-dsc-fixes-v1-0-78ae3ee9a697@xxxxxxxxxxxxxx/
- Marijn
>
> 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
