All of lore.kernel.org
 help / color / mirror / Atom feed
From: Mika Kuoppala <mika.kuoppala@linux.intel.com>
To: Chris Wilson <chris@chris-wilson.co.uk>, intel-gfx@lists.freedesktop.org
Subject: Re: [PATCH 4/8] drm/i915: Make all GPU resets atomic
Date: Thu, 17 Jan 2019 16:14:24 +0200	[thread overview]
Message-ID: <87tvi7wev3.fsf@gaia.fi.intel.com> (raw)
In-Reply-To: <20190114210408.4561-5-chris@chris-wilson.co.uk>

Chris Wilson <chris@chris-wilson.co.uk> writes:

> In preparation for the next few commits, make resetting the GPU atomic.
> Currently, we have prepared gen6+ for atomic resetting of individual
> engines, but now there is a requirement to perform the whole device
> level reset (just the register poking) from inside an atomic context.
>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> ---
>  drivers/gpu/drm/i915/i915_reset.c             | 50 ++++++++++---------
>  .../gpu/drm/i915/selftests/mock_gem_device.c  |  4 +-
>  2 files changed, 29 insertions(+), 25 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_reset.c b/drivers/gpu/drm/i915/i915_reset.c
> index e2e40b44a9a8..f9512e07646d 100644
> --- a/drivers/gpu/drm/i915/i915_reset.c
> +++ b/drivers/gpu/drm/i915/i915_reset.c
> @@ -144,14 +144,14 @@ static int i915_do_reset(struct drm_i915_private *i915,
>  
>  	/* Assert reset for at least 20 usec, and wait for acknowledgement. */
>  	pci_write_config_byte(pdev, I915_GDRST, GRDOM_RESET_ENABLE);
> -	usleep_range(50, 200);
> -	err = wait_for(i915_in_reset(pdev), 500);
> +	udelay(50);
> +	err = wait_for_atomic(i915_in_reset(pdev), 50);
>  
>  	/* Clear the reset request. */
>  	pci_write_config_byte(pdev, I915_GDRST, 0);
> -	usleep_range(50, 200);
> +	udelay(50);
>  	if (!err)
> -		err = wait_for(!i915_in_reset(pdev), 500);
> +		err = wait_for_atomic(!i915_in_reset(pdev), 50);

50ms still seems long but I guess you want to play it safe.

Wouldn't it be nice if we would get easily publish a completion
time value to some external database from CI.

>  
>  	return err;
>  }
> @@ -171,7 +171,7 @@ static int g33_do_reset(struct drm_i915_private *i915,
>  	struct pci_dev *pdev = i915->drm.pdev;
>  
>  	pci_write_config_byte(pdev, I915_GDRST, GRDOM_RESET_ENABLE);
> -	return wait_for(g4x_reset_complete(pdev), 500);
> +	return wait_for_atomic(g4x_reset_complete(pdev), 50);
>  }
>  
>  static int g4x_do_reset(struct drm_i915_private *dev_priv,
> @@ -182,13 +182,13 @@ static int g4x_do_reset(struct drm_i915_private *dev_priv,
>  	int ret;
>  
>  	/* WaVcpClkGateDisableForMediaReset:ctg,elk */
> -	I915_WRITE(VDECCLK_GATE_D,
> -		   I915_READ(VDECCLK_GATE_D) | VCP_UNIT_CLOCK_GATE_DISABLE);
> -	POSTING_READ(VDECCLK_GATE_D);
> +	I915_WRITE_FW(VDECCLK_GATE_D,
> +		      I915_READ(VDECCLK_GATE_D) | VCP_UNIT_CLOCK_GATE_DISABLE);
> +	POSTING_READ_FW(VDECCLK_GATE_D);
>  
>  	pci_write_config_byte(pdev, I915_GDRST,
>  			      GRDOM_MEDIA | GRDOM_RESET_ENABLE);
> -	ret =  wait_for(g4x_reset_complete(pdev), 500);
> +	ret =  wait_for_atomic(g4x_reset_complete(pdev), 50);
>  	if (ret) {
>  		DRM_DEBUG_DRIVER("Wait for media reset failed\n");
>  		goto out;
> @@ -196,7 +196,7 @@ static int g4x_do_reset(struct drm_i915_private *dev_priv,
>  
>  	pci_write_config_byte(pdev, I915_GDRST,
>  			      GRDOM_RENDER | GRDOM_RESET_ENABLE);
> -	ret =  wait_for(g4x_reset_complete(pdev), 500);
> +	ret =  wait_for_atomic(g4x_reset_complete(pdev), 50);
>  	if (ret) {
>  		DRM_DEBUG_DRIVER("Wait for render reset failed\n");
>  		goto out;
> @@ -205,9 +205,9 @@ static int g4x_do_reset(struct drm_i915_private *dev_priv,
>  out:
>  	pci_write_config_byte(pdev, I915_GDRST, 0);
>  
> -	I915_WRITE(VDECCLK_GATE_D,
> -		   I915_READ(VDECCLK_GATE_D) & ~VCP_UNIT_CLOCK_GATE_DISABLE);
> -	POSTING_READ(VDECCLK_GATE_D);
> +	I915_WRITE_FW(VDECCLK_GATE_D,
> +		      I915_READ(VDECCLK_GATE_D) & ~VCP_UNIT_CLOCK_GATE_DISABLE);
> +	POSTING_READ_FW(VDECCLK_GATE_D);
>  
>  	return ret;
>  }
> @@ -218,27 +218,29 @@ static int ironlake_do_reset(struct drm_i915_private *dev_priv,
>  {
>  	int ret;
>  
> -	I915_WRITE(ILK_GDSR, ILK_GRDOM_RENDER | ILK_GRDOM_RESET_ENABLE);
> -	ret = intel_wait_for_register(dev_priv,
> -				      ILK_GDSR, ILK_GRDOM_RESET_ENABLE, 0,
> -				      500);
> +	I915_WRITE_FW(ILK_GDSR, ILK_GRDOM_RENDER | ILK_GRDOM_RESET_ENABLE);
> +	ret = __intel_wait_for_register_fw(dev_priv, ILK_GDSR,
> +					   ILK_GRDOM_RESET_ENABLE, 0,
> +					   5000, 0,
> +					   NULL);

From 500ms to 5ms. There has been some slack in there for sure...

>  	if (ret) {
>  		DRM_DEBUG_DRIVER("Wait for render reset failed\n");
>  		goto out;
>  	}
>  
> -	I915_WRITE(ILK_GDSR, ILK_GRDOM_MEDIA | ILK_GRDOM_RESET_ENABLE);
> -	ret = intel_wait_for_register(dev_priv,
> -				      ILK_GDSR, ILK_GRDOM_RESET_ENABLE, 0,
> -				      500);
> +	I915_WRITE_FW(ILK_GDSR, ILK_GRDOM_MEDIA | ILK_GRDOM_RESET_ENABLE);
> +	ret = __intel_wait_for_register_fw(dev_priv, ILK_GDSR,
> +					   ILK_GRDOM_RESET_ENABLE, 0,
> +					   5000, 0,
> +					   NULL);
>  	if (ret) {
>  		DRM_DEBUG_DRIVER("Wait for media reset failed\n");
>  		goto out;
>  	}
>  
>  out:
> -	I915_WRITE(ILK_GDSR, 0);
> -	POSTING_READ(ILK_GDSR);
> +	I915_WRITE_FW(ILK_GDSR, 0);
> +	POSTING_READ_FW(ILK_GDSR);
>  	return ret;
>  }
>  
> @@ -572,7 +574,9 @@ int intel_gpu_reset(struct drm_i915_private *i915, unsigned int engine_mask)
>  		ret = -ENODEV;
>  		if (reset) {

Ok, i might have missed some spot but for me it looks
like you can lift the might_sleep_if(engine_mask == ALL_ENGINES)
from above.

>  			GEM_TRACE("engine_mask=%x\n", engine_mask);
> +			preempt_disable();
>  			ret = reset(i915, engine_mask, retry);
> +			preempt_enable();
>  		}
>  		if (ret != -ETIMEDOUT || engine_mask != ALL_ENGINES)
>  			break;
> diff --git a/drivers/gpu/drm/i915/selftests/mock_gem_device.c b/drivers/gpu/drm/i915/selftests/mock_gem_device.c
> index 3cda66292e76..888c6978bc54 100644
> --- a/drivers/gpu/drm/i915/selftests/mock_gem_device.c
> +++ b/drivers/gpu/drm/i915/selftests/mock_gem_device.c
> @@ -58,8 +58,8 @@ static void mock_device_release(struct drm_device *dev)
>  	i915_gem_contexts_lost(i915);
>  	mutex_unlock(&i915->drm.struct_mutex);
>  
> -	cancel_delayed_work_sync(&i915->gt.retire_work);
> -	cancel_delayed_work_sync(&i915->gt.idle_work);
> +	drain_delayed_work(&i915->gt.retire_work);
> +	drain_delayed_work(&i915->gt.idle_work);

Yeah, but why in this patch?
-Mika

>  	i915_gem_drain_workqueue(i915);
>  
>  	mutex_lock(&i915->drm.struct_mutex);
> -- 
> 2.20.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

  reply	other threads:[~2019-01-17 14:16 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-01-14 21:04 Mika's reward Chris Wilson
2019-01-14 21:04 ` [PATCH 1/8] drm/i915: Serialise concurrent calls to i915_gem_set_wedged() Chris Wilson
2019-01-15 11:56   ` Mika Kuoppala
2019-01-15 12:05     ` Chris Wilson
2019-01-16  9:27       ` Chris Wilson
2019-01-16 15:04         ` Mika Kuoppala
2019-01-14 21:04 ` [PATCH 2/8] drm/i915: Differentiate between ggtt->mutex and ppgtt->mutex Chris Wilson
2019-01-14 21:04 ` [PATCH 3/8] drm/i915: Pull all the reset functionality together into i915_reset.c Chris Wilson
2019-01-16 15:06   ` Mika Kuoppala
2019-01-16 15:31     ` Chris Wilson
2019-01-14 21:04 ` [PATCH 4/8] drm/i915: Make all GPU resets atomic Chris Wilson
2019-01-17 14:14   ` Mika Kuoppala [this message]
2019-01-14 21:04 ` [PATCH 5/8] drm/i915/guc: Disable global reset Chris Wilson
2019-01-17 14:24   ` Mika Kuoppala
2019-01-17 18:27     ` Daniele Ceraolo Spurio
2019-01-14 21:04 ` [PATCH 6/8] drm/i915: Remove GPU reset dependence on struct_mutex Chris Wilson
2019-01-14 21:04 ` [PATCH 7/8] drm/i915/selftests: Trim struct_mutex duration for set-wedged selftest Chris Wilson
2019-01-14 21:04 ` [PATCH 8/8] drm/i915: Issue engine resets onto idle engines Chris Wilson
2019-01-14 21:26 ` ✗ Fi.CI.BAT: failure for series starting with [1/8] drm/i915: Serialise concurrent calls to i915_gem_set_wedged() Patchwork
2019-01-14 21:59   ` Chris Wilson

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87tvi7wev3.fsf@gaia.fi.intel.com \
    --to=mika.kuoppala@linux.intel.com \
    --cc=chris@chris-wilson.co.uk \
    --cc=intel-gfx@lists.freedesktop.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body. 
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.