[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 16:37:37 CET 2017
On 12/4/17 9:10 AM, Ughreja, Rakesh A wrote:
>
>
>> -----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.
we can deal with this later, this was just a remark to think of headless
devices.
>
>
>>>
>>>>
>>>>>
>>>>> 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 ?
works for me.
>
>>
>>>
>>>>
>>>>> 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