On 10/22/2024 12:28 AM, Cavitt, Jonathan wrote:
> -----Original Message-----
> From: Intel-xe <intel-xe-bounces@lists.freedesktop.org> On Behalf Of Nirmoy Das
> Sent: Monday, October 21, 2024 2:12 PM
> To: intel-xe@lists.freedesktop.org
> Cc: Das, Nirmoy <nirmoy.das@intel.com>; Ghimiray, Himal Prasad <himal.prasad.ghimiray@intel.com>; Auld, Matthew <matthew.auld@intel.com>; Brost, Matthew <matthew.brost@intel.com>; Upadhyay, Tejas <tejas.upadhyay@intel.com>
> Subject: [PATCH] drm/xe: Don't restart parallel queues multiple times on GT reset
>> In case of parallel submissions multiple GuC id will point to the
>> same exec queue and on GT reset such exec queues will get restarted
>> multiple times which is not desirable.
>>
>> Link: https://gitlab.freedesktop.org/drm/xe/kernel/-/issues/2295
>> Cc: Himal Prasad Ghimiray <himal.prasad.ghimiray@intel.com>
>> Cc: Matthew Auld <matthew.auld@intel.com>
>> Cc: Matthew Brost <matthew.brost@intel.com>
>> Cc: Tejas Upadhyay <tejas.upadhyay@intel.com>
>> Signed-off-by: Nirmoy Das <nirmoy.das@intel.com>
> This makes sense, though I'd assume we'd want at least one of
> the GuC IDs to restart the exec queue during a GT reset.  Is that
> what exec_queue_enabled helps allow?

Yes exec_queue_enabled() should be false for the 1st one but as Matt pointed out this is racy. So I will resend this with

Matt suggested check.

Regards,

Nirmoy

>
> I won't block on it presently, just was curious if that case was being
> handled, or if we didn't need to worry about it.
>
> Reviewed-by: Jonathan Cavitt <jonathan.cavitt@intel.com>
> -Jonathan Cavitt
>
>> ---
>>  drivers/gpu/drm/xe/xe_guc_submit.c | 7 ++++++-
>>  1 file changed, 6 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/xe/xe_guc_submit.c b/drivers/gpu/drm/xe/xe_guc_submit.c
>> index 0b81972ff651..6aeb007eaf06 100644
>> --- a/drivers/gpu/drm/xe/xe_guc_submit.c
>> +++ b/drivers/gpu/drm/xe/xe_guc_submit.c
>> @@ -1784,8 +1784,13 @@ int xe_guc_submit_start(struct xe_guc *guc)
>>  
>>  	mutex_lock(&guc->submission_state.lock);
>>  	atomic_dec(&guc->submission_state.stopped);
>> -	xa_for_each(&guc->submission_state.exec_queue_lookup, index, q)
>> +	xa_for_each(&guc->submission_state.exec_queue_lookup, index, q) {
>> +		/* Skip restarting parallel queues */
>> +		if (exec_queue_enabled(q) && xe_exec_queue_is_parallel(q))
>> +			continue;
>> +
>>  		guc_exec_queue_start(q);
>> +	}
>>  	mutex_unlock(&guc->submission_state.lock);
>>  
>>  	wake_up_all(&guc->ct.wq);
>> -- 
>> 2.46.0
>>
>>