[Sound-open-firmware] [PATCH v2 0/5] Add DSP node for i.MX8QXP board to be used by DSP SOF driver
i.MX8QXP boards feature an Hifi4 DSP from Tensilica.
This patch series aims on adding the DT node describing the DSP, but it also contains the Linux SOF DSP driver that will use the DT node for easier review.
Note that we switched to the new yaml format for bindings documentation.
The DSP will run SOF Firmware [1]. Patches 1,2,3 are adding support for Linux DSP driver are already sent for review to SOF folks [2]
[1] https://github.com/thesofproject/sof [2] https://github.com/thesofproject/linux/pull/1048/commits
Changes since v1: - removed 'clk: imx8: Add DSP related clocks' as it was already applied by Shawn - add patches adding support for Linux DSP driver to make things clear for review - add maxItems property for PM in DT bindings doc
Daniel Baluta (5): ASoC: SOF: imx: Add i.MX8 HW support ASoC: SOF: topology: Add dummy support for i.MX8 DAIs ASoC: SOF: Add DT DSP device support arm64: dts: imx8qxp: Add DSP DT node dt-bindings: dsp: fsl: Add DSP core binding support
.../devicetree/bindings/dsp/fsl,dsp.yaml | 87 ++++ arch/arm64/boot/dts/freescale/imx8qxp-mek.dts | 4 + arch/arm64/boot/dts/freescale/imx8qxp.dtsi | 32 ++ include/sound/sof/dai.h | 2 + include/uapi/sound/sof/tokens.h | 8 + sound/soc/sof/Kconfig | 10 + sound/soc/sof/Makefile | 4 + sound/soc/sof/imx/Kconfig | 21 + sound/soc/sof/imx/Makefile | 7 + sound/soc/sof/imx/imx8.c | 464 ++++++++++++++++++ sound/soc/sof/sof-dt-dev.c | 159 ++++++ sound/soc/sof/topology.c | 30 ++ 12 files changed, 828 insertions(+) create mode 100644 Documentation/devicetree/bindings/dsp/fsl,dsp.yaml create mode 100644 sound/soc/sof/imx/Kconfig create mode 100644 sound/soc/sof/imx/Makefile create mode 100644 sound/soc/sof/imx/imx8.c create mode 100644 sound/soc/sof/sof-dt-dev.c
Add support for the audio DSP hardware found on NXP i.MX8 platform.
Signed-off-by: Daniel Baluta daniel.baluta@nxp.com --- - This is also on review with SOF community here: https://github.com/thesofproject/linux/pull/1048
sound/soc/sof/Kconfig | 1 + sound/soc/sof/Makefile | 1 + sound/soc/sof/imx/Kconfig | 20 ++ sound/soc/sof/imx/Makefile | 7 + sound/soc/sof/imx/imx8.c | 464 +++++++++++++++++++++++++++++++++++++ 5 files changed, 493 insertions(+) create mode 100644 sound/soc/sof/imx/Kconfig create mode 100644 sound/soc/sof/imx/Makefile create mode 100644 sound/soc/sof/imx/imx8.c
diff --git a/sound/soc/sof/Kconfig b/sound/soc/sof/Kconfig index fb01f0ca6027..61b97fc55bb2 100644 --- a/sound/soc/sof/Kconfig +++ b/sound/soc/sof/Kconfig @@ -163,6 +163,7 @@ config SND_SOC_SOF_PROBE_WORK_QUEUE When selected, the probe is handled in two steps, for example to avoid lockdeps if request_module is used in the probe.
+source "sound/soc/sof/imx/Kconfig" source "sound/soc/sof/intel/Kconfig" source "sound/soc/sof/xtensa/Kconfig"
diff --git a/sound/soc/sof/Makefile b/sound/soc/sof/Makefile index 8f14c9d2950b..6e43d411ffef 100644 --- a/sound/soc/sof/Makefile +++ b/sound/soc/sof/Makefile @@ -15,4 +15,5 @@ obj-$(CONFIG_SND_SOC_SOF_ACPI) += sof-acpi-dev.o obj-$(CONFIG_SND_SOC_SOF_PCI) += sof-pci-dev.o
obj-$(CONFIG_SND_SOC_SOF_INTEL_TOPLEVEL) += intel/ +obj-$(CONFIG_SND_SOC_SOF_IMX_TOPLEVEL) += imx/ obj-$(CONFIG_SND_SOC_SOF_XTENSA) += xtensa/ diff --git a/sound/soc/sof/imx/Kconfig b/sound/soc/sof/imx/Kconfig new file mode 100644 index 000000000000..fff64a9970f0 --- /dev/null +++ b/sound/soc/sof/imx/Kconfig @@ -0,0 +1,20 @@ +# SPDX-License-Identifier: (GPL-2.0 OR BSD-3-Clause) + +config SND_SOC_SOF_IMX_TOPLEVEL + bool "SOF support for NXP i.MX audio DSPs" + depends on ARM64 || COMPILE_TEST + help + This adds support for Sound Open Firmware for NXP i.MX platforms. + Say Y if you have such a device. + If unsure select "N". + +if SND_SOC_SOF_IMX_TOPLEVEL + +config SND_SOC_SOF_IMX8 + tristate "SOF support for i.MX8" + help + This adds support for Sound Open Firmware for NXP i.MX8 platforms + Say Y if you have such a device. + If unsure select "N". + +endif ## SND_SOC_SOF_IMX_IMX_TOPLEVEL diff --git a/sound/soc/sof/imx/Makefile b/sound/soc/sof/imx/Makefile new file mode 100644 index 000000000000..c69237971da5 --- /dev/null +++ b/sound/soc/sof/imx/Makefile @@ -0,0 +1,7 @@ +# SPDX-License-Identifier: (GPL-2.0 OR BSD-3-Clause) + +ccflags-y += -DDEBUG + +snd-sof-imx8-objs := imx8.o + +obj-$(CONFIG_SND_SOC_SOF_IMX8) += snd-sof-imx8.o diff --git a/sound/soc/sof/imx/imx8.c b/sound/soc/sof/imx/imx8.c new file mode 100644 index 000000000000..086606aa4b20 --- /dev/null +++ b/sound/soc/sof/imx/imx8.c @@ -0,0 +1,464 @@ +// SPDX-License-Identifier: (GPL-2.0 OR BSD-3-Clause) +// +// Copyright 2019 NXP +// +// Author: Daniel Baluta daniel.baluta@nxp.com +// +// Hardware interface for audio DSP on i.MX8 + +#include <linux/firmware.h> +#include <linux/of_platform.h> +#include <linux/of_address.h> +#include <linux/of_irq.h> +#include <linux/pm_domain.h> + +#include <linux/module.h> +#include <sound/sof.h> +#include <sound/sof/xtensa.h> +#include <linux/firmware/imx/ipc.h> +#include <linux/firmware/imx/dsp.h> + +#include <linux/firmware/imx/svc/misc.h> +#include <dt-bindings/firmware/imx/rsrc.h> +#include "../ops.h" + +/* DSP memories */ +#define IRAM_OFFSET 0x10000 +#define IRAM_SIZE (2 * 1024) +#define DRAM0_OFFSET 0x0 +#define DRAM0_SIZE (32 * 1024) +#define DRAM1_OFFSET 0x8000 +#define DRAM1_SIZE (32 * 1024) +#define SYSRAM_OFFSET 0x18000 +#define SYSRAM_SIZE (256 * 1024) +#define SYSROM_OFFSET 0x58000 +#define SYSROM_SIZE (192 * 1024) + +#define RESET_VECTOR_VADDR 0x596f8000 + +#define MBOX_OFFSET 0x800000 +#define MBOX_SIZE 0x1000 + +struct imx8_priv { + struct device *dev; + struct snd_sof_dev *sdev; + struct imx_dsp_ipc *dsp_ipc; + struct imx_sc_ipc *sc_ipc; +}; + +static void imx8_get_windows(struct snd_sof_dev *sdev) +{ + struct sof_ipc_window_elem *elem; + u32 outbox_offset = 0; + u32 stream_offset = 0; + u32 inbox_offset = 0; + u32 outbox_size = 0; + u32 stream_size = 0; + u32 inbox_size = 0; + int i; + + if (!sdev->info_window) { + dev_err(sdev->dev, "error: have no window info\n"); + return; + } + + for (i = 0; i < sdev->info_window->num_windows; i++) { + elem = &sdev->info_window->window[i]; + + switch (elem->type) { + case SOF_IPC_REGION_UPBOX: + inbox_offset = elem->offset + MBOX_OFFSET; + inbox_size = elem->size; + snd_sof_debugfs_io_item(sdev, + sdev->bar[SOF_FW_BLK_TYPE_SRAM] + + inbox_offset, + elem->size, "inbox", + SOF_DEBUGFS_ACCESS_D0_ONLY); + break; + case SOF_IPC_REGION_DOWNBOX: + outbox_offset = elem->offset + MBOX_OFFSET; + outbox_size = elem->size; + snd_sof_debugfs_io_item(sdev, + sdev->bar[SOF_FW_BLK_TYPE_SRAM] + + outbox_offset, + elem->size, "outbox", + SOF_DEBUGFS_ACCESS_D0_ONLY); + break; + case SOF_IPC_REGION_TRACE: + snd_sof_debugfs_io_item(sdev, + sdev->bar[SOF_FW_BLK_TYPE_SRAM] + + elem->offset + MBOX_OFFSET, + elem->size, "etrace", + SOF_DEBUGFS_ACCESS_D0_ONLY); + break; + case SOF_IPC_REGION_DEBUG: + snd_sof_debugfs_io_item(sdev, + sdev->bar[SOF_FW_BLK_TYPE_SRAM] + + elem->offset + MBOX_OFFSET, + elem->size, "debug", + SOF_DEBUGFS_ACCESS_D0_ONLY); + break; + case SOF_IPC_REGION_STREAM: + stream_offset = elem->offset + MBOX_OFFSET; + stream_size = elem->size; + snd_sof_debugfs_io_item(sdev, + sdev->bar[SOF_FW_BLK_TYPE_SRAM] + + stream_offset, + elem->size, "stream", + SOF_DEBUGFS_ACCESS_D0_ONLY); + break; + case SOF_IPC_REGION_REGS: + snd_sof_debugfs_io_item(sdev, + sdev->bar[SOF_FW_BLK_TYPE_SRAM] + + elem->offset + MBOX_OFFSET, + elem->size, "regs", + SOF_DEBUGFS_ACCESS_D0_ONLY); + break; + case SOF_IPC_REGION_EXCEPTION: + sdev->dsp_oops_offset = elem->offset + MBOX_OFFSET; + snd_sof_debugfs_io_item(sdev, + sdev->bar[SOF_FW_BLK_TYPE_SRAM] + + elem->offset + MBOX_OFFSET, + elem->size, "exception", + SOF_DEBUGFS_ACCESS_D0_ONLY); + break; + default: + dev_err(sdev->dev, "error: get illegal window info\n"); + return; + } + } + + if (outbox_size == 0 || inbox_size == 0) { + dev_err(sdev->dev, "error: get illegal mailbox window\n"); + return; + } + + snd_sof_dsp_mailbox_init(sdev, inbox_offset, inbox_size, + outbox_offset, outbox_size); + sdev->stream_box.offset = stream_offset; + sdev->stream_box.size = stream_size; + + dev_dbg(sdev->dev, " mailbox upstream 0x%x - size 0x%x\n", + inbox_offset, inbox_size); + dev_dbg(sdev->dev, " mailbox downstream 0x%x - size 0x%x\n", + outbox_offset, outbox_size); + dev_dbg(sdev->dev, " stream region 0x%x - size 0x%x\n", + stream_offset, stream_size); +} + +/* + * IPC Firmware ready. + */ +static int imx8_fw_ready(struct snd_sof_dev *sdev, u32 msg_id) +{ + struct sof_ipc_fw_ready *fw_ready = &sdev->fw_ready; + u32 offset; + int ret; + + /* mailbox must be on 4k boundary */ + offset = MBOX_OFFSET; + + dev_dbg(sdev->dev, "ipc: DSP is ready 0x%8.8x offset 0x%x\n", + msg_id, offset); + + /* no need to re-check version/ABI for subsequent boots */ + if (!sdev->first_boot) + return 0; + + /* copy data from the DSP FW ready offset */ + sof_block_read(sdev, sdev->mailbox_bar, offset, fw_ready, + sizeof(*fw_ready)); + snd_sof_dsp_mailbox_init(sdev, fw_ready->dspbox_offset, + fw_ready->dspbox_size, + fw_ready->hostbox_offset, + fw_ready->hostbox_size); + + /* make sure ABI version is compatible */ + ret = snd_sof_ipc_valid(sdev); + if (ret < 0) + return ret; + + /* now check for extended data */ + snd_sof_fw_parse_ext_data(sdev, SOF_FW_BLK_TYPE_SRAM, MBOX_OFFSET + + sizeof(struct sof_ipc_fw_ready)); + + imx8_get_windows(sdev); + + return 0; +} + +static void imx8_get_reply(struct snd_sof_dev *sdev) +{ + struct snd_sof_ipc_msg *msg = sdev->msg; + struct sof_ipc_reply reply; + unsigned long flags; + int ret = 0; + + if (!msg) { + dev_warn(sdev->dev, "unexpected ipc interrupt\n"); + return; + } + + /* get reply */ + sof_mailbox_read(sdev, sdev->host_box.offset, &reply, sizeof(reply)); + + spin_lock_irqsave(&sdev->ipc_lock, flags); + + if (reply.error < 0) { + memcpy(msg->reply_data, &reply, sizeof(reply)); + ret = reply.error; + } else { + /* reply has correct size? */ + if (reply.hdr.size != msg->reply_size) { + dev_err(sdev->dev, "error: reply expected %zu got %u bytes\n", + msg->reply_size, reply.hdr.size); + ret = -EINVAL; + } + + /* read the message */ + if (msg->reply_size > 0) + sof_mailbox_read(sdev, sdev->host_box.offset, + msg->reply_data, msg->reply_size); + } + + msg->reply_error = ret; + + spin_unlock_irqrestore(&sdev->ipc_lock, flags); +} + +void imx_dsp_handle_reply(struct imx_dsp_ipc *ipc) +{ + struct imx8_priv *priv = imx_dsp_get_data(ipc); + + imx8_get_reply(priv->sdev); + snd_sof_ipc_reply(priv->sdev, 0); +} + +void imx_dsp_handle_request(struct imx_dsp_ipc *ipc) +{ + struct imx8_priv *priv = imx_dsp_get_data(ipc); + + snd_sof_ipc_msgs_rx(priv->sdev); +} + +struct imx_dsp_ops dsp_ops = { + .handle_reply = imx_dsp_handle_reply, + .handle_request = imx_dsp_handle_request, +}; + +static int imx8_send_msg(struct snd_sof_dev *sdev, struct snd_sof_ipc_msg *msg) +{ + struct imx8_priv *priv = (struct imx8_priv *)sdev->private; + + sof_mailbox_write(sdev, sdev->host_box.offset, msg->msg_data, + msg->msg_size); + imx_dsp_ring_doorbell(priv->dsp_ipc, 0); + + return 0; +} + +/* + * DSP control. + */ +static int imx8_run(struct snd_sof_dev *sdev) +{ + int ret; + struct imx8_priv *dsp_priv = (struct imx8_priv *)sdev->private; + + ret = imx_sc_misc_set_control(dsp_priv->sc_ipc, IMX_SC_R_DSP, + IMX_SC_C_OFS_SEL, 1); + if (ret < 0) { + dev_err(sdev->dev, "Error system address offset source select\n"); + return ret; + } + + ret = imx_sc_misc_set_control(dsp_priv->sc_ipc, IMX_SC_R_DSP, + IMX_SC_C_OFS_AUDIO, 0x80); + if (ret < 0) { + dev_err(sdev->dev, "Error system address offset of AUDIO\n"); + return ret; + } + + ret = imx_sc_misc_set_control(dsp_priv->sc_ipc, IMX_SC_R_DSP, + IMX_SC_C_OFS_PERIPH, 0x5A); + if (ret < 0) { + dev_err(sdev->dev, "Error system address offset of PERIPH %d\n", + ret); + return ret; + } + + ret = imx_sc_misc_set_control(dsp_priv->sc_ipc, IMX_SC_R_DSP, + IMX_SC_C_OFS_IRQ, 0x51); + if (ret < 0) { + dev_err(sdev->dev, "Error system address offset of IRQ\n"); + return ret; + } + + imx_sc_pm_cpu_start(dsp_priv->sc_ipc, IMX_SC_R_DSP, true, + RESET_VECTOR_VADDR); + + return 0; +} + +static int imx8_probe(struct snd_sof_dev *sdev) +{ + struct imx8_priv *priv; + int i; + struct platform_device *pdev = + container_of(sdev->dev, struct platform_device, dev); + struct platform_device *ipc_dev; + struct resource *mmio; + int num_domains = 0; + u32 base, size; + int ret = 0; + struct device_node *np = pdev->dev.of_node; + struct device_node *res_node; + struct resource res; + + priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL); + if (!priv) + return -ENOMEM; + + sdev->private = priv; + priv->dev = sdev->dev; + priv->sdev = sdev; + + ret = imx_scu_get_handle(&priv->sc_ipc); + if (ret) { + dev_err(sdev->dev, "Cannot obtain SCU handle (err = %d)\n", + ret); + return ret; + } + + ipc_dev = platform_device_register_data(sdev->dev, "imx-dsp", + PLATFORM_DEVID_NONE, + pdev, sizeof(*pdev)); + if (IS_ERR(ipc_dev)) { + dev_err(sdev->dev, "Failed to register platform device\n"); + return PTR_ERR(ipc_dev); + } + + priv->dsp_ipc = dev_get_drvdata(&ipc_dev->dev); + if (!priv->dsp_ipc) + return -EPROBE_DEFER; + + imx_dsp_set_data(priv->dsp_ipc, priv); + priv->dsp_ipc->ops = &dsp_ops; + + num_domains = of_count_phandle_with_args(np, "power-domains", + "#power-domain-cells"); + for (i = 0; i < num_domains; i++) { + struct device *pd_dev; + struct device_link *link; + + pd_dev = dev_pm_domain_attach_by_id(&pdev->dev, i); + if (IS_ERR(pd_dev)) + return PTR_ERR(pd_dev); + + link = device_link_add(&pdev->dev, pd_dev, + DL_FLAG_STATELESS | + DL_FLAG_PM_RUNTIME | + DL_FLAG_RPM_ACTIVE); + if (IS_ERR(link)) + return PTR_ERR(link); + } + + /* DSP base */ + mmio = platform_get_resource(pdev, IORESOURCE_MEM, 0); + if (mmio) { + base = mmio->start; + size = resource_size(mmio); + } else { + dev_err(sdev->dev, "error: failed to get DSP base at idx 0\n"); + return -EINVAL; + } + + sdev->bar[SOF_FW_BLK_TYPE_IRAM] = devm_ioremap(sdev->dev, base, size); + if (!sdev->bar[SOF_FW_BLK_TYPE_IRAM]) { + dev_err(sdev->dev, "failed to ioremap base 0x%x size 0x%x\n", + base, size); + return -ENODEV; + } + sdev->mmio_bar = SOF_FW_BLK_TYPE_IRAM; + + res_node = of_parse_phandle(np, "memory-region", 0); + if (!res_node) { + dev_err(&pdev->dev, "failed to get memory region node\n"); + return -ENODEV; + } + if (of_address_to_resource(res_node, 0, &res)) { + dev_err(&pdev->dev, "failed to get reserved region address\n"); + return -EINVAL; + } + + sdev->bar[SOF_FW_BLK_TYPE_SRAM] = devm_ioremap_wc(sdev->dev, res.start, + res.end - res.start + + 1); + if (!sdev->bar[SOF_FW_BLK_TYPE_SRAM]) { + dev_err(sdev->dev, "failed to ioremap mem 0x%x size 0x%x\n", + base, size); + return -ENODEV; + } + sdev->mailbox_bar = SOF_FW_BLK_TYPE_SRAM; + + return ret; +} + +/* on i.MX8 there is 1 to 1 match between type and BAR idx */ +int imx8_get_bar_index(struct snd_sof_dev *sdev, u32 type) +{ + return type; +} + +void imx8_ipc_msg_data(struct snd_sof_dev *sdev, + struct snd_pcm_substream *substream, + void *p, size_t sz) +{ + sof_mailbox_read(sdev, sdev->dsp_box.offset, p, sz); +} + +int imx8_ipc_pcm_params(struct snd_sof_dev *sdev, + struct snd_pcm_substream *substream, + const struct sof_ipc_pcm_params_reply *reply) +{ + return 0; +} + +static struct snd_soc_dai_driver imx8_dai[] = { +{ + .name = "esai-port", +}, +}; + +/* i.MX8 ops */ +struct snd_sof_dsp_ops sof_imx8_ops = { + /* device init */ + .probe = imx8_probe, + + /* DSP core boot */ + .run = imx8_run, + + /* Block IO */ + .block_read = sof_block_read, + .block_write = sof_block_write, + + /* ipc */ + .send_msg = imx8_send_msg, + .fw_ready = imx8_fw_ready, + + .ipc_msg_data = imx8_ipc_msg_data, + .ipc_pcm_params = imx8_ipc_pcm_params, + + /* module loading */ + .load_module = snd_sof_parse_module_memcpy, + .get_bar_index = imx8_get_bar_index, + /* firmware loading */ + .load_firmware = snd_sof_load_firmware_memcpy, + + /* DAI drivers */ + .drv = imx8_dai, + .num_drv = 1, /* we have only 1 ESAI interface on i.MX8 */ +}; +EXPORT_SYMBOL(sof_imx8_ops); + +MODULE_LICENSE("Dual BSD/GPL");
Add dummy support for SAI/ESAI digital audio interface IPs found on i.MX8 boards.
Signed-off-by: Daniel Baluta daniel.baluta@nxp.com --- This is also on review with SOF community here:
https://github.com/thesofproject/linux/pull/1048 include/sound/sof/dai.h | 2 ++ include/uapi/sound/sof/tokens.h | 8 ++++++++ sound/soc/sof/topology.c | 30 ++++++++++++++++++++++++++++++ 3 files changed, 40 insertions(+)
diff --git a/include/sound/sof/dai.h b/include/sound/sof/dai.h index 3d174e20aa53..ec3b5c080537 100644 --- a/include/sound/sof/dai.h +++ b/include/sound/sof/dai.h @@ -50,6 +50,8 @@ enum sof_ipc_dai_type { SOF_DAI_INTEL_DMIC, /**< Intel DMIC */ SOF_DAI_INTEL_HDA, /**< Intel HD/A */ SOF_DAI_INTEL_SOUNDWIRE, /**< Intel SoundWire */ + SOF_DAI_IMX_SAI, /**< i.MX SAI */ + SOF_DAI_IMX_ESAI, /**< i.MX ESAI */ };
/* general purpose DAI configuration */ diff --git a/include/uapi/sound/sof/tokens.h b/include/uapi/sound/sof/tokens.h index dc1b27daaac6..347ce10bfd2d 100644 --- a/include/uapi/sound/sof/tokens.h +++ b/include/uapi/sound/sof/tokens.h @@ -105,4 +105,12 @@ /* for backward compatibility */ #define SOF_TKN_EFFECT_TYPE SOF_TKN_PROCESS_TYPE
+/* SAI */ +#define SOF_TKN_IMX_SAI_FIRST_TOKEN 1000 +/* TODO: Add SAI tokens */ + +/* ESAI */ +#define SOF_TKN_IMX_ESAI_FIRST_TOKEN 1100 +/* TODO: Add ESAI tokens */ + #endif diff --git a/sound/soc/sof/topology.c b/sound/soc/sof/topology.c index 432ae343f960..b5399f520366 100644 --- a/sound/soc/sof/topology.c +++ b/sound/soc/sof/topology.c @@ -339,6 +339,8 @@ static const struct sof_dai_types sof_dais[] = { {"SSP", SOF_DAI_INTEL_SSP}, {"HDA", SOF_DAI_INTEL_HDA}, {"DMIC", SOF_DAI_INTEL_DMIC}, + {"SAI", SOF_DAI_IMX_SAI}, + {"ESAI", SOF_DAI_IMX_ESAI}, };
static enum sof_ipc_dai_type find_dai(const char *name) @@ -2457,6 +2459,26 @@ static int sof_link_ssp_load(struct snd_soc_component *scomp, int index, return ret; }
+static int sof_link_sai_load(struct snd_soc_component *scomp, int index, + struct snd_soc_dai_link *link, + struct snd_soc_tplg_link_config *cfg, + struct snd_soc_tplg_hw_config *hw_config, + struct sof_ipc_dai_config *config) +{ + /*TODO: Add implementation */ + return 0; +} + +static int sof_link_esai_load(struct snd_soc_component *scomp, int index, + struct snd_soc_dai_link *link, + struct snd_soc_tplg_link_config *cfg, + struct snd_soc_tplg_hw_config *hw_config, + struct sof_ipc_dai_config *config) +{ + /*TODO: Add implementation */ + return 0; +} + static int sof_link_dmic_load(struct snd_soc_component *scomp, int index, struct snd_soc_dai_link *link, struct snd_soc_tplg_link_config *cfg, @@ -2781,6 +2803,14 @@ static int sof_link_load(struct snd_soc_component *scomp, int index, ret = sof_link_hda_load(scomp, index, link, cfg, hw_config, &config); break; + case SOF_DAI_IMX_SAI: + ret = sof_link_sai_load(scomp, index, link, cfg, hw_config, + &config); + break; + case SOF_DAI_IMX_ESAI: + ret = sof_link_esai_load(scomp, index, link, cfg, hw_config, + &config); + break; default: dev_err(sdev->dev, "error: invalid DAI type %d\n", config.type); ret = -EINVAL;
Add support for device tree based SOF DSP devices.
Signed-off-by: Daniel Baluta daniel.baluta@nxp.com --- This is also on review with SOF community here: https://github.com/thesofproject/linux/pull/1048
sound/soc/sof/Kconfig | 9 +++ sound/soc/sof/Makefile | 3 + sound/soc/sof/imx/Kconfig | 1 + sound/soc/sof/sof-dt-dev.c | 159 +++++++++++++++++++++++++++++++++++++ 4 files changed, 172 insertions(+) create mode 100644 sound/soc/sof/sof-dt-dev.c
diff --git a/sound/soc/sof/Kconfig b/sound/soc/sof/Kconfig index 61b97fc55bb2..2aa3a1cdf60c 100644 --- a/sound/soc/sof/Kconfig +++ b/sound/soc/sof/Kconfig @@ -36,6 +36,15 @@ config SND_SOC_SOF_ACPI Say Y if you need this option If unsure select "N".
+config SND_SOC_SOF_DT + tristate "SOF DT enumeration support" + select SND_SOC_SOF + select SND_SOC_SOF_OPTIONS + help + This adds support for Device Tree enumeration. This option is + required to enable i.MX8 devices. + Say Y if you need this option. If unsure select "N". + config SND_SOC_SOF_OPTIONS tristate help diff --git a/sound/soc/sof/Makefile b/sound/soc/sof/Makefile index 6e43d411ffef..8025d9cec4fb 100644 --- a/sound/soc/sof/Makefile +++ b/sound/soc/sof/Makefile @@ -5,6 +5,8 @@ snd-sof-objs := core.o ops.o loader.o ipc.o pcm.o pm.o debug.o topology.o\
snd-sof-pci-objs := sof-pci-dev.o snd-sof-acpi-objs := sof-acpi-dev.o +snd-sof-dt-objs := sof-dt-dev.o + snd-sof-nocodec-objs := nocodec.o
obj-$(CONFIG_SND_SOC_SOF) += snd-sof.o @@ -12,6 +14,7 @@ obj-$(CONFIG_SND_SOC_SOF_NOCODEC) += snd-sof-nocodec.o
obj-$(CONFIG_SND_SOC_SOF_ACPI) += sof-acpi-dev.o +obj-$(CONFIG_SND_SOC_SOF_DT) += sof-dt-dev.o obj-$(CONFIG_SND_SOC_SOF_PCI) += sof-pci-dev.o
obj-$(CONFIG_SND_SOC_SOF_INTEL_TOPLEVEL) += intel/ diff --git a/sound/soc/sof/imx/Kconfig b/sound/soc/sof/imx/Kconfig index fff64a9970f0..fa35994a79c4 100644 --- a/sound/soc/sof/imx/Kconfig +++ b/sound/soc/sof/imx/Kconfig @@ -12,6 +12,7 @@ if SND_SOC_SOF_IMX_TOPLEVEL
config SND_SOC_SOF_IMX8 tristate "SOF support for i.MX8" + select SND_SOC_SOF_DT help This adds support for Sound Open Firmware for NXP i.MX8 platforms Say Y if you have such a device. diff --git a/sound/soc/sof/sof-dt-dev.c b/sound/soc/sof/sof-dt-dev.c new file mode 100644 index 000000000000..31429bbb5c7e --- /dev/null +++ b/sound/soc/sof/sof-dt-dev.c @@ -0,0 +1,159 @@ +// SPDX-License-Identifier: (GPL-2.0 OR BSD-3-Clause) +// +// Copyright 2019 NXP +// +// Author: Daniel Baluta daniel.baluta@nxp.com +// + +#include <linux/firmware.h> +#include <linux/module.h> +#include <linux/pm_runtime.h> +#include <sound/sof.h> + +#include "ops.h" + +extern struct snd_sof_dsp_ops sof_imx8_ops; + +static char *fw_path; +module_param(fw_path, charp, 0444); +MODULE_PARM_DESC(fw_path, "alternate path for SOF firmware."); + +static char *tplg_path; +module_param(tplg_path, charp, 0444); +MODULE_PARM_DESC(tplg_path, "alternate path for SOF topology."); + +/* platform specific devices */ +#if IS_ENABLED(CONFIG_SND_SOC_SOF_IMX8) +static struct sof_dev_desc sof_dt_imx8qxp_desc = { + .default_fw_path = "imx/sof", + .default_tplg_path = "imx/sof-tplg", + .nocodec_fw_filename = "sof-imx8.ri", + .nocodec_tplg_filename = "sof-imx8-nocodec.tplg", + .ops = &sof_imx8_ops, +}; +#endif + +static const struct dev_pm_ops sof_dt_pm = { + SET_SYSTEM_SLEEP_PM_OPS(snd_sof_suspend, snd_sof_resume) + SET_RUNTIME_PM_OPS(snd_sof_runtime_suspend, snd_sof_runtime_resume, + NULL) +}; + +static void sof_dt_probe_complete(struct device *dev) +{ + /* allow runtime_pm */ + pm_runtime_set_autosuspend_delay(dev, SND_SOF_SUSPEND_DELAY_MS); + pm_runtime_use_autosuspend(dev); + pm_runtime_enable(dev); +} + +static int sof_dt_probe(struct platform_device *pdev) +{ + struct device *dev = &pdev->dev; + const struct sof_dev_desc *desc; + /*TODO: create a generic snd_soc_xxx_mach */ + struct snd_soc_acpi_mach *mach; + struct snd_sof_pdata *sof_pdata; + const struct snd_sof_dsp_ops *ops; + int ret; + + dev_info(&pdev->dev, "DT DSP detected"); + + sof_pdata = devm_kzalloc(dev, sizeof(*sof_pdata), GFP_KERNEL); + if (!sof_pdata) + return -ENOMEM; + + desc = device_get_match_data(dev); + if (!desc) + return -ENODEV; + + /* get ops for platform */ + ops = desc->ops; + if (!ops) { + dev_err(dev, "error: no matching DT descriptor ops\n"); + return -ENODEV; + } + +#if IS_ENABLED(CONFIG_SND_SOC_SOF_FORCE_NOCODEC_MODE) + /* force nocodec mode */ + dev_warn(dev, "Force to use nocodec mode\n"); + mach = devm_kzalloc(dev, sizeof(*mach), GFP_KERNEL); + if (!mach) + return -ENOMEM; + ret = sof_nocodec_setup(dev, sof_pdata, mach, desc, ops); + if (ret < 0) + return ret; +#else + /* TODO: implement case where we actually have a codec */ + return -ENODEV; +#endif + + if (mach) + mach->mach_params.platform = dev_name(dev); + + sof_pdata->machine = mach; + sof_pdata->desc = desc; + sof_pdata->dev = &pdev->dev; + sof_pdata->platform = dev_name(dev); + + /* alternate fw and tplg filenames */ + if (fw_path) + sof_pdata->fw_filename_prefix = fw_path; + else + sof_pdata->fw_filename_prefix = + sof_pdata->desc->default_fw_path; + if (tplg_path) + sof_pdata->tplg_filename_prefix = tplg_path; + else + sof_pdata->tplg_filename_prefix = + sof_pdata->desc->default_tplg_path; + +#if IS_ENABLED(CONFIG_SND_SOC_SOF_PROBE_WORK_QUEUE) + /* set callback to enable runtime_pm */ + sof_pdata->sof_probe_complete = sof_dt_probe_complete; +#endif + /* call sof helper for DSP hardware probe */ + ret = snd_sof_device_probe(dev, sof_pdata); + if (ret) { + dev_err(dev, "error: failed to probe DSP hardware\n"); + return ret; + } + +#if !IS_ENABLED(CONFIG_SND_SOC_SOF_PROBE_WORK_QUEUE) + sof_dt_probe_complete(dev); +#endif + + return ret; +} + +static int sof_dt_remove(struct platform_device *pdev) +{ + pm_runtime_disable(&pdev->dev); + + /* call sof helper for DSP hardware remove */ + snd_sof_device_remove(&pdev->dev); + + return 0; +} + +static const struct of_device_id sof_dt_ids[] = { +#if IS_ENABLED(CONFIG_SND_SOC_SOF_IMX8) + { .compatible = "fsl,imx8qxp-dsp", .data = &sof_dt_imx8qxp_desc}, +#endif + { } +}; +MODULE_DEVICE_TABLE(of, sof_dt_ids); + +/* DT driver definition */ +static struct platform_driver snd_sof_dt_driver = { + .probe = sof_dt_probe, + .remove = sof_dt_remove, + .driver = { + .name = "sof-audio-dt", + .pm = &sof_dt_pm, + .of_match_table = sof_dt_ids + }, +}; +module_platform_driver(snd_sof_dt_driver); + +MODULE_LICENSE("Dual BSD/GPL");
This includes DSP reserved memory, ADMA DSP device and DSP MU communication channels description.
Signed-off-by: Daniel Baluta daniel.baluta@nxp.com --- arch/arm64/boot/dts/freescale/imx8qxp-mek.dts | 4 +++ arch/arm64/boot/dts/freescale/imx8qxp.dtsi | 32 +++++++++++++++++++ 2 files changed, 36 insertions(+)
diff --git a/arch/arm64/boot/dts/freescale/imx8qxp-mek.dts b/arch/arm64/boot/dts/freescale/imx8qxp-mek.dts index bfdada2db176..19468058e6ae 100644 --- a/arch/arm64/boot/dts/freescale/imx8qxp-mek.dts +++ b/arch/arm64/boot/dts/freescale/imx8qxp-mek.dts @@ -230,3 +230,7 @@ >; }; }; + +&adma_dsp { + status = "okay"; +}; diff --git a/arch/arm64/boot/dts/freescale/imx8qxp.dtsi b/arch/arm64/boot/dts/freescale/imx8qxp.dtsi index 05fa0b7f36bb..b6c408fb2b7f 100644 --- a/arch/arm64/boot/dts/freescale/imx8qxp.dtsi +++ b/arch/arm64/boot/dts/freescale/imx8qxp.dtsi @@ -113,6 +113,17 @@ interrupts = <GIC_PPI 9 IRQ_TYPE_LEVEL_HIGH>; };
+ reserved-memory { + #address-cells = <2>; + #size-cells = <2>; + ranges; + + dsp_reserved: dsp@92400000 { + reg = <0 0x92400000 0 0x2000000>; + no-map; + }; + }; + pmu { compatible = "arm,armv8-pmuv3"; interrupts = <GIC_PPI 7 IRQ_TYPE_LEVEL_HIGH>; @@ -204,6 +215,27 @@ #clock-cells = <1>; };
+ adma_dsp: dsp@596e8000 { + compatible = "fsl,imx8qxp-dsp"; + reg = <0x596e8000 0x88000>; + clocks = <&adma_lpcg IMX_ADMA_LPCG_DSP_IPG_CLK>, + <&adma_lpcg IMX_ADMA_LPCG_OCRAM_IPG_CLK>, + <&adma_lpcg IMX_ADMA_LPCG_DSP_CORE_CLK>; + clock-names = "ipg", "ocram", "core"; + power-domains = <&pd IMX_SC_R_MU_13A>, + <&pd IMX_SC_R_MU_13B>, + <&pd IMX_SC_R_DSP>, + <&pd IMX_SC_R_DSP_RAM>; + mbox-names = "txdb0", "txdb1", + "rxdb0", "rxdb1"; + mboxes = <&lsio_mu13 2 0>, + <&lsio_mu13 2 1>, + <&lsio_mu13 3 0>, + <&lsio_mu13 3 1>; + memory-region = <&dsp_reserved>; + status = "disabled"; + }; + adma_lpuart0: serial@5a060000 { compatible = "fsl,imx8qxp-lpuart", "fsl,imx7ulp-lpuart"; reg = <0x5a060000 0x1000>;
This describes the DSP device tree node.
Signed-off-by: Daniel Baluta daniel.baluta@nxp.com --- .../devicetree/bindings/dsp/fsl,dsp.yaml | 87 +++++++++++++++++++ 1 file changed, 87 insertions(+) create mode 100644 Documentation/devicetree/bindings/dsp/fsl,dsp.yaml
diff --git a/Documentation/devicetree/bindings/dsp/fsl,dsp.yaml b/Documentation/devicetree/bindings/dsp/fsl,dsp.yaml new file mode 100644 index 000000000000..d112486eda0e --- /dev/null +++ b/Documentation/devicetree/bindings/dsp/fsl,dsp.yaml @@ -0,0 +1,87 @@ +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/arm/freescale/fsl,dsp.yaml# +$schema: http://devicetree.org/meta-schemas/core.yaml# + +title: NXP i.MX8 DSP core + +maintainers: + - Daniel Baluta daniel.baluta@nxp.com + +description: | + Some boards from i.MX8 family contain a DSP core used for + advanced pre- and post- audio processing. + +properties: + compatible: + enum: + - fsl,imx8qxp-dsp + + reg: + description: Should contain register location and length + + clocks: + items: + - description: ipg clock + - description: ocram clock + - description: core clock + + clock-names: + items: + - const: ipg + - const: ocram + - const: core + + power-domains: + description: + List of phandle and PM domain specifier as documented in + Documentation/devicetree/bindings/power/power_domain.txt + maxItems: 4 + mboxes: + description: + List of <&phandle type channel> - 2 channels for TXDB, 2 channels for RXDB + (see mailbox/fsl,mu.txt) + maxItems: 4 + + mbox-names: + items: + - const: txdb0 + - const: txdb1 + - const: rxdb0 + - const: rxdb1 + + memory-region: + description: + phandle to a node describing reserved memory (System RAM memory) + used by DSP (see bindings/reserved-memory/reserved-memory.txt) + maxItems: 1 + +required: + - compatible + - reg + - clocks + - clock-names + - power-domains + - mboxes + - mbox-names + - memory-region + +examples: + - | + #include <dt-bindings/firmware/imx/rsrc.h> + #include <dt-bindings/clock/imx8-clock.h> + dsp@596e8000 { + compatbile = "fsl,imx8qxp-dsp"; + reg = <0x596e8000 0x88000>; + clocks = <&adma_lpcg IMX_ADMA_LPCG_DSP_IPG_CLK>, + <&adma_lpcg IMX_ADMA_LPCG_OCRAM_IPG_CLK>, + <&adma_lpcg IMX_ADMA_LPCG_DSP_CORE_CLK>; + clock-names = "ipg", "ocram", "core"; + power-domains = <&pd IMX_SC_R_MU_13A>, + <&pd IMX_SC_R_MU_13B>, + <&pd IMX_SC_R_DSP>, + <&pd IMX_SC_R_DSP_RAM>; + mbox-names = "txdb0", "txdb1", "rxdb0", "rxdb1"; + mboxes = <&lsio_mu13 2 0>, <&lsio_mu13 2 1>, <&lsio_mu13 3 0>, <&lsio_mu13 3 1>; + };
diff --git a/sound/soc/sof/imx/Makefile b/sound/soc/sof/imx/Makefile new file mode 100644 index 000000000000..c69237971da5 --- /dev/null +++ b/sound/soc/sof/imx/Makefile @@ -0,0 +1,7 @@ +# SPDX-License-Identifier: (GPL-2.0 OR BSD-3-Clause)
+ccflags-y += -DDEBUG
this should be removed or in a separate patch.
+struct imx8_priv {
- struct device *dev;
- struct snd_sof_dev *sdev;
- struct imx_dsp_ipc *dsp_ipc;
- struct imx_sc_ipc *sc_ipc;
maybe a comment to explain what 'sc' stands for?
+};
+static void imx8_get_windows(struct snd_sof_dev *sdev) +{
- struct sof_ipc_window_elem *elem;
- u32 outbox_offset = 0;
- u32 stream_offset = 0;
- u32 inbox_offset = 0;
- u32 outbox_size = 0;
- u32 stream_size = 0;
- u32 inbox_size = 0;
- int i;
- if (!sdev->info_window) {
dev_err(sdev->dev, "error: have no window info\n");return;- }
- for (i = 0; i < sdev->info_window->num_windows; i++) {
elem = &sdev->info_window->window[i];switch (elem->type) {case SOF_IPC_REGION_UPBOX:inbox_offset = elem->offset + MBOX_OFFSET;inbox_size = elem->size;snd_sof_debugfs_io_item(sdev,sdev->bar[SOF_FW_BLK_TYPE_SRAM]+ inbox_offset,elem->size, "inbox",SOF_DEBUGFS_ACCESS_D0_ONLY);break;case SOF_IPC_REGION_DOWNBOX:outbox_offset = elem->offset + MBOX_OFFSET;outbox_size = elem->size;snd_sof_debugfs_io_item(sdev,sdev->bar[SOF_FW_BLK_TYPE_SRAM]+ outbox_offset,elem->size, "outbox",SOF_DEBUGFS_ACCESS_D0_ONLY);break;case SOF_IPC_REGION_TRACE:snd_sof_debugfs_io_item(sdev,sdev->bar[SOF_FW_BLK_TYPE_SRAM]+ elem->offset + MBOX_OFFSET,elem->size, "etrace",SOF_DEBUGFS_ACCESS_D0_ONLY);break;case SOF_IPC_REGION_DEBUG:snd_sof_debugfs_io_item(sdev,sdev->bar[SOF_FW_BLK_TYPE_SRAM]+ elem->offset + MBOX_OFFSET,elem->size, "debug",SOF_DEBUGFS_ACCESS_D0_ONLY);break;case SOF_IPC_REGION_STREAM:stream_offset = elem->offset + MBOX_OFFSET;stream_size = elem->size;snd_sof_debugfs_io_item(sdev,sdev->bar[SOF_FW_BLK_TYPE_SRAM]+ stream_offset,elem->size, "stream",SOF_DEBUGFS_ACCESS_D0_ONLY);break;case SOF_IPC_REGION_REGS:snd_sof_debugfs_io_item(sdev,sdev->bar[SOF_FW_BLK_TYPE_SRAM]+ elem->offset + MBOX_OFFSET,elem->size, "regs",SOF_DEBUGFS_ACCESS_D0_ONLY);break;case SOF_IPC_REGION_EXCEPTION:sdev->dsp_oops_offset = elem->offset + MBOX_OFFSET;snd_sof_debugfs_io_item(sdev,sdev->bar[SOF_FW_BLK_TYPE_SRAM]+ elem->offset + MBOX_OFFSET,elem->size, "exception",SOF_DEBUGFS_ACCESS_D0_ONLY);break;default:dev_err(sdev->dev, "error: get illegal window info\n");return;}- }
- if (outbox_size == 0 || inbox_size == 0) {
dev_err(sdev->dev, "error: get illegal mailbox window\n");return;- }
- snd_sof_dsp_mailbox_init(sdev, inbox_offset, inbox_size,
outbox_offset, outbox_size);- sdev->stream_box.offset = stream_offset;
- sdev->stream_box.size = stream_size;
- dev_dbg(sdev->dev, " mailbox upstream 0x%x - size 0x%x\n",
inbox_offset, inbox_size);- dev_dbg(sdev->dev, " mailbox downstream 0x%x - size 0x%x\n",
outbox_offset, outbox_size);- dev_dbg(sdev->dev, " stream region 0x%x - size 0x%x\n",
stream_offset, stream_size);+}
This looks 100% similar to Baytrail?
+/*
- IPC Firmware ready.
- */
+static int imx8_fw_ready(struct snd_sof_dev *sdev, u32 msg_id) +{
- struct sof_ipc_fw_ready *fw_ready = &sdev->fw_ready;
- u32 offset;
- int ret;
- /* mailbox must be on 4k boundary */
- offset = MBOX_OFFSET;
- dev_dbg(sdev->dev, "ipc: DSP is ready 0x%8.8x offset 0x%x\n",
msg_id, offset);/* no need to re-check version/ABI for subsequent boots */- if (!sdev->first_boot)
return 0;- /* copy data from the DSP FW ready offset */
- sof_block_read(sdev, sdev->mailbox_bar, offset, fw_ready,
sizeof(*fw_ready));- snd_sof_dsp_mailbox_init(sdev, fw_ready->dspbox_offset,
fw_ready->dspbox_size,fw_ready->hostbox_offset,fw_ready->hostbox_size);- /* make sure ABI version is compatible */
- ret = snd_sof_ipc_valid(sdev);
- if (ret < 0)
return ret;- /* now check for extended data */
- snd_sof_fw_parse_ext_data(sdev, SOF_FW_BLK_TYPE_SRAM, MBOX_OFFSET +
sizeof(struct sof_ipc_fw_ready));- imx8_get_windows(sdev);
- return 0;
+}
That code looks nearly similar to the baytrail one except for the last line, we should look into factoring this.
+static void imx8_get_reply(struct snd_sof_dev *sdev) +{
- struct snd_sof_ipc_msg *msg = sdev->msg;
- struct sof_ipc_reply reply;
- unsigned long flags;
- int ret = 0;
- if (!msg) {
dev_warn(sdev->dev, "unexpected ipc interrupt\n");return;- }
- /* get reply */
- sof_mailbox_read(sdev, sdev->host_box.offset, &reply, sizeof(reply));
- spin_lock_irqsave(&sdev->ipc_lock, flags);
- if (reply.error < 0) {
memcpy(msg->reply_data, &reply, sizeof(reply));ret = reply.error;- } else {
/* reply has correct size? */if (reply.hdr.size != msg->reply_size) {dev_err(sdev->dev, "error: reply expected %zu got %u bytes\n",msg->reply_size, reply.hdr.size);ret = -EINVAL;}/* read the message */if (msg->reply_size > 0)sof_mailbox_read(sdev, sdev->host_box.offset,msg->reply_data, msg->reply_size);- }
- msg->reply_error = ret;
- spin_unlock_irqrestore(&sdev->ipc_lock, flags);
I don't see a spin_lock/unlock for the get_reply in the Intel code, is this necessary?
+}
+void imx_dsp_handle_reply(struct imx_dsp_ipc *ipc) +{
- struct imx8_priv *priv = imx_dsp_get_data(ipc);
- imx8_get_reply(priv->sdev);
- snd_sof_ipc_reply(priv->sdev, 0);
+}
+void imx_dsp_handle_request(struct imx_dsp_ipc *ipc) +{
- struct imx8_priv *priv = imx_dsp_get_data(ipc);
- snd_sof_ipc_msgs_rx(priv->sdev);
+}
+struct imx_dsp_ops dsp_ops = {
- .handle_reply = imx_dsp_handle_reply,
- .handle_request = imx_dsp_handle_request,
+};
+static int imx8_send_msg(struct snd_sof_dev *sdev, struct snd_sof_ipc_msg *msg) +{
- struct imx8_priv *priv = (struct imx8_priv *)sdev->private;
- sof_mailbox_write(sdev, sdev->host_box.offset, msg->msg_data,
msg->msg_size);- imx_dsp_ring_doorbell(priv->dsp_ipc, 0);
- return 0;
+}
+/*
- DSP control.
- */
+static int imx8_run(struct snd_sof_dev *sdev) +{
- int ret;
- struct imx8_priv *dsp_priv = (struct imx8_priv *)sdev->private;
- ret = imx_sc_misc_set_control(dsp_priv->sc_ipc, IMX_SC_R_DSP,
IMX_SC_C_OFS_SEL, 1);- if (ret < 0) {
dev_err(sdev->dev, "Error system address offset source select\n");return ret;- }
- ret = imx_sc_misc_set_control(dsp_priv->sc_ipc, IMX_SC_R_DSP,
IMX_SC_C_OFS_AUDIO, 0x80);- if (ret < 0) {
dev_err(sdev->dev, "Error system address offset of AUDIO\n");return ret;- }
- ret = imx_sc_misc_set_control(dsp_priv->sc_ipc, IMX_SC_R_DSP,
IMX_SC_C_OFS_PERIPH, 0x5A);- if (ret < 0) {
dev_err(sdev->dev, "Error system address offset of PERIPH %d\n",ret);return ret;- }
- ret = imx_sc_misc_set_control(dsp_priv->sc_ipc, IMX_SC_R_DSP,
IMX_SC_C_OFS_IRQ, 0x51);- if (ret < 0) {
dev_err(sdev->dev, "Error system address offset of IRQ\n");return ret;- }
- imx_sc_pm_cpu_start(dsp_priv->sc_ipc, IMX_SC_R_DSP, true,
RESET_VECTOR_VADDR);- return 0;
+}
+static int imx8_probe(struct snd_sof_dev *sdev) +{
- struct imx8_priv *priv;
- int i;
- struct platform_device *pdev =
container_of(sdev->dev, struct platform_device, dev);- struct platform_device *ipc_dev;
- struct resource *mmio;
- int num_domains = 0;
- u32 base, size;
- int ret = 0;
- struct device_node *np = pdev->dev.of_node;
- struct device_node *res_node;
- struct resource res;
nit-pick: can we reorder so that we have all counters last and a nice xmas-tree shape.
- priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);
- if (!priv)
return -ENOMEM;- sdev->private = priv;
- priv->dev = sdev->dev;
- priv->sdev = sdev;
- ret = imx_scu_get_handle(&priv->sc_ipc);
- if (ret) {
dev_err(sdev->dev, "Cannot obtain SCU handle (err = %d)\n",ret);return ret;- }
- ipc_dev = platform_device_register_data(sdev->dev, "imx-dsp",
PLATFORM_DEVID_NONE,pdev, sizeof(*pdev));- if (IS_ERR(ipc_dev)) {
dev_err(sdev->dev, "Failed to register platform device\n");return PTR_ERR(ipc_dev);- }
- priv->dsp_ipc = dev_get_drvdata(&ipc_dev->dev);
- if (!priv->dsp_ipc)
return -EPROBE_DEFER;- imx_dsp_set_data(priv->dsp_ipc, priv);
- priv->dsp_ipc->ops = &dsp_ops;
- num_domains = of_count_phandle_with_args(np, "power-domains",
"#power-domain-cells");- for (i = 0; i < num_domains; i++) {
struct device *pd_dev;struct device_link *link;pd_dev = dev_pm_domain_attach_by_id(&pdev->dev, i);if (IS_ERR(pd_dev))return PTR_ERR(pd_dev);link = device_link_add(&pdev->dev, pd_dev,DL_FLAG_STATELESS |DL_FLAG_PM_RUNTIME |DL_FLAG_RPM_ACTIVE);if (IS_ERR(link))return PTR_ERR(link);
Question: is the error flow final? Wondering if we release all the resources/memory/devices on errors?
Also are all the resources device-managed, I don't see a remove()?
- }
- /* DSP base */
- mmio = platform_get_resource(pdev, IORESOURCE_MEM, 0);
- if (mmio) {
base = mmio->start;size = resource_size(mmio);- } else {
dev_err(sdev->dev, "error: failed to get DSP base at idx 0\n");return -EINVAL;- }
- sdev->bar[SOF_FW_BLK_TYPE_IRAM] = devm_ioremap(sdev->dev, base, size);
- if (!sdev->bar[SOF_FW_BLK_TYPE_IRAM]) {
dev_err(sdev->dev, "failed to ioremap base 0x%x size 0x%x\n",base, size);return -ENODEV;- }
- sdev->mmio_bar = SOF_FW_BLK_TYPE_IRAM;
- res_node = of_parse_phandle(np, "memory-region", 0);
- if (!res_node) {
dev_err(&pdev->dev, "failed to get memory region node\n");return -ENODEV;- }
- if (of_address_to_resource(res_node, 0, &res)) {
dev_err(&pdev->dev, "failed to get reserved region address\n");return -EINVAL;- }
- sdev->bar[SOF_FW_BLK_TYPE_SRAM] = devm_ioremap_wc(sdev->dev, res.start,
res.end - res.start +1);- if (!sdev->bar[SOF_FW_BLK_TYPE_SRAM]) {
dev_err(sdev->dev, "failed to ioremap mem 0x%x size 0x%x\n",base, size);return -ENODEV;- }
- sdev->mailbox_bar = SOF_FW_BLK_TYPE_SRAM;
- return ret;
+}
+/* on i.MX8 there is 1 to 1 match between type and BAR idx */ +int imx8_get_bar_index(struct snd_sof_dev *sdev, u32 type) +{
- return type;
+}
+void imx8_ipc_msg_data(struct snd_sof_dev *sdev,
struct snd_pcm_substream *substream,void *p, size_t sz)+{
- sof_mailbox_read(sdev, sdev->dsp_box.offset, p, sz);
+}
+int imx8_ipc_pcm_params(struct snd_sof_dev *sdev,
struct snd_pcm_substream *substream,const struct sof_ipc_pcm_params_reply *reply)+{
- return 0;
+}
+static struct snd_soc_dai_driver imx8_dai[] = { +{
- .name = "esai-port",
+}, +};
+/* i.MX8 ops */ +struct snd_sof_dsp_ops sof_imx8_ops = {
- /* device init */
- .probe = imx8_probe,
- /* DSP core boot */
- .run = imx8_run,
- /* Block IO */
- .block_read = sof_block_read,
- .block_write = sof_block_write,
- /* ipc */
- .send_msg = imx8_send_msg,
- .fw_ready = imx8_fw_ready,
- .ipc_msg_data = imx8_ipc_msg_data,
- .ipc_pcm_params = imx8_ipc_pcm_params,
- /* module loading */
- .load_module = snd_sof_parse_module_memcpy,
- .get_bar_index = imx8_get_bar_index,
- /* firmware loading */
- .load_firmware = snd_sof_load_firmware_memcpy,
- /* DAI drivers */
- .drv = imx8_dai,
- .num_drv = 1, /* we have only 1 ESAI interface on i.MX8 */
+}; +EXPORT_SYMBOL(sof_imx8_ops);
+MODULE_LICENSE("Dual BSD/GPL");
diff --git a/sound/soc/sof/Kconfig b/sound/soc/sof/Kconfig index 61b97fc55bb2..2aa3a1cdf60c 100644 --- a/sound/soc/sof/Kconfig +++ b/sound/soc/sof/Kconfig @@ -36,6 +36,15 @@ config SND_SOC_SOF_ACPI Say Y if you need this option If unsure select "N".
+config SND_SOC_SOF_DT
- tristate "SOF DT enumeration support"
- select SND_SOC_SOF
- select SND_SOC_SOF_OPTIONS
- help
This adds support for Device Tree enumeration. This option isrequired to enable i.MX8 devices.Say Y if you need this option. If unsure select "N".
[snip]
diff --git a/sound/soc/sof/imx/Kconfig b/sound/soc/sof/imx/Kconfig index fff64a9970f0..fa35994a79c4 100644 --- a/sound/soc/sof/imx/Kconfig +++ b/sound/soc/sof/imx/Kconfig @@ -12,6 +12,7 @@ if SND_SOC_SOF_IMX_TOPLEVEL
config SND_SOC_SOF_IMX8 tristate "SOF support for i.MX8"
- select SND_SOC_SOF_DT
This looks upside down. You should select SOF_DT first then include the NXP stuff.
help This adds support for Sound Open Firmware for NXP i.MX8 platforms Say Y if you have such a device. diff --git a/sound/soc/sof/sof-dt-dev.c b/sound/soc/sof/sof-dt-dev.c new file mode 100644 index 000000000000..31429bbb5c7e --- /dev/null +++ b/sound/soc/sof/sof-dt-dev.c @@ -0,0 +1,159 @@ +// SPDX-License-Identifier: (GPL-2.0 OR BSD-3-Clause) +// +// Copyright 2019 NXP +// +// Author: Daniel Baluta daniel.baluta@nxp.com +//
+#include <linux/firmware.h> +#include <linux/module.h> +#include <linux/pm_runtime.h> +#include <sound/sof.h>
+#include "ops.h"
+extern struct snd_sof_dsp_ops sof_imx8_ops;
+static char *fw_path; +module_param(fw_path, charp, 0444); +MODULE_PARM_DESC(fw_path, "alternate path for SOF firmware.");
+static char *tplg_path; +module_param(tplg_path, charp, 0444); +MODULE_PARM_DESC(tplg_path, "alternate path for SOF topology.");
+/* platform specific devices */ +#if IS_ENABLED(CONFIG_SND_SOC_SOF_IMX8) +static struct sof_dev_desc sof_dt_imx8qxp_desc = {
- .default_fw_path = "imx/sof",
- .default_tplg_path = "imx/sof-tplg",
- .nocodec_fw_filename = "sof-imx8.ri",
- .nocodec_tplg_filename = "sof-imx8-nocodec.tplg",
- .ops = &sof_imx8_ops,
+}; +#endif
+static const struct dev_pm_ops sof_dt_pm = {
- SET_SYSTEM_SLEEP_PM_OPS(snd_sof_suspend, snd_sof_resume)
- SET_RUNTIME_PM_OPS(snd_sof_runtime_suspend, snd_sof_runtime_resume,
NULL)+};
+static void sof_dt_probe_complete(struct device *dev) +{
- /* allow runtime_pm */
- pm_runtime_set_autosuspend_delay(dev, SND_SOF_SUSPEND_DELAY_MS);
- pm_runtime_use_autosuspend(dev);
- pm_runtime_enable(dev);
+}
+static int sof_dt_probe(struct platform_device *pdev) +{
- struct device *dev = &pdev->dev;
- const struct sof_dev_desc *desc;
- /*TODO: create a generic snd_soc_xxx_mach */
- struct snd_soc_acpi_mach *mach;
I wonder if you really need to use the same structures. For Intel we get absolutely zero info from the firmware so rely on an ACPI codec ID as a key to find information on which firmware and topology to use, and which machine driver to load. You could have all this information in a DT blob?
- struct snd_sof_pdata *sof_pdata;
- const struct snd_sof_dsp_ops *ops;
- int ret;
- dev_info(&pdev->dev, "DT DSP detected");
- sof_pdata = devm_kzalloc(dev, sizeof(*sof_pdata), GFP_KERNEL);
- if (!sof_pdata)
return -ENOMEM;- desc = device_get_match_data(dev);
- if (!desc)
return -ENODEV;- /* get ops for platform */
- ops = desc->ops;
- if (!ops) {
dev_err(dev, "error: no matching DT descriptor ops\n");return -ENODEV;- }
+#if IS_ENABLED(CONFIG_SND_SOC_SOF_FORCE_NOCODEC_MODE)
- /* force nocodec mode */
- dev_warn(dev, "Force to use nocodec mode\n");
- mach = devm_kzalloc(dev, sizeof(*mach), GFP_KERNEL);
- if (!mach)
return -ENOMEM;- ret = sof_nocodec_setup(dev, sof_pdata, mach, desc, ops);
- if (ret < 0)
return ret;+#else
- /* TODO: implement case where we actually have a codec */
- return -ENODEV;
+#endif
- if (mach)
mach->mach_params.platform = dev_name(dev);- sof_pdata->machine = mach;
- sof_pdata->desc = desc;
- sof_pdata->dev = &pdev->dev;
- sof_pdata->platform = dev_name(dev);
- /* alternate fw and tplg filenames */
- if (fw_path)
sof_pdata->fw_filename_prefix = fw_path;- else
sof_pdata->fw_filename_prefix =sof_pdata->desc->default_fw_path;- if (tplg_path)
sof_pdata->tplg_filename_prefix = tplg_path;- else
sof_pdata->tplg_filename_prefix =sof_pdata->desc->default_tplg_path;+#if IS_ENABLED(CONFIG_SND_SOC_SOF_PROBE_WORK_QUEUE)
- /* set callback to enable runtime_pm */
- sof_pdata->sof_probe_complete = sof_dt_probe_complete;
+#endif
/* call sof helper for DSP hardware probe */- ret = snd_sof_device_probe(dev, sof_pdata);
- if (ret) {
dev_err(dev, "error: failed to probe DSP hardware\n");return ret;- }
+#if !IS_ENABLED(CONFIG_SND_SOC_SOF_PROBE_WORK_QUEUE)
- sof_dt_probe_complete(dev);
+#endif
- return ret;
+}
+static int sof_dt_remove(struct platform_device *pdev) +{
- pm_runtime_disable(&pdev->dev);
- /* call sof helper for DSP hardware remove */
- snd_sof_device_remove(&pdev->dev);
- return 0;
+}
+static const struct of_device_id sof_dt_ids[] = { +#if IS_ENABLED(CONFIG_SND_SOC_SOF_IMX8)
- { .compatible = "fsl,imx8qxp-dsp", .data = &sof_dt_imx8qxp_desc},
+#endif
- { }
+}; +MODULE_DEVICE_TABLE(of, sof_dt_ids);
+/* DT driver definition */ +static struct platform_driver snd_sof_dt_driver = {
- .probe = sof_dt_probe,
- .remove = sof_dt_remove,
- .driver = {
.name = "sof-audio-dt",.pm = &sof_dt_pm,.of_match_table = sof_dt_ids- },
+}; +module_platform_driver(snd_sof_dt_driver);
+MODULE_LICENSE("Dual BSD/GPL");
On Tue, Jul 23, 2019 at 2:41 AM Daniel Baluta daniel.baluta@nxp.com wrote:
This describes the DSP device tree node.
Signed-off-by: Daniel Baluta daniel.baluta@nxp.com
.../devicetree/bindings/dsp/fsl,dsp.yaml | 87 +++++++++++++++++++ 1 file changed, 87 insertions(+) create mode 100644 Documentation/devicetree/bindings/dsp/fsl,dsp.yaml
diff --git a/Documentation/devicetree/bindings/dsp/fsl,dsp.yaml b/Documentation/devicetree/bindings/dsp/fsl,dsp.yaml new file mode 100644 index 000000000000..d112486eda0e --- /dev/null +++ b/Documentation/devicetree/bindings/dsp/fsl,dsp.yaml @@ -0,0 +1,87 @@ +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause) +%YAML 1.2 +--- +$id: http://devicetree.org/schemas/arm/freescale/fsl,dsp.yaml#
This needs updating to match the path.
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+title: NXP i.MX8 DSP core
+maintainers:
- Daniel Baluta daniel.baluta@nxp.com
+description: |
- Some boards from i.MX8 family contain a DSP core used for
- advanced pre- and post- audio processing.
+properties:
- compatible:
- enum:
- fsl,imx8qxp-dsp- reg:
- description: Should contain register location and length
- clocks:
- items:
- description: ipg clock- description: ocram clock- description: core clock- clock-names:
- items:
- const: ipg- const: ocram- const: core- power-domains:
- description:
List of phandle and PM domain specifier as documented inDocumentation/devicetree/bindings/power/power_domain.txt- maxItems: 4
Need a blank line here.
With those 2 fixes:
Reviewed-by: Rob Herring robh@kernel.org
- mboxes:
- description:
List of <&phandle type channel> - 2 channels for TXDB, 2 channels for RXDB(see mailbox/fsl,mu.txt)- maxItems: 4
- mbox-names:
- items:
- const: txdb0- const: txdb1- const: rxdb0- const: rxdb1- memory-region:
- description:
phandle to a node describing reserved memory (System RAM memory)used by DSP (see bindings/reserved-memory/reserved-memory.txt)- maxItems: 1
+required:
- compatible
- reg
- clocks
- clock-names
- power-domains
- mboxes
- mbox-names
- memory-region
+examples:
- |
- #include <dt-bindings/firmware/imx/rsrc.h>
- #include <dt-bindings/clock/imx8-clock.h>
- dsp@596e8000 {
compatbile = "fsl,imx8qxp-dsp";reg = <0x596e8000 0x88000>;clocks = <&adma_lpcg IMX_ADMA_LPCG_DSP_IPG_CLK>,<&adma_lpcg IMX_ADMA_LPCG_OCRAM_IPG_CLK>,<&adma_lpcg IMX_ADMA_LPCG_DSP_CORE_CLK>;clock-names = "ipg", "ocram", "core";power-domains = <&pd IMX_SC_R_MU_13A>,<&pd IMX_SC_R_MU_13B>,<&pd IMX_SC_R_DSP>,<&pd IMX_SC_R_DSP_RAM>;mbox-names = "txdb0", "txdb1", "rxdb0", "rxdb1";mboxes = <&lsio_mu13 2 0>, <&lsio_mu13 2 1>, <&lsio_mu13 3 0>, <&lsio_mu13 3 1>;- };
-- 2.17.1
On Tue, Jul 23, 2019 at 6:18 PM Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com wrote:
diff --git a/sound/soc/sof/imx/Makefile b/sound/soc/sof/imx/Makefile new file mode 100644 index 000000000000..c69237971da5 --- /dev/null +++ b/sound/soc/sof/imx/Makefile @@ -0,0 +1,7 @@ +# SPDX-License-Identifier: (GPL-2.0 OR BSD-3-Clause)
+ccflags-y += -DDEBUG
this should be removed or in a separate patch.
All right.
+struct imx8_priv {
struct device *dev;struct snd_sof_dev *sdev;struct imx_dsp_ipc *dsp_ipc;struct imx_sc_ipc *sc_ipc;maybe a comment to explain what 'sc' stands for?
Sure.
+};
+static void imx8_get_windows(struct snd_sof_dev *sdev) +{
struct sof_ipc_window_elem *elem;u32 outbox_offset = 0;u32 stream_offset = 0;u32 inbox_offset = 0;u32 outbox_size = 0;u32 stream_size = 0;u32 inbox_size = 0;int i;if (!sdev->info_window) {dev_err(sdev->dev, "error: have no window info\n");return;}for (i = 0; i < sdev->info_window->num_windows; i++) {elem = &sdev->info_window->window[i];switch (elem->type) {case SOF_IPC_REGION_UPBOX:inbox_offset = elem->offset + MBOX_OFFSET;inbox_size = elem->size;snd_sof_debugfs_io_item(sdev,sdev->bar[SOF_FW_BLK_TYPE_SRAM]+ inbox_offset,elem->size, "inbox",SOF_DEBUGFS_ACCESS_D0_ONLY);break;case SOF_IPC_REGION_DOWNBOX:outbox_offset = elem->offset + MBOX_OFFSET;outbox_size = elem->size;snd_sof_debugfs_io_item(sdev,sdev->bar[SOF_FW_BLK_TYPE_SRAM]+ outbox_offset,elem->size, "outbox",SOF_DEBUGFS_ACCESS_D0_ONLY);break;case SOF_IPC_REGION_TRACE:snd_sof_debugfs_io_item(sdev,sdev->bar[SOF_FW_BLK_TYPE_SRAM]+ elem->offset + MBOX_OFFSET,elem->size, "etrace",SOF_DEBUGFS_ACCESS_D0_ONLY);break;case SOF_IPC_REGION_DEBUG:snd_sof_debugfs_io_item(sdev,sdev->bar[SOF_FW_BLK_TYPE_SRAM]+ elem->offset + MBOX_OFFSET,elem->size, "debug",SOF_DEBUGFS_ACCESS_D0_ONLY);break;case SOF_IPC_REGION_STREAM:stream_offset = elem->offset + MBOX_OFFSET;stream_size = elem->size;snd_sof_debugfs_io_item(sdev,sdev->bar[SOF_FW_BLK_TYPE_SRAM]+ stream_offset,elem->size, "stream",SOF_DEBUGFS_ACCESS_D0_ONLY);break;case SOF_IPC_REGION_REGS:snd_sof_debugfs_io_item(sdev,sdev->bar[SOF_FW_BLK_TYPE_SRAM]+ elem->offset + MBOX_OFFSET,elem->size, "regs",SOF_DEBUGFS_ACCESS_D0_ONLY);break;case SOF_IPC_REGION_EXCEPTION:sdev->dsp_oops_offset = elem->offset + MBOX_OFFSET;snd_sof_debugfs_io_item(sdev,sdev->bar[SOF_FW_BLK_TYPE_SRAM]+ elem->offset + MBOX_OFFSET,elem->size, "exception",SOF_DEBUGFS_ACCESS_D0_ONLY);break;default:dev_err(sdev->dev, "error: get illegal window info\n");return;}}if (outbox_size == 0 || inbox_size == 0) {dev_err(sdev->dev, "error: get illegal mailbox window\n");return;}snd_sof_dsp_mailbox_init(sdev, inbox_offset, inbox_size,outbox_offset, outbox_size);sdev->stream_box.offset = stream_offset;sdev->stream_box.size = stream_size;dev_dbg(sdev->dev, " mailbox upstream 0x%x - size 0x%x\n",inbox_offset, inbox_size);dev_dbg(sdev->dev, " mailbox downstream 0x%x - size 0x%x\n",outbox_offset, outbox_size);dev_dbg(sdev->dev, " stream region 0x%x - size 0x%x\n",stream_offset, stream_size);+}
This looks 100% similar to Baytrail?
Yes!
+/*
- IPC Firmware ready.
- */
+static int imx8_fw_ready(struct snd_sof_dev *sdev, u32 msg_id) +{
struct sof_ipc_fw_ready *fw_ready = &sdev->fw_ready;u32 offset;int ret;/* mailbox must be on 4k boundary */offset = MBOX_OFFSET;dev_dbg(sdev->dev, "ipc: DSP is ready 0x%8.8x offset 0x%x\n",msg_id, offset);/* no need to re-check version/ABI for subsequent boots */if (!sdev->first_boot)return 0;/* copy data from the DSP FW ready offset */sof_block_read(sdev, sdev->mailbox_bar, offset, fw_ready,sizeof(*fw_ready));snd_sof_dsp_mailbox_init(sdev, fw_ready->dspbox_offset,fw_ready->dspbox_size,fw_ready->hostbox_offset,fw_ready->hostbox_size);/* make sure ABI version is compatible */ret = snd_sof_ipc_valid(sdev);if (ret < 0)return ret;/* now check for extended data */snd_sof_fw_parse_ext_data(sdev, SOF_FW_BLK_TYPE_SRAM, MBOX_OFFSET +sizeof(struct sof_ipc_fw_ready));imx8_get_windows(sdev);return 0;+}
That code looks nearly similar to the baytrail one except for the last line, we should look into factoring this.
Yes, I got most of my inspiration from intel code.
+static void imx8_get_reply(struct snd_sof_dev *sdev) +{
struct snd_sof_ipc_msg *msg = sdev->msg;struct sof_ipc_reply reply;unsigned long flags;int ret = 0;if (!msg) {dev_warn(sdev->dev, "unexpected ipc interrupt\n");return;}/* get reply */sof_mailbox_read(sdev, sdev->host_box.offset, &reply, sizeof(reply));spin_lock_irqsave(&sdev->ipc_lock, flags);if (reply.error < 0) {memcpy(msg->reply_data, &reply, sizeof(reply));ret = reply.error;} else {/* reply has correct size? */if (reply.hdr.size != msg->reply_size) {dev_err(sdev->dev, "error: reply expected %zu got %u bytes\n",msg->reply_size, reply.hdr.size);ret = -EINVAL;}/* read the message */if (msg->reply_size > 0)sof_mailbox_read(sdev, sdev->host_box.offset,msg->reply_data, msg->reply_size);}msg->reply_error = ret;spin_unlock_irqrestore(&sdev->ipc_lock, flags);I don't see a spin_lock/unlock for the get_reply in the Intel code, is this necessary?
Hmm, you are right. I think I've used an older version of the intel code where there a lock.
+}
+void imx_dsp_handle_reply(struct imx_dsp_ipc *ipc) +{
struct imx8_priv *priv = imx_dsp_get_data(ipc);imx8_get_reply(priv->sdev);snd_sof_ipc_reply(priv->sdev, 0);+}
+void imx_dsp_handle_request(struct imx_dsp_ipc *ipc) +{
struct imx8_priv *priv = imx_dsp_get_data(ipc);snd_sof_ipc_msgs_rx(priv->sdev);+}
+struct imx_dsp_ops dsp_ops = {
.handle_reply = imx_dsp_handle_reply,.handle_request = imx_dsp_handle_request,+};
+static int imx8_send_msg(struct snd_sof_dev *sdev, struct snd_sof_ipc_msg *msg) +{
struct imx8_priv *priv = (struct imx8_priv *)sdev->private;sof_mailbox_write(sdev, sdev->host_box.offset, msg->msg_data,msg->msg_size);imx_dsp_ring_doorbell(priv->dsp_ipc, 0);return 0;+}
+/*
- DSP control.
- */
+static int imx8_run(struct snd_sof_dev *sdev) +{
int ret;struct imx8_priv *dsp_priv = (struct imx8_priv *)sdev->private;ret = imx_sc_misc_set_control(dsp_priv->sc_ipc, IMX_SC_R_DSP,IMX_SC_C_OFS_SEL, 1);if (ret < 0) {dev_err(sdev->dev, "Error system address offset source select\n");return ret;}ret = imx_sc_misc_set_control(dsp_priv->sc_ipc, IMX_SC_R_DSP,IMX_SC_C_OFS_AUDIO, 0x80);if (ret < 0) {dev_err(sdev->dev, "Error system address offset of AUDIO\n");return ret;}ret = imx_sc_misc_set_control(dsp_priv->sc_ipc, IMX_SC_R_DSP,IMX_SC_C_OFS_PERIPH, 0x5A);if (ret < 0) {dev_err(sdev->dev, "Error system address offset of PERIPH %d\n",ret);return ret;}ret = imx_sc_misc_set_control(dsp_priv->sc_ipc, IMX_SC_R_DSP,IMX_SC_C_OFS_IRQ, 0x51);if (ret < 0) {dev_err(sdev->dev, "Error system address offset of IRQ\n");return ret;}imx_sc_pm_cpu_start(dsp_priv->sc_ipc, IMX_SC_R_DSP, true,RESET_VECTOR_VADDR);return 0;+}
+static int imx8_probe(struct snd_sof_dev *sdev) +{
struct imx8_priv *priv;int i;struct platform_device *pdev =container_of(sdev->dev, struct platform_device, dev);struct platform_device *ipc_dev;struct resource *mmio;int num_domains = 0;u32 base, size;int ret = 0;struct device_node *np = pdev->dev.of_node;struct device_node *res_node;struct resource res;nit-pick: can we reorder so that we have all counters last and a nice xmas-tree shape.
Ack.
priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL);if (!priv)return -ENOMEM;sdev->private = priv;priv->dev = sdev->dev;priv->sdev = sdev;ret = imx_scu_get_handle(&priv->sc_ipc);if (ret) {dev_err(sdev->dev, "Cannot obtain SCU handle (err = %d)\n",ret);return ret;}ipc_dev = platform_device_register_data(sdev->dev, "imx-dsp",PLATFORM_DEVID_NONE,pdev, sizeof(*pdev));if (IS_ERR(ipc_dev)) {dev_err(sdev->dev, "Failed to register platform device\n");return PTR_ERR(ipc_dev);}priv->dsp_ipc = dev_get_drvdata(&ipc_dev->dev);if (!priv->dsp_ipc)return -EPROBE_DEFER;imx_dsp_set_data(priv->dsp_ipc, priv);priv->dsp_ipc->ops = &dsp_ops;num_domains = of_count_phandle_with_args(np, "power-domains","#power-domain-cells");for (i = 0; i < num_domains; i++) {struct device *pd_dev;struct device_link *link;pd_dev = dev_pm_domain_attach_by_id(&pdev->dev, i);if (IS_ERR(pd_dev))return PTR_ERR(pd_dev);link = device_link_add(&pdev->dev, pd_dev,DL_FLAG_STATELESS |DL_FLAG_PM_RUNTIME |DL_FLAG_RPM_ACTIVE);if (IS_ERR(link))return PTR_ERR(link);Question: is the error flow final? Wondering if we release all the resources/memory/devices on errors?
Will check again. It seemed no need for resource freeing.
Also are all the resources device-managed, I don't see a remove()?
Good catch for pm stuff. We mostly didn't care about remove because drivers are always Y in our distribution.
Thanks Pierre for review, I will let some time for others to have a look and send a new version.
thanks, Daniel.
On Tue, Jul 23, 2019 at 6:19 PM Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com wrote:
diff --git a/sound/soc/sof/Kconfig b/sound/soc/sof/Kconfig index 61b97fc55bb2..2aa3a1cdf60c 100644 --- a/sound/soc/sof/Kconfig +++ b/sound/soc/sof/Kconfig @@ -36,6 +36,15 @@ config SND_SOC_SOF_ACPI Say Y if you need this option If unsure select "N".
+config SND_SOC_SOF_DT
tristate "SOF DT enumeration support"select SND_SOC_SOFselect SND_SOC_SOF_OPTIONShelpThis adds support for Device Tree enumeration. This option isrequired to enable i.MX8 devices.Say Y if you need this option. If unsure select "N".[snip]
diff --git a/sound/soc/sof/imx/Kconfig b/sound/soc/sof/imx/Kconfig index fff64a9970f0..fa35994a79c4 100644 --- a/sound/soc/sof/imx/Kconfig +++ b/sound/soc/sof/imx/Kconfig @@ -12,6 +12,7 @@ if SND_SOC_SOF_IMX_TOPLEVEL
config SND_SOC_SOF_IMX8 tristate "SOF support for i.MX8"
select SND_SOC_SOF_DTThis looks upside down. You should select SOF_DT first then include the NXP stuff.
help This adds support for Sound Open Firmware for NXP i.MX8 platforms Say Y if you have such a device.diff --git a/sound/soc/sof/sof-dt-dev.c b/sound/soc/sof/sof-dt-dev.c new file mode 100644 index 000000000000..31429bbb5c7e --- /dev/null +++ b/sound/soc/sof/sof-dt-dev.c @@ -0,0 +1,159 @@ +// SPDX-License-Identifier: (GPL-2.0 OR BSD-3-Clause) +// +// Copyright 2019 NXP +// +// Author: Daniel Baluta daniel.baluta@nxp.com +//
+#include <linux/firmware.h> +#include <linux/module.h> +#include <linux/pm_runtime.h> +#include <sound/sof.h>
+#include "ops.h"
+extern struct snd_sof_dsp_ops sof_imx8_ops;
+static char *fw_path; +module_param(fw_path, charp, 0444); +MODULE_PARM_DESC(fw_path, "alternate path for SOF firmware.");
+static char *tplg_path; +module_param(tplg_path, charp, 0444); +MODULE_PARM_DESC(tplg_path, "alternate path for SOF topology.");
+/* platform specific devices */ +#if IS_ENABLED(CONFIG_SND_SOC_SOF_IMX8) +static struct sof_dev_desc sof_dt_imx8qxp_desc = {
.default_fw_path = "imx/sof",.default_tplg_path = "imx/sof-tplg",.nocodec_fw_filename = "sof-imx8.ri",.nocodec_tplg_filename = "sof-imx8-nocodec.tplg",.ops = &sof_imx8_ops,+}; +#endif
+static const struct dev_pm_ops sof_dt_pm = {
SET_SYSTEM_SLEEP_PM_OPS(snd_sof_suspend, snd_sof_resume)SET_RUNTIME_PM_OPS(snd_sof_runtime_suspend, snd_sof_runtime_resume,NULL)+};
+static void sof_dt_probe_complete(struct device *dev) +{
/* allow runtime_pm */pm_runtime_set_autosuspend_delay(dev, SND_SOF_SUSPEND_DELAY_MS);pm_runtime_use_autosuspend(dev);pm_runtime_enable(dev);+}
+static int sof_dt_probe(struct platform_device *pdev) +{
struct device *dev = &pdev->dev;const struct sof_dev_desc *desc;/*TODO: create a generic snd_soc_xxx_mach */struct snd_soc_acpi_mach *mach;I wonder if you really need to use the same structures. For Intel we get absolutely zero info from the firmware so rely on an ACPI codec ID as a key to find information on which firmware and topology to use, and which machine driver to load. You could have all this information in a DT blob?
Yes. I see your point. I will still need to make a generic structure for snd_soc_acpi_mach so that everyone can use sof_nocodec_setup function.
Maybe something like this:
struct snd_soc_mach { union { struct snd_soc_acpi_mach acpi_mach; struct snd_soc_of_mach of_mach; } };
and then change the prototype of sof_nocodec_setup.
Hi Daniel,
Am Mittwoch, den 24.07.2019, 09:54 +0300 schrieb Daniel Baluta:
On Tue, Jul 23, 2019 at 6:18 PM Pierre-Louis Bossart
[...]
Also are all the resources device-managed, I don't see a remove()?
Good catch for pm stuff. We mostly didn't care about remove because drivers are always Y in our distribution.
Linux drivers need to be hotplug aware, even if they are not built as a module. You can test things by manually unbinding the driver from the device via sysfs.
Regards, Lucas
On Wed, Jul 24, 2019 at 11:32 AM Lucas Stach l.stach@pengutronix.de wrote:
Hi Daniel,
Am Mittwoch, den 24.07.2019, 09:54 +0300 schrieb Daniel Baluta:
On Tue, Jul 23, 2019 at 6:18 PM Pierre-Louis Bossart
[...]
Also are all the resources device-managed, I don't see a remove()?
Good catch for pm stuff. We mostly didn't care about remove because drivers are always Y in our distribution.
Linux drivers need to be hotplug aware, even if they are not built as a module. You can test things by manually unbinding the driver from the device via sysfs.
Agree. Will take this into consideration when sending next version!
On Wed, Jul 24, 2019 at 10:04 AM Daniel Baluta daniel.baluta@gmail.com wrote:
On Tue, Jul 23, 2019 at 6:19 PM Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com wrote:
diff --git a/sound/soc/sof/Kconfig b/sound/soc/sof/Kconfig index 61b97fc55bb2..2aa3a1cdf60c 100644 --- a/sound/soc/sof/Kconfig +++ b/sound/soc/sof/Kconfig @@ -36,6 +36,15 @@ config SND_SOC_SOF_ACPI Say Y if you need this option If unsure select "N".
+config SND_SOC_SOF_DT
tristate "SOF DT enumeration support"select SND_SOC_SOFselect SND_SOC_SOF_OPTIONShelpThis adds support for Device Tree enumeration. This option isrequired to enable i.MX8 devices.Say Y if you need this option. If unsure select "N".[snip]
diff --git a/sound/soc/sof/imx/Kconfig b/sound/soc/sof/imx/Kconfig index fff64a9970f0..fa35994a79c4 100644 --- a/sound/soc/sof/imx/Kconfig +++ b/sound/soc/sof/imx/Kconfig @@ -12,6 +12,7 @@ if SND_SOC_SOF_IMX_TOPLEVEL
config SND_SOC_SOF_IMX8 tristate "SOF support for i.MX8"
select SND_SOC_SOF_DTThis looks upside down. You should select SOF_DT first then include the NXP stuff.
help This adds support for Sound Open Firmware for NXP i.MX8 platforms Say Y if you have such a device.diff --git a/sound/soc/sof/sof-dt-dev.c b/sound/soc/sof/sof-dt-dev.c new file mode 100644 index 000000000000..31429bbb5c7e --- /dev/null +++ b/sound/soc/sof/sof-dt-dev.c @@ -0,0 +1,159 @@ +// SPDX-License-Identifier: (GPL-2.0 OR BSD-3-Clause) +// +// Copyright 2019 NXP +// +// Author: Daniel Baluta daniel.baluta@nxp.com +//
+#include <linux/firmware.h> +#include <linux/module.h> +#include <linux/pm_runtime.h> +#include <sound/sof.h>
+#include "ops.h"
+extern struct snd_sof_dsp_ops sof_imx8_ops;
+static char *fw_path; +module_param(fw_path, charp, 0444); +MODULE_PARM_DESC(fw_path, "alternate path for SOF firmware.");
+static char *tplg_path; +module_param(tplg_path, charp, 0444); +MODULE_PARM_DESC(tplg_path, "alternate path for SOF topology.");
+/* platform specific devices */ +#if IS_ENABLED(CONFIG_SND_SOC_SOF_IMX8) +static struct sof_dev_desc sof_dt_imx8qxp_desc = {
.default_fw_path = "imx/sof",.default_tplg_path = "imx/sof-tplg",.nocodec_fw_filename = "sof-imx8.ri",.nocodec_tplg_filename = "sof-imx8-nocodec.tplg",.ops = &sof_imx8_ops,+}; +#endif
+static const struct dev_pm_ops sof_dt_pm = {
SET_SYSTEM_SLEEP_PM_OPS(snd_sof_suspend, snd_sof_resume)SET_RUNTIME_PM_OPS(snd_sof_runtime_suspend, snd_sof_runtime_resume,NULL)+};
+static void sof_dt_probe_complete(struct device *dev) +{
/* allow runtime_pm */pm_runtime_set_autosuspend_delay(dev, SND_SOF_SUSPEND_DELAY_MS);pm_runtime_use_autosuspend(dev);pm_runtime_enable(dev);+}
+static int sof_dt_probe(struct platform_device *pdev) +{
struct device *dev = &pdev->dev;const struct sof_dev_desc *desc;/*TODO: create a generic snd_soc_xxx_mach */struct snd_soc_acpi_mach *mach;I wonder if you really need to use the same structures. For Intel we get absolutely zero info from the firmware so rely on an ACPI codec ID as a key to find information on which firmware and topology to use, and which machine driver to load. You could have all this information in a DT blob?
Yes. I see your point. I will still need to make a generic structure for snd_soc_acpi_mach so that everyone can use sof_nocodec_setup function.
Maybe something like this:
struct snd_soc_mach { union { struct snd_soc_acpi_mach acpi_mach; struct snd_soc_of_mach of_mach; } };
and then change the prototype of sof_nocodec_setup.
Hi Pierre,
I fixed all the comments except the one above. Replacing snd_soc_acpi_mach with a generic snd_soc_mach is not trivial task.
I wonder if it is acceptable to get the initial patches as they are now and later switch to generic ACPI/OF abstraction.
Asking this because for the moment on the i.MX side I have only implemented no codec version and we don't probe any of the machine drivers we have.
So, there is this only one member of snd_soc_acpi_mach that imx version is making use of:
mach->drv_name = "sof-nocodec";
That's all.
I think the change as it is now is very clean and non-intrusive. Later we will get options to read firmware name and stuff from DT.
Anyhow, I don't think we can get rid of snd_dev_desc structure on i.MX. This will be used to store the information read from DT:
static struct sof_dev_desc sof_of_imx8qxp_desc = { » .default_fw_path = "imx/sof", » .default_tplg_path = "imx/sof-tplg", » .nocodec_fw_filename = "sof-imx8.ri", » .nocodec_tplg_filename = "sof-imx8-nocodec.tplg", » .ops = &sof_imx8_ops, };
For the moment we will only use the default values.
thanks, Daniel.
+static int sof_dt_probe(struct platform_device *pdev) +{
struct device *dev = &pdev->dev;const struct sof_dev_desc *desc;/*TODO: create a generic snd_soc_xxx_mach */struct snd_soc_acpi_mach *mach;I wonder if you really need to use the same structures. For Intel we get absolutely zero info from the firmware so rely on an ACPI codec ID as a key to find information on which firmware and topology to use, and which machine driver to load. You could have all this information in a DT blob?
Yes. I see your point. I will still need to make a generic structure for snd_soc_acpi_mach so that everyone can use sof_nocodec_setup function.
Maybe something like this:
struct snd_soc_mach { union { struct snd_soc_acpi_mach acpi_mach; struct snd_soc_of_mach of_mach; } };
and then change the prototype of sof_nocodec_setup.
Hi Pierre,
I fixed all the comments except the one above. Replacing snd_soc_acpi_mach with a generic snd_soc_mach is not trivial task.
I wonder if it is acceptable to get the initial patches as they are now and later switch to generic ACPI/OF abstraction.
Asking this because for the moment on the i.MX side I have only implemented no codec version and we don't probe any of the machine drivers we have.
So, there is this only one member of snd_soc_acpi_mach that imx version is making use of:
mach->drv_name = "sof-nocodec";
That's all.
I think the change as it is now is very clean and non-intrusive. Later we will get options to read firmware name and stuff from DT.
Anyhow, I don't think we can get rid of snd_dev_desc structure on i.MX. This will be used to store the information read from DT:
static struct sof_dev_desc sof_of_imx8qxp_desc = { » .default_fw_path = "imx/sof", » .default_tplg_path = "imx/sof-tplg", » .nocodec_fw_filename = "sof-imx8.ri", » .nocodec_tplg_filename = "sof-imx8-nocodec.tplg", » .ops = &sof_imx8_ops, };
For the moment we will only use the default values.
Yes, that's fine for now. If you don't have a real machine driver then there's nothing urgent to change.
Is the new version on GitHub?
Thanks -Pierre
On Wed, Aug 7, 2019 at 6:22 PM Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com wrote:
+static int sof_dt_probe(struct platform_device *pdev) +{
struct device *dev = &pdev->dev;const struct sof_dev_desc *desc;/*TODO: create a generic snd_soc_xxx_mach */struct snd_soc_acpi_mach *mach;I wonder if you really need to use the same structures. For Intel we get absolutely zero info from the firmware so rely on an ACPI codec ID as a key to find information on which firmware and topology to use, and which machine driver to load. You could have all this information in a DT blob?
Yes. I see your point. I will still need to make a generic structure for snd_soc_acpi_mach so that everyone can use sof_nocodec_setup function.
Maybe something like this:
struct snd_soc_mach { union { struct snd_soc_acpi_mach acpi_mach; struct snd_soc_of_mach of_mach; } };
and then change the prototype of sof_nocodec_setup.
Hi Pierre,
I fixed all the comments except the one above. Replacing snd_soc_acpi_mach with a generic snd_soc_mach is not trivial task.
I wonder if it is acceptable to get the initial patches as they are now and later switch to generic ACPI/OF abstraction.
Asking this because for the moment on the i.MX side I have only implemented no codec version and we don't probe any of the machine drivers we have.
So, there is this only one member of snd_soc_acpi_mach that imx version is making use of:
mach->drv_name = "sof-nocodec";
That's all.
I think the change as it is now is very clean and non-intrusive. Later we will get options to read firmware name and stuff from DT.
Anyhow, I don't think we can get rid of snd_dev_desc structure on i.MX. This will be used to store the information read from DT:
static struct sof_dev_desc sof_of_imx8qxp_desc = { » .default_fw_path = "imx/sof", » .default_tplg_path = "imx/sof-tplg", » .nocodec_fw_filename = "sof-imx8.ri", » .nocodec_tplg_filename = "sof-imx8-nocodec.tplg", » .ops = &sof_imx8_ops, };
For the moment we will only use the default values.
Yes, that's fine for now. If you don't have a real machine driver then there's nothing urgent to change.
Is the new version on GitHub?
Not yet, will push it today and ping you.
On Tue, Jul 23, 2019 at 6:19 PM Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com wrote:
diff --git a/sound/soc/sof/Kconfig b/sound/soc/sof/Kconfig index 61b97fc55bb2..2aa3a1cdf60c 100644 --- a/sound/soc/sof/Kconfig +++ b/sound/soc/sof/Kconfig @@ -36,6 +36,15 @@ config SND_SOC_SOF_ACPI Say Y if you need this option If unsure select "N".
+config SND_SOC_SOF_DT
tristate "SOF DT enumeration support"select SND_SOC_SOFselect SND_SOC_SOF_OPTIONShelpThis adds support for Device Tree enumeration. This option isrequired to enable i.MX8 devices.Say Y if you need this option. If unsure select "N".[snip]
diff --git a/sound/soc/sof/imx/Kconfig b/sound/soc/sof/imx/Kconfig index fff64a9970f0..fa35994a79c4 100644 --- a/sound/soc/sof/imx/Kconfig +++ b/sound/soc/sof/imx/Kconfig @@ -12,6 +12,7 @@ if SND_SOC_SOF_IMX_TOPLEVEL
config SND_SOC_SOF_IMX8 tristate "SOF support for i.MX8"
select SND_SOC_SOF_DTThis looks upside down. You should select SOF_DT first then include the NXP stuff.
One more thing: So this should be 'depends on SND_SOC_SOF_DT' right?
On 8/7/19 10:29 AM, Daniel Baluta wrote:
On Tue, Jul 23, 2019 at 6:19 PM Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com wrote:
diff --git a/sound/soc/sof/Kconfig b/sound/soc/sof/Kconfig index 61b97fc55bb2..2aa3a1cdf60c 100644 --- a/sound/soc/sof/Kconfig +++ b/sound/soc/sof/Kconfig @@ -36,6 +36,15 @@ config SND_SOC_SOF_ACPI Say Y if you need this option If unsure select "N".
+config SND_SOC_SOF_DT
tristate "SOF DT enumeration support"select SND_SOC_SOFselect SND_SOC_SOF_OPTIONShelpThis adds support for Device Tree enumeration. This option isrequired to enable i.MX8 devices.Say Y if you need this option. If unsure select "N".[snip]
diff --git a/sound/soc/sof/imx/Kconfig b/sound/soc/sof/imx/Kconfig index fff64a9970f0..fa35994a79c4 100644 --- a/sound/soc/sof/imx/Kconfig +++ b/sound/soc/sof/imx/Kconfig @@ -12,6 +12,7 @@ if SND_SOC_SOF_IMX_TOPLEVEL
config SND_SOC_SOF_IMX8 tristate "SOF support for i.MX8"
select SND_SOC_SOF_DTThis looks upside down. You should select SOF_DT first then include the NXP stuff.
One more thing: So this should be 'depends on SND_SOC_SOF_DT' right?
I would do this:
config SND_SOC_SOF_DT tristate "SOF DT enumeration support" depends on OF # or whatever the top-level DT dependency is select SND_SOC_SOF select SND_SOC_SOF_OPTIONS
config SND_SOC_SOF_IMX_TOPLEVEL bool "SOF support for NXP i.MX audio DSPs" depends on ARM64 && SND_SOC_SOF_DT || COMPILE_TEST if SND_SOC_SOF_IMX_TOPLEVEL
config SND_SOC_SOF_IMX8 tristate "SOF support for i.MX8"
In other words push the dependencies at a higher level.
On Wed, Aug 7, 2019 at 6:28 PM Daniel Baluta daniel.baluta@gmail.com wrote:
On Wed, Aug 7, 2019 at 6:22 PM Pierre-Louis Bossart pierre-louis.bossart@linux.intel.com wrote:
+static int sof_dt_probe(struct platform_device *pdev) +{
struct device *dev = &pdev->dev;const struct sof_dev_desc *desc;/*TODO: create a generic snd_soc_xxx_mach */struct snd_soc_acpi_mach *mach;I wonder if you really need to use the same structures. For Intel we get absolutely zero info from the firmware so rely on an ACPI codec ID as a key to find information on which firmware and topology to use, and which machine driver to load. You could have all this information in a DT blob?
Yes. I see your point. I will still need to make a generic structure for snd_soc_acpi_mach so that everyone can use sof_nocodec_setup function.
Maybe something like this:
struct snd_soc_mach { union { struct snd_soc_acpi_mach acpi_mach; struct snd_soc_of_mach of_mach; } };
and then change the prototype of sof_nocodec_setup.
Hi Pierre,
I fixed all the comments except the one above. Replacing snd_soc_acpi_mach with a generic snd_soc_mach is not trivial task.
I wonder if it is acceptable to get the initial patches as they are now and later switch to generic ACPI/OF abstraction.
Asking this because for the moment on the i.MX side I have only implemented no codec version and we don't probe any of the machine drivers we have.
So, there is this only one member of snd_soc_acpi_mach that imx version is making use of:
mach->drv_name = "sof-nocodec";
That's all.
I think the change as it is now is very clean and non-intrusive. Later we will get options to read firmware name and stuff from DT.
Anyhow, I don't think we can get rid of snd_dev_desc structure on i.MX. This will be used to store the information read from DT:
static struct sof_dev_desc sof_of_imx8qxp_desc = { » .default_fw_path = "imx/sof", » .default_tplg_path = "imx/sof-tplg", » .nocodec_fw_filename = "sof-imx8.ri", » .nocodec_tplg_filename = "sof-imx8-nocodec.tplg", » .ops = &sof_imx8_ops, };
For the moment we will only use the default values.
Yes, that's fine for now. If you don't have a real machine driver then there's nothing urgent to change.
Is the new version on GitHub?
Not yet, will push it today and ping you.
PR updated: https://github.com/thesofproject/linux/pull/1048
participants (5)
-
Daniel Baluta -
Daniel Baluta -
Lucas Stach -
Pierre-Louis Bossart -
Rob Herring