Fix mach64 libpciaccess conversion issues

Mark Kettenis mark.kettenis at xs4all.nl
Mon May 4 13:37:50 PDT 2009


[ Apologies for people on the CC.  I'm afraid I sent the first copy of
  this mail to the wrong list. ]

While trying to fix some issues with the mach64 driver on
OpenBSD/sparc64, I discovered that the libpciaccess conversion borked
things slightly.

The mach64 driver uses chip IDs that cover multiple PCI device IDs, so
there is no 1:1 correspondence between them.  This causes problems,
because the libpciaccess-enabled xserver code expects such a 1:1
correspondence.  As a result the chip ID in the "entity" no longer
matches what the driver expects.  Since this chip ID is used to do
some early initialization the driver will make the wrong choice for
some of the supported hardware.  On top of that, like Tiago Vignatti
noticed, the resources aren't correctly reported.

The diff below fixes things by ditching Mach64PciChipsets in the
XSERVER_LIBPCIACCESS case, and using the "match data" member of struct
pci_id_match to pass the chip ID.  This is how for example the
radeonhd driver does things.  Since I was touching this code, I
reverted some of the style inconsistencies introduced by the
libpciaccess conversion.  I hope people don't mind.

Mark

P.S. My whole motiviation for this diff is that I have another diff
that sets the reference_clock option to a sane default for Rage XL and
Mobility cards found on sun4u machines.


Index: src/atimach64probe.c
===================================================================
RCS file: /cvs/xenocara/driver/xf86-video-mach64/src/atimach64probe.c,v
retrieving revision 1.1.1.1
diff -u -p -r1.1.1.1 atimach64probe.c
--- src/atimach64probe.c	12 Jul 2008 15:43:34 -0000	1.1.1.1
+++ src/atimach64probe.c	3 May 2009 11:56:10 -0000
@@ -68,6 +68,8 @@ Mach64Chipsets[] = {
     {-1,      NULL }
 };
 
+#ifndef XSERVER_LIBPCIACCESS
+
 /*
  * This table maps a PCI device ID to a chipset family identifier.
  */
@@ -110,43 +112,43 @@ Mach64PciChipsets[] = {
     {-1, -1, RES_UNDEFINED}
 };
 
-#ifdef XSERVER_LIBPCIACCESS
+#else /* XSERVER_LIBPCIACCESS */
 
