component: module_adapter: Move module_ext_init_decode()-call earlier

Move module_ext_init_decode()-call earlier in call chain so that
struct ipc4_module_init_ext_obj_dp_data is available when
module_adapter_mem_alloc() is called. That is, if the struct
ipc4_module_init_ext_object is present in the struct
ipc4_module_init_ext_init payload.

To accomplish this I needed to redesign how module_ext_init_decode()
works and how its called a bit.

Signed-off-by: Jyri Sarha <[email protected]>

Author: Jyri Sarha

src/audio/module_adapter/module_adapter.c

@@ -187,7 +187,7 @@ static void module_adapter_mem_free(struct processing_module *mod)
  * \brief Create a module adapter component.
  * \param[in] drv - component driver pointer.
  * \param[in] config - component ipc descriptor pointer.
- * \param[in] spec - passdowned data from driver.
+ * \param[in] const_spec - passdowned data from driver.
  * \param[in] mod_priv - Pointer to private data for processing module.
  *
  * \return: a pointer to newly created module adapter component on success. NULL on error.
@@ -196,20 +196,32 @@ static void module_adapter_mem_free(struct processing_module *mod)
  *	 the create method.
  */
 struct comp_dev *module_adapter_new_ext(const struct comp_driver *drv,
-					const struct comp_ipc_config *config, const void *spec,
-					void *mod_priv, struct userspace_context *user_ctx)
+					const struct comp_ipc_config *config,
+					const void *const_spec, void *mod_priv,
+					struct userspace_context *user_ctx)
 {
 	int ret;
 	struct module_config *dst;
 	const struct module_interface *const interface = drv->adapter_ops;
-
+	struct ipc_config_process spec =
+		*((const struct ipc_config_process *) const_spec);
+#if CONFIG_IPC_MAJOR_4
+	struct module_ext_init_data ext_data = { 0 };
+#endif
 	comp_cl_dbg(drv, "start");
 
 	if (!config) {
 		comp_cl_err(drv, "wrong input params! drv = %zx config = %zx",
 			    (size_t)drv, (size_t)config);
 		return NULL;
 	}
+#if CONFIG_IPC_MAJOR_4
+	if (config->ipc_extended_init) {
+		ret = module_ext_init_decode(drv, &ext_data, &spec);
+		if (ret != 0)
+			return NULL;
+	}
+#endif
 
 	struct processing_module *mod = module_adapter_mem_alloc(drv, config);
 
@@ -233,7 +245,17 @@ struct comp_dev *module_adapter_new_ext(const struct comp_driver *drv,
 #endif /* CONFIG_USERSPACE */
 
 	dst = &mod->priv.cfg;
-	ret = module_adapter_init_data(dev, dst, config, spec);
+	/*
+	 * NOTE: dst->ext_data points to stack variable and contains
+	 *       pointers to IPC payload mailbox, so its only valid in
+	 *       functions that called from this function. This why
+	 *       the pointer is set NULL before the this function
+	 *       exits.
+	 */
+#if CONFIG_IPC_MAJOR_4
+	dst->ext_data = &ext_data;
+#endif
+	ret = module_adapter_init_data(dev, dst, config, &spec);
 	if (ret) {
 		comp_err(dev, "%d: module init data failed",
 			 ret);
@@ -299,6 +321,9 @@ struct comp_dev *module_adapter_new_ext(const struct comp_driver *drv,
 	component_set_nearest_period_frames(dev, params.rate);
 #endif
 
+#if CONFIG_IPC_MAJOR_4
+	dst->ext_data = NULL;
+#endif
 	comp_dbg(dev, "done");
 	return dev;
 
@@ -308,6 +333,9 @@ struct comp_dev *module_adapter_new_ext(const struct comp_driver *drv,
 		schedule_task_free(dev->task);
 #endif
 	module_adapter_mem_free(mod);
+#if CONFIG_IPC_MAJOR_4
+	dst->ext_data = NULL;
+#endif
 
 	return NULL;
 }

src/audio/module_adapter/module_adapter_ipc4.c

@@ -25,19 +25,19 @@
 
 LOG_MODULE_DECLARE(module_adapter, CONFIG_SOF_LOG_LEVEL);
 
-static const struct ipc4_base_module_extended_cfg *
-module_ext_init_decode(struct comp_dev *dev, struct module_config *dst,
-		       const unsigned char *data, size_t *size)
+int module_ext_init_decode(const struct comp_driver *drv, struct module_ext_init_data *ext_data,
+			   struct ipc_config_process *spec)
 {
 	const struct ipc4_module_init_ext_init *ext_init =
-		(const struct ipc4_module_init_ext_init *)data;
+		(const struct ipc4_module_init_ext_init *)spec->data;
 	bool last_object = !ext_init->data_obj_array;
 	const struct ipc4_module_init_ext_object *obj;
 
-	if (*size < sizeof(ext_init)) {
-		comp_err(dev, "Size too small for ext init %zu < %zu",
-			 *size, sizeof(ext_init));
-		return NULL;
+	assert(drv->type == SOF_COMP_MODULE_ADAPTER);
+	if (spec->size < sizeof(ext_init)) {
+		comp_cl_err(drv, "Size too small for ext init %zu < %zu",
+			    spec->size, sizeof(ext_init));
+		return -EINVAL;
 	}
 	/* TODO: Handle ext_init->gna_used and ext_init->rtos_domain here */
 	/* Get the first obj struct right after ext_init struct */
@@ -46,18 +46,18 @@ module_ext_init_decode(struct comp_dev *dev, struct module_config *dst,
 		const struct ipc4_module_init_ext_object *next_obj;
 
 		/* Check if there is space for the object header */
-		if ((unsigned char *)(obj + 1) - data > *size) {
-			comp_err(dev, "ext init obj overflow, %u > %zu",
-				 (unsigned char *)(obj + 1) - data, *size);
-			return NULL;
+		if ((unsigned char *)(obj + 1) - spec->data > spec->size) {
+			comp_cl_err(drv, "ext init obj overflow, %u > %zu",
+				    (unsigned char *)(obj + 1) - spec->data, spec->size);
+			return -EINVAL;
 		}
 		/* Calculate would be next object position and check if current object fits */
 		next_obj = (const struct ipc4_module_init_ext_object *)
 			(((uint32_t *) (obj + 1)) + obj->object_words);
-		if ((unsigned char *)next_obj - data > *size) {
-			comp_err(dev, "ext init object array overflow, %u > %zu",
-				 (unsigned char *)obj - data, *size);
-			return NULL;
+		if ((unsigned char *)next_obj - spec->data > spec->size) {
+			comp_cl_err(drv, "ext init object array overflow, %u > %zu",
+				    (unsigned char *)obj - spec->data, spec->size);
+			return -EINVAL;
 		}
 		switch (obj->object_id) {
 		case IPC4_MOD_INIT_DATA_ID_DP_DATA:
@@ -67,37 +67,34 @@ module_ext_init_decode(struct comp_dev *dev, struct module_config *dst,
 				(const struct ipc4_module_init_ext_obj_dp_data *)(obj + 1);
 
 			if (obj->object_words * sizeof(uint32_t) < sizeof(*dp_data)) {
-				comp_warn(dev, "dp_data object too small %zu < %zu",
-					  obj->object_words * sizeof(uint32_t), sizeof(*dp_data));
+				comp_cl_warn(drv, "dp_data object too small %zu < %zu",
+					     obj->object_words * sizeof(uint32_t),
+					     sizeof(*dp_data));
 				break;
 			}
-			dst->domain_id = dp_data->domain_id;
-			dst->stack_bytes = dp_data->stack_bytes;
-			dst->interim_heap_bytes = dp_data->interim_heap_bytes;
-			dst->lifetime_heap_bytes = dp_data->interim_heap_bytes;
-			dst->shared_bytes = dp_data->shared_bytes;
-			comp_info(dev,
-				  "init_ext_obj_dp_data domain %u stack %u interim %u lifetime %u shared %u",
-				  dp_data->domain_id, dp_data->stack_bytes,
-				  dp_data->interim_heap_bytes, dp_data->lifetime_heap_bytes,
-				  dp_data->shared_bytes);
+			ext_data->dp_data = dp_data;
+			comp_cl_info(drv,
+				     "init_ext_obj_dp_data domain %u stack %u interim %u lifetime %u shared %u",
+				     dp_data->domain_id, dp_data->stack_bytes,
+				     dp_data->interim_heap_bytes, dp_data->lifetime_heap_bytes,
+				     dp_data->shared_bytes);
 			break;
 		}
 		default:
-			comp_info(dev, "Unknown ext init object id %u of %u words",
-				  obj->object_id, obj->object_words);
+			comp_cl_info(drv, "Unknown ext init object id %u of %u words",
+				     obj->object_id, obj->object_words);
 		}
 		/* Read the last object flag from obj header */
 		last_object = obj->last_object;
 		/* Move to next object */
 		obj = next_obj;
 	}
 
-	/* Remove decoded ext_init payload from the size */
-	*size -= (unsigned char *) obj - data;
+	/* Remove decoded ext_init payload from spec */
+	spec->size -= (unsigned char *)obj - spec->data;
+	spec->data = (const unsigned char *)obj;
 
-	/* return remaining payload */
-	return (const struct ipc4_base_module_extended_cfg *)obj;
+	return 0;
 }
 
 /*
@@ -119,10 +116,7 @@ int module_adapter_init_data(struct comp_dev *dev,
 	size_t cfgsz = args->size;
 
 	assert(dev->drv->type == SOF_COMP_MODULE_ADAPTER);
-	if (config->ipc_extended_init)
-		cfg = module_ext_init_decode(dev, dst, args->data, &cfgsz);
-	else
-		cfg = (const struct ipc4_base_module_extended_cfg *)args->data;
+	cfg = (const struct ipc4_base_module_extended_cfg *)args->data;
 
 	if (cfg == NULL)
 		return -EINVAL;

src/include/module/module/base.h

@@ -23,6 +23,18 @@
 #define module_get_private_data(mod) ((mod)->priv.private)
 #define module_set_private_data(mod, data) ((mod)->priv.private = data)
 
+/**
+ * \struct module_ext_init_data
+ * \brief Container for found ext init object pointers
+ * This struct contains pointers that point to IPC payload directly. The
+ * module should store what it needs in its init() callback as the data
+ * is not valid after that.
+ */
+struct ipc4_module_init_ext_obj_dp_data;
+struct module_ext_init_data {
+	const struct ipc4_module_init_ext_obj_dp_data *dp_data;
+};
+
 /**
  * \struct module_config
  * \brief Module config container, used for both config types.
@@ -38,11 +50,7 @@ struct module_config {
 	uint8_t nb_output_pins;
 	struct ipc4_input_pin_format *input_pins;
 	struct ipc4_output_pin_format *output_pins;
-	uint32_t domain_id;		/* userspace domain ID */
-	uint32_t stack_bytes;		/* stack size in bytes */
-	uint32_t interim_heap_bytes;	/* interim heap size in bytes */
-	uint32_t lifetime_heap_bytes;	/* lifetime heap size in bytes */
-	uint32_t shared_bytes;		/* shared size in bytes */
+	struct module_ext_init_data *ext_data; /**< IPC payload pointers, NULL after init() */
 #endif
 };
 

src/include/sof/audio/module_adapter/module/generic.h

@@ -431,7 +431,18 @@ int module_adapter_ts_get_op(struct comp_dev *dev, struct timestamp_data *tsd);
 void module_update_buffer_position(struct input_stream_buffer *input_buffers,
 				   struct output_stream_buffer *output_buffers,
 				   uint32_t frames);
-
+struct module_ext_init_data;
+#if CONFIG_IPC_MAJOR_4
+int module_ext_init_decode(const struct comp_driver *drv, struct module_ext_init_data *ext_data,
+			   struct ipc_config_process *spec);
+#else
+static inline
+int module_ext_init_decode(const struct comp_driver *drv, struct module_ext_init_data *ext_data,
+			   struct ipc_config_process *spec)
+{
+	return 0;
+}
+#endif
 int module_adapter_init_data(struct comp_dev *dev,
 			     struct module_config *dst,
 			     const struct comp_ipc_config *config,