From mboxrd@z Thu Jan 1 00:00:00 1970 From: Chris Wilson Subject: Re: [PATCH 15/16] drm/i915: fixup in-line clflushing on bit17 swizzled bos Date: Mon, 26 Mar 2012 13:50:42 +0100 Message-ID: <1332766252_99418@CP5-2952> References: <1332697663-31256-1-git-send-email-daniel.vetter@ffwll.ch> <1332697663-31256-15-git-send-email-daniel.vetter@ffwll.ch> <1332753528_97351@CP5-2952> <20120326092633.GG4014@phenom.ffwll.local> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from fireflyinternet.com (smtp.fireflyinternet.com [109.228.6.236]) by gabe.freedesktop.org (Postfix) with ESMTP id 595589E7B1 for ; Mon, 26 Mar 2012 05:50:54 -0700 (PDT) In-Reply-To: <20120326092633.GG4014@phenom.ffwll.local> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: intel-gfx-bounces+gcfxdi-intel-gfx=m.gmane.org@lists.freedesktop.org Errors-To: intel-gfx-bounces+gcfxdi-intel-gfx=m.gmane.org@lists.freedesktop.org To: Daniel Vetter Cc: Daniel Vetter , Intel Graphics Development List-Id: intel-gfx@lists.freedesktop.org On Mon, 26 Mar 2012 11:26:33 +0200, Daniel Vetter wrote: > On Mon, Mar 26, 2012 at 10:18:39AM +0100, Chris Wilson wrote: > > On Sun, 25 Mar 2012 19:47:42 +0200, Daniel Vetter 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. I was struggling to see where exactly we were swizzling the CPU address because I failed to make the connection between shmem_page_offset and gpu_offset. With that resolved, Daniel you deserve a special award for that!, Reviewed-by: Chris Wilson -Chris -- Chris Wilson, Intel Open Source Technology Centre