diff mbox

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

Message ID 1512119648-2700-2-git-send-email-rakesh.a.ughreja@intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Ughreja, Rakesh A Dec. 1, 2017, 9:13 a.m. UTC
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

Comments

Pierre-Louis Bossart Dec. 1, 2017, 5:58 p.m. UTC | #1
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");
>
Ughreja, Rakesh A Dec. 4, 2017, 10:55 a.m. UTC | #2
>-----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.
Pierre-Louis Bossart Dec. 4, 2017, 2:49 p.m. UTC | #3
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.
>
Ughreja, Rakesh A Dec. 4, 2017, 3:10 p.m. UTC | #4
>-----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.
Pierre-Louis Bossart Dec. 4, 2017, 3:37 p.m. UTC | #5
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.
> 
>
Vinod Koul Dec. 6, 2017, 4:17 p.m. UTC | #6
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 :)
Ughreja, Rakesh A Dec. 7, 2017, 12:27 p.m. UTC | #7
>-----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
Pierre-Louis Bossart Dec. 7, 2017, 1:05 p.m. UTC | #8
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.
Ughreja, Rakesh A Dec. 7, 2017, 3:21 p.m. UTC | #9
>-----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
Pierre-Louis Bossart Dec. 7, 2017, 4:33 p.m. UTC | #10
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.
diff mbox

Patch

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");