Re: [PATCH v2 17/20] drm/msm/dp: Track VSC packets in bridge atomic state

[sashiko-bot@kernel.org]

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 <sophon@radxa.com>

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/20261009-msm-dp-hdr10-v2-0-1835d4966da3@radxa.com?part=17

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