[PATCH weston] RFC: libweston/compositor-drm: Add support for drm-lease.

[Pekka Paalanen]

On Thu, 25 Jan 2018 19:07:43 +0000
Marius-cristian Vlad <marius-cristian.vlad at nxp.com> wrote:

> -----Original Message-----
> From: Pekka Paalanen [mailto:pekka.paalanen at collabora.co.uk] 
> Sent: Thursday, January 25, 2018 2:06 PM
> To: Daniel Stone <daniel at fooishbar.org>
> Cc: Marius-cristian Vlad <marius-cristian.vlad at nxp.com>; Keith Packard <keithp at keithp.com>; wayland-devel at lists.freedesktop.org
> Subject: Re: [PATCH weston] RFC: libweston/compositor-drm: Add support for drm-lease.
> 
> On Thu, 25 Jan 2018 11:33:34 +0000
> Daniel Stone <daniel at fooishbar.org> wrote:
> 
> > Hi Marius,
> > 
> > On 25 January 2018 at 11:10, Marius-cristian Vlad 
> > <marius-cristian.vlad at nxp.com> wrote:  
> 
> > > >> +                       wl_signal_emit(&compositor->wake_signal, compositor);
> > > >> +                       
> > > >> + wl_event_source_timer_update(compositor->idle_source,
> > > >> +
> > > >> + compositor->idle_time * 1000);  
> > > >
> > > > I assume this is just to force a repaint. If the existing 
> > > > compositor API doesn't quite work for this, we should create API 
> > > > which does, or make sure enabling the output does the right thing. 
> > > > Are you using desktop-shell, or ... ?  
> > >
> > > [mvlad] Indeed. What I've observed is that it could be some time 
> > > until the repaint fires and in that time the fb of the client can 
> > > still be present on that output. Forcing a repaint seems to fix 
> > > that. There's also a longer explanation: If the client destroyes the 
> > > fb this would cause the connector to be disabled. If weston can 
> > > reclaim the connector after it has been disabled there's no issue.
> > > I will need to check this once more, it might not be needed after 
> > > all.  
> > 
> > Right. If we create/enable a new Weston output, this should result in 
> > repaint happening by itself: just like it does with hotplug now.  
> 
> Still, should we have the client wait for the compositor to have
> actually posted a repaint of the output before the client destroys
> its fb?
> 
> [mvlad] I would assume that a client will detroy its fb and,
> afterwards will revoke the lease. You won't be able the access the
> leased fd after revoking it. Weston will not be unable to repaint
> anything if there's currently no ouput to repaint on. I hope I
> understand your question correctly :/.  

Hi,

we can specify the protocol any way we want. If we specify that clients
must follow a certain sequence, we can expect them to follow that
sequence.

Sending the "release the lease" request, i.e. destroying the Wayland
protocol object representing the lease, is independent of actually
closing the DRM fd. If the leased DRM fd is revoked by the server,
would it mean that the lessee can no longer use the still open fd to
destroy FBs?

But, there must be a clean-up mechanism in the kernel as well, so maybe
there is no need for the lessee to explicitly destroy the last FB as
closing the DRM fd and subsequently the server setting its own FB to
the CRTC should cause the now orphaned lessee FB to be
garbage-collected.

For client initiated release, we could require the sequence: client
sends the release the lease request, client waits for the server to
process the release (produces an event), only then client closes the
DRM fd. The client would never rmfb the last FB.

My question is: is there any need for that? Would it make sense or not?

> 
> Do I understand right that the client destroying the fb would cause
> the CRTC and connector to be turned off immediately? Do we want to
> avoid that flicker if Weston is to take that output back into use?
> 
> [mvlad] Yes that is correct. RMFB will lead the CRTC and connector to
> be turned off. If there's no FB present the helpers in DRM atomic
> commit part will disable that CRTC.  

Right.

> That brings to my mind the opposite question: if weston stops using
> an output so that it can lease it out, how's the flicker avoidance in
> that case?
> 
> [mvlad] There seems to be no flicker when destroying the output with
> drm_output_destroy. This is rather blunt, but that's what I see
> happening. 

