Re: [Intel-gfx] [RFC 3/8] drm/i915: Track per-context engine busyness

Chris Wilson
2020-01-10T13:46:17Z

Quoting Tvrtko Ursulin (2020-01-10 13:30:44)
> #endif /* __INTEL_CONTEXT_TYPES__ */
> diff --git a/drivers/gpu/drm/i915/gt/intel_engine_cs.c b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
> index 825c94e7ca0b..9a346c060469 100644
> --- a/drivers/gpu/drm/i915/gt/intel_engine_cs.c
> +++ b/drivers/gpu/drm/i915/gt/intel_engine_cs.c
> @@ -1543,8 +1543,20 @@ int intel_enable_engine_stats(struct intel_engine_cs *engine)
>
> engine->stats.enabled_at = ktime_get();
>
> - /* XXX submission method oblivious? */
> - for (port = execlists->active; (rq = *port); port++)
> + /*
> + * Mark currently running context as active.
> + * XXX submission method oblivious?
> + */
> +
> + rq = NULL;
> + port = execlists->active;

execlists->active is never NULL (it always points at one of the arrays).
*execlists->active may be NULL.

> + if (port)
> + rq = *port;
> + if (rq)
> + __intel_context_stats_start(&rq->context->stats,
> + engine->stats.enabled_at);
> +
> + for (; (rq = *port); port++)
> engine->stats.active++;
>
> @@ -2250,6 +2277,7 @@ static void process_csb(struct intel_engine_cs *engine)
> rmb();
>
> do {
> + struct i915_request *rq;
> bool promote;
>
> if (++head == num_entries)
> @@ -2305,7 +2333,11 @@ static void process_csb(struct intel_engine_cs *engine)
>
> WRITE_ONCE(execlists->pending[0], NULL);
> } else {
> - GEM_BUG_ON(!*execlists->active);
> + rq = *execlists->active++;
> + GEM_BUG_ON(!rq);
> +
> + GEM_BUG_ON(execlists->active - execlists->inflight >
> + execlists_num_ports(execlists));
>
> /* port0 completed, advanced to port1 */
> trace_ports(execlists, "completed", execlists->active);
> @@ -2316,13 +2348,15 @@ static void process_csb(struct intel_engine_cs *engine)
> * coherent (visible from the CPU) before the
> * user interrupt and CSB is processed.
> */
> - GEM_BUG_ON(!i915_request_completed(*execlists->active) &&
> + GEM_BUG_ON(!i915_request_completed(rq) &&
> !reset_in_progress(execlists));
> - execlists_schedule_out(*execlists->active++);
>
> - GEM_BUG_ON(execlists->active - execlists->inflight >
> - execlists_num_ports(execlists));
> + execlists_schedule_out(rq);
> }
> +
> + rq = *execlists->active;
> + if (rq)
> + intel_context_stats_start(&rq->context->stats);
> } while (head != tail);

Actually, we can do this after processing the entire event buf.

if (execlists_active(execlists))
intel_context_stats_start((*execlists->active)->context->stats);

Once we apply the fix in
https://patchwork.freedesktop.org/patch/347934/?series=71809&rev=1

We can in fact do this as a part of set_timeslice() which means we have
all the time-related updates in the same spot.
-Chris