amd-gfx.lists.freedesktop.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 1/2] drm/amdgpu: sdma v5_2 ring bo mem leak
@ 2020-06-18  7:53 Wenhui Sheng
  2020-06-18  7:53 ` [PATCH 2/2] drm/amdgpu: add fw release for sdma v5_0 Wenhui Sheng
  0 siblings, 1 reply; 9+ messages in thread
From: Wenhui Sheng @ 2020-06-18  7:53 UTC (permalink / raw)
  To: amd-gfx; +Cc: Wenhui Sheng

invoke amdgpu_ring_fini for each sdma instance

Signed-off-by: Wenhui Sheng <Wenhui.Sheng@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/sdma_v5_2.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/sdma_v5_2.c b/drivers/gpu/drm/amd/amdgpu/sdma_v5_2.c
index b523091dcde8..74ac929dd217 100644
--- a/drivers/gpu/drm/amd/amdgpu/sdma_v5_2.c
+++ b/drivers/gpu/drm/amd/amdgpu/sdma_v5_2.c
@@ -1244,6 +1244,10 @@ static int sdma_v5_2_sw_init(void *handle)
 static int sdma_v5_2_sw_fini(void *handle)
 {
 	struct amdgpu_device *adev = (struct amdgpu_device *)handle;
+	int i;
+
+	for (i = 0; i < adev->sdma.num_instances; i++)
+		amdgpu_ring_fini(&adev->sdma.instance[i].ring);
 
 	sdma_v5_2_destroy_inst_ctx(adev);
 
-- 
2.17.1

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply related	[flat|nested] 9+ messages in thread
* [PATCH 2/2] drm/amdgpu: add fw release for sdma v5_0
  2020-06-18  7:53 [PATCH 1/2] drm/amdgpu: sdma v5_2 ring bo mem leak Wenhui Sheng
@ 2020-06-18  7:53 ` Wenhui Sheng
  2020-06-18  8:25   ` Wang, Kevin(Yang)
                     ` (2 more replies)
  0 siblings, 3 replies; 9+ messages in thread
From: Wenhui Sheng @ 2020-06-18  7:53 UTC (permalink / raw)
  To: amd-gfx; +Cc: Wenhui Sheng

sdma v5_0 fw isn't released when module exit

Signed-off-by: Wenhui Sheng <Wenhui.Sheng@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/sdma_v5_0.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/sdma_v5_0.c b/drivers/gpu/drm/amd/amdgpu/sdma_v5_0.c
index 58d2a80af450..6751ad69ed90 100644
--- a/drivers/gpu/drm/amd/amdgpu/sdma_v5_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/sdma_v5_0.c
@@ -1299,8 +1299,12 @@ static int sdma_v5_0_sw_fini(void *handle)
 	struct amdgpu_device *adev = (struct amdgpu_device *)handle;
 	int i;
 
-	for (i = 0; i < adev->sdma.num_instances; i++)
+	for (i = 0; i < adev->sdma.num_instances; i++) {
+		if (adev->sdma.instance[i].fw != NULL)
+			release_firmware(adev->sdma.instance[i].fw);
+
 		amdgpu_ring_fini(&adev->sdma.instance[i].ring);
+	}
 
 	return 0;
 }
-- 
2.17.1

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply related	[flat|nested] 9+ messages in thread
* Re: [PATCH 2/2] drm/amdgpu: add fw release for sdma v5_0
  2020-06-18  7:53 ` [PATCH 2/2] drm/amdgpu: add fw release for sdma v5_0 Wenhui Sheng
@ 2020-06-18  8:25   ` Wang, Kevin(Yang)
  2020-06-18  8:31     ` Sheng, Wenhui
  2020-06-19  8:29   ` Zhang, Hawking
  2020-06-22 18:20   ` Christian König
  2 siblings, 1 reply; 9+ messages in thread
From: Wang, Kevin(Yang) @ 2020-06-18  8:25 UTC (permalink / raw)
  To: Sheng, Wenhui, amd-gfx


[-- Attachment #1.1: Type: text/plain, Size: 1933 bytes --]

[AMD Official Use Only - Internal Distribution Only]



________________________________
From: amd-gfx <amd-gfx-bounces@lists.freedesktop.org> on behalf of Wenhui Sheng <Wenhui.Sheng@amd.com>
Sent: Thursday, June 18, 2020 3:53 PM
To: amd-gfx@lists.freedesktop.org <amd-gfx@lists.freedesktop.org>
Cc: Sheng, Wenhui <Wenhui.Sheng@amd.com>
Subject: [PATCH 2/2] drm/amdgpu: add fw release for sdma v5_0

sdma v5_0 fw isn't released when module exit

Signed-off-by: Wenhui Sheng <Wenhui.Sheng@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/sdma_v5_0.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/sdma_v5_0.c b/drivers/gpu/drm/amd/amdgpu/sdma_v5_0.c
index 58d2a80af450..6751ad69ed90 100644
--- a/drivers/gpu/drm/amd/amdgpu/sdma_v5_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/sdma_v5_0.c
@@ -1299,8 +1299,12 @@ static int sdma_v5_0_sw_fini(void *handle)
         struct amdgpu_device *adev = (struct amdgpu_device *)handle;
         int i;

-       for (i = 0; i < adev->sdma.num_instances; i++)
+       for (i = 0; i < adev->sdma.num_instances; i++) {
+               if (adev->sdma.instance[i].fw != NULL)
+                       release_firmware(adev->sdma.instance[i].fw);
[kevin]:
the kernel api "release_firmware()" will check argument (is NULL pointer).
i think the patch don't need to double check it.
+
                 amdgpu_ring_fini(&adev->sdma.instance[i].ring);
+       }

         return 0;
 }
--
2.17.1

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.freedesktop.org%2Fmailman%2Flistinfo%2Famd-gfx&amp;data=02%7C01%7CKevin1.Wang%40amd.com%7Cd0b8b72e0be443a7343d08d8135cb093%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637280636699912801&amp;sdata=HrKVAmRO0HKFZBRG6oWq3thNlGBd0T9bMramDU1ijys%3D&amp;reserved=0

[-- Attachment #1.2: Type: text/html, Size: 4427 bytes --]

[-- Attachment #2: Type: text/plain, Size: 154 bytes --]

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply related	[flat|nested] 9+ messages in thread
* RE: [PATCH 2/2] drm/amdgpu: add fw release for sdma v5_0
  2020-06-18  8:25   ` Wang, Kevin(Yang)
@ 2020-06-18  8:31     ` Sheng, Wenhui
  2020-06-18  8:47       ` Wang, Kevin(Yang)
  0 siblings, 1 reply; 9+ messages in thread
From: Sheng, Wenhui @ 2020-06-18  8:31 UTC (permalink / raw)
  To: Wang, Kevin(Yang), amd-gfx


[-- Attachment #1.1: Type: text/plain, Size: 2724 bytes --]

[AMD Official Use Only - Internal Distribution Only]


[AMD Official Use Only - Internal Distribution Only]

Although we know that release_firmware has null check, but the code is not maintained by ourselves, so I think it's much more safe to add null  check before invoke release_firmware.


Brs
Wenhui
From: Wang, Kevin(Yang) <Kevin1.Wang@amd.com>
Sent: Thursday, June 18, 2020 4:25 PM
To: Sheng, Wenhui <Wenhui.Sheng@amd.com>; amd-gfx@lists.freedesktop.org
Subject: Re: [PATCH 2/2] drm/amdgpu: add fw release for sdma v5_0


[AMD Official Use Only - Internal Distribution Only]



________________________________
From: amd-gfx <amd-gfx-bounces@lists.freedesktop.org<mailto:amd-gfx-bounces@lists.freedesktop.org>> on behalf of Wenhui Sheng <Wenhui.Sheng@amd.com<mailto:Wenhui.Sheng@amd.com>>
Sent: Thursday, June 18, 2020 3:53 PM
To: amd-gfx@lists.freedesktop.org<mailto:amd-gfx@lists.freedesktop.org> <amd-gfx@lists.freedesktop.org<mailto:amd-gfx@lists.freedesktop.org>>
Cc: Sheng, Wenhui <Wenhui.Sheng@amd.com<mailto:Wenhui.Sheng@amd.com>>
Subject: [PATCH 2/2] drm/amdgpu: add fw release for sdma v5_0

sdma v5_0 fw isn't released when module exit

Signed-off-by: Wenhui Sheng <Wenhui.Sheng@amd.com<mailto:Wenhui.Sheng@amd.com>>
---
 drivers/gpu/drm/amd/amdgpu/sdma_v5_0.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/sdma_v5_0.c b/drivers/gpu/drm/amd/amdgpu/sdma_v5_0.c
index 58d2a80af450..6751ad69ed90 100644
--- a/drivers/gpu/drm/amd/amdgpu/sdma_v5_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/sdma_v5_0.c
@@ -1299,8 +1299,12 @@ static int sdma_v5_0_sw_fini(void *handle)
         struct amdgpu_device *adev = (struct amdgpu_device *)handle;
         int i;

-       for (i = 0; i < adev->sdma.num_instances; i++)
+       for (i = 0; i < adev->sdma.num_instances; i++) {
+               if (adev->sdma.instance[i].fw != NULL)
+                       release_firmware(adev->sdma.instance[i].fw);
[kevin]:
the kernel api "release_firmware()" will check argument (is NULL pointer).
i think the patch don't need to double check it.
+
                 amdgpu_ring_fini(&adev->sdma.instance[i].ring);
+       }

         return 0;
 }
