[PATCH v3 1/7] drm/amdgpu: update XGMI physical node id and GMC configs on resume

[Zhang, GuoQing (Sam)]

[AMD Official Use Only - AMD Internal Distribution Only]


> Please keep in mind that this is not the only scenario addressed by the
> driver - for ex: a resume sequence is executed after a device reset.
> This patch itself introduces unwanted sequences for other commonly used
> usecases. Please rework on the series without breaking existing usecases.
>
> Thanks,
> Lijo

Hi @Lazar, Lijo<mailto:Lijo.Lazar at amd.com>, Thank you for the feedback.

I also think the new code should be inside a check so that new code is executed only on resume with different VF and do not break existing usecases. Following is the implementation of this approach I can think of.
- introduce new field `prev_physical_node_id ` in `struct amdgpu_xgmi `. update the fields on resume.
- put new code inside code block `if (prev_physical_node_id  != physical_node_id )`

Is this approach acceptable? If not, can you suggest a better approach? @Lazar, Lijo<mailto:Lijo.Lazar at amd.com> @Koenig, Christian<mailto:Christian.Koenig at amd.com> Thank you!

Regards
Sam

From: Lazar, Lijo <Lijo.Lazar at amd.com>
Date: Tuesday, May 6, 2025 at 19:55
To: Zhang, GuoQing (Sam) <GuoQing.Zhang at amd.com>, amd-gfx at lists.freedesktop.org <amd-gfx at lists.freedesktop.org>
Cc: Zhao, Victor <Victor.Zhao at amd.com>, Chang, HaiJun <HaiJun.Chang at amd.com>, Koenig, Christian <Christian.Koenig at amd.com>, Deucher, Alexander <Alexander.Deucher at amd.com>, Zhang, Owen(SRDC) <Owen.Zhang2 at amd.com>, Ma, Qing (Mark) <Qing.Ma at amd.com>, Jiang Liu <gerry at linux.alibaba.com>
Subject: Re: [PATCH v3 1/7] drm/amdgpu: update XGMI physical node id and GMC configs on resume


On 5/6/2025 3:06 PM, Samuel Zhang wrote:
> For virtual machine with vGPUs in SRIOV single device mode and XGMI
> is enabled, XGMI physical node ids may change when waking up from
> hiberation with different vGPU devices. So update XGMI physical node
> ids on resume.
>

Please keep in mind that this is not the only scenario addressed by the
driver - for ex: a resume sequence is executed after a device reset.
This patch itself introduces unwanted sequences for other commonly used
usecases. Please rework on the series without breaking existing usecases.

Thanks,
Lijo

> Update GPU memory controller configuration on resume if XGMI physical
> node ids are changed.
>
> Signed-off-by: Jiang Liu <gerry at linux.alibaba.com>
> Signed-off-by: Samuel Zhang <guoqing.zhang at amd.com>
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 24 ++++++++++++++++++++++
>  drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c    |  3 +--
>  drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c      |  4 ++++
>  3 files changed, 29 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> index d477a901af84..e795af5067e5 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> @@ -5040,6 +5040,27 @@ int amdgpu_device_suspend(struct drm_device *dev, bool notify_clients)
>        return 0;
>  }
>
> +static int amdgpu_device_update_xgmi_info(struct amdgpu_device *adev)
> +{
> +     int r;
> +     unsigned int prev_physical_node_id;
> +
> +     /* Get xgmi info again for sriov to detect device changes */
> +     if (amdgpu_sriov_vf(adev) &&
> +         !(adev->flags & AMD_IS_APU) &&
> +         adev->gmc.xgmi.supported &&
> +         !adev->gmc.xgmi.connected_to_cpu) {
> +             prev_physical_node_id = adev->gmc.xgmi.physical_node_id;
> +             r = adev->gfxhub.funcs->get_xgmi_info(adev);
> +             if (r)
> +                     return r;
> +
> +             dev_info(adev->dev, "xgmi node, old id %d, new id %d\n",
> +                     prev_physical_node_id, adev->gmc.xgmi.physical_node_id);
> +     }
> +     return 0;
> +}
> +
>  /**
>   * amdgpu_device_resume - initiate device resume
>   *
> @@ -5059,6 +5080,9 @@ int amdgpu_device_resume(struct drm_device *dev, bool notify_clients)
>                r = amdgpu_virt_request_full_gpu(adev, true);
>                if (r)
>                        return r;
> +             r = amdgpu_device_update_xgmi_info(adev);
> +             if (r)
> +                     return r;
>        }
>
>        if (dev->switch_power_state == DRM_SWITCH_POWER_OFF)
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c
> index d1fa5e8e3937..a2abddf3c110 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gmc.c
> @@ -1298,8 +1298,7 @@ int amdgpu_gmc_get_nps_memranges(struct amdgpu_device *adev,
>        if (!mem_ranges || !exp_ranges)
>                return -EINVAL;
>
> -     refresh = (adev->init_lvl->level != AMDGPU_INIT_LEVEL_MINIMAL_XGMI) &&
> -               (adev->gmc.reset_flags & AMDGPU_GMC_INIT_RESET_NPS);
> +     refresh = true;
>        ret = amdgpu_discovery_get_nps_info(adev, &nps_type, &ranges,
>                                            &range_cnt, refresh);
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
> index 59385da80185..1eb451a3743b 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gmc_v9_0.c
> @@ -2533,6 +2533,10 @@ static int gmc_v9_0_resume(struct amdgpu_ip_block *ip_block)
>        struct amdgpu_device *adev = ip_block->adev;
>        int r;
>
> +     r = gmc_v9_0_mc_init(adev);
> +     if (r)
> +             return r;
> +
>        /* If a reset is done for NPS mode switch, read the memory range
>         * information again.
>         */
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/amd-gfx/attachments/20250507/10eee373/attachment.htm>