Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754611AbbKQTLt (ORCPT ); Tue, 17 Nov 2015 14:11:49 -0500 Received: from mail-yk0-f175.google.com ([209.85.160.175]:36704 "EHLO mail-yk0-f175.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753924AbbKQTLs convert rfc822-to-8bit (ORCPT ); Tue, 17 Nov 2015 14:11:48 -0500 MIME-Version: 1.0 In-Reply-To: <20151117184051.GV16848@phenom.ffwll.local> References: <1447762218-11017-1-git-send-email-john@metanate.com> <1447772734-6480-1-git-send-email-john@metanate.com> <20151117153932.GU4437@intel.com> <20151117155943.1e83b053.john@metanate.com> <20151117162935.GK16848@phenom.ffwll.local> <20151117165806.6f444af1.john@metanate.com> <20151117184051.GV16848@phenom.ffwll.local> Date: Tue, 17 Nov 2015 14:11:47 -0500 Message-ID: Subject: Re: [PATCH v2] drm: support hotspot for universal plane cursors From: Alex Deucher To: John Keeping , =?UTF-8?B?VmlsbGUgU3lyasOkbMOk?= , LKML , Maling list - DRI developers , =?UTF-8?Q?Michel_D=C3=A4nzer?= , Alex Deucher Cc: Daniel Vetter Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8BIT Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 4994 Lines: 112 On Tue, Nov 17, 2015 at 1:40 PM, Daniel Vetter wrote: > On Tue, Nov 17, 2015 at 04:58:06PM +0000, John Keeping wrote: >> 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 >> > > > > --- >> > > > > 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. > > I checked a bunch of X drivers and none of them adjust for the hotspot. > Also i915.ko always used x/y directly and never adjusted for the hotspot. > And as far as generic kms interfaces go i915 is pretty much the standard, > and that is what's been implemented in universal planes. > > The only exception really seems to be radeon.ko, which does adjust the x/y > position of the cursor in the crtc space like your patch does above. radeon hw has had the hotspot register since r5xx. http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/drivers/gpu/drm/radeon/radeon_cursor.c#n204 amdgpu programs it as well: http://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/tree/drivers/gpu/drm/amd/amdgpu/dce_v8_0.c#n2440 Alex > > How can I test cursor hotspots? In the end we want a) something consistent > b) that doesn't break existing code and unfortunately b) trumps a). So I'd > like to figure out what's going on on the amd/intel hw I have here. > -Daniel > -- > Daniel Vetter > Software Engineer, Intel Corporation > http://blog.ffwll.ch -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/