Re: [PATCH v3] remoteproc: imx_dsp_rproc: Add support for DSP-specific features
From: Iuliana Prodan
Date: Mon Apr 07 2025 - 07:17:02 EST
Hi Frank,
On 4/3/2025 10:12 PM, Frank Li wrote:
On Thu, Apr 03, 2025 at 01:01:24PM +0300, Iuliana Prodan (OSS) wrote:
From: Iuliana Prodan <iuliana.prodan@xxxxxxx>
subject: remoteproc: imx_dsp_rproc: add handle_rsc callback to handle DSP-specific features
Some DSP firmware requires a FW_READY signal before proceeding, while
others do not.
Therefore, add support to handle i.MX DSP-specific features.
Add support to handle i.MX DSP-specific features because Some DSP firmware
requires a FW_READY signal before proceeding
Implement handle_rsc callback to handle resource table parsing and to
process DSP-specific resource, to determine if waiting is needed.
Implement the handle_rsc callback to parse the resource table and process
DSP-specific resources to determine if waiting is needed.
Update imx_dsp_rproc_start() to handle this condition accordingly.
Signed-off-by: Iuliana Prodan <iuliana.prodan@xxxxxxx>
---
Changes in v3:
- Reviews from Mathieu Poirier:
- Added version and magic number to vendor-specific resource table entry.
- Updated defines to maintain backward compatibility with a resource table that doesn't have a vendor-specific resource.
- By default, wait for `fw_ready`, unless specified otherwise.
- Link to v2: https://lore.kernel.org/all/20250318215007.2109726-1-iuliana.prodan@xxxxxxxxxxx
Changes in v2:
- Reviews from Mathieu Poirier:
- Use vendor-specific resource table entry.
- Implement resource handler specific to the i.MX DSP.
- Revise commit message to include recent updates.
- Link to v1: https://lore.kernel.org/all/20250305123923.514386-1-iuliana.prodan@xxxxxxxxxxx/
drivers/remoteproc/imx_dsp_rproc.c | 102 ++++++++++++++++++++++++++++-
1 file changed, 100 insertions(+), 2 deletions(-)
diff --git a/drivers/remoteproc/imx_dsp_rproc.c b/drivers/remoteproc/imx_dsp_rproc.c
index b9bb15970966..80d4470cc731 100644
--- a/drivers/remoteproc/imx_dsp_rproc.c
+++ b/drivers/remoteproc/imx_dsp_rproc.c
@@ -35,9 +35,17 @@ module_param_named(no_mailboxes, no_mailboxes, int, 0644);
MODULE_PARM_DESC(no_mailboxes,
"There is no mailbox between cores, so ignore remote proc reply after start, default is 0 (off).");
+/* Flag indicating that the remote is up and running */
#define REMOTE_IS_READY BIT(0)
+/* Flag indicating that the host should wait for a firmware-ready response */
+#define WAIT_FW_READY BIT(1)
#define REMOTE_READY_WAIT_MAX_RETRIES 500
+/* This flag is set in the DSP resource table's features field to indicate
+ * that the firmware requires the host NOT to wait for a FW_READY response.
+ */
multi line comments should be
/*
* This ..
*/
+#define FEATURE_DONT_WAIT_FW_READY BIT(0)
+
/* att flags */
/* DSP own area */
#define ATT_OWN BIT(31)
@@ -72,6 +80,10 @@ MODULE_PARM_DESC(no_mailboxes,
#define IMX8ULP_SIP_HIFI_XRDC 0xc200000e
+#define FW_RSC_NXP_S_MAGIC ((uint32_t)'n' << 24 | \
+ (uint32_t)'x' << 16 | \
+ (uint32_t)'p' << 8 | \
+ (uint32_t)'s')
/*
* enum - Predefined Mailbox Messages
*
@@ -136,6 +148,24 @@ struct imx_dsp_rproc_dcfg {
int (*reset)(struct imx_dsp_rproc *priv);
};
+/**
+ * struct fw_rsc_imx_dsp - i.MX DSP specific info
+ *
+ * @len: length of the resource entry
+ * @magic_num: 32-bit magic number
+ * @version: version of data structure
+ * @features: feature flags supported by the i.MX DSP firmware
+ *
+ * This represents a DSP-specific resource in the firmware's
+ * resource table, providing information on supported features.
+ */
+struct fw_rsc_imx_dsp {
+ uint32_t len;
+ uint32_t magic_num;
+ uint32_t version;
+ uint32_t features;
+} __packed;
+
static const struct imx_rproc_att imx_dsp_rproc_att_imx8qm[] = {
/* dev addr , sys addr , size , flags */
{ 0x596e8000, 0x556e8000, 0x00008000, ATT_OWN },
@@ -300,6 +330,73 @@ static int imx_dsp_rproc_ready(struct rproc *rproc)
return -ETIMEDOUT;
}
+/**
+ * imx_dsp_rproc_handle_rsc() - Handle DSP-specific resource table entries
+ * @rproc: remote processor instance
+ * @rsc_type: resource type identifier
+ * @rsc: pointer to the resource entry
+ * @offset: offset of the resource entry
+ * @avail: available space in the resource table
+ *
+ * Parse the DSP-specific resource entry and update flags accordingly.
+ * If the WAIT_FW_READY feature is set, the host must wait for the firmware
+ * to signal readiness before proceeding with execution.
+ *
+ * Return: RSC_HANDLED if processed successfully, RSC_IGNORED otherwise.
+ */
+static int imx_dsp_rproc_handle_rsc(struct rproc *rproc, u32 rsc_type,
+ void *rsc, int offset, int avail)
+{
+ struct imx_dsp_rproc *priv = rproc->priv;
+ struct fw_rsc_imx_dsp *imx_dsp_rsc = rsc;
+ struct device *dev = rproc->dev.parent;
+ size_t expected_size;
+
put
priv->flags |= WAIT_FW_READY;
here,
all "goto ignored" can be replace with "return RSC_IGNORED"
Thanks for reviewing.
I'll hold off for a couple of days before sending v4.
Iulia
+ if (!imx_dsp_rsc) {
+ dev_dbg(dev, "Invalid fw_rsc_imx_dsp.\n");
+ goto ignored;
+ }
+
+ /* Make sure resource isn't truncated */
+ expected_size = imx_dsp_rsc->len + sizeof(imx_dsp_rsc->len);
+ if (expected_size < sizeof(struct fw_rsc_imx_dsp)) {
+ dev_dbg(dev, "Resource fw_rsc_imx_dsp is truncated.\n");
+ goto ignored;
+ }
+
+ /*
+ * If FW_RSC_NXP_S_MAGIC number is not found then
+ * wait for fw_ready reply (default work flow)
+ */
+ if (imx_dsp_rsc->magic_num != FW_RSC_NXP_S_MAGIC) {
+ dev_dbg(dev, "Invalid resource table magic number.\n");
+ goto ignored;
+ }
+
+ /*
+ * For now, in struct fw_rsc_imx_dsp, version 0,
+ * only FEATURE_DONT_WAIT_FW_READY is valid.
+ *
+ * When adding new features, please upgrade version.
+ */
+ if (imx_dsp_rsc->version > 0) {
+ dev_warn(dev, "Unexpected fw_rsc_imx_dsp version %d.\n",
+ imx_dsp_rsc->version);
+ goto ignored;
+ }
+
+ if (imx_dsp_rsc->features & FEATURE_DONT_WAIT_FW_READY)
+ priv->flags &= ~WAIT_FW_READY;
+ else
+ priv->flags |= WAIT_FW_READY;
if set WAIT_FW_READY at beginning, else branch can be removed.
Frank
+
+ return RSC_HANDLED;
+
+ignored:
+ priv->flags |= WAIT_FW_READY;
+ return RSC_IGNORED;
+}
+
/*
* Start function for rproc_ops
*
@@ -335,8 +432,8 @@ static int imx_dsp_rproc_start(struct rproc *rproc)
if (ret)
dev_err(dev, "Failed to enable remote core!\n");
- else
- ret = imx_dsp_rproc_ready(rproc);
+ else if (priv->flags & WAIT_FW_READY)
+ return imx_dsp_rproc_ready(rproc);
return ret;
}
@@ -936,6 +1033,7 @@ static const struct rproc_ops imx_dsp_rproc_ops = {
.kick = imx_dsp_rproc_kick,
.load = imx_dsp_rproc_elf_load_segments,
.parse_fw = imx_dsp_rproc_parse_fw,
+ .handle_rsc = imx_dsp_rproc_handle_rsc,
.find_loaded_rsc_table = rproc_elf_find_loaded_rsc_table,
.sanity_check = rproc_elf_sanity_check,
.get_boot_addr = rproc_elf_get_boot_addr,
--
2.25.1