Date:	Mon, 12 Oct 2009 21:16:50 +0200
From:	Thomas Hellstrom <thellstrom@...are.com>
To:	Thomas Schlichter <thomas.schlichter@....de>
CC:	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"x86@...nel.org" <x86@...nel.org>,
	"dri-devel@...ts.sourceforge.net" <dri-devel@...ts.sourceforge.net>,
	Arjan van de Ven <arjan@...ux.intel.com>,
	Yinghai Lu <yinghai@...nel.org>,
	Thomas Gleixner <tglx@...utronix.de>,
	Ingo Molnar <mingo@...hat.com>,
	"H. Peter Anvin" <hpa@...or.com>,
	Jeremy Fitzhardinge <jeremy.fitzhardinge@...rix.com>,
	Venkatesh Pallipadi <venkatesh.pallipadi@...el.com>,
	Suresh Siddha <suresh.b.siddha@...el.com>,
	Jan Beulich <jbeulich@...ell.com>, Tejun Heo <tj@...nel.org>,
	Jesse Barnes <jbarnes@...rnes-g45>,
	Henrique de Moraes Holschuh <hmh@....eng.br>,
	Robert Hancock <hancockrwd@...il.com>
Subject: Re: [RFC Patch] use MTRR for write combining if PAT is not available

Hi!

One problem with this patch is that it conflicts with the way graphics 
drivers traditionally handles
the situation, namely

1) Set up mtrr
2) Map. If fallback to uncached minus we will still have write-combined 
access.

I think mtrr-add used in this fashion will typically fail due to the 
alignment constraints. In particular,
for set_memory_wc() the typical usage pattern is a large number of pages 
in a fragmented physical address space.

So if we were to fix the problem with libpciaccess in the kernel, I 
think the best option would be to fail the user-space mapping when we 
can't make it write-combined.

Thanks,
Thomas

Thomas Schlichter wrote:
> Hi,
>
> when I first sent this E-Mail on Saturday, I unfortunately forgot to CC many 
> people. Now I used get_maintainer.pl to get the list of people that may want 
> to contribute to this topic.
>
> Because of this topic, there is already a patch from Arjan in the -tip tree to 
> make PAT and MTRR options only configurable if EMBEDDED and enabled by 
> default. I think this is a step in the right direction, but at least Henrique, 
> Robert and I seem to think something like the attached patch is still 
> required. What do you think?
>
> Kind regards,
>   Thomas
>
> -----------------------------------------------------------------------------
>
> 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
>   

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists