On 11/04/2017 12:21, Chris Wilson wrote:
On Tue, Apr 11, 2017 at 11:13:36AM +0100, Chris Wilson wrote:
Allow the caller to use the fast_timeout_us to specify how long to wait
within the atomic section, rather than transparently switching to a
sleeping loop for larger values. This is required as some callsites may
need a long wait and are in an atomic section.
Signed-off-by: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
Cc: Michal Wajdeczko <michal.wajdeczko@xxxxxxxxx>
Cc: Joonas Lahtinen <joonas.lahtinen@xxxxxxxxxxxxxxx>
---
drivers/gpu/drm/i915/intel_uncore.c | 11 ++++++-----
1 file changed, 6 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c
index eb38392a2435..53c8457869f6 100644
--- a/drivers/gpu/drm/i915/intel_uncore.c
+++ b/drivers/gpu/drm/i915/intel_uncore.c
@@ -1601,7 +1601,7 @@ static int gen6_reset_engines(struct drm_i915_private *dev_priv,
*
* 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.
+ * must be not larger than 20,0000 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
@@ -1623,16 +1623,17 @@ int __intel_wait_for_register_fw(struct drm_i915_private *dev_priv,
int ret;
/* Catch any overuse of this function */
- might_sleep_if(fast_timeout_us > 10 || slow_timeout_ms);
+ might_sleep_if(slow_timeout_ms);
For whatever reason, replies are not in my mbox, so Tvrtko,Michal wrote:
I agree with Michal here. Kerneldoc even says "must not be larger than
20ms" so it would be better and completely fine in my opinion to:
if (GEM_WARN_ON(fast_timeout_us > 20000))
return -EINVAL;
Not EINVAL, ENODEV if we must. So GEM_BUG_ON(fast_timeout_us > 20000) as
documentation?
Yeah thats fine.
We need to keep the fast_timeout_us < X check for gcc.
Hm but it would break the bisectability of the series and break the
sandybridge pcode.
No it doesn't, nobody is passing in such a large *fast_timeout_us*. What
have I missed?
Sounds like I got confused, sorry.
So patch 4/5 looks broken since it changes the timeout from 500ms to
500us. I don't see how to fix that without splitting the _fw and atomic
concepts.
That's not being broken, thats part of the fix. 500ms timeout inside an
atomic section is insane. 500ms timeout outside of that is unwise and
deserves an error of its own. I am quite happy to add that error and
fight that battle later (as BAT is happy, EXT might not be, and users
never).
I see. Ok, then, with the GEM_BUG_ON:
Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx>
Regards,
Tvrtko
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
