Chris Wilson <[email protected]> writes: > [ text/plain ] > Seal the request and mark it as pending execution before we submit it to > hardware. We assume that the actual submission cannot fail (that > guarantee is provided by preallocating space in the request for the > submission). As we may inspect this state without holding any locks > during hangcheck we should apply a barrier to ensure that we do > not see a more recent value in the HWS than we are tracking. > > Based on a patch by Mika Kuoppala. > > Suggested-by: Mika Kuoppala <[email protected]> > Signed-off-by: Chris Wilson <[email protected]> > Cc: Mika Kuoppala <[email protected]> > Cc: Joonas Lahtinen <[email protected]>
Reviewed-by: Mika Kuoppala <[email protected]> > --- > drivers/gpu/drm/i915/i915_gem.c | 39 ++++++++++++++++++++++----------------- > drivers/gpu/drm/i915/i915_irq.c | 3 ++- > 2 files changed, 24 insertions(+), 18 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c > index 7506e850913e..5a65a7663b88 100644 > --- a/drivers/gpu/drm/i915/i915_gem.c > +++ b/drivers/gpu/drm/i915/i915_gem.c > @@ -2575,6 +2575,28 @@ void __i915_add_request(struct drm_i915_gem_request > *request, > WARN(ret, "*_ring_flush_all_caches failed: %d!\n", ret); > } > > + trace_i915_gem_request_add(request); > + > + request->head = request_start; > + > + /* Whilst this request exists, batch_obj will be on the > + * active_list, and so will hold the active reference. Only when this > + * request is retired will the the batch_obj be moved onto the > + * inactive_list and lose its active reference. Hence we do not need > + * to explicitly hold another reference here. > + */ > + request->batch_obj = obj; > + > + /* Seal the request and mark it as pending execution. Note that > + * we may inspect this state, without holding any locks, during > + * hangcheck. Hence we apply the barrier to ensure that we do not > + * see a more recent value in the hws than we are tracking. > + */ > + request->emitted_jiffies = jiffies; > + request->previous_seqno = engine->last_submitted_seqno; > + smp_store_mb(engine->last_submitted_seqno, request->seqno); > + list_add_tail(&request->list, &engine->request_list); > + > /* 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 > @@ -2592,23 +2614,6 @@ void __i915_add_request(struct drm_i915_gem_request > *request, > /* Not allowed to fail! */ > WARN(ret, "emit|add_request failed: %d!\n", ret); > > - request->head = request_start; > - > - /* Whilst this request exists, batch_obj will be on the > - * active_list, and so will hold the active reference. Only when this > - * request is retired will the the batch_obj be moved onto the > - * inactive_list and lose its active reference. Hence we do not need > - * to explicitly hold another reference here. > - */ > - request->batch_obj = obj; > - > - request->emitted_jiffies = jiffies; > - request->previous_seqno = engine->last_submitted_seqno; > - engine->last_submitted_seqno = request->seqno; > - list_add_tail(&request->list, &engine->request_list); > - > - trace_i915_gem_request_add(request); > - > i915_queue_hangcheck(engine->dev); > > queue_delayed_work(dev_priv->wq, > diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c > index 5bec13844800..a5eb77d1f8cb 100644 > --- a/drivers/gpu/drm/i915/i915_irq.c > +++ b/drivers/gpu/drm/i915/i915_irq.c > @@ -2806,7 +2806,8 @@ static bool > ring_idle(struct intel_engine_cs *engine, u32 seqno) > { > return (list_empty(&engine->request_list) || > - i915_seqno_passed(seqno, engine->last_submitted_seqno)); > + i915_seqno_passed(seqno, > + READ_ONCE(engine->last_submitted_seqno))); > } > > static bool > -- > 2.8.0.rc3 _______________________________________________ Intel-gfx mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/intel-gfx
