[PATCH 0/6] drm: Add mouse cursor hotspot support to atomic KMS

[Hans de Goede]

Hi,

On 6/3/22 17:22, Simon Ser wrote:
> On Friday, June 3rd, 2022 at 17:17, Zack Rusin <zackr at vmware.com> wrote:
> 
>>
>>> On Jun 3, 2022, at 10:56 AM, Simon Ser <contact at emersion.fr> wrote:
>>> ⚠ External Email
>>>
>>> On Friday, June 3rd, 2022 at 16:38, Zack Rusin <zackr at vmware.com> wrote:
>>>
>>>>> On Jun 3, 2022, at 10:32 AM, Simon Ser <contact at emersion.fr> wrote:
>>>>>
>>>>> ⚠ External Email
>>>>>
>>>>> On Friday, June 3rd, 2022 at 16:27, Zack Rusin <zackr at vmware.com> wrote:
>>>>>
>>>>>>> In particular: since the driver will ignore the KMS cursor plane
>>>>>>> position set by user-space, I don't think it's okay to just expose
>>>>>>> without opt-in from user-space (e.g. with a DRM_CLIENT_CAP).
>>>>>>>
>>>>>>> cc wayland-devel and Pekka for user-space feedback.
>>>>>>
>>>>>> I think Thomas expressed our concerns and reasons why those wouldn’t
>>>>>> work for us back then. Is there something else you’d like to add?
>>>>>
>>>>> I disagreed, and I don't understand Thomas' reasoning.
>>>>>
>>>>> KMS clients will need an update to work correctly. Adding a new prop
>>>>> without a cap leaves existing KMS clients broken. Adding a cap allows
>>>>> both existing KMS clients and updated KMS clients to work correctly.
>>>>
>>>> I’m not sure what you mean here. They are broken right now. That’s what we’re
>>>> fixing. That’s the reason why the virtualized drivers are on deny-lists for
>>>> all atomic kms. So nothing needs to be updated. If you have a kms client that
>>>> was using virtualized drivers with atomic kms then mouse clicks never worked
>>>> correctly.
>>>>
>>>> So, yes, clients need to set cursor hotspot if they want mouse to work
>>>> correctly with virtualized drivers.
>>>
>>> My proposal was:
>>>
>>> - Introduce DRM_CLIENT_CAP_CURSOR_PLANE_NO_POSITION (or a better name). Only
>>> user-space which supports the hotspot props will enable it.
>>> - By default, don't expose a cursor plane, because current user-space doesn't
>>> support it (Weston might put a window in the cursor plane, and nobody can
>>> report hotspot).
>>> - If the KMS client enables the cap, advertise the cursor
>>> plane, and make it so the plane doesn't have the CRTC_X/CRTC_Y properties
>>> since the driver will pick the position.
>>>
>>> That way both old and new user-space are fixed.
>>
>> I don’t think I see how that fixes anything. In particular I don’t see a way
>> of fixing the old user space at all. We require hotspot info, old user space
>> doesn’t set it because there’s no way of setting it on atomic kms.
> 
> Old atomic user-space is fixed by removing the cursor plane. Then old
> atomic user-space will fallback to drawing the cursor itself, e.g. via
> OpenGL.

That is just a terrible idea, the current situation is that userspace has a
hardcoded list of drivers which need hotspots, so it uses the old non-atomic
APIs on that "hw" since the atomic APIs don't support hotspots.

The downsides I see to your proposal are:

1. Falling back to sw cursor rendering instead of using the old APIs would
be a pretty significant regression in user experience. I know that in theory
sw rendering can be made to not flicker, but in practice it tends to be a
flickery mess.

2. It does not help anything since userspace is still hardcoded to use
the old, hotspot aware non-atomic API anyways. So it would only make things
worse since hiding the cursorplane for userspace which does not set the CAP
would mean the the old non-atomic API also stops working. Or this would add
extra complications in the kernel to still keep the old APIs working.

I do think that a CAP might be a good idea, but the disabling of the
cursor plane based on the CAP does not sound like a good idea to me.

Regards,

Hans