[RFC PATCH 0/2] ALSA: hda - DP MST audio for Jack support

[RFC PATCH 0/2] ALSA: hda - DP MST audio for Jack support

[libin.yang]

From: Libin Yang <libin.yang@linux.intel.com>

This is the patch set for Jack support for DP MST audio

Libin Yang (2):
  ALSA: hda - jack support DP MST audio
  ALSA: hda - hdmi audio add dynamically jack binding to pin

 sound/pci/hda/hda_jack.c   | 186 ++++++++++++++++++++++++++++++++++++++++-----
 sound/pci/hda/hda_jack.h   |  32 +++++++-
 sound/pci/hda/patch_hdmi.c | 116 ++++++++++++++++++++++------
 3 files changed, 288 insertions(+), 46 deletions(-)

-- 
1.9.1

[RFC PATCH 1/2] ALSA: hda - jack support DP MST audio

[libin.yang]

From: Libin Yang <libin.yang@linux.intel.com>

Add jack support for DP MST audio in hda_jack.c

Signed-off-by: Libin Yang <libin.yang@linux.intel.com>
---
 sound/pci/hda/hda_jack.c   | 158 +++++++++++++++++++++++++++++++++++++++------
 sound/pci/hda/hda_jack.h   |  28 +++++++-
 sound/pci/hda/patch_hdmi.c |   2 +-
 3 files changed, 166 insertions(+), 22 deletions(-)

diff --git a/sound/pci/hda/hda_jack.c b/sound/pci/hda/hda_jack.c
index 366efbf..c0a018b 100644
--- a/sound/pci/hda/hda_jack.c
+++ b/sound/pci/hda/hda_jack.c
@@ -47,7 +47,8 @@ bool is_jack_detectable(struct hda_codec *codec, hda_nid_t nid)
 EXPORT_SYMBOL_GPL(is_jack_detectable);
 
 /* execute pin sense measurement */
-static u32 read_pin_sense(struct hda_codec *codec, hda_nid_t nid)
+static u32 read_pin_sense(struct hda_codec *codec, hda_nid_t nid,
+					hda_dev_t dev_id)
 {
 	u32 pincap;
 	u32 val;
@@ -59,7 +60,7 @@ static u32 read_pin_sense(struct hda_codec *codec, hda_nid_t nid)
 					AC_VERB_SET_PIN_SENSE, 0);
 	}
 	val = snd_hda_codec_read(codec, nid, 0,
-				  AC_VERB_GET_PIN_SENSE, 0);
+				  AC_VERB_GET_PIN_SENSE, dev_id);
 	if (codec->inv_jack_detect)
 		val ^= AC_PINSENSE_PRESENCE;
 	return val;
@@ -86,6 +87,28 @@ snd_hda_jack_tbl_get(struct hda_codec *codec, hda_nid_t nid)
 EXPORT_SYMBOL_GPL(snd_hda_jack_tbl_get);
 
 /**
+ * snd_hda_jack_tbl_get_mst - query the jack-table entry for the given NID
+ * @codec: the HDA codec
+ * @nid: pin NID to refer to
+ * @dev_id: device entry id
+ */
+struct hda_jack_tbl *
+snd_hda_jack_tbl_get_mst(struct hda_codec *codec, hda_nid_t nid,
+			hda_dev_t dev_id)
+{
+	struct hda_jack_tbl *jack = codec->jacktbl.list;
+	int i;
+
+	if (!nid || !jack)
+		return NULL;
+	for (i = 0; i < codec->jacktbl.used; i++, jack++)
+		if (jack->nid == nid && jack->dev_id == dev_id)
+			return jack;
+	return NULL;
+}
+EXPORT_SYMBOL_GPL(snd_hda_jack_tbl_get_mst);
+
+/**
  * snd_hda_jack_tbl_get_from_tag - query the jack-table entry for the given tag
  * @codec: the HDA codec
  * @tag: tag value to refer to
@@ -109,17 +132,20 @@ EXPORT_SYMBOL_GPL(snd_hda_jack_tbl_get_from_tag);
  * snd_hda_jack_tbl_new - create a jack-table entry for the given NID
  * @codec: the HDA codec
  * @nid: pin NID to assign
+ * @dev_id: device entry id
  */
 static struct hda_jack_tbl *
-snd_hda_jack_tbl_new(struct hda_codec *codec, hda_nid_t nid)
+snd_hda_jack_tbl_new(struct hda_codec *codec, hda_nid_t nid, hda_dev_t dev_id)
 {
-	struct hda_jack_tbl *jack = snd_hda_jack_tbl_get(codec, nid);
+	struct hda_jack_tbl *jack =
+		snd_hda_jack_tbl_get_mst(codec, nid, dev_id);
 	if (jack)
 		return jack;
 	jack = snd_array_new(&codec->jacktbl);
 	if (!jack)
 		return NULL;
 	jack->nid = nid;
+	jack->dev_id = dev_id;
 	jack->jack_dirty = 1;
 	jack->tag = codec->jacktbl.used;
 	return jack;
@@ -157,9 +183,11 @@ static void jack_detect_update(struct hda_codec *codec,
 	if (jack->phantom_jack)
 		jack->pin_sense = AC_PINSENSE_PRESENCE;
 	else
-		jack->pin_sense = read_pin_sense(codec, jack->nid);
+		jack->pin_sense =
+			read_pin_sense(codec, jack->nid, jack->dev_id);
 
 	/* A gating jack indicates the jack is invalid if gating is unplugged */
+	/* fixme: MST audio doesn't using gating and gated jack. */
 	if (jack->gating_jack && !snd_hda_jack_detect(codec, jack->gating_jack))
 		jack->pin_sense &= ~AC_PINSENSE_PRESENCE;
 
@@ -198,18 +226,20 @@ EXPORT_SYMBOL_GPL(snd_hda_jack_set_dirty_all);
  * snd_hda_pin_sense - execute pin sense measurement
  * @codec: the CODEC to sense
  * @nid: the pin NID to sense
+ * @dev_id: device entry id
  *
  * Execute necessary pin sense measurement and return its Presence Detect,
  * Impedance, ELD Valid etc. status bits.
  */
-u32 snd_hda_pin_sense(struct hda_codec *codec, hda_nid_t nid)
+u32 snd_hda_pin_sense(struct hda_codec *codec, hda_nid_t nid, hda_dev_t dev_id)
 {
-	struct hda_jack_tbl *jack = snd_hda_jack_tbl_get(codec, nid);
+	struct hda_jack_tbl *jack =
+		snd_hda_jack_tbl_get_mst(codec, nid, dev_id);
 	if (jack) {
 		jack_detect_update(codec, jack);
 		return jack->pin_sense;
 	}
-	return read_pin_sense(codec, nid);
+	return read_pin_sense(codec, nid, dev_id);
 }
 EXPORT_SYMBOL_GPL(snd_hda_pin_sense);
 
@@ -226,7 +256,7 @@ int snd_hda_jack_detect_state(struct hda_codec *codec, hda_nid_t nid)
 	struct hda_jack_tbl *jack = snd_hda_jack_tbl_get(codec, nid);
 	if (jack && jack->phantom_jack)
 		return HDA_JACK_PHANTOM;
-	else if (snd_hda_pin_sense(codec, nid) & AC_PINSENSE_PRESENCE)
+	else if (snd_hda_pin_sense(codec, nid, 0) & AC_PINSENSE_PRESENCE)
 		return HDA_JACK_PRESENT;
 	else
 		return HDA_JACK_NOT_PRESENT;
@@ -234,7 +264,31 @@ int snd_hda_jack_detect_state(struct hda_codec *codec, hda_nid_t nid)
 EXPORT_SYMBOL_GPL(snd_hda_jack_detect_state);
 
 /**
- * snd_hda_jack_detect_enable - enable the jack-detection
+ * snd_hda_jack_detect_state_mst - query pin Presence Detect status in mst mode
+ * @codec: the CODEC to sense
+ * @nid: the pin NID to sense
+ * @dev_id: device entry id
+ *
+ * Query and return the device entry's Presence Detect status, as either
+ * HDA_JACK_NOT_PRESENT, HDA_JACK_PRESENT or HDA_JACK_PHANTOM.
+ */
+int snd_hda_jack_detect_state_mst(struct hda_codec *codec, hda_nid_t nid,
+					hda_dev_t dev_id)
+{
+	struct hda_jack_tbl *jack =
+		snd_hda_jack_tbl_get_mst(codec, nid, dev_id);
+
+	if (jack && jack->phantom_jack)
+		return HDA_JACK_PHANTOM;
+	else if (snd_hda_pin_sense(codec, nid, dev_id) & AC_PINSENSE_PRESENCE)
+		return HDA_JACK_PRESENT;
+	else
+		return HDA_JACK_NOT_PRESENT;
+}
+EXPORT_SYMBOL_GPL(snd_hda_jack_detect_state_mst);
+
+/**
+ * snd_hda_jack_detect_enable_callback - enable the jack-detection
  * @codec: the HDA codec
  * @nid: pin NID to enable
  * @func: callback function to register
@@ -251,7 +305,7 @@ snd_hda_jack_detect_enable_callback(struct hda_codec *codec, hda_nid_t nid,
 	struct hda_jack_callback *callback = NULL;
 	int err;
 
-	jack = snd_hda_jack_tbl_new(codec, nid);
+	jack = snd_hda_jack_tbl_new(codec, nid, 0);
 	if (!jack)
 		return ERR_PTR(-ENOMEM);
 	if (func) {
@@ -279,6 +333,52 @@ snd_hda_jack_detect_enable_callback(struct hda_codec *codec, hda_nid_t nid,
 EXPORT_SYMBOL_GPL(snd_hda_jack_detect_enable_callback);
 
 /**
+ * snd_hda_jack_detect_enable_callback_mst - enable the jack-detection for mst
+ * @codec: the HDA codec
+ * @nid: pin NID to enable
+ * @dev_id: device entry id
+ * @func: callback function to register
+ *
+ * In the case of error, the return value will be a pointer embedded with
+ * errno.  Check and handle the return value appropriately with standard
+ * macros such as @IS_ERR() and @PTR_ERR().
+ */
+struct hda_jack_callback *
+snd_hda_jack_detect_enable_callback_mst(struct hda_codec *codec, hda_nid_t nid,
+			hda_dev_t dev_id, hda_jack_callback_fn func)
+{
+	struct hda_jack_tbl *jack;
+	struct hda_jack_callback *callback = NULL;
+	int err;
+
+	jack = snd_hda_jack_tbl_new(codec, nid, dev_id);
+	if (!jack)
+		return ERR_PTR(-ENOMEM);
+	if (func) {
+		callback = kzalloc(sizeof(*callback), GFP_KERNEL);
+		if (!callback)
+			return ERR_PTR(-ENOMEM);
+		callback->func = func;
+		callback->tbl = jack;
+		callback->next = jack->callback;
+		jack->callback = callback;
+	}
+
+	if (jack->jack_detect)
+		return callback; /* already registered */
+	jack->jack_detect = 1;
+	if (codec->jackpoll_interval > 0)
+		return callback; /* No unsol if we're polling instead */
+	err = snd_hda_codec_write_cache(codec, nid, 0,
+					 AC_VERB_SET_UNSOLICITED_ENABLE,
+					 AC_USRSP_EN | jack->tag);
+	if (err < 0)
+		return ERR_PTR(err);
+	return callback;
+}
+EXPORT_SYMBOL_GPL(snd_hda_jack_detect_enable_callback_mst);
+
+/**
  * snd_hda_jack_detect_enable - Enable the jack detection on the given pin
  * @codec: the HDA codec
  * @nid: pin NID to enable jack detection
@@ -303,8 +403,9 @@ EXPORT_SYMBOL_GPL(snd_hda_jack_detect_enable);
 int snd_hda_jack_set_gating_jack(struct hda_codec *codec, hda_nid_t gated_nid,
 				 hda_nid_t gating_nid)
 {
-	struct hda_jack_tbl *gated = snd_hda_jack_tbl_new(codec, gated_nid);
-	struct hda_jack_tbl *gating = snd_hda_jack_tbl_new(codec, gating_nid);
+	struct hda_jack_tbl *gated = snd_hda_jack_tbl_new(codec, gated_nid, 0);
+	struct hda_jack_tbl *gating =
+		snd_hda_jack_tbl_new(codec, gating_nid, 0);
 
 	if (!gated || !gating)
 		return -EINVAL;
@@ -369,6 +470,8 @@ static int get_input_jack_type(struct hda_codec *codec, hda_nid_t nid)
 static void hda_free_jack_priv(struct snd_jack *jack)
 {
 	struct hda_jack_tbl *jacks = jack->private_data;
+	if (jacks == NULL)
+		return;
 	jacks->nid = 0;
 	jacks->jack = NULL;
 }
@@ -377,6 +480,7 @@ static void hda_free_jack_priv(struct snd_jack *jack)
  * snd_hda_jack_add_kctl - Add a kctl for the given pin
  * @codec: the HDA codec
  * @nid: pin NID to assign
+ * @dev_id: device entry id
  * @name: string name for the jack
  * @phantom_jack: flag to deal as a phantom jack
  *
@@ -384,17 +488,18 @@ static void hda_free_jack_priv(struct snd_jack *jack)
  * will have the given name and index.
  */
 static int __snd_hda_jack_add_kctl(struct hda_codec *codec, hda_nid_t nid,
-			  const char *name, bool phantom_jack)
+		hda_dev_t dev_id, const char *name, bool phantom_jack)
 {
 	struct hda_jack_tbl *jack;
 	int err, state, type;
 
-	jack = snd_hda_jack_tbl_new(codec, nid);
+	jack = snd_hda_jack_tbl_new(codec, nid, dev_id);
 	if (!jack)
 		return 0;
 	if (jack->jack)
 		return 0; /* already created */
 
+	/* all device entries use same pincfg */
 	type = get_input_jack_type(codec, nid);
 	err = snd_jack_new(codec->card, name, type,
 			   &jack->jack, true, phantom_jack);
@@ -405,7 +510,7 @@ static int __snd_hda_jack_add_kctl(struct hda_codec *codec, hda_nid_t nid,
 	jack->type = type;
 	jack->jack->private_data = jack;
 	jack->jack->private_free = hda_free_jack_priv;
-	state = snd_hda_jack_detect(codec, nid);
+	state = snd_hda_jack_detect_mst(codec, nid, dev_id);
 	snd_jack_report(jack->jack, state ? jack->type : 0);
 
 	return 0;
@@ -422,10 +527,27 @@ static int __snd_hda_jack_add_kctl(struct hda_codec *codec, hda_nid_t nid,
 int snd_hda_jack_add_kctl(struct hda_codec *codec, hda_nid_t nid,
 			  const char *name)
 {
-	return __snd_hda_jack_add_kctl(codec, nid, name, false);
+	return __snd_hda_jack_add_kctl(codec, nid, 0, name, false);
 }
 EXPORT_SYMBOL_GPL(snd_hda_jack_add_kctl);
 
+/**
+ * snd_hda_jack_add_kctl_mst - Add a jack kctl for the given pin for mst mode
+ * @codec: the HDA codec
+ * @nid: pin NID
+ * @dev_id: device entry id
+ * @name: the name string for the jack ctl
+ *
+ * This is a simple helper calling __snd_hda_jack_add_kctl().
+ */
+int snd_hda_jack_add_kctl_mst(struct hda_codec *codec, hda_nid_t nid,
+			hda_dev_t dev_id, const char *name)
+{
+	return __snd_hda_jack_add_kctl(codec, nid, dev_id, name, false);
+}
+EXPORT_SYMBOL_GPL(snd_hda_jack_add_kctl_mst);
+
+
 static int add_jack_kctl(struct hda_codec *codec, hda_nid_t nid,
 			 const struct auto_pin_cfg *cfg,
 			 const char *base_name)
@@ -451,7 +573,7 @@ static int add_jack_kctl(struct hda_codec *codec, hda_nid_t nid,
 	if (phantom_jack)
 		/* Example final name: "Internal Mic Phantom Jack" */
 		strncat(name, " Phantom", sizeof(name) - strlen(name) - 1);
-	err = __snd_hda_jack_add_kctl(codec, nid, name, phantom_jack);
+	err = __snd_hda_jack_add_kctl(codec, nid, 0, name, phantom_jack);
 	if (err < 0)
 		return err;
 
diff --git a/sound/pci/hda/hda_jack.h b/sound/pci/hda/hda_jack.h
index 387d309..c9d4f8e 100644
--- a/sound/pci/hda/hda_jack.h
+++ b/sound/pci/hda/hda_jack.h
@@ -29,6 +29,7 @@ struct hda_jack_callback {
 
 struct hda_jack_tbl {
 	hda_nid_t nid;
+	hda_dev_t dev_id;
 	unsigned char tag;		/* unsol event tag */
 	struct hda_jack_callback *callback;
 	/* jack-detection stuff */
@@ -46,6 +47,9 @@ struct hda_jack_tbl {
 struct hda_jack_tbl *
 snd_hda_jack_tbl_get(struct hda_codec *codec, hda_nid_t nid);
 struct hda_jack_tbl *
+snd_hda_jack_tbl_get_mst(struct hda_codec *codec, hda_nid_t nid,
+				 hda_dev_t dev_id);
+struct hda_jack_tbl *
 snd_hda_jack_tbl_get_from_tag(struct hda_codec *codec, unsigned char tag);
 
 void snd_hda_jack_tbl_clear(struct hda_codec *codec);
@@ -56,11 +60,14 @@ int snd_hda_jack_detect_enable(struct hda_codec *codec, hda_nid_t nid);
 struct hda_jack_callback *
 snd_hda_jack_detect_enable_callback(struct hda_codec *codec, hda_nid_t nid,
 				    hda_jack_callback_fn cb);
+struct hda_jack_callback *
+snd_hda_jack_detect_enable_callback_mst(struct hda_codec *codec, hda_nid_t nid,
+			hda_dev_t dev_id, hda_jack_callback_fn func);
 
 int snd_hda_jack_set_gating_jack(struct hda_codec *codec, hda_nid_t gated_nid,
 				 hda_nid_t gating_nid);
 
-u32 snd_hda_pin_sense(struct hda_codec *codec, hda_nid_t nid);
+u32 snd_hda_pin_sense(struct hda_codec *codec, hda_nid_t nid, hda_dev_t dev_id);
 
 /* the jack state returned from snd_hda_jack_detect_state() */
 enum {
@@ -68,7 +75,8 @@ enum {
 };
 
 int snd_hda_jack_detect_state(struct hda_codec *codec, hda_nid_t nid);
-
+int snd_hda_jack_detect_state_mst(struct hda_codec *codec,
+					hda_nid_t nid, hda_dev_t dev_id);
 /**
  * snd_hda_jack_detect - Detect the jack
  * @codec: the HDA codec
@@ -79,13 +87,27 @@ static inline bool snd_hda_jack_detect(struct hda_codec *codec, hda_nid_t nid)
 	return snd_hda_jack_detect_state(codec, nid) != HDA_JACK_NOT_PRESENT;
 }
 
+/**
+ * snd_hda_jack_detect - Detect the jack
+ * @codec: the HDA codec
+ * @nid: pin NID to check jack detection
+ * @dev_id: device entry id
+ */
+static inline bool snd_hda_jack_detect_mst(struct hda_codec *codec,
+				hda_nid_t nid, hda_dev_t dev_id)
+{
+	return (snd_hda_jack_detect_state_mst(codec, nid, dev_id) !=
+			HDA_JACK_NOT_PRESENT);
+}
+
 bool is_jack_detectable(struct hda_codec *codec, hda_nid_t nid);
 
 int snd_hda_jack_add_kctl(struct hda_codec *codec, hda_nid_t nid,
 			  const char *name);
 int snd_hda_jack_add_kctls(struct hda_codec *codec,
 			   const struct auto_pin_cfg *cfg);
-
+int snd_hda_jack_add_kctl_mst(struct hda_codec *codec, hda_nid_t nid,
+			   hda_dev_t dev_id, const char *name);
 void snd_hda_jack_report_sync(struct hda_codec *codec);
 
 void snd_hda_jack_unsol_event(struct hda_codec *codec, unsigned int res);
diff --git a/sound/pci/hda/patch_hdmi.c b/sound/pci/hda/patch_hdmi.c
index 8d31366..39d68ba 100644
--- a/sound/pci/hda/patch_hdmi.c
+++ b/sound/pci/hda/patch_hdmi.c
@@ -1664,7 +1664,7 @@ static bool hdmi_present_sense(struct hdmi_spec_per_pin *per_pin, int repoll)
 	bool ret;
 
 	snd_hda_power_up_pm(codec);
-	present = snd_hda_pin_sense(codec, pin_nid);
+	present = snd_hda_pin_sense(codec, pin_nid, 0);
 
 	mutex_lock(&per_pin->lock);
 	pin_eld->monitor_present = !!(present & AC_PINSENSE_PRESENCE);
-- 
1.9.1

[RFC PATCH 2/2] ALSA: hda - hdmi audio add dynamically jack binding to pin

[libin.yang]

From: Libin Yang <libin.yang@linux.intel.com>

Jack is attached to PCM statically.

When there is monitor connected, dynamically
bind the Jack to pin based on the PCM assignment.

When monitor is disconnected, unbind the jack from the pin.

Signed-off-by: Libin Yang <libin.yang@linux.intel.com>
---
 sound/pci/hda/hda_jack.c   |  30 +++++++++++-
 sound/pci/hda/hda_jack.h   |   4 ++
 sound/pci/hda/patch_hdmi.c | 111 +++++++++++++++++++++++++++++++++++----------
 3 files changed, 120 insertions(+), 25 deletions(-)

diff --git a/sound/pci/hda/hda_jack.c b/sound/pci/hda/hda_jack.c
index c0a018b..a4a5398 100644
--- a/sound/pci/hda/hda_jack.c
+++ b/sound/pci/hda/hda_jack.c
@@ -134,7 +134,7 @@ EXPORT_SYMBOL_GPL(snd_hda_jack_tbl_get_from_tag);
  * @nid: pin NID to assign
  * @dev_id: device entry id
  */
-static struct hda_jack_tbl *
+struct hda_jack_tbl *
 snd_hda_jack_tbl_new(struct hda_codec *codec, hda_nid_t nid, hda_dev_t dev_id)
 {
 	struct hda_jack_tbl *jack =
@@ -150,6 +150,7 @@ snd_hda_jack_tbl_new(struct hda_codec *codec, hda_nid_t nid, hda_dev_t dev_id)
 	jack->tag = codec->jacktbl.used;
 	return jack;
 }
+EXPORT_SYMBOL_GPL(snd_hda_jack_tbl_new);
 
 void snd_hda_jack_tbl_clear(struct hda_codec *codec)
 {
@@ -517,6 +518,32 @@ static int __snd_hda_jack_add_kctl(struct hda_codec *codec, hda_nid_t nid,
 }
 
 /**
+ * snd_hda_jack_new - create a new snd_jack
+ * @codec: the HDA codec
+ * @jack: Used to provide the allocated jack object to the caller.
+ * @type:  a bitmask of enum snd_jack_type values that can be detected by
+ *         this jack
+ * @id:    an identifying string for this jack
+ * @phantom_jack: Don't create a input device for phantom jacks.
+ *
+ * Return: Zero if successful, or a negative error code on failure.
+ * On success @jack will be initialised.
+ */
+int snd_hda_jack_new(struct hda_codec *codec, struct snd_jack **jack, int type,
+					 const char *id, bool phantom_jack)
+{
+	int err;
+
+	err = snd_jack_new(codec->card, id, type,
+					   jack, true, phantom_jack);
+	if (err < 0)
+		return err;
+	(*jack)->private_free = hda_free_jack_priv;
+	return 0;
+}
+EXPORT_SYMBOL_GPL(snd_hda_jack_new);
+
+/**
  * snd_hda_jack_add_kctl - Add a jack kctl for the given pin
  * @codec: the HDA codec
  * @nid: pin NID
@@ -547,7 +574,6 @@ int snd_hda_jack_add_kctl_mst(struct hda_codec *codec, hda_nid_t nid,
 }
 EXPORT_SYMBOL_GPL(snd_hda_jack_add_kctl_mst);
 
-
 static int add_jack_kctl(struct hda_codec *codec, hda_nid_t nid,
 			 const struct auto_pin_cfg *cfg,
 			 const char *base_name)
diff --git a/sound/pci/hda/hda_jack.h b/sound/pci/hda/hda_jack.h
index c9d4f8e..cb79703 100644
--- a/sound/pci/hda/hda_jack.h
+++ b/sound/pci/hda/hda_jack.h
@@ -51,6 +51,8 @@ snd_hda_jack_tbl_get_mst(struct hda_codec *codec, hda_nid_t nid,
 				 hda_dev_t dev_id);
 struct hda_jack_tbl *
 snd_hda_jack_tbl_get_from_tag(struct hda_codec *codec, unsigned char tag);
+struct hda_jack_tbl *
+snd_hda_jack_tbl_new(struct hda_codec *codec, hda_nid_t nid, hda_dev_t dev_id);
 
 void snd_hda_jack_tbl_clear(struct hda_codec *codec);
 
@@ -104,6 +106,8 @@ bool is_jack_detectable(struct hda_codec *codec, hda_nid_t nid);
 
 int snd_hda_jack_add_kctl(struct hda_codec *codec, hda_nid_t nid,
 			  const char *name);
+int snd_hda_jack_new(struct hda_codec *codec, struct snd_jack **jack, int type,
+					 const char *id, bool phantom_jack);
 int snd_hda_jack_add_kctls(struct hda_codec *codec,
 			   const struct auto_pin_cfg *cfg);
 int snd_hda_jack_add_kctl_mst(struct hda_codec *codec, hda_nid_t nid,
diff --git a/sound/pci/hda/patch_hdmi.c b/sound/pci/hda/patch_hdmi.c
index 39d68ba..c58cf71 100644
--- a/sound/pci/hda/patch_hdmi.c
+++ b/sound/pci/hda/patch_hdmi.c
@@ -143,6 +143,7 @@ struct hdmi_spec {
 	unsigned long pcm_bitmap;
 	struct mutex pcm_lock;
 	int pcm_used;	/* counter of pcm_rec[] */
+	struct snd_jack *jack[16];
 	int dev_num;
 	unsigned int channels_max; /* max over all cvts */
 
@@ -371,14 +372,19 @@ static struct cea_channel_speaker_allocation channel_allocations[] = {
 	((struct hdmi_spec_per_cvt  *)snd_array_elem(&spec->cvts, idx))
 #define get_pcm_rec(spec, idx)	((spec)->pcm_rec[idx])
 
-static int pin_nid_to_pin_index(struct hda_codec *codec, hda_nid_t pin_nid)
+static int pin_id_to_pin_index(struct hda_codec *codec,
+				hda_nid_t pin_nid, hda_dev_t dev_id)
 {
 	struct hdmi_spec *spec = codec->spec;
 	int pin_idx;
+	struct hdmi_spec_per_pin *per_pin;
 
-	for (pin_idx = 0; pin_idx < spec->num_pins; pin_idx++)
-		if (get_pin(spec, pin_idx)->pin_nid == pin_nid)
+	for (pin_idx = 0; pin_idx < spec->num_pins; pin_idx++) {
+		per_pin = get_pin(spec, pin_idx);
+		if ((per_pin->pin_nid == pin_nid) &&
+			(per_pin->dev_id == dev_id))
 			return pin_idx;
+	}
 
 	codec_warn(codec, "HDMI: pin nid %d not registered\n", pin_nid);
 	return -EINVAL;
@@ -1184,13 +1190,15 @@ static void hdmi_setup_audio_infoframe(struct hda_codec *codec,
 
 static bool hdmi_present_sense(struct hdmi_spec_per_pin *per_pin, int repoll);
 
-static void check_presence_and_report(struct hda_codec *codec, hda_nid_t nid)
+static void check_presence_and_report(struct hda_codec *codec,
+						hda_nid_t nid, hda_dev_t dev_id)
 {
 	struct hdmi_spec *spec = codec->spec;
-	int pin_idx = pin_nid_to_pin_index(codec, nid);
+	int pin_idx = pin_id_to_pin_index(codec, nid, dev_id);
 
 	if (pin_idx < 0)
 		return;
+
 	if (hdmi_present_sense(get_pin(spec, pin_idx), 1))
 		snd_hda_jack_report_sync(codec);
 }
@@ -1198,7 +1206,7 @@ static void check_presence_and_report(struct hda_codec *codec, hda_nid_t nid)
 static void jack_callback(struct hda_codec *codec,
 			  struct hda_jack_callback *jack)
 {
-	check_presence_and_report(codec, jack->tbl->nid);
+	check_presence_and_report(codec, jack->tbl->nid, jack->tbl->dev_id);
 }
 
 static void hdmi_intrinsic_event(struct hda_codec *codec, unsigned int res)
@@ -1217,7 +1225,7 @@ static void hdmi_intrinsic_event(struct hda_codec *codec, unsigned int res)
 		codec->addr, jack->nid, dev_entry, !!(res & AC_UNSOL_RES_IA),
 		!!(res & AC_UNSOL_RES_PD), !!(res & AC_UNSOL_RES_ELDV));
 
-	check_presence_and_report(codec, jack->nid);
+	check_presence_and_report(codec, jack->nid, jack->dev_id);
 }
 
 static void hdmi_non_intrinsic_event(struct hda_codec *codec, unsigned int res)
@@ -1642,6 +1650,50 @@ static void hdmi_detach_hda_pcm(struct hdmi_spec *spec,
 	}
 }
 
+/**
+ * snd_hda_jack_bind
+ */
+static void snd_hda_jack_bind(struct hda_codec *codec,
+					  struct hda_jack_tbl *jack,
+					  struct hdmi_spec_per_pin *per_pin)
+{
+	struct hdmi_spec *spec = codec->spec;
+	int state;
+
+	if (per_pin->pcm_idx < 0 || per_pin->pcm_idx >= spec->pcm_used) {
+		codec_warn(codec, "HDMI: pin nid %d dev: %d bind jack fail\n",
+				   per_pin->pin_nid, per_pin->dev_id);
+		return;
+	}
+
+	mutex_lock(&spec->pcm_lock);
+	jack->jack = spec->jack[per_pin->pcm_idx];
+	if (jack->jack == NULL) {
+		mutex_unlock(&spec->pcm_lock);
+		return;
+	}
+
+	jack->jack->private_data = jack;
+	state = snd_hda_jack_detect_mst(codec, per_pin->pin_nid,
+						per_pin->dev_id);
+	snd_jack_report(jack->jack, state ? jack->type : 0);
+	mutex_unlock(&spec->pcm_lock);
+}
+
+static void snd_hda_jack_unbind(struct hda_codec *codec,
+					struct hda_jack_tbl *jack)
+{
+	struct hdmi_spec *spec = codec->spec;
+
+	if (jack->jack == NULL)
+		return;
+	mutex_lock(&spec->pcm_lock);
+	snd_jack_report(jack->jack, 0);
+	jack->jack->private_data = NULL;
+	jack->jack = NULL;
+	mutex_unlock(&spec->pcm_lock);
+}
+
 static bool hdmi_present_sense(struct hdmi_spec_per_pin *per_pin, int repoll)
 {
 	struct hda_jack_tbl *jack;
@@ -1677,6 +1729,7 @@ static bool hdmi_present_sense(struct hdmi_spec_per_pin *per_pin, int repoll)
 		"HDMI status: Codec=%d Pin=%d Presence_Detect=%d ELD_Valid=%d\n",
 		codec->addr, pin_nid, pin_eld->monitor_present, eld->eld_valid);
 
+	jack = snd_hda_jack_tbl_get_mst(codec, pin_nid, per_pin->dev_id);
 	if (eld->eld_valid) {
 		if (spec->ops.pin_get_eld(codec, pin_nid, eld->eld_buffer,
 						     &eld->eld_size) < 0)
@@ -1698,12 +1751,14 @@ static bool hdmi_present_sense(struct hdmi_spec_per_pin *per_pin, int repoll)
 			goto unlock;
 		}
 	}
-
 	/* need be protected by spec->lock */
-	if (eld->eld_valid)
+	if (eld->eld_valid) {
 		hdmi_attach_hda_pcm(spec, per_pin);
-	else
+		snd_hda_jack_bind(codec, jack, per_pin);
+	} else {
+		snd_hda_jack_unbind(codec, jack);
 		hdmi_detach_hda_pcm(spec, per_pin);
+	}
 
 	if (pin_eld->eld_valid != eld->eld_valid)
 		eld_changed = true;
@@ -1750,7 +1805,6 @@ static bool hdmi_present_sense(struct hdmi_spec_per_pin *per_pin, int repoll)
  unlock:
 	ret = !repoll || !pin_eld->monitor_present || pin_eld->eld_valid;
 
-	jack = snd_hda_jack_tbl_get(codec, pin_nid);
 	if (jack)
 		jack->block_report = !ret;
 
@@ -2234,20 +2288,18 @@ static int generic_hdmi_build_pcms(struct hda_codec *codec)
 	return 0;
 }
 
-static int generic_hdmi_build_jack(struct hda_codec *codec, int pin_idx)
+static int generic_hdmi_build_jack(struct hda_codec *codec, int pcm_idx)
 {
 	char hdmi_str[32] = "HDMI/DP";
 	struct hdmi_spec *spec = codec->spec;
-	struct hdmi_spec_per_pin *per_pin = get_pin(spec, pin_idx);
-	int pcmdev = get_pcm_rec(spec, pin_idx)->device;
+	int pcmdev = get_pcm_rec(spec, pcm_idx)->device;
 
 	if (pcmdev > 0)
 		sprintf(hdmi_str + strlen(hdmi_str), ",pcm=%d", pcmdev);
-	if (!is_jack_detectable(codec, per_pin->pin_nid))
-		strncat(hdmi_str, " Phantom",
-			sizeof(hdmi_str) - strlen(hdmi_str) - 1);
 
-	return snd_hda_jack_add_kctl(codec, per_pin->pin_nid, hdmi_str);
+	/* fixme: create the jack: it always SND_JACK_AVOUT, non-phantom */
+	return snd_hda_jack_new(codec, &spec->jack[pcm_idx],
+				SND_JACK_AVOUT, hdmi_str, false);
 }
 
 static int generic_hdmi_build_controls(struct hda_codec *codec)
@@ -2255,13 +2307,26 @@ static int generic_hdmi_build_controls(struct hda_codec *codec)
 	struct hdmi_spec *spec = codec->spec;
 	int err;
 	int pin_idx;
+	int i;
 
-	for (pin_idx = 0; pin_idx < spec->num_pins; pin_idx++) {
-		struct hdmi_spec_per_pin *per_pin = get_pin(spec, pin_idx);
-
-		err = generic_hdmi_build_jack(codec, pin_idx);
+	/* create snd_jack saved in spec->jack[] */
+	for (i = 0; i < spec->pcm_used; i++) {
+		err = generic_hdmi_build_jack(codec, i);
 		if (err < 0)
 			return err;
+	}
+
+	for (pin_idx = 0; pin_idx < spec->num_pins; pin_idx++) {
+		struct hdmi_spec_per_pin *per_pin = get_pin(spec, pin_idx);
+		struct hda_jack_tbl *jack;
+
+		jack = snd_hda_jack_tbl_new(codec, per_pin->pin_nid,
+						per_pin->dev_id);
+		if (jack != NULL) {
+			jack->phantom_jack = false;
+			jack->type = SND_JACK_AVOUT;
+			jack->jack = NULL;
+		}
 
 		err = snd_hda_create_dig_out_ctls(codec,
 						  per_pin->pin_nid,
@@ -2492,7 +2557,7 @@ static void intel_pin_eld_notify(void *audio_ptr, int port)
 	struct hda_codec *codec = audio_ptr;
 	int pin_nid = port + 0x04;
 
-	check_presence_and_report(codec, pin_nid);
+	check_presence_and_report(codec, pin_nid, 0);
 }
 
 static int patch_generic_hdmi(struct hda_codec *codec)
-- 
1.9.1

Re: [RFC PATCH 0/2] ALSA: hda - DP MST audio for Jack support

[Takashi Iwai]

On Tue, 03 Nov 2015 09:42:54 +0100,
libin.yang@linux.intel.com wrote:
> 
> From: Libin Yang <libin.yang@linux.intel.com>
> 
> This is the patch set for Jack support for DP MST audio
> 
> Libin Yang (2):
>   ALSA: hda - jack support DP MST audio
>   ALSA: hda - hdmi audio add dynamically jack binding to pin

I'm somehow confused by this patch series.
Is the unsolicited event handling itself changed for MST?  I mean, you
cannot know which MST dev# is given beforehand, while you seem trying
to assign such a number to the jack object.

We create all jacks statically at probe time.  And in MST case, each
jack is still supposed to be corresponding to each PCM stream, not to
each pin.  So I don't see how this fits with the big picture.

A bit more explanation is needed.


thanks

Takashi

> 
>  sound/pci/hda/hda_jack.c   | 186 ++++++++++++++++++++++++++++++++++++++++-----
>  sound/pci/hda/hda_jack.h   |  32 +++++++-
>  sound/pci/hda/patch_hdmi.c | 116 ++++++++++++++++++++++------
>  3 files changed, 288 insertions(+), 46 deletions(-)
> 
> -- 
> 1.9.1
> 

Re: [RFC PATCH 0/2] ALSA: hda - DP MST audio for Jack support

[Libin Yang]

On 11/04/2015 12:49 AM, Takashi Iwai wrote:
> On Tue, 03 Nov 2015 09:42:54 +0100,
> libin.yang@linux.intel.com wrote:
>>
>> From: Libin Yang <libin.yang@linux.intel.com>
>>
>> This is the patch set for Jack support for DP MST audio
>>
>> Libin Yang (2):
>>    ALSA: hda - jack support DP MST audio
>>    ALSA: hda - hdmi audio add dynamically jack binding to pin
>
> I'm somehow confused by this patch series.
> Is the unsolicited event handling itself changed for MST?  I mean, you
> cannot know which MST dev# is given beforehand, while you seem trying
> to assign such a number to the jack object.

Oh, sorry I forgot to mention that we only operate on device entry 0 
so far in the code. It's misleading.

These patches don't operate on MST audio. The full MST audio driver 
support will be implemented in the later patches. The patches help to 
prepare supporting MST audio.

The purpose of the patches is try to dynamically attach the Jack.

>
> We create all jacks statically at probe time.  And in MST case, each
> jack is still supposed to be corresponding to each PCM stream, not to
> each pin.  So I don't see how this fits with the big picture.

In the generic_hdmi_build_jack(), it will create the snd_jack based on 
the pcm. This is for the userspace. So userspace can see the Jack 
number is the same as PCM. The first Jack for the first PCM, and so 
on. And it is not attached to any pin at first.

In generic_hdmi_build_controls(), it will also create hda_jack_tbl 
based on the pin. The hda_jack_tbl will record the pin information.

When hotplug, in the hdmi_present_sense(), it will call 
xxx_jack_bind() to bind the snd_jack to hda_jack_tbl

>
> A bit more explanation is needed.

I will. Thanks.

Best Regards,
Libin

>
>
> thanks
>
> Takashi
>
>>
>>   sound/pci/hda/hda_jack.c   | 186 ++++++++++++++++++++++++++++++++++++++++-----
>>   sound/pci/hda/hda_jack.h   |  32 +++++++-
>>   sound/pci/hda/patch_hdmi.c | 116 ++++++++++++++++++++++------
>>   3 files changed, 288 insertions(+), 46 deletions(-)
>>
>> --
>> 1.9.1
>>

Re: [RFC PATCH 0/2] ALSA: hda - DP MST audio for Jack support

[Takashi Iwai]

On Wed, 04 Nov 2015 07:23:14 +0100,
Libin Yang wrote:
> 
> 
> On 11/04/2015 12:49 AM, Takashi Iwai wrote:
> > On Tue, 03 Nov 2015 09:42:54 +0100,
> > libin.yang@linux.intel.com wrote:
> >>
> >> From: Libin Yang <libin.yang@linux.intel.com>
> >>
> >> This is the patch set for Jack support for DP MST audio
> >>
> >> Libin Yang (2):
> >>    ALSA: hda - jack support DP MST audio
> >>    ALSA: hda - hdmi audio add dynamically jack binding to pin
> >
> > I'm somehow confused by this patch series.
> > Is the unsolicited event handling itself changed for MST?  I mean, you
> > cannot know which MST dev# is given beforehand, while you seem trying
> > to assign such a number to the jack object.
> 
> Oh, sorry I forgot to mention that we only operate on device entry 0 
> so far in the code. It's misleading.
> 
> These patches don't operate on MST audio. The full MST audio driver 
> support will be implemented in the later patches. The patches help to 
> prepare supporting MST audio.
> 
> The purpose of the patches is try to dynamically attach the Jack.

Well, only judging from the quick glance, I don't buy this strategy,
sorry.  Currently for HDMI/DP, the jack is for PCM, not for pin.
Namely, user-space watches the jack notification to judge whether the
corresponding PCM is available or not.  You're trying to change this
scheme completely.

So, please convince us why this change is needed and which merit /
demerit it'll bring.  Let's start from that point.


thanks,

Takashi

Re: [RFC PATCH 0/2] ALSA: hda - DP MST audio for Jack support

[Yang, Libin]

Hi Takashi,

> -----Original Message-----
> From: Takashi Iwai [mailto:tiwai@suse.de]
> Sent: Monday, November 09, 2015 3:59 PM
> To: Libin Yang
> Cc: alsa-devel@alsa-project.org; mengdong.lin@linux.intel.com; Yang,
> Libin
> Subject: Re: [alsa-devel] [RFC PATCH 0/2] ALSA: hda - DP MST audio for
> Jack support
> 
> On Wed, 04 Nov 2015 07:23:14 +0100,
> Libin Yang wrote:
> >
> >
> > On 11/04/2015 12:49 AM, Takashi Iwai wrote:
> > > On Tue, 03 Nov 2015 09:42:54 +0100,
> > > libin.yang@linux.intel.com wrote:
> > >>
> > >> From: Libin Yang <libin.yang@linux.intel.com>
> > >>
> > >> This is the patch set for Jack support for DP MST audio
> > >>
> > >> Libin Yang (2):
> > >>    ALSA: hda - jack support DP MST audio
> > >>    ALSA: hda - hdmi audio add dynamically jack binding to pin
> > >
> > > I'm somehow confused by this patch series.
> > > Is the unsolicited event handling itself changed for MST?  I mean, you
> > > cannot know which MST dev# is given beforehand, while you seem
> trying
> > > to assign such a number to the jack object.
> >
> > Oh, sorry I forgot to mention that we only operate on device entry 0
> > so far in the code. It's misleading.
> >
> > These patches don't operate on MST audio. The full MST audio driver
> > support will be implemented in the later patches. The patches help to
> > prepare supporting MST audio.
> >
> > The purpose of the patches is try to dynamically attach the Jack.
> 
> Well, only judging from the quick glance, I don't buy this strategy,
> sorry.  Currently for HDMI/DP, the jack is for PCM, not for pin.
> Namely, user-space watches the jack notification to judge whether the
> corresponding PCM is available or not.  You're trying to change this
> scheme completely.

The patch is to make sure the jack is bound to PCM statically.
This patch is needed because in old code,  it will create jack with:
for (pin_idx = 0; pin_idx < spec->num_pins; pin_idx++) {
	...
	err = generic_hdmi_build_jack(codec, pin_idx);
	...
}
Which means the jack is created based on pin number and pin_idx.
As a pin is bound statically to the PCM, the jack is statically bound to PCM. 

Now we are using the dynamic pcm bound. If we use the old code to
create the jack, the jack will be bound to pin. So we need the patch, 
which will use the new code:
for (pcm_idx = 0; pcm_idx < spec-> pcms_used; pcm_idx++) {
	...
	err = generic_hdmi_build_jack(codec, pcm_idx);
	...
}
which makes sure that the jack is bound to PCM (jack[0] is bound 
to pcm[0], jack[1] is bound to pcm[1] and so on).

If there is no monitor, no pin is bound to the PCM. 
So no pin is bound to the Jack. When there is a monitor connected, 
the pin is bound to a proper PCM. So we should bind the PCM's jack
to the pin. 

For example, jack0 is bound to the PCM 3 (the first jack is bound 
to the first PCM statically). When a monitor is connected to pin 5, 
PCM 3 will be assigned to the pin. We need bind jack 0 to pin 5. 
When monitor is disconnected, we must unbind the jack 0 from pin 5. 
These operation are done in function 
snd_hda_jack_bind/ snd_hda_jack_unbind.

Best Regards,
Libin

> 
> So, please convince us why this change is needed and which merit /
> demerit it'll bring.  Let's start from that point.(c)
> 
> 
> thanks,
> 
> Takashi

Re: [RFC PATCH 0/2] ALSA: hda - DP MST audio for Jack support

[Takashi Iwai]

On Mon, 09 Nov 2015 16:00:01 +0100,
Yang, Libin wrote:
> 
> Hi Takashi,
> 
> > -----Original Message-----
> > From: Takashi Iwai [mailto:tiwai@suse.de]
> > Sent: Monday, November 09, 2015 3:59 PM
> > To: Libin Yang
> > Cc: alsa-devel@alsa-project.org; mengdong.lin@linux.intel.com; Yang,
> > Libin
> > Subject: Re: [alsa-devel] [RFC PATCH 0/2] ALSA: hda - DP MST audio for
> > Jack support
> > 
> > On Wed, 04 Nov 2015 07:23:14 +0100,
> > Libin Yang wrote:
> > >
> > >
> > > On 11/04/2015 12:49 AM, Takashi Iwai wrote:
> > > > On Tue, 03 Nov 2015 09:42:54 +0100,
> > > > libin.yang@linux.intel.com wrote:
> > > >>
> > > >> From: Libin Yang <libin.yang@linux.intel.com>
> > > >>
> > > >> This is the patch set for Jack support for DP MST audio
> > > >>
> > > >> Libin Yang (2):
> > > >>    ALSA: hda - jack support DP MST audio
> > > >>    ALSA: hda - hdmi audio add dynamically jack binding to pin
> > > >
> > > > I'm somehow confused by this patch series.
> > > > Is the unsolicited event handling itself changed for MST?  I mean, you
> > > > cannot know which MST dev# is given beforehand, while you seem
> > trying
> > > > to assign such a number to the jack object.
> > >
> > > Oh, sorry I forgot to mention that we only operate on device entry 0
> > > so far in the code. It's misleading.
> > >
> > > These patches don't operate on MST audio. The full MST audio driver
> > > support will be implemented in the later patches. The patches help to
> > > prepare supporting MST audio.
> > >
> > > The purpose of the patches is try to dynamically attach the Jack.
> > 
> > Well, only judging from the quick glance, I don't buy this strategy,
> > sorry.  Currently for HDMI/DP, the jack is for PCM, not for pin.
> > Namely, user-space watches the jack notification to judge whether the
> > corresponding PCM is available or not.  You're trying to change this
> > scheme completely.
> 
> The patch is to make sure the jack is bound to PCM statically.
> This patch is needed because in old code,  it will create jack with:
> for (pin_idx = 0; pin_idx < spec->num_pins; pin_idx++) {
> 	...
> 	err = generic_hdmi_build_jack(codec, pin_idx);
> 	...
> }
> Which means the jack is created based on pin number and pin_idx.
> As a pin is bound statically to the PCM, the jack is statically bound to PCM. 
 
OK, so far, so good.

> Now we are using the dynamic pcm bound. If we use the old code to
> create the jack, the jack will be bound to pin. So we need the patch, 
> which will use the new code:
> for (pcm_idx = 0; pcm_idx < spec-> pcms_used; pcm_idx++) {
> 	...
> 	err = generic_hdmi_build_jack(codec, pcm_idx);
> 	...
> }
> which makes sure that the jack is bound to PCM (jack[0] is bound 
> to pcm[0], jack[1] is bound to pcm[1] and so on).
> 
> If there is no monitor, no pin is bound to the PCM. 
> So no pin is bound to the Jack. When there is a monitor connected, 
> the pin is bound to a proper PCM. So we should bind the PCM's jack
> to the pin. 
> 
> For example, jack0 is bound to the PCM 3 (the first jack is bound 
> to the first PCM statically). When a monitor is connected to pin 5, 
> PCM 3 will be assigned to the pin. We need bind jack 0 to pin 5. 
> When monitor is disconnected, we must unbind the jack 0 from pin 5. 
> These operation are done in function 
> snd_hda_jack_bind/ snd_hda_jack_unbind.

Then we shouldn't use the hda_jack.c code at all for HDMI/DP.
For HDMI/DP with the dynamic PCM binding, the only state to check is
its PCM assignment.  It doesn't have to issue the jack state check via
HDA verb at all.  This should make things a lot easier.


Takashi

Re: [RFC PATCH 0/2] ALSA: hda - DP MST audio for Jack support

[Libin Yang]

Hi Takashi,

On 11/09/2015 11:15 PM, Takashi Iwai wrote:
> On Mon, 09 Nov 2015 16:00:01 +0100,
> Yang, Libin wrote:
>>
>> Hi Takashi,
>>
>>> -----Original Message-----
>>> From: Takashi Iwai [mailto:tiwai@suse.de]
>>> Sent: Monday, November 09, 2015 3:59 PM
>>> To: Libin Yang
>>> Cc: alsa-devel@alsa-project.org; mengdong.lin@linux.intel.com; Yang,
>>> Libin
>>> Subject: Re: [alsa-devel] [RFC PATCH 0/2] ALSA: hda - DP MST audio for
>>> Jack support
>>>
>>> On Wed, 04 Nov 2015 07:23:14 +0100,
>>> Libin Yang wrote:
>>>>
>>>>
>>>> On 11/04/2015 12:49 AM, Takashi Iwai wrote:
>>>>> On Tue, 03 Nov 2015 09:42:54 +0100,
>>>>> libin.yang@linux.intel.com wrote:
>>>>>>
>>>>>> From: Libin Yang <libin.yang@linux.intel.com>
>>>>>>
>>>>>> This is the patch set for Jack support for DP MST audio
>>>>>>
>>>>>> Libin Yang (2):
>>>>>>     ALSA: hda - jack support DP MST audio
>>>>>>     ALSA: hda - hdmi audio add dynamically jack binding to pin
>>>>>
>>>>> I'm somehow confused by this patch series.
>>>>> Is the unsolicited event handling itself changed for MST?  I mean, you
>>>>> cannot know which MST dev# is given beforehand, while you seem
>>> trying
>>>>> to assign such a number to the jack object.
>>>>
>>>> Oh, sorry I forgot to mention that we only operate on device entry 0
>>>> so far in the code. It's misleading.
>>>>
>>>> These patches don't operate on MST audio. The full MST audio driver
>>>> support will be implemented in the later patches. The patches help to
>>>> prepare supporting MST audio.
>>>>
>>>> The purpose of the patches is try to dynamically attach the Jack.
>>>
>>> Well, only judging from the quick glance, I don't buy this strategy,
>>> sorry.  Currently for HDMI/DP, the jack is for PCM, not for pin.
>>> Namely, user-space watches the jack notification to judge whether the
>>> corresponding PCM is available or not.  You're trying to change this
>>> scheme completely.
>>
>> The patch is to make sure the jack is bound to PCM statically.
>> This patch is needed because in old code,  it will create jack with:
>> for (pin_idx = 0; pin_idx < spec->num_pins; pin_idx++) {
>> 	...
>> 	err = generic_hdmi_build_jack(codec, pin_idx);
>> 	...
>> }
>> Which means the jack is created based on pin number and pin_idx.
>> As a pin is bound statically to the PCM, the jack is statically bound to PCM.
>
> OK, so far, so good.
>
>> Now we are using the dynamic pcm bound. If we use the old code to
>> create the jack, the jack will be bound to pin. So we need the patch,
>> which will use the new code:
>> for (pcm_idx = 0; pcm_idx < spec-> pcms_used; pcm_idx++) {
>> 	...
>> 	err = generic_hdmi_build_jack(codec, pcm_idx);
>> 	...
>> }
>> which makes sure that the jack is bound to PCM (jack[0] is bound
>> to pcm[0], jack[1] is bound to pcm[1] and so on).
>>
>> If there is no monitor, no pin is bound to the PCM.
>> So no pin is bound to the Jack. When there is a monitor connected,
>> the pin is bound to a proper PCM. So we should bind the PCM's jack
>> to the pin.
>>
>> For example, jack0 is bound to the PCM 3 (the first jack is bound
>> to the first PCM statically). When a monitor is connected to pin 5,
>> PCM 3 will be assigned to the pin. We need bind jack 0 to pin 5.
>> When monitor is disconnected, we must unbind the jack 0 from pin 5.
>> These operation are done in function
>> snd_hda_jack_bind/ snd_hda_jack_unbind.
>
> Then we shouldn't use the hda_jack.c code at all for HDMI/DP.
> For HDMI/DP with the dynamic PCM binding, the only state to check is
> its PCM assignment.  It doesn't have to issue the jack state check via
> HDA verb at all.  This should make things a lot easier.

I'm now porting the jack supporting for MST audio. The code on 
hdmi-jack branch will create jack differently based codec_has_acomp() 
condition. Does this mean MST Jack binding will handle both situation?

My idea is:
1. Add struct snd_jack *jack[] member in hdmi_spec.
2. Create the jack based on PCM
3. Assign spec->jack[i] to per_pin->acomp_jack when pin is connected.

This should be OK for codec_has_acomp sitation. But what about the 
other situation? The other situation still uses the hda_jack_tbl and 
etc. So we still need handle hda jack if we want to reuse the current 
code.

Best Regards,
Libin

>
>
> Takashi
> _______________________________________________
> Alsa-devel mailing list
> Alsa-devel@alsa-project.org
> http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
>

Re: [RFC PATCH 0/2] ALSA: hda - DP MST audio for Jack support

[Takashi Iwai]

On Tue, 22 Dec 2015 09:47:12 +0100,
Libin Yang wrote:
> 
> Hi Takashi,
> 
> On 11/09/2015 11:15 PM, Takashi Iwai wrote:
> > On Mon, 09 Nov 2015 16:00:01 +0100,
> > Yang, Libin wrote:
> >>
> >> Hi Takashi,
> >>
> >>> -----Original Message-----
> >>> From: Takashi Iwai [mailto:tiwai@suse.de]
> >>> Sent: Monday, November 09, 2015 3:59 PM
> >>> To: Libin Yang
> >>> Cc: alsa-devel@alsa-project.org; mengdong.lin@linux.intel.com; Yang,
> >>> Libin
> >>> Subject: Re: [alsa-devel] [RFC PATCH 0/2] ALSA: hda - DP MST audio for
> >>> Jack support
> >>>
> >>> On Wed, 04 Nov 2015 07:23:14 +0100,
> >>> Libin Yang wrote:
> >>>>
> >>>>
> >>>> On 11/04/2015 12:49 AM, Takashi Iwai wrote:
> >>>>> On Tue, 03 Nov 2015 09:42:54 +0100,
> >>>>> libin.yang@linux.intel.com wrote:
> >>>>>>
> >>>>>> From: Libin Yang <libin.yang@linux.intel.com>
> >>>>>>
> >>>>>> This is the patch set for Jack support for DP MST audio
> >>>>>>
> >>>>>> Libin Yang (2):
> >>>>>>     ALSA: hda - jack support DP MST audio
> >>>>>>     ALSA: hda - hdmi audio add dynamically jack binding to pin
> >>>>>
> >>>>> I'm somehow confused by this patch series.
> >>>>> Is the unsolicited event handling itself changed for MST?  I mean, you
> >>>>> cannot know which MST dev# is given beforehand, while you seem
> >>> trying
> >>>>> to assign such a number to the jack object.
> >>>>
> >>>> Oh, sorry I forgot to mention that we only operate on device entry 0
> >>>> so far in the code. It's misleading.
> >>>>
> >>>> These patches don't operate on MST audio. The full MST audio driver
> >>>> support will be implemented in the later patches. The patches help to
> >>>> prepare supporting MST audio.
> >>>>
> >>>> The purpose of the patches is try to dynamically attach the Jack.
> >>>
> >>> Well, only judging from the quick glance, I don't buy this strategy,
> >>> sorry.  Currently for HDMI/DP, the jack is for PCM, not for pin.
> >>> Namely, user-space watches the jack notification to judge whether the
> >>> corresponding PCM is available or not.  You're trying to change this
> >>> scheme completely.
> >>
> >> The patch is to make sure the jack is bound to PCM statically.
> >> This patch is needed because in old code,  it will create jack with:
> >> for (pin_idx = 0; pin_idx < spec->num_pins; pin_idx++) {
> >> 	...
> >> 	err = generic_hdmi_build_jack(codec, pin_idx);
> >> 	...
> >> }
> >> Which means the jack is created based on pin number and pin_idx.
> >> As a pin is bound statically to the PCM, the jack is statically bound to PCM.
> >
> > OK, so far, so good.
> >
> >> Now we are using the dynamic pcm bound. If we use the old code to
> >> create the jack, the jack will be bound to pin. So we need the patch,
> >> which will use the new code:
> >> for (pcm_idx = 0; pcm_idx < spec-> pcms_used; pcm_idx++) {
> >> 	...
> >> 	err = generic_hdmi_build_jack(codec, pcm_idx);
> >> 	...
> >> }
> >> which makes sure that the jack is bound to PCM (jack[0] is bound
> >> to pcm[0], jack[1] is bound to pcm[1] and so on).
> >>
> >> If there is no monitor, no pin is bound to the PCM.
> >> So no pin is bound to the Jack. When there is a monitor connected,
> >> the pin is bound to a proper PCM. So we should bind the PCM's jack
> >> to the pin.
> >>
> >> For example, jack0 is bound to the PCM 3 (the first jack is bound
> >> to the first PCM statically). When a monitor is connected to pin 5,
> >> PCM 3 will be assigned to the pin. We need bind jack 0 to pin 5.
> >> When monitor is disconnected, we must unbind the jack 0 from pin 5.
> >> These operation are done in function
> >> snd_hda_jack_bind/ snd_hda_jack_unbind.
> >
> > Then we shouldn't use the hda_jack.c code at all for HDMI/DP.
> > For HDMI/DP with the dynamic PCM binding, the only state to check is
> > its PCM assignment.  It doesn't have to issue the jack state check via
> > HDA verb at all.  This should make things a lot easier.
> 
> I'm now porting the jack supporting for MST audio. The code on 
> hdmi-jack branch will create jack differently based codec_has_acomp() 
> condition. Does this mean MST Jack binding will handle both situation?

Ideally, yes.

> My idea is:
> 1. Add struct snd_jack *jack[] member in hdmi_spec.
> 2. Create the jack based on PCM
> 3. Assign spec->jack[i] to per_pin->acomp_jack when pin is connected.
> 
> This should be OK for codec_has_acomp sitation.

Hrm, not really.  As already discussed, a jack is associated to each
PCM, and notified plug/unplug when it's assigned / unassigned.
That is, you don't have to keep extra jack array.  When a PCM is
assigned or unassigned, simply notify per_pin->acomp_jack of that PCM.
That's all.  So there shouldn't be too many difference from the
current situation.

> But what about the 
> other situation? The other situation still uses the hda_jack_tbl and 
> etc. So we still need handle hda jack if we want to reuse the current 
> code.

For other, it's similar.  The jack is associated with PCM, which is
virtually bound with per_pin.  And when it's assigned / unassigned,
call it with the given per_pin->pin_nid.


Takashi

Re: [RFC PATCH 0/2] ALSA: hda - DP MST audio for Jack support

[Yang, Libin]

> -----Original Message-----
> From: Takashi Iwai [mailto:tiwai@suse.de]
> Sent: Tuesday, December 22, 2015 5:00 PM
> To: Libin Yang
> Cc: Yang, Libin; mengdong.lin@linux.intel.com; alsa-devel@alsa-
> project.org
> Subject: Re: [alsa-devel] [RFC PATCH 0/2] ALSA: hda - DP MST audio for
> Jack support
> 
> On Tue, 22 Dec 2015 09:47:12 +0100,
> Libin Yang wrote:
> >
> > Hi Takashi,
> >
> > On 11/09/2015 11:15 PM, Takashi Iwai wrote:
> > > On Mon, 09 Nov 2015 16:00:01 +0100,
> > > Yang, Libin wrote:
> > >>
> > >> Hi Takashi,
> > >>
> > >>> -----Original Message-----
> > >>> From: Takashi Iwai [mailto:tiwai@suse.de]
> > >>> Sent: Monday, November 09, 2015 3:59 PM
> > >>> To: Libin Yang
> > >>> Cc: alsa-devel@alsa-project.org; mengdong.lin@linux.intel.com;
> Yang,
> > >>> Libin
> > >>> Subject: Re: [alsa-devel] [RFC PATCH 0/2] ALSA: hda - DP MST
> audio for
> > >>> Jack support
> > >>>
> > >>> On Wed, 04 Nov 2015 07:23:14 +0100,
> > >>> Libin Yang wrote:
> > >>>>
> > >>>>
> > >>>> On 11/04/2015 12:49 AM, Takashi Iwai wrote:
> > >>>>> On Tue, 03 Nov 2015 09:42:54 +0100,
> > >>>>> libin.yang@linux.intel.com wrote:
> > >>>>>>
> > >>>>>> From: Libin Yang <libin.yang@linux.intel.com>
> > >>>>>>
> > >>>>>> This is the patch set for Jack support for DP MST audio
> > >>>>>>
> > >>>>>> Libin Yang (2):
> > >>>>>>     ALSA: hda - jack support DP MST audio
> > >>>>>>     ALSA: hda - hdmi audio add dynamically jack binding to pin
> > >>>>>
> > >>>>> I'm somehow confused by this patch series.
> > >>>>> Is the unsolicited event handling itself changed for MST?  I mean,
> you
> > >>>>> cannot know which MST dev# is given beforehand, while you
> seem
> > >>> trying
> > >>>>> to assign such a number to the jack object.
> > >>>>
> > >>>> Oh, sorry I forgot to mention that we only operate on device
> entry 0
> > >>>> so far in the code. It's misleading.
> > >>>>
> > >>>> These patches don't operate on MST audio. The full MST audio
> driver
> > >>>> support will be implemented in the later patches. The patches help
> to
> > >>>> prepare supporting MST audio.
> > >>>>
> > >>>> The purpose of the patches is try to dynamically attach the Jack.
> > >>>
> > >>> Well, only judging from the quick glance, I don't buy this strategy,
> > >>> sorry.  Currently for HDMI/DP, the jack is for PCM, not for pin.
> > >>> Namely, user-space watches the jack notification to judge whether
> the
> > >>> corresponding PCM is available or not.  You're trying to change this
> > >>> scheme completely.
> > >>
> > >> The patch is to make sure the jack is bound to PCM statically.
> > >> This patch is needed because in old code,  it will create jack with:
> > >> for (pin_idx = 0; pin_idx < spec->num_pins; pin_idx++) {
> > >> 	...
> > >> 	err = generic_hdmi_build_jack(codec, pin_idx);
> > >> 	...
> > >> }
> > >> Which means the jack is created based on pin number and pin_idx.
> > >> As a pin is bound statically to the PCM, the jack is statically bound to
> PCM.
> > >
> > > OK, so far, so good.
> > >
> > >> Now we are using the dynamic pcm bound. If we use the old code to
> > >> create the jack, the jack will be bound to pin. So we need the patch,
> > >> which will use the new code:
> > >> for (pcm_idx = 0; pcm_idx < spec-> pcms_used; pcm_idx++) {
> > >> 	...
> > >> 	err = generic_hdmi_build_jack(codec, pcm_idx);
> > >> 	...
> > >> }
> > >> which makes sure that the jack is bound to PCM (jack[0] is bound
> > >> to pcm[0], jack[1] is bound to pcm[1] and so on).
> > >>
> > >> If there is no monitor, no pin is bound to the PCM.
> > >> So no pin is bound to the Jack. When there is a monitor connected,
> > >> the pin is bound to a proper PCM. So we should bind the PCM's jack
> > >> to the pin.
> > >>
> > >> For example, jack0 is bound to the PCM 3 (the first jack is bound
> > >> to the first PCM statically). When a monitor is connected to pin 5,
> > >> PCM 3 will be assigned to the pin. We need bind jack 0 to pin 5.
> > >> When monitor is disconnected, we must unbind the jack 0 from pin
> 5.
> > >> These operation are done in function
> > >> snd_hda_jack_bind/ snd_hda_jack_unbind.
> > >
> > > Then we shouldn't use the hda_jack.c code at all for HDMI/DP.
> > > For HDMI/DP with the dynamic PCM binding, the only state to check
> is
> > > its PCM assignment.  It doesn't have to issue the jack state check via
> > > HDA verb at all.  This should make things a lot easier.
> >
> > I'm now porting the jack supporting for MST audio. The code on
> > hdmi-jack branch will create jack differently based codec_has_acomp()
> > condition. Does this mean MST Jack binding will handle both situation?
> 
> Ideally, yes.
> 
> > My idea is:
> > 1. Add struct snd_jack *jack[] member in hdmi_spec.
> > 2. Create the jack based on PCM
> > 3. Assign spec->jack[i] to per_pin->acomp_jack when pin is connected.
> >
> > This should be OK for codec_has_acomp sitation.
> 
> Hrm, not really.  As already discussed, a jack is associated to each
> PCM, and notified plug/unplug when it's assigned / unassigned.
> That is, you don't have to keep extra jack array.  When a PCM is
> assigned or unassigned, simply notify per_pin->acomp_jack of that PCM.
> That's all.  So there shouldn't be too many difference from the
> current situation.

If so, the snd_jack is dynamically bound to PCM. This means
the jack is bound to the pin, and user space must get the pcm-pin
mapping to know the jack is bound to which PCM. Then it can decide
how to operate on the PCM. Not sure it is compatible with current
pulseaudio. 

> 
> > But what about the
> > other situation? The other situation still uses the hda_jack_tbl and
> > etc. So we still need handle hda jack if we want to reuse the current
> > code.
> 
> For other, it's similar.  The jack is associated with PCM, which is
> virtually bound with per_pin.  And when it's assigned / unassigned,
> call it with the given per_pin->pin_nid.
> 
> 
> Takashi

Re: [RFC PATCH 0/2] ALSA: hda - DP MST audio for Jack support

[Takashi Iwai]

On Tue, 22 Dec 2015 12:45:25 +0100,
Yang, Libin wrote:
> 
> 
> > -----Original Message-----
> > From: Takashi Iwai [mailto:tiwai@suse.de]
> > Sent: Tuesday, December 22, 2015 5:00 PM
> > To: Libin Yang
> > Cc: Yang, Libin; mengdong.lin@linux.intel.com; alsa-devel@alsa-
> > project.org
> > Subject: Re: [alsa-devel] [RFC PATCH 0/2] ALSA: hda - DP MST audio for
> > Jack support
> > 
> > On Tue, 22 Dec 2015 09:47:12 +0100,
> > Libin Yang wrote:
> > >
> > > Hi Takashi,
> > >
> > > On 11/09/2015 11:15 PM, Takashi Iwai wrote:
> > > > On Mon, 09 Nov 2015 16:00:01 +0100,
> > > > Yang, Libin wrote:
> > > >>
> > > >> Hi Takashi,
> > > >>
> > > >>> -----Original Message-----
> > > >>> From: Takashi Iwai [mailto:tiwai@suse.de]
> > > >>> Sent: Monday, November 09, 2015 3:59 PM
> > > >>> To: Libin Yang
> > > >>> Cc: alsa-devel@alsa-project.org; mengdong.lin@linux.intel.com;
> > Yang,
> > > >>> Libin
> > > >>> Subject: Re: [alsa-devel] [RFC PATCH 0/2] ALSA: hda - DP MST
> > audio for
> > > >>> Jack support
> > > >>>
> > > >>> On Wed, 04 Nov 2015 07:23:14 +0100,
> > > >>> Libin Yang wrote:
> > > >>>>
> > > >>>>
> > > >>>> On 11/04/2015 12:49 AM, Takashi Iwai wrote:
> > > >>>>> On Tue, 03 Nov 2015 09:42:54 +0100,
> > > >>>>> libin.yang@linux.intel.com wrote:
> > > >>>>>>
> > > >>>>>> From: Libin Yang <libin.yang@linux.intel.com>
> > > >>>>>>
> > > >>>>>> This is the patch set for Jack support for DP MST audio
> > > >>>>>>
> > > >>>>>> Libin Yang (2):
> > > >>>>>>     ALSA: hda - jack support DP MST audio
> > > >>>>>>     ALSA: hda - hdmi audio add dynamically jack binding to pin
> > > >>>>>
> > > >>>>> I'm somehow confused by this patch series.
> > > >>>>> Is the unsolicited event handling itself changed for MST?  I mean,
> > you
> > > >>>>> cannot know which MST dev# is given beforehand, while you
> > seem
> > > >>> trying
> > > >>>>> to assign such a number to the jack object.
> > > >>>>
> > > >>>> Oh, sorry I forgot to mention that we only operate on device
> > entry 0
> > > >>>> so far in the code. It's misleading.
> > > >>>>
> > > >>>> These patches don't operate on MST audio. The full MST audio
> > driver
> > > >>>> support will be implemented in the later patches. The patches help
> > to
> > > >>>> prepare supporting MST audio.
> > > >>>>
> > > >>>> The purpose of the patches is try to dynamically attach the Jack.
> > > >>>
> > > >>> Well, only judging from the quick glance, I don't buy this strategy,
> > > >>> sorry.  Currently for HDMI/DP, the jack is for PCM, not for pin.
> > > >>> Namely, user-space watches the jack notification to judge whether
> > the
> > > >>> corresponding PCM is available or not.  You're trying to change this
> > > >>> scheme completely.
> > > >>
> > > >> The patch is to make sure the jack is bound to PCM statically.
> > > >> This patch is needed because in old code,  it will create jack with:
> > > >> for (pin_idx = 0; pin_idx < spec->num_pins; pin_idx++) {
> > > >> 	...
> > > >> 	err = generic_hdmi_build_jack(codec, pin_idx);
> > > >> 	...
> > > >> }
> > > >> Which means the jack is created based on pin number and pin_idx.
> > > >> As a pin is bound statically to the PCM, the jack is statically bound to
> > PCM.
> > > >
> > > > OK, so far, so good.
> > > >
> > > >> Now we are using the dynamic pcm bound. If we use the old code to
> > > >> create the jack, the jack will be bound to pin. So we need the patch,
> > > >> which will use the new code:
> > > >> for (pcm_idx = 0; pcm_idx < spec-> pcms_used; pcm_idx++) {
> > > >> 	...
> > > >> 	err = generic_hdmi_build_jack(codec, pcm_idx);
> > > >> 	...
> > > >> }
> > > >> which makes sure that the jack is bound to PCM (jack[0] is bound
> > > >> to pcm[0], jack[1] is bound to pcm[1] and so on).
> > > >>
> > > >> If there is no monitor, no pin is bound to the PCM.
> > > >> So no pin is bound to the Jack. When there is a monitor connected,
> > > >> the pin is bound to a proper PCM. So we should bind the PCM's jack
> > > >> to the pin.
> > > >>
> > > >> For example, jack0 is bound to the PCM 3 (the first jack is bound
> > > >> to the first PCM statically). When a monitor is connected to pin 5,
> > > >> PCM 3 will be assigned to the pin. We need bind jack 0 to pin 5.
> > > >> When monitor is disconnected, we must unbind the jack 0 from pin
> > 5.
> > > >> These operation are done in function
> > > >> snd_hda_jack_bind/ snd_hda_jack_unbind.
> > > >
> > > > Then we shouldn't use the hda_jack.c code at all for HDMI/DP.
> > > > For HDMI/DP with the dynamic PCM binding, the only state to check
> > is
> > > > its PCM assignment.  It doesn't have to issue the jack state check via
> > > > HDA verb at all.  This should make things a lot easier.
> > >
> > > I'm now porting the jack supporting for MST audio. The code on
> > > hdmi-jack branch will create jack differently based codec_has_acomp()
> > > condition. Does this mean MST Jack binding will handle both situation?
> > 
> > Ideally, yes.
> > 
> > > My idea is:
> > > 1. Add struct snd_jack *jack[] member in hdmi_spec.
> > > 2. Create the jack based on PCM
> > > 3. Assign spec->jack[i] to per_pin->acomp_jack when pin is connected.
> > >
> > > This should be OK for codec_has_acomp sitation.
> > 
> > Hrm, not really.  As already discussed, a jack is associated to each
> > PCM, and notified plug/unplug when it's assigned / unassigned.
> > That is, you don't have to keep extra jack array.  When a PCM is
> > assigned or unassigned, simply notify per_pin->acomp_jack of that PCM.
> > That's all.  So there shouldn't be too many difference from the
> > current situation.
> 
> If so, the snd_jack is dynamically bound to PCM. This means
> the jack is bound to the pin, and user space must get the pcm-pin
> mapping to know the jack is bound to which PCM. Then it can decide
> how to operate on the PCM. Not sure it is compatible with current
> pulseaudio. 

Erm, I was confused.  Yes, for dynamic attach/detaching PCM, we need
to keep jack object apart from per_pin.  I guess your current patchset
is broken in this regard?

Though, for having the jack pointer, I prefer avoiding to introduce
yet another array but rather extending hdmi_spec.pcm_rec[].  Currently
it contains the hda_pcm pointers.  But we can define a new object,
e.g.

struct hdmi_pcm {
	struct hda_pcm *pcm;
	struct snd_jack *acomp_jack;
	....
};

and keeping this in hdmi_spec.

struct hdmi_spec {
	....
	struct hdmi_pcm pcm_rec[16];
	....
};

get_pcm_rec() macro needs to be adjusted properly, but other than
that, there aren't so many places accessing pcm_rec[] directly, so
easy to convert.

Maybe we can start from converting this at first, moving acomp_jack to
hdmi_spec, then apply MST patchset on its top.  This might be a bit
cleaner.

In the case of non-component: the question beforehand is how to handle
MST PCM assignment at all.  Namely, the non-component case assumes
that there is 1:1 object matching with the actual pin to per_pin.  If
this assumption isn't kept, we'd need to use the same trick like the
component case, and stop using hda_jack infrastructure.
(So acomp_jack might be better renamed to just "jack" or so.)


thanks,

Takashi

Re: [RFC PATCH 0/2] ALSA: hda - DP MST audio for Jack support

[Yang, Libin]

> -----Original Message-----
> From: Takashi Iwai [mailto:tiwai@suse.de]
> Sent: Tuesday, December 22, 2015 8:08 PM
> To: Yang, Libin
> Cc: Libin Yang; mengdong.lin@linux.intel.com; alsa-devel@alsa-
> project.org
> Subject: Re: [alsa-devel] [RFC PATCH 0/2] ALSA: hda - DP MST audio for
> Jack support
> 
> On Tue, 22 Dec 2015 12:45:25 +0100,
> Yang, Libin wrote:
> >
> >
> > > -----Original Message-----
> > > From: Takashi Iwai [mailto:tiwai@suse.de]
> > > Sent: Tuesday, December 22, 2015 5:00 PM
> > > To: Libin Yang
> > > Cc: Yang, Libin; mengdong.lin@linux.intel.com; alsa-devel@alsa-
> > > project.org
> > > Subject: Re: [alsa-devel] [RFC PATCH 0/2] ALSA: hda - DP MST audio
> for
> > > Jack support
> > >
> > > On Tue, 22 Dec 2015 09:47:12 +0100,
> > > Libin Yang wrote:
> > > >
> > > > Hi Takashi,
> > > >
> > > > On 11/09/2015 11:15 PM, Takashi Iwai wrote:
> > > > > On Mon, 09 Nov 2015 16:00:01 +0100,
> > > > > Yang, Libin wrote:
> > > > >>
> > > > >> Hi Takashi,
> > > > >>
> > > > >>> -----Original Message-----
> > > > >>> From: Takashi Iwai [mailto:tiwai@suse.de]
> > > > >>> Sent: Monday, November 09, 2015 3:59 PM
> > > > >>> To: Libin Yang
> > > > >>> Cc: alsa-devel@alsa-project.org; mengdong.lin@linux.intel.com;
> > > Yang,
> > > > >>> Libin
> > > > >>> Subject: Re: [alsa-devel] [RFC PATCH 0/2] ALSA: hda - DP MST
> > > audio for
> > > > >>> Jack support
> > > > >>>
> > > > >>> On Wed, 04 Nov 2015 07:23:14 +0100,
> > > > >>> Libin Yang wrote:
> > > > >>>>
> > > > >>>>
> > > > >>>> On 11/04/2015 12:49 AM, Takashi Iwai wrote:
> > > > >>>>> On Tue, 03 Nov 2015 09:42:54 +0100,
> > > > >>>>> libin.yang@linux.intel.com wrote:
> > > > >>>>>>
> > > > >>>>>> From: Libin Yang <libin.yang@linux.intel.com>
> > > > >>>>>>
> > > > >>>>>> This is the patch set for Jack support for DP MST audio
> > > > >>>>>>
> > > > >>>>>> Libin Yang (2):
> > > > >>>>>>     ALSA: hda - jack support DP MST audio
> > > > >>>>>>     ALSA: hda - hdmi audio add dynamically jack binding to
> pin
> > > > >>>>>
> > > > >>>>> I'm somehow confused by this patch series.
> > > > >>>>> Is the unsolicited event handling itself changed for MST?  I
> mean,
> > > you
> > > > >>>>> cannot know which MST dev# is given beforehand, while you
> > > seem
> > > > >>> trying
> > > > >>>>> to assign such a number to the jack object.
> > > > >>>>
> > > > >>>> Oh, sorry I forgot to mention that we only operate on device
> > > entry 0
> > > > >>>> so far in the code. It's misleading.
> > > > >>>>
> > > > >>>> These patches don't operate on MST audio. The full MST audio
> > > driver
> > > > >>>> support will be implemented in the later patches. The patches
> help
> > > to
> > > > >>>> prepare supporting MST audio.
> > > > >>>>
> > > > >>>> The purpose of the patches is try to dynamically attach the
> Jack.
> > > > >>>
> > > > >>> Well, only judging from the quick glance, I don't buy this
> strategy,
> > > > >>> sorry.  Currently for HDMI/DP, the jack is for PCM, not for pin.
> > > > >>> Namely, user-space watches the jack notification to judge
> whether
> > > the
> > > > >>> corresponding PCM is available or not.  You're trying to change
> this
> > > > >>> scheme completely.
> > > > >>
> > > > >> The patch is to make sure the jack is bound to PCM statically.
> > > > >> This patch is needed because in old code,  it will create jack with:
> > > > >> for (pin_idx = 0; pin_idx < spec->num_pins; pin_idx++) {
> > > > >> 	...
> > > > >> 	err = generic_hdmi_build_jack(codec, pin_idx);
> > > > >> 	...
> > > > >> }
> > > > >> Which means the jack is created based on pin number and
> pin_idx.
> > > > >> As a pin is bound statically to the PCM, the jack is statically bound
> to
> > > PCM.
> > > > >
> > > > > OK, so far, so good.
> > > > >
> > > > >> Now we are using the dynamic pcm bound. If we use the old
> code to
> > > > >> create the jack, the jack will be bound to pin. So we need the
> patch,
> > > > >> which will use the new code:
> > > > >> for (pcm_idx = 0; pcm_idx < spec-> pcms_used; pcm_idx++) {
> > > > >> 	...
> > > > >> 	err = generic_hdmi_build_jack(codec, pcm_idx);
> > > > >> 	...
> > > > >> }
> > > > >> which makes sure that the jack is bound to PCM (jack[0] is bound
> > > > >> to pcm[0], jack[1] is bound to pcm[1] and so on).
> > > > >>
> > > > >> If there is no monitor, no pin is bound to the PCM.
> > > > >> So no pin is bound to the Jack. When there is a monitor
> connected,
> > > > >> the pin is bound to a proper PCM. So we should bind the PCM's
> jack
> > > > >> to the pin.
> > > > >>
> > > > >> For example, jack0 is bound to the PCM 3 (the first jack is bound
> > > > >> to the first PCM statically). When a monitor is connected to pin 5,
> > > > >> PCM 3 will be assigned to the pin. We need bind jack 0 to pin 5.
> > > > >> When monitor is disconnected, we must unbind the jack 0 from
> pin
> > > 5.
> > > > >> These operation are done in function
> > > > >> snd_hda_jack_bind/ snd_hda_jack_unbind.
> > > > >
> > > > > Then we shouldn't use the hda_jack.c code at all for HDMI/DP.
> > > > > For HDMI/DP with the dynamic PCM binding, the only state to
> check
> > > is
> > > > > its PCM assignment.  It doesn't have to issue the jack state check
> via
> > > > > HDA verb at all.  This should make things a lot easier.
> > > >
> > > > I'm now porting the jack supporting for MST audio. The code on
> > > > hdmi-jack branch will create jack differently based
> codec_has_acomp()
> > > > condition. Does this mean MST Jack binding will handle both
> situation?
> > >
> > > Ideally, yes.
> > >
> > > > My idea is:
> > > > 1. Add struct snd_jack *jack[] member in hdmi_spec.
> > > > 2. Create the jack based on PCM
> > > > 3. Assign spec->jack[i] to per_pin->acomp_jack when pin is
> connected.
> > > >
> > > > This should be OK for codec_has_acomp sitation.
> > >
> > > Hrm, not really.  As already discussed, a jack is associated to each
> > > PCM, and notified plug/unplug when it's assigned / unassigned.
> > > That is, you don't have to keep extra jack array.  When a PCM is
> > > assigned or unassigned, simply notify per_pin->acomp_jack of that
> PCM.
> > > That's all.  So there shouldn't be too many difference from the
> > > current situation.
> >
> > If so, the snd_jack is dynamically bound to PCM. This means
> > the jack is bound to the pin, and user space must get the pcm-pin
> > mapping to know the jack is bound to which PCM. Then it can decide
> > how to operate on the PCM. Not sure it is compatible with current
> > pulseaudio.
> 
> Erm, I was confused.  Yes, for dynamic attach/detaching PCM, we need
> to keep jack object apart from per_pin.  I guess your current patchset
> is broken in this regard?
> 
> Though, for having the jack pointer, I prefer avoiding to introduce
> yet another array but rather extending hdmi_spec.pcm_rec[].  Currently
> it contains the hda_pcm pointers.  But we can define a new object,
> e.g.
> 
> struct hdmi_pcm {
> 	struct hda_pcm *pcm;
> 	struct snd_jack *acomp_jack;
> 	....
> };
> 
> and keeping this in hdmi_spec.
> 
> struct hdmi_spec {
> 	....
> 	struct hdmi_pcm pcm_rec[16];
> 	....
> };
> 
> get_pcm_rec() macro needs to be adjusted properly, but other than
> that, there aren't so many places accessing pcm_rec[] directly, so
> easy to convert.
> 
> Maybe we can start from converting this at first, moving acomp_jack to
> hdmi_spec, then apply MST patchset on its top.  This might be a bit
> cleaner.

It's clear now. It should work. I will try it.

> 
> In the case of non-component: the question beforehand is how to handle
> MST PCM assignment at all.  Namely, the non-component case assumes
> that there is 1:1 object matching with the actual pin to per_pin.  If
> this assumption isn't kept, we'd need to use the same trick like the
> component case, and stop using hda_jack infrastructure.
> (So acomp_jack might be better renamed to just "jack" or so.)
> 

If MST PCM is supported, then per_pin is not 1:1 object matching with
the actual pin. It seems we will stop using hda_jack infrastructure.

I will refine the jack patches based on the new method. Thanks for your
suggestion.

Regards,
Libin

> 
> thanks,
> 
> Takashi