Calling function in interrupt hander

[pan069]

Doing a bit more work this weekend on my Sound Blaster project. I seem to have the DMA setup working [1] however, I seem to be running into a problem running on real hardware.

The IRQ seemed to be triggered by the SB at the correct intervals, however, in my IRQ handler I was calling a callback function that would update my audio buffer. All this seems to work OK within DOSBox but the moment I am running on real hardware (286/16 + SoundBlaster 2.0 [1350B]) I get weird results and crashes. When I do everything directly from the interrupt hander it seems to work fine. Is it not possible to call a function inside an interrupt handler?

My code looks something like this (stripping it down to make it less verbose):

volatile unsigned char _vchar = 'a';
volatile unsigned char _vcol = 0x1f;
volatile unsigned short _voffset = 0;

void on_sb_event()
{
  unsigned short far *v = (unsigned short far*)0xb8000000L;

  v[_voffset++] = ((_vcol << 8) | _vchar);

  if (_voffset > 80*25 - 1) {
    _voffset = 0;
    _vchar++;
    _vcol += 0x10;
  }
}

That is my callback handler that is called from the interrupt handler. I'm just updating the screen to get some visual feedback rather than using audio which can be less accurate to spot potential issues. The callback just starts filling the screen (text mode) first with 'a' characters (blue background, white foreground) and once the screen is full it restarts with 'b' characters and changes the background to green (+0x10), etc...

This is the actual interrupt handler:

void (* _on_event)(void);

void interrupt _sb_irq_handler()
{
  inp(0x220 + SB_IO_READ_BUFFER_STATUS);

  _on_event();

  outp(0x20, 0x20);
}

The above code acknowledges the transfer to the Sound Blaster by reading the status register. It then calls the callback and finally acknowledges the interrupt.

The IRQ handler is initialised as follows:

void sb_irq_capture(int sb_irq, void (* on_event)(void))
{
  _on_event = on_event;

  _sb_old_irq = _dos_getvect(8 + sb_irq);

  _dos_setvect(8 + sb_irq, _sb_irq_handler);

  outp(0x21, inp(0x21) & !(1 << sb_irq));
}

void sb_irq_release(int sb_irq)
{
  _dos_setvect(8 + sb_irq, _sb_old_irq);

  outp(0x21, inp(0x21) & (1 << sb_irq));
}

And in my main function somewhere:

sb_irq_capture(sb_irq, &on_sb_event);

Oh, I am using OpenWatcom. I am targeting 16bit DOS. These are my compiler options:

wcc -w2 -e25 -zq -2 -d0 -zp=2 -mc -bt=dos -fo=build/$@ src/$*.c -i$(WATCOM)/h

Any ideas welcome...

[1] https://forum.vcfed.org/index.php?threads/dma-addressing.1241044/

 

[Chuck(G)]

My first impression without reading your code too closely is "That's an awfully long ISR", which means that one or more higher priority interrupts are either pending or interrupting your code. How much stack space have you allocated for this? What happens if you temporarily shorten the "_on_event" code?

Usually, when faced with really long code to be written because of an interrupt, it's better to set a flag to be examined after the ISR returns, or better, stack a fake return address on the interrupt stack, such that the interrupt will return to the long servicing code, which, when complete, will return to the main interrupted routine.

The primary rule for ISRs is "keep it short".

Just the opinion of an old code spinner, mind you.

 

[kerravon]

Doing a bit more work this weekend on my Sound Blaster project. I seem to have the DMA setup working [1] however, I seem to be running into a problem running on real hardware.

The IRQ seemed to be triggered by the SB at the correct intervals, however, in my IRQ handler I was calling a callback function that would update my audio buffer. All this seems to work OK within DOSBox but the moment I am running on real hardware (286/16 + SoundBlaster 2.0 [1350B]) I get weird results and crashes. When I do everything directly from the interrupt hander it seems to work fine. Is it not possible to call a function inside an interrupt handler?

My code looks something like this (stripping it down to make it less verbose):

volatile unsigned char _vchar = 'a';
volatile unsigned char _vcol = 0x1f;
volatile unsigned short _voffset = 0;

void on_sb_event()
{
  unsigned short far *v = (unsigned short far*)0xb8000000L;

  v[_voffset++] = ((_vcol << 8) | _vchar);

  if (_voffset > 80*25 - 1) {
    _voffset = 0;
    _vchar++;
    _vcol += 0x10;
  }
}

That is my callback handler that is called from the interrupt handler. I'm just updating the screen to get some visual feedback rather than using audio which can be less accurate to spot potential issues. The callback just starts filling the screen (text mode) first with 'a' characters (blue background, white foreground) and once the screen is full it restarts with 'b' characters and changes the background to green (+0x10), etc...

This is the actual interrupt handler:

void (* _on_event)(void);

void interrupt _sb_irq_handler()
{
  inp(0x220 + SB_IO_READ_BUFFER_STATUS);

  _on_event();

  outp(0x20, 0x20);
}

The above code acknowledges the transfer to the Sound Blaster by reading the status register. It then calls the callback and finally acknowledges the interrupt.

The IRQ handler is initialised as follows:

