<html xmlns:o="urn:schemas-microsoft-com:office:office" xmlns:w="urn:schemas-microsoft-com:office:word" xmlns:m="http://schemas.microsoft.com/office/2004/12/omml" xmlns="http://www.w3.org/TR/REC-html40"> <head> <meta http-equiv="Content-Type" content="text/html; charset=us-ascii"> <meta name="Generator" content="Microsoft Word 15 (filtered medium)"> <style></style> </head> <body lang="en-CN" link="#467886" vlink="#96607D" style="word-wrap:break-word"> [AMD Official Use Only - AMD Internal Distribution Only] <div> <div class="WordSection1"> <o:p> </o:p> > Please keep in mind that this is not the only scenario addressed by the<o:p></o:p> > driver - for ex: a resume sequence is executed after a device reset.<o:p></o:p> > This patch itself introduces unwanted sequences for other commonly used<o:p></o:p> > usecases. Please rework on the series without breaking existing usecases.<o:p></o:p> > <o:p> </o:p> > Thanks,<o:p></o:p> > Lijo<o:p></o:p> <o:p> </o:p> Hi <a id="OWAAMBAFCD4633AA3514C9B5BCB5E58BDD327" href="mailto:Lijo.Lazar@amd.com">@Lazar, Lijo</a>, Thank you for the feedback. <o:p></o:p> <o:p> </o:p> 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. <o:p></o:p> - introduce new field `prev_physical_node_id ` in `struct amdgpu_xgmi `. update the fields on resume.<o:p></o:p> - put new code inside code block `if (prev_physical_node_id != physical_node_id )`<o:p></o:p> <o:p> </o:p> Is this approach acceptable? If not, can you suggest a better approach? <a id="OWAAM57DEDBE5638C4240A750A18A148939F0" href="mailto:Lijo.Lazar@amd.com">@Lazar, Lijo</a> <a id="OWAAM10C90ABE505E85408A10A63A9A44DFE8" href="mailto:Christian.Koenig@amd.com"> @Koenig, Christian</a> Thank you!<o:p></o:p> <o:p> </o:p> Regards<o:p></o:p> Sam<o:p></o:p> <o:p> </o:p> <div id="mail-editor-reference-message-container"> <div> <div style="border:none;border-top:solid #B5C4DF 1.0pt;padding:3.0pt 0cm 0cm 0cm"> From: Lazar, Lijo <Lijo.Lazar@amd.com> Date: Tuesday, May 6, 2025 at 19:55 To: Zhang, GuoQing (Sam) <GuoQing.Zhang@amd.com>, amd-gfx@lists.freedesktop.org <amd-gfx@lists.freedesktop.org> Cc: Zhao, Victor <Victor.Zhao@amd.com>, Chang, HaiJun <HaiJun.Chang@amd.com>, Koenig, Christian <Christian.Koenig@amd.com>, Deucher, Alexander <Alexander.Deucher@amd.com>, Zhang, Owen(SRDC) <Owen.Zhang2@amd.com>, Ma, Qing (Mark) <Qing.Ma@amd.com>, Jiang Liu <gerry@linux.alibaba.com> Subject: Re: [PATCH v3 1/7] drm/amdgpu: update XGMI physical node id and GMC configs on resume<o:p></o:p> </div> <div> 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@linux.alibaba.com> > Signed-off-by: Samuel Zhang <guoqing.zhang@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. > */ </div> </div> </div> </div> </div> </body> </html>