diff mbox

[RFC,v3,7/7] ASoC: hdmi-codec: Add ELD control

Message ID 1452613096-8116-8-git-send-email-p.zabel@pengutronix.de (mailing list archive)
State New, archived
Headers show

Commit Message

Philipp Zabel Jan. 12, 2016, 3:38 p.m. UTC
Exporting the ELD bytes to userspace allows an application to select
an appropriate audio format depending on the current capabilities of
the connected HDMI sink device.

Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
---
 sound/soc/codecs/hdmi-codec.c | 36 ++++++++++++++++++++++++++++++++++++
 1 file changed, 36 insertions(+)

Comments

Jyri Sarha Jan. 19, 2016, noon UTC | #1
On 01/12/16 17:38, Philipp Zabel wrote:
> Exporting the ELD bytes to userspace allows an application to select
> an appropriate audio format depending on the current capabilities of
> the connected HDMI sink device.
>

This looks good, but I was unable to test it. Obviously amixer or 
alsamixer is unable to show anything about an ELD mixer element, but 
shouldn't there be something about it shown under /proc/asound/card0/, 
or am I mistaken.

Cheers,
Jyri

> Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
> ---
>   sound/soc/codecs/hdmi-codec.c | 36 ++++++++++++++++++++++++++++++++++++
>   1 file changed, 36 insertions(+)
>
> diff --git a/sound/soc/codecs/hdmi-codec.c b/sound/soc/codecs/hdmi-codec.c
> index 6b327e2..6cb7217 100644
> --- a/sound/soc/codecs/hdmi-codec.c
> +++ b/sound/soc/codecs/hdmi-codec.c
> @@ -54,6 +54,40 @@ enum {
>   	DAI_ID_SPDIF,
>   };
>
> +static int hdmi_eld_ctl_info(struct snd_kcontrol *kcontrol,
> +			     struct snd_ctl_elem_info *uinfo)
> +{
> +	struct snd_soc_component *component = snd_kcontrol_chip(kcontrol);
> +	struct hdmi_codec_priv *hcp = snd_soc_component_get_drvdata(component);
> +
> +	uinfo->type = SNDRV_CTL_ELEM_TYPE_BYTES;
> +	uinfo->count = sizeof(hcp->eld);
> +
> +	return 0;
> +}
> +
> +static int hdmi_eld_ctl_get(struct snd_kcontrol *kcontrol,
> +			    struct snd_ctl_elem_value *ucontrol)
> +{
> +	struct snd_soc_component *component = snd_kcontrol_chip(kcontrol);
> +	struct hdmi_codec_priv *hcp = snd_soc_component_get_drvdata(component);
> +
> +	memcpy(ucontrol->value.bytes.data, hcp->eld, sizeof(hcp->eld));
> +
> +	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 void hdmi_codec_abort(struct device *dev)
>   {
>   	struct hdmi_codec_priv *hcp = dev_get_drvdata(dev);
> @@ -327,6 +361,8 @@ static const struct snd_soc_dai_driver hdmi_spdif_dai = {
>   };
>
>   static struct snd_soc_codec_driver hdmi_codec = {
> +	.controls = hdmi_controls,
> +	.num_controls = ARRAY_SIZE(hdmi_controls),
>   	.dapm_widgets = hdmi_widgets,
>   	.num_dapm_widgets = ARRAY_SIZE(hdmi_widgets),
>   	.dapm_routes = hdmi_routes,
>
Takashi Iwai Jan. 19, 2016, 12:05 p.m. UTC | #2
On Tue, 19 Jan 2016 13:00:05 +0100,
Jyri Sarha wrote:
> 
> On 01/12/16 17:38, Philipp Zabel wrote:
> > Exporting the ELD bytes to userspace allows an application to select
> > an appropriate audio format depending on the current capabilities of
> > the connected HDMI sink device.
> >
> 
> This looks good, but I was unable to test it. Obviously amixer or 
> alsamixer is unable to show anything about an ELD mixer element,

You can use amixer cget command to retrieve the information.

> but 
> shouldn't there be something about it shown under /proc/asound/card0/, 
> or am I mistaken.

It's specific to driver.  HD-audio has one, but it doesn't show the
whole ELD but only parsed contents.


Takashi
Jyri Sarha Jan. 19, 2016, 12:47 p.m. UTC | #3
On 01/19/16 14:05, Takashi Iwai wrote:
> On Tue, 19 Jan 2016 13:00:05 +0100,
> Jyri Sarha wrote:
>>
>> On 01/12/16 17:38, Philipp Zabel wrote:
>>> Exporting the ELD bytes to userspace allows an application to select
>>> an appropriate audio format depending on the current capabilities of
>>> the connected HDMI sink device.
>>>
>>
>> This looks good, but I was unable to test it. Obviously amixer or
>> alsamixer is unable to show anything about an ELD mixer element,
>
> You can use amixer cget command to retrieve the information.
>
>> but
>> shouldn't there be something about it shown under /proc/asound/card0/,
>> or am I mistaken.
>
> It's specific to driver.  HD-audio has one, but it doesn't show the
> whole ELD but only parsed contents.
>
>

Thanks,
I managed the read the ELD bytes with:

amixer -c0 cget iface=PCM,name='ELD',device=0

And the numbers change a bit when I connect to a different TV, so:

Reviewed-by: Jyri Sarha <jsarha@ti.com>
Tested-by: Jyri Sarha <jsarha@ti.com>

Cheers,
Jyri
Lars-Peter Clausen Jan. 19, 2016, 1:47 p.m. UTC | #4
On 01/12/2016 04:38 PM, Philipp Zabel wrote:
> Exporting the ELD bytes to userspace allows an application to select
> an appropriate audio format depending on the current capabilities of
> the connected HDMI sink device.

This needs an explanation why this method, which will require explicit
support in applications, should be used rather than the standard ALSA
hardware capabilities reporting.
Russell King - ARM Linux Jan. 19, 2016, 5:14 p.m. UTC | #5
On Tue, Jan 19, 2016 at 02:47:50PM +0100, Lars-Peter Clausen wrote:
> On 01/12/2016 04:38 PM, Philipp Zabel wrote:
> > Exporting the ELD bytes to userspace allows an application to select
> > an appropriate audio format depending on the current capabilities of
> > the connected HDMI sink device.
> 
> This needs an explanation why this method, which will require explicit
> support in applications, should be used rather than the standard ALSA
> hardware capabilities reporting.

That can be summed up in one line:

ALSA doesn't know about all the different formats for compressed audio.

As far as I'm aware, applications today just lie to ALSA and use the
"16-bit PCM audio" formats (SNDRV_PCM_FORMAT_S16_LE or
SNDRV_PCM_FORMAT_U16_LE) to send compressed audio via ALSA, relying
on the IEC controls to set the channel status data correctly.

Maybe it would be nice to add formats for DTS, DTS-HD, AC3, OBA, etc
to the ALSA format list.
Mark Brown Jan. 19, 2016, 6:32 p.m. UTC | #6
On Tue, Jan 19, 2016 at 01:05:19PM +0100, Takashi Iwai wrote:
> Jyri Sarha wrote:

> > but 
> > shouldn't there be something about it shown under /proc/asound/card0/, 
> > or am I mistaken.

> It's specific to driver.  HD-audio has one, but it doesn't show the
> whole ELD but only parsed contents.

Seems like it might be worth standardising at least some aspects of
that?
diff mbox

Patch

diff --git a/sound/soc/codecs/hdmi-codec.c b/sound/soc/codecs/hdmi-codec.c
index 6b327e2..6cb7217 100644
--- a/sound/soc/codecs/hdmi-codec.c
+++ b/sound/soc/codecs/hdmi-codec.c
@@ -54,6 +54,40 @@  enum {
 	DAI_ID_SPDIF,
 };
 
+static int hdmi_eld_ctl_info(struct snd_kcontrol *kcontrol,
+			     struct snd_ctl_elem_info *uinfo)
+{
+	struct snd_soc_component *component = snd_kcontrol_chip(kcontrol);
+	struct hdmi_codec_priv *hcp = snd_soc_component_get_drvdata(component);
+
+	uinfo->type = SNDRV_CTL_ELEM_TYPE_BYTES;
+	uinfo->count = sizeof(hcp->eld);
+
+	return 0;
+}
+
+static int hdmi_eld_ctl_get(struct snd_kcontrol *kcontrol,
+			    struct snd_ctl_elem_value *ucontrol)
+{
+	struct snd_soc_component *component = snd_kcontrol_chip(kcontrol);
+	struct hdmi_codec_priv *hcp = snd_soc_component_get_drvdata(component);
+
+	memcpy(ucontrol->value.bytes.data, hcp->eld, sizeof(hcp->eld));
+
+	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 void hdmi_codec_abort(struct device *dev)
 {
 	struct hdmi_codec_priv *hcp = dev_get_drvdata(dev);
@@ -327,6 +361,8 @@  static const struct snd_soc_dai_driver hdmi_spdif_dai = {
 };
 
 static struct snd_soc_codec_driver hdmi_codec = {
+	.controls = hdmi_controls,
+	.num_controls = ARRAY_SIZE(hdmi_controls),
 	.dapm_widgets = hdmi_widgets,
 	.num_dapm_widgets = ARRAY_SIZE(hdmi_widgets),
 	.dapm_routes = hdmi_routes,