Re: [PATCH 1/6] drm/vc4: bo: Add a managed action to cleanup the cache

[Daniel Vetter <daniel@ffwll.ch>]

On Thu, Oct 29, 2020 at 08:00:59PM +0100, Maxime Ripard wrote:
> The BO cache needs to be cleaned up using vc4_bo_cache_destroy, but it's
> not used consistently (vc4_drv's bind calls it in its error path, but
> doesn't in unbind), and we can make that automatic through a managed
> action. Let's remove the requirement to call vc4_bo_cache_destroy.
> 
> Fixes: c826a6e10644 ("drm/vc4: Add a BO cache.")
> Signed-off-by: Maxime Ripard <maxime@cerno.tech>
> ---
>  drivers/gpu/drm/vc4/vc4_bo.c  | 5 +++--
>  drivers/gpu/drm/vc4/vc4_drv.c | 1 -
>  drivers/gpu/drm/vc4/vc4_drv.h | 2 +-
>  3 files changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/vc4/vc4_bo.c b/drivers/gpu/drm/vc4/vc4_bo.c
> index f432278173cd..730d5775ab42 100644
> --- a/drivers/gpu/drm/vc4/vc4_bo.c
> +++ b/drivers/gpu/drm/vc4/vc4_bo.c
> @@ -1024,6 +1024,7 @@ int vc4_get_tiling_ioctl(struct drm_device *dev, void *data,
>  	return 0;
>  }
>  
> +static void vc4_bo_cache_destroy(struct drm_device *dev, void *unused);
>  int vc4_bo_cache_init(struct drm_device *dev)
>  {
>  	struct vc4_dev *vc4 = to_vc4_dev(dev);
> @@ -1052,10 +1053,10 @@ int vc4_bo_cache_init(struct drm_device *dev)
>  	INIT_WORK(&vc4->bo_cache.time_work, vc4_bo_cache_time_work);
>  	timer_setup(&vc4->bo_cache.time_timer, vc4_bo_cache_time_timer, 0);
>  
> -	return 0;
> +	return drmm_add_action(dev, vc4_bo_cache_destroy, NULL);

I think you want drmm_add_action_or_reset here, same in the other patches.

Otherwise if drmm_add_action fails, you leak the thing you just set up.
-Daniel

>  }
>  
> -void vc4_bo_cache_destroy(struct drm_device *dev)
> +static void vc4_bo_cache_destroy(struct drm_device *dev, void *unused)
>  {
>  	struct vc4_dev *vc4 = to_vc4_dev(dev);
>  	int i;
> diff --git a/drivers/gpu/drm/vc4/vc4_drv.c b/drivers/gpu/drm/vc4/vc4_drv.c
> index 8f10f609e4f8..eb3fcd8232b5 100644
> --- a/drivers/gpu/drm/vc4/vc4_drv.c
> +++ b/drivers/gpu/drm/vc4/vc4_drv.c
> @@ -303,7 +303,6 @@ static int vc4_drm_bind(struct device *dev)
>  gem_destroy:
>  	vc4_gem_destroy(drm);
>  	drm_mode_config_cleanup(drm);
> -	vc4_bo_cache_destroy(drm);
>  dev_put:
>  	drm_dev_put(drm);
>  	return ret;
> diff --git a/drivers/gpu/drm/vc4/vc4_drv.h b/drivers/gpu/drm/vc4/vc4_drv.h
> index 836fdca5e643..3a989e8b39a2 100644
> --- a/drivers/gpu/drm/vc4/vc4_drv.h
> +++ b/drivers/gpu/drm/vc4/vc4_drv.h
> @@ -14,6 +14,7 @@
>  #include <drm/drm_device.h>
>  #include <drm/drm_encoder.h>
>  #include <drm/drm_gem_cma_helper.h>
> +#include <drm/drm_managed.h>
>  #include <drm/drm_mm.h>
>  #include <drm/drm_modeset_lock.h>
>  
> @@ -808,7 +809,6 @@ struct drm_gem_object *vc4_prime_import_sg_table(struct drm_device *dev,
>  						 struct sg_table *sgt);
>  void *vc4_prime_vmap(struct drm_gem_object *obj);
>  int vc4_bo_cache_init(struct drm_device *dev);
> -void vc4_bo_cache_destroy(struct drm_device *dev);
>  int vc4_bo_inc_usecnt(struct vc4_bo *bo);
>  void vc4_bo_dec_usecnt(struct vc4_bo *bo);
>  void vc4_bo_add_to_purgeable_pool(struct vc4_bo *bo);
> -- 
> 2.26.2
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel