On Sat, Apr 14, 2012 at 05:43:49PM +0100, Chris Wilson wrote: > On Sat, 14 Apr 2012 18:17:57 +0200, Daniel Vetter <daniel.vetter at ffwll.ch> wrote: > > This regression has been introduced in > > > > commit ca9bfa7eed20ea34e862804e62aae10eb159edbb > > Author: Daniel Vetter <daniel.vetter at ffwll.ch> > > Date: Sat Jan 28 14:49:20 2012 +0100 > > > > drm/i915: fixup interlaced vertical timings confusion, part 1 > > > > Unfortunately that commit failed to take into account that the lvds > > code does some special adjustements to the crtc timings for upscaling > > an centering. > > > > Fix this by explicitly computing crtc timings in the lvds mode fixup > > function and setting a special flag in mode->private_flags if the crtc > > timings have been adjusted. > > Whilst the patch does what it says on the tin, I think this just > underlines the fact that our handling of crtcinfo is just wrong and > consists of fragile hacks. :( Actually I think it's much better now, we compute the crtc info only at select places now - encoder->mode_fixup (lvds only atm), this is required to set the new CRTC_TIMING_SET flag. In my grepping for the original cleanup patch I've failed to notice this, hence the regression. - crtc->mode_fixup, but only when timings are not yet set (with this patch). - some fixed modes we use for load detect and the fixed TV modes, I think these won't go through the above mode_fixup stuff, so I've left them out. To contrast with the state before these two patches: - No longer splattered all over (some of it was deep down in the encoder code, e.g. sdvo). - No more stupid confusion about interlaced timings. So imho things are better, but suggestions for further improvements always highly welcome. Cheers, Daniel -- Daniel Vetter Mail: daniel at ffwll.ch Mobile: +41 (0)79 365 57 48
