Linux Framebuffer Layer development
 help / color / mirror / Atom feed
* Re: [PATCH v1, TRIVIAL] Fix pxafb nonstd field documentation and add errata info
From: Tomi Valkeinen @ 2015-08-21  6:53 UTC (permalink / raw)
  To: linux-arm-kernel
In-Reply-To: <55D63391.3080403@tul.cz>

[-- Attachment #1: Type: text/plain, Size: 1753 bytes --]



On 20/08/15 23:07, Petr Cvek wrote:
> Dne 20.8.2015 v 12:55 Tomi Valkeinen napsal(a):
>>
>>
>> On 08/08/15 21:20, Petr Cvek wrote:
>>
>>> diff --git a/drivers/video/fbdev/Kconfig b/drivers/video/fbdev/Kconfig
>>> index 2d98de5..8f485ec 100644
>>> --- a/drivers/video/fbdev/Kconfig
>>> +++ b/drivers/video/fbdev/Kconfig
>>> @@ -1893,6 +1893,11 @@ config FB_PXA_OVERLAY
>>>  	bool "Support PXA27x/PXA3xx Overlay(s) as framebuffer"
>>>  	default n
>>>  	depends on FB_PXA && (PXA27x || PXA3xx)
>>> +	---help---
>>> +	  Using the overlay 2 and YUV420 mode on a PXA27x C0 chip revision will
>>> +	  hang the LCD until the next SoC restart (errata E24).
>>
>> Shouldn't the driver handle this? It should not allow the combination to
>> be used.
>>
>>  Tomi
>>
> 
> Probably, but I don't know which revisions are OK and which are not. Errata document:
> 
> 	Marvell® PXA270M Processor Specification Update
> 	MV-S900957-00
> 
> has only 3 revisions, but PXA Manual lists 6 (2.2.5.1 Processor ID Register) and one revision from errata is not shown in my PXA Manual. I can exclude mine revision (from "system_rev" variable), but there should be some warning somewhere in configuration.

No, configuration or kernel docs is not the right place for this. The
user is not going to see that. And a single kernel image is supposed to
run on different soc/board versions, so you can't do selections like
this in the kernel config.

The driver should detect the bad revision, and then apply the
workaround, or if that's not possible, refuse the use of the
configuration that leads to the HW bug. Or if even that's not possible,
the driver should print a warning when starting, so that the user is
able to see it.

 Tomi


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

^ permalink raw reply

* Re: [RFC] fbdev/riva:change to use generice function to implement reverse_order()
From: yalin wang @ 2015-08-21  7:46 UTC (permalink / raw)
  To: Tomi Valkeinen; +Cc: adaplas, plagnioj, linux-fbdev, open list
In-Reply-To: <55D6C812.6080400@ti.com>


> On Aug 21, 2015, at 14:41, Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:
> 
> 
> 
> On 20/08/15 14:30, yalin wang wrote:
>> 
>>> On Aug 20, 2015, at 19:02, Tomi Valkeinen <tomi.valkeinen@ti.com> wrote:
>>> 
>>> 
>>> On 10/08/15 13:12, yalin wang wrote:
>>>> This change to use swab32(bitrev32()) to implement reverse_order()
>>>> function, have better performance on some platforms.
>>> 
>>> Which platforms? Presuming you tested this, roughly how much better
>>> performance? If you didn't, how do you know it's faster?
>> 
>> i investigate on arm64 platforms:
> 
> Ok. So is any arm64 platform actually using these devices? If these
> devices are mostly used by 32bit x86 platforms, optimizing them for
> arm64 doesn't make any sense.
> 
> Possibly the patches are still good for x86 also, but that needs to be
> proven.
> 
not exactly, because x86_64 don’t have hardware instruction to do rbit OP,
i compile by test :

use the patch:
  use swab32(bitrev32()):
  2775:       0f b6 d0                movzbl %al,%edx                                                                                                                                                    
  2778:       0f b6 c4                movzbl %ah,%eax
  277b:       0f b6 92 00 00 00 00    movzbl 0x0(%rdx),%edx
  2782:       0f b6 80 00 00 00 00    movzbl 0x0(%rax),%eax
  2789:       c1 e2 08                shl    $0x8,%edx
  278c:       09 d0                   or     %edx,%eax
  278e:       0f b6 d5                movzbl %ch,%edx
  2791:       0f b6 c9                movzbl %cl,%ecx
  2794:       0f b6 89 00 00 00 00    movzbl 0x0(%rcx),%ecx
  279b:       0f b6 92 00 00 00 00    movzbl 0x0(%rdx),%edx
  27a2:       0f b7 c0                movzwl %ax,%eax
  27a5:       c1 e1 08                shl    $0x8,%ecx
  27a8:       09 ca                   or     %ecx,%edx
  27aa:       c1 e2 10                shl    $0x10,%edx
  27ad:       09 d0                   or     %edx,%eax
  27af:       45 85 ff                test   %r15d,%r15d
  27b2:       0f c8                   bswap  %eax
4 memory access instructions,



without the patch:
use
do {                            \
-       u8 *a = (u8 *)(l);      \
-       a[0] = bitrev8(a[0]);   \
-       a[1] = bitrev8(a[1]);   \
-       a[2] = bitrev8(a[2]);   \
-       a[3] = bitrev8(a[3]);   \
-} while(0)



    277b:       45 0f b6 80 00 00 00    movzbl 0x0(%r8),%r8d
    2782:       00 
    2783:       c1 ee 10                shr    $0x10,%esi
    2786:       89 f2                   mov    %esi,%edx
    2788:       0f b6 f4                movzbl %ah,%esi
    278b:       c1 e8 18                shr    $0x18,%eax
    278e:       0f b6 d2                movzbl %dl,%edx
    2791:       48 98                   cltq   
    2793:       45 85 ed                test   %r13d,%r13d
    2796:       0f b6 92 00 00 00 00    movzbl 0x0(%rdx),%edx
    279d:       0f b6 80 00 00 00 00    movzbl 0x0(%rax),%eax
    27a4:       44 88 85 54 ff ff ff    mov    %r8b,-0xac(%rbp)
    27ab:       44 0f b6 86 00 00 00    movzbl 0x0(%rsi),%r8d
    27b2:       00 
    27b3:       88 95 56 ff ff ff       mov    %dl,-0xaa(%rbp)
    27b9:       88 85 57 ff ff ff       mov    %al,-0xa9(%rbp)
    27bf:       44 88 85 55 ff ff ff    mov    %r8b,-0xab(%rbp)

6 memory access instructions, and generate more code that the patch .

because the original code use byte access 4 times , i don’t
think have better performance. :)

Thanks







^ permalink raw reply

* Re: [RFC] fbdev/riva:change to use generice function to implement reverse_order()
From: Tomi Valkeinen @ 2015-08-21  8:01 UTC (permalink / raw)
  To: yalin wang; +Cc: adaplas, plagnioj, linux-fbdev, open list
In-Reply-To: <4DCC50F3-9B6D-4A3A-9693-E7A7196564A8@gmail.com>

[-- Attachment #1: Type: text/plain, Size: 726 bytes --]



On 21/08/15 10:46, yalin wang wrote:

>>> i investigate on arm64 platforms:
>>
>> Ok. So is any arm64 platform actually using these devices? If these
>> devices are mostly used by 32bit x86 platforms, optimizing them for
>> arm64 doesn't make any sense.
>>
>> Possibly the patches are still good for x86 also, but that needs to be
>> proven.
>>
> not exactly, because x86_64 don’t have hardware instruction to do rbit OP,
> i compile by test :

For old drivers i386 may be more relevant than x86_64.

So you don't have the actual HW?

These kind of optimizations should have some real world measurements,
not just compiling, looking at the assembly and guessing whether it's
faster or not.

 Tomi


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

^ permalink raw reply

* Re: omapdss: Division by zero in kernel
From: Tomi Valkeinen @ 2015-08-21  8:42 UTC (permalink / raw)
  To: Pali Rohár, Jean-Christophe Plagniol-Villard, Jyri Sarha
  Cc: linux-omap, linux-fbdev, linux-kernel, Pavel Machek,
	Sebastian Reichel, Aaro Koskinen, Tony Lindgren, Nishanth Menon
In-Reply-To: <201507241803.42891@pali>

[-- Attachment #1: Type: text/plain, Size: 829 bytes --]



On 24/07/15 19:03, Pali Rohár wrote:
> Hello,
> 
> when on N900 (real HW or qemu) I run this command
> 
> / # echo 0 > /sys/devices/platform/omapdss/overlay0/enabled && echo 0 > /sys/class/graphics/fb0/size
> 
> then kernel crash with this error message
> 
> / # [   29.904113] Division by zero in kernel.

The problem is that fb console uses the kernel mmapped framebuffer, but
omapfb is not aware of the fb console. So the above commands free the
framebuffer, as omapfb thinks no one is using it, and then fb console
tries to touch the fb.

omapfb tracks mmaps from userspace, and refuses to free a fb it it's
mmapped.

I don't know how to fix it straight away. Maybe there's a way for omapfb
to check if the fbcon uses the fb in question, and if so, refuses to
release/resize the memory.

 Tomi


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

^ permalink raw reply

* Re: omapdss: Division by zero in kernel
From: Pali Rohár @ 2015-08-21  8:48 UTC (permalink / raw)
  To: Tomi Valkeinen
  Cc: Jean-Christophe Plagniol-Villard, Jyri Sarha, linux-omap,
	linux-fbdev, linux-kernel, Pavel Machek, Sebastian Reichel,
	Aaro Koskinen, Tony Lindgren, Nishanth Menon
In-Reply-To: <55D6E466.5060706@ti.com>

On Friday 21 August 2015 11:42:14 Tomi Valkeinen wrote:
> 
> 
> On 24/07/15 19:03, Pali Rohár wrote:
> > Hello,
> > 
> > when on N900 (real HW or qemu) I run this command
> > 
> > / # echo 0 > /sys/devices/platform/omapdss/overlay0/enabled && echo 0 > /sys/class/graphics/fb0/size
> > 
> > then kernel crash with this error message
> > 
> > / # [   29.904113] Division by zero in kernel.
> 

Hi! Thanks for explaining.

> The problem is that fb console uses the kernel mmapped framebuffer, but
> omapfb is not aware of the fb console. So the above commands free the
> framebuffer, as omapfb thinks no one is using it, and then fb console
> tries to touch the fb.
> 

What about refusing those calls from fb console? So fb console will not
know about this problem and omapfb will just ignore drawn functions?

> omapfb tracks mmaps from userspace, and refuses to free a fb it it's
> mmapped.
> 
> I don't know how to fix it straight away. Maybe there's a way for omapfb
> to check if the fbcon uses the fb in question, and if so, refuses to
> release/resize the memory.
> 
>  Tomi
> 

Maemo userspace (on Nokia N900) uses above commands to initialize
graphic and Xserver. So it would be nice if disabling framebuffer would
work even if fbcon.ko is loaded (or compiled directly into zImage).

-- 
Pali Rohár
pali.rohar@gmail.com

^ permalink raw reply

* [patch] fbdev: fix snprintf() limit in show_bl_curve()
From: Dan Carpenter @ 2015-08-21  8:53 UTC (permalink / raw)
  To: linux-fbdev

The limit should be "PAGE_SIZE - len" instead of PAGE_SIZE.

Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>

diff --git a/drivers/video/fbdev/core/fbsysfs.c b/drivers/video/fbdev/core/fbsysfs.c
index 60c3f0a..827098d 100644
--- a/drivers/video/fbdev/core/fbsysfs.c
+++ b/drivers/video/fbdev/core/fbsysfs.c
@@ -485,7 +485,7 @@ static ssize_t show_bl_curve(struct device *device,
 
 	mutex_lock(&fb_info->bl_curve_mutex);
 	for (i = 0; i < FB_BACKLIGHT_LEVELS; i += 8)
-		len += snprintf(&buf[len], PAGE_SIZE, "%8ph\n",
+		len += snprintf(&buf[len], PAGE_SIZE - len, "%8ph\n",
 				fb_info->bl_curve + i);
 	mutex_unlock(&fb_info->bl_curve_mutex);
 

^ permalink raw reply related

* Re: omapdss: Division by zero in kernel
From: Tomi Valkeinen @ 2015-08-21  9:17 UTC (permalink / raw)
  To: Pali Rohár
  Cc: Jean-Christophe Plagniol-Villard, Jyri Sarha, linux-omap,
	linux-fbdev, linux-kernel, Pavel Machek, Sebastian Reichel,
	Aaro Koskinen, Tony Lindgren, Nishanth Menon
In-Reply-To: <20150821084807.GD27391@pali>

[-- Attachment #1: Type: text/plain, Size: 1901 bytes --]



On 21/08/15 11:48, Pali Rohár wrote:
> On Friday 21 August 2015 11:42:14 Tomi Valkeinen wrote:
>>
>>
>> On 24/07/15 19:03, Pali Rohár wrote:
>>> Hello,
>>>
>>> when on N900 (real HW or qemu) I run this command
>>>
>>> / # echo 0 > /sys/devices/platform/omapdss/overlay0/enabled && echo 0 > /sys/class/graphics/fb0/size
>>>
>>> then kernel crash with this error message
>>>
>>> / # [   29.904113] Division by zero in kernel.
>>
> 
> Hi! Thanks for explaining.
> 
>> The problem is that fb console uses the kernel mmapped framebuffer, but
>> omapfb is not aware of the fb console. So the above commands free the
>> framebuffer, as omapfb thinks no one is using it, and then fb console
>> tries to touch the fb.
>>
> 
> What about refusing those calls from fb console? So fb console will not
> know about this problem and omapfb will just ignore drawn functions?

Hmm, I'm not sure I understand what you mean... omapfb is not drawing
anything, fbcon is doing the drawing independently to the fb. And the fb
suddenly disappears without fbcon realizing that.

>> omapfb tracks mmaps from userspace, and refuses to free a fb it it's
>> mmapped.
>>
>> I don't know how to fix it straight away. Maybe there's a way for omapfb
>> to check if the fbcon uses the fb in question, and if so, refuses to
>> release/resize the memory.
>>
>>  Tomi
>>
> 
> Maemo userspace (on Nokia N900) uses above commands to initialize
> graphic and Xserver. So it would be nice if disabling framebuffer would
> work even if fbcon.ko is loaded (or compiled directly into zImage).

Ok. And N900 has fbcon enabled? I wonder how it survives...

fbcon can be unbound from userspace with something like:

echo 0 > /sys/class/vtconsole/vtcon1/bind

After that I think the memory can be freed.

But obviously the kernel should not crash here, no question about that.

 Tomi


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

^ permalink raw reply

* Re: [patch] fbdev: fix snprintf() limit in show_bl_curve()
From: Andy Shevchenko @ 2015-08-21  9:22 UTC (permalink / raw)
  To: linux-fbdev
In-Reply-To: <20150821085302.GE25369@mwanda>

On Fri, 2015-08-21 at 11:53 +0300, Dan Carpenter wrote:
> The limit should be "PAGE_SIZE - len" instead of PAGE_SIZE.
> 

Besides that was in the original code, the problem still might happen
when FB_BACKLIGHT_LEVELS is set to 171+ since snprintf() returns
desired length. I suppose you would change this to check len on each
iteration or change to scnprintf() if I get it correct.

 
> diff --git a/drivers/video/fbdev/core/fbsysfs.c 
> b/drivers/video/fbdev/core/fbsysfs.c
> index 60c3f0a..827098d 100644
> --- a/drivers/video/fbdev/core/fbsysfs.c
> +++ b/drivers/video/fbdev/core/fbsysfs.c
> @@ -485,7 +485,7 @@ static ssize_t show_bl_curve(struct device 
> *device,
>  
>  	mutex_lock(&fb_info->bl_curve_mutex);
>  	for (i = 0; i < FB_BACKLIGHT_LEVELS; i += 8)
> -		len += snprintf(&buf[len], PAGE_SIZE, "%8ph\n",
> +		len += snprintf(&buf[len], PAGE_SIZE - len, 
> "%8ph\n",
>  				fb_info->bl_curve + i);
>  	mutex_unlock(&fb_info->bl_curve_mutex);
>  

-- 
Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Intel Finland Oy

^ permalink raw reply

* Re: [patch] fbdev: fix snprintf() limit in show_bl_curve()
From: Dan Carpenter @ 2015-08-21  9:53 UTC (permalink / raw)
  To: linux-fbdev
In-Reply-To: <20150821085302.GE25369@mwanda>

On Fri, Aug 21, 2015 at 12:22:03PM +0300, Andy Shevchenko wrote:
> On Fri, 2015-08-21 at 11:53 +0300, Dan Carpenter wrote:
> > The limit should be "PAGE_SIZE - len" instead of PAGE_SIZE.
> > 
> 
> Besides that was in the original code, the problem still might happen
> when FB_BACKLIGHT_LEVELS is set to 171+ since snprintf() returns
> desired length. I suppose you would change this to check len on each
> iteration or change to scnprintf() if I get it correct.
> 

Yeah, you're right.

If you pass a negative size to snprintf() it won't overflow, but it will
print an error.  I will send a v2 and change it to scnprintf().  I don't
think we will have 171 levels so it's not worth adding the check?

regards,
dan carpenter


^ permalink raw reply

* Re: [patch] fbdev: fix snprintf() limit in show_bl_curve()
From: Andy Shevchenko @ 2015-08-21 11:26 UTC (permalink / raw)
  To: linux-fbdev
In-Reply-To: <20150821085302.GE25369@mwanda>

On Fri, 2015-08-21 at 12:53 +0300, Dan Carpenter wrote:
> On Fri, Aug 21, 2015 at 12:22:03PM +0300, Andy Shevchenko wrote:
> > On Fri, 2015-08-21 at 11:53 +0300, Dan Carpenter wrote:
> > > The limit should be "PAGE_SIZE - len" instead of PAGE_SIZE.
> > > 
> > 
> > Besides that was in the original code, the problem still might 
> > happen
> > when FB_BACKLIGHT_LEVELS is set to 171+ since snprintf() returns
> > desired length. I suppose you would change this to check len on 
> > each
> > iteration or change to scnprintf() if I get it correct.
> > 
> 
> Yeah, you're right.
> 
> If you pass a negative size to snprintf() it won't overflow, but it 
> will
> print an error.  I will send a v2 and change it to scnprintf().  I 
> don't
> think we will have 171 levels so it's not worth adding the check?

I think there is no need to check for that. Especially it might be
changed in the future (if, for example, they decide to have 16 levels
per line instead of 8).

> 
> regards,
> dan carpenter
> 

-- 
Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Intel Finland Oy

^ permalink raw reply

* Re: [RFC] fbdev/riva:change to use generice function to implement reverse_order()
From: Afzal Mohammed @ 2015-08-22  7:53 UTC (permalink / raw)
  To: Tomi Valkeinen; +Cc: yalin wang, adaplas, plagnioj, linux-fbdev, open list
In-Reply-To: <55D6DAE5.20304@ti.com>

Hi,

On Fri, Aug 21, 2015 at 11:01:41AM +0300, Tomi Valkeinen wrote:

> >> Possibly the patches are still good for x86 also, but that needs to be
> >> proven.
> >>
> > not exactly, because x86_64 don’t have hardware instruction to do rbit OP,
> > i compile by test :
> 
> For old drivers i386 may be more relevant than x86_64.

It seems asm bit reversal is supported in Kernel on arm & arm64 only,
not sure whether any other arch even provide asm bit reversal
instruction.

> These kind of optimizations should have some real world measurements,

Not for this case, but once measured on ARM, iirc, a 32-bit asm bit
reversal as compared to doing it in C was taking 1 cycle as opposed to
~225 cycles!, of course writing optimized C could have made it fare
better, but still would reach no-way near asm bit reversal.

Regards
Afzal

^ permalink raw reply

* halo
From: barr lewis @ 2015-08-22 11:38 UTC (permalink / raw)


let us talk

^ permalink raw reply

* [PATCH 00/11] fix error return code
From: Julia Lawall @ 2015-08-23  0:11 UTC (permalink / raw)
  To: linux-samsung-soc
  Cc: kernel-janitors, linux-fbdev, linux-arm-kernel, linux-kernel,
	linux-omap, linux-mmc, netdev, dri-devel, linux-input, linux-gpio

The complate semantic patch that finds this problem is as follows:
(http://coccinelle.lip6.fr/)

// <smpl>
@ok exists@
identifier f,ret,i;
expression e;
constant c;
@@

// identify a function that returns a negative return value at least once.
f(...) {
... when any
(
return -c@i;
|
ret = -c@i;
... when != ret = e
return ret;
|
if (ret < 0) { ... return ret; }
)
... when any
}

@r exists@
identifier ret,ok.f,fn;
expression e1,e2,e3,e4,e5,e6,x;
statement S,S1;
position p1,p2,p3;
@@

// identify a case where the return variable is set to a non-negative value
// and then returned in error-handling code
f(...) {
... when any
(
if@p1 (\(ret < 0\|ret != 0\))
 { ... return ret; }
|
ret@p1 = 0
)
... when != \(ret = e1\|ret++\|ret--\|ret+á\|ret-á\)
    when != &ret
    when any
(
 if (<+... ret = e5 ...+>) S1
|
 if (<+... &ret ...+>) S1
|
if@p2(<+...x = fn(...)...+>)
 {
  ... when != ret = e6
      when forall
 return@p3 ret;
}
|
break;
|
x = fn(...)
... when != \(ret = e4\|ret++\|ret--\|ret+ä\|ret-ä\)
    when != &ret
(
 if (<+... ret = e3 ...+>) S
|
 if (<+... &ret ...+>) S
|
if@p2(<+...\(x != 0\|x < 0\|x = NULL\|IS_ERR(x)\)...+>)
 {
  ... when != ret = e2
      when forall
 return@p3 ret;
}
)
)
... when any
}

@printer depends on r@
position p;
identifier ok.f,pr;
constant char [] c;
@@

f(...) { <...pr@p(...,c,...)...> }

@bad0 exists@
identifier r.ret,ok.f,g != {ERR_PTR,IS_ERR};
position p != printer.p;
@@

f(...) { ... when any
g@p(...,ret,...)
... when any
 }

@bad depends on !bad0 exists@
position r.p1,r.p2;
statement S1,S2;
identifier r.ret;
expression e1;
@@

// ignore the above if there is some path where the variable is set to
// something else
(
if@p1 (\(ret < 0\|ret != 0\)) S1
|
ret@p1 = 0
)
... when any
 \(ret = e1\|ret++\|ret--\|ret+á\|ret-á\|&ret\)
... when any
if@p2(...) S2

@bad1 depends on !bad0 && !bad exists@
position r.p2;
statement S2;
identifier r.ret;
expression e1;
constant c;
@@

ret = -c
... when != \(ret = e1\|ret++\|ret--\|ret+á\|ret-á\)
    when != &ret
    when any
if@p2(...) S2

@bad2 depends on !bad0 && !bad && !bad1 exists@
position r.p1,r.p2;
identifier r.ret;
expression e1;
statement S2;
constant c;
@@

// likewise ignore it if there has been an intervening return
ret@p1 = 0
... when != if (...) { ... ret = e1 ... return ret; }
    when != if (...) { ... return -c; }
    when any
if@p2(...) S2

@script:python depends on !bad0 && !bad && !bad1 && !bad2@
p1 << r.p1;
p2 << r.p2;
p3 << r.p3;
@@

cocci.print_main("",p1)
cocci.print_secs("",p2)
cocci.print_secs("",p3)
// </smpl>


^ permalink raw reply

* [PATCH 04/11] fbdev: ssd1307fb: fix error return code
From: Julia Lawall @ 2015-08-23  0:11 UTC (permalink / raw)
  To: Jean-Christophe Plagniol-Villard
  Cc: kernel-janitors, Tomi Valkeinen, linux-fbdev, linux-kernel
In-Reply-To: <1440288682-12732-1-git-send-email-Julia.Lawall@lip6.fr>

Propagate error code on failure.  Also changed %ld to %d in dev_err to use
ret variable rather than putting two calls to PTR_ERR.

A simplified version of the semantic match that finds the first problem is as
follows: (http://coccinelle.lip6.fr/)

// <smpl>
@@
identifier ret; expression e1,e2;
@@
(
if (\(ret < 0\|ret != 0\))
 { ... return ret; }
|
ret = 0
)
... when != ret = e1
    when != &ret
*if(...)
{
  ... when != ret = e2
      when forall
 return ret;
}
// </smpl>

Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr>

---
 drivers/video/fbdev/ssd1307fb.c |    5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/drivers/video/fbdev/ssd1307fb.c b/drivers/video/fbdev/ssd1307fb.c
index 3e153c0..1611215 100644
--- a/drivers/video/fbdev/ssd1307fb.c
+++ b/drivers/video/fbdev/ssd1307fb.c
@@ -656,8 +656,9 @@ static int ssd1307fb_probe(struct i2c_client *client,
 	bl = backlight_device_register(bl_name, &client->dev, par,
 				       &ssd1307fb_bl_ops, NULL);
 	if (IS_ERR(bl)) {
-		dev_err(&client->dev, "unable to register backlight device: %ld\n",
-			PTR_ERR(bl));
+		ret = PTR_ERR(bl);
+		dev_err(&client->dev, "unable to register backlight device: %d\n",
+			ret);
 		goto bl_init_error;
 	}
 

^ permalink raw reply related

* [RFC v6 12/25] powerpc: Cleanup nvram includes
From: Finn Thain @ 2015-08-23 10:41 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: <20150823104129.517600532@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>

---

Changes since v4:
- Fix possible git bisect build failure with CONFIG_PPC_PMAC=n.

---
 arch/powerpc/include/asm/nvram.h           |    3 ---
 arch/powerpc/kernel/setup_32.c             |    1 +
 drivers/char/generic_nvram.c               |    1 +
 drivers/video/fbdev/matrox/matroxfb_base.c |    2 +-
 4 files changed, 3 insertions(+), 4 deletions(-)

Index: linux/arch/powerpc/include/asm/nvram.h
=================================--- linux.orig/arch/powerpc/include/asm/nvram.h	2015-08-23 20:40:53.000000000 +1000
+++ linux/arch/powerpc/include/asm/nvram.h	2015-08-23 20:41:07.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-08-23 20:40:53.000000000 +1000
+++ linux/arch/powerpc/kernel/setup_32.c	2015-08-23 20:41:07.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/video/fbdev/matrox/matroxfb_base.c
=================================--- linux.orig/drivers/video/fbdev/matrox/matroxfb_base.c	2015-08-23 20:40:53.000000000 +1000
+++ linux/drivers/video/fbdev/matrox/matroxfb_base.c	2015-08-23 20:41:07.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
Index: linux/drivers/char/generic_nvram.c
=================================--- linux.orig/drivers/char/generic_nvram.c	2015-08-23 20:40:53.000000000 +1000
+++ linux/drivers/char/generic_nvram.c	2015-08-23 20:41:07.000000000 +1000
@@ -20,6 +20,7 @@
 #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



^ permalink raw reply

* [RFC v6 16/25] powerpc, fbdev: Use NV_CMODE and NV_VMODE only when CONFIG_PPC32 and CONFIG_PPC_PMAC
From: Finn Thain @ 2015-08-23 10:41 UTC (permalink / raw)
  To: linux-kernel, linux-m68k, linuxppc-dev,
	Jean-Christophe Plagniol-Villard, Tomi Valkeinen, linux-fbdev
In-Reply-To: <20150823104129.517600532@telegraphics.com.au>

This patch addresses inconsistencies in Mac framebuffer drivers and their
use of Kconfig symbols relating to NVRAM, so PPC64 can use CONFIG_NVRAM.

Macintosh framebuffer drivers use default settings for color mode and
video mode that are found in NVRAM. On PCI Macs, MacOS stores display
settings in the Name Registry (NR) partition in NVRAM*. On NuBus Macs,
there is no NR partition and MacOS stores display mode settings in PRAM**.

Early model Macs are the ones most likely to benefit from these settings,
since they are more likely to have one display: a fixed-frequency monitor
connected to the built-in framebuffer device. Moreover, a single NV_CMODE
value and a single NV_VMODE value provide for only one display.

The NV_CMODE and NV_VMODE constants are apparently offsets into the NR
partition for Old World machines and not New World, which also suggests
that these defaults are not useful on later models.

All this argues for limiting NVRAM support in PowerMac fbdev drivers to
PPC32 and indeed this is already the case, since CONFIG_NVRAM cannot be
enabled on PPC64 at present.

For matroxfb, make the CONFIG_PPC32 condition explicit. That way the
driver won't crash on PPC64 when CONFIG_NVRAM becomes available there.

In imsttfb, add the missing CONFIG_NVRAM test to prevent a build failure,
since PPC64 does not implement nvram_read_byte(). Also add a missing
machine_is(powermac) check. Change the inconsistent CONFIG_PPC dependency
and the matching #ifdef tests to CONFIG_PPC_PMAC. Drop unused #includes.

In valkyriefb, for clarity and consistency with the other PowerMac fbdev
drivers, test for CONFIG_PPC_PMAC instead of !CONFIG_MAC. Remove a
bogus comment regarding PRAM.

* See GetPreferredConfiguration and SavePreferredConfiguration in
"Designing PCI Cards and Drivers for Power Macintosh Computers".

** See SetDefaultMode and GetDefaultMode in "Designing Cards and Drivers
for the Macintosh Family".

Signed-off-by: Finn Thain <fthain@telegraphics.com.au>

---
 drivers/video/fbdev/Kconfig                |    2 +-
 drivers/video/fbdev/imsttfb.c              |   13 +++++--------
 drivers/video/fbdev/matrox/matroxfb_base.c |    2 +-
 drivers/video/fbdev/valkyriefb.c           |   14 ++++++--------
 4 files changed, 13 insertions(+), 18 deletions(-)

Index: linux/drivers/video/fbdev/Kconfig
=================================--- linux.orig/drivers/video/fbdev/Kconfig	2015-08-23 20:40:53.000000000 +1000
+++ linux/drivers/video/fbdev/Kconfig	2015-08-23 20:41:13.000000000 +1000
@@ -544,7 +544,7 @@ config FB_IMSTT
 	bool "IMS Twin Turbo display support"
 	depends on (FB = y) && PCI
 	select FB_CFB_IMAGEBLIT
-	select FB_MACMODES if PPC
+	select FB_MACMODES if PPC_PMAC
 	help
 	  The IMS Twin Turbo is a PCI-based frame buffer card bundled with
 	  many Macintosh and compatible computers.
Index: linux/drivers/video/fbdev/imsttfb.c
=================================--- linux.orig/drivers/video/fbdev/imsttfb.c	2015-08-23 20:40:53.000000000 +1000
+++ linux/drivers/video/fbdev/imsttfb.c	2015-08-23 20:41:13.000000000 +1000
@@ -30,10 +30,8 @@
 #include <asm/io.h>
 #include <linux/uaccess.h>
 
-#if defined(CONFIG_PPC)
+#if defined(CONFIG_PPC_PMAC)
 #include <linux/nvram.h>
-#include <asm/prom.h>
-#include <asm/pci-bridge.h>
 #include "macmodes.h"
 #endif
 
@@ -328,14 +326,13 @@ enum {
 	TVP = 1
 };
 
-#define USE_NV_MODES		1
 #define INIT_BPP		8
 #define INIT_XRES		640
 #define INIT_YRES		480
 
 static int inverse = 0;
 static char fontname[40] __initdata = { 0 };
-#if defined(CONFIG_PPC)
+#if defined(CONFIG_PPC_PMAC)
 static signed char init_vmode = -1, init_cmode = -1;
 #endif
 
@@ -1391,8 +1388,8 @@ static void init_imstt(struct fb_info *i
 		}
 	}
 
-#if USE_NV_MODES && defined(CONFIG_PPC32)
-	{
+#if defined(CONFIG_PPC_PMAC) && defined(CONFIG_PPC32) && defined(CONFIG_NVRAM)
+	if (machine_is(powermac)) {
 		int vmode = init_vmode, cmode = init_cmode;
 
 		if (vmode = -1) {
@@ -1566,7 +1563,7 @@ imsttfb_setup(char *options)
 			inverse = 1;
 			fb_invert_cmaps();
 		}
-#if defined(CONFIG_PPC)
+#if defined(CONFIG_PPC_PMAC)
 		else if (!strncmp(this_opt, "vmode:", 6)) {
 			int vmode = simple_strtoul(this_opt+6, NULL, 0);
 			if (vmode > 0 && vmode <= VMODE_MAX)
Index: linux/drivers/video/fbdev/matrox/matroxfb_base.c
=================================--- linux.orig/drivers/video/fbdev/matrox/matroxfb_base.c	2015-08-23 20:41:07.000000000 +1000
+++ linux/drivers/video/fbdev/matrox/matroxfb_base.c	2015-08-23 20:41:13.000000000 +1000
@@ -1876,7 +1876,7 @@ static int initMatrox2(struct matrox_fb_
 		struct fb_var_screeninfo var;
 		if (default_vmode <= 0 || default_vmode > VMODE_MAX)
 			default_vmode = VMODE_640_480_60;
-#ifdef CONFIG_NVRAM
+#if defined(CONFIG_PPC32) && defined(CONFIG_NVRAM)
 		if (default_cmode = CMODE_NVRAM)
 			default_cmode = nvram_read_byte(NV_CMODE);
 #endif
Index: linux/drivers/video/fbdev/valkyriefb.c
=================================--- linux.orig/drivers/video/fbdev/valkyriefb.c	2015-08-23 20:40:53.000000000 +1000
+++ linux/drivers/video/fbdev/valkyriefb.c	2015-08-23 20:41:13.000000000 +1000
@@ -65,14 +65,12 @@
 #include "macmodes.h"
 #include "valkyriefb.h"
 
-#ifdef CONFIG_MAC
-/* We don't yet have functions to read the PRAM... perhaps we can
-   adapt them from the PPC code? */
-static int default_vmode = VMODE_CHOOSE;
-static int default_cmode = CMODE_8;
-#else
+#ifdef CONFIG_PPC_PMAC
 static int default_vmode = VMODE_NVRAM;
 static int default_cmode = CMODE_NVRAM;
+#else
+static int default_vmode = VMODE_CHOOSE;
+static int default_cmode = CMODE_8;
 #endif
 
 struct fb_par_valkyrie {
@@ -285,7 +283,7 @@ static void __init valkyrie_choose_mode(
 	printk(KERN_INFO "Monitor sense value = 0x%x\n", p->sense);
 
 	/* Try to pick a video mode out of NVRAM if we have one. */
-#if !defined(CONFIG_MAC) && defined(CONFIG_NVRAM)
+#if defined(CONFIG_PPC_PMAC) && defined(CONFIG_NVRAM)
 	if (default_vmode = VMODE_NVRAM) {
 		default_vmode = nvram_read_byte(NV_VMODE);
 		if (default_vmode <= 0
@@ -298,7 +296,7 @@ static void __init valkyrie_choose_mode(
 		default_vmode = mac_map_monitor_sense(p->sense);
 	if (!valkyrie_reg_init[default_vmode - 1])
 		default_vmode = VMODE_640_480_67;
-#if !defined(CONFIG_MAC) && defined(CONFIG_NVRAM)
+#if defined(CONFIG_PPC_PMAC) && defined(CONFIG_NVRAM)
 	if (default_cmode = CMODE_NVRAM)
 		default_cmode = nvram_read_byte(NV_CMODE);
 #endif



^ permalink raw reply

* [RFC v6 17/25] powerpc, fbdev: Use arch_nvram_ops methods instead of nvram_read_byte() and nvram_wri
From: Finn Thain @ 2015-08-23 10:41 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: <20150823104129.517600532@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.

Signed-off-by: Finn Thain <fthain@telegraphics.com.au>

---

Changed since v4:
- Split off the CONFIG_PPC32, CONFIG_PPC_PMAC and CONFIG_NVRAM tests into
a separate patch.

---
 arch/powerpc/kernel/setup_32.c             |    8 ++++----
 drivers/char/generic_nvram.c               |    4 ++--
 drivers/video/fbdev/controlfb.c            |    4 ++--
 drivers/video/fbdev/imsttfb.c              |    4 ++--
 drivers/video/fbdev/matrox/matroxfb_base.c |    2 +-
 drivers/video/fbdev/platinumfb.c           |    4 ++--
 drivers/video/fbdev/valkyriefb.c           |    4 ++--
 7 files changed, 15 insertions(+), 15 deletions(-)

Index: linux/arch/powerpc/kernel/setup_32.c
=================================--- linux.orig/arch/powerpc/kernel/setup_32.c	2015-08-23 20:41:11.000000000 +1000
+++ linux/arch/powerpc/kernel/setup_32.c	2015-08-23 20:41:14.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-08-23 20:41:11.000000000 +1000
+++ linux/drivers/char/generic_nvram.c	2015-08-23 20:41:14.000000000 +1000
@@ -63,7 +63,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;
@@ -83,7 +83,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-08-23 20:40:53.000000000 +1000
+++ linux/drivers/video/fbdev/controlfb.c	2015-08-23 20:41:14.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/imsttfb.c
=================================--- linux.orig/drivers/video/fbdev/imsttfb.c	2015-08-23 20:41:13.000000000 +1000
+++ linux/drivers/video/fbdev/imsttfb.c	2015-08-23 20:41:14.000000000 +1000
@@ -1393,12 +1393,12 @@ static void init_imstt(struct fb_info *i
 		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;
 		}
Index: linux/drivers/video/fbdev/matrox/matroxfb_base.c
=================================--- linux.orig/drivers/video/fbdev/matrox/matroxfb_base.c	2015-08-23 20:41:13.000000000 +1000
+++ linux/drivers/video/fbdev/matrox/matroxfb_base.c	2015-08-23 20:41:14.000000000 +1000
@@ -1878,7 +1878,7 @@ static int initMatrox2(struct matrox_fb_
 			default_vmode = VMODE_640_480_60;
 #if defined(CONFIG_PPC32) && 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
 		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-08-23 20:40:53.000000000 +1000
+++ linux/drivers/video/fbdev/platinumfb.c	2015-08-23 20:41:14.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-08-23 20:41:13.000000000 +1000
+++ linux/drivers/video/fbdev/valkyriefb.c	2015-08-23 20:41:14.000000000 +1000
@@ -285,7 +285,7 @@ static void __init valkyrie_choose_mode(
 	/* Try to pick a video mode out of NVRAM if we have one. */
 #if defined(CONFIG_PPC_PMAC) && 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])
@@ -298,7 +298,7 @@ static void __init valkyrie_choose_mode(
 		default_vmode = VMODE_640_480_67;
 #if defined(CONFIG_PPC_PMAC) && 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
 
 	/*



^ permalink raw reply

* Re: [RFC] fbdev/riva:change to use generice function to implement reverse_order()
From: yalin wang @ 2015-08-24  8:31 UTC (permalink / raw)
  To: Afzal Mohammed; +Cc: Tomi Valkeinen, adaplas, plagnioj, linux-fbdev, open list
In-Reply-To: <20150822075310.GA2337@afzalpc>


> On Aug 22, 2015, at 15:53, Afzal Mohammed <afzal.mohd.ma@gmail.com> wrote:
> 
> Hi,
> 
> On Fri, Aug 21, 2015 at 11:01:41AM +0300, Tomi Valkeinen wrote:
> 
>>>> Possibly the patches are still good for x86 also, but that needs to be
>>>> proven.
>>>> 
>>> not exactly, because x86_64 don’t have hardware instruction to do rbit OP,
>>> i compile by test :
>> 
>> For old drivers i386 may be more relevant than x86_64.
> 
> It seems asm bit reversal is supported in Kernel on arm & arm64 only,
> not sure whether any other arch even provide asm bit reversal
> instruction.

i only submit the bit reverse patch for arm / arm64 arch,
i am not sure if there are some other arch also have hard ware bit reverse 
instructions, need arch maintainers to submit if their arch also have these hard
ware instructions . :)

> 
>> These kind of optimizations should have some real world measurements,
> 
> Not for this case, but once measured on ARM, iirc, a 32-bit asm bit
> reversal as compared to doing it in C was taking 1 cycle as opposed to
> ~225 cycles!, of course writing optimized C could have made it fare
> better, but still would reach no-way near asm bit reversal.
> 



^ permalink raw reply

* Re: [RFC] fbdev/riva:change to use generice function to implement reverse_order()
From: Afzal Mohammed @ 2015-08-24 13:53 UTC (permalink / raw)
  To: yalin wang; +Cc: Tomi Valkeinen, adaplas, plagnioj, linux-fbdev, open list
In-Reply-To: <F9B74F97-EF3F-4051-B54B-B6CDD2946374@gmail.com>

Hi,

On Mon, Aug 24, 2015 at 04:31:13PM +0800, yalin wang wrote:
> 
> i only submit the bit reverse patch for arm / arm64 arch,

yes, saw later git blaming it on you :)

> > Not for this case, but once measured on ARM, iirc, a 32-bit asm bit
> > reversal as compared to doing it in C was taking 1 cycle as opposed to
> > ~225 cycles!, of course writing optimized C could have made it fare
> > better, but still would reach no-way near asm bit reversal.

The above measurement was done not in Linux, rather on a baremetal
code, but seeing the efficient Kernel C implementation, realized that
the gain would not be that much, it would be good to know if there are
measurements for Kernel bitreversal in C & asm (on supported arch)

Regards
afzal

^ permalink raw reply

* [PATCH v4 00/11] x86/dma: RIP MTRR and dma write-combine API rename
From: Luis R. Rodriguez @ 2015-08-24 19:13 UTC (permalink / raw)
  To: mingo
  Cc: bp, bhelgaas, tomi.valkeinen, airlied, linux-fbdev, luto,
	vinod.koul, dan.j.williams, toshi.kani, benh, mst, akpm,
	daniel.vetter, konrad.wilk, x86, linux-kernel, linux-pci,
	xen-devel, Luis R. Rodriguez

From: "Luis R. Rodriguez" <mcgrof@suse.com>

Ingo,

This is my pending series of patches for both write-combining and moving
Linux' use of MTRR into the grave. It combines three set of straggler patch
series which have been pending integration for a while now. The rename patches
do not depend in any way with the MTRR patches but I've combined them here as
they are all pending and relating to write-combining. I explain why integration
of such patches has been delayed but also provide reasoning for why I believe
its time to merge them.

1) The DMA API rename for write-combining goes with the old naming convention
   defines added as suggested by you for any possible stragglers which may come
   up as this goes through and gets merged. We can remove the old define
   mappings after a release once this gets sucked in and things settle. These
   patches have been in Boris tree for a while but I keep having to refresh
   them as the kernel moves on, the addition of the old mapping should allow
   us to merge this without any collateral.

2) The PCI driver changes go with Tomi Valkeinen's Acks as well as
   Arnd Bergmann's own Acks for the PCI and asm-generic changes. This series
   was technically acknowledged by Bjorn to be correct and acceptable but his
   preference was for this to not use EXPORT_SYMBOL_GPL() as not *all*
   write-combine APIs are using EXPORT_SYMBOL_GPL(). Our goal on x86 though
   is to not deal with bug reports for new PAT APIs [0] and since its now clear
   through documentation that its up to the maintainers / developers if they
   decide to use EXPORT_SYMBOL_GPL() for new *features* [1] I keep that
   practice in alignment with our own x86 goals to avoid bug reports and
   issues with proprietary drivers on new PAT interfaces. Bjorn was happy for
   this to go through someone else's tree, in particular Arnd's. Arnd Acked
   the series [2] but is unable to take these patches in at this time as he's
   out on paternal leave so sending these through you with the respective Acks.
   Boris has been hugely instrumental on helping review all MTRR related 
   series, these were sitting on his queue for a while, but he's also
   unavailable now as he's on vacation.

3) Unexporting direct MTRR access. This is last patch, I had posted this first
   on March 2015 when [3] I originally had meshed up all the MTRR work into one
   giant series. I ending up splitting up all the work into *over* 12 series.
   Now that all the work is merged except these few patches I've combined that
   patch as the last part of this series. I've amended an obituary note for
   MTRR regarding platform firmware access to MTRR based on recent discussions
   [4] and updated the documentation to reflect the status quo for Linux.

All of these patches have been tested by the 0-day bot machine. The only thing
*new* here is just my obituary amended documentation note for platform firmware
access to MTRRs on the last patch based hugely on review with Toshi.

[0] http://lkml.kernel.org/r/1424961893.17007.139.camel@misato.fc.hp.com
[1] https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?idX2ed8d51e2b6cb8a168c94852bca482685c2509
[2] http://lkml.kernel.org/r/2962702.QXZzP3RbKY@wuerfel
[3] http://lkml.kernel.org/r/1426893517-2511-48-git-send-email-mcgrof@do-not-panic.com
[4] http://lkml.kernel.org/r/1438991330.3109.196.camel@hp.com

Luis R. Rodriguez (11):
  PCI: Add pci_ioremap_wc_bar()
  drivers/video/fbdev/i740fb: Use arch_phys_wc_add() and
    pci_ioremap_wc_bar()
  drivers/video/fbdev/kyrofb: Use arch_phys_wc_add() and
    pci_ioremap_wc_bar()
  drivers/video/fbdev/gxt4500: Use pci_ioremap_wc_bar() to map
    framebuffer
  PCI: Add pci_iomap_wc() variants
  drivers/video/fbdev/arkfb.c: Use arch_phys_wc_add() and pci_iomap_wc()
  drivers/video/fbdev/s3fb: Use arch_phys_wc_add() and pci_iomap_wc()
  drivers/video/fbdev/vt8623fb: Use arch_phys_wc_add() and
    pci_iomap_wc()
  drivers/dma/iop-adma: Use dma_alloc_writecombine() kernel-style
  dma: rename dma_*_writecombine() to dma_*_wc()
  mtrr: bury MTRR - unexport mtrr_add() and mtrr_del()

 Documentation/x86/mtrr.txt                | 20 ++++++++--
 arch/arm/mach-lpc32xx/phy3250.c           | 13 +++---
 arch/arm/mach-netx/fb.c                   | 14 +++----
 arch/arm/mach-nspire/clcd.c               | 13 +++---
 arch/avr32/include/asm/dma-mapping.h      | 20 +++++-----
 arch/avr32/mm/dma-coherent.c              | 12 +++---
 arch/metag/include/asm/dma-mapping.h      |  4 +-
 arch/metag/kernel/dma.c                   |  6 +--
 arch/x86/kernel/cpu/mtrr/main.c           |  2 -
 drivers/dma/iop-adma.c                    |  9 +++--
 drivers/dma/mv_xor.c                      |  4 +-
 drivers/dma/qcom_bam_dma.c                | 14 +++----
 drivers/gpu/drm/drm_gem_cma_helper.c      | 13 +++---
 drivers/gpu/drm/omapdrm/omap_dmm_tiler.c  | 13 +++---
 drivers/gpu/drm/omapdrm/omap_gem.c        |  8 ++--
 drivers/gpu/drm/sti/sti_cursor.c          | 13 +++---
 drivers/gpu/drm/sti/sti_gdp.c             |  3 +-
 drivers/gpu/drm/sti/sti_hqvdp.c           |  6 +--
 drivers/gpu/drm/tegra/gem.c               | 11 +++---
 drivers/gpu/host1x/cdma.c                 |  8 ++--
 drivers/gpu/host1x/job.c                  | 10 ++---
 drivers/media/platform/coda/coda-bit.c    | 10 ++---
 drivers/pci/pci.c                         | 14 +++++++
 drivers/video/fbdev/acornfb.c             |  4 +-
 drivers/video/fbdev/amba-clcd-versatile.c | 14 +++----
 drivers/video/fbdev/amba-clcd.c           |  4 +-
 drivers/video/fbdev/arkfb.c               | 36 +++--------------
 drivers/video/fbdev/atmel_lcdfb.c         |  9 +++--
 drivers/video/fbdev/ep93xx-fb.c           |  9 ++---
 drivers/video/fbdev/gbefb.c               |  8 ++--
 drivers/video/fbdev/gxt4500.c             |  2 +-
 drivers/video/fbdev/i740fb.c              | 35 +++-------------
 drivers/video/fbdev/imxfb.c               | 12 +++---
 drivers/video/fbdev/kyro/fbdev.c          | 33 ++++++----------
 drivers/video/fbdev/mx3fb.c               |  9 ++---
 drivers/video/fbdev/nuc900fb.c            |  8 ++--
 drivers/video/fbdev/omap/lcdc.c           | 16 ++++----
 drivers/video/fbdev/pxa168fb.c            |  8 ++--
 drivers/video/fbdev/pxafb.c               |  4 +-
 drivers/video/fbdev/s3c-fb.c              |  7 ++--
 drivers/video/fbdev/s3c2410fb.c           |  8 ++--
 drivers/video/fbdev/s3fb.c                | 35 +++-------------
 drivers/video/fbdev/sa1100fb.c            |  8 ++--
 drivers/video/fbdev/vt8623fb.c            | 31 +++------------
 include/asm-generic/pci_iomap.h           | 14 +++++++
 include/linux/dma-mapping.h               | 16 ++++----
 include/linux/pci.h                       |  1 +
 include/video/kyro.h                      |  4 +-
 lib/pci_iomap.c                           | 66 +++++++++++++++++++++++++++++++
 sound/arm/pxa2xx-pcm-lib.c                | 20 ++++------
 sound/soc/fsl/imx-pcm-fiq.c               | 10 ++---
 sound/soc/nuc900/nuc900-pcm.c             |  6 +--
 sound/soc/omap/omap-pcm.c                 | 12 ++----
 53 files changed, 331 insertions(+), 358 deletions(-)

-- 
2.4.3


^ permalink raw reply

* [PATCH v4 01/11] PCI: Add pci_ioremap_wc_bar()
From: Luis R. Rodriguez @ 2015-08-24 19:13 UTC (permalink / raw)
  To: mingo
  Cc: bp, bhelgaas, tomi.valkeinen, airlied, linux-fbdev, luto,
	vinod.koul, dan.j.williams, toshi.kani, benh, mst, akpm,
	daniel.vetter, konrad.wilk, x86, linux-kernel, linux-pci,
	xen-devel, Luis R. Rodriguez, Antonino Daplas, Dave Airlie,
	Davidlohr Bueso, H. Peter Anvin, Jean-Christophe Plagniol-Villard,
	Juergen Gross, Mel Gorman, Suresh Siddha, Thomas Gleixner,
	Ville Syrjälä, Vlastimil Babka
In-Reply-To: <1440443613-13696-1-git-send-email-mcgrof@do-not-panic.com>

From: "Luis R. Rodriguez" <mcgrof@suse.com>

This lets drivers take advantage of PAT when available. It 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(), see:

  de33c442ed2a ("x86 PAT: fix performance drop for glx, use UC minus for ioremap(),
		ioremap_nocache() and pci_mmap_page_range()")

Signed-off-by: Luis R. Rodriguez <mcgrof@suse.com>
Acked-by: Arnd Bergmann <arnd@arndb.de>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Andy Lutomirski <luto@amacapital.net>
Cc: Antonino Daplas <adaplas@gmail.com>
Cc: benh@kernel.crashing.org
Cc: Bjorn Helgaas <bhelgaas@google.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Dave Airlie <airlied@redhat.com>
Cc: Davidlohr Bueso <dbueso@suse.de>
Cc: H. Peter Anvin <hpa@zytor.com>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Jean-Christophe Plagniol-Villard <plagnioj@jcrosoft.com>
Cc: Juergen Gross <jgross@suse.com>
Cc: linux-fbdev@vger.kernel.org
Cc: linux-pci@vger.kernel.org
Cc: Mel Gorman <mgorman@suse.de>
Cc: mst@redhat.com
Cc: Suresh Siddha <sbsiddha@gmail.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>
Cc: Toshi Kani <toshi.kani@hp.com>
Cc: Ville Syrjälä <syrjala@sci.fi>
Cc: Vlastimil Babka <vbabka@suse.cz>
Link: http://lkml.kernel.org/r/1435195342-26879-2-git-send-email-mcgrof@do-not-panic.com
Signed-off-by: Borislav Petkov <bp@suse.de>
---
 drivers/pci/pci.c   | 14 ++++++++++++++
 include/linux/pci.h |  1 +
 2 files changed, 15 insertions(+)

diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c
index 8037c27beb05..33867b8a4bc9 100644
--- a/drivers/pci/pci.c
+++ b/drivers/pci/pci.c
@@ -138,6 +138,20 @@ void __iomem *pci_ioremap_bar(struct pci_dev *pdev, int bar)
 	return ioremap_nocache(res->start, resource_size(res));
 }
 EXPORT_SYMBOL_GPL(pci_ioremap_bar);
+
+void __iomem *pci_ioremap_wc_bar(struct pci_dev *pdev, int bar)
+{
+	/*
+	 * Make sure the BAR is actually a memory resource, not an IO resource
+	 */
+	if (!(pci_resource_flags(pdev, bar) & IORESOURCE_MEM)) {
+		WARN_ON(1);
+		return NULL;
+	}
+	return ioremap_wc(pci_resource_start(pdev, bar),
+			  pci_resource_len(pdev, bar));
+}
+EXPORT_SYMBOL_GPL(pci_ioremap_wc_bar);
 #endif
 
 
diff --git a/include/linux/pci.h b/include/linux/pci.h
index 88bee285b93d..2b2d7d44c21a 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -1701,6 +1701,7 @@ static inline void pci_mmcfg_late_init(void) { }
 int pci_ext_cfg_avail(void);
 
 void __iomem *pci_ioremap_bar(struct pci_dev *pdev, int bar);
+void __iomem *pci_ioremap_wc_bar(struct pci_dev *pdev, int bar);
 
 #ifdef CONFIG_PCI_IOV
 int pci_iov_virtfn_bus(struct pci_dev *dev, int id);
-- 
2.4.3


^ permalink raw reply related

* [PATCH v4 02/11] drivers/video/fbdev/i740fb: Use arch_phys_wc_add() and pci_ioremap_wc_bar()
From: Luis R. Rodriguez @ 2015-08-24 19:13 UTC (permalink / raw)
  To: mingo
  Cc: bp, bhelgaas, tomi.valkeinen, airlied, linux-fbdev, luto,
	vinod.koul, dan.j.williams, toshi.kani, benh, mst, akpm,
	daniel.vetter, konrad.wilk, x86, linux-kernel, linux-pci,
	xen-devel, Luis R. Rodriguez, Antonino Daplas, Arnd Bergmann,
	Benoit Taine, Dave Airlie, Geert Uytterhoeven, H. Peter Anvin,
	Jean-Christophe Plagniol-Villard, Jingoo Han, Juergen Gross,
	Rob Clark, Suresh Siddha, Thomas Gleixner
In-Reply-To: <1440443613-13696-1-git-send-email-mcgrof@do-not-panic.com>

From: "Luis R. Rodriguez" <mcgrof@suse.com>

Convert the driver from using the x86-specific MTRR code to the
architecture-agnostic arch_phys_wc_add(). It will avoid MTRR if
write-combining is available, in order to take advantage of that also
ensure the ioremapped 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 it is being 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
ifdeffery 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);

Signed-off-by: Luis R. Rodriguez <mcgrof@suse.com>
Acked-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Andy Lutomirski <luto@amacapital.net>
Cc: Antonino Daplas <adaplas@gmail.com>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: benh@kernel.crashing.org
Cc: Benoit Taine <benoit.taine@lip6.fr>
Cc: Bjorn Helgaas <bhelgaas@google.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Dave Airlie <airlied@redhat.com>
Cc: Geert Uytterhoeven <geert@linux-m68k.org>
Cc: H. Peter Anvin <hpa@zytor.com>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Jean-Christophe Plagniol-Villard <plagnioj@jcrosoft.com>
Cc: Jingoo Han <jg1.han@samsung.com>
Cc: Juergen Gross <jgross@suse.com>
Cc: linux-fbdev@vger.kernel.org
Cc: linux-pci@vger.kernel.org
Cc: mst@redhat.com
Cc: Rob Clark <robdclark@gmail.com>
Cc: Suresh Siddha <sbsiddha@gmail.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: toshi.kani@hp.com
Link: http://lkml.kernel.org/r/1435195342-26879-3-git-send-email-mcgrof@do-not-panic.com
Signed-off-by: Borislav Petkov <bp@suse.de>
---
 drivers/video/fbdev/i740fb.c | 35 ++++++-----------------------------
 1 file changed, 6 insertions(+), 29 deletions(-)

diff --git a/drivers/video/fbdev/i740fb.c b/drivers/video/fbdev/i740fb.c
index a2b4204b42bb..452e1163ad02 100644
--- a/drivers/video/fbdev/i740fb.c
+++ b/drivers/video/fbdev/i740fb.c
@@ -27,24 +27,15 @@
 #include <linux/console.h>
 #include <video/vga.h>
 
-#ifdef CONFIG_MTRR
-#include <asm/mtrr.h>
-#endif
-
 #include "i740_reg.h"
 
 static char *mode_option;
-
-#ifdef CONFIG_MTRR
 static int mtrr = 1;
-#endif
 
 struct i740fb_par {
 	unsigned char __iomem *regs;
 	bool has_sgram;
-#ifdef CONFIG_MTRR
-	int mtrr_reg;
-#endif
+	int wc_cookie;
 	bool ddc_registered;
 	struct i2c_adapter ddc_adapter;
 	struct i2c_algo_bit_data ddc_algo;
@@ -1040,7 +1031,7 @@ static int i740fb_probe(struct pci_dev *dev, const struct pci_device_id *ent)
 		goto err_request_regions;
 	}
 
-	info->screen_base = pci_ioremap_bar(dev, 0);
+	info->screen_base = pci_ioremap_wc_bar(dev, 0);
 	if (!info->screen_base) {
 		dev_err(info->device, "error remapping base\n");
 		ret = -ENOMEM;
@@ -1144,13 +1135,9 @@ static int i740fb_probe(struct pci_dev *dev, const struct pci_device_id *ent)
 
 	fb_info(info, "%s frame buffer device\n", info->fix.id);
 	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);
 	return 0;
 
 err_reg_framebuffer:
@@ -1177,13 +1164,7 @@ static void i740fb_remove(struct pci_dev *dev)
 
 	if (info) {
 		struct i740fb_par *par = info->par;
-
-#ifdef CONFIG_MTRR
-		if (par->mtrr_reg >= 0) {
-			mtrr_del(par->mtrr_reg, 0, 0);
-			par->mtrr_reg = -1;
-		}
-#endif
+		arch_phys_wc_del(par->wc_cookie);
 		unregister_framebuffer(info);
 		fb_dealloc_cmap(&info->cmap);
 		if (par->ddc_registered)
@@ -1287,10 +1268,8 @@ static int  __init i740fb_setup(char *options)
 	while ((opt = strsep(&options, ",")) != NULL) {
 		if (!*opt)
 			continue;
-#ifdef CONFIG_MTRR
 		else if (!strncmp(opt, "mtrr:", 5))
 			mtrr = simple_strtoul(opt + 5, NULL, 0);
-#endif
 		else
 			mode_option = opt;
 	}
@@ -1327,7 +1306,5 @@ MODULE_DESCRIPTION("fbdev driver for Intel740");
 module_param(mode_option, charp, 0444);
 MODULE_PARM_DESC(mode_option, "Default video mode ('640x480-8@60', etc)");
 
-#ifdef CONFIG_MTRR
 module_param(mtrr, int, 0444);
 MODULE_PARM_DESC(mtrr, "Enable write-combining with MTRR (1=enable, 0=disable, default=1)");
-#endif
-- 
2.4.3


^ permalink raw reply related

* [PATCH v4 03/11] drivers/video/fbdev/kyrofb: Use arch_phys_wc_add() and pci_ioremap_wc_bar()
From: Luis R. Rodriguez @ 2015-08-24 19:13 UTC (permalink / raw)
  To: mingo
  Cc: bp, bhelgaas, tomi.valkeinen, airlied, linux-fbdev, luto,
	vinod.koul, dan.j.williams, toshi.kani, benh, mst, akpm,
	daniel.vetter, konrad.wilk, x86, linux-kernel, linux-pci,
	xen-devel, Luis R. Rodriguez, Antonino Daplas, Arnd Bergmann,
	Dave Airlie, Geert Uytterhoeven, H. Peter Anvin,
	Jean-Christophe Plagniol-Villard, Jingoo Han, Juergen Gross,
	Laurent Pinchart, Suresh Siddha, Thomas Gleixner
In-Reply-To: <1440443613-13696-1-git-send-email-mcgrof@do-not-panic.com>

From: "Luis R. Rodriguez" <mcgrof@suse.com>

Convert the driver from using the x86-specific MTRR code to the
architecture-agnostic arch_phys_wc_add(). It will avoid MTRR if
write-combining is available, in order to take advantage of that
also ensure the ioremapped 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 it is being 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
ifdeffery 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);

Signed-off-by: Luis R. Rodriguez <mcgrof@suse.com>
Acked-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Andy Lutomirski <luto@amacapital.net>
Cc: Antonino Daplas <adaplas@gmail.com>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: benh@kernel.crashing.org
Cc: bhelgaas@google.com
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Dave Airlie <airlied@redhat.com>
Cc: Geert Uytterhoeven <geert@linux-m68k.org>
Cc: H. Peter Anvin <hpa@zytor.com>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Jean-Christophe Plagniol-Villard <plagnioj@jcrosoft.com>
Cc: Jingoo Han <jg1.han@samsung.com>
Cc: Juergen Gross <jgross@suse.com>
Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Cc: linux-fbdev@vger.kernel.org
Cc: linux-pci@vger.kernel.org
Cc: mst@redhat.com
Cc: Suresh Siddha <sbsiddha@gmail.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: toshi.kani@hp.com
Link: http://lkml.kernel.org/r/1435195342-26879-4-git-send-email-mcgrof@do-not-panic.com
Signed-off-by: Borislav Petkov <bp@suse.de>
---
 drivers/video/fbdev/kyro/fbdev.c | 33 +++++++++++----------------------
 include/video/kyro.h             |  4 +---
 2 files changed, 12 insertions(+), 25 deletions(-)

diff --git a/drivers/video/fbdev/kyro/fbdev.c b/drivers/video/fbdev/kyro/fbdev.c
index 65041e15fd59..5bb01533271e 100644
--- a/drivers/video/fbdev/kyro/fbdev.c
+++ b/drivers/video/fbdev/kyro/fbdev.c
@@ -22,9 +22,6 @@
 #include <linux/pci.h>
 #include <asm/io.h>
 #include <linux/uaccess.h>
-#ifdef CONFIG_MTRR
-#include <asm/mtrr.h>
-#endif
 
 #include <video/kyro.h>
 
@@ -84,9 +81,7 @@ static device_info_t deviceInfo;
 static char *mode_option = NULL;
 static int nopan = 0;
 static int nowrap = 1;
-#ifdef CONFIG_MTRR
 static int nomtrr = 0;
-#endif
 
 /* PCI driver prototypes */
 static int kyrofb_probe(struct pci_dev *pdev, const struct pci_device_id *ent);
@@ -570,10 +565,8 @@ static int __init kyrofb_setup(char *options)
 			nopan = 1;
 		} else if (strcmp(this_opt, "nowrap") = 0) {
 			nowrap = 1;
-#ifdef CONFIG_MTRR
 		} else if (strcmp(this_opt, "nomtrr") = 0) {
 			nomtrr = 1;
-#endif
 		} else {
 			mode_option = this_opt;
 		}
@@ -691,17 +684,16 @@ static int kyrofb_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
 
 	currentpar->regbase = deviceInfo.pSTGReg  		ioremap_nocache(kyro_fix.mmio_start, kyro_fix.mmio_len);
+	if (!currentpar->regbase)
+		goto out_free_fb;
 
-	info->screen_base = ioremap_nocache(kyro_fix.smem_start,
-					    kyro_fix.smem_len);
+	info->screen_base = pci_ioremap_wc_bar(pdev, 0);
+	if (!info->screen_base)
+		goto out_unmap_regs;
 
-#ifdef CONFIG_MTRR
 	if (!nomtrr)
-		currentpar->mtrr_handle -			mtrr_add(kyro_fix.smem_start,
-				 kyro_fix.smem_len,
-				 MTRR_TYPE_WRCOMB, 1);
-#endif
+		currentpar->wc_cookie = arch_phys_wc_add(kyro_fix.smem_start,
+							 kyro_fix.smem_len);
 
 	kyro_fix.ypanstep	= nopan ? 0 : 1;
 	kyro_fix.ywrapstep	= nowrap ? 0 : 1;
@@ -745,8 +737,10 @@ static int kyrofb_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
 	return 0;
 
 out_unmap:
-	iounmap(currentpar->regbase);
 	iounmap(info->screen_base);
+out_unmap_regs:
+	iounmap(currentpar->regbase);
+out_free_fb:
 	framebuffer_release(info);
 
 	return -EINVAL;
@@ -770,12 +764,7 @@ static void kyrofb_remove(struct pci_dev *pdev)
 	iounmap(info->screen_base);
 	iounmap(par->regbase);
 
-#ifdef CONFIG_MTRR
-	if (par->mtrr_handle)
-		mtrr_del(par->mtrr_handle,
-			 info->fix.smem_start,
-			 info->fix.smem_len);
-#endif
+	arch_phys_wc_del(par->wc_cookie);
 
 	unregister_framebuffer(info);
 	framebuffer_release(info);
diff --git a/include/video/kyro.h b/include/video/kyro.h
index c563968e926c..b958c2e9c915 100644
--- a/include/video/kyro.h
+++ b/include/video/kyro.h
@@ -35,9 +35,7 @@ struct kyrofb_info {
 	/* Useful to hold depth here for Linux */
 	u8 PIXDEPTH;
 
-#ifdef CONFIG_MTRR
-	int mtrr_handle;
-#endif
+	int wc_cookie;
 };
 
 extern int kyro_dev_init(void);
-- 
2.4.3


^ permalink raw reply related

* [PATCH v4 04/11] drivers/video/fbdev/gxt4500: Use pci_ioremap_wc_bar() to map framebuffer
From: Luis R. Rodriguez @ 2015-08-24 19:13 UTC (permalink / raw)
  To: mingo
  Cc: bp, bhelgaas, tomi.valkeinen, airlied, linux-fbdev, luto,
	vinod.koul, dan.j.williams, toshi.kani, benh, mst, akpm,
	daniel.vetter, konrad.wilk, x86, linux-kernel, linux-pci,
	xen-devel, Luis R. Rodriguez, Antonino Daplas, Arnd Bergmann,
	Dave Airlie, Geert Uytterhoeven, H. Peter Anvin,
	Jean-Christophe Plagniol-Villard, Juergen Gross, Laurent Pinchart,
	Rob Clark, Suresh Siddha, Thomas Gleixner
In-Reply-To: <1440443613-13696-1-git-send-email-mcgrof@do-not-panic.com>

From: "Luis R. Rodriguez" <mcgrof@suse.com>

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()").

Signed-off-by: Luis R. Rodriguez <mcgrof@suse.com>
Acked-by: Tomi Valkeinen <tomi.valkeinen@ti.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Andy Lutomirski <luto@amacapital.net>
Cc: Antonino Daplas <adaplas@gmail.com>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: benh@kernel.crashing.org
Cc: bhelgaas@google.com
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Dave Airlie <airlied@redhat.com>
Cc: Geert Uytterhoeven <geert@linux-m68k.org>
Cc: H. Peter Anvin <hpa@zytor.com>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: Jean-Christophe Plagniol-Villard <plagnioj@jcrosoft.com>
Cc: Juergen Gross <jgross@suse.com>
Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Cc: linux-fbdev@vger.kernel.org
Cc: linux-pci@vger.kernel.org
Cc: mst@redhat.com
Cc: Rob Clark <robdclark@gmail.com>
Cc: Suresh Siddha <sbsiddha@gmail.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: toshi.kani@hp.com
Link: http://lkml.kernel.org/r/1435195342-26879-5-git-send-email-mcgrof@do-not-panic.com
Signed-off-by: Borislav Petkov <bp@suse.de>
---
 drivers/video/fbdev/gxt4500.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/video/fbdev/gxt4500.c b/drivers/video/fbdev/gxt4500.c
index 135d78a02588..f19133a80e8c 100644
--- a/drivers/video/fbdev/gxt4500.c
+++ b/drivers/video/fbdev/gxt4500.c
@@ -662,7 +662,7 @@ static int gxt4500_probe(struct pci_dev *pdev, const struct pci_device_id *ent)
 
 	info->fix.smem_start = fb_phys;
 	info->fix.smem_len = pci_resource_len(pdev, 1);
-	info->screen_base = pci_ioremap_bar(pdev, 1);
+	info->screen_base = pci_ioremap_wc_bar(pdev, 1);
 	if (!info->screen_base) {
 		dev_err(&pdev->dev, "gxt4500: cannot map framebuffer\n");
 		goto err_unmap_regs;
-- 
2.4.3


^ permalink raw reply related

* [PATCH v4 05/11] PCI: Add pci_iomap_wc() variants
From: Luis R. Rodriguez @ 2015-08-24 19:13 UTC (permalink / raw)
  To: mingo
  Cc: linux-fbdev, mst, airlied, benh, Ville Syrjälä,
	Dave Hansen, jbeulich, H. Peter Anvin, Thomas Gleixner,
	linux-arch, xen-devel, Antonino Daplas, Davidlohr Bueso,
	vinod.koul, x86, tomi.valkeinen, Mel Gorman, linux-pci,
	Dave Airlie, bp, Jean-Christophe Plagniol-Villard, Arnd Bergmann,
	Luis R. Rodriguez, Rusty Russell, Stefan Bader, daniel.vetter,
	Suresh Siddha, bhelgaas, dan.j.williams
In-Reply-To: <1440443613-13696-1-git-send-email-mcgrof@do-not-panic.com>

From: "Luis R. Rodriguez" <mcgrof@suse.com>

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()").

Signed-off-by: Luis R. Rodriguez <mcgrof@suse.com>
Acked-by: Arnd Bergmann <arnd@arndb.de>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Andy Lutomirski <luto@amacapital.net>
Cc: Antonino Daplas <adaplas@gmail.com>
Cc: Arnd Bergmann <arnd@arndb.de>
Cc: benh@kernel.crashing.org
Cc: bhelgaas@google.com
Cc: Bjorn Helgaas <bhelgaas@google.com>
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Dave Airlie <airlied@redhat.com>
Cc: Dave Hansen <dave.hansen@linux.intel.com>
Cc: Davidlohr Bueso <dbueso@suse.de>
Cc: david.vrabel@citrix.com
Cc: H. Peter Anvin <hpa@zytor.com>
Cc: Ingo Molnar <mingo@kernel.org>
Cc: jbeulich@suse.com
Cc: Jean-Christophe Plagniol-Villard <plagnioj@jcrosoft.com>
Cc: Juergen Gross <jgross@suse.com>
Cc: konrad.wilk@oracle.com
Cc: linux-arch@vger.kernel.org
Cc: linux-fbdev@vger.kernel.org
Cc: linux-pci@vger.kernel.org
Cc: Mel Gorman <mgorman@suse.de>
Cc: "Michael S. Tsirkin" <mst@redhat.com>
Cc: Roger Pau Monné <roger.pau@citrix.com>
Cc: Rusty Russell <rusty@rustcorp.com.au>
Cc: Stefan Bader <stefan.bader@canonical.com>
Cc: Suresh Siddha <sbsiddha@gmail.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Tomi Valkeinen <tomi.valkeinen@ti.com>
Cc: Toshi Kani <toshi.kani@hp.com>
Cc: venkatesh.pallipadi@intel.com
Cc: Ville Syrjälä <syrjala@sci.fi>
Cc: Vlastimil Babka <vbabka@suse.cz>
Link: http://lkml.kernel.org/r/1426893517-2511-6-git-send-email-mcgrof@do-not-panic.com
Link: http://lkml.kernel.org/r/1435195342-26879-6-git-send-email-mcgrof@do-not-panic.com
[ Move IORESOURCE_IO check up, space out statements for better readability. ]
Signed-off-by: Borislav Petkov <bp@suse.de>
---
 include/asm-generic/pci_iomap.h | 14 +++++++++
 lib/pci_iomap.c                 | 66 +++++++++++++++++++++++++++++++++++++++++
 2 files changed, 80 insertions(+)

diff --git a/include/asm-generic/pci_iomap.h b/include/asm-generic/pci_iomap.h
index 7389c87116a0..b1e17fcee2d0 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 e1930dbab2da..c10fba461454 100644
--- a/lib/pci_iomap.c
+++ b/lib/pci_iomap.c
@@ -49,6 +49,51 @@ 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 (flags & IORESOURCE_IO)
+		return NULL;
+
+	if (len <= offset || !start)
+		return NULL;
+
+	len -= offset;
+	start += offset;
+	if (maxlen && len > maxlen)
+		len = maxlen;
+
+	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
@@ -67,4 +112,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.4.3


^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox