* [PATCH] drm/amdgpu/vcn: drop extra cancel_delayed_work_sync() @ 2025-08-13 19:36 Alex Deucher 2025-08-20 18:36 ` Alex Deucher 2025-08-21 14:56 ` Liu, Leo 0 siblings, 2 replies; 6+ messages in thread From: Alex Deucher @ 2025-08-13 19:36 UTC (permalink / raw) To: amd-gfx; +Cc: Alex Deucher We already call this in the hw_fini() methods for all VCN instances, so no need to call it again in amdgpu_vcn_suspend(). Signed-off-by: Alex Deucher <alexander.deucher@amd.com> --- drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c index 9a76e11d1c184..fd8ebf4b5a824 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c @@ -357,8 +357,6 @@ int amdgpu_vcn_suspend(struct amdgpu_device *adev, int i) if (adev->vcn.harvest_config & (1 << i)) return 0; - cancel_delayed_work_sync(&adev->vcn.inst[i].idle_work); - /* err_event_athub and dpc recovery will corrupt VCPU buffer, so we need to * restore fw data and clear buffer in amdgpu_vcn_resume() */ if (in_ras_intr || adev->pcie_reset_ctx.in_link_reset) -- 2.50.1 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] drm/amdgpu/vcn: drop extra cancel_delayed_work_sync() 2025-08-13 19:36 [PATCH] drm/amdgpu/vcn: drop extra cancel_delayed_work_sync() Alex Deucher @ 2025-08-20 18:36 ` Alex Deucher 2025-08-20 19:29 ` Wu, David 2025-08-21 14:56 ` Liu, Leo 1 sibling, 1 reply; 6+ messages in thread From: Alex Deucher @ 2025-08-20 18:36 UTC (permalink / raw) To: Alex Deucher; +Cc: amd-gfx Ping? On Wed, Aug 13, 2025 at 3:37 PM Alex Deucher <alexander.deucher@amd.com> wrote: > > We already call this in the hw_fini() methods for all > VCN instances, so no need to call it again in > amdgpu_vcn_suspend(). > > Signed-off-by: Alex Deucher <alexander.deucher@amd.com> > --- > drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c | 2 -- > 1 file changed, 2 deletions(-) > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c > index 9a76e11d1c184..fd8ebf4b5a824 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c > @@ -357,8 +357,6 @@ int amdgpu_vcn_suspend(struct amdgpu_device *adev, int i) > if (adev->vcn.harvest_config & (1 << i)) > return 0; > > - cancel_delayed_work_sync(&adev->vcn.inst[i].idle_work); > - > /* err_event_athub and dpc recovery will corrupt VCPU buffer, so we need to > * restore fw data and clear buffer in amdgpu_vcn_resume() */ > if (in_ras_intr || adev->pcie_reset_ctx.in_link_reset) > -- > 2.50.1 > ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] drm/amdgpu/vcn: drop extra cancel_delayed_work_sync() 2025-08-20 18:36 ` Alex Deucher @ 2025-08-20 19:29 ` Wu, David 2025-08-20 19:43 ` Alex Deucher 0 siblings, 1 reply; 6+ messages in thread From: Wu, David @ 2025-08-20 19:29 UTC (permalink / raw) To: Alex Deucher, Deucher, Alexander; +Cc: amd-gfx@lists.freedesktop.org [-- Attachment #1: Type: text/plain, Size: 1439 bytes --] [AMD Official Use Only - AMD Internal Distribution Only] one thing I noticed: amdgpu_vcn_suspend() also gets called in vcn_v2_5_sw_fini() for example where there isn't cancel_delayed_work_sync() call. Does the removal work in this case? David On 8/20/2025 2:36 PM, Alex Deucher wrote: Ping? On Wed, Aug 13, 2025 at 3:37 PM Alex Deucher <alexander.deucher@amd.com><mailto:alexander.deucher@amd.com> wrote: We already call this in the hw_fini() methods for all VCN instances, so no need to call it again in amdgpu_vcn_suspend(). Signed-off-by: Alex Deucher <alexander.deucher@amd.com><mailto:alexander.deucher@amd.com> --- drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c index 9a76e11d1c184..fd8ebf4b5a824 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c @@ -357,8 +357,6 @@ int amdgpu_vcn_suspend(struct amdgpu_device *adev, int i) if (adev->vcn.harvest_config & (1 << i)) return 0; - cancel_delayed_work_sync(&adev->vcn.inst[i].idle_work); - /* err_event_athub and dpc recovery will corrupt VCPU buffer, so we need to * restore fw data and clear buffer in amdgpu_vcn_resume() */ if (in_ras_intr || adev->pcie_reset_ctx.in_link_reset) -- 2.50.1 [-- Attachment #2: Type: text/html, Size: 2392 bytes --] ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] drm/amdgpu/vcn: drop extra cancel_delayed_work_sync() 2025-08-20 19:29 ` Wu, David @ 2025-08-20 19:43 ` Alex Deucher 2025-08-21 20:12 ` David Wu 0 siblings, 1 reply; 6+ messages in thread From: Alex Deucher @ 2025-08-20 19:43 UTC (permalink / raw) To: Wu, David; +Cc: Deucher, Alexander, amd-gfx@lists.freedesktop.org On Wed, Aug 20, 2025 at 3:29 PM Wu, David <David.Wu3@amd.com> wrote: > > [AMD Official Use Only - AMD Internal Distribution Only] > > > one thing I noticed: > amdgpu_vcn_suspend() also gets called in vcn_v2_5_sw_fini() for example > where there isn't cancel_delayed_work_sync() call. Does the removal work in this case? The sw callbacks are not supposed to touch any hardware, so that's wrong to begin with. That said, this should never end up doing anything because we've already cancelled the delayed worker in hw_fini and after hw_fini the hw is stopped so no work should be outstanding. Alex > > David > On 8/20/2025 2:36 PM, Alex Deucher wrote: > > Ping? > > On Wed, Aug 13, 2025 at 3:37 PM Alex Deucher <alexander.deucher@amd.com> wrote: > > We already call this in the hw_fini() methods for all > VCN instances, so no need to call it again in > amdgpu_vcn_suspend(). > > Signed-off-by: Alex Deucher <alexander.deucher@amd.com> > --- > drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c | 2 -- > 1 file changed, 2 deletions(-) > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c > index 9a76e11d1c184..fd8ebf4b5a824 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c > @@ -357,8 +357,6 @@ int amdgpu_vcn_suspend(struct amdgpu_device *adev, int i) > if (adev->vcn.harvest_config & (1 << i)) > return 0; > > - cancel_delayed_work_sync(&adev->vcn.inst[i].idle_work); > - > /* err_event_athub and dpc recovery will corrupt VCPU buffer, so we need to > * restore fw data and clear buffer in amdgpu_vcn_resume() */ > if (in_ras_intr || adev->pcie_reset_ctx.in_link_reset) > -- > 2.50.1 > > ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] drm/amdgpu/vcn: drop extra cancel_delayed_work_sync() 2025-08-20 19:43 ` Alex Deucher @ 2025-08-21 20:12 ` David Wu 0 siblings, 0 replies; 6+ messages in thread From: David Wu @ 2025-08-21 20:12 UTC (permalink / raw) To: Alex Deucher, Wu, David; +Cc: Deucher, Alexander, amd-gfx@lists.freedesktop.org [-- Attachment #1: Type: text/plain, Size: 2240 bytes --] Did some tests with the sw callbacks which were called with rmmod. It will fail if there are jobs, otherwise no need to cancel delayed work as there is not any outstanding work and idle handler called or not did not affect the removal of the module. insmod continues to work after that. The case for hw_fini works as expected. Tested-by: David (Ming Qiang) Wu<David.Wu3@amd.com> Thanks, David On 2025-08-20 15:43, Alex Deucher wrote: > On Wed, Aug 20, 2025 at 3:29 PM Wu, David<David.Wu3@amd.com> wrote: >> [AMD Official Use Only - AMD Internal Distribution Only] >> >> >> one thing I noticed: >> amdgpu_vcn_suspend() also gets called in vcn_v2_5_sw_fini() for example >> where there isn't cancel_delayed_work_sync() call. Does the removal work in this case? > The sw callbacks are not supposed to touch any hardware, so that's > wrong to begin with. That said, this should never end up doing > anything because we've already cancelled the delayed worker in hw_fini > and after hw_fini the hw is stopped so no work should be outstanding. > > Alex > >> David >> On 8/20/2025 2:36 PM, Alex Deucher wrote: >> >> Ping? >> >> On Wed, Aug 13, 2025 at 3:37 PM Alex Deucher<alexander.deucher@amd.com> wrote: >> >> We already call this in the hw_fini() methods for all >> VCN instances, so no need to call it again in >> amdgpu_vcn_suspend(). >> >> Signed-off-by: Alex Deucher<alexander.deucher@amd.com> >> --- >> drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c | 2 -- >> 1 file changed, 2 deletions(-) >> >> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c >> index 9a76e11d1c184..fd8ebf4b5a824 100644 >> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c >> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c >> @@ -357,8 +357,6 @@ int amdgpu_vcn_suspend(struct amdgpu_device *adev, int i) >> if (adev->vcn.harvest_config & (1 << i)) >> return 0; >> >> - cancel_delayed_work_sync(&adev->vcn.inst[i].idle_work); >> - >> /* err_event_athub and dpc recovery will corrupt VCPU buffer, so we need to >> * restore fw data and clear buffer in amdgpu_vcn_resume() */ >> if (in_ras_intr || adev->pcie_reset_ctx.in_link_reset) >> -- >> 2.50.1 >> >> [-- Attachment #2: Type: text/html, Size: 3114 bytes --] ^ permalink raw reply [flat|nested] 6+ messages in thread
* RE: [PATCH] drm/amdgpu/vcn: drop extra cancel_delayed_work_sync() 2025-08-13 19:36 [PATCH] drm/amdgpu/vcn: drop extra cancel_delayed_work_sync() Alex Deucher 2025-08-20 18:36 ` Alex Deucher @ 2025-08-21 14:56 ` Liu, Leo 1 sibling, 0 replies; 6+ messages in thread From: Liu, Leo @ 2025-08-21 14:56 UTC (permalink / raw) To: Deucher, Alexander, amd-gfx@lists.freedesktop.org; +Cc: Deucher, Alexander [AMD Official Use Only - AMD Internal Distribution Only] Looks reasonable to me, the patch is: Reviewed-by: Leo Liu <leo.liu@amd.com> > -----Original Message----- > From: amd-gfx <amd-gfx-bounces@lists.freedesktop.org> On Behalf Of Alex > Deucher > Sent: August 13, 2025 3:37 PM > To: amd-gfx@lists.freedesktop.org > Cc: Deucher, Alexander <Alexander.Deucher@amd.com> > Subject: [PATCH] drm/amdgpu/vcn: drop extra cancel_delayed_work_sync() > > We already call this in the hw_fini() methods for all VCN instances, so no need > to call it again in amdgpu_vcn_suspend(). > > Signed-off-by: Alex Deucher <alexander.deucher@amd.com> > --- > drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c | 2 -- > 1 file changed, 2 deletions(-) > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c > b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c > index 9a76e11d1c184..fd8ebf4b5a824 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vcn.c > @@ -357,8 +357,6 @@ int amdgpu_vcn_suspend(struct amdgpu_device > *adev, int i) > if (adev->vcn.harvest_config & (1 << i)) > return 0; > > - cancel_delayed_work_sync(&adev->vcn.inst[i].idle_work); > - > /* err_event_athub and dpc recovery will corrupt VCPU buffer, so we > need to > * restore fw data and clear buffer in amdgpu_vcn_resume() */ > if (in_ras_intr || adev->pcie_reset_ctx.in_link_reset) > -- > 2.50.1 ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2025-08-21 20:12 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2025-08-13 19:36 [PATCH] drm/amdgpu/vcn: drop extra cancel_delayed_work_sync() Alex Deucher 2025-08-20 18:36 ` Alex Deucher 2025-08-20 19:29 ` Wu, David 2025-08-20 19:43 ` Alex Deucher 2025-08-21 20:12 ` David Wu 2025-08-21 14:56 ` Liu, Leo
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox