[Intel-gfx] [PATCH v1 2/2] drm/i915: Deferring uncore early sanitize, sanitize, disable pc8 to resume

Sagar Arun Kamble sagar.a.kamble at intel.com
Mon Dec 1 02:29:49 PST 2014


Thanks Daniel.
This particular commit is moving power_domain_init into resume early.
Does not have details about ordering with uncore early sanitize and
uncore sanitize.

Imre,
Can you please clarify why this ordering with power domain init was
done?

Thanks,
Sagar

On Mon, 2014-12-01 at 10:16 +0100, Daniel Vetter wrote:
> On Mon, Dec 01, 2014 at 12:28:05PM +0530, sagar.a.kamble at intel.com wrote:
> > From: Sagar Kamble <sagar.a.kamble at intel.com>
> > 
> > Due to disabling of RC6 in uncore_sanitize in early resume, power is drained
> > till it RC6 is re-enabled post resume.
> > With this change RC6 disabling will be done at beginning of resume only.
> > This helps yield additional power benefits.
> > 
> > Signed-off-by: Akash Goel <akash.goel at intel.com>
> > Signed-off-by: Sagar Kamble <sagar.a.kamble at intel.com>
> 
> A bit of git blame turns up
> 
> commit 76c4b250080fff6e4befaa3619942422fd0ea380
> Author: Imre Deak <imre.deak at intel.com>
> Date:   Tue Apr 1 19:55:22 2014 +0300
> 
>     drm/i915: move power domain init earlier during system resume
>     
>     During resume the intel hda audio driver depends on the i915 driver
>     reinitializing the audio power domain. Since the order of calling the
>     i915 resume handler wrt. that of the audio driver is not guaranteed,
>     move the power domain reinitialization step to the resume_early
>     handler. This is guaranteed to run before the resume handler of any
>     other driver.
>     
>     The power domain initialization in turn requires us to enable the i915
>     pci device first, so move that part earlier too.
>     
>     Accordingly disabling of the i915 pci device should happen after the
>     audio suspend handler ran. So move the disabling later from the i915
>     resume handler to the resume_late handler.
>     
>     v2:
>     - move intel_uncore_sanitize/early_sanitize earlier too, so they don't
>       get reordered wrt. intel_power_domains_init_hw()
>     
>     Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=76152
>     Signed-off-by: Imre Deak <imre.deak at intel.com>
>     Reviewed-by: Takashi Iwai <tiwai at suse.de>
>     Cc: stable at vger.kernel.org
>     [danvet: Add cc: stable and loud comments that this is just a hack.]
>     [danvet: Fix "Should it be static?" sparse warning reported by Wu
>     Fengguang's kbuilder.]
>     Signed-off-by: Daniel Vetter <daniel.vetter at ffwll.ch>
> 
> 
> How does this patch not break stuff?
> 
> And a general rule of thumb: If you change anything in the driver load
> sequence please digg around in the git historly (with git log --pickaxe
> and git blame) to gather evidence for your changes and make sure you don't
> break anything.
> 
> Thanks, Daniel
> 
> > ---
> >  drivers/gpu/drm/i915/i915_drv.c | 13 +++++++------
> >  1 file changed, 7 insertions(+), 6 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> > index 71be3c9..0e08ec0 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.c
> > +++ b/drivers/gpu/drm/i915/i915_drv.c
> > @@ -675,6 +675,13 @@ static int i915_drm_resume(struct drm_device *dev)
> >  {
> >  	struct drm_i915_private *dev_priv = dev->dev_private;
> >  
> > +	intel_uncore_early_sanitize(dev, true);
> > +
> > +	if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv))
> > +		hsw_disable_pc8(dev_priv);
> > +
> > +	intel_uncore_sanitize(dev);
> > +
> >  	if (drm_core_check_feature(dev, DRIVER_MODESET)) {
> >  		mutex_lock(&dev->struct_mutex);
> >  		i915_gem_restore_gtt_mappings(dev);
> > @@ -761,12 +768,6 @@ static int i915_drm_resume_early(struct drm_device *dev)
> >  	if (ret)
> >  		DRM_ERROR("Resume prepare failed: %d,Continuing resume\n", ret);
> >  
> > -	intel_uncore_early_sanitize(dev, true);
> > -
> > -	if (IS_HASWELL(dev_priv) || IS_BROADWELL(dev_priv))
> > -		hsw_disable_pc8(dev_priv);
> > -
> > -	intel_uncore_sanitize(dev);
> >  	intel_power_domains_init_hw(dev_priv);
> >  
> >  	return ret;
> > -- 
> > 1.8.5
> > 
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx at lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 





More information about the Intel-gfx mailing list