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

[Marius-cristian Vlad]

On Thu, 2018-08-23 at 08:41 +0200, Philipp Zabel wrote:
> Hi,
> 
> On Wed, 2018-08-22 at 11:12 +0000, Marius-cristian Vlad wrote:
> [...]
> > > 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. 
> 
> I'm wondering what should be used to identify a connector to a
> hypothetical Vulkan VK_EXT_acquire_wayland_display extension, or to
> other APIs that may request leases on the application's behalf.
> 
> What could be passed into a Vulkan function to request the lease and
> retrieve the corresponding VkDisplayKHR object? wl_display and
> make/model/serial strings? wl_display and drm connector name?
> Or is there need for an object describing a leasable connector,
> similar to wl_output?
> 
> Or should the leasing be done by the application itself, outside of
> Vulkan, and just the zwp_kms_lease_v1 object be passed in?
> 
> > > 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....
> 
> If there was a "leasable connector" object instead of just the
> connector
> event, that could report modes and EDID data as events, like
> wl_output.
> I'm not sure if that is a good idea or unnecessary complication.
> 
> > > > 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%7C68
> > > > 6ea1
> > > > d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C636705190740857187&sda
> > > > ta=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.
> 
> Thank you, it works now. I've poked at it a bit, and noticed that the
> compositor crashes if the same lease is requested again before it was
> improperly revoked. This happened in three cases:
> 
> - if a second weston-egl-simple-lease is started while the first one
>   is still running,
> - if weston-egl-simple-lease is stopped by a VT switch (because the
>   next pageflip fails) and started again after switching back to
> weston,
> - if weston-egl-simple-lease is killed with SIGTERM and started
> again.
> 
> In the last case the last frame the last frame of the already killed
> application was still visible on the output.
> 
> I've also tried pulling and replugging the cable on the leased
> connector, which ends with a compositor assertion after replugging:

Yes, all of those have to be fixed. I'll send an updated version of
the implementation for a new version of the protocol. 

Thanks for testing this!

> 
> weston: compositor/main.c:1726: drm_process_layoutput: Assertion
> `output->output->enabled' failed.
> 
> regards
> Philipp