All of lore.kernel.org
 help / color / mirror / Atom feed
From: Daniel Vetter <daniel@ffwll.ch>
To: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>,
	Intel Graphics Development <intel-gfx@lists.freedesktop.org>
Subject: Re: [PATCH 15/16] drm/i915: fixup in-line clflushing on bit17 swizzled bos
Date: Mon, 26 Mar 2012 11:26:33 +0200	[thread overview]
Message-ID: <20120326092633.GG4014@phenom.ffwll.local> (raw)
In-Reply-To: <1332753528_97351@CP5-2952>

On Mon, Mar 26, 2012 at 10:18:39AM +0100, Chris Wilson wrote:
> On Sun, 25 Mar 2012 19:47:42 +0200, Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
> > The issue is that with inline clflushing the clflushing isn't properly
> > swizzled. Fix this by
> > - always clflushing entire 128 byte chunks and
> > - unconditionally flush before writes when swizzling a given page.
> >   We could be clever and check whether we pwrite a partial 128 byte
> >   chunk instead of a partial cacheline, but I've figured that's not
> >   worth it.
> 
> There's some black magic here that I haven't fully grasped. We only ever
> swizzle the gpu address (by whole cachelines), so why do we need to
> invalidate a pair of cachelines for a single cacheline write?

Well, we do swizzle when doing the actual copy_to|from_user, so strictly
speaking we should also swizzle the clflushing in this case. No bit17
swizzling pwrite/pread is pretty much only around for backwards-compat
with dead-old userspace, so I've figure I'll just unconditionally align
the clflush range with even cachelines when bit17 swizzling is effective
on the current page. Instead of adding a complex and rather untested
swizzled clflush helper.


> Also we have a lot of assumptions that the cacheline is 64 bytes. Have
> we tested on gen2 where the GPU cacheline is 32 bytes?

We're lucking out in that regard because gen2 doesn't do swizzling. At
least my i855gm here, where I could run the corresponding i-g-t tests. And
there's a comment in the code that i865g is unswizzled, too.

So as long as people create dram controller where 64 bytes is the most
effective transaction size, we should be fine. It'll be a fun day though
when that changes. Otoh with gen5+ we don't have any bit17 swizzling
nonsense anymore because the gpu is much more integrated with the cpu. I
hope that trend continues and will prevent any bit17 madness in the
future.
-Daniel
-- 
Daniel Vetter
Mail: daniel@ffwll.ch

  reply	other threads:[~2012-03-26  9:25 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-03-25 17:47 [PATCH 01/16] drm/i915: merge shmem_pwrite slow&fast-path Daniel Vetter
2012-03-25 17:47 ` [PATCH 02/16] drm/i915: merge shmem_pread slow&fast-path Daniel Vetter
2012-03-25 17:47 ` [PATCH 03/16] drm: add helper to clflush a virtual address range Daniel Vetter
2012-03-25 17:47 ` [PATCH 04/16] drm/i915: move clflushing into shmem_pread Daniel Vetter
2012-03-25 17:47 ` [PATCH 05/16] drm/i915: kill ranged cpu read domain support Daniel Vetter
2012-03-25 17:47 ` [PATCH 06/16] drm/i915: don't use gtt_pwrite on LLC cached objects Daniel Vetter
2012-03-25 17:47 ` [PATCH 07/16] drm/i915: don't call shmem_read_mapping unnecessarily Daniel Vetter
2012-03-26  9:10   ` Daniel Vetter
2012-03-25 17:47 ` [PATCH 08/16] drm/i915: drop gtt slowpath Daniel Vetter
2012-03-25 17:47 ` [PATCH 09/16] drm/i915: don't clobber userspace memory before commiting to the pread Daniel Vetter
2012-03-25 17:47 ` [PATCH 10/16] drm/i915: implement inline clflush for pwrite Daniel Vetter
2012-03-25 17:47 ` [PATCH 11/16] drm/i915: fall back to shmem pwrite when the buffer is not accessible Daniel Vetter
2012-03-25 17:47 ` [PATCH 12/16] drm/i915: use uncached writes in pwrite Daniel Vetter
2012-03-25 17:47 ` [PATCH 13/16] drm/i915: extract copy helpers from shmem_pread|pwrite Daniel Vetter
2012-03-26  9:05   ` Chris Wilson
2012-03-25 17:47 ` [PATCH 14/16] mm: extend prefault helpers to fault in more than PAGE_SIZE Daniel Vetter
2012-03-26  9:09   ` Chris Wilson
2012-03-25 17:47 ` [PATCH 15/16] drm/i915: fixup in-line clflushing on bit17 swizzled bos Daniel Vetter
2012-03-26  9:18   ` Chris Wilson
2012-03-26  9:26     ` Daniel Vetter [this message]
2012-03-26 12:50       ` Chris Wilson
2012-03-25 17:47 ` [PATCH 16/16] drm/i915: mark pwrite/pread slowpaths with unlikely Daniel Vetter
2012-03-26  9:09   ` Chris Wilson
2012-03-27 11:42     ` Daniel Vetter

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=20120326092633.GG4014@phenom.ffwll.local \
    --to=daniel@ffwll.ch \
    --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.