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 <[email protected]> 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/[email protected]?part=2
