Skip to content

Commit

Permalink
drm/i915: Be more careful to drop the GT wakeref
Browse files Browse the repository at this point in the history
Since we can retire requests from multiple paths, we cannot assume that
i915_gem_retire_requests() is the sole path on which we can transition
to gt.active_requests == 0. A consequence of this is that we would skip
the function if we had already retired all the requests and not
scheduled the idle worker.

This is fallout from changing the routine from considering active_engines
(for which it was the only consumer) to active_requests.

v2: Move kicking the idle working to i915_gem_request_retire() otherwise
we could postpone the idle callback everytime we called retire_requests
even though we did no work.
v3: We only need to move the idle work kicking!
v4: Drop the BUG_ON(!awake) as we may be called from the shrinker in the
middle of constructing a request before we have marked the device awake.
v5: Add a BUG_ON() for active_requests underflow upon retirement (Joonas)

Fixes: 28176ef ("drm/i915: Reserve space in the global seqno during request allocation")
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Link: http://patchwork.freedesktop.org/patch/msgid/20161115164620.17185-1-chris@chris-wilson.co.uk
Reviewed-by: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
  • Loading branch information
Chris Wilson committed Nov 18, 2016
1 parent 5b8c8ae commit 4302055
Showing 1 changed file with 9 additions and 8 deletions.
17 changes: 9 additions & 8 deletions drivers/gpu/drm/i915/i915_gem_request.c
Original file line number Diff line number Diff line change
Expand Up @@ -201,6 +201,7 @@ static void i915_gem_request_retire(struct drm_i915_gem_request *request)

lockdep_assert_held(&request->i915->drm.struct_mutex);
GEM_BUG_ON(!i915_gem_request_completed(request));
GEM_BUG_ON(!request->i915->gt.active_requests);

trace_i915_gem_request_retire(request);

Expand All @@ -218,7 +219,12 @@ static void i915_gem_request_retire(struct drm_i915_gem_request *request)
*/
list_del(&request->ring_link);
request->ring->last_retired_head = request->postfix;
request->i915->gt.active_requests--;
if (!--request->i915->gt.active_requests) {
GEM_BUG_ON(!request->i915->gt.awake);
mod_delayed_work(request->i915->wq,
&request->i915->gt.idle_work,
msecs_to_jiffies(100));
}

/* Walk through the active list, calling retire on each. This allows
* objects to track their GPU activity and mark themselves as idle
Expand Down Expand Up @@ -763,6 +769,8 @@ static void i915_gem_mark_busy(const struct intel_engine_cs *engine)
if (dev_priv->gt.awake)
return;

GEM_BUG_ON(!dev_priv->gt.active_requests);

intel_runtime_pm_get_noresume(dev_priv);
dev_priv->gt.awake = true;

Expand Down Expand Up @@ -1146,13 +1154,6 @@ void i915_gem_retire_requests(struct drm_i915_private *dev_priv)
if (!dev_priv->gt.active_requests)
return;

GEM_BUG_ON(!dev_priv->gt.awake);

for_each_engine(engine, dev_priv, id)
engine_retire_requests(engine);

if (!dev_priv->gt.active_requests)
mod_delayed_work(dev_priv->wq,
&dev_priv->gt.idle_work,
msecs_to_jiffies(100));
}

0 comments on commit 4302055

Please sign in to comment.