Re: [PATCH] drm/amdgpu: read reg in each iterate of psp_wait_for loop

Re: [PATCH] drm/amdgpu: read reg in each iterate of psp_wait_for loop

[Zhang, Jerry (Junwei)]

+ stable@vger.kernel.org

On 07/17/2017 03:57 PM, Huang Rui wrote:
> On Mon, Jul 17, 2017 at 03:52:10PM +0800, Huang Rui wrote:
>> On Fri, Jul 14, 2017 at 06:20:17PM +0800, Junwei Zhang wrote:
>>> Signed-off-by: Junwei Zhang <Jerry.Zhang@amd.com>
>>> ---
>>>   drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c | 3 +--
>>>   1 file changed, 1 insertion(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
>>> index ba743d4..71ce3ee 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
>>> @@ -95,9 +95,8 @@ int psp_wait_for(struct psp_context *psp, uint32_t reg_index,
>>>   	int i;
>>>   	struct amdgpu_device *adev = psp->adev;
>>>
>>> -	val = RREG32(reg_index);
>>> -
>>>   	for (i = 0; i < adev->usec_timeout; i++) {
>>> +		val = RREG32(reg_index);
>>>   		if (check_changed) {
>>>   			if (val != reg_val)
>>>   				return 0;
>>
>> Nice catch. I remembered Ken also mentioned it before. This should fix the
>> issue I encountered before during bring-up. Can you open this handshake in
>> psp_v3_1_bootloader_load_sos and double check if this handshake is workable
>> with this fix. If yes, please add it back.

Yes, it could fix this.
Later I will enable it.

Jerry

>>
>> #if 0
>>          ret = psp_wait_for(psp, SOC15_REG_OFFSET(MP0, 0, mmMP0_SMN_C2PMSG_81),
>>                             RREG32_SOC15(MP0, 0, mmMP0_SMN_C2PMSG_81),
>>                             0, true);
>> #endif
>>
>> Acked-by: Huang Rui <ray.huang@amd.com>
>
> BTW: please add Cc: <stable@vger.kernel.org> in your patch, it need be
> backported to stable tree.
>
> Thanks,
> Rui
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
>

Re: [PATCH] drm/amdgpu: read reg in each iterate of psp_wait_for loop

[Greg KH]

On Mon, Jul 17, 2017 at 04:56:26PM +0800, Zhang, Jerry (Junwei) wrote:
> + stable@vger.kernel.org

<formletter>

This is not the correct way to submit patches for inclusion in the
stable kernel tree.  Please read:
    https://www.kernel.org/doc/html/latest/process/stable-kernel-rules.html
for how to do this properly.

</formletter>

Re: [PATCH] drm/amdgpu: read reg in each iterate of psp_wait_for loop

[Huang Rui]

On Mon, Jul 17, 2017 at 06:57:41PM +0800, Greg KH wrote:
> On Mon, Jul 17, 2017 at 04:56:26PM +0800, Zhang, Jerry (Junwei) wrote:
> > + stable@vger.kernel.org
> 
> <formletter>
> 
> This is not the correct way to submit patches for inclusion in the
> stable kernel tree.  Please read:
>     https://www.kernel.org/doc/html/latest/process/stable-kernel-rules.html
> for how to do this properly.
> 
> </formletter>

Thanks, Greg. :-)

>> BTW: please add Cc: <stable@vger.kernel.org> in your patch, it need be
>> backported to stable tree.

Jerry, I might not describe it clearly. We need follow the rule that Greg
provided. Actually, I meant to add Cc in your commit message like below,
then sent it out:

8<----------

Subject: [PATCH] drm/amdgpu: read reg in each iterate of psp_wait_for loop

This fixes the SOS loading failure of psp v3.1.

Signed-off-by: Junwei Zhang <Jerry.Zhang@amd.com>
Cc: stable@vger.kernel.org

8<----------

And you'd better squeeze the two patches as one (actually it's only one
fix) to make backporting more smooth.

Thanks,
Rui

Re: [PATCH] drm/amdgpu: read reg in each iterate of psp_wait_for loop

[Zhang, Jerry (Junwei)]

On 07/17/2017 07:45 PM, Huang Rui wrote:
> On Mon, Jul 17, 2017 at 06:57:41PM +0800, Greg KH wrote:
>> On Mon, Jul 17, 2017 at 04:56:26PM +0800, Zhang, Jerry (Junwei) wrote:
>> > + stable@vger.kernel.org
>>
>> <formletter>
>>
>> This is not the correct way to submit patches for inclusion in the
>> stable kernel tree.  Please read:
>>https://www.kernel.org/doc/html/latest/process/stable-kernel-rules.html
>> for how to do this properly.
>>
>> </formletter>
>
> Thanks, Greg. :-)

Thanks Greg to reminder that

>
>>> BTW: please add Cc: <stable@vger.kernel.org> in your patch, it need be
>>> backported to stable tree.
>
> Jerry, I might not describe it clearly. We need follow the rule that Greg
> provided. Actually, I meant to add Cc in your commit message like below,
> then sent it out:

Thanks to explain in detail.
I will prepare it them as one patch again.

Jerry

>
> 8<----------
>
> Subject: [PATCH] drm/amdgpu: read reg in each iterate of psp_wait_for loop
>
> This fixes the SOS loading failure of psp v3.1.
>
> Signed-off-by: Junwei Zhang <Jerry.Zhang@amd.com>
> Cc: stable@vger.kernel.org
>
> 8<----------
>
> And you'd better squeeze the two patches as one (actually it's only one
> fix) to make backporting more smooth.
>
> Thanks,
> Rui

[PATCH] drm/amdgpu: read reg in each iterate of psp_wait_for loop

[Junwei Zhang]

From: "Zhang, Jerry" <Jerry.Zhang@amd.com>

v2: fixes the SOS loading failure for PSP v3.1

Signed-off-by: Junwei Zhang <Jerry.Zhang@amd.com>
Cc: stable@vger.kernel.org
Acked-by: Alex Deucher <alexander.deucher@amd.com> (v1)
Acked-by: Huang Rui <ray.huang@amd.com> (v1)
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c | 3 +--
 drivers/gpu/drm/amd/amdgpu/psp_v3_1.c   | 2 --
 2 files changed, 1 insertion(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
index c919579..644941d 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
@@ -98,9 +98,8 @@ int psp_wait_for(struct psp_context *psp, uint32_t reg_index,
 	int i;
 	struct amdgpu_device *adev = psp->adev;
 
-	val = RREG32(reg_index);
-
 	for (i = 0; i < adev->usec_timeout; i++) {
+		val = RREG32(reg_index);
 		if (check_changed) {
 			if (val != reg_val)
 				return 0;
diff --git a/drivers/gpu/drm/amd/amdgpu/psp_v3_1.c b/drivers/gpu/drm/amd/amdgpu/psp_v3_1.c
index 2718e86..23106e3 100644
--- a/drivers/gpu/drm/amd/amdgpu/psp_v3_1.c
+++ b/drivers/gpu/drm/amd/amdgpu/psp_v3_1.c
@@ -237,11 +237,9 @@ int psp_v3_1_bootloader_load_sos(struct psp_context *psp)
 
 	/* there might be handshake issue with hardware which needs delay */
 	mdelay(20);
-#if 0
 	ret = psp_wait_for(psp, SOC15_REG_OFFSET(MP0, 0, mmMP0_SMN_C2PMSG_81),
 			   RREG32_SOC15(MP0, 0, mmMP0_SMN_C2PMSG_81),
 			   0, true);
-#endif
 
 	return ret;
 }
-- 
1.9.1

RE: [PATCH] drm/amdgpu: read reg in each iterate of psp_wait_for loop

[Deucher, Alexander]

> -----Original Message-----
> From: Junwei Zhang [mailto:Jerry.Zhang@amd.com]
> Sent: Monday, July 17, 2017 10:54 PM
> To: amd-gfx@lists.freedesktop.org
> Cc: Deucher, Alexander; Huang, Ray; gregkh@linuxfoundation.org; Zhang,
> Jerry; stable@vger.kernel.org
> Subject: [PATCH] drm/amdgpu: read reg in each iterate of psp_wait_for loop
> 
> From: "Zhang, Jerry" <Jerry.Zhang@amd.com>
> 
> v2: fixes the SOS loading failure for PSP v3.1
> 
> Signed-off-by: Junwei Zhang <Jerry.Zhang@amd.com>
> Cc: stable@vger.kernel.org
> Acked-by: Alex Deucher <alexander.deucher@amd.com> (v1)
> Acked-by: Huang Rui <ray.huang@amd.com> (v1)

Reviewed-by: Alex Deucher <alexander.deucher@amd.com>

> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c | 3 +--
>  drivers/gpu/drm/amd/amdgpu/psp_v3_1.c   | 2 --
>  2 files changed, 1 insertion(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
> index c919579..644941d 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
> @@ -98,9 +98,8 @@ int psp_wait_for(struct psp_context *psp, uint32_t
> reg_index,
>  	int i;
>  	struct amdgpu_device *adev = psp->adev;
> 
> -	val = RREG32(reg_index);
> -
>  	for (i = 0; i < adev->usec_timeout; i++) {
> +		val = RREG32(reg_index);
>  		if (check_changed) {
>  			if (val != reg_val)
>  				return 0;
> diff --git a/drivers/gpu/drm/amd/amdgpu/psp_v3_1.c
> b/drivers/gpu/drm/amd/amdgpu/psp_v3_1.c
> index 2718e86..23106e3 100644
> --- a/drivers/gpu/drm/amd/amdgpu/psp_v3_1.c
> +++ b/drivers/gpu/drm/amd/amdgpu/psp_v3_1.c
> @@ -237,11 +237,9 @@ int psp_v3_1_bootloader_load_sos(struct
> psp_context *psp)
> 
>  	/* there might be handshake issue with hardware which needs delay
> */
>  	mdelay(20);
> -#if 0
>  	ret = psp_wait_for(psp, SOC15_REG_OFFSET(MP0, 0,
> mmMP0_SMN_C2PMSG_81),
>  			   RREG32_SOC15(MP0, 0,
> mmMP0_SMN_C2PMSG_81),
>  			   0, true);
> -#endif
> 
>  	return ret;
>  }
> --
> 1.9.1