I checked the code, cpu pointer indeed only used for checking. Thanks for your suggestions!
________________________________ From: Koenig, Christian <christian.koe...@amd.com> Sent: Wednesday, August 28, 2019 19:08 To: Yin, Tianci (Rico) <tianci....@amd.com> Cc: amd-gfx@lists.freedesktop.org <amd-gfx@lists.freedesktop.org>; Xu, Feifei <feifei...@amd.com>; Ma, Le <le...@amd.com>; Xiao, Jack <jack.x...@amd.com>; Zhang, Hawking <hawking.zh...@amd.com> Subject: Re: [PATCH] drm/amdgpu/psp: keep TMR in visible vram region for SRIOV That's irrelevant in this case. CPU mapping's are destroyed automatically when the BO is freed. Giving the CPU pointer to the free function is only for extra checking that we can't access it anymore. Christian. Am 28.08.2019 13:05 schrieb "Yin, Tianci (Rico)" <tianci....@amd.com>: Might be better to use a local variable for the CPU pointer instead of duplicating the calling parameters. Local variable can't be used in cleaner. ________________________________ From: Christian König <ckoenig.leichtzumer...@gmail.com> Sent: Wednesday, August 28, 2019 17:57 To: Yin, Tianci (Rico) <tianci....@amd.com>; amd-gfx@lists.freedesktop.org <amd-gfx@lists.freedesktop.org> Cc: Xu, Feifei <feifei...@amd.com>; Ma, Le <le...@amd.com>; Xiao, Jack <jack.x...@amd.com>; Zhang, Hawking <hawking.zh...@amd.com> Subject: Re: [PATCH] drm/amdgpu/psp: keep TMR in visible vram region for SRIOV Am 28.08.19 um 11:25 schrieb Tianci Yin: > From: "Tianci.Yin" <tianci....@amd.com> > > Fix compute ring test failure in sriov scenario. > > Signed-off-by: Tianci.Yin <tianci....@amd.com> > --- > drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c | 14 ++++++++++---- > drivers/gpu/drm/amd/amdgpu/amdgpu_psp.h | 1 + > 2 files changed, 11 insertions(+), 4 deletions(-) > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c > b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c > index 9f7cc5b..92c68c9 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c > @@ -261,9 +261,15 @@ static int psp_tmr_init(struct psp_context *psp) > } > } > > - ret = amdgpu_bo_create_kernel(psp->adev, tmr_size, PSP_TMR_SIZE, > - AMDGPU_GEM_DOMAIN_VRAM, > - &psp->tmr_bo, &psp->tmr_mc_addr, NULL); > + psp->tmr_buf = NULL; > + if (!amdgpu_sriov_vf(psp->adev)) > + ret = amdgpu_bo_create_kernel(psp->adev, tmr_size, PSP_TMR_SIZE, > + AMDGPU_GEM_DOMAIN_VRAM, > + &psp->tmr_bo, &psp->tmr_mc_addr, > NULL); > + else > + ret = amdgpu_bo_create_kernel(psp->adev, tmr_size, PSP_TMR_SIZE, > + AMDGPU_GEM_DOMAIN_VRAM, > + &psp->tmr_bo, &psp->tmr_mc_addr, > &psp->tmr_buf); Might be better to use a local variable for the CPU pointer instead of duplicating the calling parameters. BTW: How did you solved the stolen_vga_memory problem I pointed out in the original patch set? Christian. > > return ret; > } > @@ -1216,7 +1222,7 @@ static int psp_hw_fini(void *handle) > > psp_ring_destroy(psp, PSP_RING_TYPE__KM); > > - amdgpu_bo_free_kernel(&psp->tmr_bo, &psp->tmr_mc_addr, NULL); > + amdgpu_bo_free_kernel(&psp->tmr_bo, &psp->tmr_mc_addr, &psp->tmr_buf); > 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, > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.h > b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.h > index bc0947f..b73d4aa 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.h > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.h > @@ -171,6 +171,7 @@ struct psp_context > /* tmr buffer */ > struct amdgpu_bo *tmr_bo; > uint64_t tmr_mc_addr; > + void *tmr_buf; > > /* asd firmware and buffer */ > const struct firmware *asd_fw;
_______________________________________________ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx
