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 +0000, Luis R. Rodriguez wrote:
> > On Thu, Mar 08, 2018 at 04:06:01AM +0000, Luis R. Rodriguez wrote:
> > > On Thu, Mar 08, 2018 at 03:16:29AM +0000, 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 0xd5000000-0xd5800000
x86/PAT: reserve_memtype added [mem 0xd5510000-0xd56b0fff], track uncached-minus, req write-combining, ret uncached-minus
ivtvfb0: Framebuffer at 0xd5510000, mapped to 0x00000000c6a7ed52, size 1665k
...
$ sudo cat /sys/kernel/debug/x86/pat_memtype_list | grep 0xd5
uncached-minus @ 0xd5000000-0xd5800000
uncached-minus @ 0xd5510000-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