All of lore.kernel.org
 help / color / mirror / Atom feed
From: Thomas Schlichter <thomas.schlichter@web.de>
To: linux-kernel@vger.kernel.org
Cc: Thomas Hellstrom <thellstrom@vmware.com>,
	Arjan van de Ven <arjan@linux.intel.com>
Subject: [RFC Patch] use MTRR for write combining if PAT is not available
Date: Sat, 10 Oct 2009 03:22:36 +0200	[thread overview]
Message-ID: <200910100322.36857.thomas.schlichter@web.de> (raw)

[-- Attachment #1: Type: Text/Plain, Size: 1494 bytes --]

Hi,

I've found a problem with X.org not setting up MTRR for the framebuffer 
memory. After I investigated I think this is not a X.org problem, but a kernel 
issue.

X.org uses libpciaccess to map the framebuffer memory. This library opens 
/sys/bus/pci/devices/*/resource0_wc and mmaps the memory. Unfortunately, the 
kernel only enables write combining if PAT is enabled, if it is not, the 
memory is mmapped uncached. But Xorg (respectively libpciaccess) thinks it was 
successfully mapped with write combining enabled and thus does not 
additionally set up MTRR entries.

The corresponding libpciaccess code can be found here:
  http://cgit.freedesktop.org/xorg/lib/libpciaccess/tree/src/linux_sysfs.c#n501

If the kernel behavior is intentional and X.org should always set up MTRR 
entries, why should it use /sys/.../resource0_wc at all? I think there are 2 
possibilities to make the kernel behavior consistent:

 1. The mmap_wc should fail if PAT is not enabled.
    (libpciaccess will then map the framebuffer uncached and set up
     MTRR entries)
 2. Use MTRR to enable write combining if PAT is not available.

In an earlier thread about ioremap_wc, Arjan van de Ven wrote that option 2 is 
preferred over option 1:

    http://lkml.indiana.edu/hypermail/linux/kernel/0805.3/2925.html

So, I've created the attached patch implementing option 2. For me this solves 
the problem with the slow Video playback due to not correctly set up MTRR 
entries.

Kind regards,
Thomas Schlichter

[-- Attachment #2: 0001-Use-MTRR-for-write-combining-mmap-ioremap-if-PAT-is-.patch --]
[-- Type: text/x-patch, Size: 3346 bytes --]

From 19fb39061825a0110d1a4feb3f83dfa3f09fb738 Mon Sep 17 00:00:00 2001
From: Thomas Schlichter <thomas.schlichter@web.de>
Date: Thu, 8 Oct 2009 21:24:07 +0200
Subject: [PATCH] Use MTRR for write combining mmap/ioremap if PAT is not available

X.org uses libpciaccess which tries to mmap with write combining enabled via
/sys/bus/pci/devices/*/resource0_wc. Currently, when PAT is not enabled, we
fall back to uncached mmap. Then libpciaccess thinks it succeeded mapping
with write combining anabled and does not set up suited MTRR entries. ;-(

So when falling back to uncached mapping, we better try to set up MTRR
entries automatically. To match this modified PCI mmap behavior, also
ioremap_wc and set_memory_wc are adjusted.

Signed-off-by: Thomas Schlichter <thomas.schlichter@web.de>
---
 arch/x86/mm/ioremap.c  |   14 +++++++++-----
 arch/x86/mm/pageattr.c |   10 ++++++++--
 arch/x86/pci/i386.c    |    6 ++++++
 3 files changed, 23 insertions(+), 7 deletions(-)

diff --git a/arch/x86/mm/ioremap.c b/arch/x86/mm/ioremap.c
index 334e63c..1a73bbc 100644
--- a/arch/x86/mm/ioremap.c
+++ b/arch/x86/mm/ioremap.c
@@ -21,6 +21,7 @@
 #include <asm/tlbflush.h>
 #include <asm/pgalloc.h>
 #include <asm/pat.h>
+#include <asm/mtrr.h>
 
 #include "physaddr.h"
 
@@ -268,11 +269,14 @@ EXPORT_SYMBOL(ioremap_nocache);
  */
 void __iomem *ioremap_wc(resource_size_t phys_addr, unsigned long size)
 {
-	if (pat_enabled)
-		return __ioremap_caller(phys_addr, size, _PAGE_CACHE_WC,
-					__builtin_return_address(0));
-	else
-		return ioremap_nocache(phys_addr, size);
+	if (!pat_enabled) {
+		void __iomem *ret = ioremap_nocache(phys_addr, size);
+		if (ret)
+			mtrr_add(phys_addr, size, MTRR_TYPE_WRCOMB, false);
+		return ret;
+	}
+	return __ioremap_caller(phys_addr, size, _PAGE_CACHE_WC,
+				__builtin_return_address(0));
 }
 EXPORT_SYMBOL(ioremap_wc);
 
diff --git a/arch/x86/mm/pageattr.c b/arch/x86/mm/pageattr.c
index dd38bfb..7f3a85b 100644
--- a/arch/x86/mm/pageattr.c
+++ b/arch/x86/mm/pageattr.c
@@ -23,6 +23,7 @@
 #include <asm/pgalloc.h>
 #include <asm/proto.h>
 #include <asm/pat.h>
+#include <asm/mtrr.h>
 
 /*
  * The current flushing context - we pass it instead of 5 arguments:
@@ -1010,8 +1011,13 @@ int set_memory_wc(unsigned long addr, int numpages)
 {
 	int ret;
 
-	if (!pat_enabled)
-		return set_memory_uc(addr, numpages);
+	if (!pat_enabled) {
+		ret = set_memory_uc(addr, numpages);
+		if (!ret)
+			mtrr_add(__pa(addr), numpages * PAGE_SIZE,
+				 MTRR_TYPE_WRCOMB, false);
+		return ret;
+	}
 
 	ret = reserve_memtype(__pa(addr), __pa(addr) + numpages * PAGE_SIZE,
 		_PAGE_CACHE_WC, NULL);
diff --git a/arch/x86/pci/i386.c b/arch/x86/pci/i386.c
index b22d13b..06cf678 100644
--- a/arch/x86/pci/i386.c
+++ b/arch/x86/pci/i386.c
@@ -33,6 +33,7 @@
 #include <linux/bootmem.h>
 
 #include <asm/pat.h>
+#include <asm/mtrr.h>
 #include <asm/e820.h>
 #include <asm/pci_x86.h>
 #include <asm/io_apic.h>
@@ -301,5 +302,10 @@ int pci_mmap_page_range(struct pci_dev *dev, struct vm_area_struct *vma,
 
 	vma->vm_ops = &pci_mmap_ops;
 
+	if (!pat_enabled && write_combine)
+		mtrr_add_page(vma->vm_pgoff,
+			     (vma->vm_end - vma->vm_start) >> PAGE_SHIFT,
+			      MTRR_TYPE_WRCOMB, false);
+
 	return 0;
 }
-- 
1.6.4.4


             reply	other threads:[~2009-10-10  1:31 UTC|newest]

Thread overview: 60+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-10-10  1:22 Thomas Schlichter [this message]
2009-10-10  4:24 ` [RFC Patch] use MTRR for write combining if PAT is not available Arjan van de Ven
2009-10-10  8:31   ` Thomas Schlichter
2009-10-10 15:45     ` Arjan van de Ven
2009-10-10 17:50       ` Roland Dreier
2009-10-11  7:40       ` Ingo Molnar
2009-10-11  9:56       ` Thomas Schlichter
2009-10-11 18:51   ` Henrique de Moraes Holschuh
2009-10-11 18:54     ` Arjan van de Ven
2009-10-11 20:19       ` Henrique de Moraes Holschuh
2009-10-12 18:09         ` Robert Hancock
     [not found] <200910122032.52168.thomas.schlichter@web.de>
2009-10-12 19:16 ` Thomas Hellstrom
2009-10-12 19:45   ` Thomas Schlichter
     [not found]     ` <1255378684.2063.5.camel@gaiman.anholt.net>
2009-10-13 21:05       ` Thomas Schlichter
  -- strict thread matches above, loose matches on Subject: below --
2009-10-13  7:34 Jan Beulich
2009-10-13 21:29 ` Thomas Schlichter
2009-10-14  8:13   ` Jan Beulich
2009-10-14 19:14     ` Thomas Schlichter
2009-10-15  7:48       ` Jan Beulich
2009-10-17 19:48         ` Thomas Schlichter
2009-10-19  9:16           ` Jan Beulich
2009-10-19 13:44             ` Suresh Siddha
2009-10-19 13:54               ` Ingo Molnar
2009-10-19 13:36           ` Konrad Rzeszutek Wilk
2009-10-19 14:47 Thomas Schlichter
2009-10-20 19:54 ` Thomas Schlichter
2009-10-21 11:57   ` Ingo Molnar
2009-10-19 14:59 Thomas Schlichter
2009-10-19 15:31 ` Ingo Molnar
2009-10-19 21:49   ` Suresh Siddha
2009-10-20 20:35   ` Thomas Schlichter
2009-10-20 21:59     ` Suresh Siddha
2009-10-21 11:52       ` Ingo Molnar
2009-10-19 15:07 Thomas Schlichter
2009-10-19 15:10 Thomas Schlichter
2009-10-19 15:28 ` Ingo Molnar
2009-10-21 13:45 Thomas Schlichter
2009-10-21 14:11 ` Jan Beulich
2009-10-21 17:35   ` Ingo Molnar
2009-10-21 20:01     ` Thomas Schlichter
2009-10-22  9:53       ` Suresh Siddha
2009-10-22 15:34         ` Eric Anholt
2009-10-22 21:47           ` Suresh Siddha
2009-10-22 23:10             ` Jesse Barnes
2009-10-23  0:11               ` Suresh Siddha
2009-10-23  1:53                 ` Eric Anholt
2009-10-23  4:31                   ` Jesse Barnes
2009-10-23  4:58                     ` Suresh Siddha
2009-10-23  7:24                       ` Thomas Schlichter
2009-10-23 14:24                         ` Suresh Siddha
2009-10-23 14:37                           ` Ingo Molnar
2009-10-23  4:33                   ` Suresh Siddha
2009-10-21 14:38 Thomas Schlichter
2009-10-21 15:14 ` Jan Beulich
2009-10-22 12:08 Thomas Schlichter
2009-10-22 12:14 ` H. Peter Anvin
2009-10-22 13:26   ` Suresh Siddha
2009-10-22 13:35 ` Suresh Siddha
2009-10-22 14:27 Thomas Schlichter
2009-10-22 14:41 Thomas Schlichter

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=200910100322.36857.thomas.schlichter@web.de \
    --to=thomas.schlichter@web.de \
    --cc=arjan@linux.intel.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=thellstrom@vmware.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.