Re: [PATCH: 1/1] ACPI: make evaluation of thermal trip points beforetemperature or vice versa dependant on new "temp_b4_trip" module parameter tosupport older AMD x86_64s

[Jason Vas Dias]

Thanks Rusty - sorry I didn't see your email until now - revised patch
addressing your comments attached -
BTW,  sorry about the word wrap on the initial posting - should I
attach a '.patch' file or inline ?  Trying both .

The Revised Patch (against :
commit bd0a521e88aa7a06ae7aabaed7ae196ed4ad867a
Author: Linus Torvalds <torvalds@xxxxxxxxxxxxxxxxxxxx>
Date:   Sat Jul 7 17:23:56 2012 -0700

    Linux 3.5-rc6
) :
$ git diff bd0a521e88aa7a06ae7aabaed7ae196ed4ad867a >
/tmp/acpi_thermal_temp_b4_trip.patch
$ cat /tmp/acpi_thermal_temp_b4_trip.patch
diff --git a/drivers/acpi/thermal.c b/drivers/acpi/thermal.c
index 7dbebea..13d3b22 100644
--- a/drivers/acpi/thermal.c
+++ b/drivers/acpi/thermal.c
@@ -96,6 +96,10 @@ static int psv;
 module_param(psv, int, 0644);
 MODULE_PARM_DESC(psv, "Disable or override all passive trip
points.");

+static bool temp_b4_trip;
+module_param(temp_b4_trip, bool, 0644);
+MODULE_PARM_DESC(temp_b4_trip, "Get the temperature before
initializing trip points.");
+
 static int acpi_thermal_add(struct acpi_device *device);
 static int acpi_thermal_remove(struct acpi_device *device, int type);
 static int acpi_thermal_resume(struct acpi_device *device);
@@ -941,27 +945,41 @@ static int acpi_thermal_get_info(struct
acpi_thermal *tz)
        if (!tz)
                return -EINVAL;

-       /* Get trip points [_CRT, _PSV, etc.] (required) */
-       result = acpi_thermal_get_trip_points(tz);
-       if (result)
+       if( temp_b4_trip )
+       { /* some CPUs, eg AMD K8 need temperature before trip points
can be obtained */
+           /* Get temperature [_TMP] (required) */
+           result = acpi_thermal_get_temperature(tz);
+           if (result)
                return result;
-
-       /* Get temperature [_TMP] (required) */
-       result = acpi_thermal_get_temperature(tz);
-       if (result)
+
+           /* Get trip points [_CRT, _PSV, etc.] (required) */
+           result = acpi_thermal_get_trip_points(tz);
+           if (result)
                return result;
-
+       }else
+       { /* newer x86_64s need trip points set before temperature
obtained */
+           /* Get trip points [_CRT, _PSV, etc.] (required) */
+           result = acpi_thermal_get_trip_points(tz);
+           if (result)
+               return result;
+
+           /* Get temperature [_TMP] (required) */
+           result = acpi_thermal_get_temperature(tz);
+           if (result)
+               return result;
+       }
+
        /* Set the cooling mode [_SCP] to active cooling (default) */
        result = acpi_thermal_set_cooling_mode(tz,
ACPI_THERMAL_MODE_ACTIVE);
        if (!result)
                tz->flags.cooling_mode = 1;
-
+
        /* Get default polling frequency [_TZP] (optional) */
        if (tzp)
                tz->polling_frequency = tzp;
        else
                acpi_thermal_get_polling_frequency(tz);
-
+
        return 0;
 }

@@ -1110,6 +1128,14 @@ static int thermal_psv(const struct
dmi_system_id *d) {
        return 0;
 }

+static int thermal_temp_b4_trip(const struct dmi_system_id *d) {
+
+       printk(KERN_NOTICE "ACPI: %s detected: : "
+                           "getting temperature before trip point
initialisation\n", d->ident);
+       temp_b4_trip = TRUE;
+       return 0;
+}
+
 static struct dmi_system_id thermal_dmi_table[] __initdata = {
        /*
         * Award BIOS on this AOpen makes thermal control almost
worthless.
@@ -1147,6 +1173,14 @@ static struct dmi_system_id thermal_dmi_table[]
__initdata = {
                DMI_MATCH(DMI_BOARD_NAME, "7ZX"),
                },
        },
+       {
+        .callback = thermal_temp_b4_trip,
+        .ident = "HP 6715b laptop",
+        .matches = {
+                    DMI_MATCH(DMI_SYS_VENDOR, "Hewlett-Packard"),
+                    DMI_MATCH(DMI_PRODUCT_NAME, "HP Compaq 6715b"),
+               },
+       },
        {}
 };

Incidentally,  there are still plenty of cpufreq and temperature
related issues on this platform :
    with the "ondemand" or "performance"  governors,  placing a large
load on system
      ( eg. building gcc-4.7.1 ) makes the CPU switch into highest
frequency, but not switch
     down after the 65 degree trip point has been toggled once .
     And once the trip point has been reached once and the temperature
falls below 65, returning CPU freq to 2GHz,
     the reported temperature seems to be stuck at 62 degrees even
though the base of   the laptop nearly burns my hand .
     So I get emergency overheating reboots unless I manually run my
cpufreq & temperature monitoring scripts -
     which, if the CPU freq is 2Ghz,  now have to down the freqency to
800Khz for 2 seconds every 8 seconds
regardless of what temperature
     is reported .


On Mon, Jul 9, 2012 at 1:30 AM, Rusty Russell <rusty@xxxxxxxxxx> wrote:
> On Sun, 8 Jul 2012 19:50:54 +0100, Jason Vas Dias <jason.vas.dias@xxxxxxxxx> wrote:
>> This patch adds a new acpi.thermal.temp_b4_trip = 1 settting, which
>> causes the temperature
>> to be set before evaluation of thermal trip points (the old default) ;
>>  this mode should
>> be selected automatically by DMI match if the system identifies as "HP
>> Compaq 6715b" .
>>
>> Please consider applying a patch like that attached to fix the issue reported
>> in lkml thread "Re: PROBLEM: Performance drop" recently,  whereby
>> it was found that HP 6715b laptops ( which have 2.2Ghz dual-core  AMD
>> x86_64 k8 CPUs)
>> get stuck running the CPU at 800Khz and cannot switch frequency. I have verified
>> that this still the case with v3.4.4 tagged "stable" kernel.
>
>
>>
>> diff --git a/drivers/acpi/thermal.c b/drivers/acpi/thermal.c
>> index 7dbebea..de2b164 100644
>> --- a/drivers/acpi/thermal.c
>> +++ b/drivers/acpi/thermal.c
>> @@ -96,6 +96,10 @@ static int psv;
>>  module_param(psv, int, 0644);
>>  MODULE_PARM_DESC(psv, "Disable or override all passive trip points.");
>>
>> +static int temp_b4_trip;
>> +module_param(temp_b4_trip, int, 0);
>> +MODULE_PARM_DESC(temp_b4_trip, "Get the temperature before
>> initializing trip points.");
>> +
>
> Hi Jason,
>
>         Two points: this is a bool, why not make it one?  I know, the
> rest of the code is old-school, but this is a new parameter.  Also, why
> not 0644 so it can be read and set at runtime?
>
> Cheers,
> Rusty.

Attachment: acpi_thermal_temp_b4_trip.patch
Description: Binary data