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

From: "Ville Syrjälä" <ville.syrjala@linux.intel.com>
To: Jesse Barnes <jbarnes@virtuousgeek.org>
Cc: intel-gfx@lists.freedesktop.org
Subject: Re: [PATCH v2 2/3] drm/i915: Move the .global_resources() hook call into modeset_update_crtc_power_domains()
Date: Mon, 10 Nov 2014 19:24:37 +0200	[thread overview]
Message-ID: <20141110172437.GC10649@intel.com> (raw)
In-Reply-To: <20141110091411.75485b68@jbarnes-t420>

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
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx