This fixes a deadlock inversion when vblank is enabled/disabled by drm.
&dev->vblank_time_lock is always taken when the vblank state is toggled,
which caused a deadlock when &event->lock was also taken during
event_get/put.

Solve the race by requiring that lock to change enable/disable state,
and always keeping vblank on the event list. Core drm ignores unwanted
vblanks, so extra calls to drm_handle_vblank are harmless.

Signed-off-by: Maarten Lankhorst <maarten.lankho...@canonical.com>
---
diff --git a/drivers/gpu/drm/nouveau/core/core/event.c 
b/drivers/gpu/drm/nouveau/core/core/event.c
index 7eb81c1..78bff7c 100644
--- a/drivers/gpu/drm/nouveau/core/core/event.c
+++ b/drivers/gpu/drm/nouveau/core/core/event.c
@@ -23,43 +23,64 @@
 #include <core/os.h>
 #include <core/event.h>
 
-static void
-nouveau_event_put_locked(struct nouveau_event *event, int index,
-                        struct nouveau_eventh *handler)
-{
-       if (!--event->index[index].refs) {
-               if (event->disable)
-                       event->disable(event, index);
-       }
-       list_del(&handler->head);
-}
-
 void
 nouveau_event_put(struct nouveau_event *event, int index,
                  struct nouveau_eventh *handler)
 {
        unsigned long flags;
 
+       if (index >= event->index_nr)
+               return;
+
        spin_lock_irqsave(&event->lock, flags);
-       if (index < event->index_nr)
-               nouveau_event_put_locked(event, index, handler);
+       list_del(&handler->head);
+
+       if (event->toggle_lock)
+               spin_lock(event->toggle_lock);
+       nouveau_event_disable_locked(event, index, 1);
+       if (event->toggle_lock)
+               spin_unlock(event->toggle_lock);
+
        spin_unlock_irqrestore(&event->lock, flags);
 }
 
 void
+nouveau_event_enable_locked(struct nouveau_event *event, int index)
+{
+       if (index >= event->index_nr)
+               return;
+
+       if (!event->index[index].refs++ && event->enable)
+               event->enable(event, index);
+}
+
+void
+nouveau_event_disable_locked(struct nouveau_event *event, int index, int refs)
+{
+       if (index >= event->index_nr)
+               return;
+
+       event->index[index].refs -= refs;
+       if (!event->index[index].refs && event->disable)
+               event->disable(event, index);
+}
+
+void
 nouveau_event_get(struct nouveau_event *event, int index,
                  struct nouveau_eventh *handler)
 {
        unsigned long flags;
 
+       if (index >= event->index_nr)
+               return;
+
        spin_lock_irqsave(&event->lock, flags);
-       if (index < event->index_nr) {
-               list_add(&handler->head, &event->index[index].list);
-               if (!event->index[index].refs++) {
-                       if (event->enable)
-                               event->enable(event, index);
-               }
-       }
+       list_add(&handler->head, &event->index[index].list);
+       if (event->toggle_lock)
+               spin_lock(event->toggle_lock);
+       nouveau_event_enable_locked(event, index);
+       if (event->toggle_lock)
+               spin_unlock(event->toggle_lock);
        spin_unlock_irqrestore(&event->lock, flags);
 }
 
@@ -68,6 +89,7 @@ nouveau_event_trigger(struct nouveau_event *event, int index)
 {
        struct nouveau_eventh *handler, *temp;
        unsigned long flags;
+       int refs = 0;
 
        if (index >= event->index_nr)
                return;
@@ -75,9 +97,17 @@ nouveau_event_trigger(struct nouveau_event *event, int index)
        spin_lock_irqsave(&event->lock, flags);
        list_for_each_entry_safe(handler, temp, &event->index[index].list, 
head) {
                if (handler->func(handler, index) == NVKM_EVENT_DROP) {
-                       nouveau_event_put_locked(event, index, handler);
+                       list_del(&handler->head);
+                       refs++;
                }
        }
+       if (refs) {
+               if (event->toggle_lock)
+                       spin_lock(event->toggle_lock);
+               nouveau_event_disable_locked(event, index, refs);
+               if (event->toggle_lock)
+                       spin_unlock(event->toggle_lock);
+       }
        spin_unlock_irqrestore(&event->lock, flags);
 }
 
diff --git a/drivers/gpu/drm/nouveau/core/include/core/event.h 
b/drivers/gpu/drm/nouveau/core/include/core/event.h
index 9e09440..b1a6c91 100644
--- a/drivers/gpu/drm/nouveau/core/include/core/event.h
+++ b/drivers/gpu/drm/nouveau/core/include/core/event.h
@@ -12,6 +12,7 @@ struct nouveau_eventh {
 
 struct nouveau_event {
        spinlock_t lock;
+       spinlock_t *toggle_lock;
 
        void *priv;
        void (*enable)(struct nouveau_event *, int index);
@@ -33,4 +34,8 @@ void nouveau_event_get(struct nouveau_event *, int index,
 void nouveau_event_put(struct nouveau_event *, int index,
                       struct nouveau_eventh *);
 
+void nouveau_event_enable_locked(struct nouveau_event *event, int index);
+void nouveau_event_disable_locked(struct nouveau_event *event, int index,
+                                 int refs);
+
 #endif
diff --git a/drivers/gpu/drm/nouveau/nouveau_crtc.h 
b/drivers/gpu/drm/nouveau/nouveau_crtc.h
index d1e5890..398baa3 100644
--- a/drivers/gpu/drm/nouveau/nouveau_crtc.h
+++ b/drivers/gpu/drm/nouveau/nouveau_crtc.h
@@ -29,6 +29,7 @@
 
 struct nouveau_crtc {
        struct drm_crtc base;
+       struct nouveau_eventh vblank;
 
        int index;
 
diff --git a/drivers/gpu/drm/nouveau/nouveau_display.h 
b/drivers/gpu/drm/nouveau/nouveau_display.h
index 1ea3e47..4ba8cb5 100644
--- a/drivers/gpu/drm/nouveau/nouveau_display.h
+++ b/drivers/gpu/drm/nouveau/nouveau_display.h
@@ -72,6 +72,7 @@ int  nouveau_display_dumb_destroy(struct drm_file *, struct 
drm_device *,
                                  u32 handle);
 
 void nouveau_hdmi_mode_set(struct drm_encoder *, struct drm_display_mode *);
+int nouveau_drm_vblank_handler(struct nouveau_eventh *event, int head);
 
 #ifdef CONFIG_DRM_NOUVEAU_BACKLIGHT
 extern int nouveau_backlight_init(struct drm_device *);
diff --git a/drivers/gpu/drm/nouveau/nouveau_drm.c 
b/drivers/gpu/drm/nouveau/nouveau_drm.c
index 4bf8dc3..bd301f4 100644
--- a/drivers/gpu/drm/nouveau/nouveau_drm.c
+++ b/drivers/gpu/drm/nouveau/nouveau_drm.c
@@ -46,6 +46,7 @@
 #include "nouveau_pm.h"
 #include "nouveau_acpi.h"
 #include "nouveau_bios.h"
+#include "nouveau_crtc.h"
 #include "nouveau_ioctl.h"
 #include "nouveau_abi16.h"
 #include "nouveau_fbcon.h"
@@ -71,12 +72,13 @@ module_param_named(modeset, nouveau_modeset, int, 0400);
 
 static struct drm_driver driver;
 
-static int
+int
 nouveau_drm_vblank_handler(struct nouveau_eventh *event, int head)
 {
-       struct nouveau_drm *drm =
-               container_of(event, struct nouveau_drm, vblank[head]);
-       drm_handle_vblank(drm->dev, head);
+       struct nouveau_crtc *nv_crtc =
+               container_of(event, struct nouveau_crtc, vblank);
+
+       drm_handle_vblank(nv_crtc->base.dev, head);
        return NVKM_EVENT_KEEP;
 }
 
@@ -86,11 +88,9 @@ nouveau_drm_vblank_enable(struct drm_device *dev, int head)
        struct nouveau_drm *drm = nouveau_drm(dev);
        struct nouveau_disp *pdisp = nouveau_disp(drm->device);
 
-       if (WARN_ON_ONCE(head > ARRAY_SIZE(drm->vblank)))
+       if (head >= pdisp->vblank->index_nr)
                return -EIO;
-       WARN_ON_ONCE(drm->vblank[head].func);
-       drm->vblank[head].func = nouveau_drm_vblank_handler;
-       nouveau_event_get(pdisp->vblank, head, &drm->vblank[head]);
+       nouveau_event_enable_locked(pdisp->vblank, head);
        return 0;
 }
 
@@ -99,11 +99,9 @@ nouveau_drm_vblank_disable(struct drm_device *dev, int head)
 {
        struct nouveau_drm *drm = nouveau_drm(dev);
        struct nouveau_disp *pdisp = nouveau_disp(drm->device);
-       if (drm->vblank[head].func)
-               nouveau_event_put(pdisp->vblank, head, &drm->vblank[head]);
-       else
-               WARN_ON_ONCE(1);
-       drm->vblank[head].func = NULL;
+
+       if (head < pdisp->vblank->index_nr)
+               nouveau_event_disable_locked(pdisp->vblank, head, 1);
 }
 
 static u64
diff --git a/drivers/gpu/drm/nouveau/nouveau_drm.h 
b/drivers/gpu/drm/nouveau/nouveau_drm.h
index 41ff7e0..0d0ba3b 100644
--- a/drivers/gpu/drm/nouveau/nouveau_drm.h
+++ b/drivers/gpu/drm/nouveau/nouveau_drm.h
@@ -125,7 +125,6 @@ struct nouveau_drm {
        struct nvbios vbios;
        struct nouveau_display *display;
        struct backlight_device *backlight;
-       struct nouveau_eventh vblank[4];
 
        /* power management */
        struct nouveau_pm *pm;
diff --git a/drivers/gpu/drm/nouveau/nv50_display.c 
b/drivers/gpu/drm/nouveau/nv50_display.c
index 007731d..738d7a2 100644
--- a/drivers/gpu/drm/nouveau/nv50_display.c
+++ b/drivers/gpu/drm/nouveau/nv50_display.c
@@ -39,6 +39,7 @@
 #include <core/client.h>
 #include <core/gpuobj.h>
 #include <core/class.h>
+#include <engine/disp.h>
 
 #include <subdev/timer.h>
 #include <subdev/bar.h>
@@ -1280,15 +1281,21 @@ nv50_crtc_gamma_set(struct drm_crtc *crtc, u16 *r, u16 
*g, u16 *b,
 static void
 nv50_crtc_destroy(struct drm_crtc *crtc)
 {
+       struct nouveau_event *event = 
nouveau_disp(nouveau_dev(crtc->dev))->vblank;
        struct nouveau_crtc *nv_crtc = nouveau_crtc(crtc);
        struct nv50_disp *disp = nv50_disp(crtc->dev);
        struct nv50_head *head = nv50_head(crtc);
+       unsigned long flags;
 
        nv50_dmac_destroy(disp->core, &head->ovly.base);
        nv50_pioc_destroy(disp->core, &head->oimm.base);
        nv50_dmac_destroy(disp->core, &head->sync.base);
        nv50_pioc_destroy(disp->core, &head->curs.base);
 
+       spin_lock_irqsave(&event->lock, flags);
+       list_del(&nv_crtc->vblank.head);
+       spin_unlock_irqrestore(&event->lock, flags);
+
        /*XXX: this shouldn't be necessary, but the core doesn't call
         *     disconnect() during the cleanup paths
         */
@@ -1344,10 +1351,12 @@ nv50_cursor_set_offset(struct nouveau_crtc *nv_crtc, 
uint32_t offset)
 static int
 nv50_crtc_create(struct drm_device *dev, struct nouveau_object *core, int 
index)
 {
+       struct nouveau_event *event = nouveau_disp(nouveau_dev(dev))->vblank;
        struct nv50_disp *disp = nv50_disp(dev);
        struct nv50_head *head;
        struct drm_crtc *crtc;
        int ret, i;
+       unsigned long flags;
 
        head = kzalloc(sizeof(*head), GFP_KERNEL);
        if (!head)
@@ -1372,6 +1381,12 @@ nv50_crtc_create(struct drm_device *dev, struct 
nouveau_object *core, int index)
        drm_crtc_helper_add(crtc, &nv50_crtc_hfunc);
        drm_mode_crtc_set_gamma_size(crtc, 256);
 
+       spin_lock_irqsave(&event->lock, flags);
+       event->toggle_lock = &dev->vblank_time_lock;
+       head->base.vblank.func = nouveau_drm_vblank_handler;
+       list_add(&head->base.vblank.head, &event->index[index].list);
+       spin_unlock_irqrestore(&event->lock, flags);
+
        ret = nouveau_bo_new(dev, 8192, 0x100, TTM_PL_FLAG_VRAM,
                             0, 0x0000, NULL, NULL, &head->base.lut.nvbo);
        if (!ret) {

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Reply via email to