Skip to content

Commit

Permalink
qxl: convert qxl driver to proper use for reservations
Browse files Browse the repository at this point in the history 
The recent addition of lockdep support to reservations and their subsequent
use by TTM showed up a number of potential problems with the way qxl was using
TTM objects.

a) it was allocating objects, and reserving them later without validating
underneath the reservation, which meant in extreme conditions the objects could
be evicted before the reservation ever used them.

b) it was reserving objects straight after allocating them, but with no
ability to back off should the reservations fail. It now allocates the necessary
objects then does a complete reservation pass on them to avoid deadlocks.

c) it had two lists per release tracking objects, unnecessary complicating
the reservation process.

This patch removes the dual object tracking, adds reservations ticket support
to the release and fence object handling. It then ports the internal fb
drawing code and the userspace facing ioctl to use the new interfaces properly,
along with cleanup up the error path handling in some codepaths.

Signed-off-by: Dave Airlie <[email protected]>
  • Loading branch information
@airlied
airlied committed Jul 24, 2013
1 parent 4f49ec9 commit 8002db6
Show file tree
Hide file tree
Showing 10 changed files with 676 additions and 464 deletions.
40 changes: 17 additions & 23 deletions drivers/gpu/drm/qxl/qxl_cmd.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -179,9 +179,10 @@ qxl_push_command_ring_release(struct qxl_device *qdev, struct qxl_release *relea
uint32_t type, bool interruptible)
{
struct qxl_command cmd;
struct qxl_bo_list *entry = list_first_entry(&release->bos, struct qxl_bo_list, tv.head);

cmd.type = type;
cmd.data = qxl_bo_physical_address(qdev, release->bos[0], release->release_offset);
cmd.data = qxl_bo_physical_address(qdev, to_qxl_bo(entry->tv.bo), release->release_offset);

return qxl_ring_push(qdev->command_ring, &cmd, interruptible);
}
Expand All @@ -191,9 +192,10 @@ qxl_push_cursor_ring_release(struct qxl_device *qdev, struct qxl_release *releas
uint32_t type, bool interruptible)
{
struct qxl_command cmd;
struct qxl_bo_list *entry = list_first_entry(&release->bos, struct qxl_bo_list, tv.head);

cmd.type = type;
cmd.data = qxl_bo_physical_address(qdev, release->bos[0], release->release_offset);
cmd.data = qxl_bo_physical_address(qdev, to_qxl_bo(entry->tv.bo), release->release_offset);

return qxl_ring_push(qdev->cursor_ring, &cmd, interruptible);
}
Expand All @@ -214,7 +216,6 @@ int qxl_garbage_collect(struct qxl_device *qdev)
struct qxl_release *release;
uint64_t id, next_id;
int i = 0;
int ret;
union qxl_release_info *info;

while (qxl_ring_pop(qdev->release_ring, &id)) {
Expand All @@ -224,17 +225,10 @@ int qxl_garbage_collect(struct qxl_device *qdev)
if (release == NULL)
break;

ret = qxl_release_reserve(qdev, release, false);
if (ret) {
qxl_io_log(qdev, "failed to reserve release on garbage collect %lld\n", id);
DRM_ERROR("failed to reserve release %lld\n", id);
}

info = qxl_release_map(qdev, release);
next_id = info->next;
qxl_release_unmap(qdev, release, info);

qxl_release_unreserve(qdev, release);
QXL_INFO(qdev, "popped %lld, next %lld\n", id,
next_id);

Expand All @@ -259,7 +253,9 @@ int qxl_garbage_collect(struct qxl_device *qdev)
return i;
}

int qxl_alloc_bo_reserved(struct qxl_device *qdev, unsigned long size,
int qxl_alloc_bo_reserved(struct qxl_device *qdev,
struct qxl_release *release,
unsigned long size,
struct qxl_bo **_bo)
{
struct qxl_bo *bo;
Expand All @@ -271,15 +267,15 @@ int qxl_alloc_bo_reserved(struct qxl_device *qdev, unsigned long size,
DRM_ERROR("failed to allocate VRAM BO\n");
return ret;
}
ret = qxl_bo_reserve(bo, false);
if (unlikely(ret != 0))
ret = qxl_release_list_add(release, bo);
if (ret)
goto out_unref;

*_bo = bo;
return 0;
out_unref:
qxl_bo_unref(&bo);
return 0;
return ret;
}

static int wait_for_io_cmd_user(struct qxl_device *qdev, uint8_t val, long port, bool intr)
Expand Down Expand Up @@ -503,6 +499,10 @@ int qxl_hw_surface_alloc(struct qxl_device *qdev,
if (ret)
return ret;

ret = qxl_release_reserve_list(release, true);
if (ret)
return ret;

cmd = (struct qxl_surface_cmd *)qxl_release_map(qdev, release);
cmd->type = QXL_SURFACE_CMD_CREATE;
cmd->u.surface_create.format = surf->surf.format;
Expand All @@ -524,14 +524,11 @@ int qxl_hw_surface_alloc(struct qxl_device *qdev,

surf->surf_create = release;

/* no need to add a release to the fence for this bo,
/* no need to add a release to the fence for this surface bo,
since it is only released when we ask to destroy the surface
and it would never signal otherwise */
qxl_fence_releaseable(qdev, release);

qxl_push_command_ring_release(qdev, release, QXL_CMD_SURFACE, false);

qxl_release_unreserve(qdev, release);
qxl_release_fence_buffer_objects(release);

surf->hw_surf_alloc = true;
spin_lock(&qdev->surf_id_idr_lock);
Expand Down Expand Up @@ -573,12 +570,9 @@ int qxl_hw_surface_dealloc(struct qxl_device *qdev,
cmd->surface_id = id;
qxl_release_unmap(qdev, release, &cmd->release_info);

qxl_fence_releaseable(qdev, release);

qxl_push_command_ring_release(qdev, release, QXL_CMD_SURFACE, false);

qxl_release_unreserve(qdev, release);

qxl_release_fence_buffer_objects(release);

return 0;
}
Expand Down
70 changes: 46 additions & 24 deletions drivers/gpu/drm/qxl/qxl_display.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -179,7 +179,7 @@ static void qxl_crtc_destroy(struct drm_crtc *crtc)
kfree(qxl_crtc);
}

static void
static int
qxl_hide_cursor(struct qxl_device *qdev)
{
struct qxl_release *release;
Expand All @@ -188,14 +188,22 @@ qxl_hide_cursor(struct qxl_device *qdev)

ret = qxl_alloc_release_reserved(qdev, sizeof(*cmd), QXL_RELEASE_CURSOR_CMD,
&release, NULL);
if (ret)
return ret;

ret = qxl_release_reserve_list(release, true);
if (ret) {
qxl_release_free(qdev, release);
return ret;
}

cmd = (struct qxl_cursor_cmd *)qxl_release_map(qdev, release);
cmd->type = QXL_CURSOR_HIDE;
qxl_release_unmap(qdev, release, &cmd->release_info);

qxl_fence_releaseable(qdev, release);
qxl_push_cursor_ring_release(qdev, release, QXL_CMD_CURSOR, false);
qxl_release_unreserve(qdev, release);
qxl_release_fence_buffer_objects(release);
return 0;
}

static int qxl_crtc_cursor_set2(struct drm_crtc *crtc,
Expand All @@ -216,10 +224,8 @@ static int qxl_crtc_cursor_set2(struct drm_crtc *crtc,

int size = 64*64*4;
int ret = 0;
if (!handle) {
qxl_hide_cursor(qdev);
return 0;
}
if (!handle)
return qxl_hide_cursor(qdev);

obj = drm_gem_object_lookup(crtc->dev, file_priv, handle);
if (!obj) {
Expand All @@ -234,8 +240,9 @@ static int qxl_crtc_cursor_set2(struct drm_crtc *crtc,
goto out_unref;

ret = qxl_bo_pin(user_bo, QXL_GEM_DOMAIN_CPU, NULL);
qxl_bo_unreserve(user_bo);
if (ret)
goto out_unreserve;
goto out_unref;

ret = qxl_bo_kmap(user_bo, &user_ptr);
if (ret)
Expand All @@ -246,14 +253,20 @@ static int qxl_crtc_cursor_set2(struct drm_crtc *crtc,
&release, NULL);
if (ret)
goto out_kunmap;
ret = qxl_alloc_bo_reserved(qdev, sizeof(struct qxl_cursor) + size,
&cursor_bo);

ret = qxl_alloc_bo_reserved(qdev, release, sizeof(struct qxl_cursor) + size,
&cursor_bo);
if (ret)
goto out_free_release;
ret = qxl_bo_kmap(cursor_bo, (void **)&cursor);

ret = qxl_release_reserve_list(release, false);
if (ret)
goto out_free_bo;

ret = qxl_bo_kmap(cursor_bo, (void **)&cursor);
if (ret)
goto out_backoff;

cursor->header.unique = 0;
cursor->header.type = SPICE_CURSOR_TYPE_ALPHA;
cursor->header.width = 64;
Expand All @@ -269,42 +282,43 @@ static int qxl_crtc_cursor_set2(struct drm_crtc *crtc,

qxl_bo_kunmap(cursor_bo);

/* finish with the userspace bo */
qxl_bo_kunmap(user_bo);
qxl_bo_unpin(user_bo);
qxl_bo_unreserve(user_bo);
drm_gem_object_unreference_unlocked(obj);

cmd = (struct qxl_cursor_cmd *)qxl_release_map(qdev, release);
cmd->type = QXL_CURSOR_SET;
cmd->u.set.position.x = qcrtc->cur_x;
cmd->u.set.position.y = qcrtc->cur_y;

cmd->u.set.shape = qxl_bo_physical_address(qdev, cursor_bo, 0);
qxl_release_add_res(qdev, release, cursor_bo);

cmd->u.set.visible = 1;
qxl_release_unmap(qdev, release, &cmd->release_info);

qxl_fence_releaseable(qdev, release);
qxl_push_cursor_ring_release(qdev, release, QXL_CMD_CURSOR, false);
qxl_release_unreserve(qdev, release);
qxl_release_fence_buffer_objects(release);

/* finish with the userspace bo */
ret = qxl_bo_reserve(user_bo, false);
if (!ret) {
qxl_bo_unpin(user_bo);
qxl_bo_unreserve(user_bo);
}
drm_gem_object_unreference_unlocked(obj);

qxl_bo_unreserve(cursor_bo);
qxl_bo_unref(&cursor_bo);

return ret;

out_backoff:
qxl_release_backoff_reserve_list(release);
out_free_bo:
qxl_bo_unref(&cursor_bo);
out_free_release:
qxl_release_unreserve(qdev, release);
qxl_release_free(qdev, release);
out_kunmap:
qxl_bo_kunmap(user_bo);
out_unpin:
qxl_bo_unpin(user_bo);
out_unreserve:
qxl_bo_unreserve(user_bo);
out_unref:
drm_gem_object_unreference_unlocked(obj);
return ret;
Expand All @@ -322,6 +336,14 @@ static int qxl_crtc_cursor_move(struct drm_crtc *crtc,

ret = qxl_alloc_release_reserved(qdev, sizeof(*cmd), QXL_RELEASE_CURSOR_CMD,
&release, NULL);
if (ret)
return ret;

ret = qxl_release_reserve_list(release, true);
if (ret) {
qxl_release_free(qdev, release);
return ret;
}

qcrtc->cur_x = x;
qcrtc->cur_y = y;
Expand All @@ -332,9 +354,9 @@ static int qxl_crtc_cursor_move(struct drm_crtc *crtc,
cmd->u.position.y = qcrtc->cur_y;
qxl_release_unmap(qdev, release, &cmd->release_info);

qxl_fence_releaseable(qdev, release);
qxl_push_cursor_ring_release(qdev, release, QXL_CMD_CURSOR, false);
qxl_release_unreserve(qdev, release);
qxl_release_fence_buffer_objects(release);

return 0;
}

Expand Down
Loading

0 comments on commit 8002db6

Please sign in to comment.