[PATCH] drm/bridge: ti-sn65dsi83: Add and use hs_rate and lp_rate

[Adam Ford]

On Mon, Aug 1, 2022 at 11:05 AM Marek Vasut <marex at denx.de> wrote:
>
> On 8/1/22 16:55, Dave Stevenson wrote:
> > Hi Marek
>
> Hi,
>
> > On Mon, 1 Aug 2022 at 14:12, Marek Vasut <marex at denx.de> wrote:
> >>
> >> Fill in hs_rate and lp_rate to struct mipi_dsi_device for this bridge and
> >> adjust DSI input frequency calculations such that they expect the DSI host
> >> to configure HS clock according to hs_rate.
> >
> > I think this falls into another of the areas that is lacking in the DSI support.
> > hs_rate is defined as the *maximum* lane frequency in high speed
> > mode[1]. As documented there is no obligation on the DSI host to
> > choose this specific rate, just some frequency below it and above or
> > equal to that required by the pixel clock. At a system level, the link
> > frequency is often prescribed for EMC purposes.
>
> The reduced upper limit could likely be defined by a DT property, but
> that's not hard to add.
>
> > The issue is then that the SN65DSI83 is configured to use the DSI
> > clock lane as the source for the PLL generating the LVDS clock,
> > therefore with no route for the DSI peripheral to query the link
> > frequency chosen by the host, you're stuck.
>
> At least making the host pick the highest supported frequency means we
> have a well defined link frequency which either is accepted by both ends
> or not at all, instead of the current guesswork on both ends, bridge and
> host.
>
> Regarding reduction of the maximum hs_rate, see above, probably use a DT
> property. Regarding check for hs_rate below minimum possible rate,
> likely something the DSI host should do based on desired mode .
>
> If you are looking for some frequency negotiation starter, look at:
> [RFC][PATCH 0/7] drm/bridge: Add support for selecting DSI host HS clock
> from DSI bridge
>
> > SN65DSI83 also supports non-burst mode (as currently), so how would
> > this be configured now?
> > Does MIPI_DSI_MODE_VIDEO_BURST [2] oblige the DSI host to run in burst
> > mode, or say that burst mode is supported by the peripheral?
>
> My understanding is that it is the former -- if the flag is set, the DSI
> host must operate the device in burst mode.
>
> > What if
> > your DSI host doesn't support burst mode and it is optional on your
> > peripheral?
>
> The limit still applies. In the sync pulses mode, you can still run the
> link at high frequency, it's just that the DSI data lanes won't go into
> LP mode between bursts of data, they would stuff the link with NOPs
> instead, as far as I can tell.
>
> > I raised these questions and others at [3], but largely got no real answers.
> >
> >
> > The patch does exactly what you describe and has value, but without
> > definition in the frameworks of exactly how burst mode must be
> > implemented by the DSI host, I would say that it's going to cause
> > issues.
> > I am aware of users of your driver with the Raspberry Pi, but your
> > expectation isn't currently valid on the Pi as there is no definition
> > of the correct thing for the host to do.
>
> This patch actually helped me deal with stability issues on MX8M . Of

I have a board with the SN65DSI83 and I have been testing Jagan's DSIM
patch series and I noticed the SN65DSI83  throws a PLL timeout error.
What kind of stability issues were you seeing?   I'll try to pull in
this patch, and I'm happy to test and reply with 'Tested-by' if it
works.

adam

> course, the DSIM driver has to handle the case where bridge provides
> hs_rate and configure its PLL accordingly. That's a two-liner patch.