[PATCH 0/5] Intel HDMI fixes

[PATCH 0/5] Intel HDMI fixes

[Wu Fengguang]

Hi Takashi,

Here are some fixes for the Intel HDMI patch.

I'm not perticular sure about the last two changes,
review comments are highly appreciated.

Thanks,
Fengguang

[PATCH 1/5] hda - show HBR(High Bit Rate) pin cap in procfs

[Wu Fengguang]

[-- Attachment #1: hda-proc-hbr.patch --]
[-- Type: text/plain, Size: 1268 bytes --]

Note that the HBR capability only applies to HDMI pin.

Signed-off-by: Wu Fengguang <fengguang.wu@intel.com>
---
 sound/pci/hda/hda_codec.h |    1 +
 sound/pci/hda/hda_proc.c  |    5 ++++-
 2 files changed, 5 insertions(+), 1 deletion(-)

--- sound-2.6.orig/sound/pci/hda/hda_codec.h	2009-12-02 10:11:39.000000000 +0800
+++ sound-2.6/sound/pci/hda/hda_codec.h	2009-12-02 10:12:00.000000000 +0800
@@ -258,6 +258,7 @@ enum {
 #define AC_PINCAP_VREF			(0x37<<8)
 #define AC_PINCAP_VREF_SHIFT		8
 #define AC_PINCAP_EAPD			(1<<16)	/* EAPD capable */
+#define AC_PINCAP_HBR			(1<<27)	/* High Bit Rate */
 /* Vref status (used in pin cap) */
 #define AC_PINCAP_VREF_HIZ		(1<<0)	/* Hi-Z */
 #define AC_PINCAP_VREF_50		(1<<1)	/* 50% */
--- sound-2.6.orig/sound/pci/hda/hda_proc.c	2009-12-02 10:11:39.000000000 +0800
+++ sound-2.6/sound/pci/hda/hda_proc.c	2009-12-02 10:12:00.000000000 +0800
@@ -240,8 +240,11 @@ static void print_pin_caps(struct snd_in
 		/* Realtek uses this bit as a different meaning */
 		if ((codec->vendor_id >> 16) == 0x10ec)
 			snd_iprintf(buffer, " R/L");
-		else
+		else {
+			if (caps & AC_PINCAP_HBR)
+				snd_iprintf(buffer, " HBR");
 			snd_iprintf(buffer, " HDMI");
+		}
 	}
 	if (caps & AC_PINCAP_TRIG_REQ)
 		snd_iprintf(buffer, " Trigger");

[PATCH 2/5] intelhdmi - accept DisplayPort pin

[Wu Fengguang]

[-- Attachment #1: alsa-parse-allow-dp.patch --]
[-- Type: text/plain, Size: 1979 bytes --]

HDA036 spec states:
  DP (Display Port) indicates whether the Pin Complex Widget supports
  connection to a Display Port sink.  Supported if set to 1. Note that
  it is possible for the pin widget to support more than one digital
  display connection type, e.g. HDMI and DP bit are both set to 1.

Also export the DP pin cap in procfs.

Signed-off-by: Wu Fengguang <fengguang.wu@intel.com>
---
 sound/pci/hda/hda_codec.h       |    3 +++
 sound/pci/hda/hda_proc.c        |    2 ++
 sound/pci/hda/patch_intelhdmi.c |    2 +-
 3 files changed, 6 insertions(+), 1 deletion(-)

--- sound-2.6.orig/sound/pci/hda/hda_codec.h	2009-12-02 10:11:41.000000000 +0800
+++ sound-2.6/sound/pci/hda/hda_codec.h	2009-12-02 10:11:47.000000000 +0800
@@ -255,6 +255,9 @@ enum {
  *       in HD-audio specification
  */
 #define AC_PINCAP_HDMI			(1<<7)	/* HDMI pin */
+#define AC_PINCAP_DP			(1<<24)	/* DisplayPort pin, can
+						 * coexist with AC_PINCAP_HDMI
+						 */
 #define AC_PINCAP_VREF			(0x37<<8)
 #define AC_PINCAP_VREF_SHIFT		8
 #define AC_PINCAP_EAPD			(1<<16)	/* EAPD capable */
--- sound-2.6.orig/sound/pci/hda/patch_intelhdmi.c	2009-12-02 10:11:39.000000000 +0800
+++ sound-2.6/sound/pci/hda/patch_intelhdmi.c	2009-12-02 10:11:47.000000000 +0800
@@ -344,7 +344,7 @@ static int intel_hdmi_parse_codec(struct
 			break;
 		case AC_WID_PIN:
 			caps = snd_hda_param_read(codec, nid, AC_PAR_PIN_CAP);
-			if (!(caps & AC_PINCAP_HDMI))
+			if (!(caps & (AC_PINCAP_HDMI | AC_PINCAP_DP)))
 				continue;
 			if (intel_hdmi_add_pin(codec, nid) < 0)
 				return -EINVAL;
--- sound-2.6.orig/sound/pci/hda/hda_proc.c	2009-12-02 10:11:41.000000000 +0800
+++ sound-2.6/sound/pci/hda/hda_proc.c	2009-12-02 10:11:47.000000000 +0800
@@ -246,6 +246,8 @@ static void print_pin_caps(struct snd_in
 			snd_iprintf(buffer, " HDMI");
 		}
 	}
+	if (caps & AC_PINCAP_DP)
+		snd_iprintf(buffer, " DP");
 	if (caps & AC_PINCAP_TRIG_REQ)
 		snd_iprintf(buffer, " Trigger");
 	if (caps & AC_PINCAP_IMP_SENSE)

[PATCH 3/5] intelhdmi - channel mapping applies to Pin

[Wu Fengguang]

[-- Attachment #1: hdmi-channel-mapping-fix-to-pin.patch --]
[-- Type: text/plain, Size: 2491 bytes --]

HDA036-A specifies that the Audio Sample Packet (ASP) Channel Mapping
verbs apply to Digital Display Pin Complex instead of Converter.

With this fix, channel mapping is working as expected for IbexPeak.

Thanks to Marcin for pointing this out!

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

--- sound-2.6.orig/sound/pci/hda/patch_intelhdmi.c	2009-11-24 09:10:55.000000000 +0800
+++ sound-2.6/sound/pci/hda/patch_intelhdmi.c	2009-11-24 09:11:01.000000000 +0800
@@ -436,14 +436,15 @@ static void hdmi_set_channel_count(struc
 				    AC_VERB_SET_CVT_CHAN_COUNT, chs - 1);
 }
 
-static void hdmi_debug_channel_mapping(struct hda_codec *codec, hda_nid_t nid)
+static void hdmi_debug_channel_mapping(struct hda_codec *codec,
+				       hda_nid_t pin_nid)
 {
 #ifdef CONFIG_SND_DEBUG_VERBOSE
 	int i;
 	int slot;
 
 	for (i = 0; i < 8; i++) {
-		slot = snd_hda_codec_read(codec, nid, 0,
+		slot = snd_hda_codec_read(codec, pin_nid, 0,
 						AC_VERB_GET_HDMI_CHAN_SLOT, i);
 		printk(KERN_DEBUG "HDMI: ASP channel %d => slot %d\n",
 						slot >> 4, slot & 0xf);
@@ -619,7 +620,8 @@ static int hdmi_setup_channel_allocation
 	return ai->CA;
 }
 
-static void hdmi_setup_channel_mapping(struct hda_codec *codec, hda_nid_t nid,
+static void hdmi_setup_channel_mapping(struct hda_codec *codec,
+				       hda_nid_t pin_nid,
 				       struct hdmi_audio_infoframe *ai)
 {
 	int i;
@@ -633,11 +635,11 @@ static void hdmi_setup_channel_mapping(s
 	 */
 
 	for (i = 0; i < 8; i++)
-		snd_hda_codec_write(codec, nid, 0,
+		snd_hda_codec_write(codec, pin_nid, 0,
 				    AC_VERB_SET_HDMI_CHAN_SLOT,
 				    (i << 4) | i);
 
-	hdmi_debug_channel_mapping(codec, nid);
+	hdmi_debug_channel_mapping(codec, pin_nid);
 }
 
 static bool hdmi_infoframe_uptodate(struct hda_codec *codec, hda_nid_t pin_nid,
@@ -676,7 +678,6 @@ static void hdmi_setup_audio_infoframe(s
 	};
 
 	hdmi_setup_channel_allocation(codec, nid, &ai);
-	hdmi_setup_channel_mapping(codec, nid, &ai);
 
 	for (i = 0; i < spec->num_pins; i++) {
 		if (spec->pin_cvt[i] != nid)
@@ -686,6 +687,7 @@ static void hdmi_setup_audio_infoframe(s
 
 		pin_nid = spec->pin[i];
 		if (!hdmi_infoframe_uptodate(codec, pin_nid, &ai)) {
+			hdmi_setup_channel_mapping(codec, pin_nid, &ai);
 			hdmi_stop_infoframe_trans(codec, pin_nid);
 			hdmi_fill_audio_infoframe(codec, pin_nid, &ai);
 			hdmi_start_infoframe_trans(codec, pin_nid);

[PATCH 4/5] intelhdmi - add channel mapping for typical configurations

[Wu Fengguang]

[-- Attachment #1: hdmi-swap-channel-mapping.patch --]
[-- Type: text/plain, Size: 6571 bytes --]

IbexPeak is the first Intel HDMI audio codec to support channel mapping.

Currently the outstanding problem is, the HDMI channel order do not
agree with that of ALSA.  This patch presents workaround for some
typical use cases. It gives priority to the typical ALSA surround
configurations, and defines channel mapping for them.

We may need better kernel+userspace interactive channel mapping scheme.
For example, in current scheme if user plays with the surround50 device,
the kernel is unaware of this and will still select the surround41
channel allocation and channel mapping..

Thanks to Marcin for offering good tips!

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

--- sound-2.6.orig/sound/pci/hda/patch_intelhdmi.c	2009-12-10 20:32:01.000000000 +0800
+++ sound-2.6/sound/pci/hda/patch_intelhdmi.c	2009-12-11 11:21:04.000000000 +0800
@@ -146,38 +146,78 @@ struct cea_channel_speaker_allocation {
 };
 
 /*
+ * ALSA sequence is:
+ *
+ *       surround40   surround41   surround50   surround51   surround71
+ * ch0   front left   =            =            =            =
+ * ch1   front right  =            =            =            =
+ * ch2   rear left    =            =            =            =
+ * ch3   rear right   =            =            =            =
+ * ch4                LFE          center       center       center
+ * ch5                                          LFE          LFE
+ * ch6                                                       side left
+ * ch7                                                       side right
+ *
+ * surround71 = {FL, FR, RLC, RRC, FC, LFE, RL, RR}
+ */
+static int hdmi_channel_mapping[0x32][8] = {
+	/* stereo */
+	[0x00] = { 0x00, 0x11, 0xf2, 0xf3, 0xf4, 0xf5, 0xf6, 0xf7 },
+	/* 2.1 */
+	[0x01] = { 0x00, 0x11, 0x22, 0xf3, 0xf4, 0xf5, 0xf6, 0xf7 },
+	/* Dolby Surround */
+	[0x02] = { 0x00, 0x11, 0x23, 0xf2, 0xf4, 0xf5, 0xf6, 0xf7 },
+	/* surround40 */
+	[0x08] = { 0x00, 0x11, 0x24, 0x35, 0xf3, 0xf2, 0xf6, 0xf7 },
+	/* 4ch */
+	[0x03] = { 0x00, 0x11, 0x23, 0x32, 0x44, 0xf5, 0xf6, 0xf7 },
+	/* surround41 */
+	[0x09] = { 0x00, 0x11, 0x24, 0x34, 0x43, 0xf2, 0xf6, 0xf7 },
+	/* surround50 */
+	[0x0a] = { 0x00, 0x11, 0x24, 0x35, 0x43, 0xf2, 0xf6, 0xf7 },
+	/* surround51 */
+	[0x0b] = { 0x00, 0x11, 0x24, 0x35, 0x43, 0x52, 0xf6, 0xf7 },
+	/* 7.1 */
+	[0x13] = { 0x00, 0x11, 0x26, 0x37, 0x43, 0x52, 0x64, 0x75 },
+};
+
+/*
  * This is an ordered list!
  *
  * The preceding ones have better chances to be selected by
  * hdmi_setup_channel_allocation().
  */
 static struct cea_channel_speaker_allocation channel_allocations[] = {
-/* 			  channel:   8     7    6    5    4     3    2    1  */
+/* 			  channel:   7     6    5    4    3     2    1    0  */
 { .ca_index = 0x00,  .speakers = {   0,    0,   0,   0,   0,    0,  FR,  FL } },
 				 /* 2.1 */
 { .ca_index = 0x01,  .speakers = {   0,    0,   0,   0,   0,  LFE,  FR,  FL } },
 				 /* Dolby Surround */
 { .ca_index = 0x02,  .speakers = {   0,    0,   0,   0,  FC,    0,  FR,  FL } },
+				 /* surround40 */
+{ .ca_index = 0x08,  .speakers = {   0,    0,  RR,  RL,   0,    0,  FR,  FL } },
+				 /* surround41 */
+{ .ca_index = 0x09,  .speakers = {   0,    0,  RR,  RL,   0,  LFE,  FR,  FL } },
+				 /* surround50 */
+{ .ca_index = 0x0a,  .speakers = {   0,    0,  RR,  RL,  FC,    0,  FR,  FL } },
+				 /* surround51 */
+{ .ca_index = 0x0b,  .speakers = {   0,    0,  RR,  RL,  FC,  LFE,  FR,  FL } },
+				 /* 6.1 */
+{ .ca_index = 0x0f,  .speakers = {   0,   RC,  RR,  RL,  FC,  LFE,  FR,  FL } },
+				 /* surround71 */
+{ .ca_index = 0x13,  .speakers = { RRC,  RLC,  RR,  RL,  FC,  LFE,  FR,  FL } },
+
 { .ca_index = 0x03,  .speakers = {   0,    0,   0,   0,  FC,  LFE,  FR,  FL } },
 { .ca_index = 0x04,  .speakers = {   0,    0,   0,  RC,   0,    0,  FR,  FL } },
 { .ca_index = 0x05,  .speakers = {   0,    0,   0,  RC,   0,  LFE,  FR,  FL } },
 { .ca_index = 0x06,  .speakers = {   0,    0,   0,  RC,  FC,    0,  FR,  FL } },
 { .ca_index = 0x07,  .speakers = {   0,    0,   0,  RC,  FC,  LFE,  FR,  FL } },
-{ .ca_index = 0x08,  .speakers = {   0,    0,  RR,  RL,   0,    0,  FR,  FL } },
-{ .ca_index = 0x09,  .speakers = {   0,    0,  RR,  RL,   0,  LFE,  FR,  FL } },
-{ .ca_index = 0x0a,  .speakers = {   0,    0,  RR,  RL,  FC,    0,  FR,  FL } },
-				 /* 5.1 */
-{ .ca_index = 0x0b,  .speakers = {   0,    0,  RR,  RL,  FC,  LFE,  FR,  FL } },
 { .ca_index = 0x0c,  .speakers = {   0,   RC,  RR,  RL,   0,    0,  FR,  FL } },
 { .ca_index = 0x0d,  .speakers = {   0,   RC,  RR,  RL,   0,  LFE,  FR,  FL } },
 { .ca_index = 0x0e,  .speakers = {   0,   RC,  RR,  RL,  FC,    0,  FR,  FL } },
-				 /* 6.1 */
-{ .ca_index = 0x0f,  .speakers = {   0,   RC,  RR,  RL,  FC,  LFE,  FR,  FL } },
 { .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 } },
-				 /* 7.1 */
-{ .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 } },
@@ -210,7 +250,6 @@ static struct cea_channel_speaker_alloca
 { .ca_index = 0x31,  .speakers = { FRW,  FLW,  RR,  RL,  FC,  LFE,  FR,  FL } },
 };
 
-
 /*
  * HDA/HDMI auto parsing
  */
@@ -625,19 +664,25 @@ static void hdmi_setup_channel_mapping(s
 				       struct hdmi_audio_infoframe *ai)
 {
 	int i;
+	int ca = ai->CA;
+	int err;
 
-	if (!ai->CA)
-		return;
-
-	/*
-	 * TODO: adjust channel mapping if necessary
-	 * ALSA sequence is front/surr/clfe/side?
-	 */
+	if (hdmi_channel_mapping[ca][1] == 0) {
+		for (i = 0; i < channel_allocations[ca].channels; i++)
+			hdmi_channel_mapping[ca][i] = i | (i << 4);
+		for (; i < 8; i++)
+			hdmi_channel_mapping[ca][i] = 0xf | (i << 4);
+	}
 
-	for (i = 0; i < 8; i++)
-		snd_hda_codec_write(codec, pin_nid, 0,
-				    AC_VERB_SET_HDMI_CHAN_SLOT,
-				    (i << 4) | i);
+	for (i = 0; i < 8; i++) {
+		err = snd_hda_codec_write(codec, pin_nid, 0,
+					  AC_VERB_SET_HDMI_CHAN_SLOT,
+					  hdmi_channel_mapping[ca][i]);
+		if (err) {
+			snd_printdd(KERN_INFO "HDMI: channel mapping failed\n");
+			break;
+		}
+	}
 
 	hdmi_debug_channel_mapping(codec, pin_nid);
 }

[PATCH 5/5] intelhdmi - dont power off HDA link

[Wu Fengguang]

[-- Attachment #1: hdmi-dont-power-off-controller.patch --]
[-- Type: text/plain, Size: 2063 bytes --]

For codecs without EPSS support (G45/IbexPeak), the hotplug event will
be lost if the HDA is powered off during the time. After that the pin
presence detection verb returns inaccurate info.

So always power-on HDA link for !EPSS codecs.

Signed-off-by: Wu Fengguang <fengguang.wu@intel.com>
---
 sound/pci/hda/hda_codec.h       |    1 +
 sound/pci/hda/hda_intel.c       |    2 ++
 sound/pci/hda/patch_intelhdmi.c |   11 +++++++++++
 3 files changed, 14 insertions(+)

--- sound-2.6.orig/sound/pci/hda/hda_intel.c	2009-12-10 12:45:24.000000000 +0800
+++ sound-2.6/sound/pci/hda/hda_intel.c	2009-12-10 20:10:51.000000000 +0800
@@ -2079,6 +2079,8 @@ static void azx_power_notify(struct hda_
 			power_on = 1;
 			break;
 		}
+		if (c->power_keep_link_on)
+			power_save_controller = 0;
 	}
 	if (power_on)
 		azx_init_chip(chip);
--- sound-2.6.orig/sound/pci/hda/patch_intelhdmi.c	2009-12-10 12:56:41.000000000 +0800
+++ sound-2.6/sound/pci/hda/patch_intelhdmi.c	2009-12-10 20:19:32.000000000 +0800
@@ -391,6 +391,17 @@ static int intel_hdmi_parse_codec(struct
 		}
 	}
 
+	/*
+	 * G45/IbexPeak don't support EPSS: the unsolicited pin hot plug event
+	 * can be lost and presence sense verb will become inaccurate if the
+	 * HDA link is powered off at hot plug or hw initialization time.
+	 */
+#ifdef CONFIG_SND_HDA_POWER_SAVE
+	if (!(snd_hda_param_read(codec, codec->afg, AC_PAR_POWER_STATE) &
+	      AC_PWRST_EPSS))
+	codec->power_keep_link_on = 1;
+#endif
+
 	return 0;
 }
 
--- sound-2.6.orig/sound/pci/hda/hda_codec.h	2009-12-10 12:45:24.000000000 +0800
+++ sound-2.6/sound/pci/hda/hda_codec.h	2009-12-10 20:10:51.000000000 +0800
@@ -819,6 +819,7 @@ struct hda_codec {
 #ifdef CONFIG_SND_HDA_POWER_SAVE
 	unsigned int power_on :1;	/* current (global) power-state */
 	unsigned int power_transition :1; /* power-state in transition */
+	unsigned int power_keep_link_on :1; /* don't power off HDA link */
 	int power_count;	/* current (global) power refcount */
 	struct delayed_work power_work; /* delayed task for powerdown */
 	unsigned long power_on_acct;

Re: [PATCH 4/5] intelhdmi - add channel mapping for typical configurations

[Takashi Iwai]

At Fri, 11 Dec 2009 12:28:36 +0800,
Wu Fengguang wrote:
> 
> IbexPeak is the first Intel HDMI audio codec to support channel mapping.
> 
> Currently the outstanding problem is, the HDMI channel order do not
> agree with that of ALSA.  This patch presents workaround for some
> typical use cases. It gives priority to the typical ALSA surround
> configurations, and defines channel mapping for them.

This looks good.  At least, this can reduce confusions.

> We may need better kernel+userspace interactive channel mapping scheme.
> For example, in current scheme if user plays with the surround50 device,
> the kernel is unaware of this and will still select the surround41
> channel allocation and channel mapping..

Yes, more flexible channel mapping is on my TODO list.
I have a half-finished patch series, but need to rebase and brush up.
Let's fix it for 2.6.34.


thanks,

Takashi

Re: [PATCH 5/5] intelhdmi - dont power off HDA link

[Takashi Iwai]

At Fri, 11 Dec 2009 12:28:37 +0800,
Wu Fengguang wrote:
> 
> For codecs without EPSS support (G45/IbexPeak), the hotplug event will
> be lost if the HDA is powered off during the time. After that the pin
> presence detection verb returns inaccurate info.
> 
> So always power-on HDA link for !EPSS codecs.
> 
> Signed-off-by: Wu Fengguang <fengguang.wu@intel.com>

I think it'd be better to add a flag to hda_bus than hda_codec like
the patch below (the change in patch_intelhdmi.c is not included).
It's a bit ugly to change the module parameter from the driver side.


thanks,

Takashi

---
diff --git a/sound/pci/hda/hda_codec.h b/sound/pci/hda/hda_codec.h
index 9000d52..1d541b7 100644
--- a/sound/pci/hda/hda_codec.h
+++ b/sound/pci/hda/hda_codec.h
@@ -639,6 +639,7 @@ struct hda_bus {
 	unsigned int rirb_error:1;	/* error in codec communication */
 	unsigned int response_reset:1;	/* controller was reset */
 	unsigned int in_reset:1;	/* during reset operation */
+	unsigned int power_keep_link_on:1; /* don't power off HDA link */
 };
 
 /*
diff --git a/sound/pci/hda/hda_intel.c b/sound/pci/hda/hda_intel.c
index efcc4f7..e54420e 100644
--- a/sound/pci/hda/hda_intel.c
+++ b/sound/pci/hda/hda_intel.c
@@ -2082,7 +2082,8 @@ static void azx_power_notify(struct hda_bus *bus)
 	}
 	if (power_on)
 		azx_init_chip(chip);
-	else if (chip->running && power_save_controller)
+	else if (chip->running && power_save_controller &&
+		 !bus->power_keep_link_on)
 		azx_stop_chip(chip);
 }
 #endif /* CONFIG_SND_HDA_POWER_SAVE */

Re: [PATCH 0/5] Intel HDMI fixes

[Takashi Iwai]

At Fri, 11 Dec 2009 12:28:32 +0800,
Wu Fengguang wrote:
> 
> Hi Takashi,
> 
> Here are some fixes for the Intel HDMI patch.
> 
> I'm not perticular sure about the last two changes,
> review comments are highly appreciated.

I applied the patches, except for the last one since I prefer a flag in
hda_bus to hda_codec as mentioned in the review comment. 


Thanks!

Takashi

Re: [PATCH 4/5] intelhdmi - add channel mapping for typical configurations

[Wu Fengguang]

On Fri, Dec 11, 2009 at 03:00:29PM +0800, Takashi Iwai wrote:
> At Fri, 11 Dec 2009 12:28:36 +0800,
> Wu Fengguang wrote:
> > 
> > IbexPeak is the first Intel HDMI audio codec to support channel mapping.
> > 
> > Currently the outstanding problem is, the HDMI channel order do not
> > agree with that of ALSA.  This patch presents workaround for some
> > typical use cases. It gives priority to the typical ALSA surround
> > configurations, and defines channel mapping for them.
> 
> This looks good.  At least, this can reduce confusions.

Thank you.

> > We may need better kernel+userspace interactive channel mapping scheme.
> > For example, in current scheme if user plays with the surround50 device,
> > the kernel is unaware of this and will still select the surround41
> > channel allocation and channel mapping..
> 
> Yes, more flexible channel mapping is on my TODO list.
> I have a half-finished patch series, but need to rebase and brush up.
> Let's fix it for 2.6.34.

That would be great. The rational I do this patch is, even if we have
some user-space channel mapping facility, it still benefits to be able
to use the hardware capability -- which helps saving power :)

Thanks,
Fengguang

Re: [PATCH 5/5] intelhdmi - dont power off HDA link

[Wu Fengguang]

On Fri, Dec 11, 2009 at 03:08:52PM +0800, Takashi Iwai wrote:
> At Fri, 11 Dec 2009 12:28:37 +0800,
> Wu Fengguang wrote:
> > 
> > For codecs without EPSS support (G45/IbexPeak), the hotplug event will
> > be lost if the HDA is powered off during the time. After that the pin
> > presence detection verb returns inaccurate info.
> > 
> > So always power-on HDA link for !EPSS codecs.
> > 
> > Signed-off-by: Wu Fengguang <fengguang.wu@intel.com>
> 
> I think it'd be better to add a flag to hda_bus than hda_codec like
> the patch below (the change in patch_intelhdmi.c is not included).
> It's a bit ugly to change the module parameter from the driver side.

Good suggestion, thanks! Here is the updated patch.

---
intelhdmi - don't power off HDA link

For codecs without EPSS support (G45/IbexPeak), the hotplug event will
be lost if the HDA is powered off during the time. After that the pin
presence detection verb returns inaccurate info.

So always power-on HDA link for !EPSS codecs.

KarL offers the fact and Takashi recommends to flag hda_bus. Thanks!

Signed-off-by: Wu Fengguang <fengguang.wu@intel.com>
---
 sound/pci/hda/hda_codec.h       |    1 +
 sound/pci/hda/hda_intel.c       |    3 ++-
 sound/pci/hda/patch_intelhdmi.c |   11 +++++++++++
 3 files changed, 14 insertions(+), 1 deletion(-)

--- sound-2.6.orig/sound/pci/hda/hda_intel.c	2009-12-11 12:46:18.000000000 +0800
+++ sound-2.6/sound/pci/hda/hda_intel.c	2009-12-11 20:12:31.000000000 +0800
@@ -2082,7 +2082,8 @@ static void azx_power_notify(struct hda_
 	}
 	if (power_on)
 		azx_init_chip(chip);
-	else if (chip->running && power_save_controller)
+	else if (chip->running && power_save_controller &&
+		 !bus->power_keep_link_on)
 		azx_stop_chip(chip);
 }
 #endif /* CONFIG_SND_HDA_POWER_SAVE */
--- sound-2.6.orig/sound/pci/hda/patch_intelhdmi.c	2009-12-11 12:46:22.000000000 +0800
+++ sound-2.6/sound/pci/hda/patch_intelhdmi.c	2009-12-11 20:07:59.000000000 +0800
@@ -391,6 +391,17 @@ static int intel_hdmi_parse_codec(struct
 		}
 	}
 
+	/*
+	 * G45/IbexPeak don't support EPSS: the unsolicited pin hot plug event
+	 * can be lost and presence sense verb will become inaccurate if the
+	 * HDA link is powered off at hot plug or hw initialization time.
+	 */
+#ifdef CONFIG_SND_HDA_POWER_SAVE
+	if (!(snd_hda_param_read(codec, codec->afg, AC_PAR_POWER_STATE) &
+	      AC_PWRST_EPSS))
+		codec->bus->power_keep_link_on = 1;
+#endif
+
 	return 0;
 }
 
--- sound-2.6.orig/sound/pci/hda/hda_codec.h	2009-12-11 12:46:21.000000000 +0800
+++ sound-2.6/sound/pci/hda/hda_codec.h	2009-12-11 20:13:18.000000000 +0800
@@ -639,6 +639,7 @@ struct hda_bus {
 	unsigned int rirb_error:1;	/* error in codec communication */
 	unsigned int response_reset:1;	/* controller was reset */
 	unsigned int in_reset:1;	/* during reset operation */
+	unsigned int power_keep_link_on:1; /* don't power off HDA link */
 };
 
 /*