Re: [RFC PATCH 11/13] drm/msm/disp/dpu1: Add support for DSC in topology

[Dmitry Baryshkov]

On 29/05/2021 01:23, abhinavk@xxxxxxxxxxxxxx wrote:
> On 2021-05-28 03:39, Dmitry Baryshkov wrote:
> > On 21/05/2021 15:49, Vinod Koul wrote:
> > > For DSC to work we typically need a 2,2,1 configuration. This should
> > > suffice for resolutions upto 4k. For more resolutions like 8k this won't
> > > work.
> > >
> > > Furthermore, we can use 1 DSC encoder in lesser resulutions, but that is
> > > not power efficient according to Abhinav, so it is recommended to always
> > > use 2 encoders.
> >
> > Not power efficient because the second DSC would also be powered on or
> > because single DSC enc would consume more power than two DSCs?
>
> I havent got through the series yet but just thought of answering this,
>
> So before coming to the power aspects of this, hard-coding was done for 
> the foll reasons:
>
> -> We do not have a topology DTSI property in upstream and will probably 
> not have as well till
> other features are added which support all the topologies
> -> The DSC panel which is being upstreamed as part of this series is 
> working with this 2,2,1 topology
> downstream ( dual lm, dual DSC encoders, single DSI ). Other topologies 
> have not been tried on it yet
> -> There needs to be a better approach to handle all topologies once we 
> have added support for them.
> It can be either a DTSI property if others agree OR some helper API 
> which will determine the best topology
> based on various factors. Till then, since this will be the only DSC 
> panel we are adding support for
> I thought we can start with a fixed topology for now.
>
> Coming to the power aspect, I only recommended 2-2-1 here because using 
> two mixers is better power wise
> as it will split the width/2. We can also do 2-1-1 by enabling 3D mux 
> but this panel has not been validated
> with a single DSC. So to keep things simple with what has been 
> validated, I thought we can go ahead with
> 2-2-1 for now.
>
> So rather than giving too much importance to the power aspect of it, the 
> other reasons should also
> be highlighted here as the main reason and the commit text should give 
> these details as well.

Sounds reasonable now, thank you!


> > > So for now we blindly create 2,2,1 topology when DSC is enabled
> > >
> > > Co-developed-by: Abhinav Kumar <abhinavk@xxxxxxxxxxxxxx>
> > > Signed-off-by: Abhinav Kumar <abhinavk@xxxxxxxxxxxxxx>
> > > Signed-off-by: Vinod Koul <vkoul@xxxxxxxxxx>
> > > ---
> > >   drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c | 14 ++++++++++++++
> > >   1 file changed, 14 insertions(+)
> > >
> > > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c 
> > > b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> > > index 18cb1274a8bb..bffb40085c67 100644
> > > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> > > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_encoder.c
> > > @@ -609,8 +609,22 @@ static struct msm_display_topology 
> > > dpu_encoder_get_topology(
> > >       topology.num_enc = 0;
> > >       topology.num_intf = intf_count;
> > >   +    drm_enc = &dpu_enc->base;
> > > +    priv = drm_enc->dev->dev_private;
> > > +    if (priv && priv->dsc) {
> > > +        /* In case of Display Stream Compression DSC, we would use
> > > +         * 2 encoders, 2 line mixers and 1 interface
> > > +         * this is power optimal and can drive upto (including) 4k
> > > +         * screens
> > > +         */
> > > +        topology.num_enc = 2;
> > > +        topology.num_intf = 1;
> > > +        topology.num_lm = 2;
> > > +    }
> > > +
> > >       return topology;
> > >   }
> > > +
> > >   static int dpu_encoder_virt_atomic_check(
> > >           struct drm_encoder *drm_enc,
> > >           struct drm_crtc_state *crtc_state,


--
With best wishes
Dmitry