[alsa-devel] [PATCH] ALSA: hda: Conexant: Allow different output types to share DAC

Takashi Iwai tiwai at suse.de
Thu Aug 25 14:56:15 CEST 2011


At Thu, 25 Aug 2011 14:03:24 +0200,
Takashi Iwai wrote:
> 
> At Thu, 25 Aug 2011 13:26:26 +0200,
> David Henningsson wrote:
> > 
> > >From 95ca8a0cd32ad781c54a349c5ce84a9119668f9c Mon Sep 17 00:00:00 2001
> > From: David Henningsson <david.henningsson at canonical.com>
> > Date: Thu, 25 Aug 2011 13:16:02 +0200
> > Subject: [PATCH] ALSA: hda: Conexant: Allow different output types to share DAC
> > 
> > Headphones has stopped working for the original reported (a regression
> > compared to 2.6.38). This is because Speaker and Headphones share the
> > same DAC, in which case no Headphones volume control was created.
> > This patch fixes so that both Speaker and Headphones volume
> > controls are created in such scenario.
> > 
> > Buglink: http://bugs.launchpad.net/bugs/817943
> > Signed-off-by: David Henningsson <david.henningsson at canonical.com>
> > ---
> >  sound/pci/hda/patch_conexant.c |   40 ++++++++++++++++++++++------------------
> >  1 files changed, 22 insertions(+), 18 deletions(-)
> > 
> > diff --git a/sound/pci/hda/patch_conexant.c b/sound/pci/hda/patch_conexant.c
> > index 197ad93..a29b88d 100644
> > --- a/sound/pci/hda/patch_conexant.c
> > +++ b/sound/pci/hda/patch_conexant.c
> > @@ -3327,22 +3327,23 @@ static int fill_cx_auto_dacs(struct hda_codec *codec, hda_nid_t *dacs)
> >  }
> >  
> >  /* fill pin_dac_pair list from the pin and dac list */
> > -static int fill_dacs_for_pins(struct hda_codec *codec, hda_nid_t *pins,
> > +static void fill_dacs_for_pins(struct hda_codec *codec, hda_nid_t *pins,
> >  			      int num_pins, hda_nid_t *dacs, int *rest,
> > -			      struct pin_dac_pair *filled, int type)
> > +			      struct pin_dac_pair *filled, int *saved_nums, 
> > +			      int type)
> >  {
> >  	int i, nums;
> >  
> > -	nums = 0;
> > +	nums = *saved_nums;
> >  	for (i = 0; i < num_pins; i++) {
> >  		filled[nums].pin = pins[i];
> >  		filled[nums].type = type;
> >  		filled[nums].dac = get_unassigned_dac(codec, pins[i], dacs, rest);
> > -		if (!filled[nums].dac && i > 0 && filled[0].dac)
> > +		if (!filled[nums].dac && filled[0].dac)
> >  			filled[nums].dac = filled[0].dac | DAC_SLAVE_FLAG;
> 
> It should try the first DAC of the same type at first, not always
> to filled[0].
> 
> 
> >  		nums++;
> >  	}
> > -	return nums;
> > +	*saved_nums = nums;
> >  }
> >  
> >  /* parse analog output paths */
> > @@ -3355,15 +3356,16 @@ static void cx_auto_parse_output(struct hda_codec *codec)
> >  
> >  	rest = fill_cx_auto_dacs(codec, dacs);
> >  	/* parse all analog output pins */
> > -	nums = fill_dacs_for_pins(codec, cfg->line_out_pins, cfg->line_outs,
> > -				  dacs, &rest, spec->dac_info,
> > -				  AUTO_PIN_LINE_OUT);
> > -	nums += fill_dacs_for_pins(codec, cfg->hp_pins, cfg->hp_outs,
> > -				  dacs, &rest, spec->dac_info + nums,
> > -				  AUTO_PIN_HP_OUT);
> > -	nums += fill_dacs_for_pins(codec, cfg->speaker_pins, cfg->speaker_outs,
> > -				  dacs, &rest, spec->dac_info + nums,
> > -				  AUTO_PIN_SPEAKER_OUT);
> > +	nums = 0;
> > +	fill_dacs_for_pins(codec, cfg->line_out_pins, cfg->line_outs,
> > +			  dacs, &rest, spec->dac_info, &nums,
> > +			  AUTO_PIN_LINE_OUT);
> > +	fill_dacs_for_pins(codec, cfg->hp_pins, cfg->hp_outs,
> > +			  dacs, &rest, spec->dac_info, &nums,
> > +			  AUTO_PIN_HP_OUT);
> > +	fill_dacs_for_pins(codec, cfg->speaker_pins, cfg->speaker_outs,
> > +			  dacs, &rest, spec->dac_info, &nums,
> > +			  AUTO_PIN_SPEAKER_OUT);
> >  	spec->dac_info_filled = nums;
> >  	/* fill multiout struct */
> >  	for (i = 0; i < nums; i++) {
> > @@ -4130,9 +4132,11 @@ static int try_add_pb_volume(struct hda_codec *codec, hda_nid_t dac,
> >  			     hda_nid_t pin, const char *name, int idx)
> >  {
> >  	unsigned int caps;
> > -	caps = query_amp_caps(codec, dac, HDA_OUTPUT);
> > -	if (caps & AC_AMPCAP_NUM_STEPS)
> > -		return cx_auto_add_pb_volume(codec, dac, name, idx);
> > +	if (!(dac & DAC_SLAVE_FLAG)) {
> > +		caps = query_amp_caps(codec, dac, HDA_OUTPUT);
> > +		if (caps & AC_AMPCAP_NUM_STEPS)
> > +			return cx_auto_add_pb_volume(codec, dac, name, idx);
> > +	}
> >  	caps = query_amp_caps(codec, pin, HDA_OUTPUT);
> >  	if (caps & AC_AMPCAP_NUM_STEPS)
> >  		return cx_auto_add_pb_volume(codec, pin, name, idx);
> > @@ -4155,7 +4159,7 @@ static int cx_auto_build_output_controls(struct hda_codec *codec)
> >  		const char *label;
> >  		int idx, type;
> >  		hda_nid_t dac = spec->dac_info[i].dac;
> > -		if (!dac || (dac & DAC_SLAVE_FLAG))
> > +		if (!dac)
> >  			continue;
> 
> With the fix like the above, the whole NULL-dac check should be
> skipped, too.
> 
> How about the revised patch below?

We should also check the connectivity to the tested DAC.
Please check this revised version.


thanks,

Takashi

---
diff --git a/sound/pci/hda/patch_conexant.c b/sound/pci/hda/patch_conexant.c
index 5616444..e5b06dd 100644
--- a/sound/pci/hda/patch_conexant.c
+++ b/sound/pci/hda/patch_conexant.c
@@ -3369,20 +3369,33 @@ static int fill_cx_auto_dacs(struct hda_codec *codec, hda_nid_t *dacs)
 	return nums;
 }
 
+/* if the pin has a connection with the given DAC, return it as a slave DAC */
+static hda_nid_t get_slave_dac(struct hda_codec *codec, hda_nid_t pin,
+			       hda_nid_t dac)
+{
+	if (get_connection_index(codec, pin, dac) >= 0)
+		return dac | DAC_SLAVE_FLAG;
+	return 0;
+}
+
 /* fill pin_dac_pair list from the pin and dac list */
 static int fill_dacs_for_pins(struct hda_codec *codec, hda_nid_t *pins,
 			      int num_pins, hda_nid_t *dacs, int *rest,
-			      struct pin_dac_pair *filled, int type)
+			      struct pin_dac_pair *filled, int nums,
+			      int type)
 {
-	int i, nums;
+	int i, start = nums;
 
-	nums = 0;
 	for (i = 0; i < num_pins; i++) {
 		filled[nums].pin = pins[i];
 		filled[nums].type = type;
 		filled[nums].dac = get_unassigned_dac(codec, pins[i], dacs, rest);
-		if (!filled[nums].dac && i > 0 && filled[0].dac)
-			filled[nums].dac = filled[0].dac | DAC_SLAVE_FLAG;
+		if (!filled[nums].dac && i > 0 && filled[start].dac)
+			filled[nums].dac = get_slave_dac(codec, pins[i],
+							 filled[start].dac);
+		if (!filled[nums].dac && filled[0].dac)
+			filled[nums].dac = get_slave_dac(codec, pins[i],
+							 filled[0].dac);
 		nums++;
 	}
 	return nums;
@@ -3398,14 +3411,15 @@ static void cx_auto_parse_output(struct hda_codec *codec)
 
 	rest = fill_cx_auto_dacs(codec, dacs);
 	/* parse all analog output pins */
+	nums = 0;
 	nums = fill_dacs_for_pins(codec, cfg->line_out_pins, cfg->line_outs,
-				  dacs, &rest, spec->dac_info,
+				  dacs, &rest, spec->dac_info, nums,
 				  AUTO_PIN_LINE_OUT);
-	nums += fill_dacs_for_pins(codec, cfg->hp_pins, cfg->hp_outs,
-				  dacs, &rest, spec->dac_info + nums,
+	nums = fill_dacs_for_pins(codec, cfg->hp_pins, cfg->hp_outs,
+				  dacs, &rest, spec->dac_info, nums,
 				  AUTO_PIN_HP_OUT);
-	nums += fill_dacs_for_pins(codec, cfg->speaker_pins, cfg->speaker_outs,
-				  dacs, &rest, spec->dac_info + nums,
+	nums = fill_dacs_for_pins(codec, cfg->speaker_pins, cfg->speaker_outs,
+				  dacs, &rest, spec->dac_info, nums,
 				  AUTO_PIN_SPEAKER_OUT);
 	spec->dac_info_filled = nums;
 	/* fill multiout struct */
@@ -4173,9 +4187,11 @@ static int try_add_pb_volume(struct hda_codec *codec, hda_nid_t dac,
 			     hda_nid_t pin, const char *name, int idx)
 {
 	unsigned int caps;
-	caps = query_amp_caps(codec, dac, HDA_OUTPUT);
-	if (caps & AC_AMPCAP_NUM_STEPS)
-		return cx_auto_add_pb_volume(codec, dac, name, idx);
+	if (dac && !(dac & DAC_SLAVE_FLAG)) {
+		caps = query_amp_caps(codec, dac, HDA_OUTPUT);
+		if (caps & AC_AMPCAP_NUM_STEPS)
+			return cx_auto_add_pb_volume(codec, dac, name, idx);
+	}
 	caps = query_amp_caps(codec, pin, HDA_OUTPUT);
 	if (caps & AC_AMPCAP_NUM_STEPS)
 		return cx_auto_add_pb_volume(codec, pin, name, idx);
@@ -4198,8 +4214,6 @@ static int cx_auto_build_output_controls(struct hda_codec *codec)
 		const char *label;
 		int idx, type;
 		hda_nid_t dac = spec->dac_info[i].dac;
-		if (!dac || (dac & DAC_SLAVE_FLAG))
-			continue;
 		type = spec->dac_info[i].type;
 		if (type == AUTO_PIN_LINE_OUT)
 			type = spec->autocfg.line_out_type;


More information about the Alsa-devel mailing list