Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [High] Redundant call to msm_dp_panel_disable_vsc_sdp() transmits Secondary Data Packet (SDP) updates over the main link to a sink that has already been powered down. --
--- Patch [9]: [PATCH v9 09/14] drm/msm/dp: re-arrange dp_display_disable() into functional parts --- commit 71c4fed95d00e4e9b083de79dc56ede059ae5722 Author: Abhinav Kumar <[email protected]> drm/msm/dp: re-arrange dp_display_disable() into functional parts dp_display_disable() handles special case of when monitor is disconnected from the dongle while the dongle stays connected thereby needing a separate function dp_ctrl_off_link_stream() for this. However with a slight rework this can still be handled by keeping common paths same for regular and special case. > diff --git a/drivers/gpu/drm/msm/dp/dp_display.c > b/drivers/gpu/drm/msm/dp/dp_display.c > index d094d484474cd..cd8a3b171d9db 100644 > --- a/drivers/gpu/drm/msm/dp/dp_display.c > +++ b/drivers/gpu/drm/msm/dp/dp_display.c > @@ -716,12 +716,20 @@ static int msm_dp_display_disable(struct > msm_dp_display_private *dp) > if (!msm_dp_display->power_on) > return 0; > > + msm_dp_panel_disable_vsc_sdp(dp->panel); > + > + /* dongle is still connected but sinks are disconnected */ > if (dp->link->sink_count == 0) { > /* > * irq_hpd with sink_count = 0 > * hdmi unplugged out of dongle > */ > - msm_dp_ctrl_off_link_stream(dp->ctrl); > + > + /* set dongle to D3 (power off) mode */ > + msm_dp_link_psm_config(dp->link, &dp->panel->link_info, true); > + msm_dp_ctrl_off(dp->ctrl); [Severity: High] Does this code call msm_dp_panel_disable_vsc_sdp() a second time after the sink is already powered down? msm_dp_display_disable() first disables the VSC SDP: msm_dp_panel_disable_vsc_sdp(dp->panel); And then it places the sink into D3 power down state: msm_dp_link_psm_config(dp->link, &dp->panel->link_info, true); However, msm_dp_ctrl_off() also inherently calls msm_dp_panel_disable_vsc_sdp(). Because msm_dp_panel_disable_vsc_sdp() writes to configuration registers and triggers msm_dp_panel_update_sdp() to transmit an SDP over the main link, could this redundant call cause the DP controller to timeout waiting for a response from the already powered-down sink, or leave the display hardware unresponsive? > + /* re-init the PHY so that we can listen to Dongle disconnect */ > + msm_dp_ctrl_reinit_phy(dp->ctrl); > } else { > /* > * unplugged interrupt -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=9
