Re: [PATCH v7 6/9] drm/msm/dp: don't take extra references on multiple plug events

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] drm/msm/dp: msm_dp_hpd_plug_handle() skips hpd_high after detect()
--

commit c63c7584ce165272a5840e2bdd5f618e2bd07d8f
Author: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>

drm/msm/dp: don't take extra references on multiple plug events

This commit skips further HPD plug events in msm_dp_hpd_plug_handle() if
dp->plugged is already set, avoiding extra runtime PM references until an
unplug event is received.

> diff --git a/drivers/gpu/drm/msm/dp/dp_display.c b/drivers/gpu/drm/msm/dp/dp_display.c
> index fa8942922b533..ce919a26fba28 100644
> --- a/drivers/gpu/drm/msm/dp/dp_display.c
> +++ b/drivers/gpu/drm/msm/dp/dp_display.c
> @@ -412,6 +412,9 @@ static int msm_dp_hpd_plug_handle(struct msm_dp_display_private *dp)
>  
>  	guard(mutex)(&dp->plugged_lock);
>  
> +	if (dp->plugged)
> +		return 0;

[Severity: High]
Can this cause msm_dp_display_process_hpd_high() to be skipped altogether
when msm_dp_bridge_detect() runs before msm_dp_hpd_plug_handle()?

In drivers/gpu/drm/display/drm_bridge_connector.c,
drm_bridge_connector_detect_ctx() invokes the bridge detect callback
immediately before notifying HPD:

	if (detect) {
		status = detect->funcs->detect(detect, connector);
		...
		drm_bridge_connector_hpd_notify(connector, status, DRM_CONNECTOR_NO_EXTRA_STATUS);
	}

When a DisplayPort sink is connected and drm_bridge_connector_detect_ctx()
runs (for example during boot-time connector probing, resume, or
DRM_IOCTL_MODE_GETCONNECTOR before an HPD interrupt is processed),
msm_dp_bridge_detect() reads DPCD into a local buffer and sets
priv->plugged = true without calling msm_dp_display_process_hpd_high():

	status = connector_status_connected;
	priv->plugged = true;

When drm_bridge_connector_hpd_notify() -> msm_dp_bridge_hpd_notify() ->
msm_dp_hpd_plug_handle() then runs, or when a subsequent HPD plug event
arrives, this new check sees dp->plugged == true and returns early.

Because msm_dp_display_process_hpd_high() is never called, LTTPR,
dp->panel->link_info, connector EDID, dp->link->sink_count, and
dp->audio_supported remain uninitialized, causing msm_dp_bridge_get_modes()
to return 0 modes, msm_dp_display_mode_valid() to reject modes with
MODE_BAD, and msm_dp_display_prepare_link() to fail with -ENOTCONN.

Also, at the end of msm_dp_hpd_plug_handle():

	ret = msm_dp_display_process_hpd_high(dp);
	...
	dp->plugged = true;

	return ret;

dp->plugged is set to true even when msm_dp_display_process_hpd_high()
fails, which with this early return prevents subsequent HPD plug events
from retrying initialization.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261023-hpd-irq-events-v7-0-57a3aea3f09e@oss.qualcomm.com?part=6