On Mon, 2014-12-01 at 15:59 +0530, Sagar Arun Kamble wrote: > 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? We call intel_uncore_early_sanitize() and intel_uncore_sanitize() before any other HW access. They are called from i915_drm_resume_early() since the hda driver's resume handler can run before i915_drm_resume() is called. The hda resume handler can in turn request the display power well resulting in an i915 HW access. > > 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@xxxxxxxxx wrote: > > > From: Sagar Kamble <sagar.a.kamble@xxxxxxxxx> > > > > > > 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@xxxxxxxxx> > > > Signed-off-by: Sagar Kamble <sagar.a.kamble@xxxxxxxxx> > > > > A bit of git blame turns up > > > > commit 76c4b250080fff6e4befaa3619942422fd0ea380 > > Author: Imre Deak <imre.deak@xxxxxxxxx> > > 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@xxxxxxxxx> > > Reviewed-by: Takashi Iwai <tiwai@xxxxxxx> > > Cc: stable@xxxxxxxxxxxxxxx > > [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@xxxxxxxx> > > > > > > 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@xxxxxxxxxxxxxxxxxxxxx > > > http://lists.freedesktop.org/mailman/listinfo/intel-gfx > > > > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@xxxxxxxxxxxxxxxxxxxxx > http://lists.freedesktop.org/mailman/listinfo/intel-gfx _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/intel-gfx
