PCI Subsystem Rework for X.org 7.1 Proposal

[Jesse Barnes]

On Friday, February 3, 2006 8:20 am, Adam Jackson wrote:
> > The big missing pieces seem to be:
> >   o VGA arbitration - we (the kernel folks) really need to tackle
> > this properly
> >   o mapping memory with different attributes - benh and I discussed
> > some ideas awhile back, but no code ever came of it (did we settle
> > on thinking that an madvise(2) addition was best?)
>
> madvise(2) currently seems to describe usage patterns rather than
> hardware attributes, so I'm mildly wary of that idea.  The flags
> argument to mmap(2), however, is already a grab-bag of attributes.

That's a good point, I knew there was another option I was forgetting.  
That may well be the best possible approach.

> > Random naming thoughts: pci_system_init instead?  Or just pci_init,
> > though that assumes that the 'pci' namespace is where we want these
> > calls; anyway somehow pci_system seems redundant...
>
> Are we really to the point where PCI is like Ethernet (ie, its
> replacement will also be called PCI)?  If not, or if we care about
> proprietary workstation busses like on older Sun or SGI kit, then we
> should possibly think about bus_* instead of pci_*.

Ugg... I think that gets ugly fast, since then the semantics of the 
various calls may be different depending on the bus your device is on.  
OTOH, having two separate drivers for the same chip whose only 
difference is the bus type is pretty painful too.  Anyway, what bus 
types do we care about?

  o ISA?  (I definitely don't care about this one, and if we did it could
    probably just have its own API)
  o PCI
  o AGP
  o PCIe
  o sbus?
  o MCA?

PCI, AGP, and PCIe can probably be covered by this API (with AGP 
requiring some additions I suppose), as long as we handle the "mapping 
with attributes" issue well.

> > struct pci_mem_region {
> > >     void * memory;
> > >     pciaddr_t bus_addr;
> > >     pciaddr_t base_addr;
> > >     pciaddr_t size;
> > > };
> >
> > Hm, I think the bus_addr and base_addr should be hidden from the
> > interface, maybe in a void *platform_data structure or something,
> > since they're generally only used by low level platform code (or am
> > I wrong here, are there cases where the driver needs this info?). 
> > I've seen way too many people mistake the bus address for something
> > else.  Which reminds me, sparse annotation of the various address
> > space pointers in X might be a good idea at some point (ala the
> > Linux kernel's __user attributes)...
>
> If addresses written to the card need to be proper bus addresses
> (instead of merely relative to some BAR) then the server needs to know
> that.

Can you provide an example of this?  I was trying to come up with one but 
could only think of low level platform stuff that probably shouldn't be 
exposed to the driver...

> > It seems like the kernel may be doing power management of various
> > devices as well, so should there be some sort of callback mechanism?
> >  This could get tricky if the kernel suspends a device without
> > telling X about it...
>
> This sounds plausible.  I think in general we can say that devices
> that are in some sense managed by userspace should be shut down by
> userspace (if the managing process asks for it), and that userspace
> shutdown has to precede kernelspace shutdown.

Yeah, this area needs more thought (and can probably be left out of the 
initial API since grabbing a device and mapping memory are orthogonal 
enough to power management that we won't get into trouble if we leave it 
out).  My comment was more of a handwavy "how do we handle this" than 
anything else...

Jesse