From: John Keeping <john@metanate.com>
To: Daniel Vetter <daniel@ffwll.ch>
Cc: "Alex Deucher" <alexdeucher@gmail.com>,
"Ville Syrjälä" <ville.syrjala@linux.intel.com>,
LKML <linux-kernel@vger.kernel.org>,
"Maling list - DRI developers" <dri-devel@lists.freedesktop.org>,
"Michel Dänzer" <michel.daenzer@amd.com>
Subject: Re: [PATCH v2] drm: support hotspot for universal plane cursors
Date: Tue, 17 Nov 2015 18:47:28 +0000 [thread overview]
Message-ID: <20151117184728.27252360.john@metanate.com> (raw)
In-Reply-To: <20151117183132.GU16848@phenom.ffwll.local>
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=c9f8f642fd495937400618a4fc25ecae3f8888fc
> > 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;
}
next prev parent reply other threads:[~2015-11-17 18:48 UTC|newest]
Thread overview: 19+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-11-17 12:10 [PATCH] drm: support hotspot for universal plane cursors John Keeping
2015-11-17 13:13 ` kbuild test robot
2015-11-17 15:05 ` [PATCH v2] " John Keeping
2015-11-17 15:39 ` Ville Syrjälä
2015-11-17 15:59 ` John Keeping
2015-11-17 16:09 ` Ville Syrjälä
2015-11-17 16:29 ` Daniel Vetter
2015-11-17 16:58 ` John Keeping
2015-11-17 18:40 ` Daniel Vetter
2015-11-17 19:11 ` Alex Deucher
2015-11-17 17:07 ` Alex Deucher
2015-11-17 18:31 ` Daniel Vetter
2015-11-17 18:47 ` John Keeping [this message]
2015-11-17 19:11 ` Daniel Vetter
2015-11-18 10:12 ` John Keeping
2015-11-18 11:08 ` Daniel Vetter
2015-11-18 8:39 ` Michel Dänzer
2015-11-18 8:51 ` Daniel Vetter
2015-11-18 8:59 ` Michel Dänzer
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20151117184728.27252360.john@metanate.com \
--to=john@metanate.com \
--cc=alexdeucher@gmail.com \
--cc=daniel@ffwll.ch \
--cc=dri-devel@lists.freedesktop.org \
--cc=linux-kernel@vger.kernel.org \
--cc=michel.daenzer@amd.com \
--cc=ville.syrjala@linux.intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox