[PATCH 0/4] ASoC: SOF: ipc4: Add support for control change notification

[PATCH 0/4] ASoC: SOF: ipc4: Add support for control change notification

[Peter Ujfalusi]

Hi,

This series adds support for handling control (switch/enum) change notifications
sent by the firmware.
The use case is similar to what is already used by IPC3 version: the firmware
can update the value of an enum or switch and sends notification to the kernel,
which in turn will notify the user space of a change.

Regards,
Peter
---
Peter Ujfalusi (4):
  ASoC: SOF: ipc4-topology: Helper to find an swidget by module/instance
    id
  ASoC: SOF: ipc4: Add data struct for module notification message from
    firmware
  ASoC: SOF: ipc4-control: Implement control update for switch/enum
    controls
  ASoC: SOF: ipc4: Handle ALSA kcontrol change notification from
    firmware

 include/sound/sof/ipc4/header.h |  29 ++++++
 sound/soc/sof/ipc4-control.c    | 179 ++++++++++++++++++++++++++++++++
 sound/soc/sof/ipc4-priv.h       |   3 +
 sound/soc/sof/ipc4-topology.c   |  20 ++++
 sound/soc/sof/ipc4.c            |  57 ++++++++++
 5 files changed, 288 insertions(+)

-- 
2.42.1

[PATCH 1/4] ASoC: SOF: ipc4-topology: Helper to find an swidget by module/instance id

[Peter Ujfalusi]

The sof_ipc4_find_swidget_by_ids() can be used to find the swidget of a
module instance.
The lookup parameters are the module_id and the instance_id.

Signed-off-by: Peter Ujfalusi <peter.ujfalusi@linux.intel.com>
Reviewed-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
Reviewed-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
---
 sound/soc/sof/ipc4-priv.h     |  3 +++
 sound/soc/sof/ipc4-topology.c | 20 ++++++++++++++++++++
 2 files changed, 23 insertions(+)

diff --git a/sound/soc/sof/ipc4-priv.h b/sound/soc/sof/ipc4-priv.h
index 9e69b7c29117..fea93b693f4d 100644
--- a/sound/soc/sof/ipc4-priv.h
+++ b/sound/soc/sof/ipc4-priv.h
@@ -115,6 +115,9 @@ int sof_ipc4_reload_fw_libraries(struct snd_sof_dev *sdev);
 struct sof_ipc4_fw_module *sof_ipc4_find_module_by_uuid(struct snd_sof_dev *sdev,
 							const guid_t *uuid);
 
