At Thu, 18 Dec 2008 01:40:47 +1100, Ben Stanley wrote:
Hi,
I previously reported problems that are still plaguing me [1] to do with SPDIF settings that don't reset properly. I am now looking at doing a patch to the ca0106 driver to fix this problem.
It appears I have to add a new control to be able to set the default settings. I presume this introduces complications with existing settings in asound.state? Is there some collected wisdom on how to add new controls while providing a migration/upgrade/interoperability path?
Not a big problem, I guess, as the "default" setting still remains.
For example, try the patch below. This will add new iec958 controls but the default controls remain. In this patch, the default control always overrides the per-stream setting, and this isn't quite good. But it's needed for compatibility of old alsa-lib config.
Once this patch is applied, we can change the alsa-lib config to use "IEC958 PCM Stream" instead of "IEC958 Default" so that the setting will be recovered at close.
Takashi
--- diff --git a/sound/pci/ca0106/ca0106.h b/sound/pci/ca0106/ca0106.h index a6943cb..14b8d9a 100644 --- a/sound/pci/ca0106/ca0106.h +++ b/sound/pci/ca0106/ca0106.h @@ -694,7 +694,8 @@ struct snd_ca0106 {
struct snd_ca0106_channel playback_channels[4]; struct snd_ca0106_channel capture_channels[4]; - u32 spdif_bits[4]; /* s/pdif out setup */ + u32 spdif_bits[4]; /* s/pdif out default setup */ + u32 spdif_str_bits[4]; /* s/pdif out per-stream setup */ int spdif_enable; int capture_source; int i2c_capture_source; diff --git a/sound/pci/ca0106/ca0106_main.c b/sound/pci/ca0106/ca0106_main.c index 5763174..f1036e2 100644 --- a/sound/pci/ca0106/ca0106_main.c +++ b/sound/pci/ca0106/ca0106_main.c @@ -479,6 +479,15 @@ static const int spi_dacd_bit[] = { [PCM_UNKNOWN_CHANNEL] = SPI_DACD1_BIT, };
+static void restore_spdif_bits(struct snd_ca0106 *chip, int idx) +{ + if (chip->spdif_str_bits[idx] != chip->spdif_bits[idx]) { + chip->spdif_str_bits[idx] = chip->spdif_bits[idx]; + snd_ca0106_ptr_write(chip, SPCS0 + idx, 0, + chip->spdif_str_bits[idx]); + } +} + /* open_playback callback */ static int snd_ca0106_pcm_open_playback_channel(struct snd_pcm_substream *substream, int channel_id) @@ -524,6 +533,9 @@ static int snd_ca0106_pcm_open_playback_channel(struct snd_pcm_substream *substr if (err < 0) return err; } + + restore_spdif_bits(chip, channel_id); + return 0; }
@@ -535,6 +547,8 @@ static int snd_ca0106_pcm_close_playback(struct snd_pcm_substream *substream) struct snd_ca0106_pcm *epcm = runtime->private_data; chip->playback_channels[epcm->channel_id].use = 0;
+ restore_spdif_bits(chip, epcm->channel_id); + if (chip->details->spi_dac && epcm->channel_id != PCM_FRONT_CHANNEL) { const int reg = spi_dacd_reg[epcm->channel_id];
diff --git a/sound/pci/ca0106/ca0106_mixer.c b/sound/pci/ca0106/ca0106_mixer.c index cccc32c..8727881 100644 --- a/sound/pci/ca0106/ca0106_mixer.c +++ b/sound/pci/ca0106/ca0106_mixer.c @@ -148,7 +148,7 @@ static void ca0106_set_capture_mic_line_in(struct snd_ca0106 *emu)
static void ca0106_set_spdif_bits(struct snd_ca0106 *emu, int idx) { - snd_ca0106_ptr_write(emu, SPCS0 + idx, 0, emu->spdif_bits[idx]); + snd_ca0106_ptr_write(emu, SPCS0 + idx, 0, emu->spdif_str_bits[idx]); }
/* @@ -353,16 +353,33 @@ static int snd_ca0106_spdif_info(struct snd_kcontrol *kcontrol, return 0; }
-static int snd_ca0106_spdif_get(struct snd_kcontrol *kcontrol, +static void decode_spdif_bits(unsigned char *status, unsigned int bits) +{ + status[0] = (bits >> 0) & 0xff; + status[1] = (bits >> 8) & 0xff; + status[2] = (bits >> 16) & 0xff; + status[3] = (bits >> 24) & 0xff; +} + +static int snd_ca0106_spdif_get_default(struct snd_kcontrol *kcontrol, struct snd_ctl_elem_value *ucontrol) { struct snd_ca0106 *emu = snd_kcontrol_chip(kcontrol); unsigned int idx = snd_ctl_get_ioffidx(kcontrol, &ucontrol->id);
- ucontrol->value.iec958.status[0] = (emu->spdif_bits[idx] >> 0) & 0xff; - ucontrol->value.iec958.status[1] = (emu->spdif_bits[idx] >> 8) & 0xff; - ucontrol->value.iec958.status[2] = (emu->spdif_bits[idx] >> 16) & 0xff; - ucontrol->value.iec958.status[3] = (emu->spdif_bits[idx] >> 24) & 0xff; + decode_spdif_bits(ucontrol->value.iec958.status, + emu->spdif_bits[idx]); + return 0; +} + +static int snd_ca0106_spdif_get_stream(struct snd_kcontrol *kcontrol, + struct snd_ctl_elem_value *ucontrol) +{ + struct snd_ca0106 *emu = snd_kcontrol_chip(kcontrol); + unsigned int idx = snd_ctl_get_ioffidx(kcontrol, &ucontrol->id); + + decode_spdif_bits(ucontrol->value.iec958.status, + emu->spdif_str_bits[idx]); return 0; }
@@ -376,24 +393,48 @@ static int snd_ca0106_spdif_get_mask(struct snd_kcontrol *kcontrol, return 0; }
-static int snd_ca0106_spdif_put(struct snd_kcontrol *kcontrol, +static unsigned int encode_spdif_bits(unsigned char *status) +{ + return ((unsigned int)status[0] << 0) | + ((unsigned int)status[1] << 8) | + ((unsigned int)status[2] << 16) | + ((unsigned int)status[3] << 24); +} + +static int snd_ca0106_spdif_put_default(struct snd_kcontrol *kcontrol, struct snd_ctl_elem_value *ucontrol) { struct snd_ca0106 *emu = snd_kcontrol_chip(kcontrol); unsigned int idx = snd_ctl_get_ioffidx(kcontrol, &ucontrol->id); - int change; unsigned int val;
- val = (ucontrol->value.iec958.status[0] << 0) | - (ucontrol->value.iec958.status[1] << 8) | - (ucontrol->value.iec958.status[2] << 16) | - (ucontrol->value.iec958.status[3] << 24); - change = val != emu->spdif_bits[idx]; - if (change) { + val = encode_spdif_bits(ucontrol->value.iec958.status); + if (val != emu->spdif_bits[idx]) { emu->spdif_bits[idx] = val; + /* FIXME: this isn't safe, but needed to keep the compatibility + * with older alsa-lib config + */ + emu->spdif_str_bits[idx] = val; ca0106_set_spdif_bits(emu, idx); + return 1; } - return change; + return 0; +} + +static int snd_ca0106_spdif_put_stream(struct snd_kcontrol *kcontrol, + struct snd_ctl_elem_value *ucontrol) +{ + struct snd_ca0106 *emu = snd_kcontrol_chip(kcontrol); + unsigned int idx = snd_ctl_get_ioffidx(kcontrol, &ucontrol->id); + unsigned int val; + + val = encode_spdif_bits(ucontrol->value.iec958.status); + if (val != emu->spdif_str_bits[idx]) { + emu->spdif_str_bits[idx] = val; + ca0106_set_spdif_bits(emu, idx); + return 1; + } + return 0; }
static int snd_ca0106_volume_info(struct snd_kcontrol *kcontrol, @@ -604,8 +645,16 @@ static struct snd_kcontrol_new snd_ca0106_volume_ctls[] __devinitdata = { .name = SNDRV_CTL_NAME_IEC958("",PLAYBACK,DEFAULT), .count = 4, .info = snd_ca0106_spdif_info, - .get = snd_ca0106_spdif_get, - .put = snd_ca0106_spdif_put + .get = snd_ca0106_spdif_get_default, + .put = snd_ca0106_spdif_put_default + }, + { + .iface = SNDRV_CTL_ELEM_IFACE_PCM, + .name = SNDRV_CTL_NAME_IEC958("",PLAYBACK,PCM_STREAM), + .count = 4, + .info = snd_ca0106_spdif_info, + .get = snd_ca0106_spdif_get_stream, + .put = snd_ca0106_spdif_put_stream }, };