[PATCH] drm/i915: Do not disable preemption for resets

[Sebastian Andrzej Siewior]

On 2023-07-04 09:36:12 [+0100], Tvrtko Ursulin wrote:
> From: Tvrtko Ursulin <tvrtko.ursulin at intel.com>
> 
> This is going to our trybot for some smoke testing so excuse the informal
> commit message.

I just added this to my i915 pile and there is no splat so far. I did
a bit of this in that in X and everything looked fine.

> Later I realized that engine resets are done from the normal and tasklet
> contexts so I can't really use the sleeping waits. Here I tried the non-
> atomic wait which runs with preemption enabled and dropped the top-level
> preempt_disable.
> 
> Lets see what happens in CI..
> 
> And not sure what to do about the udelays yet. It is not preemptible on
> RT?

The udelay() is preemptible. However, with the preempt_disable() in
front of it (which you removed) it is not.
So I don't mind if you keep that udelay() as long as preemption isn't
explicitly blocked.

> Signed-off-by: Tvrtko Ursulin <tvrtko.ursulin at intel.com>
> Cc: Sebastian Andrzej Siewior <bigeasy at linutronix.de>
> ---
>  drivers/gpu/drm/i915/gt/intel_reset.c | 12 +++++-------
>  1 file changed, 5 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/gt/intel_reset.c b/drivers/gpu/drm/i915/gt/intel_reset.c
> index e2152f75ba2e..6916eba3bd33 100644
> --- a/drivers/gpu/drm/i915/gt/intel_reset.c
> +++ b/drivers/gpu/drm/i915/gt/intel_reset.c
> @@ -167,13 +167,13 @@ static int i915_do_reset(struct intel_gt *gt,
>  	/* Assert reset for at least 20 usec, and wait for acknowledgement. */
>  	pci_write_config_byte(pdev, I915_GDRST, GRDOM_RESET_ENABLE);
>  	udelay(50);
> -	err = wait_for_atomic(i915_in_reset(pdev), 50);
> +	err = _wait_for_atomic(i915_in_reset(pdev), 50, 0);
>  
>  	/* Clear the reset request. */
>  	pci_write_config_byte(pdev, I915_GDRST, 0);
>  	udelay(50);
>  	if (!err)
> -		err = wait_for_atomic(!i915_in_reset(pdev), 50);
> +		err = _wait_for_atomic(!i915_in_reset(pdev), 50, 0);
>  
>  	return err;
>  }
> @@ -193,7 +193,7 @@ static int g33_do_reset(struct intel_gt *gt,
>  	struct pci_dev *pdev = to_pci_dev(gt->i915->drm.dev);
>  
>  	pci_write_config_byte(pdev, I915_GDRST, GRDOM_RESET_ENABLE);
> -	return wait_for_atomic(g4x_reset_complete(pdev), 50);
> +	return _wait_for_atomic(g4x_reset_complete(pdev), 50, 0);
>  }
>  
>  static int g4x_do_reset(struct intel_gt *gt,
> @@ -210,7 +210,7 @@ static int g4x_do_reset(struct intel_gt *gt,
>  
>  	pci_write_config_byte(pdev, I915_GDRST,
>  			      GRDOM_MEDIA | GRDOM_RESET_ENABLE);
> -	ret =  wait_for_atomic(g4x_reset_complete(pdev), 50);
> +	ret =  _wait_for_atomic(g4x_reset_complete(pdev), 50, 0);
>  	if (ret) {
>  		GT_TRACE(gt, "Wait for media reset failed\n");
>  		goto out;
> @@ -218,7 +218,7 @@ static int g4x_do_reset(struct intel_gt *gt,
>  
>  	pci_write_config_byte(pdev, I915_GDRST,
>  			      GRDOM_RENDER | GRDOM_RESET_ENABLE);
> -	ret =  wait_for_atomic(g4x_reset_complete(pdev), 50);
> +	ret =  _wait_for_atomic(g4x_reset_complete(pdev), 50, 0);
>  	if (ret) {
>  		GT_TRACE(gt, "Wait for render reset failed\n");
>  		goto out;
> @@ -788,9 +788,7 @@ int __intel_gt_reset(struct intel_gt *gt, intel_engine_mask_t engine_mask)
>  		reset_mask = wa_14015076503_start(gt, engine_mask, !retry);
>  
>  		GT_TRACE(gt, "engine_mask=%x\n", reset_mask);
> -		preempt_disable();
>  		ret = reset(gt, reset_mask, retry);
> -		preempt_enable();
>  
>  		wa_14015076503_end(gt, reset_mask);
>  	}
> -- 
> 2.39.2
> 

Sebastian