Re: [PATCH] drm/bridge: samsung-dsim: Fix init order

Tomi Valkeinen
2026-02-03T12:51:15Z

Hi,

On 03/02/2026 14:42, Luca Ceresoli wrote:
> Hello Tomi,
>
> On Thu Jun 19, 2025 at 2:27 PM CEST, Tomi Valkeinen wrote:
>> The commit c9b1150a68d9 ("drm/atomic-helper: Re-order bridge chain
>> pre-enable and post-disable") changed the order of enable/disable calls.
>> Previously the calls (on imx8mm) were:
>>
>> mxsfb_crtc_atomic_enable()
>> samsung_dsim_atomic_pre_enable()
>> samsung_dsim_atomic_enable()
>>
>> now the order is:
>>
>> samsung_dsim_atomic_pre_enable()
>> mxsfb_crtc_atomic_enable()
>> samsung_dsim_atomic_enable()
>>
>> On imx8mm (possibly on imx8mp, and other platforms too) this causes two
>> issues:
>>
>> 1. The DSI PLL setup depends on a refclk, but the DSI driver does not
>> set the rate, just uses it with the rate it has. On imx8mm this refclk
>> seems to be related to the LCD controller's video clock. So, when the
>> mxsfb driver sets its video clock, DSI's refclk rate changes.
>>
>> Earlier this mxsfb_crtc_atomic_enable() set the video clock, so the PLL
>> refclk rate was set (and didn't change) in the DSI enable calls. Now the
>> rate changes between DSI's pre_enable() and enable(), but the driver
>> configures the PLL in the pre_enable().
>>
>> Thus you get a black screen on a modeset. Doing the modeset again works,
>> as the video clock rate stays the same.
>>
>> 2. The image on the screen is shifted/wrapped horizontally. I have not
>> found the exact reason for this, but the documentation seems to hint
>> that the LCD controller's pixel stream should be enabled first, before
>> setting up the DSI. This would match the change, as now the pixel stream
>> starts only after DSI driver's pre_enable().
>>
>> The main function related to this issue is samsung_dsim_init() which
>> will do the clock and link configuration. samsung_dsim_init() is
>> currently called from pre_enable(), but it is also called from
>> samsung_dsim_host_transfer() to set up the link if the peripheral driver
>> wants to send a DSI command.
>>
>> This patch fixes both issues by moving the samsung_dsim_init() call from
>> pre_enable() to enable().
>>
>> However, to deal with the case where the samsung_dsim_init() has already
>> been called from samsung_dsim_host_transfer() and the refclk rate has
>> changed, we need to make sure we re-initialize the DSI with the new rate
>> in enable(). This is achieved by clearing the DSIM_STATE_INITIALIZED
>> flag and uninitializing the clocks and irqs before calling
>> samsung_dsim_init().
>>
>> Fixes: c9b1150a68d9 ("drm/atomic-helper: Re-order bridge chain pre-enable and post-disable")
>> Reported-by: Hiago De Franco <hiagofranco@gmail.com>
>> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
>
> Is this old patch still valid, or outdated/superseded?

No, not valid, ignore it =).

The enable/disable sequence was restored to as it was before.

Tomi