From mboxrd@z Thu Jan 1 00:00:00 1970 From: Sagar Arun Kamble Subject: Re: [PATCH 01/16] drm/i915: vlv: fix gunit HW state corruption during S4 suspend Date: Mon, 29 Sep 2014 21:00:37 +0530 Message-ID: <1412004637.26473.42.camel@sagar-desktop> References: <1410362229-814-1-git-send-email-imre.deak@intel.com> <1410362229-814-2-git-send-email-imre.deak@intel.com> <1410801961.26473.7.camel@sagar-desktop> <1410802976.3761.6.camel@ideak-mobl> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mga02.intel.com (mga02.intel.com [134.134.136.20]) by gabe.freedesktop.org (Postfix) with ESMTP id B043F724AF for ; Mon, 29 Sep 2014 08:31:04 -0700 (PDT) In-Reply-To: <1410802976.3761.6.camel@ideak-mobl> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" To: imre.deak@intel.com Cc: intel-gfx@lists.freedesktop.org List-Id: intel-gfx@lists.freedesktop.org Thanks Imre. Reviewed-by: Sagar Kamble On Mon, 2014-09-15 at 20:42 +0300, Imre Deak wrote: > The point of doing these in thaw_early is to work around an ordering > issue wrt. to the hda driver, see the comment in i915_resume_early(). I > don't see a problem of having them in thaw_early; if you meant that they > lack the corresponding cleanup calls in freeze_late, it's just because > they don't have any. > > On Mon, 2014-09-15 at 22:56 +0530, Sagar Arun Kamble wrote: > > From DPM documentation, thaw_early should undo actions by freeze_late. > > Can we move following snippet from thaw_early to thaw to comply with > > this? > > > > intel_uncore_early_sanitize(dev, true); > > intel_uncore_sanitize(dev); > > intel_power_domains_init_hw(dev_priv); > > > > On Wed, 2014-09-10 at 18:16 +0300, Imre Deak wrote: > > > During S4 freeze we don't call intel_suspend_complete(), which would > > > save the gunit HW state, but during S4 thaw/restore events we call > > > intel_resume_prepare() which restores it, thus ending up in a corrupted > > > HW state. > > > > > > Fix this by calling intel_suspend_complete() from the corresponding > > > freeze_late event handler. > > > > > > The issue was introduced in > > > commit 016970beb05da6285c2f3ed2bee1c676cb75972e > > > Author: Sagar Kamble > > > Date: Wed Aug 13 23:07:06 2014 +0530 > > > > > > CC: Sagar Kamble > > > Signed-off-by: Imre Deak > > > --- > > > drivers/gpu/drm/i915/i915_drv.c | 10 ++++++++++ > > > 1 file changed, 10 insertions(+) > > > > > > diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c > > > index b8bd008..2365875 100644 > > > --- a/drivers/gpu/drm/i915/i915_drv.c > > > +++ b/drivers/gpu/drm/i915/i915_drv.c > > > @@ -987,6 +987,15 @@ static int i915_pm_freeze(struct device *dev) > > > return i915_drm_freeze(drm_dev); > > > } > > > > > > +static int i915_pm_freeze_late(struct device *dev) > > > +{ > > > + struct pci_dev *pdev = to_pci_dev(dev); > > > + struct drm_device *drm_dev = pci_get_drvdata(pdev); > > > + struct drm_i915_private *dev_priv = drm_dev->dev_private; > > > + > > > + return intel_suspend_complete(dev_priv); > > > +} > > > + > > > static int i915_pm_thaw_early(struct device *dev) > > > { > > > struct pci_dev *pdev = to_pci_dev(dev); > > > @@ -1571,6 +1580,7 @@ static const struct dev_pm_ops i915_pm_ops = { > > > .resume_early = i915_pm_resume_early, > > > .resume = i915_pm_resume, > > > .freeze = i915_pm_freeze, > > > + .freeze_late = i915_pm_freeze_late, > > > .thaw_early = i915_pm_thaw_early, > > > .thaw = i915_pm_thaw, > > > .poweroff = i915_pm_poweroff, > > > > > >