legacy vga ... what do you think?

[Eric W. Biederman]

ron minnich <rminnich at lanl.gov> writes:

> The issue is that at the amdk8 northbridge, we need to know bus/dev/fn for
> the hardware so we can set up the VGA_EN bit in the right PCIIO pair as
> well as an MMIO entry for it.

Huh?  There is a fairly definitive association between resources and
non-coherent hypertransport chains.  What do you need the device for?

> It's not enough to just set a bit in the 
> bridge on the K8 -- you have to set up routing to the right Hypertransport 
> bus. 

Right and we know that bus is the bus we set the VGA_EN bit for.

> You have to know where the device is.

Yes but you should not need to know which device it is.

> By far the easiest way to do this is to add a simple structure member to
> the bus structure:
> 	struct device *vgadev;

Huh?


> so we have:
> struct bus {
>         device_t        dev;            /* This bridge device */
>         device_t        children;       /* devices behind this bridge */
> 	unsigned        bridge_ctrl;    /* Bridge control register */
> 	/* NEW */
> 	struct device 	*vgadev;	/* if bridge_ctl has 
> 					 * PCI_CB_BRIDGE_CTL_VGA set, 
> 					 * this contains pointer to 
> 					 * the device.
> 					 */
> 	/* END NEW */
>         unsigned char   link;           /* The index of this link */
>         unsigned char   secondary;      /* secondary bus number */
>         unsigned char   subordinate;    /* max subordinate bus number */
>         unsigned char   cap;            /* PCi capability offset */
> };
> 
> Setting vgadev is then trivial in the allocate_vga_resource since in that
> function you already have a pointer to the vga device; or just set the
> pointer. Either way, when you are at a bridge and know that the bridge has
> vga on the bus somewhere, you can easily get the info you need to set up
> the bridge if it is a complex bridge like the K8.

Yes the simple enough it just appears to be unnecessary.
 
>         while(bus) {
>                 bus->bridge_ctrl |= PCI_BRIDGE_CTL_VGA;
> 		/* NEW */
> 		bus->vgadev = vga;
> 		/* END NEW */
>                 bus = (bus == bus->dev->bus)? 0 : bus->dev->bus;
>         }
> 
> 
> Unless there is a huge problem with this I will try to get it done 
> tomorrow. It's a new structure member and one line of code and we're on 
> the air.
> 
> It is not totally general but ... VGA is "special". As in, really ugly.

Yes, the vga routing at least needs a special case.  It is a common
case so there is no problem with that.  The idea is to have a model
that we progressively refine into what we need.

What I don't see is what you need vgadev for.  The only case I can
think of is short cutting to the emulator.  In that case one global
would be fine.

Looking at the code though there is a bug.  When it finds a device
to give the legacy vga resources to it does not allocate any MMIO
resources.  For bridges this is a normal resource so this looks
a real bug in the generic code.  If that is why you need
the vgadev,  let's fix the generic code to handle that part properly. 

Eric