All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] drm/amdgpu: Release memory when psp sw_init is failed
@ 2022-04-11 12:42 Ma Jun
  2022-04-11 12:50 ` Paul Menzel
  2022-04-11 13:10 ` Wang, Yang(Kevin)
  0 siblings, 2 replies; 3+ messages in thread
From: Ma Jun @ 2022-04-11 12:42 UTC (permalink / raw)
  To: amd-gfx; +Cc: alexander.deucher, christian.koenig, Hawking.Zhang

Release the memory (psp->cmd) when psp initialization is
failed in psp_sw_init

Signed-off-by: Ma Jun <Jun.Ma2@amd.com>
Change-Id: I2f88b5919142d55dd7d3820a7da94823286db235
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c | 15 +++++++++------
 1 file changed, 9 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
index a6acec1a6155..1227dc014c80 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
@@ -305,9 +305,10 @@ static int psp_sw_init(void *handle)
 		ret = psp_init_sriov_microcode(psp);
 	else
 		ret = psp_init_microcode(psp);
+
 	if (ret) {
 		DRM_ERROR("Failed to load psp firmware!\n");
-		return ret;
+		goto failure;
 	}
 
 	adev->psp.xgmi_context.supports_extended_data =
@@ -339,25 +340,27 @@ static int psp_sw_init(void *handle)
 		ret = psp_memory_training_init(psp);
 		if (ret) {
 			DRM_ERROR("Failed to initialize memory training!\n");
-			return ret;
+			goto failure;
 		}
 
 		ret = psp_mem_training(psp, PSP_MEM_TRAIN_COLD_BOOT);
 		if (ret) {
 			DRM_ERROR("Failed to process memory training!\n");
-			return ret;
+			goto failure;
 		}
 	}
 
 	if (adev->ip_versions[MP0_HWIP][0] == IP_VERSION(11, 0, 0) ||
 	    adev->ip_versions[MP0_HWIP][0] == IP_VERSION(11, 0, 7)) {
 		ret= psp_sysfs_init(adev);
-		if (ret) {
-			return ret;
-		}
+		if (ret)
+			goto failure;
 	}
 
 	return 0;
+failure:
+	kfree(psp->cmd);
+	return ret;
 }
 
 static int psp_sw_fini(void *handle)
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH] drm/amdgpu: Release memory when psp sw_init is failed
  2022-04-11 12:42 [PATCH] drm/amdgpu: Release memory when psp sw_init is failed Ma Jun
@ 2022-04-11 12:50 ` Paul Menzel
  2022-04-11 13:10 ` Wang, Yang(Kevin)
  1 sibling, 0 replies; 3+ messages in thread
From: Paul Menzel @ 2022-04-11 12:50 UTC (permalink / raw)
  To: Ma Jun; +Cc: alexander.deucher, christian.koenig, amd-gfx, Hawking.Zhang

Dear Ma,


Thank you for your patch.

Am 11.04.22 um 14:42 schrieb Ma Jun:
> Release the memory (psp->cmd) when psp initialization is
> failed in psp_sw_init

s/is failed/fails/

Period/full stop at the end of sentences, and it still fits into 75 
characters per line.

Next time you could also add one simple statement like:

Add new label `failure` to jump to in case of the failure.

> Signed-off-by: Ma Jun <Jun.Ma2@amd.com>
> Change-Id: I2f88b5919142d55dd7d3820a7da94823286db235

Without the URL of the Gerrit instance, the Change-Id is not of much use.

> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c | 15 +++++++++------
>   1 file changed, 9 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
> index a6acec1a6155..1227dc014c80 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
> @@ -305,9 +305,10 @@ static int psp_sw_init(void *handle)
>   		ret = psp_init_sriov_microcode(psp);
>   	else
>   		ret = psp_init_microcode(psp);
> +

Unrelated.

