Re: [PATCH v9 18/22] drm/bridge: synopsys: dw-dp: Drop useless reservation of first slot

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Unbalanced resource lifecycle operations on error paths in dw_dp_bridge_atomic_enable
--

--- Patch [18]: [PATCH v9 18/22] drm/bridge: synopsys: dw-dp: Drop useless reservation of first slot ---
commit 7167a1f6669c417c9c943c6894f5b2a33b32f0bf
Author: Sebastian Reichel <sebastian.reichel-ZGY8ohtN/[email protected]>

drm/bridge: synopsys: dw-dp: Drop useless reservation of first slot

This commit removes the reservation of the first SDP slot in the atomic_enable
path. The reservation was unclear in origin and could race with audio SDP
reservation features.

> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-dp.c b/drivers/gpu/drm/bridge/synopsys/dw-dp.c
> index 19b6c11f672f9..05d3485a4d8db 100644
> --- a/drivers/gpu/drm/bridge/synopsys/dw-dp.c
> +++ b/drivers/gpu/drm/bridge/synopsys/dw-dp.c
> @@ -1706,9 +1706,6 @@ static void dw_dp_bridge_atomic_enable(struct drm_bridge *bridge,
>  		return;
>  	}
>  
> -	scoped_guard(mutex, &dp->sdp_lock)
> -		set_bit(0, dp->sdp_reg_bank);
> -
>  	ret = dw_dp_link_enable(dp);
>  	if (ret < 0) {
>  		dev_err(dp->dev, "failed to enable link: %d\n", ret);

[Severity: High]
Does this error path leave dp->pm_active = true while the PHY is powered off?

If dw_dp_link_enable() fails, it cleans up by calling phy_power_off():

dw_dp_link_enable() {
    ...
err_phy_power_off:
    phy_power_off(dp->phy);
    return ret;
}

However, when we return early here, dp->pm_active remains true. Because the
DRM core considers the atomic commit successful even if the bridge enable
callback returns early, a subsequent mode-set or display-off operation will
invoke dw_dp_bridge_atomic_disable().

Because dp->pm_active is true, the disable path proceeds to call
dw_dp_link_disable(), which unconditionally calls phy_power_off() again:

dw_dp_bridge_atomic_disable() {
    if (!dp->pm_active)
        return;
    dp->pm_active = false;
    ...
    dw_dp_link_disable(dp);
}

dw_dp_link_disable() {
    ...
    phy_power_off(dp->phy);
    ...
}

Can this unbalanced phy_power_off() cause the PHY power count to go negative,
causing subsequent power-on attempts to silently fail?

This same unbalanced teardown could also occur earlier in this function if
drm_atomic_get_new_connector_for_encoder() or
drm_atomic_get_new_connector_state() return NULL, as dp->pm_active is set to
true before those checks.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260803-synopsys-dw-dp-improvements-v9-0-2e6e46135312-ZGY8ohtN/[email protected]?part=18