alsa-devel.alsa-project.org archive mirror
 help / color / mirror / Atom feed
* [PATCH RFC] ALSA: oxygen: Xonar DG(X): fix Stereo Upmixing regression
@ 2014-03-17 21:12 Clemens Ladisch
  2014-03-18  7:35 ` Roman Volkov
  2014-03-18  8:31 ` [PATCH] " Clemens Ladisch
  0 siblings, 2 replies; 5+ messages in thread
From: Clemens Ladisch @ 2014-03-17 21:12 UTC (permalink / raw)
  To: Roman Volkov; +Cc: alsa-devel

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.

Signed-off-by: Clemens Ladisch <clemens@ladisch.de>
---
 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?


diff --git a/sound/pci/oxygen/xonar_dg.c b/sound/pci/oxygen/xonar_dg.c
index ed6f199..4cf3200 100644
--- a/sound/pci/oxygen/xonar_dg.c
+++ b/sound/pci/oxygen/xonar_dg.c
@@ -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,

^ permalink raw reply related	[flat|nested] 5+ messages in thread
* Re: [PATCH RFC] ALSA: oxygen: Xonar DG(X): fix Stereo Upmixing regression
  2014-03-17 21:12 [PATCH RFC] ALSA: oxygen: Xonar DG(X): fix Stereo Upmixing regression Clemens Ladisch
@ 2014-03-18  7:35 ` Roman Volkov
  2014-03-18  8:31   ` Clemens Ladisch
  2014-03-18  8:31 ` [PATCH] " Clemens Ladisch
  1 sibling, 1 reply; 5+ messages in thread
From: Roman Volkov @ 2014-03-18  7:35 UTC (permalink / raw)
  To: Clemens Ladisch; +Cc: alsa-devel

В Mon, 17 Mar 2014 22:12:54 +0100
Clemens Ladisch <clemens@ladisch.de> пишет:

> 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.
> 
> Signed-off-by: Clemens Ladisch <clemens@ladisch.de>
> ---
>  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?
> 
> 
> diff --git a/sound/pci/oxygen/xonar_dg.c b/sound/pci/oxygen/xonar_dg.c
> index ed6f199..4cf3200 100644
> --- a/sound/pci/oxygen/xonar_dg.c
> +++ b/sound/pci/oxygen/xonar_dg.c
> @@ -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,
> 

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.

-- 
Kind regards,
Roman Volkov,
v1ron@mail.ru
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

^ permalink raw reply	[flat|nested] 5+ messages in thread
* [PATCH] ALSA: oxygen: Xonar DG(X): fix Stereo Upmixing regression
  2014-03-17 21:12 [PATCH RFC] ALSA: oxygen: Xonar DG(X): fix Stereo Upmixing regression Clemens Ladisch
  2014-03-18  7:35 ` Roman Volkov
@ 2014-03-18  8:31 ` Clemens Ladisch
  2014-03-18  8:53   ` Takashi Iwai
  1 sibling, 1 reply; 5+ messages in thread
From: Clemens Ladisch @ 2014-03-18  8:31 UTC (permalink / raw)
  To: Takashi Iwai; +Cc: alsa-devel, Roman Volkov

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.

Tested-by: Roman Volkov <v1ron@mail.ru>
Signed-off-by: Clemens Ladisch <clemens@ladisch.de>
---
 sound/pci/oxygen/xonar_dg.c |   30 ++++++++++++++++++++++++------
 1 file changed, 24 insertions(+), 6 deletions(-)

--- a/sound/pci/oxygen/xonar_dg.c
+++ b/sound/pci/oxygen/xonar_dg.c
@@ -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,

^ permalink raw reply	[flat|nested] 5+ messages in thread
* Re: [PATCH RFC] ALSA: oxygen: Xonar DG(X): fix Stereo Upmixing regression
  2014-03-18  7:35 ` Roman Volkov
@ 2014-03-18  8:31   ` Clemens Ladisch
  0 siblings, 0 replies; 5+ messages in thread
From: Clemens Ladisch @ 2014-03-18  8:31 UTC (permalink / raw)
  To: Roman Volkov; +Cc: alsa-devel

Roman Volkov wrote:
> Clemens Ladisch <clemens@ladisch.de> пишет:
>> 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.
>>
>> Signed-off-by: Clemens Ladisch <clemens@ladisch.de>
>> ---
>>  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
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

^ permalink raw reply	[flat|nested] 5+ messages in thread
* Re: [PATCH] ALSA: oxygen: Xonar DG(X): fix Stereo Upmixing regression
  2014-03-18  8:31 ` [PATCH] " Clemens Ladisch
@ 2014-03-18  8:53   ` Takashi Iwai
  0 siblings, 0 replies; 5+ messages in thread
From: Takashi Iwai @ 2014-03-18  8:53 UTC (permalink / raw)
  To: Clemens Ladisch; +Cc: alsa-devel, Roman Volkov

At Tue, 18 Mar 2014 09:31:18 +0100,
Clemens Ladisch wrote:
> 
> 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.
> 
> Tested-by: Roman Volkov <v1ron@mail.ru>
> Signed-off-by: Clemens Ladisch <clemens@ladisch.de>

Thanks, applied.


Takashi

> ---
>  sound/pci/oxygen/xonar_dg.c |   30 ++++++++++++++++++++++++------
>  1 file changed, 24 insertions(+), 6 deletions(-)
> 
> --- a/sound/pci/oxygen/xonar_dg.c
> +++ b/sound/pci/oxygen/xonar_dg.c
> @@ -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,
> 

^ permalink raw reply	[flat|nested] 5+ messages in thread
end of thread, other threads:[~2014-03-18  8:53 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-03-17 21:12 [PATCH RFC] ALSA: oxygen: Xonar DG(X): fix Stereo Upmixing regression Clemens Ladisch
2014-03-18  7:35 ` Roman Volkov
2014-03-18  8:31   ` Clemens Ladisch
2014-03-18  8:31 ` [PATCH] " Clemens Ladisch
2014-03-18  8:53   ` Takashi Iwai

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).