Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [Medium] drm/msm/dp: persistent peripheral flush after disabling VSC SDP (state stickiness) --
commit b5e9083e5d055244149a4e0660254522762ffc0e Author: Xilin Wu <[email protected]> drm/msm/dp: Track VSC packets in bridge atomic state This commit moves YUV420 VSC construction to the atomic check phase and retains the packed packet in the private bridge state. It also programs the packet unchanged for normal streams while deriving the peripheral flush from both the old and new states. > diff --git a/drivers/gpu/drm/msm/dp/dp_display.c > b/drivers/gpu/drm/msm/dp/dp_display.c > --- a/drivers/gpu/drm/msm/dp/dp_display.c > +++ b/drivers/gpu/drm/msm/dp/dp_display.c > @@ -679,7 +681,8 @@ static int msm_dp_init_sub_modules(struct > msm_dp_display_private *dp) > static int msm_dp_display_set_mode(struct msm_dp *msm_dp_display, > const struct drm_display_mode *adjusted_mode, > struct msm_dp_panel *msm_dp_panel, > - const struct msm_dp_bridge_state *state) > + const struct msm_dp_bridge_state *state, > + const struct msm_dp_bridge_state *old_state) > { > struct msm_dp_display_private *dp; > u32 bpp; > @@ -691,8 +694,7 @@ static int msm_dp_display_set_mode(struct msm_dp > *msm_dp_display, > else > bpp = state->bpp; > > - msm_dp_panel->sdp_needs_flush = > msm_dp_panel->msm_dp_mode.out_fmt_is_yuv_420 || > - drm_mode_is_420_only(&msm_dp_panel->connector->display_info, > adjusted_mode); > + dp->sdp_needs_flush = old_state->vsc_enabled || state->vsc_enabled; [Severity: Medium] When transitioning from a mode with VSC SDP enabled (e.g., YUV420) to a mode without it (RGB), the display controller requires a peripheral flush to apply the disabled SDP configuration. Here, the expression correctly evaluates to true for that transition frame. However, since msm_dp_display_set_mode() is only executed during atomic_pre_enable (on modesets), does this code leave the flush flag persistently enabled? Because there is no mechanism here to clear dp->sdp_needs_flush after the transition frame completes, it appears the flag will stay true for all subsequent non-modeset page flips. [ ... ] > @@ -1563,7 +1578,7 @@ bool msm_dp_needs_periph_flush(const struct msm_dp > *msm_dp_display, > struct msm_dp_display_private *dp = container_of(msm_dp_display, > struct msm_dp_display_private, > msm_dp_display); > > - return dp->panel->sdp_needs_flush; > + return dp->sdp_needs_flush; [Severity: Medium] The DPU encoder queries msm_dp_needs_periph_flush() on every frame kickoff. If the value remains stuck at true after a transition away from a VSC SDP mode, does this cause the hardware to perform unnecessary peripheral flushes on every VSYNC during the entire duration of the new mode? Could we provide a way to clear the flush flag once the transition frame has been successfully kicked off? -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=17
