Re: [RFC][PATCH] drm: bridge: dw-mipi-dsi: Call modeset in modeset callback

[Marek Vasut]

On 4/21/24 1:09 PM, Ondřej Jirman wrote:
> Hi,

Hi,

> On Sun, Apr 21, 2024 at 02:22:35AM GMT, Marek Vasut wrote:
> > Doing modeset in .atomic_pre_enable callback instead of dedicated .mode_set
> > callback does not seem right. Undo this change, which was added as part of
>
> Actually no. If anything, mode_set callback should be dropped entirely:
>
> See https://elixir.bootlin.com/linux/latest/source/include/drm/drm_bridge.h#L231
>
> It's deprecated, and enable callback should just use adjusted_mode:
>
>      This is deprecated, do not use! New drivers shall set their mode in the
>      &drm_bridge_funcs.atomic_enable operation.

This mentions new drivers ?

> > commit 05aa61334592 ("drm: bridge: dw-mipi-dsi: Fix enable/disable of DSI
> > controller") as it breaks STM32MP15xx LTDC scanout (DSI)->TC358762 DSI-to-DPI
> > bridge->PT800480 DPI panel pipeline. The original fix for HX8394 panel likely
> > requires HX8394 panel side fix instead.
>
> There's nothing wrong with the panel driver. And that commit is not fixing issue
> with the panel driver, just like the subject hints at. Look at the referenced
> commit, at "Before:" sequence specifically.
>
> dw_mipi_dsi_mode_set may be named *_mode_set or whatever, but it's basically
> an enable function that turns on clocks, initalizes phy, etc. etc.
>
> https://elixir.bootlin.com/linux/latest/source/drivers/gpu/drm/bridge/synopsys/dw-mipi-dsi.c#L998
>
> And if you check "Before:" sequence, you'll see that .mode_set callback is just
> called once at boot and never again. And it's atomic(_pre)_enable/atomic(_post)_disable
> callbacks that actually are called in ballanced way to enable/disable the
> controller repeatedly ever after.
>
> Function dw_mipi_dsi_bridge_post_atomic_disable is the inverse of
> dw_mipi_dsi_mode_set, it undoes everything that dw_mipi_dsi_mode_set does.
>
> You need to find root cause for your issue on STM32MP15xx instead of reverting
> fixes for resource use bugs in this driver.

Actually, reverting commit 05aa61334592 ("drm: bridge: dw-mipi-dsi: Fix 
enable/disable of DSI controller") makes the STM32MP15xx work again like 
it used to since Linux 5.10 or so, so that commit breaks existing 
working use case.

It seems it is sufficient to revert only this part of the commit to make 
the STM32MP15xx work as it used to, do you have any idea why ?