Re: [PATCH v10] drm/i915: Emit to ringbuffer directly

[Chris Wilson <chris@chris-wilson.co.uk>]

On Tue, Feb 14, 2017 at 11:32:42AM +0000, Tvrtko Ursulin wrote:
> From: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> 
> This removes the usage of intel_ring_emit in favour of
> directly writing to the ring buffer.
> 
> intel_ring_emit was preventing the compiler for optimising
> fetch and increment of the current ring buffer pointer and
> therefore generating very verbose code for every write.
> 
> It had no useful purpose since all ringbuffer operations
> are started and ended with intel_ring_begin and
> intel_ring_advance respectively, with no bail out in the
> middle possible, so it is fine to increment the tail in
> intel_ring_begin and let the code manage the pointer
> itself.
> 
> Useless instruction removal amounts to approximately
> two and half kilobytes of saved text on my build.
> 
> Not sure if this has any measurable performance
> implications but executing a ton of useless instructions
> on fast paths cannot be good.
> 
> Patch is not fully polished, but it compiles and runs
> on Gen9 at least.

Probably safe to drop this comment now. :)
 
> v2:
>  * Change return from intel_ring_begin to error pointer by
>    popular demand.
>  * Move tail increment to intel_ring_advance to enable some
>    error checking.
> 
> v3:
>  * Move tail advance back into intel_ring_begin.
>  * Rebase and tidy.
> 
> v4:
>  * Complete rebase after a few months since v3.
> 
> v5:
>  * Remove unecessary cast and fix !debug compile. (Chris Wilson)
> 
> v6:
>  * Make intel_ring_offset take request as well.
>  * Fix recording of request postfix plus a sprinkle of asserts.
>    (Chris Wilson)
> 
> v7:
>  * Use intel_ring_offset to get the postfix. (Chris Wilson)
>  * Convert GVT code as well.
> 
> v8:
>  * Rename *out++ to *cs++.
> 
> v9:
>  * Fix GVT out to cs conversion in GVT.
> 
> v10:
>  * Rebase for new intel_ring_begin in selftests.
> 
> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> Cc: Zhi Wang <zhi.a.wang@intel.com>
> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk> (v9)

Looking good to me.

Joonas care to ack?
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx