[RESEND PATCH v2 2/2] x86/mm/pat, drivers/media/ivtv: move pat warn and replace WARN() with pr_warn()
From: Luis R. Rodriguez mcg...@suse.com
On built-in kernels this warning will always splat as this is part
of the module init. Fix that by shifting the PAT requirement check
out under the code that does the quasi-probe for the device. This
device driver relies on an existing driver to find its own devices,
it looks for that device driver and its own found devices, then
uses driver_for_each_device() to try to see if it can probe each of
those devices as a frambuffer device with ivtvfb_init_card(). We
tuck the PAT requiremenet check then on the ivtvfb_init_card()
call making the check at least require an ivtv device present
before complaining.
Reported-by: Fengguang Wu fengguang...@intel.com [0-day test robot]
Signed-off-by: Luis R. Rodriguez mcg...@suse.com
---
drivers/media/pci/ivtv/ivtvfb.c | 15 +--
1 file changed, 9 insertions(+), 6 deletions(-)
diff --git a/drivers/media/pci/ivtv/ivtvfb.c b/drivers/media/pci/ivtv/ivtvfb.c
index 4cb365d4ffdc..8b95eefb610b 100644
--- a/drivers/media/pci/ivtv/ivtvfb.c
+++ b/drivers/media/pci/ivtv/ivtvfb.c
@@ -38,6 +38,8 @@
Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
*/
+#define pr_fmt(fmt) KBUILD_MODNAME : fmt
+
#include linux/module.h
#include linux/kernel.h
#include linux/fb.h
@@ -1171,6 +1173,13 @@ 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;
@@ -1265,12 +1274,6 @@ static int __init ivtvfb_init(void)
int registered = 0;
int err;
-#ifdef CONFIG_X86_64
- if (WARN(pat_enabled(),
-ivtvfb needs PAT disabled, boot with nopat kernel
parameter\n)) {
- return -ENODEV;
- }
-#endif
if (ivtvfb_card_id -1 || ivtvfb_card_id = IVTV_MAX_CARDS) {
printk(KERN_ERR ivtvfb: ivtvfb_card_id parameter is out of
range (valid range: -1 - %d)\n,
--
2.3.2.209.gd67f9d5.dirty
--
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 2/2] x86/mm/pat, drivers/media/ivtv: move pat warn and replace WARN() with pr_warn()
* Luis R. Rodriguez mcg...@do-not-panic.com wrote: On Mon, Jul 6, 2015 at 5:44 PM, Luis R. Rodriguez mcg...@suse.com wrote: If we really wanted to we could consider arch_phys_wc_add() I mean adding a __arch_phys_wc_add() which does not check for pat_enabled(). and deal with that this will not check for pat_enabled() and forces MTRR... I think Andy Luto won't like that very much though ? I at least don't like it since we did all this work to finally leave only 1 piece of code with direct MTRR access... Seems a bit sad. Since ipath will be removed we'd have only ivtv driver using this API, I am not sure if its worth it. Since ipath is going away soon we'd just have one driver with the icky #ifdef code. I'd rather see that and then require semantics / grammer rules to require ioremap_wc() when used with arch_phys_wc_add(). I don't think ivtv is worth to consider breaking the semantics and requirements. Ok, let's keep your original approach with the warning then. Thanks, Ingo -- 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 2/2] x86/mm/pat, drivers/media/ivtv: move pat warn and replace WARN() with pr_warn()
On Mon, Jul 6, 2015 at 5:44 PM, Luis R. Rodriguez mcg...@suse.com wrote: If we really wanted to we could consider arch_phys_wc_add() I mean adding a __arch_phys_wc_add() which does not check for pat_enabled(). and deal with that this will not check for pat_enabled() and forces MTRR... I think Andy Luto won't like that very much though ? I at least don't like it since we did all this work to finally leave only 1 piece of code with direct MTRR access... Seems a bit sad. Since ipath will be removed we'd have only ivtv driver using this API, I am not sure if its worth it. Since ipath is going away soon we'd just have one driver with the icky #ifdef code. I'd rather see that and then require semantics / grammer rules to require ioremap_wc() when used with arch_phys_wc_add(). I don't think ivtv is worth to consider breaking the semantics and requirements. Thoughts? 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 2/2] x86/mm/pat, drivers/media/ivtv: move pat warn and replace WARN() with pr_warn()
On Mon, Jun 29, 2015 at 05:52:18AM -0400, Andy Walls wrote:
On June 29, 2015 2:55:05 AM EDT, Ingo Molnar mi...@kernel.org wrote:
* Andy Walls a...@silverblocksystems.net wrote:
On Fri, 2015-06-26 at 10:45 +0200, Ingo Molnar wrote:
* Luis R. Rodriguez mcg...@suse.com wrote:
On Thu, Jun 25, 2015 at 08:51:47AM +0200, Ingo Molnar wrote:
* Luis R. Rodriguez mcg...@do-not-panic.com wrote:
From: Luis R. Rodriguez mcg...@suse.com
On built-in kernels this warning will always splat as this is
part
of the module init. Fix that by shifting the PAT requirement
check
out under the code that does the quasi-probe for the
device. This
device driver relies on an existing driver to find its own
devices,
it looks for that device driver and its own found devices,
then
uses driver_for_each_device() to try to see if it can probe
each of
those devices as a frambuffer device with ivtvfb_init_card().
We
tuck the PAT requiremenet check then on the
ivtvfb_init_card()
call making the check at least require an ivtv device present
before complaining.
Reported-by: Fengguang Wu fengguang...@intel.com [0-day
test robot]
Signed-off-by: Luis R. Rodriguez mcg...@suse.com
---
drivers/media/pci/ivtv/ivtvfb.c | 15 +--
1 file changed, 9 insertions(+), 6 deletions(-)
diff --git a/drivers/media/pci/ivtv/ivtvfb.c
b/drivers/media/pci/ivtv/ivtvfb.c
index 4cb365d..8b95eef 100644
--- a/drivers/media/pci/ivtv/ivtvfb.c
+++ b/drivers/media/pci/ivtv/ivtvfb.c
@@ -38,6 +38,8 @@
Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA
02111-1307 USA
*/
+#define pr_fmt(fmt) KBUILD_MODNAME : fmt
+
#include linux/module.h
#include linux/kernel.h
#include linux/fb.h
@@ -1171,6 +1173,13 @@ 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;
Same argument as for ipath: why not make arch_phys_wc_add()
fail on PAT and
return -1, and check it in arch_phys_wc_del()?
The arch_phys_wc_add() is a no-op for PAT systems but for PAT to
work we need
not only need to add this in where we replace the MTRR call but
we also need to
convert ioremap_nocache() calls to ioremap_wc() but only if
things were split up
already.
Hi Ingo,
We don't need to do that: for such legacy drivers we can fall back
to UC just
fine, and inform the user that by booting with 'nopat' the old
behavior will be
back...
This is really a user experience decision.
IMO anyone who is still using ivtvfb and an old conventional PCI
PVR-350 to
render, at SDTV resolution, an X Desktop display or video playback on
a
television screen, isn't going to give a hoot about modern things
like PAT. The
user will simply want the framebuffer performance they are accustomed
to having
with their system. UC will probably yield unsatisfactory performance
for an
ivtvfb framebuffer.
With that in mind, I would think it better to obviously and clearly
disable the
ivtvfb framebuffer module with PAT enabled, so the user will check
the log and
read the steps needed to obtain acceptable performance.
Maybe that's me just wanting to head off the poor ivtvfb performance
with
latest kernel bug reports.
Whatever the decision, my stock response to bug reports related to
this will
always be What do the logs say?.
So what if that frame buffer is their only (working) frame buffer? A
slow
framebuffer is still much better at giving people logs to look at than
a
non-working one.
Thanks,
Ingo
Hi Ingo,
For an old DVR setup, I can see that being the case.
So a sluggish framebuffer is better than none.
OK, so I knew I had considered this strategy before, I did and here's
the recap:
I originally had proposed this same exact thing with my first patch set but [0]
had set to require a new __arch_phys_wc_add() which forces the setting without
checking if pat_enabled() is true. This was set out as a transient API in hopes
that device drivers that require work but hadn't been converted to split the
ioremap'd calls would later be modified / fixed with the split. Here's an
update for the status of each driver:
Driver File
fusion drivers/message/fusion/mptbase.c
This code was commented out, the code was just removed, this longer applies.
ivtv
Re: [PATCH v2 2/2] x86/mm/pat, drivers/media/ivtv: move pat warn and replace WARN() with pr_warn()
* Andy Walls a...@silverblocksystems.net wrote:
On Fri, 2015-06-26 at 10:45 +0200, Ingo Molnar wrote:
* Luis R. Rodriguez mcg...@suse.com wrote:
On Thu, Jun 25, 2015 at 08:51:47AM +0200, Ingo Molnar wrote:
* Luis R. Rodriguez mcg...@do-not-panic.com wrote:
From: Luis R. Rodriguez mcg...@suse.com
On built-in kernels this warning will always splat as this is part
of the module init. Fix that by shifting the PAT requirement check
out under the code that does the quasi-probe for the device. This
device driver relies on an existing driver to find its own devices,
it looks for that device driver and its own found devices, then
uses driver_for_each_device() to try to see if it can probe each of
those devices as a frambuffer device with ivtvfb_init_card(). We
tuck the PAT requiremenet check then on the ivtvfb_init_card()
call making the check at least require an ivtv device present
before complaining.
Reported-by: Fengguang Wu fengguang...@intel.com [0-day test robot]
Signed-off-by: Luis R. Rodriguez mcg...@suse.com
---
drivers/media/pci/ivtv/ivtvfb.c | 15 +--
1 file changed, 9 insertions(+), 6 deletions(-)
diff --git a/drivers/media/pci/ivtv/ivtvfb.c
b/drivers/media/pci/ivtv/ivtvfb.c
index 4cb365d..8b95eef 100644
--- a/drivers/media/pci/ivtv/ivtvfb.c
+++ b/drivers/media/pci/ivtv/ivtvfb.c
@@ -38,6 +38,8 @@
Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA
02111-1307 USA
*/
+#define pr_fmt(fmt) KBUILD_MODNAME : fmt
+
#include linux/module.h
#include linux/kernel.h
#include linux/fb.h
@@ -1171,6 +1173,13 @@ 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;
Same argument as for ipath: why not make arch_phys_wc_add() fail on PAT
and
return -1, and check it in arch_phys_wc_del()?
The arch_phys_wc_add() is a no-op for PAT systems but for PAT to work we
need
not only need to add this in where we replace the MTRR call but we also
need to
convert ioremap_nocache() calls to ioremap_wc() but only if things were
split up
already.
Hi Ingo,
We don't need to do that: for such legacy drivers we can fall back to UC
just
fine, and inform the user that by booting with 'nopat' the old behavior
will be
back...
This is really a user experience decision.
IMO anyone who is still using ivtvfb and an old conventional PCI PVR-350 to
render, at SDTV resolution, an X Desktop display or video playback on a
television screen, isn't going to give a hoot about modern things like PAT.
The
user will simply want the framebuffer performance they are accustomed to
having
with their system. UC will probably yield unsatisfactory performance for an
ivtvfb framebuffer.
With that in mind, I would think it better to obviously and clearly disable
the
ivtvfb framebuffer module with PAT enabled, so the user will check the log
and
read the steps needed to obtain acceptable performance.
Maybe that's me just wanting to head off the poor ivtvfb performance with
latest kernel bug reports.
Whatever the decision, my stock response to bug reports related to this will
always be What do the logs say?.
So what if that frame buffer is their only (working) frame buffer? A slow
framebuffer is still much better at giving people logs to look at than a
non-working one.
Thanks,
Ingo
--
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 2/2] x86/mm/pat, drivers/media/ivtv: move pat warn and replace WARN() with pr_warn()
* Luis R. Rodriguez mcg...@suse.com wrote:
On Thu, Jun 25, 2015 at 08:51:47AM +0200, Ingo Molnar wrote:
* Luis R. Rodriguez mcg...@do-not-panic.com wrote:
From: Luis R. Rodriguez mcg...@suse.com
On built-in kernels this warning will always splat as this is part
of the module init. Fix that by shifting the PAT requirement check
out under the code that does the quasi-probe for the device. This
device driver relies on an existing driver to find its own devices,
it looks for that device driver and its own found devices, then
uses driver_for_each_device() to try to see if it can probe each of
those devices as a frambuffer device with ivtvfb_init_card(). We
tuck the PAT requiremenet check then on the ivtvfb_init_card()
call making the check at least require an ivtv device present
before complaining.
Reported-by: Fengguang Wu fengguang...@intel.com [0-day test robot]
Signed-off-by: Luis R. Rodriguez mcg...@suse.com
---
drivers/media/pci/ivtv/ivtvfb.c | 15 +--
1 file changed, 9 insertions(+), 6 deletions(-)
diff --git a/drivers/media/pci/ivtv/ivtvfb.c
b/drivers/media/pci/ivtv/ivtvfb.c
index 4cb365d..8b95eef 100644
--- a/drivers/media/pci/ivtv/ivtvfb.c
+++ b/drivers/media/pci/ivtv/ivtvfb.c
@@ -38,6 +38,8 @@
Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307
USA
*/
+#define pr_fmt(fmt) KBUILD_MODNAME : fmt
+
#include linux/module.h
#include linux/kernel.h
#include linux/fb.h
@@ -1171,6 +1173,13 @@ 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;
Same argument as for ipath: why not make arch_phys_wc_add() fail on PAT and
return -1, and check it in arch_phys_wc_del()?
The arch_phys_wc_add() is a no-op for PAT systems but for PAT to work we need
not only need to add this in where we replace the MTRR call but we also need
to
convert ioremap_nocache() calls to ioremap_wc() but only if things were split
up
already.
We don't need to do that: for such legacy drivers we can fall back to UC just
fine, and inform the user that by booting with 'nopat' the old behavior will be
back...
Thanks,
Ingo
--
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 2/2] x86/mm/pat, drivers/media/ivtv: move pat warn and replace WARN() with pr_warn()
On Fri, 2015-06-26 at 10:45 +0200, Ingo Molnar wrote:
* Luis R. Rodriguez mcg...@suse.com wrote:
On Thu, Jun 25, 2015 at 08:51:47AM +0200, Ingo Molnar wrote:
* Luis R. Rodriguez mcg...@do-not-panic.com wrote:
From: Luis R. Rodriguez mcg...@suse.com
On built-in kernels this warning will always splat as this is part
of the module init. Fix that by shifting the PAT requirement check
out under the code that does the quasi-probe for the device. This
device driver relies on an existing driver to find its own devices,
it looks for that device driver and its own found devices, then
uses driver_for_each_device() to try to see if it can probe each of
those devices as a frambuffer device with ivtvfb_init_card(). We
tuck the PAT requiremenet check then on the ivtvfb_init_card()
call making the check at least require an ivtv device present
before complaining.
Reported-by: Fengguang Wu fengguang...@intel.com [0-day test robot]
Signed-off-by: Luis R. Rodriguez mcg...@suse.com
---
drivers/media/pci/ivtv/ivtvfb.c | 15 +--
1 file changed, 9 insertions(+), 6 deletions(-)
diff --git a/drivers/media/pci/ivtv/ivtvfb.c
b/drivers/media/pci/ivtv/ivtvfb.c
index 4cb365d..8b95eef 100644
--- a/drivers/media/pci/ivtv/ivtvfb.c
+++ b/drivers/media/pci/ivtv/ivtvfb.c
@@ -38,6 +38,8 @@
Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA
02111-1307 USA
*/
+#define pr_fmt(fmt) KBUILD_MODNAME : fmt
+
#include linux/module.h
#include linux/kernel.h
#include linux/fb.h
@@ -1171,6 +1173,13 @@ 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;
Same argument as for ipath: why not make arch_phys_wc_add() fail on PAT
and
return -1, and check it in arch_phys_wc_del()?
The arch_phys_wc_add() is a no-op for PAT systems but for PAT to work we
need
not only need to add this in where we replace the MTRR call but we also
need to
convert ioremap_nocache() calls to ioremap_wc() but only if things were
split up
already.
Hi Ingo,
We don't need to do that: for such legacy drivers we can fall back to UC just
fine, and inform the user that by booting with 'nopat' the old behavior will
be
back...
This is really a user experience decision.
IMO anyone who is still using ivtvfb and an old conventional PCI PVR-350
to render, at SDTV resolution, an X Desktop display or video playback on
a television screen, isn't going to give a hoot about modern things like
PAT. The user will simply want the framebuffer performance they are
accustomed to having with their system. UC will probably yield
unsatisfactory performance for an ivtvfb framebuffer.
With that in mind, I would think it better to obviously and clearly
disable the ivtvfb framebuffer module with PAT enabled, so the user will
check the log and read the steps needed to obtain acceptable
performance.
Maybe that's me just wanting to head off the poor ivtvfb performance
with latest kernel bug reports.
Whatever the decision, my stock response to bug reports related to this
will always be What do the logs say?.
Regards,
Andy
--
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 2/2] x86/mm/pat, drivers/media/ivtv: move pat warn and replace WARN() with pr_warn()
* Luis R. Rodriguez mcg...@do-not-panic.com wrote:
From: Luis R. Rodriguez mcg...@suse.com
On built-in kernels this warning will always splat as this is part
of the module init. Fix that by shifting the PAT requirement check
out under the code that does the quasi-probe for the device. This
device driver relies on an existing driver to find its own devices,
it looks for that device driver and its own found devices, then
uses driver_for_each_device() to try to see if it can probe each of
those devices as a frambuffer device with ivtvfb_init_card(). We
tuck the PAT requiremenet check then on the ivtvfb_init_card()
call making the check at least require an ivtv device present
before complaining.
Reported-by: Fengguang Wu fengguang...@intel.com [0-day test robot]
Signed-off-by: Luis R. Rodriguez mcg...@suse.com
---
drivers/media/pci/ivtv/ivtvfb.c | 15 +--
1 file changed, 9 insertions(+), 6 deletions(-)
diff --git a/drivers/media/pci/ivtv/ivtvfb.c b/drivers/media/pci/ivtv/ivtvfb.c
index 4cb365d..8b95eef 100644
--- a/drivers/media/pci/ivtv/ivtvfb.c
+++ b/drivers/media/pci/ivtv/ivtvfb.c
@@ -38,6 +38,8 @@
Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
*/
+#define pr_fmt(fmt) KBUILD_MODNAME : fmt
+
#include linux/module.h
#include linux/kernel.h
#include linux/fb.h
@@ -1171,6 +1173,13 @@ 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;
Same argument as for ipath: why not make arch_phys_wc_add() fail on PAT and
return
-1, and check it in arch_phys_wc_del()?
That way we don't do anything drastic, the remaining few drivers still keep
working (albeit suboptimally - can be worked around with the 'nopat' boot
option)
- yet we've reduced the use of MTRRs drastically.
Thanks,
Ingo
--
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 2/2] x86/mm/pat, drivers/media/ivtv: move pat warn and replace WARN() with pr_warn()
On Thu, Jun 25, 2015 at 08:51:47AM +0200, Ingo Molnar wrote:
* Luis R. Rodriguez mcg...@do-not-panic.com wrote:
From: Luis R. Rodriguez mcg...@suse.com
On built-in kernels this warning will always splat as this is part
of the module init. Fix that by shifting the PAT requirement check
out under the code that does the quasi-probe for the device. This
device driver relies on an existing driver to find its own devices,
it looks for that device driver and its own found devices, then
uses driver_for_each_device() to try to see if it can probe each of
those devices as a frambuffer device with ivtvfb_init_card(). We
tuck the PAT requiremenet check then on the ivtvfb_init_card()
call making the check at least require an ivtv device present
before complaining.
Reported-by: Fengguang Wu fengguang...@intel.com [0-day test robot]
Signed-off-by: Luis R. Rodriguez mcg...@suse.com
---
drivers/media/pci/ivtv/ivtvfb.c | 15 +--
1 file changed, 9 insertions(+), 6 deletions(-)
diff --git a/drivers/media/pci/ivtv/ivtvfb.c
b/drivers/media/pci/ivtv/ivtvfb.c
index 4cb365d..8b95eef 100644
--- a/drivers/media/pci/ivtv/ivtvfb.c
+++ b/drivers/media/pci/ivtv/ivtvfb.c
@@ -38,6 +38,8 @@
Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307
USA
*/
+#define pr_fmt(fmt) KBUILD_MODNAME : fmt
+
#include linux/module.h
#include linux/kernel.h
#include linux/fb.h
@@ -1171,6 +1173,13 @@ 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;
Same argument as for ipath: why not make arch_phys_wc_add() fail on PAT and
return
-1, and check it in arch_phys_wc_del()?
The arch_phys_wc_add() is a no-op for PAT systems but for PAT to work we need
not only need to add this in where we replace the MTRR call but we also need
to convert ioremap_nocache() calls to ioremap_wc() but only if things were
split up already.
We racked our heads [0] [1] trying to figure out how to do the split for ivtv.
The
issues with ivtv were that the firmware decides where the WC area is and does
not provide APIs to expose it. Then alternatives are to for example just use WC
on the entire full range and use work arounds write(); wmb(); read(); for MMIO
registers. That idea came from the use case of the Myricom Ethernet device
driver which uses WC as a compromise to address a performance regression if
it didn't use WC on an entire range, it uses the work around for the MMIO
registers. I considered very *briefly* adding a generic API that would let
device driver use this but dropped the idea as it seems this was not a common
issue and this was rather a work around.
I should note that Benjamin recenlty noted that power pc (and he says possibly
more) writel() and co contains an implicit mb(). That addresses some of it may
maybe not all requirements.
[0] http://lkml.kernel.org/r/1429146457.1899.99.ca...@palomino.walls.org
[1] https://marc.info/?t=14289474115r=1w=2
That way we don't do anything drastic, the remaining few drivers still keep
working (albeit suboptimally - can be worked around with the 'nopat' boot
option)
- yet we've reduced the use of MTRRs drastically.
It seems the 3 drivers that needed hackery are ancient, not common and likely
adding a general fix more work than the gains provided through it. We'd need
to address not only the use of the arch_phys calls but also to split their MMIO
registers / WC desire area. This later part was the harder part of all this.
Fortunately the norm is that modern devices have a full PCI bar designated
for each now. Furthermore in the future we should hope for buses that do the
negotiation of this for us and we can just map things out for them in the
kernel. benh seems to note ppc does some hackery for this but I wouldn't bet
on it being viable without issues on x86 just unless a thorough review / big
wagers are made.
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
[PATCH v2 2/2] x86/mm/pat, drivers/media/ivtv: move pat warn and replace WARN() with pr_warn()
From: Luis R. Rodriguez mcg...@suse.com
On built-in kernels this warning will always splat as this is part
of the module init. Fix that by shifting the PAT requirement check
out under the code that does the quasi-probe for the device. This
device driver relies on an existing driver to find its own devices,
it looks for that device driver and its own found devices, then
uses driver_for_each_device() to try to see if it can probe each of
those devices as a frambuffer device with ivtvfb_init_card(). We
tuck the PAT requiremenet check then on the ivtvfb_init_card()
call making the check at least require an ivtv device present
before complaining.
Reported-by: Fengguang Wu fengguang...@intel.com [0-day test robot]
Signed-off-by: Luis R. Rodriguez mcg...@suse.com
---
drivers/media/pci/ivtv/ivtvfb.c | 15 +--
1 file changed, 9 insertions(+), 6 deletions(-)
diff --git a/drivers/media/pci/ivtv/ivtvfb.c b/drivers/media/pci/ivtv/ivtvfb.c
index 4cb365d..8b95eef 100644
--- a/drivers/media/pci/ivtv/ivtvfb.c
+++ b/drivers/media/pci/ivtv/ivtvfb.c
@@ -38,6 +38,8 @@
Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
*/
+#define pr_fmt(fmt) KBUILD_MODNAME : fmt
+
#include linux/module.h
#include linux/kernel.h
#include linux/fb.h
@@ -1171,6 +1173,13 @@ 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;
@@ -1265,12 +1274,6 @@ static int __init ivtvfb_init(void)
int registered = 0;
int err;
-#ifdef CONFIG_X86_64
- if (WARN(pat_enabled(),
-ivtvfb needs PAT disabled, boot with nopat kernel
parameter\n)) {
- return -ENODEV;
- }
-#endif
if (ivtvfb_card_id -1 || ivtvfb_card_id = IVTV_MAX_CARDS) {
printk(KERN_ERR ivtvfb: ivtvfb_card_id parameter is out of
range (valid range: -1 - %d)\n,
--
2.3.2.209.gd67f9d5.dirty
--
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
