| Message ID | 20210217123213.2199186-11-kraxel@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/qxl: a collection of fixes | expand |
Hi Am 17.02.21 um 13:32 schrieb Gerd Hoffmann: > Add helper functions to create and move the cursor. > Create the cursor_bo in prepare_fb callback, in the > atomic_commit callback we only send the update command > to the host. I'm still trying to wrap my head around the qxl cursor code. Getting vmap out of the commit tail is good, but I feel like this isn't going in the right direction overall. In ast, these helper functions were only good when converting the drvier to atomic modesetting. So I removed them in the latst patchset and did all the updates in the plane helpers directly. For cursor_bo itself, it seems to be transitional state that is only used during the plane update and crtc update . It should probably be stored in a plane-state structure. Some of the primary plane's functions seem to deal with cursor handling. What's the role of the primary plane in cursor handling? For now, I suggest to merge patch 1 to 8 and 11; and move the cursor patches into a new patchset. I'd like ot hear Daniel's opinion on this. Do you have further plans here? If you absolutely want patches 9 and 10, I'd rubber-stamp an A-b on them. Best regards Thomas > > Signed-off-by: Gerd Hoffmann <kraxel@redhat.com> > --- > drivers/gpu/drm/qxl/qxl_display.c | 248 ++++++++++++++++-------------- > 1 file changed, 133 insertions(+), 115 deletions(-) > > diff --git a/drivers/gpu/drm/qxl/qxl_display.c b/drivers/gpu/drm/qxl/qxl_display.c > index b315d7484e21..4a3d272e8d6c 100644 > --- a/drivers/gpu/drm/qxl/qxl_display.c > +++ b/drivers/gpu/drm/qxl/qxl_display.c > @@ -476,12 +476,11 @@ static int qxl_primary_atomic_check(struct drm_plane *plane, > return qxl_check_framebuffer(qdev, bo); > } > > -static int qxl_primary_apply_cursor(struct drm_plane *plane) > +static int qxl_primary_apply_cursor(struct qxl_device *qdev, > + struct drm_plane_state *plane_state) > { > - struct drm_device *dev = plane->dev; > - struct qxl_device *qdev = to_qxl(dev); > - struct drm_framebuffer *fb = plane->state->fb; > - struct qxl_crtc *qcrtc = to_qxl_crtc(plane->state->crtc); > + struct drm_framebuffer *fb = plane_state->fb; > + struct qxl_crtc *qcrtc = to_qxl_crtc(plane_state->crtc); > struct qxl_cursor_cmd *cmd; > struct qxl_release *release; > int ret = 0; > @@ -505,8 +504,8 @@ static int qxl_primary_apply_cursor(struct drm_plane *plane) > > cmd = (struct qxl_cursor_cmd *)qxl_release_map(qdev, release); > cmd->type = QXL_CURSOR_SET; > - cmd->u.set.position.x = plane->state->crtc_x + fb->hot_x; > - cmd->u.set.position.y = plane->state->crtc_y + fb->hot_y; > + cmd->u.set.position.x = plane_state->crtc_x + fb->hot_x; > + cmd->u.set.position.y = plane_state->crtc_y + fb->hot_y; > > cmd->u.set.shape = qxl_bo_physical_address(qdev, qcrtc->cursor_bo, 0); > > @@ -523,6 +522,113 @@ static int qxl_primary_apply_cursor(struct drm_plane *plane) > return ret; > } > > +static int qxl_primary_move_cursor(struct qxl_device *qdev, > + struct drm_plane_state *plane_state) > +{ > + struct drm_framebuffer *fb = plane_state->fb; > + struct qxl_crtc *qcrtc = to_qxl_crtc(plane_state->crtc); > + struct qxl_cursor_cmd *cmd; > + struct qxl_release *release; > + int ret = 0; > + > + if (!qcrtc->cursor_bo) > + return 0; > + > + 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_MOVE; > + cmd->u.position.x = plane_state->crtc_x + fb->hot_x; > + cmd->u.position.y = plane_state->crtc_y + fb->hot_y; > + qxl_release_unmap(qdev, release, &cmd->release_info); > + > + qxl_release_fence_buffer_objects(release); > + qxl_push_cursor_ring_release(qdev, release, QXL_CMD_CURSOR, false); > + return ret; > +} > + > +static struct qxl_bo *qxl_create_cursor(struct qxl_device *qdev, > + struct qxl_bo *user_bo, > + int hot_x, int hot_y) > +{ > + static const u32 size = 64 * 64 * 4; > + struct qxl_bo *cursor_bo; > + struct dma_buf_map cursor_map; > + struct dma_buf_map user_map; > + struct qxl_cursor cursor; > + int ret; > + > + if (!user_bo) > + return NULL; > + > + ret = qxl_bo_create(qdev, sizeof(struct qxl_cursor) + size, > + false, true, QXL_GEM_DOMAIN_VRAM, 1, > + NULL, &cursor_bo); > + if (ret) > + goto err; > + > + ret = qxl_bo_vmap(cursor_bo, &cursor_map); > + if (ret) > + goto err_unref; > + > + ret = qxl_bo_vmap(user_bo, &user_map); > + if (ret) > + goto err_unmap; > + > + cursor.header.unique = 0; > + cursor.header.type = SPICE_CURSOR_TYPE_ALPHA; > + cursor.header.width = 64; > + cursor.header.height = 64; > + cursor.header.hot_spot_x = hot_x; > + cursor.header.hot_spot_y = hot_y; > + cursor.data_size = size; > + cursor.chunk.next_chunk = 0; > + cursor.chunk.prev_chunk = 0; > + cursor.chunk.data_size = size; > + if (cursor_map.is_iomem) { > + memcpy_toio(cursor_map.vaddr_iomem, > + &cursor, sizeof(cursor)); > + memcpy_toio(cursor_map.vaddr_iomem + sizeof(cursor), > + user_map.vaddr, size); > + } else { > + memcpy(cursor_map.vaddr, > + &cursor, sizeof(cursor)); > + memcpy(cursor_map.vaddr + sizeof(cursor), > + user_map.vaddr, size); > + } > + > + qxl_bo_vunmap(user_bo); > + qxl_bo_vunmap(cursor_bo); > + return cursor_bo; > + > +err_unmap: > + qxl_bo_vunmap(cursor_bo); > +err_unref: > + qxl_bo_unpin(cursor_bo); > + qxl_bo_unref(&cursor_bo); > +err: > + return NULL; > +} > + > +static void qxl_free_cursor(struct qxl_bo *cursor_bo) > +{ > + if (!cursor_bo) > + return; > + > + qxl_bo_unpin(cursor_bo); > + qxl_bo_unref(&cursor_bo); > +} > + > static void qxl_primary_atomic_update(struct drm_plane *plane, > struct drm_plane_state *old_state) > { > @@ -543,7 +649,7 @@ static void qxl_primary_atomic_update(struct drm_plane *plane, > if (qdev->primary_bo) > qxl_io_destroy_primary(qdev); > qxl_io_create_primary(qdev, primary); > - qxl_primary_apply_cursor(plane); > + qxl_primary_apply_cursor(qdev, plane->state); > } > > if (bo->is_dumb) > @@ -574,124 +680,21 @@ static void qxl_primary_atomic_disable(struct drm_plane *plane, > static void qxl_cursor_atomic_update(struct drm_plane *plane, > struct drm_plane_state *old_state) > { > - struct drm_device *dev = plane->dev; > - struct qxl_device *qdev = to_qxl(dev); > + struct qxl_device *qdev = to_qxl(plane->dev); > struct drm_framebuffer *fb = plane->state->fb; > - struct qxl_crtc *qcrtc = to_qxl_crtc(plane->state->crtc); > - struct qxl_release *release; > - struct qxl_cursor_cmd *cmd; > - struct qxl_cursor *cursor; > - struct drm_gem_object *obj; > - struct qxl_bo *cursor_bo = NULL, *user_bo = NULL, *old_cursor_bo = NULL; > - int ret; > - struct dma_buf_map user_map; > - struct dma_buf_map cursor_map; > - void *user_ptr; > - int size = 64*64*4; > - > - ret = qxl_alloc_release_reserved(qdev, sizeof(*cmd), > - QXL_RELEASE_CURSOR_CMD, > - &release, NULL); > - if (ret) > - return; > > if (fb != old_state->fb) { > - obj = fb->obj[0]; > - user_bo = gem_to_qxl_bo(obj); > - > - /* pinning is done in the prepare/cleanup framevbuffer */ > - ret = qxl_bo_vmap_locked(user_bo, &user_map); > - if (ret) > - goto out_free_release; > - user_ptr = user_map.vaddr; /* TODO: Use mapping abstraction properly */ > - > - ret = qxl_alloc_bo_reserved(qdev, release, > - sizeof(struct qxl_cursor) + size, > - &cursor_bo); > - if (ret) > - goto out_kunmap; > - > - ret = qxl_bo_pin(cursor_bo); > - if (ret) > - goto out_free_bo; > - > - ret = qxl_release_reserve_list(release, true); > - if (ret) > - goto out_unpin; > - > - ret = qxl_bo_vmap_locked(cursor_bo, &cursor_map); > - if (ret) > - goto out_backoff; > - if (cursor_map.is_iomem) /* TODO: Use mapping abstraction properly */ > - cursor = (struct qxl_cursor __force *)cursor_map.vaddr_iomem; > - else > - cursor = (struct qxl_cursor *)cursor_map.vaddr; > - > - cursor->header.unique = 0; > - cursor->header.type = SPICE_CURSOR_TYPE_ALPHA; > - cursor->header.width = 64; > - cursor->header.height = 64; > - cursor->header.hot_spot_x = fb->hot_x; > - cursor->header.hot_spot_y = fb->hot_y; > - cursor->data_size = size; > - cursor->chunk.next_chunk = 0; > - cursor->chunk.prev_chunk = 0; > - cursor->chunk.data_size = size; > - memcpy(cursor->chunk.data, user_ptr, size); > - qxl_bo_vunmap_locked(cursor_bo); > - qxl_bo_vunmap_locked(user_bo); > - > - cmd = (struct qxl_cursor_cmd *) qxl_release_map(qdev, release); > - cmd->u.set.visible = 1; > - cmd->u.set.shape = qxl_bo_physical_address(qdev, > - cursor_bo, 0); > - cmd->type = QXL_CURSOR_SET; > - > - old_cursor_bo = qcrtc->cursor_bo; > - qcrtc->cursor_bo = cursor_bo; > - cursor_bo = NULL; > + qxl_primary_apply_cursor(qdev, plane->state); > } else { > - > - ret = qxl_release_reserve_list(release, true); > - if (ret) > - goto out_free_release; > - > - cmd = (struct qxl_cursor_cmd *) qxl_release_map(qdev, release); > - cmd->type = QXL_CURSOR_MOVE; > + qxl_primary_move_cursor(qdev, plane->state); > } > - > - cmd->u.position.x = plane->state->crtc_x + fb->hot_x; > - cmd->u.position.y = plane->state->crtc_y + fb->hot_y; > - > - qxl_release_unmap(qdev, release, &cmd->release_info); > - qxl_release_fence_buffer_objects(release); > - qxl_push_cursor_ring_release(qdev, release, QXL_CMD_CURSOR, false); > - > - if (old_cursor_bo != NULL) > - qxl_bo_unpin(old_cursor_bo); > - qxl_bo_unref(&old_cursor_bo); > - qxl_bo_unref(&cursor_bo); > - > - return; > - > -out_backoff: > - qxl_release_backoff_reserve_list(release); > -out_unpin: > - qxl_bo_unpin(cursor_bo); > -out_free_bo: > - qxl_bo_unref(&cursor_bo); > -out_kunmap: > - qxl_bo_vunmap_locked(user_bo); > -out_free_release: > - qxl_release_free(qdev, release); > - return; > - > } > > static void qxl_cursor_atomic_disable(struct drm_plane *plane, > struct drm_plane_state *old_state) > { > struct qxl_device *qdev = to_qxl(plane->dev); > + struct qxl_crtc *qcrtc; > struct qxl_release *release; > struct qxl_cursor_cmd *cmd; > int ret; > @@ -714,6 +717,10 @@ static void qxl_cursor_atomic_disable(struct drm_plane *plane, > > qxl_release_fence_buffer_objects(release); > qxl_push_cursor_ring_release(qdev, release, QXL_CMD_CURSOR, false); > + > + qcrtc = to_qxl_crtc(old_state->crtc); > + qxl_free_cursor(qcrtc->cursor_bo); > + qcrtc->cursor_bo = NULL; > } > > static void qxl_update_dumb_head(struct qxl_device *qdev, > @@ -822,6 +829,17 @@ static int qxl_plane_prepare_fb(struct drm_plane *plane, > qxl_prepare_shadow(qdev, user_bo, new_state->crtc->index); > } > > + if (plane->type == DRM_PLANE_TYPE_CURSOR && > + plane->state->fb != new_state->fb) { > + struct qxl_crtc *qcrtc = to_qxl_crtc(new_state->crtc); > + struct qxl_bo *old_cursor_bo = qcrtc->cursor_bo; > + > + qcrtc->cursor_bo = qxl_create_cursor(qdev, user_bo, > + new_state->fb->hot_x, > + new_state->fb->hot_y); > + qxl_free_cursor(old_cursor_bo); > + } > + > return qxl_bo_pin(user_bo); > } > >
Hi, > I'm still trying to wrap my head around the qxl cursor code. > > Getting vmap out of the commit tail is good, but I feel like this isn't > going in the right direction overall. > > In ast, these helper functions were only good when converting the drvier to > atomic modesetting. So I removed them in the latst patchset and did all the > updates in the plane helpers directly. I see the helper functions more as a way to get some structure into the code flow. The callbacks are easier to read if they just call helper functions for stuff which needs more than a handful lines of code (patch 9/11 exists for the same reason). The helpers also make it easier move work from one callback to another, but that is just a useful side-effect. I had considered making that two separate patches, one factor out code into functions and one moving the calls. Turned out to not be that easy though, because the old qxl_cursor_atomic_update() code was a rather hairy mix of qxl_create_cursor() + qxl_primary_apply_cursor() + qxl_primary_move_cursor(). > For cursor_bo itself, it seems to be transitional state that is only used > during the plane update and crtc update . It should probably be stored in a > plane-state structure. > > Some of the primary plane's functions seem to deal with cursor handling. > What's the role of the primary plane in cursor handling? It's a quirk. The qxl device will forget the cursor state on qxl_io_create_primary(), so I have to remember the cursor state and re-establish it by calling qxl_primary_apply_cursor() again. So I'm not sure sticking this into plane state would work. Because of the quirk this is more than just a handover from prepare to commit. > For now, I suggest to merge patch 1 to 8 and 11; and move the cursor patches > into a new patchset. I can merge 1-8, but 11 has to wait until the cursor is sorted. There is a reason why 11 is last in the series ;) > I'd like ot hear Daniel's opinion on this. Do you have > further plans here? Well. I suspect I could easily spend a month cleaning up and party redesign the qxl driver (specifically qxl_draw.c + qxl_image.c). I'm not sure I'll find the time to actually do that anytime soon. I have plenty of other stuff on my TODO list, and given that the world is transitioning to virtio-gpu the priority for qxl isn't that high. So, no, I have no short-term plans for qxl beyond fixing pins + reservations + lockdep. take care, Gerd
Hi Am 18.02.21 um 12:50 schrieb Gerd Hoffmann: > Hi, > >> I'm still trying to wrap my head around the qxl cursor code. >> >> Getting vmap out of the commit tail is good, but I feel like this isn't >> going in the right direction overall. >> >> In ast, these helper functions were only good when converting the drvier to >> atomic modesetting. So I removed them in the latst patchset and did all the >> updates in the plane helpers directly. > > I see the helper functions more as a way to get some structure into the > code flow. The callbacks are easier to read if they just call helper > functions for stuff which needs more than a handful lines of code > (patch 9/11 exists for the same reason). > > The helpers also make it easier move work from one callback to another, > but that is just a useful side-effect. > > I had considered making that two separate patches, one factor out code > into functions and one moving the calls. Turned out to not be that easy > though, because the old qxl_cursor_atomic_update() code was a rather > hairy mix of qxl_create_cursor() + qxl_primary_apply_cursor() + > qxl_primary_move_cursor(). > >> For cursor_bo itself, it seems to be transitional state that is only used >> during the plane update and crtc update . It should probably be stored in a >> plane-state structure. >> >> Some of the primary plane's functions seem to deal with cursor handling. >> What's the role of the primary plane in cursor handling? > > It's a quirk. The qxl device will forget the cursor state on > qxl_io_create_primary(), so I have to remember the cursor state > and re-establish it by calling qxl_primary_apply_cursor() again. > > So I'm not sure sticking this into plane state would work. Because of > the quirk this is more than just a handover from prepare to commit. > >> For now, I suggest to merge patch 1 to 8 and 11; and move the cursor patches >> into a new patchset. > > I can merge 1-8, but 11 has to wait until the cursor is sorted. > There is a reason why 11 is last in the series ;) > >> I'd like ot hear Daniel's opinion on this. Do you have >> further plans here? > > Well. I suspect I could easily spend a month cleaning up and party > redesign the qxl driver (specifically qxl_draw.c + qxl_image.c). > > I'm not sure I'll find the time to actually do that anytime soon. > I have plenty of other stuff on my TODO list, and given that the > world is transitioning to virtio-gpu the priority for qxl isn't > that high. Well, in that case: Acked-by: Thomas Zimmermann <tzimmermann@suse.de> for patches 9 and 10. Having the vmap calls fixed is at least worth it. Best regards Thomas > > So, no, I have no short-term plans for qxl beyond fixing pins + > reservations + lockdep. > > take care, > Gerd > > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel >
Hi, > > Well. I suspect I could easily spend a month cleaning up and party > > redesign the qxl driver (specifically qxl_draw.c + qxl_image.c). > > > > I'm not sure I'll find the time to actually do that anytime soon. > > I have plenty of other stuff on my TODO list, and given that the > > world is transitioning to virtio-gpu the priority for qxl isn't > > that high. > > Well, in that case: > > Acked-by: Thomas Zimmermann <tzimmermann@suse.de> > > for patches 9 and 10. Having the vmap calls fixed is at least worth it. Thanks. Any chance you can ack 8/11 too (only patch missing an ack). take care, Gerd
diff --git a/drivers/gpu/drm/qxl/qxl_display.c b/drivers/gpu/drm/qxl/qxl_display.c index b315d7484e21..4a3d272e8d6c 100644 --- a/drivers/gpu/drm/qxl/qxl_display.c +++ b/drivers/gpu/drm/qxl/qxl_display.c @@ -476,12 +476,11 @@ static int qxl_primary_atomic_check(struct drm_plane *plane, return qxl_check_framebuffer(qdev, bo); } -static int qxl_primary_apply_cursor(struct drm_plane *plane) +static int qxl_primary_apply_cursor(struct qxl_device *qdev, + struct drm_plane_state *plane_state) { - struct drm_device *dev = plane->dev; - struct qxl_device *qdev = to_qxl(dev); - struct drm_framebuffer *fb = plane->state->fb; - struct qxl_crtc *qcrtc = to_qxl_crtc(plane->state->crtc); + struct drm_framebuffer *fb = plane_state->fb; + struct qxl_crtc *qcrtc = to_qxl_crtc(plane_state->crtc); struct qxl_cursor_cmd *cmd; struct qxl_release *release; int ret = 0; @@ -505,8 +504,8 @@ static int qxl_primary_apply_cursor(struct drm_plane *plane) cmd = (struct qxl_cursor_cmd *)qxl_release_map(qdev, release); cmd->type = QXL_CURSOR_SET; - cmd->u.set.position.x = plane->state->crtc_x + fb->hot_x; - cmd->u.set.position.y = plane->state->crtc_y + fb->hot_y; + cmd->u.set.position.x = plane_state->crtc_x + fb->hot_x; + cmd->u.set.position.y = plane_state->crtc_y + fb->hot_y; cmd->u.set.shape = qxl_bo_physical_address(qdev, qcrtc->cursor_bo, 0); @@ -523,6 +522,113 @@ static int qxl_primary_apply_cursor(struct drm_plane *plane) return ret; } +static int qxl_primary_move_cursor(struct qxl_device *qdev, + struct drm_plane_state *plane_state) +{ + struct drm_framebuffer *fb = plane_state->fb; + struct qxl_crtc *qcrtc = to_qxl_crtc(plane_state->crtc); + struct qxl_cursor_cmd *cmd; + struct qxl_release *release; + int ret = 0; + + if (!qcrtc->cursor_bo) + return 0; + + 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_MOVE; + cmd->u.position.x = plane_state->crtc_x + fb->hot_x; + cmd->u.position.y = plane_state->crtc_y + fb->hot_y; + qxl_release_unmap(qdev, release, &cmd->release_info); + + qxl_release_fence_buffer_objects(release); + qxl_push_cursor_ring_release(qdev, release, QXL_CMD_CURSOR, false); + return ret; +} + +static struct qxl_bo *qxl_create_cursor(struct qxl_device *qdev, + struct qxl_bo *user_bo, + int hot_x, int hot_y) +{ + static const u32 size = 64 * 64 * 4; + struct qxl_bo *cursor_bo; + struct dma_buf_map cursor_map; + struct dma_buf_map user_map; + struct qxl_cursor cursor; + int ret; + + if (!user_bo) + return NULL; + + ret = qxl_bo_create(qdev, sizeof(struct qxl_cursor) + size, + false, true, QXL_GEM_DOMAIN_VRAM, 1, + NULL, &cursor_bo); + if (ret) + goto err; + + ret = qxl_bo_vmap(cursor_bo, &cursor_map); + if (ret) + goto err_unref; + + ret = qxl_bo_vmap(user_bo, &user_map); + if (ret) + goto err_unmap; + + cursor.header.unique = 0; + cursor.header.type = SPICE_CURSOR_TYPE_ALPHA; + cursor.header.width = 64; + cursor.header.height = 64; + cursor.header.hot_spot_x = hot_x; + cursor.header.hot_spot_y = hot_y; + cursor.data_size = size; + cursor.chunk.next_chunk = 0; + cursor.chunk.prev_chunk = 0; + cursor.chunk.data_size = size; + if (cursor_map.is_iomem) { + memcpy_toio(cursor_map.vaddr_iomem, + &cursor, sizeof(cursor)); + memcpy_toio(cursor_map.vaddr_iomem + sizeof(cursor), + user_map.vaddr, size); + } else { + memcpy(cursor_map.vaddr, + &cursor, sizeof(cursor)); + memcpy(cursor_map.vaddr + sizeof(cursor), + user_map.vaddr, size); + } + + qxl_bo_vunmap(user_bo); + qxl_bo_vunmap(cursor_bo); + return cursor_bo; + +err_unmap: + qxl_bo_vunmap(cursor_bo); +err_unref: + qxl_bo_unpin(cursor_bo); + qxl_bo_unref(&cursor_bo); +err: + return NULL; +} + +static void qxl_free_cursor(struct qxl_bo *cursor_bo) +{ + if (!cursor_bo) + return; + + qxl_bo_unpin(cursor_bo); + qxl_bo_unref(&cursor_bo); +} + static void qxl_primary_atomic_update(struct drm_plane *plane, struct drm_plane_state *old_state) { @@ -543,7 +649,7 @@ static void qxl_primary_atomic_update(struct drm_plane *plane, if (qdev->primary_bo) qxl_io_destroy_primary(qdev); qxl_io_create_primary(qdev, primary); - qxl_primary_apply_cursor(plane); + qxl_primary_apply_cursor(qdev, plane->state); } if (bo->is_dumb) @@ -574,124 +680,21 @@ static void qxl_primary_atomic_disable(struct drm_plane *plane, static void qxl_cursor_atomic_update(struct drm_plane *plane, struct drm_plane_state *old_state) { - struct drm_device *dev = plane->dev; - struct qxl_device *qdev = to_qxl(dev); + struct qxl_device *qdev = to_qxl(plane->dev); struct drm_framebuffer *fb = plane->state->fb; - struct qxl_crtc *qcrtc = to_qxl_crtc(plane->state->crtc); - struct qxl_release *release; - struct qxl_cursor_cmd *cmd; - struct qxl_cursor *cursor; - struct drm_gem_object *obj; - struct qxl_bo *cursor_bo = NULL, *user_bo = NULL, *old_cursor_bo = NULL; - int ret; - struct dma_buf_map user_map; - struct dma_buf_map cursor_map; - void *user_ptr; - int size = 64*64*4; - - ret = qxl_alloc_release_reserved(qdev, sizeof(*cmd), - QXL_RELEASE_CURSOR_CMD, - &release, NULL); - if (ret) - return; if (fb != old_state->fb) { - obj = fb->obj[0]; - user_bo = gem_to_qxl_bo(obj); - - /* pinning is done in the prepare/cleanup framevbuffer */ - ret = qxl_bo_vmap_locked(user_bo, &user_map); - if (ret) - goto out_free_release; - user_ptr = user_map.vaddr; /* TODO: Use mapping abstraction properly */ - - ret = qxl_alloc_bo_reserved(qdev, release, - sizeof(struct qxl_cursor) + size, - &cursor_bo); - if (ret) - goto out_kunmap; - - ret = qxl_bo_pin(cursor_bo); - if (ret) - goto out_free_bo; - - ret = qxl_release_reserve_list(release, true); - if (ret) - goto out_unpin; - - ret = qxl_bo_vmap_locked(cursor_bo, &cursor_map); - if (ret) - goto out_backoff; - if (cursor_map.is_iomem) /* TODO: Use mapping abstraction properly */ - cursor = (struct qxl_cursor __force *)cursor_map.vaddr_iomem; - else - cursor = (struct qxl_cursor *)cursor_map.vaddr; - - cursor->header.unique = 0; - cursor->header.type = SPICE_CURSOR_TYPE_ALPHA; - cursor->header.width = 64; - cursor->header.height = 64; - cursor->header.hot_spot_x = fb->hot_x; - cursor->header.hot_spot_y = fb->hot_y; - cursor->data_size = size; - cursor->chunk.next_chunk = 0; - cursor->chunk.prev_chunk = 0; - cursor->chunk.data_size = size; - memcpy(cursor->chunk.data, user_ptr, size); - qxl_bo_vunmap_locked(cursor_bo); - qxl_bo_vunmap_locked(user_bo); - - cmd = (struct qxl_cursor_cmd *) qxl_release_map(qdev, release); - cmd->u.set.visible = 1; - cmd->u.set.shape = qxl_bo_physical_address(qdev, - cursor_bo, 0); - cmd->type = QXL_CURSOR_SET; - - old_cursor_bo = qcrtc->cursor_bo; - qcrtc->cursor_bo = cursor_bo; - cursor_bo = NULL; + qxl_primary_apply_cursor(qdev, plane->state); } else { - - ret = qxl_release_reserve_list(release, true); - if (ret) - goto out_free_release; - - cmd = (struct qxl_cursor_cmd *) qxl_release_map(qdev, release); - cmd->type = QXL_CURSOR_MOVE; + qxl_primary_move_cursor(qdev, plane->state); } - - cmd->u.position.x = plane->state->crtc_x + fb->hot_x; - cmd->u.position.y = plane->state->crtc_y + fb->hot_y; - - qxl_release_unmap(qdev, release, &cmd->release_info); - qxl_release_fence_buffer_objects(release); - qxl_push_cursor_ring_release(qdev, release, QXL_CMD_CURSOR, false); - - if (old_cursor_bo != NULL) - qxl_bo_unpin(old_cursor_bo); - qxl_bo_unref(&old_cursor_bo); - qxl_bo_unref(&cursor_bo); - - return; - -out_backoff: - qxl_release_backoff_reserve_list(release); -out_unpin: - qxl_bo_unpin(cursor_bo); -out_free_bo: - qxl_bo_unref(&cursor_bo); -out_kunmap: - qxl_bo_vunmap_locked(user_bo); -out_free_release: - qxl_release_free(qdev, release); - return; - } static void qxl_cursor_atomic_disable(struct drm_plane *plane, struct drm_plane_state *old_state) { struct qxl_device *qdev = to_qxl(plane->dev); + struct qxl_crtc *qcrtc; struct qxl_release *release; struct qxl_cursor_cmd *cmd; int ret; @@ -714,6 +717,10 @@ static void qxl_cursor_atomic_disable(struct drm_plane *plane, qxl_release_fence_buffer_objects(release); qxl_push_cursor_ring_release(qdev, release, QXL_CMD_CURSOR, false); + + qcrtc = to_qxl_crtc(old_state->crtc); + qxl_free_cursor(qcrtc->cursor_bo); + qcrtc->cursor_bo = NULL; } static void qxl_update_dumb_head(struct qxl_device *qdev, @@ -822,6 +829,17 @@ static int qxl_plane_prepare_fb(struct drm_plane *plane, qxl_prepare_shadow(qdev, user_bo, new_state->crtc->index); } + if (plane->type == DRM_PLANE_TYPE_CURSOR && + plane->state->fb != new_state->fb) { + struct qxl_crtc *qcrtc = to_qxl_crtc(new_state->crtc); + struct qxl_bo *old_cursor_bo = qcrtc->cursor_bo; + + qcrtc->cursor_bo = qxl_create_cursor(qdev, user_bo, + new_state->fb->hot_x, + new_state->fb->hot_y); + qxl_free_cursor(old_cursor_bo); + } + return qxl_bo_pin(user_bo); }
Add helper functions to create and move the cursor. Create the cursor_bo in prepare_fb callback, in the atomic_commit callback we only send the update command to the host. Signed-off-by: Gerd Hoffmann <kraxel@redhat.com> --- drivers/gpu/drm/qxl/qxl_display.c | 248 ++++++++++++++++-------------- 1 file changed, 133 insertions(+), 115 deletions(-)