[alsa-devel] Balanced output support for Xonar: Patches for Virtuoso driver to add mixer option for balanced mono output (PCM179x dacs)
This patch adds a mixer switch for soundcards using the snd-virtuoso driver and pcm1796/pcm1792(a) DAC(s) which switches between Stereo, Balanced Left, and Balanced Right output modes.
The PCM179x series has a "Monaural Mode" that outputs a single channel in differential (and therefore balanced, because of the buffer circuit) mode, which boosts the SNR and allows for common mode rejection (with the proper cable). Simply use the left output as the noninverting signal and the right output as the inverting output. The datasheets for both the pcm1792a and pcm1796 provide application suggestions, which boil down mainly to "connect an XLR jack to the outputs of the normal application circuit".
I tested this thoroughly with an Asus Xonar Essence ST, although it should also work with the HDAV, Essence STX, and probably other cards. I get no noticeable interference when running balanced audio from the card over approximately 150ft of unshielded twisted pair cable which I have running through my walls.
The driver defaults to stereo output and this patch will not cause issues for users who do not need/want this feature. The patches that I am posting only give one control regardless of whether or not more than one DAC is connected. I have another patchset that gives individual control of all 4 dacs in the Xonar Essence ST+H6 and HDAV+H6 setups (H6 is an optional daughterboard for surround sound). It allows for 4 balanced outputs, 8 unbalanced outputs, or any combination of balanced and unbalanced outputs. HOWEVER, it adds 4 mixer controls and the code isn't very pretty. If people want it, I'll post it as well and/or take suggestions on how to improve it.
Here is a diff of xonar_pcm179x.c, which is the only changed file. The diff is against alsa-driver 1.0.23 but should cleanly patch the GIT revision. Please tell me if this is not the correct way to post a patch, and tell me if it seems like a candidate to be merged.
Christian Wisner-Carlson
--- ./alsa-driver-1.0.23/alsa-kernel/pci/oxygen/xonar_pcm179x.c 2010-04-16 07:10:10.000000000 -0400 +++ ./anothermono/alsa-driver-1.0.23/alsa-kernel/pci/oxygen/xonar_pcm179x.c 2010-10-30 05:28:55.000000000 -0400 @@ -176,6 +176,8 @@ unsigned int current_rate; bool os_128; bool hp_active; + bool monomode; + bool rightmono; s8 hp_gain_offset; bool has_cs2000; u8 cs2000_fun_cfg_1; @@ -532,6 +534,13 @@ reg = PCM1796_OS_64; else reg = PCM1796_OS_32; + + if (data->monomode) + reg = reg | PCM1796_MONO; + + if (data->rightmono) + reg = reg | PCM1796_CHSL_RIGHT; + for (i = 0; i < data->dacs; ++i) pcm1796_write_cached(chip, i, 20, reg); } @@ -719,6 +728,19 @@ return 0; }
+static int monomode_info(struct snd_kcontrol *ctl, struct snd_ctl_elem_info *info) +{ + static const char *const names[3] = { "Stereo", "Balanced Left", "Balanced Right" }; + + info->type = SNDRV_CTL_ELEM_TYPE_ENUMERATED; + info->count = 1; + info->value.enumerated.items = 3; + if (info->value.enumerated.item >= 3) + info->value.enumerated.item = 0; + strcpy(info->value.enumerated.name, names[info->value.enumerated.item]); + return 0; +} + static int os_128_get(struct snd_kcontrol *ctl, struct snd_ctl_elem_value *value) { @@ -729,6 +751,18 @@ return 0; }
+static int monomode_get(struct snd_kcontrol *ctl, + struct snd_ctl_elem_value *value) +{ + struct oxygen *chip = ctl->private_data; + struct xonar_pcm179x *data = chip->model_data; + int newvalue; + newvalue = data->monomode + (data->monomode & data->rightmono); + + value->value.enumerated.item[0] = newvalue; + return 0; +} + static int os_128_put(struct snd_kcontrol *ctl, struct snd_ctl_elem_value *value) { @@ -751,6 +785,25 @@ return changed; }
+static int monomode_put(struct snd_kcontrol *ctl, + struct snd_ctl_elem_value *value) +{ + struct oxygen *chip = ctl->private_data; + struct xonar_pcm179x *data = chip->model_data; + int changed; + + mutex_lock(&chip->mutex); + changed = value->value.enumerated.item[0] != + (data->monomode + (data->monomode & data->rightmono)); + if (changed) { + data->monomode = (value->value.enumerated.item[0]) ? 1 : 0; + data->rightmono = (value->value.enumerated.item[0] == 2) ? 1 : 0; + update_pcm1796_oversampling(chip); + } + mutex_unlock(&chip->mutex); + return changed; +} + static const struct snd_kcontrol_new os_128_control = { .iface = SNDRV_CTL_ELEM_IFACE_MIXER, .name = "DAC Oversampling Playback Enum", @@ -759,6 +812,14 @@ .put = os_128_put, };
+static const struct snd_kcontrol_new monomode_control = { + .iface = SNDRV_CTL_ELEM_IFACE_MIXER, + .name = "DAC Mono Mode Playback Enum", + .info = monomode_info, + .get = monomode_get, + .put = monomode_put, +}; + static int st_output_switch_info(struct snd_kcontrol *ctl, struct snd_ctl_elem_info *info) { @@ -932,6 +993,9 @@ err = snd_ctl_add(chip->card, snd_ctl_new1(&os_128_control, chip)); if (err < 0) return err; + err = snd_ctl_add(chip->card, snd_ctl_new1(&monomode_control, chip)); + if (err < 0) + return err; return 0; }
I am also in the process of adding a Phase Invert switch and (possibly) Deemphasis control for the PCM179x DACs as well, and will post patches if anyone is interested.
Christian Wisner-Carlson wrote:
I tested this thoroughly with an Asus Xonar Essence ST, although it should also work with the HDAV, Essence STX, and probably other cards.
D2/D2X, Xense.
The patches that I am posting only give one control regardless of whether or not more than one DAC is connected. I have another patchset that gives individual control of all 4 dacs in the Xonar Essence ST+H6 and HDAV+H6 setups (H6 is an optional daughterboard for surround sound).
BTW: any new information about the H6 problem?
It allows for 4 balanced outputs, 8 unbalanced outputs, or any combination of balanced and unbalanced outputs. HOWEVER, it adds 4 mixer controls and the code isn't very pretty.
The controller can route any stereo pair to each DAC, so it would be a good idea to have one global control that also affects this routing so that, e.g., configuring the card for 4 outputs works correctly when playing 4-channel data.
Alternatively, having low-level controls for each detail of the hardware is workable when somebody (i.e., you :-) writes a special graphical tool to configure the Xonar outputs (any the other settings).
The Xense has one PCM1796 for the front channels and a CS4362A for the other channels. Like the stereo-only cards, this would result in an odd number of channels in balanced mode, which is somewhat counterintuitive because the controller would need to be fed an extra unused channel.
Please tell me if this is not the correct way to post a patch, and tell me if it seems like a candidate to be merged.
Your mailer wrapped lines; see Documentation/email-clients.txt. Also see Documentation/SubmittingPatches.
Overall, this patch looks good. But I wouldn't want to apply it without the followup patch that handles the D2's four DACs.
@@ -532,6 +534,13 @@ reg = PCM1796_OS_64; else reg = PCM1796_OS_32;
- if (data->monomode)
reg = reg | PCM1796_MONO;- if (data->rightmono)
reg = reg | PCM1796_CHSL_RIGHT;- for (i = 0; i < data->dacs; ++i) pcm1796_write_cached(chip, i, 20, reg);
}
This function's purpose is no longer to _only_ update the oversampling setting; please rename it to, e.g., update_reg20.
Regards, Clemens
I have the H6 working somewhat. I added a lot of I2C resets (and a loop to make sure the bus is free before trying to write to it). HOWEVER, when the H6 is connected, and i switch from 128x OS, the sound stops and becomes a high pitched whine or other noise, or just stops, as if the DACs lost lock on the MCLK signal. The same thing happens when i play ANYTHING at 96khz or 192khz sampling rate. However, 88.2khz still plays at 64x OS. Given that a 24.576MHz single-ended signalis being sent UNBUFFERED over an UNSHIELDED ribbon cable, this does not surprise me. I am guessing that the Asus drivers probably use a lower MCLK rate. Any ideas? My old Audigy 2 buffered the MCLK (24.576Mhz) with a 74F125, although I am reluctant to do this. Perhaps I will try replacing the relevent part of the ribbon cable with a shielded cable. Is this worth trying?
Christian Wisner-Carlson
On Mon, Nov 1, 2010 at 11:59 AM, Clemens Ladisch clemens@ladisch.de wrote:
Christian Wisner-Carlson wrote:
I tested this thoroughly with an Asus Xonar Essence ST, although it should also work with the HDAV, Essence STX, and probably other cards.
D2/D2X, Xense.
The patches that I am posting only give one control regardless of whether or not more than one DAC is connected. I have another patchset that gives individual control of all 4 dacs in the Xonar Essence ST+H6 and HDAV+H6 setups (H6 is an optional daughterboard for surround sound).
BTW: any new information about the H6 problem?
It allows for 4 balanced outputs, 8 unbalanced outputs, or any combination of balanced and unbalanced outputs. HOWEVER, it adds 4 mixer controls and the code isn't very pretty.
The controller can route any stereo pair to each DAC, so it would be a good idea to have one global control that also affects this routing so that, e.g., configuring the card for 4 outputs works correctly when playing 4-channel data.
Alternatively, having low-level controls for each detail of the hardware is workable when somebody (i.e., you :-) writes a special graphical tool to configure the Xonar outputs (any the other settings).
The Xense has one PCM1796 for the front channels and a CS4362A for the other channels. Like the stereo-only cards, this would result in an odd number of channels in balanced mode, which is somewhat counterintuitive because the controller would need to be fed an extra unused channel.
Please tell me if this is not the correct way to post a patch, and tell me if it seems like a candidate to be merged.
Your mailer wrapped lines; see Documentation/email-clients.txt. Also see Documentation/SubmittingPatches.
Overall, this patch looks good. But I wouldn't want to apply it without the followup patch that handles the D2's four DACs.
@@ -532,6 +534,13 @@ reg = PCM1796_OS_64; else reg = PCM1796_OS_32;
- if (data->monomode)
- reg = reg | PCM1796_MONO;
- if (data->rightmono)
- reg = reg | PCM1796_CHSL_RIGHT;
for (i = 0; i < data->dacs; ++i) pcm1796_write_cached(chip, i, 20, reg); }
This function's purpose is no longer to _only_ update the oversampling setting; please rename it to, e.g., update_reg20.
Regards, Clemens _______________________________________________ Alsa-devel mailing list Alsa-devel@alsa-project.org http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
Christian Wisner-Carlson wrote:
I have the H6 working somewhat. I added a lot of I2C resets (and a loop to make sure the bus is free before trying to write to it).
Can you find out when exactly the I²C bus gets wedged? (I'd guess at the first access to an off-board DAC.)
Do any of the I²C writes to the H6 actually get through (i.e., do all eight volume controls work?)
(The first D2 Windows driver versions had a bug, taken over from the AK4396 models, where it used the wrong SPI output for the fourth DAC, so that that chip would never get any register write. Nobody noticed.)
HOWEVER, when the H6 is connected, and i switch from 128x OS,
"to"?
the sound stops and becomes a high pitched whine or other noise, or just stops, as if the DACs lost lock on the MCLK signal. The same thing happens when i play ANYTHING at 96khz or 192khz sampling rate. However, 88.2khz still plays at 64x OS. Given that a 24.576MHz single-ended signalis being sent UNBUFFERED over an UNSHIELDED ribbon cable, this does not surprise me. I am guessing that the Asus drivers probably use a lower MCLK rate. Any ideas?
As far as I can tell (without running the Windows driver), it uses the standard 256x/128x MCLK rates for the standard I²S output, but the CS2000 (connected to the "ADC1" I²S MCLK) is always configured for half that rate although the PCM179x registers are still configured for the 'full' rate.
Can you find out which of the I²S MCLK outputs is used for the onboard DAC and for the H6? I'd conclude from the above that the CS2000 might be used for all of them, but then I don't know how the HDAV would work with the H6 at high clock rates.
Does the Windows driver allow 192 kHz with the H6? (This is using 128x in both drivers.)
My old Audigy 2 buffered the MCLK (24.576Mhz) with a 74F125, although I am reluctant to do this. Perhaps I will try replacing the relevent part of the ribbon cable with a shielded cable. Is this worth trying?
Better try changing mclk_from_rate() to always use the smallest possible MCLK rate; update_pcm1796_oversampling() then needs to be changed too. According to the PCM179x datasheets, up to 48 kHz requires at least 256x, while 96-192 kHz can run at 128x. (We always use I²C fast mode, even with codecs that are not documented to support it; the CMI8788 seems to be buggy at the standard I²C speed.)
Regards, Clemens
It allows for 4 balanced outputs, 8 unbalanced outputs, or any combination of balanced and unbalanced outputs. HOWEVER, it adds 4 mixer controls and the code isn't very pretty.
The controller can route any stereo pair to each DAC, so it would be a good idea to have one global control that also affects this routing so that, e.g., configuring the card for 4 outputs works correctly when playing 4-channel data.
Currently, routing is set up by void oxygen_update_dac_routing(struct oxygen *chip) in oxygen_mixer.c. However, it only allows for five different routing configurations, which it statically defines internally as int reg_values[5]. Do you mind (ie, would you mind merging/using it) if I defined a new struct: struct oxygen_routing_table = { u8 channels; unsigned int dac0; unsigned int dac1; unsigned int dac2; unsigned int dac3; } that would be passed to oxygen_update_dac_routing() as a new field of struct oxygen? This would allow for arbitrary routing configurations for the dacs (needed to implement 4 channel balanced output) and would not require many changes to the driver as a whole. Where should I define this struct? (ie, in what file?)
Thanks, Christian Wisner-Carlson
I realize that this is not completely necessary, but it really would get rid of a lot of potential duplicate code and could allow some current code to be simplified.
On Wed, Nov 3, 2010 at 12:50 AM, Christian Wisner-Carlson christian@freedomofknowledge.org wrote:
It allows for 4 balanced outputs, 8 unbalanced outputs, or any combination of balanced and unbalanced outputs. HOWEVER, it adds 4 mixer controls and the code isn't very pretty.
The controller can route any stereo pair to each DAC, so it would be a good idea to have one global control that also affects this routing so that, e.g., configuring the card for 4 outputs works correctly when playing 4-channel data.
Currently, routing is set up by void oxygen_update_dac_routing(struct oxygen *chip) in oxygen_mixer.c. However, it only allows for five different routing configurations, which it statically defines internally as int reg_values[5]. Do you mind (ie, would you mind merging/using it) if I defined a new struct: struct oxygen_routing_table = { u8 channels; unsigned int dac0; unsigned int dac1; unsigned int dac2; unsigned int dac3; } that would be passed to oxygen_update_dac_routing() as a new field of struct oxygen? This would allow for arbitrary routing configurations for the dacs (needed to implement 4 channel balanced output) and would not require many changes to the driver as a whole. Where should I define this struct? (ie, in what file?)
Thanks, Christian Wisner-Carlson
Christian Wisner-Carlson wrote:
It allows for 4 balanced outputs, 8 unbalanced outputs, or any combination of balanced and unbalanced outputs. HOWEVER, it adds 4 mixer controls and the code isn't very pretty.
The controller can route any stereo pair to each DAC, so it would be a good idea to have one global control that also affects this routing so that, e.g., configuring the card for 4 outputs works correctly when playing 4-channel data.
Currently, routing is set up by void oxygen_update_dac_routing(struct oxygen *chip) in oxygen_mixer.c. However, it only allows for five different routing configurations, which it statically defines internally as int reg_values[5]. Do you mind (ie, would you mind merging/using it) if I defined a new struct: struct oxygen_routing_table = { u8 channels; unsigned int dac0; unsigned int dac1; unsigned int dac2; unsigned int dac3; } that would be passed to oxygen_update_dac_routing() as a new field of struct oxygen? This would allow for arbitrary routing configurations for the dacs (needed to implement 4 channel balanced output) and would not require many changes to the driver as a whole.
This would move the responsibility for the actual routing decisions from this function into its callers. However, both the stereo-upmixing and the balanced-output controls affect the routing, so some code has to handle the interdependencies. At the moment, this function sets the routing depending on the current channel count and the upmixing setting; I think that the balanced-output setting is just a third such parameter.
Anyway, I'm just concerned about how the controls looks to the user, not so much about its implementation, which can be changed later (if I wanted to, and if I had the time).
I'd be happy to merge any code that handles four DACs. Separate controls for each DAC are OK too if, eventually, there are additional controls for the routing, and if there is a userspace tool to manage these settings together.
Where should I define this struct? (ie, in what file?)
As an interface between the core driver and module-specific drivers, oxygen.h.
Regards, Clemens
participants (2)
-
Christian Wisner-Carlson -
Clemens Ladisch