Hey Christian,
Do you have a preference on which solution we go with? I'm inclined
to go with my patch just because it's more self-contained and easier
to deal with for stable, but I could be convinced otherwise.
Alex
On Wed, Nov 16, 2022 at 11:38 AM Alex Deucher <alexdeucher@xxxxxxxxx> wrote:
>
> I was thinking something like this would be more straightforward.
>
> Alex
>
> On Wed, Nov 16, 2022 at 11:01 AM Christian König
> <ckoenig.leichtzumerken@xxxxxxxxx> wrote:
> >
> > That the PSP code tries to free the memory during suspend is quite
> > broken and leads to problems during resume.
> >
> > Signed-off-by: Christian König <christian.koenig@xxxxxxx>
> > ---
> > drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c | 30 ++++++++++---------------
> > 1 file changed, 12 insertions(+), 18 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
> > index 0a8c30475dda..470cd660c450 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
> > @@ -511,11 +511,10 @@ static int psp_sw_fini(void *handle)
> > kfree(cmd);
> > cmd = NULL;
> >
> > - if (psp->km_ring.ring_mem)
> > - amdgpu_bo_free_kernel(&adev->firmware.rbuf,
> > - &psp->km_ring.ring_mem_mc_addr,
> > - (void **)&psp->km_ring.ring_mem);
> > -
> > + psp_free_shared_bufs(psp);
> > + amdgpu_bo_free_kernel(&adev->firmware.rbuf,
> > + &psp->km_ring.ring_mem_mc_addr,
> > + (void **)&psp->km_ring.ring_mem);
> > amdgpu_bo_free_kernel(&psp->fw_pri_bo,
> > &psp->fw_pri_mc_addr, &psp->fw_pri_buf);
> > amdgpu_bo_free_kernel(&psp->fence_buf_bo,
> > @@ -2635,8 +2634,6 @@ static int psp_hw_fini(void *handle)
> >
> > psp_ring_destroy(psp, PSP_RING_TYPE__KM);
> >
> > - psp_free_shared_bufs(psp);
> > -
> > return 0;
> > }
> >
> > @@ -2651,7 +2648,7 @@ static int psp_suspend(void *handle)
> > ret = psp_xgmi_terminate(psp);
> > if (ret) {
> > DRM_ERROR("Failed to terminate xgmi ta\n");
> > - goto out;
> > + return ret;
> > }
> > }
> >
> > @@ -2659,40 +2656,40 @@ static int psp_suspend(void *handle)
> > ret = psp_ras_terminate(psp);
> > if (ret) {
> > DRM_ERROR("Failed to terminate ras ta\n");
> > - goto out;
> > + return ret;
> > }
> > ret = psp_hdcp_terminate(psp);
> > if (ret) {
> > DRM_ERROR("Failed to terminate hdcp ta\n");
> > - goto out;
> > + return ret;
> > }
> > ret = psp_dtm_terminate(psp);
> > if (ret) {
> > DRM_ERROR("Failed to terminate dtm ta\n");
> > - goto out;
> > + return ret;
> > }
> > ret = psp_rap_terminate(psp);
> > if (ret) {
> > DRM_ERROR("Failed to terminate rap ta\n");
> > - goto out;
> > + return ret;
> > }
> > ret = psp_securedisplay_terminate(psp);
> > if (ret) {
> > DRM_ERROR("Failed to terminate securedisplay ta\n");
> > - goto out;
> > + return ret;
> > }
> > }
> >
> > ret = psp_asd_terminate(psp);
> > if (ret) {
> > DRM_ERROR("Failed to terminate asd\n");
> > - goto out;
> > + return ret;
> > }
> >
> > ret = psp_tmr_terminate(psp);
> > if (ret) {
> > DRM_ERROR("Failed to terminate tmr\n");
> > - goto out;
> > + return ret;
> > }
> >
> > ret = psp_ring_stop(psp, PSP_RING_TYPE__KM);
> > @@ -2700,9 +2697,6 @@ static int psp_suspend(void *handle)
> > DRM_ERROR("PSP ring stop failed\n");
> > }
> >
> > -out:
> > - psp_free_shared_bufs(psp);
> > -
> > return ret;
> > }
> >
> > --
> > 2.34.1
> >
