Re: [lm-sensors] [PATCH] hwmon: fam15h_power: fix bogus values with current BIOSes

[Andre Przywara <andre.przywara@amd.com>]

On 04/09/2012 04:26 AM, Guenter Roeck wrote:
> On Sat, Apr 07, 2012 at 07:07:59PM -0400, Andre Przywara wrote:
>> Newer BKDG[1] versions recommend a different initialization value for
>> the running average range register in the northbridge. This improves
>> the power reading by avoiding counter saturations resulting in bogus
>> values for anything below about 80% of TDP power consumption.
>> Updated BIOSes will have this new value set up from the beginning,
>> but meanwhile we correct this value ourselves.
>> This needs to be done on all northbridges, even on those where the
>> driver itself does not register at.
>>
>> This fixes the driver on all current machines to provide proper
>> values for idle load.
>>
>> [1]
>> http://support.amd.com/us/Processor_TechDocs/42301_15h_Mod_00h-0Fh_BKDG.pdf
>> Chapter 3.8: D18F5xE0 Processor TDP Running Average (p. 452)
>>
>> Signed-off-by: Andre Przywara<andre.przywara@amd.com>
>> ---
>>   drivers/hwmon/fam15h_power.c |   40 ++++++++++++++++++++++++++++++++++++++++
>>   1 files changed, 40 insertions(+), 0 deletions(-)
>>
>> diff --git a/drivers/hwmon/fam15h_power.c b/drivers/hwmon/fam15h_power.c
>> index b7494af..2aa7d9d 100644
>> --- a/drivers/hwmon/fam15h_power.c
>> +++ b/drivers/hwmon/fam15h_power.c
>> @@ -122,6 +122,39 @@ static bool __devinit fam15h_power_is_internal_node0(struct pci_dev *f4)
>>   	return true;
>>   }
>>
>> +/*
>> + * Newer BKDG versions have an updated recommendation on how to properly
>> + * initialize the running average range (was: 0xE, now: 0x9). This avoids
>> + * counter saturations resulting in bogus power readings.
>> + * We correct this value ourselves to cope with older BIOSes.
>> + */
>> +static int tweak_runavg_range(struct pci_dev *pdev)
>> +{
>
> What is the point of returning a value if you don't use it ?

Probably an artifact of the idea of outputting a message when the 
tweaking was in effect. Fixed that. Patch follows.

Thanks for the review!

Andre.

>> +	u32 val;
>> +	struct pci_device_id affected_device = {
>> +		PCI_VDEVICE(AMD, PCI_DEVICE_ID_AMD_15H_NB_F4) };
>> +
>> +	/*
>> +	 * let this quirk apply only to the current version of the
>> +	 * northbridge, since future versions may change the behavior
>> +	 */
>> +	if (!pci_match_id(&affected_device, pdev))
>> +		return 0;
>> +
>> +	pci_bus_read_config_dword(pdev->bus,
>> +		PCI_DEVFN(PCI_SLOT(pdev->devfn), 5),
>> +		REG_TDP_RUNNING_AVERAGE,&val);
>> +	if ((val&  0xf) != 0xe)
>> +		return 0;
>> +
>> +	val&= ~0x0f;
>> +	val |= 9;
>> +	pci_bus_write_config_dword(pdev->bus,
>> +		PCI_DEVFN(PCI_SLOT(pdev->devfn), 5),
>> +		REG_TDP_RUNNING_AVERAGE, val);
>> +	return 1;
>> +}
>> +
>>   static void __devinit fam15h_power_init_data(struct pci_dev *f4,
>>   					     struct fam15h_power_data *data)
>>   {
>> @@ -155,6 +188,13 @@ static int __devinit fam15h_power_probe(struct pci_dev *pdev,
>>   	struct device *dev;
>>   	int err;
>>
>> +	/*
>> +	 * though we ignore every other northbridge, we still have to
>> +	 * do the tweaking on _each_ node in MCM processors as the counters
>> +	 * are working hand-in-hand
>> +	 */
>> +	tweak_runavg_range(pdev);
>> +
>>   	if (!fam15h_power_is_internal_node0(pdev)) {
>>   		err = -ENODEV;
>>   		goto exit;
>> --
>> 1.7.4.4
>>
>>
>


-- 
Andre Przywara
AMD-Operating System Research Center (OSRC), Dresden, Germany



_______________________________________________
lm-sensors mailing list
lm-sensors@lm-sensors.org
http://lists.lm-sensors.org/mailman/listinfo/lm-sensors