## PCI Rework HOWTO

Here's a list of steps you need to take to convert a video driver to the new libpciaccess library. Please feel free to fix this page where you find it inaccurate or incomplete. The examples here are from the intel driver and assume that you want to continue allowing the driver to be built with older X servers by making all of the changes conditional on whether the server uses libpciaccess.


### configure.ac changes

This patch adds a check to see if the X server is using libpciaccess (the server will have XSERVER_LIBPCIACCESS defined in xorg-server.h), then makes sure the pciaccess package is installed and available.


[[!format txt """
 save_CFLAGS="$CFLAGS"
 CFLAGS="$XORG_CFLAGS"
 AC_CHECK_HEADER(xf86Modes.h,[XMODES=yes],[XMODES=no],[#include "xorg-server.h"])
+AC_CHECK_DECL(XSERVER_LIBPCIACCESS,
+             [XSERVER_LIBPCIACCESS=yes],[XSERVER_LIBPCIACCESS=no],
+             [#include "xorg-server.h"])
 CFLAGS="$save_CFLAGS"

+if test x$XSERVER_LIBPCIACCESS = xyes; then
+       PKG_CHECK_MODULES([PCIACCESS], [pciaccess >= 0.8.0])
+fi
+
+AM_CONDITIONAL(XSERVER_LIBPCIACCESS, test "x$XSERVER_LIBPCIACCESS" = xyes)
 AM_CONDITIONAL(XMODES, test "x$XMODES" = xno)

 if test "x$XSERVER_SOURCE" = x; then
"""]]


Note: I have always encountered the xorg header files in /usr/include/xorg/, so i corrected the include. If this is not the usual location, please revert it.


### pciVideoPtr → struct pci_device *

In the `ScrnInfoRec` driver private structure, there is a pointer to the pciVideoPtr structure which contains identification information about the associated PCI device. libpciaccess has a parallel structure, struct pci_device which contains the same information using different names. Another change here is to compute (and store) the PCI base address register indices in this structure; mapping PCI space now requires using these register indices instead of just the physical address of the hardware.


[[!format txt """
@ -61,6 +61,11 @@ SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
 #include "xf86Crtc.h"
 #include "xf86RandR12.h"

+#include "xorg-server.h"
+#ifdef XSERVER_LIBPCIACCESS
+#include <pciaccess.h>
+#endif
+
 #ifdef XF86DRI
 #include "xf86drm.h"
 #include "sarea.h"
@@ -360,8 +365,15 @@ typedef struct _I830Rec {
    unsigned long MMIOAddr;
    IOADDRESS ioBase;
    EntityInfoPtr pEnt;
+#if XSERVER_LIBPCIACCESS
+   struct pci_device *PciInfo;
+   int mmio_bar;
+   int fb_bar;
+   int gtt_bar;
+#else
    pciVideoPtr PciInfo;
    PCITAG PciTag;
+#endif
    CARD8 variant;

    unsigned int BR[20];
"""]]

### Macros to fetch PCI device ids

Instead of changing the code to have conditional use of these new structures everywhere, we create macros to fetch various PCI identification data from either the old or new structures. Now, everywhere the driver used to refer to one of the old fields, just replace that using one of these macros. Placing these macros in your header file keeps the driver building against older servers.


[[!format txt """
#if XSERVER_LIBPCIACCESS
+#define I810_MEMBASE(p,n) (p)->regions[(n)].base_addr
+#define VENDOR_ID(p)      (p)->vendor_id
+#define DEVICE_ID(p)      (p)->device_id
+#define SUBVENDOR_ID(p)          (p)->subvendor_id
+#define SUBSYS_ID(p)      (p)->subdevice_id
+#define CHIP_REVISION(p)  (p)->revision
+#else
+#define I810_MEMBASE(p,n) (p)->memBase[n]
+#define VENDOR_ID(p)      (p)->vendor
+#define DEVICE_ID(p)      (p)->chipType
+#define SUBVENDOR_ID(p)          (p)->subsysVendor
+#define SUBSYS_ID(p)      (p)->subsysCard
+#define CHIP_REVISION(p)  (p)->chipRev
+#endif
"""]]

### DRI interface changes

This change is simple; DRI needs to know the PCI address of the video card in several places, as these values are now stored in a new structure, the code must change to reflect the new names.


