All of lore.kernel.org
 help / color / mirror / Atom feed
* Regression with a91d66129fb9 ("ALSA: hda - Fix incorrect TLV callback check introduced during set_fs() removal")
@ 2017-11-21 16:14 Laura Abbott
  2017-11-21 16:25   ` Takashi Iwai
  0 siblings, 1 reply; 6+ messages in thread
From: Laura Abbott @ 2017-11-21 16:14 UTC (permalink / raw)
  To: Takashi Iwai; +Cc: alsa-devel, linux-kernel, PaX Team

Hi,

Fedora got a bug report (https://bugzilla.redhat.com/show_bug.cgi?id=1512853)
that Line Out stopped working between 4.13.9 and 4.13.10. Reverting
82d745a55779 ("ALSA: hda - Fix incorrect TLV callback check introduced during set_fs() removal")
fixed the problem. I didn't ask the reporter to test on 4.14 since I didn't see
anything explicitly tagged as fixing the issue. Any ideas?

Thanks,
Laura

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

* Re: Regression with a91d66129fb9 ("ALSA: hda - Fix incorrect TLV callback check introduced during set_fs() removal")
  2017-11-21 16:14 Regression with a91d66129fb9 ("ALSA: hda - Fix incorrect TLV callback check introduced during set_fs() removal") Laura Abbott
@ 2017-11-21 16:25   ` Takashi Iwai
  0 siblings, 0 replies; 6+ messages in thread
From: Takashi Iwai @ 2017-11-21 16:25 UTC (permalink / raw)
  To: Laura Abbott; +Cc: alsa-devel, linux-kernel, PaX Team

On Tue, 21 Nov 2017 17:14:42 +0100,
Laura Abbott wrote:
> 
> Hi,
> 
> Fedora got a bug report (https://bugzilla.redhat.com/show_bug.cgi?id=1512853)
> that Line Out stopped working between 4.13.9 and 4.13.10. Reverting
> 82d745a55779 ("ALSA: hda - Fix incorrect TLV callback check introduced during set_fs() removal")
> fixed the problem. I didn't ask the reporter to test on 4.14 since I didn't see
> anything explicitly tagged as fixing the issue. Any ideas?

It might be that the formerly saved asound.state brought the
inconsistency.  Try to remove the saved state (either
/var/lib/alsa/asound.state or /etc/asound.state) after unloading the
sound driver modules and reboot/retest.

In anyway, give alsa-info.sh output with the affected machine.
Run the script with --no-upload option, and attach the generated
file.


thanks,

Takashi

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

* Re: Regression with a91d66129fb9 ("ALSA: hda - Fix incorrect TLV callback check introduced during set_fs() removal")
@ 2017-11-21 16:25   ` Takashi Iwai
  0 siblings, 0 replies; 6+ messages in thread
From: Takashi Iwai @ 2017-11-21 16:25 UTC (permalink / raw)
  To: Laura Abbott; +Cc: PaX Team, alsa-devel, linux-kernel

On Tue, 21 Nov 2017 17:14:42 +0100,
Laura Abbott wrote:
> 
> Hi,
> 
> Fedora got a bug report (https://bugzilla.redhat.com/show_bug.cgi?id=1512853)
> that Line Out stopped working between 4.13.9 and 4.13.10. Reverting
> 82d745a55779 ("ALSA: hda - Fix incorrect TLV callback check introduced during set_fs() removal")
> fixed the problem. I didn't ask the reporter to test on 4.14 since I didn't see
> anything explicitly tagged as fixing the issue. Any ideas?

It might be that the formerly saved asound.state brought the
inconsistency.  Try to remove the saved state (either
/var/lib/alsa/asound.state or /etc/asound.state) after unloading the
sound driver modules and reboot/retest.

In anyway, give alsa-info.sh output with the affected machine.
Run the script with --no-upload option, and attach the generated
file.


thanks,

Takashi

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

* Re: Regression with a91d66129fb9 ("ALSA: hda - Fix incorrect TLV callback check introduced during set_fs() removal")
  2017-11-21 16:25   ` Takashi Iwai
  (?)
@ 2017-11-22  7:52   ` Takashi Iwai
  2017-11-22 11:42       ` Takashi Iwai
  -1 siblings, 1 reply; 6+ messages in thread
From: Takashi Iwai @ 2017-11-22  7:52 UTC (permalink / raw)
  To: Laura Abbott; +Cc: alsa-devel, linux-kernel, PaX Team

On Tue, 21 Nov 2017 17:25:05 +0100,
Takashi Iwai wrote:
> 
> On Tue, 21 Nov 2017 17:14:42 +0100,
> Laura Abbott wrote:
> > 
> > Hi,
> > 
> > Fedora got a bug report (https://bugzilla.redhat.com/show_bug.cgi?id=1512853)
> > that Line Out stopped working between 4.13.9 and 4.13.10. Reverting
> > 82d745a55779 ("ALSA: hda - Fix incorrect TLV callback check introduced during set_fs() removal")
> > fixed the problem. I didn't ask the reporter to test on 4.14 since I didn't see
> > anything explicitly tagged as fixing the issue. Any ideas?
> 
> It might be that the formerly saved asound.state brought the
> inconsistency.  Try to remove the saved state (either
> /var/lib/alsa/asound.state or /etc/asound.state) after unloading the
> sound driver modules and reboot/retest.
> 
> In anyway, give alsa-info.sh output with the affected machine.
> Run the script with --no-upload option, and attach the generated
> file.

BTW, having the output of alsa-info.sh with a bug report helps
analysis and debugging quite a lot.

So it'd be appreciated if you can ask it always as the first step for
a bug report on Fedora regarding sound.  Especially when it's a
regression, the outputs on both working and non-working cases would be
nice, so that we can compare more precisely.


thanks,

Takashi

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

* Re: Regression with a91d66129fb9 ("ALSA: hda - Fix incorrect TLV callback check introduced during set_fs() removal")
  2017-11-22  7:52   ` Takashi Iwai
@ 2017-11-22 11:42       ` Takashi Iwai
  0 siblings, 0 replies; 6+ messages in thread
From: Takashi Iwai @ 2017-11-22 11:42 UTC (permalink / raw)
  To: Laura Abbott; +Cc: alsa-devel, linux-kernel, PaX Team

On Wed, 22 Nov 2017 08:52:51 +0100,
Takashi Iwai wrote:
> 
> On Tue, 21 Nov 2017 17:25:05 +0100,
> Takashi Iwai wrote:
> > 
> > On Tue, 21 Nov 2017 17:14:42 +0100,
> > Laura Abbott wrote:
> > > 
> > > Hi,
> > > 
> > > Fedora got a bug report (https://bugzilla.redhat.com/show_bug.cgi?id=1512853)
> > > that Line Out stopped working between 4.13.9 and 4.13.10. Reverting
> > > 82d745a55779 ("ALSA: hda - Fix incorrect TLV callback check introduced during set_fs() removal")
> > > fixed the problem. I didn't ask the reporter to test on 4.14 since I didn't see
> > > anything explicitly tagged as fixing the issue. Any ideas?
> > 
> > It might be that the formerly saved asound.state brought the
> > inconsistency.  Try to remove the saved state (either
> > /var/lib/alsa/asound.state or /etc/asound.state) after unloading the
> > sound driver modules and reboot/retest.
> > 
> > In anyway, give alsa-info.sh output with the affected machine.
> > Run the script with --no-upload option, and attach the generated
> > file.
> 
> BTW, having the output of alsa-info.sh with a bug report helps
> analysis and debugging quite a lot.
> 
> So it'd be appreciated if you can ask it always as the first step for
> a bug report on Fedora regarding sound.  Especially when it's a
> regression, the outputs on both working and non-working cases would be
> nice, so that we can compare more precisely.

And yet we got another bug report, and at this time, I could analyze
with alsa-info.sh outputs.  Below is the fix patch.  Give it a try.


Takashi

-- 8< --
From: Takashi Iwai <tiwai@suse.de>
Subject: [PATCH] ALSA: hda - Fix yet remaining issue with vmaster 0dB
 initialization

The previous fix for addressing the breakage in vmaster slave
initialization, commit a91d66129fb9 ("ALSA: hda - Fix incorrect TLV
callback check introduced during set_fs() removal"), introduced a new
helper to process over each slave kctl.  However, this helper passes
only the original kctl, not the virtual slave kctl.  As a result,
HD-audio driver (which is the only user so far) couldn't initialize
the slave correctly because it's trying to update the value directly
with the original kctl, not with the mapped kctl.

This patch fixes the situation again by passing both the mapped slaved
and original slave kctls to the function.  Luckily there is a single
caller as of now, so changing the call signature is no big matter.

Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=197959
Fixes: a91d66129fb9 ("ALSA: hda - Fix incorrect TLV callback check introduced during set_fs() removal")
Cc: <stable@vger.kernel.org>
Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
 include/sound/control.h   |  4 +++-
 sound/core/vmaster.c      |  6 ++++--
 sound/pci/hda/hda_codec.c | 10 +++++++---
 3 files changed, 14 insertions(+), 6 deletions(-)

diff --git a/include/sound/control.h b/include/sound/control.h
index a1f1152bc687..ca13a44ae9d4 100644
--- a/include/sound/control.h
+++ b/include/sound/control.h
@@ -249,7 +249,9 @@ int snd_ctl_add_vmaster_hook(struct snd_kcontrol *kctl,
 void snd_ctl_sync_vmaster(struct snd_kcontrol *kctl, bool hook_only);
 #define snd_ctl_sync_vmaster_hook(kctl)	snd_ctl_sync_vmaster(kctl, true)
 int snd_ctl_apply_vmaster_slaves(struct snd_kcontrol *kctl,
-				 int (*func)(struct snd_kcontrol *, void *),
+				 int (*func)(struct snd_kcontrol *vslave,
+					     struct snd_kcontrol *slave,
+					     void *arg),
 				 void *arg);
 
 /*
diff --git a/sound/core/vmaster.c b/sound/core/vmaster.c
index e43af18d4383..8632301489fa 100644
--- a/sound/core/vmaster.c
+++ b/sound/core/vmaster.c
@@ -495,7 +495,9 @@ EXPORT_SYMBOL_GPL(snd_ctl_sync_vmaster);
  * Returns 0 if successful, or a negative error code.
  */
 int snd_ctl_apply_vmaster_slaves(struct snd_kcontrol *kctl,
-				 int (*func)(struct snd_kcontrol *, void *),
+				 int (*func)(struct snd_kcontrol *vslave,
+					     struct snd_kcontrol *slave,
+					     void *arg),
 				 void *arg)
 {
 	struct link_master *master;
@@ -507,7 +509,7 @@ int snd_ctl_apply_vmaster_slaves(struct snd_kcontrol *kctl,
 	if (err < 0)
 		return err;
 	list_for_each_entry(slave, &master->slaves, list) {
-		err = func(&slave->slave, arg);
+		err = func(slave->kctl, &slave->slave, arg);
 		if (err < 0)
 			return err;
 	}
diff --git a/sound/pci/hda/hda_codec.c b/sound/pci/hda/hda_codec.c
index c1f8e5479bf3..e018ecbf78a8 100644
--- a/sound/pci/hda/hda_codec.c
+++ b/sound/pci/hda/hda_codec.c
@@ -1823,7 +1823,9 @@ struct slave_init_arg {
 };
 
 /* initialize the slave volume with 0dB via snd_ctl_apply_vmaster_slaves() */
-static int init_slave_0dB(struct snd_kcontrol *kctl, void *_arg)
+static int init_slave_0dB(struct snd_kcontrol *slave,
+			  struct snd_kcontrol *kctl,
+			  void *_arg)
 {
 	struct slave_init_arg *arg = _arg;
 	int _tlv[4];
@@ -1860,7 +1862,7 @@ static int init_slave_0dB(struct snd_kcontrol *kctl, void *_arg)
 	arg->step = step;
 	val = -tlv[2] / step;
 	if (val > 0) {
-		put_kctl_with_value(kctl, val);
+		put_kctl_with_value(slave, val);
 		return val;
 	}
 
@@ -1868,7 +1870,9 @@ static int init_slave_0dB(struct snd_kcontrol *kctl, void *_arg)
 }
 
 /* unmute the slave via snd_ctl_apply_vmaster_slaves() */
-static int init_slave_unmute(struct snd_kcontrol *slave, void *_arg)
+static int init_slave_unmute(struct snd_kcontrol *slave,
+			     struct snd_kcontrol *kctl,
+			     void *_arg)
 {
 	return put_kctl_with_value(slave, 1);
 }
-- 
2.15.0

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

* Re: Regression with a91d66129fb9 ("ALSA: hda - Fix incorrect TLV callback check introduced during set_fs() removal")
@ 2017-11-22 11:42       ` Takashi Iwai
  0 siblings, 0 replies; 6+ messages in thread
From: Takashi Iwai @ 2017-11-22 11:42 UTC (permalink / raw)
  To: Laura Abbott; +Cc: PaX Team, alsa-devel, linux-kernel

On Wed, 22 Nov 2017 08:52:51 +0100,
Takashi Iwai wrote:
> 
> On Tue, 21 Nov 2017 17:25:05 +0100,
> Takashi Iwai wrote:
> > 
> > On Tue, 21 Nov 2017 17:14:42 +0100,
> > Laura Abbott wrote:
> > > 
> > > Hi,
> > > 
> > > Fedora got a bug report (https://bugzilla.redhat.com/show_bug.cgi?id=1512853)
> > > that Line Out stopped working between 4.13.9 and 4.13.10. Reverting
> > > 82d745a55779 ("ALSA: hda - Fix incorrect TLV callback check introduced during set_fs() removal")
> > > fixed the problem. I didn't ask the reporter to test on 4.14 since I didn't see
> > > anything explicitly tagged as fixing the issue. Any ideas?
> > 
> > It might be that the formerly saved asound.state brought the
> > inconsistency.  Try to remove the saved state (either
> > /var/lib/alsa/asound.state or /etc/asound.state) after unloading the
> > sound driver modules and reboot/retest.
> > 
> > In anyway, give alsa-info.sh output with the affected machine.
> > Run the script with --no-upload option, and attach the generated
> > file.
> 
> BTW, having the output of alsa-info.sh with a bug report helps
> analysis and debugging quite a lot.
> 
> So it'd be appreciated if you can ask it always as the first step for
> a bug report on Fedora regarding sound.  Especially when it's a
> regression, the outputs on both working and non-working cases would be
> nice, so that we can compare more precisely.

And yet we got another bug report, and at this time, I could analyze
with alsa-info.sh outputs.  Below is the fix patch.  Give it a try.


Takashi

-- 8< --
From: Takashi Iwai <tiwai@suse.de>
Subject: [PATCH] ALSA: hda - Fix yet remaining issue with vmaster 0dB
 initialization

The previous fix for addressing the breakage in vmaster slave
initialization, commit a91d66129fb9 ("ALSA: hda - Fix incorrect TLV
callback check introduced during set_fs() removal"), introduced a new
helper to process over each slave kctl.  However, this helper passes
only the original kctl, not the virtual slave kctl.  As a result,
HD-audio driver (which is the only user so far) couldn't initialize
the slave correctly because it's trying to update the value directly
with the original kctl, not with the mapped kctl.

This patch fixes the situation again by passing both the mapped slaved
and original slave kctls to the function.  Luckily there is a single
caller as of now, so changing the call signature is no big matter.

Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=197959
Fixes: a91d66129fb9 ("ALSA: hda - Fix incorrect TLV callback check introduced during set_fs() removal")
Cc: <stable@vger.kernel.org>
Signed-off-by: Takashi Iwai <tiwai@suse.de>
---
 include/sound/control.h   |  4 +++-
 sound/core/vmaster.c      |  6 ++++--
 sound/pci/hda/hda_codec.c | 10 +++++++---
 3 files changed, 14 insertions(+), 6 deletions(-)

diff --git a/include/sound/control.h b/include/sound/control.h
index a1f1152bc687..ca13a44ae9d4 100644
--- a/include/sound/control.h
+++ b/include/sound/control.h
@@ -249,7 +249,9 @@ int snd_ctl_add_vmaster_hook(struct snd_kcontrol *kctl,
 void snd_ctl_sync_vmaster(struct snd_kcontrol *kctl, bool hook_only);
 #define snd_ctl_sync_vmaster_hook(kctl)	snd_ctl_sync_vmaster(kctl, true)
 int snd_ctl_apply_vmaster_slaves(struct snd_kcontrol *kctl,
-				 int (*func)(struct snd_kcontrol *, void *),
+				 int (*func)(struct snd_kcontrol *vslave,
+					     struct snd_kcontrol *slave,
+					     void *arg),
 				 void *arg);
 
 /*
diff --git a/sound/core/vmaster.c b/sound/core/vmaster.c
index e43af18d4383..8632301489fa 100644
--- a/sound/core/vmaster.c
+++ b/sound/core/vmaster.c
@@ -495,7 +495,9 @@ EXPORT_SYMBOL_GPL(snd_ctl_sync_vmaster);
  * Returns 0 if successful, or a negative error code.
  */
 int snd_ctl_apply_vmaster_slaves(struct snd_kcontrol *kctl,
-				 int (*func)(struct snd_kcontrol *, void *),
+				 int (*func)(struct snd_kcontrol *vslave,
+					     struct snd_kcontrol *slave,
+					     void *arg),
 				 void *arg)
 {
 	struct link_master *master;
@@ -507,7 +509,7 @@ int snd_ctl_apply_vmaster_slaves(struct snd_kcontrol *kctl,
 	if (err < 0)
 		return err;
 	list_for_each_entry(slave, &master->slaves, list) {
-		err = func(&slave->slave, arg);
+		err = func(slave->kctl, &slave->slave, arg);
 		if (err < 0)
 			return err;
 	}
diff --git a/sound/pci/hda/hda_codec.c b/sound/pci/hda/hda_codec.c
index c1f8e5479bf3..e018ecbf78a8 100644
--- a/sound/pci/hda/hda_codec.c
+++ b/sound/pci/hda/hda_codec.c
@@ -1823,7 +1823,9 @@ struct slave_init_arg {
 };
 
 /* initialize the slave volume with 0dB via snd_ctl_apply_vmaster_slaves() */
-static int init_slave_0dB(struct snd_kcontrol *kctl, void *_arg)
+static int init_slave_0dB(struct snd_kcontrol *slave,
+			  struct snd_kcontrol *kctl,
+			  void *_arg)
 {
 	struct slave_init_arg *arg = _arg;
 	int _tlv[4];
@@ -1860,7 +1862,7 @@ static int init_slave_0dB(struct snd_kcontrol *kctl, void *_arg)
 	arg->step = step;
 	val = -tlv[2] / step;
 	if (val > 0) {
-		put_kctl_with_value(kctl, val);
+		put_kctl_with_value(slave, val);
 		return val;
 	}
 
@@ -1868,7 +1870,9 @@ static int init_slave_0dB(struct snd_kcontrol *kctl, void *_arg)
 }
 
 /* unmute the slave via snd_ctl_apply_vmaster_slaves() */
-static int init_slave_unmute(struct snd_kcontrol *slave, void *_arg)
+static int init_slave_unmute(struct snd_kcontrol *slave,
+			     struct snd_kcontrol *kctl,
+			     void *_arg)
 {
 	return put_kctl_with_value(slave, 1);
 }
-- 
2.15.0

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

end of thread, other threads:[~2017-11-22 11:42 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-11-21 16:14 Regression with a91d66129fb9 ("ALSA: hda - Fix incorrect TLV callback check introduced during set_fs() removal") Laura Abbott
2017-11-21 16:25 ` Takashi Iwai
2017-11-21 16:25   ` Takashi Iwai
2017-11-22  7:52   ` Takashi Iwai
2017-11-22 11:42     ` Takashi Iwai
2017-11-22 11:42       ` 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.