[alsa-devel] [PATCH v3] ALSA: hdmi: expose ELD control
Applications may want to read ELD information to understand what codecs are supported on the HDMI receiver and handle the a-v delay for better lip-sync.
ELD information is exposed in a device-specific IFACE_PCM kcontrol. Tested both with amixer and PulseAudio.
ELD control size is set to zero in case of errors or wrong configurations. No notifications are implemented for now, it is expected that jack detection is used to reconfigure the audio outputs.
Signed-off-by: Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com --- sound/pci/hda/hda_eld.c | 13 ++++-- sound/pci/hda/hda_local.h | 2 + sound/pci/hda/patch_hdmi.c | 88 ++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 98 insertions(+), 5 deletions(-)
diff --git a/sound/pci/hda/hda_eld.c b/sound/pci/hda/hda_eld.c index c34f730..f1c621d 100644 --- a/sound/pci/hda/hda_eld.c +++ b/sound/pci/hda/hda_eld.c @@ -318,6 +318,11 @@ int snd_hdmi_get_eld(struct hdmi_eld *eld, int size; unsigned char *buf;
+ /* + * ELD size is initialized to zero in caller function. If no errors and + * ELD is valid, actual eld_size is assigned in hdmi_update_eld() + */ + if (!eld->eld_valid) return -ENOENT;
@@ -327,14 +332,13 @@ int snd_hdmi_get_eld(struct hdmi_eld *eld, snd_printd(KERN_INFO "HDMI: ELD buf size is 0, force 128\n"); size = 128; } - if (size < ELD_FIXED_BYTES || size > PAGE_SIZE) { + if (size < ELD_FIXED_BYTES || size > ELD_MAX_SIZE) { snd_printd(KERN_INFO "HDMI: invalid ELD buf size %d\n", size); return -ERANGE; }
- buf = kmalloc(size, GFP_KERNEL); - if (!buf) - return -ENOMEM; + /* set ELD buffer */ + buf = eld->eld_buffer;
for (i = 0; i < size; i++) { unsigned int val = hdmi_get_eld_data(codec, nid, i); @@ -356,7 +360,6 @@ int snd_hdmi_get_eld(struct hdmi_eld *eld, ret = hdmi_update_eld(eld, buf, size);
error: - kfree(buf); return ret; }
diff --git a/sound/pci/hda/hda_local.h b/sound/pci/hda/hda_local.h index aaefa7c..04d730f 100644 --- a/sound/pci/hda/hda_local.h +++ b/sound/pci/hda/hda_local.h @@ -621,6 +621,7 @@ struct cea_sad { };
#define ELD_FIXED_BYTES 20 +#define ELD_MAX_SIZE 256 #define ELD_MAX_MNL 16 #define ELD_MAX_SAD 16
@@ -645,6 +646,7 @@ struct hdmi_eld { int spk_alloc; int sad_count; struct cea_sad sad[ELD_MAX_SAD]; + char eld_buffer[ELD_MAX_SIZE]; #ifdef CONFIG_PROC_FS struct snd_info_entry *proc_entry; #endif diff --git a/sound/pci/hda/patch_hdmi.c b/sound/pci/hda/patch_hdmi.c index 3f1f6ac..177b4af 100644 --- a/sound/pci/hda/patch_hdmi.c +++ b/sound/pci/hda/patch_hdmi.c @@ -324,6 +324,86 @@ static int cvt_nid_to_cvt_index(struct hdmi_spec *spec, hda_nid_t cvt_nid) return -EINVAL; }
+static int hdmi_eld_ctl_info(struct snd_kcontrol *kcontrol, + struct snd_ctl_elem_info *uinfo) +{ + struct hda_codec *codec = snd_kcontrol_chip(kcontrol); + struct hdmi_spec *spec; + struct hdmi_spec_per_pin *per_pin; + struct hdmi_eld *eld; + int pin_idx; + + spec = codec->spec; + uinfo->type = SNDRV_CTL_ELEM_TYPE_BYTES; + + pin_idx = kcontrol->private_value; + if (snd_BUG_ON(pin_idx < 0)) + return -EINVAL; + + per_pin = &spec->pins[pin_idx]; + eld = &per_pin->sink_eld; + uinfo->count = eld->eld_size; + + return 0; +} + +static int hdmi_eld_ctl_get(struct snd_kcontrol *kcontrol, + struct snd_ctl_elem_value *ucontrol) +{ + struct hda_codec *codec = snd_kcontrol_chip(kcontrol); + struct hdmi_spec *spec; + struct hdmi_spec_per_pin *per_pin; + struct hdmi_eld *eld; + int pin_idx; + int size; + + spec = codec->spec; + pin_idx = kcontrol->private_value; + + if (snd_BUG_ON(pin_idx < 0)) + return -EINVAL; + + memset(ucontrol->value.bytes.data, 0, ELD_MAX_SIZE); + per_pin = &spec->pins[pin_idx]; + eld = &per_pin->sink_eld; + size = eld->eld_size; + + memcpy(ucontrol->value.bytes.data, eld->eld_buffer, size); + + return 0; +} + +static struct snd_kcontrol_new eld_bytes_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_create_eld_ctl(struct hda_codec *codec, int pin_idx, + int device) +{ + struct snd_kcontrol *kctl; + struct hdmi_spec *spec = codec->spec; + int err; + + if (pin_idx < 0) + return -EINVAL; + + kctl = snd_ctl_new1(&eld_bytes_ctl, codec); + if (!kctl) + return -ENOMEM; + kctl->private_value = pin_idx; + kctl->id.device = device; + + err = snd_hda_ctl_add(codec, spec->pins[pin_idx].pin_nid, kctl); + if (err < 0) + return err; + + return 0; +} + #ifdef BE_PARANOID static void hdmi_get_dip_index(struct hda_codec *codec, hda_nid_t pin_nid, int *packet_index, int *byte_index) @@ -1193,6 +1273,14 @@ static int generic_hdmi_build_controls(struct hda_codec *codec) if (err < 0) return err; snd_hda_spdif_ctls_unassign(codec, pin_idx); + + /* add control for ELD Bytes */ + err = hdmi_create_eld_ctl(codec, + pin_idx, + spec->pcm_rec[pin_idx].device); + + if (err < 0) + return err; }
return 0;
Hi Pierre-Louis,
Some nit picks for reducing LOC :)
[snip]
+static int hdmi_eld_ctl_info(struct snd_kcontrol *kcontrol,
struct snd_ctl_elem_info *uinfo)
+{
- struct hda_codec *codec = snd_kcontrol_chip(kcontrol);
- struct hdmi_spec *spec;
- struct hdmi_spec_per_pin *per_pin;
- struct hdmi_eld *eld;
- int pin_idx;
- spec = codec->spec;
- uinfo->type = SNDRV_CTL_ELEM_TYPE_BYTES;
- pin_idx = kcontrol->private_value;
- if (snd_BUG_ON(pin_idx < 0))
return -EINVAL;
- per_pin = &spec->pins[pin_idx];
- eld = &per_pin->sink_eld;
- uinfo->count = eld->eld_size;
The local variables @per_pin and @eld are referenced only and hence looks unnecessary.
- return 0;
+}
+static int hdmi_eld_ctl_get(struct snd_kcontrol *kcontrol,
struct snd_ctl_elem_value *ucontrol)
+{
- struct hda_codec *codec = snd_kcontrol_chip(kcontrol);
- struct hdmi_spec *spec;
- struct hdmi_spec_per_pin *per_pin;
- struct hdmi_eld *eld;
- int pin_idx;
- int size;
- spec = codec->spec;
- pin_idx = kcontrol->private_value;
- if (snd_BUG_ON(pin_idx < 0))
return -EINVAL;
That check looks unnecessary since the pin_idx originates from the below hdmi_create_eld_ctl() which already checked it against 0.
- memset(ucontrol->value.bytes.data, 0, ELD_MAX_SIZE);
- per_pin = &spec->pins[pin_idx];
- eld = &per_pin->sink_eld;
- size = eld->eld_size;
@per_pin and @eld seems not necessary.
- memcpy(ucontrol->value.bytes.data, eld->eld_buffer, size);
The memset/memcpy could be merged into a single
memcpy(ucontrol->value.bytes.data, eld->eld_buffer, ELD_MAX_SIZE);
- return 0;
+}
+static struct snd_kcontrol_new eld_bytes_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_create_eld_ctl(struct hda_codec *codec, int pin_idx,
int device)
+{
- struct snd_kcontrol *kctl;
- struct hdmi_spec *spec = codec->spec;
- int err;
- if (pin_idx < 0)
return -EINVAL;
Even that check looks unnecessary.. see below
- kctl = snd_ctl_new1(&eld_bytes_ctl, codec);
- if (!kctl)
return -ENOMEM;
- kctl->private_value = pin_idx;
- kctl->id.device = device;
- err = snd_hda_ctl_add(codec, spec->pins[pin_idx].pin_nid, kctl);
- if (err < 0)
return err;
- return 0;
+}
#ifdef BE_PARANOID static void hdmi_get_dip_index(struct hda_codec *codec, hda_nid_t pin_nid, int *packet_index, int *byte_index) @@ -1193,6 +1273,14 @@ static int generic_hdmi_build_controls(struct hda_codec *codec) if (err < 0) return err; snd_hda_spdif_ctls_unassign(codec, pin_idx);
/* add control for ELD Bytes */
err = hdmi_create_eld_ctl(codec,
pin_idx,
spec->pcm_rec[pin_idx].device);
The above "pcm_rec[pin_idx]" will likely oops if ever pin_idx is an invalid value, before hdmi_create_eld_ctl() checks pin_idx.
participants (2)
-
Pierre-Louis Bossart
-
Wu Fengguang