From mboxrd@z Thu Jan 1 00:00:00 1970 From: Daniel Vetter Subject: Re: [PATCH 15/16] drm/i915: fixup in-line clflushing on bit17 swizzled bos Date: Mon, 26 Mar 2012 11:26:33 +0200 Message-ID: <20120326092633.GG4014@phenom.ffwll.local> 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> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mail-we0-f177.google.com (mail-we0-f177.google.com [74.125.82.177]) by gabe.freedesktop.org (Postfix) with ESMTP id 7D682A087F for ; Mon, 26 Mar 2012 02:25:50 -0700 (PDT) Received: by werp11 with SMTP id p11so4959497wer.36 for ; Mon, 26 Mar 2012 02:25:49 -0700 (PDT) Content-Disposition: inline In-Reply-To: <1332753528_97351@CP5-2952> 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: Chris Wilson Cc: Daniel Vetter , Intel Graphics Development List-Id: intel-gfx@lists.freedesktop.org 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. > 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