* [PATCH] drm/amdgpu: handle the case of pci_channel_io_frozen only in amdgpu_pci_resume
@ 2021-10-02 15:18 Guchun Chen
2021-10-04 16:04 ` Andrey Grodzovsky
0 siblings, 1 reply; 3+ messages in thread
From: Guchun Chen @ 2021-10-02 15:18 UTC (permalink / raw)
To: amd-gfx, christian.koenig, xinhui.pan, alexander.deucher,
andrey.grodzovsky, monk.liu
Cc: Guchun Chen
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@amd.com>
Signed-off-by: Guchun Chen <guchun.chen@amd.com>
---
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;
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];
--
2.17.1
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] drm/amdgpu: handle the case of pci_channel_io_frozen only in amdgpu_pci_resume
2021-10-02 15:18 [PATCH] drm/amdgpu: handle the case of pci_channel_io_frozen only in amdgpu_pci_resume Guchun Chen
@ 2021-10-04 16:04 ` Andrey Grodzovsky
2021-10-05 11:43 ` Chen, Guchun
0 siblings, 1 reply; 3+ messages in thread
From: Andrey Grodzovsky @ 2021-10-04 16:04 UTC (permalink / raw)
To: Guchun Chen, amd-gfx, christian.koenig, xinhui.pan,
alexander.deucher, monk.liu
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@amd.com>
> Signed-off-by: Guchun Chen <guchun.chen@amd.com>
> ---
> 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@amd.com>
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];
>
^ permalink raw reply [flat|nested] 3+ messages in thread
* RE: [PATCH] drm/amdgpu: handle the case of pci_channel_io_frozen only in amdgpu_pci_resume
2021-10-04 16:04 ` Andrey Grodzovsky
@ 2021-10-05 11:43 ` Chen, Guchun
0 siblings, 0 replies; 3+ messages in thread
From: Chen, Guchun @ 2021-10-05 11:43 UTC (permalink / raw)
To: Grodzovsky, Andrey, amd-gfx, Koenig, Christian, Pan, Xinhui,
Deucher, Alexander, Liu, Monk
[Public]
Thanks Andrey, I will update the name to be " pci_channel_state " when submitting.
Regards,
Guchun
-----Original Message-----
From: Grodzovsky, Andrey <Andrey.Grodzovsky@amd.com>
Sent: Tuesday, October 5, 2021 12:04 AM
To: Chen, Guchun <Guchun.Chen@amd.com>; amd-gfx@lists.freedesktop.org; Koenig, Christian <Christian.Koenig@amd.com>; Pan, Xinhui <Xinhui.Pan@amd.com>; Deucher, Alexander <Alexander.Deucher@amd.com>; Liu, Monk <Monk.Liu@amd.com>
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@amd.com>
> Signed-off-by: Guchun Chen <guchun.chen@amd.com>
> ---
> 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@amd.com>
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];
>
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2021-10-05 11:43 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-10-02 15:18 [PATCH] drm/amdgpu: handle the case of pci_channel_io_frozen only in amdgpu_pci_resume Guchun Chen
2021-10-04 16:04 ` Andrey Grodzovsky
2021-10-05 11:43 ` Chen, Guchun
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.