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

[Daniel Vetter]

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?
-Daniel
-- 
Daniel Vetter
daniel.vetter at ffwll.ch - +41 (0) 79 365 57 48 - http://blog.ffwll.ch