Skip to contentSkip to conversation

ELD routines and proc interface

RFC PATCH In review Wu Fengguang 2 participants 19 messages Nov 13, 2008 – Nov 21, 2008 6 messages not in the archive Sound

lore
Share & tools
lore
#0 lore raw

RFC PATCH 1/1 ELD routines and proc interface

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.

---
 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/pci/hda/patch_intelhdmi.c +10 -474
@@ -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 */
 };
+
sound/pci/hda/hda_eld.c new file +532 -0

532 lines not on this page · raw message

sound/pci/Kconfig +4 -0
@@ -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/pci/hda/Makefile +1 -0
@@ -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/pci/hda/hda_local.h +53 -9

72 lines not on this page · raw message

#1 lore raw

At Thu, 13 Nov 2008 10:21:53 +0800,

Quoted 2 lines · Wu Fengguang

Create hda_eld.c for ELD routines and proc interface.

Thanks for the patch!

Quoted 9 lines

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.

Quoted 3 lines

- 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.

Quoted 8 lines

- 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:

Quoted 2 lines
sound/pci/hda/patch_intelhdmi.c +0 -0

...

sound/pci/hda/patch_intelhdmi.c static int intel_hdmi_build_controls(str · line 416

Quoted 8 lines
sound/pci/hda/patch_intelhdmi.c +4 -0
@@ -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.

sound/pci/hda/patch_intelhdmi.c struct hda_codec_preset snd_hda_preset_i · line 465

Quoted 8 lines
sound/pci/hda/patch_intelhdmi.c +1 -0
 	/* 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.

sound/pci/hda/hda_eld.c line 204

Quoted 8 lines
sound/pci/hda/hda_eld.c new file +6 -0
+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?

Quoted 1 line

+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.

sound/pci/hda/hda_eld.c line 372

Quoted 2 lines
sound/pci/hda/hda_eld.c new file +2 -0
+int hdmi_get_eld(struct sink_eld *eld,
+		 struct hda_codec *codec, hda_nid_t nid)

Ditto.

Quoted 1 line

+void hdmi_show_eld(struct sink_eld *e)

Ditto.

Quoted 23 lines
sound/pci/hda/Makefile +1 -0
@@ -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/pci/hda/hda_local.h +0 -7
@@ -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

...

sound/pci/hda/hda_local.h int snd_hda_check_amp_list_power(struct · line 480

Quoted 6 lines
sound/pci/hda/hda_local.h +6 -0
+/*
+ * 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

#2 lore raw
Quoted 6 lines · Takashi Iwai

At Thu, 13 Nov 2008 10:21:53 +0800,

Quoted 2 lines · Wu Fengguang

Create hda_eld.c for ELD routines and proc interface.

Thanks for the patch!

I take it as my responsibility :)

Quoted 53 lines
Quoted 9 lines

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.

Quoted 3 lines

- 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);
}
Quoted 6 lines

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.

Quoted 5 lines
Quoted 4 lines

- 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.

Quoted 7 lines
Quoted 3 lines

- 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?

Quoted 7 lines

Some review comments below:

Quoted 2 lines
sound/pci/hda/patch_intelhdmi.c +0 -0

...

Quoted 2 lines
sound/pci/hda/patch_intelhdmi.c +0 -0
@@ -880,6 +410,11 @@ static int intel_hdmi_build_controls(str
 
Quoted 9 lines

sound/pci/hda/patch_intelhdmi.c static int intel_hdmi_build_controls(str · line 416

Quoted 6 lines
sound/pci/hda/patch_intelhdmi.c +4 -0
 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.

Quoted 10 lines

sound/pci/hda/patch_intelhdmi.c struct hda_codec_preset snd_hda_preset_i · line 465

Quoted 8 lines
sound/pci/hda/patch_intelhdmi.c +1 -0
 	/* 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.

Quoted 12 lines

sound/pci/hda/hda_eld.c line 204

Quoted 9 lines
sound/pci/hda/hda_eld.c new file +6 -0
+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.

Quoted 4 lines
Quoted 1 line

+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.

Quoted 8 lines

sound/pci/hda/hda_eld.c line 372

Quoted 2 lines
sound/pci/hda/hda_eld.c new file +2 -0
+int hdmi_get_eld(struct sink_eld *eld,
+		 struct hda_codec *codec, hda_nid_t nid)

Ditto.

Quoted 1 line

+void hdmi_show_eld(struct sink_eld *e)

Ditto.

Three renames done.

Quoted 24 lines
Quoted 23 lines
sound/pci/hda/Makefile +1 -0
@@ -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/pci/hda/hda_local.h +0 -7
@@ -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

...

Quoted 10 lines

sound/pci/hda/hda_local.h int snd_hda_check_amp_list_power(struct · line 480

Quoted 6 lines
sound/pci/hda/hda_local.h +6 -0
+/*
+ * 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

#3 lore raw

At Fri, 14 Nov 2008 09:34:32 +0800,

Quoted 5 lines · Wu Fengguang
Quoted 5 lines
Quoted 4 lines

- 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.

Quoted 8 lines
Quoted 3 lines

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.

Quoted 12 lines
Quoted 11 lines

sound/pci/hda/hda_eld.c line 204

Quoted 8 lines
sound/pci/hda/hda_eld.c new file +6 -0
+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?

Quoted 1 line

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

#4 lore raw
Quoted 14 lines · Takashi Iwai

At Fri, 14 Nov 2008 09:34:32 +0800,

Quoted 12 lines · Wu Fengguang
Quoted 8 lines
Quoted 4 lines

- 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.

Quoted 7 lines
Quoted 1 line

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.

Quoted 12 lines
Quoted 12 lines
Quoted 11 lines

sound/pci/hda/hda_eld.c line 204

Quoted 8 lines
sound/pci/hda/hda_eld.c new file +6 -0
+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?

Quoted 6 lines
Quoted 1 line

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

#5 lore raw

At Fri, 14 Nov 2008 15:38:56 +0800,

Quoted 13 lines · Wu Fengguang
Quoted 13 lines
Quoted 13 lines
Quoted 12 lines

sound/pci/hda/hda_eld.c line 204

Quoted 9 lines
sound/pci/hda/hda_eld.c new file +6 -0
+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?

Quoted 8 lines
Quoted 6 lines
Quoted 1 line

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

#6 lore raw
Quoted 22 lines · Takashi Iwai

At Fri, 14 Nov 2008 15:38:56 +0800,

Quoted 20 lines · Wu Fengguang
Quoted 19 lines
Quoted 13 lines
Quoted 12 lines

sound/pci/hda/hda_eld.c line 204

Quoted 9 lines
sound/pci/hda/hda_eld.c new file +6 -0
+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?

Quoted 1 line

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.

Quoted 6 lines
Quoted 1 line

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.

Quoted 2 lines

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

#7 lore raw

At Fri, 14 Nov 2008 15:47:37 +0800,

Quoted 31 lines · Wu Fengguang
Quoted 22 lines · Takashi Iwai

At Fri, 14 Nov 2008 15:38:56 +0800,

Quoted 20 lines · Wu Fengguang
Quoted 19 lines
Quoted 13 lines
Quoted 12 lines

sound/pci/hda/hda_eld.c

Quoted 9 lines
sound/pci/hda/hda_eld.c new file +6 -0
+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?

Quoted 1 line

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.

Quoted 6 lines
Quoted 1 line

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().

Quoted 7 lines

OK, I'm fine with a macro.

Quoted 2 lines

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

#8 lore raw
Quoted 38 lines · Takashi Iwai

At Fri, 14 Nov 2008 15:47:37 +0800,

Quoted 36 lines · Wu Fengguang
Quoted 22 lines · Takashi Iwai

At Fri, 14 Nov 2008 15:38:56 +0800,

Quoted 20 lines · Wu Fengguang
Quoted 19 lines
Quoted 13 lines
Quoted 12 lines

sound/pci/hda/hda_eld.c

Quoted 9 lines
sound/pci/hda/hda_eld.c new file +6 -0
+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?

Quoted 1 line

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.

Quoted 6 lines
Quoted 1 line

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.

Quoted 2 lines

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

Quoted 6 lines
Quoted 2 lines

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

A message that is not in the archive (sent off-list or not yet received): [email protected]

#10 lore raw
Quoted 4 lines · Shane W
Quoted 1 line · Wu Fengguang

Create hda_eld.c for ELD routines and proc interface.

Hi,

Hi Shane,

Quoted 4 lines

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.

Quoted 15 lines
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
Quoted 2 lines

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

A message that is not in the archive (sent off-list or not yet received): [email protected]

#12 lore raw
Quoted 10 lines · Shane W
Quoted 9 lines · Wu Fengguang
Quoted 7 lines · Shane W
Quoted 1 line · Wu Fengguang

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.

Quoted 6 lines
Quoted 2 lines

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.

Quoted 7 lines

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

#13 lore raw
Quoted 18 lines · Wu Fengguang
Quoted 10 lines · Shane W
Quoted 9 lines · Wu Fengguang
Quoted 7 lines · Shane W
Quoted 1 line · Wu Fengguang

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.

Quoted 6 lines
Quoted 2 lines

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

Quoted 5 lines

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.

Hi Shane,

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.

Thanks,
Fengguang

---
 src/i810_reg.h   |   13 ++-
 src/i830.h       |    3 
 src/i830_hdmi.c  |   64 +++++++++++++++
 src/i830_modes.c |  189 +++++++++++++++++++++++++++++++++++++++++++++
 src/i830_sdvo.c  |   49 ++++++++++-
 src/i830_sdvo.h  |    2 
 6 files changed, 313 insertions(+), 7 deletions(-)
src/i810_reg.h +12 -1
@@ -1240,6 +1240,7 @@ SOFTWARE OR THE USE OR OTHER DEALINGS IN
 # define HDMID_HOTPLUG_INT_EN			(1 << 27)
 # define SDVOB_HOTPLUG_INT_EN			(1 << 26)
 # define SDVOC_HOTPLUG_INT_EN			(1 << 25)
+# define AUDIO_HOTPLUG_INT_EN			(1 << 24)
 # define TV_HOTPLUG_INT_EN			(1 << 18)
 # define CRT_HOTPLUG_INT_EN			(1 << 9)
 # define CRT_HOTPLUG_ACTIVATION_PERIOD_32	(0 << 8)
@@ -1271,7 +1272,17 @@ SOFTWARE OR THE USE OR OTHER DEALINGS IN
 # define CRT_HOTPLUG_MONITOR_NONE		(0 << 8)
 # define SDVOC_HOTPLUG_INT_STATUS		(1 << 7)
 # define SDVOB_HOTPLUG_INT_STATUS		(1 << 6)
-
+#define AUDIO_VENDOR_DEVICE_ID  0x62020
+#define INTEL_AUDIO_DEVCL        0x808629FB
+#define INTEL_AUDIO_DEVBLC       0x80862801
+#define INTEL_AUDIO_DEVCTG       0x80862802
+
+#define AUDIO_CNTL_STATUS       0x620B4
+#define AUDIO_ELD_VALID_DEVCL_DEVBLC    (1 << 13)
+#define AUDIO_ELD_VALID_DEVCTG  (1 << 14)
+#define AUDIO_ELD_ADDR          (0xf << 5)
+#define AUDIO_ELD_ACK           (1 << 4)
+#define AUDIO_HDMIW_HDMIEDID    0x6210C
 #define SDVOB			0x61140
 #define SDVOC			0x61160
 #define SDVO_ENABLE				(1 << 31)
src/i830.h +2 -1
@@ -887,7 +887,8 @@ Bool I830UnbindAGPMemory(ScrnInfoPtr pSc
 
 /* i830_modes.c */
 DisplayModePtr i830_ddc_get_modes(xf86OutputPtr output);
-
+int i830_handle_cea_like_data(xf86OutputPtr output,Uchar **buf);
+unsigned long i830_extract_max_tmds_clock(xf86OutputPtr output);
 /* i830_tv.c */
 void i830_tv_init(ScrnInfoPtr pScrn);
 
src/i830_hdmi.c +63 -1

94 lines not on this page · raw message

src/i830_modes.c +189 -0

195 lines not on this page · raw message

src/i830_sdvo.c +45 -4

98 lines not on this page · raw message

src/i830_sdvo.h +2 -0
@@ -24,6 +24,8 @@
  *    Eric Anholt <[email protected]>
  *
  */
+#define SDVO_ELD_PRESENT (1 << 0)
+#define SDVO_ELD_VALID (1 << 1)
 
 Bool
 i830_sdvo_init(ScrnInfoPtr pScrn, int output_device);
---
 hw/xfree86/ddc/edid.h            |  101 ++++++++++++++++++++++++-
 hw/xfree86/ddc/interpret_edid.c  |  102 ++++++++++++++++++++++++-
 hw/xfree86/ddc/print_edid.c      |    9 +-
 hw/xfree86/ddc/xf86DDC.c         |   41 +++++++---
 hw/xfree86/ddc/xf86DDC.h         |    4 +
 hw/xfree86/modes/xf86Crtc.c      |    2 
 hw/xfree86/modes/xf86EdidModes.c |  114 +++++++++++++++++++++++++++--
 7 files changed, 342 insertions(+), 31 deletions(-)
hw/xfree86/ddc/edid.h +96 -5

125 lines not on this page · raw message

hw/xfree86/ddc/interpret_edid.c +95 -7

145 lines not on this page · raw message

hw/xfree86/ddc/print_edid.c +5 -4
@@ -335,11 +335,12 @@ print_detailed_timings(int scrnIndex, st
 
 static void
 print_detailed_monitor_section(int scrnIndex,
-			       struct detailed_monitor_section *m)
+			       struct detailed_monitor_section *m,
+			       int det_mon_num)
 {
     int i,j;
   
-    for (i=0;i<DET_TIMINGS;i++) {
+    for (i=0;i<det_mon_num;i++) {
 	switch (m[i].type) {
 	case DT:
 	    print_detailed_timings(scrnIndex,&m[i].section.d_timings);
@@ -473,12 +474,12 @@ xf86PrintEDID(xf86MonPtr m)
     print_display(m->scrnIndex, &m->features, &m->ver);
     print_established_timings(m->scrnIndex, &m->timings1);
     print_std_timings(m->scrnIndex, m->timings2);
-    print_detailed_monitor_section(m->scrnIndex, m->det_mon);
+    print_detailed_monitor_section(m->scrnIndex, m->det_mon, m->det_mon_num);
     print_number_sections(m->scrnIndex, m->no_sections);
 
     /* extension block section stuff */
 
-    xf86DrvMsg(m->scrnIndex, X_INFO, "EDID (in hex):\n");
+    xf86DrvMsg(m->scrnIndex, X_INFO, "EDID (in hex): flags is <%d>\n",m->no_sections);
 
     n = 128;
     if (m->flags & EDID_COMPLETE_RAWDATA)
hw/xfree86/ddc/xf86DDC.c +31 -10

58 lines not on this page · raw message

hw/xfree86/ddc/xf86DDC.h +4 -0
@@ -44,6 +44,10 @@ extern xf86MonPtr xf86PrintEDID(
 extern xf86MonPtr xf86InterpretEDID(
     int screenIndex, Uchar *block
 );
+extern unsigned char * xf86DDCGetCEA(
+    xf86MonPtr MonPtr,
+    struct extension_type *type
+);
 
 extern xf86MonPtr xf86InterpretEEDID(
     int screenIndex, Uchar *block
hw/xfree86/modes/xf86Crtc.c +1 -1
@@ -2683,7 +2683,7 @@ xf86OutputSetEDID (xf86OutputPtr output,
     if (edid_mon)
     {
 	/* Pull out a phyiscal size from a detailed timing if available. */
-	for (i = 0; i < 4; i++) {
+	for (i = 0; i < edid_mon->det_mon_num; i++) {
 	    if (edid_mon->det_mon[i].type == DT &&
 		edid_mon->det_mon[i].section.d_timings.h_size != 0 &&
 		edid_mon->det_mon[i].section.d_timings.v_size != 0)
hw/xfree86/modes/xf86EdidModes.c +110 -4

150 lines not on this page · raw message

_______________________________________________
xorg mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/xorg

A message that is not in the archive (sent off-list or not yet received): [email protected]

A message that is not in the archive (sent off-list or not yet received): 20081120010204.GA25454@localhost

A message that is not in the archive (sent off-list or not yet received): [email protected]

#17 lore raw

Hi Shane,

Quoted 6 lines · Shane W

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.

Quoted 6 lines

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.

Quoted 6 lines

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.

Quoted 39 lines

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
Quoted 9 lines
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?

Quoted 1 line

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?

Quoted 5 lines

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
---

Quoted 53 lines

Shane

Quoted 50 lines · Wu Fengguang

Thanks Shane!

Ma Ling: do you have a quick fix for it? Or shall I look into it?

Thank you,
Fengguang

Quoted 42 lines · Shane W
Quoted 3 lines · Wu Fengguang

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

#18 lore raw

PATCH 1/1 properly print ELD sample bits

Fix bugs on printing the ELD sample bits.

---
 sound/pci/hda/hda_eld.c  |    2 +-
 sound/pci/hda/hda_proc.c |    3 +--
 2 files changed, 2 insertions(+), 3 deletions(-)
sound/pci/hda/hda_eld.c +1 -1
@@ -397,7 +397,7 @@ static void hdmi_show_short_audio_desc(s
 	snd_print_pcm_rates(a->rates, buf, sizeof(buf));
 
 	if (a->format == AUDIO_CODING_TYPE_LPCM)
-		snd_print_pcm_rates(a->sample_bits, buf2 + 8, sizeof(buf2 - 8));
+		snd_print_pcm_bits(a->sample_bits, buf2 + 8, sizeof(buf2 - 8));
 	else if (a->max_bitrate)
 		snprintf(buf2, sizeof(buf2),
 				", max bitrate = %d", a->max_bitrate);
sound/pci/hda/hda_proc.c +1 -2
@@ -120,7 +120,7 @@ void snd_print_pcm_bits(int pcm, char *b
 	int i, j;
 
 	for (i = 0, j = 0; i < ARRAY_SIZE(bits); i++)
-		if (pcm & (1 << i))
+		if (pcm & (AC_SUPPCM_BITS_8 << i))
 			j += snprintf(buf + j, buflen - j,  " %d", bits[i]);
 
 	buf[j] = '\0'; /* necessary when j == 0 */
@@ -130,7 +130,6 @@ static void print_pcm_bits(struct snd_in
 {
 	char buf[SND_PRINT_BITS_ADVISED_BUFSIZE];
 
-	pcm = (pcm >> 16) & 0xff;
 	snd_iprintf(buffer, "    bits [0x%x]:", pcm);
 	snd_print_pcm_bits(pcm, buf, sizeof(buf));
 	snd_iprintf(buffer, "%s\n", buf);

A message that is not in the archive (sent off-list or not yet received): [email protected]

#21 lore raw
Quoted 9 lines · Shane W
Quoted 8 lines · Wu Fengguang
Quoted 7 lines

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

Quoted 6 lines
Quoted 2 lines

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

Hmm, I thought DTS was just another way of compressing PCM whereas DSD is the format used on SACD disks, IE 6 channel.
http://en.wikipedia.org/wiki/Direct_Stream_Digital

Well my source is a bit different ;-)

http://en.wikipedia.org/wiki/Super_Audio_CD

DST

To reduce the space and bandwidth requirements of DSD (2.8 Mbit/s per channel), a lossless data compression method called Direct Stream Transfer (DST) is used — DST compression is compulsory for multi-channel regions and optional for stereo regions. This typically compresses by a factor of between two and three, allowing a disc to contain 80 minutes of both 2-channel and 5.1-channel sound.

Quoted 7 lines
Quoted 4 lines
Quoted 1 line

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?

There are six speakers connected, fl, ct, fr, rl, rr and lfe.

Takashi, I'd suggest to support writing to the ELD proc interface to alter the internal ELD struct. This could let users debug/fix quicks conveniently. But sure these fixes should eventually be incorporated into the kernel so that it just works.

Quoted 15 lines
Quoted 7 lines
Quoted 5 lines

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 :-)

Yeah, you should get:
Front left
Center
Rear left
Rear right
and a little boom from the sub

Quoted 2 lines

If you encode 51test.wav to ac3 and stream through hdmi directly, it does this properly.

OK, so 5.1 AC3 audio plays properly on HDMI?

Thanks,
Fengguang
_______________________________________________
Alsa-devel mailing list
[email protected]
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

#22 lore raw
In reply to a message not in the archive
Quoted 6 lines · Shane W
Quoted 2 lines · Wu Fengguang

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

There are six speakers connected, fl, ct, fr, rl, rr and lfe.

Hi Shane,

Apply this patch and run "echo speakers f > /proc/asound/card0/eld\#3". That will make the ELD "speakers" field right.

Thanks,
Fengguang
---
support writing to the ELD proc file

Allow users to fix quicks of ELD ROMs by writing new values to the ELD proc interface. The format is one or more lines of "name hex_value".

Users can add/remove/modify up to 32 SAD(Short Audio Descriptor) entries.

---
 sound/pci/hda/hda_eld.c |   56 ++++++++++++++++++++++++++++++++++++++
 1 file changed, 56 insertions(+)
sound/pci/hda/hda_eld.c +56 -0
@@ -500,6 +500,59 @@ static void hdmi_print_eld_info(struct s
 		hdmi_print_sad_info(i, e->sad + i, buffer);
 }
 
+static void hdmi_write_eld_item(struct snd_info_entry *entry,
+				struct snd_info_buffer *buffer)
+{
+	struct hdmi_eld *e = entry->private_data;
+	char line[64];
+	char name[64];
+	char *sname;
+	long long val;
+	int n;
+
+	while (!snd_info_get_line(buffer, line, sizeof(line))) {
+		if (sscanf(line, "%s %llx", name, &val) != 2)
+			continue;
+		if (!strcmp(name, "connection_type"))
+			e->conn_type = val;
+		else if (!strcmp(name, "port_id"))
+			e->port_id = val;
+		else if (!strcmp(name, "support_hdcp"))
+			e->support_hdcp = val;
+		else if (!strcmp(name, "support_ai"))
+			e->support_ai = val;
+		else if (!strcmp(name, "audio_sync_delay"))
+			e->aud_synch_delay = val;
+		else if (!strcmp(name, "speakers"))
+			e->spk_alloc = val;
+		else if (!strcmp(name, "sad_count"))
+			e->sad_count = val;
+		else if (!strncmp(name, "sad", 3)) {
+			sname = name + 4;
+			n = name[3] - '0';
+			if (name[4] >= '0' && name[4] <= '9') {
+				sname++;
+				n = 10 * n + name[4] - '0';
+			}
+			if (n < 0 || n > 31) /* double the CEA limit */
+				continue;
+			if (!strcmp(sname, "_coding_type"))
+				e->sad[n].format = val;
+			else if (!strcmp(sname, "_channels"))
+				e->sad[n].channels = val;
+			else if (!strcmp(sname, "_rates"))
+				e->sad[n].rates = val;
+			else if (!strcmp(sname, "_bits"))
+				e->sad[n].sample_bits = val;
+			else if (!strcmp(sname, "_max_bitrate"))
+				e->sad[n].max_bitrate = val;
+			if (n >= e->sad_count)
+				e->sad_count = n + 1;
+		}
+	}
+}
+
+
 int snd_hda_eld_proc_new(struct hda_codec *codec, struct hdmi_eld *eld)
 {
 	char name[32];
@@ -512,6 +565,9 @@ int snd_hda_eld_proc_new(struct hda_code
 		return err;
 
 	snd_info_set_text_ops(entry, eld, hdmi_print_eld_info);
+	entry->c.text.write = hdmi_write_eld_item;
+	entry->mode |= S_IWUSR;
+
 	return 0;
 }
#23 lore raw

At Fri, 21 Nov 2008 11:41:50 +0800,

Quoted 19 lines · Wu Fengguang
Quoted 6 lines · Shane W
Quoted 2 lines · Wu Fengguang

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

There are six speakers connected, fl, ct, fr, rl, rr and lfe.

Hi Shane,

Apply this patch and run "echo speakers f > /proc/asound/card0/eld\#3". That will make the ELD "speakers" field right.

Thanks,
Fengguang
---
support writing to the ELD proc file

Quoted 6 lines

Allow users to fix quicks of ELD ROMs by writing new values to the ELD proc interface. The format is one or more lines of "name hex_value".

Users can add/remove/modify up to 32 SAD(Short Audio Descriptor) entries.

This is a nice feature, so I applied now as is. Please give an incremental patch if you need more fixes.

thanks,

Takashi

Quoted 77 lines
---
 sound/pci/hda/hda_eld.c |   56 ++++++++++++++++++++++++++++++++++++++
 1 file changed, 56 insertions(+)
sound/pci/hda/hda_eld.c +56 -0
@@ -500,6 +500,59 @@ static void hdmi_print_eld_info(struct s
 		hdmi_print_sad_info(i, e->sad + i, buffer);
 }
 
+static void hdmi_write_eld_item(struct snd_info_entry *entry,
+				struct snd_info_buffer *buffer)
+{
+	struct hdmi_eld *e = entry->private_data;
+	char line[64];
+	char name[64];
+	char *sname;
+	long long val;
+	int n;
+
+	while (!snd_info_get_line(buffer, line, sizeof(line))) {
+		if (sscanf(line, "%s %llx", name, &val) != 2)
+			continue;
+		if (!strcmp(name, "connection_type"))
+			e->conn_type = val;
+		else if (!strcmp(name, "port_id"))
+			e->port_id = val;
+		else if (!strcmp(name, "support_hdcp"))
+			e->support_hdcp = val;
+		else if (!strcmp(name, "support_ai"))
+			e->support_ai = val;
+		else if (!strcmp(name, "audio_sync_delay"))
+			e->aud_synch_delay = val;
+		else if (!strcmp(name, "speakers"))
+			e->spk_alloc = val;
+		else if (!strcmp(name, "sad_count"))
+			e->sad_count = val;
+		else if (!strncmp(name, "sad", 3)) {
+			sname = name + 4;
+			n = name[3] - '0';
+			if (name[4] >= '0' && name[4] <= '9') {
+				sname++;
+				n = 10 * n + name[4] - '0';
+			}
+			if (n < 0 || n > 31) /* double the CEA limit */
+				continue;
+			if (!strcmp(sname, "_coding_type"))
+				e->sad[n].format = val;
+			else if (!strcmp(sname, "_channels"))
+				e->sad[n].channels = val;
+			else if (!strcmp(sname, "_rates"))
+				e->sad[n].rates = val;
+			else if (!strcmp(sname, "_bits"))
+				e->sad[n].sample_bits = val;
+			else if (!strcmp(sname, "_max_bitrate"))
+				e->sad[n].max_bitrate = val;
+			if (n >= e->sad_count)
+				e->sad_count = n + 1;
+		}
+	}
+}
+
+
 int snd_hda_eld_proc_new(struct hda_codec *codec, struct hdmi_eld *eld)
 {
 	char name[32];
@@ -512,6 +565,9 @@ int snd_hda_eld_proc_new(struct hda_code
 		return err;
 
 	snd_info_set_text_ops(entry, eld, hdmi_print_eld_info);
+	entry->c.text.write = hdmi_write_eld_item;
+	entry->mode |= S_IWUSR;
+
 	return 0;
 }
 
#24 lore raw
Quoted 24 lines · Takashi Iwai

At Fri, 21 Nov 2008 11:41:50 +0800,

Quoted 22 lines · Wu Fengguang
Quoted 6 lines · Shane W
Quoted 2 lines · Wu Fengguang

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

There are six speakers connected, fl, ct, fr, rl, rr and lfe.

Hi Shane,

Apply this patch and run "echo speakers f > /proc/asound/card0/eld\#3". That will make the ELD "speakers" field right.

Thanks,
Fengguang
---
support writing to the ELD proc file

Allow users to fix quicks of ELD ROMs by writing new values to the ELD proc interface. The format is one or more lines of "name hex_value".

Quoted 6 lines
Quoted 3 lines

Users can add/remove/modify up to 32 SAD(Short Audio Descriptor) entries.

This is a nice feature, so I applied now as is. Please give an incremental patch if you need more fixes.

OK, thank you!

Fengguang