diff mbox series

[v4,02/14] ASoC: SOF: Add Sound Open Firmware KControl support

Message ID 20190213220734.10471-3-pierre-louis.bossart@linux.intel.com (mailing list archive)
State New, archived
Headers show
Series ASoC: Sound Open Firmware (SOF) core | expand

Commit Message

Pierre-Louis Bossart Feb. 13, 2019, 10:07 p.m. UTC
From: Liam Girdwood <liam.r.girdwood@linux.intel.com>

SOF exposes regular ALSA Kcontrols that are defined by topology. This
patch converts the Kcontrol IO to DSP IPC.

The current implementation is aligned with previous Intel solutions,
but is not optimal and can be improved:
a) for every get/put the host wakes up the DSP and generates an
IPC. The kernel should cache the values and generate an IPC only when
strictly necessary.
b) the firmware can be implemented to only instantiate the pipelines
and related control-related parts that are needed at a given time, and
power-gate the relevant SRAM blocks.

The development tasks for these two improvements has started, once
validated they willl be provided in an update.

Signed-off-by: Liam Girdwood <liam.r.girdwood@linux.intel.com>
Signed-off-by: Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>
---
 sound/soc/sof/control.c | 361 ++++++++++++++++++++++++++++++++++++++++
 1 file changed, 361 insertions(+)
 create mode 100644 sound/soc/sof/control.c

Comments

Takashi Iwai Feb. 14, 2019, 9:30 a.m. UTC | #1
On Wed, 13 Feb 2019 23:07:22 +0100,
Pierre-Louis Bossart wrote:
> 
> +int snd_sof_bytes_ext_put(struct snd_kcontrol *kcontrol,
> +			  const unsigned int __user *binary_data,
> +			  unsigned int size)
> +{
> +	struct soc_bytes_ext *be =
> +		(struct soc_bytes_ext *)kcontrol->private_value;
> +	struct snd_sof_control *scontrol = be->dobj.private;
> +	struct snd_sof_dev *sdev = scontrol->sdev;
> +	struct sof_ipc_ctrl_data *cdata = scontrol->control_data;
> +	struct snd_ctl_tlv header;
> +	const struct snd_ctl_tlv __user *tlvd =
> +		(const struct snd_ctl_tlv __user *)binary_data;
....

Do we keep supporting TLV-typed ext access in SOF?
This seems to be disliked nowadays (since it's a sort of abuse of TLV
API), and we agreed for dropping / reducing it.


thanks,

Takashi
Pierre-Louis Bossart Feb. 14, 2019, 2:35 p.m. UTC | #2
On 2/14/19 3:30 AM, Takashi Iwai wrote:
> On Wed, 13 Feb 2019 23:07:22 +0100,
> Pierre-Louis Bossart wrote:
>>
>> +int snd_sof_bytes_ext_put(struct snd_kcontrol *kcontrol,
>> +			  const unsigned int __user *binary_data,
>> +			  unsigned int size)
>> +{
>> +	struct soc_bytes_ext *be =
>> +		(struct soc_bytes_ext *)kcontrol->private_value;
>> +	struct snd_sof_control *scontrol = be->dobj.private;
>> +	struct snd_sof_dev *sdev = scontrol->sdev;
>> +	struct sof_ipc_ctrl_data *cdata = scontrol->control_data;
>> +	struct snd_ctl_tlv header;
>> +	const struct snd_ctl_tlv __user *tlvd =
>> +		(const struct snd_ctl_tlv __user *)binary_data;
> ....
> 
> Do we keep supporting TLV-typed ext access in SOF?
> This seems to be disliked nowadays (since it's a sort of abuse of TLV
> API), and we agreed for dropping / reducing it.

I started working on a replacement of the TLV w/ the hw-dep and showed 
it could be introduced fairly easily in the topology with minor changes 
to the kernel interfaces.

https://github.com/plbossart/sound/commits/proto/hwdep
https://github.com/plbossart/alsa-lib/tree/proto/hwdep

However the big open is linking this hw-dep element in DAPM, so that 
it's handled in the same way as existing controls - specifically that 
the state is restored on resume. That part looked really invasive and 
complicated. I was even tempted to consider flipping the approach and 
define a new control that would expose a hw-dep interface to reuse a lot 
of the DAPM integration.

In short, we know the TLV is not the path forward, and as soon as we 
have a replacement we'll switch. For now we have no alternative.
Takashi Iwai Feb. 14, 2019, 3:21 p.m. UTC | #3
On Thu, 14 Feb 2019 15:35:48 +0100,
Pierre-Louis Bossart wrote:
> 
> On 2/14/19 3:30 AM, Takashi Iwai wrote:
> > On Wed, 13 Feb 2019 23:07:22 +0100,
> > Pierre-Louis Bossart wrote:
> >>
> >> +int snd_sof_bytes_ext_put(struct snd_kcontrol *kcontrol,
> >> +			  const unsigned int __user *binary_data,
> >> +			  unsigned int size)
> >> +{
> >> +	struct soc_bytes_ext *be =
> >> +		(struct soc_bytes_ext *)kcontrol->private_value;
> >> +	struct snd_sof_control *scontrol = be->dobj.private;
> >> +	struct snd_sof_dev *sdev = scontrol->sdev;
> >> +	struct sof_ipc_ctrl_data *cdata = scontrol->control_data;
> >> +	struct snd_ctl_tlv header;
> >> +	const struct snd_ctl_tlv __user *tlvd =
> >> +		(const struct snd_ctl_tlv __user *)binary_data;
> > ....
> >
> > Do we keep supporting TLV-typed ext access in SOF?
> > This seems to be disliked nowadays (since it's a sort of abuse of TLV
> > API), and we agreed for dropping / reducing it.
> 
> I started working on a replacement of the TLV w/ the hw-dep and showed
> it could be introduced fairly easily in the topology with minor
> changes to the kernel interfaces.
> 
> https://github.com/plbossart/sound/commits/proto/hwdep
> https://github.com/plbossart/alsa-lib/tree/proto/hwdep
> 
> However the big open is linking this hw-dep element in DAPM, so that
> it's handled in the same way as existing controls - specifically that
> the state is restored on resume. That part looked really invasive and
> complicated. I was even tempted to consider flipping the approach and
> define a new control that would expose a hw-dep interface to reuse a
> lot of the DAPM integration.
> 
> In short, we know the TLV is not the path forward, and as soon as we
> have a replacement we'll switch. For now we have no alternative.

OK, then let's add some comments that this interface is subject to
change / drop in future.


thanks,

Takashi
diff mbox series

Patch

diff --git a/sound/soc/sof/control.c b/sound/soc/sof/control.c
new file mode 100644
index 000000000000..71950d39aa5b
--- /dev/null
+++ b/sound/soc/sof/control.c
@@ -0,0 +1,361 @@ 
+// SPDX-License-Identifier: (GPL-2.0 OR BSD-3-Clause)
+//
+// This file is provided under a dual BSD/GPLv2 license.  When using or
+// redistributing this file, you may do so under either license.
+//
+// Copyright(c) 2018 Intel Corporation. All rights reserved.
+//
+// Author: Liam Girdwood <liam.r.girdwood@linux.intel.com>
+//
+
+/* Mixer Controls */
+
+#include <linux/pm_runtime.h>
+#include "sof-priv.h"
+
+static inline u32 mixer_to_ipc(unsigned int value, u32 *volume_map, int size)
+{
+	if (value >= size)
+		return volume_map[size - 1];
+
+	return volume_map[value];
+}
+
+static inline u32 ipc_to_mixer(u32 value, u32 *volume_map, int size)
+{
+	int i;
+
+	for (i = 0; i < size; i++) {
+		if (volume_map[i] >= value)
+			return i;
+	}
+
+	return i - 1;
+}
+
+int snd_sof_volume_get(struct snd_kcontrol *kcontrol,
+		       struct snd_ctl_elem_value *ucontrol)
+{
+	struct soc_mixer_control *sm =
+		(struct soc_mixer_control *)kcontrol->private_value;
+	struct snd_sof_control *scontrol = sm->dobj.private;
+	struct snd_sof_dev *sdev = scontrol->sdev;
+	struct sof_ipc_ctrl_data *cdata = scontrol->control_data;
+	unsigned int i, channels = scontrol->num_channels;
+	int err, ret;
+
+	ret = pm_runtime_get_sync(sdev->dev);
+	if (ret < 0) {
+		dev_err_ratelimited(sdev->dev, "error: volume get failed to resume %d\n",
+				    ret);
+		return ret;
+	}
+
+	/* get all the mixer data from DSP */
+	snd_sof_ipc_get_comp_data(sdev->ipc, scontrol, SOF_IPC_COMP_GET_VALUE,
+				  SOF_CTRL_TYPE_VALUE_CHAN_GET,
+				  SOF_CTRL_CMD_VOLUME);
+
+	/* read back each channel */
+	for (i = 0; i < channels; i++)
+		ucontrol->value.integer.value[i] =
+			ipc_to_mixer(cdata->chanv[i].value,
+				     scontrol->volume_table, sm->max + 1);
+
+	pm_runtime_mark_last_busy(sdev->dev);
+	err = pm_runtime_put_autosuspend(sdev->dev);
+	if (err < 0)
+		dev_err_ratelimited(sdev->dev, "error: volume get failed to idle %d\n",
+				    err);
+	return 0;
+}
+
+int snd_sof_volume_put(struct snd_kcontrol *kcontrol,
+		       struct snd_ctl_elem_value *ucontrol)
+{
+	struct soc_mixer_control *sm =
+		(struct soc_mixer_control *)kcontrol->private_value;
+	struct snd_sof_control *scontrol = sm->dobj.private;
+	struct snd_sof_dev *sdev = scontrol->sdev;
+	struct sof_ipc_ctrl_data *cdata = scontrol->control_data;
+	unsigned int i, channels = scontrol->num_channels;
+	int ret, err;
+
+	ret = pm_runtime_get_sync(sdev->dev);
+	if (ret < 0) {
+		dev_err_ratelimited(sdev->dev, "error: volume put failed to resume %d\n",
+				    ret);
+		return ret;
+	}
+
+	/* update each channel */
+	for (i = 0; i < channels; i++) {
+		cdata->chanv[i].value =
+			mixer_to_ipc(ucontrol->value.integer.value[i],
+				     scontrol->volume_table, sm->max + 1);
+		cdata->chanv[i].channel = i;
+	}
+
+	/* notify DSP of mixer updates */
+	snd_sof_ipc_set_comp_data(sdev->ipc, scontrol, SOF_IPC_COMP_SET_VALUE,
+				  SOF_CTRL_TYPE_VALUE_CHAN_GET,
+				  SOF_CTRL_CMD_VOLUME);
+
+	pm_runtime_mark_last_busy(sdev->dev);
+	err = pm_runtime_put_autosuspend(sdev->dev);
+	if (err < 0)
+		dev_err_ratelimited(sdev->dev, "error: volume put failed to idle %d\n",
+				    err);
+	return 0;
+}
+
+int snd_sof_bytes_get(struct snd_kcontrol *kcontrol,
+		      struct snd_ctl_elem_value *ucontrol)
+{
+	struct soc_bytes_ext *be =
+		(struct soc_bytes_ext *)kcontrol->private_value;
+	struct snd_sof_control *scontrol = be->dobj.private;
+	struct snd_sof_dev *sdev = scontrol->sdev;
+	struct sof_ipc_ctrl_data *cdata = scontrol->control_data;
+	struct sof_abi_hdr *data = cdata->data;
+	size_t size;
+	int ret, err;
+
+	if (be->max > sizeof(ucontrol->value.bytes.data)) {
+		dev_err_ratelimited(sdev->dev, "error: data max %d exceeds ucontrol data array size\n",
+				    be->max);
+		return -EINVAL;
+	}
+
+	ret = pm_runtime_get_sync(sdev->dev);
+	if (ret < 0) {
+		dev_err_ratelimited(sdev->dev, "error: bytes get failed to resume %d\n",
+				    ret);
+		return ret;
+	}
+
+	/* get all the mixer data from DSP */
+	snd_sof_ipc_get_comp_data(sdev->ipc, scontrol, SOF_IPC_COMP_GET_DATA,
+				  SOF_CTRL_TYPE_DATA_GET, scontrol->cmd);
+	size = data->size + sizeof(*data);
+	if (size > be->max) {
+		dev_err_ratelimited(sdev->dev, "error: DSP sent %zu bytes max is %d\n",
+				    size, be->max);
+		ret = -EINVAL;
+		goto out;
+	}
+
+	/* copy back to kcontrol */
+	memcpy(ucontrol->value.bytes.data, data, size);
+
+out:
+	pm_runtime_mark_last_busy(sdev->dev);
+	err = pm_runtime_put_autosuspend(sdev->dev);
+	if (err < 0)
+		dev_err_ratelimited(sdev->dev, "error: bytes get failed to idle %d\n",
+				    err);
+	return ret;
+}
+
+int snd_sof_bytes_put(struct snd_kcontrol *kcontrol,
+		      struct snd_ctl_elem_value *ucontrol)
+{
+	struct soc_bytes_ext *be =
+		(struct soc_bytes_ext *)kcontrol->private_value;
+	struct snd_sof_control *scontrol = be->dobj.private;
+	struct snd_sof_dev *sdev = scontrol->sdev;
+	struct sof_ipc_ctrl_data *cdata = scontrol->control_data;
+	struct sof_abi_hdr *data = cdata->data;
+	int ret, err;
+
+	if (be->max > sizeof(ucontrol->value.bytes.data)) {
+		dev_err_ratelimited(sdev->dev, "error: data max %d exceeds ucontrol data array size\n",
+				    be->max);
+		return -EINVAL;
+	}
+
+	if (data->size > be->max) {
+		dev_err_ratelimited(sdev->dev, "error: size too big %d bytes max is %d\n",
+				    data->size, be->max);
+		return -EINVAL;
+	}
+
+	ret = pm_runtime_get_sync(sdev->dev);
+	if (ret < 0) {
+		dev_err_ratelimited(sdev->dev, "error: bytes put failed to resume %d\n",
+				    ret);
+		return ret;
+	}
+
+	/* copy from kcontrol */
+	memcpy(data, ucontrol->value.bytes.data, data->size);
+
+	/* notify DSP of mixer updates */
+	snd_sof_ipc_set_comp_data(sdev->ipc, scontrol, SOF_IPC_COMP_SET_DATA,
+				  SOF_CTRL_TYPE_DATA_SET, scontrol->cmd);
+
+	pm_runtime_mark_last_busy(sdev->dev);
+	err = pm_runtime_put_autosuspend(sdev->dev);
+	if (err < 0)
+		dev_err_ratelimited(sdev->dev, "error: bytes put failed to idle %d\n",
+				    err);
+	return ret;
+}
+
+int snd_sof_bytes_ext_put(struct snd_kcontrol *kcontrol,
+			  const unsigned int __user *binary_data,
+			  unsigned int size)
+{
+	struct soc_bytes_ext *be =
+		(struct soc_bytes_ext *)kcontrol->private_value;
+	struct snd_sof_control *scontrol = be->dobj.private;
+	struct snd_sof_dev *sdev = scontrol->sdev;
+	struct sof_ipc_ctrl_data *cdata = scontrol->control_data;
+	struct snd_ctl_tlv header;
+	const struct snd_ctl_tlv __user *tlvd =
+		(const struct snd_ctl_tlv __user *)binary_data;
+	int ret;
+	int err;
+	int max_size = SOF_IPC_MSG_MAX_SIZE -
+		sizeof(const struct sof_ipc_ctrl_data);
+
+	/*
+	 * The beginning of bytes data contains a header from where
+	 * the length (as bytes) is needed to know the correct copy
+	 * length of data from tlvd->tlv.
+	 */
+	if (copy_from_user(&header, tlvd, sizeof(const struct snd_ctl_tlv)))
+		return -EFAULT;
+
+	/*
+	 * The maximum length that can be copied is limited by IPC max
+	 * length and topology defined length for ext bytes control.
+	 */
+	if (be->max < max_size) /* min() not used to avoid sparse warnings */
+		max_size = be->max;
+	if (header.length > max_size) {
+		dev_err_ratelimited(sdev->dev, "error: Bytes data size %d exceeds max %d.\n",
+				    header.length, max_size);
+		return -EINVAL;
+	}
+
+	/* Check that header id matches the command */
+	if (header.numid != scontrol->cmd) {
+		dev_err_ratelimited(sdev->dev, "error: incorrect numid %d\n",
+				    header.numid);
+		return -EINVAL;
+	}
+
+	if (copy_from_user(cdata->data, tlvd->tlv, header.length))
+		return -EFAULT;
+
+	if (cdata->data->magic != SOF_ABI_MAGIC) {
+		dev_err_ratelimited(sdev->dev, "error: Wrong ABI magic 0x%08x.\n",
+				    cdata->data->magic);
+		return -EINVAL;
+	}
+
+	if (SOF_ABI_VERSION_INCOMPATIBLE(SOF_ABI_VERSION, cdata->data->abi)) {
+		dev_err_ratelimited(sdev->dev, "error: Incompatible ABI version 0x%08x.\n",
+				    cdata->data->abi);
+		return -EINVAL;
+	}
+
+	if (cdata->data->size + sizeof(const struct sof_abi_hdr) > max_size) {
+		dev_err_ratelimited(sdev->dev, "error: Mismatch in ABI data size (truncated?).\n");
+		return -EINVAL;
+	}
+
+	ret = pm_runtime_get_sync(sdev->dev);
+	if (ret < 0) {
+		dev_err_ratelimited(sdev->dev, "error: bytes_ext put failed to resume %d\n",
+				    ret);
+		return ret;
+	}
+
+	/* notify DSP of mixer updates */
+	snd_sof_ipc_set_comp_data(sdev->ipc, scontrol, SOF_IPC_COMP_SET_DATA,
+				  SOF_CTRL_TYPE_DATA_SET, scontrol->cmd);
+
+	pm_runtime_mark_last_busy(sdev->dev);
+	err = pm_runtime_put_autosuspend(sdev->dev);
+	if (err < 0)
+		dev_err_ratelimited(sdev->dev, "error: bytes_ext put failed to idle %d\n",
+				    err);
+
+	return ret;
+}
+
+int snd_sof_bytes_ext_get(struct snd_kcontrol *kcontrol,
+			  unsigned int __user *binary_data,
+			  unsigned int size)
+{
+	struct soc_bytes_ext *be =
+		(struct soc_bytes_ext *)kcontrol->private_value;
+	struct snd_sof_control *scontrol = be->dobj.private;
+	struct snd_sof_dev *sdev = scontrol->sdev;
+	struct sof_ipc_ctrl_data *cdata = scontrol->control_data;
+	struct snd_ctl_tlv header;
+	struct snd_ctl_tlv __user *tlvd =
+		(struct snd_ctl_tlv __user *)binary_data;
+	int max_size = SOF_IPC_MSG_MAX_SIZE -
+		sizeof(const struct sof_ipc_ctrl_data);
+	int data_size;
+	int err;
+	int ret;
+
+	ret = pm_runtime_get_sync(sdev->dev);
+	if (ret < 0) {
+		dev_err_ratelimited(sdev->dev, "error: bytes_ext get failed to resume %d\n",
+				    ret);
+		return ret;
+	}
+
+	/*
+	 * Decrement the limit by ext bytes header size to
+	 * ensure the user space buffer is not exceeded.
+	 */
+	size -= sizeof(const struct snd_ctl_tlv);
+
+	/* set the ABI header values */
+	cdata->data->magic = SOF_ABI_MAGIC;
+	cdata->data->abi = SOF_ABI_VERSION;
+
+	/* get all the component data from DSP */
+	ret = snd_sof_ipc_get_comp_data(sdev->ipc, scontrol,
+					SOF_IPC_COMP_GET_DATA,
+					SOF_CTRL_TYPE_DATA_GET, scontrol->cmd);
+
+	/* Prevent read of other kernel data or possibly corrupt response */
+	data_size = cdata->data->size + sizeof(const struct sof_abi_hdr);
+
+	/* check size. min3() is not used to avoid sparse warnings */
+	if (size < max_size)
+		max_size = size;
+	if (be->max < max_size)
+		max_size = be->max;
+	if (data_size > max_size) {
+		dev_err_ratelimited(sdev->dev, "error: user data size %d exceeds max size %d.\n",
+				    data_size, max_size);
+		ret = -EINVAL;
+		goto out;
+	}
+
+	header.numid = scontrol->cmd;
+	header.length = data_size;
+	if (copy_to_user(tlvd, &header, sizeof(const struct snd_ctl_tlv))) {
+		ret = -EFAULT;
+		goto out;
+	}
+
+	if (copy_to_user(tlvd->tlv, cdata->data, data_size))
+		return -EFAULT;
+
+out:
+	pm_runtime_mark_last_busy(sdev->dev);
+	err = pm_runtime_put_autosuspend(sdev->dev);
+	if (err < 0)
+		dev_err_ratelimited(sdev->dev, "error: bytes_ext get failed to idle %d\n",
+				    err);
+	return ret;
+}