drm: document drm_ioctl.[hc]
authorDaniel Vetter <daniel.vetter@ffwll.ch>
Tue, 4 Apr 2017 09:52:57 +0000 (11:52 +0200)
committerDaniel Vetter <daniel.vetter@ffwll.ch>
Tue, 4 Apr 2017 18:47:54 +0000 (20:47 +0200)
Also unify/merge with the existing stuff.

I was a bit torn where to put this, but in the end I decided to put
all the ioctl/sysfs/debugfs stuff into drm-uapi.rst. That means we
have a bit a split with the other uapi related stuff used internally,
like drm_file.[hc], but I think overall this makes more sense.

If it's too confusing we can always add more cross-links to make it
more discoverable. But the auto-sprinkling of links kernel-doc already
does seems sufficient.

Also for prettier docs and more cross-links, switch the internal
defines over to an enum, as usual.

v2: Update kerneldoc fro drm_compat_ioctl too (caught by 0day), plus a
bit more drive-by polish.

v3: Fix typo, spotted by xerpi on irc (Sergi).

v4: Add missing space in comment (Neil).

Cc: Sergi Granell <xerpi.g.12@gmail.com>
Reviewed-by: Neil Armstrong <narmstrong@baylibre.com>
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
Link: http://patchwork.freedesktop.org/patch/msgid/20170404095304.17599-4-daniel.vetter@ffwll.ch
Documentation/gpu/drm-internals.rst
Documentation/gpu/drm-uapi.rst
drivers/gpu/drm/drm_ioc32.c
drivers/gpu/drm/drm_ioctl.c
include/drm/drm_ioctl.h

index a09c721..babfb61 100644 (file)
@@ -255,56 +255,6 @@ File Operations
 .. kernel-doc:: drivers/gpu/drm/drm_file.c
    :export:
 
-IOCTLs
-------
-
-struct drm_ioctl_desc \*ioctls; int num_ioctls;
-    Driver-specific ioctls descriptors table.
-
-Driver-specific ioctls numbers start at DRM_COMMAND_BASE. The ioctls
-descriptors table is indexed by the ioctl number offset from the base
-value. Drivers can use the DRM_IOCTL_DEF_DRV() macro to initialize
-the table entries.
-
-::
-
-    DRM_IOCTL_DEF_DRV(ioctl, func, flags)
-
-``ioctl`` is the ioctl name. Drivers must define the DRM_##ioctl and
-DRM_IOCTL_##ioctl macros to the ioctl number offset from
-DRM_COMMAND_BASE and the ioctl number respectively. The first macro is
-private to the device while the second must be exposed to userspace in a
-public header.
-
-``func`` is a pointer to the ioctl handler function compatible with the
-``drm_ioctl_t`` type.
-
-::
-
-    typedef int drm_ioctl_t(struct drm_device *dev, void *data,
-            struct drm_file *file_priv);
-
-``flags`` is a bitmask combination of the following values. It restricts
-how the ioctl is allowed to be called.
-
--  DRM_AUTH - Only authenticated callers allowed
-
--  DRM_MASTER - The ioctl can only be called on the master file handle
-
--  DRM_ROOT_ONLY - Only callers with the SYSADMIN capability allowed
-
--  DRM_CONTROL_ALLOW - The ioctl can only be called on a control
-   device
-
--  DRM_UNLOCKED - The ioctl handler will be called without locking the
-   DRM global mutex. This is the enforced default for kms drivers (i.e.
-   using the DRIVER_MODESET flag) and hence shouldn't be used any more
-   for new drivers.
-
-.. kernel-doc:: drivers/gpu/drm/drm_ioctl.c
-   :export:
-
-
 Misc Utilities
 ==============
 
index 8b0f0e4..8584575 100644 (file)
@@ -160,6 +160,20 @@ other hand, a driver requires shared state between clients which is
 visible to user-space and accessible beyond open-file boundaries, they
 cannot support render nodes.
 
+IOCTL Support on Device Nodes
+=============================
+
+.. kernel-doc:: drivers/gpu/drm/drm_ioctl.c
+   :doc: driver specific ioctls
+
+.. kernel-doc:: include/drm/drm_ioctl.h
+   :internal:
+
+.. kernel-doc:: drivers/gpu/drm/drm_ioctl.c
+   :export:
+
+.. kernel-doc:: drivers/gpu/drm/drm_ioc32.c
+   :export:
 
 Testing and validation
 ======================
index b134482..ae38678 100644 (file)
@@ -1,4 +1,4 @@
-/**
+/*
  * \file drm_ioc32.c
  *
  * 32-bit ioctl compatibility routines for the DRM.
 #define DRM_IOCTL_MODE_ADDFB232                DRM_IOWR(0xb8, drm_mode_fb_cmd232_t)
 
 typedef struct drm_version_32 {
-       int version_major;        /**< Major version */
-       int version_minor;        /**< Minor version */
-       int version_patchlevel;    /**< Patch level */
-       u32 name_len;             /**< Length of name buffer */
-       u32 name;                 /**< Name of driver */
-       u32 date_len;             /**< Length of date buffer */
-       u32 date;                 /**< User-space buffer to hold date */
-       u32 desc_len;             /**< Length of desc buffer */
-       u32 desc;                 /**< User-space buffer to hold desc */
+       int version_major;        /* Major version */
+       int version_minor;        /* Minor version */
+       int version_patchlevel;    /* Patch level */
+       u32 name_len;             /* Length of name buffer */
+       u32 name;                 /* Name of driver */
+       u32 date_len;             /* Length of date buffer */
+       u32 date;                 /* User-space buffer to hold date */
+       u32 desc_len;             /* Length of desc buffer */
+       u32 desc;                 /* User-space buffer to hold desc */
 } drm_version32_t;
 
 static int compat_drm_version(struct file *file, unsigned int cmd,
@@ -126,8 +126,8 @@ static int compat_drm_version(struct file *file, unsigned int cmd,
 }
 
 typedef struct drm_unique32 {
-       u32 unique_len; /**< Length of unique */
-       u32 unique;     /**< Unique name for driver instantiation */
+       u32 unique_len; /* Length of unique */
+       u32 unique;     /* Unique name for driver instantiation */
 } drm_unique32_t;
 
 static int compat_drm_getunique(struct file *file, unsigned int cmd,
@@ -180,12 +180,12 @@ static int compat_drm_setunique(struct file *file, unsigned int cmd,
 }
 
 typedef struct drm_map32 {
-       u32 offset;             /**< Requested physical address (0 for SAREA)*/
-       u32 size;               /**< Requested physical size (bytes) */
-       enum drm_map_type type; /**< Type of memory to map */
-       enum drm_map_flags flags;       /**< Flags */
-       u32 handle;             /**< User-space: "Handle" to pass to mmap() */
-       int mtrr;               /**< MTRR slot used */
+       u32 offset;             /* Requested physical address (0 for SAREA) */
+       u32 size;               /* Requested physical size (bytes) */
+       enum drm_map_type type; /* Type of memory to map */
+       enum drm_map_flags flags;       /* Flags */
+       u32 handle;             /* User-space: "Handle" to pass to mmap() */
+       int mtrr;               /* MTRR slot used */
 } drm_map32_t;
 
 static int compat_drm_getmap(struct file *file, unsigned int cmd,
@@ -286,12 +286,12 @@ static int compat_drm_rmmap(struct file *file, unsigned int cmd,
 }
 
 typedef struct drm_client32 {
-       int idx;        /**< Which client desired? */
-       int auth;       /**< Is client authenticated? */
-       u32 pid;        /**< Process ID */
-       u32 uid;        /**< User ID */
-       u32 magic;      /**< Magic */
-       u32 iocs;       /**< Ioctl count */
+       int idx;        /* Which client desired? */
+       int auth;       /* Is client authenticated? */
+       u32 pid;        /* Process ID */
+       u32 uid;        /* User ID */
+       u32 magic;      /* Magic */
+       u32 iocs;       /* Ioctl count */
 } drm_client32_t;
 
 static int compat_drm_getclient(struct file *file, unsigned int cmd,
@@ -366,12 +366,12 @@ static int compat_drm_getstats(struct file *file, unsigned int cmd,
 }
 
 typedef struct drm_buf_desc32 {
-       int count;               /**< Number of buffers of this size */
-       int size;                /**< Size in bytes */
-       int low_mark;            /**< Low water mark */
-       int high_mark;           /**< High water mark */
+       int count;               /* Number of buffers of this size */
+       int size;                /* Size in bytes */
+       int low_mark;            /* Low water mark */
+       int high_mark;           /* High water mark */
        int flags;
-       u32 agp_start;           /**< Start address in the AGP aperture */
+       u32 agp_start;           /* Start address in the AGP aperture */
 } drm_buf_desc32_t;
 
 static int compat_drm_addbufs(struct file *file, unsigned int cmd,
@@ -1111,13 +1111,18 @@ static drm_ioctl_compat_t *drm_compat_ioctls[] = {
 };
 
 /**
- * Called whenever a 32-bit process running under a 64-bit kernel
- * performs an ioctl on /dev/drm.
+ * drm_compat_ioctl - 32bit IOCTL compatibility handler for DRM drivers
+ * @filp: file this ioctl is called on
+ * @cmd: ioctl cmd number
+ * @arg: user argument
+ *
+ * Compatibility handler for 32 bit userspace running on 64 kernels. All actual
+ * IOCTL handling is forwarded to drm_ioctl(), while marshalling structures as
+ * appropriate. Note that this only handles DRM core IOCTLs, if the driver has
+ * botched IOCTL itself, it must handle those by wrapping this function.
  *
- * \param file_priv DRM file private.
- * \param cmd command.
- * \param arg user argument.
- * \return zero on success or negative number on failure.
+ * Returns:
+ * Zero on success, negative error code on failure.
  */
 long drm_compat_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
 {
@@ -1141,5 +1146,4 @@ long drm_compat_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
 
        return ret;
 }
-
 EXPORT_SYMBOL(drm_compat_ioctl);
index 7d6deaa..9f4241f 100644 (file)
@@ -647,13 +647,59 @@ static const struct drm_ioctl_desc drm_ioctls[] = {
 #define DRM_CORE_IOCTL_COUNT   ARRAY_SIZE( drm_ioctls )
 
 /**
+ * DOC: driver specific ioctls
+ *
+ * First things first, driver private IOCTLs should only be needed for drivers
+ * supporting rendering. Kernel modesetting is all standardized, and extended
+ * through properties. There are a few exceptions in some existing drivers,
+ * which define IOCTL for use by the display DRM master, but they all predate
+ * properties.
+ *
+ * Now if you do have a render driver you always have to support it through
+ * driver private properties. There's a few steps needed to wire all the things
+ * up.
+ *
+ * First you need to define the structure for your IOCTL in your driver private
+ * UAPI header in ``include/uapi/drm/my_driver_drm.h``::
+ *
+ *     struct my_driver_operation {
+ *             u32 some_thing;
+ *             u32 another_thing;
+ *     };
+ *
+ * Please make sure that you follow all the best practices from
+ * ``Documentation/ioctl/botching-up-ioctls.txt``. Note that drm_ioctl()
+ * automatically zero-extends structures, hence make sure you can add more stuff
+ * at the end, i.e. don't put a variable sized array there.
+ *
+ * Then you need to define your IOCTL number, using one of DRM_IO(), DRM_IOR(),
+ * DRM_IOW() or DRM_IOWR(). It must start with the DRM_IOCTL\_ prefix::
+ *
+ *     ##define DRM_IOCTL_MY_DRIVER_OPERATION \
+ *         DRM_IOW(DRM_COMMAND_BASE, struct my_driver_operation)
+ * 
+ * DRM driver private IOCTL must be in the range from DRM_COMMAND_BASE to
+ * DRM_COMMAND_END. Finally you need an array of &struct drm_ioctl_desc to wire
+ * up the handlers and set the access rights:
+ *
+ *     static const struct drm_ioctl_desc my_driver_ioctls[] = {
+ *         DRM_IOCTL_DEF_DRV(MY_DRIVER_OPERATION, my_driver_operation,
+ *                 DRM_AUTH|DRM_RENDER_ALLOW),
+ *     };
+ *
+ * And then assign this to the &drm_driver.ioctls field in your driver
+ * structure.
+ */
+
+/**
  * drm_ioctl - ioctl callback implementation for DRM drivers
  * @filp: file this ioctl is called on
  * @cmd: ioctl cmd number
  * @arg: user argument
  *
- * Looks up the ioctl function in the ::ioctls table, checking for root
- * previleges if so required, and dispatches to the respective function.
+ * Looks up the ioctl function in the DRM core and the driver dispatch table,
+ * stored in &drm_driver.ioctls. It checks for necessary permission by calling
+ * drm_ioctl_permit(), and dispatches to the respective function.
  *
  * Returns:
  * Zero on success, negative error code on failure.
index f17ee07..ee03b3c 100644 (file)
@@ -33,6 +33,7 @@
 #define _DRM_IOCTL_H_
 
 #include <linux/types.h>
+#include <linux/bitops.h>
 
 #include <asm/ioctl.h>
 
@@ -41,41 +42,126 @@ struct drm_file;
 struct file;
 
 /**
- * Ioctl function type.
+ * drm_ioctl_t - DRM ioctl function type.
+ * @dev: DRM device inode
+ * @data: private pointer of the ioctl call
+ * @file_priv: DRM file this ioctl was made on
  *
- * \param inode device inode.
- * \param file_priv DRM file private pointer.
- * \param cmd command.
- * \param arg argument.
+ * This is the DRM ioctl typedef. Note that drm_ioctl() has alrady copied @data
+ * into kernel-space, and will also copy it back, depending upon the read/write
+ * settings in the ioctl command code.
  */
 typedef int drm_ioctl_t(struct drm_device *dev, void *data,
                        struct drm_file *file_priv);
 
+/**
+ * drm_ioctl_compat_t - compatibility DRM ioctl function type.
+ * @filp: file pointer
+ * @cmd: ioctl command code
+ * @arg: DRM file this ioctl was made on
+ *
+ * Just a typedef to make declaring an array of compatibility handlers easier.
+ * New drivers shouldn't screw up the structure layout for their ioctl
+ * structures and hence never need this.
+ */
 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
+/**
+ * enum drm_ioctl_flags - DRM ioctl flags
+ *
+ * Various flags that can be set in &drm_ioctl_desc.flags to control how
+ * userspace can use a given ioctl.
+ */
+enum drm_ioctl_flags {
+       /**
+        * @DRM_AUTH:
+        *
+        * This is for ioctl which are used for rendering, and require that the
+        * file descriptor is either for a render node, or if it's a
+        * legacy/primary node, then it must be authenticated.
+        */
+       DRM_AUTH                = BIT(0),
+       /**
+        * @DRM_MASTER:
+        *
+        * This must be set for any ioctl which can change the modeset or
+        * display state. Userspace must call the ioctl through a primary node,
+        * while it is the active master.
+        *
+        * Note that read-only modeset ioctl can also be called by
+        * unauthenticated clients, or when a master is not the currently active
+        * one.
+        */
+       DRM_MASTER              = BIT(1),
+       /**
+        * @DRM_ROOT_ONLY:
+        *
+        * Anything that could potentially wreak a master file descriptor needs
+        * to have this flag set. Current that's only for the SETMASTER and
+        * DROPMASTER ioctl, which e.g. logind can call to force a non-behaving
+        * master (display compositor) into compliance.
+        *
+        * This is equivalent to callers with the SYSADMIN capability.
+        */
+       DRM_ROOT_ONLY           = BIT(2),
+       /**
+        * @DRM_CONTROL_ALLOW:
+        *
+        * Deprecated, do not use. Control nodes are in the process of getting
+        * removed.
+        */
+       DRM_CONTROL_ALLOW       = BIT(3),
+       /**
+        * @DRM_UNLOCKED:
+        *
+        * Whether &drm_ioctl_desc.func should be called with the DRM BKL held
+        * or not. Enforced as the default for all modern drivers, hence there
+        * should never be a need to set this flag.
+        */
+       DRM_UNLOCKED            = BIT(4),
+       /**
+        * @DRM_RENDER_ALLOW:
+        *
+        * This is used for all ioctl needed for rendering only, for drivers
+        * which support render nodes. This should be all new render drivers,
+        * and hence it should be always set for any ioctl with DRM_AUTH set.
+        * Note though that read-only query ioctl might have this set, but have
+        * not set DRM_AUTH because they do not require authentication.
+        */
+       DRM_RENDER_ALLOW        = BIT(5),
+};
 
+/**
+ * struct drm_ioctl_desc - DRM driver ioctl entry
+ * @cmd: ioctl command number, without flags
+ * @flags: a bitmask of &enum drm_ioctl_flags
+ * @func: handler for this ioctl
+ * @name: user-readable name for debug output
+ *
+ * For convenience it's easier to create these using the DRM_IOCTL_DEF_DRV()
+ * macro.
+ */
 struct drm_ioctl_desc {
        unsigned int cmd;
-       int flags;
+       enum drm_ioctl_flags 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().
+ * DRM_IOCTL_DEF_DRV() - helper macro to fill out a &struct drm_ioctl_desc
+ * @ioctl: ioctl command suffix
+ * @_func: handler for the ioctl
+ * @_flags: a bitmask of &enum drm_ioctl_flags
+ *
+ * Small helper macro to create a &struct drm_ioctl_desc entry. The ioctl
+ * command number is constructed by prepending ``DRM_IOCTL\_`` and passing that
+ * to DRM_IOCTL_NR().
  */
-
 #define DRM_IOCTL_DEF_DRV(ioctl, _func, _flags)                                \
        [DRM_IOCTL_NR(DRM_IOCTL_##ioctl) - DRM_COMMAND_BASE] = {        \
                .cmd = DRM_IOCTL_##ioctl,                               \