* [PATCH v3 0/3] Fixes and enhancements for Exynos (7870) DSIM bridge driver
@ 2026-07-22 18:58 Kaustabh Chakraborty
2026-07-22 18:58 ` [PATCH v3 1/3] drm/bridge: samsung-dsim: enable MFLUSH_VS for Exynos 7870 DSIM Kaustabh Chakraborty
` (2 more replies)
0 siblings, 3 replies; 7+ messages in thread
From: Kaustabh Chakraborty @ 2026-07-22 18:58 UTC (permalink / raw)
To: Inki Dae, Jagan Teki, Marek Szyprowski, Andrzej Hajda,
Neil Armstrong, Robert Foss, Laurent Pinchart, Jonas Karlman,
Jernej Skrabec, Luca Ceresoli, Maarten Lankhorst, Maxime Ripard,
Thomas Zimmermann, David Airlie, Simona Vetter, Seung-Woo Kim,
Kyungmin Park, Krzysztof Kozlowski, Peter Griffin, Alim Akhtar
Cc: dri-devel, linux-kernel, linux-arm-kernel, linux-samsung-soc,
Kaustabh Chakraborty, stable
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>
^ permalink raw reply [flat|nested] 7+ messages in thread* [PATCH v3 1/3] drm/bridge: samsung-dsim: enable MFLUSH_VS for Exynos 7870 DSIM 2026-07-22 18:58 [PATCH v3 0/3] Fixes and enhancements for Exynos (7870) DSIM bridge driver Kaustabh Chakraborty @ 2026-07-22 18:58 ` Kaustabh Chakraborty 2026-08-05 9:50 ` Luca Ceresoli 2026-07-22 18:58 ` [PATCH v3 2/3] drm/bridge: samsung-dsim: use DSIM interrupt to wait for PLL stability Kaustabh Chakraborty 2026-07-22 18:58 ` [PATCH v3 3/3] drm/exynos: remove simplefb nodes before init Kaustabh Chakraborty 2 siblings, 1 reply; 7+ messages in thread From: Kaustabh Chakraborty @ 2026-07-22 18:58 UTC (permalink / raw) To: Inki Dae, Jagan Teki, Marek Szyprowski, Andrzej Hajda, Neil Armstrong, Robert Foss, Laurent Pinchart, Jonas Karlman, Jernej Skrabec, Luca Ceresoli, Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie, Simona Vetter, Seung-Woo Kim, Kyungmin Park, Krzysztof Kozlowski, Peter Griffin, Alim Akhtar Cc: dri-devel, linux-kernel, linux-arm-kernel, linux-samsung-soc, Kaustabh Chakraborty, stable 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 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH v3 1/3] drm/bridge: samsung-dsim: enable MFLUSH_VS for Exynos 7870 DSIM 2026-07-22 18:58 ` [PATCH v3 1/3] drm/bridge: samsung-dsim: enable MFLUSH_VS for Exynos 7870 DSIM Kaustabh Chakraborty @ 2026-08-05 9:50 ` Luca Ceresoli 0 siblings, 0 replies; 7+ messages in thread From: Luca Ceresoli @ 2026-08-05 9:50 UTC (permalink / raw) To: Kaustabh Chakraborty Cc: Inki Dae, Jagan Teki, Marek Szyprowski, Andrzej Hajda, Neil Armstrong, Robert Foss, Laurent Pinchart, Jonas Karlman, Jernej Skrabec, Luca Ceresoli, Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie, Simona Vetter, Seung-Woo Kim, Kyungmin Park, Krzysztof Kozlowski, Peter Griffin, Alim Akhtar, dri-devel, linux-kernel, linux-arm-kernel, linux-samsung-soc, stable 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 ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH v3 2/3] drm/bridge: samsung-dsim: use DSIM interrupt to wait for PLL stability 2026-07-22 18:58 [PATCH v3 0/3] Fixes and enhancements for Exynos (7870) DSIM bridge driver Kaustabh Chakraborty 2026-07-22 18:58 ` [PATCH v3 1/3] drm/bridge: samsung-dsim: enable MFLUSH_VS for Exynos 7870 DSIM Kaustabh Chakraborty @ 2026-07-22 18:58 ` Kaustabh Chakraborty 2026-08-02 6:56 ` Inki Dae 2026-07-22 18:58 ` [PATCH v3 3/3] drm/exynos: remove simplefb nodes before init Kaustabh Chakraborty 2 siblings, 1 reply; 7+ messages in thread From: Kaustabh Chakraborty @ 2026-07-22 18:58 UTC (permalink / raw) To: Inki Dae, Jagan Teki, Marek Szyprowski, Andrzej Hajda, Neil Armstrong, Robert Foss, Laurent Pinchart, Jonas Karlman, Jernej Skrabec, Luca Ceresoli, Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie, Simona Vetter, Seung-Woo Kim, Kyungmin Park, Krzysztof Kozlowski, Peter Griffin, Alim Akhtar Cc: dri-devel, linux-kernel, linux-arm-kernel, linux-samsung-soc, 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 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH v3 2/3] drm/bridge: samsung-dsim: use DSIM interrupt to wait for PLL stability 2026-07-22 18:58 ` [PATCH v3 2/3] drm/bridge: samsung-dsim: use DSIM interrupt to wait for PLL stability Kaustabh Chakraborty @ 2026-08-02 6:56 ` Inki Dae 2026-08-09 7:18 ` Kaustabh Chakraborty 0 siblings, 1 reply; 7+ messages in thread From: Inki Dae @ 2026-08-02 6:56 UTC (permalink / raw) To: Kaustabh Chakraborty Cc: Jagan Teki, Marek Szyprowski, Andrzej Hajda, Neil Armstrong, Robert Foss, Laurent Pinchart, Jonas Karlman, Jernej Skrabec, Luca Ceresoli, Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie, Simona Vetter, Seung-Woo Kim, Kyungmin Park, Krzysztof Kozlowski, Peter Griffin, Alim Akhtar, dri-devel, linux-kernel, linux-arm-kernel, linux-samsung-soc 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 > > ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v3 2/3] drm/bridge: samsung-dsim: use DSIM interrupt to wait for PLL stability 2026-08-02 6:56 ` Inki Dae @ 2026-08-09 7:18 ` Kaustabh Chakraborty 0 siblings, 0 replies; 7+ messages in thread From: Kaustabh Chakraborty @ 2026-08-09 7:18 UTC (permalink / raw) To: Inki Dae, Kaustabh Chakraborty Cc: Jagan Teki, Marek Szyprowski, Andrzej Hajda, Neil Armstrong, Robert Foss, Laurent Pinchart, Jonas Karlman, Jernej Skrabec, Luca Ceresoli, Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie, Simona Vetter, Seung-Woo Kim, Kyungmin Park, Krzysztof Kozlowski, Peter Griffin, Alim Akhtar, dri-devel, linux-kernel, linux-arm-kernel, linux-samsung-soc 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. ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH v3 3/3] drm/exynos: remove simplefb nodes before init 2026-07-22 18:58 [PATCH v3 0/3] Fixes and enhancements for Exynos (7870) DSIM bridge driver Kaustabh Chakraborty 2026-07-22 18:58 ` [PATCH v3 1/3] drm/bridge: samsung-dsim: enable MFLUSH_VS for Exynos 7870 DSIM Kaustabh Chakraborty 2026-07-22 18:58 ` [PATCH v3 2/3] drm/bridge: samsung-dsim: use DSIM interrupt to wait for PLL stability Kaustabh Chakraborty @ 2026-07-22 18:58 ` Kaustabh Chakraborty 2 siblings, 0 replies; 7+ messages in thread From: Kaustabh Chakraborty @ 2026-07-22 18:58 UTC (permalink / raw) To: Inki Dae, Jagan Teki, Marek Szyprowski, Andrzej Hajda, Neil Armstrong, Robert Foss, Laurent Pinchart, Jonas Karlman, Jernej Skrabec, Luca Ceresoli, Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie, Simona Vetter, Seung-Woo Kim, Kyungmin Park, Krzysztof Kozlowski, Peter Griffin, Alim Akhtar Cc: dri-devel, linux-kernel, linux-arm-kernel, linux-samsung-soc, 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 ^ permalink raw reply related [flat|nested] 7+ messages in thread
end of thread, other threads:[~2026-08-09 7:19 UTC | newest] Thread overview: 7+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2026-07-22 18:58 [PATCH v3 0/3] Fixes and enhancements for Exynos (7870) DSIM bridge driver Kaustabh Chakraborty 2026-07-22 18:58 ` [PATCH v3 1/3] drm/bridge: samsung-dsim: enable MFLUSH_VS for Exynos 7870 DSIM Kaustabh Chakraborty 2026-08-05 9:50 ` Luca Ceresoli 2026-07-22 18:58 ` [PATCH v3 2/3] drm/bridge: samsung-dsim: use DSIM interrupt to wait for PLL stability Kaustabh Chakraborty 2026-08-02 6:56 ` Inki Dae 2026-08-09 7:18 ` Kaustabh Chakraborty 2026-07-22 18:58 ` [PATCH v3 3/3] drm/exynos: remove simplefb nodes before init Kaustabh Chakraborty
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox