[pulseaudio-discuss] [alsa-devel] Per board ucm files on x86?

[Hans de Goede]

Hi,

On 11-12-17 14:40, Takashi Iwai wrote:
> On Mon, 11 Dec 2017 13:30:35 +0100,
> Hans de Goede wrote:
>>
>> Hi All,
>>
>> This weekend I've created a modified ucm config based on:
>>
>> https://github.com/plbossart/UCM/tree/master/chtrt5645
>>
>> For a board which has a single speaker connected to the
>> left channel (standard mono speaker setup) and a stereo
>> headphone jack with working jack detection.
>>
>> I've been unable to come up with a ucm file which allows
>> selecting between a "Stereo Speaker + Headphone" vs
>> "Mono Speaker + Headphone" output profile.
>>
>> https://github.com/plbossart/UCM/tree/master/byt-rt5640
>> has "Mono Speaker", "Stereo Speaker" and "Headphone"
>> profiles but does not auto-switch between Headphone
>> and speaker based on jack detection, I've been unable to
>> allow selecting either stereo or mono speaker while
>> keeping auto-switching to/from the headphones.
>>
>> But thinking more about this I don't think that having
>> "Stereo Speaker + Headphone" and "Mono Speaker + Headphone"
>> profiles is the answer. Profiles make sense on machines with
>> a bunch of outputs where we don't no what the user is going
>> to plug in, but in this case the stereo vs mono speaker
>> distinction is a clear property of the device, which we
>> should IMHO autodetect based on the device-model.
>>
>> So I think we need a way to have different ucm files per board,
>> so instead of loading /usr/share/ucm/chtrt5645/*.conf on my
>> device, alsa should try to load /usr/share/ucm/chtrt5645-<boardname>/*.conf
>>
>> Specifically I'm thinking about using udev + hwdb (dmi string)
>> matching to set an ALSA_UCM_NAME udev property.
>>
>> If the consensus is that this is a good idea I can take a shot
>> at writing patches for this (in my spare time mostly), the
>> downside of this approach is it would cause a dependency on
>> libudev for the alsa ucm code.
> 
> You don't need to patch, I guess.  The recent code should set a string
> generated from DMI as the longname of the card, and alsa-lib UCM
> parser prefers the longname to the driver name field.  That is,
> /usr/share/ucm/$LONGNAME/$LONGNAME.conf would be read at first, then
> /usr/share/ucm/$DRIVER/$DRIVER.conf is used as fallback.

Interesting unfortunately the DMI data on the GPD win / pocket
is too generic for snd_soc_set_dmi_name() to work, but we
already have a machine specific dmi-match in the codec driver,
so would something like this be acceptable to set a unique
longname ?   :

diff --git a/include/sound/rt5645.h b/include/sound/rt5645.h
index d0c33a9972b9..f218c742f08e 100644
--- a/include/sound/rt5645.h
+++ b/include/sound/rt5645.h
@@ -25,6 +25,9 @@ struct rt5645_platform_data {
  	bool level_trigger_irq;
  	/* Invert JD1_1 status polarity */
  	bool inv_jd1_1;
+
+	/* Value to asign to snd_soc_card.long_name */
+	const char *long_name;
  };

  #endif
diff --git a/sound/soc/codecs/rt5645.c b/sound/soc/codecs/rt5645.c
index f020d2d1eef4..60e5f4cda18c 100644
--- a/sound/soc/codecs/rt5645.c
+++ b/sound/soc/codecs/rt5645.c
@@ -3394,6 +3394,9 @@ static int rt5645_probe(struct snd_soc_codec *codec)
  		snd_soc_dapm_sync(dapm);
  	}

+	if (rt5645->pdata.long_name)
+		codec->component.card->long_name = rt5645->pdata.long_name;
+
  	rt5645->eq_param = devm_kzalloc(codec->dev,
  		RT5645_HWEQ_NUM * sizeof(struct rt5645_eq_param_s), GFP_KERNEL);

@@ -3624,6 +3627,7 @@ static const struct dmi_system_id dmi_platform_intel_broadwell[] = {
  static const struct rt5645_platform_data gpd_win_platform_data = {
  	.jd_mode = 3,
  	.inv_jd1_1 = true,
+	.long_name = "gpd-win-pocket-rt5645",
  };

  static const struct dmi_system_id dmi_platform_gpd_win[] = {


Regards,

Hans