[PATCH 0/4] [RFC] multi-channel HDMI audio support

[PATCH 0/4] [RFC] multi-channel HDMI audio support

[Wu Fengguang]

Hi all,

This patchset attempts to support multi-channel HDMI audio.

It contains two basic operations:

- setup the CA(channel-allcation) byte in the audio infoframe
- setup the converter channel to HDMI slot mapping

Please check the last two patches for the detailed descriptions:

	[PATCH 1/4] hda: make standalone hdmi_fill_audio_infoframe()                               
	[PATCH 2/4] hda: make global snd_print_channel_allocation()                       

	[PATCH 3/4] hda: HDMI channel allocations for audio infoframe                               
	[PATCH 4/4] hda: HDMI channel mapping cleanups                                               

The current implementation only does minimal works that can ensure the
multi-channel audio playing out. However the wrong speakers may be choosed, and
I really need more information/knowledge/instructions from you to get it right.

Thank you,
Fengguang
-- 

[PATCH 1/4] hda: make standalone hdmi_fill_audio_infoframe()

[Wu Fengguang]

[-- Attachment #1: alsa-infoframe-cleanup.patch --]
[-- Type: text/plain, Size: 1418 bytes --]

code refactor: make a standalone function hdmi_fill_audio_infoframe().

Signed-off-by: Wu Fengguang <wfg@linux.intel.com>
---
 sound/pci/hda/patch_intelhdmi.c |   25 ++++++++++++++++---------
 1 file changed, 16 insertions(+), 9 deletions(-)

--- sound-2.6.orig/sound/pci/hda/patch_intelhdmi.c
+++ sound-2.6/sound/pci/hda/patch_intelhdmi.c
@@ -246,24 +246,31 @@ static void hdmi_clear_dip_buffers(struc
 #endif
 }
 
+static void hdmi_fill_audio_infoframe(struct hda_codec *codec,
+					struct hdmi_audio_infoframe *ai)
+{
+	u8 *params = (u8 *)ai;
+	int i;
+
+	hdmi_debug_dip_size(codec);
+	hdmi_clear_dip_buffers(codec); /* be paranoid */
+
+	hdmi_set_dip_index(codec, PIN_NID, 0x0, 0x0);
+	for (i = 0; i < sizeof(ai); i++)
+		hdmi_write_dip_byte(codec, PIN_NID, params[i]);
+}
+
 static void hdmi_setup_audio_infoframe(struct hda_codec *codec,
 					struct snd_pcm_substream *substream)
 {
-	struct hdmi_audio_infoframe audio_infoframe = {
+	struct hdmi_audio_infoframe ai = {
 		.type		= 0x84,
 		.ver		= 0x01,
 		.len		= 0x0a,
 		.CC02_CT47	= substream->runtime->channels - 1,
 	};
-	u8 *params = (u8 *)&audio_infoframe;
-	int i;
-
-	hdmi_debug_dip_size(codec);
-	hdmi_clear_dip_buffers(codec); /* be paranoid */
 
-	hdmi_set_dip_index(codec, PIN_NID, 0x0, 0x0);
-	for (i = 0; i < sizeof(audio_infoframe); i++)
-		hdmi_write_dip_byte(codec, PIN_NID, params[i]);
+	hdmi_fill_audio_infoframe(codec, &ai);
 }
 
 

-- 

[PATCH 2/4] hda: make global snd_print_channel_allocation()

[Wu Fengguang]

[-- Attachment #1: alsa-global-print-ca.patch --]
[-- Type: text/plain, Size: 2585 bytes --]

code refactor: make a global function snd_print_channel_allocation().

Signed-off-by: Wu Fengguang <wfg@linux.intel.com>
---
 sound/pci/hda/hda_eld.c   |   11 +++++------
 sound/pci/hda/hda_local.h |    3 +++
 2 files changed, 8 insertions(+), 6 deletions(-)

--- sound-2.6.orig/sound/pci/hda/hda_eld.c
+++ sound-2.6/sound/pci/hda/hda_eld.c
@@ -407,8 +407,7 @@ static void hdmi_show_short_audio_desc(s
 		printk(KERN_INFO "profile: %d\n", a->profile);
 }
 
-#define HDMI_PRINT_CHANNEL_ALLOCATION_ADVISED_BUFSIZE 80
-static void hdmi_print_channel_allocation(int spk_alloc, char *buf, int buflen)
+void snd_print_channel_allocation(int spk_alloc, char *buf, int buflen)
 {
 	int i, j;
 
@@ -425,7 +424,7 @@ static void hdmi_print_channel_allocatio
 void snd_hdmi_show_eld(struct sink_eld *e)
 {
 	int i;
-	char buf[HDMI_PRINT_CHANNEL_ALLOCATION_ADVISED_BUFSIZE];
+	char buf[SND_PRINT_CHANNEL_ALLOCATION_ADVISED_BUFSIZE];
 
 	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);
@@ -446,7 +445,7 @@ void snd_hdmi_show_eld(struct sink_eld *
 				eld_connection_type_names[e->conn_type]);
 	printk(KERN_INFO "monitor name     is %s\n", e->monitor_name);
 
-	hdmi_print_channel_allocation(e->spk_alloc, buf, sizeof(buf));
+	snd_print_channel_allocation(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++)
@@ -484,7 +483,7 @@ static void hdmi_print_eld_info(struct s
 				struct snd_info_buffer *buffer)
 {
 	struct sink_eld *e = entry->private_data;
-	char buf[HDMI_PRINT_CHANNEL_ALLOCATION_ADVISED_BUFSIZE];
+	char buf[SND_PRINT_CHANNEL_ALLOCATION_ADVISED_BUFSIZE];
 	int i;
 
 	snd_iprintf(buffer, "monitor name\t\t%s\n", e->monitor_name);
@@ -501,7 +500,7 @@ static void hdmi_print_eld_info(struct s
 	snd_iprintf(buffer, "support_ai\t\t%d\n", e->support_ai);
 	snd_iprintf(buffer, "audio_sync_delay\t%d\n", e->aud_synch_delay);
 
-	hdmi_print_channel_allocation(e->spk_alloc, buf, sizeof(buf));
+	snd_print_channel_allocation(e->spk_alloc, buf, sizeof(buf));
 	snd_iprintf(buffer, "speakers\t\t[0x%x] %s\n", e->spk_alloc, buf);
 
 	snd_iprintf(buffer, "sad_count\t\t%d\n", e->sad_count);
--- sound-2.6.orig/sound/pci/hda/hda_local.h
+++ sound-2.6/sound/pci/hda/hda_local.h
@@ -493,4 +493,7 @@ inline int snd_hda_eld_proc_new(struct h
 }
 #endif
 
+#define SND_PRINT_CHANNEL_ALLOCATION_ADVISED_BUFSIZE 80
+void snd_print_channel_allocation(int spk_alloc, char *buf, int buflen);
+
 #endif /* __SOUND_HDA_LOCAL_H */

-- 

[PATCH 3/4] hda: HDMI channel allocations for audio infoframe

[Wu Fengguang]

[-- Attachment #1: alsa-speakers.patch --]
[-- Type: text/plain, Size: 8915 bytes --]

To play a 3+ channels LPCM/DSD stream via HDMI,

	- HDMI sink must tell HDMI source about its speaker placements
	  (via ELD, speaker-allocation field)
	- HDMI source must tell the HDMI sink about channel allocation
	  (via audio infoframe, channel-allocation field)

(related docs: HDMI 1.3a spec section 7.4, CEA-861-D section 7.5.3 and 6.6)

This patch attempts to set the CA(channel-allocation) byte in the audio infoframe
according to
	- the number of channels in the current stream
	- the speakers attached to the HDMI sink

A channel_allocations[] line must meet the following two criterions to be
considered as a valid candidate for CA:
	1) its number of allocated channels = substream->runtime->channels
	2) its speakers are a subset of the available ones on the sink side

If there are multiple candidates, the first one is selected.  This simple
policy shall cheat the sink into playing music, but may direct data to the
wrong speakers.

Sorry, this last step is not obvious to me. Any domain experts, please?

Signed-off-by: Wu Fengguang <wfg@linux.intel.com>
---
 sound/pci/hda/patch_intelhdmi.c |  189 ++++++++++++++++++++++++++++++
 1 file changed, 189 insertions(+)

--- sound-2.6.orig/sound/pci/hda/patch_intelhdmi.c
+++ sound-2.6/sound/pci/hda/patch_intelhdmi.c
@@ -89,6 +89,118 @@ struct hdmi_audio_infoframe {
 };
 
 /*
+ * CEA speaker placement:
+ *
+ *        FLH       FCH        FRH
+ *  FLW    FL  FLC   FC   FRC   FR   FRW
+ *
+ *                                  LFE
+ *                     TC
+ *
+ *          RL  RLC   RC   RRC   RR
+ */
+enum cea_speaker_placement {
+	FL  = (1 <<  0),	/* Front Left           */
+	FC  = (1 <<  1),	/* Front Center         */
+	FR  = (1 <<  2),	/* Front Right          */
+	FLC = (1 <<  3),	/* Front Left Center    */
+	FRC = (1 <<  4),	/* Front Right Center   */
+	RL  = (1 <<  5),	/* Rear Left            */
+	RC  = (1 <<  6),	/* Rear Center          */
+	RR  = (1 <<  7),	/* Rear Right           */
+	RLC = (1 <<  8),	/* Rear Left Center     */
+	RRC = (1 <<  9),	/* Rear Right Center    */
+	LFE = (1 << 10),	/* Low Frequency Effect */
+	FLW = (1 << 11),	/* Front Left Wide      */
+	FRW = (1 << 12),	/* Front Right Wide     */
+	FLH = (1 << 13),	/* Front Left High      */
+	FCH = (1 << 14),	/* Front Center High    */
+	FRH = (1 << 15),	/* Front Right High     */
+	TC  = (1 << 16),	/* Top Center           */
+};
+
+/*
+ * ELD SA bits in the CEA Speaker Allocation data block
+ */
+static int eld_speaker_allocation_bits[] = {
+	[0] = FL | FR,
+	[1] = LFE,
+	[2] = FC,
+	[3] = RL | RR,
+	[4] = RC,
+	[5] = FLC | FRC,
+	[6] = RLC | RRC,
+	/* the following are not defined in ELD yet */
+	[7] = FLW | FRW,
+	[8] = FLH | FRH,
+	[9] = TC,
+	[10] = FCH,
+};
+
+struct cea_channel_speaker_allocation {
+	int ca_index;
+	int speakers[8];
+
+	/* derived values, just for convenience */
+	int channels;
+	int spk_mask;
+};
+
+static struct cea_channel_speaker_allocation channel_allocations[] = {
+/* channel number:  8      7     6     5     4      3     2     1 */
+	{0x00,  {   0,     0,    0,    0,    0,     0,   FR,   FL } },
+	{0x01,  {   0,     0,    0,    0,    0,   LFE,   FR,   FL } },
+	{0x02,  {   0,     0,    0,    0,   FC,     0,   FR,   FL } },
+	{0x03,  {   0,     0,    0,    0,   FC,   LFE,   FR,   FL } },
+	{0x04,  {   0,     0,    0,   RC,    0,     0,   FR,   FL } },
+	{0x05,  {   0,     0,    0,   RC,    0,   LFE,   FR,   FL } },
+	{0x06,  {   0,     0,    0,   RC,   FC,     0,   FR,   FL } },
+	{0x07,  {   0,     0,    0,   RC,   FC,   LFE,   FR,   FL } },
+	{0x08,  {   0,     0,   RR,   RL,    0,     0,   FR,   FL } },
+	{0x09,  {   0,     0,   RR,   RL,    0,   LFE,   FR,   FL } },
+	{0x0a,  {   0,     0,   RR,   RL,   FC,     0,   FR,   FL } },
+	{0x0b,  {   0,     0,   RR,   RL,   FC,   LFE,   FR,   FL } },
+	{0x0c,  {   0,    RC,   RR,   RL,    0,     0,   FR,   FL } },
+	{0x0d,  {   0,    RC,   RR,   RL,    0,   LFE,   FR,   FL } },
+	{0x0e,  {   0,    RC,   RR,   RL,   FC,     0,   FR,   FL } },
+	{0x0f,  {   0,    RC,   RR,   RL,   FC,   LFE,   FR,   FL } },
+	{0x10,  { RRC,   RLC,   RR,   RL,    0,     0,   FR,   FL } },
+	{0x11,  { RRC,   RLC,   RR,   RL,    0,   LFE,   FR,   FL } },
+	{0x12,  { RRC,   RLC,   RR,   RL,   FC,     0,   FR,   FL } },
+	{0x13,  { RRC,   RLC,   RR,   RL,   FC,   LFE,   FR,   FL } },
+	{0x14,  { FRC,   FLC,    0,    0,    0,     0,   FR,   FL } },
+	{0x15,  { FRC,   FLC,    0,    0,    0,   LFE,   FR,   FL } },
+	{0x16,  { FRC,   FLC,    0,    0,   FC,     0,   FR,   FL } },
+	{0x17,  { FRC,   FLC,    0,    0,   FC,   LFE,   FR,   FL } },
+	{0x18,  { FRC,   FLC,    0,   RC,    0,     0,   FR,   FL } },
+	{0x19,  { FRC,   FLC,    0,   RC,    0,   LFE,   FR,   FL } },
+	{0x1a,  { FRC,   FLC,    0,   RC,   FC,     0,   FR,   FL } },
+	{0x1b,  { FRC,   FLC,    0,   RC,   FC,   LFE,   FR,   FL } },
+	{0x1c,  { FRC,   FLC,   RR,   RL,    0,     0,   FR,   FL } },
+	{0x1d,  { FRC,   FLC,   RR,   RL,    0,   LFE,   FR,   FL } },
+	{0x1e,  { FRC,   FLC,   RR,   RL,   FC,     0,   FR,   FL } },
+	{0x1f,  { FRC,   FLC,   RR,   RL,   FC,   LFE,   FR,   FL } },
+	{0x20,  {   0,   FCH,   RR,   RL,   FC,     0,   FR,   FL } },
+	{0x21,  {   0,   FCH,   RR,   RL,   FC,   LFE,   FR,   FL } },
+	{0x22,  {  TC,     0,   RR,   RL,   FC,     0,   FR,   FL } },
+	{0x23,  {  TC,     0,   RR,   RL,   FC,   LFE,   FR,   FL } },
+	{0x24,  { FRH,   FLH,   RR,   RL,    0,     0,   FR,   FL } },
+	{0x25,  { FRH,   FLH,   RR,   RL,    0,   LFE,   FR,   FL } },
+	{0x26,  { FRW,   FLW,   RR,   RL,    0,     0,   FR,   FL } },
+	{0x27,  { FRW,   FLW,   RR,   RL,    0,   LFE,   FR,   FL } },
+	{0x28,  {  TC,    RC,   RR,   RL,   FC,     0,   FR,   FL } },
+	{0x29,  {  TC,    RC,   RR,   RL,   FC,   LFE,   FR,   FL } },
+	{0x2a,  { FCH,    RC,   RR,   RL,   FC,     0,   FR,   FL } },
+	{0x2b,  { FCH,    RC,   RR,   RL,   FC,   LFE,   FR,   FL } },
+	{0x2c,  {  TC,   FCH,   RR,   RL,   FC,     0,   FR,   FL } },
+	{0x2d,  {  TC,   FCH,   RR,   RL,   FC,   LFE,   FR,   FL } },
+	{0x2e,  { FRH,   FLH,   RR,   RL,   FC,     0,   FR,   FL } },
+	{0x2f,  { FRH,   FLH,   RR,   RL,   FC,   LFE,   FR,   FL } },
+	{0x30,  { FRW,   FLW,   RR,   RL,   FC,     0,   FR,   FL } },
+	{0x31,  { FRW,   FLW,   RR,   RL,   FC,   LFE,   FR,   FL } },
+};
+
+/*
  * HDMI routines
  */
 
@@ -260,6 +372,79 @@ static void hdmi_fill_audio_infoframe(st
 		hdmi_write_dip_byte(codec, PIN_NID, params[i]);
 }
 
+/*
+ * Compute derived values in channel_allocations[].
+ */
+static void init_channel_allocations(void)
+{
+	int i, j;
+	struct cea_channel_speaker_allocation *p;
+
+	for (i = 0; i < ARRAY_SIZE(channel_allocations); i++) {
+		p = channel_allocations + i;
+		for (j = 0; j < ARRAY_SIZE(p->speakers); j++)
+			if (p->speakers[j]) {
+				p->channels++;
+				p->spk_mask |= p->speakers[j];
+			}
+	}
+}
+
+/*
+ * The transformation takes two steps:
+ *
+ * 	eld->spk_alloc => (eld_speaker_allocation_bits[]) => spk_mask
+ * 	      spk_mask => (channel_allocations[])         => ai->CA
+ *
+ * TODO: it could select the wrong CA from multiple candidates.
+*/
+static int hdmi_setup_channel_allocation(struct hda_codec *codec,
+					 struct hdmi_audio_infoframe *ai)
+{
+	struct intel_hdmi_spec *spec = codec->spec;
+	struct sink_eld *eld = &spec->sink;
+	int i;
+	int spk_mask = 0;
+	int channels = 1 + (ai->CC02_CT47 & 0x7);
+	char buf[SND_PRINT_CHANNEL_ALLOCATION_ADVISED_BUFSIZE];
+
+	/*
+	 * CA defaults to 0 for basic stereo audio
+	 */
+	if (!eld->eld_ver)
+		return 0;
+	if (!eld->spk_alloc)
+		return 0;
+	if (channels <= 2)
+		return 0;
+
+	/*
+	 * 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.
+	 */
+	for (i = 0; i < ARRAY_SIZE(eld_speaker_allocation_bits); i++) {
+		if (eld->spk_alloc & (1 << i))
+			spk_mask |= eld_speaker_allocation_bits[i];
+	}
+
+	/* search for the first working match in the CA table */
+	for (i = 0; i < ARRAY_SIZE(channel_allocations); i++) {
+		if (channels == channel_allocations[i].channels &&
+		    (spk_mask & channel_allocations[i].spk_mask) ==
+				channel_allocations[i].spk_mask) {
+			ai->CA = channel_allocations[i].ca_index;
+			return 0;
+		}
+	}
+
+	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;
+}
+
 static void hdmi_setup_audio_infoframe(struct hda_codec *codec,
 					struct snd_pcm_substream *substream)
 {
@@ -270,6 +455,8 @@ static void hdmi_setup_audio_infoframe(s
 		.CC02_CT47	= substream->runtime->channels - 1,
 	};
 
+	hdmi_setup_channel_allocation(codec, &ai);
+
 	hdmi_fill_audio_infoframe(codec, &ai);
 }
 
@@ -455,6 +642,8 @@ static int patch_intel_hdmi(struct hda_c
 
 	snd_hda_eld_proc_new(codec, &spec->sink);
 
+	init_channel_allocations();
+
 	return 0;
 }
 

-- 

[PATCH 4/4] hda: HDMI channel mapping cleanups

[Wu Fengguang]

[-- Attachment #1: alsa-speakers2.patch --]
[-- Type: text/plain, Size: 2369 bytes --]

Refactor the channel mapping code for consistent naming
and make it more informed about channel allocations.

It still does nothing. Some blocking questions:
- how do we know ALSA audio stream's channel allocation?
	- ALSA sequence is front/surr/clfe/side?
	- what's the ALSA sequence for 3-9 channels?
- how do we map ALSA speakers(if available) to CEA speakers?
- Intel G45's channel mapping get verb seems to always return 0.
  that could be a trouble.

Signed-off-by: Wu Fengguang <wfg@linux.intel.com>
---
 sound/pci/hda/patch_intelhdmi.c |   29 ++++++++++++++++++-----------
 1 file changed, 18 insertions(+), 11 deletions(-)

--- sound-2.6.orig/sound/pci/hda/patch_intelhdmi.c
+++ sound-2.6/sound/pci/hda/patch_intelhdmi.c
@@ -276,7 +276,7 @@ static void hdmi_set_channel_count(struc
 				chs, hdmi_get_channel_count(codec));
 }
 
-static void hdmi_debug_slot_mapping(struct hda_codec *codec)
+static void hdmi_debug_channel_mapping(struct hda_codec *codec)
 {
 #ifdef CONFIG_SND_DEBUG_VERBOSE
 	int i;
@@ -291,13 +291,6 @@ static void hdmi_debug_slot_mapping(stru
 #endif
 }
 
-static void hdmi_setup_channel_mapping(struct hda_codec *codec)
-{
-	snd_hda_sequence_write(codec, def_chan_map);
-	hdmi_debug_slot_mapping(codec);
-}
-
-
 static void hdmi_parse_eld(struct hda_codec *codec)
 {
 	struct intel_hdmi_spec *spec = codec->spec;
@@ -445,6 +438,22 @@ static int hdmi_setup_channel_allocation
 	return -1;
 }
 
+static void hdmi_setup_channel_mapping(struct hda_codec *codec,
+					struct hdmi_audio_infoframe *ai)
+{
+	if (!ai->CA)
+		return;
+
+	/*
+	 * TODO: adjust channel mapping if necessary
+	 * ALSA sequence is front/surr/clfe/side?
+	 */
+
+	snd_hda_sequence_write(codec, def_chan_map);
+	hdmi_debug_channel_mapping(codec);
+}
+
+
 static void hdmi_setup_audio_infoframe(struct hda_codec *codec,
 					struct snd_pcm_substream *substream)
 {
@@ -456,6 +465,7 @@ static void hdmi_setup_audio_infoframe(s
 	};
 
 	hdmi_setup_channel_allocation(codec, &ai);
+	hdmi_setup_channel_mapping(codec, &ai);
 
 	hdmi_fill_audio_infoframe(codec, &ai);
 }
@@ -553,9 +563,6 @@ static int intel_hdmi_playback_pcm_prepa
 
 	hdmi_set_channel_count(codec, substream->runtime->channels);
 
-	/* wfg: channel mapping not supported by DEVCTG */
-	hdmi_setup_channel_mapping(codec);
-
 	hdmi_setup_audio_infoframe(codec, substream);
 
 	hdmi_enable_output(codec);

-- 

Re: [PATCH 3/4] hda: HDMI channel allocations for audio infoframe

[Takashi Iwai]

At Tue, 18 Nov 2008 16:57:16 +0800,
Wu Fengguang wrote:
> 
> To play a 3+ channels LPCM/DSD stream via HDMI,
> 
> 	- HDMI sink must tell HDMI source about its speaker placements
> 	  (via ELD, speaker-allocation field)
> 	- HDMI source must tell the HDMI sink about channel allocation
> 	  (via audio infoframe, channel-allocation field)
> 
> (related docs: HDMI 1.3a spec section 7.4, CEA-861-D section 7.5.3 and 6.6)
> 
> This patch attempts to set the CA(channel-allocation) byte in the audio infoframe
> according to
> 	- the number of channels in the current stream
> 	- the speakers attached to the HDMI sink
> 
> A channel_allocations[] line must meet the following two criterions to be
> considered as a valid candidate for CA:
> 	1) its number of allocated channels = substream->runtime->channels
> 	2) its speakers are a subset of the available ones on the sink side
> 
> If there are multiple candidates, the first one is selected.  This simple
> policy shall cheat the sink into playing music, but may direct data to the
> wrong speakers.
> 
> Sorry, this last step is not obvious to me. Any domain experts, please?

The problem is that the speaker positioning isn't defined in the ALSA
API yet.  I think your implementation will work in practice.

> +static struct cea_channel_speaker_allocation channel_allocations[] = {
> +/* channel number:  8      7     6     5     4      3     2     1 */

Do we need to set ca_index explicitly?
Any case ca_index != array index?

> +static void init_channel_allocations(void)
> +{
> +	int i, j;
> +	struct cea_channel_speaker_allocation *p;
> +
> +	for (i = 0; i < ARRAY_SIZE(channel_allocations); i++) {
> +		p = channel_allocations + i;

You should zero-initialize p->channels and p->spk_mask here...

> +		for (j = 0; j < ARRAY_SIZE(p->speakers); j++)
> +			if (p->speakers[j]) {
> +				p->channels++;
> +				p->spk_mask |= p->speakers[j];
> +			}
> +	}

... because patch_intel_hdmi() may be called multiple times (either
multiple codecs or reconfiguration via hwdep) as below.

> @@ -455,6 +642,8 @@ static int patch_intel_hdmi(struct hda_c
>  
>  	snd_hda_eld_proc_new(codec, &spec->sink);
>  
> +	init_channel_allocations();
> +
>  	return 0;
>  }


thanks,

Takashi

Re: [PATCH 0/4] [RFC] multi-channel HDMI audio support

[Takashi Iwai]

At Tue, 18 Nov 2008 16:57:13 +0800,
Wu Fengguang wrote:
> 
> Hi all,
> 
> This patchset attempts to support multi-channel HDMI audio.
> 
> It contains two basic operations:
> 
> - setup the CA(channel-allcation) byte in the audio infoframe
> - setup the converter channel to HDMI slot mapping
> 
> Please check the last two patches for the detailed descriptions:
> 
> 	[PATCH 1/4] hda: make standalone hdmi_fill_audio_infoframe()                               
> 	[PATCH 2/4] hda: make global snd_print_channel_allocation()                       
> 
> 	[PATCH 3/4] hda: HDMI channel allocations for audio infoframe                               
> 	[PATCH 4/4] hda: HDMI channel mapping cleanups                                               
> 
> The current implementation only does minimal works that can ensure the
> multi-channel audio playing out. However the wrong speakers may be choosed, and
> I really need more information/knowledge/instructions from you to get it right.

As mentioned in the reply to patch#3, the speaker positioning is
basically an unimplemented feature.  So, let's make it as simple as
possible right now.

There are two things we need to implement about speaker positioning:
- return PCM stream <-> speaker position map
- allow to choose different speaker model

The first one could be implemented likely using an extention to the
PCM with (a sort of) TLV entry at best.  Once the TLV format and ioctl
is defined, the rest of implementation would be relatively easy.

I still have no concrete idea how to implement the second item.  One
easy solution is to implement it as a control (e.g. mixer) element.
In this case, the implementation is pretty driver-specific, and no
change in the ALSA core side is needed.


BTW, the rest of patches look good to me.


thanks,

Takashi

Re: [PATCH 3/4] hda: HDMI channel allocations for audio infoframe

[Wu Fengguang]

On Tue, Nov 18, 2008 at 11:57:08AM +0100, Takashi Iwai wrote:
> At Tue, 18 Nov 2008 16:57:16 +0800,
> Wu Fengguang wrote:
> > 
> > To play a 3+ channels LPCM/DSD stream via HDMI,
> > 
> > 	- HDMI sink must tell HDMI source about its speaker placements
> > 	  (via ELD, speaker-allocation field)
> > 	- HDMI source must tell the HDMI sink about channel allocation
> > 	  (via audio infoframe, channel-allocation field)
> > 
> > (related docs: HDMI 1.3a spec section 7.4, CEA-861-D section 7.5.3 and 6.6)
> > 
> > This patch attempts to set the CA(channel-allocation) byte in the audio infoframe
> > according to
> > 	- the number of channels in the current stream
> > 	- the speakers attached to the HDMI sink
> > 
> > A channel_allocations[] line must meet the following two criterions to be
> > considered as a valid candidate for CA:
> > 	1) its number of allocated channels = substream->runtime->channels
> > 	2) its speakers are a subset of the available ones on the sink side
> > 
> > If there are multiple candidates, the first one is selected.  This simple
> > policy shall cheat the sink into playing music, but may direct data to the
> > wrong speakers.
> > 
> > Sorry, this last step is not obvious to me. Any domain experts, please?
> 
> The problem is that the speaker positioning isn't defined in the ALSA
> API yet.  I think your implementation will work in practice.

OK. Let's forget it until (possibly) some one complains ;-)

> > +static struct cea_channel_speaker_allocation channel_allocations[] = {
> > +/* channel number:  8      7     6     5     4      3     2     1 */
> 
> Do we need to set ca_index explicitly?
> Any case ca_index != array index?

Yes, since we stop at the first matching item, the lines could be
reordered somewhere to give priority to some popular ones.

> > +static void init_channel_allocations(void)
> > +{
> > +	int i, j;
> > +	struct cea_channel_speaker_allocation *p;
> > +
> > +	for (i = 0; i < ARRAY_SIZE(channel_allocations); i++) {
> > +		p = channel_allocations + i;
> 
> You should zero-initialize p->channels and p->spk_mask here...
> 
> > +		for (j = 0; j < ARRAY_SIZE(p->speakers); j++)
> > +			if (p->speakers[j]) {
> > +				p->channels++;
> > +				p->spk_mask |= p->speakers[j];
> > +			}
> > +	}
> 
> ... because patch_intel_hdmi() may be called multiple times (either
> multiple codecs or reconfiguration via hwdep) as below.

Got it, thanks.

> > @@ -455,6 +642,8 @@ static int patch_intel_hdmi(struct hda_c
> >  
> >  	snd_hda_eld_proc_new(codec, &spec->sink);
> >  
> > +	init_channel_allocations();
> > +
> >  	return 0;
> >  }

Thank you,
Fengguang

Re: [PATCH 3/4] hda: HDMI channel allocations for audio infoframe

[Takashi Iwai]

At Tue, 18 Nov 2008 19:08:18 +0800,
Wu Fengguang wrote:
> 
> On Tue, Nov 18, 2008 at 11:57:08AM +0100, Takashi Iwai wrote:
> > At Tue, 18 Nov 2008 16:57:16 +0800,
> > Wu Fengguang wrote:
> > > 
> > > To play a 3+ channels LPCM/DSD stream via HDMI,
> > > 
> > > 	- HDMI sink must tell HDMI source about its speaker placements
> > > 	  (via ELD, speaker-allocation field)
> > > 	- HDMI source must tell the HDMI sink about channel allocation
> > > 	  (via audio infoframe, channel-allocation field)
> > > 
> > > (related docs: HDMI 1.3a spec section 7.4, CEA-861-D section 7.5.3 and 6.6)
> > > 
> > > This patch attempts to set the CA(channel-allocation) byte in the audio infoframe
> > > according to
> > > 	- the number of channels in the current stream
> > > 	- the speakers attached to the HDMI sink
> > > 
> > > A channel_allocations[] line must meet the following two criterions to be
> > > considered as a valid candidate for CA:
> > > 	1) its number of allocated channels = substream->runtime->channels
> > > 	2) its speakers are a subset of the available ones on the sink side
> > > 
> > > If there are multiple candidates, the first one is selected.  This simple
> > > policy shall cheat the sink into playing music, but may direct data to the
> > > wrong speakers.
> > > 
> > > Sorry, this last step is not obvious to me. Any domain experts, please?
> > 
> > The problem is that the speaker positioning isn't defined in the ALSA
> > API yet.  I think your implementation will work in practice.
> 
> OK. Let's forget it until (possibly) some one complains ;-)
> 
> > > +static struct cea_channel_speaker_allocation channel_allocations[] = {
> > > +/* channel number:  8      7     6     5     4      3     2     1 */
> > 
> > Do we need to set ca_index explicitly?
> > Any case ca_index != array index?
> 
> Yes, since we stop at the first matching item, the lines could be
> reordered somewhere to give priority to some popular ones.

Fair enough.  One thing I forgot is the initialization style.
Try to initialize in C99 style, something like

static struct cea_channel_speaker_allocation channel_allocations[] = {
	{ .ca_index = 0x00,
	  .speakers = { ..... } },   
	{ .ca_index = 0x01,
	  .speakers = { ..... } },   
	...
};

You can indent as you like, though.


thanks,

Takashi

Re: [PATCH 3/4] hda: HDMI channel allocations for audio infoframe

[Wu Fengguang]

On Tue, Nov 18, 2008 at 12:12:03PM +0100, Takashi Iwai wrote:
> At Tue, 18 Nov 2008 19:08:18 +0800,
> Wu Fengguang wrote:
> > 
> > On Tue, Nov 18, 2008 at 11:57:08AM +0100, Takashi Iwai wrote:
> > > At Tue, 18 Nov 2008 16:57:16 +0800,
> > > Wu Fengguang wrote:
> > > > 
> > > > To play a 3+ channels LPCM/DSD stream via HDMI,
> > > > 
> > > > 	- HDMI sink must tell HDMI source about its speaker placements
> > > > 	  (via ELD, speaker-allocation field)
> > > > 	- HDMI source must tell the HDMI sink about channel allocation
> > > > 	  (via audio infoframe, channel-allocation field)
> > > > 
> > > > (related docs: HDMI 1.3a spec section 7.4, CEA-861-D section 7.5.3 and 6.6)
> > > > 
> > > > This patch attempts to set the CA(channel-allocation) byte in the audio infoframe
> > > > according to
> > > > 	- the number of channels in the current stream
> > > > 	- the speakers attached to the HDMI sink
> > > > 
> > > > A channel_allocations[] line must meet the following two criterions to be
> > > > considered as a valid candidate for CA:
> > > > 	1) its number of allocated channels = substream->runtime->channels
> > > > 	2) its speakers are a subset of the available ones on the sink side
> > > > 
> > > > If there are multiple candidates, the first one is selected.  This simple
> > > > policy shall cheat the sink into playing music, but may direct data to the
> > > > wrong speakers.
> > > > 
> > > > Sorry, this last step is not obvious to me. Any domain experts, please?
> > > 
> > > The problem is that the speaker positioning isn't defined in the ALSA
> > > API yet.  I think your implementation will work in practice.
> > 
> > OK. Let's forget it until (possibly) some one complains ;-)
> > 
> > > > +static struct cea_channel_speaker_allocation channel_allocations[] = {
> > > > +/* channel number:  8      7     6     5     4      3     2     1 */
> > > 
> > > Do we need to set ca_index explicitly?
> > > Any case ca_index != array index?
> > 
> > Yes, since we stop at the first matching item, the lines could be
> > reordered somewhere to give priority to some popular ones.
> 
> Fair enough.  One thing I forgot is the initialization style.
> Try to initialize in C99 style, something like
> 
> static struct cea_channel_speaker_allocation channel_allocations[] = {
> 	{ .ca_index = 0x00,
> 	  .speakers = { ..... } },   
> 	{ .ca_index = 0x01,
> 	  .speakers = { ..... } },   
> 	...
> };
> 
> You can indent as you like, though.

OK, they still fits nicely into one line :-)

        { .ca_index = 0x10, .speakers = { RRC, RLC, RR, RL,  0,   0, FR, FL } },
        { .ca_index = 0x11, .speakers = { RRC, RLC, RR, RL,  0, LFE, FR, FL } },
        { .ca_index = 0x12, .speakers = { RRC, RLC, RR, RL, FC,   0, FR, FL } },
        { .ca_index = 0x13, .speakers = { RRC, RLC, RR, RL, FC, LFE, FR, FL } },
        { .ca_index = 0x14, .speakers = { FRC, FLC,  0,  0,  0,   0, FR, FL } },
        { .ca_index = 0x15, .speakers = { FRC, FLC,  0,  0,  0, LFE, FR, FL } },
        { .ca_index = 0x16, .speakers = { FRC, FLC,  0,  0, FC,   0, FR, FL } },
        { .ca_index = 0x17, .speakers = { FRC, FLC,  0,  0, FC, LFE, FR, FL } },
        { .ca_index = 0x18, .speakers = { FRC, FLC,  0, RC,  0,   0, FR, FL } },
        { .ca_index = 0x19, .speakers = { FRC, FLC,  0, RC,  0, LFE, FR, FL } },
        { .ca_index = 0x1a, .speakers = { FRC, FLC,  0, RC, FC,   0, FR, FL } },
        { .ca_index = 0x1b, .speakers = { FRC, FLC,  0, RC, FC, LFE, FR, FL } },

But it's a bit too crowded, and I feel better with this one:

{ .ca_index = 0x14,  .speakers = { FRC,  FLC,   0,   0,   0,    0,  FR,  FL } },
{ .ca_index = 0x15,  .speakers = { FRC,  FLC,   0,   0,   0,  LFE,  FR,  FL } },
{ .ca_index = 0x16,  .speakers = { FRC,  FLC,   0,   0,  FC,    0,  FR,  FL } },
{ .ca_index = 0x17,  .speakers = { FRC,  FLC,   0,   0,  FC,  LFE,  FR,  FL } },
{ .ca_index = 0x18,  .speakers = { FRC,  FLC,   0,  RC,   0,    0,  FR,  FL } },
{ .ca_index = 0x19,  .speakers = { FRC,  FLC,   0,  RC,   0,  LFE,  FR,  FL } },
{ .ca_index = 0x1a,  .speakers = { FRC,  FLC,   0,  RC,  FC,    0,  FR,  FL } },
{ .ca_index = 0x1b,  .speakers = { FRC,  FLC,   0,  RC,  FC,  LFE,  FR,  FL } },
{ .ca_index = 0x1c,  .speakers = { FRC,  FLC,  RR,  RL,   0,    0,  FR,  FL } },
{ .ca_index = 0x1d,  .speakers = { FRC,  FLC,  RR,  RL,   0,  LFE,  FR,  FL } },

[PATCH 1/4] hda: make standalone hdmi_fill_audio_infoframe()

[Wu Fengguang]

[-- Attachment #1: alsa-infoframe-cleanup.patch --]
[-- Type: text/plain, Size: 1418 bytes --]

code refactor: make a standalone function hdmi_fill_audio_infoframe().

Signed-off-by: Wu Fengguang <wfg@linux.intel.com>
---
 sound/pci/hda/patch_intelhdmi.c |   25 ++++++++++++++++---------
 1 file changed, 16 insertions(+), 9 deletions(-)

--- sound-2.6.orig/sound/pci/hda/patch_intelhdmi.c
+++ sound-2.6/sound/pci/hda/patch_intelhdmi.c
@@ -246,24 +246,31 @@ static void hdmi_clear_dip_buffers(struc
 #endif
 }
 
+static void hdmi_fill_audio_infoframe(struct hda_codec *codec,
+					struct hdmi_audio_infoframe *ai)
+{
+	u8 *params = (u8 *)ai;
+	int i;
+
+	hdmi_debug_dip_size(codec);
+	hdmi_clear_dip_buffers(codec); /* be paranoid */
+
+	hdmi_set_dip_index(codec, PIN_NID, 0x0, 0x0);
+	for (i = 0; i < sizeof(ai); i++)
+		hdmi_write_dip_byte(codec, PIN_NID, params[i]);
+}
+
 static void hdmi_setup_audio_infoframe(struct hda_codec *codec,
 					struct snd_pcm_substream *substream)
 {
-	struct hdmi_audio_infoframe audio_infoframe = {
+	struct hdmi_audio_infoframe ai = {
 		.type		= 0x84,
 		.ver		= 0x01,
 		.len		= 0x0a,
 		.CC02_CT47	= substream->runtime->channels - 1,
 	};
-	u8 *params = (u8 *)&audio_infoframe;
-	int i;
-
-	hdmi_debug_dip_size(codec);
-	hdmi_clear_dip_buffers(codec); /* be paranoid */
 
-	hdmi_set_dip_index(codec, PIN_NID, 0x0, 0x0);
-	for (i = 0; i < sizeof(audio_infoframe); i++)
-		hdmi_write_dip_byte(codec, PIN_NID, params[i]);
+	hdmi_fill_audio_infoframe(codec, &ai);
 }
 
 

--