[PATCH] xf86VGAarbiter,vgaHW: Only wrap co-operating VGA drivers
Chris Wilson
chris at chris-wilson.co.uk
Thu Sep 12 04:37:44 PDT 2013
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 <chris at chris-wilson.co.uk>
Cc: Dave Airlie <airlied at redhat.com>
Cc: Tiago Vignatti <tiago.vignatti at intel.com>
---
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
More information about the xorg-devel
mailing list