[v2,10/26] ALSA: hda: hda_cs_dsp_ctl: Add Library to support CS_DSP ALSA controls

Commit Message

Vitaly Rodionov May 9, 2022, 9:46 p.m. UTC

From: Stefan Binding <[e-mail]>

The cs35l41 part contains a DSP which is able to run firmware.
The cs_dsp library can be used to control the DSP.
These controls can be exposed to userspace using ALSA controls.
This library adds apis to be able to interface between
cs_dsp and hda drivers and expose the relevant controls as
ALSA controls.

Signed-off-by: Stefan Binding <[e-mail]>
Signed-off-by: Vitaly Rodionov <[e-mail]>
---
 MAINTAINERS                    |   1 +
 sound/pci/hda/Kconfig          |   4 +
 sound/pci/hda/Makefile         |   2 +
 sound/pci/hda/hda_cs_dsp_ctl.c | 364 +++++++++++++++++++++++++++++++++
 sound/pci/hda/hda_cs_dsp_ctl.h |  34 +++
 5 files changed, 405 insertions(+)
 create mode 100644 sound/pci/hda/hda_cs_dsp_ctl.c
 create mode 100644 sound/pci/hda/hda_cs_dsp_ctl.h

Comments

Takashi Iwai May 11, 2022, 1:49 p.m. UTC | #1

On Mon, 09 May 2022 23:46:47 +0200,
Vitaly Rodionov wrote:
> 
> From: Stefan Binding <[e-mail]>
> 
> The cs35l41 part contains a DSP which is able to run firmware.
> The cs_dsp library can be used to control the DSP.
> These controls can be exposed to userspace using ALSA controls.
> This library adds apis to be able to interface between
> cs_dsp and hda drivers and expose the relevant controls as
> ALSA controls.

Hmm, quite lots of things aren't explained here.

First off, as far as I see, the control elements that are implemented
in this patch are pretty unique, they don't follow the standard way.
Admittedly, ASoC core (ab)uses the TLV read/write for the arbitrary
data bytes, and this seems following that instead.  If so, it needs
more clear explanation in the comments add/or commit logs.

Second, hda_cs_dsp_control_add() and _remove() functions do the
addition and the removal asynchronously with a dedicated work.  It's
fragile as an API, and even no clarification is given why it's done
so.


thanks,

Takashi

