Li, Aubrey wrote:
> On Wednesday, September 12, 2007 5:48 PM, Mark.Haywood at Sun.COM wrote:
>
>
>> Hi Aubrey,
>>
>> See comments at bottom ...
>>
>> Li, Aubrey wrote:
>>
>>> On Tuesday, September 11, 2007 12:39 AM, Mark.Haywood at Sun.COM wrote:
>>>
>>>
>>>> Li, Aubrey wrote:
>>>>
>>>>> On Friday, September 07, 2007 8:54 PM, Mark.Haywood at Sun.COM wrote:
>>>>>
>>>>>
>>>>>
>>>>>> Hi Aubrey,
>>>>>>
>>>>>> See comments inline ...
>>>>>>
>>>>>> Li, Aubrey wrote:
>>>>>>
>> >>>>> ----------
>>
>>>>>>> In addition, acpica building cpu map function assumes there is
>>>>>>> only one processor type object under _PR_ object. That causes the
>>>>>>> processor handler is always mapped to the last Processor Type
>>>>>>> object. This is not correct on case 2) machine, it will cause to
>>>>>>> give a wrong handler to evaluate _PDC object. That's another
>>>>>>> reason that _PSS not found. Here is the patch(it's attached as
>>>>>>> well in case of whitespace coding style issue),
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>> I'll have to look at this one some more. I don't understand you
>>>>>> explanation and I don't understand the proposed fix. Are you sure
>>>>>> that you didn't mean
>>>>>>
>>>>>> if (cpu_map[cpu_id]->obj != NULL)
>>>>>> cpu_map[cpu_id]->obj = obj;
>>>>>>
>>>>>>
>>>>>>
>>>>> Two kinds of processor configuration machines in my hand.
>>>>>
>>>>> 1) _PR_ has only one child CPU0, which is ok.
>>>>> CPU0 is _PDC's parent, and it will be mapped to the cpu acpi
>>>>> handler. So when passed the handler to evaluate _PDC, it will be
>>>>> sucessful.
>>>>>
>>>>> Level = 1, parent = \___ child = _PR_
>>>>> Level = 2, parent = _PR_ child = CPU0
>>>>> Level = 3, parent = CPU0 child = _PPC
>>>>> Level = 3, parent = CPU0 child = _PCT
>>>>> Level = 3, parent = CPU0 child = _PSS
>>>>> Level = 3, parent = CPU0 child = SPSS
>>>>> Level = 3, parent = CPU0 child = NPSS
>>>>> Level = 3, parent = CPU0 child = _PDC
>>>>>
>>>>> 2) _PR_ has two child: P001 and CPU1. and the last object CPU1 is
>>>>> always mapped to be the cpu acpi handler.
>>>>> However, P001 which is the parent of _PDC, should be the cpu
>>>>> acpi handler. We should always use the first one. So when
>>>>> **cpu_map[cpu_id]->obj == NULL **, we can assign **obj** to it.
>>>>> Otherwise the first matched handler will be overwrite by the last
>>>>> one. And the wrong parent parameter will be passed to evaluate
>>>>> _PDC. So we run into failure.
>>>>>
>>>>> Level = 1, parent = \___ child = _PR_
>>>>> Level = 2, parent = _PR_ child = P001
>>>>> Level = 3, parent = P001 child = HI0_
>>>>> Level = 3, parent = P001 child = _PDC
>>>>> Level = 3, parent = P001 child = _OSC
>>>>> Level = 3, parent = P001 child = _CST
>>>>> Level = 3, parent = P001 child = _PPC
>>>>> Level = 3, parent = P001 child = _PCT
>>>>> Level = 3, parent = P001 child = _PSD
>>>>> Level = 3, parent = P001 child = _PSS
>>>>> Level = 3, parent = P001 child = SPSS
>>>>> Level = 3, parent = P001 child = NPSS
>>>>> Level = 2, parent = _PR_ child = CPU1
>>>>> ------------------------------------------------------------------
>>>>>
>>>>> Did you see the same problem on Toshiba Tecra M5?
>>>>>
>>>>>
>>>> This is interesting and is not something that I've seen
>>>> before. But then
>>>> again, given the previously existing bug with _PDC
>>>> negotiation, we might
>>>> not have noticed this problem. As far as I know, folks with Toshiba
>>>> Tecra M5s are not seeing this problem.
>>>>
>>>>
>>> This problem is not depend on the previously existing bug.
>>> The current driver doesn't log _PDC failure, so we didn't notice
>>> this problem. I can get _PSS object on that machine by iasl and
>>> acpidump, but the driver still warns me it's not found.
>>> So I dig into it and found _PDC evaluating failure.
>>> So I suggest we put _PDC evaluating failure into log, on the boot
>>> screen is not necessary.
>>>
>>>
>>>> What machine had a BIOS that gave you the (_PR) Processor Tree
>>>> object in case 2 above?
>>>>
>>> I'll check if the platform is release, ;-)
>>>
>>>
>>>> That seems wrong to me. Or at least it seems to
>>>> me that we
>>>> (I) need to have a better understanding of it. I'd not have thought
>>>> that it was possible for the ACPI Processor NameSpace to define two
>>>> separate Processor objects for the same processor. For example, is
>>>> it possible to have the following tree structure?
>>>>
>>>> Level = 1, parent = \___ child = _PR_
>>>> Level = 2, parent = _PR_ child = CPU1
>>>> Level = 2, parent = _PR_ child = P001
>>>> Level = 3, parent = P001 child = HI0_
>>>> Level = 3, parent = P001 child = _PDC
>>>> Level = 3, parent = P001 child = _OSC
>>>> Level = 3, parent = P001 child = _CST
>>>> Level = 3, parent = P001 child = _PPC
>>>> Level = 3, parent = P001 child = _PCT
>>>> Level = 3, parent = P001 child = _PSD
>>>> Level = 3, parent = P001 child = _PSS
>>>> Level = 3, parent = P001 child = SPSS
>>>> Level = 3, parent = P001 child = NPSS
>>>>
>>>>
>>>> If so, then the proposed fix above would not work.
>>>>
>>>>
>>> Your change of that table is so good that we don't have to change
>>> the current code behavior. But that's the table the BIOS give out.
>>> And the driver have to be based on it. I didn't found the
>>> specification for it so far.
>>> At least, the current method of building cpu acpi map doesn't cover
>>> all kinds of machine, which need to be fixed.
>>> I'll continue to investigate and let you know when I get something.
>>>
>> I ran into a system today with the following iasl dump:
>>
>> Scope (_PR)
>> {
>> Processor (P001, 0x01, 0x00000810, 0x06) {} Alias
>> (P001, CPU1) Processor (P002, 0x02, 0x00000000, 0x00) {}
>> Alias (P002, CPU2) Processor (P003, 0x03, 0x00000000, 0x00)
>> {} Alias (P003, CPU3) Processor (P004, 0x04,
>> 0x00000000, 0x00) {} Alias (P004, CPU4) }
>>
>> I've never see an "Alias" defined before. I'm wondering if
>> that is what
>> you are running into. If so, then I think the fix might be to ignore
>> aliases when doing the mapping. Can you check to see if that's what
>> you're running into? If so, then we'll have to figure out how to
>> identify an "Alias".
>>
> Yes, I saw an "Alias" on my system. From the objects "P001" and "CPU1",
> they have the same type and flags.
> We have to find another way to identify this "Alias".
> Can we simply not add these "Alias" object into the namespace when we
> load the whole table?
>
>
>> BTW, On this particular system, there really wasn't a _PSS.
>>
>>
>
> _PSS may be in another table which needs to be loaded by evaluating
> _PDC.
> Can you give me the ssdt table on your system to check?
>
No, I already considered this. There isn't a _PDC method defined either.
Additionally, when I read the IA32_MISC_ENABLE_MSR (MSR 0x1A0), it does
not have IA32_MISC_ENABLE_EST (bit 16) enabled. I asked the
owner to make sure that SpeedStep was enabled in the BIOS setup and he
said he didn't see any way to enable/disable it. I think the BIOS for
that system is not yet cooked.
Mark
