AMD General

Thanks, will improve this.

Best Regards,

Liu, Xiang

________________________________
From: Lazar, Lijo <[email protected]>
Sent: Tuesday, July 14, 2026 6:16 PM
To: Liu, Xiang(Dean) <[email protected]>; [email protected] 
<[email protected]>
Cc: Zhang, Hawking <[email protected]>; Zhou1, Tao <[email protected]>; 
Yang, Stanley <[email protected]>; Chai, Thomas <[email protected]>
Subject: Re: [PATCH] drm/amd/ras: initialize CPER after XGMI reset on init



On 14-Jul-26 3:28 PM, Xiang Liu wrote:
> The XGMI reset-on-init path can run while the device is still at the
> minimal init level, such as during an NPS memory partition switch. In
> that flow the normal RAS IP block hw_init is skipped, so unified RAS
> is not enabled when the early CPER initialization is attempted.
>
> Resume RAS after the XGMI reset-on-init completes and retry CPER
> initialization for devices that still do not have CPER enabled. This
> lets the CPER ring come up once RAS is ready while keeping the path a
> no-op for devices where CPER was already initialized.
>
> CPER may now be initialized after the normal debugfs setup has already
> walked the ring list, so register the CPER ring debugfs entry from the
> retry path when debugfs is available. Track CPER ring debugfs
> registration to avoid duplicate registration and clear that state when
> CPER is torn down.
>
> Signed-off-by: Xiang Liu <[email protected]>
> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_cper.c    |  1 +
>   drivers/gpu/drm/amd/amdgpu/amdgpu_cper.h    |  1 +
>   drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c |  5 +++++
>   drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.c    | 15 +++++++++++++++
>   4 files changed, 22 insertions(+)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cper.c 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_cper.c
> index 6fb129025761..7513541e6eff 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cper.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cper.c
> @@ -504,6 +504,7 @@ int amdgpu_cper_fini(struct amdgpu_device *adev)
>                return 0;
>
>        adev->cper.enabled = false;
> +     adev->cper.ring_debugfs_registered = false;
>
>        amdgpu_ring_fini(&(adev->cper.ring_buf));
>        adev->cper.count = 0;
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cper.h 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_cper.h
> index d12c98077d9d..76df176eab9f 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cper.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cper.h
> @@ -51,6 +51,7 @@ enum amdgpu_cper_type {
>
>   struct amdgpu_cper {
>        bool enabled;
> +     bool ring_debugfs_registered;
>
>        atomic_t unique_id;
>        struct mutex cper_lock;
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c
> index 3d1ede5f8de2..78130f8565c5 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c
> @@ -2184,8 +2184,13 @@ int amdgpu_debugfs_init(struct amdgpu_device *adev)
>
>                if (!ring)
>                        continue;
> +             if (ring == &adev->cper.ring_buf &&
> +                 adev->cper.ring_debugfs_registered)
> +                     continue;

Instead of a new flag, can this check if (!adev->cper.enabled)?>
>                amdgpu_debugfs_ring_init(adev, ring);
> +             if (ring == &adev->cper.ring_buf)
> +                     adev->cper.ring_debugfs_registered = true;
>        }
>
>        for (i = 0; i < adev->vcn.num_vcn_inst; i++) {
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.c 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.c
> index b265b68e0f37..4ec8e446d377 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_xgmi.c
> @@ -1400,6 +1400,21 @@ static void amdgpu_xgmi_reset_on_init_work(struct 
> work_struct *work)
>                 * initialized, and for non-uniras devices.
>                 */
>                amdgpu_ras_resume_after_reset(tmp_adev);
> +
> +             if (!tmp_adev->cper.enabled) {
> +                     r = amdgpu_cper_init(tmp_adev);
> +                     if (r)
> +                             dev_err(tmp_adev->dev, "failed to initialize 
> CPER after XGMI reset-on-init\n");
> +#if defined(CONFIG_DEBUG_FS)
> +                     else if (tmp_adev->cper.enabled &&
> +                              !tmp_adev->cper.ring_debugfs_registered &&
> +                              adev_to_drm(tmp_adev)->primary->debugfs_root) {
> +                             amdgpu_debugfs_ring_init(tmp_adev,
> +                                                      
> &tmp_adev->cper.ring_buf);
> +                             tmp_adev->cper.ring_debugfs_registered = true;
> +                     }
> +#endif

Here it could then init both debugfs and cper together since cper is
enabled only at this point.

BTW, is it possible to call both inside amdgpu_ras_resume_after_reset?
If not, better to create a wrapper function like
amdgpu_cper_deferred_init or similar and move inside that.

Thanks,
Lijo

> +             }
>        }
>   }
>

Reply via email to