[RFC 00/10] Enable HDA Codec support on Intel Platforms (Series2)

[RFC 00/10] Enable HDA Codec support on Intel Platforms (Series2)

[Rakesh Ughreja]

Many Intel platforms (SKL,KBL) etc. in the market supports enahanced 
audio capabilities which also includes DSP processing. This patch carry
forwads the works that is done in the previous series to enable HD Audio
codecs on such platforms.

This patch series adds ASoC HDA codec driver for Intel platforms. It is
written by reusing the legacy HDA ALSA codec driver. Intention is to
maximize the reuse and minimize the changes in the legacy HDA codec driver.

I would like to receive feedback before proceeding further on this
direction.

TODO:

- This series is tested on KBL based product (Dell XPS 13).
- Basic playback is working with headset and speakers.
- Capture operation is not tested.
- More platforms and use cases coverage can be added once we have basic
  agreement in terms of the overall approach.

FIXME:
- KConfig changes does not look right, but I could not think of any proper
  way without making changes into legacy HDA codec driver. So need some
  help on this topic.

Rakesh Ughreja (10):
  ASoC: Intel: Boards: Machine driver for Intel platforms
  ASoC: Intel: Skylake: Add entry in sst_acpi_mach for HDA codecs
  ASoC: Intel: Skylake: add HDA BE DAIs
  ASoC: Intel: Skylake: use hda_bus instead of hdac_bus
  ALSA: hda - make some of the functions externally visible
  ASoC: hdac_hda: add ASoC based HDA codec driver
  ALSA: hda: add new API snd_hda_asoc_codec_new for ASoC codec drivers
  ASoC: hdac_hda: add DAI, widgets and related ops
  ASoC: hdac_hda: add runtime PM support
  ASoC: Intel: Boards: add support for HDA codecs

 include/sound/hdaudio_ext.h              |   3 +-
 sound/hda/ext/hdac_ext_bus.c             |   9 +-
 sound/pci/hda/Kconfig                    |  11 +
 sound/pci/hda/hda_codec.c                | 109 +++++-
 sound/pci/hda/hda_codec.h                |  32 +-
 sound/soc/codecs/Kconfig                 |   6 +
 sound/soc/codecs/Makefile                |   2 +
 sound/soc/codecs/hdac_hda.c              | 633 +++++++++++++++++++++++++++++++
 sound/soc/codecs/hdac_hda.h              |  16 +
 sound/soc/intel/boards/Kconfig           |  11 +
 sound/soc/intel/boards/Makefile          |   2 +
 sound/soc/intel/boards/skl_hda_generic.c | 386 +++++++++++++++++++
 sound/soc/intel/skylake/skl-pcm.c        |  32 +-
 sound/soc/intel/skylake/skl.c            |  79 +++-
 sound/soc/intel/skylake/skl.h            |  10 +-
 15 files changed, 1315 insertions(+), 26 deletions(-)
 create mode 100644 sound/soc/codecs/hdac_hda.c
 create mode 100644 sound/soc/codecs/hdac_hda.h
 create mode 100644 sound/soc/intel/boards/skl_hda_generic.c

-- 
2.7.4

[RFC 01/10] ASoC: Intel: Boards: Machine driver for Intel platforms

[Rakesh Ughreja]

Add machine driver for Intel platforms(SKL/KBL) with HDA and iDisp codecs.
This patch adds support for only iDisp (HDMI/DP) codec. In the
following patch support for HDA codec will be added.

This should work for other Intel platforms as well e.g. BXT,GLK,CNL
however this series is not tested on all the platforms.

Signed-off-by: Rakesh Ughreja <rakesh.a.ughreja@intel.com>
---
 sound/soc/intel/boards/Kconfig           |  10 ++
 sound/soc/intel/boards/Makefile          |   2 +
 sound/soc/intel/boards/skl_hda_generic.c | 276 +++++++++++++++++++++++++++++++
 3 files changed, 288 insertions(+)
 create mode 100644 sound/soc/intel/boards/skl_hda_generic.c

diff --git a/sound/soc/intel/boards/Kconfig b/sound/soc/intel/boards/Kconfig
index 6f75470..4f8bd02 100644
--- a/sound/soc/intel/boards/Kconfig
+++ b/sound/soc/intel/boards/Kconfig
@@ -262,4 +262,14 @@ config SND_SOC_INTEL_KBL_RT5663_RT5514_MAX98927_MACH
           Say Y if you have such a device.
           If unsure select "N".
 
+config SND_SOC_INTEL_SKL_HDA_GENERIC_MACH
+        tristate "ASoC Audio driver for SKL/KBL with HDA Codecs"
+        select SND_SOC_INTEL_SST
+        depends on SND_SOC_INTEL_SKYLAKE
+        select SND_SOC_HDAC_HDMI
+        help
+          This adds support for ASoC Onboard Codec HDA machine driver. This will
+          create an alsa sound card for HDA Codecs.
+          Say Y if you have such a device.
+          If unsure select "N".
 endif
diff --git a/sound/soc/intel/boards/Makefile b/sound/soc/intel/boards/Makefile
index 69d2dfa..4686727 100644
--- a/sound/soc/intel/boards/Makefile
+++ b/sound/soc/intel/boards/Makefile
@@ -17,6 +17,7 @@ snd-soc-sst-byt-cht-nocodec-objs := bytcht_nocodec.o
 snd-soc-kbl_rt5663_max98927-objs := kbl_rt5663_max98927.o
 snd-soc-kbl_rt5663_rt5514_max98927-objs := kbl_rt5663_rt5514_max98927.o
 snd-soc-skl_rt286-objs := skl_rt286.o
+snd-soc-skl_hda_generic-objs := skl_hda_generic.o
 snd-skl_nau88l25_max98357a-objs := skl_nau88l25_max98357a.o
 snd-soc-skl_nau88l25_ssm4567-objs := skl_nau88l25_ssm4567.o
 
@@ -40,3 +41,4 @@ obj-$(CONFIG_SND_SOC_INTEL_KBL_RT5663_RT5514_MAX98927_MACH) += snd-soc-kbl_rt566
 obj-$(CONFIG_SND_SOC_INTEL_SKL_RT286_MACH) += snd-soc-skl_rt286.o
 obj-$(CONFIG_SND_SOC_INTEL_SKL_NAU88L25_MAX98357A_MACH) += snd-skl_nau88l25_max98357a.o
 obj-$(CONFIG_SND_SOC_INTEL_SKL_NAU88L25_SSM4567_MACH) += snd-soc-skl_nau88l25_ssm4567.o
+obj-$(CONFIG_SND_SOC_INTEL_SKL_HDA_GENERIC_MACH) += snd-soc-skl_hda_generic.o
diff --git a/sound/soc/intel/boards/skl_hda_generic.c b/sound/soc/intel/boards/skl_hda_generic.c
new file mode 100644
index 0000000..ece39b5
--- /dev/null
+++ b/sound/soc/intel/boards/skl_hda_generic.c
@@ -0,0 +1,276 @@
+/*
+ * Intel Machine Driver for SKL/KBL platforms with HDA Codecs
+ *
+ * Copyright (C) 2017, Intel Corporation. All rights reserved.
+ *
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License version
+ * 2 as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <sound/core.h>
+#include <sound/jack.h>
+#include <sound/pcm.h>
+#include <sound/pcm_params.h>
+#include <sound/soc.h>
+#include "../../codecs/hdac_hdmi.h"
+#include "../skylake/skl.h"
+
+static struct snd_soc_card skl_hda_audio_card;
+static struct snd_soc_jack skl_hda_hdmi_jack[3];
+
+struct skl_hda_hdmi_pcm {
+	struct list_head head;
+	struct snd_soc_dai *codec_dai;
+	int device;
+};
+
+struct skl_hda_private {
+	struct list_head hdmi_pcm_list;
+};
+
+enum {
+	SKL_HDA_DPCM_AUDIO_HDMI1_PB = 0,
+	SKL_HDA_DPCM_AUDIO_HDMI2_PB,
+	SKL_HDA_DPCM_AUDIO_HDMI3_PB,
+};
+
+static const struct snd_soc_dapm_route skl_hda_map[] = {
+
+	{ "hifi3", NULL, "iDisp3 Tx"},
+	{ "iDisp3 Tx", NULL, "iDisp3_out"},
+	{ "hifi2", NULL, "iDisp2 Tx"},
+	{ "iDisp2 Tx", NULL, "iDisp2_out"},
+	{ "hifi1", NULL, "iDisp1 Tx"},
+	{ "iDisp1 Tx", NULL, "iDisp1_out"},
+
+};
+
+static int skl_hda_hdmi1_init(struct snd_soc_pcm_runtime *rtd)
+{
+	struct skl_hda_private *ctx = snd_soc_card_get_drvdata(rtd->card);
+	struct snd_soc_dai *dai = rtd->codec_dai;
+	struct skl_hda_hdmi_pcm *pcm;
+
+	pcm = devm_kzalloc(rtd->card->dev, sizeof(*pcm), GFP_KERNEL);
+	if (!pcm)
+		return -ENOMEM;
+
+	pcm->device = SKL_HDA_DPCM_AUDIO_HDMI1_PB;
+	pcm->codec_dai = dai;
+
+	list_add_tail(&pcm->head, &ctx->hdmi_pcm_list);
+
+	return 0;
+}
+
+static int skl_hda_hdmi2_init(struct snd_soc_pcm_runtime *rtd)
+{
+	struct skl_hda_private *ctx = snd_soc_card_get_drvdata(rtd->card);
+	struct snd_soc_dai *dai = rtd->codec_dai;
+	struct skl_hda_hdmi_pcm *pcm;
+
+	pcm = devm_kzalloc(rtd->card->dev, sizeof(*pcm), GFP_KERNEL);
+	if (!pcm)
+		return -ENOMEM;
+
+	pcm->device = SKL_HDA_DPCM_AUDIO_HDMI2_PB;
+	pcm->codec_dai = dai;
+
+	list_add_tail(&pcm->head, &ctx->hdmi_pcm_list);
+
+	return 0;
+}
+
+static int skl_hda_hdmi3_init(struct snd_soc_pcm_runtime *rtd)
+{
+	struct skl_hda_private *ctx = snd_soc_card_get_drvdata(rtd->card);
+	struct snd_soc_dai *dai = rtd->codec_dai;
+	struct skl_hda_hdmi_pcm *pcm;
+
+	pcm = devm_kzalloc(rtd->card->dev, sizeof(*pcm), GFP_KERNEL);
+	if (!pcm)
+		return -ENOMEM;
+
+	pcm->device = SKL_HDA_DPCM_AUDIO_HDMI3_PB;
+	pcm->codec_dai = dai;
+
+	list_add_tail(&pcm->head, &ctx->hdmi_pcm_list);
+
+	return 0;
+}
+
+/* skl_hda_digital audio interface glue - connects codec <--> CPU */
+static struct snd_soc_dai_link skl_hda_dais[] = {
+	/* Front End DAI links */
+	[SKL_HDA_DPCM_AUDIO_HDMI1_PB] = {
+		.name = "SKL HDA HDMI Port1",
+		.stream_name = "Hdmi1",
+		.cpu_dai_name = "HDMI1 Pin",
+		.codec_name = "snd-soc-dummy",
+		.codec_dai_name = "snd-soc-dummy-dai",
+		.platform_name = "0000:00:1f.3",
+		.dpcm_playback = 1,
+		.init = NULL,
+		.trigger = {
+			SND_SOC_DPCM_TRIGGER_POST, SND_SOC_DPCM_TRIGGER_POST},
+		.nonatomic = 1,
+		.dynamic = 1,
+	},
+	[SKL_HDA_DPCM_AUDIO_HDMI2_PB] = {
+		.name = "SKL HDA HDMI Port2",
+		.stream_name = "Hdmi2",
+		.cpu_dai_name = "HDMI2 Pin",
+		.codec_name = "snd-soc-dummy",
+		.codec_dai_name = "snd-soc-dummy-dai",
+		.platform_name = "0000:00:1f.3",
+		.dpcm_playback = 1,
+		.init = NULL,
+		.trigger = {
+			SND_SOC_DPCM_TRIGGER_POST, SND_SOC_DPCM_TRIGGER_POST},
+		.nonatomic = 1,
+		.dynamic = 1,
+	},
+	[SKL_HDA_DPCM_AUDIO_HDMI3_PB] = {
+		.name = "SKL HDA HDMI Port3",
+		.stream_name = "Hdmi3",
+		.cpu_dai_name = "HDMI3 Pin",
+		.codec_name = "snd-soc-dummy",
+		.codec_dai_name = "snd-soc-dummy-dai",
+		.platform_name = "0000:00:1f.3",
+		.trigger = {
+			SND_SOC_DPCM_TRIGGER_POST, SND_SOC_DPCM_TRIGGER_POST},
+		.dpcm_playback = 1,
+		.init = NULL,
+		.nonatomic = 1,
+		.dynamic = 1,
+	},
+
+	/* Back End DAI links */
+	{
+		.name = "iDisp1",
+		.id = 1,
+		.cpu_dai_name = "iDisp1 Pin",
+		.codec_name = "ehdaudio0D2",
+		.codec_dai_name = "intel-hdmi-hifi1",
+		.platform_name = "0000:00:1f.3",
+		.dpcm_playback = 1,
+		.init = skl_hda_hdmi1_init,
+		.no_pcm = 1,
+	},
+	{
+		.name = "iDisp2",
+		.id = 2,
+		.cpu_dai_name = "iDisp2 Pin",
+		.codec_name = "ehdaudio0D2",
+		.codec_dai_name = "intel-hdmi-hifi2",
+		.platform_name = "0000:00:1f.3",
+		.init = skl_hda_hdmi2_init,
+		.dpcm_playback = 1,
+		.no_pcm = 1,
+	},
+	{
+		.name = "iDisp3",
+		.id = 3,
+		.cpu_dai_name = "iDisp3 Pin",
+		.codec_name = "ehdaudio0D2",
+		.codec_dai_name = "intel-hdmi-hifi3",
+		.platform_name = "0000:00:1f.3",
+		.init = skl_hda_hdmi3_init,
+		.dpcm_playback = 1,
+		.no_pcm = 1,
+	},
+};
+
+#define NAME_SIZE	32
+static int skl_hda_card_late_probe(struct snd_soc_card *card)
+{
+	struct skl_hda_private *ctx = snd_soc_card_get_drvdata(card);
+	struct skl_hda_hdmi_pcm *pcm;
+	struct snd_soc_codec *codec = NULL;
+	int err, i = 0;
+	char jack_name[NAME_SIZE];
+
+	list_for_each_entry(pcm, &ctx->hdmi_pcm_list, head) {
+		codec = pcm->codec_dai->codec;
+		snprintf(jack_name, sizeof(jack_name),
+			"HDMI/DP, pcm=%d Jack", pcm->device);
+		err = snd_soc_card_jack_new(card, jack_name,
+					SND_JACK_AVOUT, &skl_hda_hdmi_jack[i],
+					NULL, 0);
+
+		if (err)
+			return err;
+
+		err = hdac_hdmi_jack_init(pcm->codec_dai, pcm->device,
+						&skl_hda_hdmi_jack[i]);
+		if (err < 0)
+			return err;
+
+		i++;
+	}
+
+	if (!codec)
+		return -EINVAL;
+
+	return hdac_hdmi_jack_port_init(codec, &card->dapm);
+}
+
+static struct snd_soc_card skl_hda_audio_card = {
+	.name = "skl_hda_card",
+	.owner = THIS_MODULE,
+	.dai_link = skl_hda_dais,
+	.num_links = ARRAY_SIZE(skl_hda_dais),
+	.dapm_routes = skl_hda_map,
+	.num_dapm_routes = ARRAY_SIZE(skl_hda_map),
+	.fully_routed = true,
+	.late_probe = skl_hda_card_late_probe,
+};
+
+static int skl_hda_audio_probe(struct platform_device *pdev)
+{
+	struct skl_hda_private *ctx;
+
+	dev_dbg(&pdev->dev, "%s: machine driver probe got called\n", __func__);
+
+	ctx = devm_kzalloc(&pdev->dev, sizeof(*ctx), GFP_ATOMIC);
+	if (!ctx)
+		return -ENOMEM;
+
+	INIT_LIST_HEAD(&ctx->hdmi_pcm_list);
+
+	skl_hda_audio_card.dev = &pdev->dev;
+	snd_soc_card_set_drvdata(&skl_hda_audio_card, ctx);
+
+	return devm_snd_soc_register_card(&pdev->dev, &skl_hda_audio_card);
+}
+
+static const struct platform_device_id skl_hda_board_ids[] = {
+	{ .name = "skl_hda_generic" },
+	{ }
+};
+
+static struct platform_driver skl_hda_audio = {
+	.probe = skl_hda_audio_probe,
+	.driver = {
+		.name = "skl_hda_generic",
+		.pm = &snd_soc_pm_ops,
+	},
+	.id_table = skl_hda_board_ids,
+};
+
+module_platform_driver(skl_hda_audio)
+
+/* Module information */
+MODULE_DESCRIPTION("SKL/KBL HDA Generic Machine driver");
+MODULE_AUTHOR("Rakesh Ughreja <rakesh.a.ughreja@intel.com>");
+MODULE_LICENSE("GPL v2");
+MODULE_ALIAS("platform:skl_hda_generic");
-- 
2.7.4

[RFC 02/10] ASoC: Intel: Skylake: Add entry in sst_acpi_mach for HDA codecs

[Rakesh Ughreja]

When no I2S based codecs are detected in the BIOS, check if there are
any HDA codecs present. If yes, load the corresponding machine driver.

TODO:
support for detecting the presence of HDA codec is not implemented.
it will be implemented in the next revision.

Signed-off-by: Rakesh Ughreja <rakesh.a.ughreja@intel.com>
---
 sound/soc/intel/skylake/skl.c | 46 ++++++++++++++++++++++++++++++++++++++++---
 1 file changed, 43 insertions(+), 3 deletions(-)

diff --git a/sound/soc/intel/skylake/skl.c b/sound/soc/intel/skylake/skl.c
index 27462c0..568a285 100644
--- a/sound/soc/intel/skylake/skl.c
+++ b/sound/soc/intel/skylake/skl.c
@@ -424,6 +424,26 @@ static int skl_free(struct hdac_bus *bus)
 	return 0;
 }
 
+static struct snd_soc_acpi_mach *skl_probe_hda_machine(
+					struct snd_soc_acpi_mach *machines)
+{
+
+	struct snd_soc_acpi_mach *mach;
+
+	/*
+	 * FIXME:
+	 * First check if there are any HDA codecs present on the system
+	 * then search the match table.
+	 * For now this function is not detecting the presence of any
+	 * HDA codecs.
+	 */
+	for (mach = machines; mach->id[0]; mach++) {
+		if (!strcmp(mach->id, "HDA_GEN"))
+			return mach;
+	}
+	return NULL;
+}
+
 static int skl_machine_device_register(struct skl *skl, void *driver_data)
 {
 	struct hdac_bus *bus = skl_to_bus(skl);
@@ -433,9 +453,14 @@ static int skl_machine_device_register(struct skl *skl, void *driver_data)
 
 	mach = snd_soc_acpi_find_machine(mach);
 	if (mach == NULL) {
-		dev_err(bus->dev, "No matching machine driver found\n");
-		return -ENODEV;
+		dev_dbg(bus->dev, "No matching I2S machine driver found\n");
+		mach = skl_probe_hda_machine(driver_data);
+		if (mach == NULL) {
+			dev_err(bus->dev, "No matching machine driver found\n");
+			return -ENODEV;
+		}
 	}
+
 	skl->fw_name = mach->fw_filename;
 
 	pdev = platform_device_alloc(mach->drv_name, -1);
@@ -911,6 +936,14 @@ static struct snd_soc_acpi_mach sst_skl_devdata[] = {
 		.quirk_data = &skl_codecs,
 		.pdata = &skl_dmic_data
 	},
+	{
+		.id = "HDA_GEN",
+		.drv_name = "skl_hda_generic",
+		.fw_filename = "intel/dsp_fw_release.bin",
+		.machine_quirk = NULL,
+		.quirk_data = NULL,
+		.pdata = NULL
+	},
 	{}
 };
 
@@ -973,7 +1006,14 @@ static struct snd_soc_acpi_mach sst_kbl_devdata[] = {
 		.drv_name = "kbl_rt5663",
 		.fw_filename = "intel/dsp_fw_kbl.bin",
 	},
-
+	{
+		.id = "HDA_GEN",
+		.drv_name = "skl_hda_generic",
+		.fw_filename = "intel/dsp_fw_kbl.bin",
+		.machine_quirk = NULL,
+		.quirk_data = NULL,
+		.pdata = NULL
+	},
 	{}
 };
 
-- 
2.7.4

[RFC 03/10] ASoC: Intel: Skylake: add HDA BE DAIs

[Rakesh Ughreja]

Add support for HDA BE DAIs in SKL platform driver.

Signed-off-by: Rakesh Ughreja <rakesh.a.ughreja@intel.com>
---
 sound/soc/intel/skylake/skl-pcm.c | 32 +++++++++++++++++++++++++-------
 1 file changed, 25 insertions(+), 7 deletions(-)

diff --git a/sound/soc/intel/skylake/skl-pcm.c b/sound/soc/intel/skylake/skl-pcm.c
index e6fee7a..3dcb241 100644
--- a/sound/soc/intel/skylake/skl-pcm.c
+++ b/sound/soc/intel/skylake/skl-pcm.c
@@ -956,21 +956,39 @@ static struct snd_soc_dai_driver skl_platform_dai[] = {
 	},
 },
 {
-	.name = "HD-Codec Pin",
+	.name = "Analog CPU DAI",
 	.ops = &skl_link_dai_ops,
 	.playback = {
-		.stream_name = "HD-Codec Tx",
-		.channels_min = HDA_STEREO,
+		.stream_name = "Analog CPU Playback",
+		.channels_min = HDA_MONO,
 		.channels_max = HDA_STEREO,
 		.rates = SNDRV_PCM_RATE_48000,
-		.formats = SNDRV_PCM_FMTBIT_S16_LE,
+		.formats = SNDRV_PCM_FMTBIT_S16_LE | SNDRV_PCM_FMTBIT_S24_LE,
 	},
 	.capture = {
-		.stream_name = "HD-Codec Rx",
-		.channels_min = HDA_STEREO,
+		.stream_name = "Analog CPU Capture",
+		.channels_min = HDA_MONO,
 		.channels_max = HDA_STEREO,
 		.rates = SNDRV_PCM_RATE_48000,
-		.formats = SNDRV_PCM_FMTBIT_S16_LE,
+		.formats = SNDRV_PCM_FMTBIT_S16_LE | SNDRV_PCM_FMTBIT_S24_LE,
+	},
+},
+{
+	.name = "Digital CPU DAI",
+	.ops = &skl_link_dai_ops,
+	.playback = {
+		.stream_name = "Digital CPU Playback",
+		.channels_min = HDA_MONO,
+		.channels_max = HDA_STEREO,
+		.rates = SNDRV_PCM_RATE_48000,
+		.formats = SNDRV_PCM_FMTBIT_S16_LE | SNDRV_PCM_FMTBIT_S24_LE,
+	},
+	.capture = {
+		.stream_name = "Digital CPU Capture",
+		.channels_min = HDA_MONO,
+		.channels_max = HDA_STEREO,
+		.rates = SNDRV_PCM_RATE_48000,
+		.formats = SNDRV_PCM_FMTBIT_S16_LE | SNDRV_PCM_FMTBIT_S24_LE,
 	},
 },
 };
-- 
2.7.4

[RFC 04/10] ASoC: Intel: Skylake: use hda_bus instead of hdac_bus

[Rakesh Ughreja]

This patch prepares SKL platform driver to make reuse of legacy HDA
codec drivers. It does following things.

use hda_bus instead of hdac_bus in the SKL platform driver to align
with the legacy controller driver.

modify snd_hdac_ext_bus_device_init definition to align with
snd_hdac_bus_device_init, used by legacy drivers.
Memory for hdac_device is allocated by the caller.

Signed-off-by: Rakesh Ughreja <rakesh.a.ughreja@intel.com>
---
 include/sound/hdaudio_ext.h   |  3 ++-
 sound/hda/ext/hdac_ext_bus.c  |  9 ++-------
 sound/soc/intel/skylake/skl.c | 19 ++++++++++++++++++-
 sound/soc/intel/skylake/skl.h | 10 +++++++---
 4 files changed, 29 insertions(+), 12 deletions(-)

diff --git a/include/sound/hdaudio_ext.h b/include/sound/hdaudio_ext.h
index 3c30247..c188b80 100644
--- a/include/sound/hdaudio_ext.h
+++ b/include/sound/hdaudio_ext.h
@@ -9,7 +9,8 @@ int snd_hdac_ext_bus_init(struct hdac_bus *bus, struct device *dev,
 		      const struct hdac_io_ops *io_ops);
 
 void snd_hdac_ext_bus_exit(struct hdac_bus *bus);
-int snd_hdac_ext_bus_device_init(struct hdac_bus *bus, int addr);
+int snd_hdac_ext_bus_device_init(struct hdac_bus *bus, int addr,
+						struct hdac_device *hdev);
 void snd_hdac_ext_bus_device_exit(struct hdac_device *hdev);
 void snd_hdac_ext_bus_device_remove(struct hdac_bus *bus);
 
diff --git a/sound/hda/ext/hdac_ext_bus.c b/sound/hda/ext/hdac_ext_bus.c
index 52f0776..e4bcb76 100644
--- a/sound/hda/ext/hdac_ext_bus.c
+++ b/sound/hda/ext/hdac_ext_bus.c
@@ -135,16 +135,12 @@ static void default_release(struct device *dev)
  *
  * Returns zero for success or a negative error code.
  */
-int snd_hdac_ext_bus_device_init(struct hdac_bus *bus, int addr)
+int snd_hdac_ext_bus_device_init(struct hdac_bus *bus, int addr,
+					struct hdac_device *hdev)
 {
-	struct hdac_device *hdev = NULL;
 	char name[15];
 	int ret;
 
-	hdev = kzalloc(sizeof(*hdev), GFP_KERNEL);
-	if (!hdev)
-		return -ENOMEM;
-
 	hdev->bus = bus;
 
 	snprintf(name, sizeof(name), "ehdaudio%dD%d", bus->idx, addr);
@@ -175,7 +171,6 @@ EXPORT_SYMBOL_GPL(snd_hdac_ext_bus_device_init);
 void snd_hdac_ext_bus_device_exit(struct hdac_device *hdev)
 {
 	snd_hdac_device_exit(hdev);
-	kfree(hdev);
 }
 EXPORT_SYMBOL_GPL(snd_hdac_ext_bus_device_exit);
 
diff --git a/sound/soc/intel/skylake/skl.c b/sound/soc/intel/skylake/skl.c
index 568a285..72a788a 100644
--- a/sound/soc/intel/skylake/skl.c
+++ b/sound/soc/intel/skylake/skl.c
@@ -35,6 +35,7 @@
 #include "skl.h"
 #include "skl-sst-dsp.h"
 #include "skl-sst-ipc.h"
+#include "../../../pci/hda/hda_codec.h"
 
 static struct skl_machine_pdata skl_dmic_data;
 
@@ -531,6 +532,8 @@ static int probe_codec(struct hdac_bus *bus, int addr)
 	unsigned int cmd = (addr << 28) | (AC_NODE_ROOT << 20) |
 		(AC_VERB_PARAMETERS << 8) | AC_PAR_VENDOR_ID;
 	unsigned int res = -1;
+	struct skl *skl = bus_to_skl(bus);
+	struct hdac_device *hdev;
 
 	mutex_lock(&bus->cmd_mutex);
 	snd_hdac_bus_send_cmd(bus, cmd);
@@ -540,7 +543,11 @@ static int probe_codec(struct hdac_bus *bus, int addr)
 		return -EIO;
 	dev_dbg(bus->dev, "codec #%d probed OK\n", addr);
 
-	return snd_hdac_ext_bus_device_init(bus, addr);
+	hdev = devm_kzalloc(&skl->pci->dev, sizeof(*hdev), GFP_KERNEL);
+	if (!hdev)
+		return -ENOMEM;
+
+	return snd_hdac_ext_bus_device_init(bus, addr, hdev);
 }
 
 /* Codec initialization */
@@ -665,6 +672,7 @@ static int skl_create(struct pci_dev *pci,
 {
 	struct skl *skl;
 	struct hdac_bus *bus;
+	struct hda_bus *hbus;
 
 	int err;
 
@@ -680,6 +688,7 @@ static int skl_create(struct pci_dev *pci,
 		return -ENOMEM;
 	}
 
+	hbus = skl_to_hbus(skl);
 	bus = skl_to_bus(skl);
 	snd_hdac_ext_bus_init(bus, &pci->dev, &bus_core_ops, io_ops);
 	bus->use_posbuf = 1;
@@ -687,6 +696,14 @@ static int skl_create(struct pci_dev *pci,
 	INIT_WORK(&skl->probe_work, skl_probe_work);
 	bus->bdl_pos_adj = 0;
 
+	/*
+	 * TODO: other parameters can be taken the way it is taken by
+	 * legacy HDA driver
+	 */
+	mutex_init(&hbus->prepare_mutex);
+	hbus->pci = pci;
+	hbus->mixer_assigned = -1;
+
 	*rskl = skl;
 
 	return 0;
diff --git a/sound/soc/intel/skylake/skl.h b/sound/soc/intel/skylake/skl.h
index f7fcd7e..c6b7aee 100644
--- a/sound/soc/intel/skylake/skl.h
+++ b/sound/soc/intel/skylake/skl.h
@@ -25,6 +25,7 @@
 #include <sound/hdaudio_ext.h>
 #include <sound/soc.h>
 #include "skl-nhlt.h"
+#include "../../../pci/hda/hda_codec.h"
 
 #define SKL_SUSPEND_DELAY 2000
 
@@ -46,7 +47,7 @@ struct skl_dsp_resource {
 struct skl_debug;
 
 struct skl {
-	struct hdac_bus hbus;
+	struct hda_bus hbus;
 	struct pci_dev *pci;
 
 	unsigned int init_done:1; /* delayed init status */
@@ -77,8 +78,11 @@ struct skl {
 	bool use_tplg_pcm;
 };
 
-#define skl_to_bus(s)  (&(s)->hbus)
-#define bus_to_skl(bus) container_of(bus, struct skl, hbus)
+#define skl_to_bus(s)  (&(s)->hbus.core)
+#define bus_to_skl(bus) container_of(bus, struct skl, hbus.core)
+
+#define skl_to_hbus(s) (&(s)->hbus)
+#define hbus_to_skl(hbus) container_of(hbus, struct skl, hbus)
 
 /* to pass dai dma data */
 struct skl_dma_params {
-- 
2.7.4

[RFC 05/10] ALSA: hda - make some of the functions externally visible

[Rakesh Ughreja]

Mark some functions with EXPORT_SYMBOL_GPL so that it can be called by
other kernel modules. These APIs would be called by ASoC based HDA codec
driver which will be added in the later patches.

Signed-off-by: Rakesh Ughreja <rakesh.a.ughreja@intel.com>
---
 sound/pci/hda/hda_codec.c | 9 +++++++--
 sound/pci/hda/hda_codec.h | 4 +++-
 2 files changed, 10 insertions(+), 3 deletions(-)

diff --git a/sound/pci/hda/hda_codec.c b/sound/pci/hda/hda_codec.c
index e018ecb..085fd9e 100644
--- a/sound/pci/hda/hda_codec.c
+++ b/sound/pci/hda/hda_codec.c
@@ -2903,7 +2903,7 @@ static void hda_call_codec_resume(struct hda_codec *codec)
 	atomic_dec(&codec->core.in_pm);
 }
 
-static int hda_codec_runtime_suspend(struct device *dev)
+int hda_codec_runtime_suspend(struct device *dev)
 {
 	struct hda_codec *codec = dev_to_hda_codec(dev);
 	struct hda_pcm *pcm;
@@ -2919,8 +2919,9 @@ static int hda_codec_runtime_suspend(struct device *dev)
 	snd_hdac_link_power(&codec->core, false);
 	return 0;
 }
+EXPORT_SYMBOL_GPL(hda_codec_runtime_suspend);
 
-static int hda_codec_runtime_resume(struct device *dev)
+int hda_codec_runtime_resume(struct device *dev)
 {
 	struct hda_codec *codec = dev_to_hda_codec(dev);
 
@@ -2930,6 +2931,7 @@ static int hda_codec_runtime_resume(struct device *dev)
 	pm_runtime_mark_last_busy(dev);
 	return 0;
 }
+EXPORT_SYMBOL_GPL(hda_codec_runtime_resume);
 #endif /* CONFIG_PM */
 
 /* referred in hda_bind.c */
@@ -3005,6 +3007,7 @@ int snd_hda_codec_build_controls(struct hda_codec *codec)
 	sync_power_up_states(codec);
 	return 0;
 }
+EXPORT_SYMBOL_GPL(snd_hda_codec_build_controls);
 
 /*
  * PCM stuff
@@ -3210,6 +3213,7 @@ int snd_hda_codec_parse_pcms(struct hda_codec *codec)
 
 	return 0;
 }
+EXPORT_SYMBOL_GPL(snd_hda_codec_parse_pcms);
 
 /* assign all PCMs of the given codec */
 int snd_hda_codec_build_pcms(struct hda_codec *codec)
@@ -3246,6 +3250,7 @@ int snd_hda_codec_build_pcms(struct hda_codec *codec)
 
 	return 0;
 }
+EXPORT_SYMBOL_GPL(snd_hda_codec_build_pcms);
 
 /**
  * snd_hda_add_new_ctls - create controls from the array
diff --git a/sound/pci/hda/hda_codec.h b/sound/pci/hda/hda_codec.h
index 681c360..2d02d02 100644
--- a/sound/pci/hda/hda_codec.h
+++ b/sound/pci/hda/hda_codec.h
@@ -83,7 +83,7 @@ struct hda_bus {
  * better than the generic parser.
  */
 typedef int (*hda_codec_patch_t)(struct hda_codec *);
-	
+
 #define HDA_CODEC_ID_GENERIC_HDMI	0x00000101
 #define HDA_CODEC_ID_GENERIC		0x00000201
 
@@ -497,6 +497,8 @@ int hda_call_check_power_status(struct hda_codec *codec, hda_nid_t nid)
 #ifdef CONFIG_PM
 void snd_hda_set_power_save(struct hda_bus *bus, int delay);
 void snd_hda_update_power_acct(struct hda_codec *codec);
+int hda_codec_runtime_suspend(struct device *dev);
+int hda_codec_runtime_resume(struct device *dev);
 #else
 static inline void snd_hda_set_power_save(struct hda_bus *bus, int delay) {}
 #endif
-- 
2.7.4

[RFC 06/10] ASoC: hdac_hda: add ASoC based HDA codec driver

[Rakesh Ughreja]

This patch adds ASoC based HDA codec driver that can be used with
all Intel platforms.

FIXME:
KConfig change allows users to compile legacy HDA codec drivers either
as ASoC or ALSA codec driver. This is not a good approach and need
suggestion to improve it.

Signed-off-by: Rakesh Ughreja <rakesh.a.ughreja@intel.com>
---
 sound/pci/hda/Kconfig         | 11 +++++
 sound/pci/hda/hda_codec.h     | 26 +++++++++++-
 sound/soc/codecs/Kconfig      |  6 +++
 sound/soc/codecs/Makefile     |  2 +
 sound/soc/codecs/hdac_hda.c   | 95 +++++++++++++++++++++++++++++++++++++++++++
 sound/soc/codecs/hdac_hda.h   | 16 ++++++++
 sound/soc/intel/skylake/skl.c | 18 +++++++-
 7 files changed, 171 insertions(+), 3 deletions(-)
 create mode 100644 sound/soc/codecs/hdac_hda.c
 create mode 100644 sound/soc/codecs/hdac_hda.h

diff --git a/sound/pci/hda/Kconfig b/sound/pci/hda/Kconfig
index 7f3b5ed..010dd7d 100644
--- a/sound/pci/hda/Kconfig
+++ b/sound/pci/hda/Kconfig
@@ -215,6 +215,17 @@ config SND_HDA_GENERIC
 comment "Set to Y if you want auto-loading the codec driver"
 	depends on SND_HDA=y && SND_HDA_GENERIC=m
 
+config SND_ASOC_HDA_GENERIC
+        bool "Build HDA drivers as HDA ASoC Codec drivers"
+	select SND_SOC_HDAC_HDA
+        def_bool n
+        help
+          Select to build legacy HD-audio codec drivers as ASoC
+          codec drivers.
+
+comment "Set to Y if you want to enable support for HDA audio codec with
+	 DSP on Intel platforms"
+
 config SND_HDA_POWER_SAVE_DEFAULT
 	int "Default time-out for HD-audio power-save mode"
 	depends on PM
diff --git a/sound/pci/hda/hda_codec.h b/sound/pci/hda/hda_codec.h
index 2d02d02..67ce2e6 100644
--- a/sound/pci/hda/hda_codec.h
+++ b/sound/pci/hda/hda_codec.h
@@ -104,9 +104,33 @@ int __hda_codec_driver_register(struct hda_codec_driver *drv, const char *name,
 #define hda_codec_driver_register(drv) \
 	__hda_codec_driver_register(drv, KBUILD_MODNAME, THIS_MODULE)
 void hda_codec_driver_unregister(struct hda_codec_driver *drv);
+
+/*
+ * FIXME:
+ * In this implementation one can compile codec drivers either as ALSA or as
+ * ASoC, but not both.
+ * This may be not the be the best way to build the HDA codec drivers as ALSA
+ * vs ASoC driver. Need to find a better way, where the driver can be built
+ * as ALSA as well as ASoC at the same time. Even though practically people
+ * would never use them at the same time. But may be disto guys may want to
+ * do it.
+ */
+
+#ifdef CONFIG_SND_ASOC_HDA_GENERIC
+int __hdac_hda_codec_driver_register(struct hda_codec_driver *drv,
+	const char *name, struct module *owner);
+#define hdac_hda_codec_driver_register(drv) \
+	__hdac_hda_codec_driver_register(drv, KBUILD_MODNAME, THIS_MODULE)
+void hdac_hda_codec_driver_unregister(struct hda_codec_driver *drv);
+
+#define module_hda_codec_driver(drv) \
+	module_driver(drv, hdac_hda_codec_driver_register, \
+			hdac_hda_codec_driver_unregister)
+#else
 #define module_hda_codec_driver(drv) \
 	module_driver(drv, hda_codec_driver_register, \
-		      hda_codec_driver_unregister)
+			hda_codec_driver_unregister)
+#endif
 
 /* ops set by the preset patch */
 struct hda_codec_ops {
diff --git a/sound/soc/codecs/Kconfig b/sound/soc/codecs/Kconfig
index a42ddbc..196fcbc 100644
--- a/sound/soc/codecs/Kconfig
+++ b/sound/soc/codecs/Kconfig
@@ -79,6 +79,7 @@ config SND_SOC_ALL_CODECS
 	select SND_SOC_ES7134
 	select SND_SOC_GTM601
 	select SND_SOC_HDAC_HDMI
+	select SND_SOC_HDAC_HDA
 	select SND_SOC_ICS43432
 	select SND_SOC_INNO_RK3036
 	select SND_SOC_ISABELLE if I2C
@@ -578,6 +579,11 @@ config SND_SOC_HDAC_HDMI
 	select SND_PCM_ELD
 	select HDMI
 
+config SND_SOC_HDAC_HDA
+	tristate
+	select SND_HDA_EXT_CORE
+	select SND_PCM_ELD
+
 config SND_SOC_ICS43432
 	tristate
 
diff --git a/sound/soc/codecs/Makefile b/sound/soc/codecs/Makefile
index 0001069..84b33c9 100644
--- a/sound/soc/codecs/Makefile
+++ b/sound/soc/codecs/Makefile
@@ -73,6 +73,7 @@ snd-soc-es8328-i2c-objs := es8328-i2c.o
 snd-soc-es8328-spi-objs := es8328-spi.o
 snd-soc-gtm601-objs := gtm601.o
 snd-soc-hdac-hdmi-objs := hdac_hdmi.o
+snd-soc-hdac-hda-objs := hdac_hda.o
 snd-soc-ics43432-objs := ics43432.o
 snd-soc-inno-rk3036-objs := inno_rk3036.o
 snd-soc-isabelle-objs := isabelle.o
@@ -313,6 +314,7 @@ obj-$(CONFIG_SND_SOC_ES8328_I2C)+= snd-soc-es8328-i2c.o
 obj-$(CONFIG_SND_SOC_ES8328_SPI)+= snd-soc-es8328-spi.o
 obj-$(CONFIG_SND_SOC_GTM601)    += snd-soc-gtm601.o
 obj-$(CONFIG_SND_SOC_HDAC_HDMI) += snd-soc-hdac-hdmi.o
+obj-$(CONFIG_SND_SOC_HDAC_HDA) += snd-soc-hdac-hda.o
 obj-$(CONFIG_SND_SOC_ICS43432)	+= snd-soc-ics43432.o
 obj-$(CONFIG_SND_SOC_INNO_RK3036)	+= snd-soc-inno-rk3036.o
 obj-$(CONFIG_SND_SOC_ISABELLE)	+= snd-soc-isabelle.o
diff --git a/sound/soc/codecs/hdac_hda.c b/sound/soc/codecs/hdac_hda.c
new file mode 100644
index 0000000..f9a3491
--- /dev/null
+++ b/sound/soc/codecs/hdac_hda.c
@@ -0,0 +1,95 @@
+/*
+ *  hdac_hda.c - ASoc HDA-HDA codec driver for Intel platforms
+ *
+ *  Copyright (C) 2015-2017 Intel Corp
+ *
+ *  ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+ *
+ *  This program is free software; you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License as published by
+ *  the Free Software Foundation; version 2 of the License.
+ *
+ *  This program is distributed in the hope that it will be useful, but
+ *  WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ *  General Public License for more details.
+ *
+ * ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+ */
+#include <linux/init.h>
+#include <linux/delay.h>
+#include <linux/module.h>
+#include <sound/pcm_params.h>
+#include <sound/soc.h>
+#include <sound/hdaudio_ext.h>
+#include <sound/hda_register.h>
+#include "../../hda/local.h"
+#include "../../pci/hda/hda_codec.h"
+#include "hdac_hda.h"
+
+static int hdac_hda_dev_probe(struct hdac_device *hdev)
+{
+	struct hdac_ext_link *hlink = NULL;
+	struct hdac_hda_priv *hda_pvt;
+
+	dev_dbg(&hdev->dev, "%s: entry\n", __func__);
+
+	/* hold the ref while we probe */
+	hlink = snd_hdac_ext_bus_get_link(hdev->bus, dev_name(&hdev->dev));
+	if (!hlink) {
+		dev_err(&hdev->dev, "hdac link not found\n");
+		return -EIO;
+	}
+	snd_hdac_ext_bus_link_get(hdev->bus, hlink);
+
+	hda_pvt = hdac_to_hda_priv(hdev);
+	if (hda_pvt == NULL)
+		return -ENOMEM;
+
+	dev_set_drvdata(&hdev->dev, hda_pvt);
+	snd_hdac_ext_bus_link_put(hdev->bus, hlink);
+
+	return 0;
+}
+
+static int hdac_hda_dev_remove(struct hdac_device *hdev)
+{
+	dev_dbg(&hdev->dev, "%s: entry\n", __func__);
+	return 0;
+}
+
+#define hdac_hda_runtime_suspend NULL
+#define hdac_hda_runtime_resume NULL
+
+static const struct dev_pm_ops hdac_hda_pm = {
+	SET_RUNTIME_PM_OPS(hdac_hda_runtime_suspend,
+				hdac_hda_runtime_resume, NULL)
+};
+
+int __hdac_hda_codec_driver_register(struct hda_codec_driver *drv,
+	const char *name, struct module *owner)
+{
+	struct hdac_driver *hdac_hda_driver = &drv->core;
+
+	hdac_hda_driver->driver.name = name;
+	hdac_hda_driver->driver.pm = &hdac_hda_pm;
+
+	hdac_hda_driver->id_table = drv->id;
+	hdac_hda_driver->probe = hdac_hda_dev_probe;
+	hdac_hda_driver->remove = hdac_hda_dev_remove;
+
+	return snd_hda_ext_driver_register(hdac_hda_driver);
+}
+EXPORT_SYMBOL_GPL(__hdac_hda_codec_driver_register);
+
+void hdac_hda_codec_driver_unregister(struct hda_codec_driver *drv)
+{
+	struct hdac_driver *hdac_hda_driver = &drv->core;
+
+	snd_hda_ext_driver_unregister(hdac_hda_driver);
+}
+EXPORT_SYMBOL_GPL(hdac_hda_codec_driver_unregister);
+
+MODULE_LICENSE("GPL v2");
+MODULE_DESCRIPTION("ASoC HDA codec driver");
+MODULE_AUTHOR("Rakesh Ughreja<rakesh.a.ughreja@intel.com>");
diff --git a/sound/soc/codecs/hdac_hda.h b/sound/soc/codecs/hdac_hda.h
new file mode 100644
index 0000000..b59f106
--- /dev/null
+++ b/sound/soc/codecs/hdac_hda.h
@@ -0,0 +1,16 @@
+#ifndef __HDAC_HDA_H__
+#define __HDAC_HDA_H__
+
+struct hdac_hda_priv {
+	struct hda_codec codec;
+	struct hda_bus *hbus;
+	int stream_tag;
+
+	struct snd_soc_codec *scodec;
+};
+
+#define hdac_to_hda_priv(_hdac) \
+			container_of(_hdac, struct hdac_hda_priv, codec.core)
+#define hdac_to_hda_codec(_hdac) container_of(_hdac, struct hda_codec, core)
+
+#endif /* __HDAC_HDA_H__ */
diff --git a/sound/soc/intel/skylake/skl.c b/sound/soc/intel/skylake/skl.c
index 72a788a..7f1971f 100644
--- a/sound/soc/intel/skylake/skl.c
+++ b/sound/soc/intel/skylake/skl.c
@@ -36,6 +36,7 @@
 #include "skl-sst-dsp.h"
 #include "skl-sst-ipc.h"
 #include "../../../pci/hda/hda_codec.h"
+#include "../../../soc/codecs/hdac_hda.h"
 
 static struct skl_machine_pdata skl_dmic_data;
 
@@ -533,6 +534,7 @@ static int probe_codec(struct hdac_bus *bus, int addr)
 		(AC_VERB_PARAMETERS << 8) | AC_PAR_VENDOR_ID;
 	unsigned int res = -1;
 	struct skl *skl = bus_to_skl(bus);
+	struct hdac_hda_priv *hda_codec;
 	struct hdac_device *hdev;
 
 	mutex_lock(&bus->cmd_mutex);
@@ -543,10 +545,22 @@ static int probe_codec(struct hdac_bus *bus, int addr)
 		return -EIO;
 	dev_dbg(bus->dev, "codec #%d probed OK\n", addr);
 
-	hdev = devm_kzalloc(&skl->pci->dev, sizeof(*hdev), GFP_KERNEL);
-	if (!hdev)
+	/*
+	 * FIXME:
+	 * The legacy HDA controller driver allocates data structure
+	 * for hda_codec. Ideally it should allocate only hdac_device
+	 * so that it has no dependency on the data structure of the
+	 * codec driver. If legacy controller driver can be changed this
+	 * code change can be avoided.
+	 */
+	hda_codec = devm_kzalloc(&skl->pci->dev, sizeof(*hda_codec),
+								GFP_KERNEL);
+	if (!hda_codec)
 		return -ENOMEM;
 
+	hda_codec->hbus = skl_to_hbus(skl);
+	hdev = &hda_codec->codec.core;
+
 	return snd_hdac_ext_bus_device_init(bus, addr, hdev);
 }
 
-- 
2.7.4

[RFC 07/10] ALSA: hda: add new API snd_hda_asoc_codec_new for ASoC codec drivers

[Rakesh Ughreja]

Add API snd_hda_asoc_codec_new, to be called by generic ASoC codec
driver to create the hda_codec instance.

Signed-off-by: Rakesh Ughreja <rakesh.a.ughreja@intel.com>
---
 sound/pci/hda/hda_codec.c | 100 ++++++++++++++++++++++++++++++++++++++++++++++
 sound/pci/hda/hda_codec.h |   2 +
 2 files changed, 102 insertions(+)

diff --git a/sound/pci/hda/hda_codec.c b/sound/pci/hda/hda_codec.c
index 085fd9e..215b04d 100644
--- a/sound/pci/hda/hda_codec.c
+++ b/sound/pci/hda/hda_codec.c
@@ -965,6 +965,106 @@ int snd_hda_codec_new(struct hda_bus *bus, struct snd_card *card,
 }
 EXPORT_SYMBOL_GPL(snd_hda_codec_new);
 
+/*
+ * FIXME:
+ * This function is very simillar to snd_hda_codec_new, but requires
+ * some special change for ASoC based codec drivers.
+ * Both functions can be combined into one.
+ */
+int snd_hda_asoc_codec_new(struct hda_bus *bus, struct snd_card *card,
+			unsigned int codec_addr, struct hda_codec **codecp)
+{
+	struct hda_codec *codec = NULL;
+	hda_nid_t fg;
+	int err;
+	static struct snd_device_ops dev_ops = {
+		.dev_register = snd_hda_codec_dev_register,
+		.dev_disconnect = snd_hda_codec_dev_disconnect,
+		.dev_free = snd_hda_codec_dev_free,
+	};
+	struct snd_device *snd_dev;
+
+	if (snd_BUG_ON(!codecp))
+		return -EINVAL;
+	if (snd_BUG_ON(!bus))
+		return -EINVAL;
+	if (snd_BUG_ON(codec_addr > HDA_MAX_CODEC_ADDRESS))
+		return -EINVAL;
+
+	codec = *codecp;
+
+	codec->core.exec_verb = codec_exec_verb;
+	codec->bus = bus;
+	codec->card = card;
+	codec->addr = codec_addr;
+	mutex_init(&codec->spdif_mutex);
+	mutex_init(&codec->control_mutex);
+	snd_array_init(&codec->mixers, sizeof(struct hda_nid_item), 32);
+	snd_array_init(&codec->nids, sizeof(struct hda_nid_item), 32);
+	snd_array_init(&codec->init_pins, sizeof(struct hda_pincfg), 16);
+	snd_array_init(&codec->driver_pins, sizeof(struct hda_pincfg), 16);
+	snd_array_init(&codec->cvt_setups, sizeof(struct hda_cvt_setup), 8);
+	snd_array_init(&codec->spdif_out, sizeof(struct hda_spdif_out), 16);
+	snd_array_init(&codec->jacktbl, sizeof(struct hda_jack_tbl), 16);
+	snd_array_init(&codec->verbs, sizeof(struct hda_verb *), 8);
+	INIT_LIST_HEAD(&codec->conn_list);
+	INIT_LIST_HEAD(&codec->pcm_list_head);
+
+	INIT_DELAYED_WORK(&codec->jackpoll_work, hda_jackpoll_work);
+	codec->depop_delay = -1;
+	codec->fixup_id = HDA_FIXUP_ID_NOT_SET;
+
+#ifdef CONFIG_PM
+	codec->power_jiffies = jiffies;
+#endif
+
+	if (codec->bus->modelname) {
+		codec->modelname = kstrdup(codec->bus->modelname, GFP_KERNEL);
+		if (!codec->modelname) {
+			err = -ENOMEM;
+			goto error;
+		}
+	}
+
+	fg = codec->core.afg ? codec->core.afg : codec->core.mfg;
+	err = read_widget_caps(codec, fg);
+	if (err < 0)
+		goto error;
+
+	err = read_pin_defaults(codec);
+	if (err < 0)
+		goto error;
+
+	/* power-up all before initialization */
+	hda_set_power_state(codec, AC_PWRST_D0);
+
+	snd_hda_codec_proc_new(codec);
+
+	snd_hda_create_hwdep(codec);
+
+	err = snd_device_new(card, SNDRV_DEV_CODEC, codec, &dev_ops);
+	if (err < 0)
+		goto error;
+
+	list_for_each_entry(snd_dev, &card->devices, list) {
+		if (snd_dev->type == SNDRV_DEV_CODEC) {
+			void *device_data = snd_dev->device_data;
+
+			err = snd_device_register(card, device_data);
+			if (err < 0)
+				dev_err(bus->core.dev,
+				"device_register_all failed err = %d\n", err);
+		}
+	}
+
+	return 0;
+
+error:
+	put_device(hda_codec_dev(codec));
+	return err;
+}
+EXPORT_SYMBOL_GPL(snd_hda_asoc_codec_new);
+
 /**
  * snd_hda_codec_update_widgets - Refresh widget caps and pin defaults
  * @codec: the HDA codec
diff --git a/sound/pci/hda/hda_codec.h b/sound/pci/hda/hda_codec.h
index 67ce2e6..6d10b25 100644
--- a/sound/pci/hda/hda_codec.h
+++ b/sound/pci/hda/hda_codec.h
@@ -331,6 +331,8 @@ struct hda_codec {
  */
 int snd_hda_codec_new(struct hda_bus *bus, struct snd_card *card,
 		      unsigned int codec_addr, struct hda_codec **codecp);
+int snd_hda_asoc_codec_new(struct hda_bus *bus, struct snd_card *card,
+		      unsigned int codec_addr, struct hda_codec **codecp);
 int snd_hda_codec_configure(struct hda_codec *codec);
 int snd_hda_codec_update_widgets(struct hda_codec *codec);
 
-- 
2.7.4

[RFC 08/10] ASoC: hdac_hda: add DAI, widgets and related ops

[Rakesh Ughreja]

Add support for widgets, routes, DAI and related handlers.

Signed-off-by: Rakesh Ughreja <rakesh.a.ughreja@intel.com>
---
 sound/soc/codecs/hdac_hda.c | 491 +++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 490 insertions(+), 1 deletion(-)

diff --git a/sound/soc/codecs/hdac_hda.c b/sound/soc/codecs/hdac_hda.c
index f9a3491..80aadde 100644
--- a/sound/soc/codecs/hdac_hda.c
+++ b/sound/soc/codecs/hdac_hda.c
@@ -19,6 +19,7 @@
 #include <linux/init.h>
 #include <linux/delay.h>
 #include <linux/module.h>
+#include <linux/pm_runtime.h>
 #include <sound/pcm_params.h>
 #include <sound/soc.h>
 #include <sound/hdaudio_ext.h>
@@ -27,10 +28,489 @@
 #include "../../pci/hda/hda_codec.h"
 #include "hdac_hda.h"
 
+#define STUB_FORMATS	(SNDRV_PCM_FMTBIT_S8 | \
+			SNDRV_PCM_FMTBIT_U8 | \
+			SNDRV_PCM_FMTBIT_S16_LE | \
+			SNDRV_PCM_FMTBIT_U16_LE | \
+			SNDRV_PCM_FMTBIT_S24_LE | \
+			SNDRV_PCM_FMTBIT_U24_LE | \
+			SNDRV_PCM_FMTBIT_S32_LE | \
+			SNDRV_PCM_FMTBIT_U32_LE | \
+			SNDRV_PCM_FMTBIT_IEC958_SUBFRAME_LE)
+
+static int hdac_hda_dai_open(struct snd_pcm_substream *substream,
+			struct snd_soc_dai *dai);
+static void hdac_hda_dai_close(struct snd_pcm_substream *substream,
+		struct snd_soc_dai *dai);
+static int hdac_hda_dai_prepare(struct snd_pcm_substream *substream,
+				struct snd_soc_dai *dai);
+static int hdac_hda_dai_hw_params(struct snd_pcm_substream *substream,
+	struct snd_pcm_hw_params *hparams, struct snd_soc_dai *dai);
+static int hdac_hda_dai_hw_free(struct snd_pcm_substream *substream,
+				struct snd_soc_dai *dai);
+static int hdac_hda_dai_set_tdm_slot(struct snd_soc_dai *dai,
+		unsigned int tx_mask, unsigned int rx_mask,
+		int slots, int slot_width);
+static struct hda_pcm *snd_soc_find_pcm_from_dai(struct hdac_hda_priv *hda_pvt,
+						struct snd_soc_dai *dai);
+
+static struct snd_soc_dai_ops hdac_hda_dai_ops = {
+	.startup = hdac_hda_dai_open,
+	.shutdown = hdac_hda_dai_close,
+	.prepare = hdac_hda_dai_prepare,
+	.hw_params = hdac_hda_dai_hw_params,
+	.hw_free = hdac_hda_dai_hw_free,
+	.set_tdm_slot = hdac_hda_dai_set_tdm_slot,
+};
+
+static struct snd_soc_dai_driver hdac_hda_dais[] = {
+{
+	.name = "Analog Codec DAI",
+	.ops = &hdac_hda_dai_ops,
+	.playback = {
+		.stream_name	= "Analog Codec Playback",
+		.channels_min	= 1,
+		.channels_max	= 16,
+		.rates		= SNDRV_PCM_RATE_8000_192000,
+		.formats	= STUB_FORMATS,
+		.sig_bits	= 24,
+	},
+	.capture = {
+		.stream_name    = "Analog Codec Capture",
+		.channels_min   = 1,
+		.channels_max   = 16,
+		.rates = SNDRV_PCM_RATE_8000_192000,
+		.formats = STUB_FORMATS,
+		.sig_bits = 24,
+	},
+},
+{
+	.name = "Digital Codec DAI",
+	.ops = &hdac_hda_dai_ops,
+	.playback = {
+		.stream_name    = "Digital Codec Playback",
+		.channels_min   = 1,
+		.channels_max   = 16,
+		.rates          = SNDRV_PCM_RATE_8000_192000,
+		.formats        = STUB_FORMATS,
+		.sig_bits = 24,
+	},
+	.capture = {
+		.stream_name    = "Digital Codec Capture",
+		.channels_min   = 1,
+		.channels_max   = 16,
+		.rates = SNDRV_PCM_RATE_8000_192000,
+		.formats = STUB_FORMATS,
+		.sig_bits = 24,
+	},
+}
+};
+
+static int hdac_codec_match(struct hdac_device *hdev, struct hdac_driver *drv)
+{
+	struct hda_codec *codec = hdac_to_hda_codec(hdev);
+	const struct hda_device_id *list;
+	u32 id, rev_id;
+
+	if (!codec) {
+		dev_err(&hdev->dev, "%p\n", codec);
+		return 0;
+	}
+
+	/* check probe_id instead of vendor_id if set */
+	id = codec->probe_id ? codec->probe_id : codec->core.vendor_id;
+	rev_id = codec->core.revision_id;
+
+	dev_dbg(&hdev->dev, "%s: probe_id=0x%x and vendor_id=0x%x\n",
+			__func__, codec->probe_id, codec->core.vendor_id);
+
+	for (list = drv->id_table; list->vendor_id; list++) {
+		if (list->vendor_id == id &&
+			(!list->rev_id || list->rev_id == rev_id)) {
+			codec->preset = list;
+			dev_dbg(&hdev->dev, "found patch file %x\n", id);
+			return 1;
+		}
+	}
+	return 0;
+}
+
+static int hdac_hda_dai_set_tdm_slot(struct snd_soc_dai *dai,
+		unsigned int tx_mask, unsigned int rx_mask,
+		int slots, int slot_width)
+{
+	struct hdac_hda_priv *hda_pvt = snd_soc_dai_get_drvdata(dai);
+	struct hdac_device *hdev = &hda_pvt->codec.core;
+
+	hda_pvt->stream_tag = tx_mask;
+	dev_dbg(&hdev->dev, "%s: strm_tag: %d\n", __func__,
+						hda_pvt->stream_tag);
+	return 0;
+}
+
+static int hdac_hda_dai_hw_params(struct snd_pcm_substream *substream,
+	struct snd_pcm_hw_params *hparams, struct snd_soc_dai *dai)
+{
+	struct hdac_hda_priv *hda_pvt = snd_soc_dai_get_drvdata(dai);
+	struct hdac_device *hdev = &hda_pvt->codec.core;
+
+	dev_dbg(&hdev->dev, "%s: entry\n", __func__);
+
+	/*
+	 * TODO:
+	 * verify if the parameters are supported by the codec
+	 */
+	return 0;
+}
+
+static int hdac_hda_dai_hw_free(struct snd_pcm_substream *substream,
+				struct snd_soc_dai *dai)
+{
+	struct hdac_hda_priv *hda_pvt = snd_soc_dai_get_drvdata(dai);
+	struct hdac_device *hdev = &hda_pvt->codec.core;
+	struct snd_pcm_runtime *runtime = substream->runtime;
+	struct hda_pcm_stream *hda_stream;
+	struct hda_pcm *pcm;
+
+	dev_dbg(&hdev->dev, "%s: entry\n", __func__);
+
+	pcm = snd_soc_find_pcm_from_dai(hda_pvt, dai);
+	if (pcm == NULL)
+		return -EINVAL;
+
+	dev_dbg(&hdev->dev, "pcm = %s: rate=%d ch=%d fmt=%d\n", pcm->name,
+		runtime->rate, runtime->channels, runtime->format);
+
+	hda_stream = &pcm->stream[substream->stream];
+
+	snd_hda_codec_cleanup(&hda_pvt->codec, hda_stream, substream);
+
+	return 0;
+}
+
+static int hdac_hda_dai_prepare(struct snd_pcm_substream *substream,
+				struct snd_soc_dai *dai)
+{
+	int ret = 0;
+	struct hdac_hda_priv *hda_pvt = snd_soc_dai_get_drvdata(dai);
+	struct hdac_device *hdev = &hda_pvt->codec.core;
+	struct snd_pcm_runtime *runtime = substream->runtime;
+	struct hda_pcm_stream *hda_stream;
+	unsigned int format_val;
+	struct hda_pcm *pcm;
+
+	dev_dbg(&hdev->dev, "%s: entry\n", __func__);
+
+	pcm = snd_soc_find_pcm_from_dai(hda_pvt, dai);
+	if (pcm == NULL)
+		return -EINVAL;
+
+	dev_dbg(&hdev->dev, "pcm = %s: rate=%d ch=%d fmt=%d\n", pcm->name,
+		runtime->rate, runtime->channels, runtime->format);
+
+	hda_stream = &pcm->stream[substream->stream];
+
+	dev_dbg(&hdev->dev, "fmt=0x%llx rate=0x%x maxbps=%d\n",
+		hda_stream->formats, hda_stream->rates, hda_stream->maxbps);
+
+	format_val = snd_hdac_calc_stream_format(runtime->rate,
+						runtime->channels,
+						runtime->format,
+						hda_stream->maxbps,
+						0);
+	if (!format_val) {
+		dev_err(&hdev->dev,
+			"invalid format_val, rate=%d, ch=%d, format=%d\n",
+			runtime->rate, runtime->channels, runtime->format);
+		return -EINVAL;
+	}
+
+	ret = snd_hda_codec_prepare(&hda_pvt->codec, hda_stream,
+			hda_pvt->stream_tag, format_val, substream);
+	if (ret < 0) {
+		dev_err(&hdev->dev, "codec prepare failed %d\n", ret);
+		return ret;
+	}
+
+	dev_dbg(&hdev->dev, "%s: exit\n", __func__);
+	return ret;
+}
+
+static int hdac_hda_dai_open(struct snd_pcm_substream *substream,
+			struct snd_soc_dai *dai)
+{
+	struct hdac_hda_priv *hda_pvt = snd_soc_dai_get_drvdata(dai);
+	struct hdac_device *hdev = &hda_pvt->codec.core;
+	struct snd_pcm_runtime *runtime = substream->runtime;
+	struct hda_pcm *pcm;
+	struct hda_pcm_stream *hda_stream;
+	int ret = 0;
+
+	dev_dbg(&hdev->dev, "%s: entry\n", __func__);
+
+	pcm = snd_soc_find_pcm_from_dai(hda_pvt, dai);
+	if (pcm == NULL)
+		return -EINVAL;
+
+	dev_dbg(&hdev->dev, "pcm = %s: rate=%d ch=%d fmt=%d\n", pcm->name,
+		runtime->rate, runtime->channels, runtime->format);
+
+	hda_stream = &pcm->stream[substream->stream];
+
+	snd_hda_codec_pcm_get(pcm);
+
+	if (hda_stream->ops.open)
+		ret = hda_stream->ops.open(hda_stream, &hda_pvt->codec,
+								substream);
+	else
+		ret = -ENODEV;
+
+	dev_dbg(&hdev->dev, "%s: exit=%d\n", __func__, ret);
+
+	return 0;
+}
+
+static void hdac_hda_dai_close(struct snd_pcm_substream *substream,
+		struct snd_soc_dai *dai)
+{
+	struct hdac_hda_priv *hda_pvt = snd_soc_dai_get_drvdata(dai);
+	struct hdac_device *hdev = &hda_pvt->codec.core;
+	struct hda_pcm *pcm;
+	struct hda_pcm_stream *hda_stream;
+
+	dev_dbg(&hdev->dev, "%s: entry\n", __func__);
+
+	pcm = snd_soc_find_pcm_from_dai(hda_pvt, dai);
+	if (pcm == NULL)
+		return;
+
+	hda_stream = &pcm->stream[substream->stream];
+
+	if (hda_stream->ops.open)
+		hda_stream->ops.close(hda_stream, &hda_pvt->codec, substream);
+
+	snd_hda_codec_pcm_put(pcm);
+}
+
+static struct hda_pcm *snd_soc_find_pcm_from_dai(struct hdac_hda_priv *hda_pvt,
+						struct snd_soc_dai *dai)
+{
+
+	struct hda_pcm *cpcm;
+	struct hda_codec *hcodec = &hda_pvt->codec;
+	const char *pcm_name;
+
+	if (strpbrk(dai->name, "Analog"))
+		pcm_name = "Analog";
+	else
+		pcm_name = "Digital";
+
+	dev_dbg(&hcodec->core.dev, "DAI %s\n", pcm_name);
+
+	list_for_each_entry(cpcm, &hcodec->pcm_list_head, list) {
+		dev_dbg(&hcodec->core.dev, "PCM %s\n", cpcm->name);
+		if (strpbrk(cpcm->name, pcm_name))
+			return cpcm;
+	}
+
+	dev_err(&hcodec->core.dev, "didn't find PCM for DAI %s\n", dai->name);
+	return NULL;
+}
+
+static int hdac_hda_codec_probe(struct snd_soc_codec *codec)
+{
+	struct hdac_hda_priv *hda_pvt = snd_soc_codec_get_drvdata(codec);
+	struct hdac_device *hdev = &hda_pvt->codec.core;
+	struct hdac_ext_link *hlink = NULL;
+	struct hda_codec *hcodec = &hda_pvt->codec;
+	struct snd_soc_dapm_context *dapm =
+			snd_soc_component_get_dapm(&codec->component);
+	int ret, i = 0;
+	u16 codec_mask;
+	hda_codec_patch_t patch;
+	struct hdac_driver *hdrv = drv_to_hdac_driver(hdev->dev.driver);
+
+	dev_dbg(&hdev->dev, "%s: Entry in_pm=%d pm_usage=%d\n", __func__,
+			hdev->in_pm.counter,
+			hdev->dev.power.usage_count.counter);
+
+	hda_pvt->scodec = codec;
+	hlink = snd_hdac_ext_bus_get_link(hdev->bus,
+					dev_name(&hdev->dev));
+	if (!hlink) {
+		dev_err(&hdev->dev, "hdac link not found\n");
+		return -EIO;
+	}
+
+	ret = snd_hdac_ext_bus_link_get(hdev->bus, hlink);
+
+	udelay(1000);
+	do {
+		codec_mask = snd_hdac_chip_readw(hdev->bus, STATESTS);
+		if (codec_mask)
+			break;
+			i++;
+		udelay(100);
+	} while (i < 100);
+
+	if (!codec_mask) {
+		dev_err(&hdev->dev, "No codecs found after link reset\n");
+		return -EIO;
+	}
+
+	snd_hdac_chip_writew(hdev->bus, STATESTS, STATESTS_INT_MASK);
+
+	dev_dbg(&hdev->dev, "before card created in_pm=%d pm_usage=%d\n",
+			hdev->in_pm.counter,
+			hdev->dev.power.usage_count.counter);
+
+	ret = snd_hda_asoc_codec_new(hda_pvt->hbus,
+			codec->component.card->snd_card, hdev->addr,
+			&hcodec);
+	if (ret < 0) {
+		dev_err(codec->dev, "%s: failed to create hda codec %d\n",
+					__func__, ret);
+		return ret;
+	}
+
+	/*
+	 * snd_hda_codec_new1 decrements the usage count and so get the pm
+	 * count else the device will be powered off
+	 */
+	pm_runtime_get_noresume(&hdev->dev);
+
+	hda_pvt->hbus->card = dapm->card->snd_card;
+
+	dev_dbg(&hdev->dev, "after card created in_pm=%d pm_usage=%d\n",
+			hdev->in_pm.counter,
+			hdev->dev.power.usage_count.counter);
+
+	ret = hdac_codec_match(hdev, hdrv);
+	if (ret < 1) {
+		dev_err(codec->dev, "no match found for the codec\n");
+		return ret;
+	}
+
+	patch = (hda_codec_patch_t)hcodec->preset->driver_data;
+	if (patch) {
+		ret = patch(hcodec);
+		if (ret < 0) {
+			dev_err(codec->dev, "no match found for the codec\n");
+			return ret;
+		}
+	} else {
+		dev_dbg(&hdev->dev, "no patch file found\n");
+	}
+
+	ret = snd_hda_codec_set_name(hcodec, hcodec->preset->name);
+	if (ret < 0) {
+		dev_err(codec->dev, "unable to set name %s\n",
+				hcodec->preset->name);
+		return ret;
+	}
+
+	ret = snd_hdac_regmap_init(&hcodec->core);
+	if (ret < 0) {
+		dev_err(codec->dev, "regmap init failed\n");
+		return ret;
+	}
+
+	ret = snd_hda_codec_parse_pcms(hcodec);
+	if (ret < 0) {
+		dev_err(&hdev->dev, "unable to map pcms to dai %d\n", ret);
+		return ret;
+	}
+
+	ret = snd_hda_codec_build_controls(hcodec);
+	if (ret < 0) {
+		dev_err(&hdev->dev, "unable to create controls %d\n", ret);
+		return ret;
+	}
+
+	hcodec->core.lazy_cache = true;
+
+	/*
+	 * hdac_device core already sets the state to active and calls
+	 * get_noresume. So enable runtime and set the device to suspend.
+	 * pm_runtime_enable is also called during codec registeration
+	 */
+	pm_runtime_put(&hdev->dev);
+	pm_runtime_suspend(&hdev->dev);
+
+	dev_dbg(&hdev->dev, "%s: exit\n", __func__);
+
+	return 0;
+}
+
+static int hdac_hda_codec_remove(struct snd_soc_codec *codec)
+{
+	struct hdac_hda_priv *hda_pvt = snd_soc_codec_get_drvdata(codec);
+	struct hdac_device *hdev = &hda_pvt->codec.core;
+	struct hdac_ext_link *hlink = NULL;
+
+	dev_dbg(&hdev->dev, "%s: entry\n", __func__);
+
+	hlink = snd_hdac_ext_bus_get_link(hdev->bus, dev_name(&hdev->dev));
+	if (!hlink) {
+		dev_err(&hdev->dev, "hdac link not found\n");
+		return -EIO;
+	}
+
+	snd_hdac_ext_bus_link_put(hdev->bus, hlink);
+
+	pm_runtime_disable(&hdev->dev);
+
+	return 0;
+}
+
+
+static const struct snd_soc_dapm_route hdac_hda_dapm_routes[] = {
+	{"AIF1TX", NULL, "Codec Input Pin1"},
+	{"AIF2TX", NULL, "Codec Input Pin2"},
+
+	{"Codec Output Pin1", NULL, "AIF1RX"},
+	{"Codec Output Pin2", NULL, "AIF2RX"},
+};
+
+static const struct snd_soc_dapm_widget hdac_hda_dapm_widgets[] = {
+
+	/* Audio Interface */
+	SND_SOC_DAPM_AIF_IN("AIF1RX", "Analog Codec Playback", 0,
+							SND_SOC_NOPM, 0, 0),
+	SND_SOC_DAPM_AIF_IN("AIF2RX", "Digital Codec Playback", 0,
+							SND_SOC_NOPM, 0, 0),
+	SND_SOC_DAPM_AIF_OUT("AIF1TX", "Analog Codec Capture", 0,
+							SND_SOC_NOPM, 0, 0),
+	SND_SOC_DAPM_AIF_OUT("AIF2TX", "Digital Codec Capture", 0,
+							SND_SOC_NOPM, 0, 0),
+
+	/* Input Pins */
+	SND_SOC_DAPM_INPUT("Codec Input Pin1"),
+	SND_SOC_DAPM_INPUT("Codec Input Pin2"),
+
+	/* Output Pins */
+	SND_SOC_DAPM_OUTPUT("Codec Output Pin1"),
+	SND_SOC_DAPM_OUTPUT("Codec Output Pin2"),
+};
+
+
+static struct snd_soc_codec_driver hdac_hda_codec = {
+	.probe		= hdac_hda_codec_probe,
+	.remove		= hdac_hda_codec_remove,
+	.idle_bias_off = true,
+	.component_driver = {
+		.dapm_widgets           = hdac_hda_dapm_widgets,
+		.num_dapm_widgets       = ARRAY_SIZE(hdac_hda_dapm_widgets),
+		.dapm_routes            = hdac_hda_dapm_routes,
+		.num_dapm_routes        = ARRAY_SIZE(hdac_hda_dapm_routes),
+	},
+};
+
 static int hdac_hda_dev_probe(struct hdac_device *hdev)
 {
 	struct hdac_ext_link *hlink = NULL;
 	struct hdac_hda_priv *hda_pvt;
+	int ret;
 
 	dev_dbg(&hdev->dev, "%s: entry\n", __func__);
 
@@ -46,15 +526,24 @@ static int hdac_hda_dev_probe(struct hdac_device *hdev)
 	if (hda_pvt == NULL)
 		return -ENOMEM;
 
+	/* ASoC specific initialization */
+	ret = snd_soc_register_codec(&hdev->dev, &hdac_hda_codec, hdac_hda_dais,
+					 ARRAY_SIZE(hdac_hda_dais));
+	if (ret < 0) {
+		dev_err(&hdev->dev, "failed to register HDA codec %d\n", ret);
+		return ret;
+	}
+
 	dev_set_drvdata(&hdev->dev, hda_pvt);
 	snd_hdac_ext_bus_link_put(hdev->bus, hlink);
 
-	return 0;
+	return ret;
 }
 
 static int hdac_hda_dev_remove(struct hdac_device *hdev)
 {
 	dev_dbg(&hdev->dev, "%s: entry\n", __func__);
+	snd_soc_unregister_codec(&hdev->dev);
 	return 0;
 }
 
-- 
2.7.4

[RFC 09/10] ASoC: hdac_hda: add runtime PM support

[Rakesh Ughreja]

Add power management support.

Signed-off-by: Rakesh Ughreja <rakesh.a.ughreja@intel.com>
---
 sound/soc/codecs/hdac_hda.c | 49 +++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 49 insertions(+)

diff --git a/sound/soc/codecs/hdac_hda.c b/sound/soc/codecs/hdac_hda.c
index 80aadde..9a5188a 100644
--- a/sound/soc/codecs/hdac_hda.c
+++ b/sound/soc/codecs/hdac_hda.c
@@ -547,8 +547,57 @@ static int hdac_hda_dev_remove(struct hdac_device *hdev)
 	return 0;
 }
 
+#ifdef CONFIG_PM
+static int hdac_hda_runtime_suspend(struct device *dev)
+{
+	struct hdac_device *hdac = dev_to_hdac_dev(dev);
+	struct hdac_bus *bus = hdac->bus;
+	struct hdac_ext_link *hlink = NULL;
+
+	dev_dbg(dev, "%s: entry\n", __func__);
+
+	dev_dbg(dev, "calling legacy codec suspend\n");
+	hda_codec_runtime_suspend(dev);
+	dev_dbg(dev, "legacy codec suspended\n");
+
+	hlink = snd_hdac_ext_bus_get_link(bus, dev_name(dev));
+	if (!hlink) {
+		dev_err(dev, "hdac link not found\n");
+		return -EIO;
+	}
+	snd_hdac_ext_bus_link_put(bus, hlink);
+	return 0;
+}
+
+static int hdac_hda_runtime_resume(struct device *dev)
+{
+	struct hdac_device *hdac = dev_to_hdac_dev(dev);
+	struct hdac_bus *bus = hdac->bus;
+	struct hdac_ext_link *hlink = NULL;
+
+	dev_dbg(dev, "%s: entry\n", __func__);
+
+	/* controller may not have been initialized for the first time */
+	if (!bus)
+		return 0;
+
+	hlink = snd_hdac_ext_bus_get_link(bus, dev_name(dev));
+	if (!hlink) {
+		dev_err(dev, "hdac link not found\n");
+		return -EIO;
+	}
+	snd_hdac_ext_bus_link_get(bus, hlink);
+
+	dev_dbg(dev, "calling legacy codec resume\n");
+	hda_codec_runtime_resume(dev);
+	dev_dbg(dev, "%s: exit\n", __func__);
+
+	return 0;
+}
+#else
 #define hdac_hda_runtime_suspend NULL
 #define hdac_hda_runtime_resume NULL
+#endif
 
 static const struct dev_pm_ops hdac_hda_pm = {
 	SET_RUNTIME_PM_OPS(hdac_hda_runtime_suspend,
-- 
2.7.4

[RFC 10/10] ASoC: Intel: Boards: add support for HDA codecs

[Rakesh Ughreja]

Add support for HDA codecs. add required widgets, controls, routes
and dai links for the same.

Signed-off-by: Rakesh Ughreja <rakesh.a.ughreja@intel.com>
---
 sound/soc/intel/boards/Kconfig           |   1 +
 sound/soc/intel/boards/skl_hda_generic.c | 112 ++++++++++++++++++++++++++++++-
 2 files changed, 112 insertions(+), 1 deletion(-)

diff --git a/sound/soc/intel/boards/Kconfig b/sound/soc/intel/boards/Kconfig
index 4f8bd02..e2a280d 100644
--- a/sound/soc/intel/boards/Kconfig
+++ b/sound/soc/intel/boards/Kconfig
@@ -267,6 +267,7 @@ config SND_SOC_INTEL_SKL_HDA_GENERIC_MACH
         select SND_SOC_INTEL_SST
         depends on SND_SOC_INTEL_SKYLAKE
         select SND_SOC_HDAC_HDMI
+        select SND_SOC_HDAC_HDA
         help
           This adds support for ASoC Onboard Codec HDA machine driver. This will
           create an alsa sound card for HDA Codecs.
diff --git a/sound/soc/intel/boards/skl_hda_generic.c b/sound/soc/intel/boards/skl_hda_generic.c
index ece39b5..0e592d4 100644
--- a/sound/soc/intel/boards/skl_hda_generic.c
+++ b/sound/soc/intel/boards/skl_hda_generic.c
@@ -38,13 +38,43 @@ struct skl_hda_private {
 };
 
 enum {
-	SKL_HDA_DPCM_AUDIO_HDMI1_PB = 0,
+	SKL_HDA_DPCM_AUDIO_PB = 0,
+	SKL_HDA_DPCM_AUDIO_CP,
+	SKL_HDA_DPCM_AUDIO_HDMI1_PB,
 	SKL_HDA_DPCM_AUDIO_HDMI2_PB,
 	SKL_HDA_DPCM_AUDIO_HDMI3_PB,
 };
 
+static const struct snd_kcontrol_new skl_hda_controls[] = {
+	SOC_DAPM_PIN_SWITCH("Headphone"),
+	SOC_DAPM_PIN_SWITCH("Headset Mic"),
+};
+
+static const struct snd_soc_dapm_widget skl_hda_widgets[] = {
+	SND_SOC_DAPM_HP("Headphone", NULL),
+	SND_SOC_DAPM_MIC("Headset Mic", NULL),
+	SND_SOC_DAPM_SPK("Codec Speaker", NULL),
+	SND_SOC_DAPM_MIC("Codec Mic", NULL),
+};
+
 static const struct snd_soc_dapm_route skl_hda_map[] = {
 
+	/* HP jack connectors - unknown if we have jack detection */
+	{ "Headphone", NULL, "Codec Output Pin1" },
+	{ "Codec Speaker", NULL, "Codec Output Pin2" },
+	{ "Codec Input Pin2", NULL, "Codec Mic" },
+	{ "Codec Input Pin1", NULL, "Headset Mic" },
+
+	/* CODEC BE connections */
+	{ "Analog Codec Playback", NULL, "Analog CPU Playback" },
+	{ "Analog CPU Playback", NULL, "codec0_out" },
+
+	{ "Digital Codec Playback", NULL, "Digital CPU Playback" },
+	{ "Digital CPU Playback", NULL, "codec1_out" },
+
+	{ "codec0_in", NULL, "Analog CPU Capture" },
+	{ "Analog CPU Capture", NULL, "Analog Codec Capture" },
+
 	{ "hifi3", NULL, "iDisp3 Tx"},
 	{ "iDisp3 Tx", NULL, "iDisp3_out"},
 	{ "hifi2", NULL, "iDisp2 Tx"},
@@ -108,9 +138,60 @@ static int skl_hda_hdmi3_init(struct snd_soc_pcm_runtime *rtd)
 	return 0;
 }
 
+static int skl_hda_link_init(struct snd_soc_pcm_runtime *rtd)
+{
+	dev_dbg(rtd->dev, "%s entry\n", __func__);
+	return 0;
+}
+
+static int skl_hda_link_fixup(struct snd_soc_pcm_runtime *rtd,
+					struct snd_pcm_hw_params *params)
+{
+	struct snd_interval *rate = hw_param_interval(params,
+						SNDRV_PCM_HW_PARAM_RATE);
+	struct snd_interval *channels = hw_param_interval(params,
+						SNDRV_PCM_HW_PARAM_CHANNELS);
+	struct snd_mask *fmt = hw_param_mask(params, SNDRV_PCM_HW_PARAM_FORMAT);
+
+	/* The output is 48KHz, stereo, 16bits */
+	rate->min = rate->max = 48000;
+	channels->min = channels->max = 2;
+
+	snd_mask_none(fmt);
+	snd_mask_set(fmt, SNDRV_PCM_FORMAT_S24_LE);
+	return 0;
+}
+
 /* skl_hda_digital audio interface glue - connects codec <--> CPU */
 static struct snd_soc_dai_link skl_hda_dais[] = {
 	/* Front End DAI links */
+	[SKL_HDA_DPCM_AUDIO_PB] = {
+		.name = "Skylake Audio Playback Port",
+		.stream_name = "Audio Playback",
+		.cpu_dai_name = "System Pin",
+		.platform_name = "0000:00:1f.3",
+		.dynamic = 1,
+		.codec_name = "snd-soc-dummy",
+		.codec_dai_name = "snd-soc-dummy-dai",
+		.nonatomic = 1,
+		.init = NULL,
+		.trigger = {
+			SND_SOC_DPCM_TRIGGER_POST, SND_SOC_DPCM_TRIGGER_POST},
+		.dpcm_playback = 1,
+	},
+	[SKL_HDA_DPCM_AUDIO_CP] = {
+		.name = "Skylake Audio Capture Port",
+		.stream_name = "Audio Record",
+		.cpu_dai_name = "System Pin",
+		.platform_name = "0000:00:1f.3",
+		.dynamic = 1,
+		.codec_name = "snd-soc-dummy",
+		.codec_dai_name = "snd-soc-dummy-dai",
+		.nonatomic = 1,
+		.trigger = {
+			SND_SOC_DPCM_TRIGGER_POST, SND_SOC_DPCM_TRIGGER_POST},
+		.dpcm_capture = 1,
+	},
 	[SKL_HDA_DPCM_AUDIO_HDMI1_PB] = {
 		.name = "SKL HDA HDMI Port1",
 		.stream_name = "Hdmi1",
@@ -188,6 +269,31 @@ static struct snd_soc_dai_link skl_hda_dais[] = {
 		.dpcm_playback = 1,
 		.no_pcm = 1,
 	},
+	{
+		.name = "Analog Playback and Capture",
+		.id = 4,
+		.cpu_dai_name = "Analog CPU DAI",
+		.codec_name = "ehdaudio0D0",
+		.codec_dai_name = "Analog Codec DAI",
+		.platform_name = "0000:00:1f.3",
+		.dpcm_playback = 1,
+		.dpcm_capture = 1,
+		.init = skl_hda_link_init,
+		.no_pcm = 1,
+		.be_hw_params_fixup = skl_hda_link_fixup,
+	},
+	{
+		.name = "Digital Playback and Capture",
+		.id = 5,
+		.cpu_dai_name = "Digital CPU DAI",
+		.codec_name = "ehdaudio0D0",
+		.codec_dai_name = "Digital Codec DAI",
+		.platform_name = "0000:00:1f.3",
+		.dpcm_playback = 1,
+		.dpcm_capture = 1,
+		.init = skl_hda_link_init,
+		.no_pcm = 1,
+	},
 };
 
 #define NAME_SIZE	32
@@ -229,6 +335,10 @@ static struct snd_soc_card skl_hda_audio_card = {
 	.owner = THIS_MODULE,
 	.dai_link = skl_hda_dais,
 	.num_links = ARRAY_SIZE(skl_hda_dais),
+	.controls = skl_hda_controls,
+	.num_controls = ARRAY_SIZE(skl_hda_controls),
+	.dapm_widgets = skl_hda_widgets,
+	.num_dapm_widgets = ARRAY_SIZE(skl_hda_widgets),
 	.dapm_routes = skl_hda_map,
 	.num_dapm_routes = ARRAY_SIZE(skl_hda_map),
 	.fully_routed = true,
-- 
2.7.4

Re: [RFC 00/10] Enable HDA Codec support on Intel Platforms (Series2)

[Takashi Iwai]

On Fri, 01 Dec 2017 10:13:58 +0100,
Rakesh Ughreja wrote:
> 
> Many Intel platforms (SKL,KBL) etc. in the market supports enahanced 
> audio capabilities which also includes DSP processing. This patch carry
> forwads the works that is done in the previous series to enable HD Audio
> codecs on such platforms.
> 
> This patch series adds ASoC HDA codec driver for Intel platforms. It is
> written by reusing the legacy HDA ALSA codec driver. Intention is to
> maximize the reuse and minimize the changes in the legacy HDA codec driver.
> 
> I would like to receive feedback before proceeding further on this
> direction.

First of all, I appreciate this kind of work finally happening, so
that ASoC HD-audio can go forward to a wider usage.  However...


> TODO:
> 
> - This series is tested on KBL based product (Dell XPS 13).
> - Basic playback is working with headset and speakers.
> - Capture operation is not tested.
> - More platforms and use cases coverage can be added once we have basic
>   agreement in terms of the overall approach.

One big thing that is unclear to me is what about the complex I/O
configuration although we provide only a single machine driver, and
what happens if the (legacy) codec driver allows the jack retasking.
I know that nowadays the number of jacks are reduced than before, but
still there will be desktop boxes with individual front and rear I/O
jacks.


> FIXME:
> - KConfig changes does not look right, but I could not think of any proper
>   way without making changes into legacy HDA codec driver. So need some
>   help on this topic.

And this is another big issue.  As long as you don't allow build both,
it never flies with distros, and it means no dramatic improvement of
the situation, either.

IOW, we have to make the codec driver working on both controller/bus
implementations, and it essentially means to rewrite the HD-audio
codec plumbing code.  Currently, it's a kind of half-baked state, and
we need to cast to unified form.


thanks,

Takashi

Re: [RFC 01/10] ASoC: Intel: Boards: Machine driver for Intel platforms

[Pierre-Louis Bossart]

On 12/1/17 3:13 AM, Rakesh Ughreja wrote:
> Add machine driver for Intel platforms(SKL/KBL) with HDA and iDisp codecs.
> This patch adds support for only iDisp (HDMI/DP) codec. In the
> following patch support for HDA codec will be added.

But to be clear this should be sufficient for headless devices with no 
audio codec (Joule or UP^2) where the only audio output is HDMI/iDisp?

> 
> This should work for other Intel platforms as well e.g. BXT,GLK,CNL
> however this series is not tested on all the platforms.
> 
> Signed-off-by: Rakesh Ughreja <rakesh.a.ughreja@intel.com>
> ---
>   sound/soc/intel/boards/Kconfig           |  10 ++
>   sound/soc/intel/boards/Makefile          |   2 +
>   sound/soc/intel/boards/skl_hda_generic.c | 276 +++++++++++++++++++++++++++++++

can we drop the Skylake reference? It's become a catch-all term to mean 
both the platform, the IP and the driver.

>   3 files changed, 288 insertions(+)
>   create mode 100644 sound/soc/intel/boards/skl_hda_generic.c
> 
> diff --git a/sound/soc/intel/boards/Kconfig b/sound/soc/intel/boards/Kconfig
> index 6f75470..4f8bd02 100644
> --- a/sound/soc/intel/boards/Kconfig
> +++ b/sound/soc/intel/boards/Kconfig
> @@ -262,4 +262,14 @@ config SND_SOC_INTEL_KBL_RT5663_RT5514_MAX98927_MACH
>             Say Y if you have such a device.
>             If unsure select "N".
>   
> +config SND_SOC_INTEL_SKL_HDA_GENERIC_MACH
> +        tristate "ASoC Audio driver for SKL/KBL with HDA Codecs"
> +        select SND_SOC_INTEL_SST
> +        depends on SND_SOC_INTEL_SKYLAKE

that's also going to have to be reworked to allow for an SOF-based 
platform driver initializing this machine driver.

> +        select SND_SOC_HDAC_HDMI
> +        help
> +          This adds support for ASoC Onboard Codec HDA machine driver. This will
> +          create an alsa sound card for HDA Codecs.
> +          Say Y if you have such a device.
> +          If unsure select "N".
>   endif
> diff --git a/sound/soc/intel/boards/Makefile b/sound/soc/intel/boards/Makefile
> index 69d2dfa..4686727 100644
> --- a/sound/soc/intel/boards/Makefile
> +++ b/sound/soc/intel/boards/Makefile
> @@ -17,6 +17,7 @@ snd-soc-sst-byt-cht-nocodec-objs := bytcht_nocodec.o
>   snd-soc-kbl_rt5663_max98927-objs := kbl_rt5663_max98927.o
>   snd-soc-kbl_rt5663_rt5514_max98927-objs := kbl_rt5663_rt5514_max98927.o
>   snd-soc-skl_rt286-objs := skl_rt286.o
> +snd-soc-skl_hda_generic-objs := skl_hda_generic.o
>   snd-skl_nau88l25_max98357a-objs := skl_nau88l25_max98357a.o
>   snd-soc-skl_nau88l25_ssm4567-objs := skl_nau88l25_ssm4567.o
>   
> @@ -40,3 +41,4 @@ obj-$(CONFIG_SND_SOC_INTEL_KBL_RT5663_RT5514_MAX98927_MACH) += snd-soc-kbl_rt566
>   obj-$(CONFIG_SND_SOC_INTEL_SKL_RT286_MACH) += snd-soc-skl_rt286.o
>   obj-$(CONFIG_SND_SOC_INTEL_SKL_NAU88L25_MAX98357A_MACH) += snd-skl_nau88l25_max98357a.o
>   obj-$(CONFIG_SND_SOC_INTEL_SKL_NAU88L25_SSM4567_MACH) += snd-soc-skl_nau88l25_ssm4567.o
> +obj-$(CONFIG_SND_SOC_INTEL_SKL_HDA_GENERIC_MACH) += snd-soc-skl_hda_generic.o
> diff --git a/sound/soc/intel/boards/skl_hda_generic.c b/sound/soc/intel/boards/skl_hda_generic.c
> new file mode 100644
> index 0000000..ece39b5
> --- /dev/null
> +++ b/sound/soc/intel/boards/skl_hda_generic.c
> @@ -0,0 +1,276 @@
> +/*
> + * Intel Machine Driver for SKL/KBL platforms with HDA Codecs
> + *
> + * Copyright (C) 2017, Intel Corporation. All rights reserved.
> + *
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License version
> + * 2 as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + */
> +
> +#include <linux/module.h>
> +#include <linux/platform_device.h>
> +#include <sound/core.h>
> +#include <sound/jack.h>
> +#include <sound/pcm.h>
> +#include <sound/pcm_params.h>
> +#include <sound/soc.h>
> +#include "../../codecs/hdac_hdmi.h"
> +#include "../skylake/skl.h"
> +
> +static struct snd_soc_card skl_hda_audio_card;
> +static struct snd_soc_jack skl_hda_hdmi_jack[3];

The code from here to ...
> +
> +struct skl_hda_hdmi_pcm {
> +	struct list_head head;
> +	struct snd_soc_dai *codec_dai;
> +	int device;
> +};
> +
> +struct skl_hda_private {
> +	struct list_head hdmi_pcm_list;
> +};
> +
> +enum {
> +	SKL_HDA_DPCM_AUDIO_HDMI1_PB = 0,
> +	SKL_HDA_DPCM_AUDIO_HDMI2_PB,
> +	SKL_HDA_DPCM_AUDIO_HDMI3_PB,
> +};
> +
> +static const struct snd_soc_dapm_route skl_hda_map[] = {
> +
> +	{ "hifi3", NULL, "iDisp3 Tx"},
> +	{ "iDisp3 Tx", NULL, "iDisp3_out"},
> +	{ "hifi2", NULL, "iDisp2 Tx"},
> +	{ "iDisp2 Tx", NULL, "iDisp2_out"},
> +	{ "hifi1", NULL, "iDisp1 Tx"},
> +	{ "iDisp1 Tx", NULL, "iDisp1_out"},
> +
> +};
> +
> +static int skl_hda_hdmi1_init(struct snd_soc_pcm_runtime *rtd)
> +{
> +	struct skl_hda_private *ctx = snd_soc_card_get_drvdata(rtd->card);
> +	struct snd_soc_dai *dai = rtd->codec_dai;
> +	struct skl_hda_hdmi_pcm *pcm;
> +
> +	pcm = devm_kzalloc(rtd->card->dev, sizeof(*pcm), GFP_KERNEL);
> +	if (!pcm)
> +		return -ENOMEM;
> +
> +	pcm->device = SKL_HDA_DPCM_AUDIO_HDMI1_PB;
> +	pcm->codec_dai = dai;
> +
> +	list_add_tail(&pcm->head, &ctx->hdmi_pcm_list);
> +
> +	return 0;
> +}
> +
> +static int skl_hda_hdmi2_init(struct snd_soc_pcm_runtime *rtd)
> +{
> +	struct skl_hda_private *ctx = snd_soc_card_get_drvdata(rtd->card);
> +	struct snd_soc_dai *dai = rtd->codec_dai;
> +	struct skl_hda_hdmi_pcm *pcm;
> +
> +	pcm = devm_kzalloc(rtd->card->dev, sizeof(*pcm), GFP_KERNEL);
> +	if (!pcm)
> +		return -ENOMEM;
> +
> +	pcm->device = SKL_HDA_DPCM_AUDIO_HDMI2_PB;
> +	pcm->codec_dai = dai;
> +
> +	list_add_tail(&pcm->head, &ctx->hdmi_pcm_list);
> +
> +	return 0;
> +}
> +
> +static int skl_hda_hdmi3_init(struct snd_soc_pcm_runtime *rtd)
> +{
> +	struct skl_hda_private *ctx = snd_soc_card_get_drvdata(rtd->card);
> +	struct snd_soc_dai *dai = rtd->codec_dai;
> +	struct skl_hda_hdmi_pcm *pcm;
> +
> +	pcm = devm_kzalloc(rtd->card->dev, sizeof(*pcm), GFP_KERNEL);
> +	if (!pcm)
> +		return -ENOMEM;
> +
> +	pcm->device = SKL_HDA_DPCM_AUDIO_HDMI3_PB;
> +	pcm->codec_dai = dai;
> +
> +	list_add_tail(&pcm->head, &ctx->hdmi_pcm_list);
> +
> +	return 0;
> +}
> +

.. here is pretty much copy/pasted across machine drivers, can we move 
it to a common include file? The experience from Baytrail/Cherrytrail is 
that the more we wait the more complicated it is to factor the code.

> +/* skl_hda_digital audio interface glue - connects codec <--> CPU */
> +static struct snd_soc_dai_link skl_hda_dais[] = {
> +	/* Front End DAI links */
> +	[SKL_HDA_DPCM_AUDIO_HDMI1_PB] = {

Are those hard-coded links required for all platforms, I thought the 
newer ones were based on topology?

> +		.name = "SKL HDA HDMI Port1",
> +		.stream_name = "Hdmi1",
> +		.cpu_dai_name = "HDMI1 Pin",
> +		.codec_name = "snd-soc-dummy",
> +		.codec_dai_name = "snd-soc-dummy-dai",
> +		.platform_name = "0000:00:1f.3",
> +		.dpcm_playback = 1,
> +		.init = NULL,
> +		.trigger = {
> +			SND_SOC_DPCM_TRIGGER_POST, SND_SOC_DPCM_TRIGGER_POST},
> +		.nonatomic = 1,
> +		.dynamic = 1,
> +	},
> +	[SKL_HDA_DPCM_AUDIO_HDMI2_PB] = {
> +		.name = "SKL HDA HDMI Port2",
> +		.stream_name = "Hdmi2",
> +		.cpu_dai_name = "HDMI2 Pin",
> +		.codec_name = "snd-soc-dummy",
> +		.codec_dai_name = "snd-soc-dummy-dai",
> +		.platform_name = "0000:00:1f.3",
> +		.dpcm_playback = 1,
> +		.init = NULL,
> +		.trigger = {
> +			SND_SOC_DPCM_TRIGGER_POST, SND_SOC_DPCM_TRIGGER_POST},
> +		.nonatomic = 1,
> +		.dynamic = 1,
> +	},
> +	[SKL_HDA_DPCM_AUDIO_HDMI3_PB] = {
> +		.name = "SKL HDA HDMI Port3",
> +		.stream_name = "Hdmi3",
> +		.cpu_dai_name = "HDMI3 Pin",
> +		.codec_name = "snd-soc-dummy",
> +		.codec_dai_name = "snd-soc-dummy-dai",
> +		.platform_name = "0000:00:1f.3",
> +		.trigger = {
> +			SND_SOC_DPCM_TRIGGER_POST, SND_SOC_DPCM_TRIGGER_POST},
> +		.dpcm_playback = 1,
> +		.init = NULL,
> +		.nonatomic = 1,
> +		.dynamic = 1,
> +	},
> +
> +	/* Back End DAI links */
> +	{
> +		.name = "iDisp1",
> +		.id = 1,
> +		.cpu_dai_name = "iDisp1 Pin",
> +		.codec_name = "ehdaudio0D2",
> +		.codec_dai_name = "intel-hdmi-hifi1",
> +		.platform_name = "0000:00:1f.3",
> +		.dpcm_playback = 1,
> +		.init = skl_hda_hdmi1_init,
> +		.no_pcm = 1,
> +	},
> +	{
> +		.name = "iDisp2",
> +		.id = 2,
> +		.cpu_dai_name = "iDisp2 Pin",
> +		.codec_name = "ehdaudio0D2",
> +		.codec_dai_name = "intel-hdmi-hifi2",
> +		.platform_name = "0000:00:1f.3",
> +		.init = skl_hda_hdmi2_init,
> +		.dpcm_playback = 1,
> +		.no_pcm = 1,
> +	},
> +	{
> +		.name = "iDisp3",
> +		.id = 3,
> +		.cpu_dai_name = "iDisp3 Pin",
> +		.codec_name = "ehdaudio0D2",
> +		.codec_dai_name = "intel-hdmi-hifi3",
> +		.platform_name = "0000:00:1f.3",
> +		.init = skl_hda_hdmi3_init,
> +		.dpcm_playback = 1,
> +		.no_pcm = 1,
> +	},
> +};
> +
> +#define NAME_SIZE	32
> +static int skl_hda_card_late_probe(struct snd_soc_card *card)
> +{
> +	struct skl_hda_private *ctx = snd_soc_card_get_drvdata(card);
> +	struct skl_hda_hdmi_pcm *pcm;
> +	struct snd_soc_codec *codec = NULL;
> +	int err, i = 0;
> +	char jack_name[NAME_SIZE];
> +
> +	list_for_each_entry(pcm, &ctx->hdmi_pcm_list, head) {
> +		codec = pcm->codec_dai->codec;
> +		snprintf(jack_name, sizeof(jack_name),
> +			"HDMI/DP, pcm=%d Jack", pcm->device);
> +		err = snd_soc_card_jack_new(card, jack_name,
> +					SND_JACK_AVOUT, &skl_hda_hdmi_jack[i],
> +					NULL, 0);
> +
> +		if (err)
> +			return err;
> +
> +		err = hdac_hdmi_jack_init(pcm->codec_dai, pcm->device,
> +						&skl_hda_hdmi_jack[i]);
> +		if (err < 0)
> +			return err;
> +
> +		i++;
> +	}
> +
> +	if (!codec)
> +		return -EINVAL;
> +
> +	return hdac_hdmi_jack_port_init(codec, &card->dapm);
> +}

this one in always the same as well, it could be factored across machine 
drivers.

> +
> +static struct snd_soc_card skl_hda_audio_card = {
> +	.name = "skl_hda_card",
> +	.owner = THIS_MODULE,
> +	.dai_link = skl_hda_dais,
> +	.num_links = ARRAY_SIZE(skl_hda_dais),
> +	.dapm_routes = skl_hda_map,
> +	.num_dapm_routes = ARRAY_SIZE(skl_hda_map),
> +	.fully_routed = true,
> +	.late_probe = skl_hda_card_late_probe,
> +};
> +
> +static int skl_hda_audio_probe(struct platform_device *pdev)
> +{
> +	struct skl_hda_private *ctx;
> +
> +	dev_dbg(&pdev->dev, "%s: machine driver probe got called\n", __func__);
> +
> +	ctx = devm_kzalloc(&pdev->dev, sizeof(*ctx), GFP_ATOMIC);
> +	if (!ctx)
> +		return -ENOMEM;
> +
> +	INIT_LIST_HEAD(&ctx->hdmi_pcm_list);
> +
> +	skl_hda_audio_card.dev = &pdev->dev;
> +	snd_soc_card_set_drvdata(&skl_hda_audio_card, ctx);
> +
> +	return devm_snd_soc_register_card(&pdev->dev, &skl_hda_audio_card);
> +}
> +
> +static const struct platform_device_id skl_hda_board_ids[] = {
> +	{ .name = "skl_hda_generic" },
> +	{ }
> +};
> +
> +static struct platform_driver skl_hda_audio = {
> +	.probe = skl_hda_audio_probe,
> +	.driver = {
> +		.name = "skl_hda_generic",
> +		.pm = &snd_soc_pm_ops,
> +	},
> +	.id_table = skl_hda_board_ids,
> +};
> +
> +module_platform_driver(skl_hda_audio)
> +
> +/* Module information */
> +MODULE_DESCRIPTION("SKL/KBL HDA Generic Machine driver");

again it's not limited to SKL/KBL, it'll work on APL and CNL and GLK ...

> +MODULE_AUTHOR("Rakesh Ughreja <rakesh.a.ughreja@intel.com>");
> +MODULE_LICENSE("GPL v2");
> +MODULE_ALIAS("platform:skl_hda_generic");
> 

Re: [RFC 02/10] ASoC: Intel: Skylake: Add entry in sst_acpi_mach for HDA codecs

[Pierre-Louis Bossart]

On 12/1/17 3:14 AM, Rakesh Ughreja wrote:
> When no I2S based codecs are detected in the BIOS, check if there are
> any HDA codecs present. If yes, load the corresponding machine driver.

Is this completely correct?
I may be splitting hair but I believe it's possible to have an HDAudio 
codec connected to some pins and an I2S/PDM device on non-conflicting 
pins. The PDM case is very real and isn't related to I2S

You may also want to explain that you *have* to look for non-HDaudio 
stuff first, because otherwise the iDisp codec will be detected but that 
is not sufficient to load the HDaudio machine driver (could be 
iDisp+I2S+PDM)

> 
> TODO:
> support for detecting the presence of HDA codec is not implemented.
> it will be implemented in the next revision.
> 
> Signed-off-by: Rakesh Ughreja <rakesh.a.ughreja@intel.com>
> ---
>   sound/soc/intel/skylake/skl.c | 46 ++++++++++++++++++++++++++++++++++++++++---
>   1 file changed, 43 insertions(+), 3 deletions(-)
> 
> diff --git a/sound/soc/intel/skylake/skl.c b/sound/soc/intel/skylake/skl.c
> index 27462c0..568a285 100644
> --- a/sound/soc/intel/skylake/skl.c
> +++ b/sound/soc/intel/skylake/skl.c
> @@ -424,6 +424,26 @@ static int skl_free(struct hdac_bus *bus)
>   	return 0;
>   }
>   
> +static struct snd_soc_acpi_mach *skl_probe_hda_machine(
> +					struct snd_soc_acpi_mach *machines)
> +{
> +
> +	struct snd_soc_acpi_mach *mach;
> +
> +	/*
> +	 * FIXME:
> +	 * First check if there are any HDA codecs present on the system
> +	 * then search the match table.
> +	 * For now this function is not detecting the presence of any
> +	 * HDA codecs.

D'oh. so how was this patchset tested then?

> +	 */
> +	for (mach = machines; mach->id[0]; mach++) {
> +		if (!strcmp(mach->id, "HDA_GEN"))

Do you mean to say this was a temporary hack assuming a modified DSDT to 
force the detection of the codec?

And while I am at it in the hair-splitting department, in theory HDAudio 
can work with multiple codecs (each using its own SDI line), did you 
think of this case and how would it impact the machine driver selection?

> +			return mach;
> +	}
> +	return NULL;
> +}
> +
>   static int skl_machine_device_register(struct skl *skl, void *driver_data)
>   {
>   	struct hdac_bus *bus = skl_to_bus(skl);
> @@ -433,9 +453,14 @@ static int skl_machine_device_register(struct skl *skl, void *driver_data)
>   
>   	mach = snd_soc_acpi_find_machine(mach);
>   	if (mach == NULL) {
> -		dev_err(bus->dev, "No matching machine driver found\n");
> -		return -ENODEV;
> +		dev_dbg(bus->dev, "No matching I2S machine driver found\n");
> +		mach = skl_probe_hda_machine(driver_data);
> +		if (mach == NULL) {
> +			dev_err(bus->dev, "No matching machine driver found\n");
> +			return -ENODEV;
> +		}
>   	}
> +
>   	skl->fw_name = mach->fw_filename;
>   
>   	pdev = platform_device_alloc(mach->drv_name, -1);
> @@ -911,6 +936,14 @@ static struct snd_soc_acpi_mach sst_skl_devdata[] = {
>   		.quirk_data = &skl_codecs,
>   		.pdata = &skl_dmic_data
>   	},
> +	{
> +		.id = "HDA_GEN",
> +		.drv_name = "skl_hda_generic",
> +		.fw_filename = "intel/dsp_fw_release.bin",
> +		.machine_quirk = NULL,
> +		.quirk_data = NULL,
> +		.pdata = NULL
> +	},
>   	{}
>   };
>   
> @@ -973,7 +1006,14 @@ static struct snd_soc_acpi_mach sst_kbl_devdata[] = {
>   		.drv_name = "kbl_rt5663",
>   		.fw_filename = "intel/dsp_fw_kbl.bin",
>   	},
> -
> +	{
> +		.id = "HDA_GEN",
> +		.drv_name = "skl_hda_generic",
> +		.fw_filename = "intel/dsp_fw_kbl.bin",
> +		.machine_quirk = NULL,
> +		.quirk_data = NULL,
> +		.pdata = NULL
> +	},
>   	{}
>   };
>   
> 

Re: [RFC 03/10] ASoC: Intel: Skylake: add HDA BE DAIs

[Pierre-Louis Bossart]

On 12/1/17 3:14 AM, Rakesh Ughreja wrote:
> Add support for HDA BE DAIs in SKL platform driver.
> 
> Signed-off-by: Rakesh Ughreja <rakesh.a.ughreja@intel.com>
> ---
>   sound/soc/intel/skylake/skl-pcm.c | 32 +++++++++++++++++++++++++-------
>   1 file changed, 25 insertions(+), 7 deletions(-)
> 
> diff --git a/sound/soc/intel/skylake/skl-pcm.c b/sound/soc/intel/skylake/skl-pcm.c
> index e6fee7a..3dcb241 100644
> --- a/sound/soc/intel/skylake/skl-pcm.c
> +++ b/sound/soc/intel/skylake/skl-pcm.c
> @@ -956,21 +956,39 @@ static struct snd_soc_dai_driver skl_platform_dai[] = {
>   	},
>   },
>   {
> -	.name = "HD-Codec Pin",
> +	.name = "Analog CPU DAI",
>   	.ops = &skl_link_dai_ops,
>   	.playback = {
> -		.stream_name = "HD-Codec Tx",
> -		.channels_min = HDA_STEREO,
> +		.stream_name = "Analog CPU Playback",
> +		.channels_min = HDA_MONO,
>   		.channels_max = HDA_STEREO,
>   		.rates = SNDRV_PCM_RATE_48000,
> -		.formats = SNDRV_PCM_FMTBIT_S16_LE,
> +		.formats = SNDRV_PCM_FMTBIT_S16_LE | SNDRV_PCM_FMTBIT_S24_LE,
>   	},
>   	.capture = {
> -		.stream_name = "HD-Codec Rx",
> -		.channels_min = HDA_STEREO,
> +		.stream_name = "Analog CPU Capture",
> +		.channels_min = HDA_MONO,
>   		.channels_max = HDA_STEREO,
>   		.rates = SNDRV_PCM_RATE_48000,
> -		.formats = SNDRV_PCM_FMTBIT_S16_LE,
> +		.formats = SNDRV_PCM_FMTBIT_S16_LE | SNDRV_PCM_FMTBIT_S24_LE,
> +	},
> +},
> +{
> +	.name = "Digital CPU DAI",

I am not sure I get the meaning of analog/digital.

Did you mean external codec vs. iDisp codec?

Or did you really mean analog? I don't think you can control this front 
the CPU side, e.g. if the codec has an S/PDIF output it remains digital.

> +	.ops = &skl_link_dai_ops,
> +	.playback = {
> +		.stream_name = "Digital CPU Playback",
> +		.channels_min = HDA_MONO,
> +		.channels_max = HDA_STEREO,
> +		.rates = SNDRV_PCM_RATE_48000,
> +		.formats = SNDRV_PCM_FMTBIT_S16_LE | SNDRV_PCM_FMTBIT_S24_LE,
> +	},
> +	.capture = {
> +		.stream_name = "Digital CPU Capture",
> +		.channels_min = HDA_MONO,
> +		.channels_max = HDA_STEREO,
> +		.rates = SNDRV_PCM_RATE_48000,
> +		.formats = SNDRV_PCM_FMTBIT_S16_LE | SNDRV_PCM_FMTBIT_S24_LE,
>   	},
>   },
>   };
> 

Re: [RFC 04/10] ASoC: Intel: Skylake: use hda_bus instead of hdac_bus

[Pierre-Louis Bossart]

On 12/1/17 3:14 AM, Rakesh Ughreja wrote:
> This patch prepares SKL platform driver to make reuse of legacy HDA
> codec drivers. It does following things.
> 
> use hda_bus instead of hdac_bus in the SKL platform driver to align
> with the legacy controller driver.
> 
> modify snd_hdac_ext_bus_device_init definition to align with
> snd_hdac_bus_device_init, used by legacy drivers.
> Memory for hdac_device is allocated by the caller.
> 
> Signed-off-by: Rakesh Ughreja <rakesh.a.ughreja@intel.com>
> ---
>   include/sound/hdaudio_ext.h   |  3 ++-
>   sound/hda/ext/hdac_ext_bus.c  |  9 ++-------
>   sound/soc/intel/skylake/skl.c | 19 ++++++++++++++++++-
>   sound/soc/intel/skylake/skl.h | 10 +++++++---
>   4 files changed, 29 insertions(+), 12 deletions(-)
> 
> diff --git a/include/sound/hdaudio_ext.h b/include/sound/hdaudio_ext.h
> index 3c30247..c188b80 100644
> --- a/include/sound/hdaudio_ext.h
> +++ b/include/sound/hdaudio_ext.h
> @@ -9,7 +9,8 @@ int snd_hdac_ext_bus_init(struct hdac_bus *bus, struct device *dev,
>   		      const struct hdac_io_ops *io_ops);
>   
>   void snd_hdac_ext_bus_exit(struct hdac_bus *bus);
> -int snd_hdac_ext_bus_device_init(struct hdac_bus *bus, int addr);
> +int snd_hdac_ext_bus_device_init(struct hdac_bus *bus, int addr,
> +						struct hdac_device *hdev);
>   void snd_hdac_ext_bus_device_exit(struct hdac_device *hdev);
>   void snd_hdac_ext_bus_device_remove(struct hdac_bus *bus);
>   
> diff --git a/sound/hda/ext/hdac_ext_bus.c b/sound/hda/ext/hdac_ext_bus.c
> index 52f0776..e4bcb76 100644
> --- a/sound/hda/ext/hdac_ext_bus.c
> +++ b/sound/hda/ext/hdac_ext_bus.c
> @@ -135,16 +135,12 @@ static void default_release(struct device *dev)
>    *
>    * Returns zero for success or a negative error code.
>    */
> -int snd_hdac_ext_bus_device_init(struct hdac_bus *bus, int addr)
> +int snd_hdac_ext_bus_device_init(struct hdac_bus *bus, int addr,
> +					struct hdac_device *hdev)
>   {
> -	struct hdac_device *hdev = NULL;
>   	char name[15];
>   	int ret;
>   
> -	hdev = kzalloc(sizeof(*hdev), GFP_KERNEL);
> -	if (!hdev)
> -		return -ENOMEM;
> -
>   	hdev->bus = bus;
>   
>   	snprintf(name, sizeof(name), "ehdaudio%dD%d", bus->idx, addr);
> @@ -175,7 +171,6 @@ EXPORT_SYMBOL_GPL(snd_hdac_ext_bus_device_init);
>   void snd_hdac_ext_bus_device_exit(struct hdac_device *hdev)
>   {
>   	snd_hdac_device_exit(hdev);
> -	kfree(hdev);
>   }
>   EXPORT_SYMBOL_GPL(snd_hdac_ext_bus_device_exit);
>   
> diff --git a/sound/soc/intel/skylake/skl.c b/sound/soc/intel/skylake/skl.c
> index 568a285..72a788a 100644
> --- a/sound/soc/intel/skylake/skl.c
> +++ b/sound/soc/intel/skylake/skl.c
> @@ -35,6 +35,7 @@
>   #include "skl.h"
>   #include "skl-sst-dsp.h"
>   #include "skl-sst-ipc.h"
> +#include "../../../pci/hda/hda_codec.h"
>   
>   static struct skl_machine_pdata skl_dmic_data;
>   
> @@ -531,6 +532,8 @@ static int probe_codec(struct hdac_bus *bus, int addr)
>   	unsigned int cmd = (addr << 28) | (AC_NODE_ROOT << 20) |
>   		(AC_VERB_PARAMETERS << 8) | AC_PAR_VENDOR_ID;
>   	unsigned int res = -1;
> +	struct skl *skl = bus_to_skl(bus);
> +	struct hdac_device *hdev;
>   
>   	mutex_lock(&bus->cmd_mutex);
>   	snd_hdac_bus_send_cmd(bus, cmd);
> @@ -540,7 +543,11 @@ static int probe_codec(struct hdac_bus *bus, int addr)
>   		return -EIO;
>   	dev_dbg(bus->dev, "codec #%d probed OK\n", addr);
>   
> -	return snd_hdac_ext_bus_device_init(bus, addr);
> +	hdev = devm_kzalloc(&skl->pci->dev, sizeof(*hdev), GFP_KERNEL);
> +	if (!hdev)
> +		return -ENOMEM;
> +
> +	return snd_hdac_ext_bus_device_init(bus, addr, hdev);
>   }
>   
>   /* Codec initialization */
> @@ -665,6 +672,7 @@ static int skl_create(struct pci_dev *pci,
>   {
>   	struct skl *skl;
>   	struct hdac_bus *bus;
> +	struct hda_bus *hbus;
>   
>   	int err;
>   
> @@ -680,6 +688,7 @@ static int skl_create(struct pci_dev *pci,
>   		return -ENOMEM;
>   	}
>   
> +	hbus = skl_to_hbus(skl);
>   	bus = skl_to_bus(skl);
>   	snd_hdac_ext_bus_init(bus, &pci->dev, &bus_core_ops, io_ops);
>   	bus->use_posbuf = 1;
> @@ -687,6 +696,14 @@ static int skl_create(struct pci_dev *pci,
>   	INIT_WORK(&skl->probe_work, skl_probe_work);
>   	bus->bdl_pos_adj = 0;
>   
> +	/*
> +	 * TODO: other parameters can be taken the way it is taken by
> +	 * legacy HDA driver

what parameters where you referring to? kernel module?

> +	 */
> +	mutex_init(&hbus->prepare_mutex);
> +	hbus->pci = pci;
> +	hbus->mixer_assigned = -1;
> +
>   	*rskl = skl;
>   
>   	return 0;
> diff --git a/sound/soc/intel/skylake/skl.h b/sound/soc/intel/skylake/skl.h
> index f7fcd7e..c6b7aee 100644
> --- a/sound/soc/intel/skylake/skl.h
> +++ b/sound/soc/intel/skylake/skl.h
> @@ -25,6 +25,7 @@
>   #include <sound/hdaudio_ext.h>
>   #include <sound/soc.h>
>   #include "skl-nhlt.h"
> +#include "../../../pci/hda/hda_codec.h"
>   
>   #define SKL_SUSPEND_DELAY 2000
>   
> @@ -46,7 +47,7 @@ struct skl_dsp_resource {
>   struct skl_debug;
>   
>   struct skl {
> -	struct hdac_bus hbus;
> +	struct hda_bus hbus;
>   	struct pci_dev *pci;
>   
>   	unsigned int init_done:1; /* delayed init status */
> @@ -77,8 +78,11 @@ struct skl {
>   	bool use_tplg_pcm;
>   };
>   
> -#define skl_to_bus(s)  (&(s)->hbus)
> -#define bus_to_skl(bus) container_of(bus, struct skl, hbus)
> +#define skl_to_bus(s)  (&(s)->hbus.core)
> +#define bus_to_skl(bus) container_of(bus, struct skl, hbus.core)
> +
> +#define skl_to_hbus(s) (&(s)->hbus)
> +#define hbus_to_skl(hbus) container_of(hbus, struct skl, hbus)
>   
>   /* to pass dai dma data */
>   struct skl_dma_params {
> 

Re: [RFC 05/10] ALSA: hda - make some of the functions externally visible

[Pierre-Louis Bossart]

On 12/1/17 3:14 AM, Rakesh Ughreja wrote:
> Mark some functions with EXPORT_SYMBOL_GPL so that it can be called by
> other kernel modules. These APIs would be called by ASoC based HDA codec
> driver which will be added in the later patches.

Looks ok. Just wondering from a code maintenance perspective if those 
functions would need to be part of a library, and if this means that to 
compile the ASoC based HDAudio codec support you still need to enable 
the legacy support - and if this leads to potential conflicts on which 
driver gets probed?

> 
> Signed-off-by: Rakesh Ughreja <rakesh.a.ughreja@intel.com>
> ---
>   sound/pci/hda/hda_codec.c | 9 +++++++--
>   sound/pci/hda/hda_codec.h | 4 +++-
>   2 files changed, 10 insertions(+), 3 deletions(-)
> 
> diff --git a/sound/pci/hda/hda_codec.c b/sound/pci/hda/hda_codec.c
> index e018ecb..085fd9e 100644
> --- a/sound/pci/hda/hda_codec.c
> +++ b/sound/pci/hda/hda_codec.c
> @@ -2903,7 +2903,7 @@ static void hda_call_codec_resume(struct hda_codec *codec)
>   	atomic_dec(&codec->core.in_pm);
>   }
>   
> -static int hda_codec_runtime_suspend(struct device *dev)
> +int hda_codec_runtime_suspend(struct device *dev)
>   {
>   	struct hda_codec *codec = dev_to_hda_codec(dev);
>   	struct hda_pcm *pcm;
> @@ -2919,8 +2919,9 @@ static int hda_codec_runtime_suspend(struct device *dev)
>   	snd_hdac_link_power(&codec->core, false);
>   	return 0;
>   }
> +EXPORT_SYMBOL_GPL(hda_codec_runtime_suspend);
>   
> -static int hda_codec_runtime_resume(struct device *dev)
> +int hda_codec_runtime_resume(struct device *dev)
>   {
>   	struct hda_codec *codec = dev_to_hda_codec(dev);
>   
> @@ -2930,6 +2931,7 @@ static int hda_codec_runtime_resume(struct device *dev)
>   	pm_runtime_mark_last_busy(dev);
>   	return 0;
>   }
> +EXPORT_SYMBOL_GPL(hda_codec_runtime_resume);
>   #endif /* CONFIG_PM */
>   
>   /* referred in hda_bind.c */
> @@ -3005,6 +3007,7 @@ int snd_hda_codec_build_controls(struct hda_codec *codec)
>   	sync_power_up_states(codec);
>   	return 0;
>   }
> +EXPORT_SYMBOL_GPL(snd_hda_codec_build_controls);
>   
>   /*
>    * PCM stuff
> @@ -3210,6 +3213,7 @@ int snd_hda_codec_parse_pcms(struct hda_codec *codec)
>   
>   	return 0;
>   }
> +EXPORT_SYMBOL_GPL(snd_hda_codec_parse_pcms);
>   
>   /* assign all PCMs of the given codec */
>   int snd_hda_codec_build_pcms(struct hda_codec *codec)
> @@ -3246,6 +3250,7 @@ int snd_hda_codec_build_pcms(struct hda_codec *codec)
>   
>   	return 0;
>   }
> +EXPORT_SYMBOL_GPL(snd_hda_codec_build_pcms);
>   
>   /**
>    * snd_hda_add_new_ctls - create controls from the array
> diff --git a/sound/pci/hda/hda_codec.h b/sound/pci/hda/hda_codec.h
> index 681c360..2d02d02 100644
> --- a/sound/pci/hda/hda_codec.h
> +++ b/sound/pci/hda/hda_codec.h
> @@ -83,7 +83,7 @@ struct hda_bus {
>    * better than the generic parser.
>    */
>   typedef int (*hda_codec_patch_t)(struct hda_codec *);
> -	
> +
>   #define HDA_CODEC_ID_GENERIC_HDMI	0x00000101
>   #define HDA_CODEC_ID_GENERIC		0x00000201
>   
> @@ -497,6 +497,8 @@ int hda_call_check_power_status(struct hda_codec *codec, hda_nid_t nid)
>   #ifdef CONFIG_PM
>   void snd_hda_set_power_save(struct hda_bus *bus, int delay);
>   void snd_hda_update_power_acct(struct hda_codec *codec);
> +int hda_codec_runtime_suspend(struct device *dev);
> +int hda_codec_runtime_resume(struct device *dev);
>   #else
>   static inline void snd_hda_set_power_save(struct hda_bus *bus, int delay) {}
>   #endif
> 

Re: [RFC 06/10] ASoC: hdac_hda: add ASoC based HDA codec driver

[Pierre-Louis Bossart]

On 12/1/17 3:14 AM, Rakesh Ughreja wrote:
> This patch adds ASoC based HDA codec driver that can be used with
> all Intel platforms.
> 
> FIXME:
> KConfig change allows users to compile legacy HDA codec drivers either
> as ASoC or ALSA codec driver. This is not a good approach and need
> suggestion to improve it.
> 
> Signed-off-by: Rakesh Ughreja <rakesh.a.ughreja@intel.com>
> ---
>   sound/pci/hda/Kconfig         | 11 +++++
>   sound/pci/hda/hda_codec.h     | 26 +++++++++++-
>   sound/soc/codecs/Kconfig      |  6 +++
>   sound/soc/codecs/Makefile     |  2 +
>   sound/soc/codecs/hdac_hda.c   | 95 +++++++++++++++++++++++++++++++++++++++++++
>   sound/soc/codecs/hdac_hda.h   | 16 ++++++++
>   sound/soc/intel/skylake/skl.c | 18 +++++++-
>   7 files changed, 171 insertions(+), 3 deletions(-)
>   create mode 100644 sound/soc/codecs/hdac_hda.c
>   create mode 100644 sound/soc/codecs/hdac_hda.h
> 
> diff --git a/sound/pci/hda/Kconfig b/sound/pci/hda/Kconfig
> index 7f3b5ed..010dd7d 100644
> --- a/sound/pci/hda/Kconfig
> +++ b/sound/pci/hda/Kconfig
> @@ -215,6 +215,17 @@ config SND_HDA_GENERIC
>   comment "Set to Y if you want auto-loading the codec driver"
>   	depends on SND_HDA=y && SND_HDA_GENERIC=m
>   
> +config SND_ASOC_HDA_GENERIC
> +        bool "Build HDA drivers as HDA ASoC Codec drivers"
> +	select SND_SOC_HDAC_HDA
> +        def_bool n
> +        help
> +          Select to build legacy HD-audio codec drivers as ASoC
> +          codec drivers.
> +
> +comment "Set to Y if you want to enable support for HDA audio codec with
> +	 DSP on Intel platforms"
> +
>   config SND_HDA_POWER_SAVE_DEFAULT
>   	int "Default time-out for HD-audio power-save mode"
>   	depends on PM
> diff --git a/sound/pci/hda/hda_codec.h b/sound/pci/hda/hda_codec.h
> index 2d02d02..67ce2e6 100644
> --- a/sound/pci/hda/hda_codec.h
> +++ b/sound/pci/hda/hda_codec.h
> @@ -104,9 +104,33 @@ int __hda_codec_driver_register(struct hda_codec_driver *drv, const char *name,
>   #define hda_codec_driver_register(drv) \
>   	__hda_codec_driver_register(drv, KBUILD_MODNAME, THIS_MODULE)
>   void hda_codec_driver_unregister(struct hda_codec_driver *drv);
> +
> +/*
> + * FIXME:
> + * In this implementation one can compile codec drivers either as ALSA or as
> + * ASoC, but not both.
> + * This may be not the be the best way to build the HDA codec drivers as ALSA
> + * vs ASoC driver. Need to find a better way, where the driver can be built
> + * as ALSA as well as ASoC at the same time. Even though practically people
> + * would never use them at the same time. But may be disto guys may want to
> + * do it.

Distros *have* to do it, since the DSP usage can be changed in 
user-visible BIOS settings or disabled.

> + */
> +
> +#ifdef CONFIG_SND_ASOC_HDA_GENERIC
> +int __hdac_hda_codec_driver_register(struct hda_codec_driver *drv,
> +	const char *name, struct module *owner);
> +#define hdac_hda_codec_driver_register(drv) \
> +	__hdac_hda_codec_driver_register(drv, KBUILD_MODNAME, THIS_MODULE)
> +void hdac_hda_codec_driver_unregister(struct hda_codec_driver *drv);
> +
> +#define module_hda_codec_driver(drv) \
> +	module_driver(drv, hdac_hda_codec_driver_register, \
> +			hdac_hda_codec_driver_unregister)
> +#else
>   #define module_hda_codec_driver(drv) \
>   	module_driver(drv, hda_codec_driver_register, \
> -		      hda_codec_driver_unregister)
> +			hda_codec_driver_unregister)
> +#endif

Couldn't you use a common registration that internally checks if you are 
running in legacy or DSP/ASoC mode and branch accordingly?
>   
>   /* ops set by the preset patch */
>   struct hda_codec_ops {
> diff --git a/sound/soc/codecs/Kconfig b/sound/soc/codecs/Kconfig
> index a42ddbc..196fcbc 100644
> --- a/sound/soc/codecs/Kconfig
> +++ b/sound/soc/codecs/Kconfig
> @@ -79,6 +79,7 @@ config SND_SOC_ALL_CODECS
>   	select SND_SOC_ES7134
>   	select SND_SOC_GTM601
>   	select SND_SOC_HDAC_HDMI
> +	select SND_SOC_HDAC_HDA
>   	select SND_SOC_ICS43432
>   	select SND_SOC_INNO_RK3036
>   	select SND_SOC_ISABELLE if I2C
> @@ -578,6 +579,11 @@ config SND_SOC_HDAC_HDMI
>   	select SND_PCM_ELD
>   	select HDMI
>   
> +config SND_SOC_HDAC_HDA
> +	tristate
> +	select SND_HDA_EXT_CORE
> +	select SND_PCM_ELD
> +
>   config SND_SOC_ICS43432
>   	tristate
>   
> diff --git a/sound/soc/codecs/Makefile b/sound/soc/codecs/Makefile
> index 0001069..84b33c9 100644
> --- a/sound/soc/codecs/Makefile
> +++ b/sound/soc/codecs/Makefile
> @@ -73,6 +73,7 @@ snd-soc-es8328-i2c-objs := es8328-i2c.o
>   snd-soc-es8328-spi-objs := es8328-spi.o
>   snd-soc-gtm601-objs := gtm601.o
>   snd-soc-hdac-hdmi-objs := hdac_hdmi.o
> +snd-soc-hdac-hda-objs := hdac_hda.o
>   snd-soc-ics43432-objs := ics43432.o
>   snd-soc-inno-rk3036-objs := inno_rk3036.o
>   snd-soc-isabelle-objs := isabelle.o
> @@ -313,6 +314,7 @@ obj-$(CONFIG_SND_SOC_ES8328_I2C)+= snd-soc-es8328-i2c.o
>   obj-$(CONFIG_SND_SOC_ES8328_SPI)+= snd-soc-es8328-spi.o
>   obj-$(CONFIG_SND_SOC_GTM601)    += snd-soc-gtm601.o
>   obj-$(CONFIG_SND_SOC_HDAC_HDMI) += snd-soc-hdac-hdmi.o
> +obj-$(CONFIG_SND_SOC_HDAC_HDA) += snd-soc-hdac-hda.o
>   obj-$(CONFIG_SND_SOC_ICS43432)	+= snd-soc-ics43432.o
>   obj-$(CONFIG_SND_SOC_INNO_RK3036)	+= snd-soc-inno-rk3036.o
>   obj-$(CONFIG_SND_SOC_ISABELLE)	+= snd-soc-isabelle.o
> diff --git a/sound/soc/codecs/hdac_hda.c b/sound/soc/codecs/hdac_hda.c
> new file mode 100644
> index 0000000..f9a3491
> --- /dev/null
> +++ b/sound/soc/codecs/hdac_hda.c
> @@ -0,0 +1,95 @@
> +/*
> + *  hdac_hda.c - ASoc HDA-HDA codec driver for Intel platforms
> + *
> + *  Copyright (C) 2015-2017 Intel Corp
> + *
> + *  ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> + *
> + *  This program is free software; you can redistribute it and/or modify
> + *  it under the terms of the GNU General Public License as published by
> + *  the Free Software Foundation; version 2 of the License.
> + *
> + *  This program is distributed in the hope that it will be useful, but
> + *  WITHOUT ANY WARRANTY; without even the implied warranty of
> + *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> + *  General Public License for more details.
> + *
> + * ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> + */
> +#include <linux/init.h>
> +#include <linux/delay.h>
> +#include <linux/module.h>
> +#include <sound/pcm_params.h>
> +#include <sound/soc.h>
> +#include <sound/hdaudio_ext.h>
> +#include <sound/hda_register.h>
> +#include "../../hda/local.h"
> +#include "../../pci/hda/hda_codec.h"
> +#include "hdac_hda.h"
> +
> +static int hdac_hda_dev_probe(struct hdac_device *hdev)
> +{
> +	struct hdac_ext_link *hlink = NULL;
> +	struct hdac_hda_priv *hda_pvt;
> +
> +	dev_dbg(&hdev->dev, "%s: entry\n", __func__);
> +
> +	/* hold the ref while we probe */
> +	hlink = snd_hdac_ext_bus_get_link(hdev->bus, dev_name(&hdev->dev));
> +	if (!hlink) {
> +		dev_err(&hdev->dev, "hdac link not found\n");
> +		return -EIO;
> +	}
> +	snd_hdac_ext_bus_link_get(hdev->bus, hlink);
> +
> +	hda_pvt = hdac_to_hda_priv(hdev);
> +	if (hda_pvt == NULL)
> +		return -ENOMEM;
> +
> +	dev_set_drvdata(&hdev->dev, hda_pvt);
> +	snd_hdac_ext_bus_link_put(hdev->bus, hlink);
> +
> +	return 0;
> +}
> +
> +static int hdac_hda_dev_remove(struct hdac_device *hdev)
> +{
> +	dev_dbg(&hdev->dev, "%s: entry\n", __func__);
> +	return 0;
> +}
> +
> +#define hdac_hda_runtime_suspend NULL
> +#define hdac_hda_runtime_resume NULL
> +
> +static const struct dev_pm_ops hdac_hda_pm = {
> +	SET_RUNTIME_PM_OPS(hdac_hda_runtime_suspend,
> +				hdac_hda_runtime_resume, NULL)
> +};
> +
> +int __hdac_hda_codec_driver_register(struct hda_codec_driver *drv,
> +	const char *name, struct module *owner)
> +{
> +	struct hdac_driver *hdac_hda_driver = &drv->core;
> +
> +	hdac_hda_driver->driver.name = name;
> +	hdac_hda_driver->driver.pm = &hdac_hda_pm;
> +
> +	hdac_hda_driver->id_table = drv->id;
> +	hdac_hda_driver->probe = hdac_hda_dev_probe;
> +	hdac_hda_driver->remove = hdac_hda_dev_remove;
> +
> +	return snd_hda_ext_driver_register(hdac_hda_driver);
> +}
> +EXPORT_SYMBOL_GPL(__hdac_hda_codec_driver_register);
> +
> +void hdac_hda_codec_driver_unregister(struct hda_codec_driver *drv)
> +{
> +	struct hdac_driver *hdac_hda_driver = &drv->core;
> +
> +	snd_hda_ext_driver_unregister(hdac_hda_driver);
> +}
> +EXPORT_SYMBOL_GPL(hdac_hda_codec_driver_unregister);
> +
> +MODULE_LICENSE("GPL v2");
> +MODULE_DESCRIPTION("ASoC HDA codec driver");
> +MODULE_AUTHOR("Rakesh Ughreja<rakesh.a.ughreja@intel.com>");
> diff --git a/sound/soc/codecs/hdac_hda.h b/sound/soc/codecs/hdac_hda.h
> new file mode 100644
> index 0000000..b59f106
> --- /dev/null
> +++ b/sound/soc/codecs/hdac_hda.h
> @@ -0,0 +1,16 @@
> +#ifndef __HDAC_HDA_H__
> +#define __HDAC_HDA_H__
> +
> +struct hdac_hda_priv {
> +	struct hda_codec codec;
> +	struct hda_bus *hbus;
> +	int stream_tag;
> +
> +	struct snd_soc_codec *scodec;
> +};
> +
> +#define hdac_to_hda_priv(_hdac) \
> +			container_of(_hdac, struct hdac_hda_priv, codec.core)
> +#define hdac_to_hda_codec(_hdac) container_of(_hdac, struct hda_codec, core)
> +
> +#endif /* __HDAC_HDA_H__ */
> diff --git a/sound/soc/intel/skylake/skl.c b/sound/soc/intel/skylake/skl.c
> index 72a788a..7f1971f 100644
> --- a/sound/soc/intel/skylake/skl.c
> +++ b/sound/soc/intel/skylake/skl.c
> @@ -36,6 +36,7 @@
>   #include "skl-sst-dsp.h"
>   #include "skl-sst-ipc.h"
>   #include "../../../pci/hda/hda_codec.h"
> +#include "../../../soc/codecs/hdac_hda.h"
>   
>   static struct skl_machine_pdata skl_dmic_data;
>   
> @@ -533,6 +534,7 @@ static int probe_codec(struct hdac_bus *bus, int addr)
>   		(AC_VERB_PARAMETERS << 8) | AC_PAR_VENDOR_ID;
>   	unsigned int res = -1;
>   	struct skl *skl = bus_to_skl(bus);
> +	struct hdac_hda_priv *hda_codec;
>   	struct hdac_device *hdev;
>   
>   	mutex_lock(&bus->cmd_mutex);
> @@ -543,10 +545,22 @@ static int probe_codec(struct hdac_bus *bus, int addr)
>   		return -EIO;
>   	dev_dbg(bus->dev, "codec #%d probed OK\n", addr);
>   
> -	hdev = devm_kzalloc(&skl->pci->dev, sizeof(*hdev), GFP_KERNEL);
> -	if (!hdev)
> +	/*
> +	 * FIXME:
> +	 * The legacy HDA controller driver allocates data structure
> +	 * for hda_codec. Ideally it should allocate only hdac_device
> +	 * so that it has no dependency on the data structure of the
> +	 * codec driver. If legacy controller driver can be changed this
> +	 * code change can be avoided.
> +	 */
> +	hda_codec = devm_kzalloc(&skl->pci->dev, sizeof(*hda_codec),
> +								GFP_KERNEL);
> +	if (!hda_codec)
>   		return -ENOMEM;
>   
> +	hda_codec->hbus = skl_to_hbus(skl);
> +	hdev = &hda_codec->codec.core;
> +
>   	return snd_hdac_ext_bus_device_init(bus, addr, hdev);
>   }
>   
> 

Re: [RFC 00/10] Enable HDA Codec support on Intel Platforms (Series2)

[Pierre-Louis Bossart]

On 12/1/17 8:56 AM, Takashi Iwai wrote:
> On Fri, 01 Dec 2017 10:13:58 +0100,
> Rakesh Ughreja wrote:
>>
>> Many Intel platforms (SKL,KBL) etc. in the market supports enahanced
>> audio capabilities which also includes DSP processing. This patch carry
>> forwads the works that is done in the previous series to enable HD Audio
>> codecs on such platforms.
>>
>> This patch series adds ASoC HDA codec driver for Intel platforms. It is
>> written by reusing the legacy HDA ALSA codec driver. Intention is to
>> maximize the reuse and minimize the changes in the legacy HDA codec driver.
>>
>> I would like to receive feedback before proceeding further on this
>> direction.
> 
> First of all, I appreciate this kind of work finally happening, so
> that ASoC HD-audio can go forward to a wider usage.  However...
> 
> 
>> TODO:
>>
>> - This series is tested on KBL based product (Dell XPS 13).
>> - Basic playback is working with headset and speakers.
>> - Capture operation is not tested.
>> - More platforms and use cases coverage can be added once we have basic
>>    agreement in terms of the overall approach.
> 
> One big thing that is unclear to me is what about the complex I/O
> configuration although we provide only a single machine driver, and
> what happens if the (legacy) codec driver allows the jack retasking.
> I know that nowadays the number of jacks are reduced than before, but
> still there will be desktop boxes with individual front and rear I/O
> jacks.
> 
> 
>> FIXME:
>> - KConfig changes does not look right, but I could not think of any proper
>>    way without making changes into legacy HDA codec driver. So need some
>>    help on this topic.
> 
> And this is another big issue.  As long as you don't allow build both,
> it never flies with distros, and it means no dramatic improvement of
> the situation, either.
> 
> IOW, we have to make the codec driver working on both controller/bus
> implementations, and it essentially means to rewrite the HD-audio
> codec plumbing code.  Currently, it's a kind of half-baked state, and
> we need to cast to unified form.

Before going into codec support, should we first deal with the selection 
of the ASoC/Legacy mode for the controller side of things? Even with 
basic HDMI/iDisp cases, we don't have a clean solution today. When I 
worked on the late Joule platform, there was no fallback mode which 
forced a recompilation depending on BIOS settings.
My understanding is that there is no support for multiple drivers 
registering for the same PCI ID, I believe we'll have to define a 
top-level wrapper which arbitrates internally based on BIOS settings and 
user/distro preferences which mode is selected.

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

Re: [RFC 00/10] Enable HDA Codec support on Intel Platforms (Series2)

[Takashi Iwai]

On Fri, 01 Dec 2017 20:45:54 +0100,
Pierre-Louis Bossart wrote:
> 
> On 12/1/17 8:56 AM, Takashi Iwai wrote:
> > On Fri, 01 Dec 2017 10:13:58 +0100,
> > Rakesh Ughreja wrote:
> >>
> >> Many Intel platforms (SKL,KBL) etc. in the market supports enahanced
> >> audio capabilities which also includes DSP processing. This patch carry
> >> forwads the works that is done in the previous series to enable HD Audio
> >> codecs on such platforms.
> >>
> >> This patch series adds ASoC HDA codec driver for Intel platforms. It is
> >> written by reusing the legacy HDA ALSA codec driver. Intention is to
> >> maximize the reuse and minimize the changes in the legacy HDA codec driver.
> >>
> >> I would like to receive feedback before proceeding further on this
> >> direction.
> >
> > First of all, I appreciate this kind of work finally happening, so
> > that ASoC HD-audio can go forward to a wider usage.  However...
> >
> >
> >> TODO:
> >>
> >> - This series is tested on KBL based product (Dell XPS 13).
> >> - Basic playback is working with headset and speakers.
> >> - Capture operation is not tested.
> >> - More platforms and use cases coverage can be added once we have basic
> >>    agreement in terms of the overall approach.
> >
> > One big thing that is unclear to me is what about the complex I/O
> > configuration although we provide only a single machine driver, and
> > what happens if the (legacy) codec driver allows the jack retasking.
> > I know that nowadays the number of jacks are reduced than before, but
> > still there will be desktop boxes with individual front and rear I/O
> > jacks.
> >
> >
> >> FIXME:
> >> - KConfig changes does not look right, but I could not think of any proper
> >>    way without making changes into legacy HDA codec driver. So need some
> >>    help on this topic.
> >
> > And this is another big issue.  As long as you don't allow build both,
> > it never flies with distros, and it means no dramatic improvement of
> > the situation, either.
> >
> > IOW, we have to make the codec driver working on both controller/bus
> > implementations, and it essentially means to rewrite the HD-audio
> > codec plumbing code.  Currently, it's a kind of half-baked state, and
> > we need to cast to unified form.
> 
> Before going into codec support, should we first deal with the
> selection of the ASoC/Legacy mode for the controller side of things?
> Even with basic HDMI/iDisp cases, we don't have a clean solution
> today. When I worked on the late Joule platform, there was no fallback
> mode which forced a recompilation depending on BIOS settings.
> My understanding is that there is no support for multiple drivers
> registering for the same PCI ID, I believe we'll have to define a
> top-level wrapper which arbitrates internally based on BIOS settings
> and user/distro preferences which mode is selected.

True, we need to sort out that.  But the selection of the card-level
driver can be relatively easily done in a manual way, either via
blacklisting or by common module options.  Currently AMDGPU and radeon
drivers are in such situation, for example.

A common single switch, or at best the automatic arbitration would be
preferred, of course, though.


Takashi

Re: [RFC 00/10] Enable HDA Codec support on Intel Platforms (Series2)

[Vinod Koul]

On Fri, Dec 01, 2017 at 09:03:25PM +0100, Takashi Iwai wrote:
> > >> FIXME:
> > >> - KConfig changes does not look right, but I could not think of any proper
> > >>    way without making changes into legacy HDA codec driver. So need some
> > >>    help on this topic.
> > >
> > > And this is another big issue.  As long as you don't allow build both,
> > > it never flies with distros, and it means no dramatic improvement of
> > > the situation, either.
> > >
> > > IOW, we have to make the codec driver working on both controller/bus
> > > implementations, and it essentially means to rewrite the HD-audio
> > > codec plumbing code.  Currently, it's a kind of half-baked state, and
> > > we need to cast to unified form.
> > 
> > Before going into codec support, should we first deal with the
> > selection of the ASoC/Legacy mode for the controller side of things?
> > Even with basic HDMI/iDisp cases, we don't have a clean solution
> > today. When I worked on the late Joule platform, there was no fallback
> > mode which forced a recompilation depending on BIOS settings.
> > My understanding is that there is no support for multiple drivers
> > registering for the same PCI ID, I believe we'll have to define a
> > top-level wrapper which arbitrates internally based on BIOS settings
> > and user/distro preferences which mode is selected.
> 
> True, we need to sort out that.  But the selection of the card-level
> driver can be relatively easily done in a manual way, either via
> blacklisting or by common module options.  Currently AMDGPU and radeon
> drivers are in such situation, for example.
> 
> A common single switch, or at best the automatic arbitration would be
> preferred, of course, though.

The switch is again short term as we should progress and move towards one
solution whilst deprecating the other, so that way this makes a simple
solution and helps in transition, change flag defaults and deprecate.

-- 
~Vinod

Re: [RFC 00/10] Enable HDA Codec support on Intel Platforms (Series2)

[Pierre-Louis Bossart]

On 12/3/17 11:20 AM, Vinod Koul wrote:
> On Fri, Dec 01, 2017 at 09:03:25PM +0100, Takashi Iwai wrote:
>>>>> FIXME:
>>>>> - KConfig changes does not look right, but I could not think of any proper
>>>>>     way without making changes into legacy HDA codec driver. So need some
>>>>>     help on this topic.
>>>>
>>>> And this is another big issue.  As long as you don't allow build both,
>>>> it never flies with distros, and it means no dramatic improvement of
>>>> the situation, either.
>>>>
>>>> IOW, we have to make the codec driver working on both controller/bus
>>>> implementations, and it essentially means to rewrite the HD-audio
>>>> codec plumbing code.  Currently, it's a kind of half-baked state, and
>>>> we need to cast to unified form.
>>>
>>> Before going into codec support, should we first deal with the
>>> selection of the ASoC/Legacy mode for the controller side of things?
>>> Even with basic HDMI/iDisp cases, we don't have a clean solution
>>> today. When I worked on the late Joule platform, there was no fallback
>>> mode which forced a recompilation depending on BIOS settings.
>>> My understanding is that there is no support for multiple drivers
>>> registering for the same PCI ID, I believe we'll have to define a
>>> top-level wrapper which arbitrates internally based on BIOS settings
>>> and user/distro preferences which mode is selected.
>>
>> True, we need to sort out that.  But the selection of the card-level
>> driver can be relatively easily done in a manual way, either via
>> blacklisting or by common module options.  Currently AMDGPU and radeon
>> drivers are in such situation, for example.
>>
>> A common single switch, or at best the automatic arbitration would be
>> preferred, of course, though.
> 
> The switch is again short term as we should progress and move towards one
> solution whilst deprecating the other, so that way this makes a simple
> solution and helps in transition, change flag defaults and deprecate.

Sorry, I don't get your point. Are you saying the legacy would be 
deprecated? that was never my understanding, Rakesh's patches only make 
it easier to use the DSP in combination with an HDaudio codec, but if 
the DSP is not present the legacy is the fallback, no?

Re: [RFC 00/10] Enable HDA Codec support on Intel Platforms (Series2)

[Vinod Koul]

On Sun, Dec 03, 2017 at 09:15:21PM -0600, Pierre-Louis Bossart wrote:
> On 12/3/17 11:20 AM, Vinod Koul wrote:
> >On Fri, Dec 01, 2017 at 09:03:25PM +0100, Takashi Iwai wrote:
> >>>>>FIXME:
> >>>>>- KConfig changes does not look right, but I could not think of any proper
> >>>>>    way without making changes into legacy HDA codec driver. So need some
> >>>>>    help on this topic.
> >>>>
> >>>>And this is another big issue.  As long as you don't allow build both,
> >>>>it never flies with distros, and it means no dramatic improvement of
> >>>>the situation, either.
> >>>>
> >>>>IOW, we have to make the codec driver working on both controller/bus
> >>>>implementations, and it essentially means to rewrite the HD-audio
> >>>>codec plumbing code.  Currently, it's a kind of half-baked state, and
> >>>>we need to cast to unified form.
> >>>
> >>>Before going into codec support, should we first deal with the
> >>>selection of the ASoC/Legacy mode for the controller side of things?
> >>>Even with basic HDMI/iDisp cases, we don't have a clean solution
> >>>today. When I worked on the late Joule platform, there was no fallback
> >>>mode which forced a recompilation depending on BIOS settings.
> >>>My understanding is that there is no support for multiple drivers
> >>>registering for the same PCI ID, I believe we'll have to define a
> >>>top-level wrapper which arbitrates internally based on BIOS settings
> >>>and user/distro preferences which mode is selected.
> >>
> >>True, we need to sort out that.  But the selection of the card-level
> >>driver can be relatively easily done in a manual way, either via
> >>blacklisting or by common module options.  Currently AMDGPU and radeon
> >>drivers are in such situation, for example.
> >>
> >>A common single switch, or at best the automatic arbitration would be
> >>preferred, of course, though.
> >
> >The switch is again short term as we should progress and move towards one
> >solution whilst deprecating the other, so that way this makes a simple
> >solution and helps in transition, change flag defaults and deprecate.
> 
> Sorry, I don't get your point. Are you saying the legacy would be
> deprecated? that was never my understanding, Rakesh's patches only make it
> easier to use the DSP in combination with an HDaudio codec, but if the DSP
> is not present the legacy is the fallback, no?

for now yes, but what prevents us from supporting coupled mode when DSP is
disabled :)

-- 
~Vinod

Re: [RFC 00/10] Enable HDA Codec support on Intel Platforms (Series2)

[Pierre-Louis Bossart]

On 12/3/17 9:22 PM, Vinod Koul wrote:
> On Sun, Dec 03, 2017 at 09:15:21PM -0600, Pierre-Louis Bossart wrote:
>> On 12/3/17 11:20 AM, Vinod Koul wrote:
>>> On Fri, Dec 01, 2017 at 09:03:25PM +0100, Takashi Iwai wrote:
>>>>>>> FIXME:
>>>>>>> - KConfig changes does not look right, but I could not think of any proper
>>>>>>>     way without making changes into legacy HDA codec driver. So need some
>>>>>>>     help on this topic.
>>>>>>
>>>>>> And this is another big issue.  As long as you don't allow build both,
>>>>>> it never flies with distros, and it means no dramatic improvement of
>>>>>> the situation, either.
>>>>>>
>>>>>> IOW, we have to make the codec driver working on both controller/bus
>>>>>> implementations, and it essentially means to rewrite the HD-audio
>>>>>> codec plumbing code.  Currently, it's a kind of half-baked state, and
>>>>>> we need to cast to unified form.
>>>>>
>>>>> Before going into codec support, should we first deal with the
>>>>> selection of the ASoC/Legacy mode for the controller side of things?
>>>>> Even with basic HDMI/iDisp cases, we don't have a clean solution
>>>>> today. When I worked on the late Joule platform, there was no fallback
>>>>> mode which forced a recompilation depending on BIOS settings.
>>>>> My understanding is that there is no support for multiple drivers
>>>>> registering for the same PCI ID, I believe we'll have to define a
>>>>> top-level wrapper which arbitrates internally based on BIOS settings
>>>>> and user/distro preferences which mode is selected.
>>>>
>>>> True, we need to sort out that.  But the selection of the card-level
>>>> driver can be relatively easily done in a manual way, either via
>>>> blacklisting or by common module options.  Currently AMDGPU and radeon
>>>> drivers are in such situation, for example.
>>>>
>>>> A common single switch, or at best the automatic arbitration would be
>>>> preferred, of course, though.
>>>
>>> The switch is again short term as we should progress and move towards one
>>> solution whilst deprecating the other, so that way this makes a simple
>>> solution and helps in transition, change flag defaults and deprecate.
>>
>> Sorry, I don't get your point. Are you saying the legacy would be
>> deprecated? that was never my understanding, Rakesh's patches only make it
>> easier to use the DSP in combination with an HDaudio codec, but if the DSP
>> is not present the legacy is the fallback, no?
> 
> for now yes, but what prevents us from supporting coupled mode when DSP is
> disabled :)

That gets complicated. you'd have
1. DSP-based ASoC driver for SKL+ w/ DSP
2. coupled-mode ASoC driver for SKL+ w/o DSP
3. legacy for pre-SKL platforms.

I don't see what simplification this brings, it's complicated enough 
with 1. and 3, and legacy is not going away.

Re: [RFC 00/10] Enable HDA Codec support on Intel Platforms (Series2)

[Vinod Koul]

On Sun, Dec 03, 2017 at 09:44:56PM -0600, Pierre-Louis Bossart wrote:
> On 12/3/17 9:22 PM, Vinod Koul wrote:
> >On Sun, Dec 03, 2017 at 09:15:21PM -0600, Pierre-Louis Bossart wrote:
> >>On 12/3/17 11:20 AM, Vinod Koul wrote:
> >>>On Fri, Dec 01, 2017 at 09:03:25PM +0100, Takashi Iwai wrote:
> >>>>>>>FIXME:
> >>>>>>>- KConfig changes does not look right, but I could not think of any proper
> >>>>>>>    way without making changes into legacy HDA codec driver. So need some
> >>>>>>>    help on this topic.
> >>>>>>
> >>>>>>And this is another big issue.  As long as you don't allow build both,
> >>>>>>it never flies with distros, and it means no dramatic improvement of
> >>>>>>the situation, either.
> >>>>>>
> >>>>>>IOW, we have to make the codec driver working on both controller/bus
> >>>>>>implementations, and it essentially means to rewrite the HD-audio
> >>>>>>codec plumbing code.  Currently, it's a kind of half-baked state, and
> >>>>>>we need to cast to unified form.
> >>>>>
> >>>>>Before going into codec support, should we first deal with the
> >>>>>selection of the ASoC/Legacy mode for the controller side of things?
> >>>>>Even with basic HDMI/iDisp cases, we don't have a clean solution
> >>>>>today. When I worked on the late Joule platform, there was no fallback
> >>>>>mode which forced a recompilation depending on BIOS settings.
> >>>>>My understanding is that there is no support for multiple drivers
> >>>>>registering for the same PCI ID, I believe we'll have to define a
> >>>>>top-level wrapper which arbitrates internally based on BIOS settings
> >>>>>and user/distro preferences which mode is selected.
> >>>>
> >>>>True, we need to sort out that.  But the selection of the card-level
> >>>>driver can be relatively easily done in a manual way, either via
> >>>>blacklisting or by common module options.  Currently AMDGPU and radeon
> >>>>drivers are in such situation, for example.
> >>>>
> >>>>A common single switch, or at best the automatic arbitration would be
> >>>>preferred, of course, though.
> >>>
> >>>The switch is again short term as we should progress and move towards one
> >>>solution whilst deprecating the other, so that way this makes a simple
> >>>solution and helps in transition, change flag defaults and deprecate.
> >>
> >>Sorry, I don't get your point. Are you saying the legacy would be
> >>deprecated? that was never my understanding, Rakesh's patches only make it
> >>easier to use the DSP in combination with an HDaudio codec, but if the DSP
> >>is not present the legacy is the fallback, no?
> >
> >for now yes, but what prevents us from supporting coupled mode when DSP is
> >disabled :)
> 
> That gets complicated. you'd have
> 1. DSP-based ASoC driver for SKL+ w/ DSP
> 2. coupled-mode ASoC driver for SKL+ w/o DSP
> 3. legacy for pre-SKL platforms.
> 
> I don't see what simplification this brings, it's complicated enough with 1.
> and 3, and legacy is not going away.

in long term we _need_ to converge. 1 and 2 should be same driver detecting
presence of DSP dynamically and doing DSP based or coupled mode.

3 would be separate drivers for different PCI IDs so no conflict.

Thanks
-- 
~Vinod

Re: [RFC 00/10] Enable HDA Codec support on Intel Platforms (Series2)

[Ughreja, Rakesh A]

>-----Original Message-----
>From: Takashi Iwai [mailto:tiwai@suse.de]
>Sent: Friday, December 1, 2017 8:26 PM
>To: Ughreja, Rakesh A <rakesh.a.ughreja@intel.com>
>Cc: alsa-devel@alsa-project.org; broonie@kernel.org;
>liam.r.girdwood@linux.intel.com; pierre-louis.bossart@linux.intel.com; Koul,
>Vinod <vinod.koul@intel.com>; Patches Audio <patches.audio@intel.com>
>Subject: Re: [RFC 00/10] Enable HDA Codec support on Intel Platforms
>(Series2)
>
>On Fri, 01 Dec 2017 10:13:58 +0100,
>Rakesh Ughreja wrote:
>>
>> Many Intel platforms (SKL,KBL) etc. in the market supports enahanced
>> audio capabilities which also includes DSP processing. This patch carry
>> forwads the works that is done in the previous series to enable HD Audio
>> codecs on such platforms.
>>
>> This patch series adds ASoC HDA codec driver for Intel platforms. It is
>> written by reusing the legacy HDA ALSA codec driver. Intention is to
>> maximize the reuse and minimize the changes in the legacy HDA codec
>driver.
>>
>> I would like to receive feedback before proceeding further on this
>> direction.
>
>First of all, I appreciate this kind of work finally happening, so
>that ASoC HD-audio can go forward to a wider usage.  However...

Thank you for encouragement and prompt feedback.

>
>
>> TODO:
>>
>> - This series is tested on KBL based product (Dell XPS 13).
>> - Basic playback is working with headset and speakers.
>> - Capture operation is not tested.
>> - More platforms and use cases coverage can be added once we have basic
>>   agreement in terms of the overall approach.
>
>One big thing that is unclear to me is what about the complex I/O
>configuration although we provide only a single machine driver, and
>what happens if the (legacy) codec driver allows the jack retasking.
>I know that nowadays the number of jacks are reduced than before, but
>still there will be desktop boxes with individual front and rear I/O
>jacks.

At this point of time, I was thinking that we should have a single machine
Driver with machine specific changes, possibly with quicks or DMIs 
or anything which works.

Single machine driver is for a case when we have one HDA codec 
and one iDisp (HDMI/DP) codec.
When we have other peripherals e.g. SoC DMICs or I2S codecs in addition
to HDA codec, we should have another machine driver.

>
>
>> FIXME:
>> - KConfig changes does not look right, but I could not think of any proper
>>   way without making changes into legacy HDA codec driver. So need some
>>   help on this topic.
>
>And this is another big issue.  As long as you don't allow build both,
>it never flies with distros, and it means no dramatic improvement of
>the situation, either.

Yes, I agree. At the moment I didn't want to do any changes in the
codec driver. Now I got your attention and so we will have a better
solution.

>
>IOW, we have to make the codec driver working on both controller/bus
>implementations, and it essentially means to rewrite the HD-audio
>codec plumbing code.  Currently, it's a kind of half-baked state, and
>we need to cast to unified form.

Yes, exactly, if we make some changes in the legacy codec driver
we can solve this problem. But I didn't want to do it without 
understanding your thinking process. That's why I just kept it as
open in the current series.

Regards,
Rakesh

Re: [RFC 01/10] ASoC: Intel: Boards: Machine driver for Intel platforms

[Ughreja, Rakesh A]

>-----Original Message-----
>From: Pierre-Louis Bossart [mailto:pierre-louis.bossart@linux.intel.com]
>Sent: Friday, December 1, 2017 11:28 PM
>To: Ughreja, Rakesh A <rakesh.a.ughreja@intel.com>; alsa-devel@alsa-
>project.org; broonie@kernel.org; tiwai@suse.de;
>liam.r.girdwood@linux.intel.com
>Cc: Koul, Vinod <vinod.koul@intel.com>; Patches Audio
><patches.audio@intel.com>
>Subject: Re: [alsa-devel] [RFC 01/10] ASoC: Intel: Boards: Machine driver for
>Intel platforms
>
>On 12/1/17 3:13 AM, Rakesh Ughreja wrote:
>> Add machine driver for Intel platforms(SKL/KBL) with HDA and iDisp codecs.
>> This patch adds support for only iDisp (HDMI/DP) codec. In the
>> following patch support for HDA codec will be added.
>
>But to be clear this should be sufficient for headless devices with no
>audio codec (Joule or UP^2) where the only audio output is HDMI/iDisp?

Yes, that’s right. Do you think it is better to have a separate machine
driver for headless devices where we don't have any HDA codecs ?

We can load that machine driver when we find only the iDisp codec.

>
>>
>> This should work for other Intel platforms as well e.g. BXT,GLK,CNL
>> however this series is not tested on all the platforms.
>>
>> Signed-off-by: Rakesh Ughreja <rakesh.a.ughreja@intel.com>
>> ---
>>   sound/soc/intel/boards/Kconfig           |  10 ++
>>   sound/soc/intel/boards/Makefile          |   2 +
>>   sound/soc/intel/boards/skl_hda_generic.c | 276
>+++++++++++++++++++++++++++++++
>
>can we drop the Skylake reference? It's become a catch-all term to mean
>both the platform, the IP and the driver.

Suggest some name. I have no problem.

>
>>   3 files changed, 288 insertions(+)
>>   create mode 100644 sound/soc/intel/boards/skl_hda_generic.c
>>
>> diff --git a/sound/soc/intel/boards/Kconfig
>b/sound/soc/intel/boards/Kconfig
>> index 6f75470..4f8bd02 100644
>> --- a/sound/soc/intel/boards/Kconfig
>> +++ b/sound/soc/intel/boards/Kconfig
>> @@ -262,4 +262,14 @@ config
>SND_SOC_INTEL_KBL_RT5663_RT5514_MAX98927_MACH
>>             Say Y if you have such a device.
>>             If unsure select "N".
>>
>> +config SND_SOC_INTEL_SKL_HDA_GENERIC_MACH
>> +        tristate "ASoC Audio driver for SKL/KBL with HDA Codecs"
>> +        select SND_SOC_INTEL_SST
>> +        depends on SND_SOC_INTEL_SKYLAKE
>
>that's also going to have to be reworked to allow for an SOF-based
>platform driver initializing this machine driver.

Is this what you are fixing with your latest KConfig patches ?

>
>> +        select SND_SOC_HDAC_HDMI
>> +        help
>> +          This adds support for ASoC Onboard Codec HDA machine driver.
>This will
>> +          create an alsa sound card for HDA Codecs.
>> +          Say Y if you have such a device.
>> +          If unsure select "N".
>>   endif
>> diff --git a/sound/soc/intel/boards/Makefile
>b/sound/soc/intel/boards/Makefile
>> index 69d2dfa..4686727 100644
>> --- a/sound/soc/intel/boards/Makefile
>> +++ b/sound/soc/intel/boards/Makefile
>> @@ -17,6 +17,7 @@ snd-soc-sst-byt-cht-nocodec-objs :=
>bytcht_nocodec.o
>>   snd-soc-kbl_rt5663_max98927-objs := kbl_rt5663_max98927.o
>>   snd-soc-kbl_rt5663_rt5514_max98927-objs :=
>kbl_rt5663_rt5514_max98927.o
>>   snd-soc-skl_rt286-objs := skl_rt286.o
>> +snd-soc-skl_hda_generic-objs := skl_hda_generic.o
>>   snd-skl_nau88l25_max98357a-objs := skl_nau88l25_max98357a.o
>>   snd-soc-skl_nau88l25_ssm4567-objs := skl_nau88l25_ssm4567.o
>>
>> @@ -40,3 +41,4 @@ obj-
>$(CONFIG_SND_SOC_INTEL_KBL_RT5663_RT5514_MAX98927_MACH) +=
>snd-soc-kbl_rt566
>>   obj-$(CONFIG_SND_SOC_INTEL_SKL_RT286_MACH) += snd-soc-
>skl_rt286.o
>>   obj-$(CONFIG_SND_SOC_INTEL_SKL_NAU88L25_MAX98357A_MACH) +=
>snd-skl_nau88l25_max98357a.o
>>   obj-$(CONFIG_SND_SOC_INTEL_SKL_NAU88L25_SSM4567_MACH) +=
>snd-soc-skl_nau88l25_ssm4567.o
>> +obj-$(CONFIG_SND_SOC_INTEL_SKL_HDA_GENERIC_MACH) += snd-soc-
>skl_hda_generic.o
>> diff --git a/sound/soc/intel/boards/skl_hda_generic.c
>b/sound/soc/intel/boards/skl_hda_generic.c
>> new file mode 100644
>> index 0000000..ece39b5
>> --- /dev/null
>> +++ b/sound/soc/intel/boards/skl_hda_generic.c
>> @@ -0,0 +1,276 @@
>> +/*
>> + * Intel Machine Driver for SKL/KBL platforms with HDA Codecs
>> + *
>> + * Copyright (C) 2017, Intel Corporation. All rights reserved.
>> + *
>> + *
>> + * This program is free software; you can redistribute it and/or
>> + * modify it under the terms of the GNU General Public License version
>> + * 2 as published by the Free Software Foundation.
>> + *
>> + * This program is distributed in the hope that it will be useful,
>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
>> + * GNU General Public License for more details.
>> + */
>> +
>> +#include <linux/module.h>
>> +#include <linux/platform_device.h>
>> +#include <sound/core.h>
>> +#include <sound/jack.h>
>> +#include <sound/pcm.h>
>> +#include <sound/pcm_params.h>
>> +#include <sound/soc.h>
>> +#include "../../codecs/hdac_hdmi.h"
>> +#include "../skylake/skl.h"
>> +
>> +static struct snd_soc_card skl_hda_audio_card;
>> +static struct snd_soc_jack skl_hda_hdmi_jack[3];
>
>The code from here to ...
>> +
>> +struct skl_hda_hdmi_pcm {
>> +	struct list_head head;
>> +	struct snd_soc_dai *codec_dai;
>> +	int device;
>> +};
>> +
>> +struct skl_hda_private {
>> +	struct list_head hdmi_pcm_list;
>> +};
>> +
>> +enum {
>> +	SKL_HDA_DPCM_AUDIO_HDMI1_PB = 0,
>> +	SKL_HDA_DPCM_AUDIO_HDMI2_PB,
>> +	SKL_HDA_DPCM_AUDIO_HDMI3_PB,
>> +};
>> +
>> +static const struct snd_soc_dapm_route skl_hda_map[] = {
>> +
>> +	{ "hifi3", NULL, "iDisp3 Tx"},
>> +	{ "iDisp3 Tx", NULL, "iDisp3_out"},
>> +	{ "hifi2", NULL, "iDisp2 Tx"},
>> +	{ "iDisp2 Tx", NULL, "iDisp2_out"},
>> +	{ "hifi1", NULL, "iDisp1 Tx"},
>> +	{ "iDisp1 Tx", NULL, "iDisp1_out"},
>> +
>> +};
>> +
>> +static int skl_hda_hdmi1_init(struct snd_soc_pcm_runtime *rtd)
>> +{
>> +	struct skl_hda_private *ctx = snd_soc_card_get_drvdata(rtd->card);
>> +	struct snd_soc_dai *dai = rtd->codec_dai;
>> +	struct skl_hda_hdmi_pcm *pcm;
>> +
>> +	pcm = devm_kzalloc(rtd->card->dev, sizeof(*pcm), GFP_KERNEL);
>> +	if (!pcm)
>> +		return -ENOMEM;
>> +
>> +	pcm->device = SKL_HDA_DPCM_AUDIO_HDMI1_PB;
>> +	pcm->codec_dai = dai;
>> +
>> +	list_add_tail(&pcm->head, &ctx->hdmi_pcm_list);
>> +
>> +	return 0;
>> +}
>> +
>> +static int skl_hda_hdmi2_init(struct snd_soc_pcm_runtime *rtd)
>> +{
>> +	struct skl_hda_private *ctx = snd_soc_card_get_drvdata(rtd->card);
>> +	struct snd_soc_dai *dai = rtd->codec_dai;
>> +	struct skl_hda_hdmi_pcm *pcm;
>> +
>> +	pcm = devm_kzalloc(rtd->card->dev, sizeof(*pcm), GFP_KERNEL);
>> +	if (!pcm)
>> +		return -ENOMEM;
>> +
>> +	pcm->device = SKL_HDA_DPCM_AUDIO_HDMI2_PB;
>> +	pcm->codec_dai = dai;
>> +
>> +	list_add_tail(&pcm->head, &ctx->hdmi_pcm_list);
>> +
>> +	return 0;
>> +}
>> +
>> +static int skl_hda_hdmi3_init(struct snd_soc_pcm_runtime *rtd)
>> +{
>> +	struct skl_hda_private *ctx = snd_soc_card_get_drvdata(rtd->card);
>> +	struct snd_soc_dai *dai = rtd->codec_dai;
>> +	struct skl_hda_hdmi_pcm *pcm;
>> +
>> +	pcm = devm_kzalloc(rtd->card->dev, sizeof(*pcm), GFP_KERNEL);
>> +	if (!pcm)
>> +		return -ENOMEM;
>> +
>> +	pcm->device = SKL_HDA_DPCM_AUDIO_HDMI3_PB;
>> +	pcm->codec_dai = dai;
>> +
>> +	list_add_tail(&pcm->head, &ctx->hdmi_pcm_list);
>> +
>> +	return 0;
>> +}
>> +
>
>.. here is pretty much copy/pasted across machine drivers, can we move
>it to a common include file? The experience from Baytrail/Cherrytrail is
>that the more we wait the more complicated it is to factor the code.

That’s a good point. I can do that in my next series. But this would be
specific to SKL onwards platforms. I am not sure if it would work for 
BYT/CHT Etc.

>
>> +/* skl_hda_digital audio interface glue - connects codec <--> CPU */
>> +static struct snd_soc_dai_link skl_hda_dais[] = {
>> +	/* Front End DAI links */
>> +	[SKL_HDA_DPCM_AUDIO_HDMI1_PB] = {
>
>Are those hard-coded links required for all platforms, I thought the
>newer ones were based on topology?

Yes, I agree. In the latest patches from Guneshwor, the DAI Links
are coming from Topology. I can fix it in the next revision.


>
>> +		.name = "SKL HDA HDMI Port1",
>> +		.stream_name = "Hdmi1",
>> +		.cpu_dai_name = "HDMI1 Pin",
>> +		.codec_name = "snd-soc-dummy",
>> +		.codec_dai_name = "snd-soc-dummy-dai",
>> +		.platform_name = "0000:00:1f.3",
>> +		.dpcm_playback = 1,
>> +		.init = NULL,
>> +		.trigger = {
>> +			SND_SOC_DPCM_TRIGGER_POST,
>SND_SOC_DPCM_TRIGGER_POST},
>> +		.nonatomic = 1,
>> +		.dynamic = 1,
>> +	},
>> +	[SKL_HDA_DPCM_AUDIO_HDMI2_PB] = {
>> +		.name = "SKL HDA HDMI Port2",
>> +		.stream_name = "Hdmi2",
>> +		.cpu_dai_name = "HDMI2 Pin",
>> +		.codec_name = "snd-soc-dummy",
>> +		.codec_dai_name = "snd-soc-dummy-dai",
>> +		.platform_name = "0000:00:1f.3",
>> +		.dpcm_playback = 1,
>> +		.init = NULL,
>> +		.trigger = {
>> +			SND_SOC_DPCM_TRIGGER_POST,
>SND_SOC_DPCM_TRIGGER_POST},
>> +		.nonatomic = 1,
>> +		.dynamic = 1,
>> +	},
>> +	[SKL_HDA_DPCM_AUDIO_HDMI3_PB] = {
>> +		.name = "SKL HDA HDMI Port3",
>> +		.stream_name = "Hdmi3",
>> +		.cpu_dai_name = "HDMI3 Pin",
>> +		.codec_name = "snd-soc-dummy",
>> +		.codec_dai_name = "snd-soc-dummy-dai",
>> +		.platform_name = "0000:00:1f.3",
>> +		.trigger = {
>> +			SND_SOC_DPCM_TRIGGER_POST,
>SND_SOC_DPCM_TRIGGER_POST},
>> +		.dpcm_playback = 1,
>> +		.init = NULL,
>> +		.nonatomic = 1,
>> +		.dynamic = 1,
>> +	},
>> +
>> +	/* Back End DAI links */
>> +	{
>> +		.name = "iDisp1",
>> +		.id = 1,
>> +		.cpu_dai_name = "iDisp1 Pin",
>> +		.codec_name = "ehdaudio0D2",
>> +		.codec_dai_name = "intel-hdmi-hifi1",
>> +		.platform_name = "0000:00:1f.3",
>> +		.dpcm_playback = 1,
>> +		.init = skl_hda_hdmi1_init,
>> +		.no_pcm = 1,
>> +	},
>> +	{
>> +		.name = "iDisp2",
>> +		.id = 2,
>> +		.cpu_dai_name = "iDisp2 Pin",
>> +		.codec_name = "ehdaudio0D2",
>> +		.codec_dai_name = "intel-hdmi-hifi2",
>> +		.platform_name = "0000:00:1f.3",
>> +		.init = skl_hda_hdmi2_init,
>> +		.dpcm_playback = 1,
>> +		.no_pcm = 1,
>> +	},
>> +	{
>> +		.name = "iDisp3",
>> +		.id = 3,
>> +		.cpu_dai_name = "iDisp3 Pin",
>> +		.codec_name = "ehdaudio0D2",
>> +		.codec_dai_name = "intel-hdmi-hifi3",
>> +		.platform_name = "0000:00:1f.3",
>> +		.init = skl_hda_hdmi3_init,
>> +		.dpcm_playback = 1,
>> +		.no_pcm = 1,
>> +	},
>> +};
>> +
>> +#define NAME_SIZE	32
>> +static int skl_hda_card_late_probe(struct snd_soc_card *card)
>> +{
>> +	struct skl_hda_private *ctx = snd_soc_card_get_drvdata(card);
>> +	struct skl_hda_hdmi_pcm *pcm;
>> +	struct snd_soc_codec *codec = NULL;
>> +	int err, i = 0;
>> +	char jack_name[NAME_SIZE];
>> +
>> +	list_for_each_entry(pcm, &ctx->hdmi_pcm_list, head) {
>> +		codec = pcm->codec_dai->codec;
>> +		snprintf(jack_name, sizeof(jack_name),
>> +			"HDMI/DP, pcm=%d Jack", pcm->device);
>> +		err = snd_soc_card_jack_new(card, jack_name,
>> +					SND_JACK_AVOUT,
>&skl_hda_hdmi_jack[i],
>> +					NULL, 0);
>> +
>> +		if (err)
>> +			return err;
>> +
>> +		err = hdac_hdmi_jack_init(pcm->codec_dai, pcm->device,
>> +						&skl_hda_hdmi_jack[i]);
>> +		if (err < 0)
>> +			return err;
>> +
>> +		i++;
>> +	}
>> +
>> +	if (!codec)
>> +		return -EINVAL;
>> +
>> +	return hdac_hdmi_jack_port_init(codec, &card->dapm);
>> +}
>
>this one in always the same as well, it could be factored across machine
>drivers.

Okay, I will put this in the common file along with other functions that
you mentioned previously.

>
>> +
>> +static struct snd_soc_card skl_hda_audio_card = {
>> +	.name = "skl_hda_card",
>> +	.owner = THIS_MODULE,
>> +	.dai_link = skl_hda_dais,
>> +	.num_links = ARRAY_SIZE(skl_hda_dais),
>> +	.dapm_routes = skl_hda_map,
>> +	.num_dapm_routes = ARRAY_SIZE(skl_hda_map),
>> +	.fully_routed = true,
>> +	.late_probe = skl_hda_card_late_probe,
>> +};
>> +
>> +static int skl_hda_audio_probe(struct platform_device *pdev)
>> +{
>> +	struct skl_hda_private *ctx;
>> +
>> +	dev_dbg(&pdev->dev, "%s: machine driver probe got called\n",
>__func__);
>> +
>> +	ctx = devm_kzalloc(&pdev->dev, sizeof(*ctx), GFP_ATOMIC);
>> +	if (!ctx)
>> +		return -ENOMEM;
>> +
>> +	INIT_LIST_HEAD(&ctx->hdmi_pcm_list);
>> +
>> +	skl_hda_audio_card.dev = &pdev->dev;
>> +	snd_soc_card_set_drvdata(&skl_hda_audio_card, ctx);
>> +
>> +	return devm_snd_soc_register_card(&pdev->dev,
>&skl_hda_audio_card);
>> +}
>> +
>> +static const struct platform_device_id skl_hda_board_ids[] = {
>> +	{ .name = "skl_hda_generic" },
>> +	{ }
>> +};
>> +
>> +static struct platform_driver skl_hda_audio = {
>> +	.probe = skl_hda_audio_probe,
>> +	.driver = {
>> +		.name = "skl_hda_generic",
>> +		.pm = &snd_soc_pm_ops,
>> +	},
>> +	.id_table = skl_hda_board_ids,
>> +};
>> +
>> +module_platform_driver(skl_hda_audio)
>> +
>> +/* Module information */
>> +MODULE_DESCRIPTION("SKL/KBL HDA Generic Machine driver");
>
>again it's not limited to SKL/KBL, it'll work on APL and CNL and GLK ...

Yes, technically there is nothing limiting this to work on other platforms.
It is just that right now I have only SKL/KBL in my test coverage for
the current RFC series. I am hoping to get some initial feedback
after which I will do send this patches for some formal preint for other
platforms also before sending it. 

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

Re: [RFC 01/10] ASoC: Intel: Boards: Machine driver for Intel platforms

[Pierre-Louis Bossart]

On 12/4/17 4:55 AM, Ughreja, Rakesh A wrote:
> 
> 
>> -----Original Message-----
>> From: Pierre-Louis Bossart [mailto:pierre-louis.bossart@linux.intel.com]
>> Sent: Friday, December 1, 2017 11:28 PM
>> To: Ughreja, Rakesh A <rakesh.a.ughreja@intel.com>; alsa-devel@alsa-
>> project.org; broonie@kernel.org; tiwai@suse.de;
>> liam.r.girdwood@linux.intel.com
>> Cc: Koul, Vinod <vinod.koul@intel.com>; Patches Audio
>> <patches.audio@intel.com>
>> Subject: Re: [alsa-devel] [RFC 01/10] ASoC: Intel: Boards: Machine driver for
>> Intel platforms
>>
>> On 12/1/17 3:13 AM, Rakesh Ughreja wrote:
>>> Add machine driver for Intel platforms(SKL/KBL) with HDA and iDisp codecs.
>>> This patch adds support for only iDisp (HDMI/DP) codec. In the
>>> following patch support for HDA codec will be added.
>>
>> But to be clear this should be sufficient for headless devices with no
>> audio codec (Joule or UP^2) where the only audio output is HDMI/iDisp?
> 
> Yes, that’s right. Do you think it is better to have a separate machine
> driver for headless devices where we don't have any HDA codecs ?
> 
> We can load that machine driver when we find only the iDisp codec.

not necessarily, it depends how you detect the HDaudio codec.
> 
>>
>>>
>>> This should work for other Intel platforms as well e.g. BXT,GLK,CNL
>>> however this series is not tested on all the platforms.
>>>
>>> Signed-off-by: Rakesh Ughreja <rakesh.a.ughreja@intel.com>
>>> ---
>>>    sound/soc/intel/boards/Kconfig           |  10 ++
>>>    sound/soc/intel/boards/Makefile          |   2 +
>>>    sound/soc/intel/boards/skl_hda_generic.c | 276
>> +++++++++++++++++++++++++++++++
>>
>> can we drop the Skylake reference? It's become a catch-all term to mean
>> both the platform, the IP and the driver.
> 
> Suggest some name. I have no problem.

HiFi3 ?
iDisp ?
HDAudio-DSP ?

> 
>>
>>>    3 files changed, 288 insertions(+)
>>>    create mode 100644 sound/soc/intel/boards/skl_hda_generic.c
>>>
>>> diff --git a/sound/soc/intel/boards/Kconfig
>> b/sound/soc/intel/boards/Kconfig
>>> index 6f75470..4f8bd02 100644
>>> --- a/sound/soc/intel/boards/Kconfig
>>> +++ b/sound/soc/intel/boards/Kconfig
>>> @@ -262,4 +262,14 @@ config
>> SND_SOC_INTEL_KBL_RT5663_RT5514_MAX98927_MACH
>>>              Say Y if you have such a device.
>>>              If unsure select "N".
>>>
>>> +config SND_SOC_INTEL_SKL_HDA_GENERIC_MACH
>>> +        tristate "ASoC Audio driver for SKL/KBL with HDA Codecs"
>>> +        select SND_SOC_INTEL_SST
>>> +        depends on SND_SOC_INTEL_SKYLAKE
>>
>> that's also going to have to be reworked to allow for an SOF-based
>> platform driver initializing this machine driver.
> 
> Is this what you are fixing with your latest KConfig patches ?

yes

> 
>>
>>> +        select SND_SOC_HDAC_HDMI
>>> +        help
>>> +          This adds support for ASoC Onboard Codec HDA machine driver.
>> This will
>>> +          create an alsa sound card for HDA Codecs.
>>> +          Say Y if you have such a device.
>>> +          If unsure select "N".
>>>    endif
>>> diff --git a/sound/soc/intel/boards/Makefile
>> b/sound/soc/intel/boards/Makefile
>>> index 69d2dfa..4686727 100644
>>> --- a/sound/soc/intel/boards/Makefile
>>> +++ b/sound/soc/intel/boards/Makefile
>>> @@ -17,6 +17,7 @@ snd-soc-sst-byt-cht-nocodec-objs :=
>> bytcht_nocodec.o
>>>    snd-soc-kbl_rt5663_max98927-objs := kbl_rt5663_max98927.o
>>>    snd-soc-kbl_rt5663_rt5514_max98927-objs :=
>> kbl_rt5663_rt5514_max98927.o
>>>    snd-soc-skl_rt286-objs := skl_rt286.o
>>> +snd-soc-skl_hda_generic-objs := skl_hda_generic.o
>>>    snd-skl_nau88l25_max98357a-objs := skl_nau88l25_max98357a.o
>>>    snd-soc-skl_nau88l25_ssm4567-objs := skl_nau88l25_ssm4567.o
>>>
>>> @@ -40,3 +41,4 @@ obj-
>> $(CONFIG_SND_SOC_INTEL_KBL_RT5663_RT5514_MAX98927_MACH) +=
>> snd-soc-kbl_rt566
>>>    obj-$(CONFIG_SND_SOC_INTEL_SKL_RT286_MACH) += snd-soc-
>> skl_rt286.o
>>>    obj-$(CONFIG_SND_SOC_INTEL_SKL_NAU88L25_MAX98357A_MACH) +=
>> snd-skl_nau88l25_max98357a.o
>>>    obj-$(CONFIG_SND_SOC_INTEL_SKL_NAU88L25_SSM4567_MACH) +=
>> snd-soc-skl_nau88l25_ssm4567.o
>>> +obj-$(CONFIG_SND_SOC_INTEL_SKL_HDA_GENERIC_MACH) += snd-soc-
>> skl_hda_generic.o
>>> diff --git a/sound/soc/intel/boards/skl_hda_generic.c
>> b/sound/soc/intel/boards/skl_hda_generic.c
>>> new file mode 100644
>>> index 0000000..ece39b5
>>> --- /dev/null
>>> +++ b/sound/soc/intel/boards/skl_hda_generic.c
>>> @@ -0,0 +1,276 @@
>>> +/*
>>> + * Intel Machine Driver for SKL/KBL platforms with HDA Codecs
>>> + *
>>> + * Copyright (C) 2017, Intel Corporation. All rights reserved.
>>> + *
>>> + *
>>> + * This program is free software; you can redistribute it and/or
>>> + * modify it under the terms of the GNU General Public License version
>>> + * 2 as published by the Free Software Foundation.
>>> + *
>>> + * This program is distributed in the hope that it will be useful,
>>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
>>> + * GNU General Public License for more details.
>>> + */
>>> +
>>> +#include <linux/module.h>
>>> +#include <linux/platform_device.h>
>>> +#include <sound/core.h>
>>> +#include <sound/jack.h>
>>> +#include <sound/pcm.h>
>>> +#include <sound/pcm_params.h>
>>> +#include <sound/soc.h>
>>> +#include "../../codecs/hdac_hdmi.h"
>>> +#include "../skylake/skl.h"
>>> +
>>> +static struct snd_soc_card skl_hda_audio_card;
>>> +static struct snd_soc_jack skl_hda_hdmi_jack[3];
>>
>> The code from here to ...
>>> +
>>> +struct skl_hda_hdmi_pcm {
>>> +	struct list_head head;
>>> +	struct snd_soc_dai *codec_dai;
>>> +	int device;
>>> +};
>>> +
>>> +struct skl_hda_private {
>>> +	struct list_head hdmi_pcm_list;
>>> +};
>>> +
>>> +enum {
>>> +	SKL_HDA_DPCM_AUDIO_HDMI1_PB = 0,
>>> +	SKL_HDA_DPCM_AUDIO_HDMI2_PB,
>>> +	SKL_HDA_DPCM_AUDIO_HDMI3_PB,
>>> +};
>>> +
>>> +static const struct snd_soc_dapm_route skl_hda_map[] = {
>>> +
>>> +	{ "hifi3", NULL, "iDisp3 Tx"},
>>> +	{ "iDisp3 Tx", NULL, "iDisp3_out"},
>>> +	{ "hifi2", NULL, "iDisp2 Tx"},
>>> +	{ "iDisp2 Tx", NULL, "iDisp2_out"},
>>> +	{ "hifi1", NULL, "iDisp1 Tx"},
>>> +	{ "iDisp1 Tx", NULL, "iDisp1_out"},
>>> +
>>> +};
>>> +
>>> +static int skl_hda_hdmi1_init(struct snd_soc_pcm_runtime *rtd)
>>> +{
>>> +	struct skl_hda_private *ctx = snd_soc_card_get_drvdata(rtd->card);
>>> +	struct snd_soc_dai *dai = rtd->codec_dai;
>>> +	struct skl_hda_hdmi_pcm *pcm;
>>> +
>>> +	pcm = devm_kzalloc(rtd->card->dev, sizeof(*pcm), GFP_KERNEL);
>>> +	if (!pcm)
>>> +		return -ENOMEM;
>>> +
>>> +	pcm->device = SKL_HDA_DPCM_AUDIO_HDMI1_PB;
>>> +	pcm->codec_dai = dai;
>>> +
>>> +	list_add_tail(&pcm->head, &ctx->hdmi_pcm_list);
>>> +
>>> +	return 0;
>>> +}
>>> +
>>> +static int skl_hda_hdmi2_init(struct snd_soc_pcm_runtime *rtd)
>>> +{
>>> +	struct skl_hda_private *ctx = snd_soc_card_get_drvdata(rtd->card);
>>> +	struct snd_soc_dai *dai = rtd->codec_dai;
>>> +	struct skl_hda_hdmi_pcm *pcm;
>>> +
>>> +	pcm = devm_kzalloc(rtd->card->dev, sizeof(*pcm), GFP_KERNEL);
>>> +	if (!pcm)
>>> +		return -ENOMEM;
>>> +
>>> +	pcm->device = SKL_HDA_DPCM_AUDIO_HDMI2_PB;
>>> +	pcm->codec_dai = dai;
>>> +
>>> +	list_add_tail(&pcm->head, &ctx->hdmi_pcm_list);
>>> +
>>> +	return 0;
>>> +}
>>> +
>>> +static int skl_hda_hdmi3_init(struct snd_soc_pcm_runtime *rtd)
>>> +{
>>> +	struct skl_hda_private *ctx = snd_soc_card_get_drvdata(rtd->card);
>>> +	struct snd_soc_dai *dai = rtd->codec_dai;
>>> +	struct skl_hda_hdmi_pcm *pcm;
>>> +
>>> +	pcm = devm_kzalloc(rtd->card->dev, sizeof(*pcm), GFP_KERNEL);
>>> +	if (!pcm)
>>> +		return -ENOMEM;
>>> +
>>> +	pcm->device = SKL_HDA_DPCM_AUDIO_HDMI3_PB;
>>> +	pcm->codec_dai = dai;
>>> +
>>> +	list_add_tail(&pcm->head, &ctx->hdmi_pcm_list);
>>> +
>>> +	return 0;
>>> +}
>>> +
>>
>> .. here is pretty much copy/pasted across machine drivers, can we move
>> it to a common include file? The experience from Baytrail/Cherrytrail is
>> that the more we wait the more complicated it is to factor the code.
> 
> That’s a good point. I can do that in my next series. But this would be
> specific to SKL onwards platforms. I am not sure if it would work for
> BYT/CHT Etc.

this code would not appl for BYT/CHT since the path doesn't go through 
the DSP, so no ASoC and machine driver?

> 
>>
>>> +/* skl_hda_digital audio interface glue - connects codec <--> CPU */
>>> +static struct snd_soc_dai_link skl_hda_dais[] = {
>>> +	/* Front End DAI links */
>>> +	[SKL_HDA_DPCM_AUDIO_HDMI1_PB] = {
>>
>> Are those hard-coded links required for all platforms, I thought the
>> newer ones were based on topology?
> 
> Yes, I agree. In the latest patches from Guneshwor, the DAI Links
> are coming from Topology. I can fix it in the next revision

I was wondering if this change to topology is across the board.

> 
> 
>>
>>> +		.name = "SKL HDA HDMI Port1",
>>> +		.stream_name = "Hdmi1",
>>> +		.cpu_dai_name = "HDMI1 Pin",
>>> +		.codec_name = "snd-soc-dummy",
>>> +		.codec_dai_name = "snd-soc-dummy-dai",
>>> +		.platform_name = "0000:00:1f.3",
>>> +		.dpcm_playback = 1,
>>> +		.init = NULL,
>>> +		.trigger = {
>>> +			SND_SOC_DPCM_TRIGGER_POST,
>> SND_SOC_DPCM_TRIGGER_POST},
>>> +		.nonatomic = 1,
>>> +		.dynamic = 1,
>>> +	},
>>> +	[SKL_HDA_DPCM_AUDIO_HDMI2_PB] = {
>>> +		.name = "SKL HDA HDMI Port2",
>>> +		.stream_name = "Hdmi2",
>>> +		.cpu_dai_name = "HDMI2 Pin",
>>> +		.codec_name = "snd-soc-dummy",
>>> +		.codec_dai_name = "snd-soc-dummy-dai",
>>> +		.platform_name = "0000:00:1f.3",
>>> +		.dpcm_playback = 1,
>>> +		.init = NULL,
>>> +		.trigger = {
>>> +			SND_SOC_DPCM_TRIGGER_POST,
>> SND_SOC_DPCM_TRIGGER_POST},
>>> +		.nonatomic = 1,
>>> +		.dynamic = 1,
>>> +	},
>>> +	[SKL_HDA_DPCM_AUDIO_HDMI3_PB] = {
>>> +		.name = "SKL HDA HDMI Port3",
>>> +		.stream_name = "Hdmi3",
>>> +		.cpu_dai_name = "HDMI3 Pin",
>>> +		.codec_name = "snd-soc-dummy",
>>> +		.codec_dai_name = "snd-soc-dummy-dai",
>>> +		.platform_name = "0000:00:1f.3",
>>> +		.trigger = {
>>> +			SND_SOC_DPCM_TRIGGER_POST,
>> SND_SOC_DPCM_TRIGGER_POST},
>>> +		.dpcm_playback = 1,
>>> +		.init = NULL,
>>> +		.nonatomic = 1,
>>> +		.dynamic = 1,
>>> +	},
>>> +
>>> +	/* Back End DAI links */
>>> +	{
>>> +		.name = "iDisp1",
>>> +		.id = 1,
>>> +		.cpu_dai_name = "iDisp1 Pin",
>>> +		.codec_name = "ehdaudio0D2",
>>> +		.codec_dai_name = "intel-hdmi-hifi1",
>>> +		.platform_name = "0000:00:1f.3",
>>> +		.dpcm_playback = 1,
>>> +		.init = skl_hda_hdmi1_init,
>>> +		.no_pcm = 1,
>>> +	},
>>> +	{
>>> +		.name = "iDisp2",
>>> +		.id = 2,
>>> +		.cpu_dai_name = "iDisp2 Pin",
>>> +		.codec_name = "ehdaudio0D2",
>>> +		.codec_dai_name = "intel-hdmi-hifi2",
>>> +		.platform_name = "0000:00:1f.3",
>>> +		.init = skl_hda_hdmi2_init,
>>> +		.dpcm_playback = 1,
>>> +		.no_pcm = 1,
>>> +	},
>>> +	{
>>> +		.name = "iDisp3",
>>> +		.id = 3,
>>> +		.cpu_dai_name = "iDisp3 Pin",
>>> +		.codec_name = "ehdaudio0D2",
>>> +		.codec_dai_name = "intel-hdmi-hifi3",
>>> +		.platform_name = "0000:00:1f.3",
>>> +		.init = skl_hda_hdmi3_init,
>>> +		.dpcm_playback = 1,
>>> +		.no_pcm = 1,
>>> +	},
>>> +};
>>> +
>>> +#define NAME_SIZE	32
>>> +static int skl_hda_card_late_probe(struct snd_soc_card *card)
>>> +{
>>> +	struct skl_hda_private *ctx = snd_soc_card_get_drvdata(card);
>>> +	struct skl_hda_hdmi_pcm *pcm;
>>> +	struct snd_soc_codec *codec = NULL;
>>> +	int err, i = 0;
>>> +	char jack_name[NAME_SIZE];
>>> +
>>> +	list_for_each_entry(pcm, &ctx->hdmi_pcm_list, head) {
>>> +		codec = pcm->codec_dai->codec;
>>> +		snprintf(jack_name, sizeof(jack_name),
>>> +			"HDMI/DP, pcm=%d Jack", pcm->device);
>>> +		err = snd_soc_card_jack_new(card, jack_name,
>>> +					SND_JACK_AVOUT,
>> &skl_hda_hdmi_jack[i],
>>> +					NULL, 0);
>>> +
>>> +		if (err)
>>> +			return err;
>>> +
>>> +		err = hdac_hdmi_jack_init(pcm->codec_dai, pcm->device,
>>> +						&skl_hda_hdmi_jack[i]);
>>> +		if (err < 0)
>>> +			return err;
>>> +
>>> +		i++;
>>> +	}
>>> +
>>> +	if (!codec)
>>> +		return -EINVAL;
>>> +
>>> +	return hdac_hdmi_jack_port_init(codec, &card->dapm);
>>> +}
>>
>> this one in always the same as well, it could be factored across machine
>> drivers.
> 
> Okay, I will put this in the common file along with other functions that
> you mentioned previously.

thanks!

> 
>>
>>> +
>>> +static struct snd_soc_card skl_hda_audio_card = {
>>> +	.name = "skl_hda_card",
>>> +	.owner = THIS_MODULE,
>>> +	.dai_link = skl_hda_dais,
>>> +	.num_links = ARRAY_SIZE(skl_hda_dais),
>>> +	.dapm_routes = skl_hda_map,
>>> +	.num_dapm_routes = ARRAY_SIZE(skl_hda_map),
>>> +	.fully_routed = true,
>>> +	.late_probe = skl_hda_card_late_probe,
>>> +};
>>> +
>>> +static int skl_hda_audio_probe(struct platform_device *pdev)
>>> +{
>>> +	struct skl_hda_private *ctx;
>>> +
>>> +	dev_dbg(&pdev->dev, "%s: machine driver probe got called\n",
>> __func__);
>>> +
>>> +	ctx = devm_kzalloc(&pdev->dev, sizeof(*ctx), GFP_ATOMIC);
>>> +	if (!ctx)
>>> +		return -ENOMEM;
>>> +
>>> +	INIT_LIST_HEAD(&ctx->hdmi_pcm_list);
>>> +
>>> +	skl_hda_audio_card.dev = &pdev->dev;
>>> +	snd_soc_card_set_drvdata(&skl_hda_audio_card, ctx);
>>> +
>>> +	return devm_snd_soc_register_card(&pdev->dev,
>> &skl_hda_audio_card);
>>> +}
>>> +
>>> +static const struct platform_device_id skl_hda_board_ids[] = {
>>> +	{ .name = "skl_hda_generic" },
>>> +	{ }
>>> +};
>>> +
>>> +static struct platform_driver skl_hda_audio = {
>>> +	.probe = skl_hda_audio_probe,
>>> +	.driver = {
>>> +		.name = "skl_hda_generic",
>>> +		.pm = &snd_soc_pm_ops,
>>> +	},
>>> +	.id_table = skl_hda_board_ids,
>>> +};
>>> +
>>> +module_platform_driver(skl_hda_audio)
>>> +
>>> +/* Module information */
>>> +MODULE_DESCRIPTION("SKL/KBL HDA Generic Machine driver");
>>
>> again it's not limited to SKL/KBL, it'll work on APL and CNL and GLK ...
> 
> Yes, technically there is nothing limiting this to work on other platforms.
> It is just that right now I have only SKL/KBL in my test coverage for
> the current RFC series. I am hoping to get some initial feedback
> after which I will do send this patches for some formal preint for other
> platforms also before sending it.
> 

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

Re: [RFC 00/10] Enable HDA Codec support on Intel Platforms (Series2)

[Pierre-Louis Bossart]

On 12/3/17 10:21 PM, Vinod Koul wrote:
> On Sun, Dec 03, 2017 at 09:44:56PM -0600, Pierre-Louis Bossart wrote:
>> On 12/3/17 9:22 PM, Vinod Koul wrote:
>>> On Sun, Dec 03, 2017 at 09:15:21PM -0600, Pierre-Louis Bossart wrote:
>>>> On 12/3/17 11:20 AM, Vinod Koul wrote:
>>>>> On Fri, Dec 01, 2017 at 09:03:25PM +0100, Takashi Iwai wrote:
>>>>>>>>> FIXME:
>>>>>>>>> - KConfig changes does not look right, but I could not think of any proper
>>>>>>>>>     way without making changes into legacy HDA codec driver. So need some
>>>>>>>>>     help on this topic.
>>>>>>>>
>>>>>>>> And this is another big issue.  As long as you don't allow build both,
>>>>>>>> it never flies with distros, and it means no dramatic improvement of
>>>>>>>> the situation, either.
>>>>>>>>
>>>>>>>> IOW, we have to make the codec driver working on both controller/bus
>>>>>>>> implementations, and it essentially means to rewrite the HD-audio
>>>>>>>> codec plumbing code.  Currently, it's a kind of half-baked state, and
>>>>>>>> we need to cast to unified form.
>>>>>>>
>>>>>>> Before going into codec support, should we first deal with the
>>>>>>> selection of the ASoC/Legacy mode for the controller side of things?
>>>>>>> Even with basic HDMI/iDisp cases, we don't have a clean solution
>>>>>>> today. When I worked on the late Joule platform, there was no fallback
>>>>>>> mode which forced a recompilation depending on BIOS settings.
>>>>>>> My understanding is that there is no support for multiple drivers
>>>>>>> registering for the same PCI ID, I believe we'll have to define a
>>>>>>> top-level wrapper which arbitrates internally based on BIOS settings
>>>>>>> and user/distro preferences which mode is selected.
>>>>>>
>>>>>> True, we need to sort out that.  But the selection of the card-level
>>>>>> driver can be relatively easily done in a manual way, either via
>>>>>> blacklisting or by common module options.  Currently AMDGPU and radeon
>>>>>> drivers are in such situation, for example.
>>>>>>
>>>>>> A common single switch, or at best the automatic arbitration would be
>>>>>> preferred, of course, though.
>>>>>
>>>>> The switch is again short term as we should progress and move towards one
>>>>> solution whilst deprecating the other, so that way this makes a simple
>>>>> solution and helps in transition, change flag defaults and deprecate.
>>>>
>>>> Sorry, I don't get your point. Are you saying the legacy would be
>>>> deprecated? that was never my understanding, Rakesh's patches only make it
>>>> easier to use the DSP in combination with an HDaudio codec, but if the DSP
>>>> is not present the legacy is the fallback, no?
>>>
>>> for now yes, but what prevents us from supporting coupled mode when DSP is
>>> disabled :)
>>
>> That gets complicated. you'd have
>> 1. DSP-based ASoC driver for SKL+ w/ DSP
>> 2. coupled-mode ASoC driver for SKL+ w/o DSP
>> 3. legacy for pre-SKL platforms.
>>
>> I don't see what simplification this brings, it's complicated enough with 1.
>> and 3, and legacy is not going away.
> 
> in long term we _need_ to converge. 1 and 2 should be same driver detecting
> presence of DSP dynamically and doing DSP based or coupled mode.

