On 04/06/2015 19:23, John.C.Harrison@xxxxxxxxx wrote:
From: John Harrison <John.C.Harrison@xxxxxxxxx>
A previous patch (read-read optimisation) changed the early exit
condition in i915_gem_retire_requests_ring() from checking the request
list to checking the active list. This assumes that all requests have
objects associated with them which are placed on the active list. The
removal of the OLR means that non-batch buffer work is no longer
tagged onto the nearest batch buffer submission and thus there are
requests going through the system which do not have objects associated
with them. This can therefore lead to the situation where an
outstanding request never gets retired.
This change reverts the early exit condition to check for requests.
Given that the purpose of the function is to retire requests, this
does seem to make much more sense.
For: VIZ-5190
Signed-off-by: John Harrison <John.C.Harrison@xxxxxxxxx>
---
drivers/gpu/drm/i915/i915_gem.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 7117659..4c5a6cd 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -2859,7 +2859,7 @@ i915_gem_retire_requests_ring(struct intel_engine_cs *ring)
{
WARN_ON(i915_verify_lists(ring->dev));
- if (list_empty(&ring->active_list))
+ if (list_empty(&ring->request_list))
return;
/* Retire requests first as we use it above for the early return.
Note to whoever is integrating this patch: This patch can either be
applied or we could drop the request_list check entirely. This according
to Chris Wilson in the following conversation:
3:26:09 PM - ickle: we just kill the check
3:26:25 PM - ickle: the final function is just request_list + trace_request
3:26:37 PM - ickle: adding a test to save one isn't a great tradeoff
3:26:58 PM - siglesias has left the room (Quit: Ping timeout: 256 seconds).
3:27:28 PM - twnqx has left the room (Quit: Ping timeout: 272 seconds).
3:28:04 PM - tomas_elf: fine
3:28:20 PM - tomas_elf: anyway, good to know
3:29:32 PM - ickle: there's actually one more, it's also where the
execlists_retire should be
3:30:48 PM - tomas_elf: maybe you can just submit a patch (unless you've
already done so) that removes all of the references
3:31:09 PM - JohnHarrison Joryn
3:31:23 PM - ickle: there's like a backlog of 50 patches before we even
get to that point
3:31:29 PM - tomas_elf: ok, cool
3:31:51 PM - tomas_elf: at any rate, JohnHarrison's patch can be
accepted either with the request_list check or no check at all
3:33:14 PM - ickle: I thought vsyrjala sent the patch to completely kill
it along with a bug citation
3:34:50 PM - doome_ [~doome@82.150.48.146] entered the room.
3:35:25 PM - ickle: 0aedb1626566efd72b369c01992ee7413c82a0c5
3:35:39 PM - darkbasic_ [~quassel@xxxxxxxxxxxxxxxxxxxx] entered the room.
3:36:13 PM - darkbasic has left the room (Quit: Read error: Connection
reset by peer).
3:39:05 PM - tomas_elf: has it been merged?
3:39:43 PM - ickle: it is in drm-intel-fixes
3:40:02 PM - tomas_elf: ah, ok
As long as the active_list check is removed since it breaks things.
Reviewed-by: Tomas Elf <tomas.elf@xxxxxxxxx>
Thanks,
Tomas
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
