Re: [PATCH 00/10] drm-intel-collector - update

From: "Ville Syrjälä" <ville.syrjala@linux.intel.com>
To: Chris Wilson <chris@chris-wilson.co.uk>,
	Daniel Vetter <daniel@ffwll.ch>,
	intel-gfx@lists.freedesktop.org,
	Rodrigo Vivi <rodrigo.vivi@intel.com>
Subject: Re: [PATCH 00/10] drm-intel-collector - update
Date: Tue, 11 Nov 2014 14:38:15 +0200	[thread overview]
Message-ID: <20141111123815.GG10649@intel.com> (raw)
In-Reply-To: <20141111122659.GE8220@nuc-i3427.alporthouse.com>

On Tue, Nov 11, 2014 at 12:26:59PM +0000, Chris Wilson wrote:
> On Tue, Nov 11, 2014 at 02:16:32PM +0200, Ville Syrjälä wrote:
> > On Tue, Nov 11, 2014 at 12:54:13PM +0100, Daniel Vetter wrote:
> > > On Tue, Nov 11, 2014 at 10:22:11AM +0000, Chris Wilson wrote:
> > > > On Tue, Nov 11, 2014 at 11:20:14AM +0100, Daniel Vetter wrote:
> > > > > On Thu, Nov 06, 2014 at 09:57:29AM +0200, Ville Syrjälä wrote:
> > > > > > On Tue, Nov 04, 2014 at 04:51:38AM -0800, Rodrigo Vivi wrote:
> > > > > > > Patch     drm/i915: Make the physical object coherent with GTT - Reviewer:
> > > > > > 
> > > > > > Already has my r-b.
> > > > > 
> > > > > I still would like to see a little testcase here, e.g. a new mode to
> > > > > kms_cursor_crc which uses gtt mmap uploads instead of pwrite.
> > > > 
> > > > But that doesn't block this patch, as the kernel already exposes and
> > > > userspace already uses gtt mmap updates to the cursor. The patch just
> > > > removes the barrier to do so using early chipsets as well.
> > > 
> > > Well yeah, but that existing code has piles of tests already to make sure
> > > that gtt writes are somewhat coherent with pwrite, and another test that
> > > pwrite is coherent with the actual cursor scanned out by hw.
> > 
> > Actually pwrite isn't coherent with scanout currently on LLC platforms
> > if you do the pwrite when the bo already has cache_level==NONE but it's
> > not yet pinned to the display. So seems our pwrite tests aren't entirely
> > comprehensive currently if they don't hit this. I still haven't seen
> > the fabled patch from Chris that would fix this.
> 
> This patch includes an unconditional clflush for phys writes (or at
> least this patch should be that patch) which is what I thought we were
> arguing about at the time.

Ah, so there's no patch then? I must have misundestood. I do remember you
had a quick hack to always flush in set_cache_level(), but I guess it
never went beyond that.

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx