diff mbox

ASoC: hdmi-codec: hdmi_codec_priv includes snd_kcontrol_new

Message ID s5h4lurdw7l.wl-tiwai@suse.de (mailing list archive)
State Superseded
Delegated to: Geert Uytterhoeven
Headers show

Commit Message

Takashi Iwai July 5, 2017, 10:17 a.m. UTC 
On Wed, 05 Jul 2017 11:29:43 +0200,
Arnaud Pouliquen wrote:
> 
> 
> 
> On 07/05/2017 10:40 AM, Takashi Iwai wrote:
> > On Wed, 05 Jul 2017 10:15:35 +0200,
> > Kuninori Morimoto wrote:
> >>
> >>
> >> From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
> >>
> >> Current hdmi-codec driver is using hdmi_controls for "ELD" control.
> >> But, hdmi-codec driver might be used from many HDMIs. In such case,
> >> they will use same "ELD" name and kernel will indicate below error.
> >>
> >> 	xxx: control x:x:x:ELD:x is already present
> >>
> >> hdmi_controls will be registered in soc_probe_component(), and we can
> >> replace it by component driver probe function.
> >>
> >> This patch registers current hdmi_controls in new hdmi_probe().
> >> If hdmi-codec is used from only 1 device, it will use "ELD" as control name
> >> (We can keep compatibility).
> >> If hdmi-codec is used from many devices, it will use "ELD.x" as control name.
> > 
> > The de facto standard way as HD-audio does is to pass the device
> > number of each ELD control as same as the corresponding PCM stream
> > number.
> > 
> > 
> > thanks,
> > 
> > Takashi
> > 
> In theory, hdmi_codec_pcm_new already does the job.
> snd_pcm_add_chmap_ctls instantiates the controls using PCM device ID. So
> you should observe (with amixer) 2 controls with same name but not same
> device ID.

Indeed, we can add each ELD control there like a (totally untested)
patch below.


thanks,

Takashi

---

Comments

Kuninori Morimoto July 6, 2017, 12:35 a.m. UTC | #1 
Hi Takashi

> > >> From: Kuninori Morimoto <kuninori.morimoto.gx@renesas.com>
> > >>
> > >> Current hdmi-codec driver is using hdmi_controls for "ELD" control.
> > >> But, hdmi-codec driver might be used from many HDMIs. In such case,
> > >> they will use same "ELD" name and kernel will indicate below error.
> > >>
> > >> 	xxx: control x:x:x:ELD:x is already present
> > >>
> > >> hdmi_controls will be registered in soc_probe_component(), and we can
> > >> replace it by component driver probe function.
> > >>
> > >> This patch registers current hdmi_controls in new hdmi_probe().
> > >> If hdmi-codec is used from only 1 device, it will use "ELD" as control name
> > >> (We can keep compatibility).
> > >> If hdmi-codec is used from many devices, it will use "ELD.x" as control name.

Thanks.
Below patch works well for me.
I will post v2 patch

> ---
> diff --git a/sound/soc/codecs/hdmi-codec.c b/sound/soc/codecs/hdmi-codec.c
> index 22ed0dc88f0a..13d9cf5b1831 100644
> --- a/sound/soc/codecs/hdmi-codec.c
> +++ b/sound/soc/codecs/hdmi-codec.c
> @@ -399,16 +399,13 @@ static int hdmi_codec_chmap_ctl_get(struct snd_kcontrol *kcontrol,
>  	return 0;
>  }
>  
> -
> -static const struct snd_kcontrol_new hdmi_controls[] = {
> -	{
> -		.access = SNDRV_CTL_ELEM_ACCESS_READ |
> -			  SNDRV_CTL_ELEM_ACCESS_VOLATILE,
> -		.iface = SNDRV_CTL_ELEM_IFACE_PCM,
> -		.name = "ELD",
> -		.info = hdmi_eld_ctl_info,
> -		.get = hdmi_eld_ctl_get,
> -	},
> +static const struct snd_kcontrol_new hdmi_eld_ctl = {
> +	.access = SNDRV_CTL_ELEM_ACCESS_READ |
> +		  SNDRV_CTL_ELEM_ACCESS_VOLATILE,
> +	.iface = SNDRV_CTL_ELEM_IFACE_PCM,
> +	.name = "ELD",
> +	.info = hdmi_eld_ctl_info,
> +	.get = hdmi_eld_ctl_get,
>  };
>  
>  static int hdmi_codec_new_stream(struct snd_pcm_substream *substream,
> @@ -668,6 +665,7 @@ static int hdmi_codec_pcm_new(struct snd_soc_pcm_runtime *rtd,
>  {
>  	struct snd_soc_dai_driver *drv = dai->driver;
>  	struct hdmi_codec_priv *hcp = snd_soc_dai_get_drvdata(dai);
> +	struct snd_kcontrol *kctl;
>  	int ret;
>  
>  	dev_dbg(dai->dev, "%s()\n", __func__);
> @@ -686,7 +684,12 @@ static int hdmi_codec_pcm_new(struct snd_soc_pcm_runtime *rtd,
>  	hcp->chmap_info->chmap = hdmi_codec_stereo_chmaps;
>  	hcp->chmap_idx = HDMI_CODEC_CHMAP_IDX_UNKNOWN;
>  
> -	return 0;
> +	/* add ELD ctl with the device number corresponding to the PCM stream */
> +	kctl = snd_ctl_new1(&hdmi_eld_ctl, dai->component);
> +	if (!kctl)
> +		return -ENOMEM;
> +	kctl->id.device = rtd->pcm->device;
> +	return snd_ctl_add(rtd->card->snd_card, kctl);
>  }
>  
>  static struct snd_soc_dai_driver hdmi_i2s_dai = {
> @@ -732,8 +735,6 @@ static int hdmi_of_xlate_dai_id(struct snd_soc_component *component,
>  
>  static struct snd_soc_codec_driver hdmi_codec = {
>  	.component_driver = {
> -		.controls		= hdmi_controls,
> -		.num_controls		= ARRAY_SIZE(hdmi_controls),
>  		.dapm_widgets		= hdmi_widgets,
>  		.num_dapm_widgets	= ARRAY_SIZE(hdmi_widgets),
>  		.dapm_routes		= hdmi_routes,


Best regards
---
Kuninori Morimoto
diff mbox

Patch

diff --git a/sound/soc/codecs/hdmi-codec.c b/sound/soc/codecs/hdmi-codec.c
index 22ed0dc88f0a..13d9cf5b1831 100644
--- a/sound/soc/codecs/hdmi-codec.c
+++ b/sound/soc/codecs/hdmi-codec.c
@@ -399,16 +399,13 @@  static int hdmi_codec_chmap_ctl_get(struct snd_kcontrol *kcontrol,
 	return 0;
 }
 
-
-static const struct snd_kcontrol_new hdmi_controls[] = {
-	{
-		.access = SNDRV_CTL_ELEM_ACCESS_READ |
-			  SNDRV_CTL_ELEM_ACCESS_VOLATILE,
-		.iface = SNDRV_CTL_ELEM_IFACE_PCM,
-		.name = "ELD",
-		.info = hdmi_eld_ctl_info,
-		.get = hdmi_eld_ctl_get,
-	},
+static const struct snd_kcontrol_new hdmi_eld_ctl = {
+	.access = SNDRV_CTL_ELEM_ACCESS_READ |
+		  SNDRV_CTL_ELEM_ACCESS_VOLATILE,
+	.iface = SNDRV_CTL_ELEM_IFACE_PCM,
+	.name = "ELD",
+	.info = hdmi_eld_ctl_info,
+	.get = hdmi_eld_ctl_get,
 };
 
 static int hdmi_codec_new_stream(struct snd_pcm_substream *substream,
@@ -668,6 +665,7 @@  static int hdmi_codec_pcm_new(struct snd_soc_pcm_runtime *rtd,
 {
 	struct snd_soc_dai_driver *drv = dai->driver;
 	struct hdmi_codec_priv *hcp = snd_soc_dai_get_drvdata(dai);
+	struct snd_kcontrol *kctl;
 	int ret;
 
 	dev_dbg(dai->dev, "%s()\n", __func__);
@@ -686,7 +684,12 @@  static int hdmi_codec_pcm_new(struct snd_soc_pcm_runtime *rtd,
 	hcp->chmap_info->chmap = hdmi_codec_stereo_chmaps;
 	hcp->chmap_idx = HDMI_CODEC_CHMAP_IDX_UNKNOWN;
 
-	return 0;
+	/* add ELD ctl with the device number corresponding to the PCM stream */
+	kctl = snd_ctl_new1(&hdmi_eld_ctl, dai->component);
+	if (!kctl)
+		return -ENOMEM;
+	kctl->id.device = rtd->pcm->device;
+	return snd_ctl_add(rtd->card->snd_card, kctl);
 }
 
 static struct snd_soc_dai_driver hdmi_i2s_dai = {
@@ -732,8 +735,6 @@  static int hdmi_of_xlate_dai_id(struct snd_soc_component *component,
 
 static struct snd_soc_codec_driver hdmi_codec = {
 	.component_driver = {
-		.controls		= hdmi_controls,
-		.num_controls		= ARRAY_SIZE(hdmi_controls),
 		.dapm_widgets		= hdmi_widgets,
 		.num_dapm_widgets	= ARRAY_SIZE(hdmi_widgets),
 		.dapm_routes		= hdmi_routes,