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.v...@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.v...@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 _______________________________________________ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
