Skip to contentSkip to conversation

drm/amd/powerplay: Minor fixes in processpptables.c

PATCH In review Ernst Sjöstrand 2 participants 9 messages Nov 19, 2017 – Nov 21, 2017 3 messages not in the archive

Subsystems (2)
DRM drivers Radeon and amdgpu DRM drivers
lore
Share & tools
lore

Ernst Sjöstrand

author
#0 lore raw

PATCH 1/5 drm/amd/powerplay: Minor fixes in processpptables.c

Reported by smatch:
init_overdrive_limits() error: uninitialized symbol 'result'. get_clock_voltage_dependency_table() warn: inconsistent indenting

Signed-off-by Ernst Sjöstrand <[email protected]>
---
 drivers/gpu/drm/amd/powerplay/hwmgr/processpptables.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)
drivers/gpu/drm/amd/powerplay/hwmgr/processpptables.c +3 -3
index afae32ee2b0d..7c5b426320f1 100644
@@ -394,8 +394,8 @@ static int get_clock_voltage_dependency_table(struct pp_hwmgr *hwmgr,
 		dep_table->entries[i].clk =
 			((unsigned long)table->entries[i].ucClockHigh << 16) |
 			le16_to_cpu(table->entries[i].usClockLow);
-			dep_table->entries[i].v =
-				(unsigned long)le16_to_cpu(table->entries[i].usVoltage);
+		dep_table->entries[i].v =
+			(unsigned long)le16_to_cpu(table->entries[i].usVoltage);
 	}
 
 	*ptable = dep_table;

@@ -1042,7 +1042,7 @@ static int init_overdrive_limits_V2_1(struct pp_hwmgr *hwmgr,
 static int init_overdrive_limits(struct pp_hwmgr *hwmgr,
 			const ATOM_PPLIB_POWERPLAYTABLE *powerplay_table)
 {
-	int result;
+	int result = 1;
 	uint8_t frev, crev;
 	uint16_t size;
Signature · 6 lines
2.14.1

_______________________________________________
amd-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

A message that is not in the archive (sent off-list or not yet received): 20171119175246.4791-1-ernstp-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org

author
#2 lore raw

PATCH 2/5 drm/amd/powerplay: Fix missing newlines at end of file

Signed-off-by Ernst Sjöstrand <[email protected]>
---
 drivers/gpu/drm/amd/include/kgd_pp_interface.h      | 2 +-
 drivers/gpu/drm/amd/powerplay/hwmgr/pp_overdriver.h | 2 +-
 drivers/gpu/drm/amd/powerplay/hwmgr/ppatomfwctrl.c  | 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)
drivers/gpu/drm/amd/include/kgd_pp_interface.h +1 -1
index eab504ecca25..ed27626dff14 100644
@@ -291,4 +291,4 @@ struct amd_pm_funcs {
 		struct amd_pp_simple_clock_info *clocks);
 };
 
-#endif
\ No newline at end of file
+#endif
drivers/gpu/drm/amd/powerplay/hwmgr/pp_overdriver.h +1 -1
index c6ba0d64cfb7..4112a9398163 100644
@@ -43,4 +43,4 @@ struct phm_fuses_default {
 extern int pp_override_get_default_fuse_value(uint64_t key,
 			struct phm_fuses_default *result);
 
-#endif
\ No newline at end of file
+#endif
drivers/gpu/drm/amd/powerplay/hwmgr/ppatomfwctrl.c +1 -1
index c062844b15f3..560c1c159fcc 100644
@@ -542,4 +542,4 @@ int pp_atomfwctrl_get_vbios_bootup_values(struct pp_hwmgr *hwmgr,
 		boot_values->ulDCEFClk   = frequency;
 
 	return 0;
-}
\ No newline at end of file
+}
Signature · 6 lines
2.14.1

_______________________________________________
amd-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
author
#3 lore raw

PATCH 3/5 drm/amd/amdgpu: Fix missing null check in atombios_i2c.c

In reply to a message not in the archive

Reported by smatch:
amdgpu_atombios_i2c_process_i2c_ch() error: we previously assumed 'buf' could be null

Signed-off-by Ernst Sjöstrand <[email protected]>
---
 drivers/gpu/drm/amd/amdgpu/atombios_i2c.c | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)
drivers/gpu/drm/amd/amdgpu/atombios_i2c.c +9 -2
index b374653bd6cf..f9b2ce9a98f3 100644
@@ -65,8 +65,15 @@ static int amdgpu_atombios_i2c_process_i2c_ch(struct amdgpu_i2c_chan *chan,
 			args.ucRegIndex = buf[0];
 		if (num)
 			num--;
-		if (num)
-			memcpy(&out, &buf[1], num);
+		if (num) {
+			if (buf) {
+				memcpy(&out, &buf[1], num);
+			} else {
+				DRM_ERROR("hw i2c: missing buf with num > 1\n");
+				r = -EINVAL;
+				goto done;
+			}
+		}
 		args.lpI2CDataOut = cpu_to_le16(out);
 	} else {
 		if (num > ATOM_MAX_HW_I2C_READ) {
Signature · 6 lines
2.14.1

_______________________________________________
amd-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
author
#4 lore raw

PATCH 4/5 drm/amd/powerplay: Fix buffer overflows with mc_reg_address

In reply to a message not in the archive

Smatch warned about the following lines:
ci_set_mc_special_registers() error: buffer overflow 'table->mc_reg_address' 16 <= 16 tonga_set_mc_special_registers() error: buffer overflow 'table->mc_reg_address' 16 <= 16

Change the logic to check before access instead of after incrementing. It's fine if j reaches max after we're done. This allows the last entry of the array to be filled without an error message for example. Changed some whitespace to clarify grouping.

Signed-off-by Ernst Sjöstrand <[email protected]>
---
 drivers/gpu/drm/amd/amdgpu/ci_dpm.c                 | 10 +++-------
 drivers/gpu/drm/amd/powerplay/smumgr/tonga_smumgr.c | 10 +++-------
 2 files changed, 6 insertions(+), 14 deletions(-)
drivers/gpu/drm/amd/amdgpu/ci_dpm.c +3 -7
index 5a60c161b0fc..f11c0aacf19f 100644
@@ -4540,9 +4540,9 @@ static int ci_set_mc_special_registers(struct amdgpu_device *adev,
 					((temp_reg & 0xffff0000)) | ((table->mc_reg_table_entry[k].mc_data[i] & 0xffff0000) >> 16);
 			}
 			j++;
+
 			if (j >= SMU7_DISCRETE_MC_REGISTER_ARRAY_SIZE)
 				return -EINVAL;
-
 			temp_reg = RREG32(mmMC_PMG_CMD_MRS);
 			table->mc_reg_address[j].s1 = mmMC_PMG_CMD_MRS;
 			table->mc_reg_address[j].s0 = mmMC_SEQ_PMG_CMD_MRS_LP;

@@ -4553,10 +4553,10 @@ static int ci_set_mc_special_registers(struct amdgpu_device *adev,
 					table->mc_reg_table_entry[k].mc_data[j] |= 0x100;
 			}
 			j++;
-			if (j > SMU7_DISCRETE_MC_REGISTER_ARRAY_SIZE)
-				return -EINVAL;
 
 			if (adev->mc.vram_type != AMDGPU_VRAM_TYPE_GDDR5) {
+				if (j >= SMU7_DISCRETE_MC_REGISTER_ARRAY_SIZE)
+					return -EINVAL;
 				table->mc_reg_address[j].s1 = mmMC_PMG_AUTO_CMD;
 				table->mc_reg_address[j].s0 = mmMC_PMG_AUTO_CMD;
 				for (k = 0; k < table->num_entries; k++) {

@@ -4564,8 +4564,6 @@ static int ci_set_mc_special_registers(struct amdgpu_device *adev,
 						(table->mc_reg_table_entry[k].mc_data[i] & 0xffff0000) >> 16;
 				}
 				j++;
-				if (j > SMU7_DISCRETE_MC_REGISTER_ARRAY_SIZE)
-					return -EINVAL;
 			}
 			break;
 		case mmMC_SEQ_RESERVE_M:

@@ -4577,8 +4575,6 @@ static int ci_set_mc_special_registers(struct amdgpu_device *adev,
 					(temp_reg & 0xffff0000) | (table->mc_reg_table_entry[k].mc_data[i] & 0x0000ffff);
 			}
 			j++;
-			if (j > SMU7_DISCRETE_MC_REGISTER_ARRAY_SIZE)
-				return -EINVAL;
 			break;
 		default:
 			break;
drivers/gpu/drm/amd/powerplay/smumgr/tonga_smumgr.c +3 -7
index 0a8e48bff219..81b8790c0d22 100644
@@ -3106,9 +3106,9 @@ static int tonga_set_mc_special_registers(struct pp_hwmgr *hwmgr,
 					((table->mc_reg_table_entry[k].mc_data[i] & 0xffff0000) >> 16);
 			}
 			j++;
+
 			PP_ASSERT_WITH_CODE((j < SMU72_DISCRETE_MC_REGISTER_ARRAY_SIZE),
 				"Invalid VramInfo table.", return -EINVAL);
-
 			temp_reg = cgs_read_register(hwmgr->device, mmMC_PMG_CMD_MRS);
 			table->mc_reg_address[j].s1 = mmMC_PMG_CMD_MRS;
 			table->mc_reg_address[j].s0 = mmMC_SEQ_PMG_CMD_MRS_LP;

@@ -3121,18 +3121,16 @@ static int tonga_set_mc_special_registers(struct pp_hwmgr *hwmgr,
 					table->mc_reg_table_entry[k].mc_data[j] |= 0x100;
 			}
 			j++;
-			PP_ASSERT_WITH_CODE((j <= SMU72_DISCRETE_MC_REGISTER_ARRAY_SIZE),
-				"Invalid VramInfo table.", return -EINVAL);
 
 			if (!data->is_memory_gddr5) {
+				PP_ASSERT_WITH_CODE((j < SMU72_DISCRETE_MC_REGISTER_ARRAY_SIZE),
+					"Invalid VramInfo table.", return -EINVAL);
 				table->mc_reg_address[j].s1 = mmMC_PMG_AUTO_CMD;
 				table->mc_reg_address[j].s0 = mmMC_PMG_AUTO_CMD;
 				for (k = 0; k < table->num_entries; k++)
 					table->mc_reg_table_entry[k].mc_data[j] =
 						(table->mc_reg_table_entry[k].mc_data[i] & 0xffff0000) >> 16;
 				j++;
-				PP_ASSERT_WITH_CODE((j <= SMU72_DISCRETE_MC_REGISTER_ARRAY_SIZE),
-					"Invalid VramInfo table.", return -EINVAL);
 			}
 
 			break;

@@ -3147,8 +3145,6 @@ static int tonga_set_mc_special_registers(struct pp_hwmgr *hwmgr,
 					(table->mc_reg_table_entry[k].mc_data[i] & 0x0000ffff);
 			}
 			j++;
-			PP_ASSERT_WITH_CODE((j <= SMU72_DISCRETE_MC_REGISTER_ARRAY_SIZE),
-				"Invalid VramInfo table.", return -EINVAL);
 			break;
 
 		default:
Signature · 6 lines
2.14.1

_______________________________________________
amd-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
author
#5 lore raw

PATCH 5/5 drm/amd/powerplay: Followup fixes to mc_reg_address

In reply to a message not in the archive

This is a followup to:
drm/amd/powerplay: Fix buffer overflows with mc_reg_address

Rework *_set_mc_special_registers for the other architectures to use the same logic as the first patch. This allows the last entry of the array to be filled without an error message for example. This doesn't fix any known problems, perhaps avoided by luck.

Signed-off-by Ernst Sjöstrand <[email protected]>
---
 drivers/gpu/drm/amd/amdgpu/si_dpm.c                   | 10 +++-------
 drivers/gpu/drm/amd/powerplay/smumgr/ci_smumgr.c      | 12 ++++--------
 drivers/gpu/drm/amd/powerplay/smumgr/iceland_smumgr.c | 12 ++++--------
 3 files changed, 11 insertions(+), 23 deletions(-)
drivers/gpu/drm/amd/amdgpu/si_dpm.c +3 -7
index 51fd0c9a20a5..299cb3161b2c 100644
@@ -5845,9 +5845,9 @@ static int si_set_mc_special_registers(struct amdgpu_device *adev,
 					((temp_reg & 0xffff0000)) |
 					((table->mc_reg_table_entry[k].mc_data[i] & 0xffff0000) >> 16);
 			j++;
+
 			if (j >= SMC_SISLANDS_MC_REGISTER_ARRAY_SIZE)
 				return -EINVAL;
-
 			temp_reg = RREG32(MC_PMG_CMD_MRS);
 			table->mc_reg_address[j].s1 = MC_PMG_CMD_MRS;
 			table->mc_reg_address[j].s0 = MC_SEQ_PMG_CMD_MRS_LP;

@@ -5859,18 +5859,16 @@ static int si_set_mc_special_registers(struct amdgpu_device *adev,
 					table->mc_reg_table_entry[k].mc_data[j] |= 0x100;
 			}
 			j++;
-			if (j >= SMC_SISLANDS_MC_REGISTER_ARRAY_SIZE)
-				return -EINVAL;
 
 			if (adev->mc.vram_type != AMDGPU_VRAM_TYPE_GDDR5) {
+				if (j >= SMC_SISLANDS_MC_REGISTER_ARRAY_SIZE)
+					return -EINVAL;
 				table->mc_reg_address[j].s1 = MC_PMG_AUTO_CMD;
 				table->mc_reg_address[j].s0 = MC_PMG_AUTO_CMD;
 				for (k = 0; k < table->num_entries; k++)
 					table->mc_reg_table_entry[k].mc_data[j] =
 						(table->mc_reg_table_entry[k].mc_data[i] & 0xffff0000) >> 16;
 				j++;
-				if (j >= SMC_SISLANDS_MC_REGISTER_ARRAY_SIZE)
-					return -EINVAL;
 			}
 			break;
 		case MC_SEQ_RESERVE_M:

@@ -5882,8 +5880,6 @@ static int si_set_mc_special_registers(struct amdgpu_device *adev,
 					(temp_reg & 0xffff0000) |
 					(table->mc_reg_table_entry[k].mc_data[i] & 0x0000ffff);
 			j++;
-			if (j >= SMC_SISLANDS_MC_REGISTER_ARRAY_SIZE)
-				return -EINVAL;
 			break;
 		default:
 			break;
drivers/gpu/drm/amd/powerplay/smumgr/ci_smumgr.c +4 -8
index ed4b37e566a3..c36f00ef46f3 100644
@@ -2600,9 +2600,9 @@ static int ci_set_mc_special_registers(struct pp_hwmgr *hwmgr,
 					((table->mc_reg_table_entry[k].mc_data[i] & 0xffff0000) >> 16);
 			}
 			j++;
+
 			PP_ASSERT_WITH_CODE((j < SMU7_DISCRETE_MC_REGISTER_ARRAY_SIZE),
 				"Invalid VramInfo table.", return -EINVAL);
-
 			temp_reg = cgs_read_register(hwmgr->device, mmMC_PMG_CMD_MRS);
 			table->mc_reg_address[j].s1 = mmMC_PMG_CMD_MRS;
 			table->mc_reg_address[j].s0 = mmMC_SEQ_PMG_CMD_MRS_LP;

@@ -2615,10 +2615,10 @@ static int ci_set_mc_special_registers(struct pp_hwmgr *hwmgr,
 					table->mc_reg_table_entry[k].mc_data[j] |= 0x100;
 			}
 			j++;
-			PP_ASSERT_WITH_CODE((j <= SMU7_DISCRETE_MC_REGISTER_ARRAY_SIZE),
-				"Invalid VramInfo table.", return -EINVAL);
 
-			if (!data->is_memory_gddr5 && j < SMU7_DISCRETE_MC_REGISTER_ARRAY_SIZE) {
+			if (!data->is_memory_gddr5) {
+				PP_ASSERT_WITH_CODE((j < SMU7_DISCRETE_MC_REGISTER_ARRAY_SIZE),
+					"Invalid VramInfo table.", return -EINVAL);
 				table->mc_reg_address[j].s1 = mmMC_PMG_AUTO_CMD;
 				table->mc_reg_address[j].s0 = mmMC_PMG_AUTO_CMD;
 				for (k = 0; k < table->num_entries; k++) {

@@ -2626,8 +2626,6 @@ static int ci_set_mc_special_registers(struct pp_hwmgr *hwmgr,
 						(table->mc_reg_table_entry[k].mc_data[i] & 0xffff0000) >> 16;
 				}
 				j++;
-				PP_ASSERT_WITH_CODE((j <= SMU7_DISCRETE_MC_REGISTER_ARRAY_SIZE),
-					"Invalid VramInfo table.", return -EINVAL);
 			}
 
 			break;

@@ -2642,8 +2640,6 @@ static int ci_set_mc_special_registers(struct pp_hwmgr *hwmgr,
 					(table->mc_reg_table_entry[k].mc_data[i] & 0x0000ffff);
 			}
 			j++;
-			PP_ASSERT_WITH_CODE((j <= SMU7_DISCRETE_MC_REGISTER_ARRAY_SIZE),
-				"Invalid VramInfo table.", return -EINVAL);
 			break;
 
 		default:
drivers/gpu/drm/amd/powerplay/smumgr/iceland_smumgr.c +4 -8
index 2ff682d44e8c..d62078681cae 100644
@@ -2549,9 +2549,9 @@ static int iceland_set_mc_special_registers(struct pp_hwmgr *hwmgr,
 					((table->mc_reg_table_entry[k].mc_data[i] & 0xffff0000) >> 16);
 			}
 			j++;
+
 			PP_ASSERT_WITH_CODE((j < SMU71_DISCRETE_MC_REGISTER_ARRAY_SIZE),
 				"Invalid VramInfo table.", return -EINVAL);
-
 			temp_reg = cgs_read_register(hwmgr->device, mmMC_PMG_CMD_MRS);
 			table->mc_reg_address[j].s1 = mmMC_PMG_CMD_MRS;
 			table->mc_reg_address[j].s0 = mmMC_SEQ_PMG_CMD_MRS_LP;

@@ -2565,10 +2565,10 @@ static int iceland_set_mc_special_registers(struct pp_hwmgr *hwmgr,
 				}
 			}
 			j++;
-			PP_ASSERT_WITH_CODE((j <= SMU71_DISCRETE_MC_REGISTER_ARRAY_SIZE),
-				"Invalid VramInfo table.", return -EINVAL);
 
-			if (!data->is_memory_gddr5 && j < SMU71_DISCRETE_MC_REGISTER_ARRAY_SIZE) {
+			if (!data->is_memory_gddr5) {
+				PP_ASSERT_WITH_CODE((j < SMU71_DISCRETE_MC_REGISTER_ARRAY_SIZE),
+					"Invalid VramInfo table.", return -EINVAL);
 				table->mc_reg_address[j].s1 = mmMC_PMG_AUTO_CMD;
 				table->mc_reg_address[j].s0 = mmMC_PMG_AUTO_CMD;
 				for (k = 0; k < table->num_entries; k++) {

@@ -2576,8 +2576,6 @@ static int iceland_set_mc_special_registers(struct pp_hwmgr *hwmgr,
 						(table->mc_reg_table_entry[k].mc_data[i] & 0xffff0000) >> 16;
 				}
 				j++;
-				PP_ASSERT_WITH_CODE((j <= SMU71_DISCRETE_MC_REGISTER_ARRAY_SIZE),
-					"Invalid VramInfo table.", return -EINVAL);
 			}
 
 			break;

@@ -2592,8 +2590,6 @@ static int iceland_set_mc_special_registers(struct pp_hwmgr *hwmgr,
 					(table->mc_reg_table_entry[k].mc_data[i] & 0x0000ffff);
 			}
 			j++;
-			PP_ASSERT_WITH_CODE((j <= SMU71_DISCRETE_MC_REGISTER_ARRAY_SIZE),
-				"Invalid VramInfo table.", return -EINVAL);
 			break;
 
 		default:
Signature · 6 lines
2.14.1

_______________________________________________
amd-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
#6 lore raw
In reply to a message not in the archive
Quoted 25 lines · Ernst Sjöstrand

Reported by smatch:
init_overdrive_limits() error: uninitialized symbol 'result'. get_clock_voltage_dependency_table() warn: inconsistent indenting

Signed-off-by Ernst Sjöstrand <[email protected]>
---
 drivers/gpu/drm/amd/powerplay/hwmgr/processpptables.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)
drivers/gpu/drm/amd/powerplay/hwmgr/processpptables.c +2 -2
index afae32ee2b0d..7c5b426320f1 100644
@@ -394,8 +394,8 @@ static int get_clock_voltage_dependency_table(struct pp_hwmgr *hwmgr,
                dep_table->entries[i].clk =
                        ((unsigned long)table->entries[i].ucClockHigh << 16) |
                        le16_to_cpu(table->entries[i].usClockLow);
-                       dep_table->entries[i].v =
-                               (unsigned long)le16_to_cpu(table->entries[i].usVoltage);
+               dep_table->entries[i].v =
+                       (unsigned long)le16_to_cpu(table->entries[i].usVoltage);
        }
 
        *ptable = dep_table;

drivers/gpu/drm/amd/powerplay/hwmgr/processpptables.c static int init_overdrive_limits_V2_1(struct pp_hwmgr *hwmgr, · line 1045

Quoted 5 lines
drivers/gpu/drm/amd/powerplay/hwmgr/processpptables.c +1 -1
@@ -1042,7 +1042,7 @@ static int init_overdrive_limits_V2_1(struct pp_hwmgr *hwmgr,
 static int init_overdrive_limits(struct pp_hwmgr *hwmgr,
                        const ATOM_PPLIB_POWERPLAYTABLE *powerplay_table)
 {
-       int result;
+       int result = 1;

I think this should probably be initialized to 0.

Alex

Quoted 10 lines
drivers/gpu/drm/amd/powerplay/hwmgr/processpptables.c +0 -1
        uint8_t frev, crev;
        uint16_t size;
 
--

2.14.1

_______________________________________________
amd-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

_______________________________________________
amd-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

A message that is not in the archive (sent off-list or not yet received): CADnq5_PxZ98J61C3WY+LfZsYWiwnJp9qhyiG9PYsCNrYetnsFw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org

#8 lore raw
Quoted 24 lines · Alex Deucher
Quoted 23 lines · Ernst Sjöstrand

Reported by smatch:
init_overdrive_limits() error: uninitialized symbol 'result'. get_clock_voltage_dependency_table() warn: inconsistent indenting

Signed-off-by Ernst Sjöstrand <[email protected]>
---
 drivers/gpu/drm/amd/powerplay/hwmgr/processpptables.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)
drivers/gpu/drm/amd/powerplay/hwmgr/processpptables.c +2 -2
index afae32ee2b0d..7c5b426320f1 100644
@@ -394,8 +394,8 @@ static int get_clock_voltage_dependency_table(struct pp_hwmgr *hwmgr,
                dep_table->entries[i].clk =
                        ((unsigned long)table->entries[i].ucClockHigh << 16) |
                        le16_to_cpu(table->entries[i].usClockLow);
-                       dep_table->entries[i].v =
-                               (unsigned long)le16_to_cpu(table->entries[i].usVoltage);
+               dep_table->entries[i].v =
+                       (unsigned long)le16_to_cpu(table->entries[i].usVoltage);
        }
Quoted 9 lines

drivers/gpu/drm/amd/powerplay/hwmgr/processpptables.c static int init_overdrive_limits_V2_1(struct pp_hwmgr *hwmgr, · line 1045

Quoted 7 lines
drivers/gpu/drm/amd/powerplay/hwmgr/processpptables.c +1 -1
        *ptable = dep_table;

@@ -1042,7 +1042,7 @@ static int init_overdrive_limits_V2_1(struct pp_hwmgr *hwmgr,
 static int init_overdrive_limits(struct pp_hwmgr *hwmgr,
                        const ATOM_PPLIB_POWERPLAYTABLE *powerplay_table)
 {
-       int result;
+       int result = 1;

I think this should probably be initialized to 0.

Applied the series with that fixed up locally.

Thanks!

Alex

Quoted 13 lines

Alex

Quoted 10 lines
drivers/gpu/drm/amd/powerplay/hwmgr/processpptables.c +0 -1
        uint8_t frev, crev;
        uint16_t size;
 
--

2.14.1

_______________________________________________
amd-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

_______________________________________________
amd-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

author
#9 lore raw
In reply to a message not in the archive

I think my idea here was that if it's not updated later for some reason we shouldn't silently return success.
But I'm guessing this can't happen with current hardware at least.

Regards
//Ernst

2017-11-21 16:15 GMT+01:00 Alex Deucher <[email protected]>:

Quoted 34 lines · Ernst Sjöstrand
Quoted 25 lines

Reported by smatch:
init_overdrive_limits() error: uninitialized symbol 'result'. get_clock_voltage_dependency_table() warn: inconsistent indenting

Signed-off-by Ernst Sjöstrand <[email protected]>
---
 drivers/gpu/drm/amd/powerplay/hwmgr/processpptables.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)
drivers/gpu/drm/amd/powerplay/hwmgr/processpptables.c +2 -2
index afae32ee2b0d..7c5b426320f1 100644
@@ -394,8 +394,8 @@ static int get_clock_voltage_dependency_table(struct pp_hwmgr *hwmgr,
                dep_table->entries[i].clk =
                        ((unsigned long)table->entries[i].ucClockHigh << 16) |
                        le16_to_cpu(table->entries[i].usClockLow);
-                       dep_table->entries[i].v =
-                               (unsigned long)le16_to_cpu(table->entries[i].usVoltage);
+               dep_table->entries[i].v =
+                       (unsigned long)le16_to_cpu(table->entries[i].usVoltage);
        }
 
        *ptable = dep_table;

drivers/gpu/drm/amd/powerplay/hwmgr/processpptables.c static int init_overdrive_limits_V2_1(struct pp_hwmgr *hwmgr, · line 1045

Quoted 5 lines
drivers/gpu/drm/amd/powerplay/hwmgr/processpptables.c +1 -1
@@ -1042,7 +1042,7 @@ static int init_overdrive_limits_V2_1(struct pp_hwmgr *hwmgr,
 static int init_overdrive_limits(struct pp_hwmgr *hwmgr,
                        const ATOM_PPLIB_POWERPLAYTABLE *powerplay_table)
 {
-       int result;
+       int result = 1;

I think this should probably be initialized to 0.

Quoted 12 lines

Alex

Quoted 10 lines
drivers/gpu/drm/amd/powerplay/hwmgr/processpptables.c +0 -1
        uint8_t frev, crev;
        uint16_t size;
 
--

2.14.1

_______________________________________________
amd-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

_______________________________________________
amd-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

A message that is not in the archive (sent off-list or not yet received): CAD=4a=WLNephGaRm1gVWNt8+4CbfmMWZbSJUNTWGYhFJH8dOjQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org

#11 lore raw
Quoted 3 lines · Ernst Sjöstrand

I think my idea here was that if it's not updated later for some reason we shouldn't silently return success.
But I'm guessing this can't happen with current hardware at least.

Right we shouldn't actually hit this case, but if we did, it's fine to return success.

Alex

Quoted 51 lines

Regards
//Ernst

2017-11-21 16:15 GMT+01:00 Alex Deucher <[email protected]>:

Quoted 46 lines · Ernst Sjöstrand
Quoted 25 lines

Reported by smatch:
init_overdrive_limits() error: uninitialized symbol 'result'. get_clock_voltage_dependency_table() warn: inconsistent indenting

Signed-off-by Ernst Sjöstrand <[email protected]>
---
 drivers/gpu/drm/amd/powerplay/hwmgr/processpptables.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)
drivers/gpu/drm/amd/powerplay/hwmgr/processpptables.c +2 -2
index afae32ee2b0d..7c5b426320f1 100644
@@ -394,8 +394,8 @@ static int get_clock_voltage_dependency_table(struct pp_hwmgr *hwmgr,
                dep_table->entries[i].clk =
                        ((unsigned long)table->entries[i].ucClockHigh << 16) |
                        le16_to_cpu(table->entries[i].usClockLow);
-                       dep_table->entries[i].v =
-                               (unsigned long)le16_to_cpu(table->entries[i].usVoltage);
+               dep_table->entries[i].v =
+                       (unsigned long)le16_to_cpu(table->entries[i].usVoltage);
        }
 
        *ptable = dep_table;

drivers/gpu/drm/amd/powerplay/hwmgr/processpptables.c static int init_overdrive_limits_V2_1(struct pp_hwmgr *hwmgr, · line 1045

Quoted 5 lines
drivers/gpu/drm/amd/powerplay/hwmgr/processpptables.c +1 -1
@@ -1042,7 +1042,7 @@ static int init_overdrive_limits_V2_1(struct pp_hwmgr *hwmgr,
 static int init_overdrive_limits(struct pp_hwmgr *hwmgr,
                        const ATOM_PPLIB_POWERPLAYTABLE *powerplay_table)
 {
-       int result;
+       int result = 1;

I think this should probably be initialized to 0.

Alex

Quoted 10 lines
drivers/gpu/drm/amd/powerplay/hwmgr/processpptables.c +0 -1
        uint8_t frev, crev;
        uint16_t size;
 
--

2.14.1

_______________________________________________
amd-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/amd-gfx

_______________________________________________
amd-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/amd-gfx