[Public]
Thanks Andrey, I will update the name to be " pci_channel_state " when submitting.
Regards,
Guchun
-----Original Message-----
From: Grodzovsky, Andrey <Andrey.Grodzovsky@xxxxxxx>
Sent: Tuesday, October 5, 2021 12:04 AM
To: Chen, Guchun <Guchun.Chen@xxxxxxx>; amd-gfx@xxxxxxxxxxxxxxxxxxxxx; Koenig, Christian <Christian.Koenig@xxxxxxx>; Pan, Xinhui <Xinhui.Pan@xxxxxxx>; Deucher, Alexander <Alexander.Deucher@xxxxxxx>; Liu, Monk <Monk.Liu@xxxxxxx>
Subject: Re: [PATCH] drm/amdgpu: handle the case of pci_channel_io_frozen only in amdgpu_pci_resume
On 2021-10-02 11:18 a.m., Guchun Chen wrote:
> In current code, when a PCI error state pci_channel_io_normal is
> detectd, it will report PCI_ERS_RESULT_CAN_RECOVER status to PCI
> driver, and PCI driver will continue the execution of PCI resume
> callback report_resume by pci_walk_bridge, and the callback will go
> into amdgpu_pci_resume finally, where write lock is releasd
> unconditionally without acquiring such lock first. In this case, a
> deadlock will happen when other threads start to acquire the read lock.
>
> To fix this, add a member in amdgpu_device strucutre to cache
> pci_channel_state, and only continue the execution in
> amdgpu_pci_resume when it's pci_channel_io_frozen.
>
> Fixes: c9a6b82f45e2("drm/amdgpu: Implement DPC recovery")
> Suggested-by: Andrey Grodzovsky <andrey.grodzovsky@xxxxxxx>
> Signed-off-by: Guchun Chen <guchun.chen@xxxxxxx>
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu.h | 1 +
> drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 6 ++++++
> 2 files changed, 7 insertions(+)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> index f4bceb2624fb..720d0ccecfe0 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
> @@ -1094,6 +1094,7 @@ struct amdgpu_device {
>
> bool no_hw_access;
> struct pci_saved_state *pci_state;
> + pci_channel_state_t cached_state;
I would give a more descriptive name to this (e.g. pci_channel_state) Other then that Reviewed-by: Andrey Grodzovsky <andrey.grodzovsky@xxxxxxx>
Andrey
>
> struct amdgpu_reset_control *reset_cntl;
> uint32_t ip_versions[HW_ID_MAX][HWIP_MAX_INSTANCE];
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> index bb5ad2b6ca13..1aaeb4b30edc 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
> @@ -5368,6 +5368,8 @@ pci_ers_result_t amdgpu_pci_error_detected(struct pci_dev *pdev, pci_channel_sta
> return PCI_ERS_RESULT_DISCONNECT;
> }
>
> + adev->cached_state = state;
> +
> switch (state) {
> case pci_channel_io_normal:
> return PCI_ERS_RESULT_CAN_RECOVER; @@ -5510,6 +5512,10 @@ void
> amdgpu_pci_resume(struct pci_dev *pdev)
>
> DRM_INFO("PCI error: resume callback!!\n");
>
> + /* Only continue execution for the case of pci_channel_io_frozen */
> + if (adev->cached_state != pci_channel_io_frozen)
> + return;
> +
> for (i = 0; i < AMDGPU_MAX_RINGS; ++i) {
> struct amdgpu_ring *ring = adev->rings[i];
>
