| Message ID | 1391007939-5741-2-git-send-email-mika.kuoppala@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Wed, Jan 29, 2014 at 05:05:36PM +0200, Mika Kuoppala wrote: > With full ppgtt support drm_i915_file_private gained knowledge > about the default context. Also reset stats are now inside > i915_hw_context so we can use proper abstraction. > > Suggested-by: Ben Widawsky <ben@bwidawsk.net> > Signed-off-by: Mika Kuoppala <mika.kuoppala@intel.com> > --- > drivers/gpu/drm/i915/i915_gem.c | 44 ++++++++++++++++++++------------------- > 1 file changed, 23 insertions(+), 21 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c > index 39770f7..e41d520 100644 > --- a/drivers/gpu/drm/i915/i915_gem.c > +++ b/drivers/gpu/drm/i915/i915_gem.c > @@ -2305,11 +2305,17 @@ static bool i915_request_guilty(struct drm_i915_gem_request *request, > return false; > } > > -static bool i915_context_is_banned(const struct i915_ctx_hang_stats *hs) > +static bool i915_context_is_banned(const struct i915_hw_context *ctx) > { > - const unsigned long elapsed = get_seconds() - hs->guilty_ts; > + struct drm_i915_private *dev_priv; > + unsigned long elapsed; > > - if (hs->banned) > + BUG_ON(!ctx->last_ring); If you're that worried, just pass in dev, or dev_priv. I think it's a valid BUG_ON btw, just a weird place to put it. I'd vote to shove the BUG_ON in i915_set_reset_status() - and drop this completely. > + > + dev_priv = ctx->last_ring->dev->dev_private; > + elapsed = get_seconds() - ctx->hang_stats.guilty_ts; > + > + if (ctx->hang_stats.banned) > return true; > > if (elapsed <= DRM_I915_CTX_BAN_PERIOD) { > @@ -2324,9 +2330,10 @@ static void i915_set_reset_status(struct intel_ring_buffer *ring, > struct drm_i915_gem_request *request, > u32 acthd) > { > - struct i915_ctx_hang_stats *hs = NULL; > bool inside, guilty; > unsigned long offset = 0; > + struct i915_hw_context *ctx = request->ctx; > + struct i915_ctx_hang_stats *hs; > > /* Innocent until proven guilty */ > guilty = false; > @@ -2341,28 +2348,23 @@ static void i915_set_reset_status(struct intel_ring_buffer *ring, > ring->name, > inside ? "inside" : "flushing", > offset, > - request->ctx ? request->ctx->id : 0, > + ctx ? ctx->id : 0, You may as well drop this ternary and move your WARN_ON(!ctx) up above. With or without my requests: Reviewed-by: Ben Widawsky <ben@bwidawsk.net> > acthd); > > guilty = true; > } > > - /* If contexts are disabled or this is the default context, use > - * file_priv->reset_state > - */ > - if (request->ctx && request->ctx->id != DEFAULT_CONTEXT_ID) > - hs = &request->ctx->hang_stats; > - else if (request->file_priv) > - hs = &request->file_priv->private_default_ctx->hang_stats; > - > - if (hs) { > - if (guilty) { > - hs->banned = i915_context_is_banned(hs); > - hs->batch_active++; > - hs->guilty_ts = get_seconds(); > - } else { > - hs->batch_pending++; > - } > + if (WARN_ON(!ctx)) > + return; > + > + hs = &ctx->hang_stats; > + > + if (guilty) { > + hs->banned = i915_context_is_banned(ctx); > + hs->batch_active++; > + hs->guilty_ts = get_seconds(); > + } else { > + hs->batch_pending++; > } > } >
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index 39770f7..e41d520 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -2305,11 +2305,17 @@ static bool i915_request_guilty(struct drm_i915_gem_request *request, return false; } -static bool i915_context_is_banned(const struct i915_ctx_hang_stats *hs) +static bool i915_context_is_banned(const struct i915_hw_context *ctx) { - const unsigned long elapsed = get_seconds() - hs->guilty_ts; + struct drm_i915_private *dev_priv; + unsigned long elapsed; - if (hs->banned) + BUG_ON(!ctx->last_ring); + + dev_priv = ctx->last_ring->dev->dev_private; + elapsed = get_seconds() - ctx->hang_stats.guilty_ts; + + if (ctx->hang_stats.banned) return true; if (elapsed <= DRM_I915_CTX_BAN_PERIOD) { @@ -2324,9 +2330,10 @@ static void i915_set_reset_status(struct intel_ring_buffer *ring, struct drm_i915_gem_request *request, u32 acthd) { - struct i915_ctx_hang_stats *hs = NULL; bool inside, guilty; unsigned long offset = 0; + struct i915_hw_context *ctx = request->ctx; + struct i915_ctx_hang_stats *hs; /* Innocent until proven guilty */ guilty = false; @@ -2341,28 +2348,23 @@ static void i915_set_reset_status(struct intel_ring_buffer *ring, ring->name, inside ? "inside" : "flushing", offset, - request->ctx ? request->ctx->id : 0, + ctx ? ctx->id : 0, acthd); guilty = true; } - /* If contexts are disabled or this is the default context, use - * file_priv->reset_state - */ - if (request->ctx && request->ctx->id != DEFAULT_CONTEXT_ID) - hs = &request->ctx->hang_stats; - else if (request->file_priv) - hs = &request->file_priv->private_default_ctx->hang_stats; - - if (hs) { - if (guilty) { - hs->banned = i915_context_is_banned(hs); - hs->batch_active++; - hs->guilty_ts = get_seconds(); - } else { - hs->batch_pending++; - } + if (WARN_ON(!ctx)) + return; + + hs = &ctx->hang_stats; + + if (guilty) { + hs->banned = i915_context_is_banned(ctx); + hs->batch_active++; + hs->guilty_ts = get_seconds(); + } else { + hs->batch_pending++; } }
With full ppgtt support drm_i915_file_private gained knowledge about the default context. Also reset stats are now inside i915_hw_context so we can use proper abstraction. Suggested-by: Ben Widawsky <ben@bwidawsk.net> Signed-off-by: Mika Kuoppala <mika.kuoppala@intel.com> --- drivers/gpu/drm/i915/i915_gem.c | 44 ++++++++++++++++++++------------------- 1 file changed, 23 insertions(+), 21 deletions(-)