* [PATCH] drm/amdgpu: limit VCN instance number to 1 for NAVY_FLOUNDER
@ 2021-10-21 7:15 Guchun Chen
2021-10-21 9:44 ` Lazar, Lijo
2021-10-21 13:05 ` Alex Deucher
0 siblings, 2 replies; 10+ messages in thread
From: Guchun Chen @ 2021-10-21 7:15 UTC (permalink / raw)
To: amd-gfx, christian.koenig, xinhui.pan, alexander.deucher, leo.liu
Cc: Guchun Chen
VCN instance 1 is power gated permanently by SMU.
Bug: https://gitlab.freedesktop.org/drm/amd/-/issues/1743
Fixes: f6b6d7d6bc2d("drm/amdgpu/vcn: remove manual instance setting")
Signed-off-by: Guchun Chen <guchun.chen@amd.com>
---
drivers/gpu/drm/amd/amdgpu/vcn_v3_0.c | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/drivers/gpu/drm/amd/amdgpu/vcn_v3_0.c b/drivers/gpu/drm/amd/amdgpu/vcn_v3_0.c
index dbfd92984655..4848922667f2 100644
--- a/drivers/gpu/drm/amd/amdgpu/vcn_v3_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/vcn_v3_0.c
@@ -103,6 +103,15 @@ static int vcn_v3_0_early_init(void *handle)
adev->vcn.num_enc_rings = 0;
else
adev->vcn.num_enc_rings = 2;
+
+ /*
+ * Fix ME.
+ * VCN instance number is limited to 1 for below ASIC due to
+ * VCN instnace 1 is permanently power gated.
+ */
+ if ((adev->ip_versions[UVD_HWIP][0] == IP_VERSION(3, 0, 0)) &&
+ (adev->ip_versions[GC_HWIP][0] == IP_VERSION(10, 3, 2)))
+ adev->vcn.num_vcn_inst = 1;
}
vcn_v3_0_set_dec_ring_funcs(adev);
--
2.17.1
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH] drm/amdgpu: limit VCN instance number to 1 for NAVY_FLOUNDER
2021-10-21 7:15 [PATCH] drm/amdgpu: limit VCN instance number to 1 for NAVY_FLOUNDER Guchun Chen
@ 2021-10-21 9:44 ` Lazar, Lijo
2021-10-21 12:40 ` Chen, Guchun
2021-10-21 13:05 ` Alex Deucher
1 sibling, 1 reply; 10+ messages in thread
From: Lazar, Lijo @ 2021-10-21 9:44 UTC (permalink / raw)
To: Guchun Chen, amd-gfx, christian.koenig, xinhui.pan,
alexander.deucher, leo.liu
On 10/21/2021 12:45 PM, Guchun Chen wrote:
> VCN instance 1 is power gated permanently by SMU.
>
> Bug: https://gitlab.freedesktop.org/drm/amd/-/issues/1743
>
> Fixes: f6b6d7d6bc2d("drm/amdgpu/vcn: remove manual instance setting")
Nice find. Looking at the fix, the logic is already broken by
5e26e52adb46("drm/amdgpu/vcn3.0: convert to IP version checking")
Any ASIC other than Sienna which has same VCN IP version (3.0.0) may be
broken. Any more extra checks?
Thanks,
Lijo
> Signed-off-by: Guchun Chen <guchun.chen@amd.com>
> ---
> drivers/gpu/drm/amd/amdgpu/vcn_v3_0.c | 9 +++++++++
> 1 file changed, 9 insertions(+)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/vcn_v3_0.c b/drivers/gpu/drm/amd/amdgpu/vcn_v3_0.c
> index dbfd92984655..4848922667f2 100644
> --- a/drivers/gpu/drm/amd/amdgpu/vcn_v3_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/vcn_v3_0.c
> @@ -103,6 +103,15 @@ static int vcn_v3_0_early_init(void *handle)
> adev->vcn.num_enc_rings = 0;
> else
> adev->vcn.num_enc_rings = 2;
> +
> + /*
> + * Fix ME.
> + * VCN instance number is limited to 1 for below ASIC due to
> + * VCN instnace 1 is permanently power gated.
> + */
> + if ((adev->ip_versions[UVD_HWIP][0] == IP_VERSION(3, 0, 0)) &&
> + (adev->ip_versions[GC_HWIP][0] == IP_VERSION(10, 3, 2)))
> + adev->vcn.num_vcn_inst = 1;
> }
>
> vcn_v3_0_set_dec_ring_funcs(adev);
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* RE: [PATCH] drm/amdgpu: limit VCN instance number to 1 for NAVY_FLOUNDER
2021-10-21 9:44 ` Lazar, Lijo
@ 2021-10-21 12:40 ` Chen, Guchun
2021-10-21 12:56 ` Lazar, Lijo
0 siblings, 1 reply; 10+ messages in thread
From: Chen, Guchun @ 2021-10-21 12:40 UTC (permalink / raw)
To: Lazar, Lijo, Koenig, Christian, Pan, Xinhui, Deucher, Alexander,
Liu, Leo, amd-gfx
Hi Lijo,
Alex has a following fix "85db7fcb2e53 drm/amdgpu: get VCN harvest information from IP discovery table" to fix that logic.
For other ASCIs like DIMGREY_CAVEFISH and BEIGE_GOBY, its instance num is 1, match with VBIOS discovery table. So there is no need to handle it.
Regards,
Guchun
-----Original Message-----
From: Lazar, Lijo <Lijo.Lazar@amd.com>
Sent: Thursday, October 21, 2021 5:45 PM
To: Chen, Guchun <Guchun.Chen@amd.com>; amd-gfx@lists.freedesktop.org; Koenig, Christian <Christian.Koenig@amd.com>; Pan, Xinhui <Xinhui.Pan@amd.com>; Deucher, Alexander <Alexander.Deucher@amd.com>; Liu, Leo <Leo.Liu@amd.com>
Subject: Re: [PATCH] drm/amdgpu: limit VCN instance number to 1 for NAVY_FLOUNDER
On 10/21/2021 12:45 PM, Guchun Chen wrote:
> VCN instance 1 is power gated permanently by SMU.
>
> Bug: https://gitlab.freedesktop.org/drm/amd/-/issues/1743
>
> Fixes: f6b6d7d6bc2d("drm/amdgpu/vcn: remove manual instance setting")
Nice find. Looking at the fix, the logic is already broken by
5e26e52adb46("drm/amdgpu/vcn3.0: convert to IP version checking")
Any ASIC other than Sienna which has same VCN IP version (3.0.0) may be broken. Any more extra checks?
Thanks,
Lijo
> Signed-off-by: Guchun Chen <guchun.chen@amd.com>
> ---
> drivers/gpu/drm/amd/amdgpu/vcn_v3_0.c | 9 +++++++++
> 1 file changed, 9 insertions(+)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/vcn_v3_0.c
> b/drivers/gpu/drm/amd/amdgpu/vcn_v3_0.c
> index dbfd92984655..4848922667f2 100644
> --- a/drivers/gpu/drm/amd/amdgpu/vcn_v3_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/vcn_v3_0.c
> @@ -103,6 +103,15 @@ static int vcn_v3_0_early_init(void *handle)
> adev->vcn.num_enc_rings = 0;
> else
> adev->vcn.num_enc_rings = 2;
> +
> + /*
> + * Fix ME.
> + * VCN instance number is limited to 1 for below ASIC due to
> + * VCN instnace 1 is permanently power gated.
> + */
> + if ((adev->ip_versions[UVD_HWIP][0] == IP_VERSION(3, 0, 0)) &&
> + (adev->ip_versions[GC_HWIP][0] == IP_VERSION(10, 3, 2)))
> + adev->vcn.num_vcn_inst = 1;
> }
>
> vcn_v3_0_set_dec_ring_funcs(adev);
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] drm/amdgpu: limit VCN instance number to 1 for NAVY_FLOUNDER
2021-10-21 12:40 ` Chen, Guchun
@ 2021-10-21 12:56 ` Lazar, Lijo
2021-10-21 13:00 ` Chen, Guchun
0 siblings, 1 reply; 10+ messages in thread
From: Lazar, Lijo @ 2021-10-21 12:56 UTC (permalink / raw)
To: Chen, Guchun, Koenig, Christian, Pan, Xinhui, Deucher, Alexander,
Liu, Leo, amd-gfx
On 10/21/2021 6:10 PM, Chen, Guchun wrote:
> Hi Lijo,
>
> Alex has a following fix "85db7fcb2e53 drm/amdgpu: get VCN harvest information from IP discovery table" to fix that logic.
But the logic applied in this fix tells that anything in IP discovery
(version table or harvest table) doesn't solve the problem. This is
equivalent to an ASIC specific logic similar to old ASIC enum checks.
>
> For other ASCIs like DIMGREY_CAVEFISH and BEIGE_GOBY, its instance num is 1, match with VBIOS discovery table. So there is no need to handle it.
>
Thanks for the clarification! It looks good to me, will leave it to
Alex/Leo/James.
Thanks,
Lijo
> Regards,
> Guchun
>
> -----Original Message-----
> From: Lazar, Lijo <Lijo.Lazar@amd.com>
> Sent: Thursday, October 21, 2021 5:45 PM
> To: Chen, Guchun <Guchun.Chen@amd.com>; amd-gfx@lists.freedesktop.org; Koenig, Christian <Christian.Koenig@amd.com>; Pan, Xinhui <Xinhui.Pan@amd.com>; Deucher, Alexander <Alexander.Deucher@amd.com>; Liu, Leo <Leo.Liu@amd.com>
> Subject: Re: [PATCH] drm/amdgpu: limit VCN instance number to 1 for NAVY_FLOUNDER
>
>
>
> On 10/21/2021 12:45 PM, Guchun Chen wrote:
>> VCN instance 1 is power gated permanently by SMU.
>>
>> Bug: https://gitlab.freedesktop.org/drm/amd/-/issues/1743
>>
>> Fixes: f6b6d7d6bc2d("drm/amdgpu/vcn: remove manual instance setting")
>
> Nice find. Looking at the fix, the logic is already broken by
> 5e26e52adb46("drm/amdgpu/vcn3.0: convert to IP version checking")
>
> Any ASIC other than Sienna which has same VCN IP version (3.0.0) may be broken. Any more extra checks?
>
> Thanks,
> Lijo
>
>> Signed-off-by: Guchun Chen <guchun.chen@amd.com>
>> ---
>> drivers/gpu/drm/amd/amdgpu/vcn_v3_0.c | 9 +++++++++
>> 1 file changed, 9 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/vcn_v3_0.c
>> b/drivers/gpu/drm/amd/amdgpu/vcn_v3_0.c
>> index dbfd92984655..4848922667f2 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/vcn_v3_0.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/vcn_v3_0.c
>> @@ -103,6 +103,15 @@ static int vcn_v3_0_early_init(void *handle)
>> adev->vcn.num_enc_rings = 0;
>> else
>> adev->vcn.num_enc_rings = 2;
>> +
>> + /*
>> + * Fix ME.
>> + * VCN instance number is limited to 1 for below ASIC due to
>> + * VCN instnace 1 is permanently power gated.
>> + */
>> + if ((adev->ip_versions[UVD_HWIP][0] == IP_VERSION(3, 0, 0)) &&
>> + (adev->ip_versions[GC_HWIP][0] == IP_VERSION(10, 3, 2)))
>> + adev->vcn.num_vcn_inst = 1;
>> }
>>
>> vcn_v3_0_set_dec_ring_funcs(adev);
>>
^ permalink raw reply [flat|nested] 10+ messages in thread
* RE: [PATCH] drm/amdgpu: limit VCN instance number to 1 for NAVY_FLOUNDER
2021-10-21 12:56 ` Lazar, Lijo
@ 2021-10-21 13:00 ` Chen, Guchun
0 siblings, 0 replies; 10+ messages in thread
From: Chen, Guchun @ 2021-10-21 13:00 UTC (permalink / raw)
To: Lazar, Lijo, Koenig, Christian, Pan, Xinhui, Deucher, Alexander,
Liu, Leo, amd-gfx
Re: But the logic applied in this fix tells that anything in IP discovery (version table or harvest table) doesn't solve the problem. This is equivalent to an ASIC specific logic similar to old ASIC enum checks.
Exactly, this is the challenge.
Regards,
Guchun
-----Original Message-----
From: Lazar, Lijo <Lijo.Lazar@amd.com>
Sent: Thursday, October 21, 2021 8:56 PM
To: Chen, Guchun <Guchun.Chen@amd.com>; Koenig, Christian <Christian.Koenig@amd.com>; Pan, Xinhui <Xinhui.Pan@amd.com>; Deucher, Alexander <Alexander.Deucher@amd.com>; Liu, Leo <Leo.Liu@amd.com>; amd-gfx@lists.freedesktop.org
Subject: Re: [PATCH] drm/amdgpu: limit VCN instance number to 1 for NAVY_FLOUNDER
On 10/21/2021 6:10 PM, Chen, Guchun wrote:
> Hi Lijo,
>
> Alex has a following fix "85db7fcb2e53 drm/amdgpu: get VCN harvest information from IP discovery table" to fix that logic.
But the logic applied in this fix tells that anything in IP discovery (version table or harvest table) doesn't solve the problem. This is equivalent to an ASIC specific logic similar to old ASIC enum checks.
>
> For other ASCIs like DIMGREY_CAVEFISH and BEIGE_GOBY, its instance num is 1, match with VBIOS discovery table. So there is no need to handle it.
>
Thanks for the clarification! It looks good to me, will leave it to Alex/Leo/James.
Thanks,
Lijo
> Regards,
> Guchun
>
> -----Original Message-----
> From: Lazar, Lijo <Lijo.Lazar@amd.com>
> Sent: Thursday, October 21, 2021 5:45 PM
> To: Chen, Guchun <Guchun.Chen@amd.com>; amd-gfx@lists.freedesktop.org;
> Koenig, Christian <Christian.Koenig@amd.com>; Pan, Xinhui
> <Xinhui.Pan@amd.com>; Deucher, Alexander <Alexander.Deucher@amd.com>;
> Liu, Leo <Leo.Liu@amd.com>
> Subject: Re: [PATCH] drm/amdgpu: limit VCN instance number to 1 for
> NAVY_FLOUNDER
>
>
>
> On 10/21/2021 12:45 PM, Guchun Chen wrote:
>> VCN instance 1 is power gated permanently by SMU.
>>
>> Bug: https://gitlab.freedesktop.org/drm/amd/-/issues/1743
>>
>> Fixes: f6b6d7d6bc2d("drm/amdgpu/vcn: remove manual instance setting")
>
> Nice find. Looking at the fix, the logic is already broken by
> 5e26e52adb46("drm/amdgpu/vcn3.0: convert to IP version checking")
>
> Any ASIC other than Sienna which has same VCN IP version (3.0.0) may be broken. Any more extra checks?
>
> Thanks,
> Lijo
>
>> Signed-off-by: Guchun Chen <guchun.chen@amd.com>
>> ---
>> drivers/gpu/drm/amd/amdgpu/vcn_v3_0.c | 9 +++++++++
>> 1 file changed, 9 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/vcn_v3_0.c
>> b/drivers/gpu/drm/amd/amdgpu/vcn_v3_0.c
>> index dbfd92984655..4848922667f2 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/vcn_v3_0.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/vcn_v3_0.c
>> @@ -103,6 +103,15 @@ static int vcn_v3_0_early_init(void *handle)
>> adev->vcn.num_enc_rings = 0;
>> else
>> adev->vcn.num_enc_rings = 2;
>> +
>> + /*
>> + * Fix ME.
>> + * VCN instance number is limited to 1 for below ASIC due to
>> + * VCN instnace 1 is permanently power gated.
>> + */
>> + if ((adev->ip_versions[UVD_HWIP][0] == IP_VERSION(3, 0, 0)) &&
>> + (adev->ip_versions[GC_HWIP][0] == IP_VERSION(10, 3, 2)))
>> + adev->vcn.num_vcn_inst = 1;
>> }
>>
>> vcn_v3_0_set_dec_ring_funcs(adev);
>>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] drm/amdgpu: limit VCN instance number to 1 for NAVY_FLOUNDER
2021-10-21 7:15 [PATCH] drm/amdgpu: limit VCN instance number to 1 for NAVY_FLOUNDER Guchun Chen
2021-10-21 9:44 ` Lazar, Lijo
@ 2021-10-21 13:05 ` Alex Deucher
2021-10-21 13:14 ` Chen, Guchun
1 sibling, 1 reply; 10+ messages in thread
From: Alex Deucher @ 2021-10-21 13:05 UTC (permalink / raw)
To: Guchun Chen
Cc: amd-gfx list, Christian Koenig, xinhui pan, Deucher, Alexander, Leo Liu
On Thu, Oct 21, 2021 at 3:15 AM Guchun Chen <guchun.chen@amd.com> wrote:
>
> VCN instance 1 is power gated permanently by SMU.
>
> Bug: https://gitlab.freedesktop.org/drm/amd/-/issues/1743
>
> Fixes: f6b6d7d6bc2d("drm/amdgpu/vcn: remove manual instance setting")
> Signed-off-by: Guchun Chen <guchun.chen@amd.com>
Doesn't this patch effectively do the same thing?
https://patchwork.freedesktop.org/patch/460329/
Where else is num_vcn_inst used that it causes a problem? Or is the
VCN harvesting not set correctly on some navy flounders?
Alex
> ---
> drivers/gpu/drm/amd/amdgpu/vcn_v3_0.c | 9 +++++++++
> 1 file changed, 9 insertions(+)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/vcn_v3_0.c b/drivers/gpu/drm/amd/amdgpu/vcn_v3_0.c
> index dbfd92984655..4848922667f2 100644
> --- a/drivers/gpu/drm/amd/amdgpu/vcn_v3_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/vcn_v3_0.c
> @@ -103,6 +103,15 @@ static int vcn_v3_0_early_init(void *handle)
> adev->vcn.num_enc_rings = 0;
> else
> adev->vcn.num_enc_rings = 2;
> +
> + /*
> + * Fix ME.
> + * VCN instance number is limited to 1 for below ASIC due to
> + * VCN instnace 1 is permanently power gated.
> + */
> + if ((adev->ip_versions[UVD_HWIP][0] == IP_VERSION(3, 0, 0)) &&
> + (adev->ip_versions[GC_HWIP][0] == IP_VERSION(10, 3, 2)))
> + adev->vcn.num_vcn_inst = 1;
> }
>
> vcn_v3_0_set_dec_ring_funcs(adev);
> --
> 2.17.1
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* RE: [PATCH] drm/amdgpu: limit VCN instance number to 1 for NAVY_FLOUNDER
2021-10-21 13:05 ` Alex Deucher
@ 2021-10-21 13:14 ` Chen, Guchun
2021-10-21 13:34 ` Chen, Guchun
0 siblings, 1 reply; 10+ messages in thread
From: Chen, Guchun @ 2021-10-21 13:14 UTC (permalink / raw)
To: Alex Deucher
Cc: amd-gfx list, Koenig, Christian, Pan, Xinhui, Deucher, Alexander,
Liu, Leo
Hi Alex,
No, it does not help.
adev->vcn.harvest_config is 0 after retrieving harvest info from VBIOS. Looks that harvest info in VBIOs does not reflect the case that VCN1 is power gated.
I checked several navy flounders SKUs, the observation is the same, so this is likely a common case. Perhaps we need to check with VBIOS/SMU guys.
Regards,
Guchun
-----Original Message-----
From: Alex Deucher <alexdeucher@gmail.com>
Sent: Thursday, October 21, 2021 9:06 PM
To: Chen, Guchun <Guchun.Chen@amd.com>
Cc: amd-gfx list <amd-gfx@lists.freedesktop.org>; Koenig, Christian <Christian.Koenig@amd.com>; Pan, Xinhui <Xinhui.Pan@amd.com>; Deucher, Alexander <Alexander.Deucher@amd.com>; Liu, Leo <Leo.Liu@amd.com>
Subject: Re: [PATCH] drm/amdgpu: limit VCN instance number to 1 for NAVY_FLOUNDER
On Thu, Oct 21, 2021 at 3:15 AM Guchun Chen <guchun.chen@amd.com> wrote:
>
> VCN instance 1 is power gated permanently by SMU.
>
> Bug:
> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgitl
> ab.freedesktop.org%2Fdrm%2Famd%2F-%2Fissues%2F1743&data=04%7C01%7C
> guchun.chen%40amd.com%7Cda80a308a28049d543ad08d99493847d%7C3dd8961fe48
> 84e608e11a82d994e183d%7C0%7C0%7C637704183581593964%7CUnknown%7CTWFpbGZ
> sb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3
> D%7C1000&sdata=2vNLj9bXE2oV97rxBiUOiaFNpKopVSJefL%2BMcQE%2BSfo%3D&
> amp;reserved=0
>
> Fixes: f6b6d7d6bc2d("drm/amdgpu/vcn: remove manual instance setting")
> Signed-off-by: Guchun Chen <guchun.chen@amd.com>
Doesn't this patch effectively do the same thing?
https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fpatchwork.freedesktop.org%2Fpatch%2F460329%2F&data=04%7C01%7Cguchun.chen%40amd.com%7Cda80a308a28049d543ad08d99493847d%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637704183581593964%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=jPu3Yh%2B6OHR4F1BS5MWL3VyZ3pui6c0dP97Zl7yBJKY%3D&reserved=0
Where else is num_vcn_inst used that it causes a problem? Or is the VCN harvesting not set correctly on some navy flounders?
Alex
> ---
> drivers/gpu/drm/amd/amdgpu/vcn_v3_0.c | 9 +++++++++
> 1 file changed, 9 insertions(+)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/vcn_v3_0.c
> b/drivers/gpu/drm/amd/amdgpu/vcn_v3_0.c
> index dbfd92984655..4848922667f2 100644
> --- a/drivers/gpu/drm/amd/amdgpu/vcn_v3_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/vcn_v3_0.c
> @@ -103,6 +103,15 @@ static int vcn_v3_0_early_init(void *handle)
> adev->vcn.num_enc_rings = 0;
> else
> adev->vcn.num_enc_rings = 2;
> +
> + /*
> + * Fix ME.
> + * VCN instance number is limited to 1 for below ASIC due to
> + * VCN instnace 1 is permanently power gated.
> + */
> + if ((adev->ip_versions[UVD_HWIP][0] == IP_VERSION(3, 0, 0)) &&
> + (adev->ip_versions[GC_HWIP][0] == IP_VERSION(10, 3, 2)))
> + adev->vcn.num_vcn_inst = 1;
> }
>
> vcn_v3_0_set_dec_ring_funcs(adev);
> --
> 2.17.1
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* RE: [PATCH] drm/amdgpu: limit VCN instance number to 1 for NAVY_FLOUNDER
2021-10-21 13:14 ` Chen, Guchun
@ 2021-10-21 13:34 ` Chen, Guchun
2021-10-21 14:01 ` Alex Deucher
0 siblings, 1 reply; 10+ messages in thread
From: Chen, Guchun @ 2021-10-21 13:34 UTC (permalink / raw)
To: Alex Deucher
Cc: amd-gfx list, Koenig, Christian, Pan, Xinhui, Deucher, Alexander,
Liu, Leo
Additionally, in sienna_cichlid_dpm_set_vcn_enable, we also use num_vcn_inst to set dpm for VCN1 if it's > 1.
The main problem here is VCN harvest info is not set correctly, so vcn.harvest_config is not reliable in this case.
if (smu_cmn_feature_is_enabled(smu, SMU_FEATURE_MM_DPM_PG_BIT)) {
ret = smu_cmn_send_smc_msg_with_param(smu, SMU_MSG_PowerUpVcn, 0, NULL);
if (ret)
return ret;
if (adev->vcn.num_vcn_inst > 1) {
ret = smu_cmn_send_smc_msg_with_param(smu, SMU_MSG_PowerUpVcn,
0x10000, NULL);
if (ret)
return ret;
}
}
Regards,
Guchun
-----Original Message-----
From: Chen, Guchun
Sent: Thursday, October 21, 2021 9:14 PM
To: Alex Deucher <alexdeucher@gmail.com>
Cc: amd-gfx list <amd-gfx@lists.freedesktop.org>; Koenig, Christian <Christian.Koenig@amd.com>; Pan, Xinhui <Xinhui.Pan@amd.com>; Deucher, Alexander <Alexander.Deucher@amd.com>; Liu, Leo <Leo.Liu@amd.com>
Subject: RE: [PATCH] drm/amdgpu: limit VCN instance number to 1 for NAVY_FLOUNDER
Hi Alex,
No, it does not help.
adev->vcn.harvest_config is 0 after retrieving harvest info from VBIOS. Looks that harvest info in VBIOs does not reflect the case that VCN1 is power gated.
I checked several navy flounders SKUs, the observation is the same, so this is likely a common case. Perhaps we need to check with VBIOS/SMU guys.
Regards,
Guchun
-----Original Message-----
From: Alex Deucher <alexdeucher@gmail.com>
Sent: Thursday, October 21, 2021 9:06 PM
To: Chen, Guchun <Guchun.Chen@amd.com>
Cc: amd-gfx list <amd-gfx@lists.freedesktop.org>; Koenig, Christian <Christian.Koenig@amd.com>; Pan, Xinhui <Xinhui.Pan@amd.com>; Deucher, Alexander <Alexander.Deucher@amd.com>; Liu, Leo <Leo.Liu@amd.com>
Subject: Re: [PATCH] drm/amdgpu: limit VCN instance number to 1 for NAVY_FLOUNDER
On Thu, Oct 21, 2021 at 3:15 AM Guchun Chen <guchun.chen@amd.com> wrote:
>
> VCN instance 1 is power gated permanently by SMU.
>
> Bug:
> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgitl
> ab.freedesktop.org%2Fdrm%2Famd%2F-%2Fissues%2F1743&data=04%7C01%7C
> guchun.chen%40amd.com%7Cda80a308a28049d543ad08d99493847d%7C3dd8961fe48
> 84e608e11a82d994e183d%7C0%7C0%7C637704183581593964%7CUnknown%7CTWFpbGZ
> sb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3
> D%7C1000&sdata=2vNLj9bXE2oV97rxBiUOiaFNpKopVSJefL%2BMcQE%2BSfo%3D&
> amp;reserved=0
>
> Fixes: f6b6d7d6bc2d("drm/amdgpu/vcn: remove manual instance setting")
> Signed-off-by: Guchun Chen <guchun.chen@amd.com>
Doesn't this patch effectively do the same thing?
https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fpatchwork.freedesktop.org%2Fpatch%2F460329%2F&data=04%7C01%7Cguchun.chen%40amd.com%7Cda80a308a28049d543ad08d99493847d%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637704183581593964%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=jPu3Yh%2B6OHR4F1BS5MWL3VyZ3pui6c0dP97Zl7yBJKY%3D&reserved=0
Where else is num_vcn_inst used that it causes a problem? Or is the VCN harvesting not set correctly on some navy flounders?
Alex
> ---
> drivers/gpu/drm/amd/amdgpu/vcn_v3_0.c | 9 +++++++++
> 1 file changed, 9 insertions(+)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/vcn_v3_0.c
> b/drivers/gpu/drm/amd/amdgpu/vcn_v3_0.c
> index dbfd92984655..4848922667f2 100644
> --- a/drivers/gpu/drm/amd/amdgpu/vcn_v3_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/vcn_v3_0.c
> @@ -103,6 +103,15 @@ static int vcn_v3_0_early_init(void *handle)
> adev->vcn.num_enc_rings = 0;
> else
> adev->vcn.num_enc_rings = 2;
> +
> + /*
> + * Fix ME.
> + * VCN instance number is limited to 1 for below ASIC due to
> + * VCN instnace 1 is permanently power gated.
> + */
> + if ((adev->ip_versions[UVD_HWIP][0] == IP_VERSION(3, 0, 0)) &&
> + (adev->ip_versions[GC_HWIP][0] == IP_VERSION(10, 3, 2)))
> + adev->vcn.num_vcn_inst = 1;
> }
>
> vcn_v3_0_set_dec_ring_funcs(adev);
> --
> 2.17.1
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] drm/amdgpu: limit VCN instance number to 1 for NAVY_FLOUNDER
2021-10-21 13:34 ` Chen, Guchun
@ 2021-10-21 14:01 ` Alex Deucher
2021-10-21 14:09 ` Chen, Guchun
0 siblings, 1 reply; 10+ messages in thread
From: Alex Deucher @ 2021-10-21 14:01 UTC (permalink / raw)
To: Chen, Guchun
Cc: amd-gfx list, Koenig, Christian, Pan, Xinhui, Deucher, Alexander,
Liu, Leo
Thanks. I think this patch set fixes it in a bit more future proof way:
https://patchwork.freedesktop.org/series/96132/
Alex
On Thu, Oct 21, 2021 at 9:34 AM Chen, Guchun <Guchun.Chen@amd.com> wrote:
>
> Additionally, in sienna_cichlid_dpm_set_vcn_enable, we also use num_vcn_inst to set dpm for VCN1 if it's > 1.
> The main problem here is VCN harvest info is not set correctly, so vcn.harvest_config is not reliable in this case.
>
> if (smu_cmn_feature_is_enabled(smu, SMU_FEATURE_MM_DPM_PG_BIT)) {
> ret = smu_cmn_send_smc_msg_with_param(smu, SMU_MSG_PowerUpVcn, 0, NULL);
> if (ret)
> return ret;
> if (adev->vcn.num_vcn_inst > 1) {
> ret = smu_cmn_send_smc_msg_with_param(smu, SMU_MSG_PowerUpVcn,
> 0x10000, NULL);
> if (ret)
> return ret;
> }
> }
>
> Regards,
> Guchun
>
> -----Original Message-----
> From: Chen, Guchun
> Sent: Thursday, October 21, 2021 9:14 PM
> To: Alex Deucher <alexdeucher@gmail.com>
> Cc: amd-gfx list <amd-gfx@lists.freedesktop.org>; Koenig, Christian <Christian.Koenig@amd.com>; Pan, Xinhui <Xinhui.Pan@amd.com>; Deucher, Alexander <Alexander.Deucher@amd.com>; Liu, Leo <Leo.Liu@amd.com>
> Subject: RE: [PATCH] drm/amdgpu: limit VCN instance number to 1 for NAVY_FLOUNDER
>
> Hi Alex,
>
> No, it does not help.
>
> adev->vcn.harvest_config is 0 after retrieving harvest info from VBIOS. Looks that harvest info in VBIOs does not reflect the case that VCN1 is power gated.
>
> I checked several navy flounders SKUs, the observation is the same, so this is likely a common case. Perhaps we need to check with VBIOS/SMU guys.
>
> Regards,
> Guchun
>
> -----Original Message-----
> From: Alex Deucher <alexdeucher@gmail.com>
> Sent: Thursday, October 21, 2021 9:06 PM
> To: Chen, Guchun <Guchun.Chen@amd.com>
> Cc: amd-gfx list <amd-gfx@lists.freedesktop.org>; Koenig, Christian <Christian.Koenig@amd.com>; Pan, Xinhui <Xinhui.Pan@amd.com>; Deucher, Alexander <Alexander.Deucher@amd.com>; Liu, Leo <Leo.Liu@amd.com>
> Subject: Re: [PATCH] drm/amdgpu: limit VCN instance number to 1 for NAVY_FLOUNDER
>
> On Thu, Oct 21, 2021 at 3:15 AM Guchun Chen <guchun.chen@amd.com> wrote:
> >
> > VCN instance 1 is power gated permanently by SMU.
> >
> > Bug:
> > https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgitl
> > ab.freedesktop.org%2Fdrm%2Famd%2F-%2Fissues%2F1743&data=04%7C01%7C
> > guchun.chen%40amd.com%7Cda80a308a28049d543ad08d99493847d%7C3dd8961fe48
> > 84e608e11a82d994e183d%7C0%7C0%7C637704183581593964%7CUnknown%7CTWFpbGZ
> > sb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3
> > D%7C1000&sdata=2vNLj9bXE2oV97rxBiUOiaFNpKopVSJefL%2BMcQE%2BSfo%3D&
> > amp;reserved=0
> >
> > Fixes: f6b6d7d6bc2d("drm/amdgpu/vcn: remove manual instance setting")
> > Signed-off-by: Guchun Chen <guchun.chen@amd.com>
>
> Doesn't this patch effectively do the same thing?
> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fpatchwork.freedesktop.org%2Fpatch%2F460329%2F&data=04%7C01%7Cguchun.chen%40amd.com%7Cda80a308a28049d543ad08d99493847d%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637704183581593964%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=jPu3Yh%2B6OHR4F1BS5MWL3VyZ3pui6c0dP97Zl7yBJKY%3D&reserved=0
> Where else is num_vcn_inst used that it causes a problem? Or is the VCN harvesting not set correctly on some navy flounders?
>
> Alex
>
> > ---
> > drivers/gpu/drm/amd/amdgpu/vcn_v3_0.c | 9 +++++++++
> > 1 file changed, 9 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/amd/amdgpu/vcn_v3_0.c
> > b/drivers/gpu/drm/amd/amdgpu/vcn_v3_0.c
> > index dbfd92984655..4848922667f2 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/vcn_v3_0.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/vcn_v3_0.c
> > @@ -103,6 +103,15 @@ static int vcn_v3_0_early_init(void *handle)
> > adev->vcn.num_enc_rings = 0;
> > else
> > adev->vcn.num_enc_rings = 2;
> > +
> > + /*
> > + * Fix ME.
> > + * VCN instance number is limited to 1 for below ASIC due to
> > + * VCN instnace 1 is permanently power gated.
> > + */
> > + if ((adev->ip_versions[UVD_HWIP][0] == IP_VERSION(3, 0, 0)) &&
> > + (adev->ip_versions[GC_HWIP][0] == IP_VERSION(10, 3, 2)))
> > + adev->vcn.num_vcn_inst = 1;
> > }
> >
> > vcn_v3_0_set_dec_ring_funcs(adev);
> > --
> > 2.17.1
> >
^ permalink raw reply [flat|nested] 10+ messages in thread
* RE: [PATCH] drm/amdgpu: limit VCN instance number to 1 for NAVY_FLOUNDER
2021-10-21 14:01 ` Alex Deucher
@ 2021-10-21 14:09 ` Chen, Guchun
0 siblings, 0 replies; 10+ messages in thread
From: Chen, Guchun @ 2021-10-21 14:09 UTC (permalink / raw)
To: Alex Deucher
Cc: amd-gfx list, Koenig, Christian, Pan, Xinhui, Deucher, Alexander,
Liu, Leo
Why using asic_type to check this? The issue is caused by the ip discovery series, and I thought that series' goal is to remove DID/asic_type as much as possible in kernel driver.
+ /* some IP discovery tables on NF don't have this set correctly */
+ if (adev->asic_type == CHIP_NAVY_FLOUNDER)
+ adev->vcn.harvest_config |= AMDGPU_VCN_HARVEST_VCN1;
Regards,
Guchun
-----Original Message-----
From: Alex Deucher <alexdeucher@gmail.com>
Sent: Thursday, October 21, 2021 10:02 PM
To: Chen, Guchun <Guchun.Chen@amd.com>
Cc: amd-gfx list <amd-gfx@lists.freedesktop.org>; Koenig, Christian <Christian.Koenig@amd.com>; Pan, Xinhui <Xinhui.Pan@amd.com>; Deucher, Alexander <Alexander.Deucher@amd.com>; Liu, Leo <Leo.Liu@amd.com>
Subject: Re: [PATCH] drm/amdgpu: limit VCN instance number to 1 for NAVY_FLOUNDER
Thanks. I think this patch set fixes it in a bit more future proof way:
https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fpatchwork.freedesktop.org%2Fseries%2F96132%2F&data=04%7C01%7CGuchun.Chen%40amd.com%7C52fab5ccf8f64b6eb09b08d9949b548f%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637704217145304873%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=2KMrUDLZZ1s3colyVy1WwY4Yz6GbyI9z53qixn%2BuUwQ%3D&reserved=0
Alex
On Thu, Oct 21, 2021 at 9:34 AM Chen, Guchun <Guchun.Chen@amd.com> wrote:
>
> Additionally, in sienna_cichlid_dpm_set_vcn_enable, we also use num_vcn_inst to set dpm for VCN1 if it's > 1.
> The main problem here is VCN harvest info is not set correctly, so vcn.harvest_config is not reliable in this case.
>
> if (smu_cmn_feature_is_enabled(smu, SMU_FEATURE_MM_DPM_PG_BIT)) {
> ret = smu_cmn_send_smc_msg_with_param(smu, SMU_MSG_PowerUpVcn, 0, NULL);
> if (ret)
> return ret;
> if (adev->vcn.num_vcn_inst > 1) {
> ret = smu_cmn_send_smc_msg_with_param(smu, SMU_MSG_PowerUpVcn,
> 0x10000, NULL);
> if (ret)
> return ret;
> }
> }
>
> Regards,
> Guchun
>
> -----Original Message-----
> From: Chen, Guchun
> Sent: Thursday, October 21, 2021 9:14 PM
> To: Alex Deucher <alexdeucher@gmail.com>
> Cc: amd-gfx list <amd-gfx@lists.freedesktop.org>; Koenig, Christian
> <Christian.Koenig@amd.com>; Pan, Xinhui <Xinhui.Pan@amd.com>; Deucher,
> Alexander <Alexander.Deucher@amd.com>; Liu, Leo <Leo.Liu@amd.com>
> Subject: RE: [PATCH] drm/amdgpu: limit VCN instance number to 1 for
> NAVY_FLOUNDER
>
> Hi Alex,
>
> No, it does not help.
>
> adev->vcn.harvest_config is 0 after retrieving harvest info from VBIOS. Looks that harvest info in VBIOs does not reflect the case that VCN1 is power gated.
>
> I checked several navy flounders SKUs, the observation is the same, so this is likely a common case. Perhaps we need to check with VBIOS/SMU guys.
>
> Regards,
> Guchun
>
> -----Original Message-----
> From: Alex Deucher <alexdeucher@gmail.com>
> Sent: Thursday, October 21, 2021 9:06 PM
> To: Chen, Guchun <Guchun.Chen@amd.com>
> Cc: amd-gfx list <amd-gfx@lists.freedesktop.org>; Koenig, Christian
> <Christian.Koenig@amd.com>; Pan, Xinhui <Xinhui.Pan@amd.com>; Deucher,
> Alexander <Alexander.Deucher@amd.com>; Liu, Leo <Leo.Liu@amd.com>
> Subject: Re: [PATCH] drm/amdgpu: limit VCN instance number to 1 for
> NAVY_FLOUNDER
>
> On Thu, Oct 21, 2021 at 3:15 AM Guchun Chen <guchun.chen@amd.com> wrote:
> >
> > VCN instance 1 is power gated permanently by SMU.
> >
> > Bug:
> > https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgi
> > tl
> > ab.freedesktop.org%2Fdrm%2Famd%2F-%2Fissues%2F1743&data=04%7C01%
> > 7C
> > guchun.chen%40amd.com%7Cda80a308a28049d543ad08d99493847d%7C3dd8961fe
> > 48
> > 84e608e11a82d994e183d%7C0%7C0%7C637704183581593964%7CUnknown%7CTWFpb
> > GZ
> > sb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0
> > %3
> > D%7C1000&sdata=2vNLj9bXE2oV97rxBiUOiaFNpKopVSJefL%2BMcQE%2BSfo%3
> > D&
> > amp;reserved=0
> >
> > Fixes: f6b6d7d6bc2d("drm/amdgpu/vcn: remove manual instance
> > setting")
> > Signed-off-by: Guchun Chen <guchun.chen@amd.com>
>
> Doesn't this patch effectively do the same thing?
> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fpatc
> hwork.freedesktop.org%2Fpatch%2F460329%2F&data=04%7C01%7CGuchun.Ch
> en%40amd.com%7C52fab5ccf8f64b6eb09b08d9949b548f%7C3dd8961fe4884e608e11
> a82d994e183d%7C0%7C0%7C637704217145304873%7CUnknown%7CTWFpbGZsb3d8eyJW
> IjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&
> amp;sdata=EmyT%2BNBnV8rIhJSqncnyFwR94smOvu2AGeb4vESFhdE%3D&reserve
> d=0 Where else is num_vcn_inst used that it causes a problem? Or is
> the VCN harvesting not set correctly on some navy flounders?
>
> Alex
>
> > ---
> > drivers/gpu/drm/amd/amdgpu/vcn_v3_0.c | 9 +++++++++
> > 1 file changed, 9 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/amd/amdgpu/vcn_v3_0.c
> > b/drivers/gpu/drm/amd/amdgpu/vcn_v3_0.c
> > index dbfd92984655..4848922667f2 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/vcn_v3_0.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/vcn_v3_0.c
> > @@ -103,6 +103,15 @@ static int vcn_v3_0_early_init(void *handle)
> > adev->vcn.num_enc_rings = 0;
> > else
> > adev->vcn.num_enc_rings = 2;
> > +
> > + /*
> > + * Fix ME.
> > + * VCN instance number is limited to 1 for below ASIC due to
> > + * VCN instnace 1 is permanently power gated.
> > + */
> > + if ((adev->ip_versions[UVD_HWIP][0] == IP_VERSION(3, 0, 0)) &&
> > + (adev->ip_versions[GC_HWIP][0] == IP_VERSION(10, 3, 2)))
> > + adev->vcn.num_vcn_inst = 1;
> > }
> >
> > vcn_v3_0_set_dec_ring_funcs(adev);
> > --
> > 2.17.1
> >
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2021-10-21 14:09 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-10-21 7:15 [PATCH] drm/amdgpu: limit VCN instance number to 1 for NAVY_FLOUNDER Guchun Chen
2021-10-21 9:44 ` Lazar, Lijo
2021-10-21 12:40 ` Chen, Guchun
2021-10-21 12:56 ` Lazar, Lijo
2021-10-21 13:00 ` Chen, Guchun
2021-10-21 13:05 ` Alex Deucher
2021-10-21 13:14 ` Chen, Guchun
2021-10-21 13:34 ` Chen, Guchun
2021-10-21 14:01 ` Alex Deucher
2021-10-21 14:09 ` Chen, Guchun
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.