* [PATCH v5 0/3] Fixes and enhancements for Exynos (7870) DSIM bridge driver
@ 2026-08-09 8:55 Kaustabh Chakraborty
2026-08-09 8:55 ` [PATCH v5 1/3] drm/bridge: samsung-dsim: enable MFLUSH_VS for Exynos 7870 DSIM Kaustabh Chakraborty
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Kaustabh Chakraborty @ 2026-08-09 8:55 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 v5:
- Fix incorrect goto label s/err_unbind_all/err_mode_config_cleanup (Sashiko)
- Link to v4: https://patch.msgid.link/20260809-exynos-dsim-fixes-v4-0-ca67c51e61b7@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>
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v5 1/3] drm/bridge: samsung-dsim: enable MFLUSH_VS for Exynos 7870 DSIM
2026-08-09 8:55 [PATCH v5 0/3] Fixes and enhancements for Exynos (7870) DSIM bridge driver Kaustabh Chakraborty
@ 2026-08-09 8:55 ` Kaustabh Chakraborty
2026-08-09 9:09 ` sashiko-bot
2026-08-09 8:56 ` [PATCH v5 2/3] drm/bridge: samsung-dsim: use DSIM interrupt to wait for PLL stability Kaustabh Chakraborty
2026-08-09 8:56 ` [PATCH v5 3/3] drm/exynos: remove simplefb nodes before init Kaustabh Chakraborty
2 siblings, 1 reply; 6+ messages in thread
From: Kaustabh Chakraborty @ 2026-08-09 8:55 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.
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
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH v5 2/3] drm/bridge: samsung-dsim: use DSIM interrupt to wait for PLL stability
2026-08-09 8:55 [PATCH v5 0/3] Fixes and enhancements for Exynos (7870) DSIM bridge driver Kaustabh Chakraborty
2026-08-09 8:55 ` [PATCH v5 1/3] drm/bridge: samsung-dsim: enable MFLUSH_VS for Exynos 7870 DSIM Kaustabh Chakraborty
@ 2026-08-09 8:56 ` Kaustabh Chakraborty
2026-08-09 9:14 ` sashiko-bot
2026-08-09 8:56 ` [PATCH v5 3/3] drm/exynos: remove simplefb nodes before init Kaustabh Chakraborty
2 siblings, 1 reply; 6+ messages in thread
From: Kaustabh Chakraborty @ 2026-08-09 8:56 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.
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
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH v5 3/3] drm/exynos: remove simplefb nodes before init
2026-08-09 8:55 [PATCH v5 0/3] Fixes and enhancements for Exynos (7870) DSIM bridge driver Kaustabh Chakraborty
2026-08-09 8:55 ` [PATCH v5 1/3] drm/bridge: samsung-dsim: enable MFLUSH_VS for Exynos 7870 DSIM Kaustabh Chakraborty
2026-08-09 8:56 ` [PATCH v5 2/3] drm/bridge: samsung-dsim: use DSIM interrupt to wait for PLL stability Kaustabh Chakraborty
@ 2026-08-09 8:56 ` Kaustabh Chakraborty
2 siblings, 0 replies; 6+ messages in thread
From: Kaustabh Chakraborty @ 2026-08-09 8:56 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..62eeacbc2180 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_mode_config_cleanup;
+
/* Try to bind all sub drivers. */
ret = component_bind_all(drm->dev, drm);
if (ret)
--
2.54.0
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v5 1/3] drm/bridge: samsung-dsim: enable MFLUSH_VS for Exynos 7870 DSIM
2026-08-09 8:55 ` [PATCH v5 1/3] drm/bridge: samsung-dsim: enable MFLUSH_VS for Exynos 7870 DSIM Kaustabh Chakraborty
@ 2026-08-09 9:09 ` sashiko-bot
0 siblings, 0 replies; 6+ messages in thread
From: sashiko-bot @ 2026-08-09 9:09 UTC (permalink / raw)
To: Kaustabh Chakraborty; +Cc: dri-devel
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues:
- [High] Hardware misidentification of Exynos 7870 causing incorrect display initialization sequence.
--
--- Patch [1]: [PATCH v5 1/3] drm/bridge: samsung-dsim: enable MFLUSH_VS for Exynos 7870 DSIM ---
commit 4604aa815a722cdb66f92b628f8218c57c5a77bb
Author: Kaustabh Chakraborty <kauschluss@disroot.org>
drm/bridge: samsung-dsim: enable MFLUSH_VS for Exynos 7870 DSIM
This commit enables the active-high MFLUSH_VS bit for the Exynos 7870 DSIM
hardware to properly flush the display FIFO at v-sync pulse. It fixes an
issue introduced by a previous commit that assumed MFLUSH_VS was active-low
across all 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;
[Severity: High]
This is a pre-existing issue, but since this patch adds specific support for
DSIM_TYPE_EXYNOS7870, does the samsung_dsim_hw_is_exynos() macro also need
to be updated to include it?
Currently, that macro is defined as:
include/drm/bridge/samsung-dsim.h:
#define samsung_dsim_hw_is_exynos(hw) \
((hw) >= DSIM_TYPE_EXYNOS3250 && (hw) <= DSIM_TYPE_EXYNOS5433)
Because DSIM_TYPE_EXYNOS7870 falls outside this range, the macro evaluates
to false for the Exynos 7870. This causes the driver to incorrectly treat
it as an i.MX device in several places.
For example, further down in this same samsung_dsim_init_link() function,
it will incorrectly apply the i.MX-specific non-continuous clock lane
configuration:
if (driver_data->has_clklane_stop &&
dsi->mode_flags & MIPI_DSI_CLOCK_NON_CONTINUOUS) {
if (!samsung_dsim_hw_is_exynos(dsi->plat_data->hw_type))
reg |= DSIM_NON_CONTINUOUS_CLKLANE;
It also causes samsung_dsim_atomic_pre_enable() to call samsung_dsim_init()
early:
if (!samsung_dsim_hw_is_exynos(dsi->plat_data->hw_type)) {
ret = samsung_dsim_init(dsi);
which breaks the expected Exynos DSI host initialization order.
Will this hardware misidentification cause display initialization failures
on Exynos 7870 hardware?
> }
>
> if (dsi->mode_flags & MIPI_DSI_MODE_NO_EOT_PACKET)
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260809-exynos-dsim-fixes-v5-0-6e63dc6c238f@disroot.org?part=1
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v5 2/3] drm/bridge: samsung-dsim: use DSIM interrupt to wait for PLL stability
2026-08-09 8:56 ` [PATCH v5 2/3] drm/bridge: samsung-dsim: use DSIM interrupt to wait for PLL stability Kaustabh Chakraborty
@ 2026-08-09 9:14 ` sashiko-bot
0 siblings, 0 replies; 6+ messages in thread
From: sashiko-bot @ 2026-08-09 9:14 UTC (permalink / raw)
To: Kaustabh Chakraborty; +Cc: dri-devel
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Using `wait_for_completion_timeout()` with a short, sub-jiffy timeout leads to premature expiration and spurious display initialization failures.
- [High] An early return in the interrupt handler causes `DSIM_INT_PLL_STABLE` events to be silently discarded if they coalesce with `DSIM_INT_SW_RST_RELEASE`.
--
--- Patch [2]: [PATCH v5 2/3] drm/bridge: samsung-dsim: use DSIM interrupt to wait for PLL stability ---
commit 9e7e8160e685bf48c532c28ad86a19b0799f53ce
Author: Kaustabh Chakraborty <kauschluss@disroot.org>
drm/bridge: samsung-dsim: use DSIM interrupt to wait for PLL stability
This commit replaces a busy-wait polling loop for checking PLL stability
with an interrupt-driven completion mechanism.
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
> index 6260c091c3df8..216a7cc9d5c55 100644
> --- 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;
> + 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]
Does this code risk premature timeout expiration?
When timeout_us is 100, usecs_to_jiffies(100) translates to 1 jiffy on
standard kernel configurations (e.g., HZ=1000 or lower). Passing a 1-jiffy
timeout to wait_for_completion_timeout() only guarantees waiting until the
next timer tick, which can expire almost immediately depending on tick
alignment.
This might cause spurious failures to initialize the display PLL, since the
robust 3000-iteration polling loop has been replaced with a single register
read after this wait.
> 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);
[Severity: High]
Can DSIM_INT_PLL_STABLE events be silently discarded here if they coalesce
with DSIM_INT_SW_RST_RELEASE?
If both DSIM_INT_SW_RST_RELEASE and DSIM_INT_PLL_STABLE are set
simultaneously in status (which may happen if reset and PLL initialization
run concurrently on platforms where wait_for_reset is false), the code inside
the if (status & DSIM_INT_SW_RST_RELEASE) block executes and returns
IRQ_HANDLED early.
Since the interrupts are already cleared in hardware by the
samsung_dsim_write(dsi, DSIM_INTSRC_REG, status) call earlier in this function,
complete(&dsi->pll_stabilized) would be skipped entirely, potentially causing
the waiter to time out.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260809-exynos-dsim-fixes-v5-0-6e63dc6c238f@disroot.org?part=2
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2026-08-09 9:14 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-08-09 8:55 [PATCH v5 0/3] Fixes and enhancements for Exynos (7870) DSIM bridge driver Kaustabh Chakraborty
2026-08-09 8:55 ` [PATCH v5 1/3] drm/bridge: samsung-dsim: enable MFLUSH_VS for Exynos 7870 DSIM Kaustabh Chakraborty
2026-08-09 9:09 ` sashiko-bot
2026-08-09 8:56 ` [PATCH v5 2/3] drm/bridge: samsung-dsim: use DSIM interrupt to wait for PLL stability Kaustabh Chakraborty
2026-08-09 9:14 ` sashiko-bot
2026-08-09 8:56 ` [PATCH v5 3/3] drm/exynos: remove simplefb nodes before init Kaustabh Chakraborty
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.