All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/2] drm/i915: Add struct_mutex locking for debugs/i915_gem_framebuffer
@ 2016-03-30 14:18 Chris Wilson
  2016-03-30 14:18 ` [PATCH 2/2] drm/i915: Show PCI power state under debugfs/i915_runtime_pm_status Chris Wilson
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Chris Wilson @ 2016-03-30 14:18 UTC (permalink / raw)
  To: intel-gfx

Since describe_obj() looks at state guarded by the struct_mutex, we need
to be holding it.

[  580.201054] drv_suspend: starting subtest debugfs-reader
[  580.239652] ------------[ cut here ]------------
[  580.239696] WARNING: CPU: 0 PID: 920 at include/linux/list_check.h:25 describe_obj+0x419/0x440()
[  580.239725] CPU: 0 PID: 920 Comm: cat Not tainted 4.5.0-rc6+ #835
[  580.239745] Hardware name:                  /NUC5CPYB, BIOS PYBSWCEL.86A.0027.2015.0507.1758 05/07/2015
[  580.239767]  0000000000000000 ffff88027554fcf8 ffffffff812c1135 0000000000000000
[  580.239815]  ffffffff8193dc42 ffff88027554fd30 ffffffff8107419d ffff880071727c00
[  580.239858]  ffff8802757d8000 ffffffff818f693c ffffffff818f693c ffff8802757b9048
[  580.239896] Call Trace:
[  580.239917]  [<ffffffff812c1135>] dump_stack+0x67/0x92
[  580.239939]  [<ffffffff8107419d>] warn_slowpath_common+0x7d/0xb0
[  580.239959]  [<ffffffff810742ba>] warn_slowpath_null+0x1a/0x20
[  580.239981]  [<ffffffff813ce579>] describe_obj+0x419/0x440
[  580.240006]  [<ffffffff813ced22>] i915_gem_framebuffer_info+0xa2/0x100
[  580.240033]  [<ffffffff811a9286>] seq_read+0xe6/0x3b0
[  580.240059]  [<ffffffff81182288>] __vfs_read+0x28/0xd0
[  580.240085]  [<ffffffff81173378>] ? SyS_fadvise64+0x228/0x2c0
[  580.240112]  [<ffffffff811823b2>] vfs_read+0x82/0x110
[  580.240137]  [<ffffffff811827d9>] SyS_read+0x49/0xa0
[  580.240162]  [<ffffffff815bac57>] entry_SYSCALL_64_fastpath+0x12/0x6b
[  580.240187] ---[ end trace 3e2cbf34576c9878 ]---
[  580.281900] ------------[ cut here ]------------
---
 drivers/gpu/drm/i915/i915_debugfs.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index d02f8ce0b1c8..adabbb47a8aa 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -134,6 +134,8 @@ describe_obj(struct seq_file *m, struct drm_i915_gem_object *obj)
 	int pin_count = 0;
 	enum intel_engine_id id;
 
+	lockdep_assert_held(&obj->base.dev->struct_mutex);
+
 	seq_printf(m, "%pK: %s%s%s%s %8zdKiB %02x %02x [ ",
 		   &obj->base,
 		   obj->active ? "*" : " ",
@@ -1895,6 +1897,11 @@ static int i915_gem_framebuffer_info(struct seq_file *m, void *data)
 	struct drm_device *dev = node->minor->dev;
 	struct intel_framebuffer *fbdev_fb = NULL;
 	struct drm_framebuffer *drm_fb;
+	int ret;
+
+	ret = mutex_lock_interruptible(&dev->struct_mutex);
+	if (ret)
+		return ret;
 
 #ifdef CONFIG_DRM_FBDEV_EMULATION
        if (to_i915(dev)->fbdev) {
@@ -1929,6 +1936,7 @@ static int i915_gem_framebuffer_info(struct seq_file *m, void *data)
 		seq_putc(m, '\n');
 	}
 	mutex_unlock(&dev->mode_config.fb_lock);
+	mutex_unlock(&dev->struct_mutex);
 
 	return 0;
 }
-- 
2.8.0.rc3

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH 2/2] drm/i915: Show PCI power state under debugfs/i915_runtime_pm_status
  2016-03-30 14:18 [PATCH 1/2] drm/i915: Add struct_mutex locking for debugs/i915_gem_framebuffer Chris Wilson
@ 2016-03-30 14:18 ` Chris Wilson
  2016-03-30 18:55   ` Daniel Vetter
  2016-03-30 18:50 ` [PATCH 1/2] drm/i915: Add struct_mutex locking for debugs/i915_gem_framebuffer Daniel Vetter
  2016-03-31 13:17 ` ✓ Fi.CI.BAT: success for series starting with [1/2] " Patchwork
  2 siblings, 1 reply; 6+ messages in thread
From: Chris Wilson @ 2016-03-30 14:18 UTC (permalink / raw)
  To: intel-gfx

As the current PCI power state is an essential feature of runtime pm,
include it in the debugfs/i915_runtime_pm_status.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Imre Deak <imre.deak@intel.com>
---
 drivers/gpu/drm/i915/i915_debugfs.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index adabbb47a8aa..05d701122259 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -2695,10 +2695,8 @@ static int i915_runtime_pm_status(struct seq_file *m, void *unused)
 	struct drm_device *dev = node->minor->dev;
 	struct drm_i915_private *dev_priv = dev->dev_private;
 
-	if (!HAS_RUNTIME_PM(dev)) {
-		seq_puts(m, "not supported\n");
-		return 0;
-	}
+	if (!HAS_RUNTIME_PM(dev_priv))
+		seq_puts(m, "Runtime power management not supported\n");
 
 	seq_printf(m, "GPU idle: %s\n", yesno(!dev_priv->mm.busy));
 	seq_printf(m, "IRQs disabled: %s\n",
@@ -2709,6 +2707,9 @@ static int i915_runtime_pm_status(struct seq_file *m, void *unused)
 #else
 	seq_printf(m, "Device Power Management (CONFIG_PM) disabled\n");
 #endif
+	seq_printf(m, "PCI device power state: %s [%d]\n",
+		   pci_power_names[dev_priv->dev->pdev->current_state],
+		   dev_priv->dev->pdev->current_state);
 
 	return 0;
 }
-- 
2.8.0.rc3

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH 1/2] drm/i915: Add struct_mutex locking for debugs/i915_gem_framebuffer
  2016-03-30 14:18 [PATCH 1/2] drm/i915: Add struct_mutex locking for debugs/i915_gem_framebuffer Chris Wilson
  2016-03-30 14:18 ` [PATCH 2/2] drm/i915: Show PCI power state under debugfs/i915_runtime_pm_status Chris Wilson
@ 2016-03-30 18:50 ` Daniel Vetter
  2016-03-31 13:17 ` ✓ Fi.CI.BAT: success for series starting with [1/2] " Patchwork
  2 siblings, 0 replies; 6+ messages in thread
From: Daniel Vetter @ 2016-03-30 18:50 UTC (permalink / raw)
  To: Chris Wilson; +Cc: intel-gfx

On Wed, Mar 30, 2016 at 03:18:40PM +0100, Chris Wilson wrote:
> Since describe_obj() looks at state guarded by the struct_mutex, we need
> to be holding it.
> 
> [  580.201054] drv_suspend: starting subtest debugfs-reader
> [  580.239652] ------------[ cut here ]------------
> [  580.239696] WARNING: CPU: 0 PID: 920 at include/linux/list_check.h:25 describe_obj+0x419/0x440()
> [  580.239725] CPU: 0 PID: 920 Comm: cat Not tainted 4.5.0-rc6+ #835
> [  580.239745] Hardware name:                  /NUC5CPYB, BIOS PYBSWCEL.86A.0027.2015.0507.1758 05/07/2015
> [  580.239767]  0000000000000000 ffff88027554fcf8 ffffffff812c1135 0000000000000000
> [  580.239815]  ffffffff8193dc42 ffff88027554fd30 ffffffff8107419d ffff880071727c00
> [  580.239858]  ffff8802757d8000 ffffffff818f693c ffffffff818f693c ffff8802757b9048
> [  580.239896] Call Trace:
> [  580.239917]  [<ffffffff812c1135>] dump_stack+0x67/0x92
> [  580.239939]  [<ffffffff8107419d>] warn_slowpath_common+0x7d/0xb0
> [  580.239959]  [<ffffffff810742ba>] warn_slowpath_null+0x1a/0x20
> [  580.239981]  [<ffffffff813ce579>] describe_obj+0x419/0x440
> [  580.240006]  [<ffffffff813ced22>] i915_gem_framebuffer_info+0xa2/0x100
> [  580.240033]  [<ffffffff811a9286>] seq_read+0xe6/0x3b0
> [  580.240059]  [<ffffffff81182288>] __vfs_read+0x28/0xd0
> [  580.240085]  [<ffffffff81173378>] ? SyS_fadvise64+0x228/0x2c0
> [  580.240112]  [<ffffffff811823b2>] vfs_read+0x82/0x110
> [  580.240137]  [<ffffffff811827d9>] SyS_read+0x49/0xa0
> [  580.240162]  [<ffffffff815bac57>] entry_SYSCALL_64_fastpath+0x12/0x6b
> [  580.240187] ---[ end trace 3e2cbf34576c9878 ]---
> [  580.281900] ------------[ cut here ]------------

Missing sob-line here. With that fixed:

Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> ---
>  drivers/gpu/drm/i915/i915_debugfs.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> index d02f8ce0b1c8..adabbb47a8aa 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -134,6 +134,8 @@ describe_obj(struct seq_file *m, struct drm_i915_gem_object *obj)
>  	int pin_count = 0;
>  	enum intel_engine_id id;
>  
> +	lockdep_assert_held(&obj->base.dev->struct_mutex);
> +
>  	seq_printf(m, "%pK: %s%s%s%s %8zdKiB %02x %02x [ ",
>  		   &obj->base,
>  		   obj->active ? "*" : " ",
> @@ -1895,6 +1897,11 @@ static int i915_gem_framebuffer_info(struct seq_file *m, void *data)
>  	struct drm_device *dev = node->minor->dev;
>  	struct intel_framebuffer *fbdev_fb = NULL;
>  	struct drm_framebuffer *drm_fb;
> +	int ret;
> +
> +	ret = mutex_lock_interruptible(&dev->struct_mutex);
> +	if (ret)
> +		return ret;
>  
>  #ifdef CONFIG_DRM_FBDEV_EMULATION
>         if (to_i915(dev)->fbdev) {
> @@ -1929,6 +1936,7 @@ static int i915_gem_framebuffer_info(struct seq_file *m, void *data)
>  		seq_putc(m, '\n');
>  	}
>  	mutex_unlock(&dev->mode_config.fb_lock);
> +	mutex_unlock(&dev->struct_mutex);
>  
>  	return 0;
>  }
> -- 
> 2.8.0.rc3
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 2/2] drm/i915: Show PCI power state under debugfs/i915_runtime_pm_status
  2016-03-30 14:18 ` [PATCH 2/2] drm/i915: Show PCI power state under debugfs/i915_runtime_pm_status Chris Wilson
@ 2016-03-30 18:55   ` Daniel Vetter
  2016-03-30 19:05     ` Chris Wilson
  0 siblings, 1 reply; 6+ messages in thread
From: Daniel Vetter @ 2016-03-30 18:55 UTC (permalink / raw)
  To: Chris Wilson; +Cc: intel-gfx

On Wed, Mar 30, 2016 at 03:18:41PM +0100, Chris Wilson wrote:
> As the current PCI power state is an essential feature of runtime pm,
> include it in the debugfs/i915_runtime_pm_status.
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Imre Deak <imre.deak@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_debugfs.c | 9 +++++----
>  1 file changed, 5 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> index adabbb47a8aa..05d701122259 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -2695,10 +2695,8 @@ static int i915_runtime_pm_status(struct seq_file *m, void *unused)
>  	struct drm_device *dev = node->minor->dev;
>  	struct drm_i915_private *dev_priv = dev->dev_private;
>  
> -	if (!HAS_RUNTIME_PM(dev)) {
> -		seq_puts(m, "not supported\n");
> -		return 0;
> -	}
> +	if (!HAS_RUNTIME_PM(dev_priv))
> +		seq_puts(m, "Runtime power management not supported\n");
>  
>  	seq_printf(m, "GPU idle: %s\n", yesno(!dev_priv->mm.busy));
>  	seq_printf(m, "IRQs disabled: %s\n",
> @@ -2709,6 +2707,9 @@ static int i915_runtime_pm_status(struct seq_file *m, void *unused)
>  #else
>  	seq_printf(m, "Device Power Management (CONFIG_PM) disabled\n");
>  #endif
> +	seq_printf(m, "PCI device power state: %s [%d]\n",
> +		   pci_power_names[dev_priv->dev->pdev->current_state],

Please use pci_power_name() instead (you are missing a + 1), and I don't
think we need the raw value. With that addressed:

Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>

> +		   dev_priv->dev->pdev->current_state);
>  
>  	return 0;
>  }
> -- 
> 2.8.0.rc3
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH 2/2] drm/i915: Show PCI power state under debugfs/i915_runtime_pm_status
  2016-03-30 18:55   ` Daniel Vetter
@ 2016-03-30 19:05     ` Chris Wilson
  0 siblings, 0 replies; 6+ messages in thread
From: Chris Wilson @ 2016-03-30 19:05 UTC (permalink / raw)
  To: Daniel Vetter; +Cc: intel-gfx

On Wed, Mar 30, 2016 at 08:55:51PM +0200, Daniel Vetter wrote:
> On Wed, Mar 30, 2016 at 03:18:41PM +0100, Chris Wilson wrote:
> > As the current PCI power state is an essential feature of runtime pm,
> > include it in the debugfs/i915_runtime_pm_status.
> > 
> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > Cc: Imre Deak <imre.deak@intel.com>
> > ---
> >  drivers/gpu/drm/i915/i915_debugfs.c | 9 +++++----
> >  1 file changed, 5 insertions(+), 4 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> > index adabbb47a8aa..05d701122259 100644
> > --- a/drivers/gpu/drm/i915/i915_debugfs.c
> > +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> > @@ -2695,10 +2695,8 @@ static int i915_runtime_pm_status(struct seq_file *m, void *unused)
> >  	struct drm_device *dev = node->minor->dev;
> >  	struct drm_i915_private *dev_priv = dev->dev_private;
> >  
> > -	if (!HAS_RUNTIME_PM(dev)) {
> > -		seq_puts(m, "not supported\n");
> > -		return 0;
> > -	}
> > +	if (!HAS_RUNTIME_PM(dev_priv))
> > +		seq_puts(m, "Runtime power management not supported\n");
> >  
> >  	seq_printf(m, "GPU idle: %s\n", yesno(!dev_priv->mm.busy));
> >  	seq_printf(m, "IRQs disabled: %s\n",
> > @@ -2709,6 +2707,9 @@ static int i915_runtime_pm_status(struct seq_file *m, void *unused)
> >  #else
> >  	seq_printf(m, "Device Power Management (CONFIG_PM) disabled\n");
> >  #endif
> > +	seq_printf(m, "PCI device power state: %s [%d]\n",
> > +		   pci_power_names[dev_priv->dev->pdev->current_state],
> 
> Please use pci_power_name() instead (you are missing a + 1), and I don't
> think we need the raw value. With that addressed:

I put in the raw value since inside igt that is the value we see when we
look at pci_device_cfg(), so I thought it would be easier for
comparison purposes.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 6+ messages in thread

* ✓ Fi.CI.BAT: success for series starting with [1/2] drm/i915: Add struct_mutex locking for debugs/i915_gem_framebuffer
  2016-03-30 14:18 [PATCH 1/2] drm/i915: Add struct_mutex locking for debugs/i915_gem_framebuffer Chris Wilson
  2016-03-30 14:18 ` [PATCH 2/2] drm/i915: Show PCI power state under debugfs/i915_runtime_pm_status Chris Wilson
  2016-03-30 18:50 ` [PATCH 1/2] drm/i915: Add struct_mutex locking for debugs/i915_gem_framebuffer Daniel Vetter
@ 2016-03-31 13:17 ` Patchwork
  2 siblings, 0 replies; 6+ messages in thread
From: Patchwork @ 2016-03-31 13:17 UTC (permalink / raw)
  To: Chris Wilson; +Cc: intel-gfx

== Series Details ==

Series: series starting with [1/2] drm/i915: Add struct_mutex locking for debugs/i915_gem_framebuffer
URL   : https://patchwork.freedesktop.org/series/5053/
State : success

== Summary ==

Series 5053v1 Series without cover letter
http://patchwork.freedesktop.org/api/1.0/series/5053/revisions/1/mbox/


bdw-nuci7        total:196  pass:184  dwarn:0   dfail:0   fail:0   skip:12 
bdw-ultra        total:196  pass:175  dwarn:0   dfail:0   fail:0   skip:21 
bsw-nuc-2        total:196  pass:158  dwarn:1   dfail:0   fail:0   skip:37 
byt-nuc          total:196  pass:161  dwarn:0   dfail:0   fail:0   skip:35 
hsw-gt2          total:28   pass:26   dwarn:0   dfail:0   fail:0   skip:1  
skl-i7k-2        total:196  pass:173  dwarn:0   dfail:0   fail:0   skip:23 
skl-nuci5        total:196  pass:185  dwarn:0   dfail:0   fail:0   skip:11 
snb-dellxps      total:1    pass:0    dwarn:0   dfail:0   fail:0   skip:0  

Results at /archive/results/CI_IGT_test/Patchwork_1756/

03c0f854e93263563f559d2bc8e47fb51adae697 drm-intel-nightly: 2016y-03m-31d-10h-50m-15s UTC integration manifest
2c59632353f8156281f58967d61d93fededfd4e1 drm/i915: Show PCI power state under debugfs/i915_runtime_pm_status
c1e95e5718eb1d2765217d7e47f83d6fc5f69f30 drm/i915: Add struct_mutex locking for debugs/i915_gem_framebuffer

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2016-03-31 13:17 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-03-30 14:18 [PATCH 1/2] drm/i915: Add struct_mutex locking for debugs/i915_gem_framebuffer Chris Wilson
2016-03-30 14:18 ` [PATCH 2/2] drm/i915: Show PCI power state under debugfs/i915_runtime_pm_status Chris Wilson
2016-03-30 18:55   ` Daniel Vetter
2016-03-30 19:05     ` Chris Wilson
2016-03-30 18:50 ` [PATCH 1/2] drm/i915: Add struct_mutex locking for debugs/i915_gem_framebuffer Daniel Vetter
2016-03-31 13:17 ` ✓ Fi.CI.BAT: success for series starting with [1/2] " Patchwork

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.