soundwire: sdca: parse function type and controls #4930
Closed
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
plbossart
force-pushed
the
sdw/parse-functions
branch
from
8141c28 to
1191999
Compare
Member
Author
|
v2: all the SDCA information is moved to a sound/soc/sdca library, and the information is parsed before the driver probe (if present). |
Collaborator
|
Thanks @plbossart Looks like a good start for SDCA driver. |
Member
Author
|
@shumingfan would you happen to know what this Function 4 might be. I wonder if the function topology uses an older version of the SDCA/DisCo spec? |
plbossart
force-pushed
the
sdw/parse-functions
branch
2 times, most recently
from
8e08c60 to
999b877
Compare
Member
Author
|
logs with the last version Now we need to figure out something useful with all this. |
I will try to ask the hardware team about this. If there are any results, I will give feedback. |
plbossart
force-pushed
the
sdw/parse-functions
branch
from
999b877 to
d43f80c
Compare
|
Thanks @plbossart for pushing the SDCA driver patches. We will also try to validate sdca tables parsing on AMD platform. |
The hardware engineer thinks reporting the function type 4 is a mistake. |
Member
Author
Right, that's what I thought @shumingfan. In this device there's no RT1713 (local mic disabled), so I can't check further. |
sound/soc/sdca/sdca_functions.c
Outdated
There was a problem hiding this comment.
Should probably acpi_get_local_address
Member
Author
There was a problem hiding this comment.
Erm, no. that function is wrong. _ADR is a 64-bit value, but the code assumes it's 32-bits.
I vaguely remember we had to fix some of the ACPI subsystem to make _ADR a true 64 bit value, this function has not been updated apparently.
Actually now I do remember, and it's even in git:
commit ca6f998cf9a259fe6019d44768064e0cfe8a925b
Author: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Date: Wed May 1 07:53:22 2019 -0500
ACPI: bus: change _ADR representation to 64 bits
So this code would need to be updated to u64.
int acpi_get_local_address(acpi_handle handle, u32 *addr)
{
unsigned long long adr;
acpi_status status;
status = acpi_evaluate_integer(handle, METHOD_NAME__ADR, NULL, &adr);
if (ACPI_FAILURE(status))
return -ENODATA;
*addr = (u32)adr;
return 0;
}Gah.
sound/soc/sdca/sdca_functions.c
Outdated
There was a problem hiding this comment.
Well you say there is no hardware that uses this feature, but cs42l43 and likely its successors make heavy use of it if they are used in SDCA mode. Not that support needs to be added right now, but just making you aware.
sound/soc/sdca/sdca_functions.c
Outdated
There was a problem hiding this comment.
Probably slightly nice to use BITS_PER_TYPE(input_pin_list)
sound/soc/sdca/sdca_functions.c
Outdated
There was a problem hiding this comment.
I would be tempted to add some sort of define for this max length, lot of max 40 byte strings used in the code.
plbossart
force-pushed
the
sdw/parse-functions
branch
from
20eb0aa to
3795d70
Compare
Add new module for SDCA (SoundWire Device Class for Audio) support. For now just add a mechanism to allocate a context. Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
SDCA devices are based on independent 'Functions', which are exposed in platform firmware using the DisCo 2.1 specification. This patch adds a mechanism to parse the functions and extract basic information (adr and function type). Example log on a MeteorLake device (which already shows problems with ACPI information...) [ 13.069728] soundwire sdw:0:0:025d:0713:01: found invalid SDCA function type 4 ADR 1, skipped [ 13.069743] soundwire sdw:0:0:025d:0713:01: found SDCA function type 6 ADR 3 [ 13.070166] soundwire sdw:0:1:025d:1316:01: found SDCA function type 1 ADR 4 [ 13.070750] soundwire sdw:0:2:025d:1316:01: found SDCA function type 1 ADR 4 Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Before the driver probe, we can parse the SDCA information in platform firmware to discover function-related information. The SDCA information is abstracted away since: a) not all implementations require SDCA support. b) SDCA support will require tight integration with ASoC, specifically the DAPM/DAI parts. The SDCA integration follows the USB model, where the Audio Class is handled by the sound/ subsystem. The ASoC codec drivers are the intended users of the SDCA information reported by ACPI. Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
For now only extract the ID, label and entity_type. Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
In contrast to the USB spec, the SDCA only allows for well-defined connections between entities. The intent was to limit errors/support with a limited set of configurations. In other words, connections that are not described in the SDCA spec are not permitted. In order to allow for product variations, optional paths can be supported. These options are described with a topology_features mask. Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
…if non-zero In their infinite wisdom, the MIPI SDCA subgroup allowed the semantics of the "Command Ignored" to be overloaded. In the MIPI SoundWire 1.x spec, the 'Command Ignored' can only happen in three cases: a) as a result of an access to an invalid register b) as a result of an access to a device that is not attached to the bus c) when enumeration is complete. For SDCA devices, the 'Command Ignored' might be a valid response if the device is currently 'busy' dealing with the previous transaction. In that case, software has to poll the 'Function_Busy' bit and re-issue the read/write command when that bit is no longer asserted. This behavior is currently NOT supported in the regmap and default read/write functions, and there is currently no hardware which implements this feature. For now the only thing we can do is to log an error and ask end-users to report this problematic scenario. Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
The DisCo specification describes the connections between entities with a 'pin-input-list' mask, and subproperties to point to the connected entity. Unfortunately there's no direct access to the connected entity id, so we have to search the entity list using the 'label' string, which is the only piece of information useable for a match. Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
The MIPI DisCo 2.1 spec defines an "initialization table", which
contains a list of 4-byte addresses and 1 byte value. The registers
need to be updated during the initial boot and if context is lost.
Note that the data in the table is represented as an ACPI "buffer",
but the buffer is accessible with a DSD property:
ToUUID ("edb12dd0-363d-4085-a3d2-49522ca160c4")
Package (0x01)
{
Package (0x02)
{
"mipi-sdca-function-initialization-table",
"BUF0"
}
}
Name (BUF0, Buffer (0x01BD)
{
0x20, 0xC7, 0x00, 0x00, 0x17, 0x21, 0xC7, 0x00,
0x00, 0x00, 0x22, 0xC7, 0x00, 0x00, 0x3E, 0x23,
0xC7, 0x00, 0x00, 0x06, 0x24, 0xC7, 0x00, 0x00,
...
}
The connection between _DSD and ACPI is defined in the 'Data Buffer'
extension [1] already supported in the Linux kernel.
[1] https://github.com/UEFI/DSD-Guide/blob/main/src/dsd-guide.adoc#buffer-data-extension-uuid
Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
plbossart
force-pushed
the
sdw/parse-functions
branch
from
3795d70 to
d9fc544
Compare
Member
Author
|
closing for now, will re-add the functionality when basic parsing is implemented with PR #4995 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
SDCA devices are based on independent 'Functions', which are exposed in platform firmware using the DisCo 2.1 specification.
This patch adds a mechanism to parse the functions and extract basic information (adr and function type).
Example log on a MeteorLake device (which already shows problems with ACPI information...)
[ 13.069728] soundwire sdw:0:0:025d:0713:01: found invalid SDCA function type 4 ADR 1, skipped
[ 13.069743] soundwire sdw:0:0:025d:0713:01: found SDCA function type 6 ADR 3
[ 13.070166] soundwire sdw:0:1:025d:1316:01: found SDCA function type 1 ADR 4
[ 13.070750] soundwire sdw:0:2:025d:1316:01: found SDCA function type 1 ADR 4