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

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

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

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx