Skip to contentSkip to conversation

minor HDMI cleanups and fixes

PATCH Applied Wu Fengguang 3 participants 12 messages Nov 22, 2008 – Nov 24, 2008

Subsystems (2)
lore
Share & tools
lore
#0 lore raw

cover minor HDMI cleanups and fixes

Takashi,

Here are another set of HDMI cleanups and fixes.

[PATCH 1/7] hda - minor HDMI code cleanups
[PATCH 2/7] hda - report selected CA index for Audio InfoFrame [PATCH 3/7] hda - make HDMI messages more user friendly [PATCH 4/7] hda - ELD proc interface write updates [PATCH 5/7] hda - document the ELD proc interface [PATCH 6/7] hda - fix DisplayPort naming
[PATCH 7/7] hda - fix build warning when CONFIG_PROC_FS=n

Thanks,
Fengguang

#1 lore raw

PATCH 1/7 hda - minor HDMI code cleanups

Some minor code cleanups.

---
 sound/pci/hda/patch_intelhdmi.c |    6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)
sound/pci/hda/patch_intelhdmi.c +3 -3
@@ -431,7 +431,7 @@ static int hdmi_setup_channel_allocation
 	 * expand ELD's speaker allocation mask
 	 *
 	 * ELD tells the speaker mask in a compact(paired) form,
-	 * expand ELD's notions to match the ones used by audio infoframe.
+	 * expand ELD's notions to match the ones used by Audio InfoFrame.
 	 */
 	for (i = 0; i < ARRAY_SIZE(eld_speaker_allocation_bits); i++) {
 		if (eld->spk_alloc & (1 << i))
@@ -592,8 +592,8 @@ static struct hda_pcm_stream intel_hdmi_
 	.channels_max = 8,
 	.nid = CVT_NID, /* NID to query formats and rates and setup streams */
 	.ops = {
-		.open = intel_hdmi_playback_pcm_open,
-		.close = intel_hdmi_playback_pcm_close,
+		.open    = intel_hdmi_playback_pcm_open,
+		.close   = intel_hdmi_playback_pcm_close,
 		.prepare = intel_hdmi_playback_pcm_prepare
 	},
 };
#2 lore raw

PATCH 2/7 hda - report selected CA index for Audio InfoFrame

In reply to Wu Fengguang

Print some CA selecting info, which could be valuable for debugging when something goes wrong.

---
 sound/pci/hda/patch_intelhdmi.c |   10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)
sound/pci/hda/patch_intelhdmi.c +6 -4
@@ -444,14 +444,16 @@ static int hdmi_setup_channel_allocation
 		    (spk_mask & channel_allocations[i].spk_mask) ==
 				channel_allocations[i].spk_mask) {
 			ai->CA = channel_allocations[i].ca_index;
-			return 0;
+			break;
 		}
 	}
 
 	snd_print_channel_allocation(eld->spk_alloc, buf, sizeof(buf));
-	snd_printd(KERN_INFO "failed to setup channel allocation: %d of %s\n",
-			channels, buf);
-	return -1;
+	snd_printdd(KERN_INFO
+			"HDMI: select CA 0x%x for %d-channel allocation: %s\n",
+			ai->CA, channels, buf);
+
+	return ai->CA;
 }
 
 static void hdmi_setup_channel_mapping(struct hda_codec *codec,
#5 lore raw

PATCH 5/7 hda - document the ELD proc interface

In reply to Wu Fengguang

Describe what ELD proc interface provides and how to fix incorrect values.

---
 Documentation/sound/alsa/Procfile.txt |   10 ++++++++++
 1 file changed, 10 insertions(+)
Documentation/sound/alsa/Procfile.txt +10 -0
@@ -153,6 +153,16 @@ card*/codec#*
 	Shows the general codec information and the attribute of each
 	widget node.
 
+card*/eld#*
+	Available for HDMI or DisplayPort interfaces.
+	Shows ELD(EDID Like Data) info retrieved from the attached HDMI sink,
+	and describes its audio capabilities and configurations.
+
+	Some ELD fields may be modified by doing `echo name hex_value > eld#*`.
+	Only do this if you are sure the HDMI sink provided value is wrong.
+	And if that makes your HDMI audio work, please report to us so that we
+	can fix it in future kernel releases.
+
 
 Sequencer Information
 ---------------------
#6 lore raw

PATCH 6/7 hda - fix DisplayPort naming

In reply to Wu Fengguang

DisplayPort is a digital display interface standard put forth by the Video Electronics Standards Association (VESA). It defines a new license-free, royalty-free, digital audio/video interconnect, intended to be used primarily between a computer and its display monitor, or a computer and a home-theater system.

- From Wikipedia, the free encyclopedia

---
 sound/pci/hda/hda_eld.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
sound/pci/hda/hda_eld.c +1 -1
@@ -72,7 +72,7 @@ static char *cea_speaker_allocation_name
 
 static char *eld_connection_type_names[4] = {
 	"HDMI",
-	"Display Port",
+	"DisplayPort",
 	"2-reserved",
 	"3-reserved"
 };
#3 lore raw

PATCH 3/7 hda - make HDMI messages more user friendly

In reply to Wu Fengguang

- make some messages more user friendly
- add message prefix "HDMI:" to indicate the problem's domain (also easier to do dmesg | grep HDMI ;-)

---
 sound/pci/hda/hda_eld.c         |   30 +++++++++++++++-------------
 sound/pci/hda/patch_intelhdmi.c |   32 +++++++++++++++---------------
 2 files changed, 32 insertions(+), 30 deletions(-)
sound/pci/hda/hda_eld.c +16 -14
@@ -168,11 +168,11 @@ static unsigned char hdmi_get_eld_byte(s
 					AC_VERB_GET_HDMI_ELDD, byte_index);
 
 #ifdef BE_PARANOID
-	printk(KERN_INFO "ELD data byte %d: 0x%x\n", byte_index, val);
+	printk(KERN_INFO "HDMI: 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",
+		snd_printd(KERN_INFO "HDMI: invalid ELD data byte %d\n",
 								byte_index);
 		val = 0;
 	}
@@ -208,7 +208,7 @@ static void hdmi_update_short_audio_desc
 	switch (a->format) {
 	case AUDIO_CODING_TYPE_REF_STREAM_HEADER:
 		snd_printd(KERN_INFO
-				"audio coding type 0 not expected in ELD\n");
+				"HDMI: audio coding type 0 not expected\n");
 		break;
 
 	case AUDIO_CODING_TYPE_LPCM:
@@ -254,7 +254,7 @@ static void hdmi_update_short_audio_desc
 		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",
+				"HDMI: audio coding xtype %d not expected\n",
 				a->format);
 			a->format = 0;
 		} else
@@ -276,7 +276,8 @@ static int hdmi_update_eld(struct hdmi_e
 	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);
+		snd_printd(KERN_INFO "HDMI: Unknown ELD version %d\n",
+								e->eld_ver);
 		goto out_fail;
 	}
 
@@ -300,17 +301,17 @@ static int hdmi_update_eld(struct hdmi_e
 	e->product_id	  = get_unaligned_le16(buf + 18);
 
 	if (mnl > ELD_MAX_MNL) {
-		snd_printd(KERN_INFO "MNL is reserved value %d\n", mnl);
+		snd_printd(KERN_INFO "HDMI: 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);
+		snd_printd(KERN_INFO "HDMI: 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);
+			snd_printd(KERN_INFO "HDMI: out of range SAD %d\n", i);
 			goto out_fail;
 		}
 		hdmi_update_short_audio_desc(e->sad + i,
@@ -339,7 +340,8 @@ static int hdmi_eld_valid(struct hda_cod
 	present = (present & AC_PINSENSE_PRESENCE);
 
 #ifdef CONFIG_SND_DEBUG_VERBOSE
-	printk(KERN_INFO "pinp = %d, eldv = %d\n", !!present, !!eldv);
+	printk(KERN_INFO "HDMI: sink_present = %d, eld_valid = %d\n",
+			!!present, !!eldv);
 #endif
 
 	return eldv && present;
@@ -365,11 +367,11 @@ int snd_hdmi_get_eld(struct hdmi_eld *el
 	size = snd_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");
+		snd_printd(KERN_INFO "HDMI: 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);
+		snd_printd(KERN_INFO "HDMI: invalid ELD buf size %d\n", size);
 		return -ERANGE;
 	}
 
@@ -404,7 +406,7 @@ static void hdmi_show_short_audio_desc(s
 	else
 		buf2[0] = '\0';
 
-	printk(KERN_INFO "supports coding type %s:"
+	printk(KERN_INFO "HDMI: supports coding type %s:"
 			" channels = %d, rates =%s%s\n",
 			cea_audio_coding_type_names[a->format],
 			a->channels,
@@ -428,14 +430,14 @@ void snd_hdmi_show_eld(struct hdmi_eld *
 {
 	int i;
 
-	printk(KERN_INFO "detected monitor %s at connection type %s\n",
+	printk(KERN_INFO "HDMI: detected monitor %s at connection type %s\n",
 			e->monitor_name,
 			eld_connection_type_names[e->conn_type]);
 
 	if (e->spk_alloc) {
 		char buf[SND_PRINT_CHANNEL_ALLOCATION_ADVISED_BUFSIZE];
 		snd_print_channel_allocation(e->spk_alloc, buf, sizeof(buf));
-		printk(KERN_INFO "available speakers:%s\n", buf);
+		printk(KERN_INFO "HDMI: available speakers:%s\n", buf);
 	}
 
 	for (i = 0; i < e->sad_count; i++)
sound/pci/hda/patch_intelhdmi.c +16 -16
@@ -286,7 +286,7 @@ static void hdmi_set_channel_count(struc
 					AC_VERB_SET_CVT_CHAN_COUNT, chs - 1);
 
 	if (chs != hdmi_get_channel_count(codec))
-		snd_printd(KERN_INFO "Channel count expect=%d, real=%d\n",
+		snd_printd(KERN_INFO "HDMI channel count: expect %d, get %d\n",
 					chs, hdmi_get_channel_count(codec));
 }
 
@@ -299,7 +299,7 @@ static void hdmi_debug_channel_mapping(s
 	for (i = 0; i < 8; i++) {
 		slot = snd_hda_codec_read(codec, CVT_NID, 0,
 						AC_VERB_GET_HDMI_CHAN_SLOT, i);
-		printk(KERN_DEBUG "ASP channel %d => slot %d\n",
+		printk(KERN_DEBUG "HDMI: ASP channel %d => slot %d\n",
 						slot >> 4, slot & 0x7);
 	}
 #endif
@@ -326,12 +326,12 @@ static void hdmi_debug_dip_size(struct h
 	int size;
 
 	size = snd_hdmi_get_eld_size(codec, PIN_NID);
-	printk(KERN_DEBUG "ELD buf size is %d\n", size);
+	printk(KERN_DEBUG "HDMI: ELD buf size is %d\n", size);
 
 	for (i = 0; i < 8; i++) {
 		size = snd_hda_codec_read(codec, PIN_NID, 0,
 						AC_VERB_GET_HDMI_DIP_SIZE, i);
-		printk(KERN_DEBUG "DIP GP[%d] buf size is %d\n", i, size);
+		printk(KERN_DEBUG "HDMI: DIP GP[%d] buf size is %d\n", i, size);
 	}
 #endif
 }
@@ -359,8 +359,8 @@ static void hdmi_clear_dip_buffers(struc
 				break;
 		}
 		snd_printd(KERN_INFO
-				"DIP GP[%d] buf reported size=%d, written=%d\n",
-				i, size, j);
+			"HDMI: DIP GP[%d] buf reported size=%d, written=%d\n",
+			i, size, j);
 	}
 #endif
 }
@@ -498,7 +498,9 @@ static void hdmi_intrinsic_event(struct 
 	int pind = !!(res & AC_UNSOL_RES_PD);
 	int eldv = !!(res & AC_UNSOL_RES_ELDV);
 
-	printk(KERN_INFO "HDMI intrinsic event: PD=%d ELDV=%d\n", pind, eldv);
+	printk(KERN_INFO
+		"HDMI hot plug event: Presence_Detect=%d ELD_Valid=%d\n",
+		pind, eldv);
 
 	if (pind && eldv) {
 		hdmi_parse_eld(codec);
@@ -512,13 +514,13 @@ static void hdmi_non_intrinsic_event(str
 	int cp_state = !!(res & AC_UNSOL_RES_CP_STATE);
 	int cp_ready = !!(res & AC_UNSOL_RES_CP_READY);
 
-	printk(KERN_INFO "HDMI non-intrinsic event: "
-			"SUBTAG=0x%x CP_STATE=%d CP_READY=%d\n",
-			subtag,
-			cp_state,
-			cp_ready);
+	printk(KERN_INFO
+		"HDMI content protection event: SUBTAG=0x%x CP_STATE=%d CP_READY=%d\n",
+		subtag,
+		cp_state,
+		cp_ready);
 
-	/* who cares? */
+	/* TODO */
 	if (cp_state)
 		;
 	if (cp_ready)
@@ -532,9 +534,7 @@ static void intel_hdmi_unsol_event(struc
 	int subtag = (res & AC_UNSOL_RES_SUBTAG) >> AC_UNSOL_RES_SUBTAG_SHIFT;
 
 	if (tag != INTEL_HDMI_EVENT_TAG) {
-		snd_printd(KERN_INFO
-				"Unexpected HDMI unsolicited event tag 0x%x\n",
-				tag);
+		snd_printd(KERN_INFO "Unexpected HDMI event tag 0x%x\n", tag);
 		return;
 	}
 
#4 lore raw

PATCH 4/7 hda - ELD proc interface write updates

In reply to Wu Fengguang

- rename ELD proc write routine to hdmi_write_eld_info()
- support modifying WMAPro's profile

Write to some ELD fields (monitor_name, manufacture_id, product_id, eld_version, edid_version) are deliberately not supported, since that won't correct wrong behaviors and only leads to confusions.

---
 sound/pci/hda/hda_eld.c |   11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)
sound/pci/hda/hda_eld.c +9 -2
@@ -502,7 +502,7 @@ 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,
+static void hdmi_write_eld_info(struct snd_info_entry *entry,
 				struct snd_info_buffer *buffer)
 {
 	struct hdmi_eld *e = entry->private_data;
@@ -515,6 +515,11 @@ static void hdmi_write_eld_item(struct s
 	while (!snd_info_get_line(buffer, line, sizeof(line))) {
 		if (sscanf(line, "%s %llx", name, &val) != 2)
 			continue;
+		/*
+		 * We don't allow modification to these fields:
+		 * 	monitor_name manufacture_id product_id
+		 * 	eld_version edid_version
+		 */
 		if (!strcmp(name, "connection_type"))
 			e->conn_type = val;
 		else if (!strcmp(name, "port_id"))
@@ -548,6 +553,8 @@ static void hdmi_write_eld_item(struct s
 				e->sad[n].sample_bits = val;
 			else if (!strcmp(sname, "_max_bitrate"))
 				e->sad[n].max_bitrate = val;
+			else if (!strcmp(sname, "_profile"))
+				e->sad[n].profile = val;
 			if (n >= e->sad_count)
 				e->sad_count = n + 1;
 		}
@@ -567,7 +574,7 @@ 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->c.text.write = hdmi_write_eld_info;
 	entry->mode |= S_IWUSR;
 	eld->proc_entry = entry;
 
#7 lore raw

PATCH 7/7 hda - fix build warning when CONFIG_PROC_FS=n

In reply to Wu Fengguang

Fix "defined but not used" build warning by moving eld_versoin_names[] and cea_edid_version_names[] into hdmi_print_eld_info().

---
 sound/pci/hda/hda_eld.c |   30 ++++++++++++++----------------
 1 file changed, 14 insertions(+), 16 deletions(-)
sound/pci/hda/hda_eld.c +14 -16
@@ -32,14 +32,6 @@ enum eld_versions {
 	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,
@@ -48,14 +40,6 @@ enum cea_edid_versions {
 	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"
-};
-
 static char *cea_speaker_allocation_names[] = {
 	/*  0 */ "FL/FR",
 	/*  1 */ "LFE",
@@ -478,6 +462,20 @@ static void hdmi_print_eld_info(struct s
 	struct hdmi_eld *e = entry->private_data;
 	char buf[SND_PRINT_CHANNEL_ALLOCATION_ADVISED_BUFSIZE];
 	int i;
+	static char *eld_versoin_names[32] = {
+		"reserved",
+		"reserved",
+		"CEA-861D or below",
+		[3 ... 30] = "reserved",
+		[31] = "partial"
+	};
+	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"
+	};
 
 	snd_iprintf(buffer, "monitor_name\t\t%s\n", e->monitor_name);
 	snd_iprintf(buffer, "connection_type\t\t%s\n",
Applied
#8 lore raw
In reply to Wu Fengguang

At Sat, 22 Nov 2008 09:40:50 +0800,

Quoted 5 lines · Wu Fengguang

Takashi,

Here are another set of HDMI cleanups and fixes.

Quoted 7 lines

[PATCH 1/7] hda - minor HDMI code cleanups
[PATCH 2/7] hda - report selected CA index for Audio InfoFrame [PATCH 3/7] hda - make HDMI messages more user friendly [PATCH 4/7] hda - ELD proc interface write updates [PATCH 5/7] hda - document the ELD proc interface [PATCH 6/7] hda - fix DisplayPort naming
[PATCH 7/7] hda - fix build warning when CONFIG_PROC_FS=n

Thanks, applied all patches now.

Takashi

#9 lore raw
Quoted 15 lines · Takashi Iwai

At Sat, 22 Nov 2008 09:40:50 +0800,

Quoted 5 lines · Wu Fengguang

Takashi,

Here are another set of HDMI cleanups and fixes.

Quoted 7 lines

[PATCH 1/7] hda - minor HDMI code cleanups
[PATCH 2/7] hda - report selected CA index for Audio InfoFrame [PATCH 3/7] hda - make HDMI messages more user friendly [PATCH 4/7] hda - ELD proc interface write updates [PATCH 5/7] hda - document the ELD proc interface [PATCH 6/7] hda - fix DisplayPort naming
[PATCH 7/7] hda - fix build warning when CONFIG_PROC_FS=n

Quoted 1 line

Thanks, applied all patches now.

Thank you!

Fengguang

#11 lore raw

Re: Serato patch review

At Sat, 22 Nov 2008 14:12:30 +0100,

Quoted 3 lines · LCID Fire

I uploaded the working patch for the serato device to the bug 0003747. Can someone please review and if it is ok submit it?

Don't hesitate to post a preliminary patch to here. ML is a better to place to review in general...

thanks,

Takashi