#1/4 for Zoom Video - resolve common vs chipset specific mixer controls
PATCH 1/1 #1/4 for Zoom Video - resolve common vs chipset specific mixer controls
First of 4 es18xx.c patches culminating in Zoom Video support.
While adding support for Zoom Video to the es18xx driver I found some of the mixer controls were wrong. Since you guys went to the trouble of supplying the datasheets for the supported chipsets I did a review of all of them and tried to get es18xx.c to accurately reflect the proper mixer controls for each chipset. If the datasheets are wrong then so are my patches.
This first patch moves some controls from the common-to-all-chipsets array 'snd_es18xx_base_controls' to a chipset-specific array and adds code to manage that new array. Also while testing on my ES1878 test machine I discovered it needed a couple of udelays in the identify function so those are in this patch as well.
Testing:
This work was initially done on the source from the Debian Sarge ALSA package, then tested on an ES1879 and an ES1878 machine. Patches were created against the Sarge code and then edited to apply correctly to the ALSA cvs code. Lastly the patched ALSA cvs code was test for successful compilation. No additional testing was done on the ALSA cvs version.
Applying:
cd alsa-driver/
cat ../../es18xx.CVSmod1.diff | patch -p1
------
alsa-kernel/isa/es18xx.c 2005-12-10 13:59:30.000000000 -0500 renamed from alsa-driver/alsa-kernel/isa/es18xx.c 2004-12-15 10:26:10.000000000 -0500 +0 -0
@@ -1181,19 +1181,22 @@ return change;
}
+/* Mixer controls
+ * These arrays contain setup data for mixer controls.
+ *
+ * The controls that are universal to all chipsets are fully initialized
+ * here.
+ */
static struct snd_kcontrol_new snd_es18xx_base_controls[] = { ES18XX_DOUBLE("Master Playback Volume", 0, 0x60, 0x62, 0, 0, 63, 0), ES18XX_DOUBLE("Master Playback Switch", 0, 0x60, 0x62, 6, 6, 1, 1), ES18XX_DOUBLE("Line Playback Volume", 0, 0x3e, 0x3e, 4, 0, 15, 0), ES18XX_DOUBLE("CD Playback Volume", 0, 0x38, 0x38, 4, 0, 15, 0), ES18XX_DOUBLE("FM Playback Volume", 0, 0x36, 0x36, 4, 0, 15, 0), -ES18XX_DOUBLE("Mono Playback Volume", 0, 0x6d, 0x6d, 4, 0, 15, 0), ES18XX_DOUBLE("Mic Playback Volume", 0, 0x1a, 0x1a, 4, 0, 15, 0), ES18XX_DOUBLE("Aux Playback Volume", 0, 0x3a, 0x3a, 4, 0, 15, 0), -ES18XX_SINGLE("PC Speaker Playback Volume", 0, 0x3c, 0, 7, 0), ES18XX_SINGLE("Record Monitor", 0, 0xa8, 3, 1, 0), ES18XX_DOUBLE("Capture Volume", 0, 0xb4, 0xb4, 4, 0, 15, 0), -ES18XX_SINGLE("Capture Switch", 0, 0x1c, 4, 1, 1),
{
.iface = SNDRV_CTL_ELEM_IFACE_MIXER,
.name = "Capture Source",
@@ -1203,19 +1206,34 @@
}
};
-static struct snd_kcontrol_new snd_es18xx_mono_in_control = -ES18XX_DOUBLE("Mono Input Playback Volume", 0, 0x6d, 0x6d, 4, 0, 15, 0); -
static struct snd_kcontrol_new snd_es18xx_recmix_controls[] = { ES18XX_DOUBLE("PCM Capture Volume", 0, 0x69, 0x69, 4, 0, 15, 0), ES18XX_DOUBLE("Mic Capture Volume", 0, 0x68, 0x68, 4, 0, 15, 0), ES18XX_DOUBLE("Line Capture Volume", 0, 0x6e, 0x6e, 4, 0, 15, 0), ES18XX_DOUBLE("FM Capture Volume", 0, 0x6b, 0x6b, 4, 0, 15, 0), -ES18XX_DOUBLE("Mono Capture Volume", 0, 0x6f, 0x6f, 4, 0, 15, 0), ES18XX_DOUBLE("CD Capture Volume", 0, 0x6a, 0x6a, 4, 0, 15, 0), ES18XX_DOUBLE("Aux Capture Volume", 0, 0x6c, 0x6c, 4, 0, 15, 0) };
+/*
+ * These chipset specific mixer controls are initialized in
+ * snd_es18xx_mixer. Those controls which are not UNSET are then
+ * instantiated.
+ */
+static char UNSETbuf = '\0';
+#define UNSET ((char *)&UNSETbuf)
+static enum optionalControlsIndex {reg1cBit4, reg3cMono, reg6d, reg6f}; +static struct snd_kcontrol_new snd_es18xx_optional_controls[] = { +ES18XX_SINGLE(UNSET, 0, 0x1c, 4, 1, 1), +ES18XX_SINGLE(UNSET, 0, 0x3c, 0, 7, 0), +ES18XX_DOUBLE(UNSET, 0, 0x6d, 0x6d, 4, 0, 15, 0), +ES18XX_DOUBLE(UNSET, 0, 0x6f, 0x6f, 4, 0, 15, 0)
+};
+static char* captureSwitchName = "Capture Switch";
+static char* speakerPlayName = "PC Speaker Playback Volume"; +static char* monoPlayName = "Mono Playback Volume";
+static char* monoRecName = "Mono Capture Volume";
+
static struct snd_kcontrol_new snd_es18xx_pcm1_controls[] = { ES18XX_DOUBLE("PCM Playback Volume", 0, 0x14, 0x14, 4, 0, 15, 0), };
@@ -1466,11 +1484,14 @@
}
outb(0x40, chip->port + 0x04);
+ udelay(10);
hi = inb(chip->port + 0x05);
+ udelay(10);
lo = inb(chip->port + 0x05);
if (hi != lo) {
chip->version = hi << 8 | lo;
chip->ctrl_port = inb(chip->port + 0x05) << 8;
+ udelay(10);
chip->ctrl_port += inb(chip->port + 0x05);
if ((chip->res_ctrl_port = request_region(chip->ctrl_port, 8,
"ES18xx - CTRL")) == NULL) {
@@ -1781,10 +1802,6 @@
}
}- if (chip->caps & ES18XX_MONO) {
- if ((err = snd_ctl_add(card, snd_ctl_new1(&snd_es18xx_mono_in_control, chip))) < 0)
- return err;
- }
if (chip->caps & ES18XX_RECMIX) {
for (idx = 0; idx < ARRAY_SIZE(snd_es18xx_recmix_controls); idx++) {
if ((err = snd_ctl_add(card,snd_ctl_new1(&snd_es18xx_recmix_controls[idx], chip))) < 0) @@ -1822,6 +1839,24 @@
}
}+/* finish initializing other chipset specific controls
+ */
+ if (chip->version != 0x1868) {
+ snd_es18xx_optional_controls[reg3cMono].name = speakerPlayName;
+ }
+ switch (chip->version) {
+ case 0x1869:
+ snd_es18xx_optional_controls[reg1cBit4].name = captureSwitchName;
+ snd_es18xx_optional_controls[reg6d].name = monoPlayName;
+ snd_es18xx_optional_controls[reg6f].name = monoRecName;
+ break;+ }
+ for (idx = 0; idx < ARRAY_SIZE(snd_es18xx_optional_controls); idx++) {
+ if (snd_es18xx_optional_controls[idx].name == UNSET)
+ continue;
+ if ((err = snd_ctl_add(card, snd_ctl_new1(&snd_es18xx_optional_controls[idx], chip))) < 0)
+ return err;
+ }
return 0; }
-------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc. Do you grep through log files for problems? Stop! Download the new AJAX search engine that makes searching your log files as easy as surfing the web. DOWNLOAD SPLUNK!
http://ads.osdn.com/?ad_id=7637&alloc_id=16865&op=click
Quoted 1 line · Mark Salazar
First of 4
es18xx.cpatches culminating in Zoom Video support.
All of these except the first are linewrapped and can't be applied.
Lee
-------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc. Do you grep through log files for problems? Stop! Download the new AJAX search engine that makes searching your log files as easy as surfing the web. DOWNLOAD SPLUNK!
http://ads.osdn.com/?ad_id=7637&alloc_id=16865&op=click
Quoted 6 lines · Lee Revell
Quoted 1 line · Mark Salazar
First of 4
es18xx.cpatches culminating in Zoom Video support.All of these except the first are linewrapped and can't be applied.
I surrender. Unable to make Thunderbird's wrapping function submit to my will I just attach my patch message and slink away.
First of 4 es18xx.c patches culminating in Zoom Video support.
While adding support for Zoom Video to the es18xx driver I found some of the mixer controls were wrong. Since you guys went to the trouble of supplying the datasheets for the supported chipsets I did a review of all of them and tried to get es18xx.c to accurately reflect the proper mixer controls for each chipset. If the datasheets are wrong then so are my patches.
This first patch moves some controls from the common-to-all-chipsets array 'snd_es18xx_base_controls' to a chipset-specific array and adds code to manage that new array. Also while testing on my ES1878 test machine I discovered it needed a couple of udelays in the identify function so those are in this patch as well.
Testing:
This work was initially done on the source from the Debian Sarge ALSA package, then tested on an ES1879 and an ES1878 machine. Patches were created against the Sarge code and then edited to apply correctly to the ALSA cvs code. Lastly the patched ALSA cvs code was test for successful compilation. No additional testing was done on the ALSA cvs version.
Applying:
cd alsa-driver/
cat ../../es18xx.CVSmod1.diff | patch -p1
------
alsa-kernel/isa/es18xx.c renamed from alsa-driver/alsa-kernel/isa/es18xx.c +46 -11
@@ -1181,19 +1181,22 @@return change; } +/* Mixer controls +* These arrays contain setup data for mixer controls. +* +* The controls that are universal to all chipsets are fully initialized +* here. +*/ static struct snd_kcontrol_new snd_es18xx_base_controls[] = {ES18XX_DOUBLE ("Master Playback Volume" ,0 ,0x60 ,0x62 ,0 ,0 ,63 ,0 ),ES18XX_DOUBLE ("Master Playback Switch" ,0 ,0x60 ,0x62 ,6 ,6 ,1 ,1 ),ES18XX_DOUBLE ("Line Playback Volume" ,0 ,0x3e ,0x3e ,4 ,0 ,15 ,0 ),ES18XX_DOUBLE ("CD Playback Volume" ,0 ,0x38 ,0x38 ,4 ,0 ,15 ,0 ),ES18XX_DOUBLE ("FM Playback Volume" ,0 ,0x36 ,0x36 ,4 ,0 ,15 ,0 ), -ES18XX_DOUBLE ("Mono Playback Volume" ,0 ,0x6d ,0x6d ,4 ,0 ,15 ,0 ),ES18XX_DOUBLE ("Mic Playback Volume" ,0 ,0x1a ,0x1a ,4 ,0 ,15 ,0 ),ES18XX_DOUBLE ("Aux Playback Volume" ,0 ,0x3a ,0x3a ,4 ,0 ,15 ,0 ), -ES18XX_SINGLE ("PC Speaker Playback Volume" ,0 ,0x3c ,0 ,7 ,0 ),ES18XX_SINGLE ("Record Monitor" ,0 ,0xa8 ,3 ,1 ,0 ),ES18XX_DOUBLE ("Capture Volume" ,0 ,0xb4 ,0xb4 ,4 ,0 ,15 ,0 ), -ES18XX_SINGLE ("Capture Switch" ,0 ,0x1c ,4 ,1 ,1 ), { .iface =SNDRV_CTL_ELEM_IFACE_MIXER , .name ="Capture Source" ,
@@ -1203,19 +1206,34 @@ } }; -static struct snd_kcontrol_new snd_es18xx_mono_in_control = -ES18XX_DOUBLE ("Mono Input Playback Volume" ,0 ,0x6d ,0x6d ,4 ,0 ,15 ,0 ); -static struct snd_kcontrol_new snd_es18xx_recmix_controls[] = {ES18XX_DOUBLE ("PCM Capture Volume" ,0 ,0x69 ,0x69 ,4 ,0 ,15 ,0 ),ES18XX_DOUBLE ("Mic Capture Volume" ,0 ,0x68 ,0x68 ,4 ,0 ,15 ,0 ),ES18XX_DOUBLE ("Line Capture Volume" ,0 ,0x6e ,0x6e ,4 ,0 ,15 ,0 ),ES18XX_DOUBLE ("FM Capture Volume" ,0 ,0x6b ,0x6b ,4 ,0 ,15 ,0 ), -ES18XX_DOUBLE ("Mono Capture Volume" ,0 ,0x6f ,0x6f ,4 ,0 ,15 ,0 ),ES18XX_DOUBLE ("CD Capture Volume" ,0 ,0x6a ,0x6a ,4 ,0 ,15 ,0 ),ES18XX_DOUBLE ("Aux Capture Volume" ,0 ,0x6c ,0x6c ,4 ,0 ,15 ,0 ) }; +/* +* These chipset specific mixer controls are initialized in +* snd_es18xx_mixer. Those controls which are not UNSET are then +* instantiated. +*/ +static char UNSETbuf ='\0' ; +#define UNSET ((char *)&UNSETbuf) +static enum optionalControlsIndex {reg1cBit4, reg3cMono, reg6d, reg6f}; +static struct snd_kcontrol_new snd_es18xx_optional_controls[] = { +ES18XX_SINGLE (UNSET ,0 ,0x1c ,4 ,1 ,1 ), +ES18XX_SINGLE (UNSET ,0 ,0x3c ,0 ,7 ,0 ), +ES18XX_DOUBLE (UNSET ,0 ,0x6d ,0x6d ,4 ,0 ,15 ,0 ), +ES18XX_DOUBLE (UNSET ,0 ,0x6f ,0x6f ,4 ,0 ,15 ,0 ) +}; +static char * captureSwitchName ="Capture Switch" ; +static char * speakerPlayName ="PC Speaker Playback Volume" ; +static char * monoPlayName ="Mono Playback Volume" ; +static char * monoRecName ="Mono Capture Volume" ; +static struct snd_kcontrol_new snd_es18xx_pcm1_controls[] = {ES18XX_DOUBLE ("PCM Playback Volume" ,0 ,0x14 ,0x14 ,4 ,0 ,15 ,0 ), };
@@ -1466,11 +1484,14 @@ }outb (0x40 , chip->port +0x04 ); +udelay (10 ); hi =inb (chip->port +0x05 ); +udelay (10 ); lo =inb (chip->port +0x05 );if (hi != lo) { chip->version = hi <<8 | lo; chip->ctrl_port =inb (chip->port +0x05 ) <<8 ; +udelay (10 ); chip->ctrl_port +=inb (chip->port +0x05 );if ((chip->res_ctrl_port =request_region (chip->ctrl_port,8 ,"ES18xx - CTRL" )) ==NULL ) {
@@ -1781,10 +1802,6 @@ } } -if (chip->caps &ES18XX_MONO ) { -if ((err =snd_ctl_add (card,snd_ctl_new1 (&snd_es18xx_mono_in_control, chip))) <0 ) -return err; - }if (chip->caps &ES18XX_RECMIX ) {for (idx =0 ; idx <ARRAY_SIZE (snd_es18xx_recmix_controls); idx++) {if ((err =snd_ctl_add (card,snd_ctl_new1 (&snd_es18xx_recmix_controls[idx], chip))) <0 )
@@ -1822,6 +1839,24 @@ } } +/* finish initializing other chipset specific controls +*/ +if (chip->version !=0x1868 ) { + snd_es18xx_optional_controls[reg3cMono].name = speakerPlayName; + } +switch (chip->version) { +case 0x1869 : + snd_es18xx_optional_controls[reg1cBit4].name = captureSwitchName; + snd_es18xx_optional_controls[reg6d].name = monoPlayName; + snd_es18xx_optional_controls[reg6f].name = monoRecName; +break ; + } +for (idx =0 ; idx <ARRAY_SIZE (snd_es18xx_optional_controls); idx++) { +if (snd_es18xx_optional_controls[idx].name ==UNSET ) +continue ; +if ((err =snd_ctl_add (card,snd_ctl_new1 (&snd_es18xx_optional_controls[idx], chip))) <0 ) +return err; + }return 0 ; }
Quoted 11 lines · Mark Salazar
Quoted 6 lines · Lee Revell
Quoted 1 line · Mark Salazar
First of 4
es18xx.cpatches culminating in Zoom Video support.All of these except the first are linewrapped and can't be applied.
I surrender. Unable to make Thunderbird's wrapping function submit to my will I just attach my patch message and slink away.
Wow is Thunderbird really such garbage that this is impossible? It would explain why we see so many line wrapped patches on LKML.
Someone should really follow up on this with the Mozilla developers.
Lee
-------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc. Do you grep through log files for problems? Stop! Download the new AJAX search engine that makes searching your log files as easy as surfing the web. DOWNLOAD SPLUNK!
http://ads.osdn.com/?ad_id=7637&alloc_id=16865&op=click
At Mon, 26 Dec 2005 12:36:14 -0500,
Quoted 9 lines · Mark Salazar
alsa-kernel/isa/es18xx.c renamed from alsa-driver/alsa-kernel/isa/es18xx.c +8
-0+/* +* These chipset specific mixer controls are initialized in +* snd_es18xx_mixer. Those controls which are not UNSET are then +* instantiated. +*/ +static char UNSETbuf ='\0' ; +#define UNSET ((char *)&UNSETbuf) +static enum optionalControlsIndex {reg1cBit4, reg3cMono, reg6d, reg6f};
alsa-kernel/isa/es18xx.c line 1235
Quoted 10 lines
alsa-kernel/isa/es18xx.c renamed from alsa-driver/alsa-kernel/isa/es18xx.c +10
-0+static struct snd_kcontrol_new snd_es18xx_optional_controls[] = { +ES18XX_SINGLE (UNSET ,0 ,0x1c ,4 ,1 ,1 ), +ES18XX_SINGLE (UNSET ,0 ,0x3c ,0 ,7 ,0 ), +ES18XX_DOUBLE (UNSET ,0 ,0x6d ,0x6d ,4 ,0 ,15 ,0 ), +ES18XX_DOUBLE (UNSET ,0 ,0x6f ,0x6f ,4 ,0 ,15 ,0 ) +}; +static char * captureSwitchName ="Capture Switch" ; +static char * speakerPlayName ="PC Speaker Playback Volume" ; +static char * monoPlayName ="Mono Playback Volume" ; +static char * monoRecName ="Mono Capture Volume" ;
This doesn't work for the multiple cards because you overwrite the static array. Better to create once kcontrol instaces via snd_ctl_new1(), then copy the name on each instance according to the model.
Other chnages look fine to me.
thanks,
Takashi
-------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc. Do you grep through log files for problems? Stop! Download the new AJAX search engine that makes searching your log files as easy as surfing the web. DOWNLOAD SPLUNK!
http://ads.osdn.com/?ad_id=7637&alloc_id=16865&op=click
Quoted 3 lines · Takashi Iwai
This doesn't work for the multiple cards because you overwrite the static array.
Yup. Good catch. Chalk up another one for peer review.
Quoted 5 lines
Better to create once kcontrol instaces via
snd_ctl_new1(), then copy the name on each instance according to the model.
Hmmm .. what if I just reset it to UNSET in the loop where it's used (as this patch does)?
New patch attached . Tested same as original version.
alsa-kernel/isa/es18xx.c renamed from alsa-driver/alsa-kernel/isa/es18xx.c +50 -13
@@ -1181,19 +1181,22 @@return change; } +/* Mixer controls +* These arrays contain setup data for mixer controls. +* +* The controls that are universal to all chipsets are fully initialized +* here. +*/ static struct snd_kcontrol_new snd_es18xx_base_controls[] = {ES18XX_DOUBLE ("Master Playback Volume" ,0 ,0x60 ,0x62 ,0 ,0 ,63 ,0 ),ES18XX_DOUBLE ("Master Playback Switch" ,0 ,0x60 ,0x62 ,6 ,6 ,1 ,1 ),ES18XX_DOUBLE ("Line Playback Volume" ,0 ,0x3e ,0x3e ,4 ,0 ,15 ,0 ),ES18XX_DOUBLE ("CD Playback Volume" ,0 ,0x38 ,0x38 ,4 ,0 ,15 ,0 ),ES18XX_DOUBLE ("FM Playback Volume" ,0 ,0x36 ,0x36 ,4 ,0 ,15 ,0 ), -ES18XX_DOUBLE ("Mono Playback Volume" ,0 ,0x6d ,0x6d ,4 ,0 ,15 ,0 ),ES18XX_DOUBLE ("Mic Playback Volume" ,0 ,0x1a ,0x1a ,4 ,0 ,15 ,0 ),ES18XX_DOUBLE ("Aux Playback Volume" ,0 ,0x3a ,0x3a ,4 ,0 ,15 ,0 ), -ES18XX_SINGLE ("PC Speaker Playback Volume" ,0 ,0x3c ,0 ,7 ,0 ),ES18XX_SINGLE ("Record Monitor" ,0 ,0xa8 ,3 ,1 ,0 ),ES18XX_DOUBLE ("Capture Volume" ,0 ,0xb4 ,0xb4 ,4 ,0 ,15 ,0 ), -ES18XX_SINGLE ("Capture Switch" ,0 ,0x1c ,4 ,1 ,1 ), { .iface =SNDRV_CTL_ELEM_IFACE_MIXER , .name ="Capture Source" ,
@@ -1203,19 +1206,34 @@ } }; -static struct snd_kcontrol_new snd_es18xx_mono_in_control = -ES18XX_DOUBLE ("Mono Input Playback Volume" ,0 ,0x6d ,0x6d ,4 ,0 ,15 ,0 ); -static struct snd_kcontrol_new snd_es18xx_recmix_controls[] = {ES18XX_DOUBLE ("PCM Capture Volume" ,0 ,0x69 ,0x69 ,4 ,0 ,15 ,0 ),ES18XX_DOUBLE ("Mic Capture Volume" ,0 ,0x68 ,0x68 ,4 ,0 ,15 ,0 ),ES18XX_DOUBLE ("Line Capture Volume" ,0 ,0x6e ,0x6e ,4 ,0 ,15 ,0 ),ES18XX_DOUBLE ("FM Capture Volume" ,0 ,0x6b ,0x6b ,4 ,0 ,15 ,0 ), -ES18XX_DOUBLE ("Mono Capture Volume" ,0 ,0x6f ,0x6f ,4 ,0 ,15 ,0 ),ES18XX_DOUBLE ("CD Capture Volume" ,0 ,0x6a ,0x6a ,4 ,0 ,15 ,0 ),ES18XX_DOUBLE ("Aux Capture Volume" ,0 ,0x6c ,0x6c ,4 ,0 ,15 ,0 ) }; +/* +* These chipset specific mixer controls are initialized in +* snd_es18xx_mixer. Those controls which are not UNSET are then +* instantiated. +*/ +static char UNSETbuf ='\0' ; +#define UNSET ((char *)&UNSETbuf) +static enum optionalControlsIndex {reg1cBit4, reg3cMono, reg6d, reg6f}; +static struct snd_kcontrol_new snd_es18xx_optional_controls[] = { +ES18XX_SINGLE (UNSET ,0 ,0x1c ,4 ,1 ,1 ), +ES18XX_SINGLE (UNSET ,0 ,0x3c ,0 ,7 ,0 ), +ES18XX_DOUBLE (UNSET ,0 ,0x6d ,0x6d ,4 ,0 ,15 ,0 ), +ES18XX_DOUBLE (UNSET ,0 ,0x6f ,0x6f ,4 ,0 ,15 ,0 ) +}; +static char * captureSwitchName ="Capture Switch" ; +static char * speakerPlayName ="PC Speaker Playback Volume" ; +static char * monoPlayName ="Mono Playback Volume" ; +static char * monoRecName ="Mono Capture Volume" ; +static struct snd_kcontrol_new snd_es18xx_pcm1_controls[] = {ES18XX_DOUBLE ("PCM Playback Volume" ,0 ,0x14 ,0x14 ,4 ,0 ,15 ,0 ), };
@@ -1466,11 +1484,14 @@ }outb (0x40 , chip->port +0x04 ); +udelay (10 ); hi =inb (chip->port +0x05 ); +udelay (10 ); lo =inb (chip->port +0x05 );if (hi != lo) { chip->version = hi <<8 | lo; chip->ctrl_port =inb (chip->port +0x05 ) <<8 ; +udelay (10 ); chip->ctrl_port +=inb (chip->port +0x05 );if ((chip->res_ctrl_port =request_region (chip->ctrl_port,8 ,"ES18xx - CTRL" )) ==NULL ) {
@@ -1744,7 +1765,7 @@static int __devinit snd_es18xx_mixer (struct snd_es18xx *chip) {struct snd_card *card; -int err; +int err, retVal;unsigned int idx; card = chip->card;
@@ -1781,10 +1802,6 @@ } } -if (chip->caps &ES18XX_MONO ) { -if ((err =snd_ctl_add (card,snd_ctl_new1 (&snd_es18xx_mono_in_control, chip))) <0 ) -return err; - }if (chip->caps &ES18XX_RECMIX ) {for (idx =0 ; idx <ARRAY_SIZE (snd_es18xx_recmix_controls); idx++) {if ((err =snd_ctl_add (card,snd_ctl_new1 (&snd_es18xx_recmix_controls[idx], chip))) <0 )
@@ -1822,7 +1839,27 @@ } } -return 0 ; +/* finish initializing other chipset specific controls +*/ +if (chip->version !=0x1868 ) { + snd_es18xx_optional_controls[reg3cMono].name = speakerPlayName; + } +switch (chip->version) { +case 0x1869 : + snd_es18xx_optional_controls[reg1cBit4].name = captureSwitchName; + snd_es18xx_optional_controls[reg6d].name = monoPlayName; + snd_es18xx_optional_controls[reg6f].name = monoRecName; +break ; + } + retVal =0 ; +for (idx =0 ; idx <ARRAY_SIZE (snd_es18xx_optional_controls); idx++) { +if (snd_es18xx_optional_controls[idx].name ==UNSET ) +continue ; +if ((err =snd_ctl_add (card,snd_ctl_new1 (&snd_es18xx_optional_controls[idx], chip))) <0 ) + retVal = err; + snd_es18xx_optional_controls[idx].name =UNSET ; + } +return retVal; }
At Sun, 08 Jan 2006 18:01:21 -0500,
Quoted 8 lines · Mark Salazar
Quoted 3 lines · Takashi Iwai
This doesn't work for the multiple cards because you overwrite the static array.
Yup. Good catch. Chalk up another one for peer review.
Quoted 7 lines
Quoted 5 lines
Better to create once kcontrol instaces via
snd_ctl_new1(), then copy the name on each instance according to the model.Hmmm .. what if I just reset it to UNSET in the loop where it's used (as this patch does)?
Well, I think the below is more straightforward.
Please let me know if it works, then I'll merge your patches to CVS.
Thanks,
Takashi
Index: alsa-kernel/isa/es18xx.c
===================================================================
RCS file: /home/iwai/cvs/alsa/alsa-kernel/isa/es18xx.c,v retrieving revision 1.58
isa/es18xx.c +38 -11
@@ -1191,19 +1191,22 @@return change; } +/* Mixer controls +* These arrays contain setup data for mixer controls. +* +* The controls that are universal to all chipsets are fully initialized +* here. +*/ static struct snd_kcontrol_new snd_es18xx_base_controls[] = {ES18XX_DOUBLE ("Master Playback Volume" ,0 ,0x60 ,0x62 ,0 ,0 ,63 ,0 ),ES18XX_DOUBLE ("Master Playback Switch" ,0 ,0x60 ,0x62 ,6 ,6 ,1 ,1 ),ES18XX_DOUBLE ("Line Playback Volume" ,0 ,0x3e ,0x3e ,4 ,0 ,15 ,0 ),ES18XX_DOUBLE ("CD Playback Volume" ,0 ,0x38 ,0x38 ,4 ,0 ,15 ,0 ),ES18XX_DOUBLE ("FM Playback Volume" ,0 ,0x36 ,0x36 ,4 ,0 ,15 ,0 ), -ES18XX_DOUBLE ("Mono Playback Volume" ,0 ,0x6d ,0x6d ,4 ,0 ,15 ,0 ),ES18XX_DOUBLE ("Mic Playback Volume" ,0 ,0x1a ,0x1a ,4 ,0 ,15 ,0 ),ES18XX_DOUBLE ("Aux Playback Volume" ,0 ,0x3a ,0x3a ,4 ,0 ,15 ,0 ), -ES18XX_SINGLE ("PC Speaker Playback Volume" ,0 ,0x3c ,0 ,7 ,0 ),ES18XX_SINGLE ("Record Monitor" ,0 ,0xa8 ,3 ,1 ,0 ),ES18XX_DOUBLE ("Capture Volume" ,0 ,0xb4 ,0xb4 ,4 ,0 ,15 ,0 ), -ES18XX_SINGLE ("Capture Switch" ,0 ,0x1c ,4 ,1 ,1 ), { .iface =SNDRV_CTL_ELEM_IFACE_MIXER , .name ="Capture Source" ,
@@ -1213,19 +1216,27 @@ } }; -static struct snd_kcontrol_new snd_es18xx_mono_in_control = -ES18XX_DOUBLE ("Mono Input Playback Volume" ,0 ,0x6d ,0x6d ,4 ,0 ,15 ,0 ); -static struct snd_kcontrol_new snd_es18xx_recmix_controls[] = {ES18XX_DOUBLE ("PCM Capture Volume" ,0 ,0x69 ,0x69 ,4 ,0 ,15 ,0 ),ES18XX_DOUBLE ("Mic Capture Volume" ,0 ,0x68 ,0x68 ,4 ,0 ,15 ,0 ),ES18XX_DOUBLE ("Line Capture Volume" ,0 ,0x6e ,0x6e ,4 ,0 ,15 ,0 ),ES18XX_DOUBLE ("FM Capture Volume" ,0 ,0x6b ,0x6b ,4 ,0 ,15 ,0 ), -ES18XX_DOUBLE ("Mono Capture Volume" ,0 ,0x6f ,0x6f ,4 ,0 ,15 ,0 ),ES18XX_DOUBLE ("CD Capture Volume" ,0 ,0x6a ,0x6a ,4 ,0 ,15 ,0 ),ES18XX_DOUBLE ("Aux Capture Volume" ,0 ,0x6c ,0x6c ,4 ,0 ,15 ,0 ) }; +/* +* The chipset specific mixer controls +*/ +static struct snd_kcontrol_new snd_es18xx_opt_speaker = +ES18XX_SINGLE ("PC Speaker Playback Volume" ,0 ,0x3c ,0 ,7 ,0 ); + +static struct snd_kcontrol_new snd_es18xx_opt_1869[] = { +ES18XX_SINGLE ("Capture Switch" ,0 ,0x1c ,4 ,1 ,1 ), +ES18XX_DOUBLE ("Mono Playback Volume" ,0 ,0x6d ,0x6d ,4 ,0 ,15 ,0 ), +ES18XX_DOUBLE ("Mono Capture Volume" ,0 ,0x6f ,0x6f ,4 ,0 ,15 ,0 ) +}; +static struct snd_kcontrol_new snd_es18xx_pcm1_controls[] = {ES18XX_DOUBLE ("PCM Playback Volume" ,0 ,0x14 ,0x14 ,4 ,0 ,15 ,0 ), };
@@ -1476,11 +1487,14 @@ }outb (0x40 , chip->port +0x04 ); +udelay (10 ); hi =inb (chip->port +0x05 ); +udelay (10 ); lo =inb (chip->port +0x05 );if (hi != lo) { chip->version = hi <<8 | lo; chip->ctrl_port =inb (chip->port +0x05 ) <<8 ; +udelay (10 ); chip->ctrl_port +=inb (chip->port +0x05 );if ((chip->res_ctrl_port =request_region (chip->ctrl_port,8 ,"ES18xx - CTRL" )) ==NULL ) {
@@ -1778,10 +1792,6 @@ } } -if (chip->caps &ES18XX_MONO ) { -if ((err =snd_ctl_add (card,snd_ctl_new1 (&snd_es18xx_mono_in_control, chip))) <0 ) -return err; - }if (chip->caps &ES18XX_RECMIX ) {for (idx =0 ; idx <ARRAY_SIZE (snd_es18xx_recmix_controls); idx++) {if ((err =snd_ctl_add (card,snd_ctl_new1 (&snd_es18xx_recmix_controls[idx], chip))) <0 )
@@ -1819,6 +1829,23 @@ } } +/* finish initializing other chipset specific controls +*/ +if (chip->version !=0x1868 ) { + err =snd_ctl_add (card,snd_ctl_new1 (&snd_es18xx_opt_speaker, + chip)); +if (err <0 ) +return err; + } +if (chip->version ==0x1869 ) { +for (idx =0 ; idx <ARRAY_SIZE (snd_es18xx_opt_1869); idx++) { + err =snd_ctl_add (card, +snd_ctl_new1 (&snd_es18xx_opt_1869[idx], + chip)); +if (err <0 ) +return err; + } + }return 0 ; }
-------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc. Do you grep through log files for problems? Stop! Download the new AJAX search engine that makes searching your log files as easy as surfing the web. DOWNLOAD SPLUNK!
http://ads.osdn.com/?ad_id=7637&alloc_id=16865&op=click
Quoted 5 lines · Takashi Iwai
Well, I think the below is more straightforward.
Please let me know if it works, then I'll merge your patches to CVS.
Haven't tried yet, but it looks like it should work. If we go this way it'll break my 4th patch though where I add the controls for Zoom Video so I'll have to rework that as well. I expect the modified version would look something like:
- adding a "Video Playback Switch" switch control to snd_es18xx_opt_1869;
- adding a "snd_es18xx_opt_1878" to hold a single volume control "Video Playback Volume";
- adding a "snd_es18xx_opt_1879" to hold stereo volume "Video Playback Volume", "Video Playback Volume", and the "Video Playback Switch" controls.
along with the necessary instantiation logic in snd_es18xx_mixer. The reason I used one array to hold all the chipset specific control information was because I saw the same registers used for different controls in different chipsets and I was trying to keep things simple and readable (and obvious swtich and single instantiation loop) in snd_es18xx_mixer. However this is my first time working ALSA code, and I will defer to your opinion. Which way do you want to go?
-------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc. Do you grep through log files for problems? Stop! Download the new AJAX search engine that makes searching your log files as easy as surfing the web. DOWNLOAD SPLUNK!
http://ads.osdn.com/?ad_id=7637&alloc_id=16865&op=click
Quoted 12 lines · Takashi Iwai
Basically, rewriting the static array and restoring it looks odd, so I prefer having multiple
kcontrol_newarrays. (Besides, you could use simply NULL for the validity check.) From the readability viewpoint, separate arrays are better, too. The most important point is that the code flow is straightforward.Takashi
OK, I've tried your version of mod1 and it seems to work fine.
Also retested the patches for mods 2 & 3 ... no changes necessary.
Revised mod4 patch to follow.
-------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc. Do you grep through log files for problems? Stop! Download the new AJAX search engine that makes searching your log files as easy as surfing the web. DOWNLOAD SPLUNK!
http://ads.osdn.com/?ad_id=7637&alloc_id=16865&op=click
At Sun, 15 Jan 2006 15:30:06 -0500,
Quoted 15 lines · Mark Salazar
Quoted 12 lines · Takashi Iwai
Basically, rewriting the static array and restoring it looks odd, so I prefer having multiple
kcontrol_newarrays. (Besides, you could use simply NULL for the validity check.) From the readability viewpoint, separate arrays are better, too. The most important point is that the code flow is straightforward.Takashi
Quoted 5 lines
OK, I've tried your version of mod1 and it seems to work fine.
Also retested the patches for mods 2 & 3 ... no changes necessary.
Revised mod4 patch to follow.
OK, I applied all 4 patches now. Thanks!
Takashi
-------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc. Do you grep through log files for problems? Stop! Download the new AJAX search engine that makes searching your log files as easy as surfing the web. DOWNLOAD SPLUNK!
http://ads.osdn.com/?ad_id=7637&alloc_id=16865&op=click