[PATCH wayland-protocols v4] unstable/drm-lease: DRM lease protocol support

[Philipp Zabel]

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.

>  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.

>  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

> >  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?

> 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.

regards
Philipp