AMD-GFX Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] drm/amdgpu: Clear VCN cache when hw_init
@ 2023-06-20 13:29 Horace Chen
  2023-06-21  9:25 ` Deng, Emily
  2023-06-30 11:38 ` Christian König
  0 siblings, 2 replies; 7+ messages in thread
From: Horace Chen @ 2023-06-20 13:29 UTC (permalink / raw)
  To: amd-gfx
  Cc: Andrey Grodzovsky, Jack Xiao, Feifei Xu, horace.chen,
	Haijun Chang, Deucher Alexander, Evan Quan, Christian König,
	Monk Liu, Hawking Zhang

[Why]
VCN will use some framebuffer space as its cache. It needs to
be reset when reset happens, such as FLR. Otherwise some error
may be kept after the reset.

Signed-off-by: Horace Chen <horace.chen@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/vcn_v4_0.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/vcn_v4_0.c b/drivers/gpu/drm/amd/amdgpu/vcn_v4_0.c
index b48bb5212488..2db73a964031 100644
--- a/drivers/gpu/drm/amd/amdgpu/vcn_v4_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/vcn_v4_0.c
@@ -1292,6 +1292,7 @@ static int vcn_v4_0_start_sriov(struct amdgpu_device *adev)
 			cache_size);
 
 		cache_addr = adev->vcn.inst[i].gpu_addr + offset;
+		memset(adev->vcn.inst[i].cpu_addr + offset, 0, AMDGPU_VCN_STACK_SIZE);
 		MMSCH_V4_0_INSERT_DIRECT_WT(SOC15_REG_OFFSET(VCN, i,
 			regUVD_LMI_VCPU_CACHE1_64BIT_BAR_LOW),
 			lower_32_bits(cache_addr));
@@ -1307,6 +1308,8 @@ static int vcn_v4_0_start_sriov(struct amdgpu_device *adev)
 
 		cache_addr = adev->vcn.inst[i].gpu_addr + offset +
 			AMDGPU_VCN_STACK_SIZE;
+		memset(adev->vcn.inst[i].cpu_addr + offset + AMDGPU_VCN_STACK_SIZE, 0,
+			AMDGPU_VCN_STACK_SIZE);
 		MMSCH_V4_0_INSERT_DIRECT_WT(SOC15_REG_OFFSET(VCN, i,
 			regUVD_LMI_VCPU_CACHE2_64BIT_BAR_LOW),
 			lower_32_bits(cache_addr));
-- 
2.34.1


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

* RE: [PATCH] drm/amdgpu: Clear VCN cache when hw_init
  2023-06-20 13:29 [PATCH] drm/amdgpu: Clear VCN cache when hw_init Horace Chen
@ 2023-06-21  9:25 ` Deng, Emily
  2023-06-30 11:38 ` Christian König
  1 sibling, 0 replies; 7+ messages in thread
From: Deng, Emily @ 2023-06-21  9:25 UTC (permalink / raw)
  To: Chen, Horace, amd-gfx@lists.freedesktop.org
  Cc: Andrey Grodzovsky, Xiao, Jack, Xu, Feifei, Chen, Horace,
	Chang, HaiJun, Deucher,  Alexander, Quan, Evan, Koenig, Christian,
	Liu, Monk, Zhang, Hawking

[AMD Official Use Only - General]

Reviewed-by: Emily Deng <Emily.Deng@amd.com>

>-----Original Message-----
>From: amd-gfx <amd-gfx-bounces@lists.freedesktop.org> On Behalf Of Horace
>Chen
>Sent: Tuesday, June 20, 2023 9:30 PM
>To: amd-gfx@lists.freedesktop.org
>Cc: Andrey Grodzovsky <Andrey.Grodzovsky@amd.com>; Xiao, Jack
><Jack.Xiao@amd.com>; Xu, Feifei <Feifei.Xu@amd.com>; Chen, Horace
><Horace.Chen@amd.com>; Chang, HaiJun <HaiJun.Chang@amd.com>;
>Deucher, Alexander <Alexander.Deucher@amd.com>; Quan, Evan
><Evan.Quan@amd.com>; Koenig, Christian <Christian.Koenig@amd.com>; Liu,
>Monk <Monk.Liu@amd.com>; Zhang, Hawking <Hawking.Zhang@amd.com>
>Subject: [PATCH] drm/amdgpu: Clear VCN cache when hw_init
>
>[Why]
>VCN will use some framebuffer space as its cache. It needs to be reset when
>reset happens, such as FLR. Otherwise some error may be kept after the reset.
>
>Signed-off-by: Horace Chen <horace.chen@amd.com>
>---
> drivers/gpu/drm/amd/amdgpu/vcn_v4_0.c | 3 +++
> 1 file changed, 3 insertions(+)
>
>diff --git a/drivers/gpu/drm/amd/amdgpu/vcn_v4_0.c
>b/drivers/gpu/drm/amd/amdgpu/vcn_v4_0.c
>index b48bb5212488..2db73a964031 100644
>--- a/drivers/gpu/drm/amd/amdgpu/vcn_v4_0.c
>+++ b/drivers/gpu/drm/amd/amdgpu/vcn_v4_0.c
>@@ -1292,6 +1292,7 @@ static int vcn_v4_0_start_sriov(struct
>amdgpu_device *adev)
>                       cache_size);
>
>               cache_addr = adev->vcn.inst[i].gpu_addr + offset;
>+              memset(adev->vcn.inst[i].cpu_addr + offset, 0,
>+AMDGPU_VCN_STACK_SIZE);
>
>       MMSCH_V4_0_INSERT_DIRECT_WT(SOC15_REG_OFFSET(VCN, i,
>                       regUVD_LMI_VCPU_CACHE1_64BIT_BAR_LOW),
>                       lower_32_bits(cache_addr));
>@@ -1307,6 +1308,8 @@ static int vcn_v4_0_start_sriov(struct
>amdgpu_device *adev)
>
>               cache_addr = adev->vcn.inst[i].gpu_addr + offset +
>                       AMDGPU_VCN_STACK_SIZE;
>+              memset(adev->vcn.inst[i].cpu_addr + offset +
>AMDGPU_VCN_STACK_SIZE, 0,
>+                      AMDGPU_VCN_STACK_SIZE);
>
>       MMSCH_V4_0_INSERT_DIRECT_WT(SOC15_REG_OFFSET(VCN, i,
>                       regUVD_LMI_VCPU_CACHE2_64BIT_BAR_LOW),
>                       lower_32_bits(cache_addr));
>--
>2.34.1


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

* Re: [PATCH] drm/amdgpu: Clear VCN cache when hw_init
  2023-06-20 13:29 [PATCH] drm/amdgpu: Clear VCN cache when hw_init Horace Chen
  2023-06-21  9:25 ` Deng, Emily
@ 2023-06-30 11:38 ` Christian König
  2023-07-05 19:57   ` Leo Liu
  1 sibling, 1 reply; 7+ messages in thread
From: Christian König @ 2023-06-30 11:38 UTC (permalink / raw)
  To: Horace Chen, amd-gfx, Liu, Leo
  Cc: Jack Xiao, Feifei Xu, Haijun Chang, Deucher Alexander, Evan Quan,
	Monk Liu, Hawking Zhang

Am 20.06.23 um 15:29 schrieb Horace Chen:
> [Why]
> VCN will use some framebuffer space as its cache. It needs to
> be reset when reset happens, such as FLR. Otherwise some error
> may be kept after the reset.

Well this doesn't make sense at all.

The full content of adev->vcn.inst[i].cpu_addr is saved and restored 
during suspend/resume and IIRC GPU resets as well.

See functions amdgpu_vcn_suspend() and amdgpu_vcn_resume().

Please let Leo's team take a look at this and review the change before 
it is committed.

Regards,
Christian.

>
> Signed-off-by: Horace Chen <horace.chen@amd.com>
> ---
>   drivers/gpu/drm/amd/amdgpu/vcn_v4_0.c | 3 +++
>   1 file changed, 3 insertions(+)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/vcn_v4_0.c b/drivers/gpu/drm/amd/amdgpu/vcn_v4_0.c
> index b48bb5212488..2db73a964031 100644
> --- a/drivers/gpu/drm/amd/amdgpu/vcn_v4_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/vcn_v4_0.c
> @@ -1292,6 +1292,7 @@ static int vcn_v4_0_start_sriov(struct amdgpu_device *adev)
>   			cache_size);
>   
>   		cache_addr = adev->vcn.inst[i].gpu_addr + offset;
> +		memset(adev->vcn.inst[i].cpu_addr + offset, 0, AMDGPU_VCN_STACK_SIZE);
>   		MMSCH_V4_0_INSERT_DIRECT_WT(SOC15_REG_OFFSET(VCN, i,
>   			regUVD_LMI_VCPU_CACHE1_64BIT_BAR_LOW),
>   			lower_32_bits(cache_addr));
> @@ -1307,6 +1308,8 @@ static int vcn_v4_0_start_sriov(struct amdgpu_device *adev)
>   
>   		cache_addr = adev->vcn.inst[i].gpu_addr + offset +
>   			AMDGPU_VCN_STACK_SIZE;
> +		memset(adev->vcn.inst[i].cpu_addr + offset + AMDGPU_VCN_STACK_SIZE, 0,
> +			AMDGPU_VCN_STACK_SIZE);
>   		MMSCH_V4_0_INSERT_DIRECT_WT(SOC15_REG_OFFSET(VCN, i,
>   			regUVD_LMI_VCPU_CACHE2_64BIT_BAR_LOW),
>   			lower_32_bits(cache_addr));


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

* Re: [PATCH] drm/amdgpu: Clear VCN cache when hw_init
  2023-06-30 11:38 ` Christian König
@ 2023-07-05 19:57   ` Leo Liu
  2023-07-06  7:24     ` Christian König
  0 siblings, 1 reply; 7+ messages in thread
From: Leo Liu @ 2023-07-05 19:57 UTC (permalink / raw)
  To: Christian König, Horace Chen, amd-gfx
  Cc: Jack Xiao, Feifei Xu, Haijun Chang, Deucher Alexander, Evan Quan,
	Monk Liu, Hawking Zhang

What Christian says is correct, esp. during the playback or encode, when 
suspend/resume happens, it will save the FW context, and after resume, 
it will continue the job to where it left during the suspend. Will this 
apply to SRIOV case? Since the changes only within the SRIOV code, 
please make sure that also please specify the SRIOV from your patch 
subject and commit message.

Regards,

Leo


On 2023-06-30 07:38, Christian König wrote:
> Am 20.06.23 um 15:29 schrieb Horace Chen:
>> [Why]
>> VCN will use some framebuffer space as its cache. It needs to
>> be reset when reset happens, such as FLR. Otherwise some error
>> may be kept after the reset.
>
> Well this doesn't make sense at all.
>
> The full content of adev->vcn.inst[i].cpu_addr is saved and restored 
> during suspend/resume and IIRC GPU resets as well.
>
> See functions amdgpu_vcn_suspend() and amdgpu_vcn_resume().
>
> Please let Leo's team take a look at this and review the change before 
> it is committed.
>
> Regards,
> Christian.
>
>>
>> Signed-off-by: Horace Chen <horace.chen@amd.com>
>> ---
>>   drivers/gpu/drm/amd/amdgpu/vcn_v4_0.c | 3 +++
>>   1 file changed, 3 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/vcn_v4_0.c 
>> b/drivers/gpu/drm/amd/amdgpu/vcn_v4_0.c
>> index b48bb5212488..2db73a964031 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/vcn_v4_0.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/vcn_v4_0.c
>> @@ -1292,6 +1292,7 @@ static int vcn_v4_0_start_sriov(struct 
>> amdgpu_device *adev)
>>               cache_size);
>>             cache_addr = adev->vcn.inst[i].gpu_addr + offset;
>> +        memset(adev->vcn.inst[i].cpu_addr + offset, 0, 
>> AMDGPU_VCN_STACK_SIZE);
>>           MMSCH_V4_0_INSERT_DIRECT_WT(SOC15_REG_OFFSET(VCN, i,
>>               regUVD_LMI_VCPU_CACHE1_64BIT_BAR_LOW),
>>               lower_32_bits(cache_addr));
>> @@ -1307,6 +1308,8 @@ static int vcn_v4_0_start_sriov(struct 
>> amdgpu_device *adev)
>>             cache_addr = adev->vcn.inst[i].gpu_addr + offset +
>>               AMDGPU_VCN_STACK_SIZE;
>> +        memset(adev->vcn.inst[i].cpu_addr + offset + 
>> AMDGPU_VCN_STACK_SIZE, 0,
>> +            AMDGPU_VCN_STACK_SIZE);
>>           MMSCH_V4_0_INSERT_DIRECT_WT(SOC15_REG_OFFSET(VCN, i,
>>               regUVD_LMI_VCPU_CACHE2_64BIT_BAR_LOW),
>>               lower_32_bits(cache_addr));
>

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

* Re: [PATCH] drm/amdgpu: Clear VCN cache when hw_init
  2023-07-05 19:57   ` Leo Liu
@ 2023-07-06  7:24     ` Christian König
  2023-07-06  8:56       ` Chen, Horace
  0 siblings, 1 reply; 7+ messages in thread
From: Christian König @ 2023-07-06  7:24 UTC (permalink / raw)
  To: Leo Liu, Horace Chen, amd-gfx
  Cc: Jack Xiao, Feifei Xu, Haijun Chang, Deucher Alexander, Evan Quan,
	Monk Liu, Hawking Zhang

Since this patch was already pushed please revert it until we have a 
technical consent on why this should be necessary.

Regards,
Christian.

Am 05.07.23 um 21:57 schrieb Leo Liu:
> What Christian says is correct, esp. during the playback or encode, 
> when suspend/resume happens, it will save the FW context, and after 
> resume, it will continue the job to where it left during the suspend. 
> Will this apply to SRIOV case? Since the changes only within the SRIOV 
> code, please make sure that also please specify the SRIOV from your 
> patch subject and commit message.
>
> Regards,
>
> Leo
>
>
> On 2023-06-30 07:38, Christian König wrote:
>> Am 20.06.23 um 15:29 schrieb Horace Chen:
>>> [Why]
>>> VCN will use some framebuffer space as its cache. It needs to
>>> be reset when reset happens, such as FLR. Otherwise some error
>>> may be kept after the reset.
>>
>> Well this doesn't make sense at all.
>>
>> The full content of adev->vcn.inst[i].cpu_addr is saved and restored 
>> during suspend/resume and IIRC GPU resets as well.
>>
>> See functions amdgpu_vcn_suspend() and amdgpu_vcn_resume().
>>
>> Please let Leo's team take a look at this and review the change 
>> before it is committed.
>>
>> Regards,
>> Christian.
>>
>>>
>>> Signed-off-by: Horace Chen <horace.chen@amd.com>
>>> ---
>>>   drivers/gpu/drm/amd/amdgpu/vcn_v4_0.c | 3 +++
>>>   1 file changed, 3 insertions(+)
>>>
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/vcn_v4_0.c 
>>> b/drivers/gpu/drm/amd/amdgpu/vcn_v4_0.c
>>> index b48bb5212488..2db73a964031 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/vcn_v4_0.c
>>> +++ b/drivers/gpu/drm/amd/amdgpu/vcn_v4_0.c
>>> @@ -1292,6 +1292,7 @@ static int vcn_v4_0_start_sriov(struct 
>>> amdgpu_device *adev)
>>>               cache_size);
>>>             cache_addr = adev->vcn.inst[i].gpu_addr + offset;
>>> +        memset(adev->vcn.inst[i].cpu_addr + offset, 0, 
>>> AMDGPU_VCN_STACK_SIZE);
>>>           MMSCH_V4_0_INSERT_DIRECT_WT(SOC15_REG_OFFSET(VCN, i,
>>>               regUVD_LMI_VCPU_CACHE1_64BIT_BAR_LOW),
>>>               lower_32_bits(cache_addr));
>>> @@ -1307,6 +1308,8 @@ static int vcn_v4_0_start_sriov(struct 
>>> amdgpu_device *adev)
>>>             cache_addr = adev->vcn.inst[i].gpu_addr + offset +
>>>               AMDGPU_VCN_STACK_SIZE;
>>> +        memset(adev->vcn.inst[i].cpu_addr + offset + 
>>> AMDGPU_VCN_STACK_SIZE, 0,
>>> +            AMDGPU_VCN_STACK_SIZE);
>>>           MMSCH_V4_0_INSERT_DIRECT_WT(SOC15_REG_OFFSET(VCN, i,
>>>               regUVD_LMI_VCPU_CACHE2_64BIT_BAR_LOW),
>>>               lower_32_bits(cache_addr));
>>


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

* RE: [PATCH] drm/amdgpu: Clear VCN cache when hw_init
  2023-07-06  7:24     ` Christian König
@ 2023-07-06  8:56       ` Chen, Horace
  2023-07-06 13:56         ` Alex Deucher
  0 siblings, 1 reply; 7+ messages in thread
From: Chen, Horace @ 2023-07-06  8:56 UTC (permalink / raw)
  To: Koenig, Christian, Liu, Leo, amd-gfx@lists.freedesktop.org
  Cc: Xiao, Jack, Xu, Feifei, Chang, HaiJun, Deucher, Alexander,
	Quan, Evan, Liu, Monk, Zhang, Hawking

[AMD Official Use Only - General]

Hi Christian & Leo,

Sorry, missed Leo's email.

Currently SR-IOV VF doesn't have suspend/resume use case and this code is in vcn_v4_0_start_sriov() which will only called by SR-IOV code path. So currently we will not hit the suspend/resume issue even with this patch.

About why this patch is necessary:
    Because SR-IOV uses FLR instead of mode 1 reset as the default reset method, and FLR will not reset VRAM during the common reset. So if there is some bad data in the cache, it will cause VF continues to fail after reset.  We have some test case(such as data poison) which may hit this issue.

If want to be safer in case we may add suspend/resume case in the future:
    how about add a "if (amdgpu_in_reset(adev))" check to make sure we only clear cache in the reset domain?

Thanks & Regards,
Horace.

-----Original Message-----
From: Koenig, Christian <Christian.Koenig@amd.com>
Sent: Thursday, July 6, 2023 3:24 PM
To: Liu, Leo <Leo.Liu@amd.com>; Chen, Horace <Horace.Chen@amd.com>; amd-gfx@lists.freedesktop.org
Cc: Quan, Evan <Evan.Quan@amd.com>; Deucher, Alexander <Alexander.Deucher@amd.com>; Xiao, Jack <Jack.Xiao@amd.com>; Zhang, Hawking <Hawking.Zhang@amd.com>; Liu, Monk <Monk.Liu@amd.com>; Xu, Feifei <Feifei.Xu@amd.com>; Chang, HaiJun <HaiJun.Chang@amd.com>
Subject: Re: [PATCH] drm/amdgpu: Clear VCN cache when hw_init

Since this patch was already pushed please revert it until we have a technical consent on why this should be necessary.

Regards,
Christian.

Am 05.07.23 um 21:57 schrieb Leo Liu:
> What Christian says is correct, esp. during the playback or encode,
> when suspend/resume happens, it will save the FW context, and after
> resume, it will continue the job to where it left during the suspend.
> Will this apply to SRIOV case? Since the changes only within the SRIOV
> code, please make sure that also please specify the SRIOV from your
> patch subject and commit message.
>
> Regards,
>
> Leo
>
>
> On 2023-06-30 07:38, Christian König wrote:
>> Am 20.06.23 um 15:29 schrieb Horace Chen:
>>> [Why]
>>> VCN will use some framebuffer space as its cache. It needs to be
>>> reset when reset happens, such as FLR. Otherwise some error may be
>>> kept after the reset.
>>
>> Well this doesn't make sense at all.
>>
>> The full content of adev->vcn.inst[i].cpu_addr is saved and restored
>> during suspend/resume and IIRC GPU resets as well.
>>
>> See functions amdgpu_vcn_suspend() and amdgpu_vcn_resume().
>>
>> Please let Leo's team take a look at this and review the change
>> before it is committed.
>>
>> Regards,
>> Christian.
>>
>>>
>>> Signed-off-by: Horace Chen <horace.chen@amd.com>
>>> ---
>>>   drivers/gpu/drm/amd/amdgpu/vcn_v4_0.c | 3 +++
>>>   1 file changed, 3 insertions(+)
>>>
>>> diff --git a/drivers/gpu/drm/amd/amdgpu/vcn_v4_0.c
>>> b/drivers/gpu/drm/amd/amdgpu/vcn_v4_0.c
>>> index b48bb5212488..2db73a964031 100644
>>> --- a/drivers/gpu/drm/amd/amdgpu/vcn_v4_0.c
>>> +++ b/drivers/gpu/drm/amd/amdgpu/vcn_v4_0.c
>>> @@ -1292,6 +1292,7 @@ static int vcn_v4_0_start_sriov(struct
>>> amdgpu_device *adev)
>>>               cache_size);
>>>             cache_addr = adev->vcn.inst[i].gpu_addr + offset;
>>> +        memset(adev->vcn.inst[i].cpu_addr + offset, 0,
>>> AMDGPU_VCN_STACK_SIZE);
>>>           MMSCH_V4_0_INSERT_DIRECT_WT(SOC15_REG_OFFSET(VCN, i,
>>>               regUVD_LMI_VCPU_CACHE1_64BIT_BAR_LOW),
>>>               lower_32_bits(cache_addr)); @@ -1307,6 +1308,8 @@
>>> static int vcn_v4_0_start_sriov(struct amdgpu_device *adev)
>>>             cache_addr = adev->vcn.inst[i].gpu_addr + offset +
>>>               AMDGPU_VCN_STACK_SIZE;
>>> +        memset(adev->vcn.inst[i].cpu_addr + offset +
>>> AMDGPU_VCN_STACK_SIZE, 0,
>>> +            AMDGPU_VCN_STACK_SIZE);
>>>           MMSCH_V4_0_INSERT_DIRECT_WT(SOC15_REG_OFFSET(VCN, i,
>>>               regUVD_LMI_VCPU_CACHE2_64BIT_BAR_LOW),
>>>               lower_32_bits(cache_addr));
>>


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

* Re: [PATCH] drm/amdgpu: Clear VCN cache when hw_init
  2023-07-06  8:56       ` Chen, Horace
@ 2023-07-06 13:56         ` Alex Deucher
  0 siblings, 0 replies; 7+ messages in thread
From: Alex Deucher @ 2023-07-06 13:56 UTC (permalink / raw)
  To: Chen, Horace
  Cc: Xiao, Jack, Xu, Feifei, amd-gfx@lists.freedesktop.org,
	Chang, HaiJun, Deucher, Alexander, Quan, Evan, Liu, Leo,
	Koenig, Christian, Liu, Monk, Zhang, Hawking

On Thu, Jul 6, 2023 at 4:56 AM Chen, Horace <Horace.Chen@amd.com> wrote:
>
> [AMD Official Use Only - General]
>
> Hi Christian & Leo,
>
> Sorry, missed Leo's email.
>
> Currently SR-IOV VF doesn't have suspend/resume use case and this code is in vcn_v4_0_start_sriov() which will only called by SR-IOV code path. So currently we will not hit the suspend/resume issue even with this patch.
>
> About why this patch is necessary:
>     Because SR-IOV uses FLR instead of mode 1 reset as the default reset method, and FLR will not reset VRAM during the common reset. So if there is some bad data in the cache, it will cause VF continues to fail after reset.  We have some test case(such as data poison) which may hit this issue.
>
> If want to be safer in case we may add suspend/resume case in the future:
>     how about add a "if (amdgpu_in_reset(adev))" check to make sure we only clear cache in the reset domain?

Yes, that seems like a better bet as that will keep suspend and resume working.

Alex

>
> Thanks & Regards,
> Horace.
>
> -----Original Message-----
> From: Koenig, Christian <Christian.Koenig@amd.com>
> Sent: Thursday, July 6, 2023 3:24 PM
> To: Liu, Leo <Leo.Liu@amd.com>; Chen, Horace <Horace.Chen@amd.com>; amd-gfx@lists.freedesktop.org
> Cc: Quan, Evan <Evan.Quan@amd.com>; Deucher, Alexander <Alexander.Deucher@amd.com>; Xiao, Jack <Jack.Xiao@amd.com>; Zhang, Hawking <Hawking.Zhang@amd.com>; Liu, Monk <Monk.Liu@amd.com>; Xu, Feifei <Feifei.Xu@amd.com>; Chang, HaiJun <HaiJun.Chang@amd.com>
> Subject: Re: [PATCH] drm/amdgpu: Clear VCN cache when hw_init
>
> Since this patch was already pushed please revert it until we have a technical consent on why this should be necessary.
>
> Regards,
> Christian.
>
> Am 05.07.23 um 21:57 schrieb Leo Liu:
> > What Christian says is correct, esp. during the playback or encode,
> > when suspend/resume happens, it will save the FW context, and after
> > resume, it will continue the job to where it left during the suspend.
> > Will this apply to SRIOV case? Since the changes only within the SRIOV
> > code, please make sure that also please specify the SRIOV from your
> > patch subject and commit message.
> >
> > Regards,
> >
> > Leo
> >
> >
> > On 2023-06-30 07:38, Christian König wrote:
> >> Am 20.06.23 um 15:29 schrieb Horace Chen:
> >>> [Why]
> >>> VCN will use some framebuffer space as its cache. It needs to be
> >>> reset when reset happens, such as FLR. Otherwise some error may be
> >>> kept after the reset.
> >>
> >> Well this doesn't make sense at all.
> >>
> >> The full content of adev->vcn.inst[i].cpu_addr is saved and restored
> >> during suspend/resume and IIRC GPU resets as well.
> >>
> >> See functions amdgpu_vcn_suspend() and amdgpu_vcn_resume().
> >>
> >> Please let Leo's team take a look at this and review the change
> >> before it is committed.
> >>
> >> Regards,
> >> Christian.
> >>
> >>>
> >>> Signed-off-by: Horace Chen <horace.chen@amd.com>
> >>> ---
> >>>   drivers/gpu/drm/amd/amdgpu/vcn_v4_0.c | 3 +++
> >>>   1 file changed, 3 insertions(+)
> >>>
> >>> diff --git a/drivers/gpu/drm/amd/amdgpu/vcn_v4_0.c
> >>> b/drivers/gpu/drm/amd/amdgpu/vcn_v4_0.c
> >>> index b48bb5212488..2db73a964031 100644
> >>> --- a/drivers/gpu/drm/amd/amdgpu/vcn_v4_0.c
> >>> +++ b/drivers/gpu/drm/amd/amdgpu/vcn_v4_0.c
> >>> @@ -1292,6 +1292,7 @@ static int vcn_v4_0_start_sriov(struct
> >>> amdgpu_device *adev)
> >>>               cache_size);
> >>>             cache_addr = adev->vcn.inst[i].gpu_addr + offset;
> >>> +        memset(adev->vcn.inst[i].cpu_addr + offset, 0,
> >>> AMDGPU_VCN_STACK_SIZE);
> >>>           MMSCH_V4_0_INSERT_DIRECT_WT(SOC15_REG_OFFSET(VCN, i,
> >>>               regUVD_LMI_VCPU_CACHE1_64BIT_BAR_LOW),
> >>>               lower_32_bits(cache_addr)); @@ -1307,6 +1308,8 @@
> >>> static int vcn_v4_0_start_sriov(struct amdgpu_device *adev)
> >>>             cache_addr = adev->vcn.inst[i].gpu_addr + offset +
> >>>               AMDGPU_VCN_STACK_SIZE;
> >>> +        memset(adev->vcn.inst[i].cpu_addr + offset +
> >>> AMDGPU_VCN_STACK_SIZE, 0,
> >>> +            AMDGPU_VCN_STACK_SIZE);
> >>>           MMSCH_V4_0_INSERT_DIRECT_WT(SOC15_REG_OFFSET(VCN, i,
> >>>               regUVD_LMI_VCPU_CACHE2_64BIT_BAR_LOW),
> >>>               lower_32_bits(cache_addr));
> >>
>

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

end of thread, other threads:[~2023-07-06 13:56 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-06-20 13:29 [PATCH] drm/amdgpu: Clear VCN cache when hw_init Horace Chen
2023-06-21  9:25 ` Deng, Emily
2023-06-30 11:38 ` Christian König
2023-07-05 19:57   ` Leo Liu
2023-07-06  7:24     ` Christian König
2023-07-06  8:56       ` Chen, Horace
2023-07-06 13:56         ` Alex Deucher

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox