-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
On 2013-11-11 15:26:58 -0500, Adrian Chadd wrote:
> On 11 November 2013 12:00, Jung-uk Kim <[email protected]> wrote:
>> -----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1
>>
>> On 2013-11-11 13:16:47 -0500, Nicholas McKenzie wrote:
>>> But wouldn't this just disable frequency scaling and the whole
>>> point of powerd?
>>
>> No. acpi_throttle (and p4tcc) controls T-state. "Frequency
>> scaling" should be done by changing P-state.
>
> Right.
>
> IIRC, T-state is just for emergency temperature throttling. It
> shouldn't be used outside of that.
>
>
>>>> There have been a number of reports of throttling causing
>>>> crashes. This setting does not prevent powerd from adjusting
>>>> your CPU's clock, it just disables some arcane feature which
>>>> pre-dates the modern power management methods.
>
> .. did anyone ever figure out why crashes would be caused by
> T-state adjustment?
My memory is vague but I think it was not able to reject a broken FADT
or _PTC table, or something like that.
>> I rewrote acpi_throttle.c at some point to fix the problem but
>> never committed it because nobody was really interested in
>> testing the patch. Also, it is really an arcane and archaic
>> feature:
>>
>> http://software.intel.com/en-us/blogs/2013/10/15/c-states-p-states-where-the-heck-are-those-t-states
>>
>>
>>
Now I think we should disable the feature by default because it is
>> causing too much hassle for us (attached). Any objection?
>
> No, I think we should correctly identify which particular features
> are required for which particular CPUs and enable/disable it based
> on that.
>
> So, if it's relevant for P4 era hardware, let's default to having
> it on for that class hardware.
>
> If it's not relevant for core and core 2 (and later) hardware,
> then let's default to leaving it off for that.
If you can maintain p4tcc for Intel processors, I am okay with that.
However, I still want to disable acpi_throttle by default. In fact,
I've never seen any non-Intel processor and/or motherboard with
correct BIOS to support T-state change.
> So, how about we come up with a table of CPUs and what particular
> power save technologies we should be using, then start
> implementing that?
Please see p4tcc.c. It already has a quirk table based on CPUID. You
just have to maintain it properly.
> I'm reading the SDM bits on the adaptive frequency stuff (turbo
> boost, etc) and I'll see about writing up some data gathering knobs
> for the kernel.
Cool.
BTW, please don't forget AMD users, i.e., check the BKDGs.
http://developer.amd.com/resources/documentation-articles/developer-guides-manuals/
> People still spin up FreeBSD on P4 (and earlier) class hardware.
> I'd rather we get it right versus just flipping crap on and off
> based on what 'current' hardware expects. I watched people do this
> with the RTC code. It's ridiculous.
Please see the attached patch, i.e., I reverted p4tcc-specific changes.
Jung-uk Kim
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.22 (FreeBSD)
iQEcBAEBAgAGBQJSgUyyAAoJEHyflib82/FGfLoH/2jejB55Eqtj134Z71bi75MA
YUCZ2z5r2PoDN5PUsJeHqyv5EyEWteYlAxLKr/mvV5rbaIk1Wlg0+6c4U7rH99Qj
+QpkS1GFL4XQFlKM8pFJ55VxQAYmUVwGCRGJxtxl0z6J6GvCIByKopqV3ywy04eG
LcxjML2Kka0FU5UmFKqjy/2j9jBBClEYfynSeVqpjc+REK970oZFMjblQqtLSNsf
GKhVwuFwaQYyZZylBTyEZh5fD7346jtA5G/mtSqjKJN2dY6nI5hIqqSWpXulLOEC
N16jqUWswO8hE8ZpgVeFSAmkznP4ITHsSjuxQgU4pyTdnF1DiOmizA7Snjekyvs=
=S1SR
-----END PGP SIGNATURE-----
Index: sys/dev/acpica/acpi_throttle.c
===================================================================
--- sys/dev/acpica/acpi_throttle.c (revision 258002)
+++ sys/dev/acpica/acpi_throttle.c (working copy)
@@ -167,7 +167,7 @@ static int
acpi_throttle_probe(device_t dev)
{
- if (resource_disabled("acpi_throttle", 0))
+ if (!resource_enabled("acpi_throttle", 0))
return (ENXIO);
/*
Index: sys/kern/subr_hints.c
===================================================================
--- sys/kern/subr_hints.c (revision 258002)
+++ sys/kern/subr_hints.c (working copy)
@@ -449,15 +449,29 @@ resource_find_dev(int *anchor, const char *name, i
}
/*
- * Check to see if a device is disabled via a disabled hint.
+ * Check to see if a device is disabled or enabled via a hint.
*/
-int
-resource_disabled(const char *name, int unit)
+static __inline int
+resource_find_hint(const char *name, int unit, const char *hint)
{
int error, value;
- error = resource_int_value(name, unit, "disabled", &value);
+ error = resource_int_value(name, unit, hint, &value);
if (error)
return (0);
return (value);
}
+
+int
+resource_disabled(const char *name, int unit)
+{
+
+ return (resource_find_hint(name, unit, "disabled"));
+}
+
+int
+resource_enabled(const char *name, int unit)
+{
+
+ return (resource_find_hint(name, unit, "enabled"));
+}
Index: sys/sys/bus.h
===================================================================
--- sys/sys/bus.h (revision 258002)
+++ sys/sys/bus.h (working copy)
@@ -503,6 +503,7 @@ int resource_long_value(const char *name, int unit
int resource_string_value(const char *name, int unit, const char *resname,
const char **result);
int resource_disabled(const char *name, int unit);
+int resource_enabled(const char *name, int unit);
int resource_find_match(int *anchor, const char **name, int *unit,
const char *resname, const char *value);
int resource_find_dev(int *anchor, const char *name, int *unit,
_______________________________________________
[email protected] mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-acpi
To unsubscribe, send any mail to "[email protected]"
