Returning -EINVAL from a bool-returning function
phm_check_smc_update_required_for_display_configuration has an unexpected
effect of returning true, which is probably not what was intended.
Replace -EINVAL by false.
The only place this function is called from is
psm_adjust_power_state_dynamic in
drivers/gpu/drm/amd/powerplay/eventmgr/psm.c:106:
if (!equal || phm_check_smc_update_required_for_display_configuration(hwmgr)) {
phm_apply_state_adjust_rules(hwmgr, requested, pcurrent);
phm_set_power_state(hwmgr, &pcurrent->hardware, &requested->hardware);
hwmgr->current_ps = requested;
}
It seems to expect a boolean value here.
This issue has been found using the following Coccinelle semantic patch
written by Peter Senna Tschudin:
<smpl>
@@
identifier f;
constant C;
typedef bool;
@@
bool f (...){
<+...
* return -C;
...+>
}
</smpl>
Signed-off-by: Andrew Shadura <[email protected]>
---
drivers/gpu/drm/amd/powerplay/hwmgr/hardwaremanager.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/amd/powerplay/hwmgr/hardwaremanager.c b/drivers/gpu/drm/amd/powerplay/hwmgr/hardwaremanager.c
index 789f98a..82038b08 100644
--- a/drivers/gpu/drm/amd/powerplay/hwmgr/hardwaremanager.c
+++ b/drivers/gpu/drm/amd/powerplay/hwmgr/hardwaremanager.c
@@ -306,7 +306,7 @@ bool phm_check_smc_update_required_for_display_configuration(struct pp_hwmgr *hw
PHM_FUNC_CHECK(hwmgr);
if (hwmgr->hwmgr_func->check_smc_update_required_for_display_configuration == NULL)
- return -EINVAL;
+ return false;
return hwmgr->hwmgr_func->check_smc_update_required_for_display_configuration(hwmgr);
}
--
2.7.4
On 03/11/16 17:39, Deucher, Alexander wrote:
>> Returning -EINVAL from a bool-returning function
>> phm_check_smc_update_required_for_display_configuration has an
>> unexpected
>> effect of returning true, which is probably not what was intended.
>> Replace -EINVAL by false.
>>
>> The only place this function is called from is
>> psm_adjust_power_state_dynamic in
>> drivers/gpu/drm/amd/powerplay/eventmgr/psm.c:106:
>>
>> if (!equal ||
>> phm_check_smc_update_required_for_display_configuration(hwmgr)) {
>> phm_apply_state_adjust_rules(hwmgr, requested,
>> pcurrent);
>> phm_set_power_state(hwmgr, &pcurrent->hardware,
>> &requested->hardware);
>> hwmgr->current_ps = requested;
>> }
>>
>> It seems to expect a boolean value here.
>>
>> This issue has been found using the following Coccinelle semantic patch
>> written by Peter Senna Tschudin:
>> <smpl>
>> @@
>> identifier f;
>> constant C;
>> typedef bool;
>> @@
>> bool f (...){
>> <+...
>> * return -C;
>> ...+>
>> }
>> </smpl>
>>
>> Signed-off-by: Andrew Shadura <[email protected]>
>
> Reviewed-by: Alex Deucher <[email protected]>
>
> I'll pick this up shortly. Thanks!
Great, thanks! I wonder who should I prod for this one:
https://patchwork.kernel.org/patch/7206081/
--
Cheers,
Andrew
> -----Original Message-----
> From: Andrew Shadura [mailto:[email protected]]
> Sent: Thursday, November 03, 2016 12:42 PM
> To: Deucher, Alexander; [email protected]; dri-
> [email protected]; Koenig, Christian; David Airlie
> Cc: Zhu, Rex; Jammy Zhou
> Subject: Re: [PATCH] drm/amd/powerplay: return false instead of -EINVAL
>
> On 03/11/16 17:39, Deucher, Alexander wrote:
> >> Returning -EINVAL from a bool-returning function
> >> phm_check_smc_update_required_for_display_configuration has an
> >> unexpected
> >> effect of returning true, which is probably not what was intended.
> >> Replace -EINVAL by false.
> >>
> >> The only place this function is called from is
> >> psm_adjust_power_state_dynamic in
> >> drivers/gpu/drm/amd/powerplay/eventmgr/psm.c:106:
> >>
> >> if (!equal ||
> >> phm_check_smc_update_required_for_display_configuration(hwmgr)) {
> >> phm_apply_state_adjust_rules(hwmgr, requested,
> >> pcurrent);
> >> phm_set_power_state(hwmgr, &pcurrent->hardware,
> >> &requested->hardware);
> >> hwmgr->current_ps = requested;
> >> }
> >>
> >> It seems to expect a boolean value here.
> >>
> >> This issue has been found using the following Coccinelle semantic patch
> >> written by Peter Senna Tschudin:
> >> <smpl>
> >> @@
> >> identifier f;
> >> constant C;
> >> typedef bool;
> >> @@
> >> bool f (...){
> >> <+...
> >> * return -C;
> >> ...+>
> >> }
> >> </smpl>
> >>
> >> Signed-off-by: Andrew Shadura <[email protected]>
> >
> > Reviewed-by: Alex Deucher <[email protected]>
> >
> > I'll pick this up shortly. Thanks!
>
> Great, thanks! I wonder who should I prod for this one:
> https://patchwork.kernel.org/patch/7206081/
PowerPC developers?
Alex
>
> --
> Cheers,
> Andrew
> -----Original Message-----
> From: Andrew Shadura [mailto:[email protected]]
> Sent: Thursday, November 03, 2016 6:09 AM
> To: [email protected]; [email protected]; Deucher,
> Alexander; Koenig, Christian; David Airlie
> Cc: Zhu, Rex; Jammy Zhou
> Subject: [PATCH] drm/amd/powerplay: return false instead of -EINVAL
>
> Returning -EINVAL from a bool-returning function
> phm_check_smc_update_required_for_display_configuration has an
> unexpected
> effect of returning true, which is probably not what was intended.
> Replace -EINVAL by false.
>
> The only place this function is called from is
> psm_adjust_power_state_dynamic in
> drivers/gpu/drm/amd/powerplay/eventmgr/psm.c:106:
>
> if (!equal ||
> phm_check_smc_update_required_for_display_configuration(hwmgr)) {
> phm_apply_state_adjust_rules(hwmgr, requested,
> pcurrent);
> phm_set_power_state(hwmgr, &pcurrent->hardware,
> &requested->hardware);
> hwmgr->current_ps = requested;
> }
>
> It seems to expect a boolean value here.
>
> This issue has been found using the following Coccinelle semantic patch
> written by Peter Senna Tschudin:
> <smpl>
> @@
> identifier f;
> constant C;
> typedef bool;
> @@
> bool f (...){
> <+...
> * return -C;
> ...+>
> }
> </smpl>
>
> Signed-off-by: Andrew Shadura <[email protected]>
Reviewed-by: Alex Deucher <[email protected]>
I'll pick this up shortly. Thanks!
Alex
> ---
> drivers/gpu/drm/amd/powerplay/hwmgr/hardwaremanager.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/amd/powerplay/hwmgr/hardwaremanager.c
> b/drivers/gpu/drm/amd/powerplay/hwmgr/hardwaremanager.c
> index 789f98a..82038b08 100644
> --- a/drivers/gpu/drm/amd/powerplay/hwmgr/hardwaremanager.c
> +++ b/drivers/gpu/drm/amd/powerplay/hwmgr/hardwaremanager.c
> @@ -306,7 +306,7 @@ bool
> phm_check_smc_update_required_for_display_configuration(struct
> pp_hwmgr *hw
> PHM_FUNC_CHECK(hwmgr);
>
> if (hwmgr->hwmgr_func-
> >check_smc_update_required_for_display_configuration == NULL)
> - return -EINVAL;
> + return false;
>
> return hwmgr->hwmgr_func-
> >check_smc_update_required_for_display_configuration(hwmgr);
> }
> --
> 2.7.4
>