[alsa-devel] [PATCH] ASoC: hdmi-codec: hdmi_codec_priv includes snd_kcontrol_new
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.
Signed-off-by: Kuninori Morimoto kuninori.morimoto.gx@renesas.com --- sound/soc/codecs/hdmi-codec.c | 60 +++++++++++++++++++++++++++++++++---------- 1 file changed, 46 insertions(+), 14 deletions(-)
diff --git a/sound/soc/codecs/hdmi-codec.c b/sound/soc/codecs/hdmi-codec.c index 22ed0dc..5835a90 100644 --- a/sound/soc/codecs/hdmi-codec.c +++ b/sound/soc/codecs/hdmi-codec.c @@ -276,6 +276,7 @@ struct hdmi_codec_cea_spk_alloc { .mask = FL | FR | LFE | FC | RL | RR | FLC | FRC }, };
+#define ELD_NAME_SIZE 8 struct hdmi_codec_priv { struct hdmi_codec_pdata hcd; struct snd_soc_dai_driver *daidrv; @@ -284,7 +285,10 @@ struct hdmi_codec_priv { struct snd_pcm_substream *current_stream; uint8_t eld[MAX_ELD_BYTES]; struct snd_pcm_chmap *chmap_info; + struct snd_kcontrol_new control; unsigned int chmap_idx; + int id; + char eld_name[ELD_NAME_SIZE]; };
static const struct snd_soc_dapm_widget hdmi_widgets[] = { @@ -399,18 +403,6 @@ 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 int hdmi_codec_new_stream(struct snd_pcm_substream *substream, struct snd_soc_dai *dai) { @@ -718,6 +710,45 @@ static int hdmi_codec_pcm_new(struct snd_soc_pcm_runtime *rtd, .pcm_new = hdmi_codec_pcm_new, };
+static int hdmi_last_id = 0; +static int hdmi_probe(struct snd_soc_component *component) +{ + struct hdmi_codec_priv *hcp = snd_soc_component_get_drvdata(component); + static struct snd_soc_component *component_1st = NULL; + char *name = "ELD"; + + /* + * It will use "ELD" if hdmi_codec was used from only 1 device. + * It will use "ELD.x" if hdmi_codec was used from many devices. + * Then, first "ELD" will be replaced to "ELD.0" + */ + snprintf(hcp->eld_name, ELD_NAME_SIZE, "ELD.%d", hcp->id); + + if (hcp->id == 0) + component_1st = component; + + if (hdmi_last_id > 1) { + name = hcp->eld_name; + + if (component_1st) { + struct hdmi_codec_priv *hcp_1st; + + /* replaced 1st "ELD" to "ELD.0" */ + hcp_1st = snd_soc_component_get_drvdata(component_1st); + hcp_1st->control.name = hcp_1st->eld_name; + } + } + + hcp->control.access = SNDRV_CTL_ELEM_ACCESS_READ | + SNDRV_CTL_ELEM_ACCESS_VOLATILE; + hcp->control.iface = SNDRV_CTL_ELEM_IFACE_PCM; + hcp->control.name = name; + hcp->control.info = hdmi_eld_ctl_info; + hcp->control.get = hdmi_eld_ctl_get; + + return snd_soc_add_component_controls(component, &hcp->control, 1); +} + static int hdmi_of_xlate_dai_id(struct snd_soc_component *component, struct device_node *endpoint) { @@ -732,8 +763,7 @@ 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), + .probe = hdmi_probe, .dapm_widgets = hdmi_widgets, .num_dapm_widgets = ARRAY_SIZE(hdmi_widgets), .dapm_routes = hdmi_routes, @@ -768,6 +798,7 @@ static int hdmi_codec_probe(struct platform_device *pdev) if (!hcp) return -ENOMEM;
+ hcp->id = hdmi_last_id; hcp->hcd = *hcd; mutex_init(&hcp->current_stream_lock);
@@ -795,6 +826,7 @@ static int hdmi_codec_probe(struct platform_device *pdev) }
dev_set_drvdata(dev, hcp); + hdmi_last_id++; return 0; }
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
Signed-off-by: Kuninori Morimoto kuninori.morimoto.gx@renesas.com
sound/soc/codecs/hdmi-codec.c | 60 +++++++++++++++++++++++++++++++++---------- 1 file changed, 46 insertions(+), 14 deletions(-)
diff --git a/sound/soc/codecs/hdmi-codec.c b/sound/soc/codecs/hdmi-codec.c index 22ed0dc..5835a90 100644 --- a/sound/soc/codecs/hdmi-codec.c +++ b/sound/soc/codecs/hdmi-codec.c @@ -276,6 +276,7 @@ struct hdmi_codec_cea_spk_alloc { .mask = FL | FR | LFE | FC | RL | RR | FLC | FRC }, };
+#define ELD_NAME_SIZE 8 struct hdmi_codec_priv { struct hdmi_codec_pdata hcd; struct snd_soc_dai_driver *daidrv; @@ -284,7 +285,10 @@ struct hdmi_codec_priv { struct snd_pcm_substream *current_stream; uint8_t eld[MAX_ELD_BYTES]; struct snd_pcm_chmap *chmap_info;
- struct snd_kcontrol_new control; unsigned int chmap_idx;
- int id;
- char eld_name[ELD_NAME_SIZE];
};
static const struct snd_soc_dapm_widget hdmi_widgets[] = { @@ -399,18 +403,6 @@ 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 int hdmi_codec_new_stream(struct snd_pcm_substream *substream, struct snd_soc_dai *dai) { @@ -718,6 +710,45 @@ static int hdmi_codec_pcm_new(struct snd_soc_pcm_runtime *rtd, .pcm_new = hdmi_codec_pcm_new, };
+static int hdmi_last_id = 0; +static int hdmi_probe(struct snd_soc_component *component) +{
- struct hdmi_codec_priv *hcp = snd_soc_component_get_drvdata(component);
- static struct snd_soc_component *component_1st = NULL;
- char *name = "ELD";
- /*
* It will use "ELD" if hdmi_codec was used from only 1 device.* It will use "ELD.x" if hdmi_codec was used from many devices.* Then, first "ELD" will be replaced to "ELD.0"*/- snprintf(hcp->eld_name, ELD_NAME_SIZE, "ELD.%d", hcp->id);
- if (hcp->id == 0)
component_1st = component;- if (hdmi_last_id > 1) {
name = hcp->eld_name;if (component_1st) {struct hdmi_codec_priv *hcp_1st;/* replaced 1st "ELD" to "ELD.0" */hcp_1st = snd_soc_component_get_drvdata(component_1st);hcp_1st->control.name = hcp_1st->eld_name;}- }
- hcp->control.access = SNDRV_CTL_ELEM_ACCESS_READ |
SNDRV_CTL_ELEM_ACCESS_VOLATILE;- hcp->control.iface = SNDRV_CTL_ELEM_IFACE_PCM;
- hcp->control.name = name;
- hcp->control.info = hdmi_eld_ctl_info;
- hcp->control.get = hdmi_eld_ctl_get;
- return snd_soc_add_component_controls(component, &hcp->control, 1);
+}
static int hdmi_of_xlate_dai_id(struct snd_soc_component *component, struct device_node *endpoint) { @@ -732,8 +763,7 @@ 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),
.probe = hdmi_probe,@@ -768,6 +798,7 @@ static int hdmi_codec_probe(struct platform_device *pdev) if (!hcp) return -ENOMEM;
- hcp->id = hdmi_last_id; hcp->hcd = *hcd; mutex_init(&hcp->current_stream_lock);
@@ -795,6 +826,7 @@ static int hdmi_codec_probe(struct platform_device *pdev) }
dev_set_drvdata(dev, hcp);
- hdmi_last_id++; return 0;
}
-- 1.9.1
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.
Regards Arnaud
Signed-off-by: Kuninori Morimoto kuninori.morimoto.gx@renesas.com
sound/soc/codecs/hdmi-codec.c | 60 +++++++++++++++++++++++++++++++++---------- 1 file changed, 46 insertions(+), 14 deletions(-)
diff --git a/sound/soc/codecs/hdmi-codec.c b/sound/soc/codecs/hdmi-codec.c index 22ed0dc..5835a90 100644 --- a/sound/soc/codecs/hdmi-codec.c +++ b/sound/soc/codecs/hdmi-codec.c @@ -276,6 +276,7 @@ struct hdmi_codec_cea_spk_alloc { .mask = FL | FR | LFE | FC | RL | RR | FLC | FRC }, };
+#define ELD_NAME_SIZE 8 struct hdmi_codec_priv { struct hdmi_codec_pdata hcd; struct snd_soc_dai_driver *daidrv; @@ -284,7 +285,10 @@ struct hdmi_codec_priv { struct snd_pcm_substream *current_stream; uint8_t eld[MAX_ELD_BYTES]; struct snd_pcm_chmap *chmap_info;
- struct snd_kcontrol_new control; unsigned int chmap_idx;
- int id;
- char eld_name[ELD_NAME_SIZE];
};
static const struct snd_soc_dapm_widget hdmi_widgets[] = { @@ -399,18 +403,6 @@ 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 int hdmi_codec_new_stream(struct snd_pcm_substream *substream, struct snd_soc_dai *dai) { @@ -718,6 +710,45 @@ static int hdmi_codec_pcm_new(struct snd_soc_pcm_runtime *rtd, .pcm_new = hdmi_codec_pcm_new, };
+static int hdmi_last_id = 0; +static int hdmi_probe(struct snd_soc_component *component) +{
- struct hdmi_codec_priv *hcp = snd_soc_component_get_drvdata(component);
- static struct snd_soc_component *component_1st = NULL;
- char *name = "ELD";
- /*
* It will use "ELD" if hdmi_codec was used from only 1 device.* It will use "ELD.x" if hdmi_codec was used from many devices.* Then, first "ELD" will be replaced to "ELD.0"*/- snprintf(hcp->eld_name, ELD_NAME_SIZE, "ELD.%d", hcp->id);
- if (hcp->id == 0)
component_1st = component;- if (hdmi_last_id > 1) {
name = hcp->eld_name;if (component_1st) {struct hdmi_codec_priv *hcp_1st;/* replaced 1st "ELD" to "ELD.0" */hcp_1st = snd_soc_component_get_drvdata(component_1st);hcp_1st->control.name = hcp_1st->eld_name;}- }
- hcp->control.access = SNDRV_CTL_ELEM_ACCESS_READ |
SNDRV_CTL_ELEM_ACCESS_VOLATILE;- hcp->control.iface = SNDRV_CTL_ELEM_IFACE_PCM;
- hcp->control.name = name;
- hcp->control.info = hdmi_eld_ctl_info;
- hcp->control.get = hdmi_eld_ctl_get;
- return snd_soc_add_component_controls(component, &hcp->control, 1);
+}
static int hdmi_of_xlate_dai_id(struct snd_soc_component *component, struct device_node *endpoint) { @@ -732,8 +763,7 @@ 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),
.probe = hdmi_probe,@@ -768,6 +798,7 @@ static int hdmi_codec_probe(struct platform_device *pdev) if (!hcp) return -ENOMEM;
- hcp->id = hdmi_last_id; hcp->hcd = *hcd; mutex_init(&hcp->current_stream_lock);
@@ -795,6 +826,7 @@ static int hdmi_codec_probe(struct platform_device *pdev) }
dev_set_drvdata(dev, hcp);
- hdmi_last_id++; return 0;
}
-- 1.9.1
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
--- 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,
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),
Best regards --- Kuninori Morimoto
participants (3)
-
Arnaud Pouliquen -
Kuninori Morimoto -
Takashi Iwai