Quoting Mika Kuoppala (2017-09-12 11:27:53)
> Chris Wilson <chris@xxxxxxxxxxxxxxxxxx> 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
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
