Subject: Re: [PATCH v2] drm: support hotspot for universal plane cursors
To: John Keeping <john@metanate.com>,
        =?UTF-8?B?VmlsbGUgU3lyasOkbMOk?= <ville.syrjala@linux.intel.com>,
        Alex Deucher <alexdeucher@gmail.com>, linux-kernel@vger.kernel.org,
        dri-devel@lists.freedesktop.org
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> <564C394B.8070408@daenzer.net>
 <20151118085151.GB20799@phenom.ffwll.local>
From: =?UTF-8?Q?Michel_D=c3=a4nzer?= <michel@daenzer.net>
Message-ID: <564C3DE7.3040502@daenzer.net>
Date: Wed, 18 Nov 2015 17:59:19 +0900
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101
 Icedove/38.3.0
MIME-Version: 1.0
In-Reply-To: <20151118085151.GB20799@phenom.ffwll.local>
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: 8bit
Sender: linux-kernel-owner@vger.kernel.org
Content-Length: 2738
Lines: 56

On 18.11.2015 17:51, Daniel Vetter wrote:
> On Wed, Nov 18, 2015 at 05:39:39PM +0900, Michel Dänzer wrote:
>> On 18.11.2015 01:29, Daniel Vetter wrote:
>>>
>>> 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?
>>
>> As described in the rest of the commit log, the intention was to avoid
>> the cursor intermittently appearing in the wrong location with existing
>> userspace which sets the cursor BO in one ioctl call and the new
>> position in another ioctl call.
>>
>>
>>> radeon is the only driver doing this, making this interface inconsistent.
>>
>> It's only inconsistent in the case that userspace updates the cursor
>> position to account for the new hotspot position in one ioctl call
>> first, and only then sets the new BO in another ioctl call. In all other
>> cases, the cursor position passed in by userspace is preserved.
>>
>> Anyway, in the meantime it has become apparent that this change didn't
>> fully fix the problem, so feel free to revert it.
> 
> Yeah I read the commit message but didn't understand what it's doing.
> After some discussion with Alex on irc I realized that the fixup is only
> applied in when updating the cursor bo and changing the hotspot to avoid
> that kind of flickering. That problem is solved though on the kernel side
> with universal planes (where we don't artificially split up the cursor
> update into a move + bo-update for the driver interface any more). And
> it's fixable in userspace even with legacy cursor interfaces since the
> ioctl allows you to move + update at the same time too. It's just that X
> doesn't provide that interface to the driver in a useful way.

Well, the legacy cursor interfaces currently don't allow the driver to
prevent the hardware from updating the cursor between the cursor_set /
cursor_move calls. Anyway, I tried adding a cursor_lock hook for that
purpose and adapting userspace accordingly, but it still doesn't seem to
fully fix the problem. So I'm leaving it to somebody else / another day. :)


-- 
Earthling Michel Dänzer               |               http://www.amd.com
Libre software enthusiast             |             Mesa and X developer
--
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/