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