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?
Thanks,
-Aubrey
