diff mbox

[07/16] drm: Extract drm_ioctl.h

Message ID 20170322083617.13361-8-daniel.vetter@ffwll.ch (mailing list archive)
State New, archived
Headers show

Commit Message

Daniel Vetter March 22, 2017, 8:36 a.m. UTC
To match the drm_ioctl.c we already have.

Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
---
 drivers/gpu/drm/drm_ioctl.c |   1 +
 include/drm/drmP.h          |  61 +-------------------------
 include/drm/drm_ioctl.h     | 102 ++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 104 insertions(+), 60 deletions(-)
 create mode 100644 include/drm/drm_ioctl.h

Comments

Ville Syrjala March 22, 2017, 1:47 p.m. UTC | #1
On Wed, Mar 22, 2017 at 09:36:08AM +0100, Daniel Vetter wrote:
> To match the drm_ioctl.c we already have.
> 
> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> ---
>  drivers/gpu/drm/drm_ioctl.c |   1 +
>  include/drm/drmP.h          |  61 +-------------------------
>  include/drm/drm_ioctl.h     | 102 ++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 104 insertions(+), 60 deletions(-)
>  create mode 100644 include/drm/drm_ioctl.h
> 
> diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c
> index 601bb0ded9d2..7f4f4f48e390 100644
> --- a/drivers/gpu/drm/drm_ioctl.c
> +++ b/drivers/gpu/drm/drm_ioctl.c
> @@ -28,6 +28,7 @@
>   * OTHER DEALINGS IN THE SOFTWARE.
>   */
>  
> +#include <drm/drm_ioctl.h>
>  #include <drm/drmP.h>
>  #include <drm/drm_auth.h>
>  #include "drm_legacy.h"
> diff --git a/include/drm/drmP.h b/include/drm/drmP.h
> index fc2a0744413d..248d2408e56b 100644
> --- a/include/drm/drmP.h
> +++ b/include/drm/drmP.h
> @@ -80,6 +80,7 @@
>  #include <drm/drm_file.h>
>  #include <drm/drm_debugfs.h>
>  #include <drm/drm_sysfs.h>
> +#include <drm/drm_ioctl.h>

Same question as for the debugfs thing. Why do we need this?

>  
>  struct module;
>  
> @@ -318,49 +319,6 @@ struct pci_controller;
>  
>  #define DRM_IF_VERSION(maj, min) (maj << 16 | min)
>  
> -/**
> - * Ioctl function type.
> - *
> - * \param inode device inode.
> - * \param file_priv DRM file private pointer.
> - * \param cmd command.
> - * \param arg argument.
> - */
> -typedef int drm_ioctl_t(struct drm_device *dev, void *data,
> -			struct drm_file *file_priv);
> -
> -typedef int drm_ioctl_compat_t(struct file *filp, unsigned int cmd,
> -			       unsigned long arg);
> -
> -#define DRM_IOCTL_NR(n)                _IOC_NR(n)
> -#define DRM_MAJOR       226
> -
> -#define DRM_AUTH	0x1
> -#define	DRM_MASTER	0x2
> -#define DRM_ROOT_ONLY	0x4
> -#define DRM_CONTROL_ALLOW 0x8
> -#define DRM_UNLOCKED	0x10
> -#define DRM_RENDER_ALLOW 0x20
> -
> -struct drm_ioctl_desc {
> -	unsigned int cmd;
> -	int flags;
> -	drm_ioctl_t *func;
> -	const char *name;
> -};
> -
> -/**
> - * Creates a driver or general drm_ioctl_desc array entry for the given
> - * ioctl, for use by drm_ioctl().
> - */
> -
> -#define DRM_IOCTL_DEF_DRV(ioctl, _func, _flags)				\
> -	[DRM_IOCTL_NR(DRM_IOCTL_##ioctl) - DRM_COMMAND_BASE] = {	\
> -		.cmd = DRM_IOCTL_##ioctl,				\
> -		.func = _func,						\
> -		.flags = _flags,					\
> -		.name = #ioctl						\
> -	 }
>  
>  /* Flags and return codes for get_vblank_timestamp() driver function. */
>  #define DRM_CALLED_FROM_VBLIRQ 1
> @@ -550,23 +508,6 @@ static inline int drm_device_is_unplugged(struct drm_device *dev)
>  /*@{*/
>  
>  				/* Driver support (drm_drv.h) */
> -extern int drm_ioctl_permit(u32 flags, struct drm_file *file_priv);
> -extern long drm_ioctl(struct file *filp,
> -		      unsigned int cmd, unsigned long arg);
> -#ifdef CONFIG_COMPAT
> -extern long drm_compat_ioctl(struct file *filp,
> -			     unsigned int cmd, unsigned long arg);
> -#else
> -/* Let drm_compat_ioctl be assigned to .compat_ioctl unconditionally */
> -#define drm_compat_ioctl NULL
> -#endif
> -extern bool drm_ioctl_flags(unsigned int nr, unsigned int *flags);
> -
> -/* Misc. IOCTL support (drm_ioctl.c) */
> -int drm_noop(struct drm_device *dev, void *data,
> -	     struct drm_file *file_priv);
> -int drm_invalid_op(struct drm_device *dev, void *data,
> -		   struct drm_file *file_priv);
>  
>  /*
>   * These are exported to drivers so that they can implement fencing using
> diff --git a/include/drm/drm_ioctl.h b/include/drm/drm_ioctl.h
> new file mode 100644
> index 000000000000..3c3677b5bdb2
> --- /dev/null
> +++ b/include/drm/drm_ioctl.h
> @@ -0,0 +1,102 @@
> +/*
> + * 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_IOCTL_H_
> +#define _DRM_IOCTL_H_
> +
> +#include <linux/types.h>
> +
> +#include <asm/ioctl.h>
> +
> +struct drm_device;
> +struct drm_file;
> +struct file;
> +
> +/**
> + * Ioctl function type.
> + *
> + * \param inode device inode.
> + * \param file_priv DRM file private pointer.
> + * \param cmd command.
> + * \param arg argument.
> + */
> +typedef int drm_ioctl_t(struct drm_device *dev, void *data,
> +			struct drm_file *file_priv);
> +
> +typedef int drm_ioctl_compat_t(struct file *filp, unsigned int cmd,
> +			       unsigned long arg);
> +
> +#define DRM_IOCTL_NR(n)                _IOC_NR(n)

I wonder what's the point of this wrapper. Maybe leftovers from the
out-of-tree days? Maybe nuke it while you're touching this stuff?

> +#define DRM_MAJOR       226
> +
> +#define DRM_AUTH	0x1
> +#define	DRM_MASTER	0x2
> +#define DRM_ROOT_ONLY	0x4
> +#define DRM_CONTROL_ALLOW 0x8
> +#define DRM_UNLOCKED	0x10
> +#define DRM_RENDER_ALLOW 0x20
> +
> +struct drm_ioctl_desc {
> +	unsigned int cmd;
> +	int flags;
> +	drm_ioctl_t *func;
> +	const char *name;
> +};
> +
> +/**

s/**/*/ ?

> + * Creates a driver or general drm_ioctl_desc array entry for the given
> + * ioctl, for use by drm_ioctl().
> + */
> +
> +#define DRM_IOCTL_DEF_DRV(ioctl, _func, _flags)				\
> +	[DRM_IOCTL_NR(DRM_IOCTL_##ioctl) - DRM_COMMAND_BASE] = {	\
> +		.cmd = DRM_IOCTL_##ioctl,				\
> +		.func = _func,						\
> +		.flags = _flags,					\
> +		.name = #ioctl						\
> +	 }
        ^

Spurious space

Otherwise looks sane so
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

> +
> +int drm_ioctl_permit(u32 flags, struct drm_file *file_priv);
> +long drm_ioctl(struct file *filp, unsigned int cmd, unsigned long arg);
> +#ifdef CONFIG_COMPAT
> +long drm_compat_ioctl(struct file *filp, unsigned int cmd, unsigned long arg);
> +#else
> +/* Let drm_compat_ioctl be assigned to .compat_ioctl unconditionally */
> +#define drm_compat_ioctl NULL
> +#endif
> +bool drm_ioctl_flags(unsigned int nr, unsigned int *flags);
> +
> +int drm_noop(struct drm_device *dev, void *data,
> +	     struct drm_file *file_priv);
> +int drm_invalid_op(struct drm_device *dev, void *data,
> +		   struct drm_file *file_priv);
> +
> +#endif /* _DRM_IOCTL_H_ */
> -- 
> 2.11.0
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Daniel Vetter March 22, 2017, 5:56 p.m. UTC | #2
On Wed, Mar 22, 2017 at 03:47:31PM +0200, Ville Syrjälä wrote:
> On Wed, Mar 22, 2017 at 09:36:08AM +0100, Daniel Vetter wrote:
> > To match the drm_ioctl.c we already have.
> > 
> > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> > ---
> >  drivers/gpu/drm/drm_ioctl.c |   1 +
> >  include/drm/drmP.h          |  61 +-------------------------
> >  include/drm/drm_ioctl.h     | 102 ++++++++++++++++++++++++++++++++++++++++++++
> >  3 files changed, 104 insertions(+), 60 deletions(-)
> >  create mode 100644 include/drm/drm_ioctl.h
> > 
> > diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c
> > index 601bb0ded9d2..7f4f4f48e390 100644
> > --- a/drivers/gpu/drm/drm_ioctl.c
> > +++ b/drivers/gpu/drm/drm_ioctl.c
> > @@ -28,6 +28,7 @@
> >   * OTHER DEALINGS IN THE SOFTWARE.
> >   */
> >  
> > +#include <drm/drm_ioctl.h>
> >  #include <drm/drmP.h>
> >  #include <drm/drm_auth.h>
> >  #include "drm_legacy.h"
> > diff --git a/include/drm/drmP.h b/include/drm/drmP.h
> > index fc2a0744413d..248d2408e56b 100644
> > --- a/include/drm/drmP.h
> > +++ b/include/drm/drmP.h
> > @@ -80,6 +80,7 @@
> >  #include <drm/drm_file.h>
> >  #include <drm/drm_debugfs.h>
> >  #include <drm/drm_sysfs.h>
> > +#include <drm/drm_ioctl.h>
> 
> Same question as for the debugfs thing. Why do we need this?

Try fixing up the drmP.h include hell, and you know that it'd take a few
hundred patches. And right now it's impossible, because the most central
structure (struct drm_device) is in here, so defacto we still have a
monolithic header which needs everything.

First we need to split it completely, then all places that include drmP.h
need to switch to individual headers (each time fixing up a pile of
fallout), and then eventually (after fixing the few hundred places or so)
we can remove all these include lines here (or well, get rid of drmP.h
entirely). But not before. I tried.
-Daniel

> 
> >  
> >  struct module;
> >  
> > @@ -318,49 +319,6 @@ struct pci_controller;
> >  
> >  #define DRM_IF_VERSION(maj, min) (maj << 16 | min)
> >  
> > -/**
> > - * Ioctl function type.
> > - *
> > - * \param inode device inode.
> > - * \param file_priv DRM file private pointer.
> > - * \param cmd command.
> > - * \param arg argument.
> > - */
> > -typedef int drm_ioctl_t(struct drm_device *dev, void *data,
> > -			struct drm_file *file_priv);
> > -
> > -typedef int drm_ioctl_compat_t(struct file *filp, unsigned int cmd,
> > -			       unsigned long arg);
> > -
> > -#define DRM_IOCTL_NR(n)                _IOC_NR(n)
> > -#define DRM_MAJOR       226
> > -
> > -#define DRM_AUTH	0x1
> > -#define	DRM_MASTER	0x2
> > -#define DRM_ROOT_ONLY	0x4
> > -#define DRM_CONTROL_ALLOW 0x8
> > -#define DRM_UNLOCKED	0x10
> > -#define DRM_RENDER_ALLOW 0x20
> > -
> > -struct drm_ioctl_desc {
> > -	unsigned int cmd;
> > -	int flags;
> > -	drm_ioctl_t *func;
> > -	const char *name;
> > -};
> > -
> > -/**
> > - * Creates a driver or general drm_ioctl_desc array entry for the given
> > - * ioctl, for use by drm_ioctl().
> > - */
> > -
> > -#define DRM_IOCTL_DEF_DRV(ioctl, _func, _flags)				\
> > -	[DRM_IOCTL_NR(DRM_IOCTL_##ioctl) - DRM_COMMAND_BASE] = {	\
> > -		.cmd = DRM_IOCTL_##ioctl,				\
> > -		.func = _func,						\
> > -		.flags = _flags,					\
> > -		.name = #ioctl						\
> > -	 }
> >  
> >  /* Flags and return codes for get_vblank_timestamp() driver function. */
> >  #define DRM_CALLED_FROM_VBLIRQ 1
> > @@ -550,23 +508,6 @@ static inline int drm_device_is_unplugged(struct drm_device *dev)
> >  /*@{*/
> >  
> >  				/* Driver support (drm_drv.h) */
> > -extern int drm_ioctl_permit(u32 flags, struct drm_file *file_priv);
> > -extern long drm_ioctl(struct file *filp,
> > -		      unsigned int cmd, unsigned long arg);
> > -#ifdef CONFIG_COMPAT
> > -extern long drm_compat_ioctl(struct file *filp,
> > -			     unsigned int cmd, unsigned long arg);
> > -#else
> > -/* Let drm_compat_ioctl be assigned to .compat_ioctl unconditionally */
> > -#define drm_compat_ioctl NULL
> > -#endif
> > -extern bool drm_ioctl_flags(unsigned int nr, unsigned int *flags);
> > -
> > -/* Misc. IOCTL support (drm_ioctl.c) */
> > -int drm_noop(struct drm_device *dev, void *data,
> > -	     struct drm_file *file_priv);
> > -int drm_invalid_op(struct drm_device *dev, void *data,
> > -		   struct drm_file *file_priv);
> >  
> >  /*
> >   * These are exported to drivers so that they can implement fencing using
> > diff --git a/include/drm/drm_ioctl.h b/include/drm/drm_ioctl.h
> > new file mode 100644
> > index 000000000000..3c3677b5bdb2
> > --- /dev/null
> > +++ b/include/drm/drm_ioctl.h
> > @@ -0,0 +1,102 @@
> > +/*
> > + * 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_IOCTL_H_
> > +#define _DRM_IOCTL_H_
> > +
> > +#include <linux/types.h>
> > +
> > +#include <asm/ioctl.h>
> > +
> > +struct drm_device;
> > +struct drm_file;
> > +struct file;
> > +
> > +/**
> > + * Ioctl function type.
> > + *
> > + * \param inode device inode.
> > + * \param file_priv DRM file private pointer.
> > + * \param cmd command.
> > + * \param arg argument.
> > + */
> > +typedef int drm_ioctl_t(struct drm_device *dev, void *data,
> > +			struct drm_file *file_priv);
> > +
> > +typedef int drm_ioctl_compat_t(struct file *filp, unsigned int cmd,
> > +			       unsigned long arg);
> > +
> > +#define DRM_IOCTL_NR(n)                _IOC_NR(n)
> 
> I wonder what's the point of this wrapper. Maybe leftovers from the
> out-of-tree days? Maybe nuke it while you're touching this stuff?
> 
> > +#define DRM_MAJOR       226
> > +
> > +#define DRM_AUTH	0x1
> > +#define	DRM_MASTER	0x2
> > +#define DRM_ROOT_ONLY	0x4
> > +#define DRM_CONTROL_ALLOW 0x8
> > +#define DRM_UNLOCKED	0x10
> > +#define DRM_RENDER_ALLOW 0x20
> > +
> > +struct drm_ioctl_desc {
> > +	unsigned int cmd;
> > +	int flags;
> > +	drm_ioctl_t *func;
> > +	const char *name;
> > +};
> > +
> > +/**
> 
> s/**/*/ ?
> 
> > + * Creates a driver or general drm_ioctl_desc array entry for the given
> > + * ioctl, for use by drm_ioctl().
> > + */
> > +
> > +#define DRM_IOCTL_DEF_DRV(ioctl, _func, _flags)				\
> > +	[DRM_IOCTL_NR(DRM_IOCTL_##ioctl) - DRM_COMMAND_BASE] = {	\
> > +		.cmd = DRM_IOCTL_##ioctl,				\
> > +		.func = _func,						\
> > +		.flags = _flags,					\
> > +		.name = #ioctl						\
> > +	 }
>         ^
> 
> Spurious space
> 
> Otherwise looks sane so
> Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> > +
> > +int drm_ioctl_permit(u32 flags, struct drm_file *file_priv);
> > +long drm_ioctl(struct file *filp, unsigned int cmd, unsigned long arg);
> > +#ifdef CONFIG_COMPAT
> > +long drm_compat_ioctl(struct file *filp, unsigned int cmd, unsigned long arg);
> > +#else
> > +/* Let drm_compat_ioctl be assigned to .compat_ioctl unconditionally */
> > +#define drm_compat_ioctl NULL
> > +#endif
> > +bool drm_ioctl_flags(unsigned int nr, unsigned int *flags);
> > +
> > +int drm_noop(struct drm_device *dev, void *data,
> > +	     struct drm_file *file_priv);
> > +int drm_invalid_op(struct drm_device *dev, void *data,
> > +		   struct drm_file *file_priv);
> > +
> > +#endif /* _DRM_IOCTL_H_ */
> > -- 
> > 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
Ville Syrjala March 22, 2017, 6:16 p.m. UTC | #3
On Wed, Mar 22, 2017 at 06:56:32PM +0100, Daniel Vetter wrote:
> On Wed, Mar 22, 2017 at 03:47:31PM +0200, Ville Syrjälä wrote:
> > On Wed, Mar 22, 2017 at 09:36:08AM +0100, Daniel Vetter wrote:
> > > To match the drm_ioctl.c we already have.
> > > 
> > > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> > > ---
> > >  drivers/gpu/drm/drm_ioctl.c |   1 +
> > >  include/drm/drmP.h          |  61 +-------------------------
> > >  include/drm/drm_ioctl.h     | 102 ++++++++++++++++++++++++++++++++++++++++++++
> > >  3 files changed, 104 insertions(+), 60 deletions(-)
> > >  create mode 100644 include/drm/drm_ioctl.h
> > > 
> > > diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c
> > > index 601bb0ded9d2..7f4f4f48e390 100644
> > > --- a/drivers/gpu/drm/drm_ioctl.c
> > > +++ b/drivers/gpu/drm/drm_ioctl.c
> > > @@ -28,6 +28,7 @@
> > >   * OTHER DEALINGS IN THE SOFTWARE.
> > >   */
> > >  
> > > +#include <drm/drm_ioctl.h>
> > >  #include <drm/drmP.h>
> > >  #include <drm/drm_auth.h>
> > >  #include "drm_legacy.h"
> > > diff --git a/include/drm/drmP.h b/include/drm/drmP.h
> > > index fc2a0744413d..248d2408e56b 100644
> > > --- a/include/drm/drmP.h
> > > +++ b/include/drm/drmP.h
> > > @@ -80,6 +80,7 @@
> > >  #include <drm/drm_file.h>
> > >  #include <drm/drm_debugfs.h>
> > >  #include <drm/drm_sysfs.h>
> > > +#include <drm/drm_ioctl.h>
> > 
> > Same question as for the debugfs thing. Why do we need this?
> 
> Try fixing up the drmP.h include hell, and you know that it'd take a few
> hundred patches. And right now it's impossible, because the most central
> structure (struct drm_device) is in here, so defacto we still have a
> monolithic header which needs everything.
> 
> First we need to split it completely, then all places that include drmP.h
> need to switch to individual headers (each time fixing up a pile of
> fallout), and then eventually (after fixing the few hundred places or so)
> we can remove all these include lines here (or well, get rid of drmP.h
> entirely). But not before. I tried.

OK. I thought this stuff wouldn't have spread that far beyond the
ioctl implementations, but I guess not.

Hmm. Had I spent any thought on this I would have realized that at
least DRM_IOCTL_DEF_DRV() is used in pretty much every driver.
Daniel Vetter March 22, 2017, 8:15 p.m. UTC | #4
On Wed, Mar 22, 2017 at 03:47:31PM +0200, Ville Syrjälä wrote:
> On Wed, Mar 22, 2017 at 09:36:08AM +0100, Daniel Vetter wrote:
> > To match the drm_ioctl.c we already have.
> > 
> > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> > ---
> >  drivers/gpu/drm/drm_ioctl.c |   1 +
> >  include/drm/drmP.h          |  61 +-------------------------
> >  include/drm/drm_ioctl.h     | 102 ++++++++++++++++++++++++++++++++++++++++++++
> >  3 files changed, 104 insertions(+), 60 deletions(-)
> >  create mode 100644 include/drm/drm_ioctl.h
> > 
> > diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c
> > index 601bb0ded9d2..7f4f4f48e390 100644
> > --- a/drivers/gpu/drm/drm_ioctl.c
> > +++ b/drivers/gpu/drm/drm_ioctl.c
> > @@ -28,6 +28,7 @@
> >   * OTHER DEALINGS IN THE SOFTWARE.
> >   */
> >  
> > +#include <drm/drm_ioctl.h>
> >  #include <drm/drmP.h>
> >  #include <drm/drm_auth.h>
> >  #include "drm_legacy.h"
> > diff --git a/include/drm/drmP.h b/include/drm/drmP.h
> > index fc2a0744413d..248d2408e56b 100644
> > --- a/include/drm/drmP.h
> > +++ b/include/drm/drmP.h
> > @@ -80,6 +80,7 @@
> >  #include <drm/drm_file.h>
> >  #include <drm/drm_debugfs.h>
> >  #include <drm/drm_sysfs.h>
> > +#include <drm/drm_ioctl.h>
> 
> Same question as for the debugfs thing. Why do we need this?

I forgot to scroll down, oops :(

> >  struct module;
> >  
> > @@ -318,49 +319,6 @@ struct pci_controller;
> >  
> >  #define DRM_IF_VERSION(maj, min) (maj << 16 | min)
> >  
> > -/**
> > - * Ioctl function type.
> > - *
> > - * \param inode device inode.
> > - * \param file_priv DRM file private pointer.
> > - * \param cmd command.
> > - * \param arg argument.
> > - */
> > -typedef int drm_ioctl_t(struct drm_device *dev, void *data,
> > -			struct drm_file *file_priv);
> > -
> > -typedef int drm_ioctl_compat_t(struct file *filp, unsigned int cmd,
> > -			       unsigned long arg);
> > -
> > -#define DRM_IOCTL_NR(n)                _IOC_NR(n)
> > -#define DRM_MAJOR       226
> > -
> > -#define DRM_AUTH	0x1
> > -#define	DRM_MASTER	0x2
> > -#define DRM_ROOT_ONLY	0x4
> > -#define DRM_CONTROL_ALLOW 0x8
> > -#define DRM_UNLOCKED	0x10
> > -#define DRM_RENDER_ALLOW 0x20
> > -
> > -struct drm_ioctl_desc {
> > -	unsigned int cmd;
> > -	int flags;
> > -	drm_ioctl_t *func;
> > -	const char *name;
> > -};
> > -
> > -/**
> > - * Creates a driver or general drm_ioctl_desc array entry for the given
> > - * ioctl, for use by drm_ioctl().
> > - */
> > -
> > -#define DRM_IOCTL_DEF_DRV(ioctl, _func, _flags)				\
> > -	[DRM_IOCTL_NR(DRM_IOCTL_##ioctl) - DRM_COMMAND_BASE] = {	\
> > -		.cmd = DRM_IOCTL_##ioctl,				\
> > -		.func = _func,						\
> > -		.flags = _flags,					\
> > -		.name = #ioctl						\
> > -	 }
> >  
> >  /* Flags and return codes for get_vblank_timestamp() driver function. */
> >  #define DRM_CALLED_FROM_VBLIRQ 1
> > @@ -550,23 +508,6 @@ static inline int drm_device_is_unplugged(struct drm_device *dev)
> >  /*@{*/
> >  
> >  				/* Driver support (drm_drv.h) */
> > -extern int drm_ioctl_permit(u32 flags, struct drm_file *file_priv);
> > -extern long drm_ioctl(struct file *filp,
> > -		      unsigned int cmd, unsigned long arg);
> > -#ifdef CONFIG_COMPAT
> > -extern long drm_compat_ioctl(struct file *filp,
> > -			     unsigned int cmd, unsigned long arg);
> > -#else
> > -/* Let drm_compat_ioctl be assigned to .compat_ioctl unconditionally */
> > -#define drm_compat_ioctl NULL
> > -#endif
> > -extern bool drm_ioctl_flags(unsigned int nr, unsigned int *flags);
> > -
> > -/* Misc. IOCTL support (drm_ioctl.c) */
> > -int drm_noop(struct drm_device *dev, void *data,
> > -	     struct drm_file *file_priv);
> > -int drm_invalid_op(struct drm_device *dev, void *data,
> > -		   struct drm_file *file_priv);
> >  
> >  /*
> >   * These are exported to drivers so that they can implement fencing using
> > diff --git a/include/drm/drm_ioctl.h b/include/drm/drm_ioctl.h
> > new file mode 100644
> > index 000000000000..3c3677b5bdb2
> > --- /dev/null
> > +++ b/include/drm/drm_ioctl.h
> > @@ -0,0 +1,102 @@
> > +/*
> > + * 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_IOCTL_H_
> > +#define _DRM_IOCTL_H_
> > +
> > +#include <linux/types.h>
> > +
> > +#include <asm/ioctl.h>
> > +
> > +struct drm_device;
> > +struct drm_file;
> > +struct file;
> > +
> > +/**
> > + * Ioctl function type.
> > + *
> > + * \param inode device inode.
> > + * \param file_priv DRM file private pointer.
> > + * \param cmd command.
> > + * \param arg argument.
> > + */
> > +typedef int drm_ioctl_t(struct drm_device *dev, void *data,
> > +			struct drm_file *file_priv);
> > +
> > +typedef int drm_ioctl_compat_t(struct file *filp, unsigned int cmd,
> > +			       unsigned long arg);
> > +
> > +#define DRM_IOCTL_NR(n)                _IOC_NR(n)
> 
> I wonder what's the point of this wrapper. Maybe leftovers from the
> out-of-tree days? Maybe nuke it while you're touching this stuff?

It's used in a lot of places (all the 32 bit compat code). I guess we
could nuke it (it does smell like *bsd compat cruft), but separate (big)
patch.

> > +#define DRM_MAJOR       226
> > +
> > +#define DRM_AUTH	0x1
> > +#define	DRM_MASTER	0x2
> > +#define DRM_ROOT_ONLY	0x4
> > +#define DRM_CONTROL_ALLOW 0x8
> > +#define DRM_UNLOCKED	0x10
> > +#define DRM_RENDER_ALLOW 0x20
> > +
> > +struct drm_ioctl_desc {
> > +	unsigned int cmd;
> > +	int flags;
> > +	drm_ioctl_t *func;
> > +	const char *name;
> > +};
> > +
> > +/**
> 
> s/**/*/ ?

It'll get addressed in the next patch, where I clean up the kernel-doc. I
assume I can still get your r-b for this patch here?

> > + * Creates a driver or general drm_ioctl_desc array entry for the given
> > + * ioctl, for use by drm_ioctl().
> > + */
> > +
> > +#define DRM_IOCTL_DEF_DRV(ioctl, _func, _flags)				\
> > +	[DRM_IOCTL_NR(DRM_IOCTL_##ioctl) - DRM_COMMAND_BASE] = {	\
> > +		.cmd = DRM_IOCTL_##ioctl,				\
> > +		.func = _func,						\
> > +		.flags = _flags,					\
> > +		.name = #ioctl						\
> > +	 }
>         ^
> 
> Spurious space

Will fix, thx a lot for reviewing.
-Daniel

> 
> Otherwise looks sane so
> Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> > +
> > +int drm_ioctl_permit(u32 flags, struct drm_file *file_priv);
> > +long drm_ioctl(struct file *filp, unsigned int cmd, unsigned long arg);
> > +#ifdef CONFIG_COMPAT
> > +long drm_compat_ioctl(struct file *filp, unsigned int cmd, unsigned long arg);
> > +#else
> > +/* Let drm_compat_ioctl be assigned to .compat_ioctl unconditionally */
> > +#define drm_compat_ioctl NULL
> > +#endif
> > +bool drm_ioctl_flags(unsigned int nr, unsigned int *flags);
> > +
> > +int drm_noop(struct drm_device *dev, void *data,
> > +	     struct drm_file *file_priv);
> > +int drm_invalid_op(struct drm_device *dev, void *data,
> > +		   struct drm_file *file_priv);
> > +
> > +#endif /* _DRM_IOCTL_H_ */
> > -- 
> > 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
Ville Syrjala March 22, 2017, 8:22 p.m. UTC | #5
On Wed, Mar 22, 2017 at 09:15:23PM +0100, Daniel Vetter wrote:
> On Wed, Mar 22, 2017 at 03:47:31PM +0200, Ville Syrjälä wrote:
> > On Wed, Mar 22, 2017 at 09:36:08AM +0100, Daniel Vetter wrote:
> > > To match the drm_ioctl.c we already have.
> > > 
> > > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> > > ---
> > >  drivers/gpu/drm/drm_ioctl.c |   1 +
> > >  include/drm/drmP.h          |  61 +-------------------------
> > >  include/drm/drm_ioctl.h     | 102 ++++++++++++++++++++++++++++++++++++++++++++
> > >  3 files changed, 104 insertions(+), 60 deletions(-)
> > >  create mode 100644 include/drm/drm_ioctl.h
> > > 
> > > diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c
> > > index 601bb0ded9d2..7f4f4f48e390 100644
> > > --- a/drivers/gpu/drm/drm_ioctl.c
> > > +++ b/drivers/gpu/drm/drm_ioctl.c
<snip>
> > > +#define DRM_MAJOR       226
> > > +
> > > +#define DRM_AUTH	0x1
> > > +#define	DRM_MASTER	0x2
> > > +#define DRM_ROOT_ONLY	0x4
> > > +#define DRM_CONTROL_ALLOW 0x8
> > > +#define DRM_UNLOCKED	0x10
> > > +#define DRM_RENDER_ALLOW 0x20
> > > +
> > > +struct drm_ioctl_desc {
> > > +	unsigned int cmd;
> > > +	int flags;
> > > +	drm_ioctl_t *func;
> > > +	const char *name;
> > > +};
> > > +
> > > +/**
> > 
> > s/**/*/ ?
> 
> It'll get addressed in the next patch, where I clean up the kernel-doc. I
> assume I can still get your r-b for this patch here?

Sure thing.
diff mbox

Patch

diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c
index 601bb0ded9d2..7f4f4f48e390 100644
--- a/drivers/gpu/drm/drm_ioctl.c
+++ b/drivers/gpu/drm/drm_ioctl.c
@@ -28,6 +28,7 @@ 
  * OTHER DEALINGS IN THE SOFTWARE.
  */
 
+#include <drm/drm_ioctl.h>
 #include <drm/drmP.h>
 #include <drm/drm_auth.h>
 #include "drm_legacy.h"
diff --git a/include/drm/drmP.h b/include/drm/drmP.h
index fc2a0744413d..248d2408e56b 100644
--- a/include/drm/drmP.h
+++ b/include/drm/drmP.h
@@ -80,6 +80,7 @@ 
 #include <drm/drm_file.h>
 #include <drm/drm_debugfs.h>
 #include <drm/drm_sysfs.h>
+#include <drm/drm_ioctl.h>
 
 struct module;
 
@@ -318,49 +319,6 @@  struct pci_controller;
 
 #define DRM_IF_VERSION(maj, min) (maj << 16 | min)
 
-/**
- * Ioctl function type.
- *
- * \param inode device inode.
- * \param file_priv DRM file private pointer.
- * \param cmd command.
- * \param arg argument.
- */
-typedef int drm_ioctl_t(struct drm_device *dev, void *data,
-			struct drm_file *file_priv);
-
-typedef int drm_ioctl_compat_t(struct file *filp, unsigned int cmd,
-			       unsigned long arg);
-
-#define DRM_IOCTL_NR(n)                _IOC_NR(n)
-#define DRM_MAJOR       226
-
-#define DRM_AUTH	0x1
-#define	DRM_MASTER	0x2
-#define DRM_ROOT_ONLY	0x4
-#define DRM_CONTROL_ALLOW 0x8
-#define DRM_UNLOCKED	0x10
-#define DRM_RENDER_ALLOW 0x20
-
-struct drm_ioctl_desc {
-	unsigned int cmd;
-	int flags;
-	drm_ioctl_t *func;
-	const char *name;
-};
-
-/**
- * Creates a driver or general drm_ioctl_desc array entry for the given
- * ioctl, for use by drm_ioctl().
- */
-
-#define DRM_IOCTL_DEF_DRV(ioctl, _func, _flags)				\
-	[DRM_IOCTL_NR(DRM_IOCTL_##ioctl) - DRM_COMMAND_BASE] = {	\
-		.cmd = DRM_IOCTL_##ioctl,				\
-		.func = _func,						\
-		.flags = _flags,					\
-		.name = #ioctl						\
-	 }
 
 /* Flags and return codes for get_vblank_timestamp() driver function. */
 #define DRM_CALLED_FROM_VBLIRQ 1
@@ -550,23 +508,6 @@  static inline int drm_device_is_unplugged(struct drm_device *dev)
 /*@{*/
 
 				/* Driver support (drm_drv.h) */
-extern int drm_ioctl_permit(u32 flags, struct drm_file *file_priv);
-extern long drm_ioctl(struct file *filp,
-		      unsigned int cmd, unsigned long arg);
-#ifdef CONFIG_COMPAT
-extern long drm_compat_ioctl(struct file *filp,
-			     unsigned int cmd, unsigned long arg);
-#else
-/* Let drm_compat_ioctl be assigned to .compat_ioctl unconditionally */
-#define drm_compat_ioctl NULL
-#endif
-extern bool drm_ioctl_flags(unsigned int nr, unsigned int *flags);
-
-/* Misc. IOCTL support (drm_ioctl.c) */
-int drm_noop(struct drm_device *dev, void *data,
-	     struct drm_file *file_priv);
-int drm_invalid_op(struct drm_device *dev, void *data,
-		   struct drm_file *file_priv);
 
 /*
  * These are exported to drivers so that they can implement fencing using
diff --git a/include/drm/drm_ioctl.h b/include/drm/drm_ioctl.h
new file mode 100644
index 000000000000..3c3677b5bdb2
--- /dev/null
+++ b/include/drm/drm_ioctl.h
@@ -0,0 +1,102 @@ 
+/*
+ * 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_IOCTL_H_
+#define _DRM_IOCTL_H_
+
+#include <linux/types.h>
+
+#include <asm/ioctl.h>
+
+struct drm_device;
+struct drm_file;
+struct file;
+
+/**
+ * Ioctl function type.
+ *
+ * \param inode device inode.
+ * \param file_priv DRM file private pointer.
+ * \param cmd command.
+ * \param arg argument.
+ */
+typedef int drm_ioctl_t(struct drm_device *dev, void *data,
+			struct drm_file *file_priv);
+
+typedef int drm_ioctl_compat_t(struct file *filp, unsigned int cmd,
+			       unsigned long arg);
+
+#define DRM_IOCTL_NR(n)                _IOC_NR(n)
+#define DRM_MAJOR       226
+
+#define DRM_AUTH	0x1
+#define	DRM_MASTER	0x2
+#define DRM_ROOT_ONLY	0x4
+#define DRM_CONTROL_ALLOW 0x8
+#define DRM_UNLOCKED	0x10
+#define DRM_RENDER_ALLOW 0x20
+
+struct drm_ioctl_desc {
+	unsigned int cmd;
+	int flags;
+	drm_ioctl_t *func;
+	const char *name;
+};
+
+/**
+ * Creates a driver or general drm_ioctl_desc array entry for the given
+ * ioctl, for use by drm_ioctl().
+ */
+
+#define DRM_IOCTL_DEF_DRV(ioctl, _func, _flags)				\
+	[DRM_IOCTL_NR(DRM_IOCTL_##ioctl) - DRM_COMMAND_BASE] = {	\
+		.cmd = DRM_IOCTL_##ioctl,				\
+		.func = _func,						\
+		.flags = _flags,					\
+		.name = #ioctl						\
+	 }
+
+int drm_ioctl_permit(u32 flags, struct drm_file *file_priv);
+long drm_ioctl(struct file *filp, unsigned int cmd, unsigned long arg);
+#ifdef CONFIG_COMPAT
+long drm_compat_ioctl(struct file *filp, unsigned int cmd, unsigned long arg);
+#else
+/* Let drm_compat_ioctl be assigned to .compat_ioctl unconditionally */
+#define drm_compat_ioctl NULL
+#endif
+bool drm_ioctl_flags(unsigned int nr, unsigned int *flags);
+
+int drm_noop(struct drm_device *dev, void *data,
+	     struct drm_file *file_priv);
+int drm_invalid_op(struct drm_device *dev, void *data,
+		   struct drm_file *file_priv);
+
+#endif /* _DRM_IOCTL_H_ */