pci_fixup_video() bogosity

pci_fixup_video() bogosity

[Benjamin Herrenschmidt]

Hi !

While working on writing a VGA access arbiter for kernel & userland,
I wondered how to properly get my "initial" state at boot. For that,
I looked at how the new PCI ROM stuff does to find out who owns the
memory shadow at c0000, and found it quite bogus.

>From what I see, the code is only based on looking at what bridges
have VGA forwarding enabled. It doesn't test the actual IO and Memory
enable bits of the VGA cards themselves.

What if you have 2 cards under the same bridge ?

Ben.

Re: pci_fixup_video() bogosity

[Jon Smirl]

On Tue, 08 Mar 2005 15:38:29 +1100, Benjamin Herrenschmidt
<benh@kernel.crashing.org> wrote:
> Hi !
> 
> While working on writing a VGA access arbiter for kernel & userland,
> I wondered how to properly get my "initial" state at boot. For that,
> I looked at how the new PCI ROM stuff does to find out who owns the
> memory shadow at c0000, and found it quite bogus.
> 
> >From what I see, the code is only based on looking at what bridges
> have VGA forwarding enabled. It doesn't test the actual IO and Memory
> enable bits of the VGA cards themselves.

Let's fix it up and make it more robust. I was playing with checking
IO/mem enable and forgot to finish it.

> What if you have 2 cards under the same bridge ?

I believe the default on x86 is to pick the one in the lowest slot
number. What happens on PPC?

-- 
Jon Smirl
jonsmirl@gmail.com

Re: pci_fixup_video() bogosity

[Benjamin Herrenschmidt]

On Tue, 2005-03-08 at 00:57 -0500, Jon Smirl wrote:
> On Tue, 08 Mar 2005 15:38:29 +1100, Benjamin Herrenschmidt
> <benh@kernel.crashing.org> wrote:
> > Hi !
> > 
> > While working on writing a VGA access arbiter for kernel & userland,
> > I wondered how to properly get my "initial" state at boot. For that,
> > I looked at how the new PCI ROM stuff does to find out who owns the
> > memory shadow at c0000, and found it quite bogus.
> > 
> > >From what I see, the code is only based on looking at what bridges
> > have VGA forwarding enabled. It doesn't test the actual IO and Memory
> > enable bits of the VGA cards themselves.
> 
> Let's fix it up and make it more robust. I was playing with checking
> IO/mem enable and forgot to finish it.

Ok, have a look at my prototype VGA arbiter. I need something similar to
spot the "default" VGA device at boot, maybe your stuff could be moved
completely to the arbiter ?
> 
> > What if you have 2 cards under the same bridge ?
> 
> I believe the default on x86 is to pick the one in the lowest slot
> number. What happens on PPC?

What I mean is your code won't work afaik, you should check the one
which has MEM/IO enabled. On PPC, I don't know, depends on the firmware,
pretty much all pmac cards come with the legacy decoding disabled anyway
(they are fully posted in "native" mode by the firmware).

Some PPC's have x86 emulators and/or some VGA capabilities, but then, I
expect only one of them to be left enabled by the firmware.
 
Ben.

Re: pci_fixup_video() bogosity

[Jon Smirl]

Patch to make detection of boot video device more robust. Should I
leave the printk in?

-- 
Jon Smirl
jonsmirl@gmail.com

===== arch/i386/pci/fixup.c 1.24 vs edited =====
--- 1.24/arch/i386/pci/fixup.c	2005-01-11 19:42:41 -05:00
+++ edited/arch/i386/pci/fixup.c	2005-03-10 22:32:35 -05:00
@@ -355,11 +355,12 @@
  * is marked here since the boot video device will be the only enabled
  * video device at this point.
  *
- */static void __devinit pci_fixup_video(struct pci_dev *pdev)
+ */
+static void __devinit pci_fixup_video(struct pci_dev *pdev)
 {
 	struct pci_dev *bridge;
 	struct pci_bus *bus;
-	u16 l;
+	u16 config;
 
 	if ((pdev->class >> 8) != PCI_CLASS_DISPLAY_VGA)
 		return;
@@ -369,12 +370,16 @@
 	while (bus) {
 		bridge = bus->self;
 		if (bridge) {
-			pci_read_config_word(bridge, PCI_BRIDGE_CONTROL, &l);
-			if (!(l & PCI_BRIDGE_CTL_VGA))
+			pci_read_config_word(bridge, PCI_BRIDGE_CONTROL, &config);
+			if (!(config & PCI_BRIDGE_CTL_VGA))
 				return;
 		}
 		bus = bus->parent;
 	}
-	pdev->resource[PCI_ROM_RESOURCE].flags |= IORESOURCE_ROM_SHADOW;
+	pci_read_config_word(pdev, PCI_COMMAND, &config);
+	if (config & (PCI_COMMAND_IO | PCI_COMMAND_MEMORY)) {
+		pdev->resource[PCI_ROM_RESOURCE].flags |= IORESOURCE_ROM_SHADOW;
+		printk(KERN_INFO "Boot video device is %s\n", pci_name(pdev));
+	}
 }
 DECLARE_PCI_FIXUP_HEADER(PCI_ANY_ID, PCI_ANY_ID, pci_fixup_video);

Re: pci_fixup_video() bogosity

[Benjamin Herrenschmidt]

On Thu, 2005-03-10 at 22:39 -0500, Jon Smirl wrote:
> Patch to make detection of boot video device more robust. Should I
> leave the printk in?

Hrm... yes, but make it KERN_DEBUG.

Ben.