| From 5771c8d04f2eb59ec8c6a0900596327ec232bcc4 Mon Sep 17 00:00:00 2001 |
| From: Chris Wilson <chris@chris-wilson.co.uk> |
| Date: Mon, 26 Aug 2013 19:50:53 -0300 |
| Subject: drm/i915: Do not add an interrupt for a context switch |
| |
| We use the request to ensure we hold a reference to the context for the |
| duration that it remains in use by the ring. Each request only holds a |
| reference to the current context, hence we emit a request after |
| switching contexts with the final reference to the old context. However, |
| the extra interrupt caused by that request is not useful (no timing |
| critical function will wait for the context object), instead the overhead |
| of servicing the IRQ shows up in some (lightweight) benchmarks. In order |
| to keep the useful property of using the request to manage the context |
| lifetime, we want to add a dummy request that is associated with the |
| interrupt from the subsequent real request following the batch. |
| |
| The extra interrupt was added as a side-effect of using |
| i915_add_request() in |
| |
| commit 112522f6789581824903f6f72082b5b841a7f0f9 |
| Author: Chris Wilson <chris@chris-wilson.co.uk> |
| Date: Thu May 2 16:48:07 2013 +0300 |
| |
| drm/i915: put context upon switching |
| |
| v2: Daniel convinced me that the request here was solely for context |
| lifetime tracking and that we have the active ref to keep the object |
| alive whilst the MI_SET_CONTEXT. So the only concern then is which |
| context should get the blame for MI_SET_CONTEXT failing. The old scheme |
| added a request for the old context so that any hang upto and including |
| the switch away would mark the old context as guilty. Now any hang here |
| implicates the new context. However since we have already gone through a |
| complete flush with the last context in its last request, and all that |
| lies in no-man's-land is an invalidate flush and the MI_SET_CONTEXT, we |
| should be safe in not unduly placing blame on the new context. |
| |
| Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> |
| Cc: Ben Widawsky <ben@bwidawsk.net> |
| Cc: Paulo Zanoni <paulo.r.zanoni@intel.com> |
| Reviewed-by: Damien Lespiau <damien.lespiau@intel.com> |
| Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch> |
| (cherry picked from commit c0321e2c5acaaff7ed41d46d97cee71ad9238481) |
| Signed-off-by: Darren Hart <dvhart@linux.intel.com> |
| --- |
| drivers/gpu/drm/i915/i915_gem.c | 1 - |
| drivers/gpu/drm/i915/i915_gem_context.c | 12 +----------- |
| 2 files changed, 1 insertion(+), 12 deletions(-) |
| |
| --- a/drivers/gpu/drm/i915/i915_gem.c |
| +++ b/drivers/gpu/drm/i915/i915_gem.c |
| @@ -2045,7 +2045,6 @@ int __i915_add_request(struct intel_ring |
| if (request == NULL) |
| return -ENOMEM; |
| |
| - |
| /* Record the position of the start of the request so that |
| * should we detect the updated seqno part-way through the |
| * GPU processing the request, we never over-estimate the |
| --- a/drivers/gpu/drm/i915/i915_gem_context.c |
| +++ b/drivers/gpu/drm/i915/i915_gem_context.c |
| @@ -449,17 +449,7 @@ static int do_switch(struct i915_hw_cont |
| from->obj->dirty = 1; |
| BUG_ON(from->obj->ring != ring); |
| |
| - ret = i915_add_request(ring, NULL); |
| - if (ret) { |
| - /* Too late, we've already scheduled a context switch. |
| - * Try to undo the change so that the hw state is |
| - * consistent with out tracking. In case of emergency, |
| - * scream. |
| - */ |
| - WARN_ON(mi_set_context(ring, from, MI_RESTORE_INHIBIT)); |
| - return ret; |
| - } |
| - |
| + /* obj is kept alive until the next request by its active ref */ |
| i915_gem_object_unpin(from->obj); |
| i915_gem_context_unreference(from); |
| } |