[alsa-devel] [PATCH] opti931: additional check for OPL3 device
From: Krzysztof Helt krzysztof.h1@wp.pl
This patch adds additional check for OPL3 device. I found that PNP region returned by the card is 0x380 - 0x38f but the OPL3 device is located at 0x388 (standard FM device address). The driver checks start of the range (0x380) which is incorrect.
Signed-off-by: Krzysztof Helt krzysztof.h1@wp.pl ---
The same problem appears on two cards I have with Opti chipset: opti931 and opti933. If someone can test if the opti92x PnP has the same problem I would be grateful.
Regards, Krzysztof
Krzysztof Helt wrote:
This patch adds additional check for OPL3 device. I found that PNP region returned by the card is 0x380 - 0x38f but the OPL3 device is located at 0x388 (standard FM device address).
On cards with an OPL4, it would use the entire range, where that last eight bytes are the OPL3-compatible registers.
AFAIK the opti93x chips have an integrated OPL3 and cannot be used with an OPL4.
If someone can test if the opti92x PnP has the same problem I would be grateful.
My card doesn't have PnP, but I guess the OPL4 detection wouldn't work on 92x either for the same reason.
Regards, Clemens
On Fri, 14 Sep 2007 16:52:47 +0200 "Clemens Ladisch" cladisch@fastmail.net wrote:
Krzysztof Helt wrote:
This patch adds additional check for OPL3 device. I found that PNP region returned by the card is 0x380 - 0x38f but the OPL3 device is located at 0x388 (standard FM device address).
On cards with an OPL4, it would use the entire range, where that last eight bytes are the OPL3-compatible registers.
Ok. I haven't touched the size of the range or the range itself. I just added a fallback to check range +8 (which is 0x388) after failure to find the OPL3 at the 0x380.
My card doesn't have PnP, but I guess the OPL4 detection wouldn't work on 92x either for the same reason.
Should the OPL4 detection has the same fallback or just all Opti cards should search the FM chip at the range + 8 address?
Regards, Krzysztof
Krzysztof Helt wrote:
"Clemens Ladisch" cladisch@fastmail.net wrote:
Krzysztof Helt wrote:
This patch adds additional check for OPL3 device. I found that PNP region returned by the card is 0x380 - 0x38f but the OPL3 device is located at 0x388 (standard FM device address).
On cards with an OPL4, it would use the entire range, where that last eight bytes are the OPL3-compatible registers.
Ok. I haven't touched the size of the range or the range itself. I just added a fallback to check range +8 (which is 0x388) after failure to find the OPL3 at the 0x380.
My card doesn't have PnP, but I guess the OPL4 detection wouldn't work on 92x either for the same reason.
Should the OPL4 detection has the same fallback or just all Opti cards should search the FM chip at the range + 8 address?
It appears a fallback shoudn't be needed because the chip always returns a range suitable for an OPL4.
Since an OPL3 range is never larger than 8 bytes, and OPL4 needs 16 bytes, we could add 8 to get the FM address _if_ the range is at least 16 bytes; thus we are safe even if a chip publishes an OPL3-only range.
Regards, Clemens
On 09/17/2007 10:14 AM, Clemens Ladisch wrote:
Krzysztof Helt wrote:
Should the OPL4 detection has the same fallback or just all Opti cards should search the FM chip at the range + 8 address?
It appears a fallback shoudn't be needed because the chip always returns a range suitable for an OPL4.
Since an OPL3 range is never larger than 8 bytes, and OPL4 needs 16 bytes, we could add 8 to get the FM address _if_ the range is at least 16 bytes; thus we are safe even if a chip publishes an OPL3-only range.
At least 16 won't do -- the OPTi chip provides a 12-byte range. More than 8 would work, but just +8 is fine. I have all OPTi ISA-PnP chips (924, 925, 931, 933) and more aren't going to be produced. The 0x380 base is what it calls the OPL4Base and ALBase (AdLib) is at 0x388.
The chip can defer to external FM it seems, but even if there are OPTi/OPL4 combo's out there in the wild (I sort of doubt it) the driver's just doing opl3 anyway. ie, still needs just the +8.
Rene.
diff --git a/sound/isa/opti9xx/opti92x-ad1848.c b/sound/isa/opti9xx/opti92x-ad1848.c index 60c120f..8bda47a 100644 --- a/sound/isa/opti9xx/opti92x-ad1848.c +++ b/sound/isa/opti9xx/opti92x-ad1848.c @@ -1732,11 +1732,11 @@ static int __devinit snd_card_opti9xx_pnp(struct snd_opti9xx *chip,
#ifdef OPTi93X port = pnp_port_start(pdev, 0) - 4; - fm_port = pnp_port_start(pdev, 1); + fm_port = pnp_port_start(pdev, 1) + 8; #else if (pid->driver_data != 0x0924) port = pnp_port_start(pdev, 1); - fm_port = pnp_port_start(pdev, 2); + fm_port = pnp_port_start(pdev, 2) + 8; #endif /* OPTi93X */ irq = pnp_irq(pdev, 0); dma1 = pnp_dma(pdev, 0);
Rene Herman wrote:
On 09/17/2007 10:14 AM, Clemens Ladisch wrote:
Krzysztof Helt wrote:
Should the OPL4 detection has the same fallback or just all Opti cards should search the FM chip at the range + 8 address?
It appears a fallback shoudn't be needed because the chip always returns a range suitable for an OPL4.
Since an OPL3 range is never larger than 8 bytes, and OPL4 needs 16 bytes, we could add 8 to get the FM address _if_ the range is at least 16 bytes; thus we are safe even if a chip publishes an OPL3-only range.
At least 16 won't do -- the OPTi chip provides a 12-byte range. More than 8 would work, but just +8 is fine. I have all OPTi ISA-PnP chips (924, 925, 931, 933) and more aren't going to be produced. The 0x380 base is what it calls the OPL4Base and ALBase (AdLib) is at 0x388.
Are there chips that have both OPL4Base and ALBase?
The chip can defer to external FM it seems, but even if there are OPTi/OPL4 combo's out there in the wild (I sort of doubt it)
Why?
the driver's just doing opl3 anyway. ie, still needs just the +8.
Only the 93x driver; the 92x driver has OPL4 support.
Regards, Clemens
On 09/17/2007 12:39 PM, Clemens Ladisch wrote:
Rene Herman wrote:
At least 16 won't do -- the OPTi chip provides a 12-byte range. More than 8 would work, but just +8 is fine. I have all OPTi ISA-PnP chips (924, 925, 931, 933) and more aren't going to be produced. The 0x380 base is what it calls the OPL4Base and ALBase (AdLib) is at 0x388.
Are there chips that have both OPL4Base and ALBase?
Not as seperate PnP resources no. Here are pnpdumps for the 4 PnP chips:
http://members.home.nl/rene.herman/opti/
(yes, OPTi 933 is OPT0931 same as OPTi 931)
The chip can defer to external FM it seems, but even if there are OPTi/OPL4 combo's out there in the wild (I sort of doubt it)
Why?
Because an OPL4 is fairly rarely seen as is and all OPTi (PnP) chips I have ever seen are on dirt cheap mass-market soundcards. It does not make a lot of sense from a business standpoint to put a cheap chip with integrated OPL3 on a card and then wire up an expensive OPL4 to it. Admittedly, some of early OPTi chips may have been somewhat less junky -- 931/3 are worst in that respect.
the driver's just doing opl3 anyway. ie, still needs just the +8.
Only the 93x driver; the 92x driver has OPL4 support.
You're right, somehow managed to miss that. According to the code, there's overlap with PnP at the 924 which is unfortunate since it means we can't just add 8 (unconditionally, nor when the area is larger than 8) to the PnP resource -- an OPL4 would be probed for at the wrong address.
How about this? This adds 8 to fm_port only if we're checking for an OPL3 at a 16-byte aligned address (such as the 0x380-0x3f0 from the PnP chips). Is still second guessing the user if (s)he supplied the value, but what can you do. Ack?
Rene.
diff --git a/sound/isa/opti9xx/opti92x-ad1848.c b/sound/isa/opti9xx/opti92x-ad1848.c index 60c120f..1f5af7b 100644 --- a/sound/isa/opti9xx/opti92x-ad1848.c +++ b/sound/isa/opti9xx/opti92x-ad1848.c @@ -1890,12 +1890,15 @@ static int __devinit snd_opti9xx_probe(struct snd_card *card) } } #endif /* !OPTi93X */ - if (!opl3 && snd_opl3_create(card, - chip->fm_port, - chip->fm_port + 2, - OPL3_HW_AUTO, 0, &opl3) < 0) { - snd_printk(KERN_WARNING "no OPL device at 0x%lx-0x%lx\n", - chip->fm_port, chip->fm_port + 4 - 1); + if (!opl3) { + if ((chip->fm_port & 15) == 0) + chip->fm_port += 8; + error = snd_opl3_create(card, chip->fm_port, + chip->fm_port + 2, + OPL3_HW_AUTO, 0, &opl3); + if (error < 0) + snd_printk(KERN_WARNING "no OPL device at 0x%lx-0x%lx\n", + chip->fm_port, chip->fm_port + 4 - 1); } if (opl3) { #ifdef CS4231
Guys,
Look at the code. Shifting the fm_port address by 8 (Rene's) is correct way to do as the OPL3 is searched at fm_port and fm_port +2, but the OPL4 is searched at fm_port and fm_port -8 (which will be the fm_port before shifting).
Is the OPL4 located at 0x380 and 0x388 or 0x378 and 0x380? If the former, the Rene's patch is correct.
Regards, Krzysztof
On 09/17/2007 02:26 PM, Krzysztof Helt wrote:
Look at the code. Shifting the fm_port address by 8 (Rene's) is correct way to do as the OPL3 is searched at fm_port and fm_port +2, but the OPL4 is searched at fm_port and fm_port -8 (which will be the fm_port before shifting).
Ah, damn, crap. Yes, forget the version I just sent. Original (attached) was correct. Thanks for paying attention.
Is the OPL4 located at 0x380 and 0x388 or 0x378 and 0x380? If the former, the Rene's patch is correct.
The OPL4 is at 0x380, with its OPL3 compatible part at 0x388. Could you add an Acked-by?
=== opti9xx: adjust OPL3 FM resource value
The OPTi ISA-PnP chips advertise their OPL4 base at 0x380 (to 0x3f0) through pnp and put their on-chip OPL3 at +8. The driver assumes the provided value is the ALBase (OPL3 address) though and checks for an OPL4 at -8, which means that simply adding 8 to the pnp provides value works to fix detection of both OPL3 and OPL4.
Problem spotted on 931 and 933 by Krzysztof Helt and confirmed on 924 and 925 (together all OPTi ISA-PnP chips) by me.
Signed-off-by; Rene Herman rene.herman@gmail.com ===
Rene.
diff --git a/sound/isa/opti9xx/opti92x-ad1848.c b/sound/isa/opti9xx/opti92x-ad1848.c index 60c120f..8bda47a 100644 --- a/sound/isa/opti9xx/opti92x-ad1848.c +++ b/sound/isa/opti9xx/opti92x-ad1848.c @@ -1732,11 +1732,11 @@ static int __devinit snd_card_opti9xx_pnp(struct snd_opti9xx *chip,
#ifdef OPTi93X port = pnp_port_start(pdev, 0) - 4; - fm_port = pnp_port_start(pdev, 1); + fm_port = pnp_port_start(pdev, 1) + 8; #else if (pid->driver_data != 0x0924) port = pnp_port_start(pdev, 1); - fm_port = pnp_port_start(pdev, 2); + fm_port = pnp_port_start(pdev, 2) + 8; #endif /* OPTi93X */ irq = pnp_irq(pdev, 0); dma1 = pnp_dma(pdev, 0);
On 09/17/2007 02:40 PM, Rene Herman wrote:
To possibly save you the trouble, yes, tested that the OPL3 address really follows along with the OPL4Base PnP resource.
Fixed description and CCed Takashi:
On 09/17/2007 02:26 PM, Krzysztof Helt wrote:
Look at the code. Shifting the fm_port address by 8 (Rene's) is correct way to do as the OPL3 is searched at fm_port and fm_port +2, but the OPL4 is searched at fm_port and fm_port -8 (which will be the fm_port before shifting).
Ah, damn, crap. Yes, forget the version I just sent. Original (attached) was correct. Thanks for paying attention.
Is the OPL4 located at 0x380 and 0x388 or 0x378 and 0x380? If the former, the Rene's patch is correct.
The OPL4 is at 0x380, with its OPL3 compatible part at 0x388. Could you add an Acked-by?
=== opti9xx: adjust OPL3 FM resource value
The OPTi ISA-PnP chips advertise their OPL4Base resource value at 0x380 (to 0x3f0) through PnP and put their on-chip OPL3 at ALBase = OPL4Base + 8. The driver assumes the PnP advertised value is the ALBase though and checks for an OPL4 at -8, which means that simply adding 8 to the PnP advertised value works to fix both OPL3 and OPL4 support.
Problem spotted on 931 and 933 by Krzysztof Helt and confirmed on 924 and 925 (together all OPTi ISA-PnP chips) by me. Has been tested.
Signed-off-by; Rene Herman rene.herman@gmail.com ===
Rene.
On Mon, 17 Sep 2007 14:40:51 +0200 Rene Herman rene.herman@gmail.com wrote:
The OPL4 is at 0x380, with its OPL3 compatible part at 0x388. Could you add an Acked-by?
You are inpatient. I have tested it on my opti933.
=== opti9xx: adjust OPL3 FM resource value
The OPTi ISA-PnP chips advertise their OPL4 base at 0x380 (to 0x3f0) through pnp and put their on-chip OPL3 at +8. The driver assumes the provided value is the ALBase (OPL3 address) though and checks for an OPL4 at -8, which means that simply adding 8 to the pnp provides value works to fix detection of both OPL3 and OPL4.
Problem spotted on 931 and 933 by Krzysztof Helt and confirmed on 924 and 925 (together all OPTi ISA-PnP chips) by me.
Signed-off-by; Rene Herman rene.herman@gmail.com
Acked-by: Krzysztof Helt krzysztof.h1@wp.pl
At Mon, 17 Sep 2007 16:45:55 +0200, Krzysztof Helt wrote:
On Mon, 17 Sep 2007 14:40:51 +0200 Rene Herman rene.herman@gmail.com wrote:
The OPL4 is at 0x380, with its OPL3 compatible part at 0x388. Could you add an Acked-by?
You are inpatient. I have tested it on my opti933.
=== opti9xx: adjust OPL3 FM resource value
The OPTi ISA-PnP chips advertise their OPL4 base at 0x380 (to 0x3f0) through pnp and put their on-chip OPL3 at +8. The driver assumes the provided value is the ALBase (OPL3 address) though and checks for an OPL4 at -8, which means that simply adding 8 to the pnp provides value works to fix detection of both OPL3 and OPL4.
Problem spotted on 931 and 933 by Krzysztof Helt and confirmed on 924 and 925 (together all OPTi ISA-PnP chips) by me.
Signed-off-by; Rene Herman rene.herman@gmail.com
Acked-by: Krzysztof Helt krzysztof.h1@wp.pl
Guys, let's relax. Now it's merged to HG tree.
thanks,
Takashi
On 09/17/2007 04:45 PM, Krzysztof Helt wrote:
On Mon, 17 Sep 2007 14:40:51 +0200 Rene Herman rene.herman@gmail.com wrote:
The OPL4 is at 0x380, with its OPL3 compatible part at 0x388. Could you add an Acked-by?
You are inpatient. I have tested it on my opti933.
Impatient? Didn't mean "immediately" or anything... just a request. Thanks.
Rene.
On 09/14/2007 04:35 PM, Krzysztof Helt wrote:
This patch adds additional check for OPL3 device. I found that PNP region returned by the card is 0x380 - 0x38f but the OPL3 device is located at 0x388 (standard FM device address). The driver checks start of the range (0x380) which is incorrect.
The same problem appears on two cards I have with Opti chipset: opti931 and opti933. If someone can test if the opti92x PnP has the same problem I would be grateful.
Yes, same for OPT0924/OPT0925 (928, 929 and 930 are Non-PnP).
I'd rather simply adjust the resource in snd_card_opti9xx_pnp though -- that way, you don't go second guessing a user-specified value.
Untested, but looks as though this should do it:
Rene.
diff --git a/sound/isa/opti9xx/opti92x-ad1848.c b/sound/isa/opti9xx/opti92x-ad1848.c index 60c120f..8bda47a 100644 --- a/sound/isa/opti9xx/opti92x-ad1848.c +++ b/sound/isa/opti9xx/opti92x-ad1848.c @@ -1732,11 +1732,11 @@ static int __devinit snd_card_opti9xx_pnp(struct snd_opti9xx *chip,
#ifdef OPTi93X port = pnp_port_start(pdev, 0) - 4; - fm_port = pnp_port_start(pdev, 1); + fm_port = pnp_port_start(pdev, 1) + 8; #else if (pid->driver_data != 0x0924) port = pnp_port_start(pdev, 1); - fm_port = pnp_port_start(pdev, 2); + fm_port = pnp_port_start(pdev, 2) + 8; #endif /* OPTi93X */ irq = pnp_irq(pdev, 0); dma1 = pnp_dma(pdev, 0);
participants (4)
-
Clemens Ladisch -
Krzysztof Helt -
Rene Herman -
Takashi Iwai