[Intel-gfx] [PATCH 5/5] drm: Use names of ioctls in debug traces

2013-05-08 Thread Ville Syrjälä
On Wed, May 08, 2013 at 05:03:34PM +0100, Damien Lespiau wrote:
> From: Chris Cummins 
> 
> The intention here is to make the output of dmesg with full verbosity a
> bit easier for a human to parse. This commit transforms:
> 
> [drm:drm_ioctl], pid=699, cmd=0x6458, nr=0x58, dev 0xe200, auth=1
> [drm:drm_ioctl], pid=699, cmd=0xc010645b, nr=0x5b, dev 0xe200, auth=1
> [drm:drm_ioctl], pid=699, cmd=0xc0106461, nr=0x61, dev 0xe200, auth=1
> [drm:drm_ioctl], pid=699, cmd=0xc01c64ae, nr=0xae, dev 0xe200, auth=1
> [drm:drm_mode_addfb], [FB:32]
> [drm:drm_ioctl], pid=699, cmd=0xc0106464, nr=0x64, dev 0xe200, auth=1
> [drm:drm_vm_open_locked], 0x7fd9302fe000,0x00a0
> [drm:drm_ioctl], pid=699, cmd=0x400c645f, nr=0x5f, dev 0xe200, auth=1
> [drm:drm_ioctl], pid=699, cmd=0xc00464af, nr=0xaf, dev 0xe200, auth=1
> [drm:intel_crtc_set_config], [CRTC:3] [NOFB]
> 
> into:
> 
> [drm:drm_ioctl], pid=699, dev=0xe200, auth=1, I915_GEM_THROTTLE
> [drm:drm_ioctl], pid=699, dev=0xe200, auth=1, I915_GEM_CREATE
> [drm:drm_ioctl], pid=699, dev=0xe200, auth=1, I915_GEM_SET_TILING
> [drm:drm_ioctl], pid=699, dev=0xe200, auth=1, IOCTL_MODE_ADDFB
> [drm:drm_mode_addfb], [FB:32]
> [drm:drm_ioctl], pid=699, dev=0xe200, auth=1, I915_GEM_MMAP_GTT
> [drm:drm_vm_open_locked], 0x7fd9302fe000,0x00a0
> [drm:drm_ioctl], pid=699, dev=0xe200, auth=1, I915_GEM_SET_DOMAIN
> [drm:drm_ioctl], pid=699, dev=0xe200, auth=1, DRM_IOCTL_MODE_RMFB
> [drm:intel_crtc_set_config], [CRTC:3] [NOFB]

I like it. But I made drm_ioctls const recently so the patch needs a
small refresh.

BTW am I the only one who hates the DRM_IOCTL_DEF_DRV() macro? Thanks to
the cpp magic it's difficult to look up the ioctls using cscope. OTOH
with this patch looking up the numbers does become less important.

> Signed-off-by: Chris Cummins 
> ---
>  drivers/gpu/drm/drm_drv.c | 20 +---
>  include/drm/drmP.h|  3 ++-
>  2 files changed, 15 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
> index 25f91cd..0382f6e 100644
> --- a/drivers/gpu/drm/drm_drv.c
> +++ b/drivers/gpu/drm/drm_drv.c
> @@ -57,7 +57,7 @@ static int drm_version(struct drm_device *dev, void *data,
>  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}
> + [DRM_IOCTL_NR(ioctl)] = {.cmd = ioctl, .func = _func, .flags = _flags, 
> .cmd_drv = 0, .name = #ioctl}
>  
>  /** Ioctl table */
>  static struct drm_ioctl_desc drm_ioctls[] = {
> @@ -375,7 +375,7 @@ long drm_ioctl(struct file *filp,
>  {
>   struct drm_file *file_priv = filp->private_data;
>   struct drm_device *dev;
> - struct drm_ioctl_desc *ioctl;
> + struct drm_ioctl_desc *ioctl = NULL;
>   drm_ioctl_t *func;
>   unsigned int nr = DRM_IOCTL_NR(cmd);
>   int retcode = -EINVAL;
> @@ -392,11 +392,6 @@ long drm_ioctl(struct file *filp,
>   atomic_inc(>counts[_DRM_STAT_IOCTLS]);
>   ++file_priv->ioctl_count;
>  
> - DRM_DEBUG("pid=%d, cmd=0x%02x, nr=0x%02x, dev 0x%lx, auth=%d\n",
> -   task_pid_nr(current), cmd, nr,
> -   (long)old_encode_dev(file_priv->minor->device),
> -   file_priv->authenticated);
> -
>   if ((nr >= DRM_CORE_IOCTL_COUNT) &&
>   ((nr < DRM_COMMAND_BASE) || (nr >= DRM_COMMAND_END)))
>   goto err_i1;
> @@ -416,6 +411,11 @@ long drm_ioctl(struct file *filp,
>   } else
>   goto err_i1;
>  
> + DRM_DEBUG("pid=%d, dev=0x%lx, auth=%d, %s\n",
> +   task_pid_nr(current),
> +   (long)old_encode_dev(file_priv->minor->device),
> +   file_priv->authenticated, ioctl->name);
> +
>   /* Do not trust userspace, use our own definition */
>   func = ioctl->func;
>   /* is there a local override? */
> @@ -470,6 +470,12 @@ long drm_ioctl(struct file *filp,
>   }
>  
>err_i1:
> + if (!ioctl)
> + DRM_DEBUG("invalid iotcl: pid=%d, dev=0x%lx, auth=%d, 
> cmd=0x%02x, nr=0x%02x\n",
> +   task_pid_nr(current),
> +   (long)old_encode_dev(file_priv->minor->device),
> +   file_priv->authenticated, cmd, nr);
> +
>   if (kdata != stack_kdata)
>   kfree(kdata);
>   atomic_dec(>ioctl_count);
> diff --git a/include/drm/drmP.h b/include/drm/drmP.h
> index 2d94d74..379787c 100644
> --- a/include/drm/drmP.h
> +++ b/include/drm/drmP.h
> @@ -316,6 +316,7 @@ struct drm_ioctl_desc {
>   int flags;
>   drm_ioctl_t *func;
>   unsigned int cmd_drv;
> + const char *name;
>  };
>  
>  /**
> @@ -324,7 +325,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}
> + [DRM_IOCTL_NR(DRM_##ioctl)] = 

[Intel-gfx] [PATCH 5/5] drm: Use names of ioctls in debug traces

2013-05-08 Thread Ben Widawsky
On Wed, May 08, 2013 at 07:55:18PM +0300, Ville Syrj?l? wrote:
> On Wed, May 08, 2013 at 05:03:34PM +0100, Damien Lespiau wrote:
> > From: Chris Cummins 
> > 
> > The intention here is to make the output of dmesg with full verbosity a
> > bit easier for a human to parse. This commit transforms:
> > 
> > [drm:drm_ioctl], pid=699, cmd=0x6458, nr=0x58, dev 0xe200, auth=1
> > [drm:drm_ioctl], pid=699, cmd=0xc010645b, nr=0x5b, dev 0xe200, auth=1
> > [drm:drm_ioctl], pid=699, cmd=0xc0106461, nr=0x61, dev 0xe200, auth=1
> > [drm:drm_ioctl], pid=699, cmd=0xc01c64ae, nr=0xae, dev 0xe200, auth=1
> > [drm:drm_mode_addfb], [FB:32]
> > [drm:drm_ioctl], pid=699, cmd=0xc0106464, nr=0x64, dev 0xe200, auth=1
> > [drm:drm_vm_open_locked], 0x7fd9302fe000,0x00a0
> > [drm:drm_ioctl], pid=699, cmd=0x400c645f, nr=0x5f, dev 0xe200, auth=1
> > [drm:drm_ioctl], pid=699, cmd=0xc00464af, nr=0xaf, dev 0xe200, auth=1
> > [drm:intel_crtc_set_config], [CRTC:3] [NOFB]
> > 
> > into:
> > 
> > [drm:drm_ioctl], pid=699, dev=0xe200, auth=1, I915_GEM_THROTTLE
> > [drm:drm_ioctl], pid=699, dev=0xe200, auth=1, I915_GEM_CREATE
> > [drm:drm_ioctl], pid=699, dev=0xe200, auth=1, I915_GEM_SET_TILING
> > [drm:drm_ioctl], pid=699, dev=0xe200, auth=1, IOCTL_MODE_ADDFB
> > [drm:drm_mode_addfb], [FB:32]
> > [drm:drm_ioctl], pid=699, dev=0xe200, auth=1, I915_GEM_MMAP_GTT
> > [drm:drm_vm_open_locked], 0x7fd9302fe000,0x00a0
> > [drm:drm_ioctl], pid=699, dev=0xe200, auth=1, I915_GEM_SET_DOMAIN
> > [drm:drm_ioctl], pid=699, dev=0xe200, auth=1, DRM_IOCTL_MODE_RMFB
> > [drm:intel_crtc_set_config], [CRTC:3] [NOFB]
> 
> I like it. But I made drm_ioctls const recently so the patch needs a
> small refresh.
> 
> BTW am I the only one who hates the DRM_IOCTL_DEF_DRV() macro? Thanks to
> the cpp magic it's difficult to look up the ioctls using cscope. OTOH
> with this patch looking up the numbers does become less important.

You are not the only one who hates it.

> 
> > Signed-off-by: Chris Cummins 
> > ---
> >  drivers/gpu/drm/drm_drv.c | 20 +---
> >  include/drm/drmP.h|  3 ++-
> >  2 files changed, 15 insertions(+), 8 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
> > index 25f91cd..0382f6e 100644
> > --- a/drivers/gpu/drm/drm_drv.c
> > +++ b/drivers/gpu/drm/drm_drv.c
> > @@ -57,7 +57,7 @@ static int drm_version(struct drm_device *dev, void *data,
> >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}
> > +   [DRM_IOCTL_NR(ioctl)] = {.cmd = ioctl, .func = _func, .flags = _flags, 
> > .cmd_drv = 0, .name = #ioctl}
> >  
> >  /** Ioctl table */
> >  static struct drm_ioctl_desc drm_ioctls[] = {
> > @@ -375,7 +375,7 @@ long drm_ioctl(struct file *filp,
> >  {
> > struct drm_file *file_priv = filp->private_data;
> > struct drm_device *dev;
> > -   struct drm_ioctl_desc *ioctl;
> > +   struct drm_ioctl_desc *ioctl = NULL;
> > drm_ioctl_t *func;
> > unsigned int nr = DRM_IOCTL_NR(cmd);
> > int retcode = -EINVAL;
> > @@ -392,11 +392,6 @@ long drm_ioctl(struct file *filp,
> > atomic_inc(>counts[_DRM_STAT_IOCTLS]);
> > ++file_priv->ioctl_count;
> >  
> > -   DRM_DEBUG("pid=%d, cmd=0x%02x, nr=0x%02x, dev 0x%lx, auth=%d\n",
> > - task_pid_nr(current), cmd, nr,
> > - (long)old_encode_dev(file_priv->minor->device),
> > - file_priv->authenticated);
> > -
> > if ((nr >= DRM_CORE_IOCTL_COUNT) &&
> > ((nr < DRM_COMMAND_BASE) || (nr >= DRM_COMMAND_END)))
> > goto err_i1;
> > @@ -416,6 +411,11 @@ long drm_ioctl(struct file *filp,
> > } else
> > goto err_i1;
> >  
> > +   DRM_DEBUG("pid=%d, dev=0x%lx, auth=%d, %s\n",
> > + task_pid_nr(current),
> > + (long)old_encode_dev(file_priv->minor->device),
> > + file_priv->authenticated, ioctl->name);
> > +
> > /* Do not trust userspace, use our own definition */
> > func = ioctl->func;
> > /* is there a local override? */
> > @@ -470,6 +470,12 @@ long drm_ioctl(struct file *filp,
> > }
> >  
> >err_i1:
> > +   if (!ioctl)
> > +   DRM_DEBUG("invalid iotcl: pid=%d, dev=0x%lx, auth=%d, 
> > cmd=0x%02x, nr=0x%02x\n",
> > + task_pid_nr(current),
> > + (long)old_encode_dev(file_priv->minor->device),
> > + file_priv->authenticated, cmd, nr);
> > +
> > if (kdata != stack_kdata)
> > kfree(kdata);
> > atomic_dec(>ioctl_count);
> > diff --git a/include/drm/drmP.h b/include/drm/drmP.h
> > index 2d94d74..379787c 100644
> > --- a/include/drm/drmP.h
> > +++ b/include/drm/drmP.h
> > @@ -316,6 +316,7 @@ struct drm_ioctl_desc {
> > int flags;
> > drm_ioctl_t *func;
> > unsigned int cmd_drv;
> > +   const char *name;
> >  };
> >  
> >  /**
> > @@ -324,7 +325,7 @@ struct 

[PATCH 5/5] drm: Use names of ioctls in debug traces

2013-05-08 Thread Damien Lespiau
From: Chris Cummins 

The intention here is to make the output of dmesg with full verbosity a
bit easier for a human to parse. This commit transforms:

[drm:drm_ioctl], pid=699, cmd=0x6458, nr=0x58, dev 0xe200, auth=1
[drm:drm_ioctl], pid=699, cmd=0xc010645b, nr=0x5b, dev 0xe200, auth=1
[drm:drm_ioctl], pid=699, cmd=0xc0106461, nr=0x61, dev 0xe200, auth=1
[drm:drm_ioctl], pid=699, cmd=0xc01c64ae, nr=0xae, dev 0xe200, auth=1
[drm:drm_mode_addfb], [FB:32]
[drm:drm_ioctl], pid=699, cmd=0xc0106464, nr=0x64, dev 0xe200, auth=1
[drm:drm_vm_open_locked], 0x7fd9302fe000,0x00a0
[drm:drm_ioctl], pid=699, cmd=0x400c645f, nr=0x5f, dev 0xe200, auth=1
[drm:drm_ioctl], pid=699, cmd=0xc00464af, nr=0xaf, dev 0xe200, auth=1
[drm:intel_crtc_set_config], [CRTC:3] [NOFB]

into:

[drm:drm_ioctl], pid=699, dev=0xe200, auth=1, I915_GEM_THROTTLE
[drm:drm_ioctl], pid=699, dev=0xe200, auth=1, I915_GEM_CREATE
[drm:drm_ioctl], pid=699, dev=0xe200, auth=1, I915_GEM_SET_TILING
[drm:drm_ioctl], pid=699, dev=0xe200, auth=1, IOCTL_MODE_ADDFB
[drm:drm_mode_addfb], [FB:32]
[drm:drm_ioctl], pid=699, dev=0xe200, auth=1, I915_GEM_MMAP_GTT
[drm:drm_vm_open_locked], 0x7fd9302fe000,0x00a0
[drm:drm_ioctl], pid=699, dev=0xe200, auth=1, I915_GEM_SET_DOMAIN
[drm:drm_ioctl], pid=699, dev=0xe200, auth=1, DRM_IOCTL_MODE_RMFB
[drm:intel_crtc_set_config], [CRTC:3] [NOFB]

Signed-off-by: Chris Cummins 
---
 drivers/gpu/drm/drm_drv.c | 20 +---
 include/drm/drmP.h|  3 ++-
 2 files changed, 15 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
index 25f91cd..0382f6e 100644
--- a/drivers/gpu/drm/drm_drv.c
+++ b/drivers/gpu/drm/drm_drv.c
@@ -57,7 +57,7 @@ static int drm_version(struct drm_device *dev, void *data,
   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}
+   [DRM_IOCTL_NR(ioctl)] = {.cmd = ioctl, .func = _func, .flags = _flags, 
.cmd_drv = 0, .name = #ioctl}

 /** Ioctl table */
 static struct drm_ioctl_desc drm_ioctls[] = {
@@ -375,7 +375,7 @@ long drm_ioctl(struct file *filp,
 {
struct drm_file *file_priv = filp->private_data;
struct drm_device *dev;
-   struct drm_ioctl_desc *ioctl;
+   struct drm_ioctl_desc *ioctl = NULL;
drm_ioctl_t *func;
unsigned int nr = DRM_IOCTL_NR(cmd);
int retcode = -EINVAL;
@@ -392,11 +392,6 @@ long drm_ioctl(struct file *filp,
atomic_inc(>counts[_DRM_STAT_IOCTLS]);
++file_priv->ioctl_count;

-   DRM_DEBUG("pid=%d, cmd=0x%02x, nr=0x%02x, dev 0x%lx, auth=%d\n",
- task_pid_nr(current), cmd, nr,
- (long)old_encode_dev(file_priv->minor->device),
- file_priv->authenticated);
-
if ((nr >= DRM_CORE_IOCTL_COUNT) &&
((nr < DRM_COMMAND_BASE) || (nr >= DRM_COMMAND_END)))
goto err_i1;
@@ -416,6 +411,11 @@ long drm_ioctl(struct file *filp,
} else
goto err_i1;

+   DRM_DEBUG("pid=%d, dev=0x%lx, auth=%d, %s\n",
+ task_pid_nr(current),
+ (long)old_encode_dev(file_priv->minor->device),
+ file_priv->authenticated, ioctl->name);
+
/* Do not trust userspace, use our own definition */
func = ioctl->func;
/* is there a local override? */
@@ -470,6 +470,12 @@ long drm_ioctl(struct file *filp,
}

   err_i1:
+   if (!ioctl)
+   DRM_DEBUG("invalid iotcl: pid=%d, dev=0x%lx, auth=%d, 
cmd=0x%02x, nr=0x%02x\n",
+ task_pid_nr(current),
+ (long)old_encode_dev(file_priv->minor->device),
+ file_priv->authenticated, cmd, nr);
+
if (kdata != stack_kdata)
kfree(kdata);
atomic_dec(>ioctl_count);
diff --git a/include/drm/drmP.h b/include/drm/drmP.h
index 2d94d74..379787c 100644
--- a/include/drm/drmP.h
+++ b/include/drm/drmP.h
@@ -316,6 +316,7 @@ struct drm_ioctl_desc {
int flags;
drm_ioctl_t *func;
unsigned int cmd_drv;
+   const char *name;
 };

 /**
@@ -324,7 +325,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}
+   [DRM_IOCTL_NR(DRM_##ioctl)] = {.cmd = DRM_##ioctl, .func = _func, 
.flags = _flags, .cmd_drv = DRM_IOCTL_##ioctl, .name = #ioctl}

 struct drm_magic_entry {
struct list_head head;
-- 
1.8.1.4



[PATCH 5/5] drm: Use names of ioctls in debug traces

2013-05-08 Thread Damien Lespiau
From: Chris Cummins christopher.e.cumm...@intel.com

The intention here is to make the output of dmesg with full verbosity a
bit easier for a human to parse. This commit transforms:

[drm:drm_ioctl], pid=699, cmd=0x6458, nr=0x58, dev 0xe200, auth=1
[drm:drm_ioctl], pid=699, cmd=0xc010645b, nr=0x5b, dev 0xe200, auth=1
[drm:drm_ioctl], pid=699, cmd=0xc0106461, nr=0x61, dev 0xe200, auth=1
[drm:drm_ioctl], pid=699, cmd=0xc01c64ae, nr=0xae, dev 0xe200, auth=1
[drm:drm_mode_addfb], [FB:32]
[drm:drm_ioctl], pid=699, cmd=0xc0106464, nr=0x64, dev 0xe200, auth=1
[drm:drm_vm_open_locked], 0x7fd9302fe000,0x00a0
[drm:drm_ioctl], pid=699, cmd=0x400c645f, nr=0x5f, dev 0xe200, auth=1
[drm:drm_ioctl], pid=699, cmd=0xc00464af, nr=0xaf, dev 0xe200, auth=1
[drm:intel_crtc_set_config], [CRTC:3] [NOFB]

into:

[drm:drm_ioctl], pid=699, dev=0xe200, auth=1, I915_GEM_THROTTLE
[drm:drm_ioctl], pid=699, dev=0xe200, auth=1, I915_GEM_CREATE
[drm:drm_ioctl], pid=699, dev=0xe200, auth=1, I915_GEM_SET_TILING
[drm:drm_ioctl], pid=699, dev=0xe200, auth=1, IOCTL_MODE_ADDFB
[drm:drm_mode_addfb], [FB:32]
[drm:drm_ioctl], pid=699, dev=0xe200, auth=1, I915_GEM_MMAP_GTT
[drm:drm_vm_open_locked], 0x7fd9302fe000,0x00a0
[drm:drm_ioctl], pid=699, dev=0xe200, auth=1, I915_GEM_SET_DOMAIN
[drm:drm_ioctl], pid=699, dev=0xe200, auth=1, DRM_IOCTL_MODE_RMFB
[drm:intel_crtc_set_config], [CRTC:3] [NOFB]

Signed-off-by: Chris Cummins christopher.e.cumm...@intel.com
---
 drivers/gpu/drm/drm_drv.c | 20 +---
 include/drm/drmP.h|  3 ++-
 2 files changed, 15 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
index 25f91cd..0382f6e 100644
--- a/drivers/gpu/drm/drm_drv.c
+++ b/drivers/gpu/drm/drm_drv.c
@@ -57,7 +57,7 @@ static int drm_version(struct drm_device *dev, void *data,
   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}
+   [DRM_IOCTL_NR(ioctl)] = {.cmd = ioctl, .func = _func, .flags = _flags, 
.cmd_drv = 0, .name = #ioctl}
 
 /** Ioctl table */
 static struct drm_ioctl_desc drm_ioctls[] = {
@@ -375,7 +375,7 @@ long drm_ioctl(struct file *filp,
 {
struct drm_file *file_priv = filp-private_data;
struct drm_device *dev;
-   struct drm_ioctl_desc *ioctl;
+   struct drm_ioctl_desc *ioctl = NULL;
drm_ioctl_t *func;
unsigned int nr = DRM_IOCTL_NR(cmd);
int retcode = -EINVAL;
@@ -392,11 +392,6 @@ long drm_ioctl(struct file *filp,
atomic_inc(dev-counts[_DRM_STAT_IOCTLS]);
++file_priv-ioctl_count;
 
-   DRM_DEBUG(pid=%d, cmd=0x%02x, nr=0x%02x, dev 0x%lx, auth=%d\n,
- task_pid_nr(current), cmd, nr,
- (long)old_encode_dev(file_priv-minor-device),
- file_priv-authenticated);
-
if ((nr = DRM_CORE_IOCTL_COUNT) 
((nr  DRM_COMMAND_BASE) || (nr = DRM_COMMAND_END)))
goto err_i1;
@@ -416,6 +411,11 @@ long drm_ioctl(struct file *filp,
} else
goto err_i1;
 
+   DRM_DEBUG(pid=%d, dev=0x%lx, auth=%d, %s\n,
+ task_pid_nr(current),
+ (long)old_encode_dev(file_priv-minor-device),
+ file_priv-authenticated, ioctl-name);
+
/* Do not trust userspace, use our own definition */
func = ioctl-func;
/* is there a local override? */
@@ -470,6 +470,12 @@ long drm_ioctl(struct file *filp,
}
 
   err_i1:
+   if (!ioctl)
+   DRM_DEBUG(invalid iotcl: pid=%d, dev=0x%lx, auth=%d, 
cmd=0x%02x, nr=0x%02x\n,
+ task_pid_nr(current),
+ (long)old_encode_dev(file_priv-minor-device),
+ file_priv-authenticated, cmd, nr);
+
if (kdata != stack_kdata)
kfree(kdata);
atomic_dec(dev-ioctl_count);
diff --git a/include/drm/drmP.h b/include/drm/drmP.h
index 2d94d74..379787c 100644
--- a/include/drm/drmP.h
+++ b/include/drm/drmP.h
@@ -316,6 +316,7 @@ struct drm_ioctl_desc {
int flags;
drm_ioctl_t *func;
unsigned int cmd_drv;
+   const char *name;
 };
 
 /**
@@ -324,7 +325,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}
+   [DRM_IOCTL_NR(DRM_##ioctl)] = {.cmd = DRM_##ioctl, .func = _func, 
.flags = _flags, .cmd_drv = DRM_IOCTL_##ioctl, .name = #ioctl}
 
 struct drm_magic_entry {
struct list_head head;
-- 
1.8.1.4

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


Re: [Intel-gfx] [PATCH 5/5] drm: Use names of ioctls in debug traces

2013-05-08 Thread Ben Widawsky
On Wed, May 08, 2013 at 07:55:18PM +0300, Ville Syrjälä wrote:
 On Wed, May 08, 2013 at 05:03:34PM +0100, Damien Lespiau wrote:
  From: Chris Cummins christopher.e.cumm...@intel.com
  
  The intention here is to make the output of dmesg with full verbosity a
  bit easier for a human to parse. This commit transforms:
  
  [drm:drm_ioctl], pid=699, cmd=0x6458, nr=0x58, dev 0xe200, auth=1
  [drm:drm_ioctl], pid=699, cmd=0xc010645b, nr=0x5b, dev 0xe200, auth=1
  [drm:drm_ioctl], pid=699, cmd=0xc0106461, nr=0x61, dev 0xe200, auth=1
  [drm:drm_ioctl], pid=699, cmd=0xc01c64ae, nr=0xae, dev 0xe200, auth=1
  [drm:drm_mode_addfb], [FB:32]
  [drm:drm_ioctl], pid=699, cmd=0xc0106464, nr=0x64, dev 0xe200, auth=1
  [drm:drm_vm_open_locked], 0x7fd9302fe000,0x00a0
  [drm:drm_ioctl], pid=699, cmd=0x400c645f, nr=0x5f, dev 0xe200, auth=1
  [drm:drm_ioctl], pid=699, cmd=0xc00464af, nr=0xaf, dev 0xe200, auth=1
  [drm:intel_crtc_set_config], [CRTC:3] [NOFB]
  
  into:
  
  [drm:drm_ioctl], pid=699, dev=0xe200, auth=1, I915_GEM_THROTTLE
  [drm:drm_ioctl], pid=699, dev=0xe200, auth=1, I915_GEM_CREATE
  [drm:drm_ioctl], pid=699, dev=0xe200, auth=1, I915_GEM_SET_TILING
  [drm:drm_ioctl], pid=699, dev=0xe200, auth=1, IOCTL_MODE_ADDFB
  [drm:drm_mode_addfb], [FB:32]
  [drm:drm_ioctl], pid=699, dev=0xe200, auth=1, I915_GEM_MMAP_GTT
  [drm:drm_vm_open_locked], 0x7fd9302fe000,0x00a0
  [drm:drm_ioctl], pid=699, dev=0xe200, auth=1, I915_GEM_SET_DOMAIN
  [drm:drm_ioctl], pid=699, dev=0xe200, auth=1, DRM_IOCTL_MODE_RMFB
  [drm:intel_crtc_set_config], [CRTC:3] [NOFB]
 
 I like it. But I made drm_ioctls const recently so the patch needs a
 small refresh.
 
 BTW am I the only one who hates the DRM_IOCTL_DEF_DRV() macro? Thanks to
 the cpp magic it's difficult to look up the ioctls using cscope. OTOH
 with this patch looking up the numbers does become less important.

You are not the only one who hates it.

 
  Signed-off-by: Chris Cummins christopher.e.cumm...@intel.com
  ---
   drivers/gpu/drm/drm_drv.c | 20 +---
   include/drm/drmP.h|  3 ++-
   2 files changed, 15 insertions(+), 8 deletions(-)
  
  diff --git a/drivers/gpu/drm/drm_drv.c b/drivers/gpu/drm/drm_drv.c
  index 25f91cd..0382f6e 100644
  --- a/drivers/gpu/drm/drm_drv.c
  +++ b/drivers/gpu/drm/drm_drv.c
  @@ -57,7 +57,7 @@ static int drm_version(struct drm_device *dev, void *data,
 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}
  +   [DRM_IOCTL_NR(ioctl)] = {.cmd = ioctl, .func = _func, .flags = _flags, 
  .cmd_drv = 0, .name = #ioctl}
   
   /** Ioctl table */
   static struct drm_ioctl_desc drm_ioctls[] = {
  @@ -375,7 +375,7 @@ long drm_ioctl(struct file *filp,
   {
  struct drm_file *file_priv = filp-private_data;
  struct drm_device *dev;
  -   struct drm_ioctl_desc *ioctl;
  +   struct drm_ioctl_desc *ioctl = NULL;
  drm_ioctl_t *func;
  unsigned int nr = DRM_IOCTL_NR(cmd);
  int retcode = -EINVAL;
  @@ -392,11 +392,6 @@ long drm_ioctl(struct file *filp,
  atomic_inc(dev-counts[_DRM_STAT_IOCTLS]);
  ++file_priv-ioctl_count;
   
  -   DRM_DEBUG(pid=%d, cmd=0x%02x, nr=0x%02x, dev 0x%lx, auth=%d\n,
  - task_pid_nr(current), cmd, nr,
  - (long)old_encode_dev(file_priv-minor-device),
  - file_priv-authenticated);
  -
  if ((nr = DRM_CORE_IOCTL_COUNT) 
  ((nr  DRM_COMMAND_BASE) || (nr = DRM_COMMAND_END)))
  goto err_i1;
  @@ -416,6 +411,11 @@ long drm_ioctl(struct file *filp,
  } else
  goto err_i1;
   
  +   DRM_DEBUG(pid=%d, dev=0x%lx, auth=%d, %s\n,
  + task_pid_nr(current),
  + (long)old_encode_dev(file_priv-minor-device),
  + file_priv-authenticated, ioctl-name);
  +
  /* Do not trust userspace, use our own definition */
  func = ioctl-func;
  /* is there a local override? */
  @@ -470,6 +470,12 @@ long drm_ioctl(struct file *filp,
  }
   
 err_i1:
  +   if (!ioctl)
  +   DRM_DEBUG(invalid iotcl: pid=%d, dev=0x%lx, auth=%d, 
  cmd=0x%02x, nr=0x%02x\n,
  + task_pid_nr(current),
  + (long)old_encode_dev(file_priv-minor-device),
  + file_priv-authenticated, cmd, nr);
  +
  if (kdata != stack_kdata)
  kfree(kdata);
  atomic_dec(dev-ioctl_count);
  diff --git a/include/drm/drmP.h b/include/drm/drmP.h
  index 2d94d74..379787c 100644
  --- a/include/drm/drmP.h
  +++ b/include/drm/drmP.h
  @@ -316,6 +316,7 @@ struct drm_ioctl_desc {
  int flags;
  drm_ioctl_t *func;
  unsigned int cmd_drv;
  +   const char *name;
   };
   
   /**
  @@ -324,7 +325,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,