Skip to content

Commit

Permalink
drm/nouveau: fix locking issues in page flipping paths
Browse files Browse the repository at this point in the history
b580c9e 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>
  • Loading branch information
Ben Skeggs committed Jul 10, 2013
1 parent 06d5a24 commit 060810d
Show file tree
Hide file tree
Showing 3 changed files with 22 additions and 36 deletions.
2 changes: 1 addition & 1 deletion drivers/gpu/drm/nouveau/nouveau_bo.c
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
53 changes: 21 additions & 32 deletions drivers/gpu/drm/nouveau/nouveau_display.c
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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)
Expand All @@ -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);
Expand All @@ -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;
Expand Down
3 changes: 0 additions & 3 deletions drivers/gpu/drm/nouveau/nouveau_drm.c
Original file line number Diff line number Diff line change
Expand Up @@ -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)
{
Expand All @@ -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;
Expand Down

0 comments on commit 060810d

Please sign in to comment.