Re: [PATCH 8/8] drm/i915: Keep track of reserved execlist ports

[Mika Kuoppala <mika.kuoppala@linux.intel.com>]

Chris Wilson <chris@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.

-Mika

> -Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx