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

[Jesse Barnes <jbarnes@virtuousgeek.org>]

On Mon, 10 Nov 2014 19:24:37 +0200
Ville Syrjälä <ville.syrjala@linux.intel.com> wrote:

> 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.

Yeah that would do what I'd like, something that wraps both the power
domain stuff and the global resources call into something that's named
more accurately.

Thanks,
Jesse
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx