Re: [PATCH 30/37] drm/bridge: add drm_bridge_is_tail() to know whether a bridge completes the pipeline

[Maxime Ripard <mripard@kernel.org>]

[-- Attachment #1: Type: text/plain, Size: 4618 bytes --]

On Tue, May 19, 2026 at 12:37:47PM +0200, Luca Ceresoli wrote:
> For bridge hotplug we need to successfully probe a card with an incomplete
> bridge chain, i.e. a chain whose last bridge currently in bridge_chain
> needs a next_bridge to work. Such a card would have no connector, and be
> able to add one as soon as the followong bridges are added up to the tail
> bridge (e.g. a panel_bridge or a connector_bridge).
> 
> Currently common DRM code is unable to tell whether the last bridge
> currently in the chain is the tail bridge (= the pipeline is complete) or
> not.
> 
> Add drm_bridge_is_tail(), and a .is_tail func for it to rely on, so common
> code can know whether a bridge is a tail bridge or needs a next_bridge.
> 
> Signed-off-by: Luca Ceresoli <luca.ceresoli@bootlin.com>
> ---
>  drivers/gpu/drm/drm_bridge.c | 28 ++++++++++++++++++++++++++++
>  include/drm/drm_bridge.h     | 19 +++++++++++++++++++
>  2 files changed, 47 insertions(+)
> 
> diff --git a/drivers/gpu/drm/drm_bridge.c b/drivers/gpu/drm/drm_bridge.c
> index c62d17e84d4f..2b539c9749a6 100644
> --- a/drivers/gpu/drm/drm_bridge.c
> +++ b/drivers/gpu/drm/drm_bridge.c
> @@ -646,6 +646,34 @@ void drm_bridge_detach(struct drm_bridge *bridge)
>  	drm_bridge_put(bridge);
>  }
>  
> +/**
> + * drm_bridge_is_tail - check whether the bridge is the last required to
> + *                      have a full pipeline
> + * @bridge: the bridge to check
> + *
> + * Tell whether this is a tail bridge, i.e. a bridge that does not need a
> + * next bridge to work. E.g. a panel_bridge is final, a DSI-to-LVDS bridge
> + * is not.
> + *
> + * In case of hotplug the last bridge currently in the chain (as in
> + * drm_bridge_chain_get_last_bridge()) might not be final, and be waiting
> + * for a pipeline tail to be connected.
> + *
> + * Return: true if this bridge does not need a next bridge to work, false
> + * otherwise
> + */
> +bool drm_bridge_is_tail(struct drm_bridge *bridge)
> +{
> +	if (!(bridge->ops & DRM_BRIDGE_OP_IS_TAIL)) {
> +		drm_warn_once(bridge->dev, "is_tail func not implemented by bridge %ps!",
> +			      bridge->funcs);
> +		return false;
> +	}
> +
> +	return bridge->funcs->is_tail(bridge);
> +}
> +EXPORT_SYMBOL(drm_bridge_is_tail);
> +
>  /**
>   * DOC: bridge operations
>   *
> diff --git a/include/drm/drm_bridge.h b/include/drm/drm_bridge.h
> index 4ba3a5deef9a..d783a6fb93a0 100644
> --- a/include/drm/drm_bridge.h
> +++ b/include/drm/drm_bridge.h
> @@ -78,6 +78,19 @@ struct drm_bridge_funcs {
>  	int (*attach)(struct drm_bridge *bridge, struct drm_encoder *encoder,
>  		      enum drm_bridge_attach_flags flags);
>  
> +	/**
> +	 * @is_tail:
> +	 *
> +	 * Returns true if this is a tail bridge, i.e. it does not need a
> +	 * next bridge to work. E.g. a panel_bridge is a tail bridge, a
> +	 * DSI-to-LVDS bridge is not a tail bridge (no matter whether the
> +	 * next bridge is present or not).

Why a DSI-to-LDVS bridge isn't a tail bridge? It only needs a panel
next, right?

> +	 * The @is_tail callback is optional but it is required if the
> +	 * bridge is part of a pipeline with hot-pluggable components.
> +	 */
> +	bool (*is_tail)(struct drm_bridge *bridge);
> +

I don't think that's the right way to think about it, if only because
you never really know at the driver level if you're supposed to be last
or not. A DSI-to-LVDS bridge might just as well be chained with an
LVDS-to-eDP bridge, or feed the panel directly without any additional
bridge.

I *think* that what you're trying to find out here is whether the chain
is complete or not. I think you can get the same information by checking
whether you have a connector for that bridge chain. If you don't, you
know the chain isn't complete, and if you do, it's supposed to be.
DRM_BRIDGE_ATTACH_NO_CONNECTOR could be useful too, because if all you
bridges support it but there's no connector, there's something wrong.

Anyway.

Maxime

>  	/**
>  	 * @destroy:
>  	 *
> @@ -1092,6 +1105,11 @@ enum drm_bridge_ops {
>  	 * &drm_bridge_funcs->hdmi_clear_spd_infoframe callbacks.
>  	 */
>  	DRM_BRIDGE_OP_HDMI_SPD_INFOFRAME = BIT(10),
> +	/**
> +	 * @DRM_BRIDGE_OP_IS_TAIL: The bridge implements the
> +	 * &drm_bridge_funcs->is_tail callback.
> +	 */
> +	DRM_BRIDGE_OP_IS_TAIL = BIT(11),
>  };

That part is confusing to me. This is meant for drm_bridge_connector,
but since we need to handle the case where we don't have a connector
(and thus no drm_bridge_connector) I don't think this should be used at
all?

Maxime

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 273 bytes --]