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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [day] [month] [year] [list]
Message-Id: <20250619-samsung-dsim-fix-v1-1-6b5de68fb115@ideasonboard.com>
Date: Thu, 19 Jun 2025 15:27:18 +0300
From: Tomi Valkeinen <tomi.valkeinen@...asonboard.com>
To: 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>
Cc: dri-devel@...ts.freedesktop.org, linux-kernel@...r.kernel.org, 
 Hiago De Franco <hiagofranco@...il.com>, 
 Francesco Dolcini <francesco@...cini.it>, 
 Tomi Valkeinen <tomi.valkeinen@...asonboard.com>
Subject: [PATCH] drm/bridge: samsung-dsim: Fix init order

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>
---
 drivers/gpu/drm/bridge/samsung-dsim.c | 29 +++++++++++++++++++----------
 1 file changed, 19 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/bridge/samsung-dsim.c b/drivers/gpu/drm/bridge/samsung-dsim.c
index f2f666b27d2d..cec383d8946d 100644
--- a/drivers/gpu/drm/bridge/samsung-dsim.c
+++ b/drivers/gpu/drm/bridge/samsung-dsim.c
@@ -1473,22 +1473,31 @@ static void samsung_dsim_atomic_pre_enable(struct drm_bridge *bridge,
 	}
 
 	dsi->state |= DSIM_STATE_ENABLED;
-
-	/*
-	 * For Exynos-DSIM the downstream bridge, or panel are expecting
-	 * the host initialization during DSI transfer.
-	 */
-	if (!samsung_dsim_hw_is_exynos(dsi->plat_data->hw_type)) {
-		ret = samsung_dsim_init(dsi);
-		if (ret)
-			return;
-	}
 }
 
 static void samsung_dsim_atomic_enable(struct drm_bridge *bridge,
 				       struct drm_atomic_state *state)
 {
 	struct samsung_dsim *dsi = bridge_to_dsi(bridge);
+	int ret;
+
+	/*
+	 * The DSI bridge may have already been initialized in
+	 * samsung_dsim_host_transfer(). It is possible that the refclk rate has
+	 * changed after that due to the display controller configuration, and
+	 * thus we need to reinitialize the DSI bridge to ensure the correct
+	 * clock settings.
+	 */
+
+	if (dsi->state & DSIM_STATE_INITIALIZED) {
+		dsi->state &= ~DSIM_STATE_INITIALIZED;
+		samsung_dsim_disable_clock(dsi);
+		samsung_dsim_disable_irq(dsi);
+	}
+
+	ret = samsung_dsim_init(dsi);
+	if (ret)
+		return;
 
 	samsung_dsim_set_display_mode(dsi);
 	samsung_dsim_set_display_enable(dsi, true);

---
base-commit: 7872997c048e989c7689c2995d230fdca7798000
change-id: 20250619-samsung-dsim-fix-58c8ec8193e9

Best regards,
-- 
Tomi Valkeinen <tomi.valkeinen@...asonboard.com>