diff mbox

drm/i915: Use GEM suspend when aborting initialisation

Message ID 20180606145441.4460-1-chris@chris-wilson.co.uk (mailing list archive)
State New, archived
Headers show

Commit Message

Chris Wilson June 6, 2018, 2:54 p.m. UTC
As part of our GEM initialisation now, we send a request to the hardware
in order to record the initial GPU state. This coupled with deferred
idle workers, makes aborting on error tricky. We already have the
mechanism in place to wait on the GPU and cancel all the deferred
workers for suspend, so let's reuse it during the error teardown. It is
already used in places for later init error handling, but doing so at
this point is slightly ugly due to the mutex dance (it's ok, the module
load is still single threaded).

Testcase: igt/drv_module_reload/basic-reload-inject
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Michał Winiarski <michal.winiarski@intel.com>
---
 drivers/gpu/drm/i915/i915_gem.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

Comments

Michał Winiarski June 7, 2018, 7:39 a.m. UTC | #1
On Wed, Jun 06, 2018 at 03:54:41PM +0100, Chris Wilson wrote:
> As part of our GEM initialisation now, we send a request to the hardware
> in order to record the initial GPU state. This coupled with deferred
> idle workers, makes aborting on error tricky. We already have the
> mechanism in place to wait on the GPU and cancel all the deferred
> workers for suspend, so let's reuse it during the error teardown. It is
> already used in places for later init error handling, but doing so at
> this point is slightly ugly due to the mutex dance (it's ok, the module
> load is still single threaded).
> 
> Testcase: igt/drv_module_reload/basic-reload-inject
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Michał Winiarski <michal.winiarski@intel.com>

Reviewed-by: Michał Winiarski <michal.winiarski@intel.com>

-Michał

> ---
>  drivers/gpu/drm/i915/i915_gem.c | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index 86f1f9aaa119..1074f47e6cda 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -5514,8 +5514,12 @@ int i915_gem_init(struct drm_i915_private *dev_priv)
>  	 * driver doesn't explode during runtime.
>  	 */
>  err_init_hw:
> -	i915_gem_wait_for_idle(dev_priv, I915_WAIT_LOCKED);
> -	i915_gem_contexts_lost(dev_priv);
> +	mutex_unlock(&dev_priv->drm.struct_mutex);
> +
> +	WARN_ON(i915_gem_suspend(dev_priv));
> +	i915_gem_suspend_late(dev_priv);
> +
> +	mutex_lock(&dev_priv->drm.struct_mutex);
>  	intel_uc_fini_hw(dev_priv);
>  err_uc_init:
>  	intel_uc_fini(dev_priv);
> -- 
> 2.17.1
>
Chris Wilson June 7, 2018, 7:43 a.m. UTC | #2
Quoting Michał Winiarski (2018-06-07 08:39:39)
> On Wed, Jun 06, 2018 at 03:54:41PM +0100, Chris Wilson wrote:
> > As part of our GEM initialisation now, we send a request to the hardware
> > in order to record the initial GPU state. This coupled with deferred
> > idle workers, makes aborting on error tricky. We already have the
> > mechanism in place to wait on the GPU and cancel all the deferred
> > workers for suspend, so let's reuse it during the error teardown. It is
> > already used in places for later init error handling, but doing so at
> > this point is slightly ugly due to the mutex dance (it's ok, the module
> > load is still single threaded).
> > 
> > Testcase: igt/drv_module_reload/basic-reload-inject
> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > Cc: Michał Winiarski <michal.winiarski@intel.com>
> 
> Reviewed-by: Michał Winiarski <michal.winiarski@intel.com>

And pushed. Now let's see how close we are to making CI great again!

Quick! Where's my coffee?
-Chris
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index 86f1f9aaa119..1074f47e6cda 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -5514,8 +5514,12 @@  int i915_gem_init(struct drm_i915_private *dev_priv)
 	 * driver doesn't explode during runtime.
 	 */
 err_init_hw:
-	i915_gem_wait_for_idle(dev_priv, I915_WAIT_LOCKED);
-	i915_gem_contexts_lost(dev_priv);
+	mutex_unlock(&dev_priv->drm.struct_mutex);
+
+	WARN_ON(i915_gem_suspend(dev_priv));
+	i915_gem_suspend_late(dev_priv);
+
+	mutex_lock(&dev_priv->drm.struct_mutex);
 	intel_uc_fini_hw(dev_priv);
 err_uc_init:
 	intel_uc_fini(dev_priv);