On Tue, Apr 18, 2017 at 03:04:45PM +0800, Trigger Huang wrote: > Fix issue that PSP initialization will fail if reload amdgpu module. > That's because the PSP ring must be destroyed to be ready for the > next time PSP initialization. > > Changes in v2: > - Move psp_ring_destroy before all BOs free according to > Ray Huang's suggestion. > > Signed-off-by: Trigger Huang <trigger.huang at amd.com> > --- > drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c | 3 +++ > drivers/gpu/drm/amd/amdgpu/amdgpu_psp.h | 3 +++ > drivers/gpu/drm/amd/amdgpu/psp_v3_1.c | 27 +++++++++++++++++++++++++++ > drivers/gpu/drm/amd/amdgpu/psp_v3_1.h | 2 ++ > 4 files changed, 35 insertions(+) > > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c b/drivers/gpu/drm/amd/ > amdgpu/amdgpu_psp.c > index 19180aa..73ddf4b 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c > @@ -56,6 +56,7 @@ static int psp_sw_init(void *handle) > psp->prep_cmd_buf = psp_v3_1_prep_cmd_buf; > psp->ring_init = psp_v3_1_ring_init; > psp->ring_create = psp_v3_1_ring_create; > + psp->ring_destroy = psp_v3_1_ring_destroy; > psp->cmd_submit = psp_v3_1_cmd_submit; > psp->compare_sram_data = psp_v3_1_compare_sram_data; > psp->smu_reload_quirk = psp_v3_1_smu_reload_quirk; > @@ -411,6 +412,8 @@ static int psp_hw_fini(void *handle) > struct amdgpu_device *adev = (struct amdgpu_device *)handle; > struct psp_context *psp = &adev->psp; > > + psp_ring_destroy(psp, PSP_RING_TYPE__KM); > + > if (adev->firmware.load_type == AMDGPU_FW_LOAD_PSP) > amdgpu_ucode_fini_bo(adev); If adev->firmware.load_type is AMDGPU_FW_LOAD_DIRECT, we should not call psp_ring_destroy, right? I already told you in last mail: if (adev->firmware.load_type != AMDGPU_FW_LOAD_PSP) return 0; amdgpu_ucode_fini_bo(adev); psp_ring_destroy(psp, PSP_RING_TYPE__KM); ... > > +int psp_v3_1_ring_destroy(struct psp_context *psp, enum psp_ring_type > ring_type) > +{ > + int ret = 0; > + struct psp_ring *ring; > + unsigned int psp_ring_reg = 0; > + struct amdgpu_device *adev = psp->adev; > + > + ring = &psp->km_ring; > + > + /* Write the ring destroy command to C2PMSG_64 */ > + psp_ring_reg = 3 << 16; > + WREG32(SOC15_REG_OFFSET(MP0, 0, mmMP0_SMN_C2PMSG_64), psp_ring_reg); > + > + /* there might be handshake issue with hardware which needs delay */ > + mdelay(20); > + > + /* Wait for response flag (bit 31) in C2PMSG_64 */ > + ret = psp_wait_for(psp, SOC15_REG_OFFSET(MP0, 0, mmMP0_SMN_C2PMSG_64), > + 0x80000000, 0x8000FFFF, false); > + > + if (ring->ring_mem) > + amdgpu_bo_free_kernel(&adev->firmware.rbuf, > + &ring->ring_mem_mc_addr, > + (void **)&ring->ring_mem); adev->firmware.rbuf is entired memory space for storing firmware data which inited by amdgpu_ucode_init_bo, and it already teared down via amdgpu_ucode_fini_bo. You cannot free it again. Thanks, Rui