+struct snd_sof_widget *sof_ipc4_find_swidget_by_ids(struct snd_sof_dev *sdev,
+						    u32 module_id, int instance_id);
+
 struct sof_ipc4_base_module_cfg;
 void sof_ipc4_update_cpc_from_manifest(struct snd_sof_dev *sdev,
 				       struct sof_ipc4_fw_module *fw_module,
diff --git a/sound/soc/sof/ipc4-topology.c b/sound/soc/sof/ipc4-topology.c
index b24a64377f68..8ab9c42069ee 100644
--- a/sound/soc/sof/ipc4-topology.c
+++ b/sound/soc/sof/ipc4-topology.c
@@ -167,6 +167,26 @@ static const struct sof_token_info ipc4_token_list[SOF_TOKEN_COUNT] = {
 	[SOF_SRC_TOKENS] = {"SRC tokens", src_tokens, ARRAY_SIZE(src_tokens)},
 };
 
+struct snd_sof_widget *sof_ipc4_find_swidget_by_ids(struct snd_sof_dev *sdev,
+						    u32 module_id, int instance_id)
+{
+	struct snd_sof_widget *swidget;
+
+	list_for_each_entry(swidget, &sdev->widget_list, list) {
+		struct sof_ipc4_fw_module *fw_module = swidget->module_info;
+
+		/* Only active module instances have valid instance_id */
+		if (!swidget->use_count)
+			continue;
+
+		if (fw_module && fw_module->man4_module_entry.id == module_id &&
+		    swidget->instance_id == instance_id)
+			return swidget;
+	}
+
+	return NULL;
+}
+
 static void sof_ipc4_dbg_audio_format(struct device *dev, struct sof_ipc4_pin_format *pin_fmt,
 				      int num_formats)
 {
-- 
2.42.1

[PATCH 2/4] ASoC: SOF: ipc4: Add data struct for module notification message from firmware

[Peter Ujfalusi]

With the module notification message the information about the notification
is provided via the mailbox with the sof_ipc4_notify_module_data struct.

It contains the module and instance id of the sender of the notification,
the event_id and optionally additional data which is module and event
specific.

At the same time add definitions to identify ALSA kcontrol change
notification.
These notifications use standardized event_id, modules must follow this if
they support such notifications:
upper 16 bit: 0xA15A as a magic identification value
lower 16 bit: param_id of the changed control

Signed-off-by: Peter Ujfalusi <peter.ujfalusi@linux.intel.com>
Reviewed-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
Reviewed-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
---
 include/sound/sof/ipc4/header.h | 29 +++++++++++++++++++++++++++++
 1 file changed, 29 insertions(+)

diff --git a/include/sound/sof/ipc4/header.h b/include/sound/sof/ipc4/header.h
index 574a9d581f88..2c81d6dde577 100644
--- a/include/sound/sof/ipc4/header.h
+++ b/include/sound/sof/ipc4/header.h
@@ -532,6 +532,35 @@ struct sof_ipc4_notify_resource_data {
 #define SOF_IPC4_DEBUG_SLOT_TELEMETRY		0x4c455400
 #define SOF_IPC4_DEBUG_SLOT_BROKEN		0x44414544
 
+/**
+ * struct sof_ipc4_notify_module_data - payload for module notification
+ * @instance_id: instance ID of the originator module of the notification
+ * @module_id: module ID of the originator of the notification
+ * @event_id: module specific event id
+ * @event_data_size: Size of the @event_data (if any) in bytes
+ * @event_data: Optional notification data, module and notification dependent
+ */
+struct sof_ipc4_notify_module_data {
+	uint16_t instance_id;
+	uint16_t module_id;
+	uint32_t event_id;
+	uint32_t event_data_size;
+	uint8_t event_data[];
+} __packed __aligned(4);
+
+/*
+ * ALSA kcontrol change notification
+ *
+ * The event_id of struct sof_ipc4_notify_module_data is divided into two u16:
+ *  upper u16: magic number for ALSA kcontrol types: 0xA15A
+ *  lower u16: param_id of the control, which is the type of the control
+ * The event_data contains the struct sof_ipc4_control_msg_payload of the control
+ * which sent the notification.
+ */
+#define SOF_IPC4_NOTIFY_MODULE_EVENTID_ALSA_MAGIC_MASK		GENMASK(31, 16)
+#define SOF_IPC4_NOTIFY_MODULE_EVENTID_ALSA_MAGIC_VAL		0xA15A0000
+#define SOF_IPC4_NOTIFY_MODULE_EVENTID_ALSA_PARAMID_MASK	GENMASK(15, 0)
+
 /** @}*/
 
 #endif
-- 
2.42.1

[PATCH 3/4] ASoC: SOF: ipc4-control: Implement control update for switch/enum controls

[Peter Ujfalusi]

Implement the sof_ipc_tplg_control_ops.update function to support a control
change notification from the firmware on switch or enum control types.

Based on the module notification message content, look up the swidget, then
the scontrol which was the source of the notification then if the message
contains the changed values update the cached values.
If only a notification without values received, marked the control as dirty
and on next read access fetch the new values from the firmware.

Signed-off-by: Peter Ujfalusi <peter.ujfalusi@linux.intel.com>
Reviewed-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
Reviewed-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
---
 sound/soc/sof/ipc4-control.c | 179 +++++++++++++++++++++++++++++++++++
 1 file changed, 179 insertions(+)

diff --git a/sound/soc/sof/ipc4-control.c b/sound/soc/sof/ipc4-control.c
index 938efaceb81c..3d2a35f27a87 100644
--- a/sound/soc/sof/ipc4-control.c
+++ b/sound/soc/sof/ipc4-control.c
@@ -240,6 +240,50 @@ sof_ipc4_set_generic_control_data(struct snd_sof_dev *sdev,
 	return ret;
 }
 
+static void sof_ipc4_refresh_generic_control(struct snd_sof_control *scontrol)
+{
+	struct sof_ipc4_control_data *cdata = scontrol->ipc_control_data;
+	struct snd_soc_component *scomp = scontrol->scomp;
+	struct sof_ipc4_control_msg_payload *data;
+	struct sof_ipc4_msg *msg = &cdata->msg;
+	size_t data_size;
+	unsigned int i;
+	int ret;
+
+	if (!scontrol->comp_data_dirty)
+		return;
+
+	if (!pm_runtime_active(scomp->dev))
+		return;
+
+	data_size = struct_size(data, chanv, scontrol->num_channels);
+	data = kmalloc(data_size, GFP_KERNEL);
+	if (!data)
+		return;
+
+	data->id = cdata->index;
+	data->num_elems = scontrol->num_channels;
+	msg->data_ptr = data;
+	msg->data_size = data_size;
+
+	scontrol->comp_data_dirty = false;
+	ret = sof_ipc4_set_get_kcontrol_data(scontrol, false, true);
+	msg->data_ptr = NULL;
+	msg->data_size = 0;
+	if (!ret) {
+		for (i = 0; i < scontrol->num_channels; i++) {
+			cdata->chanv[i].channel = data->chanv[i].channel;
+			cdata->chanv[i].value = data->chanv[i].value;
+		}
+	} else {
+		dev_err(scomp->dev, "Failed to read control data for %s\n",
+			scontrol->name);
+		scontrol->comp_data_dirty = true;
+	}
+
+	kfree(data);
+}
+
 static bool sof_ipc4_switch_put(struct snd_sof_control *scontrol,
 				struct snd_ctl_elem_value *ucontrol)
 {
@@ -290,6 +334,8 @@ static int sof_ipc4_switch_get(struct snd_sof_control *scontrol,
 	struct sof_ipc4_control_data *cdata = scontrol->ipc_control_data;
 	unsigned int i;
 
+	sof_ipc4_refresh_generic_control(scontrol);
+
 	/* read back each channel */
 	for (i = 0; i < scontrol->num_channels; i++)
 		ucontrol->value.integer.value[i] = cdata->chanv[i].value;
@@ -347,6 +393,8 @@ static int sof_ipc4_enum_get(struct snd_sof_control *scontrol,
 	struct sof_ipc4_control_data *cdata = scontrol->ipc_control_data;
 	unsigned int i;
 
+	sof_ipc4_refresh_generic_control(scontrol);
+
 	/* read back each channel */
 	for (i = 0; i < scontrol->num_channels; i++)
 		ucontrol->value.enumerated.item[i] = cdata->chanv[i].value;
@@ -601,6 +649,136 @@ sof_ipc4_volsw_setup(struct snd_sof_dev *sdev, struct snd_sof_widget *swidget,
 	return sof_ipc4_set_volume_data(sdev, swidget, scontrol, false);
 }
 
+#define PARAM_ID_FROM_EXTENSION(_ext)	(((_ext) & SOF_IPC4_MOD_EXT_MSG_PARAM_ID_MASK)	\
+					 >> SOF_IPC4_MOD_EXT_MSG_PARAM_ID_SHIFT)
+
+static void sof_ipc4_control_update(struct snd_sof_dev *sdev, void *ipc_message)
+{
+	struct sof_ipc4_msg *ipc4_msg = ipc_message;
+	struct sof_ipc4_notify_module_data *ndata = ipc4_msg->data_ptr;
+	struct sof_ipc4_control_msg_payload *msg_data;
+	struct sof_ipc4_control_data *cdata;
+	struct snd_soc_dapm_widget *widget;
+	struct snd_sof_control *scontrol;
+	struct snd_sof_widget *swidget;
+	struct snd_kcontrol *kc = NULL;
+	bool scontrol_found = false;
+	u32 event_param_id;
+	int i, type;
+
+	if (ndata->event_data_size < sizeof(*msg_data)) {
+		dev_err(sdev->dev,
+			"%s: Invalid event data size for module %u.%u: %u\n",
+			__func__, ndata->module_id, ndata->instance_id,
+			ndata->event_data_size);
+		return;
+	}
+
+	event_param_id = ndata->event_id & SOF_IPC4_NOTIFY_MODULE_EVENTID_ALSA_PARAMID_MASK;
+	switch (event_param_id) {
+	case SOF_IPC4_SWITCH_CONTROL_PARAM_ID:
+		type = SND_SOC_TPLG_TYPE_MIXER;
+		break;
+	case SOF_IPC4_ENUM_CONTROL_PARAM_ID:
+		type = SND_SOC_TPLG_TYPE_ENUM;
+		break;
+	default:
+		dev_err(sdev->dev,
+			"%s: Invalid control type for module %u.%u: %u\n",
+			__func__, ndata->module_id, ndata->instance_id,
+			event_param_id);
+		return;
+	}
+
+	/* Find the swidget based on ndata->module_id and ndata->instance_id */
+	swidget = sof_ipc4_find_swidget_by_ids(sdev, ndata->module_id,
+					       ndata->instance_id);
+	if (!swidget) {
+		dev_err(sdev->dev, "%s: Failed to find widget for module %u.%u\n",
+			__func__, ndata->module_id, ndata->instance_id);
+		return;
+	}
+
+	/* Find the scontrol which is the source of the notification */
+	msg_data = (struct sof_ipc4_control_msg_payload *)ndata->event_data;
+	list_for_each_entry(scontrol, &sdev->kcontrol_list, list) {
+		if (scontrol->comp_id == swidget->comp_id) {
+			u32 local_param_id;
+
+			cdata = scontrol->ipc_control_data;
+			/*
+			 * The scontrol's param_id is stored in the IPC message
+			 * template's extension
+			 */
+			local_param_id = PARAM_ID_FROM_EXTENSION(cdata->msg.extension);
+			if (local_param_id == event_param_id &&
+			    msg_data->id == cdata->index) {
+				scontrol_found = true;
+				break;
+			}
+		}
+	}
+
+	if (!scontrol_found) {
+		dev_err(sdev->dev,
+			"%s: Failed to find control on widget %s: %u:%u\n",
+			__func__, swidget->widget->name, ndata->event_id & 0xffff,
+			msg_data->id);
+		return;
+	}
+
+	if (msg_data->num_elems) {
+		/*
+		 * The message includes the updated value/data, update the
+		 * control's local cache using the received notification
+		 */
+		for (i = 0; i < msg_data->num_elems; i++) {
+			u32 channel = msg_data->chanv[i].channel;
+
+			if (channel >= scontrol->num_channels) {
+				dev_warn(sdev->dev,
+					 "Invalid channel index for %s: %u\n",
+					 scontrol->name, i);
+
+				/*
+				 * Mark the scontrol as dirty to force a refresh
+				 * on next read
+				 */
+				scontrol->comp_data_dirty = true;
+				break;
+			}
+
+			cdata->chanv[channel].value = msg_data->chanv[i].value;
+		}
+	} else {
+		/*
+		 * Mark the scontrol as dirty because the value/data is changed
+		 * in firmware, forcing a refresh on next read access
+		 */
+		scontrol->comp_data_dirty = true;
+	}
+
+	/*
+	 * Look up the ALSA kcontrol of the scontrol to be able to send a
+	 * notification to user space
+	 */
+	widget = swidget->widget;
+	for (i = 0; i < widget->num_kcontrols; i++) {
+		/* skip non matching types or non matching indexes within type */
+		if (widget->dobj.widget.kcontrol_type[i] == type &&
+		    widget->kcontrol_news[i].index == cdata->index) {
+			kc = widget->kcontrols[i];
+			break;
+		}
+	}
+
+	if (!kc)
+		return;
+
+	snd_ctl_notify_one(swidget->scomp->card->snd_card,
+			   SNDRV_CTL_EVENT_MASK_VALUE, kc, 0);
+}
+
 /* set up all controls for the widget */
 static int sof_ipc4_widget_kcontrol_setup(struct snd_sof_dev *sdev, struct snd_sof_widget *swidget)
 {
@@ -674,6 +852,7 @@ const struct sof_ipc_tplg_control_ops tplg_ipc4_control_ops = {
 	.bytes_ext_put = sof_ipc4_bytes_ext_put,
 	.bytes_ext_get = sof_ipc4_bytes_ext_get,
 	.bytes_ext_volatile_get = sof_ipc4_bytes_ext_volatile_get,
+	.update = sof_ipc4_control_update,
 	.widget_kcontrol_setup = sof_ipc4_widget_kcontrol_setup,
 	.set_up_volume_table = sof_ipc4_set_up_volume_table,
 };
-- 
2.42.1

[PATCH 4/4] ASoC: SOF: ipc4: Handle ALSA kcontrol change notification from firmware

[Peter Ujfalusi]

The control change notification is sent as module notification with a
standardized event_id (higher 16 bit is 0xA15A).
Add generic code to handle the module notification and invoke the control
update callback if the notification is an ALSA kcontrol change message.

Signed-off-by: Peter Ujfalusi <peter.ujfalusi@linux.intel.com>
Reviewed-by: Ranjani Sridharan <ranjani.sridharan@linux.intel.com>
Reviewed-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
---
 sound/soc/sof/ipc4.c | 57 ++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 57 insertions(+)

diff --git a/sound/soc/sof/ipc4.c b/sound/soc/sof/ipc4.c
index 8441f4ae4065..a9d9800d2fcc 100644
--- a/sound/soc/sof/ipc4.c
+++ b/sound/soc/sof/ipc4.c
@@ -78,6 +78,9 @@ static const struct sof_ipc4_fw_status {
 	{165, "Reserved (ADSP_IPC_PIPELINE_ALREADY_EXISTS removed)"},
 };
 
+typedef void (*ipc4_notification_handler)(struct snd_sof_dev *sdev,
+					  struct sof_ipc4_msg *msg);
+
 static int sof_ipc4_check_reply_status(struct snd_sof_dev *sdev, u32 status)
 {
 	int i, ret;
@@ -610,9 +613,55 @@ static int ipc4_fw_ready(struct snd_sof_dev *sdev, struct sof_ipc4_msg *ipc4_msg
 	return sof_ipc4_init_msg_memory(sdev);
 }
 
+static void sof_ipc4_module_notification_handler(struct snd_sof_dev *sdev,
+						 struct sof_ipc4_msg *ipc4_msg)
+{
+	struct sof_ipc4_notify_module_data *data = ipc4_msg->data_ptr;
+
+	/*
+	 * If the notification includes additional, module specific data, then
+	 * we need to re-allocate the buffer and re-read the whole payload,
+	 * including the event_data
+	 */
+	if (data->event_data_size) {
+		void *new;
+		int ret;
+
+		ipc4_msg->data_size += data->event_data_size;
+
+		new = krealloc(ipc4_msg->data_ptr, ipc4_msg->data_size, GFP_KERNEL);
+		if (!new) {
+			ipc4_msg->data_size -= data->event_data_size;
+			return;
+		}
+
+		/* re-read the whole payload */
+		ipc4_msg->data_ptr = new;
+		ret = snd_sof_ipc_msg_data(sdev, NULL, ipc4_msg->data_ptr,
+					   ipc4_msg->data_size);
+		if (ret < 0) {
+			dev_err(sdev->dev,
+				"Failed to read the full module notification: %d\n",
+				ret);
+			return;
+		}
+		data = ipc4_msg->data_ptr;
+	}
+
+	/* Handle ALSA kcontrol notification */
+	if ((data->event_id & SOF_IPC4_NOTIFY_MODULE_EVENTID_ALSA_MAGIC_MASK) ==
+	    SOF_IPC4_NOTIFY_MODULE_EVENTID_ALSA_MAGIC_VAL) {
+		const struct sof_ipc_tplg_ops *tplg_ops = sdev->ipc->ops->tplg;
+
+		if (tplg_ops->control->update)
+			tplg_ops->control->update(sdev, ipc4_msg);
+	}
+}
+
 static void sof_ipc4_rx_msg(struct snd_sof_dev *sdev)
 {
 	struct sof_ipc4_msg *ipc4_msg = sdev->ipc->msg.rx_data;
+	ipc4_notification_handler handler_func = NULL;
 	size_t data_size = 0;
 	int err;
 
@@ -648,6 +697,10 @@ static void sof_ipc4_rx_msg(struct snd_sof_dev *sdev)
 	case SOF_IPC4_NOTIFY_EXCEPTION_CAUGHT:
 		snd_sof_dsp_panic(sdev, 0, true);
 		break;
+	case SOF_IPC4_NOTIFY_MODULE_NOTIFICATION:
+		data_size = sizeof(struct sof_ipc4_notify_module_data);
+		handler_func = sof_ipc4_module_notification_handler;
+		break;
 	default:
 		dev_dbg(sdev->dev, "Unhandled DSP message: %#x|%#x\n",
 			ipc4_msg->primary, ipc4_msg->extension);
@@ -663,6 +716,10 @@ static void sof_ipc4_rx_msg(struct snd_sof_dev *sdev)
 		snd_sof_ipc_msg_data(sdev, NULL, ipc4_msg->data_ptr, ipc4_msg->data_size);
 	}
 
+	/* Handle notifications with payload */
+	if (handler_func)
+		handler_func(sdev, ipc4_msg);
+
 	sof_ipc4_log_header(sdev->dev, "ipc rx done ", ipc4_msg, true);
 
 	if (data_size) {
-- 
2.42.1

Re: [PATCH 0/4] ASoC: SOF: ipc4: Add support for control change notification

[Mark Brown]

On Fri, 24 Nov 2023 17:08:49 +0200, Peter Ujfalusi wrote:
> This series adds support for handling control (switch/enum) change notifications
> sent by the firmware.
> The use case is similar to what is already used by IPC3 version: the firmware
> can update the value of an enum or switch and sends notification to the kernel,
> which in turn will notify the user space of a change.
> 
> Regards,
> Peter
> 
> [...]

Applied to

   https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git for-next

Thanks!

[1/4] ASoC: SOF: ipc4-topology: Helper to find an swidget by module/instance id
      commit: 5980bda0a998a6ee6afd83b97a482a40c1c68076
[2/4] ASoC: SOF: ipc4: Add data struct for module notification message from firmware
      commit: 1a307538c9cc274b3191b9a1380bbceece262626
[3/4] ASoC: SOF: ipc4-control: Implement control update for switch/enum controls
      commit: f5eb9945cf9c17eb016aa64c7de13875f259ea07
[4/4] ASoC: SOF: ipc4: Handle ALSA kcontrol change notification from firmware
      commit: 0ff23d460718641c80c8054425256391dca1ac7d

All being well this means that it will be integrated into the linux-next
tree (usually sometime in the next 24 hours) and sent to Linus during
the next merge window (or sooner if it is a bug fix), however if
problems are discovered then the patch may be dropped or reverted.

You may get further e-mails resulting from automated or manual testing
and review of the tree, please engage with people reporting problems and
send followup patches addressing any issues that are reported if needed.

If any updates are required or you are submitting further changes they
should be sent as incremental updates against current git, existing
patches will not be replaced.

Please add any relevant lists and maintainers to the CCs when replying
to this mail.

Thanks,
Mark