Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1514424 > unrolled thread
| Started by | Andrew Shadura <andrew.shadura@collabora.co.uk> |
|---|---|
| First post | 2016-11-03 11:10 +0100 |
| Last post | 2016-11-03 18:00 +0100 |
| Articles | 4 — 2 participants |
Back to article view | Back to linux.kernel
[PATCH] drm/amd/powerplay: return false instead of -EINVAL Andrew Shadura <andrew.shadura@collabora.co.uk> - 2016-11-03 11:10 +0100
RE: [PATCH] drm/amd/powerplay: return false instead of -EINVAL "Deucher, Alexander" <Alexander.Deucher@amd.com> - 2016-11-03 17:50 +0100
Re: [PATCH] drm/amd/powerplay: return false instead of -EINVAL Andrew Shadura <andrew.shadura@collabora.co.uk> - 2016-11-03 17:50 +0100
RE: [PATCH] drm/amd/powerplay: return false instead of -EINVAL "Deucher, Alexander" <Alexander.Deucher@amd.com> - 2016-11-03 18:00 +0100
| From | Andrew Shadura <andrew.shadura@collabora.co.uk> |
|---|---|
| Date | 2016-11-03 11:10 +0100 |
| Subject | [PATCH] drm/amd/powerplay: return false instead of -EINVAL |
| Message-ID | <szn85-2uf-15@gated-at.bofh.it> |
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 <andrew.shadura@collabora.co.uk>
---
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
[toc] | [next] | [standalone]
| From | "Deucher, Alexander" <Alexander.Deucher@amd.com> |
|---|---|
| Date | 2016-11-03 17:50 +0100 |
| Message-ID | <sztnb-6qT-5@gated-at.bofh.it> |
| In reply to | #1514424 |
> -----Original Message-----
> From: Andrew Shadura [mailto:andrew.shadura@collabora.co.uk]
> Sent: Thursday, November 03, 2016 12:42 PM
> To: Deucher, Alexander; linux-kernel@vger.kernel.org; dri-
> devel@lists.freedesktop.org; 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 <andrew.shadura@collabora.co.uk>
> >
> > Reviewed-by: Alex Deucher <alexander.deucher@amd.com>
> >
> > 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
[toc] | [prev] | [next] | [standalone]
| From | Andrew Shadura <andrew.shadura@collabora.co.uk> |
|---|---|
| Date | 2016-11-03 17:50 +0100 |
| Message-ID | <sztnb-6qT-9@gated-at.bofh.it> |
| In reply to | #1514424 |
[Multipart message — attachments visible in raw view] — view raw
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 <andrew.shadura@collabora.co.uk>
>
> Reviewed-by: Alex Deucher <alexander.deucher@amd.com>
>
> 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
[toc] | [prev] | [next] | [standalone]
| From | "Deucher, Alexander" <Alexander.Deucher@amd.com> |
|---|---|
| Date | 2016-11-03 18:00 +0100 |
| Message-ID | <sztnb-6qT-7@gated-at.bofh.it> |
| In reply to | #1514424 |
> -----Original Message-----
> From: Andrew Shadura [mailto:andrew.shadura@collabora.co.uk]
> Sent: Thursday, November 03, 2016 6:09 AM
> To: linux-kernel@vger.kernel.org; dri-devel@lists.freedesktop.org; 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 <andrew.shadura@collabora.co.uk>
Reviewed-by: Alex Deucher <alexander.deucher@amd.com>
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
>
[toc] | [prev] | [standalone]
Back to top | Article view | linux.kernel
csiph-web