[PATCH 0/4] Intel HDMI audio fixes

[PATCH 0/4] Intel HDMI audio fixes

[Wu Fengguang]

Hi Takashi,

Here are four assorted fixes for Intel HDMI audio.
They have been tested in G35 SDVO HDMI and G45 integrated HDMI systems,
and shall be OK for 2.6.29.

[PATCH 1/4] hda - allow multi-channel HDMI audio playback when ELD is not present
[PATCH 2/4] hda - enable HDMI audio pin out at module loading time
[PATCH 3/4] hda - compute checksum in HDMI audio infoframe
[PATCH 4/4] hda - add id for Intel IbexPeak integrated HDMI codec

Shane: the first two patches should address the two bugs you reported, i.e.
- multi-channel HDMI audio playback problem
- first half-second audio samples missing
These two bugs was reproduced and fixed in the combination of
G35 SDVO HDMI + Yamaha RX-V1800 AV receiver. The first problem cannot be
verified in G45 system because we still cannot make G45 work with
Yamaha. The second problem is interesting in that it can be fixed by the
second patch in G35, but still remains in G45.

Thanks,
Fengguang
--

[PATCH 1/4] hda - allow multi-channel HDMI audio playback when ELD is not present

[Wu Fengguang]

[-- Attachment #1: intel-hdmi-8channel.patch --]
[-- Type: text/plain, Size: 1306 bytes --]

The YAMAHA AV-X1800 requires audio infoframe to include speaker-channel
mapping to play >2 channel HDMI audio. In theory that mapping should be
derived from its speaker configurations contained in its ELD. However we
currently cannot get ELD in console before the KMS functionalities are ready.
This is a more or less general issue at least in the near future. As a
workaround, we propose to allow playback of mult-channel audio when ELD
is not available.

Signed-off-by: Wu Fengguang <fengguang.wu@intel.com>
---
 sound/pci/hda/patch_intelhdmi.c |   12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)

--- mm.orig/sound/pci/hda/patch_intelhdmi.c
+++ mm/sound/pci/hda/patch_intelhdmi.c
@@ -419,14 +419,18 @@ static int hdmi_setup_channel_allocation
 	/*
 	 * 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;
 
 	/*
+	 * HDMI sink's ELD info cannot always be retrieved for now, e.g.
+	 * in console or for audio devices. Assume the highest speakers
+	 * configuration, to _not_ prohibit multi-channel audio playback.
+	 */
+	if (!eld->spk_alloc)
+		eld->spk_alloc = 0xffff;
+
+	/*
 	 * expand ELD's speaker allocation mask
 	 *
 	 * ELD tells the speaker mask in a compact(paired) form,

-- 

[PATCH 2/4] hda - enable HDMI audio pin out at module loading time

[Wu Fengguang]

[-- Attachment #1: intel-hdmi-oneshot-enable.patch --]
[-- Type: text/plain, Size: 3519 bytes --]

We found that enabling/disabling HDMI audio pin out at stream start/stop
time will kill the leading 500ms or so sound samples. Avoid this by enabling
pin out once and for ever at module loading time.

The leading ~500ms audio samples will still be lost when switching from
X-channel playback to Y-channel playback where X != Y. However there's no
much we can do about it: the audio infoframe has to change and it looks like
either G45 or YAMAHA requires some time to switch the configuration.

Signed-off-by: Wu Fengguang <fengguang.wu@intel.com>
---
 sound/pci/hda/patch_intelhdmi.c |   42 +++++++++++++-----------------
 1 file changed, 19 insertions(+), 23 deletions(-)

--- mm.orig/sound/pci/hda/patch_intelhdmi.c
+++ mm/sound/pci/hda/patch_intelhdmi.c
@@ -49,11 +49,6 @@ static struct hda_verb pinout_enable_ver
 	{} /* terminator */
 };
 
-static struct hda_verb pinout_disable_verb[] = {
-	{PIN_NID, AC_VERB_SET_PIN_WIDGET_CONTROL, 0x00},
-	{}
-};
-
 static struct hda_verb unsolicited_response_verb[] = {
 	{PIN_NID, AC_VERB_SET_UNSOLICITED_ENABLE, AC_USRSP_EN |
 						  INTEL_HDMI_EVENT_TAG},
@@ -248,10 +243,6 @@ static void hdmi_write_dip_byte(struct h
 
 static void hdmi_enable_output(struct hda_codec *codec)
 {
-	/* Enable Audio InfoFrame Transmission */
-	hdmi_set_dip_index(codec, PIN_NID, 0x0, 0x0);
-	snd_hda_codec_write(codec, PIN_NID, 0, AC_VERB_SET_HDMI_DIP_XMIT,
-						AC_DIPXMIT_BEST);
 	/* Unmute */
 	if (get_wcaps(codec, PIN_NID) & AC_WCAP_OUT_AMP)
 		snd_hda_codec_write(codec, PIN_NID, 0,
@@ -260,17 +251,24 @@ static void hdmi_enable_output(struct hd
 	snd_hda_sequence_write(codec, pinout_enable_verb);
 }
 
-static void hdmi_disable_output(struct hda_codec *codec)
+/*
+ * Enable Audio InfoFrame Transmission
+ */
+static void hdmi_start_infoframe_trans(struct hda_codec *codec)
 {
-	snd_hda_sequence_write(codec, pinout_disable_verb);
-	if (get_wcaps(codec, PIN_NID) & AC_WCAP_OUT_AMP)
-		snd_hda_codec_write(codec, PIN_NID, 0,
-				AC_VERB_SET_AMP_GAIN_MUTE, AMP_OUT_MUTE);
+	hdmi_set_dip_index(codec, PIN_NID, 0x0, 0x0);
+	snd_hda_codec_write(codec, PIN_NID, 0, AC_VERB_SET_HDMI_DIP_XMIT,
+						AC_DIPXMIT_BEST);
+}
 
-	/*
-	 * FIXME: noises may arise when playing music after reloading the
-	 * kernel module, until the next X restart or monitor repower.
-	 */
+/*
+ * Disable Audio InfoFrame Transmission
+ */
+static void hdmi_stop_infoframe_trans(struct hda_codec *codec)
+{
+	hdmi_set_dip_index(codec, PIN_NID, 0x0, 0x0);
+	snd_hda_codec_write(codec, PIN_NID, 0, AC_VERB_SET_HDMI_DIP_XMIT,
+						AC_DIPXMIT_DISABLE);
 }
 
 static int hdmi_get_channel_count(struct hda_codec *codec)
@@ -489,6 +487,7 @@ static void hdmi_setup_audio_infoframe(s
 	hdmi_setup_channel_mapping(codec, &ai);
 
 	hdmi_fill_audio_infoframe(codec, &ai);
+	hdmi_start_infoframe_trans(codec);
 }
 
 
@@ -566,7 +565,7 @@ static int intel_hdmi_playback_pcm_close
 {
 	struct intel_hdmi_spec *spec = codec->spec;
 
-	hdmi_disable_output(codec);
+	hdmi_stop_infoframe_trans(codec);
 
 	return snd_hda_multi_out_dig_close(codec, &spec->multiout);
 }
@@ -586,8 +585,6 @@ static int intel_hdmi_playback_pcm_prepa
 
 	hdmi_setup_audio_infoframe(codec, substream);
 
-	hdmi_enable_output(codec);
-
 	return 0;
 }
 
@@ -632,8 +629,7 @@ static int intel_hdmi_build_controls(str
 
 static int intel_hdmi_init(struct hda_codec *codec)
 {
-	/* disable audio output as early as possible */
-	hdmi_disable_output(codec);
+	hdmi_enable_output(codec);
 
 	snd_hda_sequence_write(codec, unsolicited_response_verb);
 

-- 

[PATCH 3/4] hda - compute checksum in HDMI audio infoframe

[Wu Fengguang]

[-- Attachment #1: alsa-intel-hdmi-checksum.patch --]
[-- Type: text/plain, Size: 679 bytes --]

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

--- mm.orig/sound/pci/hda/patch_intelhdmi.c
+++ mm/sound/pci/hda/patch_intelhdmi.c
@@ -366,11 +366,16 @@ static void hdmi_fill_audio_infoframe(st
 					struct hdmi_audio_infoframe *ai)
 {
 	u8 *params = (u8 *)ai;
+	u8 sum = 0;
 	int i;
 
 	hdmi_debug_dip_size(codec);
 	hdmi_clear_dip_buffers(codec); /* be paranoid */
 
+	for (i = 0; i < sizeof(ai); i++)
+		sum += params[i];
+	ai->checksum = - sum;
+
 	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]);

-- 

[PATCH 4/4] hda - add id for Intel IbexPeak integrated HDMI codec

[Wu Fengguang]

[-- Attachment #1: intel-hdmi-ibexpeak.patch --]
[-- Type: text/plain, Size: 987 bytes --]

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

--- mm.orig/sound/pci/hda/patch_intelhdmi.c
+++ mm/sound/pci/hda/patch_intelhdmi.c
@@ -684,6 +684,7 @@ static struct hda_codec_preset snd_hda_p
 	{ .id = 0x80862801, .name = "G45 DEVBLC", .patch = patch_intel_hdmi },
 	{ .id = 0x80862802, .name = "G45 DEVCTG", .patch = patch_intel_hdmi },
 	{ .id = 0x80862803, .name = "G45 DEVELK", .patch = patch_intel_hdmi },
+	{ .id = 0x80862804, .name = "G45 DEVIBX", .patch = patch_intel_hdmi },
 	{ .id = 0x10951392, .name = "SiI1392 HDMI",     .patch = patch_intel_hdmi },
 	{} /* terminator */
 };
@@ -692,6 +693,7 @@ MODULE_ALIAS("snd-hda-codec-id:808629fb"
 MODULE_ALIAS("snd-hda-codec-id:80862801");
 MODULE_ALIAS("snd-hda-codec-id:80862802");
 MODULE_ALIAS("snd-hda-codec-id:80862803");
+MODULE_ALIAS("snd-hda-codec-id:80862804");
 MODULE_ALIAS("snd-hda-codec-id:10951392");
 
 MODULE_LICENSE("GPL");

-- 

Re: [PATCH 0/4] Intel HDMI audio fixes

[Takashi Iwai]

At Wed, 11 Feb 2009 15:22:27 +0800,
Wu Fengguang wrote:
> 
> Hi Takashi,
> 
> Here are four assorted fixes for Intel HDMI audio.
> They have been tested in G35 SDVO HDMI and G45 integrated HDMI systems,
> and shall be OK for 2.6.29.
> 
> [PATCH 1/4] hda - allow multi-channel HDMI audio playback when ELD is not present
> [PATCH 2/4] hda - enable HDMI audio pin out at module loading time
> [PATCH 3/4] hda - compute checksum in HDMI audio infoframe
> [PATCH 4/4] hda - add id for Intel IbexPeak integrated HDMI codec

Thanks, all look good, so applied now.
I'll queue them for the next pull request.


Takashi

> 
> Shane: the first two patches should address the two bugs you reported, i.e.
> - multi-channel HDMI audio playback problem
> - first half-second audio samples missing
> These two bugs was reproduced and fixed in the combination of
> G35 SDVO HDMI + Yamaha RX-V1800 AV receiver. The first problem cannot be
> verified in G45 system because we still cannot make G45 work with
> Yamaha. The second problem is interesting in that it can be fixed by the
> second patch in G35, but still remains in G45.


> 
> Thanks,
> Fengguang
> --
> 

Re: [PATCH 0/4] Intel HDMI audio fixes

[Wu Fengguang]

On Wed, Feb 11, 2009 at 10:10:55AM +0200, Takashi Iwai wrote:
> At Wed, 11 Feb 2009 15:22:27 +0800,
> Wu Fengguang wrote:
> > 
> > Hi Takashi,
> > 
> > Here are four assorted fixes for Intel HDMI audio.
> > They have been tested in G35 SDVO HDMI and G45 integrated HDMI systems,
> > and shall be OK for 2.6.29.
> > 
> > [PATCH 1/4] hda - allow multi-channel HDMI audio playback when ELD is not present
> > [PATCH 2/4] hda - enable HDMI audio pin out at module loading time
> > [PATCH 3/4] hda - compute checksum in HDMI audio infoframe
> > [PATCH 4/4] hda - add id for Intel IbexPeak integrated HDMI codec
> 
> Thanks, all look good, so applied now.
> I'll queue them for the next pull request.

Thank you!

Fengguang

Re: [PATCH 0/4] Intel HDMI audio fixes

[Takashi Iwai]

At Wed, 11 Feb 2009 14:32:41 -0800,
Shane W wrote:
> 
> On Wed, Feb 11, 2009 at 03:22:27PM +0800, Wu Fengguang wrote:
> > Shane: the first two patches should address the two bugs you reported, i.e.
> > - multi-channel HDMI audio playback problem
> > - first half-second audio samples missing
> > These two bugs was reproduced and fixed in the combination of
> > G35 SDVO HDMI + Yamaha RX-V1800 AV receiver. The first problem cannot be
> > verified in G45 system because we still cannot make G45 work with
> > Yamaha. The second problem is interesting in that it can be fixed by the
> > second patch in G35, but still remains in G45.
> 
> Hi,
> 
> Ok mixed results here.  I'm using this for my test signal:
> http://www.csy.ca/~shane/51test.wav
> 
> when using:
> aplay -D hw:0,3 51test.wav
> 
> Front and rear left and right are correct but centre is
> missing.  The order in the wav is:
> front left
> centre
> front right
> rear left
> rear right
> sub buz

The order assumed here doesn't match with ALSA mapping.
In ALSA,
  front left
  front right
  rear left
  rear right
  center
  LFE
  
Better to test with speaker-test program whether the mapping is
correct.


Takashi

Re: [PATCH 0/4] Intel HDMI audio fixes

[Takashi Iwai]

At Wed, 11 Feb 2009 14:54:48 -0800,
Shane W wrote:
> 
> On Wed, Feb 11, 2009 at 11:42:00PM +0100, Takashi Iwai wrote:
> > At Wed, 11 Feb 2009 14:32:41 -0800,
> > Shane W wrote:
> > > missing.  The order in the wav is:
> > > front left
> > > centre
> > > front right
> > > rear left
> > > rear right
> > > sub buz
> > 
> > The order assumed here doesn't match with ALSA mapping.
> > In ALSA,
> >   front left
> >   front right
> >   rear left
> >   rear right
> >   center
> >   LFE
> 
> Sorry I meant the spoken order, I've no idea what the
> channel mapping of a 6 channel wav is.
> 
> > Better to test with speaker-test program whether the mapping is
> > correct.
> 
> Ah didn't know about this program.  Ok results with:
> speaker-test -D hw:0,3 -s n
> where n ranged from 0 to 6
> 
> -s 1 speaker-test says front left actual is front left
> likewise with -s 2 for front right
> -s 3 speaker-test says should be rear left actual is lfe
> that is the sound came from lfe
> -s 4 speaker-test says rear right actual is centre
> -s 5 speaker-test says centre actual was rear left
> and -s 6 speaker-test says lfe actual is rear right

Thanks.  Also make sure that you pass -c 6 option.


Takashi

Re: [PATCH 0/4] Intel HDMI audio fixes

[Wu Fengguang]

On Thu, Feb 12, 2009 at 12:42:00AM +0200, Takashi Iwai wrote:
> At Wed, 11 Feb 2009 14:32:41 -0800,
> Shane W wrote:
> > 
> > On Wed, Feb 11, 2009 at 03:22:27PM +0800, Wu Fengguang wrote:
> > > Shane: the first two patches should address the two bugs you reported, i.e.
> > > - multi-channel HDMI audio playback problem
> > > - first half-second audio samples missing
> > > These two bugs was reproduced and fixed in the combination of
> > > G35 SDVO HDMI + Yamaha RX-V1800 AV receiver. The first problem cannot be
> > > verified in G45 system because we still cannot make G45 work with
> > > Yamaha. The second problem is interesting in that it can be fixed by the
> > > second patch in G35, but still remains in G45.
> > 
> > Hi,
> > 
> > Ok mixed results here.  I'm using this for my test signal:
> > http://www.csy.ca/~shane/51test.wav
> > 
> > when using:
> > aplay -D hw:0,3 51test.wav
> > 
> > Front and rear left and right are correct but centre is
> > missing.  The order in the wav is:
> > front left
> > centre
> > front right
> > rear left
> > rear right
> > sub buz
> 
> The order assumed here doesn't match with ALSA mapping.
> In ALSA,
>   front left
>   front right
>   rear left
>   rear right
>   center
>   LFE
>   
> Better to test with speaker-test program whether the mapping is
> correct.

Hi Takashi,

Yes I've not yet taken care of the channel orders, so disorders are expected.

Before beginning our discussion, I'd like to first submit a patch for
speaker-test. I guess the ordering in it defines the ALSA order.

Thanks,
Fengguang
---
speaker-test.c - add readability comments to speaker channels

Signed-off-by: Wu Fengguang <fengguang.wu@intel.com>
---

diff --git a/speaker-test/speaker-test.c b/speaker-test/speaker-test.c
index 57a7cbc..bf97d33 100644
--- a/speaker-test/speaker-test.c
+++ b/speaker-test/speaker-test.c
@@ -84,48 +84,48 @@ static const char *given_test_wav_file = NULL;
 static char *wav_file_dir = SOUNDSDIR;
 
 static const char        *channel_name[MAX_CHANNELS] = {
-  N_("Front Left"),
-  N_("Front Right"),
-  N_("Rear Left"),
-  N_("Rear Right"),
-  N_("Center"),
-  N_("LFE"),
-  N_("Side Left"),
-  N_("Side Right"),
-  N_("Channel 9"),
-  N_("Channel 10"),
-  N_("Channel 11"),
-  N_("Channel 12"),
-  N_("Channel 13"),
-  N_("Channel 14"),
-  N_("Channel 15"),
-  N_("Channel 16")
+  /*  0 */ N_("Front Left"),
+  /*  1 */ N_("Front Right"),
+  /*  2 */ N_("Rear Left"),
+  /*  3 */ N_("Rear Right"),
+  /*  4 */ N_("Center"),
+  /*  5 */ N_("LFE"),
+  /*  6 */ N_("Side Left"),
+  /*  7 */ N_("Side Right"),
+  /*  8 */ N_("Channel 9"),
+  /*  9 */ N_("Channel 10"),
+  /* 10 */ N_("Channel 11"),
+  /* 11 */ N_("Channel 12"),
+  /* 12 */ N_("Channel 13"),
+  /* 13 */ N_("Channel 14"),
+  /* 14 */ N_("Channel 15"),
+  /* 15 */ N_("Channel 16")
 };
 
 static const int	channels4[] = {
-  0,
-  1,
-  3,
-  2
+  0, /* Front Left  */
+  1, /* Front Right */
+  3, /* Rear Right  */
+  2, /* Rear Left   */
 };
 static const int	channels6[] = {
-  0,
-  4,
-  1,
-  3,
-  2,
-  5
-}; 
+  0, /* Front Left  */
+  4, /* Center      */
+  1, /* Front Right */
+  3, /* Rear Right  */
+  2, /* Rear Left   */
+  5, /* LFE         */
+};
 static const int	channels8[] = {
-  0,
-  4,
-  1,
-  7,
-  3,
-  2,
-  6,
-  5
-}; 
+  0, /* Front Left  */
+  4, /* Center      */
+  1, /* Front Right */
+  7, /* Side Right  */
+  3, /* Rear Right  */
+  2, /* Rear Left   */
+  6, /* Side Left   */
+  5, /* LFE         */
+};
 
 static void generate_sine(uint8_t *frames, int channel, int count, double *_phase) {
   double phase = *_phase;

Re: [PATCH 0/4] Intel HDMI audio fixes

[Wu Fengguang]

On Thu, Feb 12, 2009 at 01:02:41AM +0200, Takashi Iwai wrote:
> At Wed, 11 Feb 2009 14:54:48 -0800,
> Shane W wrote:
> > 
> > On Wed, Feb 11, 2009 at 11:42:00PM +0100, Takashi Iwai wrote:
> > > At Wed, 11 Feb 2009 14:32:41 -0800,
> > > Shane W wrote:
> > > > missing.  The order in the wav is:
> > > > front left
> > > > centre
> > > > front right
> > > > rear left
> > > > rear right
> > > > sub buz
> > > 
> > > The order assumed here doesn't match with ALSA mapping.
> > > In ALSA,
> > >   front left
> > >   front right
> > >   rear left
> > >   rear right
> > >   center
> > >   LFE
> > 
> > Sorry I meant the spoken order, I've no idea what the
> > channel mapping of a 6 channel wav is.
> > 
> > > Better to test with speaker-test program whether the mapping is
> > > correct.
> > 
> > Ah didn't know about this program.  Ok results with:
> > speaker-test -D hw:0,3 -s n
> > where n ranged from 0 to 6
> > 
> > -s 1 speaker-test says front left actual is front left
> > likewise with -s 2 for front right
> > -s 3 speaker-test says should be rear left actual is lfe
> > that is the sound came from lfe
> > -s 4 speaker-test says rear right actual is centre
> > -s 5 speaker-test says centre actual was rear left
> > and -s 6 speaker-test says lfe actual is rear right
> 
> Thanks.  Also make sure that you pass -c 6 option.

If I understand it right, the ALSA channel sequence is defined in
speaker-test as:

        static const char        *channel_name[MAX_CHANNELS] = {
          /*  0 */ N_("Front Left"),
          /*  1 */ N_("Front Right"),
          /*  2 */ N_("Rear Left"),
          /*  3 */ N_("Rear Right"),
          /*  4 */ N_("Center"),       /* wav file is "Front Center.wav" */
          /*  5 */ N_("LFE"),          /* wav file is "Rear Center.wav" */
          /*  6 */ N_("Side Left"),
          /*  7 */ N_("Side Right"),

Whereas the HDMI channel mapping selected for 6-channel playback is

        { .ca_index = 0x0b,  .speakers = {   0,    0,  RR,  RL,  FC,  LFE,  FR,  FL } },

Here the two mappings disagree on channels other than Front Left/Right.

In theory the channel mapping should be adjusted via AC_VERB_SET_HDMI_CHAN_SLOT.
However I found that verb takes no effect for G35/G45. So I'm afraid
there's no trivial ways we can adjust the channel mapping.

Thanks,
Fengguang

Re: [PATCH 0/4] Intel HDMI audio fixes

[Takashi Iwai]

At Thu, 12 Feb 2009 09:25:04 +0800,
Wu Fengguang wrote:
> 
> On Thu, Feb 12, 2009 at 12:42:00AM +0200, Takashi Iwai wrote:
> > At Wed, 11 Feb 2009 14:32:41 -0800,
> > Shane W wrote:
> > > 
> > > On Wed, Feb 11, 2009 at 03:22:27PM +0800, Wu Fengguang wrote:
> > > > Shane: the first two patches should address the two bugs you reported, i.e.
> > > > - multi-channel HDMI audio playback problem
> > > > - first half-second audio samples missing
> > > > These two bugs was reproduced and fixed in the combination of
> > > > G35 SDVO HDMI + Yamaha RX-V1800 AV receiver. The first problem cannot be
> > > > verified in G45 system because we still cannot make G45 work with
> > > > Yamaha. The second problem is interesting in that it can be fixed by the
> > > > second patch in G35, but still remains in G45.
> > > 
> > > Hi,
> > > 
> > > Ok mixed results here.  I'm using this for my test signal:
> > > http://www.csy.ca/~shane/51test.wav
> > > 
> > > when using:
> > > aplay -D hw:0,3 51test.wav
> > > 
> > > Front and rear left and right are correct but centre is
> > > missing.  The order in the wav is:
> > > front left
> > > centre
> > > front right
> > > rear left
> > > rear right
> > > sub buz
> > 
> > The order assumed here doesn't match with ALSA mapping.
> > In ALSA,
> >   front left
> >   front right
> >   rear left
> >   rear right
> >   center
> >   LFE
> >   
> > Better to test with speaker-test program whether the mapping is
> > correct.
> 
> Hi Takashi,
> 
> Yes I've not yet taken care of the channel orders, so disorders are expected.
> 
> Before beginning our discussion, I'd like to first submit a patch for
> speaker-test. I guess the ordering in it defines the ALSA order.

The change looks good but the patch is rejected.
Could you create a patch based on the latest version?


thanks,

Takashi

> 
> Thanks,
> Fengguang
> ---
> speaker-test.c - add readability comments to speaker channels
> 
> Signed-off-by: Wu Fengguang <fengguang.wu@intel.com>
> ---
> 
> diff --git a/speaker-test/speaker-test.c b/speaker-test/speaker-test.c
> index 57a7cbc..bf97d33 100644
> --- a/speaker-test/speaker-test.c
> +++ b/speaker-test/speaker-test.c
> @@ -84,48 +84,48 @@ static const char *given_test_wav_file = NULL;
>  static char *wav_file_dir = SOUNDSDIR;
>  
>  static const char        *channel_name[MAX_CHANNELS] = {
> -  N_("Front Left"),
> -  N_("Front Right"),
> -  N_("Rear Left"),
> -  N_("Rear Right"),
> -  N_("Center"),
> -  N_("LFE"),
> -  N_("Side Left"),
> -  N_("Side Right"),
> -  N_("Channel 9"),
> -  N_("Channel 10"),
> -  N_("Channel 11"),
> -  N_("Channel 12"),
> -  N_("Channel 13"),
> -  N_("Channel 14"),
> -  N_("Channel 15"),
> -  N_("Channel 16")
> +  /*  0 */ N_("Front Left"),
> +  /*  1 */ N_("Front Right"),
> +  /*  2 */ N_("Rear Left"),
> +  /*  3 */ N_("Rear Right"),
> +  /*  4 */ N_("Center"),
> +  /*  5 */ N_("LFE"),
> +  /*  6 */ N_("Side Left"),
> +  /*  7 */ N_("Side Right"),
> +  /*  8 */ N_("Channel 9"),
> +  /*  9 */ N_("Channel 10"),
> +  /* 10 */ N_("Channel 11"),
> +  /* 11 */ N_("Channel 12"),
> +  /* 12 */ N_("Channel 13"),
> +  /* 13 */ N_("Channel 14"),
> +  /* 14 */ N_("Channel 15"),
> +  /* 15 */ N_("Channel 16")
>  };
>  
>  static const int	channels4[] = {
> -  0,
> -  1,
> -  3,
> -  2
> +  0, /* Front Left  */
> +  1, /* Front Right */
> +  3, /* Rear Right  */
> +  2, /* Rear Left   */
>  };
>  static const int	channels6[] = {
> -  0,
> -  4,
> -  1,
> -  3,
> -  2,
> -  5
> -}; 
> +  0, /* Front Left  */
> +  4, /* Center      */
> +  1, /* Front Right */
> +  3, /* Rear Right  */
> +  2, /* Rear Left   */
> +  5, /* LFE         */
> +};
>  static const int	channels8[] = {
> -  0,
> -  4,
> -  1,
> -  7,
> -  3,
> -  2,
> -  6,
> -  5
> -}; 
> +  0, /* Front Left  */
> +  4, /* Center      */
> +  1, /* Front Right */
> +  7, /* Side Right  */
> +  3, /* Rear Right  */
> +  2, /* Rear Left   */
> +  6, /* Side Left   */
> +  5, /* LFE         */
> +};
>  
>  static void generate_sine(uint8_t *frames, int channel, int count, double *_phase) {
>    double phase = *_phase;
> 

Re: [PATCH 0/4] Intel HDMI audio fixes

[Wu Fengguang]

On Thu, Feb 12, 2009 at 09:54:29AM +0200, Takashi Iwai wrote:
> At Thu, 12 Feb 2009 09:25:04 +0800,
> Wu Fengguang wrote:
> > 
> > Before beginning our discussion, I'd like to first submit a patch for
> > speaker-test. I guess the ordering in it defines the ALSA order.
> 
> The change looks good but the patch is rejected.
> Could you create a patch based on the latest version?
 
OK. I'll resubmit it sometime later, because it's not convenient for now
to do the git pull inside company firewall...

Thanks,
Fengguang

> > ---
> > speaker-test.c - add readability comments to speaker channels
> > 
> > Signed-off-by: Wu Fengguang <fengguang.wu@intel.com>
> > ---
> > 
> > diff --git a/speaker-test/speaker-test.c b/speaker-test/speaker-test.c
> > index 57a7cbc..bf97d33 100644
> > --- a/speaker-test/speaker-test.c
> > +++ b/speaker-test/speaker-test.c
> > @@ -84,48 +84,48 @@ static const char *given_test_wav_file = NULL;
> >  static char *wav_file_dir = SOUNDSDIR;
> >  
> >  static const char        *channel_name[MAX_CHANNELS] = {
> > -  N_("Front Left"),
> > -  N_("Front Right"),
> > -  N_("Rear Left"),
> > -  N_("Rear Right"),
> > -  N_("Center"),
> > -  N_("LFE"),
> > -  N_("Side Left"),
> > -  N_("Side Right"),
> > -  N_("Channel 9"),
> > -  N_("Channel 10"),
> > -  N_("Channel 11"),
> > -  N_("Channel 12"),
> > -  N_("Channel 13"),
> > -  N_("Channel 14"),
> > -  N_("Channel 15"),
> > -  N_("Channel 16")
> > +  /*  0 */ N_("Front Left"),
> > +  /*  1 */ N_("Front Right"),
> > +  /*  2 */ N_("Rear Left"),
> > +  /*  3 */ N_("Rear Right"),
> > +  /*  4 */ N_("Center"),
> > +  /*  5 */ N_("LFE"),
> > +  /*  6 */ N_("Side Left"),
> > +  /*  7 */ N_("Side Right"),
> > +  /*  8 */ N_("Channel 9"),
> > +  /*  9 */ N_("Channel 10"),
> > +  /* 10 */ N_("Channel 11"),
> > +  /* 11 */ N_("Channel 12"),
> > +  /* 12 */ N_("Channel 13"),
> > +  /* 13 */ N_("Channel 14"),
> > +  /* 14 */ N_("Channel 15"),
> > +  /* 15 */ N_("Channel 16")
> >  };
> >  
> >  static const int	channels4[] = {
> > -  0,
> > -  1,
> > -  3,
> > -  2
> > +  0, /* Front Left  */
> > +  1, /* Front Right */
> > +  3, /* Rear Right  */
> > +  2, /* Rear Left   */
> >  };
> >  static const int	channels6[] = {
> > -  0,
> > -  4,
> > -  1,
> > -  3,
> > -  2,
> > -  5
> > -}; 
> > +  0, /* Front Left  */
> > +  4, /* Center      */
> > +  1, /* Front Right */
> > +  3, /* Rear Right  */
> > +  2, /* Rear Left   */
> > +  5, /* LFE         */
> > +};
> >  static const int	channels8[] = {
> > -  0,
> > -  4,
> > -  1,
> > -  7,
> > -  3,
> > -  2,
> > -  6,
> > -  5
> > -}; 
> > +  0, /* Front Left  */
> > +  4, /* Center      */
> > +  1, /* Front Right */
> > +  7, /* Side Right  */
> > +  3, /* Rear Right  */
> > +  2, /* Rear Left   */
> > +  6, /* Side Left   */
> > +  5, /* LFE         */
> > +};
> >  
> >  static void generate_sine(uint8_t *frames, int channel, int count, double *_phase) {
> >    double phase = *_phase;
> > 

Re: [PATCH 0/4] Intel HDMI audio fixes

[Takashi Iwai]

At Fri, 13 Feb 2009 09:28:22 -0800,
Shane W wrote:
> 
> On Thu, Feb 12, 2009 at 12:41:11PM +0800, Wu Fengguang wrote:
> >         static const char        *channel_name[MAX_CHANNELS] = {
> >           /*  0 */ N_("Front Left"),
> >           /*  1 */ N_("Front Right"),
> >           /*  2 */ N_("Rear Left"),
> >           /*  3 */ N_("Rear Right"),
> >           /*  4 */ N_("Center"),       /* wav file is "Front Center.wav" */
> >           /*  5 */ N_("LFE"),          /* wav file is "Rear Center.wav" */
> >           /*  6 */ N_("Side Left"),
> >           /*  7 */ N_("Side Right"),
> > 
> > Whereas the HDMI channel mapping selected for 6-channel playback is
> > 
> >         { .ca_index = 0x0b,  .speakers = {   0,    0,  RR,  RL,  FC,  LFE,  FR,  FL } },
> > 
> > Here the two mappings disagree on channels other than Front Left/Right.
> > 
> > In theory the channel mapping should be adjusted via AC_VERB_SET_HDMI_CHAN_SLOT.
> > However I found that verb takes no effect for G35/G45. So I'm afraid
> > there's no trivial ways we can adjust the channel mapping.
> 
> If the driver can't reorder the channels to what ALSA
> needs, is it possible to do it somewhere else like
> HDA-Intel.conf and use the routing plugin somehow to
> reorder things.  Say for plughw:0,3 or modify the
> surround51 device?
> 
> I don't know much about this interface but it looks like
> channel routing is possible here.

Well, it's a good question.  Right now, surround51 is used for the
analog output which usually has the "right" ALSA channel mapping.  So,
we need to detect and switch the mapping using a plugin on the fly,
and that's not so trivial. 

Besides, the problem arises when both analog 5.1 and HDMI 5.1 are
implemented in the same "card".  Since a "card" is defined by a
controller, not by a codec as now, one card may have both 5.1
outputs.

We might need to switch to a codec-based card approach sometime later,
but I've postponed it simply because it can break so easily the
compatibility with older versions...


Anyway, the channel mapping issue is a missing piece.  There are a few
more issues, some to be implemented and some to be improved and fixed.


Takashi

[PATCH] speaker-test.c - add readability comments to speaker channels

[Wu Fengguang]

Signed-off-by: Wu Fengguang <fengguang.wu@intel.com>
---
 speaker-test/speaker-test.c |   72 +++++++++++++++++-----------------
 1 file changed, 36 insertions(+), 36 deletions(-)

--- alsa-utils.orig/speaker-test/speaker-test.c
+++ alsa-utils/speaker-test/speaker-test.c
@@ -85,48 +85,48 @@ static const char *given_test_wav_file =
 static char *wav_file_dir = SOUNDSDIR;
 
 static const char *const channel_name[MAX_CHANNELS] = {
-  N_("Front Left"),
-  N_("Front Right"),
-  N_("Rear Left"),
-  N_("Rear Right"),
-  N_("Center"),
-  N_("LFE"),
-  N_("Side Left"),
-  N_("Side Right"),
-  N_("Channel 9"),
-  N_("Channel 10"),
-  N_("Channel 11"),
-  N_("Channel 12"),
-  N_("Channel 13"),
-  N_("Channel 14"),
-  N_("Channel 15"),
-  N_("Channel 16")
+  /*  0 */ N_("Front Left"),
+  /*  1 */ N_("Front Right"),
+  /*  2 */ N_("Rear Left"),
+  /*  3 */ N_("Rear Right"),
+  /*  4 */ N_("Center"),
+  /*  5 */ N_("LFE"),
+  /*  6 */ N_("Side Left"),
+  /*  7 */ N_("Side Right"),
+  /*  8 */ N_("Channel 9"),
+  /*  9 */ N_("Channel 10"),
+  /* 10 */ N_("Channel 11"),
+  /* 11 */ N_("Channel 12"),
+  /* 12 */ N_("Channel 13"),
+  /* 13 */ N_("Channel 14"),
+  /* 14 */ N_("Channel 15"),
+  /* 15 */ N_("Channel 16")
 };
 
 static const int	channels4[] = {
-  0,
-  1,
-  3,
-  2
+  0, /* Front Left  */
+  1, /* Front Right */
+  3, /* Rear Right  */
+  2, /* Rear Left   */
 };
 static const int	channels6[] = {
-  0,
-  4,
-  1,
-  3,
-  2,
-  5
-}; 
+  0, /* Front Left  */
+  4, /* Center      */
+  1, /* Front Right */
+  3, /* Rear Right  */
+  2, /* Rear Left   */
+  5, /* LFE         */
+};
 static const int	channels8[] = {
-  0,
-  4,
-  1,
-  7,
-  3,
-  2,
-  6,
-  5
-}; 
+  0, /* Front Left  */
+  4, /* Center      */
+  1, /* Front Right */
+  7, /* Side Right  */
+  3, /* Rear Right  */
+  2, /* Rear Left   */
+  6, /* Side Left   */
+  5, /* LFE         */
+};
 
 static void generate_sine(uint8_t *frames, int channel, int count, double *_phase) {
   double phase = *_phase;

Re: [PATCH] speaker-test.c - add readability comments to speaker channels

[Takashi Iwai]

At Fri, 20 Feb 2009 21:05:33 +0800,
Wu Fengguang wrote:
> 
> 
> Signed-off-by: Wu Fengguang <fengguang.wu@intel.com>

Applied now.  Thanks!


Takashi

> ---
>  speaker-test/speaker-test.c |   72 +++++++++++++++++-----------------
>  1 file changed, 36 insertions(+), 36 deletions(-)
> 
> --- alsa-utils.orig/speaker-test/speaker-test.c
> +++ alsa-utils/speaker-test/speaker-test.c
> @@ -85,48 +85,48 @@ static const char *given_test_wav_file =
>  static char *wav_file_dir = SOUNDSDIR;
>  
>  static const char *const channel_name[MAX_CHANNELS] = {
> -  N_("Front Left"),
> -  N_("Front Right"),
> -  N_("Rear Left"),
> -  N_("Rear Right"),
> -  N_("Center"),
> -  N_("LFE"),
> -  N_("Side Left"),
> -  N_("Side Right"),
> -  N_("Channel 9"),
> -  N_("Channel 10"),
> -  N_("Channel 11"),
> -  N_("Channel 12"),
> -  N_("Channel 13"),
> -  N_("Channel 14"),
> -  N_("Channel 15"),
> -  N_("Channel 16")
> +  /*  0 */ N_("Front Left"),
> +  /*  1 */ N_("Front Right"),
> +  /*  2 */ N_("Rear Left"),
> +  /*  3 */ N_("Rear Right"),
> +  /*  4 */ N_("Center"),
> +  /*  5 */ N_("LFE"),
> +  /*  6 */ N_("Side Left"),
> +  /*  7 */ N_("Side Right"),
> +  /*  8 */ N_("Channel 9"),
> +  /*  9 */ N_("Channel 10"),
> +  /* 10 */ N_("Channel 11"),
> +  /* 11 */ N_("Channel 12"),
> +  /* 12 */ N_("Channel 13"),
> +  /* 13 */ N_("Channel 14"),
> +  /* 14 */ N_("Channel 15"),
> +  /* 15 */ N_("Channel 16")
>  };
>  
>  static const int	channels4[] = {
> -  0,
> -  1,
> -  3,
> -  2
> +  0, /* Front Left  */
> +  1, /* Front Right */
> +  3, /* Rear Right  */
> +  2, /* Rear Left   */
>  };
>  static const int	channels6[] = {
> -  0,
> -  4,
> -  1,
> -  3,
> -  2,
> -  5
> -}; 
> +  0, /* Front Left  */
> +  4, /* Center      */
> +  1, /* Front Right */
> +  3, /* Rear Right  */
> +  2, /* Rear Left   */
> +  5, /* LFE         */
> +};
>  static const int	channels8[] = {
> -  0,
> -  4,
> -  1,
> -  7,
> -  3,
> -  2,
> -  6,
> -  5
> -}; 
> +  0, /* Front Left  */
> +  4, /* Center      */
> +  1, /* Front Right */
> +  7, /* Side Right  */
> +  3, /* Rear Right  */
> +  2, /* Rear Left   */
> +  6, /* Side Left   */
> +  5, /* LFE         */
> +};
>  
>  static void generate_sine(uint8_t *frames, int channel, int count, double *_phase) {
>    double phase = *_phase;
>