[alsa-devel] [RFC 01/10] ASoC: Intel: Boards: Machine driver for Intel platforms
Pierre-Louis Bossart
pierre-louis.bossart at linux.intel.com
Mon Dec 4 15:49:55 CET 2017
On 12/4/17 4:55 AM, Ughreja, Rakesh A wrote:
>
>
>> -----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.
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 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.
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.
>
More information about the Alsa-devel
mailing list