| Message ID | 1467815411-21756-1-git-send-email-david.s.gordon@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Wed, Jul 06, 2016 at 03:30:11PM +0100, Dave Gordon wrote: > Rather than using wait_for_atomic() when chacking for a response from > the GuC, we can get the effect of a hybrid spin/sleep wait by breaking > it into two stages. First, spin-wait for up to 10us to minimise latency > for "quick" commands; then, if that times out, sleep-wait for up 10ms > (the maximum allowed for a "slow" command). > > Being able to do this depends on the recent patch > 18f4b84 drm/i915: Use atomic waits for short non-atomic ones > and is similar to the hybrid approach in > 1758b90 drm/i915: Use a hybrid scheme for fast register waits > (although we can't use that as-is, because that interface doesn't quite > match what we need here). Returning the status from wait_for_register would help for one other callsite (gmbus iirc), not worth the conversion. > Signed-off-by: Dave Gordon <david.s.gordon@intel.com> > Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com> > Cc: Chris Wilson <chris@chris-wilson.co.uk> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk> This entire sequence is under the FORCEWAKE_ALL (from inspection of intel_uncore, I think you only need FORCEWAKE_BLITTER), you could use I915_WRITE_FW / I915_READ_FW here - you lose both the spinlock and auto-arming on each read, but you also lose the mmiotracing. (Though realistically we should use the general mmiotracer and fix it if it doesn't work for us.) -Chris
On 06/07/16 16:17, Patchwork wrote: > == Series Details == > > Series: drm/i915: avoid wait_for_atomic() in non-atomic host2guc_action() > URL : https://patchwork.freedesktop.org/series/9565/ > State : success > > == Summary == > > Series 9565v1 drm/i915: avoid wait_for_atomic() in non-atomic host2guc_action() > http://patchwork.freedesktop.org/api/1.0/series/9565/revisions/1/mbox > > Test drv_module_reload_basic: > skip -> PASS (ro-snb-i7-2620M) > > fi-kbl-qkkr total:235 pass:163 dwarn:29 dfail:0 fail:2 skip:41 > fi-skl-i5-6260u total:235 pass:207 dwarn:0 dfail:0 fail:2 skip:26 > fi-snb-i7-2600 total:235 pass:179 dwarn:0 dfail:0 fail:2 skip:54 > ro-bdw-i5-5250u total:229 pass:204 dwarn:1 dfail:1 fail:0 skip:23 > ro-bdw-i7-5557U total:229 pass:204 dwarn:2 dfail:1 fail:0 skip:22 > ro-bdw-i7-5600u total:229 pass:190 dwarn:0 dfail:1 fail:0 skip:38 > ro-bsw-n3050 total:229 pass:177 dwarn:0 dfail:1 fail:2 skip:49 > ro-byt-n2820 total:229 pass:180 dwarn:0 dfail:1 fail:3 skip:45 > ro-hsw-i3-4010u total:229 pass:197 dwarn:0 dfail:1 fail:0 skip:31 > ro-hsw-i7-4770r total:229 pass:197 dwarn:0 dfail:1 fail:0 skip:31 > ro-ilk-i7-620lm total:229 pass:157 dwarn:0 dfail:1 fail:1 skip:70 > ro-ilk1-i5-650 total:224 pass:157 dwarn:0 dfail:1 fail:1 skip:65 > ro-ivb-i7-3770 total:229 pass:188 dwarn:0 dfail:1 fail:0 skip:40 > ro-skl3-i5-6260u total:229 pass:208 dwarn:1 dfail:1 fail:0 skip:19 > ro-snb-i7-2620M total:229 pass:179 dwarn:0 dfail:1 fail:1 skip:48 > > Results at /archive/results/CI_IGT_test/RO_Patchwork_1438/ > > 39b147d drm-intel-nightly: 2016y-07m-06d-12h-15m-56s UTC integration manifest > 803f1c2 drm/i915: avoid wait_for_atomic() in non-atomic host2guc_action() Merged to dinq, thanks for the patch and review. Regards, Tvrtko
diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c b/drivers/gpu/drm/i915/i915_guc_submission.c index bfc8bf6..2112e02 100644 --- a/drivers/gpu/drm/i915/i915_guc_submission.c +++ b/drivers/gpu/drm/i915/i915_guc_submission.c @@ -97,8 +97,14 @@ static int host2guc_action(struct intel_guc *guc, u32 *data, u32 len) I915_WRITE(HOST2GUC_INTERRUPT, HOST2GUC_TRIGGER); - /* No HOST2GUC command should take longer than 10ms */ - ret = wait_for_atomic(host2guc_action_response(dev_priv, &status), 10); + /* + * Fast commands should complete in less than 10us, so sample quickly + * up to that length of time, then switch to a slower sleep-wait loop. + * No HOST2GUC command should ever take longer than 10ms. + */ + ret = wait_for_us(host2guc_action_response(dev_priv, &status), 10); + if (ret) + ret = wait_for(host2guc_action_response(dev_priv, &status), 10); if (status != GUC2HOST_STATUS_SUCCESS) { /* * Either the GuC explicitly returned an error (which
Rather than using wait_for_atomic() when chacking for a response from the GuC, we can get the effect of a hybrid spin/sleep wait by breaking it into two stages. First, spin-wait for up to 10us to minimise latency for "quick" commands; then, if that times out, sleep-wait for up 10ms (the maximum allowed for a "slow" command). Being able to do this depends on the recent patch 18f4b84 drm/i915: Use atomic waits for short non-atomic ones and is similar to the hybrid approach in 1758b90 drm/i915: Use a hybrid scheme for fast register waits (although we can't use that as-is, because that interface doesn't quite match what we need here). Signed-off-by: Dave Gordon <david.s.gordon@intel.com> Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com> Cc: Chris Wilson <chris@chris-wilson.co.uk> --- drivers/gpu/drm/i915/i915_guc_submission.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-)