[PATCH weston 3/6] compositor: set presentation.presented flags

[Pekka Paalanen]

On Fri, 16 Jan 2015 08:48:21 +0100
Mario Kleiner <mario.kleiner.de at gmail.com> wrote:

> Hi Pekka,
> 
> as you know from our off-list conversation i've spent quite a bit of 
> time the
> last week reviewing and testing this patch series (patches 1-6). I 
> implemented
> a first basic usable Waylang backend into my own software and tested 
> this patch
> series with it wrt. timestamp reliability etc. and it so far looks good 
> to me, at least
> for a first iteration of the presentation_feedback extension.
> 
> Tested on x11 backend, wayland backend, and mostly on the native drm 
> backend.
> On the drm backend i tested single- and dual-dispaly, on Intel-kms and 
> nouveau-kms,
> windowed and fullscreen, with/without overlay planes on, opaque or 
> transparent, etc.
> 
> My hardware measurement equipment confirms that timestamps are precise
> and reliable wrt. reality as long as the overlay planes are not used, ie.
> everything goes through regular kms-pageflip. Precision on the drm backend
> is better than 40 microseconds wrt. to the external measurement hardware,
> as expected, essentially perfect :)
> 
> So fwiw the whole series is
> 
> Reviewed-by: Mario Kleiner <mario.kleiner.de at gmail.com>
> Tested-by: Mario Kleiner <mario.kleiner.de at gmail.com>
> 
> and enthusiastically
> 
> Acked-by: Mario Kleiner <mario.kleiner.de at gmail.com>
> 
> for inclusion into Wayland/Weston 1.7, which is why i'm cc'ing Bryce for 
> hopefully getting this in.

Thank you Mario! You have done some hard work. :-)

Could someone push this series with Mario's tags added, please?
The major feature of Presentation feedback is already in, so I
think this is still ok wrt. to the release cycle.


> See below for one small thing i would probably change, after looking 
> through the
> way the hardware overlay planes are handled in Weston atm.
> 
> ...snip...
> 
> On 12/17/2014 03:20 PM, Pekka Paalanen wrote:
> > From: Pekka Paalanen <pekka.paalanen at collabora.co.uk>
> >
> > DRM:
> >
> > Page flip is always vsync'd, and always gets the completion timestamp
> > from the kernel which should correspond well to hardware. Completion is
> > triggered by the kernel/hardware.
> >
> > Vblank handler is only used with the broken planes path, therefore do
> > not report VSYNC, because we cannot guarantee all the planes updated at
> > the same time. We cannot set the INVALID, because it would abort the
> > compositor if the broken planes path was ever used.  This is a hack that
> > will get fixed with nuclear pageflip support in the future.
> 
> -> I think additionally to not reporting VSYNC i also would not set the
> PRESENTATION_FEEDBACK_KIND_HW_COMPLETION flag  in the vblank_handler.
> I think there are cases where the vblank event associated with a 
> drmModeSetPlane
> could get dispatched only 1 vblank after the plane update and i think 
> that shouldn't
> qualify for the hw completion flag. Essentially i'd rather wait for all 
> the nuclear pageflip
> work in the kernel to mature and be conservative with these flags.

Yes, that is reasonable.

> My feeling is that there might be a few more funky/scary things that 
> could happen
> wrt. timestamping when hardware overlays are used, but i just started 
> looking through
> that code, and i'm tired, so maybe i'm wrong. I couldn't test this so 
> far. I'm just happy
> that the overlay planes support is off by default at the moment :)

Yup, it is off, because it is know to be seriously broken. :-)
E.g. it usually causes Weston's framerate to drop to half. You just
shouldn't use it, so I don't think fixing the flags for a case that
is known broken is important. The important thing is to get this
series in for 1.7.

> Anyway, more feedback later. I'll play with this stuff throughout the 
> next days and weeks,
> i just wanted to make sure that you get some timely feedback before 
> Wayland/Weston 1.7.

Thank you very much again,
pq