Re: [PATCH 19/19] drm/i915: power domains: add vlv power wells

Daniel Vetter
2014-03-05T10:38:31Z

On Wed, Feb 26, 2014 at 11:52:25AM -0800, Jesse Barnes wrote:
> On Wed, 26 Feb 2014 20:02:19 +0200
> Imre Deak <imre.deak@intel.com> wrote:
>
> > On Thu, 2014-02-20 at 11:58 -0800, Jesse Barnes wrote:
> > > I'd like to see the code for re-enabling the display state land
> > > eventually too, so we can get savings when userspace uses DPMS instead
> > > of NULL mode sets for things. But to do that nicely requires some more
> > > work in the mode set code to pull out more PLL bits (also needed for
> > > atomic mode setting).
> >
> > I guess you meant here the drm connector->funcs->dpms() callback, that's
> > called when setting the connector's dpms property. But I'm not sure what
> > you meant by re-enabling the display state.
> >
> > I was thinking that we need to get/put the power domains around setting
> > DPMS off/on via the above property, but we actually don't need that.
> > Internally the dpms callback just calls the
> > display.disable_crtc/enable_crtc() which will disable/enable the pipes
> > but won't do anything with the power domains (which is only updated
> > during a normal modeset through display.modeset_global_resources().
> >
> > This isn't optimal power-wise, but it's a separate issue. I think Daniel
> > had the idea of converting the dpms callback to be a proper NULL
> > modeset. In that case too the power domains will be get/put correctly.
>
> Right, that's what I'm getting at. Today when someone uses the DPMS
> prop, we won't go through the mode set path and thus won't do any power
> well toggles. I think that's a bug we should fix. Either by making
> DPMS into a NULL mode set, or pulling out more bits from our mode set
> sequence into our crtc enable/disable paths.

Yeah, I think in the end we should be able to get rid of the ->mode_set
callbacks completely and move all the remaining hw frobbing into ->enable
and all the remaining state computation (plls mostly) into
->compute_config. That way ->enable will always restore the full hw state
and so won't get screwed over when a power well switched cleared all the
registers.

With that done we can add the display power well stuff from the modeset
path to the dpms path and update so that it follows crtc->active and not
crtc->enabled.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch