Re: [PATCH RFC v3 6/8] drm/bridge: Expose link training state via new DRM_BRIDGE_OP_DP flag

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] drm: Probe failure for DP pipelines ending in non-DP connectors
--

commit 1f392a73217bd39ddc3ecb68c1400cefdd77a913
Author: Kory Maincent <kory.maincent@bootlin.com>
Subject: drm/bridge: Expose link training state via new DRM_BRIDGE_OP_DP flag

This commit introduces DRM_BRIDGE_OP_DP, a new bridge operation flag for
bridges that provide DisplayPort connector operations with link training
support. Bridges advertising this flag must fill the dp_link_caps field
with their link capabilities.

> diff --git a/drivers/gpu/drm/display/drm_bridge_connector.c b/drivers/gpu/drm/display/drm_bridge_connector.c
> --- a/drivers/gpu/drm/display/drm_bridge_connector.c
> +++ b/drivers/gpu/drm/display/drm_bridge_connector.c
[ ... ]
> @@ -1079,6 +1097,17 @@ struct drm_connector *drm_bridge_connector_init(struct drm_device *drm,
>  					       connector_type, ddc);
>  		if (ret)
>  			return ERR_PTR(ret);
> +	} else if (bridge_connector->bridge_dp) {
> +		ret = drmm_connector_init(drm, connector,
> +					  &drm_bridge_connector_funcs,
> +					  connector_type, ddc);
> +		if (ret)
> +			return ERR_PTR(ret);
> +
> +		ret = drm_dp_source_set_caps(connector,
> +					     bridge_connector->bridge_dp->dp_link_caps);
> +		if (ret)
> +			return ERR_PTR(ret);

[Severity: High]
Does this cause a probe failure for DisplayPort pipelines that end in a non-DP
connector?

Earlier in drm_bridge_connector_init(), the final connector_type is determined
by the last bridge in the chain. If a display pipeline ends with an active 
converter (like a DP-to-VGA or DP-to-LVDS bridge), the resulting connector 
type will be non-DP (e.g., DRM_MODE_CONNECTOR_VGA).

However, this block unconditionally calls drm_dp_source_set_caps() as long as 
any upstream bridge advertises DRM_BRIDGE_OP_DP and there is no HDMI bridge.

Looking at drm_dp_source_set_caps() in drm_dp_helper.c, it has a strict check
for the connector type:

	if (!(connector->connector_type == DRM_MODE_CONNECTOR_DisplayPort ||
	      connector->connector_type == DRM_MODE_CONNECTOR_eDP))
		return -EINVAL;

If the pipeline ends in a VGA or LVDS connector, this will return -EINVAL,
which is then propagated up by drm_bridge_connector_init(), causing the entire
display pipeline initialization to fail for otherwise valid bridge chains.

Could we add a check here to ensure the connector type is actually DP or eDP 
before calling drm_dp_source_set_caps()?

>  	} else {
>  		ret = drmm_connector_init(drm, connector,
>  					  &drm_bridge_connector_funcs,
[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261012-feat_link_cap-v3-0-7517f4aba0f3@bootlin.com?part=6