void sb_irq_capture(int sb_irq, void (* on_event)(void))
{
  _on_event = on_event;

  _sb_old_irq = _dos_getvect(8 + sb_irq);

  _dos_setvect(8 + sb_irq, _sb_irq_handler);

  outp(0x21, inp(0x21) & !(1 << sb_irq));
}

void sb_irq_release(int sb_irq)
{
  _dos_setvect(8 + sb_irq, _sb_old_irq);

  outp(0x21, inp(0x21) & (1 << sb_irq));
}

And in my main function somewhere:

sb_irq_capture(sb_irq, &on_sb_event);

Oh, I am using OpenWatcom. I am targeting 16bit DOS. These are my compiler options:

wcc -w2 -e25 -zq -2 -d0 -zp=2 -mc -bt=dos -fo=build/$@ src/$*.c -i$(WATCOM)/h

Any ideas welcome...

[1] https://forum.vcfed.org/index.php?threads/dma-addressing.1241044/

I spent some time looking at your code, but couldn't see anything wrong with it.

I then compared it to some code that someone else gave me (which is CC0 so you can do whatever you want with it):

PDOS / Git Code / [ab9e04] /util/mspop.c

sourceforge.net

And the only thing I noticed was that the EOI he used:

outb(0x21 + sctx.irq, 0x20); /* Send EOI */

is very different from yours:

outp(0x20, 0x20);

However, things to note:

1. His interrupt isn't actually being used.

2. He says his code works on Dosbox, but it didn't work for me (there is a scratching sound instead of music) when I try to run it (I think I tried it on both emulated and real hardware) when I ran it as 32-bit under PDOS/386. I don't think I ever tried on PDOS/86 or Freedos.

So since you're interested in SB16 (I am too), if you see anything wrong with his/my (I made some minor changes) code, I'd appreciate it if you let me know about that too.

Thanks. Paul.

 

[reenigne]

I can't tell for sure without looking at the generated assembly, but my guess is that it's assuming that DS (and possibly also SS) points to the segment containing _vchar, _vcol and _voffset. This is true if the function is called from other C code but the interrupt could be interrupting some BIOS/DOS/TSR code which temporarily changes DS for its own purposes. This often happens when writing interrupt handlers in C - better to just write them (and everything that they call) in assembly language instead so you know exactly what it's doing. If you really must write it in C, you might have better luck compiling those routines using the huge memory model, which makes fewer assumptions.

 

[kerravon]

I can't tell for sure without looking at the generated assembly, but my guess is that it's assuming that DS (and possibly also SS) points to the segment containing _vchar, _vcol and _voffset. This is true if the function is called from other C code but the interrupt could be interrupting some BIOS/DOS/TSR code which temporarily changes DS for its own purposes. This often happens when writing interrupt handlers in C - better to just write them (and everything that they call) in assembly language instead so you know exactly what it's doing. If you really must write it in C, you might have better luck compiling those routines using the huge memory model, which makes fewer assumptions.

I think large is normally sufficient, right?

You only need huge if you're accessing a single data structure bigger than 64k?

 

[Ruud]

outp(0x20, 0x20);

This is the good one.

 

[reenigne]

At least for Turbo C (not sure if it's the same for Watcom) the huge memory model also relaxes the 64kB limit on static data, which might be the critical assumption here. Depending on what the generated code is actually doing, the large or even compact model might be sufficient.

 

[resman]

For fun, call on_sb_event() directly in your interrupt handler. This will remove the call through the function pointer, potentially simplifying the code. I don't generally like writing interrupt handlers in C (in DOS, anyway) because I'm not comfortable with the generated code. To that effect, I will always have the compiler spit out the generated assembly code for review on any tricky code. I don't see an option in OpenWATCOM to generate such a file in the 16 bit compiler (am I missing something?). I also wouldn't use the ignore warnings flag in such code, either.

To ChuckG's point, I don't think the code you posted in necessarily too big for an interrupt handler, but it looks overly complicated by throwing a function pointer in there. Unless that function pointer is being updated on the fly for nearly every IRQ, why not just update the actual IRQ vector and have a simple inline routine for the interrupt handler?

 

[pan069]

Hey guys! Thanks for the feedback, much appreciated.

As has been pointed out (thanks @reenigne) it's probably some memory model related thing, but since I'm just working on a prototype in C before moving on to assembler, its not worth my time digging further into it atm. If there was a quick fix answer I might have gone with that.

However, I'm going with @Chuck(G) suggestion, i.e. set a flag in the ISR and poll for the flag in my main loop and when set, update the buffer. This seems to work great.

@kerravon Sorry, I have not been able to look at that code in any meaningful way, but the way I understand the "out 20h, 20H" is that you acknowledge with the primary PIC that new IRQ can be triggered. If you were using IRQ's higher than 7 (8 to 15) theh you'd also have to acknowledge the secondary PIC by doing an "out a0h, 20h". Since I'm only targeting 8086/8088, my code only knows about port 20h.

Interrupts - OSDev Wiki

wiki.osdev.org

 

[kerravon]

To that effect, I will always have the compiler spit out the generated assembly code for review on any tricky code. I don't see an option in OpenWATCOM to generate such a file in the 16 bit compiler (am I missing something?).

You need to use "wdis" to disassemble the object code. By default it isn't reassemblable, but there is an option to make it so if required.