On 5/16/23 05:35, Vijendar Mukunda wrote:
Create platform devices for Soundwire Manager instances and PDM controller based on ACP pin config selection and ACPI fw handle for pink sardine platform.
Signed-off-by: Vijendar Mukunda Vijendar.Mukunda@amd.com
sound/soc/amd/ps/acp63.h | 43 ++++++- sound/soc/amd/ps/pci-ps.c | 250 ++++++++++++++++++++++++++++++++++++-- 2 files changed, 280 insertions(+), 13 deletions(-)
diff --git a/sound/soc/amd/ps/acp63.h b/sound/soc/amd/ps/acp63.h index 2f94448102d0..f27f71116598 100644 --- a/sound/soc/amd/ps/acp63.h +++ b/sound/soc/amd/ps/acp63.h @@ -10,7 +10,7 @@ #define ACP_DEVICE_ID 0x15E2 #define ACP63_REG_START 0x1240000 #define ACP63_REG_END 0x1250200 -#define ACP63_DEVS 3 +#define ACP63_DEVS 5
#define ACP_SOFT_RESET_SOFTRESET_AUDDONE_MASK 0x00010001 #define ACP_PGFSM_CNTL_POWER_ON_MASK 1 @@ -55,8 +55,14 @@
#define ACP63_DMIC_ADDR 2 #define ACP63_PDM_MODE_DEVS 3 -#define ACP63_PDM_DEV_MASK 1 #define ACP_DMIC_DEV 2 +#define ACP63_SDW0_MODE_DEVS 2 +#define ACP63_SDW0_SDW1_MODE_DEVS 3 +#define ACP63_SDW0_PDM_MODE_DEVS 4 +#define ACP63_SDW0_SDW1_PDM_MODE_DEVS 5 +#define ACP63_DMIC_ADDR 2 +#define ACP63_SDW_ADDR 5 +#define AMD_SDW_MAX_MANAGERS 2
/* time in ms for acp timeout */ #define ACP_TIMEOUT 500 @@ -80,6 +86,12 @@ enum acp_config { ACP_CONFIG_15, };
+enum acp_pdev_mask {
- ACP63_PDM_DEV_MASK = 1,
- ACP63_SDW_DEV_MASK,
- ACP63_SDW_PDM_DEV_MASK,
+};
a comment or kernel-doc wouldn't hurt to explain the difference between ACP63_PDM_DEV_MASK and ACP63_SDW_PDM_DEV_MASK, the meaning of the 'SDW" prefix is far from obvious.
struct pdm_stream_instance { u16 num_pages; u16 channels; @@ -95,14 +107,41 @@ struct pdm_dev_data { struct snd_pcm_substream *capture_stream; };
+/**
- struct acp63_dev_data - acp pci driver context
- @acp63_base: acp mmio base
- @res: resource
- @pdev: child platform device node structures
array of platform devices ?
- @acp_lock: used to protect acp common registers
- @sdw_fw_node: SoundWire controller fw node handle
- @pdev_mask: platform device mask
- @pdev_count: platform devices count
- @pdm_dev_index: pdm platform device index
- @sdw0_dev_index: Soundwire Manager-0 platform device index
- @sdw1_dev_index: Soundwire Manager-1 platform device index
- @sdw_dma_dev_index: Soundwire DMA controller platform device index
- @is_dmic_dev: flag set to true if DMIC device exists
it's not clear what 'DMIC device' refers to here and why it is exposed here.
- @is_sdw_dev: flag set to true if Soundwire manager devices exists
same, what does this track? What is the difference between 'dmic dev' and 'sdw_dev', given that there are references above to SDW_PDM. It's confusing.
- @acp_reset: flag set to true when bus reset is applied across all
- the active soundwire manager instances
SoundWire
- */
struct acp63_dev_data { void __iomem *acp63_base; struct resource *res; struct platform_device *pdev[ACP63_DEVS]; struct mutex acp_lock; /* protect shared registers */
- struct fwnode_handle *sdw_fw_node; u16 pdev_mask; u16 pdev_count; u16 pdm_dev_index;
- u8 sdw_manager_count;
- u16 sdw0_dev_index;
- u16 sdw1_dev_index;
- u16 sdw_dma_dev_index;
- bool is_dmic_dev;
- bool is_sdw_dev;
- bool acp_reset;
};
int snd_amd_acp_find_config(struct pci_dev *pci); diff --git a/sound/soc/amd/ps/pci-ps.c b/sound/soc/amd/ps/pci-ps.c index c957718abefc..f3aa08dc05b2 100644 --- a/sound/soc/amd/ps/pci-ps.c +++ b/sound/soc/amd/ps/pci-ps.c @@ -6,6 +6,7 @@ */
#include <linux/pci.h> +#include <linux/bitops.h> #include <linux/module.h> #include <linux/io.h> #include <linux/delay.h> @@ -15,6 +16,7 @@ #include <sound/pcm_params.h> #include <linux/pm_runtime.h> #include <linux/iopoll.h> +#include <linux/soundwire/sdw_amd.h>
#include "acp63.h"
@@ -119,12 +121,70 @@ static irqreturn_t acp63_irq_handler(int irq, void *dev_id) return IRQ_NONE; }
-static void get_acp63_device_config(u32 config, struct pci_dev *pci,
struct acp63_dev_data *acp_data)+static int sdw_amd_scan_controller(struct device *dev) +{
- struct acp63_dev_data *acp_data;
- struct fwnode_handle *link;
- char name[32];
- u32 sdw_manager_bitmap;
- u8 count = 0;
- u32 acp_sdw_power_mode = 0;
- int index;
- int ret;
- acp_data = dev_get_drvdata(dev);
- acp_data->acp_reset = true;
- /* Found controller, find links supported */
- ret = fwnode_property_read_u32_array((acp_data->sdw_fw_node), "mipi-sdw-manager-list",
&sdw_manager_bitmap, 1);- if (ret) {
dev_err(dev, "Failed to read mipi-sdw-manager-list: %d\n", ret);return -EINVAL;- }
- count = hweight32(sdw_manager_bitmap);
- /* Check count is within bounds */
- if (count > AMD_SDW_MAX_MANAGERS) {
dev_err(dev, "Manager count %d exceeds max %d\n", count, AMD_SDW_MAX_MANAGERS);return -EINVAL;- }
- if (!count) {
dev_dbg(dev, "No SoundWire Managers detected\n");return -EINVAL;- }
- dev_dbg(dev, "ACPI reports %d Soundwire Manager devices\n", count);
- acp_data->sdw_manager_count = count;
- for (index = 0; index < count; index++) {
snprintf(name, sizeof(name), "mipi-sdw-link-%d-subproperties", index);link = fwnode_get_named_child_node(acp_data->sdw_fw_node, name);if (!link) {dev_err(dev, "Manager node %s not found\n", name);return -EIO;}fwnode_property_read_u32(link, "amd-sdw-power-mode",&acp_sdw_power_mode);/** when soundwire configuration is selected from acp pin config,* based on manager instances count, acp init/de-init sequence should be* executed as part of PM ops only when Bus reset is applied for the active* soundwire manager instances.
try to be consistent and use the MIPI spelling in comments: "SoundWire".
*/if (acp_sdw_power_mode != AMD_SDW_POWER_OFF_MODE)acp_data->acp_reset = false;- }
- return 0;
+}
+static int get_acp63_device_config(u32 config, struct pci_dev *pci, struct acp63_dev_data *acp_data) { struct acpi_device *dmic_dev;
struct acpi_device *sdw_dev; const union acpi_object *obj; bool is_dmic_dev = false;
bool is_sdw_dev = false;
int ret;
dmic_dev = acpi_find_child_device(ACPI_COMPANION(&pci->dev), ACP63_DMIC_ADDR, 0); if (dmic_dev) {
@@ -134,22 +194,83 @@ static void get_acp63_device_config(u32 config, struct pci_dev *pci, is_dmic_dev = true; }
- sdw_dev = acpi_find_child_device(ACPI_COMPANION(&pci->dev), ACP63_SDW_ADDR, 0);
- if (sdw_dev) {
acp_data->sdw_fw_node = acpi_fwnode_handle(sdw_dev);ret = sdw_amd_scan_controller(&pci->dev);if (!ret)is_sdw_dev = true;- }
- dev_dbg(&pci->dev, "Audio Mode %d\n", config); switch (config) {
- case ACP_CONFIG_0:
- case ACP_CONFIG_1:
- case ACP_CONFIG_4:
- case ACP_CONFIG_5:
- case ACP_CONFIG_10:
- case ACP_CONFIG_11:
if (is_dmic_dev) {acp_data->pdev_mask = ACP63_PDM_DEV_MASK;acp_data->pdev_count = ACP63_PDM_MODE_DEVS;}break;
- case ACP_CONFIG_9:
- case ACP_CONFIG_15:
dev_dbg(&pci->dev, "Audio Mode %d\n", config);
if (is_sdw_dev) {switch (acp_data->sdw_manager_count) {case 1:acp_data->pdev_mask = ACP63_SDW_DEV_MASK;acp_data->pdev_count = ACP63_SDW0_MODE_DEVS;break;case 2:acp_data->pdev_mask = ACP63_SDW_DEV_MASK;acp_data->pdev_count = ACP63_SDW0_SDW1_MODE_DEVS;break;default:return -EINVAL;}}
- default:
if (is_dmic_dev) {
- case ACP_CONFIG_6:
- case ACP_CONFIG_7:
- case ACP_CONFIG_12:
- case ACP_CONFIG_8:
- case ACP_CONFIG_13:
- case ACP_CONFIG_14:
if (is_dmic_dev && is_sdw_dev) {switch (acp_data->sdw_manager_count) {case 1:acp_data->pdev_mask = ACP63_SDW_PDM_DEV_MASK;acp_data->pdev_count = ACP63_SDW0_PDM_MODE_DEVS;break;case 2:acp_data->pdev_mask = ACP63_SDW_PDM_DEV_MASK;acp_data->pdev_count = ACP63_SDW0_SDW1_PDM_MODE_DEVS;break;default:return -EINVAL;}} else if (is_dmic_dev) { acp_data->pdev_mask = ACP63_PDM_DEV_MASK; acp_data->pdev_count = ACP63_PDM_MODE_DEVS;} else if (is_sdw_dev) {switch (acp_data->sdw_manager_count) {case 1:acp_data->pdev_mask = ACP63_SDW_DEV_MASK;acp_data->pdev_count = ACP63_SDW0_MODE_DEVS;break;case 2:acp_data->pdev_mask = ACP63_SDW_DEV_MASK;acp_data->pdev_count = ACP63_SDW0_SDW1_MODE_DEVS;break;default:return -EINVAL;}- default:
break;
it really wouldn't hurt to have a high-level description of this series of switch/ifs/else, it's very hard to review.
}
- return 0;
}
static void acp63_fill_platform_dev_info(struct platform_device_info *pdevinfo, @@ -173,6 +294,7 @@ static void acp63_fill_platform_dev_info(struct platform_device_info *pdevinfo,
static int create_acp63_platform_devs(struct pci_dev *pci, struct acp63_dev_data *adata, u32 addr) {
- struct acp_sdw_pdata *sdw_pdata; struct platform_device_info pdevinfo[ACP63_DEVS]; struct device *parent; int index;
@@ -205,8 +327,110 @@ static int create_acp63_platform_devs(struct pci_dev *pci, struct acp63_dev_data acp63_fill_platform_dev_info(&pdevinfo[2], parent, NULL, "acp_ps_mach", 0, NULL, 0, NULL, 0); break;
- case ACP63_SDW_DEV_MASK:
if (adata->pdev_count == ACP63_SDW0_MODE_DEVS) {sdw_pdata = devm_kzalloc(&pci->dev, sizeof(struct acp_sdw_pdata),GFP_KERNEL);if (!sdw_pdata) {ret = -ENOMEM;goto de_init;}sdw_pdata->instance = 0;sdw_pdata->acp_sdw_lock = &adata->acp_lock;adata->sdw0_dev_index = 0;adata->sdw_dma_dev_index = 1;acp63_fill_platform_dev_info(&pdevinfo[0], parent, adata->sdw_fw_node,"amd_sdw_manager", 0, adata->res, 1,sdw_pdata, sizeof(struct acp_sdw_pdata));acp63_fill_platform_dev_info(&pdevinfo[1], parent, NULL, "amd_ps_sdw_dma",0, adata->res, 1, &adata->acp_lock,sizeof(adata->acp_lock));} else if (adata->pdev_count == ACP63_SDW0_SDW1_MODE_DEVS) {sdw_pdata = devm_kzalloc(&pci->dev, sizeof(struct acp_sdw_pdata) * 2,GFP_KERNEL);if (!sdw_pdata) {ret = -ENOMEM;goto de_init;}sdw_pdata[0].instance = 0;sdw_pdata[1].instance = 1;sdw_pdata[0].acp_sdw_lock = &adata->acp_lock;sdw_pdata[1].acp_sdw_lock = &adata->acp_lock;sdw_pdata->acp_sdw_lock = &adata->acp_lock;adata->sdw0_dev_index = 0;adata->sdw1_dev_index = 1;adata->sdw_dma_dev_index = 2;acp63_fill_platform_dev_info(&pdevinfo[0], parent, adata->sdw_fw_node,"amd_sdw_manager", 0, adata->res, 1,&sdw_pdata[0], sizeof(struct acp_sdw_pdata));acp63_fill_platform_dev_info(&pdevinfo[1], parent, adata->sdw_fw_node,"amd_sdw_manager", 1, adata->res, 1,&sdw_pdata[1], sizeof(struct acp_sdw_pdata));acp63_fill_platform_dev_info(&pdevinfo[2], parent, NULL, "amd_ps_sdw_dma",0, adata->res, 1, &adata->acp_lock,sizeof(adata->acp_lock));}break;- case ACP63_SDW_PDM_DEV_MASK:
if (adata->pdev_count == ACP63_SDW0_PDM_MODE_DEVS) {sdw_pdata = devm_kzalloc(&pci->dev, sizeof(struct acp_sdw_pdata),GFP_KERNEL);if (!sdw_pdata) {ret = -ENOMEM;goto de_init;}sdw_pdata->instance = 0;sdw_pdata->acp_sdw_lock = &adata->acp_lock;adata->pdm_dev_index = 0;adata->sdw0_dev_index = 1;adata->sdw_dma_dev_index = 2;acp63_fill_platform_dev_info(&pdevinfo[0], parent, NULL, "acp_ps_pdm_dma",0, adata->res, 1, &adata->acp_lock,sizeof(adata->acp_lock));acp63_fill_platform_dev_info(&pdevinfo[1], parent, adata->sdw_fw_node,"amd_sdw_manager", 0, adata->res, 1,sdw_pdata, sizeof(struct acp_sdw_pdata));acp63_fill_platform_dev_info(&pdevinfo[2], parent, NULL, "amd_ps_sdw_dma",0, adata->res, 1, &adata->acp_lock,sizeof(adata->acp_lock));acp63_fill_platform_dev_info(&pdevinfo[3], parent, NULL, "dmic-codec",0, NULL, 0, NULL, 0);} else if (adata->pdev_count == ACP63_SDW0_SDW1_PDM_MODE_DEVS) {sdw_pdata = devm_kzalloc(&pci->dev, sizeof(struct acp_sdw_pdata) * 2,GFP_KERNEL);if (!sdw_pdata) {ret = -ENOMEM;goto de_init;}sdw_pdata[0].instance = 0;sdw_pdata[1].instance = 1;sdw_pdata[0].acp_sdw_lock = &adata->acp_lock;sdw_pdata[1].acp_sdw_lock = &adata->acp_lock;adata->pdm_dev_index = 0;adata->sdw0_dev_index = 1;adata->sdw1_dev_index = 2;adata->sdw_dma_dev_index = 3;acp63_fill_platform_dev_info(&pdevinfo[0], parent, NULL, "acp_ps_pdm_dma",0, adata->res, 1, &adata->acp_lock,sizeof(adata->acp_lock));acp63_fill_platform_dev_info(&pdevinfo[1], parent, adata->sdw_fw_node,"amd_sdw_manager", 0, adata->res, 1,&sdw_pdata[0], sizeof(struct acp_sdw_pdata));acp63_fill_platform_dev_info(&pdevinfo[2], parent, adata->sdw_fw_node,"amd_sdw_manager", 1, adata->res, 1,&sdw_pdata[1], sizeof(struct acp_sdw_pdata));acp63_fill_platform_dev_info(&pdevinfo[3], parent, NULL, "amd_ps_sdw_dma",0, adata->res, 1, &adata->acp_lock,sizeof(adata->acp_lock));acp63_fill_platform_dev_info(&pdevinfo[4], parent, NULL, "dmic-codec",0, NULL, 0, NULL, 0);}break;
dev_dbg(&pci->dev, "No PDM devices found\n");
dev_dbg(&pci->dev, "No PDM or Soundwire manager devices found\n");
what does this mean? I find this debug adds more confusion.
return 0;}
@@ -290,7 +514,11 @@ static int snd_acp63_probe(struct pci_dev *pci, goto de_init; } val = readl(adata->acp63_base + ACP_PIN_CONFIG);
- get_acp63_device_config(val, pci, adata);
- ret = get_acp63_device_config(val, pci, adata);
- if (ret) {
dev_err(&pci->dev, "get acp device config failed:%d\n", ret);goto de_init;- } ret = create_acp63_platform_devs(pci, adata, addr); if (ret < 0) { dev_err(&pci->dev, "ACP platform devices creation failed\n");