subject:"RE\: \[PATCH 1\/1\] drm\/amdgpu\: rework ip block reinit for sriov"

Re: [PATCH 1/1] drm/amdgpu: rework ip block reinit for sriov

2020-08-28 Thread Nirmoy




On 8/28/20 9:29 AM, Liu, Monk wrote:

[AMD Official Use Only - Internal Distribution Only]

Please don't change those code unless you have a full stress test and a solid 
reason (what bug fixed or what new feature introduced )

Otherwise if it's a pure personal refactor or cleanup it will be not necessary



Hi Monk,


The patch was to fix the previous inconsistent optimization patch.

I don't have a sriov device so definitely wouldn't merge it without a 
tested-by tag so I added all relevant people.



Regards,

Nirmoy



_
Monk Liu|GPU Virtualization Team |AMD


-Original Message-
From: Das, Nirmoy 
Sent: Thursday, August 27, 2020 11:19 PM
To: amd-gfx@lists.freedesktop.org
Cc: Deucher, Alexander ; Liu, Monk ; Gu, JiaWei 
(Will) ; Deng, Emily ; Das, Nirmoy 

Subject: [PATCH 1/1] drm/amdgpu: rework ip block reinit for sriov

This patch removes some unwanted code duplication and simplifies sriov's ip 
block reinit logic.

Signed-off-by: Nirmoy Das 
---
  drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 117 +++--
  1 file changed, 60 insertions(+), 57 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
index 696a61cc3ac6..0db6db03bcd3 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
@@ -2587,77 +2587,80 @@ int amdgpu_device_ip_suspend(struct amdgpu_device *adev)
  return r;
  }

-static int amdgpu_device_ip_reinit_early_sriov(struct amdgpu_device *adev)
+/** amdgpu_device_is_early_ip_block_sriov - check for early ip_blocks
+ *
+ * @ip_block: ip block that need to be check
+ *
+ * Returns a tri-state value for a given ip block.
+ * If an ip block requires early reinit sriov then return 1 or 0 otherwise.
+ * Return -1 on invalid ip block.
+ *
+ */
+
+static int
+amdgpu_device_is_early_ip_block_sriov(const enum amd_ip_block_type
+ip_block)
  {
-int i, r;
+switch (ip_block) {
+/* early ip blocks */
+case AMD_IP_BLOCK_TYPE_GMC:
+case AMD_IP_BLOCK_TYPE_COMMON:
+case AMD_IP_BLOCK_TYPE_PSP:
+case AMD_IP_BLOCK_TYPE_IH:
+return 1;
+/* late ip blocks */
+case AMD_IP_BLOCK_TYPE_SMC:
+case AMD_IP_BLOCK_TYPE_DCE:
+case AMD_IP_BLOCK_TYPE_GFX:
+case AMD_IP_BLOCK_TYPE_SDMA:
+case AMD_IP_BLOCK_TYPE_UVD:
+case AMD_IP_BLOCK_TYPE_VCE:
+case AMD_IP_BLOCK_TYPE_VCN:
+return 0;
+/* invalid ip block */
+default:
+return -1;
+}
+}

-static enum amd_ip_block_type ip_order[] = {
-AMD_IP_BLOCK_TYPE_GMC,
-AMD_IP_BLOCK_TYPE_COMMON,
-AMD_IP_BLOCK_TYPE_PSP,
-AMD_IP_BLOCK_TYPE_IH,
-};
+static int amdgpu_device_ip_reinit_sriov(struct amdgpu_device *adev,
+ const int is_early)
+{
+int i;

  for (i = 0; i < adev->num_ip_blocks; i++) {
-int j;
+int r = 0;
+bool init_ip;
  struct amdgpu_ip_block *block;
+enum amd_ip_block_type ip_block;

  block = >ip_blocks[i];
  block->status.hw = false;
+ip_block = block->version->type;
+init_ip = (is_early ==
+   amdgpu_device_is_early_ip_block_sriov(ip_block));

-for (j = 0; j < ARRAY_SIZE(ip_order); j++) {
-
-if (block->version->type != ip_order[j] ||
-!block->status.valid)
-continue;
+if (!init_ip ||
+(!is_early && block->status.hw) ||
+!block->status.valid)
+continue;

-r = block->version->funcs->hw_init(adev);
-DRM_INFO("RE-INIT-early: %s %s\n", block->version->funcs->name, 
r?"failed":"succeeded");
-if (r)
-return r;
-block->status.hw = true;
+if (init_ip && (ip_block == AMD_IP_BLOCK_TYPE_SMC)) {
+r = block->version->funcs->resume(adev);
+goto show_log;
  }
-}
-
-return 0;
-}

-static int amdgpu_device_ip_reinit_late_sriov(struct amdgpu_device *adev) -{
-int i, r;
-
-static enum amd_ip_block_type ip_order[] = {
-AMD_IP_BLOCK_TYPE_SMC,
-AMD_IP_BLOCK_TYPE_DCE,
-AMD_IP_BLOCK_TYPE_GFX,
-AMD_IP_BLOCK_TYPE_SDMA,
-AMD_IP_BLOCK_TYPE_UVD,
-AMD_IP_BLOCK_TYPE_VCE,
-AMD_IP_BLOCK_TYPE_VCN
-};
-
-for (i = 0; i < ARRAY_SIZE(ip_order); i++) {
-int j;
-struct amdgpu_ip_block *block;
+if (init_ip)
+r = block->version->funcs->hw_init(adev);

-for (j = 0; j < adev->num_ip_blocks; j++) {
-block = >ip_blocks[j];
+show_log:
+DRM_INFO("RE-INIT-%s: %s %s\n", is_early ? "early":"late",
+ block->version->funcs->name, r ? "failed":"succeeded");

-if (block->version->type != ip_order[i] ||
-!block->status.valid ||
-block->status.hw)
-continue;
+if (r)
+return r;

-if (block->version->type == AMD_IP_BLOCK_TYPE_SMC)
-r = block->version->funcs->resume(adev);
-else
-r = block->version->funcs->hw_init(adev);
+block->status.hw = true;

-DRM_INFO("RE-INIT-late: %s %s\n", block->version->funcs->name, 
r?"failed":"succeeded");
-if (r)
-return r;
-block->status.hw = true;
-}
  }

  return 0;
@@ -3901,7 +3904,7 @@ static int amdgpu_device_reset_sriov(struct amdgpu_device 
*adev,
  amdgpu_amdkfd_pre_reset(adev);

  /* Resume IP prior to SMC */
-r = amdgpu_device_ip_reinit_early_sriov(adev);
+r = amdgpu_device_ip_reinit_sriov(adev, 1);
  if (r)
  goto error;

@@ -3914,7 +3917,7 @@ static int amdgpu_device_reset_sriov(struct amdgpu_device 
*adev,
  return r;

RE: [PATCH 1/1] drm/amdgpu: rework ip block reinit for sriov

2020-08-28 Thread Gu, JiaWei (Will)

[AMD Official Use Only - Internal Distribution Only]

Per Monk and Emily's suggestion, I will not submit another patch to make 
amdgpu_device_ip_reinit_late_sriov()
and amdgpu_device_ip_reinit_early_sriov() consistent.
There's already no logic bug, the little difference in loop layer order does no 
harm.

Sorry about missing the amdgpu_device_ip_reinit_late_sriov() part.

Best regards,
Jiawei

-Original Message-
From: Das, Nirmoy  
Sent: Friday, August 28, 2020 3:14 PM
To: Gu, JiaWei (Will) ; Das, Nirmoy ; 
Deng, Emily ; amd-gfx@lists.freedesktop.org
Cc: Deucher, Alexander ; Liu, Monk 
Subject: Re: [PATCH 1/1] drm/amdgpu: rework ip block reinit for sriov


On 8/28/20 8:58 AM, Gu, JiaWei (Will) wrote:
> [AMD Official Use Only - Internal Distribution Only]
>
> Hi Nirmoy,
>
> I also found amdgpu_device_ip_reinit_late_sriov() part is missed.
> Will push another patch to make them consistent soon.


Thanks, Jiawei.


Nirmoy


>
> Best regards,
> Jiawei
>
> -Original Message-
> From: Das, Nirmoy 
> Sent: Friday, August 28, 2020 2:33 PM
> To: Deng, Emily ; Das, Nirmoy 
> ; amd-gfx@lists.freedesktop.org
> Cc: Deucher, Alexander ; Liu, Monk 
> ; Gu, JiaWei (Will) 
> Subject: Re: [PATCH 1/1] drm/amdgpu: rework ip block reinit for sriov
>
>
> On 8/28/20 3:16 AM, Deng, Emily wrote:
>> [AMD Official Use Only - Internal Distribution Only]
>>
>> Hi Nirmoy,
>>   Still think the original logical is more clear.
>
> No problem but we should at least make sure
> amdgpu_device_ip_reinit_late_sriov() and
> amdgpu_device_ip_reinit_early_sriov()
>
> are consistent. The last patch from Jiawei only touched 
> amdgpu_device_ip_reinit_early_sriov(), same optimization should apply
>
> to amdgpu_device_ip_reinit_late_sriov()
>
>
> Regards,
>
> Nirmoy
>
>
>> Best wishes
>> Emily Deng
>>
>>
>>
>>> -Original Message-
>>> From: Das, Nirmoy 
>>> Sent: Thursday, August 27, 2020 11:19 PM
>>> To: amd-gfx@lists.freedesktop.org
>>> Cc: Deucher, Alexander ; Liu, Monk 
>>> ; Gu, JiaWei (Will) ; Deng, 
>>> Emily ; Das, Nirmoy 
>>> Subject: [PATCH 1/1] drm/amdgpu: rework ip block reinit for sriov
>>>
>>> This patch removes some unwanted code duplication and simplifies 
>>> sriov's ip block reinit logic.
>>>
>>> Signed-off-by: Nirmoy Das 
>>> ---
>>> drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 117
>>> +++--
>>> 1 file changed, 60 insertions(+), 57 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>>> index 696a61cc3ac6..0db6db03bcd3 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>>> @@ -2587,77 +2587,80 @@ int amdgpu_device_ip_suspend(struct 
>>> amdgpu_device *adev) return r; }
>>>
>>> -static int amdgpu_device_ip_reinit_early_sriov(struct amdgpu_device
>>> *adev)
>>> +/** amdgpu_device_is_early_ip_block_sriov - check for early 
>>> +ip_blocks
>>> + *
>>> + * @ip_block: ip block that need to be check
>>> + *
>>> + * Returns a tri-state value for a given ip block.
>>> + * If an ip block requires early reinit sriov then return 1 or 0 otherwise.
>>> + * Return -1 on invalid ip block.
>>> + *
>>> + */
>>> +
>>> +static int
>>> +amdgpu_device_is_early_ip_block_sriov(const enum amd_ip_block_type
>>> +ip_block)
>>> {
>>> -int i, r;
>>> +switch (ip_block) {
>>> +/* early ip blocks */
>>> +case AMD_IP_BLOCK_TYPE_GMC:
>>> +case AMD_IP_BLOCK_TYPE_COMMON:
>>> +case AMD_IP_BLOCK_TYPE_PSP:
>>> +case AMD_IP_BLOCK_TYPE_IH:
>>> +return 1;
>>> +/* late ip blocks */
>>> +case AMD_IP_BLOCK_TYPE_SMC:
>>> +case AMD_IP_BLOCK_TYPE_DCE:
>>> +case AMD_IP_BLOCK_TYPE_GFX:
>>> +case AMD_IP_BLOCK_TYPE_SDMA:
>>> +case AMD_IP_BLOCK_TYPE_UVD:
>>> +case AMD_IP_BLOCK_TYPE_VCE:
>>> +case AMD_IP_BLOCK_TYPE_VCN:
>>> +return 0;
>>> +/* invalid ip block */
>>> +default:
>>> +return -1;
>>> +}
>>> +}
>>>
>>> -static enum amd_ip_block_type ip_order[] = { 
>>> -AMD_IP_BLOCK_TYPE_GMC, -AMD_IP_BLOCK_TYPE_COMMON, 
>>> -AMD_IP_BLOCK_TYPE_PSP, -AMD_IP_BLOCK_TYPE_IH, -};
>>> +static int amdgpu_device_ip_reinit_sriov(struct amdgpu_device 
>>> +*adev, const int is_early) { int i;
>>>
>>> for (i = 0; i < a

RE: [PATCH 1/1] drm/amdgpu: rework ip block reinit for sriov

2020-08-28 Thread Liu, Monk

[AMD Official Use Only - Internal Distribution Only]

Please don't change those code unless you have a full stress test and a solid 
reason (what bug fixed or what new feature introduced )

Otherwise if it's a pure personal refactor or cleanup it will be not necessary

_
Monk Liu|GPU Virtualization Team |AMD


-Original Message-
From: Das, Nirmoy 
Sent: Thursday, August 27, 2020 11:19 PM
To: amd-gfx@lists.freedesktop.org
Cc: Deucher, Alexander ; Liu, Monk 
; Gu, JiaWei (Will) ; Deng, Emily 
; Das, Nirmoy 
Subject: [PATCH 1/1] drm/amdgpu: rework ip block reinit for sriov

This patch removes some unwanted code duplication and simplifies sriov's ip 
block reinit logic.

Signed-off-by: Nirmoy Das 
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 117 +++--
 1 file changed, 60 insertions(+), 57 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
index 696a61cc3ac6..0db6db03bcd3 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
@@ -2587,77 +2587,80 @@ int amdgpu_device_ip_suspend(struct amdgpu_device *adev)
 return r;
 }

-static int amdgpu_device_ip_reinit_early_sriov(struct amdgpu_device *adev)
+/** amdgpu_device_is_early_ip_block_sriov - check for early ip_blocks
+ *
+ * @ip_block: ip block that need to be check
+ *
+ * Returns a tri-state value for a given ip block.
+ * If an ip block requires early reinit sriov then return 1 or 0 otherwise.
+ * Return -1 on invalid ip block.
+ *
+ */
+
+static int
+amdgpu_device_is_early_ip_block_sriov(const enum amd_ip_block_type
+ip_block)
 {
-int i, r;
+switch (ip_block) {
+/* early ip blocks */
+case AMD_IP_BLOCK_TYPE_GMC:
+case AMD_IP_BLOCK_TYPE_COMMON:
+case AMD_IP_BLOCK_TYPE_PSP:
+case AMD_IP_BLOCK_TYPE_IH:
+return 1;
+/* late ip blocks */
+case AMD_IP_BLOCK_TYPE_SMC:
+case AMD_IP_BLOCK_TYPE_DCE:
+case AMD_IP_BLOCK_TYPE_GFX:
+case AMD_IP_BLOCK_TYPE_SDMA:
+case AMD_IP_BLOCK_TYPE_UVD:
+case AMD_IP_BLOCK_TYPE_VCE:
+case AMD_IP_BLOCK_TYPE_VCN:
+return 0;
+/* invalid ip block */
+default:
+return -1;
+}
+}

-static enum amd_ip_block_type ip_order[] = {
-AMD_IP_BLOCK_TYPE_GMC,
-AMD_IP_BLOCK_TYPE_COMMON,
-AMD_IP_BLOCK_TYPE_PSP,
-AMD_IP_BLOCK_TYPE_IH,
-};
+static int amdgpu_device_ip_reinit_sriov(struct amdgpu_device *adev,
+ const int is_early)
+{
+int i;

 for (i = 0; i < adev->num_ip_blocks; i++) {
-int j;
+int r = 0;
+bool init_ip;
 struct amdgpu_ip_block *block;
+enum amd_ip_block_type ip_block;

 block = >ip_blocks[i];
 block->status.hw = false;
+ip_block = block->version->type;
+init_ip = (is_early ==
+   amdgpu_device_is_early_ip_block_sriov(ip_block));

-for (j = 0; j < ARRAY_SIZE(ip_order); j++) {
-
-if (block->version->type != ip_order[j] ||
-!block->status.valid)
-continue;
+if (!init_ip ||
+(!is_early && block->status.hw) ||
+!block->status.valid)
+continue;

-r = block->version->funcs->hw_init(adev);
-DRM_INFO("RE-INIT-early: %s %s\n", block->version->funcs->name, 
r?"failed":"succeeded");
-if (r)
-return r;
-block->status.hw = true;
+if (init_ip && (ip_block == AMD_IP_BLOCK_TYPE_SMC)) {
+r = block->version->funcs->resume(adev);
+goto show_log;
 }
-}
-
-return 0;
-}

-static int amdgpu_device_ip_reinit_late_sriov(struct amdgpu_device *adev) -{
-int i, r;
-
-static enum amd_ip_block_type ip_order[] = {
-AMD_IP_BLOCK_TYPE_SMC,
-AMD_IP_BLOCK_TYPE_DCE,
-AMD_IP_BLOCK_TYPE_GFX,
-AMD_IP_BLOCK_TYPE_SDMA,
-AMD_IP_BLOCK_TYPE_UVD,
-AMD_IP_BLOCK_TYPE_VCE,
-AMD_IP_BLOCK_TYPE_VCN
-};
-
-for (i = 0; i < ARRAY_SIZE(ip_order); i++) {
-int j;
-struct amdgpu_ip_block *block;
+if (init_ip)
+r = block->version->funcs->hw_init(adev);

-for (j = 0; j < adev->num_ip_blocks; j++) {
-block = >ip_blocks[j];
+show_log:
+DRM_INFO("RE-INIT-%s: %s %s\n", is_early ? "early":"late",
+ block->version->funcs->name, r ? "failed":"succeeded");

-if (block->version->type != ip_order[i] ||
-!block->status.valid ||
-block->status.hw)
-continue;
+if (r)
+return r;

-if (block->version->type == AMD_IP_BLOCK_TYPE_SMC)
-r = block->version->funcs->resume(adev);
-else
-r = block->version->funcs->hw_init(adev);
+block->status.hw = true;

-DRM_INFO("RE-INIT-late: %s %s\n", block->version->funcs->name, 
r?"failed":"succeeded");
-if (r)
-return r;
-block->status.hw = true;
-}
 }

 return 0;
@@ -3901,7 +3904,7 @@ static int amdgpu_device_reset_sriov(struct amdgpu_device 
*adev,
 amdgpu_amdkfd_pre_reset(adev);

 /* Resume IP prior to SMC */
-r = amdgpu_device_ip_reinit_early_sriov(adev);
+r = amdgpu_device_ip_reinit_sriov(adev, 1);
 if (r)
 goto error;

@@ -3914,7 +3917,7 @@ static int amdgpu_device_reset_sriov(struct amdgpu_device 
*adev,
 return r;

 /* now we are okay to resume SMC/CP/SDMA */
-r = amdgpu_device_ip_reinit_late_sriov(adev);
+r = amdgpu_device_ip_reinit_sriov(adev, 0);
 if (r)
 goto error;

--
2.28.0

___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org

Re: [PATCH 1/1] drm/amdgpu: rework ip block reinit for sriov

2020-08-28 Thread Nirmoy




On 8/28/20 8:58 AM, Gu, JiaWei (Will) wrote:

[AMD Official Use Only - Internal Distribution Only]

Hi Nirmoy,

I also found amdgpu_device_ip_reinit_late_sriov() part is missed.
Will push another patch to make them consistent soon.



Thanks, Jiawei.


Nirmoy




Best regards,
Jiawei

-Original Message-
From: Das, Nirmoy 
Sent: Friday, August 28, 2020 2:33 PM
To: Deng, Emily ; Das, Nirmoy ; 
amd-gfx@lists.freedesktop.org
Cc: Deucher, Alexander ; Liu, Monk ; Gu, 
JiaWei (Will) 
Subject: Re: [PATCH 1/1] drm/amdgpu: rework ip block reinit for sriov


On 8/28/20 3:16 AM, Deng, Emily wrote:

[AMD Official Use Only - Internal Distribution Only]

Hi Nirmoy,
  Still think the original logical is more clear.


No problem but we should at least make sure
amdgpu_device_ip_reinit_late_sriov() and
amdgpu_device_ip_reinit_early_sriov()

are consistent. The last patch from Jiawei only touched 
amdgpu_device_ip_reinit_early_sriov(), same optimization should apply

to amdgpu_device_ip_reinit_late_sriov()


Regards,

Nirmoy



Best wishes
Emily Deng




-Original Message-
From: Das, Nirmoy 
Sent: Thursday, August 27, 2020 11:19 PM
To: amd-gfx@lists.freedesktop.org
Cc: Deucher, Alexander ; Liu, Monk
; Gu, JiaWei (Will) ; Deng,
Emily ; Das, Nirmoy 
Subject: [PATCH 1/1] drm/amdgpu: rework ip block reinit for sriov

This patch removes some unwanted code duplication and simplifies
sriov's ip block reinit logic.

Signed-off-by: Nirmoy Das 
---
drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 117
+++--
1 file changed, 60 insertions(+), 57 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
index 696a61cc3ac6..0db6db03bcd3 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
@@ -2587,77 +2587,80 @@ int amdgpu_device_ip_suspend(struct
amdgpu_device *adev) return r; }

-static int amdgpu_device_ip_reinit_early_sriov(struct amdgpu_device
*adev)
+/** amdgpu_device_is_early_ip_block_sriov - check for early
+ip_blocks
+ *
+ * @ip_block: ip block that need to be check
+ *
+ * Returns a tri-state value for a given ip block.
+ * If an ip block requires early reinit sriov then return 1 or 0 otherwise.
+ * Return -1 on invalid ip block.
+ *
+ */
+
+static int
+amdgpu_device_is_early_ip_block_sriov(const enum amd_ip_block_type
+ip_block)
{
-int i, r;
+switch (ip_block) {
+/* early ip blocks */
+case AMD_IP_BLOCK_TYPE_GMC:
+case AMD_IP_BLOCK_TYPE_COMMON:
+case AMD_IP_BLOCK_TYPE_PSP:
+case AMD_IP_BLOCK_TYPE_IH:
+return 1;
+/* late ip blocks */
+case AMD_IP_BLOCK_TYPE_SMC:
+case AMD_IP_BLOCK_TYPE_DCE:
+case AMD_IP_BLOCK_TYPE_GFX:
+case AMD_IP_BLOCK_TYPE_SDMA:
+case AMD_IP_BLOCK_TYPE_UVD:
+case AMD_IP_BLOCK_TYPE_VCE:
+case AMD_IP_BLOCK_TYPE_VCN:
+return 0;
+/* invalid ip block */
+default:
+return -1;
+}
+}

-static enum amd_ip_block_type ip_order[] = { -AMD_IP_BLOCK_TYPE_GMC,
-AMD_IP_BLOCK_TYPE_COMMON, -AMD_IP_BLOCK_TYPE_PSP,
-AMD_IP_BLOCK_TYPE_IH, -};
+static int amdgpu_device_ip_reinit_sriov(struct amdgpu_device *adev,
+const int is_early) { int i;

for (i = 0; i < adev->num_ip_blocks; i++) { -int j;
+int r = 0;
+bool init_ip;
struct amdgpu_ip_block *block;
+enum amd_ip_block_type ip_block;

block = >ip_blocks[i];
block->status.hw = false;
+ip_block = block->version->type;
+init_ip = (is_early ==
+   amdgpu_device_is_early_ip_block_sriov(ip_block));

-for (j = 0; j < ARRAY_SIZE(ip_order); j++) {
-
-if (block->version->type != ip_order[j] ||
-!block->status.valid)
-continue;
+if (!init_ip ||
+(!is_early && block->status.hw) ||
+!block->status.valid)
+continue;

-r = block->version->funcs->hw_init(adev);
-DRM_INFO("RE-INIT-early: %s %s\n", block->version-

funcs->name, r?"failed":"succeeded");

-if (r)
-return r;
-block->status.hw = true;
+if (init_ip && (ip_block == AMD_IP_BLOCK_TYPE_SMC)) { r =
+block->version->funcs->resume(adev);
+goto show_log;
}
-}
-
-return 0;
-}

-static int amdgpu_device_ip_reinit_late_sriov(struct amdgpu_device
*adev) -{ -int i, r;
-
-static enum amd_ip_block_type ip_order[] = { -AMD_IP_BLOCK_TYPE_SMC,
-AMD_IP_BLOCK_TYPE_DCE, -AMD_IP_BLOCK_TYPE_GFX,
-AMD_IP_BLOCK_TYPE_SDMA, -AMD_IP_BLOCK_TYPE_UVD,
-AMD_IP_BLOCK_TYPE_VCE, -AMD_IP_BLOCK_TYPE_VCN -};
-
-for (i = 0; i < ARRAY_SIZE(ip_order); i++) { -int j; -struct
amdgpu_ip_block *block;
+if (init_ip)
+r = block->version->funcs->hw_init(adev);

-for (j = 0; j < adev->num_ip_blocks; j++) { -block =
>ip_blocks[j];
+show_log:
+DRM_INFO("RE-INIT-%s: %s %s\n", is_early ? "early":"late",
+ block->version->funcs->name, r ?
"failed":"succeeded");

-if (block->version->type != ip_order[i] || -!block->status.valid ||
-block->status.hw)
-continue;
+if (r)
+return r;

-if (block->version->type ==
AMD_IP_BLOCK_TYPE_SM

RE: [PATCH 1/1] drm/amdgpu: rework ip block reinit for sriov

2020-08-28 Thread Gu, JiaWei (Will)

[AMD Official Use Only - Internal Distribution Only]

Hi Nirmoy,

I also found amdgpu_device_ip_reinit_late_sriov() part is missed.
Will push another patch to make them consistent soon.

Best regards,
Jiawei

-Original Message-
From: Das, Nirmoy  
Sent: Friday, August 28, 2020 2:33 PM
To: Deng, Emily ; Das, Nirmoy ; 
amd-gfx@lists.freedesktop.org
Cc: Deucher, Alexander ; Liu, Monk 
; Gu, JiaWei (Will) 
Subject: Re: [PATCH 1/1] drm/amdgpu: rework ip block reinit for sriov


On 8/28/20 3:16 AM, Deng, Emily wrote:
> [AMD Official Use Only - Internal Distribution Only]
>
> Hi Nirmoy,
>  Still think the original logical is more clear.


No problem but we should at least make sure
amdgpu_device_ip_reinit_late_sriov() and
amdgpu_device_ip_reinit_early_sriov()

are consistent. The last patch from Jiawei only touched 
amdgpu_device_ip_reinit_early_sriov(), same optimization should apply

to amdgpu_device_ip_reinit_late_sriov()


Regards,

Nirmoy


>
> Best wishes
> Emily Deng
>
>
>
>> -Original Message-
>> From: Das, Nirmoy 
>> Sent: Thursday, August 27, 2020 11:19 PM
>> To: amd-gfx@lists.freedesktop.org
>> Cc: Deucher, Alexander ; Liu, Monk 
>> ; Gu, JiaWei (Will) ; Deng, 
>> Emily ; Das, Nirmoy 
>> Subject: [PATCH 1/1] drm/amdgpu: rework ip block reinit for sriov
>>
>> This patch removes some unwanted code duplication and simplifies 
>> sriov's ip block reinit logic.
>>
>> Signed-off-by: Nirmoy Das 
>> ---
>> drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 117 
>> +++--
>> 1 file changed, 60 insertions(+), 57 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>> index 696a61cc3ac6..0db6db03bcd3 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>> @@ -2587,77 +2587,80 @@ int amdgpu_device_ip_suspend(struct 
>> amdgpu_device *adev) return r; }
>>
>> -static int amdgpu_device_ip_reinit_early_sriov(struct amdgpu_device 
>> *adev)
>> +/** amdgpu_device_is_early_ip_block_sriov - check for early 
>> +ip_blocks
>> + *
>> + * @ip_block: ip block that need to be check
>> + *
>> + * Returns a tri-state value for a given ip block.
>> + * If an ip block requires early reinit sriov then return 1 or 0 otherwise.
>> + * Return -1 on invalid ip block.
>> + *
>> + */
>> +
>> +static int
>> +amdgpu_device_is_early_ip_block_sriov(const enum amd_ip_block_type
>> +ip_block)
>> {
>> -int i, r;
>> +switch (ip_block) {
>> +/* early ip blocks */
>> +case AMD_IP_BLOCK_TYPE_GMC:
>> +case AMD_IP_BLOCK_TYPE_COMMON:
>> +case AMD_IP_BLOCK_TYPE_PSP:
>> +case AMD_IP_BLOCK_TYPE_IH:
>> +return 1;
>> +/* late ip blocks */
>> +case AMD_IP_BLOCK_TYPE_SMC:
>> +case AMD_IP_BLOCK_TYPE_DCE:
>> +case AMD_IP_BLOCK_TYPE_GFX:
>> +case AMD_IP_BLOCK_TYPE_SDMA:
>> +case AMD_IP_BLOCK_TYPE_UVD:
>> +case AMD_IP_BLOCK_TYPE_VCE:
>> +case AMD_IP_BLOCK_TYPE_VCN:
>> +return 0;
>> +/* invalid ip block */
>> +default:
>> +return -1;
>> +}
>> +}
>>
>> -static enum amd_ip_block_type ip_order[] = { -AMD_IP_BLOCK_TYPE_GMC, 
>> -AMD_IP_BLOCK_TYPE_COMMON, -AMD_IP_BLOCK_TYPE_PSP, 
>> -AMD_IP_BLOCK_TYPE_IH, -};
>> +static int amdgpu_device_ip_reinit_sriov(struct amdgpu_device *adev,  
>> +const int is_early) { int i;
>>
>> for (i = 0; i < adev->num_ip_blocks; i++) { -int j;
>> +int r = 0;
>> +bool init_ip;
>> struct amdgpu_ip_block *block;
>> +enum amd_ip_block_type ip_block;
>>
>> block = >ip_blocks[i];
>> block->status.hw = false;
>> +ip_block = block->version->type;
>> +init_ip = (is_early ==
>> +   amdgpu_device_is_early_ip_block_sriov(ip_block));
>>
>> -for (j = 0; j < ARRAY_SIZE(ip_order); j++) {
>> -
>> -if (block->version->type != ip_order[j] ||
>> -!block->status.valid)
>> -continue;
>> +if (!init_ip ||
>> +(!is_early && block->status.hw) ||
>> +!block->status.valid)
>> +continue;
>>
>> -r = block->version->funcs->hw_init(adev);
>> -DRM_INFO("RE-INIT-early: %s %s\n", block->version-
>>> funcs->name, r?"failed":"succeeded");
>> -if (r)
>> -return r;
>> -block->status.hw = true;
>> +if (init_ip && (ip_block == AMD_IP_BLOCK_TYPE_SMC)) { r = 
>> +block->version->funcs->resume(adev);
>> +goto show_log;
>> }
>> -}
>> -
>> -return

Re: [PATCH 1/1] drm/amdgpu: rework ip block reinit for sriov

2020-08-28 Thread Nirmoy




On 8/28/20 3:16 AM, Deng, Emily wrote:

[AMD Official Use Only - Internal Distribution Only]

Hi Nirmoy,
 Still think the original logical is more clear.



No problem but we should at least make sure 
amdgpu_device_ip_reinit_late_sriov() and 
amdgpu_device_ip_reinit_early_sriov()


are consistent. The last patch from Jiawei only touched 
amdgpu_device_ip_reinit_early_sriov(), same optimization should apply


to amdgpu_device_ip_reinit_late_sriov()


Regards,

Nirmoy




Best wishes
Emily Deng




-Original Message-
From: Das, Nirmoy 
Sent: Thursday, August 27, 2020 11:19 PM
To: amd-gfx@lists.freedesktop.org
Cc: Deucher, Alexander ; Liu, Monk
; Gu, JiaWei (Will) ; Deng, Emily
; Das, Nirmoy 
Subject: [PATCH 1/1] drm/amdgpu: rework ip block reinit for sriov

This patch removes some unwanted code duplication and simplifies sriov's ip
block reinit logic.

Signed-off-by: Nirmoy Das 
---
drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 117 +++--
1 file changed, 60 insertions(+), 57 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
index 696a61cc3ac6..0db6db03bcd3 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
@@ -2587,77 +2587,80 @@ int amdgpu_device_ip_suspend(struct
amdgpu_device *adev)
return r;
}

-static int amdgpu_device_ip_reinit_early_sriov(struct amdgpu_device *adev)
+/** amdgpu_device_is_early_ip_block_sriov - check for early ip_blocks
+ *
+ * @ip_block: ip block that need to be check
+ *
+ * Returns a tri-state value for a given ip block.
+ * If an ip block requires early reinit sriov then return 1 or 0 otherwise.
+ * Return -1 on invalid ip block.
+ *
+ */
+
+static int
+amdgpu_device_is_early_ip_block_sriov(const enum amd_ip_block_type
+ip_block)
{
-int i, r;
+switch (ip_block) {
+/* early ip blocks */
+case AMD_IP_BLOCK_TYPE_GMC:
+case AMD_IP_BLOCK_TYPE_COMMON:
+case AMD_IP_BLOCK_TYPE_PSP:
+case AMD_IP_BLOCK_TYPE_IH:
+return 1;
+/* late ip blocks */
+case AMD_IP_BLOCK_TYPE_SMC:
+case AMD_IP_BLOCK_TYPE_DCE:
+case AMD_IP_BLOCK_TYPE_GFX:
+case AMD_IP_BLOCK_TYPE_SDMA:
+case AMD_IP_BLOCK_TYPE_UVD:
+case AMD_IP_BLOCK_TYPE_VCE:
+case AMD_IP_BLOCK_TYPE_VCN:
+return 0;
+/* invalid ip block */
+default:
+return -1;
+}
+}

-static enum amd_ip_block_type ip_order[] = {
-AMD_IP_BLOCK_TYPE_GMC,
-AMD_IP_BLOCK_TYPE_COMMON,
-AMD_IP_BLOCK_TYPE_PSP,
-AMD_IP_BLOCK_TYPE_IH,
-};
+static int amdgpu_device_ip_reinit_sriov(struct amdgpu_device *adev,
+ const int is_early)
+{
+int i;

for (i = 0; i < adev->num_ip_blocks; i++) {
-int j;
+int r = 0;
+bool init_ip;
struct amdgpu_ip_block *block;
+enum amd_ip_block_type ip_block;

block = >ip_blocks[i];
block->status.hw = false;
+ip_block = block->version->type;
+init_ip = (is_early ==
+   amdgpu_device_is_early_ip_block_sriov(ip_block));

-for (j = 0; j < ARRAY_SIZE(ip_order); j++) {
-
-if (block->version->type != ip_order[j] ||
-!block->status.valid)
-continue;
+if (!init_ip ||
+(!is_early && block->status.hw) ||
+!block->status.valid)
+continue;

-r = block->version->funcs->hw_init(adev);
-DRM_INFO("RE-INIT-early: %s %s\n", block->version-

funcs->name, r?"failed":"succeeded");

-if (r)
-return r;
-block->status.hw = true;
+if (init_ip && (ip_block == AMD_IP_BLOCK_TYPE_SMC)) {
+r = block->version->funcs->resume(adev);
+goto show_log;
}
-}
-
-return 0;
-}

-static int amdgpu_device_ip_reinit_late_sriov(struct amdgpu_device *adev) -{
-int i, r;
-
-static enum amd_ip_block_type ip_order[] = {
-AMD_IP_BLOCK_TYPE_SMC,
-AMD_IP_BLOCK_TYPE_DCE,
-AMD_IP_BLOCK_TYPE_GFX,
-AMD_IP_BLOCK_TYPE_SDMA,
-AMD_IP_BLOCK_TYPE_UVD,
-AMD_IP_BLOCK_TYPE_VCE,
-AMD_IP_BLOCK_TYPE_VCN
-};
-
-for (i = 0; i < ARRAY_SIZE(ip_order); i++) {
-int j;
-struct amdgpu_ip_block *block;
+if (init_ip)
+r = block->version->funcs->hw_init(adev);

-for (j = 0; j < adev->num_ip_blocks; j++) {
-block = >ip_blocks[j];
+show_log:
+DRM_INFO("RE-INIT-%s: %s %s\n", is_early ? "early":"late",
+ block->version->funcs->name, r ?
"failed":"succeeded");

-if (block->version->type != ip_order[i] ||
-!block->status.valid ||
-block->status.hw)
-continue;
+if (r)
+return r;

-if (block->version->type ==
AMD_IP_BLOCK_TYPE_SMC)
-r = block->version->funcs->resume(adev);
-else
-r = block->version->funcs->hw_init(adev);
+block->status.hw = true;

-DRM_INFO("RE-INIT-late: %s %s\n", block->version-

funcs->name, r?"failed":"succeeded");

-if (r)
-return r;
-block->status.hw = true;
-}
}

return 0;
@@ -3901,7 +3904,7 @@ static int amdgpu_device_reset_sriov(struct
amdgpu_device *adev,
amdgpu_amdkfd_pre_reset(adev);

/* Resume IP prior to SMC */
-r = amdgpu_device_ip_reinit_early_sriov(adev);
+r = amdgpu_device_ip_reinit_sriov(adev, 1);
if (r)
goto error;

@@ -3914,7 +3917,7 @@ static int amdgpu_device_reset_sriov(struct
amdgpu_device *adev,
return r;

/* now we are okay to resume SMC/CP/SDMA */
-r = amdgpu_device_ip_reinit_late_sriov(adev);
+r = amdgpu_device_ip_reinit_sriov(adev, 0);
if

RE: [PATCH 1/1] drm/amdgpu: rework ip block reinit for sriov

2020-08-27 Thread Deng, Emily

[AMD Official Use Only - Internal Distribution Only]

Hi Nirmoy,
Still think the original logical is more clear.

Best wishes
Emily Deng



>-Original Message-
>From: Das, Nirmoy 
>Sent: Thursday, August 27, 2020 11:19 PM
>To: amd-gfx@lists.freedesktop.org
>Cc: Deucher, Alexander ; Liu, Monk
>; Gu, JiaWei (Will) ; Deng, Emily
>; Das, Nirmoy 
>Subject: [PATCH 1/1] drm/amdgpu: rework ip block reinit for sriov
>
>This patch removes some unwanted code duplication and simplifies sriov's ip
>block reinit logic.
>
>Signed-off-by: Nirmoy Das 
>---
> drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 117 +++--
> 1 file changed, 60 insertions(+), 57 deletions(-)
>
>diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>index 696a61cc3ac6..0db6db03bcd3 100644
>--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
>@@ -2587,77 +2587,80 @@ int amdgpu_device_ip_suspend(struct
>amdgpu_device *adev)
> return r;
> }
>
>-static int amdgpu_device_ip_reinit_early_sriov(struct amdgpu_device *adev)
>+/** amdgpu_device_is_early_ip_block_sriov - check for early ip_blocks
>+ *
>+ * @ip_block: ip block that need to be check
>+ *
>+ * Returns a tri-state value for a given ip block.
>+ * If an ip block requires early reinit sriov then return 1 or 0 otherwise.
>+ * Return -1 on invalid ip block.
>+ *
>+ */
>+
>+static int
>+amdgpu_device_is_early_ip_block_sriov(const enum amd_ip_block_type
>+ip_block)
> {
>-int i, r;
>+switch (ip_block) {
>+/* early ip blocks */
>+case AMD_IP_BLOCK_TYPE_GMC:
>+case AMD_IP_BLOCK_TYPE_COMMON:
>+case AMD_IP_BLOCK_TYPE_PSP:
>+case AMD_IP_BLOCK_TYPE_IH:
>+return 1;
>+/* late ip blocks */
>+case AMD_IP_BLOCK_TYPE_SMC:
>+case AMD_IP_BLOCK_TYPE_DCE:
>+case AMD_IP_BLOCK_TYPE_GFX:
>+case AMD_IP_BLOCK_TYPE_SDMA:
>+case AMD_IP_BLOCK_TYPE_UVD:
>+case AMD_IP_BLOCK_TYPE_VCE:
>+case AMD_IP_BLOCK_TYPE_VCN:
>+return 0;
>+/* invalid ip block */
>+default:
>+return -1;
>+}
>+}
>
>-static enum amd_ip_block_type ip_order[] = {
>-AMD_IP_BLOCK_TYPE_GMC,
>-AMD_IP_BLOCK_TYPE_COMMON,
>-AMD_IP_BLOCK_TYPE_PSP,
>-AMD_IP_BLOCK_TYPE_IH,
>-};
>+static int amdgpu_device_ip_reinit_sriov(struct amdgpu_device *adev,
>+ const int is_early)
>+{
>+int i;
>
> for (i = 0; i < adev->num_ip_blocks; i++) {
>-int j;
>+int r = 0;
>+bool init_ip;
> struct amdgpu_ip_block *block;
>+enum amd_ip_block_type ip_block;
>
> block = >ip_blocks[i];
> block->status.hw = false;
>+ip_block = block->version->type;
>+init_ip = (is_early ==
>+   amdgpu_device_is_early_ip_block_sriov(ip_block));
>
>-for (j = 0; j < ARRAY_SIZE(ip_order); j++) {
>-
>-if (block->version->type != ip_order[j] ||
>-!block->status.valid)
>-continue;
>+if (!init_ip ||
>+(!is_early && block->status.hw) ||
>+!block->status.valid)
>+continue;
>
>-r = block->version->funcs->hw_init(adev);
>-DRM_INFO("RE-INIT-early: %s %s\n", block->version-
>>funcs->name, r?"failed":"succeeded");
>-if (r)
>-return r;
>-block->status.hw = true;
>+if (init_ip && (ip_block == AMD_IP_BLOCK_TYPE_SMC)) {
>+r = block->version->funcs->resume(adev);
>+goto show_log;
> }
>-}
>-
>-return 0;
>-}
>
>-static int amdgpu_device_ip_reinit_late_sriov(struct amdgpu_device *adev) -{
>-int i, r;
>-
>-static enum amd_ip_block_type ip_order[] = {
>-AMD_IP_BLOCK_TYPE_SMC,
>-AMD_IP_BLOCK_TYPE_DCE,
>-AMD_IP_BLOCK_TYPE_GFX,
>-AMD_IP_BLOCK_TYPE_SDMA,
>-AMD_IP_BLOCK_TYPE_UVD,
>-AMD_IP_BLOCK_TYPE_VCE,
>-AMD_IP_BLOCK_TYPE_VCN
>-};
>-
>-for (i = 0; i < ARRAY_SIZE(ip_order); i++) {
>-int j;
>-struct amdgpu_ip_block *block;
>+if (init_ip)
>+r = block->version->funcs->hw_init(adev);
>
>-for (j = 0; j < adev->num_ip_blocks; j++) {
>-block = >ip_blocks[j];
>+show_log:
>+DRM_INFO("RE-INIT-%s: %s %s\n", is_early ? "early":"late",
>+ block->version->funcs->name, r ?
>"failed":"succeeded");
>
>-if (block->version->type != ip_order[i] ||
>-!block->status.valid ||
>-block->status.hw)
>-continue;
>+if (r)
>+return r;
>
>-if (block->version->type ==
>AMD_IP_BLOCK_TYPE_SMC)
>-r = block->version->funcs->resume(adev);
>-else
>-r = block->version->funcs->hw_init(adev);
>+block->status.hw = true;
>
>-DRM_INFO("RE-INIT-late: %s %s\n", block->version-
>>funcs->name, r?"failed":"succeeded");
>-if (r)
>-return r;
>-block->status.hw = true;
>-}
> }
>
> return 0;
>@@ -3901,7 +3904,7 @@ static int amdgpu_device_reset_sriov(struct
>amdgpu_device *adev,
> amdgpu_amdkfd_pre_reset(adev);
>
> /* Resume IP prior to SMC */
>-r = amdgpu_device_ip_reinit_early_sriov(adev);
>+r = amdgpu_device_ip_reinit_sriov(adev, 1);
> if (r)
> goto error;
>
>@@ -3914,7 +3917,7 @@ static int amdgpu_device_reset_sriov(struct
>amdgpu_device *adev,
> return r;
>
> /* now we are okay to resume SMC/CP/SDMA */
>-r = amdgpu_device_ip_reinit_late_sriov(adev);
>+r = amdgpu_device_ip_reinit_sriov(adev, 0);
> if (r)
> goto error;
>
>--
>2.28.0

___
amd-gfx mailing list
amd-gfx@lists.freedesktop.org

Re: [PATCH 1/1] drm/amdgpu: rework ip block reinit for sriov

RE: [PATCH 1/1] drm/amdgpu: rework ip block reinit for sriov

RE: [PATCH 1/1] drm/amdgpu: rework ip block reinit for sriov

Re: [PATCH 1/1] drm/amdgpu: rework ip block reinit for sriov

RE: [PATCH 1/1] drm/amdgpu: rework ip block reinit for sriov

Re: [PATCH 1/1] drm/amdgpu: rework ip block reinit for sriov

RE: [PATCH 1/1] drm/amdgpu: rework ip block reinit for sriov

7 matches

Site Navigation

Mail list logo

Footer information