All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] sound: pci/rme9652 - implement and expose controls for output
@ 2021-01-31 21:37 fassl
  2021-02-01  8:42 ` Takashi Iwai
  0 siblings, 1 reply; 6+ messages in thread
From: fassl @ 2021-01-31 21:37 UTC (permalink / raw)
  To: Jaroslav Kysela, Takashi Iwai, Romain Perier, Colin Ian King,
	Allen Pais, alsa-devel

From 980aa253c17d233966e05a43f3611693e2e02040 Mon Sep 17 00:00:00 2001
From: Jasmin Fazlic <superfassl@gmail.com>
Date: Sun, 31 Jan 2021 22:17:22 +0100
Subject: [PATCH] sound: pci/rme9652 - implement and expose controls for output
 loopback

- so far only tested and enabled for RME HDSP9632

Signed-off-by: Jasmin Fazlic <superfassl@gmail.com>
---
 sound/pci/rme9652/hdsp.c | 85 +++++++++++++++++++++++++++++++++++++++-
 1 file changed, 84 insertions(+), 1 deletion(-)

diff --git a/sound/pci/rme9652/hdsp.c b/sound/pci/rme9652/hdsp.c
index cea53a878c36..7e832a502cea 100644
--- a/sound/pci/rme9652/hdsp.c
+++ b/sound/pci/rme9652/hdsp.c
@@ -469,6 +469,7 @@ struct hdsp {
     unsigned char          qs_out_channels;
     unsigned char         ds_out_channels;
     unsigned char         ss_out_channels;
+    u32                   io_loopback;          /* output loopback channel states*/
 
     struct snd_dma_buffer capture_dma_buf;
     struct snd_dma_buffer playback_dma_buf;
@@ -3253,6 +3254,71 @@ static const struct snd_kcontrol_new snd_hdsp_96xx_aeb =
             HDSP_AnalogExtensionBoard);
 static struct snd_kcontrol_new snd_hdsp_adat_sync_check = HDSP_ADAT_SYNC_CHECK;
 
+
+static bool hdsp_loopback_get(struct hdsp *const hdsp, const u8 channel)
+{
+    return hdsp->io_loopback & (1 << channel);
+}
+
+static int hdsp_loopback_set(struct hdsp *const hdsp, const u8 channel, const bool enable)
+{
+    if (hdsp_loopback_get(hdsp, channel) == enable)
+        return 0;
+
+    hdsp->io_loopback ^= (1 << channel);
+
+    hdsp_write(hdsp, HDSP_inputEnable + (4 * (hdsp->max_channels + channel)), enable);
+
+    return 1;
+}
+
+static int snd_hdsp_loopback_info(struct snd_kcontrol *const kcontrol,
+                  struct snd_ctl_elem_info *const uinfo)
+{
+    uinfo->type = SNDRV_CTL_ELEM_TYPE_BOOLEAN;
+    uinfo->count = 1;
+    uinfo->value.integer.min = 0;
+    uinfo->value.integer.max = 1;
+    return 0;
+}
+
+static int snd_hdsp_loopback_get(struct snd_kcontrol *const kcontrol,
+                 struct snd_ctl_elem_value *const ucontrol)
+{
+    struct hdsp *const hdsp = snd_kcontrol_chip(kcontrol);
+    const u8 channel = kcontrol->id.index;
+
+    if (channel >= hdsp->max_channels)
+        return -ENOENT;
+
+    ucontrol->value.integer.value[0] = hdsp_loopback_get(hdsp, channel);
+
+    return 0;
+}
+
+static int snd_hdsp_loopback_put(struct snd_kcontrol *const kcontrol,
+                 struct snd_ctl_elem_value *const ucontrol)
+{
+    struct hdsp *const hdsp = snd_kcontrol_chip(kcontrol);
+    const u8 channel = ucontrol->id.index;
+    const bool enable = ucontrol->value.integer.value[0] & 1;
+
+    if (channel >= hdsp->max_channels)
+        return -ENOENT;
+
+    return hdsp_loopback_set(hdsp, channel, enable);
+}
+
+static struct snd_kcontrol_new snd_hdsp_loopback_control = {
+    .iface = SNDRV_CTL_ELEM_IFACE_HWDEP,
+    .name = "Output Loopback",
+    .index = 0,
+    .access = SNDRV_CTL_ELEM_ACCESS_READWRITE,
+    .info = snd_hdsp_loopback_info,
+    .get = snd_hdsp_loopback_get,
+    .put = snd_hdsp_loopback_put
+};
+
 static int snd_hdsp_create_controls(struct snd_card *card, struct hdsp *hdsp)
 {
     unsigned int idx;
@@ -3297,6 +3363,17 @@ static int snd_hdsp_create_controls(struct snd_card *card, struct hdsp *hdsp)
         }
     }
 
+    /* Output loopback controls for H9632 cards */
+    if (hdsp->io_type == H9632) {
+        for (idx = 0; idx < hdsp->max_channels; idx++) {
+            snd_hdsp_loopback_control.index = idx;
+            kctl = snd_ctl_new1(&snd_hdsp_loopback_control, hdsp);
+            err = snd_ctl_add(card, kctl);
+            if (err < 0)
+                return err;
+        }
+    }
+
     /* AEB control for H96xx card */
     if (hdsp->io_type == H9632 || hdsp->io_type == H9652) {
         if ((err = snd_ctl_add(card, kctl = snd_ctl_new1(&snd_hdsp_96xx_aeb, hdsp))) < 0)
@@ -4956,7 +5033,7 @@ static int snd_hdsp_enable_io (struct hdsp *hdsp)
 
 static void snd_hdsp_initialize_channels(struct hdsp *hdsp)
 {
-    int status, aebi_channels, aebo_channels;
+    int status, aebi_channels, aebo_channels, i;
 
     switch (hdsp->io_type) {
     case Digiface:
@@ -4983,6 +5060,12 @@ static void snd_hdsp_initialize_channels(struct hdsp *hdsp)
         hdsp->ss_out_channels = H9632_SS_CHANNELS+aebo_channels;
         hdsp->ds_out_channels = H9632_DS_CHANNELS+aebo_channels;
         hdsp->qs_out_channels = H9632_QS_CHANNELS+aebo_channels;
+        /* Disable loopback of output channels, as the set function
+         * only sets on a change we fake all bits (channels) as enabled.
+         */
+        hdsp->io_loopback = 0xffffffff;
+        for (i = 0; i < hdsp->max_channels; ++i)
+            hdsp_loopback_set(hdsp, i, false);
         break;
 
     case Multiface:
-- 
2.27.0



^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] sound: pci/rme9652 - implement and expose controls for output
  2021-01-31 21:37 [PATCH] sound: pci/rme9652 - implement and expose controls for output fassl
@ 2021-02-01  8:42 ` Takashi Iwai
  2021-02-01 13:50   ` fassl
  0 siblings, 1 reply; 6+ messages in thread
From: Takashi Iwai @ 2021-02-01  8:42 UTC (permalink / raw)
  To: fassl; +Cc: alsa-devel, Allen Pais, Takashi Iwai, Colin Ian King,
	Romain Perier

On Sun, 31 Jan 2021 22:37:32 +0100,
fassl wrote:
> 
> >From 980aa253c17d233966e05a43f3611693e2e02040 Mon Sep 17 00:00:00 2001
> From: Jasmin Fazlic <superfassl@gmail.com>
> Date: Sun, 31 Jan 2021 22:17:22 +0100
> Subject: [PATCH] sound: pci/rme9652 - implement and expose controls for output
>  loopback
> 
> - so far only tested and enabled for RME HDSP9632
> 
> Signed-off-by: Jasmin Fazlic <superfassl@gmail.com>

Could you give a bit more description?  What exactly the additions do?

About the code change:
> +static int snd_hdsp_loopback_info(struct snd_kcontrol *const kcontrol,
> +                  struct snd_ctl_elem_info *const uinfo)
> +{
> +    uinfo->type = SNDRV_CTL_ELEM_TYPE_BOOLEAN;
> +    uinfo->count = 1;
> +    uinfo->value.integer.min = 0;
> +    uinfo->value.integer.max = 1;
> +    return 0;
> +}

This is identical with snd_ctl_boolean_mono_info.

> +static struct snd_kcontrol_new snd_hdsp_loopback_control = {
> +    .iface = SNDRV_CTL_ELEM_IFACE_HWDEP,
> +    .name = "Output Loopback",
> +    .index = 0,
> +    .access = SNDRV_CTL_ELEM_ACCESS_READWRITE,
> +    .info = snd_hdsp_loopback_info,
> +    .get = snd_hdsp_loopback_get,
> +    .put = snd_hdsp_loopback_put
> +};

IFACE_HWDEP is used in hdsp driver because it's a sort of special
mixer setup.  I don't know your mixer stuff need to follow that.

>  static int snd_hdsp_create_controls(struct snd_card *card, struct hdsp *hdsp)
>  {
>      unsigned int idx;
> @@ -3297,6 +3363,17 @@ static int snd_hdsp_create_controls(struct snd_card *card, struct hdsp *hdsp)
>          }
>      }
>  
> +    /* Output loopback controls for H9632 cards */
> +    if (hdsp->io_type == H9632) {
> +        for (idx = 0; idx < hdsp->max_channels; idx++) {
> +            snd_hdsp_loopback_control.index = idx;
> +            kctl = snd_ctl_new1(&snd_hdsp_loopback_control, hdsp);
> +            err = snd_ctl_add(card, kctl);
> +            if (err < 0)
> +                return err;
> +        }
> +    }

snd_ctl_new() can be used for allocating the multiple instances in one
kcontrol.  If the kctl object is created in this way, you'd just need
to change your code to retrieve the kctl index via
snd_ctl_get_ioff() instead of referring to kctl->index directly.


thanks,

Takashi

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] sound: pci/rme9652 - implement and expose controls for output
  2021-02-01  8:42 ` Takashi Iwai
@ 2021-02-01 13:50   ` fassl
  2021-02-01 13:52     ` Takashi Iwai
  0 siblings, 1 reply; 6+ messages in thread
From: fassl @ 2021-02-01 13:50 UTC (permalink / raw)
  To: Takashi Iwai
  Cc: alsa-devel, Allen Pais, Takashi Iwai, Colin Ian King,
	Romain Perier

On 01.02.21 09:42, Takashi Iwai wrote:

> snd_ctl_new() can be used for allocating the multiple instances in one
> kcontrol.  If the kctl object is created in this way, you'd just need
> to change your code to retrieve the kctl index via
> snd_ctl_get_ioff() instead of referring to kctl->index directly.
>
Hi, thanks for the response. snd_ctl_new() is static and unexported,
should I provide this patch (below) as well? And if so, together with
this one or separately?

Thanks, best regards


diff --git a/include/sound/control.h b/include/sound/control.h
index 77d9fa10812d..a5920090fdca 100644
--- a/include/sound/control.h
+++ b/include/sound/control.h
@@ -116,6 +116,7 @@ typedef int (*snd_kctl_ioctl_func_t) (struct snd_card * card,
 
 void snd_ctl_notify(struct snd_card * card, unsigned int mask, struct snd_ctl_elem_id * id);
 
+int snd_ctl_new(struct snd_kcontrol **kctl, unsigned int count, unsigned int access, struct snd_ctl_file *file);
 struct snd_kcontrol *snd_ctl_new1(const struct snd_kcontrol_new * kcontrolnew, void * private_data);
 void snd_ctl_free_one(struct snd_kcontrol * kcontrol);
 int snd_ctl_add(struct snd_card * card, struct snd_kcontrol * kcontrol);
diff --git a/sound/core/control.c b/sound/core/control.c
index 3b44378b9dec..066288218b81 100644
--- a/sound/core/control.c
+++ b/sound/core/control.c
@@ -194,7 +194,7 @@ EXPORT_SYMBOL(snd_ctl_notify);
  *
  * Return: 0 on success, error code on failure
  */
-static int snd_ctl_new(struct snd_kcontrol **kctl, unsigned int count,
+int snd_ctl_new(struct snd_kcontrol **kctl, unsigned int count,
                       unsigned int access, struct snd_ctl_file *file)
 {
        unsigned int idx;
@@ -214,6 +214,7 @@ static int snd_ctl_new(struct snd_kcontrol **kctl, unsigned int count,
 
        return 0;
 }
+EXPORT_SYMBOL(snd_ctl_new);


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] sound: pci/rme9652 - implement and expose controls for output
  2021-02-01 13:50   ` fassl
@ 2021-02-01 13:52     ` Takashi Iwai
  2021-02-01 14:28       ` fassl
  0 siblings, 1 reply; 6+ messages in thread
From: Takashi Iwai @ 2021-02-01 13:52 UTC (permalink / raw)
  To: fassl; +Cc: alsa-devel, Allen Pais, Takashi Iwai, Colin Ian King,
	Romain Perier

On Mon, 01 Feb 2021 14:50:08 +0100,
fassl wrote:
> 
> On 01.02.21 09:42, Takashi Iwai wrote:
> 
> > snd_ctl_new() can be used for allocating the multiple instances in one
> > kcontrol.  If the kctl object is created in this way, you'd just need
> > to change your code to retrieve the kctl index via
> > snd_ctl_get_ioff() instead of referring to kctl->index directly.
> >
> Hi, thanks for the response. snd_ctl_new() is static and unexported,
> should I provide this patch (below) as well? And if so, together with
> this one or separately?

My bad, it's the same snd_ctl_new1().  You just need to set counter
field of snd_kcontrol_new to create multiple instances in a single
shot.


Takashi

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] sound: pci/rme9652 - implement and expose controls for output
  2021-02-01 13:52     ` Takashi Iwai
@ 2021-02-01 14:28       ` fassl
  2021-02-02  9:36         ` Takashi Iwai
  0 siblings, 1 reply; 6+ messages in thread
From: fassl @ 2021-02-01 14:28 UTC (permalink / raw)
  To: Takashi Iwai
  Cc: alsa-devel, Allen Pais, Takashi Iwai, Colin Ian King,
	Romain Perier

From a9072f701f945ba00c42f612e85830395d81450d Mon Sep 17 00:00:00 2001
From: Jasmin Fazlic <superfassl@gmail.com>
Date: Sun, 31 Jan 2021 22:17:22 +0100
Subject: [PATCH] sound: pci/rme9652 - hardware output loopback

Output loopback is a feature where you can record what you hear.
The HDSP series of the RME interfaces provides this functionality
at the hardware level and this patch exposes controls to enable or
disable it per output (playback) channel.

This probably works on other cards but due to a lack of hardware
it is only tested and enabled for the HDSP9632 card with this patch.

Should this patch be accepted a separate patch will be posted to
https://github.com/alsa-project/alsa-tools/tree/master/hdspmixer
which adds "LPBK" buttons to each ouput in the playback strip for
the user to be able to control this feature from the user land.
Users from Windows tool TotalMixFX should be familiar with this.

Signed-off-by: Jasmin Fazlic <superfassl@gmail.com>
---
 sound/pci/rme9652/hdsp.c | 74 +++++++++++++++++++++++++++++++++++++++-
 1 file changed, 73 insertions(+), 1 deletion(-)

diff --git a/sound/pci/rme9652/hdsp.c b/sound/pci/rme9652/hdsp.c
index cea53a878c36..6d9029333a12 100644
--- a/sound/pci/rme9652/hdsp.c
+++ b/sound/pci/rme9652/hdsp.c
@@ -469,6 +469,7 @@ struct hdsp {
 	unsigned char	      qs_out_channels;
 	unsigned char         ds_out_channels;
 	unsigned char         ss_out_channels;
+	u32                   io_loopback;          /* output loopback channel states*/
 
 	struct snd_dma_buffer capture_dma_buf;
 	struct snd_dma_buffer playback_dma_buf;
@@ -3253,6 +3254,60 @@ static const struct snd_kcontrol_new snd_hdsp_96xx_aeb =
 			HDSP_AnalogExtensionBoard);
 static struct snd_kcontrol_new snd_hdsp_adat_sync_check = HDSP_ADAT_SYNC_CHECK;
 
+
+static bool hdsp_loopback_get(struct hdsp *const hdsp, const u8 channel)
+{
+	return hdsp->io_loopback & (1 << channel);
+}
+
+static int hdsp_loopback_set(struct hdsp *const hdsp, const u8 channel, const bool enable)
+{
+	if (hdsp_loopback_get(hdsp, channel) == enable)
+		return 0;
+
+	hdsp->io_loopback ^= (1 << channel);
+
+	hdsp_write(hdsp, HDSP_inputEnable + (4 * (hdsp->max_channels + channel)), enable);
+
+	return 1;
+}
+
+static int snd_hdsp_loopback_get(struct snd_kcontrol *const kcontrol,
+				 struct snd_ctl_elem_value *const ucontrol)
+{
+	struct hdsp *const hdsp = snd_kcontrol_chip(kcontrol);
+	const u8 channel = snd_ctl_get_ioff(kcontrol, &ucontrol->id);
+
+	if (channel >= hdsp->max_channels)
+		return -ENOENT;
+
+	ucontrol->value.integer.value[0] = hdsp_loopback_get(hdsp, channel);
+
+	return 0;
+}
+
+static int snd_hdsp_loopback_put(struct snd_kcontrol *const kcontrol,
+				 struct snd_ctl_elem_value *const ucontrol)
+{
+	struct hdsp *const hdsp = snd_kcontrol_chip(kcontrol);
+	const u8 channel = snd_ctl_get_ioff(kcontrol, &ucontrol->id);
+	const bool enable = ucontrol->value.integer.value[0] & 1;
+
+	if (channel >= hdsp->max_channels)
+		return -ENOENT;
+
+	return hdsp_loopback_set(hdsp, channel, enable);
+}
+
+static struct snd_kcontrol_new snd_hdsp_loopback_control = {
+	.iface = SNDRV_CTL_ELEM_IFACE_HWDEP,
+	.name = "Output Loopback",
+	.access = SNDRV_CTL_ELEM_ACCESS_READWRITE,
+	.info = snd_ctl_boolean_mono_info,
+	.get = snd_hdsp_loopback_get,
+	.put = snd_hdsp_loopback_put
+};
+
 static int snd_hdsp_create_controls(struct snd_card *card, struct hdsp *hdsp)
 {
 	unsigned int idx;
@@ -3297,6 +3352,17 @@ static int snd_hdsp_create_controls(struct snd_card *card, struct hdsp *hdsp)
 		}
 	}
 
+	/* Output loopback controls for H9632 cards */
+	if (hdsp->io_type == H9632) {
+		snd_hdsp_loopback_control.count = hdsp->max_channels;
+		kctl = snd_ctl_new1(&snd_hdsp_loopback_control, hdsp);
+		if (kctl == NULL)
+			return -ENOMEM;
+		err = snd_ctl_add(card, kctl);
+		if (err < 0)
+			return err;
+	}
+
 	/* AEB control for H96xx card */
 	if (hdsp->io_type == H9632 || hdsp->io_type == H9652) {
 		if ((err = snd_ctl_add(card, kctl = snd_ctl_new1(&snd_hdsp_96xx_aeb, hdsp))) < 0)
@@ -4956,7 +5022,7 @@ static int snd_hdsp_enable_io (struct hdsp *hdsp)
 
 static void snd_hdsp_initialize_channels(struct hdsp *hdsp)
 {
-	int status, aebi_channels, aebo_channels;
+	int status, aebi_channels, aebo_channels, i;
 
 	switch (hdsp->io_type) {
 	case Digiface:
@@ -4983,6 +5049,12 @@ static void snd_hdsp_initialize_channels(struct hdsp *hdsp)
 		hdsp->ss_out_channels = H9632_SS_CHANNELS+aebo_channels;
 		hdsp->ds_out_channels = H9632_DS_CHANNELS+aebo_channels;
 		hdsp->qs_out_channels = H9632_QS_CHANNELS+aebo_channels;
+		/* Disable loopback of output channels, as the set function
+		 * only sets on a change we fake all bits (channels) as enabled.
+		 */
+		hdsp->io_loopback = 0xffffffff;
+		for (i = 0; i < hdsp->max_channels; ++i)
+			hdsp_loopback_set(hdsp, i, false);
 		break;
 
 	case Multiface:
-- 
2.27.0



^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH] sound: pci/rme9652 - implement and expose controls for output
  2021-02-01 14:28       ` fassl
@ 2021-02-02  9:36         ` Takashi Iwai
  0 siblings, 0 replies; 6+ messages in thread
From: Takashi Iwai @ 2021-02-02  9:36 UTC (permalink / raw)
  To: fassl; +Cc: alsa-devel, Allen Pais, Takashi Iwai, Colin Ian King,
	Romain Perier

On Mon, 01 Feb 2021 15:28:52 +0100,
fassl wrote:
> 
> >From a9072f701f945ba00c42f612e85830395d81450d Mon Sep 17 00:00:00 2001
> From: Jasmin Fazlic <superfassl@gmail.com>
> Date: Sun, 31 Jan 2021 22:17:22 +0100
> Subject: [PATCH] sound: pci/rme9652 - hardware output loopback
> 
> Output loopback is a feature where you can record what you hear.
> The HDSP series of the RME interfaces provides this functionality
> at the hardware level and this patch exposes controls to enable or
> disable it per output (playback) channel.
> 
> This probably works on other cards but due to a lack of hardware
> it is only tested and enabled for the HDSP9632 card with this patch.
> 
> Should this patch be accepted a separate patch will be posted to
> https://github.com/alsa-project/alsa-tools/tree/master/hdspmixer
> which adds "LPBK" buttons to each ouput in the playback strip for
> the user to be able to control this feature from the user land.
> Users from Windows tool TotalMixFX should be familiar with this.
> 
> Signed-off-by: Jasmin Fazlic <superfassl@gmail.com>

Thanks, applied now with a slight modification of subject and a typo
correction in the patch description (it's always good to run
checkpatch once before submission :)


Takashi

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2021-02-03  7:05 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2021-01-31 21:37 [PATCH] sound: pci/rme9652 - implement and expose controls for output fassl
2021-02-01  8:42 ` Takashi Iwai
2021-02-01 13:50   ` fassl
2021-02-01 13:52     ` Takashi Iwai
2021-02-01 14:28       ` fassl
2021-02-02  9:36         ` Takashi Iwai

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.