Did we ever enable the coupled mode? if not then it's an hypothetical 
direction...

> 
> 3 would be separate drivers for different PCI IDs so no conflict.
> 
> Thanks
> 

Re: [RFC 01/10] ASoC: Intel: Boards: Machine driver for Intel platforms

[Ughreja, Rakesh A]

>-----Original Message-----
>From: Pierre-Louis Bossart [mailto:pierre-louis.bossart@linux.intel.com]
>Sent: Monday, December 4, 2017 8:20 PM
>To: Ughreja, Rakesh A <rakesh.a.ughreja@intel.com>; alsa-devel@alsa-
>project.org; broonie@kernel.org; tiwai@suse.de; liam.r.girdwood@linux.intel.com
>Cc: Koul, Vinod <vinod.koul@intel.com>; Patches Audio
><patches.audio@intel.com>
>Subject: Re: [alsa-devel] [RFC 01/10] ASoC: Intel: Boards: Machine driver for Intel
>platforms
>
>On 12/4/17 4:55 AM, Ughreja, Rakesh A wrote:
>>
>>
>>> -----Original Message-----
>>> From: Pierre-Louis Bossart [mailto:pierre-louis.bossart@linux.intel.com]
>>> Sent: Friday, December 1, 2017 11:28 PM
>>> To: Ughreja, Rakesh A <rakesh.a.ughreja@intel.com>; alsa-devel@alsa-
>>> project.org; broonie@kernel.org; tiwai@suse.de;
>>> liam.r.girdwood@linux.intel.com
>>> Cc: Koul, Vinod <vinod.koul@intel.com>; Patches Audio
>>> <patches.audio@intel.com>
>>> Subject: Re: [alsa-devel] [RFC 01/10] ASoC: Intel: Boards: Machine driver for
>>> Intel platforms
>>>
>>> On 12/1/17 3:13 AM, Rakesh Ughreja wrote:
>>>> Add machine driver for Intel platforms(SKL/KBL) with HDA and iDisp codecs.
>>>> This patch adds support for only iDisp (HDMI/DP) codec. In the
>>>> following patch support for HDA codec will be added.
>>>
>>> But to be clear this should be sufficient for headless devices with no
>>> audio codec (Joule or UP^2) where the only audio output is HDMI/iDisp?
>>
>> Yes, that’s right. Do you think it is better to have a separate machine
>> driver for headless devices where we don't have any HDA codecs ?
>>
>> We can load that machine driver when we find only the iDisp codec.
>
>not necessarily, it depends how you detect the HDaudio codec.

So you prefer to use the machine driver I posted even when we don't 
have HDA codec present on the system ? Technically it should work.
The only thing that you may see is some extra Front End DAI Links,
unless we use separate Topology files.
 

>>
>>>
>>>>
>>>> This should work for other Intel platforms as well e.g. BXT,GLK,CNL
>>>> however this series is not tested on all the platforms.
>>>>
>>>> Signed-off-by: Rakesh Ughreja <rakesh.a.ughreja@intel.com>
>>>> ---
>>>>    sound/soc/intel/boards/Kconfig           |  10 ++
>>>>    sound/soc/intel/boards/Makefile          |   2 +
>>>>    sound/soc/intel/boards/skl_hda_generic.c | 276
>>> +++++++++++++++++++++++++++++++
>>>
>>> can we drop the Skylake reference? It's become a catch-all term to mean
>>> both the platform, the IP and the driver.
>>
>> Suggest some name. I have no problem.
>
>HiFi3 ?
>iDisp ?
>HDAudio-DSP ?

hda_dsp_generic.c -- For the main file
hda_dsp_common.c -- for common functions 

Does it look fine ?

>
>>
>>>
>>>>    3 files changed, 288 insertions(+)
>>>>    create mode 100644 sound/soc/intel/boards/skl_hda_generic.c
>>>>
>>>> diff --git a/sound/soc/intel/boards/Kconfig
>>> b/sound/soc/intel/boards/Kconfig
>>>> index 6f75470..4f8bd02 100644
>>>> --- a/sound/soc/intel/boards/Kconfig
>>>> +++ b/sound/soc/intel/boards/Kconfig
>>>> @@ -262,4 +262,14 @@ config
>>> SND_SOC_INTEL_KBL_RT5663_RT5514_MAX98927_MACH
>>>>              Say Y if you have such a device.
>>>>              If unsure select "N".
>>>>
>>>> +config SND_SOC_INTEL_SKL_HDA_GENERIC_MACH
>>>> +        tristate "ASoC Audio driver for SKL/KBL with HDA Codecs"
>>>> +        select SND_SOC_INTEL_SST
>>>> +        depends on SND_SOC_INTEL_SKYLAKE
>>>
>>> that's also going to have to be reworked to allow for an SOF-based
>>> platform driver initializing this machine driver.
>>
>> Is this what you are fixing with your latest KConfig patches ?
>
>yes
>
>>
>>>
>>>> +        select SND_SOC_HDAC_HDMI
>>>> +        help
>>>> +          This adds support for ASoC Onboard Codec HDA machine driver.
>>> This will
>>>> +          create an alsa sound card for HDA Codecs.
>>>> +          Say Y if you have such a device.
>>>> +          If unsure select "N".
>>>>    endif
>>>> diff --git a/sound/soc/intel/boards/Makefile
>>> b/sound/soc/intel/boards/Makefile
>>>> index 69d2dfa..4686727 100644
>>>> --- a/sound/soc/intel/boards/Makefile
>>>> +++ b/sound/soc/intel/boards/Makefile
>>>> @@ -17,6 +17,7 @@ snd-soc-sst-byt-cht-nocodec-objs :=
>>> bytcht_nocodec.o
>>>>    snd-soc-kbl_rt5663_max98927-objs := kbl_rt5663_max98927.o
>>>>    snd-soc-kbl_rt5663_rt5514_max98927-objs :=
>>> kbl_rt5663_rt5514_max98927.o
>>>>    snd-soc-skl_rt286-objs := skl_rt286.o
>>>> +snd-soc-skl_hda_generic-objs := skl_hda_generic.o
>>>>    snd-skl_nau88l25_max98357a-objs := skl_nau88l25_max98357a.o
>>>>    snd-soc-skl_nau88l25_ssm4567-objs := skl_nau88l25_ssm4567.o
>>>>
>>>> @@ -40,3 +41,4 @@ obj-
>>> $(CONFIG_SND_SOC_INTEL_KBL_RT5663_RT5514_MAX98927_MACH) +=
>>> snd-soc-kbl_rt566
>>>>    obj-$(CONFIG_SND_SOC_INTEL_SKL_RT286_MACH) += snd-soc-
>>> skl_rt286.o
>>>>    obj-$(CONFIG_SND_SOC_INTEL_SKL_NAU88L25_MAX98357A_MACH) +=
>>> snd-skl_nau88l25_max98357a.o
>>>>    obj-$(CONFIG_SND_SOC_INTEL_SKL_NAU88L25_SSM4567_MACH) +=
>>> snd-soc-skl_nau88l25_ssm4567.o
>>>> +obj-$(CONFIG_SND_SOC_INTEL_SKL_HDA_GENERIC_MACH) += snd-soc-
>>> skl_hda_generic.o
>>>> diff --git a/sound/soc/intel/boards/skl_hda_generic.c
>>> b/sound/soc/intel/boards/skl_hda_generic.c
>>>> new file mode 100644
>>>> index 0000000..ece39b5
>>>> --- /dev/null
>>>> +++ b/sound/soc/intel/boards/skl_hda_generic.c
>>>> @@ -0,0 +1,276 @@
>>>> +/*
>>>> + * Intel Machine Driver for SKL/KBL platforms with HDA Codecs
>>>> + *
>>>> + * Copyright (C) 2017, Intel Corporation. All rights reserved.
>>>> + *
>>>> + *
>>>> + * This program is free software; you can redistribute it and/or
>>>> + * modify it under the terms of the GNU General Public License version
>>>> + * 2 as published by the Free Software Foundation.
>>>> + *
>>>> + * This program is distributed in the hope that it will be useful,
>>>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>>>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
>>>> + * GNU General Public License for more details.
>>>> + */
>>>> +
>>>> +#include <linux/module.h>
>>>> +#include <linux/platform_device.h>
>>>> +#include <sound/core.h>
>>>> +#include <sound/jack.h>
>>>> +#include <sound/pcm.h>
>>>> +#include <sound/pcm_params.h>
>>>> +#include <sound/soc.h>
>>>> +#include "../../codecs/hdac_hdmi.h"
>>>> +#include "../skylake/skl.h"
>>>> +
>>>> +static struct snd_soc_card skl_hda_audio_card;
>>>> +static struct snd_soc_jack skl_hda_hdmi_jack[3];
>>>
>>> The code from here to ...
>>>> +
>>>> +struct skl_hda_hdmi_pcm {
>>>> +	struct list_head head;
>>>> +	struct snd_soc_dai *codec_dai;
>>>> +	int device;
>>>> +};
>>>> +
>>>> +struct skl_hda_private {
>>>> +	struct list_head hdmi_pcm_list;
>>>> +};
>>>> +
>>>> +enum {
>>>> +	SKL_HDA_DPCM_AUDIO_HDMI1_PB = 0,
>>>> +	SKL_HDA_DPCM_AUDIO_HDMI2_PB,
>>>> +	SKL_HDA_DPCM_AUDIO_HDMI3_PB,
>>>> +};
>>>> +
>>>> +static const struct snd_soc_dapm_route skl_hda_map[] = {
>>>> +
>>>> +	{ "hifi3", NULL, "iDisp3 Tx"},
>>>> +	{ "iDisp3 Tx", NULL, "iDisp3_out"},
>>>> +	{ "hifi2", NULL, "iDisp2 Tx"},
>>>> +	{ "iDisp2 Tx", NULL, "iDisp2_out"},
>>>> +	{ "hifi1", NULL, "iDisp1 Tx"},
>>>> +	{ "iDisp1 Tx", NULL, "iDisp1_out"},
>>>> +
>>>> +};
>>>> +
>>>> +static int skl_hda_hdmi1_init(struct snd_soc_pcm_runtime *rtd)
>>>> +{
>>>> +	struct skl_hda_private *ctx = snd_soc_card_get_drvdata(rtd->card);
>>>> +	struct snd_soc_dai *dai = rtd->codec_dai;
>>>> +	struct skl_hda_hdmi_pcm *pcm;
>>>> +
>>>> +	pcm = devm_kzalloc(rtd->card->dev, sizeof(*pcm), GFP_KERNEL);
>>>> +	if (!pcm)
>>>> +		return -ENOMEM;
>>>> +
>>>> +	pcm->device = SKL_HDA_DPCM_AUDIO_HDMI1_PB;
>>>> +	pcm->codec_dai = dai;
>>>> +
>>>> +	list_add_tail(&pcm->head, &ctx->hdmi_pcm_list);
>>>> +
>>>> +	return 0;
>>>> +}
>>>> +
>>>> +static int skl_hda_hdmi2_init(struct snd_soc_pcm_runtime *rtd)
>>>> +{
>>>> +	struct skl_hda_private *ctx = snd_soc_card_get_drvdata(rtd->card);
>>>> +	struct snd_soc_dai *dai = rtd->codec_dai;
>>>> +	struct skl_hda_hdmi_pcm *pcm;
>>>> +
>>>> +	pcm = devm_kzalloc(rtd->card->dev, sizeof(*pcm), GFP_KERNEL);
>>>> +	if (!pcm)
>>>> +		return -ENOMEM;
>>>> +
>>>> +	pcm->device = SKL_HDA_DPCM_AUDIO_HDMI2_PB;
>>>> +	pcm->codec_dai = dai;
>>>> +
>>>> +	list_add_tail(&pcm->head, &ctx->hdmi_pcm_list);
>>>> +
>>>> +	return 0;
>>>> +}
>>>> +
>>>> +static int skl_hda_hdmi3_init(struct snd_soc_pcm_runtime *rtd)
>>>> +{
>>>> +	struct skl_hda_private *ctx = snd_soc_card_get_drvdata(rtd->card);
>>>> +	struct snd_soc_dai *dai = rtd->codec_dai;
>>>> +	struct skl_hda_hdmi_pcm *pcm;
>>>> +
>>>> +	pcm = devm_kzalloc(rtd->card->dev, sizeof(*pcm), GFP_KERNEL);
>>>> +	if (!pcm)
>>>> +		return -ENOMEM;
>>>> +
>>>> +	pcm->device = SKL_HDA_DPCM_AUDIO_HDMI3_PB;
>>>> +	pcm->codec_dai = dai;
>>>> +
>>>> +	list_add_tail(&pcm->head, &ctx->hdmi_pcm_list);
>>>> +
>>>> +	return 0;
>>>> +}
>>>> +
>>>
>>> .. here is pretty much copy/pasted across machine drivers, can we move
>>> it to a common include file? The experience from Baytrail/Cherrytrail is
>>> that the more we wait the more complicated it is to factor the code.
>>
>> That’s a good point. I can do that in my next series. But this would be
>> specific to SKL onwards platforms. I am not sure if it would work for
>> BYT/CHT Etc.
>
>this code would not appl for BYT/CHT since the path doesn't go through
>the DSP, so no ASoC and machine driver?

Yes, I think we don't have ASoC Machine drivers for BYT/CHT HDMI/HDA
related things. It's all legacy way.

>
>>
>>>
>>>> +/* skl_hda_digital audio interface glue - connects codec <--> CPU */
>>>> +static struct snd_soc_dai_link skl_hda_dais[] = {
>>>> +	/* Front End DAI links */
>>>> +	[SKL_HDA_DPCM_AUDIO_HDMI1_PB] = {
>>>
>>> Are those hard-coded links required for all platforms, I thought the
>>> newer ones were based on topology?
>>
>> Yes, I agree. In the latest patches from Guneshwor, the DAI Links
>> are coming from Topology. I can fix it in the next revision
>
>I was wondering if this change to topology is across the board.

In my Topology file, I didn't have FE DAI, DAI Links. That’s why those are
created manually in the Machine driver. I will check and get back how
it works when I put it into Topology file.


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

Re: [RFC 06/10] ASoC: hdac_hda: add ASoC based HDA codec driver

[Ughreja, Rakesh A]

>-----Original Message-----
>From: Pierre-Louis Bossart [mailto:pierre-louis.bossart@linux.intel.com]
>Sent: Saturday, December 2, 2017 1:07 AM
>To: Ughreja, Rakesh A <rakesh.a.ughreja@intel.com>; alsa-devel@alsa-
>project.org; broonie@kernel.org; tiwai@suse.de; liam.r.girdwood@linux.intel.com
>Cc: Koul, Vinod <vinod.koul@intel.com>; Patches Audio
><patches.audio@intel.com>
>Subject: Re: [alsa-devel] [RFC 06/10] ASoC: hdac_hda: add ASoC based HDA
>codec driver
>
>On 12/1/17 3:14 AM, Rakesh Ughreja wrote:
>> This patch adds ASoC based HDA codec driver that can be used with
>> all Intel platforms.
>>
>> FIXME:
>> KConfig change allows users to compile legacy HDA codec drivers either
>> as ASoC or ALSA codec driver. This is not a good approach and need
>> suggestion to improve it.
>>
>> Signed-off-by: Rakesh Ughreja <rakesh.a.ughreja@intel.com>
>> ---
>>   sound/pci/hda/Kconfig         | 11 +++++
>>   sound/pci/hda/hda_codec.h     | 26 +++++++++++-
>>   sound/soc/codecs/Kconfig      |  6 +++
>>   sound/soc/codecs/Makefile     |  2 +
>>   sound/soc/codecs/hdac_hda.c   | 95
>+++++++++++++++++++++++++++++++++++++++++++
>>   sound/soc/codecs/hdac_hda.h   | 16 ++++++++
>>   sound/soc/intel/skylake/skl.c | 18 +++++++-
>>   7 files changed, 171 insertions(+), 3 deletions(-)
>>   create mode 100644 sound/soc/codecs/hdac_hda.c
>>   create mode 100644 sound/soc/codecs/hdac_hda.h
>>
>> diff --git a/sound/pci/hda/Kconfig b/sound/pci/hda/Kconfig
>> index 7f3b5ed..010dd7d 100644
>> --- a/sound/pci/hda/Kconfig
>> +++ b/sound/pci/hda/Kconfig
>> @@ -215,6 +215,17 @@ config SND_HDA_GENERIC
>>   comment "Set to Y if you want auto-loading the codec driver"
>>   	depends on SND_HDA=y && SND_HDA_GENERIC=m
>>
>> +config SND_ASOC_HDA_GENERIC
>> +        bool "Build HDA drivers as HDA ASoC Codec drivers"
>> +	select SND_SOC_HDAC_HDA
>> +        def_bool n
>> +        help
>> +          Select to build legacy HD-audio codec drivers as ASoC
>> +          codec drivers.
>> +
>> +comment "Set to Y if you want to enable support for HDA audio codec with
>> +	 DSP on Intel platforms"
>> +
>>   config SND_HDA_POWER_SAVE_DEFAULT
>>   	int "Default time-out for HD-audio power-save mode"
>>   	depends on PM
>> diff --git a/sound/pci/hda/hda_codec.h b/sound/pci/hda/hda_codec.h
>> index 2d02d02..67ce2e6 100644
>> --- a/sound/pci/hda/hda_codec.h
>> +++ b/sound/pci/hda/hda_codec.h
>> @@ -104,9 +104,33 @@ int __hda_codec_driver_register(struct
>hda_codec_driver *drv, const char *name,
>>   #define hda_codec_driver_register(drv) \
>>   	__hda_codec_driver_register(drv, KBUILD_MODNAME, THIS_MODULE)
>>   void hda_codec_driver_unregister(struct hda_codec_driver *drv);
>> +
>> +/*
>> + * FIXME:
>> + * In this implementation one can compile codec drivers either as ALSA or as
>> + * ASoC, but not both.
>> + * This may be not the be the best way to build the HDA codec drivers as ALSA
>> + * vs ASoC driver. Need to find a better way, where the driver can be built
>> + * as ALSA as well as ASoC at the same time. Even though practically people
>> + * would never use them at the same time. But may be disto guys may want to
>> + * do it.
>
>Distros *have* to do it, since the DSP usage can be changed in
>user-visible BIOS settings or disabled.

Yes, agree.

>
>> + */
>> +
>> +#ifdef CONFIG_SND_ASOC_HDA_GENERIC
>> +int __hdac_hda_codec_driver_register(struct hda_codec_driver *drv,
>> +	const char *name, struct module *owner);
>> +#define hdac_hda_codec_driver_register(drv) \
>> +	__hdac_hda_codec_driver_register(drv, KBUILD_MODNAME,
>THIS_MODULE)
>> +void hdac_hda_codec_driver_unregister(struct hda_codec_driver *drv);
>> +
>> +#define module_hda_codec_driver(drv) \
>> +	module_driver(drv, hdac_hda_codec_driver_register, \
>> +			hdac_hda_codec_driver_unregister)
>> +#else
>>   #define module_hda_codec_driver(drv) \
>>   	module_driver(drv, hda_codec_driver_register, \
>> -		      hda_codec_driver_unregister)
>> +			hda_codec_driver_unregister)
>> +#endif
>
>Couldn't you use a common registration that internally checks if you are
>running in legacy or DSP/ASoC mode and branch accordingly?

Yes, this requires change in the legacy codec driver. Takashi also
suggested that we need to change the legacy driver code in order to make
it work. Now that we are okay to change the legacy driver source code,
I will come up with some proposal.

Re: [RFC 01/10] ASoC: Intel: Boards: Machine driver for Intel platforms

[Pierre-Louis Bossart]

On 12/4/17 9:10 AM, Ughreja, Rakesh A wrote:
> 
> 
>> -----Original Message-----
>> From: Pierre-Louis Bossart [mailto:pierre-louis.bossart@linux.intel.com]
>> Sent: Monday, December 4, 2017 8:20 PM
>> To: Ughreja, Rakesh A <rakesh.a.ughreja@intel.com>; alsa-devel@alsa-
>> project.org; broonie@kernel.org; tiwai@suse.de; liam.r.girdwood@linux.intel.com
>> Cc: Koul, Vinod <vinod.koul@intel.com>; Patches Audio
>> <patches.audio@intel.com>
>> Subject: Re: [alsa-devel] [RFC 01/10] ASoC: Intel: Boards: Machine driver for Intel
>> platforms
>>
>> On 12/4/17 4:55 AM, Ughreja, Rakesh A wrote:
>>>
>>>
>>>> -----Original Message-----
>>>> From: Pierre-Louis Bossart [mailto:pierre-louis.bossart@linux.intel.com]
>>>> Sent: Friday, December 1, 2017 11:28 PM
>>>> To: Ughreja, Rakesh A <rakesh.a.ughreja@intel.com>; alsa-devel@alsa-
>>>> project.org; broonie@kernel.org; tiwai@suse.de;
>>>> liam.r.girdwood@linux.intel.com
>>>> Cc: Koul, Vinod <vinod.koul@intel.com>; Patches Audio
>>>> <patches.audio@intel.com>
>>>> Subject: Re: [alsa-devel] [RFC 01/10] ASoC: Intel: Boards: Machine driver for
>>>> Intel platforms
>>>>
>>>> On 12/1/17 3:13 AM, Rakesh Ughreja wrote:
>>>>> Add machine driver for Intel platforms(SKL/KBL) with HDA and iDisp codecs.
>>>>> This patch adds support for only iDisp (HDMI/DP) codec. In the
>>>>> following patch support for HDA codec will be added.
>>>>
>>>> But to be clear this should be sufficient for headless devices with no
>>>> audio codec (Joule or UP^2) where the only audio output is HDMI/iDisp?
>>>
>>> Yes, that’s right. Do you think it is better to have a separate machine
>>> driver for headless devices where we don't have any HDA codecs ?
>>>
>>> We can load that machine driver when we find only the iDisp codec.
>>
>> not necessarily, it depends how you detect the HDaudio codec.
> 
> So you prefer to use the machine driver I posted even when we don't
> have HDA codec present on the system ? Technically it should work.
> The only thing that you may see is some extra Front End DAI Links,
> unless we use separate Topology files.

we can deal with this later, this was just a remark to think of headless 
devices.

>   
> 
>>>
>>>>
>>>>>
>>>>> This should work for other Intel platforms as well e.g. BXT,GLK,CNL
>>>>> however this series is not tested on all the platforms.
>>>>>
>>>>> Signed-off-by: Rakesh Ughreja <rakesh.a.ughreja@intel.com>
>>>>> ---
>>>>>     sound/soc/intel/boards/Kconfig           |  10 ++
>>>>>     sound/soc/intel/boards/Makefile          |   2 +
>>>>>     sound/soc/intel/boards/skl_hda_generic.c | 276
>>>> +++++++++++++++++++++++++++++++
>>>>
>>>> can we drop the Skylake reference? It's become a catch-all term to mean
>>>> both the platform, the IP and the driver.
>>>
>>> Suggest some name. I have no problem.
>>
>> HiFi3 ?
>> iDisp ?
>> HDAudio-DSP ?
> 
> hda_dsp_generic.c -- For the main file
> hda_dsp_common.c -- for common functions
> 
> Does it look fine ?

works for me.

> 
>>
>>>
>>>>
>>>>>     3 files changed, 288 insertions(+)
>>>>>     create mode 100644 sound/soc/intel/boards/skl_hda_generic.c
>>>>>
>>>>> diff --git a/sound/soc/intel/boards/Kconfig
>>>> b/sound/soc/intel/boards/Kconfig
>>>>> index 6f75470..4f8bd02 100644
>>>>> --- a/sound/soc/intel/boards/Kconfig
>>>>> +++ b/sound/soc/intel/boards/Kconfig
>>>>> @@ -262,4 +262,14 @@ config
>>>> SND_SOC_INTEL_KBL_RT5663_RT5514_MAX98927_MACH
>>>>>               Say Y if you have such a device.
>>>>>               If unsure select "N".
>>>>>
>>>>> +config SND_SOC_INTEL_SKL_HDA_GENERIC_MACH
>>>>> +        tristate "ASoC Audio driver for SKL/KBL with HDA Codecs"
>>>>> +        select SND_SOC_INTEL_SST
>>>>> +        depends on SND_SOC_INTEL_SKYLAKE
>>>>
>>>> that's also going to have to be reworked to allow for an SOF-based
>>>> platform driver initializing this machine driver.
>>>
>>> Is this what you are fixing with your latest KConfig patches ?
>>
>> yes
>>
>>>
>>>>
>>>>> +        select SND_SOC_HDAC_HDMI
>>>>> +        help
>>>>> +          This adds support for ASoC Onboard Codec HDA machine driver.
>>>> This will
>>>>> +          create an alsa sound card for HDA Codecs.
>>>>> +          Say Y if you have such a device.
>>>>> +          If unsure select "N".
>>>>>     endif
>>>>> diff --git a/sound/soc/intel/boards/Makefile
>>>> b/sound/soc/intel/boards/Makefile
>>>>> index 69d2dfa..4686727 100644
>>>>> --- a/sound/soc/intel/boards/Makefile
>>>>> +++ b/sound/soc/intel/boards/Makefile
>>>>> @@ -17,6 +17,7 @@ snd-soc-sst-byt-cht-nocodec-objs :=
>>>> bytcht_nocodec.o
>>>>>     snd-soc-kbl_rt5663_max98927-objs := kbl_rt5663_max98927.o
>>>>>     snd-soc-kbl_rt5663_rt5514_max98927-objs :=
>>>> kbl_rt5663_rt5514_max98927.o
>>>>>     snd-soc-skl_rt286-objs := skl_rt286.o
>>>>> +snd-soc-skl_hda_generic-objs := skl_hda_generic.o
>>>>>     snd-skl_nau88l25_max98357a-objs := skl_nau88l25_max98357a.o
>>>>>     snd-soc-skl_nau88l25_ssm4567-objs := skl_nau88l25_ssm4567.o
>>>>>
>>>>> @@ -40,3 +41,4 @@ obj-
>>>> $(CONFIG_SND_SOC_INTEL_KBL_RT5663_RT5514_MAX98927_MACH) +=
>>>> snd-soc-kbl_rt566
>>>>>     obj-$(CONFIG_SND_SOC_INTEL_SKL_RT286_MACH) += snd-soc-
>>>> skl_rt286.o
>>>>>     obj-$(CONFIG_SND_SOC_INTEL_SKL_NAU88L25_MAX98357A_MACH) +=
>>>> snd-skl_nau88l25_max98357a.o
>>>>>     obj-$(CONFIG_SND_SOC_INTEL_SKL_NAU88L25_SSM4567_MACH) +=
>>>> snd-soc-skl_nau88l25_ssm4567.o
>>>>> +obj-$(CONFIG_SND_SOC_INTEL_SKL_HDA_GENERIC_MACH) += snd-soc-
>>>> skl_hda_generic.o
>>>>> diff --git a/sound/soc/intel/boards/skl_hda_generic.c
>>>> b/sound/soc/intel/boards/skl_hda_generic.c
>>>>> new file mode 100644
>>>>> index 0000000..ece39b5
>>>>> --- /dev/null
>>>>> +++ b/sound/soc/intel/boards/skl_hda_generic.c
>>>>> @@ -0,0 +1,276 @@
>>>>> +/*
>>>>> + * Intel Machine Driver for SKL/KBL platforms with HDA Codecs
>>>>> + *
>>>>> + * Copyright (C) 2017, Intel Corporation. All rights reserved.
>>>>> + *
>>>>> + *
>>>>> + * This program is free software; you can redistribute it and/or
>>>>> + * modify it under the terms of the GNU General Public License version
>>>>> + * 2 as published by the Free Software Foundation.
>>>>> + *
>>>>> + * This program is distributed in the hope that it will be useful,
>>>>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>>>>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
>>>>> + * GNU General Public License for more details.
>>>>> + */
>>>>> +
>>>>> +#include <linux/module.h>
>>>>> +#include <linux/platform_device.h>
>>>>> +#include <sound/core.h>
>>>>> +#include <sound/jack.h>
>>>>> +#include <sound/pcm.h>
>>>>> +#include <sound/pcm_params.h>
>>>>> +#include <sound/soc.h>
>>>>> +#include "../../codecs/hdac_hdmi.h"
>>>>> +#include "../skylake/skl.h"
>>>>> +
>>>>> +static struct snd_soc_card skl_hda_audio_card;
>>>>> +static struct snd_soc_jack skl_hda_hdmi_jack[3];
>>>>
>>>> The code from here to ...
>>>>> +
>>>>> +struct skl_hda_hdmi_pcm {
>>>>> +	struct list_head head;
>>>>> +	struct snd_soc_dai *codec_dai;
>>>>> +	int device;
>>>>> +};
>>>>> +
>>>>> +struct skl_hda_private {
>>>>> +	struct list_head hdmi_pcm_list;
>>>>> +};
>>>>> +
>>>>> +enum {
>>>>> +	SKL_HDA_DPCM_AUDIO_HDMI1_PB = 0,
>>>>> +	SKL_HDA_DPCM_AUDIO_HDMI2_PB,
>>>>> +	SKL_HDA_DPCM_AUDIO_HDMI3_PB,
>>>>> +};
>>>>> +
>>>>> +static const struct snd_soc_dapm_route skl_hda_map[] = {
>>>>> +
>>>>> +	{ "hifi3", NULL, "iDisp3 Tx"},
>>>>> +	{ "iDisp3 Tx", NULL, "iDisp3_out"},
>>>>> +	{ "hifi2", NULL, "iDisp2 Tx"},
>>>>> +	{ "iDisp2 Tx", NULL, "iDisp2_out"},
>>>>> +	{ "hifi1", NULL, "iDisp1 Tx"},
>>>>> +	{ "iDisp1 Tx", NULL, "iDisp1_out"},
>>>>> +
>>>>> +};
>>>>> +
>>>>> +static int skl_hda_hdmi1_init(struct snd_soc_pcm_runtime *rtd)
>>>>> +{
>>>>> +	struct skl_hda_private *ctx = snd_soc_card_get_drvdata(rtd->card);
>>>>> +	struct snd_soc_dai *dai = rtd->codec_dai;
>>>>> +	struct skl_hda_hdmi_pcm *pcm;
>>>>> +
>>>>> +	pcm = devm_kzalloc(rtd->card->dev, sizeof(*pcm), GFP_KERNEL);
>>>>> +	if (!pcm)
>>>>> +		return -ENOMEM;
>>>>> +
>>>>> +	pcm->device = SKL_HDA_DPCM_AUDIO_HDMI1_PB;
>>>>> +	pcm->codec_dai = dai;
>>>>> +
>>>>> +	list_add_tail(&pcm->head, &ctx->hdmi_pcm_list);
>>>>> +
>>>>> +	return 0;
>>>>> +}
>>>>> +
>>>>> +static int skl_hda_hdmi2_init(struct snd_soc_pcm_runtime *rtd)
>>>>> +{
>>>>> +	struct skl_hda_private *ctx = snd_soc_card_get_drvdata(rtd->card);
>>>>> +	struct snd_soc_dai *dai = rtd->codec_dai;
>>>>> +	struct skl_hda_hdmi_pcm *pcm;
>>>>> +
>>>>> +	pcm = devm_kzalloc(rtd->card->dev, sizeof(*pcm), GFP_KERNEL);
>>>>> +	if (!pcm)
>>>>> +		return -ENOMEM;
>>>>> +
>>>>> +	pcm->device = SKL_HDA_DPCM_AUDIO_HDMI2_PB;
>>>>> +	pcm->codec_dai = dai;
>>>>> +
>>>>> +	list_add_tail(&pcm->head, &ctx->hdmi_pcm_list);
>>>>> +
>>>>> +	return 0;
>>>>> +}
>>>>> +
>>>>> +static int skl_hda_hdmi3_init(struct snd_soc_pcm_runtime *rtd)
>>>>> +{
>>>>> +	struct skl_hda_private *ctx = snd_soc_card_get_drvdata(rtd->card);
>>>>> +	struct snd_soc_dai *dai = rtd->codec_dai;
>>>>> +	struct skl_hda_hdmi_pcm *pcm;
>>>>> +
>>>>> +	pcm = devm_kzalloc(rtd->card->dev, sizeof(*pcm), GFP_KERNEL);
>>>>> +	if (!pcm)
>>>>> +		return -ENOMEM;
>>>>> +
>>>>> +	pcm->device = SKL_HDA_DPCM_AUDIO_HDMI3_PB;
>>>>> +	pcm->codec_dai = dai;
>>>>> +
>>>>> +	list_add_tail(&pcm->head, &ctx->hdmi_pcm_list);
>>>>> +
>>>>> +	return 0;
>>>>> +}
>>>>> +
>>>>
>>>> .. here is pretty much copy/pasted across machine drivers, can we move
>>>> it to a common include file? The experience from Baytrail/Cherrytrail is
>>>> that the more we wait the more complicated it is to factor the code.
>>>
>>> That’s a good point. I can do that in my next series. But this would be
>>> specific to SKL onwards platforms. I am not sure if it would work for
>>> BYT/CHT Etc.
>>
>> this code would not appl for BYT/CHT since the path doesn't go through
>> the DSP, so no ASoC and machine driver?
> 
> Yes, I think we don't have ASoC Machine drivers for BYT/CHT HDMI/HDA
> related things. It's all legacy way.
> 
>>
>>>
>>>>
>>>>> +/* skl_hda_digital audio interface glue - connects codec <--> CPU */
>>>>> +static struct snd_soc_dai_link skl_hda_dais[] = {
>>>>> +	/* Front End DAI links */
>>>>> +	[SKL_HDA_DPCM_AUDIO_HDMI1_PB] = {
>>>>
>>>> Are those hard-coded links required for all platforms, I thought the
>>>> newer ones were based on topology?
>>>
>>> Yes, I agree. In the latest patches from Guneshwor, the DAI Links
>>> are coming from Topology. I can fix it in the next revision
>>
>> I was wondering if this change to topology is across the board.
> 
> In my Topology file, I didn't have FE DAI, DAI Links. That’s why those are
> created manually in the Machine driver. I will check and get back how
> it works when I put it into Topology file.
> 
> 

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

Re: [RFC 05/10] ALSA: hda - make some of the functions externally visible

[Ughreja, Rakesh A]

>-----Original Message-----
>From: Pierre-Louis Bossart [mailto:pierre-louis.bossart@linux.intel.com]
>Sent: Saturday, December 2, 2017 12:57 AM
>To: Ughreja, Rakesh A <rakesh.a.ughreja@intel.com>; alsa-devel@alsa-
>project.org; broonie@kernel.org; tiwai@suse.de; liam.r.girdwood@linux.intel.com
>Cc: Koul, Vinod <vinod.koul@intel.com>; Patches Audio
><patches.audio@intel.com>
>Subject: Re: [alsa-devel] [RFC 05/10] ALSA: hda - make some of the functions
>externally visible
>
>On 12/1/17 3:14 AM, Rakesh Ughreja wrote:
>> Mark some functions with EXPORT_SYMBOL_GPL so that it can be called by
>> other kernel modules. These APIs would be called by ASoC based HDA codec
>> driver which will be added in the later patches.
>
>Looks ok. Just wondering from a code maintenance perspective if those
>functions would need to be part of a library, and if this means that to
>compile the ASoC based HDAudio codec support you still need to enable
>the legacy support - and if this leads to potential conflicts on which
>driver gets probed?

Yes, I think these functions are already part of library.
These are part of file hda_codec.c, which looks like a library to me.

Takashi is that correct understanding ?

Re: [RFC 04/10] ASoC: Intel: Skylake: use hda_bus instead of hdac_bus

[Ughreja, Rakesh A]

>-----Original Message-----
>From: Pierre-Louis Bossart [mailto:pierre-louis.bossart@linux.intel.com]
>Sent: Friday, December 1, 2017 11:57 PM
>To: Ughreja, Rakesh A <rakesh.a.ughreja@intel.com>; alsa-devel@alsa-
>project.org; broonie@kernel.org; tiwai@suse.de; liam.r.girdwood@linux.intel.com
>Cc: Koul, Vinod <vinod.koul@intel.com>; Patches Audio
><patches.audio@intel.com>
>Subject: Re: [alsa-devel] [RFC 04/10] ASoC: Intel: Skylake: use hda_bus instead of
>hdac_bus
>
>On 12/1/17 3:14 AM, Rakesh Ughreja wrote:
>> This patch prepares SKL platform driver to make reuse of legacy HDA
>> codec drivers. It does following things.
>>
>> use hda_bus instead of hdac_bus in the SKL platform driver to align
>> with the legacy controller driver.
>>
>> modify snd_hdac_ext_bus_device_init definition to align with
>> snd_hdac_bus_device_init, used by legacy drivers.
>> Memory for hdac_device is allocated by the caller.
>>
>> Signed-off-by: Rakesh Ughreja <rakesh.a.ughreja@intel.com>
>> ---
>>   include/sound/hdaudio_ext.h   |  3 ++-
>>   sound/hda/ext/hdac_ext_bus.c  |  9 ++-------
>>   sound/soc/intel/skylake/skl.c | 19 ++++++++++++++++++-
>>   sound/soc/intel/skylake/skl.h | 10 +++++++---
>>   4 files changed, 29 insertions(+), 12 deletions(-)
>>
>> diff --git a/include/sound/hdaudio_ext.h b/include/sound/hdaudio_ext.h
>> index 3c30247..c188b80 100644
>> --- a/include/sound/hdaudio_ext.h
>> +++ b/include/sound/hdaudio_ext.h
>> @@ -9,7 +9,8 @@ int snd_hdac_ext_bus_init(struct hdac_bus *bus, struct device
>*dev,
>>   		      const struct hdac_io_ops *io_ops);
>>
>>   void snd_hdac_ext_bus_exit(struct hdac_bus *bus);
>> -int snd_hdac_ext_bus_device_init(struct hdac_bus *bus, int addr);
>> +int snd_hdac_ext_bus_device_init(struct hdac_bus *bus, int addr,
>> +						struct hdac_device *hdev);
>>   void snd_hdac_ext_bus_device_exit(struct hdac_device *hdev);
>>   void snd_hdac_ext_bus_device_remove(struct hdac_bus *bus);
>>
>> diff --git a/sound/hda/ext/hdac_ext_bus.c b/sound/hda/ext/hdac_ext_bus.c
>> index 52f0776..e4bcb76 100644
>> --- a/sound/hda/ext/hdac_ext_bus.c
>> +++ b/sound/hda/ext/hdac_ext_bus.c
>> @@ -135,16 +135,12 @@ static void default_release(struct device *dev)
>>    *
>>    * Returns zero for success or a negative error code.
>>    */
>> -int snd_hdac_ext_bus_device_init(struct hdac_bus *bus, int addr)
>> +int snd_hdac_ext_bus_device_init(struct hdac_bus *bus, int addr,
>> +					struct hdac_device *hdev)
>>   {
>> -	struct hdac_device *hdev = NULL;
>>   	char name[15];
>>   	int ret;
>>
>> -	hdev = kzalloc(sizeof(*hdev), GFP_KERNEL);
>> -	if (!hdev)
>> -		return -ENOMEM;
>> -
>>   	hdev->bus = bus;
>>
>>   	snprintf(name, sizeof(name), "ehdaudio%dD%d", bus->idx, addr);
>> @@ -175,7 +171,6 @@ EXPORT_SYMBOL_GPL(snd_hdac_ext_bus_device_init);
>>   void snd_hdac_ext_bus_device_exit(struct hdac_device *hdev)
>>   {
>>   	snd_hdac_device_exit(hdev);
>> -	kfree(hdev);
>>   }
>>   EXPORT_SYMBOL_GPL(snd_hdac_ext_bus_device_exit);
>>
>> diff --git a/sound/soc/intel/skylake/skl.c b/sound/soc/intel/skylake/skl.c
>> index 568a285..72a788a 100644
>> --- a/sound/soc/intel/skylake/skl.c
>> +++ b/sound/soc/intel/skylake/skl.c
>> @@ -35,6 +35,7 @@
>>   #include "skl.h"
>>   #include "skl-sst-dsp.h"
>>   #include "skl-sst-ipc.h"
>> +#include "../../../pci/hda/hda_codec.h"
>>
>>   static struct skl_machine_pdata skl_dmic_data;
>>
>> @@ -531,6 +532,8 @@ static int probe_codec(struct hdac_bus *bus, int addr)
>>   	unsigned int cmd = (addr << 28) | (AC_NODE_ROOT << 20) |
>>   		(AC_VERB_PARAMETERS << 8) | AC_PAR_VENDOR_ID;
>>   	unsigned int res = -1;
>> +	struct skl *skl = bus_to_skl(bus);
>> +	struct hdac_device *hdev;
>>
>>   	mutex_lock(&bus->cmd_mutex);
>>   	snd_hdac_bus_send_cmd(bus, cmd);
>> @@ -540,7 +543,11 @@ static int probe_codec(struct hdac_bus *bus, int addr)
>>   		return -EIO;
>>   	dev_dbg(bus->dev, "codec #%d probed OK\n", addr);
>>
>> -	return snd_hdac_ext_bus_device_init(bus, addr);
>> +	hdev = devm_kzalloc(&skl->pci->dev, sizeof(*hdev), GFP_KERNEL);
>> +	if (!hdev)
>> +		return -ENOMEM;
>> +
>> +	return snd_hdac_ext_bus_device_init(bus, addr, hdev);
>>   }
>>
>>   /* Codec initialization */
>> @@ -665,6 +672,7 @@ static int skl_create(struct pci_dev *pci,
>>   {
>>   	struct skl *skl;
>>   	struct hdac_bus *bus;
>> +	struct hda_bus *hbus;
>>
>>   	int err;
>>
>> @@ -680,6 +688,7 @@ static int skl_create(struct pci_dev *pci,
>>   		return -ENOMEM;
>>   	}
>>
>> +	hbus = skl_to_hbus(skl);
>>   	bus = skl_to_bus(skl);
>>   	snd_hdac_ext_bus_init(bus, &pci->dev, &bus_core_ops, io_ops);
>>   	bus->use_posbuf = 1;
>> @@ -687,6 +696,14 @@ static int skl_create(struct pci_dev *pci,
>>   	INIT_WORK(&skl->probe_work, skl_probe_work);
>>   	bus->bdl_pos_adj = 0;
>>
>> +	/*
>> +	 * TODO: other parameters can be taken the way it is taken by
>> +	 * legacy HDA driver
>
>what parameters where you referring to? kernel module?

In the following code, I have not initialized all the fields of hbus.
So in this TODO, I wanted to remind myself that all the parameters
of hbus needs to be initialized as required by legacy codec driver.

>
>> +	 */
>> +	mutex_init(&hbus->prepare_mutex);
>> +	hbus->pci = pci;
>> +	hbus->mixer_assigned = -1;
>> +

Re: [RFC 03/10] ASoC: Intel: Skylake: add HDA BE DAIs

[Ughreja, Rakesh A]

>-----Original Message-----
>From: Pierre-Louis Bossart [mailto:pierre-louis.bossart@linux.intel.com]
>Sent: Friday, December 1, 2017 11:50 PM
>To: Ughreja, Rakesh A <rakesh.a.ughreja@intel.com>; alsa-devel@alsa-
>project.org; broonie@kernel.org; tiwai@suse.de; liam.r.girdwood@linux.intel.com
>Cc: Koul, Vinod <vinod.koul@intel.com>; Patches Audio
><patches.audio@intel.com>
>Subject: Re: [alsa-devel] [RFC 03/10] ASoC: Intel: Skylake: add HDA BE DAIs
>
>On 12/1/17 3:14 AM, Rakesh Ughreja wrote:
>> Add support for HDA BE DAIs in SKL platform driver.
>>
>> Signed-off-by: Rakesh Ughreja <rakesh.a.ughreja@intel.com>
>> ---
>>   sound/soc/intel/skylake/skl-pcm.c | 32 +++++++++++++++++++++++++------
>-
>>   1 file changed, 25 insertions(+), 7 deletions(-)
>>
>> diff --git a/sound/soc/intel/skylake/skl-pcm.c b/sound/soc/intel/skylake/skl-
>pcm.c
>> index e6fee7a..3dcb241 100644
>> --- a/sound/soc/intel/skylake/skl-pcm.c
>> +++ b/sound/soc/intel/skylake/skl-pcm.c
>> @@ -956,21 +956,39 @@ static struct snd_soc_dai_driver skl_platform_dai[] = {
>>   	},
>>   },
>>   {
>> -	.name = "HD-Codec Pin",
>> +	.name = "Analog CPU DAI",
>>   	.ops = &skl_link_dai_ops,
>>   	.playback = {
>> -		.stream_name = "HD-Codec Tx",
>> -		.channels_min = HDA_STEREO,
>> +		.stream_name = "Analog CPU Playback",
>> +		.channels_min = HDA_MONO,
>>   		.channels_max = HDA_STEREO,
>>   		.rates = SNDRV_PCM_RATE_48000,
>> -		.formats = SNDRV_PCM_FMTBIT_S16_LE,
>> +		.formats = SNDRV_PCM_FMTBIT_S16_LE |
>SNDRV_PCM_FMTBIT_S24_LE,
>>   	},
>>   	.capture = {
>> -		.stream_name = "HD-Codec Rx",
>> -		.channels_min = HDA_STEREO,
>> +		.stream_name = "Analog CPU Capture",
>> +		.channels_min = HDA_MONO,
>>   		.channels_max = HDA_STEREO,
>>   		.rates = SNDRV_PCM_RATE_48000,
>> -		.formats = SNDRV_PCM_FMTBIT_S16_LE,
>> +		.formats = SNDRV_PCM_FMTBIT_S16_LE |
>SNDRV_PCM_FMTBIT_S24_LE,
>> +	},
>> +},
>> +{
>> +	.name = "Digital CPU DAI",
>
>I am not sure I get the meaning of analog/digital.
>
>Did you mean external codec vs. iDisp codec?
>
>Or did you really mean analog? I don't think you can control this front
>the CPU side, e.g. if the codec has an S/PDIF output it remains digital.

I was trying to map the BE CPU DAIs to hda_pcm_stream. 
The way it is defined here.
https://elixir.free-electrons.com/linux/v4.15-rc2/source/sound/pci/hda/hda_generic.c#L5374

>
>> +	.ops = &skl_link_dai_ops,
>> +	.playback = {
>> +		.stream_name = "Digital CPU Playback",
>> +		.channels_min = HDA_MONO,
>> +		.channels_max = HDA_STEREO,
>> +		.rates = SNDRV_PCM_RATE_48000,
>> +		.formats = SNDRV_PCM_FMTBIT_S16_LE |
>SNDRV_PCM_FMTBIT_S24_LE,
>> +	},
>> +	.capture = {
>> +		.stream_name = "Digital CPU Capture",
>> +		.channels_min = HDA_MONO,
>> +		.channels_max = HDA_STEREO,
>> +		.rates = SNDRV_PCM_RATE_48000,
>> +		.formats = SNDRV_PCM_FMTBIT_S16_LE |
>SNDRV_PCM_FMTBIT_S24_LE,
>>   	},
>>   },
>>   };
>>

Re: [RFC 05/10] ALSA: hda - make some of the functions externally visible

[Takashi Iwai]

On Mon, 04 Dec 2017 16:43:45 +0100,
Ughreja, Rakesh A wrote:
> 
> 
> 
> >-----Original Message-----
> >From: Pierre-Louis Bossart [mailto:pierre-louis.bossart@linux.intel.com]
> >Sent: Saturday, December 2, 2017 12:57 AM
> >To: Ughreja, Rakesh A <rakesh.a.ughreja@intel.com>; alsa-devel@alsa-
> >project.org; broonie@kernel.org; tiwai@suse.de; liam.r.girdwood@linux.intel.com
> >Cc: Koul, Vinod <vinod.koul@intel.com>; Patches Audio
> ><patches.audio@intel.com>
> >Subject: Re: [alsa-devel] [RFC 05/10] ALSA: hda - make some of the functions
> >externally visible
> >
> >On 12/1/17 3:14 AM, Rakesh Ughreja wrote:
> >> Mark some functions with EXPORT_SYMBOL_GPL so that it can be called by
> >> other kernel modules. These APIs would be called by ASoC based HDA codec
> >> driver which will be added in the later patches.
> >
> >Looks ok. Just wondering from a code maintenance perspective if those
> >functions would need to be part of a library, and if this means that to
> >compile the ASoC based HDAudio codec support you still need to enable
> >the legacy support - and if this leads to potential conflicts on which
> >driver gets probed?
> 
> Yes, I think these functions are already part of library.
> These are part of file hda_codec.c, which looks like a library to me.
> 
> Takashi is that correct understanding ?

Yes, it's a library module.


Takashi

Re: [RFC 02/10] ASoC: Intel: Skylake: Add entry in sst_acpi_mach for HDA codecs

[Ughreja, Rakesh A]

>-----Original Message-----
>From: Pierre-Louis Bossart [mailto:pierre-louis.bossart@linux.intel.com]
>Sent: Friday, December 1, 2017 11:46 PM
>To: Ughreja, Rakesh A <rakesh.a.ughreja@intel.com>; alsa-devel@alsa-
>project.org; broonie@kernel.org; tiwai@suse.de; liam.r.girdwood@linux.intel.com
>Cc: Koul, Vinod <vinod.koul@intel.com>; Patches Audio
><patches.audio@intel.com>
>Subject: Re: [alsa-devel] [RFC 02/10] ASoC: Intel: Skylake: Add entry in
>sst_acpi_mach for HDA codecs
>
>On 12/1/17 3:14 AM, Rakesh Ughreja wrote:
>> When no I2S based codecs are detected in the BIOS, check if there are
>> any HDA codecs present. If yes, load the corresponding machine driver.
>
>Is this completely correct?
>I may be splitting hair but I believe it's possible to have an HDAudio
>codec connected to some pins and an I2S/PDM device on non-conflicting
>pins. The PDM case is very real and isn't related to I2S

Yes, agree. In that case there should be different Machine driver.
I expected the skl_hda_generic machine driver to be loaded only when
we have iDisp and HDA codecs and nothing else.

>
>You may also want to explain that you *have* to look for non-HDaudio
>stuff first, because otherwise the iDisp codec will be detected but that
>is not sufficient to load the HDaudio machine driver (could be
>iDisp+I2S+PDM)

Yes, I have done the same thing in this patch. You can see that the
HDA_GEN entry is the last one.

>
>>
>> TODO:
>> support for detecting the presence of HDA codec is not implemented.
>> it will be implemented in the next revision.
>>
>> Signed-off-by: Rakesh Ughreja <rakesh.a.ughreja@intel.com>
>> ---
>>   sound/soc/intel/skylake/skl.c | 46
>++++++++++++++++++++++++++++++++++++++++---
>>   1 file changed, 43 insertions(+), 3 deletions(-)
>>
>> diff --git a/sound/soc/intel/skylake/skl.c b/sound/soc/intel/skylake/skl.c
>> index 27462c0..568a285 100644
>> --- a/sound/soc/intel/skylake/skl.c
>> +++ b/sound/soc/intel/skylake/skl.c
>> @@ -424,6 +424,26 @@ static int skl_free(struct hdac_bus *bus)
>>   	return 0;
>>   }
>>
>> +static struct snd_soc_acpi_mach *skl_probe_hda_machine(
>> +					struct snd_soc_acpi_mach *machines)
>> +{
>> +
>> +	struct snd_soc_acpi_mach *mach;
>> +
>> +	/*
>> +	 * FIXME:
>> +	 * First check if there are any HDA codecs present on the system
>> +	 * then search the match table.
>> +	 * For now this function is not detecting the presence of any
>> +	 * HDA codecs.
>
>D'oh. so how was this patchset tested then?

I have tested this patchset on Dell XPS13 machine, where I have
only one HDA codec and one iDisp codec. The current code
does not find the match for any of the I2S codec machine
driver and so the default HDA_GEN machine driver gets loaded.

Once the FIXME is implemented HDA_GEN machine driver will
be loaded only if there is a HDA and iDisp codec present, else
no machine driver will be loaded.

>
>> +	 */
>> +	for (mach = machines; mach->id[0]; mach++) {
>> +		if (!strcmp(mach->id, "HDA_GEN"))
>
>Do you mean to say this was a temporary hack assuming a modified DSDT to
>force the detection of the codec?

There is a FIXME on the top, where I would like to detect if we
really have only HDA and iDisp codecs present on the system.

>
>And while I am at it in the hair-splitting department, in theory HDAudio
>can work with multiple codecs (each using its own SDI line), did you
>think of this case and how would it impact the machine driver selection?

As long as we have only HDA and iDisp codecs, the current machine
driver can scale. You will need more BE DAI Links in the machine driver
to bind the codecs.

Re: [RFC 03/10] ASoC: Intel: Skylake: add HDA BE DAIs

[Pierre-Louis Bossart]

On 12/4/17 10:14 AM, Ughreja, Rakesh A wrote:
> 
> 
>> -----Original Message-----
>> From: Pierre-Louis Bossart [mailto:pierre-louis.bossart@linux.intel.com]
>> Sent: Friday, December 1, 2017 11:50 PM
>> To: Ughreja, Rakesh A <rakesh.a.ughreja@intel.com>; alsa-devel@alsa-
>> project.org; broonie@kernel.org; tiwai@suse.de; liam.r.girdwood@linux.intel.com
>> Cc: Koul, Vinod <vinod.koul@intel.com>; Patches Audio
>> <patches.audio@intel.com>
>> Subject: Re: [alsa-devel] [RFC 03/10] ASoC: Intel: Skylake: add HDA BE DAIs
>>
>> On 12/1/17 3:14 AM, Rakesh Ughreja wrote:
>>> Add support for HDA BE DAIs in SKL platform driver.
>>>
>>> Signed-off-by: Rakesh Ughreja <rakesh.a.ughreja@intel.com>
>>> ---
>>>    sound/soc/intel/skylake/skl-pcm.c | 32 +++++++++++++++++++++++++------
>> -
>>>    1 file changed, 25 insertions(+), 7 deletions(-)
>>>
>>> diff --git a/sound/soc/intel/skylake/skl-pcm.c b/sound/soc/intel/skylake/skl-
>> pcm.c
>>> index e6fee7a..3dcb241 100644
>>> --- a/sound/soc/intel/skylake/skl-pcm.c
>>> +++ b/sound/soc/intel/skylake/skl-pcm.c
>>> @@ -956,21 +956,39 @@ static struct snd_soc_dai_driver skl_platform_dai[] = {
>>>    	},
>>>    },
>>>    {
>>> -	.name = "HD-Codec Pin",
>>> +	.name = "Analog CPU DAI",
>>>    	.ops = &skl_link_dai_ops,
>>>    	.playback = {
>>> -		.stream_name = "HD-Codec Tx",
>>> -		.channels_min = HDA_STEREO,
>>> +		.stream_name = "Analog CPU Playback",
>>> +		.channels_min = HDA_MONO,
>>>    		.channels_max = HDA_STEREO,
>>>    		.rates = SNDRV_PCM_RATE_48000,
>>> -		.formats = SNDRV_PCM_FMTBIT_S16_LE,
>>> +		.formats = SNDRV_PCM_FMTBIT_S16_LE |
>> SNDRV_PCM_FMTBIT_S24_LE,
>>>    	},
>>>    	.capture = {
>>> -		.stream_name = "HD-Codec Rx",
>>> -		.channels_min = HDA_STEREO,
>>> +		.stream_name = "Analog CPU Capture",
>>> +		.channels_min = HDA_MONO,
>>>    		.channels_max = HDA_STEREO,
>>>    		.rates = SNDRV_PCM_RATE_48000,
>>> -		.formats = SNDRV_PCM_FMTBIT_S16_LE,
>>> +		.formats = SNDRV_PCM_FMTBIT_S16_LE |
>> SNDRV_PCM_FMTBIT_S24_LE,
>>> +	},
>>> +},
>>> +{
>>> +	.name = "Digital CPU DAI",
>>
>> I am not sure I get the meaning of analog/digital.
>>
>> Did you mean external codec vs. iDisp codec?
>>
>> Or did you really mean analog? I don't think you can control this front
>> the CPU side, e.g. if the codec has an S/PDIF output it remains digital.
> 
> I was trying to map the BE CPU DAIs to hda_pcm_stream.
> The way it is defined here.
> https://elixir.free-electrons.com/linux/v4.15-rc2/source/sound/pci/hda/hda_generic.c#L5374

ok, but I still don't know what those BEs are supposed to refer to or mean.

> 
>>
>>> +	.ops = &skl_link_dai_ops,
>>> +	.playback = {
>>> +		.stream_name = "Digital CPU Playback",
>>> +		.channels_min = HDA_MONO,
>>> +		.channels_max = HDA_STEREO,
>>> +		.rates = SNDRV_PCM_RATE_48000,
>>> +		.formats = SNDRV_PCM_FMTBIT_S16_LE |
>> SNDRV_PCM_FMTBIT_S24_LE,
>>> +	},
>>> +	.capture = {
>>> +		.stream_name = "Digital CPU Capture",
>>> +		.channels_min = HDA_MONO,
>>> +		.channels_max = HDA_STEREO,
>>> +		.rates = SNDRV_PCM_RATE_48000,
>>> +		.formats = SNDRV_PCM_FMTBIT_S16_LE |
>> SNDRV_PCM_FMTBIT_S24_LE,
>>>    	},
>>>    },
>>>    };
>>>
> 

Re: [RFC 03/10] ASoC: Intel: Skylake: add HDA BE DAIs

[Ughreja, Rakesh A]

>-----Original Message-----
>From: Pierre-Louis Bossart [mailto:pierre-louis.bossart@linux.intel.com]
>Sent: Monday, December 4, 2017 10:11 PM
>To: Ughreja, Rakesh A <rakesh.a.ughreja@intel.com>; alsa-devel@alsa-
>project.org; broonie@kernel.org; tiwai@suse.de; liam.r.girdwood@linux.intel.com
>Cc: Koul, Vinod <vinod.koul@intel.com>; Patches Audio
><patches.audio@intel.com>
>Subject: Re: [alsa-devel] [RFC 03/10] ASoC: Intel: Skylake: add HDA BE DAIs
>
>On 12/4/17 10:14 AM, Ughreja, Rakesh A wrote:
>>
>>
>>> -----Original Message-----
>>> From: Pierre-Louis Bossart [mailto:pierre-louis.bossart@linux.intel.com]
>>> Sent: Friday, December 1, 2017 11:50 PM
>>> To: Ughreja, Rakesh A <rakesh.a.ughreja@intel.com>; alsa-devel@alsa-
>>> project.org; broonie@kernel.org; tiwai@suse.de;
>liam.r.girdwood@linux.intel.com
>>> Cc: Koul, Vinod <vinod.koul@intel.com>; Patches Audio
>>> <patches.audio@intel.com>
>>> Subject: Re: [alsa-devel] [RFC 03/10] ASoC: Intel: Skylake: add HDA BE DAIs
>>>
>>> On 12/1/17 3:14 AM, Rakesh Ughreja wrote:
>>>> Add support for HDA BE DAIs in SKL platform driver.
>>>>
>>>> Signed-off-by: Rakesh Ughreja <rakesh.a.ughreja@intel.com>
>>>> ---
>>>>    sound/soc/intel/skylake/skl-pcm.c | 32 +++++++++++++++++++++++++--
>----
>>> -
>>>>    1 file changed, 25 insertions(+), 7 deletions(-)
>>>>
>>>> diff --git a/sound/soc/intel/skylake/skl-pcm.c b/sound/soc/intel/skylake/skl-
>>> pcm.c
>>>> index e6fee7a..3dcb241 100644
>>>> --- a/sound/soc/intel/skylake/skl-pcm.c
>>>> +++ b/sound/soc/intel/skylake/skl-pcm.c
>>>> @@ -956,21 +956,39 @@ static struct snd_soc_dai_driver skl_platform_dai[]
>= {
>>>>    	},
>>>>    },
>>>>    {
>>>> -	.name = "HD-Codec Pin",
>>>> +	.name = "Analog CPU DAI",
>>>>    	.ops = &skl_link_dai_ops,
>>>>    	.playback = {
>>>> -		.stream_name = "HD-Codec Tx",
>>>> -		.channels_min = HDA_STEREO,
>>>> +		.stream_name = "Analog CPU Playback",
>>>> +		.channels_min = HDA_MONO,
>>>>    		.channels_max = HDA_STEREO,
>>>>    		.rates = SNDRV_PCM_RATE_48000,
>>>> -		.formats = SNDRV_PCM_FMTBIT_S16_LE,
>>>> +		.formats = SNDRV_PCM_FMTBIT_S16_LE |
>>> SNDRV_PCM_FMTBIT_S24_LE,
>>>>    	},
>>>>    	.capture = {
>>>> -		.stream_name = "HD-Codec Rx",
>>>> -		.channels_min = HDA_STEREO,
>>>> +		.stream_name = "Analog CPU Capture",
>>>> +		.channels_min = HDA_MONO,
>>>>    		.channels_max = HDA_STEREO,
>>>>    		.rates = SNDRV_PCM_RATE_48000,
>>>> -		.formats = SNDRV_PCM_FMTBIT_S16_LE,
>>>> +		.formats = SNDRV_PCM_FMTBIT_S16_LE |
>>> SNDRV_PCM_FMTBIT_S24_LE,
>>>> +	},
>>>> +},
>>>> +{
>>>> +	.name = "Digital CPU DAI",
>>>
>>> I am not sure I get the meaning of analog/digital.
>>>
>>> Did you mean external codec vs. iDisp codec?
>>>
>>> Or did you really mean analog? I don't think you can control this front
>>> the CPU side, e.g. if the codec has an S/PDIF output it remains digital.
>>
>> I was trying to map the BE CPU DAIs to hda_pcm_stream.
>> The way it is defined here.
>> https://elixir.free-electrons.com/linux/v4.15-
>rc2/source/sound/pci/hda/hda_generic.c#L5374
>
>ok, but I still don't know what those BEs are supposed to refer to or mean.

In the ASoC HDA codec driver it maps the hda_pcm_stream to codc DAI.
In the machine driver we map/bind the codec DAI to CPU BE DAI.

Re: [RFC 03/10] ASoC: Intel: Skylake: add HDA BE DAIs

[Pierre-Louis Bossart]

On 12/4/17 10:44 AM, Ughreja, Rakesh A wrote:
> 
> 
>> -----Original Message-----
>> From: Pierre-Louis Bossart [mailto:pierre-louis.bossart@linux.intel.com]
>> Sent: Monday, December 4, 2017 10:11 PM
>> To: Ughreja, Rakesh A <rakesh.a.ughreja@intel.com>; alsa-devel@alsa-
>> project.org; broonie@kernel.org; tiwai@suse.de; liam.r.girdwood@linux.intel.com
>> Cc: Koul, Vinod <vinod.koul@intel.com>; Patches Audio
>> <patches.audio@intel.com>
>> Subject: Re: [alsa-devel] [RFC 03/10] ASoC: Intel: Skylake: add HDA BE DAIs
>>
>> On 12/4/17 10:14 AM, Ughreja, Rakesh A wrote:
>>>
>>>
>>>> -----Original Message-----
>>>> From: Pierre-Louis Bossart [mailto:pierre-louis.bossart@linux.intel.com]
>>>> Sent: Friday, December 1, 2017 11:50 PM
>>>> To: Ughreja, Rakesh A <rakesh.a.ughreja@intel.com>; alsa-devel@alsa-
>>>> project.org; broonie@kernel.org; tiwai@suse.de;
>> liam.r.girdwood@linux.intel.com
>>>> Cc: Koul, Vinod <vinod.koul@intel.com>; Patches Audio
>>>> <patches.audio@intel.com>
>>>> Subject: Re: [alsa-devel] [RFC 03/10] ASoC: Intel: Skylake: add HDA BE DAIs
>>>>
>>>> On 12/1/17 3:14 AM, Rakesh Ughreja wrote:
>>>>> Add support for HDA BE DAIs in SKL platform driver.
>>>>>
>>>>> Signed-off-by: Rakesh Ughreja <rakesh.a.ughreja@intel.com>
>>>>> ---
>>>>>     sound/soc/intel/skylake/skl-pcm.c | 32 +++++++++++++++++++++++++--
>> ----
>>>> -
>>>>>     1 file changed, 25 insertions(+), 7 deletions(-)
>>>>>
>>>>> diff --git a/sound/soc/intel/skylake/skl-pcm.c b/sound/soc/intel/skylake/skl-
>>>> pcm.c
>>>>> index e6fee7a..3dcb241 100644
>>>>> --- a/sound/soc/intel/skylake/skl-pcm.c
>>>>> +++ b/sound/soc/intel/skylake/skl-pcm.c
>>>>> @@ -956,21 +956,39 @@ static struct snd_soc_dai_driver skl_platform_dai[]
>> = {
>>>>>     	},
>>>>>     },
>>>>>     {
>>>>> -	.name = "HD-Codec Pin",
>>>>> +	.name = "Analog CPU DAI",
>>>>>     	.ops = &skl_link_dai_ops,
>>>>>     	.playback = {
>>>>> -		.stream_name = "HD-Codec Tx",
>>>>> -		.channels_min = HDA_STEREO,
>>>>> +		.stream_name = "Analog CPU Playback",
>>>>> +		.channels_min = HDA_MONO,
>>>>>     		.channels_max = HDA_STEREO,
>>>>>     		.rates = SNDRV_PCM_RATE_48000,
>>>>> -		.formats = SNDRV_PCM_FMTBIT_S16_LE,
>>>>> +		.formats = SNDRV_PCM_FMTBIT_S16_LE |
>>>> SNDRV_PCM_FMTBIT_S24_LE,
>>>>>     	},
>>>>>     	.capture = {
>>>>> -		.stream_name = "HD-Codec Rx",
>>>>> -		.channels_min = HDA_STEREO,
>>>>> +		.stream_name = "Analog CPU Capture",
>>>>> +		.channels_min = HDA_MONO,
>>>>>     		.channels_max = HDA_STEREO,
>>>>>     		.rates = SNDRV_PCM_RATE_48000,
>>>>> -		.formats = SNDRV_PCM_FMTBIT_S16_LE,
>>>>> +		.formats = SNDRV_PCM_FMTBIT_S16_LE |
>>>> SNDRV_PCM_FMTBIT_S24_LE,
>>>>> +	},
>>>>> +},
>>>>> +{
>>>>> +	.name = "Digital CPU DAI",
>>>>
>>>> I am not sure I get the meaning of analog/digital.
>>>>
>>>> Did you mean external codec vs. iDisp codec?
>>>>
>>>> Or did you really mean analog? I don't think you can control this front
>>>> the CPU side, e.g. if the codec has an S/PDIF output it remains digital.
>>>
>>> I was trying to map the BE CPU DAIs to hda_pcm_stream.
>>> The way it is defined here.
>>> https://elixir.free-electrons.com/linux/v4.15-
>> rc2/source/sound/pci/hda/hda_generic.c#L5374
>>
>> ok, but I still don't know what those BEs are supposed to refer to or mean.
> 
> In the ASoC HDA codec driver it maps the hda_pcm_stream to codc DAI.
> In the machine driver we map/bind the codec DAI to CPU BE DAI.

I meant the difference between pcm_analog_playback, 
pcm_analog_alt_playback and pcm_digital_playback in the hda_generic.c 
file you mentioned.

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

Re: [RFC 03/10] ASoC: Intel: Skylake: add HDA BE DAIs

[Takashi Iwai]

On Mon, 04 Dec 2017 17:51:17 +0100,
Pierre-Louis Bossart wrote:
> 
> On 12/4/17 10:44 AM, Ughreja, Rakesh A wrote:
> >
> >
> >> -----Original Message-----
> >> From: Pierre-Louis Bossart [mailto:pierre-louis.bossart@linux.intel.com]
> >> Sent: Monday, December 4, 2017 10:11 PM
> >> To: Ughreja, Rakesh A <rakesh.a.ughreja@intel.com>; alsa-devel@alsa-
> >> project.org; broonie@kernel.org; tiwai@suse.de; liam.r.girdwood@linux.intel.com
> >> Cc: Koul, Vinod <vinod.koul@intel.com>; Patches Audio
> >> <patches.audio@intel.com>
> >> Subject: Re: [alsa-devel] [RFC 03/10] ASoC: Intel: Skylake: add HDA BE DAIs
> >>
> >> On 12/4/17 10:14 AM, Ughreja, Rakesh A wrote:
> >>>
> >>>
> >>>> -----Original Message-----
> >>>> From: Pierre-Louis Bossart [mailto:pierre-louis.bossart@linux.intel.com]
> >>>> Sent: Friday, December 1, 2017 11:50 PM
> >>>> To: Ughreja, Rakesh A <rakesh.a.ughreja@intel.com>; alsa-devel@alsa-
> >>>> project.org; broonie@kernel.org; tiwai@suse.de;
> >> liam.r.girdwood@linux.intel.com
> >>>> Cc: Koul, Vinod <vinod.koul@intel.com>; Patches Audio
> >>>> <patches.audio@intel.com>
> >>>> Subject: Re: [alsa-devel] [RFC 03/10] ASoC: Intel: Skylake: add HDA BE DAIs
> >>>>
> >>>> On 12/1/17 3:14 AM, Rakesh Ughreja wrote:
> >>>>> Add support for HDA BE DAIs in SKL platform driver.
> >>>>>
> >>>>> Signed-off-by: Rakesh Ughreja <rakesh.a.ughreja@intel.com>
> >>>>> ---
> >>>>>     sound/soc/intel/skylake/skl-pcm.c | 32 +++++++++++++++++++++++++--
> >> ----
> >>>> -
> >>>>>     1 file changed, 25 insertions(+), 7 deletions(-)
> >>>>>
> >>>>> diff --git a/sound/soc/intel/skylake/skl-pcm.c b/sound/soc/intel/skylake/skl-
> >>>> pcm.c
> >>>>> index e6fee7a..3dcb241 100644
> >>>>> --- a/sound/soc/intel/skylake/skl-pcm.c
> >>>>> +++ b/sound/soc/intel/skylake/skl-pcm.c
> >>>>> @@ -956,21 +956,39 @@ static struct snd_soc_dai_driver skl_platform_dai[]
> >> = {
> >>>>>     	},
> >>>>>     },
> >>>>>     {
> >>>>> -	.name = "HD-Codec Pin",
> >>>>> +	.name = "Analog CPU DAI",
> >>>>>     	.ops = &skl_link_dai_ops,
> >>>>>     	.playback = {
> >>>>> -		.stream_name = "HD-Codec Tx",
> >>>>> -		.channels_min = HDA_STEREO,
> >>>>> +		.stream_name = "Analog CPU Playback",
> >>>>> +		.channels_min = HDA_MONO,
> >>>>>     		.channels_max = HDA_STEREO,
> >>>>>     		.rates = SNDRV_PCM_RATE_48000,
> >>>>> -		.formats = SNDRV_PCM_FMTBIT_S16_LE,
> >>>>> +		.formats = SNDRV_PCM_FMTBIT_S16_LE |
> >>>> SNDRV_PCM_FMTBIT_S24_LE,
> >>>>>     	},
> >>>>>     	.capture = {
> >>>>> -		.stream_name = "HD-Codec Rx",
> >>>>> -		.channels_min = HDA_STEREO,
> >>>>> +		.stream_name = "Analog CPU Capture",
> >>>>> +		.channels_min = HDA_MONO,
> >>>>>     		.channels_max = HDA_STEREO,
> >>>>>     		.rates = SNDRV_PCM_RATE_48000,
> >>>>> -		.formats = SNDRV_PCM_FMTBIT_S16_LE,
> >>>>> +		.formats = SNDRV_PCM_FMTBIT_S16_LE |
> >>>> SNDRV_PCM_FMTBIT_S24_LE,
> >>>>> +	},
> >>>>> +},
> >>>>> +{
> >>>>> +	.name = "Digital CPU DAI",
> >>>>
> >>>> I am not sure I get the meaning of analog/digital.
> >>>>
> >>>> Did you mean external codec vs. iDisp codec?
> >>>>
> >>>> Or did you really mean analog? I don't think you can control this front
> >>>> the CPU side, e.g. if the codec has an S/PDIF output it remains digital.
> >>>
> >>> I was trying to map the BE CPU DAIs to hda_pcm_stream.
> >>> The way it is defined here.
> >>> https://elixir.free-electrons.com/linux/v4.15-
> >> rc2/source/sound/pci/hda/hda_generic.c#L5374
> >>
> >> ok, but I still don't know what those BEs are supposed to refer to or mean.
> >
> > In the ASoC HDA codec driver it maps the hda_pcm_stream to codc DAI.
> > In the machine driver we map/bind the codec DAI to CPU BE DAI.
> 
> I meant the difference between pcm_analog_playback,
> pcm_analog_alt_playback and pcm_digital_playback in the hda_generic.c
> file you mentioned.

pcm_analog_playback is for the primary PCM streams that are assigned
to the analog I/O jacks (hp, mic, line-out, etc).  pcm_analog_alt is
for the alternative PCM stream, usually assigned for the independent
headphone PCM stream on certain codec chips.  pcm_digital_playback is
for either SPDIF or HDMI/DP, but mostly only for SPDIF, as HDMI/DP is
usually not dealt by generic driver but by HDMI codec driver.


Takashi

Re: [RFC 00/10] Enable HDA Codec support on Intel Platforms (Series2)

[Vinod Koul]

On Mon, Dec 04, 2017 at 08:52:06AM -0600, Pierre-Louis Bossart wrote:
> On 12/3/17 10:21 PM, Vinod Koul wrote:
> >On Sun, Dec 03, 2017 at 09:44:56PM -0600, Pierre-Louis Bossart wrote:

> >>That gets complicated. you'd have
> >>1. DSP-based ASoC driver for SKL+ w/ DSP
> >>2. coupled-mode ASoC driver for SKL+ w/o DSP
> >>3. legacy for pre-SKL platforms.
> >>
> >>I don't see what simplification this brings, it's complicated enough with 1.
> >>and 3, and legacy is not going away.
> >
> >in long term we _need_ to converge. 1 and 2 should be same driver detecting
> >presence of DSP dynamically and doing DSP based or coupled mode.
> 
> Did we ever enable the coupled mode? if not then it's an hypothetical
> direction...

and prerequisite for that is HDA, so it can be done once this support is
merged!

-- 
~Vinod

Re: [RFC 00/10] Enable HDA Codec support on Intel Platforms (Series2)

[Vinod Koul]

On Fri, Dec 01, 2017 at 02:43:58PM +0530, Rakesh Ughreja wrote:
> Many Intel platforms (SKL,KBL) etc. in the market supports enahanced 
			  ^^

space aftre comma pls and /s/enahanced/enhanced

> audio capabilities which also includes DSP processing. This patch carry
> forwads the works that is done in the previous series to enable HD Audio

/s/forwads/forwards/
 

-- 
~Vinod

Re: [RFC 01/10] ASoC: Intel: Boards: Machine driver for Intel platforms

[Vinod Koul]

On Mon, Dec 04, 2017 at 09:37:37AM -0600, Pierre-Louis Bossart wrote:
> On 12/4/17 9:10 AM, Ughreja, Rakesh A wrote:

> >>>>>    sound/soc/intel/boards/Kconfig           |  10 ++
> >>>>>    sound/soc/intel/boards/Makefile          |   2 +
> >>>>>    sound/soc/intel/boards/skl_hda_generic.c | 276
> >>>>+++++++++++++++++++++++++++++++
> >>>>
> >>>>can we drop the Skylake reference? It's become a catch-all term to mean
> >>>>both the platform, the IP and the driver.
> >>>
> >>>Suggest some name. I have no problem.
> >>
> >>HiFi3 ?
> >>iDisp ?
> >>HDAudio-DSP ?
> >
> >hda_dsp_generic.c -- For the main file
> >hda_dsp_common.c -- for common functions
> >
> >Does it look fine ?
> 
> works for me.

Sorry not for me. hda_dsp_xxx doesnt tie it to anything. HDA and DSP are too
generic terms. But yes I don't have a better alternate than skl_generic.
Here this solution is tied to a very specfic IP which is present in SKL
onwards platforms..

Yes SKL is become an IP as well as platform. Maybe we should have a codename
for this like azx :)

-- 
~Vinod

Re: [RFC 01/10] ASoC: Intel: Boards: Machine driver for Intel platforms

[Ughreja, Rakesh A]

>-----Original Message-----
>From: Koul, Vinod
>Sent: Wednesday, December 6, 2017 9:48 PM
>To: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
>Cc: Ughreja, Rakesh A <rakesh.a.ughreja@intel.com>; alsa-devel@alsa-project.org;
>broonie@kernel.org; tiwai@suse.de; liam.r.girdwood@linux.intel.com; Patches
>Audio <patches.audio@intel.com>
>Subject: Re: [alsa-devel] [RFC 01/10] ASoC: Intel: Boards: Machine driver for Intel
>platforms
>
>On Mon, Dec 04, 2017 at 09:37:37AM -0600, Pierre-Louis Bossart wrote:
>> On 12/4/17 9:10 AM, Ughreja, Rakesh A wrote:
>
>> >>>>>    sound/soc/intel/boards/Kconfig           |  10 ++
>> >>>>>    sound/soc/intel/boards/Makefile          |   2 +
>> >>>>>    sound/soc/intel/boards/skl_hda_generic.c | 276
>> >>>>+++++++++++++++++++++++++++++++
>> >>>>
>> >>>>can we drop the Skylake reference? It's become a catch-all term to mean
>> >>>>both the platform, the IP and the driver.
>> >>>
>> >>>Suggest some name. I have no problem.
>> >>
>> >>HiFi3 ?
>> >>iDisp ?
>> >>HDAudio-DSP ?
>> >
>> >hda_dsp_generic.c -- For the main file
>> >hda_dsp_common.c -- for common functions
>> >
>> >Does it look fine ?
>>
>> works for me.
>
>Sorry not for me. hda_dsp_xxx doesnt tie it to anything. HDA and DSP are too
>generic terms. But yes I don't have a better alternate than skl_generic.
>Here this solution is tied to a very specfic IP which is present in SKL
>onwards platforms..
>
>Yes SKL is become an IP as well as platform. Maybe we should have a codename
>for this like azx :)

We do have a code name "sst".

So does this sound okay for you ?

sst_hda_dsp_generic.c -- For main file
sst_hda_dsp_common.c -- for common functions

Regards,
Rakesh

>
>--
>~Vinod

Re: [RFC 01/10] ASoC: Intel: Boards: Machine driver for Intel platforms

[Pierre-Louis Bossart]

On 12/7/17 6:27 AM, Ughreja, Rakesh A wrote:
> 
> 
>> -----Original Message-----
>> From: Koul, Vinod
>> Sent: Wednesday, December 6, 2017 9:48 PM
>> To: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
>> Cc: Ughreja, Rakesh A <rakesh.a.ughreja@intel.com>; alsa-devel@alsa-project.org;
>> broonie@kernel.org; tiwai@suse.de; liam.r.girdwood@linux.intel.com; Patches
>> Audio <patches.audio@intel.com>
>> Subject: Re: [alsa-devel] [RFC 01/10] ASoC: Intel: Boards: Machine driver for Intel
>> platforms
>>
>> On Mon, Dec 04, 2017 at 09:37:37AM -0600, Pierre-Louis Bossart wrote:
>>> On 12/4/17 9:10 AM, Ughreja, Rakesh A wrote:
>>
>>>>>>>>     sound/soc/intel/boards/Kconfig           |  10 ++
>>>>>>>>     sound/soc/intel/boards/Makefile          |   2 +
>>>>>>>>     sound/soc/intel/boards/skl_hda_generic.c | 276
>>>>>>> +++++++++++++++++++++++++++++++
>>>>>>>
>>>>>>> can we drop the Skylake reference? It's become a catch-all term to mean
>>>>>>> both the platform, the IP and the driver.
>>>>>>
>>>>>> Suggest some name. I have no problem.
>>>>>
>>>>> HiFi3 ?
>>>>> iDisp ?
>>>>> HDAudio-DSP ?
>>>>
>>>> hda_dsp_generic.c -- For the main file
>>>> hda_dsp_common.c -- for common functions
>>>>
>>>> Does it look fine ?
>>>
>>> works for me.
>>
>> Sorry not for me. hda_dsp_xxx doesnt tie it to anything. HDA and DSP are too
>> generic terms. But yes I don't have a better alternate than skl_generic.
>> Here this solution is tied to a very specfic IP which is present in SKL
>> onwards platforms..
>>
>> Yes SKL is become an IP as well as platform. Maybe we should have a codename
>> for this like azx :)
> 
> We do have a code name "sst".
> 
> So does this sound okay for you ?
> 
> sst_hda_dsp_generic.c -- For main file
> sst_hda_dsp_common.c -- for common functions

SST is also aliased to platform drivers with closed-source firmware, not 
a good idea for machine drivers.

Re: [RFC 01/10] ASoC: Intel: Boards: Machine driver for Intel platforms

[Ughreja, Rakesh A]

>-----Original Message-----
>From: Pierre-Louis Bossart [mailto:pierre-louis.bossart@linux.intel.com]
>Sent: Thursday, December 7, 2017 6:36 PM
>To: Ughreja, Rakesh A <rakesh.a.ughreja@intel.com>; Koul, Vinod
><vinod.koul@intel.com>
>Cc: alsa-devel@alsa-project.org; broonie@kernel.org; tiwai@suse.de;
>liam.r.girdwood@linux.intel.com; Patches Audio <patches.audio@intel.com>
>Subject: Re: [alsa-devel] [RFC 01/10] ASoC: Intel: Boards: Machine driver for Intel
>platforms
>
>On 12/7/17 6:27 AM, Ughreja, Rakesh A wrote:
>>
>>
>>> -----Original Message-----
>>> From: Koul, Vinod
>>> Sent: Wednesday, December 6, 2017 9:48 PM
>>> To: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
>>> Cc: Ughreja, Rakesh A <rakesh.a.ughreja@intel.com>; alsa-devel@alsa-
>project.org;
>>> broonie@kernel.org; tiwai@suse.de; liam.r.girdwood@linux.intel.com; Patches
>>> Audio <patches.audio@intel.com>
>>> Subject: Re: [alsa-devel] [RFC 01/10] ASoC: Intel: Boards: Machine driver for
>Intel
>>> platforms
>>>
>>> On Mon, Dec 04, 2017 at 09:37:37AM -0600, Pierre-Louis Bossart wrote:
>>>> On 12/4/17 9:10 AM, Ughreja, Rakesh A wrote:
>>>
>>>>>>>>>     sound/soc/intel/boards/Kconfig           |  10 ++
>>>>>>>>>     sound/soc/intel/boards/Makefile          |   2 +
>>>>>>>>>     sound/soc/intel/boards/skl_hda_generic.c | 276
>>>>>>>> +++++++++++++++++++++++++++++++
>>>>>>>>
>>>>>>>> can we drop the Skylake reference? It's become a catch-all term to
>mean
>>>>>>>> both the platform, the IP and the driver.
>>>>>>>
>>>>>>> Suggest some name. I have no problem.
>>>>>>
>>>>>> HiFi3 ?
>>>>>> iDisp ?
>>>>>> HDAudio-DSP ?
>>>>>
>>>>> hda_dsp_generic.c -- For the main file
>>>>> hda_dsp_common.c -- for common functions
>>>>>
>>>>> Does it look fine ?
>>>>
>>>> works for me.
>>>
>>> Sorry not for me. hda_dsp_xxx doesnt tie it to anything. HDA and DSP are too
>>> generic terms. But yes I don't have a better alternate than skl_generic.
>>> Here this solution is tied to a very specfic IP which is present in SKL
>>> onwards platforms..
>>>
>>> Yes SKL is become an IP as well as platform. Maybe we should have a
>codename
>>> for this like azx :)
>>
>> We do have a code name "sst".
>>
>> So does this sound okay for you ?
>>
>> sst_hda_dsp_generic.c -- For main file
>> sst_hda_dsp_common.c -- for common functions
>
>SST is also aliased to platform drivers with closed-source firmware, not
>a good idea for machine drivers.

Then I have to go back to skl_hda_dsp_generic.c and 
skl_hda_dsp_common.c as Vinod suggested.

Regards,
Rakesh

Re: [RFC 01/10] ASoC: Intel: Boards: Machine driver for Intel platforms

[Pierre-Louis Bossart]

On 12/7/17 9:21 AM, Ughreja, Rakesh A wrote:
> 
> 
>> -----Original Message-----
>> From: Pierre-Louis Bossart [mailto:pierre-louis.bossart@linux.intel.com]
>> Sent: Thursday, December 7, 2017 6:36 PM
>> To: Ughreja, Rakesh A <rakesh.a.ughreja@intel.com>; Koul, Vinod
>> <vinod.koul@intel.com>
>> Cc: alsa-devel@alsa-project.org; broonie@kernel.org; tiwai@suse.de;
>> liam.r.girdwood@linux.intel.com; Patches Audio <patches.audio@intel.com>
>> Subject: Re: [alsa-devel] [RFC 01/10] ASoC: Intel: Boards: Machine driver for Intel
>> platforms
>>
>> On 12/7/17 6:27 AM, Ughreja, Rakesh A wrote:
>>>
>>>
>>>> -----Original Message-----
>>>> From: Koul, Vinod
>>>> Sent: Wednesday, December 6, 2017 9:48 PM
>>>> To: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
>>>> Cc: Ughreja, Rakesh A <rakesh.a.ughreja@intel.com>; alsa-devel@alsa-
>> project.org;
>>>> broonie@kernel.org; tiwai@suse.de; liam.r.girdwood@linux.intel.com; Patches
>>>> Audio <patches.audio@intel.com>
>>>> Subject: Re: [alsa-devel] [RFC 01/10] ASoC: Intel: Boards: Machine driver for
>> Intel
>>>> platforms
>>>>
>>>> On Mon, Dec 04, 2017 at 09:37:37AM -0600, Pierre-Louis Bossart wrote:
>>>>> On 12/4/17 9:10 AM, Ughreja, Rakesh A wrote:
>>>>
>>>>>>>>>>      sound/soc/intel/boards/Kconfig           |  10 ++
>>>>>>>>>>      sound/soc/intel/boards/Makefile          |   2 +
>>>>>>>>>>      sound/soc/intel/boards/skl_hda_generic.c | 276
>>>>>>>>> +++++++++++++++++++++++++++++++
>>>>>>>>>
>>>>>>>>> can we drop the Skylake reference? It's become a catch-all term to
>> mean
>>>>>>>>> both the platform, the IP and the driver.
>>>>>>>>
>>>>>>>> Suggest some name. I have no problem.
>>>>>>>
>>>>>>> HiFi3 ?
>>>>>>> iDisp ?
>>>>>>> HDAudio-DSP ?
>>>>>>
>>>>>> hda_dsp_generic.c -- For the main file
>>>>>> hda_dsp_common.c -- for common functions
>>>>>>
>>>>>> Does it look fine ?
>>>>>
>>>>> works for me.
>>>>
>>>> Sorry not for me. hda_dsp_xxx doesnt tie it to anything. HDA and DSP are too
>>>> generic terms. But yes I don't have a better alternate than skl_generic.
>>>> Here this solution is tied to a very specfic IP which is present in SKL
>>>> onwards platforms..
>>>>
>>>> Yes SKL is become an IP as well as platform. Maybe we should have a
>> codename
>>>> for this like azx :)
>>>
>>> We do have a code name "sst".
>>>
>>> So does this sound okay for you ?
>>>
>>> sst_hda_dsp_generic.c -- For main file
>>> sst_hda_dsp_common.c -- for common functions
>>
>> SST is also aliased to platform drivers with closed-source firmware, not
>> a good idea for machine drivers.
> 
> Then I have to go back to skl_hda_dsp_generic.c and
> skl_hda_dsp_common.c as Vinod suggested.

Fine for now.
you will have to do this work of allowing for more granularity in the 
'SKL' platform driver in the near future, to e.g. use it for the Skylake 
and Kabylake platforms and not for APL, CNL and followups, so we'll have 
to have this naming context again for the second coat of paint on the 
bikeshed.