[Intel-gfx] [PATCH] drm/i915: compute masks of crtcs affected in set_mode

[Jesse Barnes]

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