[Intel-gfx] [RFC] Runtime display PM for VLV/BYT

[Jesse Barnes]

On Tue, 15 Oct 2013 15:16:11 +0300
Imre Deak <imre.deak at intel.com> wrote:

> On Tue, 2013-10-15 at 11:06 +0300, Ville Syrjälä wrote:
> > On Mon, Oct 14, 2013 at 04:07:44PM -0700, Jesse Barnes wrote:
> > > This set adds bits needed for runtime power support, currently only
> > > lightly tested on VLV/BYT:
> > >   1) suspend/resume callbacks for different platforms
> > >   2) save/restore of display state across a power well toggle
> > >   3) get/put of display power well in critical places
> > > 
> > > The TODO list still has a few items on it, and I'm looking for feedback:
> > >   1) sprinkle around some power well WARNs so we can catch things easily
> > >   2) add some tests using DPMS and NULL mode sets and comparing power
> > >      well state
> > >   3) better debugfs support for multiple wells
> > >   4) refcount of power well in debugfs (with ref holders?)
> > >   5) more testing - I think the load time ref is still busted here and
> > >      on HSW
> > >   6) convert HSW as well so DPMS will shut things down, not just mode
> > >      sets
> > > 
> > > Thoughts or comments?
> > 
> > I'd also like to see what Imre cooked up, and then come up with some
> > grand unified design. Based on our discussions I think his power well
> > abstraction sounded somewhat nicer and more general.
> 
> I've pushed what I have so far to:
> https://github.com/ideak/linux/commits/powerwells
> 
> I've tested this on VLV with VGA output so far and somewhat on HSW. I'd
> still have to check the need to do any HW state save/restore and the GFX
> clock forcing, afaics Jesse has already code for these in his patchset.

I think a few can be pushed upstream now:

drm/i915: change power_well->lock to be mutex
drm/i915: factor out is_always_on_domain 
drm/i915: factor out reset_vblank_counter
drm/i915: fix HAS_POWER_WELL->IS_HASWELL - pushed this but it doesn't
  look like Daniel has picked it up yet
drm/i915: check powerwell in get_pipe_config

drm/i915: enable only the needed power domains during modeset
Looks good too, and I guess there's no harm in pushing the earlier
patch to move the get of the power wells out of the HSW global res
function, even if we do move to get/put later.

For drm/i915: support for multiple power wells, I think I prefer a
single uncore function taking a power well, rather than an array of
power_well structs.  If we went that direction, we could probably
rename the power_well variable to audio_power_wells or something and
just track the ones we need to expose to the audio driver there, rather
than everything.  That might be a little clearer than what we have
today, but I guess I'd have to see it coded to be sure.

For drm/i915: add intel_encoder_get_hw_state, do you think it would be
clearer to take refs in the caller of the encoder->get_hw_state instead?

I like drm/i915: get port power domains for connector detect and 
drm/i915: add output power domains too, we'll need those if we want to
do fine grained output control on BYT and future chips.  But they'll
probably need to be rebased on top of the above once it goes upstream.

Overall I don't think there's a ton of conflict between our patchsets,
they seem mostly complimentary and let me remove hacks in mine.

Thanks,
-- 
Jesse Barnes, Intel Open Source Technology Center