Skip to contentSkip to conversation

drm: add hotspot support for cursors.

PATCH In review Dave Airlie 2 participants 3 messages Jun 20, 2013 – Jun 24, 2013

Subsystems (3)
DRM drivers DRM misc DRM driver for QXL virtual GPU
lore
Share & tools
lore

Dave Airlie

author maintainer
#0 lore raw

PATCH 1/2 drm: add hotspot support for cursors.

From: Dave Airlie <[email protected]>

So it looks like for virtual hw cursors on QXL we need to inform the "hw" device what the cursor hotspot parameters are. This makes sense if you think the host has to draw the cursor and interpret clicks from it. However the current modesetting interface doesn't support passing the hotspot information from userspace.

This implements a new cursor ioctl, that takes the hotspot info as well, userspace can try calling the new interface and if it -ENOSYS, can just fallback to the old non-hotspot interface.

Signed-off-by Dave Airlie <[email protected]>
---
 drivers/gpu/drm/drm_crtc.c  | 35 +++++++++++++++++++++++++++++------
 drivers/gpu/drm/drm_drv.c   |  1 +
 include/drm/drm_crtc.h      |  5 +++++
 include/uapi/drm/drm.h      |  1 +
 include/uapi/drm/drm_mode.h | 13 +++++++++++++
 5 files changed, 49 insertions(+), 6 deletions(-)
drivers/gpu/drm/drm_crtc.c +29 -6
index e7e9242..cc9eada 100644
@@ -2099,10 +2099,10 @@ out:
 	return ret;
 }
 
-int drm_mode_cursor_ioctl(struct drm_device *dev,
-			void *data, struct drm_file *file_priv)
+static int drm_mode_cursor_common(struct drm_device *dev,
+				  struct drm_mode_cursor2 *req,
+				  struct drm_file *file_priv)
 {
-	struct drm_mode_cursor *req = data;
 	struct drm_mode_object *obj;
 	struct drm_crtc *crtc;
 	int ret = 0;

@@ -2122,13 +2122,17 @@ int drm_mode_cursor_ioctl(struct drm_device *dev,
 
 	mutex_lock(&crtc->mutex);
 	if (req->flags & DRM_MODE_CURSOR_BO) {
-		if (!crtc->funcs->cursor_set) {
+		if (!crtc->funcs->cursor_set && !crtc->funcs->cursor_set2) {
 			ret = -ENXIO;
 			goto out;
 		}
 		/* Turns off the cursor if handle is 0 */
-		ret = crtc->funcs->cursor_set(crtc, file_priv, req->handle,
-					      req->width, req->height);
+		if (crtc->funcs->cursor_set2)
+			ret = crtc->funcs->cursor_set2(crtc, file_priv, req->handle,
+						      req->width, req->height, req->hot_x, req->hot_y);
+		else
+			ret = crtc->funcs->cursor_set(crtc, file_priv, req->handle,
+						      req->width, req->height);
 	}
 
 	if (req->flags & DRM_MODE_CURSOR_MOVE) {

@@ -2143,6 +2147,25 @@ out:
 	mutex_unlock(&crtc->mutex);
 
 	return ret;
+
+}
+int drm_mode_cursor_ioctl(struct drm_device *dev,
+			void *data, struct drm_file *file_priv)
+{
+	struct drm_mode_cursor *req = data;
+	struct drm_mode_cursor2 new_req;
+
+	memcpy(&new_req, req, sizeof(struct drm_mode_cursor));
+	new_req.hot_x = new_req.hot_y = 0;
+
+	return drm_mode_cursor_common(dev, &new_req, file_priv);
+}
+
+int drm_mode_cursor2_ioctl(struct drm_device *dev,
+			   void *data, struct drm_file *file_priv)
+{
+	struct drm_mode_cursor2 *req = data;
+	return drm_mode_cursor_common(dev, req, file_priv);
 }
 
 /* Original addfb only supported RGB formats, so figure out which one */
drivers/gpu/drm/drm_drv.c +1 -0
index 9cc247f..99fcd7c 100644
@@ -166,6 +166,7 @@ static const struct drm_ioctl_desc drm_ioctls[] = {
 	DRM_IOCTL_DEF(DRM_IOCTL_MODE_DESTROY_DUMB, drm_mode_destroy_dumb_ioctl, DRM_CONTROL_ALLOW|DRM_UNLOCKED),
 	DRM_IOCTL_DEF(DRM_IOCTL_MODE_OBJ_GETPROPERTIES, drm_mode_obj_get_properties_ioctl, DRM_CONTROL_ALLOW|DRM_UNLOCKED),
 	DRM_IOCTL_DEF(DRM_IOCTL_MODE_OBJ_SETPROPERTY, drm_mode_obj_set_property_ioctl, DRM_MASTER|DRM_CONTROL_ALLOW|DRM_UNLOCKED),
+	DRM_IOCTL_DEF(DRM_IOCTL_MODE_CURSOR2, drm_mode_cursor2_ioctl, DRM_MASTER|DRM_CONTROL_ALLOW|DRM_UNLOCKED),
 };
 
 #define DRM_CORE_IOCTL_COUNT	ARRAY_SIZE( drm_ioctls )
include/drm/drm_crtc.h +5 -0
index adb3f9b..093c030 100644
@@ -339,6 +339,9 @@ struct drm_crtc_funcs {
 	/* cursor controls */
 	int (*cursor_set)(struct drm_crtc *crtc, struct drm_file *file_priv,
 			  uint32_t handle, uint32_t width, uint32_t height);
+	int (*cursor_set2)(struct drm_crtc *crtc, struct drm_file *file_priv,
+			   uint32_t handle, uint32_t width, uint32_t height,
+			   int32_t hot_x, int32_t hot_y);
 	int (*cursor_move)(struct drm_crtc *crtc, int x, int y);
 
 	/* Set gamma on the CRTC */

@@ -1022,6 +1025,8 @@ extern int drm_mode_setplane(struct drm_device *dev,
 			       void *data, struct drm_file *file_priv);
 extern int drm_mode_cursor_ioctl(struct drm_device *dev,
 				void *data, struct drm_file *file_priv);
+extern int drm_mode_cursor2_ioctl(struct drm_device *dev,
+				void *data, struct drm_file *file_priv);
 extern int drm_mode_addfb(struct drm_device *dev,
 			  void *data, struct drm_file *file_priv);
 extern int drm_mode_addfb2(struct drm_device *dev,
include/uapi/drm/drm.h +1 -0
index 5a57be6..238a166 100644
@@ -732,6 +732,7 @@ struct drm_prime_handle {
 #define DRM_IOCTL_MODE_ADDFB2		DRM_IOWR(0xB8, struct drm_mode_fb_cmd2)
 #define DRM_IOCTL_MODE_OBJ_GETPROPERTIES	DRM_IOWR(0xB9, struct drm_mode_obj_get_properties)
 #define DRM_IOCTL_MODE_OBJ_SETPROPERTY	DRM_IOWR(0xBA, struct drm_mode_obj_set_property)
+#define DRM_IOCTL_MODE_CURSOR2		DRM_IOWR(0xBB, struct drm_mode_cursor2)
 
 /**
  * Device specific ioctls should only be in their respective headers
include/uapi/drm/drm_mode.h +13 -0
index 090e533..53db7ce 100644
@@ -388,6 +388,19 @@ struct drm_mode_cursor {
 	__u32 handle;
 };
 
+struct drm_mode_cursor2 {
+	__u32 flags;
+	__u32 crtc_id;
+	__s32 x;
+	__s32 y;
+	__u32 width;
+	__u32 height;
+	/* driver specific handle */
+	__u32 handle;
+	__s32 hot_x;
+	__s32 hot_y;
+};
+
 struct drm_mode_crtc_lut {
 	__u32 crtc_id;
 	__u32 gamma_size;
Signature · 1 line
1.8.2.1
author maintainer
#1 lore raw

PATCH 2/2 drm/qxl: add support for cursor hotspot.

From: Dave Airlie <[email protected]>

This uses the cursor hotspot info from userspace and passes it to the qxl hw layer.

Signed-off-by Dave Airlie <[email protected]>
---
 drivers/gpu/drm/qxl/qxl_display.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)
drivers/gpu/drm/qxl/qxl_display.c +8 -8
index 823d29e..489e879 100644
@@ -255,11 +255,11 @@ qxl_hide_cursor(struct qxl_device *qdev)
 	qxl_release_unreserve(qdev, release);
 }
 
-static int qxl_crtc_cursor_set(struct drm_crtc *crtc,
-			       struct drm_file *file_priv,
-			       uint32_t handle,
-			       uint32_t width,
-			       uint32_t height)
+static int qxl_crtc_cursor_set2(struct drm_crtc *crtc,
+				struct drm_file *file_priv,
+				uint32_t handle,
+				uint32_t width,
+				uint32_t height, int32_t hot_x, int32_t hot_y)
 {
 	struct drm_device *dev = crtc->dev;
 	struct qxl_device *qdev = dev->dev_private;

@@ -315,8 +315,8 @@ static int qxl_crtc_cursor_set(struct drm_crtc *crtc,
 	cursor->header.type = SPICE_CURSOR_TYPE_ALPHA;
 	cursor->header.width = 64;
 	cursor->header.height = 64;
-	cursor->header.hot_spot_x = 0;
-	cursor->header.hot_spot_y = 0;
+	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;

@@ -397,7 +397,7 @@ static int qxl_crtc_cursor_move(struct drm_crtc *crtc,
 
 
 static const struct drm_crtc_funcs qxl_crtc_funcs = {
-	.cursor_set = qxl_crtc_cursor_set,
+	.cursor_set2 = qxl_crtc_cursor_set2,
 	.cursor_move = qxl_crtc_cursor_move,
 	.gamma_set = qxl_crtc_gamma_set,
 	.set_config = drm_crtc_helper_set_config,
Signature · 1 line
1.8.2.1
Reviewed-by on v1 1/2
#2 lore raw
In reply to Dave Airlie
Quoted 11 lines · Dave Airlie

From: Dave Airlie <[email protected]>

So it looks like for virtual hw cursors on QXL we need to inform the "hw" device what the cursor hotspot parameters are. This makes sense if you think the host has to draw the cursor and interpret clicks from it. However the current modesetting interface doesn't support passing the hotspot information from userspace.

This implements a new cursor ioctl, that takes the hotspot info as well, userspace can try calling the new interface and if it -ENOSYS, can just fallback to the old non-hotspot interface.

This needs to be updated since your patch implements the fallback to the old interface transparently already. Otherwise looks sane.

Reviewed-by Daniel Vetter <[email protected]>
Quoted 153 lines
Signed-off-by Dave Airlie <[email protected]>
---
 drivers/gpu/drm/drm_crtc.c  | 35 +++++++++++++++++++++++++++++------
 drivers/gpu/drm/drm_drv.c   |  1 +
 include/drm/drm_crtc.h      |  5 +++++
 include/uapi/drm/drm.h      |  1 +
 include/uapi/drm/drm_mode.h | 13 +++++++++++++
 5 files changed, 49 insertions(+), 6 deletions(-)
drivers/gpu/drm/drm_crtc.c +29 -6
index e7e9242..cc9eada 100644
@@ -2099,10 +2099,10 @@ out:
 	return ret;
 }
 
-int drm_mode_cursor_ioctl(struct drm_device *dev,
-			void *data, struct drm_file *file_priv)
+static int drm_mode_cursor_common(struct drm_device *dev,
+				  struct drm_mode_cursor2 *req,
+				  struct drm_file *file_priv)
 {
-	struct drm_mode_cursor *req = data;
 	struct drm_mode_object *obj;
 	struct drm_crtc *crtc;
 	int ret = 0;

@@ -2122,13 +2122,17 @@ int drm_mode_cursor_ioctl(struct drm_device *dev,
 
 	mutex_lock(&crtc->mutex);
 	if (req->flags & DRM_MODE_CURSOR_BO) {
-		if (!crtc->funcs->cursor_set) {
+		if (!crtc->funcs->cursor_set && !crtc->funcs->cursor_set2) {
 			ret = -ENXIO;
 			goto out;
 		}
 		/* Turns off the cursor if handle is 0 */
-		ret = crtc->funcs->cursor_set(crtc, file_priv, req->handle,
-					      req->width, req->height);
+		if (crtc->funcs->cursor_set2)
+			ret = crtc->funcs->cursor_set2(crtc, file_priv, req->handle,
+						      req->width, req->height, req->hot_x, req->hot_y);
+		else
+			ret = crtc->funcs->cursor_set(crtc, file_priv, req->handle,
+						      req->width, req->height);
 	}
 
 	if (req->flags & DRM_MODE_CURSOR_MOVE) {

@@ -2143,6 +2147,25 @@ out:
 	mutex_unlock(&crtc->mutex);
 
 	return ret;
+
+}
+int drm_mode_cursor_ioctl(struct drm_device *dev,
+			void *data, struct drm_file *file_priv)
+{
+	struct drm_mode_cursor *req = data;
+	struct drm_mode_cursor2 new_req;
+
+	memcpy(&new_req, req, sizeof(struct drm_mode_cursor));
+	new_req.hot_x = new_req.hot_y = 0;
+
+	return drm_mode_cursor_common(dev, &new_req, file_priv);
+}
+
+int drm_mode_cursor2_ioctl(struct drm_device *dev,
+			   void *data, struct drm_file *file_priv)
+{
+	struct drm_mode_cursor2 *req = data;
+	return drm_mode_cursor_common(dev, req, file_priv);
 }
 
 /* Original addfb only supported RGB formats, so figure out which one */
drivers/gpu/drm/drm_drv.c +1 -0
index 9cc247f..99fcd7c 100644
@@ -166,6 +166,7 @@ static const struct drm_ioctl_desc drm_ioctls[] = {
 	DRM_IOCTL_DEF(DRM_IOCTL_MODE_DESTROY_DUMB, drm_mode_destroy_dumb_ioctl, DRM_CONTROL_ALLOW|DRM_UNLOCKED),
 	DRM_IOCTL_DEF(DRM_IOCTL_MODE_OBJ_GETPROPERTIES, drm_mode_obj_get_properties_ioctl, DRM_CONTROL_ALLOW|DRM_UNLOCKED),
 	DRM_IOCTL_DEF(DRM_IOCTL_MODE_OBJ_SETPROPERTY, drm_mode_obj_set_property_ioctl, DRM_MASTER|DRM_CONTROL_ALLOW|DRM_UNLOCKED),
+	DRM_IOCTL_DEF(DRM_IOCTL_MODE_CURSOR2, drm_mode_cursor2_ioctl, DRM_MASTER|DRM_CONTROL_ALLOW|DRM_UNLOCKED),
 };
 
 #define DRM_CORE_IOCTL_COUNT	ARRAY_SIZE( drm_ioctls )
include/drm/drm_crtc.h +5 -0
index adb3f9b..093c030 100644
@@ -339,6 +339,9 @@ struct drm_crtc_funcs {
 	/* cursor controls */
 	int (*cursor_set)(struct drm_crtc *crtc, struct drm_file *file_priv,
 			  uint32_t handle, uint32_t width, uint32_t height);
+	int (*cursor_set2)(struct drm_crtc *crtc, struct drm_file *file_priv,
+			   uint32_t handle, uint32_t width, uint32_t height,
+			   int32_t hot_x, int32_t hot_y);
 	int (*cursor_move)(struct drm_crtc *crtc, int x, int y);
 
 	/* Set gamma on the CRTC */

@@ -1022,6 +1025,8 @@ extern int drm_mode_setplane(struct drm_device *dev,
 			       void *data, struct drm_file *file_priv);
 extern int drm_mode_cursor_ioctl(struct drm_device *dev,
 				void *data, struct drm_file *file_priv);
+extern int drm_mode_cursor2_ioctl(struct drm_device *dev,
+				void *data, struct drm_file *file_priv);
 extern int drm_mode_addfb(struct drm_device *dev,
 			  void *data, struct drm_file *file_priv);
 extern int drm_mode_addfb2(struct drm_device *dev,
include/uapi/drm/drm.h +1 -0
index 5a57be6..238a166 100644
@@ -732,6 +732,7 @@ struct drm_prime_handle {
 #define DRM_IOCTL_MODE_ADDFB2		DRM_IOWR(0xB8, struct drm_mode_fb_cmd2)
 #define DRM_IOCTL_MODE_OBJ_GETPROPERTIES	DRM_IOWR(0xB9, struct drm_mode_obj_get_properties)
 #define DRM_IOCTL_MODE_OBJ_SETPROPERTY	DRM_IOWR(0xBA, struct drm_mode_obj_set_property)
+#define DRM_IOCTL_MODE_CURSOR2		DRM_IOWR(0xBB, struct drm_mode_cursor2)
 
 /**
  * Device specific ioctls should only be in their respective headers
include/uapi/drm/drm_mode.h +13 -0
index 090e533..53db7ce 100644
@@ -388,6 +388,19 @@ struct drm_mode_cursor {
 	__u32 handle;
 };
 
+struct drm_mode_cursor2 {
+	__u32 flags;
+	__u32 crtc_id;
+	__s32 x;
+	__s32 y;
+	__u32 width;
+	__u32 height;
+	/* driver specific handle */
+	__u32 handle;
+	__s32 hot_x;
+	__s32 hot_y;
+};
+
 struct drm_mode_crtc_lut {
 	__u32 crtc_id;
 	__u32 gamma_size;
Signature · 6 lines
1.8.2.1

_______________________________________________
dri-devel mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/dri-devel
Signature · 3 lines
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch