Re: [PATCH v2] drm: support hotspot for universal plane cursors

17 Nov 2015

On Tue, 17 Nov 2015 19:31:32 +0100, Daniel Vetter wrote:
...
On Tue, Nov 17, 2015 at 12:07:24PM -0500, Alex Deucher wrote:
...
On Tue, Nov 17, 2015 at 11:29 AM, Daniel Vetter daniel@ffwll.ch
wrote:
...
On Tue, Nov 17, 2015 at 03:59:43PM +0000, John Keeping wrote:
...
On Tue, 17 Nov 2015 17:39:32 +0200, Ville Syrjälä wrote:
...
On Tue, Nov 17, 2015 at 03:05:34PM +0000, John Keeping wrote:
...
The request's hot_x and hot_y are set correctly for both
DRM_IOCTL_MODE_CURSOR and DRM_IOCTL_MODE_CURSOR2 so we just
need to save the values and then apply the offset to the
cursor plane when the cursor moves.
Signed-off-by: John Keeping john@metanate.com
v2:

add kerneldoc for hot_x and hot_y in struct drm_crtc

drivers/gpu/drm/drm_crtc.c | 11 +++++++----
 include/drm/drm_crtc.h     |  6 ++++++
 2 files changed, 13 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/drm_crtc.c
b/drivers/gpu/drm/drm_crtc.c index 720a153..40f5b84 100644
--- a/drivers/gpu/drm/drm_crtc.c
+++ b/drivers/gpu/drm/drm_crtc.c
@@ -2831,6 +2831,9 @@ static int
drm_mode_cursor_universal(struct drm_crtc *crtc,
DRM_DEBUG_KMS("failed to wrap cursor buffer in drm
framebuffer\n"); return PTR_ERR(fb); }


            crtc->hot_x = req->hot_x;


            crtc->hot_y = req->hot_y;
    } else {
            fb = NULL;
    }



@@ -2841,11 +2844,11 @@ static int
drm_mode_cursor_universal(struct drm_crtc *crtc, }
if (req->flags & DRM_MODE_CURSOR_MOVE) {

    crtc_x = req->x;


    crtc_y = req->y;




    crtc_x = req->x - crtc->hot_x;


    crtc_y = req->y - crtc->hot_y;

} else {


    crtc_x = crtc->cursor_x;


    crtc_y = crtc->cursor_y;




    crtc_x = crtc->cursor_x - crtc->hot_x;


    crtc_y = crtc->cursor_y - crtc->hot_y;  



Why does the location of the hotspot affect the plane
position?
hot_{x,y} specify the location of the active pixel within the
cursor plane and cursor_{x,y} specify the location of the active
pixel on the display so we need to offset the plane position in
order for the active pixel to be in the correct place.
Nope, hot_x/y is just for virtual machines to indicate where the
logical cursor position is within the cursor plane. It should
have 2 effect on how something is displayed. And no, I have
absolutely no idea why radeon is pulling some tricks here, which
have been added in
commit 78b1a6010b46a69bcd47b723a80f92693f26d17b
Author: Michel Dänzer michel.daenzer@amd.com
Date:   Tue Nov 18 18:00:08 2014 +0900
drm/radeon: Use cursor_set2 hook for enabling / disabling the

HW cursor
Michel/Alex, can you please shed some light onto this? radeon is
the only driver doing this, making this interface inconsistent.
Is the ddx doing something funny compared to -modeseting or
weston?
At least a quick look in the -ati sources didn't even show a user
for this, it's still using the old cursor ioctl. And there's no
other indication in the commit of a bug or anything. Can we
perhaps just revert this?
We use this is xf86-video-ati.  See:
http://cgit.freedesktop.org/xorg/driver/xf86-video-ati/commit/?id=c9f8f642fd...
Our hw cursor has a hotspot register that needs to be programmed for
proper operation.  I don't remember the details of the specific bug.
Michel can provide more info.
Yeah I was blind and didn't spot this. But I can't find your hotspot
cursor reg - it's only used to adjust the x/y offsets (similar to
John's patch). And amdgpu doesn't do this at all, it's only
radoen.ko. Still confused.
Having investigated a bit more, I think xserver handles the hotspot
itself without using the kernel hotspot handling and xorg-video-qxl
carefully reverses this in order to take advantage of
drmModeSetCursor2(), so with X11 drivers you don't notice that the
kernel handling of this is broken in nearly all drivers.
Here's a small test program that demonstrates whether or not cursor
hotspots work.  There should be a single colored pixel immediately to
the left of the pointer but with a broken driver the white pixel will be
64 pixels to the left of the pointer.
Compile with:
cc -o test -I/usr/include/libdrm test.c -lkms -ldrm
-- >8 --
#include <err.h>
#include <poll.h>
#include <string.h>
#include <sys/mman.h>
#include <libkms/libkms.h>
#include <xf86drm.h>
#include <xf86drmMode.h>
static int drm_fd;
static int crtc_id = -1;
static drmModeModeInfo mode_info;
static struct kms_driver *kms;
static struct {
    int id;
    size_t size;
    unsigned char *data;
} framebuffer;
static struct {
    int x, y;
    struct kms_bo *buffer;
} cursor;
#define CURSOR_WIDTH	21
#define CURSOR_HEIGHT	21
#define CURSOR_HOT_X	21
#define CURSOR_HOT_Y	0
unsigned char cursor_data[] =
  "\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0"
  "\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0"
  "\0\0\0\0\0\0\0\377\0\0\0\377\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0"
  "\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0"
  "\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\377\377\377\377\377\0\0\0\377\0\0\0\0"
  "\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0"
  "\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\377\377\377"
  "\377\377\377\377\377\377\0\0\0\377\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0"
  "\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0"
  "\0\0\0\0\0\0\0\0\0\0\0\0\377\377\377\377\377\377\377\377\377\377\377\377"
  "\377\0\0\0\377\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0"
  "\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\377"
  "\377\377\377\377\377\377\377\377\377\377\377\377\377\377\377\377\0\0\0\377"
  "\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0"
  "\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\377\377\377\377\377\377\377"
  "\377\377\377\377\377\377\377\377\377\377\377\377\377\377\0\0\0\377\0\0\0"
  "\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0"
  "\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\377\377\377\377\377\377\377\377\377\377"
  "\377\377\377\377\377\377\377\377\377\377\377\377\377\377\377\0\0\0\377\0"
  "\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0"
  "\0\0\0\0\0\0\0\0\0\0\0\0\0\0\377\377\377\377\377\377\377\377\377\377\377"
  "\377\377\377\377\377\377\377\377\377\377\377\377\377\377\377\377\377\377"
  "\0\0\0\377\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0"
  "\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\377\377\377\377\377\377\377\377\377\377"
  "\377\377\377\377\377\377\377\377\377\377\377\377\377\377\377\377\377\377"
  "\377\377\377\377\377\0\0\0\377\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0"
  "\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\377\377\377\377\377\377\377"
  "\377\377\377\377\377\377\377\377\377\377\377\377\377\377\377\377\377\377"
  "\377\377\377\377\377\377\377\377\377\377\377\377\0\0\0\377\0\0\0\0\0\0\0"
  "\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\377\0\0"
  "\0\377\0\0\0\377\0\0\0\377\0\0\0\377\377\377\377\377\377\377\377\377\377"
  "\377\377\377\377\377\377\377\377\377\377\377\377\377\377\377\0\0\0\377\0"
  "\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0"
  "\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\377\377\377\377\377\377\377\377\377"
  "\377\377\377\377\377\377\377\377\377\377\377\377\377\377\377\377\0\0\0\377"
  "\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0"
  "\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\377\377\377\377\377\377\377\377\377"
  "\0\0\0\377\0\0\0\377\377\377\377\377\377\377\377\377\0\0\0\377\0\0\0\0\0"
  "\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0"
  "\0\0\0\0\0\0\0\0\0\0\377\377\377\377\377\377\377\377\377\0\0\0\377\0\0\0"
  "\0\0\0\0\0\0\0\0\377\377\377\377\377\0\0\0\377\0\0\0\0\0\0\0\0\0\0\0\0\0"
  "\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0"
  "\0\0\377\377\377\377\377\377\377\377\377\0\0\0\377\0\0\0\0\0\0\0\0\0\0\0"
  "\0\0\0\0\377\0\0\0\377\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0"
  "\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\377\377\377\377\377\377\377"
  "\377\377\0\0\0\377\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\377\0\0"
  "\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0"
  "\0\0\0\0\0\0\0\0\0\377\377\377\377\377\377\377\377\377\0\0\0\377\0\0\0\0"
  "\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0"
  "\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\377\377\377\377\377"
  "\377\377\377\377\0\0\0\377\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0"
  "\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0"
  "\0\0\0\0\0\0\0\0\0\0\0\0\377\0\0\0\377\377\377\377\377\0\0\0\377\0\0\0\0"
  "\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0"
  "\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0"
  "\0\0\0\377\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0"
  "\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0"
  "\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0\0"
  "\0\0\0\0\0\0\0\0\0\0\0\0\0";
static void draw_cursor(void)
{
    void *result;
    unsigned char *ptr;
    int x, y;
    if (kms_bo_map(cursor.buffer, &result))
    	err(1, "kms_bo_map");
ptr = result;
    memset(ptr, 0, 64 * 64 * 4);
for (y = 0; y < CURSOR_HEIGHT; y++)
    	memcpy(ptr + 64 * 4 * y, cursor_data + 4 * CURSOR_WIDTH * y, CURSOR_WIDTH * 4);
kms_bo_unmap(cursor.buffer);
}
static void setup_cursor(void)
{
    const unsigned int attr[] = {
    	KMS_BO_TYPE, KMS_BO_TYPE_CURSOR_64X64_A8R8G8B8,
    	KMS_WIDTH, 64,
    	KMS_HEIGHT, 64,
    	KMS_TERMINATE_PROP_LIST
    };
    unsigned int handle;
    if (kms_bo_create(kms, attr, &cursor.buffer))
    	err(1, "kms_bo_create");
draw_cursor();
if (kms_bo_get_prop(cursor.buffer, KMS_HANDLE, &handle))
    	err(1, "kms_bo_get_prop");
if (drmModeSetCursor2(drm_fd, crtc_id, handle, 64, 64, CURSOR_HOT_X, CURSOR_HOT_Y))
    	err(1, "drmModeSetCursor2");
if (drmModeMoveCursor(drm_fd, crtc_id, cursor.x, cursor.y))
    	err(1, "drmModeMoveCursor");
}
static void create_framebuffer(void)
{
    struct drm_mode_create_dumb creq = {
    	.width = mode_info.hdisplay,
    	.height = mode_info.vdisplay,
    	.bpp = 32,
    	.flags = 0,
    };
    struct drm_mode_map_dumb mreq = { 0 };
    if (drmIoctl(drm_fd, DRM_IOCTL_MODE_CREATE_DUMB, &creq))
    	err(1, "DRM_IOCTL_MODE_CREATE_DUMB");
if (drmModeAddFB(drm_fd, creq.width, creq.height, 24, 32, creq.pitch, creq.handle, &framebuffer.id))
    	err(1, "drmModeAddFB");
mreq.handle = creq.handle;
    if (drmIoctl(drm_fd, DRM_IOCTL_MODE_MAP_DUMB, &mreq))
    	err(1, "DRM_IOCTL_MODE_MAP_DUMB");
framebuffer.data = mmap(0, creq.size, PROT_READ | PROT_WRITE, MAP_SHARED, drm_fd, mreq.offset);
    if (framebuffer.data == MAP_FAILED)
    	err(1, "mmap");
framebuffer.size = creq.size;
    memset(framebuffer.data, 0, framebuffer.size);
/* Paint cursor location with a single colored pixel. */
    framebuffer.data[1 + cursor.x * 4 + cursor.y * creq.pitch] = '\xff';
    framebuffer.data[2 + cursor.x * 4 + cursor.y * creq.pitch] = '\xff';
}
static int setup_connector(drmModeConnector *conn, drmModeRes *res)
{
    drmModeModeInfo *mode = NULL;
    int i;
if (conn->connection != DRM_MODE_CONNECTED)
    	return -1;
for (int j = 0; j < conn->count_modes; j++) {
    	if (!mode || (conn->modes[j].type & DRM_MODE_TYPE_PREFERRED)) {
    		mode = &conn->modes[j];
    		if (conn->modes[j].type & DRM_MODE_TYPE_PREFERRED)
    			break;
    	}
    }
if (!mode)
    	return -1;
for (i = 0; i < conn->count_encoders; i++) {
    	int j;
    	drmModeEncoder *enc = drmModeGetEncoder(drm_fd, conn->encoders[j]);
    	if (!enc)
    		continue;
for (j = 0; j < res->count_crtcs; j++) {
    		if (enc->possible_crtcs & (1u << j)) {
    			crtc_id = res->crtcs[j];
    			break;
    		}
    	}
drmModeFreeEncoder(enc);
    	if (crtc_id >= 0)
    		break;
    }
memcpy(&mode_info, mode, sizeof(mode_info));
    cursor.x = mode->hdisplay / 2;
    cursor.y = mode->vdisplay / 2;
    create_framebuffer();
if (drmModeSetCrtc(drm_fd, crtc_id, framebuffer.id, 0, 0, &conn->connector_id, 1, mode))
    	err(1, "drmModeSetCrtc");
return 0;
}
static void setup_screen(drmModeRes *res)
{
    int i;
for (i = 0; i < res->count_connectors; i++) {
    	int result;
    	drmModeConnector *conn = drmModeGetConnector(drm_fd, res->connectors[i]);
    	if (!conn)
    		continue;
    	result = setup_connector(conn, res);
    	drmModeFreeConnector(conn);
    	if (!result)
    		break;
    }
if (crtc_id < 0)
    	errx(1, "no screen found");
}
static void setup_drm(const char *module)
{
    drmModeRes *res;
drm_fd = drmOpen(module, NULL);
    if (drm_fd < 0)
    	err(1, "drmOpen");
if (kms_create(drm_fd, &kms) < 0)
    	err(1, "kms_create");
res = drmModeGetResources(drm_fd);
    if (!res)
    	err(1, "drmModeGetResources");
setup_screen(res);
}
static void wait_for_input(void)
{
    struct pollfd pfd = {
    	.fd = 0,
    	.events = POLLIN,
    };
poll(&pfd, 1, -1);
}
int main(int argc, char *argv[])
{
    const char *module = "qxl";
    if (argc > 1)
    	module = argv[1];
setup_drm(module);
    setup_cursor();
wait_for_input();
return 0;
}

    

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

2012

2011

2010

Re: [PATCH v2] drm: support hotspot for universal plane cursors

Signed-off-by: John Keeping john@metanate.com