All of lore.kernel.org
 help / color / mirror / Atom feed
From: Daniel Vetter <daniel@ffwll.ch>
To: Chris Wilson <chris@chris-wilson.co.uk>,
	Daniel Vetter <daniel@ffwll.ch>,
	intel-gfx@lists.freedesktop.org,
	Ben Widawsky <benjamin.widawsky@intel.com>,
	Daniel Vetter <daniel.vetter@ffwll.ch>
Subject: Re: [PATCH 1/3] drm/i915: Only bind each object rather than for every execbuffer
Date: Wed, 8 Jan 2014 11:38:21 +0100	[thread overview]
Message-ID: <20140108103821.GP4770@phenom.ffwll.local> (raw)
In-Reply-To: <20140108103016.GH8991@nuc-i3427.alporthouse.com>

On Wed, Jan 08, 2014 at 10:30:16AM +0000, Chris Wilson wrote:
> On Tue, Jan 07, 2014 at 08:43:28AM +0100, Daniel Vetter wrote:
> > On Wed, Jan 01, 2014 at 02:00:54PM +0000, Chris Wilson wrote:
> > > One side-effect of the introduction of ppgtt was that we needed to
> > > rebind the object into the appropriate vm (and global gtt in some
> > > peculiar cases). For simplicity this was done twice for every object on
> > > every call to execbuffer. However, that adds a tremendous amount of CPU
> > > overhead (rewriting all the PTE for all objects into WC memory) per
> > > draw. The fix is to push all the decision about which vm to bind into
> > > and when down into the low-level bind routines through hints rather than
> > > inside the execbuffer routine.
> > > 
> > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=72906
> > > Tested-by: jianx.zhou@intel.com
> > > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > > Cc: Ben Widawsky <benjamin.widawsky@intel.com>
> > > Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> > 
> > Can you please split out the code cleanups into a separate patch? I like
> > them, but as is they're hiding the actual bugfix in the diff quite badly
> > imo.
> 
> Which part of this is cleanup?

Coalescing the two boolean parameters mappable and nonblocking into the
flags parameter in a bunch of bind/pin functions and the return value
change for bind_to_vm look like prep cleanup. And there's 1-2 whitespace
fixup hunks in there. Splitting that out as a prep patch will make the
actual fix stick out much more I think.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

      reply	other threads:[~2014-01-08 10:37 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-01-01 14:00 [PATCH 1/3] drm/i915: Only bind each object rather than for every execbuffer Chris Wilson
2014-01-01 14:00 ` [PATCH 2/3] drm/i915: Fix obj->map_and_fenceable for ppgtt Chris Wilson
2014-01-01 14:00 ` [PATCH 3/3] drm/i915: Remove redundant list_empty(eb->vmas) tests in execbuffer Chris Wilson
2014-01-01 21:51 ` [PATCH 1/3] drm/i915: Only bind each object rather than for every execbuffer Ben Widawsky
2014-01-02 10:37   ` Chris Wilson
2014-01-07  7:43 ` Daniel Vetter
2014-01-08 10:30   ` Chris Wilson
2014-01-08 10:38     ` Daniel Vetter [this message]

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=20140108103821.GP4770@phenom.ffwll.local \
    --to=daniel@ffwll.ch \
    --cc=benjamin.widawsky@intel.com \
    --cc=chris@chris-wilson.co.uk \
    --cc=daniel.vetter@ffwll.ch \
    --cc=intel-gfx@lists.freedesktop.org \
    /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.