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

[Felix Kuehling <felix.kuehling@amd.com>]

On 2024-10-21 10:07, Lazar, Lijo wrote:
> 
> 
> 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.

I agree it's not 1:1. Multiple compute partitions can share a single memory partition. But there is no way for a compute partition to use multiple memory partitions, at least with compute APIs. Each partition exposes only one VRAM heap. Therefore I think the memory partition mode should influence the default compute partition mode. E.g. SPX can only work in NPS1 mode.

Regards,
  Felix

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