Re: [PATCH 1/1] drm/i915: Adding Gfx Clock, Wake and Gunit save/restore logic in PM suspend/resume paths.

[Daniel Vetter <daniel@ffwll.ch>]

On Fri, Aug 08, 2014 at 12:22:44PM +0530, Sagar Arun Kamble wrote:
> Hi Daniel,
> On Mon, 2014-08-04 at 10:07 +0200, Daniel Vetter wrote:
> > On Fri, Aug 01, 2014 at 12:34:56PM +0530, sagar.a.kamble@intel.com wrote:
> > > From: Sagar Kamble <sagar.a.kamble@intel.com>
> > > @@ -562,7 +567,12 @@ static int i915_drm_freeze(struct drm_device *dev)
> > >  
> > >  	intel_display_set_init_power(dev_priv, false);
> > >  
> > > -	return 0;
> > > +	/* Save Gunit State and clear wake - Need to make sure
> > > +	 * changes in vlv_runtime_suspend path don't impact this path */
> > > +	if (IS_VALLEYVIEW(dev))
> > > +		ret = vlv_runtime_suspend(dev_priv);
> > 
> > Maybe I wasn't clear, but I absolutely don't want any IS_VLV additions to
> > core resume/thaw code. This should be shovelled into the runtime pm
> > handling code, which should be reused in the suspend/resume code.
> This piece of code does not fit into any of the power well get/put path.
> Its specific sequence that need to be followed in VLV when Gunit gets
> power gated. So we have to keep this IS_VLV related functionality in
> both runtime and pm suspend/resume.

Well we support S0ix now. Which means our system suspend/resume code
actually calls into the runtime pm code. So either that design is broken
(and we need to fix this) or something else is amiss. Or we don't need
this code any more.

But duplicating it is not the right approach. And yeah the power domain
stuff might not be the right place, I've just used that as a place-holder
for all the runtime pm code we have.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch