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

Submitted by Hans de Goede on Dec. 11, 2017, 2:18 p.m.

Details

Message ID 127bca9f-3f4f-0926-335a-daa5b2883f07@redhat.com
State New
Headers show
Series "Per board ucm files on x86?" ( rev: 1 ) in PulseAudio

Not browsing as part of any series.

Commit Message

Hans de Goede Dec. 11, 2017, 2:18 p.m.
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 ?   :



Regards,

Hans

Patch hide | download patch | download mbox

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[] = {

Comments

On Mon, 11 Dec 2017 15:18:37 +0100,
Hans de Goede wrote:
> 
> 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,

Argh, that beast...

> 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 ?   :

Yes, it would work.


Takashi

> 
> 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
>