On Thu, 2018-08-30 at 14:38 +0200, Philipp Zabel wrote: > On Thu, 2018-08-30 at 11:01 +0000, Marius-cristian Vlad wrote: > [...] > > > One interesting question is how to handle the situation when the > > > client deliberately, or not, holds the lease indefinitely. > > > > The client implicitly cancels the lease by closing the lease fd (or > > by > > having it closed automatically when it is terminated). The > > compositor > > will get a hotplug event then, and it has to check the lessee list > > when this > > > happens and revoke leases to lost lessees. > > > > Mvlad: That's what I expected to see, but I did some brief tests > > here, > > and it could the our lease kernel version could a little bit older, > > but when SIGKILL'ing the client, the DRM subsystem doesn't revoke > > the > > lease, or calls drm_lease_destroy. I suppose the following happens: > > when creating the lease the compositor will hold a reference of the > > leased fd -- well technically the DRM leases part does -- (and ofc > > I > > can't close it then because the client won't be able to use it), > > The compositor has to close lease_fd after sending it to the client > via > zwp_kms_lease_v1_send_leased_fd. Otherwise there's two open > references > to the lease drm_master, and closing only the client's reference will > not cause it to be destroyed.
Yes, you are correct. I mistakenly though I can only close it after the client sends the revoke request. > > > we pass that thru weston (over the unix socket), the client will > > also > > hold a reference for that leased fd, so basically the lease will > > only > > be destroyed when that ref counter hits 0. In the implementation I > > close the leased fd deliberately on the compositor side, when I get > > the revoke request from the client. > > If there is only one open lease_fd reference in the client, there is > no > need for the revoke request. The client can just close the fd and the > compositor will revoke the lease object after gets the hotplug event > and > notices the lessee is gone. > > Now if the client keeps the leased_fd open but requests to destroy > the > zwp_kms_lease_v1 object, the compositor must revoke the DRM lease, > same > as when a VT switch happens or when the connector gets unplugged. Alright I'll re-work this part and send an update. > > > There's no issue when the client behaves, but only when the client > > dies abruptly, and it won't notify the compositor that it is no > > longer > > using the lease. I've only seen this once when you brought it up, > > so I > > need to test it further to be sure about this. > > My understanding is that if the client dies, the last open fd > referencing the lease drm_master is closed Yep, I've tested it now and that's how it works. > > > > This has multiple > > > ramification like what happens when the client un-expectingly > > > dies and > > > doesn't revoking the lease, > > > > See above, a lease terminates when its last fd is closed. > > > > Mvlad: I'm not really sure on this, see my above comment. > > > > > or when hot-plugging the connector and weston tries to get a > > > hold of > > > a connector previously leased? > > > > I'd say hot-unplugging a connector that is part of a lease should > > cause this lease to be canceled. > > > > mvlad: Which means that the client would have to be notified when > > that happens, so it can shut down cleanly. > > Could that be via the revoked event on the zwp_kms_lease_v1? I'd say yes. > > > Somehow the client (in its rendering part perhaps) has to check if > > the lease is still valid. Guess we can also > > verify errno, so it should not complicate the client that much. > > Right. If at some point the client suddenly fails to pageflip, it > should > be able to cope. > > [...] > > Mvlad: This relates also the above comments as well. Need to verify > > if > > indeed it works like this. But a follow up question here regarding > > planes: > > We've discussed this quite extensively, and I've concluded that it > > is > > much easier to gather the objects ids required to create the lease, > > directly in the compositor as it already has that information. I > > kind > > of assume that the connector object will also provide the CRTC and > > implicitly the planes it can drive. > > I think the connector should bring its CRTC and a primary plane. > > At least for devices that have overlay planes that can move between > CRTCs, overlay planes should not be included implicitly without the > client asking for them, because the compositor may have use for them. > > I suppose for devices that have planes fixed to CRTCs it wouldn't > hurt > to just lease all planes for the connected CRTC, but for consistency > I > wouldn't add unrequested overlay planes here either. > > Overlay planes could be listed as zwp_kms_plane_v1 objects, just as > connectors. Uhum, alright. I'll have to test this as well. Thanks for the feedback. Will send an update soon. > > regards > Philipp _______________________________________________ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel