lkml.org 
[lkml]   [2015]   [Nov]   [17]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH v2] drm: support hotspot for universal plane cursors
On Tue, 17 Nov 2015 17:29:35 +0100, Daniel Vetter 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.

Hmm... I've run the same client code on QXL and Rockchip (which uses
universal planes) and without this patch the behaviour is just plain
wrong on Rockchip.

With a 32x32 cursor with the hotspot in the bottom-right using:

drmModeSetCursor2(..., 0, 32)
drmModeMoveCursor(..., x, y)

then with QXL when I click I get an event at (x, y) and this is
precisely under the bottom-right of the cursor.

With Rockchip the click appears to happen at the top-left of the cursor
(as if the hotspot were (0, 0)). This patch makes the behaviour match
that on QXL.

I can't see how the hotspot can be ignored here unless you're saying
that the client code needs to offset the cursor position by the hotspot,
but in that case it will quite clearly be wrong on QXL.


\
 
 \ /
  Last update: 2015-11-17 18:21    [W:0.077 / U:0.604 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site