From: Luis R. Rodriguez <hidden> Date: 2015-06-25 01:24:48
From: "Luis R. Rodriguez" <redacted>
Boris,
This patchset is part of the long haul of series that addresses removal of
direct use of MTRR and transforms drivers over to use PAT interfaces when
available [0]. Other than this series there is only one more pending series for
that effort, the other one being the atyfb device driver specific changes which
no one has replied to for over one month and I'll soon repost and hope that
Andrew might pick up. The patches in this series were originally split in two
series but I've combined them now given all Acks have been collected and they
are all related. Tomi has provided his Acked-by for all device driver changes.
Bjorn had originally reviewed this series and was comfortable with all the code
except for the use of EXPORT_SYMBOL_GPL() despite new clarifications of how we
can use this for new symbols and our preference for it on new PAT interfaces
[1], despite this Bjorn has clarified he's comfortable with this going in
through another maintainer and in particular Arnd [2]. The v7 series was posted
addressing Arnd, Arnd provided his Acked-by for all PCI and devres changes but
noted he's on parental leave and not taking any patches for arm-soc or
asm-generic until he's back at work in around 3 months from now [2] so he
suggested to see if I could find another maintainer to have these go through.
This v8 goes unmodified, except for the devres commit, since those routines
are not yet used by any device driver for now I've just skipped exporting
the symbols but did note that if they will be it must be exported with
EXPORT_SYMBOL_GPL(). Once we have a driver need them upstream we can export
these.
Although I had test compiled this before just to be safe I went ahead and
successfully test-compiled this set with allmodconfig, specially since I've now
removed the exports for the devres routines. Please let me know if these might
be able to go through you or if there are any questions. I will note the recent
discussion with Benjamin over the v7 series concluded that the ideas we both
were alluding to, on automating instead the WC effects for devices seems a bit
too idealistic for PCI / PCIE for now, but perhaps we should at least consider
this in the future for userspace mmap() calls [4].
[0] http://lkml.kernel.org/r/CAB=NE6UgtdSoBsA=8+ueYRAZHDnWUSmQAoHhAaefqudBrSY7Zw@mail.gmail.com
[1] http://lkml.kernel.org/r/CAErSpo4sHA-f83X1nW2QdLT9GdubFXCQ7UEJmSfFc5GBjj8FSA@mail.gmail.com
[2] http://lkml.kernel.org/r/CAErSpo7CNH1WpgqJCEU8EtxiFNp_PiQ3cBwnKiWQpUaD-fd4YA@mail.gmail.com
[3] http://lkml.kernel.org/r/1435193521.3790.26.camel@kernel.crashing.org
Luis R. Rodriguez (9):
pci: add pci_ioremap_wc_bar()
video: fbdev: i740fb: use arch_phys_wc_add() and pci_ioremap_wc_bar()
video: fbdev: kyrofb: use arch_phys_wc_add() and pci_ioremap_wc_bar()
video: fbdev: gxt4500: use pci_ioremap_wc_bar() for framebuffer
PCI: Add pci_iomap_wc() variants
lib: devres: add pcim_iomap_wc() variants
video: fbdev: arkfb: use arch_phys_wc_add() and pci_iomap_wc()
video: fbdev: s3fb: use arch_phys_wc_add() and pci_iomap_wc()
video: fbdev: vt8623fb: use arch_phys_wc_add() and pci_iomap_wc()
drivers/pci/pci.c | 14 ++++++++
drivers/video/fbdev/arkfb.c | 36 +++----------------
drivers/video/fbdev/gxt4500.c | 2 +-
drivers/video/fbdev/i740fb.c | 35 ++++--------------
drivers/video/fbdev/kyro/fbdev.c | 33 ++++++-----------
drivers/video/fbdev/s3fb.c | 35 ++++--------------
drivers/video/fbdev/vt8623fb.c | 31 ++++------------
include/asm-generic/pci_iomap.h | 14 ++++++++
include/linux/pci.h | 3 ++
include/video/kyro.h | 4 +--
lib/devres.c | 76 ++++++++++++++++++++++++++++++++++++++++
lib/pci_iomap.c | 61 ++++++++++++++++++++++++++++++++
12 files changed, 204 insertions(+), 140 deletions(-)
--
2.3.2.209.gd67f9d5.dirty
From: Luis R. Rodriguez <hidden> Date: 2015-06-25 01:26:57
From: "Luis R. Rodriguez" <redacted>
This lets drivers take advantage of PAT when available. This
should help with the transition of converting video drivers over
to ioremap_wc() to help with the goal of eventually using
_PAGE_CACHE_UC over _PAGE_CACHE_UC_MINUS on x86 on
ioremap_nocache() (de33c442e titled "x86 PAT: fix performance
drop for glx, use UC minus for ioremap(), ioremap_nocache() and
pci_mmap_page_range()")
Cc: Toshi Kani <redacted>
Cc: Bjorn Helgaas <bhelgaas@google.com>
Cc: Suresh Siddha <redacted>
Cc: Ingo Molnar <redacted>
Cc: Thomas Gleixner <redacted>
Cc: Juergen Gross <jgross@suse.com>
Cc: Daniel Vetter <redacted>
Cc: Andy Lutomirski <luto@amacapital.net>
Cc: Dave Airlie <airlied@redhat.com>
Cc: Antonino Daplas <adaplas@gmail.com>
Cc: Jean-Christophe Plagniol-Villard <redacted>
Cc: Tomi Valkeinen <redacted>
Cc: Ville Syrjälä <syrjala@sci.fi>
Cc: Mel Gorman <mgorman@suse.de>
Cc: Vlastimil Babka <redacted>
Cc: Borislav Petkov <redacted>
Cc: Davidlohr Bueso <redacted>
Cc: linux-fbdev@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Acked-by: Arnd Bergmann <arnd@arndb.de>
Signed-off-by: Luis R. Rodriguez <redacted>
---
drivers/pci/pci.c | 14 ++++++++++++++
include/linux/pci.h | 1 +
2 files changed, 15 insertions(+)
From: Luis R. Rodriguez <hidden> Date: 2015-06-25 01:29:10
From: "Luis R. Rodriguez" <redacted>
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: Jingoo Han <redacted>
Cc: Bjorn Helgaas <bhelgaas@google.com>
Cc: Geert Uytterhoeven <geert@linux-m68k.org>
Cc: Rob Clark <redacted>
Cc: Benoit Taine <redacted>
Cc: Suresh Siddha <redacted>
Cc: Ingo Molnar <redacted>
Cc: Thomas Gleixner <redacted>
Cc: Juergen Gross <jgross@suse.com>
Cc: Daniel Vetter <redacted>
Cc: Andy Lutomirski <luto@amacapital.net>
Cc: Dave Airlie <airlied@redhat.com>
Cc: Antonino Daplas <adaplas@gmail.com>
Cc: Jean-Christophe Plagniol-Villard <redacted>
Cc: Tomi Valkeinen <redacted>
Cc: linux-fbdev@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Acked-by: Tomi Valkeinen <redacted>
Signed-off-by: Luis R. Rodriguez <redacted>
---
drivers/video/fbdev/i740fb.c | 35 ++++++-----------------------------
1 file changed, 6 insertions(+), 29 deletions(-)
From: Luis R. Rodriguez <hidden> Date: 2015-06-25 01:31:21
From: "Luis R. Rodriguez" <redacted>
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: Jingoo Han <redacted>
Cc: Geert Uytterhoeven <geert@linux-m68k.org>
Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Cc: Suresh Siddha <redacted>
Cc: Ingo Molnar <redacted>
Cc: Thomas Gleixner <redacted>
Cc: Juergen Gross <jgross@suse.com>
Cc: Daniel Vetter <redacted>
Cc: Andy Lutomirski <luto@amacapital.net>
Cc: Dave Airlie <airlied@redhat.com>
Cc: Antonino Daplas <adaplas@gmail.com>
Cc: Jean-Christophe Plagniol-Villard <redacted>
Cc: Tomi Valkeinen <redacted>
Cc: linux-fbdev@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Acked-by: Tomi Valkeinen <redacted>
Signed-off-by: Luis R. Rodriguez <redacted>
---
drivers/video/fbdev/kyro/fbdev.c | 33 +++++++++++----------------------
include/video/kyro.h | 4 +---
2 files changed, 12 insertions(+), 25 deletions(-)
@@ -35,9 +35,7 @@ struct kyrofb_info {/* Useful to hold depth here for Linux */u8PIXDEPTH;-#ifdef CONFIG_MTRR-intmtrr_handle;-#endif+intwc_cookie;};externintkyro_dev_init(void);
From: Luis R. Rodriguez <hidden> Date: 2015-06-25 01:33:34
From: "Luis R. Rodriguez" <redacted>
The driver doesn't use mtrr_add() or arch_phys_wc_add() but
since we know the framebuffer is isolated already on an
ioremap() we can take advantage of write combining for
performance where possible.
In this case there are a few motivations for this:
a) Take advantage of PAT when available
b) 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()")
Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Cc: Rob Clark <redacted>
Cc: Geert Uytterhoeven <geert@linux-m68k.org>
Cc: Suresh Siddha <redacted>
Cc: Ingo Molnar <redacted>
Cc: Thomas Gleixner <redacted>
Cc: Juergen Gross <jgross@suse.com>
Cc: Daniel Vetter <redacted>
Cc: Andy Lutomirski <luto@amacapital.net>
Cc: Dave Airlie <airlied@redhat.com>
Cc: Antonino Daplas <adaplas@gmail.com>
Cc: Jean-Christophe Plagniol-Villard <redacted>
Cc: Tomi Valkeinen <redacted>
Cc: linux-fbdev@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Acked-by: Tomi Valkeinen <redacted>
Signed-off-by: Luis R. Rodriguez <redacted>
---
drivers/video/fbdev/gxt4500.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
From: Luis R. Rodriguez <hidden> Date: 2015-06-25 01:35:44
From: "Luis R. Rodriguez" <redacted>
PCI BARs tell us whether prefetching is safe, but they don't say anything
about write combining (WC). WC changes ordering rules and allows writes to
be collapsed, so it's not safe in general to use it on a prefetchable
region.
Add pci_iomap_wc() and pci_iomap_wc_range() so drivers can take advantage
of write combining when they know it's safe.
On architectures that don't fully support WC, e.g., x86 without PAT,
drivers for legacy framebuffers may get some of the benefit by using
arch_phys_wc_add() in addition to pci_iomap_wc(). But arch_phys_wc_add()
is unreliable and should be avoided in general. On x86, it uses MTRRs,
which are limited in number and size, so the results will vary based on
driver loading order.
The goals of adding pci_iomap_wc() are to:
- Give drivers an architecture-independent way to use WC so they can stop
using interfaces like mtrr_add() (on x86, pci_iomap_wc() uses
PAT when available)
- Move toward using _PAGE_CACHE_MODE_UC, not _PAGE_CACHE_MODE_UC_MINUS,
on x86 on ioremap_nocache() (see de33c442ed2a ("x86 PAT: fix
performance drop for glx, use UC minus for ioremap(), ioremap_nocache()
and pci_mmap_page_range()")
Link: http://lkml.kernel.org/r/1426893517-2511-6-git-send-email-mcgrof@do-not-panic.com
Original-posting: http://lkml.kernel.org/r/1432163293-20965-1-git-send-email-mcgrof@do-not-panic.com
Cc: Toshi Kani <redacted>
Cc: Andy Lutomirski <luto@amacapital.net>
Cc: Suresh Siddha <redacted>
Cc: Ingo Molnar <redacted>
Cc: Thomas Gleixner <redacted>
Cc: Juergen Gross <jgross@suse.com>
Cc: Daniel Vetter <redacted>
Cc: Dave Airlie <airlied@redhat.com>
Cc: Bjorn Helgaas <bhelgaas@google.com>
Cc: Antonino Daplas <adaplas@gmail.com>
Cc: Jean-Christophe Plagniol-Villard <redacted>
Cc: Tomi Valkeinen <redacted>
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 <redacted>
Cc: Ville Syrjälä <syrjala@sci.fi>
Cc: Mel Gorman <mgorman@suse.de>
Cc: Vlastimil Babka <redacted>
Cc: Borislav Petkov <redacted>
Cc: Davidlohr Bueso <redacted>
Cc: konrad.wilk@oracle.com
Cc: ville.syrjala@linux.intel.com
Cc: david.vrabel@citrix.com
Cc: jbeulich@suse.com
Cc: Roger Pau Monné <redacted>
Cc: linux-fbdev@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Cc: xen-devel@lists.xensource.com
Acked-by: Arnd Bergmann <arnd@arndb.de>
Signed-off-by: Luis R. Rodriguez <redacted>
---
include/asm-generic/pci_iomap.h | 14 ++++++++++
lib/pci_iomap.c | 61 +++++++++++++++++++++++++++++++++++++++++
2 files changed, 75 insertions(+)
@@ -15,9 +15,13 @@ struct pci_dev;#ifdef CONFIG_PCI/* Create a virtual mapping cookie for a PCI BAR (memory or IO) */externvoid__iomem*pci_iomap(structpci_dev*dev,intbar,unsignedlongmax);+externvoid__iomem*pci_iomap_wc(structpci_dev*dev,intbar,unsignedlongmax);externvoid__iomem*pci_iomap_range(structpci_dev*dev,intbar,unsignedlongoffset,unsignedlongmaxlen);+externvoid__iomem*pci_iomap_wc_range(structpci_dev*dev,intbar,+unsignedlongoffset,+unsignedlongmaxlen);/* Create a virtual mapping cookie for a port on a given PCI device.*Donotcallthisdirectly,itexiststomakeiteasierforarchitectures*tooverride*/
@@ -34,12 +38,22 @@ static inline void __iomem *pci_iomap(struct pci_dev *dev, int bar, unsigned lonreturnNULL;}+staticinlinevoid__iomem*pci_iomap_wc(structpci_dev*dev,intbar,unsignedlongmax)+{+returnNULL;+}staticinlinevoid__iomem*pci_iomap_range(structpci_dev*dev,intbar,unsignedlongoffset,unsignedlongmaxlen){returnNULL;}+staticinlinevoid__iomem*pci_iomap_wc_range(structpci_dev*dev,intbar,+unsignedlongoffset,+unsignedlongmaxlen)+{+returnNULL;+}#endif#endif /* __ASM_GENERIC_IO_H */
From: Luis R. Rodriguez <hidden> Date: 2015-06-25 01:37:56
From: "Luis R. Rodriguez" <redacted>
Now that we have pci_iomap_wc() add the respective
devres helpers. These go unexported for now but
note that should they later be exported this
must go with EXPORT_SYMBOL_GPL().
Cc: Toshi Kani <redacted>
Cc: Andy Lutomirski <luto@amacapital.net>
Cc: Suresh Siddha <redacted>
Cc: Ingo Molnar <redacted>
Cc: Thomas Gleixner <redacted>
Cc: Juergen Gross <jgross@suse.com>
Cc: Daniel Vetter <redacted>
Cc: Dave Airlie <airlied@redhat.com>
Cc: Bjorn Helgaas <bhelgaas@google.com>
Cc: Antonino Daplas <adaplas@gmail.com>
Cc: Jean-Christophe Plagniol-Villard <redacted>
Cc: Tomi Valkeinen <redacted>
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 <redacted>
Cc: Ville Syrjälä <syrjala@sci.fi>
Cc: Mel Gorman <mgorman@suse.de>
Cc: Vlastimil Babka <redacted>
Cc: Borislav Petkov <redacted>
Cc: Davidlohr Bueso <redacted>
Cc: konrad.wilk@oracle.com
Cc: ville.syrjala@linux.intel.com
Cc: david.vrabel@citrix.com
Cc: jbeulich@suse.com
Cc: Roger Pau Monné <redacted>
Cc: linux-fbdev@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Cc: xen-devel@lists.xensource.com
Acked-by: Arnd Bergmann <arnd@arndb.de>
Signed-off-by: Luis R. Rodriguez <redacted>
---
include/linux/pci.h | 2 ++
lib/devres.c | 76 +++++++++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 78 insertions(+)
@@ -304,6 +304,29 @@ void __iomem *pcim_iomap(struct pci_dev *pdev, int bar, unsigned long maxlen)EXPORT_SYMBOL(pcim_iomap);/**+*pcim_iomap_wc-Managedpcim_iomap_wc()+*@pdev:PCIdevicetoiomapfor+*@bar:BARtoiomap+*@maxlen:Maximumlengthofiomap+*+*Managedpci_iomap_wc().Mapisautomaticallyunmappedondriver+*detach.+*/+void__iomem*pcim_iomap_wc(structpci_dev*pdev,intbar,unsignedlongmaxlen)+{+void__iomem**tbl;++BUG_ON(bar>=PCIM_IOMAP_MAX);++tbl=(void__iomem**)pcim_iomap_table(pdev);+if(!tbl||tbl[bar])/* duplicate mappings not allowed */+returnNULL;++tbl[bar]=pci_iomap_wc(pdev,bar,maxlen);+returntbl[bar];+}++/***pcim_iounmap-Managedpci_iounmap()*@pdev:PCIdevicetoiounmapfor*@addr:Addresstounmap
@@ -383,6 +406,59 @@ int pcim_iomap_regions(struct pci_dev *pdev, int mask, const char *name)EXPORT_SYMBOL(pcim_iomap_regions);/**+*pcim_iomap_wc_regions-RequestandiomapPCIBARswithwrite-combining+*@pdev:PCIdevicetomapIOresourcesfor+*@mask:MaskofBARstorequestandiomap+*@name:Nameusedwhenrequestingregions+*+*Requestandiomapregionsspecifiedby@maskwithapreferencefor+*write-combining.+*/+intpcim_iomap_wc_regions(structpci_dev*pdev,intmask,constchar*name)+{+void__iomem*const*iomap;+inti,rc;++iomap=pcim_iomap_table(pdev);+if(!iomap)+return-ENOMEM;++for(i=0;i<DEVICE_COUNT_RESOURCE;i++){+unsignedlonglen;++if(!(mask&(1<<i)))+continue;++rc=-EINVAL;+len=pci_resource_len(pdev,i);+if(!len)+gotoerr_inval;++rc=pci_request_region(pdev,i,name);+if(rc)+gotoerr_inval;++rc=-ENOMEM;+if(!pcim_iomap_wc(pdev,i,0))+gotoerr_region;+}++return0;++err_region:+pci_release_region(pdev,i);+err_inval:+while(--i>=0){+if(!(mask&(1<<i)))+continue;+pcim_iounmap(pdev,iomap[i]);+pci_release_region(pdev,i);+}++returnrc;+}++/***pcim_iomap_regions_request_all-RequestallBARsandiomapspecifiedones*@pdev:PCIdevicetomapIOresourcesfor*@mask:MaskofBARstoiomap
From: Luis R. Rodriguez <hidden> Date: 2015-06-25 01:40:01
From: "Luis R. Rodriguez" <redacted>
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: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Cc: Geert Uytterhoeven <geert@linux-m68k.org>
Cc: "Lad, Prabhakar" <prabhakar.csengg@gmail.com>
Cc: Suresh Siddha <redacted>
Cc: Ingo Molnar <redacted>
Cc: Thomas Gleixner <redacted>
Cc: Juergen Gross <jgross@suse.com>
Cc: Daniel Vetter <redacted>
Cc: Andy Lutomirski <luto@amacapital.net>
Cc: Dave Airlie <airlied@redhat.com>
Cc: Antonino Daplas <adaplas@gmail.com>
Cc: Jean-Christophe Plagniol-Villard <redacted>
Cc: Tomi Valkeinen <redacted>
Cc: linux-fbdev@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Acked-by: Tomi Valkeinen <redacted>
Signed-off-by: Luis R. Rodriguez <redacted>
---
drivers/video/fbdev/arkfb.c | 36 +++++-------------------------------
1 file changed, 5 insertions(+), 31 deletions(-)
@@ -26,13 +26,9 @@#include<linux/console.h> /* Why should fb driver call console functions? because console_lock() */#include<video/vga.h>-#ifdef CONFIG_MTRR-#include<asm/mtrr.h>-#endif-structarkfb_info{intmclk_freq;-intmtrr_reg;+intwc_cookie;structdac_info*dac;structvgastatestate;
@@ -102,10 +98,6 @@ static const struct svga_timing_regs ark_timing_regs = {staticchar*mode_option="640x480-8@60";-#ifdef CONFIG_MTRR-staticintmtrr=1;-#endif-MODULE_AUTHOR("(c) 2007 Ondrej Zajicek <santiago@crfreenet.org>");MODULE_LICENSE("GPL");MODULE_DESCRIPTION("fbdev driver for ARK 2000PV");
@@ -115,11 +107,6 @@ MODULE_PARM_DESC(mode_option, "Default video mode ('640x480-8@60', etc)");module_param_named(mode,mode_option,charp,0444);MODULE_PARM_DESC(mode,"Default video mode ('640x480-8@60', etc) (deprecated)");-#ifdef CONFIG_MTRR-module_param(mtrr,int,0444);-MODULE_PARM_DESC(mtrr,"Enable write-combining with MTRR (1=enable, 0=disable, default=1)");-#endif-staticintthreshold=4;module_param(threshold,int,0644);
@@ -1002,7 +989,7 @@ static int ark_pci_probe(struct pci_dev *dev, const struct pci_device_id *id)info->fix.smem_len=pci_resource_len(dev,0);/* Map physical IO memory address into kernel space */-info->screen_base=pci_iomap(dev,0,0);+info->screen_base=pci_iomap_wc(dev,0,0);if(!info->screen_base){rc=-ENOMEM;dev_err(info->device,"iomap for framebuffer failed\n");
@@ -1057,14 +1044,8 @@ static int ark_pci_probe(struct pci_dev *dev, const struct pci_device_id *id)/* Record a reference to the driver data */pci_set_drvdata(dev,info);--#ifdef CONFIG_MTRR-if(mtrr){-par->mtrr_reg=-1;-par->mtrr_reg=mtrr_add(info->fix.smem_start,info->fix.smem_len,MTRR_TYPE_WRCOMB,1);-}-#endif-+par->wc_cookie=arch_phys_wc_add(info->fix.smem_start,+info->fix.smem_len);return0;/* Error handling */
From: Luis R. Rodriguez <hidden> Date: 2015-06-25 01:42:17
From: "Luis R. Rodriguez" <redacted>
This driver uses the same area for MTRR as for the ioremap().
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: Jean-Christophe Plagniol-Villard <redacted>
Cc: Tomi Valkeinen <redacted>
Cc: Jingoo Han <redacted>
Cc: Geert Uytterhoeven <geert@linux-m68k.org>
Cc: Daniel Vetter <redacted>
Cc: "Lad, Prabhakar" <prabhakar.csengg@gmail.com>
Cc: Rickard Strandqvist <redacted>
Cc: Suresh Siddha <redacted>
Cc: Ingo Molnar <redacted>
Cc: Thomas Gleixner <redacted>
Cc: Juergen Gross <jgross@suse.com>
Cc: Andy Lutomirski <luto@amacapital.net>
Cc: Dave Airlie <airlied@redhat.com>
Cc: Antonino Daplas <adaplas@gmail.com>
Cc: linux-fbdev@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Acked-by: Tomi Valkeinen <redacted>
Signed-off-by: Luis R. Rodriguez <redacted>
---
drivers/video/fbdev/s3fb.c | 35 ++++++-----------------------------
1 file changed, 6 insertions(+), 29 deletions(-)
@@ -170,11 +162,8 @@ module_param(mode_option, charp, 0444);MODULE_PARM_DESC(mode_option,"Default video mode ('640x480-8@60', etc)");module_param_named(mode,mode_option,charp,0444);MODULE_PARM_DESC(mode,"Default video mode ('640x480-8@60', etc) (deprecated)");--#ifdef CONFIG_MTRRmodule_param(mtrr,int,0444);MODULE_PARM_DESC(mtrr,"Enable write-combining with MTRR (1=enable, 0=disable, default=1)");-#endifmodule_param(fasttext,int,0644);MODULE_PARM_DESC(fasttext,"Enable S3 fast text mode (1=enable, 0=disable, default=1)");
@@ -1168,7 +1157,7 @@ static int s3_pci_probe(struct pci_dev *dev, const struct pci_device_id *id)info->fix.smem_len=pci_resource_len(dev,0);/* Map physical IO memory address into kernel space */-info->screen_base=pci_iomap(dev,0,0);+info->screen_base=pci_iomap_wc(dev,0,0);if(!info->screen_base){rc=-ENOMEM;dev_err(info->device,"iomap for framebuffer failed\n");
@@ -1365,12 +1354,9 @@ static int s3_pci_probe(struct pci_dev *dev, const struct pci_device_id *id)/* Record a reference to the driver data */pci_set_drvdata(dev,info);-#ifdef CONFIG_MTRR-if(mtrr){-par->mtrr_reg=-1;-par->mtrr_reg=mtrr_add(info->fix.smem_start,info->fix.smem_len,MTRR_TYPE_WRCOMB,1);-}-#endif+if(mtrr)+par->wc_cookie=arch_phys_wc_add(info->fix.smem_start,+info->fix.smem_len);return0;
From: Luis R. Rodriguez <hidden> Date: 2015-06-25 01:44:33
From: "Luis R. Rodriguez" <redacted>
This driver uses the same area for MTRR as for the ioremap().
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: Rob Clark <redacted>
Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Cc: Jingoo Han <redacted>
Cc: "Lad, Prabhakar" <prabhakar.csengg@gmail.com>
Cc: Suresh Siddha <redacted>
Cc: Ingo Molnar <redacted>
Cc: Thomas Gleixner <redacted>
Cc: Juergen Gross <jgross@suse.com>
Cc: Daniel Vetter <redacted>
Cc: Andy Lutomirski <luto@amacapital.net>
Cc: Dave Airlie <airlied@redhat.com>
Cc: Antonino Daplas <adaplas@gmail.com>
Cc: Jean-Christophe Plagniol-Villard <redacted>
Cc: Tomi Valkeinen <redacted>
Cc: linux-fbdev@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Acked-by: Tomi Valkeinen <redacted>
Signed-off-by: Luis R. Rodriguez <redacted>
---
drivers/video/fbdev/vt8623fb.c | 31 ++++++-------------------------
1 file changed, 6 insertions(+), 25 deletions(-)
@@ -112,11 +105,8 @@ module_param(mode_option, charp, 0644);MODULE_PARM_DESC(mode_option,"Default video mode ('640x480-8@60', etc)");module_param_named(mode,mode_option,charp,0);MODULE_PARM_DESC(mode,"Default video mode e.g. '648x480-8@60' (deprecated)");--#ifdef CONFIG_MTRRmodule_param(mtrr,int,0444);MODULE_PARM_DESC(mtrr,"Enable write-combining with MTRR (1=enable, 0=disable, default=1)");-#endif/* ------------------------------------------------------------------------- */
@@ -710,7 +700,7 @@ static int vt8623_pci_probe(struct pci_dev *dev, const struct pci_device_id *id)info->fix.mmio_len=pci_resource_len(dev,1);/* Map physical IO memory address into kernel space */-info->screen_base=pci_iomap(dev,0,0);+info->screen_base=pci_iomap_wc(dev,0,0);if(!info->screen_base){rc=-ENOMEM;dev_err(info->device,"iomap for framebuffer failed\n");
@@ -781,12 +771,9 @@ static int vt8623_pci_probe(struct pci_dev *dev, const struct pci_device_id *id)/* Record a reference to the driver data */pci_set_drvdata(dev,info);-#ifdef CONFIG_MTRR-if(mtrr){-par->mtrr_reg=-1;-par->mtrr_reg=mtrr_add(info->fix.smem_start,info->fix.smem_len,MTRR_TYPE_WRCOMB,1);-}-#endif+if(mtrr)+par->wc_cookie=arch_phys_wc_add(info->fix.smem_start,+info->fix.smem_len);return0;
On Wed, Jun 24, 2015 at 06:22:18PM -0700, Luis R. Rodriguez wrote:
From: "Luis R. Rodriguez" <redacted>
PCI BARs tell us whether prefetching is safe, but they don't say anything
about write combining (WC). WC changes ordering rules and allows writes to
be collapsed, so it's not safe in general to use it on a prefetchable
region.
Add pci_iomap_wc() and pci_iomap_wc_range() so drivers can take advantage
of write combining when they know it's safe.
On architectures that don't fully support WC, e.g., x86 without PAT,
drivers for legacy framebuffers may get some of the benefit by using
arch_phys_wc_add() in addition to pci_iomap_wc(). But arch_phys_wc_add()
is unreliable and should be avoided in general. On x86, it uses MTRRs,
which are limited in number and size, so the results will vary based on
driver loading order.
The goals of adding pci_iomap_wc() are to:
- Give drivers an architecture-independent way to use WC so they can stop
using interfaces like mtrr_add() (on x86, pci_iomap_wc() uses
PAT when available)
- Move toward using _PAGE_CACHE_MODE_UC, not _PAGE_CACHE_MODE_UC_MINUS,
on x86 on ioremap_nocache() (see de33c442ed2a ("x86 PAT: fix
performance drop for glx, use UC minus for ioremap(), ioremap_nocache()
and pci_mmap_page_range()")
I've moved this check at the beginning of the function so that we bail
out before doing the computations above it.
--
Regards/Gruss,
Boris.
ECO tip #101: Trim your mails when you reply.
--
On Wed, Jun 24, 2015 at 06:22:19PM -0700, Luis R. Rodriguez wrote:
From: "Luis R. Rodriguez" <redacted>
Now that we have pci_iomap_wc() add the respective
devres helpers. These go unexported for now but
note that should they later be exported this
must go with EXPORT_SYMBOL_GPL().
Do I see it correctly, those are not used in this patchset?
If so, then let's keep this patch in the bag and pick it up only when
those functions have users.
--
Regards/Gruss,
Boris.
ECO tip #101: Trim your mails when you reply.
--
From: Luis R. Rodriguez <hidden> Date: 2015-06-25 15:52:23
On Thu, Jun 25, 2015 at 8:40 AM, Borislav Petkov [off-list ref] wrote:
On Wed, Jun 24, 2015 at 06:22:19PM -0700, Luis R. Rodriguez wrote:
quoted
From: "Luis R. Rodriguez" <redacted>
Now that we have pci_iomap_wc() add the respective
devres helpers. These go unexported for now but
note that should they later be exported this
must go with EXPORT_SYMBOL_GPL().
Do I see it correctly, those are not used in this patchset?
That's correct. It was a preemptive implementation of devres pci wc APIs.
If so, then let's keep this patch in the bag and pick it up only when
those functions have users.
From: Benjamin Herrenschmidt <benh@kernel.crashing.org> Date: 2015-06-26 02:35:59
On Wed, 2015-06-24 at 18:22 -0700, Luis R. Rodriguez wrote:
Although I had test compiled this before just to be safe I went ahead and
successfully test-compiled this set with allmodconfig, specially since I've now
removed the exports for the devres routines. Please let me know if these might
be able to go through you or if there are any questions. I will note the recent
discussion with Benjamin over the v7 series concluded that the ideas we both
were alluding to, on automating instead the WC effects for devices seems a bit
too idealistic for PCI / PCIE for now, but perhaps we should at least consider
this in the future for userspace mmap() calls [4].
So I've been trying to figure out how to make this practically work for us (powerpc).
writel() will never write combine for us, it uses too heavy barriers.
writel_relaxed() today is identical to writel() but we can change it.
The problem is that switching to G=0 mappings (which is what provides us with write
combining) also architecturally enables prefetch and speculative loads... and again
architecturally (the implementations may differ), kills the effect of the lightweight
io barrier eieio which we would have to use in readl_relaxed() and writel_relaxed()
to provide their normal semantics.
So it boils down to: Can we modify the documentation of readl_relaxed() and writel_relaxed()
to define them as being even further relaxed when using a "wc" mapping ?
Otherwise, the only way out I see for us on powerpc is to bias massively writel_relaxed()
against real_relaxed() by putting heavy barriers around the load in the latter so we can
keep them completely out of the former and still enable wc.
Ben.
From: Luis R. Rodriguez <hidden> Date: 2015-07-07 16:15:18
On Fri, Jun 26, 2015 at 12:12:06PM +1000, Benjamin Herrenschmidt wrote:
On Wed, 2015-06-24 at 18:22 -0700, Luis R. Rodriguez wrote:
quoted
Although I had test compiled this before just to be safe I went ahead and
successfully test-compiled this set with allmodconfig, specially since I've now
removed the exports for the devres routines. Please let me know if these might
be able to go through you or if there are any questions. I will note the recent
discussion with Benjamin over the v7 series concluded that the ideas we both
were alluding to, on automating instead the WC effects for devices seems a bit
too idealistic for PCI / PCIE for now, but perhaps we should at least consider
this in the future for userspace mmap() calls [4].
So I've been trying to figure out how to make this practically work for us (powerpc).
writel() will never write combine for us, it uses too heavy barriers.
writel_relaxed() today is identical to writel() but we can change it.
The problem is that switching to G=0 mappings (which is what provides us with write
combining) also architecturally enables prefetch and speculative loads... and again
architecturally (the implementations may differ), kills the effect of the lightweight
io barrier eieio which we would have to use in readl_relaxed() and writel_relaxed()
to provide their normal semantics.
So it boils down to: Can we modify the documentation of readl_relaxed() and writel_relaxed()
to define them as being even further relaxed when using a "wc" mapping ?
Otherwise, the only way out I see for us on powerpc is to bias massively writel_relaxed()
against real_relaxed() by putting heavy barriers around the load in the latter so we can
keep them completely out of the former and still enable wc.
Depends if you semantically then also are implicating its use for the ioremap_wc()
area and if we've ensured we've visited all other possibilities to avoid this. Instead
of replying here though it seems we have a large general ioremap() semantic discussion
ongoing on another thread which is far ahead of this one and more generalized. Mind
following up there, seems the party is there:
http://lkml.kernel.org/r/20150707160703.GR7021@wotan.suse.de
Luis