* [PATCH 3/4] video: fbdev: mxsfb: Constify platform_device_id
From: Krzysztof Kozlowski @ 2015-05-01 15:38 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1430494721-30793-1-git-send-email-k.kozlowski.k@gmail.com>
The platform_device_id is not modified by the driver and core uses it as
const.
Signed-off-by: Krzysztof Kozlowski <k.kozlowski.k@gmail.com>
---
drivers/video/fbdev/mxsfb.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/video/fbdev/mxsfb.c b/drivers/video/fbdev/mxsfb.c
index f8ac4a452f26..497971c82bb1 100644
--- a/drivers/video/fbdev/mxsfb.c
+++ b/drivers/video/fbdev/mxsfb.c
@@ -814,7 +814,7 @@ static void mxsfb_free_videomem(struct mxsfb_info *host)
free_pages_exact(fb_info->screen_base, fb_info->fix.smem_len);
}
-static struct platform_device_id mxsfb_devtype[] = {
+static const struct platform_device_id mxsfb_devtype[] = {
{
.name = "imx23-fb",
.driver_data = MXSFB_V3,
--
2.1.4
^ permalink raw reply related
* [PATCH 2/4] video: fbdev: imxfb: Constify platform_device_id
From: Krzysztof Kozlowski @ 2015-05-01 15:38 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1430494721-30793-1-git-send-email-k.kozlowski.k@gmail.com>
The platform_device_id is not modified by the driver and core uses it as
const.
Signed-off-by: Krzysztof Kozlowski <k.kozlowski.k@gmail.com>
---
drivers/video/fbdev/imxfb.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/video/fbdev/imxfb.c b/drivers/video/fbdev/imxfb.c
index 84d1d29e532c..cee88603efc9 100644
--- a/drivers/video/fbdev/imxfb.c
+++ b/drivers/video/fbdev/imxfb.c
@@ -170,7 +170,7 @@ struct imxfb_info {
struct regulator *lcd_pwr;
};
-static struct platform_device_id imxfb_devtype[] = {
+static const struct platform_device_id imxfb_devtype[] = {
{
.name = "imx1-fb",
.driver_data = IMX1_FB,
--
2.1.4
^ permalink raw reply related
* [PATCH 1/4] video: backlight: da9052: Constify platform_device_id
From: Krzysztof Kozlowski @ 2015-05-01 15:38 UTC (permalink / raw)
To: linux-arm-kernel
The platform_device_id is not modified by the driver and core uses it as
const.
Signed-off-by: Krzysztof Kozlowski <k.kozlowski.k@gmail.com>
---
drivers/video/backlight/da9052_bl.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/video/backlight/da9052_bl.c b/drivers/video/backlight/da9052_bl.c
index b1943e7735a1..fd2be417aa64 100644
--- a/drivers/video/backlight/da9052_bl.c
+++ b/drivers/video/backlight/da9052_bl.c
@@ -152,7 +152,7 @@ static int da9052_backlight_remove(struct platform_device *pdev)
return 0;
}
-static struct platform_device_id da9052_wled_ids[] = {
+static const struct platform_device_id da9052_wled_ids[] = {
{
.name = "da9052-wled1",
.driver_data = DA9052_TYPE_WLED1,
--
2.1.4
^ permalink raw reply related
* Re: [RFT PATCH] drm/exynos: Enable DP clock to fix display on Exynos5250 and other
From: Krzysztof Kozlowski @ 2015-05-01 9:14 UTC (permalink / raw)
To: Olof Johansson
Cc: Krzysztof Kozlowski, linux-samsung-soc@vger.kernel.org,
Kevin Hilman, Jingoo Han, Seung-Woo Kim,
linux-fbdev@vger.kernel.org, DRI mailing list,
linux-kernel@vger.kernel.org, Andrzej Hajda, Kyungmin Park,
Kukjin Kim, stable, Javier Martinez Canillas,
linux-arm-kernel@lists.infradead.org, Marek Szyprowski
In-Reply-To: <CAOesGMiA7DCGCg2xgLwu_r_rRYKUjpw2HUePCW5Q9ewU61WbdA@mail.gmail.com>
2015-05-01 1:50 GMT+09:00 Olof Johansson <olof@lixom.net>:
> On Thu, Apr 30, 2015 at 9:40 AM, Javier Martinez Canillas
> <javier.martinez@collabora.co.uk> wrote:
>> Hello Olof,
>>
>> On 04/30/2015 05:57 PM, Olof Johansson wrote:
>>> On Thu, Apr 30, 2015 at 8:44 AM, Kevin Hilman <khilman@kernel.org> wrote:
>>>> Krzysztof Kozlowski <k.kozlowski@samsung.com> writes:
>>>>>>>
>>>>>>> This should fix issue reported by Javier [1][2].
>>>>>>>
>>>>>>> Tested on Chromebook Snow (Exynos 5250). More testing would be great,
>>>>>>> especially on other Exynos 5xxx products.
>>>>>>
>>>>>> I hoped to try this on my exynos5 boards, but it doesn't seem to apply
>>>>>> to linux-next or to Linus' master branch.
>>>>>>
>>>>>> Are there some other dependencies here?
>>>>>
>>>>> It is already applied:
>>>>> https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id\x1c363c7cccf64128087002b0779986ad16aff6dc
>>>>
>>>> Er, yup. That would explain it. ;)
>>>>
>>>> Sorry for the noise,
>>>
>>> Well, noise or not, Exynos is still broken in mainline and was broken
>>> on -next for so long in different ways that bisecting it is a futile
>>> exercise in frustration.
>>>
>>> It doesn't seem to show up with a trivial boot using only ramdisk, but
>>> when booting a real distro from disk, it certainly does.
>>>
>>> For example:
>>>
>>> http://arm-soc.lixom.net/bootlogs/mainline/v4.1-rc1-56-g3d99e3f/pi-arm-exynos_defconfig.html
>>>
>>> Disabling CONFIG_DRM makes it boot reliably.
>>>
>>> Arndale doesn't show it for me, but it also doesn't have working graphics.
>>>
>>
>> Both Exynos5250 and Exynos5420 had similar issues and $subject is the fix
>> for 5250 while 5420 is fixed by my "ARM: dts: Make DP a consumer of DISP1
>> power domain on Exynos5420" patch that was posted a long time ago. I have
>> pinged Kukjin several times about this patch and he said that he will pick
>> it this weekend [0].
>>
>> It is indeed very frustrating how many Exynos patches seems to be falling
>> through the crack, even important fixes like these ones that allow boards
>> to boot again.
>>
>> Kevin suggested that Krzysztof could collect and queue patches [1] to help
>> Kukjin and start acting as a co-maintatiner which I think it's a very good
>> idea and Krzysztof already did for some patches during the 4.1 cycle.
Yes, I did. It turned quite well, most of the patches I collected were
pulled :) .
> Yes, that's a much needed improvement. I suggest starting out by
> collecting critical fixes like these, and we'd be happy to merge them
> directly if going through Kukjin will add latency.
>
> Krzysztof, if you can, make sure you get a PGP key setup and start
> collecting signatures from kernel developers.
Sure, I'll start right away!
^ permalink raw reply
* Re: [PATCH v4 2/5] lib: devres: add pcim_iomap_wc() variants
From: Luis R. Rodriguez @ 2015-05-01 0:20 UTC (permalink / raw)
To: Bjorn Helgaas
Cc: Luis R. Rodriguez, mst, plagnioj, tomi.valkeinen, airlied,
daniel.vetter, linux-fbdev, luto, cocci, linux-kernel, Toshi Kani,
Suresh Siddha, Ingo Molnar, Thomas Gleixner, Juergen Gross,
Daniel Vetter, Dave Airlie, Antonino Daplas, Dave Hansen,
Arnd Bergmann, venkatesh.pallipadi, Stefan Bader,
Ville Syrjälä, Mel Gorman, Vlastimil Babka,
Borislav Petkov, Davidlohr Bueso, konrad.wilk, ville.syrjala,
david.vrabel, jbeulich, Roger Pau Monné, xen-devel,
linux-pci
In-Reply-To: <20150430214638.GE7888@google.com>
On Thu, Apr 30, 2015 at 04:46:38PM -0500, Bjorn Helgaas wrote:
> On Thu, Apr 30, 2015 at 07:27:23PM +0200, Luis R. Rodriguez wrote:
> > On Thu, Apr 30, 2015 at 11:26:47AM -0500, Bjorn Helgaas wrote:
>
> > > I don't see users of either pcim_iomap_wc() or pcim_iomap_wc_regions() so
> > > far. Did I miss them, or do you just expect them in the near future?
> >
> > The later, and also I hate seeing folks later add code under EXPORT_SYMBOL()
> > rather than EXPORT_SYMBOL_GPL() so I figure I'd rather do it first. It happened
> > recently in my v1 series, someone beat me to a write-combining export symbol
> > and changed it to EXPORT_SYMBOL(). Feel free to drop this though but I hope
> > no one out there then tries to just add an EXPORT_SYMBOL() later for this...
>
> Why do you want them to be EXPORT_SYMBOL_GPL? I would expect them to be
> exported the same way pcim_iomap(), pcim_iomap_regions(), and ioremap_wc()
> are exported, i.e., with EXPORT_SYMBOL.
>>
> Per Documentation/DocBook/kernel-hacking.tmpl, EXPORT_SYMBOL_GPL "implies
> that the function is considered an internal implementation issue, and not
> really an interface." I don't think these are internal implementation
> issues.
What Documentation/DocBook/kernel-hacking.tmpl states over EXPORT_SYMBOL_GPL()
is old and in no way reflects current trends and reality. For instance, some
folks believe that if some code has EXPORT_SYMBOL() declared that they can use
it on proprietary modules. This is terribly incorrect, quite a few developers
do not in any way stand by this as a "needed" clarification on their code [0].
I'm one of them, but to be even more clear on this I simply *always* use
EXPORT_SYMBOL_GPL() to remove any possible doubt over this on any symbols
that I export. Heck, even tons of driver library code uses EXPORT_SYMBOL_GPL().
[0] https://lkml.org/lkml/2012/4/20/402
Luis
^ permalink raw reply
* Re: [PATCH v4 2/6] x86: document WC MTRR effects on PAT / non-PAT pages
From: Randy Dunlap @ 2015-04-30 22:01 UTC (permalink / raw)
To: Luis R. Rodriguez, mingo, tglx, hpa, bp, plagnioj, tomi.valkeinen,
daniel.vetter, airlied
Cc: dledford, awalls, syrjala, luto, mst, cocci, linux-kernel,
Luis R. Rodriguez, Toshi Kani, Jonathan Corbet, Dave Hansen,
Suresh Siddha, Juergen Gross, Daniel Vetter, Dave Airlie,
Antonino Daplas, Mel Gorman, Vlastimil Babka, Davidlohr Bueso,
linux-fbdev
In-Reply-To: <1430343851-967-3-git-send-email-mcgrof@do-not-panic.com>
On 04/29/15 14:44, Luis R. Rodriguez wrote:
> From: "Luis R. Rodriguez" <mcgrof@suse.com>
>
> ---
> Documentation/x86/mtrr.txt | 18 +++++++++++++++---
> Documentation/x86/pat.txt | 40 +++++++++++++++++++++++++++++++++++++++-
> arch/x86/kernel/cpu/mtrr/main.c | 3 +++
> 3 files changed, 57 insertions(+), 4 deletions(-)
>
> diff --git a/Documentation/x86/pat.txt b/Documentation/x86/pat.txt
> index cf08c9f..7e183e3 100644
> --- a/Documentation/x86/pat.txt
> +++ b/Documentation/x86/pat.txt
> @@ -102,7 +104,43 @@ wants to export a RAM region, it has to do set_memory_uc() or set_memory_wc()
> as step 0 above and also track the usage of those pages and use set_memory_wb()
> before the page is freed to free pool.
>
> -
> +MTRR effects on PAT / non-PAT systems
> +-------------------------------------
> +
> +The following table provides the effects of using write-combining MTRRs when
> +using ioremap*() calls on x86 for both non-PAT and PAT systems. Ideally
> +mtrr_add() usage will be phased in favor of arch_phys_wc_add() which will
> +be a no-op on PAT enabled systems. The region over which a arch_phys_wc_add()
> +is made should already have be ioremap'd with write-combining page attributes
> +or PAT entries, this can be done by using ioremap_wc() / or respective helpers.
> +Devices which combine areas of IO memory desired to remain uncachable with
I would spell it uncacheable. In kernel Documentation/, grep uncacheable finds
14 hits vs. 6 hits for uncachable. No big deal.
> +areas where write-combining is desirable and are restricted by the size
> +requirements of MTRRs should consider splitting up their IO memory space
> +cleanly with ioremap_uc() and ioremap_wc() followed by an arch_phys_wc_add()
> +encompassing both regions. Such use is nevertheless heavily discouraged as
> +the effective memory type is considered implementation defined. This strategy
> +should only be used as last resort on devices with size-contrained regions
size-constrained
> +where otherwise MTRR write-combining would not be effective.
> +
> +Note that you cannot use set_memory_wc() to override / whitelist IO remapped
> +memory space mapped with ioremap*() calls, set_memory_wc() can only be used
> +on RAM.
> +
> +----------------------------------------------------------------------
> +MTRR Non-PAT PAT Linux ioremap value Effective memory type
> +----------------------------------------------------------------------
> + Non-PAT | PAT
> + PAT
> + |PCD
> + ||PWT
> + |||
> +WC 000 WB _PAGE_CACHE_MODE_WB WC | WC
> +WC 001 WC _PAGE_CACHE_MODE_WC WC* | WC
> +WC 010 UC- _PAGE_CACHE_MODE_UC_MINUS WC* | WC
> +WC 011 UC _PAGE_CACHE_MODE_UC UC | UC
> +----------------------------------------------------------------------
> +
> +(*) denotes implementation defined and is discouraged
>
> Notes:
>
> diff --git a/arch/x86/kernel/cpu/mtrr/main.c b/arch/x86/kernel/cpu/mtrr/main.c
> index ea5f363..12abdbe 100644
> --- a/arch/x86/kernel/cpu/mtrr/main.c
> +++ b/arch/x86/kernel/cpu/mtrr/main.c
> @@ -538,6 +538,9 @@ EXPORT_SYMBOL(mtrr_del);
> * attempts to add a WC MTRR covering size bytes starting at base and
> * logs an error if this fails.
> *
> + * The caller should expect to need to provide a power of two size on an
* The called should provide a power of two size on an equivalent
* power of two boundary.
> + * equivalent power of two boundary.
> + *
> * Drivers must store the return value to pass to mtrr_del_wc_if_needed,
> * but drivers should not try to interpret that return value.
> */
>
--
~Randy
^ permalink raw reply
* Re: [PATCH v4 2/5] lib: devres: add pcim_iomap_wc() variants
From: Bjorn Helgaas @ 2015-04-30 21:46 UTC (permalink / raw)
To: Luis R. Rodriguez
Cc: Luis R. Rodriguez, mst, plagnioj, tomi.valkeinen, airlied,
daniel.vetter, linux-fbdev, luto, cocci, linux-kernel, Toshi Kani,
Suresh Siddha, Ingo Molnar, Thomas Gleixner, Juergen Gross,
Daniel Vetter, Dave Airlie, Antonino Daplas, Dave Hansen,
Arnd Bergmann, venkatesh.pallipadi, Stefan Bader,
Ville Syrjälä, Mel Gorman, Vlastimil Babka,
Borislav Petkov, Davidlohr Bueso, konrad.wilk, ville.syrjala,
david.vrabel, jbeulich, Roger Pau Monné, xen-devel,
linux-pci
In-Reply-To: <20150430172723.GU5622@wotan.suse.de>
On Thu, Apr 30, 2015 at 07:27:23PM +0200, Luis R. Rodriguez wrote:
> On Thu, Apr 30, 2015 at 11:26:47AM -0500, Bjorn Helgaas wrote:
> > I don't see users of either pcim_iomap_wc() or pcim_iomap_wc_regions() so
> > far. Did I miss them, or do you just expect them in the near future?
>
> The later, and also I hate seeing folks later add code under EXPORT_SYMBOL()
> rather than EXPORT_SYMBOL_GPL() so I figure I'd rather do it first. It happened
> recently in my v1 series, someone beat me to a write-combining export symbol
> and changed it to EXPORT_SYMBOL(). Feel free to drop this though but I hope
> no one out there then tries to just add an EXPORT_SYMBOL() later for this...
Why do you want them to be EXPORT_SYMBOL_GPL? I would expect them to be
exported the same way pcim_iomap(), pcim_iomap_regions(), and ioremap_wc()
are exported, i.e., with EXPORT_SYMBOL.
Per Documentation/DocBook/kernel-hacking.tmpl, EXPORT_SYMBOL_GPL "implies
that the function is considered an internal implementation issue, and not
really an interface." I don't think these are internal implementation
issues.
Bjorn
^ permalink raw reply
* [PATCH v5 6/6] IB/ipath: use arch_phys_wc_add() and require PAT disabled
From: Luis R. Rodriguez @ 2015-04-30 20:25 UTC (permalink / raw)
To: bp, mingo, tglx, hpa, plagnioj, tomi.valkeinen, daniel.vetter,
airlied
Cc: dledford, awalls, syrjala, luto, mst, cocci, linux-kernel,
Luis R. Rodriguez, Hal Rosenstock, Sean Hefty, Suresh Siddha,
Rickard Strandqvist, Mike Marciniszyn, Roland Dreier,
Linus Torvalds, Juergen Gross, Daniel Vetter, Dave Airlie,
Bjorn Helgaas, Antonino Daplas, Mel Gorman, Vlastimil Babka,
Davidlohr Bueso, Dave Hansen, Arnd Bergmann,
Stefan Bader <stefan.>
In-Reply-To: <1430425520-22275-1-git-send-email-mcgrof@do-not-panic.com>
From: "Luis R. Rodriguez" <mcgrof@suse.com>
We are burrying direct access to MTRR code support on
x86 in order to take advantage of PAT. In the future we
also want to make the default behaviour of ioremap_nocache()
to use strong UC, use of mtrr_add() on those systems
would make write-combining void.
In order to help both enable us to later make strong
UC default and in order to phase out direct MTRR access
code port the driver over to arch_phys_wc_add() and
annotate that the device driver requires systems to
boot with PAT disabled, with the nopat kernel parameter.
This is a worthy compromise given that the ipath device
driver powers the old HTX bus cards that only work in
AMD systems, while the newer IB/qib device driver
powers all PCI-e cards. The ipath device driver is
obsolete, hardware hard to find and because of this
this its a reasonable compromise to make to require
users of ipath to boot with nopat.
Acked-by: Doug Ledford <dledford@redhat.com>
Cc: Doug Ledford <dledford@redhat.com>
Cc: Andy Walls <awalls@md.metrocast.net>
Cc: Hal Rosenstock <hal.rosenstock@gmail.com>
Cc: Sean Hefty <sean.hefty@intel.com>
Cc: Suresh Siddha <sbsiddha@gmail.com>
Cc: Rickard Strandqvist <rickard_strandqvist@spectrumdigital.se>
Cc: Mike Marciniszyn <mike.marciniszyn@intel.com>
Cc: Roland Dreier <roland@purestorage.com>
Cc: Andy Lutomirski <luto@amacapital.net>
Cc: Ingo Molnar <mingo@elte.hu>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Juergen Gross <jgross@suse.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Dave Airlie <airlied@redhat.com>
Cc: Bjorn Helgaas <bhelgaas@google.com>
Cc: Antonino Daplas <adaplas@gmail.com>
Cc: Jean-Christophe Plagniol-Villard <plagnioj@jcrosoft.com>
Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>
Cc: Ville Syrjälä <syrjala@sci.fi>
Cc: Mel Gorman <mgorman@suse.de>
Cc: Vlastimil Babka <vbabka@suse.cz>
Cc: Borislav Petkov <bp@suse.de>
Cc: Davidlohr Bueso <dbueso@suse.de>
Cc: Dave Hansen <dave.hansen@linux.intel.com>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: Michael S. Tsirkin <mst@redhat.com>
Cc: Stefan Bader <stefan.bader@canonical.com>
Cc: konrad.wilk@oracle.com
Cc: ville.syrjala@linux.intel.com
Cc: david.vrabel@citrix.com
Cc: jbeulich@suse.com
Cc: toshi.kani@hp.com
Cc: Roger Pau Monné <roger.pau@citrix.com>
Cc: infinipath@intel.com
Cc: linux-rdma@vger.kernel.org
Cc: linux-fbdev@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Cc: xen-devel@lists.xensource.com
Signed-off-by: Luis R. Rodriguez <mcgrof@suse.com>
---
drivers/infiniband/hw/ipath/Kconfig | 3 ++
drivers/infiniband/hw/ipath/ipath_driver.c | 18 +++++++----
drivers/infiniband/hw/ipath/ipath_kernel.h | 4 +--
drivers/infiniband/hw/ipath/ipath_wc_x86_64.c | 43 ++++++---------------------
4 files changed, 26 insertions(+), 42 deletions(-)
diff --git a/drivers/infiniband/hw/ipath/Kconfig b/drivers/infiniband/hw/ipath/Kconfig
index 1d9bb11..8fe54ff 100644
--- a/drivers/infiniband/hw/ipath/Kconfig
+++ b/drivers/infiniband/hw/ipath/Kconfig
@@ -9,3 +9,6 @@ config INFINIBAND_IPATH
as IP-over-InfiniBand as well as with userspace applications
(in conjunction with InfiniBand userspace access).
For QLogic PCIe QLE based cards, use the QIB driver instead.
+
+ If you have this hardware you will need to boot with PAT disabled
+ on your x86-64 systems, use the nopat kernel parameter.
diff --git a/drivers/infiniband/hw/ipath/ipath_driver.c b/drivers/infiniband/hw/ipath/ipath_driver.c
index bd0caed..441cfe5 100644
--- a/drivers/infiniband/hw/ipath/ipath_driver.c
+++ b/drivers/infiniband/hw/ipath/ipath_driver.c
@@ -42,6 +42,9 @@
#include <linux/bitmap.h>
#include <linux/slab.h>
#include <linux/module.h>
+#ifdef CONFIG_X86_64
+#include <asm/pat.h>
+#endif
#include "ipath_kernel.h"
#include "ipath_verbs.h"
@@ -395,6 +398,14 @@ static int ipath_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
unsigned long long addr;
u32 bar0 = 0, bar1 = 0;
+#ifdef CONFIG_X86_64
+ if (WARN(pat_enabled(),
+ "ipath needs PAT disabled, boot with nopat kernel parameter\n")) {
+ ret = EINVAL;
+ goto bail;
+ }
+#endif
+
dd = ipath_alloc_devdata(pdev);
if (IS_ERR(dd)) {
ret = PTR_ERR(dd);
@@ -542,6 +553,7 @@ static int ipath_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
dd->ipath_kregbase = __ioremap(addr, len,
(_PAGE_NO_CACHE|_PAGE_WRITETHRU));
#else
+ /* XXX: split this properly to enable on PAT */
dd->ipath_kregbase = ioremap_nocache(addr, len);
#endif
@@ -587,12 +599,8 @@ static int ipath_init_one(struct pci_dev *pdev, const struct pci_device_id *ent)
ret = ipath_enable_wc(dd);
- if (ret) {
- ipath_dev_err(dd, "Write combining not enabled "
- "(err %d): performance may be poor\n",
- -ret);
+ if (ret)
ret = 0;
- }
ipath_verify_pioperf(dd);
diff --git a/drivers/infiniband/hw/ipath/ipath_kernel.h b/drivers/infiniband/hw/ipath/ipath_kernel.h
index e08db70..f0f9471 100644
--- a/drivers/infiniband/hw/ipath/ipath_kernel.h
+++ b/drivers/infiniband/hw/ipath/ipath_kernel.h
@@ -463,9 +463,7 @@ struct ipath_devdata {
/* offset in HT config space of slave/primary interface block */
u8 ipath_ht_slave_off;
/* for write combining settings */
- unsigned long ipath_wc_cookie;
- unsigned long ipath_wc_base;
- unsigned long ipath_wc_len;
+ int wc_cookie;
/* ref count for each pkey */
atomic_t ipath_pkeyrefs[4];
/* shadow copy of struct page *'s for exp tid pages */
diff --git a/drivers/infiniband/hw/ipath/ipath_wc_x86_64.c b/drivers/infiniband/hw/ipath/ipath_wc_x86_64.c
index 70c1f3a..7b6e4c8 100644
--- a/drivers/infiniband/hw/ipath/ipath_wc_x86_64.c
+++ b/drivers/infiniband/hw/ipath/ipath_wc_x86_64.c
@@ -37,7 +37,6 @@
*/
#include <linux/pci.h>
-#include <asm/mtrr.h>
#include <asm/processor.h>
#include "ipath_kernel.h"
@@ -122,27 +121,14 @@ int ipath_enable_wc(struct ipath_devdata *dd)
}
if (!ret) {
- int cookie;
- ipath_cdbg(VERBOSE, "Setting mtrr for chip to WC "
- "(addr %llx, len=0x%llx)\n",
- (unsigned long long) pioaddr,
- (unsigned long long) piolen);
- cookie = mtrr_add(pioaddr, piolen, MTRR_TYPE_WRCOMB, 1);
- if (cookie < 0) {
- {
- dev_info(&dd->pcidev->dev,
- "mtrr_add() WC for PIO bufs "
- "failed (%d)\n",
- cookie);
- ret = -EINVAL;
- }
- } else {
- ipath_cdbg(VERBOSE, "Set mtrr for chip to WC, "
- "cookie is %d\n", cookie);
- dd->ipath_wc_cookie = cookie;
- dd->ipath_wc_base = (unsigned long) pioaddr;
- dd->ipath_wc_len = (unsigned long) piolen;
- }
+ dd->wc_cookie = arch_phys_wc_add(pioaddr, piolen);
+ if (dd->wc_cookie < 0) {
+ ipath_dev_err(dd, "Seting mtrr failed on PIO buffers\n");
+ ret = -ENODEV;
+ } else if (dd->wc_cookie = 0)
+ ipath_cdbg(VERBOSE, "Set mtrr for chip to WC not needed\n");
+ else
+ ipath_cdbg(VERBOSE, "Set mtrr for chip to WC\n");
}
return ret;
@@ -154,16 +140,5 @@ int ipath_enable_wc(struct ipath_devdata *dd)
*/
void ipath_disable_wc(struct ipath_devdata *dd)
{
- if (dd->ipath_wc_cookie) {
- int r;
- ipath_cdbg(VERBOSE, "undoing WCCOMB on pio buffers\n");
- r = mtrr_del(dd->ipath_wc_cookie, dd->ipath_wc_base,
- dd->ipath_wc_len);
- if (r < 0)
- dev_info(&dd->pcidev->dev,
- "mtrr_del(%lx, %lx, %lx) failed: %d\n",
- dd->ipath_wc_cookie, dd->ipath_wc_base,
- dd->ipath_wc_len, r);
- dd->ipath_wc_cookie = 0; /* even on failure */
- }
+ arch_phys_wc_del(dd->wc_cookie);
}
--
2.3.2.209.gd67f9d5.dirty
^ permalink raw reply related
* [PATCH v5 5/6] IB/ipath: add counting for MTRR
From: Luis R. Rodriguez @ 2015-04-30 20:25 UTC (permalink / raw)
To: bp, mingo, tglx, hpa, plagnioj, tomi.valkeinen, daniel.vetter,
airlied
Cc: dledford, awalls, syrjala, luto, mst, cocci, linux-kernel,
Luis R. Rodriguez, Toshi Kani, Roland Dreier, Sean Hefty,
Hal Rosenstock, Suresh Siddha, Juergen Gross, Daniel Vetter,
Dave Airlie, Antonino Daplas, infinipath, linux-rdma, linux-fbdev
In-Reply-To: <1430425520-22275-1-git-send-email-mcgrof@do-not-panic.com>
From: "Luis R. Rodriguez" <mcgrof@suse.com>
There is no good reason not to, we eventually delete it as well.
Cc: Toshi Kani <toshi.kani@hp.com>
Cc: Roland Dreier <roland@kernel.org>
Cc: Sean Hefty <sean.hefty@intel.com>
Cc: Hal Rosenstock <hal.rosenstock@gmail.com>
Cc: Suresh Siddha <sbsiddha@gmail.com>
Cc: Ingo Molnar <mingo@elte.hu>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Juergen Gross <jgross@suse.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Andy Lutomirski <luto@amacapital.net>
Cc: Dave Airlie <airlied@redhat.com>
Cc: Antonino Daplas <adaplas@gmail.com>
Cc: Jean-Christophe Plagniol-Villard <plagnioj@jcrosoft.com>
Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>
Cc: infinipath@intel.com
Cc: linux-rdma@vger.kernel.org
Cc: linux-fbdev@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Signed-off-by: Luis R. Rodriguez <mcgrof@suse.com>
---
drivers/infiniband/hw/ipath/ipath_wc_x86_64.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/infiniband/hw/ipath/ipath_wc_x86_64.c b/drivers/infiniband/hw/ipath/ipath_wc_x86_64.c
index 4ad0b93..70c1f3a 100644
--- a/drivers/infiniband/hw/ipath/ipath_wc_x86_64.c
+++ b/drivers/infiniband/hw/ipath/ipath_wc_x86_64.c
@@ -127,7 +127,7 @@ int ipath_enable_wc(struct ipath_devdata *dd)
"(addr %llx, len=0x%llx)\n",
(unsigned long long) pioaddr,
(unsigned long long) piolen);
- cookie = mtrr_add(pioaddr, piolen, MTRR_TYPE_WRCOMB, 0);
+ cookie = mtrr_add(pioaddr, piolen, MTRR_TYPE_WRCOMB, 1);
if (cookie < 0) {
{
dev_info(&dd->pcidev->dev,
--
2.3.2.209.gd67f9d5.dirty
^ permalink raw reply related
* [PATCH v5 4/6] ivtv: use arch_phys_wc_add() and require PAT disabled
From: Luis R. Rodriguez @ 2015-04-30 20:25 UTC (permalink / raw)
To: bp, mingo, tglx, hpa, plagnioj, tomi.valkeinen, daniel.vetter,
airlied
Cc: dledford, awalls, syrjala, luto, mst, cocci, linux-kernel,
Luis R. Rodriguez, Mauro Carvalho Chehab, Suresh Siddha,
Juergen Gross, Daniel Vetter, Dave Airlie, Bjorn Helgaas,
Antonino Daplas, Dave Hansen, Arnd Bergmann, Stefan Bader,
Mel Gorman, Vlastimil Babka, Davidlohr Bueso, konrad.wilk,
ville.syrjala, david.vrabel, jbeulich, toshi.kani,
Roger Pau Monné, linux-fbdev, ivtv-devel, linux-media,
xen-devel
In-Reply-To: <1430425520-22275-1-git-send-email-mcgrof@do-not-panic.com>
From: "Luis R. Rodriguez" <mcgrof@suse.com>
We are burrying direct access to MTRR code support on
x86 in order to take advantage of PAT. In the future we
also want to make the default behaviour of ioremap_nocache()
to use strong UC, use of mtrr_add() on those systems
would make write-combining void.
In order to help both enable us to later make strong
UC default and in order to phase out direct MTRR access
code port the driver over to arch_phys_wc_add() and
annotate that the device driver requires systems to
boot with PAT disabled, with the nopat kernel parameter.
This is a worthy comprmise given that the hardware is
really rare these days, and perhaps only some lost souls
in some third world country are expected to be using this
feature of the device driver.
Acked-by: Andy Walls <awalls@md.metrocast.net>
Cc: Andy Walls <awalls@md.metrocast.net>
Cc: Doug Ledford <dledford@redhat.com>
Cc: Mauro Carvalho Chehab <mchehab@osg.samsung.com>
Cc: Andy Lutomirski <luto@amacapital.net>
Cc: Suresh Siddha <sbsiddha@gmail.com>
Cc: Ingo Molnar <mingo@elte.hu>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Juergen Gross <jgross@suse.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Dave Airlie <airlied@redhat.com>
Cc: Bjorn Helgaas <bhelgaas@google.com>
Cc: Antonino Daplas <adaplas@gmail.com>
Cc: Jean-Christophe Plagniol-Villard <plagnioj@jcrosoft.com>
Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>
Cc: Dave Hansen <dave.hansen@linux.intel.com>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: Michael S. Tsirkin <mst@redhat.com>
Cc: Stefan Bader <stefan.bader@canonical.com>
Cc: Ville Syrjälä <syrjala@sci.fi>
Cc: Mel Gorman <mgorman@suse.de>
Cc: Vlastimil Babka <vbabka@suse.cz>
Cc: Borislav Petkov <bp@suse.de>
Cc: Davidlohr Bueso <dbueso@suse.de>
Cc: konrad.wilk@oracle.com
Cc: ville.syrjala@linux.intel.com
Cc: david.vrabel@citrix.com
Cc: jbeulich@suse.com
Cc: toshi.kani@hp.com
Cc: Roger Pau Monné <roger.pau@citrix.com>
Cc: linux-fbdev@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Cc: ivtv-devel@ivtvdriver.org
Cc: linux-media@vger.kernel.org
Cc: xen-devel@lists.xensource.com
Signed-off-by: Luis R. Rodriguez <mcgrof@suse.com>
---
drivers/media/pci/ivtv/Kconfig | 3 +++
drivers/media/pci/ivtv/ivtvfb.c | 58 ++++++++++++++++-------------------------
2 files changed, 26 insertions(+), 35 deletions(-)
diff --git a/drivers/media/pci/ivtv/Kconfig b/drivers/media/pci/ivtv/Kconfig
index dd6ee57e..b2a7f88 100644
--- a/drivers/media/pci/ivtv/Kconfig
+++ b/drivers/media/pci/ivtv/Kconfig
@@ -57,5 +57,8 @@ config VIDEO_FB_IVTV
This is used in the Hauppauge PVR-350 card. There is a driver
homepage at <http://www.ivtvdriver.org>.
+ If you have this hardware you will need to boot with PAT disabled
+ on your x86 systems, use the nopat kernel parameter.
+
To compile this driver as a module, choose M here: the
module will be called ivtvfb.
diff --git a/drivers/media/pci/ivtv/ivtvfb.c b/drivers/media/pci/ivtv/ivtvfb.c
index 9ff1230..7685ae3 100644
--- a/drivers/media/pci/ivtv/ivtvfb.c
+++ b/drivers/media/pci/ivtv/ivtvfb.c
@@ -44,8 +44,8 @@
#include <linux/ivtvfb.h>
#include <linux/slab.h>
-#ifdef CONFIG_MTRR
-#include <asm/mtrr.h>
+#ifdef CONFIG_X86_64
+#include <asm/pat.h>
#endif
#include "ivtv-driver.h"
@@ -155,12 +155,11 @@ struct osd_info {
/* Buffer size */
u32 video_buffer_size;
-#ifdef CONFIG_MTRR
/* video_base rounded down as required by hardware MTRRs */
unsigned long fb_start_aligned_physaddr;
/* video_base rounded up as required by hardware MTRRs */
unsigned long fb_end_aligned_physaddr;
-#endif
+ int wc_cookie;
/* Store the buffer offset */
int set_osd_coords_x;
@@ -1099,6 +1098,8 @@ static int ivtvfb_init_vidmode(struct ivtv *itv)
static int ivtvfb_init_io(struct ivtv *itv)
{
struct osd_info *oi = itv->osd_info;
+ /* Find the largest power of two that maps the whole buffer */
+ int size_shift = 31;
mutex_lock(&itv->serialize_lock);
if (ivtv_init_on_first_open(itv)) {
@@ -1132,29 +1133,16 @@ static int ivtvfb_init_io(struct ivtv *itv)
oi->video_pbase, oi->video_vbase,
oi->video_buffer_size / 1024);
-#ifdef CONFIG_MTRR
- {
- /* Find the largest power of two that maps the whole buffer */
- int size_shift = 31;
-
- while (!(oi->video_buffer_size & (1 << size_shift))) {
- size_shift--;
- }
- size_shift++;
- oi->fb_start_aligned_physaddr = oi->video_pbase & ~((1 << size_shift) - 1);
- oi->fb_end_aligned_physaddr = oi->video_pbase + oi->video_buffer_size;
- oi->fb_end_aligned_physaddr += (1 << size_shift) - 1;
- oi->fb_end_aligned_physaddr &= ~((1 << size_shift) - 1);
- if (mtrr_add(oi->fb_start_aligned_physaddr,
- oi->fb_end_aligned_physaddr - oi->fb_start_aligned_physaddr,
- MTRR_TYPE_WRCOMB, 1) < 0) {
- IVTVFB_INFO("disabled mttr\n");
- oi->fb_start_aligned_physaddr = 0;
- oi->fb_end_aligned_physaddr = 0;
- }
- }
-#endif
-
+ while (!(oi->video_buffer_size & (1 << size_shift)))
+ size_shift--;
+ size_shift++;
+ oi->fb_start_aligned_physaddr = oi->video_pbase & ~((1 << size_shift) - 1);
+ oi->fb_end_aligned_physaddr = oi->video_pbase + oi->video_buffer_size;
+ oi->fb_end_aligned_physaddr += (1 << size_shift) - 1;
+ oi->fb_end_aligned_physaddr &= ~((1 << size_shift) - 1);
+ oi->wc_cookie = arch_phys_wc_add(oi->fb_start_aligned_physaddr,
+ oi->fb_end_aligned_physaddr -
+ oi->fb_start_aligned_physaddr);
/* Blank the entire osd. */
memset_io(oi->video_vbase, 0, oi->video_buffer_size);
@@ -1172,14 +1160,7 @@ static void ivtvfb_release_buffers (struct ivtv *itv)
/* Release pseudo palette */
kfree(oi->ivtvfb_info.pseudo_palette);
-
-#ifdef CONFIG_MTRR
- if (oi->fb_end_aligned_physaddr) {
- mtrr_del(-1, oi->fb_start_aligned_physaddr,
- oi->fb_end_aligned_physaddr - oi->fb_start_aligned_physaddr);
- }
-#endif
-
+ arch_phys_wc_del(oi->wc_cookie);
kfree(oi);
itv->osd_info = NULL;
}
@@ -1284,6 +1265,13 @@ static int __init ivtvfb_init(void)
int registered = 0;
int err;
+#ifdef CONFIG_X86_64
+ if (WARN(pat_enabled(),
+ "ivtvfb needs PAT disabled, boot with nopat kernel parameter\n")) {
+ return EINVAL;
+ }
+#endif
+
if (ivtvfb_card_id < -1 || ivtvfb_card_id >= IVTV_MAX_CARDS) {
printk(KERN_ERR "ivtvfb: ivtvfb_card_id parameter is out of range (valid range: -1 - %d)\n",
IVTV_MAX_CARDS - 1);
--
2.3.2.209.gd67f9d5.dirty
^ permalink raw reply related
* Re: [PATCH v3] staging: sm750fb: use arch_phys_wc_add() and ioremap_wc()
From: Greg Kroah-Hartman @ 2015-04-30 20:20 UTC (permalink / raw)
To: Luis R. Rodriguez
Cc: arnaud.patard, aaro.koskinen, devel@driverdev.osuosl.org,
Andy Lutomirski, cocci@systeme.lip6.fr, Sudip Mukherjee,
Teddy Wang, Suresh Siddha, Ingo Molnar, Thomas Gleixner,
Juergen Gross, Daniel Vetter, Dave Airlie, Antonino Daplas,
Jean-Christophe Plagniol-Villard, Tomi Valkeinen, linux-fbdev,
linux-kernel@vger.kernel.org
In-Reply-To: <CAB=NE6Vb-u05VZhm5KDnwxqYeDuJjhd89=mFmGJkofSg2y2rnw@mail.gmail.com>
On Thu, Apr 30, 2015 at 10:38:27AM -0700, Luis R. Rodriguez wrote:
> On Tue, Apr 21, 2015 at 1:13 PM, Luis R. Rodriguez
> <mcgrof@do-not-panic.com> wrote:
> > From: "Luis R. Rodriguez" <mcgrof@suse.com>
> >
> > The same area used for ioremap() is used for the MTRR area.
> > Convert the driver from using the x86 specific MTRR code to
> > the architecture agnostic arch_phys_wc_add(). arch_phys_wc_add()
> > will avoid MTRR if write-combining is available, in order to
> > take advantage of that also ensure the ioremap'd area is requested
> > as write-combining.
> >
> > There are a few motivations for this:
> >
> > a) Take advantage of PAT when available
> >
> > b) Help bury MTRR code away, MTRR is architecture specific and on
> > x86 its replaced by PAT
> >
> > c) Help with the goal of eventually using _PAGE_CACHE_UC over
> > _PAGE_CACHE_UC_MINUS on x86 on ioremap_nocache() (see commit
> > de33c442e titled "x86 PAT: fix performance drop for glx,
> > use UC minus for ioremap(), ioremap_nocache() and
> > pci_mmap_page_range()")
> >
> > The conversion done is expressed by the following Coccinelle
> > SmPL patch, it additionally required manual intervention to
> > address all the #ifdery and removal of redundant things which
> > arch_phys_wc_add() already addresses such as verbose message
> > about when MTRR fails and doing nothing when we didn't get
> > an MTRR.
> >
> > @ mtrr_found @
> > expression index, base, size;
> > @@
> >
> > -index = mtrr_add(base, size, MTRR_TYPE_WRCOMB, 1);
> > +index = arch_phys_wc_add(base, size);
> >
> > @ mtrr_rm depends on mtrr_found @
> > expression mtrr_found.index, mtrr_found.base, mtrr_found.size;
> > @@
> >
> > -mtrr_del(index, base, size);
> > +arch_phys_wc_del(index);
> >
> > @ mtrr_rm_zero_arg depends on mtrr_found @
> > expression mtrr_found.index;
> > @@
> >
> > -mtrr_del(index, 0, 0);
> > +arch_phys_wc_del(index);
> >
> > @ mtrr_rm_fb_info depends on mtrr_found @
> > struct fb_info *info;
> > expression mtrr_found.index;
> > @@
> >
> > -mtrr_del(index, info->fix.smem_start, info->fix.smem_len);
> > +arch_phys_wc_del(index);
> >
> > @ ioremap_replace_nocache depends on mtrr_found @
> > struct fb_info *info;
> > expression base, size;
> > @@
> >
> > -info->screen_base = ioremap_nocache(base, size);
> > +info->screen_base = ioremap_wc(base, size);
> >
> > @ ioremap_replace_default depends on mtrr_found @
> > struct fb_info *info;
> > expression base, size;
> > @@
> >
> > -info->screen_base = ioremap(base, size);
> > +info->screen_base = ioremap_wc(base, size);
> >
> > Generated-by: Coccinelle SmPL
> > Cc: Sudip Mukherjee <sudipm.mukherjee@gmail.com>
> > Cc: Teddy Wang <teddy.wang@siliconmotion.com>
> > Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> > Cc: Suresh Siddha <sbsiddha@gmail.com>
> > Cc: Ingo Molnar <mingo@elte.hu>
> > Cc: Thomas Gleixner <tglx@linutronix.de>
> > Cc: Juergen Gross <jgross@suse.com>
> > Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> > Cc: Andy Lutomirski <luto@amacapital.net>
> > Cc: Dave Airlie <airlied@redhat.com>
> > Cc: Antonino Daplas <adaplas@gmail.com>
> > Cc: Jean-Christophe Plagniol-Villard <plagnioj@jcrosoft.com>
> > Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>
> > Cc: devel@driverdev.osuosl.org
> > Cc: linux-fbdev@vger.kernel.org
> > Cc: linux-kernel@vger.kernel.org
> > Signed-off-by: Luis R. Rodriguez <mcgrof@suse.com>
>
> Hey folks, just a follow up. Can this be considered to be merged?
>
> Luis
I'm just starting to get to staging tree patches after the -rc1 window
was over. Give me a week or so, they are at the bottom of my list.
thanks,
greg k-h
^ permalink raw reply
* Re: [PATCH v1 23/47] staging: xgifb: use arch_phys_wc_add() and ioremap_wc()
From: Luis R. Rodriguez @ 2015-04-30 17:40 UTC (permalink / raw)
To: Andy Lutomirski, Ingo Molnar, Thomas Gleixner, H. Peter Anvin,
Juergen Gross, Jan Beulich, Borislav Petkov, Suresh Siddha,
venkatesh.pallipadi, Dave Airlie
Cc: linux-kernel@vger.kernel.org, linux-fbdev, X86 ML, xen-devel,
Luis R. Rodriguez, Ingo Molnar, Daniel Vetter, Antonino Daplas,
Jean-Christophe Plagniol-Villard, Tomi Valkeinen
In-Reply-To: <1426893517-2511-24-git-send-email-mcgrof@do-not-panic.com>
On Fri, Mar 20, 2015 at 4:18 PM, Luis R. Rodriguez
<mcgrof@do-not-panic.com> wrote:
> The same area used for ioremap() is used for the MTRR area.
> Convert the driver from using the x86 specific MTRR code to
> the architecture agnostic arch_phys_wc_add(). arch_phys_wc_add()
> will avoid MTRR if write-combining is available, in order to
> take advantage of that also ensure the ioremap'd area is requested
> as write-combining.
>
> There are a few motivations for this:
>
> a) Take advantage of PAT when available
>
> b) Help bury MTRR code away, MTRR is architecture specific and on
> x86 its replaced by PAT
>
> c) Help with the goal of eventually using _PAGE_CACHE_UC over
> _PAGE_CACHE_UC_MINUS on x86 on ioremap_nocache() (de33c442e)
>
> The conversion done is expressed by the following Coccinelle
> SmPL patch, it additionally required manual intervention to
> address all the #ifdery and removal of redundant things which
> arch_phys_wc_add() already addresses such as verbose message
> about when MTRR fails and doing nothing when we didn't get
> an MTRR.
>
> @ mtrr_found @
> expression index, base, size;
> @@
>
> -index = mtrr_add(base, size, MTRR_TYPE_WRCOMB, 1);
> +index = arch_phys_wc_add(base, size);
>
> @ mtrr_rm depends on mtrr_found @
> expression mtrr_found.index, mtrr_found.base, mtrr_found.size;
> @@
>
> -mtrr_del(index, base, size);
> +arch_phys_wc_del(index);
>
> @ mtrr_rm_zero_arg depends on mtrr_found @
> expression mtrr_found.index;
> @@
>
> -mtrr_del(index, 0, 0);
> +arch_phys_wc_del(index);
>
> @ mtrr_rm_fb_info depends on mtrr_found @
> struct fb_info *info;
> expression mtrr_found.index;
> @@
>
> -mtrr_del(index, info->fix.smem_start, info->fix.smem_len);
> +arch_phys_wc_del(index);
>
> @ ioremap_replace_nocache depends on mtrr_found @
> struct fb_info *info;
> expression base, size;
> @@
>
> -info->screen_base = ioremap_nocache(base, size);
> +info->screen_base = ioremap_wc(base, size);
>
> @ ioremap_replace_default depends on mtrr_found @
> struct fb_info *info;
> expression base, size;
> @@
>
> -info->screen_base = ioremap(base, size);
> +info->screen_base = ioremap_wc(base, size);
>
> Generated-by: Coccinelle SmPL
> Cc: Suresh Siddha <suresh.b.siddha@intel.com>
> Cc: Venkatesh Pallipadi <venkatesh.pallipadi@intel.com>
> Cc: Ingo Molnar <mingo@elte.hu>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: Juergen Gross <jgross@suse.com>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Cc: Andy Lutomirski <luto@amacapital.net>
> Cc: Dave Airlie <airlied@redhat.com>
> Cc: Antonino Daplas <adaplas@gmail.com>
> Cc: Jean-Christophe Plagniol-Villard <plagnioj@jcrosoft.com>
> Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>
> Cc: linux-fbdev@vger.kernel.org
> Cc: linux-kernel@vger.kernel.org
> Signed-off-by: Luis R. Rodriguez <mcgrof@suse.com>
Hey folks, can this be considered to be merged.
Thanks,
Luis
^ permalink raw reply
* Re: [PATCH v3] staging: sm750fb: use arch_phys_wc_add() and ioremap_wc()
From: Luis R. Rodriguez @ 2015-04-30 17:38 UTC (permalink / raw)
To: arnaud.patard, aaro.koskinen, Greg Kroah-Hartman
Cc: devel@driverdev.osuosl.org, Andy Lutomirski,
cocci@systeme.lip6.fr, Luis R. Rodriguez, Sudip Mukherjee,
Teddy Wang, Suresh Siddha, Ingo Molnar, Thomas Gleixner,
Juergen Gross, Daniel Vetter, Dave Airlie, Antonino Daplas,
Jean-Christophe Plagniol-Villard, Tomi Valkeinen, linux-fbdev,
linux-kernel@vger.kernel.org
In-Reply-To: <1429647230-16870-1-git-send-email-mcgrof@do-not-panic.com>
On Tue, Apr 21, 2015 at 1:13 PM, Luis R. Rodriguez
<mcgrof@do-not-panic.com> wrote:
> From: "Luis R. Rodriguez" <mcgrof@suse.com>
>
> The same area used for ioremap() is used for the MTRR area.
> Convert the driver from using the x86 specific MTRR code to
> the architecture agnostic arch_phys_wc_add(). arch_phys_wc_add()
> will avoid MTRR if write-combining is available, in order to
> take advantage of that also ensure the ioremap'd area is requested
> as write-combining.
>
> There are a few motivations for this:
>
> a) Take advantage of PAT when available
>
> b) Help bury MTRR code away, MTRR is architecture specific and on
> x86 its replaced by PAT
>
> c) Help with the goal of eventually using _PAGE_CACHE_UC over
> _PAGE_CACHE_UC_MINUS on x86 on ioremap_nocache() (see commit
> de33c442e titled "x86 PAT: fix performance drop for glx,
> use UC minus for ioremap(), ioremap_nocache() and
> pci_mmap_page_range()")
>
> The conversion done is expressed by the following Coccinelle
> SmPL patch, it additionally required manual intervention to
> address all the #ifdery and removal of redundant things which
> arch_phys_wc_add() already addresses such as verbose message
> about when MTRR fails and doing nothing when we didn't get
> an MTRR.
>
> @ mtrr_found @
> expression index, base, size;
> @@
>
> -index = mtrr_add(base, size, MTRR_TYPE_WRCOMB, 1);
> +index = arch_phys_wc_add(base, size);
>
> @ mtrr_rm depends on mtrr_found @
> expression mtrr_found.index, mtrr_found.base, mtrr_found.size;
> @@
>
> -mtrr_del(index, base, size);
> +arch_phys_wc_del(index);
>
> @ mtrr_rm_zero_arg depends on mtrr_found @
> expression mtrr_found.index;
> @@
>
> -mtrr_del(index, 0, 0);
> +arch_phys_wc_del(index);
>
> @ mtrr_rm_fb_info depends on mtrr_found @
> struct fb_info *info;
> expression mtrr_found.index;
> @@
>
> -mtrr_del(index, info->fix.smem_start, info->fix.smem_len);
> +arch_phys_wc_del(index);
>
> @ ioremap_replace_nocache depends on mtrr_found @
> struct fb_info *info;
> expression base, size;
> @@
>
> -info->screen_base = ioremap_nocache(base, size);
> +info->screen_base = ioremap_wc(base, size);
>
> @ ioremap_replace_default depends on mtrr_found @
> struct fb_info *info;
> expression base, size;
> @@
>
> -info->screen_base = ioremap(base, size);
> +info->screen_base = ioremap_wc(base, size);
>
> Generated-by: Coccinelle SmPL
> Cc: Sudip Mukherjee <sudipm.mukherjee@gmail.com>
> Cc: Teddy Wang <teddy.wang@siliconmotion.com>
> Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Cc: Suresh Siddha <sbsiddha@gmail.com>
> Cc: Ingo Molnar <mingo@elte.hu>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: Juergen Gross <jgross@suse.com>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Cc: Andy Lutomirski <luto@amacapital.net>
> Cc: Dave Airlie <airlied@redhat.com>
> Cc: Antonino Daplas <adaplas@gmail.com>
> Cc: Jean-Christophe Plagniol-Villard <plagnioj@jcrosoft.com>
> Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>
> Cc: devel@driverdev.osuosl.org
> Cc: linux-fbdev@vger.kernel.org
> Cc: linux-kernel@vger.kernel.org
> Signed-off-by: Luis R. Rodriguez <mcgrof@suse.com>
Hey folks, just a follow up. Can this be considered to be merged?
Luis
^ permalink raw reply
* [PATCH v5 1/5] pci: add pci_iomap_wc() variants
From: Luis R. Rodriguez @ 2015-04-30 17:36 UTC (permalink / raw)
To: bhelgaas, mst, plagnioj, tomi.valkeinen, airlied, daniel.vetter
Cc: linux-fbdev, luto, cocci, linux-kernel, linux-pci,
Luis R. Rodriguez, Toshi Kani, Suresh Siddha, Ingo Molnar,
Thomas Gleixner, Juergen Gross, Daniel Vetter, Dave Airlie,
Antonino Daplas, Dave Hansen, Arnd Bergmann, venkatesh.pallipadi,
Stefan Bader, Ville Syrjälä, Mel Gorman,
Vlastimil Babka, Borislav Petkov, Davidlohr Bueso, konrad.wilk,
ville.syrjala, david.vrabel, jbeulich, Roger Pau Monné,
xen-devel
From: "Luis R. Rodriguez" <mcgrof@suse.com>
This allows drivers to take advantage of write-combining
when possible. The PCI specification does not allow for us
to automatically identify a memory region which needs
write-combining so drivers have to identify these areas
on their own. There is IORESOURCE_PREFETCH but as clarified
by Michael and confirmed later by Bjorn, PCI prefetch bit
merely means bridges can combine writes and prefetch reads.
Prefetch does not affect ordering rules and does not allow
writes to be collapsed [0]. WC is stronger, it allows collapsing
and changes ordering rules. WC can also hurt latency as small
writes are buffered. Because of all this drivers needs to
know what they are doing, we can't set a write-combining
preference flag in the pci core automatically for drivers.
Lastly although there is also arch_phys_wc_add() this makes
use of architecture specific write-combining *hacks* and
the only one currently defined and used is MTRR for x86.
MTRRs are legacy, limited in number, have restrictive size
constraints, and are known to interact pooly with the BIOS.
MTRRs should only really be considered on old video framebuffer
drivers. If we made ioremap_wc() and similar calls start
automatically adding MTRRs, then performance will vary wildly
with the order of driver loading because we'll run out of MTRRs
part-way through bootup.
There are a few motivations for phasing out of MTRR and
helping driver change over to use write-combining with PAT:
a) Take advantage of PAT when available
b) Help bury MTRR code away, MTRR is architecture specific and on
x86 its replaced by PAT
c) Help with the goal of eventually using _PAGE_CACHE_UC over
_PAGE_CACHE_UC_MINUS on x86 on ioremap_nocache() (see commit
de33c442e titled "x86 PAT: fix performance drop for glx,
use UC minus for ioremap(), ioremap_nocache() and
pci_mmap_page_range()")
[0] https://lkml.org/lkml/2015/4/21/714
Cc: Toshi Kani <toshi.kani@hp.com>
Cc: Andy Lutomirski <luto@amacapital.net>
Cc: Suresh Siddha <sbsiddha@gmail.com>
Cc: Ingo Molnar <mingo@elte.hu>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Juergen Gross <jgross@suse.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Dave Airlie <airlied@redhat.com>
Cc: Bjorn Helgaas <bhelgaas@google.com>
Cc: Antonino Daplas <adaplas@gmail.com>
Cc: Jean-Christophe Plagniol-Villard <plagnioj@jcrosoft.com>
Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>
Cc: Dave Hansen <dave.hansen@linux.intel.com>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: Michael S. Tsirkin <mst@redhat.com>
Cc: venkatesh.pallipadi@intel.com
Cc: Stefan Bader <stefan.bader@canonical.com>
Cc: Ville Syrjälä <syrjala@sci.fi>
Cc: Mel Gorman <mgorman@suse.de>
Cc: Vlastimil Babka <vbabka@suse.cz>
Cc: Borislav Petkov <bp@suse.de>
Cc: Davidlohr Bueso <dbueso@suse.de>
Cc: konrad.wilk@oracle.com
Cc: ville.syrjala@linux.intel.com
Cc: david.vrabel@citrix.com
Cc: jbeulich@suse.com
Cc: Roger Pau Monné <roger.pau@citrix.com>
Cc: linux-fbdev@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Cc: xen-devel@lists.xensource.com
Cc: linux-pci@vger.kernel.org
Signed-off-by: Luis R. Rodriguez <mcgrof@suse.com>
---
This v5 makes the code return NULL for IORESOURCE_IO and fixes the commit
log to clarify the conclusions reached for MTRR and our review of
IORESOURCE_PREFETCH.
include/asm-generic/pci_iomap.h | 14 ++++++++++
lib/pci_iomap.c | 61 +++++++++++++++++++++++++++++++++++++++++
2 files changed, 75 insertions(+)
diff --git a/include/asm-generic/pci_iomap.h b/include/asm-generic/pci_iomap.h
index 7389c87..b1e17fc 100644
--- a/include/asm-generic/pci_iomap.h
+++ b/include/asm-generic/pci_iomap.h
@@ -15,9 +15,13 @@ struct pci_dev;
#ifdef CONFIG_PCI
/* Create a virtual mapping cookie for a PCI BAR (memory or IO) */
extern void __iomem *pci_iomap(struct pci_dev *dev, int bar, unsigned long max);
+extern void __iomem *pci_iomap_wc(struct pci_dev *dev, int bar, unsigned long max);
extern void __iomem *pci_iomap_range(struct pci_dev *dev, int bar,
unsigned long offset,
unsigned long maxlen);
+extern void __iomem *pci_iomap_wc_range(struct pci_dev *dev, int bar,
+ unsigned long offset,
+ unsigned long maxlen);
/* Create a virtual mapping cookie for a port on a given PCI device.
* Do not call this directly, it exists to make it easier for architectures
* to override */
@@ -34,12 +38,22 @@ static inline void __iomem *pci_iomap(struct pci_dev *dev, int bar, unsigned lon
return NULL;
}
+static inline void __iomem *pci_iomap_wc(struct pci_dev *dev, int bar, unsigned long max)
+{
+ return NULL;
+}
static inline void __iomem *pci_iomap_range(struct pci_dev *dev, int bar,
unsigned long offset,
unsigned long maxlen)
{
return NULL;
}
+static inline void __iomem *pci_iomap_wc_range(struct pci_dev *dev, int bar,
+ unsigned long offset,
+ unsigned long maxlen)
+{
+ return NULL;
+}
#endif
#endif /* __ASM_GENERIC_IO_H */
diff --git a/lib/pci_iomap.c b/lib/pci_iomap.c
index bcce5f1..9604dcb 100644
--- a/lib/pci_iomap.c
+++ b/lib/pci_iomap.c
@@ -52,6 +52,46 @@ void __iomem *pci_iomap_range(struct pci_dev *dev,
EXPORT_SYMBOL(pci_iomap_range);
/**
+ * pci_iomap_wc_range - create a virtual WC mapping cookie for a PCI BAR
+ * @dev: PCI device that owns the BAR
+ * @bar: BAR number
+ * @offset: map memory at the given offset in BAR
+ * @maxlen: max length of the memory to map
+ *
+ * Using this function you will get a __iomem address to your device BAR.
+ * You can access it using ioread*() and iowrite*(). These functions hide
+ * the details if this is a MMIO or PIO address space and will just do what
+ * you expect from them in the correct way. When possible write combining
+ * is used.
+ *
+ * @maxlen specifies the maximum length to map. If you want to get access to
+ * the complete BAR from offset to the end, pass %0 here.
+ * */
+void __iomem *pci_iomap_wc_range(struct pci_dev *dev,
+ int bar,
+ unsigned long offset,
+ unsigned long maxlen)
+{
+ resource_size_t start = pci_resource_start(dev, bar);
+ resource_size_t len = pci_resource_len(dev, bar);
+ unsigned long flags = pci_resource_flags(dev, bar);
+
+ if (len <= offset || !start)
+ return NULL;
+ len -= offset;
+ start += offset;
+ if (maxlen && len > maxlen)
+ len = maxlen;
+ if (flags & IORESOURCE_IO)
+ return NULL;
+ if (flags & IORESOURCE_MEM)
+ return ioremap_wc(start, len);
+ /* What? */
+ return NULL;
+}
+EXPORT_SYMBOL_GPL(pci_iomap_wc_range);
+
+/**
* pci_iomap - create a virtual mapping cookie for a PCI BAR
* @dev: PCI device that owns the BAR
* @bar: BAR number
@@ -70,4 +110,25 @@ void __iomem *pci_iomap(struct pci_dev *dev, int bar, unsigned long maxlen)
return pci_iomap_range(dev, bar, 0, maxlen);
}
EXPORT_SYMBOL(pci_iomap);
+
+/**
+ * pci_iomap_wc - create a virtual WC mapping cookie for a PCI BAR
+ * @dev: PCI device that owns the BAR
+ * @bar: BAR number
+ * @maxlen: length of the memory to map
+ *
+ * Using this function you will get a __iomem address to your device BAR.
+ * You can access it using ioread*() and iowrite*(). These functions hide
+ * the details if this is a MMIO or PIO address space and will just do what
+ * you expect from them in the correct way. When possible write combining
+ * is used.
+ *
+ * @maxlen specifies the maximum length to map. If you want to get access to
+ * the complete BAR without checking for its length first, pass %0 here.
+ * */
+void __iomem *pci_iomap_wc(struct pci_dev *dev, int bar, unsigned long maxlen)
+{
+ return pci_iomap_wc_range(dev, bar, 0, maxlen);
+}
+EXPORT_SYMBOL_GPL(pci_iomap_wc);
#endif /* CONFIG_PCI */
--
2.3.2.209.gd67f9d5.dirty
^ permalink raw reply related
* Re: [PATCH v4 2/5] lib: devres: add pcim_iomap_wc() variants
From: Luis R. Rodriguez @ 2015-04-30 17:27 UTC (permalink / raw)
To: Bjorn Helgaas
Cc: Luis R. Rodriguez, mst, plagnioj, tomi.valkeinen, airlied,
daniel.vetter, linux-fbdev, luto, cocci, linux-kernel, Toshi Kani,
Suresh Siddha, Ingo Molnar, Thomas Gleixner, Juergen Gross,
Daniel Vetter, Dave Airlie, Antonino Daplas, Dave Hansen,
Arnd Bergmann, venkatesh.pallipadi, Stefan Bader,
Ville Syrjälä, Mel Gorman, Vlastimil Babka,
Borislav Petkov, Davidlohr Bueso, konrad.wilk, ville.syrjala,
david.vrabel, jbeulich, Roger Pau Monné, xen-devel,
linux-pci
In-Reply-To: <20150430162647.GD7888@google.com>
On Thu, Apr 30, 2015 at 11:26:47AM -0500, Bjorn Helgaas wrote:
> [+cc linux-pci]
>
> Hi Luis,
>
> On Wed, Apr 29, 2015 at 02:36:09PM -0700, Luis R. Rodriguez wrote:
> > From: "Luis R. Rodriguez" <mcgrof@suse.com>
> >
> > Now that we have pci_iomap_wc() add the respective devres helpers.
>
> I guess I'm still confused about the relationship between pci_iomap_wc()
> and arch_phys_wc_add().
>
> Do you expect every caller of pcim_iomap_wc() to also call
> arch_phys_wc_add()?
Yeap.
> If so, I'm not sure how pcim_iomap_wc() fits into the picture. A driver
> can call both pcim_iomap_wc() and arch_phys_wc_add(), but the driver
> doesn't explicitly do the unmap, so where would the arch_phys_wc_del()
> happen?
As with other current drivers not using devres, upon exit or where they
would otherwise typically iounmap().
> If not, how does a driver know whether it should call arch_phys_wc_add()?
Sadly they'd have to figure it out, as Andy notes arch_phys_wc_add() is
a hack so I think we need to leave it as such and hope to see arch_phys_wc_add()
use phased as it won't be needed anymore really. arch_phys_wc_add() really should
only be used by device drivers that know that are working with non-PAT systems.
The code already takes care of this but since its an x86 write-combining hack
we should not consider meshing it with devres.
> > ...
> > /**
> > + * pcim_iomap_wc_regions - Request and iomap PCI BARs with write-combining
> > + * @pdev: PCI device to map IO resources for
> > + * @mask: Mask of BARs to request and iomap
> > + * @name: Name used when requesting regions
> > + *
> > + * Request and iomap regions specified by @mask with a preference for
> > + * write-combining.
> > + */
> > +int pcim_iomap_wc_regions(struct pci_dev *pdev, int mask, const char *name)
> > +{
> > + void __iomem * const *iomap;
> > + int i, rc;
> > +
> > + iomap = pcim_iomap_table(pdev);
> > + if (!iomap)
> > + return -ENOMEM;
> > +
> > + for (i = 0; i < DEVICE_COUNT_RESOURCE; i++) {
> > + unsigned long len;
> > +
> > + if (!(mask & (1 << i)))
> > + continue;
> > +
> > + rc = -EINVAL;
> > + len = pci_resource_len(pdev, i);
> > + if (!len)
> > + goto err_inval;
> > +
> > + rc = pci_request_region(pdev, i, name);
> > + if (rc)
> > + goto err_inval;
> > +
> > + rc = -ENOMEM;
> > + if (!pcim_iomap_wc(pdev, i, 0))
> > + goto err_region;
>
> Is there a user for this? Are there really devices where *all* the BARs
> can be mapped with WC? Are there enough of them to make it worth adding
> this?
Not right now, I did this more to help with a friend who is testing one
driver for a feature. The driver is upstream but a way to make the feature
take effect only under certain conditions still would need to be done.
> I don't see users of either pcim_iomap_wc() or pcim_iomap_wc_regions() so
> far. Did I miss them, or do you just expect them in the near future?
The later, and also I hate seeing folks later add code under EXPORT_SYMBOL()
rather than EXPORT_SYMBOL_GPL() so I figure I'd rather do it first. It happened
recently in my v1 series, someone beat me to a write-combining export symbol
and changed it to EXPORT_SYMBOL(). Feel free to drop this though but I hope
no one out there then tries to just add an EXPORT_SYMBOL() later for this...
Luis
^ permalink raw reply
* Re: [PATCH v4 1/5] pci: add pci_iomap_wc() variants
From: Luis R. Rodriguez @ 2015-04-30 17:15 UTC (permalink / raw)
To: Andy Lutomirski
Cc: Bjorn Helgaas, Luis R. Rodriguez, Michael S. Tsirkin,
Jean-Christophe Plagniol-Villard, Tomi Valkeinen, David Airlie,
daniel.vetter, Linux Fbdev development list, cocci,
linux-kernel@vger.kernel.org, Toshi Kani, Suresh Siddha,
Ingo Molnar, Thomas Gleixner, Juergen Gross, Daniel Vetter,
Dave Airlie, Antonino Daplas, Dave Hansen, Arnd Bergmann,
venkatesh.pallipadi, Stefan Bader, Ville Syrjälä,
Mel Gorman, Vlastimil Babka, Borislav Petkov, Davidlohr Bueso,
Konrad Rzeszutek Wilk, ville.syrjala, David Vrabel, Jan Beulich,
Roger Pau Monné, xen-devel, linux-pci@vger.kernel.org
In-Reply-To: <CALCETrXWBWq-az=AMgrV5t5BHwmPd-NHY31vC7eEsxpmrhhRQQ@mail.gmail.com>
On Thu, Apr 30, 2015 at 10:03:18AM -0700, Andy Lutomirski wrote:
> On Thu, Apr 30, 2015 at 9:52 AM, Luis R. Rodriguez <mcgrof@suse.com> wrote:
> > On Thu, Apr 30, 2015 at 10:59:17AM -0500, Bjorn Helgaas wrote:
> >> [+cc linux-pci]
> >>
> >> Hi Luis,
> >>
> >> On Wed, Apr 29, 2015 at 02:36:08PM -0700, Luis R. Rodriguez wrote:
> >> > From: "Luis R. Rodriguez" <mcgrof@suse.com>
> >> >
> >> > This allows drivers to take advantage of write-combining
> >> > when possible. Ideally we'd have pci_read_bases() just
> >> > peg an IORESOURCE_WC flag for us
> >>
> >> This makes it sound like pci_read_bases() could do a better job
> >> if we just tried harder, but I don't think that's the case. All
> >> pci_read_bases() can do is look at the bits in the BAR. For
> >> memory BARs, there's a "prefetchable" bit and a "64-bit" bit.
> >>
> >> If you just want to complain that the PCI spec didn't define a
> >> way for software to discover whether a BAR can be mapped with WC,
> >> that's fine, but it's misleading to suggest that pci_read_bases()
> >> could figure out WC without some help from the spec.
> >
> > You're right sorry about that, in my original patch this was more
> > of a question and I did not have a full answer for but mst had
> > clarified before the spec doesn't allow for this [0] and you are
> > confirming this now as well.
> >
> > [0] https://lkml.org/lkml/2015/4/21/714
> >
> > I'll update the patch and at least document we did think about
> > this and that its a shortcoming of the spec.
> >
> >> > but where exactly
> >> > video devices memory lie varies *largely* and at times things
> >> > are mixed with MMIO registers, sometimes we can address
> >> > the changes in drivers, other times the change requires
> >> > intrusive changes.
> >> >
> >> > Although there is also arch_phys_wc_add() that makes use of
> >> > architecture specific write-combining alternatives (MTRR on
> >> > x86 when a system does not have PAT) we void polluting
> >> > pci_iomap() space with it and force drivers and subsystems
> >> > that want to use it to be explicit.
> >>
> >> I'm not quite sure I understand the point you're making here
> >> about not polluting pci_iomap_wc() with arch_phys_wc_add(). I
> >> think the choice is for a driver to do either this:
> >>
> >> info->screen_base = pci_iomap_wc(dev, 0, 0);
> >>
> >> or this:
> >>
> >> info->screen_base = pci_iomap_wc(dev, 0, 0);
> >> par->wc_cookie = arch_phys_wc_add(pci_resource_start(dev, 0),
> >> pci_resource_len(dev, 0));
> >>
> >> The driver is *already* being explicit because it calls
> >> pci_iomap_wc() instead of pci_iomap().
> >>
> >> It seems like it would be ideal if ioremap_wc() could call
> >> arch_phys_wc_add() internally.
> >
> > Indeed, that's what I was alluding to.
> >
> >> Doesn't any caller of
> >> arch_phys_wc_add() have to also do some sort of ioremap()
> >> beforehand?
> >
> > This is not a requirement as the physical address is used,
> > not the virtual address.
> >
> >> I assume there's some reason for separating them,
> >
> > Well a full sweep to change to arch_phys_wc_add() was never done,
> > consider this part of the last effort to do so. In retrospect now
> > that I've covered all other drivers in 12 different series of patches
> > I think its perhaps best to not mesh them together as we're phasing
> > out MTRR and the only reason to have arch_phys_wc_add() is for MTRR
> > which is legacy.
>
> I would say it much more strongly.
>
> Drivers for new hardware SHOULD NOT call arch_phys_wc_add, directly or
> otherwise. MTRRs are crap. They have nasty alignment requirements,
> they are a very limited and unpredictable resource, and the interact
> poorly with BIOS. They should really only be used for old video
> framebuffers and such.
>
> Anything new should use PAT (it's been available for a long time) and
> possibly streaming memory writes. Even fancy server gear (myri10ge,
> for example) should stay far away from MTRRs and such: it's very easy
> to put enough devices in a server board that you simply run out of
> MTRRs and arch_phys_wc_add will stop working.
>
> If we make ioremap_wc and similar start automatically adding MTRRs,
> then performance will vary wildly with the order of driver loading,
> because we'll run out of MTRRs part-way through bootup.
>
> ioremap_wc via PAT, on the other hand, is 100% reliable on newer hardware.
>
> Maybe I should have called it arch_phys_wc_add_awful_legacy_hack.
Thanks, I'll document such technicalities as well ;)
Luis
^ permalink raw reply
* Re: [PATCH v4 1/5] pci: add pci_iomap_wc() variants
From: Andy Lutomirski @ 2015-04-30 17:03 UTC (permalink / raw)
To: Luis R. Rodriguez
Cc: Bjorn Helgaas, Luis R. Rodriguez, Michael S. Tsirkin,
Jean-Christophe Plagniol-Villard, Tomi Valkeinen, David Airlie,
daniel.vetter, Linux Fbdev development list, cocci,
linux-kernel@vger.kernel.org, Toshi Kani, Suresh Siddha,
Ingo Molnar, Thomas Gleixner, Juergen Gross, Daniel Vetter,
Dave Airlie, Antonino Daplas, Dave Hansen, Arnd Bergmann,
venkatesh.pallipadi, Stefan Bader, Ville Syrjälä,
Mel Gorman, Vlastimil Babka, Borislav Petkov, Davidlohr Bueso,
Konrad Rzeszutek Wilk, ville.syrjala, David Vrabel, Jan Beulich,
Roger Pau Monné, xen-devel, linux-pci@vger.kernel.org
In-Reply-To: <20150430165238.GS5622@wotan.suse.de>
On Thu, Apr 30, 2015 at 9:52 AM, Luis R. Rodriguez <mcgrof@suse.com> wrote:
> On Thu, Apr 30, 2015 at 10:59:17AM -0500, Bjorn Helgaas wrote:
>> [+cc linux-pci]
>>
>> Hi Luis,
>>
>> On Wed, Apr 29, 2015 at 02:36:08PM -0700, Luis R. Rodriguez wrote:
>> > From: "Luis R. Rodriguez" <mcgrof@suse.com>
>> >
>> > This allows drivers to take advantage of write-combining
>> > when possible. Ideally we'd have pci_read_bases() just
>> > peg an IORESOURCE_WC flag for us
>>
>> This makes it sound like pci_read_bases() could do a better job
>> if we just tried harder, but I don't think that's the case. All
>> pci_read_bases() can do is look at the bits in the BAR. For
>> memory BARs, there's a "prefetchable" bit and a "64-bit" bit.
>>
>> If you just want to complain that the PCI spec didn't define a
>> way for software to discover whether a BAR can be mapped with WC,
>> that's fine, but it's misleading to suggest that pci_read_bases()
>> could figure out WC without some help from the spec.
>
> You're right sorry about that, in my original patch this was more
> of a question and I did not have a full answer for but mst had
> clarified before the spec doesn't allow for this [0] and you are
> confirming this now as well.
>
> [0] https://lkml.org/lkml/2015/4/21/714
>
> I'll update the patch and at least document we did think about
> this and that its a shortcoming of the spec.
>
>> > but where exactly
>> > video devices memory lie varies *largely* and at times things
>> > are mixed with MMIO registers, sometimes we can address
>> > the changes in drivers, other times the change requires
>> > intrusive changes.
>> >
>> > Although there is also arch_phys_wc_add() that makes use of
>> > architecture specific write-combining alternatives (MTRR on
>> > x86 when a system does not have PAT) we void polluting
>> > pci_iomap() space with it and force drivers and subsystems
>> > that want to use it to be explicit.
>>
>> I'm not quite sure I understand the point you're making here
>> about not polluting pci_iomap_wc() with arch_phys_wc_add(). I
>> think the choice is for a driver to do either this:
>>
>> info->screen_base = pci_iomap_wc(dev, 0, 0);
>>
>> or this:
>>
>> info->screen_base = pci_iomap_wc(dev, 0, 0);
>> par->wc_cookie = arch_phys_wc_add(pci_resource_start(dev, 0),
>> pci_resource_len(dev, 0));
>>
>> The driver is *already* being explicit because it calls
>> pci_iomap_wc() instead of pci_iomap().
>>
>> It seems like it would be ideal if ioremap_wc() could call
>> arch_phys_wc_add() internally.
>
> Indeed, that's what I was alluding to.
>
>> Doesn't any caller of
>> arch_phys_wc_add() have to also do some sort of ioremap()
>> beforehand?
>
> This is not a requirement as the physical address is used,
> not the virtual address.
>
>> I assume there's some reason for separating them,
>
> Well a full sweep to change to arch_phys_wc_add() was never done,
> consider this part of the last effort to do so. In retrospect now
> that I've covered all other drivers in 12 different series of patches
> I think its perhaps best to not mesh them together as we're phasing
> out MTRR and the only reason to have arch_phys_wc_add() is for MTRR
> which is legacy.
I would say it much more strongly.
Drivers for new hardware SHOULD NOT call arch_phys_wc_add, directly or
otherwise. MTRRs are crap. They have nasty alignment requirements,
they are a very limited and unpredictable resource, and the interact
poorly with BIOS. They should really only be used for old video
framebuffers and such.
Anything new should use PAT (it's been available for a long time) and
possibly streaming memory writes. Even fancy server gear (myri10ge,
for example) should stay far away from MTRRs and such: it's very easy
to put enough devices in a server board that you simply run out of
MTRRs and arch_phys_wc_add will stop working.
If we make ioremap_wc and similar start automatically adding MTRRs,
then performance will vary wildly with the order of driver loading,
because we'll run out of MTRRs part-way through bootup.
ioremap_wc via PAT, on the other hand, is 100% reliable on newer hardware.
Maybe I should have called it arch_phys_wc_add_awful_legacy_hack.
--Andy
^ permalink raw reply
* Re: [PATCH v4 1/5] pci: add pci_iomap_wc() variants
From: Luis R. Rodriguez @ 2015-04-30 16:52 UTC (permalink / raw)
To: Bjorn Helgaas
Cc: Luis R. Rodriguez, mst, plagnioj, tomi.valkeinen, airlied,
daniel.vetter, linux-fbdev, luto, cocci, linux-kernel, Toshi Kani,
Suresh Siddha, Ingo Molnar, Thomas Gleixner, Juergen Gross,
Daniel Vetter, Dave Airlie, Antonino Daplas, Dave Hansen,
Arnd Bergmann, venkatesh.pallipadi, Stefan Bader,
Ville Syrjälä, Mel Gorman, Vlastimil Babka,
Borislav Petkov, Davidlohr Bueso, konrad.wilk, ville.syrjala,
david.vrabel, jbeulich, Roger Pau Monné, xen-devel,
linux-pci
In-Reply-To: <20150430155917.GC7888@google.com>
On Thu, Apr 30, 2015 at 10:59:17AM -0500, Bjorn Helgaas wrote:
> [+cc linux-pci]
>
> Hi Luis,
>
> On Wed, Apr 29, 2015 at 02:36:08PM -0700, Luis R. Rodriguez wrote:
> > From: "Luis R. Rodriguez" <mcgrof@suse.com>
> >
> > This allows drivers to take advantage of write-combining
> > when possible. Ideally we'd have pci_read_bases() just
> > peg an IORESOURCE_WC flag for us
>
> This makes it sound like pci_read_bases() could do a better job
> if we just tried harder, but I don't think that's the case. All
> pci_read_bases() can do is look at the bits in the BAR. For
> memory BARs, there's a "prefetchable" bit and a "64-bit" bit.
>
> If you just want to complain that the PCI spec didn't define a
> way for software to discover whether a BAR can be mapped with WC,
> that's fine, but it's misleading to suggest that pci_read_bases()
> could figure out WC without some help from the spec.
You're right sorry about that, in my original patch this was more
of a question and I did not have a full answer for but mst had
clarified before the spec doesn't allow for this [0] and you are
confirming this now as well.
[0] https://lkml.org/lkml/2015/4/21/714
I'll update the patch and at least document we did think about
this and that its a shortcoming of the spec.
> > but where exactly
> > video devices memory lie varies *largely* and at times things
> > are mixed with MMIO registers, sometimes we can address
> > the changes in drivers, other times the change requires
> > intrusive changes.
> >
> > Although there is also arch_phys_wc_add() that makes use of
> > architecture specific write-combining alternatives (MTRR on
> > x86 when a system does not have PAT) we void polluting
> > pci_iomap() space with it and force drivers and subsystems
> > that want to use it to be explicit.
>
> I'm not quite sure I understand the point you're making here
> about not polluting pci_iomap_wc() with arch_phys_wc_add(). I
> think the choice is for a driver to do either this:
>
> info->screen_base = pci_iomap_wc(dev, 0, 0);
>
> or this:
>
> info->screen_base = pci_iomap_wc(dev, 0, 0);
> par->wc_cookie = arch_phys_wc_add(pci_resource_start(dev, 0),
> pci_resource_len(dev, 0));
>
> The driver is *already* being explicit because it calls
> pci_iomap_wc() instead of pci_iomap().
>
> It seems like it would be ideal if ioremap_wc() could call
> arch_phys_wc_add() internally.
Indeed, that's what I was alluding to.
> Doesn't any caller of
> arch_phys_wc_add() have to also do some sort of ioremap()
> beforehand?
This is not a requirement as the physical address is used,
not the virtual address.
> I assume there's some reason for separating them,
Well a full sweep to change to arch_phys_wc_add() was never done,
consider this part of the last effort to do so. In retrospect now
that I've covered all other drivers in 12 different series of patches
I think its perhaps best to not mesh them together as we're phasing
out MTRR and the only reason to have arch_phys_wc_add() is for MTRR
which is legacy.
I'll update the commit log to mention that.
> and I see that the current arch_phys_wc_add() requires the caller
> to store a handle, but doing both seems confusing.
That's just a cookie so that later when we undo the driver we can
tell the backend to remove it.
> > There are a few motivations for this:
> >
> > a) Take advantage of PAT when available
> >
> > b) Help bury MTRR code away, MTRR is architecture specific and on
> > x86 its replaced by PAT
> >
> > c) Help with the goal of eventually using _PAGE_CACHE_UC over
> > _PAGE_CACHE_UC_MINUS on x86 on ioremap_nocache() (see commit
> > de33c442e titled "x86 PAT: fix performance drop for glx,
> > use UC minus for ioremap(), ioremap_nocache() and
> > pci_mmap_page_range()")
>
> I think these are now _PAGE_CACHE_MODE_UC and
> _PAGE_CACHE_MODE_UC_MINUS, right?
Indeed, thanks, I'll fix that in the commit log.
> > ...
>
> > +void __iomem *pci_iomap_wc_range(struct pci_dev *dev,
> > + int bar,
> > + unsigned long offset,
> > + unsigned long maxlen)
> > +{
> > + resource_size_t start = pci_resource_start(dev, bar);
> > + resource_size_t len = pci_resource_len(dev, bar);
> > + unsigned long flags = pci_resource_flags(dev, bar);
> > +
> > + if (len <= offset || !start)
> > + return NULL;
> > + len -= offset;
> > + start += offset;
> > + if (maxlen && len > maxlen)
> > + len = maxlen;
> > + if (flags & IORESOURCE_IO)
> > + return __pci_ioport_map(dev, start, len);
>
> Is there any point in checking for IORESOURCE_IO? If a driver
> calls pci_iomap_wc_range(), I assume it already knows this is an
> IORESOURCE_MEM BAR, so if we see IORESOURCE_IO here we should
> just return an error, i.e., NULL.
Agreed, will fix with all the other changes on the commit log and
repost. I won't repost the full series but just this one patch as
a v5.
Thanks for the review.
Luis
^ permalink raw reply
* Re: [RFT PATCH] drm/exynos: Enable DP clock to fix display on Exynos5250 and other
From: Olof Johansson @ 2015-04-30 16:50 UTC (permalink / raw)
To: Javier Martinez Canillas
Cc: Krzysztof Kozlowski, linux-samsung-soc@vger.kernel.org,
Kevin Hilman, Jingoo Han, Seung-Woo Kim,
linux-fbdev@vger.kernel.org, DRI mailing list,
linux-kernel@vger.kernel.org, Andrzej Hajda, Kyungmin Park,
Kukjin Kim, stable, linux-arm-kernel@lists.infradead.org,
Marek Szyprowski
In-Reply-To: <55425AFB.8020904@collabora.co.uk>
On Thu, Apr 30, 2015 at 9:40 AM, Javier Martinez Canillas
<javier.martinez@collabora.co.uk> wrote:
> Hello Olof,
>
> On 04/30/2015 05:57 PM, Olof Johansson wrote:
>> On Thu, Apr 30, 2015 at 8:44 AM, Kevin Hilman <khilman@kernel.org> wrote:
>>> Krzysztof Kozlowski <k.kozlowski@samsung.com> writes:
>>>>>>
>>>>>> This should fix issue reported by Javier [1][2].
>>>>>>
>>>>>> Tested on Chromebook Snow (Exynos 5250). More testing would be great,
>>>>>> especially on other Exynos 5xxx products.
>>>>>
>>>>> I hoped to try this on my exynos5 boards, but it doesn't seem to apply
>>>>> to linux-next or to Linus' master branch.
>>>>>
>>>>> Are there some other dependencies here?
>>>>
>>>> It is already applied:
>>>> https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id\x1c363c7cccf64128087002b0779986ad16aff6dc
>>>
>>> Er, yup. That would explain it. ;)
>>>
>>> Sorry for the noise,
>>
>> Well, noise or not, Exynos is still broken in mainline and was broken
>> on -next for so long in different ways that bisecting it is a futile
>> exercise in frustration.
>>
>> It doesn't seem to show up with a trivial boot using only ramdisk, but
>> when booting a real distro from disk, it certainly does.
>>
>> For example:
>>
>> http://arm-soc.lixom.net/bootlogs/mainline/v4.1-rc1-56-g3d99e3f/pi-arm-exynos_defconfig.html
>>
>> Disabling CONFIG_DRM makes it boot reliably.
>>
>> Arndale doesn't show it for me, but it also doesn't have working graphics.
>>
>
> Both Exynos5250 and Exynos5420 had similar issues and $subject is the fix
> for 5250 while 5420 is fixed by my "ARM: dts: Make DP a consumer of DISP1
> power domain on Exynos5420" patch that was posted a long time ago. I have
> pinged Kukjin several times about this patch and he said that he will pick
> it this weekend [0].
>
> It is indeed very frustrating how many Exynos patches seems to be falling
> through the crack, even important fixes like these ones that allow boards
> to boot again.
>
> Kevin suggested that Krzysztof could collect and queue patches [1] to help
> Kukjin and start acting as a co-maintatiner which I think it's a very good
> idea and Krzysztof already did for some patches during the 4.1 cycle.
Yes, that's a much needed improvement. I suggest starting out by
collecting critical fixes like these, and we'd be happy to merge them
directly if going through Kukjin will add latency.
Krzysztof, if you can, make sure you get a PGP key setup and start
collecting signatures from kernel developers.
-Olof
^ permalink raw reply
* Re: [RFT PATCH] drm/exynos: Enable DP clock to fix display on Exynos5250 and other
From: Javier Martinez Canillas @ 2015-04-30 16:40 UTC (permalink / raw)
To: Olof Johansson, Kevin Hilman
Cc: Krzysztof Kozlowski, linux-fbdev@vger.kernel.org,
linux-samsung-soc@vger.kernel.org, Joonyoung Shim, David Airlie,
Jingoo Han, Seung-Woo Kim, linux-kernel@vger.kernel.org,
DRI mailing list, Inki Dae, Andrzej Hajda, Kyungmin Park,
Kukjin Kim, stable, linux-arm-kernel@lists.infradead.org,
Marek Szyprowski
In-Reply-To: <CAOesGMiHbcVK+BjiidPphGuoETyUroXtKgKqcVEgdSBiZQ+GmQ@mail.gmail.com>
Hello Olof,
On 04/30/2015 05:57 PM, Olof Johansson wrote:
> On Thu, Apr 30, 2015 at 8:44 AM, Kevin Hilman <khilman@kernel.org> wrote:
>> Krzysztof Kozlowski <k.kozlowski@samsung.com> writes:
>>>>>
>>>>> This should fix issue reported by Javier [1][2].
>>>>>
>>>>> Tested on Chromebook Snow (Exynos 5250). More testing would be great,
>>>>> especially on other Exynos 5xxx products.
>>>>
>>>> I hoped to try this on my exynos5 boards, but it doesn't seem to apply
>>>> to linux-next or to Linus' master branch.
>>>>
>>>> Are there some other dependencies here?
>>>
>>> It is already applied:
>>> https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id\x1c363c7cccf64128087002b0779986ad16aff6dc
>>
>> Er, yup. That would explain it. ;)
>>
>> Sorry for the noise,
>
> Well, noise or not, Exynos is still broken in mainline and was broken
> on -next for so long in different ways that bisecting it is a futile
> exercise in frustration.
>
> It doesn't seem to show up with a trivial boot using only ramdisk, but
> when booting a real distro from disk, it certainly does.
>
> For example:
>
> http://arm-soc.lixom.net/bootlogs/mainline/v4.1-rc1-56-g3d99e3f/pi-arm-exynos_defconfig.html
>
> Disabling CONFIG_DRM makes it boot reliably.
>
> Arndale doesn't show it for me, but it also doesn't have working graphics.
>
Both Exynos5250 and Exynos5420 had similar issues and $subject is the fix
for 5250 while 5420 is fixed by my "ARM: dts: Make DP a consumer of DISP1
power domain on Exynos5420" patch that was posted a long time ago. I have
pinged Kukjin several times about this patch and he said that he will pick
it this weekend [0].
It is indeed very frustrating how many Exynos patches seems to be falling
through the crack, even important fixes like these ones that allow boards
to boot again.
Kevin suggested that Krzysztof could collect and queue patches [1] to help
Kukjin and start acting as a co-maintatiner which I think it's a very good
idea and Krzysztof already did for some patches during the 4.1 cycle.
>
> -Olof
>
Best regards,
Javier
[0]: https://lkml.org/lkml/2015/4/29/781
[1]: https://lkml.org/lkml/2015/4/30/576
[2]: https://lkml.org/lkml/2015/3/11/403
^ permalink raw reply
* Re: [PATCH v4 2/5] lib: devres: add pcim_iomap_wc() variants
From: Bjorn Helgaas @ 2015-04-30 16:26 UTC (permalink / raw)
To: Luis R. Rodriguez
Cc: mst, plagnioj, tomi.valkeinen, airlied, daniel.vetter,
linux-fbdev, luto, cocci, linux-kernel, Luis R. Rodriguez,
Toshi Kani, Suresh Siddha, Ingo Molnar, Thomas Gleixner,
Juergen Gross, Daniel Vetter, Dave Airlie, Antonino Daplas,
Dave Hansen, Arnd Bergmann, venkatesh.pallipadi, Stefan Bader,
Ville Syrjälä, Mel Gorman, Vlastimil Babka,
Borislav Petkov, Davidlohr Bueso, konrad.wilk, ville.syrjala,
david.vrabel, jbeulich, Roger Pau Monné, xen-devel,
linux-pci
In-Reply-To: <1430343372-687-3-git-send-email-mcgrof@do-not-panic.com>
[+cc linux-pci]
Hi Luis,
On Wed, Apr 29, 2015 at 02:36:09PM -0700, Luis R. Rodriguez wrote:
> From: "Luis R. Rodriguez" <mcgrof@suse.com>
>
> Now that we have pci_iomap_wc() add the respective devres helpers.
I guess I'm still confused about the relationship between pci_iomap_wc()
and arch_phys_wc_add().
Do you expect every caller of pcim_iomap_wc() to also call
arch_phys_wc_add()?
If so, I'm not sure how pcim_iomap_wc() fits into the picture. A driver
can call both pcim_iomap_wc() and arch_phys_wc_add(), but the driver
doesn't explicitly do the unmap, so where would the arch_phys_wc_del()
happen?
If not, how does a driver know whether it should call arch_phys_wc_add()?
> ...
> /**
> + * pcim_iomap_wc_regions - Request and iomap PCI BARs with write-combining
> + * @pdev: PCI device to map IO resources for
> + * @mask: Mask of BARs to request and iomap
> + * @name: Name used when requesting regions
> + *
> + * Request and iomap regions specified by @mask with a preference for
> + * write-combining.
> + */
> +int pcim_iomap_wc_regions(struct pci_dev *pdev, int mask, const char *name)
> +{
> + void __iomem * const *iomap;
> + int i, rc;
> +
> + iomap = pcim_iomap_table(pdev);
> + if (!iomap)
> + return -ENOMEM;
> +
> + for (i = 0; i < DEVICE_COUNT_RESOURCE; i++) {
> + unsigned long len;
> +
> + if (!(mask & (1 << i)))
> + continue;
> +
> + rc = -EINVAL;
> + len = pci_resource_len(pdev, i);
> + if (!len)
> + goto err_inval;
> +
> + rc = pci_request_region(pdev, i, name);
> + if (rc)
> + goto err_inval;
> +
> + rc = -ENOMEM;
> + if (!pcim_iomap_wc(pdev, i, 0))
> + goto err_region;
Is there a user for this? Are there really devices where *all* the BARs
can be mapped with WC? Are there enough of them to make it worth adding
this?
I don't see users of either pcim_iomap_wc() or pcim_iomap_wc_regions() so
far. Did I miss them, or do you just expect them in the near future?
Bjorn
^ permalink raw reply
* Re: [PATCH v4 1/5] pci: add pci_iomap_wc() variants
From: Bjorn Helgaas @ 2015-04-30 15:59 UTC (permalink / raw)
To: Luis R. Rodriguez
Cc: mst, plagnioj, tomi.valkeinen, airlied, daniel.vetter,
linux-fbdev, luto, cocci, linux-kernel, Luis R. Rodriguez,
Toshi Kani, Suresh Siddha, Ingo Molnar, Thomas Gleixner,
Juergen Gross, Daniel Vetter, Dave Airlie, Antonino Daplas,
Dave Hansen, Arnd Bergmann, venkatesh.pallipadi, Stefan Bader,
Ville Syrjälä, Mel Gorman, Vlastimil Babka,
Borislav Petkov, Davidlohr Bueso, konrad.wilk, ville.syrjala,
david.vrabel, jbeulich, Roger Pau Monné, xen-devel,
linux-pci
In-Reply-To: <1430343372-687-2-git-send-email-mcgrof@do-not-panic.com>
[+cc linux-pci]
Hi Luis,
On Wed, Apr 29, 2015 at 02:36:08PM -0700, Luis R. Rodriguez wrote:
> From: "Luis R. Rodriguez" <mcgrof@suse.com>
>
> This allows drivers to take advantage of write-combining
> when possible. Ideally we'd have pci_read_bases() just
> peg an IORESOURCE_WC flag for us
This makes it sound like pci_read_bases() could do a better job
if we just tried harder, but I don't think that's the case. All
pci_read_bases() can do is look at the bits in the BAR. For
memory BARs, there's a "prefetchable" bit and a "64-bit" bit.
If you just want to complain that the PCI spec didn't define a
way for software to discover whether a BAR can be mapped with WC,
that's fine, but it's misleading to suggest that pci_read_bases()
could figure out WC without some help from the spec.
> but where exactly
> video devices memory lie varies *largely* and at times things
> are mixed with MMIO registers, sometimes we can address
> the changes in drivers, other times the change requires
> intrusive changes.
>
> Although there is also arch_phys_wc_add() that makes use of
> architecture specific write-combining alternatives (MTRR on
> x86 when a system does not have PAT) we void polluting
> pci_iomap() space with it and force drivers and subsystems
> that want to use it to be explicit.
I'm not quite sure I understand the point you're making here
about not polluting pci_iomap_wc() with arch_phys_wc_add(). I
think the choice is for a driver to do either this:
info->screen_base = pci_iomap_wc(dev, 0, 0);
or this:
info->screen_base = pci_iomap_wc(dev, 0, 0);
par->wc_cookie = arch_phys_wc_add(pci_resource_start(dev, 0),
pci_resource_len(dev, 0));
The driver is *already* being explicit because it calls
pci_iomap_wc() instead of pci_iomap().
It seems like it would be ideal if ioremap_wc() could call
arch_phys_wc_add() internally. Doesn't any caller of
arch_phys_wc_add() have to also do some sort of ioremap()
beforehand? I assume there's some reason for separating them,
and I see that the current arch_phys_wc_add() requires the caller
to store a handle, but doing both seems confusing.
> There are a few motivations for this:
>
> a) Take advantage of PAT when available
>
> b) Help bury MTRR code away, MTRR is architecture specific and on
> x86 its replaced by PAT
>
> c) Help with the goal of eventually using _PAGE_CACHE_UC over
> _PAGE_CACHE_UC_MINUS on x86 on ioremap_nocache() (see commit
> de33c442e titled "x86 PAT: fix performance drop for glx,
> use UC minus for ioremap(), ioremap_nocache() and
> pci_mmap_page_range()")
I think these are now _PAGE_CACHE_MODE_UC and
_PAGE_CACHE_MODE_UC_MINUS, right?
> ...
> +void __iomem *pci_iomap_wc_range(struct pci_dev *dev,
> + int bar,
> + unsigned long offset,
> + unsigned long maxlen)
> +{
> + resource_size_t start = pci_resource_start(dev, bar);
> + resource_size_t len = pci_resource_len(dev, bar);
> + unsigned long flags = pci_resource_flags(dev, bar);
> +
> + if (len <= offset || !start)
> + return NULL;
> + len -= offset;
> + start += offset;
> + if (maxlen && len > maxlen)
> + len = maxlen;
> + if (flags & IORESOURCE_IO)
> + return __pci_ioport_map(dev, start, len);
Is there any point in checking for IORESOURCE_IO? If a driver
calls pci_iomap_wc_range(), I assume it already knows this is an
IORESOURCE_MEM BAR, so if we see IORESOURCE_IO here we should
just return an error, i.e., NULL.
> + if (flags & IORESOURCE_MEM)
> + return ioremap_wc(start, len);
> + /* What? */
> + return NULL;
> +}
> +EXPORT_SYMBOL_GPL(pci_iomap_wc_range);
Bjorn
^ permalink raw reply
* Re: [RFT PATCH] drm/exynos: Enable DP clock to fix display on Exynos5250 and other
From: Olof Johansson @ 2015-04-30 15:57 UTC (permalink / raw)
To: Kevin Hilman
Cc: Krzysztof Kozlowski, linux-fbdev@vger.kernel.org,
linux-samsung-soc@vger.kernel.org, Joonyoung Shim, David Airlie,
Jingoo Han, Seung-Woo Kim, linux-kernel@vger.kernel.org,
DRI mailing list, Inki Dae, Andrzej Hajda, Kyungmin Park,
Kukjin Kim, stable, Javier Martinez Canillas,
linux-arm-kernel@lists.infradead.org, Marek Szyprowski
In-Reply-To: <7h8ud9pqmm.fsf@deeprootsystems.com>
On Thu, Apr 30, 2015 at 8:44 AM, Kevin Hilman <khilman@kernel.org> wrote:
> Krzysztof Kozlowski <k.kozlowski@samsung.com> writes:
>
>> 2015-04-30 2:31 GMT+09:00 Kevin Hilman <khilman@kernel.org>:
>>> Krzysztof Kozlowski <k.kozlowski@samsung.com> writes:
>>>
>>>> After adding display power domain for Exynos5250 in commit
>>>> 2d2c9a8d0a4f ("ARM: dts: add display power domain for exynos5250") the
>>>> display on Chromebook Snow and others stopped working after boot.
>>>>
>>>> The reason for this suggested Andrzej Hajda: the DP clock was disabled.
>>>> This clock is required by Display Port and is enabled by bootloader.
>>>> However when FIMD driver probing was deferred, the display power domain
>>>> was turned off. This effectively reset the value of DP clock enable
>>>> register.
>>>>
>>>> When exynos-dp is later probed, the clock is not enabled and display is
>>>> not properly configured:
>>>>
>>>> exynos-dp 145b0000.dp-controller: Timeout of video streamclk ok
>>>> exynos-dp 145b0000.dp-controller: unable to config video
>>>>
>>>> Signed-off-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
>>>> Reported-by: Javier Martinez Canillas <javier.martinez@collabora.co.uk>
>>>> Fixes: 2d2c9a8d0a4f ("ARM: dts: add display power domain for exynos5250")
>>>> Cc: <stable@vger.kernel.org>
>>>>
>>>> ---
>>>>
>>>> This should fix issue reported by Javier [1][2].
>>>>
>>>> Tested on Chromebook Snow (Exynos 5250). More testing would be great,
>>>> especially on other Exynos 5xxx products.
>>>
>>> I hoped to try this on my exynos5 boards, but it doesn't seem to apply
>>> to linux-next or to Linus' master branch.
>>>
>>> Are there some other dependencies here?
>>
>> It is already applied:
>> https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id\x1c363c7cccf64128087002b0779986ad16aff6dc
>
> Er, yup. That would explain it. ;)
>
> Sorry for the noise,
Well, noise or not, Exynos is still broken in mainline and was broken
on -next for so long in different ways that bisecting it is a futile
exercise in frustration.
It doesn't seem to show up with a trivial boot using only ramdisk, but
when booting a real distro from disk, it certainly does.
For example:
http://arm-soc.lixom.net/bootlogs/mainline/v4.1-rc1-56-g3d99e3f/pi-arm-exynos_defconfig.html
Disabling CONFIG_DRM makes it boot reliably.
Arndale doesn't show it for me, but it also doesn't have working graphics.
-Olof
^ permalink raw reply
* Re: [RFT PATCH] drm/exynos: Enable DP clock to fix display on Exynos5250 and other
From: Kevin Hilman @ 2015-04-30 15:44 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: linux-fbdev, linux-samsung-soc, Joonyoung Shim, David Airlie,
Jingoo Han, Seung-Woo Kim, linux-kernel, dri-devel, Inki Dae,
Andrzej Hajda, Kyungmin Park, Kukjin Kim, stable, Olof Johansson,
Javier Martinez Canillas, linux-arm-kernel, Marek Szyprowski
In-Reply-To: <CAJKOXPc6Epy_L+djManggcaYLaOrQ_c2S0=_GgU=dikuY1UU0g@mail.gmail.com>
Krzysztof Kozlowski <k.kozlowski@samsung.com> writes:
> 2015-04-30 2:31 GMT+09:00 Kevin Hilman <khilman@kernel.org>:
>> Krzysztof Kozlowski <k.kozlowski@samsung.com> writes:
>>
>>> After adding display power domain for Exynos5250 in commit
>>> 2d2c9a8d0a4f ("ARM: dts: add display power domain for exynos5250") the
>>> display on Chromebook Snow and others stopped working after boot.
>>>
>>> The reason for this suggested Andrzej Hajda: the DP clock was disabled.
>>> This clock is required by Display Port and is enabled by bootloader.
>>> However when FIMD driver probing was deferred, the display power domain
>>> was turned off. This effectively reset the value of DP clock enable
>>> register.
>>>
>>> When exynos-dp is later probed, the clock is not enabled and display is
>>> not properly configured:
>>>
>>> exynos-dp 145b0000.dp-controller: Timeout of video streamclk ok
>>> exynos-dp 145b0000.dp-controller: unable to config video
>>>
>>> Signed-off-by: Krzysztof Kozlowski <k.kozlowski@samsung.com>
>>> Reported-by: Javier Martinez Canillas <javier.martinez@collabora.co.uk>
>>> Fixes: 2d2c9a8d0a4f ("ARM: dts: add display power domain for exynos5250")
>>> Cc: <stable@vger.kernel.org>
>>>
>>> ---
>>>
>>> This should fix issue reported by Javier [1][2].
>>>
>>> Tested on Chromebook Snow (Exynos 5250). More testing would be great,
>>> especially on other Exynos 5xxx products.
>>
>> I hoped to try this on my exynos5 boards, but it doesn't seem to apply
>> to linux-next or to Linus' master branch.
>>
>> Are there some other dependencies here?
>
> It is already applied:
> https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id\x1c363c7cccf64128087002b0779986ad16aff6dc
Er, yup. That would explain it. ;)
Sorry for the noise,
Kevin
^ permalink raw reply
* Re: [PATCH v4 1/6] x86: add ioremap_uc() - force strong UC, PCD=1, PWT=1
From: Borislav Petkov @ 2015-04-30 10:18 UTC (permalink / raw)
To: Luis R. Rodriguez
Cc: mingo, tglx, hpa, bp, plagnioj, tomi.valkeinen, daniel.vetter,
airlied, dledford, awalls, syrjala, luto, mst, cocci,
linux-kernel, Luis R. Rodriguez, Toshi Kani, Bjorn Helgaas,
Suresh Siddha, Juergen Gross, Daniel Vetter, Dave Airlie,
Antonino Daplas, Will Deacon, Thierry Reding, Mike Travis,
Mel Gorman, Vlastimil Babka, Davidlohr Bueso, x86, linux-fbdev
In-Reply-To: <1430343851-967-2-git-send-email-mcgrof@do-not-panic.com>
On Wed, Apr 29, 2015 at 02:44:06PM -0700, Luis R. Rodriguez wrote:
> From: "Luis R. Rodriguez" <mcgrof@suse.com>
>
> ioremap_nocache() currently uses UC- by default.
> Our goal is to eventually make UC the default.
> Linux maps UC- to PCD=1, PWT=0 page attributes on
> non-PAT systems. Linux maps UC to PCD=1, PWT=1
> page attributes on non-PAT systems. On non-PAT
> and PAT systems a WC MTRR has different effects on
> pages with either of these attributes. In order to
> help with a smooth transition its best to enable
> use of UC (PCD,1, PWT=1) on a region as that ensures
> a WC MTRR will have no effect on a region, this
> however requires us to have an way to declare a
> region as UC and we currently do not have a way
> to do this.
>
> WC MTRR on non-PAT system with PCD=1, PWT=0 (UC-) yields WC.
> WC MTRR on non-PAT system with PCD=1, PWT=1 (UC) yields UC.
>
> WC MTRR on PAT system with PCD=1, PWT=0 (UC-) yields WC.
> WC MTRR on PAT system with PCD=1, PWT=1 (UC) yields UC.
>
> A flip of the default ioremap_nocache() behaviour
> from UC- to UC can therefore regress a memory
> region from effective memory type WC to UC if MTRRs
> are used. Use of MTRRs should be phased out and in
> the best case only arch_phys_wc_add() use will remain,
> even if this happens arch_phys_wc_add() will have an
> effect on non-PAT systems and changes to default
> ioremap_nocache() behaviour could regress drivers.
>
> Now, ideally we'd use ioremap_nocache() on the regions
> in which we'd need uncachable memory types and avoid
> any MTRRs on those regions. There are however some
> restrictions on MTRRs use, such as the requirement of
> having the base and size of variable sized MTRRs
> to be powers of two, which could mean having to use
> a WC MTRR over a large area which includes a region
> in which write-combining effects are undesirable.
>
> Add ioremap_uc() to help with the both phasing out of
> MTRR use and also provide a way to blacklist small
> WC undesirable regions in devices with mixed regions
> which are size-implicated to use large WC MTRRs. Use
> of ioremap_uc() helps phase out MTRR use by avoiding
> regressions with an eventual flip of default behaviour
> or ioremap_nocache() from UC- to UC.
>
> Drivers working with WC MTRRs can use the below table
> to review and consider the use of ioremap*() and similar
> helpers to ensure appropriate behaviour long term even
> if default ioremap_nocache() behaviour changes from UC-
> to UC.
>
> Although ioremap_uc() is being added we leave set_memory_uc()
> to use UC- as only initial memory type setup is required
> to be able to accomodate existing device drivers and phase
> out MTRR use. It should also be clarified that set_memory_uc()
> cannot be used with IO memory, even though its use will
> not return any errors, it really has no effect.
>
> ----------------------------------------------------------------------
> MTRR Non-PAT PAT Linux ioremap value Effective memory type
> ----------------------------------------------------------------------
> Non-PAT | PAT
> PAT
> |PCD
> ||PWT
> |||
> WC 000 WB _PAGE_CACHE_MODE_WB WC | WC
> WC 001 WC _PAGE_CACHE_MODE_WC WC* | WC
> WC 010 UC- _PAGE_CACHE_MODE_UC_MINUS WC* | WC
> WC 011 UC _PAGE_CACHE_MODE_UC UC | UC
> ----------------------------------------------------------------------
>
> Cc: Toshi Kani <toshi.kani@hp.com>
> Cc: Andy Lutomirski <luto@amacapital.net>
> Cc: Bjorn Helgaas <bhelgaas@google.com>
> Cc: Suresh Siddha <sbsiddha@gmail.com>
> Cc: Ingo Molnar <mingo@elte.hu>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: Juergen Gross <jgross@suse.com>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Cc: Dave Airlie <airlied@redhat.com>
> Cc: Antonino Daplas <adaplas@gmail.com>
> Cc: Jean-Christophe Plagniol-Villard <plagnioj@jcrosoft.com>
> Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>
> Cc: Ville Syrjälä <syrjala@sci.fi>
> Cc: Will Deacon <will.deacon@arm.com>
> Cc: Thierry Reding <treding@nvidia.com>
> Cc: Mike Travis <travis@sgi.com>
> Cc: Mel Gorman <mgorman@suse.de>
> Cc: Vlastimil Babka <vbabka@suse.cz>
> Cc: Borislav Petkov <bp@suse.de>
> Cc: Davidlohr Bueso <dbueso@suse.de>
> Cc: x86@kernel.org
> Cc: linux-fbdev@vger.kernel.org
> Cc: linux-kernel@vger.kernel.org
> Signed-off-by: Luis R. Rodriguez <mcgrof@suse.com>
> ---
> arch/x86/include/asm/io.h | 1 +
> arch/x86/mm/ioremap.c | 36 +++++++++++++++++++++++++++++++++++-
> arch/x86/mm/pageattr.c | 3 +++
> include/asm-generic/io.h | 8 ++++++++
> 4 files changed, 47 insertions(+), 1 deletion(-)
Looks ok to me. Applied, thanks.
--
Regards/Gruss,
Boris.
ECO tip #101: Trim your mails when you reply.
--
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox