All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] drm/i915: Extend rpm wakelock for debugfs/i915_drpc_info
@ 2017-03-13  9:56 Chris Wilson
  2017-03-13 10:18 ` ✓ Fi.CI.BAT: success for " Patchwork
  2017-03-13 13:41 ` [PATCH] " Mika Kuoppala
  0 siblings, 2 replies; 5+ messages in thread
From: Chris Wilson @ 2017-03-13  9:56 UTC (permalink / raw)
  To: intel-gfx

i915_drpc_info missed covering a few register read with the runtime pm
wakelock. Be simple and cover the entire function with a single wakelock
so that new additions are not similarly missed in future.

  WARNING: CPU: 2 PID: 1334 at drivers/gpu/drm/i915/intel_drv.h:1743 gen6_read32+0x192/0x1e0 [i915]
  RPM wakelock ref not held during HW access
  Modules linked in: rpcsec_gss_krb5 nfsv4 dns_resolver netconsole nfsd auth_rpcgss ipmi_watchdog ipmi_poweroff ipmi_devintf ipmi_msghandler overlay btrfs xor raid6_pq dm_mod sg sd_mod snd_hda_codec_hdmi snd_hda_codec_realtek snd_hda_codec_generic ata_generic pata_acpi intel_rapl x86_pkg_temp_thermal intel_powerclamp coretemp snd_hda_intel kvm_intel snd_hda_codec kvm eeepc_wmi irqbypass snd_hda_core crct10dif_pclmul crc32_pclmul crc32c_intel asus_wmi sparse_keymap ghash_clmulni_intel snd_hwdep i915 rfkill ppdev pcbc aesni_intel ata_piix crypto_simd glue_helper snd_pcm pata_via cryptd pcspkr snd_timer drm_kms_helper syscopyarea snd sysfillrect libata sysimgblt fb_sys_fops soundcore shpchp drm wmi parport_pc parport tpm_infineon video
  CPU: 2 PID: 1334 Comm: php5 Not tainted 4.10.0-rc8-01615-g1f58c8e #1
  Hardware name: System manufacturer System Product Name/P8H67-M PRO, BIOS 1002 04/01/2011
  Call Trace:
   dump_stack+0x63/0x8a
   __warn+0xcb/0xf0
   warn_slowpath_fmt+0x4f/0x60
   ? seq_vprintf+0x35/0x50
   gen6_read32+0x192/0x1e0 [i915]
   i915_drpc_info+0x55d/0x990 [i915]
   seq_read+0xf2/0x3b0
   full_proxy_read+0x51/0x80
   __vfs_read+0x28/0x130
   ? security_file_permission+0x9b/0xc0
   ? rw_verify_area+0x4e/0xb0
   vfs_read+0xa8/0x170
   SyS_read+0x46/0xa0
   entry_SYSCALL_64_fastpath+0x1a/0xa9
  RIP: 0033:0x7fd97bf175a0
  RSP: 002b:00007ffdf730db68 EFLAGS: 00000246 ORIG_RAX: 0000000000000000
  RAX: ffffffffffffffda RBX: 00007fd978028738 RCX: 00007fd97bf175a0
  RDX: 0000000000002000 RSI: 00007fd97740e0d8 RDI: 0000000000000005
  RBP: 0000000000000001 R08: 0000000000e97840 R09: 00007fd977ef8d58
  R10: 0000000000000027 R11: 0000000000000246 R12: 00007fd977ef8d58
  R13: 0000000000000000 R14: 0000000000eb4640 R15: 0000000000000000

Reported-by: kernel test robot <xiaolong.ye@intel.com>
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/i915_debugfs.c | 38 +++++++++++++------------------------
 1 file changed, 13 insertions(+), 25 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index 3e36599c0b58..44c7db48d349 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -1400,14 +1400,10 @@ static int ironlake_drpc_info(struct seq_file *m)
 	u32 rgvmodectl, rstdbyctl;
 	u16 crstandvid;
 
-	intel_runtime_pm_get(dev_priv);
-
 	rgvmodectl = I915_READ(MEMMODECTL);
 	rstdbyctl = I915_READ(RSTDBYCTL);
 	crstandvid = I915_READ16(CRSTANDVID);
 
-	intel_runtime_pm_put(dev_priv);
-
 	seq_printf(m, "HD boost: %s\n", yesno(rgvmodectl & MEMMODE_BOOST_EN));
 	seq_printf(m, "Boost freq: %d\n",
 		   (rgvmodectl & MEMMODE_BOOST_FREQ_MASK) >>
@@ -1476,14 +1472,10 @@ static int vlv_drpc_info(struct seq_file *m)
 	struct drm_i915_private *dev_priv = node_to_i915(m->private);
 	u32 rpmodectl1, rcctl1, pw_status;
 
-	intel_runtime_pm_get(dev_priv);
-
 	pw_status = I915_READ(VLV_GTLC_PW_STATUS);
 	rpmodectl1 = I915_READ(GEN6_RP_CONTROL);
 	rcctl1 = I915_READ(GEN6_RC_CONTROL);
 
-	intel_runtime_pm_put(dev_priv);
-
 	seq_printf(m, "Video Turbo Mode: %s\n",
 		   yesno(rpmodectl1 & GEN6_RP_MEDIA_TURBO));
 	seq_printf(m, "Turbo enabled: %s\n",
@@ -1512,21 +1504,12 @@ static int vlv_drpc_info(struct seq_file *m)
 static int gen6_drpc_info(struct seq_file *m)
 {
 	struct drm_i915_private *dev_priv = node_to_i915(m->private);
-	struct drm_device *dev = &dev_priv->drm;
 	u32 rpmodectl1, gt_core_status, rcctl1, rc6vids = 0;
 	u32 gen9_powergate_enable = 0, gen9_powergate_status = 0;
 	unsigned forcewake_count;
-	int count = 0, ret;
-
-	ret = mutex_lock_interruptible(&dev->struct_mutex);
-	if (ret)
-		return ret;
-	intel_runtime_pm_get(dev_priv);
-
-	spin_lock_irq(&dev_priv->uncore.lock);
-	forcewake_count = dev_priv->uncore.fw_domain[FW_DOMAIN_ID_RENDER].wake_count;
-	spin_unlock_irq(&dev_priv->uncore.lock);
+	int count = 0;
 
+	forcewake_count = READ_ONCE(dev_priv->uncore.fw_domain[FW_DOMAIN_ID_RENDER].wake_count);
 	if (forcewake_count) {
 		seq_puts(m, "RC information inaccurate because somebody "
 			    "holds a forcewake reference \n");
@@ -1546,13 +1529,11 @@ static int gen6_drpc_info(struct seq_file *m)
 		gen9_powergate_enable = I915_READ(GEN9_PG_ENABLE);
 		gen9_powergate_status = I915_READ(GEN9_PWRGT_DOMAIN_STATUS);
 	}
-	mutex_unlock(&dev->struct_mutex);
+
 	mutex_lock(&dev_priv->rps.hw_lock);
 	sandybridge_pcode_read(dev_priv, GEN6_PCODE_READ_RC6VIDS, &rc6vids);
 	mutex_unlock(&dev_priv->rps.hw_lock);
 
-	intel_runtime_pm_put(dev_priv);
-
 	seq_printf(m, "Video Turbo Mode: %s\n",
 		   yesno(rpmodectl1 & GEN6_RP_MEDIA_TURBO));
 	seq_printf(m, "HW control enabled: %s\n",
@@ -1629,13 +1610,20 @@ static int gen6_drpc_info(struct seq_file *m)
 static int i915_drpc_info(struct seq_file *m, void *unused)
 {
 	struct drm_i915_private *dev_priv = node_to_i915(m->private);
+	int err;
+
+	intel_runtime_pm_get(dev_priv);
 
 	if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv))
-		return vlv_drpc_info(m);
+		err = vlv_drpc_info(m);
 	else if (INTEL_GEN(dev_priv) >= 6)
-		return gen6_drpc_info(m);
+		err = gen6_drpc_info(m);
 	else
-		return ironlake_drpc_info(m);
+		err = ironlake_drpc_info(m);
+
+	intel_runtime_pm_put(dev_priv);
+
+	return err;
 }
 
 static int i915_frontbuffer_tracking(struct seq_file *m, void *unused)
-- 
2.11.0

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

^ permalink raw reply related	[flat|nested] 5+ messages in thread
* ✓ Fi.CI.BAT: success for drm/i915: Extend rpm wakelock for debugfs/i915_drpc_info
  2017-03-13  9:56 [PATCH] drm/i915: Extend rpm wakelock for debugfs/i915_drpc_info Chris Wilson
@ 2017-03-13 10:18 ` Patchwork
  2017-03-13 13:41 ` [PATCH] " Mika Kuoppala
  1 sibling, 0 replies; 5+ messages in thread
From: Patchwork @ 2017-03-13 10:18 UTC (permalink / raw)
  To: Chris Wilson; +Cc: intel-gfx

== Series Details ==

Series: drm/i915: Extend rpm wakelock for debugfs/i915_drpc_info
URL   : https://patchwork.freedesktop.org/series/21139/
State : success

== Summary ==

Series 21139v1 drm/i915: Extend rpm wakelock for debugfs/i915_drpc_info
https://patchwork.freedesktop.org/api/1.0/series/21139/revisions/1/mbox/

Test gem_exec_flush:
        Subgroup basic-batch-kernel-default-uc:
                fail       -> PASS       (fi-snb-2600) fdo#100007
Test gem_exec_suspend:
        Subgroup basic-s3:
                fail       -> DMESG-WARN (fi-kbl-7500u) fdo#100124
        Subgroup basic-s4-devices:
                fail       -> PASS       (fi-kbl-7500u) fdo#100099
Test kms_pipe_crc_basic:
        Subgroup suspend-read-crc-pipe-a:
                fail       -> PASS       (fi-kbl-7500u) fdo#100123
        Subgroup suspend-read-crc-pipe-b:
                fail       -> PASS       (fi-kbl-7500u) fdo#100123
        Subgroup suspend-read-crc-pipe-c:
                fail       -> PASS       (fi-kbl-7500u) fdo#100044

fdo#100007 https://bugs.freedesktop.org/show_bug.cgi?id=100007
fdo#100124 https://bugs.freedesktop.org/show_bug.cgi?id=100124
fdo#100099 https://bugs.freedesktop.org/show_bug.cgi?id=100099
fdo#100123 https://bugs.freedesktop.org/show_bug.cgi?id=100123
fdo#100044 https://bugs.freedesktop.org/show_bug.cgi?id=100044

fi-bdw-5557u     total:278  pass:267  dwarn:0   dfail:0   fail:0   skip:11  time: 464s
fi-bsw-n3050     total:278  pass:239  dwarn:0   dfail:0   fail:0   skip:39  time: 610s
fi-bxt-j4205     total:278  pass:259  dwarn:0   dfail:0   fail:0   skip:19  time: 538s
fi-bxt-t5700     total:278  pass:258  dwarn:0   dfail:0   fail:0   skip:20  time: 596s
fi-byt-j1900     total:278  pass:251  dwarn:0   dfail:0   fail:0   skip:27  time: 505s
fi-byt-n2820     total:278  pass:247  dwarn:0   dfail:0   fail:0   skip:31  time: 501s
fi-hsw-4770      total:278  pass:262  dwarn:0   dfail:0   fail:0   skip:16  time: 445s
fi-hsw-4770r     total:278  pass:262  dwarn:0   dfail:0   fail:0   skip:16  time: 437s
fi-ilk-650       total:278  pass:228  dwarn:0   dfail:0   fail:0   skip:50  time: 447s
fi-ivb-3520m     total:278  pass:260  dwarn:0   dfail:0   fail:0   skip:18  time: 506s
fi-ivb-3770      total:278  pass:260  dwarn:0   dfail:0   fail:0   skip:18  time: 489s
fi-kbl-7500u     total:278  pass:259  dwarn:1   dfail:0   fail:0   skip:18  time: 474s
fi-skl-6260u     total:278  pass:268  dwarn:0   dfail:0   fail:0   skip:10  time: 503s
fi-skl-6700hq    total:278  pass:261  dwarn:0   dfail:0   fail:0   skip:17  time: 583s
fi-skl-6700k     total:278  pass:256  dwarn:4   dfail:0   fail:0   skip:18  time: 496s
fi-skl-6770hq    total:278  pass:268  dwarn:0   dfail:0   fail:0   skip:10  time: 548s
fi-snb-2520m     total:278  pass:250  dwarn:0   dfail:0   fail:0   skip:28  time: 550s
fi-snb-2600      total:278  pass:249  dwarn:0   dfail:0   fail:0   skip:29  time: 422s

9f5422bbf45b2d8a0bb0538b08335c4ba7c2fba5 drm-tip: 2017y-03m-13d-08h-27m-09s UTC integration manifest
6f03d7c drm/i915: Extend rpm wakelock for debugfs/i915_drpc_info

== Logs ==

For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_4150/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 5+ messages in thread
* Re: [PATCH] drm/i915: Extend rpm wakelock for debugfs/i915_drpc_info
  2017-03-13  9:56 [PATCH] drm/i915: Extend rpm wakelock for debugfs/i915_drpc_info Chris Wilson
  2017-03-13 10:18 ` ✓ Fi.CI.BAT: success for " Patchwork
@ 2017-03-13 13:41 ` Mika Kuoppala
  2017-03-13 14:10   ` Chris Wilson
  2017-03-13 14:28   ` Chris Wilson
  1 sibling, 2 replies; 5+ messages in thread
From: Mika Kuoppala @ 2017-03-13 13:41 UTC (permalink / raw)
  To: Chris Wilson, intel-gfx

Chris Wilson <chris@chris-wilson.co.uk> writes:

> i915_drpc_info missed covering a few register read with the runtime pm
> wakelock. Be simple and cover the entire function with a single wakelock
> so that new additions are not similarly missed in future.
>
>   WARNING: CPU: 2 PID: 1334 at drivers/gpu/drm/i915/intel_drv.h:1743 gen6_read32+0x192/0x1e0 [i915]
>   RPM wakelock ref not held during HW access
>   Modules linked in: rpcsec_gss_krb5 nfsv4 dns_resolver netconsole nfsd auth_rpcgss ipmi_watchdog ipmi_poweroff ipmi_devintf ipmi_msghandler overlay btrfs xor raid6_pq dm_mod sg sd_mod snd_hda_codec_hdmi snd_hda_codec_realtek snd_hda_codec_generic ata_generic pata_acpi intel_rapl x86_pkg_temp_thermal intel_powerclamp coretemp snd_hda_intel kvm_intel snd_hda_codec kvm eeepc_wmi irqbypass snd_hda_core crct10dif_pclmul crc32_pclmul crc32c_intel asus_wmi sparse_keymap ghash_clmulni_intel snd_hwdep i915 rfkill ppdev pcbc aesni_intel ata_piix crypto_simd glue_helper snd_pcm pata_via cryptd pcspkr snd_timer drm_kms_helper syscopyarea snd sysfillrect libata sysimgblt fb_sys_fops soundcore shpchp drm wmi parport_pc parport tpm_infineon video
>   CPU: 2 PID: 1334 Comm: php5 Not tainted 4.10.0-rc8-01615-g1f58c8e #1
>   Hardware name: System manufacturer System Product Name/P8H67-M PRO, BIOS 1002 04/01/2011
>   Call Trace:
>    dump_stack+0x63/0x8a
>    __warn+0xcb/0xf0
>    warn_slowpath_fmt+0x4f/0x60
>    ? seq_vprintf+0x35/0x50
>    gen6_read32+0x192/0x1e0 [i915]
>    i915_drpc_info+0x55d/0x990 [i915]
>    seq_read+0xf2/0x3b0
>    full_proxy_read+0x51/0x80
>    __vfs_read+0x28/0x130
>    ? security_file_permission+0x9b/0xc0
>    ? rw_verify_area+0x4e/0xb0
>    vfs_read+0xa8/0x170
>    SyS_read+0x46/0xa0
>    entry_SYSCALL_64_fastpath+0x1a/0xa9
>   RIP: 0033:0x7fd97bf175a0
>   RSP: 002b:00007ffdf730db68 EFLAGS: 00000246 ORIG_RAX: 0000000000000000
>   RAX: ffffffffffffffda RBX: 00007fd978028738 RCX: 00007fd97bf175a0
>   RDX: 0000000000002000 RSI: 00007fd97740e0d8 RDI: 0000000000000005
>   RBP: 0000000000000001 R08: 0000000000e97840 R09: 00007fd977ef8d58
>   R10: 0000000000000027 R11: 0000000000000246 R12: 00007fd977ef8d58
>   R13: 0000000000000000 R14: 0000000000eb4640 R15: 0000000000000000
>
> Reported-by: kernel test robot <xiaolong.ye@intel.com>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>

While reading this I pondered if mutexing the whole read sequence
would add value. To give 'atomic' snapshot of state.

But that is a separate issue.

Reviewed-by: Mika Kuoppala <mika.kuoppala@intel.com>

> ---
>  drivers/gpu/drm/i915/i915_debugfs.c | 38 +++++++++++++------------------------
>  1 file changed, 13 insertions(+), 25 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> index 3e36599c0b58..44c7db48d349 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -1400,14 +1400,10 @@ static int ironlake_drpc_info(struct seq_file *m)
>  	u32 rgvmodectl, rstdbyctl;
>  	u16 crstandvid;
>  
> -	intel_runtime_pm_get(dev_priv);
> -
>  	rgvmodectl = I915_READ(MEMMODECTL);
>  	rstdbyctl = I915_READ(RSTDBYCTL);
>  	crstandvid = I915_READ16(CRSTANDVID);
>  
> -	intel_runtime_pm_put(dev_priv);
> -
>  	seq_printf(m, "HD boost: %s\n", yesno(rgvmodectl & MEMMODE_BOOST_EN));
>  	seq_printf(m, "Boost freq: %d\n",
>  		   (rgvmodectl & MEMMODE_BOOST_FREQ_MASK) >>
> @@ -1476,14 +1472,10 @@ static int vlv_drpc_info(struct seq_file *m)
>  	struct drm_i915_private *dev_priv = node_to_i915(m->private);
>  	u32 rpmodectl1, rcctl1, pw_status;
>  
> -	intel_runtime_pm_get(dev_priv);
> -
>  	pw_status = I915_READ(VLV_GTLC_PW_STATUS);
>  	rpmodectl1 = I915_READ(GEN6_RP_CONTROL);
>  	rcctl1 = I915_READ(GEN6_RC_CONTROL);
>  
> -	intel_runtime_pm_put(dev_priv);
> -
>  	seq_printf(m, "Video Turbo Mode: %s\n",
>  		   yesno(rpmodectl1 & GEN6_RP_MEDIA_TURBO));
>  	seq_printf(m, "Turbo enabled: %s\n",
> @@ -1512,21 +1504,12 @@ static int vlv_drpc_info(struct seq_file *m)
>  static int gen6_drpc_info(struct seq_file *m)
>  {
>  	struct drm_i915_private *dev_priv = node_to_i915(m->private);
> -	struct drm_device *dev = &dev_priv->drm;
>  	u32 rpmodectl1, gt_core_status, rcctl1, rc6vids = 0;
>  	u32 gen9_powergate_enable = 0, gen9_powergate_status = 0;
>  	unsigned forcewake_count;
> -	int count = 0, ret;
> -
> -	ret = mutex_lock_interruptible(&dev->struct_mutex);
> -	if (ret)
> -		return ret;
> -	intel_runtime_pm_get(dev_priv);
> -
> -	spin_lock_irq(&dev_priv->uncore.lock);
> -	forcewake_count = dev_priv->uncore.fw_domain[FW_DOMAIN_ID_RENDER].wake_count;
> -	spin_unlock_irq(&dev_priv->uncore.lock);
> +	int count = 0;
>  
> +	forcewake_count = READ_ONCE(dev_priv->uncore.fw_domain[FW_DOMAIN_ID_RENDER].wake_count);
>  	if (forcewake_count) {
>  		seq_puts(m, "RC information inaccurate because somebody "
>  			    "holds a forcewake reference \n");
> @@ -1546,13 +1529,11 @@ static int gen6_drpc_info(struct seq_file *m)
>  		gen9_powergate_enable = I915_READ(GEN9_PG_ENABLE);
>  		gen9_powergate_status = I915_READ(GEN9_PWRGT_DOMAIN_STATUS);
>  	}
> -	mutex_unlock(&dev->struct_mutex);
> +
>  	mutex_lock(&dev_priv->rps.hw_lock);
>  	sandybridge_pcode_read(dev_priv, GEN6_PCODE_READ_RC6VIDS, &rc6vids);
>  	mutex_unlock(&dev_priv->rps.hw_lock);
>  
> -	intel_runtime_pm_put(dev_priv);
> -
>  	seq_printf(m, "Video Turbo Mode: %s\n",
>  		   yesno(rpmodectl1 & GEN6_RP_MEDIA_TURBO));
>  	seq_printf(m, "HW control enabled: %s\n",
> @@ -1629,13 +1610,20 @@ static int gen6_drpc_info(struct seq_file *m)
>  static int i915_drpc_info(struct seq_file *m, void *unused)
>  {
>  	struct drm_i915_private *dev_priv = node_to_i915(m->private);
> +	int err;
> +
> +	intel_runtime_pm_get(dev_priv);
>  
>  	if (IS_VALLEYVIEW(dev_priv) || IS_CHERRYVIEW(dev_priv))
> -		return vlv_drpc_info(m);
> +		err = vlv_drpc_info(m);
>  	else if (INTEL_GEN(dev_priv) >= 6)
> -		return gen6_drpc_info(m);
> +		err = gen6_drpc_info(m);
>  	else
> -		return ironlake_drpc_info(m);
> +		err = ironlake_drpc_info(m);
> +
> +	intel_runtime_pm_put(dev_priv);
> +
> +	return err;
>  }
>  
>  static int i915_frontbuffer_tracking(struct seq_file *m, void *unused)
> -- 
> 2.11.0
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 5+ messages in thread
* Re: [PATCH] drm/i915: Extend rpm wakelock for debugfs/i915_drpc_info
  2017-03-13 13:41 ` [PATCH] " Mika Kuoppala
@ 2017-03-13 14:10   ` Chris Wilson
  2017-03-13 14:28   ` Chris Wilson
  1 sibling, 0 replies; 5+ messages in thread
From: Chris Wilson @ 2017-03-13 14:10 UTC (permalink / raw)
  To: Mika Kuoppala; +Cc: intel-gfx

On Mon, Mar 13, 2017 at 03:41:34PM +0200, Mika Kuoppala wrote:
> Chris Wilson <chris@chris-wilson.co.uk> writes:
> 
> > i915_drpc_info missed covering a few register read with the runtime pm
> > wakelock. Be simple and cover the entire function with a single wakelock
> > so that new additions are not similarly missed in future.
> >
> >   WARNING: CPU: 2 PID: 1334 at drivers/gpu/drm/i915/intel_drv.h:1743 gen6_read32+0x192/0x1e0 [i915]
> >   RPM wakelock ref not held during HW access
> >   Modules linked in: rpcsec_gss_krb5 nfsv4 dns_resolver netconsole nfsd auth_rpcgss ipmi_watchdog ipmi_poweroff ipmi_devintf ipmi_msghandler overlay btrfs xor raid6_pq dm_mod sg sd_mod snd_hda_codec_hdmi snd_hda_codec_realtek snd_hda_codec_generic ata_generic pata_acpi intel_rapl x86_pkg_temp_thermal intel_powerclamp coretemp snd_hda_intel kvm_intel snd_hda_codec kvm eeepc_wmi irqbypass snd_hda_core crct10dif_pclmul crc32_pclmul crc32c_intel asus_wmi sparse_keymap ghash_clmulni_intel snd_hwdep i915 rfkill ppdev pcbc aesni_intel ata_piix crypto_simd glue_helper snd_pcm pata_via cryptd pcspkr snd_timer drm_kms_helper syscopyarea snd sysfillrect libata sysimgblt fb_sys_fops soundcore shpchp drm wmi parport_pc parport tpm_infineon video
> >   CPU: 2 PID: 1334 Comm: php5 Not tainted 4.10.0-rc8-01615-g1f58c8e #1
> >   Hardware name: System manufacturer System Product Name/P8H67-M PRO, BIOS 1002 04/01/2011
> >   Call Trace:
> >    dump_stack+0x63/0x8a
> >    __warn+0xcb/0xf0
> >    warn_slowpath_fmt+0x4f/0x60
> >    ? seq_vprintf+0x35/0x50
> >    gen6_read32+0x192/0x1e0 [i915]
> >    i915_drpc_info+0x55d/0x990 [i915]
> >    seq_read+0xf2/0x3b0
> >    full_proxy_read+0x51/0x80
> >    __vfs_read+0x28/0x130
> >    ? security_file_permission+0x9b/0xc0
> >    ? rw_verify_area+0x4e/0xb0
> >    vfs_read+0xa8/0x170
> >    SyS_read+0x46/0xa0
> >    entry_SYSCALL_64_fastpath+0x1a/0xa9
> >   RIP: 0033:0x7fd97bf175a0
> >   RSP: 002b:00007ffdf730db68 EFLAGS: 00000246 ORIG_RAX: 0000000000000000
> >   RAX: ffffffffffffffda RBX: 00007fd978028738 RCX: 00007fd97bf175a0
> >   RDX: 0000000000002000 RSI: 00007fd97740e0d8 RDI: 0000000000000005
> >   RBP: 0000000000000001 R08: 0000000000e97840 R09: 00007fd977ef8d58
> >   R10: 0000000000000027 R11: 0000000000000246 R12: 00007fd977ef8d58
> >   R13: 0000000000000000 R14: 0000000000eb4640 R15: 0000000000000000
> >
> > Reported-by: kernel test robot <xiaolong.ye@intel.com>
> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> 
> While reading this I pondered if mutexing the whole read sequence
> would add value. To give 'atomic' snapshot of state.

An atomic snapshot of state that may be updated by hw in some cases? :)
Otoh, adding locking around hw state reads makes debugging harder.

I don't think it would help very much, and hinder in others. Atomic
reports are nice, in theory, and sometime vital. Not convinced that this
is the case here.
-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] 5+ messages in thread
* Re: [PATCH] drm/i915: Extend rpm wakelock for debugfs/i915_drpc_info
  2017-03-13 13:41 ` [PATCH] " Mika Kuoppala
  2017-03-13 14:10   ` Chris Wilson
@ 2017-03-13 14:28   ` Chris Wilson
  1 sibling, 0 replies; 5+ messages in thread
From: Chris Wilson @ 2017-03-13 14:28 UTC (permalink / raw)
  To: Mika Kuoppala; +Cc: intel-gfx

On Mon, Mar 13, 2017 at 03:41:34PM +0200, Mika Kuoppala wrote:
> Chris Wilson <chris@chris-wilson.co.uk> writes:
> 
> > i915_drpc_info missed covering a few register read with the runtime pm
> > wakelock. Be simple and cover the entire function with a single wakelock
> > so that new additions are not similarly missed in future.
> >
> >   WARNING: CPU: 2 PID: 1334 at drivers/gpu/drm/i915/intel_drv.h:1743 gen6_read32+0x192/0x1e0 [i915]
> >   RPM wakelock ref not held during HW access
> >   Modules linked in: rpcsec_gss_krb5 nfsv4 dns_resolver netconsole nfsd auth_rpcgss ipmi_watchdog ipmi_poweroff ipmi_devintf ipmi_msghandler overlay btrfs xor raid6_pq dm_mod sg sd_mod snd_hda_codec_hdmi snd_hda_codec_realtek snd_hda_codec_generic ata_generic pata_acpi intel_rapl x86_pkg_temp_thermal intel_powerclamp coretemp snd_hda_intel kvm_intel snd_hda_codec kvm eeepc_wmi irqbypass snd_hda_core crct10dif_pclmul crc32_pclmul crc32c_intel asus_wmi sparse_keymap ghash_clmulni_intel snd_hwdep i915 rfkill ppdev pcbc aesni_intel ata_piix crypto_simd glue_helper snd_pcm pata_via cryptd pcspkr snd_timer drm_kms_helper syscopyarea snd sysfillrect libata sysimgblt fb_sys_fops soundcore shpchp drm wmi parport_pc parport tpm_infineon video
> >   CPU: 2 PID: 1334 Comm: php5 Not tainted 4.10.0-rc8-01615-g1f58c8e #1
> >   Hardware name: System manufacturer System Product Name/P8H67-M PRO, BIOS 1002 04/01/2011
> >   Call Trace:
> >    dump_stack+0x63/0x8a
> >    __warn+0xcb/0xf0
> >    warn_slowpath_fmt+0x4f/0x60
> >    ? seq_vprintf+0x35/0x50
> >    gen6_read32+0x192/0x1e0 [i915]
> >    i915_drpc_info+0x55d/0x990 [i915]
> >    seq_read+0xf2/0x3b0
> >    full_proxy_read+0x51/0x80
> >    __vfs_read+0x28/0x130
> >    ? security_file_permission+0x9b/0xc0
> >    ? rw_verify_area+0x4e/0xb0
> >    vfs_read+0xa8/0x170
> >    SyS_read+0x46/0xa0
> >    entry_SYSCALL_64_fastpath+0x1a/0xa9
> >   RIP: 0033:0x7fd97bf175a0
> >   RSP: 002b:00007ffdf730db68 EFLAGS: 00000246 ORIG_RAX: 0000000000000000
> >   RAX: ffffffffffffffda RBX: 00007fd978028738 RCX: 00007fd97bf175a0
> >   RDX: 0000000000002000 RSI: 00007fd97740e0d8 RDI: 0000000000000005
> >   RBP: 0000000000000001 R08: 0000000000e97840 R09: 00007fd977ef8d58
> >   R10: 0000000000000027 R11: 0000000000000246 R12: 00007fd977ef8d58
> >   R13: 0000000000000000 R14: 0000000000eb4640 R15: 0000000000000000
> >
> > Reported-by: kernel test robot <xiaolong.ye@intel.com>
> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> 
> While reading this I pondered if mutexing the whole read sequence
> would add value. To give 'atomic' snapshot of state.
> 
> But that is a separate issue.
> 
> Reviewed-by: Mika Kuoppala <mika.kuoppala@intel.com>

Thanks, pushed.
-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] 5+ messages in thread
end of thread, other threads:[~2017-03-13 14:28 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-03-13  9:56 [PATCH] drm/i915: Extend rpm wakelock for debugfs/i915_drpc_info Chris Wilson
2017-03-13 10:18 ` ✓ Fi.CI.BAT: success for " Patchwork
2017-03-13 13:41 ` [PATCH] " Mika Kuoppala
2017-03-13 14:10   ` Chris Wilson
2017-03-13 14:28   ` Chris Wilson

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.