* [PATCH][next] drm/amd/pp: initialized result to zero before or'ing in data
@ 2018-06-06 12:18 Colin King
2018-06-06 12:19 ` Colin Ian King
2018-06-07 10:32 ` Huang Rui
0 siblings, 2 replies; 3+ messages in thread
From: Colin King @ 2018-06-06 12:18 UTC (permalink / raw)
To: Alex Deucher, Christian König, David Zhou, David Airlie,
Rex Zhu, amd-gfx, dri-devel
Cc: kernel-janitors, linux-kernel
From: Colin Ian King <colin.king@canonical.com>
The current use of result is or'ing in values and checking for
a non-zero result, however, result is not initialized to zero
so it potentially contains garbage to start with. Fix this by
initializing it to the first return from the call to
vega10_program_didt_config_registers.
Detected by cppcheck:
"(error) Uninitialized variable: result"
Fixes: 9b7b8154cdb8 ("drm/amd/powerplay: added didt support for vega10")
Signed-off-by: Colin Ian King <colin.king@canonical.com>
---
drivers/gpu/drm/amd/powerplay/hwmgr/vega10_powertune.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/amd/powerplay/hwmgr/vega10_powertune.c b/drivers/gpu/drm/amd/powerplay/hwmgr/vega10_powertune.c
index a9efd8554fbc..dbe4b1f66784 100644
--- a/drivers/gpu/drm/amd/powerplay/hwmgr/vega10_powertune.c
+++ b/drivers/gpu/drm/amd/powerplay/hwmgr/vega10_powertune.c
@@ -1104,7 +1104,7 @@ static int vega10_enable_psm_gc_edc_config(struct pp_hwmgr *hwmgr)
for (count = 0; count < num_se; count++) {
data = GRBM_GFX_INDEX__INSTANCE_BROADCAST_WRITES_MASK | GRBM_GFX_INDEX__SH_BROADCAST_WRITES_MASK | ( count << GRBM_GFX_INDEX__SE_INDEX__SHIFT);
WREG32_SOC15(GC, 0, mmGRBM_GFX_INDEX, data);
- result |= vega10_program_didt_config_registers(hwmgr, PSMSEEDCStallPatternConfig_Vega10, VEGA10_CONFIGREG_DIDT);
+ result = vega10_program_didt_config_registers(hwmgr, PSMSEEDCStallPatternConfig_Vega10, VEGA10_CONFIGREG_DIDT);
result |= vega10_program_didt_config_registers(hwmgr, PSMSEEDCStallDelayConfig_Vega10, VEGA10_CONFIGREG_DIDT);
result |= vega10_program_didt_config_registers(hwmgr, PSMSEEDCCtrlResetConfig_Vega10, VEGA10_CONFIGREG_DIDT);
result |= vega10_program_didt_config_registers(hwmgr, PSMSEEDCCtrlConfig_Vega10, VEGA10_CONFIGREG_DIDT);
--
2.17.0
^ permalink raw reply related [flat|nested] 3+ messages in thread* Re: [PATCH][next] drm/amd/pp: initialized result to zero before or'ing in data
2018-06-06 12:18 [PATCH][next] drm/amd/pp: initialized result to zero before or'ing in data Colin King
@ 2018-06-06 12:19 ` Colin Ian King
2018-06-07 10:32 ` Huang Rui
1 sibling, 0 replies; 3+ messages in thread
From: Colin Ian King @ 2018-06-06 12:19 UTC (permalink / raw)
To: Alex Deucher, Christian König, David Zhou, David Airlie,
Rex Zhu, amd-gfx, dri-devel
Cc: kernel-janitors, linux-kernel
On 06/06/18 13:18, Colin King wrote:
> From: Colin Ian King <colin.king@canonical.com>
>
> The current use of result is or'ing in values and checking for
> a non-zero result, however, result is not initialized to zero
> so it potentially contains garbage to start with. Fix this by
> initializing it to the first return from the call to
> vega10_program_didt_config_registers.
>
> Detected by cppcheck:
> "(error) Uninitialized variable: result"
>
> Fixes: 9b7b8154cdb8 ("drm/amd/powerplay: added didt support for vega10")
> Signed-off-by: Colin Ian King <colin.king@canonical.com>
> ---
> drivers/gpu/drm/amd/powerplay/hwmgr/vega10_powertune.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/amd/powerplay/hwmgr/vega10_powertune.c b/drivers/gpu/drm/amd/powerplay/hwmgr/vega10_powertune.c
> index a9efd8554fbc..dbe4b1f66784 100644
> --- a/drivers/gpu/drm/amd/powerplay/hwmgr/vega10_powertune.c
> +++ b/drivers/gpu/drm/amd/powerplay/hwmgr/vega10_powertune.c
> @@ -1104,7 +1104,7 @@ static int vega10_enable_psm_gc_edc_config(struct pp_hwmgr *hwmgr)
> for (count = 0; count < num_se; count++) {
> data = GRBM_GFX_INDEX__INSTANCE_BROADCAST_WRITES_MASK | GRBM_GFX_INDEX__SH_BROADCAST_WRITES_MASK | ( count << GRBM_GFX_INDEX__SE_INDEX__SHIFT);
> WREG32_SOC15(GC, 0, mmGRBM_GFX_INDEX, data);
> - result |= vega10_program_didt_config_registers(hwmgr, PSMSEEDCStallPatternConfig_Vega10, VEGA10_CONFIGREG_DIDT);
> + result = vega10_program_didt_config_registers(hwmgr, PSMSEEDCStallPatternConfig_Vega10, VEGA10_CONFIGREG_DIDT);
> result |= vega10_program_didt_config_registers(hwmgr, PSMSEEDCStallDelayConfig_Vega10, VEGA10_CONFIGREG_DIDT);
> result |= vega10_program_didt_config_registers(hwmgr, PSMSEEDCCtrlResetConfig_Vega10, VEGA10_CONFIGREG_DIDT);
> result |= vega10_program_didt_config_registers(hwmgr, PSMSEEDCCtrlConfig_Vega10, VEGA10_CONFIGREG_DIDT);
>
Oops, subject should be "initialize result to before or'ing in data"
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH][next] drm/amd/pp: initialized result to zero before or'ing in data
2018-06-06 12:18 [PATCH][next] drm/amd/pp: initialized result to zero before or'ing in data Colin King
2018-06-06 12:19 ` Colin Ian King
@ 2018-06-07 10:32 ` Huang Rui
1 sibling, 0 replies; 3+ messages in thread
From: Huang Rui @ 2018-06-07 10:32 UTC (permalink / raw)
To: Colin King
Cc: David Airlie, kernel-janitors, linux-kernel, amd-gfx, dri-devel,
Alex Deucher, Rex Zhu, Christian König
On Wed, Jun 06, 2018 at 01:18:31PM +0100, Colin King wrote:
> From: Colin Ian King <colin.king@canonical.com>
>
> The current use of result is or'ing in values and checking for
> a non-zero result, however, result is not initialized to zero
> so it potentially contains garbage to start with. Fix this by
> initializing it to the first return from the call to
> vega10_program_didt_config_registers.
>
> Detected by cppcheck:
> "(error) Uninitialized variable: result"
>
> Fixes: 9b7b8154cdb8 ("drm/amd/powerplay: added didt support for vega10")
> Signed-off-by: Colin Ian King <colin.king@canonical.com>
Nice catch, we must check return values for programming didt registers all
as 0.
Acked-by: Huang Rui <ray.huang@amd.com>
> ---
> drivers/gpu/drm/amd/powerplay/hwmgr/vega10_powertune.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/amd/powerplay/hwmgr/vega10_powertune.c b/drivers/gpu/drm/amd/powerplay/hwmgr/vega10_powertune.c
> index a9efd8554fbc..dbe4b1f66784 100644
> --- a/drivers/gpu/drm/amd/powerplay/hwmgr/vega10_powertune.c
> +++ b/drivers/gpu/drm/amd/powerplay/hwmgr/vega10_powertune.c
> @@ -1104,7 +1104,7 @@ static int vega10_enable_psm_gc_edc_config(struct pp_hwmgr *hwmgr)
> for (count = 0; count < num_se; count++) {
> data = GRBM_GFX_INDEX__INSTANCE_BROADCAST_WRITES_MASK | GRBM_GFX_INDEX__SH_BROADCAST_WRITES_MASK | ( count << GRBM_GFX_INDEX__SE_INDEX__SHIFT);
> WREG32_SOC15(GC, 0, mmGRBM_GFX_INDEX, data);
> - result |= vega10_program_didt_config_registers(hwmgr, PSMSEEDCStallPatternConfig_Vega10, VEGA10_CONFIGREG_DIDT);
> + result = vega10_program_didt_config_registers(hwmgr, PSMSEEDCStallPatternConfig_Vega10, VEGA10_CONFIGREG_DIDT);
> result |= vega10_program_didt_config_registers(hwmgr, PSMSEEDCStallDelayConfig_Vega10, VEGA10_CONFIGREG_DIDT);
> result |= vega10_program_didt_config_registers(hwmgr, PSMSEEDCCtrlResetConfig_Vega10, VEGA10_CONFIGREG_DIDT);
> result |= vega10_program_didt_config_registers(hwmgr, PSMSEEDCCtrlConfig_Vega10, VEGA10_CONFIGREG_DIDT);
> --
> 2.17.0
>
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2018-06-07 10:32 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-06-06 12:18 [PATCH][next] drm/amd/pp: initialized result to zero before or'ing in data Colin King
2018-06-06 12:19 ` Colin Ian King
2018-06-07 10:32 ` Huang Rui
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox