From mboxrd@z Thu Jan 1 00:00:00 1970 From: Imre Deak Subject: Re: [PATCH v2 08/25] drm/i915: get a runtime PM ref for the deferred GT powersave enabling Date: Fri, 25 Apr 2014 11:14:26 +0300 Message-ID: <1398413666.29729.6.camel@intelbox> References: <1397496286-29649-1-git-send-email-imre.deak@intel.com> <1397496286-29649-9-git-send-email-imre.deak@intel.com> <20140425075917.GH26374@phenom.ffwll.local> Reply-To: imre.deak@intel.com Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============1663857133==" Return-path: Received: from mga09.intel.com (mga09.intel.com [134.134.136.24]) by gabe.freedesktop.org (Postfix) with ESMTP id AC1436EDEB for ; Fri, 25 Apr 2014 01:14:29 -0700 (PDT) In-Reply-To: <20140425075917.GH26374@phenom.ffwll.local> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" To: Daniel Vetter Cc: intel-gfx@lists.freedesktop.org List-Id: intel-gfx@lists.freedesktop.org --===============1663857133== Content-Type: multipart/signed; micalg="pgp-sha1"; protocol="application/pgp-signature"; boundary="=-ERCGGFk0aIKNEWBzQZ/a" --=-ERCGGFk0aIKNEWBzQZ/a Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable On Fri, 2014-04-25 at 09:59 +0200, Daniel Vetter wrote: > On Mon, Apr 14, 2014 at 08:24:29PM +0300, Imre Deak wrote: > > At least on VLV but probably on other platforms too we depend on RC6 > > being enabled for RPM, so disable RPM until the delayed RC6 enabling > > completes. > >=20 > > v2: > > - explain the reason for the _noresume version of RPM get (Daniel) > > - use the simpler 'if (schedule_work()) rpm_get();' instead of > > 'if (!cancel_work_sync()) rpm_get(); schedule_work();' > >=20 > > Signed-off-by: Imre Deak > > --- > > drivers/gpu/drm/i915/i915_drv.c | 5 ++++- > > drivers/gpu/drm/i915/intel_drv.h | 2 ++ > > drivers/gpu/drm/i915/intel_pm.c | 34 ++++++++++++++++++++++++++++++++= -- > > 3 files changed, 38 insertions(+), 3 deletions(-) > >=20 > > diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i91= 5_drv.c > > index a821608..a20d2d1 100644 > > --- a/drivers/gpu/drm/i915/i915_drv.c > > +++ b/drivers/gpu/drm/i915/i915_drv.c > > @@ -754,7 +754,7 @@ int i915_reset(struct drm_device *dev) > > * previous concerns that it doesn't respond well to some forms > > * of re-init after reset. */ > > if (INTEL_INFO(dev)->gen > 5) > > - intel_enable_gt_powersave(dev); > > + intel_reset_gt_powersave(dev); > > =20 > > intel_hpd_init(dev); > > } else { > > @@ -923,6 +923,9 @@ static int intel_runtime_suspend(struct device *dev= ice) > > struct drm_device *dev =3D pci_get_drvdata(pdev); > > struct drm_i915_private *dev_priv =3D dev->dev_private; > > =20 > > + if (WARN_ON_ONCE(!dev_priv->rps.enabled)) > > + return -ENODEV; > > + > > WARN_ON(!HAS_RUNTIME_PM(dev)); > > assert_force_wake_inactive(dev_priv); > > =20 > > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/in= tel_drv.h > > index c551472..618d05a 100644 > > --- a/drivers/gpu/drm/i915/intel_drv.h > > +++ b/drivers/gpu/drm/i915/intel_drv.h > > @@ -924,6 +924,7 @@ void intel_init_gt_powersave(struct drm_device *dev= ); > > void intel_cleanup_gt_powersave(struct drm_device *dev); > > void intel_enable_gt_powersave(struct drm_device *dev); > > void intel_disable_gt_powersave(struct drm_device *dev); > > +void intel_reset_gt_powersave(struct drm_device *dev); > > void ironlake_teardown_rc6(struct drm_device *dev); > > void gen6_update_ring_freq(struct drm_device *dev); > > void gen6_rps_idle(struct drm_i915_private *dev_priv); > > @@ -931,6 +932,7 @@ void gen6_rps_boost(struct drm_i915_private *dev_pr= iv); > > void intel_aux_display_runtime_get(struct drm_i915_private *dev_priv); > > void intel_aux_display_runtime_put(struct drm_i915_private *dev_priv); > > void intel_runtime_pm_get(struct drm_i915_private *dev_priv); > > +void intel_runtime_pm_get_noresume(struct drm_i915_private *dev_priv); > > void intel_runtime_pm_put(struct drm_i915_private *dev_priv); > > void intel_init_runtime_pm(struct drm_i915_private *dev_priv); > > void intel_fini_runtime_pm(struct drm_i915_private *dev_priv); > > diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/int= el_pm.c > > index e5b9f08..0e8b263 100644 > > --- a/drivers/gpu/drm/i915/intel_pm.c > > +++ b/drivers/gpu/drm/i915/intel_pm.c > > @@ -4549,6 +4549,8 @@ static void intel_gen6_powersave_work(struct work= _struct *work) > > } > > dev_priv->rps.enabled =3D true; > > mutex_unlock(&dev_priv->rps.hw_lock); > > + > > + intel_runtime_pm_put(dev_priv); > > } > > =20 > > void intel_enable_gt_powersave(struct drm_device *dev) > > @@ -4566,12 +4568,28 @@ void intel_enable_gt_powersave(struct drm_devic= e *dev) > > * PCU communication is slow and this doesn't need to be > > * done at any specific time, so do this out of our fast path > > * to make resume and init faster. > > + * > > + * We depend on the HW RC6 power context save/restore > > + * mechanism when entering D3 through runtime PM suspend. So > > + * disable RPM until RPS/RC6 is properly setup. We can only > > + * get here via the driver load/system resume/runtime resume > > + * paths, so the _noresume version is enough (and in case of > > + * runtime resume it's necessary). > > */ > > - schedule_delayed_work(&dev_priv->rps.delayed_resume_work, > > - round_jiffies_up_relative(HZ)); > > + if (schedule_delayed_work(&dev_priv->rps.delayed_resume_work, > > + round_jiffies_up_relative(HZ))) > > + intel_runtime_pm_get_noresume(dev_priv); > > } > > } > > =20 > > +void intel_reset_gt_powersave(struct drm_device *dev) > > +{ > > + struct drm_i915_private *dev_priv =3D dev->dev_private; > > + > > + dev_priv->rps.enabled =3D false; > > + intel_enable_gt_powersave(dev); > > +} > > + > > static void ibx_init_clock_gating(struct drm_device *dev) > > { > > struct drm_i915_private *dev_priv =3D dev->dev_private; > > @@ -6025,6 +6043,18 @@ void intel_runtime_pm_get(struct drm_i915_privat= e *dev_priv) > > WARN(dev_priv->pm.suspended, "Device still suspended.\n"); > > } > > =20 > > +void intel_runtime_pm_get_noresume(struct drm_i915_private *dev_priv) > > +{ > > + struct drm_device *dev =3D dev_priv->dev; > > + struct device *device =3D &dev->pdev->dev; > > + > > + if (!HAS_RUNTIME_PM(dev)) > > + return; > > + > > + WARN(dev_priv->pm.suspended, "Getting nosync-ref while suspended.\n")= ; >=20 > Do we really need our own tracking in dev_priv->pm.suspended, is there > nothing in the runtime pm core which we could use instaed? Hm, yes by a quick look pm_runtime_suspended() seems like what we could use instead. But this was just copy&paste, and the rest of the spots would need changing too, so could we do it as a follow-up? --Imre > -Daniel >=20 > > + pm_runtime_get_noresume(device); > > +} > > + > > void intel_runtime_pm_put(struct drm_i915_private *dev_priv) > > { > > struct drm_device *dev =3D dev_priv->dev; > > --=20 > > 1.8.4 > >=20 > > _______________________________________________ > > Intel-gfx mailing list > > Intel-gfx@lists.freedesktop.org > > http://lists.freedesktop.org/mailman/listinfo/intel-gfx >=20 --=-ERCGGFk0aIKNEWBzQZ/a Content-Type: application/pgp-signature; name="signature.asc" Content-Description: This is a digitally signed message part Content-Transfer-Encoding: 7bit -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.14 (GNU/Linux) iQEcBAABAgAGBQJTWhliAAoJEORIIAnNuWDFDmYH/A6uy0oIYnyiaHwL04xP7nVM pzPqtuW9A17d47Zd+/kjpqR895bJI7UJQetWccpvdZt0o91jzVemqrLyyTf87rtU EUlcKl88sb5HxlrUIApAqJTt/Yp8UwY35i/VLxSsfiCcptZnzXajQ+XNifeVDOrF Vtwal1yR5pWt2XMC6Jpz5//a7hkNv7fCICCjYXM/FOBAT1GULhs7i6JDLepWONFE kq0dy9NuWmPizQHs1VVOLjZOAkZCQSBwIaQKHzCduSWsnUGaRy0wjFsBvBvsTEal XQtvTchpzTCtEvNc+3J26FhEA7b1hs6dXCWYVwcb3DolxaLaPL0FtWVSoyeoQ6k= =N4MC -----END PGP SIGNATURE----- --=-ERCGGFk0aIKNEWBzQZ/a-- --===============1663857133== Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Disposition: inline _______________________________________________ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx --===============1663857133==--