[PATCH 00/19] drm: debugfs: Remove all files automatically on cleanup

[PATCH 00/19] drm: debugfs: Remove all files automatically on cleanup

[Noralf Trønnes]

This patchset removes the need for drivers to clean up their debugfs
files on exit. It is done automatically in drm_debugfs_cleanup().
This funtion is also called should the driver error out in it's
drm_driver.debugfs_init callback.

Two drivers still use drm_debugfs_remove_files():
- tegra in it's connectors, not sure if I can remove it.
- qxl because it's debugfs files list is part of struct qxl_device which
  is freed on unload before drm_minor_unregister() is called cleaning debugfs.

Daniel,
I was unable to remove the drm_driver.debugfs_cleanup hook completely,
since drm/msm uses it to free memory. Maybe it can be freed elsewhere.


Note:
The driver patches are only compile tested.


Noralf.


Noralf Trønnes (19):
  drm: debugfs: Remove all files automatically on cleanup
  drm: drm_minor_register(): Clean up debugfs on failure
  drm/atomic: Remove drm_atomic_debugfs_cleanup()
  drm/amd/amdgpu: Remove drm_debugfs_remove_files() call
  drm/armada: Remove armada_drm_debugfs_cleanup()
  drm/etnaviv: allow build with COMPILE_TEST
  drm/etnaviv: Remove etnaviv_debugfs_cleanup()
  drm/hdlcd: Remove hdlcd_debugfs_cleanup()
  drm/msm: Remove drm_debugfs_remove_files() calls
  drm/nouveau: Remove nouveau_drm_debugfs_cleanup()
  drm/omap: Remove omap_debugfs_cleanup()
  drm/radeon: Remove drm_debugfs_remove_files() call
  drm/sti: Remove drm_debugfs_remove_files() calls
  drm/tegra: Remove tegra_debugfs_cleanup()
  drm/tilcdc: Remove tilcdc_debugfs_cleanup()
  drm/vc4: Remove vc4_debugfs_cleanup()
  drm/virtio: Remove virtio_gpu_debugfs_takedown()
  drm/qxl: Remove qxl_debugfs_takedown()
  drm/i915: Remove i915_debugfs_unregister()

 drivers/gpu/drm/amd/amdgpu/amdgpu.h        |  1 -
 drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 20 ------
 drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c    |  1 -
 drivers/gpu/drm/arm/hdlcd_drv.c            |  7 ---
 drivers/gpu/drm/armada/armada_debugfs.c    | 65 +++-----------------
 drivers/gpu/drm/armada/armada_drm.h        |  1 -
 drivers/gpu/drm/armada/armada_drv.c        |  3 -
 drivers/gpu/drm/drm_atomic.c               |  7 ---
 drivers/gpu/drm/drm_crtc_internal.h        |  1 -
 drivers/gpu/drm/drm_debugfs.c              | 29 +++++----
 drivers/gpu/drm/drm_drv.c                  |  2 +-
 drivers/gpu/drm/etnaviv/Kconfig            |  2 +-
 drivers/gpu/drm/etnaviv/etnaviv_drv.c      |  7 ---
 drivers/gpu/drm/i915/i915_debugfs.c        | 97 ++++--------------------------
 drivers/gpu/drm/i915/i915_drv.c            |  1 -
 drivers/gpu/drm/i915/i915_drv.h            |  2 -
 drivers/gpu/drm/i915/intel_drv.h           |  1 -
 drivers/gpu/drm/i915/intel_pipe_crc.c      | 68 ++++-----------------
 drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.c    |  7 ---
 drivers/gpu/drm/msm/msm_debugfs.c          |  2 -
 drivers/gpu/drm/msm/msm_perf.c             | 29 +--------
 drivers/gpu/drm/msm/msm_rd.c               | 31 +---------
 drivers/gpu/drm/nouveau/nouveau_debugfs.c  | 62 ++++---------------
 drivers/gpu/drm/nouveau/nouveau_debugfs.h  |  6 --
 drivers/gpu/drm/nouveau/nouveau_drm.c      |  1 -
 drivers/gpu/drm/omapdrm/omap_debugfs.c     |  9 ---
 drivers/gpu/drm/omapdrm/omap_drv.c         |  1 -
 drivers/gpu/drm/omapdrm/omap_drv.h         |  1 -
 drivers/gpu/drm/qxl/qxl_debugfs.c          |  9 ---
 drivers/gpu/drm/qxl/qxl_drv.c              |  1 -
 drivers/gpu/drm/qxl/qxl_drv.h              |  1 -
 drivers/gpu/drm/radeon/radeon_device.c     | 16 -----
 drivers/gpu/drm/sti/sti_drv.c              | 48 ++-------------
 drivers/gpu/drm/sti/sti_dvo.c              | 10 ---
 drivers/gpu/drm/sti/sti_hda.c              | 11 ----
 drivers/gpu/drm/sti/sti_hdmi.c             | 11 ----
 drivers/gpu/drm/sti/sti_tvout.c            |  8 ---
 drivers/gpu/drm/tegra/drm.c                |  7 ---
 drivers/gpu/drm/tilcdc/tilcdc_drv.c        | 12 ----
 drivers/gpu/drm/tilcdc/tilcdc_drv.h        |  2 -
 drivers/gpu/drm/vc4/vc4_debugfs.c          |  6 --
 drivers/gpu/drm/vc4/vc4_drv.c              |  1 -
 drivers/gpu/drm/vc4/vc4_drv.h              |  1 -
 drivers/gpu/drm/virtio/virtgpu_debugfs.c   |  8 ---
 drivers/gpu/drm/virtio/virtgpu_drv.c       |  1 -
 drivers/gpu/drm/virtio/virtgpu_drv.h       |  1 -
 46 files changed, 76 insertions(+), 542 deletions(-)

--
2.10.2

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 01/19] drm: debugfs: Remove all files automatically on cleanup

[Noralf Trønnes]

Instead of having the drivers call drm_debugfs_remove_files() in
their drm_driver->debugfs_cleanup hook, do it automatically by
traversing minor->debugfs_list.
Also use debugfs_remove_recursive() so drivers who add their own
debugfs files don't have to keep track of them for removal.

Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
---
 drivers/gpu/drm/drm_debugfs.c | 20 +++++++++++++++++---
 1 file changed, 17 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/drm_debugfs.c b/drivers/gpu/drm/drm_debugfs.c
