* Re: [PATCH 3/3] powerpc: Set default VGA device
@ 2013-04-05 20:24 ` Bjorn Helgaas
0 siblings, 0 replies; 15+ messages in thread
From: Bjorn Helgaas @ 2013-04-05 20:24 UTC (permalink / raw)
To: Brian King
Cc: linux-pci@vger.kernel.org, linuxppc-dev, Benjamin Herrenschmidt,
Lucas Kannebley Tavares, klebers, Michael Ellerman, sparclinux
On Thu, Apr 4, 2013 at 3:58 PM, Brian King <brking@linux.vnet.ibm.com> wrote:
>
> Add a PCI quirk for VGA devices on Power to set the default VGA device.
> Ensures a default VGA is always set if a graphics adapter is present,
> even if firmware did not initialize it. If more than one graphics
> adapter is present, ensure the one initialized by firmware is set
> as the default VGA device. This ensures that X autoconfiguration
> will work.
>
> Signed-off-by: Brian King <brking@linux.vnet.ibm.com>
> ---
>
> arch/powerpc/kernel/pci-common.c | 13 +++++++++++++
> 1 file changed, 13 insertions(+)
>
> diff -puN arch/powerpc/kernel/pci-common.c~powerpc_vga_default_device arch/powerpc/kernel/pci-common.c
> --- linux/arch/powerpc/kernel/pci-common.c~powerpc_vga_default_device 2013-04-03 09:50:33.000000000 -0500
> +++ linux-bjking1/arch/powerpc/kernel/pci-common.c 2013-04-03 09:50:33.000000000 -0500
> @@ -30,6 +30,7 @@
> #include <linux/irq.h>
> #include <linux/vmalloc.h>
> #include <linux/slab.h>
> +#include <linux/vgaarb.h>
>
> #include <asm/processor.h>
> #include <asm/io.h>
> @@ -1725,3 +1726,15 @@ static void fixup_hide_host_resource_fsl
> }
> DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_MOTOROLA, PCI_ANY_ID, fixup_hide_host_resource_fsl);
> DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_FREESCALE, PCI_ANY_ID, fixup_hide_host_resource_fsl);
> +
> +static void fixup_vga(struct pci_dev *pdev)
> +{
> + u16 cmd;
> +
> + pci_read_config_word(pdev, PCI_COMMAND, &cmd);
> + if ((cmd & (PCI_COMMAND_IO | PCI_COMMAND_MEMORY)) || !vga_default_device())
> + vga_set_default_device(pdev);
> +
> +}
> +DECLARE_PCI_FIXUP_CLASS_FINAL(PCI_ANY_ID, PCI_ANY_ID,
> + PCI_CLASS_DISPLAY_VGA, 8, fixup_vga);
This not really an arch-specific issue, so it's a shame to add another
arch-specific quirk for it (in addition to the x86 and ia64 ones we
already have).
In b5e4efe7e0, Eiichiro Oiwa <eiichiro.oiwa.nm@hitachi.com> tried to
make this quirk generic, but the implementation was naive and it
didn't work for sparc64. There's a good thread about the sparc issue
at https://lkml.org/lkml/2006/10/19/17
The outcome was to basically revert back to arch-specific quirks with
6b5c76b8e2, but I think the generic version probably could have been
made to work, possibly with a pcibios hook or something for anything
that really is arch-dependent, like the shadowed ROM image.
This particular patch is arch-specific, and I'm not going to try to
nack it because I'm not the powerpc maintainer, but I will certainly
try to help you if you want to push on making a generic version.
Bjorn
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [PATCH 3/3] powerpc: Set default VGA device
@ 2013-04-05 20:24 ` Bjorn Helgaas
0 siblings, 0 replies; 15+ messages in thread
From: Bjorn Helgaas @ 2013-04-05 20:24 UTC (permalink / raw)
To: Brian King
Cc: linux-pci@vger.kernel.org, klebers, Lucas Kannebley Tavares,
sparclinux, linuxppc-dev
On Thu, Apr 4, 2013 at 3:58 PM, Brian King <brking@linux.vnet.ibm.com> wrote:
>
> Add a PCI quirk for VGA devices on Power to set the default VGA device.
> Ensures a default VGA is always set if a graphics adapter is present,
> even if firmware did not initialize it. If more than one graphics
> adapter is present, ensure the one initialized by firmware is set
> as the default VGA device. This ensures that X autoconfiguration
> will work.
>
> Signed-off-by: Brian King <brking@linux.vnet.ibm.com>
> ---
>
> arch/powerpc/kernel/pci-common.c | 13 +++++++++++++
> 1 file changed, 13 insertions(+)
>
> diff -puN arch/powerpc/kernel/pci-common.c~powerpc_vga_default_device arch/powerpc/kernel/pci-common.c
> --- linux/arch/powerpc/kernel/pci-common.c~powerpc_vga_default_device 2013-04-03 09:50:33.000000000 -0500
> +++ linux-bjking1/arch/powerpc/kernel/pci-common.c 2013-04-03 09:50:33.000000000 -0500
> @@ -30,6 +30,7 @@
> #include <linux/irq.h>
> #include <linux/vmalloc.h>
> #include <linux/slab.h>
> +#include <linux/vgaarb.h>
>
> #include <asm/processor.h>
> #include <asm/io.h>
> @@ -1725,3 +1726,15 @@ static void fixup_hide_host_resource_fsl
> }
> DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_MOTOROLA, PCI_ANY_ID, fixup_hide_host_resource_fsl);
> DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_FREESCALE, PCI_ANY_ID, fixup_hide_host_resource_fsl);
> +
> +static void fixup_vga(struct pci_dev *pdev)
> +{
> + u16 cmd;
> +
> + pci_read_config_word(pdev, PCI_COMMAND, &cmd);
> + if ((cmd & (PCI_COMMAND_IO | PCI_COMMAND_MEMORY)) || !vga_default_device())
> + vga_set_default_device(pdev);
> +
> +}
> +DECLARE_PCI_FIXUP_CLASS_FINAL(PCI_ANY_ID, PCI_ANY_ID,
> + PCI_CLASS_DISPLAY_VGA, 8, fixup_vga);
This not really an arch-specific issue, so it's a shame to add another
arch-specific quirk for it (in addition to the x86 and ia64 ones we
already have).
In b5e4efe7e0, Eiichiro Oiwa <eiichiro.oiwa.nm@hitachi.com> tried to
make this quirk generic, but the implementation was naive and it
didn't work for sparc64. There's a good thread about the sparc issue
at https://lkml.org/lkml/2006/10/19/17
The outcome was to basically revert back to arch-specific quirks with
6b5c76b8e2, but I think the generic version probably could have been
made to work, possibly with a pcibios hook or something for anything
that really is arch-dependent, like the shadowed ROM image.
This particular patch is arch-specific, and I'm not going to try to
nack it because I'm not the powerpc maintainer, but I will certainly
try to help you if you want to push on making a generic version.
Bjorn
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [PATCH 3/3] powerpc: Set default VGA device
2013-04-05 20:24 ` Bjorn Helgaas
(?)
@ 2013-04-08 5:21 ` Michael Ellerman
-1 siblings, 0 replies; 15+ messages in thread
From: Michael Ellerman @ 2013-04-08 5:21 UTC (permalink / raw)
To: Bjorn Helgaas
Cc: Brian King, linux-pci@vger.kernel.org, linuxppc-dev,
Benjamin Herrenschmidt, Lucas Kannebley Tavares, klebers,
sparclinux
On Fri, Apr 05, 2013 at 02:24:09PM -0600, Bjorn Helgaas wrote:
> On Thu, Apr 4, 2013 at 3:58 PM, Brian King <brking@linux.vnet.ibm.com> wrote:
> > @@ -1725,3 +1726,15 @@ static void fixup_hide_host_resource_fsl
> > }
> > DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_MOTOROLA, PCI_ANY_ID, fixup_hide_host_resource_fsl);
> > DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_FREESCALE, PCI_ANY_ID, fixup_hide_host_resource_fsl);
> > +
> > +static void fixup_vga(struct pci_dev *pdev)
> > +{
> > + u16 cmd;
> > +
> > + pci_read_config_word(pdev, PCI_COMMAND, &cmd);
> > + if ((cmd & (PCI_COMMAND_IO | PCI_COMMAND_MEMORY)) || !vga_default_device())
> > + vga_set_default_device(pdev);
> > +
> > +}
> > +DECLARE_PCI_FIXUP_CLASS_FINAL(PCI_ANY_ID, PCI_ANY_ID,
> > + PCI_CLASS_DISPLAY_VGA, 8, fixup_vga);
>
> This not really an arch-specific issue, so it's a shame to add another
> arch-specific quirk for it (in addition to the x86 and ia64 ones we
> already have).
>
> In b5e4efe7e0, Eiichiro Oiwa <eiichiro.oiwa.nm@hitachi.com> tried to
> make this quirk generic, but the implementation was naive and it
> didn't work for sparc64. There's a good thread about the sparc issue
> at https://lkml.org/lkml/2006/10/19/17
>
> The outcome was to basically revert back to arch-specific quirks with
> 6b5c76b8e2, but I think the generic version probably could have been
> made to work, possibly with a pcibios hook or something for anything
> that really is arch-dependent, like the shadowed ROM image.
>
> This particular patch is arch-specific, and I'm not going to try to
> nack it because I'm not the powerpc maintainer, but I will certainly
> try to help you if you want to push on making a generic version.
OK.
Looking at x86, ia64 and ours, there's a fair bit of difference.
x86/ia64 both walk up the parents checking PCI_BRIDGE_CTL_VGA, which
powerpc doesn't (though maybe it should?).
x86/ia64 set IORESOURCE_ROM_SHADOW, which powerpc doesn't.
ia64 doesn't call vga_set_default_device(), x86 and powerpc do.
So we'll merge this and maybe someone can tease out the common bits, but
personally I don't see that there's an obvious chunk of generic logic.
cheers
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [PATCH 3/3] powerpc: Set default VGA device
@ 2013-04-08 5:21 ` Michael Ellerman
0 siblings, 0 replies; 15+ messages in thread
From: Michael Ellerman @ 2013-04-08 5:21 UTC (permalink / raw)
To: Bjorn Helgaas
Cc: Brian King, linux-pci@vger.kernel.org, linuxppc-dev,
Benjamin Herrenschmidt, Lucas Kannebley Tavares, klebers,
sparclinux
On Fri, Apr 05, 2013 at 02:24:09PM -0600, Bjorn Helgaas wrote:
> On Thu, Apr 4, 2013 at 3:58 PM, Brian King <brking@linux.vnet.ibm.com> wrote:
> > @@ -1725,3 +1726,15 @@ static void fixup_hide_host_resource_fsl
> > }
> > DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_MOTOROLA, PCI_ANY_ID, fixup_hide_host_resource_fsl);
> > DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_FREESCALE, PCI_ANY_ID, fixup_hide_host_resource_fsl);
> > +
> > +static void fixup_vga(struct pci_dev *pdev)
> > +{
> > + u16 cmd;
> > +
> > + pci_read_config_word(pdev, PCI_COMMAND, &cmd);
> > + if ((cmd & (PCI_COMMAND_IO | PCI_COMMAND_MEMORY)) || !vga_default_device())
> > + vga_set_default_device(pdev);
> > +
> > +}
> > +DECLARE_PCI_FIXUP_CLASS_FINAL(PCI_ANY_ID, PCI_ANY_ID,
> > + PCI_CLASS_DISPLAY_VGA, 8, fixup_vga);
>
> This not really an arch-specific issue, so it's a shame to add another
> arch-specific quirk for it (in addition to the x86 and ia64 ones we
> already have).
>
> In b5e4efe7e0, Eiichiro Oiwa <eiichiro.oiwa.nm@hitachi.com> tried to
> make this quirk generic, but the implementation was naive and it
> didn't work for sparc64. There's a good thread about the sparc issue
> at https://lkml.org/lkml/2006/10/19/17
>
> The outcome was to basically revert back to arch-specific quirks with
> 6b5c76b8e2, but I think the generic version probably could have been
> made to work, possibly with a pcibios hook or something for anything
> that really is arch-dependent, like the shadowed ROM image.
>
> This particular patch is arch-specific, and I'm not going to try to
> nack it because I'm not the powerpc maintainer, but I will certainly
> try to help you if you want to push on making a generic version.
OK.
Looking at x86, ia64 and ours, there's a fair bit of difference.
x86/ia64 both walk up the parents checking PCI_BRIDGE_CTL_VGA, which
powerpc doesn't (though maybe it should?).
x86/ia64 set IORESOURCE_ROM_SHADOW, which powerpc doesn't.
ia64 doesn't call vga_set_default_device(), x86 and powerpc do.
So we'll merge this and maybe someone can tease out the common bits, but
personally I don't see that there's an obvious chunk of generic logic.
cheers
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [PATCH 3/3] powerpc: Set default VGA device
@ 2013-04-08 5:21 ` Michael Ellerman
0 siblings, 0 replies; 15+ messages in thread
From: Michael Ellerman @ 2013-04-08 5:21 UTC (permalink / raw)
To: Bjorn Helgaas
Cc: linux-pci@vger.kernel.org, klebers, sparclinux,
Lucas Kannebley Tavares, Brian King, linuxppc-dev
On Fri, Apr 05, 2013 at 02:24:09PM -0600, Bjorn Helgaas wrote:
> On Thu, Apr 4, 2013 at 3:58 PM, Brian King <brking@linux.vnet.ibm.com> wrote:
> > @@ -1725,3 +1726,15 @@ static void fixup_hide_host_resource_fsl
> > }
> > DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_MOTOROLA, PCI_ANY_ID, fixup_hide_host_resource_fsl);
> > DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_FREESCALE, PCI_ANY_ID, fixup_hide_host_resource_fsl);
> > +
> > +static void fixup_vga(struct pci_dev *pdev)
> > +{
> > + u16 cmd;
> > +
> > + pci_read_config_word(pdev, PCI_COMMAND, &cmd);
> > + if ((cmd & (PCI_COMMAND_IO | PCI_COMMAND_MEMORY)) || !vga_default_device())
> > + vga_set_default_device(pdev);
> > +
> > +}
> > +DECLARE_PCI_FIXUP_CLASS_FINAL(PCI_ANY_ID, PCI_ANY_ID,
> > + PCI_CLASS_DISPLAY_VGA, 8, fixup_vga);
>
> This not really an arch-specific issue, so it's a shame to add another
> arch-specific quirk for it (in addition to the x86 and ia64 ones we
> already have).
>
> In b5e4efe7e0, Eiichiro Oiwa <eiichiro.oiwa.nm@hitachi.com> tried to
> make this quirk generic, but the implementation was naive and it
> didn't work for sparc64. There's a good thread about the sparc issue
> at https://lkml.org/lkml/2006/10/19/17
>
> The outcome was to basically revert back to arch-specific quirks with
> 6b5c76b8e2, but I think the generic version probably could have been
> made to work, possibly with a pcibios hook or something for anything
> that really is arch-dependent, like the shadowed ROM image.
>
> This particular patch is arch-specific, and I'm not going to try to
> nack it because I'm not the powerpc maintainer, but I will certainly
> try to help you if you want to push on making a generic version.
OK.
Looking at x86, ia64 and ours, there's a fair bit of difference.
x86/ia64 both walk up the parents checking PCI_BRIDGE_CTL_VGA, which
powerpc doesn't (though maybe it should?).
x86/ia64 set IORESOURCE_ROM_SHADOW, which powerpc doesn't.
ia64 doesn't call vga_set_default_device(), x86 and powerpc do.
So we'll merge this and maybe someone can tease out the common bits, but
personally I don't see that there's an obvious chunk of generic logic.
cheers
^ permalink raw reply [flat|nested] 15+ messages in thread* Re: [PATCH 3/3] powerpc: Set default VGA device
2013-04-08 5:21 ` Michael Ellerman
(?)
@ 2013-04-08 6:39 ` Benjamin Herrenschmidt
-1 siblings, 0 replies; 15+ messages in thread
From: Benjamin Herrenschmidt @ 2013-04-08 6:39 UTC (permalink / raw)
To: Michael Ellerman
Cc: Bjorn Helgaas, Brian King, linux-pci@vger.kernel.org,
linuxppc-dev, Lucas Kannebley Tavares, klebers, sparclinux
On Mon, 2013-04-08 at 15:21 +1000, Michael Ellerman wrote:
> Looking at x86, ia64 and ours, there's a fair bit of difference.
>
> x86/ia64 both walk up the parents checking PCI_BRIDGE_CTL_VGA, which
> powerpc doesn't (though maybe it should?).
Unclear for several reasons.
That bit indicates that the bridge forwards the hard coded legacy VGA IO
ports and memory aperture. These are not necessary to get modern video
cards going. Only if legacy modes are used.
On x86, that tends to be the case (is it always even with EFI
nowadays ?).
On other architectures that is not necessarily the case. The firmware
can (and will) initialize the card using MMIO entirely and even if
possible disabling the legacy stuff, which means that turning those bits
on in the bridge is also unnecessary.
In fact, on such setups, the isn't really a concept of a "primary" video
card to begin with.
On the other hand, that also means that a video card initialized like
that is pretty much out of the grasp of the vga arbiter which has no
effect on it either.
Also be careful that while it may be relevant on x86, the VGA fwd bit is
not on the PCIe root complex on IBM machines.
Finally, P8 has no IO space at all...
> x86/ia64 set IORESOURCE_ROM_SHADOW, which powerpc doesn't.
>
> ia64 doesn't call vga_set_default_device(), x86 and powerpc do.
>
> So we'll merge this and maybe someone can tease out the common bits, but
> personally I don't see that there's an obvious chunk of generic logic.
Cheers,
Ben.
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 3/3] powerpc: Set default VGA device
@ 2013-04-08 6:39 ` Benjamin Herrenschmidt
0 siblings, 0 replies; 15+ messages in thread
From: Benjamin Herrenschmidt @ 2013-04-08 6:39 UTC (permalink / raw)
To: Michael Ellerman
Cc: Bjorn Helgaas, Brian King, linux-pci@vger.kernel.org,
linuxppc-dev, Lucas Kannebley Tavares, klebers, sparclinux
On Mon, 2013-04-08 at 15:21 +1000, Michael Ellerman wrote:
> Looking at x86, ia64 and ours, there's a fair bit of difference.
>
> x86/ia64 both walk up the parents checking PCI_BRIDGE_CTL_VGA, which
> powerpc doesn't (though maybe it should?).
Unclear for several reasons.
That bit indicates that the bridge forwards the hard coded legacy VGA IO
ports and memory aperture. These are not necessary to get modern video
cards going. Only if legacy modes are used.
On x86, that tends to be the case (is it always even with EFI
nowadays ?).
On other architectures that is not necessarily the case. The firmware
can (and will) initialize the card using MMIO entirely and even if
possible disabling the legacy stuff, which means that turning those bits
on in the bridge is also unnecessary.
In fact, on such setups, the isn't really a concept of a "primary" video
card to begin with.
On the other hand, that also means that a video card initialized like
that is pretty much out of the grasp of the vga arbiter which has no
effect on it either.
Also be careful that while it may be relevant on x86, the VGA fwd bit is
not on the PCIe root complex on IBM machines.
Finally, P8 has no IO space at all...
> x86/ia64 set IORESOURCE_ROM_SHADOW, which powerpc doesn't.
>
> ia64 doesn't call vga_set_default_device(), x86 and powerpc do.
>
> So we'll merge this and maybe someone can tease out the common bits, but
> personally I don't see that there's an obvious chunk of generic logic.
Cheers,
Ben.
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH 3/3] powerpc: Set default VGA device
@ 2013-04-08 6:39 ` Benjamin Herrenschmidt
0 siblings, 0 replies; 15+ messages in thread
From: Benjamin Herrenschmidt @ 2013-04-08 6:39 UTC (permalink / raw)
To: Michael Ellerman
Cc: sparclinux, linux-pci@vger.kernel.org, klebers, Brian King,
Lucas Kannebley Tavares, Bjorn Helgaas, linuxppc-dev
On Mon, 2013-04-08 at 15:21 +1000, Michael Ellerman wrote:
> Looking at x86, ia64 and ours, there's a fair bit of difference.
>
> x86/ia64 both walk up the parents checking PCI_BRIDGE_CTL_VGA, which
> powerpc doesn't (though maybe it should?).
Unclear for several reasons.
That bit indicates that the bridge forwards the hard coded legacy VGA IO
ports and memory aperture. These are not necessary to get modern video
cards going. Only if legacy modes are used.
On x86, that tends to be the case (is it always even with EFI
nowadays ?).
On other architectures that is not necessarily the case. The firmware
can (and will) initialize the card using MMIO entirely and even if
possible disabling the legacy stuff, which means that turning those bits
on in the bridge is also unnecessary.
In fact, on such setups, the isn't really a concept of a "primary" video
card to begin with.
On the other hand, that also means that a video card initialized like
that is pretty much out of the grasp of the vga arbiter which has no
effect on it either.
Also be careful that while it may be relevant on x86, the VGA fwd bit is
not on the PCIe root complex on IBM machines.
Finally, P8 has no IO space at all...
> x86/ia64 set IORESOURCE_ROM_SHADOW, which powerpc doesn't.
>
> ia64 doesn't call vga_set_default_device(), x86 and powerpc do.
>
> So we'll merge this and maybe someone can tease out the common bits, but
> personally I don't see that there's an obvious chunk of generic logic.
Cheers,
Ben.
^ permalink raw reply [flat|nested] 15+ messages in thread