-static const struct pci_id_match mach64_device_match[] = {
-    ATI_DEVICE_MATCH( PCI_CHIP_MACH64GX, 0 ),
-    ATI_DEVICE_MATCH( PCI_CHIP_MACH64CX, 0 ),
-    ATI_DEVICE_MATCH( PCI_CHIP_MACH64CT, 0 ),
-    ATI_DEVICE_MATCH( PCI_CHIP_MACH64ET, 0 ),
-    ATI_DEVICE_MATCH( PCI_CHIP_MACH64VT, 0 ),
-    ATI_DEVICE_MATCH( PCI_CHIP_MACH64GT, 0 ),
-    ATI_DEVICE_MATCH( PCI_CHIP_MACH64VU, 0 ),
-    ATI_DEVICE_MATCH( PCI_CHIP_MACH64GU, 0 ),
-    ATI_DEVICE_MATCH( PCI_CHIP_MACH64LG, 0 ),
-    ATI_DEVICE_MATCH( PCI_CHIP_MACH64VV, 0 ),
-    ATI_DEVICE_MATCH( PCI_CHIP_MACH64GV, 0 ),
-    ATI_DEVICE_MATCH( PCI_CHIP_MACH64GW, 0 ),
-    ATI_DEVICE_MATCH( PCI_CHIP_MACH64GY, 0 ),
-    ATI_DEVICE_MATCH( PCI_CHIP_MACH64GZ, 0 ),
-    ATI_DEVICE_MATCH( PCI_CHIP_MACH64GB, 0 ),
-    ATI_DEVICE_MATCH( PCI_CHIP_MACH64GD, 0 ),
-    ATI_DEVICE_MATCH( PCI_CHIP_MACH64GI, 0 ),
-    ATI_DEVICE_MATCH( PCI_CHIP_MACH64GP, 0 ),
-    ATI_DEVICE_MATCH( PCI_CHIP_MACH64GQ, 0 ),
-    ATI_DEVICE_MATCH( PCI_CHIP_MACH64LB, 0 ),
-    ATI_DEVICE_MATCH( PCI_CHIP_MACH64LD, 0 ),
-    ATI_DEVICE_MATCH( PCI_CHIP_MACH64LI, 0 ),
-    ATI_DEVICE_MATCH( PCI_CHIP_MACH64LP, 0 ),
-    ATI_DEVICE_MATCH( PCI_CHIP_MACH64LQ, 0 ),
-    ATI_DEVICE_MATCH( PCI_CHIP_MACH64GL, 0 ),
-    ATI_DEVICE_MATCH( PCI_CHIP_MACH64GM, 0 ),
-    ATI_DEVICE_MATCH( PCI_CHIP_MACH64GN, 0 ),
-    ATI_DEVICE_MATCH( PCI_CHIP_MACH64GO, 0 ),
-    ATI_DEVICE_MATCH( PCI_CHIP_MACH64GR, 0 ),
-    ATI_DEVICE_MATCH( PCI_CHIP_MACH64GS, 0 ),
-    ATI_DEVICE_MATCH( PCI_CHIP_MACH64LM, 0 ),
-    ATI_DEVICE_MATCH( PCI_CHIP_MACH64LN, 0 ),
-    ATI_DEVICE_MATCH( PCI_CHIP_MACH64LR, 0 ),
-    ATI_DEVICE_MATCH( PCI_CHIP_MACH64LS, 0 ),
+static const struct pci_id_match Mach64DeviceMatch[] = {
+    ATI_DEVICE_MATCH(PCI_CHIP_MACH64GX, ATI_CHIP_88800GX),
+    ATI_DEVICE_MATCH(PCI_CHIP_MACH64CX, ATI_CHIP_88800CX),
+    ATI_DEVICE_MATCH(PCI_CHIP_MACH64CT, ATI_CHIP_264CT),
+    ATI_DEVICE_MATCH(PCI_CHIP_MACH64ET, ATI_CHIP_264ET),
+    ATI_DEVICE_MATCH(PCI_CHIP_MACH64VT, ATI_CHIP_264VT),
+    ATI_DEVICE_MATCH(PCI_CHIP_MACH64GT, ATI_CHIP_264GT),
+    ATI_DEVICE_MATCH(PCI_CHIP_MACH64VU, ATI_CHIP_264VT3),
+    ATI_DEVICE_MATCH(PCI_CHIP_MACH64GU, ATI_CHIP_264GTDVD),
+    ATI_DEVICE_MATCH(PCI_CHIP_MACH64LG, ATI_CHIP_264LT),
+    ATI_DEVICE_MATCH(PCI_CHIP_MACH64VV, ATI_CHIP_264VT4),
+    ATI_DEVICE_MATCH(PCI_CHIP_MACH64GV, ATI_CHIP_264GT2C),
+    ATI_DEVICE_MATCH(PCI_CHIP_MACH64GW, ATI_CHIP_264GT2C),
+    ATI_DEVICE_MATCH(PCI_CHIP_MACH64GY, ATI_CHIP_264GT2C),
+    ATI_DEVICE_MATCH(PCI_CHIP_MACH64GZ, ATI_CHIP_264GT2C),
+    ATI_DEVICE_MATCH(PCI_CHIP_MACH64GB, ATI_CHIP_264GTPRO),
+    ATI_DEVICE_MATCH(PCI_CHIP_MACH64GD, ATI_CHIP_264GTPRO),
+    ATI_DEVICE_MATCH(PCI_CHIP_MACH64GI, ATI_CHIP_264GTPRO),
+    ATI_DEVICE_MATCH(PCI_CHIP_MACH64GP, ATI_CHIP_264GTPRO),
+    ATI_DEVICE_MATCH(PCI_CHIP_MACH64GQ, ATI_CHIP_264GTPRO),
+    ATI_DEVICE_MATCH(PCI_CHIP_MACH64LB, ATI_CHIP_264LTPRO),
+    ATI_DEVICE_MATCH(PCI_CHIP_MACH64LD, ATI_CHIP_264LTPRO),
+    ATI_DEVICE_MATCH(PCI_CHIP_MACH64LI, ATI_CHIP_264LTPRO),
+    ATI_DEVICE_MATCH(PCI_CHIP_MACH64LP, ATI_CHIP_264LTPRO),
+    ATI_DEVICE_MATCH(PCI_CHIP_MACH64LQ, ATI_CHIP_264LTPRO),
+    ATI_DEVICE_MATCH(PCI_CHIP_MACH64GL, ATI_CHIP_264XL),
+    ATI_DEVICE_MATCH(PCI_CHIP_MACH64GM, ATI_CHIP_264XL),
+    ATI_DEVICE_MATCH(PCI_CHIP_MACH64GN, ATI_CHIP_264XL),
+    ATI_DEVICE_MATCH(PCI_CHIP_MACH64GO, ATI_CHIP_264XL),
+    ATI_DEVICE_MATCH(PCI_CHIP_MACH64GR, ATI_CHIP_264XL),
+    ATI_DEVICE_MATCH(PCI_CHIP_MACH64GS, ATI_CHIP_264XL),
+    ATI_DEVICE_MATCH(PCI_CHIP_MACH64LM, ATI_CHIP_MOBILITY),
+    ATI_DEVICE_MATCH(PCI_CHIP_MACH64LN, ATI_CHIP_MOBILITY),
+    ATI_DEVICE_MATCH(PCI_CHIP_MACH64LR, ATI_CHIP_MOBILITY),
+    ATI_DEVICE_MATCH(PCI_CHIP_MACH64LS, ATI_CHIP_MOBILITY),
     { 0, 0, 0 }
 };
 
@@ -173,37 +175,6 @@ Mach64Identify
             "Driver for ATI Mach64 chipsets");
 }
 
-static Bool
-mach64_get_scrninfo(int entity_num)
-{
-    ScrnInfoPtr pScrn;
-
-    pScrn = xf86ConfigPciEntity(NULL, 0, entity_num, Mach64PciChipsets,
-                                0, 0, 0, 0, NULL);
-
-    if (!pScrn)
-        return FALSE;
-
-    pScrn->driverVersion = MACH64_VERSION_CURRENT;
-    pScrn->driverName    = MACH64_DRIVER_NAME;
-    pScrn->name          = MACH64_NAME;
-#ifdef XSERVER_LIBPCIACCESS
-    pScrn->Probe         = NULL;
-#else
-    pScrn->Probe         = Mach64Probe;
-#endif
-    pScrn->PreInit       = ATIPreInit;
-    pScrn->ScreenInit    = ATIScreenInit;
-    pScrn->SwitchMode    = ATISwitchMode;
-    pScrn->AdjustFrame   = ATIAdjustFrame;
-    pScrn->EnterVT       = ATIEnterVT;
-    pScrn->LeaveVT       = ATILeaveVT;
-    pScrn->FreeScreen    = ATIFreeScreen;
-    pScrn->ValidMode     = ATIValidMode;
-
-    return TRUE;
-}
-
 #ifndef XSERVER_LIBPCIACCESS
 
 /*
@@ -215,6 +186,7 @@ mach64_get_scrninfo(int entity_num)
 static Bool
 Mach64Probe(DriverPtr pDriver, int flags)
 {
+    ScrnInfoPtr pScrn;
     GDevPtr *devSections;
     int     *usedChips;
     int     numDevSections;
@@ -243,8 +215,26 @@ Mach64Probe(DriverPtr pDriver, int flags
         ProbeSuccess = TRUE;
     } else {
         for (i = 0; i < numUsed; i++) {
-            if (mach64_get_scrninfo(usedChips[i]))
-                ProbeSuccess = TRUE;
+	    pScrn = xf86ConfigPciEntity(NULL, 0, usedChips[i],
+					Mach64PciChipsets, 0, 0, 0, 0, NULL);
+
+	    if (!pScrn)
+		continue;
+
+	    pScrn->driverVersion = MACH64_VERSION_CURRENT;
+	    pScrn->driverName    = MACH64_DRIVER_NAME;
+	    pScrn->name          = MACH64_NAME;
+	    pScrn->Probe         = Mach64Probe;
+	    pScrn->PreInit       = ATIPreInit;
+	    pScrn->ScreenInit    = ATIScreenInit;
+	    pScrn->SwitchMode    = ATISwitchMode;
+	    pScrn->AdjustFrame   = ATIAdjustFrame;
+	    pScrn->EnterVT       = ATIEnterVT;
+	    pScrn->LeaveVT       = ATILeaveVT;
+	    pScrn->FreeScreen    = ATIFreeScreen;
+	    pScrn->ValidMode     = ATIValidMode;
+
+	    ProbeSuccess = TRUE;
         }
     }
 
@@ -256,14 +246,42 @@ Mach64Probe(DriverPtr pDriver, int flags
 #else /* XSERVER_LIBPCIACCESS */
 
 static Bool
-mach64_pci_probe(
+Mach64PciProbe(
     DriverPtr          pDriver,
-    int                entity_num,
-    struct pci_device *device,
-    intptr_t           match_data
+    int                entityNum,
+    struct pci_device *dev,
+    intptr_t           matchData
 )
 {
-    return mach64_get_scrninfo(entity_num);
+    ScrnInfoPtr pScrn;
+    ATIPtr pATI;
+
+    pScrn = xf86ConfigPciEntity(NULL, 0, entityNum, NULL,
+                                RES_SHARED_VGA, NULL, NULL, NULL, NULL);
+
+    if (!pScrn)
+        return FALSE;
+
+    pScrn->driverVersion = MACH64_VERSION_CURRENT;
+    pScrn->driverName    = MACH64_DRIVER_NAME;
+    pScrn->name          = MACH64_NAME;
+    pScrn->Probe         = NULL;
+    pScrn->PreInit       = ATIPreInit;
+    pScrn->ScreenInit    = ATIScreenInit;
+    pScrn->SwitchMode    = ATISwitchMode;
+    pScrn->AdjustFrame   = ATIAdjustFrame;
+    pScrn->EnterVT       = ATIEnterVT;
+    pScrn->LeaveVT       = ATILeaveVT;
+    pScrn->FreeScreen    = ATIFreeScreen;
+    pScrn->ValidMode     = ATIValidMode;
+
+    if (!ATIGetRec(pScrn))
+	return FALSE;
+
+    pATI = ATIPTR(pScrn);
+    pATI->Chip = matchData;
+
+    return TRUE;
 }
 
 #endif /* XSERVER_LIBPCIACCESS */
@@ -283,7 +301,7 @@ _X_EXPORT DriverRec MACH64 =
     0,
     NULL,
 #ifdef XSERVER_LIBPCIACCESS
-    mach64_device_match,
-    mach64_pci_probe
+    Mach64DeviceMatch,
+    Mach64PciProbe
 #endif
 };
Index: src/atipreinit.c
===================================================================
RCS file: /cvs/xenocara/driver/xf86-video-mach64/src/atipreinit.c,v
retrieving revision 1.1.1.1
diff -u -p -r1.1.1.1 atipreinit.c
--- src/atipreinit.c	12 Jul 2008 15:43:32 -0000	1.1.1.1
+++ src/atipreinit.c	3 May 2009 11:56:11 -0000
@@ -57,8 +57,8 @@
 /*
  * FreeScreen handles the clean-up.
  */
-static Bool
-Mach64GetRec(ScrnInfoPtr pScrn)
+Bool
+ATIGetRec(ScrnInfoPtr pScrn)
 {
     if (!pScrn->driverPrivate) {
         pScrn->driverPrivate = xnfcalloc(sizeof(ATIRec), 1);
@@ -540,7 +540,7 @@ ATIPreInit
         return FALSE;
     }
 
-    if (!Mach64GetRec(pScreenInfo))
+    if (!ATIGetRec(pScreenInfo))
         return FALSE;
 
     pATI = ATIPTR(pScreenInfo);
@@ -551,7 +551,9 @@ ATIPreInit
     pResources = pEntity->resources;
 
     pATI->iEntity = pEntity->index;
+#ifndef XSERVER_LIBPCIACCESS
     pATI->Chip = pEntity->chipset;
+#endif
     pVideo = xf86GetPciInfoForEntity(pATI->iEntity);
 
     xfree(pEntity);
Index: src/atipreinit.h
===================================================================
RCS file: /cvs/xenocara/driver/xf86-video-mach64/src/atipreinit.h,v
retrieving revision 1.1.1.1
diff -u -p -r1.1.1.1 atipreinit.h
--- src/atipreinit.h	12 Jul 2008 15:43:34 -0000	1.1.1.1
+++ src/atipreinit.h	3 May 2009 11:56:11 -0000
@@ -25,6 +25,7 @@
 
 #include "xf86str.h"
 
+extern Bool ATIGetRec(ScrnInfoPtr);
 extern Bool ATIPreInit(ScrnInfoPtr, int);
 
 #endif /* ___ATIPREINIT_H___ */


More information about the xorg-devel mailing list