Re: [PATCH v6 9/9] usb: typec: ucsi: huawei-gaokun: pass down HPD_IRQ events

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] drm/msm/dp: missing closing brace in msm_dp_bridge_hpd_notify()
- [High] drm/msm/dp: msm_dp_hpd_plug_handle() skips HPD high processing after detect
--

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

drm/msm: dp: handle the IRQ_HPD events reported by USB-C

This commit updates msm_dp_bridge_hpd_notify() in the msm DisplayPort driver
to handle DRM_CONNECTOR_DP_IRQ_HPD events reported via out-of-band
notifications.

> diff --git a/drivers/gpu/drm/msm/dp/dp_display.c b/drivers/gpu/drm/msm/dp/dp_display.c
> index ce919a26fba28..6a8b3b8e1fe7f 100644
> --- a/drivers/gpu/drm/msm/dp/dp_display.c
> +++ b/drivers/gpu/drm/msm/dp/dp_display.c
> @@ -1566,11 +1566,13 @@ void msm_dp_bridge_hpd_notify(struct drm_bridge *bridge,
>  		if (hpd_link_status == ISR_HPD_REPLUG_COUNT) {
>  			msm_dp_hpd_unplug_handle(dp);
>  			msm_dp_hpd_plug_handle(dp);
> -		} else if (hpd_link_status == ISR_IRQ_HPD_PULSE_COUNT) {
> -			msm_dp_irq_hpd_handle(dp);
>  		} else {
>  			msm_dp_hpd_plug_handle(dp);
>  		}
> +
> +		if (hpd_link_status == ISR_IRQ_HPD_PULSE_COUNT ||
> +		    event == DRM_CONNECTOR_DP_IRQ_HPD) {
> +			msm_dp_irq_hpd_handle(dp);
>  	} else {
>  		msm_dp_hpd_unplug_handle(dp);
>  	}

[Severity: High]
Is there a missing closing brace for the inner if statement in
msm_dp_bridge_hpd_notify() in drivers/gpu/drm/msm/dp/dp_display.c?

The opening brace after checking ISR_IRQ_HPD_PULSE_COUNT and
DRM_CONNECTOR_DP_IRQ_HPD is never closed before the else branch:

	if (status == connector_status_connected) {
		...
		if (hpd_link_status == ISR_IRQ_HPD_PULSE_COUNT ||
		    event == DRM_CONNECTOR_DP_IRQ_HPD) {
			msm_dp_irq_hpd_handle(dp);
	} else {
		msm_dp_hpd_unplug_handle(dp);
	}

This leaves the outer if block unclosed, causing a build failure at the end
of drivers/gpu/drm/msm/dp/dp_display.c as well as pairing the
msm_dp_hpd_unplug_handle(dp) else branch with the inner check instead of
status == connector_status_connected.


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

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

This commit adds an early return in msm_dp_hpd_plug_handle() when
dp->plugged is already set so sequential plug events do not take extra
runtime power management references.

> 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;
> +
>  	ret = pm_runtime_resume_and_get(&pdev->dev);
>  	if (ret) {
>  		DRM_ERROR("failed to pm_runtime_resume\n");

[Severity: High]
Will returning early when dp->plugged is true in msm_dp_hpd_plug_handle()
cause msm_dp_display_process_hpd_high() to be skipped after connector
detection?

During connector probing in
drivers/gpu/drm/display/drm_bridge_connector.c:drm_bridge_connector_detect_ctx(),
detect->funcs->detect() is called before drm_bridge_connector_hpd_notify():

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

When detect->funcs->detect() runs msm_dp_bridge_detect() in
drivers/gpu/drm/msm/dp/dp_display.c, it reads the receiver capabilities and
sets priv->plugged = true without calling msm_dp_display_process_hpd_high():

	ret = drm_dp_read_desc(priv->aux, &desc, drm_dp_is_branch(dpcd));
	if (ret) {
		DRM_DEBUG_DP("failed to read desc\n");
		priv->plugged = false;
		goto end;
	}

	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 plug notification
arrives), dp->plugged is already true, so msm_dp_hpd_plug_handle() returns 0
immediately without calling msm_dp_display_process_hpd_high().

Without msm_dp_display_process_hpd_high(), drm_edid_connector_update() is
not called, dp->panel link parameters remain uninitialized, and
dp->link->sink_count stays 0, causing link preparation to fail with
-ENOTCONN.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261022-hpd-irq-events-v6-0-337ea3ddaa62@oss.qualcomm.com?part=9

_______________________________________________
linux-amlogic mailing list
linux-amlogic@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-amlogic