Simple protocol extension to manage DRM lease. Based on the work by Keith
Packard in [1], respectively [2].

[1] https://cgit.freedesktop.org/mesa/drm/commit/?id=c4171535389d72e9135c9615cecd07b346fd6d7e
[2] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?h=v4.15-rc9&id=62884cd386b876638720ef88374b31a84ca7ee5f

Signed-off-by: Marius Vlad <marius-cristian.vlad at nxp.com>

Changes since v1:
- added manager: advertise lease capability and manage the lease (Daniel Stone)
- add request(s) for adding connector/crtc/plane to behave more like dmabuf (Daniel Stone)
---
 Makefile.am                                  |   1 +
 unstable/drm-lease/README                    |   4 +
 unstable/drm-lease/drm-lease-unstable-v1.xml | 150 +++++++++++++++++++++++++++
 3 files changed, 155 insertions(+)
 create mode 100644 unstable/drm-lease/README
 create mode 100644 unstable/drm-lease/drm-lease-unstable-v1.xml
[inline thread by ppaalanen at gmail.com (Pekka Paalanen) at Tue, 29 May 2018 17:10:02 +0300]
| Hi Marius,
|
| it's great to have someone working on this!
|
| I finally got a chance to look at it. Comments are inline as usual. Most
| of my questions call for an answer in the spec text.
| [standalone thread by marius-cristian.vlad at nxp.com (Marius-cristian Vlad) at Wed, 30 May 2018 12:38:22 +0000]
| | Thanks for taking the time to go over this. I have some minor follow-up
| | questions bellow.
| | [standalone thread by ppaalanen at gmail.com (Pekka Paalanen) at Thu, 31 May 2018 15:38:35 +0300]
| | | Hi Marius,
| | |
| | | I have written some pondering as replies to your questions below. I hope
| | | they make sense.
| | |
| | | I had a chat with Keith Packard in IRC about some details of how DRM
| | | leasing actually works. These details affect the implementations more,
| | | but there might be something we need to take into account with the
| | | protocol as well.
| | |
| | | These are my notes from the chat:
| | |
| | | - When there is a change with DRM leases, a normal DRM hotplug event
| | |   will be emitted.
| | |
| | | - Particularly Lessor needs to listen for hotplug events and re-query
| | |   the state of all DRM leases. This is how Lessor finds out a lease has
| | |   been terminated by a Lessee. This needs to happen also on gaining DRM
| | |   master.
| | |
| | | - VT-switching: When the original DRM master (Lessor) drops the master
| | |   status, all DRM leases it has given out will be temporarily disabled.
| | |   When Lessor regains DRM master, the disabled leases are automatically
| | |   reinstated.
| | |
| | | - Lessee can tell the difference between a permanent revoke and a
| | |   temporary disable of a lease by the error it gets from the KMS API
| | |   (drmModeSetCrtc, drmModePageFlip, drmModeAtomicCommit): EACCESS for
| | |   temporary disable, and ENOENT for a revoked lease that is not coming
| | |   back.
| | |
| | | - If Lessee gets EACCESS, it should stand by and wait for hotplug
| | |   events to try again with the hope that the lease comes back.
| | |
| | |
| | | Some random thoughts from me:
| | |
| | | - Need to make sure the client has enough information to listen for
| | |   hotplug events in a multi-card system.
| | |
| | | - If the Wayland client disconnects, revoke all its leases.
| | |   Implementation-wise this is easy as the wl_resource gets
| | |   destroyed.
| | |
| | | - We probably do not want a Wayland event for compositor dropping DRM
| | |   master, because it would be racy in the client against KMS calls and
| | |   the client will notice on its own anyway.
| | |
| | | - There could be a Wayland event for compositor gaining DRM master, to
| | |   tell the client to try KMS again now, but it seems redundant: the DRM
| | |   hotplug event provides the same, and needs to be listened to anyway
| | |   if the client wants to react to hotplug on its leased connector.

