>-----Original Message----- >From: Pierre-Louis Bossart [mailto:pierre-louis.bossart@xxxxxxxxxxxxxxx] >Sent: Monday, December 4, 2017 8:20 PM >To: Ughreja, Rakesh A <rakesh.a.ughreja@xxxxxxxxx>; alsa-devel@alsa- >project.org; broonie@xxxxxxxxxx; tiwai@xxxxxxx; liam.r.girdwood@xxxxxxxxxxxxxxx >Cc: Koul, Vinod <vinod.koul@xxxxxxxxx>; Patches Audio ><patches.audio@xxxxxxxxx> >Subject: Re: [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@xxxxxxxxxxxxxxx] >>> Sent: Friday, December 1, 2017 11:28 PM >>> To: Ughreja, Rakesh A <rakesh.a.ughreja@xxxxxxxxx>; alsa-devel@alsa- >>> project.org; broonie@xxxxxxxxxx; tiwai@xxxxxxx; >>> liam.r.girdwood@xxxxxxxxxxxxxxx >>> Cc: Koul, Vinod <vinod.koul@xxxxxxxxx>; Patches Audio >>> <patches.audio@xxxxxxxxx> >>> Subject: Re: [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@xxxxxxxxx> >>>> --- >>>> 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. _______________________________________________ Alsa-devel mailing list Alsa-devel@xxxxxxxxxxxxxxxx http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
