diff mbox series

[v8,01/14] ALSA: hda: hda_cs_dsp_ctl: Add Library to support CS_DSP ALSA controls

Message ID 20220630002335.366545-2-vitalyr@opensource.cirrus.com (mailing list archive)
State New, archived
Headers show
Series ALSA: hda: cirrus: Add initial DSP support and firmware loading | expand

Commit Message

Vitaly Rodionov June 30, 2022, 12:23 a.m. UTC
From: Stefan Binding <sbinding@opensource.cirrus.com>

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 <sbinding@opensource.cirrus.com>
Signed-off-by: Vitaly Rodionov <vitalyr@opensource.cirrus.com>
---
 MAINTAINERS                    |   1 +
 sound/pci/hda/Kconfig          |   4 +
 sound/pci/hda/Makefile         |   2 +
 sound/pci/hda/hda_cs_dsp_ctl.c | 193 +++++++++++++++++++++++++++++++++
 sound/pci/hda/hda_cs_dsp_ctl.h |  33 ++++++
 5 files changed, 233 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 July 14, 2022, 9:03 a.m. UTC | #1
On Thu, 30 Jun 2022 02:23:22 +0200,
Vitaly Rodionov wrote:
> --- 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

This change alone doesn't give anything useful, unfortunately.

The above form (without prompt) is basically only to be "selected" by
others.  And when selected, the "depends" there is just ignored, so
it's useless.

That is, a proper way would be something like:

config SND_HDA_CS_DSP_CONTROLS
	tristate

config SND_HDA_SCODEC_CS35L41
	....
	select SND_HDA_CS_DSP_CONTROLS if CS_DSP

... if you want / need to enable CONFIG_SND_HDA_CS_DSP_CONTROLS
conditionally.


thanks,

Takashi
Takashi Iwai July 14, 2022, 9:06 a.m. UTC | #2
On Thu, 14 Jul 2022 11:03:16 +0200,
Takashi Iwai wrote:
> 
> On Thu, 30 Jun 2022 02:23:22 +0200,
> Vitaly Rodionov wrote:
> > --- 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
> 
> This change alone doesn't give anything useful, unfortunately.
> 
> The above form (without prompt) is basically only to be "selected" by
> others.  And when selected, the "depends" there is just ignored, so
> it's useless.
> 
> That is, a proper way would be something like:
> 
> config SND_HDA_CS_DSP_CONTROLS
> 	tristate
> 
> config SND_HDA_SCODEC_CS35L41
> 	....
> 	select SND_HDA_CS_DSP_CONTROLS if CS_DSP
> 
> ... if you want / need to enable CONFIG_SND_HDA_CS_DSP_CONTROLS
> conditionally.

And now I see that the patch 4 has those selects (but select both
SND_HDA_CS_DSP_CONTROLS and CS_DSP).  So what we need here is to drop
the superfluous "depends on CS_DSP" and mention in the description
that the Kconfig will be actually enabled in the later patch.


thanks,

Takashi
Takashi Iwai July 14, 2022, 9:26 a.m. UTC | #3
On Thu, 14 Jul 2022 11:06:16 +0200,
Takashi Iwai wrote:
> 
> On Thu, 14 Jul 2022 11:03:16 +0200,
> Takashi Iwai wrote:
> > 
> > On Thu, 30 Jun 2022 02:23:22 +0200,
> > Vitaly Rodionov wrote:
> > > --- 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
> > 
> > This change alone doesn't give anything useful, unfortunately.
> > 
> > The above form (without prompt) is basically only to be "selected" by
> > others.  And when selected, the "depends" there is just ignored, so
> > it's useless.
> > 
> > That is, a proper way would be something like:
> > 
> > config SND_HDA_CS_DSP_CONTROLS
> > 	tristate
> > 
> > config SND_HDA_SCODEC_CS35L41
> > 	....
> > 	select SND_HDA_CS_DSP_CONTROLS if CS_DSP
> > 
> > ... if you want / need to enable CONFIG_SND_HDA_CS_DSP_CONTROLS
> > conditionally.
> 
> And now I see that the patch 4 has those selects (but select both
> SND_HDA_CS_DSP_CONTROLS and CS_DSP).  So what we need here is to drop
> the superfluous "depends on CS_DSP" and mention in the description
> that the Kconfig will be actually enabled in the later patch.

But no need for resubmission; I fixed this locally.

So I applied up to the patch 8.  At patch 9, it can't build because of
the lack of ASoC side change.

Still waiting for *ANY* reaction from Mark...  Sigh.


thanks,

Takashi
diff mbox series

Patch

diff --git a/MAINTAINERS b/MAINTAINERS
index 3547b6eddbab..4438e206d648 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -4803,6 +4803,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..74e2c5bd1b08
--- /dev/null
+++ b/sound/pci/hda/hda_cs_dsp_ctl.c
@@ -0,0 +1,193 @@ 
+// SPDX-License-Identifier: GPL-2.0
+//
+// HDA DSP ALSA Control Driver
+//
+// Copyright 2022 Cirrus Logic, Inc.
+//
+// Author: Stefan Binding <sbinding@opensource.cirrus.com>
+
+#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"
+
+#define ADSP_MAX_STD_CTRL_SIZE               512
+
+struct hda_cs_dsp_coeff_ctl {
+	struct cs_dsp_coeff_ctl *cs_ctl;
+	struct snd_card *card;
+	struct snd_kcontrol *kctl;
+};
+
+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 int hda_cs_dsp_coeff_info(struct snd_kcontrol *kctl, struct snd_ctl_elem_info *uinfo)
+{
+	struct hda_cs_dsp_coeff_ctl *ctl = (struct hda_cs_dsp_coeff_ctl *)snd_kcontrol_chip(kctl);
+	struct cs_dsp_coeff_ctl *cs_ctl = ctl->cs_ctl;
+
+	uinfo->type = SNDRV_CTL_ELEM_TYPE_BYTES;
+	uinfo->count = cs_ctl->len;
+
+	return 0;
+}
+
+static int hda_cs_dsp_coeff_put(struct snd_kcontrol *kctl, struct snd_ctl_elem_value *ucontrol)
+{
+	struct hda_cs_dsp_coeff_ctl *ctl = (struct hda_cs_dsp_coeff_ctl *)snd_kcontrol_chip(kctl);
+	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_get(struct snd_kcontrol *kctl, struct snd_ctl_elem_value *ucontrol)
+{
+	struct hda_cs_dsp_coeff_ctl *ctl = (struct hda_cs_dsp_coeff_ctl *)snd_kcontrol_chip(kctl);
+	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 unsigned int wmfw_convert_flags(unsigned int in)
+{
+	unsigned int out, rd, wr, vol;
+
+	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 int hda_cs_dsp_add_kcontrol(struct hda_cs_dsp_coeff_ctl *ctl, const char *name)
+{
+	struct cs_dsp_coeff_ctl *cs_ctl = ctl->cs_ctl;
+	struct snd_kcontrol_new kcontrol = {0};
+	struct snd_kcontrol *kctl;
+	int ret = 0;
+
+	if (cs_ctl->len > ADSP_MAX_STD_CTRL_SIZE) {
+		dev_err(cs_ctl->dsp->dev, "KControl %s: length %zu exceeds maximum %d\n", name,
+			cs_ctl->len, ADSP_MAX_STD_CTRL_SIZE);
+		return -EINVAL;
+	}
+
+	kcontrol.name = name;
+	kcontrol.info = hda_cs_dsp_coeff_info;
+	kcontrol.iface = SNDRV_CTL_ELEM_IFACE_MIXER;
+	kcontrol.access = wmfw_convert_flags(cs_ctl->flags);
+	kcontrol.get = hda_cs_dsp_coeff_get;
+	kcontrol.put = hda_cs_dsp_coeff_put;
+
+	/* Save ctl inside private_data, ctl is owned by cs_dsp,
+	 * and will be freed when cs_dsp removes the control */
+	kctl = snd_ctl_new1(&kcontrol, (void *)ctl);
+	if (!kctl) {
+		ret = -ENOMEM;
+		return ret;
+	}
+
+	ret = snd_ctl_add(ctl->card, kctl);
+	if (ret) {
+		dev_err(cs_ctl->dsp->dev, "Failed to add KControl %s = %d\n", kcontrol.name, ret);
+		return ret;
+	}
+
+	dev_dbg(cs_ctl->dsp->dev, "Added KControl: %s\n", kcontrol.name);
+	ctl->kctl = kctl;
+
+	return 0;
+}
+
+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)
+		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;
+	}
+
+	ret = scnprintf(name, SNDRV_CTL_ELEM_ID_NAME_MAXLEN, "%s %s %.12s %x", info->device_name,
+			cs_dsp->name, hda_cs_dsp_fw_text[info->fw_type], cs_ctl->alg_region.alg);
+
+	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;
+	cs_ctl->priv = ctl;
+
+	ret = hda_cs_dsp_add_kcontrol(ctl, name);
+	if (ret) {
+		dev_err(cs_dsp->dev, "Error (%d) adding control %s\n", ret, name);
+		kfree(ctl);
+		return ret;
+	}
+
+	return 0;
+}
+EXPORT_SYMBOL_NS_GPL(hda_cs_dsp_control_add, SND_HDA_CS_DSP_CONTROLS);
+
+void hda_cs_dsp_control_remove(struct cs_dsp_coeff_ctl *cs_ctl)
+{
+	struct hda_cs_dsp_coeff_ctl *ctl = cs_ctl->priv;
+
+	kfree(ctl);
+}
+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@opensource.cirrus.com>");
+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..1c6d0fc9a2cc
--- /dev/null
+++ b/sound/pci/hda/hda_cs_dsp_ctl.h
@@ -0,0 +1,33 @@ 
+/* SPDX-License-Identifier: GPL-2.0
+ *
+ * HDA DSP ALSA Control Driver
+ *
+ * Copyright 2022 Cirrus Logic, Inc.
+ *
+ * Author: Stefan Binding <sbinding@opensource.cirrus.com>
+ */
+
+#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 *device_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);
+
+#endif /*__HDA_CS_DSP_CTL_H__*/