Re: [PATCH 2/2] drm/xe: make change ccs_mode a synchronous action

[Maciej Patelczyk <maciej.patelczyk@intel.com>]

On 9.12.2024 17:26, Lucas De Marchi wrote:
> On Mon, Dec 09, 2024 at 03:12:10PM +0100, Maciej Patelczyk wrote:
>> If ccs_mode is being modified via
>>   /sys/class/drm/cardX/device/tileY/gtY/ccs_mode
>> the asynchronous reset is triggered and the write returns immediately.
>>
>> With that some test receive false information about number of CCS 
>> engines
>> or even fail if they proceed without delay after changing the ccs_mode.
>>
>> Changing the ccs_mode change from async to sync to prevent failures in
>> tests.
>>
>> Signed-off-by: Maciej Patelczyk <maciej.patelczyk@intel.com>
>> ---
>> drivers/gpu/drm/xe/xe_gt_ccs_mode.c | 6 +++++-
>> 1 file changed, 5 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/xe/xe_gt_ccs_mode.c 
>> b/drivers/gpu/drm/xe/xe_gt_ccs_mode.c
>> index b6adfb9f2030..8d02ec3f7d12 100644
>> --- a/drivers/gpu/drm/xe/xe_gt_ccs_mode.c
>> +++ b/drivers/gpu/drm/xe/xe_gt_ccs_mode.c
>> @@ -115,7 +115,7 @@ ccs_mode_store(struct device *kdev, struct 
>> device_attribute *attr,
>>     struct xe_gt *gt = kobj_to_gt(&kdev->kobj);
>>     struct xe_device *xe = gt_to_xe(gt);
>>     u32 num_engines, num_slices;
>> -    int ret;
>> +    int mode_changed = 0, ret;
>>
>>     if (IS_SRIOV(xe)) {
>>         xe_gt_dbg(gt, "Can't change compute mode when running as %s\n",
>> @@ -151,10 +151,14 @@ ccs_mode_store(struct device *kdev, struct 
>> device_attribute *attr,
>>         gt->ccs_mode = num_engines;
>>         xe_gt_record_user_engines(gt);
>>         xe_gt_reset_async(gt);
>> +        mode_changed = 1;
>
> I think this can simply be the sync version. We don't want to allow new
> clients being created while this reset is taking place, which is what
> the mutex below is protecting against.
> Lucas De Marchi 
Hi!

Does it mean that the xe_gt_reset_wait() shall be in the critical section?

The wait can be unconditional as flush_work won't block if there is no work,

but I haven't tested that.

Maciej

>>     }
>>
>>     mutex_unlock(&xe->drm.filelist_mutex);
>>
>> +    if (mode_changed)
>> +        xe_gt_reset_wait(gt);
>> +
>>     return count;
>> }
>>
>> -- 
>> 2.43.0
>>