Re: [PATCH v9 12/14] drm/msm/dp: simplify link and clock disable sequence

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Time-Of-Check to Time-Of-Use (TOCTOU) race condition on `dp->link->sink_count` leading to mismatched hardware state.
--

--- Patch [12]: [PATCH v9 12/14] drm/msm/dp: simplify link and clock disable sequence ---
commit 68b08e6a0d7bd864688b398d6052cc9412efcc7c
Author: Yongxing Mou <yongxing.mou@oss.qualcomm.com>

drm/msm/dp: simplify link and clock disable sequence

This commit moves common disable steps out of the sink_count check to make
the flow easier to follow.

> diff --git a/drivers/gpu/drm/msm/dp/dp_display.c b/drivers/gpu/drm/msm/dp/dp_display.c
> index 6e392a99438f8..9b9e96b683c71 100644
> --- a/drivers/gpu/drm/msm/dp/dp_display.c
> +++ b/drivers/gpu/drm/msm/dp/dp_display.c

[ ... ]

> +	/* dongle is still connected but sinks are disconnected */
> +	if (dp->link->sink_count == 0)
>  		msm_dp_link_psm_config(dp->link, &dp->panel->link_info, true);
> -		msm_dp_ctrl_off_pixel_clk(dp->ctrl);
> -		msm_dp_ctrl_off_link(dp->ctrl, dp->panel);
> +
> +	msm_dp_ctrl_off_link(dp->ctrl, dp->panel);
> +
> +	if (dp->link->sink_count == 0)

[Severity: High]
Can this introduce a race condition if sink_count changes between these two
checks?

Looking at msm_dp_display_disable(), the sink_count is now evaluated twice.
If a user plugs or unplugs a monitor exactly when this disable sequence
is occurring, the HPD event thread might locklessly update sink_count:

CPU1 (msm_dp_display_disable)
    if (dp->link->sink_count == 0) // evaluates to true
        msm_dp_link_psm_config(...);

CPU2 (HPD IRQ thread)
    msm_dp_link_process_request()
        dp->link->sink_count = 1; // updated locklessly

CPU1
    msm_dp_ctrl_off_link(...);
    if (dp->link->sink_count == 0) // now evaluates to false
    else
        msm_dp_display_host_phy_exit(dp);

If this state diverges, the driver might completely disable the PHY while
a dongle is still connected and in PSM mode, preventing the hardware from
detecting subsequent dongle disconnects.

Would it be safer to cache the initial sink_count in a local variable to
ensure consistent state transitions throughout the function?

> +		/* re-init the PHY so that we can listen to Dongle disconnect */
>  		msm_dp_ctrl_reinit_phy(dp->ctrl);
> -	} else {
> -		/*
> -		 * unplugged interrupt
> -		 * dongle unplugged out of DUT
> -		 */
> -		msm_dp_ctrl_off_pixel_clk(dp->ctrl);
> -		msm_dp_ctrl_off_link(dp->ctrl, dp->panel);
> +	else
>  		msm_dp_display_host_phy_exit(dp);
> -	}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260728-dp_mstclean-v9-0-f7779fce10f4@oss.qualcomm.com?part=12