Re: [PATCH v6 15/26] drm/bridge: devm_drm_of_get_bridge and drmm_of_get_bridge: automatically put the bridge

Dmitry Baryshkov
2025-02-07T03:17:47Z

On Thu, Feb 06, 2025 at 07:14:30PM +0100, Luca Ceresoli wrote:
> Add a devm/drmm action to these functions so the bridge reference is
> dropped automatically when the caller is removed.

I think the get() should go to the underlying of_drm_bridge_find() function.

Also it really feels like it's an overkill to keep the wrappers. After
getting bridge being handled by the panel code would it be possible to
drop all of them? Then this patch might introduce one new devm_
function? Or are drmm_ functions actually being used to store data in
the drmm-managed memory?

>
> Signed-off-by: Luca Ceresoli <luca.ceresoli@bootlin.com>
>
> ---
>
> This patch was added in v6.
> ---
> drivers/gpu/drm/drm_bridge.c | 39 +++++++++++++++++++++++++++++++++++++++
> 1 file changed, 39 insertions(+)
>
> diff --git a/drivers/gpu/drm/drm_bridge.c b/drivers/gpu/drm/drm_bridge.c
> index f694b32ca59cb91c32846bc00b43360df41cc1ad..497ec06dfcb05ab5dee8ea5e8f1eafb9c2807612 100644
> --- a/drivers/gpu/drm/drm_bridge.c
> +++ b/drivers/gpu/drm/drm_bridge.c
> @@ -33,6 +33,7 @@
> #include <drm/drm_edid.h>
> #include <drm/drm_encoder.h>
> #include <drm/drm_file.h>
> +#include <drm/drm_managed.h>
> #include <drm/drm_of.h>
> #include <drm/drm_print.h>
>
> @@ -1459,6 +1460,13 @@ static int of_drm_find_bridge_by_endpoint(const struct device_node *np,
> return ret;
> }
>
> +static void devm_drm_bridge_put_void(void *data)
> +{
> + struct drm_bridge *bridge = (struct drm_bridge *)data;
> +
> + drm_bridge_put(bridge);
> +}
> +
> /**
> * devm_drm_of_get_bridge - Return next bridge in the chain
> * @dev: device to tie the bridge lifetime to
> @@ -1469,6 +1477,10 @@ static int of_drm_find_bridge_by_endpoint(const struct device_node *np,
> * Given a DT node's port and endpoint number, finds the connected node
> * and returns the associated bridge if any.
> *
> + * The refcount of the returned bridge is incremented, but the caller does
> + * not have to call drm_bridge_put() when done with the bridge. It will be
> + * done by devres when @dev is removed.
> + *
> * Returns a pointer to the bridge if successful, or an error pointer
> * otherwise.
> */
> @@ -1483,6 +1495,10 @@ struct drm_bridge *devm_drm_of_get_bridge(struct device *dev,
> if (ret)
> return ERR_PTR(ret);
>
> + ret = devm_add_action_or_reset(dev, devm_drm_bridge_put_void, bridge);
> + if (ret)
> + return ERR_PTR(ret);
> +
> return bridge;
> }
> EXPORT_SYMBOL(devm_drm_of_get_bridge);
> @@ -1497,6 +1513,10 @@ EXPORT_SYMBOL(devm_drm_of_get_bridge);
> * graph link search is not enough, e.g. for drivers that need to support
> * panels described only as subnodes.
> *
> + * The refcount of the returned bridge is incremented, but the caller does
> + * not have to call drm_bridge_put() when done with the bridge. It will be
> + * done by devres when @dev is removed.
> + *
> * RETURNS:
> * A pointer to the bridge if successful, or an error pointer otherwise.
> */
> @@ -1513,10 +1533,21 @@ struct drm_bridge *devm_drm_of_get_bridge_by_node(struct device *dev,
> if (!bridge)
> return ERR_PTR(-ENODEV);
>
> + ret = devm_add_action_or_reset(dev, devm_drm_bridge_put_void, bridge);
> + if (ret)
> + return ERR_PTR(ret);
> +
> return bridge;
> }
> EXPORT_SYMBOL(devm_drm_of_get_bridge_by_node);
>
> +static void drmm_bridge_put_void(struct drm_device *drm, void *ptr)
> +{
> + struct drm_bridge *bridge = ptr;
> +
> + drm_bridge_put(bridge);
> +}
> +
> /**
> * drmm_of_get_bridge - Return next bridge in the chain
> * @drm: device to tie the bridge lifetime to
> @@ -1527,6 +1558,10 @@ EXPORT_SYMBOL(devm_drm_of_get_bridge_by_node);
> * Given a DT node's port and endpoint number, finds the connected node
> * and returns the associated bridge if any.
> *
> + * The refcount of the returned bridge is incremented, but the caller does
> + * not have to call drm_bridge_put() when done with the bridge. It will be
> + * done by drmm when @dev is removed.
> + *
> * Returns a drmm managed pointer to the bridge if successful, or an error
> * pointer otherwise.
> */
> @@ -1541,6 +1576,10 @@ struct drm_bridge *drmm_of_get_bridge(struct drm_device *drm,
> if (ret)
> return ERR_PTR(ret);
>
> + ret = drmm_add_action_or_reset(drm, drmm_bridge_put_void, bridge);
> + if (ret)
> + return ERR_PTR(ret);
> +
> return bridge;
> }
> EXPORT_SYMBOL(drmm_of_get_bridge);
>
> --
> 2.34.1
>

--
With best wishes
Dmitry