index 37fd612..04b0af3 100644
--- a/drivers/gpu/drm/drm_debugfs.c
+++ b/drivers/gpu/drm/drm_debugfs.c
@@ -81,7 +81,8 @@ static const struct file_operations drm_debugfs_fops = {
  * \return Zero on success, non-zero on failure
  *
  * Create a given set of debugfs files represented by an array of
- * gdm_debugfs_lists in the given root directory.
+ * &drm_info_list in the given root directory. These files will be removed
+ * automatically on drm_debugfs_cleanup().
  */
 int drm_debugfs_create_files(const struct drm_info_list *files, int count,
 			     struct dentry *root, struct drm_minor *minor)
@@ -218,6 +219,19 @@ int drm_debugfs_remove_files(const struct drm_info_list *files, int count,
 }
 EXPORT_SYMBOL(drm_debugfs_remove_files);
 
+static void drm_debugfs_remove_all_files(struct drm_minor *minor)
+{
+	struct drm_info_node *node, *tmp;
+
+	mutex_lock(&minor->debugfs_lock);
+	list_for_each_entry_safe(node, tmp, &minor->debugfs_list, list) {
+		debugfs_remove(node->dent);
+		list_del(&node->list);
+		kfree(node);
+	}
+	mutex_unlock(&minor->debugfs_lock);
+}
+
 /**
  * Cleanup the debugfs filesystem resources.
  *
@@ -245,9 +259,9 @@ int drm_debugfs_cleanup(struct drm_minor *minor)
 		}
 	}
 
-	drm_debugfs_remove_files(drm_debugfs_list, DRM_DEBUGFS_ENTRIES, minor);
+	drm_debugfs_remove_all_files(minor);
 
-	debugfs_remove(minor->debugfs_root);
+	debugfs_remove_recursive(minor->debugfs_root);
 	minor->debugfs_root = NULL;
 
 	return 0;
-- 
2.10.2

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 01/19] drm: debugfs: Remove all files automatically on cleanup

[Daniel Vetter]

On Thu, Jan 26, 2017 at 11:56:03PM +0100, Noralf Trønnes wrote:
> Instead of having the drivers call drm_debugfs_remove_files() in
> their drm_driver->debugfs_cleanup hook, do it automatically by
> traversing minor->debugfs_list.
> Also use debugfs_remove_recursive() so drivers who add their own
> debugfs files don't have to keep track of them for removal.
> 
> Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
> ---
>  drivers/gpu/drm/drm_debugfs.c | 20 +++++++++++++++++---
>  1 file changed, 17 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_debugfs.c b/drivers/gpu/drm/drm_debugfs.c
> index 37fd612..04b0af3 100644
> --- a/drivers/gpu/drm/drm_debugfs.c
> +++ b/drivers/gpu/drm/drm_debugfs.c
> @@ -81,7 +81,8 @@ static const struct file_operations drm_debugfs_fops = {
>   * \return Zero on success, non-zero on failure
>   *
>   * Create a given set of debugfs files represented by an array of
> - * gdm_debugfs_lists in the given root directory.
> + * &drm_info_list in the given root directory. These files will be removed
> + * automatically on drm_debugfs_cleanup().

I'm starting to feel guilty about asking you all this, but you're doing
great work so here we go: I just noticed that the comments here aren't
really kernel-doc, and that the drm_debugfs.[hc] stuff ins't pulled into
the sphinx documentation (which is why it's not checked). Would be really
awesome if you could add a chapter to gpu/drm-uapi.rst about debugfs
interfaces (with a big warning that it's not stable api and for debugging
only), and pull the kerneldoc for it (plus fix it to be properly parsing
kerenldoc)? Strictly optional :-)
-Daniel

>   */
>  int drm_debugfs_create_files(const struct drm_info_list *files, int count,
>  			     struct dentry *root, struct drm_minor *minor)
> @@ -218,6 +219,19 @@ int drm_debugfs_remove_files(const struct drm_info_list *files, int count,
>  }
>  EXPORT_SYMBOL(drm_debugfs_remove_files);
>  
> +static void drm_debugfs_remove_all_files(struct drm_minor *minor)
> +{
> +	struct drm_info_node *node, *tmp;
> +
> +	mutex_lock(&minor->debugfs_lock);
> +	list_for_each_entry_safe(node, tmp, &minor->debugfs_list, list) {
> +		debugfs_remove(node->dent);
> +		list_del(&node->list);
> +		kfree(node);
> +	}
> +	mutex_unlock(&minor->debugfs_lock);
> +}
> +
>  /**
>   * Cleanup the debugfs filesystem resources.
>   *
> @@ -245,9 +259,9 @@ int drm_debugfs_cleanup(struct drm_minor *minor)
>  		}
>  	}
>  
> -	drm_debugfs_remove_files(drm_debugfs_list, DRM_DEBUGFS_ENTRIES, minor);
> +	drm_debugfs_remove_all_files(minor);
>  
> -	debugfs_remove(minor->debugfs_root);
> +	debugfs_remove_recursive(minor->debugfs_root);
>  	minor->debugfs_root = NULL;
>  
>  	return 0;
> -- 
> 2.10.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
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 02/19] drm: drm_minor_register(): Clean up debugfs on failure

[Noralf Trønnes]

Call drm_debugfs_cleanup() in case drm_debugfs_init() fails to
cover for failure in the drm_driver.debugfs_init callback.

Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
---
 drivers/gpu/drm/drm_drv.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
index 1b11ab6..fb4f17b 100644
--- a/drivers/gpu/drm/drm_drv.c
+++ b/drivers/gpu/drm/drm_drv.c
@@ -221,7 +221,7 @@ static int drm_minor_register(struct drm_device *dev, unsigned int type)
 	ret = drm_debugfs_init(minor, minor->index, drm_debugfs_root);
 	if (ret) {
 		DRM_ERROR("DRM: Failed to initialize /sys/kernel/debug/dri.\n");
-		return ret;
+		goto err_debugfs;
 	}
 
 	ret = device_add(minor->kdev);
-- 
2.10.2

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 03/19] drm/atomic: Remove drm_atomic_debugfs_cleanup()

[Noralf Trønnes]

drm_debugfs_cleanup() now removes all minor->debugfs_list entries
automatically, so no need to call drm_debugfs_remove_files().

Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
---
 drivers/gpu/drm/drm_atomic.c        | 7 -------
 drivers/gpu/drm/drm_crtc_internal.h | 1 -
 drivers/gpu/drm/drm_debugfs.c       | 9 ---------
 3 files changed, 17 deletions(-)

diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
index 723392f..861e130 100644
--- a/drivers/gpu/drm/drm_atomic.c
+++ b/drivers/gpu/drm/drm_atomic.c
@@ -1731,13 +1731,6 @@ int drm_atomic_debugfs_init(struct drm_minor *minor)
 			ARRAY_SIZE(drm_atomic_debugfs_list),
 			minor->debugfs_root, minor);
 }
-
-int drm_atomic_debugfs_cleanup(struct drm_minor *minor)
-{
-	return drm_debugfs_remove_files(drm_atomic_debugfs_list,
-					ARRAY_SIZE(drm_atomic_debugfs_list),
-					minor);
-}
 #endif
 
 /*
diff --git a/drivers/gpu/drm/drm_crtc_internal.h b/drivers/gpu/drm/drm_crtc_internal.h
index 724c329..1bdcfd5 100644
--- a/drivers/gpu/drm/drm_crtc_internal.h
+++ b/drivers/gpu/drm/drm_crtc_internal.h
@@ -177,7 +177,6 @@ int drm_mode_dirtyfb_ioctl(struct drm_device *dev,
 #ifdef CONFIG_DEBUG_FS
 struct drm_minor;
 int drm_atomic_debugfs_init(struct drm_minor *minor);
-int drm_atomic_debugfs_cleanup(struct drm_minor *minor);
 #endif
 
 int drm_atomic_get_property(struct drm_mode_object *obj,
diff --git a/drivers/gpu/drm/drm_debugfs.c b/drivers/gpu/drm/drm_debugfs.c
index 04b0af3..2290a74 100644
--- a/drivers/gpu/drm/drm_debugfs.c
+++ b/drivers/gpu/drm/drm_debugfs.c
@@ -243,7 +243,6 @@ static void drm_debugfs_remove_all_files(struct drm_minor *minor)
 int drm_debugfs_cleanup(struct drm_minor *minor)
 {
 	struct drm_device *dev = minor->dev;
-	int ret;
 
 	if (!minor->debugfs_root)
 		return 0;
@@ -251,14 +250,6 @@ int drm_debugfs_cleanup(struct drm_minor *minor)
 	if (dev->driver->debugfs_cleanup)
 		dev->driver->debugfs_cleanup(minor);
 
-	if (drm_core_check_feature(dev, DRIVER_ATOMIC)) {
-		ret = drm_atomic_debugfs_cleanup(minor);
-		if (ret) {
-			DRM_ERROR("DRM: Failed to remove atomic debugfs entries\n");
-			return ret;
-		}
-	}
-
 	drm_debugfs_remove_all_files(minor);
 
 	debugfs_remove_recursive(minor->debugfs_root);
-- 
2.10.2

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 03/19] drm/atomic: Remove drm_atomic_debugfs_cleanup()

[Daniel Vetter]

On Thu, Jan 26, 2017 at 11:56:05PM +0100, Noralf Trønnes wrote:
> drm_debugfs_cleanup() now removes all minor->debugfs_list entries
> automatically, so no need to call drm_debugfs_remove_files().
> 
> Signed-off-by: Noralf Trønnes <noralf@tronnes.org>

Merged up to this one to drm-misc, I'll wait with the others for acks to
trickle in.

Thanks a lot, Daniel
> ---
>  drivers/gpu/drm/drm_atomic.c        | 7 -------
>  drivers/gpu/drm/drm_crtc_internal.h | 1 -
>  drivers/gpu/drm/drm_debugfs.c       | 9 ---------
>  3 files changed, 17 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
> index 723392f..861e130 100644
> --- a/drivers/gpu/drm/drm_atomic.c
> +++ b/drivers/gpu/drm/drm_atomic.c
> @@ -1731,13 +1731,6 @@ int drm_atomic_debugfs_init(struct drm_minor *minor)
>  			ARRAY_SIZE(drm_atomic_debugfs_list),
>  			minor->debugfs_root, minor);
>  }
> -
> -int drm_atomic_debugfs_cleanup(struct drm_minor *minor)
> -{
> -	return drm_debugfs_remove_files(drm_atomic_debugfs_list,
> -					ARRAY_SIZE(drm_atomic_debugfs_list),
> -					minor);
> -}
>  #endif
>  
>  /*
> diff --git a/drivers/gpu/drm/drm_crtc_internal.h b/drivers/gpu/drm/drm_crtc_internal.h
> index 724c329..1bdcfd5 100644
> --- a/drivers/gpu/drm/drm_crtc_internal.h
> +++ b/drivers/gpu/drm/drm_crtc_internal.h
> @@ -177,7 +177,6 @@ int drm_mode_dirtyfb_ioctl(struct drm_device *dev,
>  #ifdef CONFIG_DEBUG_FS
>  struct drm_minor;
>  int drm_atomic_debugfs_init(struct drm_minor *minor);
> -int drm_atomic_debugfs_cleanup(struct drm_minor *minor);
>  #endif
>  
>  int drm_atomic_get_property(struct drm_mode_object *obj,
> diff --git a/drivers/gpu/drm/drm_debugfs.c b/drivers/gpu/drm/drm_debugfs.c
> index 04b0af3..2290a74 100644
> --- a/drivers/gpu/drm/drm_debugfs.c
> +++ b/drivers/gpu/drm/drm_debugfs.c
> @@ -243,7 +243,6 @@ static void drm_debugfs_remove_all_files(struct drm_minor *minor)
>  int drm_debugfs_cleanup(struct drm_minor *minor)
>  {
>  	struct drm_device *dev = minor->dev;
> -	int ret;
>  
>  	if (!minor->debugfs_root)
>  		return 0;
> @@ -251,14 +250,6 @@ int drm_debugfs_cleanup(struct drm_minor *minor)
>  	if (dev->driver->debugfs_cleanup)
>  		dev->driver->debugfs_cleanup(minor);
>  
> -	if (drm_core_check_feature(dev, DRIVER_ATOMIC)) {
> -		ret = drm_atomic_debugfs_cleanup(minor);
> -		if (ret) {
> -			DRM_ERROR("DRM: Failed to remove atomic debugfs entries\n");
> -			return ret;
> -		}
> -	}
> -
>  	drm_debugfs_remove_all_files(minor);
>  
>  	debugfs_remove_recursive(minor->debugfs_root);
> -- 
> 2.10.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
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 04/19] drm/amd/amdgpu: Remove drm_debugfs_remove_files() call

[Noralf Trønnes]

drm_debugfs_cleanup() now removes all minor->debugfs_list entries
automatically, so no need to call drm_debugfs_remove_files().
Also remove empty drm_driver.debugfs_cleanup callback.

Cc: alexander.deucher@amd.com
Cc: christian.koenig@amd.com
Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu.h        |  1 -
 drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 20 --------------------
 drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c    |  1 -
 3 files changed, 22 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
index f4f371f..73863d4 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
@@ -1133,7 +1133,6 @@ int amdgpu_debugfs_fence_init(struct amdgpu_device *adev);
 
 #if defined(CONFIG_DEBUG_FS)
 int amdgpu_debugfs_init(struct drm_minor *minor);
-void amdgpu_debugfs_cleanup(struct drm_minor *minor);
 #endif
 
 int amdgpu_debugfs_firmware_init(struct amdgpu_device *adev);
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
index fe3bb94..2201303 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
@@ -1852,8 +1852,6 @@ int amdgpu_device_init(struct amdgpu_device *adev,
 	return r;
 }
 
-static void amdgpu_debugfs_remove_files(struct amdgpu_device *adev);
-
 /**
  * amdgpu_device_fini - tear down the driver
  *
@@ -1893,7 +1891,6 @@ void amdgpu_device_fini(struct amdgpu_device *adev)
 	if (adev->asic_type >= CHIP_BONAIRE)
 		amdgpu_doorbell_fini(adev);
 	amdgpu_debugfs_regs_cleanup(adev);
-	amdgpu_debugfs_remove_files(adev);
 }
 
 
@@ -2507,19 +2504,6 @@ int amdgpu_debugfs_add_files(struct amdgpu_device *adev,
 	return 0;
 }
 
-static void amdgpu_debugfs_remove_files(struct amdgpu_device *adev)
-{
-#if defined(CONFIG_DEBUG_FS)
-	unsigned i;
-
-	for (i = 0; i < adev->debugfs_count; i++) {
-		drm_debugfs_remove_files(adev->debugfs[i].files,
-					 adev->debugfs[i].num_files,
-					 adev->ddev->primary);
-	}
-#endif
-}
-
 #if defined(CONFIG_DEBUG_FS)
 
 static ssize_t amdgpu_debugfs_regs_read(struct file *f, char __user *buf,
@@ -3153,10 +3137,6 @@ int amdgpu_debugfs_init(struct drm_minor *minor)
 {
 	return 0;
 }
-
-void amdgpu_debugfs_cleanup(struct drm_minor *minor)
-{
-}
 #else
 static int amdgpu_debugfs_regs_init(struct amdgpu_device *adev)
 {
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
index 2534ada..51cfea7 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
@@ -701,7 +701,6 @@ static struct drm_driver kms_driver = {
 	.get_scanout_position = amdgpu_get_crtc_scanoutpos,
 #if defined(CONFIG_DEBUG_FS)
 	.debugfs_init = amdgpu_debugfs_init,
-	.debugfs_cleanup = amdgpu_debugfs_cleanup,
 #endif
 	.irq_preinstall = amdgpu_irq_preinstall,
 	.irq_postinstall = amdgpu_irq_postinstall,
-- 
2.10.2

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 04/19] drm/amd/amdgpu: Remove drm_debugfs_remove_files() call

[Christian König]

Am 26.01.2017 um 23:56 schrieb Noralf Trønnes:
> drm_debugfs_cleanup() now removes all minor->debugfs_list entries
> automatically, so no need to call drm_debugfs_remove_files().
> Also remove empty drm_driver.debugfs_cleanup callback.
>
> Cc: alexander.deucher@amd.com
> Cc: christian.koenig@amd.com
> Signed-off-by: Noralf Trønnes <noralf@tronnes.org>

Everything said for Radeon applies here as well.

Just let me know if you want to do the additional cleanup patches or if 
we should be taking care of that.

This patch is Reviewed-by: Christian König <christian.koenig@amd.com> 
for now.

Regards,
Christian.

> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu.h        |  1 -
>   drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 20 --------------------
>   drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c    |  1 -
>   3 files changed, 22 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> index f4f371f..73863d4 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> @@ -1133,7 +1133,6 @@ int amdgpu_debugfs_fence_init(struct amdgpu_device *adev);
>   
>   #if defined(CONFIG_DEBUG_FS)
>   int amdgpu_debugfs_init(struct drm_minor *minor);
> -void amdgpu_debugfs_cleanup(struct drm_minor *minor);
>   #endif
>   
>   int amdgpu_debugfs_firmware_init(struct amdgpu_device *adev);
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> index fe3bb94..2201303 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> @@ -1852,8 +1852,6 @@ int amdgpu_device_init(struct amdgpu_device *adev,
>   	return r;
>   }
>   
> -static void amdgpu_debugfs_remove_files(struct amdgpu_device *adev);
> -
>   /**
>    * amdgpu_device_fini - tear down the driver
>    *
> @@ -1893,7 +1891,6 @@ void amdgpu_device_fini(struct amdgpu_device *adev)
>   	if (adev->asic_type >= CHIP_BONAIRE)
>   		amdgpu_doorbell_fini(adev);
>   	amdgpu_debugfs_regs_cleanup(adev);
> -	amdgpu_debugfs_remove_files(adev);
>   }
>   
>   
> @@ -2507,19 +2504,6 @@ int amdgpu_debugfs_add_files(struct amdgpu_device *adev,
>   	return 0;
>   }
>   
> -static void amdgpu_debugfs_remove_files(struct amdgpu_device *adev)
> -{
> -#if defined(CONFIG_DEBUG_FS)
> -	unsigned i;
> -
> -	for (i = 0; i < adev->debugfs_count; i++) {
> -		drm_debugfs_remove_files(adev->debugfs[i].files,
> -					 adev->debugfs[i].num_files,
> -					 adev->ddev->primary);
> -	}
> -#endif
> -}
> -
>   #if defined(CONFIG_DEBUG_FS)
>   
>   static ssize_t amdgpu_debugfs_regs_read(struct file *f, char __user *buf,
> @@ -3153,10 +3137,6 @@ int amdgpu_debugfs_init(struct drm_minor *minor)
>   {
>   	return 0;
>   }
> -
> -void amdgpu_debugfs_cleanup(struct drm_minor *minor)
> -{
> -}
>   #else
>   static int amdgpu_debugfs_regs_init(struct amdgpu_device *adev)
>   {
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> index 2534ada..51cfea7 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
> @@ -701,7 +701,6 @@ static struct drm_driver kms_driver = {
>   	.get_scanout_position = amdgpu_get_crtc_scanoutpos,
>   #if defined(CONFIG_DEBUG_FS)
>   	.debugfs_init = amdgpu_debugfs_init,
> -	.debugfs_cleanup = amdgpu_debugfs_cleanup,
>   #endif
>   	.irq_preinstall = amdgpu_irq_preinstall,
>   	.irq_postinstall = amdgpu_irq_postinstall,


_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 05/19] drm/armada: Remove armada_drm_debugfs_cleanup()

[Noralf Trønnes]

drm_debugfs_cleanup() now removes all minor->debugfs_list entries
automatically, so no need to do this explicitly. Additionally it
uses debugfs_remove_recursive() to clean up the debugfs files,
so no need for adding fake drm_info_node entries.
And finally there's no need to clean up on error,
drm_debugfs_cleanup() is called in the error path.

Cc: linux@armlinux.org.uk
Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
---
 drivers/gpu/drm/armada/armada_debugfs.c | 65 +++++----------------------------
 drivers/gpu/drm/armada/armada_drm.h     |  1 -
 drivers/gpu/drm/armada/armada_drv.c     |  3 --
 3 files changed, 10 insertions(+), 59 deletions(-)

diff --git a/drivers/gpu/drm/armada/armada_debugfs.c b/drivers/gpu/drm/armada/armada_debugfs.c
index a8020cf..6758c3a 100644
--- a/drivers/gpu/drm/armada/armada_debugfs.c
+++ b/drivers/gpu/drm/armada/armada_debugfs.c
@@ -107,40 +107,9 @@ static struct drm_info_list armada_debugfs_list[] = {
 };
 #define ARMADA_DEBUGFS_ENTRIES ARRAY_SIZE(armada_debugfs_list)
 
-static int drm_add_fake_info_node(struct drm_minor *minor, struct dentry *ent,
-	const void *key)
-{
-	struct drm_info_node *node;
-
-	node = kmalloc(sizeof(struct drm_info_node), GFP_KERNEL);
-	if (!node) {
-		debugfs_remove(ent);
-		return -ENOMEM;
-	}
-
-	node->minor = minor;
-	node->dent = ent;
-	node->info_ent = (void *) key;
-
-	mutex_lock(&minor->debugfs_lock);
-	list_add(&node->list, &minor->debugfs_list);
-	mutex_unlock(&minor->debugfs_lock);
-
-	return 0;
-}
-
-static int armada_debugfs_create(struct dentry *root, struct drm_minor *minor,
-	const char *name, umode_t mode, const struct file_operations *fops)
-{
-	struct dentry *de;
-
-	de = debugfs_create_file(name, mode, root, minor->dev, fops);
-
-	return drm_add_fake_info_node(minor, de, fops);
-}
-
 int armada_drm_debugfs_init(struct drm_minor *minor)
 {
+	struct dentry *de;
 	int ret;
 
 	ret = drm_debugfs_create_files(armada_debugfs_list,
@@ -149,29 +118,15 @@ int armada_drm_debugfs_init(struct drm_minor *minor)
 	if (ret)
 		return ret;
 
-	ret = armada_debugfs_create(minor->debugfs_root, minor,
-				   "reg", S_IFREG | S_IRUSR, &fops_reg_r);
-	if (ret)
-		goto err_1;
+	de = debugfs_create_file("reg", S_IFREG | S_IRUSR,
+				 minor->debugfs_root, minor->dev, &fops_reg_r);
+	if (!de)
+		return -ENOMEM;
 
-	ret = armada_debugfs_create(minor->debugfs_root, minor,
-				"reg_wr", S_IFREG | S_IWUSR, &fops_reg_w);
-	if (ret)
-		goto err_2;
-	return ret;
-
- err_2:
-	drm_debugfs_remove_files((struct drm_info_list *)&fops_reg_r, 1, minor);
- err_1:
-	drm_debugfs_remove_files(armada_debugfs_list, ARMADA_DEBUGFS_ENTRIES,
-				 minor);
-	return ret;
-}
+	de = debugfs_create_file("reg_wr", S_IFREG | S_IWUSR,
+				 minor->debugfs_root, minor->dev, &fops_reg_w);
+	if (!de)
+		return -ENOMEM;
 
-void armada_drm_debugfs_cleanup(struct drm_minor *minor)
-{
-	drm_debugfs_remove_files((struct drm_info_list *)&fops_reg_w, 1, minor);
-	drm_debugfs_remove_files((struct drm_info_list *)&fops_reg_r, 1, minor);
-	drm_debugfs_remove_files(armada_debugfs_list, ARMADA_DEBUGFS_ENTRIES,
-				 minor);
+	return 0;
 }
diff --git a/drivers/gpu/drm/armada/armada_drm.h b/drivers/gpu/drm/armada/armada_drm.h
index 77952d5..b064879 100644
--- a/drivers/gpu/drm/armada/armada_drm.h
+++ b/drivers/gpu/drm/armada/armada_drm.h
@@ -90,6 +90,5 @@ void armada_fbdev_fini(struct drm_device *);
 int armada_overlay_plane_create(struct drm_device *, unsigned long);
 
 int armada_drm_debugfs_init(struct drm_minor *);
-void armada_drm_debugfs_cleanup(struct drm_minor *);
 
 #endif
diff --git a/drivers/gpu/drm/armada/armada_drv.c b/drivers/gpu/drm/armada/armada_drv.c
index 63f42d0..51b4eae 100644
--- a/drivers/gpu/drm/armada/armada_drv.c
+++ b/drivers/gpu/drm/armada/armada_drv.c
@@ -226,9 +226,6 @@ static void armada_drm_unbind(struct device *dev)
 	drm_kms_helper_poll_fini(&priv->drm);
 	armada_fbdev_fini(&priv->drm);
 
-#ifdef CONFIG_DEBUG_FS
-	armada_drm_debugfs_cleanup(priv->drm.primary);
-#endif
 	drm_dev_unregister(&priv->drm);
 
 	component_unbind_all(dev, &priv->drm);
-- 
2.10.2

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 06/19] drm/etnaviv: allow build with COMPILE_TEST

[Noralf Trønnes]

Make it possible to compile test the driver on other platforms.

Cc: l.stach@pengutronix.de
Cc: linux+etnaviv@armlinux.org.uk
Cc: christian.gmeiner@gmail.com
Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
---
 drivers/gpu/drm/etnaviv/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/etnaviv/Kconfig b/drivers/gpu/drm/etnaviv/Kconfig
index 656c061..cc1731c 100644
--- a/drivers/gpu/drm/etnaviv/Kconfig
+++ b/drivers/gpu/drm/etnaviv/Kconfig
@@ -2,7 +2,7 @@
 config DRM_ETNAVIV
 	tristate "ETNAVIV (DRM support for Vivante GPU IP cores)"
 	depends on DRM
-	depends on ARCH_MXC || ARCH_DOVE
+	depends on ARCH_MXC || ARCH_DOVE || (ARM && COMPILE_TEST)
 	depends on MMU
 	select SHMEM
 	select TMPFS
-- 
2.10.2

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 06/19] drm/etnaviv: allow build with COMPILE_TEST

[Lucas Stach]

Am Donnerstag, den 26.01.2017, 23:56 +0100 schrieb Noralf Trønnes:
> Make it possible to compile test the driver on other platforms.
> 
> Cc: l.stach@pengutronix.de
> Cc: linux+etnaviv@armlinux.org.uk
> Cc: christian.gmeiner@gmail.com
> Signed-off-by: Noralf Trønnes <noralf@tronnes.org>

I'm not sure we are pulling in all dependencies on other platforms, but
as I have no good way to check right now, lets go with the patch and see
if anything blows up:

Acked-by: Lucas Stach <l.stach@pengutronix.de>

> ---
>  drivers/gpu/drm/etnaviv/Kconfig | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/etnaviv/Kconfig b/drivers/gpu/drm/etnaviv/Kconfig
> index 656c061..cc1731c 100644
> --- a/drivers/gpu/drm/etnaviv/Kconfig
> +++ b/drivers/gpu/drm/etnaviv/Kconfig
> @@ -2,7 +2,7 @@
>  config DRM_ETNAVIV
>  	tristate "ETNAVIV (DRM support for Vivante GPU IP cores)"
>  	depends on DRM
> -	depends on ARCH_MXC || ARCH_DOVE
> +	depends on ARCH_MXC || ARCH_DOVE || (ARM && COMPILE_TEST)
>  	depends on MMU
>  	select SHMEM
>  	select TMPFS


_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 06/19] drm/etnaviv: allow build with COMPILE_TEST

[Daniel Vetter]

On Fri, Jan 27, 2017 at 10:57:30AM +0100, Lucas Stach wrote:
> Am Donnerstag, den 26.01.2017, 23:56 +0100 schrieb Noralf Trønnes:
> > Make it possible to compile test the driver on other platforms.
> > 
> > Cc: l.stach@pengutronix.de
> > Cc: linux+etnaviv@armlinux.org.uk
> > Cc: christian.gmeiner@gmail.com
> > Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
> 
> I'm not sure we are pulling in all dependencies on other platforms, but
> as I have no good way to check right now, lets go with the patch and see
> if anything blows up:

There's no other way to validate KConfig patches than to apply them and
let 0day beat for a few weeks through all the combinations once it's in
linux-next. 0day doesn't do randomized configs on stuff that's not in
linux-next unfortunately (probably too much of a cpu waste).

> Acked-by: Lucas Stach <l.stach@pengutronix.de>

Thanks, applied.
-Daniel

> 
> > ---
> >  drivers/gpu/drm/etnaviv/Kconfig | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/drivers/gpu/drm/etnaviv/Kconfig b/drivers/gpu/drm/etnaviv/Kconfig
> > index 656c061..cc1731c 100644
> > --- a/drivers/gpu/drm/etnaviv/Kconfig
> > +++ b/drivers/gpu/drm/etnaviv/Kconfig
> > @@ -2,7 +2,7 @@
> >  config DRM_ETNAVIV
> >  	tristate "ETNAVIV (DRM support for Vivante GPU IP cores)"
> >  	depends on DRM
> > -	depends on ARCH_MXC || ARCH_DOVE
> > +	depends on ARCH_MXC || ARCH_DOVE || (ARM && COMPILE_TEST)
> >  	depends on MMU
> >  	select SHMEM
> >  	select TMPFS
> 
> 
> _______________________________________________
> 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
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 07/19] drm/etnaviv: Remove etnaviv_debugfs_cleanup()

[Noralf Trønnes]

drm_debugfs_cleanup() now removes all minor->debugfs_list entries
automatically, so the drm_driver.debugfs_cleanup callback is not
needed.

Cc: l.stach@pengutronix.de
Cc: linux+etnaviv@armlinux.org.uk
Cc: christian.gmeiner@gmail.com
Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
---
 drivers/gpu/drm/etnaviv/etnaviv_drv.c | 7 -------
 1 file changed, 7 deletions(-)

diff --git a/drivers/gpu/drm/etnaviv/etnaviv_drv.c b/drivers/gpu/drm/etnaviv/etnaviv_drv.c
index b92c24e..590be0d 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_drv.c
+++ b/drivers/gpu/drm/etnaviv/etnaviv_drv.c
@@ -258,12 +258,6 @@ static int etnaviv_debugfs_init(struct drm_minor *minor)
 
 	return ret;
 }
-
-static void etnaviv_debugfs_cleanup(struct drm_minor *minor)
-{
-	drm_debugfs_remove_files(etnaviv_debugfs_list,
-			ARRAY_SIZE(etnaviv_debugfs_list), minor);
-}
 #endif
 
 /*
@@ -509,7 +503,6 @@ static struct drm_driver etnaviv_drm_driver = {
 	.gem_prime_mmap     = etnaviv_gem_prime_mmap,
 #ifdef CONFIG_DEBUG_FS
 	.debugfs_init       = etnaviv_debugfs_init,
-	.debugfs_cleanup    = etnaviv_debugfs_cleanup,
 #endif
 	.ioctls             = etnaviv_ioctls,
 	.num_ioctls         = DRM_ETNAVIV_NUM_IOCTLS,
-- 
2.10.2

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 07/19] drm/etnaviv: Remove etnaviv_debugfs_cleanup()

[Lucas Stach]

Am Donnerstag, den 26.01.2017, 23:56 +0100 schrieb Noralf Trønnes:
> drm_debugfs_cleanup() now removes all minor->debugfs_list entries
> automatically, so the drm_driver.debugfs_cleanup callback is not
> needed.
> 
> Cc: l.stach@pengutronix.de
> Cc: linux+etnaviv@armlinux.org.uk
> Cc: christian.gmeiner@gmail.com
> Signed-off-by: Noralf Trønnes <noralf@tronnes.org>

Reviewed-by: Lucas Stach <l.stach@pengutronix.de>

> ---
>  drivers/gpu/drm/etnaviv/etnaviv_drv.c | 7 -------
>  1 file changed, 7 deletions(-)
> 
> diff --git a/drivers/gpu/drm/etnaviv/etnaviv_drv.c b/drivers/gpu/drm/etnaviv/etnaviv_drv.c
> index b92c24e..590be0d 100644
> --- a/drivers/gpu/drm/etnaviv/etnaviv_drv.c
> +++ b/drivers/gpu/drm/etnaviv/etnaviv_drv.c
> @@ -258,12 +258,6 @@ static int etnaviv_debugfs_init(struct drm_minor *minor)
>  
>  	return ret;
>  }
> -
> -static void etnaviv_debugfs_cleanup(struct drm_minor *minor)
> -{
> -	drm_debugfs_remove_files(etnaviv_debugfs_list,
> -			ARRAY_SIZE(etnaviv_debugfs_list), minor);
> -}
>  #endif
>  
>  /*
> @@ -509,7 +503,6 @@ static struct drm_driver etnaviv_drm_driver = {
>  	.gem_prime_mmap     = etnaviv_gem_prime_mmap,
>  #ifdef CONFIG_DEBUG_FS
>  	.debugfs_init       = etnaviv_debugfs_init,
> -	.debugfs_cleanup    = etnaviv_debugfs_cleanup,
>  #endif
>  	.ioctls             = etnaviv_ioctls,
>  	.num_ioctls         = DRM_ETNAVIV_NUM_IOCTLS,


_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 08/19] drm/hdlcd: Remove hdlcd_debugfs_cleanup()

[Noralf Trønnes]

drm_debugfs_cleanup() now removes all minor->debugfs_list entries
automatically, so the drm_driver.debugfs_cleanup callback is not
needed.

Cc: liviu.dudau@arm.com
Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
---
 drivers/gpu/drm/arm/hdlcd_drv.c | 7 -------
 1 file changed, 7 deletions(-)

diff --git a/drivers/gpu/drm/arm/hdlcd_drv.c b/drivers/gpu/drm/arm/hdlcd_drv.c
index e5f4f4a..a2e5b04 100644
--- a/drivers/gpu/drm/arm/hdlcd_drv.c
+++ b/drivers/gpu/drm/arm/hdlcd_drv.c
@@ -255,12 +255,6 @@ static int hdlcd_debugfs_init(struct drm_minor *minor)
 	return drm_debugfs_create_files(hdlcd_debugfs_list,
 		ARRAY_SIZE(hdlcd_debugfs_list),	minor->debugfs_root, minor);
 }
-
-static void hdlcd_debugfs_cleanup(struct drm_minor *minor)
-{
-	drm_debugfs_remove_files(hdlcd_debugfs_list,
-		ARRAY_SIZE(hdlcd_debugfs_list), minor);
-}
 #endif
 
 static const struct file_operations fops = {
@@ -303,7 +297,6 @@ static struct drm_driver hdlcd_driver = {
 	.gem_prime_mmap = drm_gem_cma_prime_mmap,
 #ifdef CONFIG_DEBUG_FS
 	.debugfs_init = hdlcd_debugfs_init,
-	.debugfs_cleanup = hdlcd_debugfs_cleanup,
 #endif
 	.fops = &fops,
 	.name = "hdlcd",
-- 
2.10.2

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 08/19] drm/hdlcd: Remove hdlcd_debugfs_cleanup()

[Local user for Liviu Dudau]

On Thu, Jan 26, 2017 at 11:56:10PM +0100, Noralf Trønnes wrote:
> drm_debugfs_cleanup() now removes all minor->debugfs_list entries
> automatically, so the drm_driver.debugfs_cleanup callback is not
> needed.
> 
> Cc: liviu.dudau@arm.com
> Signed-off-by: Noralf Trønnes <noralf@tronnes.org>

Acked-by: Liviu Dudau <liviu.dudau@arm.com>

Thanks!
Liviu

> ---
>  drivers/gpu/drm/arm/hdlcd_drv.c | 7 -------
>  1 file changed, 7 deletions(-)
> 
> diff --git a/drivers/gpu/drm/arm/hdlcd_drv.c b/drivers/gpu/drm/arm/hdlcd_drv.c
> index e5f4f4a..a2e5b04 100644
> --- a/drivers/gpu/drm/arm/hdlcd_drv.c
> +++ b/drivers/gpu/drm/arm/hdlcd_drv.c
> @@ -255,12 +255,6 @@ static int hdlcd_debugfs_init(struct drm_minor *minor)
>  	return drm_debugfs_create_files(hdlcd_debugfs_list,
>  		ARRAY_SIZE(hdlcd_debugfs_list),	minor->debugfs_root, minor);
>  }
> -
> -static void hdlcd_debugfs_cleanup(struct drm_minor *minor)
> -{
> -	drm_debugfs_remove_files(hdlcd_debugfs_list,
> -		ARRAY_SIZE(hdlcd_debugfs_list), minor);
> -}
>  #endif
>  
>  static const struct file_operations fops = {
> @@ -303,7 +297,6 @@ static struct drm_driver hdlcd_driver = {
>  	.gem_prime_mmap = drm_gem_cma_prime_mmap,
>  #ifdef CONFIG_DEBUG_FS
>  	.debugfs_init = hdlcd_debugfs_init,
> -	.debugfs_cleanup = hdlcd_debugfs_cleanup,
>  #endif
>  	.fops = &fops,
>  	.name = "hdlcd",
> -- 
> 2.10.2
> 
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 09/19] drm/msm: Remove drm_debugfs_remove_files() calls

[Noralf Trønnes]

drm_debugfs_cleanup() now removes all minor->debugfs_list entries
automatically, so it's not necessary to call
drm_debugfs_remove_files(). Additionally it uses
debugfs_remove_recursive() to clean up the debugfs files, so no need
to do that.

Cc: robdclark@gmail.com
Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
---
 drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.c |  7 -------
 drivers/gpu/drm/msm/msm_debugfs.c       |  2 --
 drivers/gpu/drm/msm/msm_perf.c          | 29 +++--------------------------
 drivers/gpu/drm/msm/msm_rd.c            | 31 +++----------------------------
 4 files changed, 6 insertions(+), 63 deletions(-)

diff --git a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.c b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.c
index 5f6cd87..2f21a18 100644
--- a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.c
+++ b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.c
@@ -212,12 +212,6 @@ static int mdp5_kms_debugfs_init(struct msm_kms *kms, struct drm_minor *minor)
 
 	return 0;
 }
-
-static void mdp5_kms_debugfs_cleanup(struct msm_kms *kms, struct drm_minor *minor)
-{
-	drm_debugfs_remove_files(mdp5_debugfs_list,
-			ARRAY_SIZE(mdp5_debugfs_list), minor);
-}
 #endif
 
 static const struct mdp_kms_funcs kms_funcs = {
@@ -239,7 +233,6 @@ static const struct mdp_kms_funcs kms_funcs = {
 		.destroy         = mdp5_kms_destroy,
 #ifdef CONFIG_DEBUG_FS
 		.debugfs_init    = mdp5_kms_debugfs_init,
-		.debugfs_cleanup = mdp5_kms_debugfs_cleanup,
 #endif
 	},
 	.set_irqmask         = mdp5_set_irqmask,
diff --git a/drivers/gpu/drm/msm/msm_debugfs.c b/drivers/gpu/drm/msm/msm_debugfs.c
index 387f0616..75609a1 100644
--- a/drivers/gpu/drm/msm/msm_debugfs.c
+++ b/drivers/gpu/drm/msm/msm_debugfs.c
@@ -170,8 +170,6 @@ void msm_debugfs_cleanup(struct drm_minor *minor)
 	struct drm_device *dev = minor->dev;
 	struct msm_drm_private *priv = dev->dev_private;
 
-	drm_debugfs_remove_files(msm_debugfs_list,
-			ARRAY_SIZE(msm_debugfs_list), minor);
 	if (!priv)
 		return;
 
diff --git a/drivers/gpu/drm/msm/msm_perf.c b/drivers/gpu/drm/msm/msm_perf.c
index 1627294..fc5a948 100644
--- a/drivers/gpu/drm/msm/msm_perf.c
+++ b/drivers/gpu/drm/msm/msm_perf.c
@@ -41,9 +41,6 @@ struct msm_perf_state {
 	int buftot, bufpos;
 
 	unsigned long next_jiffies;
-
-	struct dentry *ent;
-	struct drm_info_node *node;
 };
 
 #define SAMPLE_TIME (HZ/4)
@@ -208,6 +205,7 @@ int msm_perf_debugfs_init(struct drm_minor *minor)
 {
 	struct msm_drm_private *priv = minor->dev->dev_private;
 	struct msm_perf_state *perf;
+	struct dentry *ent;
 
 	/* only create on first minor: */
 	if (priv->perf)
@@ -222,26 +220,14 @@ int msm_perf_debugfs_init(struct drm_minor *minor)
 	mutex_init(&perf->read_lock);
 	priv->perf = perf;
 
-	perf->node = kzalloc(sizeof(*perf->node), GFP_KERNEL);
-	if (!perf->node)
-		goto fail;
-
-	perf->ent = debugfs_create_file("perf", S_IFREG | S_IRUGO,
+	ent = debugfs_create_file("perf", S_IFREG | S_IRUGO,
 			minor->debugfs_root, perf, &perf_debugfs_fops);
-	if (!perf->ent) {
+	if (!ent) {
 		DRM_ERROR("Cannot create /sys/kernel/debug/dri/%pd/perf\n",
 				minor->debugfs_root);
 		goto fail;
 	}
 
-	perf->node->minor = minor;
-	perf->node->dent  = perf->ent;
-	perf->node->info_ent = NULL;
-
-	mutex_lock(&minor->debugfs_lock);
-	list_add(&perf->node->list, &minor->debugfs_list);
-	mutex_unlock(&minor->debugfs_lock);
-
 	return 0;
 
 fail:
@@ -259,15 +245,6 @@ void msm_perf_debugfs_cleanup(struct drm_minor *minor)
 
 	priv->perf = NULL;
 
-	debugfs_remove(perf->ent);
-
-	if (perf->node) {
-		mutex_lock(&minor->debugfs_lock);
-		list_del(&perf->node->list);
-		mutex_unlock(&minor->debugfs_lock);
-		kfree(perf->node);
-	}
-
 	mutex_destroy(&perf->read_lock);
 
 	kfree(perf);
diff --git a/drivers/gpu/drm/msm/msm_rd.c b/drivers/gpu/drm/msm/msm_rd.c
index 6607456..ab0b39f 100644
--- a/drivers/gpu/drm/msm/msm_rd.c
+++ b/drivers/gpu/drm/msm/msm_rd.c
@@ -84,9 +84,6 @@ struct msm_rd_state {
 
 	bool open;
 
-	struct dentry *ent;
-	struct drm_info_node *node;
-
 	/* current submit to read out: */
 	struct msm_gem_submit *submit;
 
@@ -219,6 +216,7 @@ int msm_rd_debugfs_init(struct drm_minor *minor)
 {
 	struct msm_drm_private *priv = minor->dev->dev_private;
 	struct msm_rd_state *rd;
+	struct dentry *ent;
 
 	/* only create on first minor: */
 	if (priv->rd)
@@ -236,26 +234,14 @@ int msm_rd_debugfs_init(struct drm_minor *minor)
 
 	init_waitqueue_head(&rd->fifo_event);
 
-	rd->node = kzalloc(sizeof(*rd->node), GFP_KERNEL);
-	if (!rd->node)
-		goto fail;
-
-	rd->ent = debugfs_create_file("rd", S_IFREG | S_IRUGO,
+	ent = debugfs_create_file("rd", S_IFREG | S_IRUGO,
 			minor->debugfs_root, rd, &rd_debugfs_fops);
-	if (!rd->ent) {
+	if (!ent) {
 		DRM_ERROR("Cannot create /sys/kernel/debug/dri/%pd/rd\n",
 				minor->debugfs_root);
 		goto fail;
 	}
 
-	rd->node->minor = minor;
-	rd->node->dent  = rd->ent;
-	rd->node->info_ent = NULL;
-
-	mutex_lock(&minor->debugfs_lock);
-	list_add(&rd->node->list, &minor->debugfs_list);
-	mutex_unlock(&minor->debugfs_lock);
-
 	return 0;
 
 fail:
@@ -272,18 +258,7 @@ void msm_rd_debugfs_cleanup(struct drm_minor *minor)
 		return;
 
 	priv->rd = NULL;
-
-	debugfs_remove(rd->ent);
-
-	if (rd->node) {
-		mutex_lock(&minor->debugfs_lock);
-		list_del(&rd->node->list);
-		mutex_unlock(&minor->debugfs_lock);
-		kfree(rd->node);
-	}
-
 	mutex_destroy(&rd->read_lock);
-
 	kfree(rd);
 }
 
-- 
2.10.2

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 09/19] drm/msm: Remove drm_debugfs_remove_files() calls

[Daniel Vetter]

On Thu, Jan 26, 2017 at 11:56:11PM +0100, Noralf Trønnes wrote:
> drm_debugfs_cleanup() now removes all minor->debugfs_list entries
> automatically, so it's not necessary to call
> drm_debugfs_remove_files(). Additionally it uses
> debugfs_remove_recursive() to clean up the debugfs files, so no need
> to do that.
> 
> Cc: robdclark@gmail.com
> Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
> ---
>  drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.c |  7 -------
>  drivers/gpu/drm/msm/msm_debugfs.c       |  2 --
>  drivers/gpu/drm/msm/msm_perf.c          | 29 +++--------------------------
>  drivers/gpu/drm/msm/msm_rd.c            | 31 +++----------------------------
>  4 files changed, 6 insertions(+), 63 deletions(-)
> 
> diff --git a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.c b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.c
> index 5f6cd87..2f21a18 100644
> --- a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.c
> +++ b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.c
> @@ -212,12 +212,6 @@ static int mdp5_kms_debugfs_init(struct msm_kms *kms, struct drm_minor *minor)
>  
>  	return 0;
>  }
> -
> -static void mdp5_kms_debugfs_cleanup(struct msm_kms *kms, struct drm_minor *minor)
> -{
> -	drm_debugfs_remove_files(mdp5_debugfs_list,
> -			ARRAY_SIZE(mdp5_debugfs_list), minor);
> -}
>  #endif
>  
>  static const struct mdp_kms_funcs kms_funcs = {
> @@ -239,7 +233,6 @@ static const struct mdp_kms_funcs kms_funcs = {
>  		.destroy         = mdp5_kms_destroy,
>  #ifdef CONFIG_DEBUG_FS
>  		.debugfs_init    = mdp5_kms_debugfs_init,
> -		.debugfs_cleanup = mdp5_kms_debugfs_cleanup,

Afaics the mdp_kms_funcs->debugfs_cleanup function is now entirely unused,
you can remove it (plus its only caller in msm_debugfs_cleanup) too.
Otherwise lgtm.
-Daniel

>  #endif
>  	},
>  	.set_irqmask         = mdp5_set_irqmask,
> diff --git a/drivers/gpu/drm/msm/msm_debugfs.c b/drivers/gpu/drm/msm/msm_debugfs.c
> index 387f0616..75609a1 100644
> --- a/drivers/gpu/drm/msm/msm_debugfs.c
> +++ b/drivers/gpu/drm/msm/msm_debugfs.c
> @@ -170,8 +170,6 @@ void msm_debugfs_cleanup(struct drm_minor *minor)
>  	struct drm_device *dev = minor->dev;
>  	struct msm_drm_private *priv = dev->dev_private;
>  
> -	drm_debugfs_remove_files(msm_debugfs_list,
> -			ARRAY_SIZE(msm_debugfs_list), minor);
>  	if (!priv)
>  		return;
>  
> diff --git a/drivers/gpu/drm/msm/msm_perf.c b/drivers/gpu/drm/msm/msm_perf.c
> index 1627294..fc5a948 100644
> --- a/drivers/gpu/drm/msm/msm_perf.c
> +++ b/drivers/gpu/drm/msm/msm_perf.c
> @@ -41,9 +41,6 @@ struct msm_perf_state {
>  	int buftot, bufpos;
>  
>  	unsigned long next_jiffies;
> -
> -	struct dentry *ent;
> -	struct drm_info_node *node;
>  };
>  
>  #define SAMPLE_TIME (HZ/4)
> @@ -208,6 +205,7 @@ int msm_perf_debugfs_init(struct drm_minor *minor)
>  {
>  	struct msm_drm_private *priv = minor->dev->dev_private;
>  	struct msm_perf_state *perf;
> +	struct dentry *ent;
>  
>  	/* only create on first minor: */
>  	if (priv->perf)
> @@ -222,26 +220,14 @@ int msm_perf_debugfs_init(struct drm_minor *minor)
>  	mutex_init(&perf->read_lock);
>  	priv->perf = perf;
>  
> -	perf->node = kzalloc(sizeof(*perf->node), GFP_KERNEL);
> -	if (!perf->node)
> -		goto fail;
> -
> -	perf->ent = debugfs_create_file("perf", S_IFREG | S_IRUGO,
> +	ent = debugfs_create_file("perf", S_IFREG | S_IRUGO,
>  			minor->debugfs_root, perf, &perf_debugfs_fops);
> -	if (!perf->ent) {
> +	if (!ent) {
>  		DRM_ERROR("Cannot create /sys/kernel/debug/dri/%pd/perf\n",
>  				minor->debugfs_root);
>  		goto fail;
>  	}
>  
> -	perf->node->minor = minor;
> -	perf->node->dent  = perf->ent;
> -	perf->node->info_ent = NULL;
> -
> -	mutex_lock(&minor->debugfs_lock);
> -	list_add(&perf->node->list, &minor->debugfs_list);
> -	mutex_unlock(&minor->debugfs_lock);
> -
>  	return 0;
>  
>  fail:
> @@ -259,15 +245,6 @@ void msm_perf_debugfs_cleanup(struct drm_minor *minor)
>  
>  	priv->perf = NULL;
>  
> -	debugfs_remove(perf->ent);
> -
> -	if (perf->node) {
> -		mutex_lock(&minor->debugfs_lock);
> -		list_del(&perf->node->list);
> -		mutex_unlock(&minor->debugfs_lock);
> -		kfree(perf->node);
> -	}
> -
>  	mutex_destroy(&perf->read_lock);
>  
>  	kfree(perf);
> diff --git a/drivers/gpu/drm/msm/msm_rd.c b/drivers/gpu/drm/msm/msm_rd.c
> index 6607456..ab0b39f 100644
> --- a/drivers/gpu/drm/msm/msm_rd.c
> +++ b/drivers/gpu/drm/msm/msm_rd.c
> @@ -84,9 +84,6 @@ struct msm_rd_state {
>  
>  	bool open;
>  
> -	struct dentry *ent;
> -	struct drm_info_node *node;
> -
>  	/* current submit to read out: */
>  	struct msm_gem_submit *submit;
>  
> @@ -219,6 +216,7 @@ int msm_rd_debugfs_init(struct drm_minor *minor)
>  {
>  	struct msm_drm_private *priv = minor->dev->dev_private;
>  	struct msm_rd_state *rd;
> +	struct dentry *ent;
>  
>  	/* only create on first minor: */
>  	if (priv->rd)
> @@ -236,26 +234,14 @@ int msm_rd_debugfs_init(struct drm_minor *minor)
>  
>  	init_waitqueue_head(&rd->fifo_event);
>  
> -	rd->node = kzalloc(sizeof(*rd->node), GFP_KERNEL);
> -	if (!rd->node)
> -		goto fail;
> -
> -	rd->ent = debugfs_create_file("rd", S_IFREG | S_IRUGO,
> +	ent = debugfs_create_file("rd", S_IFREG | S_IRUGO,
>  			minor->debugfs_root, rd, &rd_debugfs_fops);
> -	if (!rd->ent) {
> +	if (!ent) {
>  		DRM_ERROR("Cannot create /sys/kernel/debug/dri/%pd/rd\n",
>  				minor->debugfs_root);
>  		goto fail;
>  	}
>  
> -	rd->node->minor = minor;
> -	rd->node->dent  = rd->ent;
> -	rd->node->info_ent = NULL;
> -
> -	mutex_lock(&minor->debugfs_lock);
> -	list_add(&rd->node->list, &minor->debugfs_list);
> -	mutex_unlock(&minor->debugfs_lock);
> -
>  	return 0;
>  
>  fail:
> @@ -272,18 +258,7 @@ void msm_rd_debugfs_cleanup(struct drm_minor *minor)
>  		return;
>  
>  	priv->rd = NULL;
> -
> -	debugfs_remove(rd->ent);
> -
> -	if (rd->node) {
> -		mutex_lock(&minor->debugfs_lock);
> -		list_del(&rd->node->list);
> -		mutex_unlock(&minor->debugfs_lock);
> -		kfree(rd->node);
> -	}
> -
>  	mutex_destroy(&rd->read_lock);
> -
>  	kfree(rd);
>  }
>  
> -- 
> 2.10.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
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 10/19] drm/nouveau: Remove nouveau_drm_debugfs_cleanup()

[Noralf Trønnes]

drm_debugfs_cleanup() now removes all minor->debugfs_list entries
automatically, so the drm_driver.debugfs_cleanup callback is not
needed. Additionally it uses debugfs_remove_recursive() to clean
up the debugfs files, so no need for adding fake drm_info_node
entries.

Cc: bskeggs@redhat.com
Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
---
 drivers/gpu/drm/nouveau/nouveau_debugfs.c | 62 ++++++-------------------------
 drivers/gpu/drm/nouveau/nouveau_debugfs.h |  6 ---
 drivers/gpu/drm/nouveau/nouveau_drm.c     |  1 -
 3 files changed, 12 insertions(+), 57 deletions(-)

diff --git a/drivers/gpu/drm/nouveau/nouveau_debugfs.c b/drivers/gpu/drm/nouveau/nouveau_debugfs.c
index 411c12c..ece1987 100644
--- a/drivers/gpu/drm/nouveau/nouveau_debugfs.c
+++ b/drivers/gpu/drm/nouveau/nouveau_debugfs.c
@@ -49,8 +49,8 @@ nouveau_debugfs_vbios_image(struct seq_file *m, void *data)
 static int
 nouveau_debugfs_pstate_get(struct seq_file *m, void *data)
 {
-	struct drm_info_node *node = (struct drm_info_node *) m->private;
-	struct nouveau_debugfs *debugfs = nouveau_debugfs(node->minor->dev);
+	struct drm_device *drm = m->private;
+	struct nouveau_debugfs *debugfs = nouveau_debugfs(drm);
 	struct nvif_object *ctrl = &debugfs->ctrl;
 	struct nvif_control_pstate_info_v0 info = {};
 	int ret, i;
@@ -120,8 +120,8 @@ nouveau_debugfs_pstate_set(struct file *file, const char __user *ubuf,
 			   size_t len, loff_t *offp)
 {
 	struct seq_file *m = file->private_data;
-	struct drm_info_node *node = (struct drm_info_node *) m->private;
-	struct nouveau_debugfs *debugfs = nouveau_debugfs(node->minor->dev);
+	struct drm_device *drm = m->private;
+	struct nouveau_debugfs *debugfs = nouveau_debugfs(drm);
 	struct nvif_object *ctrl = &debugfs->ctrl;
 	struct nvif_control_pstate_user_v0 args = { .pwrsrc = -EINVAL };
 	char buf[32] = {}, *tmp, *cur = buf;
@@ -192,42 +192,19 @@ static const struct nouveau_debugfs_files {
 	{"pstate", &nouveau_pstate_fops},
 };
 
-static int
-nouveau_debugfs_create_file(struct drm_minor *minor,
-		const struct nouveau_debugfs_files *ndf)
-{
-	struct drm_info_node *node;
-
-	node = kmalloc(sizeof(*node), GFP_KERNEL);
-	if (node == NULL)
-		return -ENOMEM;
-
-	node->minor = minor;
-	node->info_ent = (const void *)ndf->fops;
-	node->dent = debugfs_create_file(ndf->name, S_IRUGO | S_IWUSR,
-					 minor->debugfs_root, node, ndf->fops);
-	if (!node->dent) {
-		kfree(node);
-		return -ENOMEM;
-	}
-
-	mutex_lock(&minor->debugfs_lock);
-	list_add(&node->list, &minor->debugfs_list);
-	mutex_unlock(&minor->debugfs_lock);
-	return 0;
-}
-
 int
 nouveau_drm_debugfs_init(struct drm_minor *minor)
 {
-	int i, ret;
+	struct dentry *dentry;
+	int i;
 
 	for (i = 0; i < ARRAY_SIZE(nouveau_debugfs_files); i++) {
-		ret = nouveau_debugfs_create_file(minor,
-						  &nouveau_debugfs_files[i]);
-
-		if (ret)
-			return ret;
+		dentry = debugfs_create_file(nouveau_debugfs_files[i].name,
+					     S_IRUGO | S_IWUSR,
+					     minor->debugfs_root, minor->dev,
+					     nouveau_debugfs_files[i].fops);
+		if (!dentry)
+			return -ENOMEM;
 	}
 
 	return drm_debugfs_create_files(nouveau_debugfs_list,
@@ -235,21 +212,6 @@ nouveau_drm_debugfs_init(struct drm_minor *minor)
 					minor->debugfs_root, minor);
 }
 
-void
-nouveau_drm_debugfs_cleanup(struct drm_minor *minor)
-{
-	int i;
-
-	drm_debugfs_remove_files(nouveau_debugfs_list, NOUVEAU_DEBUGFS_ENTRIES,
-				 minor);
-
-	for (i = 0; i < ARRAY_SIZE(nouveau_debugfs_files); i++) {
-		drm_debugfs_remove_files((struct drm_info_list *)
-					 nouveau_debugfs_files[i].fops,
-					 1, minor);
-	}
-}
-
 int
 nouveau_debugfs_init(struct nouveau_drm *drm)
 {
diff --git a/drivers/gpu/drm/nouveau/nouveau_debugfs.h b/drivers/gpu/drm/nouveau/nouveau_debugfs.h
index eab5881..b799f8d 100644
--- a/drivers/gpu/drm/nouveau/nouveau_debugfs.h
+++ b/drivers/gpu/drm/nouveau/nouveau_debugfs.h
@@ -18,7 +18,6 @@ nouveau_debugfs(struct drm_device *dev)
 }
 
 extern int  nouveau_drm_debugfs_init(struct drm_minor *);
-extern void nouveau_drm_debugfs_cleanup(struct drm_minor *);
 extern int  nouveau_debugfs_init(struct nouveau_drm *);
 extern void nouveau_debugfs_fini(struct nouveau_drm *);
 #else
@@ -28,11 +27,6 @@ nouveau_drm_debugfs_init(struct drm_minor *minor)
        return 0;
 }
 
-static inline void
-nouveau_drm_debugfs_cleanup(struct drm_minor *minor)
-{
-}
-
 static inline int
 nouveau_debugfs_init(struct nouveau_drm *drm)
 {
diff --git a/drivers/gpu/drm/nouveau/nouveau_drm.c b/drivers/gpu/drm/nouveau/nouveau_drm.c
index dd7b52a..715741a 100644
--- a/drivers/gpu/drm/nouveau/nouveau_drm.c
+++ b/drivers/gpu/drm/nouveau/nouveau_drm.c
@@ -971,7 +971,6 @@ driver_stub = {
 
 #if defined(CONFIG_DEBUG_FS)
 	.debugfs_init = nouveau_drm_debugfs_init,
-	.debugfs_cleanup = nouveau_drm_debugfs_cleanup,
 #endif
 
 	.get_vblank_counter = drm_vblank_no_hw_counter,
-- 
2.10.2

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 11/19] drm/omap: Remove omap_debugfs_cleanup()

[Noralf Trønnes]

drm_debugfs_cleanup() now removes all minor->debugfs_list entries
automatically, so the drm_driver.debugfs_cleanup callback is not
needed.

Cc: tomi.valkeinen@ti.com
Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
---
 drivers/gpu/drm/omapdrm/omap_debugfs.c | 9 ---------
 drivers/gpu/drm/omapdrm/omap_drv.c     | 1 -
 drivers/gpu/drm/omapdrm/omap_drv.h     | 1 -
 3 files changed, 11 deletions(-)

diff --git a/drivers/gpu/drm/omapdrm/omap_debugfs.c b/drivers/gpu/drm/omapdrm/omap_debugfs.c
index bf65862..19b7167 100644
--- a/drivers/gpu/drm/omapdrm/omap_debugfs.c
+++ b/drivers/gpu/drm/omapdrm/omap_debugfs.c
@@ -123,13 +123,4 @@ int omap_debugfs_init(struct drm_minor *minor)
 	return ret;
 }
 
-void omap_debugfs_cleanup(struct drm_minor *minor)
-{
-	drm_debugfs_remove_files(omap_debugfs_list,
-			ARRAY_SIZE(omap_debugfs_list), minor);
-	if (dmm_is_available())
-		drm_debugfs_remove_files(omap_dmm_debugfs_list,
-				ARRAY_SIZE(omap_dmm_debugfs_list), minor);
-}
-
 #endif
diff --git a/drivers/gpu/drm/omapdrm/omap_drv.c b/drivers/gpu/drm/omapdrm/omap_drv.c
index 4fd2e17..42330e0 100644
--- a/drivers/gpu/drm/omapdrm/omap_drv.c
+++ b/drivers/gpu/drm/omapdrm/omap_drv.c
@@ -812,7 +812,6 @@ static struct drm_driver omap_drm_driver = {
 	.disable_vblank = omap_irq_disable_vblank,
 #ifdef CONFIG_DEBUG_FS
 	.debugfs_init = omap_debugfs_init,
-	.debugfs_cleanup = omap_debugfs_cleanup,
 #endif
 	.prime_handle_to_fd = drm_gem_prime_handle_to_fd,
 	.prime_fd_to_handle = drm_gem_prime_fd_to_handle,
diff --git a/drivers/gpu/drm/omapdrm/omap_drv.h b/drivers/gpu/drm/omapdrm/omap_drv.h
index 7d9dd54..8b113ba 100644
--- a/drivers/gpu/drm/omapdrm/omap_drv.h
+++ b/drivers/gpu/drm/omapdrm/omap_drv.h
@@ -116,7 +116,6 @@ struct omap_drm_private {
 
 #ifdef CONFIG_DEBUG_FS
 int omap_debugfs_init(struct drm_minor *minor);
-void omap_debugfs_cleanup(struct drm_minor *minor);
 void omap_framebuffer_describe(struct drm_framebuffer *fb, struct seq_file *m);
 void omap_gem_describe(struct drm_gem_object *obj, struct seq_file *m);
 void omap_gem_describe_objects(struct list_head *list, struct seq_file *m);
-- 
2.10.2

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 11/19] drm/omap: Remove omap_debugfs_cleanup()

[Tomi Valkeinen]

[-- Attachment #1.1.1: Type: text/plain, Size: 544 bytes --]

On 27/01/17 00:56, Noralf Trønnes wrote:
> drm_debugfs_cleanup() now removes all minor->debugfs_list entries
> automatically, so the drm_driver.debugfs_cleanup callback is not
> needed.
> 
> Cc: tomi.valkeinen@ti.com
> Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
> ---
>  drivers/gpu/drm/omapdrm/omap_debugfs.c | 9 ---------
>  drivers/gpu/drm/omapdrm/omap_drv.c     | 1 -
>  drivers/gpu/drm/omapdrm/omap_drv.h     | 1 -
>  3 files changed, 11 deletions(-)

Reviewed-by: Tomi Valkeinen <tomi.valkeinen@ti.com>

 Tomi


[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 12/19] drm/radeon: Remove drm_debugfs_remove_files() call

[Noralf Trønnes]

drm_debugfs_cleanup() now removes all minor->debugfs_list entries
automatically, so it's not necessary to call drm_debugfs_remove_files().

Cc: alexander.deucher@amd.com
Cc: christian.koenig@amd.com
Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
---
 drivers/gpu/drm/radeon/radeon_device.c | 16 ----------------
 1 file changed, 16 deletions(-)

diff --git a/drivers/gpu/drm/radeon/radeon_device.c b/drivers/gpu/drm/radeon/radeon_device.c
index 8a1df2a..4b0c388 100644
--- a/drivers/gpu/drm/radeon/radeon_device.c
+++ b/drivers/gpu/drm/radeon/radeon_device.c
@@ -1549,8 +1549,6 @@ int radeon_device_init(struct radeon_device *rdev,
 	return r;
 }
 
-static void radeon_debugfs_remove_files(struct radeon_device *rdev);
-
 /**
  * radeon_device_fini - tear down the driver
  *
@@ -1577,7 +1575,6 @@ void radeon_device_fini(struct radeon_device *rdev)
 	rdev->rmmio = NULL;
 	if (rdev->family >= CHIP_BONAIRE)
 		radeon_doorbell_fini(rdev);
-	radeon_debugfs_remove_files(rdev);
 }
 
 
@@ -1954,16 +1951,3 @@ int radeon_debugfs_add_files(struct radeon_device *rdev,
 #endif
 	return 0;
 }
-
-static void radeon_debugfs_remove_files(struct radeon_device *rdev)
-{
-#if defined(CONFIG_DEBUG_FS)
-	unsigned i;
-
-	for (i = 0; i < rdev->debugfs_count; i++) {
-		drm_debugfs_remove_files(rdev->debugfs[i].files,
-					 rdev->debugfs[i].num_files,
-					 rdev->ddev->primary);
-	}
-#endif
-}
-- 
2.10.2

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 12/19] drm/radeon: Remove drm_debugfs_remove_files() call

[Christian König]

Am 26.01.2017 um 23:56 schrieb Noralf Trønnes:
> drm_debugfs_cleanup() now removes all minor->debugfs_list entries
> automatically, so it's not necessary to call drm_debugfs_remove_files().
>
> Cc: alexander.deucher@amd.com
> Cc: christian.koenig@amd.com
> Signed-off-by: Noralf Trønnes <noralf@tronnes.org>

Great work! I wanted to do this for quite a while as well, but never 
found time to look into it.

A good first step, but when we don't have radeon_debugfs_remove_files() 
any more we don't need rdev->debugfs and all the related housekeeping 
either.

Additional to that we have a couple of entries added/removed to the 
minor manually, just grep for "debugfs_remove" in the radeon code. Those 
calls can now be removed as well.

But all that can be done in separate patches, this one is Reviewed-by: 
Christian König <christian.koenig@amd.com>.

Regards,
Christian.

> ---
>   drivers/gpu/drm/radeon/radeon_device.c | 16 ----------------
>   1 file changed, 16 deletions(-)
>
> diff --git a/drivers/gpu/drm/radeon/radeon_device.c b/drivers/gpu/drm/radeon/radeon_device.c
> index 8a1df2a..4b0c388 100644
> --- a/drivers/gpu/drm/radeon/radeon_device.c
> +++ b/drivers/gpu/drm/radeon/radeon_device.c
> @@ -1549,8 +1549,6 @@ int radeon_device_init(struct radeon_device *rdev,
>   	return r;
>   }
>   
> -static void radeon_debugfs_remove_files(struct radeon_device *rdev);
> -
>   /**
>    * radeon_device_fini - tear down the driver
>    *
> @@ -1577,7 +1575,6 @@ void radeon_device_fini(struct radeon_device *rdev)
>   	rdev->rmmio = NULL;
>   	if (rdev->family >= CHIP_BONAIRE)
>   		radeon_doorbell_fini(rdev);
> -	radeon_debugfs_remove_files(rdev);
>   }
>   
>   
> @@ -1954,16 +1951,3 @@ int radeon_debugfs_add_files(struct radeon_device *rdev,
>   #endif
>   	return 0;
>   }
> -
> -static void radeon_debugfs_remove_files(struct radeon_device *rdev)
> -{
> -#if defined(CONFIG_DEBUG_FS)
> -	unsigned i;
> -
> -	for (i = 0; i < rdev->debugfs_count; i++) {
> -		drm_debugfs_remove_files(rdev->debugfs[i].files,
> -					 rdev->debugfs[i].num_files,
> -					 rdev->ddev->primary);
> -	}
> -#endif
> -}


_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 13/19] drm/sti: Remove drm_debugfs_remove_files() calls

[Noralf Trønnes]

drm_debugfs_cleanup() now removes all minor->debugfs_list entries
automatically, so it's not necessary to call
drm_debugfs_remove_files(). Additionally it uses
debugfs_remove_recursive() to clean up the debugfs files, so no need
for adding fake drm_info_node entries.

Cc: benjamin.gaignard@linaro.org
Cc: vincent.abriou@st.com
Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
---
 drivers/gpu/drm/sti/sti_drv.c   | 48 ++++++-----------------------------------
 drivers/gpu/drm/sti/sti_dvo.c   | 10 ---------
 drivers/gpu/drm/sti/sti_hda.c   | 11 ----------
 drivers/gpu/drm/sti/sti_hdmi.c  | 11 ----------
 drivers/gpu/drm/sti/sti_tvout.c |  8 -------
 5 files changed, 6 insertions(+), 82 deletions(-)

diff --git a/drivers/gpu/drm/sti/sti_drv.c b/drivers/gpu/drm/sti/sti_drv.c
index ff71e25..d3db224 100644
--- a/drivers/gpu/drm/sti/sti_drv.c
+++ b/drivers/gpu/drm/sti/sti_drv.c
@@ -89,38 +89,9 @@ static struct drm_info_list sti_drm_dbg_list[] = {
 	{"fps_get", sti_drm_fps_dbg_show, 0},
 };
 
-static int sti_drm_debugfs_create(struct dentry *root,
-				  struct drm_minor *minor,
-				  const char *name,
-				  const struct file_operations *fops)
-{
-	struct drm_device *dev = minor->dev;
-	struct drm_info_node *node;
-	struct dentry *ent;
-
-	ent = debugfs_create_file(name, S_IRUGO | S_IWUSR, root, dev, fops);
-	if (IS_ERR(ent))
-		return PTR_ERR(ent);
-
-	node = kmalloc(sizeof(*node), GFP_KERNEL);
-	if (!node) {
-		debugfs_remove(ent);
-		return -ENOMEM;
-	}
-
-	node->minor = minor;
-	node->dent = ent;
-	node->info_ent = (void *)fops;
-
-	mutex_lock(&minor->debugfs_lock);
-	list_add(&node->list, &minor->debugfs_list);
-	mutex_unlock(&minor->debugfs_lock);
-
-	return 0;
-}
-
 static int sti_drm_dbg_init(struct drm_minor *minor)
 {
+	struct dentry *dentry;
 	int ret;
 
 	ret = drm_debugfs_create_files(sti_drm_dbg_list,
@@ -129,10 +100,13 @@ static int sti_drm_dbg_init(struct drm_minor *minor)
 	if (ret)
 		goto err;
 
-	ret = sti_drm_debugfs_create(minor->debugfs_root, minor, "fps_show",
+	dentry = debugfs_create_file("fps_show", S_IRUGO | S_IWUSR,
+				     minor->debugfs_root, minor->dev,
 				     &sti_drm_fps_fops);
-	if (ret)
+	if (!dentry) {
+		ret = -ENOMEM;
 		goto err;
+	}
 
 	DRM_INFO("%s: debugfs installed\n", DRIVER_NAME);
 	return 0;
@@ -141,15 +115,6 @@ static int sti_drm_dbg_init(struct drm_minor *minor)
 	return ret;
 }
 
-static void sti_drm_dbg_cleanup(struct drm_minor *minor)
-{
-	drm_debugfs_remove_files(sti_drm_dbg_list,
-				 ARRAY_SIZE(sti_drm_dbg_list), minor);
-
-	drm_debugfs_remove_files((struct drm_info_list *)&sti_drm_fps_fops,
-				 1, minor);
-}
-
 static void sti_atomic_schedule(struct sti_private *private,
 				struct drm_atomic_state *state)
 {
@@ -326,7 +291,6 @@ static struct drm_driver sti_driver = {
 	.gem_prime_mmap = drm_gem_cma_prime_mmap,
 
 	.debugfs_init = sti_drm_dbg_init,
-	.debugfs_cleanup = sti_drm_dbg_cleanup,
 
 	.name = DRIVER_NAME,
 	.desc = DRIVER_DESC,
diff --git a/drivers/gpu/drm/sti/sti_dvo.c b/drivers/gpu/drm/sti/sti_dvo.c
index 411dc6e..bb23318 100644
--- a/drivers/gpu/drm/sti/sti_dvo.c
+++ b/drivers/gpu/drm/sti/sti_dvo.c
@@ -195,13 +195,6 @@ static struct drm_info_list dvo_debugfs_files[] = {
 	{ "dvo", dvo_dbg_show, 0, NULL },
 };
 
-static void dvo_debugfs_exit(struct sti_dvo *dvo, struct drm_minor *minor)
-{
-	drm_debugfs_remove_files(dvo_debugfs_files,
-				 ARRAY_SIZE(dvo_debugfs_files),
-				 minor);
-}
-
 static int dvo_debugfs_init(struct sti_dvo *dvo, struct drm_minor *minor)
 {
 	unsigned int i;
@@ -514,9 +507,6 @@ static void sti_dvo_unbind(struct device *dev,
 			   struct device *master, void *data)
 {
 	struct sti_dvo *dvo = dev_get_drvdata(dev);
-	struct drm_device *drm_dev = data;
-
-	dvo_debugfs_exit(dvo, drm_dev->primary);
 
 	drm_bridge_remove(dvo->bridge);
 }
diff --git a/drivers/gpu/drm/sti/sti_hda.c b/drivers/gpu/drm/sti/sti_hda.c
index 66d37d78..0c0a75b 100644
--- a/drivers/gpu/drm/sti/sti_hda.c
+++ b/drivers/gpu/drm/sti/sti_hda.c
@@ -365,13 +365,6 @@ static struct drm_info_list hda_debugfs_files[] = {
 	{ "hda", hda_dbg_show, 0, NULL },
 };
 
-static void hda_debugfs_exit(struct sti_hda *hda, struct drm_minor *minor)
-{
-	drm_debugfs_remove_files(hda_debugfs_files,
-				 ARRAY_SIZE(hda_debugfs_files),
-				 minor);
-}
-
 static int hda_debugfs_init(struct sti_hda *hda, struct drm_minor *minor)
 {
 	unsigned int i;
@@ -739,10 +732,6 @@ static int sti_hda_bind(struct device *dev, struct device *master, void *data)
 static void sti_hda_unbind(struct device *dev,
 		struct device *master, void *data)
 {
-	struct sti_hda *hda = dev_get_drvdata(dev);
-	struct drm_device *drm_dev = data;
-
-	hda_debugfs_exit(hda, drm_dev->primary);
 }
 
 static const struct component_ops sti_hda_ops = {
diff --git a/drivers/gpu/drm/sti/sti_hdmi.c b/drivers/gpu/drm/sti/sti_hdmi.c
index f0af1ae..4a8bd62 100644
--- a/drivers/gpu/drm/sti/sti_hdmi.c
+++ b/drivers/gpu/drm/sti/sti_hdmi.c
@@ -731,13 +731,6 @@ static struct drm_info_list hdmi_debugfs_files[] = {
 	{ "hdmi", hdmi_dbg_show, 0, NULL },
 };
 
-static void hdmi_debugfs_exit(struct sti_hdmi *hdmi, struct drm_minor *minor)
-{
-	drm_debugfs_remove_files(hdmi_debugfs_files,
-				 ARRAY_SIZE(hdmi_debugfs_files),
-				 minor);
-}
-
 static int hdmi_debugfs_init(struct sti_hdmi *hdmi, struct drm_minor *minor)
 {
 	unsigned int i;
@@ -1359,10 +1352,6 @@ static int sti_hdmi_bind(struct device *dev, struct device *master, void *data)
 static void sti_hdmi_unbind(struct device *dev,
 		struct device *master, void *data)
 {
-	struct sti_hdmi *hdmi = dev_get_drvdata(dev);
-	struct drm_device *drm_dev = data;
-
-	hdmi_debugfs_exit(hdmi, drm_dev->primary);
 }
 
 static const struct component_ops sti_hdmi_ops = {
diff --git a/drivers/gpu/drm/sti/sti_tvout.c b/drivers/gpu/drm/sti/sti_tvout.c
index ad46d35..8b8ea71 100644
--- a/drivers/gpu/drm/sti/sti_tvout.c
+++ b/drivers/gpu/drm/sti/sti_tvout.c
@@ -567,13 +567,6 @@ static struct drm_info_list tvout_debugfs_files[] = {
 	{ "tvout", tvout_dbg_show, 0, NULL },
 };
 
-static void tvout_debugfs_exit(struct sti_tvout *tvout, struct drm_minor *minor)
-{
-	drm_debugfs_remove_files(tvout_debugfs_files,
-				 ARRAY_SIZE(tvout_debugfs_files),
-				 minor);
-}
-
 static int tvout_debugfs_init(struct sti_tvout *tvout, struct drm_minor *minor)
 {
 	unsigned int i;
@@ -627,7 +620,6 @@ static void sti_tvout_early_unregister(struct drm_encoder *encoder)
 	if (!tvout->debugfs_registered)
 		return;
 
-	tvout_debugfs_exit(tvout, encoder->dev->primary);
 	tvout->debugfs_registered = false;
 }
 
-- 
2.10.2

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 13/19] drm/sti: Remove drm_debugfs_remove_files() calls

[Vincent ABRIOU]

Thank for this patch.
It is working fine with sti driver.

Acked-by: Vincent Abriou <vincent.abriou@st.com>
Tested-by: Vincent Abriou <vincent.abriou@st.com>

Vincent

On 01/26/2017 11:56 PM, Noralf Trønnes wrote:
> drm_debugfs_cleanup() now removes all minor->debugfs_list entries
> automatically, so it's not necessary to call
> drm_debugfs_remove_files(). Additionally it uses
> debugfs_remove_recursive() to clean up the debugfs files, so no need
> for adding fake drm_info_node entries.
>
> Cc: benjamin.gaignard@linaro.org
> Cc: vincent.abriou@st.com
> Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
> ---
>  drivers/gpu/drm/sti/sti_drv.c   | 48 ++++++-----------------------------------
>  drivers/gpu/drm/sti/sti_dvo.c   | 10 ---------
>  drivers/gpu/drm/sti/sti_hda.c   | 11 ----------
>  drivers/gpu/drm/sti/sti_hdmi.c  | 11 ----------
>  drivers/gpu/drm/sti/sti_tvout.c |  8 -------
>  5 files changed, 6 insertions(+), 82 deletions(-)
>
> diff --git a/drivers/gpu/drm/sti/sti_drv.c b/drivers/gpu/drm/sti/sti_drv.c
> index ff71e25..d3db224 100644
> --- a/drivers/gpu/drm/sti/sti_drv.c
> +++ b/drivers/gpu/drm/sti/sti_drv.c
> @@ -89,38 +89,9 @@ static struct drm_info_list sti_drm_dbg_list[] = {
>  	{"fps_get", sti_drm_fps_dbg_show, 0},
>  };
>
> -static int sti_drm_debugfs_create(struct dentry *root,
> -				  struct drm_minor *minor,
> -				  const char *name,
> -				  const struct file_operations *fops)
> -{
> -	struct drm_device *dev = minor->dev;
> -	struct drm_info_node *node;
> -	struct dentry *ent;
> -
> -	ent = debugfs_create_file(name, S_IRUGO | S_IWUSR, root, dev, fops);
> -	if (IS_ERR(ent))
> -		return PTR_ERR(ent);
> -
> -	node = kmalloc(sizeof(*node), GFP_KERNEL);
> -	if (!node) {
> -		debugfs_remove(ent);
> -		return -ENOMEM;
> -	}
> -
> -	node->minor = minor;
> -	node->dent = ent;
> -	node->info_ent = (void *)fops;
> -
> -	mutex_lock(&minor->debugfs_lock);
> -	list_add(&node->list, &minor->debugfs_list);
> -	mutex_unlock(&minor->debugfs_lock);
> -
> -	return 0;
> -}
> -
>  static int sti_drm_dbg_init(struct drm_minor *minor)
>  {
> +	struct dentry *dentry;
>  	int ret;
>
>  	ret = drm_debugfs_create_files(sti_drm_dbg_list,
> @@ -129,10 +100,13 @@ static int sti_drm_dbg_init(struct drm_minor *minor)
>  	if (ret)
>  		goto err;
>
> -	ret = sti_drm_debugfs_create(minor->debugfs_root, minor, "fps_show",
> +	dentry = debugfs_create_file("fps_show", S_IRUGO | S_IWUSR,
> +				     minor->debugfs_root, minor->dev,
>  				     &sti_drm_fps_fops);
> -	if (ret)
> +	if (!dentry) {
> +		ret = -ENOMEM;
>  		goto err;
> +	}
>
>  	DRM_INFO("%s: debugfs installed\n", DRIVER_NAME);
>  	return 0;
> @@ -141,15 +115,6 @@ static int sti_drm_dbg_init(struct drm_minor *minor)
>  	return ret;
>  }
>
> -static void sti_drm_dbg_cleanup(struct drm_minor *minor)
> -{
> -	drm_debugfs_remove_files(sti_drm_dbg_list,
> -				 ARRAY_SIZE(sti_drm_dbg_list), minor);
> -
> -	drm_debugfs_remove_files((struct drm_info_list *)&sti_drm_fps_fops,
> -				 1, minor);
> -}
> -
>  static void sti_atomic_schedule(struct sti_private *private,
>  				struct drm_atomic_state *state)
>  {
> @@ -326,7 +291,6 @@ static struct drm_driver sti_driver = {
>  	.gem_prime_mmap = drm_gem_cma_prime_mmap,
>
>  	.debugfs_init = sti_drm_dbg_init,
> -	.debugfs_cleanup = sti_drm_dbg_cleanup,
>
>  	.name = DRIVER_NAME,
>  	.desc = DRIVER_DESC,
> diff --git a/drivers/gpu/drm/sti/sti_dvo.c b/drivers/gpu/drm/sti/sti_dvo.c
> index 411dc6e..bb23318 100644
> --- a/drivers/gpu/drm/sti/sti_dvo.c
> +++ b/drivers/gpu/drm/sti/sti_dvo.c
> @@ -195,13 +195,6 @@ static struct drm_info_list dvo_debugfs_files[] = {
>  	{ "dvo", dvo_dbg_show, 0, NULL },
>  };
>
> -static void dvo_debugfs_exit(struct sti_dvo *dvo, struct drm_minor *minor)
> -{
> -	drm_debugfs_remove_files(dvo_debugfs_files,
> -				 ARRAY_SIZE(dvo_debugfs_files),
> -				 minor);
> -}
> -
>  static int dvo_debugfs_init(struct sti_dvo *dvo, struct drm_minor *minor)
>  {
>  	unsigned int i;
> @@ -514,9 +507,6 @@ static void sti_dvo_unbind(struct device *dev,
>  			   struct device *master, void *data)
>  {
>  	struct sti_dvo *dvo = dev_get_drvdata(dev);
> -	struct drm_device *drm_dev = data;
> -
> -	dvo_debugfs_exit(dvo, drm_dev->primary);
>
>  	drm_bridge_remove(dvo->bridge);
>  }
> diff --git a/drivers/gpu/drm/sti/sti_hda.c b/drivers/gpu/drm/sti/sti_hda.c
> index 66d37d78..0c0a75b 100644
> --- a/drivers/gpu/drm/sti/sti_hda.c
> +++ b/drivers/gpu/drm/sti/sti_hda.c
> @@ -365,13 +365,6 @@ static struct drm_info_list hda_debugfs_files[] = {
>  	{ "hda", hda_dbg_show, 0, NULL },
>  };
>
> -static void hda_debugfs_exit(struct sti_hda *hda, struct drm_minor *minor)
> -{
> -	drm_debugfs_remove_files(hda_debugfs_files,
> -				 ARRAY_SIZE(hda_debugfs_files),
> -				 minor);
> -}
> -
>  static int hda_debugfs_init(struct sti_hda *hda, struct drm_minor *minor)
>  {
>  	unsigned int i;
> @@ -739,10 +732,6 @@ static int sti_hda_bind(struct device *dev, struct device *master, void *data)
>  static void sti_hda_unbind(struct device *dev,
>  		struct device *master, void *data)
>  {
> -	struct sti_hda *hda = dev_get_drvdata(dev);
> -	struct drm_device *drm_dev = data;
> -
> -	hda_debugfs_exit(hda, drm_dev->primary);
>  }
>
>  static const struct component_ops sti_hda_ops = {
> diff --git a/drivers/gpu/drm/sti/sti_hdmi.c b/drivers/gpu/drm/sti/sti_hdmi.c
> index f0af1ae..4a8bd62 100644
> --- a/drivers/gpu/drm/sti/sti_hdmi.c
> +++ b/drivers/gpu/drm/sti/sti_hdmi.c
> @@ -731,13 +731,6 @@ static struct drm_info_list hdmi_debugfs_files[] = {
>  	{ "hdmi", hdmi_dbg_show, 0, NULL },
>  };
>
> -static void hdmi_debugfs_exit(struct sti_hdmi *hdmi, struct drm_minor *minor)
> -{
> -	drm_debugfs_remove_files(hdmi_debugfs_files,
> -				 ARRAY_SIZE(hdmi_debugfs_files),
> -				 minor);
> -}
> -
>  static int hdmi_debugfs_init(struct sti_hdmi *hdmi, struct drm_minor *minor)
>  {
>  	unsigned int i;
> @@ -1359,10 +1352,6 @@ static int sti_hdmi_bind(struct device *dev, struct device *master, void *data)
>  static void sti_hdmi_unbind(struct device *dev,
>  		struct device *master, void *data)
>  {
> -	struct sti_hdmi *hdmi = dev_get_drvdata(dev);
> -	struct drm_device *drm_dev = data;
> -
> -	hdmi_debugfs_exit(hdmi, drm_dev->primary);
>  }
>
>  static const struct component_ops sti_hdmi_ops = {
> diff --git a/drivers/gpu/drm/sti/sti_tvout.c b/drivers/gpu/drm/sti/sti_tvout.c
> index ad46d35..8b8ea71 100644
> --- a/drivers/gpu/drm/sti/sti_tvout.c
> +++ b/drivers/gpu/drm/sti/sti_tvout.c
> @@ -567,13 +567,6 @@ static struct drm_info_list tvout_debugfs_files[] = {
>  	{ "tvout", tvout_dbg_show, 0, NULL },
>  };
>
> -static void tvout_debugfs_exit(struct sti_tvout *tvout, struct drm_minor *minor)
> -{
> -	drm_debugfs_remove_files(tvout_debugfs_files,
> -				 ARRAY_SIZE(tvout_debugfs_files),
> -				 minor);
> -}
> -
>  static int tvout_debugfs_init(struct sti_tvout *tvout, struct drm_minor *minor)
>  {
>  	unsigned int i;
> @@ -627,7 +620,6 @@ static void sti_tvout_early_unregister(struct drm_encoder *encoder)
>  	if (!tvout->debugfs_registered)
>  		return;
>
> -	tvout_debugfs_exit(tvout, encoder->dev->primary);
>  	tvout->debugfs_registered = false;
>  }
>
>
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 14/19] drm/tegra: Remove tegra_debugfs_cleanup()

[Noralf Trønnes]

drm_debugfs_cleanup() now removes all minor->debugfs_list entries
automatically, so the drm_driver.debugfs_cleanup callback is not
needed.

Cc: thierry.reding@gmail.com
Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
---
 drivers/gpu/drm/tegra/drm.c | 7 -------
 1 file changed, 7 deletions(-)

diff --git a/drivers/gpu/drm/tegra/drm.c b/drivers/gpu/drm/tegra/drm.c
index 2d57f62..ef215fe 100644
--- a/drivers/gpu/drm/tegra/drm.c
+++ b/drivers/gpu/drm/tegra/drm.c
@@ -894,12 +894,6 @@ static int tegra_debugfs_init(struct drm_minor *minor)
 					ARRAY_SIZE(tegra_debugfs_list),
 					minor->debugfs_root, minor);
 }
-
-static void tegra_debugfs_cleanup(struct drm_minor *minor)
-{
-	drm_debugfs_remove_files(tegra_debugfs_list,
-				 ARRAY_SIZE(tegra_debugfs_list), minor);
-}
 #endif
 
 static struct drm_driver tegra_drm_driver = {
@@ -917,7 +911,6 @@ static struct drm_driver tegra_drm_driver = {
 
 #if defined(CONFIG_DEBUG_FS)
 	.debugfs_init = tegra_debugfs_init,
-	.debugfs_cleanup = tegra_debugfs_cleanup,
 #endif
 
 	.gem_free_object_unlocked = tegra_bo_free_object,
-- 
2.10.2

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 14/19] drm/tegra: Remove tegra_debugfs_cleanup()

[Daniel Vetter]

On Thu, Jan 26, 2017 at 11:56:16PM +0100, Noralf Trønnes wrote:
> drm_debugfs_cleanup() now removes all minor->debugfs_list entries
> automatically, so the drm_driver.debugfs_cleanup callback is not
> needed.
> 
> Cc: thierry.reding@gmail.com
> Signed-off-by: Noralf Trønnes <noralf@tronnes.org>

Already mentioned on the cover letter, but I think you can savely remove
all other debugfs cleanup calls too.
-Daniel
> ---
>  drivers/gpu/drm/tegra/drm.c | 7 -------
>  1 file changed, 7 deletions(-)
> 
> diff --git a/drivers/gpu/drm/tegra/drm.c b/drivers/gpu/drm/tegra/drm.c
> index 2d57f62..ef215fe 100644
> --- a/drivers/gpu/drm/tegra/drm.c
> +++ b/drivers/gpu/drm/tegra/drm.c
> @@ -894,12 +894,6 @@ static int tegra_debugfs_init(struct drm_minor *minor)
>  					ARRAY_SIZE(tegra_debugfs_list),
>  					minor->debugfs_root, minor);
>  }
> -
> -static void tegra_debugfs_cleanup(struct drm_minor *minor)
> -{
> -	drm_debugfs_remove_files(tegra_debugfs_list,
> -				 ARRAY_SIZE(tegra_debugfs_list), minor);
> -}
>  #endif
>  
>  static struct drm_driver tegra_drm_driver = {
> @@ -917,7 +911,6 @@ static struct drm_driver tegra_drm_driver = {
>  
>  #if defined(CONFIG_DEBUG_FS)
>  	.debugfs_init = tegra_debugfs_init,
> -	.debugfs_cleanup = tegra_debugfs_cleanup,
>  #endif
>  
>  	.gem_free_object_unlocked = tegra_bo_free_object,
> -- 
> 2.10.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
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 14/19] drm/tegra: Remove tegra_debugfs_cleanup()

[Thierry Reding]

[-- Attachment #1.1: Type: text/plain, Size: 443 bytes --]

On Thu, Jan 26, 2017 at 11:56:16PM +0100, Noralf Trønnes wrote:
> drm_debugfs_cleanup() now removes all minor->debugfs_list entries
> automatically, so the drm_driver.debugfs_cleanup callback is not
> needed.
> 
> Cc: thierry.reding@gmail.com
> Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
> ---
>  drivers/gpu/drm/tegra/drm.c | 7 -------
>  1 file changed, 7 deletions(-)

Reviewed-by: Thierry Reding <treding@nvidia.com>

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 15/19] drm/tilcdc: Remove tilcdc_debugfs_cleanup()

[Noralf Trønnes]

drm_debugfs_cleanup() now removes all minor->debugfs_list entries
automatically, so the drm_driver.debugfs_cleanup callback is not
needed. Also remove the unused tilcdc_module_ops.debugfs_cleanup()
callback. drm_debugfs_cleanup() removes all debugfs files using
debugfs_remove_recursive(), so there should be no need for such a
callback in the future.

Cc: jsarha@ti.com
Cc: tomi.valkeinen@ti.com
Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
---
 drivers/gpu/drm/tilcdc/tilcdc_drv.c | 12 ------------
 drivers/gpu/drm/tilcdc/tilcdc_drv.h |  2 --
 2 files changed, 14 deletions(-)

diff --git a/drivers/gpu/drm/tilcdc/tilcdc_drv.c b/drivers/gpu/drm/tilcdc/tilcdc_drv.c
index ec15585..919294a 100644
--- a/drivers/gpu/drm/tilcdc/tilcdc_drv.c
+++ b/drivers/gpu/drm/tilcdc/tilcdc_drv.c
@@ -539,17 +539,6 @@ static int tilcdc_debugfs_init(struct drm_minor *minor)
 
 	return ret;
 }
-
-static void tilcdc_debugfs_cleanup(struct drm_minor *minor)
-{
-	struct tilcdc_module *mod;
-	drm_debugfs_remove_files(tilcdc_debugfs_list,
-			ARRAY_SIZE(tilcdc_debugfs_list), minor);
-
-	list_for_each_entry(mod, &module_list, list)
-		if (mod->funcs->debugfs_cleanup)
-			mod->funcs->debugfs_cleanup(mod, minor);
-}
 #endif
 
 static const struct file_operations fops = {
@@ -589,7 +578,6 @@ static struct drm_driver tilcdc_driver = {
 	.gem_prime_mmap		= drm_gem_cma_prime_mmap,
 #ifdef CONFIG_DEBUG_FS
 	.debugfs_init       = tilcdc_debugfs_init,
-	.debugfs_cleanup    = tilcdc_debugfs_cleanup,
 #endif
 	.fops               = &fops,
 	.name               = "tilcdc",
diff --git a/drivers/gpu/drm/tilcdc/tilcdc_drv.h b/drivers/gpu/drm/tilcdc/tilcdc_drv.h
index 0e71daf..8caa11b 100644
--- a/drivers/gpu/drm/tilcdc/tilcdc_drv.h
+++ b/drivers/gpu/drm/tilcdc/tilcdc_drv.h
@@ -111,8 +111,6 @@ struct tilcdc_module_ops {
 #ifdef CONFIG_DEBUG_FS
 	/* create debugfs nodes (can be NULL): */
 	int (*debugfs_init)(struct tilcdc_module *mod, struct drm_minor *minor);
-	/* cleanup debugfs nodes (can be NULL): */
-	void (*debugfs_cleanup)(struct tilcdc_module *mod, struct drm_minor *minor);
 #endif
 };
 
-- 
2.10.2

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 15/19] drm/tilcdc: Remove tilcdc_debugfs_cleanup()

[Jyri Sarha]

On 01/27/17 00:56, Noralf Trønnes wrote:
> drm_debugfs_cleanup() now removes all minor->debugfs_list entries
> automatically, so the drm_driver.debugfs_cleanup callback is not
> needed. Also remove the unused tilcdc_module_ops.debugfs_cleanup()
> callback. drm_debugfs_cleanup() removes all debugfs files using
> debugfs_remove_recursive(), so there should be no need for such a
> callback in the future.
> 
> Cc: jsarha@ti.com
> Cc: tomi.valkeinen@ti.com
> Signed-off-by: Noralf Trønnes <noralf@tronnes.org>

Reviewed-by: Jyri Sarha <jsarha@ti.com>

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 16/19] drm/vc4: Remove vc4_debugfs_cleanup()

[Noralf Trønnes]

drm_debugfs_cleanup() now removes all minor->debugfs_list entries
automatically, so the drm_driver.debugfs_cleanup callback is not
needed.

Cc: eric@anholt.net
Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
---
 drivers/gpu/drm/vc4/vc4_debugfs.c | 6 ------
 drivers/gpu/drm/vc4/vc4_drv.c     | 1 -
 drivers/gpu/drm/vc4/vc4_drv.h     | 1 -
 3 files changed, 8 deletions(-)

diff --git a/drivers/gpu/drm/vc4/vc4_debugfs.c b/drivers/gpu/drm/vc4/vc4_debugfs.c
index caf817b..c4d5e6a 100644
--- a/drivers/gpu/drm/vc4/vc4_debugfs.c
+++ b/drivers/gpu/drm/vc4/vc4_debugfs.c
@@ -36,9 +36,3 @@ vc4_debugfs_init(struct drm_minor *minor)
 	return drm_debugfs_create_files(vc4_debugfs_list, VC4_DEBUGFS_ENTRIES,
 					minor->debugfs_root, minor);
 }
-
-void
-vc4_debugfs_cleanup(struct drm_minor *minor)
-{
-	drm_debugfs_remove_files(vc4_debugfs_list, VC4_DEBUGFS_ENTRIES, minor);
-}
diff --git a/drivers/gpu/drm/vc4/vc4_drv.c b/drivers/gpu/drm/vc4/vc4_drv.c
index ac09ca7..e4f42eb 100644
--- a/drivers/gpu/drm/vc4/vc4_drv.c
+++ b/drivers/gpu/drm/vc4/vc4_drv.c
@@ -145,7 +145,6 @@ static struct drm_driver vc4_drm_driver = {
 
 #if defined(CONFIG_DEBUG_FS)
 	.debugfs_init = vc4_debugfs_init,
-	.debugfs_cleanup = vc4_debugfs_cleanup,
 #endif
 
 	.gem_create_object = vc4_create_object,
diff --git a/drivers/gpu/drm/vc4/vc4_drv.h b/drivers/gpu/drm/vc4/vc4_drv.h
index affcdeb..78e3e5a 100644
--- a/drivers/gpu/drm/vc4/vc4_drv.h
+++ b/drivers/gpu/drm/vc4/vc4_drv.h
@@ -457,7 +457,6 @@ int vc4_crtc_get_vblank_timestamp(struct drm_device *dev, unsigned int crtc_id,
 
 /* vc4_debugfs.c */
 int vc4_debugfs_init(struct drm_minor *minor);
-void vc4_debugfs_cleanup(struct drm_minor *minor);
 
 /* vc4_drv.c */
 void __iomem *vc4_ioremap_regs(struct platform_device *dev, int index);
-- 
2.10.2

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 16/19] drm/vc4: Remove vc4_debugfs_cleanup()

[Eric Anholt]

[-- Attachment #1.1: Type: text/plain, Size: 251 bytes --]

Noralf Trønnes <noralf@tronnes.org> writes:

> drm_debugfs_cleanup() now removes all minor->debugfs_list entries
> automatically, so the drm_driver.debugfs_cleanup callback is not
> needed.

Nice!

Reviewed-by: Eric Anholt <eric@anholt.net>

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 832 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 16/19] drm/vc4: Remove vc4_debugfs_cleanup()

[Daniel Vetter]

On Fri, Jan 27, 2017 at 09:56:07AM -0800, Eric Anholt wrote:
> Noralf Trønnes <noralf@tronnes.org> writes:
> 
> > drm_debugfs_cleanup() now removes all minor->debugfs_list entries
> > automatically, so the drm_driver.debugfs_cleanup callback is not
> > needed.
> 
> Nice!
> 
> Reviewed-by: Eric Anholt <eric@anholt.net>

Applied to drm-misc, thanks.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 17/19] drm/virtio: Remove virtio_gpu_debugfs_takedown()

[Noralf Trønnes]

drm_debugfs_cleanup() now removes all minor->debugfs_list entries
automatically, so the drm_driver.debugfs_cleanup callback is not
needed.

Cc: airlied@linux.ie
Cc: kraxel@redhat.com
Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
---
 drivers/gpu/drm/virtio/virtgpu_debugfs.c | 8 --------
 drivers/gpu/drm/virtio/virtgpu_drv.c     | 1 -
 drivers/gpu/drm/virtio/virtgpu_drv.h     | 1 -
 3 files changed, 10 deletions(-)

diff --git a/drivers/gpu/drm/virtio/virtgpu_debugfs.c b/drivers/gpu/drm/virtio/virtgpu_debugfs.c
index 5122639..f51240a 100644
--- a/drivers/gpu/drm/virtio/virtgpu_debugfs.c
+++ b/drivers/gpu/drm/virtio/virtgpu_debugfs.c
@@ -54,11 +54,3 @@ virtio_gpu_debugfs_init(struct drm_minor *minor)
 				 minor->debugfs_root, minor);
 	return 0;
 }
-
-void
-virtio_gpu_debugfs_takedown(struct drm_minor *minor)
-{
-	drm_debugfs_remove_files(virtio_gpu_debugfs_list,
-				 VIRTIO_GPU_DEBUGFS_ENTRIES,
-				 minor);
-}
diff --git a/drivers/gpu/drm/virtio/virtgpu_drv.c b/drivers/gpu/drm/virtio/virtgpu_drv.c
index d824898..2d29b01 100644
--- a/drivers/gpu/drm/virtio/virtgpu_drv.c
+++ b/drivers/gpu/drm/virtio/virtgpu_drv.c
@@ -126,7 +126,6 @@ static struct drm_driver driver = {
 
 #if defined(CONFIG_DEBUG_FS)
 	.debugfs_init = virtio_gpu_debugfs_init,
-	.debugfs_cleanup = virtio_gpu_debugfs_takedown,
 #endif
 	.prime_handle_to_fd = drm_gem_prime_handle_to_fd,
 	.prime_fd_to_handle = drm_gem_prime_fd_to_handle,
diff --git a/drivers/gpu/drm/virtio/virtgpu_drv.h b/drivers/gpu/drm/virtio/virtgpu_drv.h
index 2f76673..d59f689 100644
--- a/drivers/gpu/drm/virtio/virtgpu_drv.h
+++ b/drivers/gpu/drm/virtio/virtgpu_drv.h
@@ -422,6 +422,5 @@ static inline void virtio_gpu_object_unreserve(struct virtio_gpu_object *bo)
 
 /* virgl debufs */
 int virtio_gpu_debugfs_init(struct drm_minor *minor);
-void virtio_gpu_debugfs_takedown(struct drm_minor *minor);
 
 #endif
-- 
2.10.2

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 18/19] drm/qxl: Remove qxl_debugfs_takedown()

[Noralf Trønnes]

drm_debugfs_cleanup() now removes all minor->debugfs_list entries
automatically, so the drm_driver.debugfs_cleanup callback is not
needed.

Cc: airlied@linux.ie
Cc: kraxel@redhat.com
Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
---
 drivers/gpu/drm/qxl/qxl_debugfs.c | 9 ---------
 drivers/gpu/drm/qxl/qxl_drv.c     | 1 -
 drivers/gpu/drm/qxl/qxl_drv.h     | 1 -
 3 files changed, 11 deletions(-)

diff --git a/drivers/gpu/drm/qxl/qxl_debugfs.c b/drivers/gpu/drm/qxl/qxl_debugfs.c
index 057b2b5..147e20e 100644
--- a/drivers/gpu/drm/qxl/qxl_debugfs.c
+++ b/drivers/gpu/drm/qxl/qxl_debugfs.c
@@ -100,15 +100,6 @@ qxl_debugfs_init(struct drm_minor *minor)
 	return 0;
 }
 
-void
-qxl_debugfs_takedown(struct drm_minor *minor)
-{
-#if defined(CONFIG_DEBUG_FS)
-	drm_debugfs_remove_files(qxl_debugfs_list, QXL_DEBUGFS_ENTRIES,
-				 minor);
-#endif
-}
-
 int qxl_debugfs_add_files(struct qxl_device *qdev,
 			  struct drm_info_list *files,
 			  unsigned nfiles)
diff --git a/drivers/gpu/drm/qxl/qxl_drv.c b/drivers/gpu/drm/qxl/qxl_drv.c
index 6e0f8a2..74c9af1 100644
--- a/drivers/gpu/drm/qxl/qxl_drv.c
+++ b/drivers/gpu/drm/qxl/qxl_drv.c
@@ -304,7 +304,6 @@ static struct drm_driver qxl_driver = {
 	.dumb_destroy = drm_gem_dumb_destroy,
 #if defined(CONFIG_DEBUG_FS)
 	.debugfs_init = qxl_debugfs_init,
-	.debugfs_cleanup = qxl_debugfs_takedown,
 #endif
 	.prime_handle_to_fd = drm_gem_prime_handle_to_fd,
 	.prime_fd_to_handle = drm_gem_prime_fd_to_handle,
diff --git a/drivers/gpu/drm/qxl/qxl_drv.h b/drivers/gpu/drm/qxl/qxl_drv.h
index 0d877fa..7c6c46a 100644
--- a/drivers/gpu/drm/qxl/qxl_drv.h
+++ b/drivers/gpu/drm/qxl/qxl_drv.h
@@ -531,7 +531,6 @@ int qxl_garbage_collect(struct qxl_device *qdev);
 /* debugfs */
 
 int qxl_debugfs_init(struct drm_minor *minor);
-void qxl_debugfs_takedown(struct drm_minor *minor);
 int qxl_ttm_debugfs_init(struct qxl_device *qdev);
 
 /* qxl_prime.c */
-- 
2.10.2

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 19/19] drm/i915: Remove i915_debugfs_unregister()

[Noralf Trønnes]

drm_debugfs_cleanup() now removes all minor->debugfs_list entries
automatically, so no need to do this explicitly. Additionally it
uses debugfs_remove_recursive() to clean up the debugfs files,
so no need for adding fake drm_info_node entries.

Cc: daniel.vetter@intel.com
Cc: jani.nikula@linux.intel.com
Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
---
 drivers/gpu/drm/i915/i915_debugfs.c   | 97 +++++------------------------------
 drivers/gpu/drm/i915/i915_drv.c       |  1 -
 drivers/gpu/drm/i915/i915_drv.h       |  2 -
 drivers/gpu/drm/i915/intel_drv.h      |  1 -
 drivers/gpu/drm/i915/intel_pipe_crc.c | 68 ++++--------------------
 5 files changed, 23 insertions(+), 146 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index fa69d72..7d72447 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -35,32 +35,6 @@ static inline struct drm_i915_private *node_to_i915(struct drm_info_node *node)
 	return to_i915(node->minor->dev);
 }
 
-/* As the drm_debugfs_init() routines are called before dev->dev_private is
- * allocated we need to hook into the minor for release. */
-static int
-drm_add_fake_info_node(struct drm_minor *minor,
-		       struct dentry *ent,
-		       const void *key)
-{
-	struct drm_info_node *node;
-
-	node = kmalloc(sizeof(*node), GFP_KERNEL);
-	if (node == NULL) {
-		debugfs_remove(ent);
-		return -ENOMEM;
-	}
-
-	node->minor = minor;
-	node->dent = ent;
-	node->info_ent = (void *)key;
-
-	mutex_lock(&minor->debugfs_lock);
-	list_add(&node->list, &minor->debugfs_list);
-	mutex_unlock(&minor->debugfs_lock);
-
-	return 0;
-}
-
 static int i915_capabilities(struct seq_file *m, void *data)
 {
 	struct drm_i915_private *dev_priv = node_to_i915(m->private);
@@ -4593,37 +4567,6 @@ static const struct file_operations i915_forcewake_fops = {
 	.release = i915_forcewake_release,
 };
 
-static int i915_forcewake_create(struct dentry *root, struct drm_minor *minor)
-{
-	struct dentry *ent;
-
-	ent = debugfs_create_file("i915_forcewake_user",
-				  S_IRUSR,
-				  root, to_i915(minor->dev),
-				  &i915_forcewake_fops);
-	if (!ent)
-		return -ENOMEM;
-
-	return drm_add_fake_info_node(minor, ent, &i915_forcewake_fops);
-}
-
-static int i915_debugfs_create(struct dentry *root,
-			       struct drm_minor *minor,
-			       const char *name,
-			       const struct file_operations *fops)
-{
-	struct dentry *ent;
-
-	ent = debugfs_create_file(name,
-				  S_IRUGO | S_IWUSR,
-				  root, to_i915(minor->dev),
-				  fops);
-	if (!ent)
-		return -ENOMEM;
-
-	return drm_add_fake_info_node(minor, ent, fops);
-}
-
 static const struct drm_info_list i915_debugfs_list[] = {
 	{"i915_capabilities", i915_capabilities, 0},
 	{"i915_gem_objects", i915_gem_object_info, 0},
@@ -4706,22 +4649,27 @@ static const struct i915_debugfs_files {
 int i915_debugfs_register(struct drm_i915_private *dev_priv)
 {
 	struct drm_minor *minor = dev_priv->drm.primary;
+	struct dentry *ent;
 	int ret, i;
 
-	ret = i915_forcewake_create(minor->debugfs_root, minor);
-	if (ret)
-		return ret;
+	ent = debugfs_create_file("i915_forcewake_user", S_IRUSR,
+				  minor->debugfs_root, to_i915(minor->dev),
+				  &i915_forcewake_fops);
+	if (!ent)
+		return -ENOMEM;
 
 	ret = intel_pipe_crc_create(minor);
 	if (ret)
 		return ret;
 
 	for (i = 0; i < ARRAY_SIZE(i915_debugfs_files); i++) {
-		ret = i915_debugfs_create(minor->debugfs_root, minor,
-					  i915_debugfs_files[i].name,
+		ent = debugfs_create_file(i915_debugfs_files[i].name,
+					  S_IRUGO | S_IWUSR,
+					  minor->debugfs_root,
+					  to_i915(minor->dev),
 					  i915_debugfs_files[i].fops);
-		if (ret)
-			return ret;
+		if (!ent)
+			return -ENOMEM;
 	}
 
 	return drm_debugfs_create_files(i915_debugfs_list,
@@ -4729,27 +4677,6 @@ int i915_debugfs_register(struct drm_i915_private *dev_priv)
 					minor->debugfs_root, minor);
 }
 
-void i915_debugfs_unregister(struct drm_i915_private *dev_priv)
-{
-	struct drm_minor *minor = dev_priv->drm.primary;
-	int i;
-
-	drm_debugfs_remove_files(i915_debugfs_list,
-				 I915_DEBUGFS_ENTRIES, minor);
-
-	drm_debugfs_remove_files((struct drm_info_list *)&i915_forcewake_fops,
-				 1, minor);
-
-	intel_pipe_crc_cleanup(minor);
-
-	for (i = 0; i < ARRAY_SIZE(i915_debugfs_files); i++) {
-		struct drm_info_list *info_list =
-			(struct drm_info_list *)i915_debugfs_files[i].fops;
-
-		drm_debugfs_remove_files(info_list, 1, minor);
-	}
-}
-
 struct dpcd_block {
 	/* DPCD dump start address. */
 	unsigned int offset;
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 4ae69eb..8e78822 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -1168,7 +1168,6 @@ static void i915_driver_unregister(struct drm_i915_private *dev_priv)
 
 	i915_teardown_sysfs(dev_priv);
 	i915_guc_log_unregister(dev_priv);
-	i915_debugfs_unregister(dev_priv);
 	drm_dev_unregister(&dev_priv->drm);
 
 	i915_gem_shrinker_cleanup(dev_priv);
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 322e8c9..608f143 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -3517,12 +3517,10 @@ u32 i915_gem_fence_alignment(struct drm_i915_private *dev_priv, u32 size,
 /* i915_debugfs.c */
 #ifdef CONFIG_DEBUG_FS
 int i915_debugfs_register(struct drm_i915_private *dev_priv);
-void i915_debugfs_unregister(struct drm_i915_private *dev_priv);
 int i915_debugfs_connector_add(struct drm_connector *connector);
 void intel_display_crc_init(struct drm_i915_private *dev_priv);
 #else
 static inline int i915_debugfs_register(struct drm_i915_private *dev_priv) {return 0;}
-static inline void i915_debugfs_unregister(struct drm_i915_private *dev_priv) {}
 static inline int i915_debugfs_connector_add(struct drm_connector *connector)
 { return 0; }
 static inline void intel_display_crc_init(struct drm_i915_private *dev_priv) {}
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 0cec001..0411be5 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1884,7 +1884,6 @@ void lspcon_wait_pcon_mode(struct intel_lspcon *lspcon);
 
 /* intel_pipe_crc.c */
 int intel_pipe_crc_create(struct drm_minor *minor);
-void intel_pipe_crc_cleanup(struct drm_minor *minor);
 #ifdef CONFIG_DEBUG_FS
 int intel_crtc_set_crc_source(struct drm_crtc *crtc, const char *source_name,
 			      size_t *values_cnt);
diff --git a/drivers/gpu/drm/i915/intel_pipe_crc.c b/drivers/gpu/drm/i915/intel_pipe_crc.c
index c0b1f99..5aa524e 100644
--- a/drivers/gpu/drm/i915/intel_pipe_crc.c
+++ b/drivers/gpu/drm/i915/intel_pipe_crc.c
@@ -36,31 +36,6 @@ struct pipe_crc_info {
 	enum pipe pipe;
 };
 
-/* As the drm_debugfs_init() routines are called before dev->dev_private is
- * allocated we need to hook into the minor for release.
- */
-static int drm_add_fake_info_node(struct drm_minor *minor,
-				  struct dentry *ent, const void *key)
-{
-	struct drm_info_node *node;
-
-	node = kmalloc(sizeof(*node), GFP_KERNEL);
-	if (node == NULL) {
-		debugfs_remove(ent);
-		return -ENOMEM;
-	}
-
-	node->minor = minor;
-	node->dent = ent;
-	node->info_ent = (void *) key;
-
-	mutex_lock(&minor->debugfs_lock);
-	list_add(&node->list, &minor->debugfs_list);
-	mutex_unlock(&minor->debugfs_lock);
-
-	return 0;
-}
-
 static int i915_pipe_crc_open(struct inode *inode, struct file *filep)
 {
 	struct pipe_crc_info *info = inode->i_private;
@@ -209,22 +184,6 @@ static struct pipe_crc_info i915_pipe_crc_data[I915_MAX_PIPES] = {
 	},
 };
 
-static int i915_pipe_crc_create(struct dentry *root, struct drm_minor *minor,
-				enum pipe pipe)
-{
-	struct drm_i915_private *dev_priv = to_i915(minor->dev);
-	struct dentry *ent;
-	struct pipe_crc_info *info = &i915_pipe_crc_data[pipe];
-
-	info->dev_priv = dev_priv;
-	ent = debugfs_create_file(info->name, S_IRUGO, root, info,
-				  &i915_pipe_crc_fops);
-	if (!ent)
-		return -ENOMEM;
-
-	return drm_add_fake_info_node(minor, ent, info);
-}
-
 static const char * const pipe_crc_sources[] = {
 	"none",
 	"plane1",
@@ -928,27 +887,22 @@ void intel_display_crc_init(struct drm_i915_private *dev_priv)
 
 int intel_pipe_crc_create(struct drm_minor *minor)
 {
-	int ret, i;
-
-	for (i = 0; i < ARRAY_SIZE(i915_pipe_crc_data); i++) {
-		ret = i915_pipe_crc_create(minor->debugfs_root, minor, i);
-		if (ret)
-			return ret;
-	}
-
-	return 0;
-}
-
-void intel_pipe_crc_cleanup(struct drm_minor *minor)
-{
+	struct drm_i915_private *dev_priv = to_i915(minor->dev);
+	struct dentry *ent;
 	int i;
 
 	for (i = 0; i < ARRAY_SIZE(i915_pipe_crc_data); i++) {
-		struct drm_info_list *info_list =
-			(struct drm_info_list *)&i915_pipe_crc_data[i];
+		struct pipe_crc_info *info = &i915_pipe_crc_data[i];
 
-		drm_debugfs_remove_files(info_list, 1, minor);
+		info->dev_priv = dev_priv;
+		ent = debugfs_create_file(info->name, S_IRUGO,
+					  minor->debugfs_root, info,
+					  &i915_pipe_crc_fops);
+		if (!ent)
+			return -ENOMEM;
 	}
+
+	return 0;
 }
 
 int intel_crtc_set_crc_source(struct drm_crtc *crtc, const char *source_name,
-- 
2.10.2

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 00/19] drm: debugfs: Remove all files automatically on cleanup

[Daniel Vetter]

On Thu, Jan 26, 2017 at 11:56:02PM +0100, Noralf Trønnes wrote:
> This patchset removes the need for drivers to clean up their debugfs
> files on exit. It is done automatically in drm_debugfs_cleanup().
> This funtion is also called should the driver error out in it's
> drm_driver.debugfs_init callback.
> 
> Two drivers still use drm_debugfs_remove_files():
> - tegra in it's connectors, not sure if I can remove it.

I read through them, and they're removed on the component device nodes
stuff. That looks somewhat fishy from a lifetime point of view, and I
think removing all that code would be better, too.

> - qxl because it's debugfs files list is part of struct qxl_device which
>   is freed on unload before drm_minor_unregister() is called cleaning debugfs.

In -next qxl is already demidlayered and there's no longer an unload hook.
This should be all safe ... why is it not?

> Daniel,
> I was unable to remove the drm_driver.debugfs_cleanup hook completely,
> since drm/msm uses it to free memory. Maybe it can be freed elsewhere.

Well this is definitely a massive step into a good direction, great work.
For msm I think we can just move the two calls left in msm_debugfs_cleanup
in msm_drm_uninit, right after the call to drm_dev_unregister.
-Daniel

> 
> 
> Note:
> The driver patches are only compile tested.
> 
> 
> Noralf.
> 
> 
> Noralf Trønnes (19):
>   drm: debugfs: Remove all files automatically on cleanup
>   drm: drm_minor_register(): Clean up debugfs on failure
>   drm/atomic: Remove drm_atomic_debugfs_cleanup()
>   drm/amd/amdgpu: Remove drm_debugfs_remove_files() call
>   drm/armada: Remove armada_drm_debugfs_cleanup()
>   drm/etnaviv: allow build with COMPILE_TEST
>   drm/etnaviv: Remove etnaviv_debugfs_cleanup()
>   drm/hdlcd: Remove hdlcd_debugfs_cleanup()
>   drm/msm: Remove drm_debugfs_remove_files() calls
>   drm/nouveau: Remove nouveau_drm_debugfs_cleanup()
>   drm/omap: Remove omap_debugfs_cleanup()
>   drm/radeon: Remove drm_debugfs_remove_files() call
>   drm/sti: Remove drm_debugfs_remove_files() calls
>   drm/tegra: Remove tegra_debugfs_cleanup()
>   drm/tilcdc: Remove tilcdc_debugfs_cleanup()
>   drm/vc4: Remove vc4_debugfs_cleanup()
>   drm/virtio: Remove virtio_gpu_debugfs_takedown()
>   drm/qxl: Remove qxl_debugfs_takedown()
>   drm/i915: Remove i915_debugfs_unregister()
> 
>  drivers/gpu/drm/amd/amdgpu/amdgpu.h        |  1 -
>  drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 20 ------
>  drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c    |  1 -
>  drivers/gpu/drm/arm/hdlcd_drv.c            |  7 ---
>  drivers/gpu/drm/armada/armada_debugfs.c    | 65 +++-----------------
>  drivers/gpu/drm/armada/armada_drm.h        |  1 -
>  drivers/gpu/drm/armada/armada_drv.c        |  3 -
>  drivers/gpu/drm/drm_atomic.c               |  7 ---
>  drivers/gpu/drm/drm_crtc_internal.h        |  1 -
>  drivers/gpu/drm/drm_debugfs.c              | 29 +++++----
>  drivers/gpu/drm/drm_drv.c                  |  2 +-
>  drivers/gpu/drm/etnaviv/Kconfig            |  2 +-
>  drivers/gpu/drm/etnaviv/etnaviv_drv.c      |  7 ---
>  drivers/gpu/drm/i915/i915_debugfs.c        | 97 ++++--------------------------
>  drivers/gpu/drm/i915/i915_drv.c            |  1 -
>  drivers/gpu/drm/i915/i915_drv.h            |  2 -
>  drivers/gpu/drm/i915/intel_drv.h           |  1 -
>  drivers/gpu/drm/i915/intel_pipe_crc.c      | 68 ++++-----------------
>  drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.c    |  7 ---
>  drivers/gpu/drm/msm/msm_debugfs.c          |  2 -
>  drivers/gpu/drm/msm/msm_perf.c             | 29 +--------
>  drivers/gpu/drm/msm/msm_rd.c               | 31 +---------
>  drivers/gpu/drm/nouveau/nouveau_debugfs.c  | 62 ++++---------------
>  drivers/gpu/drm/nouveau/nouveau_debugfs.h  |  6 --
>  drivers/gpu/drm/nouveau/nouveau_drm.c      |  1 -
>  drivers/gpu/drm/omapdrm/omap_debugfs.c     |  9 ---
>  drivers/gpu/drm/omapdrm/omap_drv.c         |  1 -
>  drivers/gpu/drm/omapdrm/omap_drv.h         |  1 -
>  drivers/gpu/drm/qxl/qxl_debugfs.c          |  9 ---
>  drivers/gpu/drm/qxl/qxl_drv.c              |  1 -
>  drivers/gpu/drm/qxl/qxl_drv.h              |  1 -
>  drivers/gpu/drm/radeon/radeon_device.c     | 16 -----
>  drivers/gpu/drm/sti/sti_drv.c              | 48 ++-------------
>  drivers/gpu/drm/sti/sti_dvo.c              | 10 ---
>  drivers/gpu/drm/sti/sti_hda.c              | 11 ----
>  drivers/gpu/drm/sti/sti_hdmi.c             | 11 ----
>  drivers/gpu/drm/sti/sti_tvout.c            |  8 ---
>  drivers/gpu/drm/tegra/drm.c                |  7 ---
>  drivers/gpu/drm/tilcdc/tilcdc_drv.c        | 12 ----
>  drivers/gpu/drm/tilcdc/tilcdc_drv.h        |  2 -
>  drivers/gpu/drm/vc4/vc4_debugfs.c          |  6 --
>  drivers/gpu/drm/vc4/vc4_drv.c              |  1 -
>  drivers/gpu/drm/vc4/vc4_drv.h              |  1 -
>  drivers/gpu/drm/virtio/virtgpu_debugfs.c   |  8 ---
>  drivers/gpu/drm/virtio/virtgpu_drv.c       |  1 -
>  drivers/gpu/drm/virtio/virtgpu_drv.h       |  1 -
>  46 files changed, 76 insertions(+), 542 deletions(-)
> 
> --
> 2.10.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
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 00/19] drm: debugfs: Remove all files automatically on cleanup

[Thierry Reding]

[-- Attachment #1.1: Type: text/plain, Size: 1227 bytes --]

On Fri, Jan 27, 2017 at 08:49:34AM +0100, Daniel Vetter wrote:
> On Thu, Jan 26, 2017 at 11:56:02PM +0100, Noralf Trønnes wrote:
> > This patchset removes the need for drivers to clean up their debugfs
> > files on exit. It is done automatically in drm_debugfs_cleanup().
> > This funtion is also called should the driver error out in it's
> > drm_driver.debugfs_init callback.
> > 
> > Two drivers still use drm_debugfs_remove_files():
> > - tegra in it's connectors, not sure if I can remove it.
> 
> I read through them, and they're removed on the component device nodes
> stuff. That looks somewhat fishy from a lifetime point of view, and I
> think removing all that code would be better, too.

What makes you think that's problematic from a lifetime point of view?
The component device is tied to the DRM device, so these callbacks are
called at the right time.

That said, I think it's safe to remove the other debugfs files from
Tegra. It might not be possible to remove the cleanup functions
altogether, though, because they have to do a special dance involving
kmemdup() drm_debugfs_create_files() and kfree() in order to support
debugfs files for multiple instances of subdevices.

Thierry

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 00/19] drm: debugfs: Remove all files automatically on cleanup

[Daniel Vetter]

On Fri, Jan 27, 2017 at 10:36:16AM +0100, Thierry Reding wrote:
> On Fri, Jan 27, 2017 at 08:49:34AM +0100, Daniel Vetter wrote:
> > On Thu, Jan 26, 2017 at 11:56:02PM +0100, Noralf Trønnes wrote:
> > > This patchset removes the need for drivers to clean up their debugfs
> > > files on exit. It is done automatically in drm_debugfs_cleanup().
> > > This funtion is also called should the driver error out in it's
> > > drm_driver.debugfs_init callback.
> > > 
> > > Two drivers still use drm_debugfs_remove_files():
> > > - tegra in it's connectors, not sure if I can remove it.
> > 
> > I read through them, and they're removed on the component device nodes
> > stuff. That looks somewhat fishy from a lifetime point of view, and I
> > think removing all that code would be better, too.
> 
> What makes you think that's problematic from a lifetime point of view?
> The component device is tied to the DRM device, so these callbacks are
> called at the right time.

debugfs is a userspace interface, which should disappear when
drm_dev_unregister gets called. I'm not sure at all whether that lines up
with the cleanup of all your component nodes, but otoh it's rather
academic since you can't hotplug a tegra.

> That said, I think it's safe to remove the other debugfs files from
> Tegra. It might not be possible to remove the cleanup functions
> altogether, though, because they have to do a special dance involving
> kmemdup() drm_debugfs_create_files() and kfree() in order to support
> debugfs files for multiple instances of subdevices.

Hm, that entire "do debugfs on the minor" thing makes almost never sense.
All the things we have left in modern drivers are either per-fd, or
per-device. Nothing of interest is per-minor. Or do you mean something
else?

Either way would be nice if we can remove them too, so that
drm_debugfs_remove_files could be unexported ...
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 00/19] drm: debugfs: Remove all files automatically on cleanup

[Thierry Reding]

[-- Attachment #1.1: Type: text/plain, Size: 2147 bytes --]

On Fri, Jan 27, 2017 at 03:05:46PM +0100, Daniel Vetter wrote:
> On Fri, Jan 27, 2017 at 10:36:16AM +0100, Thierry Reding wrote:
> > On Fri, Jan 27, 2017 at 08:49:34AM +0100, Daniel Vetter wrote:
> > > On Thu, Jan 26, 2017 at 11:56:02PM +0100, Noralf Trønnes wrote:
> > > > This patchset removes the need for drivers to clean up their debugfs
> > > > files on exit. It is done automatically in drm_debugfs_cleanup().
> > > > This funtion is also called should the driver error out in it's
> > > > drm_driver.debugfs_init callback.
> > > > 
> > > > Two drivers still use drm_debugfs_remove_files():
> > > > - tegra in it's connectors, not sure if I can remove it.
> > > 
> > > I read through them, and they're removed on the component device nodes
> > > stuff. That looks somewhat fishy from a lifetime point of view, and I
> > > think removing all that code would be better, too.
> > 
> > What makes you think that's problematic from a lifetime point of view?
> > The component device is tied to the DRM device, so these callbacks are
> > called at the right time.
> 
> debugfs is a userspace interface, which should disappear when
> drm_dev_unregister gets called. I'm not sure at all whether that lines up
> with the cleanup of all your component nodes, but otoh it's rather
> academic since you can't hotplug a tegra.
> 
> > That said, I think it's safe to remove the other debugfs files from
> > Tegra. It might not be possible to remove the cleanup functions
> > altogether, though, because they have to do a special dance involving
> > kmemdup() drm_debugfs_create_files() and kfree() in order to support
> > debugfs files for multiple instances of subdevices.
> 
> Hm, that entire "do debugfs on the minor" thing makes almost never sense.
> All the things we have left in modern drivers are either per-fd, or
> per-device. Nothing of interest is per-minor. Or do you mean something
> else?

I'm not sure I understand what you're saying. We have plenty of code
that adds debugfs files to the connector's debugfs entry. And that's
within the minor's debugfs root.

Am I missing something?

Thierry

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 00/19] drm: debugfs: Remove all files automatically on cleanup

[Daniel Vetter]

On Mon, Jan 30, 2017 at 09:58:48AM +0100, Thierry Reding wrote:
> On Fri, Jan 27, 2017 at 03:05:46PM +0100, Daniel Vetter wrote:
> > On Fri, Jan 27, 2017 at 10:36:16AM +0100, Thierry Reding wrote:
> > > On Fri, Jan 27, 2017 at 08:49:34AM +0100, Daniel Vetter wrote:
> > > > On Thu, Jan 26, 2017 at 11:56:02PM +0100, Noralf Trønnes wrote:
> > > > > This patchset removes the need for drivers to clean up their debugfs
> > > > > files on exit. It is done automatically in drm_debugfs_cleanup().
> > > > > This funtion is also called should the driver error out in it's
> > > > > drm_driver.debugfs_init callback.
> > > > > 
> > > > > Two drivers still use drm_debugfs_remove_files():
> > > > > - tegra in it's connectors, not sure if I can remove it.
> > > > 
> > > > I read through them, and they're removed on the component device nodes
> > > > stuff. That looks somewhat fishy from a lifetime point of view, and I
> > > > think removing all that code would be better, too.
> > > 
> > > What makes you think that's problematic from a lifetime point of view?
> > > The component device is tied to the DRM device, so these callbacks are
> > > called at the right time.
> > 
> > debugfs is a userspace interface, which should disappear when
> > drm_dev_unregister gets called. I'm not sure at all whether that lines up
> > with the cleanup of all your component nodes, but otoh it's rather
> > academic since you can't hotplug a tegra.
> > 
> > > That said, I think it's safe to remove the other debugfs files from
> > > Tegra. It might not be possible to remove the cleanup functions
> > > altogether, though, because they have to do a special dance involving
> > > kmemdup() drm_debugfs_create_files() and kfree() in order to support
> > > debugfs files for multiple instances of subdevices.
> > 
> > Hm, that entire "do debugfs on the minor" thing makes almost never sense.
> > All the things we have left in modern drivers are either per-fd, or
> > per-device. Nothing of interest is per-minor. Or do you mean something
> > else?
> 
> I'm not sure I understand what you're saying. We have plenty of code
> that adds debugfs files to the connector's debugfs entry. And that's
> within the minor's debugfs root.
> 
> Am I missing something?

Per-connector entries are fine, per-minor imo not. This is a historical
accident, but it also doesn't really hurt anyone. I think it'd make much
more sense to move everything into a per-devices entry (with maybe
backwards compat links from minor to devices). But really, this is 100%
orthogonal to the cleanup here.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 00/19] drm: debugfs: Remove all files automatically on cleanup

[Thierry Reding]

[-- Attachment #1.1: Type: text/plain, Size: 3375 bytes --]

On Mon, Jan 30, 2017 at 10:03:44AM +0100, Daniel Vetter wrote:
> On Mon, Jan 30, 2017 at 09:58:48AM +0100, Thierry Reding wrote:
> > On Fri, Jan 27, 2017 at 03:05:46PM +0100, Daniel Vetter wrote:
> > > On Fri, Jan 27, 2017 at 10:36:16AM +0100, Thierry Reding wrote:
> > > > On Fri, Jan 27, 2017 at 08:49:34AM +0100, Daniel Vetter wrote:
> > > > > On Thu, Jan 26, 2017 at 11:56:02PM +0100, Noralf Trønnes wrote:
> > > > > > This patchset removes the need for drivers to clean up their debugfs
> > > > > > files on exit. It is done automatically in drm_debugfs_cleanup().
> > > > > > This funtion is also called should the driver error out in it's
> > > > > > drm_driver.debugfs_init callback.
> > > > > > 
> > > > > > Two drivers still use drm_debugfs_remove_files():
> > > > > > - tegra in it's connectors, not sure if I can remove it.
> > > > > 
> > > > > I read through them, and they're removed on the component device nodes
> > > > > stuff. That looks somewhat fishy from a lifetime point of view, and I
> > > > > think removing all that code would be better, too.
> > > > 
> > > > What makes you think that's problematic from a lifetime point of view?
> > > > The component device is tied to the DRM device, so these callbacks are
> > > > called at the right time.
> > > 
> > > debugfs is a userspace interface, which should disappear when
> > > drm_dev_unregister gets called. I'm not sure at all whether that lines up
> > > with the cleanup of all your component nodes, but otoh it's rather
> > > academic since you can't hotplug a tegra.
> > > 
> > > > That said, I think it's safe to remove the other debugfs files from
> > > > Tegra. It might not be possible to remove the cleanup functions
> > > > altogether, though, because they have to do a special dance involving
> > > > kmemdup() drm_debugfs_create_files() and kfree() in order to support
> > > > debugfs files for multiple instances of subdevices.
> > > 
> > > Hm, that entire "do debugfs on the minor" thing makes almost never sense.
> > > All the things we have left in modern drivers are either per-fd, or
> > > per-device. Nothing of interest is per-minor. Or do you mean something
> > > else?
> > 
> > I'm not sure I understand what you're saying. We have plenty of code
> > that adds debugfs files to the connector's debugfs entry. And that's
> > within the minor's debugfs root.
> > 
> > Am I missing something?
> 
> Per-connector entries are fine, per-minor imo not.

Most, if not all, debugfs files in Tegra a per-connector. We have a
couple that are per-CRTC. And then we have two files that are on the
minor, which is something I had copied from i915, if I remember
correctly, though I can't seem to find the original anymore. Maybe
that was moved somewhere else in the meantime?

> This is a historical accident, but it also doesn't really hurt anyone.
> I think it'd make much more sense to move everything into a
> per-devices entry (with maybe backwards compat links from minor to
> devices).

With per-device entries you mean rooted at the device backing the CRTC,
encoder, connector, ...?

> But really, this is 100% orthogonal to the cleanup here.

If we want to get rid of the remainder of the cleanup, then it's not
entirely orthogonal anymore. =)

Not to say that this cleanup isn't useful in its own right.

Thierry

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 00/19] drm: debugfs: Remove all files automatically on cleanup

[Daniel Vetter]

On Mon, Jan 30, 2017 at 10:10:15AM +0100, Thierry Reding wrote:
> On Mon, Jan 30, 2017 at 10:03:44AM +0100, Daniel Vetter wrote:
> > On Mon, Jan 30, 2017 at 09:58:48AM +0100, Thierry Reding wrote:
> > > On Fri, Jan 27, 2017 at 03:05:46PM +0100, Daniel Vetter wrote:
> > > > On Fri, Jan 27, 2017 at 10:36:16AM +0100, Thierry Reding wrote:
> > > > > On Fri, Jan 27, 2017 at 08:49:34AM +0100, Daniel Vetter wrote:
> > > > > > On Thu, Jan 26, 2017 at 11:56:02PM +0100, Noralf Trønnes wrote:
> > > > > > > This patchset removes the need for drivers to clean up their debugfs
> > > > > > > files on exit. It is done automatically in drm_debugfs_cleanup().
> > > > > > > This funtion is also called should the driver error out in it's
> > > > > > > drm_driver.debugfs_init callback.
> > > > > > > 
> > > > > > > Two drivers still use drm_debugfs_remove_files():
> > > > > > > - tegra in it's connectors, not sure if I can remove it.
> > > > > > 
> > > > > > I read through them, and they're removed on the component device nodes
> > > > > > stuff. That looks somewhat fishy from a lifetime point of view, and I
> > > > > > think removing all that code would be better, too.
> > > > > 
> > > > > What makes you think that's problematic from a lifetime point of view?
> > > > > The component device is tied to the DRM device, so these callbacks are
> > > > > called at the right time.
> > > > 
> > > > debugfs is a userspace interface, which should disappear when
> > > > drm_dev_unregister gets called. I'm not sure at all whether that lines up
> > > > with the cleanup of all your component nodes, but otoh it's rather
> > > > academic since you can't hotplug a tegra.
> > > > 
> > > > > That said, I think it's safe to remove the other debugfs files from
> > > > > Tegra. It might not be possible to remove the cleanup functions
> > > > > altogether, though, because they have to do a special dance involving
> > > > > kmemdup() drm_debugfs_create_files() and kfree() in order to support
> > > > > debugfs files for multiple instances of subdevices.
> > > > 
> > > > Hm, that entire "do debugfs on the minor" thing makes almost never sense.
> > > > All the things we have left in modern drivers are either per-fd, or
> > > > per-device. Nothing of interest is per-minor. Or do you mean something
> > > > else?
> > > 
> > > I'm not sure I understand what you're saying. We have plenty of code
> > > that adds debugfs files to the connector's debugfs entry. And that's
> > > within the minor's debugfs root.
> > > 
> > > Am I missing something?
> > 
> > Per-connector entries are fine, per-minor imo not.
> 
> Most, if not all, debugfs files in Tegra a per-connector. We have a
> couple that are per-CRTC. And then we have two files that are on the
> minor, which is something I had copied from i915, if I remember
> correctly, though I can't seem to find the original anymore. Maybe
> that was moved somewhere else in the meantime?

All the code I've found in tegra about debugfs is per drm_minor. Some of
it create subdirectories within that, but nothing uses the crtc and
connector ->debugfs_root stuff (which is only in the primary drm_minor
debugfs directory).

> > This is a historical accident, but it also doesn't really hurt anyone.
> > I think it'd make much more sense to move everything into a
> > per-devices entry (with maybe backwards compat links from minor to
> > devices).
> 
> With per-device entries you mean rooted at the device backing the CRTC,
> encoder, connector, ...?

I didn't see any drm support for encoders, no idea what you mean.

> > But really, this is 100% orthogonal to the cleanup here.
> 
> If we want to get rid of the remainder of the cleanup, then it's not
> entirely orthogonal anymore. =)
> 
> Not to say that this cleanup isn't useful in its own right.

Well, looking at this a bit more we might go even further by using
debugfs_remove_recursive(), then we could remove even the tegra stuff. Atm
I think that's not doable because tegra creates its own subdirectories in
drm_minor->debugfs_root. I guess that's a mess for a different day though.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 00/19] drm: debugfs: Remove all files automatically on cleanup

[Noralf Trønnes]

Den 27.01.2017 08.49, skrev Daniel Vetter:
> On Thu, Jan 26, 2017 at 11:56:02PM +0100, Noralf Trønnes wrote:
>> This patchset removes the need for drivers to clean up their debugfs
>> files on exit. It is done automatically in drm_debugfs_cleanup().
>> This funtion is also called should the driver error out in it's
>> drm_driver.debugfs_init callback.
>>
>> Two drivers still use drm_debugfs_remove_files():
>> - tegra in it's connectors, not sure if I can remove it.
> I read through them, and they're removed on the component device nodes
> stuff. That looks somewhat fishy from a lifetime point of view, and I
> think removing all that code would be better, too.
>
>> - qxl because it's debugfs files list is part of struct qxl_device which
>>    is freed on unload before drm_minor_unregister() is called cleaning debugfs.
> In -next qxl is already demidlayered and there's no longer an unload hook.
> This should be all safe ... why is it not?

My bad, I fetched linux-next a few days ago and figured it was
up-to-date-enough. Duh, I should have known better after following drm for
a year now, it is constantly changing, no pauses.

Can you please ping me when you have pulled driver patches and I'll respin
msm, tegra and qxl (and others if necessary), and remove the hook.
It's much easier for me to do a small patchset, it's really a strain to get
everything lined up correctly with big changes. I didn't have that patch
juggling class when in school, so I'm late to the game :-)


Noralf.

>> Daniel,
>> I was unable to remove the drm_driver.debugfs_cleanup hook completely,
>> since drm/msm uses it to free memory. Maybe it can be freed elsewhere.
> Well this is definitely a massive step into a good direction, great work.
> For msm I think we can just move the two calls left in msm_debugfs_cleanup
> in msm_drm_uninit, right after the call to drm_dev_unregister.
> -Daniel
>
>>
>> Note:
>> The driver patches are only compile tested.
>>
>>
>> Noralf.
>>
>>
>> Noralf Trønnes (19):
>>    drm: debugfs: Remove all files automatically on cleanup
>>    drm: drm_minor_register(): Clean up debugfs on failure
>>    drm/atomic: Remove drm_atomic_debugfs_cleanup()
>>    drm/amd/amdgpu: Remove drm_debugfs_remove_files() call
>>    drm/armada: Remove armada_drm_debugfs_cleanup()
>>    drm/etnaviv: allow build with COMPILE_TEST
>>    drm/etnaviv: Remove etnaviv_debugfs_cleanup()
>>    drm/hdlcd: Remove hdlcd_debugfs_cleanup()
>>    drm/msm: Remove drm_debugfs_remove_files() calls
>>    drm/nouveau: Remove nouveau_drm_debugfs_cleanup()
>>    drm/omap: Remove omap_debugfs_cleanup()
>>    drm/radeon: Remove drm_debugfs_remove_files() call
>>    drm/sti: Remove drm_debugfs_remove_files() calls
>>    drm/tegra: Remove tegra_debugfs_cleanup()
>>    drm/tilcdc: Remove tilcdc_debugfs_cleanup()
>>    drm/vc4: Remove vc4_debugfs_cleanup()
>>    drm/virtio: Remove virtio_gpu_debugfs_takedown()
>>    drm/qxl: Remove qxl_debugfs_takedown()
>>    drm/i915: Remove i915_debugfs_unregister()
>>
>>   drivers/gpu/drm/amd/amdgpu/amdgpu.h        |  1 -
>>   drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 20 ------
>>   drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c    |  1 -
>>   drivers/gpu/drm/arm/hdlcd_drv.c            |  7 ---
>>   drivers/gpu/drm/armada/armada_debugfs.c    | 65 +++-----------------
>>   drivers/gpu/drm/armada/armada_drm.h        |  1 -
>>   drivers/gpu/drm/armada/armada_drv.c        |  3 -
>>   drivers/gpu/drm/drm_atomic.c               |  7 ---
>>   drivers/gpu/drm/drm_crtc_internal.h        |  1 -
>>   drivers/gpu/drm/drm_debugfs.c              | 29 +++++----
>>   drivers/gpu/drm/drm_drv.c                  |  2 +-
>>   drivers/gpu/drm/etnaviv/Kconfig            |  2 +-
>>   drivers/gpu/drm/etnaviv/etnaviv_drv.c      |  7 ---
>>   drivers/gpu/drm/i915/i915_debugfs.c        | 97 ++++--------------------------
>>   drivers/gpu/drm/i915/i915_drv.c            |  1 -
>>   drivers/gpu/drm/i915/i915_drv.h            |  2 -
>>   drivers/gpu/drm/i915/intel_drv.h           |  1 -
>>   drivers/gpu/drm/i915/intel_pipe_crc.c      | 68 ++++-----------------
>>   drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.c    |  7 ---
>>   drivers/gpu/drm/msm/msm_debugfs.c          |  2 -
>>   drivers/gpu/drm/msm/msm_perf.c             | 29 +--------
>>   drivers/gpu/drm/msm/msm_rd.c               | 31 +---------
>>   drivers/gpu/drm/nouveau/nouveau_debugfs.c  | 62 ++++---------------
>>   drivers/gpu/drm/nouveau/nouveau_debugfs.h  |  6 --
>>   drivers/gpu/drm/nouveau/nouveau_drm.c      |  1 -
>>   drivers/gpu/drm/omapdrm/omap_debugfs.c     |  9 ---
>>   drivers/gpu/drm/omapdrm/omap_drv.c         |  1 -
>>   drivers/gpu/drm/omapdrm/omap_drv.h         |  1 -
>>   drivers/gpu/drm/qxl/qxl_debugfs.c          |  9 ---
>>   drivers/gpu/drm/qxl/qxl_drv.c              |  1 -
>>   drivers/gpu/drm/qxl/qxl_drv.h              |  1 -
>>   drivers/gpu/drm/radeon/radeon_device.c     | 16 -----
>>   drivers/gpu/drm/sti/sti_drv.c              | 48 ++-------------
>>   drivers/gpu/drm/sti/sti_dvo.c              | 10 ---
>>   drivers/gpu/drm/sti/sti_hda.c              | 11 ----
>>   drivers/gpu/drm/sti/sti_hdmi.c             | 11 ----
>>   drivers/gpu/drm/sti/sti_tvout.c            |  8 ---
>>   drivers/gpu/drm/tegra/drm.c                |  7 ---
>>   drivers/gpu/drm/tilcdc/tilcdc_drv.c        | 12 ----
>>   drivers/gpu/drm/tilcdc/tilcdc_drv.h        |  2 -
>>   drivers/gpu/drm/vc4/vc4_debugfs.c          |  6 --
>>   drivers/gpu/drm/vc4/vc4_drv.c              |  1 -
>>   drivers/gpu/drm/vc4/vc4_drv.h              |  1 -
>>   drivers/gpu/drm/virtio/virtgpu_debugfs.c   |  8 ---
>>   drivers/gpu/drm/virtio/virtgpu_drv.c       |  1 -
>>   drivers/gpu/drm/virtio/virtgpu_drv.h       |  1 -
>>   46 files changed, 76 insertions(+), 542 deletions(-)
>>
>> --
>> 2.10.2
>>
>> _______________________________________________
>> dri-devel mailing list
>> dri-devel@lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/dri-devel

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 00/19] drm: debugfs: Remove all files automatically on cleanup

[Daniel Vetter]

On Fri, Jan 27, 2017 at 03:23:43PM +0100, Noralf Trønnes wrote:
> 
> Den 27.01.2017 08.49, skrev Daniel Vetter:
> > On Thu, Jan 26, 2017 at 11:56:02PM +0100, Noralf Trønnes wrote:
> > > This patchset removes the need for drivers to clean up their debugfs
> > > files on exit. It is done automatically in drm_debugfs_cleanup().
> > > This funtion is also called should the driver error out in it's
> > > drm_driver.debugfs_init callback.
> > > 
> > > Two drivers still use drm_debugfs_remove_files():
> > > - tegra in it's connectors, not sure if I can remove it.
> > I read through them, and they're removed on the component device nodes
> > stuff. That looks somewhat fishy from a lifetime point of view, and I
> > think removing all that code would be better, too.
> > 
> > > - qxl because it's debugfs files list is part of struct qxl_device which
> > >    is freed on unload before drm_minor_unregister() is called cleaning debugfs.
> > In -next qxl is already demidlayered and there's no longer an unload hook.
> > This should be all safe ... why is it not?
> 
> My bad, I fetched linux-next a few days ago and figured it was
> up-to-date-enough. Duh, I should have known better after following drm for
> a year now, it is constantly changing, no pauses.
> 
> Can you please ping me when you have pulled driver patches and I'll respin
> msm, tegra and qxl (and others if necessary), and remove the hook.
> It's much easier for me to do a small patchset, it's really a strain to get
> everything lined up correctly with big changes. I didn't have that patch
> juggling class when in school, so I'm late to the game :-)

You're doing great with the patch juggling :-) I've just made a pass
through the series and merge what's already reviewed/acked.

Thanks, Daniel

> 
> 
> Noralf.
> 
> > > Daniel,
> > > I was unable to remove the drm_driver.debugfs_cleanup hook completely,
> > > since drm/msm uses it to free memory. Maybe it can be freed elsewhere.
> > Well this is definitely a massive step into a good direction, great work.
> > For msm I think we can just move the two calls left in msm_debugfs_cleanup
> > in msm_drm_uninit, right after the call to drm_dev_unregister.
> > -Daniel
> > 
> > > 
> > > Note:
> > > The driver patches are only compile tested.
> > > 
> > > 
> > > Noralf.
> > > 
> > > 
> > > Noralf Trønnes (19):
> > >    drm: debugfs: Remove all files automatically on cleanup
> > >    drm: drm_minor_register(): Clean up debugfs on failure
> > >    drm/atomic: Remove drm_atomic_debugfs_cleanup()
> > >    drm/amd/amdgpu: Remove drm_debugfs_remove_files() call
> > >    drm/armada: Remove armada_drm_debugfs_cleanup()
> > >    drm/etnaviv: allow build with COMPILE_TEST
> > >    drm/etnaviv: Remove etnaviv_debugfs_cleanup()
> > >    drm/hdlcd: Remove hdlcd_debugfs_cleanup()
> > >    drm/msm: Remove drm_debugfs_remove_files() calls
> > >    drm/nouveau: Remove nouveau_drm_debugfs_cleanup()
> > >    drm/omap: Remove omap_debugfs_cleanup()
> > >    drm/radeon: Remove drm_debugfs_remove_files() call
> > >    drm/sti: Remove drm_debugfs_remove_files() calls
> > >    drm/tegra: Remove tegra_debugfs_cleanup()
> > >    drm/tilcdc: Remove tilcdc_debugfs_cleanup()
> > >    drm/vc4: Remove vc4_debugfs_cleanup()
> > >    drm/virtio: Remove virtio_gpu_debugfs_takedown()
> > >    drm/qxl: Remove qxl_debugfs_takedown()
> > >    drm/i915: Remove i915_debugfs_unregister()
> > > 
> > >   drivers/gpu/drm/amd/amdgpu/amdgpu.h        |  1 -
> > >   drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 20 ------
> > >   drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c    |  1 -
> > >   drivers/gpu/drm/arm/hdlcd_drv.c            |  7 ---
> > >   drivers/gpu/drm/armada/armada_debugfs.c    | 65 +++-----------------
> > >   drivers/gpu/drm/armada/armada_drm.h        |  1 -
> > >   drivers/gpu/drm/armada/armada_drv.c        |  3 -
> > >   drivers/gpu/drm/drm_atomic.c               |  7 ---
> > >   drivers/gpu/drm/drm_crtc_internal.h        |  1 -
> > >   drivers/gpu/drm/drm_debugfs.c              | 29 +++++----
> > >   drivers/gpu/drm/drm_drv.c                  |  2 +-
> > >   drivers/gpu/drm/etnaviv/Kconfig            |  2 +-
> > >   drivers/gpu/drm/etnaviv/etnaviv_drv.c      |  7 ---
> > >   drivers/gpu/drm/i915/i915_debugfs.c        | 97 ++++--------------------------
> > >   drivers/gpu/drm/i915/i915_drv.c            |  1 -
> > >   drivers/gpu/drm/i915/i915_drv.h            |  2 -
> > >   drivers/gpu/drm/i915/intel_drv.h           |  1 -
> > >   drivers/gpu/drm/i915/intel_pipe_crc.c      | 68 ++++-----------------
> > >   drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.c    |  7 ---
> > >   drivers/gpu/drm/msm/msm_debugfs.c          |  2 -
> > >   drivers/gpu/drm/msm/msm_perf.c             | 29 +--------
> > >   drivers/gpu/drm/msm/msm_rd.c               | 31 +---------
> > >   drivers/gpu/drm/nouveau/nouveau_debugfs.c  | 62 ++++---------------
> > >   drivers/gpu/drm/nouveau/nouveau_debugfs.h  |  6 --
> > >   drivers/gpu/drm/nouveau/nouveau_drm.c      |  1 -
> > >   drivers/gpu/drm/omapdrm/omap_debugfs.c     |  9 ---
> > >   drivers/gpu/drm/omapdrm/omap_drv.c         |  1 -
> > >   drivers/gpu/drm/omapdrm/omap_drv.h         |  1 -
> > >   drivers/gpu/drm/qxl/qxl_debugfs.c          |  9 ---
> > >   drivers/gpu/drm/qxl/qxl_drv.c              |  1 -
> > >   drivers/gpu/drm/qxl/qxl_drv.h              |  1 -
> > >   drivers/gpu/drm/radeon/radeon_device.c     | 16 -----
> > >   drivers/gpu/drm/sti/sti_drv.c              | 48 ++-------------
> > >   drivers/gpu/drm/sti/sti_dvo.c              | 10 ---
> > >   drivers/gpu/drm/sti/sti_hda.c              | 11 ----
> > >   drivers/gpu/drm/sti/sti_hdmi.c             | 11 ----
> > >   drivers/gpu/drm/sti/sti_tvout.c            |  8 ---
> > >   drivers/gpu/drm/tegra/drm.c                |  7 ---
> > >   drivers/gpu/drm/tilcdc/tilcdc_drv.c        | 12 ----
> > >   drivers/gpu/drm/tilcdc/tilcdc_drv.h        |  2 -
> > >   drivers/gpu/drm/vc4/vc4_debugfs.c          |  6 --
> > >   drivers/gpu/drm/vc4/vc4_drv.c              |  1 -
> > >   drivers/gpu/drm/vc4/vc4_drv.h              |  1 -
> > >   drivers/gpu/drm/virtio/virtgpu_debugfs.c   |  8 ---
> > >   drivers/gpu/drm/virtio/virtgpu_drv.c       |  1 -
> > >   drivers/gpu/drm/virtio/virtgpu_drv.h       |  1 -
> > >   46 files changed, 76 insertions(+), 542 deletions(-)
> > > 
> > > --
> > > 2.10.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
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 00/19] drm: debugfs: Remove all files automatically on cleanup

[Daniel Vetter]

On Fri, Jan 27, 2017 at 03:29:29PM +0100, Daniel Vetter wrote:
> On Fri, Jan 27, 2017 at 03:23:43PM +0100, Noralf Trønnes wrote:
> > 
> > Den 27.01.2017 08.49, skrev Daniel Vetter:
> > > On Thu, Jan 26, 2017 at 11:56:02PM +0100, Noralf Trønnes wrote:
> > > > This patchset removes the need for drivers to clean up their debugfs
> > > > files on exit. It is done automatically in drm_debugfs_cleanup().
> > > > This funtion is also called should the driver error out in it's
> > > > drm_driver.debugfs_init callback.
> > > > 
> > > > Two drivers still use drm_debugfs_remove_files():
> > > > - tegra in it's connectors, not sure if I can remove it.
> > > I read through them, and they're removed on the component device nodes
> > > stuff. That looks somewhat fishy from a lifetime point of view, and I
> > > think removing all that code would be better, too.
> > > 
> > > > - qxl because it's debugfs files list is part of struct qxl_device which
> > > >    is freed on unload before drm_minor_unregister() is called cleaning debugfs.
> > > In -next qxl is already demidlayered and there's no longer an unload hook.
> > > This should be all safe ... why is it not?
> > 
> > My bad, I fetched linux-next a few days ago and figured it was
> > up-to-date-enough. Duh, I should have known better after following drm for
> > a year now, it is constantly changing, no pauses.
> > 
> > Can you please ping me when you have pulled driver patches and I'll respin
> > msm, tegra and qxl (and others if necessary), and remove the hook.
> > It's much easier for me to do a small patchset, it's really a strain to get
> > everything lined up correctly with big changes. I didn't have that patch
> > juggling class when in school, so I'm late to the game :-)
> 
> You're doing great with the patch juggling :-) I've just made a pass
> through the series and merge what's already reviewed/acked.

Ok, pulled in the remaining patch (I hope, pls ping if I missed one). We
have only a few debugfs_remove calls left, and I think it's safe to remove
them all too. Up to do that too? Then we could remove the export, to make
sure no new driver gets this wrong ...

Thanks a lot, Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 00/19] drm: debugfs: Remove all files automatically on cleanup

[Noralf Trønnes]

Den 01.03.2017 15.31, skrev Daniel Vetter:
> On Fri, Jan 27, 2017 at 03:29:29PM +0100, Daniel Vetter wrote:
>> On Fri, Jan 27, 2017 at 03:23:43PM +0100, Noralf Trønnes wrote:
>>> Den 27.01.2017 08.49, skrev Daniel Vetter:
>>>> On Thu, Jan 26, 2017 at 11:56:02PM +0100, Noralf Trønnes wrote:
>>>>> This patchset removes the need for drivers to clean up their debugfs
>>>>> files on exit. It is done automatically in drm_debugfs_cleanup().
>>>>> This funtion is also called should the driver error out in it's
>>>>> drm_driver.debugfs_init callback.
>>>>>
>>>>> Two drivers still use drm_debugfs_remove_files():
>>>>> - tegra in it's connectors, not sure if I can remove it.
>>>> I read through them, and they're removed on the component device nodes
>>>> stuff. That looks somewhat fishy from a lifetime point of view, and I
>>>> think removing all that code would be better, too.
>>>>
>>>>> - qxl because it's debugfs files list is part of struct qxl_device which
>>>>>     is freed on unload before drm_minor_unregister() is called cleaning debugfs.
>>>> In -next qxl is already demidlayered and there's no longer an unload hook.
>>>> This should be all safe ... why is it not?
>>> My bad, I fetched linux-next a few days ago and figured it was
>>> up-to-date-enough. Duh, I should have known better after following drm for
>>> a year now, it is constantly changing, no pauses.
>>>
>>> Can you please ping me when you have pulled driver patches and I'll respin
>>> msm, tegra and qxl (and others if necessary), and remove the hook.
>>> It's much easier for me to do a small patchset, it's really a strain to get
>>> everything lined up correctly with big changes. I didn't have that patch
>>> juggling class when in school, so I'm late to the game :-)
>> You're doing great with the patch juggling :-) I've just made a pass
>> through the series and merge what's already reviewed/acked.
> Ok, pulled in the remaining patch (I hope, pls ping if I missed one). We
> have only a few debugfs_remove calls left, and I think it's safe to remove
> them all too. Up to do that too? Then we could remove the export, to make
> sure no new driver gets this wrong ...

Yes, they're all in now. I'll fix up what's left.

Noralf.

> Thanks a lot, Daniel

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel