2021-10-11 13:33:47

by Vijendar Mukunda

[permalink] [raw]
Subject: [PATCH 1/3] ASoc: amd: create platform device for VG machine driver

Create platform device for Vangogh machine driver.

Signed-off-by: Vijendar Mukunda <[email protected]>
---
sound/soc/amd/vangogh/acp5x.h | 2 +-
sound/soc/amd/vangogh/pci-acp5x.c | 3 +++
2 files changed, 4 insertions(+), 1 deletion(-)

diff --git a/sound/soc/amd/vangogh/acp5x.h b/sound/soc/amd/vangogh/acp5x.h
index a808635f9740..fe5e1fa98974 100644
--- a/sound/soc/amd/vangogh/acp5x.h
+++ b/sound/soc/amd/vangogh/acp5x.h
@@ -23,7 +23,7 @@
#define ACP_ERR_INTR_MASK 0x20000000
#define ACP_EXT_INTR_STAT_CLEAR_MASK 0xFFFFFFFF

-#define ACP5x_DEVS 3
+#define ACP5x_DEVS 4
#define ACP5x_REG_START 0x1240000
#define ACP5x_REG_END 0x1250200
#define ACP5x_I2STDM_REG_START 0x1242400
diff --git a/sound/soc/amd/vangogh/pci-acp5x.c b/sound/soc/amd/vangogh/pci-acp5x.c
index a57b762d9f2e..137e64c47882 100644
--- a/sound/soc/amd/vangogh/pci-acp5x.c
+++ b/sound/soc/amd/vangogh/pci-acp5x.c
@@ -213,6 +213,9 @@ static int snd_acp5x_probe(struct pci_dev *pci,
pdevinfo[2].num_res = 1;
pdevinfo[2].res = &adata->res[2];

+ pdevinfo[3].name = "acp5x_nu8821_cs35l41_mach";
+ pdevinfo[3].id = 0;
+ pdevinfo[3].parent = &pci->dev;
for (i = 0; i < ACP5x_DEVS; i++) {
adata->pdev[i] =
platform_device_register_full(&pdevinfo[i]);
--
2.25.1


2021-10-11 13:33:47

by Vijendar Mukunda

[permalink] [raw]
Subject: [PATCH 3/3] ASoC: amd: enable Vangogh platform machine driver build

Enable Vangogh platform machine driver build.

Signed-off-by: Vijendar Mukunda <[email protected]>
---
sound/soc/amd/Kconfig | 8 ++++++++
sound/soc/amd/vangogh/Makefile | 2 ++
2 files changed, 10 insertions(+)

diff --git a/sound/soc/amd/Kconfig b/sound/soc/amd/Kconfig
index 49ff5e73e9ba..bab6d6fb3165 100644
--- a/sound/soc/amd/Kconfig
+++ b/sound/soc/amd/Kconfig
@@ -61,3 +61,11 @@ config SND_SOC_AMD_ACP5x

By enabling this flag build will trigger for ACP PCI driver,
ACP DMA driver, CPU DAI driver.
+
+config SND_SOC_AMD_VANGOGH_MACH
+ tristate "AMD Vangogh support for NAU8821 CS35L41"
+ select SND_SOC_NAU8821
+ select SND_SOC_CS35L41_SPI
+ depends on SND_SOC_AMD_ACP5x && I2C
+ help
+ This option enables machine driver for Vangogh platform.
diff --git a/sound/soc/amd/vangogh/Makefile b/sound/soc/amd/vangogh/Makefile
index 3353f93dc610..ae2cda804e2f 100644
--- a/sound/soc/amd/vangogh/Makefile
+++ b/sound/soc/amd/vangogh/Makefile
@@ -3,7 +3,9 @@
snd-pci-acp5x-objs := pci-acp5x.o
snd-acp5x-i2s-objs := acp5x-i2s.o
snd-acp5x-pcm-dma-objs := acp5x-pcm-dma.o
+snd-soc-acp5x-mach-objs := acp5x-nu8821-cs35l41.o

obj-$(CONFIG_SND_SOC_AMD_ACP5x) += snd-pci-acp5x.o
obj-$(CONFIG_SND_SOC_AMD_ACP5x) += snd-acp5x-i2s.o
obj-$(CONFIG_SND_SOC_AMD_ACP5x) += snd-acp5x-pcm-dma.o
+obj-$(CONFIG_SND_SOC_AMD_VANGOGH_MACH) += snd-soc-acp5x-mach.o
--
2.25.1

2021-10-11 13:33:48

by Vijendar Mukunda

[permalink] [raw]
Subject: [PATCH 2/3] ASoC: amd: add vangogh machine driver

Add Vangogh machine driver using NAU8821 & CS35L41 Codecs.

Signed-off-by: Vijendar Mukunda <[email protected]>
---
sound/soc/amd/vangogh/acp5x-nu8821-cs35l41.c | 357 +++++++++++++++++++
1 file changed, 357 insertions(+)
create mode 100644 sound/soc/amd/vangogh/acp5x-nu8821-cs35l41.c

diff --git a/sound/soc/amd/vangogh/acp5x-nu8821-cs35l41.c b/sound/soc/amd/vangogh/acp5x-nu8821-cs35l41.c
new file mode 100644
index 000000000000..a443950ffa1e
--- /dev/null
+++ b/sound/soc/amd/vangogh/acp5x-nu8821-cs35l41.c
@@ -0,0 +1,357 @@
+// SPDX-License-Identifier: GPL-2.0+
+//
+// Machine driver for AMD Vangogh platform using NAU8821 & CS35L41
+//
+//Copyright 2021 Advanced Micro Devices, Inc.
+
+#include <sound/soc.h>
+#include <sound/soc-dapm.h>
+#include <linux/module.h>
+#include <linux/io.h>
+#include <sound/pcm.h>
+#include <sound/pcm_params.h>
+
+#include <sound/jack.h>
+#include <linux/clk.h>
+#include <linux/gpio.h>
+#include <linux/gpio/consumer.h>
+#include <linux/module.h>
+#include <linux/i2c.h>
+#include <linux/input.h>
+#include <linux/io.h>
+#include <linux/acpi.h>
+
+#include "../../codecs/nau8821.h"
+#include "../../codecs/cs35l41.h"
+
+#include "acp5x.h"
+
+#define DRV_NAME "acp5x_nu8821_cs35l41_mach"
+#define DUAL_CHANNEL 2
+#define ACP5X_NUVOTON_CODEC_DAI "nau8821-hifi"
+
+static struct snd_soc_jack vg_headset;
+
+static struct snd_soc_jack_pin acp5x_nau8821_jack_pins[] = {
+ {
+ .pin = "Headphone",
+ .mask = SND_JACK_HEADPHONE,
+ },
+ {
+ .pin = "Headset Mic",
+ .mask = SND_JACK_MICROPHONE,
+ },
+};
+
+static int acp5x_8821_init(struct snd_soc_pcm_runtime *rtd)
+{
+ int ret;
+ struct snd_soc_card *card = rtd->card;
+ struct snd_soc_component *component =
+ asoc_rtd_to_codec(rtd, 0)->component;
+
+ /*
+ * Headset buttons map to the google Reference headset.
+ * These can be configured by userspace.
+ */
+ ret = snd_soc_card_jack_new(card, "Headset Jack",
+ SND_JACK_HEADSET | SND_JACK_BTN_0,
+ &vg_headset, acp5x_nau8821_jack_pins,
+ ARRAY_SIZE(acp5x_nau8821_jack_pins));
+ if (ret) {
+ dev_err(rtd->dev, "Headset Jack creation failed %d\n", ret);
+ return ret;
+ }
+
+ snd_jack_set_key(vg_headset.jack, SND_JACK_BTN_0, KEY_MEDIA);
+ nau8821_enable_jack_detect(component, &vg_headset);
+ return ret;
+}
+
+static int acp5x_cs35l41_init(struct snd_soc_pcm_runtime *rtd)
+{
+ return 0;
+}
+
+static const unsigned int rates[] = {
+ 48000,
+};
+
+static const struct snd_pcm_hw_constraint_list constraints_rates = {
+ .count = ARRAY_SIZE(rates),
+ .list = rates,
+ .mask = 0,
+};
+
+static const unsigned int channels[] = {
+ 2,
+};
+
+static const struct snd_pcm_hw_constraint_list constraints_channels = {
+ .count = ARRAY_SIZE(channels),
+ .list = channels,
+ .mask = 0,
+};
+
+static int acp5x_8821_startup(struct snd_pcm_substream *substream)
+{
+ struct snd_pcm_runtime *runtime = substream->runtime;
+ struct snd_soc_pcm_runtime *rtd = substream->private_data;
+ struct snd_soc_card *card = rtd->card;
+ struct acp5x_platform_info *machine = snd_soc_card_get_drvdata(card);
+
+ machine->play_i2s_instance = I2S_SP_INSTANCE;
+ machine->cap_i2s_instance = I2S_SP_INSTANCE;
+
+ runtime->hw.channels_max = DUAL_CHANNEL;
+ snd_pcm_hw_constraint_list(runtime, 0, SNDRV_PCM_HW_PARAM_CHANNELS,
+ &constraints_channels);
+ snd_pcm_hw_constraint_list(runtime, 0, SNDRV_PCM_HW_PARAM_RATE,
+ &constraints_rates);
+ return 0;
+}
+
+static int acp5x_nau8821_hw_params(struct snd_pcm_substream *substream,
+ struct snd_pcm_hw_params *params)
+{
+ struct snd_soc_pcm_runtime *rtd = substream->private_data;
+ struct snd_soc_card *card = rtd->card;
+ struct snd_soc_dai *codec_dai =
+ snd_soc_card_get_codec_dai(card,
+ ACP5X_NUVOTON_CODEC_DAI);
+ int ret;
+
+ ret = snd_soc_dai_set_sysclk(codec_dai, NAU8821_CLK_FLL_BLK, 0,
+ SND_SOC_CLOCK_IN);
+ if (ret < 0)
+ dev_err(card->dev, "can't set FS clock %d\n", ret);
+ ret = snd_soc_dai_set_pll(codec_dai, 0, 0, snd_soc_params_to_bclk(params),
+ params_rate(params) * 256);
+ if (ret < 0)
+ dev_err(card->dev, "can't set FLL: %d\n", ret);
+
+ return ret;
+}
+
+static int acp5x_cs35l41_startup(struct snd_pcm_substream *substream)
+{
+ struct snd_pcm_runtime *runtime = substream->runtime;
+ struct snd_soc_pcm_runtime *rtd = substream->private_data;
+ struct snd_soc_card *card = rtd->card;
+ struct acp5x_platform_info *machine = snd_soc_card_get_drvdata(card);
+
+ machine->play_i2s_instance = I2S_HS_INSTANCE;
+
+ runtime->hw.channels_max = DUAL_CHANNEL;
+ snd_pcm_hw_constraint_list(runtime, 0, SNDRV_PCM_HW_PARAM_CHANNELS,
+ &constraints_channels);
+ snd_pcm_hw_constraint_list(runtime, 0, SNDRV_PCM_HW_PARAM_RATE,
+ &constraints_rates);
+ return 0;
+}
+
+static int acp5x_cs35l41_hw_params(struct snd_pcm_substream *substream,
+ struct snd_pcm_hw_params *params)
+{
+ struct snd_soc_pcm_runtime *rtd = substream->private_data;
+ struct snd_soc_card *card = rtd->card;
+ struct snd_soc_dai *codec_dai;
+ int ret, i;
+ unsigned int num_codecs = rtd->num_codecs;
+ unsigned int bclk_val;
+
+ for (i = 0; i < num_codecs; i++) {
+ codec_dai = asoc_rtd_to_codec(rtd, i);
+ if ((strcmp(codec_dai->name, "spi-VLV1776:00") == 0) ||
+ (strcmp(codec_dai->name, "spi-VLV1776:01") == 0)) {
+ switch (params_rate(params)) {
+ case 48000:
+ bclk_val = 1536000;
+ break;
+ default:
+ dev_err(card->dev, "Invalid Samplerate:0x%x\n",
+ params_rate(params));
+ return -EINVAL;
+ }
+ ret = snd_soc_component_set_sysclk(codec_dai->component,
+ 0, 0, bclk_val, SND_SOC_CLOCK_IN);
+ if (ret < 0) {
+ dev_err(card->dev, "failed to set sysclk for CS35l41 dai\n");
+ return ret;
+ }
+ }
+ }
+
+ return ret;
+}
+
+static const struct snd_soc_ops acp5x_8821_ops = {
+ .startup = acp5x_8821_startup,
+ .hw_params = acp5x_nau8821_hw_params,
+};
+
+static const struct snd_soc_ops acp5x_cs35l41_play_ops = {
+ .startup = acp5x_cs35l41_startup,
+ .hw_params = acp5x_cs35l41_hw_params,
+};
+
+static struct snd_soc_codec_conf cs35l41_conf[] = {
+ {
+ .dlc = COMP_CODEC_CONF("spi-VLV1776:00"),
+ .name_prefix = "Left",
+ },
+ {
+ .dlc = COMP_CODEC_CONF("spi-VLV1776:01"),
+ .name_prefix = "Right",
+ },
+};
+
+SND_SOC_DAILINK_DEF(acp5x_i2s,
+ DAILINK_COMP_ARRAY(COMP_CPU("acp5x_i2s_playcap.0")));
+
+SND_SOC_DAILINK_DEF(acp5x_bt,
+ DAILINK_COMP_ARRAY(COMP_CPU("acp5x_i2s_playcap.1")));
+
+SND_SOC_DAILINK_DEF(nau8821,
+ DAILINK_COMP_ARRAY(COMP_CODEC("i2c-NVTN2020:00",
+ "nau8821-hifi")));
+
+SND_SOC_DAILINK_DEF(cs35l41,
+ DAILINK_COMP_ARRAY(COMP_CODEC("spi-VLV1776:00", "cs35l41-pcm"),
+ COMP_CODEC("spi-VLV1776:01", "cs35l41-pcm")));
+
+SND_SOC_DAILINK_DEF(platform,
+ DAILINK_COMP_ARRAY(COMP_PLATFORM("acp5x_i2s_dma.0")));
+
+static struct snd_soc_dai_link acp5x_dai[] = {
+ {
+ .name = "acp5x-8825-play",
+ .stream_name = "Playback/Capture",
+ .dai_fmt = SND_SOC_DAIFMT_I2S | SND_SOC_DAIFMT_NB_NF |
+ SND_SOC_DAIFMT_CBC_CFC,
+ .dpcm_playback = 1,
+ .dpcm_capture = 1,
+ .ops = &acp5x_8821_ops,
+ .init = acp5x_8821_init,
+ SND_SOC_DAILINK_REG(acp5x_i2s, nau8821, platform),
+ },
+ {
+ .name = "acp5x-CS35L41-Stereo",
+ .stream_name = "CS35L41 Stereo Playback",
+ .dai_fmt = SND_SOC_DAIFMT_I2S | SND_SOC_DAIFMT_NB_NF |
+ SND_SOC_DAIFMT_CBC_CFC,
+ .dpcm_playback = 1,
+ .playback_only = 1,
+ .ops = &acp5x_cs35l41_play_ops,
+ .init = acp5x_cs35l41_init,
+ SND_SOC_DAILINK_REG(acp5x_bt, cs35l41, platform),
+ },
+};
+
+static int platform_clock_control(struct snd_soc_dapm_widget *w,
+ struct snd_kcontrol *k, int event)
+{
+ struct snd_soc_dapm_context *dapm = w->dapm;
+ struct snd_soc_card *card = dapm->card;
+ struct snd_soc_dai *codec_dai;
+ int ret = 0;
+
+ codec_dai = snd_soc_card_get_codec_dai(card, ACP5X_NUVOTON_CODEC_DAI);
+ if (!codec_dai) {
+ dev_err(card->dev, "Codec dai not found\n");
+ return -EIO;
+ }
+
+ if (SND_SOC_DAPM_EVENT_OFF(event)) {
+ ret = snd_soc_dai_set_sysclk(codec_dai, NAU8821_CLK_INTERNAL,
+ 0, SND_SOC_CLOCK_IN);
+ if (ret < 0) {
+ dev_err(card->dev, "set sysclk err = %d\n", ret);
+ return -EIO;
+ }
+ }
+ return ret;
+}
+
+static const struct snd_kcontrol_new acp5x_8821_controls[] = {
+ SOC_DAPM_PIN_SWITCH("Headphone"),
+ SOC_DAPM_PIN_SWITCH("Headset Mic"),
+ SOC_DAPM_PIN_SWITCH("Int Mic"),
+};
+
+static const struct snd_soc_dapm_widget acp5x_8821_widgets[] = {
+ SND_SOC_DAPM_HP("Headphone", NULL),
+ SND_SOC_DAPM_MIC("Headset Mic", NULL),
+ SND_SOC_DAPM_MIC("Int Mic", NULL),
+ SND_SOC_DAPM_SUPPLY("Platform Clock", SND_SOC_NOPM, 0, 0,
+ platform_clock_control, SND_SOC_DAPM_POST_PMD),
+};
+
+static const struct snd_soc_dapm_route acp5x_8821_audio_route[] = {
+ /* HP jack connectors - unknown if we have jack detection */
+ { "Headphone", NULL, "HPOL" },
+ { "Headphone", NULL, "HPOR" },
+ { "MICL", NULL, "Headset Mic" },
+ { "MICR", NULL, "Headset Mic" },
+ { "DMIC", NULL, "Int Mic" },
+
+ { "Headphone", NULL, "Platform Clock" },
+ { "Headset Mic", NULL, "Platform Clock" },
+ { "Int Mic", NULL, "Platform Clock" },
+};
+
+static struct snd_soc_card acp5x_card = {
+ .name = "acp5x",
+ .owner = THIS_MODULE,
+ .dai_link = acp5x_dai,
+ .num_links = ARRAY_SIZE(acp5x_dai),
+ .dapm_widgets = acp5x_8821_widgets,
+ .num_dapm_widgets = ARRAY_SIZE(acp5x_8821_widgets),
+ .dapm_routes = acp5x_8821_audio_route,
+ .num_dapm_routes = ARRAY_SIZE(acp5x_8821_audio_route),
+ .codec_conf = cs35l41_conf,
+ .num_configs = ARRAY_SIZE(cs35l41_conf),
+ .controls = acp5x_8821_controls,
+ .num_controls = ARRAY_SIZE(acp5x_8821_controls),
+};
+
+static int acp5x_probe(struct platform_device *pdev)
+{
+ int ret;
+ struct acp5x_platform_info *machine;
+ struct snd_soc_card *card;
+
+ machine = devm_kzalloc(&pdev->dev, sizeof(struct acp5x_platform_info),
+ GFP_KERNEL);
+ if (!machine)
+ return -ENOMEM;
+
+ card = &acp5x_card;
+ acp5x_card.dev = &pdev->dev;
+
+ platform_set_drvdata(pdev, card);
+ snd_soc_card_set_drvdata(card, machine);
+
+ ret = devm_snd_soc_register_card(&pdev->dev, card);
+ if (ret) {
+ return dev_err_probe(&pdev->dev, ret,
+ "snd_soc_register_card(%s) failed\n",
+ acp5x_card.name);
+ }
+ return 0;
+}
+
+static struct platform_driver acp5x_mach_driver = {
+ .driver = {
+ .name = "acp5x_nu8821_cs35l41_mach",
+ .pm = &snd_soc_pm_ops,
+ },
+ .probe = acp5x_probe,
+};
+
+module_platform_driver(acp5x_mach_driver);
+
+MODULE_AUTHOR("[email protected]");
+MODULE_DESCRIPTION("NAU8821 & CS35L41 audio support");
+MODULE_LICENSE("GPL v2");
+MODULE_ALIAS("platform:" DRV_NAME);
--
2.25.1

2021-10-11 17:52:16

by Mark Brown

[permalink] [raw]
Subject: Re: [PATCH 1/3] ASoc: amd: create platform device for VG machine driver

On Tue, Oct 12, 2021 at 12:17:01AM +0530, Vijendar Mukunda wrote:

> pdevinfo[2].res = &adata->res[2];
>
> + pdevinfo[3].name = "acp5x_nu8821_cs35l41_mach";
> + pdevinfo[3].id = 0;
> + pdevinfo[3].parent = &pci->dev;

This appears to unconditionally assume that any machine with this
hardware is going to have exactly the same CODEC/amp combination - that
seems like it's going to go badly at some point. Shouldn't there be
some other check that we're instantiating the right machine driver?

BTW your clock appears to be set quite a way into the future (possibly
about 24 hours?) which is confusing things.


Attachments:
(No filename) (632.00 B)
signature.asc (499.00 B)
Download all attachments

2021-10-11 18:22:52

by Vijendar Mukunda

[permalink] [raw]
Subject: Re: [PATCH 1/3] ASoc: amd: create platform device for VG machine driver

On 10/11/21 11:19 PM, Mark Brown wrote:
> On Tue, Oct 12, 2021 at 12:17:01AM +0530, Vijendar Mukunda wrote:
>
>> pdevinfo[2].res = &adata->res[2];
>>
>> + pdevinfo[3].name = "acp5x_nu8821_cs35l41_mach";
>> + pdevinfo[3].id = 0;
>> + pdevinfo[3].parent = &pci->dev;
>
> This appears to unconditionally assume that any machine with this
> hardware is going to have exactly the same CODEC/amp combination - that
> seems like it's going to go badly at some point. Shouldn't there be
> some other check that we're instantiating the right machine driver?
>

we will make the platform device as generic one something like "acp5x_mach".

Currently we have only one target platform with above codec combinations
for this APU series.

When we get new codecs combinations to support, we will address it in
machine driver by adding DMI checks for distinguishing codec combinations.



> BTW your clock appears to be set quite a way into the future (possibly
> about 24 hours?) which is confusing things.
>

Will fix the system time settings.

2021-10-11 18:32:14

by Mark Brown

[permalink] [raw]
Subject: Re: [PATCH 1/3] ASoc: amd: create platform device for VG machine driver

On Mon, Oct 11, 2021 at 11:48:40AM +0530, Mukunda,Vijendar wrote:
> On 10/11/21 11:19 PM, Mark Brown wrote:

> >> + pdevinfo[3].name = "acp5x_nu8821_cs35l41_mach";

> > This appears to unconditionally assume that any machine with this
> > hardware is going to have exactly the same CODEC/amp combination - that
> > seems like it's going to go badly at some point. Shouldn't there be
> > some other check that we're instantiating the right machine driver?

> we will make the platform device as generic one something like "acp5x_mach".

> Currently we have only one target platform with above codec combinations
> for this APU series.

> When we get new codecs combinations to support, we will address it in
> machine driver by adding DMI checks for distinguishing codec combinations.

If that's the case then this machine driver that's being instantiated is
poorly named, and I expect you're going to get issues with assuming a
default here and trying to instantiate this machine on unsuitable
hardware. It'd be better to at least put a bit of the framework in now
and positively indentify systems that can run this machine driver.

It really would be good if ACPI system vendors were to adopt a more
standards based approach to platform enumeration, this really isn't
good. Something more standards based like DT has would be much more
scalable.


Attachments:
(No filename) (1.35 kB)
signature.asc (499.00 B)
Download all attachments

2021-10-11 18:45:09

by Vijendar Mukunda

[permalink] [raw]
Subject: Re: [PATCH 1/3] ASoc: amd: create platform device for VG machine driver

On 10/11/21 11:59 PM, Mark Brown wrote:
> On Mon, Oct 11, 2021 at 11:48:40AM +0530, Mukunda,Vijendar wrote:
>> On 10/11/21 11:19 PM, Mark Brown wrote:
>
>>>> + pdevinfo[3].name = "acp5x_nu8821_cs35l41_mach";
>
>>> This appears to unconditionally assume that any machine with this
>>> hardware is going to have exactly the same CODEC/amp combination - that
>>> seems like it's going to go badly at some point. Shouldn't there be
>>> some other check that we're instantiating the right machine driver?
>
>> we will make the platform device as generic one something like "acp5x_mach".
>
>> Currently we have only one target platform with above codec combinations
>> for this APU series.
>
>> When we get new codecs combinations to support, we will address it in
>> machine driver by adding DMI checks for distinguishing codec combinations.
>
> If that's the case then this machine driver that's being instantiated is
> poorly named, and I expect you're going to get issues with assuming a
> default here and trying to instantiate this machine on unsuitable
> hardware. It'd be better to at least put a bit of the framework in now
> and positively indentify systems that can run this machine driver.

Will address it by adding DMI checks in machine driver code.
>
> It really would be good if ACPI system vendors were to adopt a more
> standards based approach to platform enumeration, this really isn't
> good. Something more standards based like DT has would be much more
> scalable.
>