[AMD Official Use Only - Internal Distribution Only]
Thanks for catching this. The patch is reviewed-by: Evan Quan <evan.quan@xxxxxxx>
I have the same copy as Matt. @Li, Dennis maybe you were working on an old copy?
BR
Evan
-----Original Message-----
From: amd-gfx <amd-gfx-bounces@xxxxxxxxxxxxxxxxxxxxx> On Behalf Of Matt Coffin
Sent: Friday, August 14, 2020 11:14 AM
To: Li, Dennis <Dennis.Li@xxxxxxx>; amd-gfx@xxxxxxxxxxxxxxxxxxxxx
Cc: Koenig, Christian <Christian.Koenig@xxxxxxx>
Subject: Re: [PATCH] drm/amdgpu: Fix incorrect return value in sysfs for pp_od_clk_voltage
Hey Dennis,
Thanks for the testing.
I'm having some issues reproducing, as that command is working fine for me in sh, bash, and zsh. So just to confirm a few things while I look at it...
1. What kind of SMU is on whatever card you're testing on? Looks like smu_v11+ to me?
2. (shouldn't matter if you're right about which -EINVAL return is being hit), but is OverDrive enabled?
3. Is this based off of latest amd-staging-drm-next?
This is the code block I'm seeing on the HEAD of alex's branch... which is a bit different from what you pasted.
This error also happens **before** the infinite loop I was fixing with this patch, but might as well get both birds with one stone if there's still an issue.
while (tmp_str[0]) {
sub_str = strsep(&tmp_str, delimiter);
ret = kstrtol(sub_str, 0, ¶meter[parameter_size]);
if (ret)
return -EINVAL;
parameter_size++;
while (isspace(*tmp_str))
tmp_str++;
}
On 8/13/20 8:14 PM, Li, Dennis wrote:
> [AMD Official Use Only - Internal Distribution Only]
>
> Hi, Matt,
> With your change, I still could reproduce the following issue:
>
> # echo "s 1 1900" > /sys/class/drm/card0/device/pp_od_clk_voltage
> bash: echo: write error: Invalid argument
>
> I found that it is related the following lines code, could you help double check it?
>
> while ((sub_str = strsep(&tmp_str, delimiter)) != NULL) { // sub_str will be empty string
> ret = kstrtol(sub_str, 0, ¶meter[parameter_size]);
> if (ret)
> return -EINVAL; // return here
> parameter_size++;
>
> while (isspace(*tmp_str))
> tmp_str++;
> }
>
> Best Regards
> Dennis Li
> -----Original Message-----
> From: Matt Coffin <mcoffin13@xxxxxxxxx>
> Sent: Friday, August 14, 2020 9:15 AM
> To: amd-gfx@xxxxxxxxxxxxxxxxxxxxx
> Cc: Koenig, Christian <Christian.Koenig@xxxxxxx>; Li, Dennis
> <Dennis.Li@xxxxxxx>; Matt Coffin <mcoffin13@xxxxxxxxx>
> Subject: [PATCH] drm/amdgpu: Fix incorrect return value in sysfs for
> pp_od_clk_voltage
>
> The changes in edad8312cbbf9a33c86873fc4093664f150dd5c1 introduced an issue with the sysfs interface for pp_od_clk_voltage. It overwrites the return value to 0 when it calls another function, then returns 0. The intended behavior is that a positive return value indicates the number of bytes from the buffer that you processed in that call.
>
> With the 0 return value, clients would submit the same value to be written over and over again, resulting in an infinite loop.
>
> This is resolved by returning the count of bytes read (in this case the whole message), when the desired return is 0 (success).
>
> Fixes: edad8312cbbf ("drm/amdgpu: fix system hang issue during GPU")
> Bug:
> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgitl
> ab.freedesktop.org%2Fdrm%2Famd%2F-%2Fissues%2F1245&data=02%7C01%7C
> evan.quan%40amd.com%7C08991e3858f144a4dd0908d840001324%7C3dd8961fe4884
> e608e11a82d994e183d%7C0%7C0%7C637329716385273288&sdata=w%2FBlX8CpG
> 0TfTYd1InX8Z%2FMBrRbVu%2FT4zWehWKHClCE%3D&reserved=0
> Signed-off-by: Matt Coffin <mcoffin13@xxxxxxxxx>
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c | 6 +++++-
> 1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c
> index 1705e328c6fc..f00c7ed361d4 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_pm.c
> @@ -937,7 +937,11 @@ static ssize_t
> amdgpu_set_pp_od_clk_voltage(struct device *dev,
>
> pro_end:
> up_read(&adev->reset_sem);
> -return ret;
> +if (ret) {
> +return ret;
> +} else {
> +return count;
> +}
> }
>
> static ssize_t amdgpu_get_pp_od_clk_voltage(struct device *dev,
> --
> 2.28.0
>
_______________________________________________
amd-gfx mailing list
amd-gfx@xxxxxxxxxxxxxxxxxxxxx
https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.freedesktop.org%2Fmailman%2Flistinfo%2Famd-gfx&data=02%7C01%7Cevan.quan%40amd.com%7C08991e3858f144a4dd0908d840001324%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637329716385273288&sdata=IbsPW7%2Fr2HXLxKK4%2FOb6fqFmZXtyivbTsP9ftd7P2Dw%3D&reserved=0
_______________________________________________
amd-gfx mailing list
amd-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
