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

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

[Kaustabh Chakraborty]

Since v6.17, there is a regression for the Exynos 7870 DSIM driver. The
display occasionally had random aberration when the panel was turned on.
The first patch addresses that.

The second 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>
---
Changes in v3:
- Add patch for removing conflicting devices for DRM (Sam Day)
- Link to v2: https://lore.kernel.org/r/20260208-exynos-dsim-fixes-v2-0-a857e8130a2a@disroot.org

Changes in v2:
- drop now-not-required [v1 1/3] (Marek Szyprowski)
- Link to v1: https://lore.kernel.org/r/20260124-exynos-dsim-fixes-v1-0-122d047a23d1@disroot.org

---
Kaustabh Chakraborty (3):
      drm/bridge: samsung-dsim: enable MFLUSH_VS for Exynos 7870 DSIM
      drm/bridge: samsung-dsim: use DSIM interrupt to wait for PLL stability
      drm/exynos: remove simplefb nodes before init

 drivers/gpu/drm/bridge/samsung-dsim.c   | 48 +++++++++++++++++++++++----------
 drivers/gpu/drm/exynos/exynos_drm_drv.c |  6 +++++
 include/drm/bridge/samsung-dsim.h       |  1 +
 3 files changed, 41 insertions(+), 14 deletions(-)
---
base-commit: 290aaf24a551d5a0dce037e3fab30820f9113a10
change-id: 20260124-exynos-dsim-fixes-5383d6a6f073

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

[PATCH v3 1/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/a3762bb1761ae/drivers/video/fbdev/exynos/decon_7870/dsim_reg_7870.c#L699 [1]
Tested-by: Marek Szyprowski <m.szyprowski@samsung.com>
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 25ab475309e8..da753ff6eed4 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.54.0

[PATCH v3 2/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
Tested-by: Marek Szyprowski <m.szyprowski@samsung.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 da753ff6eed4..866cff205e71 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))
@@ -2148,6 +2160,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 03005e474704..e3433da21ad0 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.54.0

[PATCH v3 3/3] drm/exynos: remove simplefb nodes before init

[Kaustabh Chakraborty]

The simple-framebuffer uses the framebuffer region initialized by the
boot loader for display. However, with Linux the DSI and FIMD/DECON
stack is initialized from scratch.

However, both are destined use the same memory region, so both cannot
co-exist. For a seamless display transition during boot, the framebuffer
must be destroyed, which releases the memory region, followed by proper
IP re-initialization. Achieve it using
aperture_remove_all_conflicting_devices(). This is also implemented in
other similar systems such as in Qualcomm's msm_kms driver.

Signed-off-by: Kaustabh Chakraborty <kauschluss@disroot.org>
---
 drivers/gpu/drm/exynos/exynos_drm_drv.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/gpu/drm/exynos/exynos_drm_drv.c b/drivers/gpu/drm/exynos/exynos_drm_drv.c
index 9ee30086879f..7657f86a9d19 100644
--- a/drivers/gpu/drm/exynos/exynos_drm_drv.c
+++ b/drivers/gpu/drm/exynos/exynos_drm_drv.c
@@ -7,6 +7,7 @@
  *	Seung-Woo Kim <sw0312.kim@samsung.com>
  */
 
+#include <linux/aperture.h>
 #include <linux/component.h>
 #include <linux/dma-mapping.h>
 #include <linux/platform_device.h>
@@ -273,6 +274,11 @@ static int exynos_drm_bind(struct device *dev)
 	if (ret)
 		goto err_mode_config_cleanup;
 
+	/* Remove existing devices which own the framebuffer memory. */
+	ret = aperture_remove_all_conflicting_devices(exynos_drm_driver.name);
+	if (ret)
+		goto err_unbind_all;
+
 	ret = drm_vblank_init(drm, drm->mode_config.num_crtc);
 	if (ret)
 		goto err_unbind_all;

-- 
2.54.0

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

[Inki Dae]

HI,

2026년 7월 23일 (목) 오전 4:04, Kaustabh Chakraborty <kauschluss@disroot.org>님이 작성:
>
> 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
> Tested-by: Marek Szyprowski <m.szyprowski@samsung.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 da753ff6eed4..866cff205e71 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;

The value assigned to `timeout` below, `reg_values[PLL_TIMER] / fin`,
is an `unsigned long`. It compiles thanks to `__careful_cmp()` in
`max()`, but the result is silently narrowed to `unsigned int`. Please
make this `unsigned long timeout` instead.

>         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.

Typo: "Calcutate" -> "Calculate".

> +        * 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;

`fin` is used earlier in this function as the PLL reference clock in
Hz, and is printed by dev_dbg(). Overwriting it here with a completely
unrelated value -  the APB bus clock in MHz - is confusing when
debugging. Please use a separate variable:

unsigned long bus_clk_mhz;
...
bus_clk_mhz = clk_get_rate(driver_data->clk_data[0].clk) / HZ_PER_MHZ;


> +       if (fin)
> +               timeout = max(dsi->driver_data->reg_values[PLL_TIMER] / fin,
> +                             timeout);

Also, there is already a `driver_data` local at the top of the
function which the rest of the function uses. Only the newly added
code dereferences `dsi->driver_data->` directly, which is
inconsistent.

> +
> +       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;
> +       }

This is the main problem: the condition is inverted.

ait_for_completion_timeout() returns 0 on timeout, and the number of
remaining jiffies (> 0) on completion. As written, this reports an
error and returns 0 exactly when the PLL *did* stabilize, and silently
succeeds when it timed out.

Since samsung_dsim_set_pll() returning 0 makes its caller
samsung_dsim_enable_clock() bail out with -EFAULT, the display would
fail to come up entirely the moment a PLL_STABLE interrupt actually
arrives.

    if (!wait_for_completion_timeout(&dsi->pll_stabilized,
                                   usecs_to_jiffies(timeout))) {

The reason this still passed testing is the second issue below: the
PLL_STABLE interrupt never fires in the first place. The two bugs are
masking each other.

One more thing - this removes the only user of
driver_data->pll_stable_bit. The field declaration in the header and
all seven driver_data initialisers are now dead. Please drop them in
the same series. (has_legacy_status_reg is still used by
samsung_dsim_init_link(), so that one should stay.)

>
>         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;

Two problems here.

First, DSIM_INT_PLL_STABLE is masked in INTMSK, so this interrupt can
never be delivered. The only place that writes INTMSK is the
SW_RST_RELEASE path just above:

        unsigned long mask = ~(DSIM_INT_RX_DONE |
                               DSIM_INT_SFR_FIFO_EMPTY |
                               DSIM_INT_SFR_HDR_FIFO_EMPTY |
                               DSIM_INT_RX_ECC_ERR |
                               DSIM_INT_SW_RST_RELEASE);
        samsung_dsim_write(dsi, DSIM_INTMSK_REG, mask);

DSIM_INT_PLL_STABLE is not in that list, so it stays masked. And
Exynos 7870 has wait_for_reset = 1, so samsung_dsim_init() runs:

        samsung_dsim_wait_for_reset()   <- runs this handler, programs INTMSK
        samsung_dsim_enable_clock()     <- calls samsung_dsim_set_pll()

meaning PLL_STABLE is guaranteed to be masked by the time set_pll()
runs. As a result this patch performs no PLL stabilization wait at
all, and instead sleeps for the full timeout on every enable. Because
of usecs_to_jiffies() resolution, 100us rounds up to at least one
jiffy - 4ms at HZ=250, 10ms at HZ=100. That is a new latency
regression compared to the old polling loop.

Please add DSIM_INT_PLL_STABLE to the INTMSK list. It would also be
safer to program INTMSK explicitly right after reset, so that SoCs
with wait_for_reset = 0 (which never reach this handler path) are
covered too.

Second, the early return swallows concurrent events. `status` can have
several bits set at once, and everything has already been cleared
write-1-to-clear in INTSRC above. If PLL_STABLE and SFR_FIFO_EMPTY are
raised together, the samsung_dsim_transfer_finish() handling is lost
permanently and that transfer stalls until its own timeout. The
pre-patch code handled all three bits in one condition and fell
through, so it did not have this problem.

Please make the handling non-exclusive:

        if (status & DSIM_INT_PLL_STABLE)
                complete(&dsi->pll_stabilized);

        if (!(status & (DSIM_INT_RX_DONE | DSIM_INT_SFR_FIFO_EMPTY)))
                return IRQ_HANDLED;

Thanks,
Inki Dae

>
>         if (samsung_dsim_transfer_finish(dsi))
> @@ -2148,6 +2160,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 03005e474704..e3433da21ad0 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.54.0
>
>

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

[Luca Ceresoli]

On Thu, 23 Jul 2026 00:28:32 +0530, Kaustabh Chakraborty <kauschluss@disroot.org> wrote:

Hello Kaustabh,

> 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

Being a fix, there must be a Fixes: tag.

>
>
> diff --git a/drivers/gpu/drm/bridge/samsung-dsim.c b/drivers/gpu/drm/bridge/samsung-dsim.c
> index e2fc69fc51b6..b72542a0afb6 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;
>  	}

Based on your commit message, the differentiating point of the Exynos 7870
not "needs to be enabled" but rather "it is active high". So a more
understandable comment would be "The Mflush_VS bit is active low on most
devices but active high on the Exynos 7870", or something similar.

Luca

-- 
Luca Ceresoli, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

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

[Kaustabh Chakraborty]

On 2026-08-02 15:56 +09:00, Inki Dae wrote:
> HI,
>
> 2026년 7월 23일 (목) 오전 4:04, Kaustabh Chakraborty <kauschluss@disroot.org>님이 작성:
>>
>> 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
>> Tested-by: Marek Szyprowski <m.szyprowski@samsung.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 da753ff6eed4..866cff205e71 100644
>> --- a/drivers/gpu/drm/bridge/samsung-dsim.c
>> +++ b/drivers/gpu/drm/bridge/samsung-dsim.c

...

>>
>> -       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;
>> +       }
>
> This is the main problem: the condition is inverted.
>
> wait_for_completion_timeout() returns 0 on timeout, and the number of
> remaining jiffies (> 0) on completion. As written, this reports an
> error and returns 0 exactly when the PLL *did* stabilize, and silently
> succeeds when it timed out.
>
> Since samsung_dsim_set_pll() returning 0 makes its caller
> samsung_dsim_enable_clock() bail out with -EFAULT, the display would
> fail to come up entirely the moment a PLL_STABLE interrupt actually
> arrives.
>
>     if (!wait_for_completion_timeout(&dsi->pll_stabilized,
>                                    usecs_to_jiffies(timeout))) {
>
> The reason this still passed testing is the second issue below: the
> PLL_STABLE interrupt never fires in the first place. The two bugs are
> masking each other.

Unfortunately, fixing these my hardware does not issue the PLL_STABLE
interrupt for the first panel reset during boot. For subsequent ones it
does issue the same.

One lazy solution is to try to peek at the PLL stable bit after the
timeout and continue execution if it indicates that PLL is stable. In
that case we should be able to validate it with a fallback.

I should also remember to drop the Reviewed-by: tag from this patch.