2005-01-24 19:19:27

by [email protected]

[permalink] [raw]
Subject: Re: Fwd: Patch to control VGA bus routing and active VGA device.

On Mon, 24 Jan 2005 17:51:31 +0000, Matthew Wilcox <[email protected]> wrote:
> Yes -- *very* platform specific. Some are even configurable as to how
> much they support. See http://ftp.parisc-linux.org/docs/chips/zx1-mio.pdf

Is this a justification for doing device drivers for bridge chips? It
has been mentioned before but no one has done it.

Any ideas why the code I sent won't work on the x86? I can shut
routing off but I can't get it back on again.

The motivation behind the code is to get X to quit doing this from user space.

--
Jon Smirl
[email protected]


2005-01-24 19:48:12

by Jeff Garzik

[permalink] [raw]
Subject: Re: Fwd: Patch to control VGA bus routing and active VGA device.

Jon Smirl wrote:
> Is this a justification for doing device drivers for bridge chips? It
> has been mentioned before but no one has done it.


Yeah, people are usually slack and work around the problem.

A bridge driver is really wanted for several situations in today's
hardware...

Jeff


2005-01-24 19:55:46

by Russell King

[permalink] [raw]
Subject: Re: Fwd: Patch to control VGA bus routing and active VGA device.

On Mon, Jan 24, 2005 at 02:42:57PM -0500, Jeff Garzik wrote:
> Jon Smirl wrote:
> > Is this a justification for doing device drivers for bridge chips? It
> > has been mentioned before but no one has done it.
>
>
> Yeah, people are usually slack and work around the problem.
>
> A bridge driver is really wanted for several situations in today's
> hardware...

There's a very good reason not to have a bridge driver at the moment -
some PCI to PCI bridges need special drivers. Currently, as the device
model stands today, we can only have ONE PCI to PCI bridge driver for
all P2P bridges, which is bad news if you need a specific driver for,
eg, a mobility docking station P2P bridge.

As I said back in 2002, the device model needs a way to have driver
priories - how well a driver matches the hardware.

My idea was for the bus match function to return the "goodness"
factor of the match. For PCI, matching on just the class IDs would
be low goodness, but an exact match with both the vendor and device
IDs would yeild a good match.

When the device model has a driver or device added, it scans all current
devices or drivers (respectively) and chooses the best matched pair.
If the device already has an existing driver, it calls the ->remove
method to unbind the current device driver relationship before handing
it over to the more specific driver.

Unfortunately, I never got around to writing the mobility P2P bridge
driver because I didn't see much chance of this idea being adopted.

However, if we're going to start having generic drivers for such
hardware, this kind of functionality needs to be thought about.

--
Russell King
Linux kernel 2.6 ARM Linux - http://www.arm.linux.org.uk/
maintainer of: 2.6 PCMCIA - http://pcmcia.arm.linux.org.uk/
2.6 Serial core

2005-01-24 20:14:32

by Matthew Wilcox

[permalink] [raw]
Subject: Re: Fwd: Patch to control VGA bus routing and active VGA device.

On Mon, Jan 24, 2005 at 02:42:57PM -0500, Jeff Garzik wrote:
> Jon Smirl wrote:
> >Is this a justification for doing device drivers for bridge chips? It
> >has been mentioned before but no one has done it.
>
> Yeah, people are usually slack and work around the problem.
>
> A bridge driver is really wanted for several situations in today's
> hardware...

Annoyingly, we already have one, it's just special-cased for PCI Express.
On my todo for this week is to take it out of the pcie directory and fix it.

--
"Next the statesmen will invent cheap lies, putting the blame upon
the nation that is attacked, and every man will be glad of those
conscience-soothing falsities, and will diligently study them, and refuse
to examine any refutations of them; and thus he will by and by convince
himself that the war is just, and will thank God for the better sleep
he enjoys after this process of grotesque self-deception." -- Mark Twain

2005-01-24 20:24:53

by Matthew Wilcox

[permalink] [raw]
Subject: Re: Fwd: Patch to control VGA bus routing and active VGA device.

On Mon, Jan 24, 2005 at 02:17:15PM -0500, Jon Smirl wrote:
> On Mon, 24 Jan 2005 17:51:31 +0000, Matthew Wilcox <[email protected]> wrote:
> > Yes -- *very* platform specific. Some are even configurable as to how
> > much they support. See http://ftp.parisc-linux.org/docs/chips/zx1-mio.pdf
>
> Is this a justification for doing device drivers for bridge chips? It
> has been mentioned before but no one has done it.

I don't think this is ... the bridges mentioned are host->pci bridges.

> Any ideas why the code I sent won't work on the x86? I can shut
> routing off but I can't get it back on again.
>
> The motivation behind the code is to get X to quit doing this from user space.

I suppose we could log all access X does and compare to what your code
ends up doing ...

--
"Next the statesmen will invent cheap lies, putting the blame upon
the nation that is attacked, and every man will be glad of those
conscience-soothing falsities, and will diligently study them, and refuse
to examine any refutations of them; and thus he will by and by convince
himself that the war is just, and will thank God for the better sleep
he enjoys after this process of grotesque self-deception." -- Mark Twain

2005-01-24 23:23:54

by [email protected]

[permalink] [raw]
Subject: Re: Fwd: Patch to control VGA bus routing and active VGA device.

On Mon, 24 Jan 2005 19:55:23 +0000, Russell King
<[email protected]> wrote:
> There's a very good reason not to have a bridge driver at the moment -
> some PCI to PCI bridges need special drivers. Currently, as the device
> model stands today, we can only have ONE PCI to PCI bridge driver for
> all P2P bridges, which is bad news if you need a specific driver for,
> eg, a mobility docking station P2P bridge.

There is no requirement that the bridge driver be generic code. I
agree that it is simpler but there is nothing stopping a "generic"
driver from just having PCI IDs for all of the existing bridges built
into it and then load it like a normal driver. Loading all of the
bridge PCI IDs into the "generic" driver gets rid of the need for
driver priorities.

--
Jon Smirl
[email protected]

2005-01-25 04:31:06

by Greg KH

[permalink] [raw]
Subject: Re: Fwd: Patch to control VGA bus routing and active VGA device.

On Mon, Jan 24, 2005 at 07:55:23PM +0000, Russell King wrote:
> On Mon, Jan 24, 2005 at 02:42:57PM -0500, Jeff Garzik wrote:
> > Jon Smirl wrote:
> > > Is this a justification for doing device drivers for bridge chips? It
> > > has been mentioned before but no one has done it.
> >
> >
> > Yeah, people are usually slack and work around the problem.
> >
> > A bridge driver is really wanted for several situations in today's
> > hardware...
>
> There's a very good reason not to have a bridge driver at the moment -
> some PCI to PCI bridges need special drivers. Currently, as the device
> model stands today, we can only have ONE PCI to PCI bridge driver for
> all P2P bridges, which is bad news if you need a specific driver for,
> eg, a mobility docking station P2P bridge.
>
> As I said back in 2002, the device model needs a way to have driver
> priories - how well a driver matches the hardware.
>
> My idea was for the bus match function to return the "goodness"
> factor of the match. For PCI, matching on just the class IDs would
> be low goodness, but an exact match with both the vendor and device
> IDs would yeild a good match.

This can be done today in the bus specific match functions. And because
of that, I would argue that this belongs in the bus specific code, and
not in the driver core, as it's up to the bus to know what the different
types of "matches" that can happen, and what the priority is.

And yes, I agree that this needs to be done, I've been talking with a
few other people who are interested in it. I think the lock-rework code
needs to be finished before it can happen properly, so that we can do
the "unbind from one driver and give it to another one" type logic
properly.

thanks,

greg k-h

2005-01-27 10:00:19

by [email protected]

[permalink] [raw]
Subject: Re: Fwd: Patch to control VGA bus routing and active VGA device.

On Mon, 24 Jan 2005 20:24:59 -0800, Greg KH <[email protected]> wrote:
> This can be done today in the bus specific match functions. And because
> of that, I would argue that this belongs in the bus specific code, and
> not in the driver core, as it's up to the bus to know what the different
> types of "matches" that can happen, and what the priority is.

Here's a version of the VGA control code that actually works. It helps
if I pci_enable() the device before using it. There was nothing wrong
with the routing code.

I can also control multiple cards on the same bus by turning on/off
their response to IO space and then enabling VGA via the standard
ports. I can use this to move my console from card to card.

I have this code in drivers/pci because it needs to know add/remove
from hotplug. Is there a better way to structure it? Note that this is
not a VGA device, it is just a mechanism for controlling which VGA
device is active.

Another item I need to add is generating an initial hotplug event for
each secondary card. This event has to happen even if there is a card
specific driver loaded. The event will be used to run the reset
program needed by secondary cards.

--
Jon Smirl
[email protected]


Attachments:
(No filename) (1.19 kB)
patch (10.74 kB)
Download all attachments

2005-01-27 16:29:07

by Jesse Barnes

[permalink] [raw]
Subject: Re: Fwd: Patch to control VGA bus routing and active VGA device.

On Thursday, January 27, 2005 1:59 am, Jon Smirl wrote:
> Another item I need to add is generating an initial hotplug event for
> each secondary card. This event has to happen even if there is a card
> specific driver loaded. The event will be used to run the reset
> program needed by secondary cards.

Makes sense, having hotplug events would be nice. I've got a standalone bios
emulator (based on the X int10 library) that I hope to open source soon, we
could use that as a starting piont for the reset app.

+static void vga_enable(struct pci_dev *pdev, unsigned int enable)
...
+ outb(~0x01 & inb(0x3C3), 0x3C3);
+ outb(~0x08 & inb(0x46e8), 0x46e8);
+ outb(~0x01 & inb(0x102), 0x102);
+ pdev->resource[PCI_ROM_RESOURCE].flags &= ~IORESOURCE_VGA_ACTIVE;
+ if (pdev == vga_active)
+ vga_active = NULL;
+ bridge_no(pdev);

Those ins and outs won't work on all platforms unless they have a base address
(assigned by arch code) associated with them. But then again, I suppose if a
platform supports more than one legacy I/O space, it also supports multiple
active VGAs, so maybe this enable/disable code isn't needed for them...

Jesse

2005-01-28 17:35:04

by Grant Grundler

[permalink] [raw]
Subject: Re: Fwd: Patch to control VGA bus routing and active VGA device.

On Thu, Jan 27, 2005 at 08:28:43AM -0800, Jesse Barnes wrote:
> But then again,
> I suppose if a platform supports more than one legacy I/O space,

Eh?! there can only be *one* legacy I/O space.
We can support multipl IO port spaces, but only one can be the "legacy".

Moving the VGA device can only function within that legacy space
the way the code is written now (using hard coded addresses).
If it is intended to work with multiple IO Port address spaces,
then it needs to use the pci_dev->resource[] and mangle that appropriately.

grant

2005-01-28 18:47:16

by Jesse Barnes

[permalink] [raw]
Subject: Re: Fwd: Patch to control VGA bus routing and active VGA device.

On Friday, January 28, 2005 9:32 am, Grant Grundler wrote:
> On Thu, Jan 27, 2005 at 08:28:43AM -0800, Jesse Barnes wrote:
> > But then again,
> > I suppose if a platform supports more than one legacy I/O space,
>
> Eh?! there can only be *one* legacy I/O space.
> We can support multipl IO port spaces, but only one can be the "legacy".

What do you mean? If you define legacy I/O space to be
0x0000000000000000-0x000000000000ffff, then yes of course you're right. But
if you mean being able to access legacy ports at all, then no. On SGI
machines, there's a per-bus base address that can be used as the base for
port I/O, which is what I was getting at.

> Moving the VGA device can only function within that legacy space
> the way the code is written now (using hard coded addresses).
> If it is intended to work with multiple IO Port address spaces,
> then it needs to use the pci_dev->resource[] and mangle that appropriately.

There is no resource for some of the I/O port space that cards respond to. I
can set the I/O BAR of my VGA card to 0x400 and it'll still respond to
accesses at 0x3bc for example. That's what I mean by legacy space--space
that cards respond to but don't report in their PCI resources.

Jesse

2005-01-28 18:51:31

by [email protected]

[permalink] [raw]
Subject: Re: Fwd: Patch to control VGA bus routing and active VGA device.

On Fri, 28 Jan 2005 10:32:22 -0700, Grant Grundler
<[email protected]> wrote:
> Moving the VGA device can only function within that legacy space
> the way the code is written now (using hard coded addresses).
> If it is intended to work with multiple IO Port address spaces,
> then it needs to use the pci_dev->resource[] and mangle that appropriately.

Post a patch an I will incorporate it.

--
Jon Smirl
[email protected]

2005-01-28 19:16:30

by Grant Grundler

[permalink] [raw]
Subject: Re: Fwd: Patch to control VGA bus routing and active VGA device.

On Fri, Jan 28, 2005 at 01:36:48PM -0500, Jon Smirl wrote:
> > If it is intended to work with multiple IO Port address spaces,
> > then it needs to use the pci_dev->resource[] and mangle that appropriately.
>
> Post a patch an I will incorporate it.

Sorry - I only wanted to point out the short coming.
I don't care if it gets fixed (or not) since I don't use
or need to support multiple VGA cards. If someone else (in
HP) does, it's just nice to warn them what's broken.

thanks,
grant

2005-01-28 19:34:15

by [email protected]

[permalink] [raw]
Subject: Re: Fwd: Patch to control VGA bus routing and active VGA device.

On Fri, 28 Jan 2005 12:15:49 -0700, Grant Grundler
<[email protected]> wrote:
> I don't care if it gets fixed (or not) since I don't use
> or need to support multiple VGA cards. If someone else (in

Next year we are going to see a lot of multiple VGAs. Depending on
configuration the Nvidia4 chipset can support from one up to eight PCI
Express video cards simultaneously.

I would like to get this fixed in the kernel so that apps like X won't
do it from user space. Every time X does a read/alter/write to PCI
config space from user space is a place where bad things can happen.
The goal of this is to remove the PCI bus probing code from X.

--
Jon Smirl
[email protected]

2005-01-28 19:41:02

by Grant Grundler

[permalink] [raw]
Subject: Re: Fwd: Patch to control VGA bus routing and active VGA device.

On Fri, Jan 28, 2005 at 02:26:40PM -0500, Jon Smirl wrote:
> Next year we are going to see a lot of multiple VGAs. Depending on
> configuration the Nvidia4 chipset can support from one up to eight PCI
> Express video cards simultaneously.

Oh geez....someone is going to implement IO port space on PCI express device?!

/me gets out the cluebat...

grant

2005-01-28 19:55:00

by Jesse Barnes

[permalink] [raw]
Subject: Re: Fwd: Patch to control VGA bus routing and active VGA device.

On Friday, January 28, 2005 11:33 am, Grant Grundler wrote:
> > But
> > if you mean being able to access legacy ports at all, then no. On SGI
> > machines, there's a per-bus base address that can be used as the base for
> > port I/O, which is what I was getting at.
>
> Ok - my point was "0x3fc" will get routed to exactly one of those
> IO port address spaces.

Agreed, or it will cause a machine check if legacy I/O remapping isn't
supported (like on SGI).

> > There is no resource for some of the I/O port space that cards respond
> > to.
>
> Yes - I've heard several graphics cards are horrible broken WRT address
> decoding. Are PCI quirks supposed to handle that sort of thing?

I'm not sure if broken is the right work. All this stuff predates PCI by
quite a bit, so certain device classes claimed certain port ranges way before
cards were required to have programmable address decoders. VGA cards are
probably the most tenacious example of this, they respond to certain well
known ports after reset, regardless of BAR values.

> > I can set the I/O BAR of my VGA card to 0x400 and it'll still respond to
> > accesses at 0x3bc for example. That's what I mean by legacy space--space
> > that cards respond to but don't report in their PCI resources.
>
> Can't PCI quirks fix up the resources to reflect this?

Not sure, the I/O BAR may correspond to real registers too--and they may not
overlap with the ones in the well known space.

> I think one needs to fix up PCI IO Port resources to adjust
> for "The One" legacy IO port space getting routed to a different
> PCI segment - assuming no one submits a patch to change current
> behavior of using hard coded addresses.

I think we might just need a new resource for these well known ports, if my
last statement is true.

> Am I making more sense now?

Yeah, I think I understand. We could probably do the same thing on sn2 as you
do on parisc--add a 'segment 0' offset to the port so that it's routed
correctly. I think that's a little less flexible than adding a new resource
though, since it makes it harder for drivers to support more than one device
or devices on non-segment 0 busses.

Jesse

2005-01-28 19:59:30

by Grant Grundler

[permalink] [raw]
Subject: Re: Fwd: Patch to control VGA bus routing and active VGA device.

On Fri, Jan 28, 2005 at 10:41:41AM -0800, Jesse Barnes wrote:
> > Eh?! there can only be *one* legacy I/O space.
> > We can support multipl IO port spaces, but only one can be the "legacy".
>
> What do you mean? If you define legacy I/O space to be
> 0x0000000000000000-0x000000000000ffff, then yes of course you're right.

Yes - exactly.

> But
> if you mean being able to access legacy ports at all, then no. On SGI
> machines, there's a per-bus base address that can be used as the base for
> port I/O, which is what I was getting at.

Ok - my point was "0x3fc" will get routed to exactly one of those
IO port address spaces.

> > If it is intended to work with multiple IO Port address spaces,
> > then it needs to use the pci_dev->resource[] and mangle that appropriately.
>
> There is no resource for some of the I/O port space that cards respond to.

Yes - I've heard several graphics cards are horrible broken WRT address
decoding. Are PCI quirks supposed to handle that sort of thing?

Another example was Xf86 was poking around in MMIO space
to determine if such broken cards are installed.

> I can set the I/O BAR of my VGA card to 0x400 and it'll still respond to
> accesses at 0x3bc for example. That's what I mean by legacy space--space
> that cards respond to but don't report in their PCI resources.

Can't PCI quirks fix up the resources to reflect this?

I think one needs to fix up PCI IO Port resources to adjust
for "The One" legacy IO port space getting routed to a different
PCI segment - assuming no one submits a patch to change current
behavior of using hard coded addresses.

HP parisc and ia64 platforms implement seperate PCI segments under
each PCI host bus controller. Linux PCI "BIOS" support provides
the illusion it's all in one PCI segment on most (not all) platforms.
Some HP chipsets also provide a "Legacy" IO Port space that gets
routed to a chosen PCI Host bus controller.

parisc PCI BIOS adds the controller instance number to the IO port space
resource to help "inb()" generate the IO port cycle on the right
PCI segment. This needs to be fixed up if we decide a different
PCI segment should be segment 0 (and thus get references to 0x3fc).
I expect other arches with multi-segment support to do similar
fix ups.

Am I making more sense now?

thanks,
grant

2005-01-28 20:14:56

by Grant Grundler

[permalink] [raw]
Subject: Re: Fwd: Patch to control VGA bus routing and active VGA device.

On Fri, Jan 28, 2005 at 11:41:16AM -0800, Jesse Barnes wrote:
> Yeah, I think I understand. We could probably do the same thing on sn2
> as you do on parisc--add a 'segment 0' offset to the port so that it's routed
> correctly. I think that's a little less flexible than adding a new resource
> though, since it makes it harder for drivers to support more than one device
> or devices on non-segment 0 busses.

To be clear, the parisc code defines this in include/asm-parisc/pci.h:

#define HBA_PORT_SPACE_BITS 16
#define PCI_PORT_HBA(a) ((a) >> HBA_PORT_SPACE_BITS)

and PCI_PORT_HBA gets used in arch/parisc/kernel/pci.c.

Offhand, I don't know if ia64 has the equivalent.
But it sounds like it might need it.

grant

2005-01-28 20:10:15

by Matthew Wilcox

[permalink] [raw]
Subject: Re: Fwd: Patch to control VGA bus routing and active VGA device.

On Fri, Jan 28, 2005 at 12:33:20PM -0700, Grant Grundler wrote:
> > > If it is intended to work with multiple IO Port address spaces,
> > > then it needs to use the pci_dev->resource[] and mangle that appropriately.
> >
> > There is no resource for some of the I/O port space that cards respond to.
>
> Yes - I've heard several graphics cards are horrible broken WRT address
> decoding. Are PCI quirks supposed to handle that sort of thing?

No, PCI quirks are for fixing broken things. VGA cards are entitled to
3c0-3df and all their 10-bit aliases. I've been thinking for a while that we should mark the 10-bit aliases of ISA devices as used ... ie:

x100-x3ff
x500-x7ff
x900-xbff
xd00-xfff

Unfortunately, that may break some legitimate setups, but is hinted at
being a good idea in the EISA docs I've read. My laptop uses only the
10-bit aliases of motherboard space (x000-x0ff, x400-x4ff, x800-x8ff,
xc00-xcff) for devices, except for the ISA bridge, which gets:
1180-11bf : 0000:00:1f.0
1180-11bf : pnp 00:0b

The K6-2 is similar; only 10-bit aliases of motherboard space except for:
0a79-0a79 : isapnp write

Possibly a better solution (less likely to break things) would be to allow
drivers to reserve the 10-bit aliases too. Something like this:

static inline void request_isa_alias_regions(unsigned long start,
unsigned long n, const char *name)
{
int base;
for (base = 0x400; base < 0x10000; base += 0x400) {
request_region(base + start, n, name);
}
}

and then call that in drivers/video/console/vgacon.c

Russell, would that allay your issues with the kernel io resource database?

--
"Next the statesmen will invent cheap lies, putting the blame upon
the nation that is attacked, and every man will be glad of those
conscience-soothing falsities, and will diligently study them, and refuse
to examine any refutations of them; and thus he will by and by convince
himself that the war is just, and will thank God for the better sleep
he enjoys after this process of grotesque self-deception." -- Mark Twain

2005-01-28 20:19:51

by Russell King

[permalink] [raw]
Subject: Re: Fwd: Patch to control VGA bus routing and active VGA device.

On Fri, Jan 28, 2005 at 08:00:10PM +0000, Matthew Wilcox wrote:
> I've been thinking for a while that we should mark the 10-bit aliases
> of ISA devices as used

ISTR that windows does this.

> Russell, would that allay your issues with the kernel io resource database?

It makes the situation a whole lot clearer from the point of working
out what is free and what isn't, making it less likely that we'll
trample over some magic port which your VGA card needs.

This, along with throwing in the ports found via ACPI (which Dominik
has hinted) should (maybe that's the wrong word) give us a complete
picture and allow things like PCMCIA to do reliable resource
allocation. The same goes for Cardbus/PCI as well of course.

Maybe at this point the idea that you need to tell PCMCIA about which
resources it can validly use can finally be eliminated, which is a
big step towards eliminating it's dependence on userspace.

(I hope this is what you were talking about and I haven't just produced
a load of unrelated waffle!)

--
Russell King
Linux kernel 2.6 ARM Linux - http://www.arm.linux.org.uk/
maintainer of: 2.6 PCMCIA - http://pcmcia.arm.linux.org.uk/
2.6 Serial core

2005-01-30 07:51:31

by [email protected]

[permalink] [raw]
Subject: Re: Fwd: Patch to control VGA bus routing and active VGA device.

On Mon, 24 Jan 2005 20:24:59 -0800, Greg KH <[email protected]> wrote:
> And yes, I agree that this needs to be done, I've been talking with a
> few other people who are interested in it. I think the lock-rework code
> needs to be finished before it can happen properly, so that we can do
> the "unbind from one driver and give it to another one" type logic
> properly.

Unbind/give does not address what VGA control needs. VGA control needs
to track hotplug remove events so that it can route the display to
another VGA card if the active display is pulled. As far as I can tell
there is no way to monitor hotplug remove events, that's why I had to
add a callout in pci_destroy_dev().

VGA control also can't just install a device driver since that will
prevent the real DRM/FB device drivers from installing. It looks to me
like VGA control is more of a strange property of the PCI bus
architecture than a device driver.

Another strategy would be to create the concept of a PCI class driver
which a normal device driver would inherit from instead of replacing
it. But is there a need for another PCI class driver other than VGA?


--
Jon Smirl
[email protected]

2005-01-31 17:09:03

by Alan

[permalink] [raw]
Subject: Re: Fwd: Patch to control VGA bus routing and active VGA device.

On Gwe, 2005-01-28 at 20:00, Matthew Wilcox wrote:
> Possibly a better solution (less likely to break things) would be to allow
> drivers to reserve the 10-bit aliases too. Something like this:
>
> static inline void request_isa_alias_regions(unsigned long start,
> unsigned long n, const char *name)
> {
> int base;
> for (base = 0x400; base < 0x10000; base += 0x400) {
> request_region(base + start, n, name);
> }
> }
>
> and then call that in drivers/video/console/vgacon.c

Almost every x86 I/O device in ISA space has this problem so it would be
better if request_region learned to take a decode mask instead of adding
another 500 entries

2005-02-01 07:43:09

by Greg KH

[permalink] [raw]
Subject: Re: Fwd: Patch to control VGA bus routing and active VGA device.

On Thu, Jan 27, 2005 at 04:59:45AM -0500, Jon Smirl wrote:
> On Mon, 24 Jan 2005 20:24:59 -0800, Greg KH <[email protected]> wrote:
> > This can be done today in the bus specific match functions. And because
> > of that, I would argue that this belongs in the bus specific code, and
> > not in the driver core, as it's up to the bus to know what the different
> > types of "matches" that can happen, and what the priority is.
>
> Here's a version of the VGA control code that actually works. It helps
> if I pci_enable() the device before using it. There was nothing wrong
> with the routing code.
>
> I can also control multiple cards on the same bus by turning on/off
> their response to IO space and then enabling VGA via the standard
> ports. I can use this to move my console from card to card.
>
> I have this code in drivers/pci because it needs to know add/remove
> from hotplug. Is there a better way to structure it? Note that this is
> not a VGA device, it is just a mechanism for controlling which VGA
> device is active.
>
> Another item I need to add is generating an initial hotplug event for
> each secondary card. This event has to happen even if there is a card
> specific driver loaded. The event will be used to run the reset
> program needed by secondary cards.

Ick, patch wasn't inline for me to comment on it :(

Anyway, get rid of the ifdefs in the kernel .c code to start with.

thanks,

greg k-h

2005-02-01 16:24:50

by [email protected]

[permalink] [raw]
Subject: Re: Fwd: Patch to control VGA bus routing and active VGA device.

On Mon, 31 Jan 2005 22:38:17 -0800, Greg KH <[email protected]> wrote:
> Ick, patch wasn't inline for me to comment on it :(

Here's the patch inline. Big things that need to be addressed...

1) Generating the user space reset event. I tried using the pci
hotplug event but ran into the fb blacklist. The reset events need to
be serialized across multiple cards. Make a temp kobj or just use
call_userspacehelp?

2) Things need to be set up so that it will generate the reset event
when compiled in. So the event has to come after early user space is
set up.

3) Where does this code go? It's not a specific device driver since it
doesn't attach to a single piece of hardware. I also run into
interference from existing fb driver as a normal device driver. It
needs to tie into drivers for bus chips when those happen.

4) It needs to monitor hotplug add/remove. If you pull the bus with
the console on it, it will try to move the console to another VGA
device.

diff -Nru a/arch/i386/pci/fixup.c b/arch/i386/pci/fixup.c
--- a/arch/i386/pci/fixup.c 2005-01-27 04:01:08 -05:00
+++ b/arch/i386/pci/fixup.c 2005-01-27 04:01:08 -05:00
@@ -375,6 +375,6 @@
}
bus = bus->parent;
}
- pdev->resource[PCI_ROM_RESOURCE].flags |= IORESOURCE_ROM_SHADOW;
+ pdev->resource[PCI_ROM_RESOURCE].flags |= IORESOURCE_ROM_SHADOW |
IORESOURCE_VGA_ACTIVE;
}
DECLARE_PCI_FIXUP_HEADER(PCI_ANY_ID, PCI_ANY_ID, pci_fixup_video);
diff -Nru a/drivers/pci/Kconfig b/drivers/pci/Kconfig
--- a/drivers/pci/Kconfig 2005-01-27 04:01:08 -05:00
+++ b/drivers/pci/Kconfig 2005-01-27 04:01:08 -05:00
@@ -47,3 +47,13 @@

When in doubt, say Y.

+config VGA_CONTROL
+ bool "VGA Control"
+ depends on PCI
+ ---help---
+ Provides sysfs attributes for ensuring that only a single VGA
+ device can be enabled per PCI domain. If a VGA device is removed
+ via hotplug, display is routed to another VGA device if available.
+
+ If you have more than one VGA device, say Y.
+
diff -Nru a/drivers/pci/Makefile b/drivers/pci/Makefile
--- a/drivers/pci/Makefile 2005-01-27 04:01:08 -05:00
+++ b/drivers/pci/Makefile 2005-01-27 04:01:08 -05:00
@@ -28,6 +28,7 @@
obj-$(CONFIG_MIPS) += setup-bus.o setup-irq.o
obj-$(CONFIG_X86_VISWS) += setup-irq.o
obj-$(CONFIG_PCI_MSI) += msi.o
+obj-$(CONFIG_VGA_CONTROL) += vga.o

#
# ACPI Related PCI FW Functions
diff -Nru a/drivers/pci/bus.c b/drivers/pci/bus.c
--- a/drivers/pci/bus.c 2005-01-27 04:01:08 -05:00
+++ b/drivers/pci/bus.c 2005-01-27 04:01:08 -05:00
@@ -85,6 +85,9 @@

pci_proc_attach_device(dev);
pci_create_sysfs_dev_files(dev);
+#if CONFIG_VGA_CONTROL
+ pci_vga_add_device(dev);
+#endif
}

/**
diff -Nru a/drivers/pci/pci.h b/drivers/pci/pci.h
--- a/drivers/pci/pci.h 2005-01-27 04:01:08 -05:00
+++ b/drivers/pci/pci.h 2005-01-27 04:01:08 -05:00
@@ -11,6 +11,8 @@
void (*alignf)(void *, struct resource *,
unsigned long, unsigned long),
void *alignf_data);
+extern int pci_vga_add_device(struct pci_dev *pdev);
+extern int pci_vga_remove_device(struct pci_dev *pdev);
/* PCI /proc functions */
#ifdef CONFIG_PROC_FS
extern int pci_proc_attach_device(struct pci_dev *dev);
diff -Nru a/drivers/pci/remove.c b/drivers/pci/remove.c
--- a/drivers/pci/remove.c 2005-01-27 04:01:08 -05:00
+++ b/drivers/pci/remove.c 2005-01-27 04:01:08 -05:00
@@ -26,6 +26,9 @@

static void pci_destroy_dev(struct pci_dev *dev)
{
+#if CONFIG_VGA_CONTROL
+ pci_vga_remove_device(dev);
+#endif
pci_proc_detach_device(dev);
pci_remove_sysfs_dev_files(dev);
device_unregister(&dev->dev);
diff -Nru a/drivers/pci/setup-bus.c b/drivers/pci/setup-bus.c
--- a/drivers/pci/setup-bus.c 2005-01-27 04:01:08 -05:00
+++ b/drivers/pci/setup-bus.c 2005-01-27 04:01:08 -05:00
@@ -64,7 +64,9 @@

if (class == PCI_CLASS_DISPLAY_VGA ||
class == PCI_CLASS_NOT_DEFINED_VGA)
- bus->bridge_ctl |= PCI_BRIDGE_CTL_VGA;
+ /* only route to the active VGA, ignore inactive ones */
+ if (dev->resource[PCI_ROM_RESOURCE].flags & IORESOURCE_VGA_ACTIVE)
+ bus->bridge_ctl |= PCI_BRIDGE_CTL_VGA;

pdev_sort_resources(dev, &head);
}
diff -Nru a/drivers/pci/vga.c b/drivers/pci/vga.c
--- /dev/null Wed Dec 31 16:00:00 196900
+++ b/drivers/pci/vga.c 2005-01-27 04:01:08 -05:00
@@ -0,0 +1,254 @@
+/*
+ * linux/drivers/pci/vga.c
+ *
+ * (C) Copyright 2004 Jon Smirl <[email protected]>
+ *
+ * VGA control logic for ensuring only a single enabled VGA device
+ */
+
+#include <linux/init.h>
+#include <linux/fs.h>
+#include <linux/cdev.h>
+#include <linux/pci.h>
+#include <linux/major.h>
+
+static int vga_initialized = 0;
+static struct pci_dev *vga_active = NULL;
+
+static void bridge_yes(struct pci_dev *pdev)
+{
+ struct pci_dev *bridge;
+ struct pci_bus *bus;
+
+ /* Make sure the bridges route to us */
+ bus = pdev->bus;
+ while (bus) {
+ bridge = bus->self;
+ if (bridge) {
+ bus->bridge_ctl |= PCI_BRIDGE_CTL_VGA;
+ pci_write_config_word(bridge, PCI_BRIDGE_CONTROL, bus->bridge_ctl);
+ }
+ bus = bus->parent;
+ }
+}
+
+static void bridge_no(struct pci_dev *pdev)
+{
+ struct pci_dev *bridge;
+ struct pci_bus *bus;
+
+ /* Make sure the bridges don't route to us */
+ bus = pdev->bus;
+ while (bus) {
+ bridge = bus->self;
+ if (bridge) {
+ bus->bridge_ctl &= ~PCI_BRIDGE_CTL_VGA;
+ pci_write_config_word(bridge, PCI_BRIDGE_CONTROL, bus->bridge_ctl);
+ }
+ bus = bus->parent;
+ }
+}
+
+static void vga_enable(struct pci_dev *pdev, unsigned int enable)
+{
+ u16 command;
+
+ bridge_yes(pdev);
+
+ if (enable) {
+ pci_enable_device(pdev);
+ /* this assumes all other potential VGA devices are disabled */
+ outb(0x01 | inb(0x3C3), 0x3C3); /* 0 - enable */
+ outb(0x08 | inb(0x46e8), 0x46e8);
+ outb(0x01 | inb(0x102), 0x102);
+ pdev->resource[PCI_ROM_RESOURCE].flags |= IORESOURCE_VGA_ACTIVE;
+ vga_active = pdev;
+
+ /* return and leave the card enabled */
+ return;
+ }
+
+ pci_read_config_word(pdev, PCI_COMMAND, &command);
+ pci_write_config_word(pdev, PCI_COMMAND, command | PCI_COMMAND_IO |
PCI_COMMAND_MEMORY);
+
+ outb(~0x01 & inb(0x3C3), 0x3C3);
+ outb(~0x08 & inb(0x46e8), 0x46e8);
+ outb(~0x01 & inb(0x102), 0x102);
+ pdev->resource[PCI_ROM_RESOURCE].flags &= ~IORESOURCE_VGA_ACTIVE;
+ if (pdev == vga_active)
+ vga_active = NULL;
+ bridge_no(pdev);
+
+ pci_write_config_word(pdev, PCI_COMMAND, command);
+}
+
+/* echo these values to the sysfs vga attribute on a VGA device */
+enum eEnable {
+ VGA_DISABLE_THIS = 0, /* If this VGA is enabled, disable it. */
+ VGA_ENABLE_THIS = 1, /* Disable all VGAs then enable this VGA, mark
as active VGA */
+ /* Used while resetting a board, board being reset may not be the
active VGA */
+ VGA_DISABLE_ALL = 2, /* Remember active VGA then disable all VGAa
and devices */
+ VGA_ENABLE_ACTIVE = 3, /* Make sure all VGAs are disabled, then
reenable active VGA */
+};
+
+static void set_state(struct pci_dev *pdev, enum eEnable enable)
+{
+ struct pci_dev *pcidev = NULL;
+ unsigned int class;
+
+ if (enable == VGA_DISABLE_THIS)
+ if (vga_active != pdev)
+ return;
+
+ vga_enable(vga_active, 0);
+
+ /* loop over all devices and make sure no multiple routings */
+ while ((pcidev = pci_get_subsys(PCI_ANY_ID, PCI_ANY_ID, PCI_ANY_ID,
PCI_ANY_ID, pcidev)) != NULL) {
+ class = pcidev->class >> 8;
+
+ if (class == PCI_CLASS_DISPLAY_VGA)
+ bridge_no(pcidev);
+ }
+
+ /* loop over all devices and make sure everyone is disabled */
+ while ((pcidev = pci_get_subsys(PCI_ANY_ID, PCI_ANY_ID, PCI_ANY_ID,
PCI_ANY_ID, pcidev)) != NULL) {
+ class = pcidev->class >> 8;
+
+ if (class == PCI_CLASS_DISPLAY_VGA)
+ vga_enable(pcidev, 0);
+ }
+
+ switch (enable) {
+ case VGA_DISABLE_THIS:
+ case VGA_DISABLE_ALL:
+ break;
+
+ /* Mark us active if requested */
+ case VGA_ENABLE_THIS:
+ vga_enable(pdev, 1);
+ break;
+
+ /* Restore active device if requested */
+ case VGA_ENABLE_ACTIVE:
+ vga_enable(vga_active, 1);
+ break;
+ }
+}
+
+/* sysfs store for VGA device */
+static ssize_t vga_device_store(struct device *dev, const char *buf,
size_t count)
+{
+ char *last;
+ struct pci_dev *pdev = to_pci_dev(dev);
+ enum eEnable enable;
+
+ /* sysfs strings are terminated by \n */
+ enable = simple_strtoul(buf, &last, 0);
+ if (last == buf)
+ return -EINVAL;
+
+ if ((enable < VGA_DISABLE_THIS) || (enable > VGA_ENABLE_ACTIVE))
+ return -EINVAL;
+
+ set_state(pdev, enable);
+
+ return count;
+}
+
+/* sysfs show for VGA device */
+static ssize_t vga_device_show(struct device *dev, char *buf)
+{
+ struct pci_dev *pdev = to_pci_dev(dev);
+ return sprintf(buf, "%d\n", (pdev->resource[PCI_ROM_RESOURCE].flags
& IORESOURCE_VGA_ACTIVE) != 0);
+}
+
+static struct device_attribute vga_device_attr = __ATTR(vga,
S_IRUGO|S_IWUSR, vga_device_show, vga_device_store);
+
+/* sysfs show for VGA routing bridge */
+static ssize_t vga_bridge_show(struct device *dev, char *buf)
+{
+ struct pci_dev *pdev = to_pci_dev(dev);
+ u16 l;
+
+ /* don't trust the shadow PCI_BRIDGE_CTL_VGA in pdev */
+ /* user space may change hardware without telling the kernel */
+ pci_read_config_word(pdev, PCI_BRIDGE_CONTROL, &l);
+ return sprintf(buf, "%d\n", (l & PCI_BRIDGE_CTL_VGA) != 0);
+}
+
+static struct device_attribute vga_bridge_attr = __ATTR(vga, S_IRUGO,
vga_bridge_show, NULL);
+
+/* If the device is a VGA or a bridge, add a VGA sysfs attribute */
+int pci_vga_add_device(struct pci_dev *pdev)
+{
+ int class = pdev->class >> 8;
+
+ if (!vga_initialized)
+ return -EACCES;
+
+ if (class == PCI_CLASS_DISPLAY_VGA) {
+ device_create_file(&pdev->dev, &vga_device_attr);
+
+ /* record the active boot device when located */
+ if (pdev->resource[PCI_ROM_RESOURCE].flags & IORESOURCE_VGA_ACTIVE)
+ vga_active = pdev;
+ return 0;
+ }
+
+ if ((class == PCI_CLASS_BRIDGE_PCI) || (class == PCI_CLASS_BRIDGE_CARDBUS)) {
+ device_create_file(&pdev->dev, &vga_bridge_attr);
+ }
+ return 0;
+}
+
+/* If the device is a VGA or a bridge, remove the VGA sysfs attribute */
+int pci_vga_remove_device(struct pci_dev *pdev)
+{
+ struct pci_dev *pcidev = NULL;
+ int class = pdev->class >> 8;
+
+ if (!vga_initialized)
+ return -EACCES;
+
+ if (class == PCI_CLASS_DISPLAY_VGA) {
+ device_remove_file(&pdev->dev, &vga_device_attr);
+
+ /* record the active boot device when located */
+ if (vga_active == pdev) {
+ while ((pcidev = pci_get_subsys(PCI_ANY_ID, PCI_ANY_ID,
PCI_ANY_ID, PCI_ANY_ID, pcidev)) != NULL) {
+ class = pcidev->class >> 8;
+ if (class != PCI_CLASS_DISPLAY_VGA)
+ continue;
+ if (pcidev == pdev)
+ continue;
+ set_state(pcidev, VGA_ENABLE_THIS);
+ break;
+ }
+ if (pcidev == NULL)
+ set_state(NULL, VGA_DISABLE_ALL);
+
+ }
+ return 0;
+ }
+
+ if ((class == PCI_CLASS_BRIDGE_PCI) || (class == PCI_CLASS_BRIDGE_CARDBUS))
+ device_remove_file(&pdev->dev, &vga_bridge_attr);
+
+ return 0;
+}
+
+/* Initialize by scanning all devices */
+static int __init vga_init(void)
+{
+ struct pci_dev *pdev = NULL;
+
+ vga_initialized = 1;
+
+ while ((pdev = pci_get_subsys(PCI_ANY_ID, PCI_ANY_ID, PCI_ANY_ID,
PCI_ANY_ID, pdev)) != NULL)
+ pci_vga_add_device(pdev);
+
+ return 0;
+}
+
+__initcall(vga_init);
+
diff -Nru a/include/linux/ioport.h b/include/linux/ioport.h
--- a/include/linux/ioport.h 2005-01-27 04:01:08 -05:00
+++ b/include/linux/ioport.h 2005-01-27 04:01:08 -05:00
@@ -41,7 +41,6 @@
#define IORESOURCE_CACHEABLE 0x00004000
#define IORESOURCE_RANGELENGTH 0x00008000
#define IORESOURCE_SHADOWABLE 0x00010000
-#define IORESOURCE_BUS_HAS_VGA 0x00080000

#define IORESOURCE_DISABLED 0x10000000
#define IORESOURCE_UNSET 0x20000000
@@ -86,6 +85,7 @@
#define IORESOURCE_ROM_ENABLE (1<<0) /* ROM is enabled, same as
PCI_ROM_ADDRESS_ENABLE */
#define IORESOURCE_ROM_SHADOW (1<<1) /* ROM is copy at C000:0 */
#define IORESOURCE_ROM_COPY (1<<2) /* ROM is alloc'd copy, resource
field overlaid */
+#define IORESOURCE_VGA_ACTIVE (1<<3) /* VGA device is active */

/* PC/ISA/whatever - the normal PC address spaces: IO and memory */
extern struct resource ioport_resource;

--
Jon Smirl
[email protected]