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 <[email protected]>
>> Signed-off-by: Tomi Valkeinen <[email protected]>
>
> 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
