[alsa-devel] [PATCH] ess1688: fix OPL3 port setting
From: Krzysztof Helt krzysztof.h1@wp.pl
The ess1688 driver uses the same port for PCM audio (SB compatible) and OPL3 synthesis. It is wrong so allow to choose a correct port for OPL3 synthesis.
Signed-off-by: Krzysztof Helt krzysztof.h1@wp.pl --- The bug was entered as issue #282 and closed due to lack of response. However, the code is plainly wrong as MS Win95 driver reserves two separate IO areas starting from 0x220 and 0x388 for ess688 and the third 0x300 for ess1688.
I have no card to test as I cut my es1688 to reuse its audio jacks few months ago.
sound/isa/es1688/es1688.c | 20 +++++++++++++------- 1 files changed, 13 insertions(+), 7 deletions(-)
diff --git a/sound/isa/es1688/es1688.c b/sound/isa/es1688/es1688.c index b463771..5e4d766 100644 --- a/sound/isa/es1688/es1688.c +++ b/sound/isa/es1688/es1688.c @@ -49,6 +49,7 @@ static int index[SNDRV_CARDS] = SNDRV_DEFAULT_IDX; /* Index 0-MAX */ static char *id[SNDRV_CARDS] = SNDRV_DEFAULT_STR; /* ID for this card */ static int enable[SNDRV_CARDS] = SNDRV_DEFAULT_ENABLE; /* Enable this card */ static long port[SNDRV_CARDS] = SNDRV_DEFAULT_PORT; /* 0x220,0x240,0x260 */ +static long fm_port[SNDRV_CARDS] = SNDRV_DEFAULT_PORT; /* Usually 0x388 */ static long mpu_port[SNDRV_CARDS] = {[0 ... (SNDRV_CARDS - 1)] = -1}; static int irq[SNDRV_CARDS] = SNDRV_DEFAULT_IRQ; /* 5,7,9,10 */ static int mpu_irq[SNDRV_CARDS] = SNDRV_DEFAULT_IRQ; /* 5,7,9,10 */ @@ -65,6 +66,8 @@ MODULE_PARM_DESC(port, "Port # for " CRD_NAME " driver."); module_param_array(mpu_port, long, NULL, 0444); MODULE_PARM_DESC(mpu_port, "MPU-401 port # for " CRD_NAME " driver."); module_param_array(irq, int, NULL, 0444); +module_param_array(fm_port, long, NULL, 0444); +MODULE_PARM_DESC(fm_port, "FM port # for ES1688 driver."); MODULE_PARM_DESC(irq, "IRQ # for " CRD_NAME " driver."); module_param_array(mpu_irq, int, NULL, 0444); MODULE_PARM_DESC(mpu_irq, "MPU-401 IRQ # for " CRD_NAME " driver."); @@ -143,13 +146,16 @@ static int __devinit snd_es1688_probe(struct device *dev, unsigned int n) sprintf(card->longname, "%s at 0x%lx, irq %i, dma %i", pcm->name, chip->port, chip->irq, chip->dma8);
- if (snd_opl3_create(card, chip->port, chip->port + 2, - OPL3_HW_OPL3, 0, &opl3) < 0) - dev_warn(dev, "opl3 not detected at 0x%lx\n", chip->port); - else { - error = snd_opl3_hwdep_new(opl3, 0, 1, NULL); - if (error < 0) - goto out; + if (fm_port[n] > 0 && fm_port[n] != SNDRV_AUTO_PORT) { + if (snd_opl3_create(card, fm_port[n], fm_port[n] + 2, + OPL3_HW_OPL3, 0, &opl3) < 0) + dev_warn(dev, + "opl3 not detected at 0x%lx\n", fm_port[n]); + else { + error = snd_opl3_hwdep_new(opl3, 0, 1, NULL); + if (error < 0) + goto out; + } }
if (mpu_irq[n] >= 0 && mpu_irq[n] != SNDRV_AUTO_IRQ &&
On 25-01-09 21:10, Krzysztof Helt wrote:
From: Krzysztof Helt krzysztof.h1@wp.pl
The ess1688 driver uses the same port for PCM audio (SB compatible) and OPL3 synthesis. It is wrong so allow to choose a correct port for OPL3 synthesis.
Only a very quick reply -- generally, the low 2 SB ports are indeed OPL3 compatible (or was it just OPL2? ...)
Rene.
At Sun, 25 Jan 2009 23:05:55 +0100, Rene Herman wrote:
On 25-01-09 21:10, Krzysztof Helt wrote:
From: Krzysztof Helt krzysztof.h1@wp.pl
The ess1688 driver uses the same port for PCM audio (SB compatible) and OPL3 synthesis. It is wrong so allow to choose a correct port for OPL3 synthesis.
Only a very quick reply -- generally, the low 2 SB ports are indeed OPL3 compatible (or was it just OPL2? ...)
All devices have separate OPL3 I/O ports. So there must be a reason...
Takashi
On Mon, 26 Jan 2009 10:42:29 +0100 Takashi Iwai tiwai@suse.de wrote:
At Sun, 25 Jan 2009 23:05:55 +0100, Rene Herman wrote:
On 25-01-09 21:10, Krzysztof Helt wrote:
From: Krzysztof Helt krzysztof.h1@wp.pl
The ess1688 driver uses the same port for PCM audio (SB compatible) and OPL3 synthesis. It is wrong so allow to choose a correct port for OPL3 synthesis.
Only a very quick reply -- generally, the low 2 SB ports are indeed OPL3 compatible (or was it just OPL2? ...)
All devices have separate OPL3 I/O ports. So there must be a reason...
The ESS 688 datasheet states that FM chip select signal (FMCSB) is activated when one of the port ranges: 388H-389H, 2x8H-2x9H, 2x0H-2x3H is selected. The current code may work on on the ESS 688. However, it obviously does not work with the ESS 1688 (bug #282). The bug report states that the OPL3 synthesis works with the OSS driver which chooses the 0x388 port for the OPL3.
My patch adds a new port setting so it is still possible to set the fm_port value the same as the port value. No change here. However, it allows setting different values to the port and the fm_port setting if needed.
Takashi, please consider applying my patch to the alsa tree.
Regards, Krzysztof
---------------------------------------------------------------------- Speak Up. Angielski szybko i skutecznie. 3 miesiace nauki gratis. Sprawdz. >> http://link.interia.pl/f2019
At Thu, 29 Jan 2009 15:55:27 +0100, Krzysztof Helt wrote:
On Mon, 26 Jan 2009 10:42:29 +0100 Takashi Iwai tiwai@suse.de wrote:
At Sun, 25 Jan 2009 23:05:55 +0100, Rene Herman wrote:
On 25-01-09 21:10, Krzysztof Helt wrote:
From: Krzysztof Helt krzysztof.h1@wp.pl
The ess1688 driver uses the same port for PCM audio (SB compatible) and OPL3 synthesis. It is wrong so allow to choose a correct port for OPL3 synthesis.
Only a very quick reply -- generally, the low 2 SB ports are indeed OPL3 compatible (or was it just OPL2? ...)
All devices have separate OPL3 I/O ports. So there must be a reason...
The ESS 688 datasheet states that FM chip select signal (FMCSB) is activated when one of the port ranges: 388H-389H, 2x8H-2x9H, 2x0H-2x3H is selected. The current code may work on on the ESS 688. However, it obviously does not work with the ESS 1688 (bug #282). The bug report states that the OPL3 synthesis works with the OSS driver which chooses the 0x388 port for the OPL3.
My patch adds a new port setting so it is still possible to set the fm_port value the same as the port value. No change here. However, it allows setting different values to the port and the fm_port setting if needed.
Fair enough. However...
Takashi, please consider applying my patch to the alsa tree.
One thing I noticed in your patch is that opl3 won't be created after your change as default, as fmport=SNDRV_AUTO_PORT. This is a regression. The code should be like below instead:
if (fm_port[n] == SNDRV_AUTO_PORT) fm_port[n] = chip->port; /* share the same port */
if (fm_port[n] > 0) { if (snd_opl3_create(card, fm_port[n], fm_port[n] + 2, OPL3_HW_OPL3, 0, &opl3) < 0) ...
Could you fix and repost?
thanks,
Takashi
From: Krzysztof Helt krzysztof.h1@wp.pl
The ess1688 driver uses the same port for PCM audio (SB compatible) and OPL3 synthesis. It is not always right so allow to choose a different port for OPL3 synthesis.
Signed-off-by: Krzysztof Helt krzysztof.h1@wp.pl --- Changed fm_port behaviour after Takashi's request.
diff --git a/sound/isa/es1688/es1688.c b/sound/isa/es1688/es1688.c index b463771..e7fbb06 100644 --- a/sound/isa/es1688/es1688.c +++ b/sound/isa/es1688/es1688.c @@ -49,6 +49,7 @@ static int index[SNDRV_CARDS] = SNDRV_DEFAULT_IDX; /* Index 0-MAX */ static char *id[SNDRV_CARDS] = SNDRV_DEFAULT_STR; /* ID for this card */ static int enable[SNDRV_CARDS] = SNDRV_DEFAULT_ENABLE; /* Enable this card */ static long port[SNDRV_CARDS] = SNDRV_DEFAULT_PORT; /* 0x220,0x240,0x260 */ +static long fm_port[SNDRV_CARDS] = SNDRV_DEFAULT_PORT; /* Usually 0x388 */ static long mpu_port[SNDRV_CARDS] = {[0 ... (SNDRV_CARDS - 1)] = -1}; static int irq[SNDRV_CARDS] = SNDRV_DEFAULT_IRQ; /* 5,7,9,10 */ static int mpu_irq[SNDRV_CARDS] = SNDRV_DEFAULT_IRQ; /* 5,7,9,10 */ @@ -65,6 +66,8 @@ MODULE_PARM_DESC(port, "Port # for " CRD_NAME " driver."); module_param_array(mpu_port, long, NULL, 0444); MODULE_PARM_DESC(mpu_port, "MPU-401 port # for " CRD_NAME " driver."); module_param_array(irq, int, NULL, 0444); +module_param_array(fm_port, long, NULL, 0444); +MODULE_PARM_DESC(fm_port, "FM port # for ES1688 driver."); MODULE_PARM_DESC(irq, "IRQ # for " CRD_NAME " driver."); module_param_array(mpu_irq, int, NULL, 0444); MODULE_PARM_DESC(mpu_irq, "MPU-401 IRQ # for " CRD_NAME " driver."); @@ -143,13 +146,19 @@ static int __devinit snd_es1688_probe(struct device *dev, unsigned int n) sprintf(card->longname, "%s at 0x%lx, irq %i, dma %i", pcm->name, chip->port, chip->irq, chip->dma8);
- if (snd_opl3_create(card, chip->port, chip->port + 2, - OPL3_HW_OPL3, 0, &opl3) < 0) - dev_warn(dev, "opl3 not detected at 0x%lx\n", chip->port); - else { - error = snd_opl3_hwdep_new(opl3, 0, 1, NULL); - if (error < 0) - goto out; + if (fm_port[n] != SNDRV_AUTO_PORT) + fm_port[n] = port[n]; /* share the same port */ + + if (fm_port[n] > 0) { + if (snd_opl3_create(card, fm_port[n], fm_port[n] + 2, + OPL3_HW_OPL3, 0, &opl3) < 0) + dev_warn(dev, + "opl3 not detected at 0x%lx\n", fm_port[n]); + else { + error = snd_opl3_hwdep_new(opl3, 0, 1, NULL); + if (error < 0) + goto out; + } }
if (mpu_irq[n] >= 0 && mpu_irq[n] != SNDRV_AUTO_IRQ &&
----------------------------------------------------------------------- Promocja w Speak Up. Kwartal angielskiego za darmo. 3 miesiace nauki gratis. Sprawdz teraz! >> http://link.interia.pl/f2019
At Fri, 30 Jan 2009 09:55:28 +0100, Krzysztof Helt wrote:
@@ -143,13 +146,19 @@ static int __devinit snd_es1688_probe(struct device *dev, unsigned int n) sprintf(card->longname, "%s at 0x%lx, irq %i, dma %i", pcm->name, chip->port, chip->irq, chip->dma8);
- if (snd_opl3_create(card, chip->port, chip->port + 2,
OPL3_HW_OPL3, 0, &opl3) < 0)dev_warn(dev, "opl3 not detected at 0x%lx\n", chip->port);- else {
error = snd_opl3_hwdep_new(opl3, 0, 1, NULL);if (error < 0)goto out;
- if (fm_port[n] != SNDRV_AUTO_PORT)
fm_port[n] = port[n]; /* share the same port */
It should be
if (fm_port[n] == SNDRV_AUTO_PORT)
Takashi
From: Krzysztof Helt krzysztof.h1@wp.pl
The ess1688 driver uses the same port for PCM audio (SB compatible) and OPL3 synthesis. It is not always right so allow to choose a different port for OPL3 synthesis.
Signed-off-by: Krzysztof Helt krzysztof.h1@wp.pl --- I am sorry for stupid copy-paste error. It is fixed now.
diff --git a/sound/isa/es1688/es1688.c b/sound/isa/es1688/es1688.c index b463771..e7fbb06 100644 --- a/sound/isa/es1688/es1688.c +++ b/sound/isa/es1688/es1688.c @@ -49,6 +49,7 @@ static int index[SNDRV_CARDS] = SNDRV_DEFAULT_IDX; /* Index 0-MAX */ static char *id[SNDRV_CARDS] = SNDRV_DEFAULT_STR; /* ID for this card */ static int enable[SNDRV_CARDS] = SNDRV_DEFAULT_ENABLE; /* Enable this card */ static long port[SNDRV_CARDS] = SNDRV_DEFAULT_PORT; /* 0x220,0x240,0x260 */ +static long fm_port[SNDRV_CARDS] = SNDRV_DEFAULT_PORT; /* Usually 0x388 */ static long mpu_port[SNDRV_CARDS] = {[0 ... (SNDRV_CARDS - 1)] = -1}; static int irq[SNDRV_CARDS] = SNDRV_DEFAULT_IRQ; /* 5,7,9,10 */ static int mpu_irq[SNDRV_CARDS] = SNDRV_DEFAULT_IRQ; /* 5,7,9,10 */ @@ -65,6 +66,8 @@ MODULE_PARM_DESC(port, "Port # for " CRD_NAME " driver."); module_param_array(mpu_port, long, NULL, 0444); MODULE_PARM_DESC(mpu_port, "MPU-401 port # for " CRD_NAME " driver."); module_param_array(irq, int, NULL, 0444); +module_param_array(fm_port, long, NULL, 0444); +MODULE_PARM_DESC(fm_port, "FM port # for ES1688 driver."); MODULE_PARM_DESC(irq, "IRQ # for " CRD_NAME " driver."); module_param_array(mpu_irq, int, NULL, 0444); MODULE_PARM_DESC(mpu_irq, "MPU-401 IRQ # for " CRD_NAME " driver."); @@ -143,13 +146,19 @@ static int __devinit snd_es1688_probe(struct device *dev, unsigned int n) sprintf(card->longname, "%s at 0x%lx, irq %i, dma %i", pcm->name, chip->port, chip->irq, chip->dma8);
- if (snd_opl3_create(card, chip->port, chip->port + 2, - OPL3_HW_OPL3, 0, &opl3) < 0) - dev_warn(dev, "opl3 not detected at 0x%lx\n", chip->port); - else { - error = snd_opl3_hwdep_new(opl3, 0, 1, NULL); - if (error < 0) - goto out; + if (fm_port[n] == SNDRV_AUTO_PORT) + fm_port[n] = port[n]; /* share the same port */ + + if (fm_port[n] > 0) { + if (snd_opl3_create(card, fm_port[n], fm_port[n] + 2, + OPL3_HW_OPL3, 0, &opl3) < 0) + dev_warn(dev, + "opl3 not detected at 0x%lx\n", fm_port[n]); + else { + error = snd_opl3_hwdep_new(opl3, 0, 1, NULL); + if (error < 0) + goto out; + } }
if (mpu_irq[n] >= 0 && mpu_irq[n] != SNDRV_AUTO_IRQ &&
----------------------------------------------------------------------- Promocja w Speak Up. Kwartal angielskiego za darmo. 3 miesiace nauki gratis. Sprawdz teraz! >> http://link.interia.pl/f2019
At Fri, 30 Jan 2009 19:20:29 +0100, Krzysztof Helt wrote:
From: Krzysztof Helt krzysztof.h1@wp.pl
The ess1688 driver uses the same port for PCM audio (SB compatible) and OPL3 synthesis. It is not always right so allow to choose a different port for OPL3 synthesis.
Signed-off-by: Krzysztof Helt krzysztof.h1@wp.pl
Applied now. Thanks!
Takashi
participants (3)
-
Krzysztof Helt -
Rene Herman -
Takashi Iwai