* [PATCH] drm/i915: Only warn for might_sleep() before a slow wait_for_register
@ 2018-03-28 17:53 Chris Wilson
2018-03-28 19:01 ` Pandiyan, Dhinakaran
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Chris Wilson @ 2018-03-28 17:53 UTC (permalink / raw)
To: intel-gfx
As intel_wait_for_register_fw() may use, and if successful only use, a
busy-wait loop, the might_sleep() warning is a little over-zealous.
Restrict it to a might_sleep_if() a slow timeout is specified (and so
the caller authorises use of a usleep).
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
drivers/gpu/drm/i915/intel_uncore.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c
index f37ecfc69e49..44c4654443ba 100644
--- a/drivers/gpu/drm/i915/intel_uncore.c
+++ b/drivers/gpu/drm/i915/intel_uncore.c
@@ -1996,7 +1996,7 @@ int __intel_wait_for_register(struct drm_i915_private *dev_priv,
u32 reg_value;
int ret;
- might_sleep();
+ might_sleep_if(slow_timeout_ms);
spin_lock_irq(&dev_priv->uncore.lock);
intel_uncore_forcewake_get__locked(dev_priv, fw);
@@ -2008,7 +2008,7 @@ int __intel_wait_for_register(struct drm_i915_private *dev_priv,
intel_uncore_forcewake_put__locked(dev_priv, fw);
spin_unlock_irq(&dev_priv->uncore.lock);
- if (ret)
+ if (ret && slow_timeout_ms)
ret = __wait_for(reg_value = I915_READ_NOTRACE(reg),
(reg_value & mask) == value,
slow_timeout_ms * 1000, 10, 1000);
--
2.16.3
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH] drm/i915: Only warn for might_sleep() before a slow wait_for_register
2018-03-28 17:53 [PATCH] drm/i915: Only warn for might_sleep() before a slow wait_for_register Chris Wilson
@ 2018-03-28 19:01 ` Pandiyan, Dhinakaran
2018-03-28 19:13 ` Chris Wilson
2018-03-28 19:10 ` ✓ Fi.CI.BAT: success for " Patchwork
2018-03-29 1:45 ` ✗ Fi.CI.IGT: failure " Patchwork
2 siblings, 1 reply; 6+ messages in thread
From: Pandiyan, Dhinakaran @ 2018-03-28 19:01 UTC (permalink / raw)
To: chris; +Cc: intel-gfx
On Wed, 2018-03-28 at 18:53 +0100, Chris Wilson wrote:
> As intel_wait_for_register_fw() may use, and if successful only use, a
> busy-wait loop, the might_sleep() warning is a little over-zealous.
> Restrict it to a might_sleep_if() a slow timeout is specified (and so
> the caller authorises use of a usleep).
>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> ---
> drivers/gpu/drm/i915/intel_uncore.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c
> index f37ecfc69e49..44c4654443ba 100644
> --- a/drivers/gpu/drm/i915/intel_uncore.c
> +++ b/drivers/gpu/drm/i915/intel_uncore.c
> @@ -1996,7 +1996,7 @@ int __intel_wait_for_register(struct drm_i915_private *dev_priv,
> u32 reg_value;
> int ret;
>
> - might_sleep();
> + might_sleep_if(slow_timeout_ms);
__wait_for() already has a might_sleep(), why is this needed?
>
> spin_lock_irq(&dev_priv->uncore.lock);
> intel_uncore_forcewake_get__locked(dev_priv, fw);
> @@ -2008,7 +2008,7 @@ int __intel_wait_for_register(struct drm_i915_private *dev_priv,
> intel_uncore_forcewake_put__locked(dev_priv, fw);
> spin_unlock_irq(&dev_priv->uncore.lock);
>
> - if (ret)
> + if (ret && slow_timeout_ms)
> ret = __wait_for(reg_value = I915_READ_NOTRACE(reg),
> (reg_value & mask) == value,
> slow_timeout_ms * 1000, 10, 1000);
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] drm/i915: Only warn for might_sleep() before a slow wait_for_register
2018-03-28 19:01 ` Pandiyan, Dhinakaran
@ 2018-03-28 19:13 ` Chris Wilson
2018-03-28 19:35 ` Pandiyan, Dhinakaran
0 siblings, 1 reply; 6+ messages in thread
From: Chris Wilson @ 2018-03-28 19:13 UTC (permalink / raw)
To: Pandiyan, Dhinakaran; +Cc: intel-gfx
Quoting Pandiyan, Dhinakaran (2018-03-28 20:01:40)
>
> On Wed, 2018-03-28 at 18:53 +0100, Chris Wilson wrote:
> > As intel_wait_for_register_fw() may use, and if successful only use, a
> > busy-wait loop, the might_sleep() warning is a little over-zealous.
> > Restrict it to a might_sleep_if() a slow timeout is specified (and so
> > the caller authorises use of a usleep).
> >
> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > ---
> > drivers/gpu/drm/i915/intel_uncore.c | 4 ++--
> > 1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c
> > index f37ecfc69e49..44c4654443ba 100644
> > --- a/drivers/gpu/drm/i915/intel_uncore.c
> > +++ b/drivers/gpu/drm/i915/intel_uncore.c
> > @@ -1996,7 +1996,7 @@ int __intel_wait_for_register(struct drm_i915_private *dev_priv,
> > u32 reg_value;
> > int ret;
> >
> > - might_sleep();
> > + might_sleep_if(slow_timeout_ms);
>
> __wait_for() already has a might_sleep(), why is this needed?
To document that this routine is a sleeper, irrespective of the
implementation. Sometimes it is implicit on the implementation and so
should only be at the low level, sometimes we want to call out the
requirements explicitly and clearly. We have "wait" in the name so
shouting out that this may indeed sleep rather than busyspin seems to
be in order.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] drm/i915: Only warn for might_sleep() before a slow wait_for_register
2018-03-28 19:13 ` Chris Wilson
@ 2018-03-28 19:35 ` Pandiyan, Dhinakaran
0 siblings, 0 replies; 6+ messages in thread
From: Pandiyan, Dhinakaran @ 2018-03-28 19:35 UTC (permalink / raw)
To: chris; +Cc: intel-gfx
On Wed, 2018-03-28 at 20:13 +0100, Chris Wilson wrote:
> Quoting Pandiyan, Dhinakaran (2018-03-28 20:01:40)
> >
> > On Wed, 2018-03-28 at 18:53 +0100, Chris Wilson wrote:
> > > As intel_wait_for_register_fw() may use, and if successful only use, a
> > > busy-wait loop, the might_sleep() warning is a little over-zealous.
> > > Restrict it to a might_sleep_if() a slow timeout is specified (and so
> > > the caller authorises use of a usleep).
> > >
> > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > > ---
> > > drivers/gpu/drm/i915/intel_uncore.c | 4 ++--
> > > 1 file changed, 2 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c
> > > index f37ecfc69e49..44c4654443ba 100644
> > > --- a/drivers/gpu/drm/i915/intel_uncore.c
> > > +++ b/drivers/gpu/drm/i915/intel_uncore.c
> > > @@ -1996,7 +1996,7 @@ int __intel_wait_for_register(struct drm_i915_private *dev_priv,
> > > u32 reg_value;
> > > int ret;
> > >
> > > - might_sleep();
> > > + might_sleep_if(slow_timeout_ms);
> >
> > __wait_for() already has a might_sleep(), why is this needed?
>
> To document that this routine is a sleeper, irrespective of the
> implementation. Sometimes it is implicit on the implementation and so
> should only be at the low level, sometimes we want to call out the
> requirements explicitly and clearly. We have "wait" in the name so
> shouting out that this may indeed sleep rather than busyspin seems to
> be in order.
> -Chris
Fair enough.
There seems to be a side effect from the second hunk that your commit
message does not explicitly state.
> - if (ret)
> + if (ret && slow_timeout_ms)
This results in a different return value if condition == true and
slow_timeout_ms == 0 after fast_timeout_us was exceeded.
Previously, __intel_wait_for_register would have passed along the 0 from
__wait_for(), now it returns -ETIMEDOUT. Which means this change should
have been a separate patch.
As the patch itself is sensible,
Reviewed-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> _______________________________________________
> 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] 6+ messages in thread
* ✓ Fi.CI.BAT: success for drm/i915: Only warn for might_sleep() before a slow wait_for_register
2018-03-28 17:53 [PATCH] drm/i915: Only warn for might_sleep() before a slow wait_for_register Chris Wilson
2018-03-28 19:01 ` Pandiyan, Dhinakaran
@ 2018-03-28 19:10 ` Patchwork
2018-03-29 1:45 ` ✗ Fi.CI.IGT: failure " Patchwork
2 siblings, 0 replies; 6+ messages in thread
From: Patchwork @ 2018-03-28 19:10 UTC (permalink / raw)
To: Chris Wilson; +Cc: intel-gfx
== Series Details ==
Series: drm/i915: Only warn for might_sleep() before a slow wait_for_register
URL : https://patchwork.freedesktop.org/series/40823/
State : success
== Summary ==
Series 40823v1 drm/i915: Only warn for might_sleep() before a slow wait_for_register
https://patchwork.freedesktop.org/api/1.0/series/40823/revisions/1/mbox/
---- Known issues:
Test gem_exec_suspend:
Subgroup basic-s4-devices:
dmesg-warn -> PASS (fi-kbl-7500u) fdo#105128
Test prime_vgem:
Subgroup basic-fence-flip:
pass -> FAIL (fi-ilk-650) fdo#104008
fdo#105128 https://bugs.freedesktop.org/show_bug.cgi?id=105128
fdo#104008 https://bugs.freedesktop.org/show_bug.cgi?id=104008
fi-bdw-5557u total:285 pass:264 dwarn:0 dfail:0 fail:0 skip:21 time:427s
fi-bdw-gvtdvm total:285 pass:261 dwarn:0 dfail:0 fail:0 skip:24 time:441s
fi-blb-e6850 total:285 pass:220 dwarn:1 dfail:0 fail:0 skip:64 time:378s
fi-bsw-n3050 total:285 pass:239 dwarn:0 dfail:0 fail:0 skip:46 time:546s
fi-bwr-2160 total:285 pass:180 dwarn:0 dfail:0 fail:0 skip:105 time:294s
fi-bxt-dsi total:285 pass:255 dwarn:0 dfail:0 fail:0 skip:30 time:513s
fi-bxt-j4205 total:285 pass:256 dwarn:0 dfail:0 fail:0 skip:29 time:514s
fi-byt-j1900 total:285 pass:250 dwarn:0 dfail:0 fail:0 skip:35 time:519s
fi-byt-n2820 total:285 pass:246 dwarn:0 dfail:0 fail:0 skip:39 time:507s
fi-cfl-8700k total:285 pass:257 dwarn:0 dfail:0 fail:0 skip:28 time:411s
fi-cfl-s3 total:285 pass:259 dwarn:0 dfail:0 fail:0 skip:26 time:562s
fi-cfl-u total:285 pass:259 dwarn:0 dfail:0 fail:0 skip:26 time:509s
fi-cnl-y3 total:285 pass:259 dwarn:0 dfail:0 fail:0 skip:26 time:588s
fi-elk-e7500 total:285 pass:225 dwarn:1 dfail:0 fail:0 skip:59 time:426s
fi-gdg-551 total:285 pass:176 dwarn:0 dfail:0 fail:1 skip:108 time:317s
fi-glk-1 total:285 pass:257 dwarn:0 dfail:0 fail:0 skip:28 time:536s
fi-hsw-4770 total:285 pass:258 dwarn:0 dfail:0 fail:0 skip:27 time:404s
fi-ilk-650 total:285 pass:224 dwarn:0 dfail:0 fail:1 skip:60 time:425s
fi-ivb-3520m total:285 pass:256 dwarn:0 dfail:0 fail:0 skip:29 time:467s
fi-ivb-3770 total:285 pass:252 dwarn:0 dfail:0 fail:0 skip:33 time:432s
fi-kbl-7500u total:285 pass:260 dwarn:1 dfail:0 fail:0 skip:24 time:472s
fi-kbl-7567u total:285 pass:265 dwarn:0 dfail:0 fail:0 skip:20 time:459s
fi-kbl-r total:285 pass:258 dwarn:0 dfail:0 fail:0 skip:27 time:507s
fi-pnv-d510 total:285 pass:219 dwarn:1 dfail:0 fail:0 skip:65 time:661s
fi-skl-6260u total:285 pass:265 dwarn:0 dfail:0 fail:0 skip:20 time:438s
fi-skl-6600u total:285 pass:258 dwarn:0 dfail:0 fail:0 skip:27 time:540s
fi-skl-6700k2 total:285 pass:261 dwarn:0 dfail:0 fail:0 skip:24 time:507s
fi-skl-6770hq total:285 pass:265 dwarn:0 dfail:0 fail:0 skip:20 time:513s
fi-skl-guc total:285 pass:257 dwarn:0 dfail:0 fail:0 skip:28 time:430s
fi-skl-gvtdvm total:285 pass:262 dwarn:0 dfail:0 fail:0 skip:23 time:446s
fi-snb-2520m total:3 pass:2 dwarn:0 dfail:0 fail:0 skip:0
fi-snb-2600 total:285 pass:245 dwarn:0 dfail:0 fail:0 skip:40 time:398s
Blacklisted hosts:
fi-cnl-psr total:285 pass:256 dwarn:3 dfail:0 fail:0 skip:26 time:538s
03f62197cf791e1a517f8122e0e7257b33e0c652 drm-tip: 2018y-03m-28d-18h-18m-48s UTC integration manifest
181d8238ba72 drm/i915: Only warn for might_sleep() before a slow wait_for_register
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_8519/issues.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 6+ messages in thread* ✗ Fi.CI.IGT: failure for drm/i915: Only warn for might_sleep() before a slow wait_for_register
2018-03-28 17:53 [PATCH] drm/i915: Only warn for might_sleep() before a slow wait_for_register Chris Wilson
2018-03-28 19:01 ` Pandiyan, Dhinakaran
2018-03-28 19:10 ` ✓ Fi.CI.BAT: success for " Patchwork
@ 2018-03-29 1:45 ` Patchwork
2 siblings, 0 replies; 6+ messages in thread
From: Patchwork @ 2018-03-29 1:45 UTC (permalink / raw)
To: Chris Wilson; +Cc: intel-gfx
== Series Details ==
Series: drm/i915: Only warn for might_sleep() before a slow wait_for_register
URL : https://patchwork.freedesktop.org/series/40823/
State : failure
== Summary ==
---- Possible new issues:
Test kms_cursor_crc:
Subgroup cursor-128x128-dpms:
fail -> PASS (shard-apl)
Test kms_frontbuffer_tracking:
Subgroup fbc-1p-primscrn-cur-indfb-move:
pass -> FAIL (shard-apl)
---- Known issues:
Test kms_cursor_legacy:
Subgroup flip-vs-cursor-atomic:
pass -> FAIL (shard-hsw) fdo#102670
Test kms_flip:
Subgroup 2x-dpms-vs-vblank-race:
fail -> PASS (shard-hsw) fdo#103060
Subgroup 2x-plain-flip-fb-recreate:
pass -> FAIL (shard-hsw) fdo#100368 +2
fdo#102670 https://bugs.freedesktop.org/show_bug.cgi?id=102670
fdo#103060 https://bugs.freedesktop.org/show_bug.cgi?id=103060
fdo#100368 https://bugs.freedesktop.org/show_bug.cgi?id=100368
shard-apl total:3495 pass:1830 dwarn:1 dfail:0 fail:8 skip:1655 time:12772s
shard-hsw total:3495 pass:1778 dwarn:1 dfail:0 fail:6 skip:1709 time:11390s
shard-snb total:3495 pass:1375 dwarn:1 dfail:0 fail:2 skip:2117 time:6973s
Blacklisted hosts:
shard-kbl total:3495 pass:1887 dwarn:72 dfail:0 fail:7 skip:1529 time:9249s
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_8519/shards.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2018-03-29 1:45 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-03-28 17:53 [PATCH] drm/i915: Only warn for might_sleep() before a slow wait_for_register Chris Wilson
2018-03-28 19:01 ` Pandiyan, Dhinakaran
2018-03-28 19:13 ` Chris Wilson
2018-03-28 19:35 ` Pandiyan, Dhinakaran
2018-03-28 19:10 ` ✓ Fi.CI.BAT: success for " Patchwork
2018-03-29 1:45 ` ✗ Fi.CI.IGT: failure " Patchwork
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.