From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ville =?iso-8859-1?Q?Syrj=E4l=E4?= Subject: Re: [PATCH v3 19/25] drm/i915: reinit GT power save during resume Date: Wed, 23 Apr 2014 11:06:45 +0300 Message-ID: <20140423080645.GB18465@intel.com> References: <1397496286-29649-20-git-send-email-imre.deak@intel.com> <1398187267-2758-1-git-send-email-imre.deak@intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable Return-path: Received: from mga11.intel.com (mga11.intel.com [192.55.52.93]) by gabe.freedesktop.org (Postfix) with ESMTP id 5B8EB6EA12 for ; Wed, 23 Apr 2014 01:09:27 -0700 (PDT) Content-Disposition: inline In-Reply-To: <1398187267-2758-1-git-send-email-imre.deak@intel.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" To: Imre Deak Cc: intel-gfx@lists.freedesktop.org List-Id: intel-gfx@lists.freedesktop.org On Tue, Apr 22, 2014 at 08:21:07PM +0300, Imre Deak wrote: > During runtime suspend there can be a last pending rps.work, so make > sure it's canceled. Note that in the runtime suspend callback we can't > get any RPS interrupts since it's called only after the GPU goes idle > and we set the minimum RPS frequency. The next possibility for an RPS > interrupt is only after getting an RPM ref (for example because of a new > GPU command) and calling the RPM resume callback. > = > v2: > - patch introduced in v2 of the patchset > v3: > - Change the order of canceling the rps.work and disabling interrupts to > avoid the race between interrupt disabling and the the rps.work. Race > spotted by Ville. > = > Signed-off-by: Imre Deak > --- > drivers/gpu/drm/i915/i915_drv.c | 7 +++++++ > 1 file changed, 7 insertions(+) > = > diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_= drv.c > index b87109c..edd4ab8 100644 > --- a/drivers/gpu/drm/i915/i915_drv.c > +++ b/drivers/gpu/drm/i915/i915_drv.c > @@ -919,6 +919,12 @@ static int intel_runtime_suspend(struct device *devi= ce) > = > DRM_DEBUG_KMS("Suspending device\n"); > = > + /* > + * rps.work can't be rearmed here, since we get here only after making > + * sure the GPU is idle and the RPS freq is set to the minimum. See > + * intel_mark_idle(). > + */ > + cancel_work_sync(&dev_priv->rps.work); Yeah makes sense. Well, unless the hardware is bonkers and generates an UP interrupt while idle. Reviewed-by: Ville Syrj=E4l=E4 > intel_runtime_pm_disable_interrupts(dev); > = > if (IS_GEN6(dev)) > @@ -970,6 +976,7 @@ static int intel_runtime_resume(struct device *device) > gen6_update_ring_freq(dev); > = > intel_runtime_pm_restore_interrupts(dev); > + intel_reset_gt_powersave(dev); > = > DRM_DEBUG_KMS("Device resumed\n"); > return 0; > -- = > 1.8.4 > = > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- = Ville Syrj=E4l=E4 Intel OTC