From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ben Widawsky Subject: Re: [PATCH] drm/i915: fix module unload after context merge Date: Tue, 19 Jun 2012 14:19:15 -0700 Message-ID: <20120619141915.57908827@bwidawsk.net> References: <20120619103247.64614557@bwidawsk.net> <1340135732-4402-1-git-send-email-daniel.vetter@ffwll.ch> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from cloud01.chad-versace.us (184-106-247-128.static.cloud-ips.com [184.106.247.128]) by gabe.freedesktop.org (Postfix) with ESMTP id 59DA5A08E3 for ; Tue, 19 Jun 2012 14:19:23 -0700 (PDT) In-Reply-To: <1340135732-4402-1-git-send-email-daniel.vetter@ffwll.ch> 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: Daniel Vetter Cc: Intel Graphics Development List-Id: intel-gfx@lists.freedesktop.org On Tue, 19 Jun 2012 21:55:32 +0200 Daniel Vetter wrote: > commit 8e96d9c4d9843f00ebeb4a9b33596d96602ea101 > Author: Ben Widawsky > Date: Mon Jun 4 14:42:56 2012 -0700 > > drm/i915: reset the GPU on context fini > > broke module unload because it reset the gpu before we've stopped > touching it. Later on in the unload sequence the ringbuffer code > complained that the gpu would idle properly (because intel_gpu_reset > only resets the hw and not our sw state). > > v2: Reorder things so that we reset the gpu _before_ we release the > backing storage of the default context. > > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=51183 > Signed-Off-by: Daniel Vetter Reviewed-by: Ben Widawsky > --- > drivers/gpu/drm/i915/i915_dma.c | 2 +- > drivers/gpu/drm/i915/i915_gem_context.c | 7 +++++-- > 2 files changed, 6 insertions(+), 3 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c > index f1544c5..efba4e8 100644 > --- a/drivers/gpu/drm/i915/i915_dma.c > +++ b/drivers/gpu/drm/i915/i915_dma.c > @@ -1669,7 +1669,6 @@ int i915_driver_unload(struct drm_device *dev) > if (ret) > DRM_ERROR("failed to idle hardware: %d\n", ret); > i915_gem_retire_requests(dev); > - i915_gem_context_fini(dev); > mutex_unlock(&dev->struct_mutex); > > /* Cancel the retire work handler, which should be idle now. */ > @@ -1720,6 +1719,7 @@ int i915_driver_unload(struct drm_device *dev) > mutex_lock(&dev->struct_mutex); > i915_gem_free_all_phys_object(dev); > i915_gem_cleanup_ringbuffer(dev); > + i915_gem_context_fini(dev); > mutex_unlock(&dev->struct_mutex); > i915_gem_cleanup_aliasing_ppgtt(dev); > i915_gem_cleanup_stolen(dev); > diff --git a/drivers/gpu/drm/i915/i915_gem_context.c b/drivers/gpu/drm/i915/i915_gem_context.c > index 8fb8cd8..48e41df 100644 > --- a/drivers/gpu/drm/i915/i915_gem_context.c > +++ b/drivers/gpu/drm/i915/i915_gem_context.c > @@ -277,11 +277,14 @@ void i915_gem_context_fini(struct drm_device *dev) > if (dev_priv->hw_contexts_disabled) > return; > > + /* The only known way to stop the gpu from accessing the hw context is > + * to reset it. Do this as the very last operation to avoid confusing > + * other code, leading to spurious errors. */ > + intel_gpu_reset(dev); > + > i915_gem_object_unpin(dev_priv->ring[RCS].default_context->obj); > > do_destroy(dev_priv->ring[RCS].default_context); > - > - intel_gpu_reset(dev); > } > > void i915_gem_context_open(struct drm_device *dev, struct drm_file *file) -- Ben Widawsky, Intel Open Source Technology Center