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

[PATCH v4 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 v4:
- Clarify active-hight MFLUSH_VS in code comment (Luca Seresoli)
- Move aperture_remove_all_conflicting_devices() before
  component_bind_all() (Sashiko)
- Change `timeout` type to unsigned long (Inki Dae)
- Correct typo s/Calcutate/Calculate (Inki Dae)
- Correct inverted condition waiting for PLL stable completion
  (Inki Dae)
- Do not exclusively conclude IRQ routine for PLL_STABLE interrupt
  (Inki Dae)
- Explicitly enable PLL_STABLE and SW_RST_RELEASE interrupts during
  reset action (Inki Dae)
- Wait for SW_RST_RELEASE interrupt before setting up clock (Inki Dae)
- Fall back to bit testing for PLL stabilization when timed out
- Link to v3: https://patch.msgid.link/20260723-exynos-dsim-fixes-v3-0-0c31ae1dbecc@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   | 56 +++++++++++++++++++++++++--------
 drivers/gpu/drm/exynos/exynos_drm_drv.c |  6 ++++
 include/drm/bridge/samsung-dsim.h       |  1 +
 3 files changed, 50 insertions(+), 13 deletions(-)
---
base-commit: 290aaf24a551d5a0dce037e3fab30820f9113a10
change-id: 20260124-exynos-dsim-fixes-5383d6a6f073

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

[PATCH v4 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.

Fixes: a36c533ad3e1 ("drm/bridge: samsung-dsim: Always flush display FIFO on vsync pulse")
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..fc42acc6b9ef 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 as its active-high.
+		 */
+		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 v4 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.

To utilize this, explicitly unmask PLL_STABLE and SW_RST_RELEASE right
after reset, and wait for reset before setting up clock.

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 | 49 +++++++++++++++++++++++++----------
 include/drm/bridge/samsung-dsim.h     |  1 +
 2 files changed, 37 insertions(+), 13 deletions(-)

diff --git a/drivers/gpu/drm/bridge/samsung-dsim.c b/drivers/gpu/drm/bridge/samsung-dsim.c
index fc42acc6b9ef..9f58c7b1be53 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>
@@ -722,9 +723,14 @@ static void samsung_dsim_wait_for_reset(struct samsung_dsim *dsi)
 static void samsung_dsim_reset(struct samsung_dsim *dsi)
 {
 	u32 reset_val = dsi->driver_data->reg_values[RESET_TYPE];
+	u32 status;
 
 	reinit_completion(&dsi->completed);
 	samsung_dsim_write(dsi, DSIM_SWRST_REG, reset_val);
+
+	status = samsung_dsim_read(dsi, DSIM_INTMSK_REG);
+	status &= ~(DSIM_INT_PLL_STABLE | DSIM_INT_SW_RST_RELEASE);
+	samsung_dsim_write(dsi, DSIM_INTMSK_REG, status);
 }
 
 static unsigned long samsung_dsim_pll_find_pms(struct samsung_dsim *dsi,
@@ -787,8 +793,7 @@ static unsigned long samsung_dsim_set_pll(struct samsung_dsim *dsi,
 					  unsigned long freq)
 {
 	const struct samsung_dsim_driver_data *driver_data = dsi->driver_data;
-	unsigned long fin, fout;
-	int timeout;
+	unsigned long fin, fout, timeout_us, bus_clk_mhz;
 	u8 p, s;
 	u16 m;
 	u32 reg;
@@ -849,19 +854,33 @@ 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. Calculate 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_us = 100;
+	bus_clk_mhz = clk_get_rate(dsi->driver_data->clk_data[0].clk) / HZ_PER_MHZ;
+	if (bus_clk_mhz)
+		timeout_us = max(driver_data->reg_values[PLL_TIMER] / bus_clk_mhz,
+				 timeout_us);
+
+	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 (!wait_for_completion_timeout(&dsi->pll_stabilized,
+					 usecs_to_jiffies(timeout_us))) {
 		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 (!(reg & BIT(driver_data->pll_stable_bit))) {
+			dev_err(dsi->dev, "PLL failed to stabilize\n");
+			return 0;
+		}
+	}
 
 	dsi->hs_clock = fout;
 
@@ -1586,7 +1605,8 @@ static irqreturn_t samsung_dsim_irq(int irq, void *dev_id)
 	samsung_dsim_write(dsi, DSIM_INTSRC_REG, status);
 
 	if (status & DSIM_INT_SW_RST_RELEASE) {
-		unsigned long mask = ~(DSIM_INT_RX_DONE |
+		unsigned long mask = ~(DSIM_INT_PLL_STABLE |
+				       DSIM_INT_RX_DONE |
 				       DSIM_INT_SFR_FIFO_EMPTY |
 				       DSIM_INT_SFR_HDR_FIFO_EMPTY |
 				       DSIM_INT_RX_ECC_ERR |
@@ -1596,8 +1616,10 @@ 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);
+
+	if (!(status & (DSIM_INT_RX_DONE | DSIM_INT_SFR_FIFO_EMPTY)))
 		return IRQ_HANDLED;
 
 	if (samsung_dsim_transfer_finish(dsi))
@@ -1635,9 +1657,9 @@ static int samsung_dsim_init(struct samsung_dsim *dsi)
 	if (driver_data->reg_values[RESET_TYPE] == DSIM_FUNCRST)
 		samsung_dsim_enable_lane(dsi, BIT(dsi->lanes) - 1);
 
-	samsung_dsim_enable_clock(dsi);
 	if (driver_data->wait_for_reset)
 		samsung_dsim_wait_for_reset(dsi);
+	samsung_dsim_enable_clock(dsi);
 	samsung_dsim_set_phy_ctrl(dsi);
 	samsung_dsim_init_link(dsi);
 
@@ -2148,6 +2170,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 v4 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..e05e520449ed 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>
@@ -268,6 +269,11 @@ static int exynos_drm_bind(struct device *dev)
 	list_for_each_entry(encoder, &drm->mode_config.encoder_list, head)
 		encoder->possible_clones = clone_mask;
 
+	/* Remove existing devices which own the framebuffer memory. */
+	ret = aperture_remove_all_conflicting_devices(exynos_drm_driver.name);
+	if (ret)
+		goto err_unbind_all;
+
 	/* Try to bind all sub drivers. */
 	ret = component_bind_all(drm->dev, drm);
 	if (ret)

-- 
2.54.0

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

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:

Pre-existing issues:
- [High] Unchecked NULL pointer from `of_device_get_match_data()` leads to kernel panic in probe.
- [High] Hardware polling loop for lane stop states terminates prematurely and lacks a time-bound delay.
- [High] Macro `samsung_dsim_hw_is_exynos()` incorrectly evaluates to false for `DSIM_TYPE_EXYNOS7870`.
--

--- Patch [1]: [PATCH v4 1/3] drm/bridge: samsung-dsim: enable MFLUSH_VS for Exynos 7870 DSIM ---
commit 6317440f4c2cdec241fc86381b097531928140af
Author: Kaustabh Chakraborty <kauschluss@disroot.org>

drm/bridge: samsung-dsim: enable MFLUSH_VS for Exynos 7870 DSIM

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. This commit enables the bit
for Exynos 7870 since it is active-high on this hardware.

Link: https://github.com/samsungexynos7870/android_kernel_samsung_exynos7870/blob/a3762bb1761ae/drivers/video/fbdev/exynos/decon_7870/dsim_reg_7870.c#L699 [1]

> diff --git a/drivers/gpu/drm/bridge/samsung-dsim.c b/drivers/gpu/drm/bridge/samsung-dsim.c
> index e2fc69fc51b60..6260c091c3df8 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 as its active-high.
> +		 */
> +		if (dsi->plat_data->hw_type == DSIM_TYPE_EXYNOS7870)
> +			reg |= DSIM_MFLUSH_VS;
>  	}
>  
>  	if (dsi->mode_flags & MIPI_DSI_MODE_NO_EOT_PACKET)

[Severity: High]
This isn't a bug introduced by this patch, but since Exynos 7870 support is
being actively modified here, does the samsung_dsim_hw_is_exynos() macro need
updating to include DSIM_TYPE_EXYNOS7870?

Currently, it evaluates to false for Exynos 7870 because the macro strictly
checks up to DSIM_TYPE_EXYNOS5433:

include/drm/bridge/samsung-dsim.h:
#define samsung_dsim_hw_is_exynos(hw) \
	((hw) >= DSIM_TYPE_EXYNOS3250 && (hw) <= DSIM_TYPE_EXYNOS5433)

This causes the driver to erroneously execute the i.MX early initialization
path in samsung_dsim_atomic_pre_enable() and incorrectly set the
DSIM_NON_CONTINUOUS_CLKLANE bit further down in samsung_dsim_init_link().

[ ... ]

> 	/* Check clock and data lane state are stop state */
> 	timeout = 100;
> 	do {
> 		if (timeout-- == 0) {
> 			dev_err(dsi->dev, "waiting for bus lanes timed out\n");
> 			return -EFAULT;
> 		}
> 
> 		if (driver_data->has_legacy_status_reg)
> 			reg = samsung_dsim_read(dsi, DSIM_STATUS_REG);
> 		else
> 			reg = samsung_dsim_read(dsi, DSIM_DPHY_STATUS_REG);
> 		if ((reg & DSIM_STOP_STATE_DAT(lanes_mask))
> 		    != DSIM_STOP_STATE_DAT(lanes_mask))
> 			continue;
> 	} while (!(reg & (DSIM_STOP_STATE_CLK | DSIM_TX_READY_HS_CLK)));

[Severity: High]
This is a pre-existing issue, but can this hardware polling loop terminate
prematurely?

If the clock lane happens to be ready but the data lanes are not, the
continue statement jumps to the loop condition evaluation. The condition
evaluates to false, causing the loop to exit successfully instead of waiting
for the data lanes.

Also, should this loop include a time-bound delay (like ndelay) to prevent
premature aborts on faster processors?

[ ... ]

[Severity: High]
This is a pre-existing issue elsewhere in the file, but can we hit a NULL
pointer dereference in samsung_dsim_probe() if the driver is bound via sysfs?

If a platform device is manually created and the driver is bound via sysfs
(e.g., via "echo samsung-dsim > bind") without an associated device tree node,
of_device_get_match_data() returns NULL:

samsung_dsim_probe() {
    ...
    dsi->plat_data = of_device_get_match_data(dev);
    dsi->driver_data = samsung_dsim_types[dsi->plat_data->hw_type];
    ...
}

Does this need a NULL check before dereferencing dsi->plat_data?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260809-exynos-dsim-fixes-v4-0-ca67c51e61b7@disroot.org?part=1

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

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:

New issues:
- [High] An early return in the interrupt handler permanently drops the newly added `DSIM_INT_PLL_STABLE` interrupt on platforms that do not wait for a reset completion.
- [High] The jiffy-based timeout for waiting on the PLL stabilization completion can expire prematurely, leading to spurious initialization failures.

Pre-existing issues:
- [High] `devm_clk_bulk_get()` overwrites a statically allocated global clock data array, breaking multi-instance platforms.
- [High] The return value of `samsung_dsim_enable_clock()` is ignored in `samsung_dsim_init()`, causing silent hardware initialization failures.
--

--- Patch [2]: [PATCH v4 2/3] drm/bridge: samsung-dsim: use DSIM interrupt to wait for PLL stability ---
commit 2d251655e36dcc7e05ab4e2b1dc184bbb561aeb9
Author: Kaustabh Chakraborty <kauschluss@disroot.org>

drm/bridge: samsung-dsim: use DSIM interrupt to wait for PLL stability

This commit transitions the PLL stabilization wait in the samsung-dsim driver
from a busy loop to an interrupt-driven completion. It calculates a dynamic
timeout and updates the IRQ handler to complete the wait when the PLL is
stable.

Link: https://lore.kernel.org/r/CAAQKjZMLMbwDVZRb5+Xb_5yz3AEP4uuzFJMuuZy9NFDu13VU5w@mail.gmail.com

> diff --git a/drivers/gpu/drm/bridge/samsung-dsim.c b/drivers/gpu/drm/bridge/samsung-dsim.c
> --- a/drivers/gpu/drm/bridge/samsung-dsim.c
> +++ b/drivers/gpu/drm/bridge/samsung-dsim.c
[ ... ]
> @@ -849,19 +854,33 @@ 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. Calculate 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_us = 100;
> +	bus_clk_mhz = clk_get_rate(dsi->driver_data->clk_data[0].clk) / HZ_PER_MHZ;

[Severity: High]
This is a pre-existing issue, but does using dsi->driver_data->clk_data here
expose a data race on multi-instance setups? In samsung_dsim_probe(),
devm_clk_bulk_get() writes the acquired clock pointers directly into the
shared global static array referenced by driver_data. On multi-instance
platforms, a second instance overwrites the clocks of the first, and unbinding
either instance causes clk_bulk_put() to free the other instance's clocks,
leading to a double-free on the second unbind.

> +	if (bus_clk_mhz)
> +		timeout_us = max(driver_data->reg_values[PLL_TIMER] / bus_clk_mhz,
> +				 timeout_us);
> +
> +	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 (!wait_for_completion_timeout(&dsi->pll_stabilized,
> +					 usecs_to_jiffies(timeout_us))) {

[Severity: High]
Could this jiffy-based timeout expire prematurely and lead to spurious
initialization failures? The dynamically calculated timeout_us is around
100-300us, which evaluates to 1 jiffy via usecs_to_jiffies(). A call to
wait_for_completion_timeout(..., 1) sleeps until the next timer tick, which
can happen in less than 1us. If the timeout expires prematurely, the fallback
code checks the status register exactly once, and if the PLL hasn't stabilized
in that narrow window, it aborts.

>  		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 (!(reg & BIT(driver_data->pll_stable_bit))) {
> +			dev_err(dsi->dev, "PLL failed to stabilize\n");
> +			return 0;
> +		}
> +	}
[ ... ]
> @@ -1586,7 +1605,8 @@ static irqreturn_t samsung_dsim_irq(int irq, void *dev_id)
>  	samsung_dsim_write(dsi, DSIM_INTSRC_REG, status);
>  
>  	if (status & DSIM_INT_SW_RST_RELEASE) {
> -		unsigned long mask = ~(DSIM_INT_RX_DONE |
> +		unsigned long mask = ~(DSIM_INT_PLL_STABLE |
> +				       DSIM_INT_RX_DONE |
>  				       DSIM_INT_SFR_FIFO_EMPTY |
>  				       DSIM_INT_SFR_HDR_FIFO_EMPTY |
>  				       DSIM_INT_RX_ECC_ERR |
> @@ -1596,8 +1616,10 @@ static irqreturn_t samsung_dsim_irq(int irq, void *dev_id)
>  		return IRQ_HANDLED;
>  	}

[Severity: High]
Does this unconditional early return drop the newly added DSIM_INT_PLL_STABLE
interrupt? On platforms where wait_for_reset is false, samsung_dsim_init()
triggers a software reset and immediately enables the clock without waiting.
Both the DSIM_INT_SW_RST_RELEASE and DSIM_INT_PLL_STABLE interrupts can fire
concurrently. If coalesced, the handler clears both hardware interrupts,
processes the reset, and executes this early return in samsung_dsim_irq(),
skipping the DSIM_INT_PLL_STABLE check entirely.

[ ... ]
> @@ -1635,9 +1657,9 @@ static int samsung_dsim_init(struct samsung_dsim *dsi)
>  	if (driver_data->reg_values[RESET_TYPE] == DSIM_FUNCRST)
>  		samsung_dsim_enable_lane(dsi, BIT(dsi->lanes) - 1);
>  
> -	samsung_dsim_enable_clock(dsi);
>  	if (driver_data->wait_for_reset)
>  		samsung_dsim_wait_for_reset(dsi);
> +	samsung_dsim_enable_clock(dsi);

[Severity: High]
This is a pre-existing issue, but should the return value of
samsung_dsim_enable_clock() be checked here? If samsung_dsim_set_pll() fails
and returns 0, the ignored -EFAULT allows the driver to continue
initialization with a broken clock state, computing a byte_clock of 0, writing
0s to PHY timing registers, and leaving the hardware in an unusable state.

>  	samsung_dsim_set_phy_ctrl(dsi);
>  	samsung_dsim_init_link(dsi);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260809-exynos-dsim-fixes-v4-0-ca67c51e61b7@disroot.org?part=2

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

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

New issues:
- [Medium] Incorrect error handling path for aperture_remove_all_conflicting_devices() triggers WARN_ON() storms by attempting to unbind unbound components.

Pre-existing issues:
- [Medium] The setup loop for `possible_clones` iterates over an empty `encoder_list` because encoders are not registered until sub-components are bound.
--

--- Patch [3]: [PATCH v4 3/3] drm/exynos: remove simplefb nodes before init ---
commit 9e7932152628258cc8174577476729d0609085e6
Author: Kaustabh Chakraborty <kauschluss@disroot.org>

drm/exynos: remove simplefb nodes before init

This commit uses aperture_remove_all_conflicting_devices() to remove
simple-framebuffer devices before initializing the DSI and FIMD/DECON stack.
This ensures memory regions do not overlap during display transitions.

> diff --git a/drivers/gpu/drm/exynos/exynos_drm_drv.c b/drivers/gpu/drm/exynos/exynos_drm_drv.c
> index 9ee30086879f9..e05e520449edc 100644
> --- a/drivers/gpu/drm/exynos/exynos_drm_drv.c
> +++ b/drivers/gpu/drm/exynos/exynos_drm_drv.c
[ ... ]
> @@ -268,6 +269,11 @@ static int exynos_drm_bind(struct device *dev)
>  	list_for_each_entry(encoder, &drm->mode_config.encoder_list, head)
>  		encoder->possible_clones = clone_mask;
>  

[Severity: Medium]
This is a pre-existing issue, but does this loop successfully find any
encoders?

In the Exynos DRM architecture, encoders are registered by sub-components
during their bind phase, which is triggered by component_bind_all().
Since this setup block is placed before component_bind_all(), the
encoder_list is always empty, making this dead code and leaving
possible_clones unconfigured for all encoders.

> +	/* Remove existing devices which own the framebuffer memory. */
> +	ret = aperture_remove_all_conflicting_devices(exynos_drm_driver.name);
> +	if (ret)
> +		goto err_unbind_all;
                ^^^^

[Severity: Medium]
Will this error path trigger WARN_ON() storms?

If aperture_remove_all_conflicting_devices() fails, the code jumps to
err_unbind_all, which executes component_unbind_all().

Since component_bind_all() has not yet been executed, all matched components
have component->bound == false. For each component, component_unbind()
evaluates WARN_ON(!component->bound) and triggers a kernel warning stack
trace.

Could this jump to err_mode_config_cleanup instead, similar to the failure
path for component_bind_all()?

> +
>  	/* Try to bind all sub drivers. */
>  	ret = component_bind_all(drm->dev, drm);
>  	if (ret)

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260809-exynos-dsim-fixes-v4-0-ca67c51e61b7@disroot.org?part=3