[Intel-gfx] [PATCH 2/7] drm/i915: Fixup Oops in the pipe config computation

[Daniel Vetter]

On Fri, Apr 12, 2013 at 11:46 AM, Chris Wilson <chris at chris-wilson.co.uk> wrote:
> On Thu, Apr 11, 2013 at 04:29:05PM +0200, Daniel Vetter wrote:
>> Yet again our current confusion between doing the modeset globally,
>> but only having the new parameters for one crtc at a time.
>>
>> This time things blew up when restoring modes in the switchless resume
>> code - intel_modeset_affected_pipes figured out that pipe 2 should
>> be restored, but since pipe 1 was disabled there was no mode nor fb
>> when trying to restore the first crtc.
>>
>> Hilarity ensued and broke resume on my i945gme machine since the
>> pipe_config_set_bpp added in
>
> I can see how the hack works, but I'm not clear on how we got into the
> situation of trying to enable multiple pipes in the first place. Do you
> mind expanding upon the failure conditions a bit more?

Yeah, that's worth spilling a few words ;-)

So the issue is that intel_set_mode essentially already does a global
modeset: intel_modeset_affected_pipes compares the current state with
where we want to go to (which is carefully set up by
intel_crtc_set_config) and then goes through the modeset sequence for
any crtc which needs updating.

Now the issue is that the actual interface with the remaining code
still only works on one crtc, and so we only pass in one fb and one
mode. In intel_set_mode we also only compute one intel_crtc_config
(which should be the one for the crtc we're doing a modeset on).

The reason for that mismatch is twofold:
- We want to eventually do all modeset as global state changes, so
it's just infrastructure prep.
- But even the old semantics can change more than one crtc when you
e.g. move a connector from crtc A to crtc B, then both crtc A and B
need to be updated. Usually that means one pipe is disabled and the
other enabled. This is also the reason why the hack doesn't touch the
disable_pipes mask.

Now hilarity ensued in our kms config restore paths when we actually
try to do a modeset on all crtcs: If the first crtc should be off and
the second should be on, then the call on the first crtc will notice
that the 2nd one should be switched on and so tries to compute the
pipe_config. But due to a lack of passed-in fb (crtc 1 should be off
after all) it only results in tears.

This case is ridiculously easy to hit on gen2/3 where the lvds output
is restricted to pipe B ;-) Also, before the pipe_config bpp rework
gen2/3 didn't care really about the fb->depth and so just stumbled a
bit, but never fell over completely. But apparently Ajax also managed
to blow up pch platforms, probably with some randomized configs, and
pch platforms trip up over the lack of an fb even in the old code.

Cheers, Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch