[Intel-gfx] [PATCH 01/11] drm/i915: flush CPU wc cache when flushing GTT write domain

[Chris Wilson]

On Fri, 15 Jan 2010 14:07:52 +0100, Daniel Vetter <daniel at ffwll.ch> wrote:
> On Fri, Jan 15, 2010 at 12:39:54PM +0000, Chris Wilson wrote:
> > On Fri, 15 Jan 2010 13:24:08 +0100, Daniel Vetter <daniel.vetter at ffwll.ch> wrote:
> > > There are no other instructions that force the CPU to flush the wc
> > > buffer before we tear down the corresponding fence reg with a
> > > pipelined write. IIRC this _might_ get reordered, so enforce correct
> > > ordering with a posting read to a harmless reg.
> > > 
> > > Also move one flush out from under an if (IS_I965), now that it
> > > actually does something.
> > 
> > Let's split these into separate patches, one to do the wc-flush and then
> > we can review all the places that need to worry about GTT write flushes.
> 
> Ok, I think my changeset comment was slightly to terse. Here's the
> relevant code (where I move an gtt flush out from under an if):
> 
> 	/* On the i915, GPU access to tiled buffers is via a fence,
> 	 * therefore we must wait for any outstanding access to complete
> 	 * before clearing the fence.
> 	 */
> 	if (!IS_I965G(dev)) {
> 		int ret;
> 
> 		i915_gem_object_flush_gpu_write_domain(obj);
> 		i915_gem_object_flush_gtt_write_domain(obj);
> 		ret = i915_gem_object_wait_rendering(obj);
> 		if (ret != 0)
> 			return ret;
> 	}
> 
> With the comment it's clear that the gtt should be flushed
> unconditionally, i.e. there's a bug. But it didn't mather till now because
> our gtt flush didn't do anything than update the bookkeeping. So I
> merged the correctness fix with the gtt flush patch.

Once we move userspace to execbuffer2, we can rely on always doing the
correct thing and make the flushes unconditional. Tomorrow. ;-)
 
> Or have you meant something else?

No, it was just mixing the two patches into one.

> > For instance, is a posting read the best method for a wc-flush?
> At least this is the recommended way for pci devices (because it flushes
> all cache from the cpu up to the device, not just the cpu wc cache). Now
> integrated graphics is obviously a different beast altogether, but like I
> said, at least for the overlay code it seems to get the job done.

Ok, thanks for the clarification. I was just hoping that we might be able
to wrap that up in a self-documenting helper. :)
-ickle

-- 
Chris Wilson, Intel Open Source Technology Centre