[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <50d8590c-108e-4576-904c-138b6403ac92@phytec.de>
Date: Thu, 25 Sep 2025 13:53:16 +0000
From: Jan Remmet <J.Remmet@...tec.de>
To: Tomi Valkeinen <tomi.valkeinen@...asonboard.com>, 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" <dri-devel@...ts.freedesktop.org>,
"linux-kernel@...r.kernel.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
Am 19.06.25 um 14:27 schrieb Tomi Valkeinen:
> 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>
I run also in this issue and tried as similar solution.
Tested on imx8mm-phyboard-polis with a sn65dsi83 MIPI to LVDS converter [1]
Tested-by: Jan Remmet <j.remmet@...tec.de>
This issue still persists on current master.
Best regards Jan
[1]
https://lore.kernel.org/all/20250925-wip-j-remmet-phytec-de-bspimx8m-3801_peb-av-10_with_ac209-v1-0-94f9f775ee62@phytec.de/
> ---
> 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,
Powered by blists - more mailing lists