[Intel-gfx] [PATCH 8/8] drm/i915: Keep track of reserved execlist ports

[Chris Wilson]

Quoting Mika Kuoppala (2017-09-12 11:27:53)
> Chris Wilson <chris at chris-wilson.co.uk> writes:
> 
> > Quoting Mika Kuoppala (2017-09-12 09:36:18)
> >> +execlist_request_port(struct intel_engine_execlist * const el)
> >> +{
> >> +       GEM_DEBUG_BUG_ON(el->port_count == el->port_mask + 1);
> >> +
> >> +       el->port_count++;
> >> +
> >> +       GEM_DEBUG_BUG_ON(port_isset(execlist_port_tail(el)));
> >> +
> >> +       return execlist_port_tail(el);
> >> +}
> >> 
> >> diff --git a/drivers/gpu/drm/i915/i915_guc_submission.c b/drivers/gpu/drm/i915/i915_guc_submission.c
> >> index 0dfb03a0cee4..fdda3a1835ad 100644
> >> --- a/drivers/gpu/drm/i915/i915_guc_submission.c
> >> +++ b/drivers/gpu/drm/i915/i915_guc_submission.c
> >> @@ -662,12 +662,19 @@ static void port_assign(struct execlist_port *port,
> >>  static bool i915_guc_dequeue(struct intel_engine_cs *engine)
> >>  {
> >>         struct intel_engine_execlist * const el = &engine->execlist;
> >> -       struct execlist_port *port = execlist_port_head(el);
> >> -       const struct execlist_port * const last_port = execlist_port_tail(el);
> >> -       struct drm_i915_gem_request *last = port_request(port);
> >> +       struct execlist_port *port;
> >> +       struct drm_i915_gem_request *last;
> >>         struct rb_node *rb;
> >>         bool submit = false;
> >>  
> >> +       if (execlist_active_ports(el)) {
> >> +               port = execlist_port_tail(el);
> >> +               last = port_request(port);
> >> +       } else {
> >> +               port = NULL;
> >> +               last = NULL;
> >> +       }
> >> +
> >>         spin_lock_irq(&engine->timeline->lock);
> >>         rb = el->first;
> >>         GEM_BUG_ON(rb_first(&el->queue) != rb);
> >> @@ -675,9 +682,12 @@ static bool i915_guc_dequeue(struct intel_engine_cs *engine)
> >>                 struct i915_priolist *p = rb_entry(rb, typeof(*p), node);
> >>                 struct drm_i915_gem_request *rq, *rn;
> >>  
> >> +               if (!port)
> >> +                       port = execlist_request_port(el);
> >> +
> >
> > I can't see port becoming NULL inside the loop, so why can't we do it
> > during the init above? What did I miss?
> 
> Hmm this might have warranted a comment. I wanted to avoid requesting
> a port if there is nothing to dequeue, that is why it inside while (rb).
> We could allocate early and let the port linger if nothing to dequeue,
> but then the GEM_DEBUG's need to be relaxed more.

Ah, that's where only advancing at the end comes into play. Would also
help not to call it request_port... Or just not hiding the mechanics.
-Chris