* [PATCH] drm/i915: fix comment on I915_{READ,WRITE}_FW
@ 2016-10-25 12:15 Arkadiusz Hiler
2016-10-25 12:21 ` [PATCH] drm/i915: fix comment on I915_{READ, WRITE}_FW Chris Wilson
` (3 more replies)
0 siblings, 4 replies; 10+ messages in thread
From: Arkadiusz Hiler @ 2016-10-25 12:15 UTC (permalink / raw)
To: intel-gfx; +Cc: Matthew Auld
Comment mentioned use of intel_uncore_forcewake_irq{unlock, lock}
functions which are nonexistent (and never were).
The description was also incomplete and could cause confusion. Updated
comment is more elaborate on usage and caveats.
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Matthew Auld <matthew.auld@intel.com>
Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
Signed-off-by: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
---
drivers/gpu/drm/i915/i915_drv.h | 28 +++++++++++++++++++++++++---
1 file changed, 25 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index b4cb1f0..39238fc 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -3840,11 +3840,33 @@ __raw_write(64, q)
#undef __raw_write
/* These are untraced mmio-accessors that are only valid to be used inside
- * critical sections inside IRQ handlers where forcewake is explicitly
+ * critical sections, such as inside IRQ handlers, where forcewake is explicitly
* controlled.
+ *
* Think twice, and think again, before using these.
- * Note: Should only be used between intel_uncore_forcewake_irqlock() and
- * intel_uncore_forcewake_irqunlock().
+ *
+ * As an example, these accessors can possibly be used between:
+ *
+ * spin_lock_irq(&dev_priv->uncore.lock);
+ * intel_uncore_forcewake_get();
+ *
+ * and
+ *
+ * intel_uncore_forcewake_put();
+ * spin_unlock_irq(&dev_priv->uncore.lock);
+ *
+ *
+ * Note: some registers may not need forcewake held, so
+ * intel_uncore_forcewake_{get,put} can be omitted, see
+ * intel_uncore_forcewake_for_reg().
+ *
+ * Certain architectures will die if the same cacheline is concurrently accessed
+ * by different clients (e.g. Ivybridge). Access to registers should therefore
+ * generally be serialised, by either the dev_priv->uncore.lock or a more
+ * localised lock guarding all access to that bank of registers.
+ *
+ * Code may be serialised by different lock, so immediate
+ * spin_{lock,unlock}_irq() may not be necessary.
*/
#define I915_READ_FW(reg__) __raw_i915_read32(dev_priv, (reg__))
#define I915_WRITE_FW(reg__, val__) __raw_i915_write32(dev_priv, (reg__), (val__))
--
2.7.4
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH] drm/i915: fix comment on I915_{READ, WRITE}_FW
2016-10-25 12:15 [PATCH] drm/i915: fix comment on I915_{READ,WRITE}_FW Arkadiusz Hiler
@ 2016-10-25 12:21 ` Chris Wilson
2016-10-25 12:32 ` [PATCH] drm/i915: fix comment on I915_{READ,WRITE}_FW Arkadiusz Hiler
` (2 subsequent siblings)
3 siblings, 0 replies; 10+ messages in thread
From: Chris Wilson @ 2016-10-25 12:21 UTC (permalink / raw)
To: Arkadiusz Hiler; +Cc: intel-gfx, Matthew Auld
On Tue, Oct 25, 2016 at 02:15:23PM +0200, Arkadiusz Hiler wrote:
> Comment mentioned use of intel_uncore_forcewake_irq{unlock, lock}
> functions which are nonexistent (and never were).
>
> The description was also incomplete and could cause confusion. Updated
> comment is more elaborate on usage and caveats.
>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Matthew Auld <matthew.auld@intel.com>
> Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
> Signed-off-by: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
> ---
> drivers/gpu/drm/i915/i915_drv.h | 28 +++++++++++++++++++++++++---
> 1 file changed, 25 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index b4cb1f0..39238fc 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -3840,11 +3840,33 @@ __raw_write(64, q)
> #undef __raw_write
>
> /* These are untraced mmio-accessors that are only valid to be used inside
> - * critical sections inside IRQ handlers where forcewake is explicitly
> + * critical sections, such as inside IRQ handlers, where forcewake is explicitly
> * controlled.
> + *
> * Think twice, and think again, before using these.
> - * Note: Should only be used between intel_uncore_forcewake_irqlock() and
> - * intel_uncore_forcewake_irqunlock().
> + *
> + * As an example, these accessors can possibly be used between:
> + *
> + * spin_lock_irq(&dev_priv->uncore.lock);
> + * intel_uncore_forcewake_get();
Ugh, intel_uncore_forcewake_get__locked();
> + *
> + * and
> + *
> + * intel_uncore_forcewake_put();
intel_uncore_forcewake_put__locked();
> + * spin_unlock_irq(&dev_priv->uncore.lock);
--
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] 10+ messages in thread
* [PATCH] drm/i915: fix comment on I915_{READ,WRITE}_FW
2016-10-25 12:15 [PATCH] drm/i915: fix comment on I915_{READ,WRITE}_FW Arkadiusz Hiler
2016-10-25 12:21 ` [PATCH] drm/i915: fix comment on I915_{READ, WRITE}_FW Chris Wilson
@ 2016-10-25 12:32 ` Arkadiusz Hiler
2016-10-25 12:48 ` [PATCH v2] drm/i915: fix comment on I915_{READ, WRITE}_FW Arkadiusz Hiler
2016-10-25 13:46 ` ✗ Fi.CI.BAT: failure for drm/i915: fix comment on I915_{READ,WRITE}_FW (rev2) Patchwork
3 siblings, 0 replies; 10+ messages in thread
From: Arkadiusz Hiler @ 2016-10-25 12:32 UTC (permalink / raw)
To: intel-gfx; +Cc: Matthew Auld
Comment mentioned use of intel_uncore_forcewake_irq{unlock, lock}
functions which are nonexistent (and never were).
The description was also incomplete and could cause confusion. Updated
comment is more elaborate on usage and caveats.
v2: mention __locked variant of intel_uncore_forcewake_{get,put} instead
of plain ones
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Matthew Auld <matthew.auld@intel.com>
Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
Signed-off-by: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
---
drivers/gpu/drm/i915/i915_drv.h | 28 +++++++++++++++++++++++++---
1 file changed, 25 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index b4cb1f0..e0f3fa4 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -3840,11 +3840,33 @@ __raw_write(64, q)
#undef __raw_write
/* These are untraced mmio-accessors that are only valid to be used inside
- * critical sections inside IRQ handlers where forcewake is explicitly
+ * critical sections, such as inside IRQ handlers, where forcewake is explicitly
* controlled.
+ *
* Think twice, and think again, before using these.
- * Note: Should only be used between intel_uncore_forcewake_irqlock() and
- * intel_uncore_forcewake_irqunlock().
+ *
+ * As an example, these accessors can possibly be used between:
+ *
+ * spin_lock_irq(&dev_priv->uncore.lock);
+ * intel_uncore_forcewake_get__locked();
+ *
+ * and
+ *
+ * intel_uncore_forcewake_put__locked();
+ * spin_unlock_irq(&dev_priv->uncore.lock);
+ *
+ *
+ * Note: some registers may not need forcewake held, so
+ * intel_uncore_forcewake_{get,put} can be omitted, see
+ * intel_uncore_forcewake_for_reg().
+ *
+ * Certain architectures will die if the same cacheline is concurrently accessed
+ * by different clients (e.g. Ivybridge). Access to registers should therefore
+ * generally be serialised, by either the dev_priv->uncore.lock or a more
+ * localised lock guarding all access to that bank of registers.
+ *
+ * Code may be serialised by different lock, so immediate
+ * spin_{lock,unlock}_irq() may not be necessary.
*/
#define I915_READ_FW(reg__) __raw_i915_read32(dev_priv, (reg__))
#define I915_WRITE_FW(reg__, val__) __raw_i915_write32(dev_priv, (reg__), (val__))
--
2.7.4
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH v2] drm/i915: fix comment on I915_{READ, WRITE}_FW
2016-10-25 12:15 [PATCH] drm/i915: fix comment on I915_{READ,WRITE}_FW Arkadiusz Hiler
2016-10-25 12:21 ` [PATCH] drm/i915: fix comment on I915_{READ, WRITE}_FW Chris Wilson
2016-10-25 12:32 ` [PATCH] drm/i915: fix comment on I915_{READ,WRITE}_FW Arkadiusz Hiler
@ 2016-10-25 12:48 ` Arkadiusz Hiler
2016-10-25 16:00 ` Matthew Auld
2016-10-25 20:41 ` Chris Wilson
2016-10-25 13:46 ` ✗ Fi.CI.BAT: failure for drm/i915: fix comment on I915_{READ,WRITE}_FW (rev2) Patchwork
3 siblings, 2 replies; 10+ messages in thread
From: Arkadiusz Hiler @ 2016-10-25 12:48 UTC (permalink / raw)
To: intel-gfx; +Cc: Matthew Auld
Comment mentioned use of intel_uncore_forcewake_irq{unlock, lock}
functions which are nonexistent (and never were).
The description was also incomplete and could cause confusion. Updated
comment is more elaborate on usage and caveats.
v2: mention __locked variant of intel_uncore_forcewake_{get,put} instead
of plain ones
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Matthew Auld <matthew.auld@intel.com>
Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
Signed-off-by: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
---
drivers/gpu/drm/i915/i915_drv.h | 28 +++++++++++++++++++++++++---
1 file changed, 25 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index b4cb1f0..e0f3fa4 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -3840,11 +3840,33 @@ __raw_write(64, q)
#undef __raw_write
/* These are untraced mmio-accessors that are only valid to be used inside
- * critical sections inside IRQ handlers where forcewake is explicitly
+ * critical sections, such as inside IRQ handlers, where forcewake is explicitly
* controlled.
+ *
* Think twice, and think again, before using these.
- * Note: Should only be used between intel_uncore_forcewake_irqlock() and
- * intel_uncore_forcewake_irqunlock().
+ *
+ * As an example, these accessors can possibly be used between:
+ *
+ * spin_lock_irq(&dev_priv->uncore.lock);
+ * intel_uncore_forcewake_get__locked();
+ *
+ * and
+ *
+ * intel_uncore_forcewake_put__locked();
+ * spin_unlock_irq(&dev_priv->uncore.lock);
+ *
+ *
+ * Note: some registers may not need forcewake held, so
+ * intel_uncore_forcewake_{get,put} can be omitted, see
+ * intel_uncore_forcewake_for_reg().
+ *
+ * Certain architectures will die if the same cacheline is concurrently accessed
+ * by different clients (e.g. Ivybridge). Access to registers should therefore
+ * generally be serialised, by either the dev_priv->uncore.lock or a more
+ * localised lock guarding all access to that bank of registers.
+ *
+ * Code may be serialised by different lock, so immediate
+ * spin_{lock,unlock}_irq() may not be necessary.
*/
#define I915_READ_FW(reg__) __raw_i915_read32(dev_priv, (reg__))
#define I915_WRITE_FW(reg__, val__) __raw_i915_write32(dev_priv, (reg__), (val__))
--
2.7.4
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 10+ messages in thread
* ✗ Fi.CI.BAT: failure for drm/i915: fix comment on I915_{READ,WRITE}_FW (rev2)
2016-10-25 12:15 [PATCH] drm/i915: fix comment on I915_{READ,WRITE}_FW Arkadiusz Hiler
` (2 preceding siblings ...)
2016-10-25 12:48 ` [PATCH v2] drm/i915: fix comment on I915_{READ, WRITE}_FW Arkadiusz Hiler
@ 2016-10-25 13:46 ` Patchwork
2016-10-25 14:27 ` Saarinen, Jani
3 siblings, 1 reply; 10+ messages in thread
From: Patchwork @ 2016-10-25 13:46 UTC (permalink / raw)
To: Arkadiusz Hiler; +Cc: intel-gfx
== Series Details ==
Series: drm/i915: fix comment on I915_{READ,WRITE}_FW (rev2)
URL : https://patchwork.freedesktop.org/series/14334/
State : failure
== Summary ==
Series 14334v2 drm/i915: fix comment on I915_{READ,WRITE}_FW
https://patchwork.freedesktop.org/api/1.0/series/14334/revisions/2/mbox/
Test drv_module_reload_basic:
pass -> DMESG-WARN (fi-skl-6700hq)
skip -> PASS (fi-skl-6260u)
Test gem_exec_suspend:
Subgroup basic-s3:
dmesg-warn -> PASS (fi-skl-6700hq)
Test gem_sync:
Subgroup basic-many-each:
pass -> FAIL (fi-ilk-650)
Test kms_pipe_crc_basic:
Subgroup suspend-read-crc-pipe-a:
dmesg-warn -> PASS (fi-skl-6700hq)
Subgroup suspend-read-crc-pipe-b:
pass -> DMESG-WARN (fi-skl-6770hq)
dmesg-warn -> PASS (fi-skl-6700hq)
Subgroup suspend-read-crc-pipe-c:
dmesg-warn -> PASS (fi-skl-6700hq)
fi-bdw-5557u total:246 pass:231 dwarn:0 dfail:0 fail:0 skip:15
fi-bsw-n3050 total:246 pass:204 dwarn:0 dfail:0 fail:0 skip:42
fi-bxt-t5700 total:246 pass:216 dwarn:0 dfail:0 fail:0 skip:30
fi-byt-j1900 total:246 pass:215 dwarn:0 dfail:0 fail:0 skip:31
fi-byt-n2820 total:246 pass:210 dwarn:0 dfail:0 fail:0 skip:36
fi-hsw-4770 total:246 pass:224 dwarn:0 dfail:0 fail:0 skip:22
fi-hsw-4770r total:246 pass:223 dwarn:0 dfail:0 fail:0 skip:23
fi-ilk-650 total:246 pass:184 dwarn:0 dfail:0 fail:1 skip:61
fi-ivb-3520m total:246 pass:220 dwarn:0 dfail:0 fail:0 skip:26
fi-ivb-3770 total:246 pass:220 dwarn:0 dfail:0 fail:0 skip:26
fi-kbl-7200u total:246 pass:222 dwarn:0 dfail:0 fail:0 skip:24
fi-skl-6260u total:246 pass:232 dwarn:0 dfail:0 fail:0 skip:14
fi-skl-6700hq total:246 pass:222 dwarn:1 dfail:0 fail:0 skip:23
fi-skl-6700k total:246 pass:222 dwarn:1 dfail:0 fail:0 skip:23
fi-skl-6770hq total:246 pass:231 dwarn:1 dfail:0 fail:0 skip:14
fi-snb-2520m total:246 pass:209 dwarn:0 dfail:0 fail:0 skip:37
fi-snb-2600 total:246 pass:208 dwarn:0 dfail:0 fail:0 skip:38
cd1dba8d045ce0d59029226108f0ad7b35a9d061 drm-intel-nightly: 2016y-10m-25d-09h-26m-24s UTC integration manifest
98dec85 drm/i915: fix comment on I915_{READ, WRITE}_FW
Full results at https://intel-gfx-ci.01.org/CI/Patchwork_2809/
== Logs ==
For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_2809/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: ✗ Fi.CI.BAT: failure for drm/i915: fix comment on I915_{READ,WRITE}_FW (rev2)
2016-10-25 13:46 ` ✗ Fi.CI.BAT: failure for drm/i915: fix comment on I915_{READ,WRITE}_FW (rev2) Patchwork
@ 2016-10-25 14:27 ` Saarinen, Jani
2016-10-25 14:53 ` Arkadiusz Hiler
0 siblings, 1 reply; 10+ messages in thread
From: Saarinen, Jani @ 2016-10-25 14:27 UTC (permalink / raw)
To: intel-gfx, Hiler, Arkadiusz
> == Series Details ==
>
> Series: drm/i915: fix comment on I915_{READ,WRITE}_FW (rev2)
> URL : https://patchwork.freedesktop.org/series/14334/
> State : failure
>
> == Summary ==
>
> Series 14334v2 drm/i915: fix comment on I915_{READ,WRITE}_FW
> https://patchwork.freedesktop.org/api/1.0/series/14334/revisions/2/mbox/
>
> Test drv_module_reload_basic:
> pass -> DMESG-WARN (fi-skl-6700hq)
> skip -> PASS (fi-skl-6260u)
> Test gem_exec_suspend:
> Subgroup basic-s3:
> dmesg-warn -> PASS (fi-skl-6700hq)
> Test gem_sync:
> Subgroup basic-many-each:
> pass -> FAIL (fi-ilk-650)
All can now see data details on logs link below but here still few times to remind ;)
Out
IGT-Version: 1.16-g1df15af (x86_64) (Linux: 4.9.0-rc2-CI-Patchwork_2809+ x86_64)
Using Legacy submission
render completed 737 cycles
bsd completed 324 cycles
Stack trace:
#0 [__igt_fail_assert+0x101]
#1 [store_many+0x284]
#2 [<unknown>+0x284]
Subtest basic-many-each: FAIL (5.573s)
Err
(gem_sync:7909) CRITICAL: Test assertion failure function store_many, file gem_sync.c:506:
(gem_sync:7909) CRITICAL: Failed assertion: intel_detect_and_clear_missed_interrupts(fd) == 0
(gem_sync:7909) CRITICAL: error: 4 != 0
Subtest basic-many-each failed.
**** DEBUG ****
(gem_sync:7909) INFO: render completed 737 cycles
(gem_sync:7909) INFO: bsd completed 324 cycles
(gem_sync:7909) CRITICAL: Test assertion failure function store_many, file gem_sync.c:506:
(gem_sync:7909) CRITICAL: Failed assertion: intel_detect_and_clear_missed_interrupts(fd) == 0
(gem_sync:7909) CRITICAL: error: 4 != 0
**** END ****
> Test kms_pipe_crc_basic:
> Subgroup suspend-read-crc-pipe-a:
> dmesg-warn -> PASS (fi-skl-6700hq)
> Subgroup suspend-read-crc-pipe-b:
> pass -> DMESG-WARN (fi-skl-6770hq)
> dmesg-warn -> PASS (fi-skl-6700hq)
> Subgroup suspend-read-crc-pipe-c:
> dmesg-warn -> PASS (fi-skl-6700hq)
>
> fi-bdw-5557u total:246 pass:231 dwarn:0 dfail:0 fail:0 skip:15
> fi-bsw-n3050 total:246 pass:204 dwarn:0 dfail:0 fail:0 skip:42
> fi-bxt-t5700 total:246 pass:216 dwarn:0 dfail:0 fail:0 skip:30
> fi-byt-j1900 total:246 pass:215 dwarn:0 dfail:0 fail:0 skip:31
> fi-byt-n2820 total:246 pass:210 dwarn:0 dfail:0 fail:0 skip:36
> fi-hsw-4770 total:246 pass:224 dwarn:0 dfail:0 fail:0 skip:22
> fi-hsw-4770r total:246 pass:223 dwarn:0 dfail:0 fail:0 skip:23
> fi-ilk-650 total:246 pass:184 dwarn:0 dfail:0 fail:1 skip:61
> fi-ivb-3520m total:246 pass:220 dwarn:0 dfail:0 fail:0 skip:26
> fi-ivb-3770 total:246 pass:220 dwarn:0 dfail:0 fail:0 skip:26
> fi-kbl-7200u total:246 pass:222 dwarn:0 dfail:0 fail:0 skip:24
> fi-skl-6260u total:246 pass:232 dwarn:0 dfail:0 fail:0 skip:14
> fi-skl-6700hq total:246 pass:222 dwarn:1 dfail:0 fail:0 skip:23
> fi-skl-6700k total:246 pass:222 dwarn:1 dfail:0 fail:0 skip:23
> fi-skl-6770hq total:246 pass:231 dwarn:1 dfail:0 fail:0 skip:14
> fi-snb-2520m total:246 pass:209 dwarn:0 dfail:0 fail:0 skip:37
> fi-snb-2600 total:246 pass:208 dwarn:0 dfail:0 fail:0 skip:38
>
> cd1dba8d045ce0d59029226108f0ad7b35a9d061 drm-intel-nightly: 2016y-10m-
> 25d-09h-26m-24s UTC integration manifest
> 98dec85 drm/i915: fix comment on I915_{READ, WRITE}_FW
>
> Full results at https://intel-gfx-ci.01.org/CI/Patchwork_2809/
>
> == Logs ==
>
> For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_2809/
Jani Saarinen
Intel Finland Oy - BIC 0357606-4 - Westendinkatu 7, 02160 Espoo
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: ✗ Fi.CI.BAT: failure for drm/i915: fix comment on I915_{READ,WRITE}_FW (rev2)
2016-10-25 14:27 ` Saarinen, Jani
@ 2016-10-25 14:53 ` Arkadiusz Hiler
0 siblings, 0 replies; 10+ messages in thread
From: Arkadiusz Hiler @ 2016-10-25 14:53 UTC (permalink / raw)
To: Saarinen, Jani; +Cc: intel-gfx
On Tue, Oct 25, 2016 at 04:27:26PM +0200, Saarinen, Jani wrote:
> > == Series Details ==
> >
> > Series: drm/i915: fix comment on I915_{READ,WRITE}_FW (rev2)
> > URL : https://patchwork.freedesktop.org/series/14334/
> > State : failure
> >
> > == Summary ==
> >
> > Series 14334v2 drm/i915: fix comment on I915_{READ,WRITE}_FW
> > https://patchwork.freedesktop.org/api/1.0/series/14334/revisions/2/mbox/
> >
> > Test drv_module_reload_basic:
> > pass -> DMESG-WARN (fi-skl-6700hq)
> > skip -> PASS (fi-skl-6260u)
> > Test gem_exec_suspend:
> > Subgroup basic-s3:
> > dmesg-warn -> PASS (fi-skl-6700hq)
> > Test gem_sync:
> > Subgroup basic-many-each:
> > pass -> FAIL (fi-ilk-650)
> All can now see data details on logs link below but here still few times to remind ;)
Thanks! I was figuring out what to do with this error, therefore my
dealy with the reply. I had access to the results even before c;
> Out
> IGT-Version: 1.16-g1df15af (x86_64) (Linux: 4.9.0-rc2-CI-Patchwork_2809+ x86_64)
> Using Legacy submission
> render completed 737 cycles
> bsd completed 324 cycles
> Stack trace:
> #0 [__igt_fail_assert+0x101]
> #1 [store_many+0x284]
> #2 [<unknown>+0x284]
> Subtest basic-many-each: FAIL (5.573s)
>
> Err
> (gem_sync:7909) CRITICAL: Test assertion failure function store_many, file gem_sync.c:506:
> (gem_sync:7909) CRITICAL: Failed assertion: intel_detect_and_clear_missed_interrupts(fd) == 0
> (gem_sync:7909) CRITICAL: error: 4 != 0
> Subtest basic-many-each failed.
> **** DEBUG ****
> (gem_sync:7909) INFO: render completed 737 cycles
> (gem_sync:7909) INFO: bsd completed 324 cycles
> (gem_sync:7909) CRITICAL: Test assertion failure function store_many, file gem_sync.c:506:
> (gem_sync:7909) CRITICAL: Failed assertion: intel_detect_and_clear_missed_interrupts(fd) == 0
> (gem_sync:7909) CRITICAL: error: 4 != 0
> **** END ****
That's definately not caused by this change as only one comment in the
code is affected.
Should I put bugzilla on it? (I haven't found any on the issue already).
> > Test kms_pipe_crc_basic:
> > Subgroup suspend-read-crc-pipe-a:
> > dmesg-warn -> PASS (fi-skl-6700hq)
> > Subgroup suspend-read-crc-pipe-b:
> > pass -> DMESG-WARN (fi-skl-6770hq)
> > dmesg-warn -> PASS (fi-skl-6700hq)
> > Subgroup suspend-read-crc-pipe-c:
> > dmesg-warn -> PASS (fi-skl-6700hq)
> >
> >
> > cd1dba8d045ce0d59029226108f0ad7b35a9d061 drm-intel-nightly: 2016y-10m-
> > 25d-09h-26m-24s UTC integration manifest
> > 98dec85 drm/i915: fix comment on I915_{READ, WRITE}_FW
> >
> > Full results at https://intel-gfx-ci.01.org/CI/Patchwork_2809/
> >
> > == Logs ==
> >
> > For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_2809/
--
Cheers,
Arek
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2] drm/i915: fix comment on I915_{READ, WRITE}_FW
2016-10-25 12:48 ` [PATCH v2] drm/i915: fix comment on I915_{READ, WRITE}_FW Arkadiusz Hiler
@ 2016-10-25 16:00 ` Matthew Auld
2016-10-25 20:41 ` Chris Wilson
1 sibling, 0 replies; 10+ messages in thread
From: Matthew Auld @ 2016-10-25 16:00 UTC (permalink / raw)
To: Arkadiusz Hiler; +Cc: Intel Graphics Development, Matthew Auld
On 25 October 2016 at 13:48, Arkadiusz Hiler <arkadiusz.hiler@intel.com> wrote:
> Comment mentioned use of intel_uncore_forcewake_irq{unlock, lock}
> functions which are nonexistent (and never were).
>
> The description was also incomplete and could cause confusion. Updated
> comment is more elaborate on usage and caveats.
>
> v2: mention __locked variant of intel_uncore_forcewake_{get,put} instead
> of plain ones
>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Matthew Auld <matthew.auld@intel.com>
> Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
> Signed-off-by: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
Reviewed-by: Matthew Auld <matthew.auld@intel.com>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH v2] drm/i915: fix comment on I915_{READ, WRITE}_FW
2016-10-25 12:48 ` [PATCH v2] drm/i915: fix comment on I915_{READ, WRITE}_FW Arkadiusz Hiler
2016-10-25 16:00 ` Matthew Auld
@ 2016-10-25 20:41 ` Chris Wilson
2016-10-26 11:53 ` Mika Kuoppala
1 sibling, 1 reply; 10+ messages in thread
From: Chris Wilson @ 2016-10-25 20:41 UTC (permalink / raw)
To: Arkadiusz Hiler; +Cc: intel-gfx, Matthew Auld
On Tue, Oct 25, 2016 at 02:48:02PM +0200, Arkadiusz Hiler wrote:
> Comment mentioned use of intel_uncore_forcewake_irq{unlock, lock}
> functions which are nonexistent (and never were).
>
> The description was also incomplete and could cause confusion. Updated
> comment is more elaborate on usage and caveats.
>
> v2: mention __locked variant of intel_uncore_forcewake_{get,put} instead
> of plain ones
>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Matthew Auld <matthew.auld@intel.com>
> Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
> Signed-off-by: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
> ---
> drivers/gpu/drm/i915/i915_drv.h | 28 +++++++++++++++++++++++++---
> 1 file changed, 25 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index b4cb1f0..e0f3fa4 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -3840,11 +3840,33 @@ __raw_write(64, q)
> #undef __raw_write
>
> /* These are untraced mmio-accessors that are only valid to be used inside
> - * critical sections inside IRQ handlers where forcewake is explicitly
> + * critical sections, such as inside IRQ handlers, where forcewake is explicitly
> * controlled.
> + *
> * Think twice, and think again, before using these.
> - * Note: Should only be used between intel_uncore_forcewake_irqlock() and
> - * intel_uncore_forcewake_irqunlock().
> + *
> + * As an example, these accessors can possibly be used between:
> + *
> + * spin_lock_irq(&dev_priv->uncore.lock);
> + * intel_uncore_forcewake_get__locked();
> + *
> + * and
> + *
> + * intel_uncore_forcewake_put__locked();
> + * spin_unlock_irq(&dev_priv->uncore.lock);
> + *
> + *
> + * Note: some registers may not need forcewake held, so
> + * intel_uncore_forcewake_{get,put} can be omitted, see
> + * intel_uncore_forcewake_for_reg().
> + *
> + * Certain architectures will die if the same cacheline is concurrently accessed
> + * by different clients (e.g. Ivybridge). Access to registers should therefore
e.g. on Ivybridge
> + * generally be serialised, by either the dev_priv->uncore.lock or a more
> + * localised lock guarding all access to that bank of registers.
> + *
> + * Code may be serialised by different lock, so immediate
> + * spin_{lock,unlock}_irq() may not be necessary.
This last sentence is redundant since the reason why we need some lock
somewhere is given above.
With that,
Reviewed-by: Chris Wilson <chris@chris-wilsono.c.uk>
-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] 10+ messages in thread
* Re: [PATCH v2] drm/i915: fix comment on I915_{READ, WRITE}_FW
2016-10-25 20:41 ` Chris Wilson
@ 2016-10-26 11:53 ` Mika Kuoppala
0 siblings, 0 replies; 10+ messages in thread
From: Mika Kuoppala @ 2016-10-26 11:53 UTC (permalink / raw)
To: Chris Wilson, Arkadiusz Hiler; +Cc: intel-gfx, Matthew Auld
Chris Wilson <chris@chris-wilson.co.uk> writes:
> On Tue, Oct 25, 2016 at 02:48:02PM +0200, Arkadiusz Hiler wrote:
>> Comment mentioned use of intel_uncore_forcewake_irq{unlock, lock}
>> functions which are nonexistent (and never were).
>>
>> The description was also incomplete and could cause confusion. Updated
>> comment is more elaborate on usage and caveats.
>>
>> v2: mention __locked variant of intel_uncore_forcewake_{get,put} instead
>> of plain ones
>>
>> Cc: Chris Wilson <chris@chris-wilson.co.uk>
>> Cc: Matthew Auld <matthew.auld@intel.com>
>> Cc: Mika Kuoppala <mika.kuoppala@linux.intel.com>
>> Signed-off-by: Arkadiusz Hiler <arkadiusz.hiler@intel.com>
>> ---
>> drivers/gpu/drm/i915/i915_drv.h | 28 +++++++++++++++++++++++++---
>> 1 file changed, 25 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
>> index b4cb1f0..e0f3fa4 100644
>> --- a/drivers/gpu/drm/i915/i915_drv.h
>> +++ b/drivers/gpu/drm/i915/i915_drv.h
>> @@ -3840,11 +3840,33 @@ __raw_write(64, q)
>> #undef __raw_write
>>
>> /* These are untraced mmio-accessors that are only valid to be used inside
>> - * critical sections inside IRQ handlers where forcewake is explicitly
>> + * critical sections, such as inside IRQ handlers, where forcewake is explicitly
>> * controlled.
>> + *
>> * Think twice, and think again, before using these.
>> - * Note: Should only be used between intel_uncore_forcewake_irqlock() and
>> - * intel_uncore_forcewake_irqunlock().
>> + *
>> + * As an example, these accessors can possibly be used between:
>> + *
>> + * spin_lock_irq(&dev_priv->uncore.lock);
>> + * intel_uncore_forcewake_get__locked();
>> + *
>> + * and
>> + *
>> + * intel_uncore_forcewake_put__locked();
>> + * spin_unlock_irq(&dev_priv->uncore.lock);
>> + *
>> + *
>> + * Note: some registers may not need forcewake held, so
>> + * intel_uncore_forcewake_{get,put} can be omitted, see
>> + * intel_uncore_forcewake_for_reg().
>> + *
>> + * Certain architectures will die if the same cacheline is concurrently accessed
>> + * by different clients (e.g. Ivybridge). Access to registers should therefore
>
> e.g. on Ivybridge
>
>> + * generally be serialised, by either the dev_priv->uncore.lock or a more
>> + * localised lock guarding all access to that bank of registers.
>> + *
>> + * Code may be serialised by different lock, so immediate
>> + * spin_{lock,unlock}_irq() may not be necessary.
>
> This last sentence is redundant since the reason why we need some lock
> somewhere is given above.
>
Squashed those out and pushed, thanks you for patch and review.
-Mika
> With that,
>
> Reviewed-by: Chris Wilson <chris@chris-wilsono.c.uk>
> -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] 10+ messages in thread
end of thread, other threads:[~2016-10-26 11:54 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-10-25 12:15 [PATCH] drm/i915: fix comment on I915_{READ,WRITE}_FW Arkadiusz Hiler
2016-10-25 12:21 ` [PATCH] drm/i915: fix comment on I915_{READ, WRITE}_FW Chris Wilson
2016-10-25 12:32 ` [PATCH] drm/i915: fix comment on I915_{READ,WRITE}_FW Arkadiusz Hiler
2016-10-25 12:48 ` [PATCH v2] drm/i915: fix comment on I915_{READ, WRITE}_FW Arkadiusz Hiler
2016-10-25 16:00 ` Matthew Auld
2016-10-25 20:41 ` Chris Wilson
2016-10-26 11:53 ` Mika Kuoppala
2016-10-25 13:46 ` ✗ Fi.CI.BAT: failure for drm/i915: fix comment on I915_{READ,WRITE}_FW (rev2) Patchwork
2016-10-25 14:27 ` Saarinen, Jani
2016-10-25 14:53 ` Arkadiusz Hiler
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.