On Mon, Mar 02, 2020 at 11:26:04PM +0100, Daniel Vetter wrote:
> Nothing special here, except that this is the first time that we
> automatically clean up something that's initialized with an explicit
> driver call. But the cleanup was done at the very of the release
the very what?
> sequence for all drivers, and that's still the case. At least without
> more uses of drmm_ through explicit driver calls.
This patch does not simplify things in itself.
The motivation here is to remove the drm_dev_fini()
dependencies from the drivers.
So drm_dev_fini() can be dropped in a follow-up patch.
Maybe extend the changelog a little?
> Also for this one we need drmm_kcalloc, so lets add those
>
> v2: Sort includes (Laurent)
>
> Signed-off-by: Daniel Vetter <daniel.vetter@xxxxxxxxx>
Acked-by: Sam Ravnborg <sam@xxxxxxxxxxxx>
> ---
> drivers/gpu/drm/drm_drv.c | 1 -
> drivers/gpu/drm/drm_internal.h | 1 -
> drivers/gpu/drm/drm_vblank.c | 31 ++++++++++++-------------------
> include/drm/drm_managed.h | 16 ++++++++++++++++
> 4 files changed, 28 insertions(+), 21 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
> index 9a646155dfc6..90b6ae81d431 100644
> --- a/drivers/gpu/drm/drm_drv.c
> +++ b/drivers/gpu/drm/drm_drv.c
> @@ -752,7 +752,6 @@ EXPORT_SYMBOL(devm_drm_dev_init);
> */
> void drm_dev_fini(struct drm_device *dev)
> {
> - drm_vblank_cleanup(dev);
> }
> EXPORT_SYMBOL(drm_dev_fini);
>
> diff --git a/drivers/gpu/drm/drm_internal.h b/drivers/gpu/drm/drm_internal.h
> index cb09e95a795e..e67015d07c4c 100644
> --- a/drivers/gpu/drm/drm_internal.h
> +++ b/drivers/gpu/drm/drm_internal.h
> @@ -94,7 +94,6 @@ void drm_managed_release(struct drm_device *dev);
>
> /* drm_vblank.c */
> void drm_vblank_disable_and_save(struct drm_device *dev, unsigned int pipe);
> -void drm_vblank_cleanup(struct drm_device *dev);
>
> /* IOCTLS */
> int drm_wait_vblank_ioctl(struct drm_device *dev, void *data,
> diff --git a/drivers/gpu/drm/drm_vblank.c b/drivers/gpu/drm/drm_vblank.c
> index 47fc4339ec7f..5a6ec8aa0873 100644
> --- a/drivers/gpu/drm/drm_vblank.c
> +++ b/drivers/gpu/drm/drm_vblank.c
> @@ -30,6 +30,7 @@
> #include <drm/drm_crtc.h>
> #include <drm/drm_drv.h>
> #include <drm/drm_framebuffer.h>
> +#include <drm/drm_managed.h>
> #include <drm/drm_modeset_helper_vtables.h>
> #include <drm/drm_print.h>
> #include <drm/drm_vblank.h>
> @@ -425,14 +426,10 @@ static void vblank_disable_fn(struct timer_list *t)
> spin_unlock_irqrestore(&dev->vbl_lock, irqflags);
> }
>
> -void drm_vblank_cleanup(struct drm_device *dev)
> +static void drm_vblank_init_release(struct drm_device *dev, void *ptr)
> {
> unsigned int pipe;
>
> - /* Bail if the driver didn't call drm_vblank_init() */
> - if (dev->num_crtcs == 0)
> - return;
> -
> for (pipe = 0; pipe < dev->num_crtcs; pipe++) {
> struct drm_vblank_crtc *vblank = &dev->vblank[pipe];
>
> @@ -441,10 +438,6 @@ void drm_vblank_cleanup(struct drm_device *dev)
>
> del_timer_sync(&vblank->disable_timer);
> }
> -
> - kfree(dev->vblank);
> -
> - dev->num_crtcs = 0;
> }
>
> /**
> @@ -453,25 +446,29 @@ void drm_vblank_cleanup(struct drm_device *dev)
> * @num_crtcs: number of CRTCs supported by @dev
> *
> * This function initializes vblank support for @num_crtcs display pipelines.
> - * Cleanup is handled by the DRM core, or through calling drm_dev_fini() for
> - * drivers with a &drm_driver.release callback.
> + * Cleanup is handled automatically through a cleanup function added with
> + * drmm_add_action().
> *
> * Returns:
> * Zero on success or a negative error code on failure.
> */
> int drm_vblank_init(struct drm_device *dev, unsigned int num_crtcs)
> {
> - int ret = -ENOMEM;
> + int ret;
> unsigned int i;
>
> spin_lock_init(&dev->vbl_lock);
> spin_lock_init(&dev->vblank_time_lock);
>
> + dev->vblank = drmm_kcalloc(dev, num_crtcs, sizeof(*dev->vblank), GFP_KERNEL);
> + if (!dev->vblank)
> + return -ENOMEM;
> +
> dev->num_crtcs = num_crtcs;
>
> - dev->vblank = kcalloc(num_crtcs, sizeof(*dev->vblank), GFP_KERNEL);
> - if (!dev->vblank)
> - goto err;
> + ret = drmm_add_action(dev, drm_vblank_init_release, NULL);
> + if (ret)
> + return ret;
>
> for (i = 0; i < num_crtcs; i++) {
> struct drm_vblank_crtc *vblank = &dev->vblank[i];
> @@ -486,10 +483,6 @@ int drm_vblank_init(struct drm_device *dev, unsigned int num_crtcs)
> DRM_INFO("Supports vblank timestamp caching Rev 2 (21.10.2013).\n");
>
> return 0;
> -
> -err:
> - dev->num_crtcs = 0;
> - return ret;
> }
> EXPORT_SYMBOL(drm_vblank_init);
>
> diff --git a/include/drm/drm_managed.h b/include/drm/drm_managed.h
> index 5280209dff92..2b1ba2ad5582 100644
> --- a/include/drm/drm_managed.h
> +++ b/include/drm/drm_managed.h
> @@ -4,6 +4,7 @@
> #define _DRM_MANAGED_H_
>
> #include <linux/gfp.h>
> +#include <linux/overflow.h>
> #include <linux/types.h>
>
> struct drm_device;
> @@ -28,6 +29,21 @@ static inline void *drmm_kzalloc(struct drm_device *dev, size_t size, gfp_t gfp)
> {
> return drmm_kmalloc(dev, size, gfp | __GFP_ZERO);
> }
> +static inline void *drmm_kmalloc_array(struct drm_device *dev,
> + size_t n, size_t size, gfp_t flags)
> +{
> + size_t bytes;
> +
> + if (unlikely(check_mul_overflow(n, size, &bytes)))
> + return NULL;
> +
> + return drmm_kmalloc(dev, bytes, flags);
> +}
> +static inline void *drmm_kcalloc(struct drm_device *dev,
> + size_t n, size_t size, gfp_t flags)
> +{
> + return drmm_kmalloc_array(dev, n, size, flags | __GFP_ZERO);
> +}
> char *drmm_kstrdup(struct drm_device *dev, const char *s, gfp_t gfp);
>
> void drmm_kfree(struct drm_device *dev, void *data);
> --
> 2.24.1
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@xxxxxxxxxxxxxxxxxxxxx
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel
