Re: ivtv: use arch_phys_wc_add() and require PAT disabled

[Ian Armstrong]

On Sun, 11 Mar 2018 23:04:02 -0500
Nick French  wrote:

> On Sun, Mar 11, 2018 at 11:24:38PM +, Ian Armstrong wrote:
> > On Sat, 10 Mar 2018 16:57:41 +
> > "French, Nicholas A."  wrote:
> >   
> > > > > No what if the framebuffer driver is just requested as a
> > > > > secondary step after firmware loading?
> > > >
> > > > Its a possibility. The decoder firmware gets loaded at the
> > > > beginning of the decoder memory range and we know its length, so
> > > > its possible to ioremap_nocache enough room for the firmware
> > > > only on init and then ioremap the remaining non-firmware
> > > > decoder memory areas appropriately after the firmware load
> > > > succeeds...
> > > 
> > > I looked in more detail, and this would be "hard" due to the way
> > > the rest of the decoder offsets are determined by either making
> > > firmware calls or scanning the decoder memory range for magic
> > > bytes and other mess.  
> > 
> > The buffers used for yuv output are fixed. They are located both
> > before and after the framebuffer. Their offset is fixed at
> > 'base_addr + IVTV_DECODER_OFFSET + yuv_offset[]'. The yuv offsets
> > can be found in 'ivtv-yuv.c'. The buffers are 622080 bytes in
> > length.
> > 
> > The range would be from 'base_addr + 0x0100 + 0x00029000' to
> > 'base_addr + 0x0100 + 0x00748200 + 0x97dff'. This is larger than
> > required, but will catch the framebuffer and should not cause any
> > problems. If you wanted to render direct to the yuv buffers, you
> > would probably want this region included anyway (not that the
> > current driver supports that).  
> 
> Am I correct that you are talking about the possibility of
> re-ioremap()-ing the 'yuv-fb-yuv' area *after* loading the firmware,
> not of mapping ranges correctly on the first go-around?
> 
> Because unless my math is letting me down, the decoder firmware is
> already loaded from 'base_addr + 0x0100 + 0x0' to 'base_addr +
> 0x0100 + 0x3' which overlaps the beginning of the yuv range.

Good catch. I'd forgotten that the firmware moves after being loaded.

Although ivtvfb.c requests the offset to the framebuffer via the
firmware, I don't believe it ever actually moves. The firmware allows
more memory to be allocated for video buffers, but will then reduce the
length of the framebuffer. At present the ivtv driver requests two more
video buffers, so the framebuffer is shortened (the start address
doesn't move). Those two buffers are located at base_addr + 0x16B0400
and 0x1748200. The now shortened framebuffer is of size 1704960 bytes
(0x1A0400).

These two offsets, plus those of the other video buffers are hardcoded
in ivtv-yuv.c. It was written on the assumption that the location of the
video buffers on the card are fixed. To the best of my knowledge, this
has never caused a problem.

>From ivtvfb.c :

/* The osd buffer size depends on the number of video buffers allocated
   on the PVR350 itself. For now we'll hardcode the smallest osd buffer
   size to prevent any overlap. */
oi->video_buffer_size = 1704960;

We know that one of the additional video buffers is at 0x16B0400, and
that this is located at the end of the now shortened 1704960 byte
framebuffer.

0x16B0400 - 0x1A0400 = 0x151.

This gives us a 0x1A0400 byte framebuffer at base_addr + 0x151.

Assuming the hardware is a PVR350 and is running stock firmware, I
thinks its safe to say this won't change.

-- 
Ian

Re: ivtv: use arch_phys_wc_add() and require PAT disabled

[Nick French]

On Sun, Mar 11, 2018 at 11:24:38PM +, Ian Armstrong wrote:
> On Sat, 10 Mar 2018 16:57:41 +
> "French, Nicholas A."  wrote:
> 
> > > > No what if the framebuffer driver is just requested as a
> > > > secondary step after firmware loading?  
> > >
> > > Its a possibility. The decoder firmware gets loaded at the
> > > beginning of the decoder memory range and we know its length, so
> > > its possible to ioremap_nocache enough room for the firmware only
> > > on init and then ioremap the remaining non-firmware decoder memory
> > > areas appropriately after the firmware load succeeds...  
> > 
> > I looked in more detail, and this would be "hard" due to the way the
> > rest of the decoder offsets are determined by either making firmware
> > calls or scanning the decoder memory range for magic bytes and other
> > mess.
> 
> The buffers used for yuv output are fixed. They are located both before
> and after the framebuffer. Their offset is fixed at 'base_addr +
> IVTV_DECODER_OFFSET + yuv_offset[]'. The yuv offsets can be found in
> 'ivtv-yuv.c'. The buffers are 622080 bytes in length.
> 
> The range would be from 'base_addr + 0x0100 + 0x00029000' to
> 'base_addr + 0x0100 + 0x00748200 + 0x97dff'. This is larger than
> required, but will catch the framebuffer and should not cause any
> problems. If you wanted to render direct to the yuv buffers, you would
> probably want this region included anyway (not that the current driver
> supports that).

Am I correct that you are talking about the possibility of re-ioremap()-ing
the 'yuv-fb-yuv' area *after* loading the firmware, not of mapping ranges
correctly on the first go-around?

Because unless my math is letting me down, the decoder firmware is already
loaded from 'base_addr + 0x0100 + 0x0' to 'base_addr + 0x0100 + 0x3'
which overlaps the beginning of the yuv range.

- Nick

Re: ivtv: use arch_phys_wc_add() and require PAT disabled

[Ian Armstrong]

On Sat, 10 Mar 2018 16:57:41 +
"French, Nicholas A."  wrote:

> > > No what if the framebuffer driver is just requested as a
> > > secondary step after firmware loading?  
> >
> > Its a possibility. The decoder firmware gets loaded at the
> > beginning of the decoder memory range and we know its length, so
> > its possible to ioremap_nocache enough room for the firmware only
> > on init and then ioremap the remaining non-firmware decoder memory
> > areas appropriately after the firmware load succeeds...  
> 
> I looked in more detail, and this would be "hard" due to the way the
> rest of the decoder offsets are determined by either making firmware
> calls or scanning the decoder memory range for magic bytes and other
> mess.

The buffers used for yuv output are fixed. They are located both before
and after the framebuffer. Their offset is fixed at 'base_addr +
IVTV_DECODER_OFFSET + yuv_offset[]'. The yuv offsets can be found in
'ivtv-yuv.c'. The buffers are 622080 bytes in length.

The range would be from 'base_addr + 0x0100 + 0x00029000' to
'base_addr + 0x0100 + 0x00748200 + 0x97dff'. This is larger than
required, but will catch the framebuffer and should not cause any
problems. If you wanted to render direct to the yuv buffers, you would
probably want this region included anyway (not that the current driver
supports that).

-- 
Ian

Re: ivtv: use arch_phys_wc_add() and require PAT disabled

[Nick French]

On Sun, Mar 11, 2018 at 01:19:03PM -0700, Andy Lutomirski wrote:
> From memory, I see two potentially reasonable real fixes. One is to find a 
> way to punch a hole in an ioremap.
> So you’d find the framebuffer, remove it from theproblematic mapping, and 
> then make a new mapping. 
> The second is to change the mapping type in place. 

For the changing-in-place method, is there already an exported API that exposes 
change_page_attr_set without first
calling reserve_memtype? I can't seem to find one.

> Or maybe you could just iounmap the whole thing after firmware is loaded and 
> the framebuffer is found and then
> redo the mapping right. 

I guess this would require a lock so that the ivtv-driver proper wasn't 
accessing the decoder's mapped memory 
during ivtvfb's iounmap-ioremap window. And a way to notify ivtv-driver proper 
if things go wrong? I think this method
would be very awkward because its not even memory owned by ivtvfb itself.

- Nick

Re: ivtv: use arch_phys_wc_add() and require PAT disabled

[Andy Lutomirski]

> On Mar 11, 2018, at 12:51 PM, Nick French  wrote:
> 
> On Sat, Mar 10, 2018 at 10:20:23AM -0800, Andy Lutomirski wrote:
 Perhaps the easy answer is to change the fatal is-pat-enabled check to just
 a warning like "you have PAT enabled, so wc is disabled for the 
 framebuffer.
 if you want wc, use the nopat parameter"?
>>> 
>>> I like this idea more and more. I haven't experience any problems running
>>> with PAT-enabled and no write-combining on the framebuffer. Any objections?
>>> 
>> 
>> None from me.
>> 
>> However, since you have the hardware, you could see if you can use the
>> change_page_attr machinery to change the memory type on the framebuffer once
>> you figure out where it is.
> 
> I am certainly willing to try this, but my understanding of the goal of the
> changes that disabled ivtvfb originally is that it was trying to hide the
> architecture-specific memory management from the driver.

Not really. The goal was to eliminate all code that touches MTRRs on PAT 
systems. So mtrr_add got unexported and the arch_phys are legacy hints that do 
nothing on modern machines. 

> 
> Wouldn't (figuring out a way to) expose x86/mm/pageattr internals to the
> driver be doing the opposite? (or maybe I misunderstand your suggestion)

It doesn’t conflict at all.  Obviously the code should be tidy. 

From memory, I see two potentially reasonable real fixes. One is to find a way 
to punch a hole in an ioremap. So you’d find the framebuffer, remove it from 
theproblematic mapping, and then make a new mapping. The second is to change 
the mapping type in place. 

Or maybe you could just iounmap the whole thing after firmware is loaded and 
the framebuffer is found and then redo the mapping right. 

Re: ivtv: use arch_phys_wc_add() and require PAT disabled

[Nick French]

On Sat, Mar 10, 2018 at 10:20:23AM -0800, Andy Lutomirski wrote:
>>> Perhaps the easy answer is to change the fatal is-pat-enabled check to just
>>> a warning like "you have PAT enabled, so wc is disabled for the framebuffer.
>>> if you want wc, use the nopat parameter"?
>>
>> I like this idea more and more. I haven't experience any problems running
>> with PAT-enabled and no write-combining on the framebuffer. Any objections?
>>
>
> None from me.
>
> However, since you have the hardware, you could see if you can use the
> change_page_attr machinery to change the memory type on the framebuffer once
> you figure out where it is.

I am certainly willing to try this, but my understanding of the goal of the
changes that disabled ivtvfb originally is that it was trying to hide the
architecture-specific memory management from the driver.

Wouldn't (figuring out a way to) expose x86/mm/pageattr internals to the
driver be doing the opposite? (or maybe I misunderstand your suggestion)

- Nick

Re: ivtv: use arch_phys_wc_add() and require PAT disabled

[Luis R. Rodriguez]

On Sat, Mar 10, 2018 at 11:03 AM, Luis R. Rodriguez  wrote:
> On Sat, Mar 10, 2018 at 8:57 AM, French, Nicholas A.  wrote:
>> On Wed, Mar 07, 2018 at 11:23:09PM -0600, French, Nicholas A. wrote:
>>> On Thu, Mar 08, 2018 at 04:14:11AM +, Luis R. Rodriguez wrote:
>>> > On Thu, Mar 08, 2018 at 04:06:01AM +, Luis R. Rodriguez wrote:
>>> > > On Thu, Mar 08, 2018 at 03:16:29AM +, French, Nicholas A. wrote:
>>> > > >
>>> > > > Ah, I see. So my proposed ioremap_wc call was only "working" by 
>>> > > > aliasing the
>>> > > > ioremap_nocache()'d mem area and not actually using write combining 
>>> > > > at all.
>>> > >
>>> > > There are some debugging PAT toys out there I think but I haven't 
>>> > > played with
>>> > > them yet or I forgot how to to confirm or deny this sort of effort, but
>>> > > likeley.
>>> >
>>> >  In fact come to think of it I believe some neurons are telling me that if
>>> >  two type does not match we'd get an error?
>>
>> I can confirm that my original suggested patch just aliases to ivtv-driver's 
>> nocache mapping:
>> $ sudo modprobe ivtvfb
>> $ sudo dmesg
>> ...
>> x86/PAT: Overlap at 0xd500-0xd580
>> x86/PAT: reserve_memtype added [mem 0xd551-0xd56b0fff], track 
>> uncached-minus, req write-combining, ret uncached-minus
>> ivtvfb0: Framebuffer at 0xd551, mapped to 0xc6a7ed52, size 1665k
>> ...
>> $ sudo cat /sys/kernel/debug/x86/pat_memtype_list | grep 0xd5
>> uncached-minus @ 0xd500-0xd580
>> uncached-minus @ 0xd551-0xd56b1000
>>
>> So nix that.
>>
>>> > No what if the framebuffer driver is just requested as a secondary step
>>> > after firmware loading?
>>>
>>> Its a possibility. The decoder firmware gets loaded at the beginning of the 
>>> decoder
>>> memory range and we know its length, so its possible to ioremap_nocache 
>>> enough
>>> room for the firmware only on init and then ioremap the remaining 
>>> non-firmware
>>> decoder memory areas appropriately after the firmware load succeeds...
>>
>> I looked in more detail, and this would be "hard" due to the way the rest of 
>> the
>> decoder offsets are determined by either making firmware calls or scanning 
>> the
>> decoder memory range for magic bytes and other mess.
>>
>> I think some smart guy named mcgrof apparently came to the same conclusion
>> in a really old email chain I found 
>> [https://lists.gt.net/linux/kernel/2387536]:
>> "The ivtv case is the *worst* example we can expect where the firmware
>> hides from us the exact ranges for write-combining, that we should somehow
>> just hope no one will ever do again."
>> :-)
>
> This is tribal knowledge worth formalizing a bit more for the long run
> for this ivtv driver.
>
>>> Perhaps the easy answer is to change the fatal is-pat-enabled check to just 
>>> a
>>> warning like "you have PAT enabled, so wc is disabled for the framebuffer.
>>> if you want wc, use the nopat parameter"?
>>
>> I like this idea more and more. I haven't experience any problems running
>> with PAT-enabled and no write-combining on the framebuffer. Any objections?
>
> I think its worth it, and perhaps best folded under a new kernel
> parameter option which also documents the limitation noted above,
> thereby knocking two birds with one stone. This way also users who
> *want* to opt-in to PAT do so willing-fully and knowing of the
> limitation. The kconfig option can just enable a module parameter to a
> default value, which if the kconfig is disabled would otherwise be
> unset.
>
> static bool ivtv_force_pat = IS_ENABLED(CONFIG_IVTV_WHATEVER);
> module_param_named(force_pat, ivtv_force_pat, bool, S_IRUGO | S_IWUSR);

And  I wonder if its better to have a generic kconfig option so that
in case other drivers have similar issue they can make use of it as
well. For now that's a non-issue, but worth pointing out if we're
going to do this for more than one driver later.

 Luis

Re: ivtv: use arch_phys_wc_add() and require PAT disabled

[Luis R. Rodriguez]

On Sat, Mar 10, 2018 at 8:57 AM, French, Nicholas A.  wrote:
> On Wed, Mar 07, 2018 at 11:23:09PM -0600, French, Nicholas A. wrote:
>> On Thu, Mar 08, 2018 at 04:14:11AM +, Luis R. Rodriguez wrote:
>> > On Thu, Mar 08, 2018 at 04:06:01AM +, Luis R. Rodriguez wrote:
>> > > On Thu, Mar 08, 2018 at 03:16:29AM +, French, Nicholas A. wrote:
>> > > >
>> > > > Ah, I see. So my proposed ioremap_wc call was only "working" by 
>> > > > aliasing the
>> > > > ioremap_nocache()'d mem area and not actually using write combining at 
>> > > > all.
>> > >
>> > > There are some debugging PAT toys out there I think but I haven't played 
>> > > with
>> > > them yet or I forgot how to to confirm or deny this sort of effort, but
>> > > likeley.
>> >
>> >  In fact come to think of it I believe some neurons are telling me that if
>> >  two type does not match we'd get an error?
>
> I can confirm that my original suggested patch just aliases to ivtv-driver's 
> nocache mapping:
> $ sudo modprobe ivtvfb
> $ sudo dmesg
> ...
> x86/PAT: Overlap at 0xd500-0xd580
> x86/PAT: reserve_memtype added [mem 0xd551-0xd56b0fff], track 
> uncached-minus, req write-combining, ret uncached-minus
> ivtvfb0: Framebuffer at 0xd551, mapped to 0xc6a7ed52, size 1665k
> ...
> $ sudo cat /sys/kernel/debug/x86/pat_memtype_list | grep 0xd5
> uncached-minus @ 0xd500-0xd580
> uncached-minus @ 0xd551-0xd56b1000
>
> So nix that.
>
>> > No what if the framebuffer driver is just requested as a secondary step
>> > after firmware loading?
>>
>> Its a possibility. The decoder firmware gets loaded at the beginning of the 
>> decoder
>> memory range and we know its length, so its possible to ioremap_nocache 
>> enough
>> room for the firmware only on init and then ioremap the remaining 
>> non-firmware
>> decoder memory areas appropriately after the firmware load succeeds...
>
> I looked in more detail, and this would be "hard" due to the way the rest of 
> the
> decoder offsets are determined by either making firmware calls or scanning the
> decoder memory range for magic bytes and other mess.
>
> I think some smart guy named mcgrof apparently came to the same conclusion
> in a really old email chain I found 
> [https://lists.gt.net/linux/kernel/2387536]:
> "The ivtv case is the *worst* example we can expect where the firmware
> hides from us the exact ranges for write-combining, that we should somehow
> just hope no one will ever do again."
> :-)

This is tribal knowledge worth formalizing a bit more for the long run
for this ivtv driver.

>> Perhaps the easy answer is to change the fatal is-pat-enabled check to just a
>> warning like "you have PAT enabled, so wc is disabled for the framebuffer.
>> if you want wc, use the nopat parameter"?
>
> I like this idea more and more. I haven't experience any problems running
> with PAT-enabled and no write-combining on the framebuffer. Any objections?

I think its worth it, and perhaps best folded under a new kernel
parameter option which also documents the limitation noted above,
thereby knocking two birds with one stone. This way also users who
*want* to opt-in to PAT do so willing-fully and knowing of the
limitation. The kconfig option can just enable a module parameter to a
default value, which if the kconfig is disabled would otherwise be
unset.

static bool ivtv_force_pat = IS_ENABLED(CONFIG_IVTV_WHATEVER);
module_param_named(force_pat, ivtv_force_pat, bool, S_IRUGO | S_IWUSR);

  Luis

Re: ivtv: use arch_phys_wc_add() and require PAT disabled

[Andy Lutomirski]

> On Mar 10, 2018, at 8:57 AM, French, Nicholas A.  wrote:
> 
>> On Wed, Mar 07, 2018 at 11:23:09PM -0600, French, Nicholas A. wrote:
>>> On Thu, Mar 08, 2018 at 04:14:11AM +, Luis R. Rodriguez wrote:
 On Thu, Mar 08, 2018 at 04:06:01AM +, Luis R. Rodriguez wrote:
> On Thu, Mar 08, 2018 at 03:16:29AM +, French, Nicholas A. wrote:
> 
> Ah, I see. So my proposed ioremap_wc call was only "working" by aliasing 
> the
> ioremap_nocache()'d mem area and not actually using write combining at 
> all.
 
 There are some debugging PAT toys out there I think but I haven't played 
 with
 them yet or I forgot how to to confirm or deny this sort of effort, but
 likeley.
>>> 
>>> In fact come to think of it I believe some neurons are telling me that if
>>> two type does not match we'd get an error?
> 
> I can confirm that my original suggested patch just aliases to ivtv-driver's 
> nocache mapping:
> $ sudo modprobe ivtvfb
> $ sudo dmesg
> ...
> x86/PAT: Overlap at 0xd500-0xd580
> x86/PAT: reserve_memtype added [mem 0xd551-0xd56b0fff], track 
> uncached-minus, req write-combining, ret uncached-minus
> ivtvfb0: Framebuffer at 0xd551, mapped to 0xc6a7ed52, size 1665k
> ...
> $ sudo cat /sys/kernel/debug/x86/pat_memtype_list | grep 0xd5
> uncached-minus @ 0xd500-0xd580
> uncached-minus @ 0xd551-0xd56b1000
> 
> So nix that.
> 
>>> No what if the framebuffer driver is just requested as a secondary step
>>> after firmware loading?
>> 
>> Its a possibility. The decoder firmware gets loaded at the beginning of the 
>> decoder
>> memory range and we know its length, so its possible to ioremap_nocache 
>> enough
>> room for the firmware only on init and then ioremap the remaining 
>> non-firmware
>> decoder memory areas appropriately after the firmware load succeeds...
> 
> I looked in more detail, and this would be "hard" due to the way the rest of 
> the
> decoder offsets are determined by either making firmware calls or scanning the
> decoder memory range for magic bytes and other mess.
> 
> I think some smart guy named mcgrof apparently came to the same conclusion 
> in a really old email chain I found 
> [https://lists.gt.net/linux/kernel/2387536]:
> "The ivtv case is the *worst* example we can expect where the firmware
> hides from us the exact ranges for write-combining, that we should somehow
> just hope no one will ever do again."
> :-)
> 
>> Perhaps the easy answer is to change the fatal is-pat-enabled check to just a
>> warning like "you have PAT enabled, so wc is disabled for the framebuffer.
>> if you want wc, use the nopat parameter"?
> 
> I like this idea more and more. I haven't experience any problems running
> with PAT-enabled and no write-combining on the framebuffer. Any objections?
> 
> 

None from me. 

However, since you have the hardware, you could see if you can use the 
change_page_attr machinery to change the memory type on the framebuffer once 
you figure out where it is.

Re: ivtv: use arch_phys_wc_add() and require PAT disabled

[French, Nicholas A.]

On Wed, Mar 07, 2018 at 11:23:09PM -0600, French, Nicholas A. wrote:
> On Thu, Mar 08, 2018 at 04:14:11AM +, Luis R. Rodriguez wrote:
> > On Thu, Mar 08, 2018 at 04:06:01AM +, Luis R. Rodriguez wrote:
> > > On Thu, Mar 08, 2018 at 03:16:29AM +, French, Nicholas A. wrote:
> > > >
> > > > Ah, I see. So my proposed ioremap_wc call was only "working" by 
> > > > aliasing the
> > > > ioremap_nocache()'d mem area and not actually using write combining at 
> > > > all.
> > >
> > > There are some debugging PAT toys out there I think but I haven't played 
> > > with
> > > them yet or I forgot how to to confirm or deny this sort of effort, but
> > > likeley.
> >
> >  In fact come to think of it I believe some neurons are telling me that if
> >  two type does not match we'd get an error?

I can confirm that my original suggested patch just aliases to ivtv-driver's 
nocache mapping:
$ sudo modprobe ivtvfb
$ sudo dmesg
...
x86/PAT: Overlap at 0xd500-0xd580
x86/PAT: reserve_memtype added [mem 0xd551-0xd56b0fff], track 
uncached-minus, req write-combining, ret uncached-minus
ivtvfb0: Framebuffer at 0xd551, mapped to 0xc6a7ed52, size 1665k
...
$ sudo cat /sys/kernel/debug/x86/pat_memtype_list | grep 0xd5
uncached-minus @ 0xd500-0xd580
uncached-minus @ 0xd551-0xd56b1000

So nix that.

> > No what if the framebuffer driver is just requested as a secondary step
> > after firmware loading?
>
> Its a possibility. The decoder firmware gets loaded at the beginning of the 
> decoder
> memory range and we know its length, so its possible to ioremap_nocache enough
> room for the firmware only on init and then ioremap the remaining non-firmware
> decoder memory areas appropriately after the firmware load succeeds...

I looked in more detail, and this would be "hard" due to the way the rest of the
decoder offsets are determined by either making firmware calls or scanning the
decoder memory range for magic bytes and other mess.

I think some smart guy named mcgrof apparently came to the same conclusion 
in a really old email chain I found [https://lists.gt.net/linux/kernel/2387536]:
"The ivtv case is the *worst* example we can expect where the firmware
hides from us the exact ranges for write-combining, that we should somehow
just hope no one will ever do again."
:-)

> Perhaps the easy answer is to change the fatal is-pat-enabled check to just a
> warning like "you have PAT enabled, so wc is disabled for the framebuffer.
> if you want wc, use the nopat parameter"?

I like this idea more and more. I haven't experience any problems running
with PAT-enabled and no write-combining on the framebuffer. Any objections?

- Nick

Re: ivtv: use arch_phys_wc_add() and require PAT disabled

[French, Nicholas A.]

On Thu, Mar 08, 2018 at 04:14:11AM +, Luis R. Rodriguez wrote:
> On Thu, Mar 08, 2018 at 04:06:01AM +, Luis R. Rodriguez wrote:
>  > On Thu, Mar 08, 2018 at 03:16:29AM +, French, Nicholas A. wrote:
> > > 
>  > > Ah, I see. So my proposed ioremap_wc call was only "working" by aliasing 
> the
>  > > ioremap_nocache()'d mem area and not actually using write combining at 
> all.
>  > 
>  > There are some debugging PAT toys out there I think but I haven't played 
> with
>  > them yet or I forgot how to to confirm or deny this sort of effort, but
>  > likeley.
> 
>  In fact come to think of it I believe some neurons are telling me that if
>  two type does not match we'd get an error?

I based my guess on some text i read in "PATting Linux" [1]:
"ioremap interfaces will succeed if there is an existing,
more lenient mapping. Example: If there is an existing
uncached mapping to a physical range, any request for
write-back or write-combine mapping will succeed, but
will eventually map the memory as uncached"

But I will try to get some debugpat going to confirm.

[1] = https://www.kernel.org/doc/ols/2008/ols2008v2-pages-135-144.pdf

> > So unless there is a io-re-remap to change the caching status of a subset of
> > the decoder's memory once we find out what the framebuffer offset is inside
> > the original iremap_nocache'd area, then its a no go for write combining to
> > the framebuffer with PAT.
> 
> No what if the framebuffer driver is just requested as a secondary step
> after firmware loading?

Its a possibility. The decoder firmware gets loaded at the beginning of the 
decoder
memory range and we know its length, so its possible to ioremap_nocache enough 
room for the firmware only on init and then ioremap the remaining non-firmware
decoder memory areas appropriately after the firmware load succeeds...
 
> > > On the other hand, it works fine for me with a nocache'd framebuffer. It's
> > > certainly better for me personally to have a nocache framebuffer with
> > > PAT-enabled than the framebuffer completely disabled with PAT-enabled, 
> > > but I
> > > don't think I would even propose to rollback the x86 nopat requirement in
> > > general. Apparently the throngs of people using this super-popular driver
> > > feature haven't complained in the last couple years, so maybe its OK for 
> > > me
> > > to just patch the pat-enabled guard out and deal with a nocache'd
> > > framebuffer.
> > 
> > Nope, best you add a feature to just let you disable wc stuff, to enable
> > life with PAT.

I'm not sure I understand what you mean.

Perhaps the easy answer is to change the fatal is-pat-enabled check to just a
warning like "you have PAT enabled, so wc is disabled for the framebuffer. 
if you want wc, use the nopat parameter"?

- Nick

Re: ivtv: use arch_phys_wc_add() and require PAT disabled

[Luis R. Rodriguez]

On Thu, Mar 08, 2018 at 04:06:01AM +, Luis R. Rodriguez wrote:
> On Thu, Mar 08, 2018 at 03:16:29AM +, French, Nicholas A. wrote:
> > On Wed, Mar 07, 2018 at 07:02:05PM +, Luis R. Rodriguez wrote:
> > > On Tue, Mar 06, 2018 at 09:01:10PM +, French, Nicholas A. wrote:
> > > > any reason why PAT can't be enabled for ivtvfb as simply as in the 
> > > > attached
> > > > patch?
> > >
> > > Prior to your change the OSD buffer was obtained using the itv->dec_mem + 
> > > oi->video_rbase
> > > given itv->dec_mem was initialized via [...]
> > > itv->dec_mem = ioremap_nocache(itv->base_addr + 
> > > IVTV_DECODER_OFFSET - oi->video_buffer_size,
> > > IVTV_DECODER_SIZE);
> > 
> > Ah, I see. So my proposed ioremap_wc call was only "working" by aliasing the
> > ioremap_nocache()'d mem area and not actually using write combining at all.
> 
> There are some debugging PAT toys out there I think but I haven't played with
> them yet or I forgot how to to confirm or deny this sort of effort, but
> likeley.

In fact come to think of it I believe some neurons are telling me that if
two type does not match we'd get an error?

  Luis

Re: ivtv: use arch_phys_wc_add() and require PAT disabled

[Luis R. Rodriguez]

On Thu, Mar 08, 2018 at 03:16:29AM +, French, Nicholas A. wrote:
> On Wed, Mar 07, 2018 at 07:02:05PM +, Luis R. Rodriguez wrote:
> > On Tue, Mar 06, 2018 at 09:01:10PM +, French, Nicholas A. wrote:
> > > any reason why PAT can't be enabled for ivtvfb as simply as in the 
> > > attached
> > > patch?
> >
> > Prior to your change the OSD buffer was obtained using the itv->dec_mem + 
> > oi->video_rbase
> > given itv->dec_mem was initialized via [...]
> > itv->dec_mem = ioremap_nocache(itv->base_addr + IVTV_DECODER_OFFSET 
> > - oi->video_buffer_size,
> > IVTV_DECODER_SIZE);
> 
> Ah, I see. So my proposed ioremap_wc call was only "working" by aliasing the
> ioremap_nocache()'d mem area and not actually using write combining at all.

There are some debugging PAT toys out there I think but I haven't played with
them yet or I forgot how to to confirm or deny this sort of effort, but
likeley.

> > So what I'd do is change the ioremap_nocache()'d size by substracting
> > oi->video_buffer_size -- but then you have to ask yourself how you'd get
> > that size. If its something you can figure out then great.
> 
> Size is easy since its hardcoded, but unfortunately getting the offset of the
> framebuffer inside the decoders memory to remove from the ioremap_nocache
> call is a chicken and egg problem: the offset is determined by querying the
> firmware that has been loaded to the decoder. the firmware itself will be
> loaded after the ioremap_nocache call at an offset from the address it
> returns.

What I expected. Probably why no one had tried before to clean it up.

> So unless there is a io-re-remap to change the caching status of a subset of
> the decoder's memory once we find out what the framebuffer offset is inside
> the original iremap_nocache'd area, then its a no go for write combining to
> the framebuffer with PAT.

No what if the framebuffer driver is just requested as a secondary step
after firmware loading?

> On the other hand, it works fine for me with a nocache'd framebuffer. It's
> certainly better for me personally to have a nocache framebuffer with
> PAT-enabled than the framebuffer completely disabled with PAT-enabled, but I
> don't think I would even propose to rollback the x86 nopat requirement in
> general. Apparently the throngs of people using this super-popular driver
> feature haven't complained in the last couple years, so maybe its OK for me
> to just patch the pat-enabled guard out and deal with a nocache'd
> framebuffer.

Nope, best you add a feature to just let you disable wc stuff, to enable
life with PAT.

  Luis

Re: ivtv: use arch_phys_wc_add() and require PAT disabled

[French, Nicholas A.]

On Wed, Mar 07, 2018 at 07:02:05PM +, Luis R. Rodriguez wrote:
> On Tue, Mar 06, 2018 at 09:01:10PM +, French, Nicholas A. wrote:
> > any reason why PAT can't be enabled for ivtvfb as simply as in the attached
> > patch?
>
> Prior to your change the OSD buffer was obtained using the itv->dec_mem + 
> oi->video_rbase
> given itv->dec_mem was initialized via [...]
> itv->dec_mem = ioremap_nocache(itv->base_addr + IVTV_DECODER_OFFSET - 
> oi->video_buffer_size,
> IVTV_DECODER_SIZE);

Ah, I see. So my proposed ioremap_wc call was only "working" by aliasing the 
ioremap_nocache()'d mem area and not actually using write combining at all.

> So what I'd do is change the ioremap_nocache()'d size by substracting
> oi->video_buffer_size -- but then you have to ask yourself how you'd get
> that size. If its something you can figure out then great.

Size is easy since its hardcoded, but unfortunately getting the offset of the 
framebuffer inside the decoders memory to remove from the ioremap_nocache call 
is a chicken and egg problem: the offset is determined by querying the firmware 
that has been loaded to the decoder. the firmware itself will be loaded after 
the ioremap_nocache call at an offset from the address it returns.

So unless there is a io-re-remap to change the caching status of a subset of 
the decoder's memory once we find out what the framebuffer offset is inside the 
original iremap_nocache'd area, then its a no go for write combining to the 
framebuffer with PAT.

On the other hand, it works fine for me with a nocache'd framebuffer. It's 
certainly better for me personally to have a nocache framebuffer with 
PAT-enabled than the framebuffer completely disabled with PAT-enabled, but I 
don't think I would even propose to rollback the x86 nopat requirement in 
general. Apparently the throngs of people using this super-popular driver 
feature haven't complained in the last couple years, so maybe its OK for me to 
just patch the pat-enabled guard out and deal with a nocache'd framebuffer.

- Nick

Re: ivtv: use arch_phys_wc_add() and require PAT disabled

[Luis R. Rodriguez]

On Tue, Mar 06, 2018 at 09:01:10PM +, French, Nicholas A. wrote:
> any reason why PAT can't be enabled for ivtvfb as simply as in the attached
> patch?

diff --git a/drivers/media/pci/ivtv/ivtvfb.c b/drivers/media/pci/ivtv/ivtvfb.c
index 621b2f613d81..69de110726e8 100644
--- a/drivers/media/pci/ivtv/ivtvfb.c
+++ b/drivers/media/pci/ivtv/ivtvfb.c
@@ -1117,7 +1117,7 @@ static int ivtvfb_init_io(struct ivtv *itv)
oi->video_buffer_size = 1704960;
 
oi->video_pbase = itv->base_addr + IVTV_DECODER_OFFSET + 
oi->video_rbase;
-   oi->video_vbase = itv->dec_mem + oi->video_rbase;
+   oi->video_vbase = ioremap_wc(oi->video_pbase, oi->video_buffer_size);

Note that this is the OSD buffer setup. The OSD buffer info is setup at the
start of the routine:

struct osd_info *oi = itv->osd_info;

And note that itv->osd_info is kzalloc()'d via ivtvfb_init_card() right before
ivtvfb_init_io(), which is the routine you are modifying.

Prior to your change the OSD buffer was obtained using the itv->dec_mem + 
oi->video_rbase
given itv->dec_mem was initialized via the ivtv driver module, one of which's C 
files
is:

drivers/media/pci/ivtv/ivtv-driver.c

and has:

   if (itv->has_cx23415) {
...
itv->dec_mem = ioremap_nocache(itv->base_addr + IVTV_DECODER_OFFSET - 
oi->video_buffer_size,
IVTV_DECODER_SIZE);
...
   else {
itv->dec_mem = itv->enc_mem;
   }


The way it used to work then it seems to be that we have a main ivtv driver 
which
does the ioremap off of the decoder and uses that as offset. If its not
the special cx23415 it still sets the decoder mapped offset to the encoder
offset.

So if you wanted to do what you mention in the above hunk I think you'd then
have to also proactively reduce the size of the ioremap_nocache()'d size on
the ivtv driver first. It would probably make your programming easier if
you know if the cx23415 had no frame buffer too, as then the ivtvfb driver
would not have to be concerned for variants, or the ivtv change would only
be relevant for cx23415 varaint users.

So what I'd do is change the ioremap_nocache()'d size by substracting
oi->video_buffer_size -- but then you have to ask yourself how you'd get
that size. If its something you can figure out then great.

The ivtv driver is a bit odd in that ivtvfb_init() will issue
ivtvfb_callback_init() on each registered device the ivtv driver registered, so
care must be taken with order as well on tear down.

Good luck!

  Luis

@@ -1157,6 +1157,8 @@ static void ivtvfb_release_buffers (struct ivtv *itv)
/* Release pseudo palette */
kfree(oi->ivtvfb_info.pseudo_palette);
arch_phys_wc_del(oi->wc_cookie);
+   if (oi->video_vbase)
+   iounmap(oi->video_vbase);
kfree(oi);
itv->osd_info = NULL;
 }
@@ -1167,13 +1169,6 @@ static int ivtvfb_init_card(struct ivtv *itv)
 {
int rc;
 
-#ifdef CONFIG_X86_64
-   if (pat_enabled()) {
-   pr_warn("ivtvfb needs PAT disabled, boot with nopat kernel 
parameter\n");
-   return -ENODEV;
-   }
-#endif
-
if (itv->osd_info) {
IVTVFB_ERR("Card %d already initialised\n", ivtvfb_card_id);
return -EBUSY;

[PATCH v6 1/3] ivtv: use arch_phys_wc_add() and require PAT disabled

[Luis R. Rodriguez]

From: Luis R. Rodriguez mcg...@suse.com

We are burrying direct access to MTRR code support on
x86 in order to take advantage of PAT. In the future we
also want to make the default behaviour of ioremap_nocache()
to use strong UC, use of mtrr_add() on those systems
would make write-combining void.

In order to help both enable us to later make strong
UC default and in order to phase out direct MTRR access
code port the driver over to arch_phys_wc_add() and
annotate that the device driver requires systems to
boot with PAT disabled, with the nopat kernel parameter.

This is a worthy comprmise given that the hardware is
really rare these days, and perhaps only some lost souls
in some third world country are expected to be using this
feature of the device driver.

Acked-by: Andy Walls awa...@md.metrocast.net
Cc: Andy Walls awa...@md.metrocast.net
Cc: Doug Ledford dledf...@redhat.com
Cc: Mauro Carvalho Chehab mche...@osg.samsung.com
Cc: Andy Lutomirski l...@amacapital.net
Cc: Suresh Siddha sbsid...@gmail.com
Cc: Ingo Molnar mi...@elte.hu
Cc: Thomas Gleixner t...@linutronix.de
Cc: Juergen Gross jgr...@suse.com
Cc: Daniel Vetter daniel.vet...@ffwll.ch
Cc: Dave Airlie airl...@redhat.com
Cc: Bjorn Helgaas bhelg...@google.com
Cc: Antonino Daplas adap...@gmail.com
Cc: Jean-Christophe Plagniol-Villard plagn...@jcrosoft.com
Cc: Tomi Valkeinen tomi.valkei...@ti.com
Cc: Dave Hansen dave.han...@linux.intel.com
Cc: Arnd Bergmann a...@arndb.de
Cc: Michael S. Tsirkin m...@redhat.com
Cc: Stefan Bader stefan.ba...@canonical.com
Cc: Ville Syrjälä syrj...@sci.fi
Cc: Mel Gorman mgor...@suse.de
Cc: Vlastimil Babka vba...@suse.cz
Cc: Borislav Petkov b...@suse.de
Cc: Davidlohr Bueso dbu...@suse.de
Cc: konrad.w...@oracle.com
Cc: ville.syrj...@linux.intel.com
Cc: david.vra...@citrix.com
Cc: jbeul...@suse.com
Cc: toshi.k...@hp.com
Cc: Roger Pau Monné roger@citrix.com
Cc: linux-fb...@vger.kernel.org
Cc: linux-ker...@vger.kernel.org
Cc: ivtv-de...@ivtvdriver.org
Cc: linux-media@vger.kernel.org
Cc: xen-de...@lists.xensource.com
Signed-off-by: Luis R. Rodriguez mcg...@suse.com
---
 drivers/media/pci/ivtv/Kconfig  |  3 +++
 drivers/media/pci/ivtv/ivtvfb.c | 58 -
 2 files changed, 26 insertions(+), 35 deletions(-)

diff --git a/drivers/media/pci/ivtv/Kconfig b/drivers/media/pci/ivtv/Kconfig
index dd6ee57e..b2a7f88 100644
--- a/drivers/media/pci/ivtv/Kconfig
+++ b/drivers/media/pci/ivtv/Kconfig
@@ -57,5 +57,8 @@ config VIDEO_FB_IVTV
  This is used in the Hauppauge PVR-350 card. There is a driver
  homepage at http://www.ivtvdriver.org.
 
+ If you have this hardware you will need to boot with PAT disabled
+ on your x86 systems, use the nopat kernel parameter.
+
  To compile this driver as a module, choose M here: the
  module will be called ivtvfb.
diff --git a/drivers/media/pci/ivtv/ivtvfb.c b/drivers/media/pci/ivtv/ivtvfb.c
index 9ff1230..7685ae3 100644
--- a/drivers/media/pci/ivtv/ivtvfb.c
+++ b/drivers/media/pci/ivtv/ivtvfb.c
@@ -44,8 +44,8 @@
 #include linux/ivtvfb.h
 #include linux/slab.h
 
-#ifdef CONFIG_MTRR
-#include asm/mtrr.h
+#ifdef CONFIG_X86_64
+#include asm/pat.h
 #endif
 
 #include ivtv-driver.h
@@ -155,12 +155,11 @@ struct osd_info {
/* Buffer size */
u32 video_buffer_size;
 
-#ifdef CONFIG_MTRR
/* video_base rounded down as required by hardware MTRRs */
unsigned long fb_start_aligned_physaddr;
/* video_base rounded up as required by hardware MTRRs */
unsigned long fb_end_aligned_physaddr;
-#endif
+   int wc_cookie;
 
/* Store the buffer offset */
int set_osd_coords_x;
@@ -1099,6 +1098,8 @@ static int ivtvfb_init_vidmode(struct ivtv *itv)
 static int ivtvfb_init_io(struct ivtv *itv)
 {
struct osd_info *oi = itv-osd_info;
+   /* Find the largest power of two that maps the whole buffer */
+   int size_shift = 31;
 
mutex_lock(itv-serialize_lock);
if (ivtv_init_on_first_open(itv)) {
@@ -1132,29 +1133,16 @@ static int ivtvfb_init_io(struct ivtv *itv)
oi-video_pbase, oi-video_vbase,
oi-video_buffer_size / 1024);
 
-#ifdef CONFIG_MTRR
-   {
-   /* Find the largest power of two that maps the whole buffer */
-   int size_shift = 31;
-
-   while (!(oi-video_buffer_size  (1  size_shift))) {
-   size_shift--;
-   }
-   size_shift++;
-   oi-fb_start_aligned_physaddr = oi-video_pbase  ~((1  
size_shift) - 1);
-   oi-fb_end_aligned_physaddr = oi-video_pbase + 
oi-video_buffer_size;
-   oi-fb_end_aligned_physaddr += (1  size_shift) - 1;
-   oi-fb_end_aligned_physaddr = ~((1  size_shift) - 1);
-   if (mtrr_add(oi-fb_start_aligned_physaddr,
-   oi-fb_end_aligned_physaddr - 
oi-fb_start_aligned_physaddr,
-

[PATCH v7 1/3] ivtv: use arch_phys_wc_add() and require PAT disabled

[Luis R. Rodriguez]

From: Luis R. Rodriguez mcg...@suse.com

We are burrying direct access to MTRR code support on
x86 in order to take advantage of PAT. In the future we
also want to make the default behaviour of ioremap_nocache()
to use strong UC, use of mtrr_add() on those systems
would make write-combining void.

In order to help both enable us to later make strong
UC default and in order to phase out direct MTRR access
code port the driver over to arch_phys_wc_add() and
annotate that the device driver requires systems to
boot with PAT disabled, with the nopat kernel parameter.

This is a worthy compromise given that the hardware is
really rare these days, and perhaps only some lost souls
in some third world country are expected to be using this
feature of the device driver.

Acked-by: Andy Walls awa...@md.metrocast.net
Acked-by: Mauro Carvalho Chehab mche...@osg.samsung.com
Cc: Andy Walls awa...@md.metrocast.net
Cc: Doug Ledford dledf...@redhat.com
Cc: Mauro Carvalho Chehab mche...@osg.samsung.com
Cc: Andy Lutomirski l...@amacapital.net
Cc: Suresh Siddha sbsid...@gmail.com
Cc: Ingo Molnar mi...@elte.hu
Cc: Thomas Gleixner t...@linutronix.de
Cc: Juergen Gross jgr...@suse.com
Cc: Daniel Vetter daniel.vet...@ffwll.ch
Cc: Dave Airlie airl...@redhat.com
Cc: Bjorn Helgaas bhelg...@google.com
Cc: Antonino Daplas adap...@gmail.com
Cc: Jean-Christophe Plagniol-Villard plagn...@jcrosoft.com
Cc: Tomi Valkeinen tomi.valkei...@ti.com
Cc: Dave Hansen dave.han...@linux.intel.com
Cc: Arnd Bergmann a...@arndb.de
Cc: Michael S. Tsirkin m...@redhat.com
Cc: Stefan Bader stefan.ba...@canonical.com
Cc: Ville Syrjälä syrj...@sci.fi
Cc: Borislav Petkov b...@suse.de
Cc: konrad.w...@oracle.com
Cc: toshi.k...@hp.com
Cc: Roger Pau Monné roger@citrix.com
Cc: linux-media@vger.kernel.org
Cc: xen-de...@lists.xensource.com
Cc: linux-ker...@vger.kernel.org
Signed-off-by: Luis R. Rodriguez mcg...@suse.com
---
 drivers/media/pci/ivtv/Kconfig  |  3 +++
 drivers/media/pci/ivtv/ivtvfb.c | 58 -
 2 files changed, 26 insertions(+), 35 deletions(-)

diff --git a/drivers/media/pci/ivtv/Kconfig b/drivers/media/pci/ivtv/Kconfig
index dd6ee57e..6e5867c 100644
--- a/drivers/media/pci/ivtv/Kconfig
+++ b/drivers/media/pci/ivtv/Kconfig
@@ -57,5 +57,8 @@ config VIDEO_FB_IVTV
  This is used in the Hauppauge PVR-350 card. There is a driver
  homepage at http://www.ivtvdriver.org.
 
+ In order to use this module, you will need to boot with PAT disabled
+ on x86 systems, using the nopat kernel parameter.
+
  To compile this driver as a module, choose M here: the
  module will be called ivtvfb.
diff --git a/drivers/media/pci/ivtv/ivtvfb.c b/drivers/media/pci/ivtv/ivtvfb.c
index 9ff1230..4cb365d 100644
--- a/drivers/media/pci/ivtv/ivtvfb.c
+++ b/drivers/media/pci/ivtv/ivtvfb.c
@@ -44,8 +44,8 @@
 #include linux/ivtvfb.h
 #include linux/slab.h
 
-#ifdef CONFIG_MTRR
-#include asm/mtrr.h
+#ifdef CONFIG_X86_64
+#include asm/pat.h
 #endif
 
 #include ivtv-driver.h
@@ -155,12 +155,11 @@ struct osd_info {
/* Buffer size */
u32 video_buffer_size;
 
-#ifdef CONFIG_MTRR
/* video_base rounded down as required by hardware MTRRs */
unsigned long fb_start_aligned_physaddr;
/* video_base rounded up as required by hardware MTRRs */
unsigned long fb_end_aligned_physaddr;
-#endif
+   int wc_cookie;
 
/* Store the buffer offset */
int set_osd_coords_x;
@@ -1099,6 +1098,8 @@ static int ivtvfb_init_vidmode(struct ivtv *itv)
 static int ivtvfb_init_io(struct ivtv *itv)
 {
struct osd_info *oi = itv-osd_info;
+   /* Find the largest power of two that maps the whole buffer */
+   int size_shift = 31;
 
mutex_lock(itv-serialize_lock);
if (ivtv_init_on_first_open(itv)) {
@@ -1132,29 +1133,16 @@ static int ivtvfb_init_io(struct ivtv *itv)
oi-video_pbase, oi-video_vbase,
oi-video_buffer_size / 1024);
 
-#ifdef CONFIG_MTRR
-   {
-   /* Find the largest power of two that maps the whole buffer */
-   int size_shift = 31;
-
-   while (!(oi-video_buffer_size  (1  size_shift))) {
-   size_shift--;
-   }
-   size_shift++;
-   oi-fb_start_aligned_physaddr = oi-video_pbase  ~((1  
size_shift) - 1);
-   oi-fb_end_aligned_physaddr = oi-video_pbase + 
oi-video_buffer_size;
-   oi-fb_end_aligned_physaddr += (1  size_shift) - 1;
-   oi-fb_end_aligned_physaddr = ~((1  size_shift) - 1);
-   if (mtrr_add(oi-fb_start_aligned_physaddr,
-   oi-fb_end_aligned_physaddr - 
oi-fb_start_aligned_physaddr,
-MTRR_TYPE_WRCOMB, 1)  0) {
-   IVTVFB_INFO(disabled mttr\n);
-   oi-fb_start_aligned_physaddr = 0;
-   oi-fb_end_aligned_physaddr = 0;
-  

Re: [PATCH v6 1/3] ivtv: use arch_phys_wc_add() and require PAT disabled

[Hans Verkuil]

On 06/09/2015 02:56 AM, Mauro Carvalho Chehab wrote:
 Em Mon, 08 Jun 2015 17:20:20 -0700
 Luis R. Rodriguez mcg...@do-not-panic.com escreveu:
 
 From: Luis R. Rodriguez mcg...@suse.com

 We are burrying direct access to MTRR code support on
 x86 in order to take advantage of PAT. In the future we
 also want to make the default behaviour of ioremap_nocache()
 to use strong UC, use of mtrr_add() on those systems
 would make write-combining void.

 In order to help both enable us to later make strong
 UC default and in order to phase out direct MTRR access
 code port the driver over to arch_phys_wc_add() and
 annotate that the device driver requires systems to
 boot with PAT disabled, with the nopat kernel parameter.

 This is a worthy comprmise given that the hardware is
 really rare these days, and perhaps only some lost souls
 in some third world country are expected to be using this
 feature of the device driver.

 Acked-by: Andy Walls awa...@md.metrocast.net
 Cc: Andy Walls awa...@md.metrocast.net
 Cc: Doug Ledford dledf...@redhat.com
 Cc: Mauro Carvalho Chehab mche...@osg.samsung.com
 
 Provided that you fix the issues below:
 Acked-by: Mauro Carvalho Chehab mche...@osg.samsung.com
 
 Cc: Andy Lutomirski l...@amacapital.net
 Cc: Suresh Siddha sbsid...@gmail.com
 Cc: Ingo Molnar mi...@elte.hu
 Cc: Thomas Gleixner t...@linutronix.de
 Cc: Juergen Gross jgr...@suse.com
 Cc: Daniel Vetter daniel.vet...@ffwll.ch
 Cc: Dave Airlie airl...@redhat.com
 Cc: Bjorn Helgaas bhelg...@google.com
 Cc: Antonino Daplas adap...@gmail.com
 Cc: Jean-Christophe Plagniol-Villard plagn...@jcrosoft.com
 Cc: Tomi Valkeinen tomi.valkei...@ti.com
 Cc: Dave Hansen dave.han...@linux.intel.com
 Cc: Arnd Bergmann a...@arndb.de
 Cc: Michael S. Tsirkin m...@redhat.com
 Cc: Stefan Bader stefan.ba...@canonical.com
 Cc: Ville Syrjälä syrj...@sci.fi
 Cc: Mel Gorman mgor...@suse.de
 Cc: Vlastimil Babka vba...@suse.cz
 Cc: Borislav Petkov b...@suse.de
 Cc: Davidlohr Bueso dbu...@suse.de
 Cc: konrad.w...@oracle.com
 Cc: ville.syrj...@linux.intel.com
 Cc: david.vra...@citrix.com
 Cc: jbeul...@suse.com
 Cc: toshi.k...@hp.com
 Cc: Roger Pau Monné roger@citrix.com
 Cc: linux-fb...@vger.kernel.org
 Cc: linux-ker...@vger.kernel.org
 Cc: ivtv-de...@ivtvdriver.org
 Cc: linux-media@vger.kernel.org
 Cc: xen-de...@lists.xensource.com
 Signed-off-by: Luis R. Rodriguez mcg...@suse.com
 ---
  drivers/media/pci/ivtv/Kconfig  |  3 +++
  drivers/media/pci/ivtv/ivtvfb.c | 58 
 -
  2 files changed, 26 insertions(+), 35 deletions(-)

 diff --git a/drivers/media/pci/ivtv/Kconfig b/drivers/media/pci/ivtv/Kconfig
 index dd6ee57e..b2a7f88 100644
 --- a/drivers/media/pci/ivtv/Kconfig
 +++ b/drivers/media/pci/ivtv/Kconfig
 @@ -57,5 +57,8 @@ config VIDEO_FB_IVTV
This is used in the Hauppauge PVR-350 card. There is a driver
homepage at http://www.ivtvdriver.org.
  
 +  If you have this hardware you will need to boot with PAT disabled
 +  on your x86 systems, use the nopat kernel parameter.
 +
 
 Hmm... FB_IVTV is not hardware... it is framebuffer support for IVTV.
 It is optional to use FB API for the video output port of this board,
 instead of using V4L2 API.

That's not true. It is hardware: it drives the video output OSD which overlays
the video output. The reason it is optional is that it is hard to unload a
framebuffer module and most people don't need it.

So V4L2 drives the video output and ivtvfb drives the OSD overlay. So it is
not a case of 'instead of'.

 
 I would say, instead, something like: 
 
   In order to use this module, you will need to boot with PAT disabled
 on x86 systems, using the nopat kernel parameter.

I do agree with this change, but that's because this module is optional and
not for the reasons you mentioned above.

Regards,

Hans

 
To compile this driver as a module, choose M here: the
module will be called ivtvfb.
 diff --git a/drivers/media/pci/ivtv/ivtvfb.c 
 b/drivers/media/pci/ivtv/ivtvfb.c
 index 9ff1230..7685ae3 100644
 --- a/drivers/media/pci/ivtv/ivtvfb.c
 +++ b/drivers/media/pci/ivtv/ivtvfb.c
 @@ -44,8 +44,8 @@
  #include linux/ivtvfb.h
  #include linux/slab.h
  
 -#ifdef CONFIG_MTRR
 -#include asm/mtrr.h
 +#ifdef CONFIG_X86_64
 +#include asm/pat.h
  #endif
  
  #include ivtv-driver.h
 @@ -155,12 +155,11 @@ struct osd_info {
  /* Buffer size */
  u32 video_buffer_size;
  
 -#ifdef CONFIG_MTRR
  /* video_base rounded down as required by hardware MTRRs */
  unsigned long fb_start_aligned_physaddr;
  /* video_base rounded up as required by hardware MTRRs */
  unsigned long fb_end_aligned_physaddr;
 -#endif
 +int wc_cookie;
  
  /* Store the buffer offset */
  int set_osd_coords_x;
 @@ -1099,6 +1098,8 @@ static int ivtvfb_init_vidmode(struct ivtv *itv)
  static int ivtvfb_init_io(struct ivtv *itv)
  {
  struct osd_info *oi = itv-osd_info;
 +/* Find the largest power of two that maps the 

[PATCH v6 1/3] ivtv: use arch_phys_wc_add() and require PAT disabled

[Luis R. Rodriguez]

From: Luis R. Rodriguez mcg...@suse.com

We are burrying direct access to MTRR code support on
x86 in order to take advantage of PAT. In the future we
also want to make the default behaviour of ioremap_nocache()
to use strong UC, use of mtrr_add() on those systems
would make write-combining void.

In order to help both enable us to later make strong
UC default and in order to phase out direct MTRR access
code port the driver over to arch_phys_wc_add() and
annotate that the device driver requires systems to
boot with PAT disabled, with the nopat kernel parameter.

This is a worthy comprmise given that the hardware is
really rare these days, and perhaps only some lost souls
in some third world country are expected to be using this
feature of the device driver.

Acked-by: Andy Walls awa...@md.metrocast.net
Cc: Andy Walls awa...@md.metrocast.net
Cc: Doug Ledford dledf...@redhat.com
Cc: Mauro Carvalho Chehab mche...@osg.samsung.com
Cc: Andy Lutomirski l...@amacapital.net
Cc: Suresh Siddha sbsid...@gmail.com
Cc: Ingo Molnar mi...@elte.hu
Cc: Thomas Gleixner t...@linutronix.de
Cc: Juergen Gross jgr...@suse.com
Cc: Daniel Vetter daniel.vet...@ffwll.ch
Cc: Dave Airlie airl...@redhat.com
Cc: Bjorn Helgaas bhelg...@google.com
Cc: Antonino Daplas adap...@gmail.com
Cc: Jean-Christophe Plagniol-Villard plagn...@jcrosoft.com
Cc: Tomi Valkeinen tomi.valkei...@ti.com
Cc: Dave Hansen dave.han...@linux.intel.com
Cc: Arnd Bergmann a...@arndb.de
Cc: Michael S. Tsirkin m...@redhat.com
Cc: Stefan Bader stefan.ba...@canonical.com
Cc: Ville Syrjälä syrj...@sci.fi
Cc: Mel Gorman mgor...@suse.de
Cc: Vlastimil Babka vba...@suse.cz
Cc: Borislav Petkov b...@suse.de
Cc: Davidlohr Bueso dbu...@suse.de
Cc: konrad.w...@oracle.com
Cc: ville.syrj...@linux.intel.com
Cc: david.vra...@citrix.com
Cc: jbeul...@suse.com
Cc: toshi.k...@hp.com
Cc: Roger Pau Monné roger@citrix.com
Cc: linux-fb...@vger.kernel.org
Cc: linux-ker...@vger.kernel.org
Cc: ivtv-de...@ivtvdriver.org
Cc: linux-media@vger.kernel.org
Cc: xen-de...@lists.xensource.com
Signed-off-by: Luis R. Rodriguez mcg...@suse.com
---
 drivers/media/pci/ivtv/Kconfig  |  3 +++
 drivers/media/pci/ivtv/ivtvfb.c | 58 -
 2 files changed, 26 insertions(+), 35 deletions(-)

diff --git a/drivers/media/pci/ivtv/Kconfig b/drivers/media/pci/ivtv/Kconfig
index dd6ee57e..b2a7f88 100644
--- a/drivers/media/pci/ivtv/Kconfig
+++ b/drivers/media/pci/ivtv/Kconfig
@@ -57,5 +57,8 @@ config VIDEO_FB_IVTV
  This is used in the Hauppauge PVR-350 card. There is a driver
  homepage at http://www.ivtvdriver.org.
 
+ If you have this hardware you will need to boot with PAT disabled
+ on your x86 systems, use the nopat kernel parameter.
+
  To compile this driver as a module, choose M here: the
  module will be called ivtvfb.
diff --git a/drivers/media/pci/ivtv/ivtvfb.c b/drivers/media/pci/ivtv/ivtvfb.c
index 9ff1230..7685ae3 100644
--- a/drivers/media/pci/ivtv/ivtvfb.c
+++ b/drivers/media/pci/ivtv/ivtvfb.c
@@ -44,8 +44,8 @@
 #include linux/ivtvfb.h
 #include linux/slab.h
 
-#ifdef CONFIG_MTRR
-#include asm/mtrr.h
+#ifdef CONFIG_X86_64
+#include asm/pat.h
 #endif
 
 #include ivtv-driver.h
@@ -155,12 +155,11 @@ struct osd_info {
/* Buffer size */
u32 video_buffer_size;
 
-#ifdef CONFIG_MTRR
/* video_base rounded down as required by hardware MTRRs */
unsigned long fb_start_aligned_physaddr;
/* video_base rounded up as required by hardware MTRRs */
unsigned long fb_end_aligned_physaddr;
-#endif
+   int wc_cookie;
 
/* Store the buffer offset */
int set_osd_coords_x;
@@ -1099,6 +1098,8 @@ static int ivtvfb_init_vidmode(struct ivtv *itv)
 static int ivtvfb_init_io(struct ivtv *itv)
 {
struct osd_info *oi = itv-osd_info;
+   /* Find the largest power of two that maps the whole buffer */
+   int size_shift = 31;
 
mutex_lock(itv-serialize_lock);
if (ivtv_init_on_first_open(itv)) {
@@ -1132,29 +1133,16 @@ static int ivtvfb_init_io(struct ivtv *itv)
oi-video_pbase, oi-video_vbase,
oi-video_buffer_size / 1024);
 
-#ifdef CONFIG_MTRR
-   {
-   /* Find the largest power of two that maps the whole buffer */
-   int size_shift = 31;
-
-   while (!(oi-video_buffer_size  (1  size_shift))) {
-   size_shift--;
-   }
-   size_shift++;
-   oi-fb_start_aligned_physaddr = oi-video_pbase  ~((1  
size_shift) - 1);
-   oi-fb_end_aligned_physaddr = oi-video_pbase + 
oi-video_buffer_size;
-   oi-fb_end_aligned_physaddr += (1  size_shift) - 1;
-   oi-fb_end_aligned_physaddr = ~((1  size_shift) - 1);
-   if (mtrr_add(oi-fb_start_aligned_physaddr,
-   oi-fb_end_aligned_physaddr - 
oi-fb_start_aligned_physaddr,
-

Re: [PATCH v6 1/3] ivtv: use arch_phys_wc_add() and require PAT disabled

[Mauro Carvalho Chehab]

Em Mon, 08 Jun 2015 17:20:20 -0700
Luis R. Rodriguez mcg...@do-not-panic.com escreveu:

 From: Luis R. Rodriguez mcg...@suse.com
 
 We are burrying direct access to MTRR code support on
 x86 in order to take advantage of PAT. In the future we
 also want to make the default behaviour of ioremap_nocache()
 to use strong UC, use of mtrr_add() on those systems
 would make write-combining void.
 
 In order to help both enable us to later make strong
 UC default and in order to phase out direct MTRR access
 code port the driver over to arch_phys_wc_add() and
 annotate that the device driver requires systems to
 boot with PAT disabled, with the nopat kernel parameter.
 
 This is a worthy comprmise given that the hardware is
 really rare these days, and perhaps only some lost souls
 in some third world country are expected to be using this
 feature of the device driver.
 
 Acked-by: Andy Walls awa...@md.metrocast.net
 Cc: Andy Walls awa...@md.metrocast.net
 Cc: Doug Ledford dledf...@redhat.com
 Cc: Mauro Carvalho Chehab mche...@osg.samsung.com

Provided that you fix the issues below:
Acked-by: Mauro Carvalho Chehab mche...@osg.samsung.com

 Cc: Andy Lutomirski l...@amacapital.net
 Cc: Suresh Siddha sbsid...@gmail.com
 Cc: Ingo Molnar mi...@elte.hu
 Cc: Thomas Gleixner t...@linutronix.de
 Cc: Juergen Gross jgr...@suse.com
 Cc: Daniel Vetter daniel.vet...@ffwll.ch
 Cc: Dave Airlie airl...@redhat.com
 Cc: Bjorn Helgaas bhelg...@google.com
 Cc: Antonino Daplas adap...@gmail.com
 Cc: Jean-Christophe Plagniol-Villard plagn...@jcrosoft.com
 Cc: Tomi Valkeinen tomi.valkei...@ti.com
 Cc: Dave Hansen dave.han...@linux.intel.com
 Cc: Arnd Bergmann a...@arndb.de
 Cc: Michael S. Tsirkin m...@redhat.com
 Cc: Stefan Bader stefan.ba...@canonical.com
 Cc: Ville Syrjälä syrj...@sci.fi
 Cc: Mel Gorman mgor...@suse.de
 Cc: Vlastimil Babka vba...@suse.cz
 Cc: Borislav Petkov b...@suse.de
 Cc: Davidlohr Bueso dbu...@suse.de
 Cc: konrad.w...@oracle.com
 Cc: ville.syrj...@linux.intel.com
 Cc: david.vra...@citrix.com
 Cc: jbeul...@suse.com
 Cc: toshi.k...@hp.com
 Cc: Roger Pau Monné roger@citrix.com
 Cc: linux-fb...@vger.kernel.org
 Cc: linux-ker...@vger.kernel.org
 Cc: ivtv-de...@ivtvdriver.org
 Cc: linux-media@vger.kernel.org
 Cc: xen-de...@lists.xensource.com
 Signed-off-by: Luis R. Rodriguez mcg...@suse.com
 ---
  drivers/media/pci/ivtv/Kconfig  |  3 +++
  drivers/media/pci/ivtv/ivtvfb.c | 58 
 -
  2 files changed, 26 insertions(+), 35 deletions(-)
 
 diff --git a/drivers/media/pci/ivtv/Kconfig b/drivers/media/pci/ivtv/Kconfig
 index dd6ee57e..b2a7f88 100644
 --- a/drivers/media/pci/ivtv/Kconfig
 +++ b/drivers/media/pci/ivtv/Kconfig
 @@ -57,5 +57,8 @@ config VIDEO_FB_IVTV
 This is used in the Hauppauge PVR-350 card. There is a driver
 homepage at http://www.ivtvdriver.org.
  
 +   If you have this hardware you will need to boot with PAT disabled
 +   on your x86 systems, use the nopat kernel parameter.
 +

Hmm... FB_IVTV is not hardware... it is framebuffer support for IVTV.
It is optional to use FB API for the video output port of this board,
instead of using V4L2 API.

I would say, instead, something like: 

In order to use this module, you will need to boot with PAT disabled
  on x86 systems, using the nopat kernel parameter.

 To compile this driver as a module, choose M here: the
 module will be called ivtvfb.
 diff --git a/drivers/media/pci/ivtv/ivtvfb.c b/drivers/media/pci/ivtv/ivtvfb.c
 index 9ff1230..7685ae3 100644
 --- a/drivers/media/pci/ivtv/ivtvfb.c
 +++ b/drivers/media/pci/ivtv/ivtvfb.c
 @@ -44,8 +44,8 @@
  #include linux/ivtvfb.h
  #include linux/slab.h
  
 -#ifdef CONFIG_MTRR
 -#include asm/mtrr.h
 +#ifdef CONFIG_X86_64
 +#include asm/pat.h
  #endif
  
  #include ivtv-driver.h
 @@ -155,12 +155,11 @@ struct osd_info {
   /* Buffer size */
   u32 video_buffer_size;
  
 -#ifdef CONFIG_MTRR
   /* video_base rounded down as required by hardware MTRRs */
   unsigned long fb_start_aligned_physaddr;
   /* video_base rounded up as required by hardware MTRRs */
   unsigned long fb_end_aligned_physaddr;
 -#endif
 + int wc_cookie;
  
   /* Store the buffer offset */
   int set_osd_coords_x;
 @@ -1099,6 +1098,8 @@ static int ivtvfb_init_vidmode(struct ivtv *itv)
  static int ivtvfb_init_io(struct ivtv *itv)
  {
   struct osd_info *oi = itv-osd_info;
 + /* Find the largest power of two that maps the whole buffer */
 + int size_shift = 31;
  
   mutex_lock(itv-serialize_lock);
   if (ivtv_init_on_first_open(itv)) {
 @@ -1132,29 +1133,16 @@ static int ivtvfb_init_io(struct ivtv *itv)
   oi-video_pbase, oi-video_vbase,
   oi-video_buffer_size / 1024);
  
 -#ifdef CONFIG_MTRR
 - {
 - /* Find the largest power of two that maps the whole buffer */
 - int size_shift = 31;
 -
 - while (!(oi-video_buffer_size 

[PATCH v5 4/6] ivtv: use arch_phys_wc_add() and require PAT disabled

[Luis R. Rodriguez]

From: Luis R. Rodriguez mcg...@suse.com

We are burrying direct access to MTRR code support on
x86 in order to take advantage of PAT. In the future we
also want to make the default behaviour of ioremap_nocache()
to use strong UC, use of mtrr_add() on those systems
would make write-combining void.

In order to help both enable us to later make strong
UC default and in order to phase out direct MTRR access
code port the driver over to arch_phys_wc_add() and
annotate that the device driver requires systems to
boot with PAT disabled, with the nopat kernel parameter.

This is a worthy comprmise given that the hardware is
really rare these days, and perhaps only some lost souls
in some third world country are expected to be using this
feature of the device driver.

Acked-by: Andy Walls awa...@md.metrocast.net
Cc: Andy Walls awa...@md.metrocast.net
Cc: Doug Ledford dledf...@redhat.com
Cc: Mauro Carvalho Chehab mche...@osg.samsung.com
Cc: Andy Lutomirski l...@amacapital.net
Cc: Suresh Siddha sbsid...@gmail.com
Cc: Ingo Molnar mi...@elte.hu
Cc: Thomas Gleixner t...@linutronix.de
Cc: Juergen Gross jgr...@suse.com
Cc: Daniel Vetter daniel.vet...@ffwll.ch
Cc: Dave Airlie airl...@redhat.com
Cc: Bjorn Helgaas bhelg...@google.com
Cc: Antonino Daplas adap...@gmail.com
Cc: Jean-Christophe Plagniol-Villard plagn...@jcrosoft.com
Cc: Tomi Valkeinen tomi.valkei...@ti.com
Cc: Dave Hansen dave.han...@linux.intel.com
Cc: Arnd Bergmann a...@arndb.de
Cc: Michael S. Tsirkin m...@redhat.com
Cc: Stefan Bader stefan.ba...@canonical.com
Cc: Ville Syrjälä syrj...@sci.fi
Cc: Mel Gorman mgor...@suse.de
Cc: Vlastimil Babka vba...@suse.cz
Cc: Borislav Petkov b...@suse.de
Cc: Davidlohr Bueso dbu...@suse.de
Cc: konrad.w...@oracle.com
Cc: ville.syrj...@linux.intel.com
Cc: david.vra...@citrix.com
Cc: jbeul...@suse.com
Cc: toshi.k...@hp.com
Cc: Roger Pau Monné roger@citrix.com
Cc: linux-fb...@vger.kernel.org
Cc: linux-ker...@vger.kernel.org
Cc: ivtv-de...@ivtvdriver.org
Cc: linux-media@vger.kernel.org
Cc: xen-de...@lists.xensource.com
Signed-off-by: Luis R. Rodriguez mcg...@suse.com
---
 drivers/media/pci/ivtv/Kconfig  |  3 +++
 drivers/media/pci/ivtv/ivtvfb.c | 58 -
 2 files changed, 26 insertions(+), 35 deletions(-)

diff --git a/drivers/media/pci/ivtv/Kconfig b/drivers/media/pci/ivtv/Kconfig
index dd6ee57e..b2a7f88 100644
--- a/drivers/media/pci/ivtv/Kconfig
+++ b/drivers/media/pci/ivtv/Kconfig
@@ -57,5 +57,8 @@ config VIDEO_FB_IVTV
  This is used in the Hauppauge PVR-350 card. There is a driver
  homepage at http://www.ivtvdriver.org.
 
+ If you have this hardware you will need to boot with PAT disabled
+ on your x86 systems, use the nopat kernel parameter.
+
  To compile this driver as a module, choose M here: the
  module will be called ivtvfb.
diff --git a/drivers/media/pci/ivtv/ivtvfb.c b/drivers/media/pci/ivtv/ivtvfb.c
index 9ff1230..7685ae3 100644
--- a/drivers/media/pci/ivtv/ivtvfb.c
+++ b/drivers/media/pci/ivtv/ivtvfb.c
@@ -44,8 +44,8 @@
 #include linux/ivtvfb.h
 #include linux/slab.h
 
-#ifdef CONFIG_MTRR
-#include asm/mtrr.h
+#ifdef CONFIG_X86_64
+#include asm/pat.h
 #endif
 
 #include ivtv-driver.h
@@ -155,12 +155,11 @@ struct osd_info {
/* Buffer size */
u32 video_buffer_size;
 
-#ifdef CONFIG_MTRR
/* video_base rounded down as required by hardware MTRRs */
unsigned long fb_start_aligned_physaddr;
/* video_base rounded up as required by hardware MTRRs */
unsigned long fb_end_aligned_physaddr;
-#endif
+   int wc_cookie;
 
/* Store the buffer offset */
int set_osd_coords_x;
@@ -1099,6 +1098,8 @@ static int ivtvfb_init_vidmode(struct ivtv *itv)
 static int ivtvfb_init_io(struct ivtv *itv)
 {
struct osd_info *oi = itv-osd_info;
+   /* Find the largest power of two that maps the whole buffer */
+   int size_shift = 31;
 
mutex_lock(itv-serialize_lock);
if (ivtv_init_on_first_open(itv)) {
@@ -1132,29 +1133,16 @@ static int ivtvfb_init_io(struct ivtv *itv)
oi-video_pbase, oi-video_vbase,
oi-video_buffer_size / 1024);
 
-#ifdef CONFIG_MTRR
-   {
-   /* Find the largest power of two that maps the whole buffer */
-   int size_shift = 31;
-
-   while (!(oi-video_buffer_size  (1  size_shift))) {
-   size_shift--;
-   }
-   size_shift++;
-   oi-fb_start_aligned_physaddr = oi-video_pbase  ~((1  
size_shift) - 1);
-   oi-fb_end_aligned_physaddr = oi-video_pbase + 
oi-video_buffer_size;
-   oi-fb_end_aligned_physaddr += (1  size_shift) - 1;
-   oi-fb_end_aligned_physaddr = ~((1  size_shift) - 1);
-   if (mtrr_add(oi-fb_start_aligned_physaddr,
-   oi-fb_end_aligned_physaddr - 
oi-fb_start_aligned_physaddr,
-

[PATCH v4 6/8] ivtv: use arch_phys_wc_add() and require PAT disabled

[Luis R. Rodriguez]

From: Luis R. Rodriguez mcg...@suse.com

We are burrying direct access to MTRR code support on
x86 in order to take advantage of PAT. In the future we
also want to make the default behaviour of ioremap_nocache()
to use strong UC, use of mtrr_add() on those systems
would make write-combining void.

In order to help both enable us to later make strong
UC default and in order to phase out direct MTRR access
code port the driver over to arch_phys_wc_add() and
annotate that the device driver requires systems to
boot with PAT disabled, with the nopat kernel parameter.

This is a worthy comprmise given that the hardware is
really rare these days, and perhaps only some lost souls
in some third world country are expected to be using this
feature of the device driver.

Acked-by: Andy Walls awa...@md.metrocast.net
Cc: Andy Walls awa...@md.metrocast.net
Cc: Doug Ledford dledf...@redhat.com
Cc: Mauro Carvalho Chehab mche...@osg.samsung.com
Cc: Andy Lutomirski l...@amacapital.net
Cc: Suresh Siddha sbsid...@gmail.com
Cc: Ingo Molnar mi...@elte.hu
Cc: Thomas Gleixner t...@linutronix.de
Cc: Juergen Gross jgr...@suse.com
Cc: Daniel Vetter daniel.vet...@ffwll.ch
Cc: Dave Airlie airl...@redhat.com
Cc: Bjorn Helgaas bhelg...@google.com
Cc: Antonino Daplas adap...@gmail.com
Cc: Jean-Christophe Plagniol-Villard plagn...@jcrosoft.com
Cc: Tomi Valkeinen tomi.valkei...@ti.com
Cc: Dave Hansen dave.han...@linux.intel.com
Cc: Arnd Bergmann a...@arndb.de
Cc: Michael S. Tsirkin m...@redhat.com
Cc: Stefan Bader stefan.ba...@canonical.com
Cc: Ville Syrjälä syrj...@sci.fi
Cc: Mel Gorman mgor...@suse.de
Cc: Vlastimil Babka vba...@suse.cz
Cc: Borislav Petkov b...@suse.de
Cc: Davidlohr Bueso dbu...@suse.de
Cc: konrad.w...@oracle.com
Cc: ville.syrj...@linux.intel.com
Cc: david.vra...@citrix.com
Cc: jbeul...@suse.com
Cc: toshi.k...@hp.com
Cc: Roger Pau Monné roger@citrix.com
Cc: linux-fb...@vger.kernel.org
Cc: linux-ker...@vger.kernel.org
Cc: ivtv-de...@ivtvdriver.org
Cc: linux-media@vger.kernel.org
Cc: xen-de...@lists.xensource.com
Signed-off-by: Luis R. Rodriguez mcg...@suse.com
---
 drivers/media/pci/ivtv/Kconfig  |  3 +++
 drivers/media/pci/ivtv/ivtvfb.c | 58 -
 2 files changed, 26 insertions(+), 35 deletions(-)

diff --git a/drivers/media/pci/ivtv/Kconfig b/drivers/media/pci/ivtv/Kconfig
index dd6ee57e..b2a7f88 100644
--- a/drivers/media/pci/ivtv/Kconfig
+++ b/drivers/media/pci/ivtv/Kconfig
@@ -57,5 +57,8 @@ config VIDEO_FB_IVTV
  This is used in the Hauppauge PVR-350 card. There is a driver
  homepage at http://www.ivtvdriver.org.
 
+ If you have this hardware you will need to boot with PAT disabled
+ on your x86 systems, use the nopat kernel parameter.
+
  To compile this driver as a module, choose M here: the
  module will be called ivtvfb.
diff --git a/drivers/media/pci/ivtv/ivtvfb.c b/drivers/media/pci/ivtv/ivtvfb.c
index 9ff1230..7685ae3 100644
--- a/drivers/media/pci/ivtv/ivtvfb.c
+++ b/drivers/media/pci/ivtv/ivtvfb.c
@@ -44,8 +44,8 @@
 #include linux/ivtvfb.h
 #include linux/slab.h
 
-#ifdef CONFIG_MTRR
-#include asm/mtrr.h
+#ifdef CONFIG_X86_64
+#include asm/pat.h
 #endif
 
 #include ivtv-driver.h
@@ -155,12 +155,11 @@ struct osd_info {
/* Buffer size */
u32 video_buffer_size;
 
-#ifdef CONFIG_MTRR
/* video_base rounded down as required by hardware MTRRs */
unsigned long fb_start_aligned_physaddr;
/* video_base rounded up as required by hardware MTRRs */
unsigned long fb_end_aligned_physaddr;
-#endif
+   int wc_cookie;
 
/* Store the buffer offset */
int set_osd_coords_x;
@@ -1099,6 +1098,8 @@ static int ivtvfb_init_vidmode(struct ivtv *itv)
 static int ivtvfb_init_io(struct ivtv *itv)
 {
struct osd_info *oi = itv-osd_info;
+   /* Find the largest power of two that maps the whole buffer */
+   int size_shift = 31;
 
mutex_lock(itv-serialize_lock);
if (ivtv_init_on_first_open(itv)) {
@@ -1132,29 +1133,16 @@ static int ivtvfb_init_io(struct ivtv *itv)
oi-video_pbase, oi-video_vbase,
oi-video_buffer_size / 1024);
 
-#ifdef CONFIG_MTRR
-   {
-   /* Find the largest power of two that maps the whole buffer */
-   int size_shift = 31;
-
-   while (!(oi-video_buffer_size  (1  size_shift))) {
-   size_shift--;
-   }
-   size_shift++;
-   oi-fb_start_aligned_physaddr = oi-video_pbase  ~((1  
size_shift) - 1);
-   oi-fb_end_aligned_physaddr = oi-video_pbase + 
oi-video_buffer_size;
-   oi-fb_end_aligned_physaddr += (1  size_shift) - 1;
-   oi-fb_end_aligned_physaddr = ~((1  size_shift) - 1);
-   if (mtrr_add(oi-fb_start_aligned_physaddr,
-   oi-fb_end_aligned_physaddr - 
oi-fb_start_aligned_physaddr,
-

[PATCH v2] ivtv: use arch_phys_wc_add() and require PAT disabled

[Luis R. Rodriguez]

From: Luis R. Rodriguez mcg...@suse.com

We are burrying direct access to MTRR code support on
x86 in order to take advantage of PAT. In the future we
also want to make the default behaviour of ioremap_nocache()
to use strong UC, use of mtrr_add() on those systems
would make write-combining void.

In order to help both enable us to later make strong
UC default and in order to phase out direct MTRR access
code port the driver over to arch_phys_wc_add() and
annotate that the device driver requires systems to
boot with PAT disabled, with the nopat kernel parameter.

This is a worthy comprmise given that the hardware is
really rare these days, and perhaps only some lost souls
in some third world country are expected to be using this
feature of the device driver.

Cc: Andy Walls awa...@md.metrocast.net
Cc: Mauro Carvalho Chehab mche...@osg.samsung.com
Cc: Andy Lutomirski l...@amacapital.net
Cc: Suresh Siddha sbsid...@gmail.com
Cc: Ingo Molnar mi...@elte.hu
Cc: Thomas Gleixner t...@linutronix.de
Cc: Juergen Gross jgr...@suse.com
Cc: Daniel Vetter daniel.vet...@ffwll.ch
Cc: Dave Airlie airl...@redhat.com
Cc: Bjorn Helgaas bhelg...@google.com
Cc: Antonino Daplas adap...@gmail.com
Cc: Jean-Christophe Plagniol-Villard plagn...@jcrosoft.com
Cc: Tomi Valkeinen tomi.valkei...@ti.com
Cc: Dave Hansen dave.han...@linux.intel.com
Cc: Arnd Bergmann a...@arndb.de
Cc: Michael S. Tsirkin m...@redhat.com
Cc: Stefan Bader stefan.ba...@canonical.com
Cc: Ville Syrjälä syrj...@sci.fi
Cc: Mel Gorman mgor...@suse.de
Cc: Vlastimil Babka vba...@suse.cz
Cc: Borislav Petkov b...@suse.de
Cc: Davidlohr Bueso dbu...@suse.de
Cc: konrad.w...@oracle.com
Cc: ville.syrj...@linux.intel.com
Cc: david.vra...@citrix.com
Cc: jbeul...@suse.com
Cc: toshi.k...@hp.com
Cc: Roger Pau Monné roger@citrix.com
Cc: linux-fb...@vger.kernel.org
Cc: linux-ker...@vger.kernel.org
Cc: ivtv-de...@ivtvdriver.org
Cc: linux-media@vger.kernel.org
Cc: xen-de...@lists.xensource.com
Signed-off-by: Luis R. Rodriguez mcg...@suse.com
---

This v2 moves the PAT bail out error check on to ivtvfb_init()
as per Andy's request. It also removes some comment about TODO
items for PAT.

 drivers/media/pci/ivtv/Kconfig  |  3 +++
 drivers/media/pci/ivtv/ivtvfb.c | 58 -
 2 files changed, 26 insertions(+), 35 deletions(-)

diff --git a/drivers/media/pci/ivtv/Kconfig b/drivers/media/pci/ivtv/Kconfig
index dd6ee57e..b2a7f88 100644
--- a/drivers/media/pci/ivtv/Kconfig
+++ b/drivers/media/pci/ivtv/Kconfig
@@ -57,5 +57,8 @@ config VIDEO_FB_IVTV
  This is used in the Hauppauge PVR-350 card. There is a driver
  homepage at http://www.ivtvdriver.org.
 
+ If you have this hardware you will need to boot with PAT disabled
+ on your x86 systems, use the nopat kernel parameter.
+
  To compile this driver as a module, choose M here: the
  module will be called ivtvfb.
diff --git a/drivers/media/pci/ivtv/ivtvfb.c b/drivers/media/pci/ivtv/ivtvfb.c
index 9ff1230..8761e3e 100644
--- a/drivers/media/pci/ivtv/ivtvfb.c
+++ b/drivers/media/pci/ivtv/ivtvfb.c
@@ -44,8 +44,8 @@
 #include linux/ivtvfb.h
 #include linux/slab.h
 
-#ifdef CONFIG_MTRR
-#include asm/mtrr.h
+#ifdef CONFIG_X86_64
+#include asm/pat.h
 #endif
 
 #include ivtv-driver.h
@@ -155,12 +155,11 @@ struct osd_info {
/* Buffer size */
u32 video_buffer_size;
 
-#ifdef CONFIG_MTRR
/* video_base rounded down as required by hardware MTRRs */
unsigned long fb_start_aligned_physaddr;
/* video_base rounded up as required by hardware MTRRs */
unsigned long fb_end_aligned_physaddr;
-#endif
+   int wc_cookie;
 
/* Store the buffer offset */
int set_osd_coords_x;
@@ -1099,6 +1098,8 @@ static int ivtvfb_init_vidmode(struct ivtv *itv)
 static int ivtvfb_init_io(struct ivtv *itv)
 {
struct osd_info *oi = itv-osd_info;
+   /* Find the largest power of two that maps the whole buffer */
+   int size_shift = 31;
 
mutex_lock(itv-serialize_lock);
if (ivtv_init_on_first_open(itv)) {
@@ -1132,29 +1133,16 @@ static int ivtvfb_init_io(struct ivtv *itv)
oi-video_pbase, oi-video_vbase,
oi-video_buffer_size / 1024);
 
-#ifdef CONFIG_MTRR
-   {
-   /* Find the largest power of two that maps the whole buffer */
-   int size_shift = 31;
-
-   while (!(oi-video_buffer_size  (1  size_shift))) {
-   size_shift--;
-   }
-   size_shift++;
-   oi-fb_start_aligned_physaddr = oi-video_pbase  ~((1  
size_shift) - 1);
-   oi-fb_end_aligned_physaddr = oi-video_pbase + 
oi-video_buffer_size;
-   oi-fb_end_aligned_physaddr += (1  size_shift) - 1;
-   oi-fb_end_aligned_physaddr = ~((1  size_shift) - 1);
-   if (mtrr_add(oi-fb_start_aligned_physaddr,
-   oi-fb_end_aligned_physaddr - 

Re: [PATCH] [media] ivtv: use arch_phys_wc_add() and require PAT disabled

[Luis R. Rodriguez]

On Sat, Apr 25, 2015 at 07:12:05AM -0400, Andy Walls wrote:
 Hi Luis,
 
 Sorry for the late reply.
 
 Thank you for the patch! See my comments below:
 
 On Wed, 2015-04-22 at 12:33 -0700, Luis R. Rodriguez wrote:
  From: Luis R. Rodriguez mcg...@suse.com
  
  We are burrying direct access to MTRR code support on
  x86 in order to take advantage of PAT. In the future we
  also want to make the default behaviour of ioremap_nocache()
  to use strong UC, use of mtrr_add() on those systems
  would make write-combining void.
  
  In order to help both enable us to later make strong
  UC default and in order to phase out direct MTRR access
  code port the driver over to arch_phys_wc_add() and
  annotate that the device driver requires systems to
  boot with PAT disabled, with the nopat kernel parameter.
  
  This is a worthy comprmise given that the hardware is
  really rare these days,
 
 I'm OK with the compromise solution.  It makes sense.

OK great!

  diff --git a/drivers/media/pci/ivtv/ivtvfb.c 
  b/drivers/media/pci/ivtv/ivtvfb.c
  index 9ff1230..552408b 100644
  --- a/drivers/media/pci/ivtv/ivtvfb.c
  +++ b/drivers/media/pci/ivtv/ivtvfb.c
  @@ -1120,6 +1121,7 @@ static int ivtvfb_init_io(struct ivtv *itv)
  oi-video_buffer_size = 1704960;
   
  oi-video_pbase = itv-base_addr + IVTV_DECODER_OFFSET + 
  oi-video_rbase;
  +   /* XXX: split this for PAT */
 
 Please remove this comment. 

Done.

  @@ -1190,6 +1172,13 @@ static int ivtvfb_init_card(struct ivtv *itv)
   {
  int rc;
   
  +#ifdef CONFIG_X86_64
  +   if (WARN(pat_enabled,
 
 This check might be better placed in ivtvfb_init().  This check is going
 to have the same result for every PVR-350 card in the system that is
 found by ivtvfb.

OK moved!

  +ivtv needs PAT disabled, boot with nopat kernel 
  parameter\n)) {
 
 This needs to read ivtvfb needs [...] to avoid user confusion with the
 main ivtv driver module.

OK!

 This change is the only one I really care about.  Then I can give my
 Ack.

OK!

  Luis
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH v2] ivtv: use arch_phys_wc_add() and require PAT disabled

[Andy Walls]

On Mon, 2015-04-27 at 09:43 -0700, Luis R. Rodriguez wrote:
 From: Luis R. Rodriguez mcg...@suse.com
 
 We are burrying direct access to MTRR code support on
 x86 in order to take advantage of PAT. In the future we
 also want to make the default behaviour of ioremap_nocache()
 to use strong UC, use of mtrr_add() on those systems
 would make write-combining void.
 
 In order to help both enable us to later make strong
 UC default and in order to phase out direct MTRR access
 code port the driver over to arch_phys_wc_add() and
 annotate that the device driver requires systems to
 boot with PAT disabled, with the nopat kernel parameter.
 
 This is a worthy comprmise given that the hardware is
 really rare these days, and perhaps only some lost souls
 in some third world country are expected to be using this
 feature of the device driver.
 
 Cc: Andy Walls awa...@md.metrocast.net

Acked-by: Andy Walls awa...@md.metrocast.net

Regards,
Andy

 Cc: Mauro Carvalho Chehab mche...@osg.samsung.com
 Cc: Andy Lutomirski l...@amacapital.net
 Cc: Suresh Siddha sbsid...@gmail.com
 Cc: Ingo Molnar mi...@elte.hu
 Cc: Thomas Gleixner t...@linutronix.de
 Cc: Juergen Gross jgr...@suse.com
 Cc: Daniel Vetter daniel.vet...@ffwll.ch
 Cc: Dave Airlie airl...@redhat.com
 Cc: Bjorn Helgaas bhelg...@google.com
 Cc: Antonino Daplas adap...@gmail.com
 Cc: Jean-Christophe Plagniol-Villard plagn...@jcrosoft.com
 Cc: Tomi Valkeinen tomi.valkei...@ti.com
 Cc: Dave Hansen dave.han...@linux.intel.com
 Cc: Arnd Bergmann a...@arndb.de
 Cc: Michael S. Tsirkin m...@redhat.com
 Cc: Stefan Bader stefan.ba...@canonical.com
 Cc: Ville Syrjälä syrj...@sci.fi
 Cc: Mel Gorman mgor...@suse.de
 Cc: Vlastimil Babka vba...@suse.cz
 Cc: Borislav Petkov b...@suse.de
 Cc: Davidlohr Bueso dbu...@suse.de
 Cc: konrad.w...@oracle.com
 Cc: ville.syrj...@linux.intel.com
 Cc: david.vra...@citrix.com
 Cc: jbeul...@suse.com
 Cc: toshi.k...@hp.com
 Cc: Roger Pau Monné roger@citrix.com
 Cc: linux-fb...@vger.kernel.org
 Cc: linux-ker...@vger.kernel.org
 Cc: ivtv-de...@ivtvdriver.org
 Cc: linux-media@vger.kernel.org
 Cc: xen-de...@lists.xensource.com
 Signed-off-by: Luis R. Rodriguez mcg...@suse.com
 ---
 
 This v2 moves the PAT bail out error check on to ivtvfb_init()
 as per Andy's request. It also removes some comment about TODO
 items for PAT.
 
  drivers/media/pci/ivtv/Kconfig  |  3 +++
  drivers/media/pci/ivtv/ivtvfb.c | 58 
 -
  2 files changed, 26 insertions(+), 35 deletions(-)
 
 diff --git a/drivers/media/pci/ivtv/Kconfig b/drivers/media/pci/ivtv/Kconfig
 index dd6ee57e..b2a7f88 100644
 --- a/drivers/media/pci/ivtv/Kconfig
 +++ b/drivers/media/pci/ivtv/Kconfig
 @@ -57,5 +57,8 @@ config VIDEO_FB_IVTV
 This is used in the Hauppauge PVR-350 card. There is a driver
 homepage at http://www.ivtvdriver.org.
  
 +   If you have this hardware you will need to boot with PAT disabled
 +   on your x86 systems, use the nopat kernel parameter.
 +
 To compile this driver as a module, choose M here: the
 module will be called ivtvfb.
 diff --git a/drivers/media/pci/ivtv/ivtvfb.c b/drivers/media/pci/ivtv/ivtvfb.c
 index 9ff1230..8761e3e 100644
 --- a/drivers/media/pci/ivtv/ivtvfb.c
 +++ b/drivers/media/pci/ivtv/ivtvfb.c
 @@ -44,8 +44,8 @@
  #include linux/ivtvfb.h
  #include linux/slab.h
  
 -#ifdef CONFIG_MTRR
 -#include asm/mtrr.h
 +#ifdef CONFIG_X86_64
 +#include asm/pat.h
  #endif
  
  #include ivtv-driver.h
 @@ -155,12 +155,11 @@ struct osd_info {
   /* Buffer size */
   u32 video_buffer_size;
  
 -#ifdef CONFIG_MTRR
   /* video_base rounded down as required by hardware MTRRs */
   unsigned long fb_start_aligned_physaddr;
   /* video_base rounded up as required by hardware MTRRs */
   unsigned long fb_end_aligned_physaddr;
 -#endif
 + int wc_cookie;
  
   /* Store the buffer offset */
   int set_osd_coords_x;
 @@ -1099,6 +1098,8 @@ static int ivtvfb_init_vidmode(struct ivtv *itv)
  static int ivtvfb_init_io(struct ivtv *itv)
  {
   struct osd_info *oi = itv-osd_info;
 + /* Find the largest power of two that maps the whole buffer */
 + int size_shift = 31;
  
   mutex_lock(itv-serialize_lock);
   if (ivtv_init_on_first_open(itv)) {
 @@ -1132,29 +1133,16 @@ static int ivtvfb_init_io(struct ivtv *itv)
   oi-video_pbase, oi-video_vbase,
   oi-video_buffer_size / 1024);
  
 -#ifdef CONFIG_MTRR
 - {
 - /* Find the largest power of two that maps the whole buffer */
 - int size_shift = 31;
 -
 - while (!(oi-video_buffer_size  (1  size_shift))) {
 - size_shift--;
 - }
 - size_shift++;
 - oi-fb_start_aligned_physaddr = oi-video_pbase  ~((1  
 size_shift) - 1);
 - oi-fb_end_aligned_physaddr = oi-video_pbase + 
 oi-video_buffer_size;
 - oi-fb_end_aligned_physaddr += (1  size_shift) - 1;
 -  

Re: [PATCH] [media] ivtv: use arch_phys_wc_add() and require PAT disabled

[Andy Walls]

Hi Luis,

Sorry for the late reply.

Thank you for the patch! See my comments below:

On Wed, 2015-04-22 at 12:33 -0700, Luis R. Rodriguez wrote:
 From: Luis R. Rodriguez mcg...@suse.com
 
 We are burrying direct access to MTRR code support on
 x86 in order to take advantage of PAT. In the future we
 also want to make the default behaviour of ioremap_nocache()
 to use strong UC, use of mtrr_add() on those systems
 would make write-combining void.
 
 In order to help both enable us to later make strong
 UC default and in order to phase out direct MTRR access
 code port the driver over to arch_phys_wc_add() and
 annotate that the device driver requires systems to
 boot with PAT disabled, with the nopat kernel parameter.
 
 This is a worthy comprmise given that the hardware is
 really rare these days,

I'm OK with the compromise solution.  It makes sense.

  and perhaps only some lost souls
 in some third world country are expected to be using this
 feature of the device driver.
 
 Cc: Mauro Carvalho Chehab mche...@osg.samsung.com
 Cc: Andy Lutomirski l...@amacapital.net
 Cc: Andy Walls awa...@md.metrocast.net
 Cc: Suresh Siddha sbsid...@gmail.com
 Cc: Ingo Molnar mi...@elte.hu
 Cc: Thomas Gleixner t...@linutronix.de
 Cc: Juergen Gross jgr...@suse.com
 Cc: Daniel Vetter daniel.vet...@ffwll.ch
 Cc: Dave Airlie airl...@redhat.com
 Cc: Bjorn Helgaas bhelg...@google.com
 Cc: Antonino Daplas adap...@gmail.com
 Cc: Jean-Christophe Plagniol-Villard plagn...@jcrosoft.com
 Cc: Tomi Valkeinen tomi.valkei...@ti.com
 Cc: Dave Hansen dave.han...@linux.intel.com
 Cc: Arnd Bergmann a...@arndb.de
 Cc: Michael S. Tsirkin m...@redhat.com
 Cc: Stefan Bader stefan.ba...@canonical.com
 Cc: Ville Syrjälä syrj...@sci.fi
 Cc: Mel Gorman mgor...@suse.de
 Cc: Vlastimil Babka vba...@suse.cz
 Cc: Borislav Petkov b...@suse.de
 Cc: Davidlohr Bueso dbu...@suse.de
 Cc: konrad.w...@oracle.com
 Cc: ville.syrj...@linux.intel.com
 Cc: david.vra...@citrix.com
 Cc: jbeul...@suse.com
 Cc: toshi.k...@hp.com
 Cc: Roger Pau Monné roger@citrix.com
 Cc: linux-fb...@vger.kernel.org
 Cc: linux-ker...@vger.kernel.org
 Cc: ivtv-de...@ivtvdriver.org
 Cc: linux-media@vger.kernel.org
 Cc: xen-de...@lists.xensource.com
 Signed-off-by: Luis R. Rodriguez mcg...@suse.com
 ---
  drivers/media/pci/ivtv/Kconfig  |  3 +++
  drivers/media/pci/ivtv/ivtvfb.c | 59 
 +
  2 files changed, 27 insertions(+), 35 deletions(-)
 
 diff --git a/drivers/media/pci/ivtv/Kconfig b/drivers/media/pci/ivtv/Kconfig
 index dd6ee57e..b2a7f88 100644
 --- a/drivers/media/pci/ivtv/Kconfig
 +++ b/drivers/media/pci/ivtv/Kconfig
 @@ -57,5 +57,8 @@ config VIDEO_FB_IVTV
 This is used in the Hauppauge PVR-350 card. There is a driver
 homepage at http://www.ivtvdriver.org.
  
 +   If you have this hardware you will need to boot with PAT disabled
 +   on your x86 systems, use the nopat kernel parameter.
 +
 To compile this driver as a module, choose M here: the
 module will be called ivtvfb.
 diff --git a/drivers/media/pci/ivtv/ivtvfb.c b/drivers/media/pci/ivtv/ivtvfb.c
 index 9ff1230..552408b 100644
 --- a/drivers/media/pci/ivtv/ivtvfb.c
 +++ b/drivers/media/pci/ivtv/ivtvfb.c
 @@ -44,8 +44,8 @@
  #include linux/ivtvfb.h
  #include linux/slab.h
  
 -#ifdef CONFIG_MTRR
 -#include asm/mtrr.h
 +#ifdef CONFIG_X86_64
 +#include asm/pat.h
  #endif
  
  #include ivtv-driver.h
 @@ -155,12 +155,11 @@ struct osd_info {
   /* Buffer size */
   u32 video_buffer_size;
  
 -#ifdef CONFIG_MTRR
   /* video_base rounded down as required by hardware MTRRs */
   unsigned long fb_start_aligned_physaddr;
   /* video_base rounded up as required by hardware MTRRs */
   unsigned long fb_end_aligned_physaddr;
 -#endif
 + int wc_cookie;
  
   /* Store the buffer offset */
   int set_osd_coords_x;
 @@ -1099,6 +1098,8 @@ static int ivtvfb_init_vidmode(struct ivtv *itv)
  static int ivtvfb_init_io(struct ivtv *itv)
  {
   struct osd_info *oi = itv-osd_info;
 + /* Find the largest power of two that maps the whole buffer */
 + int size_shift = 31;
  
   mutex_lock(itv-serialize_lock);
   if (ivtv_init_on_first_open(itv)) {
 @@ -1120,6 +1121,7 @@ static int ivtvfb_init_io(struct ivtv *itv)
   oi-video_buffer_size = 1704960;
  
   oi-video_pbase = itv-base_addr + IVTV_DECODER_OFFSET + 
 oi-video_rbase;
 + /* XXX: split this for PAT */

Please remove this comment.  It is prescriptive of a particular
solution, and probably not the one I'm going to implement.  The ivtv
main driver alreay splits the encoder, decoder, and register regions
into 3 mappings.  The final solution will set the whole 8 MB decoder
region mapping to WC, and then fix up all calls in the ivtvfb and ivtv
drivers where writes to the decoder memory with WC enabled could be a
problem.

Also many other places in the driver need audit in a conversion to work
with PAT, so no need to call out this one location with a 

[PATCH] [media] ivtv: use arch_phys_wc_add() and require PAT disabled

[Luis R. Rodriguez]

From: Luis R. Rodriguez mcg...@suse.com

We are burrying direct access to MTRR code support on
x86 in order to take advantage of PAT. In the future we
also want to make the default behaviour of ioremap_nocache()
to use strong UC, use of mtrr_add() on those systems
would make write-combining void.

In order to help both enable us to later make strong
UC default and in order to phase out direct MTRR access
code port the driver over to arch_phys_wc_add() and
annotate that the device driver requires systems to
boot with PAT disabled, with the nopat kernel parameter.

This is a worthy comprmise given that the hardware is
really rare these days, and perhaps only some lost souls
in some third world country are expected to be using this
feature of the device driver.

Cc: Mauro Carvalho Chehab mche...@osg.samsung.com
Cc: Andy Lutomirski l...@amacapital.net
Cc: Andy Walls awa...@md.metrocast.net
Cc: Suresh Siddha sbsid...@gmail.com
Cc: Ingo Molnar mi...@elte.hu
Cc: Thomas Gleixner t...@linutronix.de
Cc: Juergen Gross jgr...@suse.com
Cc: Daniel Vetter daniel.vet...@ffwll.ch
Cc: Dave Airlie airl...@redhat.com
Cc: Bjorn Helgaas bhelg...@google.com
Cc: Antonino Daplas adap...@gmail.com
Cc: Jean-Christophe Plagniol-Villard plagn...@jcrosoft.com
Cc: Tomi Valkeinen tomi.valkei...@ti.com
Cc: Dave Hansen dave.han...@linux.intel.com
Cc: Arnd Bergmann a...@arndb.de
Cc: Michael S. Tsirkin m...@redhat.com
Cc: Stefan Bader stefan.ba...@canonical.com
Cc: Ville Syrjälä syrj...@sci.fi
Cc: Mel Gorman mgor...@suse.de
Cc: Vlastimil Babka vba...@suse.cz
Cc: Borislav Petkov b...@suse.de
Cc: Davidlohr Bueso dbu...@suse.de
Cc: konrad.w...@oracle.com
Cc: ville.syrj...@linux.intel.com
Cc: david.vra...@citrix.com
Cc: jbeul...@suse.com
Cc: toshi.k...@hp.com
Cc: Roger Pau Monné roger@citrix.com
Cc: linux-fb...@vger.kernel.org
Cc: linux-ker...@vger.kernel.org
Cc: ivtv-de...@ivtvdriver.org
Cc: linux-media@vger.kernel.org
Cc: xen-de...@lists.xensource.com
Signed-off-by: Luis R. Rodriguez mcg...@suse.com
---
 drivers/media/pci/ivtv/Kconfig  |  3 +++
 drivers/media/pci/ivtv/ivtvfb.c | 59 +
 2 files changed, 27 insertions(+), 35 deletions(-)

diff --git a/drivers/media/pci/ivtv/Kconfig b/drivers/media/pci/ivtv/Kconfig
index dd6ee57e..b2a7f88 100644
--- a/drivers/media/pci/ivtv/Kconfig
+++ b/drivers/media/pci/ivtv/Kconfig
@@ -57,5 +57,8 @@ config VIDEO_FB_IVTV
  This is used in the Hauppauge PVR-350 card. There is a driver
  homepage at http://www.ivtvdriver.org.
 
+ If you have this hardware you will need to boot with PAT disabled
+ on your x86 systems, use the nopat kernel parameter.
+
  To compile this driver as a module, choose M here: the
  module will be called ivtvfb.
diff --git a/drivers/media/pci/ivtv/ivtvfb.c b/drivers/media/pci/ivtv/ivtvfb.c
index 9ff1230..552408b 100644
--- a/drivers/media/pci/ivtv/ivtvfb.c
+++ b/drivers/media/pci/ivtv/ivtvfb.c
@@ -44,8 +44,8 @@
 #include linux/ivtvfb.h
 #include linux/slab.h
 
-#ifdef CONFIG_MTRR
-#include asm/mtrr.h
+#ifdef CONFIG_X86_64
+#include asm/pat.h
 #endif
 
 #include ivtv-driver.h
@@ -155,12 +155,11 @@ struct osd_info {
/* Buffer size */
u32 video_buffer_size;
 
-#ifdef CONFIG_MTRR
/* video_base rounded down as required by hardware MTRRs */
unsigned long fb_start_aligned_physaddr;
/* video_base rounded up as required by hardware MTRRs */
unsigned long fb_end_aligned_physaddr;
-#endif
+   int wc_cookie;
 
/* Store the buffer offset */
int set_osd_coords_x;
@@ -1099,6 +1098,8 @@ static int ivtvfb_init_vidmode(struct ivtv *itv)
 static int ivtvfb_init_io(struct ivtv *itv)
 {
struct osd_info *oi = itv-osd_info;
+   /* Find the largest power of two that maps the whole buffer */
+   int size_shift = 31;
 
mutex_lock(itv-serialize_lock);
if (ivtv_init_on_first_open(itv)) {
@@ -1120,6 +1121,7 @@ static int ivtvfb_init_io(struct ivtv *itv)
oi-video_buffer_size = 1704960;
 
oi-video_pbase = itv-base_addr + IVTV_DECODER_OFFSET + 
oi-video_rbase;
+   /* XXX: split this for PAT */
oi-video_vbase = itv-dec_mem + oi-video_rbase;
 
if (!oi-video_vbase) {
@@ -1132,29 +1134,16 @@ static int ivtvfb_init_io(struct ivtv *itv)
oi-video_pbase, oi-video_vbase,
oi-video_buffer_size / 1024);
 
-#ifdef CONFIG_MTRR
-   {
-   /* Find the largest power of two that maps the whole buffer */
-   int size_shift = 31;
-
-   while (!(oi-video_buffer_size  (1  size_shift))) {
-   size_shift--;
-   }
-   size_shift++;
-   oi-fb_start_aligned_physaddr = oi-video_pbase  ~((1  
size_shift) - 1);
-   oi-fb_end_aligned_physaddr = oi-video_pbase + 
oi-video_buffer_size;
-   oi-fb_end_aligned_physaddr += (1  size_shift) - 1;
-