[alsa-devel] [PATCH 1/1] ALSA: virtuoso: add de-emphasis control
From: Tom Yan tom.ty89@gmail.com
Add control for the de-emphasis filter in the PCM179x DACs
Signed-off-by: Tom Yan tom.ty89@gmail.com
diff --git a/sound/pci/oxygen/xonar_pcm179x.c b/sound/pci/oxygen/xonar_pcm179x.c index 24109d37ca09..0014c55ecc3d 100644 --- a/sound/pci/oxygen/xonar_pcm179x.c +++ b/sound/pci/oxygen/xonar_pcm179x.c @@ -331,7 +331,7 @@ static void pcm1796_init(struct oxygen *chip) struct xonar_pcm179x *data = chip->model_data;
data->pcm1796_regs[0][18 - PCM1796_REG_BASE] = - PCM1796_DMF_DISABLED | PCM1796_FMT_24_I2S | PCM1796_ATLD; + PCM1796_FMT_24_I2S | PCM1796_ATLD; if (!data->broken_i2c) data->pcm1796_regs[0][18 - PCM1796_REG_BASE] |= PCM1796_MUTE; data->pcm1796_regs[0][19 - PCM1796_REG_BASE] = @@ -621,6 +621,33 @@ static void update_pcm1796_oversampling(struct oxygen *chip) pcm1796_write_cached(chip, i, 20, reg); }
+static void update_pcm1796_deemph(struct oxygen *chip) +{ + struct xonar_pcm179x *data = chip->model_data; + unsigned int i; + u8 reg; + + reg = data->pcm1796_regs[0][18 - PCM1796_REG_BASE] & ~PCM1796_DMF_MASK; + switch (data->current_rate) { + case 32000: + case 64000: + reg |= PCM1796_DMF_32; + break; + case 44100: + case 88200: + case 176400: + reg |= PCM1796_DMF_441; + break; + case 48000: + case 96000: + case 192000: + reg |= PCM1796_DMF_48; + break; + } + for (i = 0; i < data->dacs; ++i) + pcm1796_write_cached(chip, i, 18, reg); +} + static void set_pcm1796_params(struct oxygen *chip, struct snd_pcm_hw_params *params) { @@ -629,6 +656,7 @@ static void set_pcm1796_params(struct oxygen *chip, msleep(1); data->current_rate = params_rate(params); update_pcm1796_oversampling(chip); + update_pcm1796_deemph(chip); }
static void update_pcm1796_volume(struct oxygen *chip) @@ -653,9 +681,11 @@ static void update_pcm1796_mute(struct oxygen *chip) unsigned int i; u8 value;
- value = PCM1796_DMF_DISABLED | PCM1796_FMT_24_I2S | PCM1796_ATLD; + value = data->pcm1796_regs[0][18 - PCM1796_REG_BASE]; if (chip->dac_mute) value |= PCM1796_MUTE; + else + value &= ~PCM1796_MUTE; for (i = 0; i < data->dacs; ++i) pcm1796_write_cached(chip, i, 18, value); } @@ -777,6 +807,50 @@ static const struct snd_kcontrol_new rolloff_control = { .put = rolloff_put, };
+static int deemph_get(struct snd_kcontrol *ctl, + struct snd_ctl_elem_value *value) +{ + struct oxygen *chip = ctl->private_data; + struct xonar_pcm179x *data = chip->model_data; + + value->value.integer.value[0] = + (data->pcm1796_regs[0][18 - PCM1796_REG_BASE] & + PCM1796_DME) != PCM1796_DMF_DISABLED; + return 0; +} + +static int deemph_put(struct snd_kcontrol *ctl, + struct snd_ctl_elem_value *value) +{ + struct oxygen *chip = ctl->private_data; + struct xonar_pcm179x *data = chip->model_data; + unsigned int i; + int changed; + u8 reg; + + mutex_lock(&chip->mutex); + reg = data->pcm1796_regs[0][18 - PCM1796_REG_BASE]; + if (!value->value.integer.value[0]) + reg &= ~PCM1796_DME; + else + reg |= PCM1796_DME; + changed = reg != data->pcm1796_regs[0][18 - PCM1796_REG_BASE]; + if (changed) { + for (i = 0; i < data->dacs; ++i) + pcm1796_write(chip, i, 18, reg); + } + mutex_unlock(&chip->mutex); + return changed; +} + +static const struct snd_kcontrol_new deemph_control = { + .iface = SNDRV_CTL_ELEM_IFACE_MIXER, + .name = "De-emphasis Switch", + .info = snd_ctl_boolean_mono_info, + .get = deemph_get, + .put = deemph_put, +}; + static const struct snd_kcontrol_new hdav_hdmi_control = { .iface = SNDRV_CTL_ELEM_IFACE_MIXER, .name = "HDMI Playback Switch", @@ -1011,6 +1085,10 @@ static int add_pcm1796_controls(struct oxygen *chip) snd_ctl_new1(&rolloff_control, chip)); if (err < 0) return err; + err = snd_ctl_add(chip->card, + snd_ctl_new1(&deemph_control, chip)); + if (err < 0) + return err; } return 0; }
tom.ty89@gmail.com wrote:
- switch (data->current_rate) {
- case 32000:
- case 64000:
reg |= PCM1796_DMF_32;break;- case 44100:
- case 88200:
- case 176400:
reg |= PCM1796_DMF_441;break;- case 48000:
- case 96000:
- case 192000:
reg |= PCM1796_DMF_48;break;
The DAC does not have filters for frequencies higher than 48 kHz.
+static const struct snd_kcontrol_new deemph_control = {
- .iface = SNDRV_CTL_ELEM_IFACE_MIXER,
- .name = "De-emphasis Switch",
This should be called "De-emphasis Playback Switch". (But I see that other drivers are quite inconsistent about this.)
Regards, Clemens
From: Tom Yan tom.ty89@gmail.com
Add control for the de-emphasis filter in the PCM179x DACs
Signed-off-by: Tom Yan tom.ty89@gmail.com
diff --git a/sound/pci/oxygen/xonar_pcm179x.c b/sound/pci/oxygen/xonar_pcm179x.c index 24109d37ca09..429411d10e08 100644 --- a/sound/pci/oxygen/xonar_pcm179x.c +++ b/sound/pci/oxygen/xonar_pcm179x.c @@ -331,7 +331,7 @@ static void pcm1796_init(struct oxygen *chip) struct xonar_pcm179x *data = chip->model_data;
data->pcm1796_regs[0][18 - PCM1796_REG_BASE] = - PCM1796_DMF_DISABLED | PCM1796_FMT_24_I2S | PCM1796_ATLD; + PCM1796_FMT_24_I2S | PCM1796_ATLD; if (!data->broken_i2c) data->pcm1796_regs[0][18 - PCM1796_REG_BASE] |= PCM1796_MUTE; data->pcm1796_regs[0][19 - PCM1796_REG_BASE] = @@ -621,6 +621,23 @@ static void update_pcm1796_oversampling(struct oxygen *chip) pcm1796_write_cached(chip, i, 20, reg); }
+static void update_pcm1796_deemph(struct oxygen *chip) +{ + struct xonar_pcm179x *data = chip->model_data; + unsigned int i; + u8 reg; + + reg = data->pcm1796_regs[0][18 - PCM1796_REG_BASE] & ~PCM1796_DMF_MASK; + if (data->current_rate == 48000) + reg |= PCM1796_DMF_48; + else if (data->current_rate == 44100) + reg |= PCM1796_DMF_441; + else if (data->current_rate == 32000) + reg |= PCM1796_DMF_32; + for (i = 0; i < data->dacs; ++i) + pcm1796_write_cached(chip, i, 18, reg); +} + static void set_pcm1796_params(struct oxygen *chip, struct snd_pcm_hw_params *params) { @@ -629,6 +646,7 @@ static void set_pcm1796_params(struct oxygen *chip, msleep(1); data->current_rate = params_rate(params); update_pcm1796_oversampling(chip); + update_pcm1796_deemph(chip); }
static void update_pcm1796_volume(struct oxygen *chip) @@ -653,9 +671,11 @@ static void update_pcm1796_mute(struct oxygen *chip) unsigned int i; u8 value;
- value = PCM1796_DMF_DISABLED | PCM1796_FMT_24_I2S | PCM1796_ATLD; + value = data->pcm1796_regs[0][18 - PCM1796_REG_BASE]; if (chip->dac_mute) value |= PCM1796_MUTE; + else + value &= ~PCM1796_MUTE; for (i = 0; i < data->dacs; ++i) pcm1796_write_cached(chip, i, 18, value); } @@ -777,6 +797,50 @@ static const struct snd_kcontrol_new rolloff_control = { .put = rolloff_put, };
+static int deemph_get(struct snd_kcontrol *ctl, + struct snd_ctl_elem_value *value) +{ + struct oxygen *chip = ctl->private_data; + struct xonar_pcm179x *data = chip->model_data; + + value->value.integer.value[0] = + (data->pcm1796_regs[0][18 - PCM1796_REG_BASE] & + PCM1796_DME) != PCM1796_DMF_DISABLED; + return 0; +} + +static int deemph_put(struct snd_kcontrol *ctl, + struct snd_ctl_elem_value *value) +{ + struct oxygen *chip = ctl->private_data; + struct xonar_pcm179x *data = chip->model_data; + unsigned int i; + int changed; + u8 reg; + + mutex_lock(&chip->mutex); + reg = data->pcm1796_regs[0][18 - PCM1796_REG_BASE]; + if (!value->value.integer.value[0]) + reg &= ~PCM1796_DME; + else + reg |= PCM1796_DME; + changed = reg != data->pcm1796_regs[0][18 - PCM1796_REG_BASE]; + if (changed) { + for (i = 0; i < data->dacs; ++i) + pcm1796_write(chip, i, 18, reg); + } + mutex_unlock(&chip->mutex); + return changed; +} + +static const struct snd_kcontrol_new deemph_control = { + .iface = SNDRV_CTL_ELEM_IFACE_MIXER, + .name = "De-emphasis Playback Switch", + .info = snd_ctl_boolean_mono_info, + .get = deemph_get, + .put = deemph_put, +}; + static const struct snd_kcontrol_new hdav_hdmi_control = { .iface = SNDRV_CTL_ELEM_IFACE_MIXER, .name = "HDMI Playback Switch", @@ -1011,6 +1075,10 @@ static int add_pcm1796_controls(struct oxygen *chip) snd_ctl_new1(&rolloff_control, chip)); if (err < 0) return err; + err = snd_ctl_add(chip->card, + snd_ctl_new1(&deemph_control, chip)); + if (err < 0) + return err; } return 0; }
tom.ty89@gmail.com wrote:
(data->pcm1796_regs[0][18 - PCM1796_REG_BASE] &PCM1796_DME) != PCM1796_DMF_DISABLED;
DME and DMF are different fields. Just use !!.
With this fixed: Acked-by: Clemens Ladisch clemens@ladisch.de
Regards, Clemens
Considering with my patch this is the only remaining usage of it, do you think I should just change it into "PCM1796_DME_DISABLED" and keep this manner (so that it is consistent with rolloff_get() / PCM1796_FLT_SHARP)? Or do you insist that I should use !! anyway?
On Thu, 20 Dec 2018 at 02:00, Clemens Ladisch clemens@ladisch.de wrote:
tom.ty89@gmail.com wrote:
(data->pcm1796_regs[0][18 - PCM1796_REG_BASE] &PCM1796_DME) != PCM1796_DMF_DISABLED;DME and DMF are different fields. Just use !!.
With this fixed: Acked-by: Clemens Ladisch clemens@ladisch.de
Regards, Clemens
Tom Yan wrote:
Considering with my patch this is the only remaining usage of it, do you think I should just change it into "PCM1796_DME_DISABLED" and keep this manner (so that it is consistent with rolloff_get() / PCM1796_FLT_SHARP)? Or do you insist that I should use !! anyway?
There is no "PCM1796_DME_DISABLED" symbol. IMO this is not necessary for register bits with a boolean meaning.
You could add it if you wanted to, but I do not think that consistency would be sufficient reason for that.
Regards, Clemens
From: Tom Yan tom.ty89@gmail.com
Add control for the de-emphasis filter in the PCM179x DACs
Signed-off-by: Tom Yan tom.ty89@gmail.com
diff --git a/sound/pci/oxygen/pcm1796.h b/sound/pci/oxygen/pcm1796.h index 34d07dd2d22e..d5dcb09e44cd 100644 --- a/sound/pci/oxygen/pcm1796.h +++ b/sound/pci/oxygen/pcm1796.h @@ -10,7 +10,6 @@ #define PCM1796_MUTE 0x01 #define PCM1796_DME 0x02 #define PCM1796_DMF_MASK 0x0c -#define PCM1796_DMF_DISABLED 0x00 #define PCM1796_DMF_48 0x04 #define PCM1796_DMF_441 0x08 #define PCM1796_DMF_32 0x0c diff --git a/sound/pci/oxygen/xonar_pcm179x.c b/sound/pci/oxygen/xonar_pcm179x.c index 24109d37ca09..a1c6b98b191e 100644 --- a/sound/pci/oxygen/xonar_pcm179x.c +++ b/sound/pci/oxygen/xonar_pcm179x.c @@ -331,7 +331,7 @@ static void pcm1796_init(struct oxygen *chip) struct xonar_pcm179x *data = chip->model_data;
data->pcm1796_regs[0][18 - PCM1796_REG_BASE] = - PCM1796_DMF_DISABLED | PCM1796_FMT_24_I2S | PCM1796_ATLD; + PCM1796_FMT_24_I2S | PCM1796_ATLD; if (!data->broken_i2c) data->pcm1796_regs[0][18 - PCM1796_REG_BASE] |= PCM1796_MUTE; data->pcm1796_regs[0][19 - PCM1796_REG_BASE] = @@ -621,6 +621,23 @@ static void update_pcm1796_oversampling(struct oxygen *chip) pcm1796_write_cached(chip, i, 20, reg); }
+static void update_pcm1796_deemph(struct oxygen *chip) +{ + struct xonar_pcm179x *data = chip->model_data; + unsigned int i; + u8 reg; + + reg = data->pcm1796_regs[0][18 - PCM1796_REG_BASE] & ~PCM1796_DMF_MASK; + if (data->current_rate == 48000) + reg |= PCM1796_DMF_48; + else if (data->current_rate == 44100) + reg |= PCM1796_DMF_441; + else if (data->current_rate == 32000) + reg |= PCM1796_DMF_32; + for (i = 0; i < data->dacs; ++i) + pcm1796_write_cached(chip, i, 18, reg); +} + static void set_pcm1796_params(struct oxygen *chip, struct snd_pcm_hw_params *params) { @@ -629,6 +646,7 @@ static void set_pcm1796_params(struct oxygen *chip, msleep(1); data->current_rate = params_rate(params); update_pcm1796_oversampling(chip); + update_pcm1796_deemph(chip); }
static void update_pcm1796_volume(struct oxygen *chip) @@ -653,9 +671,11 @@ static void update_pcm1796_mute(struct oxygen *chip) unsigned int i; u8 value;
- value = PCM1796_DMF_DISABLED | PCM1796_FMT_24_I2S | PCM1796_ATLD; + value = data->pcm1796_regs[0][18 - PCM1796_REG_BASE]; if (chip->dac_mute) value |= PCM1796_MUTE; + else + value &= ~PCM1796_MUTE; for (i = 0; i < data->dacs; ++i) pcm1796_write_cached(chip, i, 18, value); } @@ -777,6 +797,49 @@ static const struct snd_kcontrol_new rolloff_control = { .put = rolloff_put, };
+static int deemph_get(struct snd_kcontrol *ctl, + struct snd_ctl_elem_value *value) +{ + struct oxygen *chip = ctl->private_data; + struct xonar_pcm179x *data = chip->model_data; + + value->value.integer.value[0] = + !!(data->pcm1796_regs[0][18 - PCM1796_REG_BASE] & PCM1796_DME); + return 0; +} + +static int deemph_put(struct snd_kcontrol *ctl, + struct snd_ctl_elem_value *value) +{ + struct oxygen *chip = ctl->private_data; + struct xonar_pcm179x *data = chip->model_data; + unsigned int i; + int changed; + u8 reg; + + mutex_lock(&chip->mutex); + reg = data->pcm1796_regs[0][18 - PCM1796_REG_BASE]; + if (!value->value.integer.value[0]) + reg &= ~PCM1796_DME; + else + reg |= PCM1796_DME; + changed = reg != data->pcm1796_regs[0][18 - PCM1796_REG_BASE]; + if (changed) { + for (i = 0; i < data->dacs; ++i) + pcm1796_write(chip, i, 18, reg); + } + mutex_unlock(&chip->mutex); + return changed; +} + +static const struct snd_kcontrol_new deemph_control = { + .iface = SNDRV_CTL_ELEM_IFACE_MIXER, + .name = "De-emphasis Playback Switch", + .info = snd_ctl_boolean_mono_info, + .get = deemph_get, + .put = deemph_put, +}; + static const struct snd_kcontrol_new hdav_hdmi_control = { .iface = SNDRV_CTL_ELEM_IFACE_MIXER, .name = "HDMI Playback Switch", @@ -1011,6 +1074,10 @@ static int add_pcm1796_controls(struct oxygen *chip) snd_ctl_new1(&rolloff_control, chip)); if (err < 0) return err; + err = snd_ctl_add(chip->card, + snd_ctl_new1(&deemph_control, chip)); + if (err < 0) + return err; } return 0; }
On Thu, 20 Dec 2018 00:45:51 +0100, tom.ty89@gmail.com wrote:
From: Tom Yan tom.ty89@gmail.com
Add control for the de-emphasis filter in the PCM179x DACs
Signed-off-by: Tom Yan tom.ty89@gmail.com
Applied now for 5.1. Thanks.
Takashi
participants (4)
-
Clemens Ladisch -
Takashi Iwai -
Tom Yan -
tom.ty89＠gmail.com