Re: [PATCH 0/4] ALSA: scarlett2: note gen 3 support + fix four issues

Re: [PATCH 0/4] ALSA: scarlett2: note gen 3 support + fix four issues

[Geraldo Nascimento]

On Thu, Jul 22, 2021 at 8:12 PM Geoffrey D. Bennett <g@b4.vu> wrote:
>
> Hi Takashi,
>
> In the sound-5.14-rc1 merge commit log the highlights included:
> "Scarlett2 mixer code fixes and enhancements". I think that the new
> support for Gen 3 devices is significant enough to be worth
> mentioning.

I wholeheartedly agree with Geoffrey, Dr. Iwai.
The commit log highlights are picked up by various media outlets, e.g. Phoronix.

Most of the time these mainstream channels are the ones that will end
up notifying the end-user that new hardware support is available.
It would be not only wise to divulge our efforts that add support and
fix bugs in supported hardware.

It would be kind too, sometimes some piece of Audio hardware is the
one key aspect holding back an user in an OS they don't particularly
like.
I'd go so far and say holding the user hostage, but I'll just leave it
here and end my 2 cents worth of comment.

Thanks,
Geraldo Nascimento

[PATCH 0/4] ALSA: scarlett2: note gen 3 support + fix four issues

[Geoffrey D. Bennett]

Hi Takashi,

In the sound-5.14-rc1 merge commit log the highlights included:
"Scarlett2 mixer code fixes and enhancements". I think that the new
support for Gen 3 devices is significant enough to be worth
mentioning.

Can you add to the next update a note along the lines of "Support for
Focusrite Scarlett Solo/2i2/4i4/8i6/18i8/18i20 Gen 3 audio interface
proprietary mixer controls"?

This set of patches is relative to v5.14-rc2 and fixes four issues:

1. The Mute/Dim/MSD Mode controls are missing the direction/function
parts of the syntax of standard control names as per
Documentation/sound/designs/control-names.rst

- This could be considered a breaking-stable change if someone is
  relying on the Mute/Dim control names not changing. I think it's
  unlikely to be a problem as this driver is still considered
  experimental and not enabled by default, but if never changing
  control names is important, then you can drop this patch.

2. The Direct Monitor control on the 2i2 interface is an Enum, not a
Switch.

- This changes a control name, but that control was only introduced in
  v5.14-rc1, so not a problem to change it now.

3. Fixes the mute status not being correctly read when the mute button
is pressed.

- Not applicable for stable; those controls were introduced in
  v5.14-rc1.

4. Sends the correct notification on line out and speaker switching
changes.

- Partly applicable to stable, but will need a separate patch.

Thanks,
Geoffrey.

Geoffrey D. Bennett (4):
  ALSA: scarlett2: Fix Mute/Dim/MSD Mode control names
  ALSA: scarlett2: Fix Direct Monitor control name for 2i2
  ALSA: scarlett2: Correct channel mute status after mute button pressed
  ALSA: scarlett2: Fix line out/speaker switching notifications

 sound/usb/mixer_scarlett_gen2.c | 34 +++++++++++++++++++++++----------
 1 file changed, 24 insertions(+), 10 deletions(-)

-- 
2.31.1

[PATCH 1/4] ALSA: scarlett2: Fix Mute/Dim/MSD Mode control names

[Geoffrey D. Bennett]

Append "Playback Switch" to the names of "Mute" and "Dim" controls,
and append "Switch" to the "MSD Mode" control as per
Documentation/sound/designs/control-names.rst.

Signed-off-by: Geoffrey D. Bennett <g@b4.vu>
---
 sound/usb/mixer_scarlett_gen2.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/sound/usb/mixer_scarlett_gen2.c b/sound/usb/mixer_scarlett_gen2.c
index f9d698a37153..347995ea39e4 100644
--- a/sound/usb/mixer_scarlett_gen2.c
+++ b/sound/usb/mixer_scarlett_gen2.c
@@ -228,7 +228,7 @@ enum {
 };
 
 static const char *const scarlett2_dim_mute_names[SCARLETT2_DIM_MUTE_COUNT] = {
-	"Mute", "Dim"
+	"Mute Playback Switch", "Dim Playback Switch"
 };
 
 /* Description of each hardware port type:
@@ -3455,7 +3455,7 @@ static int scarlett2_add_msd_ctl(struct usb_mixer_interface *mixer)
 
 	/* Add MSD control */
 	return scarlett2_add_new_ctl(mixer, &scarlett2_msd_ctl,
-				     0, 1, "MSD Mode", NULL);
+				     0, 1, "MSD Mode Switch", NULL);
 }
 
 /*** Cleanup/Suspend Callbacks ***/
-- 
2.31.1

[PATCH 2/4] ALSA: scarlett2: Fix Direct Monitor control name for 2i2

[Geoffrey D. Bennett]

The Direct Monitor control for the 2i2 is an enumerated value, not a
boolean. Fix the control name to say "Playback Enum" instead of
"Playback Switch" in this case.

Signed-off-by: Geoffrey D. Bennett <g@b4.vu>
---
 sound/usb/mixer_scarlett_gen2.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/sound/usb/mixer_scarlett_gen2.c b/sound/usb/mixer_scarlett_gen2.c
index 347995ea39e4..fa604b61066f 100644
--- a/sound/usb/mixer_scarlett_gen2.c
+++ b/sound/usb/mixer_scarlett_gen2.c
@@ -2530,14 +2530,18 @@ static int scarlett2_add_direct_monitor_ctl(struct usb_mixer_interface *mixer)
 {
 	struct scarlett2_data *private = mixer->private_data;
 	const struct scarlett2_device_info *info = private->info;
+	const char *s;
 
 	if (!info->direct_monitor)
 		return 0;
 
+	s = info->direct_monitor == 1
+	      ? "Direct Monitor Playback Switch"
+	      : "Direct Monitor Playback Enum";
+
 	return scarlett2_add_new_ctl(
 		mixer, &scarlett2_direct_monitor_ctl[info->direct_monitor - 1],
-		0, 1, "Direct Monitor Playback Switch",
-		&private->direct_monitor_ctl);
+		0, 1, s, &private->direct_monitor_ctl);
 }
 
 /*** Speaker Switching Control ***/
-- 
2.31.1

[PATCH 3/4] ALSA: scarlett2: Correct channel mute status after mute button pressed

[Geoffrey D. Bennett]

After the hardware mute button is pressed, private->vol_updated is set
so that the mute status is invalidated. As the channel mute values may
be affected by the global mute value, update scarlett2_mute_ctl_get()
to call scarlett2_update_volumes() if private->vol_updated is set.

Signed-off-by: Geoffrey D. Bennett <g@b4.vu>
---
 sound/usb/mixer_scarlett_gen2.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/sound/usb/mixer_scarlett_gen2.c b/sound/usb/mixer_scarlett_gen2.c
index fa604b61066f..3457fbc8108f 100644
--- a/sound/usb/mixer_scarlett_gen2.c
+++ b/sound/usb/mixer_scarlett_gen2.c
@@ -1856,9 +1856,15 @@ static int scarlett2_mute_ctl_get(struct snd_kcontrol *kctl,
 					struct snd_ctl_elem_value *ucontrol)
 {
 	struct usb_mixer_elem_info *elem = kctl->private_data;
-	struct scarlett2_data *private = elem->head.mixer->private_data;
+	struct usb_mixer_interface *mixer = elem->head.mixer;
+	struct scarlett2_data *private = mixer->private_data;
 	int index = line_out_remap(private, elem->control);
 
+	mutex_lock(&private->data_mutex);
+	if (private->vol_updated)
+		scarlett2_update_volumes(mixer);
+	mutex_unlock(&private->data_mutex);
+
 	ucontrol->value.integer.value[0] = private->mute_switch[index];
 	return 0;
 }
-- 
2.31.1

[PATCH 4/4] ALSA: scarlett2: Fix line out/speaker switching notifications

[Geoffrey D. Bennett]

The values of the line output controls can change when the SW/HW
switches are set to HW, and also when speaker switching is enabled.
These notifications were sent with a mask of only
SNDRV_CTL_EVENT_MASK_INFO. Change the notifications to set the
SNDRV_CTL_EVENT_MASK_VALUE mask bit as well.

When the mute control is updated, the notification was sent with a
mask of SNDRV_CTL_EVENT_MASK_INFO. Change the mask to the correct
value of SNDRV_CTL_EVENT_MASK_VALUE.

Signed-off-by: Geoffrey D. Bennett <g@b4.vu>
---
 sound/usb/mixer_scarlett_gen2.c | 14 +++++++++-----
 1 file changed, 9 insertions(+), 5 deletions(-)

diff --git a/sound/usb/mixer_scarlett_gen2.c b/sound/usb/mixer_scarlett_gen2.c
index 3457fbc8108f..573e4a190f28 100644
--- a/sound/usb/mixer_scarlett_gen2.c
+++ b/sound/usb/mixer_scarlett_gen2.c
@@ -1961,10 +1961,12 @@ static void scarlett2_vol_ctl_set_writable(struct usb_mixer_interface *mixer,
 			~SNDRV_CTL_ELEM_ACCESS_WRITE;
 	}
 
-	/* Notify of write bit change */
-	snd_ctl_notify(card, SNDRV_CTL_EVENT_MASK_INFO,
+	/* Notify of write bit and possible value change */
+	snd_ctl_notify(card,
+		       SNDRV_CTL_EVENT_MASK_VALUE | SNDRV_CTL_EVENT_MASK_INFO,
 		       &private->vol_ctls[index]->id);
-	snd_ctl_notify(card, SNDRV_CTL_EVENT_MASK_INFO,
+	snd_ctl_notify(card,
+		       SNDRV_CTL_EVENT_MASK_VALUE | SNDRV_CTL_EVENT_MASK_INFO,
 		       &private->mute_ctls[index]->id);
 }
 
@@ -2599,7 +2601,9 @@ static int scarlett2_speaker_switch_enable(struct usb_mixer_interface *mixer)
 
 		/* disable the line out SW/HW switch */
 		scarlett2_sw_hw_ctl_ro(private, i);
-		snd_ctl_notify(card, SNDRV_CTL_EVENT_MASK_INFO,
+		snd_ctl_notify(card,
+			       SNDRV_CTL_EVENT_MASK_VALUE |
+				 SNDRV_CTL_EVENT_MASK_INFO,
 			       &private->sw_hw_ctls[i]->id);
 	}
 
@@ -2923,7 +2927,7 @@ static int scarlett2_dim_mute_ctl_put(struct snd_kcontrol *kctl,
 			if (private->vol_sw_hw_switch[line_index]) {
 				private->mute_switch[line_index] = val;
 				snd_ctl_notify(mixer->chip->card,
-					       SNDRV_CTL_EVENT_MASK_INFO,
+					       SNDRV_CTL_EVENT_MASK_VALUE,
 					       &private->mute_ctls[i]->id);
 			}
 		}
-- 
2.31.1

Re: [PATCH 0/4] ALSA: scarlett2: note gen 3 support + fix four issues

[Takashi Iwai]

On Thu, 22 Jul 2021 22:11:40 +0200,
Geoffrey D. Bennett wrote:
> 
> Hi Takashi,
> 
> In the sound-5.14-rc1 merge commit log the highlights included:
> "Scarlett2 mixer code fixes and enhancements". I think that the new
> support for Gen 3 devices is significant enough to be worth
> mentioning.
> 
> Can you add to the next update a note along the lines of "Support for
> Focusrite Scarlett Solo/2i2/4i4/8i6/18i8/18i20 Gen 3 audio interface
> proprietary mixer controls"?
> 
> This set of patches is relative to v5.14-rc2 and fixes four issues:
> 
> 1. The Mute/Dim/MSD Mode controls are missing the direction/function
> parts of the syntax of standard control names as per
> Documentation/sound/designs/control-names.rst
> 
> - This could be considered a breaking-stable change if someone is
>   relying on the Mute/Dim control names not changing. I think it's
>   unlikely to be a problem as this driver is still considered
>   experimental and not enabled by default, but if never changing
>   control names is important, then you can drop this patch.
> 
> 2. The Direct Monitor control on the 2i2 interface is an Enum, not a
> Switch.
> 
> - This changes a control name, but that control was only introduced in
>   v5.14-rc1, so not a problem to change it now.
> 
> 3. Fixes the mute status not being correctly read when the mute button
> is pressed.
> 
> - Not applicable for stable; those controls were introduced in
>   v5.14-rc1.
> 
> 4. Sends the correct notification on line out and speaker switching
> changes.
> 
> - Partly applicable to stable, but will need a separate patch.
> 
> Thanks,
> Geoffrey.
> 
> Geoffrey D. Bennett (4):
>   ALSA: scarlett2: Fix Mute/Dim/MSD Mode control names
>   ALSA: scarlett2: Fix Direct Monitor control name for 2i2
>   ALSA: scarlett2: Correct channel mute status after mute button pressed
>   ALSA: scarlett2: Fix line out/speaker switching notifications

Applied all four patches now.  Thanks.


Takashi