[RFC][PATCH] ELD routines and proc interface

[RFC][PATCH] ELD routines and proc interface

[Wu Fengguang]

Create hda_eld.c for ELD routines and proc interface.

ELD handling routines can be shared by all HDMI codecs,
and they are large enough to make a standalone source file.

Some notes on the code:

- Shall we show an empty file if the ELD content is not valid?
  Well it's not that simple. There could be partially populated ELD,
  and there may be malformed ELD provided by buggy drivers/monitors.
  So I tend to expose ELD as it is. 

- hdmi_show_eld() and hdmi_print_eld_info() are somehow duplicated
  The plan is to shrink hdmi_show_eld()/hdmi_show_short_audio_desc(), and to
  show a compact message at monitor hotplug and driver initialization time.

- The ELD retrieval routines rely on the Intel HDA interface,
  others are/could be universal and independent ones.

- The ELD proc interface code could be moved to hda_proc.c if necessary.

- How do we name the proc file?
  If there are going to be two HDMI pins per codec, then the current naming
  scheme (eld#<codec no>) will fail.

- The ELD proc file content is designed to be easy for scripts and human reading.
  Its lines all have the pattern:
	  <item_name>\t[\t]*<item_value>
  where <item_name> is a keyword in c language, while <item_value> could be any
  contents, including white spaces. <item_value> could also be a null value.


Signed-off-by: Wu Fengguang <wfg@linux.intel.com>
---
 sound/pci/Kconfig               |    4 
 sound/pci/hda/Makefile          |    1 
 sound/pci/hda/hda_eld.c         |  532 ++++++++++++++++++++++++++++++
 sound/pci/hda/hda_local.h       |   62 ++-
 sound/pci/hda/patch_intelhdmi.c |  484 ---------------------------
 5 files changed, 600 insertions(+), 483 deletions(-)

--- sound-2.6.orig/sound/pci/hda/patch_intelhdmi.c
+++ sound-2.6/sound/pci/hda/patch_intelhdmi.c
@@ -30,7 +30,6 @@
 #include <linux/delay.h>
 #include <linux/slab.h>
 #include <sound/core.h>
-#include <asm/unaligned.h>
 #include "hda_codec.h"
 #include "hda_local.h"
 #include "hda_patch.h"
@@ -40,43 +39,6 @@
 
 #define INTEL_HDMI_EVENT_TAG		0x08
 
-/*
- * CEA Short Audio Descriptor data
- */
-struct cea_sad {
-	int	channels;
-	int	format;		/* (format == 0) indicates invalid SAD */
-	int	rates;
-	int	sample_bits;	/* for LPCM */
-	int	max_bitrate;	/* for AC3...ATRAC */
-	int	profile;	/* for WMAPRO */
-};
-
-#define ELD_FIXED_BYTES	20
-#define ELD_MAX_MNL	16
-#define ELD_MAX_SAD	16
-
-/*
- * ELD: EDID Like Data
- */
-struct sink_eld {
-	int	eld_size;
-	int	baseline_len;
-	int	eld_ver;	/* (eld_ver == 0) indicates invalid ELD */
-	int	cea_edid_ver;
-	char	monitor_name[ELD_MAX_MNL + 1];
-	int	manufacture_id;
-	int	product_id;
-	u64	port_id;
-	int	support_hdcp;
-	int	support_ai;
-	int	conn_type;
-	int	aud_synch_delay;
-	int	spk_alloc;
-	int	sad_count;
-	struct cea_sad sad[ELD_MAX_SAD];
-};
-
 struct intel_hdmi_spec {
 	struct hda_multi_out multiout;
 	struct hda_pcm pcm_rec;
@@ -127,160 +89,9 @@ struct hdmi_audio_infoframe {
 };
 
 /*
- * SS1:SS0 index => sample size
- */
-static int cea_sample_sizes[4] = {
-	0,	 		/* 0: Refer to Stream Header */
-	AC_SUPPCM_BITS_16,	/* 1: 16 bits */
-	AC_SUPPCM_BITS_20,	/* 2: 20 bits */
-	AC_SUPPCM_BITS_24,	/* 3: 24 bits */
-};
-
-/*
- * SF2:SF1:SF0 index => sampling frequency
- */
-static int cea_sampling_frequencies[8] = {
-	0,			/* 0: Refer to Stream Header */
-	SNDRV_PCM_RATE_32000,	/* 1:  32000Hz */
-	SNDRV_PCM_RATE_44100,	/* 2:  44100Hz */
-	SNDRV_PCM_RATE_48000,	/* 3:  48000Hz */
-	SNDRV_PCM_RATE_88200,	/* 4:  88200Hz */
-	SNDRV_PCM_RATE_96000,	/* 5:  96000Hz */
-	SNDRV_PCM_RATE_176400,	/* 6: 176400Hz */
-	SNDRV_PCM_RATE_192000,	/* 7: 192000Hz */
-};
-
-enum eld_versions {
-	ELD_VER_CEA_861D	= 2,
-	ELD_VER_PARTIAL		= 31,
-};
-
-static char *eld_versoin_names[32] = {
-	"0-reserved",
-	"1-reserved",
-	"CEA-861D or below",
-	"3-reserved",
-	[4 ... 30] = "reserved",
-	[31] = "partial"
-};
-
-enum cea_edid_versions {
-	CEA_EDID_VER_NONE	= 0,
-	CEA_EDID_VER_CEA861	= 1,
-	CEA_EDID_VER_CEA861A	= 2,
-	CEA_EDID_VER_CEA861BCD	= 3,
-	CEA_EDID_VER_RESERVED	= 4,
-};
-
-static char *cea_edid_version_names[8] = {
-	"no CEA EDID Timing Extension block present",
-	"CEA-861",
-	"CEA-861-A",
-	"CEA-861-B, C or D",
-	"4-reserved",
-	[5 ... 7] = "reserved"
-};
-
-/*
- * CEA Speaker Allocation data block bits
- */
-#define CEA_SA_FLR	(0 << 0)
-#define CEA_SA_LFE	(1 << 1)
-#define CEA_SA_FC	(1 << 2)
-#define CEA_SA_RLR	(1 << 3)
-#define CEA_SA_RC	(1 << 4)
-#define CEA_SA_FLRC	(1 << 5)
-#define CEA_SA_RLRC	(1 << 6)
-/* the following are not defined in ELD yet */
-#define CEA_SA_FLRW	(1 << 7)
-#define CEA_SA_FLRH	(1 << 8)
-#define CEA_SA_TC	(1 << 9)
-#define CEA_SA_FCH	(1 << 10)
-
-static char *cea_speaker_allocation_names[] = {
-	/*  0 */ "FL/FR",
-	/*  1 */ "LFE",
-	/*  2 */ "FC",
-	/*  3 */ "RL/RR",
-	/*  4 */ "RC",
-	/*  5 */ "FLC/FRC",
-	/*  6 */ "RLC/RRC",
-	/*  7 */ "FLW/FRW",
-	/*  8 */ "FLH/FRH",
-	/*  9 */ "TC",
-	/* 10 */ "FCH",
-};
-
-static char *eld_connection_type_names[4] = {
-	"HDMI",
-	"Display Port",
-	"2-reserved",
-	"3-reserved"
-};
-
-enum cea_audio_coding_types {
-	AUDIO_CODING_TYPE_REF_STREAM_HEADER	=  0,
-	AUDIO_CODING_TYPE_LPCM			=  1,
-	AUDIO_CODING_TYPE_AC3			=  2,
-	AUDIO_CODING_TYPE_MPEG1			=  3,
-	AUDIO_CODING_TYPE_MP3			=  4,
-	AUDIO_CODING_TYPE_MPEG2			=  5,
-	AUDIO_CODING_TYPE_AACLC			=  6,
-	AUDIO_CODING_TYPE_DTS			=  7,
-	AUDIO_CODING_TYPE_ATRAC			=  8,
-	AUDIO_CODING_TYPE_SACD			=  9,
-	AUDIO_CODING_TYPE_EAC3			= 10,
-	AUDIO_CODING_TYPE_DTS_HD		= 11,
-	AUDIO_CODING_TYPE_MLP			= 12,
-	AUDIO_CODING_TYPE_DST			= 13,
-	AUDIO_CODING_TYPE_WMAPRO		= 14,
-	AUDIO_CODING_TYPE_REF_CXT		= 15,
-	/* also include valid xtypes below */
-	AUDIO_CODING_TYPE_HE_AAC		= 15,
-	AUDIO_CODING_TYPE_HE_AAC2		= 16,
-	AUDIO_CODING_TYPE_MPEG_SURROUND		= 17,
-};
-
-enum cea_audio_coding_xtypes {
-	AUDIO_CODING_XTYPE_HE_REF_CT		= 0,
-	AUDIO_CODING_XTYPE_HE_AAC		= 1,
-	AUDIO_CODING_XTYPE_HE_AAC2		= 2,
-	AUDIO_CODING_XTYPE_MPEG_SURROUND	= 3,
-	AUDIO_CODING_XTYPE_FIRST_RESERVED	= 4,
-};
-
-static char *cea_audio_coding_type_names[] = {
-	/*  0 */ "undefined",
-	/*  1 */ "LPCM",
-	/*  2 */ "AC-3",
-	/*  3 */ "MPEG1",
-	/*  4 */ "MP3",
-	/*  5 */ "MPEG2",
-	/*  6 */ "AAC-LC",
-	/*  7 */ "DTS",
-	/*  8 */ "ATRAC",
-	/*  9 */ "DSD(1-bit audio)",
-	/* 10 */ "Dolby Digital Plus(E-AC-3/DD+)",
-	/* 11 */ "DTS-HD",
-	/* 12 */ "Dolby TrueHD(MLP)",
-	/* 13 */ "DST",
-	/* 14 */ "WMAPro",
-	/* 15 */ "HE-AAC",
-	/* 16 */ "HE-AACv2",
-	/* 17 */ "MPEG Surround",
-};
-
-
-/*
  * HDMI routines
  */
 
-static int hdmi_get_eld_size(struct hda_codec *codec, hda_nid_t nid)
-{
-	return snd_hda_codec_read(codec, nid, 0, AC_VERB_GET_HDMI_DIP_SIZE,
-						 AC_DIPSIZE_ELD_BUF);
-}
-
 #ifdef BE_PARANOID
 static void hdmi_get_dip_index(struct hda_codec *codec, hda_nid_t nid,
 				int *packet_index, int *byte_index)
@@ -375,294 +186,13 @@ static void hdmi_setup_channel_mapping(s
 }
 
 
-/*
- * ELD(EDID Like Data) routines
- */
-
-static int hdmi_present_sense(struct hda_codec *codec, hda_nid_t nid)
-{
-	return snd_hda_codec_read(codec, nid, 0, AC_VERB_GET_PIN_SENSE, 0);
-}
-
-static void hdmi_debug_present_sense(struct hda_codec *codec)
-{
-#ifdef CONFIG_SND_DEBUG_VERBOSE
-	int eldv;
-	int present;
-
-	present = hdmi_present_sense(codec, PIN_NID);
-	eldv    = (present & AC_PINSENSE_ELDV);
-	present = (present & AC_PINSENSE_PRESENCE);
-
-	printk(KERN_INFO "pinp = %d, eldv = %d\n", !!present, !!eldv);
-#endif
-}
-
-static unsigned char hdmi_get_eld_byte(struct hda_codec *codec, int byte_index)
-{
-	unsigned int val;
-
-	val = snd_hda_codec_read(codec, PIN_NID, 0,
-					AC_VERB_GET_HDMI_ELDD, byte_index);
-
-#ifdef BE_PARANOID
-	printk(KERN_INFO "ELD data byte %d: 0x%x\n", byte_index, val);
-#endif
-
-	if ((val & AC_ELDD_ELD_VALID) == 0) {
-		snd_printd(KERN_INFO "Invalid ELD data byte %d\n",
-								byte_index);
-		val = 0;
-	}
-
-	return val & AC_ELDD_ELD_DATA;
-}
-
-static inline unsigned char grab_bits(const unsigned char *buf,
-						int byte, int lowbit, int bits)
-{
-	BUG_ON(lowbit > 7);
-	BUG_ON(bits > 8);
-	BUG_ON(bits <= 0);
-
-	return (buf[byte] >> lowbit) & ((1 << bits) - 1);
-}
-
-static void hdmi_update_short_audio_desc(struct cea_sad *a,
-					 const unsigned char *buf)
-{
-	int i;
-	int val;
-
-	val = grab_bits(buf, 1, 0, 7);
-	a->rates = 0;
-	for (i = 0; i < 7; i++)
-		if (val & (1 << i))
-			a->rates |= cea_sampling_frequencies[i + 1];
-
-	a->channels = grab_bits(buf, 0, 0, 3);
-	a->channels++;
-
-	a->format = grab_bits(buf, 0, 3, 4);
-	switch (a->format) {
-	case AUDIO_CODING_TYPE_REF_STREAM_HEADER:
-		snd_printd(KERN_INFO
-				"audio coding type 0 not expected in ELD\n");
-		break;
-
-	case AUDIO_CODING_TYPE_LPCM:
-		val = grab_bits(buf, 2, 0, 3);
-		a->sample_bits = 0;
-		for (i = 0; i < 3; i++)
-			if (val & (1 << i))
-				a->sample_bits |= cea_sample_sizes[i + 1];
-		break;
-
-	case AUDIO_CODING_TYPE_AC3:
-	case AUDIO_CODING_TYPE_MPEG1:
-	case AUDIO_CODING_TYPE_MP3:
-	case AUDIO_CODING_TYPE_MPEG2:
-	case AUDIO_CODING_TYPE_AACLC:
-	case AUDIO_CODING_TYPE_DTS:
-	case AUDIO_CODING_TYPE_ATRAC:
-		a->max_bitrate = grab_bits(buf, 2, 0, 8);
-		a->max_bitrate *= 8000;
-		break;
-
-	case AUDIO_CODING_TYPE_SACD:
-		break;
-
-	case AUDIO_CODING_TYPE_EAC3:
-		break;
-
-	case AUDIO_CODING_TYPE_DTS_HD:
-		break;
-
-	case AUDIO_CODING_TYPE_MLP:
-		break;
-
-	case AUDIO_CODING_TYPE_DST:
-		break;
-
-	case AUDIO_CODING_TYPE_WMAPRO:
-		a->profile = grab_bits(buf, 2, 0, 3);
-		break;
-
-	case AUDIO_CODING_TYPE_REF_CXT:
-		a->format = grab_bits(buf, 2, 3, 5);
-		if (a->format == AUDIO_CODING_XTYPE_HE_REF_CT ||
-		    a->format >= AUDIO_CODING_XTYPE_FIRST_RESERVED) {
-			snd_printd(KERN_INFO
-				"audio coding xtype %d not expected in ELD\n",
-				a->format);
-			a->format = 0;
-		} else
-			a->format += AUDIO_CODING_TYPE_HE_AAC -
-				     AUDIO_CODING_XTYPE_HE_AAC;
-		break;
-	}
-}
-
-static int hdmi_update_sink_eld(struct hda_codec *codec,
-				const unsigned char *buf, int size)
-{
-	struct intel_hdmi_spec *spec = codec->spec;
-	struct sink_eld *e = &spec->sink;
-	int mnl;
-	int i;
-
-	e->eld_ver = grab_bits(buf, 0, 3, 5);
-	if (e->eld_ver != ELD_VER_CEA_861D &&
-	    e->eld_ver != ELD_VER_PARTIAL) {
-		snd_printd(KERN_INFO "Unknown ELD version %d\n", e->eld_ver);
-		goto out_fail;
-	}
-
-	e->eld_size = size;
-	e->baseline_len = grab_bits(buf, 2, 0, 8);
-	mnl		= grab_bits(buf, 4, 0, 5);
-	e->cea_edid_ver	= grab_bits(buf, 4, 5, 3);
-
-	e->support_hdcp	= grab_bits(buf, 5, 0, 1);
-	e->support_ai	= grab_bits(buf, 5, 1, 1);
-	e->conn_type	= grab_bits(buf, 5, 2, 2);
-	e->sad_count	= grab_bits(buf, 5, 4, 4);
-
-	e->aud_synch_delay = grab_bits(buf, 6, 0, 8);
-	e->spk_alloc	= grab_bits(buf, 7, 0, 7);
-
-	e->port_id	  = get_unaligned_le64(buf + 8);
-
-	/* not specified, but the spec's tendency is little endian */
-	e->manufacture_id = get_unaligned_le16(buf + 16);
-	e->product_id	  = get_unaligned_le16(buf + 18);
-
-	if (mnl > ELD_MAX_MNL) {
-		snd_printd(KERN_INFO "MNL is reserved value %d\n", mnl);
-		goto out_fail;
-	} else if (ELD_FIXED_BYTES + mnl > size) {
-		snd_printd(KERN_INFO "out of range MNL %d\n", mnl);
-		goto out_fail;
-	} else
-		strlcpy(e->monitor_name, buf + ELD_FIXED_BYTES, mnl);
-
-	for (i = 0; i < e->sad_count; i++) {
-		if (ELD_FIXED_BYTES + mnl + 3 * (i + 1) > size) {
-			snd_printd(KERN_INFO "out of range SAD %d\n", i);
-			goto out_fail;
-		}
-		hdmi_update_short_audio_desc(e->sad + i,
-					buf + ELD_FIXED_BYTES + mnl + 3 * i);
-	}
-
-	return 0;
-
-out_fail:
-	e->eld_ver = 0;
-	return -EINVAL;
-}
-
-static int hdmi_get_eld(struct hda_codec *codec)
-{
-	int i;
-	int ret;
-	int size;
-	unsigned char *buf;
-
-	i = hdmi_present_sense(codec, PIN_NID) & AC_PINSENSE_ELDV;
-	if (!i)
-		return -ENOENT;
-
-	size = hdmi_get_eld_size(codec, PIN_NID);
-	if (size == 0) {
-		/* wfg: workaround for ASUS P5E-VM HDMI board */
-		snd_printd(KERN_INFO "ELD buf size is 0, force 128\n");
-		size = 128;
-	}
-	if (size < ELD_FIXED_BYTES || size > PAGE_SIZE) {
-		snd_printd(KERN_INFO "Invalid ELD buf size %d\n", size);
-		return -ERANGE;
-	}
-
-	buf = kmalloc(size, GFP_KERNEL);
-	if (!buf)
-		return -ENOMEM;
-
-	for (i = 0; i < size; i++)
-		buf[i] = hdmi_get_eld_byte(codec, i);
-
-	ret = hdmi_update_sink_eld(codec, buf, size);
-
-	kfree(buf);
-	return ret;
-}
-
-static void hdmi_show_short_audio_desc(struct cea_sad *a)
-{
-	printk(KERN_INFO "coding type: %s\n",
-					cea_audio_coding_type_names[a->format]);
-	printk(KERN_INFO "channels: %d\n", a->channels);
-	printk(KERN_INFO "sampling frequencies: 0x%x\n", a->rates);
-
-	if (a->format == AUDIO_CODING_TYPE_LPCM)
-		printk(KERN_INFO "sample bits: 0x%x\n", a->sample_bits);
-
-	if (a->max_bitrate)
-		printk(KERN_INFO "max bitrate: %d HZ\n", a->max_bitrate);
-
-	if (a->profile)
-		printk(KERN_INFO "profile: %d\n", a->profile);
-}
-
-static void hdmi_show_eld(struct hda_codec *codec)
-{
-	int i;
-	int j;
-	struct intel_hdmi_spec *spec = codec->spec;
-	struct sink_eld *e = &spec->sink;
-	char buf[80];
-
-	printk(KERN_INFO "ELD buffer size  is %d\n", e->eld_size);
-	printk(KERN_INFO "ELD baseline len is %d*4\n", e->baseline_len);
-	printk(KERN_INFO "vendor block len is %d\n",
-					e->eld_size - e->baseline_len * 4 - 4);
-	printk(KERN_INFO "ELD version      is %s\n",
-					eld_versoin_names[e->eld_ver]);
-	printk(KERN_INFO "CEA EDID version is %s\n",
-				cea_edid_version_names[e->cea_edid_ver]);
-	printk(KERN_INFO "manufacture id   is 0x%x\n", e->manufacture_id);
-	printk(KERN_INFO "product id       is 0x%x\n", e->product_id);
-	printk(KERN_INFO "port id          is 0x%llx\n", (long long)e->port_id);
-	printk(KERN_INFO "HDCP support     is %d\n", e->support_hdcp);
-	printk(KERN_INFO "AI support       is %d\n", e->support_ai);
-	printk(KERN_INFO "SAD count        is %d\n", e->sad_count);
-	printk(KERN_INFO "audio sync delay is %x\n", e->aud_synch_delay);
-	printk(KERN_INFO "connection type  is %s\n",
-				eld_connection_type_names[e->conn_type]);
-	printk(KERN_INFO "monitor name     is %s\n", e->monitor_name);
-
-	j = 0;
-	for (i = 0; i < ARRAY_SIZE(cea_speaker_allocation_names); i++) {
-		if (e->spk_alloc & (1 << i))
-			j += snprintf(buf + j, sizeof(buf) - j,  " %s",
-					cea_speaker_allocation_names[i]);
-	}
-	buf[j] = '\0'; /* necessary when j == 0 */
-	printk(KERN_INFO "speaker allocations: (0x%x)%s\n", e->spk_alloc, buf);
-
-	for (i = 0; i < e->sad_count; i++)
-		hdmi_show_short_audio_desc(e->sad + i);
-}
-
-/*
- * Be careful, ELD buf could be totally rubbish!
- */
 static void hdmi_parse_eld(struct hda_codec *codec)
 {
-	hdmi_debug_present_sense(codec);
+	struct intel_hdmi_spec *spec = codec->spec;
+	struct sink_eld *eld = &spec->sink;
 
-	if (!hdmi_get_eld(codec))
-		hdmi_show_eld(codec);
+	if (!hdmi_get_eld(eld, codec, PIN_NID))
+		hdmi_show_eld(eld);
 }
 
 
@@ -880,6 +410,11 @@ static int intel_hdmi_build_controls(str
 
 static int intel_hdmi_init(struct hda_codec *codec)
 {
+	struct intel_hdmi_spec *spec = codec->spec;
+	struct sink_eld *eld = &spec->sink;
+
+	snd_hda_eld_proc_new(codec, eld);
+
 	/* disable audio output as early as possible */
 	hdmi_disable_output(codec);
 
@@ -927,3 +462,4 @@ struct hda_codec_preset snd_hda_preset_i
 	{ .id = 0x10951392, .name = "SiI1392 HDMI",     .patch = patch_intel_hdmi },
 	{} /* terminator */
 };
+
--- /dev/null
+++ sound-2.6/sound/pci/hda/hda_eld.c
@@ -0,0 +1,532 @@
+/*
+ * Generic routines and proc interface for ELD(EDID Like Data) information
+ *
+ * Copyright(c) 2008 Intel Corporation.
+ *
+ * Authors:
+ * 		Wu Fengguang <wfg@linux.intel.com>
+ *
+ *  This driver is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License as published by
+ *  the Free Software Foundation; either version 2 of the License, or
+ *  (at your option) any later version.
+ *
+ *  This driver is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *  GNU General Public License for more details.
+ *
+ *  You should have received a copy of the GNU General Public License
+ *  along with this program; if not, write to the Free Software
+ *  Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307 USA
+ */
+
+#include <linux/init.h>
+#include <sound/core.h>
+#include <asm/unaligned.h>
+#include "hda_codec.h"
+#include "hda_local.h"
+
+enum eld_versions {
+	ELD_VER_CEA_861D	= 2,
+	ELD_VER_PARTIAL		= 31,
+};
+
+static char *eld_versoin_names[32] = {
+	"reserved",
+	"reserved",
+	"CEA-861D or below",
+	[3 ... 30] = "reserved",
+	[31] = "partial"
+};
+
+enum cea_edid_versions {
+	CEA_EDID_VER_NONE	= 0,
+	CEA_EDID_VER_CEA861	= 1,
+	CEA_EDID_VER_CEA861A	= 2,
+	CEA_EDID_VER_CEA861BCD	= 3,
+	CEA_EDID_VER_RESERVED	= 4,
+};
+
+static char *cea_edid_version_names[8] = {
+	"no CEA EDID Timing Extension block present",
+	"CEA-861",
+	"CEA-861-A",
+	"CEA-861-B, C or D",
+	[4 ... 7] = "reserved"
+};
+
+/*
+ * CEA Speaker Allocation data block bits
+ */
+#define CEA_SA_FLR	(0 << 0)
+#define CEA_SA_LFE	(1 << 1)
+#define CEA_SA_FC	(1 << 2)
+#define CEA_SA_RLR	(1 << 3)
+#define CEA_SA_RC	(1 << 4)
+#define CEA_SA_FLRC	(1 << 5)
+#define CEA_SA_RLRC	(1 << 6)
+/* the following are not defined in ELD yet */
+#define CEA_SA_FLRW	(1 << 7)
+#define CEA_SA_FLRH	(1 << 8)
+#define CEA_SA_TC	(1 << 9)
+#define CEA_SA_FCH	(1 << 10)
+
+static char *cea_speaker_allocation_names[] = {
+	/*  0 */ "FL/FR",
+	/*  1 */ "LFE",
+	/*  2 */ "FC",
+	/*  3 */ "RL/RR",
+	/*  4 */ "RC",
+	/*  5 */ "FLC/FRC",
+	/*  6 */ "RLC/RRC",
+	/*  7 */ "FLW/FRW",
+	/*  8 */ "FLH/FRH",
+	/*  9 */ "TC",
+	/* 10 */ "FCH",
+};
+
+static char *eld_connection_type_names[4] = {
+	"HDMI",
+	"Display Port",
+	"2-reserved",
+	"3-reserved"
+};
+
+enum cea_audio_coding_types {
+	AUDIO_CODING_TYPE_REF_STREAM_HEADER	=  0,
+	AUDIO_CODING_TYPE_LPCM			=  1,
+	AUDIO_CODING_TYPE_AC3			=  2,
+	AUDIO_CODING_TYPE_MPEG1			=  3,
+	AUDIO_CODING_TYPE_MP3			=  4,
+	AUDIO_CODING_TYPE_MPEG2			=  5,
+	AUDIO_CODING_TYPE_AACLC			=  6,
+	AUDIO_CODING_TYPE_DTS			=  7,
+	AUDIO_CODING_TYPE_ATRAC			=  8,
+	AUDIO_CODING_TYPE_SACD			=  9,
+	AUDIO_CODING_TYPE_EAC3			= 10,
+	AUDIO_CODING_TYPE_DTS_HD		= 11,
+	AUDIO_CODING_TYPE_MLP			= 12,
+	AUDIO_CODING_TYPE_DST			= 13,
+	AUDIO_CODING_TYPE_WMAPRO		= 14,
+	AUDIO_CODING_TYPE_REF_CXT		= 15,
+	/* also include valid xtypes below */
+	AUDIO_CODING_TYPE_HE_AAC		= 15,
+	AUDIO_CODING_TYPE_HE_AAC2		= 16,
+	AUDIO_CODING_TYPE_MPEG_SURROUND		= 17,
+};
+
+enum cea_audio_coding_xtypes {
+	AUDIO_CODING_XTYPE_HE_REF_CT		= 0,
+	AUDIO_CODING_XTYPE_HE_AAC		= 1,
+	AUDIO_CODING_XTYPE_HE_AAC2		= 2,
+	AUDIO_CODING_XTYPE_MPEG_SURROUND	= 3,
+	AUDIO_CODING_XTYPE_FIRST_RESERVED	= 4,
+};
+
+static char *cea_audio_coding_type_names[] = {
+	/*  0 */ "undefined",
+	/*  1 */ "LPCM",
+	/*  2 */ "AC-3",
+	/*  3 */ "MPEG1",
+	/*  4 */ "MP3",
+	/*  5 */ "MPEG2",
+	/*  6 */ "AAC-LC",
+	/*  7 */ "DTS",
+	/*  8 */ "ATRAC",
+	/*  9 */ "DSD(1-bit audio)",
+	/* 10 */ "Dolby Digital Plus(E-AC-3/DD+)",
+	/* 11 */ "DTS-HD",
+	/* 12 */ "Dolby TrueHD(MLP)",
+	/* 13 */ "DST",
+	/* 14 */ "WMAPro",
+	/* 15 */ "HE-AAC",
+	/* 16 */ "HE-AACv2",
+	/* 17 */ "MPEG Surround",
+};
+
+/*
+ * The following two lists are shared between
+ * 	- HDMI audio infoframe (source to sink)
+ * 	- CEA E-EDID extension (sink to source)
+ */
+
+/*
+ * SS1:SS0 index => sample size
+ */
+static int cea_sample_sizes[4] = {
+	0,	 		/* 0: Refer to Stream Header */
+	AC_SUPPCM_BITS_16,	/* 1: 16 bits */
+	AC_SUPPCM_BITS_20,	/* 2: 20 bits */
+	AC_SUPPCM_BITS_24,	/* 3: 24 bits */
+};
+
+/*
+ * SF2:SF1:SF0 index => sampling frequency
+ */
+static int cea_sampling_frequencies[8] = {
+	0,			/* 0: Refer to Stream Header */
+	SNDRV_PCM_RATE_32000,	/* 1:  32000Hz */
+	SNDRV_PCM_RATE_44100,	/* 2:  44100Hz */
+	SNDRV_PCM_RATE_48000,	/* 3:  48000Hz */
+	SNDRV_PCM_RATE_88200,	/* 4:  88200Hz */
+	SNDRV_PCM_RATE_96000,	/* 5:  96000Hz */
+	SNDRV_PCM_RATE_176400,	/* 6: 176400Hz */
+	SNDRV_PCM_RATE_192000,	/* 7: 192000Hz */
+};
+
+static unsigned char hdmi_get_eld_byte(struct hda_codec *codec, hda_nid_t nid,
+					int byte_index)
+{
+	unsigned int val;
+
+	val = snd_hda_codec_read(codec, nid, 0,
+					AC_VERB_GET_HDMI_ELDD, byte_index);
+
+#ifdef BE_PARANOID
+	printk(KERN_INFO "ELD data byte %d: 0x%x\n", byte_index, val);
+#endif
+
+	if ((val & AC_ELDD_ELD_VALID) == 0) {
+		snd_printd(KERN_INFO "Invalid ELD data byte %d\n",
+								byte_index);
+		val = 0;
+	}
+
+	return val & AC_ELDD_ELD_DATA;
+}
+
+static inline unsigned char grab_bits(const unsigned char *buf,
+						int byte, int lowbit, int bits)
+{
+	BUG_ON(lowbit > 7);
+	BUG_ON(bits > 8);
+	BUG_ON(bits <= 0);
+
+	return (buf[byte] >> lowbit) & ((1 << bits) - 1);
+}
+
+static void hdmi_update_short_audio_desc(struct cea_sad *a,
+					 const unsigned char *buf)
+{
+	int i;
+	int val;
+
+	val = grab_bits(buf, 1, 0, 7);
+	a->rates = 0;
+	for (i = 0; i < 7; i++)
+		if (val & (1 << i))
+			a->rates |= cea_sampling_frequencies[i + 1];
+
+	a->channels = grab_bits(buf, 0, 0, 3);
+	a->channels++;
+
+	a->format = grab_bits(buf, 0, 3, 4);
+	switch (a->format) {
+	case AUDIO_CODING_TYPE_REF_STREAM_HEADER:
+		snd_printd(KERN_INFO
+				"audio coding type 0 not expected in ELD\n");
+		break;
+
+	case AUDIO_CODING_TYPE_LPCM:
+		val = grab_bits(buf, 2, 0, 3);
+		a->sample_bits = 0;
+		for (i = 0; i < 3; i++)
+			if (val & (1 << i))
+				a->sample_bits |= cea_sample_sizes[i + 1];
+		break;
+
+	case AUDIO_CODING_TYPE_AC3:
+	case AUDIO_CODING_TYPE_MPEG1:
+	case AUDIO_CODING_TYPE_MP3:
+	case AUDIO_CODING_TYPE_MPEG2:
+	case AUDIO_CODING_TYPE_AACLC:
+	case AUDIO_CODING_TYPE_DTS:
+	case AUDIO_CODING_TYPE_ATRAC:
+		a->max_bitrate = grab_bits(buf, 2, 0, 8);
+		a->max_bitrate *= 8000;
+		break;
+
+	case AUDIO_CODING_TYPE_SACD:
+		break;
+
+	case AUDIO_CODING_TYPE_EAC3:
+		break;
+
+	case AUDIO_CODING_TYPE_DTS_HD:
+		break;
+
+	case AUDIO_CODING_TYPE_MLP:
+		break;
+
+	case AUDIO_CODING_TYPE_DST:
+		break;
+
+	case AUDIO_CODING_TYPE_WMAPRO:
+		a->profile = grab_bits(buf, 2, 0, 3);
+		break;
+
+	case AUDIO_CODING_TYPE_REF_CXT:
+		a->format = grab_bits(buf, 2, 3, 5);
+		if (a->format == AUDIO_CODING_XTYPE_HE_REF_CT ||
+		    a->format >= AUDIO_CODING_XTYPE_FIRST_RESERVED) {
+			snd_printd(KERN_INFO
+				"audio coding xtype %d not expected in ELD\n",
+				a->format);
+			a->format = 0;
+		} else
+			a->format += AUDIO_CODING_TYPE_HE_AAC -
+				     AUDIO_CODING_XTYPE_HE_AAC;
+		break;
+	}
+}
+
+/*
+ * Be careful, ELD buf could be totally rubbish!
+ */
+static int hdmi_update_sink_eld(struct sink_eld *e,
+				const unsigned char *buf, int size)
+{
+	int mnl;
+	int i;
+
+	e->eld_ver = grab_bits(buf, 0, 3, 5);
+	if (e->eld_ver != ELD_VER_CEA_861D &&
+	    e->eld_ver != ELD_VER_PARTIAL) {
+		snd_printd(KERN_INFO "Unknown ELD version %d\n", e->eld_ver);
+		goto out_fail;
+	}
+
+	e->eld_size = size;
+	e->baseline_len = grab_bits(buf, 2, 0, 8);
+	mnl		= grab_bits(buf, 4, 0, 5);
+	e->cea_edid_ver	= grab_bits(buf, 4, 5, 3);
+
+	e->support_hdcp	= grab_bits(buf, 5, 0, 1);
+	e->support_ai	= grab_bits(buf, 5, 1, 1);
+	e->conn_type	= grab_bits(buf, 5, 2, 2);
+	e->sad_count	= grab_bits(buf, 5, 4, 4);
+
+	e->aud_synch_delay = grab_bits(buf, 6, 0, 8) * 2;
+	e->spk_alloc	= grab_bits(buf, 7, 0, 7);
+
+	e->port_id	  = get_unaligned_le64(buf + 8);
+
+	/* not specified, but the spec's tendency is little endian */
+	e->manufacture_id = get_unaligned_le16(buf + 16);
+	e->product_id	  = get_unaligned_le16(buf + 18);
+
+	if (mnl > ELD_MAX_MNL) {
+		snd_printd(KERN_INFO "MNL is reserved value %d\n", mnl);
+		goto out_fail;
+	} else if (ELD_FIXED_BYTES + mnl > size) {
+		snd_printd(KERN_INFO "out of range MNL %d\n", mnl);
+		goto out_fail;
+	} else
+		strlcpy(e->monitor_name, buf + ELD_FIXED_BYTES, mnl);
+
+	for (i = 0; i < e->sad_count; i++) {
+		if (ELD_FIXED_BYTES + mnl + 3 * (i + 1) > size) {
+			snd_printd(KERN_INFO "out of range SAD %d\n", i);
+			goto out_fail;
+		}
+		hdmi_update_short_audio_desc(e->sad + i,
+					buf + ELD_FIXED_BYTES + mnl + 3 * i);
+	}
+
+	return 0;
+
+out_fail:
+	e->eld_ver = 0;
+	return -EINVAL;
+}
+
+static int hdmi_present_sense(struct hda_codec *codec, hda_nid_t nid)
+{
+	return snd_hda_codec_read(codec, nid, 0, AC_VERB_GET_PIN_SENSE, 0);
+}
+
+static int hdmi_eld_valid(struct hda_codec *codec, hda_nid_t nid)
+{
+	int eldv;
+	int present;
+
+	present = hdmi_present_sense(codec, nid);
+	eldv    = (present & AC_PINSENSE_ELDV);
+	present = (present & AC_PINSENSE_PRESENCE);
+
+#ifdef CONFIG_SND_DEBUG_VERBOSE
+	printk(KERN_INFO "pinp = %d, eldv = %d\n", !!present, !!eldv);
+#endif
+
+	return eldv && present;
+}
+
+int hdmi_get_eld_size(struct hda_codec *codec, hda_nid_t nid)
+{
+	return snd_hda_codec_read(codec, nid, 0, AC_VERB_GET_HDMI_DIP_SIZE,
+						 AC_DIPSIZE_ELD_BUF);
+}
+
+int hdmi_get_eld(struct sink_eld *eld,
+		 struct hda_codec *codec, hda_nid_t nid)
+{
+	int i;
+	int ret;
+	int size;
+	unsigned char *buf;
+
+	if (!hdmi_eld_valid(codec, nid))
+		return -ENOENT;
+
+	size = hdmi_get_eld_size(codec, nid);
+	if (size == 0) {
+		/* wfg: workaround for ASUS P5E-VM HDMI board */
+		snd_printd(KERN_INFO "ELD buf size is 0, force 128\n");
+		size = 128;
+	}
+	if (size < ELD_FIXED_BYTES || size > PAGE_SIZE) {
+		snd_printd(KERN_INFO "Invalid ELD buf size %d\n", size);
+		return -ERANGE;
+	}
+
+	buf = kmalloc(size, GFP_KERNEL);
+	if (!buf)
+		return -ENOMEM;
+
+	for (i = 0; i < size; i++)
+		buf[i] = hdmi_get_eld_byte(codec, nid, i);
+
+	ret = hdmi_update_sink_eld(eld, buf, size);
+
+	kfree(buf);
+	return ret;
+}
+
+static void hdmi_show_short_audio_desc(struct cea_sad *a)
+{
+	printk(KERN_INFO "coding type: %s\n",
+					cea_audio_coding_type_names[a->format]);
+	printk(KERN_INFO "channels: %d\n", a->channels);
+	printk(KERN_INFO "sampling frequencies: 0x%x\n", a->rates);
+
+	if (a->format == AUDIO_CODING_TYPE_LPCM)
+		printk(KERN_INFO "sample bits: 0x%x\n", a->sample_bits);
+
+	if (a->max_bitrate)
+		printk(KERN_INFO "max bitrate: %d HZ\n", a->max_bitrate);
+
+	if (a->profile)
+		printk(KERN_INFO "profile: %d\n", a->profile);
+}
+
+static void hdmi_print_speaker_allocations(int spk_alloc, char *buf, int buflen)
+{
+	int i, j;
+
+	for (i = 0, j = 0; i < ARRAY_SIZE(cea_speaker_allocation_names); i++) {
+		if (spk_alloc & (1 << i))
+			j += snprintf(buf + j, buflen - j,  " %s",
+					cea_speaker_allocation_names[i]);
+	}
+	buf[j] = '\0'; /* necessary when j == 0 */
+}
+
+void hdmi_show_eld(struct sink_eld *e)
+{
+	int i;
+	char buf[80];
+
+	printk(KERN_INFO "ELD buffer size  is %d\n", e->eld_size);
+	printk(KERN_INFO "ELD baseline len is %d*4\n", e->baseline_len);
+	printk(KERN_INFO "vendor block len is %d\n",
+					e->eld_size - e->baseline_len * 4 - 4);
+	printk(KERN_INFO "ELD version      is %s\n",
+					eld_versoin_names[e->eld_ver]);
+	printk(KERN_INFO "CEA EDID version is %s\n",
+				cea_edid_version_names[e->cea_edid_ver]);
+	printk(KERN_INFO "manufacture id   is 0x%x\n", e->manufacture_id);
+	printk(KERN_INFO "product id       is 0x%x\n", e->product_id);
+	printk(KERN_INFO "port id          is 0x%llx\n", (long long)e->port_id);
+	printk(KERN_INFO "HDCP support     is %d\n", e->support_hdcp);
+	printk(KERN_INFO "AI support       is %d\n", e->support_ai);
+	printk(KERN_INFO "SAD count        is %d\n", e->sad_count);
+	printk(KERN_INFO "audio sync delay is %x\n", e->aud_synch_delay);
+	printk(KERN_INFO "connection type  is %s\n",
+				eld_connection_type_names[e->conn_type]);
+	printk(KERN_INFO "monitor name     is %s\n", e->monitor_name);
+
+	hdmi_print_speaker_allocations(e->spk_alloc, buf, sizeof(buf));
+	printk(KERN_INFO "speaker allocations: (0x%x)%s\n", e->spk_alloc, buf);
+
+	for (i = 0; i < e->sad_count; i++)
+		hdmi_show_short_audio_desc(e->sad + i);
+}
+
+#ifdef CONFIG_PROC_FS
+
+static void hdmi_print_sad_info(int i, struct cea_sad *a,
+				struct snd_info_buffer *buffer)
+{
+	snd_iprintf(buffer, "sad%d_coding_type\t%s\n",
+			i, cea_audio_coding_type_names[a->format]);
+	snd_iprintf(buffer, "sad%d_channels\t\t%d\n", i, a->channels);
+	snd_iprintf(buffer, "sad%d_sampling_rates\t0x%x\n", i, a->rates);
+
+	if (a->format == AUDIO_CODING_TYPE_LPCM)
+		snd_iprintf(buffer, "sad%d_sample_bits\t0x%x\n", i, a->sample_bits);
+
+	if (a->max_bitrate)
+		snd_iprintf(buffer, "sad%d_max_bitrate\t%d HZ\n",
+							i, a->max_bitrate);
+
+	if (a->profile)
+		snd_iprintf(buffer, "sad%d_profile\t\t%d\n", i, a->profile);
+}
+
+static void hdmi_print_eld_info(struct snd_info_entry *entry,
+				struct snd_info_buffer *buffer)
+{
+	struct sink_eld *e = entry->private_data;
+	char buf[80];
+	int i;
+
+	snd_iprintf(buffer, "monitor name\t\t%s\n", e->monitor_name);
+	snd_iprintf(buffer, "connection_type\t\t%s\n",
+				eld_connection_type_names[e->conn_type]);
+	snd_iprintf(buffer, "eld_version\t\t%d:%s\n", e->eld_ver,
+					eld_versoin_names[e->eld_ver]);
+	snd_iprintf(buffer, "edid_version\t\t%d:%s\n", e->cea_edid_ver,
+				cea_edid_version_names[e->cea_edid_ver]);
+	snd_iprintf(buffer, "manufacture_id\t\t0x%x\n", e->manufacture_id);
+	snd_iprintf(buffer, "product_id\t\t0x%x\n", e->product_id);
+	snd_iprintf(buffer, "port_id\t\t\t0x%llx\n", (long long)e->port_id);
+	snd_iprintf(buffer, "support_hdcp\t\t%d\n", e->support_hdcp);
+	snd_iprintf(buffer, "support_ai\t\t%d\n", e->support_ai);
+	snd_iprintf(buffer, "audio_sync_delay\t%d ms\n", e->aud_synch_delay);
+
+	hdmi_print_speaker_allocations(e->spk_alloc, buf, sizeof(buf));
+	snd_iprintf(buffer, "speakers\t\t%s\n", buf);
+
+	snd_iprintf(buffer, "sad_count\t\t%d\n", e->sad_count);
+
+	for (i = 0; i < e->sad_count; i++)
+		hdmi_print_sad_info(i, e->sad + i, buffer);
+}
+
+int snd_hda_eld_proc_new(struct hda_codec *codec, struct sink_eld *eld)
+{
+	char name[32];
+	struct snd_info_entry *entry;
+	int err;
+
+	snprintf(name, sizeof(name), "eld#%d", codec->addr);
+	err = snd_card_proc_new(codec->bus->card, name, &entry);
+	if (err < 0)
+		return err;
+
+	snd_info_set_text_ops(entry, eld, hdmi_print_eld_info);
+	return 0;
+}
+
+#endif
--- sound-2.6.orig/sound/pci/Kconfig
+++ sound-2.6/sound/pci/Kconfig
@@ -582,6 +582,10 @@ config SND_HDA_CODEC_INTELHDMI
 	  Say Y here to include INTEL HDMI HD-audio codec support in
 	  snd-hda-intel driver, such as Eaglelake integrated HDMI.
 
+config SND_HDA_ELD
+	def_bool y
+	depends on SND_HDA_CODEC_INTELHDMI
+
 config SND_HDA_CODEC_CONEXANT
 	bool "Build Conexant HD-audio codec support"
 	depends on SND_HDA_INTEL
--- sound-2.6.orig/sound/pci/hda/Makefile
+++ sound-2.6/sound/pci/hda/Makefile
@@ -4,6 +4,7 @@ snd-hda-intel-y := hda_intel.o
 # designed to be individual modules
 snd-hda-intel-y += hda_codec.o
 snd-hda-intel-$(CONFIG_PROC_FS) += hda_proc.o
+snd-hda-intel-$(CONFIG_SND_HDA_ELD) += hda_eld.o
 snd-hda-intel-$(CONFIG_SND_HDA_HWDEP) += hda_hwdep.o
 snd-hda-intel-$(CONFIG_SND_HDA_INPUT_BEEP) += hda_beep.o
 snd-hda-intel-$(CONFIG_SND_HDA_GENERIC) += hda_generic.o
--- sound-2.6.orig/sound/pci/hda/hda_local.h
+++ sound-2.6/sound/pci/hda/hda_local.h
@@ -276,15 +276,6 @@ static inline int snd_hda_parse_generic_
 #endif
 
 /*
- * generic proc interface
- */
-#ifdef CONFIG_PROC_FS
-int snd_hda_codec_proc_new(struct hda_codec *codec);
-#else
-static inline int snd_hda_codec_proc_new(struct hda_codec *codec) { return 0; }
-#endif
-
-/*
  * Misc
  */
 int snd_hda_check_board_config(struct hda_codec *codec, int num_configs,
@@ -440,4 +431,57 @@ int snd_hda_check_amp_list_power(struct 
 #define get_amp_direction(kc)	(((kc)->private_value >> 18) & 0x1)
 #define get_amp_index(kc)	(((kc)->private_value >> 19) & 0xf)
 
+/*
+ * CEA Short Audio Descriptor data
+ */
+struct cea_sad {
+	int	channels;
+	int	format;		/* (format == 0) indicates invalid SAD */
+	int	rates;
+	int	sample_bits;	/* for LPCM */
+	int	max_bitrate;	/* for AC3...ATRAC */
+	int	profile;	/* for WMAPRO */
+};
+
+#define ELD_FIXED_BYTES	20
+#define ELD_MAX_MNL	16
+#define ELD_MAX_SAD	16
+
+/*
+ * ELD: EDID Like Data
+ */
+struct sink_eld {
+	int	eld_size;
+	int	baseline_len;
+	int	eld_ver;	/* (eld_ver == 0) indicates invalid ELD */
+	int	cea_edid_ver;
+	char	monitor_name[ELD_MAX_MNL + 1];
+	int	manufacture_id;
+	int	product_id;
+	u64	port_id;
+	int	support_hdcp;
+	int	support_ai;
+	int	conn_type;
+	int	aud_synch_delay;
+	int	spk_alloc;
+	int	sad_count;
+	struct cea_sad sad[ELD_MAX_SAD];
+};
+
+int hdmi_get_eld_size(struct hda_codec *codec, hda_nid_t nid);
+int hdmi_get_eld(struct sink_eld *eld, struct hda_codec *codec, hda_nid_t nid);
+void hdmi_show_eld(struct sink_eld *eld);
+
+/*
+ * generic proc interface
+ */
+#ifdef CONFIG_PROC_FS
+int snd_hda_codec_proc_new(struct hda_codec *codec);
+int snd_hda_eld_proc_new(struct hda_codec *codec, struct sink_eld *eld);
+#else
+static inline int snd_hda_codec_proc_new(struct hda_codec *codec) { return 0; }
+static inline int snd_hda_eld_proc_new(struct hda_codec *codec,
+					struct sink_eld *eld) { return 0; }
+#endif
+
 #endif /* __SOUND_HDA_LOCAL_H */

Re: [RFC][PATCH] ELD routines and proc interface

[Wu Fengguang]

On Thu, Nov 13, 2008 at 10:21:53AM +0800, Wu Fengguang wrote:
> Create hda_eld.c for ELD routines and proc interface.

Here is a simple example of the proc interface, in which the ELD data
structure has not been populated yet:

# cat /proc/asound/card0/eld\#3
monitor name
connection_type         HDMI
eld_version             0:reserved
edid_version            0:no CEA EDID Timing Extension block present
manufacture_id          0x0
product_id              0x0
port_id                 0x0
support_hdcp            0
support_ai              0
audio_sync_delay        0
speakers
sad_count               0

Re: [RFC][PATCH] ELD routines and proc interface

[Takashi Iwai]

At Thu, 13 Nov 2008 10:21:53 +0800,
Wu Fengguang wrote:
> 
> Create hda_eld.c for ELD routines and proc interface.

Thanks for the patch!

> ELD handling routines can be shared by all HDMI codecs,
> and they are large enough to make a standalone source file.
> 
> Some notes on the code:
> 
> - Shall we show an empty file if the ELD content is not valid?
>   Well it's not that simple. There could be partially populated ELD,
>   and there may be malformed ELD provided by buggy drivers/monitors.
>   So I tend to expose ELD as it is. 

Fair enough.

> - hdmi_show_eld() and hdmi_print_eld_info() are somehow duplicated
>   The plan is to shrink hdmi_show_eld()/hdmi_show_short_audio_desc(), and to
>   show a compact message at monitor hotplug and driver initialization time.

You can create a common print function to accept the callback, such
as,

static void hdmi_print(void (*print)(void *, const char *line), void *data,
			   const char *fmt, ...)
{
	char line[80];
	va_list ap;
	va_start(fp, fmt);
	vsnprintf(line, sizeof(line), fmt, ap);
	va_end(ap);
	print(data, line);
}

static void hdmi_print_eld(void (*print)(void *, const char *), void *data,
				struct sink_eld *e)
{
	hdmi_print(print, data, "monitor name\t\t%s\n", e->monitor_name);
	...
}
				
static void print_proc(void *data, const char *line)
{
	snd_iprintf(data, "%s", line);
}

static void hdmi_print_eld_info(struct snd_info_entry *entry,
				struct snd_info_buffer *buffer)
{
	hdmi_print_eld(print_proc, buffer, entry->private_data);
}

static void print_console(void *data, const char *line)
{
	printk(KERN_DEBUG "%s", line);
}

void snd_hdmi_show_eld(struct sink_eld *e)
{
	hdmi_print_eld(print_console, NULL, e);
}

Just an idea, though.


> - The ELD retrieval routines rely on the Intel HDA interface,
>   others are/could be universal and independent ones.
> 
> - The ELD proc interface code could be moved to hda_proc.c if necessary.

> - How do we name the proc file?
>   If there are going to be two HDMI pins per codec, then the current naming
>   scheme (eld#<codec no>) will fail.

In theory, yes, but I don't think this would happen.
If this is needed, the currently existing codec#* proc must be fixed,
too.  So, we can use eld#codec as the simplest way.

Some review comments below:

> --- sound-2.6.orig/sound/pci/hda/patch_intelhdmi.c
> +++ sound-2.6/sound/pci/hda/patch_intelhdmi.c
...
> @@ -880,6 +410,11 @@ static int intel_hdmi_build_controls(str
>  
>  static int intel_hdmi_init(struct hda_codec *codec)
>  {
> +	struct intel_hdmi_spec *spec = codec->spec;
> +	struct sink_eld *eld = &spec->sink;
> +
> +	snd_hda_eld_proc_new(codec, eld);

The proc creation should be rather in patch_intelhdmi().
The init callback is called also at suspend/resume usually.

>  	/* disable audio output as early as possible */
>  	hdmi_disable_output(codec);
>  
> @@ -927,3 +462,4 @@ struct hda_codec_preset snd_hda_preset_i
>  	{ .id = 0x10951392, .name = "SiI1392 HDMI",     .patch = patch_intel_hdmi },
>  	{} /* terminator */
>  };
> +

Useless space addition.


> --- /dev/null
> +++ sound-2.6/sound/pci/hda/hda_eld.c
> +static inline unsigned char grab_bits(const unsigned char *buf,
> +						int byte, int lowbit, int bits)
> +{
> +	BUG_ON(lowbit > 7);
> +	BUG_ON(bits > 8);
> +	BUG_ON(bits <= 0);

Can it be rather BUILD_BUG_ON(), BTW?
Or, hmm, doesn't work if it's an inline function?

> +int hdmi_get_eld_size(struct hda_codec *codec, hda_nid_t nid)

I prefer snd_ prefix for global functions to avoid any potential name
crashes.

> +int hdmi_get_eld(struct sink_eld *eld,
> +		 struct hda_codec *codec, hda_nid_t nid)

Ditto.

> +void hdmi_show_eld(struct sink_eld *e)

Ditto.

> --- sound-2.6.orig/sound/pci/hda/Makefile
> +++ sound-2.6/sound/pci/hda/Makefile
> @@ -4,6 +4,7 @@ snd-hda-intel-y := hda_intel.o
>  # designed to be individual modules
>  snd-hda-intel-y += hda_codec.o
>  snd-hda-intel-$(CONFIG_PROC_FS) += hda_proc.o
> +snd-hda-intel-$(CONFIG_SND_HDA_ELD) += hda_eld.o
>  snd-hda-intel-$(CONFIG_SND_HDA_HWDEP) += hda_hwdep.o
>  snd-hda-intel-$(CONFIG_SND_HDA_INPUT_BEEP) += hda_beep.o
>  snd-hda-intel-$(CONFIG_SND_HDA_GENERIC) += hda_generic.o
> --- sound-2.6.orig/sound/pci/hda/hda_local.h
> +++ sound-2.6/sound/pci/hda/hda_local.h
> @@ -276,15 +276,6 @@ static inline int snd_hda_parse_generic_
>  #endif
>  
>  /*
> - * generic proc interface
> - */
> -#ifdef CONFIG_PROC_FS
> -int snd_hda_codec_proc_new(struct hda_codec *codec);
> -#else
> -static inline int snd_hda_codec_proc_new(struct hda_codec *codec) { return 0; }
> -#endif
...
> +/*
> + * generic proc interface
> + */
> +#ifdef CONFIG_PROC_FS
> +int snd_hda_codec_proc_new(struct hda_codec *codec);
> +int snd_hda_eld_proc_new(struct hda_codec *codec, struct sink_eld *eld);

I'd put these two declarations rather separately, since
snd_hda_proc_new() has nothing to do with ELD.  Keep it in the
original place.


thanks,

Takashi

Re: [RFC][PATCH] ELD routines and proc interface

[Wu Fengguang]

On Thu, Nov 13, 2008 at 08:26:48AM +0100, Takashi Iwai wrote:
> At Thu, 13 Nov 2008 10:21:53 +0800,
> Wu Fengguang wrote:
> > 
> > Create hda_eld.c for ELD routines and proc interface.
> 
> Thanks for the patch!

I take it as my responsibility :)

> > ELD handling routines can be shared by all HDMI codecs,
> > and they are large enough to make a standalone source file.
> > 
> > Some notes on the code:
> > 
> > - Shall we show an empty file if the ELD content is not valid?
> >   Well it's not that simple. There could be partially populated ELD,
> >   and there may be malformed ELD provided by buggy drivers/monitors.
> >   So I tend to expose ELD as it is. 
> 
> Fair enough.
> 
> > - hdmi_show_eld() and hdmi_print_eld_info() are somehow duplicated
> >   The plan is to shrink hdmi_show_eld()/hdmi_show_short_audio_desc(), and to
> >   show a compact message at monitor hotplug and driver initialization time.
> 
> You can create a common print function to accept the callback, such
> as,
> 
> static void hdmi_print(void (*print)(void *, const char *line), void *data,
> 			   const char *fmt, ...)
> {
> 	char line[80];
> 	va_list ap;
> 	va_start(fp, fmt);
> 	vsnprintf(line, sizeof(line), fmt, ap);
> 	va_end(ap);
> 	print(data, line);
> }
> 
> static void hdmi_print_eld(void (*print)(void *, const char *), void *data,
> 				struct sink_eld *e)
> {
> 	hdmi_print(print, data, "monitor name\t\t%s\n", e->monitor_name);
> 	...
> }
> 				
> static void print_proc(void *data, const char *line)
> {
> 	snd_iprintf(data, "%s", line);
> }
> 
> static void hdmi_print_eld_info(struct snd_info_entry *entry,
> 				struct snd_info_buffer *buffer)
> {
> 	hdmi_print_eld(print_proc, buffer, entry->private_data);
> }
> 
> static void print_console(void *data, const char *line)
> {
> 	printk(KERN_DEBUG "%s", line);
> }
> 
> void snd_hdmi_show_eld(struct sink_eld *e)
> {
> 	hdmi_print_eld(print_console, NULL, e);
> }
> 
> Just an idea, though.
 
Wow, it would be really nice trick for a big project!
But in this case, it seems to create more code than it can reduce...

Anyway my point is that, there seems to pointless to show full
detailed ELD info in dmesg.

> > - The ELD retrieval routines rely on the Intel HDA interface,
> >   others are/could be universal and independent ones.
> > 
> > - The ELD proc interface code could be moved to hda_proc.c if necessary.
> 
> > - How do we name the proc file?
> >   If there are going to be two HDMI pins per codec, then the current naming
> >   scheme (eld#<codec no>) will fail.
> 
> In theory, yes, but I don't think this would happen.
> If this is needed, the currently existing codec#* proc must be fixed,
> too.  So, we can use eld#codec as the simplest way.

I mean one HDMI codec equipped with two output converters and two HDMI pins.
In this case there could be two HDMI sinks mapped to one single codec.

Or it would be trivial to do the rename in the future anyway?

> Some review comments below:
> 
> > --- sound-2.6.orig/sound/pci/hda/patch_intelhdmi.c
> > +++ sound-2.6/sound/pci/hda/patch_intelhdmi.c
> ...
> > @@ -880,6 +410,11 @@ static int intel_hdmi_build_controls(str
> >  
> >  static int intel_hdmi_init(struct hda_codec *codec)
> >  {
> > +	struct intel_hdmi_spec *spec = codec->spec;
> > +	struct sink_eld *eld = &spec->sink;
> > +
> > +	snd_hda_eld_proc_new(codec, eld);
> 
> The proc creation should be rather in patch_intelhdmi().
> The init callback is called also at suspend/resume usually.

OK, fixed.

> >  	/* disable audio output as early as possible */
> >  	hdmi_disable_output(codec);
> >  
> > @@ -927,3 +462,4 @@ struct hda_codec_preset snd_hda_preset_i
> >  	{ .id = 0x10951392, .name = "SiI1392 HDMI",     .patch = patch_intel_hdmi },
> >  	{} /* terminator */
> >  };
> > +
> 
> Useless space addition.

Fixed, wondering how it slipped in.

> 
> > --- /dev/null
> > +++ sound-2.6/sound/pci/hda/hda_eld.c
> > +static inline unsigned char grab_bits(const unsigned char *buf,
> > +						int byte, int lowbit, int bits)
> > +{
> > +	BUG_ON(lowbit > 7);
> > +	BUG_ON(bits > 8);
> > +	BUG_ON(bits <= 0);
> 
> Can it be rather BUILD_BUG_ON(), BTW?
> Or, hmm, doesn't work if it's an inline function?

Yes, converted to BUILD_BUG_ON() and it compiles OK.

> > +int hdmi_get_eld_size(struct hda_codec *codec, hda_nid_t nid)
> 
> I prefer snd_ prefix for global functions to avoid any potential name
> crashes.

OK.

> > +int hdmi_get_eld(struct sink_eld *eld,
> > +		 struct hda_codec *codec, hda_nid_t nid)
> 
> Ditto.
> 
> > +void hdmi_show_eld(struct sink_eld *e)
> 
> Ditto.

Three renames done.

> > --- sound-2.6.orig/sound/pci/hda/Makefile
> > +++ sound-2.6/sound/pci/hda/Makefile
> > @@ -4,6 +4,7 @@ snd-hda-intel-y := hda_intel.o
> >  # designed to be individual modules
> >  snd-hda-intel-y += hda_codec.o
> >  snd-hda-intel-$(CONFIG_PROC_FS) += hda_proc.o
> > +snd-hda-intel-$(CONFIG_SND_HDA_ELD) += hda_eld.o
> >  snd-hda-intel-$(CONFIG_SND_HDA_HWDEP) += hda_hwdep.o
> >  snd-hda-intel-$(CONFIG_SND_HDA_INPUT_BEEP) += hda_beep.o
> >  snd-hda-intel-$(CONFIG_SND_HDA_GENERIC) += hda_generic.o
> > --- sound-2.6.orig/sound/pci/hda/hda_local.h
> > +++ sound-2.6/sound/pci/hda/hda_local.h
> > @@ -276,15 +276,6 @@ static inline int snd_hda_parse_generic_
> >  #endif
> >  
> >  /*
> > - * generic proc interface
> > - */
> > -#ifdef CONFIG_PROC_FS
> > -int snd_hda_codec_proc_new(struct hda_codec *codec);
> > -#else
> > -static inline int snd_hda_codec_proc_new(struct hda_codec *codec) { return 0; }
> > -#endif
> ...
> > +/*
> > + * generic proc interface
> > + */
> > +#ifdef CONFIG_PROC_FS
> > +int snd_hda_codec_proc_new(struct hda_codec *codec);
> > +int snd_hda_eld_proc_new(struct hda_codec *codec, struct sink_eld *eld);
> 
> I'd put these two declarations rather separately, since
> snd_hda_proc_new() has nothing to do with ELD.  Keep it in the
> original place.

OK.

Thank you,
Fengguang

Re: [RFC][PATCH] ELD routines and proc interface

[Takashi Iwai]

At Fri, 14 Nov 2008 09:34:32 +0800,
Wu Fengguang wrote:
> 
> > > - How do we name the proc file?
> > >   If there are going to be two HDMI pins per codec, then the current naming
> > >   scheme (eld#<codec no>) will fail.
> > 
> > In theory, yes, but I don't think this would happen.
> > If this is needed, the currently existing codec#* proc must be fixed,
> > too.  So, we can use eld#codec as the simplest way.
> 
> I mean one HDMI codec equipped with two output converters and two HDMI pins.
> In this case there could be two HDMI sinks mapped to one single codec.
> 
> Or it would be trivial to do the rename in the future anyway?

Ah, OK, understood.  One easy solution is to name the proc file with
either pin of audio-out widget NID.

But I'm not sure whether it's worth.  The proc file naming isn't
strict, so I'd leave it as is.


> > > --- /dev/null
> > > +++ sound-2.6/sound/pci/hda/hda_eld.c
> > > +static inline unsigned char grab_bits(const unsigned char *buf,
> > > +						int byte, int lowbit, int bits)
> > > +{
> > > +	BUG_ON(lowbit > 7);
> > > +	BUG_ON(bits > 8);
> > > +	BUG_ON(bits <= 0);
> > 
> > Can it be rather BUILD_BUG_ON(), BTW?
> > Or, hmm, doesn't work if it's an inline function?
> 
> Yes, converted to BUILD_BUG_ON() and it compiles OK.

The question is whether this really triggers the build error
properly.  Could you check it, simply by changing the caller of
grab_bits() with some invalid values?  Then you should get a compile
error.


thanks,

Takashi

Re: [RFC][PATCH] ELD routines and proc interface

[Wu Fengguang]

On Fri, Nov 14, 2008 at 08:25:51AM +0100, Takashi Iwai wrote:
> At Fri, 14 Nov 2008 09:34:32 +0800,
> Wu Fengguang wrote:
> > 
> > > > - How do we name the proc file?
> > > >   If there are going to be two HDMI pins per codec, then the current naming
> > > >   scheme (eld#<codec no>) will fail.
> > > 
> > > In theory, yes, but I don't think this would happen.
> > > If this is needed, the currently existing codec#* proc must be fixed,
> > > too.  So, we can use eld#codec as the simplest way.
> > 
> > I mean one HDMI codec equipped with two output converters and two HDMI pins.
> > In this case there could be two HDMI sinks mapped to one single codec.
> > 
> > Or it would be trivial to do the rename in the future anyway?
> 
> Ah, OK, understood.  One easy solution is to name the proc file with
> either pin of audio-out widget NID.
> 
> But I'm not sure whether it's worth.  The proc file naming isn't
> strict, so I'd leave it as is.

OK, I'll leave it as is: there should be no many dependencies on it.

> > > > --- /dev/null
> > > > +++ sound-2.6/sound/pci/hda/hda_eld.c
> > > > +static inline unsigned char grab_bits(const unsigned char *buf,
> > > > +						int byte, int lowbit, int bits)
> > > > +{
> > > > +	BUG_ON(lowbit > 7);
> > > > +	BUG_ON(bits > 8);
> > > > +	BUG_ON(bits <= 0);
> > > 
> > > Can it be rather BUILD_BUG_ON(), BTW?
> > > Or, hmm, doesn't work if it's an inline function?
> > 
> > Yes, converted to BUILD_BUG_ON() and it compiles OK.
> 
> The question is whether this really triggers the build error
> properly.  Could you check it, simply by changing the caller of
> grab_bits() with some invalid values?  Then you should get a compile
> error.

BUILD_BUG_ON() won't emit errors! So use BUG_ON()?

Thank you,
Fengguang

Re: [RFC][PATCH] ELD routines and proc interface

[Takashi Iwai]

At Fri, 14 Nov 2008 15:38:56 +0800,
Wu Fengguang wrote:
> 
> > > > > --- /dev/null
> > > > > +++ sound-2.6/sound/pci/hda/hda_eld.c
> > > > > +static inline unsigned char grab_bits(const unsigned char *buf,
> > > > > +						int byte, int lowbit, int bits)
> > > > > +{
> > > > > +	BUG_ON(lowbit > 7);
> > > > > +	BUG_ON(bits > 8);
> > > > > +	BUG_ON(bits <= 0);
> > > > 
> > > > Can it be rather BUILD_BUG_ON(), BTW?
> > > > Or, hmm, doesn't work if it's an inline function?
> > > 
> > > Yes, converted to BUILD_BUG_ON() and it compiles OK.
> > 
> > The question is whether this really triggers the build error
> > properly.  Could you check it, simply by changing the caller of
> > grab_bits() with some invalid values?  Then you should get a compile
> > error.
> 
> BUILD_BUG_ON() won't emit errors! So use BUG_ON()?

Try to make grab_bits() a macro and check whether BUILD_BUG_ON()
works.  I think it won't be too bad to use a macro for such a pretty
simple case.  If the resultant code looks too ugly, we should switch
back to BUG_ON().

The difference is that BUILD_BUG_ON() would add no real code while
BUG_ON() is a pure run-time check.


thanks,

Takashi

Re: [RFC][PATCH] ELD routines and proc interface

[Wu Fengguang]

On Fri, Nov 14, 2008 at 08:43:59AM +0100, Takashi Iwai wrote:
> At Fri, 14 Nov 2008 15:38:56 +0800,
> Wu Fengguang wrote:
> > 
> > > > > > --- /dev/null
> > > > > > +++ sound-2.6/sound/pci/hda/hda_eld.c
> > > > > > +static inline unsigned char grab_bits(const unsigned char *buf,
> > > > > > +						int byte, int lowbit, int bits)
> > > > > > +{
> > > > > > +	BUG_ON(lowbit > 7);
> > > > > > +	BUG_ON(bits > 8);
> > > > > > +	BUG_ON(bits <= 0);
> > > > > 
> > > > > Can it be rather BUILD_BUG_ON(), BTW?
> > > > > Or, hmm, doesn't work if it's an inline function?
> > > > 
> > > > Yes, converted to BUILD_BUG_ON() and it compiles OK.
> > > 
> > > The question is whether this really triggers the build error
> > > properly.  Could you check it, simply by changing the caller of
> > > grab_bits() with some invalid values?  Then you should get a compile
> > > error.
> > 
> > BUILD_BUG_ON() won't emit errors! So use BUG_ON()?
> 
> Try to make grab_bits() a macro and check whether BUILD_BUG_ON()
> works.  I think it won't be too bad to use a macro for such a pretty
> simple case.  If the resultant code looks too ugly, we should switch
> back to BUG_ON().

OK, I'm fine with a macro.

> The difference is that BUILD_BUG_ON() would add no real code while
> BUG_ON() is a pure run-time check.

But the code should be optimize away by gcc when the constant
expression is false? 

Thanks,
Fengguang

Re: [RFC][PATCH] ELD routines and proc interface

[Takashi Iwai]

At Fri, 14 Nov 2008 15:47:37 +0800,
Wu Fengguang wrote:
> 
> On Fri, Nov 14, 2008 at 08:43:59AM +0100, Takashi Iwai wrote:
> > At Fri, 14 Nov 2008 15:38:56 +0800,
> > Wu Fengguang wrote:
> > > 
> > > > > > > --- /dev/null
> > > > > > > +++ sound-2.6/sound/pci/hda/hda_eld.c
> > > > > > > +static inline unsigned char grab_bits(const unsigned char *buf,
> > > > > > > +						int byte, int lowbit, int bits)
> > > > > > > +{
> > > > > > > +	BUG_ON(lowbit > 7);
> > > > > > > +	BUG_ON(bits > 8);
> > > > > > > +	BUG_ON(bits <= 0);
> > > > > > 
> > > > > > Can it be rather BUILD_BUG_ON(), BTW?
> > > > > > Or, hmm, doesn't work if it's an inline function?
> > > > > 
> > > > > Yes, converted to BUILD_BUG_ON() and it compiles OK.
> > > > 
> > > > The question is whether this really triggers the build error
> > > > properly.  Could you check it, simply by changing the caller of
> > > > grab_bits() with some invalid values?  Then you should get a compile
> > > > error.
> > > 
> > > BUILD_BUG_ON() won't emit errors! So use BUG_ON()?
> > 
> > Try to make grab_bits() a macro and check whether BUILD_BUG_ON()
> > works.  I think it won't be too bad to use a macro for such a pretty
> > simple case.  If the resultant code looks too ugly, we should switch
> > back to BUG_ON().
> 
> OK, I'm fine with a macro.
> 
> > The difference is that BUILD_BUG_ON() would add no real code while
> > BUG_ON() is a pure run-time check.
> 
> But the code should be optimize away by gcc when the constant
> expression is false? 

Well, I think BUG_ON() code remains in this case because it's no
constant check as it's in a function (although inlined).  Otherwise
BUILD_BUG_ON() should have worked.


Takashi

Re: [RFC][PATCH] ELD routines and proc interface

[Wu Fengguang]

On Fri, Nov 14, 2008 at 08:50:44AM +0100, Takashi Iwai wrote:
> At Fri, 14 Nov 2008 15:47:37 +0800,
> Wu Fengguang wrote:
> > 
> > On Fri, Nov 14, 2008 at 08:43:59AM +0100, Takashi Iwai wrote:
> > > At Fri, 14 Nov 2008 15:38:56 +0800,
> > > Wu Fengguang wrote:
> > > > 
> > > > > > > > --- /dev/null
> > > > > > > > +++ sound-2.6/sound/pci/hda/hda_eld.c
> > > > > > > > +static inline unsigned char grab_bits(const unsigned char *buf,
> > > > > > > > +						int byte, int lowbit, int bits)
> > > > > > > > +{
> > > > > > > > +	BUG_ON(lowbit > 7);
> > > > > > > > +	BUG_ON(bits > 8);
> > > > > > > > +	BUG_ON(bits <= 0);
> > > > > > > 
> > > > > > > Can it be rather BUILD_BUG_ON(), BTW?
> > > > > > > Or, hmm, doesn't work if it's an inline function?
> > > > > > 
> > > > > > Yes, converted to BUILD_BUG_ON() and it compiles OK.
> > > > > 
> > > > > The question is whether this really triggers the build error
> > > > > properly.  Could you check it, simply by changing the caller of
> > > > > grab_bits() with some invalid values?  Then you should get a compile
> > > > > error.
> > > > 
> > > > BUILD_BUG_ON() won't emit errors! So use BUG_ON()?
> > > 
> > > Try to make grab_bits() a macro and check whether BUILD_BUG_ON()
> > > works.  I think it won't be too bad to use a macro for such a pretty
> > > simple case.  If the resultant code looks too ugly, we should switch
> > > back to BUG_ON().
> > 
> > OK, I'm fine with a macro.
> > 
> > > The difference is that BUILD_BUG_ON() would add no real code while
> > > BUG_ON() is a pure run-time check.
> > 
> > But the code should be optimize away by gcc when the constant
> > expression is false? 
> 
> Well, I think BUG_ON() code remains in this case because it's no
> constant check as it's in a function (although inlined).  Otherwise
> BUILD_BUG_ON() should have worked.


Hehe, not so ugly as a macro:

#define GRAB_BITS(buf, byte, lowbit, bits)              \
({                                                      \
        BUILD_BUG_ON(lowbit > 7);                       \
        BUILD_BUG_ON(bits > 8);                         \
        BUILD_BUG_ON(bits <= 0);                        \
                                                        \
        (buf[byte] >> (lowbit)) & ((1 << (bits)) - 1);  \
})


Cheers,
Fengguang

Re: [RFC][PATCH] ELD routines and proc interface

[Wu Fengguang]

On Tue, Nov 18, 2008 at 11:11:35PM -0800, Shane W wrote:
> On Thu, Nov 13, 2008 at 10:21:53AM +0800, Wu Fengguang wrote:
> > Create hda_eld.c for ELD routines and proc interface.
> 
> Hi,

Hi Shane,

> Just testing this and your multichannel HDMI code over here
> but it doesn't seem to get ELD data for my setup.  Intel
> g45-id board with an Yamaha RX-v1800 receiver which is HDMI
> 1.3 capable.

Do you run the latest xorg intel driver?  Also dmesg should tell us
something if anything goes wrong with ELD retrieving/parsing.

> shane@chintoka:~$ cat /proc/asound/Intel/eld#3
> monitor name
> connection_type         HDMI
> eld_version             [0x0] reserved
> edid_version            [0x0] no CEA EDID Timing Extension
> block present
> manufacture_id          0x0
> product_id              0x0
> port_id                 0x0
> support_hdcp            0
> support_ai              0
> audio_sync_delay        0
> speakers                [0x0]
> sad_count               0
> 
> 2ch audio is coming through just fine over HDMI though so
> the connection itself is there.

Yes. Obviously the ELD stuct has not been populated yet.

Thanks,
Fengguang

Re: [RFC][PATCH] ELD routines and proc interface

[Wu Fengguang]

On Tue, Nov 18, 2008 at 11:55:45PM -0800, Shane W wrote:
> On Wed, Nov 19, 2008 at 03:17:57PM +0800, Wu Fengguang wrote:
> > On Tue, Nov 18, 2008 at 11:11:35PM -0800, Shane W wrote:
> > > On Thu, Nov 13, 2008 at 10:21:53AM +0800, Wu Fengguang wrote:
> > > > Create hda_eld.c for ELD routines and proc interface.
> > 
> > > Just testing this and your multichannel HDMI code over here
> > > but it doesn't seem to get ELD data for my setup.  Intel
> > > g45-id board with an Yamaha RX-v1800 receiver which is HDMI
> > > 1.3 capable.
> > 
> > Do you run the latest xorg intel driver?  Also dmesg should tell us
> > something if anything goes wrong with ELD retrieving/parsing.
> 
> Yes though Xorg wasn't running when I was doing the audio
> tests.  Pure console at that point but I fired up Xorg
> which is latest git as of today but still no ELD

Yeah sorry I missed one note. The xorg git tree only contains the
audio output enabling patches, but not yet ELD transferring patches.
(the ELD data flow is monitor => video driver => audio driver)
I called for the Xorg ELD enabling patches from Ma Ling just after
replying to your email. He should send the patches soon.

> population.  When Xorg starts, I get this in dmesg:
> [drm] Initialized i915 1.6.0 20080730 on minor 0
> [drm:i915_gem_object_bind_to_gtt] *ERROR* GTT full, but LRU
> list empty
> [drm:i915_gem_object_pin] *ERROR* Failure to bind:

> -12<6>HDMI intrinsic event: PD=1 ELDV=1

This last dmesg line is funny. The ELDV(i.e. ELD Valid) is true,
but the video driver obviously has no ELD data prepared for us.

Thanks,
Fengguang

Re: [RFC][PATCH] ELD routines and proc interface

[Wu Fengguang]

Hi Shane,

On Thu, Nov 20, 2008 at 12:06:06PM -0800, Shane W wrote:
> Hi,
> 
> Ok, it appears to be the kernel I was using.  Going to a
> 2.6.27 kernel rather than 2.6.28-rc fixed the segfault.  So
> now I get ELD data though 5.1 PCM isn't quite there yet. 
> Here's the ELD info.

Thanks.

> HDMI intrinsic event: PD=1 ELDV=1
> detected monitor Haier TV*** at connection type HDMI
> supports coding type LPCM: channels = 2, rates = 44100
> 48000 88200 176400 192000 384000, bits =
> supports coding type LPCM: channels = 8, rates = 44100
> 48000 88200 176400 192000 384000, bits =

It reveals bugs in showing the bits field. Will fix it in a following
email.

> supports coding type AC-3: channels = 6, rates = 44100
> 48000 88200, max bitrate = 640000
> supports coding type DTS: channels = 7, rates = 44100 48000
> 88200 176400 192000, max bitrate = 1536000
> supports coding type DSD (One Bit Audio): channels = 6,
> rates = 48000

It's weird that DTS supports 7 channels while DSD supports 6.
DTS is simple the compressed form of DSD.

> set status page addr 0x08360000
> [drm:i915_getparam] *ERROR* Unknown parameter 5
> 
> shane@chintoka:~$ cat /proc/asound/Intel/eld#3
> monitor name            Haier TV***
> connection_type         HDMI
> eld_version             [0x2] CEA-861D or below
> edid_version            [0x1] CEA-861
> manufacture_id          0x4148
> product_id              0xb01
> port_id                 0x0
> support_hdcp            0
> support_ai              0
> audio_sync_delay        0
> speakers                [0x0]
> sad_count               8
> sad0_coding_type        [0x1] LPCM
> sad0_channels           2
> sad0_rates              [0x1ee0] 44100 48000 88200 176400
> 192000 384000
> sad0_bits               [0xe0000]
> sad1_coding_type        [0x1] LPCM
> sad1_channels           8
> sad1_rates              [0x1ee0] 44100 48000 88200 176400
> 192000 384000
> sad1_bits               [0xe0000]
> sad2_coding_type        [0x2] AC-3
> sad2_channels           6
> sad2_rates              [0xe0] 44100 48000 88200
> sad2_max_bitrate        640000
> sad3_coding_type        [0x7] DTS
> sad3_channels           7
> sad3_rates              [0x6e0] 44100 48000 88200 176400
> 192000
> sad3_max_bitrate        1536000
> sad4_coding_type        [0x9] DSD (One Bit Audio)
> sad4_channels           6
> sad4_rates              [0x40] 48000

> sad5_coding_type        [0x0] undefined
> sad5_channels           1
> sad5_rates              [0x0]
> sad6_coding_type        [0x0] undefined
> sad6_channels           1
> sad6_rates              [0x0]
> sad7_coding_type        [0x0] undefined
> sad7_channels           1
> sad7_rates              [0x0]

I wonder why the monitor report these zero valued SADs?

> The speakers 0 line is a bit confusing, not sure if that's

Yes it's unexpected. Do you know its real speaker numbers and
allocations? Does it provide some number of line-out ports?

> what's doing it.  I am using:
> aplay 51test.wav
> 
> which I have put here:
> http://www.csy.ca/~shane/51test.wav

I hear only "front left" and "front right" in my T61 :-)

Thanks,
Fengguang
---

> 
> Shane
> On Thu, Nov 20, 2008 at 09:02:04AM +0800, Wu Fengguang wrote:
> > Thanks Shane!
> > 
> > Ma Ling: do you have a quick fix for it?  Or shall I look into it?
> > 
> > Thank you,
> > Fengguang
> > 
> > On Wed, Nov 19, 2008 at 12:02:01PM -0800, Shane W wrote:
> > > On Wed, Nov 19, 2008 at 05:39:40PM +0800, Wu Fengguang wrote:
> > > > I managed to update the last working Xorg ELD patches to the latest
> > > > git tree, and they compile flawlessly. The attached 2 patches are for 
> > > > xf86-video-intel/ and xserver/ respectively.
> > > 
> > > They do compile but I can't get them to run.  Here's the
> > > Xorg backtrace when starting.  I'm having trouble
> > > generating a core but I'll work on getting gdb output if
> > > needed.
> > > 
> > > Backtrace:
> > > 0: /opt/gfx-test/bin/Xorg(xorg_backtrace+0x26) [0x4ebbb6]
> > > 1: /opt/gfx-test/bin/Xorg(xf86SigHandler+0x39) [0x495b49]
> > > 2: /lib/libc.so.6 [0x7f2d1648bf60]
> > > 3:
> > > /opt/gfx-test/lib/xorg/modules/drivers//intel_drv.so(gen4_render_state_cleanup+0x1c)
> > > [0x7f2d14ebab9c]
> > > 4: /opt/gfx-test/lib/xorg/modules/drivers//intel_drv.so
> > > [0x7f2d14e92dfd]
> > > 5: /opt/gfx-test/bin/Xorg(AbortDDX+0x8d) [0x4690fd]
> > > 6: /opt/gfx-test/bin/Xorg(AbortServer+0x1d) [0x4f505d]
> > > 7: /opt/gfx-test/bin/Xorg(LogVMessageVerb+0) [0x4f5700]
> > > 8:
> > > /opt/gfx-test/lib/xorg/modules/drivers//intel_drv.so(i830_bind_all_memory+0x102)
> > > [0x7f2d14e99592]
> > > 9: /opt/gfx-test/lib/xorg/modules/drivers//intel_drv.so
> > > [0x7f2d14e93ad0]
> > > 10: /opt/gfx-test/lib/xorg/modules/drivers//intel_drv.so
> > > [0x7f2d14e950f8]
> > > 11: /opt/gfx-test/bin/Xorg(AddScreen+0x1c6) [0x431086]
> > > 12: /opt/gfx-test/bin/Xorg(InitOutput+0x241) [0x469ae1]
> > > 13: /opt/gfx-test/bin/Xorg(main+0x205) [0x431795]
> > > 14: /lib/libc.so.6(__libc_start_main+0xe6) [0x7f2d164781a6]
> > > 15: /opt/gfx-test/bin/Xorg [0x430dd9]
> > > 
> > > It seems to be the xserver patch that's doing it.  When they
> > > are both unapplied, X starts fine, when the Xserver patch
> > > is applied, we segfault, when the intel is applied alone,
> > > it starts fine and when both are applied the segfault
> > > returns.
> > > 
> > > Shane