Re: [PATCH v2] drm/bridge: imx93-mipi-dsi: Fix mode validation

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Self-deadlock due to recursive acquisition of the non-recursive bridge_chain_mutex.
--

commit 6ef22e8ad4391e28924570bebb26f8dec0d5af77
Author: Liu Ying <victor.liu@nxp.com>

drm/bridge: imx93-mipi-dsi: Fix mode validation

This commit changes mode validation to iterate over all bridges after
the current bridge to check for DRM_BRIDGE_OP_DETECT and DRM_BRIDGE_OP_EDID
flags, instead of only checking the last bridge.

> diff --git a/drivers/gpu/drm/bridge/imx/imx93-mipi-dsi.c b/drivers/gpu/drm/bridge/imx/imx93-mipi-dsi.c
> index 8f312f9edf974..6d65df9ed9701 100644
> --- a/drivers/gpu/drm/bridge/imx/imx93-mipi-dsi.c
> +++ b/drivers/gpu/drm/bridge/imx/imx93-mipi-dsi.c
> @@ -493,21 +493,24 @@ static enum drm_mode_status
>  imx93_dsi_validate_mode(struct imx93_dsi *dsi, const struct drm_display_mode *mode)
>  {
>  	struct drm_bridge *dmd_bridge = dw_mipi_dsi_get_bridge(dsi->dmd);
> -	struct drm_bridge *last_bridge __free(drm_bridge_put) =
> -		drm_bridge_chain_get_last_bridge(dmd_bridge->encoder);
> -
> -	if ((last_bridge->ops & DRM_BRIDGE_OP_DETECT) &&
> -	    (last_bridge->ops & DRM_BRIDGE_OP_EDID)) {
> -		unsigned long pixel_clock_rate = mode->clock * 1000;
> -		unsigned long rounded_rate;
> -
> -		/* Allow +/-0.5% pixel clock rate deviation */
> -		rounded_rate = clk_round_rate(dsi->clk_pixel, pixel_clock_rate);
> -		if (rounded_rate < pixel_clock_rate * 995 / 1000 ||
> -		    rounded_rate > pixel_clock_rate * 1005 / 1000) {
> -			dev_dbg(dsi->dev, "failed to round clock for mode " DRM_MODE_FMT "\n",
> -				DRM_MODE_ARG(mode));
> -			return MODE_NOCLOCK;
> +
> +	drm_for_each_bridge_in_chain_from(dmd_bridge, bridge) {

Can this cause a self-deadlock?

imx93_dsi_validate_mode() acts as the .mode_valid callback for the bridge,
which is invoked by drm_bridge_chain_mode_valid(). The DRM core iterates over
the bridge chain using the drm_for_each_bridge_in_chain_from() macro, which
acquires and holds the non-recursive bridge->encoder->bridge_chain_mutex for
the duration of the loop.

By calling drm_for_each_bridge_in_chain_from() again inside the callback,
does it attempt to acquire the exact same bridge_chain_mutex? Since standard
mutexes are not recursive, this could result in an immediate deadlock.

> +		if ((bridge->ops & DRM_BRIDGE_OP_DETECT) &&
> +		    (bridge->ops & DRM_BRIDGE_OP_EDID)) {
> +			unsigned long pixel_clock_rate = mode->clock * 1000;

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260512-imx93-mipi-dsi-fix-mode-validation-v2-1-7aec3be5da2c@nxp.com?part=1