Hi just seeing that in MIXER sources (own soundcards drivers) uses DISABLE()/ENABLE() function want to know if it's possible (and safe) to change'em with a mutex/semaphore or just remove from code as they are not important/usefull.
Disable/Enable are on 3 _ctrl.c sources: emu10kx_ctrl.c, fm801_ctrl.c and sb128_ctrl.c
On emu10kx_ctrl.c, fm801_ctrl.c they are commented out (by me, since V1.39 or alike) and seems ok, as nobody complained "system freezes, doesn't work for me now,....):
void codec_write(unsigned short reg, unsigned short val)
{
int i;
//IExec->Disable();
for(i = 0; i < 10000; i++)
if( !(dev->InWord(iobase + FM801_AC97_CMD) & (1<<9)) )
goto ok1;
#ifdef _DEBUG_
IExec->DebugPrintF("Couldn't write to ac97!\n");
#endif
ok1:
dev->OutWord(iobase + FM801_AC97_DATA, val);
dev->OutWord(iobase + FM801_AC97_CMD, reg);
//IExec->Enable();
}
void codec_write(unsigned short reg, unsigned short val)
{
//Disable();
if(Audigy == FALSE)
{
dev->OutByte(iobase + AC97ADDRESS, reg);
dev->OutWord(iobase + AC97DATA, val);
}
else
emu10k1_set_volume_gpr(reg, val);
//Enable();
}Is it ok to just comment out? Or is better coding to change'em to use Mutex/Semaphore?
TIA
@jabirulo
Well I am not really into the Mixer source code, anyway.
This reading can be of help (Disabling Tasks)
http://wiki.amigaos.net/wiki/Exec_Tasks
Basically disabling is required when a task accesses structures that are shared by interrupt code.
Thus the fact that no one complained after you commented those statements out doesn' t necessarily mean that it is garanteed to always work well.
@jabirulo
I'm pretty sure you should not comment out those calls. The point is to sure that the AHI driver's interrupt code (or any other code for that matter) doesn't interfere while Mixer is writing to the audio h/w. While disabling all interrupts is overkill it's probably the only way to do this in a safe manner from a program that doesn't know anything about the inner workings of AHI or its drivers.
Ideally the mixer functionality should be done inside AHI and it's drivers and Mixer would simply be a program making use of this.
@jabirulo
Mutexes and Semaphores only make sense when all the involved tasks use the same locking convention accessing shared data and this is not the case.
Be also aware that disabling interrupts is not a good practice in a multitasking environment like AmigaOS4, but it is probably the only possible way for your purposes, as salass00 said.
I would then restore those commented statements.
@all
THX for explanations, will restore such Disable/Enable functions in next MIXER update.
AOS4.1/SAM460ex/PPC460EX-1155MHZ/2048MB/RadeonHD6570/SSD120GB/DVDRW :-P
I wonder if another alternate is to use a soft interrupt routine instead. This would work above a Forbid() level by running higher than a task and the code would run without interruption. :-)
However, it does have a priority so could be below the desire Disable() level.
Syill, it's would avoid disabling multitasking and common OS like DOS and Linux used this type of function call that called a kernel routine interrupt vector.
This wouldn't help since interrupts can still be interrupted by higher level interrupts which in the case of software interrupts would be all h/w interrupts.
You could use Disable() but then you would be where you were before but with the added problem that if you do anything that causes a DSI or ISI inside a software interrupt it will cause the whole OS to crash rather than just your one program.
Good point. Just when I thought that sort of thing would only happen on actual Amiga hardware when considering the original Amiga interrupts.
Still, for other code that needs protection, it could be an alternate to disabling multitasking. If a semaphore or mutex wasn't suitable.
@hypex
Overall disabling multitasking in a multitasking environment is not an advisable practice either.
Semaphores / mutexes only make sense when the involved concurrent "parts" agree on the same locking policy.
@jabirulo
There are specific instances where enable()/disable() should be used. I can't tell for sure but the "reg" variable makes it look like some of the code you mention is read/writing system or card registers. Reading/writing to registers should usually be done as an atomic operation (it appears to the rest of the system to occur instantaneously).
In this code:
dev->OutByte(iobase + AC97ADDRESS, reg);
dev->OutWord(iobase + AC97DATA, val);
it looks like a data direction register is written to; followed by a write to the data register. If that operation is interrupted before completion the results are unpredictable and could lead to failure.
On the other hand this code:
for(i = 0; i < 10000; i++)
if( !(dev->InWord(iobase + FM801_AC97_CMD) & (1<<9)) )
goto ok1;
#ifdef _DEBUG_
IExec->DebugPrintF("Couldn't write to ac97!\n");
#endif
is potentially doing 10000 loop iterations and possibly debug output while interrupts are disabled which seems like a bad idea to me. I'm not an expert but in that case, I would only use disable()/enable() around the write operation below that code. I think you might be better off consulting someone who has experience accessing hardware registers like Lyle Hazelwood.
X1000 - OS 4.1FE
You are right on target.
Most audio cards appear in PCI space.
The drivers usually set up a hardware interrupt that gets kicked at the end of each buffer.
That interrupt calls driver code, which calls AHI for "more audio" and refills the buffer.
There are no provisions made for sharing the audio card with other programs, like Mixer.
So the largest part of executable code is driven from hard interrupts.
Since there is no provision for mutual exclusion made in each audio driver, there's no way I can think of to prevent collisions.
Some of this may change for newer devices that don't generate hard interrupts to refill their buffers, like USB audio devices or HDAudio devices, which don't appear in PCI space directly. But that doesn't solve todays question.
If we could do it all over again, I can think of a few things to do differently. But then it wouldn't be AHI, would it?
LyleHaze