linux-kernel - Re: [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>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20250623100249.GA32388@francesco-nb>
Date: Mon, 23 Jun 2025 12:02:49 +0200
From: Francesco Dolcini <francesco@...cini.it>
To: Marek Szyprowski <m.szyprowski@...sung.com>
Cc: Tomi Valkeinen <tomi.valkeinen@...asonboard.com>,
	Inki Dae <inki.dae@...sung.com>,
	Jagan Teki <jagan@...rulasolutions.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>,
	dri-devel@...ts.freedesktop.org, linux-kernel@...r.kernel.org,
	Hiago De Franco <hiagofranco@...il.com>,
	Francesco Dolcini <francesco@...cini.it>
Subject: Re: [PATCH] drm/bridge: samsung-dsim: Fix init order

On Mon, Jun 23, 2025 at 11:34:34AM +0200, Marek Szyprowski wrote:
> On 19.06.2025 14:27, 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>
> 
> Seems to be working fine on all my Exynos based boards:
> 
> Tested-by: Marek Szyprowski <m.szyprowski@...sung.com>
> 
> 
> BTW, it was a long discussion how to handle the dsim initialization and 
> we agreed to keep calling samsung_dsim_init() on first dsi transfer for 
> Exynos case and from pre-enable for others:
> 
> https://lore.kernel.org/all/20221209152343.180139-11-jagan@amarulasolutions.com/
> 
> I'm not sure if changing this won't break again something, especially 
> the boards with DSI bridge or panel controlled via I2C instead of the 
> DSI commands. This has to be tested on the all supported variants of 
> this hardware.

FWIW, DSI bridges (LT8912B and SN65DSI83) controlled over I2C were
tested fine with this patch on both NXP i.MX8MP and 8MM (see Hiago
tested-by).


Francesco