On Sun, Aug 12, 2012 at 09:12:14PM +0100, Chris Wilson wrote: > On Sun, 12 Aug 2012 22:01:02 +0200, Daniel Vetter <daniel at ffwll.ch> wrote: > > On Sun, Aug 12, 2012 at 08:47:56PM +0100, Chris Wilson wrote: > > > > drm/i915: simplify dvo dpms interface > > > > > > This just looks like churn for churn's sake? The changes look correct. > > > > We don't bother with anything else than dpms on/off states in most of the > > modeset code (even for crt newer hw drops the intermediate states). Hence > > the new interfaces have only enable/disable functions at the encoder/crtc > > level. I've figured it looks odd if we keep the full dpms interface for > > dvo. But since it's rather independant churn I've moved it into this > > odds bits series. > > The full fledged dpms mode isn't going to completely disappear thanks > to the CRT dinosaur. I just wonder if we can achieve the same > simplification by recognising that all non-zero dpms modes are off, e.g. > s/dpms_mode/powersave/ > > if (powersave) > switch_off() > else > switch_on() Yeah, I'm still handling the crt dinosaur (on hw that supports it) in the modeset rework code. I've just noticed while planning the new modeset code that we handle these dpms codes rather inconsistently. Hence the code has grown a simple bool at the encoder level to track the desired state, which then controls the on/off state of the entire output pipeline (the additional dpms states of the dinosaur are just a nuisance on top of that - every state that requires an active pipes force the entire pipe to be on). This has the advantage that we don't force the pipe on for dpms == standby/suspend just in case we have a crt on it - the new code actually shuts it off if possible. So with the new code the only thing still even dealing with dpms codes is at the connector level. And even there it's just crt (and sdvo, if anyone would fancy writing the patch to enable that ...). -Daniel -- Daniel Vetter Mail: daniel at ffwll.ch Mobile: +41 (0)79 365 57 48