Chris Wilson <[email protected]> writes: > We would like to start doing some bookkeeping at the beginning, between > contexts and at the end of execlists submission. We already mark the > beginning and end using EXECLISTS_ACTIVE_USER, to provide an indication > when the HW is idle. This give us a pair of sequence points we can then > expand on for further bookkeeping. > > Signed-off-by: Chris Wilson <[email protected]> > Cc: Mika Kuoppala <[email protected]> > Cc: Francisco Jerez <[email protected]> > --- > drivers/gpu/drm/i915/intel_lrc.c | 42 > ++++++++++++++++++++++++--------- > drivers/gpu/drm/i915/intel_ringbuffer.h | 11 ++++++++- > 2 files changed, 41 insertions(+), 12 deletions(-) > > diff --git a/drivers/gpu/drm/i915/intel_lrc.c > b/drivers/gpu/drm/i915/intel_lrc.c > index 654634254b64..61fb1387feb3 100644 > --- a/drivers/gpu/drm/i915/intel_lrc.c > +++ b/drivers/gpu/drm/i915/intel_lrc.c > @@ -374,6 +374,19 @@ execlists_context_status_change(struct i915_request *rq, > unsigned long status) > status, rq); > } > > +static inline void > +execlists_user_begin(struct intel_engine_execlists *execlists, > + const struct execlist_port *port) > +{ > + execlists_set_active_once(execlists, EXECLISTS_ACTIVE_USER); > +} > + > +static inline void > +execlists_user_end(struct intel_engine_execlists *execlists) > +{ > + execlists_clear_active(execlists, EXECLISTS_ACTIVE_USER); > +} > + > static inline void > execlists_context_schedule_in(struct i915_request *rq) > { > @@ -710,7 +723,7 @@ static void execlists_dequeue(struct intel_engine_cs > *engine) > spin_unlock_irq(&engine->timeline->lock); > > if (submit) { > - execlists_set_active(execlists, EXECLISTS_ACTIVE_USER); > + execlists_user_begin(execlists, execlists->port); > execlists_submit_ports(engine); > } > > @@ -741,7 +754,7 @@ execlists_cancel_port_requests(struct > intel_engine_execlists * const execlists) > port++; > } > > - execlists_clear_active(execlists, EXECLISTS_ACTIVE_USER); > + execlists_user_end(execlists); > } > > static void clear_gtiir(struct intel_engine_cs *engine) > @@ -872,7 +885,7 @@ static void execlists_submission_tasklet(unsigned long > data) > { > struct intel_engine_cs * const engine = (struct intel_engine_cs *)data; > struct intel_engine_execlists * const execlists = &engine->execlists; > - struct execlist_port * const port = execlists->port; > + struct execlist_port *port = execlists->port; > struct drm_i915_private *dev_priv = engine->i915; > bool fw = false; > > @@ -1010,9 +1023,19 @@ static void execlists_submission_tasklet(unsigned long > data) > > GEM_BUG_ON(count == 0); > if (--count == 0) { > + /* > + * On the final event corresponding to the > + * submission of this context, we expect either > + * an element-switch event or a completion > + * event (and on completion, the active-idle > + * marker). No more preemptions, lite-restore > + * or otherwise
Missing punctuation in the last sentence.
> + */
> GEM_BUG_ON(status & GEN8_CTX_STATUS_PREEMPTED);
> GEM_BUG_ON(port_isset(&port[1]) &&
> !(status &
> GEN8_CTX_STATUS_ELEMENT_SWITCH));
> + GEM_BUG_ON(!port_isset(&port[1]) &&
> + !(status &
> GEN8_CTX_STATUS_ACTIVE_IDLE));
> GEM_BUG_ON(!i915_request_completed(rq));
> execlists_context_schedule_out(rq);
> trace_i915_request_out(rq);
> @@ -1021,17 +1044,14 @@ static void execlists_submission_tasklet(unsigned
> long data)
> GEM_TRACE("%s completed ctx=%d\n",
> engine->name, port->context_id);
>
> - execlists_port_complete(execlists, port);
> + port = execlists_port_complete(execlists, port);
> + if (port_isset(port))
> + execlists_user_begin(execlists, port);
> + else
> + execlists_user_end(execlists);
> } else {
> port_set(port, port_pack(rq, count));
> }
> -
> - /* After the final element, the hw should be idle */
> - GEM_BUG_ON(port_count(port) == 0 &&
> - !(status & GEN8_CTX_STATUS_ACTIVE_IDLE));
> - if (port_count(port) == 0)
> - execlists_clear_active(execlists,
> - EXECLISTS_ACTIVE_USER);
Do we want to update the EXECLISTS_ACTIVE_USER tracking done in
intel_guc_submission.c to use the same wrappers? Either way looks okay
to me:
Reviewed-by: Francisco Jerez <[email protected]>
> }
>
> if (head != execlists->csb_head) {
> diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h
> b/drivers/gpu/drm/i915/intel_ringbuffer.h
> index a02c7b3b9d55..2e20627e254b 100644
> --- a/drivers/gpu/drm/i915/intel_ringbuffer.h
> +++ b/drivers/gpu/drm/i915/intel_ringbuffer.h
> @@ -638,6 +638,13 @@ execlists_set_active(struct intel_engine_execlists
> *execlists,
> __set_bit(bit, (unsigned long *)&execlists->active);
> }
>
> +static inline bool
> +execlists_set_active_once(struct intel_engine_execlists *execlists,
> + unsigned int bit)
> +{
> + return !__test_and_set_bit(bit, (unsigned long *)&execlists->active);
> +}
> +
> static inline void
> execlists_clear_active(struct intel_engine_execlists *execlists,
> unsigned int bit)
> @@ -664,7 +671,7 @@ execlists_num_ports(const struct intel_engine_execlists *
> const execlists)
> return execlists->port_mask + 1;
> }
>
> -static inline void
> +static inline struct execlist_port *
> execlists_port_complete(struct intel_engine_execlists * const execlists,
> struct execlist_port * const port)
> {
> @@ -675,6 +682,8 @@ execlists_port_complete(struct intel_engine_execlists *
> const execlists,
>
> memmove(port, port + 1, m * sizeof(struct execlist_port));
> memset(port + m, 0, sizeof(struct execlist_port));
> +
> + return port;
> }
>
> static inline unsigned int
> --
> 2.16.3
signature.asc
Description: PGP signature
_______________________________________________ Intel-gfx mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/intel-gfx
