pci rework not quite ready yet?

[Keith Packard]

On Wed, 2007-08-29 at 08:11 -0700, Jesse Barnes wrote:

> I'd be much happier if we just got away from using bus 
> addresses entirely.  There's really no way we can use them reliably short of 
> growing sophisticated bus drivers in the X server...

I disagree -- all bus addresses for a single card live in a single
address space, and the driver must use many of them while programming
various operations. In particular, if you want to split a single BAR
into multiple 'regions' (as we do on Intel cards when mapping the GTT),
then you must talk about bus addresses instead of trying to hide
entirely behind a BAR abstraction.

It's also far easier to store just the bus address in the driver than to
track both BAR index and offset. Given that you must often program the
card with bus addresses, it doesn't make sense to make the driver
constantly refer back to the pci BAR tables unless you think those are
going to change.

> Yeah, the MTRR API for Linux at least isn't that great.  It takes a CPU 
> physical address as a base argument, which means we have to program it with 
> the value from the Linux pci_dev->resource fields, rather than the BAR 
> address since these can be different on some platforms.

Which means libpciaccess must manage this for us; I certainly agree that
CPU physical addresses should be hidden from the X server and drivers.

> VGA legacy resources (VGA I/O ports and memory) for example fall outside this 
> range; devices may respond to cycles on those addresses but not list them in 
> their BARs.  I'm not sure about other ranges.  But libpciaccess shouldn't 
> really be dealing with those regions I think.

It does already for VBIOS rom reading. In fact, it would be nice if it
could deal with VGA ports as well; we may want to migrate the VGA s

> Also, adding this new interface seems like a lot of churn for just dealing 
> with cacheability attributes.  Maybe it would be best to have a separate, 
> madvise-like call to set the attributes instead?

libpciaccess is a brand new API, so changing it now is far easier than
at any future point. In particular, as every mapping must indicate what
kind of access it needs, we should enforce this by including it in the
mapping call.

Right now, the library API isn't sufficient for the X server's needs;
let's just make it right.

> Ick, yeah, getting rid of PCITAG entirely would be nice.  But any cleanups (I 
> think there are many more we could pursue) should be put off until things are 
> working well.

Just thinking about preserving our existing API to make driver porting
easier, but as we cannot retain it entirely, perhaps it's better to just
bail and remove the old code.

-- 
keith.packard at intel.com
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 189 bytes
Desc: This is a digitally signed message part
URL: <http://lists.x.org/archives/xorg/attachments/20070829/2e921144/attachment-0001.pgp>