Permalink
Find file Copy path
Fetching contributors…
Cannot retrieve contributors at this time
517 lines (512 sloc) 24.6 KB
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