diff mbox

[v1,1/9] ASoC: Intel: Boards: Machine driver for Intel platforms

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

Commit Message

Ughreja, Rakesh A Feb. 23, 2018, 8:12 a.m. UTC
Add machine driver for Intel platforms (SKL/KBL/BXT) 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. 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_dsp_common.c  | 107 +++++++++++++++++++++++++
 sound/soc/intel/boards/skl_hda_dsp_common.h  |  34 ++++++++
 sound/soc/intel/boards/skl_hda_dsp_generic.c | 114 +++++++++++++++++++++++++++
 sound/soc/intel/skylake/skl.h                |   1 +
 6 files changed, 268 insertions(+)
 create mode 100644 sound/soc/intel/boards/skl_hda_dsp_common.c
 create mode 100644 sound/soc/intel/boards/skl_hda_dsp_common.h
 create mode 100644 sound/soc/intel/boards/skl_hda_dsp_generic.c

Comments

Pierre-Louis Bossart Feb. 23, 2018, 4:33 p.m. UTC | #1
On 2/23/18 2:12 AM, Rakesh Ughreja wrote:
> Add machine driver for Intel platforms (SKL/KBL/BXT) 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. 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_dsp_common.c  | 107 +++++++++++++++++++++++++
>   sound/soc/intel/boards/skl_hda_dsp_common.h  |  34 ++++++++
>   sound/soc/intel/boards/skl_hda_dsp_generic.c | 114 +++++++++++++++++++++++++++
>   sound/soc/intel/skylake/skl.h                |   1 +
>   6 files changed, 268 insertions(+)
>   create mode 100644 sound/soc/intel/boards/skl_hda_dsp_common.c
>   create mode 100644 sound/soc/intel/boards/skl_hda_dsp_common.h
>   create mode 100644 sound/soc/intel/boards/skl_hda_dsp_generic.c
> 
> diff --git a/sound/soc/intel/boards/Kconfig b/sound/soc/intel/boards/Kconfig
> index eedc83b..3b20601 100644
> --- a/sound/soc/intel/boards/Kconfig
> +++ b/sound/soc/intel/boards/Kconfig
> @@ -268,6 +268,16 @@ config SND_SOC_INTEL_KBL_DA7219_MAX98357A_MACH
>   	  This adds support for ASoC Onboard Codec I2S machine driver. This will
>   	  create an alsa sound card for DA7219 + MAX98357A I2S audio codec.
>   	  Say Y if you have such a device.
> +
> +config SND_SOC_INTEL_SKL_HDA_DSP_GENERIC_MACH
> +	tristate "SKL/KBL/BXT with HDA Codecs"
> +	select SND_SOC_INTEL_SST
> +	depends on SND_SOC_INTEL_SKYLAKE

There two lines are no longer necessary (done at the platform level for 
SST and you already in an if SKYLAKE block

> +	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.

does this handle idisp as well? If yes, does this option conflict with 
the enablement of the other machine drivers which use the hdac_hdmi codec?


> +          Say Y or m if you have such a device. This is a recommended option.
>   	  If unsure select "N".
>   
>   endif ## SND_SOC_INTEL_SKYLAKE
> diff --git a/sound/soc/intel/boards/Makefile b/sound/soc/intel/boards/Makefile
> index 6fae506..5d65481 100644
> --- a/sound/soc/intel/boards/Makefile
> +++ b/sound/soc/intel/boards/Makefile
> @@ -18,6 +18,7 @@ snd-soc-kbl_da7219_max98357a-objs := kbl_da7219_max98357a.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_dsp-objs := skl_hda_dsp_generic.o skl_hda_dsp_common.o
>   snd-skl_nau88l25_max98357a-objs := skl_nau88l25_max98357a.o
>   snd-soc-skl_nau88l25_ssm4567-objs := skl_nau88l25_ssm4567.o
>   
> @@ -42,3 +43,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_DSP_GENERIC_MACH) += snd-soc-skl_hda_dsp.o
> diff --git a/sound/soc/intel/boards/skl_hda_dsp_common.c b/sound/soc/intel/boards/skl_hda_dsp_common.c
> new file mode 100644
> index 0000000..7066bed
> --- /dev/null
> +++ b/sound/soc/intel/boards/skl_hda_dsp_common.c
> @@ -0,0 +1,107 @@
> +// SPDX-License-Identifier: GPL-2.0
> +// Copyright(c) 2015-17 Intel Corporation.
> +
> +/*
> + * Common functions used in different Intel machine drivers
> + */
> +#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"
> +#include "skl_hda_dsp_common.h"
> +
> +#define NAME_SIZE	32
> +
> +int skl_hda_hdmi_add_pcm(struct snd_soc_card *card, int device)
> +{
> +	char dai_name[NAME_SIZE];
> +	struct skl_hda_private *ctx = snd_soc_card_get_drvdata(card);
> +	struct skl_hda_hdmi_pcm *pcm;
> +	static int i = 1;	/* hdmi codec dai name starts from index 1 */
> +
> +	pcm = devm_kzalloc(card->dev, sizeof(*pcm), GFP_KERNEL);
> +	if (!pcm)
> +		return -ENOMEM;
> +
> +	snprintf(dai_name, sizeof(dai_name), "intel-hdmi-hifi%d", i++);
> +	pcm->codec_dai = snd_soc_card_get_codec_dai(card, dai_name);
> +	if (!pcm->codec_dai)
> +		return -EINVAL;
> +
> +	pcm->device = device;
> +	list_add_tail(&pcm->head, &ctx->hdmi_pcm_list);
> +
> +	return 0;
> +}
> +
> +/* skl_hda_digital audio interface glue - connects codec <--> CPU */
> +struct snd_soc_dai_link skl_hda_be_dai_links[HDA_DSP_MAX_BE_DAI_LINKS] = {
> +
> +	/* 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",

what happens if you run on a device with a different ID, e.g. APL or GLK?

> +		.dpcm_playback = 1,
> +		.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",
> +		.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",
> +		.dpcm_playback = 1,
> +		.no_pcm = 1,
> +	},
> +};
> +
> +int skl_hda_hdmi_jack_init(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_component *component = NULL;
> +	int err;
> +	char jack_name[NAME_SIZE];
> +
> +	list_for_each_entry(pcm, &ctx->hdmi_pcm_list, head) {
> +		component = pcm->codec_dai->component;
> +		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, &pcm->hdmi_jack,
> +					NULL, 0);
> +
> +		if (err)
> +			return err;
> +
> +		err = hdac_hdmi_jack_init(pcm->codec_dai, pcm->device,
> +						&pcm->hdmi_jack);
> +		if (err < 0)
> +			return err;
> +	}
> +
> +	if (!component)
> +		return -EINVAL;
> +
> +	return hdac_hdmi_jack_port_init(component, &card->dapm);
> +}
> diff --git a/sound/soc/intel/boards/skl_hda_dsp_common.h b/sound/soc/intel/boards/skl_hda_dsp_common.h
> new file mode 100644
> index 0000000..adbf552
> --- /dev/null
> +++ b/sound/soc/intel/boards/skl_hda_dsp_common.h
> @@ -0,0 +1,34 @@
> +// SPDX-License-Identifier: GPL-2.0
> +// Copyright(c) 2015-17 Intel Corporation.
> +
> +/*
> + * This file defines data structures used in Machine Driver for Intel
> + * platforms with HDA Codecs.
> + */
> +
> +#ifndef __SOUND_SOC_HDA_DSP_COMMON_H
> +#define __SOUND_SOC_HDA_DSP_COMMON_H
> +#include <linux/module.h>
> +#include <linux/platform_device.h>
> +#include <sound/core.h>
> +#include <sound/jack.h>
> +
> +#define HDA_DSP_MAX_BE_DAI_LINKS 3
> +
> +struct skl_hda_hdmi_pcm {
> +	struct list_head head;
> +	struct snd_soc_dai *codec_dai;
> +	struct snd_soc_jack hdmi_jack;
> +	int device;
> +};
> +
> +struct skl_hda_private {
> +	struct list_head hdmi_pcm_list;
> +	int pcm_count;
> +};
> +
> +extern struct snd_soc_dai_link skl_hda_be_dai_links[HDA_DSP_MAX_BE_DAI_LINKS];
> +int skl_hda_hdmi_jack_init(struct snd_soc_card *card);
> +int skl_hda_hdmi_add_pcm(struct snd_soc_card *card, int device);
> +
> +#endif /* __SOUND_SOC_HDA_DSP_COMMON_H */
> diff --git a/sound/soc/intel/boards/skl_hda_dsp_generic.c b/sound/soc/intel/boards/skl_hda_dsp_generic.c
> new file mode 100644
> index 0000000..9e925ba
> --- /dev/null
> +++ b/sound/soc/intel/boards/skl_hda_dsp_generic.c
> @@ -0,0 +1,114 @@
> +// SPDX-License-Identifier: GPL-2.0
> +// Copyright(c) 2015-17 Intel Corporation.
> +
> +/*
> + * Machine Driver for SKL/KBL platforms with HDA Codecs

should this be SKL/KBL/APL/GLK....

> + */
> +
> +#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"
> +#include "skl_hda_dsp_common.h"
> +
> +static const char *platform_name = "0000:00:1f.3";

same comment as above, this is not constant across all devices

> +
> +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_card_late_probe(struct snd_soc_card *card)
> +{
> +	return skl_hda_hdmi_jack_init(card);
> +}
> +
> +static int
> +skl_hda_add_dai_link(struct snd_soc_card *card, struct snd_soc_dai_link *link)
> +{
> +	struct skl_hda_private *ctx = snd_soc_card_get_drvdata(card);
> +	int ret = 0;
> +
> +	dev_dbg(card->dev, "%s: dai link name - %s\n", __func__, link->name);
> +	link->platform_name = platform_name;
> +	link->nonatomic = 1;
> +
> +	if (strstr(link->name, "HDMI"))
> +		ret = skl_hda_hdmi_add_pcm(card, ctx->pcm_count);
> +	ctx->pcm_count++;
> +
> +	return ret;
> +}
> +
> +static struct snd_soc_card hda_soc_card = {
> +	.name = "skl_hda_card",
> +	.owner = THIS_MODULE,
> +	.dai_link = skl_hda_be_dai_links,
> +	.num_links = ARRAY_SIZE(skl_hda_be_dai_links),
> +	.dapm_routes = skl_hda_map,
> +	.num_dapm_routes = ARRAY_SIZE(skl_hda_map),
> +	.add_dai_link = skl_hda_add_dai_link,
> +	.fully_routed = true,
> +	.late_probe = skl_hda_card_late_probe,
> +};
> +
> +static int skl_hda_audio_probe(struct platform_device *pdev)
> +{
> +	struct skl_machine_pdata *pdata;
> +	struct skl_hda_private *ctx;
> +	int i;
> +
> +	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);
> +	ctx->pcm_count = ARRAY_SIZE(skl_hda_be_dai_links);
> +
> +	pdata = dev_get_drvdata(&pdev->dev);
> +	if (pdata && pdata->platform) {
> +		platform_name = pdata->platform;
> +		for (i = 0; i < ctx->pcm_count; i++)
> +			skl_hda_be_dai_links[i].platform_name = platform_name;
> +	}
> +
> +	hda_soc_card.dev = &pdev->dev;
> +	snd_soc_card_set_drvdata(&hda_soc_card, ctx);
> +
> +	return devm_snd_soc_register_card(&pdev->dev, &hda_soc_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,

is this needed if you have a single id?

> +};
> +
> +module_platform_driver(skl_hda_audio)
> +
> +/* Module information */
> +MODULE_DESCRIPTION("SKL/KBL HDA Generic Machine driver");

I don't see how this handles HDAudio codecs in general, is this limited 
to iDISP/HDMI support?

> +MODULE_AUTHOR("Rakesh Ughreja <rakesh.a.ughreja@intel.com>");
> +MODULE_LICENSE("GPL v2");
> +MODULE_ALIAS("platform:skl_hda_generic");
> diff --git a/sound/soc/intel/skylake/skl.h b/sound/soc/intel/skylake/skl.h
> index 75adce8..8a63626 100644
> --- a/sound/soc/intel/skylake/skl.h
> +++ b/sound/soc/intel/skylake/skl.h
> @@ -113,6 +113,7 @@ struct skl_dma_params {
>   struct skl_machine_pdata {
>   	u32 dmic_num;
>   	bool use_tplg_pcm; /* use dais and dai links from topology */
> +	const char *platform;
>   };
>   
>   struct skl_dsp_ops {
>
Ughreja, Rakesh A Feb. 26, 2018, 8:18 a.m. UTC | #2
>-----Original Message-----
>From: Pierre-Louis Bossart [mailto:pierre-louis.bossart@linux.intel.com]
>Sent: Friday, February 23, 2018 10:03 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: [PATCH v1 1/9] ASoC: Intel: Boards: Machine driver for Intel
>platforms
>
>On 2/23/18 2:12 AM, Rakesh Ughreja wrote:
>> Add machine driver for Intel platforms (SKL/KBL/BXT) 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. 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_dsp_common.c  | 107
>+++++++++++++++++++++++++
>>   sound/soc/intel/boards/skl_hda_dsp_common.h  |  34 ++++++++
>>   sound/soc/intel/boards/skl_hda_dsp_generic.c | 114
>+++++++++++++++++++++++++++
>>   sound/soc/intel/skylake/skl.h                |   1 +
>>   6 files changed, 268 insertions(+)
>>   create mode 100644 sound/soc/intel/boards/skl_hda_dsp_common.c
>>   create mode 100644 sound/soc/intel/boards/skl_hda_dsp_common.h
>>   create mode 100644 sound/soc/intel/boards/skl_hda_dsp_generic.c
>>
>> diff --git a/sound/soc/intel/boards/Kconfig b/sound/soc/intel/boards/Kconfig
>> index eedc83b..3b20601 100644
>> --- a/sound/soc/intel/boards/Kconfig
>> +++ b/sound/soc/intel/boards/Kconfig
>> @@ -268,6 +268,16 @@ config
>SND_SOC_INTEL_KBL_DA7219_MAX98357A_MACH
>>   	  This adds support for ASoC Onboard Codec I2S machine driver. This will
>>   	  create an alsa sound card for DA7219 + MAX98357A I2S audio codec.
>>   	  Say Y if you have such a device.
>> +
>> +config SND_SOC_INTEL_SKL_HDA_DSP_GENERIC_MACH
>> +	tristate "SKL/KBL/BXT with HDA Codecs"
>> +	select SND_SOC_INTEL_SST
>> +	depends on SND_SOC_INTEL_SKYLAKE
>
>There two lines are no longer necessary (done at the platform level for
>SST and you already in an if SKYLAKE block

Good point. Now, removed these two lines.

>
>> +	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.
>
>does this handle idisp as well? If yes, does this option conflict with
>the enablement of the other machine drivers which use the hdac_hdmi codec?

Yes, the machine driver does handle iDisp codec. I didn't understand your
comment about conflict and other machine driver using the hdac_hdmi
codec. There is only one iDisp codec and so what is the reason for having
two machine drivers accessing the same codec ? 

>
>
>> +          Say Y or m if you have such a device. This is a recommended option.
>>   	  If unsure select "N".
>>
>>   endif ## SND_SOC_INTEL_SKYLAKE
>> diff --git a/sound/soc/intel/boards/Makefile
>b/sound/soc/intel/boards/Makefile
>> index 6fae506..5d65481 100644
>> --- a/sound/soc/intel/boards/Makefile
>> +++ b/sound/soc/intel/boards/Makefile
>> @@ -18,6 +18,7 @@ snd-soc-kbl_da7219_max98357a-objs :=
>kbl_da7219_max98357a.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_dsp-objs := skl_hda_dsp_generic.o skl_hda_dsp_common.o
>>   snd-skl_nau88l25_max98357a-objs := skl_nau88l25_max98357a.o
>>   snd-soc-skl_nau88l25_ssm4567-objs := skl_nau88l25_ssm4567.o
>>
>> @@ -42,3 +43,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_DSP_GENERIC_MACH) += snd-soc-
>skl_hda_dsp.o
>> diff --git a/sound/soc/intel/boards/skl_hda_dsp_common.c
>b/sound/soc/intel/boards/skl_hda_dsp_common.c
>> new file mode 100644
>> index 0000000..7066bed
>> --- /dev/null
>> +++ b/sound/soc/intel/boards/skl_hda_dsp_common.c
>> @@ -0,0 +1,107 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +// Copyright(c) 2015-17 Intel Corporation.
>> +
>> +/*
>> + * Common functions used in different Intel machine drivers
>> + */
>> +#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"
>> +#include "skl_hda_dsp_common.h"
>> +
>> +#define NAME_SIZE	32
>> +
>> +int skl_hda_hdmi_add_pcm(struct snd_soc_card *card, int device)
>> +{
>> +	char dai_name[NAME_SIZE];
>> +	struct skl_hda_private *ctx = snd_soc_card_get_drvdata(card);
>> +	struct skl_hda_hdmi_pcm *pcm;
>> +	static int i = 1;	/* hdmi codec dai name starts from index 1 */
>> +
>> +	pcm = devm_kzalloc(card->dev, sizeof(*pcm), GFP_KERNEL);
>> +	if (!pcm)
>> +		return -ENOMEM;
>> +
>> +	snprintf(dai_name, sizeof(dai_name), "intel-hdmi-hifi%d", i++);
>> +	pcm->codec_dai = snd_soc_card_get_codec_dai(card, dai_name);
>> +	if (!pcm->codec_dai)
>> +		return -EINVAL;
>> +
>> +	pcm->device = device;
>> +	list_add_tail(&pcm->head, &ctx->hdmi_pcm_list);
>> +
>> +	return 0;
>> +}
>> +
>> +/* skl_hda_digital audio interface glue - connects codec <--> CPU */
>> +struct snd_soc_dai_link skl_hda_be_dai_links[HDA_DSP_MAX_BE_DAI_LINKS]
>= {
>> +
>> +	/* 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",
>
>what happens if you run on a device with a different ID, e.g. APL or GLK?

This is just a default ID. The same machine driver should work. 
The platform name (ID) is passed from platform driver using platform_data. 
You can see that in the later part of the code. So this name gets overwritten. 

>
>> +		.dpcm_playback = 1,
>> +		.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",
>> +		.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",
>> +		.dpcm_playback = 1,
>> +		.no_pcm = 1,
>> +	},
>> +};
>> +
>> +int skl_hda_hdmi_jack_init(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_component *component = NULL;
>> +	int err;
>> +	char jack_name[NAME_SIZE];
>> +
>> +	list_for_each_entry(pcm, &ctx->hdmi_pcm_list, head) {
>> +		component = pcm->codec_dai->component;
>> +		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, &pcm-
>>hdmi_jack,
>> +					NULL, 0);
>> +
>> +		if (err)
>> +			return err;
>> +
>> +		err = hdac_hdmi_jack_init(pcm->codec_dai, pcm->device,
>> +						&pcm->hdmi_jack);
>> +		if (err < 0)
>> +			return err;
>> +	}
>> +
>> +	if (!component)
>> +		return -EINVAL;
>> +
>> +	return hdac_hdmi_jack_port_init(component, &card->dapm);
>> +}
>> diff --git a/sound/soc/intel/boards/skl_hda_dsp_common.h
>b/sound/soc/intel/boards/skl_hda_dsp_common.h
>> new file mode 100644
>> index 0000000..adbf552
>> --- /dev/null
>> +++ b/sound/soc/intel/boards/skl_hda_dsp_common.h
>> @@ -0,0 +1,34 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +// Copyright(c) 2015-17 Intel Corporation.
>> +
>> +/*
>> + * This file defines data structures used in Machine Driver for Intel
>> + * platforms with HDA Codecs.
>> + */
>> +
>> +#ifndef __SOUND_SOC_HDA_DSP_COMMON_H
>> +#define __SOUND_SOC_HDA_DSP_COMMON_H
>> +#include <linux/module.h>
>> +#include <linux/platform_device.h>
>> +#include <sound/core.h>
>> +#include <sound/jack.h>
>> +
>> +#define HDA_DSP_MAX_BE_DAI_LINKS 3
>> +
>> +struct skl_hda_hdmi_pcm {
>> +	struct list_head head;
>> +	struct snd_soc_dai *codec_dai;
>> +	struct snd_soc_jack hdmi_jack;
>> +	int device;
>> +};
>> +
>> +struct skl_hda_private {
>> +	struct list_head hdmi_pcm_list;
>> +	int pcm_count;
>> +};
>> +
>> +extern struct snd_soc_dai_link
>skl_hda_be_dai_links[HDA_DSP_MAX_BE_DAI_LINKS];
>> +int skl_hda_hdmi_jack_init(struct snd_soc_card *card);
>> +int skl_hda_hdmi_add_pcm(struct snd_soc_card *card, int device);
>> +
>> +#endif /* __SOUND_SOC_HDA_DSP_COMMON_H */
>> diff --git a/sound/soc/intel/boards/skl_hda_dsp_generic.c
>b/sound/soc/intel/boards/skl_hda_dsp_generic.c
>> new file mode 100644
>> index 0000000..9e925ba
>> --- /dev/null
>> +++ b/sound/soc/intel/boards/skl_hda_dsp_generic.c
>> @@ -0,0 +1,114 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +// Copyright(c) 2015-17 Intel Corporation.
>> +
>> +/*
>> + * Machine Driver for SKL/KBL platforms with HDA Codecs
>
>should this be SKL/KBL/APL/GLK....

Yes true. Will change it.

>
>> + */
>> +
>> +#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"
>> +#include "skl_hda_dsp_common.h"
>> +
>> +static const char *platform_name = "0000:00:1f.3";
>
>same comment as above, this is not constant across all devices

Yes and its overwritten when it is received from platform driver.
You can see in the later part of the code.

Copy/pasting the code here

>> +	pdata = dev_get_drvdata(&pdev->dev);
>> +	if (pdata && pdata->platform) {
>> +		platform_name = pdata->platform;
>> +		for (i = 0; i < ctx->pcm_count; i++)
>> +			skl_hda_be_dai_links[i].platform_name =
>platform_name;
>> +	}



>
>> +
>> +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_card_late_probe(struct snd_soc_card *card)
>> +{
>> +	return skl_hda_hdmi_jack_init(card);
>> +}
>> +
>> +static int
>> +skl_hda_add_dai_link(struct snd_soc_card *card, struct snd_soc_dai_link *link)
>> +{
>> +	struct skl_hda_private *ctx = snd_soc_card_get_drvdata(card);
>> +	int ret = 0;
>> +
>> +	dev_dbg(card->dev, "%s: dai link name - %s\n", __func__, link->name);
>> +	link->platform_name = platform_name;
>> +	link->nonatomic = 1;
>> +
>> +	if (strstr(link->name, "HDMI"))
>> +		ret = skl_hda_hdmi_add_pcm(card, ctx->pcm_count);
>> +	ctx->pcm_count++;
>> +
>> +	return ret;
>> +}
>> +
>> +static struct snd_soc_card hda_soc_card = {
>> +	.name = "skl_hda_card",
>> +	.owner = THIS_MODULE,
>> +	.dai_link = skl_hda_be_dai_links,
>> +	.num_links = ARRAY_SIZE(skl_hda_be_dai_links),
>> +	.dapm_routes = skl_hda_map,
>> +	.num_dapm_routes = ARRAY_SIZE(skl_hda_map),
>> +	.add_dai_link = skl_hda_add_dai_link,
>> +	.fully_routed = true,
>> +	.late_probe = skl_hda_card_late_probe,
>> +};
>> +
>> +static int skl_hda_audio_probe(struct platform_device *pdev)
>> +{
>> +	struct skl_machine_pdata *pdata;
>> +	struct skl_hda_private *ctx;
>> +	int i;
>> +
>> +	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);
>> +	ctx->pcm_count = ARRAY_SIZE(skl_hda_be_dai_links);
>> +
>> +	pdata = dev_get_drvdata(&pdev->dev);
>> +	if (pdata && pdata->platform) {
>> +		platform_name = pdata->platform;
>> +		for (i = 0; i < ctx->pcm_count; i++)
>> +			skl_hda_be_dai_links[i].platform_name =
>platform_name;
>> +	}
>> +
>> +	hda_soc_card.dev = &pdev->dev;
>> +	snd_soc_card_set_drvdata(&hda_soc_card, ctx);
>> +
>> +	return devm_snd_soc_register_card(&pdev->dev, &hda_soc_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,
>
>is this needed if you have a single id?

I did the way other machine drivers are doing.
But I think it's not needed for single ID. But I think it's this way so that
more IDs can be added ? So I will keep it as it is.

>
>> +};
>> +
>> +module_platform_driver(skl_hda_audio)
>> +
>> +/* Module information */
>> +MODULE_DESCRIPTION("SKL/KBL HDA Generic Machine driver");
>
>I don't see how this handles HDAudio codecs in general, is this limited
>to iDISP/HDMI support?

In this patch only the iDisp/HDMI support is added. In the later patches
[Patch 9/9] support for HDA codec is added.

Regards,
Rakesh
Pierre-Louis Bossart Feb. 26, 2018, 7:11 p.m. UTC | #3
>>> +	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.
>>
>> does this handle idisp as well? If yes, does this option conflict with
>> the enablement of the other machine drivers which use the hdac_hdmi codec?
> 
> Yes, the machine driver does handle iDisp codec. I didn't understand your
> comment about conflict and other machine driver using the hdac_hdmi
> codec. There is only one iDisp codec and so what is the reason for having
> two machine drivers accessing the same codec ?

thinking a bit more, all the existing machine drivers which provide 
support for HDMI/iDisp also cater to I2S codecs. Since you deal with 
HDaudio only if you haven't found a known I2S codec there is no real 
source of conflict.


>>> +/* skl_hda_digital audio interface glue - connects codec <--> CPU */
>>> +struct snd_soc_dai_link skl_hda_be_dai_links[HDA_DSP_MAX_BE_DAI_LINKS]
>> = {
>>> +
>>> +	/* 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",
>>
>> what happens if you run on a device with a different ID, e.g. APL or GLK?
> 
> This is just a default ID. The same machine driver should work.
> The platform name (ID) is passed from platform driver using platform_data.
> You can see that in the later part of the code. So this name gets overwritten.

i don't see the point of having a default then? It could just as well be 
"deadbeef" or not initialized to avoid storing useless strings.


>>> +static const char *platform_name = "0000:00:1f.3";
>>
>> same comment as above, this is not constant across all devices
> 
> Yes and its overwritten when it is received from platform driver.
> You can see in the later part of the code.
> 
> Copy/pasting the code here
> 
>>> +	pdata = dev_get_drvdata(&pdev->dev);
>>> +	if (pdata && pdata->platform) {
>>> +		platform_name = pdata->platform;

can be be a const char * then?

>>> +		for (i = 0; i < ctx->pcm_count; i++)
>>> +			skl_hda_be_dai_links[i].platform_name =
>> platform_name;

and why do you need a static variable in the first place, wouldn't

skl_hda_be_dai_links[i].platform_name = pdata->platform work just as well?


>>> +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,
>>
>> is this needed if you have a single id?
> 
> I did the way other machine drivers are doing.
> But I think it's not needed for single ID. But I think it's this way so that
> more IDs can be added ? So I will keep it as it is.

what other IDS would you use?

> 
>>
>>> +};
>>> +
>>> +module_platform_driver(skl_hda_audio)
>>> +
>>> +/* Module information */
>>> +MODULE_DESCRIPTION("SKL/KBL HDA Generic Machine driver");
>>
>> I don't see how this handles HDAudio codecs in general, is this limited
>> to iDISP/HDMI support?
> 
> In this patch only the iDisp/HDMI support is added. In the later patches
> [Patch 9/9] support for HDA codec is added.

yes, but I have to go read 8 patches before finding it out... I comment 
patches one by one.
Ughreja, Rakesh A Feb. 27, 2018, 5:12 a.m. UTC | #4
>-----Original Message-----
>From: Pierre-Louis Bossart [mailto:pierre-louis.bossart@linux.intel.com]
>Sent: Tuesday, February 27, 2018 12:41 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] [PATCH v1 1/9] ASoC: Intel: Boards: Machine driver for
>Intel platforms
>
>
>>>> +	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.
>>>
>>> does this handle idisp as well? If yes, does this option conflict with
>>> the enablement of the other machine drivers which use the hdac_hdmi codec?
>>
>> Yes, the machine driver does handle iDisp codec. I didn't understand your
>> comment about conflict and other machine driver using the hdac_hdmi
>> codec. There is only one iDisp codec and so what is the reason for having
>> two machine drivers accessing the same codec ?
>
>thinking a bit more, all the existing machine drivers which provide
>support for HDMI/iDisp also cater to I2S codecs. Since you deal with
>HDaudio only if you haven't found a known I2S codec there is no real
>source of conflict.
>

So is it clear now ? Do I need to change anything ?

>
>>>> +/* skl_hda_digital audio interface glue - connects codec <--> CPU */
>>>> +struct snd_soc_dai_link
>skl_hda_be_dai_links[HDA_DSP_MAX_BE_DAI_LINKS]
>>> = {
>>>> +
>>>> +	/* 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",
>>>
>>> what happens if you run on a device with a different ID, e.g. APL or GLK?
>>
>> This is just a default ID. The same machine driver should work.
>> The platform name (ID) is passed from platform driver using platform_data.
>> You can see that in the later part of the code. So this name gets overwritten.
>
>i don't see the point of having a default then? It could just as well be
>"deadbeef" or not initialized to avoid storing useless strings.
>

Yes, I can make default as NULL.

>
>>>> +static const char *platform_name = "0000:00:1f.3";
>>>
>>> same comment as above, this is not constant across all devices
>>
>> Yes and its overwritten when it is received from platform driver.
>> You can see in the later part of the code.
>>
>> Copy/pasting the code here
>>
>>>> +	pdata = dev_get_drvdata(&pdev->dev);
>>>> +	if (pdata && pdata->platform) {
>>>> +		platform_name = pdata->platform;
>
>can be be a const char * then?

Since its used only within the file, I marked it as 
static const char *

>
>>>> +		for (i = 0; i < ctx->pcm_count; i++)
>>>> +			skl_hda_be_dai_links[i].platform_name =
>>> platform_name;
>
>and why do you need a static variable in the first place, wouldn't
>
>skl_hda_be_dai_links[i].platform_name = pdata->platform work just as well?

platform_name variable is used in skl_hda_add_dai_link as well.

>
>
>>>> +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,
>>>
>>> is this needed if you have a single id?
>>
>> I did the way other machine drivers are doing.
>> But I think it's not needed for single ID. But I think it's this way so that
>> more IDs can be added ? So I will keep it as it is.
>
>what other IDS would you use?

I would like to use different IDs for different combination of the codecs.
e.g. HDMI Only, HDA+HDMI to begin with.

>
>>
>>>
>>>> +};
>>>> +
>>>> +module_platform_driver(skl_hda_audio)
>>>> +
>>>> +/* Module information */
>>>> +MODULE_DESCRIPTION("SKL/KBL HDA Generic Machine driver");
>>>
>>> I don't see how this handles HDAudio codecs in general, is this limited
>>> to iDISP/HDMI support?
>>
>> In this patch only the iDisp/HDMI support is added. In the later patches
>> [Patch 9/9] support for HDA codec is added.
>
>yes, but I have to go read 8 patches before finding it out... I comment
>patches one by one.

This is mentioned in the commit message.
diff mbox

Patch

diff --git a/sound/soc/intel/boards/Kconfig b/sound/soc/intel/boards/Kconfig
index eedc83b..3b20601 100644
--- a/sound/soc/intel/boards/Kconfig
+++ b/sound/soc/intel/boards/Kconfig
@@ -268,6 +268,16 @@  config SND_SOC_INTEL_KBL_DA7219_MAX98357A_MACH
 	  This adds support for ASoC Onboard Codec I2S machine driver. This will
 	  create an alsa sound card for DA7219 + MAX98357A I2S audio codec.
 	  Say Y if you have such a device.
+
+config SND_SOC_INTEL_SKL_HDA_DSP_GENERIC_MACH
+	tristate "SKL/KBL/BXT 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 or m if you have such a device. This is a recommended option.
 	  If unsure select "N".
 
 endif ## SND_SOC_INTEL_SKYLAKE
diff --git a/sound/soc/intel/boards/Makefile b/sound/soc/intel/boards/Makefile
index 6fae506..5d65481 100644
--- a/sound/soc/intel/boards/Makefile
+++ b/sound/soc/intel/boards/Makefile
@@ -18,6 +18,7 @@  snd-soc-kbl_da7219_max98357a-objs := kbl_da7219_max98357a.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_dsp-objs := skl_hda_dsp_generic.o skl_hda_dsp_common.o
 snd-skl_nau88l25_max98357a-objs := skl_nau88l25_max98357a.o
 snd-soc-skl_nau88l25_ssm4567-objs := skl_nau88l25_ssm4567.o
 
@@ -42,3 +43,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_DSP_GENERIC_MACH) += snd-soc-skl_hda_dsp.o
diff --git a/sound/soc/intel/boards/skl_hda_dsp_common.c b/sound/soc/intel/boards/skl_hda_dsp_common.c
new file mode 100644
index 0000000..7066bed
--- /dev/null
+++ b/sound/soc/intel/boards/skl_hda_dsp_common.c
@@ -0,0 +1,107 @@ 
+// SPDX-License-Identifier: GPL-2.0
+// Copyright(c) 2015-17 Intel Corporation.
+
+/*
+ * Common functions used in different Intel machine drivers
+ */
+#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"
+#include "skl_hda_dsp_common.h"
+
+#define NAME_SIZE	32
+
+int skl_hda_hdmi_add_pcm(struct snd_soc_card *card, int device)
+{
+	char dai_name[NAME_SIZE];
+	struct skl_hda_private *ctx = snd_soc_card_get_drvdata(card);
+	struct skl_hda_hdmi_pcm *pcm;
+	static int i = 1;	/* hdmi codec dai name starts from index 1 */
+
+	pcm = devm_kzalloc(card->dev, sizeof(*pcm), GFP_KERNEL);
+	if (!pcm)
+		return -ENOMEM;
+
+	snprintf(dai_name, sizeof(dai_name), "intel-hdmi-hifi%d", i++);
+	pcm->codec_dai = snd_soc_card_get_codec_dai(card, dai_name);
+	if (!pcm->codec_dai)
+		return -EINVAL;
+
+	pcm->device = device;
+	list_add_tail(&pcm->head, &ctx->hdmi_pcm_list);
+
+	return 0;
+}
+
+/* skl_hda_digital audio interface glue - connects codec <--> CPU */
+struct snd_soc_dai_link skl_hda_be_dai_links[HDA_DSP_MAX_BE_DAI_LINKS] = {
+
+	/* 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,
+		.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",
+		.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",
+		.dpcm_playback = 1,
+		.no_pcm = 1,
+	},
+};
+
+int skl_hda_hdmi_jack_init(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_component *component = NULL;
+	int err;
+	char jack_name[NAME_SIZE];
+
+	list_for_each_entry(pcm, &ctx->hdmi_pcm_list, head) {
+		component = pcm->codec_dai->component;
+		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, &pcm->hdmi_jack,
+					NULL, 0);
+
+		if (err)
+			return err;
+
+		err = hdac_hdmi_jack_init(pcm->codec_dai, pcm->device,
+						&pcm->hdmi_jack);
+		if (err < 0)
+			return err;
+	}
+
+	if (!component)
+		return -EINVAL;
+
+	return hdac_hdmi_jack_port_init(component, &card->dapm);
+}
diff --git a/sound/soc/intel/boards/skl_hda_dsp_common.h b/sound/soc/intel/boards/skl_hda_dsp_common.h
new file mode 100644
index 0000000..adbf552
--- /dev/null
+++ b/sound/soc/intel/boards/skl_hda_dsp_common.h
@@ -0,0 +1,34 @@ 
+// SPDX-License-Identifier: GPL-2.0
+// Copyright(c) 2015-17 Intel Corporation.
+
+/*
+ * This file defines data structures used in Machine Driver for Intel
+ * platforms with HDA Codecs.
+ */
+
+#ifndef __SOUND_SOC_HDA_DSP_COMMON_H
+#define __SOUND_SOC_HDA_DSP_COMMON_H
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <sound/core.h>
+#include <sound/jack.h>
+
+#define HDA_DSP_MAX_BE_DAI_LINKS 3
+
+struct skl_hda_hdmi_pcm {
+	struct list_head head;
+	struct snd_soc_dai *codec_dai;
+	struct snd_soc_jack hdmi_jack;
+	int device;
+};
+
+struct skl_hda_private {
+	struct list_head hdmi_pcm_list;
+	int pcm_count;
+};
+
+extern struct snd_soc_dai_link skl_hda_be_dai_links[HDA_DSP_MAX_BE_DAI_LINKS];
+int skl_hda_hdmi_jack_init(struct snd_soc_card *card);
+int skl_hda_hdmi_add_pcm(struct snd_soc_card *card, int device);
+
+#endif /* __SOUND_SOC_HDA_DSP_COMMON_H */
diff --git a/sound/soc/intel/boards/skl_hda_dsp_generic.c b/sound/soc/intel/boards/skl_hda_dsp_generic.c
new file mode 100644
index 0000000..9e925ba
--- /dev/null
+++ b/sound/soc/intel/boards/skl_hda_dsp_generic.c
@@ -0,0 +1,114 @@ 
+// SPDX-License-Identifier: GPL-2.0
+// Copyright(c) 2015-17 Intel Corporation.
+
+/*
+ * Machine Driver for SKL/KBL platforms with HDA Codecs
+ */
+
+#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"
+#include "skl_hda_dsp_common.h"
+
+static const char *platform_name = "0000:00:1f.3";
+
+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_card_late_probe(struct snd_soc_card *card)
+{
+	return skl_hda_hdmi_jack_init(card);
+}
+
+static int
+skl_hda_add_dai_link(struct snd_soc_card *card, struct snd_soc_dai_link *link)
+{
+	struct skl_hda_private *ctx = snd_soc_card_get_drvdata(card);
+	int ret = 0;
+
+	dev_dbg(card->dev, "%s: dai link name - %s\n", __func__, link->name);
+	link->platform_name = platform_name;
+	link->nonatomic = 1;
+
+	if (strstr(link->name, "HDMI"))
+		ret = skl_hda_hdmi_add_pcm(card, ctx->pcm_count);
+	ctx->pcm_count++;
+
+	return ret;
+}
+
+static struct snd_soc_card hda_soc_card = {
+	.name = "skl_hda_card",
+	.owner = THIS_MODULE,
+	.dai_link = skl_hda_be_dai_links,
+	.num_links = ARRAY_SIZE(skl_hda_be_dai_links),
+	.dapm_routes = skl_hda_map,
+	.num_dapm_routes = ARRAY_SIZE(skl_hda_map),
+	.add_dai_link = skl_hda_add_dai_link,
+	.fully_routed = true,
+	.late_probe = skl_hda_card_late_probe,
+};
+
+static int skl_hda_audio_probe(struct platform_device *pdev)
+{
+	struct skl_machine_pdata *pdata;
+	struct skl_hda_private *ctx;
+	int i;
+
+	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);
+	ctx->pcm_count = ARRAY_SIZE(skl_hda_be_dai_links);
+
+	pdata = dev_get_drvdata(&pdev->dev);
+	if (pdata && pdata->platform) {
+		platform_name = pdata->platform;
+		for (i = 0; i < ctx->pcm_count; i++)
+			skl_hda_be_dai_links[i].platform_name = platform_name;
+	}
+
+	hda_soc_card.dev = &pdev->dev;
+	snd_soc_card_set_drvdata(&hda_soc_card, ctx);
+
+	return devm_snd_soc_register_card(&pdev->dev, &hda_soc_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");
diff --git a/sound/soc/intel/skylake/skl.h b/sound/soc/intel/skylake/skl.h
index 75adce8..8a63626 100644
--- a/sound/soc/intel/skylake/skl.h
+++ b/sound/soc/intel/skylake/skl.h
@@ -113,6 +113,7 @@  struct skl_dma_params {
 struct skl_machine_pdata {
 	u32 dmic_num;
 	bool use_tplg_pcm; /* use dais and dai links from topology */
+	const char *platform;
 };
 
 struct skl_dsp_ops {