Re: [PATCH v9 13/14] drm/msm/dp: make bridge helpers use dp_display to allow re-use

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] Hardcoded encoder lookup in `msm_dp_display_atomic_pre_enable` breaks the intended MST reuse by dropping the virtual bridge context.
--

--- Patch [13]: [PATCH v9 13/14] drm/msm/dp: make bridge helpers use dp_display to allow re-use ---
commit 9cd0096573edf67dd79ce85b7bf559799b84cbe4
Author: Abhinav Kumar <quic_abhinavk@quicinc.com>

drm/msm/dp: make bridge helpers use dp_display to allow re-use

This patch modifies DP bridge helpers to directly take a msm_dp display
object rather than extracting it from drm_bridge. The stated goal is to
allow the MST bridge to reuse these same helpers.

> diff --git a/drivers/gpu/drm/msm/dp/dp_display.c b/drivers/gpu/drm/msm/dp/dp_display.c
> index 9b9e96b683c71..88ca263010c60 100644
> --- a/drivers/gpu/drm/msm/dp/dp_display.c
> +++ b/drivers/gpu/drm/msm/dp/dp_display.c

[ ... ]

> @@ -1408,18 +1405,16 @@ int msm_dp_modeset_init(struct msm_dp *msm_dp_display, struct drm_device *dev,
>  	return 0;
>  }
>  
> -void msm_dp_bridge_atomic_pre_enable(struct drm_bridge *drm_bridge,
> -				     struct drm_atomic_commit *state)
> +void msm_dp_display_atomic_pre_enable(struct msm_dp *msm_dp_display,
> +				      struct drm_atomic_commit *state)
>  {
> -	struct msm_dp_bridge *msm_dp_bridge = to_dp_bridge(drm_bridge);
> -	struct msm_dp *dp = msm_dp_bridge->msm_dp_display;
> -	struct msm_dp_display_private *msm_dp_display;
> +	struct msm_dp_display_private *dp;
>  	struct drm_crtc *crtc;
>  	struct drm_crtc_state *crtc_state;
>  
> -	msm_dp_display = container_of(dp, struct msm_dp_display_private, msm_dp_display);
> +	dp = container_of(msm_dp_display, struct msm_dp_display_private, msm_dp_display);
>  
> -	crtc = drm_atomic_get_new_crtc_for_encoder(state, drm_bridge->encoder);
> +	crtc = drm_atomic_get_new_crtc_for_encoder(state, msm_dp_display->bridge->encoder);

[Severity: Medium]
Will this hardcoded encoder lookup prevent MST bridges from reusing this
helper?

During an MST commit, the atomic state contains the virtual MST encoder. By
changing this from the caller's drm_bridge->encoder to the root DP encoder
(msm_dp_display->bridge->encoder), drm_atomic_get_new_crtc_for_encoder()
will return NULL for MST streams. This causes the function to return early
without programming the mode.

Should the helper still take the encoder or the calling bridge as an
argument to support MST reuse?

>  	if (!crtc)
>  		return;
>  	crtc_state = drm_atomic_get_new_crtc_state(state, crtc);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260728-dp_mstclean-v9-0-f7779fce10f4@oss.qualcomm.com?part=13