diff --git a/Makefile.am b/Makefile.am
index 4b9a901..4f6a874 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -2,6 +2,7 @@ unstable_protocols =								\
 	unstable/pointer-gestures/pointer-gestures-unstable-v1.xml		\
 	unstable/fullscreen-shell/fullscreen-shell-unstable-v1.xml		\
 	unstable/linux-dmabuf/linux-dmabuf-unstable-v1.xml			\
+	unstable/drm-lease/drm-lease-unstable-v1.xml				\
 	unstable/text-input/text-input-unstable-v1.xml				\
 	unstable/input-method/input-method-unstable-v1.xml			\
 	unstable/xdg-shell/xdg-shell-unstable-v5.xml				\
diff --git a/unstable/drm-lease/README b/unstable/drm-lease/README
new file mode 100644
index 0000000..a25600c
--- /dev/null
+++ b/unstable/drm-lease/README
@@ -0,0 +1,4 @@
+Linux DRM lease
+
+Maintainers:
+Marius Vlad <marius-cristian.vlad at nxp.com>
diff --git a/unstable/drm-lease/drm-lease-unstable-v1.xml b/unstable/drm-lease/drm-lease-unstable-v1.xml
new file mode 100644
index 0000000..907efb0
--- /dev/null
+++ b/unstable/drm-lease/drm-lease-unstable-v1.xml
@@ -0,0 +1,150 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<protocol name="drm_lease_unstable_v1">
+
+  <copyright>
+    Copyright 2018 NXP
+
+    Permission is hereby granted, free of charge, to any person obtaining a
+    copy of this software and associated documentation files (the "Software"),
+    to deal in the Software without restriction, including without limitation
+    the rights to use, copy, modify, merge, publish, distribute, sublicense,
+    and/or sell copies of the Software, and to permit persons to whom the
+    Software is furnished to do so, subject to the following conditions:
+
+    The above copyright notice and this permission notice (including the next
+    paragraph) shall be included in all copies or substantial portions of the
+    Software.
+
+    THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+    IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+    FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
+    THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+    LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+    FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
+    DEALINGS IN THE SOFTWARE.
+  </copyright>
+
+  <interface name="zwp_kms_lease_manager_v1" version="1">
+    <description summary="lease manager">
+      This interface makes use of DRM lease written by Keith Packard.
+
+      A DRM master can create another DRM master and ``lease'' resources it has
+      control over to the new DRM master. Once leased, resources can not be
+      controlled by the owner unless the owner cancels the lease, or the new
+      DRM master is closed.
+
+      A lease is a contract between the Lessor (DRM master which has leased out
+      resources to one or more other DRM masters) and a Lessee
+      (DRM master which controls resources leased from another DRM master). This
+      contract specifies which resources may be controlled by the Lessee.
+
+      The Lessee can issue modesetting/page-flipping atomic operations etc.,
+      just like a Lesor using the leased file-descriptor passed by the Lesor.
[inline thread by ppaalanen at gmail.com (Pekka Paalanen) at Tue, 29 May 2018 17:10:02 +0300]
| s/Lesor/Lessor/ twice.
+
+      Besides the leased file-description, an integer is used to uniquely
[inline thread by ppaalanen at gmail.com (Pekka Paalanen) at Tue, 29 May 2018 17:10:02 +0300]
| file-descriptor
+      identify a Lessee within the tree of DRM masters descending from a single
+      Owner. Once the Lessee has finished with the resources it had used, the
+      Lessee ID can be used to revoke that lease.
[inline thread by ppaalanen at gmail.com (Pekka Paalanen) at Tue, 29 May 2018 17:10:02 +0300]
| I think we should use a Wayland protocol object to represent a tentative
| Lessee ID so it will never need to be communicated. The only thing a
| client does with it is pass it back to the compositor. What if a client
| maliciously or by accident passes someone else's lessee ID? Using a
| protocol object instead will make such mistake impossible.
| [standalone thread by marius-cristian.vlad at nxp.com (Marius-cristian Vlad) at Wed, 30 May 2018 12:38:22 +0000]
| | Alright, this sounds quite fine.
+
+      Warning! The protocol described in this file is experimental and
+      backward incompatible changes may be made. Backward compatible changes
+      may be added together with the corresponding interface version bump.
+      Backward incompatible changes are done by bumping the version number in
+      the protocol and interface names and resetting the interface version.
+      Once the protocol is to be declared stable, the 'z' prefix and the
+      version number in the protocol and interface names are removed and the
+      interface version number is reset.
+    </description>
+
+    <request name="destroy" type="destructor">
+      <description summary="destroys the manager">
+      Destroys the lease manager object.
[inline thread by ppaalanen at gmail.com (Pekka Paalanen) at Tue, 29 May 2018 17:10:02 +0300]
| + This has no effect on any remaining objects created through this
|   interface.
+      </description>
+    </request>
+
+    <request name="create_lease_req">
+      <description summary="create a temporary object for managing the lease">
+      Create a lease request object to manage the lease. All further interaction
+      is achived using this object. Returns a zwp_kms_lease_request_v1.
[inline thread by ppaalanen at gmail.com (Pekka Paalanen) at Tue, 29 May 2018 17:10:02 +0300]
| Create an object for temporarily storing all the KMS resources to be leased.
+      </description>
+      <arg name="params_id" type="new_id" interface="zwp_kms_lease_request_v1"
+	   summary="the new temporary"/>
+    </request>
+
+  </interface>
+
+  <interface name="zwp_kms_lease_request_v1" version="1">
+  <description summary="lease request object">
+      Typical usage is to create this request lease object using the
[inline thread by ppaalanen at gmail.com (Pekka Paalanen) at Tue, 29 May 2018 17:10:02 +0300]
| a lease request object
+      'zwp_kms_lease_manager_v1', then call 'add_*' requests to add a connector,
+      crtc (and plane) id.
+
+      At the end, use 'create' request to actually create the lease. The client
+      is responsible for finding a suitable combination of connector/crtc/plane
+      to pass. This can be achived by going over all connected connectors and
+      and determine if a lease can be created.
[inline thread by ppaalanen at gmail.com (Pekka Paalanen) at Tue, 29 May 2018 17:10:02 +0300]
| To me this sounds like a client needs to first open a DRM card node,
| iterate through all the resources (is that even possible if it is not a
| DRM master?), look at the information and guess which ones the
| compositor might be willing to lease out, and then through trial and
| error maybe find something that will actually be accepted.
|
| [inline thread by marius-cristian.vlad at nxp.com (Marius-cristian Vlad) at Wed, 30 May 2018 12:38:22 +0000]
| | Indeed, that's how the client demo/example does it now.
| I do not like this plan.
|
| It assumes that the client can open a DRM card node itself - this is
| often not the case. Even display servers do not open the DRM card node
| themselves, they ask logind, so it is actually likely that the client
| does not have the file system access to the card node.
|
| [inline thread by marius-cristian.vlad at nxp.com (Marius-cristian Vlad) at Wed, 30 May 2018 12:38:22 +0000]
| | Well... I assumed that the client can do this on its own.
| | [standalone thread by ppaalanen at gmail.com (Pekka Paalanen) at Thu, 31 May 2018 15:38:35 +0300]
| | | DRM leasing protocol could be used to allow direct display access from
| | | application sandboxes.
| | |
| | | There is one more problem I forgot about: multiple card nodes.
| | | How would a client know which card nodes the compositor is using and
| | | might be able to give out leases for?
| | |
| | | It could probably try and probe to see what it can get from DRM card
| | | nodes directly, but that seems awkward.
| | |
| | | A compositor could also be using multiple card nodes. If we have DRM
| | | resource advertisements as part of this extension, it needs to take
| | | into account that some resources could be from a different card and
| | | therefore cannot be paired with some other resources.
| Another problem is that the client has no idea which resources the
| compositor is willing to lease out. That set of resources could even
| change at runtime when the compositor output configuration changes, if
| the compositor has a policy that everything it is not using itself can
| be leased (I would propose Weston to have this policy).
|
| [inline thread by marius-cristian.vlad at nxp.com (Marius-cristian Vlad) at Wed, 30 May 2018 12:38:22 +0000]
| | Right.
| A compositor could even be willing to lease out resources it is using,
| e.g. plane resources it can fall back from, or a CRTC for an output
| that will get temporarily disabled if a Lessee wants it. Especially in
| the latter case, the compositor might ask the user if he is willing to
| give the output to an app and explaining how he can get his output back
| in case the app malfunctions (e.g. compositor hotkey to revoke all
| leases).
|
| [inline thread by marius-cristian.vlad at nxp.com (Marius-cristian Vlad) at Wed, 30 May 2018 12:38:22 +0000]
| | Yes.
| Both problems could be solved by extending the kms_lease_manager
| interface to advertise the leasable KMS resources, including enough
| information to allow the client to make a good guess on e.g. which
| connectors it might be interested in.
|
| Another fun problem is how to pick a CRTC, since CRTCs cannot be
| assumed to be equal. It's quite possible that the client needs to lease
| every available CRTC one at a time, attempt modeset, and see if it can
| drive the connector with the mode it wants. If it doesn't work, try the
| next available CRTC. Since we probably cannot avoid trial and error
| completely, it is important to reduce the number of possible
| combinations as much as possible.
|
| Another approach would be to not let the client pick a CRTC on its own.
| Let the client pick a connector from a list, and have the compositor
| find a suitable CRTC to go with it.
| [standalone thread by marius-cristian.vlad at nxp.com (Marius-cristian Vlad) at Wed, 30 May 2018 12:38:22 +0000]
| | Alright, if I got this right: the compositor will advertise which
| | connectors can be leased.
| | Will present that to the client, client will pick one of those. On the
| | reply the compositor will determine a valid CRTC to drive that
| | connector then will create the lease and send back to the client the
| | lease fd.
| |
| | [inline thread by ppaalanen at gmail.com (Pekka Paalanen) at Thu, 31 May 2018 15:38:35 +0300]
| | | Yes. If a client wants to lease multiple connectors, I assume it can
| | | just ask each separately. This does assume that the client is not
| | | aiming for shared-CRTC clone mode as it would get more CRTCs than it
| | | needs in that case.
| | |
| | | I'm not sure it would be worth the complexity to be able to lease out
| | | some connectors without CRTCs, the chances of being able to use
| | | shared-CRTC clone mode are slim and use cases probably hard to find. If
| | | a use case pops up, we can look at it then.
| | |
| | | As for a compositor determining a valid CRTC, it could go as far as
| | | doing a TEST_ONLY or even a real atomic commit of the CRTC/connector
| | | with the preferred video mode to ensure it's supposed to work.
| | How will the compositor choose which connectors can be leased? We
| | assume that all connectors can be leased?
| |
| | [inline thread by ppaalanen at gmail.com (Pekka Paalanen) at Thu, 31 May 2018 15:38:35 +0300]
| | | This would be left as compositor policy.
| | |
| | | I would guess that most desktop compositors would lease out only
| | | connectors that are marked as "non-desktop" or explicitly configured to
| | | be leasable and not used by the compositor.
| | |
| | | For Weston it would be nice to take things further than that, maybe
| | | even allow leasing connectors that are in use, so that we can
| | | experiment with CRTC/connector hand-over with leases. If a non-VR
| | | application, e.g. a 3D game, would want to drive a display directly,
| | | this is how it would happen. Another problem is how that game would get
| | | input, but that's irrelevant here.
| | |
| | | I'm totally ok with the initial weston implementation only leasing out
| | | unused resources.
| | For a client a unsigned int doesn't really tell anything .... is this
| | the panel on my right, the display on my left or the upper panel -- or
| | the VR is just hooked up? Do we advertise this to the client as well in
| | the form of output names or something equivalent? I guess more general
| | question show we present the potential leasable resource(s) in a human
| | form? Would that make sense?
| |
| | The approach with the client choosing which crtc/connector/plane
| | somehow assumes that client has a-priory knowledge of which resource
| | wants... albeit will all the drawbacks you enumerated before.
| | [standalone thread by ppaalanen at gmail.com (Pekka Paalanen) at Thu, 31 May 2018 15:38:35 +0300]
| | | Right, the advertisement needs to provide enough information.
| | |
| | | How do applications that want to drive a display directly actually pick
| | | the displays?
| | |
| | | I assume the primary use case here are VR compositors, who are only
| | | looking for HMDs or any "non-desktop" outputs. We need to provide
| | | enough information to let those be identified. I suppose it would be at
| | | least:
| | | - EDID: make
| | | - EDID: model
| | | - EDID: serial number
| | | - physical connector type
| | | - connection status
| | |
| | | The protocol needs to be designed to allow new fields to be added, IOW
| | | to be able to add new events carrying bits of information not sent
| | | before, in a backward-compatible manner. The bits of information that
| | | are not available due to the connector being disconnected etc. could be
| | | simply left unsent.
| | |
| | | Almost all of the above depend on the currently connected monitor,
| | | which means that they can change at runtime. Therefore hotplug and
| | | unplug needs to be accounted for, in case an application could take a
| | | long while (e.g. interactive UI) to choose the connectors it wants.
| | |
| | | The secondary use case are all other programs. They could be interested
| | | in any kind of monitors. They would probably be interested in name and
| | | description from the xdg_output interface. See:
| | | https://cgit.freedesktop.org/wayland/wayland-protocols/commit/?id=d296d0760c186e540438174843f3e93849cc4d70
| | |
| | | But because the compositor would be primarily leasing out connectors it
| | | is not using, there is no respective xdg_output, so we should probably
| | | duplicate the name and description.
| | |
| | | Saying anything explicitly about monitor layout is likely useless: if
| | | the compositor is not using the monitor, it's not part of its layout.
| | | If a compositor is using a monitor, it could add a word or two about
| | | the layout in the description.
| | |
| | |
| | | Btw. a compositor should probably send out connector protocol objects,
| | | not events with connector IDs. That would fit the Wayland object model
| | | better. There might not be a need to send the actual connector ID at
| | | all.
| | |
| | | If we go with leasing one connector at a time and the compositor
| | | automatically picking the CRTC for it, a connector protocol object
| | | could have a request "lease", creating a lease object which then
| | | delivers the failure or success events.
| | |
| | |
| | | Thanks,
| | | pq
+  </description>
+
+    <request name="add_connector">
+      <description summary="connector id">
+      Request to add a connector id to current lease request object.
+      </description>
+      <arg name="id" type="uint" summary="connector id"/>
+    </request>
+
+    <request name="add_crtc">
+      <description summary="crtc id">
+      Request to add a CRTC id to current lease request object.
+      </description>
+      <arg name="id" type="uint" summary="crtc id"/>
+    </request>
+
+    <request name="add_plane">
+      <description summary="plane id">
+      Request to add a plane id to current lease request object.
+      </description>
+      <arg name="id" type="uint" summary="plane id"/>
+    </request>
[inline thread by ppaalanen at gmail.com (Pekka Paalanen) at Tue, 29 May 2018 17:10:02 +0300]
| What happens if any of the ids is invalid?
| I suppose the compositor just fails the lease.
|
| Should we not forbid using zero though, because that will always be
| invalid? In which case we need an error code for an invalid resource id
| protocol error (enum name="error" in the interface).
|
| Adding the same resource twice should be caught too.
+
+    <request name="create">
+      <description summary="create the lease">
+      This request is to be called last to get the lease. Either a 'created'
+      event in case of success, or 'failed' event in case of failure is
+      generated.
+      </description>
[inline thread by ppaalanen at gmail.com (Pekka Paalanen) at Tue, 29 May 2018 17:10:02 +0300]
| As Daniel suggested before, this request should create a new protocol
| object that represents the actual lease. All requests and events below
| would be part of that object's interface instead of kms_lease_request's.
|
| Let me call the new protocol object interface kms_lease, in lack of a
| better name. The lessee ID will not be needed in the protocol at all,
| it will be represented by the kms_lease protocol object. (Wayland
| objects are essentially typesafe per-client IDs.)
+    </request>
+
+    <request name="revoke">
+      <description summary="revoke a lease">
+       This asks to revoke a lease using the lessee id previously given in event
+       created.
[inline thread by ppaalanen at gmail.com (Pekka Paalanen) at Tue, 29 May 2018 17:10:02 +0300]
| What happens when a client asks for a revoke?
|
| Will it result in a revoked event?
|
| What are the requirements a client must have done with the leased DRM
| resources before it sends this request?
|
| What if a client simply closes the DRM fd it got with the lease and
| destroys this protocol object? Or does it in the opposite order,
| destroys first and closes then?
+      </description>
+      <arg name="id" type="uint" summary="lessee id"/>
+    </request>
+
+
+    <event name="created">
+    <description summary="lease created successfully">
+	This event indicates that the lease has been created. It provides the
+	leased fd which the client can use to perform modesetting and a lessee
+	id to revoke the lease when it has finished with it.
[inline thread by ppaalanen at gmail.com (Pekka Paalanen) at Tue, 29 May 2018 17:10:02 +0300]
| How should the compositor have programmed the leased DRM resources
| before sending this event?
|
| My question is related to flicker avoidance (avoid having the CRTC go
| through a temporary off-state if it was on already), and information
| leaks (the FB left on the CRTC could be queried and read back by the
| Lessee.)
|
| Should we instruct the compositor to program any CRTCs with FBs that do
| not contain any sensitive information? How can the compositor ensure
| those FBs get destroyed after the client has programmed its own FBs?
+    </description>
+	<arg name="fd" type="fd" summary="leased fd"/>
+	<arg name="id" type="uint" summary="lessee id"/>
+    </event>
+
+    <event name="failed">
+    <description summary="drm lease could not be created">
+	This event indicates that the lease could not be created/revoked.
[inline thread by ppaalanen at gmail.com (Pekka Paalanen) at Tue, 29 May 2018 17:10:02 +0300]
| + The client should destroy this object, and possibly try again with a
|   different set of DRM resources.
+    </description>
+    </event>
[inline thread by ppaalanen at gmail.com (Pekka Paalanen) at Tue, 29 May 2018 17:10:02 +0300]
| This interface is missing a destroy request. Interfaces must always
| have a destroy request unless there is a very good reason to not have
| one. In any case, every object must be destroyable somehow.
+
+    <event name="revoked">
+    <description summary="lease revoked">
+	This event indicates that the lease has been revoked.
[inline thread by ppaalanen at gmail.com (Pekka Paalanen) at Tue, 29 May 2018 17:10:02 +0300]
| Does the compositor send this event before or after it has called the
| revoke ioctl?
|
| I would assume after, which means that the client possibly starts
| hitting KMS errors before it gets this event. That could be worth to
| note here.
|
| On revoke, what happens to the FB on a leased CRTC? Can a compositor
| query and read it back? In this direction the information leak is
| probably ok.
+    </description>
+    </event>
+
+  </interface>
+
+</protocol>
[inline thread by ppaalanen at gmail.com (Pekka Paalanen) at Tue, 29 May 2018 17:10:02 +0300]
| Thanks,
| pq
--
2.9.3
