[PATCH] drm/panel/panel-sitronix-st7701: Move init sequence from prepare() to enable()

[Marek Vasut]

On 8/26/23 20:33, Mimoja wrote:

Hi,

+CC Dave , he might be able to help with the last part.

> I appreciate you taking the time to respond!
> 
> On 26.08.23 17:18, Marek Vasut wrote:
>> On 8/26/23 11:55, Mimoja wrote:
>>> "The .prepare() function is typically called before the display 
>>> controller
>>> starts to transmit video data."
>>> and
>>> "After the display controller has started transmitting video data, 
>>> it's safe
>>>   to call the .enable() function."
>>
>> DSI commands are not DSI video, so this should be OK ?
> 
> You are correct, my commit message is mixing things up here. I wanted to 
> emphasize roughly the thought of
> "when enable() is called the dsi core is expected to have its clock 
> initialized". Will take note to clarify this if I succeed to
> make a case for this patch below :)

I vaguely recall there was this flag in include/drm/drm_bridge.h which 
might be related:

748 /**
749  * @pre_enable_prev_first: The bridge requires that the prev
750  * bridge @pre_enable function is called before its @pre_enable,
751  * and conversely for post_disable. This is most frequently a
752  * requirement for DSI devices which need the host to be initialised
753  * before the peripheral.
754  */
755 bool pre_enable_prev_first;

Could it be, this is what you need ?

>>> While generally fine this can lead to a fillup of the transmission 
>>> queue before
>>> the transmission is set up on certain dsi bridges.
>>> This issue can also be seen on downstream imx8m* kernels.
>>
>> Can you reproduce this with current mainline Linux or linux-next tree ?
>> I recall the display pipeline in the NXP downstream stuff is very 
>> different from mainline .
> 
> You are very much correct. The NXP downstream kernel is completely 
> different from the upstream one
> and is really a great example to show the issue (code cleaned up for 
> readability):
> 
> https://github.com/varigit/linux-imx/blob/5.15-2.0.x-imx_var01/drivers/gpu/drm/bridge/sec-dsim.c#L1368
> ```
>      ret = drm_panel_prepare(dsim->panel);
>      if (unlikely(ret)) [...]
> 
>      /* config esc clock, byte clock and etc */
>      sec_mipi_dsim_config_clkctrl(dsim);
> 
>      ret = drm_panel_enable(dsim->panel);
>      if (unlikely(ret)) [...]
> 
> ```
> 
>> Which SoC does have this problem ?
> Sadly I don't have any SoCs available which would work perfectly with 
> linux-next, let alone are confirmed affected :/
> 
> I were able to make my Kingway Panel work (Custom one and so far 
> unsupported by the st7701 driver) with this
> patch on downstream 5.4 and 5.15 imx8mn as well as on a raspberry pi CM4 
> with 6.1. However raspberrypi/linux brings
> SPI support to the st7701 driver which should not affect this but I 
> would just like to document it here.
> I could not find any success story with st7701 and the rpi on 6.1 online 
> after a short search (and only one
> reference with 5.10 which seems to me a bit different in a short 
> comparison)  but again I can only offer
> circumstantial evidence. Sorry :/

Maybe Dave can help with this part .