From mboxrd@z Thu Jan 1 00:00:00 1970 From: Chris Wilson Subject: Re: [PATCH v3] drm/i915: Rework GPU reset sequence to match driver load & thaw Date: Wed, 20 Aug 2014 15:58:26 +0100 Message-ID: <20140820145826.GD12830@nuc-i3427.alporthouse.com> References: <8761ht7kiq.fsf@gaia.fi.intel.com> <1408125095-10699-1-git-send-email-alistair.mcaulay@intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from fireflyinternet.com (mail.fireflyinternet.com [87.106.93.118]) by gabe.freedesktop.org (Postfix) with ESMTP id 1722489216 for ; Wed, 20 Aug 2014 07:58:29 -0700 (PDT) Content-Disposition: inline In-Reply-To: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: intel-gfx-bounces@lists.freedesktop.org Sender: "Intel-gfx" To: "Daniel, Thomas" Cc: "intel-gfx@lists.freedesktop.org" List-Id: intel-gfx@lists.freedesktop.org On Wed, Aug 20, 2014 at 02:46:37PM +0000, Daniel, Thomas wrote: > > > > -----Original Message----- > > From: Intel-gfx [mailto:intel-gfx-bounces@lists.freedesktop.org] On Behalf > > Of alistair.mcaulay@intel.com > > Sent: Friday, August 15, 2014 6:52 PM > > To: intel-gfx@lists.freedesktop.org > > Subject: [Intel-gfx] [PATCH v3] drm/i915: Rework GPU reset sequence to > > match driver load & thaw > > > > From: "McAulay, Alistair" > > > > This patch is to address Daniels concerns over different code during reset: > > > > http://lists.freedesktop.org/archives/intel-gfx/2014-June/047758.html > > > > "The reason for aiming as hard as possible to use the exact same code for > > driver load, gpu reset and runtime pm/system resume is that we've simply > > seen too many bugs due to slight variations and unintended omissions." > > > > Tested using igt drv_hangman. > > > > V2: Cleaner way of preventing check_wedge returning -EAGAIN > > V3: Clean the last_context during reset, to ensure do_switch() does the > > MI_SET_CONTEXT. As per review. > > Signed-off-by: McAulay, Alistair > > --- > > drivers/gpu/drm/i915/i915_drv.c | 6 +++ > > drivers/gpu/drm/i915/i915_drv.h | 3 ++ > > drivers/gpu/drm/i915/i915_gem.c | 4 +- > > drivers/gpu/drm/i915/i915_gem_context.c | 33 +++------------- > > drivers/gpu/drm/i915/i915_gem_gtt.c | 67 +++++---------------------------- > > drivers/gpu/drm/i915/i915_gem_gtt.h | 3 +- > > 6 files changed, 28 insertions(+), 88 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/i915_drv.c > > b/drivers/gpu/drm/i915/i915_drv.c index 5e4fefd..3bfafe6 100644 > > --- a/drivers/gpu/drm/i915/i915_drv.c > > +++ b/drivers/gpu/drm/i915/i915_drv.c > > @@ -806,7 +806,13 @@ int i915_reset(struct drm_device *dev) > > !dev_priv->ums.mm_suspended) { > > dev_priv->ums.mm_suspended = 0; > > > > + /* Used to prevent gem_check_wedged returning -EAGAIN > > during gpu reset */ > > + dev_priv->gpu_error.reload_in_reset = true; > > + > > ret = i915_gem_init_hw(dev); > > + > > + dev_priv->gpu_error.reload_in_reset = false; > > + > > mutex_unlock(&dev->struct_mutex); > > if (ret) { > > DRM_ERROR("Failed hw init on reset %d\n", ret); diff > > --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h > > index 991b663..116daff 100644 > > --- a/drivers/gpu/drm/i915/i915_drv.h > > +++ b/drivers/gpu/drm/i915/i915_drv.h > > @@ -1217,6 +1217,9 @@ struct i915_gpu_error { > > > > /* For missed irq/seqno simulation. */ > > unsigned int test_irq_rings; > > + > > + /* Used to prevent gem_check_wedged returning -EAGAIN during > > gpu reset */ > > + bool reload_in_reset; > > }; > > > > enum modeset_restore { > > diff --git a/drivers/gpu/drm/i915/i915_gem.c > > b/drivers/gpu/drm/i915/i915_gem.c index ef047bc..e7396eb 100644 > > --- a/drivers/gpu/drm/i915/i915_gem.c > > +++ b/drivers/gpu/drm/i915/i915_gem.c > > @@ -1085,7 +1085,9 @@ i915_gem_check_wedge(struct i915_gpu_error > > *error, > > if (i915_terminally_wedged(error)) > > return -EIO; > > > > - return -EAGAIN; > > + /* Check if GPU Reset is in progress */ > > + if (!error->reload_in_reset) > > + return -EAGAIN; This is silly. You already have the same flag above. Look closer. -Chris -- Chris Wilson, Intel Open Source Technology Centre