Re: Atomic KMS API lacks the ability to set cursor hot-spot coordinates

["Thomas Hellström (VMware)" <thomas_os@shipmail.org>]

On 3/20/20 12:27 PM, Simon Ser wrote:
> On Friday, March 20, 2020 11:59 AM, Thomas Hellström <thomas_os@shipmail.org> wrote:
>
>> On 3/20/20 10:13 AM, Pekka Paalanen wrote:
>>
>>> On Thu, 19 Mar 2020 23:57:09 +0100
>>> Thomas Hellström (VMware) thomas_os@shipmail.org wrote:
>>>
>>>> Hi,
>>>> On 3/19/20 10:07 PM, Simon Ser wrote:
>>>>
>>>>>>>>> Is that something that should be done?
>>>>>>>>> If the hotspot property also had a "disabled" value, then Weston could
>>>>>>>>> set the hotspot to disabled when it is using the cursor plane for
>>>>>>>>> non-cursor content and not lose the feature. And of course set hotspot
>>>>>>>>> correctly when it in fact is a cursor (but for what input?).
>>>>>>>>> I believe cursor planes in the affected virtual gfx-cards do not
>>>>>>>>> really have a mode where they can actually be used as a generic overlay
>>>>>>>>> plane, certainly not in a useful manner (if anything works it will all
>>>>>>>>> be software emulation), implementing a hotspot disabled mode would be
>>>>>>>>> tricky and this would needs to be duplicated in all virtual-gfx cards
>>>>>>>>> kms drivers.
>>>>>>>> If I understood Daniel's proposal for how to deal with this properly,
>>>>>>>> then only cursor planes which actually need them would get the new
>>>>>>>> hotspot x/y properties. If we do that then Weston could use the
>>>>>>>> presence of the hotspot x/y properties to detect if it is dealing
>>>>>>>> with a proper hw plane which can also be used as a generic
>>>>>>>> plane; or a virtual-gfx cards cursor-plane, and then just not
>>>>>>>> bother with trying to use the plane as a generic hw plane.
>>>>>>>> Would that work?
>>>>>>>> That would need to at least be hidden behind a DRM capability, otherwise
>>>>>>>> it would break existing user-space ignoring the hotspot props (e.g.
>>>>>>>> current Weston).
>>>>>>>> Current Weston is already broken, fixing that is what this whole
>>>>>>>> thread is about.
>>>>>> The virtual gfx-cards drivers simply must now the hotspot for things to
>>>>>> work; and a capability is not going to help here for 2 reasons:
>>>>>>
>>>>>> 1.  Short of disabling seamless mode there is nothing the virtual
>>>>>>      gfx-cards drivers can do when clients do not pass the hotspot info;
>>>>>>      and in some cases they cannot even do this as it is under control
>>>>>>      of a userspace agent process with its own channel to the hypervisor.
>>>>>>
>>>>>> 2.  Most existing clients which obviously do not set this to-be-introduced
>>>>>>      capability already pass the hotspot info using the DRM_IOCTL_MODE_CURSOR2
>>>>>>      ioctl. Disabling seamless mode when this to-be-introduced capability is
>>>>>>      not set would cause a huge regression for all these existing clients.
>>>>>>      Compositors which don't support the hotspot prop are and will continue to break
>>>>>>      seamless mode. Setting the prop will never be mandatory, because all user-space
>>>>>>      today doesn't do it.
>>>>>>
>>>>> Since we need to update all user-space to add support for this prop, it would
>>>>> be a good idea to recognize user-space that doesn't support it. Doing something
>>>>> sensible with user-space that doesn't support the prop is up to the virtual
>>>>> driver.
>>>>> If we have a capability, the virtual driver could for instance not advertise a
>>>>> cursor plane at all if user-space won't correctly set the hotspot anyway.
>>>>> What compositors do we have around today that wouldn't want to support
>>>>> setting the hotspot correctly? Do we like to keep them that way, and do
>>>>> we want to encourage any new compositors to also not do this properly?
>>>>> All compositors that sometimes want to use cursor planes for non-cursor
>>>>> content. If a cursor plane does not actually have any performance
>>>>> benefits, then somehow userspace would need to know to not use it in
>>>>> that case.
>> Yes, but is that Weston only? Do we know about others?
> wlroots plans to do the same as Weston.
>
>>> I think the capability is a well-designed solution: a driver that
>>> knows the cursor plane will not be always exactly like userspace
>>> programs it should not advertise the cursor plane at all if userspace
>>> does not set the new cap, and the cap is a promise that userspace will
>>> set the hotspot correctly.
>>> This cap will not break userspace that uses DRM_IOCTL_MODE_CURSOR2 with
>>> correct hotspot info because that is a legacy KMS API and we are
>>> talking about atomic here. The legacy ioctl can continue to ignore the
>>> new cap.
>>> It does mean that if legacy KMS userspace uses the cursor plane for
>>> non-cursor content, it keeps being broken for drivers that mangle cursor
>>> planes. I can certainly live with that.
>> What I'm afraid of is that people writing compositors will just leave
>> that cap off, ignoring the huge benefit for virtual environments and
>> future kms-based remoting solutions.
>>
>> Couldn't compositors that see hotspot properties on a plane just ignore
>> that plane for other stuff than cursors?
> KMS clients will need patching anyway to populate the new prop.
> Enabling a cap is a one-liner.
>
> Is the plan to intentionally break KMS clients to force them to add
> support for the property?

We can't do that. As Hans pointed out, some KMS clients are already 
broken, and hardware planes that doesn't fiddle with the cursor (on 
which Weston works now) don't have to expose the hotspot properties.


> This increases the number of things KMS
> clients need to do in order to correctly work. I think the KMS clients'
> job is already hard enough as-is.

That's exactly the kind of thinking that I'm afraid will leave virtual- 
and remoting solutions unsupported.

> There will always exist user-space which doesn't set the prop. I'm not
> just thinking about old compositors here: KMS clients are not limited
> to just compositors. Think about Kodi for instance or DRM lease
> clients.
>
>>> It seems people are also forgetting the problem of associating the
>>> cursor plane with an input device, so that whatever is looking to
>>> mangle the cursor plane behind the KMS app's back would know how to do
>>> it right.
>>> My first thought for that is a new cursor plane property with the value
>>> of major, minor of the kernel input device that userspace is using to
>>> control the cursor plane. This property should be set by userspace only
>>> when there is exactly one kernel input device it uses for controlling
>>> the cursor plane. Setting this property to none/disabled would be a
>>> clear indication that "seamless mode" would be unwanted. The DRM
>>> driver or whatever it talks to could then check if the cursor plane is
>>> indeed controlled by the input it so far has only assumed and
>>> automatically choose correctly between seamless mode or not.
>> Are you sure this i really needed? VMware's SVGA device checks whether
>> the guest cursor position and host cursor position are reasonably
>> aligned, and if not, composites the guest cursor over the display plane.
>> So if you, for example,  attach a passthrough USB mouse to the VM while
>> running in seamless mode, things "just work". Similarly if you were to
>> attach a kms-based vnc solution that behaved in the same way and that
>> created a new input device, things would also look fine, except for
>> temporary cursor jumps when you switch input device.
> Seems more like a workaround than a real solution.

That may be the case, but still it works and if you have multiple 
clients it always allows the active client run in seamless mode. 
Besides, if people think supporting KMS hotspots is already hard enough, 
the chance of having them supplying the correct input device is small.

/Thomas

> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel


_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel