Module Name: src
Committed By: riastradh
Date: Wed Jan 15 13:53:53 UTC 2014
Modified Files:
src/sys/external/bsd/drm2/dist/drm [riastradh-drm2]: drm_bufs.c
drm_context.c drm_stub.c
src/sys/external/bsd/drm2/dist/include/drm [riastradh-drm2]: drmP.h
src/sys/external/bsd/drm2/drm [riastradh-drm2]: drm_fops.c drm_lock.c
Log Message:
Rewrite drm locking support for NetBSD.
To generate a diff of this commit:
cvs rdiff -u -r1.1.1.1.2.13 -r1.1.1.1.2.14 \
src/sys/external/bsd/drm2/dist/drm/drm_bufs.c
cvs rdiff -u -r1.1.1.1.2.7 -r1.1.1.1.2.8 \
src/sys/external/bsd/drm2/dist/drm/drm_context.c
cvs rdiff -u -r1.1.1.1.2.10 -r1.1.1.1.2.11 \
src/sys/external/bsd/drm2/dist/drm/drm_stub.c
cvs rdiff -u -r1.1.1.1.2.53 -r1.1.1.1.2.54 \
src/sys/external/bsd/drm2/dist/include/drm/drmP.h
cvs rdiff -u -r1.1.2.6 -r1.1.2.7 src/sys/external/bsd/drm2/drm/drm_fops.c
cvs rdiff -u -r1.1.2.3 -r1.1.2.4 src/sys/external/bsd/drm2/drm/drm_lock.c
Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.
Modified files:
Index: src/sys/external/bsd/drm2/dist/drm/drm_bufs.c
diff -u src/sys/external/bsd/drm2/dist/drm/drm_bufs.c:1.1.1.1.2.13 src/sys/external/bsd/drm2/dist/drm/drm_bufs.c:1.1.1.1.2.14
--- src/sys/external/bsd/drm2/dist/drm/drm_bufs.c:1.1.1.1.2.13 Sun Sep 8 16:10:23 2013
+++ src/sys/external/bsd/drm2/dist/drm/drm_bufs.c Wed Jan 15 13:53:53 2014
@@ -39,6 +39,7 @@
#include <linux/log2.h>
#include <linux/export.h>
#include <linux/mm.h>
+#include <asm/bug.h>
#include <asm/shmparam.h>
#include <drm/drmP.h>
@@ -246,12 +247,15 @@ static int drm_addmap_core(struct drm_de
map->offset = (unsigned long)map->handle;
if (map->flags & _DRM_CONTAINS_LOCK) {
/* Prevent a 2nd X Server from creating a 2nd lock */
+ spin_lock(&dev->primary->master->lock.spinlock);
if (dev->primary->master->lock.hw_lock != NULL) {
vfree(map->handle);
kfree(map);
+ spin_unlock(&dev->primary->master->lock.spinlock);
return -EBUSY;
}
dev->sigdata.lock = dev->primary->master->lock.hw_lock = map->handle; /* Pointer to lock */
+ spin_unlock(&dev->primary->master->lock.spinlock);
}
break;
case _DRM_AGP: {
@@ -492,19 +496,36 @@ int drm_rmmap_locked(struct drm_device *
}
break;
case _DRM_SHM:
- vfree(map->handle);
if (master) {
+ spin_lock(&master->lock.spinlock);
+ /*
+ * If we successfully removed this mapping,
+ * then the mapping must have been there in the
+ * first place, and we must have had a
+ * heavyweight lock, so we assert here instead
+ * of just checking and failing.
+ *
+ * XXX What about the _DRM_CONTAINS_LOCK flag?
+ * Where is that supposed to be set? Is it
+ * equivalent to having a master set?
+ *
+ * XXX There is copypasta of this in
+ * drm_fops.c.
+ */
+ BUG_ON(master->lock.hw_lock == NULL);
if (dev->sigdata.lock == master->lock.hw_lock)
dev->sigdata.lock = NULL;
master->lock.hw_lock = NULL; /* SHM removed */
master->lock.file_priv = NULL;
#ifdef __NetBSD__
- DRM_WAKEUP_ALL(&master->lock.lock_queue,
- &drm_global_mutex);
+ DRM_SPIN_WAKEUP_ALL(&master->lock.lock_queue,
+ &master->lock.spinlock);
#else
wake_up_interruptible_all(&master->lock.lock_queue);
#endif
+ spin_unlock(&master->lock.spinlock);
}
+ vfree(map->handle);
break;
case _DRM_AGP:
case _DRM_SCATTER_GATHER:
Index: src/sys/external/bsd/drm2/dist/drm/drm_context.c
diff -u src/sys/external/bsd/drm2/dist/drm/drm_context.c:1.1.1.1.2.7 src/sys/external/bsd/drm2/dist/drm/drm_context.c:1.1.1.1.2.8
--- src/sys/external/bsd/drm2/dist/drm/drm_context.c:1.1.1.1.2.7 Sun Sep 8 16:11:29 2013
+++ src/sys/external/bsd/drm2/dist/drm/drm_context.c Wed Jan 15 13:53:53 2014
@@ -267,9 +267,12 @@ static int drm_context_switch_complete(s
dev->last_context = new; /* PRE/POST: This is the _only_ writer. */
dev->last_switch = jiffies;
- if (!_DRM_LOCK_IS_HELD(file_priv->master->lock.hw_lock->lock)) {
+ spin_lock(&file_priv->master->lock.spinlock);
+ if (file_priv->master->lock.hw_lock == NULL ||
+ !_DRM_LOCK_IS_HELD(file_priv->master->lock.hw_lock->lock)) {
DRM_ERROR("Lock isn't held after context switch\n");
}
+ spin_unlock(&file_priv->master->lock.spinlock);
/* If a context switch is ever initiated
when the kernel holds the lock, release
Index: src/sys/external/bsd/drm2/dist/drm/drm_stub.c
diff -u src/sys/external/bsd/drm2/dist/drm/drm_stub.c:1.1.1.1.2.10 src/sys/external/bsd/drm2/dist/drm/drm_stub.c:1.1.1.1.2.11
--- src/sys/external/bsd/drm2/dist/drm/drm_stub.c:1.1.1.1.2.10 Wed Jul 24 04:03:45 2013
+++ src/sys/external/bsd/drm2/dist/drm/drm_stub.c Wed Jan 15 13:53:53 2014
@@ -170,8 +170,7 @@ struct drm_master *drm_master_create(str
kref_init(&master->refcount);
spin_lock_init(&master->lock.spinlock);
#ifdef __NetBSD__
- DRM_INIT_WAITQUEUE(&master->lock.lock_queue, "drmulckq");
- DRM_INIT_WAITQUEUE(&master->lock.kernel_lock_queue, "drmklckq");
+ DRM_INIT_WAITQUEUE(&master->lock.lock_queue, "drmlockq");
#else
init_waitqueue_head(&master->lock.lock_queue);
#endif
@@ -230,7 +229,6 @@ static void drm_master_destroy(struct kr
#ifdef __NetBSD__
spin_lock_destroy(&master->lock.spinlock);
DRM_DESTROY_WAITQUEUE(&master->lock.lock_queue);
- DRM_DESTROY_WAITQUEUE(&master->lock.kernel_lock_queue);
#endif
kfree(master);
Index: src/sys/external/bsd/drm2/dist/include/drm/drmP.h
diff -u src/sys/external/bsd/drm2/dist/include/drm/drmP.h:1.1.1.1.2.53 src/sys/external/bsd/drm2/dist/include/drm/drmP.h:1.1.1.1.2.54
--- src/sys/external/bsd/drm2/dist/include/drm/drmP.h:1.1.1.1.2.53 Sun Sep 8 16:30:13 2013
+++ src/sys/external/bsd/drm2/dist/include/drm/drmP.h Wed Jan 15 13:53:53 2014
@@ -546,18 +546,11 @@ struct drm_lock_data {
#else
wait_queue_head_t lock_queue; /**< Queue of blocked processes */
#endif
-#ifndef __NetBSD__ /* XXX nothing seems to use this */
unsigned long lock_time; /**< Time of last lock in jiffies */
-#endif
spinlock_t spinlock;
-#ifdef __NetBSD__
- unsigned int n_kernel_waiters;
- drm_waitqueue_t kernel_lock_queue;
-#else
uint32_t kernel_waiters;
uint32_t user_waiters;
int idle_has_lock;
-#endif
};
/**
Index: src/sys/external/bsd/drm2/drm/drm_fops.c
diff -u src/sys/external/bsd/drm2/drm/drm_fops.c:1.1.2.6 src/sys/external/bsd/drm2/drm/drm_fops.c:1.1.2.7
--- src/sys/external/bsd/drm2/drm/drm_fops.c:1.1.2.6 Wed Jan 15 13:52:39 2014
+++ src/sys/external/bsd/drm2/drm/drm_fops.c Wed Jan 15 13:53:53 2014
@@ -1,4 +1,4 @@
-/* $NetBSD: drm_fops.c,v 1.1.2.6 2014/01/15 13:52:39 riastradh Exp $ */
+/* $NetBSD: drm_fops.c,v 1.1.2.7 2014/01/15 13:53:53 riastradh Exp $ */
/*-
* Copyright (c) 2013 The NetBSD Foundation, Inc.
@@ -30,7 +30,7 @@
*/
#include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: drm_fops.c,v 1.1.2.6 2014/01/15 13:52:39 riastradh Exp $");
+__KERNEL_RCSID(0, "$NetBSD: drm_fops.c,v 1.1.2.7 2014/01/15 13:53:53 riastradh Exp $");
#include <drm/drmP.h>
@@ -243,6 +243,11 @@ static void
drm_master_release(struct drm_file *file)
{
+ /*
+ * XXX I think this locking concept is wrong -- we need to hold
+ * file->master->lock.spinlock across the two calls to
+ * drm_i_have_hw_lock and drm_lock_free.
+ */
if (drm_i_have_hw_lock(file->minor->dev, file))
drm_lock_free(&file->master->lock,
_DRM_LOCKING_CONTEXT(file->master->lock.hw_lock->lock));
@@ -323,15 +328,17 @@ drm_close_file_master(struct drm_file *f
other_file->authenticated = 0;
}
+ spin_lock(&master->lock.spinlock);
if (master->lock.hw_lock) {
/* XXX There is copypasta of this in drm_bufs.c. */
if (dev->sigdata.lock == master->lock.hw_lock)
dev->sigdata.lock = NULL;
master->lock.hw_lock = NULL;
master->lock.file_priv = NULL;
- DRM_WAKEUP_ALL(&master->lock.lock_queue,
- &drm_global_mutex);
+ DRM_SPIN_WAKEUP_ALL(&master->lock.lock_queue,
+ &master->lock.spinlock);
}
+ spin_unlock(&master->lock.spinlock);
if (file->minor->master == file->master) {
if (dev->driver->master_drop)
Index: src/sys/external/bsd/drm2/drm/drm_lock.c
diff -u src/sys/external/bsd/drm2/drm/drm_lock.c:1.1.2.3 src/sys/external/bsd/drm2/drm/drm_lock.c:1.1.2.4
--- src/sys/external/bsd/drm2/drm/drm_lock.c:1.1.2.3 Wed Jul 24 02:38:23 2013
+++ src/sys/external/bsd/drm2/drm/drm_lock.c Wed Jan 15 13:53:53 2014
@@ -1,4 +1,4 @@
-/* $NetBSD: drm_lock.c,v 1.1.2.3 2013/07/24 02:38:23 riastradh Exp $ */
+/* $NetBSD: drm_lock.c,v 1.1.2.4 2014/01/15 13:53:53 riastradh Exp $ */
/*-
* Copyright (c) 2013 The NetBSD Foundation, Inc.
@@ -30,20 +30,23 @@
*/
/*
- * DRM lock. Each drm master has a lock to provide mutual exclusion
- * for access to something about the hardware (XXX what?). The lock
- * can be held by the kernel or by a drm file (XXX not by a process!
- * and multiple processes may share a common drm file). (XXX What
- * excludes different kthreads?) The DRM_LOCK ioctl grants the lock to
- * the userland. The kernel may need to steal this lock in order to
- * close a drm file; I believe drm_global_mutex excludes different
- * kernel threads from doing this simultaneously.
+ * DRM lock. Each drm master has a heavy-weight lock to provide mutual
+ * exclusion for access to the hardware. The lock can be held by the
+ * kernel or by a drm file; the kernel takes access only for unusual
+ * purposes, with drm_idlelock_take, mainly for idling the GPU when
+ * closing down.
*
- * XXX This description is incoherent and incomplete.
+ * The physical memory storing the lock state is shared between
+ * userland and kernel: the pointer at dev->master->lock->hw_lock is
+ * mapped into both userland and kernel address spaces. This way,
+ * userland can try to take the hardware lock without a system call,
+ * although if it fails then it will use the DRM_LOCK ioctl to block
+ * atomically until the lock is available. All this means that the
+ * kernel must use atomic_ops to manage the lock state.
*/
#include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: drm_lock.c,v 1.1.2.3 2013/07/24 02:38:23 riastradh Exp $");
+__KERNEL_RCSID(0, "$NetBSD: drm_lock.c,v 1.1.2.4 2014/01/15 13:53:53 riastradh Exp $");
#include <sys/types.h>
#include <sys/errno.h>
@@ -66,14 +69,24 @@ drm_lock(struct drm_device *dev, void *d
{
struct drm_lock *lock_request = data;
struct drm_master *master = file->master;
- bool acquired;
int error;
+ /* Sanitize the drm global mutex bollocks until we get rid of it. */
KASSERT(mutex_is_locked(&drm_global_mutex));
+ mutex_unlock(&drm_global_mutex);
/* Refuse to lock on behalf of the kernel. */
- if (lock_request->context == DRM_KERNEL_CONTEXT)
- return -EINVAL;
+ if (lock_request->context == DRM_KERNEL_CONTEXT) {
+ error = -EINVAL;
+ goto out0;
+ }
+
+ /* Refuse to set the magic bits. */
+ if (lock_request->context !=
+ _DRM_LOCKING_CONTEXT(lock_request->context)) {
+ error = -EINVAL;
+ goto out0;
+ }
/* Count it in the file and device statistics (XXX why here?). */
file->lock_count++;
@@ -81,13 +94,21 @@ drm_lock(struct drm_device *dev, void *d
/* Wait until the hardware lock is gone or we can acquire it. */
spin_lock(&master->lock.spinlock);
+
+ if (master->lock.user_waiters == UINT32_MAX) {
+ error = -EBUSY;
+ goto out1;
+ }
+
+ master->lock.user_waiters++;
DRM_SPIN_WAIT_UNTIL(error, &master->lock.lock_queue,
&master->lock.spinlock,
((master->lock.hw_lock == NULL) ||
- (acquired = drm_lock_acquire(&master->lock,
- lock_request->context))));
+ drm_lock_acquire(&master->lock, lock_request->context)));
+ KASSERT(0 < master->lock.user_waiters);
+ master->lock.user_waiters--;
if (error)
- goto out;
+ goto out1;
/* If the lock is gone, give up. */
if (master->lock.hw_lock == NULL) {
@@ -99,36 +120,36 @@ drm_lock(struct drm_device *dev, void *d
#else
error = -ENXIO;
#endif
- goto out;
+ goto out1;
}
/* Mark the lock as owned by file. */
master->lock.file_priv = file;
-#ifndef __NetBSD__
master->lock.lock_time = jiffies; /* XXX Unused? */
-#endif
/* Block signals while the lock is held. */
error = drm_lock_block_signals(dev, lock_request, file);
if (error)
- goto fail0;
+ goto fail2;
/* Enter the DMA quiescent state if requested and available. */
+ /* XXX Drop the spin lock first... */
if (ISSET(lock_request->flags, _DRM_LOCK_QUIESCENT) &&
(dev->driver->dma_quiescent != NULL)) {
error = (*dev->driver->dma_quiescent)(dev);
if (error)
- goto fail1;
+ goto fail3;
}
/* Success! */
error = 0;
- goto out;
+ goto out1;
-fail1: drm_lock_unblock_signals(dev, lock_request, file);
-fail0: drm_lock_release(&master->lock, lock_request->context);
+fail3: drm_lock_unblock_signals(dev, lock_request, file);
+fail2: drm_lock_release(&master->lock, lock_request->context);
master->lock.file_priv = NULL;
-out: spin_unlock(&master->lock.spinlock);
+out1: spin_unlock(&master->lock.spinlock);
+out0: mutex_lock(&drm_global_mutex);
return error;
}
@@ -143,11 +164,15 @@ drm_unlock(struct drm_device *dev, void
struct drm_master *master = file->master;
int error;
+ /* Sanitize the drm global mutex bollocks until we get rid of it. */
KASSERT(mutex_is_locked(&drm_global_mutex));
+ mutex_unlock(&drm_global_mutex);
/* Refuse to unlock on behalf of the kernel. */
- if (lock_request->context == DRM_KERNEL_CONTEXT)
- return -EINVAL;
+ if (lock_request->context == DRM_KERNEL_CONTEXT) {
+ error = -EINVAL;
+ goto out0;
+ }
/* Count it in the device statistics. */
atomic_inc(&dev->counts[_DRM_STAT_UNLOCKS]);
@@ -158,20 +183,20 @@ drm_unlock(struct drm_device *dev, void
/* Make sure it's actually locked. */
if (!_DRM_LOCK_IS_HELD(master->lock.hw_lock->lock)) {
error = -EINVAL; /* XXX Right error? */
- goto out;
+ goto out1;
}
/* Make sure it's locked in the right context. */
if (_DRM_LOCKING_CONTEXT(master->lock.hw_lock->lock) !=
lock_request->context) {
error = -EACCES; /* XXX Right error? */
- goto out;
+ goto out1;
}
/* Make sure it's locked by us. */
if (master->lock.file_priv != file) {
error = -EACCES; /* XXX Right error? */
- goto out;
+ goto out1;
}
/* Actually release the lock. */
@@ -186,7 +211,8 @@ drm_unlock(struct drm_device *dev, void
/* Success! */
error = 0;
-out: spin_unlock(&master->lock.spinlock);
+out1: spin_unlock(&master->lock.spinlock);
+out0: mutex_lock(&drm_global_mutex);
return error;
}
@@ -195,6 +221,8 @@ out: spin_unlock(&master->lock.spinlock)
*
* Return value is an artefact of Linux. Caller must guarantee
* preconditions; failure is fatal.
+ *
+ * XXX Should we also unblock signals like drm_unlock does?
*/
int
drm_lock_free(struct drm_lock_data *lock_data, unsigned int context)
@@ -234,49 +262,82 @@ drm_idlelock_release(struct drm_lock_dat
/*
* Does this file hold this drm device's hardware lock?
+ *
+ * Used to decide whether to release the lock when the file is being
+ * closed.
+ *
+ * XXX I don't think this answers correctly in the case that the
+ * userland has taken the lock and it is uncontended. But I don't
+ * think we can know what the correct answer is in that case.
*/
int
drm_i_have_hw_lock(struct drm_device *dev, struct drm_file *file)
{
struct drm_lock_data *const lock_data = &file->master->lock;
+ int answer = 0;
/* If this file has never locked anything, then no. */
if (file->lock_count == 0)
return 0;
+ spin_lock(&lock_data->spinlock);
+
/* If there is no lock, then this file doesn't hold it. */
if (lock_data->hw_lock == NULL)
- return 0;
+ goto out;
/* If this lock is not held, then this file doesn't hold it. */
if (!_DRM_LOCK_IS_HELD(lock_data->hw_lock->lock))
- return 0;
+ goto out;
/*
* Otherwise, it boils down to whether this file is the owner
* or someone else.
+ *
+ * XXX This is not reliable! Userland doesn't update this when
+ * it takes the lock...
*/
- return (file == lock_data->file_priv);
+ answer = (file == lock_data->file_priv);
+
+out: spin_unlock(&lock_data->spinlock);
+ return answer;
}
/*
* Try to acquire the lock. Return true if successful, false if not.
*
- * Lock's spinlock must be held.
+ * This is hairy because it races with userland, and if userland
+ * already holds the lock, we must tell it, by marking it
+ * _DRM_LOCK_CONT (contended), that it must call ioctl(DRM_UNLOCK) to
+ * release the lock so that we can wake waiters.
+ *
+ * XXX What happens if the process is interrupted?
*/
static bool
drm_lock_acquire(struct drm_lock_data *lock_data, int context)
{
+ volatile unsigned int *const lock = &lock_data->hw_lock->lock;
+ unsigned int old, new;
KASSERT(spin_is_locked(&lock_data->spinlock));
- /* Test and set. */
- if (_DRM_LOCK_IS_HELD(lock_data->hw_lock->lock)) {
- return false;
- } else {
- lock_data->hw_lock->lock = (context | _DRM_LOCK_HELD);
- return true;
- }
+ do {
+ old = *lock;
+ if (!_DRM_LOCK_IS_HELD(old)) {
+ new = (context | _DRM_LOCK_HELD);
+ if ((0 < lock_data->user_waiters) ||
+ (0 < lock_data->kernel_waiters))
+ new |= _DRM_LOCK_CONT;
+ } else if (_DRM_LOCKING_CONTEXT(old) != context) {
+ new = (old | _DRM_LOCK_CONT);
+ } else {
+ DRM_ERROR("%d already holds heavyweight lock\n",
+ context);
+ return false;
+ }
+ } while (atomic_cas_uint(lock, old, new) != old);
+
+ return !_DRM_LOCK_IS_HELD(old);
}
/*
@@ -294,20 +355,8 @@ drm_lock_release(struct drm_lock_data *l
KASSERT(_DRM_LOCK_IS_HELD(lock_data->hw_lock->lock));
KASSERT(_DRM_LOCKING_CONTEXT(lock_data->hw_lock->lock) == context);
- /* Are there any kernel waiters? */
- if (lock_data->n_kernel_waiters > 0) {
- /* If there's a kernel waiter, grant it ownership. */
- lock_data->hw_lock->lock = (DRM_KERNEL_CONTEXT |
- _DRM_LOCK_HELD);
- lock_data->n_kernel_waiters--;
- DRM_SPIN_WAKEUP_ONE(&lock_data->kernel_lock_queue,
- &lock_data->spinlock);
- } else {
- /* Otherwise, free it up and wake someone else. */
- lock_data->hw_lock->lock = 0;
- DRM_SPIN_WAKEUP_ONE(&lock_data->lock_queue,
- &lock_data->spinlock);
- }
+ lock_data->hw_lock->lock = 0;
+ DRM_SPIN_WAKEUP_ONE(&lock_data->lock_queue, &lock_data->spinlock);
}
/*
