[PATCH v2] drm/i915: Don't allow overuse of __intel_wait_for_register_fw()

[PATCH v2] drm/i915: Don't allow overuse of __intel_wait_for_register_fw()

[Michal Wajdeczko]

This function should not be called with long timeouts in atomic context.
Annotate it as might_sleep if timeout is longer than 10us.

v2: fix comment (Michal)

Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
Suggested-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/intel_uncore.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c
index 1deb1a4..eb38392 100644
--- a/drivers/gpu/drm/i915/intel_uncore.c
+++ b/drivers/gpu/drm/i915/intel_uncore.c
@@ -1600,6 +1600,8 @@ static int gen6_reset_engines(struct drm_i915_private *dev_priv,
  *     (I915_READ_FW(reg) & mask) == value
  *
  * Otherwise, the wait will timeout after @slow_timeout_ms milliseconds.
+ * For atomic context @slow_timeout_ms must be zero and @fast_timeout_us
+ * must be not larger than 10 microseconds.
  *
  * Note that this routine assumes the caller holds forcewake asserted, it is
  * not suitable for very long waits. See intel_wait_for_register() if you
@@ -1620,6 +1622,9 @@ int __intel_wait_for_register_fw(struct drm_i915_private *dev_priv,
 #define done (((reg_value = I915_READ_FW(reg)) & mask) == value)
 	int ret;
 
+	/* Catch any overuse of this function */
+	might_sleep_if(fast_timeout_us > 10 || slow_timeout_ms);
+
 	if (fast_timeout_us > 10)
 		ret = _wait_for(done, fast_timeout_us, 10);
 	else
-- 
2.7.4

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

✗ Fi.CI.BAT: warning for drm/i915: Don't allow overuse of __intel_wait_for_register_fw() (rev2)

[Patchwork]

== Series Details ==

Series: drm/i915: Don't allow overuse of __intel_wait_for_register_fw() (rev2)
URL   : https://patchwork.freedesktop.org/series/22774/
State : warning

== Summary ==

Series 22774v2 drm/i915: Don't allow overuse of __intel_wait_for_register_fw()
https://patchwork.freedesktop.org/api/1.0/series/22774/revisions/2/mbox/

Test core_auth:
        Subgroup basic-auth:
                pass       -> DMESG-WARN (fi-snb-2520m)
                pass       -> DMESG-WARN (fi-snb-2600)
Test drv_getparams_basic:
        Subgroup basic-subslice-total:
                pass       -> DMESG-WARN (fi-snb-2520m)
Test drv_module_reload:
        Subgroup basic-no-display:
                pass       -> DMESG-WARN (fi-snb-2520m)
                pass       -> DMESG-WARN (fi-snb-2600)
        Subgroup basic-reload:
                pass       -> DMESG-WARN (fi-snb-2520m)
                pass       -> DMESG-WARN (fi-snb-2600)
        Subgroup basic-reload-final:
                pass       -> DMESG-WARN (fi-snb-2520m)
                pass       -> DMESG-WARN (fi-snb-2600)
Test gem_basic:
        Subgroup create-close:
                pass       -> DMESG-WARN (fi-snb-2520m)
Test gem_busy:
        Subgroup basic-hang-default:
                pass       -> DMESG-WARN (fi-snb-2520m)
                pass       -> DMESG-WARN (fi-snb-2600)
Test gem_close_race:
        Subgroup basic-threads:
                pass       -> DMESG-WARN (fi-snb-2520m)
                pass       -> DMESG-WARN (fi-snb-2600)
Test gem_cs_tlb:
        Subgroup basic-default:
                pass       -> DMESG-WARN (fi-snb-2520m)
Test gem_ctx_basic:
                pass       -> DMESG-WARN (fi-snb-2520m)
                pass       -> DMESG-WARN (fi-snb-2600)
Test gem_ctx_create:
        Subgroup basic-files:
                pass       -> DMESG-WARN (fi-snb-2520m) fdo#100247
                pass       -> DMESG-WARN (fi-snb-2600)
Test gem_ctx_param:
        Subgroup basic-default:
                pass       -> DMESG-WARN (fi-snb-2520m)
Test gem_ctx_switch:
        Subgroup basic-default:
                pass       -> DMESG-WARN (fi-snb-2520m)
                pass       -> DMESG-WARN (fi-snb-2600)
        Subgroup basic-default-heavy:
                pass       -> DMESG-WARN (fi-snb-2520m)
                pass       -> DMESG-WARN (fi-snb-2600)
Test gem_exec_basic:
        Subgroup basic-render:
                pass       -> DMESG-WARN (fi-snb-2520m)
        Subgroup gtt-bsd:
                pass       -> DMESG-WARN (fi-snb-2520m)
                pass       -> DMESG-WARN (fi-snb-2600)
        Subgroup gtt-default:
                pass       -> DMESG-WARN (fi-snb-2520m)
        Subgroup readonly-blt:
                pass       -> DMESG-WARN (fi-snb-2520m)
Test gem_exec_create:
        Subgroup basic:
                pass       -> DMESG-WARN (fi-snb-2520m)
                pass       -> DMESG-WARN (fi-snb-2600)
Test gem_exec_fence:
        Subgroup await-hang-default:
                pass       -> DMESG-WARN (fi-snb-2520m)
                pass       -> DMESG-WARN (fi-snb-2600)
        Subgroup basic-await-default:
                pass       -> DMESG-WARN (fi-snb-2520m)
                pass       -> DMESG-WARN (fi-snb-2600)
        Subgroup basic-busy-default:
                pass       -> DMESG-WARN (fi-snb-2520m)
        Subgroup nb-await-default:
                pass       -> DMESG-WARN (fi-snb-2520m)
                pass       -> DMESG-WARN (fi-snb-2600)
Test gem_exec_flush:
        Subgroup basic-batch-kernel-default-uc:
                pass       -> DMESG-WARN (fi-snb-2520m)
                fail       -> DMESG-WARN (fi-snb-2600) fdo#100007
        Subgroup basic-batch-kernel-default-wb:
                pass       -> DMESG-WARN (fi-snb-2520m)
                pass       -> DMESG-WARN (fi-snb-2600)
        Subgroup basic-uc-pro-default:
                pass       -> DMESG-WARN (fi-snb-2520m)
                pass       -> DMESG-WARN (fi-snb-2600)
        Subgroup basic-uc-prw-default:
                pass       -> DMESG-WARN (fi-snb-2520m)
                pass       -> DMESG-WARN (fi-snb-2600)
        Subgroup basic-uc-ro-default:
                pass       -> DMESG-WARN (fi-snb-2520m)
                pass       -> DMESG-WARN (fi-snb-2600)
        Subgroup basic-uc-rw-default:
                pass       -> DMESG-WARN (fi-snb-2520m)
                pass       -> DMESG-WARN (fi-snb-2600)
        Subgroup basic-uc-set-default:
                pass       -> DMESG-WARN (fi-snb-2520m)
                pass       -> DMESG-WARN (fi-snb-2600)
        Subgroup basic-wb-pro-default:
WARNING: Long output truncated
fi-bdw-gvtdvm failed to collect. IGT log at Patchwork_4460/fi-bdw-gvtdvm/igt.log

02f6d3e01f3e1b6b91efeaceabd24e36e0540b49 drm-tip: 2017y-04m-10d-11h-06m-04s UTC integration manifest
303fb68 drm/i915: Don't allow overuse of __intel_wait_for_register_fw()

== Logs ==

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

Re: [PATCH v2] drm/i915: Don't allow overuse of __intel_wait_for_register_fw()

[Chris Wilson]

On Mon, Apr 10, 2017 at 12:17:47PM +0000, Michal Wajdeczko wrote:
> This function should not be called with long timeouts in atomic context.
> Annotate it as might_sleep if timeout is longer than 10us.
> 
> v2: fix comment (Michal)
> 
> Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
> Suggested-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>

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

And pushed, thanks for the patch and for including the limits in the
doc.
-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

Re: [PATCH v2] drm/i915: Don't allow overuse of __intel_wait_for_register_fw()

[Saarinen, Jani]

HI, 
> -----Original Message-----
> From: Intel-gfx [mailto:intel-gfx-bounces@lists.freedesktop.org] On Behalf
> Of Chris Wilson
> Sent: Monday, April 10, 2017 5:26 PM
> To: Wajdeczko, Michal <Michal.Wajdeczko@intel.com>
> Cc: intel-gfx@lists.freedesktop.org
> Subject: Re: [Intel-gfx] [PATCH v2] drm/i915: Don't allow overuse of
> __intel_wait_for_register_fw()
> 
> On Mon, Apr 10, 2017 at 12:17:47PM +0000, Michal Wajdeczko wrote:
> > This function should not be called with long timeouts in atomic context.
> > Annotate it as might_sleep if timeout is longer than 10us.
> >
> > v2: fix comment (Michal)
> >
> > Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
> > Suggested-by: Chris Wilson <chris@chris-wilson.co.uk>
> > Cc: Chris Wilson <chris@chris-wilson.co.uk>
> 
> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
> 
> And pushed, thanks for the patch and for including the limits in the doc.
> -Chris
Was there reason why pushed as clearly seen issues on pw run: https://patchwork.freedesktop.org/series/22774/
Or is there known issues we are planning to fix now? Now same faults on on tip: https://intel-gfx-ci.01.org/CI/

Br,
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

Re: [PATCH v2] drm/i915: Don't allow overuse of __intel_wait_for_register_fw()

[Chris Wilson]

On Mon, Apr 10, 2017 at 03:26:06PM +0100, Chris Wilson wrote:
> On Mon, Apr 10, 2017 at 12:17:47PM +0000, Michal Wajdeczko wrote:
> > This function should not be called with long timeouts in atomic context.
> > Annotate it as might_sleep if timeout is longer than 10us.
> > 
> > v2: fix comment (Michal)
> > 
> > Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
> > Suggested-by: Chris Wilson <chris@chris-wilson.co.uk>
> > Cc: Chris Wilson <chris@chris-wilson.co.uk>
> 
> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
> 
> And pushed, thanks for the patch and for including the limits in the
> doc.

Drat, skimmed over the warning from patchwork. Just didn't believe we
had that latent bug...
-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

Re: [PATCH v2] drm/i915: Don't allow overuse of __intel_wait_for_register_fw()

[Chris Wilson]

On Mon, Apr 10, 2017 at 02:58:34PM +0000, Saarinen, Jani wrote:
> HI, 
> > -----Original Message-----
> > From: Intel-gfx [mailto:intel-gfx-bounces@lists.freedesktop.org] On Behalf
> > Of Chris Wilson
> > Sent: Monday, April 10, 2017 5:26 PM
> > To: Wajdeczko, Michal <Michal.Wajdeczko@intel.com>
> > Cc: intel-gfx@lists.freedesktop.org
> > Subject: Re: [Intel-gfx] [PATCH v2] drm/i915: Don't allow overuse of
> > __intel_wait_for_register_fw()
> > 
> > On Mon, Apr 10, 2017 at 12:17:47PM +0000, Michal Wajdeczko wrote:
> > > This function should not be called with long timeouts in atomic context.
> > > Annotate it as might_sleep if timeout is longer than 10us.
> > >
> > > v2: fix comment (Michal)
> > >
> > > Signed-off-by: Michal Wajdeczko <michal.wajdeczko@intel.com>
> > > Suggested-by: Chris Wilson <chris@chris-wilson.co.uk>
> > > Cc: Chris Wilson <chris@chris-wilson.co.uk>
> > 
> > Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
> > 
> > And pushed, thanks for the patch and for including the limits in the doc.
> > -Chris
> Was there reason why pushed as clearly seen issues on pw run: https://patchwork.freedesktop.org/series/22774/
> Or is there known issues we are planning to fix now? Now same faults on on tip: https://intel-gfx-ci.01.org/CI/

Because I was blind. The bug was latent and fix is sent.
-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