-----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_SSTdepends on SND_SOC_INTEL_SKYLAKEthat'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_HDMIhelpThis 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".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.