[[!format txt """
   } else {
       pDRIInfo->busIdString = xalloc(64);
       sprintf(pDRIInfo->busIdString, "PCI:%d:%d:%d",
+#if XSERVER_LIBPCIACCESS
+             ((pI810->PciInfo->domain << 8) | pI810->PciInfo->bus),
+             pI810->PciInfo->dev, pI810->PciInfo->func
+#else
              ((pciConfigPtr) pI810->PciInfo->thisCard)->busnum,
              ((pciConfigPtr) pI810->PciInfo->thisCard)->devnum,
-             ((pciConfigPtr) pI810->PciInfo->thisCard)->funcnum);
+             ((pciConfigPtr) pI810->PciInfo->thisCard)->funcnum
+#endif
+             );
    }
    pDRIInfo->ddxDriverMajorVersion = I810_MAJOR_VERSION;
    pDRIInfo->ddxDriverMinorVersion = I810_MINOR_VERSION;
@@ -972,12 +978,20 @@ I810DRIScreenInit(ScreenPtr pScreen)

    if (!pI810DRI->irq) {
       pI810DRI->irq = drmGetInterruptFromBusID(pI810->drmSubFD,
+#if XSERVER_LIBPCIACCESS
+                                              ((pI810->PciInfo->domain << 8) |
+                                               pI810->PciInfo->bus),
+                                              pI810->PciInfo->dev,
+                                              pI810->PciInfo->func
+#else
                                               ((pciConfigPtr) pI810->
                                                PciInfo->thisCard)->busnum,
                                               ((pciConfigPtr) pI810->
                                                PciInfo->thisCard)->devnum,
                                               ((pciConfigPtr) pI810->
-                                               PciInfo->thisCard)->funcnum);
+                                               PciInfo->thisCard)->funcnum
+#endif
+                                              );
       if ((drmCtlInstHandler(pI810->drmSubFD, pI810DRI->irq)) != 0) {
         xf86DrvMsg(pScrn->scrnIndex, X_INFO,
                    "[drm] failure adding irq handler, there is a device "
@@ -991,7 +1005,7 @@ I810DRIScreenInit(ScreenPtr pScreen)
    xf86DrvMsg(pScrn->scrnIndex, X_INFO,
              "[drm] dma control initialized, using IRQ %d\n", pI810DRI->irq);

-   pI810DRI->deviceID = pI810->PciInfo->chipType;
+   pI810DRI->deviceID = DEVICE_ID(pI810->PciInfo);
    pI810DRI->width = pScrn->virtualX;
    pI810DRI->height = pScrn->virtualY;
    pI810DRI->mem = pScrn->videoRam * 1024;
"""]]

### Mechanical API conversions

Where your driver code calls various pci access functions, you need to mechanically convert them to using functions from libpciaccess
[[!table header="no" class="mointable" data="""
Mapping built-in PCI functions to libpciaccess functions|||
 Built-in  |  libpciaccess
 xf86ReadPciBIOS  |  pci_device_read_rom
 pciReadByte  |  pci_device_cfg_read_u8
 pciReadWord  |  pci_device_cfg_read_u16
 pciReadLong  |  pci_device_cfg_read_u32
"""]]


### Listing the supported devices

The device probing function will now relies on the libpciaccess code to identify all devices automatically. The match works much like the kernel; you fill in a description of which devices your driver supports and the library finds the available devices on the system.

Here's a list of all of the devices supported by the intel driver:


