Chris Wilson <chris@xxxxxxxxxxxxxxxxxx> 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 <mika.kuoppala@xxxxxxxxxxxxxxx>
> Signed-off-by: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx>
> Cc: Mika Kuoppala <mika.kuoppala@xxxxxxxxxxxxxxx>
> Cc: Joonas Lahtinen <joonas.lahtinen@xxxxxxxxxxxxxxx>
Reviewed-by: Mika Kuoppala <mika.kuoppala@xxxxxxxxx>
> ---
> 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
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
