[v1 2/4] drm/amdgpu: accumulate gfx_off_entrycount in smu_suspend()

[Gerry Liu]

> 2025年2月7日 16:34，Lazar, Lijo <lijo.lazar at amd.com> 写道：
> 
> 
> 
> On 2/7/2025 2:00 PM, Gerry Liu wrote:
>> 
>> 
>>> 2025年2月7日 16:04，Lazar, Lijo <lijo.lazar at amd.com> 写道：
>>> 
>>> 
>>> 
>>> On 2/7/2025 12:14 PM, Jiang Liu wrote:
>>>> As pwfw resets entrycount when device is suspended, so we should
>>>> accmulate the gfx_off_entrycount value instead of save the last value
>>>> of it.
>>>> 
>>>> Signed-off-by: Jiang Liu <gerry at linux.alibaba.com>
>>>> ---
>>>> drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c | 6 +++---
>>>> 1 file changed, 3 insertions(+), 3 deletions(-)
>>>> 
>>>> diff --git a/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c b/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
>>>> index ed9dac00ebfb..70a5ab649e5f 100644
>>>> --- a/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
>>>> +++ b/drivers/gpu/drm/amd/pm/swsmu/amdgpu_smu.c
>>>> @@ -2134,12 +2134,12 @@ static int smu_suspend(struct amdgpu_ip_block *ip_block)
>>>> 	smu_set_gfx_cgpg(smu, false);
>>>> 
>>>> 	/*
>>>> -	 * pwfw resets entrycount when device is suspended, so we save the
>>>> -	 * last value to be used when we resume to keep it consistent
>>>> +	 * pwfw resets entrycount when device is suspended, so we accumulate
>>>> +	 * the `gfx_off_entrycount` value.
>>>> 	 */
>>>> 	ret = smu_get_entrycount_gfxoff(smu, &count);
>>>> 	if (!ret)
>>>> -		adev->gfx.gfx_off_entrycount = count;
>>>> +		adev->gfx.gfx_off_entrycount += count;
>>> 
>>> This is slightly misleading - only Vangogh implements
>>> get_gfx_off_entrycount and its implementation,
>>> vangogh_get_gfxoff_entrycount, is already doing something like this -
>>> 
>>> *entrycount = value + adev->gfx.gfx_off_entrycount;
>> 
>> Because we have not restored the hardware counter by writing back the value on resume, adev->gfx.gfx_off_entrycount only remembers the latest value of latest suspend/resume cycle. And history information is lost, so changed it to accumulate all values.
> 
> What I meant is that accumulation is already done in the implementation
> side - check implementation of vangogh_get_gfxoff_entrycount.
Aha, you are right, smu_suspend() calls smu_get_entrycount_gfxoff(), it already accumulates along the way.
Will drop it.

> 
> Thanks,
> Lijo
> 
>> 
>>> 
>>> Thanks,
>>> Lijo
>>> 
>>>> 
>>>> 	/* clear this on suspend so it will get reprogrammed on resume */
>>>> 	smu->workload_mask = 0;