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.ceres...@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
