Egbert and I have been looking into the issues that are preventing a second
X server to be started for i810/830M platforms when DRI is enabled. Several
issues were found:
1. The i810 support doesn't unbind/release the agpgart module when VT
switching away, and this prevents a second X server from acquiring
it. Since the i810 driver requires agpgart access even for 2D
operation, this is prevents a second X server from running. A fix
for this is to add the unbind/release calls at LeaveVT, and
acquire/rebind at EnterVT. The attached patch does this. It also
makes a small change to the unbind code in the DRM kernel modules
to handle this.
2. The 830M support does unbind/release the agpgart module, but code
in DRM(release) called when closing a /dev/dri/* device blocks
until it can obtain the lock. Since the first server holds the
lock while switched away, the second one can never get it, so it
ends up blocking in close(). The second server opens/closes the
devices to find out whether DRI can be enabled. DRI can't be
enabled on the second server (which isn't a problem), but this
blocking behaviour is preventing it from starting up at all. I've
found that this can be solved by keeping track of whether the opener
has ever tried to acquire the lock, and not try to acquire it at
close/release when it had never previously been acquired. The patch
below implements this.
3. The drm module AGP support code keeps track of a "handle" for
allocated AGP regions. It uses the memory address returned from
the allocation for this purpose. This would normally be unique,
but for the i810 driver case where "DCACHE" memory is "allocated".
In the DCACHE case, the allocated memory is freed immediately (since
DCACHE doesn't come from the system memory pool), and the next
allocation often ends up getting the same memory address, and thus
the same "handle". This showed up as a problem when the unbind/rebind
code was added.
The user-level agpgart interfaces use a "key" value to uniquely
identify allocated AGP regions. I don't know why the DRM module
doesn't do the same, since the key is guaranteed to be unique.
The patch below changes the handle to be the "key" value.
I'm wary of making changes like this so close to the 4.3 release, but
I would like to see this problem fixed in 4.3. Does anyone see any
potential problems with the attached patch?
David
--
David Dawes
Release Engineer/Architect The XFree86 Project
www.XFree86.org/~dawes
Index: drivers/i810/i810.h
===================================================================
RCS file: /home/x-cvs/xc/programs/Xserver/hw/xfree86/drivers/i810/i810.h,v
retrieving revision 1.37
diff -u -r1.37 i810.h
--- drivers/i810/i810.h 2002/12/10 01:27:04 1.37
+++ drivers/i810/i810.h 2003/02/14 20:00:33
@@ -264,6 +264,9 @@
extern Bool I810DRIFinishScreenInit(ScreenPtr pScreen);
extern Bool I810InitDma(ScrnInfoPtr pScrn);
extern Bool I810CleanupDma(ScrnInfoPtr pScrn);
+extern Bool I810DRILeave(ScrnInfoPtr pScrn);
+extern Bool I810DRIEnter(ScrnInfoPtr pScrn);
+
#define I810PTR(p) ((I810Ptr)((p)->driverPrivate))
#define I810REGPTR(p) (&(I810PTR(p)->ModeReg))
Index: drivers/i810/i810_dri.c
===================================================================
RCS file: /home/x-cvs/xc/programs/Xserver/hw/xfree86/drivers/i810/i810_dri.c,v
retrieving revision 1.33
diff -u -r1.33 i810_dri.c
--- drivers/i810/i810_dri.c 2002/12/10 01:27:04 1.33
+++ drivers/i810/i810_dri.c 2003/02/14 20:00:33
@@ -1218,3 +1218,84 @@
pI810->AccelInfoRec->NeedToSync = TRUE;
}
+
+Bool
+I810DRILeave(ScrnInfoPtr pScrn)
+{
+ I810Ptr pI810 = I810PTR(pScrn);
+
+ if (pI810->directRenderingEnabled) {
+ if (pI810->dcacheHandle != 0)
+ if (drmAgpUnbind(pI810->drmSubFD, pI810->dcacheHandle) != 0) {
+ xf86DrvMsg(pScrn->scrnIndex, X_ERROR,"%s\n",strerror(errno));
+ return FALSE;
+ }
+ if (pI810->backHandle != 0)
+ if (drmAgpUnbind(pI810->drmSubFD, pI810->backHandle) != 0) {
+ xf86DrvMsg(pScrn->scrnIndex, X_ERROR,"%s\n",strerror(errno));
+ return FALSE;
+ }
+ if (pI810->zHandle != 0)
+ if (drmAgpUnbind(pI810->drmSubFD, pI810->zHandle) != 0) {
+ xf86DrvMsg(pScrn->scrnIndex, X_ERROR,"%s\n",strerror(errno));
+ return FALSE;
+ }
+ if (pI810->sysmemHandle != 0)
+ if (drmAgpUnbind(pI810->drmSubFD, pI810->sysmemHandle) != 0) {
+ xf86DrvMsg(pScrn->scrnIndex, X_ERROR,"%s\n",strerror(errno));
+ return FALSE;
+ }
+ if (pI810->xvmcHandle != 0)
+ if (drmAgpUnbind(pI810->drmSubFD, pI810->xvmcHandle) != 0) {
+ xf86DrvMsg(pScrn->scrnIndex, X_ERROR,"%s\n",strerror(errno));
+ return FALSE;
+ }
+ if (pI810->cursorHandle != 0)
+ if (drmAgpUnbind(pI810->drmSubFD, pI810->cursorHandle) != 0) {
+ xf86DrvMsg(pScrn->scrnIndex, X_ERROR,"%s\n",strerror(errno));
+ return FALSE;
+ }
+ if (pI810->agpAcquired == TRUE)
+ drmAgpRelease(pI810->drmSubFD);
+ pI810->agpAcquired = FALSE;
+ }
+ return TRUE;
+}
+
+Bool
+I810DRIEnter(ScrnInfoPtr pScrn)
+{
+ I810Ptr pI810 = I810PTR(pScrn);
+
+ if (pI810->directRenderingEnabled) {
+
+ if (pI810->agpAcquired == FALSE)
+ drmAgpAcquire(pI810->drmSubFD);
+ pI810->agpAcquired = TRUE;
+ if (pI810->dcacheHandle != 0)
+ if (drmAgpBind(pI810->drmSubFD, pI810->dcacheHandle,
+ pI810->DepthOffset) != 0)
+ return FALSE;
+ if (pI810->backHandle != 0)
+ if (drmAgpBind(pI810->drmSubFD, pI810->backHandle,
+ pI810->BackOffset) != 0)
+ return FALSE;
+ if (pI810->zHandle != 0)
+ if (drmAgpBind(pI810->drmSubFD, pI810->zHandle,
+ pI810->DepthOffset) != 0)
+ return FALSE;
+ if (pI810->sysmemHandle != 0)
+ if (drmAgpBind(pI810->drmSubFD, pI810->sysmemHandle,
+ 0) != 0)
+ return FALSE;
+ if (pI810->xvmcHandle != 0)
+ if (drmAgpBind(pI810->drmSubFD, pI810->xvmcHandle,
+ pI810->MC.Start) != 0)
+ return FALSE;
+ if (pI810->cursorHandle != 0)
+ if (drmAgpBind(pI810->drmSubFD, pI810->cursorHandle,
+ pI810->CursorStart) != 0)
+ return FALSE;
+ }
+ return TRUE;
+}
Index: drivers/i810/i810_driver.c
===================================================================
RCS file: /home/x-cvs/xc/programs/Xserver/hw/xfree86/drivers/i810/i810_driver.c,v
retrieving revision 1.79
diff -u -r1.79 i810_driver.c
--- drivers/i810/i810_driver.c 2003/02/14 17:12:42 1.79
+++ drivers/i810/i810_driver.c 2003/02/14 20:00:21
@@ -2218,6 +2218,9 @@
return FALSE;
#ifdef XF86DRI
+ if (!I810DRIEnter(pScrn))
+ return FALSE;
+
if (pI810->directRenderingEnabled) {
if (I810_DEBUG & DEBUG_VERBOSE_DRI)
ErrorF("calling dri unlock\n");
@@ -2260,6 +2263,11 @@
if (!I810UnbindGARTMemory(pScrn))
return;
+
+#ifdef XF86DRI
+ if (!I810DRILeave(pScrn))
+ return;
+#endif
vgaHWLock(hwp);
}
Index: os-support/linux/drm/kernel/drmP.h
===================================================================
RCS file:
/home/x-cvs/xc/programs/Xserver/hw/xfree86/os-support/linux/drm/kernel/drmP.h,v
retrieving revision 1.27
diff -u -r1.27 drmP.h
--- os-support/linux/drm/kernel/drmP.h 2003/01/29 23:00:43 1.27
+++ os-support/linux/drm/kernel/drmP.h 2003/02/14 19:53:44
@@ -418,6 +418,7 @@
struct drm_file *prev;
struct drm_device *dev;
int remove_auth_on_close;
+ unsigned long lock_count;
} drm_file_t;
@@ -484,7 +485,7 @@
#if __REALLY_HAVE_AGP
typedef struct drm_agp_mem {
- unsigned long handle;
+ int key;
agp_memory *memory;
unsigned long bound; /* address */
int pages;
Index: os-support/linux/drm/kernel/drm_agpsupport.h
===================================================================
RCS file:
/home/x-cvs/xc/programs/Xserver/hw/xfree86/os-support/linux/drm/kernel/drm_agpsupport.h,v
retrieving revision 1.7
diff -u -r1.7 drm_agpsupport.h
--- os-support/linux/drm/kernel/drm_agpsupport.h 2002/12/16 16:19:28 1.7
+++ os-support/linux/drm/kernel/drm_agpsupport.h 2003/02/14 20:01:18
@@ -147,7 +147,7 @@
return -ENOMEM;
}
- entry->handle = (unsigned long)memory->memory;
+ entry->key = memory->key;
entry->memory = memory;
entry->bound = 0;
entry->pages = pages;
@@ -156,7 +156,7 @@
if (dev->agp->memory) dev->agp->memory->prev = entry;
dev->agp->memory = entry;
- request.handle = entry->handle;
+ request.handle = entry->key;
request.physical = memory->physical;
if (copy_to_user((drm_agp_buffer_t *)arg, &request, sizeof(request))) {
@@ -175,7 +175,7 @@
drm_agp_mem_t *entry;
for (entry = dev->agp->memory; entry; entry = entry->next) {
- if (entry->handle == handle) return entry;
+ if (entry->key == handle) return entry;
}
return NULL;
}
@@ -187,6 +187,7 @@
drm_device_t *dev = priv->dev;
drm_agp_binding_t request;
drm_agp_mem_t *entry;
+ int ret;
if (!dev->agp || !dev->agp->acquired) return -EINVAL;
if (copy_from_user(&request, (drm_agp_binding_t *)arg, sizeof(request)))
@@ -194,7 +195,10 @@
if (!(entry = DRM(agp_lookup_entry)(dev, request.handle)))
return -EINVAL;
if (!entry->bound) return -EINVAL;
- return DRM(unbind_agp)(entry->memory);
+ ret = DRM(unbind_agp)(entry->memory);
+ if (ret == 0)
+ entry->bound = 0;
+ return ret;
}
int DRM(agp_bind)(struct inode *inode, struct file *filp,
Index: os-support/linux/drm/kernel/drm_drv.h
===================================================================
RCS file:
/home/x-cvs/xc/programs/Xserver/hw/xfree86/os-support/linux/drm/kernel/drm_drv.h,v
retrieving revision 1.10
diff -u -r1.10 drm_drv.h
--- os-support/linux/drm/kernel/drm_drv.h 2002/11/25 14:05:04 1.10
+++ os-support/linux/drm/kernel/drm_drv.h 2003/02/14 17:32:02
@@ -768,7 +768,7 @@
DRM_DEBUG( "pid = %d, device = 0x%lx, open_count = %d\n",
current->pid, (long)dev->device, dev->open_count );
- if ( dev->lock.hw_lock &&
+ if ( priv->lock_count && dev->lock.hw_lock &&
_DRM_LOCK_IS_HELD(dev->lock.hw_lock->lock) &&
dev->lock.pid == current->pid ) {
DRM_DEBUG( "Process %d dead, freeing lock for context %d\n",
@@ -786,7 +786,7 @@
server. */
}
#if __HAVE_RELEASE
- else if ( dev->lock.hw_lock ) {
+ else if ( priv->lock_count && dev->lock.hw_lock ) {
/* The lock is required to reclaim buffers */
DECLARE_WAITQUEUE( entry, current );
add_wait_queue( &dev->lock.lock_queue, &entry );
@@ -932,6 +932,8 @@
dev->lck_start = start = get_cycles();
#endif
+
+ ++priv->lock_count;
if ( copy_from_user( &lock, (drm_lock_t *)arg, sizeof(lock) ) )
return -EFAULT;
Index: os-support/linux/drm/kernel/drm_fops.h
===================================================================
RCS file:
/home/x-cvs/xc/programs/Xserver/hw/xfree86/os-support/linux/drm/kernel/drm_fops.h,v
retrieving revision 1.7
diff -u -r1.7 drm_fops.h
--- os-support/linux/drm/kernel/drm_fops.h 2002/11/25 14:05:04 1.7
+++ os-support/linux/drm/kernel/drm_fops.h 2003/02/14 17:32:26
@@ -57,6 +57,7 @@
priv->dev = dev;
priv->ioctl_count = 0;
priv->authenticated = capable(CAP_SYS_ADMIN);
+ priv->lock_count = 0;
down(&dev->struct_sem);
if (!dev->file_last) {
