[PATCH 1/2] drm: add helper to clflush a virtual address range

[PATCH 1/2] drm: add helper to clflush a virtual address range

[Daniel Vetter]

Useful when the page is already mapped to copy date in/out.

For -stable because the next patch (fixing phys obj pwrite) needs this
little helper function.

Cc: stable@kernel.org
Cc: dri-devel@lists.freedesktop.org
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/drm_cache.c |   23 +++++++++++++++++++++++
 include/drm/drmP.h          |    1 +
 2 files changed, 24 insertions(+), 0 deletions(-)

diff --git a/drivers/gpu/drm/drm_cache.c b/drivers/gpu/drm/drm_cache.c
index 5928653..c7c8f6b 100644
--- a/drivers/gpu/drm/drm_cache.c
+++ b/drivers/gpu/drm/drm_cache.c
@@ -98,3 +98,26 @@ drm_clflush_pages(struct page *pages[], unsigned long num_pages)
 #endif
 }
 EXPORT_SYMBOL(drm_clflush_pages);
+
+void
+drm_clflush_virt_range(char *addr, unsigned long length)
+{
+#if defined(CONFIG_X86)
+	if (cpu_has_clflush) {
+		char *end = addr + length;
+		mb();
+		for (; addr < end; addr += boot_cpu_data.x86_clflush_size)
+			clflush(addr);
+		clflush(end - 1);
+		mb();
+		return;
+	}
+
+	if (on_each_cpu(drm_clflush_ipi_handler, NULL, 1) != 0)
+		printk(KERN_ERR "Timed out waiting for cache flush.\n");
+#else
+	printk(KERN_ERR "Architecture has no drm_cache.c support\n");
+	WARN_ON_ONCE(1);
+#endif
+}
+EXPORT_SYMBOL(drm_clflush_virt_range);
diff --git a/include/drm/drmP.h b/include/drm/drmP.h
index 1f9e951..ededdd0 100644
--- a/include/drm/drmP.h
+++ b/include/drm/drmP.h
@@ -1331,6 +1331,7 @@ extern int drm_authmagic(struct drm_device *dev, void *data,
 
 /* Cache management (drm_cache.c) */
 void drm_clflush_pages(struct page *pages[], unsigned long num_pages);
+void drm_clflush_virt_range(char *addr, unsigned long length);
 
 				/* Locking IOCTL support (drm_lock.h) */
 extern int drm_lock(struct drm_device *dev, void *data,
-- 
1.7.7.3

[PATCH 2/2] drm/i915: properly clflush pwrites to phys objects

[Daniel Vetter]

Usually results in (rare) cursor corruptions on platforms
requiring physically addressed cursors.

Note to the stable team: This requires the drm core patch
"drm: add helper to clflush a virtual address range" which
creates the helper used here.

Tested-and-reported-by: Bruno Prémont <bonbons@linux-vserver.org>
Cc: stable@kernel.org
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=35460
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=21442
Signed-Off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/i915/i915_gem.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 39459d2..e395a7d 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -4123,6 +4123,7 @@ i915_gem_phys_pwrite(struct drm_device *dev,
 			return -EFAULT;
 	}
 
+	drm_clflush_virt_range(vaddr, args->size);
 	intel_gtt_chipset_flush();
 	return 0;
 }
-- 
1.7.7.3

Re: [PATCH 2/2] drm/i915: properly clflush pwrites to phys objects

[Chris Wilson]

On Tue, 29 Nov 2011 16:09:29 +0100, Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
> Usually results in (rare) cursor corruptions on platforms
> requiring physically addressed cursors.

So the phys cursor pages are set to WC upon creation, are we just
missing the mb()? Or more likely the CPUs don't have PAT and we are
being lazy in not detecting the error.

Anyway as this is reported to fix the issue on 8xx at least, overkill
is fine. :)

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

-- 
Chris Wilson, Intel Open Source Technology Centre

Re: [PATCH 2/2] drm/i915: properly clflush pwrites to phys objects

[Daniel Vetter]

On Tue, Nov 29, 2011 at 03:35:54PM +0000, Chris Wilson wrote:
> On Tue, 29 Nov 2011 16:09:29 +0100, Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
> > Usually results in (rare) cursor corruptions on platforms
> > requiring physically addressed cursors.
> 
> So the phys cursor pages are set to WC upon creation, are we just
> missing the mb()? Or more likely the CPUs don't have PAT and we are
> being lazy in not detecting the error.

Yes, on reconsidering the tested-by is from a pentium m, which has working
pat, and we do a wbinvd in the i8xx chipset flush, so I don't know anymore
how this patch actually works.

But it seems to indeed fix the issue for at least one reporter and cursor
update is about as far away from a perf critical path as possible, so who
cares about such minor quibbles, it works ;-)
-Daniel
-- 
Daniel Vetter
Mail: daniel@ffwll.ch
Mobile: +41 (0)79 365 57 48

Re: [PATCH 2/2] drm/i915: properly clflush pwrites to phys objects

[Daniel Vetter]

On Tue, Nov 29, 2011 at 05:16:52PM +0100, Daniel Vetter wrote:
> On Tue, Nov 29, 2011 at 03:35:54PM +0000, Chris Wilson wrote:
> > On Tue, 29 Nov 2011 16:09:29 +0100, Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
> > > Usually results in (rare) cursor corruptions on platforms
> > > requiring physically addressed cursors.
> > 
> > So the phys cursor pages are set to WC upon creation, are we just
> > missing the mb()? Or more likely the CPUs don't have PAT and we are
> > being lazy in not detecting the error.
> 
> Yes, on reconsidering the tested-by is from a pentium m, which has working
> pat, and we do a wbinvd in the i8xx chipset flush, so I don't know anymore
> how this patch actually works.
> 
> But it seems to indeed fix the issue for at least one reporter and cursor
> update is about as far away from a perf critical path as possible, so who
> cares about such minor quibbles, it works ;-)

After some irc discussion with Dave Airlie I'll now try whether a simple
wmb(); to flush the wc cache isn't good enough. Will take at least a week
to run by testers and gathe feedback, though.
-Daniel
-- 
Daniel Vetter
Mail: daniel@ffwll.ch
Mobile: +41 (0)79 365 57 48