| Message ID | 20250203171808.4108-2-laurentiumihalcea111@gmail.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | Refactor imx drivers and introduce support for imx95 | expand |
On Mon, Feb 03, 2025 at 12:18:00PM -0500, Laurentiu Mihalcea wrote: > From: Laurentiu Mihalcea <laurentiu.mihalcea@nxp.com> > > The SOF drivers for imx chips have a lot of duplicate code > and routines/code snippets that could certainly be reused > among drivers. > > As such, introduce a new set of structures and functions > that will help eliminate the redundancy and code size of > the drivers. please wrap at 75 chars > > Signed-off-by: Laurentiu Mihalcea <laurentiu.mihalcea@nxp.com> > Reviewed-by: Daniel Baluta <daniel.baluta@nxp.com> > Reviewed-by: Iuliana Prodan <iuliana.prodan@nxp.com> > --- > sound/soc/sof/imx/imx-common.c | 419 ++++++++++++++++++++++++++++++++- > sound/soc/sof/imx/imx-common.h | 149 ++++++++++++ > 2 files changed, 567 insertions(+), 1 deletion(-) > > diff --git a/sound/soc/sof/imx/imx-common.c b/sound/soc/sof/imx/imx-common.c > index fce6d9cf6a6b..5921900335c8 100644 > --- a/sound/soc/sof/imx/imx-common.c > +++ b/sound/soc/sof/imx/imx-common.c > @@ -1,11 +1,16 @@ > // SPDX-License-Identifier: (GPL-2.0-only OR BSD-3-Clause) > // > -// Copyright 2020 NXP > +// Copyright 2020-2025 NXP > // > // Common helpers for the audio DSP on i.MX8 > > +#include <linux/firmware/imx/dsp.h> > #include <linux/module.h> > +#include <linux/of_reserved_mem.h> > +#include <linux/of_address.h> keep alphabet order > +#include <linux/pm_domain.h> > #include <sound/sof/xtensa.h> > + > #include "../ops.h" > > #include "imx-common.h" > @@ -74,5 +79,417 @@ void imx8_dump(struct snd_sof_dev *sdev, u32 flags) > } > EXPORT_SYMBOL(imx8_dump); > > +static void imx_handle_reply(struct imx_dsp_ipc *ipc) > +{ > + unsigned long flags; > + struct snd_sof_dev *sdev = imx_dsp_get_data(ipc); > + > + spin_lock_irqsave(&sdev->ipc_lock, flags); > + snd_sof_ipc_process_reply(sdev, 0); > + spin_unlock_irqrestore(&sdev->ipc_lock, flags); Are you sure have to use spin_lock? > +} > + > +static void imx_handle_request(struct imx_dsp_ipc *ipc) > +{ > + struct snd_sof_dev *sdev; > + u32 panic_code; > + > + sdev = imx_dsp_get_data(ipc); > + > + if (get_chip_info(sdev)->ipc_info.has_panic_code) { > + sof_mailbox_read(sdev, sdev->debug_box.offset + 0x4, > + &panic_code, > + sizeof(panic_code)); > + > + if ((panic_code & SOF_IPC_PANIC_MAGIC_MASK) == SOF_IPC_PANIC_MAGIC) { > + snd_sof_dsp_panic(sdev, panic_code, true); > + return; > + } > + } > + > + snd_sof_ipc_msgs_rx(sdev); > +} > + > +static struct imx_dsp_ops imx_ipc_ops = { > + .handle_reply = imx_handle_reply, > + .handle_request = imx_handle_request, > +}; > + > +static int imx_send_msg(struct snd_sof_dev *sdev, struct snd_sof_ipc_msg *msg) > +{ > + struct imx_common_data *common = sdev->pdata->hw_pdata; > + > + sof_mailbox_write(sdev, sdev->host_box.offset, msg->msg_data, msg->msg_size); > + imx_dsp_ring_doorbell(common->ipc_handle, 0x0); > + > + return 0; > +} > + > +static int imx_get_bar_index(struct snd_sof_dev *sdev, u32 type) > +{ > + switch (type) { > + case SOF_FW_BLK_TYPE_IRAM: > + case SOF_FW_BLK_TYPE_SRAM: > + return type; > + default: > + return -EINVAL; > + } > +} > + > +static int imx_get_mailbox_offset(struct snd_sof_dev *sdev) > +{ > + return get_chip_info(sdev)->ipc_info.boot_mbox_offset; > +} > + > +static int imx_get_window_offset(struct snd_sof_dev *sdev, u32 id) > +{ > + return get_chip_info(sdev)->ipc_info.window_offset; > +} > + > +static int imx_set_power_state(struct snd_sof_dev *sdev, > + const struct sof_dsp_power_state *target) > +{ > + sdev->dsp_power_state = *target; > + return 0; > +} > + > +static int imx_common_resume(struct snd_sof_dev *sdev) > +{ > + struct imx_common_data *common; > + int ret, i; > + > + common = sdev->pdata->hw_pdata; > + > + ret = clk_bulk_prepare_enable(common->clk_num, common->clks); > + if (ret) > + dev_err(sdev->dev, "failed to enable clocks: %d\n", ret); > + > + for (i = 0; i < DSP_MU_CHAN_NUM; i++) > + imx_dsp_request_channel(common->ipc_handle, i); > + > + /* done. If need be, core will be started by SOF core immediately after */ > + return 0; > +} > + > +static int imx_common_suspend(struct snd_sof_dev *sdev) > +{ > + struct imx_common_data *common; > + int i, ret; > + > + common = sdev->pdata->hw_pdata; > + > + ret = imx_chip_core_shutdown(sdev); > + if (ret < 0) { > + dev_err(sdev->dev, "failed to shutdown core: %d\n", ret); > + return ret; > + } > + > + for (i = 0; i < DSP_MU_CHAN_NUM; i++) > + imx_dsp_free_channel(common->ipc_handle, i); > + > + clk_bulk_disable_unprepare(common->clk_num, common->clks); > + > + return 0; > +} > + > +static int imx_runtime_resume(struct snd_sof_dev *sdev) > +{ > + int ret; > + const struct sof_dsp_power_state target_state = { > + .state = SOF_DSP_PM_D0, > + }; > + > + ret = imx_common_resume(sdev); > + if (ret < 0) { > + dev_err(sdev->dev, "failed to runtime common resume: %d\n", ret); > + return ret; > + } > + > + return snd_sof_dsp_set_power_state(sdev, &target_state); > +} > + > +static int imx_resume(struct snd_sof_dev *sdev) > +{ > + int ret; > + const struct sof_dsp_power_state target_state = { > + .state = SOF_DSP_PM_D0, > + }; > + > + ret = imx_common_resume(sdev); > + if (ret < 0) { > + dev_err(sdev->dev, "failed to common resume: %d\n", ret); > + return ret; > + } > + > + 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_state); > +} > + > +static int imx_runtime_suspend(struct snd_sof_dev *sdev) > +{ > + int ret; > + const struct sof_dsp_power_state target_state = { > + .state = SOF_DSP_PM_D3, > + }; > + > + ret = imx_common_suspend(sdev); > + if (ret < 0) > + dev_err(sdev->dev, "failed to runtime common suspend: %d\n", ret); > + > + return snd_sof_dsp_set_power_state(sdev, &target_state); > +} > + > +static int imx_suspend(struct snd_sof_dev *sdev, unsigned int target_state) > +{ > + int ret; > + const struct sof_dsp_power_state target_power_state = { > + .state = target_state, > + }; > + > + if (!pm_runtime_suspended(sdev->dev)) { > + ret = imx_common_suspend(sdev); > + if (ret < 0) { > + dev_err(sdev->dev, "failed to common suspend: %d\n", ret); > + return ret; > + } > + } > + > + return snd_sof_dsp_set_power_state(sdev, &target_power_state); does pm_runtime_force_suspend()/pm_runtime_force_resume() work? > +} > + > +static int imx_region_name_to_blk_type(const char *region_name) > +{ > + if (!strcmp(region_name, "iram")) > + return SOF_FW_BLK_TYPE_IRAM; > + else if (!strcmp(region_name, "dram")) > + return SOF_FW_BLK_TYPE_DRAM; > + else if (!strcmp(region_name, "sram")) > + return SOF_FW_BLK_TYPE_SRAM; > + else > + return -EINVAL; > +} > + > +static int imx_parse_ioremap_memory(struct snd_sof_dev *sdev) > +{ > + struct platform_device *pdev; > + const struct imx_chip_info *chip_info; > + phys_addr_t base, size; > + struct resource *res; > + struct reserved_mem *reserved; > + struct device_node *res_np; > + int i, blk_type, ret; > + > + pdev = to_platform_device(sdev->dev); > + chip_info = get_chip_info(sdev); > + > + for (i = 0; chip_info->memory[i].name; i++) { > + blk_type = imx_region_name_to_blk_type(chip_info->memory[i].name); > + if (blk_type < 0) > + return dev_err_probe(sdev->dev, blk_type, > + "no blk type for region %s\n", > + chip_info->memory[i].name); > + > + if (!chip_info->memory[i].reserved) { > + res = platform_get_resource_byname(pdev, IORESOURCE_MEM, > + chip_info->memory[i].name); > + if (!res) > + return dev_err_probe(sdev->dev, -ENODEV, > + "failed to fetch %s resource\n", > + chip_info->memory[i].name); > + > + base = res->start; > + size = resource_size(res); > + } else { > + ret = of_property_match_string(pdev->dev.of_node, > + "memory-region-names", > + chip_info->memory[i].name); > + if (ret < 0) > + return dev_err_probe(sdev->dev, ret, > + "no valid index for %s\n", > + chip_info->memory[i].name); > + > + res_np = of_parse_phandle(pdev->dev.of_node, > + "memory-region", > + ret); > + if (!res_np) > + return dev_err_probe(sdev->dev, -ENODEV, > + "failed to parse phandle %s\n", > + chip_info->memory[i].name); > + > + reserved = of_reserved_mem_lookup(res_np); > + of_node_put(res_np); > + if (!reserved) > + return dev_err_probe(sdev->dev, -ENODEV, > + "failed to get %s reserved\n", > + chip_info->memory[i].name); > + > + base = reserved->base; > + size = reserved->size; > + } > + > + sdev->bar[blk_type] = devm_ioremap(sdev->dev, base, size); > + if (IS_ERR(sdev->bar[blk_type])) > + return dev_err_probe(sdev->dev, > + PTR_ERR(sdev->bar[blk_type]), > + "failed to ioremap %s region\n", > + chip_info->memory[i].name); > + } > + > + return 0; > +} > + > +static void imx_unregister_action(void *data) > +{ > + platform_device_unregister(data); > +} > + > +static int imx_probe(struct snd_sof_dev *sdev) > +{ > + int ret; > + struct platform_device *pdev; > + struct imx_common_data *common; > + struct dev_pm_domain_attach_data domain_data = { > + .pd_names = NULL, /* no filtering */ > + .pd_flags = PD_FLAG_DEV_LINK_ON, > + }; try keep reverse Christmas tree. > + > + pdev = to_platform_device(sdev->dev); > + > + common = devm_kzalloc(sdev->dev, sizeof(*common), GFP_KERNEL); > + if (!common) > + return dev_err_probe(sdev->dev, -ENOMEM, > + "failed to allocate common data\n"); > + > + common->ipc_dev = platform_device_register_data(sdev->dev, "imx-dsp", > + PLATFORM_DEVID_NONE, > + pdev, sizeof(*pdev)); > + if (IS_ERR(common->ipc_dev)) > + return dev_err_probe(sdev->dev, PTR_ERR(common->ipc_dev), > + "failed to create IPC device\n"); > + > + /* let the devres API take care of unregistering this platform > + * driver when no longer required. > + */ I remember only network subsystem use this type comments style. > + ret = devm_add_action_or_reset(sdev->dev, > + imx_unregister_action, > + common->ipc_dev); > + if (ret) > + return dev_err_probe(sdev->dev, ret, "failed to add devm action\n"); > + > + common->ipc_handle = dev_get_drvdata(&common->ipc_dev->dev); > + if (!common->ipc_handle) > + return dev_err_probe(sdev->dev, -EPROBE_DEFER, > + "failed to fetch IPC handle\n"); > + > + ret = imx_parse_ioremap_memory(sdev); > + if (ret < 0) > + return dev_err_probe(sdev->dev, ret, > + "failed to parse/ioremap memory regions\n"); > + > + if (get_chip_info(sdev)->has_dma_reserved) { > + ret = of_reserved_mem_device_init_by_name(sdev->dev, > + pdev->dev.of_node, > + "dma"); do you need put "of_reserved_mem_device_release()" at imx_unregister_action? The below error path will miss call of_reserved_mem_device_release(). > + if (ret) > + return dev_err_probe(sdev->dev, ret, > + "failed to bind DMA region\n"); > + } > + > + if (!sdev->dev->pm_domain) { > + ret = devm_pm_domain_attach_list(sdev->dev, > + &domain_data, &common->pd_list); > + if (ret < 0) > + return dev_err_probe(sdev->dev, ret, "failed to attach PDs\n"); > + } > + > + ret = devm_clk_bulk_get_all(sdev->dev, &common->clks); > + if (ret < 0) > + return dev_err_probe(sdev->dev, common->clk_num, > + "failed to fetch clocks\n"); > + common->clk_num = ret; > + > + /* no effect if number of clocks is 0 */ > + ret = clk_bulk_prepare_enable(common->clk_num, common->clks); > + if (ret < 0) > + return dev_err_probe(sdev->dev, ret, "failed to enable clocks\n"); > + > + common->ipc_handle->ops = &imx_ipc_ops; > + imx_dsp_set_data(common->ipc_handle, sdev); > + > + sdev->num_cores = 1; > + sdev->pdata->hw_pdata = common; > + sdev->mailbox_bar = SOF_FW_BLK_TYPE_SRAM; > + sdev->dsp_box.offset = get_chip_info(sdev)->ipc_info.boot_mbox_offset; > + > + return imx_chip_probe(sdev); > +} > + > +static void imx_remove(struct snd_sof_dev *sdev) > +{ > + struct imx_common_data *common = sdev->pdata->hw_pdata; > + int ret; > + > + common = sdev->pdata->hw_pdata; > + > + if (!pm_runtime_suspended(sdev->dev)) { > + ret = imx_chip_core_shutdown(sdev); > + if (ret < 0) > + dev_err(sdev->dev, "failed to shutdown core: %d\n", ret); > + > + clk_bulk_disable_unprepare(common->clk_num, common->clks); > + } > +} > + > +const struct snd_sof_dsp_ops sof_imx_ops = { > + .probe = imx_probe, > + .remove = imx_remove, > + > + .run = imx_chip_core_kick, > + .reset = imx_chip_core_reset, > + > + .block_read = sof_block_read, > + .block_write = sof_block_write, > + > + .mailbox_read = sof_mailbox_read, > + .mailbox_write = sof_mailbox_write, > + > + .send_msg = imx_send_msg, > + .get_mailbox_offset = imx_get_mailbox_offset, > + .get_window_offset = imx_get_window_offset, > + > + .ipc_msg_data = sof_ipc_msg_data, > + .set_stream_data_offset = sof_set_stream_data_offset, > + > + .get_bar_index = imx_get_bar_index, > + .load_firmware = snd_sof_load_firmware_memcpy, > + > + .debugfs_add_region_item = snd_sof_debugfs_add_region_item_iomem, > + > + .pcm_open = sof_stream_pcm_open, > + .pcm_close = sof_stream_pcm_close, > + > + .runtime_suspend = imx_runtime_suspend, > + .runtime_resume = imx_runtime_resume, > + .suspend = imx_suspend, > + .resume = imx_resume, > + > + .set_power_state = imx_set_power_state, > + > + .hw_info = SNDRV_PCM_INFO_MMAP | > + SNDRV_PCM_INFO_MMAP_VALID | > + SNDRV_PCM_INFO_INTERLEAVED | > + SNDRV_PCM_INFO_PAUSE | > + SNDRV_PCM_INFO_BATCH | > + SNDRV_PCM_INFO_NO_PERIOD_WAKEUP, > +}; > +EXPORT_SYMBOL(sof_imx_ops); > + > MODULE_LICENSE("Dual BSD/GPL"); > MODULE_DESCRIPTION("SOF helpers for IMX platforms"); > diff --git a/sound/soc/sof/imx/imx-common.h b/sound/soc/sof/imx/imx-common.h > index 13d7f3ef675e..b31898866681 100644 > --- a/sound/soc/sof/imx/imx-common.h > +++ b/sound/soc/sof/imx/imx-common.h > @@ -4,10 +4,157 @@ > #define __IMX_COMMON_H__ > > #include <linux/clk.h> > +#include <linux/of_platform.h> > +#include <sound/sof/xtensa.h> > + > +#include "../sof-of-dev.h" > +#include "../ops.h" > > #define EXCEPT_MAX_HDR_SIZE 0x400 > #define IMX8_STACK_DUMP_SIZE 32 > > +/* chip_info refers to the data stored in struct sof_dev_desc's chip_info */ > +#define get_chip_info(sdev)\ > + ((const struct imx_chip_info *)((sdev)->pdata->desc->chip_info)) > + > +/* chip_pdata refers to the data stored in struct imx_common_data's chip_pdata */ > +#define get_chip_pdata(sdev)\ > + (((struct imx_common_data *)((sdev)->pdata->hw_pdata))->chip_pdata) > + > +/* can be used if: > + * 1) The only supported IPC version is IPC3. > + * 2) The default paths/FW name match values below. > + * > + * otherwise, just explicitly declare the structure > + */ > +#define IMX_SOF_DEV_DESC(mach_name, of_machs, \ > + mach_chip_info, mach_ops, mach_ops_init) \ > +static struct sof_dev_desc sof_of_##mach_name##_desc = { \ > + .of_machines = of_machs, \ > + .chip_info = mach_chip_info, \ > + .ipc_supported_mask = BIT(SOF_IPC_TYPE_3), \ > + .ipc_default = SOF_IPC_TYPE_3, \ > + .default_fw_path = { \ > + [SOF_IPC_TYPE_3] = "imx/sof", \ > + }, \ > + .default_tplg_path = { \ > + [SOF_IPC_TYPE_3] = "imx/sof-tplg", \ > + }, \ > + .default_fw_filename = { \ > + [SOF_IPC_TYPE_3] = "sof-" #mach_name ".ri", \ > + }, \ > + .ops = mach_ops, \ > + .ops_init = mach_ops_init, \ > +} > + > +/* to be used alongside IMX_SOF_DEV_DESC() */ > +#define IMX_SOF_DEV_DESC_NAME(mach_name) sof_of_##mach_name##_desc > + > +/* dai driver entry w/ playback and capture caps. If one direction is missing > + * then set the channels to 0. > + */ > +#define IMX_SOF_DAI_DRV_ENTRY(dai_name, pb_cmin, pb_cmax, cap_cmin, cap_cmax) \ > +{ \ > + .name = dai_name, \ > + .playback = { \ > + .channels_min = pb_cmin, \ > + .channels_max = pb_cmax, \ > + }, \ > + .capture = { \ > + .channels_min = cap_cmin, \ > + .channels_max = cap_cmax, \ > + }, \ > +} > + > +/* use if playback and capture have the same min/max channel count */ > +#define IMX_SOF_DAI_DRV_ENTRY_BIDIR(dai_name, cmin, cmax)\ > + IMX_SOF_DAI_DRV_ENTRY(dai_name, cmin, cmax, cmin, cmax) > + > +struct imx_ipc_info { > + /* true if core is able to write a panic code to the debug box */ > + bool has_panic_code; > + /* offset to mailbox in which firmware initially writes FW_READY */ > + int boot_mbox_offset; > + /* offset to region at which the mailboxes start */ > + int window_offset; > +}; > + > +struct imx_chip_ops { > + /* called after clocks and PDs are enabled */ > + int (*probe)(struct snd_sof_dev *sdev); > + /* used directly by the SOF core */ > + int (*core_kick)(struct snd_sof_dev *sdev); > + /* called during suspend()/remove() before clocks are disabled */ > + int (*core_shutdown)(struct snd_sof_dev *sdev); > + /* used directly by the SOF core */ > + int (*core_reset)(struct snd_sof_dev *sdev); > +}; > + > +struct imx_memory_info { > + const char *name; > + bool reserved; > +}; > + > +struct imx_chip_info { > + struct imx_ipc_info ipc_info; > + /* does the chip have a reserved memory region for DMA? */ > + bool has_dma_reserved; > + struct imx_memory_info *memory; > + /* optional */ > + const struct imx_chip_ops *ops; > +}; > + > +struct imx_common_data { > + struct platform_device *ipc_dev; > + struct imx_dsp_ipc *ipc_handle; > + /* core may have no clocks */ > + struct clk_bulk_data *clks; > + int clk_num; > + /* core may have no PDs */ > + struct dev_pm_domain_list *pd_list; > + void *chip_pdata; > +}; > + > +static inline int imx_chip_core_kick(struct snd_sof_dev *sdev) > +{ > + const struct imx_chip_ops *ops = get_chip_info(sdev)->ops; > + > + if (ops && ops->core_kick) > + return ops->core_kick(sdev); > + > + return 0; > +} > + > +static inline int imx_chip_core_shutdown(struct snd_sof_dev *sdev) > +{ > + const struct imx_chip_ops *ops = get_chip_info(sdev)->ops; > + > + if (ops && ops->core_shutdown) > + return ops->core_shutdown(sdev); > + > + return 0; > +} > + > +static inline int imx_chip_core_reset(struct snd_sof_dev *sdev) > +{ > + const struct imx_chip_ops *ops = get_chip_info(sdev)->ops; > + > + if (ops && ops->core_reset) > + return ops->core_reset(sdev); > + > + return 0; > +} > + > +static inline int imx_chip_probe(struct snd_sof_dev *sdev) > +{ > + const struct imx_chip_ops *ops = get_chip_info(sdev)->ops; > + > + if (ops && ops->probe) > + return ops->probe(sdev); > + > + return 0; > +} > + > void imx8_get_registers(struct snd_sof_dev *sdev, > struct sof_ipc_dsp_oops_xtensa *xoops, > struct sof_ipc_panic_info *panic_info, > @@ -15,4 +162,6 @@ void imx8_get_registers(struct snd_sof_dev *sdev, > > void imx8_dump(struct snd_sof_dev *sdev, u32 flags); > > +extern const struct snd_sof_dsp_ops sof_imx_ops; > + > #endif > -- > 2.34.1 >
On 2/3/2025 9:52 PM, Frank Li wrote: > On Mon, Feb 03, 2025 at 12:18:00PM -0500, Laurentiu Mihalcea wrote: >> From: Laurentiu Mihalcea <laurentiu.mihalcea@nxp.com> >> >> The SOF drivers for imx chips have a lot of duplicate code >> and routines/code snippets that could certainly be reused >> among drivers. >> >> As such, introduce a new set of structures and functions >> that will help eliminate the redundancy and code size of >> the drivers. > please wrap at 75 chars sure, fix in v2 > >> Signed-off-by: Laurentiu Mihalcea <laurentiu.mihalcea@nxp.com> >> Reviewed-by: Daniel Baluta <daniel.baluta@nxp.com> >> Reviewed-by: Iuliana Prodan <iuliana.prodan@nxp.com> >> --- >> sound/soc/sof/imx/imx-common.c | 419 ++++++++++++++++++++++++++++++++- >> sound/soc/sof/imx/imx-common.h | 149 ++++++++++++ >> 2 files changed, 567 insertions(+), 1 deletion(-) >> >> diff --git a/sound/soc/sof/imx/imx-common.c b/sound/soc/sof/imx/imx-common.c >> index fce6d9cf6a6b..5921900335c8 100644 >> --- a/sound/soc/sof/imx/imx-common.c >> +++ b/sound/soc/sof/imx/imx-common.c >> @@ -1,11 +1,16 @@ >> // SPDX-License-Identifier: (GPL-2.0-only OR BSD-3-Clause) >> // >> -// Copyright 2020 NXP >> +// Copyright 2020-2025 NXP >> // >> // Common helpers for the audio DSP on i.MX8 >> >> +#include <linux/firmware/imx/dsp.h> >> #include <linux/module.h> >> +#include <linux/of_reserved_mem.h> >> +#include <linux/of_address.h> > keep alphabet order ok > >> +#include <linux/pm_domain.h> >> #include <sound/sof/xtensa.h> >> + >> #include "../ops.h" >> >> #include "imx-common.h" >> @@ -74,5 +79,417 @@ void imx8_dump(struct snd_sof_dev *sdev, u32 flags) >> } >> EXPORT_SYMBOL(imx8_dump); >> >> +static void imx_handle_reply(struct imx_dsp_ipc *ipc) >> +{ >> + unsigned long flags; >> + struct snd_sof_dev *sdev = imx_dsp_get_data(ipc); >> + >> + spin_lock_irqsave(&sdev->ipc_lock, flags); >> + snd_sof_ipc_process_reply(sdev, 0); >> + spin_unlock_irqrestore(&sdev->ipc_lock, flags); > Are you sure have to use spin_lock? not sure, this definition was taken from previous drivers. I'd say keep it for now as removing it would require some more testing which will take some time. (snip) >> +static int imx_suspend(struct snd_sof_dev *sdev, unsigned int target_state) >> +{ >> + int ret; >> + const struct sof_dsp_power_state target_power_state = { >> + .state = target_state, >> + }; >> + >> + if (!pm_runtime_suspended(sdev->dev)) { >> + ret = imx_common_suspend(sdev); >> + if (ret < 0) { >> + dev_err(sdev->dev, "failed to common suspend: %d\n", ret); >> + return ret; >> + } >> + } >> + >> + return snd_sof_dsp_set_power_state(sdev, &target_power_state); > does pm_runtime_force_suspend()/pm_runtime_force_resume() work? no, these functions are not called directly by the PM core, but rather by the SOF core. Using the proposed functions would result in the SOF core PM functions (i.e: sof_resume/sof_suspend) being called twice in the case of system suspend/resume, which is wrong. (snip) >> + >> +static int imx_probe(struct snd_sof_dev *sdev) >> +{ >> + int ret; >> + struct platform_device *pdev; >> + struct imx_common_data *common; >> + struct dev_pm_domain_attach_data domain_data = { >> + .pd_names = NULL, /* no filtering */ >> + .pd_flags = PD_FLAG_DEV_LINK_ON, >> + }; > try keep reverse Christmas tree. sure, fix in V2 > >> + >> + pdev = to_platform_device(sdev->dev); >> + >> + common = devm_kzalloc(sdev->dev, sizeof(*common), GFP_KERNEL); >> + if (!common) >> + return dev_err_probe(sdev->dev, -ENOMEM, >> + "failed to allocate common data\n"); >> + >> + common->ipc_dev = platform_device_register_data(sdev->dev, "imx-dsp", >> + PLATFORM_DEVID_NONE, >> + pdev, sizeof(*pdev)); >> + if (IS_ERR(common->ipc_dev)) >> + return dev_err_probe(sdev->dev, PTR_ERR(common->ipc_dev), >> + "failed to create IPC device\n"); >> + >> + /* let the devres API take care of unregistering this platform >> + * driver when no longer required. >> + */ > I remember only network subsystem use this type comments style. will fix in V2 > >> + ret = devm_add_action_or_reset(sdev->dev, >> + imx_unregister_action, >> + common->ipc_dev); >> + if (ret) >> + return dev_err_probe(sdev->dev, ret, "failed to add devm action\n"); >> + >> + common->ipc_handle = dev_get_drvdata(&common->ipc_dev->dev); >> + if (!common->ipc_handle) >> + return dev_err_probe(sdev->dev, -EPROBE_DEFER, >> + "failed to fetch IPC handle\n"); >> + >> + ret = imx_parse_ioremap_memory(sdev); >> + if (ret < 0) >> + return dev_err_probe(sdev->dev, ret, >> + "failed to parse/ioremap memory regions\n"); >> + >> + if (get_chip_info(sdev)->has_dma_reserved) { >> + ret = of_reserved_mem_device_init_by_name(sdev->dev, >> + pdev->dev.of_node, >> + "dma"); > do you need put "of_reserved_mem_device_release()" at imx_unregister_action? > > The below error path will miss call of_reserved_mem_device_release(). good catch, thx. Fix in V2.
On Tue, Feb 04, 2025 at 06:59:53PM +0200, Laurentiu Mihalcea wrote: > On 2/3/2025 9:52 PM, Frank Li wrote: > > On Mon, Feb 03, 2025 at 12:18:00PM -0500, Laurentiu Mihalcea wrote: > >> +static void imx_handle_reply(struct imx_dsp_ipc *ipc) > >> +{ > >> + unsigned long flags; > >> + struct snd_sof_dev *sdev = imx_dsp_get_data(ipc); > >> + > >> + spin_lock_irqsave(&sdev->ipc_lock, flags); > >> + snd_sof_ipc_process_reply(sdev, 0); > >> + spin_unlock_irqrestore(&sdev->ipc_lock, flags); > > Are you sure have to use spin_lock? > not sure, this definition was taken from previous drivers. I'd say keep it for now > as removing it would require some more testing which will take some time. If it's just factoring out common code it does make sense to do the factoring out then incrementally change the locking as a separate change.
diff --git a/sound/soc/sof/imx/imx-common.c b/sound/soc/sof/imx/imx-common.c index fce6d9cf6a6b..5921900335c8 100644 --- a/sound/soc/sof/imx/imx-common.c +++ b/sound/soc/sof/imx/imx-common.c @@ -1,11 +1,16 @@ // SPDX-License-Identifier: (GPL-2.0-only OR BSD-3-Clause) // -// Copyright 2020 NXP +// Copyright 2020-2025 NXP // // Common helpers for the audio DSP on i.MX8 +#include <linux/firmware/imx/dsp.h> #include <linux/module.h> +#include <linux/of_reserved_mem.h> +#include <linux/of_address.h> +#include <linux/pm_domain.h> #include <sound/sof/xtensa.h> + #include "../ops.h" #include "imx-common.h" @@ -74,5 +79,417 @@ void imx8_dump(struct snd_sof_dev *sdev, u32 flags) } EXPORT_SYMBOL(imx8_dump); +static void imx_handle_reply(struct imx_dsp_ipc *ipc) +{ + unsigned long flags; + struct snd_sof_dev *sdev = imx_dsp_get_data(ipc); + + spin_lock_irqsave(&sdev->ipc_lock, flags); + snd_sof_ipc_process_reply(sdev, 0); + spin_unlock_irqrestore(&sdev->ipc_lock, flags); +} + +static void imx_handle_request(struct imx_dsp_ipc *ipc) +{ + struct snd_sof_dev *sdev; + u32 panic_code; + + sdev = imx_dsp_get_data(ipc); + + if (get_chip_info(sdev)->ipc_info.has_panic_code) { + sof_mailbox_read(sdev, sdev->debug_box.offset + 0x4, + &panic_code, + sizeof(panic_code)); + + if ((panic_code & SOF_IPC_PANIC_MAGIC_MASK) == SOF_IPC_PANIC_MAGIC) { + snd_sof_dsp_panic(sdev, panic_code, true); + return; + } + } + + snd_sof_ipc_msgs_rx(sdev); +} + +static struct imx_dsp_ops imx_ipc_ops = { + .handle_reply = imx_handle_reply, + .handle_request = imx_handle_request, +}; + +static int imx_send_msg(struct snd_sof_dev *sdev, struct snd_sof_ipc_msg *msg) +{ + struct imx_common_data *common = sdev->pdata->hw_pdata; + + sof_mailbox_write(sdev, sdev->host_box.offset, msg->msg_data, msg->msg_size); + imx_dsp_ring_doorbell(common->ipc_handle, 0x0); + + return 0; +} + +static int imx_get_bar_index(struct snd_sof_dev *sdev, u32 type) +{ + switch (type) { + case SOF_FW_BLK_TYPE_IRAM: + case SOF_FW_BLK_TYPE_SRAM: + return type; + default: + return -EINVAL; + } +} + +static int imx_get_mailbox_offset(struct snd_sof_dev *sdev) +{ + return get_chip_info(sdev)->ipc_info.boot_mbox_offset; +} + +static int imx_get_window_offset(struct snd_sof_dev *sdev, u32 id) +{ + return get_chip_info(sdev)->ipc_info.window_offset; +} + +static int imx_set_power_state(struct snd_sof_dev *sdev, + const struct sof_dsp_power_state *target) +{ + sdev->dsp_power_state = *target; + return 0; +} + +static int imx_common_resume(struct snd_sof_dev *sdev) +{ + struct imx_common_data *common; + int ret, i; + + common = sdev->pdata->hw_pdata; + + ret = clk_bulk_prepare_enable(common->clk_num, common->clks); + if (ret) + dev_err(sdev->dev, "failed to enable clocks: %d\n", ret); + + for (i = 0; i < DSP_MU_CHAN_NUM; i++) + imx_dsp_request_channel(common->ipc_handle, i); + + /* done. If need be, core will be started by SOF core immediately after */ + return 0; +} + +static int imx_common_suspend(struct snd_sof_dev *sdev) +{ + struct imx_common_data *common; + int i, ret; + + common = sdev->pdata->hw_pdata; + + ret = imx_chip_core_shutdown(sdev); + if (ret < 0) { + dev_err(sdev->dev, "failed to shutdown core: %d\n", ret); + return ret; + } + + for (i = 0; i < DSP_MU_CHAN_NUM; i++) + imx_dsp_free_channel(common->ipc_handle, i); + + clk_bulk_disable_unprepare(common->clk_num, common->clks); + + return 0; +} + +static int imx_runtime_resume(struct snd_sof_dev *sdev) +{ + int ret; + const struct sof_dsp_power_state target_state = { + .state = SOF_DSP_PM_D0, + }; + + ret = imx_common_resume(sdev); + if (ret < 0) { + dev_err(sdev->dev, "failed to runtime common resume: %d\n", ret); + return ret; + } + + return snd_sof_dsp_set_power_state(sdev, &target_state); +} + +static int imx_resume(struct snd_sof_dev *sdev) +{ + int ret; + const struct sof_dsp_power_state target_state = { + .state = SOF_DSP_PM_D0, + }; + + ret = imx_common_resume(sdev); + if (ret < 0) { + dev_err(sdev->dev, "failed to common resume: %d\n", ret); + return ret; + } + + 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_state); +} + +static int imx_runtime_suspend(struct snd_sof_dev *sdev) +{ + int ret; + const struct sof_dsp_power_state target_state = { + .state = SOF_DSP_PM_D3, + }; + + ret = imx_common_suspend(sdev); + if (ret < 0) + dev_err(sdev->dev, "failed to runtime common suspend: %d\n", ret); + + return snd_sof_dsp_set_power_state(sdev, &target_state); +} + +static int imx_suspend(struct snd_sof_dev *sdev, unsigned int target_state) +{ + int ret; + const struct sof_dsp_power_state target_power_state = { + .state = target_state, + }; + + if (!pm_runtime_suspended(sdev->dev)) { + ret = imx_common_suspend(sdev); + if (ret < 0) { + dev_err(sdev->dev, "failed to common suspend: %d\n", ret); + return ret; + } + } + + return snd_sof_dsp_set_power_state(sdev, &target_power_state); +} + +static int imx_region_name_to_blk_type(const char *region_name) +{ + if (!strcmp(region_name, "iram")) + return SOF_FW_BLK_TYPE_IRAM; + else if (!strcmp(region_name, "dram")) + return SOF_FW_BLK_TYPE_DRAM; + else if (!strcmp(region_name, "sram")) + return SOF_FW_BLK_TYPE_SRAM; + else + return -EINVAL; +} + +static int imx_parse_ioremap_memory(struct snd_sof_dev *sdev) +{ + struct platform_device *pdev; + const struct imx_chip_info *chip_info; + phys_addr_t base, size; + struct resource *res; + struct reserved_mem *reserved; + struct device_node *res_np; + int i, blk_type, ret; + + pdev = to_platform_device(sdev->dev); + chip_info = get_chip_info(sdev); + + for (i = 0; chip_info->memory[i].name; i++) { + blk_type = imx_region_name_to_blk_type(chip_info->memory[i].name); + if (blk_type < 0) + return dev_err_probe(sdev->dev, blk_type, + "no blk type for region %s\n", + chip_info->memory[i].name); + + if (!chip_info->memory[i].reserved) { + res = platform_get_resource_byname(pdev, IORESOURCE_MEM, + chip_info->memory[i].name); + if (!res) + return dev_err_probe(sdev->dev, -ENODEV, + "failed to fetch %s resource\n", + chip_info->memory[i].name); + + base = res->start; + size = resource_size(res); + } else { + ret = of_property_match_string(pdev->dev.of_node, + "memory-region-names", + chip_info->memory[i].name); + if (ret < 0) + return dev_err_probe(sdev->dev, ret, + "no valid index for %s\n", + chip_info->memory[i].name); + + res_np = of_parse_phandle(pdev->dev.of_node, + "memory-region", + ret); + if (!res_np) + return dev_err_probe(sdev->dev, -ENODEV, + "failed to parse phandle %s\n", + chip_info->memory[i].name); + + reserved = of_reserved_mem_lookup(res_np); + of_node_put(res_np); + if (!reserved) + return dev_err_probe(sdev->dev, -ENODEV, + "failed to get %s reserved\n", + chip_info->memory[i].name); + + base = reserved->base; + size = reserved->size; + } + + sdev->bar[blk_type] = devm_ioremap(sdev->dev, base, size); + if (IS_ERR(sdev->bar[blk_type])) + return dev_err_probe(sdev->dev, + PTR_ERR(sdev->bar[blk_type]), + "failed to ioremap %s region\n", + chip_info->memory[i].name); + } + + return 0; +} + +static void imx_unregister_action(void *data) +{ + platform_device_unregister(data); +} + +static int imx_probe(struct snd_sof_dev *sdev) +{ + int ret; + struct platform_device *pdev; + struct imx_common_data *common; + struct dev_pm_domain_attach_data domain_data = { + .pd_names = NULL, /* no filtering */ + .pd_flags = PD_FLAG_DEV_LINK_ON, + }; + + pdev = to_platform_device(sdev->dev); + + common = devm_kzalloc(sdev->dev, sizeof(*common), GFP_KERNEL); + if (!common) + return dev_err_probe(sdev->dev, -ENOMEM, + "failed to allocate common data\n"); + + common->ipc_dev = platform_device_register_data(sdev->dev, "imx-dsp", + PLATFORM_DEVID_NONE, + pdev, sizeof(*pdev)); + if (IS_ERR(common->ipc_dev)) + return dev_err_probe(sdev->dev, PTR_ERR(common->ipc_dev), + "failed to create IPC device\n"); + + /* let the devres API take care of unregistering this platform + * driver when no longer required. + */ + ret = devm_add_action_or_reset(sdev->dev, + imx_unregister_action, + common->ipc_dev); + if (ret) + return dev_err_probe(sdev->dev, ret, "failed to add devm action\n"); + + common->ipc_handle = dev_get_drvdata(&common->ipc_dev->dev); + if (!common->ipc_handle) + return dev_err_probe(sdev->dev, -EPROBE_DEFER, + "failed to fetch IPC handle\n"); + + ret = imx_parse_ioremap_memory(sdev); + if (ret < 0) + return dev_err_probe(sdev->dev, ret, + "failed to parse/ioremap memory regions\n"); + + if (get_chip_info(sdev)->has_dma_reserved) { + ret = of_reserved_mem_device_init_by_name(sdev->dev, + pdev->dev.of_node, + "dma"); + if (ret) + return dev_err_probe(sdev->dev, ret, + "failed to bind DMA region\n"); + } + + if (!sdev->dev->pm_domain) { + ret = devm_pm_domain_attach_list(sdev->dev, + &domain_data, &common->pd_list); + if (ret < 0) + return dev_err_probe(sdev->dev, ret, "failed to attach PDs\n"); + } + + ret = devm_clk_bulk_get_all(sdev->dev, &common->clks); + if (ret < 0) + return dev_err_probe(sdev->dev, common->clk_num, + "failed to fetch clocks\n"); + common->clk_num = ret; + + /* no effect if number of clocks is 0 */ + ret = clk_bulk_prepare_enable(common->clk_num, common->clks); + if (ret < 0) + return dev_err_probe(sdev->dev, ret, "failed to enable clocks\n"); + + common->ipc_handle->ops = &imx_ipc_ops; + imx_dsp_set_data(common->ipc_handle, sdev); + + sdev->num_cores = 1; + sdev->pdata->hw_pdata = common; + sdev->mailbox_bar = SOF_FW_BLK_TYPE_SRAM; + sdev->dsp_box.offset = get_chip_info(sdev)->ipc_info.boot_mbox_offset; + + return imx_chip_probe(sdev); +} + +static void imx_remove(struct snd_sof_dev *sdev) +{ + struct imx_common_data *common = sdev->pdata->hw_pdata; + int ret; + + common = sdev->pdata->hw_pdata; + + if (!pm_runtime_suspended(sdev->dev)) { + ret = imx_chip_core_shutdown(sdev); + if (ret < 0) + dev_err(sdev->dev, "failed to shutdown core: %d\n", ret); + + clk_bulk_disable_unprepare(common->clk_num, common->clks); + } +} + +const struct snd_sof_dsp_ops sof_imx_ops = { + .probe = imx_probe, + .remove = imx_remove, + + .run = imx_chip_core_kick, + .reset = imx_chip_core_reset, + + .block_read = sof_block_read, + .block_write = sof_block_write, + + .mailbox_read = sof_mailbox_read, + .mailbox_write = sof_mailbox_write, + + .send_msg = imx_send_msg, + .get_mailbox_offset = imx_get_mailbox_offset, + .get_window_offset = imx_get_window_offset, + + .ipc_msg_data = sof_ipc_msg_data, + .set_stream_data_offset = sof_set_stream_data_offset, + + .get_bar_index = imx_get_bar_index, + .load_firmware = snd_sof_load_firmware_memcpy, + + .debugfs_add_region_item = snd_sof_debugfs_add_region_item_iomem, + + .pcm_open = sof_stream_pcm_open, + .pcm_close = sof_stream_pcm_close, + + .runtime_suspend = imx_runtime_suspend, + .runtime_resume = imx_runtime_resume, + .suspend = imx_suspend, + .resume = imx_resume, + + .set_power_state = imx_set_power_state, + + .hw_info = SNDRV_PCM_INFO_MMAP | + SNDRV_PCM_INFO_MMAP_VALID | + SNDRV_PCM_INFO_INTERLEAVED | + SNDRV_PCM_INFO_PAUSE | + SNDRV_PCM_INFO_BATCH | + SNDRV_PCM_INFO_NO_PERIOD_WAKEUP, +}; +EXPORT_SYMBOL(sof_imx_ops); + MODULE_LICENSE("Dual BSD/GPL"); MODULE_DESCRIPTION("SOF helpers for IMX platforms"); diff --git a/sound/soc/sof/imx/imx-common.h b/sound/soc/sof/imx/imx-common.h index 13d7f3ef675e..b31898866681 100644 --- a/sound/soc/sof/imx/imx-common.h +++ b/sound/soc/sof/imx/imx-common.h @@ -4,10 +4,157 @@ #define __IMX_COMMON_H__ #include <linux/clk.h> +#include <linux/of_platform.h> +#include <sound/sof/xtensa.h> + +#include "../sof-of-dev.h" +#include "../ops.h" #define EXCEPT_MAX_HDR_SIZE 0x400 #define IMX8_STACK_DUMP_SIZE 32 +/* chip_info refers to the data stored in struct sof_dev_desc's chip_info */ +#define get_chip_info(sdev)\ + ((const struct imx_chip_info *)((sdev)->pdata->desc->chip_info)) + +/* chip_pdata refers to the data stored in struct imx_common_data's chip_pdata */ +#define get_chip_pdata(sdev)\ + (((struct imx_common_data *)((sdev)->pdata->hw_pdata))->chip_pdata) + +/* can be used if: + * 1) The only supported IPC version is IPC3. + * 2) The default paths/FW name match values below. + * + * otherwise, just explicitly declare the structure + */ +#define IMX_SOF_DEV_DESC(mach_name, of_machs, \ + mach_chip_info, mach_ops, mach_ops_init) \ +static struct sof_dev_desc sof_of_##mach_name##_desc = { \ + .of_machines = of_machs, \ + .chip_info = mach_chip_info, \ + .ipc_supported_mask = BIT(SOF_IPC_TYPE_3), \ + .ipc_default = SOF_IPC_TYPE_3, \ + .default_fw_path = { \ + [SOF_IPC_TYPE_3] = "imx/sof", \ + }, \ + .default_tplg_path = { \ + [SOF_IPC_TYPE_3] = "imx/sof-tplg", \ + }, \ + .default_fw_filename = { \ + [SOF_IPC_TYPE_3] = "sof-" #mach_name ".ri", \ + }, \ + .ops = mach_ops, \ + .ops_init = mach_ops_init, \ +} + +/* to be used alongside IMX_SOF_DEV_DESC() */ +#define IMX_SOF_DEV_DESC_NAME(mach_name) sof_of_##mach_name##_desc + +/* dai driver entry w/ playback and capture caps. If one direction is missing + * then set the channels to 0. + */ +#define IMX_SOF_DAI_DRV_ENTRY(dai_name, pb_cmin, pb_cmax, cap_cmin, cap_cmax) \ +{ \ + .name = dai_name, \ + .playback = { \ + .channels_min = pb_cmin, \ + .channels_max = pb_cmax, \ + }, \ + .capture = { \ + .channels_min = cap_cmin, \ + .channels_max = cap_cmax, \ + }, \ +} + +/* use if playback and capture have the same min/max channel count */ +#define IMX_SOF_DAI_DRV_ENTRY_BIDIR(dai_name, cmin, cmax)\ + IMX_SOF_DAI_DRV_ENTRY(dai_name, cmin, cmax, cmin, cmax) + +struct imx_ipc_info { + /* true if core is able to write a panic code to the debug box */ + bool has_panic_code; + /* offset to mailbox in which firmware initially writes FW_READY */ + int boot_mbox_offset; + /* offset to region at which the mailboxes start */ + int window_offset; +}; + +struct imx_chip_ops { + /* called after clocks and PDs are enabled */ + int (*probe)(struct snd_sof_dev *sdev); + /* used directly by the SOF core */ + int (*core_kick)(struct snd_sof_dev *sdev); + /* called during suspend()/remove() before clocks are disabled */ + int (*core_shutdown)(struct snd_sof_dev *sdev); + /* used directly by the SOF core */ + int (*core_reset)(struct snd_sof_dev *sdev); +}; + +struct imx_memory_info { + const char *name; + bool reserved; +}; + +struct imx_chip_info { + struct imx_ipc_info ipc_info; + /* does the chip have a reserved memory region for DMA? */ + bool has_dma_reserved; + struct imx_memory_info *memory; + /* optional */ + const struct imx_chip_ops *ops; +}; + +struct imx_common_data { + struct platform_device *ipc_dev; + struct imx_dsp_ipc *ipc_handle; + /* core may have no clocks */ + struct clk_bulk_data *clks; + int clk_num; + /* core may have no PDs */ + struct dev_pm_domain_list *pd_list; + void *chip_pdata; +}; + +static inline int imx_chip_core_kick(struct snd_sof_dev *sdev) +{ + const struct imx_chip_ops *ops = get_chip_info(sdev)->ops; + + if (ops && ops->core_kick) + return ops->core_kick(sdev); + + return 0; +} + +static inline int imx_chip_core_shutdown(struct snd_sof_dev *sdev) +{ + const struct imx_chip_ops *ops = get_chip_info(sdev)->ops; + + if (ops && ops->core_shutdown) + return ops->core_shutdown(sdev); + + return 0; +} + +static inline int imx_chip_core_reset(struct snd_sof_dev *sdev) +{ + const struct imx_chip_ops *ops = get_chip_info(sdev)->ops; + + if (ops && ops->core_reset) + return ops->core_reset(sdev); + + return 0; +} + +static inline int imx_chip_probe(struct snd_sof_dev *sdev) +{ + const struct imx_chip_ops *ops = get_chip_info(sdev)->ops; + + if (ops && ops->probe) + return ops->probe(sdev); + + return 0; +} + void imx8_get_registers(struct snd_sof_dev *sdev, struct sof_ipc_dsp_oops_xtensa *xoops, struct sof_ipc_panic_info *panic_info, @@ -15,4 +162,6 @@ void imx8_get_registers(struct snd_sof_dev *sdev, void imx8_dump(struct snd_sof_dev *sdev, u32 flags); +extern const struct snd_sof_dsp_ops sof_imx_ops; + #endif