From: "Michel Dänzer" <michel@daenzer.net>
To: "John Keeping" <john@metanate.com>,
"Ville Syrjälä" <ville.syrjala@linux.intel.com>,
"Alex Deucher" <alexdeucher@gmail.com>,
linux-kernel@vger.kernel.org, dri-devel@lists.freedesktop.org
Subject: Re: [PATCH v2] drm: support hotspot for universal plane cursors
Date: Wed, 18 Nov 2015 17:59:19 +0900 [thread overview]
Message-ID: <564C3DE7.3040502@daenzer.net> (raw)
In-Reply-To: <20151118085151.GB20799@phenom.ffwll.local>
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
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel
WARNING: multiple messages have this Message-ID (diff)
From: "Michel Dänzer" <michel@daenzer.net>
To: "John Keeping" <john@metanate.com>,
"Ville Syrjälä" <ville.syrjala@linux.intel.com>,
"Alex Deucher" <alexdeucher@gmail.com>,
linux-kernel@vger.kernel.org, dri-devel@lists.freedesktop.org
Subject: Re: [PATCH v2] drm: support hotspot for universal plane cursors
Date: Wed, 18 Nov 2015 17:59:19 +0900 [thread overview]
Message-ID: <564C3DE7.3040502@daenzer.net> (raw)
In-Reply-To: <20151118085151.GB20799@phenom.ffwll.local>
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
next prev parent reply other threads:[~2015-11-18 8:59 UTC|newest]
Thread overview: 36+ 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 13:13 ` kbuild test robot
2015-11-17 15:05 ` [PATCH v2] " John Keeping
2015-11-17 15:05 ` John Keeping
2015-11-17 15:39 ` Ville Syrjälä
2015-11-17 15:39 ` Ville Syrjälä
2015-11-17 15:59 ` John Keeping
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:29 ` Daniel Vetter
2015-11-17 16:58 ` John Keeping
2015-11-17 16:58 ` John Keeping
2015-11-17 18:40 ` Daniel Vetter
2015-11-17 18:40 ` Daniel Vetter
2015-11-17 19:11 ` Alex Deucher
2015-11-17 19:11 ` Alex Deucher
2015-11-17 17:07 ` Alex Deucher
2015-11-17 17:07 ` Alex Deucher
2015-11-17 18:31 ` Daniel Vetter
2015-11-17 18:31 ` Daniel Vetter
2015-11-17 18:47 ` John Keeping
2015-11-17 18:47 ` John Keeping
2015-11-17 19:11 ` Daniel Vetter
2015-11-17 19:11 ` Daniel Vetter
2015-11-18 10:12 ` John Keeping
2015-11-18 10:12 ` John Keeping
2015-11-18 11:08 ` Daniel Vetter
2015-11-18 11:08 ` Daniel Vetter
2015-11-18 8:39 ` Michel Dänzer
2015-11-18 8:39 ` Michel Dänzer
2015-11-18 8:51 ` Daniel Vetter
2015-11-18 8:51 ` Daniel Vetter
2015-11-18 8:59 ` Michel Dänzer [this message]
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=564C3DE7.3040502@daenzer.net \
--to=michel@daenzer.net \
--cc=alexdeucher@gmail.com \
--cc=dri-devel@lists.freedesktop.org \
--cc=john@metanate.com \
--cc=linux-kernel@vger.kernel.org \
--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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.