All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] drm/qxl: reapply cursor after resetting primary
@ 2017-11-27 21:07 Ray Strode
  2017-11-27 21:42 ` Ray Strode
  2017-11-27 21:50 ` [PATCH v2 0/2] qxl cursor fixes Ray Strode
  0 siblings, 2 replies; 5+ messages in thread
From: Ray Strode @ 2017-11-27 21:07 UTC (permalink / raw)
  To: dri-devel; +Cc: Dave Airlie, Ray Strode, Gerd Hoffmann

From: Ray Strode <rstrode@redhat.com>

QXL associates mouse state with its primary plane.

Destroying a primary plane and putting a new one in place has the side
effect of destroying the cursor as well.

This commit changes the driver to reapply the cursor any time a new
primary is created. It achieves this by keeping a reference to the
cursor bo on the qxl_crtc struct.

This fix is very similar to

commit 4532b241a4b7 ("drm/qxl: reapply cursor after SetCrtc calls")

which got implicitly reverted as part of implementing the atomic
modeset feature.

Cc: Gerd Hoffmann <kraxel@redhat.com>
Cc: Dave Airlie <airlied@redhat.com>
Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1512097
Fixes: 1277eed5fecb ("drm: qxl: Atomic phase 1: convert cursor to universal plane")
Signed-off-by: Ray Strode <rstrode@redhat.com>
---
 drivers/gpu/drm/qxl/qxl_display.c | 59 +++++++++++++++++++++++++++++++++++++++
 drivers/gpu/drm/qxl/qxl_drv.h     |  2 ++
 2 files changed, 61 insertions(+)

diff --git a/drivers/gpu/drm/qxl/qxl_display.c b/drivers/gpu/drm/qxl/qxl_display.c
index fc9e86f3811f..a51a5abb977d 100644
--- a/drivers/gpu/drm/qxl/qxl_display.c
+++ b/drivers/gpu/drm/qxl/qxl_display.c
@@ -262,60 +262,61 @@ static int qxl_add_common_modes(struct drm_connector *connector,
 		mode = drm_cvt_mode(dev, common_modes[i].w, common_modes[i].h,
 				    60, false, false, false);
 		if (common_modes[i].w == pwidth && common_modes[i].h == pheight)
 			mode->type |= DRM_MODE_TYPE_PREFERRED;
 		drm_mode_probed_add(connector, mode);
 	}
 	return i - 1;
 }
 
 static void qxl_crtc_atomic_flush(struct drm_crtc *crtc,
 				  struct drm_crtc_state *old_crtc_state)
 {
 	struct drm_device *dev = crtc->dev;
 	struct drm_pending_vblank_event *event;
 	unsigned long flags;
 
 	if (crtc->state && crtc->state->event) {
 		event = crtc->state->event;
 		crtc->state->event = NULL;
 
 		spin_lock_irqsave(&dev->event_lock, flags);
 		drm_crtc_send_vblank_event(crtc, event);
 		spin_unlock_irqrestore(&dev->event_lock, flags);
 	}
 }
 
 static void qxl_crtc_destroy(struct drm_crtc *crtc)
 {
 	struct qxl_crtc *qxl_crtc = to_qxl_crtc(crtc);
 
+	qxl_bo_unref(&qxl_crtc->cursor_bo);
 	drm_crtc_cleanup(crtc);
 	kfree(qxl_crtc);
 }
 
 static const struct drm_crtc_funcs qxl_crtc_funcs = {
 	.set_config = drm_atomic_helper_set_config,
 	.destroy = qxl_crtc_destroy,
 	.page_flip = drm_atomic_helper_page_flip,
 	.reset = drm_atomic_helper_crtc_reset,
 	.atomic_duplicate_state = drm_atomic_helper_crtc_duplicate_state,
 	.atomic_destroy_state = drm_atomic_helper_crtc_destroy_state,
 };
 
 void qxl_user_framebuffer_destroy(struct drm_framebuffer *fb)
 {
 	struct qxl_framebuffer *qxl_fb = to_qxl_framebuffer(fb);
 	struct qxl_bo *bo = gem_to_qxl_bo(qxl_fb->obj);
 
 	WARN_ON(bo->shadow);
 	drm_gem_object_unreference_unlocked(qxl_fb->obj);
 	drm_framebuffer_cleanup(fb);
 	kfree(qxl_fb);
 }
 
 static int qxl_framebuffer_surface_dirty(struct drm_framebuffer *fb,
 					 struct drm_file *file_priv,
 					 unsigned flags, unsigned color,
 					 struct drm_clip_rect *clips,
 					 unsigned num_clips)
 {
@@ -468,193 +469,251 @@ static void qxl_crtc_atomic_disable(struct drm_crtc *crtc,
 
 static const struct drm_crtc_helper_funcs qxl_crtc_helper_funcs = {
 	.mode_fixup = qxl_crtc_mode_fixup,
 	.mode_set_nofb = qxl_mode_set_nofb,
 	.atomic_flush = qxl_crtc_atomic_flush,
 	.atomic_enable = qxl_crtc_atomic_enable,
 	.atomic_disable = qxl_crtc_atomic_disable,
 };
 
 static int qxl_primary_atomic_check(struct drm_plane *plane,
 				    struct drm_plane_state *state)
 {
 	struct qxl_device *qdev = plane->dev->dev_private;
 	struct qxl_framebuffer *qfb;
 	struct qxl_bo *bo;
 
 	if (!state->crtc || !state->fb)
 		return 0;
 
 	qfb = to_qxl_framebuffer(state->fb);
 	bo = gem_to_qxl_bo(qfb->obj);
 
 	if (bo->surf.stride * bo->surf.height > qdev->vram_size) {
 		DRM_ERROR("Mode doesn't fit in vram size (vgamem)");
 		return -EINVAL;
 	}
 
 	return 0;
 }
 
+static int qxl_primary_apply_cursor(struct drm_plane *plane)
+{
+	struct drm_device *dev = plane->dev;
+	struct qxl_device *qdev = dev->dev_private;
+	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_list_add(release, qcrtc->cursor_bo);
+	if (ret)
+		goto out_free_release;
+
+	ret = qxl_release_reserve_list(release, false);
+	if (ret)
+		goto out_free_release;
+
+	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.shape = qxl_bo_physical_address(qdev, qcrtc->cursor_bo, 0);
+
+	cmd->u.set.visible = 1;
+	qxl_release_unmap(qdev, release, &cmd->release_info);
+
+	qxl_push_cursor_ring_release(qdev, release, QXL_CMD_CURSOR, false);
+	qxl_release_fence_buffer_objects(release);
+
+	return ret;
+
+out_free_release:
+	qxl_release_free(qdev, release);
+	return ret;
+}
+
 static void qxl_primary_atomic_update(struct drm_plane *plane,
 				      struct drm_plane_state *old_state)
 {
 	struct qxl_device *qdev = plane->dev->dev_private;
 	struct qxl_framebuffer *qfb =
 		to_qxl_framebuffer(plane->state->fb);
 	struct qxl_framebuffer *qfb_old;
 	struct qxl_bo *bo = gem_to_qxl_bo(qfb->obj);
 	struct qxl_bo *bo_old;
 	struct drm_clip_rect norect = {
 	    .x1 = 0,
 	    .y1 = 0,
 	    .x2 = qfb->base.width,
 	    .y2 = qfb->base.height
 	};
+	int ret;
 	bool same_shadow = false;
 
 	if (old_state->fb) {
 		qfb_old = to_qxl_framebuffer(old_state->fb);
 		bo_old = gem_to_qxl_bo(qfb_old->obj);
 	} else {
 		bo_old = NULL;
 	}
 
 	if (bo == bo_old)
 		return;
 
 	if (bo_old && bo_old->shadow && bo->shadow &&
 	    bo_old->shadow == bo->shadow) {
 		same_shadow = true;
 	}
 
 	if (bo_old && bo_old->is_primary) {
 		if (!same_shadow)
 			qxl_io_destroy_primary(qdev);
 		bo_old->is_primary = false;
+
+		ret = qxl_primary_apply_cursor(plane);
+		if (ret)
+			DRM_ERROR(
+			"could not set cursor after creating primary");
 	}
 
 	if (!bo->is_primary) {
 		if (!same_shadow)
 			qxl_io_create_primary(qdev, 0, bo);
 		bo->is_primary = true;
 	}
 
 	qxl_draw_dirty_fb(qdev, qfb, bo, 0, 0, &norect, 1, 1);
 }
 
 static void qxl_primary_atomic_disable(struct drm_plane *plane,
 				       struct drm_plane_state *old_state)
 {
 	struct qxl_device *qdev = plane->dev->dev_private;
 
 	if (old_state->fb) {
 		struct qxl_framebuffer *qfb =
 			to_qxl_framebuffer(old_state->fb);
 		struct qxl_bo *bo = gem_to_qxl_bo(qfb->obj);
 
 		if (bo->is_primary) {
 			qxl_io_destroy_primary(qdev);
 			bo->is_primary = false;
 		}
 	}
 }
 
 static int qxl_plane_atomic_check(struct drm_plane *plane,
 				  struct drm_plane_state *state)
 {
 	return 0;
 }
 
 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 = dev->dev_private;
 	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;
 	int ret;
 	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 = to_qxl_framebuffer(fb)->obj;
 		user_bo = gem_to_qxl_bo(obj);
 
 		/* pinning is done in the prepare/cleanup framevbuffer */
 		ret = qxl_bo_kmap(user_bo, &user_ptr);
 		if (ret)
 			goto out_free_release;
 
 		ret = qxl_alloc_bo_reserved(qdev, release,
 					    sizeof(struct qxl_cursor) + size,
 					    &cursor_bo);
 		if (ret)
 			goto out_kunmap;
 
 		ret = qxl_release_reserve_list(release, true);
 		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;
 		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_kunmap(cursor_bo);
 		qxl_bo_kunmap(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;
+
+		qxl_bo_unref(&qcrtc->cursor_bo);
+		qcrtc->cursor_bo = cursor_bo;
+		cursor_bo = NULL;
 	} 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;
 	}
 
 	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_push_cursor_ring_release(qdev, release, QXL_CMD_CURSOR, false);
 	qxl_release_fence_buffer_objects(release);
 
 	qxl_bo_unref (&cursor_bo);
 
 	return;
 
 out_backoff:
 	qxl_release_backoff_reserve_list(release);
 out_free_bo:
 	qxl_bo_unref(&cursor_bo);
 out_kunmap:
 	qxl_bo_kunmap(user_bo);
 out_free_release:
 	qxl_release_free(qdev, release);
 	return;
diff --git a/drivers/gpu/drm/qxl/qxl_drv.h b/drivers/gpu/drm/qxl/qxl_drv.h
index 08752c0ffb35..00a1a66b052a 100644
--- a/drivers/gpu/drm/qxl/qxl_drv.h
+++ b/drivers/gpu/drm/qxl/qxl_drv.h
@@ -84,60 +84,62 @@ struct qxl_bo {
 	struct ttm_bo_kmap_obj		kmap;
 	unsigned			pin_count;
 	void				*kptr;
 	int                             type;
 
 	/* Constant after initialization */
 	struct drm_gem_object		gem_base;
 	bool is_primary; /* is this now a primary surface */
 	bool is_dumb;
 	struct qxl_bo *shadow;
 	bool hw_surf_alloc;
 	struct qxl_surface surf;
 	uint32_t surface_id;
 	struct qxl_release *surf_create;
 };
 #define gem_to_qxl_bo(gobj) container_of((gobj), struct qxl_bo, gem_base)
 #define to_qxl_bo(tobj) container_of((tobj), struct qxl_bo, tbo)
 
 struct qxl_gem {
 	struct mutex		mutex;
 	struct list_head	objects;
 };
 
 struct qxl_bo_list {
 	struct ttm_validate_buffer tv;
 };
 
 struct qxl_crtc {
 	struct drm_crtc base;
 	int index;
+
+	struct qxl_bo *cursor_bo;
 };
 
 struct qxl_output {
 	int index;
 	struct drm_connector base;
 	struct drm_encoder enc;
 };
 
 struct qxl_framebuffer {
 	struct drm_framebuffer base;
 	struct drm_gem_object *obj;
 };
 
 #define to_qxl_crtc(x) container_of(x, struct qxl_crtc, base)
 #define drm_connector_to_qxl_output(x) container_of(x, struct qxl_output, base)
 #define drm_encoder_to_qxl_output(x) container_of(x, struct qxl_output, enc)
 #define to_qxl_framebuffer(x) container_of(x, struct qxl_framebuffer, base)
 
 struct qxl_mman {
 	struct ttm_bo_global_ref        bo_global_ref;
 	struct drm_global_reference	mem_global_ref;
 	bool				mem_global_referenced;
 	struct ttm_bo_device		bdev;
 };
 
 struct qxl_mode_info {
 	bool mode_config_initialized;
 
 	/* pointer to fbdev info structure */
 	struct qxl_fbdev *qfbdev;
-- 
2.14.3

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

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH] drm/qxl: reapply cursor after resetting primary
  2017-11-27 21:07 [PATCH] drm/qxl: reapply cursor after resetting primary Ray Strode
@ 2017-11-27 21:42 ` Ray Strode
  2017-11-27 21:50 ` [PATCH v2 0/2] qxl cursor fixes Ray Strode
  1 sibling, 0 replies; 5+ messages in thread
From: Ray Strode @ 2017-11-27 21:42 UTC (permalink / raw)
  To: dri-devel

Hi,

On Mon, Nov 27, 2017 at 4:07 PM Ray Strode <halfline@gmail.com> wrote:
...
> This commit changes the driver to reapply the cursor any time a new
> primary is created. It achieves this by keeping a reference to the
> cursor bo on the qxl_crtc struct.
So i forgot this is on top of a small leak fix, too.

For clarity, I'll resend with both patches in the series.

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

^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH v2 0/2] qxl cursor fixes
  2017-11-27 21:07 [PATCH] drm/qxl: reapply cursor after resetting primary Ray Strode
  2017-11-27 21:42 ` Ray Strode
@ 2017-11-27 21:50 ` Ray Strode
  2017-11-27 21:50   ` [PATCH 1/2] drm/qxl: unref cursor bo when finished with it Ray Strode
  2017-11-27 21:50   ` [PATCH 2/2] drm/qxl: reapply cursor after resetting primary Ray Strode
  1 sibling, 2 replies; 5+ messages in thread
From: Ray Strode @ 2017-11-27 21:50 UTC (permalink / raw)
  To: dri-devel; +Cc: Ray Strode

From: Ray Strode <rstrode@redhat.com>

This series includes a small leak fix and a fix for an initial invisible
cursor on wayland sessions.

Ray Strode (2):
  drm/qxl: unref cursor bo when finished with it
  drm/qxl: reapply cursor after resetting primary

 drivers/gpu/drm/qxl/qxl_display.c | 63 ++++++++++++++++++++++++++++++++++++++-
 drivers/gpu/drm/qxl/qxl_drv.h     |  2 ++
 2 files changed, 64 insertions(+), 1 deletion(-)

-- 
2.14.3

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

^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH 1/2] drm/qxl: unref cursor bo when finished with it
  2017-11-27 21:50 ` [PATCH v2 0/2] qxl cursor fixes Ray Strode
@ 2017-11-27 21:50   ` Ray Strode
  2017-11-27 21:50   ` [PATCH 2/2] drm/qxl: reapply cursor after resetting primary Ray Strode
  1 sibling, 0 replies; 5+ messages in thread
From: Ray Strode @ 2017-11-27 21:50 UTC (permalink / raw)
  To: dri-devel; +Cc: Ray Strode

From: Ray Strode <rstrode@redhat.com>

qxl_cursor_atomic_update allocs a bo for the cursor that
it never frees up at the end of the function.

This commit fixes that.

Signed-off-by: Ray Strode <rstrode@redhat.com>
---
 drivers/gpu/drm/qxl/qxl_display.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/qxl/qxl_display.c b/drivers/gpu/drm/qxl/qxl_display.c
index 4756b3c9bf2c..7335d99244d5 100644
--- a/drivers/gpu/drm/qxl/qxl_display.c
+++ b/drivers/gpu/drm/qxl/qxl_display.c
@@ -548,61 +548,61 @@ static void qxl_primary_atomic_disable(struct drm_plane *plane,
 	struct qxl_device *qdev = plane->dev->dev_private;
 
 	if (old_state->fb) {
 		struct qxl_framebuffer *qfb =
 			to_qxl_framebuffer(old_state->fb);
 		struct qxl_bo *bo = gem_to_qxl_bo(qfb->obj);
 
 		if (bo->is_primary) {
 			qxl_io_destroy_primary(qdev);
 			bo->is_primary = false;
 		}
 	}
 }
 
 static int qxl_plane_atomic_check(struct drm_plane *plane,
 				  struct drm_plane_state *state)
 {
 	return 0;
 }
 
 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 = dev->dev_private;
 	struct drm_framebuffer *fb = plane->state->fb;
 	struct qxl_release *release;
 	struct qxl_cursor_cmd *cmd;
 	struct qxl_cursor *cursor;
 	struct drm_gem_object *obj;
-	struct qxl_bo *cursor_bo, *user_bo = NULL;
+	struct qxl_bo *cursor_bo = NULL, *user_bo = NULL;
 	int ret;
 	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 = to_qxl_framebuffer(fb)->obj;
 		user_bo = gem_to_qxl_bo(obj);
 
 		/* pinning is done in the prepare/cleanup framevbuffer */
 		ret = qxl_bo_kmap(user_bo, &user_ptr);
 		if (ret)
 			goto out_free_release;
 
 		ret = qxl_alloc_bo_reserved(qdev, release,
 					    sizeof(struct qxl_cursor) + size,
 					    &cursor_bo);
 		if (ret)
 			goto out_kunmap;
 
 		ret = qxl_release_reserve_list(release, true);
 		if (ret)
 			goto out_free_bo;
 
 		ret = qxl_bo_kmap(cursor_bo, (void **)&cursor);
@@ -618,60 +618,62 @@ static void qxl_cursor_atomic_update(struct drm_plane *plane,
 		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_kunmap(cursor_bo);
 		qxl_bo_kunmap(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;
 	} 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;
 	}
 
 	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_push_cursor_ring_release(qdev, release, QXL_CMD_CURSOR, false);
 	qxl_release_fence_buffer_objects(release);
 
+	qxl_bo_unref(&cursor_bo);
+
 	return;
 
 out_backoff:
 	qxl_release_backoff_reserve_list(release);
 out_free_bo:
 	qxl_bo_unref(&cursor_bo);
 out_kunmap:
 	qxl_bo_kunmap(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 = plane->dev->dev_private;
 	struct qxl_release *release;
 	struct qxl_cursor_cmd *cmd;
 	int ret;
 
 	ret = qxl_alloc_release_reserved(qdev, sizeof(*cmd),
 					 QXL_RELEASE_CURSOR_CMD,
 					 &release, NULL);
 	if (ret)
 		return;
 
 	ret = qxl_release_reserve_list(release, true);
 	if (ret) {
-- 
2.14.3

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

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [PATCH 2/2] drm/qxl: reapply cursor after resetting primary
  2017-11-27 21:50 ` [PATCH v2 0/2] qxl cursor fixes Ray Strode
  2017-11-27 21:50   ` [PATCH 1/2] drm/qxl: unref cursor bo when finished with it Ray Strode
@ 2017-11-27 21:50   ` Ray Strode
  1 sibling, 0 replies; 5+ messages in thread
