All of lore.kernel.org
 help / color / mirror / Atom feed
* [RFC V2 PATCH 0/5] hdmi audio dynamically bind PCM to pin
@ 2015-11-23 14:09 libin.yang
  2015-11-23 14:09 ` libin.yang
                   ` (5 more replies)
  0 siblings, 6 replies; 13+ messages in thread
From: libin.yang @ 2015-11-23 14:09 UTC (permalink / raw)
  To: alsa-devel, tiwai; +Cc: libin.yang, Libin Yang, mengdong.lin

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

The patches are trying to support dynamically binding PCM to pin
in HDMI/DP audio.

The pin will not bind to any PCM if there is not monitor connected.
When there is a monitor connected to the port (pin), driver will
try to find a proper PCM to assign to the port and setup the pin
if necessary.

Libin Yang (5):
  ALSA: hda - hdmi playback without monitor in dynamic pcm bind mode
  ALSA: hda - hdmi operate spdif based on pcm
  ALSA: hda - hdmi dynamically bind PCM to pin when monitor hotplug
  ALSA: hda - add in_use flag in hda_pcm
  ALSA: hda - hdmi setup pin when monitor hotplug in pcm dynamic
    assignment mode

 sound/pci/hda/hda_codec.h  |   1 +
 sound/pci/hda/patch_hdmi.c | 361 ++++++++++++++++++++++++++++++++++++++++++---
 2 files changed, 343 insertions(+), 19 deletions(-)

-- 
1.9.1

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [RFC V2 PATCH 0/5] hdmi audio dynamically bind PCM to pin
  2015-11-23 14:09 [RFC V2 PATCH 0/5] hdmi audio dynamically bind PCM to pin libin.yang
@ 2015-11-23 14:09 ` libin.yang
  2015-11-23 14:09 ` [RFC V2 PATCH 1/5] ALSA: hda - hdmi playback without monitor in dynamic pcm bind mode libin.yang
                   ` (4 subsequent siblings)
  5 siblings, 0 replies; 13+ messages in thread
From: libin.yang @ 2015-11-23 14:09 UTC (permalink / raw)
  To: alsa-devel, tiwai; +Cc: libin.yang, Libin Yang, mengdong.lin

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

The patches are trying to support dynamically binding PCM to pin
in HDMI/DP audio.

The pin will not bind to any PCM if there is not monitor connected.
When there is a monitor connected to the port (pin), driver will
try to find a proper PCM to assign to the port and setup the pin
if necessary.

Libin Yang (5):
  ALSA: hda - hdmi playback without monitor in dynamic pcm bind mode
  ALSA: hda - hdmi operate spdif based on pcm
  ALSA: hda - hdmi dynamically bind PCM to pin when monitor hotplug
  ALSA: hda - add in_use flag in hda_pcm
  ALSA: hda - hdmi setup pin when monitor hotplug in pcm dynamic
    assignment mode

 sound/pci/hda/hda_codec.h  |   1 +
 sound/pci/hda/patch_hdmi.c | 361 ++++++++++++++++++++++++++++++++++++++++++---
 2 files changed, 343 insertions(+), 19 deletions(-)

-- 
1.9.1

^ permalink raw reply	[flat|nested] 13+ messages in thread

* [RFC V2 PATCH 1/5] ALSA: hda - hdmi playback without monitor in dynamic pcm bind mode
  2015-11-23 14:09 [RFC V2 PATCH 0/5] hdmi audio dynamically bind PCM to pin libin.yang
  2015-11-23 14:09 ` libin.yang
@ 2015-11-23 14:09 ` libin.yang
  2015-11-23 16:01   ` Takashi Iwai
  2015-11-23 14:09 ` [RFC V2 PATCH 2/5] ALSA: hda - hdmi operate spdif based on pcm libin.yang
                   ` (3 subsequent siblings)
  5 siblings, 1 reply; 13+ messages in thread
From: libin.yang @ 2015-11-23 14:09 UTC (permalink / raw)
  To: alsa-devel, tiwai; +Cc: libin.yang, Libin Yang, mengdong.lin

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

Pulseaudio requires open pcm successfully when probing.

This patch handles playback without monitor in dynamic pcm assignment
mode. It tries to open/prepare/close pcm successfully even there is
no pin bound to the PCM. On the meantime, it will try to find a proper
converter for the PCM.

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

diff --git a/sound/pci/hda/patch_hdmi.c b/sound/pci/hda/patch_hdmi.c
index 92e05fb..d0294bf 100644
--- a/sound/pci/hda/patch_hdmi.c
+++ b/sound/pci/hda/patch_hdmi.c
@@ -135,6 +135,7 @@ struct hdmi_spec {
 	int num_pins;
 	struct snd_array pins; /* struct hdmi_spec_per_pin */
 	struct hda_pcm *pcm_rec[16];
+	struct mutex pcm_lock;
 	unsigned int channels_max; /* max over all cvts */
 
 	struct hdmi_eld temp_eld;
@@ -1331,6 +1332,11 @@ static int hdmi_setup_stream(struct hda_codec *codec, hda_nid_t cvt_nid,
 	return 0;
 }
 
+/* Try to find an available converter
+ * If pin_idx is less then zero, just try to find an available converter.
+ * Otherwise, try to find an available converter and get the cvt mux index
+ * of the pin.
+ */
 static int hdmi_choose_cvt(struct hda_codec *codec,
 			int pin_idx, int *cvt_id, int *mux_id)
 {
@@ -1339,7 +1345,11 @@ static int hdmi_choose_cvt(struct hda_codec *codec,
 	struct hdmi_spec_per_cvt *per_cvt = NULL;
 	int cvt_idx, mux_idx = 0;
 
-	per_pin = get_pin(spec, pin_idx);
+	/* pin_idx < 0 means no pin will be bound to the converter */
+	if (pin_idx < 0)
+		per_pin = NULL;
+	else
+		per_pin = get_pin(spec, pin_idx);
 
 	/* Dynamically assign converter to stream */
 	for (cvt_idx = 0; cvt_idx < spec->num_cvts; cvt_idx++) {
@@ -1348,6 +1358,8 @@ static int hdmi_choose_cvt(struct hda_codec *codec,
 		/* Must not already be assigned */
 		if (per_cvt->assigned)
 			continue;
+		if (per_pin == NULL)
+			break;
 		/* Must be in pin's mux's list of converters */
 		for (mux_idx = 0; mux_idx < per_pin->num_mux_nids; mux_idx++)
 			if (per_pin->mux_nids[mux_idx] == per_cvt->cvt_nid)
@@ -1360,9 +1372,10 @@ static int hdmi_choose_cvt(struct hda_codec *codec,
 
 	/* No free converters */
 	if (cvt_idx == spec->num_cvts)
-		return -ENODEV;
+		return -EBUSY;
 
-	per_pin->mux_idx = mux_idx;
+	if (per_pin != NULL)
+		per_pin->mux_idx = mux_idx;
 
 	if (cvt_id)
 		*cvt_id = cvt_idx;
@@ -1388,6 +1401,20 @@ static void intel_verify_pin_cvt_connect(struct hda_codec *codec,
 					    mux_idx);
 }
 
+/* get the mux index for the converter of the pins
+ * converter's mux index is the same for all pins on Intel platform
+ */
+static int intel_cvt_id_to_mux_idx(struct hdmi_spec *spec,
+			hda_nid_t cvt_nid)
+{
+	int i;
+
+	for (i = 0; i < spec->num_cvts; i++)
+		if (spec->cvt_nids[i] == cvt_nid)
+			return i;
+	return -EINVAL;
+}
+
 /* Intel HDMI workaround to fix audio routing issue:
  * For some Intel display codecs, pins share the same connection list.
  * So a conveter can be selected by multiple pins and playback on any of these
@@ -1439,6 +1466,69 @@ static void intel_not_share_assigned_cvt(struct hda_codec *codec,
 	}
 }
 
+/* A wrapper of intel_not_share_asigned_cvt() */
+static void intel_not_share_assigned_cvt_nid(struct hda_codec *codec,
+			hda_nid_t pin_nid, hda_nid_t cvt_nid)
+{
+	int mux_idx;
+	struct hdmi_spec *spec = codec->spec;
+
+	if (!is_haswell_plus(codec) && !is_valleyview_plus(codec))
+		return;
+
+	/* On Intel platform, the mapping of converter nid to
+	 * mux index of the pins are always the same.
+	 * The pin nid may be 0, this means all pins will not
+	 * share the converter.
+	 */
+	mux_idx = intel_cvt_id_to_mux_idx(spec, cvt_nid);
+	if (mux_idx > 0)
+		intel_not_share_assigned_cvt(codec, pin_nid, mux_idx);
+}
+
+/* called in hdmi_pcm_open when no pin is assigned to the PCM
+ * in dyn_pcm_assign mode.
+ */
+static int hdmi_pcm_open_no_pin(struct hda_pcm_stream *hinfo,
+			 struct hda_codec *codec,
+			 struct snd_pcm_substream *substream)
+{
+	struct hdmi_spec *spec = codec->spec;
+	struct snd_pcm_runtime *runtime = substream->runtime;
+	int cvt_idx;
+	struct hdmi_spec_per_cvt *per_cvt = NULL;
+	int err;
+
+	err = hdmi_choose_cvt(codec, -1, &cvt_idx, NULL);
+	if (err)
+		return err;
+
+	per_cvt = get_cvt(spec, cvt_idx);
+	per_cvt->assigned = 1;
+	hinfo->nid = per_cvt->cvt_nid;
+
+	intel_not_share_assigned_cvt_nid(codec, 0, per_cvt->cvt_nid);
+
+	/* todo: setup spdif ctls assign */
+
+	/* Initially set the converter's capabilities */
+	hinfo->channels_min = per_cvt->channels_min;
+	hinfo->channels_max = per_cvt->channels_max;
+	hinfo->rates = per_cvt->rates;
+	hinfo->formats = per_cvt->formats;
+	hinfo->maxbps = per_cvt->maxbps;
+
+	/* Store the updated parameters */
+	runtime->hw.channels_min = hinfo->channels_min;
+	runtime->hw.channels_max = hinfo->channels_max;
+	runtime->hw.formats = hinfo->formats;
+	runtime->hw.rates = hinfo->rates;
+
+	snd_pcm_hw_constraint_step(substream->runtime, 0,
+				   SNDRV_PCM_HW_PARAM_CHANNELS, 2);
+	return 0;
+}
+
 /*
  * HDA PCM callbacks
  */
@@ -1455,19 +1545,36 @@ static int hdmi_pcm_open(struct hda_pcm_stream *hinfo,
 	int err;
 
 	/* Validate hinfo */
+	mutex_lock(&spec->pcm_lock);
 	pin_idx = hinfo_to_pin_index(codec, hinfo);
-	if (snd_BUG_ON(pin_idx < 0))
-		return -EINVAL;
-	per_pin = get_pin(spec, pin_idx);
-	eld = &per_pin->sink_eld;
+	if (!spec->dyn_pcm_assign) {
+		if (snd_BUG_ON(pin_idx < 0)) {
+			mutex_unlock(&spec->pcm_lock);
+			return -EINVAL;
+		}
+	} else {
+		/* no pin is assigned to the PCM
+		 * PA need pcm open successfully when probe
+		 */
+		if (pin_idx < 0) {
+			err = hdmi_pcm_open_no_pin(hinfo, codec, substream);
+			mutex_unlock(&spec->pcm_lock);
+			return err;
+		}
+	}
 
 	err = hdmi_choose_cvt(codec, pin_idx, &cvt_idx, &mux_idx);
-	if (err < 0)
+	if (err < 0) {
+		mutex_unlock(&spec->pcm_lock);
 		return err;
+	}
 
 	per_cvt = get_cvt(spec, cvt_idx);
 	/* Claim converter */
 	per_cvt->assigned = 1;
+
+
+	per_pin = get_pin(spec, pin_idx);
 	per_pin->cvt_nid = per_cvt->cvt_nid;
 	hinfo->nid = per_cvt->cvt_nid;
 
@@ -1488,6 +1595,7 @@ static int hdmi_pcm_open(struct hda_pcm_stream *hinfo,
 	hinfo->formats = per_cvt->formats;
 	hinfo->maxbps = per_cvt->maxbps;
 
+	eld = &per_pin->sink_eld;
 	/* Restrict capabilities by ELD if this isn't disabled */
 	if (!static_hdmi_pcm && eld->eld_valid) {
 		snd_hdmi_eld_update_pcm_info(&eld->info, hinfo);
@@ -1496,10 +1604,12 @@ static int hdmi_pcm_open(struct hda_pcm_stream *hinfo,
 			per_cvt->assigned = 0;
 			hinfo->nid = 0;
 			snd_hda_spdif_ctls_unassign(codec, pin_idx);
+			mutex_unlock(&spec->pcm_lock);
 			return -ENODEV;
 		}
 	}
 
+	mutex_unlock(&spec->pcm_lock);
 	/* Store the updated parameters */
 	runtime->hw.channels_min = hinfo->channels_min;
 	runtime->hw.channels_max = hinfo->channels_max;
@@ -1803,13 +1913,33 @@ static int generic_hdmi_playback_pcm_prepare(struct hda_pcm_stream *hinfo,
 {
 	hda_nid_t cvt_nid = hinfo->nid;
 	struct hdmi_spec *spec = codec->spec;
-	int pin_idx = hinfo_to_pin_index(codec, hinfo);
-	struct hdmi_spec_per_pin *per_pin = get_pin(spec, pin_idx);
-	hda_nid_t pin_nid = per_pin->pin_nid;
+	int pin_idx;
+	struct hdmi_spec_per_pin *per_pin;
+	hda_nid_t pin_nid;
 	struct snd_pcm_runtime *runtime = substream->runtime;
 	struct i915_audio_component *acomp = codec->bus->core.audio_component;
 	bool non_pcm;
 	int pinctl;
+	int err;
+
+	mutex_lock(&spec->pcm_lock);
+	pin_idx = hinfo_to_pin_index(codec, hinfo);
+	if (spec->dyn_pcm_assign && pin_idx < 0) {
+		/* when dyn_pcm_assign and pcm is not bound to a pin
+		 * skip pin setup and return 0 to make audio playback
+		 * be ongoing
+		 */
+		intel_not_share_assigned_cvt_nid(codec, 0, cvt_nid);
+		snd_hda_codec_setup_stream(codec, cvt_nid,
+					stream_tag, 0, format);
+		mutex_unlock(&spec->pcm_lock);
+		return 0;
+	}
+
+	if (snd_BUG_ON(pin_idx < 0))
+		return -EINVAL;
+	per_pin = get_pin(spec, pin_idx);
+	pin_nid = per_pin->pin_nid;
 
 	if (is_haswell_plus(codec) || is_valleyview_plus(codec)) {
 		/* Verify pin:cvt selections to avoid silent audio after S3.
@@ -1838,7 +1968,7 @@ static int generic_hdmi_playback_pcm_prepare(struct hda_pcm_stream *hinfo,
 
 	hdmi_setup_audio_infoframe(codec, per_pin, non_pcm);
 	mutex_unlock(&per_pin->lock);
-
+	mutex_unlock(&spec->pcm_lock);
 	if (spec->dyn_pin_out) {
 		pinctl = snd_hda_codec_read(codec, pin_nid, 0,
 					    AC_VERB_GET_PIN_WIDGET_CONTROL, 0);
@@ -1847,7 +1977,10 @@ static int generic_hdmi_playback_pcm_prepare(struct hda_pcm_stream *hinfo,
 				    pinctl | PIN_OUT);
 	}
 
-	return spec->ops.setup_stream(codec, cvt_nid, pin_nid, stream_tag, format);
+	err = spec->ops.setup_stream(codec, cvt_nid, pin_nid,
+				 stream_tag, format);
+	mutex_unlock(&spec->pcm_lock);
+	return err;
 }
 
 static int generic_hdmi_playback_pcm_cleanup(struct hda_pcm_stream *hinfo,
@@ -1878,9 +2011,17 @@ static int hdmi_pcm_close(struct hda_pcm_stream *hinfo,
 		per_cvt->assigned = 0;
 		hinfo->nid = 0;
 
+		mutex_lock(&spec->pcm_lock);
 		pin_idx = hinfo_to_pin_index(codec, hinfo);
-		if (snd_BUG_ON(pin_idx < 0))
+		if (spec->dyn_pcm_assign && pin_idx < 0) {
+			mutex_unlock(&spec->pcm_lock);
+			return 0;
+		}
+
+		if (snd_BUG_ON(pin_idx < 0)) {
+			mutex_unlock(&spec->pcm_lock);
 			return -EINVAL;
+		}
 		per_pin = get_pin(spec, pin_idx);
 
 		if (spec->dyn_pin_out) {
@@ -1900,6 +2041,7 @@ static int hdmi_pcm_close(struct hda_pcm_stream *hinfo,
 		per_pin->setup = false;
 		per_pin->channels = 0;
 		mutex_unlock(&per_pin->lock);
+		mutex_unlock(&spec->pcm_lock);
 	}
 
 	return 0;
@@ -2370,6 +2512,7 @@ static int patch_generic_hdmi(struct hda_codec *codec)
 		return -ENOMEM;
 
 	spec->ops = generic_standard_hdmi_ops;
+	mutex_init(&spec->pcm_lock);
 	codec->spec = spec;
 	hdmi_array_init(spec, 4);
 
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [RFC V2 PATCH 2/5] ALSA: hda - hdmi operate spdif based on pcm
  2015-11-23 14:09 [RFC V2 PATCH 0/5] hdmi audio dynamically bind PCM to pin libin.yang
  2015-11-23 14:09 ` libin.yang
  2015-11-23 14:09 ` [RFC V2 PATCH 1/5] ALSA: hda - hdmi playback without monitor in dynamic pcm bind mode libin.yang
@ 2015-11-23 14:09 ` libin.yang
  2015-11-23 14:09 ` [RFC V2 PATCH 3/5] ALSA: hda - hdmi dynamically bind PCM to pin when monitor hotplug libin.yang
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 13+ messages in thread
From: libin.yang @ 2015-11-23 14:09 UTC (permalink / raw)
  To: alsa-devel, tiwai; +Cc: libin.yang, Libin Yang, mengdong.lin

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

Currently, the driver operates the spdif based on pin.
This is ok for the current driver as pcm is statically
bound to the pin.

However, if the driver uses dynamically pcm assignment,
this will cause confusion for user space.

The patch changes spdif operation from pin based to pcm based.

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

diff --git a/sound/pci/hda/patch_hdmi.c b/sound/pci/hda/patch_hdmi.c
index d0294bf..de22ac2 100644
--- a/sound/pci/hda/patch_hdmi.c
+++ b/sound/pci/hda/patch_hdmi.c
@@ -136,6 +136,7 @@ struct hdmi_spec {
 	struct snd_array pins; /* struct hdmi_spec_per_pin */
 	struct hda_pcm *pcm_rec[16];
 	struct mutex pcm_lock;
+	int pcm_used;	/* counter of pcm_rec[] */
 	unsigned int channels_max; /* max over all cvts */
 
 	struct hdmi_eld temp_eld;
@@ -376,6 +377,20 @@ static int pin_nid_to_pin_index(struct hda_codec *codec, hda_nid_t pin_nid)
 	return -EINVAL;
 }
 
+static int hinfo_to_pcm_index(struct hda_codec *codec,
+			struct hda_pcm_stream *hinfo)
+{
+	struct hdmi_spec *spec = codec->spec;
+	int pcm_idx;
+
+	for (pcm_idx = 0; pcm_idx < spec->pcm_used; pcm_idx++)
+		if (get_pcm_rec(spec, pcm_idx)->stream == hinfo)
+			return pcm_idx;
+
+	codec_warn(codec, "HDMI: hinfo %p not registered\n", hinfo);
+	return -EINVAL;
+}
+
 static int hinfo_to_pin_index(struct hda_codec *codec,
 			      struct hda_pcm_stream *hinfo)
 {
@@ -1538,13 +1553,17 @@ static int hdmi_pcm_open(struct hda_pcm_stream *hinfo,
 {
 	struct hdmi_spec *spec = codec->spec;
 	struct snd_pcm_runtime *runtime = substream->runtime;
-	int pin_idx, cvt_idx, mux_idx = 0;
+	int pin_idx, cvt_idx, pcm_idx, mux_idx = 0;
 	struct hdmi_spec_per_pin *per_pin;
 	struct hdmi_eld *eld;
 	struct hdmi_spec_per_cvt *per_cvt = NULL;
 	int err;
 
 	/* Validate hinfo */
+	pcm_idx = hinfo_to_pcm_index(codec, hinfo);
+	if (pcm_idx < 0)
+		return -EINVAL;
+
 	mutex_lock(&spec->pcm_lock);
 	pin_idx = hinfo_to_pin_index(codec, hinfo);
 	if (!spec->dyn_pcm_assign) {
@@ -1586,7 +1605,7 @@ static int hdmi_pcm_open(struct hda_pcm_stream *hinfo,
 	if (is_haswell_plus(codec) || is_valleyview_plus(codec))
 		intel_not_share_assigned_cvt(codec, per_pin->pin_nid, mux_idx);
 
-	snd_hda_spdif_ctls_assign(codec, pin_idx, per_cvt->cvt_nid);
+	snd_hda_spdif_ctls_assign(codec, pcm_idx, per_cvt->cvt_nid);
 
 	/* Initially set the converter's capabilities */
 	hinfo->channels_min = per_cvt->channels_min;
@@ -1603,7 +1622,7 @@ static int hdmi_pcm_open(struct hda_pcm_stream *hinfo,
 		    !hinfo->rates || !hinfo->formats) {
 			per_cvt->assigned = 0;
 			hinfo->nid = 0;
-			snd_hda_spdif_ctls_unassign(codec, pin_idx);
+			snd_hda_spdif_ctls_unassign(codec, pcm_idx);
 			mutex_unlock(&spec->pcm_lock);
 			return -ENODEV;
 		}
@@ -1996,12 +2015,15 @@ static int hdmi_pcm_close(struct hda_pcm_stream *hinfo,
 			  struct snd_pcm_substream *substream)
 {
 	struct hdmi_spec *spec = codec->spec;
-	int cvt_idx, pin_idx;
+	int cvt_idx, pin_idx, pcm_idx;
 	struct hdmi_spec_per_cvt *per_cvt;
 	struct hdmi_spec_per_pin *per_pin;
 	int pinctl;
 
 	if (hinfo->nid) {
+		pcm_idx = hinfo_to_pcm_index(codec, hinfo);
+		if (snd_BUG_ON(pcm_idx < 0))
+			return -EINVAL;
 		cvt_idx = cvt_nid_to_cvt_index(codec, hinfo->nid);
 		if (snd_BUG_ON(cvt_idx < 0))
 			return -EINVAL;
@@ -2032,7 +2054,7 @@ static int hdmi_pcm_close(struct hda_pcm_stream *hinfo,
 					    pinctl & ~PIN_OUT);
 		}
 
-		snd_hda_spdif_ctls_unassign(codec, pin_idx);
+		snd_hda_spdif_ctls_unassign(codec, pcm_idx);
 
 		mutex_lock(&per_pin->lock);
 		per_pin->chmap_set = false;
@@ -2230,6 +2252,7 @@ static int generic_hdmi_build_pcms(struct hda_codec *codec)
 			per_pin->pcm = info;
 		}
 		spec->pcm_rec[pin_idx] = info;
+		spec->pcm_used++;
 		info->pcm_type = HDA_PCM_TYPE_HDMI;
 		info->own_chmap = true;
 
@@ -2277,6 +2300,7 @@ static int generic_hdmi_build_controls(struct hda_codec *codec)
 						  HDA_PCM_TYPE_HDMI);
 		if (err < 0)
 			return err;
+		/* pin number is the same with pcm number so far */
 		snd_hda_spdif_ctls_unassign(codec, pin_idx);
 
 		/* add control for ELD Bytes */
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [RFC V2 PATCH 3/5] ALSA: hda - hdmi dynamically bind PCM to pin when monitor hotplug
  2015-11-23 14:09 [RFC V2 PATCH 0/5] hdmi audio dynamically bind PCM to pin libin.yang
                   ` (2 preceding siblings ...)
  2015-11-23 14:09 ` [RFC V2 PATCH 2/5] ALSA: hda - hdmi operate spdif based on pcm libin.yang
@ 2015-11-23 14:09 ` libin.yang
  2015-11-23 15:59   ` Takashi Iwai
  2015-11-23 14:09 ` [RFC V2 PATCH 4/5] ALSA: hda - add in_use flag in hda_pcm libin.yang
  2015-11-23 14:09 ` [RFC V2 PATCH 5/5] ALSA: hda - hdmi setup pin when monitor hotplug in pcm dynamic assignment mode libin.yang
  5 siblings, 1 reply; 13+ messages in thread
From: libin.yang @ 2015-11-23 14:09 UTC (permalink / raw)
  To: alsa-devel, tiwai; +Cc: libin.yang, Libin Yang, mengdong.lin

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

Dynamically bind/unbind the PCM to pin when HDMI/DP monitor hotplug.

When monitor is connected, find a proper PCM for the monitor.
When monitor is disconnected, unbind the PCM from the pin.

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

diff --git a/sound/pci/hda/patch_hdmi.c b/sound/pci/hda/patch_hdmi.c
index de22ac2..bcc2a90 100644
--- a/sound/pci/hda/patch_hdmi.c
+++ b/sound/pci/hda/patch_hdmi.c
@@ -72,6 +72,8 @@ struct hdmi_spec_per_cvt {
 
 struct hdmi_spec_per_pin {
 	hda_nid_t pin_nid;
+	/* pin idx, different device entries on the same pin use the same idx */
+	int pin_nid_idx;
 	int num_mux_nids;
 	hda_nid_t mux_nids[HDA_MAX_CONNECTIONS];
 	int mux_idx;
@@ -83,6 +85,7 @@ struct hdmi_spec_per_pin {
 	struct delayed_work work;
 	struct snd_kcontrol *eld_ctl;
 	struct hda_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 */
 	int channels; /* current number of channels */
@@ -136,6 +139,7 @@ struct hdmi_spec {
 	struct snd_array pins; /* struct hdmi_spec_per_pin */
 	struct hda_pcm *pcm_rec[16];
 	struct mutex pcm_lock;
+	unsigned long pcm_bitmap;
 	int pcm_used;	/* counter of pcm_rec[] */
 	unsigned int channels_max; /* max over all cvts */
 
@@ -1663,6 +1667,70 @@ static int hdmi_read_pin_conn(struct hda_codec *codec, int pin_idx)
 	return 0;
 }
 
+static int get_preferred_pcm_slot(struct hdmi_spec *spec,
+				struct hdmi_spec_per_pin *per_pin)
+{
+	if ((spec->pcm_bitmap & (1 << per_pin->pin_nid_idx)) == 0)
+		return per_pin->pin_nid_idx;
+	return -1;
+}
+
+static int hdmi_find_pcm_slot(struct hdmi_spec *spec,
+				struct hdmi_spec_per_pin *per_pin)
+{
+	int slot;
+	int i;
+
+	/* try the prefer PCM */
+	slot = get_preferred_pcm_slot(spec, per_pin);
+	if (slot != -1)
+		return slot;
+
+	/* have a second try */
+	for (i = spec->num_pins; i < spec->pcm_used; i++) {
+		if ((spec->pcm_bitmap & (1 << i)) == 0)
+			return i;
+	}
+
+	/* the last try */
+	for (i = 0; i < spec->pcm_used; i++) {
+		if ((spec->pcm_bitmap & (1 << i)) == 0)
+			return i;
+	}
+	return -ENODEV;
+}
+
+static void hdmi_attach_hda_pcm(struct hdmi_spec *spec,
+				struct hdmi_spec_per_pin *per_pin)
+{
+	int idx;
+
+	/* pcm already be attached to the pin */
+	if (per_pin->pcm)
+		return;
+	idx = hdmi_find_pcm_slot(spec, per_pin);
+	if (idx == -ENODEV)
+		return;
+	per_pin->pcm_idx = idx;
+	per_pin->pcm = spec->pcm_rec[idx];
+	set_bit(idx, &spec->pcm_bitmap);
+}
+
+static void hdmi_detach_hda_pcm(struct hdmi_spec *spec,
+				struct hdmi_spec_per_pin *per_pin)
+{
+	int idx;
+
+	/* pcm already be detached from the pin */
+	if (!per_pin->pcm)
+		return;
+	idx = per_pin->pcm_idx;
+	per_pin->pcm_idx = -1;
+	per_pin->pcm = NULL;
+	if (idx >= 0 && idx < spec->pcm_used)
+		clear_bit(idx, &spec->pcm_bitmap);
+}
+
 static bool hdmi_present_sense(struct hdmi_spec_per_pin *per_pin, int repoll)
 {
 	struct hda_jack_tbl *jack;
@@ -1687,6 +1755,7 @@ static bool hdmi_present_sense(struct hdmi_spec_per_pin *per_pin, int repoll)
 	snd_hda_power_up_pm(codec);
 	present = snd_hda_pin_sense(codec, pin_nid);
 
+	mutex_lock(&spec->pcm_lock);
 	mutex_lock(&per_pin->lock);
 	pin_eld->monitor_present = !!(present & AC_PINSENSE_PRESENCE);
 	if (pin_eld->monitor_present)
@@ -1720,6 +1789,13 @@ static bool hdmi_present_sense(struct hdmi_spec_per_pin *per_pin, int repoll)
 		}
 	}
 
+	if (spec->dyn_pcm_assign) {
+		if (eld->eld_valid)
+			hdmi_attach_hda_pcm(spec, per_pin);
+		else
+			hdmi_detach_hda_pcm(spec, per_pin);
+	}
+
 	if (pin_eld->eld_valid != eld->eld_valid)
 		eld_changed = true;
 
@@ -1770,6 +1846,7 @@ static bool hdmi_present_sense(struct hdmi_spec_per_pin *per_pin, int repoll)
 		jack->block_report = !ret;
 
 	mutex_unlock(&per_pin->lock);
+	mutex_unlock(&spec->pcm_lock);
 	snd_hda_power_down_pm(codec);
 	return ret;
 }
@@ -1815,6 +1892,11 @@ static int hdmi_add_pin(struct hda_codec *codec, hda_nid_t pin_nid)
 
 	per_pin->pin_nid = pin_nid;
 	per_pin->non_pcm = false;
+	if (spec->dyn_pcm_assign)
+		per_pin->pcm_idx = -1;
+	else
+		per_pin->pcm_idx = pin_idx;
+	per_pin->pin_nid_idx = pin_idx;
 
 	err = hdmi_read_pin_conn(codec, pin_idx);
 	if (err < 0)
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [RFC V2 PATCH 4/5] ALSA: hda - add in_use flag in hda_pcm
  2015-11-23 14:09 [RFC V2 PATCH 0/5] hdmi audio dynamically bind PCM to pin libin.yang
                   ` (3 preceding siblings ...)
  2015-11-23 14:09 ` [RFC V2 PATCH 3/5] ALSA: hda - hdmi dynamically bind PCM to pin when monitor hotplug libin.yang
@ 2015-11-23 14:09 ` libin.yang
  2015-11-23 15:50   ` Takashi Iwai
  2015-11-23 14:09 ` [RFC V2 PATCH 5/5] ALSA: hda - hdmi setup pin when monitor hotplug in pcm dynamic assignment mode libin.yang
  5 siblings, 1 reply; 13+ messages in thread
From: libin.yang @ 2015-11-23 14:09 UTC (permalink / raw)
  To: alsa-devel, tiwai; +Cc: libin.yang, Libin Yang, mengdong.lin

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

Add in_use flag in struct hda_pcm for HD Audio dynamic PCM assignment.

Signed-off-by: Libin Yang <libin.yang@linux.intel.com>
---
 sound/pci/hda/hda_codec.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/sound/pci/hda/hda_codec.h b/sound/pci/hda/hda_codec.h
index 373fcad..ee97401 100644
--- a/sound/pci/hda/hda_codec.h
+++ b/sound/pci/hda/hda_codec.h
@@ -167,6 +167,7 @@ enum {
 /* for PCM creation */
 struct hda_pcm {
 	char *name;
+	bool in_use;
 	struct hda_pcm_stream stream[2];
 	unsigned int pcm_type;	/* HDA_PCM_TYPE_XXX */
 	int device;		/* device number to assign */
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [RFC V2 PATCH 5/5] ALSA: hda - hdmi setup pin when monitor hotplug in pcm dynamic assignment mode
  2015-11-23 14:09 [RFC V2 PATCH 0/5] hdmi audio dynamically bind PCM to pin libin.yang
                   ` (4 preceding siblings ...)
  2015-11-23 14:09 ` [RFC V2 PATCH 4/5] ALSA: hda - add in_use flag in hda_pcm libin.yang
@ 2015-11-23 14:09 ` libin.yang
  5 siblings, 0 replies; 13+ messages in thread
From: libin.yang @ 2015-11-23 14:09 UTC (permalink / raw)
  To: alsa-devel, tiwai; +Cc: libin.yang, Libin Yang, mengdong.lin

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

Setup pin configuration when monitor is hotplugged
in pcm dynamic assignment if the PCM is in open state.

When monitor is disconnect, The pin will be reset.

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

diff --git a/sound/pci/hda/patch_hdmi.c b/sound/pci/hda/patch_hdmi.c
index bcc2a90..67e2d43 100644
--- a/sound/pci/hda/patch_hdmi.c
+++ b/sound/pci/hda/patch_hdmi.c
@@ -1514,10 +1514,14 @@ static int hdmi_pcm_open_no_pin(struct hda_pcm_stream *hinfo,
 {
 	struct hdmi_spec *spec = codec->spec;
 	struct snd_pcm_runtime *runtime = substream->runtime;
-	int cvt_idx;
+	int cvt_idx, pcm_idx;
 	struct hdmi_spec_per_cvt *per_cvt = NULL;
 	int err;
 
+	pcm_idx = hinfo_to_pcm_index(codec, hinfo);
+	if (pcm_idx < 0)
+		return -EINVAL;
+
 	err = hdmi_choose_cvt(codec, -1, &cvt_idx, NULL);
 	if (err)
 		return err;
@@ -1528,6 +1532,7 @@ static int hdmi_pcm_open_no_pin(struct hda_pcm_stream *hinfo,
 
 	intel_not_share_assigned_cvt_nid(codec, 0, per_cvt->cvt_nid);
 
+	spec->pcm_rec[pcm_idx]->in_use = true;
 	/* todo: setup spdif ctls assign */
 
 	/* Initially set the converter's capabilities */
@@ -1596,7 +1601,7 @@ static int hdmi_pcm_open(struct hda_pcm_stream *hinfo,
 	/* Claim converter */
 	per_cvt->assigned = 1;
 
-
+	spec->pcm_rec[pcm_idx]->in_use = true;
 	per_pin = get_pin(spec, pin_idx);
 	per_pin->cvt_nid = per_cvt->cvt_nid;
 	hinfo->nid = per_cvt->cvt_nid;
@@ -1731,6 +1736,71 @@ static void hdmi_detach_hda_pcm(struct hdmi_spec *spec,
 		clear_bit(idx, &spec->pcm_bitmap);
 }
 
+static int hdmi_get_pin_cvt_mux(struct hdmi_spec *spec,
+		struct hdmi_spec_per_pin *per_pin, hda_nid_t cvt_nid)
+{
+	int mux_idx;
+
+	for (mux_idx = 0; mux_idx < per_pin->num_mux_nids; mux_idx++)
+		if (per_pin->mux_nids[mux_idx] == cvt_nid)
+			break;
+	return mux_idx;
+}
+
+static bool check_non_pcm_per_cvt(struct hda_codec *codec, hda_nid_t cvt_nid);
+static void hdmi_pcm_setup_pin(struct hdmi_spec *spec,
+			   struct hdmi_spec_per_pin *per_pin)
+{
+	struct hda_codec *codec = per_pin->codec;
+	struct hda_pcm *pcm;
+	struct hda_pcm_stream *hinfo;
+	struct snd_pcm_substream *substream;
+
+	int mux_idx;
+	bool non_pcm;
+
+	if (per_pin->pcm_idx >= 0 && per_pin->pcm_idx < spec->pcm_used)
+		pcm = spec->pcm_rec[per_pin->pcm_idx];
+	else
+		return;
+	if (!pcm->in_use)
+		return;
+
+	/* hdmi audio only uses playback and one substream */
+	hinfo = pcm->stream;
+	substream = pcm->pcm->streams[0].substream;
+
+	per_pin->cvt_nid = hinfo->nid;
+
+	mux_idx = hdmi_get_pin_cvt_mux(spec, per_pin, hinfo->nid);
+	if (mux_idx < per_pin->num_mux_nids)
+		snd_hda_codec_write_cache(codec, per_pin->pin_nid, 0,
+				AC_VERB_SET_CONNECT_SEL,
+				mux_idx);
+	snd_hda_spdif_ctls_assign(codec, per_pin->pcm_idx, hinfo->nid);
+
+	non_pcm = check_non_pcm_per_cvt(codec, hinfo->nid);
+	if (substream->runtime)
+		per_pin->channels = substream->runtime->channels;
+	per_pin->setup = true;
+	per_pin->mux_idx = mux_idx;
+
+	hdmi_setup_audio_infoframe(codec, per_pin, non_pcm);
+}
+
+static void hdmi_pcm_reset_pin(struct hdmi_spec *spec,
+			   struct hdmi_spec_per_pin *per_pin)
+{
+	if (per_pin->pcm_idx >= 0 && per_pin->pcm_idx < spec->pcm_used)
+		snd_hda_spdif_ctls_unassign(per_pin->codec, per_pin->pcm_idx);
+
+	per_pin->chmap_set = false;
+	memset(per_pin->chmap, 0, sizeof(per_pin->chmap));
+
+	per_pin->setup = false;
+	per_pin->channels = 0;
+}
+
 static bool hdmi_present_sense(struct hdmi_spec_per_pin *per_pin, int repoll)
 {
 	struct hda_jack_tbl *jack;
@@ -1790,10 +1860,13 @@ static bool hdmi_present_sense(struct hdmi_spec_per_pin *per_pin, int repoll)
 	}
 
 	if (spec->dyn_pcm_assign) {
-		if (eld->eld_valid)
+		if (eld->eld_valid) {
 			hdmi_attach_hda_pcm(spec, per_pin);
-		else
+			hdmi_pcm_setup_pin(spec, per_pin);
+		} else {
+			hdmi_pcm_reset_pin(spec, per_pin);
 			hdmi_detach_hda_pcm(spec, per_pin);
+		}
 	}
 
 	if (pin_eld->eld_valid != eld->eld_valid)
@@ -2116,6 +2189,7 @@ static int hdmi_pcm_close(struct hda_pcm_stream *hinfo,
 		hinfo->nid = 0;
 
 		mutex_lock(&spec->pcm_lock);
+		spec->pcm_rec[pcm_idx]->in_use = false;
 		pin_idx = hinfo_to_pin_index(codec, hinfo);
 		if (spec->dyn_pcm_assign && pin_idx < 0) {
 			mutex_unlock(&spec->pcm_lock);
-- 
1.9.1

^ permalink raw reply related	[flat|nested] 13+ messages in thread

* Re: [RFC V2 PATCH 4/5] ALSA: hda - add in_use flag in hda_pcm
  2015-11-23 14:09 ` [RFC V2 PATCH 4/5] ALSA: hda - add in_use flag in hda_pcm libin.yang
@ 2015-11-23 15:50   ` Takashi Iwai
  2015-11-25  6:50     ` Yang, Libin
  0 siblings, 1 reply; 13+ messages in thread
From: Takashi Iwai @ 2015-11-23 15:50 UTC (permalink / raw)
  To: libin.yang; +Cc: libin.yang, alsa-devel, mengdong.lin

On Mon, 23 Nov 2015 15:09:33 +0100,
libin.yang@linux.intel.com wrote:
> 
> From: Libin Yang <libin.yang@linux.intel.com>
> 
> Add in_use flag in struct hda_pcm for HD Audio dynamic PCM assignment.
> 
> Signed-off-by: Libin Yang <libin.yang@linux.intel.com>

This should be folded into the last patch.  The addition of the flag
alone doesn't make much sense.


Takashi

> ---
>  sound/pci/hda/hda_codec.h | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/sound/pci/hda/hda_codec.h b/sound/pci/hda/hda_codec.h
> index 373fcad..ee97401 100644
> --- a/sound/pci/hda/hda_codec.h
> +++ b/sound/pci/hda/hda_codec.h
> @@ -167,6 +167,7 @@ enum {
>  /* for PCM creation */
>  struct hda_pcm {
>  	char *name;
> +	bool in_use;
>  	struct hda_pcm_stream stream[2];
>  	unsigned int pcm_type;	/* HDA_PCM_TYPE_XXX */
>  	int device;		/* device number to assign */
> -- 
> 1.9.1
> 

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [RFC V2 PATCH 3/5] ALSA: hda - hdmi dynamically bind PCM to pin when monitor hotplug
  2015-11-23 14:09 ` [RFC V2 PATCH 3/5] ALSA: hda - hdmi dynamically bind PCM to pin when monitor hotplug libin.yang
@ 2015-11-23 15:59   ` Takashi Iwai
  2015-11-25  6:53     ` Yang, Libin
  0 siblings, 1 reply; 13+ messages in thread
From: Takashi Iwai @ 2015-11-23 15:59 UTC (permalink / raw)
  To: libin.yang; +Cc: libin.yang, alsa-devel, mengdong.lin

On Mon, 23 Nov 2015 15:09:32 +0100,
libin.yang@linux.intel.com wrote:
> 
> From: Libin Yang <libin.yang@linux.intel.com>
> 
> Dynamically bind/unbind the PCM to pin when HDMI/DP monitor hotplug.
> 
> When monitor is connected, find a proper PCM for the monitor.
> When monitor is disconnected, unbind the PCM from the pin.

You must have some strategy about binding.  Please tell the story
behind the scene, too.

> 
> Signed-off-by: Libin Yang <libin.yang@linux.intel.com>
> ---
>  sound/pci/hda/patch_hdmi.c | 82 ++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 82 insertions(+)
> 
> diff --git a/sound/pci/hda/patch_hdmi.c b/sound/pci/hda/patch_hdmi.c
> index de22ac2..bcc2a90 100644
> --- a/sound/pci/hda/patch_hdmi.c
> +++ b/sound/pci/hda/patch_hdmi.c
> @@ -72,6 +72,8 @@ struct hdmi_spec_per_cvt {
>  
>  struct hdmi_spec_per_pin {
>  	hda_nid_t pin_nid;
> +	/* pin idx, different device entries on the same pin use the same idx */
> +	int pin_nid_idx;
>  	int num_mux_nids;
>  	hda_nid_t mux_nids[HDA_MAX_CONNECTIONS];
>  	int mux_idx;
> @@ -83,6 +85,7 @@ struct hdmi_spec_per_pin {
>  	struct delayed_work work;
>  	struct snd_kcontrol *eld_ctl;
>  	struct hda_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 */
>  	int channels; /* current number of channels */
> @@ -136,6 +139,7 @@ struct hdmi_spec {
>  	struct snd_array pins; /* struct hdmi_spec_per_pin */
>  	struct hda_pcm *pcm_rec[16];
>  	struct mutex pcm_lock;
> +	unsigned long pcm_bitmap;
>  	int pcm_used;	/* counter of pcm_rec[] */
>  	unsigned int channels_max; /* max over all cvts */
>  
> @@ -1663,6 +1667,70 @@ static int hdmi_read_pin_conn(struct hda_codec *codec, int pin_idx)
>  	return 0;
>  }
>  
> +static int get_preferred_pcm_slot(struct hdmi_spec *spec,
> +				struct hdmi_spec_per_pin *per_pin)
> +{
> +	if ((spec->pcm_bitmap & (1 << per_pin->pin_nid_idx)) == 0)

Use test_bit().

> +		return per_pin->pin_nid_idx;
> +	return -1;
> +}
> +
> +static int hdmi_find_pcm_slot(struct hdmi_spec *spec,
> +				struct hdmi_spec_per_pin *per_pin)
> +{
> +	int slot;
> +	int i;
> +
> +	/* try the prefer PCM */
> +	slot = get_preferred_pcm_slot(spec, per_pin);
> +	if (slot != -1)
> +		return slot;
> +
> +	/* have a second try */
> +	for (i = spec->num_pins; i < spec->pcm_used; i++) {
> +		if ((spec->pcm_bitmap & (1 << i)) == 0)

Ditto.

> +			return i;
> +	}
> +
> +	/* the last try */
> +	for (i = 0; i < spec->pcm_used; i++) {
> +		if ((spec->pcm_bitmap & (1 << i)) == 0)

Ditto.

> +			return i;
> +	}
> +	return -ENODEV;

I think you don't need to split get_preferred_pcm_slot().
It's a function that is called only from here, and it's even better to
be open-coded.  Then you can see the code flow more easily.
(Also, you may notice that the last loop doesn't go all, but just up
 to num_pins, by looking through the code closely.)

static int hdmi_find_pcm_slot(struct hdmi_spec *spec,
				struct hdmi_spec_per_pin *per_pin)
{
	int i;

	/* try the prefer PCM */
	if (!test_bit(per_pin->pin_nid_idx, &spec->pcm_bitmap))
		return per_pin->pin_nid_idx;

	/* have a second try; check the "reserved area" over num_pins */
	for (i = spec->num_pins; i < spec->pcm_used; i++)
		if (!test_bit(i, &spec->pcm_bitmap))
			return i;

	/* the last try; check the empty slots in pins */
	for (i = 0; i < spec->num_pins; i++)
		if (!test_bit(i, &spec->pcm_bitmap))
			return i;

	return -EBUSY;
}



Takashi

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [RFC V2 PATCH 1/5] ALSA: hda - hdmi playback without monitor in dynamic pcm bind mode
  2015-11-23 14:09 ` [RFC V2 PATCH 1/5] ALSA: hda - hdmi playback without monitor in dynamic pcm bind mode libin.yang
@ 2015-11-23 16:01   ` Takashi Iwai
  2015-11-25  6:50     ` Yang, Libin
  0 siblings, 1 reply; 13+ messages in thread
From: Takashi Iwai @ 2015-11-23 16:01 UTC (permalink / raw)
  To: libin.yang; +Cc: libin.yang, alsa-devel, mengdong.lin

On Mon, 23 Nov 2015 15:09:30 +0100,
libin.yang@linux.intel.com wrote:
> 
> From: Libin Yang <libin.yang@linux.intel.com>
> 
> Pulseaudio requires open pcm successfully when probing.
> 
> This patch handles playback without monitor in dynamic pcm assignment
> mode. It tries to open/prepare/close pcm successfully even there is
> no pin bound to the PCM. On the meantime, it will try to find a proper
> converter for the PCM.

The explanation why you added pcm_lock is missing.


Takashi

> 
> Signed-off-by: Libin Yang <libin.yang@linux.intel.com>
> ---
>  sound/pci/hda/patch_hdmi.c | 171 +++++++++++++++++++++++++++++++++++++++++----
>  1 file changed, 157 insertions(+), 14 deletions(-)
> 
> diff --git a/sound/pci/hda/patch_hdmi.c b/sound/pci/hda/patch_hdmi.c
> index 92e05fb..d0294bf 100644
> --- a/sound/pci/hda/patch_hdmi.c
> +++ b/sound/pci/hda/patch_hdmi.c
> @@ -135,6 +135,7 @@ struct hdmi_spec {
>  	int num_pins;
>  	struct snd_array pins; /* struct hdmi_spec_per_pin */
>  	struct hda_pcm *pcm_rec[16];
> +	struct mutex pcm_lock;
>  	unsigned int channels_max; /* max over all cvts */
>  
>  	struct hdmi_eld temp_eld;
> @@ -1331,6 +1332,11 @@ static int hdmi_setup_stream(struct hda_codec *codec, hda_nid_t cvt_nid,
>  	return 0;
>  }
>  
> +/* Try to find an available converter
> + * If pin_idx is less then zero, just try to find an available converter.
> + * Otherwise, try to find an available converter and get the cvt mux index
> + * of the pin.
> + */
>  static int hdmi_choose_cvt(struct hda_codec *codec,
>  			int pin_idx, int *cvt_id, int *mux_id)
>  {
> @@ -1339,7 +1345,11 @@ static int hdmi_choose_cvt(struct hda_codec *codec,
>  	struct hdmi_spec_per_cvt *per_cvt = NULL;
>  	int cvt_idx, mux_idx = 0;
>  
> -	per_pin = get_pin(spec, pin_idx);
> +	/* pin_idx < 0 means no pin will be bound to the converter */
> +	if (pin_idx < 0)
> +		per_pin = NULL;
> +	else
> +		per_pin = get_pin(spec, pin_idx);
>  
>  	/* Dynamically assign converter to stream */
>  	for (cvt_idx = 0; cvt_idx < spec->num_cvts; cvt_idx++) {
> @@ -1348,6 +1358,8 @@ static int hdmi_choose_cvt(struct hda_codec *codec,
>  		/* Must not already be assigned */
>  		if (per_cvt->assigned)
>  			continue;
> +		if (per_pin == NULL)
> +			break;
>  		/* Must be in pin's mux's list of converters */
>  		for (mux_idx = 0; mux_idx < per_pin->num_mux_nids; mux_idx++)
>  			if (per_pin->mux_nids[mux_idx] == per_cvt->cvt_nid)
> @@ -1360,9 +1372,10 @@ static int hdmi_choose_cvt(struct hda_codec *codec,
>  
>  	/* No free converters */
>  	if (cvt_idx == spec->num_cvts)
> -		return -ENODEV;
> +		return -EBUSY;
>  
> -	per_pin->mux_idx = mux_idx;
> +	if (per_pin != NULL)
> +		per_pin->mux_idx = mux_idx;
>  
>  	if (cvt_id)
>  		*cvt_id = cvt_idx;
> @@ -1388,6 +1401,20 @@ static void intel_verify_pin_cvt_connect(struct hda_codec *codec,
>  					    mux_idx);
>  }
>  
> +/* get the mux index for the converter of the pins
> + * converter's mux index is the same for all pins on Intel platform
> + */
> +static int intel_cvt_id_to_mux_idx(struct hdmi_spec *spec,
> +			hda_nid_t cvt_nid)
> +{
> +	int i;
> +
> +	for (i = 0; i < spec->num_cvts; i++)
> +		if (spec->cvt_nids[i] == cvt_nid)
> +			return i;
> +	return -EINVAL;
> +}
> +
>  /* Intel HDMI workaround to fix audio routing issue:
>   * For some Intel display codecs, pins share the same connection list.
>   * So a conveter can be selected by multiple pins and playback on any of these
> @@ -1439,6 +1466,69 @@ static void intel_not_share_assigned_cvt(struct hda_codec *codec,
>  	}
>  }
>  
> +/* A wrapper of intel_not_share_asigned_cvt() */
> +static void intel_not_share_assigned_cvt_nid(struct hda_codec *codec,
> +			hda_nid_t pin_nid, hda_nid_t cvt_nid)
> +{
> +	int mux_idx;
> +	struct hdmi_spec *spec = codec->spec;
> +
> +	if (!is_haswell_plus(codec) && !is_valleyview_plus(codec))
> +		return;
> +
> +	/* On Intel platform, the mapping of converter nid to
> +	 * mux index of the pins are always the same.
> +	 * The pin nid may be 0, this means all pins will not
> +	 * share the converter.
> +	 */
> +	mux_idx = intel_cvt_id_to_mux_idx(spec, cvt_nid);
> +	if (mux_idx > 0)
> +		intel_not_share_assigned_cvt(codec, pin_nid, mux_idx);
> +}
> +
> +/* called in hdmi_pcm_open when no pin is assigned to the PCM
> + * in dyn_pcm_assign mode.
> + */
> +static int hdmi_pcm_open_no_pin(struct hda_pcm_stream *hinfo,
> +			 struct hda_codec *codec,
> +			 struct snd_pcm_substream *substream)
> +{
> +	struct hdmi_spec *spec = codec->spec;
> +	struct snd_pcm_runtime *runtime = substream->runtime;
> +	int cvt_idx;
> +	struct hdmi_spec_per_cvt *per_cvt = NULL;
> +	int err;
> +
> +	err = hdmi_choose_cvt(codec, -1, &cvt_idx, NULL);
> +	if (err)
> +		return err;
> +
> +	per_cvt = get_cvt(spec, cvt_idx);
> +	per_cvt->assigned = 1;
> +	hinfo->nid = per_cvt->cvt_nid;
> +
> +	intel_not_share_assigned_cvt_nid(codec, 0, per_cvt->cvt_nid);
> +
> +	/* todo: setup spdif ctls assign */
> +
> +	/* Initially set the converter's capabilities */
> +	hinfo->channels_min = per_cvt->channels_min;
> +	hinfo->channels_max = per_cvt->channels_max;
> +	hinfo->rates = per_cvt->rates;
> +	hinfo->formats = per_cvt->formats;
> +	hinfo->maxbps = per_cvt->maxbps;
> +
> +	/* Store the updated parameters */
> +	runtime->hw.channels_min = hinfo->channels_min;
> +	runtime->hw.channels_max = hinfo->channels_max;
> +	runtime->hw.formats = hinfo->formats;
> +	runtime->hw.rates = hinfo->rates;
> +
> +	snd_pcm_hw_constraint_step(substream->runtime, 0,
> +				   SNDRV_PCM_HW_PARAM_CHANNELS, 2);
> +	return 0;
> +}
> +
>  /*
>   * HDA PCM callbacks
>   */
> @@ -1455,19 +1545,36 @@ static int hdmi_pcm_open(struct hda_pcm_stream *hinfo,
>  	int err;
>  
>  	/* Validate hinfo */
> +	mutex_lock(&spec->pcm_lock);
>  	pin_idx = hinfo_to_pin_index(codec, hinfo);
> -	if (snd_BUG_ON(pin_idx < 0))
> -		return -EINVAL;
> -	per_pin = get_pin(spec, pin_idx);
> -	eld = &per_pin->sink_eld;
> +	if (!spec->dyn_pcm_assign) {
> +		if (snd_BUG_ON(pin_idx < 0)) {
> +			mutex_unlock(&spec->pcm_lock);
> +			return -EINVAL;
> +		}
> +	} else {
> +		/* no pin is assigned to the PCM
> +		 * PA need pcm open successfully when probe
> +		 */
> +		if (pin_idx < 0) {
> +			err = hdmi_pcm_open_no_pin(hinfo, codec, substream);
> +			mutex_unlock(&spec->pcm_lock);
> +			return err;
> +		}
> +	}
>  
>  	err = hdmi_choose_cvt(codec, pin_idx, &cvt_idx, &mux_idx);
> -	if (err < 0)
> +	if (err < 0) {
> +		mutex_unlock(&spec->pcm_lock);
>  		return err;
> +	}
>  
>  	per_cvt = get_cvt(spec, cvt_idx);
>  	/* Claim converter */
>  	per_cvt->assigned = 1;
> +
> +
> +	per_pin = get_pin(spec, pin_idx);
>  	per_pin->cvt_nid = per_cvt->cvt_nid;
>  	hinfo->nid = per_cvt->cvt_nid;
>  
> @@ -1488,6 +1595,7 @@ static int hdmi_pcm_open(struct hda_pcm_stream *hinfo,
>  	hinfo->formats = per_cvt->formats;
>  	hinfo->maxbps = per_cvt->maxbps;
>  
> +	eld = &per_pin->sink_eld;
>  	/* Restrict capabilities by ELD if this isn't disabled */
>  	if (!static_hdmi_pcm && eld->eld_valid) {
>  		snd_hdmi_eld_update_pcm_info(&eld->info, hinfo);
> @@ -1496,10 +1604,12 @@ static int hdmi_pcm_open(struct hda_pcm_stream *hinfo,
>  			per_cvt->assigned = 0;
>  			hinfo->nid = 0;
>  			snd_hda_spdif_ctls_unassign(codec, pin_idx);
> +			mutex_unlock(&spec->pcm_lock);
>  			return -ENODEV;
>  		}
>  	}
>  
> +	mutex_unlock(&spec->pcm_lock);
>  	/* Store the updated parameters */
>  	runtime->hw.channels_min = hinfo->channels_min;
>  	runtime->hw.channels_max = hinfo->channels_max;
> @@ -1803,13 +1913,33 @@ static int generic_hdmi_playback_pcm_prepare(struct hda_pcm_stream *hinfo,
>  {
>  	hda_nid_t cvt_nid = hinfo->nid;
>  	struct hdmi_spec *spec = codec->spec;
> -	int pin_idx = hinfo_to_pin_index(codec, hinfo);
> -	struct hdmi_spec_per_pin *per_pin = get_pin(spec, pin_idx);
> -	hda_nid_t pin_nid = per_pin->pin_nid;
> +	int pin_idx;
> +	struct hdmi_spec_per_pin *per_pin;
> +	hda_nid_t pin_nid;
>  	struct snd_pcm_runtime *runtime = substream->runtime;
>  	struct i915_audio_component *acomp = codec->bus->core.audio_component;
>  	bool non_pcm;
>  	int pinctl;
> +	int err;
> +
> +	mutex_lock(&spec->pcm_lock);
> +	pin_idx = hinfo_to_pin_index(codec, hinfo);
> +	if (spec->dyn_pcm_assign && pin_idx < 0) {
> +		/* when dyn_pcm_assign and pcm is not bound to a pin
> +		 * skip pin setup and return 0 to make audio playback
> +		 * be ongoing
> +		 */
> +		intel_not_share_assigned_cvt_nid(codec, 0, cvt_nid);
> +		snd_hda_codec_setup_stream(codec, cvt_nid,
> +					stream_tag, 0, format);
> +		mutex_unlock(&spec->pcm_lock);
> +		return 0;
> +	}
> +
> +	if (snd_BUG_ON(pin_idx < 0))
> +		return -EINVAL;
> +	per_pin = get_pin(spec, pin_idx);
> +	pin_nid = per_pin->pin_nid;
>  
>  	if (is_haswell_plus(codec) || is_valleyview_plus(codec)) {
>  		/* Verify pin:cvt selections to avoid silent audio after S3.
> @@ -1838,7 +1968,7 @@ static int generic_hdmi_playback_pcm_prepare(struct hda_pcm_stream *hinfo,
>  
>  	hdmi_setup_audio_infoframe(codec, per_pin, non_pcm);
>  	mutex_unlock(&per_pin->lock);
> -
> +	mutex_unlock(&spec->pcm_lock);
>  	if (spec->dyn_pin_out) {
>  		pinctl = snd_hda_codec_read(codec, pin_nid, 0,
>  					    AC_VERB_GET_PIN_WIDGET_CONTROL, 0);
> @@ -1847,7 +1977,10 @@ static int generic_hdmi_playback_pcm_prepare(struct hda_pcm_stream *hinfo,
>  				    pinctl | PIN_OUT);
>  	}
>  
> -	return spec->ops.setup_stream(codec, cvt_nid, pin_nid, stream_tag, format);
> +	err = spec->ops.setup_stream(codec, cvt_nid, pin_nid,
> +				 stream_tag, format);
> +	mutex_unlock(&spec->pcm_lock);
> +	return err;
>  }
>  
>  static int generic_hdmi_playback_pcm_cleanup(struct hda_pcm_stream *hinfo,
> @@ -1878,9 +2011,17 @@ static int hdmi_pcm_close(struct hda_pcm_stream *hinfo,
>  		per_cvt->assigned = 0;
>  		hinfo->nid = 0;
>  
> +		mutex_lock(&spec->pcm_lock);
>  		pin_idx = hinfo_to_pin_index(codec, hinfo);
> -		if (snd_BUG_ON(pin_idx < 0))
> +		if (spec->dyn_pcm_assign && pin_idx < 0) {
> +			mutex_unlock(&spec->pcm_lock);
> +			return 0;
> +		}
> +
> +		if (snd_BUG_ON(pin_idx < 0)) {
> +			mutex_unlock(&spec->pcm_lock);
>  			return -EINVAL;
> +		}
>  		per_pin = get_pin(spec, pin_idx);
>  
>  		if (spec->dyn_pin_out) {
> @@ -1900,6 +2041,7 @@ static int hdmi_pcm_close(struct hda_pcm_stream *hinfo,
>  		per_pin->setup = false;
>  		per_pin->channels = 0;
>  		mutex_unlock(&per_pin->lock);
> +		mutex_unlock(&spec->pcm_lock);
>  	}
>  
>  	return 0;
> @@ -2370,6 +2512,7 @@ static int patch_generic_hdmi(struct hda_codec *codec)
>  		return -ENOMEM;
>  
>  	spec->ops = generic_standard_hdmi_ops;
> +	mutex_init(&spec->pcm_lock);
>  	codec->spec = spec;
>  	hdmi_array_init(spec, 4);
>  
> -- 
> 1.9.1
> 

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [RFC V2 PATCH 1/5] ALSA: hda - hdmi playback without monitor in dynamic pcm bind mode
  2015-11-23 16:01   ` Takashi Iwai
@ 2015-11-25  6:50     ` Yang, Libin
  0 siblings, 0 replies; 13+ messages in thread
From: Yang, Libin @ 2015-11-25  6:50 UTC (permalink / raw)
  To: Takashi Iwai, libin.yang; +Cc: alsa-devel, mengdong.lin



> -----Original Message-----
> From: Takashi Iwai [mailto:tiwai@suse.de]
> Sent: Tuesday, November 24, 2015 12:01 AM
> To: libin.yang@linux.intel.com
> Cc: alsa-devel@alsa-project.org; mengdong.lin@linux.intel.com; Yang,
> Libin
> Subject: Re: [alsa-devel] [RFC V2 PATCH 1/5] ALSA: hda - hdmi playback
> without monitor in dynamic pcm bind mode
> 
> On Mon, 23 Nov 2015 15:09:30 +0100,
> libin.yang@linux.intel.com wrote:
> >
> > From: Libin Yang <libin.yang@linux.intel.com>
> >
> > Pulseaudio requires open pcm successfully when probing.
> >
> > This patch handles playback without monitor in dynamic pcm
> assignment
> > mode. It tries to open/prepare/close pcm successfully even there is
> > no pin bound to the PCM. On the meantime, it will try to find a proper
> > converter for the PCM.
> 
> The explanation why you added pcm_lock is missing.

Get it. I will add the explanation for pcm_lock.

Regards,
Libin

> 
> 
> Takashi
> 
> >
> > Signed-off-by: Libin Yang <libin.yang@linux.intel.com>
> > ---
> >  sound/pci/hda/patch_hdmi.c | 171
> +++++++++++++++++++++++++++++++++++++++++----
> >  1 file changed, 157 insertions(+), 14 deletions(-)
> >
> > diff --git a/sound/pci/hda/patch_hdmi.c
> b/sound/pci/hda/patch_hdmi.c
> > index 92e05fb..d0294bf 100644
> > --- a/sound/pci/hda/patch_hdmi.c
> > +++ b/sound/pci/hda/patch_hdmi.c
> > @@ -135,6 +135,7 @@ struct hdmi_spec {
> >  	int num_pins;
> >  	struct snd_array pins; /* struct hdmi_spec_per_pin */
> >  	struct hda_pcm *pcm_rec[16];
> > +	struct mutex pcm_lock;
> >  	unsigned int channels_max; /* max over all cvts */
> >
> >  	struct hdmi_eld temp_eld;
> > @@ -1331,6 +1332,11 @@ static int hdmi_setup_stream(struct
> hda_codec *codec, hda_nid_t cvt_nid,
> >  	return 0;
> >  }
> >
> > +/* Try to find an available converter
> > + * If pin_idx is less then zero, just try to find an available converter.
> > + * Otherwise, try to find an available converter and get the cvt mux
> index
> > + * of the pin.
> > + */
> >  static int hdmi_choose_cvt(struct hda_codec *codec,
> >  			int pin_idx, int *cvt_id, int *mux_id)
> >  {
> > @@ -1339,7 +1345,11 @@ static int hdmi_choose_cvt(struct
> hda_codec *codec,
> >  	struct hdmi_spec_per_cvt *per_cvt = NULL;
> >  	int cvt_idx, mux_idx = 0;
> >
> > -	per_pin = get_pin(spec, pin_idx);
> > +	/* pin_idx < 0 means no pin will be bound to the converter */
> > +	if (pin_idx < 0)
> > +		per_pin = NULL;
> > +	else
> > +		per_pin = get_pin(spec, pin_idx);
> >
> >  	/* Dynamically assign converter to stream */
> >  	for (cvt_idx = 0; cvt_idx < spec->num_cvts; cvt_idx++) {
> > @@ -1348,6 +1358,8 @@ static int hdmi_choose_cvt(struct
> hda_codec *codec,
> >  		/* Must not already be assigned */
> >  		if (per_cvt->assigned)
> >  			continue;
> > +		if (per_pin == NULL)
> > +			break;
> >  		/* Must be in pin's mux's list of converters */
> >  		for (mux_idx = 0; mux_idx < per_pin->num_mux_nids;
> mux_idx++)
> >  			if (per_pin->mux_nids[mux_idx] == per_cvt-
> >cvt_nid)
> > @@ -1360,9 +1372,10 @@ static int hdmi_choose_cvt(struct
> hda_codec *codec,
> >
> >  	/* No free converters */
> >  	if (cvt_idx == spec->num_cvts)
> > -		return -ENODEV;
> > +		return -EBUSY;
> >
> > -	per_pin->mux_idx = mux_idx;
> > +	if (per_pin != NULL)
> > +		per_pin->mux_idx = mux_idx;
> >
> >  	if (cvt_id)
> >  		*cvt_id = cvt_idx;
> > @@ -1388,6 +1401,20 @@ static void
> intel_verify_pin_cvt_connect(struct hda_codec *codec,
> >  					    mux_idx);
> >  }
> >
> > +/* get the mux index for the converter of the pins
> > + * converter's mux index is the same for all pins on Intel platform
> > + */
> > +static int intel_cvt_id_to_mux_idx(struct hdmi_spec *spec,
> > +			hda_nid_t cvt_nid)
> > +{
> > +	int i;
> > +
> > +	for (i = 0; i < spec->num_cvts; i++)
> > +		if (spec->cvt_nids[i] == cvt_nid)
> > +			return i;
> > +	return -EINVAL;
> > +}
> > +
> >  /* Intel HDMI workaround to fix audio routing issue:
> >   * For some Intel display codecs, pins share the same connection list.
> >   * So a conveter can be selected by multiple pins and playback on any
> of these
> > @@ -1439,6 +1466,69 @@ static void
> intel_not_share_assigned_cvt(struct hda_codec *codec,
> >  	}
> >  }
> >
> > +/* A wrapper of intel_not_share_asigned_cvt() */
> > +static void intel_not_share_assigned_cvt_nid(struct hda_codec
> *codec,
> > +			hda_nid_t pin_nid, hda_nid_t cvt_nid)
> > +{
> > +	int mux_idx;
> > +	struct hdmi_spec *spec = codec->spec;
> > +
> > +	if (!is_haswell_plus(codec) && !is_valleyview_plus(codec))
> > +		return;
> > +
> > +	/* On Intel platform, the mapping of converter nid to
> > +	 * mux index of the pins are always the same.
> > +	 * The pin nid may be 0, this means all pins will not
> > +	 * share the converter.
> > +	 */
> > +	mux_idx = intel_cvt_id_to_mux_idx(spec, cvt_nid);
> > +	if (mux_idx > 0)
> > +		intel_not_share_assigned_cvt(codec, pin_nid, mux_idx);
> > +}
> > +
> > +/* called in hdmi_pcm_open when no pin is assigned to the PCM
> > + * in dyn_pcm_assign mode.
> > + */
> > +static int hdmi_pcm_open_no_pin(struct hda_pcm_stream *hinfo,
> > +			 struct hda_codec *codec,
> > +			 struct snd_pcm_substream *substream)
> > +{
> > +	struct hdmi_spec *spec = codec->spec;
> > +	struct snd_pcm_runtime *runtime = substream->runtime;
> > +	int cvt_idx;
> > +	struct hdmi_spec_per_cvt *per_cvt = NULL;
> > +	int err;
> > +
> > +	err = hdmi_choose_cvt(codec, -1, &cvt_idx, NULL);
> > +	if (err)
> > +		return err;
> > +
> > +	per_cvt = get_cvt(spec, cvt_idx);
> > +	per_cvt->assigned = 1;
> > +	hinfo->nid = per_cvt->cvt_nid;
> > +
> > +	intel_not_share_assigned_cvt_nid(codec, 0, per_cvt->cvt_nid);
> > +
> > +	/* todo: setup spdif ctls assign */
> > +
> > +	/* Initially set the converter's capabilities */
> > +	hinfo->channels_min = per_cvt->channels_min;
> > +	hinfo->channels_max = per_cvt->channels_max;
> > +	hinfo->rates = per_cvt->rates;
> > +	hinfo->formats = per_cvt->formats;
> > +	hinfo->maxbps = per_cvt->maxbps;
> > +
> > +	/* Store the updated parameters */
> > +	runtime->hw.channels_min = hinfo->channels_min;
> > +	runtime->hw.channels_max = hinfo->channels_max;
> > +	runtime->hw.formats = hinfo->formats;
> > +	runtime->hw.rates = hinfo->rates;
> > +
> > +	snd_pcm_hw_constraint_step(substream->runtime, 0,
> > +
> SNDRV_PCM_HW_PARAM_CHANNELS, 2);
> > +	return 0;
> > +}
> > +
> >  /*
> >   * HDA PCM callbacks
> >   */
> > @@ -1455,19 +1545,36 @@ static int hdmi_pcm_open(struct
> hda_pcm_stream *hinfo,
> >  	int err;
> >
> >  	/* Validate hinfo */
> > +	mutex_lock(&spec->pcm_lock);
> >  	pin_idx = hinfo_to_pin_index(codec, hinfo);
> > -	if (snd_BUG_ON(pin_idx < 0))
> > -		return -EINVAL;
> > -	per_pin = get_pin(spec, pin_idx);
> > -	eld = &per_pin->sink_eld;
> > +	if (!spec->dyn_pcm_assign) {
> > +		if (snd_BUG_ON(pin_idx < 0)) {
> > +			mutex_unlock(&spec->pcm_lock);
> > +			return -EINVAL;
> > +		}
> > +	} else {
> > +		/* no pin is assigned to the PCM
> > +		 * PA need pcm open successfully when probe
> > +		 */
> > +		if (pin_idx < 0) {
> > +			err = hdmi_pcm_open_no_pin(hinfo, codec,
> substream);
> > +			mutex_unlock(&spec->pcm_lock);
> > +			return err;
> > +		}
> > +	}
> >
> >  	err = hdmi_choose_cvt(codec, pin_idx, &cvt_idx, &mux_idx);
> > -	if (err < 0)
> > +	if (err < 0) {
> > +		mutex_unlock(&spec->pcm_lock);
> >  		return err;
> > +	}
> >
> >  	per_cvt = get_cvt(spec, cvt_idx);
> >  	/* Claim converter */
> >  	per_cvt->assigned = 1;
> > +
> > +
> > +	per_pin = get_pin(spec, pin_idx);
> >  	per_pin->cvt_nid = per_cvt->cvt_nid;
> >  	hinfo->nid = per_cvt->cvt_nid;
> >
> > @@ -1488,6 +1595,7 @@ static int hdmi_pcm_open(struct
> hda_pcm_stream *hinfo,
> >  	hinfo->formats = per_cvt->formats;
> >  	hinfo->maxbps = per_cvt->maxbps;
> >
> > +	eld = &per_pin->sink_eld;
> >  	/* Restrict capabilities by ELD if this isn't disabled */
> >  	if (!static_hdmi_pcm && eld->eld_valid) {
> >  		snd_hdmi_eld_update_pcm_info(&eld->info, hinfo);
> > @@ -1496,10 +1604,12 @@ static int hdmi_pcm_open(struct
> hda_pcm_stream *hinfo,
> >  			per_cvt->assigned = 0;
> >  			hinfo->nid = 0;
> >  			snd_hda_spdif_ctls_unassign(codec, pin_idx);
> > +			mutex_unlock(&spec->pcm_lock);
> >  			return -ENODEV;
> >  		}
> >  	}
> >
> > +	mutex_unlock(&spec->pcm_lock);
> >  	/* Store the updated parameters */
> >  	runtime->hw.channels_min = hinfo->channels_min;
> >  	runtime->hw.channels_max = hinfo->channels_max;
> > @@ -1803,13 +1913,33 @@ static int
> generic_hdmi_playback_pcm_prepare(struct hda_pcm_stream *hinfo,
> >  {
> >  	hda_nid_t cvt_nid = hinfo->nid;
> >  	struct hdmi_spec *spec = codec->spec;
> > -	int pin_idx = hinfo_to_pin_index(codec, hinfo);
> > -	struct hdmi_spec_per_pin *per_pin = get_pin(spec, pin_idx);
> > -	hda_nid_t pin_nid = per_pin->pin_nid;
> > +	int pin_idx;
> > +	struct hdmi_spec_per_pin *per_pin;
> > +	hda_nid_t pin_nid;
> >  	struct snd_pcm_runtime *runtime = substream->runtime;
> >  	struct i915_audio_component *acomp = codec->bus-
> >core.audio_component;
> >  	bool non_pcm;
> >  	int pinctl;
> > +	int err;
> > +
> > +	mutex_lock(&spec->pcm_lock);
> > +	pin_idx = hinfo_to_pin_index(codec, hinfo);
> > +	if (spec->dyn_pcm_assign && pin_idx < 0) {
> > +		/* when dyn_pcm_assign and pcm is not bound to a pin
> > +		 * skip pin setup and return 0 to make audio playback
> > +		 * be ongoing
> > +		 */
> > +		intel_not_share_assigned_cvt_nid(codec, 0, cvt_nid);
> > +		snd_hda_codec_setup_stream(codec, cvt_nid,
> > +					stream_tag, 0, format);
> > +		mutex_unlock(&spec->pcm_lock);
> > +		return 0;
> > +	}
> > +
> > +	if (snd_BUG_ON(pin_idx < 0))
> > +		return -EINVAL;
> > +	per_pin = get_pin(spec, pin_idx);
> > +	pin_nid = per_pin->pin_nid;
> >
> >  	if (is_haswell_plus(codec) || is_valleyview_plus(codec)) {
> >  		/* Verify pin:cvt selections to avoid silent audio after S3.
> > @@ -1838,7 +1968,7 @@ static int
> generic_hdmi_playback_pcm_prepare(struct hda_pcm_stream *hinfo,
> >
> >  	hdmi_setup_audio_infoframe(codec, per_pin, non_pcm);
> >  	mutex_unlock(&per_pin->lock);
> > -
> > +	mutex_unlock(&spec->pcm_lock);
> >  	if (spec->dyn_pin_out) {
> >  		pinctl = snd_hda_codec_read(codec, pin_nid, 0,
> >
> AC_VERB_GET_PIN_WIDGET_CONTROL, 0);
> > @@ -1847,7 +1977,10 @@ static int
> generic_hdmi_playback_pcm_prepare(struct hda_pcm_stream *hinfo,
> >  				    pinctl | PIN_OUT);
> >  	}
> >
> > -	return spec->ops.setup_stream(codec, cvt_nid, pin_nid,
> stream_tag, format);
> > +	err = spec->ops.setup_stream(codec, cvt_nid, pin_nid,
> > +				 stream_tag, format);
> > +	mutex_unlock(&spec->pcm_lock);
> > +	return err;
> >  }
> >
> >  static int generic_hdmi_playback_pcm_cleanup(struct
> hda_pcm_stream *hinfo,
> > @@ -1878,9 +2011,17 @@ static int hdmi_pcm_close(struct
> hda_pcm_stream *hinfo,
> >  		per_cvt->assigned = 0;
> >  		hinfo->nid = 0;
> >
> > +		mutex_lock(&spec->pcm_lock);
> >  		pin_idx = hinfo_to_pin_index(codec, hinfo);
> > -		if (snd_BUG_ON(pin_idx < 0))
> > +		if (spec->dyn_pcm_assign && pin_idx < 0) {
> > +			mutex_unlock(&spec->pcm_lock);
> > +			return 0;
> > +		}
> > +
> > +		if (snd_BUG_ON(pin_idx < 0)) {
> > +			mutex_unlock(&spec->pcm_lock);
> >  			return -EINVAL;
> > +		}
> >  		per_pin = get_pin(spec, pin_idx);
> >
> >  		if (spec->dyn_pin_out) {
> > @@ -1900,6 +2041,7 @@ static int hdmi_pcm_close(struct
> hda_pcm_stream *hinfo,
> >  		per_pin->setup = false;
> >  		per_pin->channels = 0;
> >  		mutex_unlock(&per_pin->lock);
> > +		mutex_unlock(&spec->pcm_lock);
> >  	}
> >
> >  	return 0;
> > @@ -2370,6 +2512,7 @@ static int patch_generic_hdmi(struct
> hda_codec *codec)
> >  		return -ENOMEM;
> >
> >  	spec->ops = generic_standard_hdmi_ops;
> > +	mutex_init(&spec->pcm_lock);
> >  	codec->spec = spec;
> >  	hdmi_array_init(spec, 4);
> >
> > --
> > 1.9.1
> >

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [RFC V2 PATCH 4/5] ALSA: hda - add in_use flag in hda_pcm
  2015-11-23 15:50   ` Takashi Iwai
@ 2015-11-25  6:50     ` Yang, Libin
  0 siblings, 0 replies; 13+ messages in thread
From: Yang, Libin @ 2015-11-25  6:50 UTC (permalink / raw)
  To: Takashi Iwai, libin.yang; +Cc: alsa-devel, mengdong.lin


> -----Original Message-----
> From: Takashi Iwai [mailto:tiwai@suse.de]
> Sent: Monday, November 23, 2015 11:50 PM
> To: libin.yang@linux.intel.com
> Cc: alsa-devel@alsa-project.org; mengdong.lin@linux.intel.com; Yang,
> Libin
> Subject: Re: [alsa-devel] [RFC V2 PATCH 4/5] ALSA: hda - add in_use flag
> in hda_pcm
> 
> On Mon, 23 Nov 2015 15:09:33 +0100,
> libin.yang@linux.intel.com wrote:
> >
> > From: Libin Yang <libin.yang@linux.intel.com>
> >
> > Add in_use flag in struct hda_pcm for HD Audio dynamic PCM
> assignment.
> >
> > Signed-off-by: Libin Yang <libin.yang@linux.intel.com>
> 
> This should be folded into the last patch.  The addition of the flag
> alone doesn't make much sense.

Get it. Thanks.

Regards,
Libin

> 
> 
> Takashi
> 
> > ---
> >  sound/pci/hda/hda_codec.h | 1 +
> >  1 file changed, 1 insertion(+)
> >
> > diff --git a/sound/pci/hda/hda_codec.h b/sound/pci/hda/hda_codec.h
> > index 373fcad..ee97401 100644
> > --- a/sound/pci/hda/hda_codec.h
> > +++ b/sound/pci/hda/hda_codec.h
> > @@ -167,6 +167,7 @@ enum {
> >  /* for PCM creation */
> >  struct hda_pcm {
> >  	char *name;
> > +	bool in_use;
> >  	struct hda_pcm_stream stream[2];
> >  	unsigned int pcm_type;	/* HDA_PCM_TYPE_XXX */
> >  	int device;		/* device number to assign */
> > --
> > 1.9.1
> >

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [RFC V2 PATCH 3/5] ALSA: hda - hdmi dynamically bind PCM to pin when monitor hotplug
  2015-11-23 15:59   ` Takashi Iwai
@ 2015-11-25  6:53     ` Yang, Libin
  0 siblings, 0 replies; 13+ messages in thread
From: Yang, Libin @ 2015-11-25  6:53 UTC (permalink / raw)
  To: Takashi Iwai, libin.yang; +Cc: alsa-devel, mengdong.lin


> -----Original Message-----
> From: Takashi Iwai [mailto:tiwai@suse.de]
> Sent: Tuesday, November 24, 2015 12:00 AM
> To: libin.yang@linux.intel.com
> Cc: alsa-devel@alsa-project.org; mengdong.lin@linux.intel.com; Yang,
> Libin
> Subject: Re: [alsa-devel] [RFC V2 PATCH 3/5] ALSA: hda - hdmi
> dynamically bind PCM to pin when monitor hotplug
> 
> On Mon, 23 Nov 2015 15:09:32 +0100,
> libin.yang@linux.intel.com wrote:
> >
> > From: Libin Yang <libin.yang@linux.intel.com>
> >
> > Dynamically bind/unbind the PCM to pin when HDMI/DP monitor
> hotplug.
> >
> > When monitor is connected, find a proper PCM for the monitor.
> > When monitor is disconnected, unbind the PCM from the pin.
> 
> You must have some strategy about binding.  Please tell the story
> behind the scene, too.
> 
> >
> > Signed-off-by: Libin Yang <libin.yang@linux.intel.com>
> > ---
> >  sound/pci/hda/patch_hdmi.c | 82
> ++++++++++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 82 insertions(+)
> >
> > diff --git a/sound/pci/hda/patch_hdmi.c
> b/sound/pci/hda/patch_hdmi.c
> > index de22ac2..bcc2a90 100644
> > --- a/sound/pci/hda/patch_hdmi.c
> > +++ b/sound/pci/hda/patch_hdmi.c
> > @@ -72,6 +72,8 @@ struct hdmi_spec_per_cvt {
> >
> >  struct hdmi_spec_per_pin {
> >  	hda_nid_t pin_nid;
> > +	/* pin idx, different device entries on the same pin use the same
> idx */
> > +	int pin_nid_idx;
> >  	int num_mux_nids;
> >  	hda_nid_t mux_nids[HDA_MAX_CONNECTIONS];
> >  	int mux_idx;
> > @@ -83,6 +85,7 @@ struct hdmi_spec_per_pin {
> >  	struct delayed_work work;
> >  	struct snd_kcontrol *eld_ctl;
> >  	struct hda_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 */
> >  	int channels; /* current number of channels */
> > @@ -136,6 +139,7 @@ struct hdmi_spec {
> >  	struct snd_array pins; /* struct hdmi_spec_per_pin */
> >  	struct hda_pcm *pcm_rec[16];
> >  	struct mutex pcm_lock;
> > +	unsigned long pcm_bitmap;
> >  	int pcm_used;	/* counter of pcm_rec[] */
> >  	unsigned int channels_max; /* max over all cvts */
> >
> > @@ -1663,6 +1667,70 @@ static int hdmi_read_pin_conn(struct
> hda_codec *codec, int pin_idx)
> >  	return 0;
> >  }
> >
> > +static int get_preferred_pcm_slot(struct hdmi_spec *spec,
> > +				struct hdmi_spec_per_pin *per_pin)
> > +{
> > +	if ((spec->pcm_bitmap & (1 << per_pin->pin_nid_idx)) == 0)
> 
> Use test_bit().

Get it.

> 
> > +		return per_pin->pin_nid_idx;
> > +	return -1;
> > +}
> > +
> > +static int hdmi_find_pcm_slot(struct hdmi_spec *spec,
> > +				struct hdmi_spec_per_pin *per_pin)
> > +{
> > +	int slot;
> > +	int i;
> > +
> > +	/* try the prefer PCM */
> > +	slot = get_preferred_pcm_slot(spec, per_pin);
> > +	if (slot != -1)
> > +		return slot;
> > +
> > +	/* have a second try */
> > +	for (i = spec->num_pins; i < spec->pcm_used; i++) {
> > +		if ((spec->pcm_bitmap & (1 << i)) == 0)
> 
> Ditto.
> 
> > +			return i;
> > +	}
> > +
> > +	/* the last try */
> > +	for (i = 0; i < spec->pcm_used; i++) {
> > +		if ((spec->pcm_bitmap & (1 << i)) == 0)
> 
> Ditto.
> 
> > +			return i;
> > +	}
> > +	return -ENODEV;
> 
> I think you don't need to split get_preferred_pcm_slot().
> It's a function that is called only from here, and it's even better to
> be open-coded.  Then you can see the code flow more easily.
> (Also, you may notice that the last loop doesn't go all, but just up
>  to num_pins, by looking through the code closely.)
> 
> static int hdmi_find_pcm_slot(struct hdmi_spec *spec,
> 				struct hdmi_spec_per_pin *per_pin)
> {
> 	int i;
> 
> 	/* try the prefer PCM */
> 	if (!test_bit(per_pin->pin_nid_idx, &spec->pcm_bitmap))
> 		return per_pin->pin_nid_idx;
> 
> 	/* have a second try; check the "reserved area" over num_pins
> */
> 	for (i = spec->num_pins; i < spec->pcm_used; i++)
> 		if (!test_bit(i, &spec->pcm_bitmap))
> 			return i;
> 
> 	/* the last try; check the empty slots in pins */
> 	for (i = 0; i < spec->num_pins; i++)
> 		if (!test_bit(i, &spec->pcm_bitmap))
> 			return i;
> 
> 	return -EBUSY;
> }

OK. I will use the new code.

Regards,
Libin


> 
> 
> 
> Takashi

^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2015-11-25  6:53 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-11-23 14:09 [RFC V2 PATCH 0/5] hdmi audio dynamically bind PCM to pin libin.yang
2015-11-23 14:09 ` libin.yang
2015-11-23 14:09 ` [RFC V2 PATCH 1/5] ALSA: hda - hdmi playback without monitor in dynamic pcm bind mode libin.yang
2015-11-23 16:01   ` Takashi Iwai
2015-11-25  6:50     ` Yang, Libin
2015-11-23 14:09 ` [RFC V2 PATCH 2/5] ALSA: hda - hdmi operate spdif based on pcm libin.yang
2015-11-23 14:09 ` [RFC V2 PATCH 3/5] ALSA: hda - hdmi dynamically bind PCM to pin when monitor hotplug libin.yang
2015-11-23 15:59   ` Takashi Iwai
2015-11-25  6:53     ` Yang, Libin
2015-11-23 14:09 ` [RFC V2 PATCH 4/5] ALSA: hda - add in_use flag in hda_pcm libin.yang
2015-11-23 15:50   ` Takashi Iwai
2015-11-25  6:50     ` Yang, Libin
2015-11-23 14:09 ` [RFC V2 PATCH 5/5] ALSA: hda - hdmi setup pin when monitor hotplug in pcm dynamic assignment mode libin.yang

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.