Re: [PATCH 3/3] drm/i915: disable wc gtt pte mappings on gen2

[Chris Wilson <chris@chris-wilson.co.uk>]

On Thu, 11 Oct 2012 10:37:54 +0200, Daniel Vetter <daniel@ffwll.ch> wrote:
> On Wed, Oct 10, 2012 at 11:37:36PM +0100, Chris Wilson wrote:
> > On Wed, 10 Oct 2012 23:14:01 +0200, Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
> > > It doesn't work since the gtt pte range sits in the middle of the mmio
> > > bar. We didn't notice that since both my and Chris' gen2 machines
> > > don't support PAT and hence all wc io mapping request will
> > > automatically be demoted to uc.
> > > 
> > > This regression has been introduce in
> > > 
> > > commit edef7e685da05c13cce50c0126189c80fe2c8f71
> > > Author: Chris Wilson <chris@chris-wilson.co.uk>
> > > Date:   Fri Sep 14 11:57:47 2012 +0100
> > > 
> > >     agp/intel: Use a write-combining map for updating PTEs
> > > 
> > > Reported-by: Egbert Eich <eich@pdx.freedesktop.org>
> > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=55834
> > > Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> > 
> > Thanks for the explanation,
> > Acked-by: Chris Wilson <chris@chris-wilson.co.uk>
> Patch merged to -fixes.
> -Daniel
> -- 
> Daniel Vetter
> Software Engineer, Intel Corporation
> +41 (0) 79 365 57 48 - http://blog.ffwll.ch
From: Chris Wilson <chris@chris-wilson.co.uk>
Subject: Re: [Intel-gfx] [PATCH 2/3] drm/i915: fixup the plane->pipe fixup code
To: Daniel Vetter <daniel.vetter@ffwll.ch>, Intel Graphics Development <intel-gfx@lists.freedesktop.org>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
In-Reply-To: <1349903641-18378-2-git-send-email-daniel.vetter@ffwll.ch>
References: <1349903641-18378-1-git-send-email-daniel.vetter@ffwll.ch> <1349903641-18378-2-git-send-email-daniel.vetter@ffwll.ch>

On Wed, 10 Oct 2012 23:14:00 +0200, Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
> We need to check whether the _other plane is on our pipe, not whether
> our plane is on the other pipe. Otherwise if not both pipes/planes are
> active, we won't properly clean up the mess and set up our desired
> plane->pipe mapping.
> 
> v2: Fixup the logic, I've totally fumbled it. Noticed by Chris Wilson.
> 
> v3: I've checked Bspec, and the flexible plane->pipe mapping is a
> gen2/3 feature, so test for that instead of PCH_SPLIT
> 
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=51265
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=49838
> Tested-by: Dave Airlie <airlied@gmail.com>
> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>

Ok, looks good. I would move the gen check into
intel_check_plane_mapping() itself with a short note that gen4+ has a
fixed mapping.

Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre