Hi Emmanuel, On 2 May 2016 at 22:40, Emmanuel Gil Peyrot <emmanuel.pey...@collabora.com> wrote: > Introduces a “same-as” configuration option for each output, which > bypasses the rest of the output configuration (mode, scale, transform > and seat) and instead makes it a clone of the specified output. > > This is implemented by splitting the drm_output struct into the > per-connector drm_output and the per-weston_output drm_logical_output, > with the latter containing one or more of the former in a wl_list.
Hmm. Let me stop you there ... > +struct drm_logical_output { > + struct weston_output base; > + struct wl_list output_list; > + > + int page_flip_refcount; This to me is a red flag. I think you've built this patch at the wrong level. There are really two kinds of clone mode: same-CRTC and different CRTC. Same-CRTC can share planes, and they share timing as well. I think same-CRTC clones should be ganged together within the backend, and different-CRTC clones should appear entirely disjoint from the backend's point of view. With the caveat that my review of Armin's drm_keep_current_mode patch shows that I comprehensively don't yet understand the new output configuration API - so I'd like his input on this as well - I'd suggest the following approach. Firstly, keep every connector as a separate weston_output, as it is today. When you parse an outputB same-as outputA configuration, call a new backend vfunc: outputA->chain_output(outputB). If this returns success, mark output B as chained from output A: not appearing in a surface's output_mask[0], not having dpms/repaint/... called on it, etc. If this returns failure, then continue to consider them as totally separate outputs. In the DRM backend, this would simply mean that we passed multiple connectors to drmModeSetCrtc and multiple connector_states bound to the same CRTC. This would remove a lot of complexity from the DRM backend, and also allow planes to work when we can clone multiple connectors on the same CRTC. It would also mean we avoid a possible drop to 30fps, if the two CRTCs end up clocked half the repaint cycle apart, and thus didn't call weston_output_finish_frame until too late. Plus it has the added benefit of working better when we have a constrained number of CRTCs, or constrained CRTC capability, or whatever. The core already has to deal with surfaces overlapping multiple 'real' outputs - i.e. running on different paint cycles - today, so I don't see that punting clones running on disjoint CRTCs makes that problem worse in terms of complexity for the core. We could certainly do better with repaint-cycle time reporting[1] in the core, but just punting down to the backend only makes that worse rather than better, I think. Doing it this way would make also make the patchset a lot less invasive, particularly if rebased on top of the atomic/plane_state/output_state patchset ... ;) Sorry to be the bearer of bad news. Cheers, Daniel [0]: Well. The client should still see the output for wl_surface::{enter,leave} events, but the backend should only see the 'real' outputs, not the chained ones. Maybe we need a client_output_mask, and a backend_output_mask ... ? [1]: Have a surface span two weston_outputs with disjoint paint clocks. Watch the surface's frame-event timing bounce between the two outputs, with resultant jittery framerate. Presumably we should pick one 'master' surface to clock from, and stick to that. _______________________________________________ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel