[PATCH] drm/amdgpu: only WARN freeing buffers when DMA is unavailable

[Christian König]

Am 06.02.23 um 09:28 schrieb Xiao, Jack:
>
> [AMD Official Use Only - General]
>
>                >> >> It's simply not allowed to free up resources 
> during suspend since those can't be acquired again during resume.
>
>                               >> The in_suspend flag is set at the 
> beginning of suspend and unset at the end of resume. It can’t filter 
> the case you mentioned.
>
>
>                Why not? This is exactly what it should do.
>
> [Jack] If freeing up resources during resume, it should not hit the 
> issue you described. But only checking in_suspend flag would take 
> these cases as warning.
>

No, once more: Freeing up or allocating resources between suspend and 
resume is illegal!

If you free up a resource during resume you should absolutely hit that, 
this is intentional!

Regards,
Christian.

> Regards,
>
> Jack
>
> *From:* Koenig, Christian <Christian.Koenig at amd.com>
> *Sent:* Monday, February 6, 2023 4:06 PM
> *To:* Xiao, Jack <Jack.Xiao at amd.com>; Christian König 
> <ckoenig.leichtzumerken at gmail.com>; amd-gfx at lists.freedesktop.org; 
> Deucher, Alexander <Alexander.Deucher at amd.com>
> *Subject:* Re: [PATCH] drm/amdgpu: only WARN freeing buffers when DMA 
> is unavailable
>
> Am 06.02.23 um 08:23 schrieb Xiao, Jack:
>
>     [AMD Official Use Only - General]
>
>     >> Nope, that is not related to any hw state.
>
>     can use other flag.
>
>     >> It's simply not allowed to free up resources during suspend
>     since those can't be acquired again during resume.
>
>     The in_suspend flag is set at the beginning of suspend and unset
>     at the end of resume. It can’t filter the case you mentioned.
>
>
> Why not? This is exactly what it should do.
>
>     Do you know the root cause of these cases hitting the issue? So
>     that we can get an exact point to warn the freeing up behavior.
>
>
> Well the root cause are programming errors. See between suspending and 
> resuming you should not allocate nor free memory.
>
> Otherwise we can run into trouble. And this check here is one part of 
> that, we should probably add another warning during allocation of 
> memory. But this here is certainly correct.
>
> Regards,
> Christian.
>
>     Thanks,
>
>     Jack
>
>     *From:* Christian König <ckoenig.leichtzumerken at gmail.com>
>     <mailto:ckoenig.leichtzumerken at gmail.com>
>     *Sent:* Friday, February 3, 2023 9:20 PM
>     *To:* Xiao, Jack <Jack.Xiao at amd.com> <mailto:Jack.Xiao at amd.com>;
>     Koenig, Christian <Christian.Koenig at amd.com>
>     <mailto:Christian.Koenig at amd.com>; amd-gfx at lists.freedesktop.org;
>     Deucher, Alexander <Alexander.Deucher at amd.com>
>     <mailto:Alexander.Deucher at amd.com>
>     *Subject:* Re: [PATCH] drm/amdgpu: only WARN freeing buffers when
>     DMA is unavailable
>
>     Nope, that is not related to any hw state.
>
>     It's simply not allowed to free up resources during suspend since
>     those can't be acquired again during resume.
>
>     We had a couple of cases now where this was wrong. If you get a
>     warning from that please fix the code which tried to free
>     something during suspend instead.
>
>     Regards,
>     Christian.
>
>     Am 03.02.23 um 07:04 schrieb Xiao, Jack:
>
>         [AMD Official Use Only - General]
>
>         >> It's simply illegal to free up memory during suspend.
>
>         Why? In my understanding, the limit was caused by DMA shutdown.
>
>         Regards,
>
>         Jack
>
>         *From:* Koenig, Christian <Christian.Koenig at amd.com>
>         <mailto:Christian.Koenig at amd.com>
>         *Sent:* Thursday, February 2, 2023 7:43 PM
>         *To:* Xiao, Jack <Jack.Xiao at amd.com>
>         <mailto:Jack.Xiao at amd.com>; amd-gfx at lists.freedesktop.org;
>         Deucher, Alexander <Alexander.Deucher at amd.com>
>         <mailto:Alexander.Deucher at amd.com>
>         *Subject:* AW: [PATCH] drm/amdgpu: only WARN freeing buffers
>         when DMA is unavailable
>
>         Big NAK to this! This warning is not related in any way to the
>         hw state.
>
>         It's simply illegal to free up memory during suspend.
>
>         Regards,
>
>         Christian.
>
>         ------------------------------------------------------------------------
>
>         *Von:*Xiao, Jack <Jack.Xiao at amd.com>
>         *Gesendet:* Donnerstag, 2. Februar 2023 10:54
>         *An:* amd-gfx at lists.freedesktop.org
>         <amd-gfx at lists.freedesktop.org>; Deucher, Alexander
>         <Alexander.Deucher at amd.com>; Koenig, Christian
>         <Christian.Koenig at amd.com>
>         *Cc:* Xiao, Jack <Jack.Xiao at amd.com>
>         *Betreff:* [PATCH] drm/amdgpu: only WARN freeing buffers when
>         DMA is unavailable
>
>         Reduce waringings, only warn when DMA is unavailable.
>
>         Signed-off-by: Jack Xiao <Jack.Xiao at amd.com>
>         ---
>          drivers/gpu/drm/amd/amdgpu/amdgpu_object.c | 3 ++-
>          1 file changed, 2 insertions(+), 1 deletion(-)
>
>         diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
>         b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
>         index 2d237f3d3a2e..e3e3764ea697 100644
>         --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
>         +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_object.c
>         @@ -422,7 +422,8 @@ void amdgpu_bo_free_kernel(struct
>         amdgpu_bo **bo, u64 *gpu_addr,
>                  if (*bo == NULL)
>                          return;
>
>         - WARN_ON(amdgpu_ttm_adev((*bo)->tbo.bdev)->in_suspend);
>         + WARN_ON(amdgpu_ttm_adev((*bo)->tbo.bdev)->in_suspend &&
>         +
>         !amdgpu_ttm_adev((*bo)->tbo.bdev)->ip_blocks[AMD_IP_BLOCK_TYPE_SDMA].status.hw);
>
>                  if (likely(amdgpu_bo_reserve(*bo, true) == 0)) {
>                          if (cpu_addr)
>         -- 
>         2.37.3
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/amd-gfx/attachments/20230206/7eb45364/attachment-0001.htm>