[PATCH 2/2] acpi: method: add _PSS test
Alex Hung
alex.hung at canonical.com
Thu Apr 19 06:36:49 UTC 2012
On 04/19/2012 02:33 PM, Keng-Yu Lin wrote:
> On Thu, Apr 19, 2012 at 11:45 AM, Alex Hung<alex.hung at canonical.com> wrote:
>> On 04/18/2012 07:41 PM, Colin King wrote:
>>>
>>> From: Colin Ian King<colin.king at canonical.com>
>>>
>>> Signed-off-by: Colin Ian King<colin.king at canonical.com>
>>> ---
>>> src/acpi/method/method.c | 106
>>> +++++++++++++++++++++++++++++++++++++++++++++-
>>> 1 file changed, 104 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/src/acpi/method/method.c b/src/acpi/method/method.c
>>> index 6797868..fae4b31 100644
>>> --- a/src/acpi/method/method.c
>>> +++ b/src/acpi/method/method.c
>>> @@ -102,7 +102,7 @@
>>> * _PSD 8.4.4.5 N
>>> * _PSL 11.4.8 N
>>> * _PSR 10.3.1 Y
>>> - * _PSS 8.4.4.2 N
>>> + * _PSS 8.4.4.2 Y
>>> * _PSV 11.4.9 Y
>>> * _PSW 7.2.12 Y
>>> * _PTC 8.4.3.1 N
>>> @@ -307,7 +307,7 @@ static int method_name_check(fwts_framework *fw)
>>> int failed = 0;
>>>
>>> if ((methods = fwts_method_get_names()) != NULL) {
>>> - fwts_log_info(fw, "Found %d Methods\n", methods->len);
>>> + fwts_log_info(fw, "Found %d Objects\n", methods->len);
>>>
>>> fwts_list_foreach(item, methods) {
>>> char *ptr;
>>> @@ -1846,6 +1846,105 @@ static int method_test_UID(fwts_framework *fw)
>>> }
>>>
>>>
>>> +/* Section 8.4 */
>>> +
>>> +static void method_test_PSS_return(fwts_framework *fw, char *name,
>>> ACPI_BUFFER *buf, ACPI_OBJECT *obj, void *private)
>>> +{
>>> + int i;
>>> + bool failed = false;
>>> + uint32_t max_freq = 0;
>>> + uint32_t prev_power = 0;
>>> +
>>> + if (method_check_type(fw, name, buf, ACPI_TYPE_PACKAGE) !=
>>> FWTS_OK)
>>> + return;
>>> +
>>> + /* Something is really wrong if we don't have any elements in _PSS
>>> */
>>> + if (obj->Package.Count< 1) {
>>> + fwts_failed(fw, LOG_LEVEL_MEDIUM,
>>> "Method_PSSElementCount",
>>> + "_PSS should return package of at least 1 element,
>>> "
>>> + "got %d elements instead.",
>>> + obj->Package.Count);
>>> + fwts_tag_failed(fw, FWTS_TAG_ACPI_METHOD_RETURN);
>>> + return;
>>> + }
>>> +
>>> + for (i=0; i< obj->Package.Count; i++) {
>>> + ACPI_OBJECT *pstate;
>>> +
>>> + if (obj->Package.Elements[i].Type != ACPI_TYPE_PACKAGE) {
>>> + fwts_failed(fw, LOG_LEVEL_MEDIUM,
>>> "Method_PSSElementType",
>>> + "_PSS package element %d was not a package.", i);
>>> + fwts_tag_failed(fw, FWTS_TAG_ACPI_METHOD_RETURN);
>>> + failed = true;
>>> + continue; /* Skip processing sub-package */
>>> + }
>>> +
>>> + pstate =&obj->Package.Elements[i];
>
> pstate points to the i-th sub-package.
>
>>>
>>> + if (pstate->Package.Count != 6) {
>>> + fwts_failed(fw, LOG_LEVEL_MEDIUM,
>>> "Method_PSSSubPackageElementCount",
>>> + "_PSS P-State sub-package %d was expected
>>> to have "
>>> + "6 elements, got %d elements instead.",
>>> + i, obj->Package.Count);
>>> + fwts_tag_failed(fw, FWTS_TAG_ACPI_METHOD_RETURN);
>>> + failed = true;
>>> + continue; /* Skip processing sub-package */
>>> + }
>>> +
>>> + /* Elements need to be all ACPI integer types */
>>> + if ((pstate->Package.Elements[0].Type !=
>>> ACPI_TYPE_INTEGER) ||
>>> + (pstate->Package.Elements[1].Type !=
>>> ACPI_TYPE_INTEGER) ||
>>> + (pstate->Package.Elements[2].Type !=
>>> ACPI_TYPE_INTEGER) ||
>>> + (pstate->Package.Elements[3].Type !=
>>> ACPI_TYPE_INTEGER) ||
>>> + (pstate->Package.Elements[4].Type !=
>>> ACPI_TYPE_INTEGER) ||
>>> + (pstate->Package.Elements[5].Type !=
>>> ACPI_TYPE_INTEGER)) {
>>> + fwts_failed(fw, LOG_LEVEL_MEDIUM,
>>> "Method_PSSSubPackageElementType",
>>> + "_PSS P-State sub-package %d was expected
>>> to have "
>>> + "6 Integer elements but didn't", i);
>>> + failed = true;
>>> + continue;
>>> + }
>>> +
>>> + fwts_log_info(fw, "PState %d: CPU %ld Mhz, %lu mW, latency
>>> %lu us, bus master latency %lu us.",
>>> + i,
>>> + (unsigned
>>> long)pstate->Package.Elements[0].Integer.Value,
>>> + (unsigned
>>> long)pstate->Package.Elements[1].Integer.Value,
>>> + (unsigned
>>> long)pstate->Package.Elements[2].Integer.Value,
>>> + (unsigned
>>> long)pstate->Package.Elements[3].Integer.Value);
>>> +
>>> + if (max_freq< pstate->Package.Elements[0].Integer.Value)
>>> + max_freq =
>>> pstate->Package.Elements[0].Integer.Value;
>>
>> max_freq is supposed to be in the first package, is this if-statement
>> necessary?
>>
>
> I think pstate is modified in each iteration as a pointer to a
> sub-package (see above).
> pstate->Package.Elements[0] is CoreFrequency
>
ah.. Keng-yu you are right. I did make mistake about the pstate package.
Thanks for pointing out.
>> Do you intend to check max_freq with all frequencies in following packages
>> in case the packages are not in descending order?
>>
>> i.e
>> if (max_freq< pstate->Package.Elements[i].Integer.Value)
>> max_freq = pstate->Package.Elements[i].Integer.Value;
>>
> suppose you mean obj->Package.Elements[i]->Package.Elements[0].Integer.Value.
>
>>
>>> +
>>> + /* Sanity check descending power dissipation levels */
>>> + if ((i> 0)&& (prev_power != 0)&&
>>> + (pstate->Package.Elements[1].Integer.Value>=
>>> prev_power)) {
>>> + fwts_failed(fw, LOG_LEVEL_MEDIUM,
>>> "Method_PSSSubPackagePowerNotDecending",
>>> + "_PSS P-State sub-package %d has a larger
>>> power dissipation "
>>> + "setting than the previous sub-package.",
>>> i);
>>> + fwts_advice(fw, "_PSS P-States must be ordered in
>>> decending order of "
>>> + "power dissipation, so that the zero'th
>>> entry has the highest "
>>> + "power dissipation level and the Nth has
>>> the lowest.");
>>> + failed = true;
>>> + }
>>> + prev_power = pstate->Package.Elements[1].Integer.Value;
>>> + }
>>> +
>>> + if (max_freq< 1000) {
>>
>> Will it be better if we use
>> "if (!failed&& (max_freq = Package.Elements[0].Integer.Value)< 1000)"
>> and "max_freq" needs not be set in the for-loop?
>>
>> If the sanity check fails in for-loop, may it not be necessary to check
>> max_freq?
>>
>>
>>> + fwts_warning(fw,
>>> + "Maximum CPU frequency is %dHz and this is low for
>>> "
>>> + "a modern processor. This may indicate the _PSS
>>> PStates "
>>> + "are incorrect\n", max_freq);
>>> + failed = true;
>>> + }
>>> +
>>> + if (!failed)
>>> + fwts_passed(fw, "_PSS correctly returned sane looking
>>> package.");
>>> +}
>>> +
>>> +static int method_test_PSS(fwts_framework *fw)
>>> +{
>>> + return method_evaluate_method(fw, METHOD_OPTIONAL, "_PSS", NULL,
>>> 0, method_test_PSS_return, NULL);
>>> +}
>>> +
>>> /* Tests */
>>>
>>> static fwts_framework_minor_test method_tests[] = {
>>> @@ -1973,6 +2072,9 @@ static fwts_framework_minor_test method_tests[] = {
>>> { method_test_ON, "Check _ON (Set resource on)." },
>>> { method_test_OFF, "Check _OFF (Set resource off)." },
>>>
>>> + /* Section 8.4 */
>>> + { method_test_PSS, "Check _PSS (Performance Supported States)." },
>>> +
>>> /* Appendix B, ACPI Extensions for Display Adapters */
>>>
>>> { method_test_DOS, "Check _DOS (Enable/Disable Output Switching)."
>>> },
>>
More information about the fwts-devel
mailing list