[PATCH 0/3] drm: debugfs: Remove all files automatically on cleanup part 2

[PATCH 0/3] drm: debugfs: Remove all files automatically on cleanup part 2

[Noralf Trønnes]

This is a follow up that removes the drm_driver.debugfs_cleanup callback.

Tegra is the only remaining user of drm_debugfs_remove_files().

Note:
Patches are only compile tested.

Noralf.


Noralf Trønnes (3):
  drm/msm: Remove msm_debugfs_cleanup()
  drm/debugfs: Remove the drm_driver.debugfs_cleanup callback
  drm/qxl: Remove qxl_debugfs_remove_files()

 drivers/gpu/drm/drm_debugfs.c     |  5 -----
 drivers/gpu/drm/msm/msm_debugfs.c | 15 ---------------
 drivers/gpu/drm/msm/msm_debugfs.h |  1 -
 drivers/gpu/drm/msm/msm_drv.c     |  4 +++-
 drivers/gpu/drm/msm/msm_drv.h     |  4 ++--
 drivers/gpu/drm/msm/msm_kms.h     |  1 -
 drivers/gpu/drm/msm/msm_perf.c    |  5 ++---
 drivers/gpu/drm/msm/msm_rd.c      |  5 ++---
 drivers/gpu/drm/qxl/qxl_debugfs.c | 13 -------------
 drivers/gpu/drm/qxl/qxl_drv.h     |  1 -
 drivers/gpu/drm/qxl/qxl_kms.c     |  1 -
 include/drm/drm_drv.h             |  1 -
 12 files changed, 9 insertions(+), 47 deletions(-)

--
2.10.2

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

[PATCH 1/3] drm/msm: Remove msm_debugfs_cleanup()

[Noralf Trønnes]

Move the contents of msm_debugfs_cleanup() to msm_drm_uninit() to free
up the drm_driver->debugfs_cleanup callback. Also remove the
mdp_kms_funcs->debugfs_cleanup callback which has no users.

Cc: robdclark@gmail.com
Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
---
 drivers/gpu/drm/msm/msm_debugfs.c | 15 ---------------
 drivers/gpu/drm/msm/msm_debugfs.h |  1 -
 drivers/gpu/drm/msm/msm_drv.c     |  4 +++-
 drivers/gpu/drm/msm/msm_drv.h     |  4 ++--
 drivers/gpu/drm/msm/msm_kms.h     |  1 -
 drivers/gpu/drm/msm/msm_perf.c    |  5 ++---
 drivers/gpu/drm/msm/msm_rd.c      |  5 ++---
 7 files changed, 9 insertions(+), 26 deletions(-)

diff --git a/drivers/gpu/drm/msm/msm_debugfs.c b/drivers/gpu/drm/msm/msm_debugfs.c
index e754429..1855182 100644
--- a/drivers/gpu/drm/msm/msm_debugfs.c
+++ b/drivers/gpu/drm/msm/msm_debugfs.c
@@ -166,20 +166,5 @@ int msm_debugfs_init(struct drm_minor *minor)
 
 	return ret;
 }
-
-void msm_debugfs_cleanup(struct drm_minor *minor)
-{
-	struct drm_device *dev = minor->dev;
-	struct msm_drm_private *priv = dev->dev_private;
-
-	if (!priv)
-		return;
-
-	if (priv->kms->funcs->debugfs_cleanup)
-		priv->kms->funcs->debugfs_cleanup(priv->kms, minor);
-
-	msm_rd_debugfs_cleanup(minor);
-	msm_perf_debugfs_cleanup(minor);
-}
 #endif
 
diff --git a/drivers/gpu/drm/msm/msm_debugfs.h b/drivers/gpu/drm/msm/msm_debugfs.h
index 6110c97..f4077e3 100644
--- a/drivers/gpu/drm/msm/msm_debugfs.h
+++ b/drivers/gpu/drm/msm/msm_debugfs.h
@@ -20,7 +20,6 @@
 
 #ifdef CONFIG_DEBUG_FS
 int msm_debugfs_init(struct drm_minor *minor);
-void msm_debugfs_cleanup(struct drm_minor *minor);
 #endif
 
 #endif /* __MSM_DEBUGFS_H__ */
diff --git a/drivers/gpu/drm/msm/msm_drv.c b/drivers/gpu/drm/msm/msm_drv.c
index 6c1a499..500ad18 100644
--- a/drivers/gpu/drm/msm/msm_drv.c
+++ b/drivers/gpu/drm/msm/msm_drv.c
@@ -241,6 +241,9 @@ static int msm_drm_uninit(struct device *dev)
 
 	drm_dev_unregister(ddev);
 
+	msm_perf_debugfs_cleanup(priv);
+	msm_rd_debugfs_cleanup(priv);
+
 #ifdef CONFIG_DRM_FBDEV_EMULATION
 	if (fbdev && priv->fbdev)
 		msm_fbdev_free(ddev);
@@ -838,7 +841,6 @@ static struct drm_driver msm_driver = {
 	.gem_prime_mmap     = msm_gem_prime_mmap,
 #ifdef CONFIG_DEBUG_FS
 	.debugfs_init       = msm_debugfs_init,
-	.debugfs_cleanup    = msm_debugfs_cleanup,
 #endif
 	.ioctls             = msm_ioctls,
 	.num_ioctls         = DRM_MSM_NUM_IOCTLS,
diff --git a/drivers/gpu/drm/msm/msm_drv.h b/drivers/gpu/drm/msm/msm_drv.h
index c3b1487..b51fb0d 100644
--- a/drivers/gpu/drm/msm/msm_drv.h
+++ b/drivers/gpu/drm/msm/msm_drv.h
@@ -304,10 +304,10 @@ void msm_gem_describe_objects(struct list_head *list, struct seq_file *m);
 void msm_framebuffer_describe(struct drm_framebuffer *fb, struct seq_file *m);
 int msm_debugfs_late_init(struct drm_device *dev);
 int msm_rd_debugfs_init(struct drm_minor *minor);
-void msm_rd_debugfs_cleanup(struct drm_minor *minor);
+void msm_rd_debugfs_cleanup(struct msm_drm_private *priv);
 void msm_rd_dump_submit(struct msm_gem_submit *submit);
 int msm_perf_debugfs_init(struct drm_minor *minor);
-void msm_perf_debugfs_cleanup(struct drm_minor *minor);
+void msm_perf_debugfs_cleanup(struct msm_drm_private *priv);
 #else
 static inline int msm_debugfs_late_init(struct drm_device *dev) { return 0; }
 static inline void msm_rd_dump_submit(struct msm_gem_submit *submit) {}
diff --git a/drivers/gpu/drm/msm/msm_kms.h b/drivers/gpu/drm/msm/msm_kms.h
index 117635d2..faa22c7 100644
--- a/drivers/gpu/drm/msm/msm_kms.h
+++ b/drivers/gpu/drm/msm/msm_kms.h
@@ -64,7 +64,6 @@ struct msm_kms_funcs {
 #ifdef CONFIG_DEBUG_FS
 	/* debugfs: */
 	int (*debugfs_init)(struct msm_kms *kms, struct drm_minor *minor);
-	void (*debugfs_cleanup)(struct msm_kms *kms, struct drm_minor *minor);
 #endif
 };
 
diff --git a/drivers/gpu/drm/msm/msm_perf.c b/drivers/gpu/drm/msm/msm_perf.c
index fc5a948..5ab21bd 100644
--- a/drivers/gpu/drm/msm/msm_perf.c
+++ b/drivers/gpu/drm/msm/msm_perf.c
@@ -231,13 +231,12 @@ int msm_perf_debugfs_init(struct drm_minor *minor)
 	return 0;
 
 fail:
-	msm_perf_debugfs_cleanup(minor);
+	msm_perf_debugfs_cleanup(priv);
 	return -1;
 }
 
-void msm_perf_debugfs_cleanup(struct drm_minor *minor)
+void msm_perf_debugfs_cleanup(struct msm_drm_private *priv)
 {
-	struct msm_drm_private *priv = minor->dev->dev_private;
 	struct msm_perf_state *perf = priv->perf;
 
 	if (!perf)
diff --git a/drivers/gpu/drm/msm/msm_rd.c b/drivers/gpu/drm/msm/msm_rd.c
index ab0b39f..3df7322 100644
--- a/drivers/gpu/drm/msm/msm_rd.c
+++ b/drivers/gpu/drm/msm/msm_rd.c
@@ -245,13 +245,12 @@ int msm_rd_debugfs_init(struct drm_minor *minor)
 	return 0;
 
 fail:
-	msm_rd_debugfs_cleanup(minor);
+	msm_rd_debugfs_cleanup(priv);
 	return -1;
 }
 
-void msm_rd_debugfs_cleanup(struct drm_minor *minor)
+void msm_rd_debugfs_cleanup(struct msm_drm_private *priv)
 {
-	struct msm_drm_private *priv = minor->dev->dev_private;
 	struct msm_rd_state *rd = priv->rd;
 
 	if (!rd)
-- 
2.10.2

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

[PATCH 2/3] drm/debugfs: Remove the drm_driver.debugfs_cleanup callback

[Noralf Trønnes]

Remove the .debugfs_cleanup() callback now that all the users are gone.

Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
---
 drivers/gpu/drm/drm_debugfs.c | 5 -----
 include/drm/drm_drv.h         | 1 -
 2 files changed, 6 deletions(-)

diff --git a/drivers/gpu/drm/drm_debugfs.c b/drivers/gpu/drm/drm_debugfs.c
index 8b2c61a..1d2d18d 100644
--- a/drivers/gpu/drm/drm_debugfs.c
+++ b/drivers/gpu/drm/drm_debugfs.c
@@ -242,14 +242,9 @@ 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;
-
 	if (!minor->debugfs_root)
 		return 0;
 
-	if (dev->driver->debugfs_cleanup)
-		dev->driver->debugfs_cleanup(minor);
-
 	drm_debugfs_remove_all_files(minor);
 
 	debugfs_remove_recursive(minor->debugfs_root);
diff --git a/include/drm/drm_drv.h b/include/drm/drm_drv.h
index 661ca24..8ed1ef5 100644
--- a/include/drm/drm_drv.h
+++ b/include/drm/drm_drv.h
@@ -302,7 +302,6 @@ struct drm_driver {
 	void (*master_drop)(struct drm_device *dev, struct drm_file *file_priv);
 
 	int (*debugfs_init)(struct drm_minor *minor);
-	void (*debugfs_cleanup)(struct drm_minor *minor);
 
 	/**
 	 * @gem_free_object: deconstructor for drm_gem_objects
-- 
2.10.2

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

[PATCH 3/3] drm/qxl: Remove qxl_debugfs_remove_files()

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

diff --git a/drivers/gpu/drm/qxl/qxl_debugfs.c b/drivers/gpu/drm/qxl/qxl_debugfs.c
index 8e6c780..ffe821b 100644
--- a/drivers/gpu/drm/qxl/qxl_debugfs.c
+++ b/drivers/gpu/drm/qxl/qxl_debugfs.c
@@ -129,16 +129,3 @@ int qxl_debugfs_add_files(struct qxl_device *qdev,
 #endif
 	return 0;
 }
-
-void qxl_debugfs_remove_files(struct qxl_device *qdev)
-{
-#if defined(CONFIG_DEBUG_FS)
-	unsigned i;
-
-	for (i = 0; i < qdev->debugfs_count; i++) {
-		drm_debugfs_remove_files(qdev->debugfs[i].files,
-					 qdev->debugfs[i].num_files,
-					 qdev->ddev.primary);
-	}
-#endif
-}
diff --git a/drivers/gpu/drm/qxl/qxl_drv.h b/drivers/gpu/drm/qxl/qxl_drv.h
index c048170..ed50a5b 100644
--- a/drivers/gpu/drm/qxl/qxl_drv.h
+++ b/drivers/gpu/drm/qxl/qxl_drv.h
@@ -232,7 +232,6 @@ int qxl_debugfs_add_files(struct qxl_device *rdev,
 			     struct drm_info_list *files,
 			     unsigned nfiles);
 int qxl_debugfs_fence_init(struct qxl_device *rdev);
-void qxl_debugfs_remove_files(struct qxl_device *qdev);
 
 struct qxl_device;
 
diff --git a/drivers/gpu/drm/qxl/qxl_kms.c b/drivers/gpu/drm/qxl/qxl_kms.c
index 2b1e1f3..1c46532 100644
--- a/drivers/gpu/drm/qxl/qxl_kms.c
+++ b/drivers/gpu/drm/qxl/qxl_kms.c
@@ -284,5 +284,4 @@ void qxl_device_fini(struct qxl_device *qdev)
 	qdev->rom = NULL;
 	qdev->mode_info.modes = NULL;
 	qdev->mode_info.num_modes = 0;
-	qxl_debugfs_remove_files(qdev);
 }
-- 
2.10.2

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

Re: [PATCH 3/3] drm/qxl: Remove qxl_debugfs_remove_files()

[Gerd Hoffmann]

On Di, 2017-03-07 at 21:49 +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().
> 
> Cc: airlied@linux.ie
> Cc: kraxel@redhat.com
> Signed-off-by: Noralf Trønnes <noralf@tronnes.org>

Reviewed-by: Gerd Hoffmann <kraxel@redhat.com>

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

Re: [PATCH 3/3] drm/qxl: Remove qxl_debugfs_remove_files()

[Daniel Vetter]

On Wed, Mar 08, 2017 at 08:45:13AM +0100, Gerd Hoffmann wrote:
> On Di, 2017-03-07 at 21:49 +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().
> > 
> > Cc: airlied@linux.ie
> > Cc: kraxel@redhat.com
> > Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
> 
> Reviewed-by: Gerd Hoffmann <kraxel@redhat.com>

I assume you'll push to drm-misc yourself since its qxl?

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

Re: [PATCH 2/3] drm/debugfs: Remove the drm_driver.debugfs_cleanup callback

[Daniel Vetter]

On Tue, Mar 07, 2017 at 09:49:23PM +0100, Noralf Trønnes wrote:
> Remove the .debugfs_cleanup() callback now that all the users are gone.
> 
> Signed-off-by: Noralf Trønnes <noralf@tronnes.org>

First 2 patches merged to drm-misc, with Rob's irc ack on the first one.
I'll leave the 3rd for Gerd.

Thanks, Daniel

> ---
>  drivers/gpu/drm/drm_debugfs.c | 5 -----
>  include/drm/drm_drv.h         | 1 -
>  2 files changed, 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_debugfs.c b/drivers/gpu/drm/drm_debugfs.c
> index 8b2c61a..1d2d18d 100644
> --- a/drivers/gpu/drm/drm_debugfs.c
> +++ b/drivers/gpu/drm/drm_debugfs.c
> @@ -242,14 +242,9 @@ 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;
> -
>  	if (!minor->debugfs_root)
>  		return 0;
>  
> -	if (dev->driver->debugfs_cleanup)
> -		dev->driver->debugfs_cleanup(minor);
> -
>  	drm_debugfs_remove_all_files(minor);
>  
>  	debugfs_remove_recursive(minor->debugfs_root);
> diff --git a/include/drm/drm_drv.h b/include/drm/drm_drv.h
> index 661ca24..8ed1ef5 100644
> --- a/include/drm/drm_drv.h
> +++ b/include/drm/drm_drv.h
> @@ -302,7 +302,6 @@ struct drm_driver {
>  	void (*master_drop)(struct drm_device *dev, struct drm_file *file_priv);
>  
>  	int (*debugfs_init)(struct drm_minor *minor);
> -	void (*debugfs_cleanup)(struct drm_minor *minor);
>  
>  	/**
>  	 * @gem_free_object: deconstructor for drm_gem_objects
> -- 
> 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 3/3] drm/qxl: Remove qxl_debugfs_remove_files()

[Gerd Hoffmann]

On Mi, 2017-03-08 at 10:52 +0100, Daniel Vetter wrote:
> On Wed, Mar 08, 2017 at 08:45:13AM +0100, Gerd Hoffmann wrote:
> > On Di, 2017-03-07 at 21:49 +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().
> > > 
> > > Cc: airlied@linux.ie
> > > Cc: kraxel@redhat.com
> > > Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
> > 
> > Reviewed-by: Gerd Hoffmann <kraxel@redhat.com>
> 
> I assume you'll push to drm-misc yourself since its qxl?

Can surely do that.

Question though:  This seems to be a cross-driver cleanup series.  At
least only 3/3 landed in my inbox, did't look @ dri-devel for the other
patches.  What is the usual policy on those?

From the past I had the impression that in such a case the whole series
gets applied by one of the drm-misc maintainers instead of each
individual driver maintainer cherry-picking the patches from the
series ...

cheers,
  Gerd

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

Re: [PATCH 3/3] drm/qxl: Remove qxl_debugfs_remove_files()

[Daniel Vetter]

On Wed, Mar 08, 2017 at 12:33:01PM +0100, Gerd Hoffmann wrote:
> On Mi, 2017-03-08 at 10:52 +0100, Daniel Vetter wrote:
> > On Wed, Mar 08, 2017 at 08:45:13AM +0100, Gerd Hoffmann wrote:
> > > On Di, 2017-03-07 at 21:49 +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().
> > > > 
> > > > Cc: airlied@linux.ie
> > > > Cc: kraxel@redhat.com
> > > > Signed-off-by: Noralf Trønnes <noralf@tronnes.org>
> > > 
> > > Reviewed-by: Gerd Hoffmann <kraxel@redhat.com>
> > 
> > I assume you'll push to drm-misc yourself since its qxl?
> 
> Can surely do that.
> 
> Question though:  This seems to be a cross-driver cleanup series.  At
> least only 3/3 landed in my inbox, did't look @ dri-devel for the other
> patches.  What is the usual policy on those?
> 
> From the past I had the impression that in such a case the whole series
> gets applied by one of the drm-misc maintainers instead of each
> individual driver maintainer cherry-picking the patches from the
> series ...

Well mostly because many driver maintainers aren't dutiful with picking up
patches, or lose them again, or just take forever. But for this series
here there's no such excuse, and there's also no technical reason to order
patches a certain way (at least your qxl one is free standing and depends
upon nothing else and blocks nothing else).

One of my goals with drm-misc is that I can reduce my efforts spending on
catchging other maintainer's fallout, so I'd very much encourage everyone
to just push stuff. Especially if you're the maintainer, and you slap an
r-b onto a patch with no indication whether you merge it yourself or
except someone else to merge it it's confusing and just improves the odds
of the patch hitting one of our holes ...
-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