[PATCH 0/3] Fixes and enhancements for Exynos (7870) DSIM bridge driver

[PATCH 0/3] Fixes and enhancements for Exynos (7870) DSIM bridge driver

[Kaustabh Chakraborty]

Since v6.17, there were a few regressive changes for the Exynos 7870
DSIM driver. These changes resulted in weird artifacts on the display,
such as random RGB channel swaps and random aberration (the occurrences
of both were mutually exclusive).

The first two commits of this patch series address the aforementioned
changes.

The third patch replaces an implicit loop for waiting for PLL
stabilization with an interrupt-based solution, which should be more
reliable. This solution was suggested by Inki Dae in a discussion of an
earlier patch series sent by me. For further details, refer to its
commit description.

Signed-off-by: Kaustabh Chakraborty <kauschluss@disroot.org>
---
Kaustabh Chakraborty (3):
      drm/bridge: samsung-dsim: move bridge init sequence to atomic_enable
      drm/bridge: samsung-dsim: enable MFLUSH_VS for Exynos 7870 DSIM
      drm/bridge: samsung-dsim: use DSIM interrupt to wait for PLL stability

 drivers/gpu/drm/bridge/samsung-dsim.c | 61 +++++++++++++++++++++++------------
 include/drm/bridge/samsung-dsim.h     |  1 +
 2 files changed, 42 insertions(+), 20 deletions(-)
---
base-commit: ca3a02fda4da8e2c1cb6baee5d72352e9e2cfaea
change-id: 20260124-exynos-dsim-fixes-5383d6a6f073

Best regards,
-- 
Kaustabh Chakraborty <kauschluss@disroot.org>

[PATCH 1/3] drm/bridge: samsung-dsim: move bridge init sequence to atomic_enable

[Kaustabh Chakraborty]

Since commit c9b1150a68d9 ("drm/atomic-helper: Re-order bridge chain
pre-enable and post-disable"), pre-enable sequence is called before the
CRTC is enabled.

This causes unintended side-effects (abberation among potentially other
things) in the display when samsung_dsim_init() is called in the
pre-enable part of the sequence. Call it in samsung_dsim_atomic_enable()
instead.

Cc: stable@vger.kernel.org # v6.17 and later
Signed-off-by: Kaustabh Chakraborty <kauschluss@disroot.org>
---
 drivers/gpu/drm/bridge/samsung-dsim.c | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/bridge/samsung-dsim.c b/drivers/gpu/drm/bridge/samsung-dsim.c
index 1d85e706c74b9..975f8b50ae660 100644
--- a/drivers/gpu/drm/bridge/samsung-dsim.c
+++ b/drivers/gpu/drm/bridge/samsung-dsim.c
@@ -1655,6 +1655,13 @@ static void samsung_dsim_atomic_pre_enable(struct drm_bridge *bridge,
 	}
 
 	dsi->state |= DSIM_STATE_ENABLED;
+}
+
+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;
 
 	/*
 	 * For Exynos-DSIM the downstream bridge, or panel are expecting
@@ -1665,12 +1672,6 @@ static void samsung_dsim_atomic_pre_enable(struct drm_bridge *bridge,
 		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);
 
 	samsung_dsim_set_display_mode(dsi);
 	samsung_dsim_set_display_enable(dsi, true);

-- 
2.52.0

[PATCH 2/3] drm/bridge: samsung-dsim: enable MFLUSH_VS for Exynos 7870 DSIM

[Kaustabh Chakraborty]

Commit a36c533ad3e1 ("drm/bridge: samsung-dsim: Always flush display
FIFO on vsync pulse") intends to enable FIFO flushing at v-sync pulse by
not setting the active-low MFLUSH_VS bit.

However, in Exynos 7870 DSIM, the MFLUSH_VS bit is active-high. There is
no publicly available documentation to the best of my knowledge, but
downstream kernel code [1] supports this claim. Enable the bit for
Exynos 7870.

Cc: stable@vger.kernel.org # v6.17 and later
Link: https://github.com/samsungexynos7870/android_kernel_samsung_exynos7870/blob/a3762bb1761aec8543fae511b087da620e24cee5/drivers/video/fbdev/exynos/decon_7870/dsim_reg_7870.c#L699 [1]
Signed-off-by: Kaustabh Chakraborty <kauschluss@disroot.org>
---
 drivers/gpu/drm/bridge/samsung-dsim.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/gpu/drm/bridge/samsung-dsim.c b/drivers/gpu/drm/bridge/samsung-dsim.c
index 975f8b50ae660..91f230953a49f 100644
--- a/drivers/gpu/drm/bridge/samsung-dsim.c
+++ b/drivers/gpu/drm/bridge/samsung-dsim.c
@@ -1089,6 +1089,13 @@ static int samsung_dsim_init_link(struct samsung_dsim *dsi)
 			reg |= DSIM_HBP_DISABLE_MODE;
 		if (dsi->mode_flags & MIPI_DSI_MODE_VIDEO_NO_HSA)
 			reg |= DSIM_HSA_DISABLE_MODE;
+
+		/*
+		 * For some hardware types, DSIM_MFLUSH_VS bit needs to be
+		 * enabled explicitly.
+		 */
+		if (dsi->plat_data->hw_type == DSIM_TYPE_EXYNOS7870)
+			reg |= DSIM_MFLUSH_VS;
 	}
 
 	if (dsi->mode_flags & MIPI_DSI_MODE_NO_EOT_PACKET)

-- 
2.52.0

[PATCH 3/3] drm/bridge: samsung-dsim: use DSIM interrupt to wait for PLL stability

[Kaustabh Chakraborty]

Stabilizing PLL needs to be waited for. This is done using a loop,
checking the PLL_STABLE bit in the status register.

DSIM fires an interrupt when the PLL is stabilized. Rely on this
functionality for stabilization wait, getting rid of the implicit loop.

This has been tested on a Galaxy J6 (Exynos 7870). Unfortunately, since
testing on all supported devices is less feasible, introduce a stop-gap
measure where the timeout has a gracious lower bound of 100
microseconds. This will (hopefully) prevent regressions due to timeout
on other devices.

Suggested-by: Inki Dae <inki.dae@samsung.com>
Link: https://lore.kernel.org/r/CAAQKjZMLMbwDVZRb5+Xb_5yz3AEP4uuzFJMuuZy9NFDu13VU5w@mail.gmail.com
Signed-off-by: Kaustabh Chakraborty <kauschluss@disroot.org>
---
 drivers/gpu/drm/bridge/samsung-dsim.c | 41 +++++++++++++++++++++++------------
 include/drm/bridge/samsung-dsim.h     |  1 +
 2 files changed, 28 insertions(+), 14 deletions(-)

diff --git a/drivers/gpu/drm/bridge/samsung-dsim.c b/drivers/gpu/drm/bridge/samsung-dsim.c
index 91f230953a49f..14d78e41886ee 100644
--- a/drivers/gpu/drm/bridge/samsung-dsim.c
+++ b/drivers/gpu/drm/bridge/samsung-dsim.c
@@ -17,6 +17,7 @@
 #include <linux/export.h>
 #include <linux/irq.h>
 #include <linux/media-bus-format.h>
+#include <linux/minmax.h>
 #include <linux/of.h>
 #include <linux/phy/phy.h>
 #include <linux/platform_device.h>
@@ -788,7 +789,7 @@ static unsigned long samsung_dsim_set_pll(struct samsung_dsim *dsi,
 {
 	const struct samsung_dsim_driver_data *driver_data = dsi->driver_data;
 	unsigned long fin, fout;
-	int timeout;
+	unsigned int timeout;
 	u8 p, s;
 	u16 m;
 	u32 reg;
@@ -849,19 +850,26 @@ static unsigned long samsung_dsim_set_pll(struct samsung_dsim *dsi,
 	if (dsi->swap_dn_dp_data)
 		reg |= DSIM_PLL_DPDNSWAP_DAT;
 
+	/*
+	 * The PLL_TIMER value is the product of the timeout delay and the APB
+	 * bus clock rate. Calcutate the timeout delay on-the-fly here.
+	 * It is assumed that the bus clock is the first clock in the provided
+	 * bulk clock data.
+	 */
+	timeout = 100;
+	fin = clk_get_rate(dsi->driver_data->clk_data[0].clk) / HZ_PER_MHZ;
+	if (fin)
+		timeout = max(dsi->driver_data->reg_values[PLL_TIMER] / fin,
+			      timeout);
+
+	reinit_completion(&dsi->pll_stabilized);
 	samsung_dsim_write(dsi, DSIM_PLLCTRL_REG, reg);
 
-	timeout = 3000;
-	do {
-		if (timeout-- == 0) {
-			dev_err(dsi->dev, "PLL failed to stabilize\n");
-			return 0;
-		}
-		if (driver_data->has_legacy_status_reg)
-			reg = samsung_dsim_read(dsi, DSIM_STATUS_REG);
-		else
-			reg = samsung_dsim_read(dsi, DSIM_LINK_STATUS_REG);
-	} while ((reg & BIT(driver_data->pll_stable_bit)) == 0);
+	if (wait_for_completion_timeout(&dsi->pll_stabilized,
+					usecs_to_jiffies(timeout))) {
+		dev_err(dsi->dev, "PLL failed to stabilize\n");
+		return 0;
+	}
 
 	dsi->hs_clock = fout;
 
@@ -1596,8 +1604,12 @@ static irqreturn_t samsung_dsim_irq(int irq, void *dev_id)
 		return IRQ_HANDLED;
 	}
 
-	if (!(status & (DSIM_INT_RX_DONE | DSIM_INT_SFR_FIFO_EMPTY |
-			DSIM_INT_PLL_STABLE)))
+	if (status & DSIM_INT_PLL_STABLE) {
+		complete(&dsi->pll_stabilized);
+		return IRQ_HANDLED;
+	}
+
+	if (!(status & (DSIM_INT_RX_DONE | DSIM_INT_SFR_FIFO_EMPTY)))
 		return IRQ_HANDLED;
 
 	if (samsung_dsim_transfer_finish(dsi))
@@ -2147,6 +2159,7 @@ int samsung_dsim_probe(struct platform_device *pdev)
 		return PTR_ERR(dsi);
 
 	init_completion(&dsi->completed);
+	init_completion(&dsi->pll_stabilized);
 	spin_lock_init(&dsi->transfer_lock);
 	INIT_LIST_HEAD(&dsi->transfer_list);
 
diff --git a/include/drm/bridge/samsung-dsim.h b/include/drm/bridge/samsung-dsim.h
index 03005e474704b..e3433da21ad08 100644
--- a/include/drm/bridge/samsung-dsim.h
+++ b/include/drm/bridge/samsung-dsim.h
@@ -123,6 +123,7 @@ struct samsung_dsim {
 	int state;
 	struct drm_property *brightness;
 	struct completion completed;
+	struct completion pll_stabilized;
 
 	spinlock_t transfer_lock; /* protects transfer_list */
 	struct list_head transfer_list;

-- 
2.52.0

Re: [PATCH 1/3] drm/bridge: samsung-dsim: move bridge init sequence to atomic_enable

[Marek Szyprowski]

On 24.01.2026 18:20, Kaustabh Chakraborty wrote:
> Since commit c9b1150a68d9 ("drm/atomic-helper: Re-order bridge chain
> pre-enable and post-disable"), pre-enable sequence is called before the
> CRTC is enabled.
>
> This causes unintended side-effects (abberation among potentially other
> things) in the display when samsung_dsim_init() is called in the
> pre-enable part of the sequence. Call it in samsung_dsim_atomic_enable()
> instead.
>
> Cc: stable@vger.kernel.org # v6.17 and later
> Signed-off-by: Kaustabh Chakraborty <kauschluss@disroot.org>

I'm not sure if this will be needed:

https://lore.kernel.org/all/20251205-drm-seq-fix-v1-0-fda68fa1b3de@ideasonboard.com/


> ---
>   drivers/gpu/drm/bridge/samsung-dsim.c | 13 +++++++------
>   1 file changed, 7 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/bridge/samsung-dsim.c b/drivers/gpu/drm/bridge/samsung-dsim.c
> index 1d85e706c74b9..975f8b50ae660 100644
> --- a/drivers/gpu/drm/bridge/samsung-dsim.c
> +++ b/drivers/gpu/drm/bridge/samsung-dsim.c
> @@ -1655,6 +1655,13 @@ static void samsung_dsim_atomic_pre_enable(struct drm_bridge *bridge,
>   	}
>   
>   	dsi->state |= DSIM_STATE_ENABLED;
> +}
> +
> +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;
>   
>   	/*
>   	 * For Exynos-DSIM the downstream bridge, or panel are expecting
> @@ -1665,12 +1672,6 @@ static void samsung_dsim_atomic_pre_enable(struct drm_bridge *bridge,
>   		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);
>   
>   	samsung_dsim_set_display_mode(dsi);
>   	samsung_dsim_set_display_enable(dsi, true);
>
Best regards
-- 
Marek Szyprowski, PhD
Samsung R&D Institute Poland

Re: [PATCH 1/3] drm/bridge: samsung-dsim: move bridge init sequence to atomic_enable

[Marek Szyprowski]

On 26.01.2026 09:57, Marek Szyprowski wrote:
> On 24.01.2026 18:20, Kaustabh Chakraborty wrote:
>> Since commit c9b1150a68d9 ("drm/atomic-helper: Re-order bridge chain
>> pre-enable and post-disable"), pre-enable sequence is called before the
>> CRTC is enabled.
>>
>> This causes unintended side-effects (abberation among potentially other
>> things) in the display when samsung_dsim_init() is called in the
>> pre-enable part of the sequence. Call it in samsung_dsim_atomic_enable()
>> instead.
>>
>> Cc: stable@vger.kernel.org # v6.17 and later
>> Signed-off-by: Kaustabh Chakraborty <kauschluss@disroot.org>
>
> I'm not sure if this will be needed:
>
> https://lore.kernel.org/all/20251205-drm-seq-fix-v1-0-fda68fa1b3de@ideasonboard.com/ 
>

Even more, there is a pending similar patch:

https://lore.kernel.org/all/20250619-samsung-dsim-fix-v1-1-6b5de68fb115@ideasonboard.com/

Best regards
-- 
Marek Szyprowski, PhD
Samsung R&D Institute Poland

Re: [PATCH 0/3] Fixes and enhancements for Exynos (7870) DSIM bridge driver

[Marek Szyprowski]

On 24.01.2026 18:20, Kaustabh Chakraborty wrote:
> Since v6.17, there were a few regressive changes for the Exynos 7870
> DSIM driver. These changes resulted in weird artifacts on the display,
> such as random RGB channel swaps and random aberration (the occurrences
> of both were mutually exclusive).
>
> The first two commits of this patch series address the aforementioned
> changes.
>
> The third patch replaces an implicit loop for waiting for PLL
> stabilization with an interrupt-based solution, which should be more
> reliable. This solution was suggested by Inki Dae in a discussion of an
> earlier patch series sent by me. For further details, refer to its
> commit description.
>
> Signed-off-by: Kaustabh Chakraborty <kauschluss@disroot.org>

Works fine on legacy Exynos based boards in my test lab.

Tested-by: Marek Szyprowski <m.szyprowski@samsung.com>

> ---
> Kaustabh Chakraborty (3):
>        drm/bridge: samsung-dsim: move bridge init sequence to atomic_enable
>        drm/bridge: samsung-dsim: enable MFLUSH_VS for Exynos 7870 DSIM
>        drm/bridge: samsung-dsim: use DSIM interrupt to wait for PLL stability
>
>   drivers/gpu/drm/bridge/samsung-dsim.c | 61 +++++++++++++++++++++++------------
>   include/drm/bridge/samsung-dsim.h     |  1 +
>   2 files changed, 42 insertions(+), 20 deletions(-)
> ---
> base-commit: ca3a02fda4da8e2c1cb6baee5d72352e9e2cfaea
> change-id: 20260124-exynos-dsim-fixes-5383d6a6f073
>
> Best regards,

Best regards
-- 
Marek Szyprowski, PhD
Samsung R&D Institute Poland

Re: [PATCH 1/3] drm/bridge: samsung-dsim: move bridge init sequence to atomic_enable

[Kaustabh Chakraborty]

On 2026-01-26 09:57 +01:00, Marek Szyprowski wrote:
> On 24.01.2026 18:20, Kaustabh Chakraborty wrote:
>> Since commit c9b1150a68d9 ("drm/atomic-helper: Re-order bridge chain
>> pre-enable and post-disable"), pre-enable sequence is called before the
>> CRTC is enabled.
>>
>> This causes unintended side-effects (abberation among potentially other
>> things) in the display when samsung_dsim_init() is called in the
>> pre-enable part of the sequence. Call it in samsung_dsim_atomic_enable()
>> instead.
>>
>> Cc: stable@vger.kernel.org # v6.17 and later
>> Signed-off-by: Kaustabh Chakraborty <kauschluss@disroot.org>
>
> I'm not sure if this will be needed:
>
> https://lore.kernel.org/all/20251205-drm-seq-fix-v1-0-fda68fa1b3de@ideasonboard.com/

Tested on v6.19-rc7, this is fixed now. This can be dropped.

>
>
>> ---
>>   drivers/gpu/drm/bridge/samsung-dsim.c | 13 +++++++------
>>   1 file changed, 7 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/bridge/samsung-dsim.c b/drivers/gpu/drm/bridge/samsung-dsim.c
>> index 1d85e706c74b9..975f8b50ae660 100644
>> --- a/drivers/gpu/drm/bridge/samsung-dsim.c
>> +++ b/drivers/gpu/drm/bridge/samsung-dsim.c
>> @@ -1655,6 +1655,13 @@ static void samsung_dsim_atomic_pre_enable(struct drm_bridge *bridge,
>>   	}
>>   
>>   	dsi->state |= DSIM_STATE_ENABLED;
>> +}
>> +
>> +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;
>>   
>>   	/*
>>   	 * For Exynos-DSIM the downstream bridge, or panel are expecting
>> @@ -1665,12 +1672,6 @@ static void samsung_dsim_atomic_pre_enable(struct drm_bridge *bridge,
>>   		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);
>>   
>>   	samsung_dsim_set_display_mode(dsi);
>>   	samsung_dsim_set_display_enable(dsi, true);
>>
> Best regards