All of lore.kernel.org
 help / color / mirror / Atom feed
From: Peter Ujfalusi <peter.ujfalusi@linux.intel.com>
To: lgirdwood@gmail.com, broonie@kernel.org
Cc: guennadi.liakhovetski@linux.intel.com,
	alsa-devel@alsa-project.org, seppo.ingalsuo@linux.intel.com,
	pierre-louis.bossart@linux.intel.com,
	kai.vehmanen@linux.intel.com
Subject: [PATCH] ASoC: SOF: Handle control change notification from firmware
Date: Thu,  2 Sep 2021 14:53:28 +0300	[thread overview]
Message-ID: <20210902115328.28478-1-peter.ujfalusi@linux.intel.com> (raw)

If the value/data associated with a control changes in SOF it will send a
notification (SOF_IPC_GLB_COMP_MSG with SOF_IPC_COMP_GET_VALUE/DATA).

We have support for binary volatile control type, but we might have
features where enum/switch/volume changes. Re-implementing everything as
volatile as well would be not much of a gain for several reasons:
- volatile controls would do an IPC all the time, regardless if there is a
  need or not.
- We still don't have notification which forces userspace to continuously
  poll.

When such notification arrives we use snd_ctl_notify_one() to signal
userspace about the change.

The kernel is prepared for two types of notification:
- the notification carries the new data for the control (num_elems != 0)
The new value/data is copied to the control's local data

- blank message about a change
The new flag for the scontrol (comp_data_dirty) is set and when next
time user space reads the value via the kcontrol's get callback we will
refresh the control's local data from the firmware.

Signed-off-by: Peter Ujfalusi <peter.ujfalusi@linux.intel.com>
Reviewed-by: Kai Vehmanen <kai.vehmanen@linux.intel.com>
Reviewed-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
Reviewed-by: Guennadi Liakhovetski <guennadi.liakhovetski@linux.intel.com>
Tested-by: Seppo Ingalsuo <seppo.ingalsuo@linux.intel.com>
---
 sound/soc/sof/control.c   | 186 ++++++++++++++++++++++++++++++++++++++
 sound/soc/sof/ipc.c       |  35 +++++++
 sound/soc/sof/sof-audio.h |   5 +
 3 files changed, 226 insertions(+)

diff --git a/sound/soc/sof/control.c b/sound/soc/sof/control.c
index 504500dd4d43..58bb89af4de1 100644
--- a/sound/soc/sof/control.c
+++ b/sound/soc/sof/control.c
@@ -65,6 +65,40 @@ static inline u32 ipc_to_mixer(u32 value, u32 *volume_map, int size)
 	return i - 1;
 }
 
+static void snd_sof_refresh_control(struct snd_sof_control *scontrol)
+{
+	struct sof_ipc_ctrl_data *cdata = scontrol->control_data;
+	struct snd_soc_component *scomp = scontrol->scomp;
+	enum sof_ipc_ctrl_type ctrl_type;
+	int ret;
+
+	if (!scontrol->comp_data_dirty)
+		return;
+
+	if (!pm_runtime_active(scomp->dev))
+		return;
+
+	if (scontrol->cmd == SOF_CTRL_CMD_BINARY)
+		ctrl_type = SOF_IPC_COMP_GET_DATA;
+	else
+		ctrl_type = SOF_IPC_COMP_GET_VALUE;
+
+	/* set the ABI header values */
+	cdata->data->magic = SOF_ABI_MAGIC;
+	cdata->data->abi = SOF_ABI_VERSION;
+
+	/* refresh the component data from DSP */
+	scontrol->comp_data_dirty = false;
+	ret = snd_sof_ipc_set_get_comp_data(scontrol, ctrl_type,
+					    SOF_CTRL_TYPE_VALUE_CHAN_GET,
+					    scontrol->cmd, false);
+	if (ret < 0) {
+		dev_err(scomp->dev, "error: failed to get control data: %d\n", ret);
+		/* Set the flag to re-try next time to get the data */
+		scontrol->comp_data_dirty = true;
+	}
+}
+
 int snd_sof_volume_get(struct snd_kcontrol *kcontrol,
 		       struct snd_ctl_elem_value *ucontrol)
 {
@@ -74,6 +108,8 @@ int snd_sof_volume_get(struct snd_kcontrol *kcontrol,
 	struct sof_ipc_ctrl_data *cdata = scontrol->control_data;
 	unsigned int i, channels = scontrol->num_channels;
 
+	snd_sof_refresh_control(scontrol);
+
 	/* read back each channel */
 	for (i = 0; i < channels; i++)
 		ucontrol->value.integer.value[i] =
@@ -145,6 +181,8 @@ int snd_sof_switch_get(struct snd_kcontrol *kcontrol,
 	struct sof_ipc_ctrl_data *cdata = scontrol->control_data;
 	unsigned int i, channels = scontrol->num_channels;
 
+	snd_sof_refresh_control(scontrol);
+
 	/* read back each channel */
 	for (i = 0; i < channels; i++)
 		ucontrol->value.integer.value[i] = cdata->chanv[i].value;
@@ -195,6 +233,8 @@ int snd_sof_enum_get(struct snd_kcontrol *kcontrol,
 	struct sof_ipc_ctrl_data *cdata = scontrol->control_data;
 	unsigned int i, channels = scontrol->num_channels;
 
+	snd_sof_refresh_control(scontrol);
+
 	/* read back each channel */
 	for (i = 0; i < channels; i++)
 		ucontrol->value.enumerated.item[i] = cdata->chanv[i].value;
@@ -244,6 +284,8 @@ int snd_sof_bytes_get(struct snd_kcontrol *kcontrol,
 	struct sof_abi_hdr *data = cdata->data;
 	size_t size;
 
+	snd_sof_refresh_control(scontrol);
+
 	if (be->max > sizeof(ucontrol->value.bytes.data)) {
 		dev_err_ratelimited(scomp->dev,
 				    "error: data max %d exceeds ucontrol data array size\n",
@@ -475,6 +517,8 @@ int snd_sof_bytes_ext_get(struct snd_kcontrol *kcontrol,
 		(struct snd_ctl_tlv __user *)binary_data;
 	size_t data_size;
 
+	snd_sof_refresh_control(scontrol);
+
 	/*
 	 * Decrement the limit by ext bytes header size to
 	 * ensure the user space buffer is not exceeded.
@@ -511,3 +555,145 @@ int snd_sof_bytes_ext_get(struct snd_kcontrol *kcontrol,
 
 	return 0;
 }
+
+static void snd_sof_update_control(struct snd_sof_control *scontrol,
+				   struct sof_ipc_ctrl_data *cdata)
+{
+	struct snd_soc_component *scomp = scontrol->scomp;
+	struct sof_ipc_ctrl_data *local_cdata;
+	int i;
+
+	local_cdata = scontrol->control_data;
+
+	if (cdata->cmd == SOF_CTRL_CMD_BINARY) {
+		if (cdata->num_elems != local_cdata->data->size) {
+			dev_err(scomp->dev,
+				"error: cdata binary size mismatch %u - %u\n",
+				cdata->num_elems, local_cdata->data->size);
+			return;
+		}
+
+		/* copy the new binary data */
+		memcpy(local_cdata->data, cdata->data, cdata->num_elems);
+	} else if (cdata->num_elems != scontrol->num_channels) {
+		dev_err(scomp->dev,
+			"error: cdata channel count mismatch %u - %d\n",
+			cdata->num_elems, scontrol->num_channels);
+	} else {
+		/* copy the new values */
+		for (i = 0; i < cdata->num_elems; i++)
+			local_cdata->chanv[i].value = cdata->chanv[i].value;
+	}
+}
+
+void snd_sof_control_notify(struct snd_sof_dev *sdev,
+			    struct sof_ipc_ctrl_data *cdata)
+{
+	struct snd_soc_dapm_widget *widget;
+	struct snd_sof_control *scontrol;
+	struct snd_sof_widget *swidget;
+	struct snd_kcontrol *kc = NULL;
+	struct soc_mixer_control *sm;
+	struct soc_bytes_ext *be;
+	size_t expected_size;
+	struct soc_enum *se;
+	bool found = false;
+	int i, type;
+
+	/* Find the swidget first */
+	list_for_each_entry(swidget, &sdev->widget_list, list) {
+		if (swidget->comp_id == cdata->comp_id) {
+			found = true;
+			break;
+		}
+	}
+
+	if (!found)
+		return;
+
+	/* Translate SOF cmd to TPLG type */
+	switch (cdata->cmd) {
+	case SOF_CTRL_CMD_VOLUME:
+	case SOF_CTRL_CMD_SWITCH:
+		type = SND_SOC_TPLG_TYPE_MIXER;
+		break;
+	case SOF_CTRL_CMD_BINARY:
+		type = SND_SOC_TPLG_TYPE_BYTES;
+		break;
+	case SOF_CTRL_CMD_ENUM:
+		type = SND_SOC_TPLG_TYPE_ENUM;
+		break;
+	default:
+		dev_err(sdev->dev, "error: unknown cmd %u\n", cdata->cmd);
+		return;
+	}
+
+	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;
+
+	switch (cdata->cmd) {
+	case SOF_CTRL_CMD_VOLUME:
+	case SOF_CTRL_CMD_SWITCH:
+		sm = (struct soc_mixer_control *)kc->private_value;
+		scontrol = sm->dobj.private;
+		break;
+	case SOF_CTRL_CMD_BINARY:
+		be = (struct soc_bytes_ext *)kc->private_value;
+		scontrol = be->dobj.private;
+		break;
+	case SOF_CTRL_CMD_ENUM:
+		se = (struct soc_enum *)kc->private_value;
+		scontrol = se->dobj.private;
+		break;
+	default:
+		return;
+	}
+
+	expected_size = sizeof(struct sof_ipc_ctrl_data);
+	switch (cdata->type) {
+	case SOF_CTRL_TYPE_VALUE_CHAN_GET:
+	case SOF_CTRL_TYPE_VALUE_CHAN_SET:
+		expected_size += cdata->num_elems *
+				 sizeof(struct sof_ipc_ctrl_value_chan);
+		break;
+	case SOF_CTRL_TYPE_VALUE_COMP_GET:
+	case SOF_CTRL_TYPE_VALUE_COMP_SET:
+		expected_size += cdata->num_elems *
+				 sizeof(struct sof_ipc_ctrl_value_comp);
+		break;
+	case SOF_CTRL_TYPE_DATA_GET:
+	case SOF_CTRL_TYPE_DATA_SET:
+		expected_size += cdata->num_elems + sizeof(struct sof_abi_hdr);
+		break;
+	default:
+		return;
+	}
+
+	if (cdata->rhdr.hdr.size != expected_size) {
+		dev_err(sdev->dev, "error: component notification size mismatch\n");
+		return;
+	}
+
+	if (cdata->num_elems)
+		/*
+		 * The message includes the updated value/data, update the
+		 * control's local cache using the received notification
+		 */
+		snd_sof_update_control(scontrol, cdata);
+	else
+		/* Mark the scontrol that the value/data is changed in SOF */
+		scontrol->comp_data_dirty = true;
+
+	snd_ctl_notify_one(swidget->scomp->card->snd_card,
+			   SNDRV_CTL_EVENT_MASK_VALUE, kc, 0);
+}
diff --git a/sound/soc/sof/ipc.c b/sound/soc/sof/ipc.c
index c2d07b783f60..6260f4ba5618 100644
--- a/sound/soc/sof/ipc.c
+++ b/sound/soc/sof/ipc.c
@@ -369,6 +369,39 @@ void snd_sof_ipc_reply(struct snd_sof_dev *sdev, u32 msg_id)
 }
 EXPORT_SYMBOL(snd_sof_ipc_reply);
 
+static void ipc_comp_notification(struct snd_sof_dev *sdev,
+				  struct sof_ipc_cmd_hdr *hdr)
+{
+	u32 msg_type = hdr->cmd & SOF_CMD_TYPE_MASK;
+	struct sof_ipc_ctrl_data *cdata;
+	int ret;
+
+	switch (msg_type) {
+	case SOF_IPC_COMP_GET_VALUE:
+	case SOF_IPC_COMP_GET_DATA:
+		cdata = kmalloc(hdr->size, GFP_KERNEL);
+		if (!cdata)
+			return;
+
+		/* read back full message */
+		ret = snd_sof_ipc_msg_data(sdev, NULL, cdata, hdr->size);
+		if (ret < 0) {
+			dev_err(sdev->dev,
+				"error: failed to read component event: %d\n", ret);
+			goto err;
+		}
+		break;
+	default:
+		dev_err(sdev->dev, "error: unhandled component message %#x\n", msg_type);
+		return;
+	}
+
+	snd_sof_control_notify(sdev, cdata);
+
+err:
+	kfree(cdata);
+}
+
 /* DSP firmware has sent host a message  */
 void snd_sof_ipc_msgs_rx(struct snd_sof_dev *sdev)
 {
@@ -404,7 +437,9 @@ void snd_sof_ipc_msgs_rx(struct snd_sof_dev *sdev)
 	case SOF_IPC_GLB_COMPOUND:
 	case SOF_IPC_GLB_TPLG_MSG:
 	case SOF_IPC_GLB_PM_MSG:
+		break;
 	case SOF_IPC_GLB_COMP_MSG:
+		ipc_comp_notification(sdev, &hdr);
 		break;
 	case SOF_IPC_GLB_STREAM_MSG:
 		/* need to pass msg id into the function */
diff --git a/sound/soc/sof/sof-audio.h b/sound/soc/sof/sof-audio.h
index dc274e63ed9a..9a8d005e75a0 100644
--- a/sound/soc/sof/sof-audio.h
+++ b/sound/soc/sof/sof-audio.h
@@ -75,6 +75,9 @@ struct snd_sof_control {
 	struct list_head list;	/* list in sdev control list */
 
 	struct snd_sof_led_control led_ctl;
+
+	/* if true, the control's data needs to be updated from Firmware */
+	bool comp_data_dirty;
 };
 
 /* ASoC SOF DAPM widget */
@@ -148,6 +151,8 @@ int snd_sof_bytes_ext_get(struct snd_kcontrol *kcontrol,
 			  unsigned int size);
 int snd_sof_bytes_ext_volatile_get(struct snd_kcontrol *kcontrol, unsigned int __user *binary_data,
 				   unsigned int size);
+void snd_sof_control_notify(struct snd_sof_dev *sdev,
+			    struct sof_ipc_ctrl_data *cdata);
 
 /*
  * Topology.
-- 
2.33.0


             reply	other threads:[~2021-09-02 11:54 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-09-02 11:53 Peter Ujfalusi [this message]
2021-09-02 14:00 ` [PATCH] ASoC: SOF: Handle control change notification from firmware kernel test robot
2021-09-13 10:53 ` Mark Brown

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20210902115328.28478-1-peter.ujfalusi@linux.intel.com \
    --to=peter.ujfalusi@linux.intel.com \
    --cc=alsa-devel@alsa-project.org \
    --cc=broonie@kernel.org \
    --cc=guennadi.liakhovetski@linux.intel.com \
    --cc=kai.vehmanen@linux.intel.com \
    --cc=lgirdwood@gmail.com \
    --cc=pierre-louis.bossart@linux.intel.com \
    --cc=seppo.ingalsuo@linux.intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.