On Fri, May 3, 2013 at 6:50 PM, Daniel Vetter <daniel at ffwll.ch> wrote: > On Fri, May 3, 2013 at 6:28 PM, Chris Wilson <chris at chris-wilson.co.uk> wrote: >> Not quite. It was added afterwards, I know for it is one of my mistakes >> that I belatedly recognised as trying to workaround a bug in UXA. And we >> then layered on further bugs to try and patch up the glaring holes then >> introduced. > > I've done some digging around and found some cool stuff, but no change > which introduce the original bug that we've never consulted dpms state > in drm_crtc_helper_set_mode but just blindly enabled encoders. Most of > the commits I've dug out try to paper over that mismatch by updating > dpms state, too. E.g. bf9dc102 which was then fixed up a bit in > 811aaa55 but I don't see any mention of someone explicitly kill dpms > checks. > > Of course for DP support we've started to add dpms state checks to our > dp encoder (since double-enable kills DP). Then duplicated the > drm_connector->dpms into our own intel_dp->dpms state since the crtc > helpers liked to frob around with dpms in a rather ill-defined way. > But that's just hilarity and flailing around on top, nothing that I > could see fundamentally changed that a full setcrtc implicitly enabled > the encoders/crtc hw. And since we have our own modeset code that all > disappeared (since a full modeset simply force-updates the dpms state > to reflect reality). > > But nowhere have a found a commit to support your claim, just lots of > flailing around due to the ill-defined semantics of this all. Care to > do some more digging? Ping for history digging ... or an ack on the patch ;-) Alternative ideas are welcome too ofc, but under the assumption that setcrtc always kinda enabled encoders/connectors implicitly (neglecting a few funny corner cases every screwed up in different ways) this hack here seems the most sensible&consistent option we have. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch
