| Message ID | 1477397723-24616-1-git-send-email-arkadiusz.hiler@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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);
> == 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
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/
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__))
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(-)