[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 16:10:16 CET 2017
>-----Original Message-----
>From: Pierre-Louis Bossart [mailto:pierre-louis.bossart at linux.intel.com]
>Sent: Monday, December 4, 2017 8:20 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/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.
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 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 ?
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.
More information about the Alsa-devel
mailing list