I would assume that's just a race, or a side-effect of the delayed
drm_output destruction as Daniel explained. I presume that in fact the
CRTC is still running with the server FB when the lessee sets its own.
I would expect there to be flicker if you actually guaranteed the
server's FB has been destroyed and the destruction handled by the
kernel (cross a vblank), before giving the lease to the client.

I believe one should guarantee that either the server FB stays up until
the client has taken over, or the server FB has been removed before the
client takes over.

> What about leaking fb contents between the lessor and lessee?
> 
> [mvlad] The client has its own fbs. What could happen is that the
> ouput "shared" by lessor/lessee can have inter-leaved fbs if the
> lesor is still using the output, but I see that happening only on
> purpose. 

I was thinking about read-back, not mixed-up display.

That is, if there is an FB set up by process A, then CRTC is handed
over to process B, then process B can query the current FB on the CRTC.
I believe it is possible to read back the FB contents. This would be
deliberately passing (e.g. smooth hand-over from, say, bootsplash to
login manager) or leaking (unintended data passing) the display
contents.

I think display servers are expected to ensure that the FB left on the
CRTC when they release the ownership of the CRTC (e.g. VT-switch away)
cannot contain anything sensitive.


> I'm kind of guessing that avoiding flicker is out of scope and it may
> well happen, and that preventing fb content leaking is more
> important. Is that right?
> 
> Does this result in toggling the CRTC off and on again on every
> hand-over? Given Weston's opportunistical usage of KMS resources,
> would it not create a risk of the lessee not being able to turn the
> CRTC back on if Weston manages to take more KMS resources (e.g.
> memory bandwidth via use of overlay planes) into use first?
> 
> [mvlad] Yes on every "contract" the CRTC will go thru that on/off
> stage. As long as weston is no longer using/owning that output
> (connector) I don't see how that's possible. How can it "take" more
> resources if it is not aware of that connector? Right?  

Going through CRTC on/off/on cycle seems wasteful and might take a bit
of time as well. It could be avoided if wanted, and avoiding it would
ensure Weston cannot exhaust the resources from the lessee, at least
for the primary plane.

A hypothetical scenario:

Let's say the display controller memory bandwidth is limited. It can
driver either two primary planes, or one primary plane and one big
overlay plane.

Start with Weston driving two connectors, which takes two primary
planes. Weston will fail to use the overlay plane for insufficient
memory bandwidth, so it falls back to compositing.

Lease one of the connectors to a client. The respective CRTC is
temporarily turned off. The kernel driver notices that the memory
bandwidth consumption has dropped. Weston repaints and again attempts
to use the overlay plane, as it does on every frame. This time it
succeeds, because one of the two primary planes is off. Weston programs
the remaining connector it has with one primary plane and one big
overlay plane.

When the client tries to set up its own FB on the connector it leased,
the kernel driver says it won't work, because there is not enough
memory bandwidth to drive all of two primary plane and a big overlay
plane at the same time.


This might be a contrived example, but there are more kinds of competed
resources than just the ones explicitly leased. The DRM lease design
purposefully ignores those shared but hidden resources, because they
are very much hardware dependent and properly exposing them would be
near impossible.

Aside from memory bandwidth, another example could be a limited memory
pool, e.g. CMA, from which FBs must be allocated from. The one who
allocates first wins. But I'm sure there are more.


I understand the original use case for DRM leases are HMDs. However,
it's not inconceivable to have e.g. games lease a standard display to
get full control of display updates and tearing. It that case, going
through CRTC on/off/on cycle would be really visible to the user, as
the same output is being used for normal desktop while not in game use.

Avoiding the on/off/on flicker is something that needs to be taken into
account already at the Wayland protocol extension design level, so that
we can get a guaranteed behaviour.


Thanks,
pq
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 833 bytes
Desc: OpenPGP digital signature
URL: <https://lists.freedesktop.org/archives/wayland-devel/attachments/20180126/20fe0079/attachment-0001.sig>