[Intel-gfx] [PATCH 07/15] drm/i915/guc/slpc: Remove BUG_ON in guc_submission_disable

[Belgaumkar, Vinay]

On 7/27/2021 5:20 PM, Matthew Brost wrote:
> On Mon, Jul 26, 2021 at 12:07:52PM -0700, Vinay Belgaumkar wrote:
>> The assumption when it was added was there would be no wakerefs
>> held. However, if we fail to enable SLPC, we will still be
>> holding a wakeref.
>>
> 
> So this is if intel_guc_slpc_enable() fails, right? Not seeing where the
> wakeref is taken. It also seems wrong not to drop the wakeref before
> calling intel_guc_submission_disable, hence the GEM_BUG_ON in this
> function.
> 
> Can you explain this bit more?

I should change the desc a little. The BUG_ON assumed GT would not be 
awake i.e at shutdown, and there would be 0 GT_PM references. However, 
this slpc_enable is in gt_resume path (gt_init_hw calls uc_init_hw). 
Here, gt_pm_get reference is held, so it will result in BUG_ON when 
submission_disable is called.

Thanks,
Vinay.
> 
> Matt
> 
>> Signed-off-by: Vinay Belgaumkar <vinay.belgaumkar at intel.com>
>> ---
>>   drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c | 4 ----
>>   1 file changed, 4 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
>> index b6338742a594..48cbd800ca54 100644
>> --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
>> +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c
>> @@ -2523,10 +2523,6 @@ void intel_guc_submission_enable(struct intel_guc *guc)
>>   
>>   void intel_guc_submission_disable(struct intel_guc *guc)
>>   {
>> -	struct intel_gt *gt = guc_to_gt(guc);
>> -
>> -	GEM_BUG_ON(gt->awake); /* GT should be parked first */
>> -
>>   	/* Note: By the time we're here, GuC may have already been reset */
>>   }
>>   
>> -- 
>> 2.25.0
>>