drm/nouveau: fix locking issues in page flipping paths
authorBen Skeggs <bskeggs@redhat.com>
Mon, 8 Jul 2013 04:15:51 +0000 (14:15 +1000)
committerBen Skeggs <bskeggs@redhat.com>
Wed, 10 Jul 2013 00:47:12 +0000 (10:47 +1000)
b580c9e2b7ba5030a795aa2fb73b796523d65a78 introduced additional problems
while trying to solve issues that became apparent while porting to the
new reservation stuff.

The major problem was that the the previously mentioned patch took the
client mutex earlier than previously, but the pinning of new_bo can
can potentially cause a buffer move, which would result in attempting to
acquire the same mutex again.

This commit attempts to fix that "fix".

Thanks to Maarten for the tips on keeping lockdep happy and cooking :)

Reported-by: Maarten Lankhorst <maarten.lankhorst@canonical.com>
Signed-off-by: Ben Skeggs <bskeggs@redhat.com>
Acked-by: Maarten Lankhorst <maarten.lankhorst@canonical.com>
drivers/gpu/drm/nouveau/nouveau_bo.c
drivers/gpu/drm/nouveau/nouveau_display.c
drivers/gpu/drm/nouveau/nouveau_drm.c

index 4b1afb131380312bc971117635c9ac072fbdba1a..85fed108d7e4471c9e908e095acc1a7f26ab203a 100644 (file)
@@ -973,7 +973,7 @@ nouveau_bo_move_m2mf(struct ttm_buffer_object *bo, int evict, bool intr,
        struct ttm_mem_reg *old_mem = &bo->mem;
        int ret;
 
-       mutex_lock(&chan->cli->mutex);
+       mutex_lock_nested(&chan->cli->mutex, SINGLE_DEPTH_NESTING);
 
        /* create temporary vmas for the transfer and attach them to the
         * old nouveau_mem node, these will get cleaned up after ttm has
index 708b2d1c0037e689b579b860d812463c01a9bf2e..61fdef8eac494f638f43231e413b027c158d474c 100644 (file)
@@ -524,9 +524,12 @@ nouveau_crtc_page_flip(struct drm_crtc *crtc, struct drm_framebuffer *fb,
        struct nouveau_page_flip_state *s;
        struct nouveau_channel *chan = NULL;
        struct nouveau_fence *fence;
-       struct list_head res;
-       struct ttm_validate_buffer res_val[2];
+       struct ttm_validate_buffer resv[2] = {
+               { .bo = &old_bo->bo },
+               { .bo = &new_bo->bo },
+       };
        struct ww_acquire_ctx ticket;
+       LIST_HEAD(res);
        int ret;
 
        if (!drm->channel)
@@ -545,27 +548,19 @@ nouveau_crtc_page_flip(struct drm_crtc *crtc, struct drm_framebuffer *fb,
                chan = drm->channel;
        spin_unlock(&old_bo->bo.bdev->fence_lock);
 
-       mutex_lock(&chan->cli->mutex);
-
        if (new_bo != old_bo) {
                ret = nouveau_bo_pin(new_bo, TTM_PL_FLAG_VRAM);
-               if (likely(!ret)) {
-                       res_val[0].bo = &old_bo->bo;
-                       res_val[1].bo = &new_bo->bo;
-                       INIT_LIST_HEAD(&res);
-                       list_add_tail(&res_val[0].head, &res);
-                       list_add_tail(&res_val[1].head, &res);
-                       ret = ttm_eu_reserve_buffers(&ticket, &res);
-                       if (ret)
-                               nouveau_bo_unpin(new_bo);
-               }
-       } else
-               ret = ttm_bo_reserve(&new_bo->bo, false, false, false, 0);
+               if (ret)
+                       goto fail_free;
 
-       if (ret) {
-               mutex_unlock(&chan->cli->mutex);
-               goto fail_free;
+               list_add(&resv[1].head, &res);
        }
+       list_add(&resv[0].head, &res);
+
+       mutex_lock(&chan->cli->mutex);
+       ret = ttm_eu_reserve_buffers(&ticket, &res);
+       if (ret)
+               goto fail_unpin;
 
        /* Initialize a page flip struct */
        *s = (struct nouveau_page_flip_state)
@@ -576,10 +571,8 @@ nouveau_crtc_page_flip(struct drm_crtc *crtc, struct drm_framebuffer *fb,
        /* Emit a page flip */
        if (nv_device(drm->device)->card_type >= NV_50) {
                ret = nv50_display_flip_next(crtc, fb, chan, 0);
-               if (ret) {
-                       mutex_unlock(&chan->cli->mutex);
+               if (ret)
                        goto fail_unreserve;
-               }
        }
 
        ret = nouveau_page_flip_emit(chan, old_bo, new_bo, s, &fence);
@@ -590,22 +583,18 @@ nouveau_crtc_page_flip(struct drm_crtc *crtc, struct drm_framebuffer *fb,
        /* Update the crtc struct and cleanup */
        crtc->fb = fb;
 
-       if (old_bo != new_bo) {
-               ttm_eu_fence_buffer_objects(&ticket, &res, fence);
+       ttm_eu_fence_buffer_objects(&ticket, &res, fence);
+       if (old_bo != new_bo)
                nouveau_bo_unpin(old_bo);
-       } else {
-               nouveau_bo_fence(new_bo, fence);
-               ttm_bo_unreserve(&new_bo->bo);
-       }
        nouveau_fence_unref(&fence);
        return 0;
 
 fail_unreserve:
-       if (old_bo != new_bo) {
-               ttm_eu_backoff_reservation(&ticket, &res);
+       ttm_eu_backoff_reservation(&ticket, &res);
+fail_unpin:
+       mutex_unlock(&chan->cli->mutex);
+       if (old_bo != new_bo)
                nouveau_bo_unpin(new_bo);
-       } else
-               ttm_bo_unreserve(&new_bo->bo);
 fail_free:
        kfree(s);
        return ret;
index 218a4b522fe591ecca92eee558afc2e1baf9ff0c..4eca52b8d5738229849443622671d641ca904638 100644 (file)
@@ -284,8 +284,6 @@ static int nouveau_drm_probe(struct pci_dev *pdev,
        return 0;
 }
 
-static struct lock_class_key drm_client_lock_class_key;
-
 static int
 nouveau_drm_load(struct drm_device *dev, unsigned long flags)
 {
@@ -297,7 +295,6 @@ nouveau_drm_load(struct drm_device *dev, unsigned long flags)
        ret = nouveau_cli_create(pdev, "DRM", sizeof(*drm), (void**)&drm);
        if (ret)
                return ret;
-       lockdep_set_class(&drm->client.mutex, &drm_client_lock_class_key);
 
        dev->dev_private = drm;
        drm->dev = dev;