Source-Changes-HG archive
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]
[src/riastradh-drm2]: src/sys/external/bsd/drm2 Rewrite drm locking support f...
details: https://anonhg.NetBSD.org/src/rev/112da75f3ffc
branches: riastradh-drm2
changeset: 788598:112da75f3ffc
user: riastradh <riastradh%NetBSD.org@localhost>
date: Wed Jan 15 13:53:53 2014 +0000
description:
Rewrite drm locking support for NetBSD.
diffstat:
sys/external/bsd/drm2/dist/drm/drm_bufs.c | 27 +++-
sys/external/bsd/drm2/dist/drm/drm_context.c | 5 +-
sys/external/bsd/drm2/dist/drm/drm_stub.c | 4 +-
sys/external/bsd/drm2/dist/include/drm/drmP.h | 7 -
sys/external/bsd/drm2/drm/drm_fops.c | 15 +-
sys/external/bsd/drm2/drm/drm_lock.c | 163 ++++++++++++++++---------
6 files changed, 146 insertions(+), 75 deletions(-)
diffs (truncated from 497 to 300 lines):
diff -r d3d508c7f489 -r 112da75f3ffc sys/external/bsd/drm2/dist/drm/drm_bufs.c
--- a/sys/external/bsd/drm2/dist/drm/drm_bufs.c Wed Jan 15 13:53:42 2014 +0000
+++ b/sys/external/bsd/drm2/dist/drm/drm_bufs.c Wed Jan 15 13:53:53 2014 +0000
@@ -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 @@
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 @@
}
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:
diff -r d3d508c7f489 -r 112da75f3ffc sys/external/bsd/drm2/dist/drm/drm_context.c
--- a/sys/external/bsd/drm2/dist/drm/drm_context.c Wed Jan 15 13:53:42 2014 +0000
+++ b/sys/external/bsd/drm2/dist/drm/drm_context.c Wed Jan 15 13:53:53 2014 +0000
@@ -267,9 +267,12 @@
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
diff -r d3d508c7f489 -r 112da75f3ffc sys/external/bsd/drm2/dist/drm/drm_stub.c
--- a/sys/external/bsd/drm2/dist/drm/drm_stub.c Wed Jan 15 13:53:42 2014 +0000
+++ b/sys/external/bsd/drm2/dist/drm/drm_stub.c Wed Jan 15 13:53:53 2014 +0000
@@ -170,8 +170,7 @@
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 @@
#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);
diff -r d3d508c7f489 -r 112da75f3ffc sys/external/bsd/drm2/dist/include/drm/drmP.h
--- a/sys/external/bsd/drm2/dist/include/drm/drmP.h Wed Jan 15 13:53:42 2014 +0000
+++ b/sys/external/bsd/drm2/dist/include/drm/drmP.h Wed Jan 15 13:53:53 2014 +0000
@@ -546,18 +546,11 @@
#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
};
/**
diff -r d3d508c7f489 -r 112da75f3ffc sys/external/bsd/drm2/drm/drm_fops.c
--- a/sys/external/bsd/drm2/drm/drm_fops.c Wed Jan 15 13:53:42 2014 +0000
+++ b/sys/external/bsd/drm2/drm/drm_fops.c Wed Jan 15 13:53:53 2014 +0000
@@ -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 @@
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 @@
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)
diff -r d3d508c7f489 -r 112da75f3ffc sys/external/bsd/drm2/drm/drm_lock.c
--- a/sys/external/bsd/drm2/drm/drm_lock.c Wed Jan 15 13:53:42 2014 +0000
+++ b/sys/external/bsd/drm2/drm/drm_lock.c Wed Jan 15 13:53:53 2014 +0000
@@ -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 @@
{
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 @@
/* 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 @@
#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;
Home |
Main Index |
Thread Index |
Old Index