>   	if (ret) {
>   		DRM_ERROR("Failed to load psp firmware!\n");
> -		return ret;
> +		goto failure;
>   	}
>   
>   	adev->psp.xgmi_context.supports_extended_data =
> @@ -339,25 +340,27 @@ static int psp_sw_init(void *handle)
>   		ret = psp_memory_training_init(psp);
>   		if (ret) {
>   			DRM_ERROR("Failed to initialize memory training!\n");
> -			return ret;
> +			goto failure;
>   		}
>   
>   		ret = psp_mem_training(psp, PSP_MEM_TRAIN_COLD_BOOT);
>   		if (ret) {
>   			DRM_ERROR("Failed to process memory training!\n");
> -			return ret;
> +			goto failure;
>   		}
>   	}
>   
>   	if (adev->ip_versions[MP0_HWIP][0] == IP_VERSION(11, 0, 0) ||
>   	    adev->ip_versions[MP0_HWIP][0] == IP_VERSION(11, 0, 7)) {
>   		ret= psp_sysfs_init(adev);
> -		if (ret) {
> -			return ret;
> -		}
> +		if (ret)
> +			goto failure;
>   	}
>   
>   	return 0;
> +failure:
> +	kfree(psp->cmd);
> +	return ret;
>   }
>   
>   static int psp_sw_fini(void *handle)

Acked-by: Paul Menzel <pmenzel@molgen.mpg.de>


Kind regards,

Paul

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] drm/amdgpu: Release memory when psp sw_init is failed
  2022-04-11 12:42 [PATCH] drm/amdgpu: Release memory when psp sw_init is failed Ma Jun
  2022-04-11 12:50 ` Paul Menzel
@ 2022-04-11 13:10 ` Wang, Yang(Kevin)
  1 sibling, 0 replies; 3+ messages in thread
From: Wang, Yang(Kevin) @ 2022-04-11 13:10 UTC (permalink / raw)
  To: Ma, Jun, amd-gfx; +Cc: Deucher, Alexander, Koenig, Christian, Zhang, Hawking

[-- Attachment #1: Type: text/plain, Size: 2753 bytes --]

[AMD Official Use Only]


________________________________
From: amd-gfx <amd-gfx-bounces@lists.freedesktop.org> on behalf of Ma Jun <Jun.Ma2@amd.com>
Sent: Monday, April 11, 2022 8:42 PM
To: amd-gfx@lists.freedesktop.org <amd-gfx@lists.freedesktop.org>
Cc: Deucher, Alexander <Alexander.Deucher@amd.com>; Koenig, Christian <Christian.Koenig@amd.com>; Zhang, Hawking <Hawking.Zhang@amd.com>
Subject: [PATCH] drm/amdgpu: Release memory when psp sw_init is failed

Release the memory (psp->cmd) when psp initialization is
failed in psp_sw_init

Signed-off-by: Ma Jun <Jun.Ma2@amd.com>
Change-Id: I2f88b5919142d55dd7d3820a7da94823286db235
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c | 15 +++++++++------
 1 file changed, 9 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
index a6acec1a6155..1227dc014c80 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_psp.c
@@ -305,9 +305,10 @@ static int psp_sw_init(void *handle)
                 ret = psp_init_sriov_microcode(psp);
         else
                 ret = psp_init_microcode(psp);
+
         if (ret) {
                 DRM_ERROR("Failed to load psp firmware!\n");
-               return ret;
+               goto failure;
         }

         adev->psp.xgmi_context.supports_extended_data =
@@ -339,25 +340,27 @@ static int psp_sw_init(void *handle)
                 ret = psp_memory_training_init(psp);
                 if (ret) {
                         DRM_ERROR("Failed to initialize memory training!\n");
-                       return ret;
+                       goto failure;
                 }

                 ret = psp_mem_training(psp, PSP_MEM_TRAIN_COLD_BOOT);
                 if (ret) {
                         DRM_ERROR("Failed to process memory training!\n");
-                       return ret;
+                       goto failure;
                 }
         }

         if (adev->ip_versions[MP0_HWIP][0] == IP_VERSION(11, 0, 0) ||
             adev->ip_versions[MP0_HWIP][0] == IP_VERSION(11, 0, 7)) {
                 ret= psp_sysfs_init(adev);
-               if (ret) {
-                       return ret;
-               }
+               if (ret)
+                       goto failure;
         }

         return 0;
+failure:
+       kfree(psp->cmd);
+       return ret;
[kevin]:

  1.  do you forget to set 'psp->cmd' to NULL?
  2.  according to my understanding, the buf will not be used in the function (in psp sw init stage), can you move this to end of function to mke patch logic is clearly.

Best Regards,
Kevin
 }

 static int psp_sw_fini(void *handle)
--
2.25.1


[-- Attachment #2: Type: text/html, Size: 6681 bytes --]

^ permalink raw reply related	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2022-04-11 13:10 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-04-11 12:42 [PATCH] drm/amdgpu: Release memory when psp sw_init is failed Ma Jun
2022-04-11 12:50 ` Paul Menzel
2022-04-11 13:10 ` Wang, Yang(Kevin)

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.