On 27/11/2023 17:36, Pierre-Louis Bossart wrote:
+static const struct snd_soc_acpi_adr_device cs35l56_0_adr[] = {
+ {
+ .adr = 0x00003301FA355601ull,
+ .num_endpoints = 1,
+ .endpoints = &spk_r_endpoint,
Assigning CS35L56 to "left" or "right" endpoints might be confusing.
All CS35L56 in a system receive both left and right channels and by
default they output a mono-mix of left+right.
The left/right of an amp is determined by the firmware file (.bin) that
is loaded and the current settings of the "Posture" ALSA control. So
this amp might be the left channel after a .bin is loaded.
That's a problem if the kernel does not know which amplifier is on which
side, no? How would one change the balance if this information is known
only within a binary/opaque firmware?
SDCA allows the posture (orientation) of amplifiers to be changed at
runtime. CS35L56 is designed as a SDCA device so it doesn't have any
hardwired position. SDCA doesn't define what the posture numbers mean,
they are an integer that is system-specific.
Because SDCA doesn't define the meaning of postures, and an SDCA device
should work with a generic SDCA driver (which obviously wouldn't have
hardcoded knowledge of the system) the speaker positions and postures
are coded into the firmware
It's difficult to say what is "default". For example, if you say that
the default for a tablet is left/right/top/bottom, assuming it is
used in portrait orientation, that would be wrong if the user always
uses it in landscape.
Matching by what amp is on what bus doesn't work well here because two
systems could have the same arrangement of CS35L56 on each bus but use
them for different purposes. So they could both match the "2 on bus 0, 2
on bus 1" table entry, but could be left/right/top/bottom on one device
and left woofer/right woofer/left tweeter/right tweeter on another
device.
I assume that if the system supports rotation there should be something
in the UCM or other userland that manages this. At least, it seems like
it's a UCM problem to decide which speakers are doing what audio.
If Linux-based distros don't have something like that - well, that just
means Linux is behind Windows.
It would be better to have generic names for the endpoint that don't
imply position, for example:
group1_spk1_endpoint
group1_spk2_endpoint
group1_spk3_endpoint
group1_spk4_endpoint.
The notion of endpoint is completely half-baked today and the settings
used have no bearing on the behavior and user-experience. I am inches
away from sending a patch that removes all of the endpoint definitions,
we can re-add them if/when we can get the information from platform
firmware.
+ .name_prefix = "cs35l56-8"
Can these prefixes be "AMPn" to match the CS35L41, CS35L51 and
CS35L56-hda driver? This prefix is used to find the matching firmware
files and our naming convention for these has been cs35lxx-xxxx-ampn
Is there anything that depends on the prefixes being "cs35l56-n" ?
IIRC this name_prefix is just used for the codec_conf and hence for
control names/UCM. At some point userspace/driver need to know if amp5
is left or right.
We can certainly align on conventions but the values set in this ACPI
match table will not be used for firmware download - different scope.
They are used for our firmware download. Each amp can have its own
unique firmware file. The ALSA prefix is used to identify which firmware
file to load to which amp.
+ },
+ {
+ .adr = 0x00003201FA355601ull,
+ .num_endpoints = 1,
+ .endpoints = &spk_3_endpoint,
+ .name_prefix = "cs35l56-7"
+ }
+};
+
+static const struct snd_soc_acpi_adr_device cs35l56_1_adr[] = {
+ {
+ .adr = 0x00013701FA355601ull,
+ .num_endpoints = 1,
+ .endpoints = &spk_l_endpoint,
+ .name_prefix = "cs35l56-1"
+ },
+ {
+ .adr = 0x00013601FA355601ull,
+ .num_endpoints = 1,
+ .endpoints = &spk_2_endpoint,
+ .name_prefix = "cs35l56-2"
+ }
+};
+
+static const struct snd_soc_acpi_link_adr tgl_cs42l43_cs35l56[] = {
+ {
+ .mask = BIT(3),
+ .num_adr = ARRAY_SIZE(cs42l43_3_adr),
+ .adr_d = cs42l43_3_adr,
+ },
+ {
+ .mask = BIT(0),
+ .num_adr = ARRAY_SIZE(cs35l56_0_adr),
+ .adr_d = cs35l56_0_adr,
+ },
+ {
+ .mask = BIT(1),
+ .num_adr = ARRAY_SIZE(cs35l56_1_adr),
+ .adr_d = cs35l56_1_adr,
+ },
+ {}
+};
+
