On Wed, 5 Sep 2012 21:38:27 +0200 Daniel Vetter <daniel.vetter at ffwll.ch> wrote: > On Wed, Sep 5, 2012 at 8:09 PM, Jesse Barnes <jbarnes at virtuousgeek.org> wrote: > > On Wed, 29 Aug 2012 12:34:04 +0200 > > Daniel Vetter <daniel.vetter at ffwll.ch> wrote: > > > >> This is definetely a bit more generic than currently required, but > >> if we keep track of all crtcs that need to be disabled/enable (because > >> they loose an encoder or something similar), crtcs that get completely > >> disabled and those that we need to do an actual mode change on nicely > >> prepares us for global modeset operations on multiple crtcs. > >> > >> The only big thing missing here would be a global resource allocation > >> step (for e.g. pch plls), which would equally frob these bitmasks if > >> e.g. a crtc only needs a new pll. > >> > >> These masks aren't yet put to use in this patch, this will follow in the > >> next one. > >> > >> v2-v5: Fix up the computations for good (hopefully). > >> > >> v6: Fixup a confusion reported by Damien Lespiau: I've conserved the > >> (imo braindead) behaviour of the crtc helper to disable _any_ > >> disconnected outputs if we do a modeset, even when that newly disabled > >> connector isn't connected to the crtc being changed by the modeset. > >> > >> The effect of that is that we could disable an arbitrary number of > >> unrelated crtcs, which I haven't taken into account when writing this > >> code. Fix this up. > > > > Might not need to clean it up here though, since the only time that > > should matter is during the first mode set after a power on or resume > > when a superfluous CRTC might be enabled. And with the new hw state > > readout, we can fix that up. > > > > You choose though. > > If your comment is just about the v6 fixup, that's definitely need > here. The failure mode is that we unplug a connector, then the window > systems disables _all_ connectors since it wants to reassign them (or > change the fb or whatever, atomic modeset would fix this better). On > the first modeset we then disable crtc 0, but crtc 1 gets disabled, > too (since the connector is now disconnected). Since I didn't consider > things, the state tracking would get out of synced with reality and > not enable crtc 0 again on the new framebuffer (worked though when > retrying). > > If the comment is just about changing the behaviour, that's definitely > planned, but as a follow-up cleanup - it changes the abi behaviour, so > we need to have it in a separate patch for revertability. > > Or do you mean something totally different? No that covers it, thanks for both explanations. -- Jesse Barnes, Intel Open Source Technology Center
