From mboxrd@z Thu Jan 1 00:00:00 1970 From: Daniel Vetter Subject: Re: [PATCH 12/18] drm/i915: try to reset the gpu before unload Date: Fri, 30 Mar 2012 19:30:53 +0200 Message-ID: <20120330173053.GC25907@phenom.ffwll.local> References: <1332103198-25852-1-git-send-email-ben@bwidawsk.net> <1332103198-25852-13-git-send-email-ben@bwidawsk.net> <20120330095431.1185a765@jbarnes-desktop> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mail-wi0-f177.google.com (mail-wi0-f177.google.com [209.85.212.177]) by gabe.freedesktop.org (Postfix) with ESMTP id 270D59ED98 for ; Fri, 30 Mar 2012 10:30:09 -0700 (PDT) Received: by wibhj13 with SMTP id hj13so621254wib.12 for ; Fri, 30 Mar 2012 10:30:09 -0700 (PDT) In-Reply-To: <20120330095431.1185a765@jbarnes-desktop> Content-Disposition: inline List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: intel-gfx-bounces+gcfxdi-intel-gfx=m.gmane.org@lists.freedesktop.org Errors-To: intel-gfx-bounces+gcfxdi-intel-gfx=m.gmane.org@lists.freedesktop.org To: Jesse Barnes Cc: Ben Widawsky , intel-gfx@lists.freedesktop.org List-Id: intel-gfx@lists.freedesktop.org On Fri, Mar 30, 2012 at 09:54:31AM -0700, Jesse Barnes wrote: > On Sun, 18 Mar 2012 13:39:52 -0700 > Ben Widawsky wrote: > > > paranoia > > > > For context support the HW expects the default context to always be > > available as there is no way to shut off HW contexts once turned on > > (afaics). This is problematic when unloading the driver as we have no > > way to prevent the GPU from expecting the BO to still be present once > > unloaded. > > > > The best we can do to remedy the situation is to attempt a GPU reset > > when doing the unload. > > > > NOTE: this patch isn't *really* required to go with the rest of the > > context serious. > > > > Signed-off-by: Ben Widawsky > > --- > > drivers/gpu/drm/i915/i915_gem.c | 3 +++ > > 1 file changed, 3 insertions(+) > > > > diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c > > index c1aab45..848cc45 100644 > > --- a/drivers/gpu/drm/i915/i915_gem.c > > +++ b/drivers/gpu/drm/i915/i915_gem.c > > @@ -3910,6 +3910,9 @@ i915_gem_lastclose(struct drm_device *dev) > > ret = i915_gem_idle(dev); > > if (ret) > > DRM_ERROR("failed to idle hardware: %d\n", ret); > > + ret = i915_reset(dev, GRDOM_FULL); > > + if (ret) > > + DRM_ERROR("failed to reset gpu: %d\n", ret); > > } > > > > static void > > This reminds me that we should make our reset finer grained. We > generally only need to reset the render engine when we detect an error, > but today we reset display and media unconditionally too. Not > resetting the display would make reset more invisible like it ought to > be... > > I haven't tested it, but it's yet another thing for our TODO list. I think with the pch split stuff we don't already reset display. And in any cases the global gtt seems to just keep going. The real problem is that you will notice the reset anyway, because we tend to wait a few seconds before we declare the gpu dead. For that we also need some improvements (and better culprit detection so that we can start to reset earlier). That one is on my todo ;-) -Daniel -- Daniel Vetter Mail: daniel@ffwll.ch Mobile: +41 (0)79 365 57 48