2023-10-21 14:52:15

by Saba Kareem, Syed

[permalink] [raw]
Subject: [PATCH 01/13] ASoC: amd: acp: Add acp6.3 pci legacy driver support

Add pci legacy driver support and create platform driver for
acp6.3 based platforms.

Signed-off-by: Syed Saba Kareem <[email protected]>
---
sound/soc/amd/acp/acp-legacy-common.c | 4 +
sound/soc/amd/acp/acp-pci.c | 4 +
sound/soc/amd/acp/acp63.c | 314 ++++++++++++++++++++++++++
sound/soc/amd/acp/amd.h | 4 +
4 files changed, 326 insertions(+)
create mode 100644 sound/soc/amd/acp/acp63.c

diff --git a/sound/soc/amd/acp/acp-legacy-common.c b/sound/soc/amd/acp/acp-legacy-common.c
index 217b4c89b975..e16ef94e6336 100644
--- a/sound/soc/amd/acp/acp-legacy-common.c
+++ b/sound/soc/amd/acp/acp-legacy-common.c
@@ -260,6 +260,10 @@ static int acp_power_on(struct acp_chip_info *chip)
acp_pgfsm_stat_reg = ACP6X_PGFSM_STATUS;
acp_pgfsm_ctrl_reg = ACP6X_PGFSM_CONTROL;
break;
+ case ACP63_DEV:
+ acp_pgfsm_stat_reg = ACP63_PGFSM_STATUS;
+ acp_pgfsm_ctrl_reg = ACP63_PGFSM_CONTROL;
+ break;
default:
return -EINVAL;
}
diff --git a/sound/soc/amd/acp/acp-pci.c b/sound/soc/amd/acp/acp-pci.c
index a32c14a109b7..d7fc4a0e9245 100644
--- a/sound/soc/amd/acp/acp-pci.c
+++ b/sound/soc/amd/acp/acp-pci.c
@@ -87,6 +87,10 @@ static int acp_pci_probe(struct pci_dev *pci, const struct pci_device_id *pci_id
chip->name = "acp_asoc_rembrandt";
chip->acp_rev = ACP6X_DEV;
break;
+ case 0x63:
+ chip->name = "acp_asoc_acp63";
+ chip->acp_rev = ACP63_DEV;
+ break;
default:
dev_err(dev, "Unsupported device revision:0x%x\n", pci->revision);
ret = -EINVAL;
diff --git a/sound/soc/amd/acp/acp63.c b/sound/soc/amd/acp/acp63.c
new file mode 100644
index 000000000000..f94348ad863d
--- /dev/null
+++ b/sound/soc/amd/acp/acp63.c
@@ -0,0 +1,314 @@
+// SPDX-License-Identifier: (GPL-2.0-only OR BSD-3-Clause)
+//
+// This file is provided under a dual BSD/GPLv2 license. When using or
+// redistributing this file, you may do so under either license.
+//
+// Copyright(c) 2023 Advanced Micro Devices, Inc.
+//
+// Authors: Syed Saba kareem <[email protected]>
+/*
+ * Hardware interface for ACP6.3 block
+ */
+
+#include <linux/platform_device.h>
+#include <linux/module.h>
+#include <linux/err.h>
+#include <linux/io.h>
+#include <sound/pcm_params.h>
+#include <sound/soc.h>
+#include <sound/soc-dai.h>
+#include <linux/dma-mapping.h>
+#include <linux/pm_runtime.h>
+#include <linux/pci.h>
+#include "amd.h"
+
+#define DRV_NAME "acp_asoc_acp63"
+
+#define CLK_PLL_PWR_REQ_N0 0X0006C2C0
+#define CLK_SPLL_FIELD_2_N0 0X0006C114
+#define CLK_PLL_REQ_N0 0X0006C0DC
+#define CLK_DFSBYPASS_CONTR 0X0006C2C8
+#define CLK_DFS_CNTL_N0 0X0006C1A4
+
+#define PLL_AUTO_STOP_REQ BIT(4)
+#define PLL_AUTO_START_REQ BIT(0)
+#define PLL_FRANCE_EN BIT(4)
+#define EXIT_DPF_BYPASS_0 BIT(16)
+#define EXIT_DPF_BYPASS_1 BIT(17)
+#define CLK0_DIVIDER 0X30
+
+union clk_pll_req_no {
+ struct {
+ u32 fb_mult_int : 9;
+ u32 reserved : 3;
+ u32 pll_spine_div : 4;
+ u32 gb_mult_frac : 16;
+ } bitfields, bits;
+ u32 clk_pll_req_no_reg;
+};
+
+static struct acp_resource rsrc = {
+ .offset = 0,
+ .no_of_ctrls = 2,
+ .irqp_used = 1,
+ .soc_mclk = true,
+ .irq_reg_offset = 0x1a00,
+ .i2s_pin_cfg_offset = 0x1440,
+ .i2s_mode = 0x0a,
+ .scratch_reg_offset = 0x12800,
+ .sram_pte_offset = 0x03802800,
+};
+
+static struct snd_soc_acpi_mach snd_soc_acpi_amd_acp63_acp_machines[] = {
+ {
+ .id = "AMDI0052",
+ .drv_name = "acp63-acp",
+ },
+ {},
+};
+
+static struct snd_soc_dai_driver acp63_dai[] = {
+{
+ .name = "acp-i2s-sp",
+ .id = I2S_SP_INSTANCE,
+ .playback = {
+ .stream_name = "I2S SP Playback",
+ .rates = SNDRV_PCM_RATE_8000_96000,
+ .formats = SNDRV_PCM_FMTBIT_S16_LE | SNDRV_PCM_FMTBIT_S8 |
+ SNDRV_PCM_FMTBIT_U8 | SNDRV_PCM_FMTBIT_S32_LE,
+ .channels_min = 2,
+ .channels_max = 8,
+ .rate_min = 8000,
+ .rate_max = 96000,
+ },
+ .capture = {
+ .stream_name = "I2S SP Capture",
+ .rates = SNDRV_PCM_RATE_8000_48000,
+ .formats = SNDRV_PCM_FMTBIT_S16_LE | SNDRV_PCM_FMTBIT_S8 |
+ SNDRV_PCM_FMTBIT_U8 | SNDRV_PCM_FMTBIT_S32_LE,
+ .channels_min = 2,
+ .channels_max = 2,
+ .rate_min = 8000,
+ .rate_max = 48000,
+ },
+ .ops = &asoc_acp_cpu_dai_ops,
+},
+{
+ .name = "acp-i2s-bt",
+ .id = I2S_BT_INSTANCE,
+ .playback = {
+ .stream_name = "I2S BT Playback",
+ .rates = SNDRV_PCM_RATE_8000_96000,
+ .formats = SNDRV_PCM_FMTBIT_S16_LE | SNDRV_PCM_FMTBIT_S8 |
+ SNDRV_PCM_FMTBIT_U8 | SNDRV_PCM_FMTBIT_S32_LE,
+ .channels_min = 2,
+ .channels_max = 8,
+ .rate_min = 8000,
+ .rate_max = 96000,
+ },
+ .capture = {
+ .stream_name = "I2S BT Capture",
+ .rates = SNDRV_PCM_RATE_8000_48000,
+ .formats = SNDRV_PCM_FMTBIT_S16_LE | SNDRV_PCM_FMTBIT_S8 |
+ SNDRV_PCM_FMTBIT_U8 | SNDRV_PCM_FMTBIT_S32_LE,
+ .channels_min = 2,
+ .channels_max = 2,
+ .rate_min = 8000,
+ .rate_max = 48000,
+ },
+ .ops = &asoc_acp_cpu_dai_ops,
+},
+{
+ .name = "acp-i2s-hs",
+ .id = I2S_HS_INSTANCE,
+ .playback = {
+ .stream_name = "I2S HS Playback",
+ .rates = SNDRV_PCM_RATE_8000_96000,
+ .formats = SNDRV_PCM_FMTBIT_S16_LE | SNDRV_PCM_FMTBIT_S8 |
+ SNDRV_PCM_FMTBIT_U8 | SNDRV_PCM_FMTBIT_S32_LE,
+ .channels_min = 2,
+ .channels_max = 8,
+ .rate_min = 8000,
+ .rate_max = 96000,
+ },
+ .capture = {
+ .stream_name = "I2S HS Capture",
+ .rates = SNDRV_PCM_RATE_8000_48000,
+ .formats = SNDRV_PCM_FMTBIT_S16_LE | SNDRV_PCM_FMTBIT_S8 |
+ SNDRV_PCM_FMTBIT_U8 | SNDRV_PCM_FMTBIT_S32_LE,
+ .channels_min = 2,
+ .channels_max = 8,
+ .rate_min = 8000,
+ .rate_max = 48000,
+ },
+ .ops = &asoc_acp_cpu_dai_ops,
+},
+{
+ .name = "acp-pdm-dmic",
+ .id = DMIC_INSTANCE,
+ .capture = {
+ .rates = SNDRV_PCM_RATE_8000_48000,
+ .formats = SNDRV_PCM_FMTBIT_S32_LE,
+ .channels_min = 2,
+ .channels_max = 2,
+ .rate_min = 8000,
+ .rate_max = 48000,
+ },
+ .ops = &acp_dmic_dai_ops,
+},
+};
+
+static int acp63_i2s_master_clock_generate(struct acp_dev_data *adata)
+{
+ u32 data;
+ union clk_pll_req_no clk_pll;
+ struct pci_dev *smn_dev;
+
+ smn_dev = pci_get_device(PCI_VENDOR_ID_AMD, 0x14E8, NULL);
+ if (!smn_dev)
+ return -ENODEV;
+
+ /* Clk5 pll register values to get mclk as 196.6MHz*/
+ clk_pll.bits.fb_mult_int = 0x31;
+ clk_pll.bits.pll_spine_div = 0;
+ clk_pll.bits.gb_mult_frac = 0x26E9;
+
+ data = smn_read(smn_dev, CLK_PLL_PWR_REQ_N0);
+ smn_write(smn_dev, CLK_PLL_PWR_REQ_N0, data | PLL_AUTO_STOP_REQ);
+
+ data = smn_read(smn_dev, CLK_SPLL_FIELD_2_N0);
+ if (data & PLL_FRANCE_EN)
+ smn_write(smn_dev, CLK_SPLL_FIELD_2_N0, data | PLL_FRANCE_EN);
+
+ smn_write(smn_dev, CLK_PLL_REQ_N0, clk_pll.clk_pll_req_no_reg);
+
+ data = smn_read(smn_dev, CLK_PLL_PWR_REQ_N0);
+ smn_write(smn_dev, CLK_PLL_PWR_REQ_N0, data | PLL_AUTO_START_REQ);
+
+ data = smn_read(smn_dev, CLK_DFSBYPASS_CONTR);
+ smn_write(smn_dev, CLK_DFSBYPASS_CONTR, data | EXIT_DPF_BYPASS_0);
+ smn_write(smn_dev, CLK_DFSBYPASS_CONTR, data | EXIT_DPF_BYPASS_1);
+
+ smn_write(smn_dev, CLK_DFS_CNTL_N0, CLK0_DIVIDER);
+ return 0;
+}
+
+static int acp63_audio_probe(struct platform_device *pdev)
+{
+ struct device *dev = &pdev->dev;
+ struct acp_chip_info *chip;
+ struct acp_dev_data *adata;
+ struct resource *res;
+ int ret;
+
+ chip = dev_get_platdata(&pdev->dev);
+ if (!chip || !chip->base) {
+ dev_err(&pdev->dev, "ACP chip data is NULL\n");
+ return -ENODEV;
+ }
+
+ if (chip->acp_rev != ACP63_DEV) {
+ dev_err(&pdev->dev, "Un-supported ACP Revision %d\n", chip->acp_rev);
+ return -ENODEV;
+ }
+
+ adata = devm_kzalloc(dev, sizeof(struct acp_dev_data), GFP_KERNEL);
+ if (!adata)
+ return -ENOMEM;
+
+ res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "acp_mem");
+ if (!res) {
+ dev_err(&pdev->dev, "IORESOURCE_MEM FAILED\n");
+ return -ENODEV;
+ }
+
+ adata->acp_base = devm_ioremap(&pdev->dev, res->start, resource_size(res));
+ if (!adata->acp_base)
+ return -ENOMEM;
+
+ res = platform_get_resource_byname(pdev, IORESOURCE_IRQ, "acp_dai_irq");
+ if (!res) {
+ dev_err(&pdev->dev, "IORESOURCE_IRQ FAILED\n");
+ return -ENODEV;
+ }
+
+ adata->i2s_irq = res->start;
+ adata->dev = dev;
+ adata->dai_driver = acp63_dai;
+ adata->num_dai = ARRAY_SIZE(acp63_dai);
+ adata->rsrc = &rsrc;
+ adata->machines = snd_soc_acpi_amd_acp63_acp_machines;
+ acp_machine_select(adata);
+ dev_set_drvdata(dev, adata);
+ ret = acp63_i2s_master_clock_generate(adata);
+ if (ret)
+ return ret;
+
+ acp_enable_interrupts(adata);
+ acp_platform_register(dev);
+ pm_runtime_set_autosuspend_delay(&pdev->dev, ACP_SUSPEND_DELAY_MS);
+ pm_runtime_use_autosuspend(&pdev->dev);
+ pm_runtime_mark_last_busy(&pdev->dev);
+ pm_runtime_set_active(&pdev->dev);
+ pm_runtime_enable(&pdev->dev);
+ return 0;
+}
+
+static void acp63_audio_remove(struct platform_device *pdev)
+{
+ struct device *dev = &pdev->dev;
+ struct acp_dev_data *adata = dev_get_drvdata(dev);
+
+ acp_disable_interrupts(adata);
+ acp_platform_unregister(dev);
+ pm_runtime_disable(&pdev->dev);
+}
+
+static int __maybe_unused acp63_pcm_resume(struct device *dev)
+{
+ struct acp_dev_data *adata = dev_get_drvdata(dev);
+ struct acp_stream *stream;
+ struct snd_pcm_substream *substream;
+ snd_pcm_uframes_t buf_in_frames;
+ u64 buf_size;
+
+ acp63_i2s_master_clock_generate(adata);
+ spin_lock(&adata->acp_lock);
+ list_for_each_entry(stream, &adata->stream_list, list) {
+ if (stream) {
+ substream = stream->substream;
+ if (substream && substream->runtime) {
+ buf_in_frames = (substream->runtime->buffer_size);
+ buf_size = frames_to_bytes(substream->runtime, buf_in_frames);
+ config_pte_for_stream(adata, stream);
+ config_acp_dma(adata, stream, buf_size);
+ if (stream->dai_id)
+ restore_acp_i2s_params(substream, adata, stream);
+ else
+ restore_acp_pdm_params(substream, adata);
+ }
+ }
+ }
+ spin_unlock(&adata->acp_lock);
+ return 0;
+}
+
+static const struct dev_pm_ops acp63_dma_pm_ops = {
+ SET_SYSTEM_SLEEP_PM_OPS(NULL, acp63_pcm_resume)
+};
+
+static struct platform_driver acp63_driver = {
+ .probe = acp63_audio_probe,
+ .remove_new = acp63_audio_remove,
+ .driver = {
+ .name = "acp_asoc_acp63",
+ .pm = &acp63_dma_pm_ops,
+ },
+};
+
+module_platform_driver(acp63_driver);
+
+MODULE_DESCRIPTION("AMD ACP acp63 Driver");
+MODULE_IMPORT_NS(SND_SOC_ACP_COMMON);
+MODULE_LICENSE("Dual BSD/GPL");
+MODULE_ALIAS("platform:" DRV_NAME);
diff --git a/sound/soc/amd/acp/amd.h b/sound/soc/amd/acp/amd.h
index d6cfae6ec5f7..487eefa5985f 100644
--- a/sound/soc/amd/acp/amd.h
+++ b/sound/soc/amd/acp/amd.h
@@ -20,6 +20,7 @@

#define ACP3X_DEV 3
#define ACP6X_DEV 6
+#define ACP63_DEV 0x63

#define DMIC_INSTANCE 0x00
#define I2S_SP_INSTANCE 0x01
@@ -95,6 +96,9 @@
#define ACP6X_PGFSM_CONTROL 0x1024
#define ACP6X_PGFSM_STATUS 0x1028

+#define ACP63_PGFSM_CONTROL ACP6X_PGFSM_CONTROL
+#define ACP63_PGFSM_STATUS ACP6X_PGFSM_STATUS
+
#define ACP_SOFT_RST_DONE_MASK 0x00010001

#define ACP_PGFSM_CNTL_POWER_ON_MASK 0x01
--
2.25.1


2023-10-21 14:52:31

by Saba Kareem, Syed

[permalink] [raw]
Subject: [PATCH 03/13] ASoC: amd: acp: add i2s clock generation support for acp6.3 based platforms

Add I2S LRCLK & BCLK generation code for ACP6.3 based platforms.

Signed-off-by: Syed Saba Kareem <[email protected]>
---
sound/soc/amd/acp/acp-i2s.c | 19 ++++++++++++++++---
1 file changed, 16 insertions(+), 3 deletions(-)

diff --git a/sound/soc/amd/acp/acp-i2s.c b/sound/soc/amd/acp/acp-i2s.c
index 59d3a499771a..1185e5aac523 100644
--- a/sound/soc/amd/acp/acp-i2s.c
+++ b/sound/soc/amd/acp/acp-i2s.c
@@ -27,14 +27,19 @@
#define DRV_NAME "acp_i2s_playcap"
#define I2S_MASTER_MODE_ENABLE 1
#define I2S_MODE_ENABLE 0
-#define I2S_FORMAT_MODE GENMASK(1, 1)
#define LRCLK_DIV_FIELD GENMASK(10, 2)
#define BCLK_DIV_FIELD GENMASK(23, 11)
+#define ACP63_LRCLK_DIV_FIELD GENMASK(12, 2)
+#define ACP63_BCLK_DIV_FIELD GENMASK(23, 13)

static inline void acp_set_i2s_clk(struct acp_dev_data *adata, int dai_id)
{
u32 i2s_clk_reg, val;
+ struct acp_chip_info *chip;
+ struct device *dev;

+ dev = adata->dev;
+ chip = dev_get_platdata(dev);
switch (dai_id) {
case I2S_SP_INSTANCE:
i2s_clk_reg = ACP_I2STDM0_MSTRCLKGEN;
@@ -52,8 +57,16 @@ static inline void acp_set_i2s_clk(struct acp_dev_data *adata, int dai_id)

val = I2S_MASTER_MODE_ENABLE;
val |= I2S_MODE_ENABLE & BIT(1);
- val |= FIELD_PREP(LRCLK_DIV_FIELD, adata->lrclk_div);
- val |= FIELD_PREP(BCLK_DIV_FIELD, adata->bclk_div);
+
+ switch (chip->acp_rev) {
+ case ACP63_DEV:
+ val |= FIELD_PREP(ACP63_LRCLK_DIV_FIELD, adata->lrclk_div);
+ val |= FIELD_PREP(ACP63_BCLK_DIV_FIELD, adata->bclk_div);
+ break;
+ default:
+ val |= FIELD_PREP(LRCLK_DIV_FIELD, adata->lrclk_div);
+ val |= FIELD_PREP(BCLK_DIV_FIELD, adata->bclk_div);
+ }
writel(val, adata->acp_base + i2s_clk_reg);
}

--
2.25.1

2023-10-21 14:52:53

by Saba Kareem, Syed

[permalink] [raw]
Subject: [PATCH 02/13] ASoC: amd: acp: refactor acp i2s clock generation code

Refactor acp i2s LRCLK,BCLK generation code and move to commnon file.

Signed-off-by: Syed Saba Kareem <[email protected]>
---
sound/soc/amd/acp/acp-i2s.c | 32 ++++++++++++++++++++++++++++++
sound/soc/amd/acp/amd.h | 39 -------------------------------------
2 files changed, 32 insertions(+), 39 deletions(-)

diff --git a/sound/soc/amd/acp/acp-i2s.c b/sound/soc/amd/acp/acp-i2s.c
index df350014966a..59d3a499771a 100644
--- a/sound/soc/amd/acp/acp-i2s.c
+++ b/sound/soc/amd/acp/acp-i2s.c
@@ -20,10 +20,42 @@
#include <sound/soc.h>
#include <sound/soc-dai.h>
#include <linux/dma-mapping.h>
+#include <linux/bitfield.h>

#include "amd.h"

#define DRV_NAME "acp_i2s_playcap"
+#define I2S_MASTER_MODE_ENABLE 1
+#define I2S_MODE_ENABLE 0
+#define I2S_FORMAT_MODE GENMASK(1, 1)
+#define LRCLK_DIV_FIELD GENMASK(10, 2)
+#define BCLK_DIV_FIELD GENMASK(23, 11)
+
+static inline void acp_set_i2s_clk(struct acp_dev_data *adata, int dai_id)
+{
+ u32 i2s_clk_reg, val;
+
+ switch (dai_id) {
+ case I2S_SP_INSTANCE:
+ i2s_clk_reg = ACP_I2STDM0_MSTRCLKGEN;
+ break;
+ case I2S_BT_INSTANCE:
+ i2s_clk_reg = ACP_I2STDM1_MSTRCLKGEN;
+ break;
+ case I2S_HS_INSTANCE:
+ i2s_clk_reg = ACP_I2STDM2_MSTRCLKGEN;
+ break;
+ default:
+ i2s_clk_reg = ACP_I2STDM0_MSTRCLKGEN;
+ break;
+ }
+
+ val = I2S_MASTER_MODE_ENABLE;
+ val |= I2S_MODE_ENABLE & BIT(1);
+ val |= FIELD_PREP(LRCLK_DIV_FIELD, adata->lrclk_div);
+ val |= FIELD_PREP(BCLK_DIV_FIELD, adata->bclk_div);
+ writel(val, adata->acp_base + i2s_clk_reg);
+}

static int acp_i2s_set_fmt(struct snd_soc_dai *cpu_dai,
unsigned int fmt)
diff --git a/sound/soc/amd/acp/amd.h b/sound/soc/amd/acp/amd.h
index 487eefa5985f..87d1e1f7d6b6 100644
--- a/sound/soc/amd/acp/amd.h
+++ b/sound/soc/amd/acp/amd.h
@@ -188,17 +188,6 @@ struct acp_dev_data {
u32 xfer_rx_resolution[3];
};

-union acp_i2stdm_mstrclkgen {
- struct {
- u32 i2stdm_master_mode : 1;
- u32 i2stdm_format_mode : 1;
- u32 i2stdm_lrclk_div_val : 9;
- u32 i2stdm_bclk_div_val : 11;
- u32:10;
- } bitfields, bits;
- u32 u32_all;
-};
-
extern const struct snd_soc_dai_ops asoc_acp_cpu_dai_ops;
extern const struct snd_soc_dai_ops acp_dmic_dai_ops;

@@ -276,32 +265,4 @@ static inline u64 acp_get_byte_count(struct acp_dev_data *adata, int dai_id, int
POINTER_RETURN_BYTES:
return byte_count;
}
-
-static inline void acp_set_i2s_clk(struct acp_dev_data *adata, int dai_id)
-{
- union acp_i2stdm_mstrclkgen mclkgen;
- u32 master_reg;
-
- switch (dai_id) {
- case I2S_SP_INSTANCE:
- master_reg = ACP_I2STDM0_MSTRCLKGEN;
- break;
- case I2S_BT_INSTANCE:
- master_reg = ACP_I2STDM1_MSTRCLKGEN;
- break;
- case I2S_HS_INSTANCE:
- master_reg = ACP_I2STDM2_MSTRCLKGEN;
- break;
- default:
- master_reg = ACP_I2STDM0_MSTRCLKGEN;
- break;
- }
-
- mclkgen.bits.i2stdm_master_mode = 0x1;
- mclkgen.bits.i2stdm_format_mode = 0x00;
-
- mclkgen.bits.i2stdm_bclk_div_val = adata->bclk_div;
- mclkgen.bits.i2stdm_lrclk_div_val = adata->lrclk_div;
- writel(mclkgen.u32_all, adata->acp_base + master_reg);
-}
#endif
--
2.25.1

2023-10-21 14:53:01

by Saba Kareem, Syed

[permalink] [raw]
Subject: [PATCH 05/13] ASoC: amd: acp: add Kconfig options for acp6.3 based platform driver

ACP6.3 based platform legacy drivers can be built by selecting
necessary kernel config option. This patch enables build support
of the same.

Signed-off-by: Syed Saba Kareem <[email protected]>
---
sound/soc/amd/acp/Kconfig | 11 +++++++++++
sound/soc/amd/acp/Makefile | 2 ++
2 files changed, 13 insertions(+)

diff --git a/sound/soc/amd/acp/Kconfig b/sound/soc/amd/acp/Kconfig
index 631cdf96d637..c0b2a2df8f80 100644
--- a/sound/soc/amd/acp/Kconfig
+++ b/sound/soc/amd/acp/Kconfig
@@ -57,6 +57,17 @@ config SND_AMD_ASOC_REMBRANDT
Say Y if you want to enable AUDIO on Rembrandt
If unsure select "N".

+config SND_AMD_ASOC_ACP63
+ tristate "AMD ACP ASOC ACP6.3 Support"
+ select SND_SOC_AMD_ACP_PCM
+ select SND_SOC_AMD_ACP_I2S
+ select SND_SOC_AMD_ACP_PDM
+ depends on X86 && PCI
+ help
+ This option enables Acp6.3 I2S support on AMD platform.
+ Say Y if you want to enable AUDIO on ACP6.3
+ If unsure select "N".
+
config SND_SOC_AMD_MACH_COMMON
tristate
depends on X86 && PCI && I2C
diff --git a/sound/soc/amd/acp/Makefile b/sound/soc/amd/acp/Makefile
index dc70691bc293..dd85700f1c5f 100644
--- a/sound/soc/amd/acp/Makefile
+++ b/sound/soc/amd/acp/Makefile
@@ -14,6 +14,7 @@ snd-acp-pci-objs := acp-pci.o
#platform specific driver
snd-acp-renoir-objs := acp-renoir.o
snd-acp-rembrandt-objs := acp-rembrandt.o
+snd-acp63-objs := acp63.o

#machine specific driver
snd-acp-mach-objs := acp-mach-common.o
@@ -28,6 +29,7 @@ obj-$(CONFIG_SND_SOC_AMD_ACP_PCI) += snd-acp-pci.o

obj-$(CONFIG_SND_AMD_ASOC_RENOIR) += snd-acp-renoir.o
obj-$(CONFIG_SND_AMD_ASOC_REMBRANDT) += snd-acp-rembrandt.o
+obj-$(CONFIG_SND_AMD_ASOC_ACP63) += snd-acp63.o

obj-$(CONFIG_SND_SOC_AMD_MACH_COMMON) += snd-acp-mach.o
obj-$(CONFIG_SND_SOC_AMD_LEGACY_MACH) += snd-acp-legacy-mach.o
--
2.25.1

2023-10-21 14:53:01

by Saba Kareem, Syed

[permalink] [raw]
Subject: [PATCH 04/13] ASoC: amd: acp: add machine driver support for acp6.3 platform

add legacy machine driver support for acp6.3 based platform.

Signed-off-by: Syed Saba Kareem <[email protected]>
---
sound/soc/amd/acp/acp-mach-common.c | 15 +++++++++++++++
sound/soc/amd/acp/acp-mach.h | 1 +
2 files changed, 16 insertions(+)

diff --git a/sound/soc/amd/acp/acp-mach-common.c b/sound/soc/amd/acp/acp-mach-common.c
index 9def77bfc608..88e91af47532 100644
--- a/sound/soc/amd/acp/acp-mach-common.c
+++ b/sound/soc/amd/acp/acp-mach-common.c
@@ -1260,6 +1260,12 @@ static struct snd_soc_dai_link_component platform_rmb_component[] = {
}
};

