Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754654AbbKQSfN (ORCPT ); Tue, 17 Nov 2015 13:35:13 -0500 Received: from mail-wm0-f44.google.com ([74.125.82.44]:34479 "EHLO mail-wm0-f44.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932541AbbKQSbh (ORCPT ); Tue, 17 Nov 2015 13:31:37 -0500 Date: Tue, 17 Nov 2015 19:31:32 +0100 From: Daniel Vetter To: Alex Deucher Cc: John Keeping , Ville =?iso-8859-1?Q?Syrj=E4l=E4?= , LKML , Maling list - DRI developers , Michel =?iso-8859-1?Q?D=E4nzer?= Subject: Re: [PATCH v2] drm: support hotspot for universal plane cursors Message-ID: <20151117183132.GU16848@phenom.ffwll.local> Mail-Followup-To: Alex Deucher , John Keeping , Ville =?iso-8859-1?Q?Syrj=E4l=E4?= , LKML , Maling list - DRI developers , Michel =?iso-8859-1?Q?D=E4nzer?= 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> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: X-Operating-System: Linux phenom 4.1.0-2-amd64 User-Agent: Mutt/1.5.23 (2014-03-12) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 4316 Lines: 95 On Tue, Nov 17, 2015 at 12:07:24PM -0500, Alex Deucher wrote: > On Tue, Nov 17, 2015 at 11:29 AM, 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. 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 > > 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. -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/