The current drm_locked_task*() code sometimes tries to sleep in an

interrupt handler.

This is bad and wrong. So change it so that if we can't immediately grab
the hardware lock, to just leave the task flagged so that we can run it
when we release the lock. The linux implementation uses a similar
scheme.

Tested by guenther@, landry@ and bernd@. Also tested by many a while
ago as part of a larger diff.

diff --git a/sys/dev/pci/drm/drmP.h b/sys/dev/pci/drm/drmP.h
index 31d73c0..3b448af 100644 (file)
--- a/sys/dev/pci/drm/drmP.h
+++ b/sys/dev/pci/drm/drmP.h
@@ -638,6 +638,7 @@ struct drm_device {
        DRM_SPINTYPE      irq_lock;     /* protects irq condition checks */
        DRM_SPINTYPE      dev_lock;     /* protects everything else */
        DRM_SPINTYPE      drw_lock;
+       DRM_SPINTYPE      tsk_lock;
 
                                /* Usage Counters */
        int               open_count;   /* Outstanding files open          */

diff --git a/sys/dev/pci/drm/drm_drv.c b/sys/dev/pci/drm/drm_drv.c
index 3175dac..9f5a107 100644 (file)
--- a/sys/dev/pci/drm/drm_drv.c
+++ b/sys/dev/pci/drm/drm_drv.c
@@ -169,7 +169,8 @@ drm_attach(struct device *parent, struct device *kdev,
        dev->pci_slot = pa->pa_device;
        dev->pci_func = pa->pa_function;
        DRM_SPININIT(&dev->dev_lock, "drm device");
-       DRM_SPININIT(&dev->drw_lock, "drm drawable lock");
+       mtx_init(&dev->drw_lock, IPL_BIO);
+       mtx_init(&dev->tsk_lock, IPL_BIO);
 
        id_entry = drm_find_description(PCI_VENDOR(pa->pa_id),
            PCI_PRODUCT(pa->pa_id), idlist);

diff --git a/sys/dev/pci/drm/drm_irq.c b/sys/dev/pci/drm/drm_irq.c
index 76477a1..9c216f5 100644 (file)
--- a/sys/dev/pci/drm/drm_irq.c
+++ b/sys/dev/pci/drm/drm_irq.c
@@ -519,38 +519,43 @@ drm_locked_task(void *context, void *pending)
 {
        struct drm_device *dev = context;
 
-       DRM_LOCK();
-       for (;;) {
-               int ret;
-
-               if (drm_lock_take(&dev->lock.hw_lock->lock,
-                   DRM_KERNEL_CONTEXT))
-               {
-                       dev->lock.file_priv = NULL; /* kernel owned */
-                       dev->lock.lock_time = jiffies;
-                       atomic_inc(&dev->counts[_DRM_STAT_LOCKS]);
-                       break;  /* Got lock */
-               }
+       DRM_SPINLOCK(&dev->tsk_lock);
 
-               /* Contention */
-               ret = DRM_SLEEPLOCK((void *)&dev->lock.lock_queue, &dev->dev_lock,
-                   PZERO | PCATCH, "drmlk2", 0);
-               if (ret != 0) {
-                       DRM_UNLOCK();
-                       return;
-               }
+       DRM_LOCK(); /* XXX drm_lock_take() should do its own locking */
+       if (dev->locked_task_call == NULL ||
+           drm_lock_take(&dev->lock.hw_lock->lock, DRM_KERNEL_CONTEXT) == 0) {
+               DRM_UNLOCK();
+               DRM_SPINUNLOCK(&dev->tsk_lock);
+               return;
        }
+
+       dev->lock.file_priv = NULL; /* kernel owned */
+       dev->lock.lock_time = jiffies;
+       atomic_inc(&dev->counts[_DRM_STAT_LOCKS]);
+
        DRM_UNLOCK();
 
        dev->locked_task_call(dev);
 
        drm_lock_free(dev, &dev->lock.hw_lock->lock, DRM_KERNEL_CONTEXT);
+
+       dev->locked_task_call = NULL;
+
+       DRM_SPINUNLOCK(&dev->tsk_lock);
 }
 
 void
 drm_locked_tasklet(struct drm_device *dev, void (*tasklet)(struct drm_device *))
 {
+       DRM_SPINLOCK(&dev->tsk_lock);
+       if (dev->locked_task_call != NULL) {
+               DRM_SPINUNLOCK(&dev->tsk_lock);
+               return;
+       }
+
        dev->locked_task_call = tasklet;
+       DRM_SPINUNLOCK(&dev->tsk_lock);
+
        if (workq_add_task(NULL, 0, drm_locked_task, dev, NULL) == ENOMEM)
                DRM_ERROR("error adding task to workq\n");
 }

diff --git a/sys/dev/pci/drm/drm_lock.c b/sys/dev/pci/drm/drm_lock.c
index 69a8323..f1640fc 100644 (file)
--- a/sys/dev/pci/drm/drm_lock.c
+++ b/sys/dev/pci/drm/drm_lock.c
@@ -180,6 +180,13 @@ drm_unlock(struct drm_device *dev, void *data, struct drm_file *file_priv)
            _DRM_LOCKING_CONTEXT(dev->lock.hw_lock->lock) != lock->context)
                return EINVAL;
 
+       DRM_SPINLOCK(&dev->tsk_lock);
+       if (dev->locked_task_call != NULL) {
+               dev->locked_task_call(dev);
+               dev->locked_task_call = NULL;
+       }
+       DRM_SPINUNLOCK(&dev->tsk_lock);
+
        atomic_inc(&dev->counts[_DRM_STAT_UNLOCKS]);
 
        DRM_LOCK();