Re: [RFC] drm/i915: Emit to ringbuffer directly

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

On Thu, Sep 08, 2016 at 04:12:55PM +0100, 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.

I have the same patch! But I called it out, for historical reasons.

Oh, except mine uses out[0]...out[N] because gcc prefers that over
*out++ = ...

> 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
> 2384 bytes 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.

It does show up in perf.
 
> Patch is not fully polished, but it compiles and runs
> on Gen9 at least.
> 
> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_gem_context.c    |  62 ++--
>  drivers/gpu/drm/i915/i915_gem_execbuffer.c |  27 +-
>  drivers/gpu/drm/i915/i915_gem_gtt.c        |  57 ++--
>  drivers/gpu/drm/i915/intel_display.c       | 113 ++++---
>  drivers/gpu/drm/i915/intel_lrc.c           | 223 +++++++-------
>  drivers/gpu/drm/i915/intel_mocs.c          |  43 +--
>  drivers/gpu/drm/i915/intel_overlay.c       |  69 ++---
>  drivers/gpu/drm/i915/intel_ringbuffer.c    | 480 +++++++++++++++--------------
>  drivers/gpu/drm/i915/intel_ringbuffer.h    |  19 +-
>  9 files changed, 555 insertions(+), 538 deletions(-)

Hmm, mine is bigger.

 drivers/gpu/drm/i915/i915_gem_context.c    |  85 ++--
 drivers/gpu/drm/i915/i915_gem_execbuffer.c |  37 +-
 drivers/gpu/drm/i915/i915_gem_gtt.c        |  62 +--
 drivers/gpu/drm/i915/i915_gem_request.c    | 135 ++++-
 drivers/gpu/drm/i915/i915_gem_request.h    |   2 +
 drivers/gpu/drm/i915/intel_display.c       | 133 +++--
 drivers/gpu/drm/i915/intel_lrc.c           | 188 ++++---
 drivers/gpu/drm/i915/intel_lrc.h           |   2 -
 drivers/gpu/drm/i915/intel_mocs.c          |  50 +-
 drivers/gpu/drm/i915/intel_overlay.c       |  77 ++-
 drivers/gpu/drm/i915/intel_ringbuffer.c    | 762 ++++++++++++-----------------
 drivers/gpu/drm/i915/intel_ringbuffer.h    |  36 +-
 12 files changed, 721 insertions(+), 848 deletions(-)

(this includes moving the intel_ring_begin to i915_gem_request)

plus an ealier

 drivers/gpu/drm/i915/i915_gem_request.c |  26 ++---
 drivers/gpu/drm/i915/intel_lrc.c        | 121 ++++++++---------------
 drivers/gpu/drm/i915/intel_ringbuffer.c | 168 +++++++++++---------------------
 drivers/gpu/drm/i915/intel_ringbuffer.h |  10 +-
 4 files changed, 112 insertions(+), 213 deletions(-)

since I wanted parts of it for emitting timelines.
-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