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 <[email protected]> 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 <[email protected]> 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/[email protected]?part=9
