On 5/10/18 10:48 AM, Hans de Goede wrote:
Hi,
On 10-05-18 17:00, Pierre-Louis Bossart wrote:
On 5/10/18 5:27 AM, Hans de Goede wrote:
Hi,
On 08-05-18 20:35, Pierre-Louis Bossart wrote:
On 5/8/18 10:36 AM, Hans de Goede wrote:
Many X86 devices using a BYT SoC + RT5640 codec are cheap devices with
generic DMI strings, causing snd_soc_set_dmi_name() to fail to set a
long_name, making it impossible for userspace to have a correct UCM
profile which only uses inputs / outputs which are actually hooked up
on the device.
Our quirks already specify which input the internal mic is
connected to
and if a single (mono) speaker is used or if the device has stereo
speakers.
This commit sets a long_name based on the quirks so that userspace can
have UCM profiles doing the right thing based on the long_name.
Isn't this going to be complicated to manage for UCM? Just with this
patch alone, you'd need 8 UCM files to cover all the combinations.
16 if you add the 'sof-' prefix.
seems like UCM should become more 'dynamic' and get quirk
information somehow (sysfs?) to enable/disable endpoints rather than
rely on name encoding to select the right profile?
I agree that this is not ideal, but this is an improvement from the
current state where we would need 1 UCM profile per board
(assuming valid DMI data and thus a proper long-name being set),
6 profiles (dmic2 is not used anywhere sofar) is a whole lot easier
to manage then 1 profile per board. So as said I believe this is
a step in the right direction.
And looking at the foreseeable future I simply don't see any of us
having the time to implement an ideal solution for this. I would
really like for end users to be able to run the latest upstream
kernel + alsa-lib and have things just work, before this hardware
becomes obsolete. I know that no-one having time to work on reworking
UCM to make it more dynamic is not the best of arguments but it
is something to take into consideration.
Thinking more about this on the alsa-lib / UCM profile side we
could have something like this:
/usr/share/alsa/ucm/bytcr-rt5640-mono-spk-in1-mic/bytcr-rt5640-mono-spk-in1-mic.conf:
SectionUseCase."HiFi" {
File "../bytcr-rt5640/Generic.conf"
File "../bytcr-rt5640/MonoSpeaker.conf"
File "../bytcr-rt5640/In1Mic.conf"
Comment "Play HiFi quality Music"
}
SectionDefaults [
cdev "hw:bytcrrt5640"
]
The only problem I can see with that is that the "ConflictingDevice"
sections for the various inputs / outputs then would refer to not
present SectionDevice sections. I have not tested this suggestion yet,
but I'm willing to write an alsa-lib patch to ignore non present
ConflictingDevice references, to make my suggestion work.
I think doing things this way, thus avoiding the need to copy and
paste a whole lot of UCM code for the 6 profiles it will not be
a problem to maintain 6 profiles, as we're really just maintaining
6 config snippets such as the above example and only one complete
profile.
Would the solution I outlined above be acceptable to you?
The includes and disabling conflicting devices that aren't present
make sense. I have another issue though: for SOF integration I already
prepared a set of files, which are mostly identical to the regular
ones except that the platform-side mixer controls are removed (or
different) and the name of the card/device is different (sof- prefix).
See on github.
Hmm, it might make sense to split the includes in platform and codec
includes, so
to pick my example again we would get:
/usr/share/alsa/ucm/bytcr-rt5640-mono-spk-in1-mic/bytcr-rt5640-mono-spk-in1-mic.conf:
SectionUseCase."HiFi" {
SectionVerb {
EnableSequence [
cdev "hw:bytcrrt5640"
File "../bytcr-rt5640/EnableSeq.conf" # This contains the
platform mixer settings
File "../rt5640/EnableSeq.conf"
]
DisableSequence [
]
Value {
PlaybackPCM "hw:bytcrrt5640"
CapturePCM "hw:bytcrrt5640"
}
}
File "../rt5640/Headset.conf"
File "../rt5640/MonoSpeaker.conf"
File "../rt5640/In1Mic.conf"
Comment "Play HiFi quality Music"
}
SectionDefaults [
cdev "hw:bytcrrt5640"
]
And then for sof you would just need to
offer a sof-rt5640/EnableSeq.conf, or
maybe even leave it out completely.
And we might also be able to merge the platform
enable sequences into a generic:
bytcr/EnableSeq.conf
I think that will at least fly for bytcr-rt5640 and
butcr-rt5651, leading us being able to remove more
duplicated UCM config.
How does this sound?
splitting platform and codec sides is a good idea (and something that
was done by removing all platform mixer settings from the HiFi files)
the problem remains that we have all these cdev strings that are
hard-codec with a card name. Same when the match happens based on a DMI
string, how would I know which of the platform settings to apply without
querying what the platform driver is?
Regards,
Hans
I was nearly ready to submit all those files to the new repository, I
wasn't aware of your work so now need to figure out how to reuse all
this once we upstream SOF (any time now if I look at Liam's patches).
Regards,
Hans
Note that if we ever encounter the need for a special UCM profile for
some device we can add a quirk to set a specific long_name for the
device,
Signed-off-by: Hans de Goede <hdegoede@xxxxxxxxxx>
---
sound/soc/intel/boards/bytcr_rt5640.c | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/sound/soc/intel/boards/bytcr_rt5640.c
b/sound/soc/intel/boards/bytcr_rt5640.c
index cfc520200214..9a1204dcdbc6 100644
--- a/sound/soc/intel/boards/bytcr_rt5640.c
+++ b/sound/soc/intel/boards/bytcr_rt5640.c
@@ -929,6 +929,7 @@ static struct snd_soc_dai_link
byt_rt5640_dais[] = {
static char byt_rt5640_codec_name[SND_ACPI_I2C_ID_LEN];
static char byt_rt5640_codec_aif_name[12]; /* =
"rt5640-aif[1|2]" */
static char byt_rt5640_cpu_dai_name[10]; /* = "ssp[0|2]-port" */
+static char byt_rt5640_long_name[40]; /* =
"bytcr-rt5640-*-spk-*-mic" */
static int byt_rt5640_suspend(struct snd_soc_card *card)
{
@@ -1000,6 +1001,7 @@ struct acpi_chan_package { /* ACPICA seems
to require 64 bit integers */
static int snd_byt_rt5640_mc_probe(struct platform_device *pdev)
{
+ const char * const map_name[] = { "dmic1", "dmic2", "in1",
"in3" };
const struct dmi_system_id *dmi_id;
struct byt_rt5640_private *priv;
struct snd_soc_acpi_mach *mach;
@@ -1163,6 +1165,13 @@ static int snd_byt_rt5640_mc_probe(struct
platform_device *pdev)
}
}
+ snprintf(byt_rt5640_long_name, sizeof(byt_rt5640_long_name),
+ "bytcr-rt5640-%s-spk-%s-mic",
+ (byt_rt5640_quirk & BYT_RT5640_MONO_SPEAKER) ?
+ "mono" : "stereo",
+ map_name[BYT_RT5640_MAP(byt_rt5640_quirk)]);
+ byt_rt5640_card.long_name = byt_rt5640_long_name;
+
ret_val = devm_snd_soc_register_card(&pdev->dev,
&byt_rt5640_card);
if (ret_val) {
_______________________________________________
Alsa-devel mailing list
Alsa-devel@xxxxxxxxxxxxxxxx
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