+static struct snd_soc_dai_link_component platform_acp63_component[] = {
+ {
+ .name = "acp_asoc_acp63.0",
+ }
+};
+
static struct snd_soc_dai_link_component sof_component[] = {
{
.name = "0000:04:00.5",
@@ -1570,6 +1576,9 @@ int acp_legacy_dai_links_create(struct snd_soc_card *card)
if (drv_data->platform == REMBRANDT) {
links[i].platforms = platform_rmb_component;
links[i].num_platforms = ARRAY_SIZE(platform_rmb_component);
+ } else if (drv_data->platform == ACP63) {
+ links[i].platforms = platform_acp63_component;
+ links[i].num_platforms = ARRAY_SIZE(platform_acp63_component);
} else {
links[i].platforms = platform_component;
links[i].num_platforms = ARRAY_SIZE(platform_component);
@@ -1634,6 +1643,9 @@ int acp_legacy_dai_links_create(struct snd_soc_card *card)
if (drv_data->platform == REMBRANDT) {
links[i].platforms = platform_rmb_component;
links[i].num_platforms = ARRAY_SIZE(platform_rmb_component);
+ } else if (drv_data->platform == ACP63) {
+ links[i].platforms = platform_acp63_component;
+ links[i].num_platforms = ARRAY_SIZE(platform_acp63_component);
} else {
links[i].platforms = platform_component;
links[i].num_platforms = ARRAY_SIZE(platform_component);
@@ -1677,6 +1689,9 @@ int acp_legacy_dai_links_create(struct snd_soc_card *card)
if (drv_data->platform == REMBRANDT) {
links[i].platforms = platform_rmb_component;
links[i].num_platforms = ARRAY_SIZE(platform_rmb_component);
+ } else if (drv_data->platform == ACP63) {
+ links[i].platforms = platform_acp63_component;
+ links[i].num_platforms = ARRAY_SIZE(platform_acp63_component);
} else {
links[i].platforms = platform_component;
links[i].num_platforms = ARRAY_SIZE(platform_component);
diff --git a/sound/soc/amd/acp/acp-mach.h b/sound/soc/amd/acp/acp-mach.h
index b0a3f6bd172f..69db61f12eca 100644
--- a/sound/soc/amd/acp/acp-mach.h
+++ b/sound/soc/amd/acp/acp-mach.h
@@ -53,6 +53,7 @@ enum codec_endpoints {
enum platform_end_point {
RENOIR = 0,
REMBRANDT,
+ ACP63,
};

struct acp_mach_ops {
--
2.25.1

2023-10-21 14:53:11

by Saba Kareem, Syed

[permalink] [raw]
Subject: [PATCH 07/13] ASoC: amd: acp: add platform and flag data to acp data structure

add name of the platform and flag data in private data structure.
name of the platform will be used to differentiate platforms where as
flag will be used to know what kind of endpoint configuration is selected
where its legacy(I2S + PDM) or only ACP PDM.

Signed-off-by: Syed Saba Kareem <[email protected]>
---
sound/soc/amd/acp/acp-rembrandt.c | 5 ++++-
sound/soc/amd/acp/acp-renoir.c | 3 +++
sound/soc/amd/acp/acp63.c | 4 ++++
sound/soc/amd/acp/amd.h | 2 ++
4 files changed, 13 insertions(+), 1 deletion(-)

diff --git a/sound/soc/amd/acp/acp-rembrandt.c b/sound/soc/amd/acp/acp-rembrandt.c
index 1bf7b2e68a11..ef5fe6438efd 100644
--- a/sound/soc/amd/acp/acp-rembrandt.c
+++ b/sound/soc/amd/acp/acp-rembrandt.c
@@ -23,6 +23,8 @@
#include <linux/pm_runtime.h>

#include "amd.h"
+#include "../mach-config.h"
+#include "acp-mach.h"

#define DRV_NAME "acp_asoc_rembrandt"

@@ -226,7 +228,8 @@ static int rembrandt_audio_probe(struct platform_device *pdev)
adata->dai_driver = acp_rmb_dai;
adata->num_dai = ARRAY_SIZE(acp_rmb_dai);
adata->rsrc = &rsrc;
-
+ adata->platform = REMBRANDT;
+ adata->flag = chip->flag;
adata->machines = snd_soc_acpi_amd_rmb_acp_machines;
acp_machine_select(adata);

diff --git a/sound/soc/amd/acp/acp-renoir.c b/sound/soc/amd/acp/acp-renoir.c
index b15cbdf7fa9b..a591482a0726 100644
--- a/sound/soc/amd/acp/acp-renoir.c
+++ b/sound/soc/amd/acp/acp-renoir.c
@@ -22,6 +22,7 @@
#include <linux/dma-mapping.h>

#include "amd.h"
+#include "acp-mach.h"

#define DRV_NAME "acp_asoc_renoir"

@@ -185,6 +186,8 @@ static int renoir_audio_probe(struct platform_device *pdev)
adata->dai_driver = acp_renoir_dai;
adata->num_dai = ARRAY_SIZE(acp_renoir_dai);
adata->rsrc = &rsrc;
+ adata->platform = RENOIR;
+ adata->flag = chip->flag;

adata->machines = snd_soc_acpi_amd_acp_machines;
acp_machine_select(adata);
diff --git a/sound/soc/amd/acp/acp63.c b/sound/soc/amd/acp/acp63.c
index f94348ad863d..0cec6ecaadfa 100644
--- a/sound/soc/amd/acp/acp63.c
+++ b/sound/soc/amd/acp/acp63.c
@@ -21,6 +21,8 @@
#include <linux/pm_runtime.h>
#include <linux/pci.h>
#include "amd.h"
+#include "acp-mach.h"
+#include "../mach-config.h"

#define DRV_NAME "acp_asoc_acp63"

@@ -237,6 +239,8 @@ static int acp63_audio_probe(struct platform_device *pdev)
adata->dai_driver = acp63_dai;
adata->num_dai = ARRAY_SIZE(acp63_dai);
adata->rsrc = &rsrc;
+ adata->platform = ACP63;
+ adata->flag = chip->flag;
adata->machines = snd_soc_acpi_amd_acp63_acp_machines;
acp_machine_select(adata);
dev_set_drvdata(dev, adata);
diff --git a/sound/soc/amd/acp/amd.h b/sound/soc/amd/acp/amd.h
index 2ffe1effc6b5..62d0793027f2 100644
--- a/sound/soc/amd/acp/amd.h
+++ b/sound/soc/amd/acp/amd.h
@@ -187,6 +187,8 @@ struct acp_dev_data {
u32 tdm_rx_fmt[3];
u32 xfer_tx_resolution[3];
u32 xfer_rx_resolution[3];
+ unsigned int flag;
+ unsigned int platform;
};

enum acp_config {
--
2.25.1

2023-10-21 14:53:11

by Saba Kareem, Syed

[permalink] [raw]
Subject: [PATCH 06/13] ASoC: amd: acp: add code for scanning acp pdm controller

Add common code for scanning acp pdm controller and create
platform device for the same.

Signed-off-by: Syed Saba Kareem <[email protected]>
---
sound/soc/amd/acp/acp-legacy-common.c | 52 +++++++++++++++++++++++++++
sound/soc/amd/acp/acp-pci.c | 11 +++++-
sound/soc/amd/acp/amd.h | 22 ++++++++++++
sound/soc/amd/acp/chip_offset_byte.h | 1 +
sound/soc/amd/mach-config.h | 1 +
5 files changed, 86 insertions(+), 1 deletion(-)

diff --git a/sound/soc/amd/acp/acp-legacy-common.c b/sound/soc/amd/acp/acp-legacy-common.c
index e16ef94e6336..af85a153a770 100644
--- a/sound/soc/amd/acp/acp-legacy-common.c
+++ b/sound/soc/amd/acp/acp-legacy-common.c
@@ -16,6 +16,10 @@
#include <linux/pci.h>
#include <linux/export.h>

+#define ACP_RENOIR_PDM_ADDR 0x02
+#define ACP_REMBRANDT_PDM_ADDR 0x03
+#define ACP63_PDM_ADDR 0x02
+
void acp_enable_interrupts(struct acp_dev_data *adata)
{
struct acp_resource *rsrc = adata->rsrc;
@@ -348,4 +352,52 @@ int smn_read(struct pci_dev *dev, u32 smn_addr)
}
EXPORT_SYMBOL_NS_GPL(smn_read, SND_SOC_ACP_COMMON);

+int check_acp_pdm(struct pci_dev *pci, struct acp_chip_info *chip)
+{
+ struct acpi_device *pdm_dev;
+ const union acpi_object *obj;
+ u32 pdm_addr, val;
+
+ val = readl(chip->base + ACP_PIN_CONFIG);
+ switch (val) {
+ case ACP_CONFIG_4:
+ case ACP_CONFIG_5:
+ case ACP_CONFIG_6:
+ case ACP_CONFIG_7:
+ case ACP_CONFIG_8:
+ case ACP_CONFIG_10:
+ case ACP_CONFIG_11:
+ case ACP_CONFIG_12:
+ case ACP_CONFIG_13:
+ case ACP_CONFIG_14:
+ break;
+ default:
+ return -EINVAL;
+ }
+
+ switch (chip->acp_rev) {
+ case ACP3X_DEV:
+ pdm_addr = ACP_RENOIR_PDM_ADDR;
+ break;
+ case ACP6X_DEV:
+ pdm_addr = ACP_REMBRANDT_PDM_ADDR;
+ break;
+ case ACP63_DEV:
+ pdm_addr = ACP63_PDM_ADDR;
+ break;
+ default:
+ return -EINVAL;
+ }
+
+ pdm_dev = acpi_find_child_device(ACPI_COMPANION(&pci->dev), pdm_addr, 0);
+ if (pdm_dev) {
+ if (!acpi_dev_get_property(pdm_dev, "acp-audio-device-type",
+ ACPI_TYPE_INTEGER, &obj) &&
+ obj->integer.value == pdm_addr)
+ return 0;
+ }
+ return -ENODEV;
+}
+EXPORT_SYMBOL_NS_GPL(check_acp_pdm, SND_SOC_ACP_COMMON);
+
MODULE_LICENSE("Dual BSD/GPL");
diff --git a/sound/soc/amd/acp/acp-pci.c b/sound/soc/amd/acp/acp-pci.c
index d7fc4a0e9245..bbf079d47dc4 100644
--- a/sound/soc/amd/acp/acp-pci.c
+++ b/sound/soc/amd/acp/acp-pci.c
@@ -55,7 +55,7 @@ static int acp_pci_probe(struct pci_dev *pci, const struct pci_device_id *pci_id
int ret;

flag = snd_amd_acp_find_config(pci);
- if (flag != FLAG_AMD_LEGACY)
+ if (flag != FLAG_AMD_LEGACY && flag != FLAG_AMD_LEGACY_ONLY_DMIC)
return -ENODEV;

chip = devm_kzalloc(&pci->dev, sizeof(*chip), GFP_KERNEL);
@@ -129,6 +129,13 @@ static int acp_pci_probe(struct pci_dev *pci, const struct pci_device_id *pci_id
}
}

+ if (flag == FLAG_AMD_LEGACY_ONLY_DMIC) {
+ ret = check_acp_pdm(pci, chip);
+ if (ret < 0)
+ goto skip_pdev_creation;
+ }
+
+ chip->flag = flag;
memset(&pdevinfo, 0, sizeof(pdevinfo));

pdevinfo.name = chip->name;
@@ -145,6 +152,8 @@ static int acp_pci_probe(struct pci_dev *pci, const struct pci_device_id *pci_id
ret = PTR_ERR(pdev);
goto unregister_dmic_dev;
}
+
+skip_pdev_creation:
chip->chip_pdev = pdev;
dev_set_drvdata(&pci->dev, chip);
pm_runtime_set_autosuspend_delay(&pci->dev, 2000);
diff --git a/sound/soc/amd/acp/amd.h b/sound/soc/amd/acp/amd.h
index 87d1e1f7d6b6..2ffe1effc6b5 100644
--- a/sound/soc/amd/acp/amd.h
+++ b/sound/soc/amd/acp/amd.h
@@ -133,6 +133,7 @@ struct acp_chip_info {
unsigned int acp_rev; /* ACP Revision id */
void __iomem *base; /* ACP memory PCI base */
struct platform_device *chip_pdev;
+ unsigned int flag; /* Distinguish b/w Legacy or Only PDM */
};

struct acp_stream {
@@ -188,6 +189,25 @@ struct acp_dev_data {
u32 xfer_rx_resolution[3];
};

+enum acp_config {
+ ACP_CONFIG_0 = 0,
+ ACP_CONFIG_1,
+ ACP_CONFIG_2,
+ ACP_CONFIG_3,
+ ACP_CONFIG_4,
+ ACP_CONFIG_5,
+ ACP_CONFIG_6,
+ ACP_CONFIG_7,
+ ACP_CONFIG_8,
+ ACP_CONFIG_9,
+ ACP_CONFIG_10,
+ ACP_CONFIG_11,
+ ACP_CONFIG_12,
+ ACP_CONFIG_13,
+ ACP_CONFIG_14,
+ ACP_CONFIG_15,
+};
+
extern const struct snd_soc_dai_ops asoc_acp_cpu_dai_ops;
extern const struct snd_soc_dai_ops acp_dmic_dai_ops;

@@ -214,6 +234,8 @@ void restore_acp_pdm_params(struct snd_pcm_substream *substream,
int restore_acp_i2s_params(struct snd_pcm_substream *substream,
struct acp_dev_data *adata, struct acp_stream *stream);

+int check_acp_pdm(struct pci_dev *pci, struct acp_chip_info *chip);
+
static inline u64 acp_get_byte_count(struct acp_dev_data *adata, int dai_id, int direction)
{
u64 byte_count = 0, low = 0, high = 0;
diff --git a/sound/soc/amd/acp/chip_offset_byte.h b/sound/soc/amd/acp/chip_offset_byte.h
index ce3948e0679c..cfd6c4d07594 100644
--- a/sound/soc/amd/acp/chip_offset_byte.h
+++ b/sound/soc/amd/acp/chip_offset_byte.h
@@ -19,6 +19,7 @@
#define ACP_PGFSM_STATUS 0x1420
#define ACP_SOFT_RESET 0x1000
#define ACP_CONTROL 0x1004
+#define ACP_PIN_CONFIG 0x1440

#define ACP_EXTERNAL_INTR_REG_ADDR(adata, offset, ctrl) \
(adata->acp_base + adata->rsrc->irq_reg_offset + offset + (ctrl * 0x04))
diff --git a/sound/soc/amd/mach-config.h b/sound/soc/amd/mach-config.h
index d392e6d6e6e1..e6b520459164 100644
--- a/sound/soc/amd/mach-config.h
+++ b/sound/soc/amd/mach-config.h
@@ -15,6 +15,7 @@
#define FLAG_AMD_SOF BIT(1)
#define FLAG_AMD_SOF_ONLY_DMIC BIT(2)
#define FLAG_AMD_LEGACY BIT(3)
+#define FLAG_AMD_LEGACY_ONLY_DMIC BIT(4)

#define ACP_PCI_DEV_ID 0x15E2

--
2.25.1

2023-10-21 14:53:20

by Saba Kareem, Syed

[permalink] [raw]
Subject: [PATCH 09/13] ASoC: amd: acp: add machine driver support for pdm use case

add pdm use case machine driver support

Signed-off-by: Syed Saba Kareem <[email protected]>
---
sound/soc/amd/acp/acp-legacy-mach.c | 12 ++++++++++++
sound/soc/amd/acp/acp-platform.c | 29 ++++++++++++++++++-----------
2 files changed, 30 insertions(+), 11 deletions(-)

diff --git a/sound/soc/amd/acp/acp-legacy-mach.c b/sound/soc/amd/acp/acp-legacy-mach.c
index 1ab3edffe0ce..47c3b5f167f5 100644
--- a/sound/soc/amd/acp/acp-legacy-mach.c
+++ b/sound/soc/amd/acp/acp-legacy-mach.c
@@ -84,6 +84,11 @@ static struct acp_card_drvdata rt5682s_rt1019_rmb_data = {
.tdm_mode = false,
};

+static struct acp_card_drvdata acp_dmic_data = {
+ .dmic_cpu_id = DMIC,
+ .dmic_codec_id = DMIC,
+};
+
static bool acp_asoc_init_ops(struct acp_card_drvdata *priv)
{
bool has_ops = false;
@@ -165,6 +170,8 @@ static int acp_asoc_probe(struct platform_device *pdev)
card->name, ret);
goto out;
}
+ if (!strcmp(pdev->name, "acp-pdm-mach"))
+ acp_card_drvdata->platform = *((int *)dev->platform_data);

dmi_id = dmi_first_match(acp_quirk_table);
if (dmi_id && dmi_id->driver_data)
@@ -214,6 +221,10 @@ static const struct platform_device_id board_ids[] = {
.name = "rmb-rt5682s-rt1019",
.driver_data = (kernel_ulong_t)&rt5682s_rt1019_rmb_data,
},
+ {
+ .name = "acp-pdm-mach",
+ .driver_data = (kernel_ulong_t)&acp_dmic_data,
+ },
{ }
};
static struct platform_driver acp_asoc_audio = {
@@ -235,4 +246,5 @@ MODULE_ALIAS("platform:acp3xalc5682s1019");
MODULE_ALIAS("platform:acp3x-es83xx");
MODULE_ALIAS("platform:rmb-nau8825-max");
MODULE_ALIAS("platform:rmb-rt5682s-rt1019");
+MODULE_ALIAS("platform:acp-pdm-mach");
MODULE_LICENSE("GPL v2");
diff --git a/sound/soc/amd/acp/acp-platform.c b/sound/soc/amd/acp/acp-platform.c
index f516daf6fef4..aaac8aa744cb 100644
--- a/sound/soc/amd/acp/acp-platform.c
+++ b/sound/soc/amd/acp/acp-platform.c
@@ -21,6 +21,8 @@
#include <linux/dma-mapping.h>

#include "amd.h"
+#include "../mach-config.h"
+#include "acp-mach.h"

#define DRV_NAME "acp_i2s_dma"

@@ -69,20 +71,25 @@ static const struct snd_pcm_hardware acp_pcm_hardware_capture = {
int acp_machine_select(struct acp_dev_data *adata)
{
struct snd_soc_acpi_mach *mach;
- int size;
-
- size = sizeof(*adata->machines);
- mach = snd_soc_acpi_find_machine(adata->machines);
- if (!mach) {
- dev_err(adata->dev, "warning: No matching ASoC machine driver found\n");
- return -EINVAL;
+ int size, platform;
+
+ if (adata->flag == FLAG_AMD_LEGACY_ONLY_DMIC) {
+ platform = adata->platform;
+ adata->mach_dev = platform_device_register_data(adata->dev, "acp-pdm-mach",
+ PLATFORM_DEVID_NONE, &platform,
+ sizeof(platform));
+ } else {
+ size = sizeof(*adata->machines);
+ mach = snd_soc_acpi_find_machine(adata->machines);
+ if (!mach) {
+ dev_err(adata->dev, "warning: No matching ASoC machine driver found\n");
+ return -EINVAL;
+ }
+ adata->mach_dev = platform_device_register_data(adata->dev, mach->drv_name,
+ PLATFORM_DEVID_NONE, mach, size);
}
-
- adata->mach_dev = platform_device_register_data(adata->dev, mach->drv_name,
- PLATFORM_DEVID_NONE, mach, size);
if (IS_ERR(adata->mach_dev))
dev_warn(adata->dev, "Unable to register Machine device\n");
-
return 0;
}
EXPORT_SYMBOL_NS_GPL(acp_machine_select, SND_SOC_ACP_COMMON);
--
2.25.1

2023-10-21 14:53:38

by Saba Kareem, Syed

[permalink] [raw]
Subject: [PATCH 10/13] ASoC: amd: acp: change acp-deinit function arguments

acp-deinit function will not be same for all platforms.
To make platform specific changes in acp-deinit
function, instead of passing base address pass chip
structure which contains acp_rev feild.
chip->acp_rev will be used to add platform specific code
in acp-deinit().

Signed-off-by: Syed Saba Kareem <[email protected]>
---
sound/soc/amd/acp/acp-legacy-common.c | 6 +++---
sound/soc/amd/acp/acp-pci.c | 4 ++--
sound/soc/amd/acp/amd.h | 2 +-
3 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/sound/soc/amd/acp/acp-legacy-common.c b/sound/soc/amd/acp/acp-legacy-common.c
index af85a153a770..cba0aabefb34 100644
--- a/sound/soc/amd/acp/acp-legacy-common.c
+++ b/sound/soc/amd/acp/acp-legacy-common.c
@@ -320,16 +320,16 @@ int acp_init(struct acp_chip_info *chip)
}
EXPORT_SYMBOL_NS_GPL(acp_init, SND_SOC_ACP_COMMON);

-int acp_deinit(void __iomem *base)
+int acp_deinit(struct acp_chip_info *chip)
{
int ret;

/* Reset */
- ret = acp_reset(base);
+ ret = acp_reset(chip->base);
if (ret)
return ret;

- writel(0, base + ACP_CONTROL);
+ writel(0, chip->base + ACP_CONTROL);
return 0;
}
EXPORT_SYMBOL_NS_GPL(acp_deinit, SND_SOC_ACP_COMMON);
diff --git a/sound/soc/amd/acp/acp-pci.c b/sound/soc/amd/acp/acp-pci.c
index bbf079d47dc4..696c9ee6786f 100644
--- a/sound/soc/amd/acp/acp-pci.c
+++ b/sound/soc/amd/acp/acp-pci.c
@@ -178,7 +178,7 @@ static int __maybe_unused snd_acp_suspend(struct device *dev)
int ret;

chip = dev_get_drvdata(dev);
- ret = acp_deinit(chip->base);
+ ret = acp_deinit(chip);
if (ret)
dev_err(dev, "ACP de-init failed\n");
return ret;
@@ -219,7 +219,7 @@ static void acp_pci_remove(struct pci_dev *pci)
platform_device_unregister(dmic_dev);
if (pdev)
platform_device_unregister(pdev);
- ret = acp_deinit(chip->base);
+ ret = acp_deinit(chip);
if (ret)
dev_err(&pci->dev, "ACP de-init failed\n");
}
diff --git a/sound/soc/amd/acp/amd.h b/sound/soc/amd/acp/amd.h
index 62d0793027f2..e3bb470d1f32 100644
--- a/sound/soc/amd/acp/amd.h
+++ b/sound/soc/amd/acp/amd.h
@@ -222,7 +222,7 @@ int smn_read(struct pci_dev *dev, u32 smn_addr);
int smn_write(struct pci_dev *dev, u32 smn_addr, u32 data);

int acp_init(struct acp_chip_info *chip);
-int acp_deinit(void __iomem *base);
+int acp_deinit(struct acp_chip_info *chip);
void acp_enable_interrupts(struct acp_dev_data *adata);
void acp_disable_interrupts(struct acp_dev_data *adata);
/* Machine configuration */
--
2.25.1

2023-10-21 14:53:46

by Saba Kareem, Syed

[permalink] [raw]
Subject: [PATCH 08/13] ASoC: amd: acp: add condition check for i2s clock generation

for only PDM endpoint i2s master clock is not required.
Add a condition check for the same based on chip flag value.

Signed-off-by: Syed Saba Kareem <[email protected]>
---
sound/soc/amd/acp/acp-rembrandt.c | 12 ++++++++++--
sound/soc/amd/acp/acp63.c | 12 ++++++++----
2 files changed, 18 insertions(+), 6 deletions(-)

diff --git a/sound/soc/amd/acp/acp-rembrandt.c b/sound/soc/amd/acp/acp-rembrandt.c
index ef5fe6438efd..158f819f8da4 100644
--- a/sound/soc/amd/acp/acp-rembrandt.c
+++ b/sound/soc/amd/acp/acp-rembrandt.c
@@ -191,6 +191,7 @@ static int rembrandt_audio_probe(struct platform_device *pdev)
struct acp_chip_info *chip;
struct acp_dev_data *adata;
struct resource *res;
+ u32 ret;

chip = dev_get_platdata(&pdev->dev);
if (!chip || !chip->base) {
@@ -234,7 +235,12 @@ static int rembrandt_audio_probe(struct platform_device *pdev)
acp_machine_select(adata);

dev_set_drvdata(dev, adata);
- acp6x_master_clock_generate(dev);
+
+ if (chip->flag != FLAG_AMD_LEGACY_ONLY_DMIC) {
+ ret = acp6x_master_clock_generate(dev);
+ if (ret)
+ return ret;
+ }
acp_enable_interrupts(adata);
acp_platform_register(dev);
pm_runtime_set_autosuspend_delay(&pdev->dev, ACP_SUSPEND_DELAY_MS);
@@ -263,7 +269,9 @@ static int __maybe_unused rmb_pcm_resume(struct device *dev)
snd_pcm_uframes_t buf_in_frames;
u64 buf_size;

- acp6x_master_clock_generate(dev);
+ if (adata->flag != FLAG_AMD_LEGACY_ONLY_DMIC)
+ acp6x_master_clock_generate(dev);
+
spin_lock(&adata->acp_lock);
list_for_each_entry(stream, &adata->stream_list, list) {
substream = stream->substream;
diff --git a/sound/soc/amd/acp/acp63.c b/sound/soc/amd/acp/acp63.c
index 0cec6ecaadfa..b871a216a6af 100644
--- a/sound/soc/amd/acp/acp63.c
+++ b/sound/soc/amd/acp/acp63.c
@@ -244,10 +244,12 @@ static int acp63_audio_probe(struct platform_device *pdev)
adata->machines = snd_soc_acpi_amd_acp63_acp_machines;
acp_machine_select(adata);
dev_set_drvdata(dev, adata);
- ret = acp63_i2s_master_clock_generate(adata);
- if (ret)
- return ret;

+ if (chip->flag != FLAG_AMD_LEGACY_ONLY_DMIC) {
+ ret = acp63_i2s_master_clock_generate(adata);
+ if (ret)
+ return ret;
+ }
acp_enable_interrupts(adata);
acp_platform_register(dev);
pm_runtime_set_autosuspend_delay(&pdev->dev, ACP_SUSPEND_DELAY_MS);
@@ -276,7 +278,9 @@ static int __maybe_unused acp63_pcm_resume(struct device *dev)
snd_pcm_uframes_t buf_in_frames;
u64 buf_size;

- acp63_i2s_master_clock_generate(adata);
+ if (adata->flag != FLAG_AMD_LEGACY_ONLY_DMIC)
+ acp63_i2s_master_clock_generate(adata);
+
spin_lock(&adata->acp_lock);
list_for_each_entry(stream, &adata->stream_list, list) {
if (stream) {
--
2.25.1

2023-10-21 14:53:47

by Saba Kareem, Syed

[permalink] [raw]
Subject: [PATCH 11/13] ASoC: amd: acp: change acp power on mask macro value

change acp power on mask macro value so that same macro can be used
for all amd platforms.

Signed-off-by: Syed Saba Kareem <[email protected]>
---
sound/soc/amd/acp/amd.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/sound/soc/amd/acp/amd.h b/sound/soc/amd/acp/amd.h
index e3bb470d1f32..937ce13c7d40 100644
--- a/sound/soc/amd/acp/amd.h
+++ b/sound/soc/amd/acp/amd.h
@@ -101,7 +101,7 @@

#define ACP_SOFT_RST_DONE_MASK 0x00010001

-#define ACP_PGFSM_CNTL_POWER_ON_MASK 0x01
+#define ACP_PGFSM_CNTL_POWER_ON_MASK 0xffffffff
#define ACP_PGFSM_CNTL_POWER_OFF_MASK 0x00
#define ACP_PGFSM_STATUS_MASK 0x03
#define ACP_POWERED_ON 0x00
--
2.25.1

2023-10-21 14:54:26

by Saba Kareem, Syed

[permalink] [raw]
Subject: [PATCH 13/13] ASoC: amd: acp: add machine driver support for acp7.0

add machine driver support for ACP7.0 on legacy stack.

Signed-off-by: Syed Saba Kareem <[email protected]>
---
sound/soc/amd/acp/acp-mach-common.c | 9 +++++++++
1 file changed, 9 insertions(+)

diff --git a/sound/soc/amd/acp/acp-mach-common.c b/sound/soc/amd/acp/acp-mach-common.c
index 88e91af47532..34b14f2611ba 100644
--- a/sound/soc/amd/acp/acp-mach-common.c
+++ b/sound/soc/amd/acp/acp-mach-common.c
@@ -1266,6 +1266,12 @@ static struct snd_soc_dai_link_component platform_acp63_component[] = {
}
};

+static struct snd_soc_dai_link_component platform_acp70_component[] = {
+ {
+ .name = "acp_asoc_acp70.0",
+ }
+};
+
static struct snd_soc_dai_link_component sof_component[] = {
{
.name = "0000:04:00.5",
@@ -1692,6 +1698,9 @@ int acp_legacy_dai_links_create(struct snd_soc_card *card)
} else if (drv_data->platform == ACP63) {
links[i].platforms = platform_acp63_component;
links[i].num_platforms = ARRAY_SIZE(platform_acp63_component);
+ } else if (drv_data->platform == ACP70) {
+ links[i].platforms = platform_acp70_component;
+ links[i].num_platforms = ARRAY_SIZE(platform_acp70_component);
} else {
links[i].platforms = platform_component;
links[i].num_platforms = ARRAY_SIZE(platform_component);
--
2.25.1

2023-10-21 14:54:40

by Saba Kareem, Syed

[permalink] [raw]
Subject: [PATCH 12/13] ASoC: amd: acp: Add pci legacy driver support for acp7.0 platform

Add pci legacy driver support and create platform driver for
acp7.0 platform.

Signed-off-by: Syed Saba Kareem <[email protected]>
---
sound/soc/amd/acp/acp-legacy-common.c | 11 +-
sound/soc/amd/acp/acp-mach.h | 1 +
sound/soc/amd/acp/acp-pci.c | 4 +
sound/soc/amd/acp/acp70.c | 254 ++++++++++++++++++++++++++
sound/soc/amd/acp/amd.h | 4 +
5 files changed, 273 insertions(+), 1 deletion(-)
create mode 100644 sound/soc/amd/acp/acp70.c

diff --git a/sound/soc/amd/acp/acp-legacy-common.c b/sound/soc/amd/acp/acp-legacy-common.c
index cba0aabefb34..b5aff3f230be 100644
--- a/sound/soc/amd/acp/acp-legacy-common.c
+++ b/sound/soc/amd/acp/acp-legacy-common.c
@@ -19,6 +19,7 @@
#define ACP_RENOIR_PDM_ADDR 0x02
#define ACP_REMBRANDT_PDM_ADDR 0x03
#define ACP63_PDM_ADDR 0x02
+#define ACP70_PDM_ADDR 0x02

void acp_enable_interrupts(struct acp_dev_data *adata)
{
@@ -268,6 +269,10 @@ static int acp_power_on(struct acp_chip_info *chip)
acp_pgfsm_stat_reg = ACP63_PGFSM_STATUS;
acp_pgfsm_ctrl_reg = ACP63_PGFSM_CONTROL;
break;
+ case ACP70_DEV:
+ acp_pgfsm_stat_reg = ACP70_PGFSM_STATUS;
+ acp_pgfsm_ctrl_reg = ACP70_PGFSM_CONTROL;
+ break;
default:
return -EINVAL;
}
@@ -329,7 +334,8 @@ int acp_deinit(struct acp_chip_info *chip)
if (ret)
return ret;

- writel(0, chip->base + ACP_CONTROL);
+ if (chip->acp_rev != ACP70_DEV)
+ writel(0, chip->base + ACP_CONTROL);
return 0;
}
EXPORT_SYMBOL_NS_GPL(acp_deinit, SND_SOC_ACP_COMMON);
@@ -385,6 +391,9 @@ int check_acp_pdm(struct pci_dev *pci, struct acp_chip_info *chip)
case ACP63_DEV:
pdm_addr = ACP63_PDM_ADDR;
break;
+ case ACP70_DEV:
+ pdm_addr = ACP70_PDM_ADDR;
+ break;
default:
return -EINVAL;
}
diff --git a/sound/soc/amd/acp/acp-mach.h b/sound/soc/amd/acp/acp-mach.h
index 69db61f12eca..cd681101bea7 100644
--- a/sound/soc/amd/acp/acp-mach.h
+++ b/sound/soc/amd/acp/acp-mach.h
@@ -54,6 +54,7 @@ enum platform_end_point {
RENOIR = 0,
REMBRANDT,
ACP63,
+ ACP70,
};

struct acp_mach_ops {
diff --git a/sound/soc/amd/acp/acp-pci.c b/sound/soc/amd/acp/acp-pci.c
index 696c9ee6786f..8c8b1dcac628 100644
--- a/sound/soc/amd/acp/acp-pci.c
+++ b/sound/soc/amd/acp/acp-pci.c
@@ -91,6 +91,10 @@ static int acp_pci_probe(struct pci_dev *pci, const struct pci_device_id *pci_id
chip->name = "acp_asoc_acp63";
chip->acp_rev = ACP63_DEV;
break;
+ case 0x70:
+ chip->name = "acp_asoc_acp70";
+ chip->acp_rev = ACP70_DEV;
+ break;
default:
dev_err(dev, "Unsupported device revision:0x%x\n", pci->revision);
ret = -EINVAL;
diff --git a/sound/soc/amd/acp/acp70.c b/sound/soc/amd/acp/acp70.c
new file mode 100644
index 000000000000..dd384c966ae9
--- /dev/null
+++ b/sound/soc/amd/acp/acp70.c
@@ -0,0 +1,254 @@
+// SPDX-License-Identifier: (GPL-2.0-only OR BSD-3-Clause)
+//
+// This file is provided under a dual BSD/GPLv2 license. When using or
+// redistributing this file, you may do so under either license.
+//
+// Copyright(c) 2023 Advanced Micro Devices, Inc.
+//
+// Authors: Syed Saba kareem <[email protected]>
+/*
+ * Hardware interface for ACP7.0 block
+ */
+
+#include <linux/platform_device.h>
+#include <linux/module.h>
+#include <linux/err.h>
+#include <linux/io.h>
+#include <sound/pcm_params.h>
+#include <sound/soc.h>
+#include <sound/soc-dai.h>
+#include <linux/dma-mapping.h>
+#include <linux/pm_runtime.h>
+#include <linux/pci.h>
+#include "amd.h"
+#include "acp-mach.h"
+
+#define DRV_NAME "acp_asoc_acp70"
+
+static struct acp_resource rsrc = {
+ .offset = 0,
+ .no_of_ctrls = 2,
+ .irqp_used = 1,
+ .soc_mclk = true,
+ .irq_reg_offset = 0x1a00,
+ .i2s_pin_cfg_offset = 0x1440,
+ .i2s_mode = 0x0a,
+ .scratch_reg_offset = 0x12800,
+ .sram_pte_offset = 0x03802800,
+};
+
+static struct snd_soc_acpi_mach snd_soc_acpi_amd_acp70_acp_machines[] = {
+ {
+ .id = "AMDI0029",
+ .drv_name = "acp70-acp",
+ },
+ {},
+};
+
+static struct snd_soc_dai_driver acp70_dai[] = {
+{
+ .name = "acp-i2s-sp",
+ .id = I2S_SP_INSTANCE,
+ .playback = {
+ .stream_name = "I2S SP Playback",
+ .rates = SNDRV_PCM_RATE_8000_96000,
+ .formats = SNDRV_PCM_FMTBIT_S16_LE | SNDRV_PCM_FMTBIT_S8 |
+ SNDRV_PCM_FMTBIT_U8 | SNDRV_PCM_FMTBIT_S32_LE,
+ .channels_min = 2,
+ .channels_max = 8,
+ .rate_min = 8000,
+ .rate_max = 96000,
+ },
+ .capture = {
+ .stream_name = "I2S SP Capture",
+ .rates = SNDRV_PCM_RATE_8000_48000,
+ .formats = SNDRV_PCM_FMTBIT_S16_LE | SNDRV_PCM_FMTBIT_S8 |
+ SNDRV_PCM_FMTBIT_U8 | SNDRV_PCM_FMTBIT_S32_LE,
+ .channels_min = 2,
+ .channels_max = 2,
+ .rate_min = 8000,
+ .rate_max = 48000,
+ },
+ .ops = &asoc_acp_cpu_dai_ops,
+},
+{
+ .name = "acp-i2s-bt",
+ .id = I2S_BT_INSTANCE,
+ .playback = {
+ .stream_name = "I2S BT Playback",
+ .rates = SNDRV_PCM_RATE_8000_96000,
+ .formats = SNDRV_PCM_FMTBIT_S16_LE | SNDRV_PCM_FMTBIT_S8 |
+ SNDRV_PCM_FMTBIT_U8 | SNDRV_PCM_FMTBIT_S32_LE,
+ .channels_min = 2,
+ .channels_max = 8,
+ .rate_min = 8000,
+ .rate_max = 96000,
+ },
+ .capture = {
+ .stream_name = "I2S BT Capture",
+ .rates = SNDRV_PCM_RATE_8000_48000,
+ .formats = SNDRV_PCM_FMTBIT_S16_LE | SNDRV_PCM_FMTBIT_S8 |
+ SNDRV_PCM_FMTBIT_U8 | SNDRV_PCM_FMTBIT_S32_LE,
+ .channels_min = 2,
+ .channels_max = 2,
+ .rate_min = 8000,
+ .rate_max = 48000,
+ },
+ .ops = &asoc_acp_cpu_dai_ops,
+},
+{
+ .name = "acp-i2s-hs",
+ .id = I2S_HS_INSTANCE,
+ .playback = {
+ .stream_name = "I2S HS Playback",
+ .rates = SNDRV_PCM_RATE_8000_96000,
+ .formats = SNDRV_PCM_FMTBIT_S16_LE | SNDRV_PCM_FMTBIT_S8 |
+ SNDRV_PCM_FMTBIT_U8 | SNDRV_PCM_FMTBIT_S32_LE,
+ .channels_min = 2,
+ .channels_max = 8,
+ .rate_min = 8000,
+ .rate_max = 96000,
+ },
+ .capture = {
+ .stream_name = "I2S HS Capture",
+ .rates = SNDRV_PCM_RATE_8000_48000,
+ .formats = SNDRV_PCM_FMTBIT_S16_LE | SNDRV_PCM_FMTBIT_S8 |
+ SNDRV_PCM_FMTBIT_U8 | SNDRV_PCM_FMTBIT_S32_LE,
+ .channels_min = 2,
+ .channels_max = 8,
+ .rate_min = 8000,
+ .rate_max = 48000,
+ },
+ .ops = &asoc_acp_cpu_dai_ops,
+},
+{
+ .name = "acp-pdm-dmic",
+ .id = DMIC_INSTANCE,
+ .capture = {
+ .rates = SNDRV_PCM_RATE_8000_48000,
+ .formats = SNDRV_PCM_FMTBIT_S32_LE,
+ .channels_min = 2,
+ .channels_max = 2,
+ .rate_min = 8000,
+ .rate_max = 48000,
+ },
+ .ops = &acp_dmic_dai_ops,
+},
+};
+
+static int acp_acp70_audio_probe(struct platform_device *pdev)
+{
+ struct device *dev = &pdev->dev;
+ struct acp_chip_info *chip;
+ struct acp_dev_data *adata;
+ struct resource *res;
+
+ chip = dev_get_platdata(&pdev->dev);
+ if (!chip || !chip->base) {
+ dev_err(&pdev->dev, "ACP chip data is NULL\n");
+ return -ENODEV;
+ }
+
+ if (chip->acp_rev != ACP70_DEV) {
+ dev_err(&pdev->dev, "Un-supported ACP Revision %d\n", chip->acp_rev);
+ return -ENODEV;
+ }
+
+ adata = devm_kzalloc(dev, sizeof(struct acp_dev_data), GFP_KERNEL);
+ if (!adata)
+ return -ENOMEM;
+
+ res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "acp_mem");
+ if (!res) {
+ dev_err(&pdev->dev, "IORESOURCE_MEM FAILED\n");
+ return -ENODEV;
+ }
+
+ adata->acp_base = devm_ioremap(&pdev->dev, res->start, resource_size(res));
+ if (!adata->acp_base)
+ return -ENOMEM;
+
+ res = platform_get_resource_byname(pdev, IORESOURCE_IRQ, "acp_dai_irq");
+ if (!res) {
+ dev_err(&pdev->dev, "IORESOURCE_IRQ FAILED\n");
+ return -ENODEV;
+ }
+
+ adata->i2s_irq = res->start;
+ adata->dev = dev;
+ adata->dai_driver = acp70_dai;
+ adata->num_dai = ARRAY_SIZE(acp70_dai);
+ adata->rsrc = &rsrc;
+ adata->machines = snd_soc_acpi_amd_acp70_acp_machines;
+ adata->platform = ACP70;
+ adata->flag = chip->flag;
+ acp_machine_select(adata);
+
+ dev_set_drvdata(dev, adata);
+ acp_enable_interrupts(adata);
+ acp_platform_register(dev);
+ pm_runtime_set_autosuspend_delay(&pdev->dev, ACP_SUSPEND_DELAY_MS);
+ pm_runtime_use_autosuspend(&pdev->dev);
+ pm_runtime_mark_last_busy(&pdev->dev);
+ pm_runtime_set_active(&pdev->dev);
+ pm_runtime_enable(&pdev->dev);
+ return 0;
+}
+
+static void acp_acp70_audio_remove(struct platform_device *pdev)
+{
+ struct device *dev = &pdev->dev;
+ struct acp_dev_data *adata = dev_get_drvdata(dev);
+
+ acp_disable_interrupts(adata);
+ acp_platform_unregister(dev);
+ pm_runtime_disable(&pdev->dev);
+}
+
+static int __maybe_unused acp70_pcm_resume(struct device *dev)
+{
+ struct acp_dev_data *adata = dev_get_drvdata(dev);
+ struct acp_stream *stream;
+ struct snd_pcm_substream *substream;
+ snd_pcm_uframes_t buf_in_frames;
+ u64 buf_size;
+
+ spin_lock(&adata->acp_lock);
+ list_for_each_entry(stream, &adata->stream_list, list) {
+ if (stream) {
+ substream = stream->substream;
+ if (substream && substream->runtime) {
+ buf_in_frames = (substream->runtime->buffer_size);
+ buf_size = frames_to_bytes(substream->runtime, buf_in_frames);
+ config_pte_for_stream(adata, stream);
+ config_acp_dma(adata, stream, buf_size);
+ if (stream->dai_id)
+ restore_acp_i2s_params(substream, adata, stream);
+ else
+ restore_acp_pdm_params(substream, adata);
+ }
+ }
+ }
+ spin_unlock(&adata->acp_lock);
+ return 0;
+}
+
+static const struct dev_pm_ops acp70_dma_pm_ops = {
+ SET_SYSTEM_SLEEP_PM_OPS(NULL, acp70_pcm_resume)
+};
+
+static struct platform_driver acp70_driver = {
+ .probe = acp_acp70_audio_probe,
+ .remove_new = acp_acp70_audio_remove,
+ .driver = {
+ .name = "acp_asoc_acp70",
+ .pm = &acp70_dma_pm_ops,
+ },
+};
+
+module_platform_driver(acp70_driver);
+
+MODULE_DESCRIPTION("AMD ACP ACP70 Driver");
+MODULE_IMPORT_NS(SND_SOC_ACP_COMMON);
+MODULE_LICENSE("Dual BSD/GPL");
+MODULE_ALIAS("platform:" DRV_NAME);
diff --git a/sound/soc/amd/acp/amd.h b/sound/soc/amd/acp/amd.h
index 937ce13c7d40..5017e868f39b 100644
--- a/sound/soc/amd/acp/amd.h
+++ b/sound/soc/amd/acp/amd.h
@@ -21,6 +21,7 @@
#define ACP3X_DEV 3
#define ACP6X_DEV 6
#define ACP63_DEV 0x63
+#define ACP70_DEV 0x70

#define DMIC_INSTANCE 0x00
#define I2S_SP_INSTANCE 0x01
@@ -99,6 +100,9 @@
#define ACP63_PGFSM_CONTROL ACP6X_PGFSM_CONTROL
#define ACP63_PGFSM_STATUS ACP6X_PGFSM_STATUS

+#define ACP70_PGFSM_CONTROL ACP6X_PGFSM_CONTROL
+#define ACP70_PGFSM_STATUS ACP6X_PGFSM_STATUS
+
#define ACP_SOFT_RST_DONE_MASK 0x00010001

#define ACP_PGFSM_CNTL_POWER_ON_MASK 0xffffffff
--
2.25.1

2023-10-23 07:37:26

by Amadeusz Sławiński

[permalink] [raw]
Subject: Re: [PATCH 02/13] ASoC: amd: acp: refactor acp i2s clock generation code

On 10/21/2023 4:50 PM, Syed Saba Kareem wrote:
> Refactor acp i2s LRCLK,BCLK generation code and move to commnon file.
>
> Signed-off-by: Syed Saba Kareem <[email protected]>
> ---
> sound/soc/amd/acp/acp-i2s.c | 32 ++++++++++++++++++++++++++++++
> sound/soc/amd/acp/amd.h | 39 -------------------------------------
> 2 files changed, 32 insertions(+), 39 deletions(-)
>
> diff --git a/sound/soc/amd/acp/acp-i2s.c b/sound/soc/amd/acp/acp-i2s.c
> index df350014966a..59d3a499771a 100644
> --- a/sound/soc/amd/acp/acp-i2s.c
> +++ b/sound/soc/amd/acp/acp-i2s.c
> @@ -20,10 +20,42 @@
> #include <sound/soc.h>
> #include <sound/soc-dai.h>
> #include <linux/dma-mapping.h>
> +#include <linux/bitfield.h>
>
> #include "amd.h"
>
> #define DRV_NAME "acp_i2s_playcap"
> +#define I2S_MASTER_MODE_ENABLE 1
> +#define I2S_MODE_ENABLE 0
> +#define I2S_FORMAT_MODE GENMASK(1, 1)
> +#define LRCLK_DIV_FIELD GENMASK(10, 2)
> +#define BCLK_DIV_FIELD GENMASK(23, 11)
> +
> +static inline void acp_set_i2s_clk(struct acp_dev_data *adata, int dai_id)
> +{
> + u32 i2s_clk_reg, val;
> +
> + switch (dai_id) {
> + case I2S_SP_INSTANCE:
> + i2s_clk_reg = ACP_I2STDM0_MSTRCLKGEN;
> + break;
> + case I2S_BT_INSTANCE:
> + i2s_clk_reg = ACP_I2STDM1_MSTRCLKGEN;
> + break;
> + case I2S_HS_INSTANCE:
> + i2s_clk_reg = ACP_I2STDM2_MSTRCLKGEN;
> + break;
> + default:
> + i2s_clk_reg = ACP_I2STDM0_MSTRCLKGEN;
> + break;
> + }
> +
> + val = I2S_MASTER_MODE_ENABLE;
> + val |= I2S_MODE_ENABLE & BIT(1);

There is I2S_FORMAT_MODE define, you probably want to use it instead of
BIT(1), so there is no "magic number" mask?

> + val |= FIELD_PREP(LRCLK_DIV_FIELD, adata->lrclk_div);
> + val |= FIELD_PREP(BCLK_DIV_FIELD, adata->bclk_div);
> + writel(val, adata->acp_base + i2s_clk_reg);
> +}
>
> static int acp_i2s_set_fmt(struct snd_soc_dai *cpu_dai,
> unsigned int fmt)
> diff --git a/sound/soc/amd/acp/amd.h b/sound/soc/amd/acp/amd.h
> index 487eefa5985f..87d1e1f7d6b6 100644
> --- a/sound/soc/amd/acp/amd.h
> +++ b/sound/soc/amd/acp/amd.h
> @@ -188,17 +188,6 @@ struct acp_dev_data {
> u32 xfer_rx_resolution[3];
> };
>
> -union acp_i2stdm_mstrclkgen {
> - struct {
> - u32 i2stdm_master_mode : 1;
> - u32 i2stdm_format_mode : 1;
> - u32 i2stdm_lrclk_div_val : 9;
> - u32 i2stdm_bclk_div_val : 11;
> - u32:10;
> - } bitfields, bits;
> - u32 u32_all;
> -};
> -
> extern const struct snd_soc_dai_ops asoc_acp_cpu_dai_ops;
> extern const struct snd_soc_dai_ops acp_dmic_dai_ops;
>
> @@ -276,32 +265,4 @@ static inline u64 acp_get_byte_count(struct acp_dev_data *adata, int dai_id, int
> POINTER_RETURN_BYTES:
> return byte_count;
> }
> -
> -static inline void acp_set_i2s_clk(struct acp_dev_data *adata, int dai_id)
> -{
> - union acp_i2stdm_mstrclkgen mclkgen;
> - u32 master_reg;
> -
> - switch (dai_id) {
> - case I2S_SP_INSTANCE:
> - master_reg = ACP_I2STDM0_MSTRCLKGEN;
> - break;
> - case I2S_BT_INSTANCE:
> - master_reg = ACP_I2STDM1_MSTRCLKGEN;
> - break;
> - case I2S_HS_INSTANCE:
> - master_reg = ACP_I2STDM2_MSTRCLKGEN;
> - break;
> - default:
> - master_reg = ACP_I2STDM0_MSTRCLKGEN;
> - break;
> - }
> -
> - mclkgen.bits.i2stdm_master_mode = 0x1;
> - mclkgen.bits.i2stdm_format_mode = 0x00;
> -
> - mclkgen.bits.i2stdm_bclk_div_val = adata->bclk_div;
> - mclkgen.bits.i2stdm_lrclk_div_val = adata->lrclk_div;
> - writel(mclkgen.u32_all, adata->acp_base + master_reg);
> -}
> #endif

2023-10-23 07:51:27

by Amadeusz Sławiński

[permalink] [raw]
Subject: Re: [PATCH 12/13] ASoC: amd: acp: Add pci legacy driver support for acp7.0 platform

On 10/21/2023 4:50 PM, Syed Saba Kareem wrote:
> Add pci legacy driver support and create platform driver for
> acp7.0 platform.
>
> Signed-off-by: Syed Saba Kareem <[email protected]>
> ---

...

> +
> +static struct snd_soc_dai_driver acp70_dai[] = {
> +{
> + .name = "acp-i2s-sp",
> + .id = I2S_SP_INSTANCE,
> + .playback = {
> + .stream_name = "I2S SP Playback",
> + .rates = SNDRV_PCM_RATE_8000_96000,
> + .formats = SNDRV_PCM_FMTBIT_S16_LE | SNDRV_PCM_FMTBIT_S8 |
> + SNDRV_PCM_FMTBIT_U8 | SNDRV_PCM_FMTBIT_S32_LE,

Any reason to not go from lowest bit width to higher? Similarly in
further definitions.

> + .channels_min = 2,
> + .channels_max = 8,
> + .rate_min = 8000,
> + .rate_max = 96000,
> + },

...

> +
> +static int __maybe_unused acp70_pcm_resume(struct device *dev)
> +{
> + struct acp_dev_data *adata = dev_get_drvdata(dev);
> + struct acp_stream *stream;
> + struct snd_pcm_substream *substream;
> + snd_pcm_uframes_t buf_in_frames;
> + u64 buf_size;
> +
> + spin_lock(&adata->acp_lock);
> + list_for_each_entry(stream, &adata->stream_list, list) {
> + if (stream) {
> + substream = stream->substream;
> + if (substream && substream->runtime) {
> + buf_in_frames = (substream->runtime->buffer_size);
> + buf_size = frames_to_bytes(substream->runtime, buf_in_frames);
> + config_pte_for_stream(adata, stream);
> + config_acp_dma(adata, stream, buf_size);
> + if (stream->dai_id)
> + restore_acp_i2s_params(substream, adata, stream);
> + else
> + restore_acp_pdm_params(substream, adata);
> + }
> + }
> + }
> + spin_unlock(&adata->acp_lock);
> + return 0;

Indentation is wrong in above two lines.

> +}
> +
> +static const struct dev_pm_ops acp70_dma_pm_ops = {
> + SET_SYSTEM_SLEEP_PM_OPS(NULL, acp70_pcm_resume)
> +};
> +


2023-10-23 08:02:06

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH 01/13] ASoC: amd: acp: Add acp6.3 pci legacy driver support

On 21/10/2023 16:50, Syed Saba Kareem wrote:
> Add pci legacy driver support and create platform driver for
> acp6.3 based platforms.
>
> Signed-off-by: Syed Saba Kareem <[email protected]>
> ---
> sound/soc/amd/acp/acp-legacy-common.c | 4 +
> sound/soc/amd/acp/acp-pci.c | 4 +
> sound/soc/amd/acp/acp63.c | 314 ++++++++++++++++++++++++++
> sound/soc/amd/acp/amd.h | 4 +
> 4 files changed, 326 insertions(+)
> create mode 100644 sound/soc/amd/acp/acp63.c
>


> +
> +static const struct dev_pm_ops acp63_dma_pm_ops = {
> + SET_SYSTEM_SLEEP_PM_OPS(NULL, acp63_pcm_resume)
> +};
> +
> +static struct platform_driver acp63_driver = {
> + .probe = acp63_audio_probe,
> + .remove_new = acp63_audio_remove,
> + .driver = {
> + .name = "acp_asoc_acp63",
> + .pm = &acp63_dma_pm_ops,
> + },
> +};
> +
> +module_platform_driver(acp63_driver);
> +
> +MODULE_DESCRIPTION("AMD ACP acp63 Driver");
> +MODULE_IMPORT_NS(SND_SOC_ACP_COMMON);
> +MODULE_LICENSE("Dual BSD/GPL");
> +MODULE_ALIAS("platform:" DRV_NAME);

You should not need MODULE_ALIAS() in normal cases. If you need it,
usually it means your device ID table is wrong.


Best regards,
Krzysztof

2023-10-25 16:51:06

by Mark Brown

[permalink] [raw]
Subject: Re: [PATCH 01/13] ASoC: amd: acp: Add acp6.3 pci legacy driver support

On Sat, Oct 21, 2023 at 08:20:42PM +0530, Syed Saba Kareem wrote:
> Add pci legacy driver support and create platform driver for
> acp6.3 based platforms.

I've queued this series for CI - there were some valid concerns that
Amadeusz and Krzysztof raised but they're relatively minor, please send
incremental patches fixing these issues (assuming CI is fine).


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

2023-10-26 09:44:47

by syed saba kareem

[permalink] [raw]
Subject: Re: [PATCH 01/13] ASoC: amd: acp: Add acp6.3 pci legacy driver support

Will send the fixes as incremental patches.

On 10/25/23 22:20, Mark Brown wrote:
> On Sat, Oct 21, 2023 at 08:20:42PM +0530, Syed Saba Kareem wrote:
>> Add pci legacy driver support and create platform driver for
>> acp6.3 based platforms.
> I've queued this series for CI - there were some valid concerns that
> Amadeusz and Krzysztof raised but they're relatively minor, please send
> incremental patches fixing these issues (assuming CI is fine).

2023-10-26 10:05:06

by syed saba kareem

[permalink] [raw]
Subject: Re: [PATCH 12/13] ASoC: amd: acp: Add pci legacy driver support for acp7.0 platform


On 10/23/23 13:20, Amadeusz Sławiński wrote:
> On 10/21/2023 4:50 PM, Syed Saba Kareem wrote:
>> Add pci legacy driver support and create platform driver for
>> acp7.0 platform.
>>
>> Signed-off-by: Syed Saba Kareem <[email protected]>
>> ---
>
> ...
>
>> +
>> +static struct snd_soc_dai_driver acp70_dai[] = {
>> +{
>> +    .name = "acp-i2s-sp",
>> +    .id = I2S_SP_INSTANCE,
>> +    .playback = {
>> +        .stream_name = "I2S SP Playback",
>> +        .rates = SNDRV_PCM_RATE_8000_96000,
>> +        .formats = SNDRV_PCM_FMTBIT_S16_LE | SNDRV_PCM_FMTBIT_S8 |
>> +               SNDRV_PCM_FMTBIT_U8 | SNDRV_PCM_FMTBIT_S32_LE,
>
> Any reason to not go from lowest bit width to higher? Similarly in
> further definitions.
>  This has to be corrected, will push the changes as an incremental patch.
>> +        .channels_min = 2,
>> +        .channels_max = 8,
>> +        .rate_min = 8000,
>> +        .rate_max = 96000,
>> +    },
>
> ...
>
>> +
>> +static int __maybe_unused acp70_pcm_resume(struct device *dev)
>> +{
>> +    struct acp_dev_data *adata = dev_get_drvdata(dev);
>> +    struct acp_stream *stream;
>> +    struct snd_pcm_substream *substream;
>> +    snd_pcm_uframes_t buf_in_frames;
>> +    u64 buf_size;
>> +
>> +    spin_lock(&adata->acp_lock);
>> +    list_for_each_entry(stream, &adata->stream_list, list) {
>> +        if (stream) {
>> +            substream = stream->substream;
>> +            if (substream && substream->runtime) {
>> +                buf_in_frames = (substream->runtime->buffer_size);
>> +                buf_size = frames_to_bytes(substream->runtime,
>> buf_in_frames);
>> +                config_pte_for_stream(adata, stream);
>> +                config_acp_dma(adata, stream, buf_size);
>> +                if (stream->dai_id)
>> +                    restore_acp_i2s_params(substream, adata, stream);
>> +                else
>> +                    restore_acp_pdm_params(substream, adata);
>> +            }
>> +        }
>> +    }
>> +        spin_unlock(&adata->acp_lock);
>> +        return 0;
>
> Indentation is wrong in above two lines.
>   This has to be corrected, will push the changes as an incremental
> patch.
>> +}
>> +
>> +static const struct dev_pm_ops acp70_dma_pm_ops = {
>> +    SET_SYSTEM_SLEEP_PM_OPS(NULL, acp70_pcm_resume)
>> +};
>> +
>
>

2023-10-26 15:02:28

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH 01/13] ASoC: amd: acp: Add acp6.3 pci legacy driver support

On Thu, 26 Oct 2023 at 12:09, syed saba kareem <[email protected]> wrote:
> +
> +module_platform_driver(acp63_driver);
> +
> +MODULE_DESCRIPTION("AMD ACP acp63 Driver");
> +MODULE_IMPORT_NS(SND_SOC_ACP_COMMON);
> +MODULE_LICENSE("Dual BSD/GPL");
> +MODULE_ALIAS("platform:" DRV_NAME);
>
> You should not need MODULE_ALIAS() in normal cases. If you need it,
> usually it means your device ID table is wrong.
>
> It is platform driver ,for auto loading MODULE_ALIAS() is required.

Hm, not really. platform_driver does not need MODULE_ALIAS(). At least
99% of them do not need it. Please help us understand what is broken
here that this one platform driver needs alias.

BR,
Krzysztof

2023-10-26 15:13:16

by Mark Brown

[permalink] [raw]
Subject: Re: [PATCH 01/13] ASoC: amd: acp: Add acp6.3 pci legacy driver support

On Sat, 21 Oct 2023 20:20:42 +0530, Syed Saba Kareem wrote:
> Add pci legacy driver support and create platform driver for
> acp6.3 based platforms.
>
>

Applied to

https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next

Thanks!

[01/13] ASoC: amd: acp: Add acp6.3 pci legacy driver support
commit: 33d120a49b970acbe465041d7b71e0cd6a1d1de0
[02/13] ASoC: amd: acp: refactor acp i2s clock generation code
commit: 40f74d5f09d7c068bd7a980dc06a688dc8d2b3e3
[03/13] ASoC: amd: acp: add i2s clock generation support for acp6.3 based platforms
commit: c7bf9156f811bcffb4201a0bf5d1b32d97ec0e5f
[04/13] ASoC: amd: acp: add machine driver support for acp6.3 platform
commit: 9393bfb4c4dea406dd345820a6b39b9c70a7f934
[05/13] ASoC: amd: acp: add Kconfig options for acp6.3 based platform driver
commit: d4c2d5391d7efc29fdd59d54355526c9ace16bec
[06/13] ASoC: amd: acp: add code for scanning acp pdm controller
commit: 3a94c8ad0aae2b14a55059869871ea2d199af489
[07/13] ASoC: amd: acp: add platform and flag data to acp data structure
commit: 57e857770f6021a73af85e6b201203520cb1a529
[08/13] ASoC: amd: acp: add condition check for i2s clock generation
commit: 16fb2a25440a85708778f6442914066c98dc2f1e
[09/13] ASoC: amd: acp: add machine driver support for pdm use case
commit: 39d9ee47167a2210d803f651c8fdcac84f03e766
[10/13] ASoC: amd: acp: change acp-deinit function arguments
commit: 1b6180c095bc9a6c25e38ae1ec6481f8df20a81f
[11/13] ASoC: amd: acp: change acp power on mask macro value
commit: caa126f2b0c821811eedf2e2fd435b11844bf0f1
[12/13] ASoC: amd: acp: Add pci legacy driver support for acp7.0 platform
commit: e84db124cb2158b538820f31f641c28b86fb3ca3
[13/13] ASoC: amd: acp: add machine driver support for acp7.0
commit: b97f4dac40eecfc2fc9b818b427a8eda44cb7763

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark

2023-10-27 08:50:43

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH 09/13] ASoC: amd: acp: add machine driver support for pdm use case

On 21/10/2023 16:50, Syed Saba Kareem wrote:
> add pdm use case machine driver support
>
> Signed-off-by: Syed Saba Kareem <[email protected]>
> ---


> dmi_id = dmi_first_match(acp_quirk_table);
> if (dmi_id && dmi_id->driver_data)
> @@ -214,6 +221,10 @@ static const struct platform_device_id board_ids[] = {
> .name = "rmb-rt5682s-rt1019",
> .driver_data = (kernel_ulong_t)&rt5682s_rt1019_rmb_data,
> },
> + {
> + .name = "acp-pdm-mach",
> + .driver_data = (kernel_ulong_t)&acp_dmic_data,
> + },
> { }
> };
> static struct platform_driver acp_asoc_audio = {
> @@ -235,4 +246,5 @@ MODULE_ALIAS("platform:acp3xalc5682s1019");
> MODULE_ALIAS("platform:acp3x-es83xx");
> MODULE_ALIAS("platform:rmb-nau8825-max");
> MODULE_ALIAS("platform:rmb-rt5682s-rt1019");
> +MODULE_ALIAS("platform:acp-pdm-mach");

Please stop growing the aliases. Module alias is not a substitute for
missing MODULE_DEVICE_TABLE.

Best regards,
Krzysztof

2023-10-27 15:29:14

by Mario Limonciello

[permalink] [raw]
Subject: Re: [PATCH 09/13] ASoC: amd: acp: add machine driver support for pdm use case

On 10/27/2023 03:49, Krzysztof Kozlowski wrote:
> On 21/10/2023 16:50, Syed Saba Kareem wrote:
>> add pdm use case machine driver support
>>
>> Signed-off-by: Syed Saba Kareem <[email protected]>
>> ---
>
>
>> dmi_id = dmi_first_match(acp_quirk_table);
>> if (dmi_id && dmi_id->driver_data)
>> @@ -214,6 +221,10 @@ static const struct platform_device_id board_ids[] = {
>> .name = "rmb-rt5682s-rt1019",
>> .driver_data = (kernel_ulong_t)&rt5682s_rt1019_rmb_data,
>> },
>> + {
>> + .name = "acp-pdm-mach",
>> + .driver_data = (kernel_ulong_t)&acp_dmic_data,
>> + },
>> { }
>> };
>> static struct platform_driver acp_asoc_audio = {
>> @@ -235,4 +246,5 @@ MODULE_ALIAS("platform:acp3xalc5682s1019");
>> MODULE_ALIAS("platform:acp3x-es83xx");
>> MODULE_ALIAS("platform:rmb-nau8825-max");
>> MODULE_ALIAS("platform:rmb-rt5682s-rt1019");
>> +MODULE_ALIAS("platform:acp-pdm-mach");
>
> Please stop growing the aliases. Module alias is not a substitute for
> missing MODULE_DEVICE_TABLE.
>
> Best regards,
> Krzysztof
>

I thought the way that this works is that top level ACP driver (IE
acp-pci.c) will have MODULE_DEVICE_TABLE. This is how that module gets
loaded.

Then it creates platform devices based on the detected needs for the
situation and the creation of those platform devices triggers a uevent
which due to MODULE_ALIAS will get appropriate other platform drivers
like this one loaded.

2023-10-27 15:52:34

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH 09/13] ASoC: amd: acp: add machine driver support for pdm use case

On 27/10/2023 17:28, Mario Limonciello wrote:
> On 10/27/2023 03:49, Krzysztof Kozlowski wrote:
>> On 21/10/2023 16:50, Syed Saba Kareem wrote:
>>> add pdm use case machine driver support
>>>
>>> Signed-off-by: Syed Saba Kareem <[email protected]>
>>> ---
>>
>>
>>> dmi_id = dmi_first_match(acp_quirk_table);
>>> if (dmi_id && dmi_id->driver_data)
>>> @@ -214,6 +221,10 @@ static const struct platform_device_id board_ids[] = {
>>> .name = "rmb-rt5682s-rt1019",
>>> .driver_data = (kernel_ulong_t)&rt5682s_rt1019_rmb_data,
>>> },
>>> + {
>>> + .name = "acp-pdm-mach",
>>> + .driver_data = (kernel_ulong_t)&acp_dmic_data,
>>> + },
>>> { }
>>> };
>>> static struct platform_driver acp_asoc_audio = {
>>> @@ -235,4 +246,5 @@ MODULE_ALIAS("platform:acp3xalc5682s1019");
>>> MODULE_ALIAS("platform:acp3x-es83xx");
>>> MODULE_ALIAS("platform:rmb-nau8825-max");
>>> MODULE_ALIAS("platform:rmb-rt5682s-rt1019");
>>> +MODULE_ALIAS("platform:acp-pdm-mach");
>>
>> Please stop growing the aliases. Module alias is not a substitute for
>> missing MODULE_DEVICE_TABLE.
>>
>> Best regards,
>> Krzysztof
>>
>
> I thought the way that this works is that top level ACP driver (IE
> acp-pci.c) will have MODULE_DEVICE_TABLE. This is how that module gets
> loaded.
>
> Then it creates platform devices based on the detected needs for the
> situation and the creation of those platform devices triggers a uevent
> which due to MODULE_ALIAS will get appropriate other platform drivers
> like this one loaded.

And why you cannot use MODULE_DEVICE_TABLE here? IOW, why do you need to
manually duplicate entire table and re-invent MODULE_DEVICE_TABLE with
MODULE_ALIAS?

Best regards,
Krzysztof

2023-10-27 15:55:11

by Mario Limonciello

[permalink] [raw]
Subject: Re: [PATCH 09/13] ASoC: amd: acp: add machine driver support for pdm use case

On 10/27/2023 10:51, Krzysztof Kozlowski wrote:
> On 27/10/2023 17:28, Mario Limonciello wrote:
>> On 10/27/2023 03:49, Krzysztof Kozlowski wrote:
>>> On 21/10/2023 16:50, Syed Saba Kareem wrote:
>>>> add pdm use case machine driver support
>>>>
>>>> Signed-off-by: Syed Saba Kareem <[email protected]>
>>>> ---
>>>
>>>
>>>> dmi_id = dmi_first_match(acp_quirk_table);
>>>> if (dmi_id && dmi_id->driver_data)
>>>> @@ -214,6 +221,10 @@ static const struct platform_device_id board_ids[] = {
>>>> .name = "rmb-rt5682s-rt1019",
>>>> .driver_data = (kernel_ulong_t)&rt5682s_rt1019_rmb_data,
>>>> },
>>>> + {
>>>> + .name = "acp-pdm-mach",
>>>> + .driver_data = (kernel_ulong_t)&acp_dmic_data,
>>>> + },
>>>> { }
>>>> };
>>>> static struct platform_driver acp_asoc_audio = {
>>>> @@ -235,4 +246,5 @@ MODULE_ALIAS("platform:acp3xalc5682s1019");
>>>> MODULE_ALIAS("platform:acp3x-es83xx");
>>>> MODULE_ALIAS("platform:rmb-nau8825-max");
>>>> MODULE_ALIAS("platform:rmb-rt5682s-rt1019");
>>>> +MODULE_ALIAS("platform:acp-pdm-mach");
>>>
>>> Please stop growing the aliases. Module alias is not a substitute for
>>> missing MODULE_DEVICE_TABLE.
>>>
>>> Best regards,
>>> Krzysztof
>>>
>>
>> I thought the way that this works is that top level ACP driver (IE
>> acp-pci.c) will have MODULE_DEVICE_TABLE. This is how that module gets
>> loaded.
>>
>> Then it creates platform devices based on the detected needs for the
>> situation and the creation of those platform devices triggers a uevent
>> which due to MODULE_ALIAS will get appropriate other platform drivers
>> like this one loaded.
>
> And why you cannot use MODULE_DEVICE_TABLE here? IOW, why do you need to
> manually duplicate entire table and re-invent MODULE_DEVICE_TABLE with
> MODULE_ALIAS?

What would actually go into MODULE_DEVICE_TABLE?

The platform devices created are contingent upon what was found during
the top level ACP driver probe. You don't want all the "child" platform
drivers to load unless they're needed.

2023-10-27 15:57:53

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH 09/13] ASoC: amd: acp: add machine driver support for pdm use case

On 27/10/2023 17:54, Mario Limonciello wrote:
> On 10/27/2023 10:51, Krzysztof Kozlowski wrote:
>> On 27/10/2023 17:28, Mario Limonciello wrote:
>>> On 10/27/2023 03:49, Krzysztof Kozlowski wrote:
>>>> On 21/10/2023 16:50, Syed Saba Kareem wrote:
>>>>> add pdm use case machine driver support
>>>>>
>>>>> Signed-off-by: Syed Saba Kareem <[email protected]>
>>>>> ---
>>>>
>>>>
>>>>> dmi_id = dmi_first_match(acp_quirk_table);
>>>>> if (dmi_id && dmi_id->driver_data)
>>>>> @@ -214,6 +221,10 @@ static const struct platform_device_id board_ids[] = {
>>>>> .name = "rmb-rt5682s-rt1019",
>>>>> .driver_data = (kernel_ulong_t)&rt5682s_rt1019_rmb_data,
>>>>> },
>>>>> + {
>>>>> + .name = "acp-pdm-mach",
>>>>> + .driver_data = (kernel_ulong_t)&acp_dmic_data,
>>>>> + },
>>>>> { }
>>>>> };
>>>>> static struct platform_driver acp_asoc_audio = {
>>>>> @@ -235,4 +246,5 @@ MODULE_ALIAS("platform:acp3xalc5682s1019");
>>>>> MODULE_ALIAS("platform:acp3x-es83xx");
>>>>> MODULE_ALIAS("platform:rmb-nau8825-max");
>>>>> MODULE_ALIAS("platform:rmb-rt5682s-rt1019");
>>>>> +MODULE_ALIAS("platform:acp-pdm-mach");
>>>>
>>>> Please stop growing the aliases. Module alias is not a substitute for
>>>> missing MODULE_DEVICE_TABLE.
>>>>
>>>> Best regards,
>>>> Krzysztof
>>>>
>>>
>>> I thought the way that this works is that top level ACP driver (IE
>>> acp-pci.c) will have MODULE_DEVICE_TABLE. This is how that module gets
>>> loaded.
>>>
>>> Then it creates platform devices based on the detected needs for the
>>> situation and the creation of those platform devices triggers a uevent
>>> which due to MODULE_ALIAS will get appropriate other platform drivers
>>> like this one loaded.
>>
>> And why you cannot use MODULE_DEVICE_TABLE here? IOW, why do you need to
>> manually duplicate entire table and re-invent MODULE_DEVICE_TABLE with
>> MODULE_ALIAS?
>
> What would actually go into MODULE_DEVICE_TABLE?

The table you have few lines above aliases.

>
> The platform devices created are contingent upon what was found during
> the top level ACP driver probe. You don't want all the "child" platform
> drivers to load unless they're needed.

How static alias differs here from static device ID table? Both are
built into the module and always there. I don't even understand what
does it mean by "loading child platform drivers". Why would unneeded
driver be loaded?

Best regards,
Krzysztof

2023-10-27 16:24:04

by Mark Brown

[permalink] [raw]
Subject: Re: [PATCH 09/13] ASoC: amd: acp: add machine driver support for pdm use case

On Fri, Oct 27, 2023 at 10:54:47AM -0500, Mario Limonciello wrote:

> What would actually go into MODULE_DEVICE_TABLE?

> The platform devices created are contingent upon what was found during the
> top level ACP driver probe. You don't want all the "child" platform drivers
> to load unless they're needed.

You want

MODULE_DEVICE_TABLE(platform, board_ids);

which is effectively the same as all the MODULE_ALIAS items you have
there (which can be removed).


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

2023-10-27 17:33:11

by syed saba kareem

[permalink] [raw]
Subject: Re: [PATCH 09/13] ASoC: amd: acp: add machine driver support for pdm use case


On 10/27/23 21:53, Mark Brown wrote:
> On Fri, Oct 27, 2023 at 10:54:47AM -0500, Mario Limonciello wrote:
>
>> What would actually go into MODULE_DEVICE_TABLE?
>> The platform devices created are contingent upon what was found during the
>> top level ACP driver probe. You don't want all the "child" platform drivers
>> to load unless they're needed.
> You want
>
> MODULE_DEVICE_TABLE(platform, board_ids);
>
> which is effectively the same as all the MODULE_ALIAS items you have
> there (which can be removed).

@krzk:as Mark Brown explained we can use platform device id table

instead of MODULE_ALIAS. As effectively there is no difference between

using platform device id table and MODULE_ALIAS.

If you are still expecting us to use platform id table instead of
MODULE_ALIAS

we will provide the changes as an incremental patch.

2023-10-28 09:17:39

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH 09/13] ASoC: amd: acp: add machine driver support for pdm use case

On 27/10/2023 19:32, syed saba kareem wrote:
>
> On 10/27/23 21:53, Mark Brown wrote:
>> On Fri, Oct 27, 2023 at 10:54:47AM -0500, Mario Limonciello wrote:
>>
>>> What would actually go into MODULE_DEVICE_TABLE?
>>> The platform devices created are contingent upon what was found during the
>>> top level ACP driver probe. You don't want all the "child" platform drivers
>>> to load unless they're needed.
>> You want
>>
>> MODULE_DEVICE_TABLE(platform, board_ids);
>>
>> which is effectively the same as all the MODULE_ALIAS items you have
>> there (which can be removed).
>
> @krzk:as Mark Brown explained we can use platform device id table
>
> instead of MODULE_ALIAS. As effectively there is no difference between
>
> using platform device id table and MODULE_ALIAS.

There is a difference. MODULE_DEVICE_TABLE solves the problem and you do
not need to spread aliases all over. This code is not equivalent. What's
more, DEVICE_TABLE could be used for other purposes like dependency
detection or ordering or whatever. ALIAS not.

>
> If you are still expecting us to use platform id table instead of
> MODULE_ALIAS

Yes, I asked this first time.

>
> we will provide the changes as an incremental patch.

Fix existing driver before adding new aliases. Then don't add ALIAS, how
I asked already two times before.

Best regards,
Krzysztof