Re: [PATCH 1/2] drm/i915: Introduce execlist_port_* accessors

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

Chris Wilson <chris@chris-wilson.co.uk> writes:

> Quoting Mika Kuoppala (2017-10-19 15:39:41)
>>         while (test_bit(ENGINE_IRQ_EXECLIST, &engine->irq_posted)) {
>> +               struct execlist_port *port;
>> +
>>                 /* The HWSP contains a (cacheable) mirror of the CSB */
>>                 const u32 *buf =
>>                         &engine->status_page.page_addr[I915_HWS_CSB_BUF0_INDEX];
>> @@ -855,7 +865,7 @@ static void intel_lrc_irq_handler(unsigned long data)
>>  
>>                         if (status & GEN8_CTX_STATUS_ACTIVE_IDLE &&
>>                             buf[2*head + 1] == PREEMPT_ID) {
>> -                               execlist_cancel_port_requests(execlists);
>> +                               execlists_cancel_port_requests(execlists);
>>  
>>                                 spin_lock_irq(&engine->timeline->lock);
>>                                 unwind_incomplete_requests(engine);
>> @@ -870,6 +880,8 @@ static void intel_lrc_irq_handler(unsigned long data)
>>                             execlists->preempt)
>>                                 continue;
>>  
>> +                       port = execlists_port_head(execlists);
>> +
>>                         /* Check the context/desc id for this event matches */
>>                         GEM_DEBUG_BUG_ON(buf[2 * head + 1] != port->context_id);
>>  
>> @@ -890,7 +902,7 @@ static void intel_lrc_irq_handler(unsigned long data)
>>                         }
>>  
>>                         /* After the final element, the hw should be idle */
>> -                       GEM_BUG_ON(port_count(port) == 0 &&
>> +                       GEM_BUG_ON(port_count(execlists_port_head(execlists)) == 0 &&
>>                                    !(status & GEN8_CTX_STATUS_ACTIVE_IDLE));
>>                 }
>
> Can you try reworking this such that port is kept local without having
> to go back to the struct on every loop? And then compare code
> generation.

New one is with the last_port introduce and set outside loop.

add/remove: 0/0 grow/shrink: 1/1 up/down: 10/-7 (3)
function                                     old     new   delta
intel_lrc_irq_handler                       1847    1857     +10
i915_guc_irq_handler                        1327    1320      -7

Without looking at the assembler, I would go with last_port
as a net win.

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