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

[Marek Vasut]

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 
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.