Re: [PATCH] drm/amdgpu: Use SPX as default in partition config

["Lazar, Lijo" <lijo.lazar@amd.com>]

On 10/19/2024 12:46 AM, Felix Kuehling wrote:
> 
> On 2024-10-14 05:19, Lijo Lazar wrote:
>> In certain cases - ex: when a reset is required on initialization - XCP
>> manager won't have a valid partition mode. In such cases, use SPX as the
>> default selected mode for which partition configuration details are
>> populated.
>>
>> Signed-off-by: Lijo Lazar <lijo.lazar@amd.com>
>> Reported-by: Hao Zhou <hao.zhou@amd.com>
>>
>> Fixes: c7de57033d9b ("drm/amdgpu: Add sysfs nodes to get xcp details")
>> ---
>>   drivers/gpu/drm/amd/amdgpu/amdgpu_xcp.c | 10 +++++++---
>>   1 file changed, 7 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_xcp.c
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_xcp.c
>> index 111bf897e72e..83a16918ea76 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_xcp.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_xcp.c
>> @@ -606,7 +606,7 @@ void amdgpu_xcp_cfg_sysfs_init(struct
>> amdgpu_device *adev)
>>   {
>>       struct amdgpu_xcp_res_details *xcp_res;
>>       struct amdgpu_xcp_cfg *xcp_cfg;
>> -    int i, r, j, rid;
>> +    int i, r, j, rid, mode;
>>         if (!adev->xcp_mgr)
>>           return;
>> @@ -625,11 +625,15 @@ void amdgpu_xcp_cfg_sysfs_init(struct
>> amdgpu_device *adev)
>>       if (r)
>>           goto err1;
>>   -    r = amdgpu_xcp_get_res_info(xcp_cfg->xcp_mgr,
>> xcp_cfg->xcp_mgr->mode, xcp_cfg);
>> +    mode = (xcp_cfg->xcp_mgr->mode ==
>> +        AMDGPU_UNKNOWN_COMPUTE_PARTITION_MODE) ?
>> +               AMDGPU_SPX_PARTITION_MODE :
>> +               xcp_cfg->xcp_mgr->mode;
> 
> Shouldn't this depend on the memory partition mode as well? You must
> have at least as many compute partitions as memory partitions because
> each compute partition can only use a single memory partition.

This is not dependent on the current/active compute partition mode. It
is to show the configuration (number of xccs, vcns, shared etc.)
supported for a partition mode. SPX is the default partition mode at
boot up. That is used as the default mode.

It's not a strict one-to-one, a compute partition may use other memory
partitions also non-coherently.

Thanks,
Lijo

> 
> Regards,
>   Felix
> 
> 
>> +    r = amdgpu_xcp_get_res_info(xcp_cfg->xcp_mgr, mode, xcp_cfg);
>>       if (r)
>>           goto err1;
>>   -    xcp_cfg->mode = xcp_cfg->xcp_mgr->mode;
>> +    xcp_cfg->mode = mode;
>>       for (i = 0; i < xcp_cfg->num_res; i++) {
>>           xcp_res = &xcp_cfg->xcp_res[i];
>>           rid = xcp_res->id;