* RE: [PATCH V2] drm/amdgpu: Destroy psp ring in hw_fini
[not found] ` <1492499085-27761-1-git-send-email-trigger.huang-5C7GfCeVMHo@public.gmane.org>
@ 2017-04-19 11:33 ` Huang, Trigger
2017-04-19 19:18 ` Alex Deucher
2017-04-20 0:53 ` Huang Rui
2 siblings, 0 replies; 7+ messages in thread
From: Huang, Trigger @ 2017-04-19 11:33 UTC (permalink / raw)
To: Huang, Trigger, amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW
Cc: Huang, Ray, Yu, Xiangliang, Liu, Monk
Hi,
Would you please help to review this patch?
Thanks & Best Wishes,
Trigger Huang
-----Original Message-----
From: Trigger Huang [mailto:trigger.huang@amd.com]
Sent: Tuesday, April 18, 2017 3:05 PM
To: amd-gfx@lists.freedesktop.org
Cc: Liu, Monk <Monk.Liu@amd.com>; Yu, Xiangliang <Xiangliang.Yu@amd.com>; Huang, Ray <Ray.Huang@amd.com>; Huang, Trigger <Trigger.Huang@amd.com>
Subject: [PATCH V2] drm/amdgpu: Destroy psp ring in hw_fini
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@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);
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.h
index 28faba5..0301e4e 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.h
@@ -66,6 +66,8 @@ struct psp_context
struct psp_gfx_cmd_resp *cmd);
int (*ring_init)(struct psp_context *psp, enum psp_ring_type ring_type);
int (*ring_create)(struct psp_context *psp, enum psp_ring_type ring_type);
+ int (*ring_destroy)(struct psp_context *psp,
+ enum psp_ring_type ring_type);
int (*cmd_submit)(struct psp_context *psp, struct amdgpu_firmware_info *ucode,
uint64_t cmd_buf_mc_addr, uint64_t fence_mc_addr, int index);
bool (*compare_sram_data)(struct psp_context *psp, @@ -116,6 +118,7 @@ struct amdgpu_psp_funcs { #define psp_prep_cmd_buf(ucode, type) (psp)->prep_cmd_buf((ucode), (type)) #define psp_ring_init(psp, type) (psp)->ring_init((psp), (type)) #define psp_ring_create(psp, type) (psp)->ring_create((psp), (type))
+#define psp_ring_destroy(psp, type) ((psp)->ring_destroy((psp),
+(type)))
#define psp_cmd_submit(psp, ucode, cmd_mc, fence_mc, index) \
(psp)->cmd_submit((psp), (ucode), (cmd_mc), (fence_mc), (index)) #define psp_compare_sram_data(psp, ucode, type) \ diff --git a/drivers/gpu/drm/amd/amdgpu/psp_v3_1.c b/drivers/gpu/drm/amd/amdgpu/psp_v3_1.c
index d351583..e834b55 100644
--- a/drivers/gpu/drm/amd/amdgpu/psp_v3_1.c
+++ b/drivers/gpu/drm/amd/amdgpu/psp_v3_1.c
@@ -321,6 +321,33 @@ int psp_v3_1_ring_create(struct psp_context *psp, enum psp_ring_type ring_type)
return ret;
}
+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);
+ return ret;
+}
+
int psp_v3_1_cmd_submit(struct psp_context *psp,
struct amdgpu_firmware_info *ucode,
uint64_t cmd_buf_mc_addr, uint64_t fence_mc_addr, diff --git a/drivers/gpu/drm/amd/amdgpu/psp_v3_1.h b/drivers/gpu/drm/amd/amdgpu/psp_v3_1.h
index 5230d278..9dcd0b2 100644
--- a/drivers/gpu/drm/amd/amdgpu/psp_v3_1.h
+++ b/drivers/gpu/drm/amd/amdgpu/psp_v3_1.h
@@ -41,6 +41,8 @@ extern int psp_v3_1_ring_init(struct psp_context *psp,
enum psp_ring_type ring_type); extern int psp_v3_1_ring_create(struct psp_context *psp,
enum psp_ring_type ring_type);
+extern int psp_v3_1_ring_destroy(struct psp_context *psp,
+ enum psp_ring_type ring_type);
extern int psp_v3_1_cmd_submit(struct psp_context *psp,
struct amdgpu_firmware_info *ucode,
uint64_t cmd_buf_mc_addr, uint64_t fence_mc_addr,
--
2.7.4
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply related [flat|nested] 7+ messages in thread* Re: [PATCH V2] drm/amdgpu: Destroy psp ring in hw_fini
[not found] ` <1492499085-27761-1-git-send-email-trigger.huang-5C7GfCeVMHo@public.gmane.org>
2017-04-19 11:33 ` Huang, Trigger
@ 2017-04-19 19:18 ` Alex Deucher
[not found] ` <CADnq5_OqZ6U9T2q1KxVCJ1CmEHe6dqXr_mqafqJmtvdWHyiOiw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2017-04-20 0:53 ` Huang Rui
2 siblings, 1 reply; 7+ messages in thread
From: Alex Deucher @ 2017-04-19 19:18 UTC (permalink / raw)
To: Trigger Huang; +Cc: Huang Rui, Yu, Xiangliang, monk.liu, amd-gfx list
On Tue, Apr 18, 2017 at 3:04 AM, Trigger Huang <trigger.huang@amd.com> 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@amd.com>
I'm not really a PSP expert, but the change looks logical.
Acked-by: Alex Deucher <alexander.deucher@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);
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.h
> index 28faba5..0301e4e 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.h
> @@ -66,6 +66,8 @@ struct psp_context
> struct psp_gfx_cmd_resp *cmd);
> int (*ring_init)(struct psp_context *psp, enum psp_ring_type ring_type);
> int (*ring_create)(struct psp_context *psp, enum psp_ring_type ring_type);
> + int (*ring_destroy)(struct psp_context *psp,
> + enum psp_ring_type ring_type);
> int (*cmd_submit)(struct psp_context *psp, struct amdgpu_firmware_info *ucode,
> uint64_t cmd_buf_mc_addr, uint64_t fence_mc_addr, int index);
> bool (*compare_sram_data)(struct psp_context *psp,
> @@ -116,6 +118,7 @@ struct amdgpu_psp_funcs {
> #define psp_prep_cmd_buf(ucode, type) (psp)->prep_cmd_buf((ucode), (type))
> #define psp_ring_init(psp, type) (psp)->ring_init((psp), (type))
> #define psp_ring_create(psp, type) (psp)->ring_create((psp), (type))
> +#define psp_ring_destroy(psp, type) ((psp)->ring_destroy((psp), (type)))
> #define psp_cmd_submit(psp, ucode, cmd_mc, fence_mc, index) \
> (psp)->cmd_submit((psp), (ucode), (cmd_mc), (fence_mc), (index))
> #define psp_compare_sram_data(psp, ucode, type) \
> diff --git a/drivers/gpu/drm/amd/amdgpu/psp_v3_1.c b/drivers/gpu/drm/amd/amdgpu/psp_v3_1.c
> index d351583..e834b55 100644
> --- a/drivers/gpu/drm/amd/amdgpu/psp_v3_1.c
> +++ b/drivers/gpu/drm/amd/amdgpu/psp_v3_1.c
> @@ -321,6 +321,33 @@ int psp_v3_1_ring_create(struct psp_context *psp, enum psp_ring_type ring_type)
> return ret;
> }
>
> +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);
> + return ret;
> +}
> +
> int psp_v3_1_cmd_submit(struct psp_context *psp,
> struct amdgpu_firmware_info *ucode,
> uint64_t cmd_buf_mc_addr, uint64_t fence_mc_addr,
> diff --git a/drivers/gpu/drm/amd/amdgpu/psp_v3_1.h b/drivers/gpu/drm/amd/amdgpu/psp_v3_1.h
> index 5230d278..9dcd0b2 100644
> --- a/drivers/gpu/drm/amd/amdgpu/psp_v3_1.h
> +++ b/drivers/gpu/drm/amd/amdgpu/psp_v3_1.h
> @@ -41,6 +41,8 @@ extern int psp_v3_1_ring_init(struct psp_context *psp,
> enum psp_ring_type ring_type);
> extern int psp_v3_1_ring_create(struct psp_context *psp,
> enum psp_ring_type ring_type);
> +extern int psp_v3_1_ring_destroy(struct psp_context *psp,
> + enum psp_ring_type ring_type);
> extern int psp_v3_1_cmd_submit(struct psp_context *psp,
> struct amdgpu_firmware_info *ucode,
> uint64_t cmd_buf_mc_addr, uint64_t fence_mc_addr,
> --
> 2.7.4
>
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH V2] drm/amdgpu: Destroy psp ring in hw_fini
[not found] ` <1492499085-27761-1-git-send-email-trigger.huang-5C7GfCeVMHo@public.gmane.org>
2017-04-19 11:33 ` Huang, Trigger
2017-04-19 19:18 ` Alex Deucher
@ 2017-04-20 0:53 ` Huang Rui
2017-04-20 1:16 ` Huang Rui
2017-04-20 4:17 ` Huang, Trigger
2 siblings, 2 replies; 7+ messages in thread
From: Huang Rui @ 2017-04-20 0:53 UTC (permalink / raw)
To: Trigger Huang
Cc: Yu, Xiangliang, Liu, Monk, amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW
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@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
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH V2] drm/amdgpu: Destroy psp ring in hw_fini
2017-04-20 0:53 ` Huang Rui
@ 2017-04-20 1:16 ` Huang Rui
2017-04-20 4:17 ` Huang, Trigger
1 sibling, 0 replies; 7+ messages in thread
From: Huang Rui @ 2017-04-20 1:16 UTC (permalink / raw)
To: Trigger Huang
Cc: Yu, Xiangliang, Liu, Monk, amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW
On Thu, Apr 20, 2017 at 08:53:54AM +0800, Huang Rui wrote:
> On Tue, Apr 18, 2017 at 03:04:45PM +0800, Trigger Huang wrote:
> > +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.
>
Trigger, I missed read fw_buf(inited by ucode_init) and rbuf, sorry. Yes,
it need free rbuf here. Please ignore above comments.
But you still need check the load_type before destory ring.
Thanks,
Rui
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply [flat|nested] 7+ messages in thread* RE: [PATCH V2] drm/amdgpu: Destroy psp ring in hw_fini
2017-04-20 0:53 ` Huang Rui
2017-04-20 1:16 ` Huang Rui
@ 2017-04-20 4:17 ` Huang, Trigger
1 sibling, 0 replies; 7+ messages in thread
From: Huang, Trigger @ 2017-04-20 4:17 UTC (permalink / raw)
To: Huang, Ray
Cc: Yu, Xiangliang, amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW, Liu, Monk
Hi Ray,
Thanks for your suggestions. As discussed, free adev->firmware.rbuf is still needed.
I have made a patch V3 according to your suggestions.
Thanks & Best Wishes,
Trigger Huang
-----Original Message-----
From: amd-gfx [mailto:amd-gfx-bounces@lists.freedesktop.org] On Behalf Of Huang Rui
Sent: Thursday, April 20, 2017 8:54 AM
To: Huang, Trigger <Trigger.Huang@amd.com>
Cc: Yu, Xiangliang <Xiangliang.Yu@amd.com>; Liu, Monk <Monk.Liu@amd.com>; amd-gfx@lists.freedesktop.org
Subject: Re: [PATCH V2] drm/amdgpu: Destroy psp ring in hw_fini
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@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
adev->which
inited by amdgpu_ucode_init_bo, and it already teared down via amdgpu_ucode_fini_bo. You cannot free it again.
Thanks,
Rui
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply [flat|nested] 7+ messages in thread