[PATCH 0/4] ALSA: hda - hdmi jack support for dynamic pcm assignment

[PATCH 0/4] ALSA: hda - hdmi jack support for dynamic pcm assignment

[libin.yang]

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

The patchset adds hdmi jack support for dynamic pcm assignment

Libin Yang (4):
  ALSA: Add documentation about HD-audio DP MST
  ALSA: hda - add hdmi_pcm to manage hdmi pcm related features
  ALSA: hda - hdmi jack created based on pcm
  ALSA: hda - hdmi monitor hotplug support for dynamic pcm assignment

 Documentation/sound/alsa/HD-Audio-DP-MST-audio.txt |  75 +++++++
 sound/pci/hda/patch_hdmi.c                         | 229 +++++++++++++++------
 2 files changed, 243 insertions(+), 61 deletions(-)
 create mode 100644 Documentation/sound/alsa/HD-Audio-DP-MST-audio.txt

-- 
1.9.1

[PATCH 1/4] ALSA: Add documentation about HD-audio DP MST

[libin.yang]

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

Add the documentation about HD-audio DP MST:
1. Overview
2. Jack

Others will be added later.

Signed-off-by: Libin Yang <libin.yang@linux.intel.com>
---
 Documentation/sound/alsa/HD-Audio-DP-MST-audio.txt | 78 ++++++++++++++++++++++
 1 file changed, 78 insertions(+)
 create mode 100644 Documentation/sound/alsa/HD-Audio-DP-MST-audio.txt

diff --git a/Documentation/sound/alsa/HD-Audio-DP-MST-audio.txt b/Documentation/sound/alsa/HD-Audio-DP-MST-audio.txt
new file mode 100644
index 0000000..ff28e55
--- /dev/null
+++ b/Documentation/sound/alsa/HD-Audio-DP-MST-audio.txt
@@ -0,0 +1,78 @@
+To support DP MST audio, HD Audio hdmi codec driver introduces virtual pin
+and dynamic pcm assignment.
+
+Virtual pin is an extension of per_pin. The most difference of DP MST
+from legacy is that DP MST introduces device entry. Each pin can contain
+several device entries. Each device entry behaves as a pin.
+
+As each pin may contain several device entries and each codec may contain
+several pins, if we use one pcm per per_pin, there will be many PCMs.
+The new solution is to create a few PCMs and to dynamically bind pcm to
+per_pin. Driver uses spec->dyn_pcm_assign flag to indicate whether to use
+the new solution.
+
+PCM
+===
+To be added
+
+
+Jack
+====
+
+Presume:
+ - MST must be dyn_pcm_assign, it can be acomp or !acomp;
+ - NON-MST may or may not be dyn_pcm_assign, it can be acomp or !acomp;
+
+So there are the following scenarios:
+ a.MST (&& dyn_pcm_assign) && acomp
+ b.MST (&& dyn_pcm_assign) && !acomp
+ c.NON-MST && dyn_pcm_assign && acomp
+ d.NON-MST && dyn_pcm_assign && !acomp
+ e.NON-MST && !dyn_pcm_assign && acomp
+ f.NON-MST && !dyn_pcm_assign && !acomp
+
+Below discussion will ignore MST and NON-MST difference as it doesn't
+impact on jack handling too much.
+
+Driver uses new struct hdmi_pcm pcm[] array in hdmi_spec and snd_jack is
+a member of hdmi_pcm. Each pin has one struct hdmi_pcm * pcm pointer.
+
+For !dyn_pcm_assign, per_pin->pcm will assigned to spec->pcm[n] statically.
+
+For dyn_pcm_assign, per_pin->pcm will assigned to spec->pcm[n]
+when monitor is hotplugged.
+
+
+Build Jack
+----------
+
+Build jack based on the pcm. acomp or !acomp will not impact on jack buiding.
+- dyn_pcm_assign
+Will not use hda_jack but use snd_jack in spec->pcm_rec[pcm_idx].jack directly.
+
+- !dyn_pcm_assign
+Use hda_jack and assign spec->pcm_rec[pcm_idx].jack = jack->jack statically.
+
+
+Unsolicited Event Enabling
+--------------------------
+Enable unsolicited event if !acomp.
+
+
+Monitor Hotplug Event Handling
+------------------------------
+- acomp
+pin_eld_notify() -> check_presence_and_report() -> hdmi_present_sense() ->
+sync_eld_via_acomp().
+Use directly snd_jack_report() on spec->pcm_rec[pcm_idx].jack for
+both dyn_pcm_assign and !dyn_pcm_assign
+
+- !acomp
+Hdmi_unsol_event() -> hdmi_intrinsic_event() -> check_presence_and_report() ->
+hdmi_present_sense() -> hdmi_prepsent_sense_via_verbs()
+Use directly snd_jack_report() on spec->pcm_rec[pcm_idx].jack for dyn_pcm_assign.
+Use hda_jack mechanism to handle jack events.
+
+
+Others to be added later
+========================
-- 
1.9.1

[PATCH 2/4] ALSA: hda - add hdmi_pcm to manage hdmi pcm related features

[libin.yang]

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

Use struct hdmi_pcm wrapper for hdmi pcm management.
All PCM related features, like jack,  will be put in this structure.

Signed-off-by: Libin Yang <libin.yang@linux.intel.com>
---
 sound/pci/hda/patch_hdmi.c | 44 ++++++++++++++++++++++++--------------------
 1 file changed, 24 insertions(+), 20 deletions(-)

diff --git a/sound/pci/hda/patch_hdmi.c b/sound/pci/hda/patch_hdmi.c
index 6d6dba4..5549ddf 100644
--- a/sound/pci/hda/patch_hdmi.c
+++ b/sound/pci/hda/patch_hdmi.c
@@ -86,7 +86,7 @@ struct hdmi_spec_per_pin {
 	struct delayed_work work;
 	struct snd_kcontrol *eld_ctl;
 	struct snd_jack *acomp_jack; /* jack via audio component */
-	struct hda_pcm *pcm; /* pointer to spec->pcm_rec[n] dynamically*/
+	struct hdmi_pcm *pcm; /* pointer to spec->pcm_rec[n] dynamically*/
 	int pcm_idx; /* which pcm is attached. -1 means no pcm is attached */
 	int repoll_count;
 	bool setup; /* the stream has been set up by prepare callback */
@@ -132,6 +132,11 @@ struct hdmi_ops {
 	int (*chmap_validate)(int ca, int channels, unsigned char *chmap);
 };
 
+struct hdmi_pcm {
+	struct hda_pcm *pcm;
+	struct snd_jack *jack;
+};
+
 struct hdmi_spec {
 	int num_cvts;
 	struct snd_array cvts; /* struct hdmi_spec_per_cvt */
@@ -139,7 +144,7 @@ struct hdmi_spec {
 
 	int num_pins;
 	struct snd_array pins; /* struct hdmi_spec_per_pin */
-	struct hda_pcm *pcm_rec[16];
+	struct hdmi_pcm pcm_rec[16];
 	struct mutex pcm_lock;
 	/* pcm_bitmap means which pcms have been assigned to pins*/
 	unsigned long pcm_bitmap;
@@ -403,7 +408,7 @@ static int hinfo_to_pcm_index(struct hda_codec *codec,
 	int pcm_idx;
 
 	for (pcm_idx = 0; pcm_idx < spec->pcm_used; pcm_idx++)
-		if (get_pcm_rec(spec, pcm_idx)->stream == hinfo)
+		if (get_pcm_rec(spec, pcm_idx).pcm->stream == hinfo)
 			return pcm_idx;
 
 	codec_warn(codec, "HDMI: hinfo %p not registered\n", hinfo);
@@ -419,7 +424,8 @@ static int hinfo_to_pin_index(struct hda_codec *codec,
 
 	for (pin_idx = 0; pin_idx < spec->num_pins; pin_idx++) {
 		per_pin = get_pin(spec, pin_idx);
-		if (per_pin->pcm && per_pin->pcm->stream == hinfo)
+		if (per_pin->pcm &&
+			per_pin->pcm->pcm->stream == hinfo)
 			return pin_idx;
 	}
 
@@ -1721,7 +1727,7 @@ static void hdmi_attach_hda_pcm(struct hdmi_spec *spec,
 	if (idx == -ENODEV)
 		return;
 	per_pin->pcm_idx = idx;
-	per_pin->pcm = spec->pcm_rec[idx];
+	per_pin->pcm = &spec->pcm_rec[idx];
 	set_bit(idx, &spec->pcm_bitmap);
 }
 
@@ -1764,7 +1770,7 @@ static void hdmi_pcm_setup_pin(struct hdmi_spec *spec,
 	bool non_pcm;
 
 	if (per_pin->pcm_idx >= 0 && per_pin->pcm_idx < spec->pcm_used)
-		pcm = spec->pcm_rec[per_pin->pcm_idx];
+		pcm = spec->pcm_rec[per_pin->pcm_idx].pcm;
 	else
 		return;
 	if (!test_bit(per_pin->pcm_idx, &spec->pcm_in_use))
@@ -2028,8 +2034,10 @@ static int hdmi_add_pin(struct hda_codec *codec, hda_nid_t pin_nid)
 	per_pin->non_pcm = false;
 	if (spec->dyn_pcm_assign)
 		per_pin->pcm_idx = -1;
-	else
+	else {
+		per_pin->pcm = &spec->pcm_rec[pin_idx];
 		per_pin->pcm_idx = pin_idx;
+	}
 	per_pin->pin_nid_idx = pin_idx;
 
 	err = hdmi_read_pin_conn(codec, pin_idx);
@@ -2439,7 +2447,6 @@ static int hdmi_chmap_ctl_put(struct snd_kcontrol *kcontrol,
 static int generic_hdmi_build_pcms(struct hda_codec *codec)
 {
 	struct hdmi_spec *spec = codec->spec;
-	struct hdmi_spec_per_pin *per_pin;
 	int pin_idx;
 
 	for (pin_idx = 0; pin_idx < spec->num_pins; pin_idx++) {
@@ -2449,11 +2456,8 @@ static int generic_hdmi_build_pcms(struct hda_codec *codec)
 		info = snd_hda_codec_pcm_new(codec, "HDMI %d", pin_idx);
 		if (!info)
 			return -ENOMEM;
-		if (!spec->dyn_pcm_assign) {
-			per_pin = get_pin(spec, pin_idx);
-			per_pin->pcm = info;
-		}
-		spec->pcm_rec[pin_idx] = info;
+
+		spec->pcm_rec[pin_idx].pcm = info;
 		spec->pcm_used++;
 		info->pcm_type = HDA_PCM_TYPE_HDMI;
 		info->own_chmap = true;
@@ -2496,7 +2500,7 @@ static int generic_hdmi_build_jack(struct hda_codec *codec, int pin_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, pin_idx).pcm->device;
 	bool phantom_jack;
 
 	if (pcmdev > 0)
@@ -2536,7 +2540,7 @@ static int generic_hdmi_build_controls(struct hda_codec *codec)
 
 		/* add control for ELD Bytes */
 		err = hdmi_create_eld_ctl(codec, pin_idx,
-					  get_pcm_rec(spec, pin_idx)->device);
+				get_pcm_rec(spec, pin_idx).pcm->device);
 
 		if (err < 0)
 			return err;
@@ -2551,7 +2555,7 @@ static int generic_hdmi_build_controls(struct hda_codec *codec)
 		struct snd_kcontrol *kctl;
 		int i;
 
-		pcm = spec->pcm_rec[pin_idx];
+		pcm = spec->pcm_rec[pin_idx].pcm;
 		if (!pcm || !pcm->pcm)
 			break;
 		err = snd_pcm_add_chmap_ctls(pcm->pcm,
@@ -2857,7 +2861,7 @@ static int simple_playback_build_pcms(struct hda_codec *codec)
 	info = snd_hda_codec_pcm_new(codec, "HDMI 0");
 	if (!info)
 		return -ENOMEM;
-	spec->pcm_rec[0] = info;
+	spec->pcm_rec[0].pcm = info;
 	info->pcm_type = HDA_PCM_TYPE_HDMI;
 	pstr = &info->stream[SNDRV_PCM_STREAM_PLAYBACK];
 	*pstr = spec->pcm_playback;
@@ -3306,7 +3310,7 @@ static int nvhdmi_7x_8ch_build_pcms(struct hda_codec *codec)
 	struct hdmi_spec *spec = codec->spec;
 	int err = simple_playback_build_pcms(codec);
 	if (!err) {
-		struct hda_pcm *info = get_pcm_rec(spec, 0);
+		struct hda_pcm *info = get_pcm_rec(spec, 0).pcm;
 		info->own_chmap = true;
 	}
 	return err;
@@ -3324,7 +3328,7 @@ static int nvhdmi_7x_8ch_build_controls(struct hda_codec *codec)
 		return err;
 
 	/* add channel maps */
-	info = get_pcm_rec(spec, 0);
+	info = get_pcm_rec(spec, 0).pcm;
 	err = snd_pcm_add_chmap_ctls(info->pcm,
 				     SNDRV_PCM_STREAM_PLAYBACK,
 				     snd_pcm_alt_chmaps, 8, 0, &chmap);
@@ -3522,7 +3526,7 @@ static struct hda_pcm *hda_find_pcm_by_type(struct hda_codec *codec, int type)
 	unsigned int i;
 
 	for (i = 0; i < spec->num_pins; i++) {
-		struct hda_pcm *pcm = get_pcm_rec(spec, i);
+		struct hda_pcm *pcm = get_pcm_rec(spec, i).pcm;
 
 		if (pcm->pcm_type == type)
 			return pcm;
-- 
1.9.1

[PATCH 3/4] ALSA: hda - hdmi jack created based on pcm

[libin.yang]

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

Jack is created based on pcm.

For dyn_pcm_assign:
 Driver will not use hda_jack. It will operate snd_jack directly.
 snd_jack pointer will be stored in spec->pcm.jack. When pcm is
 assigned to pin, jack will be assigned to pin automatically.
For !dyn_pcm_assign:
 Driver will continue using hda_jack for less impact on the old cases.
 Pcm is statically assigned to pin. So is jack. spec->pcm.jack will
 save the snd_jack pointer created in hda_jack.

Signed-off-by: Libin Yang <libin.yang@linux.intel.com>
---
 sound/pci/hda/patch_hdmi.c | 96 ++++++++++++++++++++++++++++++++--------------
 1 file changed, 67 insertions(+), 29 deletions(-)

diff --git a/sound/pci/hda/patch_hdmi.c b/sound/pci/hda/patch_hdmi.c
index 5549ddf..356ae04 100644
--- a/sound/pci/hda/patch_hdmi.c
+++ b/sound/pci/hda/patch_hdmi.c
@@ -85,7 +85,6 @@ struct hdmi_spec_per_pin {
 	struct mutex lock;
 	struct delayed_work work;
 	struct snd_kcontrol *eld_ctl;
-	struct snd_jack *acomp_jack; /* jack via audio component */
 	struct hdmi_pcm *pcm; /* pointer to spec->pcm_rec[n] dynamically*/
 	int pcm_idx; /* which pcm is attached. -1 means no pcm is attached */
 	int repoll_count;
@@ -1943,6 +1942,7 @@ static void sync_eld_via_acomp(struct hda_codec *codec,
 {
 	struct hdmi_spec *spec = codec->spec;
 	struct hdmi_eld *eld = &spec->temp_eld;
+	struct snd_jack *jack = NULL;
 	int size;
 
 	mutex_lock(&per_pin->lock);
@@ -1966,8 +1966,18 @@ static void sync_eld_via_acomp(struct hda_codec *codec,
 		eld->eld_size = 0;
 	}
 
+	/* pcm_idx >=0 before update_eld() means it is in monitor
+	 * disconnected event. Jack must be fetched before update_eld()
+	 * Get jack before update_eld()
+	 */
+	if (per_pin->pcm_idx >= 0)
+		jack = spec->pcm_rec[per_pin->pcm_idx].jack;
 	update_eld(codec, per_pin, eld);
-	snd_jack_report(per_pin->acomp_jack,
+	if (jack == NULL && per_pin->pcm_idx >= 0)
+		jack = spec->pcm_rec[per_pin->pcm_idx].jack;
+	if (jack == NULL)
+		goto unlock;
+	snd_jack_report(jack,
 			eld->monitor_present ? SND_JACK_AVOUT : 0);
  unlock:
 	mutex_unlock(&per_pin->lock);
@@ -2471,15 +2481,16 @@ static int generic_hdmi_build_pcms(struct hda_codec *codec)
 	return 0;
 }
 
-static void free_acomp_jack_priv(struct snd_jack *jack)
+static void free_hdmi_jack_priv(struct snd_jack *jack)
 {
-	struct hdmi_spec_per_pin *per_pin = jack->private_data;
+	struct hdmi_pcm *pcm = jack->private_data;
 
-	per_pin->acomp_jack = NULL;
+	pcm->jack = NULL;
 }
 
-static int add_acomp_jack_kctl(struct hda_codec *codec,
-			       struct hdmi_spec_per_pin *per_pin,
+static int add_hdmi_jack_kctl(struct hda_codec *codec,
+			       struct hdmi_spec *spec,
+			       int pcm_idx,
 			       const char *name)
 {
 	struct snd_jack *jack;
@@ -2489,45 +2500,67 @@ static int add_acomp_jack_kctl(struct hda_codec *codec,
 			   true, false);
 	if (err < 0)
 		return err;
-	per_pin->acomp_jack = jack;
-	jack->private_data = per_pin;
-	jack->private_free = free_acomp_jack_priv;
+
+	spec->pcm_rec[pcm_idx].jack = jack;
+	jack->private_data = &spec->pcm_rec[pcm_idx];
+	jack->private_free = free_hdmi_jack_priv;
 	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).pcm->device;
+	struct hdmi_spec_per_pin *per_pin;
+	struct hda_jack_tbl *jack;
+	int pcmdev = get_pcm_rec(spec, pcm_idx).pcm->device;
 	bool phantom_jack;
+	int ret;
 
 	if (pcmdev > 0)
 		sprintf(hdmi_str + strlen(hdmi_str), ",pcm=%d", pcmdev);
-	if (codec_has_acomp(codec))
-		return add_acomp_jack_kctl(codec, per_pin, hdmi_str);
-	phantom_jack = !is_jack_detectable(codec, per_pin->pin_nid);
-	if (phantom_jack)
-		strncat(hdmi_str, " Phantom",
-			sizeof(hdmi_str) - strlen(hdmi_str) - 1);
 
-	return snd_hda_jack_add_kctl(codec, per_pin->pin_nid, hdmi_str,
-				     phantom_jack);
+	/* for !dyn_pcm_assign, we still use hda_jack for compatibility */
+	if (!spec->dyn_pcm_assign) {
+		/* if not dyn_pcm_assign, it must be non-MST mode.
+		 * This means pcms and pins are statically mapped.
+		 * And pcm_idx is pin_idx.
+		 */
+		per_pin = get_pin(spec, pcm_idx);
+		phantom_jack = !is_jack_detectable(codec, per_pin->pin_nid);
+		if (phantom_jack)
+			strncat(hdmi_str, " Phantom",
+				sizeof(hdmi_str) - strlen(hdmi_str) - 1);
+		ret = snd_hda_jack_add_kctl(codec, per_pin->pin_nid, hdmi_str,
+					    phantom_jack);
+		if (ret < 0)
+			return ret;
+		jack = snd_hda_jack_tbl_get(codec, per_pin->pin_nid);
+		if (jack == NULL)
+			return 0;
+		/* statically bind jack */
+		spec->pcm_rec[pcm_idx].jack = jack->jack;
+		return 0;
+	}
+
+	return add_hdmi_jack_kctl(codec, spec, pcm_idx, hdmi_str);
 }
 
 static int generic_hdmi_build_controls(struct hda_codec *codec)
 {
 	struct hdmi_spec *spec = codec->spec;
 	int err;
-	int pin_idx;
+	int pin_idx, pcm_idx;
 
-	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);
+	for (pcm_idx = 0; pcm_idx < spec->pcm_used; pcm_idx++) {
+		err = generic_hdmi_build_jack(codec, pcm_idx);
 		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);
 
 		err = snd_hda_create_dig_out_ctls(codec,
 						  per_pin->pin_nid,
@@ -2626,18 +2659,23 @@ static void hdmi_array_free(struct hdmi_spec *spec)
 static void generic_hdmi_free(struct hda_codec *codec)
 {
 	struct hdmi_spec *spec = codec->spec;
-	int pin_idx;
+	int pin_idx, pcm_idx;
 
 	if (codec_has_acomp(codec))
 		snd_hdac_i915_register_notifier(NULL);
 
 	for (pin_idx = 0; pin_idx < spec->num_pins; pin_idx++) {
 		struct hdmi_spec_per_pin *per_pin = get_pin(spec, pin_idx);
-
 		cancel_delayed_work_sync(&per_pin->work);
 		eld_proc_free(per_pin);
-		if (per_pin->acomp_jack)
-			snd_device_free(codec->card, per_pin->acomp_jack);
+	}
+
+	for (pcm_idx = 0; pcm_idx < spec->pcm_used; pcm_idx++) {
+		if (spec->dyn_pcm_assign)
+			snd_device_free(codec->card,
+					spec->pcm_rec[pcm_idx].jack);
+		else
+			spec->pcm_rec[pcm_idx].jack = NULL;
 	}
 
 	if (spec->i915_bound)
-- 
1.9.1

[PATCH 4/4] ALSA: hda - hdmi monitor hotplug support for dynamic pcm assignment

[libin.yang]

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

This patch adds the support for monitor hotplug of dynamic pcm assignment.

1. unsol_event enabling
 - For codec_has_acomp, unsol_event is disabled.
 - For !codec_has_acomp && !dyn_pcm_assign, use the hda_jack helper to
   enable unsol_event
 - For !codec_has_acomp && dyn_pcm_assign, enable unsol_event with verb
   directly

2. unsol_event handling
 - For !dyn_pcm_assign, use hda_jack helper to report the event
 - For dyn_pcm_assign, use snd_jack_report() directly

Signed-off-by: Libin Yang <libin.yang@linux.intel.com>
---
 sound/pci/hda/patch_hdmi.c | 97 ++++++++++++++++++++++++++++++++++++++--------
 1 file changed, 81 insertions(+), 16 deletions(-)

diff --git a/sound/pci/hda/patch_hdmi.c b/sound/pci/hda/patch_hdmi.c
index 356ae04..293b0e3 100644
--- a/sound/pci/hda/patch_hdmi.c
+++ b/sound/pci/hda/patch_hdmi.c
@@ -87,6 +87,7 @@ struct hdmi_spec_per_pin {
 	struct snd_kcontrol *eld_ctl;
 	struct hdmi_pcm *pcm; /* pointer to spec->pcm_rec[n] dynamically*/
 	int pcm_idx; /* which pcm is attached. -1 means no pcm is attached */
+	int unsol_tag;
 	int repoll_count;
 	bool setup; /* the stream has been set up by prepare callback */
 	int channels; /* current number of channels */
@@ -1229,23 +1230,44 @@ static void jack_callback(struct hda_codec *codec,
 	check_presence_and_report(codec, jack->tbl->nid);
 }
 
+static hda_nid_t hdmi_get_nid_from_tag(struct hda_codec *codec, int tag)
+{
+	int pin_idx;
+	struct hdmi_spec *spec = codec->spec;
+
+	for (pin_idx = 0; pin_idx < spec->num_pins; pin_idx++) {
+		struct hdmi_spec_per_pin *per_pin = get_pin(spec, pin_idx);
+
+		if (per_pin->unsol_tag == tag)
+			return per_pin->pin_nid;
+	}
+	/* this will never happen, tag is checked before */
+	return 0;
+}
+
 static void hdmi_intrinsic_event(struct hda_codec *codec, unsigned int res)
 {
 	int tag = res >> AC_UNSOL_RES_TAG_SHIFT;
 	struct hda_jack_tbl *jack;
+	struct hdmi_spec *spec = codec->spec;
 	int dev_entry = (res & AC_UNSOL_RES_DE) >> AC_UNSOL_RES_DE_SHIFT;
+	hda_nid_t nid;
 
-	jack = snd_hda_jack_tbl_get_from_tag(codec, tag);
-	if (!jack)
-		return;
-	jack->jack_dirty = 1;
+	if (!spec->dyn_pcm_assign) {
+		jack = snd_hda_jack_tbl_get_from_tag(codec, tag);
+		if (!jack)
+			return;
+		jack->jack_dirty = 1;
+		nid = jack->nid;
+	} else
+		nid = hdmi_get_nid_from_tag(codec, tag);
 
 	codec_dbg(codec,
 		"HDMI hot plug event: Codec=%d Pin=%d Device=%d Inactive=%d Presence_Detect=%d ELD_Valid=%d\n",
-		codec->addr, jack->nid, dev_entry, !!(res & AC_UNSOL_RES_IA),
+		codec->addr, 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, nid);
 }
 
 static void hdmi_non_intrinsic_event(struct hda_codec *codec, unsigned int res)
@@ -1270,13 +1292,30 @@ static void hdmi_non_intrinsic_event(struct hda_codec *codec, unsigned int res)
 		;
 }
 
+static bool hdmi_unsol_tag_is_valid(struct hda_codec *codec, int tag)
+{
+	struct hdmi_spec *spec = codec->spec;
+	int i;
+
+	if (!spec->dyn_pcm_assign) {
+		if (snd_hda_jack_tbl_get_from_tag(codec, tag))
+			return true;
+	} else {
+		for (i = 0; i < spec->num_pins; i++) {
+			if (get_pin(spec, i)->unsol_tag == tag)
+				return true;
+		}
+	}
+
+	return false;
+}
 
 static void hdmi_unsol_event(struct hda_codec *codec, unsigned int res)
 {
 	int tag = res >> AC_UNSOL_RES_TAG_SHIFT;
 	int subtag = (res & AC_UNSOL_RES_SUBTAG) >> AC_UNSOL_RES_SUBTAG_SHIFT;
 
-	if (!snd_hda_jack_tbl_get_from_tag(codec, tag)) {
+	if (!hdmi_unsol_tag_is_valid(codec, tag)) {
 		codec_dbg(codec, "Unexpected HDMI event tag 0x%x\n", tag);
 		return;
 	}
@@ -1875,7 +1914,8 @@ static void update_eld(struct hda_codec *codec,
 static bool hdmi_present_sense_via_verbs(struct hdmi_spec_per_pin *per_pin,
 					 int repoll)
 {
-	struct hda_jack_tbl *jack;
+	struct hda_jack_tbl *jack = NULL;
+	struct snd_jack *sjack = NULL;
 	struct hda_codec *codec = per_pin->codec;
 	struct hdmi_spec *spec = codec->spec;
 	struct hdmi_eld *eld = &spec->temp_eld;
@@ -1890,7 +1930,7 @@ static bool hdmi_present_sense_via_verbs(struct hdmi_spec_per_pin *per_pin,
 	 * the unsolicited response to avoid custom WARs.
 	 */
 	int present;
-	bool ret;
+	bool ret = false;
 	bool do_repoll = false;
 
 	snd_hda_power_up_pm(codec);
@@ -1920,14 +1960,26 @@ static bool hdmi_present_sense_via_verbs(struct hdmi_spec_per_pin *per_pin,
 			do_repoll = true;
 	}
 
+	if (spec->dyn_pcm_assign && per_pin->pcm_idx >= 0) {
+		sjack = spec->pcm_rec[per_pin->pcm_idx].jack;
+		snd_jack_report(sjack,
+			pin_eld->monitor_present ? SND_JACK_AVOUT : 0);
+	}
+
 	if (do_repoll)
 		schedule_delayed_work(&per_pin->work, msecs_to_jiffies(300));
 	else
 		update_eld(codec, per_pin, eld);
 
-	ret = !repoll || !pin_eld->monitor_present || pin_eld->eld_valid;
-
-	jack = snd_hda_jack_tbl_get(codec, pin_nid);
+	if (!spec->dyn_pcm_assign) {
+		ret = !repoll || !pin_eld->monitor_present ||
+			pin_eld->eld_valid;
+		jack = snd_hda_jack_tbl_get(codec, pin_nid);
+	} else if (per_pin->pcm_idx >= 0 && !sjack) {
+		sjack = spec->pcm_rec[per_pin->pcm_idx].jack;
+		snd_jack_report(sjack,
+			pin_eld->monitor_present ? SND_JACK_AVOUT : 0);
+	}
 	if (jack)
 		jack->block_report = !ret;
 
@@ -2511,7 +2563,7 @@ 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;
+	struct hdmi_spec_per_pin *per_pin = get_pin(spec, pcm_idx);
 	struct hda_jack_tbl *jack;
 	int pcmdev = get_pcm_rec(spec, pcm_idx).pcm->device;
 	bool phantom_jack;
@@ -2526,7 +2578,7 @@ static int generic_hdmi_build_jack(struct hda_codec *codec, int pcm_idx)
 		 * This means pcms and pins are statically mapped.
 		 * And pcm_idx is pin_idx.
 		 */
-		per_pin = get_pin(spec, pcm_idx);
+
 		phantom_jack = !is_jack_detectable(codec, per_pin->pin_nid);
 		if (phantom_jack)
 			strncat(hdmi_str, " Phantom",
@@ -2540,10 +2592,17 @@ static int generic_hdmi_build_jack(struct hda_codec *codec, int pcm_idx)
 			return 0;
 		/* statically bind jack */
 		spec->pcm_rec[pcm_idx].jack = jack->jack;
+		per_pin->unsol_tag = jack->tag;
 		return 0;
 	}
 
-	return add_hdmi_jack_kctl(codec, spec, pcm_idx, hdmi_str);
+	per_pin->unsol_tag = per_pin->pin_nid_idx + 1;
+	ret = add_hdmi_jack_kctl(codec, spec, pcm_idx, hdmi_str);
+	if (!codec_has_acomp(codec))
+		snd_hda_codec_write_cache(codec, per_pin->pin_nid, 0,
+				AC_VERB_SET_UNSOLICITED_ENABLE,
+				AC_USRSP_EN | per_pin->unsol_tag);
+	return ret;
 }
 
 static int generic_hdmi_build_controls(struct hda_codec *codec)
@@ -2636,7 +2695,13 @@ static int generic_hdmi_init(struct hda_codec *codec)
 		hda_nid_t pin_nid = per_pin->pin_nid;
 
 		hdmi_init_pin(codec, pin_nid);
-		if (!codec_has_acomp(codec))
+		/* if codec_has_acomp, will not enable unsol event
+		 * if !codec_has_acomp && ! dyn_pcm_assign,
+		 *  will use hda_jack
+		 * if !codec_has_acomp && dyn_pcm_assign,
+		 * enable unsol_event when building jack
+		 */
+		if (!codec_has_acomp(codec) && !spec->dyn_pcm_assign)
 			snd_hda_jack_detect_enable_callback(codec, pin_nid,
 				codec->jackpoll_interval > 0 ?
 				jack_callback : NULL);
-- 
1.9.1

Re: [PATCH 0/4] ALSA: hda - hdmi jack support for dynamic pcm assignment

[Takashi Iwai]

On Thu, 31 Dec 2015 02:22:18 +0100,
libin.yang@linux.intel.com wrote:
> 
> From: Libin Yang <libin.yang@linux.intel.com>
> 
> The patchset adds hdmi jack support for dynamic pcm assignment

To which branch is this supposed to be applied?


thanks,

Takashi

> 
> Libin Yang (4):
>   ALSA: Add documentation about HD-audio DP MST
>   ALSA: hda - add hdmi_pcm to manage hdmi pcm related features
>   ALSA: hda - hdmi jack created based on pcm
>   ALSA: hda - hdmi monitor hotplug support for dynamic pcm assignment
> 
>  Documentation/sound/alsa/HD-Audio-DP-MST-audio.txt |  75 +++++++
>  sound/pci/hda/patch_hdmi.c                         | 229 +++++++++++++++------
>  2 files changed, 243 insertions(+), 61 deletions(-)
>  create mode 100644 Documentation/sound/alsa/HD-Audio-DP-MST-audio.txt
> 
> -- 
> 1.9.1
> 

Re: [PATCH 0/4] ALSA: hda - hdmi jack support for dynamic pcm assignment

[Yang, Libin]

Hi Takashi,

> -----Original Message-----
> From: Takashi Iwai [mailto:tiwai@suse.de]
> Sent: Wednesday, January 06, 2016 5:18 PM
> To: libin.yang@linux.intel.com
> Cc: alsa-devel@alsa-project.org; Lin, Mengdong; Yang, Libin
> Subject: Re: [alsa-devel] [PATCH 0/4] ALSA: hda - hdmi jack support for
> dynamic pcm assignment
> 
> On Thu, 31 Dec 2015 02:22:18 +0100,
> libin.yang@linux.intel.com wrote:
> >
> > From: Libin Yang <libin.yang@linux.intel.com>
> >
> > The patchset adds hdmi jack support for dynamic pcm assignment
> 
> To which branch is this supposed to be applied?

This is preparation for DP MST audio. And it is for hdmi-jack branch.

Regards,
Libin

> 
> 
> thanks,
> 
> Takashi
> 
> >
> > Libin Yang (4):
> >   ALSA: Add documentation about HD-audio DP MST
> >   ALSA: hda - add hdmi_pcm to manage hdmi pcm related features
> >   ALSA: hda - hdmi jack created based on pcm
> >   ALSA: hda - hdmi monitor hotplug support for dynamic pcm
> assignment
> >
> >  Documentation/sound/alsa/HD-Audio-DP-MST-audio.txt |  75
> +++++++
> >  sound/pci/hda/patch_hdmi.c                         | 229 +++++++++++++++----
> --
> >  2 files changed, 243 insertions(+), 61 deletions(-)
> >  create mode 100644 Documentation/sound/alsa/HD-Audio-DP-MST-
> audio.txt
> >
> > --
> > 1.9.1
> >

Re: [PATCH 0/4] ALSA: hda - hdmi jack support for dynamic pcm assignment

[Takashi Iwai]

On Thu, 07 Jan 2016 02:41:13 +0100,
Yang, Libin wrote:
> 
> Hi Takashi,
> 
> > -----Original Message-----
> > From: Takashi Iwai [mailto:tiwai@suse.de]
> > Sent: Wednesday, January 06, 2016 5:18 PM
> > To: libin.yang@linux.intel.com
> > Cc: alsa-devel@alsa-project.org; Lin, Mengdong; Yang, Libin
> > Subject: Re: [alsa-devel] [PATCH 0/4] ALSA: hda - hdmi jack support for
> > dynamic pcm assignment
> > 
> > On Thu, 31 Dec 2015 02:22:18 +0100,
> > libin.yang@linux.intel.com wrote:
> > >
> > > From: Libin Yang <libin.yang@linux.intel.com>
> > >
> > > The patchset adds hdmi jack support for dynamic pcm assignment
> > 
> > To which branch is this supposed to be applied?
> 
> This is preparation for DP MST audio. And it is for hdmi-jack branch.

Do you mean my test/hdmi-jack branch?  So these are additional fixes
on top of your former MST patches?  If yes, it should go to
topic/hda-mst branch instead.

test/hdmi-jack branch contains two unofficial patches in addition to
your i915 MST patches that haven't been merged yet.


Takashi

> 
> Regards,
> Libin
> 
> > 
> > 
> > thanks,
> > 
> > Takashi
> > 
> > >
> > > Libin Yang (4):
> > >   ALSA: Add documentation about HD-audio DP MST
> > >   ALSA: hda - add hdmi_pcm to manage hdmi pcm related features
> > >   ALSA: hda - hdmi jack created based on pcm
> > >   ALSA: hda - hdmi monitor hotplug support for dynamic pcm
> > assignment
> > >
> > >  Documentation/sound/alsa/HD-Audio-DP-MST-audio.txt |  75
> > +++++++
> > >  sound/pci/hda/patch_hdmi.c                         | 229 +++++++++++++++----
> > --
> > >  2 files changed, 243 insertions(+), 61 deletions(-)
> > >  create mode 100644 Documentation/sound/alsa/HD-Audio-DP-MST-
> > audio.txt
> > >
> > > --
> > > 1.9.1
> > >
> _______________________________________________
> Alsa-devel mailing list
> Alsa-devel@alsa-project.org
> http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
> 

Re: [PATCH 2/4] ALSA: hda - add hdmi_pcm to manage hdmi pcm related features

[Takashi Iwai]

On Thu, 31 Dec 2015 02:22:20 +0100,
libin.yang@linux.intel.com wrote:
> 
> From: Libin Yang <libin.yang@linux.intel.com>
> 
> Use struct hdmi_pcm wrapper for hdmi pcm management.
> All PCM related features, like jack,  will be put in this structure.
> 
> Signed-off-by: Libin Yang <libin.yang@linux.intel.com>
> ---
>  sound/pci/hda/patch_hdmi.c | 44 ++++++++++++++++++++++++--------------------
>  1 file changed, 24 insertions(+), 20 deletions(-)
> 
> diff --git a/sound/pci/hda/patch_hdmi.c b/sound/pci/hda/patch_hdmi.c
> index 6d6dba4..5549ddf 100644
> --- a/sound/pci/hda/patch_hdmi.c
> +++ b/sound/pci/hda/patch_hdmi.c
> @@ -86,7 +86,7 @@ struct hdmi_spec_per_pin {
>  	struct delayed_work work;
>  	struct snd_kcontrol *eld_ctl;
>  	struct snd_jack *acomp_jack; /* jack via audio component */
> -	struct hda_pcm *pcm; /* pointer to spec->pcm_rec[n] dynamically*/
> +	struct hdmi_pcm *pcm; /* pointer to spec->pcm_rec[n] dynamically*/
>  	int pcm_idx; /* which pcm is attached. -1 means no pcm is attached */
>  	int repoll_count;
>  	bool setup; /* the stream has been set up by prepare callback */
> @@ -132,6 +132,11 @@ struct hdmi_ops {
>  	int (*chmap_validate)(int ca, int channels, unsigned char *chmap);
>  };
>  
> +struct hdmi_pcm {
> +	struct hda_pcm *pcm;
> +	struct snd_jack *jack;
> +};
> +
>  struct hdmi_spec {
>  	int num_cvts;
>  	struct snd_array cvts; /* struct hdmi_spec_per_cvt */
> @@ -139,7 +144,7 @@ struct hdmi_spec {
>  
>  	int num_pins;
>  	struct snd_array pins; /* struct hdmi_spec_per_pin */
> -	struct hda_pcm *pcm_rec[16];
> +	struct hdmi_pcm pcm_rec[16];
>  	struct mutex pcm_lock;
>  	/* pcm_bitmap means which pcms have been assigned to pins*/
>  	unsigned long pcm_bitmap;
> @@ -403,7 +408,7 @@ static int hinfo_to_pcm_index(struct hda_codec *codec,
>  	int pcm_idx;
>  
>  	for (pcm_idx = 0; pcm_idx < spec->pcm_used; pcm_idx++)
> -		if (get_pcm_rec(spec, pcm_idx)->stream == hinfo)
> +		if (get_pcm_rec(spec, pcm_idx).pcm->stream == hinfo)

I would change rather get_pcm_rec() itself to point to hda_pcm.
For referencing to hdmi_pcm, we can introduce another macro, e.g.
get_hdmi_pcm(spec, idx).  For example:

/* obtain hdmi_pcm object assigned to idx */
#define get_hdmi_pcm(spec, idx)	(&(spec)->pcm_rec[idx])

/* obtain hda_pcm object assigned to idx */
#define get_pcm_rec(spec, idx)	get_hdmi_pcm(spec, idx)->pcm


Takashi

Re: [PATCH 3/4] ALSA: hda - hdmi jack created based on pcm

[Takashi Iwai]

On Thu, 31 Dec 2015 02:22:21 +0100,
libin.yang@linux.intel.com wrote:
> 
> From: Libin Yang <libin.yang@linux.intel.com>
> 
> Jack is created based on pcm.
> 
> For dyn_pcm_assign:
>  Driver will not use hda_jack. It will operate snd_jack directly.

This is nothing new, it's already so for Intel codecs (via audio
component).

>  snd_jack pointer will be stored in spec->pcm.jack.

.... instead of the current spec->acomp_jack.


> When pcm is
>  assigned to pin, jack will be assigned to pin automatically.
> 
> For !dyn_pcm_assign:
>  Driver will continue using hda_jack for less impact on the old cases.
>  Pcm is statically assigned to pin. So is jack. spec->pcm.jack will
>  save the snd_jack pointer created in hda_jack.
> 
> Signed-off-by: Libin Yang <libin.yang@linux.intel.com>
(snip)

> -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).pcm->device;
> +	struct hdmi_spec_per_pin *per_pin;
> +	struct hda_jack_tbl *jack;
> +	int pcmdev = get_pcm_rec(spec, pcm_idx).pcm->device;
>  	bool phantom_jack;
> +	int ret;
>  
>  	if (pcmdev > 0)
>  		sprintf(hdmi_str + strlen(hdmi_str), ",pcm=%d", pcmdev);
> -	if (codec_has_acomp(codec))
> -		return add_acomp_jack_kctl(codec, per_pin, hdmi_str);
> -	phantom_jack = !is_jack_detectable(codec, per_pin->pin_nid);
> -	if (phantom_jack)
> -		strncat(hdmi_str, " Phantom",
> -			sizeof(hdmi_str) - strlen(hdmi_str) - 1);
>  
> -	return snd_hda_jack_add_kctl(codec, per_pin->pin_nid, hdmi_str,
> -				     phantom_jack);
> +	/* for !dyn_pcm_assign, we still use hda_jack for compatibility */
> +	if (!spec->dyn_pcm_assign) {

Actually, the code changes become smaller with the following:

	if (spec->dyn_pcm_assign)
		return add_hdmi_jack_kctl(codec, spec, pcm_idx, hdmi_str);

Then the rest is almost same as the original code, and you'd need to
give a few more comments and assignment of pcm_rec[].jack in
addition.


> @@ -2626,18 +2659,23 @@ static void hdmi_array_free(struct hdmi_spec *spec)
>  static void generic_hdmi_free(struct hda_codec *codec)
>  {
>  	struct hdmi_spec *spec = codec->spec;
> -	int pin_idx;
> +	int pin_idx, pcm_idx;
>  
>  	if (codec_has_acomp(codec))
>  		snd_hdac_i915_register_notifier(NULL);
>  
>  	for (pin_idx = 0; pin_idx < spec->num_pins; pin_idx++) {
>  		struct hdmi_spec_per_pin *per_pin = get_pin(spec, pin_idx);
> -
>  		cancel_delayed_work_sync(&per_pin->work);
>  		eld_proc_free(per_pin);
> -		if (per_pin->acomp_jack)
> -			snd_device_free(codec->card, per_pin->acomp_jack);
> +	}
> +
> +	for (pcm_idx = 0; pcm_idx < spec->pcm_used; pcm_idx++) {
> +		if (spec->dyn_pcm_assign)
> +			snd_device_free(codec->card,
> +					spec->pcm_rec[pcm_idx].jack);
> +		else
> +			spec->pcm_rec[pcm_idx].jack = NULL;

Check whether spec->pcm_rec[pcm_idx].jack is NULL beforehand.


Takashi

Re: [PATCH 4/4] ALSA: hda - hdmi monitor hotplug support for dynamic pcm assignment

[Takashi Iwai]

On Thu, 31 Dec 2015 02:22:22 +0100,
libin.yang@linux.intel.com wrote:
> 
> From: Libin Yang <libin.yang@linux.intel.com>
> 
> This patch adds the support for monitor hotplug of dynamic pcm assignment.
> 
> 1. unsol_event enabling
>  - For codec_has_acomp, unsol_event is disabled.
>  - For !codec_has_acomp && !dyn_pcm_assign, use the hda_jack helper to
>    enable unsol_event
>  - For !codec_has_acomp && dyn_pcm_assign, enable unsol_event with verb
>    directly
> 
> 2. unsol_event handling
>  - For !dyn_pcm_assign, use hda_jack helper to report the event
>  - For dyn_pcm_assign, use snd_jack_report() directly

I guess we can reduce lots of codes if ignoring the case with
dyn_pcm_assign but without audio component?  If so, for simplicity, we
can limit dyn_pcm_assign only tied with audio component.  Just add a
WARN_ON() in such a case.  Although the functionality is somehow
unrelated, the actual testing and coding is more targeted with Intel
chips that are only with audio component.


Takashi

Re: [PATCH 0/4] ALSA: hda - hdmi jack support for dynamic pcm assignment

[Yang, Libin]

> -----Original Message-----
> From: Takashi Iwai [mailto:tiwai@suse.de]
> Sent: Thursday, January 07, 2016 6:32 PM
> To: Yang, Libin
> Cc: libin.yang@linux.intel.com; Lin, Mengdong; alsa-devel@alsa-
> project.org
> Subject: Re: [alsa-devel] [PATCH 0/4] ALSA: hda - hdmi jack support for
> dynamic pcm assignment
> 
> On Thu, 07 Jan 2016 02:41:13 +0100,
> Yang, Libin wrote:
> >
> > Hi Takashi,
> >
> > > -----Original Message-----
> > > From: Takashi Iwai [mailto:tiwai@suse.de]
> > > Sent: Wednesday, January 06, 2016 5:18 PM
> > > To: libin.yang@linux.intel.com
> > > Cc: alsa-devel@alsa-project.org; Lin, Mengdong; Yang, Libin
> > > Subject: Re: [alsa-devel] [PATCH 0/4] ALSA: hda - hdmi jack support
> for
> > > dynamic pcm assignment
> > >
> > > On Thu, 31 Dec 2015 02:22:18 +0100,
> > > libin.yang@linux.intel.com wrote:
> > > >
> > > > From: Libin Yang <libin.yang@linux.intel.com>
> > > >
> > > > The patchset adds hdmi jack support for dynamic pcm assignment
> > >
> > > To which branch is this supposed to be applied?
> >
> > This is preparation for DP MST audio. And it is for hdmi-jack branch.
> 
> Do you mean my test/hdmi-jack branch?  So these are additional fixes
> on top of your former MST patches?  If yes, it should go to
> topic/hda-mst branch instead.
> 
> test/hdmi-jack branch contains two unofficial patches in addition to
> your i915 MST patches that haven't been merged yet.

Oh, I see. So it should go to topic/hda-mst branch. Thanks.

Regards,
Libin

> 
> 
> Takashi
> 
> >
> > Regards,
> > Libin
> >
> > >
> > >
> > > thanks,
> > >
> > > Takashi
> > >
> > > >
> > > > Libin Yang (4):
> > > >   ALSA: Add documentation about HD-audio DP MST
> > > >   ALSA: hda - add hdmi_pcm to manage hdmi pcm related features
> > > >   ALSA: hda - hdmi jack created based on pcm
> > > >   ALSA: hda - hdmi monitor hotplug support for dynamic pcm
> > > assignment
> > > >
> > > >  Documentation/sound/alsa/HD-Audio-DP-MST-audio.txt |  75
> > > +++++++
> > > >  sound/pci/hda/patch_hdmi.c                         | 229
> +++++++++++++++----
> > > --
> > > >  2 files changed, 243 insertions(+), 61 deletions(-)
> > > >  create mode 100644 Documentation/sound/alsa/HD-Audio-DP-
> MST-
> > > audio.txt
> > > >
> > > > --
> > > > 1.9.1
> > > >
> > _______________________________________________
> > Alsa-devel mailing list
> > Alsa-devel@alsa-project.org
> > http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
> >

Re: [PATCH 2/4] ALSA: hda - add hdmi_pcm to manage hdmi pcm related features

[Yang, Libin]

> -----Original Message-----
> From: Takashi Iwai [mailto:tiwai@suse.de]
> Sent: Thursday, January 07, 2016 9:59 PM
> To: libin.yang@linux.intel.com
> Cc: alsa-devel@alsa-project.org; Lin, Mengdong; Yang, Libin
> Subject: Re: [alsa-devel] [PATCH 2/4] ALSA: hda - add hdmi_pcm to
> manage hdmi pcm related features
> 
> On Thu, 31 Dec 2015 02:22:20 +0100,
> libin.yang@linux.intel.com wrote:
> >
> > From: Libin Yang <libin.yang@linux.intel.com>
> >
> > Use struct hdmi_pcm wrapper for hdmi pcm management.
> > All PCM related features, like jack,  will be put in this structure.
> >
> > Signed-off-by: Libin Yang <libin.yang@linux.intel.com>
> > ---
> >  sound/pci/hda/patch_hdmi.c | 44 ++++++++++++++++++++++++------
> --------------
> >  1 file changed, 24 insertions(+), 20 deletions(-)
> >
> > diff --git a/sound/pci/hda/patch_hdmi.c
> b/sound/pci/hda/patch_hdmi.c
> > index 6d6dba4..5549ddf 100644
> > --- a/sound/pci/hda/patch_hdmi.c
> > +++ b/sound/pci/hda/patch_hdmi.c
> > @@ -86,7 +86,7 @@ struct hdmi_spec_per_pin {
> >  	struct delayed_work work;
> >  	struct snd_kcontrol *eld_ctl;
> >  	struct snd_jack *acomp_jack; /* jack via audio component */
> > -	struct hda_pcm *pcm; /* pointer to spec->pcm_rec[n]
> dynamically*/
> > +	struct hdmi_pcm *pcm; /* pointer to spec->pcm_rec[n]
> dynamically*/
> >  	int pcm_idx; /* which pcm is attached. -1 means no pcm is
> attached */
> >  	int repoll_count;
> >  	bool setup; /* the stream has been set up by prepare callback */
> > @@ -132,6 +132,11 @@ struct hdmi_ops {
> >  	int (*chmap_validate)(int ca, int channels, unsigned char
> *chmap);
> >  };
> >
> > +struct hdmi_pcm {
> > +	struct hda_pcm *pcm;
> > +	struct snd_jack *jack;
> > +};
> > +
> >  struct hdmi_spec {
> >  	int num_cvts;
> >  	struct snd_array cvts; /* struct hdmi_spec_per_cvt */
> > @@ -139,7 +144,7 @@ struct hdmi_spec {
> >
> >  	int num_pins;
> >  	struct snd_array pins; /* struct hdmi_spec_per_pin */
> > -	struct hda_pcm *pcm_rec[16];
> > +	struct hdmi_pcm pcm_rec[16];
> >  	struct mutex pcm_lock;
> >  	/* pcm_bitmap means which pcms have been assigned to pins*/
> >  	unsigned long pcm_bitmap;
> > @@ -403,7 +408,7 @@ static int hinfo_to_pcm_index(struct
> hda_codec *codec,
> >  	int pcm_idx;
> >
> >  	for (pcm_idx = 0; pcm_idx < spec->pcm_used; pcm_idx++)
> > -		if (get_pcm_rec(spec, pcm_idx)->stream == hinfo)
> > +		if (get_pcm_rec(spec, pcm_idx).pcm->stream == hinfo)
> 
> I would change rather get_pcm_rec() itself to point to hda_pcm.
> For referencing to hdmi_pcm, we can introduce another macro, e.g.
> get_hdmi_pcm(spec, idx).  For example:

Get it. It makes sense for compatibility. I will change it.

> 
> /* obtain hdmi_pcm object assigned to idx */
> #define get_hdmi_pcm(spec, idx)	(&(spec)->pcm_rec[idx])
> 
> /* obtain hda_pcm object assigned to idx */
> #define get_pcm_rec(spec, idx)	get_hdmi_pcm(spec, idx)->pcm
> 
> 
> Takashi

Re: [PATCH 3/4] ALSA: hda - hdmi jack created based on pcm

[Yang, Libin]

> -----Original Message-----
> From: Takashi Iwai [mailto:tiwai@suse.de]
> Sent: Thursday, January 07, 2016 10:14 PM
> To: libin.yang@linux.intel.com
> Cc: alsa-devel@alsa-project.org; Lin, Mengdong; Yang, Libin
> Subject: Re: [alsa-devel] [PATCH 3/4] ALSA: hda - hdmi jack created based
> on pcm
> 
> On Thu, 31 Dec 2015 02:22:21 +0100,
> libin.yang@linux.intel.com wrote:
> >
> > From: Libin Yang <libin.yang@linux.intel.com>
> >
> > Jack is created based on pcm.
> >
> > For dyn_pcm_assign:
> >  Driver will not use hda_jack. It will operate snd_jack directly.
> 
> This is nothing new, it's already so for Intel codecs (via audio
> component).

Yes. And I'm try to use this new method to all codecs as an option. :)

> 
> >  snd_jack pointer will be stored in spec->pcm.jack.
> 
> .... instead of the current spec->acomp_jack.
> 
> 
> > When pcm is
> >  assigned to pin, jack will be assigned to pin automatically.
> >
> > For !dyn_pcm_assign:
> >  Driver will continue using hda_jack for less impact on the old cases.
> >  Pcm is statically assigned to pin. So is jack. spec->pcm.jack will
> >  save the snd_jack pointer created in hda_jack.
> >
> > Signed-off-by: Libin Yang <libin.yang@linux.intel.com>
> (snip)
> 
> > -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).pcm->device;
> > +	struct hdmi_spec_per_pin *per_pin;
> > +	struct hda_jack_tbl *jack;
> > +	int pcmdev = get_pcm_rec(spec, pcm_idx).pcm->device;
> >  	bool phantom_jack;
> > +	int ret;
> >
> >  	if (pcmdev > 0)
> >  		sprintf(hdmi_str + strlen(hdmi_str), ",pcm=%d",
> pcmdev);
> > -	if (codec_has_acomp(codec))
> > -		return add_acomp_jack_kctl(codec, per_pin, hdmi_str);
> > -	phantom_jack = !is_jack_detectable(codec, per_pin->pin_nid);
> > -	if (phantom_jack)
> > -		strncat(hdmi_str, " Phantom",
> > -			sizeof(hdmi_str) - strlen(hdmi_str) - 1);
> >
> > -	return snd_hda_jack_add_kctl(codec, per_pin->pin_nid,
> hdmi_str,
> > -				     phantom_jack);
> > +	/* for !dyn_pcm_assign, we still use hda_jack for compatibility */
> > +	if (!spec->dyn_pcm_assign) {
> 
> Actually, the code changes become smaller with the following:
> 
> 	if (spec->dyn_pcm_assign)
> 		return add_hdmi_jack_kctl(codec, spec, pcm_idx,
> hdmi_str);

OK, I will change it. Anyway, we need enable unsol_event here
if !acomp. 

> 
> Then the rest is almost same as the original code, and you'd need to
> give a few more comments and assignment of pcm_rec[].jack in
> addition.
> 
> 
> > @@ -2626,18 +2659,23 @@ static void hdmi_array_free(struct
> hdmi_spec *spec)
> >  static void generic_hdmi_free(struct hda_codec *codec)
> >  {
> >  	struct hdmi_spec *spec = codec->spec;
> > -	int pin_idx;
> > +	int pin_idx, pcm_idx;
> >
> >  	if (codec_has_acomp(codec))
> >  		snd_hdac_i915_register_notifier(NULL);
> >
> >  	for (pin_idx = 0; pin_idx < spec->num_pins; pin_idx++) {
> >  		struct hdmi_spec_per_pin *per_pin = get_pin(spec,
> pin_idx);
> > -
> >  		cancel_delayed_work_sync(&per_pin->work);
> >  		eld_proc_free(per_pin);
> > -		if (per_pin->acomp_jack)
> > -			snd_device_free(codec->card, per_pin-
> >acomp_jack);
> > +	}
> > +
> > +	for (pcm_idx = 0; pcm_idx < spec->pcm_used; pcm_idx++) {
> > +		if (spec->dyn_pcm_assign)
> > +			snd_device_free(codec->card,
> > +					spec->pcm_rec[pcm_idx].jack);
> > +		else
> > +			spec->pcm_rec[pcm_idx].jack = NULL;
> 
> Check whether spec->pcm_rec[pcm_idx].jack is NULL beforehand.
> 

Do you mean:
if (spec->pcm_rec[pcm_idx].jack)
    spec->pcm_rec[pcm_idx].jack = NULL;

Regards,
Libin
> 
> Takashi

Re: [PATCH 4/4] ALSA: hda - hdmi monitor hotplug support for dynamic pcm assignment

[Yang, Libin]

Hi Takashi,

> -----Original Message-----
> From: Takashi Iwai [mailto:tiwai@suse.de]
> Sent: Thursday, January 07, 2016 10:18 PM
> To: libin.yang@linux.intel.com
> Cc: alsa-devel@alsa-project.org; Lin, Mengdong; Yang, Libin
> Subject: Re: [alsa-devel] [PATCH 4/4] ALSA: hda - hdmi monitor hotplug
> support for dynamic pcm assignment
> 
> On Thu, 31 Dec 2015 02:22:22 +0100,
> libin.yang@linux.intel.com wrote:
> >
> > From: Libin Yang <libin.yang@linux.intel.com>
> >
> > This patch adds the support for monitor hotplug of dynamic pcm
> assignment.
> >
> > 1. unsol_event enabling
> >  - For codec_has_acomp, unsol_event is disabled.
> >  - For !codec_has_acomp && !dyn_pcm_assign, use the hda_jack
> helper to
> >    enable unsol_event
> >  - For !codec_has_acomp && dyn_pcm_assign, enable unsol_event
> with verb
> >    directly
> >
> > 2. unsol_event handling
> >  - For !dyn_pcm_assign, use hda_jack helper to report the event
> >  - For dyn_pcm_assign, use snd_jack_report() directly
> 
> I guess we can reduce lots of codes if ignoring the case with
> dyn_pcm_assign but without audio component?  If so, for simplicity, we
> can limit dyn_pcm_assign only tied with audio component.  Just add a
> WARN_ON() in such a case.  Although the functionality is somehow
> unrelated, the actual testing and coding is more targeted with Intel
> chips that are only with audio component.

My initial idea is to support:
1. dyn_pcm_assign && acomp: Intel case
2. dyn_pcm_assign && !acomp: other vendors MST case
3. !dyn_pcm_assign && acomp: Intel non-mst case
4. !dyn_pcm_assign && !acomp: other vendors current case

Do you mean we can ignore other vendors' case so far? If so, the
code will be simpler. 

> 
> 
> Takashi

Re: [PATCH 3/4] ALSA: hda - hdmi jack created based on pcm

[Takashi Iwai]

On Fri, 08 Jan 2016 04:15:03 +0100,
Yang, Libin wrote:
> > > @@ -2626,18 +2659,23 @@ static void hdmi_array_free(struct
> > hdmi_spec *spec)
> > >  static void generic_hdmi_free(struct hda_codec *codec)
> > >  {
> > >  	struct hdmi_spec *spec = codec->spec;
> > > -	int pin_idx;
> > > +	int pin_idx, pcm_idx;
> > >
> > >  	if (codec_has_acomp(codec))
> > >  		snd_hdac_i915_register_notifier(NULL);
> > >
> > >  	for (pin_idx = 0; pin_idx < spec->num_pins; pin_idx++) {
> > >  		struct hdmi_spec_per_pin *per_pin = get_pin(spec,
> > pin_idx);
> > > -
> > >  		cancel_delayed_work_sync(&per_pin->work);
> > >  		eld_proc_free(per_pin);
> > > -		if (per_pin->acomp_jack)
> > > -			snd_device_free(codec->card, per_pin-
> > >acomp_jack);
> > > +	}
> > > +
> > > +	for (pcm_idx = 0; pcm_idx < spec->pcm_used; pcm_idx++) {
> > > +		if (spec->dyn_pcm_assign)
> > > +			snd_device_free(codec->card,
> > > +					spec->pcm_rec[pcm_idx].jack);
> > > +		else
> > > +			spec->pcm_rec[pcm_idx].jack = NULL;
> > 
> > Check whether spec->pcm_rec[pcm_idx].jack is NULL beforehand.
> > 
> 
> Do you mean:
> if (spec->pcm_rec[pcm_idx].jack)
>     spec->pcm_rec[pcm_idx].jack = NULL;

Not only that.  Calling snd_device_free() with NULL isn't allowed.


Takashi

Re: [PATCH 4/4] ALSA: hda - hdmi monitor hotplug support for dynamic pcm assignment

[Takashi Iwai]

On Fri, 08 Jan 2016 06:25:37 +0100,
Yang, Libin wrote:
> 
> Hi Takashi,
> 
> > -----Original Message-----
> > From: Takashi Iwai [mailto:tiwai@suse.de]
> > Sent: Thursday, January 07, 2016 10:18 PM
> > To: libin.yang@linux.intel.com
> > Cc: alsa-devel@alsa-project.org; Lin, Mengdong; Yang, Libin
> > Subject: Re: [alsa-devel] [PATCH 4/4] ALSA: hda - hdmi monitor hotplug
> > support for dynamic pcm assignment
> > 
> > On Thu, 31 Dec 2015 02:22:22 +0100,
> > libin.yang@linux.intel.com wrote:
> > >
> > > From: Libin Yang <libin.yang@linux.intel.com>
> > >
> > > This patch adds the support for monitor hotplug of dynamic pcm
> > assignment.
> > >
> > > 1. unsol_event enabling
> > >  - For codec_has_acomp, unsol_event is disabled.
> > >  - For !codec_has_acomp && !dyn_pcm_assign, use the hda_jack
> > helper to
> > >    enable unsol_event
> > >  - For !codec_has_acomp && dyn_pcm_assign, enable unsol_event
> > with verb
> > >    directly
> > >
> > > 2. unsol_event handling
> > >  - For !dyn_pcm_assign, use hda_jack helper to report the event
> > >  - For dyn_pcm_assign, use snd_jack_report() directly
> > 
> > I guess we can reduce lots of codes if ignoring the case with
> > dyn_pcm_assign but without audio component?  If so, for simplicity, we
> > can limit dyn_pcm_assign only tied with audio component.  Just add a
> > WARN_ON() in such a case.  Although the functionality is somehow
> > unrelated, the actual testing and coding is more targeted with Intel
> > chips that are only with audio component.
> 
> My initial idea is to support:
> 1. dyn_pcm_assign && acomp: Intel case
> 2. dyn_pcm_assign && !acomp: other vendors MST case
> 3. !dyn_pcm_assign && acomp: Intel non-mst case
> 4. !dyn_pcm_assign && !acomp: other vendors current case
> 
> Do you mean we can ignore other vendors' case so far? If so, the
> code will be simpler. 

Yes.  And give WARN_ON(dyn_pcm_assign && !acomp);


Takashi

Re: [PATCH 3/4] ALSA: hda - hdmi jack created based on pcm

[Yang, Libin]

> -----Original Message-----
> From: Takashi Iwai [mailto:tiwai@suse.de]
> Sent: Friday, January 08, 2016 3:44 PM
> To: Yang, Libin
> Cc: libin.yang@linux.intel.com; alsa-devel@alsa-project.org; Lin,
> Mengdong
> Subject: Re: [alsa-devel] [PATCH 3/4] ALSA: hda - hdmi jack created based
> on pcm
> 
> On Fri, 08 Jan 2016 04:15:03 +0100,
> Yang, Libin wrote:
> > > > @@ -2626,18 +2659,23 @@ static void hdmi_array_free(struct
> > > hdmi_spec *spec)
> > > >  static void generic_hdmi_free(struct hda_codec *codec)
> > > >  {
> > > >  	struct hdmi_spec *spec = codec->spec;
> > > > -	int pin_idx;
> > > > +	int pin_idx, pcm_idx;
> > > >
> > > >  	if (codec_has_acomp(codec))
> > > >  		snd_hdac_i915_register_notifier(NULL);
> > > >
> > > >  	for (pin_idx = 0; pin_idx < spec->num_pins; pin_idx++) {
> > > >  		struct hdmi_spec_per_pin *per_pin = get_pin(spec,
> > > pin_idx);
> > > > -
> > > >  		cancel_delayed_work_sync(&per_pin->work);
> > > >  		eld_proc_free(per_pin);
> > > > -		if (per_pin->acomp_jack)
> > > > -			snd_device_free(codec->card, per_pin-
> > > >acomp_jack);
> > > > +	}
> > > > +
> > > > +	for (pcm_idx = 0; pcm_idx < spec->pcm_used; pcm_idx++) {
> > > > +		if (spec->dyn_pcm_assign)
> > > > +			snd_device_free(codec->card,
> > > > +					spec->pcm_rec[pcm_idx].jack);
> > > > +		else
> > > > +			spec->pcm_rec[pcm_idx].jack = NULL;
> > >
> > > Check whether spec->pcm_rec[pcm_idx].jack is NULL beforehand.
> > >
> >
> > Do you mean:
> > if (spec->pcm_rec[pcm_idx].jack)
> >     spec->pcm_rec[pcm_idx].jack = NULL;
> 
> Not only that.  Calling snd_device_free() with NULL isn't allowed.

I see. It will be:
+for (pcm_idx = 0; pcm_idx < spec->pcm_used; pcm_idx++) {
+		if (spec->pcm_rec[pcm_idx].jack == NULL)
+			continue;
+		if (spec->dyn_pcm_assign)

BTW: It seems snd_device_free() will check the pointer.

Regards,
Libin

> Takashi

Re: [PATCH 4/4] ALSA: hda - hdmi monitor hotplug support for dynamic pcm assignment

[Yang, Libin]

> -----Original Message-----
> From: Takashi Iwai [mailto:tiwai@suse.de]
> Sent: Friday, January 08, 2016 3:45 PM
> To: Yang, Libin
> Cc: libin.yang@linux.intel.com; alsa-devel@alsa-project.org; Lin,
> Mengdong
> Subject: Re: [alsa-devel] [PATCH 4/4] ALSA: hda - hdmi monitor hotplug
> support for dynamic pcm assignment
> 
> On Fri, 08 Jan 2016 06:25:37 +0100,
> Yang, Libin wrote:
> >
> > Hi Takashi,
> >
> > > -----Original Message-----
> > > From: Takashi Iwai [mailto:tiwai@suse.de]
> > > Sent: Thursday, January 07, 2016 10:18 PM
> > > To: libin.yang@linux.intel.com
> > > Cc: alsa-devel@alsa-project.org; Lin, Mengdong; Yang, Libin
> > > Subject: Re: [alsa-devel] [PATCH 4/4] ALSA: hda - hdmi monitor
> hotplug
> > > support for dynamic pcm assignment
> > >
> > > On Thu, 31 Dec 2015 02:22:22 +0100,
> > > libin.yang@linux.intel.com wrote:
> > > >
> > > > From: Libin Yang <libin.yang@linux.intel.com>
> > > >
> > > > This patch adds the support for monitor hotplug of dynamic pcm
> > > assignment.
> > > >
> > > > 1. unsol_event enabling
> > > >  - For codec_has_acomp, unsol_event is disabled.
> > > >  - For !codec_has_acomp && !dyn_pcm_assign, use the hda_jack
> > > helper to
> > > >    enable unsol_event
> > > >  - For !codec_has_acomp && dyn_pcm_assign, enable unsol_event
> > > with verb
> > > >    directly
> > > >
> > > > 2. unsol_event handling
> > > >  - For !dyn_pcm_assign, use hda_jack helper to report the event
> > > >  - For dyn_pcm_assign, use snd_jack_report() directly
> > >
> > > I guess we can reduce lots of codes if ignoring the case with
> > > dyn_pcm_assign but without audio component?  If so, for simplicity,
> we
> > > can limit dyn_pcm_assign only tied with audio component.  Just add a
> > > WARN_ON() in such a case.  Although the functionality is somehow
> > > unrelated, the actual testing and coding is more targeted with Intel
> > > chips that are only with audio component.
> >
> > My initial idea is to support:
> > 1. dyn_pcm_assign && acomp: Intel case
> > 2. dyn_pcm_assign && !acomp: other vendors MST case
> > 3. !dyn_pcm_assign && acomp: Intel non-mst case
> > 4. !dyn_pcm_assign && !acomp: other vendors current case
> >
> > Do you mean we can ignore other vendors' case so far? If so, the
> > code will be simpler.
> 
> Yes.  And give WARN_ON(dyn_pcm_assign && !acomp);

Get it. I will follow it.

Regards,
Libin

> 
> 
> Takashi

Re: [PATCH 3/4] ALSA: hda - hdmi jack created based on pcm

[Takashi Iwai]

On Fri, 08 Jan 2016 08:52:17 +0100,
Yang, Libin wrote:
> 
> 
> > -----Original Message-----
> > From: Takashi Iwai [mailto:tiwai@suse.de]
> > Sent: Friday, January 08, 2016 3:44 PM
> > To: Yang, Libin
> > Cc: libin.yang@linux.intel.com; alsa-devel@alsa-project.org; Lin,
> > Mengdong
> > Subject: Re: [alsa-devel] [PATCH 3/4] ALSA: hda - hdmi jack created based
> > on pcm
> > 
> > On Fri, 08 Jan 2016 04:15:03 +0100,
> > Yang, Libin wrote:
> > > > > @@ -2626,18 +2659,23 @@ static void hdmi_array_free(struct
> > > > hdmi_spec *spec)
> > > > >  static void generic_hdmi_free(struct hda_codec *codec)
> > > > >  {
> > > > >  	struct hdmi_spec *spec = codec->spec;
> > > > > -	int pin_idx;
> > > > > +	int pin_idx, pcm_idx;
> > > > >
> > > > >  	if (codec_has_acomp(codec))
> > > > >  		snd_hdac_i915_register_notifier(NULL);
> > > > >
> > > > >  	for (pin_idx = 0; pin_idx < spec->num_pins; pin_idx++) {
> > > > >  		struct hdmi_spec_per_pin *per_pin = get_pin(spec,
> > > > pin_idx);
> > > > > -
> > > > >  		cancel_delayed_work_sync(&per_pin->work);
> > > > >  		eld_proc_free(per_pin);
> > > > > -		if (per_pin->acomp_jack)
> > > > > -			snd_device_free(codec->card, per_pin-
> > > > >acomp_jack);
> > > > > +	}
> > > > > +
> > > > > +	for (pcm_idx = 0; pcm_idx < spec->pcm_used; pcm_idx++) {
> > > > > +		if (spec->dyn_pcm_assign)
> > > > > +			snd_device_free(codec->card,
> > > > > +					spec->pcm_rec[pcm_idx].jack);
> > > > > +		else
> > > > > +			spec->pcm_rec[pcm_idx].jack = NULL;
> > > >
> > > > Check whether spec->pcm_rec[pcm_idx].jack is NULL beforehand.
> > > >
> > >
> > > Do you mean:
> > > if (spec->pcm_rec[pcm_idx].jack)
> > >     spec->pcm_rec[pcm_idx].jack = NULL;
> > 
> > Not only that.  Calling snd_device_free() with NULL isn't allowed.
> 
> I see. It will be:
> +for (pcm_idx = 0; pcm_idx < spec->pcm_used; pcm_idx++) {
> +		if (spec->pcm_rec[pcm_idx].jack == NULL)
> +			continue;
> +		if (spec->dyn_pcm_assign)
> 
> BTW: It seems snd_device_free() will check the pointer.

Yes but with snd_BUG_ON(), i.e. with a warning.  It shouldn't be
called with NULL.


Takashi

Re: [PATCH 3/4] ALSA: hda - hdmi jack created based on pcm

[Yang, Libin]

> -----Original Message-----
> From: Takashi Iwai [mailto:tiwai@suse.de]
> Sent: Friday, January 08, 2016 3:55 PM
> To: Yang, Libin
> Cc: libin.yang@linux.intel.com; alsa-devel@alsa-project.org; Lin,
> Mengdong
> Subject: Re: [alsa-devel] [PATCH 3/4] ALSA: hda - hdmi jack created based
> on pcm
> 
> On Fri, 08 Jan 2016 08:52:17 +0100,
> Yang, Libin wrote:
> >
> >
> > > -----Original Message-----
> > > From: Takashi Iwai [mailto:tiwai@suse.de]
> > > Sent: Friday, January 08, 2016 3:44 PM
> > > To: Yang, Libin
> > > Cc: libin.yang@linux.intel.com; alsa-devel@alsa-project.org; Lin,
> > > Mengdong
> > > Subject: Re: [alsa-devel] [PATCH 3/4] ALSA: hda - hdmi jack created
> based
> > > on pcm
> > >
> > > On Fri, 08 Jan 2016 04:15:03 +0100,
> > > Yang, Libin wrote:
> > > > > > @@ -2626,18 +2659,23 @@ static void hdmi_array_free(struct
> > > > > hdmi_spec *spec)
> > > > > >  static void generic_hdmi_free(struct hda_codec *codec)
> > > > > >  {
> > > > > >  	struct hdmi_spec *spec = codec->spec;
> > > > > > -	int pin_idx;
> > > > > > +	int pin_idx, pcm_idx;
> > > > > >
> > > > > >  	if (codec_has_acomp(codec))
> > > > > >  		snd_hdac_i915_register_notifier(NULL);
> > > > > >
> > > > > >  	for (pin_idx = 0; pin_idx < spec->num_pins; pin_idx++) {
> > > > > >  		struct hdmi_spec_per_pin *per_pin =
> get_pin(spec,
> > > > > pin_idx);
> > > > > > -
> > > > > >  		cancel_delayed_work_sync(&per_pin->work);
> > > > > >  		eld_proc_free(per_pin);
> > > > > > -		if (per_pin->acomp_jack)
> > > > > > -			snd_device_free(codec->card, per_pin-
> > > > > >acomp_jack);
> > > > > > +	}
> > > > > > +
> > > > > > +	for (pcm_idx = 0; pcm_idx < spec->pcm_used;
> pcm_idx++) {
> > > > > > +		if (spec->dyn_pcm_assign)
> > > > > > +			snd_device_free(codec->card,
> > > > > > +					spec-
> >pcm_rec[pcm_idx].jack);
> > > > > > +		else
> > > > > > +			spec->pcm_rec[pcm_idx].jack = NULL;
> > > > >
> > > > > Check whether spec->pcm_rec[pcm_idx].jack is NULL beforehand.
> > > > >
> > > >
> > > > Do you mean:
> > > > if (spec->pcm_rec[pcm_idx].jack)
> > > >     spec->pcm_rec[pcm_idx].jack = NULL;
> > >
> > > Not only that.  Calling snd_device_free() with NULL isn't allowed.
> >
> > I see. It will be:
> > +for (pcm_idx = 0; pcm_idx < spec->pcm_used; pcm_idx++) {
> > +		if (spec->pcm_rec[pcm_idx].jack == NULL)
> > +			continue;
> > +		if (spec->dyn_pcm_assign)
> >
> > BTW: It seems snd_device_free() will check the pointer.
> 
> Yes but with snd_BUG_ON(), i.e. with a warning.  It shouldn't be
> called with NULL.

Get it :-)

Regards,
Libin

> 
> 
> Takashi