[alsa-devel] [PATCH v1 9/9] ASoC: Intel: Boards: add support for HDA codecs
Ughreja, Rakesh A
rakesh.a.ughreja at intel.com
Mon Feb 26 17:01:01 CET 2018
>-----Original Message-----
>From: Pierre-Louis Bossart [mailto:pierre-louis.bossart at linux.intel.com]
>Sent: Friday, February 23, 2018 10:34 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: [PATCH v1 9/9] ASoC: Intel: Boards: add support for HDA codecs
>
>On 2/23/18 2:12 AM, Rakesh Ughreja wrote:
>> Add support for HDA codecs. add required widgets, controls, routes
>> and dai links for the same.
>>
>> Signed-off-by: Rakesh Ughreja <rakesh.a.ughreja at intel.com>
>> ---
>> sound/soc/intel/Kconfig | 1 +
>> sound/soc/intel/boards/skl_hda_dsp_common.c | 24
>+++++++++++++++++++++++
>> sound/soc/intel/boards/skl_hda_dsp_common.h | 2 +-
>> sound/soc/intel/boards/skl_hda_dsp_generic.c | 29
>++++++++++++++++++++++++++++
>> sound/soc/intel/skylake/skl.c | 27 ++++++++++++++++++++++----
>> 5 files changed, 78 insertions(+), 5 deletions(-)
>>
>> diff --git a/sound/soc/intel/Kconfig b/sound/soc/intel/Kconfig
>> index ceb105c..d44cf1e 100644
>> --- a/sound/soc/intel/Kconfig
>> +++ b/sound/soc/intel/Kconfig
>> @@ -107,6 +107,7 @@ config SND_SOC_INTEL_SKYLAKE
>> select SND_HDA_DSP_LOADER
>> select SND_SOC_TOPOLOGY
>> select SND_SOC_INTEL_SST
>> + select SND_SOC_HDAC_HDA if
>SND_SOC_INTEL_SKL_HDA_DSP_GENERIC_MACH
>
>This looks odd, it beats the purpose of the clean split between platform
>and machine Kconfig added in 2017. You should select SND_SOC_HDAC_HDA in
>the config for SND_SOC_INTEL_SKL_HDA_DSP_GENERIC_MACH
If I move into the machine driver Kconfig, then it will be loaded after
the SKL platform driver loads and so symbol dependency will create an issue.
hdac_hda needs to be loaded before the SKL driver, so that it can
get the ops from library. So I will have to load it manually in the platform
driver code. Let me try that, if it does not work, I will come back on this
topic.
>
>> select SND_SOC_ACPI_INTEL_MATCH
>> help
>> If you have a Intel Skylake/Broxton/ApolloLake/KabyLake/
>> diff --git a/sound/soc/intel/boards/skl_hda_dsp_common.c
>b/sound/soc/intel/boards/skl_hda_dsp_common.c
>> index 7066bed..f5fa475 100644
>> --- a/sound/soc/intel/boards/skl_hda_dsp_common.c
>> +++ b/sound/soc/intel/boards/skl_hda_dsp_common.c
>> @@ -73,6 +73,30 @@ struct snd_soc_dai_link
>skl_hda_be_dai_links[HDA_DSP_MAX_BE_DAI_LINKS] = {
>> .dpcm_playback = 1,
>> .no_pcm = 1,
>> },
>> + {
>> + .name = "Analog Playback and Capture",
>> + .id = 4,
>> + .cpu_dai_name = "Analog CPU DAI",
>> + .codec_name = "ehdaudio0D0",
>> + .codec_dai_name = "Analog Codec DAI",
>> + .platform_name = "0000:00:1f.3",
>> + .dpcm_playback = 1,
>> + .dpcm_capture = 1,
>> + .init = NULL,
>> + .no_pcm = 1,
>> + },
>> + {
>> + .name = "Digital Playback and Capture",
>> + .id = 5,
>> + .cpu_dai_name = "Digital CPU DAI",
>> + .codec_name = "ehdaudio0D0",
>> + .codec_dai_name = "Digital Codec DAI",
>> + .platform_name = "0000:00:1f.3",
>> + .dpcm_playback = 1,
>> + .dpcm_capture = 1,
>> + .init = NULL,
>> + .no_pcm = 1,
>> + },
>> };
>>
>> int skl_hda_hdmi_jack_init(struct snd_soc_card *card)
>> diff --git a/sound/soc/intel/boards/skl_hda_dsp_common.h
>b/sound/soc/intel/boards/skl_hda_dsp_common.h
>> index adbf552..a9bc92b 100644
>> --- a/sound/soc/intel/boards/skl_hda_dsp_common.h
>> +++ b/sound/soc/intel/boards/skl_hda_dsp_common.h
>> @@ -13,7 +13,7 @@
>> #include <sound/core.h>
>> #include <sound/jack.h>
>>
>> -#define HDA_DSP_MAX_BE_DAI_LINKS 3
>> +#define HDA_DSP_MAX_BE_DAI_LINKS 5
>>
>> struct skl_hda_hdmi_pcm {
>> struct list_head head;
>> diff --git a/sound/soc/intel/boards/skl_hda_dsp_generic.c
>b/sound/soc/intel/boards/skl_hda_dsp_generic.c
>> index 9e925ba..009683d 100644
>> --- a/sound/soc/intel/boards/skl_hda_dsp_generic.c
>> +++ b/sound/soc/intel/boards/skl_hda_dsp_generic.c
>> @@ -18,8 +18,33 @@
>>
>> static const char *platform_name = "0000:00:1f.3";
>>
>> +static const struct snd_kcontrol_new skl_hda_controls[] = {
>> + SOC_DAPM_PIN_SWITCH("Headphone"),
>> + SOC_DAPM_PIN_SWITCH("Headset Mic"),
>> +};
>> +
>> +static const struct snd_soc_dapm_widget skl_hda_widgets[] = {
>> + SND_SOC_DAPM_HP("Headphone", NULL),
>> + SND_SOC_DAPM_MIC("Headset Mic", NULL),
>> + SND_SOC_DAPM_SPK("Codec Speaker", NULL),
>> + SND_SOC_DAPM_MIC("Codec Mic", NULL),
>> +};
>
>what about all the other outputs, e.g. line out? And how does this work
>with pin retasking?
Good point.
The hdac_hda is just a wrapper around the legacy codec driver
and so it relies on the functionality of the legacy HDA codec driver
for all the functionality including pin re-tasking.
The widget names that you see above is just to complete the
DAPM route. Based on your comment I am planning to rename it as
following
Analog In Endpoint
Analog Output Endpoint
Digital In Endpoint
Digital Out Endpoint
and will connect it to the Codec Pins.
Also I think it makes sense to rename the codec Pin names accordingly
Codec Analog Input Pin
Codec Analog Output Pin
Codec Digital Input Pin
Codec Digital Output Pin
>
>> +
>> static const struct snd_soc_dapm_route skl_hda_map[] = {
>>
>> + /* HP jack connectors - unknown if we have jack detection */
>> + { "Headphone", NULL, "Codec Output Pin1" },
>> + { "Codec Speaker", NULL, "Codec Output Pin2" },
>> + { "Codec Input Pin2", NULL, "Codec Mic" },
>> + { "Codec Input Pin1", NULL, "Headset Mic" },
>> +
>> + /* CODEC BE connections */
>> + { "Analog Codec Playback", NULL, "Analog CPU Playback" },
>> + { "Analog CPU Playback", NULL, "codec0_out" },
>> +
>> + { "codec0_in", NULL, "Analog CPU Capture" },
>> + { "Analog CPU Capture", NULL, "Analog Codec Capture" },
>> +
>> { "hifi3", NULL, "iDisp3 Tx"},
>> { "iDisp3 Tx", NULL, "iDisp3_out"},
>> { "hifi2", NULL, "iDisp2 Tx"},
>> @@ -56,6 +81,10 @@ static struct snd_soc_card hda_soc_card = {
>> .owner = THIS_MODULE,
>> .dai_link = skl_hda_be_dai_links,
>> .num_links = ARRAY_SIZE(skl_hda_be_dai_links),
>> + .controls = skl_hda_controls,
>> + .num_controls = ARRAY_SIZE(skl_hda_controls),
>> + .dapm_widgets = skl_hda_widgets,
>> + .num_dapm_widgets = ARRAY_SIZE(skl_hda_widgets),
>> .dapm_routes = skl_hda_map,
>> .num_dapm_routes = ARRAY_SIZE(skl_hda_map),
>> .add_dai_link = skl_hda_add_dai_link,
>> diff --git a/sound/soc/intel/skylake/skl.c b/sound/soc/intel/skylake/skl.c
>> index 1a5ac1b..d6a008b 100644
>> --- a/sound/soc/intel/skylake/skl.c
>> +++ b/sound/soc/intel/skylake/skl.c
>> @@ -36,6 +36,7 @@
>> #include "skl-sst-dsp.h"
>> #include "skl-sst-ipc.h"
>> #include "../../../pci/hda/hda_codec.h"
>> +#include "../../../soc/codecs/hdac_hda.h"
>>
>> static struct skl_machine_pdata skl_dmic_data;
>>
>> @@ -632,7 +633,9 @@ static int probe_codec(struct hdac_bus *bus, int addr)
>> (AC_VERB_PARAMETERS << 8) | AC_PAR_VENDOR_ID;
>> unsigned int res = -1;
>> struct skl *skl = bus_to_skl(bus);
>> + struct hdac_hda_priv *hda_codec;
>> struct hdac_device *hdev;
>> + int err;
>>
>> mutex_lock(&bus->cmd_mutex);
>> snd_hdac_bus_send_cmd(bus, cmd);
>> @@ -642,11 +645,22 @@ static int probe_codec(struct hdac_bus *bus, int
>addr)
>> return -EIO;
>> dev_dbg(bus->dev, "codec #%d probed OK: %x\n", addr, res);
>>
>> - hdev = devm_kzalloc(&skl->pci->dev, sizeof(*hdev), GFP_KERNEL);
>> - if (!hdev)
>> + hda_codec = devm_kzalloc(&skl->pci->dev, sizeof(*hda_codec),
>> +
> GFP_KERNEL);
>> + if (!hda_codec)
>> return -ENOMEM;
>>
>> - return snd_hdac_ext_bus_device_init(bus, addr, hdev);
>> + hda_codec->codec.bus = skl_to_hbus(skl);
>> + hdev = &hda_codec->codec.core;
>> +
>> + err = snd_hdac_ext_bus_device_init(bus, addr, hdev);
>> + if (err < 0)
>> + return err;
>> +
>> + if ((res & 0xFFFF0000) != 0x80860000)
>> + hdev->type = HDA_DEV_LEGACY;
>
>I don't get what this test does?
I am using the legacy HDA codec drivers only for the HDA codecs.
For iDisp I will continue to use the ASoC hdac_hdmi driver, which
is using ASOC BUS.
To make it more clear I will convert the if condition into inline
function with proper name.
Regards,
Rakesh
More information about the Alsa-devel
mailing list