Re: [PATCH v6 1/7] drm/bridge: aux-hpd-bridge: Add drm_dev_has_dp_hpd_bridge()

[Sebastian Reichel <sebastian.reichel@collabora.com>]

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

Hi,

On Tue, Aug 04, 2026 at 03:07:24PM +0800, Chaoyi Chen wrote:
> From: Chaoyi Chen <chaoyi.chen@rock-chips.com>
> 
> Add a new API to check whether a DisplayPort HPD bridge has already
> been registered. This helps avoid duplicate registration of the same
> HPD bridge, although the current framework allows doing so.
> 
> Suggested-by: Sebastian Reichel <sebastian.reichel@collabora.com>
> Signed-off-by: Chaoyi Chen <chaoyi.chen@rock-chips.com>
> ---

Reviewed-by: Sebastian Reichel <sebastian.reichel@collabora.com>
Tested-by: Sebastian Reichel <sebastian.reichel@collabora.com>

I still see the bridge registered twice for fusb302. Apparently the
the new notifier is faster than the explicit registration in the
driver. The code added here works though; reloading
aux_hpd_typec_dp_bridge does not register more and more bridges :)

Greetings,

-- Sebastian

>  drivers/gpu/drm/bridge/aux-hpd-bridge.c | 42 ++++++++++++++++++++++++-
>  include/drm/bridge/aux-bridge.h         |  6 ++++
>  2 files changed, 47 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/bridge/aux-hpd-bridge.c b/drivers/gpu/drm/bridge/aux-hpd-bridge.c
> index f02a38a2638a..a56c88eba005 100644
> --- a/drivers/gpu/drm/bridge/aux-hpd-bridge.c
> +++ b/drivers/gpu/drm/bridge/aux-hpd-bridge.c
> @@ -12,6 +12,8 @@
>  #include <drm/drm_bridge.h>
>  #include <drm/bridge/aux-bridge.h>
>  
> +#define DRM_AUX_HPD_BRIDGE_NAME	"dp_hpd_bridge"
> +
>  static DEFINE_IDA(drm_aux_hpd_bridge_ida);
>  
>  struct drm_aux_hpd_bridge_data {
> @@ -36,6 +38,44 @@ static void drm_aux_hpd_bridge_free_adev(void *_adev)
>  	auxiliary_device_uninit(_adev);
>  }
>  
> +static int hpd_bridge_match(struct device *dev, const void *data)
> +{
> +	const struct device_node *np = data;
> +	struct auxiliary_device *adev;
> +
> +	if (!dev_is_auxiliary(dev))
> +		return 0;
> +
> +	adev = to_auxiliary_dev(dev);
> +	if (strcmp(adev->name, DRM_AUX_HPD_BRIDGE_NAME))
> +		return 0;
> +
> +	return adev->dev.platform_data == np;
> +}
> +
> +/**
> + * drm_dev_has_dp_hpd_bridge - check whether a HPD DisplayPort bridge is registered
> + * @parent: device instance providing this bridge
> + * @np: device node pointer corresponding to this bridge instance
> + *
> + * Walk the children of @parent and check whether a HPD DisplayPort bridge for
> + * the given @np has already been registered via devm_drm_dp_hpd_bridge_add().
> + *
> + * Return: true if a HPD bridge for @parent / @np already exists, false otherwise
> + */
> +bool drm_dev_has_dp_hpd_bridge(struct device *parent, struct device_node *np)
> +{
> +	struct device *child;
> +
> +	child = device_find_child(parent, np, hpd_bridge_match);
> +	if (child) {
> +		put_device(child);
> +		return true;
> +	}
> +	return false;
> +}
> +EXPORT_SYMBOL_GPL(drm_dev_has_dp_hpd_bridge);
> +
>  /**
>   * devm_drm_dp_hpd_bridge_alloc - allocate a HPD DisplayPort bridge
>   * @parent: device instance providing this bridge
> @@ -63,7 +103,7 @@ struct auxiliary_device *devm_drm_dp_hpd_bridge_alloc(struct device *parent, str
>  	}
>  
>  	adev->id = ret;
> -	adev->name = "dp_hpd_bridge";
> +	adev->name = DRM_AUX_HPD_BRIDGE_NAME;
>  	adev->dev.parent = parent;
>  	adev->dev.release = drm_aux_hpd_bridge_release;
>  	adev->dev.platform_data = of_node_get(np);
> diff --git a/include/drm/bridge/aux-bridge.h b/include/drm/bridge/aux-bridge.h
> index c2f5a855512f..cca07a8e2d45 100644
> --- a/include/drm/bridge/aux-bridge.h
> +++ b/include/drm/bridge/aux-bridge.h
> @@ -25,6 +25,7 @@ struct auxiliary_device *devm_drm_dp_hpd_bridge_alloc(struct device *parent, str
>  int devm_drm_dp_hpd_bridge_add(struct device *dev, struct auxiliary_device *adev);
>  struct device *drm_dp_hpd_bridge_register(struct device *parent,
>  					  struct device_node *np);
> +bool drm_dev_has_dp_hpd_bridge(struct device *parent, struct device_node *np);
>  void drm_aux_hpd_bridge_notify(struct device *dev, enum drm_connector_status status);
>  #else
>  static inline struct auxiliary_device *devm_drm_dp_hpd_bridge_alloc(struct device *parent,
> @@ -44,6 +45,11 @@ static inline struct device *drm_dp_hpd_bridge_register(struct device *parent,
>  	return NULL;
>  }
>  
> +static inline bool drm_dev_has_dp_hpd_bridge(struct device *parent, struct device_node *np)
> +{
> +	return false;
> +}
> +
>  static inline void drm_aux_hpd_bridge_notify(struct device *dev, enum drm_connector_status status)
>  {
>  }
> -- 
> 2.53.0
> 

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