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.