[Intel-gfx] [PATCH v10 6/6] drm/i915: Cache last IRQ seqno to reduce IRQ overhead

[Maarten Lankhorst]

Op 16-06-16 om 14:54 schreef John.C.Harrison at Intel.com:
> From: John Harrison <John.C.Harrison at Intel.com>
>
> The notify function can be called many times without the seqno
> changing. Some are to prevent races due to the requirement of not
> enabling interrupts until requested. However, when interrupts are
> enabled the IRQ handler can be called multiple times without the
> ring's seqno value changing. E.g. two interrupts are generated by
> batch buffers completing in quick succession, the first call to the
> handler processes both completions but the handler still gets executed
> a second time. This patch reduces the overhead of these extra calls by
> caching the last processed seqno value and early exiting if it has not
> changed.
>
> v3: New patch for series.
>
> v5: Added comment about last_irq_seqno usage due to code review
> feedback (Tvrtko Ursulin).
>
> v6: Minor update to resolve a race condition with the wait_request
> optimisation.
>
> v7: Updated to newer nightly - lots of ring -> engine renaming plus an
> interface change to get_seqno().
>
> v10: Renamed the cached variable as it is no longer used at IRQ time.
> [Review comment from Tvrtko Ursulin]
>
> For: VIZ-5190
> Signed-off-by: John Harrison <John.C.Harrison at Intel.com>
> Cc: Maarten Lankhorst <maarten.lankhorst at linux.intel.com>
> Cc: Tvrtko Ursulin <tvrtko.ursulin at intel.com>

I think it would be useful to add in the commit message that this happens in most cases.

>From your earlier mail:

"Doing the cache check hits the early exit approx 98% of the time when 
running GLBenchmark. Although the vast majority of duplicate calls are 
from having to call the notify function from 
i915_gem_retire_requests_ring() and that being called at least once for 
every execbuf IOCTL (possibly multiple times). I have just made a couple 
of tweaks to further reduce the number of these calls and their impact, 
but there are still a lot of them."

If added,

Reviewed-by: Maarten Lankhorst <maarten.lankhorst at linux.intel.com>