Re: [Intel-gfx] [PATCH 07/16] drm: Extract drm_ioctl.h

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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@xxxxxxxxx>
> > ---
> >  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@xxxxxxxxxxx>
> > + * Author: Gareth Hughes <gareth@xxxxxxxxxxx>
> > + *
> > + * 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@xxxxxxxxxxxxxxx>
> 
> > +
> > +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@xxxxxxxxxxxxxxxxxxxxx
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> -- 
> Ville Syrjälä
> Intel OTC

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




[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux