diff mbox series

[24/51] drm: Manage drm_vblank_cleanup with drmm_

Message ID 20200302222631.3861340-25-daniel.vetter@ffwll.ch (mailing list archive)
State New, archived
Headers show
Series drm_device managed resources, v4 | expand

Commit Message

Daniel Vetter March 2, 2020, 10:26 p.m. UTC
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
sequence for all drivers, and that's still the case. At least without
more uses of drmm_ through explicit driver calls.

Also for this one we need drmm_kcalloc, so lets add those

v2: Sort includes (Laurent)

Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
---
 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(-)

Comments

Sam Ravnborg March 7, 2020, 8:28 a.m. UTC | #1
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@intel.com>

Acked-by: Sam Ravnborg <sam@ravnborg.org>

> ---
>  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@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
diff mbox series

Patch

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);