> 
> Signed-off-by: Stefan Binding <[e-mail]>
> Signed-off-by: Vitaly Rodionov <[e-mail]>
> ---
>  MAINTAINERS                    |   1 +
>  sound/pci/hda/Kconfig          |   4 +
>  sound/pci/hda/Makefile         |   2 +
>  sound/pci/hda/hda_cs_dsp_ctl.c | 364 +++++++++++++++++++++++++++++++++
>  sound/pci/hda/hda_cs_dsp_ctl.h |  34 +++
>  5 files changed, 405 insertions(+)
>  create mode 100644 sound/pci/hda/hda_cs_dsp_ctl.c
>  create mode 100644 sound/pci/hda/hda_cs_dsp_ctl.h
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index f57e6d38a542..b7e4d2a0ef41 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -4713,6 +4713,7 @@ S:	Maintained
>  F:	Documentation/devicetree/bindings/sound/cirrus,cs*
>  F:	include/dt-bindings/sound/cs*
>  F:	sound/pci/hda/cs*
> +F:	sound/pci/hda/hda_cs_dsp_ctl.*
>  F:	sound/soc/codecs/cs*
>  
>  CIRRUS LOGIC DSP FIRMWARE DRIVER
> diff --git a/sound/pci/hda/Kconfig b/sound/pci/hda/Kconfig
> index 79ade4787d95..d1fd6cf82beb 100644
> --- a/sound/pci/hda/Kconfig
> +++ b/sound/pci/hda/Kconfig
> @@ -94,6 +94,10 @@ config SND_HDA_PATCH_LOADER
>  config SND_HDA_SCODEC_CS35L41
>  	tristate
>  
> +config SND_HDA_CS_DSP_CONTROLS
> +	tristate
> +	depends on CS_DSP
> +
>  config SND_HDA_SCODEC_CS35L41_I2C
>  	tristate "Build CS35L41 HD-audio side codec support for I2C Bus"
>  	depends on I2C
> diff --git a/sound/pci/hda/Makefile b/sound/pci/hda/Makefile
> index 3e7bc608d45f..00d306104484 100644
> --- a/sound/pci/hda/Makefile
> +++ b/sound/pci/hda/Makefile
> @@ -31,6 +31,7 @@ snd-hda-codec-hdmi-objs :=	patch_hdmi.o hda_eld.o
>  snd-hda-scodec-cs35l41-objs :=		cs35l41_hda.o
>  snd-hda-scodec-cs35l41-i2c-objs :=	cs35l41_hda_i2c.o
>  snd-hda-scodec-cs35l41-spi-objs :=	cs35l41_hda_spi.o
> +snd-hda-cs-dsp-ctls-objs :=		hda_cs_dsp_ctl.o
>  
>  # common driver
>  obj-$(CONFIG_SND_HDA) := snd-hda-codec.o
> @@ -54,6 +55,7 @@ obj-$(CONFIG_SND_HDA_CODEC_HDMI) += snd-hda-codec-hdmi.o
>  obj-$(CONFIG_SND_HDA_SCODEC_CS35L41) += snd-hda-scodec-cs35l41.o
>  obj-$(CONFIG_SND_HDA_SCODEC_CS35L41_I2C) += snd-hda-scodec-cs35l41-i2c.o
>  obj-$(CONFIG_SND_HDA_SCODEC_CS35L41_SPI) += snd-hda-scodec-cs35l41-spi.o
> +obj-$(CONFIG_SND_HDA_CS_DSP_CONTROLS) += snd-hda-cs-dsp-ctls.o
>  
>  # this must be the last entry after codec drivers;
>  # otherwise the codec patches won't be hooked before the PCI probe
> diff --git a/sound/pci/hda/hda_cs_dsp_ctl.c b/sound/pci/hda/hda_cs_dsp_ctl.c
> new file mode 100644
> index 000000000000..e94740c5557a
> --- /dev/null
> +++ b/sound/pci/hda/hda_cs_dsp_ctl.c
> @@ -0,0 +1,364 @@
> +// SPDX-License-Identifier: GPL-2.0
> +//
> +// HDA DSP ALSA Control Driver
> +//
> +// Copyright 2022 Cirrus Logic, Inc.
> +//
> +// Author: Stefan Binding <[e-mail]>
> +
> +#include <linux/module.h>
> +#include <sound/soc.h>
> +#include <linux/firmware/cirrus/cs_dsp.h>
> +#include <linux/firmware/cirrus/wmfw.h>
> +#include "hda_cs_dsp_ctl.h"
> +
> +struct hda_cs_dsp_coeff_ctl {
> +	const char *name;
> +	struct cs_dsp_coeff_ctl *cs_ctl;
> +	struct snd_card *card;
> +	struct soc_bytes_ext bytes_ext;
> +	struct work_struct add_work;
> +	struct work_struct remove_work;
> +};
> +
> +static const char * const hda_cs_dsp_fw_text[HDA_CS_DSP_NUM_FW] = {
> +	[HDA_CS_DSP_FW_SPK_PROT] = "Prot",
> +	[HDA_CS_DSP_FW_SPK_CALI] = "Cali",
> +	[HDA_CS_DSP_FW_SPK_DIAG] = "Diag",
> +	[HDA_CS_DSP_FW_MISC] =     "Misc",
> +};
> +
> +static inline struct hda_cs_dsp_coeff_ctl *bytes_ext_to_ctl(struct soc_bytes_ext *ext)
> +{
> +	return container_of(ext, struct hda_cs_dsp_coeff_ctl, bytes_ext);
> +}
> +
> +static int hda_cs_dsp_coeff_info(struct snd_kcontrol *kctl, struct snd_ctl_elem_info *uinfo)
> +{
> +	struct soc_bytes_ext *bytes_ext =
> +		(struct soc_bytes_ext *)kctl->private_value;
> +	struct hda_cs_dsp_coeff_ctl *ctl = bytes_ext_to_ctl(bytes_ext);
> +	struct cs_dsp_coeff_ctl *cs_ctl = ctl->cs_ctl;
> +
> +	switch (cs_ctl->type) {
> +	case WMFW_CTL_TYPE_ACKED:
> +		uinfo->type = SNDRV_CTL_ELEM_TYPE_INTEGER;
> +		uinfo->value.integer.min = CS_DSP_ACKED_CTL_MIN_VALUE;
> +		uinfo->value.integer.max = CS_DSP_ACKED_CTL_MAX_VALUE;
> +		uinfo->value.integer.step = 1;
> +		uinfo->count = 1;
> +		break;
> +	default:
> +		uinfo->type = SNDRV_CTL_ELEM_TYPE_BYTES;
> +		uinfo->count = cs_ctl->len;
> +		break;
> +	}
> +
> +	return 0;
> +}
> +
> +static int hda_cs_dsp_coeff_put(struct snd_kcontrol *kctl, struct snd_ctl_elem_value *ucontrol)
> +{
> +	struct soc_bytes_ext *bytes_ext =
> +		(struct soc_bytes_ext *)kctl->private_value;
> +	struct hda_cs_dsp_coeff_ctl *ctl = bytes_ext_to_ctl(bytes_ext);
> +	struct cs_dsp_coeff_ctl *cs_ctl = ctl->cs_ctl;
> +	char *p = ucontrol->value.bytes.data;
> +	int ret = 0;
> +
> +	mutex_lock(&cs_ctl->dsp->pwr_lock);
> +	ret = cs_dsp_coeff_write_ctrl(cs_ctl, 0, p, cs_ctl->len);
> +	mutex_unlock(&cs_ctl->dsp->pwr_lock);
> +
> +	return ret;
> +}
> +
> +static int hda_cs_dsp_coeff_tlv_put(struct snd_kcontrol *kctl,
> +				    const unsigned int __user *bytes, unsigned int size)
> +{
> +	struct soc_bytes_ext *bytes_ext =
> +		(struct soc_bytes_ext *)kctl->private_value;
> +	struct hda_cs_dsp_coeff_ctl *ctl = bytes_ext_to_ctl(bytes_ext);
> +	struct cs_dsp_coeff_ctl *cs_ctl = ctl->cs_ctl;
> +	int ret = 0;
> +
> +	mutex_lock(&cs_ctl->dsp->pwr_lock);
> +
> +	if (copy_from_user(cs_ctl->cache, bytes, size))
> +		ret = -EFAULT;
> +	else
> +		ret = cs_dsp_coeff_write_ctrl(cs_ctl, 0, cs_ctl->cache, size);
> +
> +	mutex_unlock(&cs_ctl->dsp->pwr_lock);
> +
> +	return ret;
> +}
> +
> +static int hda_cs_dsp_coeff_put_acked(struct snd_kcontrol *kctl,
> +				      struct snd_ctl_elem_value *ucontrol)
> +{
> +	struct soc_bytes_ext *bytes_ext =
> +		(struct soc_bytes_ext *)kctl->private_value;
> +	struct hda_cs_dsp_coeff_ctl *ctl = bytes_ext_to_ctl(bytes_ext);
> +	struct cs_dsp_coeff_ctl *cs_ctl = ctl->cs_ctl;
> +	unsigned int val = ucontrol->value.integer.value[0];
> +	int ret;
> +
> +	if (val == 0)
> +		return 0;	/* 0 means no event */
> +
> +	mutex_lock(&cs_ctl->dsp->pwr_lock);
> +
> +	if (cs_ctl->enabled)
> +		ret = cs_dsp_coeff_write_acked_control(cs_ctl, val);
> +	else
> +		ret = -EPERM;
> +
> +	mutex_unlock(&cs_ctl->dsp->pwr_lock);
> +
> +	return ret;
> +}
> +
> +static int hda_cs_dsp_coeff_get(struct snd_kcontrol *kctl, struct snd_ctl_elem_value *ucontrol)
> +{
> +	struct soc_bytes_ext *bytes_ext =
> +		(struct soc_bytes_ext *)kctl->private_value;
> +	struct hda_cs_dsp_coeff_ctl *ctl = bytes_ext_to_ctl(bytes_ext);
> +	struct cs_dsp_coeff_ctl *cs_ctl = ctl->cs_ctl;
> +	char *p = ucontrol->value.bytes.data;
> +	int ret;
> +
> +	mutex_lock(&cs_ctl->dsp->pwr_lock);
> +	ret = cs_dsp_coeff_read_ctrl(cs_ctl, 0, p, cs_ctl->len);
> +	mutex_unlock(&cs_ctl->dsp->pwr_lock);
> +
> +	return ret;
> +}
> +
> +static int hda_cs_dsp_coeff_tlv_get(struct snd_kcontrol *kctl,
> +				    unsigned int __user *bytes, unsigned int size)
> +{
> +	struct soc_bytes_ext *bytes_ext =
> +		(struct soc_bytes_ext *)kctl->private_value;
> +	struct hda_cs_dsp_coeff_ctl *ctl = bytes_ext_to_ctl(bytes_ext);
> +	struct cs_dsp_coeff_ctl *cs_ctl = ctl->cs_ctl;
> +	int ret = 0;
> +
> +	mutex_lock(&cs_ctl->dsp->pwr_lock);
> +
> +	ret = cs_dsp_coeff_read_ctrl(cs_ctl, 0, cs_ctl->cache, size);
> +
> +	if (!ret && copy_to_user(bytes, cs_ctl->cache, size))
> +		ret = -EFAULT;
> +
> +	mutex_unlock(&cs_ctl->dsp->pwr_lock);
> +
> +	return ret;
> +}
> +
> +static int hda_cs_dsp_coeff_get_acked(struct snd_kcontrol *kcontrol,
> +				      struct snd_ctl_elem_value *ucontrol)
> +{
> +	/*
> +	 * Although it's not useful to read an acked control, we must satisfy
> +	 * user-side assumptions that all controls are readable and that a
> +	 * write of the same value should be filtered out (it's valid to send
> +	 * the same event number again to the firmware). We therefore return 0,
> +	 * meaning "no event" so valid event numbers will always be a change
> +	 */
> +	ucontrol->value.integer.value[0] = 0;
> +
> +	return 0;
> +}
> +#define ADSP_MAX_STD_CTRL_SIZE               512
> +
> +static unsigned int wmfw_convert_flags(unsigned int in, unsigned int len)
> +{
> +	unsigned int out, rd, wr, vol;
> +
> +	if (len > ADSP_MAX_STD_CTRL_SIZE) {
> +		rd = SNDRV_CTL_ELEM_ACCESS_TLV_READ;
> +		wr = SNDRV_CTL_ELEM_ACCESS_TLV_WRITE;
> +		vol = SNDRV_CTL_ELEM_ACCESS_VOLATILE;
> +
> +		out = SNDRV_CTL_ELEM_ACCESS_TLV_CALLBACK;
> +	} else {
> +		rd = SNDRV_CTL_ELEM_ACCESS_READ;
> +		wr = SNDRV_CTL_ELEM_ACCESS_WRITE;
> +		vol = SNDRV_CTL_ELEM_ACCESS_VOLATILE;
> +
> +		out = 0;
> +	}
> +
> +	if (in) {
> +		out |= rd;
> +		if (in & WMFW_CTL_FLAG_WRITEABLE)
> +			out |= wr;
> +		if (in & WMFW_CTL_FLAG_VOLATILE)
> +			out |= vol;
> +	} else {
> +		out |= rd | wr | vol;
> +	}
> +
> +	return out;
> +}
> +
> +static void hda_cs_dsp_ctl_add_work(struct work_struct *work)
> +{
> +	struct hda_cs_dsp_coeff_ctl *ctl = container_of(work,
> +							struct hda_cs_dsp_coeff_ctl,
> +							add_work);
> +	struct cs_dsp_coeff_ctl *cs_ctl = ctl->cs_ctl;
> +	struct snd_kcontrol_new *kcontrol;
> +
> +	kcontrol = kzalloc(sizeof(*kcontrol), GFP_KERNEL);
> +	if (!kcontrol)
> +		return;
> +
> +	kcontrol->name = ctl->name;
> +	kcontrol->info = hda_cs_dsp_coeff_info;
> +	kcontrol->iface = SNDRV_CTL_ELEM_IFACE_MIXER;
> +	kcontrol->tlv.c = snd_soc_bytes_tlv_callback;
> +	kcontrol->private_value = (unsigned long)&ctl->bytes_ext;
> +	kcontrol->access = wmfw_convert_flags(cs_ctl->flags, cs_ctl->len);
> +
> +	switch (cs_ctl->type) {
> +	case WMFW_CTL_TYPE_ACKED:
> +		kcontrol->get = hda_cs_dsp_coeff_get_acked;
> +		kcontrol->put = hda_cs_dsp_coeff_put_acked;
> +		break;
> +	default:
> +		if (kcontrol->access & SNDRV_CTL_ELEM_ACCESS_TLV_CALLBACK) {
> +			ctl->bytes_ext.max = cs_ctl->len;
> +			ctl->bytes_ext.get = hda_cs_dsp_coeff_tlv_get;
> +			ctl->bytes_ext.put = hda_cs_dsp_coeff_tlv_put;
> +		} else {
> +			kcontrol->get = hda_cs_dsp_coeff_get;
> +			kcontrol->put = hda_cs_dsp_coeff_put;
> +		}
> +		break;
> +	}
> +
> +	if (snd_ctl_add(ctl->card, snd_ctl_new1(kcontrol, NULL)))
> +		dev_err(cs_ctl->dsp->dev, "Failed to add KControl: %s\n", kcontrol->name);
> +	else
> +		dev_dbg(cs_ctl->dsp->dev, "Added KControl: %s\n", kcontrol->name);
> +
> +	kfree(kcontrol);
> +}
> +
> +int hda_cs_dsp_control_add(struct cs_dsp_coeff_ctl *cs_ctl, struct hda_cs_dsp_ctl_info *info)
> +{
> +	struct cs_dsp *cs_dsp = cs_ctl->dsp;
> +	char name[SNDRV_CTL_ELEM_ID_NAME_MAXLEN];
> +	struct hda_cs_dsp_coeff_ctl *ctl;
> +	const char *region_name;
> +	int ret;
> +
> +	if (cs_ctl->flags & WMFW_CTL_FLAG_SYS) {
> +		dev_dbg(cs_dsp->dev, "cs_ctl->flags = WMFW_CTL_FLAG_SYS\n");
> +		return 0;
> +	}
> +
> +	region_name = cs_dsp_mem_region_name(cs_ctl->alg_region.type);
> +	if (!region_name) {
> +		dev_err(cs_dsp->dev, "Unknown region type: %d\n", cs_ctl->alg_region.type);
> +		return -EINVAL;
> +	}
> +
> +	switch (cs_dsp->fw_ver) {
> +	case 0:
> +	case 1:
> +		ret = scnprintf(name, SNDRV_CTL_ELEM_ID_NAME_MAXLEN,
> +				"%s %s %s %x", info->amp_name, cs_dsp->name, region_name,
> +				cs_ctl->alg_region.alg);
> +		break;
> +	case 2:
> +		ret = scnprintf(name, SNDRV_CTL_ELEM_ID_NAME_MAXLEN,
> +				"%s %s%c %.12s %x", info->amp_name, cs_dsp->name, *region_name,
> +				hda_cs_dsp_fw_text[info->fw_type], cs_ctl->alg_region.alg);
> +		break;
> +	default:
> +		ret = scnprintf(name, SNDRV_CTL_ELEM_ID_NAME_MAXLEN,
> +				"%s %s %.12s %x", info->amp_name, cs_dsp->name,
> +				hda_cs_dsp_fw_text[info->fw_type], cs_ctl->alg_region.alg);
> +		break;
> +	}
> +
> +	if (cs_ctl->subname) {
> +		int avail = SNDRV_CTL_ELEM_ID_NAME_MAXLEN - ret - 2;
> +		int skip = 0;
> +
> +		/* Truncate the subname from the start if it is too long */
> +		if (cs_ctl->subname_len > avail)
> +			skip = cs_ctl->subname_len - avail;
> +
> +		snprintf(name + ret, SNDRV_CTL_ELEM_ID_NAME_MAXLEN - ret,
> +			 " %.*s", cs_ctl->subname_len - skip, cs_ctl->subname + skip);
> +	}
> +
> +	ctl = kzalloc(sizeof(*ctl), GFP_KERNEL);
> +	if (!ctl)
> +		return -ENOMEM;
> +	ctl->cs_ctl = cs_ctl;
> +	ctl->card = info->card;
> +
> +	ctl->name = kmemdup(name, strlen(name) + 1, GFP_KERNEL);
> +	if (!ctl->name) {
> +		ret = -ENOMEM;
> +		dev_err(cs_dsp->dev, "Cannot save ctl name\n");
> +		goto err_ctl;
> +	}
> +
> +	cs_ctl->priv = ctl;
> +
> +	INIT_WORK(&ctl->add_work, hda_cs_dsp_ctl_add_work);
> +	schedule_work(&ctl->add_work);
> +
> +	return 0;
> +
> +err_ctl:
> +	dev_err(cs_dsp->dev, "Error adding control: %s\n", name);
> +	kfree(ctl);
> +	return ret;
> +}
> +EXPORT_SYMBOL_NS_GPL(hda_cs_dsp_control_add, SND_HDA_CS_DSP_CONTROLS);
> +
> +int hda_cs_dsp_remove_kcontrol(struct snd_card *card, const char *name)
> +{
> +	struct snd_kcontrol *kctl;
> +
> +	list_for_each_entry(kctl, &card->controls, list)
> +		if (!strncmp(kctl->id.name, name, sizeof(kctl->id.name)))
> +			return snd_ctl_remove_id(card, &kctl->id);
> +
> +	return -EINVAL;
> +}
> +EXPORT_SYMBOL_NS_GPL(hda_cs_dsp_remove_kcontrol, SND_HDA_CS_DSP_CONTROLS);
> +
> +static void hda_cs_dsp_ctl_del_work(struct work_struct *work)
> +{
> +	struct hda_cs_dsp_coeff_ctl *ctl = container_of(work,
> +							struct hda_cs_dsp_coeff_ctl,
> +							remove_work);
> +
> +	cancel_work_sync(&ctl->add_work);
> +
> +	hda_cs_dsp_remove_kcontrol(ctl->card, ctl->name);
> +
> +	kfree(ctl->name);
> +	kfree(ctl);
> +}
> +
> +void hda_cs_dsp_control_remove(struct cs_dsp_coeff_ctl *cs_ctl)
> +{
> +	struct hda_cs_dsp_coeff_ctl *ctl = cs_ctl->priv;
> +
> +	INIT_WORK(&ctl->remove_work, hda_cs_dsp_ctl_del_work);
> +	schedule_work(&ctl->remove_work);
> +}
> +EXPORT_SYMBOL_NS_GPL(hda_cs_dsp_control_remove, SND_HDA_CS_DSP_CONTROLS);
> +
> +MODULE_DESCRIPTION("CS_DSP ALSA Control HDA Library");
> +MODULE_AUTHOR("Stefan Binding, <[e-mail]>");
> +MODULE_LICENSE("GPL");
> diff --git a/sound/pci/hda/hda_cs_dsp_ctl.h b/sound/pci/hda/hda_cs_dsp_ctl.h
> new file mode 100644
> index 000000000000..3c90312b45d6
> --- /dev/null
> +++ b/sound/pci/hda/hda_cs_dsp_ctl.h
> @@ -0,0 +1,34 @@
> +/* SPDX-License-Identifier: GPL-2.0
> + *
> + * HDA DSP ALSA Control Driver
> + *
> + * Copyright 2022 Cirrus Logic, Inc.
> + *
> + * Author: Stefan Binding <[e-mail]>
> + */
> +
> +#ifndef __HDA_CS_DSP_CTL_H__
> +#define __HDA_CS_DSP_CTL_H__
> +
> +#include <sound/soc.h>
> +#include <linux/firmware/cirrus/cs_dsp.h>
> +
> +enum hda_cs_dsp_fw_id {
> +	HDA_CS_DSP_FW_SPK_PROT,
> +	HDA_CS_DSP_FW_SPK_CALI,
> +	HDA_CS_DSP_FW_SPK_DIAG,
> +	HDA_CS_DSP_FW_MISC,
> +	HDA_CS_DSP_NUM_FW
> +};
> +
> +struct hda_cs_dsp_ctl_info {
> +	struct snd_card *card;
> +	enum hda_cs_dsp_fw_id fw_type;
> +	const char *amp_name;
> +};
> +
> +int hda_cs_dsp_control_add(struct cs_dsp_coeff_ctl *cs_ctl, struct hda_cs_dsp_ctl_info *info);
> +void hda_cs_dsp_control_remove(struct cs_dsp_coeff_ctl *cs_ctl);
> +int hda_cs_dsp_remove_kcontrol(struct snd_card *card, const char *name);
> +
> +#endif /*__HDA_CS_DSP_CTL_H__*/
> -- 
> 2.34.1
>

Charles Keepax May 11, 2022, 1:57 p.m. UTC | #2

On Wed, May 11, 2022 at 03:49:12PM +0200, Takashi Iwai wrote:
> On Mon, 09 May 2022 23:46:47 +0200,
> Vitaly Rodionov wrote:
> > 
> > From: Stefan Binding <[e-mail]>
> > 
> > The cs35l41 part contains a DSP which is able to run firmware.
> > The cs_dsp library can be used to control the DSP.
> > These controls can be exposed to userspace using ALSA controls.
> > This library adds apis to be able to interface between
> > cs_dsp and hda drivers and expose the relevant controls as
> > ALSA controls.
> 
> Hmm, quite lots of things aren't explained here.
> 
> First off, as far as I see, the control elements that are implemented
> in this patch are pretty unique, they don't follow the standard way.
> Admittedly, ASoC core (ab)uses the TLV read/write for the arbitrary
> data bytes, and this seems following that instead.  If so, it needs
> more clear explanation in the comments add/or commit logs.
> 

Hm... yes I think we should be very clear on if the TLV support
is needed here. This should only be needed if the firmware has
controls (and we need to use those controls) over 512 bytes, and
most firmwares don't. Can we check this and remove the TLV
support if we can? We probably don't want to add more users of
that stuff if we can help it.

Thanks,
Charles

Charles Keepax May 11, 2022, 2:02 p.m. UTC | #3

On Mon, May 09, 2022 at 10:46:47PM +0100, Vitaly Rodionov wrote:
> From: Stefan Binding <[e-mail]>
> 
> The cs35l41 part contains a DSP which is able to run firmware.
> The cs_dsp library can be used to control the DSP.
> These controls can be exposed to userspace using ALSA controls.
> This library adds apis to be able to interface between
> cs_dsp and hda drivers and expose the relevant controls as
> ALSA controls.
> 
> Signed-off-by: Stefan Binding <[e-mail]>
> Signed-off-by: Vitaly Rodionov <[e-mail]>
> ---
> +static int hda_cs_dsp_coeff_info(struct snd_kcontrol *kctl, struct snd_ctl_elem_info *uinfo)
> +{
> +	struct soc_bytes_ext *bytes_ext =
> +		(struct soc_bytes_ext *)kctl->private_value;
> +	struct hda_cs_dsp_coeff_ctl *ctl = bytes_ext_to_ctl(bytes_ext);
> +	struct cs_dsp_coeff_ctl *cs_ctl = ctl->cs_ctl;
> +
> +	switch (cs_ctl->type) {
> +	case WMFW_CTL_TYPE_ACKED:
> +		uinfo->type = SNDRV_CTL_ELEM_TYPE_INTEGER;
> +		uinfo->value.integer.min = CS_DSP_ACKED_CTL_MIN_VALUE;
> +		uinfo->value.integer.max = CS_DSP_ACKED_CTL_MAX_VALUE;
> +		uinfo->value.integer.step = 1;
> +		uinfo->count = 1;
> +		break;

With the ACKED controls as well as the TLVs it would be worth
verifing if any of the relevant firmwares actually require these,
if we are going to duplicate a bunch of the wm_adsp code into HDA
lets duplicate only exactly what we need.

> +	switch (cs_dsp->fw_ver) {
> +	case 0:
> +	case 1:
> +		ret = scnprintf(name, SNDRV_CTL_ELEM_ID_NAME_MAXLEN,
> +				"%s %s %s %x", info->amp_name, cs_dsp->name, region_name,
> +				cs_ctl->alg_region.alg);
> +		break;
> +	case 2:
> +		ret = scnprintf(name, SNDRV_CTL_ELEM_ID_NAME_MAXLEN,
> +				"%s %s%c %.12s %x", info->amp_name, cs_dsp->name, *region_name,
> +				hda_cs_dsp_fw_text[info->fw_type], cs_ctl->alg_region.alg);
> +		break;
> +	default:
> +		ret = scnprintf(name, SNDRV_CTL_ELEM_ID_NAME_MAXLEN,
> +				"%s %s %.12s %x", info->amp_name, cs_dsp->name,
> +				hda_cs_dsp_fw_text[info->fw_type], cs_ctl->alg_region.alg);
> +		break;
> +	}

Do we actually need to support all the historical fw_versions?
ADSP does since CODECs using it have been around forever, but I
would suggest it is quite like this stuff only needs to support
the latest version.

Thanks,
Charles

Patch

diff --git a/MAINTAINERS b/MAINTAINERS
index f57e6d38a542..b7e4d2a0ef41 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -4713,6 +4713,7 @@  S:	Maintained
 F:	Documentation/devicetree/bindings/sound/cirrus,cs*
 F:	include/dt-bindings/sound/cs*
 F:	sound/pci/hda/cs*
+F:	sound/pci/hda/hda_cs_dsp_ctl.*
 F:	sound/soc/codecs/cs*
 
 CIRRUS LOGIC DSP FIRMWARE DRIVER
diff --git a/sound/pci/hda/Kconfig b/sound/pci/hda/Kconfig
index 79ade4787d95..d1fd6cf82beb 100644
--- a/sound/pci/hda/Kconfig
+++ b/sound/pci/hda/Kconfig
@@ -94,6 +94,10 @@  config SND_HDA_PATCH_LOADER
 config SND_HDA_SCODEC_CS35L41
 	tristate
 
+config SND_HDA_CS_DSP_CONTROLS
+	tristate
+	depends on CS_DSP
+
 config SND_HDA_SCODEC_CS35L41_I2C
 	tristate "Build CS35L41 HD-audio side codec support for I2C Bus"
 	depends on I2C
diff --git a/sound/pci/hda/Makefile b/sound/pci/hda/Makefile
index 3e7bc608d45f..00d306104484 100644
--- a/sound/pci/hda/Makefile
+++ b/sound/pci/hda/Makefile
@@ -31,6 +31,7 @@  snd-hda-codec-hdmi-objs :=	patch_hdmi.o hda_eld.o
 snd-hda-scodec-cs35l41-objs :=		cs35l41_hda.o
 snd-hda-scodec-cs35l41-i2c-objs :=	cs35l41_hda_i2c.o
 snd-hda-scodec-cs35l41-spi-objs :=	cs35l41_hda_spi.o
+snd-hda-cs-dsp-ctls-objs :=		hda_cs_dsp_ctl.o
 
 # common driver
 obj-$(CONFIG_SND_HDA) := snd-hda-codec.o
@@ -54,6 +55,7 @@  obj-$(CONFIG_SND_HDA_CODEC_HDMI) += snd-hda-codec-hdmi.o
 obj-$(CONFIG_SND_HDA_SCODEC_CS35L41) += snd-hda-scodec-cs35l41.o
 obj-$(CONFIG_SND_HDA_SCODEC_CS35L41_I2C) += snd-hda-scodec-cs35l41-i2c.o
 obj-$(CONFIG_SND_HDA_SCODEC_CS35L41_SPI) += snd-hda-scodec-cs35l41-spi.o
+obj-$(CONFIG_SND_HDA_CS_DSP_CONTROLS) += snd-hda-cs-dsp-ctls.o
 
 # this must be the last entry after codec drivers;
 # otherwise the codec patches won't be hooked before the PCI probe
diff --git a/sound/pci/hda/hda_cs_dsp_ctl.c b/sound/pci/hda/hda_cs_dsp_ctl.c
new file mode 100644
index 000000000000..e94740c5557a
--- /dev/null
+++ b/sound/pci/hda/hda_cs_dsp_ctl.c
@@ -0,0 +1,364 @@ 
+// SPDX-License-Identifier: GPL-2.0
+//
+// HDA DSP ALSA Control Driver
+//
+// Copyright 2022 Cirrus Logic, Inc.
+//
+// Author: Stefan Binding <[e-mail]>
+
+#include <linux/module.h>
+#include <sound/soc.h>
+#include <linux/firmware/cirrus/cs_dsp.h>
+#include <linux/firmware/cirrus/wmfw.h>
+#include "hda_cs_dsp_ctl.h"
+
+struct hda_cs_dsp_coeff_ctl {
+	const char *name;
+	struct cs_dsp_coeff_ctl *cs_ctl;
+	struct snd_card *card;
+	struct soc_bytes_ext bytes_ext;
+	struct work_struct add_work;
+	struct work_struct remove_work;
+};
+
+static const char * const hda_cs_dsp_fw_text[HDA_CS_DSP_NUM_FW] = {
+	[HDA_CS_DSP_FW_SPK_PROT] = "Prot",
+	[HDA_CS_DSP_FW_SPK_CALI] = "Cali",
+	[HDA_CS_DSP_FW_SPK_DIAG] = "Diag",
+	[HDA_CS_DSP_FW_MISC] =     "Misc",
+};
+
+static inline struct hda_cs_dsp_coeff_ctl *bytes_ext_to_ctl(struct soc_bytes_ext *ext)
+{
+	return container_of(ext, struct hda_cs_dsp_coeff_ctl, bytes_ext);
+}
+
+static int hda_cs_dsp_coeff_info(struct snd_kcontrol *kctl, struct snd_ctl_elem_info *uinfo)
+{
+	struct soc_bytes_ext *bytes_ext =
+		(struct soc_bytes_ext *)kctl->private_value;
+	struct hda_cs_dsp_coeff_ctl *ctl = bytes_ext_to_ctl(bytes_ext);
+	struct cs_dsp_coeff_ctl *cs_ctl = ctl->cs_ctl;
+
+	switch (cs_ctl->type) {
+	case WMFW_CTL_TYPE_ACKED:
+		uinfo->type = SNDRV_CTL_ELEM_TYPE_INTEGER;
+		uinfo->value.integer.min = CS_DSP_ACKED_CTL_MIN_VALUE;
+		uinfo->value.integer.max = CS_DSP_ACKED_CTL_MAX_VALUE;
+		uinfo->value.integer.step = 1;
+		uinfo->count = 1;
+		break;
+	default:
+		uinfo->type = SNDRV_CTL_ELEM_TYPE_BYTES;
+		uinfo->count = cs_ctl->len;
+		break;
+	}
+
+	return 0;
+}
+
+static int hda_cs_dsp_coeff_put(struct snd_kcontrol *kctl, struct snd_ctl_elem_value *ucontrol)
+{
+	struct soc_bytes_ext *bytes_ext =
+		(struct soc_bytes_ext *)kctl->private_value;
+	struct hda_cs_dsp_coeff_ctl *ctl = bytes_ext_to_ctl(bytes_ext);
+	struct cs_dsp_coeff_ctl *cs_ctl = ctl->cs_ctl;
+	char *p = ucontrol->value.bytes.data;
+	int ret = 0;
+
+	mutex_lock(&cs_ctl->dsp->pwr_lock);
+	ret = cs_dsp_coeff_write_ctrl(cs_ctl, 0, p, cs_ctl->len);
+	mutex_unlock(&cs_ctl->dsp->pwr_lock);
+
+	return ret;
+}
+
+static int hda_cs_dsp_coeff_tlv_put(struct snd_kcontrol *kctl,
+				    const unsigned int __user *bytes, unsigned int size)
+{
+	struct soc_bytes_ext *bytes_ext =
+		(struct soc_bytes_ext *)kctl->private_value;
+	struct hda_cs_dsp_coeff_ctl *ctl = bytes_ext_to_ctl(bytes_ext);
+	struct cs_dsp_coeff_ctl *cs_ctl = ctl->cs_ctl;
+	int ret = 0;
+
+	mutex_lock(&cs_ctl->dsp->pwr_lock);
+
+	if (copy_from_user(cs_ctl->cache, bytes, size))
+		ret = -EFAULT;
+	else
+		ret = cs_dsp_coeff_write_ctrl(cs_ctl, 0, cs_ctl->cache, size);
+
+	mutex_unlock(&cs_ctl->dsp->pwr_lock);
+
+	return ret;
+}
+
+static int hda_cs_dsp_coeff_put_acked(struct snd_kcontrol *kctl,
+				      struct snd_ctl_elem_value *ucontrol)
+{
+	struct soc_bytes_ext *bytes_ext =
+		(struct soc_bytes_ext *)kctl->private_value;
+	struct hda_cs_dsp_coeff_ctl *ctl = bytes_ext_to_ctl(bytes_ext);
+	struct cs_dsp_coeff_ctl *cs_ctl = ctl->cs_ctl;
+	unsigned int val = ucontrol->value.integer.value[0];
+	int ret;
+
+	if (val == 0)
+		return 0;	/* 0 means no event */
+
+	mutex_lock(&cs_ctl->dsp->pwr_lock);
+
+	if (cs_ctl->enabled)
+		ret = cs_dsp_coeff_write_acked_control(cs_ctl, val);
+	else
+		ret = -EPERM;
+
+	mutex_unlock(&cs_ctl->dsp->pwr_lock);
+
+	return ret;
+}
+
+static int hda_cs_dsp_coeff_get(struct snd_kcontrol *kctl, struct snd_ctl_elem_value *ucontrol)
+{
+	struct soc_bytes_ext *bytes_ext =
+		(struct soc_bytes_ext *)kctl->private_value;
+	struct hda_cs_dsp_coeff_ctl *ctl = bytes_ext_to_ctl(bytes_ext);
+	struct cs_dsp_coeff_ctl *cs_ctl = ctl->cs_ctl;
+	char *p = ucontrol->value.bytes.data;
+	int ret;
+
+	mutex_lock(&cs_ctl->dsp->pwr_lock);
+	ret = cs_dsp_coeff_read_ctrl(cs_ctl, 0, p, cs_ctl->len);
+	mutex_unlock(&cs_ctl->dsp->pwr_lock);
+
+	return ret;
+}
+
+static int hda_cs_dsp_coeff_tlv_get(struct snd_kcontrol *kctl,
+				    unsigned int __user *bytes, unsigned int size)
+{
+	struct soc_bytes_ext *bytes_ext =
+		(struct soc_bytes_ext *)kctl->private_value;
+	struct hda_cs_dsp_coeff_ctl *ctl = bytes_ext_to_ctl(bytes_ext);
+	struct cs_dsp_coeff_ctl *cs_ctl = ctl->cs_ctl;
+	int ret = 0;
+
+	mutex_lock(&cs_ctl->dsp->pwr_lock);
+
+	ret = cs_dsp_coeff_read_ctrl(cs_ctl, 0, cs_ctl->cache, size);
+
+	if (!ret && copy_to_user(bytes, cs_ctl->cache, size))
+		ret = -EFAULT;
+
+	mutex_unlock(&cs_ctl->dsp->pwr_lock);
+
+	return ret;
+}
+
+static int hda_cs_dsp_coeff_get_acked(struct snd_kcontrol *kcontrol,
+				      struct snd_ctl_elem_value *ucontrol)
+{
+	/*
+	 * Although it's not useful to read an acked control, we must satisfy
+	 * user-side assumptions that all controls are readable and that a
+	 * write of the same value should be filtered out (it's valid to send
+	 * the same event number again to the firmware). We therefore return 0,
+	 * meaning "no event" so valid event numbers will always be a change
+	 */
+	ucontrol->value.integer.value[0] = 0;
+
+	return 0;
+}
+#define ADSP_MAX_STD_CTRL_SIZE               512
+
+static unsigned int wmfw_convert_flags(unsigned int in, unsigned int len)
+{
+	unsigned int out, rd, wr, vol;
+
+	if (len > ADSP_MAX_STD_CTRL_SIZE) {
+		rd = SNDRV_CTL_ELEM_ACCESS_TLV_READ;
+		wr = SNDRV_CTL_ELEM_ACCESS_TLV_WRITE;
+		vol = SNDRV_CTL_ELEM_ACCESS_VOLATILE;
+
+		out = SNDRV_CTL_ELEM_ACCESS_TLV_CALLBACK;
+	} else {
+		rd = SNDRV_CTL_ELEM_ACCESS_READ;
+		wr = SNDRV_CTL_ELEM_ACCESS_WRITE;
+		vol = SNDRV_CTL_ELEM_ACCESS_VOLATILE;
+
+		out = 0;
+	}
+
+	if (in) {
+		out |= rd;
+		if (in & WMFW_CTL_FLAG_WRITEABLE)
+			out |= wr;
+		if (in & WMFW_CTL_FLAG_VOLATILE)
+			out |= vol;
+	} else {
+		out |= rd | wr | vol;
+	}
+
+	return out;
+}
+
+static void hda_cs_dsp_ctl_add_work(struct work_struct *work)
+{
+	struct hda_cs_dsp_coeff_ctl *ctl = container_of(work,
+							struct hda_cs_dsp_coeff_ctl,
+							add_work);
+	struct cs_dsp_coeff_ctl *cs_ctl = ctl->cs_ctl;
+	struct snd_kcontrol_new *kcontrol;
+
+	kcontrol = kzalloc(sizeof(*kcontrol), GFP_KERNEL);
+	if (!kcontrol)
+		return;
+
+	kcontrol->name = ctl->name;
+	kcontrol->info = hda_cs_dsp_coeff_info;
+	kcontrol->iface = SNDRV_CTL_ELEM_IFACE_MIXER;
+	kcontrol->tlv.c = snd_soc_bytes_tlv_callback;
+	kcontrol->private_value = (unsigned long)&ctl->bytes_ext;
+	kcontrol->access = wmfw_convert_flags(cs_ctl->flags, cs_ctl->len);
+
+	switch (cs_ctl->type) {
+	case WMFW_CTL_TYPE_ACKED:
+		kcontrol->get = hda_cs_dsp_coeff_get_acked;
+		kcontrol->put = hda_cs_dsp_coeff_put_acked;
+		break;
+	default:
+		if (kcontrol->access & SNDRV_CTL_ELEM_ACCESS_TLV_CALLBACK) {
+			ctl->bytes_ext.max = cs_ctl->len;
+			ctl->bytes_ext.get = hda_cs_dsp_coeff_tlv_get;
+			ctl->bytes_ext.put = hda_cs_dsp_coeff_tlv_put;
+		} else {
+			kcontrol->get = hda_cs_dsp_coeff_get;
+			kcontrol->put = hda_cs_dsp_coeff_put;
+		}
+		break;
+	}
+
+	if (snd_ctl_add(ctl->card, snd_ctl_new1(kcontrol, NULL)))
+		dev_err(cs_ctl->dsp->dev, "Failed to add KControl: %s\n", kcontrol->name);
+	else
+		dev_dbg(cs_ctl->dsp->dev, "Added KControl: %s\n", kcontrol->name);
+
+	kfree(kcontrol);
+}
+
+int hda_cs_dsp_control_add(struct cs_dsp_coeff_ctl *cs_ctl, struct hda_cs_dsp_ctl_info *info)
+{
+	struct cs_dsp *cs_dsp = cs_ctl->dsp;
+	char name[SNDRV_CTL_ELEM_ID_NAME_MAXLEN];
+	struct hda_cs_dsp_coeff_ctl *ctl;
+	const char *region_name;
+	int ret;
+
+	if (cs_ctl->flags & WMFW_CTL_FLAG_SYS) {
+		dev_dbg(cs_dsp->dev, "cs_ctl->flags = WMFW_CTL_FLAG_SYS\n");
+		return 0;
+	}
+
+	region_name = cs_dsp_mem_region_name(cs_ctl->alg_region.type);
+	if (!region_name) {
+		dev_err(cs_dsp->dev, "Unknown region type: %d\n", cs_ctl->alg_region.type);
+		return -EINVAL;
+	}
+
+	switch (cs_dsp->fw_ver) {
+	case 0:
+	case 1:
+		ret = scnprintf(name, SNDRV_CTL_ELEM_ID_NAME_MAXLEN,
+				"%s %s %s %x", info->amp_name, cs_dsp->name, region_name,
+				cs_ctl->alg_region.alg);
+		break;
+	case 2:
+		ret = scnprintf(name, SNDRV_CTL_ELEM_ID_NAME_MAXLEN,
+				"%s %s%c %.12s %x", info->amp_name, cs_dsp->name, *region_name,
+				hda_cs_dsp_fw_text[info->fw_type], cs_ctl->alg_region.alg);
+		break;
+	default:
+		ret = scnprintf(name, SNDRV_CTL_ELEM_ID_NAME_MAXLEN,
+				"%s %s %.12s %x", info->amp_name, cs_dsp->name,
+				hda_cs_dsp_fw_text[info->fw_type], cs_ctl->alg_region.alg);
+		break;
+	}
+
+	if (cs_ctl->subname) {
+		int avail = SNDRV_CTL_ELEM_ID_NAME_MAXLEN - ret - 2;
+		int skip = 0;
+
+		/* Truncate the subname from the start if it is too long */
+		if (cs_ctl->subname_len > avail)
+			skip = cs_ctl->subname_len - avail;
+
+		snprintf(name + ret, SNDRV_CTL_ELEM_ID_NAME_MAXLEN - ret,
+			 " %.*s", cs_ctl->subname_len - skip, cs_ctl->subname + skip);
+	}
+
+	ctl = kzalloc(sizeof(*ctl), GFP_KERNEL);
+	if (!ctl)
+		return -ENOMEM;
+	ctl->cs_ctl = cs_ctl;
+	ctl->card = info->card;
+
+	ctl->name = kmemdup(name, strlen(name) + 1, GFP_KERNEL);
+	if (!ctl->name) {
+		ret = -ENOMEM;
+		dev_err(cs_dsp->dev, "Cannot save ctl name\n");
+		goto err_ctl;
+	}
+
+	cs_ctl->priv = ctl;
+
+	INIT_WORK(&ctl->add_work, hda_cs_dsp_ctl_add_work);
+	schedule_work(&ctl->add_work);
+
+	return 0;
+
+err_ctl:
+	dev_err(cs_dsp->dev, "Error adding control: %s\n", name);
+	kfree(ctl);
+	return ret;
+}
+EXPORT_SYMBOL_NS_GPL(hda_cs_dsp_control_add, SND_HDA_CS_DSP_CONTROLS);
+
+int hda_cs_dsp_remove_kcontrol(struct snd_card *card, const char *name)
+{
+	struct snd_kcontrol *kctl;
+
+	list_for_each_entry(kctl, &card->controls, list)
+		if (!strncmp(kctl->id.name, name, sizeof(kctl->id.name)))
+			return snd_ctl_remove_id(card, &kctl->id);
+
+	return -EINVAL;
+}
+EXPORT_SYMBOL_NS_GPL(hda_cs_dsp_remove_kcontrol, SND_HDA_CS_DSP_CONTROLS);
+
+static void hda_cs_dsp_ctl_del_work(struct work_struct *work)
+{
+	struct hda_cs_dsp_coeff_ctl *ctl = container_of(work,
+							struct hda_cs_dsp_coeff_ctl,
+							remove_work);
+
+	cancel_work_sync(&ctl->add_work);
+
+	hda_cs_dsp_remove_kcontrol(ctl->card, ctl->name);
+
+	kfree(ctl->name);
+	kfree(ctl);
+}
+
+void hda_cs_dsp_control_remove(struct cs_dsp_coeff_ctl *cs_ctl)
+{
+	struct hda_cs_dsp_coeff_ctl *ctl = cs_ctl->priv;
+
+	INIT_WORK(&ctl->remove_work, hda_cs_dsp_ctl_del_work);
+	schedule_work(&ctl->remove_work);
+}
+EXPORT_SYMBOL_NS_GPL(hda_cs_dsp_control_remove, SND_HDA_CS_DSP_CONTROLS);
+
+MODULE_DESCRIPTION("CS_DSP ALSA Control HDA Library");
+MODULE_AUTHOR("Stefan Binding, <[e-mail]>");
+MODULE_LICENSE("GPL");
diff --git a/sound/pci/hda/hda_cs_dsp_ctl.h b/sound/pci/hda/hda_cs_dsp_ctl.h
new file mode 100644
index 000000000000..3c90312b45d6
--- /dev/null
+++ b/sound/pci/hda/hda_cs_dsp_ctl.h
@@ -0,0 +1,34 @@ 
+/* SPDX-License-Identifier: GPL-2.0
+ *
+ * HDA DSP ALSA Control Driver
+ *
+ * Copyright 2022 Cirrus Logic, Inc.
+ *
+ * Author: Stefan Binding <[e-mail]>
+ */
+
+#ifndef __HDA_CS_DSP_CTL_H__
+#define __HDA_CS_DSP_CTL_H__
+
+#include <sound/soc.h>
+#include <linux/firmware/cirrus/cs_dsp.h>
+
+enum hda_cs_dsp_fw_id {
+	HDA_CS_DSP_FW_SPK_PROT,
+	HDA_CS_DSP_FW_SPK_CALI,
+	HDA_CS_DSP_FW_SPK_DIAG,
+	HDA_CS_DSP_FW_MISC,
+	HDA_CS_DSP_NUM_FW
+};
+
+struct hda_cs_dsp_ctl_info {
+	struct snd_card *card;
+	enum hda_cs_dsp_fw_id fw_type;
+	const char *amp_name;
+};
+
+int hda_cs_dsp_control_add(struct cs_dsp_coeff_ctl *cs_ctl, struct hda_cs_dsp_ctl_info *info);
+void hda_cs_dsp_control_remove(struct cs_dsp_coeff_ctl *cs_ctl);
+int hda_cs_dsp_remove_kcontrol(struct snd_card *card, const char *name);
+
+#endif /*__HDA_CS_DSP_CTL_H__*/