Message-ID: <0e90460a-333b-42c4-8c0a-956c209c4077@ideasonboard.com>
Date: Tue, 3 Feb 2026 14:51:08 +0200
From: Tomi Valkeinen <tomi.valkeinen@...asonboard.com>
To: Luca Ceresoli <luca.ceresoli@...tlin.com>
Cc: dri-devel@...ts.freedesktop.org, linux-kernel@...r.kernel.org,
 Hiago De Franco <hiagofranco@...il.com>,
 Francesco Dolcini <francesco@...cini.it>, Inki Dae <inki.dae@...sung.com>,
 Jagan Teki <jagan@...rulasolutions.com>,
 Marek Szyprowski <m.szyprowski@...sung.com>,
 Andrzej Hajda <andrzej.hajda@...el.com>,
 Neil Armstrong <neil.armstrong@...aro.org>, Robert Foss <rfoss@...nel.org>,
 Laurent Pinchart <Laurent.pinchart@...asonboard.com>,
 Jonas Karlman <jonas@...boo.se>, Jernej Skrabec <jernej.skrabec@...il.com>,
 Maarten Lankhorst <maarten.lankhorst@...ux.intel.com>,
 Maxime Ripard <mripard@...nel.org>, Thomas Zimmermann <tzimmermann@...e.de>,
 David Airlie <airlied@...il.com>, Simona Vetter <simona@...ll.ch>,
 Aradhya Bhatia <a-bhatia1@...com>, Dmitry Baryshkov <lumag@...nel.org>
Subject: Re: [PATCH] drm/bridge: samsung-dsim: Fix init order

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@...il.com>
>> Signed-off-by: Tomi Valkeinen <tomi.valkeinen@...asonboard.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

Powered by blists - more mailing lists