All of lore.kernel.org
 help / color / mirror / Atom feed
From: Andy Lutomirski <luto@amacapital.net>
To: Nick French <naf@ou.edu>
Cc: "Luis R. Rodriguez" <mcgrof@kernel.org>,
	Andy Lutomirski <luto@kernel.org>,
	"hans.verkuil@cisco.com" <hans.verkuil@cisco.com>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"linux-media@vger.kernel.org" <linux-media@vger.kernel.org>
Subject: Re: ivtv: use arch_phys_wc_add() and require PAT disabled
Date: Sun, 11 Mar 2018 13:19:03 -0700	[thread overview]
Message-ID: <38CB7D59-7F11-4BC3-B73C-C2F0BF16EFF8@amacapital.net> (raw)
In-Reply-To: <20180311195116.GB4645@tivo.lan>




> On Mar 11, 2018, at 12:51 PM, Nick French <naf@ou.edu> 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. 

WARNING: multiple messages have this Message-ID (diff)
From: Andy Lutomirski <luto@amacapital.net>
To: Nick French <naf@ou.edu>
Cc: "Luis R. Rodriguez" <mcgrof@kernel.org>,
	Andy Lutomirski <luto@kernel.org>,
	"hans.verkuil@cisco.com" <hans.verkuil@cisco.com>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"linux-media@vger.kernel.org" <linux-media@vger.kernel.org>
Subject: Re: ivtv: use arch_phys_wc_add() and require PAT disabled
Date: Sun, 11 Mar 2018 13:19:03 -0700	[thread overview]
Message-ID: <38CB7D59-7F11-4BC3-B73C-C2F0BF16EFF8@amacapital.net> (raw)
In-Reply-To: <20180311195116.GB4645@tivo.lan>




> On Mar 11, 2018, at 12:51 PM, Nick French <naf@ou.edu> 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. 

  reply	other threads:[~2018-03-11 20:19 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <DM5PR03MB3035EE1AFCEE298AFB15AC46D3C60@DM5PR03MB3035.namprd03.prod.outlook.com>
     [not found] ` <20180301171936.GU14069@wotan.suse.de>
     [not found]   ` <DM5PR03MB303587F12D7E56B951730A76D3D90@DM5PR03MB3035.namprd03.prod.outlook.com>
2018-03-07 19:02     ` ivtv: use arch_phys_wc_add() and require PAT disabled Luis R. Rodriguez
2018-03-08  3:16       ` French, Nicholas A.
2018-03-08  4:06         ` Luis R. Rodriguez
2018-03-08  4:14           ` Luis R. Rodriguez
2018-03-08  5:23             ` French, Nicholas A.
2018-03-10 16:57               ` French, Nicholas A.
2018-03-10 18:20                 ` Andy Lutomirski
2018-03-11 19:51                   ` Nick French
2018-03-11 20:19                     ` Andy Lutomirski [this message]
2018-03-11 20:19                       ` Andy Lutomirski
2018-03-11 22:08                       ` Nick French
2018-03-10 19:03                 ` Luis R. Rodriguez
2018-03-10 19:05                   ` Luis R. Rodriguez
2018-03-11 23:24                   ` Ian Armstrong
2018-03-12  4:04                     ` Nick French
2018-03-12 19:05                       ` Ian Armstrong

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=38CB7D59-7F11-4BC3-B73C-C2F0BF16EFF8@amacapital.net \
    --to=luto@amacapital.net \
    --cc=hans.verkuil@cisco.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-media@vger.kernel.org \
    --cc=luto@kernel.org \
    --cc=mcgrof@kernel.org \
    --cc=naf@ou.edu \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.