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

[Tvrtko Ursulin]

On 04/07/2023 10:41, Sebastian Andrzej Siewior wrote:
> 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.

CI also thinks it looks fine across all platforms and including some GPU 
reset stress testing. I'll try to come up with a good commit message and 
send it out shortly.

Regards,

Tvrtko

>> 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