On 01/02/23 06:21, Pierre-Louis Bossart wrote:
>>>>>>> we should create two separate ACPI companion devices for separate
>>>>>>> manager instance. Currently we have limitations with BIOS.
>>>>>>> we are going with single ACPI companion device.
>>>>>>> We will update the changes later.
>>>>>> Humm, this is tricky. The BIOS interface isn't something that can be
>>>>>> changed at will on the kernel side, you'd have to maintain two solutions
>>>>>> with a means to detect which one to use.
>>>>>>
>>>>>> Or is this is a temporary issue on development devices, then that part
>>>>>> should probably not be upstreamed.
>>>>> It's a temporary issue on development devices.
>>>>> We had discussion with Windows dev team and BIOS team.
>>>>> They have agreed to modify ACPI companion device logic.
>>>>> We will update the two companion devices logic for two manager
>>>>> instances in V2 version.
>>>> After experimenting, two ACPI companion devices approach,
>>>> we got an update from Windows team, there is a limitation
>>>> on windows stack. For current platform, we can't proceed
>>>> with two ACPI companion devices.
>>> so how would the two controllers be declared then in the DSDT used by
>>> Windows? There's a contradiction between having a single companion
>>> device and the ability to set the 'manager-number' to one.
>>>
>>> You probably want to give an example of what you have, otherwise we
>>> probably will talk past each other.
>>>> Even on Linux side, if we create two ACPI companion devices
>>>> followed by creating a single soundwire manager instance per
>>>> Soundwire controller, we have observed an issue in a scenario,
>>>> where similar codec parts(UID are also same) are connected on
>>>> both soundwire manager instances.
>>> We've been handling this case of two identical amplifiers on two
>>> different links for the last 3 years. I don't see how this could be a
>>> problem, the codecs are declared in the scope of the companion device
>>> and the _ADR defines in bits [51..48] which link the codec is connected to.
>>>
>> The problem is that there are two managers in the specified AMD design, and
>> the codecs are both on "Link 0" for each manager.
> You're confusing Controller and Manager.
>
> A Manager is the same as a 'Link', the two terms are interchangeable. It
> makes no sense to refer to a link number for a manager because there is
> no such concept.
>
> Only a Controller can have multiple links or managers. And each
> Controller needs to be declared as an ACPI device if you want to use the
> DisCo properties.
>
> The Managers/Links are not described as ACPI devices, that's a
> regrettable design decision made in MIPI circles many moons ago, that's
> why in the Intel code we have to manually create auxiliary devices based
> on the 'mipi-sdw-master-count' property.
>
>> So the _ADR really is identical for both.
Yes Controller has ACPI scope. Under controller based on
"mipi-sdw-manager-list" property manager instances will be created.
Manager and Link terms are interchangeable.
Below is the sample DSDT file if we go with two ACPI companion
devices.
Scope (\_SB.ACP)
{
Device (SWC0)
{
Name (_ADR, 0x05) // _ADR: Address
Name(_DSD, Package() {
ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
Package () {
Package (2) {"mipi-sdw-sw-interface-revision", 0x00010000},
Package (2) {"mipi-sdw-manager-list", 1}, // v 1.0
},
ToUUID("dbb8e3e6-5886-4ba6-8795-1319f52a966b"), // Hierarchical Extension
Package () {
Package (2) {"mipi-sdw-link-0-subproperties", "SWM0"},
}
}) // End _DSD
Name(SWM0, Package() {
ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
Package () {
Package (2) {"mipi-sdw-sw-interface-revision", 0x00010000},
// ... place holder for SWM0 additional properties
}
}) // End SWM0.SWM
Device (SLV0) { // SoundWire Slave 0
Name(_ADR, 0x000032025D131601)
} // END SLV0
} // END SWC0
Device (SWC1)
{
Name (_ADR, 0x09) // _ADR: Address
Name(_DSD, Package() {
ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
Package () {
Package (2) {"mipi-sdw-sw-interface-revision", 0x00010000},
Package (2) {"mipi-sdw-manager-list", 1},
},
ToUUID("dbb8e3e6-5886-4ba6-8795-1319f52a966b"),
Package () {
Package (2) {"mipi-sdw-link-0-subproperties", "SWM0"},
}
}) // End _DSD
Name(SWM0, Package() {
ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
Package () {
Package (2) {"mipi-sdw-sw-interface-revision", 0x00010000},
// ... place holder for SWM0 additional properties
}
}) // End SWM0.SWM
Device (SLV0) { // SoundWire Slave 0
Name(_ADR, 0x000032025D131601)
} // END SLV0
} // END SWC1
}
}
In above case, two manager instances will be created.
When manager under SWC1 scope tries to add peripheral
device, In sdw_slave_add() API its failing because peripheral
device descriptor uses link id followed by 48bit encoded address.
In above scenarios, both the manager's link id is zero only.
> That cannot possible work, even for Windows. You need to have a
> controller scope, and the _ADR can then be identical for different
> peripherals as long as this ADR is local to a controller scope.
>
