Skip to contentSkip to conversation

ALSA: oxygen: Xonar DG(X): fix Stereo Upmixing regression

RFC PATCH In review Clemens Ladisch 3 participants 5 messages Mar 17, 2014 – Mar 18, 2014

Subsystems (2)
lore
Share & tools
lore

Clemens Ladisch

author maintainer
#0 lore raw

RFC PATCH 1/1 ALSA: oxygen: Xonar DG(X): fix Stereo Upmixing regression

The code introduced in commit 1f91ecc14dee ("ALSA: oxygen: modify adjust_dg_dac_routing function") accidentally disregarded the old value of the playback routing register, so it broke the "Stereo Upmixing" mixer control.

The unmuted parts of the channel routing are the same for all settings of the output destination, so it suffices to revert that part of the patch.

---
 sound/pci/oxygen/xonar_dg.c |   30 ++++++++++++++++++++++++------
 1 file changed, 24 insertions(+), 6 deletions(-)

Roman, could you please test this patch, and add your Tested-by?

sound/pci/oxygen/xonar_dg.c +24 -6
index ed6f199..4cf3200 100644
@@ -238,11 +238,21 @@ void set_cs4245_adc_params(struct oxygen *chip,
 	cs4245_write_spi(chip, CS4245_MCLK_FREQ);
 }
 
+static inline unsigned int shift_bits(unsigned int value,
+				      unsigned int shift_from,
+				      unsigned int shift_to,
+				      unsigned int mask)
+{
+	if (shift_from < shift_to)
+		return (value << (shift_to - shift_from)) & mask;
+	else
+		return (value >> (shift_from - shift_to)) & mask;
+}
+
 unsigned int adjust_dg_dac_routing(struct oxygen *chip,
 					  unsigned int play_routing)
 {
 	struct dg *data = chip->model_data;
-	unsigned int routing = 0;
 
 	switch (data->output_sel) {
 	case PLAYBACK_DST_HP:
@@ -252,15 +262,23 @@ unsigned int adjust_dg_dac_routing(struct oxygen *chip,
 			OXYGEN_PLAY_MUTE67, OXYGEN_PLAY_MUTE_MASK);
 		break;
 	case PLAYBACK_DST_MULTICH:
-		routing = (0 << OXYGEN_PLAY_DAC0_SOURCE_SHIFT) |
-			  (2 << OXYGEN_PLAY_DAC1_SOURCE_SHIFT) |
-			  (1 << OXYGEN_PLAY_DAC2_SOURCE_SHIFT) |
-			  (0 << OXYGEN_PLAY_DAC3_SOURCE_SHIFT);
 		oxygen_write8_masked(chip, OXYGEN_PLAY_ROUTING,
 			OXYGEN_PLAY_MUTE01, OXYGEN_PLAY_MUTE_MASK);
 		break;
 	}
-	return routing;
+	return (play_routing & OXYGEN_PLAY_DAC0_SOURCE_MASK) |
+	       shift_bits(play_routing,
+			  OXYGEN_PLAY_DAC2_SOURCE_SHIFT,
+			  OXYGEN_PLAY_DAC1_SOURCE_SHIFT,
+			  OXYGEN_PLAY_DAC1_SOURCE_MASK) |
+	       shift_bits(play_routing,
+			  OXYGEN_PLAY_DAC1_SOURCE_SHIFT,
+			  OXYGEN_PLAY_DAC2_SOURCE_SHIFT,
+			  OXYGEN_PLAY_DAC2_SOURCE_MASK) |
+	       shift_bits(play_routing,
+			  OXYGEN_PLAY_DAC0_SOURCE_SHIFT,
+			  OXYGEN_PLAY_DAC3_SOURCE_SHIFT,
+			  OXYGEN_PLAY_DAC3_SOURCE_MASK);
 }
 
 void dump_cs4245_registers(struct oxygen *chip,
#1 lore raw

В Mon, 17 Mar 2014 22:12:54 +0100
Clemens Ladisch <[email protected]> пишет:

Quoted 65 lines

The code introduced in commit 1f91ecc14dee ("ALSA: oxygen: modify adjust_dg_dac_routing function") accidentally disregarded the old value of the playback routing register, so it broke the "Stereo Upmixing" mixer control.

The unmuted parts of the channel routing are the same for all settings of the output destination, so it suffices to revert that part of the patch.

---
 sound/pci/oxygen/xonar_dg.c |   30 ++++++++++++++++++++++++------
 1 file changed, 24 insertions(+), 6 deletions(-)

Roman, could you please test this patch, and add your Tested-by?

sound/pci/oxygen/xonar_dg.c +17 -6
index ed6f199..4cf3200 100644
@@ -238,11 +238,21 @@ void set_cs4245_adc_params(struct oxygen *chip,
 	cs4245_write_spi(chip, CS4245_MCLK_FREQ);
 }
 
+static inline unsigned int shift_bits(unsigned int value,
+				      unsigned int shift_from,
+				      unsigned int shift_to,
+				      unsigned int mask)
+{
+	if (shift_from < shift_to)
+		return (value << (shift_to - shift_from)) & mask;
+	else
+		return (value >> (shift_from - shift_to)) & mask;
+}
+
 unsigned int adjust_dg_dac_routing(struct oxygen *chip,
 					  unsigned int play_routing)
 {
 	struct dg *data = chip->model_data;
-	unsigned int routing = 0;
 
 	switch (data->output_sel) {
 	case PLAYBACK_DST_HP:
@@ -252,15 +262,23 @@ unsigned int adjust_dg_dac_routing(struct
 oxygen *chip, OXYGEN_PLAY_MUTE67, OXYGEN_PLAY_MUTE_MASK);
 		break;
 	case PLAYBACK_DST_MULTICH:
-		routing = (0 << OXYGEN_PLAY_DAC0_SOURCE_SHIFT) |
-			  (2 << OXYGEN_PLAY_DAC1_SOURCE_SHIFT) |
-			  (1 << OXYGEN_PLAY_DAC2_SOURCE_SHIFT) |
-			  (0 << OXYGEN_PLAY_DAC3_SOURCE_SHIFT);
 		oxygen_write8_masked(chip, OXYGEN_PLAY_ROUTING,
 			OXYGEN_PLAY_MUTE01, OXYGEN_PLAY_MUTE_MASK);
 		break;
 	}
-	return routing;
+	return (play_routing & OXYGEN_PLAY_DAC0_SOURCE_MASK) |
+	       shift_bits(play_routing,
+			  OXYGEN_PLAY_DAC2_SOURCE_SHIFT,
+			  OXYGEN_PLAY_DAC1_SOURCE_SHIFT,
+			  OXYGEN_PLAY_DAC1_SOURCE_MASK) |
+	       shift_bits(play_routing,

sound/pci/oxygen/xonar_dg.c unsigned int adjust_dg_dac_routing(struct · line 284

Quoted 10 lines
sound/pci/oxygen/xonar_dg.c +7 -0
+			  OXYGEN_PLAY_DAC1_SOURCE_SHIFT,
+			  OXYGEN_PLAY_DAC2_SOURCE_SHIFT,
+			  OXYGEN_PLAY_DAC2_SOURCE_MASK) |
+	       shift_bits(play_routing,
+			  OXYGEN_PLAY_DAC0_SOURCE_SHIFT,
+			  OXYGEN_PLAY_DAC3_SOURCE_SHIFT,
+			  OXYGEN_PLAY_DAC3_SOURCE_MASK);
 }
 
 void dump_cs4245_registers(struct oxygen *chip,

Hello Clemens,

I have problems with applying this patch on my local repo. Could you please tell me which git repo are you using?

I have manually reproduced the patch on my local repo and tested it. I can confirm that the "Stereo Upmixing" control was broken and the patch you attached solves this problem. I think we can apply this fix.

Signature · 7 lines
Kind regards,
Roman Volkov,
[email protected]
_______________________________________________
Alsa-devel mailing list
[email protected]
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
author maintainer
#2 lore raw
Quoted 10 lines · Roman Volkov

Clemens Ladisch <[email protected]> пишет:

Quoted 9 lines

The code introduced in commit 1f91ecc14dee ("ALSA: oxygen: modify adjust_dg_dac_routing function") accidentally disregarded the old value of the playback routing register, so it broke the "Stereo Upmixing" mixer control.

The unmuted parts of the channel routing are the same for all settings of the output destination, so it suffices to revert that part of the patch.

Quoted 7 lines
Quoted 4 lines
---
 sound/pci/oxygen/xonar_dg.c |   30 ++++++++++++++++++++++++------
 1 file changed, 24 insertions(+), 6 deletions(-)

I have problems with applying this patch on my local repo. Could you please tell me which git repo are you using?

It's based on tiwai/sound.git, but Linus' tree should work too.

Regards,
Clemens
_______________________________________________
Alsa-devel mailing list
[email protected]
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

author maintainer
#3 lore raw

PATCH 1/1 ALSA: oxygen: Xonar DG(X): fix Stereo Upmixing regression

In reply to Clemens Ladisch

The code introduced in commit 1f91ecc14dee ("ALSA: oxygen: modify adjust_dg_dac_routing function") accidentally disregarded the old value of the playback routing register, so it broke the "Stereo Upmixing" mixer control.

The unmuted parts of the channel routing are the same for all settings of the output destination, so it suffices to revert that part of the patch.

---
 sound/pci/oxygen/xonar_dg.c |   30 ++++++++++++++++++++++++------
 1 file changed, 24 insertions(+), 6 deletions(-)
sound/pci/oxygen/xonar_dg.c +24 -6
@@ -238,11 +238,21 @@ void set_cs4245_adc_params(struct oxygen *chip,
 	cs4245_write_spi(chip, CS4245_MCLK_FREQ);
 }
 
+static inline unsigned int shift_bits(unsigned int value,
+				      unsigned int shift_from,
+				      unsigned int shift_to,
+				      unsigned int mask)
+{
+	if (shift_from < shift_to)
+		return (value << (shift_to - shift_from)) & mask;
+	else
+		return (value >> (shift_from - shift_to)) & mask;
+}
+
 unsigned int adjust_dg_dac_routing(struct oxygen *chip,
 					  unsigned int play_routing)
 {
 	struct dg *data = chip->model_data;
-	unsigned int routing = 0;
 
 	switch (data->output_sel) {
 	case PLAYBACK_DST_HP:
@@ -252,15 +262,23 @@ unsigned int adjust_dg_dac_routing(struct oxygen *chip,
 			OXYGEN_PLAY_MUTE67, OXYGEN_PLAY_MUTE_MASK);
 		break;
 	case PLAYBACK_DST_MULTICH:
-		routing = (0 << OXYGEN_PLAY_DAC0_SOURCE_SHIFT) |
-			  (2 << OXYGEN_PLAY_DAC1_SOURCE_SHIFT) |
-			  (1 << OXYGEN_PLAY_DAC2_SOURCE_SHIFT) |
-			  (0 << OXYGEN_PLAY_DAC3_SOURCE_SHIFT);
 		oxygen_write8_masked(chip, OXYGEN_PLAY_ROUTING,
 			OXYGEN_PLAY_MUTE01, OXYGEN_PLAY_MUTE_MASK);
 		break;
 	}
-	return routing;
+	return (play_routing & OXYGEN_PLAY_DAC0_SOURCE_MASK) |
+	       shift_bits(play_routing,
+			  OXYGEN_PLAY_DAC2_SOURCE_SHIFT,
+			  OXYGEN_PLAY_DAC1_SOURCE_SHIFT,
+			  OXYGEN_PLAY_DAC1_SOURCE_MASK) |
+	       shift_bits(play_routing,
+			  OXYGEN_PLAY_DAC1_SOURCE_SHIFT,
+			  OXYGEN_PLAY_DAC2_SOURCE_SHIFT,
+			  OXYGEN_PLAY_DAC2_SOURCE_MASK) |
+	       shift_bits(play_routing,
+			  OXYGEN_PLAY_DAC0_SOURCE_SHIFT,
+			  OXYGEN_PLAY_DAC3_SOURCE_SHIFT,
+			  OXYGEN_PLAY_DAC3_SOURCE_MASK);
 }
 
 void dump_cs4245_registers(struct oxygen *chip,
Applied
#4 lore raw

At Tue, 18 Mar 2014 09:31:18 +0100,

Quoted 6 lines · Clemens Ladisch

The code introduced in commit 1f91ecc14dee ("ALSA: oxygen: modify adjust_dg_dac_routing function") accidentally disregarded the old value of the playback routing register, so it broke the "Stereo Upmixing" mixer control.

Quoted 6 lines

The unmuted parts of the channel routing are the same for all settings of the output destination, so it suffices to revert that part of the patch.

Thanks, applied.

Takashi

Quoted 59 lines
---
 sound/pci/oxygen/xonar_dg.c |   30 ++++++++++++++++++++++++------
 1 file changed, 24 insertions(+), 6 deletions(-)
sound/pci/oxygen/xonar_dg.c +24 -6
@@ -238,11 +238,21 @@ void set_cs4245_adc_params(struct oxygen *chip,
 	cs4245_write_spi(chip, CS4245_MCLK_FREQ);
 }
 
+static inline unsigned int shift_bits(unsigned int value,
+				      unsigned int shift_from,
+				      unsigned int shift_to,
+				      unsigned int mask)
+{
+	if (shift_from < shift_to)
+		return (value << (shift_to - shift_from)) & mask;
+	else
+		return (value >> (shift_from - shift_to)) & mask;
+}
+
 unsigned int adjust_dg_dac_routing(struct oxygen *chip,
 					  unsigned int play_routing)
 {
 	struct dg *data = chip->model_data;
-	unsigned int routing = 0;
 
 	switch (data->output_sel) {
 	case PLAYBACK_DST_HP:
@@ -252,15 +262,23 @@ unsigned int adjust_dg_dac_routing(struct oxygen *chip,
 			OXYGEN_PLAY_MUTE67, OXYGEN_PLAY_MUTE_MASK);
 		break;
 	case PLAYBACK_DST_MULTICH:
-		routing = (0 << OXYGEN_PLAY_DAC0_SOURCE_SHIFT) |
-			  (2 << OXYGEN_PLAY_DAC1_SOURCE_SHIFT) |
-			  (1 << OXYGEN_PLAY_DAC2_SOURCE_SHIFT) |
-			  (0 << OXYGEN_PLAY_DAC3_SOURCE_SHIFT);
 		oxygen_write8_masked(chip, OXYGEN_PLAY_ROUTING,
 			OXYGEN_PLAY_MUTE01, OXYGEN_PLAY_MUTE_MASK);
 		break;
 	}
-	return routing;
+	return (play_routing & OXYGEN_PLAY_DAC0_SOURCE_MASK) |
+	       shift_bits(play_routing,
+			  OXYGEN_PLAY_DAC2_SOURCE_SHIFT,
+			  OXYGEN_PLAY_DAC1_SOURCE_SHIFT,
+			  OXYGEN_PLAY_DAC1_SOURCE_MASK) |
+	       shift_bits(play_routing,
+			  OXYGEN_PLAY_DAC1_SOURCE_SHIFT,
+			  OXYGEN_PLAY_DAC2_SOURCE_SHIFT,
+			  OXYGEN_PLAY_DAC2_SOURCE_MASK) |
+	       shift_bits(play_routing,
+			  OXYGEN_PLAY_DAC0_SOURCE_SHIFT,
+			  OXYGEN_PLAY_DAC3_SOURCE_SHIFT,
+			  OXYGEN_PLAY_DAC3_SOURCE_MASK);
 }
 
 void dump_cs4245_registers(struct oxygen *chip,