On Mon, 21 Nov 2016 19:41:33 +0000 Daniel Stone <dan...@fooishbar.org> wrote:
> 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. Hi, about the surface/output overlap here... we certainly do handle a surface overlapping several disjoint outputs, but I still have not convinced myself that we handle mutually overlapping outputs properly. Why do I say that? Because struct weston_plane has a member called 'damage', and drm_output_render() directly subtracts damage from the primary plane. Pretty much every backend does that. Thanks, pq > 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
pgpkGRAzBM4Kw.pgp
Description: OpenPGP digital signature
_______________________________________________ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel