[PATCH 0/6] drm: Ioctl code cleanups

[PATCH 0/6] drm: Ioctl code cleanups

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

In response to a recently noticed fumble with some i915 ioctls, I
proceeded to read the ioctl code again and didn't really like what
I saw. So I tried to clean it up a bit to make it less confusing.

grep also tells me the drm_ioctl pattern has been dutifully copied
into amdkfd so it could perhaps use the same treatment too, but I
don't have much interest in going there so left it alone.

Ville Syrjälä (6):
  drm: Fix DRM_IOCTL_DEF_DRV()
  drm: Drop ioctl->cmd_drv
  drm/vmwgfx: Replace VMW_IOCTL_DEF with DRM_IOCTL_DEF_DRV
  drm: Simplify core vs. drv ioctl handling
  drm: Use max() to make the ioctl alloc size code cleaner
  drm: Rewrite drm_ioctl_flags() to resemble the new drm_ioctl() code

 drivers/gpu/drm/drm_ioctl.c         |  53 ++++++--------
 drivers/gpu/drm/vmwgfx/vmwgfx_drv.c | 136 +++++++++++++++++-------------------
 include/drm/drmP.h                  |   3 +-
 3 files changed, 87 insertions(+), 105 deletions(-)

-- 
2.0.5

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 1/6] drm: Fix DRM_IOCTL_DEF_DRV()

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Currently DRM_IOCTL_DEF_DRV does '[DRM_IOCTL_NR(DRM_##ioctl)]' which
doesn't make much sense since DRM_##ioctl is already a the raw ioctl
number. So change it to 'DRM_IOCTL_NR(DRM_IOCTL_##ioctl) - DRM_COMMAND_BASE'
which means the DRM_IOCTL_NR() now makes sense, and also this also means
if there's a mistake in the DRM_IOCTL_##ioctl macros we might get a
warning about it (eg. we would have gotten a sparse warning about the
i915 colorkey get/set ioctl being defined to be the same thing).

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 include/drm/drmP.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/drm/drmP.h b/include/drm/drmP.h
index 63c0b01..6195ee9b 100644
--- a/include/drm/drmP.h
+++ b/include/drm/drmP.h
@@ -263,7 +263,7 @@ struct drm_ioctl_desc {
  */
 
 #define DRM_IOCTL_DEF_DRV(ioctl, _func, _flags)			\
-	[DRM_IOCTL_NR(DRM_##ioctl)] = {.cmd = DRM_##ioctl, .func = _func, .flags = _flags, .cmd_drv = DRM_IOCTL_##ioctl, .name = #ioctl}
+	[DRM_IOCTL_NR(DRM_IOCTL_##ioctl) - DRM_COMMAND_BASE] = {.cmd = DRM_##ioctl, .func = _func, .flags = _flags, .cmd_drv = DRM_IOCTL_##ioctl, .name = #ioctl}
 
 /* Event queued up for userspace to read */
 struct drm_pending_event {
-- 
2.0.5

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 2/6] drm: Drop ioctl->cmd_drv

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

ioctl->cmd_drv is pointless and we can just as well stick the full ioctl
definition into ioctl->cmd.

Cc: Jakob Bornecrantz <jakob@vmware.com>
Cc: Thomas Hellstrom <thellstrom@vmware.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/drm_ioctl.c         | 6 +++---
 drivers/gpu/drm/vmwgfx/vmwgfx_drv.c | 4 ++--
 include/drm/drmP.h                  | 3 +--
 3 files changed, 6 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c
index a6d773a..fc0cf4a 100644
--- a/drivers/gpu/drm/drm_ioctl.c
+++ b/drivers/gpu/drm/drm_ioctl.c
@@ -525,7 +525,7 @@ static int drm_ioctl_permit(u32 flags, struct drm_file *file_priv)
 }
 
 #define DRM_IOCTL_DEF(ioctl, _func, _flags) \
-	[DRM_IOCTL_NR(ioctl)] = {.cmd = ioctl, .func = _func, .flags = _flags, .cmd_drv = 0, .name = #ioctl}
+	[DRM_IOCTL_NR(ioctl)] = {.cmd = ioctl, .func = _func, .flags = _flags, .name = #ioctl}
 
 /** Ioctl table */
 static const struct drm_ioctl_desc drm_ioctls[] = {
@@ -677,11 +677,11 @@ long drm_ioctl(struct file *filp,
 	    (nr < DRM_COMMAND_BASE + dev->driver->num_ioctls)) {
 		u32 drv_size;
 		ioctl = &dev->driver->ioctls[nr - DRM_COMMAND_BASE];
-		drv_size = _IOC_SIZE(ioctl->cmd_drv);
+		drv_size = _IOC_SIZE(ioctl->cmd);
 		usize = asize = _IOC_SIZE(cmd);
 		if (drv_size > asize)
 			asize = drv_size;
-		cmd = ioctl->cmd_drv;
+		cmd = ioctl->cmd;
 	}
 	else if ((nr >= DRM_COMMAND_END) || (nr < DRM_COMMAND_BASE)) {
 		u32 drv_size;
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
index e13b9cb..620bb5c 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
@@ -134,7 +134,7 @@
  */
 
 #define VMW_IOCTL_DEF(ioctl, func, flags) \
-  [DRM_IOCTL_NR(DRM_IOCTL_##ioctl) - DRM_COMMAND_BASE] = {DRM_##ioctl, flags, func, DRM_IOCTL_##ioctl}
+  [DRM_IOCTL_NR(DRM_IOCTL_##ioctl) - DRM_COMMAND_BASE] = {DRM_IOCTL_##ioctl, flags, func}
 
 /**
  * Ioctl definitions.
@@ -1044,7 +1044,7 @@ static long vmw_generic_ioctl(struct file *filp, unsigned int cmd,
 		const struct drm_ioctl_desc *ioctl =
 			&vmw_ioctls[nr - DRM_COMMAND_BASE];
 
-		if (unlikely(ioctl->cmd_drv != cmd)) {
+		if (unlikely(ioctl->cmd != cmd)) {
 			DRM_ERROR("Invalid command format, ioctl %d\n",
 				  nr - DRM_COMMAND_BASE);
 			return -EINVAL;
diff --git a/include/drm/drmP.h b/include/drm/drmP.h
index 6195ee9b..0d501ed 100644
--- a/include/drm/drmP.h
+++ b/include/drm/drmP.h
@@ -253,7 +253,6 @@ struct drm_ioctl_desc {
 	unsigned int cmd;
 	int flags;
 	drm_ioctl_t *func;
-	unsigned int cmd_drv;
 	const char *name;
 };
 
@@ -263,7 +262,7 @@ struct drm_ioctl_desc {
  */
 
 #define DRM_IOCTL_DEF_DRV(ioctl, _func, _flags)			\
-	[DRM_IOCTL_NR(DRM_IOCTL_##ioctl) - DRM_COMMAND_BASE] = {.cmd = DRM_##ioctl, .func = _func, .flags = _flags, .cmd_drv = DRM_IOCTL_##ioctl, .name = #ioctl}
+	[DRM_IOCTL_NR(DRM_IOCTL_##ioctl) - DRM_COMMAND_BASE] = {.cmd = DRM_IOCTL_##ioctl, .func = _func, .flags = _flags, .name = #ioctl}
 
 /* Event queued up for userspace to read */
 struct drm_pending_event {
-- 
2.0.5

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 3/6] drm/vmwgfx: Replace VMW_IOCTL_DEF with DRM_IOCTL_DEF_DRV

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

DRM_IOCTL_DEF_DRV is now identical to VMW_IOCTL_DEF (apart from the
.name assignment), so just replace the use of the latter with the former.

With .name now assigned this should also make decoding kernel logs a
bit easier.

Cc: Jakob Bornecrantz <jakob@vmware.com>
Cc: Thomas Hellstrom <thellstrom@vmware.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/vmwgfx/vmwgfx_drv.c | 134 +++++++++++++++++-------------------
 1 file changed, 63 insertions(+), 71 deletions(-)

diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
index 620bb5c..ef74c4a 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
@@ -129,83 +129,75 @@
 		 struct drm_vmw_synccpu_arg)
 
 /**
- * The core DRM version of this macro doesn't account for
- * DRM_COMMAND_BASE.
- */
-
-#define VMW_IOCTL_DEF(ioctl, func, flags) \
-  [DRM_IOCTL_NR(DRM_IOCTL_##ioctl) - DRM_COMMAND_BASE] = {DRM_IOCTL_##ioctl, flags, func}
-
-/**
  * Ioctl definitions.
  */
 
 static const struct drm_ioctl_desc vmw_ioctls[] = {
-	VMW_IOCTL_DEF(VMW_GET_PARAM, vmw_getparam_ioctl,
-		      DRM_AUTH | DRM_UNLOCKED | DRM_RENDER_ALLOW),
-	VMW_IOCTL_DEF(VMW_ALLOC_DMABUF, vmw_dmabuf_alloc_ioctl,
-		      DRM_AUTH | DRM_UNLOCKED | DRM_RENDER_ALLOW),
-	VMW_IOCTL_DEF(VMW_UNREF_DMABUF, vmw_dmabuf_unref_ioctl,
-		      DRM_UNLOCKED | DRM_RENDER_ALLOW),
-	VMW_IOCTL_DEF(VMW_CURSOR_BYPASS,
-		      vmw_kms_cursor_bypass_ioctl,
-		      DRM_MASTER | DRM_CONTROL_ALLOW | DRM_UNLOCKED),
-
-	VMW_IOCTL_DEF(VMW_CONTROL_STREAM, vmw_overlay_ioctl,
-		      DRM_MASTER | DRM_CONTROL_ALLOW | DRM_UNLOCKED),
-	VMW_IOCTL_DEF(VMW_CLAIM_STREAM, vmw_stream_claim_ioctl,
-		      DRM_MASTER | DRM_CONTROL_ALLOW | DRM_UNLOCKED),
-	VMW_IOCTL_DEF(VMW_UNREF_STREAM, vmw_stream_unref_ioctl,
-		      DRM_MASTER | DRM_CONTROL_ALLOW | DRM_UNLOCKED),
-
-	VMW_IOCTL_DEF(VMW_CREATE_CONTEXT, vmw_context_define_ioctl,
-		      DRM_AUTH | DRM_UNLOCKED | DRM_RENDER_ALLOW),
-	VMW_IOCTL_DEF(VMW_UNREF_CONTEXT, vmw_context_destroy_ioctl,
-		      DRM_UNLOCKED | DRM_RENDER_ALLOW),
-	VMW_IOCTL_DEF(VMW_CREATE_SURFACE, vmw_surface_define_ioctl,
-		      DRM_AUTH | DRM_UNLOCKED | DRM_RENDER_ALLOW),
-	VMW_IOCTL_DEF(VMW_UNREF_SURFACE, vmw_surface_destroy_ioctl,
-		      DRM_UNLOCKED | DRM_RENDER_ALLOW),
-	VMW_IOCTL_DEF(VMW_REF_SURFACE, vmw_surface_reference_ioctl,
-		      DRM_AUTH | DRM_UNLOCKED | DRM_RENDER_ALLOW),
-	VMW_IOCTL_DEF(VMW_EXECBUF, vmw_execbuf_ioctl,
-		      DRM_AUTH | DRM_UNLOCKED | DRM_RENDER_ALLOW),
-	VMW_IOCTL_DEF(VMW_FENCE_WAIT, vmw_fence_obj_wait_ioctl,
-		      DRM_UNLOCKED | DRM_RENDER_ALLOW),
-	VMW_IOCTL_DEF(VMW_FENCE_SIGNALED,
-		      vmw_fence_obj_signaled_ioctl,
-		      DRM_UNLOCKED | DRM_RENDER_ALLOW),
-	VMW_IOCTL_DEF(VMW_FENCE_UNREF, vmw_fence_obj_unref_ioctl,
-		      DRM_UNLOCKED | DRM_RENDER_ALLOW),
-	VMW_IOCTL_DEF(VMW_FENCE_EVENT, vmw_fence_event_ioctl,
-		      DRM_AUTH | DRM_UNLOCKED | DRM_RENDER_ALLOW),
-	VMW_IOCTL_DEF(VMW_GET_3D_CAP, vmw_get_cap_3d_ioctl,
-		      DRM_AUTH | DRM_UNLOCKED | DRM_RENDER_ALLOW),
+	DRM_IOCTL_DEF_DRV(VMW_GET_PARAM, vmw_getparam_ioctl,
+			  DRM_AUTH | DRM_UNLOCKED | DRM_RENDER_ALLOW),
+	DRM_IOCTL_DEF_DRV(VMW_ALLOC_DMABUF, vmw_dmabuf_alloc_ioctl,
+			  DRM_AUTH | DRM_UNLOCKED | DRM_RENDER_ALLOW),
+	DRM_IOCTL_DEF_DRV(VMW_UNREF_DMABUF, vmw_dmabuf_unref_ioctl,
+			  DRM_UNLOCKED | DRM_RENDER_ALLOW),
+	DRM_IOCTL_DEF_DRV(VMW_CURSOR_BYPASS,
+			  vmw_kms_cursor_bypass_ioctl,
+			  DRM_MASTER | DRM_CONTROL_ALLOW | DRM_UNLOCKED),
+
+	DRM_IOCTL_DEF_DRV(VMW_CONTROL_STREAM, vmw_overlay_ioctl,
+			  DRM_MASTER | DRM_CONTROL_ALLOW | DRM_UNLOCKED),
+	DRM_IOCTL_DEF_DRV(VMW_CLAIM_STREAM, vmw_stream_claim_ioctl,
+			  DRM_MASTER | DRM_CONTROL_ALLOW | DRM_UNLOCKED),
+	DRM_IOCTL_DEF_DRV(VMW_UNREF_STREAM, vmw_stream_unref_ioctl,
+			  DRM_MASTER | DRM_CONTROL_ALLOW | DRM_UNLOCKED),
+
+	DRM_IOCTL_DEF_DRV(VMW_CREATE_CONTEXT, vmw_context_define_ioctl,
+			  DRM_AUTH | DRM_UNLOCKED | DRM_RENDER_ALLOW),
+	DRM_IOCTL_DEF_DRV(VMW_UNREF_CONTEXT, vmw_context_destroy_ioctl,
+			  DRM_UNLOCKED | DRM_RENDER_ALLOW),
+	DRM_IOCTL_DEF_DRV(VMW_CREATE_SURFACE, vmw_surface_define_ioctl,
+			  DRM_AUTH | DRM_UNLOCKED | DRM_RENDER_ALLOW),
+	DRM_IOCTL_DEF_DRV(VMW_UNREF_SURFACE, vmw_surface_destroy_ioctl,
+			  DRM_UNLOCKED | DRM_RENDER_ALLOW),
+	DRM_IOCTL_DEF_DRV(VMW_REF_SURFACE, vmw_surface_reference_ioctl,
+			  DRM_AUTH | DRM_UNLOCKED | DRM_RENDER_ALLOW),
+	DRM_IOCTL_DEF_DRV(VMW_EXECBUF, vmw_execbuf_ioctl,
+			  DRM_AUTH | DRM_UNLOCKED | DRM_RENDER_ALLOW),
+	DRM_IOCTL_DEF_DRV(VMW_FENCE_WAIT, vmw_fence_obj_wait_ioctl,
+			  DRM_UNLOCKED | DRM_RENDER_ALLOW),
+	DRM_IOCTL_DEF_DRV(VMW_FENCE_SIGNALED,
+			  vmw_fence_obj_signaled_ioctl,
+			  DRM_UNLOCKED | DRM_RENDER_ALLOW),
+	DRM_IOCTL_DEF_DRV(VMW_FENCE_UNREF, vmw_fence_obj_unref_ioctl,
+			  DRM_UNLOCKED | DRM_RENDER_ALLOW),
+	DRM_IOCTL_DEF_DRV(VMW_FENCE_EVENT, vmw_fence_event_ioctl,
+			  DRM_AUTH | DRM_UNLOCKED | DRM_RENDER_ALLOW),
+	DRM_IOCTL_DEF_DRV(VMW_GET_3D_CAP, vmw_get_cap_3d_ioctl,
+			  DRM_AUTH | DRM_UNLOCKED | DRM_RENDER_ALLOW),
 
 	/* these allow direct access to the framebuffers mark as master only */
-	VMW_IOCTL_DEF(VMW_PRESENT, vmw_present_ioctl,
-		      DRM_MASTER | DRM_AUTH | DRM_UNLOCKED),
-	VMW_IOCTL_DEF(VMW_PRESENT_READBACK,
-		      vmw_present_readback_ioctl,
-		      DRM_MASTER | DRM_AUTH | DRM_UNLOCKED),
-	VMW_IOCTL_DEF(VMW_UPDATE_LAYOUT,
-		      vmw_kms_update_layout_ioctl,
-		      DRM_MASTER | DRM_UNLOCKED),
-	VMW_IOCTL_DEF(VMW_CREATE_SHADER,
-		      vmw_shader_define_ioctl,
-		      DRM_AUTH | DRM_UNLOCKED | DRM_RENDER_ALLOW),
-	VMW_IOCTL_DEF(VMW_UNREF_SHADER,
-		      vmw_shader_destroy_ioctl,
-		      DRM_UNLOCKED | DRM_RENDER_ALLOW),
-	VMW_IOCTL_DEF(VMW_GB_SURFACE_CREATE,
-		      vmw_gb_surface_define_ioctl,
-		      DRM_AUTH | DRM_UNLOCKED | DRM_RENDER_ALLOW),
-	VMW_IOCTL_DEF(VMW_GB_SURFACE_REF,
-		      vmw_gb_surface_reference_ioctl,
-		      DRM_AUTH | DRM_UNLOCKED | DRM_RENDER_ALLOW),
-	VMW_IOCTL_DEF(VMW_SYNCCPU,
-		      vmw_user_dmabuf_synccpu_ioctl,
-		      DRM_UNLOCKED | DRM_RENDER_ALLOW),
+	DRM_IOCTL_DEF_DRV(VMW_PRESENT, vmw_present_ioctl,
+			  DRM_MASTER | DRM_AUTH | DRM_UNLOCKED),
+	DRM_IOCTL_DEF_DRV(VMW_PRESENT_READBACK,
+			  vmw_present_readback_ioctl,
+			  DRM_MASTER | DRM_AUTH | DRM_UNLOCKED),
+	DRM_IOCTL_DEF_DRV(VMW_UPDATE_LAYOUT,
+			  vmw_kms_update_layout_ioctl,
+			  DRM_MASTER | DRM_UNLOCKED),
+	DRM_IOCTL_DEF_DRV(VMW_CREATE_SHADER,
+			  vmw_shader_define_ioctl,
+			  DRM_AUTH | DRM_UNLOCKED | DRM_RENDER_ALLOW),
+	DRM_IOCTL_DEF_DRV(VMW_UNREF_SHADER,
+			  vmw_shader_destroy_ioctl,
+			  DRM_UNLOCKED | DRM_RENDER_ALLOW),
+	DRM_IOCTL_DEF_DRV(VMW_GB_SURFACE_CREATE,
+			  vmw_gb_surface_define_ioctl,
+			  DRM_AUTH | DRM_UNLOCKED | DRM_RENDER_ALLOW),
+	DRM_IOCTL_DEF_DRV(VMW_GB_SURFACE_REF,
+			  vmw_gb_surface_reference_ioctl,
+			  DRM_AUTH | DRM_UNLOCKED | DRM_RENDER_ALLOW),
+	DRM_IOCTL_DEF_DRV(VMW_SYNCCPU,
+			  vmw_user_dmabuf_synccpu_ioctl,
+			  DRM_UNLOCKED | DRM_RENDER_ALLOW),
 };
 
 static struct pci_device_id vmw_pci_id_list[] = {
-- 
2.0.5

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 4/6] drm: Simplify core vs. drv ioctl handling

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Now that cmd_drv is gone the handling for core and driver ioctls is
mostly identical, so eliminate the duplication. Also take the
opportunity to simplify the range checks to be less cluttered.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/drm_ioctl.c | 39 +++++++++++++++------------------------
 1 file changed, 15 insertions(+), 24 deletions(-)

diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c
index fc0cf4a..68ba014 100644
--- a/drivers/gpu/drm/drm_ioctl.c
+++ b/drivers/gpu/drm/drm_ioctl.c
@@ -663,39 +663,30 @@ long drm_ioctl(struct file *filp,
 	int retcode = -EINVAL;
 	char stack_kdata[128];
 	char *kdata = NULL;
-	unsigned int usize, asize;
+	unsigned int usize, asize, drv_size;
 
 	dev = file_priv->minor->dev;
 
 	if (drm_device_is_unplugged(dev))
 		return -ENODEV;
 
-	if ((nr >= DRM_CORE_IOCTL_COUNT) &&
-	    ((nr < DRM_COMMAND_BASE) || (nr >= DRM_COMMAND_END)))
-		goto err_i1;
-	if ((nr >= DRM_COMMAND_BASE) && (nr < DRM_COMMAND_END) &&
-	    (nr < DRM_COMMAND_BASE + dev->driver->num_ioctls)) {
-		u32 drv_size;
+	if (nr >= DRM_COMMAND_BASE && nr < DRM_COMMAND_END) {
+		/* driver ioctl */
+		if (nr - DRM_COMMAND_BASE >= dev->driver->num_ioctls)
+			goto err_i1;
 		ioctl = &dev->driver->ioctls[nr - DRM_COMMAND_BASE];
-		drv_size = _IOC_SIZE(ioctl->cmd);
-		usize = asize = _IOC_SIZE(cmd);
-		if (drv_size > asize)
-			asize = drv_size;
-		cmd = ioctl->cmd;
-	}
-	else if ((nr >= DRM_COMMAND_END) || (nr < DRM_COMMAND_BASE)) {
-		u32 drv_size;
-
+	} else {
+		/* core ioctl */
+		if (nr >= DRM_CORE_IOCTL_COUNT)
+			goto err_i1;
 		ioctl = &drm_ioctls[nr];
+	}
 
-		drv_size = _IOC_SIZE(ioctl->cmd);
-		usize = asize = _IOC_SIZE(cmd);
-		if (drv_size > asize)
-			asize = drv_size;
-
-		cmd = ioctl->cmd;
-	} else
-		goto err_i1;
+	drv_size = _IOC_SIZE(ioctl->cmd);
+	usize = asize = _IOC_SIZE(cmd);
+	if (drv_size > asize)
+		asize = drv_size;
+	cmd = ioctl->cmd;
 
 	DRM_DEBUG("pid=%d, dev=0x%lx, auth=%d, %s\n",
 		  task_pid_nr(current),
-- 
2.0.5

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 5/6] drm: Use max() to make the ioctl alloc size code cleaner

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Use max() to make the code to determine the allocation size for
the ioctl data easier to read.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/drm_ioctl.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c
index 68ba014..4a2bf2b 100644
--- a/drivers/gpu/drm/drm_ioctl.c
+++ b/drivers/gpu/drm/drm_ioctl.c
@@ -683,9 +683,8 @@ long drm_ioctl(struct file *filp,
 	}
 
 	drv_size = _IOC_SIZE(ioctl->cmd);
-	usize = asize = _IOC_SIZE(cmd);
-	if (drv_size > asize)
-		asize = drv_size;
+	usize = _IOC_SIZE(cmd);
+	asize = max(usize, drv_size);
 	cmd = ioctl->cmd;
 
 	DRM_DEBUG("pid=%d, dev=0x%lx, auth=%d, %s\n",
-- 
2.0.5

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 6/6] drm: Rewrite drm_ioctl_flags() to resemble the new drm_ioctl() code

[ville.syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Use the same logic when checking for valid ioctl range in
drm_ioctl_flags() that is used in drm_ioctl() to avoid
confusion.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/drm_ioctl.c | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c
index 4a2bf2b..1f257ae 100644
--- a/drivers/gpu/drm/drm_ioctl.c
+++ b/drivers/gpu/drm/drm_ioctl.c
@@ -766,12 +766,13 @@ EXPORT_SYMBOL(drm_ioctl);
  */
 bool drm_ioctl_flags(unsigned int nr, unsigned int *flags)
 {
-	if ((nr >= DRM_COMMAND_END && nr < DRM_CORE_IOCTL_COUNT) ||
-	    (nr < DRM_COMMAND_BASE)) {
-		*flags = drm_ioctls[nr].flags;
-		return true;
-	}
+	if (nr >= DRM_COMMAND_BASE && nr < DRM_COMMAND_END)
+		return false;
+
+	if (nr >= DRM_CORE_IOCTL_COUNT)
+		return false;
 
-	return false;
+	*flags = drm_ioctls[nr].flags;
+	return true;
 }
 EXPORT_SYMBOL(drm_ioctl_flags);
-- 
2.0.5

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 0/6] drm: Ioctl code cleanups

[Daniel Vetter]

On Fri, Mar 27, 2015 at 03:51:54PM +0200, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> In response to a recently noticed fumble with some i915 ioctls, I
> proceeded to read the ioctl code again and didn't really like what
> I saw. So I tried to clean it up a bit to make it less confusing.
> 
> grep also tells me the drm_ioctl pattern has been dutifully copied
> into amdkfd so it could perhaps use the same treatment too, but I
> don't have much interest in going there so left it alone.
> 
> Ville Syrjälä (6):
>   drm: Fix DRM_IOCTL_DEF_DRV()
>   drm: Drop ioctl->cmd_drv
>   drm/vmwgfx: Replace VMW_IOCTL_DEF with DRM_IOCTL_DEF_DRV
>   drm: Simplify core vs. drv ioctl handling
>   drm: Use max() to make the ioctl alloc size code cleaner
>   drm: Rewrite drm_ioctl_flags() to resemble the new drm_ioctl() code

Merged all except the pure vmwgfx one to drm-misc, thanks. If Thomas wants
I can pull that one in too.
-Daniel

> 
>  drivers/gpu/drm/drm_ioctl.c         |  53 ++++++--------
>  drivers/gpu/drm/vmwgfx/vmwgfx_drv.c | 136 +++++++++++++++++-------------------
>  include/drm/drmP.h                  |   3 +-
>  3 files changed, 87 insertions(+), 105 deletions(-)
> 
> -- 
> 2.0.5
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel

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

Re: [PATCH 1/6] drm: Fix DRM_IOCTL_DEF_DRV()

[Emil Velikov]

Hi Ville,

On 27 March 2015 at 13:51,  <ville.syrjala@linux.intel.com> wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Currently DRM_IOCTL_DEF_DRV does '[DRM_IOCTL_NR(DRM_##ioctl)]' which
> doesn't make much sense since DRM_##ioctl is already a the raw ioctl
> number. So change it to 'DRM_IOCTL_NR(DRM_IOCTL_##ioctl) - DRM_COMMAND_BASE'
> which means the DRM_IOCTL_NR() now makes sense, and also this also means
> if there's a mistake in the DRM_IOCTL_##ioctl macros we might get a
> warning about it (eg. we would have gotten a sparse warning about the
> i915 colorkey get/set ioctl being defined to be the same thing).
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  include/drm/drmP.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/include/drm/drmP.h b/include/drm/drmP.h
> index 63c0b01..6195ee9b 100644
> --- a/include/drm/drmP.h
> +++ b/include/drm/drmP.h
> @@ -263,7 +263,7 @@ struct drm_ioctl_desc {
>   */
>
>  #define DRM_IOCTL_DEF_DRV(ioctl, _func, _flags)                        \
> -       [DRM_IOCTL_NR(DRM_##ioctl)] = {.cmd = DRM_##ioctl, .func = _func, .flags = _flags, .cmd_drv = DRM_IOCTL_##ioctl, .name = #ioctl}
> +       [DRM_IOCTL_NR(DRM_IOCTL_##ioctl) - DRM_COMMAND_BASE] = {.cmd = DRM_##ioctl, .func = _func, .flags = _flags, .cmd_drv = DRM_IOCTL_##ioctl, .name = #ioctl}
>
Humble request: Can you split this long line to something line the
following. It should improve the overall readability.
Plus is will make your follow up changes clearer.

+       [DRM_IOCTL_NR(DRM_IOCTL_##ioctl) - DRM_COMMAND_BASE] = { \
+             .cmd = DRM_##ioctl, \
+             .func = _func, \
+             .flags = _flags, \
+             .cmd_drv = DRM_IOCTL_##ioctl, \
+             .name = #ioctl \
+       }


Cheers,
Emil
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 1/6] drm: Fix DRM_IOCTL_DEF_DRV()

[Ville Syrjälä]

On Fri, Mar 27, 2015 at 03:14:10PM +0000, Emil Velikov wrote:
> Hi Ville,
> 
> On 27 March 2015 at 13:51,  <ville.syrjala@linux.intel.com> wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >
> > Currently DRM_IOCTL_DEF_DRV does '[DRM_IOCTL_NR(DRM_##ioctl)]' which
> > doesn't make much sense since DRM_##ioctl is already a the raw ioctl
> > number. So change it to 'DRM_IOCTL_NR(DRM_IOCTL_##ioctl) - DRM_COMMAND_BASE'
> > which means the DRM_IOCTL_NR() now makes sense, and also this also means
> > if there's a mistake in the DRM_IOCTL_##ioctl macros we might get a
> > warning about it (eg. we would have gotten a sparse warning about the
> > i915 colorkey get/set ioctl being defined to be the same thing).
> >
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> >  include/drm/drmP.h | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/include/drm/drmP.h b/include/drm/drmP.h
> > index 63c0b01..6195ee9b 100644
> > --- a/include/drm/drmP.h
> > +++ b/include/drm/drmP.h
> > @@ -263,7 +263,7 @@ struct drm_ioctl_desc {
> >   */
> >
> >  #define DRM_IOCTL_DEF_DRV(ioctl, _func, _flags)                        \
> > -       [DRM_IOCTL_NR(DRM_##ioctl)] = {.cmd = DRM_##ioctl, .func = _func, .flags = _flags, .cmd_drv = DRM_IOCTL_##ioctl, .name = #ioctl}
> > +       [DRM_IOCTL_NR(DRM_IOCTL_##ioctl) - DRM_COMMAND_BASE] = {.cmd = DRM_##ioctl, .func = _func, .flags = _flags, .cmd_drv = DRM_IOCTL_##ioctl, .name = #ioctl}
> >
> Humble request: Can you split this long line to something line the
> following. It should improve the overall readability.
> Plus is will make your follow up changes clearer.
> 
> +       [DRM_IOCTL_NR(DRM_IOCTL_##ioctl) - DRM_COMMAND_BASE] = { \
> +             .cmd = DRM_##ioctl, \
> +             .func = _func, \
> +             .flags = _flags, \
> +             .cmd_drv = DRM_IOCTL_##ioctl, \
> +             .name = #ioctl \
> +       }

Daniel already picked these up, so my work is done ;) But feel free to
toss in a followup.

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH] drm: line wrap DRM_IOCTL_DEF* macros

[Emil Velikov]

Improve the readability and keeps the lines shorter than 80 columns.

Signed-off-by: Emil Velikov <emil.l.velikov@gmail.com>
---

Written against drm-intel/topic/drm-misc.

-Emil

---
 drivers/gpu/drm/drm_ioctl.c | 10 ++++++++--
 include/drm/drmP.h          |  8 +++++++-
 2 files changed, 15 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c
index a6d773a..ae50f7b 100644
--- a/drivers/gpu/drm/drm_ioctl.c
+++ b/drivers/gpu/drm/drm_ioctl.c
@@ -524,8 +524,14 @@ static int drm_ioctl_permit(u32 flags, struct drm_file *file_priv)
 	return 0;
 }
 
-#define DRM_IOCTL_DEF(ioctl, _func, _flags) \
-	[DRM_IOCTL_NR(ioctl)] = {.cmd = ioctl, .func = _func, .flags = _flags, .cmd_drv = 0, .name = #ioctl}
+#define DRM_IOCTL_DEF(ioctl, _func, _flags)	\
+	[DRM_IOCTL_NR(ioctl)] = {		\
+		.cmd = ioctl,			\
+		.func = _func,			\
+		.flags = _flags,		\
+		.cmd_drv = 0,			\
+		.name = #ioctl			\
+	}
 
 /** Ioctl table */
 static const struct drm_ioctl_desc drm_ioctls[] = {
diff --git a/include/drm/drmP.h b/include/drm/drmP.h
index 63c0b01..c95543c 100644
--- a/include/drm/drmP.h
+++ b/include/drm/drmP.h
@@ -263,7 +263,13 @@ struct drm_ioctl_desc {
  */
 
 #define DRM_IOCTL_DEF_DRV(ioctl, _func, _flags)			\
-	[DRM_IOCTL_NR(DRM_##ioctl)] = {.cmd = DRM_##ioctl, .func = _func, .flags = _flags, .cmd_drv = DRM_IOCTL_##ioctl, .name = #ioctl}
+	[DRM_IOCTL_NR(DRM_##ioctl)] = {				\
+		.cmd = DRM_##ioctl,				\
+		.func = _func,					\
+		.flags = _flags,				\
+		.cmd_drv = DRM_IOCTL_##ioctl,			\
+		.name = #ioctl					\
+	}
 
 /* Event queued up for userspace to read */
 struct drm_pending_event {
-- 
2.1.3

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm: line wrap DRM_IOCTL_DEF* macros

[Daniel Vetter]

On Mon, Mar 30, 2015 at 12:58:10PM +0000, Emil Velikov wrote:
> Improve the readability and keeps the lines shorter than 80 columns.
> 
> Signed-off-by: Emil Velikov <emil.l.velikov@gmail.com>
> ---
> 
> Written against drm-intel/topic/drm-misc.

Ah I've forgotten to push it out, silly me. Can you please rebase?

Thanks, Daniel

> 
> -Emil
> 
> ---
>  drivers/gpu/drm/drm_ioctl.c | 10 ++++++++--
>  include/drm/drmP.h          |  8 +++++++-
>  2 files changed, 15 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c
> index a6d773a..ae50f7b 100644
> --- a/drivers/gpu/drm/drm_ioctl.c
> +++ b/drivers/gpu/drm/drm_ioctl.c
> @@ -524,8 +524,14 @@ static int drm_ioctl_permit(u32 flags, struct drm_file *file_priv)
>  	return 0;
>  }
>  
> -#define DRM_IOCTL_DEF(ioctl, _func, _flags) \
> -	[DRM_IOCTL_NR(ioctl)] = {.cmd = ioctl, .func = _func, .flags = _flags, .cmd_drv = 0, .name = #ioctl}
> +#define DRM_IOCTL_DEF(ioctl, _func, _flags)	\
> +	[DRM_IOCTL_NR(ioctl)] = {		\
> +		.cmd = ioctl,			\
> +		.func = _func,			\
> +		.flags = _flags,		\
> +		.cmd_drv = 0,			\
> +		.name = #ioctl			\
> +	}
>  
>  /** Ioctl table */
>  static const struct drm_ioctl_desc drm_ioctls[] = {
> diff --git a/include/drm/drmP.h b/include/drm/drmP.h
> index 63c0b01..c95543c 100644
> --- a/include/drm/drmP.h
> +++ b/include/drm/drmP.h
> @@ -263,7 +263,13 @@ struct drm_ioctl_desc {
>   */
>  
>  #define DRM_IOCTL_DEF_DRV(ioctl, _func, _flags)			\
> -	[DRM_IOCTL_NR(DRM_##ioctl)] = {.cmd = DRM_##ioctl, .func = _func, .flags = _flags, .cmd_drv = DRM_IOCTL_##ioctl, .name = #ioctl}
> +	[DRM_IOCTL_NR(DRM_##ioctl)] = {				\
> +		.cmd = DRM_##ioctl,				\
> +		.func = _func,					\
> +		.flags = _flags,				\
> +		.cmd_drv = DRM_IOCTL_##ioctl,			\
> +		.name = #ioctl					\
> +	}
>  
>  /* Event queued up for userspace to read */
>  struct drm_pending_event {
> -- 
> 2.1.3
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel

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

[PATCH v2] drm: line wrap DRM_IOCTL_DEF* macros

[Emil Velikov]

Improve the readability and keeps the lines shorter than 80 columns.

Signed-off-by: Emil Velikov <emil.l.velikov@gmail.com>
---

v2: Rebased against drm-intel topic/drm-misc, commit 
a0211bb482c(drm/atomic: Don't try to free a NULL state)

-Emil

---
 drivers/gpu/drm/drm_ioctl.c | 9 +++++++--
 include/drm/drmP.h          | 9 +++++++--
 2 files changed, 14 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c
index 1f257ae..266dcd6 100644
--- a/drivers/gpu/drm/drm_ioctl.c
+++ b/drivers/gpu/drm/drm_ioctl.c
@@ -524,8 +524,13 @@ static int drm_ioctl_permit(u32 flags, struct drm_file *file_priv)
 	return 0;
 }
 
-#define DRM_IOCTL_DEF(ioctl, _func, _flags) \
-	[DRM_IOCTL_NR(ioctl)] = {.cmd = ioctl, .func = _func, .flags = _flags, .name = #ioctl}
+#define DRM_IOCTL_DEF(ioctl, _func, _flags)	\
+	[DRM_IOCTL_NR(ioctl)] = {		\
+		.cmd = ioctl,			\
+		.func = _func,			\
+		.flags = _flags,		\
+		.name = #ioctl			\
+	}
 
 /** Ioctl table */
 static const struct drm_ioctl_desc drm_ioctls[] = {
diff --git a/include/drm/drmP.h b/include/drm/drmP.h
index 0d501ed..62c40777 100644
--- a/include/drm/drmP.h
+++ b/include/drm/drmP.h
@@ -261,8 +261,13 @@ struct drm_ioctl_desc {
  * 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}
+#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						\
+	 }
 
 /* Event queued up for userspace to read */
 struct drm_pending_event {
-- 
2.1.3

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2] drm: line wrap DRM_IOCTL_DEF* macros

[Daniel Vetter]

On Mon, Mar 30, 2015 at 05:10:36PM +0000, Emil Velikov wrote:
> Improve the readability and keeps the lines shorter than 80 columns.
> 
> Signed-off-by: Emil Velikov <emil.l.velikov@gmail.com>

Applied to topic/drm-misc, thanks.
-Daniel

> ---
> 
> v2: Rebased against drm-intel topic/drm-misc, commit 
> a0211bb482c(drm/atomic: Don't try to free a NULL state)
> 
> -Emil
> 
> ---
>  drivers/gpu/drm/drm_ioctl.c | 9 +++++++--
>  include/drm/drmP.h          | 9 +++++++--
>  2 files changed, 14 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c
> index 1f257ae..266dcd6 100644
> --- a/drivers/gpu/drm/drm_ioctl.c
> +++ b/drivers/gpu/drm/drm_ioctl.c
> @@ -524,8 +524,13 @@ static int drm_ioctl_permit(u32 flags, struct drm_file *file_priv)
>  	return 0;
>  }
>  
> -#define DRM_IOCTL_DEF(ioctl, _func, _flags) \
> -	[DRM_IOCTL_NR(ioctl)] = {.cmd = ioctl, .func = _func, .flags = _flags, .name = #ioctl}
> +#define DRM_IOCTL_DEF(ioctl, _func, _flags)	\
> +	[DRM_IOCTL_NR(ioctl)] = {		\
> +		.cmd = ioctl,			\
> +		.func = _func,			\
> +		.flags = _flags,		\
> +		.name = #ioctl			\
> +	}
>  
>  /** Ioctl table */
>  static const struct drm_ioctl_desc drm_ioctls[] = {
> diff --git a/include/drm/drmP.h b/include/drm/drmP.h
> index 0d501ed..62c40777 100644
> --- a/include/drm/drmP.h
> +++ b/include/drm/drmP.h
> @@ -261,8 +261,13 @@ struct drm_ioctl_desc {
>   * 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}
> +#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						\
> +	 }
>  
>  /* Event queued up for userspace to read */
>  struct drm_pending_event {
> -- 
> 2.1.3
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel

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