hwmon, fam15h_power: Tweak runavg_range on resume

The quirk introduced with commit 00250ec90963b7ef6678438888f3244985ecde14 (hwmon: fam15h_power: fix bogus values with current BIOSes) is not only required during driver load but also when system resumes from suspend. The BIOS might set the previously recommended (but unsuitable) initilization value for the running average range register during resume.

Signed-off-by Andreas Herrmann <[email protected]>

The quirk introduced with commit 00250ec90963b7ef6678438888f3244985ecde14 (hwmon: fam15h_power: fix bogus values with current BIOSes) is not only required during driver load but also when system resumes from suspend. The BIOS might set the previously recommended (but unsuitable) initilization value for the running average range register during resume.

Signed-off-by Andreas Herrmann <[email protected]>

---
 drivers/hwmon/fam15h_power.c |   15 +++++++++++++--
 1 file changed, 13 insertions(+), 2 deletions(-)

Hi Jean,

Thanks for spotting the issues.
Here's a new patch version, esp. to fix the section mismatch.

Please apply.

Thanks,

Andreas

drivers/hwmon/fam15h_power.c +0 -0

index 2764b78a784b..af69073b3fe8 100644

drivers/hwmon/fam15h_power.c +12 -1

 	{ PCI_VDEVICE(AMD, PCI_DEVICE_ID_AMD_15H_NB_F4) },
 	{ 0 }
 };
 
-static void __devinit tweak_runavg_range(struct pci_dev *pdev)
+static void tweak_runavg_range(struct pci_dev *pdev)
 {
 	u32 val;
 

@@ -158,6 +158,16 @@ static void __devinit tweak_runavg_range(struct pci_dev *pdev)
 		REG_TDP_RUNNING_AVERAGE, val);
 }
 
+#ifdef CONFIG_PM
+static int fam15h_power_resume(struct pci_dev *pdev)
+{
+	tweak_runavg_range(pdev);
+	return 0;
+}
+#else
+#define fam15h_power_resume NULL
+#endif
+
 static void __devinit fam15h_power_init_data(struct pci_dev *f4,
 					     struct fam15h_power_data *data)
 {

@@ -256,6 +266,7 @@ static struct pci_driver fam15h_power_driver = {
 	.id_table = fam15h_power_id_table,
 	.probe = fam15h_power_probe,
 	.remove = __devexit_p(fam15h_power_remove),
+	.resume = fam15h_power_resume,
 };
 
 module_pci_driver(fam15h_power_driver);

Signature · 1 line

1.7.10.4

Quoted 30 lines · Andreas Herrmann

The quirk introduced with commit 00250ec90963b7ef6678438888f3244985ecde14 (hwmon: fam15h_power: fix bogus values with current BIOSes) is not only required during driver load but also when system resumes from suspend. The BIOS might set the previously recommended (but unsuitable) initilization value for the running average range register during resume.

Signed-off-by Andreas Herrmann <[email protected]>

---
 drivers/hwmon/fam15h_power.c |   15 +++++++++++++--
 1 file changed, 13 insertions(+), 2 deletions(-)

Hi Jean,

Thanks for spotting the issues.
Here's a new patch version, esp. to fix the section mismatch.

Please apply.

Thanks,

Andreas

drivers/hwmon/fam15h_power.c +0 -0

index 2764b78a784b..af69073b3fe8 100644

drivers/hwmon/fam15h_power.c static bool __devinit fam15h_power_is_internal_node0(struct pci_dev *f4) · line 132

Quoted 5 lines

drivers/hwmon/fam15h_power.c +1 -1

@@ -129,12 +129,12 @@ static bool __devinit fam15h_power_is_internal_node0(struct pci_dev *f4)
  * counter saturations resulting in bogus power readings.
  * We correct this value ourselves to cope with older BIOSes.
  */
-static DEFINE_PCI_DEVICE_TABLE(affected_device) = {
+static const struct pci_device_id affected_device[] = {

I would prefer if we could keep DEFINE_PCI_DEVICE_TABLE to avoid the checkpatch warning. Should be easy to implement, by moving pci_match_id into the probe function and only call tweak_runavg_range() if it returns true. You could keep the response in fam15h_power_data, as a flag such as needs_tweak, for use in fam15h_power_resume().

Just my $0.02, though. I'll leave it up to Jean to decide if the patch is ok as-is.

Hello Mr. Andreas Herrmann,

I'm trying to measure power consumption of AMD Opteron 6272 using fam15h_power. Linux kernel version is 3.9.4.
Even when all cores are idle, fam15h_power shows about 85W. However, the power consumption decreases to about 74W
when a process is executed with all cores.

For Linux kernel 3.9.4, the patch for fam15h_power is already applied. Moreover, "RunAvgRange" (p. 460 of BKDG for Family 15h) is
appropriately set to "0x9" as following:

% setpci -s 00:18.5 0xe0.l
% 00372c99

Why is the power consumption so high (85W) when all cores are idle? Why does it decrease when a process is executed with all cores? I would appreciate if you could give me a advice.

Sincerely yours,

Satoshi

Hi Satoshi,

Quoted 1 line · Satoshi Imamura

Hello Mr. Andreas Herrmann,

I don't think Andreas is still reading the lm-sensors list, so I am adding him to Cc.

Quoted 20 lines

I'm trying to measure power consumption of AMD Opteron 6272 using fam15h_power. Linux kernel version is 3.9.4.
Even when all cores are idle, fam15h_power shows about 85W. However, the power consumption decreases to about 74W
when a process is executed with all cores.

For Linux kernel 3.9.4, the patch for fam15h_power is already applied. Moreover, "RunAvgRange" (p. 460 of BKDG for Family 15h) is appropriately set to "0x9" as following:

% setpci -s 00:18.5 0xe0.l
% 00372c99

Why is the power consumption so high (85W) when all cores are idle? Why does it decrease when a process is executed with all cores? I would appreciate if you could give me a advice.

Sincerely yours,

Satoshi