From: Ray Strode @ 2017-11-27 21:50 UTC (permalink / raw)
  To: dri-devel; +Cc: Dave Airlie, Ray Strode, Gerd Hoffmann

From: Ray Strode <rstrode@redhat.com>

QXL associates mouse state with its primary plane.

Destroying a primary plane and putting a new one in place has the side
effect of destroying the cursor as well.

This commit changes the driver to reapply the cursor any time a new
primary is created. It achieves this by keeping a reference to the
cursor bo on the qxl_crtc struct.

This fix is very similar to

commit 4532b241a4b7 ("drm/qxl: reapply cursor after SetCrtc calls")

which got implicitly reverted as part of implementing the atomic
modeset feature.

Cc: Gerd Hoffmann <kraxel@redhat.com>
Cc: Dave Airlie <airlied@redhat.com>
Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1512097
Fixes: 1277eed5fecb ("drm: qxl: Atomic phase 1: convert cursor to universal plane")
Signed-off-by: Ray Strode <rstrode@redhat.com>
---
 drivers/gpu/drm/qxl/qxl_display.c | 59 +++++++++++++++++++++++++++++++++++++++
 drivers/gpu/drm/qxl/qxl_drv.h     |  2 ++
 2 files changed, 61 insertions(+)

diff --git a/drivers/gpu/drm/qxl/qxl_display.c b/drivers/gpu/drm/qxl/qxl_display.c
index 7335d99244d5..9a9214ae0fb5 100644
--- a/drivers/gpu/drm/qxl/qxl_display.c
+++ b/drivers/gpu/drm/qxl/qxl_display.c
@@ -262,60 +262,61 @@ static int qxl_add_common_modes(struct drm_connector *connector,
 		mode = drm_cvt_mode(dev, common_modes[i].w, common_modes[i].h,
 				    60, false, false, false);
 		if (common_modes[i].w == pwidth && common_modes[i].h == pheight)
 			mode->type |= DRM_MODE_TYPE_PREFERRED;
 		drm_mode_probed_add(connector, mode);
 	}
 	return i - 1;
 }
 
 static void qxl_crtc_atomic_flush(struct drm_crtc *crtc,
 				  struct drm_crtc_state *old_crtc_state)
 {
 	struct drm_device *dev = crtc->dev;
 	struct drm_pending_vblank_event *event;
 	unsigned long flags;
 
 	if (crtc->state && crtc->state->event) {
 		event = crtc->state->event;
 		crtc->state->event = NULL;
 
 		spin_lock_irqsave(&dev->event_lock, flags);
 		drm_crtc_send_vblank_event(crtc, event);
 		spin_unlock_irqrestore(&dev->event_lock, flags);
 	}
 }
 
 static void qxl_crtc_destroy(struct drm_crtc *crtc)
 {
 	struct qxl_crtc *qxl_crtc = to_qxl_crtc(crtc);
 
+	qxl_bo_unref(&qxl_crtc->cursor_bo);
 	drm_crtc_cleanup(crtc);
 	kfree(qxl_crtc);
 }
 
 static const struct drm_crtc_funcs qxl_crtc_funcs = {
 	.set_config = drm_atomic_helper_set_config,
 	.destroy = qxl_crtc_destroy,
 	.page_flip = drm_atomic_helper_page_flip,
 	.reset = drm_atomic_helper_crtc_reset,
 	.atomic_duplicate_state = drm_atomic_helper_crtc_duplicate_state,
 	.atomic_destroy_state = drm_atomic_helper_crtc_destroy_state,
 };
 
 void qxl_user_framebuffer_destroy(struct drm_framebuffer *fb)
 {
 	struct qxl_framebuffer *qxl_fb = to_qxl_framebuffer(fb);
 	struct qxl_bo *bo = gem_to_qxl_bo(qxl_fb->obj);
 
 	WARN_ON(bo->shadow);
 	drm_gem_object_unreference_unlocked(qxl_fb->obj);
 	drm_framebuffer_cleanup(fb);
 	kfree(qxl_fb);
 }
 
 static int qxl_framebuffer_surface_dirty(struct drm_framebuffer *fb,
 					 struct drm_file *file_priv,
 					 unsigned flags, unsigned color,
 					 struct drm_clip_rect *clips,
 					 unsigned num_clips)
 {
@@ -468,193 +469,251 @@ static void qxl_crtc_atomic_disable(struct drm_crtc *crtc,
 
 static const struct drm_crtc_helper_funcs qxl_crtc_helper_funcs = {
 	.mode_fixup = qxl_crtc_mode_fixup,
 	.mode_set_nofb = qxl_mode_set_nofb,
 	.atomic_flush = qxl_crtc_atomic_flush,
 	.atomic_enable = qxl_crtc_atomic_enable,
 	.atomic_disable = qxl_crtc_atomic_disable,
 };
 
 static int qxl_primary_atomic_check(struct drm_plane *plane,
 				    struct drm_plane_state *state)
 {
 	struct qxl_device *qdev = plane->dev->dev_private;
 	struct qxl_framebuffer *qfb;
 	struct qxl_bo *bo;
 
 	if (!state->crtc || !state->fb)
 		return 0;
 
 	qfb = to_qxl_framebuffer(state->fb);
 	bo = gem_to_qxl_bo(qfb->obj);
 
 	if (bo->surf.stride * bo->surf.height > qdev->vram_size) {
 		DRM_ERROR("Mode doesn't fit in vram size (vgamem)");
 		return -EINVAL;
 	}
 
 	return 0;
 }
 
+static int qxl_primary_apply_cursor(struct drm_plane *plane)
+{
+	struct drm_device *dev = plane->dev;
+	struct qxl_device *qdev = dev->dev_private;
+	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_list_add(release, qcrtc->cursor_bo);
+	if (ret)
+		goto out_free_release;
+
+	ret = qxl_release_reserve_list(release, false);
+	if (ret)
+		goto out_free_release;
+
+	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.shape = qxl_bo_physical_address(qdev, qcrtc->cursor_bo, 0);
+
+	cmd->u.set.visible = 1;
+	qxl_release_unmap(qdev, release, &cmd->release_info);
+
+	qxl_push_cursor_ring_release(qdev, release, QXL_CMD_CURSOR, false);
+	qxl_release_fence_buffer_objects(release);
+
+	return ret;
+
+out_free_release:
+	qxl_release_free(qdev, release);
+	return ret;
+}
+
 static void qxl_primary_atomic_update(struct drm_plane *plane,
 				      struct drm_plane_state *old_state)
 {
 	struct qxl_device *qdev = plane->dev->dev_private;
 	struct qxl_framebuffer *qfb =
 		to_qxl_framebuffer(plane->state->fb);
 	struct qxl_framebuffer *qfb_old;
 	struct qxl_bo *bo = gem_to_qxl_bo(qfb->obj);
 	struct qxl_bo *bo_old;
 	struct drm_clip_rect norect = {
 	    .x1 = 0,
 	    .y1 = 0,
 	    .x2 = qfb->base.width,
 	    .y2 = qfb->base.height
 	};
+	int ret;
 	bool same_shadow = false;
 
 	if (old_state->fb) {
 		qfb_old = to_qxl_framebuffer(old_state->fb);
 		bo_old = gem_to_qxl_bo(qfb_old->obj);
 	} else {
 		bo_old = NULL;
 	}
 
 	if (bo == bo_old)
 		return;
 
 	if (bo_old && bo_old->shadow && bo->shadow &&
 	    bo_old->shadow == bo->shadow) {
 		same_shadow = true;
 	}
 
 	if (bo_old && bo_old->is_primary) {
 		if (!same_shadow)
 			qxl_io_destroy_primary(qdev);
 		bo_old->is_primary = false;
+
+		ret = qxl_primary_apply_cursor(plane);
+		if (ret)
+			DRM_ERROR(
+			"could not set cursor after creating primary");
 	}
 
 	if (!bo->is_primary) {
 		if (!same_shadow)
 			qxl_io_create_primary(qdev, 0, bo);
 		bo->is_primary = true;
 	}
 
 	qxl_draw_dirty_fb(qdev, qfb, bo, 0, 0, &norect, 1, 1);
 }
 
 static void qxl_primary_atomic_disable(struct drm_plane *plane,
 				       struct drm_plane_state *old_state)
 {
 	struct qxl_device *qdev = plane->dev->dev_private;
 
 	if (old_state->fb) {
 		struct qxl_framebuffer *qfb =
 			to_qxl_framebuffer(old_state->fb);
 		struct qxl_bo *bo = gem_to_qxl_bo(qfb->obj);
 
 		if (bo->is_primary) {
 			qxl_io_destroy_primary(qdev);
 			bo->is_primary = false;
 		}
 	}
 }
 
 static int qxl_plane_atomic_check(struct drm_plane *plane,
 				  struct drm_plane_state *state)
 {
 	return 0;
 }
 
 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 = dev->dev_private;
 	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;
 	int ret;
 	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 = to_qxl_framebuffer(fb)->obj;
 		user_bo = gem_to_qxl_bo(obj);
 
 		/* pinning is done in the prepare/cleanup framevbuffer */
 		ret = qxl_bo_kmap(user_bo, &user_ptr);
 		if (ret)
 			goto out_free_release;
 
 		ret = qxl_alloc_bo_reserved(qdev, release,
 					    sizeof(struct qxl_cursor) + size,
 					    &cursor_bo);
 		if (ret)
 			goto out_kunmap;
 
 		ret = qxl_release_reserve_list(release, true);
 		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;
 		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_kunmap(cursor_bo);
 		qxl_bo_kunmap(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;
+
+		qxl_bo_unref(&qcrtc->cursor_bo);
+		qcrtc->cursor_bo = cursor_bo;
+		cursor_bo = NULL;
 	} 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;
 	}
 
 	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_push_cursor_ring_release(qdev, release, QXL_CMD_CURSOR, false);
 	qxl_release_fence_buffer_objects(release);
 
 	qxl_bo_unref(&cursor_bo);
 
 	return;
 
 out_backoff:
 	qxl_release_backoff_reserve_list(release);
 out_free_bo:
 	qxl_bo_unref(&cursor_bo);
 out_kunmap:
 	qxl_bo_kunmap(user_bo);
 out_free_release:
 	qxl_release_free(qdev, release);
 	return;
diff --git a/drivers/gpu/drm/qxl/qxl_drv.h b/drivers/gpu/drm/qxl/qxl_drv.h
index 08752c0ffb35..00a1a66b052a 100644
--- a/drivers/gpu/drm/qxl/qxl_drv.h
+++ b/drivers/gpu/drm/qxl/qxl_drv.h
@@ -84,60 +84,62 @@ struct qxl_bo {
 	struct ttm_bo_kmap_obj		kmap;
 	unsigned			pin_count;
 	void				*kptr;
 	int                             type;
 
 	/* Constant after initialization */
 	struct drm_gem_object		gem_base;
 	bool is_primary; /* is this now a primary surface */
 	bool is_dumb;
 	struct qxl_bo *shadow;
 	bool hw_surf_alloc;
 	struct qxl_surface surf;
 	uint32_t surface_id;
 	struct qxl_release *surf_create;
 };
 #define gem_to_qxl_bo(gobj) container_of((gobj), struct qxl_bo, gem_base)
 #define to_qxl_bo(tobj) container_of((tobj), struct qxl_bo, tbo)
 
 struct qxl_gem {
 	struct mutex		mutex;
 	struct list_head	objects;
 };
 
 struct qxl_bo_list {
 	struct ttm_validate_buffer tv;
 };
 
 struct qxl_crtc {
 	struct drm_crtc base;
 	int index;
+
+	struct qxl_bo *cursor_bo;
 };
 
 struct qxl_output {
 	int index;
 	struct drm_connector base;
 	struct drm_encoder enc;
 };
 
 struct qxl_framebuffer {
 	struct drm_framebuffer base;
 	struct drm_gem_object *obj;
 };
 
 #define to_qxl_crtc(x) container_of(x, struct qxl_crtc, base)
 #define drm_connector_to_qxl_output(x) container_of(x, struct qxl_output, base)
 #define drm_encoder_to_qxl_output(x) container_of(x, struct qxl_output, enc)
 #define to_qxl_framebuffer(x) container_of(x, struct qxl_framebuffer, base)
 
 struct qxl_mman {
 	struct ttm_bo_global_ref        bo_global_ref;
 	struct drm_global_reference	mem_global_ref;
 	bool				mem_global_referenced;
 	struct ttm_bo_device		bdev;
 };
 
 struct qxl_mode_info {
 	bool mode_config_initialized;
 
 	/* pointer to fbdev info structure */
 	struct qxl_fbdev *qfbdev;
-- 
2.14.3

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

^ permalink raw reply related	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2017-11-27 21:50 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-11-27 21:07 [PATCH] drm/qxl: reapply cursor after resetting primary Ray Strode
2017-11-27 21:42 ` Ray Strode
2017-11-27 21:50 ` [PATCH v2 0/2] qxl cursor fixes Ray Strode
2017-11-27 21:50   ` [PATCH 1/2] drm/qxl: unref cursor bo when finished with it Ray Strode
2017-11-27 21:50   ` [PATCH 2/2] drm/qxl: reapply cursor after resetting primary Ray Strode

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.