From mboxrd@z Thu Jan 1 00:00:00 1970 From: Imre Deak Subject: Re: [PATCH v2] drm/i915: remove user GTT mappings early during runtime suspend Date: Wed, 07 May 2014 20:11:26 +0300 Message-ID: <1399482686.11648.4.camel@intelbox> References: <1399375730-4355-1-git-send-email-imre.deak@intel.com> <1399481869-15239-1-git-send-email-imre.deak@intel.com> Reply-To: imre.deak@intel.com Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="===============1056705560==" Return-path: Received: from mga01.intel.com (mga01.intel.com [192.55.52.88]) by gabe.freedesktop.org (Postfix) with ESMTP id B71626ED68 for ; Wed, 7 May 2014 10:12:08 -0700 (PDT) In-Reply-To: <1399481869-15239-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: intel-gfx@lists.freedesktop.org List-Id: intel-gfx@lists.freedesktop.org --===============1056705560== Content-Type: multipart/signed; micalg="pgp-sha1"; protocol="application/pgp-signature"; boundary="=-cmQ1sDEephY1bN7S+Kxm" --=-cmQ1sDEephY1bN7S+Kxm Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable On Wed, 2014-05-07 at 19:57 +0300, Imre Deak wrote: > Currently user space can access GEM buffers mapped to GTT through > existing mappings concurrently while the platform specific suspend > handlers are running. Since these handlers may change the HW state in a > way that would break such accesses, remove the mappings before calling > the handlers. Spotted by Ville. >=20 > Also Chris pointed out that the lists that i915_gem_release_all_mmaps() > walks through need dev->struct_mutex, so take this lock. There is a > potential deadlock against a concurrent RPM resume, resolve this by > aborting and rescheduling the suspend (Daniel). >=20 > v2: > - take struct_mutex around i915_gem_release_all_mmaps() (Chris, Daniel) >=20 > Signed-off-by: Imre Deak > --- > drivers/gpu/drm/i915/i915_drv.c | 27 +++++++++++++++++++++++++-- > 1 file changed, 25 insertions(+), 2 deletions(-) >=20 > diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_= drv.c > index 4024e16..0c9858c 100644 > --- a/drivers/gpu/drm/i915/i915_drv.c > +++ b/drivers/gpu/drm/i915/i915_drv.c > @@ -36,6 +36,7 @@ > =20 > #include > #include > +#include > #include > =20 > static struct drm_driver driver; > @@ -1315,6 +1316,30 @@ static int intel_runtime_suspend(struct device *de= vice) > DRM_DEBUG_KMS("Suspending device\n"); > =20 > /* > + * We could deadlock here in case another thread holding struct_mutex > + * calls RPM suspend concurrently, since the RPM suspend will wait resume^ resume^ > + * first for this RPM suspend to finish. In this case the concurrent > + * RPM resume will be followed by its RPM suspend counterpart. Still > + * for consistency return -EAGAIN, which will reschedule this suspend. > + */ > + if (!mutex_trylock(&dev->struct_mutex)) { > + DRM_DEBUG_KMS("device lock contention, deffering suspend\n"); > + /* > + * Bump the expiration timestamp, otherwise the suspend won't > + * be rescheduled. > + */ > + pm_runtime_mark_last_busy(device); > + > + return -EAGAIN; > + } > + /* > + * We are safe here against re-faults, since the fault handler takes > + * an RPM reference. > + */ > + i915_gem_release_all_mmaps(dev_priv); > + mutex_unlock(&dev->struct_mutex); > + > + /* > * 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(). > @@ -1340,8 +1365,6 @@ static int intel_runtime_suspend(struct device *dev= ice) > return ret; > } > =20 > - i915_gem_release_all_mmaps(dev_priv); > - > del_timer_sync(&dev_priv->gpu_error.hangcheck_timer); > dev_priv->pm.suspended =3D true; > =20 --=-cmQ1sDEephY1bN7S+Kxm 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) iQEcBAABAgAGBQJTamk+AAoJEORIIAnNuWDFGnEH/jJOdAD4F3S3ehedj6qiiEkw KGJlqFcdXeYUFJ0tWG9+GcPfOnptrDDtMvnBlotxQebl41sr6me5qaeHQgdR+Nvy 59JTC4sMThOlqc01mTjxrLqutZqT4ERcwNL5PufqxcwaMH+LAIT2e8p46X5Oi2iG rEx1m4wyBm6Nd84HJ1e5tLY4tAEs5EiD+W6+ViBaB8q/c8V8E9fQ7va1wNDTlRZF GLlNwzO9Od5KLHburdAT54MOcWVUBZWDU4xpaDjwOppU43T6soyus9z62hHFnsMs Vee4cB+57+FAOwZuAFrzae0oSej+gm7oD8XwsGqRRTpYUlJdUEdQCMxDBXmcg0Y= =QioH -----END PGP SIGNATURE----- --=-cmQ1sDEephY1bN7S+Kxm-- --===============1056705560== 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 --===============1056705560==--