Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add UCMs for Google Asus Nexus 7 and Acer Iconia A500 #92

Closed
wants to merge 7 commits into from

Conversation

digetx
Copy link
Contributor

@digetx digetx commented May 20, 2021

@perexg As was discussed previouly at https://www.spinics.net/lists/alsa-devel/msg126382.html I'm opening PR with the UCMs for Nexus 7 and A500. Both these devices are supported by the mainline Linux kernel.

You're right that Tegra ASoC driver doesn't set card's name properly. In particular I found that we need to set the card's driver_name to tegra, then UCM get a proper lookup path.

I rebased UCMs on top of the recent alsa-ucm-conf master and switched Nexus 7 UCM to use generic RT5640 UCM. Everything works great once the card name of the kernel driver is corrected. I'll send the kernel patches ASAP for review.

Nexus 7 alsa-info: https://alsa-project.org/db/?f=0a29f1dab39ec5b33e01215dee42dbfd1dbbf08e
A500 alsa-info: https://alsa-project.org/db/?f=bde4f7938a39b1d26904b414969fc811e726242f

The alsa-info is taken using the updated kernel ASoC driver.

@digetx digetx changed the title Add UCMs for Google Asus Nexus 7 and Acer Iconial A500 Add UCMs for Google Asus Nexus 7 and Acer Iconia A500 May 20, 2021
@@ -10,7 +10,15 @@ SectionDevice."Mic" {
cset "name='Mono ADC MIXR ADC2 Switch' on"
cset "name='Stereo ADC MIXL ADC2 Switch' on"
cset "name='Stereo ADC MIXR ADC2 Switch' on"
cset "name='Internal Mic Switch' on"
If.Controls {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

At first of all, thank you for your work. It seems like a right track to make things more maintainable. The BayTrail (Intel) specific controls should be moved to the upper level configs. @plbossart , FYI,

Also, If blocks cannot be in the sequences. You can define [] arrays multiple times and the sequences will be added to the end of array. So this is fine:

EnableSequence [
   sequence3
]
If.x {
   True.EnableSequence [
      sequence1
   ]
}
If.y {
  False.EnableSequence [
     sequence2
  ]
}

Note that Include / If blocks are evaluated as first (see the sequence numbers). That's the reason why we have some dummy (empty) string If blocks in the configs.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I pushed the updated version that implements yours suggestion, it works using the latest alsa-lib master. Please take a look.

@digetx
Copy link
Contributor Author

digetx commented May 20, 2021

Do you also mean that it's also possible to move the device sequences to ucm2/bytcr-rt5640/HiFi.conf?

SectionDevice."Headset" {
	If.HeadsetControls {
		Condition {
			Type ControlExists
			Control "name='Headset Mic Switch'"
		}
		True {
			cset "name='Headset Mic Switch' off"
		}
	}

	EnableSequence [
		If.Controls {
			Condition {
				Type ControlExists
				Control "name='Headset Mic Switch'"
			}
			True {
				cset "name='Headset Mic Switch' on"
			}
		}
	]

	DisableSequence [
		If.Controls {
			Condition {
				Type ControlExists
				Control "name='Headset Mic Switch'"
			}
			True {
				cset "name='Headset Mic Switch' off"
			}
		}
	]
}

@perexg
Copy link
Member

perexg commented May 21, 2021

Do you also mean that it's also possible to move the device sequences to ucm2/bytcr-rt5640/HiFi.conf?

Yes, but the config should be like (ucm2/bytcr-rt5640/HiFi-Components.conf):

If.hsmic {
	Condition { Type String Empty "" }
	True {
		Include.hsmic.File "/codecs/rt5640/HeadsetMic.conf"
		If.switch {
			Condition {
				Type ControlExists
				Control "name='Headset Mic Switch'"
			}
			True {
				SectionDevice."Headset" {
					EnableSequence [
						cset "name='Headset Mic Switch' on"
					]
					DisableSequence [
						cset "name='Headset Mic Switch' off"
					]
				}
			}
		}
	}
}

In this case Include is evaluated at first, so the cset will be appended to the corresponding Enable/Disable sequences.

But in this case, we know that Headset Mic Switch exists for this platform (bytcr-rt5640), so you can remove the control If completely and keep only Enable/Disable sequences.

@digetx
Copy link
Contributor Author

digetx commented May 23, 2021

@perexg I'm getting I: [pulseaudio] (alsa-lib)main.c: error: failed to import hw:0 use case configuration -17 with:

If.spk {
	Condition {
		Type String
		Haystack "${CardComponents}"
		Needle "cfg-spk:2"
	}
	True {
		Include.spk.File "/codecs/rt5640/Speaker.conf"
		Define.HaveSpeaker "yes"

+		SectionDevice."Speaker" {
+			EnableSequence [
+				cset "name='Speaker Switch' on"
+			]
+
+			DisableSequence [
+				cset "name='Speaker Switch' off"
+			]
+		}
	}
}

alsa-lib doesn't like SectionDevice specified within conditions. What should we do about it?

@digetx
Copy link
Contributor Author

digetx commented May 23, 2021

Also, second SectionDevice definition overrides the previous definition, instead of appending it. I thought that this may work:

If.spk {
	Condition {
		Type String
		Haystack "${CardComponents}"
		Needle "cfg-spk:2"
	}
	True {
		Include.spk.File "/codecs/rt5640/Speaker.conf"
+		Include.spk.File "/bytcr-rt5640/Speaker.conf"
		Define.HaveSpeaker "yes"
	}
}
...

ucm2/bytcr-rt5640/Speaker.conf:
+SectionDevice."Speaker" {
+	EnableSequence [
+		cset "name='Speaker Switch' on"
+	]
+
+	DisableSequence [
+		cset "name='Speaker Switch' off"
+	]
+}

but it's not working as expected.

Apparently syntax upgrade is required here. We could temporally use the version that I proposed with the conditions within the codec and move them to /bytcr-rt5640 once syntax will allow to do that.

@perexg
Copy link
Member

perexg commented May 24, 2021

Oops, I fixed this in 6b728405697e38339773a6901ca8f557c89f0088 . The merging was previously handled in only one level, thus:

SectionDevice."Speaker" {
	Comment "Speakers"
	EnableSequence [
		shell "echo 1st"
	]
	Value {
		PlaybackChannels 2
		PlaybackPriority 200
		PlaybackPCM "hw:0"
	}
}

If.0 {
	# Does not work before alsa-lib's commit 6b728405697e38339773a6901ca8f557c89f0088
	Condition { Type String Empty "" }
	True {
		SectionDevice."Speaker" {
			EnableSequence [
				shell "echo 2nd"
			]
		}
	}
}

SectionDevice."Speaker" {
	EnableSequence [
		shell "echo 3rd"
	]
}

SectionDevice."Speaker" {
	If.1 {
		Condition { Type String Empty "" }
		True {
			EnableSequence [
				shell "echo 4rd"
			]
		}
	}
}
$ alsaucm -c test set _verb HiFi set _enadev Speaker
1st
3rd
2nd
4rd

Note the order.

If If.0 block is commented (or removed), it works with the previous master:

$ alsaucm -c test set _verb HiFi set _enadev Speaker
1st
3rd
4rd

The UCM of RT5640 codec toggles switches that exists only in a case of
the Intel BayTrail ASoC machine driver, RT5640 codec driver doesn't have
them. Move these switches to the BayTrail UCM in order to make generic UCM
rules reusable by other SoCs.

Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
Previously RT5640 was turning off switches that are specific to the Intel
BayTrail SoC. In a case of other SoCs we need to turn off the codec switches.
This fixes audio playing from both speaker and headphones simultaneously on
Nexus 7, until headphones are re-inserted.

Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
The playback volumes shall be initialized only during of the first
enable sequence, not during the initialization of speaker/headphones
devices. This fixes sound level restoring by pulseaudio when headphones
are inserted/ejected. Previously sound level was always reset to the
default level on insert/ejection, which is the incorrect behaviour.
The correct behaviour is to restore volume the previous level, i.e.
if sound level was 3% before headphones were ejected, then level should
be restored to 3% when headphones are inserted back.

Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
Nexus 7 has a 3-pin 3.5mm headphones jack, it doesn't support headset
microphone, and thus, it doesn't have headset microphone jack. Make
headset device optional.

Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
The old UCM names were matched only by the name of the kernel driver module.
This means that built-in kernel drivers never matched.

Tegra ASoC machine kernel drivers never specified the card's driver_name
and long_name properly, which is required in order to have a proper
'ucm2/conf.d/tegra/' path being used for the matching of UCMs. This will
be fixed in the kernel ASAP, the change will be backported to stable kernels.

This patch adds symlinks for the PAZ00 netbook and NyanBig Chromebook
to the new 'ucm2/conf.d/tegra/' directory that will match devices properly
in conjunction with the updated Linux kernel regardless of whether sound
driver is a loadable module or built-in.

Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
@digetx
Copy link
Contributor Author

digetx commented May 24, 2021

@perexg Thank you very much, it works now using the latest alsa-lib master. Will you backport that change to a stable alsa-lib version or bump the syntax version?

Acer Iconia Tab A500 is a tablet device which is powered by NVIDIA Tegra20
SoC, it has WM8903 audio CODEC. The device has built-in 2-channel speaker,
built-in mono microphone and 4-pin 3.5mm jack for headphones and headset.

Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
ASUS Google Nexus 7 tablet device is powered by NVIDIA Tegra30 SoC, it
has Realtek ALC5642 audio CODEC, which is compatible with the RT5640 CODEC.
Nexus 7 has a 2-channel built-in speaker, built-in 2-channel microphone,
a 3-pin 3.5mm jack for headphones, and a custom external audio output for
docking-station. The docking-station configuration isn't supported because
it's unsupported by the upstream Linux kernel yet.

Signed-off-by: Dmitry Osipenko <digetx@gmail.com>
perexg added a commit that referenced this pull request May 27, 2021
- merge HaveAif1 and HaveAif2 to HaveAif with values 1 and 2
- HaveSpeaker identifies stereo and mono (values 2 and 1)
- HaveInternalMic identifies internal mic input (dmic, in1, in3)
- rewrite (unify) bytcr-rt5640 components detection

BugLink: #92
Signed-off-by: Jaroslav Kysela <perex@perex.cz>
@perexg
Copy link
Member

perexg commented May 27, 2021

I've merged your changes with the 4884ef8 fix / cleanups for bytcr-rt5640 . The internal variables (define) sections must be evaluated before the device includes. Thank you for your contribution.

@perexg perexg closed this May 27, 2021
@digetx
Copy link
Contributor Author

digetx commented May 27, 2021

@perexg Thank you, everything working great using the latest alsa-ucm-conf + alsa-lib master.

lgirdwood pushed a commit to thesofproject/alsa-lib that referenced this pull request May 31, 2021
It's useful to do the full tree merge (append).

Fixes: alsa-project/alsa-ucm-conf#92
Signed-off-by: Jaroslav Kysela <perex@perex.cz>
fengguang pushed a commit to 0day-ci/linux that referenced this pull request Jun 16, 2021
…ipenko <digetx@gmail.com>:

This series squashes all the ASoC machine drivers into a single one,
this change was suggested by Jon Hunter. It also sets driver_name and
components string of each card, allowing userspace alsa-lib to find
UCMs at predictable path.

Changelog:

v6: - Fixed missed configuration of AC97 clock rate for the WM9712 codec
      in the unified driver.

    - Added new patch that removes now obsolete "utils" helpers and moves
      code into the unified driver.

        ASoC: tegra: Squash utils into common machine driver

v5: - The v4 removed the customization of components string for Nexus 7,
      but I missed to remove the "components" hook which is unused now,
      it's removed in v5 for consistency.

    - Slightly improved naming of the common 12MHz MCLK rate function
      to make it more consistent with the rest of the driver functions.

v4: - Moved out mclk_rate callback that is currently used only by WM8903
      machine driver from the common driver. This was suggested by Jon Hunter.

    - Dropped patch which was setting custom components string for Nexus 7.
      Jaroslav Kysela wants it to be specified in a device-tree, but the
      components string doesn't have a firm specification for today. It's
      better to drop this change for now since it's optional anyways.

    - Fixed compilation error that was reported by kernel robot for v3.

    - Jaroslav Kysela merged alsa-ucm-conf PR [1] which added UCMs for
      Nexus 7 and Acer A500. The UCMs are fully working using a combination
      of updated kernel + alsa-ucm-conf master + alsa-lib master, meaning
      that they will work with the next releases of kernel and ALSA userspace
      upstream packages.

    - Added ack from Jaroslav Kysela to the "Specify components string for
      each card" patch that he gave to v3.

v3: - Added components string as was suggested by Jaroslav Kysela to v2.

    - Renamed MCLK rate function that is used by max98090 and other codecs
      to make it look more generic. Added option for specifying CLK ID per
      device. This all was suggested by Jon Hunter to v2.

v2: - Dropped use of of_device_compatible_match(), like it was suggested
      by Rob Herring in a review comment to v1.

    - Added patch that sets card's driver_name of as Tegra ASoC drivers.
      In a comment to v1 Jaroslav Kysela suggested that the Tegra drivers
      don't set the card name properly and he was right.

      I opened pull request with the new Tegra UCMs and updated lookup paths
      for older UCMs [1].

      [1] alsa-project/alsa-ucm-conf#92

Dmitry Osipenko (4):
  ASoC: tegra: Set driver_name=tegra for all machine drivers
  ASoC: tegra: Unify ASoC machine drivers
  ASoC: tegra: Specify components string for each card
  ASoC: tegra: Squash utils into common machine driver

 sound/soc/tegra/Kconfig              |  12 +
 sound/soc/tegra/Makefile             |  19 +-
 sound/soc/tegra/tegra_alc5632.c      | 259 --------
 sound/soc/tegra/tegra_asoc_machine.c | 854 +++++++++++++++++++++++++++
 sound/soc/tegra/tegra_asoc_machine.h |  49 ++
 sound/soc/tegra/tegra_asoc_utils.c   | 225 -------
 sound/soc/tegra/tegra_asoc_utils.h   |  38 --
 sound/soc/tegra/tegra_max98090.c     | 276 ---------
 sound/soc/tegra/tegra_rt5640.c       | 222 -------
 sound/soc/tegra/tegra_rt5677.c       | 324 ----------
 sound/soc/tegra/tegra_sgtl5000.c     | 211 -------
 sound/soc/tegra/tegra_wm8753.c       | 185 ------
 sound/soc/tegra/tegra_wm8903.c       | 351 +++--------
 sound/soc/tegra/tegra_wm9712.c       | 166 ------
 sound/soc/tegra/trimslice.c          | 172 ------
 15 files changed, 996 insertions(+), 2367 deletions(-)
 delete mode 100644 sound/soc/tegra/tegra_alc5632.c
 create mode 100644 sound/soc/tegra/tegra_asoc_machine.c
 create mode 100644 sound/soc/tegra/tegra_asoc_machine.h
 delete mode 100644 sound/soc/tegra/tegra_asoc_utils.c
 delete mode 100644 sound/soc/tegra/tegra_asoc_utils.h
 delete mode 100644 sound/soc/tegra/tegra_max98090.c
 delete mode 100644 sound/soc/tegra/tegra_rt5640.c
 delete mode 100644 sound/soc/tegra/tegra_rt5677.c
 delete mode 100644 sound/soc/tegra/tegra_sgtl5000.c
 delete mode 100644 sound/soc/tegra/tegra_wm8753.c
 delete mode 100644 sound/soc/tegra/tegra_wm9712.c
 delete mode 100644 sound/soc/tegra/trimslice.c

--
2.30.2
ninelore pushed a commit to ninelore/alsa-ucm-conf-cros that referenced this pull request Jun 10, 2024
- merge HaveAif1 and HaveAif2 to HaveAif with values 1 and 2
- HaveSpeaker identifies stereo and mono (values 2 and 1)
- HaveInternalMic identifies internal mic input (dmic, in1, in3)
- rewrite (unify) bytcr-rt5640 components detection

BugLink: alsa-project#92
Signed-off-by: Jaroslav Kysela <perex@perex.cz>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants