July 2022 - Sound-open-firmware - mailman.alsa-project.org

[Sound-open-firmware] Hifi4 DSP
by Laurent Ovaert 27 Jul '22

27 Jul '22

Dear list, I wonder how much is the Hifi4 DSP supported in sof for the GCC (not XCC) toolchain ? I am a DSP engineer not part of an organization and i cannot get XCC from Cadence so i really count on GCC. I read in the FAQs that GCC has no Hifi SIMD support. Does it mean just no intrinsics support and of course no chance for auto-vectorization ? What about writing pure Hifi4 assembly code? When building the lx7hifi4-gcc10.2-gdb9 toolchain, can the assembler actually assemble Hifi4 code without restriction ? What about qemu support for Hifi4 ? I am surprised Cadence is not giving the compiler or at least the assrmbler for the Hifi4 :-/ Thanks, Laurent

3 3

[Sound-open-firmware] [PATCH v2 1/2] dt-bindings: dsp: fsl: Add SOF compatile string for i.MX8ULP
by Shengjiu Wang 20 Jul '22

20 Jul '22

Add SOF compatile string "fsl,imx8ulp-dsp" for supporting DSP device on i.MX8ULP platform. Signed-off-by: Shengjiu Wang <shengjiu.wang(a)nxp.com> Acked-by: Rob Herring <robh(a)kernel.org> --- changes in v2 - add acked-by Rob Documentation/devicetree/bindings/dsp/fsl,dsp.yaml | 1 + 1 file changed, 1 insertion(+) diff --git a/Documentation/devicetree/bindings/dsp/fsl,dsp.yaml b/Documentation/devicetree/bindings/dsp/fsl,dsp.yaml index e66ef2da7879..9af40da5688e 100644 --- a/Documentation/devicetree/bindings/dsp/fsl,dsp.yaml +++ b/Documentation/devicetree/bindings/dsp/fsl,dsp.yaml @@ -20,6 +20,7 @@ properties: - fsl,imx8qxp-dsp - fsl,imx8qm-dsp - fsl,imx8mp-dsp + - fsl,imx8ulp-dsp - fsl,imx8qxp-hifi4 - fsl,imx8qm-hifi4 - fsl,imx8mp-hifi4 -- 2.34.1

1 1

[Sound-open-firmware] [PATCH 1/2] ASoC: SOF: imx: Add i.MX8ULP HW support
by Shengjiu Wang 20 Jul '22

20 Jul '22

From: Zhang Peng <peng.zhang_8(a)nxp.com> This adds skeleton support for the audio DSP hardware found on NXP i.MX8ULP platform. On i.MX8ULP resources (clocks, power, etc) are managed by the System Integration Module in LPAV domain and XRDC which is handled by arm trusted firmware. Signed-off-by: Zhang Peng <peng.zhang_8(a)nxp.com> Signed-off-by: Shengjiu Wang <shengjiu.wang(a)nxp.com> --- sound/soc/sof/imx/Kconfig | 9 + sound/soc/sof/imx/Makefile | 2 + sound/soc/sof/imx/imx8ulp.c | 541 ++++++++++++++++++++++++++++++++++++ 3 files changed, 552 insertions(+) create mode 100644 sound/soc/sof/imx/imx8ulp.c diff --git a/sound/soc/sof/imx/Kconfig b/sound/soc/sof/imx/Kconfig index cc6e695f913a..4751b04d5e6f 100644 --- a/sound/soc/sof/imx/Kconfig +++ b/sound/soc/sof/imx/Kconfig @@ -41,4 +41,13 @@ config SND_SOC_SOF_IMX8M Say Y if you have such a device. If unsure select "N". +config SND_SOC_SOF_IMX8ULP + tristate "SOF support for i.MX8ULP" + depends on IMX_DSP + select SND_SOC_SOF_IMX_COMMON + help + This adds support for Sound Open Firmware for NXP i.MX8ULP platforms. + Say Y if you have such a device. + If unsure select "N". + endif ## SND_SOC_SOF_IMX_TOPLEVEL diff --git a/sound/soc/sof/imx/Makefile b/sound/soc/sof/imx/Makefile index dba93c3466ec..798b43a415bf 100644 --- a/sound/soc/sof/imx/Makefile +++ b/sound/soc/sof/imx/Makefile @@ -1,9 +1,11 @@ # SPDX-License-Identifier: (GPL-2.0-only OR BSD-3-Clause) snd-sof-imx8-objs := imx8.o snd-sof-imx8m-objs := imx8m.o +snd-sof-imx8ulp-objs := imx8ulp.o snd-sof-imx-common-objs := imx-common.o obj-$(CONFIG_SND_SOC_SOF_IMX8) += snd-sof-imx8.o obj-$(CONFIG_SND_SOC_SOF_IMX8M) += snd-sof-imx8m.o +obj-$(CONFIG_SND_SOC_SOF_IMX8ULP) += snd-sof-imx8ulp.o obj-$(CONFIG_SND_SOC_SOF_IMX_COMMON) += imx-common.o diff --git a/sound/soc/sof/imx/imx8ulp.c b/sound/soc/sof/imx/imx8ulp.c new file mode 100644 index 000000000000..ecb8750de7ea --- /dev/null +++ b/sound/soc/sof/imx/imx8ulp.c @@ -0,0 +1,541 @@ +// SPDX-License-Identifier: (GPL-2.0-only OR BSD-3-Clause) +// +// Copyright 2021-2022 NXP +// +// Author: Peng Zhang <peng.zhang_8(a)nxp.com> +// +// Hardware interface for audio DSP on i.MX8ULP + +#include <linux/clk.h> +#include <linux/firmware.h> +#include <linux/firmware/imx/dsp.h> +#include <linux/firmware/imx/ipc.h> +#include <linux/firmware/imx/svc/misc.h> +#include <linux/mfd/syscon.h> +#include <linux/module.h> +#include <linux/of_address.h> +#include <linux/of_irq.h> +#include <linux/of_platform.h> +#include <linux/of_reserved_mem.h> + +#include <sound/sof.h> +#include <sound/sof/xtensa.h> + +#include "../ops.h" +#include "../sof-of-dev.h" +#include "imx-common.h" + +#define FSL_SIP_HIFI_XRDC 0xc200000e + +/* SIM Domain register */ +#define SYSCTRL0 0x8 +#define EXECUTE_BIT BIT(13) +#define RESET_BIT BIT(16) +#define HIFI4_CLK_BIT BIT(17) +#define PB_CLK_BIT BIT(18) +#define PLAT_CLK_BIT BIT(19) +#define DEBUG_LOGIC_BIT BIT(25) + +#define MBOX_OFFSET 0x800000 +#define MBOX_SIZE 0x1000 + +struct arm_smccc_res smc_resource; + +static struct clk_bulk_data imx8ulp_dsp_clks[] = { + { .id = "core" }, + { .id = "ipg" }, + { .id = "ocram" }, + { .id = "mu" }, +}; + +struct imx8ulp_priv { + struct device *dev; + struct snd_sof_dev *sdev; + + /* DSP IPC handler */ + struct imx_dsp_ipc *dsp_ipc; + struct platform_device *ipc_dev; + + struct regmap *regmap; + struct imx_clocks *clks; +}; + +static void imx8ulp_sim_lpav_start(struct imx8ulp_priv *priv) +{ + /* Controls the HiFi4 DSP Reset: 1 in reset, 0 out of reset */ + regmap_update_bits(priv->regmap, SYSCTRL0, RESET_BIT, 0); + /* Reset HiFi4 DSP Debug logic: 1 reset, 0 not set */ + regmap_update_bits(priv->regmap, SYSCTRL0, DEBUG_LOGIC_BIT, 0); + /* Stall HIFI4 DSP Execution: 1 stall, 0 not stall */ + regmap_update_bits(priv->regmap, SYSCTRL0, EXECUTE_BIT, 0); +} + +static void imx8ulp_get_reply(struct snd_sof_dev *sdev) +{ + struct snd_sof_ipc_msg *msg = sdev->msg; + struct sof_ipc_reply reply; + 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)); + + 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; +} + +static int imx8ulp_get_mailbox_offset(struct snd_sof_dev *sdev) +{ + return MBOX_OFFSET; +} + +static int imx8ulp_get_window_offset(struct snd_sof_dev *sdev, u32 id) +{ + return MBOX_OFFSET; +} + +static void imx8ulp_dsp_handle_reply(struct imx_dsp_ipc *ipc) +{ + struct imx8ulp_priv *priv = imx_dsp_get_data(ipc); + unsigned long flags; + + spin_lock_irqsave(&priv->sdev->ipc_lock, flags); + + imx8ulp_get_reply(priv->sdev); + snd_sof_ipc_reply(priv->sdev, 0); + spin_unlock_irqrestore(&priv->sdev->ipc_lock, flags); +} + +static void imx8ulp_dsp_handle_request(struct imx_dsp_ipc *ipc) +{ + struct imx8ulp_priv *priv = imx_dsp_get_data(ipc); + u32 p; /* panic code */ + + /* Read the message from the debug box. */ + sof_mailbox_read(priv->sdev, priv->sdev->debug_box.offset + 4, &p, sizeof(p)); + + /* Check to see if the message is a panic code (0x0dead***) */ + if ((p & SOF_IPC_PANIC_MAGIC_MASK) == SOF_IPC_PANIC_MAGIC) + snd_sof_dsp_panic(priv->sdev, p, true); + else + snd_sof_ipc_msgs_rx(priv->sdev); +} + +static struct imx_dsp_ops dsp_ops = { + .handle_reply = imx8ulp_dsp_handle_reply, + .handle_request = imx8ulp_dsp_handle_request, +}; + +static int imx8ulp_send_msg(struct snd_sof_dev *sdev, struct snd_sof_ipc_msg *msg) +{ + struct imx8ulp_priv *priv = sdev->pdata->hw_pdata; + + sof_mailbox_write(sdev, sdev->host_box.offset, msg->msg_data, + msg->msg_size); + imx_dsp_ring_doorbell(priv->dsp_ipc, 0); + + return 0; +} + +static int imx8ulp_run(struct snd_sof_dev *sdev) +{ + struct imx8ulp_priv *priv = sdev->pdata->hw_pdata; + + imx8ulp_sim_lpav_start(priv); + + return 0; +} + +static int imx8ulp_reset(struct snd_sof_dev *sdev) +{ + struct imx8ulp_priv *priv = sdev->pdata->hw_pdata; + + /* HiFi4 Platform Clock Enable: 1 enabled, 0 disabled */ + regmap_update_bits(priv->regmap, SYSCTRL0, PLAT_CLK_BIT, PLAT_CLK_BIT); + /* HiFi4 PBCLK clock enable: 1 enabled, 0 disabled */ + regmap_update_bits(priv->regmap, SYSCTRL0, PB_CLK_BIT, PB_CLK_BIT); + /* HiFi4 Clock Enable: 1 enabled, 0 disabled */ + regmap_update_bits(priv->regmap, SYSCTRL0, HIFI4_CLK_BIT, HIFI4_CLK_BIT); + + regmap_update_bits(priv->regmap, SYSCTRL0, RESET_BIT, RESET_BIT); + usleep_range(1, 2); + /* Stall HIFI4 DSP Execution: 1 stall, 0 not stall */ + regmap_update_bits(priv->regmap, SYSCTRL0, EXECUTE_BIT, EXECUTE_BIT); + usleep_range(1, 2); + + arm_smccc_smc(FSL_SIP_HIFI_XRDC, 0, 0, 0, 0, 0, 0, 0, &smc_resource); + + return 0; +} + +static int imx8ulp_probe(struct snd_sof_dev *sdev) +{ + struct platform_device *pdev = + container_of(sdev->dev, struct platform_device, dev); + struct device_node *np = pdev->dev.of_node; + struct device_node *res_node; + struct resource *mmio; + struct imx8ulp_priv *priv; + struct resource res; + u32 base, size; + int ret = 0; + + priv = devm_kzalloc(&pdev->dev, sizeof(*priv), GFP_KERNEL); + if (!priv) + return -ENOMEM; + + priv->clks = devm_kzalloc(&pdev->dev, sizeof(*priv->clks), GFP_KERNEL); + if (!priv->clks) + return -ENOMEM; + + sdev->num_cores = 1; + sdev->pdata->hw_pdata = priv; + priv->dev = sdev->dev; + priv->sdev = sdev; + + /* System integration module(SIM) control dsp configurtion */ + priv->regmap = syscon_regmap_lookup_by_phandle(np, "fsl,dsp-ctrl"); + if (IS_ERR(priv->regmap)) + return PTR_ERR(priv->regmap); + + priv->ipc_dev = platform_device_register_data(sdev->dev, "imx-dsp", + PLATFORM_DEVID_NONE, + pdev, sizeof(*pdev)); + if (IS_ERR(priv->ipc_dev)) + return PTR_ERR(priv->ipc_dev); + + priv->dsp_ipc = dev_get_drvdata(&priv->ipc_dev->dev); + if (!priv->dsp_ipc) { + /* DSP IPC driver not probed yet, try later */ + ret = -EPROBE_DEFER; + dev_err(sdev->dev, "Failed to get drvdata\n"); + goto exit_pdev_unregister; + } + + imx_dsp_set_data(priv->dsp_ipc, priv); + priv->dsp_ipc->ops = &dsp_ops; + + /* 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"); + ret = -EINVAL; + goto exit_pdev_unregister; + } + + 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); + ret = -ENODEV; + goto exit_pdev_unregister; + } + sdev->mmio_bar = SOF_FW_BLK_TYPE_IRAM; + + res_node = of_parse_phandle(np, "memory-reserved", 0); + if (!res_node) { + dev_err(&pdev->dev, "failed to get memory region node\n"); + ret = -ENODEV; + goto exit_pdev_unregister; + } + + ret = of_address_to_resource(res_node, 0, &res); + if (ret) { + dev_err(&pdev->dev, "failed to get reserved region address\n"); + goto exit_pdev_unregister; + } + + sdev->bar[SOF_FW_BLK_TYPE_SRAM] = devm_ioremap_wc(sdev->dev, res.start, + resource_size(&res)); + if (!sdev->bar[SOF_FW_BLK_TYPE_SRAM]) { + dev_err(sdev->dev, "failed to ioremap mem 0x%x size 0x%x\n", + base, size); + ret = -ENOMEM; + goto exit_pdev_unregister; + } + sdev->mailbox_bar = SOF_FW_BLK_TYPE_SRAM; + + /* set default mailbox offset for FW ready message */ + sdev->dsp_box.offset = MBOX_OFFSET; + + ret = of_reserved_mem_device_init(sdev->dev); + if (ret) { + dev_err(&pdev->dev, "failed to init reserved memory region %d\n", ret); + goto exit_pdev_unregister; + } + + priv->clks->dsp_clks = imx8ulp_dsp_clks; + priv->clks->num_dsp_clks = ARRAY_SIZE(imx8ulp_dsp_clks); + + ret = imx8_parse_clocks(sdev, priv->clks); + if (ret < 0) + goto exit_pdev_unregister; + + ret = imx8_enable_clocks(sdev, priv->clks); + if (ret < 0) + goto exit_pdev_unregister; + + return 0; + +exit_pdev_unregister: + platform_device_unregister(priv->ipc_dev); + + return ret; +} + +static int imx8ulp_remove(struct snd_sof_dev *sdev) +{ + struct imx8ulp_priv *priv = sdev->pdata->hw_pdata; + + platform_device_unregister(priv->ipc_dev); + + return 0; +} + +/* on i.MX8 there is 1 to 1 match between type and BAR idx */ +static int imx8ulp_get_bar_index(struct snd_sof_dev *sdev, u32 type) +{ + return type; +} + +static int imx8ulp_suspend(struct snd_sof_dev *sdev) +{ + int i; + struct imx8ulp_priv *priv = (struct imx8ulp_priv *)sdev->pdata->hw_pdata; + + regmap_update_bits(priv->regmap, SYSCTRL0, EXECUTE_BIT, EXECUTE_BIT); + + for (i = 0; i < DSP_MU_CHAN_NUM; i++) + imx_dsp_free_channel(priv->dsp_ipc, i); + + imx8_disable_clocks(sdev, priv->clks); + + return 0; +} + +static int imx8ulp_resume(struct snd_sof_dev *sdev) +{ + struct imx8ulp_priv *priv = (struct imx8ulp_priv *)sdev->pdata->hw_pdata; + int i; + + imx8_enable_clocks(sdev, priv->clks); + + for (i = 0; i < DSP_MU_CHAN_NUM; i++) + imx_dsp_request_channel(priv->dsp_ipc, i); + + return 0; +} + +static int imx8ulp_dsp_runtime_resume(struct snd_sof_dev *sdev) +{ + const struct sof_dsp_power_state target_dsp_state = { + .state = SOF_DSP_PM_D0, + .substate = 0, + }; + + imx8ulp_resume(sdev); + + return snd_sof_dsp_set_power_state(sdev, &target_dsp_state); +} + +static int imx8ulp_dsp_runtime_suspend(struct snd_sof_dev *sdev) +{ + const struct sof_dsp_power_state target_dsp_state = { + .state = SOF_DSP_PM_D3, + .substate = 0, + }; + + imx8ulp_suspend(sdev); + + return snd_sof_dsp_set_power_state(sdev, &target_dsp_state); +} + +static int imx8ulp_dsp_suspend(struct snd_sof_dev *sdev, unsigned int target_state) +{ + const struct sof_dsp_power_state target_dsp_state = { + .state = target_state, + .substate = 0, + }; + + if (!pm_runtime_suspended(sdev->dev)) + imx8ulp_suspend(sdev); + + return snd_sof_dsp_set_power_state(sdev, &target_dsp_state); +} + +static int imx8ulp_dsp_resume(struct snd_sof_dev *sdev) +{ + const struct sof_dsp_power_state target_dsp_state = { + .state = SOF_DSP_PM_D0, + .substate = 0, + }; + + imx8ulp_resume(sdev); + + if (pm_runtime_suspended(sdev->dev)) { + pm_runtime_disable(sdev->dev); + pm_runtime_set_active(sdev->dev); + pm_runtime_mark_last_busy(sdev->dev); + pm_runtime_enable(sdev->dev); + pm_runtime_idle(sdev->dev); + } + + return snd_sof_dsp_set_power_state(sdev, &target_dsp_state); +} + +static struct snd_soc_dai_driver imx8ulp_dai[] = { + { + .name = "sai5", + .playback = { + .channels_min = 1, + .channels_max = 32, + }, + .capture = { + .channels_min = 1, + .channels_max = 32, + }, + }, + { + .name = "sai6", + .playback = { + .channels_min = 1, + .channels_max = 32, + }, + .capture = { + .channels_min = 1, + .channels_max = 32, + }, + }, +}; + +static int imx8ulp_dsp_set_power_state(struct snd_sof_dev *sdev, + const struct sof_dsp_power_state *target_state) +{ + sdev->dsp_power_state = *target_state; + + return 0; +} + +/* i.MX8 ops */ +struct snd_sof_dsp_ops sof_imx8ulp_ops = { + /* probe and remove */ + .probe = imx8ulp_probe, + .remove = imx8ulp_remove, + /* DSP core boot */ + .run = imx8ulp_run, + .reset = imx8ulp_reset, + + /* Block IO */ + .block_read = sof_block_read, + .block_write = sof_block_write, + + /* Module IO */ + .read64 = sof_io_read64, + + /* Mailbox IO */ + .mailbox_read = sof_mailbox_read, + .mailbox_write = sof_mailbox_write, + + /* ipc */ + .send_msg = imx8ulp_send_msg, + .get_mailbox_offset = imx8ulp_get_mailbox_offset, + .get_window_offset = imx8ulp_get_window_offset, + + .ipc_msg_data = sof_ipc_msg_data, + .set_stream_data_offset = sof_set_stream_data_offset, + + /* stream callbacks */ + .pcm_open = sof_stream_pcm_open, + .pcm_close = sof_stream_pcm_close, + + /* module loading */ + .get_bar_index = imx8ulp_get_bar_index, + /* firmware loading */ + .load_firmware = snd_sof_load_firmware_memcpy, + + /* Debug information */ + .dbg_dump = imx8_dump, + + /* Firmware ops */ + .dsp_arch_ops = &sof_xtensa_arch_ops, + + /* DAI drivers */ + .drv = imx8ulp_dai, + .num_drv = ARRAY_SIZE(imx8ulp_dai), + + /* ALSA HW info flags */ + .hw_info = SNDRV_PCM_INFO_MMAP | + SNDRV_PCM_INFO_MMAP_VALID | + SNDRV_PCM_INFO_INTERLEAVED | + SNDRV_PCM_INFO_PAUSE | + SNDRV_PCM_INFO_NO_PERIOD_WAKEUP, + + /* PM */ + .runtime_suspend = imx8ulp_dsp_runtime_suspend, + .runtime_resume = imx8ulp_dsp_runtime_resume, + + .suspend = imx8ulp_dsp_suspend, + .resume = imx8ulp_dsp_resume, + + .set_power_state = imx8ulp_dsp_set_power_state, +}; + +static struct sof_dev_desc sof_of_imx8ulp_desc = { + .ipc_supported_mask = BIT(SOF_IPC), + .ipc_default = SOF_IPC, + .default_fw_path = { + [SOF_IPC] = "imx/sof", + }, + .default_tplg_path = { + [SOF_IPC] = "imx/sof-tplg", + }, + .default_fw_filename = { + [SOF_IPC] = "sof-imx8ulp.ri", + }, + .nocodec_tplg_filename = "sof-imx8ulp-nocodec.tplg", + .ops = &sof_imx8ulp_ops, +}; + +static const struct of_device_id sof_of_imx8ulp_ids[] = { + { .compatible = "fsl,imx8ulp-dsp", .data = &sof_of_imx8ulp_desc}, + { } +}; +MODULE_DEVICE_TABLE(of, sof_of_imx8ulp_ids); + +/* DT driver definition */ +static struct platform_driver snd_sof_of_imx8ulp_driver = { + .probe = sof_of_probe, + .remove = sof_of_remove, + .driver = { + .name = "sof-audio-of-imx8ulp", + .pm = &sof_of_pm, + .of_match_table = sof_of_imx8ulp_ids, + }, +}; +module_platform_driver(snd_sof_of_imx8ulp_driver); + +MODULE_IMPORT_NS(SND_SOC_SOF_XTENSA); +MODULE_LICENSE("Dual BSD/GPL"); -- 2.34.1

5 7

Re: [Sound-open-firmware] [PATCH 1/2] ASoC: SOF: imx: Add i.MX8ULP HW support
by Shengjiu Wang 20 Jul '22

20 Jul '22

On Tue, Jul 19, 2022 at 7:41 PM Mark Brown <broonie(a)kernel.org> wrote: > On Tue, Jul 19, 2022 at 01:26:06PM +0800, Shengjiu Wang wrote: > > Not a thorough review, just a few nitpicks: > Thanks. > > > +#define MBOX_SIZE 0x1000 > > + > > +struct arm_smccc_res smc_resource; > > This should be static shouldn't it? > I will move it to function as a local variable. > > > +static void imx8ulp_dsp_handle_reply(struct imx_dsp_ipc *ipc) > > +{ > > + struct imx8ulp_priv *priv = imx_dsp_get_data(ipc); > > + unsigned long flags; > > + > > + spin_lock_irqsave(&priv->sdev->ipc_lock, flags); > > + > > + imx8ulp_get_reply(priv->sdev); > > + snd_sof_ipc_reply(priv->sdev, 0); > > + spin_unlock_irqrestore(&priv->sdev->ipc_lock, flags); > > Minor nitpick but a blank line before the unlock to match the one after > the lock would be a bit easier to read. > ok, will update > > > + regmap_update_bits(priv->regmap, SYSCTRL0, EXECUTE_BIT, > EXECUTE_BIT); > > + usleep_range(1, 2); > > + > > + arm_smccc_smc(FSL_SIP_HIFI_XRDC, 0, 0, 0, 0, 0, 0, 0, > &smc_resource); > > You need linux/arm-smccc.h for this (as 0day said). > Yes, right. > > > + if (ret) { > > + dev_err(&pdev->dev, "failed to init reserved memory region > %d\n", ret); > > + goto exit_pdev_unregister; > > + } > > + > > + priv->clks->dsp_clks = imx8ulp_dsp_clks; > > + priv->clks->num_dsp_clks = ARRAY_SIZE(imx8ulp_dsp_clks); > > + > > + ret = imx8_parse_clocks(sdev, priv->clks); > > + if (ret < 0) > > + goto exit_pdev_unregister; > > + > > + ret = imx8_enable_clocks(sdev, priv->clks); > > + if (ret < 0) > > + goto exit_pdev_unregister; > > We're registering the platform device before we enable the clocks - is > that safe? > Yes, it is safe. > > > +static int imx8ulp_remove(struct snd_sof_dev *sdev) > > +{ > > + struct imx8ulp_priv *priv = sdev->pdata->hw_pdata; > > + > > + platform_device_unregister(priv->ipc_dev); > > + > > + return 0; > > +} > > Could we just use devm? I'm not seeing an ordering issue but I might be > missing something. > Which devm do you mean? There seems no devm_platform_device_register_data(). best regards wang shengjiu

1 0

[Sound-open-firmware] [PATCH v2] ASoC: SOF: ipc-msg-injector: fix copy in sof_msg_inject_ipc4_dfs_write()
by Dan Carpenter 14 Jul '22

14 Jul '22

There are two bugs that have to do with when we copy the payload: size = simple_write_to_buffer(ipc4_msg->data_ptr, priv->max_msg_size, ppos, buffer, count); The value of "*ppos" was supposed to be zero but it is sizeof(ipc4_msg->header_u64) so it will copy the data into the middle of the "ipc4_msg->data_ptr" buffer instead of to the start. The second problem is "buffer" should be "buffer + sizeof(ipc4_msg->header_u64)". This function is used for fuzz testing so the data is normally random and this bug likely does not affect anyone very much. In this context, it's simpler and more appropriate to use copy_from_user() instead of simple_write_to_buffer() so I have re-written the function. Fixes: 066c67624d8c ("ASoC: SOF: ipc-msg-injector: Add support for IPC4 messages") Signed-off-by: Dan Carpenter <dan.carpenter(a)oracle.com> --- v2: In v1 I missed the "buffer + sizeof(ipc4_msg->header_u64)" bug. sound/soc/sof/sof-client-ipc-msg-injector.c | 29 +++++++++------------ 1 file changed, 12 insertions(+), 17 deletions(-) diff --git a/sound/soc/sof/sof-client-ipc-msg-injector.c b/sound/soc/sof/sof-client-ipc-msg-injector.c index 6bdfa527b7f7..752d5320680f 100644 --- a/sound/soc/sof/sof-client-ipc-msg-injector.c +++ b/sound/soc/sof/sof-client-ipc-msg-injector.c @@ -181,7 +181,7 @@ static ssize_t sof_msg_inject_ipc4_dfs_write(struct file *file, struct sof_client_dev *cdev = file->private_data; struct sof_msg_inject_priv *priv = cdev->data; struct sof_ipc4_msg *ipc4_msg = priv->tx_buffer; - ssize_t size; + size_t data_size; int ret; if (*ppos) @@ -191,25 +191,20 @@ static ssize_t sof_msg_inject_ipc4_dfs_write(struct file *file, return -EINVAL; /* copy the header first */ - size = simple_write_to_buffer(&ipc4_msg->header_u64, - sizeof(ipc4_msg->header_u64), - ppos, buffer, count); - if (size < 0) - return size; - if (size != sizeof(ipc4_msg->header_u64)) + if (copy_from_user(&ipc4_msg->header_u64, buffer, + sizeof(ipc4_msg->header_u64))) return -EFAULT; - count -= size; + data_size = count - sizeof(ipc4_msg->header_u64); + if (data_size > priv->max_msg_size) + return -EINVAL; + /* Copy the payload */ - size = simple_write_to_buffer(ipc4_msg->data_ptr, - priv->max_msg_size, ppos, buffer, - count); - if (size < 0) - return size; - if (size != count) + if (copy_from_user(ipc4_msg->data_ptr, + buffer + sizeof(ipc4_msg->header_u64), data_size)) return -EFAULT; - ipc4_msg->data_size = count; + ipc4_msg->data_size = data_size; /* Initialize the reply storage */ ipc4_msg = priv->rx_buffer; @@ -221,9 +216,9 @@ static ssize_t sof_msg_inject_ipc4_dfs_write(struct file *file, /* return the error code if test failed */ if (ret < 0) - size = ret; + return ret; - return size; + return count; }; static int sof_msg_inject_dfs_release(struct inode *inode, struct file *file) -- 2.35.1

2 1

[Sound-open-firmware] [PATCH AUTOSEL 5.10 03/15] ASoC: SOF: Intel: hda-loader: Clarify the cl_dsp_init() flow
by Sasha Levin 14 Jul '22

14 Jul '22

From: Peter Ujfalusi <peter.ujfalusi(a)linux.intel.com> [ Upstream commit bbfef046c6613404c01aeb9e9928bebb78dd327a ] Update the comment for the cl_dsp_init() to clarify what is done by the function and use the chip->init_core_mask instead of BIT(0) when unstalling/running the init core. Complements: 2a68ff846164 ("ASoC: SOF: Intel: hda: Revisit IMR boot sequence") Signed-off-by: Peter Ujfalusi <peter.ujfalusi(a)linux.intel.com> Reviewed-by: Pierre-Louis Bossart <pierre-louis.bossart(a)linux.intel.com> Reviewed-by: Bard Liao <yung-chuan.liao(a)linux.intel.com> Reviewed-by: Ranjani Sridharan <ranjani.sridharan(a)linux.intel.com> Link: https://lore.kernel.org/r/20220609085949.29062-4-peter.ujfalusi@linux.intel… Signed-off-by: Mark Brown <broonie(a)kernel.org> Signed-off-by: Sasha Levin <sashal(a)kernel.org> --- sound/soc/sof/intel/hda-loader.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/sound/soc/sof/intel/hda-loader.c b/sound/soc/sof/intel/hda-loader.c index 347636a80b48..4012097a9d60 100644 --- a/sound/soc/sof/intel/hda-loader.c +++ b/sound/soc/sof/intel/hda-loader.c @@ -79,9 +79,9 @@ static struct hdac_ext_stream *cl_stream_prepare(struct snd_sof_dev *sdev, unsig } /* - * first boot sequence has some extra steps. core 0 waits for power - * status on core 1, so power up core 1 also momentarily, keep it in - * reset/stall and then turn it off + * first boot sequence has some extra steps. + * power on all host managed cores and only unstall/run the boot core to boot the + * DSP then turn off all non boot cores (if any) is powered on. */ static int cl_dsp_init(struct snd_sof_dev *sdev, int stream_tag) { @@ -115,7 +115,7 @@ static int cl_dsp_init(struct snd_sof_dev *sdev, int stream_tag) ((stream_tag - 1) << 9))); /* step 3: unset core 0 reset state & unstall/run core 0 */ - ret = hda_dsp_core_run(sdev, BIT(0)); + ret = hda_dsp_core_run(sdev, chip->init_core_mask); if (ret < 0) { if (hda->boot_iteration == HDA_FW_BOOT_ATTEMPTS) dev_err(sdev->dev, -- 2.35.1

1 0

[Sound-open-firmware] [PATCH AUTOSEL 5.15 09/28] ASoC: SOF: Intel: hda-loader: Clarify the cl_dsp_init() flow
by Sasha Levin 14 Jul '22

14 Jul '22

From: Peter Ujfalusi <peter.ujfalusi(a)linux.intel.com> [ Upstream commit bbfef046c6613404c01aeb9e9928bebb78dd327a ] Update the comment for the cl_dsp_init() to clarify what is done by the function and use the chip->init_core_mask instead of BIT(0) when unstalling/running the init core. Complements: 2a68ff846164 ("ASoC: SOF: Intel: hda: Revisit IMR boot sequence") Signed-off-by: Peter Ujfalusi <peter.ujfalusi(a)linux.intel.com> Reviewed-by: Pierre-Louis Bossart <pierre-louis.bossart(a)linux.intel.com> Reviewed-by: Bard Liao <yung-chuan.liao(a)linux.intel.com> Reviewed-by: Ranjani Sridharan <ranjani.sridharan(a)linux.intel.com> Link: https://lore.kernel.org/r/20220609085949.29062-4-peter.ujfalusi@linux.intel… Signed-off-by: Mark Brown <broonie(a)kernel.org> Signed-off-by: Sasha Levin <sashal(a)kernel.org> --- sound/soc/sof/intel/hda-loader.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/sound/soc/sof/intel/hda-loader.c b/sound/soc/sof/intel/hda-loader.c index 14469e087b00..ee09393d42cb 100644 --- a/sound/soc/sof/intel/hda-loader.c +++ b/sound/soc/sof/intel/hda-loader.c @@ -80,9 +80,9 @@ static struct hdac_ext_stream *cl_stream_prepare(struct snd_sof_dev *sdev, unsig } /* - * first boot sequence has some extra steps. core 0 waits for power - * status on core 1, so power up core 1 also momentarily, keep it in - * reset/stall and then turn it off + * first boot sequence has some extra steps. + * power on all host managed cores and only unstall/run the boot core to boot the + * DSP then turn off all non boot cores (if any) is powered on. */ static int cl_dsp_init(struct snd_sof_dev *sdev, int stream_tag) { @@ -117,7 +117,7 @@ static int cl_dsp_init(struct snd_sof_dev *sdev, int stream_tag) ((stream_tag - 1) << 9))); /* step 3: unset core 0 reset state & unstall/run core 0 */ - ret = hda_dsp_core_run(sdev, BIT(0)); + ret = hda_dsp_core_run(sdev, chip->init_core_mask); if (ret < 0) { if (hda->boot_iteration == HDA_FW_BOOT_ATTEMPTS) dev_err(sdev->dev, -- 2.35.1

1 0

[Sound-open-firmware] [PATCH AUTOSEL 5.18 11/41] ASoC: SOF: Intel: hda-loader: Clarify the cl_dsp_init() flow
by Sasha Levin 14 Jul '22

14 Jul '22

From: Peter Ujfalusi <peter.ujfalusi(a)linux.intel.com> [ Upstream commit bbfef046c6613404c01aeb9e9928bebb78dd327a ] Update the comment for the cl_dsp_init() to clarify what is done by the function and use the chip->init_core_mask instead of BIT(0) when unstalling/running the init core. Complements: 2a68ff846164 ("ASoC: SOF: Intel: hda: Revisit IMR boot sequence") Signed-off-by: Peter Ujfalusi <peter.ujfalusi(a)linux.intel.com> Reviewed-by: Pierre-Louis Bossart <pierre-louis.bossart(a)linux.intel.com> Reviewed-by: Bard Liao <yung-chuan.liao(a)linux.intel.com> Reviewed-by: Ranjani Sridharan <ranjani.sridharan(a)linux.intel.com> Link: https://lore.kernel.org/r/20220609085949.29062-4-peter.ujfalusi@linux.intel… Signed-off-by: Mark Brown <broonie(a)kernel.org> Signed-off-by: Sasha Levin <sashal(a)kernel.org> --- sound/soc/sof/intel/hda-loader.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/sound/soc/sof/intel/hda-loader.c b/sound/soc/sof/intel/hda-loader.c index 9f624a84182b..88d23924e1bf 100644 --- a/sound/soc/sof/intel/hda-loader.c +++ b/sound/soc/sof/intel/hda-loader.c @@ -97,9 +97,9 @@ static struct hdac_ext_stream *cl_stream_prepare(struct snd_sof_dev *sdev, unsig } /* - * first boot sequence has some extra steps. core 0 waits for power - * status on core 1, so power up core 1 also momentarily, keep it in - * reset/stall and then turn it off + * first boot sequence has some extra steps. + * power on all host managed cores and only unstall/run the boot core to boot the + * DSP then turn off all non boot cores (if any) is powered on. */ static int cl_dsp_init(struct snd_sof_dev *sdev, int stream_tag) { @@ -127,7 +127,7 @@ static int cl_dsp_init(struct snd_sof_dev *sdev, int stream_tag) ((stream_tag - 1) << 9))); /* step 3: unset core 0 reset state & unstall/run core 0 */ - ret = hda_dsp_core_run(sdev, BIT(0)); + ret = hda_dsp_core_run(sdev, chip->init_core_mask); if (ret < 0) { if (hda->boot_iteration == HDA_FW_BOOT_ATTEMPTS) dev_err(sdev->dev, -- 2.35.1

1 0

[Sound-open-firmware] [PATCH AUTOSEL 5.18 10/41] ASoC: SOF: Intel: hda-loader: Make sure that the fw load sequence is followed
by Sasha Levin 14 Jul '22

14 Jul '22

From: Peter Ujfalusi <peter.ujfalusi(a)linux.intel.com> [ Upstream commit c31691e0d126ec5d60d2b6b03f699c11b613b219 ] The hda_dsp_enable_core() is powering up _and_ unstall the core in one call while the first step of the firmware loading must not unstall the core. The core can be unstalled only after the set cpb_cfp and the configuration of the IPC register for the ROM_CONTROL message. Complements: 2a68ff846164 ("ASoC: SOF: Intel: hda: Revisit IMR boot sequence") Signed-off-by: Peter Ujfalusi <peter.ujfalusi(a)linux.intel.com> Reviewed-by: Pierre-Louis Bossart <pierre-louis.bossart(a)linux.intel.com> Reviewed-by: Bard Liao <yung-chuan.liao(a)linux.intel.com> Reviewed-by: Ranjani Sridharan <ranjani.sridharan(a)linux.intel.com> Link: https://lore.kernel.org/r/20220609085949.29062-3-peter.ujfalusi@linux.intel… Signed-off-by: Mark Brown <broonie(a)kernel.org> Signed-off-by: Sasha Levin <sashal(a)kernel.org> --- sound/soc/sof/intel/hda-loader.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sound/soc/sof/intel/hda-loader.c b/sound/soc/sof/intel/hda-loader.c index 2ac5d9d0719b..9f624a84182b 100644 --- a/sound/soc/sof/intel/hda-loader.c +++ b/sound/soc/sof/intel/hda-loader.c @@ -112,7 +112,7 @@ static int cl_dsp_init(struct snd_sof_dev *sdev, int stream_tag) int ret; /* step 1: power up corex */ - ret = hda_dsp_enable_core(sdev, chip->host_managed_cores_mask); + ret = hda_dsp_core_power_up(sdev, chip->host_managed_cores_mask); if (ret < 0) { if (hda->boot_iteration == HDA_FW_BOOT_ATTEMPTS) dev_err(sdev->dev, "error: dsp core 0/1 power up failed\n"); -- 2.35.1

1 0

[Sound-open-firmware] [PATCH AUTOSEL 5.18 09/41] ASoC: SOF: Intel: hda-dsp: Expose hda_dsp_core_power_up()
by Sasha Levin 14 Jul '22

14 Jul '22

From: Peter Ujfalusi <peter.ujfalusi(a)linux.intel.com> [ Upstream commit 08f8a93198e300dff9649bbae424cd805d313326 ] The hda_dsp_core_power_up() needs to be exposed so that it can be used in hda-loader.c to correct the boot flow. The first step must not unstall the core, it should only power up the core(s). Add sanity check for the core_mask while exposing it to be safe. Complements: 2a68ff846164 ("ASoC: SOF: Intel: hda: Revisit IMR boot sequence") Signed-off-by: Peter Ujfalusi <peter.ujfalusi(a)linux.intel.com> Reviewed-by: Pierre-Louis Bossart <pierre-louis.bossart(a)linux.intel.com> Reviewed-by: Bard Liao <yung-chuan.liao(a)linux.intel.com> Reviewed-by: Ranjani Sridharan <ranjani.sridharan(a)linux.intel.com> Link: https://lore.kernel.org/r/20220609085949.29062-2-peter.ujfalusi@linux.intel… Signed-off-by: Mark Brown <broonie(a)kernel.org> Signed-off-by: Sasha Levin <sashal(a)kernel.org> --- sound/soc/sof/intel/hda-dsp.c | 10 +++++++++- sound/soc/sof/intel/hda.h | 1 + 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/sound/soc/sof/intel/hda-dsp.c b/sound/soc/sof/intel/hda-dsp.c index 8ddde60c56b3..68a8074c956a 100644 --- a/sound/soc/sof/intel/hda-dsp.c +++ b/sound/soc/sof/intel/hda-dsp.c @@ -181,12 +181,20 @@ int hda_dsp_core_run(struct snd_sof_dev *sdev, unsigned int core_mask) * Power Management. */ -static int hda_dsp_core_power_up(struct snd_sof_dev *sdev, unsigned int core_mask) +int hda_dsp_core_power_up(struct snd_sof_dev *sdev, unsigned int core_mask) { + struct sof_intel_hda_dev *hda = sdev->pdata->hw_pdata; + const struct sof_intel_dsp_desc *chip = hda->desc; unsigned int cpa; u32 adspcs; int ret; + /* restrict core_mask to host managed cores mask */ + core_mask &= chip->host_managed_cores_mask; + /* return if core_mask is not valid */ + if (!core_mask) + return 0; + /* update bits */ snd_sof_dsp_update_bits(sdev, HDA_DSP_BAR, HDA_DSP_REG_ADSPCS, HDA_DSP_ADSPCS_SPA_MASK(core_mask), diff --git a/sound/soc/sof/intel/hda.h b/sound/soc/sof/intel/hda.h index 05e5e158614a..b80fa5c77cf7 100644 --- a/sound/soc/sof/intel/hda.h +++ b/sound/soc/sof/intel/hda.h @@ -490,6 +490,7 @@ struct sof_intel_hda_stream { */ int hda_dsp_probe(struct snd_sof_dev *sdev); int hda_dsp_remove(struct snd_sof_dev *sdev); +int hda_dsp_core_power_up(struct snd_sof_dev *sdev, unsigned int core_mask); int hda_dsp_core_run(struct snd_sof_dev *sdev, unsigned int core_mask); int hda_dsp_enable_core(struct snd_sof_dev *sdev, unsigned int core_mask); int hda_dsp_core_reset_power_down(struct snd_sof_dev *sdev, -- 2.35.1

1 0