* Re: [PATCH v5 0/3] atyfb: address MTRR corner case
From: Borislav Petkov @ 2015-06-25 20:48 UTC (permalink / raw)
To: Luis R. Rodriguez
Cc: akpm, mingo, syrjala, ville.syrjala, luto, tomi.valkeinen, mst,
benh, linux-kernel, linux-fbdev, linux-pci, airlied,
Luis R. Rodriguez
In-Reply-To: <1435196060-27350-1-git-send-email-mcgrof@do-not-panic.com>
On Wed, Jun 24, 2015 at 06:34:17PM -0700, Luis R. Rodriguez wrote:
> Luis R. Rodriguez (3):
> video: fbdev: atyfb: clarify ioremap() base and length used
> video: fbdev: atyfb: replace MTRR UC hole with strong UC
> video: fbdev: atyfb: use arch_phys_wc_add() and ioremap_wc()
>
> drivers/video/fbdev/aty/atyfb.h | 5 +--
> drivers/video/fbdev/aty/atyfb_base.c | 74 +++++++++++-------------------------
> 2 files changed, 24 insertions(+), 55 deletions(-)
Took those too along with
"[PATCH] video: fbdev: atyfb: move framebuffer length fudging to helper"
which was missing here.
--
Regards/Gruss,
Boris.
ECO tip #101: Trim your mails when you reply.
--
^ permalink raw reply
* Re: [PATCH v3 16/19] staging: sm750fb: fix brace placement
From: Juston Li @ 2015-06-25 21:27 UTC (permalink / raw)
To: Dan Carpenter
Cc: Sudip Mukherjee, teddy.wang, Greg KH, linux-fbdev, devel,
linux-kernel
In-Reply-To: <20150625203115.GN30834@mwanda>
On Thu, Jun 25, 2015 at 1:31 PM, Dan Carpenter <dan.carpenter@oracle.com> wrote:
> Once you add the else statement, then that kind of doesn't make sense.
> Sudip is right. It should be:
>
> } else {
> /* commentary about else side */
Yeah, that does make more sense. I'll change it, thanks.
Patchset was responded by Greg's patch bot, I'll resend when I figure out why.
Regards
Juston
^ permalink raw reply
* RE: [PATCH v7 5/9] PCI: Add pci_iomap_wc() variants
From: Casey Leedom @ 2015-06-25 21:40 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Benjamin Herrenschmidt, Luis R. Rodriguez, Michael S. Tsirkin,
Bjorn Helgaas, Toshi Kani, Andy Lutomirski, Juergen Gross,
Tomi Valkeinen, linux-pci@vger.kernel.org,
linux-kernel@vger.kernel.org, xen-devel@lists.xensource.com,
linux-fbdev, Suresh Siddha, Ingo Molnar, Thomas Gleixner,
Daniel Vetter, Dave Airlie, Antonino Daplas,
Jean-Christophe Plagniol-Villard, Dave Hansen,
venkatesh.pallipadi@intel.com, Stefan Bader,
Ville =?ISO-8859-1?Q?Syrj=E4l=E4? =, Mel Gorman, Vlastimil Babka,
Borislav Petkov, Davidlohr Bueso, Konrad Rzeszutek Wilk,
Ville Syrjälä, David Vrabel, Jan Beulich,
Roger Pau Monné
In-Reply-To: <6806026.xb91q6Ad7G@wuerfel>
| From: Arnd Bergmann [arnd@arndb.de]
| Sent: Thursday, June 25, 2015 1:44 PM
|
| On Thursday 25 June 2015 15:01:56 Casey Leedom wrote:
| >
| > Is there a reference I can read on this so I can understand
| > when and where we can use the __raw_*() APIs? Can these
| > Raw Read/Write operations be reordered with respect to
| > each other or are the use of the various flavors of SYNC
| > instructions just to maintain order between Cached Memory
| > Accesses and I/O Instructions?
|
| The interpretation is not consistent across architectures.
|
| My best description would be that the __raw_*() accessors should
| only be used for accessing RAM areas that are known to have no
| side-effects and can be read in any size (8-bit to 64-bit wide),
| any alignment, and do not have a specific endianess.
|
| If you are dealing with MMIO registers that have a fixed endianess
| and size, the correct accessor would be readl_relaxed(), which
| is like readl() but lacks the barriers on certain architectures.
Ah, thanks. I see now that the __raw_*() APIs don't do any of the
Endian Swizzling. Unfortunately the *_relaxed() APIs on PowerPC
are just defined as the normal *() routines. From
arch/powerpc/include/asm/io.h:
/*
* We don't do relaxed operations yet, at least not with this semantic
*/
#define readb_relaxed(addr) readb(addr)
#define readw_relaxed(addr) readw(addr)
#define readl_relaxed(addr) readl(addr)
#define readq_relaxed(addr) readq(addr)
#define writeb_relaxed(v, addr) writeb(v, addr)
#define writew_relaxed(v, addr) writew(v, addr)
#define writel_relaxed(v, addr) writel(v, addr)
#define writeq_relaxed(v, addr) writeq(v, addr)
(And in fact, this is true for most of the architectures.)
So we could go ahead and use these but for now there wouldn't be
any effect.
Hhmmm, so what do PowerPC Drivers do when they want to take
advantage of Write Combining? Do their own Endian Swizzling
with the __raw_*() APIs?
Casey
^ permalink raw reply
* Re: [PATCH v7 5/9] PCI: Add pci_iomap_wc() variants
From: Benjamin Herrenschmidt @ 2015-06-25 22:51 UTC (permalink / raw)
To: Casey Leedom
Cc: Arnd Bergmann, Luis R. Rodriguez, Michael S. Tsirkin,
Bjorn Helgaas, Toshi Kani, Andy Lutomirski, Juergen Gross,
Tomi Valkeinen, linux-pci@vger.kernel.org,
linux-kernel@vger.kernel.org, xen-devel@lists.xensource.com,
linux-fbdev, Suresh Siddha, Ingo Molnar, Thomas Gleixner,
Daniel Vetter, Dave Airlie, Antonino Daplas,
Jean-Christophe Plagniol-Villard, Dave Hansen,
venkatesh.pallipadi@intel.com, Stefan Bader, ville.syrjala,
David Vrabel, Jan Beulich, Roger Pau Monné
In-Reply-To: <4985EFDD773FCB459EF7915D2A3621ADC031F8@nice.asicdesigners.com>
On Thu, 2015-06-25 at 21:40 +0000, Casey Leedom wrote:
> Hhmmm, so what do PowerPC Drivers do when they want to take
> advantage of Write Combining? Do their own Endian Swizzling
> with the __raw_*() APIs?
Yeah either, we need to fix our relaxed implementation (patch
welcome :-)
Cheers,
Ben.
^ permalink raw reply
* Re: [PATCH v5 1/3] video: fbdev: atyfb: clarify ioremap() base and length used
From: Ville Syrjälä @ 2015-06-25 23:04 UTC (permalink / raw)
To: Luis R. Rodriguez
Cc: akpm, bp, mingo, ville.syrjala, luto, tomi.valkeinen, mst, benh,
linux-kernel, linux-fbdev, linux-pci, airlied, Luis R. Rodriguez,
Toshi Kani, Suresh Siddha, Linus Torvalds, Thomas Gleixner,
Juergen Gross, Daniel Vetter, Antonino Daplas,
Jean-Christophe Plagniol-Villard, Rob Clark, Mathias Krause,
Andrzej Hajda, Mel Gorman, Vlastimil Babka, Davidlohr Bueso
In-Reply-To: <1435196060-27350-2-git-send-email-mcgrof@do-not-panic.com>
On Wed, Jun 24, 2015 at 06:34:18PM -0700, Luis R. Rodriguez wrote:
> From: "Luis R. Rodriguez" <mcgrof@suse.com>
>
> This has no functional changes, it just adjusts
> the ioremap() call for the framebuffer to use
> the same values we later use for the framebuffer,
> this will make it easier to review the next change.
>
> The size of the framebuffer varies but since this is
> for PCI we *know* this defaults to 0x800000.
> atyfb_setup_generic() is *only* used on PCI probe.
>
> Cc: Toshi Kani <toshi.kani@hp.com>
> Cc: Suresh Siddha <sbsiddha@gmail.com>
> 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: 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: Ville Syrjälä <syrjala@sci.fi>
> Cc: Rob Clark <robdclark@gmail.com>
> Cc: Mathias Krause <minipli@googlemail.com>
> Cc: Andrzej Hajda <a.hajda@samsung.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: linux-fbdev@vger.kernel.org
> Cc: linux-kernel@vger.kernel.org
> Signed-off-by: Luis R. Rodriguez <mcgrof@suse.com>
> ---
> drivers/video/fbdev/aty/atyfb_base.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/video/fbdev/aty/atyfb_base.c b/drivers/video/fbdev/aty/atyfb_base.c
> index 16936bb..8025624 100644
> --- a/drivers/video/fbdev/aty/atyfb_base.c
> +++ b/drivers/video/fbdev/aty/atyfb_base.c
> @@ -3489,7 +3489,9 @@ static int atyfb_setup_generic(struct pci_dev *pdev, struct fb_info *info,
>
> /* Map in frame buffer */
> info->fix.smem_start = addr;
> - info->screen_base = ioremap(addr, 0x800000);
> + info->fix.smem_len = 0x800000;
> +
> + info->screen_base = ioremap(info->fix.smem_start, info->fix.smem_len);
The framebuffer size isn't always 8MB. That's the size of the BAR. So
this change isn't really correct. I suppose it doesn't hurt too much
since smem_len gets overwritten later in aty_init().
> if (info->screen_base = NULL) {
> ret = -ENOMEM;
> goto atyfb_setup_generic_fail;
> --
> 2.3.2.209.gd67f9d5.dirty
>
--
Ville Syrjälä
syrjala@sci.fi
http://www.sci.fi/~syrjala/
^ permalink raw reply
* Re: [PATCH v5 1/3] video: fbdev: atyfb: clarify ioremap() base and length used
From: Luis R. Rodriguez @ 2015-06-25 23:06 UTC (permalink / raw)
To: Ville Syrjälä, Luis R. Rodriguez, Andrew Morton,
Borislav Petkov, Ingo Molnar, Ville Syrjälä,
Andy Lutomirski, Tomi Valkeinen, Michael S. Tsirkin,
Benjamin Herrenschmidt, linux-kernel@vger.kernel.org, linux-fbdev,
linux-pci@vger.kernel.org, Dave Airlie, Luis R. Rodriguez,
Toshi Kani, Suresh Siddha, Linus Torvalds, Thomas Gleixner,
Juergen Gross, Daniel Vetter, Antonino Daplas,
Jean-Christophe Plagniol-Villard, Rob Clark, Mathias Krause,
Andrzej Hajda, Mel Gorman, Vlastimil Babka, Davidlohr Bueso
In-Reply-To: <20150625230437.GA4362@sci.fi>
On Thu, Jun 25, 2015 at 4:04 PM, Ville Syrjälä <syrjala@sci.fi> wrote:
> it doesn't hurt too much
> since smem_len gets overwritten later in aty_init().
That's the idea, we set it with a default as it will be overwritten
later anyway.
Luis
^ permalink raw reply
* Re: [PATCH v5 1/3] video: fbdev: atyfb: clarify ioremap() base and length used
From: Ville Syrjälä @ 2015-06-25 23:11 UTC (permalink / raw)
To: Luis R. Rodriguez
Cc: Andrew Morton, Borislav Petkov, Ingo Molnar,
Ville Syrjälä, Andy Lutomirski, Tomi Valkeinen,
Michael S. Tsirkin, Benjamin Herrenschmidt,
linux-kernel@vger.kernel.org, linux-fbdev,
linux-pci@vger.kernel.org, Dave Airlie, Luis R. Rodriguez,
Toshi Kani, Suresh Siddha, Linus Torvalds, Thomas Gleixner,
Juergen Gross, Daniel Vetter, Antonino Daplas,
Jean-Christophe Plagniol-Villard, Rob Clark, Mathias Krause,
Andrzej Hajda, Mel Gorman, Vlastimil Babka, Davidlohr Bueso
In-Reply-To: <CAB=NE6V0HGCkpYFghUyyk=+St-494Y8e9PEakF9F7cEB8vpKvA@mail.gmail.com>
On Thu, Jun 25, 2015 at 04:06:45PM -0700, Luis R. Rodriguez wrote:
> On Thu, Jun 25, 2015 at 4:04 PM, Ville Syrjälä <syrjala@sci.fi> wrote:
> > it doesn't hurt too much
> > since smem_len gets overwritten later in aty_init().
>
> That's the idea, we set it with a default as it will be overwritten
> later anyway.
Maybe toss in a comment? Otherwise it's a bit dishonest and might give
someone the impression that all PCI cards really have 8MB of memory.
--
Ville Syrjälä
syrjala@sci.fi
http://www.sci.fi/~syrjala/
^ permalink raw reply
* Re: [PATCH v5 1/3] video: fbdev: atyfb: clarify ioremap() base and length used
From: Luis R. Rodriguez @ 2015-06-26 1:09 UTC (permalink / raw)
To: Ville Syrjälä, Luis R. Rodriguez, Andrew Morton,
Borislav Petkov, Ingo Molnar, Ville Syrjälä,
Andy Lutomirski, Tomi Valkeinen, Michael S. Tsirkin,
Benjamin Herrenschmidt, linux-kernel@vger.kernel.org, linux-fbdev,
linux-pci@vger.kernel.org, Dave Airlie, Toshi Kani, Suresh Siddha,
Linus Torvalds, Thomas Gleixner, Juergen Gross, Daniel Vetter,
Antonino Daplas, Jean-Christophe Plagniol-Villard, Rob Clark,
Mathias Krause, Andrzej Hajda, Mel Gorman, Vlastimil Babka,
Davidlohr Bueso
In-Reply-To: <20150625231103.GB4362@sci.fi>
On Fri, Jun 26, 2015 at 02:11:03AM +0300, Ville Syrjälä wrote:
> On Thu, Jun 25, 2015 at 04:06:45PM -0700, Luis R. Rodriguez wrote:
> > On Thu, Jun 25, 2015 at 4:04 PM, Ville Syrjälä <syrjala@sci.fi> wrote:
> > > it doesn't hurt too much
> > > since smem_len gets overwritten later in aty_init().
> >
> > That's the idea, we set it with a default as it will be overwritten
> > later anyway.
>
> Maybe toss in a comment? Otherwise it's a bit dishonest and might give
> someone the impression that all PCI cards really have 8MB of memory.
Sure, mind this as a follow up patch if its too late?
Luis
^ permalink raw reply
* Re: [PATCH v7 5/9] PCI: Add pci_iomap_wc() variants
From: Benjamin Herrenschmidt @ 2015-06-26 2:02 UTC (permalink / raw)
To: Casey Leedom
Cc: Arnd Bergmann, Luis R. Rodriguez, Michael S. Tsirkin,
Bjorn Helgaas, Toshi Kani, Andy Lutomirski, Juergen Gross,
Tomi Valkeinen, linux-pci@vger.kernel.org,
linux-kernel@vger.kernel.org, xen-devel@lists.xensource.com,
linux-fbdev, Suresh Siddha, Ingo Molnar, Thomas Gleixner,
Daniel Vetter, Dave Airlie, Antonino Daplas,
Jean-Christophe Plagniol-Villard, Dave Hansen,
venkatesh.pallipadi@intel.com, Stefan Bader, ville.syrjala,
David Vrabel, Jan Beulich, Roger Pau Monné
In-Reply-To: <4985EFDD773FCB459EF7915D2A3621ADC031F8@nice.asicdesigners.com>
On Thu, 2015-06-25 at 21:40 +0000, Casey Leedom wrote:
>
> Ah, thanks. I see now that the __raw_*() APIs don't do any of the
> Endian Swizzling. Unfortunately the *_relaxed() APIs on PowerPC
> are just defined as the normal *() routines. From
> arch/powerpc/include/asm/io.h:
>
> /*
> * We don't do relaxed operations yet, at least not with this
> semantic
> */
Yes so I was looking at this but there are some difficulties.
Architecturally, even with I=1 G=1 mappings (normal ioremap), we have no
guarantee of ordering of load vs. store unless I misunderstood
something. I think all current implementations provide some of that but
without barriers in the accessors, we aren't architecturally correct.
However, having those barriers will cause issues with G=0 (write
combine). It's unclear whether eieio() will provide the required
ordering for I=1 G=0 mappings and it will probably break write combine.
I'm looking into it with our HW guys and will try to come up with a
solution for power, but it doesn't help that our memory model conflates
write combining with other relaxations and that all our barriers also
prevent write combine.
Maybe we can bias the relaxed accessors toward write, by having no
barriers in it, and putting extra ones in reads.
Cheers,
Ben.
^ permalink raw reply
* Re: [PATCH v8 0/9] pci: add pci_iomap_wc() and pci_ioremap_wc_bar()
From: Benjamin Herrenschmidt @ 2015-06-26 2:12 UTC (permalink / raw)
To: Luis R. Rodriguez
Cc: bp, mingo, arnd, bhelgaas, luto, akpm, linux-pci, linux-kernel,
tomi.valkeinen, mst, toshi.kani, linux-fbdev, xen-devel,
Luis R. Rodriguez
In-Reply-To: <1435195342-26879-1-git-send-email-mcgrof@do-not-panic.com>
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.
^ permalink raw reply
* Re: [PATCH v3 16/19] staging: sm750fb: fix brace placement
From: Sudip Mukherjee @ 2015-06-26 4:12 UTC (permalink / raw)
To: Juston Li
Cc: Dan Carpenter, teddy.wang, Greg KH, linux-fbdev, devel,
linux-kernel
In-Reply-To: <CAFow2F_St7szm9m4y-M66q=h6VafBjUSRwQ=DcfHWSi8kLf5mw@mail.gmail.com>
On Thu, Jun 25, 2015 at 02:27:02PM -0700, Juston Li wrote:
> On Thu, Jun 25, 2015 at 1:31 PM, Dan Carpenter <dan.carpenter@oracle.com> wrote:
> Patchset was responded by Greg's patch bot, I'll resend when I figure out why.
Greg will not apply any patch since the merge window is open. That
patchbot will respond when anyone send patch to Greg during this time.
Just send the updated patch as a reply to the original patch with proper
version number and Greg will pick it up later.
regards
sudip
^ permalink raw reply
* Re: [PATCH v3 11/19] staging: sm750fb: consistent spacing around operators
From: Juston Li @ 2015-06-26 4:14 UTC (permalink / raw)
To: Sudip Mukherjee
Cc: teddy.wang, Greg KH, linux-fbdev, devel, linux-kernel,
Dan Carpenter
In-Reply-To: <20150626040543.GB3050@sudip-PC>
On Thu, Jun 25, 2015 at 9:05 PM, Sudip Mukherjee
<sudipm.mukherjee@gmail.com> wrote:
> Usually I use the checkpatch which is in linux-next. That will be the
> latest version. If you compare, last commit on checkpatch of staging
> tree was on April 16th, where as last commit in linux-next is on
> June 19th. And if you use this checkpatch you will see these warnings
> also. :)
Thanks for the tip, I'll use the latest checkpatch and fix the warnings.
Your help has been much appreciated
Regards
Juston
^ permalink raw reply
* Re: [PATCH v3 11/19] staging: sm750fb: consistent spacing around operators
From: Sudip Mukherjee @ 2015-06-26 4:17 UTC (permalink / raw)
To: Juston Li
Cc: teddy.wang, Greg KH, linux-fbdev, devel, linux-kernel,
Dan Carpenter
In-Reply-To: <CAFow2F9=DhqioyV=tha=KBiTi0pz=Ee+NJJL2up41jUUcF66xg@mail.gmail.com>
On Thu, Jun 25, 2015 at 09:26:00AM -0700, Juston Li wrote:
> On Thu, Jun 25, 2015 at 4:56 AM, Sudip Mukherjee
> <sudipm.mukherjee@gmail.com> wrote:
> > On Wed, Jun 24, 2015 at 09:25:12AM -0700, Juston Li wrote:
<snip>
>
> > 2) Considering that you are giving consitent space around operators, then
> > i think you have missed few operators like '|','&',':' in this
> > patch.
> >
>
> These were the only instances the checkpatch.pl error came up. This patch
> only fixes even spacing around a specific operator. Other operators either
> have even or no spacing around them with PATCH 11/19 fixing any operators
> with spacing around required. This leaves operators that checkpatch deems
> spacing as optional which I left as a separate issue to be addressed of making
> optional spacing of operators consistent with each other.
Usually I use the checkpatch which is in linux-next. That will be the
latest version. If you compare, last commit on checkpatch of staging
tree was on April 16th, where as last commit in linux-next is on
June 19th. And if you use this checkpatch you will see these warnings
also. :)
regards
sudip
^ permalink raw reply
* Re: [PATCH v3 11/19] staging: sm750fb: consistent spacing around operators
From: Juston Li @ 2015-06-26 5:53 UTC (permalink / raw)
To: Sudip Mukherjee
Cc: teddy.wang, Greg KH, linux-fbdev, devel, linux-kernel,
Dan Carpenter
In-Reply-To: <CAFow2F-Cj25rv3-5Y0ck29On=WVQVakeo5-7s_dWmnd=rj+hig@mail.gmail.com>
On Thu, Jun 25, 2015 at 9:05 PM, Sudip Mukherjee
<sudipm.mukherjee@gmail.com> wrote:
> Usually I use the checkpatch which is in linux-next. That will be the
> latest version. If you compare, last commit on checkpatch of staging
> tree was on April 16th, where as last commit in linux-next is on
> June 19th. And if you use this checkpatch you will see these warnings
> also. :)
Just wanted to clarify. So even with the linux-next checkpatch.pl, no
errors/warnings show for me. Only when I run checkpatch.pl with the
'--strict' option does it give the following checks for various operators:
CHECK: space preferred before that '|' (ctx:VxE)
Are these the warnings you are referring too? Do you want me to fix all
of these for this patch?
Regards
Juston
^ permalink raw reply
* Re: [PATCH v5 1/3] video: fbdev: atyfb: clarify ioremap() base and length used
From: Borislav Petkov @ 2015-06-26 7:30 UTC (permalink / raw)
To: Luis R. Rodriguez
Cc: Ville Syrjälä, Luis R. Rodriguez, Andrew Morton,
Ingo Molnar, Ville Syrjälä, Andy Lutomirski,
Tomi Valkeinen, Michael S. Tsirkin, Benjamin Herrenschmidt,
linux-kernel@vger.kernel.org, linux-fbdev,
linux-pci@vger.kernel.org, Dave Airlie, Toshi Kani, Suresh Siddha,
Linus Torvalds, Thomas Gleixner, Juergen Gross, Daniel Vetter,
Antonino Daplas, Jean-Christophe Plagniol-Villard, Rob Clark,
Mathias Krause, Andrzej Hajda, Mel Gorman, Vlastimil Babka,
Davidlohr Bueso
In-Reply-To: <20150626010927.GI3005@wotan.suse.de>
On Fri, Jun 26, 2015 at 03:09:27AM +0200, Luis R. Rodriguez wrote:
> Sure, mind this as a follow up patch if its too late?
No need, you can send me an updated one - I'll replace it.
Thanks.
--
Regards/Gruss,
Boris.
ECO tip #101: Trim your mails when you reply.
--
^ permalink raw reply
* RE: [PATCH v7 5/9] PCI: Add pci_iomap_wc() variants
From: Casey Leedom @ 2015-06-26 16:24 UTC (permalink / raw)
To: Benjamin Herrenschmidt
Cc: Arnd Bergmann, Luis R. Rodriguez, Michael S. Tsirkin,
Bjorn Helgaas, Toshi Kani, Andy Lutomirski, Juergen Gross,
Tomi Valkeinen, linux-pci@vger.kernel.org,
linux-kernel@vger.kernel.org, xen-devel@lists.xensource.com,
linux-fbdev, Suresh Siddha, Ingo Molnar, Thomas Gleixner,
Daniel Vetter, Dave Airlie, Antonino Daplas,
Jean-Christophe Plagniol-Villard, Dave Hansen,
venkatesh.pallipadi@intel.com, Stefan Bader,
ville.syrjala@linux.intel.com, David Vrabel, Jan Beulich,
Roger Pau Monné
In-Reply-To: <1435284123.3822.24.camel@kernel.crashing.org>
Thanks for looking into this Ben. As it stands now, it seems as
if Write Combined mappings simply aren't supported and/or all
driver writers trying to utilize Write Combined mappings have to
"hand roll" their own solutions which really isn't supportable.
One thing that might be considered is simply to treat the desire
to utilize the Write Combining hardware as a separate issue and
develop writel_wc(), writeq_wc(), etc. Those could be defined
as legal only for Write Combined Mappings and would "do the
right thing" for each architecture. The initial implementations
could simply be writel(), writeq(), etc. till each architecture'si
ssues were investigated and understood.
Additionally, it would be very useful to have some sort of
predicate which could be used to determine if an architecture
supported Write Combining. Our drivers would use this to
eliminate a wasteful attempt to use write combining on those
architectures where it didn't work.
Casey
________________________________________
From: Benjamin Herrenschmidt [benh@kernel.crashing.org]
Sent: Thursday, June 25, 2015 7:02 PM
To: Casey Leedom
Cc: Arnd Bergmann; Luis R. Rodriguez; Michael S. Tsirkin; Bjorn Helgaas; Toshi Kani; Andy Lutomirski; Juergen Gross; Tomi Valkeinen; linux-pci@vger.kernel.org; linux-kernel@vger.kernel.org; xen-devel@lists.xensource.com; linux-fbdev; Suresh Siddha; Ingo Molnar; Thomas Gleixner; Daniel Vetter; Dave Airlie; Antonino Daplas; Jean-Christophe Plagniol-Villard; Dave Hansen; venkatesh.pallipadi@intel.com; Stefan Bader; ville.syrjala@linux.intel.com; David Vrabel; Jan Beulich; Roger Pau Monné
Subject: Re: [PATCH v7 5/9] PCI: Add pci_iomap_wc() variants
On Thu, 2015-06-25 at 21:40 +0000, Casey Leedom wrote:
>
> Ah, thanks. I see now that the __raw_*() APIs don't do any of the
> Endian Swizzling. Unfortunately the *_relaxed() APIs on PowerPC
> are just defined as the normal *() routines. From
> arch/powerpc/include/asm/io.h:
>
> /*
> * We don't do relaxed operations yet, at least not with this
> semantic
> */
Yes so I was looking at this but there are some difficulties.
Architecturally, even with I=1 G=1 mappings (normal ioremap), we have no
guarantee of ordering of load vs. store unless I misunderstood
something. I think all current implementations provide some of that but
without barriers in the accessors, we aren't architecturally correct.
However, having those barriers will cause issues with G=0 (write
combine). It's unclear whether eieio() will provide the required
ordering for I=1 G=0 mappings and it will probably break write combine.
I'm looking into it with our HW guys and will try to come up with a
solution for power, but it doesn't help that our memory model conflates
write combining with other relaxations and that all our barriers also
prevent write combine.
Maybe we can bias the relaxed accessors toward write, by having no
barriers in it, and putting extra ones in reads.
Cheers,
Ben.
^ permalink raw reply
* Re: [PATCH v7 5/9] PCI: Add pci_iomap_wc() variants
From: Luis R. Rodriguez @ 2015-06-26 19:31 UTC (permalink / raw)
To: Benjamin Herrenschmidt
Cc: Casey Leedom, Arnd Bergmann, Michael S. Tsirkin, Bjorn Helgaas,
Toshi Kani, Andy Lutomirski, Juergen Gross, Tomi Valkeinen,
linux-pci@vger.kernel.org, linux-kernel@vger.kernel.org,
xen-devel@lists.xensource.com, linux-fbdev, Suresh Siddha,
Ingo Molnar, Thomas Gleixner, Daniel Vetter, Dave Airlie,
Antonino Daplas, Jean-Christophe Plagniol-Villard, Dave Hansen,
venkatesh.pallipadi@intel.com, Stefan Bader, ville.syrjala,
David Vrabel, Jan Beulich, Roger Pau Monné
In-Reply-To: <1435272718.3822.7.camel@kernel.crashing.org>
On Fri, Jun 26, 2015 at 08:51:58AM +1000, Benjamin Herrenschmidt wrote:
> On Thu, 2015-06-25 at 21:40 +0000, Casey Leedom wrote:
> > Hhmmm, so what do PowerPC Drivers do when they want to take
> > advantage of Write Combining? Do their own Endian Swizzling
> > with the __raw_*() APIs?
>
> Yeah either, we need to fix our relaxed implementation (patch
> welcome :-)
Yikes, so although powerpc has useful heuristics to automatically
enable WC the default write ops have been nullifying its effects?
Shouldn't this have been blatantly obvious in performance benchmarks
and expectations? If not a change should give considerable performance
gains... If the WC was nullified on powerpc then the experience and
value of the heuristics have less value and even if they are going
to be only considered for userspace mmap() it should be taken with
a bit grain of salt it seems.
Luis
^ permalink raw reply
* Re: [PATCH v7 5/9] PCI: Add pci_iomap_wc() variants
From: Benjamin Herrenschmidt @ 2015-06-26 22:00 UTC (permalink / raw)
To: Casey Leedom
Cc: Arnd Bergmann, Luis R. Rodriguez, Michael S. Tsirkin,
Bjorn Helgaas, Toshi Kani, Andy Lutomirski, Juergen Gross,
Tomi Valkeinen, linux-pci@vger.kernel.org,
linux-kernel@vger.kernel.org, xen-devel@lists.xensource.com,
linux-fbdev, Suresh Siddha, Ingo Molnar, Thomas Gleixner,
Daniel Vetter, Dave Airlie, Antonino Daplas,
Jean-Christophe Plagniol-Villard, Dave Hansen,
venkatesh.pallipadi@intel.com, Stefan Bader,
ville.syrjala@linux.intel.com, David Vrabel, Jan Beulich,
Roger Pau Monné
In-Reply-To: <4985EFDD773FCB459EF7915D2A3621ADC03621@nice.asicdesigners.com>
On Fri, 2015-06-26 at 16:24 +0000, Casey Leedom wrote:
> Thanks for looking into this Ben. As it stands now, it seems as
> if Write Combined mappings simply aren't supported and/or all
> driver writers trying to utilize Write Combined mappings have to
> "hand roll" their own solutions which really isn't supportable.
>
> One thing that might be considered is simply to treat the desire
> to utilize the Write Combining hardware as a separate issue and
> develop writel_wc(), writeq_wc(), etc. Those could be defined
> as legal only for Write Combined Mappings and would "do the
> right thing" for each architecture.
The question then is what is "the right thing". In the powerpc case,
we'll have a non-garded mapping, which means we also get no ordering
between load and stores.
> The initial implementations
> could simply be writel(), writeq(), etc. till each architecture'si
> ssues were investigated and understood.
>
> Additionally, it would be very useful to have some sort of
> predicate which could be used to determine if an architecture
> supported Write Combining. Our drivers would use this to
> eliminate a wasteful attempt to use write combining on those
> architectures where it didn't work.
>
> Casey
>
> ________________________________________
> From: Benjamin Herrenschmidt [benh@kernel.crashing.org]
> Sent: Thursday, June 25, 2015 7:02 PM
> To: Casey Leedom
> Cc: Arnd Bergmann; Luis R. Rodriguez; Michael S. Tsirkin; Bjorn Helgaas; Toshi Kani; Andy Lutomirski; Juergen Gross; Tomi Valkeinen; linux-pci@vger.kernel.org; linux-kernel@vger.kernel.org; xen-devel@lists.xensource.com; linux-fbdev; Suresh Siddha; Ingo Molnar; Thomas Gleixner; Daniel Vetter; Dave Airlie; Antonino Daplas; Jean-Christophe Plagniol-Villard; Dave Hansen; venkatesh.pallipadi@intel.com; Stefan Bader; ville.syrjala@linux.intel.com; David Vrabel; Jan Beulich; Roger Pau Monné
> Subject: Re: [PATCH v7 5/9] PCI: Add pci_iomap_wc() variants
>
> On Thu, 2015-06-25 at 21:40 +0000, Casey Leedom wrote:
> >
> > Ah, thanks. I see now that the __raw_*() APIs don't do any of the
> > Endian Swizzling. Unfortunately the *_relaxed() APIs on PowerPC
> > are just defined as the normal *() routines. From
> > arch/powerpc/include/asm/io.h:
> >
> > /*
> > * We don't do relaxed operations yet, at least not with this
> > semantic
> > */
>
> Yes so I was looking at this but there are some difficulties.
> Architecturally, even with I=1 G=1 mappings (normal ioremap), we have no
> guarantee of ordering of load vs. store unless I misunderstood
> something. I think all current implementations provide some of that but
> without barriers in the accessors, we aren't architecturally correct.
>
> However, having those barriers will cause issues with G=0 (write
> combine). It's unclear whether eieio() will provide the required
> ordering for I=1 G=0 mappings and it will probably break write combine.
>
> I'm looking into it with our HW guys and will try to come up with a
> solution for power, but it doesn't help that our memory model conflates
> write combining with other relaxations and that all our barriers also
> prevent write combine.
>
> Maybe we can bias the relaxed accessors toward write, by having no
> barriers in it, and putting extra ones in reads.
>
> Cheers,
> Ben.
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-pci" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH v7 5/9] PCI: Add pci_iomap_wc() variants
From: Benjamin Herrenschmidt @ 2015-06-26 22:04 UTC (permalink / raw)
To: Luis R. Rodriguez
Cc: Casey Leedom, Arnd Bergmann, Michael S. Tsirkin, Bjorn Helgaas,
Toshi Kani, Andy Lutomirski, Juergen Gross, Tomi Valkeinen,
linux-pci@vger.kernel.org, linux-kernel@vger.kernel.org,
xen-devel@lists.xensource.com, linux-fbdev, Suresh Siddha,
Ingo Molnar, Thomas Gleixner, Daniel Vetter, Dave Airlie,
Antonino Daplas, Jean-Christophe Plagniol-Villard, Dave Hansen,
venkatesh.pallipadi@intel.com, Stefan Bader, ville.syrjala,
David Vrabel, Jan Beulich, Roger Pau Monné
In-Reply-To: <20150626193158.GJ3005@wotan.suse.de>
On Fri, 2015-06-26 at 21:31 +0200, Luis R. Rodriguez wrote:
> > Yeah either, we need to fix our relaxed implementation (patch
> > welcome :-)
>
> Yikes, so although powerpc has useful heuristics to automatically
> enable WC the default write ops have been nullifying its effects?
The heuristic is for userspace mapping which don't use the kernel
accessors. To cut a long story short, this was all done back in the day
to speed up X.org which doesn't use fancy accessors with barriers to
access the framebuffer (neither does the kernel fbdev layer).
> Shouldn't this have been blatantly obvious in performance benchmarks
> and expectations?
> If not a change should give considerable performance
> gains... If the WC was nullified on powerpc then the experience and
> value of the heuristics have less value and even if they are going
> to be only considered for userspace mmap() it should be taken with
> a bit grain of salt it seems.
It wasn't nullified for the main user at the time, the fb. And I
mentioned an IB adapter or two for which the code had been hand tuned.
Cheers,
Ben.
^ permalink raw reply
* Re: [Xen-devel] [PATCH v7 5/9] PCI: Add pci_iomap_wc() variants
From: Benjamin Herrenschmidt @ 2015-06-26 23:54 UTC (permalink / raw)
To: Luis R. Rodriguez
Cc: Thomas Gleixner, Dave Airlie, Michael S. Tsirkin, Andy Lutomirski,
Bjorn Helgaas, Suresh Siddha, Ingo Molnar, Antonino Daplas,
linux-pci@vger.kernel.org, Roger Pau Monné,
linux-kernel@vger.kernel.org, Daniel Vetter, ville.syrjala,
venkatesh.pallipadi@intel.com, Jean-Christophe Plagniol-Villard,
Tomi Valkeinen, Dave Hansen, linux-fbdev, David Vrabel,
Casey Leedom, Juergen Gross, Toshi Kani,
xen-devel@lists.xensource.com, Stefan Bader, Arnd Bergmann,
Jan Beulich
In-Reply-To: <CAB=NE6XwczF6FYXV=RHyK=7ikVk_tGp+h9ouWPyORNENq4+=Kw@mail.gmail.com>
On Fri, 2015-06-26 at 15:41 -0700, Luis R. Rodriguez wrote:
> > It wasn't nullified for the main user at the time, the fb. And I
> > mentioned an IB adapter or two for which the code had been hand
> tuned.
>
> This still means there could be some affected drivers when used on
> powerpc, no?
Yes. In fact what about things like ARM who also have barriers in their
writel() ? Won't they also break WC ?
I'm trying to work with the architect and designers here to figure out
exactly where we stand and what we can do. As spelled out by our
architecture, things don't look great, because basically, we only have
attribute bit (garded) which when not set implies both WC and out of
order (& prefetch), and unclear barrier semantics in that case as well.
I *think* we might be able to settle with something along the lines of
"writel_relaxed() will allow combine on a WC mapping" but how I'm going
to get there is TBD.
It would be interesting to clarify the semantics of using the relaxed
accessors in combination with WC anyway. I wouldn't mind if the
definition involved also relaxing general ordering :-) It would
definitely make my life easier.
Cheers,
Ben.
^ permalink raw reply
* Re: [Xen-devel] [PATCH v7 5/9] PCI: Add pci_iomap_wc() variants
From: Luis R. Rodriguez @ 2015-06-27 1:16 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Russell King, Stefano Stabellini
Cc: linux-fbdev, Michael S. Tsirkin, linux-pci@vger.kernel.org,
Dave Hansen, Jan Beulich, Ville Syrjälä,
xen-devel@lists.xensource.com, Arnd Bergmann, Tomi Valkeinen,
Daniel Vetter, Dave Airlie, Ingo Molnar,
Jean-Christophe Plagniol-Villard, Casey Leedom, Antonino Daplas,
Suresh Siddha, Stefan Bader, Bjorn Helgaas, Thomas Gleixner,
Juergen Gross, Toshi Kani, linux-kernel@vger.kernel.org,
Andy Lutomirski, David Vrabel, venkatesh.pallipadi@intel.com,
Roger Pau Monné
In-Reply-To: <1435362871.26815.24.camel@kernel.crashing.org>
On Fri, Jun 26, 2015 at 4:54 PM, Benjamin Herrenschmidt
<benh@kernel.crashing.org> wrote:
> On Fri, 2015-06-26 at 15:41 -0700, Luis R. Rodriguez wrote:
>
>> > It wasn't nullified for the main user at the time, the fb. And I
>> > mentioned an IB adapter or two for which the code had been hand
>> tuned.
>>
>> This still means there could be some affected drivers when used on
>> powerpc, no?
>
> Yes. In fact what about things like ARM who also have barriers in their
> writel() ? Won't they also break WC ?
Not sure if they have that write-combine notion on their CPUs /
interconnects. Russel, Stefano, does ARM have write-combining ?
Luis
^ permalink raw reply
* [RFC v3 12/24] powerpc: Cleanup nvram includes
From: Finn Thain @ 2015-06-28 1:42 UTC (permalink / raw)
To: linux-kernel, linux-m68k, linuxppc-dev, Benjamin Herrenschmidt,
Paul Mackerras, Michael Ellerman, Arnd Bergmann,
Greg Kroah-Hartman, Jean-Christophe Plagniol-Villard,
Tomi Valkeinen, linux-fbdev
In-Reply-To: <20150628014159.732792697@telegraphics.com.au>
The nvram_read_byte() and nvram_write_byte() definitions in asm/nvram.h
duplicate those in linux/nvram.h. Get rid of the former to prepare for
adoption of struct arch_nvram_ops (which is defined in linux/nvram.h for
general use).
Signed-off-by: Finn Thain <fthain@telegraphics.com.au>
---
arch/powerpc/include/asm/nvram.h | 3 ---
arch/powerpc/kernel/setup_32.c | 1 +
drivers/char/generic_nvram.c | 4 +++-
drivers/video/fbdev/matrox/matroxfb_base.c | 2 +-
4 files changed, 5 insertions(+), 5 deletions(-)
Index: linux/arch/powerpc/include/asm/nvram.h
=================================--- linux.orig/arch/powerpc/include/asm/nvram.h 2015-06-28 11:41:27.000000000 +1000
+++ linux/arch/powerpc/include/asm/nvram.h 2015-06-28 11:41:41.000000000 +1000
@@ -101,7 +101,4 @@ extern int nvram_write_os_partition(stru
/* Determine NVRAM size */
extern ssize_t nvram_get_size(void);
-/* Normal access to NVRAM */
-extern unsigned char nvram_read_byte(int i);
-extern void nvram_write_byte(unsigned char c, int i);
#endif /* _ASM_POWERPC_NVRAM_H */
Index: linux/arch/powerpc/kernel/setup_32.c
=================================--- linux.orig/arch/powerpc/kernel/setup_32.c 2015-06-28 11:41:27.000000000 +1000
+++ linux/arch/powerpc/kernel/setup_32.c 2015-06-28 11:41:41.000000000 +1000
@@ -16,6 +16,7 @@
#include <linux/cpu.h>
#include <linux/console.h>
#include <linux/memblock.h>
+#include <linux/nvram.h>
#include <asm/io.h>
#include <asm/prom.h>
Index: linux/drivers/char/generic_nvram.c
=================================--- linux.orig/drivers/char/generic_nvram.c 2015-06-28 11:41:27.000000000 +1000
+++ linux/drivers/char/generic_nvram.c 2015-06-28 11:41:41.000000000 +1000
@@ -20,9 +20,11 @@
#include <linux/fcntl.h>
#include <linux/init.h>
#include <linux/mutex.h>
+#include <linux/nvram.h>
#include <asm/uaccess.h>
-#include <asm/nvram.h>
+
#ifdef CONFIG_PPC_PMAC
+#include <asm/nvram.h>
#include <asm/machdep.h>
#endif
Index: linux/drivers/video/fbdev/matrox/matroxfb_base.c
=================================--- linux.orig/drivers/video/fbdev/matrox/matroxfb_base.c 2015-06-28 11:41:27.000000000 +1000
+++ linux/drivers/video/fbdev/matrox/matroxfb_base.c 2015-06-28 11:41:41.000000000 +1000
@@ -111,12 +111,12 @@
#include "matroxfb_g450.h"
#include <linux/matroxfb.h>
#include <linux/interrupt.h>
+#include <linux/nvram.h>
#include <linux/slab.h>
#include <linux/uaccess.h>
#ifdef CONFIG_PPC_PMAC
#include <asm/machdep.h>
-unsigned char nvram_read_byte(int);
static int default_vmode = VMODE_NVRAM;
static int default_cmode = CMODE_NVRAM;
#endif
^ permalink raw reply
* [RFC v3 16/24] powerpc, fbdev: Use arch_nvram_ops methods instead of nvram_read_byte() and nvram_wri
From: Finn Thain @ 2015-06-28 1:42 UTC (permalink / raw)
To: linux-kernel, linux-m68k, linuxppc-dev, Benjamin Herrenschmidt,
Paul Mackerras, Michael Ellerman, Arnd Bergmann,
Greg Kroah-Hartman, Jean-Christophe Plagniol-Villard,
Tomi Valkeinen, linux-fbdev
In-Reply-To: <20150628014159.732792697@telegraphics.com.au>
Make use of arch_nvram_ops in device drivers so that the nvram_*
function exports can be removed.
Since they are no longer global symbols, rename the PPC32 nvram_* functions
appropriately.
Add the missing CONFIG_NVRAM test to imsttfb to avoid a build failure.
Signed-off-by: Finn Thain <fthain@telegraphics.com.au>
---
arch/powerpc/kernel/setup_32.c | 8 ++++----
drivers/char/generic_nvram.c | 4 ++--
drivers/video/fbdev/controlfb.c | 4 ++--
drivers/video/fbdev/imsttfb.c | 7 +++----
drivers/video/fbdev/matrox/matroxfb_base.c | 2 +-
drivers/video/fbdev/platinumfb.c | 4 ++--
drivers/video/fbdev/valkyriefb.c | 4 ++--
7 files changed, 16 insertions(+), 17 deletions(-)
Index: linux/arch/powerpc/kernel/setup_32.c
=================================--- linux.orig/arch/powerpc/kernel/setup_32.c 2015-06-28 11:41:44.000000000 +1000
+++ linux/arch/powerpc/kernel/setup_32.c 2015-06-28 11:41:46.000000000 +1000
@@ -170,20 +170,18 @@ __setup("l3cr=", ppc_setup_l3cr);
#ifdef CONFIG_GENERIC_NVRAM
-unsigned char nvram_read_byte(int addr)
+static unsigned char ppc_nvram_read_byte(int addr)
{
if (ppc_md.nvram_read_val)
return ppc_md.nvram_read_val(addr);
return 0xff;
}
-EXPORT_SYMBOL(nvram_read_byte);
-void nvram_write_byte(unsigned char val, int addr)
+static void ppc_nvram_write_byte(unsigned char val, int addr)
{
if (ppc_md.nvram_write_val)
ppc_md.nvram_write_val(addr, val);
}
-EXPORT_SYMBOL(nvram_write_byte);
static ssize_t ppc_nvram_get_size(void)
{
@@ -200,6 +198,8 @@ static long ppc_nvram_sync(void)
}
const struct nvram_ops arch_nvram_ops = {
+ .read_byte = ppc_nvram_read_byte,
+ .write_byte = ppc_nvram_write_byte,
.get_size = ppc_nvram_get_size,
.sync = ppc_nvram_sync,
};
Index: linux/drivers/char/generic_nvram.c
=================================--- linux.orig/drivers/char/generic_nvram.c 2015-06-28 11:41:44.000000000 +1000
+++ linux/drivers/char/generic_nvram.c 2015-06-28 11:41:46.000000000 +1000
@@ -64,7 +64,7 @@ static ssize_t read_nvram(struct file *f
if (*ppos >= nvram_len)
return 0;
for (i = *ppos; count > 0 && i < nvram_len; ++i, ++p, --count)
- if (__put_user(nvram_read_byte(i), p))
+ if (__put_user(arch_nvram_ops.read_byte(i), p))
return -EFAULT;
*ppos = i;
return p - buf;
@@ -84,7 +84,7 @@ static ssize_t write_nvram(struct file *
for (i = *ppos; count > 0 && i < nvram_len; ++i, ++p, --count) {
if (__get_user(c, p))
return -EFAULT;
- nvram_write_byte(c, i);
+ arch_nvram_ops.write_byte(c, i);
}
*ppos = i;
return p - buf;
Index: linux/drivers/video/fbdev/controlfb.c
=================================--- linux.orig/drivers/video/fbdev/controlfb.c 2015-06-28 11:41:27.000000000 +1000
+++ linux/drivers/video/fbdev/controlfb.c 2015-06-28 11:41:46.000000000 +1000
@@ -415,7 +415,7 @@ static int __init init_control(struct fb
/* Try to pick a video mode out of NVRAM if we have one. */
#ifdef CONFIG_NVRAM
if (default_cmode = CMODE_NVRAM) {
- cmode = nvram_read_byte(NV_CMODE);
+ cmode = arch_nvram_ops.read_byte(NV_CMODE);
if(cmode < CMODE_8 || cmode > CMODE_32)
cmode = CMODE_8;
} else
@@ -423,7 +423,7 @@ static int __init init_control(struct fb
cmodeÞfault_cmode;
#ifdef CONFIG_NVRAM
if (default_vmode = VMODE_NVRAM) {
- vmode = nvram_read_byte(NV_VMODE);
+ vmode = arch_nvram_ops.read_byte(NV_VMODE);
if (vmode < 1 || vmode > VMODE_MAX ||
control_mac_modes[vmode - 1].m[full] < cmode) {
sense = read_control_sense(p);
Index: linux/drivers/video/fbdev/matrox/matroxfb_base.c
=================================--- linux.orig/drivers/video/fbdev/matrox/matroxfb_base.c 2015-06-28 11:41:41.000000000 +1000
+++ linux/drivers/video/fbdev/matrox/matroxfb_base.c 2015-06-28 11:41:46.000000000 +1000
@@ -1888,7 +1888,7 @@ static int initMatrox2(struct matrox_fb_
default_vmode = VMODE_640_480_60;
#ifdef CONFIG_NVRAM
if (default_cmode = CMODE_NVRAM)
- default_cmode = nvram_read_byte(NV_CMODE);
+ default_cmode = arch_nvram_ops.read_byte(NV_CMODE);
#endif
if (default_cmode < CMODE_8 || default_cmode > CMODE_32)
default_cmode = CMODE_8;
Index: linux/drivers/video/fbdev/platinumfb.c
=================================--- linux.orig/drivers/video/fbdev/platinumfb.c 2015-06-28 11:41:27.000000000 +1000
+++ linux/drivers/video/fbdev/platinumfb.c 2015-06-28 11:41:46.000000000 +1000
@@ -349,7 +349,7 @@ static int platinum_init_fb(struct fb_in
printk(KERN_INFO "platinumfb: Monitor sense value = 0x%x, ", sense);
if (default_vmode = VMODE_NVRAM) {
#ifdef CONFIG_NVRAM
- default_vmode = nvram_read_byte(NV_VMODE);
+ default_vmode = arch_nvram_ops.read_byte(NV_VMODE);
if (default_vmode <= 0 || default_vmode > VMODE_MAX ||
!platinum_reg_init[default_vmode-1])
#endif
@@ -362,7 +362,7 @@ static int platinum_init_fb(struct fb_in
default_vmode = VMODE_640_480_60;
#ifdef CONFIG_NVRAM
if (default_cmode = CMODE_NVRAM)
- default_cmode = nvram_read_byte(NV_CMODE);
+ default_cmode = arch_nvram_ops.read_byte(NV_CMODE);
#endif
if (default_cmode < CMODE_8 || default_cmode > CMODE_32)
default_cmode = CMODE_8;
Index: linux/drivers/video/fbdev/valkyriefb.c
=================================--- linux.orig/drivers/video/fbdev/valkyriefb.c 2015-06-28 11:41:27.000000000 +1000
+++ linux/drivers/video/fbdev/valkyriefb.c 2015-06-28 11:41:46.000000000 +1000
@@ -287,7 +287,7 @@ static void __init valkyrie_choose_mode(
/* Try to pick a video mode out of NVRAM if we have one. */
#if !defined(CONFIG_MAC) && defined(CONFIG_NVRAM)
if (default_vmode = VMODE_NVRAM) {
- default_vmode = nvram_read_byte(NV_VMODE);
+ default_vmode = arch_nvram_ops.read_byte(NV_VMODE);
if (default_vmode <= 0
|| default_vmode > VMODE_MAX
|| !valkyrie_reg_init[default_vmode - 1])
@@ -300,7 +300,7 @@ static void __init valkyrie_choose_mode(
default_vmode = VMODE_640_480_67;
#if !defined(CONFIG_MAC) && defined(CONFIG_NVRAM)
if (default_cmode = CMODE_NVRAM)
- default_cmode = nvram_read_byte(NV_CMODE);
+ default_cmode = arch_nvram_ops.read_byte(NV_CMODE);
#endif
/*
Index: linux/drivers/video/fbdev/imsttfb.c
=================================--- linux.orig/drivers/video/fbdev/imsttfb.c 2015-06-28 11:41:27.000000000 +1000
+++ linux/drivers/video/fbdev/imsttfb.c 2015-06-28 11:41:46.000000000 +1000
@@ -328,7 +328,6 @@ enum {
TVP = 1
};
-#define USE_NV_MODES 1
#define INIT_BPP 8
#define INIT_XRES 640
#define INIT_YRES 480
@@ -1391,17 +1390,17 @@ static void init_imstt(struct fb_info *i
}
}
-#if USE_NV_MODES && defined(CONFIG_PPC32)
+#if defined(CONFIG_NVRAM) && defined(CONFIG_PPC32)
{
int vmode = init_vmode, cmode = init_cmode;
if (vmode = -1) {
- vmode = nvram_read_byte(NV_VMODE);
+ vmode = arch_nvram_ops.read_byte(NV_VMODE);
if (vmode <= 0 || vmode > VMODE_MAX)
vmode = VMODE_640_480_67;
}
if (cmode = -1) {
- cmode = nvram_read_byte(NV_CMODE);
+ cmode = arch_nvram_ops.read_byte(NV_CMODE);
if (cmode < CMODE_8 || cmode > CMODE_32)
cmode = CMODE_8;
}
^ permalink raw reply
* Re: [PATCH v2] backlight: lp8788: Deletion of a check before backlight_device_unregister()
From: SF Markus Elfring @ 2015-06-28 12:07 UTC (permalink / raw)
To: Lee Jones, Bryan Wu, Jean-Christophe Plagniol-Villard, Jingoo Han,
Tomi Valkeinen, linux-fbdev
Cc: LKML, kernel-janitors, Julia Lawall
In-Reply-To: <5473736F.8070705@users.sourceforge.net>
> From: Markus Elfring <elfring@users.sourceforge.net>
>
> The backlight_device_unregister() function tests whether its argument is NULL
> and then returns immediately. Thus the test around the call is not needed.
>
> This issue was detected by using the Coccinelle software.
>
> Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
> ---
> drivers/video/backlight/lp8788_bl.c | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/drivers/video/backlight/lp8788_bl.c b/drivers/video/backlight/lp8788_bl.c
> index d6c4f6a..24a055c 100644
> --- a/drivers/video/backlight/lp8788_bl.c
> +++ b/drivers/video/backlight/lp8788_bl.c
> @@ -221,8 +221,7 @@ static void lp8788_backlight_unregister(struct lp8788_bl *bl)
> {
> struct backlight_device *bl_dev = bl->bl_dev;
>
> - if (bl_dev)
> - backlight_device_unregister(bl_dev);
> + backlight_device_unregister(bl_dev);
> }
>
> static ssize_t lp8788_get_bl_ctl_mode(struct device *dev,
>
Would you like to integrate this update suggestion
into another source code repository?
Regards,
Markus
^ permalink raw reply
* [PATCH] video: fbdev: omap2: displays-new: Delete a check before backlight_device_unregister()
From: SF Markus Elfring @ 2015-06-28 12:36 UTC (permalink / raw)
To: Jean-Christophe Plagniol-Villard, Tomi Valkeinen, linux-omap,
linux-fbdev
Cc: LKML, kernel-janitors, Julia Lawall
In-Reply-To: <5317A59D.4@users.sourceforge.net>
From: Markus Elfring <elfring@users.sourceforge.net>
Date: Sun, 28 Jun 2015 14:30:17 +0200
The backlight_device_unregister() function tests whether its argument is NULL
and then returns immediately. Thus the test around the call is not needed.
This issue was detected by using the Coccinelle software.
Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
drivers/video/fbdev/omap2/displays-new/panel-dsi-cm.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
diff --git a/drivers/video/fbdev/omap2/displays-new/panel-dsi-cm.c b/drivers/video/fbdev/omap2/displays-new/panel-dsi-cm.c
index 3414c26..d2caa41 100644
--- a/drivers/video/fbdev/omap2/displays-new/panel-dsi-cm.c
+++ b/drivers/video/fbdev/omap2/displays-new/panel-dsi-cm.c
@@ -1323,8 +1323,7 @@ static int dsicm_probe(struct platform_device *pdev)
return 0;
err_sysfs_create:
- if (bldev != NULL)
- backlight_device_unregister(bldev);
+ backlight_device_unregister(bldev);
err_bl:
destroy_workqueue(ddata->workqueue);
err_reg:
--
2.4.4
^ permalink raw reply related
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