Re: [PATCH 02/16] drm: Extract drm_debugfs.h

[Daniel Vetter <daniel@ffwll.ch>]

On Wed, Mar 22, 2017 at 03:34:10PM +0200, Ville Syrjälä wrote:
> On Wed, Mar 22, 2017 at 09:36:03AM +0100, Daniel Vetter wrote:
> > Doc polish will follow in the next patch.
> > 
> > Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> > ---
> >  drivers/gpu/drm/drm_debugfs.c |  5 ++-
> >  include/drm/drmP.h            | 44 +------------------------
> >  include/drm/drm_debugfs.h     | 76 +++++++++++++++++++++++++++++++++++++++++++
> >  3 files changed, 81 insertions(+), 44 deletions(-)
> >  create mode 100644 include/drm/drm_debugfs.h
> > 
> > diff --git a/drivers/gpu/drm/drm_debugfs.c b/drivers/gpu/drm/drm_debugfs.c
> > index 1d2d18d82d2e..4b02f4230562 100644
> > --- a/drivers/gpu/drm/drm_debugfs.c
> > +++ b/drivers/gpu/drm/drm_debugfs.c
> > @@ -34,9 +34,12 @@
> >  #include <linux/seq_file.h>
> >  #include <linux/slab.h>
> >  #include <linux/export.h>
> > -#include <drm/drmP.h>
> > +
> > +#include <drm/drm_debugfs.h>
> >  #include <drm/drm_edid.h>
> >  #include <drm/drm_atomic.h>
> > +#include <drm/drmP.h>
> > +
> >  #include "drm_internal.h"
> >  #include "drm_crtc_internal.h"
> >  
> > diff --git a/include/drm/drmP.h b/include/drm/drmP.h
> > index 0e383438f793..60f3255c43a8 100644
> > --- a/include/drm/drmP.h
> > +++ b/include/drm/drmP.h
> > @@ -78,6 +78,7 @@
> >  #include <drm/drm_prime.h>
> >  #include <drm/drm_pci.h>
> >  #include <drm/drm_file.h>
> > +#include <drm/drm_debugfs.h>
> 
> Why do we need this?
> 
> >  
> >  struct module;
> >  
> > @@ -371,27 +372,6 @@ struct drm_ioctl_desc {
> >  #define DRM_SCANOUTPOS_ACCURATE     (1 << 2)
> >  
> >  /**
> > - * Info file list entry. This structure represents a debugfs or proc file to
> > - * be created by the drm core
> > - */
> > -struct drm_info_list {
> > -	const char *name; /** file name */
> > -	int (*show)(struct seq_file*, void*); /** show callback */
> > -	u32 driver_features; /**< Required driver features for this entry */
> > -	void *data;
> > -};
> > -
> > -/**
> > - * debugfs node structure. This structure represents a debugfs file.
> > - */
> > -struct drm_info_node {
> > -	struct list_head list;
> > -	struct drm_minor *minor;
> > -	const struct drm_info_list *info_ent;
> > -	struct dentry *dent;
> > -};
> > -
> > -/**
> >   * DRM device structure. This structure represent a complete card that
> >   * may contain multiple heads.
> >   */
> > @@ -592,28 +572,6 @@ int drm_invalid_op(struct drm_device *dev, void *data,
> >   * DMA quiscent + idle. DMA quiescent usually requires the hardware lock.
> >   */
> >  
> > -				/* Debugfs support */
> > -#if defined(CONFIG_DEBUG_FS)
> > -extern int drm_debugfs_create_files(const struct drm_info_list *files,
> > -				    int count, struct dentry *root,
> > -				    struct drm_minor *minor);
> > -extern int drm_debugfs_remove_files(const struct drm_info_list *files,
> > -				    int count, struct drm_minor *minor);
> > -#else
> > -static inline int drm_debugfs_create_files(const struct drm_info_list *files,
> > -					   int count, struct dentry *root,
> > -					   struct drm_minor *minor)
> > -{
> > -	return 0;
> > -}
> > -
> > -static inline int drm_debugfs_remove_files(const struct drm_info_list *files,
> > -					   int count, struct drm_minor *minor)
> > -{
> > -	return 0;
> > -}
> > -#endif
> > -
> >  			       /* sysfs support (drm_sysfs.c) */
> >  extern void drm_sysfs_hotplug_event(struct drm_device *dev);
> >  
> > diff --git a/include/drm/drm_debugfs.h b/include/drm/drm_debugfs.h
> > new file mode 100644
> > index 000000000000..17e47c073fa9
> > --- /dev/null
> > +++ b/include/drm/drm_debugfs.h
> > @@ -0,0 +1,76 @@
> > +/*
> > + * Internal Header for the Direct Rendering Manager
> > + *
> > + * Copyright 1999 Precision Insight, Inc., Cedar Park, Texas.
> > + * Copyright 2000 VA Linux Systems, Inc., Sunnyvale, California.
> > + * Copyright (c) 2009-2010, Code Aurora Forum.
> > + * All rights reserved.
> > + *
> > + * Author: Rickard E. (Rik) Faith <faith@valinux.com>
> > + * Author: Gareth Hughes <gareth@valinux.com>
> > + *
> > + * Permission is hereby granted, free of charge, to any person obtaining a
> > + * copy of this software and associated documentation files (the "Software"),
> > + * to deal in the Software without restriction, including without limitation
> > + * the rights to use, copy, modify, merge, publish, distribute, sublicense,
> > + * and/or sell copies of the Software, and to permit persons to whom the
> > + * Software is furnished to do so, subject to the following conditions:
> > + *
> > + * The above copyright notice and this permission notice (including the next
> > + * paragraph) shall be included in all copies or substantial portions of the
> > + * Software.
> > + *
> > + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> > + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> > + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
> > + * VA LINUX SYSTEMS AND/OR ITS SUPPLIERS BE LIABLE FOR ANY CLAIM, DAMAGES OR
> > + * OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE,
> > + * ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
> > + * OTHER DEALINGS IN THE SOFTWARE.
> > + */
> > +
> > +#ifndef _DRM_DEBUGFS_H_
> > +#define _DRM_DEBUGFS_H_
> > +
> 
> Do we want to make headers standalone? If we do then we would need a few
> includes and forward declarations.

Yes, but it's hard to make this work properly in one go. I sometimes try
when it's easy, but very often you end up in loops. I think better to
first split things, then try to get rid of drmP.h, one .c file at a time.

> > +/**
> > + * Info file list entry. This structure represents a debugfs or proc file to
> > + * be created by the drm core
> > + */
> > +struct drm_info_list {
> > +	const char *name; /** file name */
> > +	int (*show)(struct seq_file*, void*); /** show callback */
> > +	u32 driver_features; /**< Required driver features for this entry */
> > +	void *data;
> > +};
> > +
> > +/**
> > + * debugfs node structure. This structure represents a debugfs file.
> > + */
> > +struct drm_info_node {
> > +	struct list_head list;
> > +	struct drm_minor *minor;
> > +	const struct drm_info_list *info_ent;
> > +	struct dentry *dent;
> > +};
> > +
> > +#endif /* _DRM_DEBUGFS_H_ */
> 
> Why is this guy in the middle of the file?

Oops, misplaced, will fix up and resend.
-Daniel

> > +#if defined(CONFIG_DEBUG_FS)
> > +int drm_debugfs_create_files(const struct drm_info_list *files,
> > +			     int count, struct dentry *root,
> > +			     struct drm_minor *minor);
> > +int drm_debugfs_remove_files(const struct drm_info_list *files,
> > +			     int count, struct drm_minor *minor);
> > +#else
> > +static inline int drm_debugfs_create_files(const struct drm_info_list *files,
> > +					   int count, struct dentry *root,
> > +					   struct drm_minor *minor)
> > +{
> > +	return 0;
> > +}
> > +
> > +static inline int drm_debugfs_remove_files(const struct drm_info_list *files,
> > +					   int count, struct drm_minor *minor)
> > +{
> > +	return 0;
> > +}
> > +#endif
> > -- 
> > 2.11.0
> > 
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> -- 
> Ville Syrjälä
> Intel OTC
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx