-----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 <j...@freebsd.org> 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,
_______________________________________________
freebsd-acpi@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-acpi
To unsubscribe, send any mail to "freebsd-acpi-unsubscr...@freebsd.org"

Reply via email to