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

[sashiko-bot@kernel.org]

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