Re: [PATCH v2 2/3] drm/i915: Move the .global_resources() hook call into modeset_update_crtc_power_domains()

Ville Syrjälä
2014-11-10T17:24:42Z

On Mon, Nov 10, 2014 at 09:14:11AM -0800, Jesse Barnes wrote:
> On Thu, 6 Nov 2014 14:10:49 +0100
> Daniel Vetter <daniel@ffwll.ch> wrote:
>
> > On Thu, Nov 06, 2014 at 02:49:12PM +0200,
> > ville.syrjala@linux.intel.com wrote:
> > > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > >
> > > We may need to access various hardware bits in
> > > the .global_resources() hook, so move the call to occur after
> > > enabling all the newly required power wells, but before disabling
> > > all the now unneeded wells. This should guarantee that we have all
> > > the sufficient hardware resources available during
> > > the .global_resources() call. And if not, any additional resources
> > > must be explicitly acquired by the .global_resorces() hook.
> > >
> > > For instance on VLV/CHV we need to access the gunit mailbox so that
> > > we can talk to punit/cck over sideband. In addition some PFI credit
> > > reprogramming may need to be addes as well, which may require the
> > > disp2d well.
> > >
> > > This should also make the power domain refcounts consistent on
> > > platforms which don't have a .global_resource() hook since now they
> > > too will call modeset_update_crtc_power_domains() which will drop
> > > the init power. Previously init power was just left enabled for
> > > such platforms.
> > >
> > > Cc: Imre Deak <imre.deak@intel.com>
> > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >
> > Yeah I think that's a lot saner and hopefully allows us to unify the
> > power domain more. Thanks for the patch, queued for next.
>
> As a cleanup later it might be good to pull it out into a separate
> function. The global resources can be related to power domains, but
> not everything we do there is (e.g. re-clocking cdclk), so it may be a
> little confusing for readers in the future.

Well, we can't pull it out. That's the whole point with the patch. Or
maybe I misunderstood what you want to pull and where?

But we can certainly rename modeset_update_crtc_power_domains() itself.
But I suck at names anyway so didn't even bother to try :P Now that I
actually think about it, I guess I could have just called it
intel_global_resources() or something.

--
Ville Syrjälä
Intel OTC