diff mbox series

[4.19,15/39] ALSA: hda/generic: Add option to enforce preferred_dacs pairs

Message ID 20201210142603.037114540@linuxfoundation.org
State New
Headers show
Series None | expand

Commit Message

Greg KH Dec. 10, 2020, 2:26 p.m. UTC 
From: Takashi Iwai <tiwai@suse.de>

commit 242d990c158d5b1dabd166516e21992baef5f26a upstream.

The generic parser accepts the preferred_dacs[] pairs as a hint for
assigning a DAC to each pin, but this hint doesn't work always
effectively.  Currently it's merely a secondary choice after the trial
with the path index failed.  This made sometimes it difficult to
assign DACs without mimicking the connection list and/or the badness
table.

This patch adds a new flag, obey_preferred_dacs, that changes the
behavior of the parser.  As its name stands, the parser obeys the
given preferred_dacs[] pairs by skipping the path index matching and
giving a high penalty if no DAC is assigned by the pairs.  This mode
will help for assigning the fixed DACs forcibly from the codec
driver.

Cc: <stable@vger.kernel.org>
Link: https://lore.kernel.org/r/20201127141104.11041-1-tiwai@suse.de
Signed-off-by: Takashi Iwai <tiwai@suse.de>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

---
 sound/pci/hda/hda_generic.c |   12 ++++++++----
 sound/pci/hda/hda_generic.h |    1 +
 2 files changed, 9 insertions(+), 4 deletions(-)

Comments

Pavel Machek Dec. 11, 2020, 8:36 a.m. UTC | #1 
Hi!

> From: Takashi Iwai <tiwai@suse.de>

> 

> commit 242d990c158d5b1dabd166516e21992baef5f26a upstream.

> 

> The generic parser accepts the preferred_dacs[] pairs as a hint for

> assigning a DAC to each pin, but this hint doesn't work always

> effectively.  Currently it's merely a secondary choice after the trial

> with the path index failed.  This made sometimes it difficult to

> assign DACs without mimicking the connection list and/or the badness

> table.

> 

> This patch adds a new flag, obey_preferred_dacs, that changes the

> behavior of the parser.  As its name stands, the parser obeys the


Option added is never used as variable is never set. We don't need
this in 4.19.

Best regards,
								Pavel
-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Takashi Iwai Dec. 11, 2020, 8:41 a.m. UTC | #2 
On Fri, 11 Dec 2020 09:36:21 +0100,
Pavel Machek wrote:
> 

> Hi!

> 

> > From: Takashi Iwai <tiwai@suse.de>

> > 

> > commit 242d990c158d5b1dabd166516e21992baef5f26a upstream.

> > 

> > The generic parser accepts the preferred_dacs[] pairs as a hint for

> > assigning a DAC to each pin, but this hint doesn't work always

> > effectively.  Currently it's merely a secondary choice after the trial

> > with the path index failed.  This made sometimes it difficult to

> > assign DACs without mimicking the connection list and/or the badness

> > table.

> > 

> > This patch adds a new flag, obey_preferred_dacs, that changes the

> > behavior of the parser.  As its name stands, the parser obeys the

> 

> Option added is never used as variable is never set. We don't need

> this in 4.19.


Right, it seems that the succeeding fix is queued only for 5.4 and
5.9.

OTOH, this change will help if another quirk is added in near future,
and it's pretty safe to apply, so I think it's worth to keep it.


thanks,

Takashi
Pavel Machek Dec. 11, 2020, 10:38 p.m. UTC | #3 
Hi!

> > > From: Takashi Iwai <tiwai@suse.de>

> > > 

> > > commit 242d990c158d5b1dabd166516e21992baef5f26a upstream.

> > > 

> > > The generic parser accepts the preferred_dacs[] pairs as a hint for

> > > assigning a DAC to each pin, but this hint doesn't work always

> > > effectively.  Currently it's merely a secondary choice after the trial

> > > with the path index failed.  This made sometimes it difficult to

> > > assign DACs without mimicking the connection list and/or the badness

> > > table.

> > > 

> > > This patch adds a new flag, obey_preferred_dacs, that changes the

> > > behavior of the parser.  As its name stands, the parser obeys the

> > 

> > Option added is never used as variable is never set. We don't need

> > this in 4.19.

> 

> Right, it seems that the succeeding fix is queued only for 5.4 and

> 5.9.

> 

> OTOH, this change will help if another quirk is added in near future,

> and it's pretty safe to apply, so I think it's worth to keep it.


I agree that this is safe to apply and makes sense if we get another
quirk soon. OTOH it is trivial to backport if it is needed.

Best regards,

								Pavel
-- 
DENX Software Engineering GmbH,      Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Takashi Iwai Dec. 11, 2020, 10:40 p.m. UTC | #4 
On Fri, 11 Dec 2020 23:38:22 +0100,
Pavel Machek wrote:
> 

> Hi!

> 

> > > > From: Takashi Iwai <tiwai@suse.de>

> > > > 

> > > > commit 242d990c158d5b1dabd166516e21992baef5f26a upstream.

> > > > 

> > > > The generic parser accepts the preferred_dacs[] pairs as a hint for

> > > > assigning a DAC to each pin, but this hint doesn't work always

> > > > effectively.  Currently it's merely a secondary choice after the trial

> > > > with the path index failed.  This made sometimes it difficult to

> > > > assign DACs without mimicking the connection list and/or the badness

> > > > table.

> > > > 

> > > > This patch adds a new flag, obey_preferred_dacs, that changes the

> > > > behavior of the parser.  As its name stands, the parser obeys the

> > > 

> > > Option added is never used as variable is never set. We don't need

> > > this in 4.19.

> > 

> > Right, it seems that the succeeding fix is queued only for 5.4 and

> > 5.9.

> > 

> > OTOH, this change will help if another quirk is added in near future,

> > and it's pretty safe to apply, so I think it's worth to keep it.

> 

> I agree that this is safe to apply and makes sense if we get another

> quirk soon. OTOH it is trivial to backport if it is needed.


It's trivial to backport, but it's not trivial to let stable
maintainers to inform beforehand if the next patch has CC-to-stable
tag.  Keeping this makes the backport easier, that's the point.


thanks,

Takashi
diff mbox series

Patch

--- a/sound/pci/hda/hda_generic.c
+++ b/sound/pci/hda/hda_generic.c
@@ -1376,16 +1376,20 @@  static int try_assign_dacs(struct hda_co
 		struct nid_path *path;
 		hda_nid_t pin = pins[i];
 
-		path = snd_hda_get_path_from_idx(codec, path_idx[i]);
-		if (path) {
-			badness += assign_out_path_ctls(codec, path);
-			continue;
+		if (!spec->obey_preferred_dacs) {
+			path = snd_hda_get_path_from_idx(codec, path_idx[i]);
+			if (path) {
+				badness += assign_out_path_ctls(codec, path);
+				continue;
+			}
 		}
 
 		dacs[i] = get_preferred_dac(codec, pin);
 		if (dacs[i]) {
 			if (is_dac_already_used(codec, dacs[i]))
 				badness += bad->shared_primary;
+		} else if (spec->obey_preferred_dacs) {
+			badness += BAD_NO_PRIMARY_DAC;
 		}
 
 		if (!dacs[i])
--- a/sound/pci/hda/hda_generic.h
+++ b/sound/pci/hda/hda_generic.h
@@ -240,6 +240,7 @@  struct hda_gen_spec {
 	unsigned int power_down_unused:1; /* power down unused widgets */
 	unsigned int dac_min_mute:1; /* minimal = mute for DACs */
 	unsigned int suppress_vmaster:1; /* don't create vmaster kctls */
+	unsigned int obey_preferred_dacs:1; /* obey preferred_dacs assignment */
 
 	/* other internal flags */
 	unsigned int no_analog:1; /* digital I/O only */