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 init when dp->plugged is set --
commit 4113312f878c771b378b82c05f3076f2ed79cfc7 Author: Dmitry Baryshkov <[email protected]> drm/msm/dp: don't take extra references on multiple plug events This commit skips subsequent HPD plug events in msm_dp_hpd_plug_handle() when dp->plugged is already set, avoiding extra runtime PM 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; [Severity: High] Could this early return cause msm_dp_hpd_plug_handle() to skip msm_dp_display_process_hpd_high() completely when detection runs before HPD plug handling? When connector detection runs while dp->plugged is false and a DisplayPort sink is connected (for example at boot via drm_helper_probe_single_connector_modes() -> drm_bridge_connector_detect_ctx(), on system resume, or via DRM_IOCTL_MODE_GETCONNECTOR), drm_bridge_connector_detect_ctx() calls detect->funcs->detect() immediately before drm_bridge_connector_hpd_notify(): drivers/gpu/drm/display/drm_bridge_connector.c:drm_bridge_connector_detect_ctx() { if (detect) { status = detect->funcs->detect(detect, connector); ... drm_bridge_connector_hpd_notify(connector, status, DRM_CONNECTOR_NO_EXTRA_STATUS); } } In msm_dp_bridge_detect(), priv->plugged is set to true after reading DPCD into local stack buffers, without calling msm_dp_display_process_hpd_high(): drivers/gpu/drm/msm/dp/dp_display.c:msm_dp_bridge_detect() { ... status = connector_status_connected; priv->plugged = true; } When drm_bridge_connector_hpd_notify() then invokes msm_dp_bridge_hpd_notify() -> msm_dp_hpd_plug_handle(), dp->plugged is already true, so msm_dp_hpd_plug_handle() returns 0 without calling msm_dp_display_process_hpd_high(). Similarly, if msm_dp_hpd_plug_handle() runs first and msm_dp_display_process_hpd_high() fails (for example returning -ENOTCONN in msm_dp_panel_read_link_caps() when a branch device is connected with sink_count == 0, or on a transient AUX timeout), msm_dp_hpd_plug_handle() still sets dp->plugged = true unconditionally: drivers/gpu/drm/msm/dp/dp_display.c:msm_dp_hpd_plug_handle() { ... ret = msm_dp_display_process_hpd_high(dp); ... dp->plugged = true; return ret; } Wouldn't both paths leave dp->panel->link_info, the connector EDID, LTTPR, and dp->link->sink_count uninitialized, causing msm_dp_display_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? -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=6
