Mir

Code review comment for lp:~kdub/mir/display-groups

Revision history for this message
Kevin DuBois (kdub) wrote :

@code rewrite:
The current interfaces are insufficient for android multimonitor, because android has a weird commit interface (line 587, https://android.googlesource.com/platform/hardware/libhardware/+/master/include/hardware/hwcomposer.h). Our current platform interfaces do not have explicit hints about threading, and this avoids this by saying "this group of monitors must post together in DisplayGroup".

I tried to work within the existing API and thread model in lp:~kdub/mir/hwc-sync-and-post-external, but this led to some complex, not-universal and heavy-handed synchronization. (eg, You couldn't drive that code from one thread, one thread per display was the only way. Furthermore, the displays were still synced together, making the second thread mostly useless)

(2) Its true that having two DisplayBuffers in one DisplayGroup ties the displays together, but mesa remain 1DisplayBuffer/1DisplayGroup, so mesa can still be driven in its preferred way, that is, one thread that syncs at the vsync rate of the respective monitor. Android also does not wait in mg::DisplayGroup::post(), so the disadvantages of having one thread drive 2 monitors is less severe. I share the sentiment that the android mm interface is strange, but we have to sync between the different display threads for the interface.

(3) Its important to remember that android's set() call does not block, so the timings are quite different than a model that relies on waiting on the vsync of a particular display. The vsync signal and the commit call are detached; the buffers are protected by fences. Its more that we're scheduling work in the kernel as opposed to controlling the timing carefully like in KMS/drm.

I did look into using OverlappingOutputGrouping in the same way that mesa does, that is, to model 2 monitors an 1 DisplayBuffer, with one common scanout buffer. This relies on being able set the scanout buffer of the monitors, which android doesn't let us do. We need one eglContext per display, and cannot force both monitors to read from one bundle of framebuffers.

« Back to merge proposal