[[!format txt """
+#if XSERVER_LIBPCIACCESS
+
+#define INTEL_DEVICE_MATCH(d,i) \
+    { 0x8086, (d), PCI_MATCH_ANY, PCI_MATCH_ANY, 0, 0, (i) }
+
+static const struct pci_id_match intel_device_match[] = {
+#ifndef I830_ONLY
+   INTEL_DEVICE_MATCH (PCI_CHIP_I810, 0 ),
+   INTEL_DEVICE_MATCH (PCI_CHIP_I810_DC100, 0 ),
+   INTEL_DEVICE_MATCH (PCI_CHIP_I810_E, 0 ),
+   INTEL_DEVICE_MATCH (PCI_CHIP_I815, 0 ),
+#endif
+   INTEL_DEVICE_MATCH (PCI_CHIP_I830_M, 0 ),
+   INTEL_DEVICE_MATCH (PCI_CHIP_845_G, 0 ),
+   INTEL_DEVICE_MATCH (PCI_CHIP_I855_GM, 0 ),
+   INTEL_DEVICE_MATCH (PCI_CHIP_I865_G, 0 ),
+   INTEL_DEVICE_MATCH (PCI_CHIP_I915_G, 0 ),
+   INTEL_DEVICE_MATCH (PCI_CHIP_E7221_G, 0 ),
+   INTEL_DEVICE_MATCH (PCI_CHIP_I915_GM, 0 ),
+   INTEL_DEVICE_MATCH (PCI_CHIP_I945_G, 0 ),
+   INTEL_DEVICE_MATCH (PCI_CHIP_I945_GM, 0 ),
+   INTEL_DEVICE_MATCH (PCI_CHIP_I945_GME, 0 ),
+   INTEL_DEVICE_MATCH (PCI_CHIP_I965_G, 0 ),
+   INTEL_DEVICE_MATCH (PCI_CHIP_I965_G_1, 0 ),
+   INTEL_DEVICE_MATCH (PCI_CHIP_I965_Q, 0 ),
+   INTEL_DEVICE_MATCH (PCI_CHIP_I946_GZ, 0 ),
+   INTEL_DEVICE_MATCH (PCI_CHIP_I965_GM, 0 ),
+   INTEL_DEVICE_MATCH (PCI_CHIP_I965_GME, 0 ),
+   INTEL_DEVICE_MATCH (PCI_CHIP_G33_G, 0 ),
+   INTEL_DEVICE_MATCH (PCI_CHIP_Q35_G, 0 ),
+   INTEL_DEVICE_MATCH (PCI_CHIP_Q33_G, 0 ),
+    { 0, 0, 0 },
+};
+
+#endif /* XSERVER_LIBPCIACCESS */
"""]]

### DriverRec changes

The exported `DriverRec` has some new fields that refer to the above list of devices and a new libpciaccess-compatible probe function. Note that the structure places NULL for the old Probe field while listing the new probe function in the new slot.


[[!format txt """
_X_EXPORT DriverRec I810 = {
    I810_VERSION,
    I810_DRIVER_NAME,
    I810Identify,
+#if XSERVER_LIBPCIACCESS
+   NULL,
+#else
    I810Probe,
+#endif
    I810AvailableOptions,
    NULL,
-   0
+   0,
+   NULL,
+#if XSERVER_LIBPCIACCESS
+   intel_device_match,
+   intel_pci_probe
+#endif
 };
"""]]

### Registering the driver with the system

In your setup function, you'll be adding a driver to the system. To make the server know that your `DriverRec` contains the new fields for libpciaccess, you need to pass the `HaveDriverFuncs` or the server will never call your new probe function:


[[!format txt """
@@ -429,7 +483,13 @@ i810Setup(pointer module, pointer opts, int *errmaj, int *errmin)
     */
    if (!setupDone) {
       setupDone = 1;
-      xf86AddDriver(&I810, module, 0);
+      xf86AddDriver(&I810, module,
+#if XSERVER_LIBPCIACCESS
+                   HaveDriverFuncs
+#else
+                   0
+#endif
+                   );
"""]]

### The new probe function

The hardest part of the conversion is building a new probe function. This will replace your existing probe function when the server uses libpciaccess as the two mechanisms are incompatible.

Here's the new intel probe function


[[!format txt """
/*
+ * intel_pci_probe --
+ *
+ * Look through the PCI bus to find cards that are intel boards.
+ * Setup the dispatch table for the rest of the driver functions.
+ *
+ */
+static Bool intel_pci_probe (DriverPtr         driver,
+                            int                entity_num,
+                            struct pci_device  *device,
+                            intptr_t           match_data)
+{
+    ScrnInfoPtr            scrn = NULL;
+    EntityInfoPtr   entity;
+    I830EntPtr     i830_ent = NULL;
+    DevUnion       *private;
+
+    scrn = xf86ConfigPciEntity (scrn, 0, entity_num, I810PciChipsets,
+                               NULL,
+                               NULL, NULL, NULL, NULL);
+    if (scrn != NULL)
+    {
+       scrn->driverVersion = I810_VERSION;
+       scrn->driverName = I810_DRIVER_NAME;
+       scrn->name = I810_NAME;
+       scrn->Probe = NULL;
+
+       entity = xf86GetEntityInfo (entity_num);
+
+       switch (DEVICE_ID(device)) {
+#ifndef I830_ONLY
+       case PCI_CHIP_I810:
+       case PCI_CHIP_I810_DC100:
+       case PCI_CHIP_I810_E:
+       case PCI_CHIP_I815:
+           scrn->PreInit = I810PreInit;
+           scrn->ScreenInit = I810ScreenInit;
+           scrn->SwitchMode = I810SwitchMode;
+           scrn->AdjustFrame = I810AdjustFrame;
+           scrn->EnterVT = I810EnterVT;
+           scrn->LeaveVT = I810LeaveVT;
+           scrn->FreeScreen = I810FreeScreen;
+           scrn->ValidMode = I810ValidMode;
+           break;
+#endif
+       case PCI_CHIP_845_G:
+       case PCI_CHIP_I865_G:
+           /*
+            * These two chips have only one pipe, and
+            * cannot do dual-head
+            */
+           I830InitpScrn(scrn);
+           break;
+       default:
+           /*
+            * Everything else is an i830-ish dual-pipe chip
+            */
+           xf86SetEntitySharable(entity_num);
+
+           /* Allocate an entity private if necessary */
+           if (I830EntityIndex < 0)
+               I830EntityIndex = xf86AllocateEntityPrivateIndex();
+
+           private = xf86GetEntityPrivate(scrn->entityList[0],
+                                          I830EntityIndex);
+           i830_ent = private->ptr;
+           if (!i830_ent)
+           {
+               private->ptr = xnfcalloc(sizeof(I830EntRec), 1);
+               i830_ent = private->ptr;
+               i830_ent->lastInstance = -1;
+           }
+
+           /*
+            * Set the entity instance for this instance of the driver.
+            * For dual head per card, instance 0 is the "master"
+            * instance, driving the primary head, and instance 1 is
+            * the "slave".
+            */
+           i830_ent->lastInstance++;
+           xf86SetEntityInstanceForScreen(scrn,
+                                          scrn->entityList[0],
+                                          i830_ent->lastInstance);
+           I830InitpScrn(scrn);
+           break;
+       }
+    }
+    return scrn != NULL;
+}
"""]]

### Mapping device regions

The final changes here affect how the driver maps bus regions into the server address space. This is a fairly mechanical change, except that the driver must know which BAR refers to each portion of the card address space.

Here's a sample patch that maps the intel MMIO region:


[[!format txt """
 {
    int mmioFlags;
    I810Ptr pI810 = I810PTR(pScrn);
+#if XSERVER_LIBPCIACCESS
+   struct pci_device *const device = pI810->PciInfo;
+   int err;
+#endif

 #if !defined(__alpha__)
    mmioFlags = VIDMEM_MMIO | VIDMEM_READSIDEEFFECT;
@@ -1211,11 +1408,23 @@ I810MapMMIO(ScrnInfoPtr pScrn)
    mmioFlags = VIDMEM_MMIO | VIDMEM_READSIDEEFFECT | VIDMEM_SPARSE;
 #endif

+#if XSERVER_LIBPCIACCESS
+   err = pci_device_map_region (device, pI810->mmio_bar, TRUE);
+   if (err)
+   {
+      xf86DrvMsg (pScrn->scrnIndex, X_ERROR,
+                 "Unable to map mmio BAR. %s (%d)\n",
+                 strerror (err), err);
+      return FALSE;
+   }
+   pI810->MMIOBase = device->regions[pI810->mmio_bar].memory;
+#else
    pI810->MMIOBase = xf86MapPciMem(pScrn->scrnIndex, mmioFlags,
                                   pI810->PciTag,
                                   pI810->MMIOAddr, I810_REG_SIZE);
    if (!pI810->MMIOBase)
       return FALSE;
+#endif
    return TRUE;
 }
"""]]
Unmapping requires similar changes:


[[!format txt """
@ -1246,8 +1472,12 @@ I810UnmapMMIO(ScrnInfoPtr pScrn)
 {
    I810Ptr pI810 = I810PTR(pScrn);

+#if XSERVER_LIBPCIACCESS
+   pci_device_unmap_region (pI810->PciInfo, pI810->mmio_bar);
+#else
    xf86UnMapVidMem(pScrn->scrnIndex, (pointer) pI810->MMIOBase,
                   I810_REG_SIZE);
+#endif
    pI810->MMIOBase = NULL;
 }
"""]]

### That's it

The steps above are all that I found necessary to convert the intel driver; of course there are lots of other changes than the short patch excerpts here, but they're all available by looking at the intel driver git log.