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".
BTW, On this particular system, there really wasn't a _PSS.
Thanks,
Mark
