[PATCH wayland-protocols v3] unstable/drm-lease: DRM lease protocol support
Marius-cristian Vlad
marius-cristian.vlad at nxp.com
Wed Aug 22 11:12:20 UTC 2018
On Wed, 2018-08-22 at 09:17 +0200, Philipp Zabel wrote:
> Hi Marius,
Hi,
>
> On Mon, 2018-08-20 at 23:00 +0300, Marius Vlad wrote:
> > Simple protocol extension to manage DRM lease. Based on the work by
> > Keith
> > Packard in [1], respectively [2].
> >
> > [1] https://emea01.safelinks.protection.outlook.com/?url=https%3A%2
> > F%2Fcgit.freedesktop.org%2Fmesa%2Fdrm%2Fcommit%2F%3Fid%3Dc417153538
> > 9d72e9135c9615cecd07b346fd6d7e&data=02%7C01%7Cmarius-
> > cristian.vlad%40nxp.com%7C44bf17ed24d748c059fb08d607ff5fda%7C686ea1
> > d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C636705190740857187&sdata=M
> > gPbYibU3CTjKcSMJaiqBSP7FfAJD2h%2BLPYiE%2FXJ8z0%3D&reserved=0
> > [2] https://emea01.safelinks.protection.outlook.com/?url=https%3A%2
> > F%2Fgit.kernel.org%2Fpub%2Fscm%2Flinux%2Fkernel%2Fgit%2Ftorvalds%2F
> > linux.git%2Fcommit%2F%3Fh%3Dv4.15-
> > rc9%26id%3D62884cd386b876638720ef88374b31a84ca7ee5f&data=02%7C0
> > 1%7Cmarius-
> > cristian.vlad%40nxp.com%7C44bf17ed24d748c059fb08d607ff5fda%7C686ea1
> > d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C636705190740857187&sdata=S
> > h%2B1TbPeqbUnaYF%2FwenCZIKTcR9P7l585Ellujk0Bq8%3D&reserved=0
> >
> > Signed-off-by: Marius Vlad <marius-cristian.vlad at nxp.com>
> >
> > Changes since v2:
> > - advertise connector names when creating a lease request object
> > (Pekka Paalanen)
> > - when revoking the lease use the lease object instead of lessee id
> > (Pekka Paalanen)
> > - various grammar/spelling fixes (Pekka Paalanen)
> >
> > 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)
> > ---
> > Some caveats while at it. This is just in RFC form adapted from the
> > comments
> > I've mostly got from Pekka Paalanen. It most certainly doesn't
> > address all the issues
> > brought up: like multi-node card environments/system, doing a
> > TEST_ONLY
> > commit before giving out a lease, or takes hot-plugging into
> > consideration.
> > As I was side-tracked to other things and
> > being on hiatus for a while, I wanted first to get some impressions
> > first if
> > indeed this is the right approach and do some incremental updates
> > afterwards.
> >
> > The are some issues which I'd like to point out from the beginning,
> > which
> > require some further comments. In no particular order:
> >
> > I've found that I can't pass a wl_array as a way to advertise
> > various
> > information to the client. (wl_array works fine with integers not
> > with
> > allocated data). It would probably be better to advertise also
> > monitor name, other EDID info or available modes, but at the moment
> > I only
> > join-split a delimiter between connector names and send it out as a
> > string.
> > While it is ugly I'm not aware of a way to send this information
> > back to the
> > client in the form of a list.
> >
> > The client is aware before hand of this delimiter and has the
> > number of entries:
> > can easily choose or pick one of the connectors. It could be that
> > there are
> > some other ways this can be achieved, I welcome any kind of
> > feedback here.
>
> Why not just send the connectors one by one, a single event with all
> relevant information for each?
Hmm, okay, I'll try do that.
>
> Especially EDID info would be most useful for finding the right VR
> headset.
>
> > Secondly, when doing a modeset, the client requires a valid mode
> > (drmModeModeInfo). I'm currently unsure how to pass this back to
> > the client.
> > The obvious type="object" interface="drmModeModeInfo" fails to link
> > and instead
> > I rely on the fact that a) the client can retrieve IDs from the
> > lease using
> > lease API (drmModeGetLease()) which gives a connector id --
> > or alternately can also use a wl_array to pass that,
> > and b) the client can use the leased fd to iterate over connectors.
> > Matching those two would allow to get a valid
> > drmModeModeInfo object to pass it when modesetting (for more info
> > see the client implementation provided at the end).
> > Question is, is this an acceptable way of doing it? Do note that
> > this
> > can only be "used" after the lease has been created.
>
> Can't the client query available modes on the passed connector via
> the
> leased fd?
That's how the client does it now, it uses the leased fd to query
available modes. Presumably the client should have/receive all the data
from the compositor....
>
> > A server/client implementation to match this protocol
> > can be found at https://emea01.safelinks.protection.outlook.com/?ur
> > l=https%3A%2F%2Fgitlab.freedesktop.org%2Fmarius.vlad0%2Fweston%2Fco
> > mmits%2Fnew-drm-lease&data=02%7C01%7Cmarius-
> > cristian.vlad%40nxp.com%7C44bf17ed24d748c059fb08d607ff5fda%7C686ea1
> > d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C636705190740857187&sdata=G
> > 4P%2F9BhTT8i0RI3bRpYsXPJQQ0uJjmp9TL8UrboMgDI%3D&reserved=0
>
> This crashed for me in drm_lease_manager_create_lease_req with a NULL
> pointer dereference because head->output == NULL for an unconnected
> head.
> Also I noticed zwm_kms_lease_request_v1_implementation is missing the
> .destroy request callback.
Good catch, fixed. You need to fetch it again.
>
> > Makefile.am | 1 +
> > unstable/drm-lease/README | 4 +
> > unstable/drm-lease/drm-lease-unstable-v1.xml | 173
> > +++++++++++++++++++++++++++
> > 3 files changed, 178 insertions(+)
> > create mode 100644 unstable/drm-lease/README
> > create mode 100644 unstable/drm-lease/drm-lease-unstable-v1.xml
> >
> > diff --git a/Makefile.am b/Makefile.am
> > index 6394e26..5d13dca 100644
> > --- a/Makefile.am
> > +++ b/Makefile.am
> > @@ -4,6 +4,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/text-input/text-input-unstable-v3.xml
> > \
> > unstable/input-method/input-method-unstable-v1.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..6cb3c0a
> > --- /dev/null
> > +++ b/unstable/drm-lease/drm-lease-unstable-v1.xml
> > @@ -0,0 +1,173 @@
> > +<?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 Lessor using the leased file-descriptor passed
> > by the Lessor.
> > +
> > + Besides the leased file-descriptor, an integer is used to
> > uniquely
> > + 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.
> > +
> > + 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">
> > + 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 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>
> > +
> > + <event name="connectors">
> > + <description summary="advertise which connectors can be used
> > to request creation for a lease">
> > + This event advertises leasable connectors. When multiple
> > connectors are
> > + advertised, the client should properly parse and choose
> > one of connectors
> > + A client trying to create a lease using
> > zwp_kms_lease_request_v1::create
> > + request would use this connector name.
> > + </description>
> > + <arg name="connectors" type="string" summary="list of
> > connector entries, split by '|' char"/>
> > + <arg name="connectors_entries" type="uint" summary="number
> > of connectors found in connectors string"/>
> > + </event>
> > +
> > + </interface>
> > +
> > + <interface name="zwp_kms_lease_v1" version="1">
> > + <description summary="the lease object itself">
> > + This interface represents the lease object and encapsulates
> > the lease
> > + properties. This objected is sent by
> > zwp_kms_lease_request_v1::created
> > + event. Use this object to retrieve lease properties.
> > + </description>
> > +
> > + <request name="destroy" type="destructor">
> > + <description summary="destroys the temporary lease request
> > object">
> > + This has no effect on any remaining objects created
> > through this
> > + interface.
> > + </description>
> > + </request>
> > +
> > + <request name="get_lease_info">
> > + <description summary="request to retrieve info about the
> > lease">
> > + Request to retrieve lease information, like leased fd,
> > monitor
> > + and connector name.
> > + </description>
> > + </request>
> > +
> > + <event name="lease_info">
> > + <description summary="returns information about the lease">
> > + This event returns (among other information about the
> > connector leased)
> > + the leased fd which is required for modesetting or queying
> > page flips.
> > + The client can use the leased fd to find out DRM-related
> > information
> > + like connector, CRTC, and plane ID using
> > drmModeGetLease(). Using that
> > + information it can derive a suitable mode useful when
> > performing modeset.
> > + </description>
> > + <arg name="leased_fd" type="fd" summary="leased fd" />
> > + <arg name="conn_name" type="string" summary="connector name
> > of the lease" />
> > + <arg name="monitor_name" type="string" summary="monitor
> > name" />
> > + </event>
> > + </interface>
> > +
> > + <interface name="zwp_kms_lease_request_v1" version="1">
> > + <description summary="lease request object">
> > + This interface is used for managing zwp_kms_lease_v1 object.
> > It is used
> > + to create a zwp_kms_lease_v1 object (the actual lease
> > object) and also to
> > + revoke the lease.
> > + </description>
> > +
> > + <request name="destroy" type="destructor">
> > + <description summary="destroys the lease request object">
> > + This has no effect on any remaining objects created
> > through this
> > + interface.
> > + </description>
> > + </request>
> > +
> > + <request name="create">
>
> There's a level of indentation is missing from here.
>
> > + <description summary="create">
> > + Create a lease using the connector output name received in
> > zwp_kms_lease_manager_v1::connectors.
> > + Any attempt to use an incorrect connector name will reswult
> > in zwp_kms_lease_request_v1::failed.
> > + </description>
> > + <arg name="connector" type="string" summary="connector output
> > name"/>
> > + </request>
> > +
> > + <request name="revoke">
> > + <description summary="revoke">
> > + Revoke the lease, using the zwp_kms_lease_v1 objected
> > received in
> > + zwp_kms_lease_request_v1::created event.
> > + </description>
> > + <arg name="lease_obj" type="object"
> > interface="zwp_kms_lease_v1" summary="lease object to remove" />
> > + </request>
> > +
> > + <event name="created">
> > + <description summary="lease created successfully">
> > + This event indicates that the lease has been created. It
> > provides a zwp_kms_lease_v1 object
> > + used for retrieving the fd representing the lease.
> > + </description>
> > + <arg name="lease_obj" type="new_id"
> > interface="zwp_kms_lease_v1" summary="the lease obj"/>
> > + </event>
> > +
> > + <event name="failed">
> > + <description summary="lease could not be created">
> > + This event indicates that the lease could not be
> > created/revoked.
> > + </description>
> > + </event>
> > +
> > + <event name="revoked">
> > + <description summary="lease revoked successfully">
> > + This event indicates that the lease has been revoked.
> > + </description>
> > + </event>
> > +
> > + </interface>
> > +</protocol>
>
> regards
> Philipp
More information about the wayland-devel
mailing list