[alsa-devel] [RFC 01/10] ASoC: Intel: Boards: Machine driver for Intel platforms
Ughreja, Rakesh A
rakesh.a.ughreja at intel.com
Mon Dec 4 11:55:06 CET 2017
>-----Original Message-----
>From: Pierre-Louis Bossart [mailto:pierre-louis.bossart at linux.intel.com]
>Sent: Friday, December 1, 2017 11:28 PM
>To: Ughreja, Rakesh A <rakesh.a.ughreja at intel.com>; alsa-devel at alsa-
>project.org; broonie at kernel.org; tiwai at suse.de;
>liam.r.girdwood at linux.intel.com
>Cc: Koul, Vinod <vinod.koul at intel.com>; Patches Audio
><patches.audio at 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 at 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.
More information about the Alsa-devel
mailing list