[PATCH wayland-protocols v3] unstable/drm-lease: DRM lease protocol support
Marius-cristian Vlad
marius-cristian.vlad at nxp.com
Fri Aug 24 09:21:59 UTC 2018
On Fri, 2018-08-24 at 10:57 +0200, Philipp Zabel wrote:
> Hi Pekka,
>
> On Thu, 2018-08-23 at 14:39 +0300, Pekka Paalanen wrote:
> > Sorry for the potentially duplicate email, my MUA messed up the CC
> > list
> > on the first go.
> >
> >
> > On Thu, 23 Aug 2018 08:41:30 +0200
> > Philipp Zabel <p.zabel at pengutronix.de> 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?
> >
> > Hi,
> >
> > yes, sending one event per connector is a good design, but see
> > below if
> > we actually might want to extend that to creating an object per
> > connector with "per-attribute" events for pieces of information.
>
> One reason for creating an object would be that in some cases (like
> VR)
> the client will want to match on vendor/model/serial from EDID, but
> in
> other scenarios EDID data might not even be available.
> When leasing a DPI or LVDS panel without EDID information, the
> connector
> would have to be identified via the connector name, for example.
>
> And object would allow handling both cases the same way when building
> the lease request.
To make sure I understand this "object" leasing. Instead of advertising
connectors this way:
<event name="connector">
<description summary="advertise a leasable connector">
This event advertises a leasable connector. This allows the
client to
choose which connector the compositor should lease. It can
either
use connector name, monitor name, or if that's not sufficient
to parse
EDID blob.
</description>
<arg name="conn_name" type="string" summary="connector name" />
<arg name="eisa_id" type="string" summary="eisa id"/>
<arg name="monitor_name" type="string" summary="monitor name"/>
<arg name="pnp_id" type="string" summary="pnp id"/>
<arg name="serial_num" type="string" summary="serial number"/>
<arg name="edid" type="array" summary="EDID blob"/>
</event>
We do something like this:
<event name="connector_object">
<description summary="advertise a leasable connector">
This event advertises a leasable connector object.
</description>
<arg name="conn_obj" type="new_id"
interface="zwp_kms_lease_connector_v1"
summary="the new temporary" />
</event>
Then that interface is used to query info (like EDID/conn_name/monitor
name) and using that information to ask/revoke the lease (based on
either connector name, or based on EDID)?
Is this the right assumption?
>
> > > > 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?
> >
> > One certainly cannot rely on wl_output, because that will not be
> > present for outputs that are not currently used as part of the
> > desktop.
> > IOW, HMDs are likely never exposed as a wl_output.
>
> Understood, it would have to be a new object. There could be more of
> them than wl_outputs, and they wouldn't carry the same information:
> output geometry obviously has no place here. But there is some
> overlap.
> The wl_output geometry event also contains "make" and "model"
> arguments <event
> name="connector"><
>
> <description summary="advertise a leasable
> connector"><
> This event advertises a leasable connector. This allows the
> client to<
> choose which connector the compositor should lease. It can
> either<
> use connector name, monitor name, or if that's not sufficient
> to parse<
> EDID
> blob.<
>
> </description><
>
> <arg name="conn_name" type="string" summary="connector name"
> /><
> <arg name="eisa_id" type="string" summary="eisa
> id"/><
> <arg name="monitor_name" type="string" summary="monitor
> name"/><
> <arg name="pnp_id" type="string" summary="pnp
> id"/><
> <arg name="serial_num" type="string" summary="serial
> number"/><
> <arg name="edid" type="array" summary="EDID
> blob"/><
> </event><
>
> (but is missing product id or serial). And I notice that
> zxdg_output_v1
> already has a "name" event that likely contains the connector name.
> Maybe the leasable connector object should report all of these as
> events.
>
> > > Or should the leasing be done by the application itself, outside
> > > of
> > > Vulkan, and just the zwp_kms_lease_v1 object be passed in?
> >
> > These are good questions, I hope answers will be found for what the
> > Vulkan and other APIs will actually need for advertising what they
> > need
> > to advertise.
>
> I suppose this is something that should be discussed on the Vulkan
> side
> first? Given we'd need a new extension there anyway, maybe the
> solution
> is not making this wayland specific at all and just adding a way to
> import the leased DRM fd.
>
> > > > > Especially EDID info would be most useful for finding the
> > > > > right VR
> > > > > headset.
> >
> > Sharing EDID could be tricky mechanically. If we assume that an
> > average
> > EDID blob is 256 bytes, it would be small enough to pass "inline"
> > in
> > Wayland, e.g. as a wl_array argument on an event. But since we want
> > to
> > provide it for all leasable connectors, the burst of data could
> > grow
> > bigger than the buffers in libwayland, and we start relying on the
> > kernel socket buffers which are much larger usually.
>
> I'm not sure if pushing the complete EDID is necessary. Maybe there
> should be an EDID request if preparsed vendor/product/serial or
> connector name are good enough to identify the to-be-leased connector
> in
> most cases.
>
> > Maybe it's ok to start with a wl_array while we are still in
> > unstable
> > protocol, but this question may need to be revisited in the future.
> >
> > Is there a defined upper limit on EDID size?
>
> EDID contains an 8-bit number of extension blocks, there should be an
> upper limit of about 32 KiB.
>
> > Btw. since we may need to share EDID, and we may need applications
> > to
> > parse EDID to dig out what they need, I believe there will be a
> > demand
> > for a library to do that. Does any such exist already?
>
> I'm not sure if there is a commonly used one.
>
> [...]
> > The client should not receive "all" the data from the compositor
> > via Wayland, but only the bits it needs to make the decision
> > whether to
> > attempt leasing a connector or not.
>
> Seconded, the protocol should contain just enough to make a decision
> to
> lease.
>
> > Modes are part of EDID, so if we send EDID, I think that's good
> > enough.
> > After all, we want to describe the monitor/HMD foremost, not what
> > the
> > kernel or the compositor have decided it can do (e.g. add standard
> > modes the EDID might miss, or prune modes that cannot be used).
>
> What about legacy LVDS and DPI displays that have no EDID
> information?
> I expect those would be matched by connector name, but maybe there
> are
> also use cases where somebody would select a connector to lease
> depending on its native resolution or refresh rate.
>
> > That data does not need to be complete either, I think. It should
> > allow
> > coarse pruning, and yes, ideally it would allow choosing the right
> > connectors immediately, but clients still have the opportunity to
> > lease
> > a connector and ask DRM for more details before making the
> > decision.
>
> Ok.
>
> > In summary, I don't know if we need to send the whole EDID or just
> > selected standard fields from EDID (make, model, serial, ...), and
> > likewise I'm not sure there is need to send video mode info if
> > that's
> > not useful for choosing a connector/monitor.
>
> For the use cases that I can currently imagine (VR, special fixed
> displays, projectors on a special connector) either
>
> manufacturer id (make), product code (better than monitor name
> string) and monitor serial number
> or
> connector name
>
> should be good enough. But as said, I'm not sure if there could be
> cases where just matching native resolution or refresh rate would
> be required.
>
> > After the client has chosen a connector and leased it, then it will
> > receive everything it needs directly from the kernel, e.g. a video
> > mode
> > list.
>
> Yes.
>
> > > 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.
> >
> > That depends on how much data we want to send. A single message has
> > a
> > quite limited size. libwayland buffering sets a hard limit at 4096
> > bytes, but in practise we should use, say, 1024 bytes max. If that
> > message includes EDID inline, that's already 128-512 bytes taken.
> >
> > From protocol specification point of view, if the ad is just one
> > event
> > that carries all per-connector information, it cannot be extended.
> > If we
> > later want to include more information, we need to duplicate the
> > event
> > to add new arguments to it, because existing events cannot be
> > modified
> > without breaking all consumers. OTOH, if each piece of information
> > is
> > sent as a separate event on a "leasable connector" object, adding
> > new
> > pieces of information is much cleaner, no need to duplicate the
> > existing events.
>
> If we had an object, we could start with an event that just carries
> model/vendor/serial and add an edid event (possibly even with an edid
> request) later, if necessary.
>
> [...]
> > > Instead of submitting the connector identification as an argument
> > > to the
> > > create request, would adding a separate add_connector request be
> > > more
> > > extensible?
> > >
> > > That would allow to request a single lease for multiple
> > > connectors,
> > > or to add a mechanism to request adding overlay planes to the
> > > lease.
> >
> > If there is any possibility that we might want to build a single
> > lease
> > request from additional bits (e.g. planes) in the future, then I
> > would
> > recommend the add_connector request on a tentative lease config
> > object
> > approach. The linux_dmabuf extension already uses the same pattern
> > to
> > have a variable number of things in the final request.
> >
> > As said, one cannot add arguments to an existing request or event
> > later
> > - you have to replace the whole message if you want something
> > added.
> >
> > Sorry, I'm a bit too busy to do a proper review of the protocol for
> > now, but I hope these comments push it in a good direction.
>
> Thank you for the detailed initial feedback.
>
> regards
> Philipphttps://www.youtube.com/watch?v=QulEM-Azzughttps://www.youtube
> .com/watch?v=QulEM-Azz
More information about the wayland-devel
mailing list