All of lore.kernel.org
 help / color / mirror / Atom feed
From: Chris Wilson <chris@chris-wilson.co.uk>
To: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>
Cc: Intel-gfx@lists.freedesktop.org
Subject: Re: [RFC v2] drm/i915: Emit to ringbuffer directly
Date: Fri, 9 Sep 2016 17:04:50 +0100	[thread overview]
Message-ID: <20160909160450.GE20027@nuc-i3427.alporthouse.com> (raw)
In-Reply-To: <1473436348-21940-1-git-send-email-tvrtko.ursulin@linux.intel.com>

On Fri, Sep 09, 2016 at 04:52:28PM +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.
> 
> 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.
> 
> 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.

The increment can stay in begin (it's not intel_ring_begin() anymore
since it operates on the request!) as that will be smaller at no
usability cost.

Just check that rbuf == ring->vaddr + ring->tail at end.
-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

  reply	other threads:[~2016-09-09 16:05 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-09-08 15:12 [RFC] drm/i915: Emit to ringbuffer directly Tvrtko Ursulin
2016-09-08 15:54 ` ✗ Fi.CI.BAT: failure for " Patchwork
2016-09-08 16:40 ` [RFC] " Chris Wilson
2016-09-09  8:32   ` Tvrtko Ursulin
2016-09-09 13:20     ` Dave Gordon
2016-09-09 13:58       ` Tvrtko Ursulin
2016-09-09 15:52         ` [RFC v2] " Tvrtko Ursulin
2016-09-09 16:04           ` Chris Wilson [this message]
2016-09-12  9:44             ` [PATCH v3] " Tvrtko Ursulin
2016-09-12 15:04               ` Dave Gordon
2016-09-09 13:40     ` [RFC] " Chris Wilson
2016-09-09 13:45     ` Chris Wilson
2016-09-09 14:14       ` Tvrtko Ursulin
2016-09-09 16:26 ` ✗ Fi.CI.BAT: failure for drm/i915: Emit to ringbuffer directly (rev2) Patchwork
2016-09-12 10:19 ` ✓ Fi.CI.BAT: success for drm/i915: Emit to ringbuffer directly (rev3) Patchwork

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20160909160450.GE20027@nuc-i3427.alporthouse.com \
    --to=chris@chris-wilson.co.uk \
    --cc=Intel-gfx@lists.freedesktop.org \
    --cc=tvrtko.ursulin@linux.intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.