[alsa-devel] PATCH - vt1618 7.1 Audio
Signed-off-by: John L. Utz III john.utz@dmx.com
diff --git a/pci/ac97/ac97_codec.c b/pci/ac97/ac97_codec.c index d0023e9..a34f1ea 100644 --- a/pci/ac97/ac97_codec.c +++ b/pci/ac97/ac97_codec.c @@ -168,7 +168,7 @@ static const struct ac97_codec_id snd_ac97_codec_ids[] = { { 0x54584e20, 0xffffffff, "TLC320AD9xC", NULL, NULL }, { 0x56494161, 0xffffffff, "VIA1612A", NULL, NULL }, // modified ICE1232 with S/PDIF { 0x56494170, 0xffffffff, "VIA1617A", patch_vt1617a, NULL }, // modified VT1616 with S/PDIF -{ 0x56494182, 0xffffffff, "VIA1618", NULL, NULL }, +{ 0x56494182, 0xffffffff, "VIA1618", patch_vt1618, NULL }, // clean sheet of crinkled paper { 0x57454301, 0xffffffff, "W83971D", NULL, NULL }, { 0x574d4c00, 0xffffffff, "WM9701,WM9701A", NULL, NULL }, { 0x574d4C03, 0xffffffff, "WM9703,WM9707,WM9708,WM9717", patch_wolfson03, NULL}, @@ -609,7 +609,6 @@ AC97_SINGLE("PC Speaker Playback Volume", AC97_PC_BEEP, 1, 15, 1) static const struct snd_kcontrol_new snd_ac97_controls_mic_boost = AC97_SINGLE("Mic Boost (+20dB)", AC97_MIC, 6, 1, 0);
- static const char* std_rec_sel[] = {"Mic", "CD", "Video", "Aux", "Line", "Mix", "Mix Mono", "Phone"}; static const char* std_3d_path[] = {"pre 3D", "post 3D"}; static const char* std_mix[] = {"Mix", "Mic"}; diff --git a/pci/ac97/ac97_patch.c b/pci/ac97/ac97_patch.c index bb028f8..2042415 100644 --- a/pci/ac97/ac97_patch.c +++ b/pci/ac97/ac97_patch.c @@ -3465,7 +3465,7 @@ static int patch_vt1616(struct snd_ac97 * ac97)
/* * unfortunately, the vt1617a stashes the twiddlers required for - * nooding the i/o jacks on 2 different regs. * thameans that we cant + * nooding the i/o jacks on 2 different regs. that means that we cant * use the easy way provided by AC97_ENUM_DOUBLE() we have to write * are own funcs. * @@ -3576,6 +3576,413 @@ int patch_vt1617a(struct snd_ac97 * ac97) return err; }
+ +/* use these alot in the 1618 code but i cant find a better place to put them */ + +static const char* std_enable[] = {"Enabled", "Disabled"}; +static const char* std_disable[] = {"Disabled","Enabled"}; + +/* disable enable c/lfe exchange */ + +static int snd_ac97_vt1618_clex_info(struct snd_kcontrol *kcontrol, + struct snd_ctl_elem_info *uinfo) +{ + return ac97_enum_text_info(kcontrol, uinfo, std_disable, 2); +} + +static int snd_ac97_vt1618_clex_get(struct snd_kcontrol *kcontrol, + struct snd_ctl_elem_value *ucontrol) +{ + struct snd_ac97 *pac97 = snd_kcontrol_chip(kcontrol); + + ucontrol->value.enumerated.item[0] = (snd_ac97_read(pac97, 0x5a) & + 0x0100) >> 8; + return 0; +} + +static int snd_ac97_vt1618_clex_put(struct snd_kcontrol *kcontrol, + struct snd_ctl_elem_value *ucontrol) +{ + struct snd_ac97 *pac97 = snd_kcontrol_chip(kcontrol); + + /* notice relationship between mask and shift! */ + + return snd_ac97_update_bits(pac97, 0x5a, 0x0100, + ucontrol->value.enumerated.item[0] << 8); +} + + +/* disable enable dc offset */ + +static int snd_ac97_vt1618_dcof_info(struct snd_kcontrol *kcontrol, + struct snd_ctl_elem_info *uinfo) +{ + return ac97_enum_text_info(kcontrol, uinfo, std_disable, 2); +} + +static int snd_ac97_vt1618_dcof_get(struct snd_kcontrol *kcontrol, + struct snd_ctl_elem_value *ucontrol) +{ + struct snd_ac97 *pac97 = snd_kcontrol_chip(kcontrol); + + ucontrol->value.enumerated.item[0] = (snd_ac97_read(pac97, 0x5a) & + 0x0400) >> 10; + return 0; +} + +static int snd_ac97_vt1618_dcof_put(struct snd_kcontrol *kcontrol, + struct snd_ctl_elem_value *ucontrol) +{ + struct snd_ac97 *pac97 = snd_kcontrol_chip(kcontrol); + + /* notice relationship between mask and shift! */ + + return snd_ac97_update_bits(pac97, 0x5a, 0x0400, + ucontrol->value.enumerated.item[0] << 10); +} + + +/* enable disable headphone amp */ + +static int snd_ac97_vt1618_hamp_info(struct snd_kcontrol *kcontrol, + struct snd_ctl_elem_info *uinfo) +{ + return ac97_enum_text_info(kcontrol, uinfo, std_enable, 2); +} + +static int snd_ac97_vt1618_hamp_get(struct snd_kcontrol *kcontrol, + struct snd_ctl_elem_value *ucontrol) +{ + struct snd_ac97 *pac97 = snd_kcontrol_chip(kcontrol); + + ucontrol->value.enumerated.item[0] = (snd_ac97_read(pac97, 0x5c) & + 0x0020) >> 5; + return 0; +} + +static int snd_ac97_vt1618_hamp_put(struct snd_kcontrol *kcontrol, + struct snd_ctl_elem_value *ucontrol) +{ + struct snd_ac97 *pac97 = snd_kcontrol_chip(kcontrol); + + /* notice relationship between mask and shift! */ + + return snd_ac97_update_bits(pac97, 0x5c, 0x0020, + ucontrol->value.enumerated.item[0] << 5); +} + + +/* enable disable surround back channel */ + +static int snd_ac97_vt1618_srbk_info(struct snd_kcontrol *kcontrol, + struct snd_ctl_elem_info *uinfo) +{ + return ac97_enum_text_info(kcontrol, uinfo, std_enable, 2); +} + +static int snd_ac97_vt1618_srbk_get(struct snd_kcontrol *kcontrol, + struct snd_ctl_elem_value *ucontrol) +{ + struct snd_ac97 *pac97 = snd_kcontrol_chip(kcontrol); + + ucontrol->value.enumerated.item[0] = (snd_ac97_read(pac97, 0x5c) & + 0x0008) >> 3; + return 0; +} + +static int snd_ac97_vt1618_srbk_put(struct snd_kcontrol *kcontrol, + struct snd_ctl_elem_value *ucontrol) +{ + struct snd_ac97 *pac97 = snd_kcontrol_chip(kcontrol); + + /* notice relationship between mask and shift! */ + + return snd_ac97_update_bits(pac97, 0x5c, 0x0008, + ucontrol->value.enumerated.item[0] << 3); +} + + +/* enable disable soft mute */ + +static int snd_ac97_vt1618_smute_info(struct snd_kcontrol *kcontrol, + struct snd_ctl_elem_info *uinfo) +{ + return ac97_enum_text_info(kcontrol, uinfo, std_enable, 2); +} + +static int snd_ac97_vt1618_smute_get(struct snd_kcontrol *kcontrol, + struct snd_ctl_elem_value *ucontrol) +{ + struct snd_ac97 *pac97 = snd_kcontrol_chip(kcontrol); + + ucontrol->value.enumerated.item[0] = (snd_ac97_read(pac97, 0x5c) & + 0x0001); + return 0; +} + +static int snd_ac97_vt1618_smute_put(struct snd_kcontrol *kcontrol, + struct snd_ctl_elem_value *ucontrol) +{ + struct snd_ac97 *pac97 = snd_kcontrol_chip(kcontrol); + + /* notice relationship between mask and shift! */ + + return snd_ac97_update_bits(pac97, 0x5c, 0x0001, + ucontrol->value.enumerated.item[0]); +} + + +/* config aux in jack - not found on 3 jack motherboards or soundcards */ + +static int snd_ac97_vt1618_auxin_info(struct snd_kcontrol *kcontrol, + struct snd_ctl_elem_info *uinfo) +{ + static const char* texts[] = {"Aux In", "Surround Back Out"}; + return ac97_enum_text_info(kcontrol, uinfo, texts, 2); +} + +static int snd_ac97_vt1618_auxin_get(struct snd_kcontrol *kcontrol, + struct snd_ctl_elem_value *ucontrol) +{ + struct snd_ac97 *pac97 = snd_kcontrol_chip(kcontrol); + + ucontrol->value.enumerated.item[0] = (snd_ac97_read(pac97, 0x76) & + 0x0008) >> 3; + + return 0; +} + +static int snd_ac97_vt1618_auxin_put(struct snd_kcontrol *kcontrol, + struct snd_ctl_elem_value *ucontrol) +{ + struct snd_ac97 *pac97 = snd_kcontrol_chip(kcontrol); + + /* notice relationship between mask and shift! */ + + return snd_ac97_update_bits(pac97, 0x76, 0x0008, + ucontrol->value.enumerated.item[0] << 3); +} + + +/* + * VIA implements 'Smart 5.1' completely differently on the 1618 than + * it does on the 1617a. awesome! They seem to have sourced this + * particular revision of the technology from somebody else, it's + * called Universal Audio Jack, it shows up on some other folk's chips + * as well. + * + * ordering in this list reflects vt1618 docs for Reg 60h and + * the block diagram, DACs are as follows: + * + * OUT_O -> Front, + * OUT_1 -> Surround, + * OUT_2 -> C/LFE + * + * Unlike the 1617a, each OUT has a consistent set of mappings + * for all bitpatterns other than 00: + * + * 01 Unmixed Output + * 10 Line In + * 11 Mic In + * + * Special Case of 00: + * + * OUT_0 Mixed Output + * OUT_1 Reserved + * OUT_2 Reserved + * + * I have no idea what the hell Reserved does, but on an MSI + * CN700T, i have to set it to get surround output - YMMV, bad + * shit may happen. + * + * If other chips use Universal Audio Jack, then this code might be + * applicable to them. + */ + +/* copied from ac97_surround_jack_mode_info() ordering in this list + * reflects vt1618 docs for Vendor Defined Register 0x60 */ + +static int snd_ac97_vt1618_UAJ0_info(struct snd_kcontrol *kcontrol, + struct snd_ctl_elem_info *uinfo) +{ + static const char* texts[] = { "Speaker Out", "DAC Unmixed Out", "Line In", "Mic In"}; + return ac97_enum_text_info(kcontrol, uinfo, texts, 4); +} + +/* UAJ1 and UAJ2 are not supposed to have 00 written to them?? i + * dunno, because thats something that i have to do to get 5.1 out to + * work. */ + +static int snd_ac97_vt1618_UAJ1_info(struct snd_kcontrol *kcontrol, + struct snd_ctl_elem_info *uinfo) +{ + static const char* texts[] = { "Surround Out", "DAC Unmixed Out", "Line In", "Mic In"}; + return ac97_enum_text_info(kcontrol, uinfo, texts, 4); +} + +static int snd_ac97_vt1618_UAJ2_info(struct snd_kcontrol *kcontrol, struct snd_ctl_elem_info *uinfo) +{ + static const char* texts[] = { "Center/LowFre Out", "DAC Unmixed Out", "Line In", "Mic In"}; + return ac97_enum_text_info(kcontrol, uinfo, texts, 4); +} + +/* All of the vt1618 Universal Audio Jack twiddlers are on + Vendor Defined Register 0x60, page 0. The bits, and thus + the mask, are the only thing that changes */ + +static int snd_ac97_vt1618_UAJ_get(struct snd_kcontrol *kcontrol, + struct snd_ctl_elem_value *ucontrol, unsigned short mask) +{ + struct snd_ac97 *pac97; + ushort usDatPag, usUAJ, usShift = 0; // 0 is a possible value + + if(0x000C == mask) usShift = 2; + if(0x0030 == mask) usShift = 4; + + pac97 = snd_kcontrol_chip(kcontrol); + + mutex_lock(&pac97->page_mutex); + + usDatPag = snd_ac97_read(pac97, AC97_INT_PAGING) & AC97_PAGE_MASK; + snd_ac97_update_bits(pac97, AC97_INT_PAGING, AC97_PAGE_MASK, 0); + + usUAJ = snd_ac97_read(pac97, 0x60) & mask; + + snd_ac97_update_bits(pac97, AC97_INT_PAGING, AC97_PAGE_MASK, usDatPag); + mutex_unlock(&pac97->page_mutex); // unlock paging + + ucontrol->value.enumerated.item[0] = usUAJ >> usShift; + + return 0; +} + +static int snd_ac97_vt1618_UAJ_put(struct snd_kcontrol *kcontrol, + struct snd_ctl_elem_value *ucontrol, + unsigned short mask) +{ + struct snd_ac97 *pac97; + ushort usUAJ, usShift = 0; /* 0 is a possible value */ + + if(0x000C == mask) usShift = 2; + if(0x0030 == mask) usShift = 4; + + pac97 = snd_kcontrol_chip(kcontrol); + + usUAJ = ucontrol->value.enumerated.item[0] << usShift; + + return ac97_update_bits_page(pac97, 0x60, mask, usUAJ, 0); +} + +static int snd_ac97_vt1618_UAJ0_get(struct snd_kcontrol *kcontrol, + struct snd_ctl_elem_value *ucontrol) +{ + return snd_ac97_vt1618_UAJ_get(kcontrol, ucontrol, 0x0003); +} + +static int snd_ac97_vt1618_UAJ1_get(struct snd_kcontrol *kcontrol, + struct snd_ctl_elem_value *ucontrol) +{ + return snd_ac97_vt1618_UAJ_get(kcontrol, ucontrol, 0x000C); +} + +static int snd_ac97_vt1618_UAJ2_get(struct snd_kcontrol *kcontrol, + struct snd_ctl_elem_value *ucontrol) +{ + return snd_ac97_vt1618_UAJ_get(kcontrol, ucontrol, 0x0030); +} + +static int snd_ac97_vt1618_UAJ0_put(struct snd_kcontrol *kcontrol, + struct snd_ctl_elem_value *ucontrol) +{ + return snd_ac97_vt1618_UAJ_put(kcontrol, ucontrol, 0x0003); +} + +static int snd_ac97_vt1618_UAJ1_put(struct snd_kcontrol *kcontrol, + struct snd_ctl_elem_value *ucontrol) +{ + return snd_ac97_vt1618_UAJ_put(kcontrol, ucontrol, 0x000C); +} + +static int snd_ac97_vt1618_UAJ2_put(struct snd_kcontrol *kcontrol, + struct snd_ctl_elem_value *ucontrol) +{ + return snd_ac97_vt1618_UAJ_put(kcontrol, ucontrol, 0x0030); +} + +static const struct snd_kcontrol_new snd_ac97_controls_vt1618[] = { + + { + .iface = SNDRV_CTL_ELEM_IFACE_MIXER, + .name = "Center LFE Exchange", + .info = snd_ac97_vt1618_clex_info, + .get = snd_ac97_vt1618_clex_get, + .put = snd_ac97_vt1618_clex_put + }, + { + .iface = SNDRV_CTL_ELEM_IFACE_MIXER, + .name = "DC Offset", + .info = snd_ac97_vt1618_dcof_info, + .get = snd_ac97_vt1618_dcof_get, + .put = snd_ac97_vt1618_dcof_put + },diff --git a/pci/ac97/ac97_codec.c b/pci/ac97/ac97_codec.c index d0023e9..a34f1ea 100644 --- a/pci/ac97/ac97_codec.c +++ b/pci/ac97/ac97_codec.c @@ -168,7 +168,7 @@ static const struct ac97_codec_id snd_ac97_codec_ids[] = { { 0x54584e20, 0xffffffff, "TLC320AD9xC", NULL, NULL }, { 0x56494161, 0xffffffff, "VIA1612A", NULL, NULL }, // modified ICE1232 with S/PDIF { 0x56494170, 0xffffffff, "VIA1617A", patch_vt1617a, NULL }, // modified VT1616 with S/PDIF -{ 0x56494182, 0xffffffff, "VIA1618", NULL, NULL }, +{ 0x56494182, 0xffffffff, "VIA1618", patch_vt1618, NULL }, // clean sheet of crinkled paper { 0x57454301, 0xffffffff, "W83971D", NULL, NULL }, { 0x574d4c00, 0xffffffff, "WM9701,WM9701A", NULL, NULL }, { 0x574d4C03, 0xffffffff, "WM9703,WM9707,WM9708,WM9717", patch_wolfson03, NULL}, @@ -609,7 +609,6 @@ AC97_SINGLE("PC Speaker Playback Volume", AC97_PC_BEEP, 1, 15, 1) static const struct snd_kcontrol_new snd_ac97_controls_mic_boost = AC97_SINGLE("Mic Boost (+20dB)", AC97_MIC, 6, 1, 0);
- static const char* std_rec_sel[] = {"Mic", "CD", "Video", "Aux", "Line", "Mix", "Mix Mono", "Phone"}; static const char* std_3d_path[] = {"pre 3D", "post 3D"}; static const char* std_mix[] = {"Mix", "Mic"}; diff --git a/pci/ac97/ac97_patch.c b/pci/ac97/ac97_patch.c index bb028f8..2042415 100644 --- a/pci/ac97/ac97_patch.c +++ b/pci/ac97/ac97_patch.c @@ -3465,7 +3465,7 @@ static int patch_vt1616(struct snd_ac97 * ac97)
/* * unfortunately, the vt1617a stashes the twiddlers required for - * nooding the i/o jacks on 2 different regs. * thameans that we cant + * nooding the i/o jacks on 2 different regs. that means that we cant * use the easy way provided by AC97_ENUM_DOUBLE() we have to write * are own funcs. * @@ -3576,6 +3576,413 @@ int patch_vt1617a(struct snd_ac97 * ac97) return err; }
+ +/* use these alot in the 1618 code but i cant find a better place to put them */ + +static const char* std_enable[] = {"Enabled", "Disabled"}; +static const char* std_disable[] = {"Disabled","Enabled"}; + +/* disable enable c/lfe exchange */ + +static int snd_ac97_vt1618_clex_info(struct snd_kcontrol *kcontrol, + struct snd_ctl_elem_info *uinfo) +{ + return ac97_enum_text_info(kcontrol, uinfo, std_disable, 2); +} + +static int snd_ac97_vt1618_clex_get(struct snd_kcontrol *kcontrol, + struct snd_ctl_elem_value *ucontrol) +{ + struct snd_ac97 *pac97 = snd_kcontrol_chip(kcontrol); + + ucontrol->value.enumerated.item[0] = (snd_ac97_read(pac97, 0x5a) & + 0x0100) >> 8; + return 0; +} + +static int snd_ac97_vt1618_clex_put(struct snd_kcontrol *kcontrol, + struct snd_ctl_elem_value *ucontrol) +{ + struct snd_ac97 *pac97 = snd_kcontrol_chip(kcontrol); + + /* notice relationship between mask and shift! */ + + return snd_ac97_update_bits(pac97, 0x5a, 0x0100, + ucontrol->value.enumerated.item[0] << 8); +} + + +/* disable enable dc offset */ + +static int snd_ac97_vt1618_dcof_info(struct snd_kcontrol *kcontrol, + struct snd_ctl_elem_info *uinfo) +{ + return ac97_enum_text_info(kcontrol, uinfo, std_disable, 2); +} + +static int snd_ac97_vt1618_dcof_get(struct snd_kcontrol *kcontrol, + struct snd_ctl_elem_value *ucontrol) +{ + struct snd_ac97 *pac97 = snd_kcontrol_chip(kcontrol); + + ucontrol->value.enumerated.item[0] = (snd_ac97_read(pac97, 0x5a) & + 0x0400) >> 10; + return 0; +} + +static int snd_ac97_vt1618_dcof_put(struct snd_kcontrol *kcontrol, + struct snd_ctl_elem_value *ucontrol) +{ + struct snd_ac97 *pac97 = snd_kcontrol_chip(kcontrol); + + /* notice relationship between mask and shift! */ + + return snd_ac97_update_bits(pac97, 0x5a, 0x0400, + ucontrol->value.enumerated.item[0] << 10); +} + + +/* enable disable headphone amp */ + +static int snd_ac97_vt1618_hamp_info(struct snd_kcontrol *kcontrol, + struct snd_ctl_elem_info *uinfo) +{ + return ac97_enum_text_info(kcontrol, uinfo, std_enable, 2); +} + +static int snd_ac97_vt1618_hamp_get(struct snd_kcontrol *kcontrol, + struct snd_ctl_elem_value *ucontrol) +{ + struct snd_ac97 *pac97 = snd_kcontrol_chip(kcontrol); + + ucontrol->value.enumerated.item[0] = (snd_ac97_read(pac97, 0x5c) & + 0x0020) >> 5; + return 0; +} + +static int snd_ac97_vt1618_hamp_put(struct snd_kcontrol *kcontrol, + struct snd_ctl_elem_value *ucontrol) +{ + struct snd_ac97 *pac97 = snd_kcontrol_chip(kcontrol); + + /* notice relationship between mask and shift! */ + + return snd_ac97_update_bits(pac97, 0x5c, 0x0020, + ucontrol->value.enumerated.item[0] << 5); +} + + +/* enable disable surround back channel */ + +static int snd_ac97_vt1618_srbk_info(struct snd_kcontrol *kcontrol, + struct snd_ctl_elem_info *uinfo) +{ + return ac97_enum_text_info(kcontrol, uinfo, std_enable, 2); +} + +static int snd_ac97_vt1618_srbk_get(struct snd_kcontrol *kcontrol, + struct snd_ctl_elem_value *ucontrol) +{ + struct snd_ac97 *pac97 = snd_kcontrol_chip(kcontrol); + + ucontrol->value.enumerated.item[0] = (snd_ac97_read(pac97, 0x5c) & + 0x0008) >> 3; + return 0; +} + +static int snd_ac97_vt1618_srbk_put(struct snd_kcontrol *kcontrol, + struct snd_ctl_elem_value *ucontrol) +{ + struct snd_ac97 *pac97 = snd_kcontrol_chip(kcontrol); + + /* notice relationship between mask and shift! */ + + return snd_ac97_update_bits(pac97, 0x5c, 0x0008, + ucontrol->value.enumerated.item[0] << 3); +} + + +/* enable disable soft mute */ + +static int snd_ac97_vt1618_smute_info(struct snd_kcontrol *kcontrol, + struct snd_ctl_elem_info *uinfo) +{ + return ac97_enum_text_info(kcontrol, uinfo, std_enable, 2); +} + +static int snd_ac97_vt1618_smute_get(struct snd_kcontrol *kcontrol, + struct snd_ctl_elem_value *ucontrol) +{ + struct snd_ac97 *pac97 = snd_kcontrol_chip(kcontrol); + + ucontrol->value.enumerated.item[0] = (snd_ac97_read(pac97, 0x5c) & + 0x0001); + return 0; +} + +static int snd_ac97_vt1618_smute_put(struct snd_kcontrol *kcontrol, + struct snd_ctl_elem_value *ucontrol) +{ + struct snd_ac97 *pac97 = snd_kcontrol_chip(kcontrol); + + /* notice relationship between mask and shift! */ + + return snd_ac97_update_bits(pac97, 0x5c, 0x0001, + ucontrol->value.enumerated.item[0]); +} + + +/* config aux in jack - not found on 3 jack motherboards or soundcards */ + +static int snd_ac97_vt1618_auxin_info(struct snd_kcontrol *kcontrol, + struct snd_ctl_elem_info *uinfo) +{ + static const char* texts[] = {"Aux In", "Surround Back Out"}; + return ac97_enum_text_info(kcontrol, uinfo, texts, 2); +} + +static int snd_ac97_vt1618_auxin_get(struct snd_kcontrol *kcontrol, + struct snd_ctl_elem_value *ucontrol) +{ + struct snd_ac97 *pac97 = snd_kcontrol_chip(kcontrol); + + ucontrol->value.enumerated.item[0] = (snd_ac97_read(pac97, 0x76) & + 0x0008) >> 3; + + return 0; +} + +static int snd_ac97_vt1618_auxin_put(struct snd_kcontrol *kcontrol, + struct snd_ctl_elem_value *ucontrol) +{ + struct snd_ac97 *pac97 = snd_kcontrol_chip(kcontrol); + + /* notice relationship between mask and shift! */ + + return snd_ac97_update_bits(pac97, 0x76, 0x0008, + ucontrol->value.enumerated.item[0] << 3); +} + + +/* + * VIA implements 'Smart 5.1' completely differently on the 1618 than + * it does on the 1617a. awesome! They seem to have sourced this + * particular revision of the technology from somebody else, it's + * called Universal Audio Jack, it shows up on some other folk's chips + * as well. + * + * ordering in this list reflects vt1618 docs for Reg 60h and + * the block diagram, DACs are as follows: + * + * OUT_O -> Front, + * OUT_1 -> Surround, + * OUT_2 -> C/LFE + * + * Unlike the 1617a, each OUT has a consistent set of mappings + * for all bitpatterns other than 00: + * + * 01 Unmixed Output + * 10 Line In + * 11 Mic In + * + * Special Case of 00: + * + * OUT_0 Mixed Output + * OUT_1 Reserved + * OUT_2 Reserved + * + * I have no idea what the hell Reserved does, but on an MSI + * CN700T, i have to set it to get surround output - YMMV, bad + * shit may happen. + * + * If other chips use Universal Audio Jack, then this code might be + * applicable to them. + */ + +/* copied from ac97_surround_jack_mode_info() ordering in this list + * reflects vt1618 docs for Vendor Defined Register 0x60 */ + +static int snd_ac97_vt1618_UAJ0_info(struct snd_kcontrol *kcontrol, + struct snd_ctl_elem_info *uinfo) +{ + static const char* texts[] = { "Speaker Out", "DAC Unmixed Out", "Line In", "Mic In"}; + return ac97_enum_text_info(kcontrol, uinfo, texts, 4); +} + +/* UAJ1 and UAJ2 are not supposed to have 00 written to them?? i + * dunno, because thats something that i have to do to get 5.1 out to + * work. */ + +static int snd_ac97_vt1618_UAJ1_info(struct snd_kcontrol *kcontrol, + struct snd_ctl_elem_info *uinfo) +{ + static const char* texts[] = { "Surround Out", "DAC Unmixed Out", "Line In", "Mic In"}; + return ac97_enum_text_info(kcontrol, uinfo, texts, 4); +} + +static int snd_ac97_vt1618_UAJ2_info(struct snd_kcontrol *kcontrol, struct snd_ctl_elem_info *uinfo) +{ + static const char* texts[] = { "Center/LowFre Out", "DAC Unmixed Out", "Line In", "Mic In"}; + return ac97_enum_text_info(kcontrol, uinfo, texts, 4); +} + +/* All of the vt1618 Universal Audio Jack twiddlers are on + Vendor Defined Register 0x60, page 0. The bits, and thus + the mask, are the only thing that changes */ + +static int snd_ac97_vt1618_UAJ_get(struct snd_kcontrol *kcontrol, + struct snd_ctl_elem_value *ucontrol, unsigned short mask) +{ + struct snd_ac97 *pac97; + ushort usDatPag, usUAJ, usShift = 0; // 0 is a possible value + + if(0x000C == mask) usShift = 2; + if(0x0030 == mask) usShift = 4; + + pac97 = snd_kcontrol_chip(kcontrol); + + mutex_lock(&pac97->page_mutex); + + usDatPag = snd_ac97_read(pac97, AC97_INT_PAGING) & AC97_PAGE_MASK; + snd_ac97_update_bits(pac97, AC97_INT_PAGING, AC97_PAGE_MASK, 0); + + usUAJ = snd_ac97_read(pac97, 0x60) & mask; + + snd_ac97_update_bits(pac97, AC97_INT_PAGING, AC97_PAGE_MASK, usDatPag); + mutex_unlock(&pac97->page_mutex); // unlock paging + + ucontrol->value.enumerated.item[0] = usUAJ >> usShift; + + return 0; +} + +static int snd_ac97_vt1618_UAJ_put(struct snd_kcontrol *kcontrol, + struct snd_ctl_elem_value *ucontrol, + unsigned short mask) +{ + struct snd_ac97 *pac97; + ushort usUAJ, usShift = 0; /* 0 is a possible value */ + + if(0x000C == mask) usShift = 2; + if(0x0030 == mask) usShift = 4; + + pac97 = snd_kcontrol_chip(kcontrol); + + usUAJ = ucontrol->value.enumerated.item[0] << usShift; + + return ac97_update_bits_page(pac97, 0x60, mask, usUAJ, 0); +} + +static int snd_ac97_vt1618_UAJ0_get(struct snd_kcontrol *kcontrol, + struct snd_ctl_elem_value *ucontrol) +{ + return snd_ac97_vt1618_UAJ_get(kcontrol, ucontrol, 0x0003); +} + +static int snd_ac97_vt1618_UAJ1_get(struct snd_kcontrol *kcontrol, + struct snd_ctl_elem_value *ucontrol) +{ + return snd_ac97_vt1618_UAJ_get(kcontrol, ucontrol, 0x000C); +} + +static int snd_ac97_vt1618_UAJ2_get(struct snd_kcontrol *kcontrol, + struct snd_ctl_elem_value *ucontrol) +{ + return snd_ac97_vt1618_UAJ_get(kcontrol, ucontrol, 0x0030); +} + +static int snd_ac97_vt1618_UAJ0_put(struct snd_kcontrol *kcontrol, + struct snd_ctl_elem_value *ucontrol) +{ + return snd_ac97_vt1618_UAJ_put(kcontrol, ucontrol, 0x0003); +} + +static int snd_ac97_vt1618_UAJ1_put(struct snd_kcontrol *kcontrol, + struct snd_ctl_elem_value *ucontrol) +{ + return snd_ac97_vt1618_UAJ_put(kcontrol, ucontrol, 0x000C); +} + +static int snd_ac97_vt1618_UAJ2_put(struct snd_kcontrol *kcontrol, + struct snd_ctl_elem_value *ucontrol) +{ + return snd_ac97_vt1618_UAJ_put(kcontrol, ucontrol, 0x0030); +} + +static const struct snd_kcontrol_new snd_ac97_controls_vt1618[] = { + + { + .iface = SNDRV_CTL_ELEM_IFACE_MIXER, + .name = "Center LFE Exchange", + .info = snd_ac97_vt1618_clex_info, + .get = snd_ac97_vt1618_clex_get, + .put = snd_ac97_vt1618_clex_put + }, + { + .iface = SNDRV_CTL_ELEM_IFACE_MIXER, + .name = "DC Offset", + .info = snd_ac97_vt1618_dcof_info, + .get = snd_ac97_vt1618_dcof_get, + .put = snd_ac97_vt1618_dcof_diff --git a/pci/ac97/ac97_codec.c b/pci/ac97/ac97_codec.c index d0023e9..a34f1ea 100644 --- a/pci/ac97/ac97_codec.c +++ b/pci/ac97/ac97_codec.c @@ -168,7 +168,7 @@ static const struct ac97_codec_id snd_ac97_codec_ids[] = { { 0x54584e20, 0xffffffff, "TLC320AD9xC", NULL, NULL }, { 0x56494161, 0xffffffff, "VIA1612A", NULL, NULL }, // modified ICE1232 with S/PDIF { 0x56494170, 0xffffffff, "VIA1617A", patch_vt1617a, NULL }, // modified VT1616 with S/PDIF -{ 0x56494182, 0xffffffff, "VIA1618", NULL, NULL }, +{ 0x56494182, 0xffffffff, "VIA1618", patch_vt1618, NULL }, // clean sheet of crinkled paper { 0x57454301, 0xffffffff, "W83971D", NULL, NULL }, { 0x574d4c00, 0xffffffff, "WM9701,WM9701A", NULL, NULL }, { 0x574d4C03, 0xffffffff, "WM9703,WM9707,WM9708,WM9717", patch_wolfson03, NULL}, @@ -609,7 +609,6 @@ AC97_SINGLE("PC Speaker Playback Volume", AC97_PC_BEEP, 1, 15, 1) static const struct snd_kcontrol_new snd_ac97_controls_mic_boost = AC97_SINGLE("Mic Boost (+20dB)", AC97_MIC, 6, 1, 0);
- static const char* std_rec_sel[] = {"Mic", "CD", "Video", "Aux", "Line", "Mix", "Mix Mono", "Phone"}; static const char* std_3d_path[] = {"pre 3D", "post 3D"}; static const char* std_mix[] = {"Mix", "Mic"}; diff --git a/pci/ac97/ac97_patch.c b/pci/ac97/ac97_patch.c index bb028f8..2042415 100644 --- a/pci/ac97/ac97_patch.c +++ b/pci/ac97/ac97_patch.c @@ -3465,7 +3465,7 @@ static int patch_vt1616(struct snd_ac97 * ac97)
/* * unfortunately, the vt1617a stashes the twiddlers required for - * nooding the i/o jacks on 2 different regs. * thameans that we cant + * nooding the i/o jacks on 2 different regs. that means that we cant * use the easy way provided by AC97_ENUM_DOUBLE() we have to write * are own funcs. * @@ -3576,6 +3576,413 @@ int patch_vt1617a(struct snd_ac97 * ac97) return err; }
+ +/* use these alot in the 1618 code but i cant find a better place to put them */ + +static const char* std_enable[] = {"Enabled", "Disabled"}; +static const char* std_disable[] = {"Disabled","Enabled"}; + +/* disable enable c/lfe exchange */ + +static int snd_ac97_vt1618_clex_info(struct snd_kcontrol *kcontrol, + struct snd_ctl_elem_info *uinfo) +{ + return ac97_enum_text_info(kcontrol, uinfo, std_disable, 2); +} + +static int snd_ac97_vt1618_clex_get(struct snd_kcontrol *kcontrol, + struct snd_ctl_elem_value *ucontrol) +{ + struct snd_ac97 *pac97 = snd_kcontrol_chip(kcontrol); + + ucontrol->value.enumerated.item[0] = (snd_ac97_read(pac97, 0x5a) & + 0x0100) >> 8; + return 0; +} + +static int snd_ac97_vt1618_clex_put(struct snd_kcontrol *kcontrol, + struct snd_ctl_elem_value *ucontrol) +{ + struct snd_ac97 *pac97 = snd_kcontrol_chip(kcontrol); + + /* notice relationship between mask and shift! */ + + return snd_ac97_update_bits(pac97, 0x5a, 0x0100, + ucontrol->value.enumerated.item[0] << 8); +} + + +/* disable enable dc offset */ + +static int snd_ac97_vt1618_dcof_info(struct snd_kcontrol *kcontrol, + struct snd_ctl_elem_info *uinfo) +{ + return ac97_enum_text_info(kcontrol, uinfo, std_disable, 2); +} + +static int snd_ac97_vt1618_dcof_get(struct snd_kcontrol *kcontrol, + struct snd_ctl_elem_value *ucontrol) +{ + struct snd_ac97 *pac97 = snd_kcontrol_chip(kcontrol); + + ucontrol->value.enumerated.item[0] = (snd_ac97_read(pac97, 0x5a) & + 0x0400) >> 10; + return 0; +} + +static int snd_ac97_vt1618_dcof_put(struct snd_kcontrol *kcontrol, + struct snd_ctl_elem_value *ucontrol) +{ + struct snd_ac97 *pac97 = snd_kcontrol_chip(kcontrol); + + /* notice relationship between mask and shift! */ + + return snd_ac97_update_bits(pac97, 0x5a, 0x0400, + ucontrol->value.enumerated.item[0] << 10); +} + + +/* enable disable headphone amp */ + +static int snd_ac97_vt1618_hamp_info(struct snd_kcontrol *kcontrol, + struct snd_ctl_elem_info *uinfo) +{ + return ac97_enum_text_info(kcontrol, uinfo, std_enable, 2); +} + +static int snd_ac97_vt1618_hamp_get(struct snd_kcontrol *kcontrol, + struct snd_ctl_elem_value *ucontrol) +{ + struct snd_ac97 *pac97 = snd_kcontrol_chip(kcontrol); + + ucontrol->value.enumerated.item[0] = (snd_ac97_read(pac97, 0x5c) & + 0x0020) >> 5; + return 0; +} + +static int snd_ac97_vt1618_hamp_put(struct snd_kcontrol *kcontrol, + struct snd_ctl_elem_value *ucontrol) +{ + struct snd_ac97 *pac97 = snd_kcontrol_chip(kcontrol); + + /* notice relationship between mask and shift! */ + + return snd_ac97_update_bits(pac97, 0x5c, 0x0020, + ucontrol->value.enumerated.item[0] << 5); +} + + +/* enable disable surround back channel */ + +static int snd_ac97_vt1618_srbk_info(struct snd_kcontrol *kcontrol, + struct snd_ctl_elem_info *uinfo) +{ + return ac97_enum_text_info(kcontrol, uinfo, std_enable, 2); +} + +static int snd_ac97_vt1618_srbk_get(struct snd_kcontrol *kcontrol, + struct snd_ctl_elem_value *ucontrol) +{ + struct snd_ac97 *pac97 = snd_kcontrol_chip(kcontrol); + + ucontrol->value.enumerated.item[0] = (snd_ac97_read(pac97, 0x5c) & + 0x0008) >> 3; + return 0; +} + +static int snd_ac97_vt1618_srbk_put(struct snd_kcontrol *kcontrol, + struct snd_ctl_elem_value *ucontrol) +{ + struct snd_ac97 *pac97 = snd_kcontrol_chip(kcontrol); + + /* notice relationship between mask and shift! */ + + return snd_ac97_update_bits(pac97, 0x5c, 0x0008, + ucontrol->value.enumerated.item[0] << 3); +} + + +/* enable disable soft mute */ + +static int snd_ac97_vt1618_smute_info(struct snd_kcontrol *kcontrol, + struct snd_ctl_elem_info *uinfo) +{ + return ac97_enum_text_info(kcontrol, uinfo, std_enable, 2); +} + +static int snd_ac97_vt1618_smute_get(struct snd_kcontrol *kcontrol, + struct snd_ctl_elem_value *ucontrol) +{ + struct snd_ac97 *pac97 = snd_kcontrol_chip(kcontrol); + + ucontrol->value.enumerated.item[0] = (snd_ac97_read(pac97, 0x5c) & + 0x0001); + return 0; +} + +static int snd_ac97_vt1618_smute_put(struct snd_kcontrol *kcontrol, + struct snd_ctl_elem_value *ucontrol) +{ + struct snd_ac97 *pac97 = snd_kcontrol_chip(kcontrol); + + /* notice relationship between mask and shift! */ + + return snd_ac97_update_bits(pac97, 0x5c, 0x0001, + ucontrol->value.enumerated.item[0]); +} + + +/* config aux in jack - not found on 3 jack motherboards or soundcards */ + +static int snd_ac97_vt1618_auxin_info(struct snd_kcontrol *kcontrol, + struct snd_ctl_elem_info *uinfo) +{ + static const char* texts[] = {"Aux In", "Surround Back Out"}; + return ac97_enum_text_info(kcontrol, uinfo, texts, 2); +} + +static int snd_ac97_vt1618_auxin_get(struct snd_kcontrol *kcontrol, + struct snd_ctl_elem_value *ucontrol) +{ + struct snd_ac97 *pac97 = snd_kcontrol_chip(kcontrol); + + ucontrol->value.enumerated.item[0] = (snd_ac97_read(pac97, 0x76) & + 0x0008) >> 3; + + return 0; +} + +static int snd_ac97_vt1618_auxin_put(struct snd_kcontrol *kcontrol, + struct snd_ctl_elem_value *ucontrol) +{ + struct snd_ac97 *pac97 = snd_kcontrol_chip(kcontrol); + + /* notice relationship between mask and shift! */ + + return snd_ac97_update_bits(pac97, 0x76, 0x0008, + ucontrol->value.enumerated.item[0] << 3); +} + + +/* + * VIA implements 'Smart 5.1' completely differently on the 1618 than + * it does on the 1617a. awesome! They seem to have sourced this + * particular revision of the technology from somebody else, it's + * called Universal Audio Jack, it shows up on some other folk's chips + * as well. + * + * ordering in this list reflects vt1618 docs for Reg 60h and + * the block diagram, DACs are as follows: + * + * OUT_O -> Front, + * OUT_1 -> Surround, + * OUT_2 -> C/LFE + * + * Unlike the 1617a, each OUT has a consistent set of mappings + * for all bitpatterns other than 00: + * + * 01 Unmixed Output + * 10 Line In + * 11 Mic In + * + * Special Case of 00: + * + * OUT_0 Mixed Output + * OUT_1 Reserved + * OUT_2 Reserved + * + * I have no idea what the hell Reserved does, but on an MSI + * CN700T, i have to set it to get surround output - YMMV, bad + * shit may happen. + * + * If other chips use Universal Audio Jack, then this code might be + * applicable to them. + */ + +/* copied from ac97_surround_jack_mode_info() ordering in this list + * reflects vt1618 docs for Vendor Defined Register 0x60 */ + +static int snd_ac97_vt1618_UAJ0_info(struct snd_kcontrol *kcontrol, + struct snd_ctl_elem_info *uinfo) +{ + static const char* texts[] = { "Speaker Out", "DAC Unmixed Out", "Line In", "Mic In"}; + return ac97_enum_text_info(kcontrol, uinfo, texts, 4); +} + +/* UAJ1 and UAJ2 are not supposed to have 00 written to them?? i + * dunno, because thats something that i have to do to get 5.1 out to + * work. */ + +static int snd_ac97_vt1618_UAJ1_info(struct snd_kcontrol *kcontrol, + struct snd_ctl_elem_info *uinfo) +{ + static const char* texts[] = { "Surround Out", "DAC Unmixed Out", "Line In", "Mic In"}; + return ac97_enum_text_info(kcontrol, uinfo, texts, 4); +} + +static int snd_ac97_vt1618_UAJ2_info(struct snd_kcontrol *kcontrol, struct snd_ctl_elem_info *uinfo) +{ + static const char* texts[] = { "Center/LowFre Out", "DAC Unmixed Out", "Line In", "Mic In"}; + return ac97_enum_text_info(kcontrol, uinfo, texts, 4); +} + +/* All of the vt1618 Universal Audio Jack twiddlers are on + Vendor Defined Register 0x60, page 0. The bits, and thus + the mask, are the only thing that changes */ + +static int snd_ac97_vt1618_UAJ_get(struct snd_kcontrol *kcontrol, + struct snd_ctl_elem_value *ucontrol, unsigned short mask) +{ + struct snd_ac97 *pac97; + ushort usDatPag, usUAJ, usShift = 0; // 0 is a possible value + + if(0x000C == mask) usShift = 2; + if(0x0030 == mask) usShift = 4; + + pac97 = snd_kcontrol_chip(kcontrol); + + mutex_lock(&pac97->page_mutex); + + usDatPag = snd_ac97_read(pac97, AC97_INT_PAGING) & AC97_PAGE_MASK; + snd_ac97_update_bits(pac97, AC97_INT_PAGING, AC97_PAGE_MASK, 0); + + usUAJ = snd_ac97_read(pac97, 0x60) & mask; + + snd_ac97_update_bits(pac97, AC97_INT_PAGING, AC97_PAGE_MASK, usDatPag); + mutex_unlock(&pac97->page_mutex); // unlock paging + + ucontrol->value.enumerated.item[0] = usUAJ >> usShift; + + return 0; +} + +static int snd_ac97_vt1618_UAJ_put(struct snd_kcontrol *kcontrol, + struct snd_ctl_elem_value *ucontrol, + unsigned short mask) +{ + struct snd_ac97 *pac97; + ushort usUAJ, usShift = 0; /* 0 is a possible value */ + + if(0x000C == mask) usShift = 2; + if(0x0030 == mask) usShift = 4; + + pac97 = snd_kcontrol_chip(kcontrol); + + usUAJ = ucontrol->value.enumerated.item[0] << usShift; + + return ac97_update_bits_page(pac97, 0x60, mask, usUAJ, 0); +} + +static int snd_ac97_vt1618_UAJ0_get(struct snd_kcontrol *kcontrol, + struct snd_ctl_elem_value *ucontrol) +{ + return snd_ac97_vt1618_UAJ_get(kcontrol, ucontrol, 0x0003); +} + +static int snd_ac97_vt1618_UAJ1_get(struct snd_kcontrol *kcontrol, + struct snd_ctl_elem_value *ucontrol) +{ + return snd_ac97_vt1618_UAJ_get(kcontrol, ucontrol, 0x000C); +} + +static int snd_ac97_vt1618_UAJ2_get(struct snd_kcontrol *kcontrol, + struct snd_ctl_elem_value *ucontrol) +{ + return snd_ac97_vt1618_UAJ_get(kcontrol, ucontrol, 0x0030); +} + +static int snd_ac97_vt1618_UAJ0_put(struct snd_kcontrol *kcontrol, + struct snd_ctl_elem_value *ucontrol) +{ + return snd_ac97_vt1618_UAJ_put(kcontrol, ucontrol, 0x0003); +} + +static int snd_ac97_vt1618_UAJ1_put(struct snd_kcontrol *kcontrol, + struct snd_ctl_elem_value *ucontrol) +{ + return snd_ac97_vt1618_UAJ_put(kcontrol, ucontrol, 0x000C); +} + +static int snd_ac97_vt1618_UAJ2_put(struct snd_kcontrol *kcontrol, + struct snd_ctl_elem_value *ucontrol) +{ + return snd_ac97_vt1618_UAJ_put(kcontrol, ucontrol, 0x0030); +} + +static const struct snd_kcontrol_new snd_ac97_controls_vt1618[] = { + + { + .iface = SNDRV_CTL_ELEM_IFACE_MIXER, + .name = "Center LFE Exchange", + .info = snd_ac97_vt1618_clex_info, + .get = snd_ac97_vt1618_clex_get, + .put = snd_ac97_vt1618_clex_put + }, + { + .iface = SNDRV_CTL_ELEM_IFACE_MIXER, + .name = "DC Offset", + .info = snd_ac97_vt1618_dcof_info, + .get = snd_ac97_vt1618_dcof_get, + .put = snd_ac97_vt1618_dcof_put + }, + { + .iface = SNDRV_CTL_ELEM_IFACE_MIXER, + .name = "Headphone Amp", + .info = snd_ac97_vt1618_hamp_info, + .get = snd_ac97_vt1618_hamp_get, + .put = snd_ac97_vt1618_hamp_put + }, + { + .iface = SNDRV_CTL_ELEM_IFACE_MIXER, + .name = "Surround Back", + .info = snd_ac97_vt1618_srbk_info, + .get = snd_ac97_vt1618_srbk_get, + .put = snd_ac97_vt1618_srbk_put + }, + { + .iface = SNDRV_CTL_ELEM_IFACE_MIXER, + .name = "Soft Mute", + .info = snd_ac97_vt1618_smute_info, + .get = snd_ac97_vt1618_smute_get, + .put = snd_ac97_vt1618_smute_put + }, + { + .iface = SNDRV_CTL_ELEM_IFACE_MIXER, + .name = "Aux In Jack", + .info = snd_ac97_vt1618_auxin_info, + .get = snd_ac97_vt1618_auxin_get, + .put = snd_ac97_vt1618_auxin_put + }, + { + .iface = SNDRV_CTL_ELEM_IFACE_MIXER, + .name = "Speaker Jack", + .info = snd_ac97_vt1618_UAJ0_info, + .get = snd_ac97_vt1618_UAJ0_get, + .put = snd_ac97_vt1618_UAJ0_put + }, + { + .iface = SNDRV_CTL_ELEM_IFACE_MIXER, + .name = "Line In Jack", + .info = snd_ac97_vt1618_UAJ1_info, + .get = snd_ac97_vt1618_UAJ1_get, + .put = snd_ac97_vt1618_UAJ1_put + }, + { + .iface = SNDRV_CTL_ELEM_IFACE_MIXER, + .name = "Mic In Jack", + .info = snd_ac97_vt1618_UAJ2_info, + .get = snd_ac97_vt1618_UAJ2_get, + .put = snd_ac97_vt1618_UAJ2_put + } +}; + +int patch_vt1618(struct snd_ac97 *ac97) +{ + return patch_build_controls(ac97, snd_ac97_controls_vt1618, + ARRAY_SIZE(snd_ac97_controls_vt1618)); +} + + /* */ static void it2646_update_jacks(struct snd_ac97 *ac97) put + }, + { + .iface = SNDRV_CTL_ELEM_IFACE_MIXER, + .name = "Headphone Amp", + .info = snd_ac97_vt1618_hamp_info, + .get = snd_ac97_vt1618_hamp_get, + .put = snd_ac97_vt1618_hamp_put + }, + { + .iface = SNDRV_CTL_ELEM_IFACE_MIXER, + .name = "Surround Back", + .info = snd_ac97_vt1618_srbk_info, + .get = snd_ac97_vt1618_srbk_get, + .put = snd_ac97_vt1618_srbk_put + }, + { + .iface = SNDRV_CTL_ELEM_IFACE_MIXER, + .name = "Soft Mute", + .info = snd_ac97_vt1618_smute_info, + .get = snd_ac97_vt1618_smute_get, + .put = snd_ac97_vt1618_smute_put + }, + { + .iface = SNDRV_CTL_ELEM_IFACE_MIXER, + .name = "Aux In Jack", + .info = snd_ac97_vt1618_auxin_info, + .get = snd_ac97_vt1618_auxin_get, + .put = snd_ac97_vt1618_auxin_put + }, + { + .iface = SNDRV_CTL_ELEM_IFACE_MIXER, + .name = "Speaker Jack", + .info = snd_ac97_vt1618_UAJ0_info, + .get = snd_ac97_vt1618_UAJ0_get, + .put = snd_ac97_vt1618_UAJ0_put + }, + { + .iface = SNDRV_CTL_ELEM_IFACE_MIXER, + .name = "Line In Jack", + .info = snd_ac97_vt1618_UAJ1_info, + .get = snd_ac97_vt1618_UAJ1_get, + .put = snd_ac97_vt1618_UAJ1_put + }, + { + .iface = SNDRV_CTL_ELEM_IFACE_MIXER, + .name = "Mic In Jack", + .info = snd_ac97_vt1618_UAJ2_info, + .get = snd_ac97_vt1618_UAJ2_get, + .put = snd_ac97_vt1618_UAJ2_put + } +}; + +int patch_vt1618(struct snd_ac97 *ac97) +{ + return patch_build_controls(ac97, snd_ac97_controls_vt1618, + ARRAY_SIZE(snd_ac97_controls_vt1618)); +} + + /* */ static void it2646_update_jacks(struct snd_ac97 *ac97)
+ { + .iface = SNDRV_CTL_ELEM_IFACE_MIXER, + .name = "Headphone Amp", + .info = snd_ac97_vt1618_hamp_info, + .get = snd_ac97_vt1618_hamp_get, + .put = snd_ac97_vt1618_hamp_put + }, + { + .iface = SNDRV_CTL_ELEM_IFACE_MIXER, + .name = "Surround Back", + .info = snd_ac97_vt1618_srbk_info, + .get = snd_ac97_vt1618_srbk_get, + .put = snd_ac97_vt1618_srbk_put + }, + { + .iface = SNDRV_CTL_ELEM_IFACE_MIXER, + .name = "Soft Mute", + .info = snd_ac97_vt1618_smute_info, + .get = snd_ac97_vt1618_smute_get, + .put = snd_ac97_vt1618_smute_put + }, + { + .iface = SNDRV_CTL_ELEM_IFACE_MIXER, + .name = "Aux In Jack", + .info = snd_ac97_vt1618_auxin_info, + .get = snd_ac97_vt1618_auxin_get, + .put = snd_ac97_vt1618_auxin_put + }, + { + .iface = SNDRV_CTL_ELEM_IFACE_MIXER, + .name = "Speaker Jack", + .info = snd_ac97_vt1618_UAJ0_info, + .get = snd_ac97_vt1618_UAJ0_get, + .put = snd_ac97_vt1618_UAJ0_put + }, + { + .iface = SNDRV_CTL_ELEM_IFACE_MIXER, + .name = "Line In Jack", + .info = snd_ac97_vt1618_UAJ1_info, + .get = snd_ac97_vt1618_UAJ1_get, + .put = snd_ac97_vt1618_UAJ1_put + }, + { + .iface = SNDRV_CTL_ELEM_IFACE_MIXER, + .name = "Mic In Jack", + .info = snd_ac97_vt1618_UAJ2_info, + .get = snd_ac97_vt1618_UAJ2_get, + .put = snd_ac97_vt1618_UAJ2_put + } +}; + +int patch_vt1618(struct snd_ac97 *ac97) +{ + return patch_build_controls(ac97, snd_ac97_controls_vt1618, + ARRAY_SIZE(snd_ac97_controls_vt1618)); +} + + /* */ static void it2646_update_jacks(struct snd_ac97 *ac97)
At Tue, 19 Aug 2008 17:17:44 -0700, John L. Utz III wrote:
Signed-off-by: John L. Utz III john.utz@dmx.com
Thanks for the patch.
First off, could you give a brief description what this patch does/fixes exactly?
Below is a quick review.
diff --git a/pci/ac97/ac97_codec.c b/pci/ac97/ac97_codec.c index d0023e9..a34f1ea 100644 --- a/pci/ac97/ac97_codec.c +++ b/pci/ac97/ac97_codec.c @@ -168,7 +168,7 @@ static const struct ac97_codec_id snd_ac97_codec_ids[] = { { 0x54584e20, 0xffffffff, "TLC320AD9xC", NULL, NULL }, { 0x56494161, 0xffffffff, "VIA1612A", NULL, NULL }, // modified ICE1232 with S/PDIF { 0x56494170, 0xffffffff, "VIA1617A", patch_vt1617a, NULL }, // modified VT1616 with S/PDIF -{ 0x56494182, 0xffffffff, "VIA1618", NULL, NULL }, +{ 0x56494182, 0xffffffff, "VIA1618", patch_vt1618, NULL }, // clean
Please avoid C++ comments. Yeah, we see it in other places here and there, but you don't need to introduce more in the newly added code.
+/* use these alot in the 1618 code but i cant find a better place to put them */
+static const char* std_enable[] = {"Enabled", "Disabled"}; +static const char* std_disable[] = {"Disabled","Enabled"};
They must be consistent -- usually 0 = disable, 1 = enable. In general, the controls like "XXX Disable" is bad. A switch should be to turn on, not to turn off. If the register bit is to disable something, implement the control element in a reverse way.
Also, you can consider to implement it as a switch, not as an enum, depending on the role. Then it'll be often better handled in many mixer apps.
The rest are mostly about coding styles.
+static int snd_ac97_vt1618_UAJ0_info(struct snd_kcontrol *kcontrol,
struct snd_ctl_elem_info *uinfo)
+{
- static const char* texts[] = { "Speaker Out", "DAC Unmixed Out", "Line
In", "Mic In"};
Avoid too long lines (also in other places).
+{
- struct snd_ac97 *pac97;
- ushort usDatPag, usUAJ, usShift = 0; // 0 is a possible value
ushort isn't a standard type. Use either unsigned short or u16 if it must be 16bit. Also, a Windows style variable name should be avoided. People tend to hate it.
- if(0x000C == mask) usShift = 2;
- if(0x0030 == mask) usShift = 4;
Fix coding style please.
- pac97 = snd_kcontrol_chip(kcontrol);
mutex_lock(&pac97->page_mutex);
Keep to use tabs instead of space letters (found in other places).
+static int snd_ac97_vt1618_UAJ0_get(struct snd_kcontrol *kcontrol,
struct snd_ctl_elem_value *ucontrol)
+{
- return snd_ac97_vt1618_UAJ_get(kcontrol, ucontrol, 0x0003);
+}
+static int snd_ac97_vt1618_UAJ1_get(struct snd_kcontrol *kcontrol,
struct snd_ctl_elem_value *ucontrol)
+{
- return snd_ac97_vt1618_UAJ_get(kcontrol, ucontrol, 0x000C);
+}
You can use private_value field in each definition of the control, so that you don't have to create each different function.
About the coding style, I recommend to run checkpatch.pl script once before reposting your patch. The script is found in scripts directory of the recent linux-kernel source tree.
Takashi
Hi Takashi;
tnx for the review, a few questions inline.
On Wed, 20 Aug 2008 01:28:39 -0700, Takashi Iwai tiwai@suse.de wrote:
At Tue, 19 Aug 2008 17:17:44 -0700, John L. Utz III wrote:
Signed-off-by: John L. Utz III john.utz@dmx.com
Thanks for the patch.
First off, could you give a brief description what this patch does/fixes exactly?
Below is a quick review.
diff --git a/pci/ac97/ac97_codec.c b/pci/ac97/ac97_codec.c index d0023e9..a34f1ea 100644 --- a/pci/ac97/ac97_codec.c +++ b/pci/ac97/ac97_codec.c @@ -168,7 +168,7 @@ static const struct ac97_codec_id snd_ac97_codec_ids[] = { { 0x54584e20, 0xffffffff, "TLC320AD9xC", NULL, NULL }, { 0x56494161, 0xffffffff, "VIA1612A", NULL, NULL }, // modified ICE1232 with S/PDIF { 0x56494170, 0xffffffff, "VIA1617A", patch_vt1617a, NULL }, // modified VT1616 with S/PDIF -{ 0x56494182, 0xffffffff, "VIA1618", NULL, NULL }, +{ 0x56494182, 0xffffffff, "VIA1618", patch_vt1618, NULL }, // clean
Please avoid C++ comments. Yeah, we see it in other places here and there, but you don't need to introduce more in the newly added code.
OK. takes more space tho.
+/* use these alot in the 1618 code but i cant find a better place to put them */
+static const char* std_enable[] = {"Enabled", "Disabled"}; +static const char* std_disable[] = {"Disabled","Enabled"};
They must be consistent -- usually 0 = disable, 1 = enable. In general, the controls like "XXX Disable" is bad. A switch should be to turn on, not to turn off. If the register bit is to disable something, implement the control element in a reverse way.
OK.
note that it makes the code more complex in some places because the chip is inconsistent.
Also, you can consider to implement it as a switch, not as an enum, depending on the role. Then it'll be often better handled in many mixer apps.
The rest are mostly about coding styles.
+static int snd_ac97_vt1618_UAJ0_info(struct snd_kcontrol *kcontrol,
struct snd_ctl_elem_info *uinfo)
+{
- static const char* texts[] = { "Speaker Out", "DAC Unmixed Out", "Line
In", "Mic In"};
Avoid too long lines (also in other places).
OK. 80 column?
+{
- struct snd_ac97 *pac97;
- ushort usDatPag, usUAJ, usShift = 0; // 0 is a possible value
ushort isn't a standard type. Use either unsigned short or u16 if it must be 16bit. Also, a Windows style variable name should be avoided. People tend to hate it.
Why would you call this 'Windows Style?' Is that supposed to be a perjorative comment?
The code compiled with gcc and creates a linux kernel module and I am using Gentoo. Wouldnt that then make it reasonable to call it 'Linux Style' or 'GPL Style' or 'Gentoo Style? or 'John Utz Style' ?
Having variable type decorations provides valuable context and (IMHO) makes the code more maintainable.
- if(0x000C == mask) usShift = 2;
- if(0x0030 == mask) usShift = 4;
Fix coding style please.
specifics please. wait. nevermind, i'll run checkpatch against it. i forgot to do that last nite. :-)
- pac97 = snd_kcontrol_chip(kcontrol);
mutex_lock(&pac97->page_mutex);
Keep to use tabs instead of space letters (found in other places).
OK
+static int snd_ac97_vt1618_UAJ0_get(struct snd_kcontrol *kcontrol,
struct snd_ctl_elem_value *ucontrol)
+{
- return snd_ac97_vt1618_UAJ_get(kcontrol, ucontrol, 0x0003);
+}
+static int snd_ac97_vt1618_UAJ1_get(struct snd_kcontrol *kcontrol,
struct snd_ctl_elem_value *ucontrol)
+{
- return snd_ac97_vt1618_UAJ_get(kcontrol, ucontrol, 0x000C);
+}
You can use private_value field in each definition of the control, so that you don't have to create each different function.
OH, that would be AWESOME. Can you elaborate? I'll go grep for private value in the existing code and see if i can find an example whilst i wait for your explanation.
About the coding style, I recommend to run checkpatch.pl script once before reposting your patch. The script is found in scripts directory of the recent linux-kernel source tree.
yes. i realized as i was walking to the bus last nite that i had forgotten to do so.
johnu
Takashi
On 20-08-08 19:50, John L. Utz III wrote:
On Wed, 20 Aug 2008 01:28:39 -0700, Takashi Iwai tiwai@suse.de wrote:
Also, a Windows style variable name should be avoided. People tend to hate it.
Why would you call this 'Windows Style?' Is that supposed to be a perjorative comment?
You insult really quickly... :-)
The style is more generally known as Hungarian Notation after its Hungarian "inventor" who was a chief architect at Microsoft. This notational conventation saw widespread use in the Windows API and not much use anywhere beyond that.
Rene.
On Wed, 20 Aug 2008 12:05:33 -0700, Rene Herman rene.herman@keyaccess.nl wrote:
On 20-08-08 19:50, John L. Utz III wrote:
On Wed, 20 Aug 2008 01:28:39 -0700, Takashi Iwai tiwai@suse.de wrote:
Also, a Windows style variable name should be avoided. People tend to hate it.
Why would you call this 'Windows Style?' Is that supposed to be a perjorative comment?
You insult really quickly... :-)
Quickly, but not deeply. :-)
The style is more generally known as Hungarian Notation after its Hungarian "inventor" who was a chief architect at Microsoft.
Yuppers. I know. Charles Simonyi. I really, really, like it.
This notational conventation saw widespread use in the Windows API and not much use anywhere beyond that.
with an example of 'beyond that' being John Utz's code.
Be it C, Java, C#, Javascript, Ada or any languages that use strong types.
I dont do it in shell script tho:
# Let any boot observers know what devices we found
echo "Type MotherBoard: ($TYPEMBD)" echo "Type Soundcard: ($TYPESND)" echo "Type Ethenet: ($TYPEETH)" echo "Type Modem: ($TYPEMDM)"
anyway, we can argue about it again after i get a checkpatch compliant patch to submit....
johnu
Rene.
On 20-08-08 21:16, John L. Utz III wrote:
anyway, we can argue about it again after i get a checkpatch compliant patch to submit....
Let's get a headstart...
Note that Hungarian is not a private Takashi Iwai or ALSA dislike; it's rather specifically against the Linux kernel coding style (although the actual CodingStyle document only mentions Hungarian for function names).
Most contributors have had to adjust a few local habits and in the end, it's generally worth it. If all is good and well, open source code is read many more times than it's written after all and any (significant) style inconsistency costs every reader just a little mental hickup. Review resources are one of the more scarcely available ones in this development process, so a consistent style definitely pays.
Sure, sure, I also try ty get away with things, but Hungarians hickup rather loudly.
Rene.
On Wed, 20 Aug 2008 12:54:23 -0700, Rene Herman rene.herman@keyaccess.nl wrote:
On 20-08-08 21:16, John L. Utz III wrote:
anyway, we can argue about it again after i get a checkpatch compliant patch to submit....
Let's get a headstart...
hokydokysmoky!
Note that Hungarian is not a private Takashi Iwai or ALSA dislike; it's rather specifically against the Linux kernel coding style (although the actual CodingStyle document only mentions Hungarian for function names).
'rather specifically' is your interpretation, not a fact.
CodingStyle.C4 just asks that naming be 'Spartan'.
So, to map 'Spartan' to my code:
this is an unsigned short variable that contains a value that will be used to shift another variable:
usigned short usShift;
ucontrol->value.enumerated.item[0] << usShift;
IMHO, this is a legitimate name in the context of CodingStyle.C4
Most contributors have had to adjust a few local habits and in the end, it's generally worth it.
right.
'tis why i didnt submit something with a 2 space indent. :-)
`tis why i left my leading 'p's out of the function argument signatures.
If all is good and well, open source code is read many more times than it's written after all and any (significant) style inconsistency costs every reader just a little mental hickup.
right. but we differ on what is an inconsistency.
Review resources are one of the more scarcely available ones in this development process, so a consistent style definitely pays.
Sure, sure, I also try ty get away with things, but Hungarians hickup rather loudly.
I think that folks might be really surprised how helpful hungarian notation can be.
But this is a team effort and i am not gonna get all mulish about it because it's a very silly thing to be mulish about.
If Takashi tells me that he rejects the next version of the patch solely due to the naming of my variables then i will change them.
But if he isnt going to reject it on that basis then they will stay the way they are.
It is open source after all. Somebody could submit a patch to change them to something they like better.
Rene.
On 20-08-08 22:50, John L. Utz III wrote:
On Wed, 20 Aug 2008 12:54:23 -0700, Rene Herman rene.herman@keyaccess.nl wrote:
Note that Hungarian is not a private Takashi Iwai or ALSA dislike; it's rather specifically against the Linux kernel coding style (although the actual CodingStyle document only mentions Hungarian for function names).
'rather specifically' is your interpretation, not a fact.
No it's not my interpretation. It's based on many years of reading the linux kernel mailing list and seing patches being rejected over things like it for example. Also ssee the rest of the tree.
As to being surprised, I wouldn't be, having used Hungarian for years in Windows programming.
Rene.
On Wed, Aug 20, 2008 at 10:50:49AM -0700, John L. Utz III wrote:
On Wed, 20 Aug 2008 01:28:39 -0700, Takashi Iwai tiwai@suse.de wrote:
They must be consistent -- usually 0 = disable, 1 = enable. In general, the controls like "XXX Disable" is bad. A switch should be to turn on, not to turn off. If the register bit is to disable something, implement the control element in a reverse way.
OK.
note that it makes the code more complex in some places because the chip is inconsistent.
ASoC does this by having a flag in the control specific data saying if the control is inverted which works pretty well for dealing with this situation.
Avoid too long lines (also in other places).
OK. 80 column?
Yes. See Documentation/CodingStyle (and checkpatch).
Also, a Windows style variable name should be avoided. People tend to hate it.
Why would you call this 'Windows Style?' Is that supposed to be a perjorative comment?
This style is used throughout the Windows API documentation and is therefore frequently encounterd in Windows code but it's rarely encountered anywhere else. It's called Hungarian notation, though in the form it's normally seen it's very far from what was originally intended by that.
Having variable type decorations provides valuable context and (IMHO) makes the code more maintainable.
Many people have forcefuly held views in the opposite direction - google will turn up a bunch. I have the following in my fortune database:
prepAs nounOthers verbHave verbNoted, pronThis nounStyle verbIs verbCalled adjHungarian nounNotation. pronI verbFind pronIt advCompletely adjUseless. pronI verbHope pronYou verbCan verbSee possMy nounPoint -- verbEspecially nounWhen possThe advPrefixes conjBecome verbUtterly adjWrong. :-) -- Chris Torek nospam@elf.eng.bsdi.com
participants (4)
-
John L. Utz III
-
Mark Brown
-
Rene Herman
-
Takashi Iwai