--
2.17.1

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org<mailto:amd-gfx@lists.freedesktop.org>
https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.freedesktop.org%2Fmailman%2Flistinfo%2Famd-gfx&amp;data=02%7C01%7CKevin1.Wang%40amd.com%7Cd0b8b72e0be443a7343d08d8135cb093%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637280636699912801&amp;sdata=HrKVAmRO0HKFZBRG6oWq3thNlGBd0T9bMramDU1ijys%3D&amp;reserved=0

[-- Attachment #1.2: Type: text/html, Size: 7920 bytes --]

[-- Attachment #2: Type: text/plain, Size: 154 bytes --]

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply related	[flat|nested] 9+ messages in thread
* Re: [PATCH 2/2] drm/amdgpu: add fw release for sdma v5_0
  2020-06-18  8:31     ` Sheng, Wenhui
@ 2020-06-18  8:47       ` Wang, Kevin(Yang)
  2020-06-18  9:21         ` Sheng, Wenhui
  0 siblings, 1 reply; 9+ messages in thread
From: Wang, Kevin(Yang) @ 2020-06-18  8:47 UTC (permalink / raw)
  To: Sheng, Wenhui, amd-gfx


[-- Attachment #1.1: Type: text/plain, Size: 3386 bytes --]

[AMD Official Use Only - Internal Distribution Only]

afte the API firmware_realease created, the api logic never changed. (the first commit by Linus)
and you can use below command to reference the api usage case in our drm driver.

$ grep -nR "release_firmware" -A 5 -B 5 drivers/gpu/drm/ | vim -

Best Regards,
Kevin
________________________________
From: Sheng, Wenhui <Wenhui.Sheng@amd.com>
Sent: Thursday, June 18, 2020 4:31 PM
To: Wang, Kevin(Yang) <Kevin1.Wang@amd.com>; amd-gfx@lists.freedesktop.org <amd-gfx@lists.freedesktop.org>
Subject: RE: [PATCH 2/2] drm/amdgpu: add fw release for sdma v5_0


[AMD Official Use Only - Internal Distribution Only]


[AMD Official Use Only - Internal Distribution Only]



Although we know that release_firmware has null check, but the code is not maintained by ourselves, so I think it’s much more safe to add null  check before invoke release_firmware.





Brs

Wenhui

From: Wang, Kevin(Yang) <Kevin1.Wang@amd.com>
Sent: Thursday, June 18, 2020 4:25 PM
To: Sheng, Wenhui <Wenhui.Sheng@amd.com>; amd-gfx@lists.freedesktop.org
Subject: Re: [PATCH 2/2] drm/amdgpu: add fw release for sdma v5_0



[AMD Official Use Only - Internal Distribution Only]







________________________________

From: amd-gfx <amd-gfx-bounces@lists.freedesktop.org<mailto:amd-gfx-bounces@lists.freedesktop.org>> on behalf of Wenhui Sheng <Wenhui.Sheng@amd.com<mailto:Wenhui.Sheng@amd.com>>
Sent: Thursday, June 18, 2020 3:53 PM
To: amd-gfx@lists.freedesktop.org<mailto:amd-gfx@lists.freedesktop.org> <amd-gfx@lists.freedesktop.org<mailto:amd-gfx@lists.freedesktop.org>>
Cc: Sheng, Wenhui <Wenhui.Sheng@amd.com<mailto:Wenhui.Sheng@amd.com>>
Subject: [PATCH 2/2] drm/amdgpu: add fw release for sdma v5_0



sdma v5_0 fw isn't released when module exit

Signed-off-by: Wenhui Sheng <Wenhui.Sheng@amd.com<mailto:Wenhui.Sheng@amd.com>>
---
 drivers/gpu/drm/amd/amdgpu/sdma_v5_0.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/sdma_v5_0.c b/drivers/gpu/drm/amd/amdgpu/sdma_v5_0.c
index 58d2a80af450..6751ad69ed90 100644
--- a/drivers/gpu/drm/amd/amdgpu/sdma_v5_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/sdma_v5_0.c
@@ -1299,8 +1299,12 @@ static int sdma_v5_0_sw_fini(void *handle)
         struct amdgpu_device *adev = (struct amdgpu_device *)handle;
         int i;

-       for (i = 0; i < adev->sdma.num_instances; i++)
+       for (i = 0; i < adev->sdma.num_instances; i++) {
+               if (adev->sdma.instance[i].fw != NULL)
+                       release_firmware(adev->sdma.instance[i].fw);

[kevin]:

the kernel api "release_firmware()" will check argument (is NULL pointer).

i think the patch don't need to double check it.

+
                 amdgpu_ring_fini(&adev->sdma.instance[i].ring);
+       }

         return 0;
 }
--
2.17.1

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org<mailto:amd-gfx@lists.freedesktop.org>
https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.freedesktop.org%2Fmailman%2Flistinfo%2Famd-gfx&amp;data=02%7C01%7CKevin1.Wang%40amd.com%7Cd0b8b72e0be443a7343d08d8135cb093%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637280636699912801&amp;sdata=HrKVAmRO0HKFZBRG6oWq3thNlGBd0T9bMramDU1ijys%3D&amp;reserved=0

[-- Attachment #1.2: Type: text/html, Size: 9714 bytes --]

[-- Attachment #2: Type: text/plain, Size: 154 bytes --]

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply related	[flat|nested] 9+ messages in thread
* RE: [PATCH 2/2] drm/amdgpu: add fw release for sdma v5_0
  2020-06-18  8:47       ` Wang, Kevin(Yang)
@ 2020-06-18  9:21         ` Sheng, Wenhui
  2020-06-18  9:29           ` Wang, Kevin(Yang)
  0 siblings, 1 reply; 9+ messages in thread
From: Sheng, Wenhui @ 2020-06-18  9:21 UTC (permalink / raw)
  To: Wang, Kevin(Yang), amd-gfx


[-- Attachment #1.1: Type: text/plain, Size: 4209 bytes --]

[AMD Official Use Only - Internal Distribution Only]


[AMD Official Use Only - Internal Distribution Only]

Hmmmm....


  1.  There are some places in amdgpu driver using release_firmware like this way
  2.  This case is a little different, we use it in loop and maybe not all sdma instances' fw is initialized, just like we did in v5_2

Brs
Wenhui

From: Wang, Kevin(Yang) <Kevin1.Wang@amd.com>
Sent: Thursday, June 18, 2020 4:48 PM
To: Sheng, Wenhui <Wenhui.Sheng@amd.com>; amd-gfx@lists.freedesktop.org
Subject: Re: [PATCH 2/2] drm/amdgpu: add fw release for sdma v5_0


[AMD Official Use Only - Internal Distribution Only]

afte the API firmware_realease created, the api logic never changed. (the first commit by Linus)
and you can use below command to reference the api usage case in our drm driver.

$ grep -nR "release_firmware" -A 5 -B 5 drivers/gpu/drm/ | vim -

Best Regards,
Kevin
________________________________
From: Sheng, Wenhui <Wenhui.Sheng@amd.com<mailto:Wenhui.Sheng@amd.com>>
Sent: Thursday, June 18, 2020 4:31 PM
To: Wang, Kevin(Yang) <Kevin1.Wang@amd.com<mailto:Kevin1.Wang@amd.com>>; amd-gfx@lists.freedesktop.org<mailto:amd-gfx@lists.freedesktop.org> <amd-gfx@lists.freedesktop.org<mailto:amd-gfx@lists.freedesktop.org>>
Subject: RE: [PATCH 2/2] drm/amdgpu: add fw release for sdma v5_0


[AMD Official Use Only - Internal Distribution Only]


[AMD Official Use Only - Internal Distribution Only]



Although we know that release_firmware has null check, but the code is not maintained by ourselves, so I think it's much more safe to add null  check before invoke release_firmware.





Brs

Wenhui

From: Wang, Kevin(Yang) <Kevin1.Wang@amd.com<mailto:Kevin1.Wang@amd.com>>
Sent: Thursday, June 18, 2020 4:25 PM
To: Sheng, Wenhui <Wenhui.Sheng@amd.com<mailto:Wenhui.Sheng@amd.com>>; amd-gfx@lists.freedesktop.org<mailto:amd-gfx@lists.freedesktop.org>
Subject: Re: [PATCH 2/2] drm/amdgpu: add fw release for sdma v5_0



[AMD Official Use Only - Internal Distribution Only]







________________________________

From: amd-gfx <amd-gfx-bounces@lists.freedesktop.org<mailto:amd-gfx-bounces@lists.freedesktop.org>> on behalf of Wenhui Sheng <Wenhui.Sheng@amd.com<mailto:Wenhui.Sheng@amd.com>>
Sent: Thursday, June 18, 2020 3:53 PM
To: amd-gfx@lists.freedesktop.org<mailto:amd-gfx@lists.freedesktop.org> <amd-gfx@lists.freedesktop.org<mailto:amd-gfx@lists.freedesktop.org>>
Cc: Sheng, Wenhui <Wenhui.Sheng@amd.com<mailto:Wenhui.Sheng@amd.com>>
Subject: [PATCH 2/2] drm/amdgpu: add fw release for sdma v5_0



sdma v5_0 fw isn't released when module exit

Signed-off-by: Wenhui Sheng <Wenhui.Sheng@amd.com<mailto:Wenhui.Sheng@amd.com>>
---
 drivers/gpu/drm/amd/amdgpu/sdma_v5_0.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/sdma_v5_0.c b/drivers/gpu/drm/amd/amdgpu/sdma_v5_0.c
index 58d2a80af450..6751ad69ed90 100644
--- a/drivers/gpu/drm/amd/amdgpu/sdma_v5_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/sdma_v5_0.c
@@ -1299,8 +1299,12 @@ static int sdma_v5_0_sw_fini(void *handle)
         struct amdgpu_device *adev = (struct amdgpu_device *)handle;
         int i;

-       for (i = 0; i < adev->sdma.num_instances; i++)
+       for (i = 0; i < adev->sdma.num_instances; i++) {
+               if (adev->sdma.instance[i].fw != NULL)
+                       release_firmware(adev->sdma.instance[i].fw);

[kevin]:

the kernel api "release_firmware()" will check argument (is NULL pointer).

i think the patch don't need to double check it.

+
                 amdgpu_ring_fini(&adev->sdma.instance[i].ring);
+       }

         return 0;
 }
--
2.17.1

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org<mailto:amd-gfx@lists.freedesktop.org>
https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.freedesktop.org%2Fmailman%2Flistinfo%2Famd-gfx&amp;data=02%7C01%7CKevin1.Wang%40amd.com%7Cd0b8b72e0be443a7343d08d8135cb093%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637280636699912801&amp;sdata=HrKVAmRO0HKFZBRG6oWq3thNlGBd0T9bMramDU1ijys%3D&amp;reserved=0

[-- Attachment #1.2: Type: text/html, Size: 14330 bytes --]

[-- Attachment #2: Type: text/plain, Size: 154 bytes --]

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply related	[flat|nested] 9+ messages in thread
* Re: [PATCH 2/2] drm/amdgpu: add fw release for sdma v5_0
  2020-06-18  9:21         ` Sheng, Wenhui
@ 2020-06-18  9:29           ` Wang, Kevin(Yang)
  0 siblings, 0 replies; 9+ messages in thread
From: Wang, Kevin(Yang) @ 2020-06-18  9:29 UTC (permalink / raw)
  To: Sheng, Wenhui, amd-gfx


[-- Attachment #1.1: Type: text/plain, Size: 6057 bytes --]

[AMD Official Use Only - Internal Distribution Only]

before this patch, the driver has same code logic at "drivers/gpu/drm/amd/amdgpu/sdma_v3_0.c"
related patch: drm/amdgpu: fix sdma3 ucode mem leak

commit 14d83e78c578a6c45163fb399ee760fe0d314bad
Author: Monk Liu <Monk.Liu@amd.com>
Date:   Mon May 30 15:15:32 2016 +0800

    drm/amdgpu: fix sdma3 ucode mem leak

    Signed-off-by: Monk Liu <Monk.Liu@amd.com>
    Reviewed-by: Christian König <christian.koenig@amd.com>
    Signed-off-by: Alex Deucher <alexander.deucher@amd.com>

diff --git a/drivers/gpu/drm/amd/amdgpu/sdma_v3_0.c b/drivers/gpu/drm/amd/amdgpu/sdma_v3_0.c
index 33605d4ac2d9..532ea88da66a 100644
--- a/drivers/gpu/drm/amd/amdgpu/sdma_v3_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/sdma_v3_0.c
@@ -236,6 +236,15 @@ static void sdma_v3_0_init_golden_registers(struct amdgpu_device *adev)
        }
 }

+static void sdma_v3_0_free_microcode(struct amdgpu_device *adev)
+{
+       int i;
+       for (i = 0; i < adev->sdma.num_instances; i++) {
+               release_firmware(adev->sdma.instance[i].fw);
+               adev->sdma.instance[i].fw = NULL;
+       }
+}
+
 /**
  * sdma_v3_0_init_microcode - load ucode images from disk
  *
@@ -1256,6 +1265,7 @@ static int sdma_v3_0_sw_fini(void *handle)
        for (i = 0; i < adev->sdma.num_instances; i++)
                amdgpu_ring_fini(&adev->sdma.instance[i].ring);

+       sdma_v3_0_free_microcode(adev);
        return 0;
 }

Best Regards,
Kevin
________________________________
From: Sheng, Wenhui <Wenhui.Sheng@amd.com>
Sent: Thursday, June 18, 2020 5:21 PM
To: Wang, Kevin(Yang) <Kevin1.Wang@amd.com>; amd-gfx@lists.freedesktop.org <amd-gfx@lists.freedesktop.org>
Subject: RE: [PATCH 2/2] drm/amdgpu: add fw release for sdma v5_0


[AMD Official Use Only - Internal Distribution Only]


[AMD Official Use Only - Internal Distribution Only]



Hmmmm….



  1.  There are some places in amdgpu driver using release_firmware like this way
  2.  This case is a little different, we use it in loop and maybe not all sdma instances’ fw is initialized, just like we did in v5_2



Brs

Wenhui



From: Wang, Kevin(Yang) <Kevin1.Wang@amd.com>
Sent: Thursday, June 18, 2020 4:48 PM
To: Sheng, Wenhui <Wenhui.Sheng@amd.com>; amd-gfx@lists.freedesktop.org
Subject: Re: [PATCH 2/2] drm/amdgpu: add fw release for sdma v5_0



[AMD Official Use Only - Internal Distribution Only]



afte the API firmware_realease created, the api logic never changed. (the first commit by Linus)

and you can use below command to reference the api usage case in our drm driver.



$ grep -nR "release_firmware" -A 5 -B 5 drivers/gpu/drm/ | vim -



Best Regards,

Kevin

________________________________

From: Sheng, Wenhui <Wenhui.Sheng@amd.com<mailto:Wenhui.Sheng@amd.com>>
Sent: Thursday, June 18, 2020 4:31 PM
To: Wang, Kevin(Yang) <Kevin1.Wang@amd.com<mailto:Kevin1.Wang@amd.com>>; amd-gfx@lists.freedesktop.org<mailto:amd-gfx@lists.freedesktop.org> <amd-gfx@lists.freedesktop.org<mailto:amd-gfx@lists.freedesktop.org>>
Subject: RE: [PATCH 2/2] drm/amdgpu: add fw release for sdma v5_0



[AMD Official Use Only - Internal Distribution Only]



[AMD Official Use Only - Internal Distribution Only]



Although we know that release_firmware has null check, but the code is not maintained by ourselves, so I think it’s much more safe to add null  check before invoke release_firmware.





Brs

Wenhui

From: Wang, Kevin(Yang) <Kevin1.Wang@amd.com<mailto:Kevin1.Wang@amd.com>>
Sent: Thursday, June 18, 2020 4:25 PM
To: Sheng, Wenhui <Wenhui.Sheng@amd.com<mailto:Wenhui.Sheng@amd.com>>; amd-gfx@lists.freedesktop.org<mailto:amd-gfx@lists.freedesktop.org>
Subject: Re: [PATCH 2/2] drm/amdgpu: add fw release for sdma v5_0



[AMD Official Use Only - Internal Distribution Only]







________________________________

From: amd-gfx <amd-gfx-bounces@lists.freedesktop.org<mailto:amd-gfx-bounces@lists.freedesktop.org>> on behalf of Wenhui Sheng <Wenhui.Sheng@amd.com<mailto:Wenhui.Sheng@amd.com>>
Sent: Thursday, June 18, 2020 3:53 PM
To: amd-gfx@lists.freedesktop.org<mailto:amd-gfx@lists.freedesktop.org> <amd-gfx@lists.freedesktop.org<mailto:amd-gfx@lists.freedesktop.org>>
Cc: Sheng, Wenhui <Wenhui.Sheng@amd.com<mailto:Wenhui.Sheng@amd.com>>
Subject: [PATCH 2/2] drm/amdgpu: add fw release for sdma v5_0



sdma v5_0 fw isn't released when module exit

Signed-off-by: Wenhui Sheng <Wenhui.Sheng@amd.com<mailto:Wenhui.Sheng@amd.com>>
---
 drivers/gpu/drm/amd/amdgpu/sdma_v5_0.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/sdma_v5_0.c b/drivers/gpu/drm/amd/amdgpu/sdma_v5_0.c
index 58d2a80af450..6751ad69ed90 100644
--- a/drivers/gpu/drm/amd/amdgpu/sdma_v5_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/sdma_v5_0.c
@@ -1299,8 +1299,12 @@ static int sdma_v5_0_sw_fini(void *handle)
         struct amdgpu_device *adev = (struct amdgpu_device *)handle;
         int i;

-       for (i = 0; i < adev->sdma.num_instances; i++)
+       for (i = 0; i < adev->sdma.num_instances; i++) {
+               if (adev->sdma.instance[i].fw != NULL)
+                       release_firmware(adev->sdma.instance[i].fw);

[kevin]:

the kernel api "release_firmware()" will check argument (is NULL pointer).

i think the patch don't need to double check it.

+
                 amdgpu_ring_fini(&adev->sdma.instance[i].ring);
+       }

         return 0;
 }
--
2.17.1

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org<mailto:amd-gfx@lists.freedesktop.org>
https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.freedesktop.org%2Fmailman%2Flistinfo%2Famd-gfx&amp;data=02%7C01%7CKevin1.Wang%40amd.com%7Cd0b8b72e0be443a7343d08d8135cb093%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637280636699912801&amp;sdata=HrKVAmRO0HKFZBRG6oWq3thNlGBd0T9bMramDU1ijys%3D&amp;reserved=0

[-- Attachment #1.2: Type: text/html, Size: 17716 bytes --]

[-- Attachment #2: Type: text/plain, Size: 154 bytes --]

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply related	[flat|nested] 9+ messages in thread
* RE: [PATCH 2/2] drm/amdgpu: add fw release for sdma v5_0
  2020-06-18  7:53 ` [PATCH 2/2] drm/amdgpu: add fw release for sdma v5_0 Wenhui Sheng
  2020-06-18  8:25   ` Wang, Kevin(Yang)
@ 2020-06-19  8:29   ` Zhang, Hawking
  2020-06-22 18:20   ` Christian König
  2 siblings, 0 replies; 9+ messages in thread
From: Zhang, Hawking @ 2020-06-19  8:29 UTC (permalink / raw)
  To: Sheng, Wenhui, amd-gfx; +Cc: Sheng, Wenhui

[AMD Public Use]

Series is:

Reviewed-by: Hawking Zhang <Hawking.Zhang@amd.com>

Regards,
Hawking
-----Original Message-----
From: amd-gfx <amd-gfx-bounces@lists.freedesktop.org> On Behalf Of Wenhui Sheng
Sent: Thursday, June 18, 2020 15:53
To: amd-gfx@lists.freedesktop.org
Cc: Sheng, Wenhui <Wenhui.Sheng@amd.com>
Subject: [PATCH 2/2] drm/amdgpu: add fw release for sdma v5_0

sdma v5_0 fw isn't released when module exit

Signed-off-by: Wenhui Sheng <Wenhui.Sheng@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/sdma_v5_0.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/sdma_v5_0.c b/drivers/gpu/drm/amd/amdgpu/sdma_v5_0.c
index 58d2a80af450..6751ad69ed90 100644
--- a/drivers/gpu/drm/amd/amdgpu/sdma_v5_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/sdma_v5_0.c
@@ -1299,8 +1299,12 @@ static int sdma_v5_0_sw_fini(void *handle)
 	struct amdgpu_device *adev = (struct amdgpu_device *)handle;
 	int i;
 
-	for (i = 0; i < adev->sdma.num_instances; i++)
+	for (i = 0; i < adev->sdma.num_instances; i++) {
+		if (adev->sdma.instance[i].fw != NULL)
+			release_firmware(adev->sdma.instance[i].fw);
+
 		amdgpu_ring_fini(&adev->sdma.instance[i].ring);
+	}
 
 	return 0;
 }
-- 
2.17.1

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.freedesktop.org%2Fmailman%2Flistinfo%2Famd-gfx&amp;data=02%7C01%7Chawking.zhang%40amd.com%7Cd0b8b72e0be443a7343d08d8135cb093%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637280636689938431&amp;sdata=2g%2BOLo5Hg6gxO2pF%2B2uDnshodSAmLaSh9RZQsKrnFUI%3D&amp;reserved=0
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply related	[flat|nested] 9+ messages in thread
* Re: [PATCH 2/2] drm/amdgpu: add fw release for sdma v5_0
  2020-06-18  7:53 ` [PATCH 2/2] drm/amdgpu: add fw release for sdma v5_0 Wenhui Sheng
  2020-06-18  8:25   ` Wang, Kevin(Yang)
  2020-06-19  8:29   ` Zhang, Hawking
@ 2020-06-22 18:20   ` Christian König
  2 siblings, 0 replies; 9+ messages in thread
From: Christian König @ 2020-06-22 18:20 UTC (permalink / raw)
  To: Wenhui Sheng, amd-gfx

Am 18.06.20 um 09:53 schrieb Wenhui Sheng:
> sdma v5_0 fw isn't released when module exit
>
> Signed-off-by: Wenhui Sheng <Wenhui.Sheng@amd.com>
> ---
>   drivers/gpu/drm/amd/amdgpu/sdma_v5_0.c | 6 +++++-
>   1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/sdma_v5_0.c b/drivers/gpu/drm/amd/amdgpu/sdma_v5_0.c
> index 58d2a80af450..6751ad69ed90 100644
> --- a/drivers/gpu/drm/amd/amdgpu/sdma_v5_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/sdma_v5_0.c
> @@ -1299,8 +1299,12 @@ static int sdma_v5_0_sw_fini(void *handle)
>   	struct amdgpu_device *adev = (struct amdgpu_device *)handle;
>   	int i;
>   
> -	for (i = 0; i < adev->sdma.num_instances; i++)
> +	for (i = 0; i < adev->sdma.num_instances; i++) {
> +		if (adev->sdma.instance[i].fw != NULL)
> +			release_firmware(adev->sdma.instance[i].fw);

Please drop the extra NULL check here.

Kernel APIs like kfree() etc.. are usually NULL tolerant and we have 
automated scripts complaining about this as bad coding style if you 
duplicate the check in the driver.

Apart from that the series looks good to me.

Regards,
Christian.

> +
>   		amdgpu_ring_fini(&adev->sdma.instance[i].ring);
> +	}
>   
>   	return 0;
>   }

_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

^ permalink raw reply	[flat|nested] 9+ messages in thread
end of thread, other threads:[~2020-06-22 18:20 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-06-18  7:53 [PATCH 1/2] drm/amdgpu: sdma v5_2 ring bo mem leak Wenhui Sheng
2020-06-18  7:53 ` [PATCH 2/2] drm/amdgpu: add fw release for sdma v5_0 Wenhui Sheng
2020-06-18  8:25   ` Wang, Kevin(Yang)
2020-06-18  8:31     ` Sheng, Wenhui
2020-06-18  8:47       ` Wang, Kevin(Yang)
2020-06-18  9:21         ` Sheng, Wenhui
2020-06-18  9:29           ` Wang, Kevin(Yang)
2020-06-19  8:29   ` Zhang, Hawking
2020-06-22 18:20   ` Christian König

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).