Re: [Freedreno] [PATCH 3/3] drm/msm/dsi: Enable DATABUS_WIDEN for DSI command mode

[Abhinav Kumar]

On 6/23/2023 12:19 AM, Marijn Suijten wrote:
> On 2023-06-22 17:01:34, Abhinav Kumar wrote:
> <snip>
> > > More interesting would be a link to the Mesa MR upstreaming this
> > > bitfield to dsi.xml [2] (which I have not found on my own yet).
> > >
> > > [2]: https://gitlab.freedesktop.org/mesa/mesa/-/blame/main/src/freedreno/registers/dsi/dsi.xml
> >
> > Thats because we havent submitted a MR yet for this on mesa.
> >
> > Generally, our team does not have legal permissions yet for mesa MRs
> > other than mesa drm because we got permissions for the modetest.
> >
> > Rob/Dmitry, can one of you pls help with the corresponding mesa MR for this?
>
> Thanks!
>
> > The xml file change was autogenerated so this patch can go in.
> <snip>
> > > >    		 *
> > > >    		 * hdisplay will be divided by 3 here to account for the fact
> > > >    		 * that DPU sends 3 bytes per pclk cycle to DSI.
> > > > +		 *
> > > > +		 * If widebus is supported, set DATABUS_WIDEN register and divide hdisplay by 6
> > > > +		 * instead of 3
> > >
> > > So this should allow us to divide pclk by 2, or have much lower latency?
> > > Otherwise it'll tick enough times to transmit the data twice.
> > >
> > > Note that I brought up the exact same concerns when you used the 3:1
> > > ratio from dsi_bpp / dsc_bpp in your pclk reduction patch (instad of the
> > > number of bits/bytes that DPU sends to DSI per pclk), but no-one has
> > > replied to my inquiry yet.
> >
> > Ideally yes, we could have done pclk/2 on uncompressed pixels but we are
> > not going to add support for widebus on DSI without DSC as that is not
> > recommended in our docs.
>
> No-one here mentioned uncompressed pixels?
>
> None of this code suddenly makes DPU send twice as many pixels/bytes to
> the DSI, yet we are enabling a feature that makes the bus twice as wide,
> so the clock can be halved *for comressed pixels*?

The concept is quite simple

one pixel per clock for uncompresssed without widebubus

2 pixels per clock for uncompressed with widebus (only enabled for DP 
not DSI)

3 bytes worth of data for compressed without widebus

6 bytes worth of data for compressed with widebus

When compression happens, we cannot quantify with pixels as the boundary 
is not defined with respect to bytes.

You brought up uncompressed in your below comment so I assumed your 
question of /2 was about uncompressed too.


> > So this cannot be done.
> >
> > We tried our best to respond and explain to all your queries both on the
> > bug and the patch but i guess it just kept coming :)
>
> Then send less patches!  As long as there is activity on the mailing
> list there'll always be questions going back and forth, and I don't
> think that's unreasonable.
>
> Unless you want to push patches into mainline without questioning.

the comments were bordering the line of becoming irrelevant to the 
patches like discussing video mode on a command mode patch when we had 
explained many many times that we did not validate them.

I dont want to add more comments to this. Lets stop discussing this and 
focus more on this patch.

> > I am going to try one more time to explain it in the documentation change.
>
> Would love to hear, why, for compressed streams, the bus is twice as
> wide yet pclk is not reduced to account for that.
>
> <snip>
> > > Can we also support widebus for uncompressed streams (sending 2 pixels
> > > of bpp=24 per pclk), and if so, is that something you want to add in the
> > > future (a comment would be nice)?
> >
> > No, we cannot support widebus on uncompressed streams on DSI so we wont
> > be adding that.
>
> And here we start talking about uncompressed pixels *separately*.  Okay,
> if it is not supported (e.g. widebus is specific to / reserved for DSC)
> it of course makes no sense to add it.
>
> - Marijn