Presently, we wrap every single operation on every driver if the kernel reports that there is more than one VGA capable device in the system. This is irrespective of whether VGA is being used by any driver, and causes a significant performance impact (4-5x) for CPU bound operations.
The approach taken in this patch is to first only enable VGA arbitration for drivers that require VGA resources. This is detected by moving the initialisation from the common xf86 code to the vgaHW module. This is strictly an ABI break as any driver that directly uses VGA (i.e. without using the vgaHW module) will need to make its own declaration of intent. Secondly, we then only wrap the operations with vgaarb get/put for the drivers that require VGA access. If we only have a single driver requring VGA access, we just wrap Enter/LeaveVT and lock the VGA arbiter for the entire duration that the Xserver is active. Signed-off-by: Chris Wilson <[email protected]> Cc: Dave Airlie <[email protected]> Cc: Tiago Vignatti <[email protected]> --- hw/xfree86/common/xf86.h | 2 - hw/xfree86/common/xf86Bus.c | 2 - hw/xfree86/common/xf86Init.c | 8 --- hw/xfree86/common/xf86VGAarbiter.c | 129 ++++++++++++++++++++++--------------- hw/xfree86/common/xf86VGAarbiter.h | 9 ++- hw/xfree86/vgahw/vgaHW.c | 5 ++ 6 files changed, 88 insertions(+), 67 deletions(-) diff --git a/hw/xfree86/common/xf86.h b/hw/xfree86/common/xf86.h index 828d958..4831333 100644 --- a/hw/xfree86/common/xf86.h +++ b/hw/xfree86/common/xf86.h @@ -138,8 +138,6 @@ extern _X_EXPORT Bool xf86ConfigActivePciEntity(ScrnInfoPtr pScrn, EntityProc leave, pointer private); #else -#define xf86VGAarbiterInit() do {} while (0) -#define xf86VGAarbiterFini() do {} while (0) #define xf86VGAarbiterLock(x) do {} while (0) #define xf86VGAarbiterUnlock(x) do {} while (0) #define xf86VGAarbiterScrnInit(x) do {} while (0) diff --git a/hw/xfree86/common/xf86Bus.c b/hw/xfree86/common/xf86Bus.c index e101537..2678ea4 100644 --- a/hw/xfree86/common/xf86Bus.c +++ b/hw/xfree86/common/xf86Bus.c @@ -132,8 +132,6 @@ xf86BusConfig(void) return FALSE; } - xf86VGAarbiterInit(); - /* * Match up the screens found by the probes against those specified * in the config file. Remove the ones that won't be used. Sort diff --git a/hw/xfree86/common/xf86Init.c b/hw/xfree86/common/xf86Init.c index 91ec4c8..82bbc6d 100644 --- a/hw/xfree86/common/xf86Init.c +++ b/hw/xfree86/common/xf86Init.c @@ -588,24 +588,18 @@ InitOutput(ScreenInfo * pScreenInfo, int argc, char **argv) */ for (i = 0; i < xf86NumScreens; i++) { - xf86VGAarbiterScrnInit(xf86Screens[i]); - xf86VGAarbiterLock(xf86Screens[i]); if (xf86Screens[i]->PreInit && xf86Screens[i]->PreInit(xf86Screens[i], 0)) xf86Screens[i]->configured = TRUE; - xf86VGAarbiterUnlock(xf86Screens[i]); } for (i = 0; i < xf86NumScreens; i++) if (!xf86Screens[i]->configured) xf86DeleteScreen(xf86Screens[i--]); for (i = 0; i < xf86NumGPUScreens; i++) { - xf86VGAarbiterScrnInit(xf86GPUScreens[i]); - xf86VGAarbiterLock(xf86GPUScreens[i]); if (xf86GPUScreens[i]->PreInit && xf86GPUScreens[i]->PreInit(xf86GPUScreens[i], 0)) xf86GPUScreens[i]->configured = TRUE; - xf86VGAarbiterUnlock(xf86GPUScreens[i]); } for (i = 0; i < xf86NumGPUScreens; i++) if (!xf86GPUScreens[i]->configured) @@ -1033,8 +1027,6 @@ ddxGiveUp(enum ExitCode error) { int i; - xf86VGAarbiterFini(); - #ifdef XF86PM if (xf86OSPMClose) xf86OSPMClose(); diff --git a/hw/xfree86/common/xf86VGAarbiter.c b/hw/xfree86/common/xf86VGAarbiter.c index 225fff0..1d945fc 100644 --- a/hw/xfree86/common/xf86VGAarbiter.c +++ b/hw/xfree86/common/xf86VGAarbiter.c @@ -65,30 +65,40 @@ static DevPrivateKeyRec VGAarbiterGCKeyRec; #define VGAarbiterGCKey (&VGAarbiterGCKeyRec) -static int vga_no_arb = 0; -void +static int vga_arb_count; + +static Bool xf86VGAarbiterInit(void) { - if (pci_device_vgaarb_init() != 0) { - vga_no_arb = 1; - xf86Msg(X_WARNING, - "VGA arbiter: cannot open kernel arbiter, no multi-card support\n"); + if (vga_arb_count < 0) + return FALSE; + + if (vga_arb_count++ == 0) { + if (pci_device_vgaarb_init() != 0) { + vga_arb_count = -1; + xf86Msg(X_WARNING, + "VGA arbiter: cannot open kernel arbiter, no multi-card support\n"); + } } + + return vga_arb_count > 0; } -void +static void xf86VGAarbiterFini(void) { - if (vga_no_arb) - return; + if (--vga_arb_count) + return; + pci_device_vgaarb_fini(); } void xf86VGAarbiterLock(ScrnInfoPtr pScrn) { - if (vga_no_arb) + if (vga_arb_count <= 0) return; + pci_device_vgaarb_set_target(pScrn->vgaDev); pci_device_vgaarb_lock(); } @@ -96,8 +106,9 @@ xf86VGAarbiterLock(ScrnInfoPtr pScrn) void xf86VGAarbiterUnlock(ScrnInfoPtr pScrn) { - if (vga_no_arb) + if (vga_arb_count <= 0) return; + pci_device_vgaarb_unlock(); } @@ -108,7 +119,7 @@ xf86VGAarbiterAllowDRI(ScreenPtr pScreen) int rsrc_decodes; ScrnInfoPtr pScrn = xf86ScreenToScrn(pScreen); - if (vga_no_arb) + if (vga_arb_count <= 0) return TRUE; pci_device_vgaarb_get_info(pScrn->vgaDev, &vga_count, &rsrc_decodes); @@ -120,28 +131,30 @@ xf86VGAarbiterAllowDRI(ScreenPtr pScreen) return TRUE; } -void +Bool xf86VGAarbiterScrnInit(ScrnInfoPtr pScrn) { struct pci_device *dev; EntityPtr pEnt; - if (vga_no_arb) - return; + if (!xf86VGAarbiterInit()) + return FALSE; pEnt = xf86Entities[pScrn->entityList[0]]; if (pEnt->bus.type != BUS_PCI) - return; + return FALSE; dev = pEnt->bus.id.pci; pScrn->vgaDev = dev; + return TRUE; } void xf86VGAarbiterDeviceDecodes(ScrnInfoPtr pScrn, int rsrc) { - if (vga_no_arb) + if (vga_arb_count <= 0) return; + pci_device_vgaarb_set_target(pScrn->vgaDev); pci_device_vgaarb_decodes(rsrc); } @@ -149,14 +162,9 @@ xf86VGAarbiterDeviceDecodes(ScrnInfoPtr pScrn, int rsrc) Bool xf86VGAarbiterWrapFunctions(void) { - ScrnInfoPtr pScrn; - VGAarbiterScreenPtr pScreenPriv; - miPointerScreenPtr PointPriv; - PictureScreenPtr ps; - ScreenPtr pScreen; int vga_count, i; - if (vga_no_arb) + if (vga_arb_count <= 0) return FALSE; /* @@ -171,10 +179,12 @@ xf86VGAarbiterWrapFunctions(void) vga_count); for (i = 0; i < xf86NumScreens; i++) { - pScreen = xf86Screens[i]->pScreen; - ps = GetPictureScreenIfSet(pScreen); - pScrn = xf86ScreenToScrn(pScreen); - PointPriv = dixLookupPrivate(&pScreen->devPrivates, miPointerScreenKey); + ScrnInfoPtr pScrn = xf86Screens[i]; + ScreenPtr pScreen = pScrn->pScreen; + VGAarbiterScreenPtr pScreenPriv; + + if (pScrn->vgaDev == NULL) + continue; if (!dixRegisterPrivateKey (&VGAarbiterGCKeyRec, PRIVATE_GC, sizeof(VGAarbiterGCRec))) @@ -188,32 +198,43 @@ xf86VGAarbiterWrapFunctions(void) dixSetPrivate(&pScreen->devPrivates, VGAarbiterScreenKey, pScreenPriv); - WRAP_SCREEN(CloseScreen, VGAarbiterCloseScreen); - WRAP_SCREEN(SaveScreen, VGAarbiterSaveScreen); - WRAP_SCREEN(WakeupHandler, VGAarbiterWakeupHandler); - WRAP_SCREEN(BlockHandler, VGAarbiterBlockHandler); - WRAP_SCREEN(CreateGC, VGAarbiterCreateGC); - WRAP_SCREEN(GetImage, VGAarbiterGetImage); - WRAP_SCREEN(GetSpans, VGAarbiterGetSpans); - WRAP_SCREEN(SourceValidate, VGAarbiterSourceValidate); - WRAP_SCREEN(CopyWindow, VGAarbiterCopyWindow); - WRAP_SCREEN(ClearToBackground, VGAarbiterClearToBackground); - WRAP_SCREEN(CreatePixmap, VGAarbiterCreatePixmap); - WRAP_SCREEN(StoreColors, VGAarbiterStoreColors); - WRAP_SCREEN(DisplayCursor, VGAarbiterDisplayCursor); - WRAP_SCREEN(RealizeCursor, VGAarbiterRealizeCursor); - WRAP_SCREEN(UnrealizeCursor, VGAarbiterUnrealizeCursor); - WRAP_SCREEN(RecolorCursor, VGAarbiterRecolorCursor); - WRAP_SCREEN(SetCursorPosition, VGAarbiterSetCursorPosition); - WRAP_PICT(Composite, VGAarbiterComposite); - WRAP_PICT(Glyphs, VGAarbiterGlyphs); - WRAP_PICT(CompositeRects, VGAarbiterCompositeRects); - WRAP_SCREEN_INFO(AdjustFrame, VGAarbiterAdjustFrame); - WRAP_SCREEN_INFO(SwitchMode, VGAarbiterSwitchMode); WRAP_SCREEN_INFO(EnterVT, VGAarbiterEnterVT); WRAP_SCREEN_INFO(LeaveVT, VGAarbiterLeaveVT); WRAP_SCREEN_INFO(FreeScreen, VGAarbiterFreeScreen); - WRAP_SPRITE; + + if (vga_arb_count > 1) { + PictureScreenPtr ps; + miPointerScreenPtr PointPriv; + + WRAP_SCREEN_INFO(AdjustFrame, VGAarbiterAdjustFrame); + WRAP_SCREEN_INFO(SwitchMode, VGAarbiterSwitchMode); + + WRAP_SCREEN(CloseScreen, VGAarbiterCloseScreen); + WRAP_SCREEN(SaveScreen, VGAarbiterSaveScreen); + WRAP_SCREEN(WakeupHandler, VGAarbiterWakeupHandler); + WRAP_SCREEN(BlockHandler, VGAarbiterBlockHandler); + WRAP_SCREEN(CreateGC, VGAarbiterCreateGC); + WRAP_SCREEN(GetImage, VGAarbiterGetImage); + WRAP_SCREEN(GetSpans, VGAarbiterGetSpans); + WRAP_SCREEN(SourceValidate, VGAarbiterSourceValidate); + WRAP_SCREEN(CopyWindow, VGAarbiterCopyWindow); + WRAP_SCREEN(ClearToBackground, VGAarbiterClearToBackground); + WRAP_SCREEN(CreatePixmap, VGAarbiterCreatePixmap); + WRAP_SCREEN(StoreColors, VGAarbiterStoreColors); + WRAP_SCREEN(DisplayCursor, VGAarbiterDisplayCursor); + WRAP_SCREEN(RealizeCursor, VGAarbiterRealizeCursor); + WRAP_SCREEN(UnrealizeCursor, VGAarbiterUnrealizeCursor); + WRAP_SCREEN(RecolorCursor, VGAarbiterRecolorCursor); + WRAP_SCREEN(SetCursorPosition, VGAarbiterSetCursorPosition); + + ps = GetPictureScreenIfSet(pScreen); + WRAP_PICT(Composite, VGAarbiterComposite); + WRAP_PICT(Glyphs, VGAarbiterGlyphs); + WRAP_PICT(CompositeRects, VGAarbiterCompositeRects); + + PointPriv = dixLookupPrivate(&pScreen->devPrivates, miPointerScreenKey); + WRAP_SPRITE; + } } return TRUE; @@ -503,7 +524,8 @@ VGAarbiterEnterVT(ScrnInfoPtr pScrn) val = (*pScrn->EnterVT) (pScrn); pScreenPriv->EnterVT = pScrn->EnterVT; pScrn->EnterVT = VGAarbiterEnterVT; - VGAPut(); + if (vga_arb_count > 1) + VGAPut(); return val; } @@ -515,7 +537,8 @@ VGAarbiterLeaveVT(ScrnInfoPtr pScrn) (VGAarbiterScreenPtr) dixLookupPrivate(&pScreen->devPrivates, VGAarbiterScreenKey); - VGAGet(pScreen); + if (vga_arb_count > 1) + VGAGet(pScreen); pScrn->LeaveVT = pScreenPriv->LeaveVT; (*pScreenPriv->LeaveVT) (pScrn); pScreenPriv->LeaveVT = pScrn->LeaveVT; @@ -534,6 +557,8 @@ VGAarbiterFreeScreen(ScrnInfoPtr pScrn) VGAGet(pScreen); (*pScreenPriv->FreeScreen) (pScrn); VGAPut(); + + xf86VGAarbiterFini(); } static Bool diff --git a/hw/xfree86/common/xf86VGAarbiter.h b/hw/xfree86/common/xf86VGAarbiter.h index bb52cf7..f35b547 100644 --- a/hw/xfree86/common/xf86VGAarbiter.h +++ b/hw/xfree86/common/xf86VGAarbiter.h @@ -31,9 +31,12 @@ #include "xf86.h" /* Functions */ -extern void xf86VGAarbiterInit(void); -extern void xf86VGAarbiterFini(void); -void xf86VGAarbiterScrnInit(ScrnInfoPtr pScrn); + +/* Declare that this driver uses legacy VGA commands and so + * requires VGA arbitration between multiple devices. + */ +extern _X_EXPORT Bool xf86VGAarbiterScrnInit(ScrnInfoPtr pScrn); + extern Bool xf86VGAarbiterWrapFunctions(void); extern void xf86VGAarbiterLock(ScrnInfoPtr pScrn); extern void xf86VGAarbiterUnlock(ScrnInfoPtr pScrn); diff --git a/hw/xfree86/vgahw/vgaHW.c b/hw/xfree86/vgahw/vgaHW.c index a64f4f8..7e60df3 100644 --- a/hw/xfree86/vgahw/vgaHW.c +++ b/hw/xfree86/vgahw/vgaHW.c @@ -24,6 +24,7 @@ #include "xf86.h" #include "xf86_OSproc.h" +#include "xf86VGAarbiter.h" #include "vgaHW.h" #include "compiler.h" @@ -1631,6 +1632,10 @@ vgaHWGetHWRec(ScrnInfoPtr scrp) */ if (VGAHWPTR(scrp)) return TRUE; + + if (!xf86VGAarbiterScrnInit(scrp)) + return FALSE; + hwp = VGAHWPTRLVAL(scrp) = xnfcalloc(sizeof(vgaHWRec), 1); regp = &VGAHWPTR(scrp)->ModeReg; -- 1.8.4.rc3 _______________________________________________ [email protected]: X.Org development Archives: http://lists.x.org/archives/xorg-devel Info: http://lists.x.org/mailman/listinfo/xorg-devel
