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

[Drew DeVault]

On Mon Jul 15, 2019 at 5:46 PM Simon Ser wrote:
> > +    <request name="stop">
> > +      <description summary="stop sending events">
> > +        Indicates the client no longer wishes to receive connector events. The
> > +        compositor may still send connector events until it sends the finish
> > +        event, however.
> > +
> > +        The client must not send any requests after this one.
> 
> Protocol error?

ack

> > +  <interface name="zwp_drm_lease_connector_v1" version="1">
> > +    <description summary="a leasable DRM connector">
> > +      Represents a DRM connector which is available for lease. These objects are
> > +      created via zwp_drm_lease_manager_v1.connector, and should be passed into
> > +      lease requests via zwp_drm_lease_request_v1.add_connector.
> 
> request_connector? (Maybe add_connector is a better name?)

nack, this doesn't represent a connector which has been added to a
request, but a connector which is available to be.

> > +    <event name="description">
> > +      <description summary="description">
> > +        The compositor sends this event once the connector is created to provide
> > +        a human-readable description for this connector, which may be presented
> > +        to the user. This will not change for the duration of the Wayland
> > +        session, but is not guaranteed to be consistent between sessions.
> 
> There is a pending patch to make xdg_output.description mutable.
> 
> > +        If the compositor also supports zxdg_output_manager_v1 and this
> > +        connector corresponds to a zxdg_output_v1, this description will match
> > +        the description of this zxdg_output_v1 object.
> 
> This is probably not strictly necessary, and will make it difficult to honour if
> xdg_output.description becomes mutable (both events aren't applied atomically).

ack

> > +      </description>
> > +      <arg name="name" type="string" summary="connector name" />
> 
> s/name/description/g

ack

> > +    <event name="edid">
> > +      <description summary="edid">
> > +        The compositor sends this event once the connector is created to
> > +        provide a file descriptor which may be memory-mapped to read the
> > +        connector's EDID, to assist in selecting the correct connectors
> > +        for lease.
> 
> Make it necessary to map with MAP_PRIVATE. See [1] for details.
> 
> Some connectors may not have an EDID (e.g. old VGA outputs, or when the EDID is
> corrupted). Maybe this event should be optional?
> 
> [1]: https://gitlab.freedesktop.org/wayland/wayland/commit/905c0a341ddf0a885811d19e2b79c65a3f1d210c

ack

> > +    <event name="withdrawn">
> > +      <description summary="lease offer withdrawn">
> > +        Sent to indicate that the compositor will no longer honor requests for
> > +        DRM leases which include this connector. The client may still issue a
> > +        lease request including this connector, but the compositor will send
> > +        zwp_drm_lease_v1.finished without issuing a lease fd.
> 
> What if multiple connectors have been requested?
> 
> Maybe it's safer to say that the compositor won't issue a lease with this
> connector.

This already does say that, it just words it more specifically to
address possible races. Per your next bit of feedback...

> > +    <event name="lease_fd">
> > +      <description summary="shares the DRM file descriptor">
> > +        This event returns a file descriptor suitable for use with DRM-related
> > +        ioctls. The client should use drmModeGetLease to enumerate the DRM
> > +        objects which have been leased to them, which may not be the objects
> > +        they requested. The lease may have zero DRM objects.
> 
> Zero objects is not possible IIRC, right?

I'm going to update this wording a bit to make it clearer. Zero objects
is indeed not possible today but I wanted to leave the door open to
future kernel changes without the need to update this protocol.