[PATCH v2 0/3] drm/bridge: cdns-dsi: A few fixes

[PATCH v2 0/3] drm/bridge: cdns-dsi: A few fixes

[Tomi Valkeinen]

Cadence DSI bridge fixes for missing stop-state and bus flags.

Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
---
Changes in v2:
- Add Fixes: tags
- Link to v1: https://lore.kernel.org/r/20260327-cdns-dsi-fixes-v1-0-088edd40e97f@ideasonboard.com

---
Tomi Valkeinen (3):
      drm/bridge: cdns-dsi: Add clk debug print
      drm/bridge: cdns-dsi: Fix stop-state at enable time
      drm/bridge: cdns-dsi: Fix mode and bus flags

 drivers/gpu/drm/bridge/cadence/cdns-dsi-core.c | 25 ++++++++++++++++++++++---
 1 file changed, 22 insertions(+), 3 deletions(-)
---
base-commit: df2908090cda368b01ff43709f51890076c56157
change-id: 20260327-cdns-dsi-fixes-4e1b506e415d

Best regards,
--  
Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>

[PATCH v2 1/3] drm/bridge: cdns-dsi: Add clk debug print

[Tomi Valkeinen]

I keep adding this debug print while debugging panel problems. So let's
add it properly.

Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
---
 drivers/gpu/drm/bridge/cadence/cdns-dsi-core.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/gpu/drm/bridge/cadence/cdns-dsi-core.c b/drivers/gpu/drm/bridge/cadence/cdns-dsi-core.c
index c3e466ded84a..4b7a3bf58255 100644
--- a/drivers/gpu/drm/bridge/cadence/cdns-dsi-core.c
+++ b/drivers/gpu/drm/bridge/cadence/cdns-dsi-core.c
@@ -718,6 +718,9 @@ static void cdns_dsi_bridge_atomic_pre_enable(struct drm_bridge *bridge,
 	u32 tmp, reg_wakeup, div, status;
 	int nlanes;
 
+	dev_dbg(dsi->base.dev, "bitclk %lu, byteclk %lu\n",
+		phy_cfg->hs_clk_rate, phy_cfg->hs_clk_rate / 8);
+
 	/*
 	 * The cdns-dsi controller needs to be enabled before it's DPI source
 	 * has begun streaming. If this is not followed, there is a brief window

-- 
2.43.0

[PATCH v2 2/3] drm/bridge: cdns-dsi: Fix stop-state at enable time

[Tomi Valkeinen]

When using continuous clock, the cdns dsi doesn't keep the clock lanes in
LP-11 long enough to signal a stop state, although the data lanes are
kept high long enough. The data lanes most likely stay in LP-11 until
there is data to send, so the LP-11 duration is not defined.

While some DSI peripherals seem to work fine without correct stop-state
on the clock lane, at least Xilinx D-PHY RX fails to initialize if it
does not observe correct stop-state.

Add explicit stop-state handling to the driver. This is not described in
the documentation, but observing the clk and data lanes with an
oscilloscope, the solution in this patch seems to work:

Set the FORCE_STOP flags for clock and data lanes before starting the
D-PHY power up and init sequence, and drop the FORCE_STOP flags after
the init has been done, and 100 us has passed.

Fixes: e19233955d9e ("drm/bridge: Add Cadence DSI driver")
Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
---
 drivers/gpu/drm/bridge/cadence/cdns-dsi-core.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/drivers/gpu/drm/bridge/cadence/cdns-dsi-core.c b/drivers/gpu/drm/bridge/cadence/cdns-dsi-core.c
index 4b7a3bf58255..b9c87f13177a 100644
--- a/drivers/gpu/drm/bridge/cadence/cdns-dsi-core.c
+++ b/drivers/gpu/drm/bridge/cadence/cdns-dsi-core.c
@@ -635,9 +635,16 @@ static void cdns_dsi_hs_init(struct cdns_dsi *dsi)
 {
 	struct cdns_dsi_output *output = &dsi->output;
 	u32 status;
+	u32 val;
 
 	if (dsi->phy_initialized)
 		return;
+
+	/* Set force stop state on all lanes */
+	val = readl(dsi->regs + MCTL_MAIN_EN);
+	val |= DATA_FORCE_STOP | CLK_FORCE_STOP;
+	writel(val, dsi->regs + MCTL_MAIN_EN);
+
 	/*
 	 * Power all internal DPHY blocks down and maintain their reset line
 	 * asserted before changing the DPHY config.
@@ -661,6 +668,13 @@ static void cdns_dsi_hs_init(struct cdns_dsi *dsi)
 	writel(DPHY_CMN_PSO | DPHY_ALL_D_PDN | DPHY_C_PDN | DPHY_CMN_PDN |
 	       DPHY_D_RSTB(output->dev->lanes) | DPHY_C_RSTB,
 	       dsi->regs + MCTL_DPHY_CFG0);
+
+	/* Keep stop state for at least 100 us */
+	usleep_range(100, 200);
+
+	val &= ~(DATA_FORCE_STOP | CLK_FORCE_STOP);
+	writel(val, dsi->regs + MCTL_MAIN_EN);
+
 	dsi->phy_initialized = true;
 }
 

-- 
2.43.0

[PATCH v2 3/3] drm/bridge: cdns-dsi: Fix mode and bus flags

[Tomi Valkeinen]

The driver adjusts the flags in adjusted_mode to force the source to
send negative syncs. However, cdns-dsi also requires DE high, which is
not handled at all.

Add the correct DE high, along with explicit drive flags, via
bridge_state->input_bus_cfg.flags.

Also make the mode flags more robust: instead of passing the flags
forward with corrected h/v syncs, just set the full flags variable, as
there's no reason to pass any flags from the connector side to the
source.

Fixes: e19233955d9e ("drm/bridge: Add Cadence DSI driver")
Signed-off-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
---
 drivers/gpu/drm/bridge/cadence/cdns-dsi-core.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/bridge/cadence/cdns-dsi-core.c b/drivers/gpu/drm/bridge/cadence/cdns-dsi-core.c
index b9c87f13177a..ebaa060025a9 100644
--- a/drivers/gpu/drm/bridge/cadence/cdns-dsi-core.c
+++ b/drivers/gpu/drm/bridge/cadence/cdns-dsi-core.c
@@ -967,9 +967,11 @@ static int cdns_dsi_bridge_atomic_check(struct drm_bridge *bridge,
 	struct videomode vm;
 	long pclk;
 
-	/* cdns-dsi requires negative syncs */
-	adjusted_mode->flags &= ~(DRM_MODE_FLAG_PHSYNC | DRM_MODE_FLAG_PVSYNC);
-	adjusted_mode->flags |= DRM_MODE_FLAG_NHSYNC | DRM_MODE_FLAG_NVSYNC;
+	/* cdns-dsi requires negative syncs, and DE high */
+	adjusted_mode->flags = DRM_MODE_FLAG_NHSYNC | DRM_MODE_FLAG_NVSYNC;
+	bridge_state->input_bus_cfg.flags = DRM_BUS_FLAG_DE_HIGH |
+					    DRM_BUS_FLAG_PIXDATA_DRIVE_POSEDGE |
+					    DRM_BUS_FLAG_SYNC_DRIVE_POSEDGE;
 
 	/*
 	 * The DPHY PLL has quite a coarsely grained clock rate options. See

-- 
2.43.0

Re: [PATCH v2 2/3] drm/bridge: cdns-dsi: Fix stop-state at enable time

[sashiko-bot]

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

New issues:
- [High] The patch introduces a sleeping delay (`usleep_range`) in an atomic context path, violating DRM display driver guidelines.

Pre-existing issues:
- [High] Pre-existing use of a sleeping poll macro (`readl_poll_timeout`) in an atomic context path.
--

commit 831dd94ae2ea6c134a442329e348bfa99f691859
Author: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>

drm/bridge: cdns-dsi: Fix stop-state at enable time

This patch addresses an initialization failure with the Xilinx D-PHY RX by
ensuring the clock and data lanes are kept in a stop-state during the D-PHY
power up and init sequence. It adds FORCE_STOP flags and a delay to allow
the stop-state to take effect.

> diff --git a/drivers/gpu/drm/bridge/cadence/cdns-dsi-core.c b/drivers/gpu/drm/bridge/cadence/cdns-dsi-core.c
> index 4b7a3bf582551..b9c87f13177af 100644
> --- a/drivers/gpu/drm/bridge/cadence/cdns-dsi-core.c
> +++ b/drivers/gpu/drm/bridge/cadence/cdns-dsi-core.c
[ ... ]
> @@ -661,6 +668,13 @@ static void cdns_dsi_hs_init(struct cdns_dsi *dsi)
>  	writel(DPHY_CMN_PSO | DPHY_ALL_D_PDN | DPHY_C_PDN | DPHY_CMN_PDN |
>  	       DPHY_D_RSTB(output->dev->lanes) | DPHY_C_RSTB,
>  	       dsi->regs + MCTL_DPHY_CFG0);
> +
> +	/* Keep stop state for at least 100 us */
> +	usleep_range(100, 200);

[Severity: High]
Is it safe to introduce a sleeping delay in an atomic context path here?

Looking at the call chain:
drm_atomic_helper_commit_tail()
  drm_atomic_helper_commit_modeset_enables()
    cdns_dsi_bridge_atomic_pre_enable()
      cdns_dsi_hs_init()

The DRM guide specifies drm_atomic_helper_commit_tail() as an atomic
context path where sleeping is forbidden. Introducing usleep_range() here
might lead to kernel warnings, system instability, or regressions during a
non-blocking modeset request.

[Severity: High]
This is a pre-existing issue, but are the readl_poll_timeout() calls in this
path also sleeping in an atomic context?

For example, earlier in cdns_dsi_hs_init():

cdns_dsi_hs_init() {
    ...
    WARN_ON_ONCE(readl_poll_timeout(dsi->regs + MCTL_MAIN_STS, status,
                                    status & PLL_LOCKED, 100, 100));
    ...
}

There is also a similar call in cdns_dsi_bridge_atomic_pre_enable():

cdns_dsi_bridge_atomic_pre_enable() {
    ...
    if (readl_poll_timeout(dsi->regs + MCTL_MAIN_STS, status,
                           (tmp == (status & tmp)), 100, 500000))
    ...
}

Since readl_poll_timeout() internally calls usleep_range(), could these
similarly cause regressions in the atomic commit path?

> +
> +	val &= ~(DATA_FORCE_STOP | CLK_FORCE_STOP);
> +	writel(val, dsi->regs + MCTL_MAIN_EN);
> +
>  	dsi->phy_initialized = true;
>  }
>

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260907-cdns-dsi-fixes-v2-0-96d9b116e9bc@ideasonboard.com?part=2