On Mon, 09 May 2022 23:46:47 +0200,
Vitaly Rodionov wrote:
>
> From: Stefan Binding <sbinding@xxxxxxxxxxxxxxxxxxxxx>
>
> 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 <sbinding@xxxxxxxxxxxxxxxxxxxxx>
> Signed-off-by: Vitaly Rodionov <vitalyr@xxxxxxxxxxxxxxxxxxxxx>
> ---
> 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 <sbinding@xxxxxxxxxxxxxxxxxxxxx>
> +
> +#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, <sbinding@xxxxxxxxxxxxxxxxxxxxx>");
> +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 <sbinding@xxxxxxxxxxxxxxxxxxxxx>
> + */
> +
> +#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
>
