[PATCH v3 00/15] ASoC: hdac_hdmi: Add DP & notification support

[PATCH v3 00/15] ASoC: hdac_hdmi: Add DP & notification support

[Subhransu S. Prusty]

This patch series adds DP audio and hotplug notification support.

On Skylake two DP ports are available and to enable DP on both
ports all pins need to be enabled.

There is a special vendor widget which need to be programmed to
enable all pins and converters. This series adds hotplug
notification, read/set constraint based on ELD, enable all
pin/cvts, DP1.2, programs the audio infoframe for DP. There is a
one to one mapping between converter and stream, so the dais are
created based on the no of streams supported on hdmi codec.  Even
though cvts can be mapped dynamically to the streams, currently
it is statically mapped as simultaneous playback on both DP and
HDMI is not supported as of now.

Pin muxes and controls are created dynamically to map converter
to pin widget. So at run time specific pin is mapped to the dai
based on the control selected (based on the display type DP/HDMI
connected).

Finally the DP audio infoframe programming is added to support
the DP feature. 

Also with hotplug notification support, ELD is read and
capabilities are set for rate, formats and channels. drm_eld
sound/core framework is updated to limit the formats based on
ELD.

There are few fixes one fixing the static checker warning and
other one not to fail if no connection list is found for a pin
widget.

Pls note, the 10th patch is adding a small macro for getting
connection type in drm header, we have CCed drm folks on that and
this one. Pls ack so that we can have this series merged thru
sound trees

changes in v3:
	- Addressed review comments
	- Use of list_for_each_entry_safe for node deletion.
	- Moved the format constraint based on ELD to driver
	- Fix constant as per kernel doc for connection type in
	  drm helper
	- Fix dp infoframe
	- Add hotplug event reporting to user space
	- Move device configuration from dai startup to hw_params
	  as the user space can keep the device open always.

Jeeja KP (2):
  ASoC: hdac_hdmi: Add codec suspend/resume handler
  ASoC: hdac_hdmi: Fix to enable device configuration in hw_params

Ramesh Babu (1):
  ASoC: hdac_hdmi: Fix to keep display active while enumerating codec

Subhransu S. Prusty (12):
  ASoC: hdac_hdmi: Fix to check num nodes correctly
  ASoC: hdac_hdmi: Fix to warn instead of err for no connected nids
  ASoC: hdac_hdmi: Use list to add pins and converters
  ASoC: hdac_hdmi: Add hotplug notification and read eld
  ASoC: hdac_hdmi: Apply constraints based on ELD
  ASoC: hdac_hdmi: Enable DP1.2 and all converters/pins
  ASoC: hdac_hdmi: create dais based on number of streams
  ASoC: hdac_hdmi: Create widget/route based on nodes enumerated
  ASoC: hdac_hdmi: Assign pin for stream based on dapm connection
  drm/edid: Add API to help find connection type
  ASoC: hdac_hdmi: Add infoframe support for dp audio
  ASoC: hdac_hdmi: Add jack reporting for user space

 include/drm/drm_edid.h       |   12 +
 sound/soc/codecs/Kconfig     |    1 +
 sound/soc/codecs/hdac_hdmi.c | 1101 +++++++++++++++++++++++++++++++++++-------
 3 files changed, 936 insertions(+), 178 deletions(-)

-- 
1.9.1

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v3 00/15] ASoC: hdac_hdmi: Add DP & notification support

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 401 bytes --]

On Tue, Dec 08, 2015 at 02:47:58AM +0530, Subhransu S. Prusty wrote:
> This patch series adds DP audio and hotplug notification support.

Not related to the code but there seems to be something wrong with the
time configuration on your system which is causing all your patches to
be submitted quite a few hours in the future which confuses time based
mailbox sorting - might be worth looking at that.

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

Re: [PATCH v3 00/15] ASoC: hdac_hdmi: Add DP & notification support

[Subhransu S. Prusty]

On Mon, Dec 07, 2015 at 08:12:00PM +0000, Mark Brown wrote:
> On Tue, Dec 08, 2015 at 02:47:58AM +0530, Subhransu S. Prusty wrote:
> > This patch series adds DP audio and hotplug notification support.
> 
> Not related to the code but there seems to be something wrong with the
> time configuration on your system which is causing all your patches to
> be submitted quite a few hours in the future which confuses time based
> mailbox sorting - might be worth looking at that.

Sorry about that. Will fix it.



-- 
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v3 01/15] ASoC: hdac_hdmi: Fix to check num nodes correctly

[Subhransu S. Prusty]

commit 3c83ac23253c ("ASoC: hdac_hdmi: check error return") fixes
the static checker warning reported by Dan Carpenter:

	sound/soc/codecs/hdac_hdmi.c:416 hdac_hdmi_parse_and_map_nid()
	warn: unsigned 'hdac->num_nodes' is never less than zero.

But it doesn't fix the issue completely.

It's also a failure if no sub nodes found for an afg node. So modify
the return condition appropriately.

Signed-off-by: Subhransu S. Prusty <subhransu.s.prusty@intel.com>
Signed-off-by: Vinod Koul <vinod.koul@intel.com>
---
 sound/soc/codecs/hdac_hdmi.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/sound/soc/codecs/hdac_hdmi.c b/sound/soc/codecs/hdac_hdmi.c
index 1a2f33b..929f27b 100644
--- a/sound/soc/codecs/hdac_hdmi.c
+++ b/sound/soc/codecs/hdac_hdmi.c
@@ -415,7 +415,7 @@ static int hdac_hdmi_parse_and_map_nid(struct hdac_ext_device *edev)
 	int cvt_nid = 0, pin_nid = 0;
 
 	num_nodes = snd_hdac_get_sub_nodes(hdac, hdac->afg, &nid);
-	if (!nid || num_nodes < 0) {
+	if (!nid || num_nodes <= 0) {
 		dev_warn(&hdac->dev, "HDMI: failed to get afg sub nodes\n");
 		return -EINVAL;
 	}
-- 
1.9.1

[PATCH v3 02/15] ASoC: hdac_hdmi: Fix to warn instead of err for no connected nids

[Subhransu S. Prusty]

It is possible that some pin widget may return with no converter
connected. So don't throw error if none are found to be connected.
Instead print a warning and continue.

Signed-off-by: Subhransu S. Prusty <subhransu.s.prusty@intel.com>
Signed-off-by: Vinod Koul <vinod.koul@intel.com>
---
 sound/soc/codecs/hdac_hdmi.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/sound/soc/codecs/hdac_hdmi.c b/sound/soc/codecs/hdac_hdmi.c
index 929f27b..429fa14 100644
--- a/sound/soc/codecs/hdac_hdmi.c
+++ b/sound/soc/codecs/hdac_hdmi.c
@@ -316,10 +316,12 @@ static int hdac_hdmi_query_pin_connlist(struct hdac_ext_device *hdac,
 
 	pin->num_mux_nids = snd_hdac_get_connections(&hdac->hdac, pin->nid,
 			pin->mux_nids, HDA_MAX_CONNECTIONS);
-	if (pin->num_mux_nids == 0) {
-		dev_err(&hdac->hdac.dev, "No connections found\n");
-		return -ENODEV;
-	}
+	if (pin->num_mux_nids == 0)
+		dev_warn(&hdac->hdac.dev, "No connections found for pin: %d\n",
+								pin->nid);
+
+	dev_dbg(&hdac->hdac.dev, "num_mux_nids %d for pin: %d\n",
+			pin->num_mux_nids, pin->nid);
 
 	return pin->num_mux_nids;
 }
-- 
1.9.1

[PATCH v3 03/15] ASoC: hdac_hdmi: Use list to add pins and converters

[Subhransu S. Prusty]

Future platforms may have a different set of pins/converters.
So use lists to add pins and converters based on enumeration.

Also it may be required to connect any converter to any pin
dynamically as per different use cases (for example DP is
connected to pin 6 on skylake board). So this will help in
dynamically select and route.

Fix the dai map as well to use the pin/cvt from list. Not
enabling all dai maps for now.

Signed-off-by: Subhransu S. Prusty <subhransu.s.prusty@intel.com>
Signed-off-by: Vinod Koul <vinod.koul@intel.com>
---
 sound/soc/codecs/hdac_hdmi.c | 154 +++++++++++++++++++++++++++++--------------
 1 file changed, 106 insertions(+), 48 deletions(-)

diff --git a/sound/soc/codecs/hdac_hdmi.c b/sound/soc/codecs/hdac_hdmi.c
index 429fa14..b206d5e 100644
--- a/sound/soc/codecs/hdac_hdmi.c
+++ b/sound/soc/codecs/hdac_hdmi.c
@@ -43,11 +43,13 @@ struct hdac_hdmi_cvt_params {
 };
 
 struct hdac_hdmi_cvt {
+	struct list_head head;
 	hda_nid_t nid;
 	struct hdac_hdmi_cvt_params params;
 };
 
 struct hdac_hdmi_pin {
+	struct list_head head;
 	hda_nid_t nid;
 	int num_mux_nids;
 	hda_nid_t mux_nids[HDA_MAX_CONNECTIONS];
@@ -55,14 +57,16 @@ struct hdac_hdmi_pin {
 
 struct hdac_hdmi_dai_pin_map {
 	int dai_id;
-	struct hdac_hdmi_pin pin;
-	struct hdac_hdmi_cvt cvt;
+	struct hdac_hdmi_pin *pin;
+	struct hdac_hdmi_cvt *cvt;
 };
 
 struct hdac_hdmi_priv {
-	hda_nid_t pin_nid[3];
-	hda_nid_t cvt_nid[3];
 	struct hdac_hdmi_dai_pin_map dai_map[3];
+	struct list_head pin_list;
+	struct list_head cvt_list;
+	int num_pin;
+	int num_cvt;
 };
 
 static inline struct hdac_ext_device *to_hda_ext_device(struct device *dev)
@@ -149,13 +153,15 @@ static void hdac_hdmi_set_power_state(struct hdac_ext_device *edev,
 		struct hdac_hdmi_dai_pin_map *dai_map, unsigned int pwr_state)
 {
 	/* Power up pin widget */
-	if (!snd_hdac_check_power_state(&edev->hdac, dai_map->pin.nid, pwr_state))
-		snd_hdac_codec_write(&edev->hdac, dai_map->pin.nid, 0,
+	if (!snd_hdac_check_power_state(&edev->hdac, dai_map->pin->nid,
+						pwr_state))
+		snd_hdac_codec_write(&edev->hdac, dai_map->pin->nid, 0,
 			AC_VERB_SET_POWER_STATE, pwr_state);
 
 	/* Power up converter */
-	if (!snd_hdac_check_power_state(&edev->hdac, dai_map->cvt.nid, pwr_state))
-		snd_hdac_codec_write(&edev->hdac, dai_map->cvt.nid, 0,
+	if (!snd_hdac_check_power_state(&edev->hdac, dai_map->cvt->nid,
+						pwr_state))
+		snd_hdac_codec_write(&edev->hdac, dai_map->cvt->nid, 0,
 			AC_VERB_SET_POWER_STATE, pwr_state);
 }
 
@@ -179,13 +185,13 @@ static int hdac_hdmi_playback_prepare(struct snd_pcm_substream *substream,
 	dev_dbg(&hdac->hdac.dev, "stream tag from cpu dai %d format in cvt 0x%x\n",
 			dd->stream_tag,	dd->format);
 
-	ret = hdac_hdmi_setup_audio_infoframe(hdac, dai_map->cvt.nid,
-						dai_map->pin.nid);
+	ret = hdac_hdmi_setup_audio_infoframe(hdac, dai_map->cvt->nid,
+						dai_map->pin->nid);
 	if (ret < 0)
 		return ret;
 
-	return hdac_hdmi_setup_stream(hdac, dai_map->cvt.nid, dai_map->pin.nid,
-					dd->stream_tag, dd->format);
+	return hdac_hdmi_setup_stream(hdac, dai_map->cvt->nid,
+			dai_map->pin->nid, dd->stream_tag, dd->format);
 }
 
 static int hdac_hdmi_set_hw_params(struct snd_pcm_substream *substream,
@@ -221,9 +227,9 @@ static int hdac_hdmi_playback_cleanup(struct snd_pcm_substream *substream,
 
 	dai_map = &hdmi->dai_map[dai->id];
 
-	snd_hdac_codec_write(&edev->hdac, dai_map->cvt.nid, 0,
+	snd_hdac_codec_write(&edev->hdac, dai_map->cvt->nid, 0,
 				AC_VERB_SET_CHANNEL_STREAMID, 0);
-	snd_hdac_codec_write(&edev->hdac, dai_map->cvt.nid, 0,
+	snd_hdac_codec_write(&edev->hdac, dai_map->cvt->nid, 0,
 				AC_VERB_SET_STREAM_FORMAT, 0);
 
 	dd = (struct hdac_ext_dma_params *)snd_soc_dai_get_dma_data(dai, substream);
@@ -249,7 +255,7 @@ static int hdac_hdmi_pcm_open(struct snd_pcm_substream *substream,
 
 	dai_map = &hdmi->dai_map[dai->id];
 
-	val = snd_hdac_codec_read(&hdac->hdac, dai_map->pin.nid, 0,
+	val = snd_hdac_codec_read(&hdac->hdac, dai_map->pin->nid, 0,
 					AC_VERB_GET_PIN_SENSE, 0);
 	dev_info(&hdac->hdac.dev, "Val for AC_VERB_GET_PIN_SENSE: %x\n", val);
 
@@ -260,7 +266,7 @@ static int hdac_hdmi_pcm_open(struct snd_pcm_substream *substream,
 
 	hdac_hdmi_set_power_state(hdac, dai_map, AC_PWRST_D0);
 
-	snd_hdac_codec_write(&hdac->hdac, dai_map->pin.nid, 0,
+	snd_hdac_codec_write(&hdac->hdac, dai_map->pin->nid, 0,
 			AC_VERB_SET_AMP_GAIN_MUTE, AMP_OUT_UNMUTE);
 
 	snd_pcm_hw_constraint_step(substream->runtime, 0,
@@ -280,7 +286,7 @@ static void hdac_hdmi_pcm_close(struct snd_pcm_substream *substream,
 
 	hdac_hdmi_set_power_state(hdac, dai_map, AC_PWRST_D3);
 
-	snd_hdac_codec_write(&hdac->hdac, dai_map->pin.nid, 0,
+	snd_hdac_codec_write(&hdac->hdac, dai_map->pin->nid, 0,
 			AC_VERB_SET_AMP_GAIN_MUTE, AMP_OUT_MUTE);
 }
 
@@ -368,40 +374,79 @@ static void create_fill_widget_route_map(struct snd_soc_dapm_context *dapm,
 	snd_soc_dapm_add_routes(dapm, route, ARRAY_SIZE(route));
 }
 
-static int hdac_hdmi_init_dai_map(struct hdac_ext_device *edev,
-			struct hdac_hdmi_dai_pin_map *dai_map,
-			hda_nid_t pin_nid, hda_nid_t cvt_nid, int dai_id)
+static int hdac_hdmi_init_dai_map(struct hdac_ext_device *edev)
 {
-	int ret;
+	struct hdac_hdmi_priv *hdmi = edev->private_data;
+	struct hdac_hdmi_dai_pin_map *dai_map = &hdmi->dai_map[0];
+	struct hdac_hdmi_cvt *cvt;
+	struct hdac_hdmi_pin *pin;
 
-	dai_map->dai_id = dai_id;
-	dai_map->pin.nid = pin_nid;
+	if (list_empty(&hdmi->cvt_list) || list_empty(&hdmi->pin_list))
+		return -EINVAL;
 
-	ret = hdac_hdmi_query_pin_connlist(edev, &dai_map->pin);
-	if (ret < 0) {
-		dev_err(&edev->hdac.dev,
-			"Error querying connection list: %d\n", ret);
-		return ret;
-	}
+	/*
+	 * Currently on board only 1 pin and 1 converter is enabled for
+	 * simplification, more will be added eventually
+	 * So using fixed map for dai_id:pin:cvt
+	 */
+	cvt = list_first_entry(&hdmi->cvt_list, struct hdac_hdmi_cvt, head);
+	pin = list_first_entry(&hdmi->pin_list, struct hdac_hdmi_pin, head);
+
+	dai_map->dai_id = 0;
+	dai_map->pin = pin;
 
-	dai_map->cvt.nid = cvt_nid;
+	dai_map->cvt = cvt;
 
 	/* Enable out path for this pin widget */
-	snd_hdac_codec_write(&edev->hdac, pin_nid, 0,
+	snd_hdac_codec_write(&edev->hdac, pin->nid, 0,
 			AC_VERB_SET_PIN_WIDGET_CONTROL, PIN_OUT);
 
 	/* Enable transmission */
-	snd_hdac_codec_write(&edev->hdac, cvt_nid, 0,
+	snd_hdac_codec_write(&edev->hdac, cvt->nid, 0,
 			AC_VERB_SET_DIGI_CONVERT_1, 1);
 
 	/* Category Code (CC) to zero */
-	snd_hdac_codec_write(&edev->hdac, cvt_nid, 0,
+	snd_hdac_codec_write(&edev->hdac, cvt->nid, 0,
 			AC_VERB_SET_DIGI_CONVERT_2, 0);
 
-	snd_hdac_codec_write(&edev->hdac, pin_nid, 0,
+	snd_hdac_codec_write(&edev->hdac, pin->nid, 0,
 			AC_VERB_SET_CONNECT_SEL, 0);
 
-	return hdac_hdmi_query_cvt_params(&edev->hdac, &dai_map->cvt);
+	return 0;
+}
+
+static int hdac_hdmi_add_cvt(struct hdac_ext_device *edev, hda_nid_t nid)
+{
+	struct hdac_hdmi_priv *hdmi = edev->private_data;
+	struct hdac_hdmi_cvt *cvt;
+
+	cvt = kzalloc(sizeof(*cvt), GFP_KERNEL);
+	if (!cvt)
+		return -ENOMEM;
+
+	cvt->nid = nid;
+
+	list_add_tail(&cvt->head, &hdmi->cvt_list);
+	hdmi->num_cvt++;
+
+	return hdac_hdmi_query_cvt_params(&edev->hdac, cvt);
+}
+
+static int hdac_hdmi_add_pin(struct hdac_ext_device *edev, hda_nid_t nid)
+{
+	struct hdac_hdmi_priv *hdmi = edev->private_data;
+	struct hdac_hdmi_pin *pin;
+
+	pin = kzalloc(sizeof(*pin), GFP_KERNEL);
+	if (!pin)
+		return -ENOMEM;
+
+	pin->nid = nid;
+
+	list_add_tail(&pin->head, &hdmi->pin_list);
+	hdmi->num_pin++;
+
+	return 0;
 }
 
 /*
@@ -414,7 +459,7 @@ static int hdac_hdmi_parse_and_map_nid(struct hdac_ext_device *edev)
 	int i, num_nodes;
 	struct hdac_device *hdac = &edev->hdac;
 	struct hdac_hdmi_priv *hdmi = edev->private_data;
-	int cvt_nid = 0, pin_nid = 0;
+	int ret;
 
 	num_nodes = snd_hdac_get_sub_nodes(hdac, hdac->afg, &nid);
 	if (!nid || num_nodes <= 0) {
@@ -438,29 +483,25 @@ static int hdac_hdmi_parse_and_map_nid(struct hdac_ext_device *edev)
 		switch (type) {
 
 		case AC_WID_AUD_OUT:
-			hdmi->cvt_nid[cvt_nid] = nid;
-			cvt_nid++;
+			ret = hdac_hdmi_add_cvt(edev, nid);
+			if (ret < 0)
+				return ret;
 			break;
 
 		case AC_WID_PIN:
-			hdmi->pin_nid[pin_nid] = nid;
-			pin_nid++;
+			ret = hdac_hdmi_add_pin(edev, nid);
+			if (ret < 0)
+				return ret;
 			break;
 		}
 	}
 
 	hdac->end_nid = nid;
 
-	if (!pin_nid || !cvt_nid)
+	if (!hdmi->num_pin || !hdmi->num_cvt)
 		return -EIO;
 
-	/*
-	 * Currently on board only 1 pin and 1 converter is enabled for
-	 * simplification, more will be added eventually
-	 * So using fixed map for dai_id:pin:cvt
-	 */
-	return hdac_hdmi_init_dai_map(edev, &hdmi->dai_map[0], hdmi->pin_nid[0],
-			hdmi->cvt_nid[0], 0);
+	return hdac_hdmi_init_dai_map(edev);
 }
 
 static int hdmi_codec_probe(struct snd_soc_codec *codec)
@@ -544,6 +585,9 @@ static int hdac_hdmi_dev_probe(struct hdac_ext_device *edev)
 
 	dev_set_drvdata(&codec->dev, edev);
 
+	INIT_LIST_HEAD(&hdmi_priv->pin_list);
+	INIT_LIST_HEAD(&hdmi_priv->cvt_list);
+
 	ret = hdac_hdmi_parse_and_map_nid(edev);
 	if (ret < 0)
 		return ret;
@@ -555,8 +599,22 @@ static int hdac_hdmi_dev_probe(struct hdac_ext_device *edev)
 
 static int hdac_hdmi_dev_remove(struct hdac_ext_device *edev)
 {
+	struct hdac_hdmi_priv *hdmi = edev->private_data;
+	struct hdac_hdmi_pin *pin, *pin_next;
+	struct hdac_hdmi_cvt *cvt, *cvt_next;
+
 	snd_soc_unregister_codec(&edev->hdac.dev);
 
+	list_for_each_entry_safe(cvt, cvt_next, &hdmi->cvt_list, head) {
+		list_del(&cvt->head);
+		kfree(cvt);
+	}
+
+	list_for_each_entry_safe(pin, pin_next, &hdmi->pin_list, head) {
+		list_del(&pin->head);
+		kfree(pin);
+	}
+
 	return 0;
 }
 
-- 
1.9.1

Applied "ASoC: hdac_hdmi: Use list to add pins and converters" to the asoc tree

[Mark Brown]

The patch

   ASoC: hdac_hdmi: Use list to add pins and converters

has been applied to the asoc tree at

   git://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git 

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.  

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark

>From 15b914476bf24185534a59fb8e149d465ff79c59 Mon Sep 17 00:00:00 2001
From: "Subhransu S. Prusty" <subhransu.s.prusty@intel.com>
Date: Wed, 9 Dec 2015 21:46:10 +0530
Subject: [PATCH] ASoC: hdac_hdmi: Use list to add pins and converters

Future platforms may have a different set of pins/converters.
So use lists to add pins and converters based on enumeration.

Also it may be required to connect any converter to any pin
dynamically as per different use cases (for example DP is
connected to pin 6 on skylake board). So this will help in
dynamically select and route.

Fix the dai map as well to use the pin/cvt from list. Not
enabling all dai maps for now.

Signed-off-by: Subhransu S. Prusty <subhransu.s.prusty@intel.com>
Signed-off-by: Vinod Koul <vinod.koul@intel.com>
Signed-off-by: Mark Brown <broonie@kernel.org>
---
 sound/soc/codecs/hdac_hdmi.c | 154 +++++++++++++++++++++++++++++--------------
 1 file changed, 106 insertions(+), 48 deletions(-)

diff --git a/sound/soc/codecs/hdac_hdmi.c b/sound/soc/codecs/hdac_hdmi.c
index 41117e130ce0..f5df7232405b 100644
--- a/sound/soc/codecs/hdac_hdmi.c
+++ b/sound/soc/codecs/hdac_hdmi.c
@@ -43,11 +43,13 @@ struct hdac_hdmi_cvt_params {
 };
 
 struct hdac_hdmi_cvt {
+	struct list_head head;
 	hda_nid_t nid;
 	struct hdac_hdmi_cvt_params params;
 };
 
 struct hdac_hdmi_pin {
+	struct list_head head;
 	hda_nid_t nid;
 	int num_mux_nids;
 	hda_nid_t mux_nids[HDA_MAX_CONNECTIONS];
@@ -55,14 +57,16 @@ struct hdac_hdmi_pin {
 
 struct hdac_hdmi_dai_pin_map {
 	int dai_id;
-	struct hdac_hdmi_pin pin;
-	struct hdac_hdmi_cvt cvt;
+	struct hdac_hdmi_pin *pin;
+	struct hdac_hdmi_cvt *cvt;
 };
 
 struct hdac_hdmi_priv {
-	hda_nid_t pin_nid[3];
-	hda_nid_t cvt_nid[3];
 	struct hdac_hdmi_dai_pin_map dai_map[3];
+	struct list_head pin_list;
+	struct list_head cvt_list;
+	int num_pin;
+	int num_cvt;
 };
 
 static inline struct hdac_ext_device *to_hda_ext_device(struct device *dev)
@@ -149,13 +153,15 @@ static void hdac_hdmi_set_power_state(struct hdac_ext_device *edev,
 		struct hdac_hdmi_dai_pin_map *dai_map, unsigned int pwr_state)
 {
 	/* Power up pin widget */
-	if (!snd_hdac_check_power_state(&edev->hdac, dai_map->pin.nid, pwr_state))
-		snd_hdac_codec_write(&edev->hdac, dai_map->pin.nid, 0,
+	if (!snd_hdac_check_power_state(&edev->hdac, dai_map->pin->nid,
+						pwr_state))
+		snd_hdac_codec_write(&edev->hdac, dai_map->pin->nid, 0,
 			AC_VERB_SET_POWER_STATE, pwr_state);
 
 	/* Power up converter */
-	if (!snd_hdac_check_power_state(&edev->hdac, dai_map->cvt.nid, pwr_state))
-		snd_hdac_codec_write(&edev->hdac, dai_map->cvt.nid, 0,
+	if (!snd_hdac_check_power_state(&edev->hdac, dai_map->cvt->nid,
+						pwr_state))
+		snd_hdac_codec_write(&edev->hdac, dai_map->cvt->nid, 0,
 			AC_VERB_SET_POWER_STATE, pwr_state);
 }
 
@@ -179,13 +185,13 @@ static int hdac_hdmi_playback_prepare(struct snd_pcm_substream *substream,
 	dev_dbg(&hdac->hdac.dev, "stream tag from cpu dai %d format in cvt 0x%x\n",
 			dd->stream_tag,	dd->format);
 
-	ret = hdac_hdmi_setup_audio_infoframe(hdac, dai_map->cvt.nid,
-						dai_map->pin.nid);
+	ret = hdac_hdmi_setup_audio_infoframe(hdac, dai_map->cvt->nid,
+						dai_map->pin->nid);
 	if (ret < 0)
 		return ret;
 
-	return hdac_hdmi_setup_stream(hdac, dai_map->cvt.nid, dai_map->pin.nid,
-					dd->stream_tag, dd->format);
+	return hdac_hdmi_setup_stream(hdac, dai_map->cvt->nid,
+			dai_map->pin->nid, dd->stream_tag, dd->format);
 }
 
 static int hdac_hdmi_set_hw_params(struct snd_pcm_substream *substream,
@@ -221,9 +227,9 @@ static int hdac_hdmi_playback_cleanup(struct snd_pcm_substream *substream,
 
 	dai_map = &hdmi->dai_map[dai->id];
 
-	snd_hdac_codec_write(&edev->hdac, dai_map->cvt.nid, 0,
+	snd_hdac_codec_write(&edev->hdac, dai_map->cvt->nid, 0,
 				AC_VERB_SET_CHANNEL_STREAMID, 0);
-	snd_hdac_codec_write(&edev->hdac, dai_map->cvt.nid, 0,
+	snd_hdac_codec_write(&edev->hdac, dai_map->cvt->nid, 0,
 				AC_VERB_SET_STREAM_FORMAT, 0);
 
 	dd = (struct hdac_ext_dma_params *)snd_soc_dai_get_dma_data(dai, substream);
@@ -249,7 +255,7 @@ static int hdac_hdmi_pcm_open(struct snd_pcm_substream *substream,
 
 	dai_map = &hdmi->dai_map[dai->id];
 
-	val = snd_hdac_codec_read(&hdac->hdac, dai_map->pin.nid, 0,
+	val = snd_hdac_codec_read(&hdac->hdac, dai_map->pin->nid, 0,
 					AC_VERB_GET_PIN_SENSE, 0);
 	dev_info(&hdac->hdac.dev, "Val for AC_VERB_GET_PIN_SENSE: %x\n", val);
 
@@ -260,7 +266,7 @@ static int hdac_hdmi_pcm_open(struct snd_pcm_substream *substream,
 
 	hdac_hdmi_set_power_state(hdac, dai_map, AC_PWRST_D0);
 
-	snd_hdac_codec_write(&hdac->hdac, dai_map->pin.nid, 0,
+	snd_hdac_codec_write(&hdac->hdac, dai_map->pin->nid, 0,
 			AC_VERB_SET_AMP_GAIN_MUTE, AMP_OUT_UNMUTE);
 
 	snd_pcm_hw_constraint_step(substream->runtime, 0,
@@ -280,7 +286,7 @@ static void hdac_hdmi_pcm_close(struct snd_pcm_substream *substream,
 
 	hdac_hdmi_set_power_state(hdac, dai_map, AC_PWRST_D3);
 
-	snd_hdac_codec_write(&hdac->hdac, dai_map->pin.nid, 0,
+	snd_hdac_codec_write(&hdac->hdac, dai_map->pin->nid, 0,
 			AC_VERB_SET_AMP_GAIN_MUTE, AMP_OUT_MUTE);
 }
 
@@ -368,40 +374,79 @@ static void create_fill_widget_route_map(struct snd_soc_dapm_context *dapm,
 	snd_soc_dapm_add_routes(dapm, route, ARRAY_SIZE(route));
 }
 
-static int hdac_hdmi_init_dai_map(struct hdac_ext_device *edev,
-			struct hdac_hdmi_dai_pin_map *dai_map,
-			hda_nid_t pin_nid, hda_nid_t cvt_nid, int dai_id)
+static int hdac_hdmi_init_dai_map(struct hdac_ext_device *edev)
 {
-	int ret;
+	struct hdac_hdmi_priv *hdmi = edev->private_data;
+	struct hdac_hdmi_dai_pin_map *dai_map = &hdmi->dai_map[0];
+	struct hdac_hdmi_cvt *cvt;
+	struct hdac_hdmi_pin *pin;
 
-	dai_map->dai_id = dai_id;
-	dai_map->pin.nid = pin_nid;
+	if (list_empty(&hdmi->cvt_list) || list_empty(&hdmi->pin_list))
+		return -EINVAL;
 
-	ret = hdac_hdmi_query_pin_connlist(edev, &dai_map->pin);
-	if (ret < 0) {
-		dev_err(&edev->hdac.dev,
-			"Error querying connection list: %d\n", ret);
-		return ret;
-	}
+	/*
+	 * Currently on board only 1 pin and 1 converter is enabled for
+	 * simplification, more will be added eventually
+	 * So using fixed map for dai_id:pin:cvt
+	 */
+	cvt = list_first_entry(&hdmi->cvt_list, struct hdac_hdmi_cvt, head);
+	pin = list_first_entry(&hdmi->pin_list, struct hdac_hdmi_pin, head);
+
+	dai_map->dai_id = 0;
+	dai_map->pin = pin;
 
-	dai_map->cvt.nid = cvt_nid;
+	dai_map->cvt = cvt;
 
 	/* Enable out path for this pin widget */
-	snd_hdac_codec_write(&edev->hdac, pin_nid, 0,
+	snd_hdac_codec_write(&edev->hdac, pin->nid, 0,
 			AC_VERB_SET_PIN_WIDGET_CONTROL, PIN_OUT);
 
 	/* Enable transmission */
-	snd_hdac_codec_write(&edev->hdac, cvt_nid, 0,
+	snd_hdac_codec_write(&edev->hdac, cvt->nid, 0,
 			AC_VERB_SET_DIGI_CONVERT_1, 1);
 
 	/* Category Code (CC) to zero */
-	snd_hdac_codec_write(&edev->hdac, cvt_nid, 0,
+	snd_hdac_codec_write(&edev->hdac, cvt->nid, 0,
 			AC_VERB_SET_DIGI_CONVERT_2, 0);
 
-	snd_hdac_codec_write(&edev->hdac, pin_nid, 0,
+	snd_hdac_codec_write(&edev->hdac, pin->nid, 0,
 			AC_VERB_SET_CONNECT_SEL, 0);
 
-	return hdac_hdmi_query_cvt_params(&edev->hdac, &dai_map->cvt);
+	return 0;
+}
+
+static int hdac_hdmi_add_cvt(struct hdac_ext_device *edev, hda_nid_t nid)
+{
+	struct hdac_hdmi_priv *hdmi = edev->private_data;
+	struct hdac_hdmi_cvt *cvt;
+
+	cvt = kzalloc(sizeof(*cvt), GFP_KERNEL);
+	if (!cvt)
+		return -ENOMEM;
+
+	cvt->nid = nid;
+
+	list_add_tail(&cvt->head, &hdmi->cvt_list);
+	hdmi->num_cvt++;
+
+	return hdac_hdmi_query_cvt_params(&edev->hdac, cvt);
+}
+
+static int hdac_hdmi_add_pin(struct hdac_ext_device *edev, hda_nid_t nid)
+{
+	struct hdac_hdmi_priv *hdmi = edev->private_data;
+	struct hdac_hdmi_pin *pin;
+
+	pin = kzalloc(sizeof(*pin), GFP_KERNEL);
+	if (!pin)
+		return -ENOMEM;
+
+	pin->nid = nid;
+
+	list_add_tail(&pin->head, &hdmi->pin_list);
+	hdmi->num_pin++;
+
+	return 0;
 }
 
 /*
@@ -414,7 +459,7 @@ static int hdac_hdmi_parse_and_map_nid(struct hdac_ext_device *edev)
 	int i, num_nodes;
 	struct hdac_device *hdac = &edev->hdac;
 	struct hdac_hdmi_priv *hdmi = edev->private_data;
-	int cvt_nid = 0, pin_nid = 0;
+	int ret;
 
 	num_nodes = snd_hdac_get_sub_nodes(hdac, hdac->afg, &nid);
 	if (!nid || num_nodes <= 0) {
@@ -438,29 +483,25 @@ static int hdac_hdmi_parse_and_map_nid(struct hdac_ext_device *edev)
 		switch (type) {
 
 		case AC_WID_AUD_OUT:
-			hdmi->cvt_nid[cvt_nid] = nid;
-			cvt_nid++;
+			ret = hdac_hdmi_add_cvt(edev, nid);
+			if (ret < 0)
+				return ret;
 			break;
 
 		case AC_WID_PIN:
-			hdmi->pin_nid[pin_nid] = nid;
-			pin_nid++;
+			ret = hdac_hdmi_add_pin(edev, nid);
+			if (ret < 0)
+				return ret;
 			break;
 		}
 	}
 
 	hdac->end_nid = nid;
 
-	if (!pin_nid || !cvt_nid)
+	if (!hdmi->num_pin || !hdmi->num_cvt)
 		return -EIO;
 
-	/*
-	 * Currently on board only 1 pin and 1 converter is enabled for
-	 * simplification, more will be added eventually
-	 * So using fixed map for dai_id:pin:cvt
-	 */
-	return hdac_hdmi_init_dai_map(edev, &hdmi->dai_map[0], hdmi->pin_nid[0],
-			hdmi->cvt_nid[0], 0);
+	return hdac_hdmi_init_dai_map(edev);
 }
 
 static int hdmi_codec_probe(struct snd_soc_codec *codec)
@@ -544,6 +585,9 @@ static int hdac_hdmi_dev_probe(struct hdac_ext_device *edev)
 
 	dev_set_drvdata(&codec->dev, edev);
 
+	INIT_LIST_HEAD(&hdmi_priv->pin_list);
+	INIT_LIST_HEAD(&hdmi_priv->cvt_list);
+
 	ret = hdac_hdmi_parse_and_map_nid(edev);
 	if (ret < 0)
 		return ret;
@@ -555,8 +599,22 @@ static int hdac_hdmi_dev_probe(struct hdac_ext_device *edev)
 
 static int hdac_hdmi_dev_remove(struct hdac_ext_device *edev)
 {
+	struct hdac_hdmi_priv *hdmi = edev->private_data;
+	struct hdac_hdmi_pin *pin, *pin_next;
+	struct hdac_hdmi_cvt *cvt, *cvt_next;
+
 	snd_soc_unregister_codec(&edev->hdac.dev);
 
+	list_for_each_entry_safe(cvt, cvt_next, &hdmi->cvt_list, head) {
+		list_del(&cvt->head);
+		kfree(cvt);
+	}
+
+	list_for_each_entry_safe(pin, pin_next, &hdmi->pin_list, head) {
+		list_del(&pin->head);
+		kfree(pin);
+	}
+
 	return 0;
 }
 
-- 
2.7.0.rc3

[PATCH v3 04/15] ASoC: hdac_hdmi: Add hotplug notification and read eld

[Subhransu S. Prusty]

This patch uses i915 component framework to register for hotplug
notification. And once it identifies valid pin sense and valid eld,
reads the eld into the corresponding pin map buffer. For now it
directly sends the verbs and reads the eld. Later this will use
the i915 framework to populate ELD buffer once available.

The eld reading APIs in legacy hda are required for ASoC skylake
hdmi driver as well. So keeping a copy here and will remove once
component ops for reading ELD are merged in hda core.

Also read the monitor present sense during resume and ignore the
ELD notify from graphics during PM as is done in legacy hda,
commit 8ae743e82f0b ("ALSA: hda - Skip ELD notification during
system suspend")

Signed-off-by: Subhransu S. Prusty <subhransu.s.prusty@intel.com>
Signed-off-by: Vinod Koul <vinod.koul@intel.com>
---
 sound/soc/codecs/hdac_hdmi.c | 216 +++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 209 insertions(+), 7 deletions(-)

diff --git a/sound/soc/codecs/hdac_hdmi.c b/sound/soc/codecs/hdac_hdmi.c
index b206d5e..b9d300d 100644
--- a/sound/soc/codecs/hdac_hdmi.c
+++ b/sound/soc/codecs/hdac_hdmi.c
@@ -34,6 +34,9 @@
 
 #define HDA_MAX_CONNECTIONS     32
 
+#define ELD_MAX_SIZE    256
+#define ELD_FIXED_BYTES	20
+
 struct hdac_hdmi_cvt_params {
 	unsigned int channels_min;
 	unsigned int channels_max;
@@ -48,11 +51,22 @@ struct hdac_hdmi_cvt {
 	struct hdac_hdmi_cvt_params params;
 };
 
+struct hdmi_eld {
+	bool	monitor_present;
+	bool	eld_valid;
+	int	eld_size;
+	char    eld_buffer[ELD_MAX_SIZE];
+};
+
 struct hdac_hdmi_pin {
 	struct list_head head;
 	hda_nid_t nid;
 	int num_mux_nids;
 	hda_nid_t mux_nids[HDA_MAX_CONNECTIONS];
+	struct hdmi_eld eld;
+	struct hdac_ext_device *edev;
+	int repoll_count;
+	struct delayed_work work;
 };
 
 struct hdac_hdmi_dai_pin_map {
@@ -76,6 +90,81 @@ static inline struct hdac_ext_device *to_hda_ext_device(struct device *dev)
 	return container_of(hdac, struct hdac_ext_device, hdac);
 }
 
+ /* HDMI Eld routines */
+static unsigned int hdac_hdmi_get_eld_data(struct hdac_device *codec,
+				hda_nid_t nid, int byte_index)
+{
+	unsigned int val;
+
+	val = snd_hdac_codec_read(codec, nid, 0, AC_VERB_GET_HDMI_ELDD,
+							byte_index);
+
+	dev_dbg(&codec->dev, "HDMI: ELD data byte %d: 0x%x\n",
+			byte_index, val);
+
+	return val;
+}
+
+static int hdac_hdmi_get_eld_size(struct hdac_device *codec, hda_nid_t nid)
+{
+	return snd_hdac_codec_read(codec, nid, 0, AC_VERB_GET_HDMI_DIP_SIZE,
+						 AC_DIPSIZE_ELD_BUF);
+}
+
+/*
+ * This function queries the eld size and eld data and fills in the buffer
+ * passed by user
+ */
+static int hdac_hdmi_get_eld(struct hdac_device *codec, hda_nid_t nid,
+		     unsigned char *buf, int *eld_size)
+{
+	int i;
+	int ret = 0;
+	int size;
+
+	/*
+	 * ELD size is initialized to zero in caller function. If no errors and
+	 * ELD is valid, actual eld_size is assigned.
+	 */
+
+	size = hdac_hdmi_get_eld_size(codec, nid);
+	if (size < ELD_FIXED_BYTES || size > ELD_MAX_SIZE) {
+		dev_info(&codec->dev, "HDMI: invalid ELD buf size %d\n", size);
+		return -ERANGE;
+	}
+
+	/* set ELD buffer */
+	for (i = 0; i < size; i++) {
+		unsigned int val = hdac_hdmi_get_eld_data(codec, nid, i);
+		/*
+		 * Graphics driver might be writing to ELD buffer right now.
+		 * Just abort. The caller will repoll after a while.
+		 */
+		if (!(val & AC_ELDD_ELD_VALID)) {
+			dev_info(&codec->dev, "HDMI: invalid ELD data byte %d\n", i);
+			ret = -EINVAL;
+			goto error;
+		}
+		val &= AC_ELDD_ELD_DATA;
+		/*
+		 * The first byte cannot be zero. This can happen on some DVI
+		 * connections. Some Intel chips may also need some 250ms delay
+		 * to return non-zero ELD data, even when the graphics driver
+		 * correctly writes ELD content before setting ELD_valid bit.
+		 */
+		if (!val && !i) {
+			dev_dbg(&codec->dev, "HDMI: 0 ELD data\n");
+			ret = -EINVAL;
+			goto error;
+		}
+		buf[i] = val;
+	}
+
+	*eld_size = size;
+error:
+	return ret;
+}
+
 static int hdac_hdmi_setup_stream(struct hdac_ext_device *hdac,
 				hda_nid_t cvt_nid, hda_nid_t pin_nid,
 				u32 stream_tag, int format)
@@ -246,7 +335,6 @@ static int hdac_hdmi_pcm_open(struct snd_pcm_substream *substream,
 	struct hdac_ext_device *hdac = snd_soc_dai_get_drvdata(dai);
 	struct hdac_hdmi_priv *hdmi = hdac->private_data;
 	struct hdac_hdmi_dai_pin_map *dai_map;
-	int val;
 
 	if (dai->id > 0) {
 		dev_err(&hdac->hdac.dev, "Only one dai supported as of now\n");
@@ -255,12 +343,11 @@ static int hdac_hdmi_pcm_open(struct snd_pcm_substream *substream,
 
 	dai_map = &hdmi->dai_map[dai->id];
 
-	val = snd_hdac_codec_read(&hdac->hdac, dai_map->pin->nid, 0,
-					AC_VERB_GET_PIN_SENSE, 0);
-	dev_info(&hdac->hdac.dev, "Val for AC_VERB_GET_PIN_SENSE: %x\n", val);
-
-	if ((!(val & AC_PINSENSE_PRESENCE)) || (!(val & AC_PINSENSE_ELDV))) {
-		dev_err(&hdac->hdac.dev, "Monitor presence invalid with val: %x\n", val);
+	if ((!dai_map->pin->eld.monitor_present) ||
+		(!dai_map->pin->eld.eld_valid)) {
+		dev_err(&hdac->hdac.dev, "Failed: montior present? %d eld valid?: %d\n",
+				dai_map->pin->eld.monitor_present,
+				dai_map->pin->eld.eld_valid);
 		return -ENODEV;
 	}
 
@@ -432,6 +519,70 @@ static int hdac_hdmi_add_cvt(struct hdac_ext_device *edev, hda_nid_t nid)
 	return hdac_hdmi_query_cvt_params(&edev->hdac, cvt);
 }
 
+static void hdac_hdmi_present_sense(struct hdac_hdmi_pin *pin, int repoll)
+{
+	struct hdac_ext_device *edev = pin->edev;
+	int val;
+
+	if (!edev)
+		return;
+
+	pin->repoll_count = repoll;
+
+	pm_runtime_get_sync(&edev->hdac.dev);
+	val = snd_hdac_codec_read(&edev->hdac, pin->nid, 0,
+					AC_VERB_GET_PIN_SENSE, 0);
+
+	dev_dbg(&edev->hdac.dev, "Pin sense val %x for pin: %d\n",
+			val, pin->nid);
+	pin->eld.monitor_present = !!(val & AC_PINSENSE_PRESENCE);
+	pin->eld.eld_valid = !!(val & AC_PINSENSE_ELDV);
+
+	if (!pin->eld.monitor_present || !pin->eld.eld_valid) {
+		dev_info(&edev->hdac.dev, "%s: disconnect or eld_invalid\n",
+				__func__);
+		goto put_hdac_device;
+	}
+
+	if (pin->eld.monitor_present && pin->eld.eld_valid) {
+		/* TODO: Use i915 cmpnt framework when available */
+		if (hdac_hdmi_get_eld(&edev->hdac, pin->nid,
+				pin->eld.eld_buffer,
+				&pin->eld.eld_size) == 0) {
+			print_hex_dump_bytes("Eld: ", DUMP_PREFIX_OFFSET,
+					pin->eld.eld_buffer, pin->eld.eld_size);
+		} else {
+			dev_err(&edev->hdac.dev, "ELD invalid\n");
+			pin->eld.monitor_present = false;
+			pin->eld.eld_valid = false;
+		}
+
+	}
+
+	/*
+	 * Sometimes the pin_sense may present invalid monitor
+	 * present and eld_valid. If eld data is not valid loop, few
+	 * more times to get correct pin sense and valid eld.
+	 */
+	if ((!pin->eld.monitor_present || !pin->eld.eld_valid) && repoll)
+		schedule_delayed_work(&pin->work, msecs_to_jiffies(300));
+
+put_hdac_device:
+	pm_runtime_put_sync(&edev->hdac.dev);
+}
+
+static void hdac_hdmi_repoll_eld(struct work_struct *work)
+{
+	struct hdac_hdmi_pin *pin =
+		container_of(to_delayed_work(work), struct hdac_hdmi_pin, work);
+
+	/* picked from legacy */
+	if (pin->repoll_count++ > 6)
+		pin->repoll_count = 0;
+
+	hdac_hdmi_present_sense(pin, pin->repoll_count);
+}
+
 static int hdac_hdmi_add_pin(struct hdac_ext_device *edev, hda_nid_t nid)
 {
 	struct hdac_hdmi_priv *hdmi = edev->private_data;
@@ -446,6 +597,9 @@ static int hdac_hdmi_add_pin(struct hdac_ext_device *edev, hda_nid_t nid)
 	list_add_tail(&pin->head, &hdmi->pin_list);
 	hdmi->num_pin++;
 
+	pin->edev = edev;
+	INIT_DELAYED_WORK(&pin->work, hdac_hdmi_repoll_eld);
+
 	return 0;
 }
 
@@ -504,17 +658,64 @@ static int hdac_hdmi_parse_and_map_nid(struct hdac_ext_device *edev)
 	return hdac_hdmi_init_dai_map(edev);
 }
 
+static void hdac_hdmi_eld_notify_cb(void *aptr, int port)
+{
+	struct hdac_ext_device *edev = aptr;
+	struct hdac_hdmi_priv *hdmi = edev->private_data;
+	struct hdac_hdmi_pin *pin;
+	struct snd_soc_codec *codec = edev->scodec;
+	/* Don't know how this mapping is derived */
+	hda_nid_t pin_nid = port + 0x04;
+
+	dev_dbg(&edev->hdac.dev, "%s: for pin: %d\n", __func__, pin_nid);
+
+	/*
+	 * skip notification during system suspend (but not in runtime PM);
+	 * the state will be updated at resume. Also since the ELD and
+	 * connection states are updated in anyway at the end of the resume,
+	 * we can skip it when received during PM process.
+	 */
+	if (snd_power_get_state(codec->component.card->snd_card) !=
+			SNDRV_CTL_POWER_D0)
+		return;
+
+	if (atomic_read(&edev->hdac.in_pm))
+		return;
+
+	list_for_each_entry(pin, &hdmi->pin_list, head) {
+		if (pin->nid == pin_nid)
+			hdac_hdmi_present_sense(pin, 1);
+	}
+}
+
+static struct i915_audio_component_audio_ops aops = {
+	.pin_eld_notify	= hdac_hdmi_eld_notify_cb,
+};
+
 static int hdmi_codec_probe(struct snd_soc_codec *codec)
 {
 	struct hdac_ext_device *edev = snd_soc_codec_get_drvdata(codec);
 	struct hdac_hdmi_priv *hdmi = edev->private_data;
 	struct snd_soc_dapm_context *dapm =
 		snd_soc_component_get_dapm(&codec->component);
+	struct hdac_hdmi_pin *pin;
+	int ret;
 
 	edev->scodec = codec;
 
 	create_fill_widget_route_map(dapm, &hdmi->dai_map[0]);
 
+	aops.audio_ptr = edev;
+	ret = snd_hdac_i915_register_notifier(&aops);
+	if (ret < 0) {
+		dev_err(&edev->hdac.dev, "notifier register failed: err: %d\n",
+				ret);
+		return ret;
+	}
+
+	list_for_each_entry(pin, &hdmi->pin_list, head)
+		hdac_hdmi_present_sense(pin, 1);
+
 	/* Imp: Store the card pointer in hda_codec */
 	edev->card = dapm->card->snd_card;
 
@@ -659,6 +860,7 @@ static int hdac_hdmi_runtime_resume(struct device *dev)
 	if (!bus)
 		return 0;
 
+
 	err = snd_hdac_display_power(bus, true);
 	if (err < 0) {
 		dev_err(bus->dev, "Cannot turn on display power on i915\n");
-- 
1.9.1

[PATCH v3 05/15] ASoC: hdac_hdmi: Apply constraints based on ELD

[Subhransu S. Prusty]

Uses the drm eld core framework to apply rate and channel

Also compute the format to be set based on ELD.

Even though the channel constraint is based on ELD, infoframe
is set with stereo only. Multichannel support will be added
later.

Signed-off-by: Subhransu S. Prusty <subhransu.s.prusty@intel.com>
Signed-off-by: Vinod Koul <vinod.koul@intel.com>
---
 sound/soc/codecs/Kconfig     |  1 +
 sound/soc/codecs/hdac_hdmi.c | 48 +++++++++++++++++++++++++++++++++++++++++---
 2 files changed, 46 insertions(+), 3 deletions(-)

diff --git a/sound/soc/codecs/Kconfig b/sound/soc/codecs/Kconfig
index c0c5c8e..f805cb6 100644
--- a/sound/soc/codecs/Kconfig
+++ b/sound/soc/codecs/Kconfig
@@ -486,6 +486,7 @@ config SND_SOC_GTM601
 config SND_SOC_HDAC_HDMI
 	tristate
 	select SND_HDA_EXT_CORE
+	select SND_PCM_ELD
 	select HDMI
 
 config SND_SOC_ICS43432
diff --git a/sound/soc/codecs/hdac_hdmi.c b/sound/soc/codecs/hdac_hdmi.c
index b9d300d..3cc0217 100644
--- a/sound/soc/codecs/hdac_hdmi.c
+++ b/sound/soc/codecs/hdac_hdmi.c
@@ -22,10 +22,12 @@
 #include <linux/module.h>
 #include <linux/pm_runtime.h>
 #include <linux/hdmi.h>
+#include <drm/drm_edid.h>
 #include <sound/pcm_params.h>
 #include <sound/soc.h>
 #include <sound/hdaudio_ext.h>
 #include <sound/hda_i915.h>
+#include <sound/pcm_drm_eld.h>
 #include "../../hda/local.h"
 
 #define AMP_OUT_MUTE		0xb080
@@ -90,6 +92,42 @@ static inline struct hdac_ext_device *to_hda_ext_device(struct device *dev)
 	return container_of(hdac, struct hdac_ext_device, hdac);
 }
 
+static unsigned int sad_format(const u8 *sad)
+{
+	return ((sad[0] >> 0x3) & 0x1f);
+}
+
+static unsigned int sad_sample_bits_lpcm(const u8 *sad)
+{
+	return (sad[2] & 7);
+}
+
+static int hdac_hdmi_eld_limit_formats(struct snd_pcm_runtime *runtime,
+						void *eld)
+{
+	u64 formats = SNDRV_PCM_FMTBIT_S16;
+	int i;
+	const u8 *sad, *eld_buf = eld;
+
+	sad = drm_eld_sad(eld_buf);
+	if (!sad)
+		goto format_constraint;
+
+	for (i = drm_eld_sad_count(eld_buf); i > 0; i--, sad += 3) {
+		if (sad_format(sad) == 1) { /* AUDIO_CODING_TYPE_LPCM */
+
+			/* 20 bit and 24 bit */
+			if (sad_sample_bits_lpcm(sad) & 0x6)
+				formats |= SNDRV_PCM_FMTBIT_S32;
+		}
+	}
+
+format_constraint:
+	return snd_pcm_hw_constraint_mask64(runtime, SNDRV_PCM_HW_PARAM_FORMAT,
+				formats);
+
+}
+
  /* HDMI Eld routines */
 static unsigned int hdac_hdmi_get_eld_data(struct hdac_device *codec,
 				hda_nid_t nid, int byte_index)
@@ -335,6 +373,7 @@ static int hdac_hdmi_pcm_open(struct snd_pcm_substream *substream,
 	struct hdac_ext_device *hdac = snd_soc_dai_get_drvdata(dai);
 	struct hdac_hdmi_priv *hdmi = hdac->private_data;
 	struct hdac_hdmi_dai_pin_map *dai_map;
+	int ret;
 
 	if (dai->id > 0) {
 		dev_err(&hdac->hdac.dev, "Only one dai supported as of now\n");
@@ -356,10 +395,13 @@ static int hdac_hdmi_pcm_open(struct snd_pcm_substream *substream,
 	snd_hdac_codec_write(&hdac->hdac, dai_map->pin->nid, 0,
 			AC_VERB_SET_AMP_GAIN_MUTE, AMP_OUT_UNMUTE);
 
-	snd_pcm_hw_constraint_step(substream->runtime, 0,
-				   SNDRV_PCM_HW_PARAM_CHANNELS, 2);
+	ret = hdac_hdmi_eld_limit_formats(substream->runtime,
+				dai_map->pin->eld.eld_buffer);
+	if (ret < 0)
+		return ret;
 
-	return 0;
+	return snd_pcm_hw_constraint_eld(substream->runtime,
+				dai_map->pin->eld.eld_buffer);
 }
 
 static void hdac_hdmi_pcm_close(struct snd_pcm_substream *substream,
-- 
1.9.1

[PATCH v3 06/15] ASoC: hdac_hdmi: Enable DP1.2 and all converters/pins

[Subhransu S. Prusty]

By default only one converter and pin widget are enabled. A vendor
widget required to be configured to enable all the widgets of the
codec.

As we are enabling the DP support enable the DP1.2 feature as well.

The changes below are copied from patch_hdmi.c

Signed-off-by: Subhransu S. Prusty <subhransu.s.prusty@intel.com>
Signed-off-by: Vinod Koul <vinod.koul@intel.com>
---
 sound/soc/codecs/hdac_hdmi.c | 43 +++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 43 insertions(+)

diff --git a/sound/soc/codecs/hdac_hdmi.c b/sound/soc/codecs/hdac_hdmi.c
index 3cc0217..27d1a54 100644
--- a/sound/soc/codecs/hdac_hdmi.c
+++ b/sound/soc/codecs/hdac_hdmi.c
@@ -645,6 +645,46 @@ static int hdac_hdmi_add_pin(struct hdac_ext_device *edev, hda_nid_t nid)
 	return 0;
 }
 
+#define INTEL_VENDOR_NID 0x08
+#define INTEL_GET_VENDOR_VERB 0xf81
+#define INTEL_SET_VENDOR_VERB 0x781
+#define INTEL_EN_DP12			0x02 /* enable DP 1.2 features */
+#define INTEL_EN_ALL_PIN_CVTS	0x01 /* enable 2nd & 3rd pins and convertors */
+
+static void hdac_hdmi_skl_enable_all_pins(struct hdac_device *hdac)
+{
+	unsigned int vendor_param;
+
+	vendor_param = snd_hdac_codec_read(hdac, INTEL_VENDOR_NID, 0,
+				INTEL_GET_VENDOR_VERB, 0);
+	if (vendor_param == -1 || vendor_param & INTEL_EN_ALL_PIN_CVTS)
+		return;
+
+	vendor_param |= INTEL_EN_ALL_PIN_CVTS;
+	vendor_param = snd_hdac_codec_read(hdac, INTEL_VENDOR_NID, 0,
+				INTEL_SET_VENDOR_VERB, vendor_param);
+	if (vendor_param == -1)
+		return;
+}
+
+static void hdac_hdmi_skl_enable_dp12(struct hdac_device *hdac)
+{
+	unsigned int vendor_param;
+
+	vendor_param = snd_hdac_codec_read(hdac, INTEL_VENDOR_NID, 0,
+				INTEL_GET_VENDOR_VERB, 0);
+	if (vendor_param == -1 || vendor_param & INTEL_EN_DP12)
+		return;
+
+	/* enable DP1.2 mode */
+	vendor_param |= INTEL_EN_DP12;
+	vendor_param = snd_hdac_codec_read(hdac, INTEL_VENDOR_NID, 0,
+				INTEL_SET_VENDOR_VERB, vendor_param);
+	if (vendor_param == -1)
+		return;
+
+}
+
 /*
  * Parse all nodes and store the cvt/pin nids in array
  * Add one time initialization for pin and cvt widgets
@@ -657,6 +697,9 @@ static int hdac_hdmi_parse_and_map_nid(struct hdac_ext_device *edev)
 	struct hdac_hdmi_priv *hdmi = edev->private_data;
 	int ret;
 
+	hdac_hdmi_skl_enable_all_pins(hdac);
+	hdac_hdmi_skl_enable_dp12(hdac);
+
 	num_nodes = snd_hdac_get_sub_nodes(hdac, hdac->afg, &nid);
 	if (!nid || num_nodes <= 0) {
 		dev_warn(&hdac->dev, "HDMI: failed to get afg sub nodes\n");
-- 
1.9.1

[PATCH v3 07/15] ASoC: hdac_hdmi: create dais based on number of streams

[Subhransu S. Prusty]

A stream is mapped to a converter. So based on the converters
queried, dais are created.

The streams can be dynamically routed to any converter. For
now it is mapped statically. The dynamic mapping of stream
to converter will be added when required.

Signed-off-by: Subhransu S. Prusty <subhransu.s.prusty@intel.com>
Signed-off-by: Vinod Koul <vinod.koul@intel.com>
---
 sound/soc/codecs/hdac_hdmi.c | 108 ++++++++++++++++++++++++++++++-------------
 1 file changed, 76 insertions(+), 32 deletions(-)

diff --git a/sound/soc/codecs/hdac_hdmi.c b/sound/soc/codecs/hdac_hdmi.c
index 27d1a54..01d063e 100644
--- a/sound/soc/codecs/hdac_hdmi.c
+++ b/sound/soc/codecs/hdac_hdmi.c
@@ -30,6 +30,8 @@
 #include <sound/pcm_drm_eld.h>
 #include "../../hda/local.h"
 
+#define NAME_SIZE	32
+
 #define AMP_OUT_MUTE		0xb080
 #define AMP_OUT_UNMUTE		0xb000
 #define PIN_OUT			(AC_PINCTL_OUT_EN)
@@ -685,11 +687,67 @@ static void hdac_hdmi_skl_enable_dp12(struct hdac_device *hdac)
 
 }
 
+static struct snd_soc_dai_ops hdmi_dai_ops = {
+	.startup = hdac_hdmi_pcm_open,
+	.shutdown = hdac_hdmi_pcm_close,
+	.hw_params = hdac_hdmi_set_hw_params,
+	.prepare = hdac_hdmi_playback_prepare,
+	.hw_free = hdac_hdmi_playback_cleanup,
+};
+
+static int hdac_hdmi_create_dais(struct hdac_device *hdac,
+		struct snd_soc_dai_driver **dais,
+		struct hdac_hdmi_priv *hdmi, int num_dais)
+{
+	struct snd_soc_dai_driver *hdmi_dais;
+	struct hdac_hdmi_cvt *cvt;
+	char name[NAME_SIZE], dai_name[NAME_SIZE];
+	int i = 0;
+	u32 rates, bps;
+	unsigned int rate_max = 384000, rate_min = 8000;
+	u64 formats;
+	int ret;
+
+	hdmi_dais = devm_kzalloc(&hdac->dev, (sizeof(*hdmi_dais) * num_dais),
+			GFP_KERNEL);
+	if (!hdmi_dais)
+		return -ENOMEM;
+
+	list_for_each_entry(cvt, &hdmi->cvt_list, head) {
+		ret = snd_hdac_query_supported_pcm(hdac, cvt->nid, &rates,
+			&formats, &bps);
+		if (ret)
+			return ret;
+
+		sprintf(dai_name, "intel-hdmi-hifi%d", i+1);
+		hdmi_dais[i].name = devm_kstrdup(&hdac->dev, dai_name,
+						GFP_KERNEL);
+
+		snprintf(name, sizeof(name), "hifi%d", i+1);
+		hdmi_dais[i].playback.stream_name =
+				devm_kstrdup(&hdac->dev, name, GFP_KERNEL);
+		hdmi_dais[i].playback.formats = formats;
+		hdmi_dais[i].playback.rates = rates;
+		hdmi_dais[i].playback.rate_max = rate_max;
+		hdmi_dais[i].playback.rate_min = rate_min;
+		hdmi_dais[i].playback.channels_min = 2;
+		hdmi_dais[i].playback.channels_max = 2;
+		hdmi_dais[i].ops = &hdmi_dai_ops;
+
+		i++;
+	}
+
+	*dais = hdmi_dais;
+
+	return 0;
+}
+
 /*
  * Parse all nodes and store the cvt/pin nids in array
  * Add one time initialization for pin and cvt widgets
  */
-static int hdac_hdmi_parse_and_map_nid(struct hdac_ext_device *edev)
+static int hdac_hdmi_parse_and_map_nid(struct hdac_ext_device *edev,
+		struct snd_soc_dai_driver **dais, int *num_dais)
 {
 	hda_nid_t nid;
 	int i, num_nodes;
@@ -740,6 +798,15 @@ static int hdac_hdmi_parse_and_map_nid(struct hdac_ext_device *edev)
 	if (!hdmi->num_pin || !hdmi->num_cvt)
 		return -EIO;
 
+	ret = hdac_hdmi_create_dais(hdac, dais, hdmi, hdmi->num_cvt);
+	if (ret) {
+		dev_err(&hdac->dev, "Failed to create dais with err: %d\n",
+							ret);
+		return ret;
+	}
+
+	*num_dais = hdmi->num_cvt;
+
 	return hdac_hdmi_init_dai_map(edev);
 }
 
@@ -829,38 +896,12 @@ static struct snd_soc_codec_driver hdmi_hda_codec = {
 	.idle_bias_off	= true,
 };
 
-static struct snd_soc_dai_ops hdmi_dai_ops = {
-	.startup = hdac_hdmi_pcm_open,
-	.shutdown = hdac_hdmi_pcm_close,
-	.hw_params = hdac_hdmi_set_hw_params,
-	.prepare = hdac_hdmi_playback_prepare,
-	.hw_free = hdac_hdmi_playback_cleanup,
-};
-
-static struct snd_soc_dai_driver hdmi_dais[] = {
-	{	.name = "intel-hdmi-hif1",
-		.playback = {
-			.stream_name = "hif1",
-			.channels_min = 2,
-			.channels_max = 2,
-			.rates = SNDRV_PCM_RATE_32000 |
-				SNDRV_PCM_RATE_44100 | SNDRV_PCM_RATE_48000 |
-				SNDRV_PCM_RATE_88200 | SNDRV_PCM_RATE_96000 |
-				SNDRV_PCM_RATE_176400 | SNDRV_PCM_RATE_192000,
-			.formats = SNDRV_PCM_FMTBIT_S16_LE |
-				SNDRV_PCM_FMTBIT_S20_3LE |
-				SNDRV_PCM_FMTBIT_S24_LE |
-				SNDRV_PCM_FMTBIT_S32_LE,
-
-		},
-		.ops = &hdmi_dai_ops,
-	},
-};
-
 static int hdac_hdmi_dev_probe(struct hdac_ext_device *edev)
 {
 	struct hdac_device *codec = &edev->hdac;
 	struct hdac_hdmi_priv *hdmi_priv;
+	struct snd_soc_dai_driver *hdmi_dais = NULL;
+	int num_dais = 0;
 	int ret = 0;
 
 	hdmi_priv = devm_kzalloc(&codec->dev, sizeof(*hdmi_priv), GFP_KERNEL);
@@ -874,13 +915,16 @@ static int hdac_hdmi_dev_probe(struct hdac_ext_device *edev)
 	INIT_LIST_HEAD(&hdmi_priv->pin_list);
 	INIT_LIST_HEAD(&hdmi_priv->cvt_list);
 
-	ret = hdac_hdmi_parse_and_map_nid(edev);
-	if (ret < 0)
+	ret = hdac_hdmi_parse_and_map_nid(edev, &hdmi_dais, &num_dais);
+	if (ret < 0) {
+		dev_err(&codec->dev,
+			"Failed in parse and map nid with err: %d\n", ret);
 		return ret;
+	}
 
 	/* ASoC specific initialization */
 	return snd_soc_register_codec(&codec->dev, &hdmi_hda_codec,
-			hdmi_dais, ARRAY_SIZE(hdmi_dais));
+			hdmi_dais, num_dais);
 }
 
 static int hdac_hdmi_dev_remove(struct hdac_ext_device *edev)
-- 
1.9.1

Re: [PATCH v3 07/15] ASoC: hdac_hdmi: create dais based on number of streams

[Takashi Iwai]

On Mon, 07 Dec 2015 22:24:21 +0100,
Subhransu S. Prusty wrote:
> 
> A stream is mapped to a converter. So based on the converters
> queried, dais are created.
> 
> The streams can be dynamically routed to any converter. For
> now it is mapped statically. The dynamic mapping of stream
> to converter will be added when required.
> 
> Signed-off-by: Subhransu S. Prusty <subhransu.s.prusty@intel.com>
> Signed-off-by: Vinod Koul <vinod.koul@intel.com>
> ---
>  sound/soc/codecs/hdac_hdmi.c | 108 ++++++++++++++++++++++++++++++-------------
>  1 file changed, 76 insertions(+), 32 deletions(-)
> 
> diff --git a/sound/soc/codecs/hdac_hdmi.c b/sound/soc/codecs/hdac_hdmi.c
> index 27d1a54..01d063e 100644
> --- a/sound/soc/codecs/hdac_hdmi.c
> +++ b/sound/soc/codecs/hdac_hdmi.c
> @@ -30,6 +30,8 @@
>  #include <sound/pcm_drm_eld.h>
>  #include "../../hda/local.h"
>  
> +#define NAME_SIZE	32
> +
>  #define AMP_OUT_MUTE		0xb080
>  #define AMP_OUT_UNMUTE		0xb000
>  #define PIN_OUT			(AC_PINCTL_OUT_EN)
> @@ -685,11 +687,67 @@ static void hdac_hdmi_skl_enable_dp12(struct hdac_device *hdac)
>  
>  }
>  
> +static struct snd_soc_dai_ops hdmi_dai_ops = {
> +	.startup = hdac_hdmi_pcm_open,
> +	.shutdown = hdac_hdmi_pcm_close,
> +	.hw_params = hdac_hdmi_set_hw_params,
> +	.prepare = hdac_hdmi_playback_prepare,
> +	.hw_free = hdac_hdmi_playback_cleanup,
> +};
> +
> +static int hdac_hdmi_create_dais(struct hdac_device *hdac,
> +		struct snd_soc_dai_driver **dais,
> +		struct hdac_hdmi_priv *hdmi, int num_dais)
> +{
> +	struct snd_soc_dai_driver *hdmi_dais;
> +	struct hdac_hdmi_cvt *cvt;
> +	char name[NAME_SIZE], dai_name[NAME_SIZE];
> +	int i = 0;
> +	u32 rates, bps;
> +	unsigned int rate_max = 384000, rate_min = 8000;
> +	u64 formats;
> +	int ret;
> +
> +	hdmi_dais = devm_kzalloc(&hdac->dev, (sizeof(*hdmi_dais) * num_dais),
> +			GFP_KERNEL);
> +	if (!hdmi_dais)
> +		return -ENOMEM;
> +
> +	list_for_each_entry(cvt, &hdmi->cvt_list, head) {
> +		ret = snd_hdac_query_supported_pcm(hdac, cvt->nid, &rates,
> +			&formats, &bps);
> +		if (ret)
> +			return ret;
> +
> +		sprintf(dai_name, "intel-hdmi-hifi%d", i+1);
> +		hdmi_dais[i].name = devm_kstrdup(&hdac->dev, dai_name,
> +						GFP_KERNEL);
> +

Missing NULL check.

> +		snprintf(name, sizeof(name), "hifi%d", i+1);
> +		hdmi_dais[i].playback.stream_name =
> +				devm_kstrdup(&hdac->dev, name, GFP_KERNEL);

Ditto.


Takashi

[PATCH v3 08/15] ASoC: hdac_hdmi: Create widget/route based on nodes enumerated

[Subhransu S. Prusty]

Instead of direct mapping between converter and pin, Muxes are
added between them to support any converter connection to any pin.

As the possible mux inputs can only be identified during runtime,
all possible routes are created to connect all converters to all
pin muxes. The user should enable appropriate mux inputs to enable
correct port.

In the process to support the above changes widget/route fill APIs
are updated to take all required parameters.

Signed-off-by: Subhransu S. Prusty <subhransu.s.prusty@intel.com>
Signed-off-by: Vinod Koul <vinod.koul@intel.com>
---
 sound/soc/codecs/hdac_hdmi.c | 224 ++++++++++++++++++++++++++++++++++++++-----
 1 file changed, 202 insertions(+), 22 deletions(-)

diff --git a/sound/soc/codecs/hdac_hdmi.c b/sound/soc/codecs/hdac_hdmi.c
index 01d063e..467143cc 100644
--- a/sound/soc/codecs/hdac_hdmi.c
+++ b/sound/soc/codecs/hdac_hdmi.c
@@ -463,46 +463,224 @@ static int hdac_hdmi_query_pin_connlist(struct hdac_ext_device *hdac,
 	return pin->num_mux_nids;
 }
 
-static void hdac_hdmi_fill_widget_info(struct snd_soc_dapm_widget *w,
-				enum snd_soc_dapm_type id,
-				const char *wname, const char *stream)
+static void hdac_hdmi_fill_widget_info(struct device *dev,
+				struct snd_soc_dapm_widget *w,
+				enum snd_soc_dapm_type id, void *priv,
+				const char *wname, const char *stream,
+				struct snd_kcontrol_new *wc, int numkc)
 {
 	w->id = id;
-	w->name = wname;
+	w->name = devm_kstrdup(dev, wname, GFP_KERNEL);
 	w->sname = stream;
 	w->reg = SND_SOC_NOPM;
 	w->shift = 0;
-	w->kcontrol_news = NULL;
-	w->num_kcontrols = 0;
-	w->priv = NULL;
+	w->kcontrol_news = wc;
+	w->num_kcontrols = numkc;
+	w->priv = priv;
 }
 
 static void hdac_hdmi_fill_route(struct snd_soc_dapm_route *route,
-		const char *sink, const char *control, const char *src)
+		const char *sink, const char *control, const char *src,
+		int (*handler)(struct snd_soc_dapm_widget *sr,
+			struct snd_soc_dapm_widget *snk))
 {
 	route->sink = sink;
 	route->source = src;
 	route->control = control;
-	route->connected = NULL;
+	route->connected = handler;
 }
 
-static void create_fill_widget_route_map(struct snd_soc_dapm_context *dapm,
-					struct hdac_hdmi_dai_pin_map *dai_map)
+/*
+ * Ideally the Mux inputs should be based on the num_muxs enumerated, but
+ * the display driver seem to be programming the connection list for the pin
+ * widget runtime.
+ *
+ * So programming all the possible inputs for the mux, the user has to take
+ * care of selecting the right one and leaving all other inputs selected to
+ * "NONE"
+ */
+static int hdac_hdmi_create_pin_muxs(struct hdac_ext_device *edev,
+				struct hdac_hdmi_pin *pin,
+				struct snd_soc_dapm_widget *widget,
+				const char *widget_name)
+{
+	struct hdac_hdmi_priv *hdmi = edev->private_data;
+	struct snd_kcontrol_new *kc;
+	struct hdac_hdmi_cvt *cvt;
+	struct soc_enum *se;
+	char kc_name[NAME_SIZE];
+	char mux_items[NAME_SIZE];
+	/* To hold inputs to the Pin mux */
+	char *items[HDA_MAX_CONNECTIONS];
+	int i = 0;
+	int num_items = hdmi->num_cvt + 1;
+
+	kc = devm_kzalloc(&edev->hdac.dev, sizeof(*kc), GFP_KERNEL);
+	if (!kc)
+		return -ENOMEM;
+
+	se = devm_kzalloc(&edev->hdac.dev, sizeof(*se), GFP_KERNEL);
+	if (!se)
+		return -ENOMEM;
+
+	sprintf(kc_name, "Pin %d Input", pin->nid);
+	kc->name = devm_kstrdup(&edev->hdac.dev, kc_name, GFP_KERNEL);
+	kc->private_value = (long)se;
+	kc->iface = SNDRV_CTL_ELEM_IFACE_MIXER;
+	kc->access = 0;
+	kc->info = snd_soc_info_enum_double;
+	kc->put = snd_soc_dapm_put_enum_double;
+	kc->get = snd_soc_dapm_get_enum_double;
+
+	se->reg = SND_SOC_NOPM;
+
+	/* enum texts: ["NONE", "cvt #", "cvt #", ...] */
+	se->items = num_items;
+	se->mask = roundup_pow_of_two(se->items) - 1;
+
+	sprintf(mux_items, "NONE");
+	items[i] = devm_kstrdup(&edev->hdac.dev, mux_items, GFP_KERNEL);
+
+	list_for_each_entry(cvt, &hdmi->cvt_list, head) {
+		i++;
+		sprintf(mux_items, "cvt %d", cvt->nid);
+		items[i] = devm_kstrdup(&edev->hdac.dev, mux_items, GFP_KERNEL);
+	}
+
+	se->texts = devm_kmemdup(&edev->hdac.dev, items,
+			(num_items  * sizeof(char *)), GFP_KERNEL);
+
+	hdac_hdmi_fill_widget_info(&edev->hdac.dev, widget, snd_soc_dapm_mux,
+				&pin->nid, widget_name, NULL, kc, 1);
+
+	return 0;
+}
+
+/* Add cvt <- input <- mux route map */
+static void hdac_hdmi_add_pinmux_cvt_route(struct hdac_ext_device *edev,
+		struct snd_soc_dapm_widget *widgets,
+		struct snd_soc_dapm_route *route, int rindex)
+{
+	struct hdac_hdmi_priv *hdmi = edev->private_data;
+	const struct snd_kcontrol_new *kc;
+	struct soc_enum *se;
+	int mux_index = hdmi->num_cvt + hdmi->num_pin;
+	int i, j;
+
+	for (i = 0; i < hdmi->num_pin; i++) {
+		kc = widgets[mux_index].kcontrol_news;
+		se = (struct soc_enum *)kc->private_value;
+		for (j = 0; j < hdmi->num_cvt; j++) {
+			hdac_hdmi_fill_route(&route[rindex],
+					widgets[mux_index].name,
+					se->texts[j + 1],
+					widgets[j].name, NULL);
+
+			rindex++;
+		}
+
+		mux_index++;
+	}
+}
+
+/*
+ * Widgets are added in the below sequence
+ *	Converter widgets for num converters enumerated
+ *	Pin widgets for num pins enumerated
+ *	Pin mux widgets to represent connenction list of pin widget
+ *
+ * Total widgets elements = num_cvt + num_pin + num_pin;
+ *
+ * Routes are added as below:
+ *	pin mux -> pin (based on num_pins)
+ *	cvt -> "Input sel control" -> pin_mux
+ *
+ * Total route elements:
+ *	num_pins + (pin_muxes * num_cvt)
+ */
+static int create_fill_widget_route_map(struct snd_soc_dapm_context *dapm)
 {
-	struct snd_soc_dapm_route route[1];
-	struct snd_soc_dapm_widget widgets[2] = { {0} };
+	struct snd_soc_dapm_widget *widgets;
+	struct snd_soc_dapm_route *route;
+	struct hdac_ext_device *edev = to_hda_ext_device(dapm->dev);
+	struct hdac_hdmi_priv *hdmi = edev->private_data;
+	struct snd_soc_dai_driver *dai_drv = dapm->component->dai_drv;
+	char widget_name[NAME_SIZE];
+	struct hdac_hdmi_cvt *cvt;
+	struct hdac_hdmi_pin *pin;
+	int i = 0;
+	int ret;
+	int num_routes = 0;
 
-	memset(&route, 0, sizeof(route));
+	if (list_empty(&hdmi->cvt_list) || list_empty(&hdmi->pin_list))
+		return -EINVAL;
 
-	hdac_hdmi_fill_widget_info(&widgets[0], snd_soc_dapm_output,
-			"hif1 Output", NULL);
-	hdac_hdmi_fill_widget_info(&widgets[1], snd_soc_dapm_aif_in,
-			"Coverter 1", "hif1");
+	widgets = devm_kzalloc(dapm->dev,
+			(sizeof(*widgets) * ((2 * hdmi->num_pin) + hdmi->num_cvt)),
+			GFP_KERNEL);
+	if (!widgets)
+		return -ENOMEM;
 
-	hdac_hdmi_fill_route(&route[0], "hif1 Output", NULL, "Coverter 1");
+	/* DAPM widgets to represent each converter widget */
+	list_for_each_entry(cvt, &hdmi->cvt_list, head) {
+		sprintf(widget_name, "Converter %d", cvt->nid);
+		hdac_hdmi_fill_widget_info(dapm->dev, &widgets[i],
+			snd_soc_dapm_aif_in, &cvt->nid,
+			widget_name, dai_drv[i].playback.stream_name, NULL, 0);
+		i++;
+	}
+
+	list_for_each_entry(pin, &hdmi->pin_list, head) {
+		sprintf(widget_name, "hif%d Output", pin->nid);
+		hdac_hdmi_fill_widget_info(dapm->dev, &widgets[i],
+				snd_soc_dapm_output, &pin->nid,
+				widget_name, NULL, NULL, 0);
+		i++;
+	}
+
+	/* DAPM widgets to represent the connection list to pin widget */
+	list_for_each_entry(pin, &hdmi->pin_list, head) {
+		sprintf(widget_name, "Pin %d Mux", pin->nid);
+		ret = hdac_hdmi_create_pin_muxs(edev, pin, &widgets[i],
+							widget_name);
+		if (ret < 0)
+			return ret;
+		i++;
+
+		/* For cvt to pin_mux mapping */
+		num_routes += hdmi->num_cvt;
+
+		/* For pin_mux to pin mapping */
+		num_routes++;
+	}
+
+	route = devm_kzalloc(dapm->dev, (sizeof(*route) * num_routes),
+							GFP_KERNEL);
+	if (!route)
+		return -ENOMEM;
+
+	i = 0;
+	/* Add pin <- NULL <- mux route map */
+	list_for_each_entry(pin, &hdmi->pin_list, head) {
+		int sink_index = i + hdmi->num_cvt;
+		int src_index = sink_index + hdmi->num_pin;
+
+		hdac_hdmi_fill_route(&route[i], widgets[sink_index].name,
+				NULL, widgets[src_index].name,
+				NULL);
+		i++;
+
+	}
+
+	hdac_hdmi_add_pinmux_cvt_route(edev, widgets, route, i);
+
+	snd_soc_dapm_new_controls(dapm, widgets,
+		((2 * hdmi->num_pin) + hdmi->num_cvt));
+	snd_soc_dapm_add_routes(dapm, route, num_routes);
+	snd_soc_dapm_new_widgets(dapm->card);
+
+	return 0;
 
-	snd_soc_dapm_new_controls(dapm, widgets, ARRAY_SIZE(widgets));
-	snd_soc_dapm_add_routes(dapm, route, ARRAY_SIZE(route));
 }
 
 static int hdac_hdmi_init_dai_map(struct hdac_ext_device *edev)
@@ -855,7 +1033,9 @@ static int hdmi_codec_probe(struct snd_soc_codec *codec)
 
 	edev->scodec = codec;
 
-	create_fill_widget_route_map(dapm, &hdmi->dai_map[0]);
+	ret = create_fill_widget_route_map(dapm);
+	if (ret < 0)
+		return ret;
 
 	aops.audio_ptr = edev;
 	ret = snd_hdac_i915_register_notifier(&aops);
-- 
1.9.1

Re: [PATCH v3 08/15] ASoC: hdac_hdmi: Create widget/route based on nodes enumerated

[Takashi Iwai]

On Mon, 07 Dec 2015 22:24:22 +0100,
Subhransu S. Prusty wrote:
> 
> Instead of direct mapping between converter and pin, Muxes are
> added between them to support any converter connection to any pin.
> 
> As the possible mux inputs can only be identified during runtime,
> all possible routes are created to connect all converters to all
> pin muxes. The user should enable appropriate mux inputs to enable
> correct port.
> 
> In the process to support the above changes widget/route fill APIs
> are updated to take all required parameters.
> 
> Signed-off-by: Subhransu S. Prusty <subhransu.s.prusty@intel.com>
> Signed-off-by: Vinod Koul <vinod.koul@intel.com>
> ---
>  sound/soc/codecs/hdac_hdmi.c | 224 ++++++++++++++++++++++++++++++++++++++-----
>  1 file changed, 202 insertions(+), 22 deletions(-)
> 
> diff --git a/sound/soc/codecs/hdac_hdmi.c b/sound/soc/codecs/hdac_hdmi.c
> index 01d063e..467143cc 100644
> --- a/sound/soc/codecs/hdac_hdmi.c
> +++ b/sound/soc/codecs/hdac_hdmi.c
> @@ -463,46 +463,224 @@ static int hdac_hdmi_query_pin_connlist(struct hdac_ext_device *hdac,
>  	return pin->num_mux_nids;
>  }
>  
> -static void hdac_hdmi_fill_widget_info(struct snd_soc_dapm_widget *w,
> -				enum snd_soc_dapm_type id,
> -				const char *wname, const char *stream)
> +static void hdac_hdmi_fill_widget_info(struct device *dev,
> +				struct snd_soc_dapm_widget *w,
> +				enum snd_soc_dapm_type id, void *priv,
> +				const char *wname, const char *stream,
> +				struct snd_kcontrol_new *wc, int numkc)
>  {
>  	w->id = id;
> -	w->name = wname;
> +	w->name = devm_kstrdup(dev, wname, GFP_KERNEL);

Missing NULL check.  Not only here but in handful places in this
patch.


Takashi

Re: [PATCH v3 08/15] ASoC: hdac_hdmi: Create widget/route based on nodes enumerated

[Subhransu S. Prusty]

On Mon, Dec 07, 2015 at 05:14:41PM +0100, Takashi Iwai wrote:
> On Mon, 07 Dec 2015 22:24:22 +0100,
> Subhransu S. Prusty wrote:
> >  	return pin->num_mux_nids;
> >  }
> >  
> > -static void hdac_hdmi_fill_widget_info(struct snd_soc_dapm_widget *w,
> > -				enum snd_soc_dapm_type id,
> > -				const char *wname, const char *stream)
> > +static void hdac_hdmi_fill_widget_info(struct device *dev,
> > +				struct snd_soc_dapm_widget *w,
> > +				enum snd_soc_dapm_type id, void *priv,
> > +				const char *wname, const char *stream,
> > +				struct snd_kcontrol_new *wc, int numkc)
> >  {
> >  	w->id = id;
> > -	w->name = wname;
> > +	w->name = devm_kstrdup(dev, wname, GFP_KERNEL);
> 
> Missing NULL check.  Not only here but in handful places in this
> patch.

Sorry to have missed to add check on these. Will fix them.

Regards,
Subhransu
> 
> 
> Takashi

-- 

[PATCH v3 09/15] ASoC: hdac_hdmi: Assign pin for stream based on dapm connection

[Subhransu S. Prusty]

Now that we have all the widgets enumerated we can route the stream
to any pin widget based on Mux connection. So map the pin to stream
accordingly.

Also seems the connection list to the pin widgets are not static
and is updated runtime based on type of display attached to the port.
This looks to be a hardware behavior.

So querying of the connection list is removed from pin initialization
and used in selecting the pin for a dai map.

Signed-off-by: Subhransu S. Prusty <subhransu.s.prusty@intel.com>
Signed-off-by: Vinod Koul <vinod.koul@intel.com>
---
 sound/soc/codecs/hdac_hdmi.c | 212 +++++++++++++++++++++++++++++++------------
 1 file changed, 153 insertions(+), 59 deletions(-)

diff --git a/sound/soc/codecs/hdac_hdmi.c b/sound/soc/codecs/hdac_hdmi.c
index 467143cc..2d971c4 100644
--- a/sound/soc/codecs/hdac_hdmi.c
+++ b/sound/soc/codecs/hdac_hdmi.c
@@ -38,6 +38,8 @@
 
 #define HDA_MAX_CONNECTIONS     32
 
+#define HDA_MAX_CVTS		3
+
 #define ELD_MAX_SIZE    256
 #define ELD_FIXED_BYTES	20
 
@@ -80,7 +82,7 @@ struct hdac_hdmi_dai_pin_map {
 };
 
 struct hdac_hdmi_priv {
-	struct hdac_hdmi_dai_pin_map dai_map[3];
+	struct hdac_hdmi_dai_pin_map dai_map[HDA_MAX_CVTS];
 	struct list_head pin_list;
 	struct list_head cvt_list;
 	int num_pin;
@@ -369,12 +371,125 @@ static int hdac_hdmi_playback_cleanup(struct snd_pcm_substream *substream,
 	return 0;
 }
 
+static int hdac_hdmi_enable_pin(struct hdac_ext_device *hdac,
+		struct hdac_hdmi_dai_pin_map *dai_map)
+{
+	int mux_idx;
+	struct hdac_hdmi_pin *pin = dai_map->pin;
+
+	for (mux_idx = 0; mux_idx < pin->num_mux_nids; mux_idx++) {
+		if (pin->mux_nids[mux_idx] == dai_map->cvt->nid) {
+			snd_hdac_codec_write(&hdac->hdac, pin->nid, 0,
+					AC_VERB_SET_CONNECT_SEL, mux_idx);
+			break;
+		}
+	}
+
+	if (mux_idx == pin->num_mux_nids)
+		return -EIO;
+
+	/* Enable out path for this pin widget */
+	snd_hdac_codec_write(&hdac->hdac, pin->nid, 0,
+			AC_VERB_SET_PIN_WIDGET_CONTROL, PIN_OUT);
+
+	hdac_hdmi_set_power_state(hdac, dai_map, AC_PWRST_D0);
+
+	snd_hdac_codec_write(&hdac->hdac, pin->nid, 0,
+			AC_VERB_SET_AMP_GAIN_MUTE, AMP_OUT_UNMUTE);
+
+	return 0;
+}
+
+static int hdac_hdmi_query_pin_connlist(struct hdac_ext_device *hdac,
+					struct hdac_hdmi_pin *pin)
+{
+	if (!(get_wcaps(&hdac->hdac, pin->nid) & AC_WCAP_CONN_LIST)) {
+		dev_warn(&hdac->hdac.dev,
+			"HDMI: pin %d wcaps %#x does not support connection list\n",
+			pin->nid, get_wcaps(&hdac->hdac, pin->nid));
+		return -EINVAL;
+	}
+
+	pin->num_mux_nids = snd_hdac_get_connections(&hdac->hdac, pin->nid,
+			pin->mux_nids, HDA_MAX_CONNECTIONS);
+	if (pin->num_mux_nids == 0)
+		dev_warn(&hdac->hdac.dev, "No connections found for pin: %d\n", pin->nid);
+
+	dev_dbg(&hdac->hdac.dev, "num_mux_nids %d for pin: %d\n",
+			pin->num_mux_nids, pin->nid);
+
+	return pin->num_mux_nids;
+}
+
+static inline struct hdac_hdmi_pin *hdac_hdmi_get_pin(
+			struct hdac_ext_device *edev,
+			struct snd_soc_dapm_path *p,
+			struct hdac_hdmi_cvt *cvt)
+{
+	struct hdac_hdmi_priv *hdmi = edev->private_data;
+	struct hdac_hdmi_pin *pin;
+	hda_nid_t *nid;
+	int ret, i;
+
+	nid = (hda_nid_t *)p->sink->priv;
+	list_for_each_entry(pin, &hdmi->pin_list, head) {
+		if (pin->nid == *nid) {
+			ret = hdac_hdmi_query_pin_connlist(edev, pin);
+			if (ret < 0)
+				continue;
+
+			for (i = 0; i < pin->num_mux_nids; i++) {
+				if (pin->mux_nids[i] == cvt->nid)
+					return pin;
+			}
+		}
+	}
+
+	return NULL;
+}
+
+/*
+ * This queries mux widgets in each sink path of the dai widget and returns
+ * a matching pin widget to which the stream may be routed.
+ *
+ * The converter may be input to multiple pin muxes. So each
+ * pin mux (basically each pin widget) is queried to identify if
+ * the converter as one of the input, then the first pin match
+ * is selected for rendering.
+ *
+ * Same stream rendering to multiple pins simultaneously can be done
+ * possibly, but not supported for now.
+ *
+ * So return the first pin connected
+ */
+static struct hdac_hdmi_pin *hdac_hdmi_get_pin_from_daistream(
+			struct hdac_ext_device *edev,
+			struct snd_soc_dapm_widget *strm_w,
+			struct hdac_hdmi_cvt *cvt)
+{
+	struct snd_soc_dapm_path *p;
+
+	snd_soc_dapm_widget_for_each_sink_path(strm_w, p) {
+		if (!p->connect)
+			continue;
+
+		if (strstr(p->sink->name, "Mux"))
+			return hdac_hdmi_get_pin(edev, p, cvt);
+		else
+			return hdac_hdmi_get_pin_from_daistream(edev, p->sink, cvt);
+	}
+
+	return NULL;
+}
+
 static int hdac_hdmi_pcm_open(struct snd_pcm_substream *substream,
 			struct snd_soc_dai *dai)
 {
 	struct hdac_ext_device *hdac = snd_soc_dai_get_drvdata(dai);
 	struct hdac_hdmi_priv *hdmi = hdac->private_data;
 	struct hdac_hdmi_dai_pin_map *dai_map;
+	struct hdac_hdmi_cvt *cvt;
+	struct hdac_hdmi_pin *pin;
 	int ret;
 
 	if (dai->id > 0) {
@@ -384,26 +499,32 @@ static int hdac_hdmi_pcm_open(struct snd_pcm_substream *substream,
 
 	dai_map = &hdmi->dai_map[dai->id];
 
-	if ((!dai_map->pin->eld.monitor_present) ||
-		(!dai_map->pin->eld.eld_valid)) {
-		dev_err(&hdac->hdac.dev, "Failed: montior present? %d eld valid?: %d\n",
-				dai_map->pin->eld.monitor_present,
-				dai_map->pin->eld.eld_valid);
+	cvt = dai_map->cvt;
+	pin = hdac_hdmi_get_pin_from_daistream(hdac, dai->playback_widget, cvt);
+	if (!pin)
+		return -EIO;
+
+	if ((!pin->eld.monitor_present) ||
+		(!pin->eld.eld_valid)) {
+		dev_err(&hdac->hdac.dev,
+			"failed: montior present? %d eld valid?: %d for pin: %d\n",
+			pin->eld.monitor_present, pin->eld.eld_valid, pin->nid);
 		return -ENODEV;
 	}
 
-	hdac_hdmi_set_power_state(hdac, dai_map, AC_PWRST_D0);
+	dai_map->pin = pin;
 
-	snd_hdac_codec_write(&hdac->hdac, dai_map->pin->nid, 0,
-			AC_VERB_SET_AMP_GAIN_MUTE, AMP_OUT_UNMUTE);
+	ret = hdac_hdmi_enable_pin(hdac, dai_map);
+	if (ret < 0)
+		return ret;
 
 	ret = hdac_hdmi_eld_limit_formats(substream->runtime,
-				dai_map->pin->eld.eld_buffer);
+				pin->eld.eld_buffer);
 	if (ret < 0)
 		return ret;
 
 	return snd_pcm_hw_constraint_eld(substream->runtime,
-				dai_map->pin->eld.eld_buffer);
+				pin->eld.eld_buffer);
 }
 
 static void hdac_hdmi_pcm_close(struct snd_pcm_substream *substream,
@@ -419,6 +540,8 @@ static void hdac_hdmi_pcm_close(struct snd_pcm_substream *substream,
 
 	snd_hdac_codec_write(&hdac->hdac, dai_map->pin->nid, 0,
 			AC_VERB_SET_AMP_GAIN_MUTE, AMP_OUT_MUTE);
+
+	dai_map->pin = NULL;
 }
 
 static int
@@ -441,28 +564,6 @@ hdac_hdmi_query_cvt_params(struct hdac_device *hdac, struct hdac_hdmi_cvt *cvt)
 	return err;
 }
 
-static int hdac_hdmi_query_pin_connlist(struct hdac_ext_device *hdac,
-					struct hdac_hdmi_pin *pin)
-{
-	if (!(get_wcaps(&hdac->hdac, pin->nid) & AC_WCAP_CONN_LIST)) {
-		dev_warn(&hdac->hdac.dev,
-			"HDMI: pin %d wcaps %#x does not support connection list\n",
-			pin->nid, get_wcaps(&hdac->hdac, pin->nid));
-		return -EINVAL;
-	}
-
-	pin->num_mux_nids = snd_hdac_get_connections(&hdac->hdac, pin->nid,
-			pin->mux_nids, HDA_MAX_CONNECTIONS);
-	if (pin->num_mux_nids == 0)
-		dev_warn(&hdac->hdac.dev, "No connections found for pin: %d\n",
-								pin->nid);
-
-	dev_dbg(&hdac->hdac.dev, "num_mux_nids %d for pin: %d\n",
-			pin->num_mux_nids, pin->nid);
-
-	return pin->num_mux_nids;
-}
-
 static void hdac_hdmi_fill_widget_info(struct device *dev,
 				struct snd_soc_dapm_widget *w,
 				enum snd_soc_dapm_type id, void *priv,
@@ -686,40 +787,33 @@ static int create_fill_widget_route_map(struct snd_soc_dapm_context *dapm)
 static int hdac_hdmi_init_dai_map(struct hdac_ext_device *edev)
 {
 	struct hdac_hdmi_priv *hdmi = edev->private_data;
-	struct hdac_hdmi_dai_pin_map *dai_map = &hdmi->dai_map[0];
+	struct hdac_hdmi_dai_pin_map *dai_map;
 	struct hdac_hdmi_cvt *cvt;
-	struct hdac_hdmi_pin *pin;
+	int dai_id = 0;
 
-	if (list_empty(&hdmi->cvt_list) || list_empty(&hdmi->pin_list))
+	if (list_empty(&hdmi->cvt_list))
 		return -EINVAL;
 
-	/*
-	 * Currently on board only 1 pin and 1 converter is enabled for
-	 * simplification, more will be added eventually
-	 * So using fixed map for dai_id:pin:cvt
-	 */
-	cvt = list_first_entry(&hdmi->cvt_list, struct hdac_hdmi_cvt, head);
-	pin = list_first_entry(&hdmi->pin_list, struct hdac_hdmi_pin, head);
-
-	dai_map->dai_id = 0;
-	dai_map->pin = pin;
-
-	dai_map->cvt = cvt;
+	list_for_each_entry(cvt, &hdmi->cvt_list, head) {
+		dai_map = &hdmi->dai_map[dai_id];
+		dai_map->dai_id = dai_id;
+		dai_map->cvt = cvt;
 
-	/* Enable out path for this pin widget */
-	snd_hdac_codec_write(&edev->hdac, pin->nid, 0,
-			AC_VERB_SET_PIN_WIDGET_CONTROL, PIN_OUT);
+		/* Enable transmission */
+		snd_hdac_codec_write(&edev->hdac, cvt->nid, 0,
+				AC_VERB_SET_DIGI_CONVERT_1, 1);
 
-	/* Enable transmission */
-	snd_hdac_codec_write(&edev->hdac, cvt->nid, 0,
-			AC_VERB_SET_DIGI_CONVERT_1, 1);
+		/* Category Code (CC) to zero */
+		snd_hdac_codec_write(&edev->hdac, cvt->nid, 0,
+				AC_VERB_SET_DIGI_CONVERT_2, 0);
 
-	/* Category Code (CC) to zero */
-	snd_hdac_codec_write(&edev->hdac, cvt->nid, 0,
-			AC_VERB_SET_DIGI_CONVERT_2, 0);
+		dai_id++;
 
-	snd_hdac_codec_write(&edev->hdac, pin->nid, 0,
-			AC_VERB_SET_CONNECT_SEL, 0);
+		if (dai_id == HDA_MAX_CVTS) {
+			dev_warn(&edev->hdac.dev, "Max dais supported: %d\n", dai_id);
+			break;
+		}
+	}
 
 	return 0;
 }
-- 
1.9.1

[PATCH v3 10/15] drm/edid: Add API to help find connection type

[Subhransu S. Prusty]

To fill the audio infoframe it is required to identify the
connection type as DP or HDMI. This patch adds an API which
parses ELD and returns the display type of connected.

Signed-off-by: Subhransu S. Prusty <subhransu.s.prusty@intel.com>
Signed-off-by: Vinod Koul <vinod.koul@intel.com>
Cc: David Airlie <airlied@linux.ie>
Cc: dri-devel@lists.freedesktop.org
Cc: Daniel Vetter <daniel.vetter@intel.com>
---
 include/drm/drm_edid.h | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/include/drm/drm_edid.h b/include/drm/drm_edid.h
index 2af9769..8c537a0 100644
--- a/include/drm/drm_edid.h
+++ b/include/drm/drm_edid.h
@@ -403,6 +403,18 @@ static inline int drm_eld_size(const uint8_t *eld)
 	return DRM_ELD_HEADER_BLOCK_SIZE + eld[DRM_ELD_BASELINE_ELD_LEN] * 4;
 }
 
+/**
+ * drm_eld_get_conn_type - Get device type hdmi/dp connected
+ * @eld: pointer to an eld memory structure
+ *
+ * The caller need to use %DRM_ELD_CONN_TYPE_HDMI or %DRM_ELD_CONN_TYPE_DP to
+ * identify the display type connected.
+ */
+static inline u8 drm_eld_get_conn_type(const uint8_t *eld)
+{
+	return eld[DRM_ELD_SAD_COUNT_CONN_TYPE] & DRM_ELD_CONN_TYPE_MASK;
+}
+
 struct edid *drm_do_get_edid(struct drm_connector *connector,
 	int (*get_edid_block)(void *data, u8 *buf, unsigned int block,
 			      size_t len),
-- 
1.9.1

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v3 10/15] drm/edid: Add API to help find connection type

[Jani Nikula]

On Mon, 07 Dec 2015, "Subhransu S. Prusty" <subhransu.s.prusty@intel.com> wrote:
> To fill the audio infoframe it is required to identify the
> connection type as DP or HDMI. This patch adds an API which
> parses ELD and returns the display type of connected.
>
> Signed-off-by: Subhransu S. Prusty <subhransu.s.prusty@intel.com>
> Signed-off-by: Vinod Koul <vinod.koul@intel.com>
> Cc: David Airlie <airlied@linux.ie>
> Cc: dri-devel@lists.freedesktop.org
> Cc: Daniel Vetter <daniel.vetter@intel.com>

Reviewed-by: Jani Nikula <jani.nikula@intel.com>


> ---
>  include/drm/drm_edid.h | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
>
> diff --git a/include/drm/drm_edid.h b/include/drm/drm_edid.h
> index 2af9769..8c537a0 100644
> --- a/include/drm/drm_edid.h
> +++ b/include/drm/drm_edid.h
> @@ -403,6 +403,18 @@ static inline int drm_eld_size(const uint8_t *eld)
>  	return DRM_ELD_HEADER_BLOCK_SIZE + eld[DRM_ELD_BASELINE_ELD_LEN] * 4;
>  }
>  
> +/**
> + * drm_eld_get_conn_type - Get device type hdmi/dp connected
> + * @eld: pointer to an eld memory structure
> + *
> + * The caller need to use %DRM_ELD_CONN_TYPE_HDMI or %DRM_ELD_CONN_TYPE_DP to
> + * identify the display type connected.
> + */
> +static inline u8 drm_eld_get_conn_type(const uint8_t *eld)
> +{
> +	return eld[DRM_ELD_SAD_COUNT_CONN_TYPE] & DRM_ELD_CONN_TYPE_MASK;
> +}
> +
>  struct edid *drm_do_get_edid(struct drm_connector *connector,
>  	int (*get_edid_block)(void *data, u8 *buf, unsigned int block,
>  			      size_t len),

-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v3 11/15] ASoC: hdac_hdmi: Add infoframe support for dp audio

[Subhransu S. Prusty]

This uses the get_conn_type API added in the previous patch to
identify the type of display connected and fills the infoframe
accordingly.

Signed-off-by: Subhransu S. Prusty <subhransu.s.prusty@intel.com>
Signed-off-by: Vinod Koul <vinod.koul@intel.com>
---
 sound/soc/codecs/hdac_hdmi.c | 70 +++++++++++++++++++++++++++++++++++++-------
 1 file changed, 60 insertions(+), 10 deletions(-)

diff --git a/sound/soc/codecs/hdac_hdmi.c b/sound/soc/codecs/hdac_hdmi.c
index 2d971c4..32d1938 100644
--- a/sound/soc/codecs/hdac_hdmi.c
+++ b/sound/soc/codecs/hdac_hdmi.c
@@ -238,27 +238,71 @@ hdac_hdmi_set_dip_index(struct hdac_ext_device *hdac, hda_nid_t pin_nid,
 				AC_VERB_SET_HDMI_DIP_INDEX, val);
 }
 
+struct dp_audio_infoframe {
+	u8 type; /* 0x84 */
+	u8 len;  /* 0x1b */
+	u8 ver;  /* 0x11 << 2 */
+
+	u8 CC02_CT47;	/* match with HDMI infoframe from this on */
+	u8 SS01_SF24;
+	u8 CXT04;
+	u8 CA;
+	u8 LFEPBL01_LSV36_DM_INH7;
+};
+
 static int hdac_hdmi_setup_audio_infoframe(struct hdac_ext_device *hdac,
 				hda_nid_t cvt_nid, hda_nid_t pin_nid)
 {
 	uint8_t buffer[HDMI_INFOFRAME_HEADER_SIZE + HDMI_AUDIO_INFOFRAME_SIZE];
 	struct hdmi_audio_infoframe frame;
-	u8 *dip = (u8 *)&frame;
+	struct dp_audio_infoframe dp_ai;
+	struct hdac_hdmi_priv *hdmi = hdac->private_data;
+	struct hdac_hdmi_pin *pin;
+	u8 *dip;
 	int ret;
 	int i;
+	const u8 *eld_buf;
+	u8 conn_type;
+	int channels = 2;
 
-	hdmi_audio_infoframe_init(&frame);
+	list_for_each_entry(pin, &hdmi->pin_list, head) {
+		if (pin->nid == pin_nid)
+			break;
+	}
 
-	/* Default stereo for now */
-	frame.channels = 2;
+	eld_buf = pin->eld.eld_buffer;
+	conn_type = drm_eld_get_conn_type(eld_buf);
 
 	/* setup channel count */
 	snd_hdac_codec_write(&hdac->hdac, cvt_nid, 0,
-			    AC_VERB_SET_CVT_CHAN_COUNT, frame.channels - 1);
+			    AC_VERB_SET_CVT_CHAN_COUNT, channels - 1);
 
-	ret = hdmi_audio_infoframe_pack(&frame, buffer, sizeof(buffer));
-	if (ret < 0)
-		return ret;
+	if (conn_type == DRM_ELD_CONN_TYPE_HDMI) {
+		hdmi_audio_infoframe_init(&frame);
+
+		/* Default stereo for now */
+		frame.channels = channels;
+
+		ret = hdmi_audio_infoframe_pack(&frame, buffer, sizeof(buffer));
+		if (ret < 0)
+			return ret;
+
+		dip = (u8 *)&frame;
+
+	} else if (conn_type == DRM_ELD_CONN_TYPE_DP) {
+		memset(&dp_ai, 0, sizeof(dp_ai));
+		dp_ai.type	= 0x84;
+		dp_ai.len	= 0x1b;
+		dp_ai.ver	= 0x11 << 2;
+		dp_ai.CC02_CT47	= channels - 1;
+		dp_ai.CA	= 0;
+
+		dip = (u8 *)&dp_ai;
+	} else {
+		dev_err(&hdac->hdac.dev, "Invalid connection type: %d\n",
+						conn_type);
+		return -EIO;
+	}
 
 	/* stop infoframe transmission */
 	hdac_hdmi_set_dip_index(hdac, pin_nid, 0x0, 0x0);
@@ -268,9 +312,15 @@ static int hdac_hdmi_setup_audio_infoframe(struct hdac_ext_device *hdac,
 
 	/*  Fill infoframe. Index auto-incremented */
 	hdac_hdmi_set_dip_index(hdac, pin_nid, 0x0, 0x0);
-	for (i = 0; i < sizeof(frame); i++)
-		snd_hdac_codec_write(&hdac->hdac, pin_nid, 0,
+	if (conn_type == DRM_ELD_CONN_TYPE_HDMI) {
+		for (i = 0; i < sizeof(frame); i++)
+			snd_hdac_codec_write(&hdac->hdac, pin_nid, 0,
 				AC_VERB_SET_HDMI_DIP_DATA, dip[i]);
+	} else {
+		for (i = 0; i < sizeof(dp_ai); i++)
+			snd_hdac_codec_write(&hdac->hdac, pin_nid, 0,
+				AC_VERB_SET_HDMI_DIP_DATA, dip[i]);
+	}
 
 	/* Start infoframe */
 	hdac_hdmi_set_dip_index(hdac, pin_nid, 0x0, 0x0);
-- 
1.9.1

[PATCH v3 12/15] ASoC: hdac_hdmi: Add codec suspend/resume handler

[Subhransu S. Prusty]

From: Jeeja KP <jeeja.kp@intel.com>

Need to add enabling of all pins and DP1.2 feature again after
resume. Also during the device in suspended state ELD notify
callback is not processed. So add ELD check for all pins here.

Signed-off-by: Jeeja KP <jeeja.kp@intel.com>
Signed-off-by: Subhransu S. Prusty <subhransu.s.prusty@intel.com>
Signed-off-by: Vinod Koul <vinod.koul@intel.com>
---
 sound/soc/codecs/hdac_hdmi.c | 25 +++++++++++++++++++++++++
 1 file changed, 25 insertions(+)

diff --git a/sound/soc/codecs/hdac_hdmi.c b/sound/soc/codecs/hdac_hdmi.c
index 32d1938..e64e505 100644
--- a/sound/soc/codecs/hdac_hdmi.c
+++ b/sound/soc/codecs/hdac_hdmi.c
@@ -1214,9 +1214,34 @@ static int hdmi_codec_remove(struct snd_soc_codec *codec)
 	return 0;
 }
 
+#ifdef CONFIG_PM
+static int hdmi_codec_resume(struct snd_soc_codec *codec)
+{
+	struct hdac_ext_device *edev = snd_soc_codec_get_drvdata(codec);
+	struct hdac_hdmi_priv *hdmi = edev->private_data;
+	struct hdac_hdmi_pin *pin;
+
+	hdac_hdmi_skl_enable_all_pins(&edev->hdac);
+	hdac_hdmi_skl_enable_dp12(&edev->hdac);
+
+	/*
+	 * As the ELD notify callback request is not entertained while the
+	 * device is in suspend state. Need to manually check detection of
+	 * all pins here.
+	 */
+	list_for_each_entry(pin, &hdmi->pin_list, head)
+		hdac_hdmi_present_sense(pin, 1);
+
+	return 0;
+}
+#else
+#define hdmi_codec_resume NULL
+#endif
+
 static struct snd_soc_codec_driver hdmi_hda_codec = {
 	.probe		= hdmi_codec_probe,
 	.remove		= hdmi_codec_remove,
+	.resume		= hdmi_codec_resume,
 	.idle_bias_off	= true,
 };
 
-- 
1.9.1

[PATCH v3 13/15] ASoC: hdac_hdmi: Fix to keep display active while enumerating codec

[Subhransu S. Prusty]

From: Ramesh Babu <ramesh.babu@intel.com>

The display power reference count is decremented with the first
explicit call to pm_runtime_suspend. Otherwise the display power
reference count is balanced with runtime resume/suspend. Rest
of the time it is kept off.

Signed-off-by: Ramesh Babu <ramesh.babu@intel.com>
Signed-off-by: Subhransu S. Prusty <subhransu.s.prusty@intel.com>
Signed-off-by: Vinod Koul <vinod.koul@intel.com>
---
 sound/soc/codecs/hdac_hdmi.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/sound/soc/codecs/hdac_hdmi.c b/sound/soc/codecs/hdac_hdmi.c
index e64e505..008a29f 100644
--- a/sound/soc/codecs/hdac_hdmi.c
+++ b/sound/soc/codecs/hdac_hdmi.c
@@ -1264,6 +1264,18 @@ static int hdac_hdmi_dev_probe(struct hdac_ext_device *edev)
 	INIT_LIST_HEAD(&hdmi_priv->pin_list);
 	INIT_LIST_HEAD(&hdmi_priv->cvt_list);
 
+	/*
+	 * Turned off in the runtime_suspend during the first explicit
+	 * pm_runtime_suspend call.
+	 */
+	ret = snd_hdac_display_power(edev->hdac.bus, true);
+	if (ret < 0) {
+		dev_err(&edev->hdac.dev,
+			"Cannot turn on display power on i915 err: %d\n",
+			ret);
+		return ret;
+	}
+
 	ret = hdac_hdmi_parse_and_map_nid(edev, &hdmi_dais, &num_dais);
 	if (ret < 0) {
 		dev_err(&codec->dev,
-- 
1.9.1

[PATCH v3 14/15] ASoC: hdac_hdmi: Add jack reporting for user space

[Subhransu S. Prusty]

User space also need be notified of HDMI hotplug notification so
that it can select the appropriate device to route audio.

Signed-off-by: Subhransu S. Prusty <subhransu.s.prusty@intel.com>
Signed-off-by: Vinod Koul <vinod.koul@intel.com>
---
 sound/soc/codecs/hdac_hdmi.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/sound/soc/codecs/hdac_hdmi.c b/sound/soc/codecs/hdac_hdmi.c
index 008a29f..cce2af0 100644
--- a/sound/soc/codecs/hdac_hdmi.c
+++ b/sound/soc/codecs/hdac_hdmi.c
@@ -24,6 +24,7 @@
 #include <linux/hdmi.h>
 #include <drm/drm_edid.h>
 #include <sound/pcm_params.h>
+#include <sound/jack.h>
 #include <sound/soc.h>
 #include <sound/hdaudio_ext.h>
 #include <sound/hda_i915.h>
@@ -73,6 +74,7 @@ struct hdac_hdmi_pin {
 	struct hdac_ext_device *edev;
 	int repoll_count;
 	struct delayed_work work;
+	struct snd_jack *jack;
 };
 
 struct hdac_hdmi_dai_pin_map {
@@ -905,6 +907,7 @@ static void hdac_hdmi_present_sense(struct hdac_hdmi_pin *pin, int repoll)
 	pin->eld.eld_valid = !!(val & AC_PINSENSE_ELDV);
 
 	if (!pin->eld.monitor_present || !pin->eld.eld_valid) {
+		snd_jack_report(pin->jack, 0);
 		dev_info(&edev->hdac.dev, "%s: disconnect or eld_invalid\n",
 				__func__);
 		goto put_hdac_device;
@@ -915,6 +918,7 @@ static void hdac_hdmi_present_sense(struct hdac_hdmi_pin *pin, int repoll)
 		if (hdac_hdmi_get_eld(&edev->hdac, pin->nid,
 				pin->eld.eld_buffer,
 				&pin->eld.eld_size) == 0) {
+			snd_jack_report(pin->jack, SND_JACK_AVOUT);
 			print_hex_dump_bytes("Eld: ", DUMP_PREFIX_OFFSET,
 					pin->eld.eld_buffer, pin->eld.eld_size);
 		} else {
@@ -1173,6 +1177,7 @@ static int hdmi_codec_probe(struct snd_soc_codec *codec)
 	struct snd_soc_dapm_context *dapm =
 		snd_soc_component_get_dapm(&codec->component);
 	struct hdac_hdmi_pin *pin;
+	char jack_name[NAME_SIZE];
 	int ret;
 
 	edev->scodec = codec;
@@ -1189,8 +1194,12 @@ static int hdmi_codec_probe(struct snd_soc_codec *codec)
 		return ret;
 	}
 
-	list_for_each_entry(pin, &hdmi->pin_list, head)
+	list_for_each_entry(pin, &hdmi->pin_list, head) {
+		sprintf(jack_name, "HDMI/DP, Pin=%d Jack", pin->nid);
+		snd_jack_new(dapm->card->snd_card, jack_name, SND_JACK_AVOUT,
+				&pin->jack, true, false);
 		hdac_hdmi_present_sense(pin, 1);
+	}
 
 	/* Imp: Store the card pointer in hda_codec */
 	edev->card = dapm->card->snd_card;
-- 
1.9.1

Re: [PATCH v3 14/15] ASoC: hdac_hdmi: Add jack reporting for user space

[Takashi Iwai]

On Mon, 07 Dec 2015 22:24:28 +0100,
Subhransu S. Prusty wrote:
> 
> User space also need be notified of HDMI hotplug notification so
> that it can select the appropriate device to route audio.
> 
> Signed-off-by: Subhransu S. Prusty <subhransu.s.prusty@intel.com>
> Signed-off-by: Vinod Koul <vinod.koul@intel.com>
> ---
>  sound/soc/codecs/hdac_hdmi.c | 11 ++++++++++-
>  1 file changed, 10 insertions(+), 1 deletion(-)
> 
> diff --git a/sound/soc/codecs/hdac_hdmi.c b/sound/soc/codecs/hdac_hdmi.c
> index 008a29f..cce2af0 100644
> --- a/sound/soc/codecs/hdac_hdmi.c
> +++ b/sound/soc/codecs/hdac_hdmi.c
> @@ -24,6 +24,7 @@
>  #include <linux/hdmi.h>
>  #include <drm/drm_edid.h>
>  #include <sound/pcm_params.h>
> +#include <sound/jack.h>
>  #include <sound/soc.h>
>  #include <sound/hdaudio_ext.h>
>  #include <sound/hda_i915.h>
> @@ -73,6 +74,7 @@ struct hdac_hdmi_pin {
>  	struct hdac_ext_device *edev;
>  	int repoll_count;
>  	struct delayed_work work;
> +	struct snd_jack *jack;
>  };
>  
>  struct hdac_hdmi_dai_pin_map {
> @@ -905,6 +907,7 @@ static void hdac_hdmi_present_sense(struct hdac_hdmi_pin *pin, int repoll)
>  	pin->eld.eld_valid = !!(val & AC_PINSENSE_ELDV);
>  
>  	if (!pin->eld.monitor_present || !pin->eld.eld_valid) {
> +		snd_jack_report(pin->jack, 0);
>  		dev_info(&edev->hdac.dev, "%s: disconnect or eld_invalid\n",
>  				__func__);
>  		goto put_hdac_device;
> @@ -915,6 +918,7 @@ static void hdac_hdmi_present_sense(struct hdac_hdmi_pin *pin, int repoll)
>  		if (hdac_hdmi_get_eld(&edev->hdac, pin->nid,
>  				pin->eld.eld_buffer,
>  				&pin->eld.eld_size) == 0) {
> +			snd_jack_report(pin->jack, SND_JACK_AVOUT);
>  			print_hex_dump_bytes("Eld: ", DUMP_PREFIX_OFFSET,
>  					pin->eld.eld_buffer, pin->eld.eld_size);
>  		} else {
> @@ -1173,6 +1177,7 @@ static int hdmi_codec_probe(struct snd_soc_codec *codec)
>  	struct snd_soc_dapm_context *dapm =
>  		snd_soc_component_get_dapm(&codec->component);
>  	struct hdac_hdmi_pin *pin;
> +	char jack_name[NAME_SIZE];
>  	int ret;
>  
>  	edev->scodec = codec;
> @@ -1189,8 +1194,12 @@ static int hdmi_codec_probe(struct snd_soc_codec *codec)
>  		return ret;
>  	}
>  
> -	list_for_each_entry(pin, &hdmi->pin_list, head)
> +	list_for_each_entry(pin, &hdmi->pin_list, head) {
> +		sprintf(jack_name, "HDMI/DP, Pin=%d Jack", pin->nid);

Such a name makes sense only to be compatible for PA, and then this
string isn't compatible.  Note that it's not about pin but for PCM
stream.  You may wonder why it matters -- see the whole discussion on
MST support.


Takashi

Re: [PATCH v3 14/15] ASoC: hdac_hdmi: Add jack reporting for user space

[Vinod Koul]

On Mon, Dec 07, 2015 at 05:18:06PM +0100, Takashi Iwai wrote:
> On Mon, 07 Dec 2015 22:24:28 +0100,
> > +		sprintf(jack_name, "HDMI/DP, Pin=%d Jack", pin->nid);
> 
> Such a name makes sense only to be compatible for PA, and then this
> string isn't compatible.  Note that it's not about pin but for PCM
> stream.  You may wonder why it matters -- see the whole discussion on
> MST support.

So in our case we have added only one PCM Backend in DSP. In codec we have
one DAI supported, thus creating a DAIlink

The Codec DAI is mapped to a stream which we fix to a CVT. Future we
will add two more streams which are mapped to other two CVTs.

We use MUX controls to allow user to specfiy how CVT and PINs are connected
togther, this way we can route stream to any pin.

Extending this concept, behind a pin for MST there may be different 'ports',
right? Shouldn't that be another mux configuration :) So we can treat three
streams coming to codec to be routed to any pin and then any port. Since we
have only 3 CVTs we will have only 3 streams...

Back on present :), what do you recommend for us for jack name.

Thanks
-- 
~Vinod

Re: [PATCH v3 14/15] ASoC: hdac_hdmi: Add jack reporting for user space

[Takashi Iwai]

On Tue, 08 Dec 2015 07:31:36 +0100,
Vinod Koul wrote:
> 
> On Mon, Dec 07, 2015 at 05:18:06PM +0100, Takashi Iwai wrote:
> > On Mon, 07 Dec 2015 22:24:28 +0100,
> > > +		sprintf(jack_name, "HDMI/DP, Pin=%d Jack", pin->nid);
> > 
> > Such a name makes sense only to be compatible for PA, and then this
> > string isn't compatible.  Note that it's not about pin but for PCM
> > stream.  You may wonder why it matters -- see the whole discussion on
> > MST support.
> 
> So in our case we have added only one PCM Backend in DSP. In codec we have
> one DAI supported, thus creating a DAIlink
> 
> The Codec DAI is mapped to a stream which we fix to a CVT. Future we
> will add two more streams which are mapped to other two CVTs.
> 
> We use MUX controls to allow user to specfiy how CVT and PINs are connected
> togther, this way we can route stream to any pin.
> 
> Extending this concept, behind a pin for MST there may be different 'ports',
> right? Shouldn't that be another mux configuration :) So we can treat three
> streams coming to codec to be routed to any pin and then any port. Since we
> have only 3 CVTs we will have only 3 streams...

You can't do that easily.  Remember that you may connect up to 64
different devices at the same time per pin, and this can switch on the
fly.  How would you implement a MUX?

Though, the max number of converters is limited, thus what you can
actually use is defined by this constraint.  Due to this, we'll likely
manage MST for the legacy HDA by assigning MST devs dynamically to
pins in a certain procedure to make things compatible.

> Back on present :), what do you recommend for us for jack name.

Depends on your purpose.  If you want to keep it compatible, then use
the compatible string.


HTH,

Takashi

Re: [PATCH v3 14/15] ASoC: hdac_hdmi: Add jack reporting for user space

[Vinod Koul]

On Tue, Dec 08, 2015 at 07:38:46AM +0100, Takashi Iwai wrote:
> On Tue, 08 Dec 2015 07:31:36 +0100,
> Vinod Koul wrote:
> > 
> > On Mon, Dec 07, 2015 at 05:18:06PM +0100, Takashi Iwai wrote:
> > > On Mon, 07 Dec 2015 22:24:28 +0100,
> > > > +		sprintf(jack_name, "HDMI/DP, Pin=%d Jack", pin->nid);
> > > 
> > > Such a name makes sense only to be compatible for PA, and then this
> > > string isn't compatible.  Note that it's not about pin but for PCM
> > > stream.  You may wonder why it matters -- see the whole discussion on
> > > MST support.
> > 
> > So in our case we have added only one PCM Backend in DSP. In codec we have
> > one DAI supported, thus creating a DAIlink
> > 
> > The Codec DAI is mapped to a stream which we fix to a CVT. Future we
> > will add two more streams which are mapped to other two CVTs.
> > 
> > We use MUX controls to allow user to specfiy how CVT and PINs are connected
> > togther, this way we can route stream to any pin.
> > 
> > Extending this concept, behind a pin for MST there may be different 'ports',
> > right? Shouldn't that be another mux configuration :) So we can treat three
> > streams coming to codec to be routed to any pin and then any port. Since we
> > have only 3 CVTs we will have only 3 streams...
> 
> You can't do that easily.  Remember that you may connect up to 64
> different devices at the same time per pin, and this can switch on the
> fly.  How would you implement a MUX?

somehow I was under impression that it is 3.. Btw even if 64 devices can be
connected how many can we render to?

> Though, the max number of converters is limited, thus what you can
> actually use is defined by this constraint.  Due to this, we'll likely
> manage MST for the legacy HDA by assigning MST devs dynamically to
> pins in a certain procedure to make things compatible.

but how many MST devs are you going to create?

> > Back on present :), what do you recommend for us for jack name.
> 
> Depends on your purpose.  If you want to keep it compatible, then use
> the compatible string.

Since compatible string would mean specfying PCM, which we don;t know and is
per machine driver dailink defination, we would go ahead with logical
"HDMI/DP, Pin=%d Jack" names.

Thanks
-- 
~Vinod

Re: [PATCH v3 14/15] ASoC: hdac_hdmi: Add jack reporting for user space

[Takashi Iwai]

On Tue, 08 Dec 2015 08:42:16 +0100,
Vinod Koul wrote:
> 
> On Tue, Dec 08, 2015 at 07:38:46AM +0100, Takashi Iwai wrote:
> > On Tue, 08 Dec 2015 07:31:36 +0100,
> > Vinod Koul wrote:
> > > 
> > > On Mon, Dec 07, 2015 at 05:18:06PM +0100, Takashi Iwai wrote:
> > > > On Mon, 07 Dec 2015 22:24:28 +0100,
> > > > > +		sprintf(jack_name, "HDMI/DP, Pin=%d Jack", pin->nid);
> > > > 
> > > > Such a name makes sense only to be compatible for PA, and then this
> > > > string isn't compatible.  Note that it's not about pin but for PCM
> > > > stream.  You may wonder why it matters -- see the whole discussion on
> > > > MST support.
> > > 
> > > So in our case we have added only one PCM Backend in DSP. In codec we have
> > > one DAI supported, thus creating a DAIlink
> > > 
> > > The Codec DAI is mapped to a stream which we fix to a CVT. Future we
> > > will add two more streams which are mapped to other two CVTs.
> > > 
> > > We use MUX controls to allow user to specfiy how CVT and PINs are connected
> > > togther, this way we can route stream to any pin.
> > > 
> > > Extending this concept, behind a pin for MST there may be different 'ports',
> > > right? Shouldn't that be another mux configuration :) So we can treat three
> > > streams coming to codec to be routed to any pin and then any port. Since we
> > > have only 3 CVTs we will have only 3 streams...
> > 
> > You can't do that easily.  Remember that you may connect up to 64
> > different devices at the same time per pin, and this can switch on the
> > fly.  How would you implement a MUX?
> 
> somehow I was under impression that it is 3.. Btw even if 64 devices can be
> connected how many can we render to?

That's what I meant below.  Although you can may connect quite a lot
of devices, the number of devices that can be actually output at the
same time is limited, actually equal with the number of converters.

> > Though, the max number of converters is limited, thus what you can
> > actually use is defined by this constraint.  Due to this, we'll likely
> > manage MST for the legacy HDA by assigning MST devs dynamically to
> > pins in a certain procedure to make things compatible.
> 
> but how many MST devs are you going to create?

This is exactly the point: for the legacy HDA, instead of creating the
entry for each MST devices, they are assigned dynamically to PCM at
activation.  So the number of devices exposed to user-space is
limited -- or better to say, user-space won't notice the difference.

> > > Back on present :), what do you recommend for us for jack name.
> > 
> > Depends on your purpose.  If you want to keep it compatible, then use
> > the compatible string.
> 
> Since compatible string would mean specfying PCM, which we don;t know and is
> per machine driver dailink defination, we would go ahead with logical
> "HDMI/DP, Pin=%d Jack" names.

But then it makes little sense.  Multiple MST devices can be on the
same pin.


Takashi

Re: [PATCH v3 14/15] ASoC: hdac_hdmi: Add jack reporting for user space

[Vinod Koul]

On Tue, Dec 08, 2015 at 08:52:10AM +0100, Takashi Iwai wrote:
> On Tue, 08 Dec 2015 08:42:16 +0100,
> Vinod Koul wrote:
> > 
> > On Tue, Dec 08, 2015 at 07:38:46AM +0100, Takashi Iwai wrote:
> > > On Tue, 08 Dec 2015 07:31:36 +0100,
> > > Vinod Koul wrote:
> > > > 
> > > > On Mon, Dec 07, 2015 at 05:18:06PM +0100, Takashi Iwai wrote:
> > > > > On Mon, 07 Dec 2015 22:24:28 +0100,
> > > > > > +		sprintf(jack_name, "HDMI/DP, Pin=%d Jack", pin->nid);
> > > > > 
> > > > > Such a name makes sense only to be compatible for PA, and then this
> > > > > string isn't compatible.  Note that it's not about pin but for PCM
> > > > > stream.  You may wonder why it matters -- see the whole discussion on
> > > > > MST support.
> > > > 
> > > > So in our case we have added only one PCM Backend in DSP. In codec we have
> > > > one DAI supported, thus creating a DAIlink
> > > > 
> > > > The Codec DAI is mapped to a stream which we fix to a CVT. Future we
> > > > will add two more streams which are mapped to other two CVTs.
> > > > 
> > > > We use MUX controls to allow user to specfiy how CVT and PINs are connected
> > > > togther, this way we can route stream to any pin.
> > > > 
> > > > Extending this concept, behind a pin for MST there may be different 'ports',
> > > > right? Shouldn't that be another mux configuration :) So we can treat three
> > > > streams coming to codec to be routed to any pin and then any port. Since we
> > > > have only 3 CVTs we will have only 3 streams...
> > > 
> > > You can't do that easily.  Remember that you may connect up to 64
> > > different devices at the same time per pin, and this can switch on the
> > > fly.  How would you implement a MUX?
> > 
> > somehow I was under impression that it is 3.. Btw even if 64 devices can be
> > connected how many can we render to?
> 
> That's what I meant below.  Although you can may connect quite a lot
> of devices, the number of devices that can be actually output at the
> same time is limited, actually equal with the number of converters.

Okay got it now, that is how my 3 limit for MST is coming from as we
have 3 CVTs so we can actually stream to only 3 devices per port
although someone may theoretically connect 64!

> > > Though, the max number of converters is limited, thus what you can
> > > actually use is defined by this constraint.  Due to this, we'll likely
> > > manage MST for the legacy HDA by assigning MST devs dynamically to
> > > pins in a certain procedure to make things compatible.
> > 
> > but how many MST devs are you going to create?
> 
> This is exactly the point: for the legacy HDA, instead of creating the
> entry for each MST devices, they are assigned dynamically to PCM at
> activation.  So the number of devices exposed to user-space is
> limited -- or better to say, user-space won't notice the difference.

And how many PCMs are you proposing for MST?

> > > > Back on present :), what do you recommend for us for jack name.
> > > 
> > > Depends on your purpose.  If you want to keep it compatible, then use
> > > the compatible string.
> > 
> > Since compatible string would mean specfying PCM, which we don;t know and is
> > per machine driver dailink defination, we would go ahead with logical
> > "HDMI/DP, Pin=%d Jack" names.
> 
> But then it makes little sense.  Multiple MST devices can be on the
> same pin.

Well am not supporting MST at the moment. How is legacy handling this,
how do you report jack for 64 X 3 devices :)

Or, are you reporting jack for PCMs based on connection to pin/device, I
think latter.

But then you would hit limit of PCMs, I assume you maybe doing 9 PCMs for
this, so if someone connects 10th device we won't report it's jack?

-- 
~Vinod

Re: [PATCH v3 14/15] ASoC: hdac_hdmi: Add jack reporting for user space

[Takashi Iwai]

On Tue, 08 Dec 2015 09:38:31 +0100,
Vinod Koul wrote:
> 
> On Tue, Dec 08, 2015 at 08:52:10AM +0100, Takashi Iwai wrote:
> > On Tue, 08 Dec 2015 08:42:16 +0100,
> > Vinod Koul wrote:
> > > 
> > > On Tue, Dec 08, 2015 at 07:38:46AM +0100, Takashi Iwai wrote:
> > > > On Tue, 08 Dec 2015 07:31:36 +0100,
> > > > Vinod Koul wrote:
> > > > > 
> > > > > On Mon, Dec 07, 2015 at 05:18:06PM +0100, Takashi Iwai wrote:
> > > > > > On Mon, 07 Dec 2015 22:24:28 +0100,
> > > > > > > +		sprintf(jack_name, "HDMI/DP, Pin=%d Jack", pin->nid);
> > > > > > 
> > > > > > Such a name makes sense only to be compatible for PA, and then this
> > > > > > string isn't compatible.  Note that it's not about pin but for PCM
> > > > > > stream.  You may wonder why it matters -- see the whole discussion on
> > > > > > MST support.
> > > > > 
> > > > > So in our case we have added only one PCM Backend in DSP. In codec we have
> > > > > one DAI supported, thus creating a DAIlink
> > > > > 
> > > > > The Codec DAI is mapped to a stream which we fix to a CVT. Future we
> > > > > will add two more streams which are mapped to other two CVTs.
> > > > > 
> > > > > We use MUX controls to allow user to specfiy how CVT and PINs are connected
> > > > > togther, this way we can route stream to any pin.
> > > > > 
> > > > > Extending this concept, behind a pin for MST there may be different 'ports',
> > > > > right? Shouldn't that be another mux configuration :) So we can treat three
> > > > > streams coming to codec to be routed to any pin and then any port. Since we
> > > > > have only 3 CVTs we will have only 3 streams...
> > > > 
> > > > You can't do that easily.  Remember that you may connect up to 64
> > > > different devices at the same time per pin, and this can switch on the
> > > > fly.  How would you implement a MUX?
> > > 
> > > somehow I was under impression that it is 3.. Btw even if 64 devices can be
> > > connected how many can we render to?
> > 
> > That's what I meant below.  Although you can may connect quite a lot
> > of devices, the number of devices that can be actually output at the
> > same time is limited, actually equal with the number of converters.
> 
> Okay got it now, that is how my 3 limit for MST is coming from as we
> have 3 CVTs so we can actually stream to only 3 devices per port
> although someone may theoretically connect 64!
> 
> > > > Though, the max number of converters is limited, thus what you can
> > > > actually use is defined by this constraint.  Due to this, we'll likely
> > > > manage MST for the legacy HDA by assigning MST devs dynamically to
> > > > pins in a certain procedure to make things compatible.
> > > 
> > > but how many MST devs are you going to create?
> > 
> > This is exactly the point: for the legacy HDA, instead of creating the
> > entry for each MST devices, they are assigned dynamically to PCM at
> > activation.  So the number of devices exposed to user-space is
> > limited -- or better to say, user-space won't notice the difference.
> 
> And how many PCMs are you proposing for MST?

5 for Intel, i.e. Nconv * 2 - 1.  This could be even Nconv, but we
provide the reserved slots just for the compatible behavior that
assumes the static pin/slot assignment.

> > > > > Back on present :), what do you recommend for us for jack name.
> > > > 
> > > > Depends on your purpose.  If you want to keep it compatible, then use
> > > > the compatible string.
> > > 
> > > Since compatible string would mean specfying PCM, which we don;t know and is
> > > per machine driver dailink defination, we would go ahead with logical
> > > "HDMI/DP, Pin=%d Jack" names.
> > 
> > But then it makes little sense.  Multiple MST devices can be on the
> > same pin.
> 
> Well am not supporting MST at the moment. How is legacy handling this,
> how do you report jack for 64 X 3 devices :)
> 
> Or, are you reporting jack for PCMs based on connection to pin/device, I
> think latter.

Yes, the jack reporting is for PCM in this case, which is I mentioned
in the first reply: it's not about pin but PCM.

> But then you would hit limit of PCMs, I assume you maybe doing 9 PCMs for
> this, so if someone connects 10th device we won't report it's jack?

The jack reporting is done actually at the time the audio is
enabled/disabled.  And at this moment, the PCM assignment is assured
as well.  So, to user-space, only the actually usable devices are
exposed no matter how many devices are plugged.


Takashi

Re: [PATCH v3 14/15] ASoC: hdac_hdmi: Add jack reporting for user space

[Vinod Koul]

On Tue, Dec 08, 2015 at 09:42:09AM +0100, Takashi Iwai wrote:
> > Okay got it now, that is how my 3 limit for MST is coming from as we
> > have 3 CVTs so we can actually stream to only 3 devices per port
> > although someone may theoretically connect 64!
> > 
> > > > > Though, the max number of converters is limited, thus what you can
> > > > > actually use is defined by this constraint.  Due to this, we'll likely
> > > > > manage MST for the legacy HDA by assigning MST devs dynamically to
> > > > > pins in a certain procedure to make things compatible.
> > > > 
> > > > but how many MST devs are you going to create?
> > > 
> > > This is exactly the point: for the legacy HDA, instead of creating the
> > > entry for each MST devices, they are assigned dynamically to PCM at
> > > activation.  So the number of devices exposed to user-space is
> > > limited -- or better to say, user-space won't notice the difference.
> > 
> > And how many PCMs are you proposing for MST?
> 
> 5 for Intel, i.e. Nconv * 2 - 1.  This could be even Nconv, but we
> provide the reserved slots just for the compatible behavior that
> assumes the static pin/slot assignment.
> 
> > > > > > Back on present :), what do you recommend for us for jack name.
> > > > > 
> > > > > Depends on your purpose.  If you want to keep it compatible, then use
> > > > > the compatible string.
> > > > 
> > > > Since compatible string would mean specfying PCM, which we don;t know and is
> > > > per machine driver dailink defination, we would go ahead with logical
> > > > "HDMI/DP, Pin=%d Jack" names.
> > > 
> > > But then it makes little sense.  Multiple MST devices can be on the
> > > same pin.
> > 
> > Well am not supporting MST at the moment. How is legacy handling this,
> > how do you report jack for 64 X 3 devices :)
> > 
> > Or, are you reporting jack for PCMs based on connection to pin/device, I
> > think latter.
> 
> Yes, the jack reporting is for PCM in this case, which is I mentioned
> in the first reply: it's not about pin but PCM.
> 
> > But then you would hit limit of PCMs, I assume you maybe doing 9 PCMs for
> > this, so if someone connects 10th device we won't report it's jack?
> 
> The jack reporting is done actually at the time the audio is
> enabled/disabled.  And at this moment, the PCM assignment is assured
> as well.  So, to user-space, only the actually usable devices are
> exposed no matter how many devices are plugged.

Yes that was my thinking based on discussion and patches posted by Libin.
But one more question who does PCM assignment, looking at patches it seems
driver and no way to change that..

For example I have 10 devices connected  and we have 5 PCMs.
I would like to route to 7th device, how would that be done?

Thanks
-- 
~Vinod

Re: [PATCH v3 14/15] ASoC: hdac_hdmi: Add jack reporting for user space

[Takashi Iwai]

On Tue, 08 Dec 2015 11:20:59 +0100,
Vinod Koul wrote:
> 
> On Tue, Dec 08, 2015 at 09:42:09AM +0100, Takashi Iwai wrote:
> > > Okay got it now, that is how my 3 limit for MST is coming from as we
> > > have 3 CVTs so we can actually stream to only 3 devices per port
> > > although someone may theoretically connect 64!
> > > 
> > > > > > Though, the max number of converters is limited, thus what you can
> > > > > > actually use is defined by this constraint.  Due to this, we'll likely
> > > > > > manage MST for the legacy HDA by assigning MST devs dynamically to
> > > > > > pins in a certain procedure to make things compatible.
> > > > > 
> > > > > but how many MST devs are you going to create?
> > > > 
> > > > This is exactly the point: for the legacy HDA, instead of creating the
> > > > entry for each MST devices, they are assigned dynamically to PCM at
> > > > activation.  So the number of devices exposed to user-space is
> > > > limited -- or better to say, user-space won't notice the difference.
> > > 
> > > And how many PCMs are you proposing for MST?
> > 
> > 5 for Intel, i.e. Nconv * 2 - 1.  This could be even Nconv, but we
> > provide the reserved slots just for the compatible behavior that
> > assumes the static pin/slot assignment.
> > 
> > > > > > > Back on present :), what do you recommend for us for jack name.
> > > > > > 
> > > > > > Depends on your purpose.  If you want to keep it compatible, then use
> > > > > > the compatible string.
> > > > > 
> > > > > Since compatible string would mean specfying PCM, which we don;t know and is
> > > > > per machine driver dailink defination, we would go ahead with logical
> > > > > "HDMI/DP, Pin=%d Jack" names.
> > > > 
> > > > But then it makes little sense.  Multiple MST devices can be on the
> > > > same pin.
> > > 
> > > Well am not supporting MST at the moment. How is legacy handling this,
> > > how do you report jack for 64 X 3 devices :)
> > > 
> > > Or, are you reporting jack for PCMs based on connection to pin/device, I
> > > think latter.
> > 
> > Yes, the jack reporting is for PCM in this case, which is I mentioned
> > in the first reply: it's not about pin but PCM.
> > 
> > > But then you would hit limit of PCMs, I assume you maybe doing 9 PCMs for
> > > this, so if someone connects 10th device we won't report it's jack?
> > 
> > The jack reporting is done actually at the time the audio is
> > enabled/disabled.  And at this moment, the PCM assignment is assured
> > as well.  So, to user-space, only the actually usable devices are
> > exposed no matter how many devices are plugged.
> 
> Yes that was my thinking based on discussion and patches posted by Libin.
> But one more question who does PCM assignment, looking at patches it seems
> driver and no way to change that..
> 
> For example I have 10 devices connected  and we have 5 PCMs.
> I would like to route to 7th device, how would that be done?

Manage xrandr to enable the target monitor.  Then one PCM stream will
be switched to the corresponding audio, and it'll be notified to
user-space.


Takashi

Re: [PATCH v3 14/15] ASoC: hdac_hdmi: Add jack reporting for user space

[Vinod Koul]

On Tue, Dec 08, 2015 at 11:28:16AM +0100, Takashi Iwai wrote:
> > Yes that was my thinking based on discussion and patches posted by Libin.
> > But one more question who does PCM assignment, looking at patches it seems
> > driver and no way to change that..
> > 
> > For example I have 10 devices connected  and we have 5 PCMs.
> > I would like to route to 7th device, how would that be done?
> 
> Manage xrandr to enable the target monitor.  Then one PCM stream will
> be switched to the corresponding audio, and it'll be notified to
> user-space.

Thanks, that helps..

Looking at RFC patches I do not see that implemented, so maybe Libin
and you will add that eventually

The question here is how will the switch be handled, can you please
elaborate on that. From xrandr selection how will audio driver be
notified and how will change be done?

-- 
~Vinod

Re: [PATCH v3 14/15] ASoC: hdac_hdmi: Add jack reporting for user space

[Takashi Iwai]

On Tue, 08 Dec 2015 11:42:57 +0100,
Vinod Koul wrote:
> 
> On Tue, Dec 08, 2015 at 11:28:16AM +0100, Takashi Iwai wrote:
> > > Yes that was my thinking based on discussion and patches posted by Libin.
> > > But one more question who does PCM assignment, looking at patches it seems
> > > driver and no way to change that..
> > > 
> > > For example I have 10 devices connected  and we have 5 PCMs.
> > > I would like to route to 7th device, how would that be done?
> > 
> > Manage xrandr to enable the target monitor.  Then one PCM stream will
> > be switched to the corresponding audio, and it'll be notified to
> > user-space.
> 
> Thanks, that helps..
> 
> Looking at RFC patches I do not see that implemented, so maybe Libin
> and you will add that eventually
> 
> The question here is how will the switch be handled, can you please
> elaborate on that. From xrandr selection how will audio driver be
> notified and how will change be done?

The switch itself has to be done voluntarily by user-space,
unfortunately, as in the current form.  We thought of the preemptive
disconnection.  It works for aplay, but screws up PulseAudio, so
no-go.


Takashi

Re: [PATCH v3 14/15] ASoC: hdac_hdmi: Add jack reporting for user space

[Takashi Iwai]

On Tue, 08 Dec 2015 11:51:40 +0100,
Takashi Iwai wrote:
> 
> On Tue, 08 Dec 2015 11:42:57 +0100,
> Vinod Koul wrote:
> > 
> > On Tue, Dec 08, 2015 at 11:28:16AM +0100, Takashi Iwai wrote:
> > > > Yes that was my thinking based on discussion and patches posted by Libin.
> > > > But one more question who does PCM assignment, looking at patches it seems
> > > > driver and no way to change that..
> > > > 
> > > > For example I have 10 devices connected  and we have 5 PCMs.
> > > > I would like to route to 7th device, how would that be done?
> > > 
> > > Manage xrandr to enable the target monitor.  Then one PCM stream will
> > > be switched to the corresponding audio, and it'll be notified to
> > > user-space.
> > 
> > Thanks, that helps..
> > 
> > Looking at RFC patches I do not see that implemented, so maybe Libin
> > and you will add that eventually
> > 
> > The question here is how will the switch be handled, can you please
> > elaborate on that. From xrandr selection how will audio driver be
> > notified and how will change be done?
> 
> The switch itself has to be done voluntarily by user-space,
> unfortunately, as in the current form.  We thought of the preemptive
> disconnection.  It works for aplay, but screws up PulseAudio, so
> no-go.

Maybe the above wasn't clear enough.  Put in that way: the assignment
of PCM stream and the switching of PCM stream are different things.
The former is done at the time the HDA driver gets notified by i915
(actually happens twice, one for disabling and one for enabling).  The
audio driver notifies this to user-space.  Then user-space closes /
reopens a stream accordingly, which is equivalent as switching a PCM
stream.

We've tested the PCM switching in the driver side, but it broke the
compatibility with PA.  So the switching itself is voluntarily done by
user-space.


Takashi

Re: [PATCH v3 14/15] ASoC: hdac_hdmi: Add jack reporting for user space

[Vinod Koul]

On Tue, Dec 08, 2015 at 11:59:41AM +0100, Takashi Iwai wrote:
> On Tue, 08 Dec 2015 11:51:40 +0100,
> Takashi Iwai wrote:
> > 
> > On Tue, 08 Dec 2015 11:42:57 +0100,
> > Vinod Koul wrote:
> > > 
> > > On Tue, Dec 08, 2015 at 11:28:16AM +0100, Takashi Iwai wrote:
> > > > > Yes that was my thinking based on discussion and patches posted by Libin.
> > > > > But one more question who does PCM assignment, looking at patches it seems
> > > > > driver and no way to change that..
> > > > > 
> > > > > For example I have 10 devices connected  and we have 5 PCMs.
> > > > > I would like to route to 7th device, how would that be done?
> > > > 
> > > > Manage xrandr to enable the target monitor.  Then one PCM stream will
> > > > be switched to the corresponding audio, and it'll be notified to
> > > > user-space.
> > > 
> > > Thanks, that helps..
> > > 
> > > Looking at RFC patches I do not see that implemented, so maybe Libin
> > > and you will add that eventually
> > > 
> > > The question here is how will the switch be handled, can you please
> > > elaborate on that. From xrandr selection how will audio driver be
> > > notified and how will change be done?
> > 
> > The switch itself has to be done voluntarily by user-space,
> > unfortunately, as in the current form.  We thought of the preemptive
> > disconnection.  It works for aplay, but screws up PulseAudio, so
> > no-go.
> 
> Maybe the above wasn't clear enough.  Put in that way: the assignment
> of PCM stream and the switching of PCM stream are different things.
> The former is done at the time the HDA driver gets notified by i915
> (actually happens twice, one for disabling and one for enabling).  The
> audio driver notifies this to user-space.  Then user-space closes /
> reopens a stream accordingly, which is equivalent as switching a PCM
> stream.

Thanks for the explanation :)

SO how do you notify, thru alsa jack control. In that case if we had
assigned PCM to pin 5, device 62 and change to pin 5 device 35, how would
userspace view this

> We've tested the PCM switching in the driver side, but it broke the
> compatibility with PA.  So the switching itself is voluntarily done by
> user-space.

-- 
~Vinod

Re: [PATCH v3 14/15] ASoC: hdac_hdmi: Add jack reporting for user space

[Subhransu S. Prusty]

Hi Takashi,

> > 
> > > > > Though, the max number of converters is limited, thus what you can
> > > > > actually use is defined by this constraint.  Due to this, we'll likely
> > > > > manage MST for the legacy HDA by assigning MST devs dynamically to
> > > > > pins in a certain procedure to make things compatible.
> > > > 
> > > > but how many MST devs are you going to create?
> > > 
> > > This is exactly the point: for the legacy HDA, instead of creating the
> > > entry for each MST devices, they are assigned dynamically to PCM at
> > > activation.  So the number of devices exposed to user-space is
> > > limited -- or better to say, user-space won't notice the difference.
> > 
> > And how many PCMs are you proposing for MST?
> 
> 5 for Intel, i.e. Nconv * 2 - 1.  This could be even Nconv, but we
> provide the reserved slots just for the compatible behavior that
> assumes the static pin/slot assignment.
>

How is Nconv * 2 - 1 PCMs are derived? Lets assume, there are 3 cvts and 4pins,
I think it will not work.

-- 
Subhransu

Re: [PATCH v3 14/15] ASoC: hdac_hdmi: Add jack reporting for user space

[Takashi Iwai]

On Wed, 09 Dec 2015 09:03:38 +0100,
Subhransu S. Prusty wrote:
> 
> Hi Takashi,
> 
> > > 
> > > > > > Though, the max number of converters is limited, thus what you can
> > > > > > actually use is defined by this constraint.  Due to this, we'll likely
> > > > > > manage MST for the legacy HDA by assigning MST devs dynamically to
> > > > > > pins in a certain procedure to make things compatible.
> > > > > 
> > > > > but how many MST devs are you going to create?
> > > > 
> > > > This is exactly the point: for the legacy HDA, instead of creating the
> > > > entry for each MST devices, they are assigned dynamically to PCM at
> > > > activation.  So the number of devices exposed to user-space is
> > > > limited -- or better to say, user-space won't notice the difference.
> > > 
> > > And how many PCMs are you proposing for MST?
> > 
> > 5 for Intel, i.e. Nconv * 2 - 1.  This could be even Nconv, but we
> > provide the reserved slots just for the compatible behavior that
> > assumes the static pin/slot assignment.
> >
> 
> How is Nconv * 2 - 1 PCMs are derived? Lets assume, there are 3 cvts and 4pins,
> I think it will not work.

We're talking about Intel case, right?  Then both numbers are same.


Takashi

Re: [PATCH v3 14/15] ASoC: hdac_hdmi: Add jack reporting for user space

[Subhransu S. Prusty]

On Wed, Dec 09, 2015 at 09:13:56AM +0100, Takashi Iwai wrote:
> On Wed, 09 Dec 2015 09:03:38 +0100,
> Subhransu S. Prusty wrote:
> > 
> > Hi Takashi,
> > 
> > > > 
> > > > > > > Though, the max number of converters is limited, thus what you can
> > > > > > > actually use is defined by this constraint.  Due to this, we'll likely
> > > > > > > manage MST for the legacy HDA by assigning MST devs dynamically to
> > > > > > > pins in a certain procedure to make things compatible.
> > > > > > 
> > > > > > but how many MST devs are you going to create?
> > > > > 
> > > > > This is exactly the point: for the legacy HDA, instead of creating the
> > > > > entry for each MST devices, they are assigned dynamically to PCM at
> > > > > activation.  So the number of devices exposed to user-space is
> > > > > limited -- or better to say, user-space won't notice the difference.
> > > > 
> > > > And how many PCMs are you proposing for MST?
> > > 
> > > 5 for Intel, i.e. Nconv * 2 - 1.  This could be even Nconv, but we
> > > provide the reserved slots just for the compatible behavior that
> > > assumes the static pin/slot assignment.
> > >
> > 
> > How is Nconv * 2 - 1 PCMs are derived? Lets assume, there are 3 cvts and 4pins,
> > I think it will not work.
> 
> We're talking about Intel case, right?  Then both numbers are same.

Yes that's true. But was wondering what if it changes in future.

Regards,
Subhransu
> 
> 
> Takashi

-- 

Re: [PATCH v3 14/15] ASoC: hdac_hdmi: Add jack reporting for user space

[Takashi Iwai]

On Wed, 09 Dec 2015 12:14:55 +0100,
Subhransu S. Prusty wrote:
> 
> On Wed, Dec 09, 2015 at 09:13:56AM +0100, Takashi Iwai wrote:
> > On Wed, 09 Dec 2015 09:03:38 +0100,
> > Subhransu S. Prusty wrote:
> > > 
> > > Hi Takashi,
> > > 
> > > > > 
> > > > > > > > Though, the max number of converters is limited, thus what you can
> > > > > > > > actually use is defined by this constraint.  Due to this, we'll likely
> > > > > > > > manage MST for the legacy HDA by assigning MST devs dynamically to
> > > > > > > > pins in a certain procedure to make things compatible.
> > > > > > > 
> > > > > > > but how many MST devs are you going to create?
> > > > > > 
> > > > > > This is exactly the point: for the legacy HDA, instead of creating the
> > > > > > entry for each MST devices, they are assigned dynamically to PCM at
> > > > > > activation.  So the number of devices exposed to user-space is
> > > > > > limited -- or better to say, user-space won't notice the difference.
> > > > > 
> > > > > And how many PCMs are you proposing for MST?
> > > > 
> > > > 5 for Intel, i.e. Nconv * 2 - 1.  This could be even Nconv, but we
> > > > provide the reserved slots just for the compatible behavior that
> > > > assumes the static pin/slot assignment.
> > > >
> > > 
> > > How is Nconv * 2 - 1 PCMs are derived? Lets assume, there are 3 cvts and 4pins,
> > > I think it will not work.
> > 
> > We're talking about Intel case, right?  Then both numbers are same.
> 
> Yes that's true. But was wondering what if it changes in future.

Well, then the formula will be changed, too :)  As mentioned, the
number of PCMs doesn't have to be changed in theory.  The point is
only to map the active connection to the available PCM slot
dynamically, then this is notified to user-space.  We reserve a few
more PCM slots than of now, but it's just for subtle compatibility
reason.


Takashi

[PATCH v3 15/15] ASoC: hdac_hdmi: Fix to enable device configuration in hw_params

[Subhransu S. Prusty]

From: Jeeja KP <jeeja.kp@intel.com>

User can keep the device open and with the device configuration
done in pcm_open, this may fail. Anyway hw_params is the right
place for this. So moving the configuration to hw_params and free
in hw_free. Remove the redundant startup/shutdown ops.

Signed-off-by: Jeeja KP <jeeja.kp@intel.com>
Signed-off-by: Subhransu S. Prusty <subhransu.s.prusty@intel.com>
Signed-off-by: Vinod Koul <vinod.koul@intel.com>
---
 sound/soc/codecs/hdac_hdmi.c | 100 ++++++++++++++++++-------------------------
 1 file changed, 42 insertions(+), 58 deletions(-)

diff --git a/sound/soc/codecs/hdac_hdmi.c b/sound/soc/codecs/hdac_hdmi.c
index cce2af0..cb34c6a 100644
--- a/sound/soc/codecs/hdac_hdmi.c
+++ b/sound/soc/codecs/hdac_hdmi.c
@@ -377,52 +377,6 @@ static int hdac_hdmi_playback_prepare(struct snd_pcm_substream *substream,
 			dai_map->pin->nid, dd->stream_tag, dd->format);
 }
 
-static int hdac_hdmi_set_hw_params(struct snd_pcm_substream *substream,
-	struct snd_pcm_hw_params *hparams, struct snd_soc_dai *dai)
-{
-	struct hdac_ext_device *hdac = snd_soc_dai_get_drvdata(dai);
-	struct hdac_ext_dma_params *dd;
-
-	if (dai->id > 0) {
-		dev_err(&hdac->hdac.dev, "Only one dai supported as of now\n");
-		return -ENODEV;
-	}
-
-	dd = kzalloc(sizeof(*dd), GFP_KERNEL);
-	if (!dd)
-		return -ENOMEM;
-	dd->format = snd_hdac_calc_stream_format(params_rate(hparams),
-			params_channels(hparams), params_format(hparams),
-			24, 0);
-
-	snd_soc_dai_set_dma_data(dai, substream, (void *)dd);
-
-	return 0;
-}
-
-static int hdac_hdmi_playback_cleanup(struct snd_pcm_substream *substream,
-		struct snd_soc_dai *dai)
-{
-	struct hdac_ext_device *edev = snd_soc_dai_get_drvdata(dai);
-	struct hdac_ext_dma_params *dd;
-	struct hdac_hdmi_priv *hdmi = edev->private_data;
-	struct hdac_hdmi_dai_pin_map *dai_map;
-
-	dai_map = &hdmi->dai_map[dai->id];
-
-	snd_hdac_codec_write(&edev->hdac, dai_map->cvt->nid, 0,
-				AC_VERB_SET_CHANNEL_STREAMID, 0);
-	snd_hdac_codec_write(&edev->hdac, dai_map->cvt->nid, 0,
-				AC_VERB_SET_STREAM_FORMAT, 0);
-
-	dd = (struct hdac_ext_dma_params *)snd_soc_dai_get_dma_data(dai, substream);
-	snd_soc_dai_set_dma_data(dai, substream, NULL);
-
-	kfree(dd);
-
-	return 0;
-}
-
 static int hdac_hdmi_enable_pin(struct hdac_ext_device *hdac,
 		struct hdac_hdmi_dai_pin_map *dai_map)
 {
@@ -534,8 +488,8 @@ static struct hdac_hdmi_pin *hdac_hdmi_get_pin_from_daistream(
 	return NULL;
 }
 
-static int hdac_hdmi_pcm_open(struct snd_pcm_substream *substream,
-			struct snd_soc_dai *dai)
+static int hdac_hdmi_set_hw_params(struct snd_pcm_substream *substream,
+	struct snd_pcm_hw_params *hparams, struct snd_soc_dai *dai)
 {
 	struct hdac_ext_device *hdac = snd_soc_dai_get_drvdata(dai);
 	struct hdac_hdmi_priv *hdmi = hdac->private_data;
@@ -543,6 +497,7 @@ static int hdac_hdmi_pcm_open(struct snd_pcm_substream *substream,
 	struct hdac_hdmi_cvt *cvt;
 	struct hdac_hdmi_pin *pin;
 	int ret;
+	struct hdac_ext_dma_params *dd;
 
 	if (dai->id > 0) {
 		dev_err(&hdac->hdac.dev, "Only one dai supported as of now\n");
@@ -575,25 +530,56 @@ static int hdac_hdmi_pcm_open(struct snd_pcm_substream *substream,
 	if (ret < 0)
 		return ret;
 
-	return snd_pcm_hw_constraint_eld(substream->runtime,
-				pin->eld.eld_buffer);
+	ret = snd_pcm_hw_constraint_eld(substream->runtime,
+			dai_map->pin->eld.eld_buffer);
+
+	if (ret < 0)
+		return ret;
+
+	dd = kzalloc(sizeof(*dd), GFP_KERNEL);
+	if (!dd)
+		return -ENOMEM;
+	dd->format = snd_hdac_calc_stream_format(params_rate(hparams),
+			params_channels(hparams), params_format(hparams),
+			24, 0);
+
+	snd_soc_dai_set_dma_data(dai, substream, (void *)dd);
+
+	return 0;
 }
 
-static void hdac_hdmi_pcm_close(struct snd_pcm_substream *substream,
+static int hdac_hdmi_playback_cleanup(struct snd_pcm_substream *substream,
 		struct snd_soc_dai *dai)
 {
-	struct hdac_ext_device *hdac = snd_soc_dai_get_drvdata(dai);
-	struct hdac_hdmi_priv *hdmi = hdac->private_data;
+	struct hdac_ext_device *edev = snd_soc_dai_get_drvdata(dai);
+	struct hdac_ext_dma_params *dd;
+	struct hdac_hdmi_priv *hdmi = edev->private_data;
 	struct hdac_hdmi_dai_pin_map *dai_map;
 
 	dai_map = &hdmi->dai_map[dai->id];
 
-	hdac_hdmi_set_power_state(hdac, dai_map, AC_PWRST_D3);
+	dd = (struct hdac_ext_dma_params *)snd_soc_dai_get_dma_data(dai, substream);
 
-	snd_hdac_codec_write(&hdac->hdac, dai_map->pin->nid, 0,
+	if (dd) {
+		snd_soc_dai_set_dma_data(dai, substream, NULL);
+		kfree(dd);
+	}
+
+	if (dai_map->pin) {
+		snd_hdac_codec_write(&edev->hdac, dai_map->cvt->nid, 0,
+				AC_VERB_SET_CHANNEL_STREAMID, 0);
+		snd_hdac_codec_write(&edev->hdac, dai_map->cvt->nid, 0,
+				AC_VERB_SET_STREAM_FORMAT, 0);
+
+		hdac_hdmi_set_power_state(edev, dai_map, AC_PWRST_D3);
+
+		snd_hdac_codec_write(&edev->hdac, dai_map->pin->nid, 0,
 			AC_VERB_SET_AMP_GAIN_MUTE, AMP_OUT_MUTE);
 
-	dai_map->pin = NULL;
+		dai_map->pin = NULL;
+	}
+
+	return 0;
 }
 
 static int
@@ -1014,8 +1000,6 @@ static void hdac_hdmi_skl_enable_dp12(struct hdac_device *hdac)
 }
 
 static struct snd_soc_dai_ops hdmi_dai_ops = {
-	.startup = hdac_hdmi_pcm_open,
-	.shutdown = hdac_hdmi_pcm_close,
 	.hw_params = hdac_hdmi_set_hw_params,
 	.prepare = hdac_hdmi_playback_prepare,
 	.hw_free = hdac_hdmi_playback_cleanup,
-- 
1.9.1