Forum: Mikrocontroller und Digitale Elektronik IIR Implementation: C-Code verschnellern


von M. W. (rallini94)


Lesenswert?

Hallo Leute,

ich hatte letztes Semester in meinen Vorlesungen unter anderem das Thema 
"Digitale Filter". Nun wollte ich damit ein wenig experementieren. Der 
Filterentwurf mittels Matlab und auch das Testen mit Matlab funktioniert 
reibungslos.
Nun habe ich versucht, meine Filter (ein TP, ein HP und ein BP sind 
angedacht) in einen Atmega328 zu quetschen. Platzprobleme gibt es keine, 
jedoch ist mein Code trotz all meiner Bemühungen zu langsam. Ich habe 
nun schon versucht alles zu optimieren, soweit meine Kenntnisse das zu 
lassen. Also
- Speicherzugriffe vermeiden
- aufwändige Schleifen weggeschmissen
- Integer Rechnungen statt floating point
- Filterordnung so klein wie möglich

Das Resultat ist folgende ISR, die mit 15 kHz = Abtastfrequenz 
aufgerufen wird.
1
ISR(ADC_vect)
2
{
3
  static uint8_t    ADChigh = 0;
4
  static uint8_t    ADClow = 0;
5
  
6
  static int32_t    wnTP = 0;
7
  static int32_t    wn1TP = 0;
8
  static int32_t    wn2TP = 0;
9
  static int32_t    TPout = 0;
10
  
11
  static int32_t    wnHP = 0;
12
  static int32_t    wn1HP = 0;
13
  static int32_t    wn2HP = 0;
14
  static int32_t    HPout = 0;
15
  
16
17
  TIFR1  |= 0xFF;   // flags des Counters loeschen
18
  
19
  ADClow = ADCL;
20
  ADChigh = ADCH;
21
  
22
  ADCwert = (ADChigh<<8) | (0xFFFF & ADClow);    
23
  ADCwert -= 511;    // Mittelwert entfernen
24
  
25
26
  // TP filtern    
27
  wnTP = ADCwert + ((3746 * wn1TP - 1726 * wn2TP)/2048);    
28
  TPout = (wnTP + (wn1TP*2) + wn2TP);
29
  TPout = TPout/1024;
30
      
31
  wn2TP = wn1TP;
32
  wn1TP = wnTP;
33
  
34
  // HP filtern
35
  wnHP = ADCwert - ((804 * wn1HP + 408 * wn2HP)/2048);
36
  HPout = ((wnHP - (wn1HP*2) + wn2HP));
37
  HPout = HPout/16;
38
  
39
  wn2HP = wn1HP;
40
  wn1HP = wnHP;
41
}

Wie man sieht, sind die Filter in der direct form II implementiert. Der 
HP und der TP passen auch zeitlich in die ISR rein, der BP jedoch nicht 
deswegen steht dieser auch noch nicht im Code.

Die Code-Gurus unter euch kennen doch sicherlich noch ein paar Tricks, 
den ein oder anderen Taktzyklus zu sparen oder ?

Vielen Dank und liebe Grüße,
M.W.

PS: Ich arbeiten mit dem AtmelStudio und der Optimizer ist auf -O2 
(Speed) eingestellt.

von Frank M. (ukw) (Moderator) Benutzerseite


Lesenswert?

Wie oft ist auch dieser Codeschnipsel unvollständig.

Welchen Typ hat ADCwert? Wahrscheinlich volatile int32_t. Benutze eine 
lokale Variable tmp_ADCwert und setze ADCwert erst am Schluss aller 
Berechnungen - wenn Du sie dann überhaupt noch brauchst.

int32-Berechnungen sind auf einem AVR halt aufwendig. Versuche, den 
Wertebereich einzugrenzen und zu prüfen, ob Du evtl. mit int16 
auskommst.

Wenn nicht, wechsle den µC. Wenn ich 32-Bit benötige, würde ich auch 
einen 32-Bit-µC verwenden.

: Bearbeitet durch Moderator
von m.n. (Gast)


Lesenswert?

Man kann Einiges optimieren, aber letztlich wird der AVR etwas langsam 
bleiben.
Beispiel:

M. W. schrieb:
> ADClow = ADCL;
>   ADChigh = ADCH;
>
>   ADCwert = (ADChigh<<8) | (0xFFFF & ADClow);
>   ADCwert -= 511;    // Mittelwert entfernen

schreibt man besser als: ADCwert = ADC-511;

oder
> TPout = (wnTP + (wn1TP*2) + wn2TP);

zumindest als: TPout = wnTP + wn1TP + wn1TP + wn2TP;
obwohl TPout wohl nirgends verwendet wird?

von Falk B. (falk)


Lesenswert?

Verschnellern? Ist das sowas wie hochskillen?

von Falk B. (falk)


Lesenswert?

@  M. W. (rallini94)

>- Speicherzugriffe vermeiden
>- aufwändige Schleifen weggeschmissen
>- Integer Rechnungen statt floating point
>- Filterordnung so klein wie möglich

ISR(ADC_vect)
{
> static uint8_t    ADChigh = 0;
> static uint8_t    ADClow = 0;

Brauchst du nicht, der avr gcc kann das mit einem Rutsch richtig 
auslesen.

> static int32_t    wnTP = 0;
> static int32_t    wn1TP = 0;
> static int32_t    wn2TP = 0;
> static int32_t    TPout = 0;

32 Bit Arithmetik ist nicht die Stärke des AVR und auch nicht im 
Zusammenhang mit dem avr gcc, das ist etwas stiefmütterlich umgesetzt.


  static int32_t    wnHP = 0;
  static int32_t    wn1HP = 0;
  static int32_t    wn2HP = 0;
  static int32_t    HPout = 0;


> TIFR1  |= 0xFF;   // flags des Counters loeschen

Musst du nicht, das macht die Hardware allein.

> ADClow = ADCL;
> ADChigh = ADCH;
> ADCwert = (ADChigh<<8) | (0xFFFF & ADClow);

Mist. Das geht einfach so.

  ADCwert = ADC -511; // Mittelwert entfernen


>  // TP filtern
>  wnTP = ADCwert + ((3746  wn1TP - 1726  wn2TP)/2048);

Hier würde ich im .lss File prüfen, ob der avr gcc schlau genug war, das 
als Bitschiebung umzusetzen. Mit etwas Pech ist das eine echte Division.

>  TPout = (wnTP + (wn1TP*2) + wn2TP);
>  TPout = TPout/1024;

Hier das Gleiche.

von Peter D. (peda)


Lesenswert?

Was soll die static Orgie?
Damit zwingst Du den Compiler, die Variablen im RAM anzulegen.
Static macht man nur die Variablen, die bis zum nächsten Interrupt 
überleben müssen.
Und muß es unbedingt signed sein?
Bei unsigned kann der Compiler besser optimieren, z.B. teure Divisionen 
/2^n durch Schieben ersetzen.
Den DC-Offset kann man bequem nach dem TP-Filter abziehen (HP, BP 
schmeißen ihn ja selber raus).

von Sascha (Gast)


Lesenswert?

Da war deine Matlab-Arbeit unvollständig. Man macht das Filter natürlich 
so, dass die Koeffizienten Zweierpotenzen sind, dann sind das nur noch 
Bitshifts.

Weiterhin teilt man die Koeffizienten so, dass die Ergebnisse der 
Multiplikationen in 24 Bit passen, ergibt 16*16=24 Bit MULs. Die sind 
schneller.

Und dann kann man das Filter noch symmetrisch machen und in der Mitte 
zusammenklappen, dann sparst du eine Multiplikation.

8 Tap FIR mit 35kS/s passt auf nen 16MHz Mega32 grad so drauf von der 
CPU her. Und du hast ja nur 2 Filterkoeffizienten.

von Ralf G. (ralg)


Lesenswert?

Wie genau soll's denn sein. Vielleicht reichen ja auch erstmal 8Bit für 
den ADC.

von Hannäs (Gast)


Lesenswert?

Du hast ziemlich viel schon richtig gemacht, laß Dir nichts 
aufschwatzen. Der Tipp von Peter Dannegger, nur die History static zu 
machen, erlaubt dem Compiler diverse spürbare Optimierungen.

Aber sieh der Realität ins Auge: 32 Bit Multiplikationen und Divisionen 
sind auf dem ATmega viel zu langsam, um drei Filter in Echtzeit zu 
rechnen. Ende.

Du kannst aber mit 8 Vor- und 8 Nachkommabits eine rein akademische 
Lösung hinbekommen (natürlich reicht die Wortlänge nicht mehr, um die 
Filter korrekt durchzurechnen).

Schließlich könntest Du alles in optimiertem Assembler ausführen. Das 
wird erfahrungsgemäß etwa doppelt so schnell wie C.

von M. W. (rallini94)


Lesenswert?

Erst einmal vielen Dank für diese reichlichen Ideen. Ich versuche mal 
auf all eure Vorschläge einzugehen


Frank M. schrieb:
> Wie oft ist auch dieser Codeschnipsel unvollständig.
>
> Welchen Typ hat ADCwert? Wahrscheinlich volatile int32_t. Benutze eine
> lokale Variable tmp_ADCwert und setze ADCwert erst am Schluss aller
> Berechnungen - wenn Du sie dann überhaupt noch brauchst.

Ja, da hab ich tatsächlich die Deklaration vergessen. Ich hatte int16_t 
benutzt.

> int32-Berechnungen sind auf einem AVR halt aufwendig. Versuche, den
> Wertebereich einzugrenzen und zu prüfen, ob Du evtl. mit int16
> auskommst.

Das kam mir auch schon in den Sinn und werde ich mal machen


m.n. schrieb:
> Man kann Einiges optimieren, aber letztlich wird der AVR etwas langsam
> bleiben.
> Beispiel:
>
> M. W. schrieb:
>> ADClow = ADCL;
>>   ADChigh = ADCH;
>>
>>   ADCwert = (ADChigh<<8) | (0xFFFF & ADClow);
>>   ADCwert -= 511;    // Mittelwert entfernen
>
> schreibt man besser als: ADCwert = ADC-511;
>
> oder
>> TPout = (wnTP + (wn1TP*2) + wn2TP);
>
> zumindest als: TPout = wnTP + wn1TP + wn1TP + wn2TP;
> obwohl TPout wohl nirgends verwendet wird?

TPout wird momentan noch nicht verwendet, weil ich mich erstmal auf die 
Ausführgeschwindigkeit konzentieren wollte. Deine Tipps werde ich 
umsetzen.



Falk B. schrieb:
> @  M. W. (rallini94)
>
>>- Speicherzugriffe vermeiden
>>- aufwändige Schleifen weggeschmissen
>>- Integer Rechnungen statt floating point
>>- Filterordnung so klein wie möglich
>
> ISR(ADC_vect)
> {
>> static uint8_t    ADChigh = 0;
>> static uint8_t    ADClow = 0;
>
> Brauchst du nicht, der avr gcc kann das mit einem Rutsch richtig
> auslesen.
>
>> static int32_t    wnTP = 0;
>> static int32_t    wn1TP = 0;
>> static int32_t    wn2TP = 0;
>> static int32_t    TPout = 0;
>
> 32 Bit Arithmetik ist nicht die Stärke des AVR und auch nicht im
> Zusammenhang mit dem avr gcc, das ist etwas stiefmütterlich umgesetzt.
>
>
>   static int32_t    wnHP = 0;
>   static int32_t    wn1HP = 0;
>   static int32_t    wn2HP = 0;
>   static int32_t    HPout = 0;
>
>
>> TIFR1  |= 0xFF;   // flags des Counters loeschen
>
> Musst du nicht, das macht die Hardware allein.

Soweit ich das verstanden habe eben nicht. Denn das Flag vom Timer wird 
nur zurückgesetzt, wenn die ISR vom Timer aufgerufen wird. Da ich aber 
die ISr vom ADC nutze, bleibt das Timer-Interrupt-Flag unberüht. Oder 
habe ich das missverstanden?

>
>> ADClow = ADCL;
>> ADChigh = ADCH;
>> ADCwert = (ADChigh<<8) | (0xFFFF & ADClow);
>
> Mist. Das geht einfach so.
>
>   ADCwert = ADC -511; // Mittelwert entfernen
>

wird gemacht

>
>>  // TP filtern
>>  wnTP = ADCwert + ((3746  wn1TP - 1726  wn2TP)/2048);
>
> Hier würde ich im .lss File prüfen, ob der avr gcc schlau genug war, das
> als Bitschiebung umzusetzen. Mit etwas Pech ist das eine echte Division.
>
>>  TPout = (wnTP + (wn1TP*2) + wn2TP);
>>  TPout = TPout/1024;
>
> Hier das Gleiche.

Ich habe es auch schonmal mit expliziten Shifts versucht, war - wenn ich 
mich richtig erinnere - auch nicht wirklich schneller


Peter D. schrieb:
> Was soll die static Orgie?
> Damit zwingst Du den Compiler, die Variablen im RAM anzulegen.
> Static macht man nur die Variablen, die bis zum nächsten Interrupt
> überleben müssen.

Ok, alles Überflüssige wird weggeschmissen

> Und muß es unbedingt signed sein?
> Bei unsigned kann der Compiler besser optimieren, z.B. teure Divisionen
> /2^n durch Schieben ersetzen.
> Den DC-Offset kann man bequem nach dem TP-Filter abziehen (HP, BP
> schmeißen ihn ja selber raus).

Ich schau mal.

Ausführliche Berichterstattung gibt es, wenn ich das alles ausprobieren 
konnte. Danke nochmal

: Bearbeitet durch User
von Klaus R. (Gast)


Lesenswert?

hier vermutlich ein Fehler:
1
static int32_t    wn1TP = 0;
2
  static int32_t    wn2TP = 0;

diese Variablen dürfen wohl vermutlich eher nicht auf Null gesetzt 
werden.

Zum Test habe ich das Ganze mal durch den Luna Compiler gejagt und komme 
auf 18 kHz.
1
isr myAdc fastauto
2
  'global: HPout, TPout
3
  dim ADCwert as static word
4
  dim wnTP,wn1TP,wn2TP,wnHP,wn1HP,wn2HP as static int32
5
6
  avr.TIFR1  or= 0xFF    //flags des Counters loeschen
7
  ADCwert = Adc.ADCW - 511 'Mittelwert entfernen
8
9
  'TP filtern
10
  wnTP = ADCwert + ((3746 * wn1TP - 1726 * wn2TP) / 2048)
11
  TPout = (wnTP + (wn1TP * 2) + wn2TP)
12
  TPout /= 1024
13
  
14
  wn2TP = wn1TP
15
  wn1TP = wnTP
16
  
17
  'HP filtern
18
  wnHP = ADCwert - ((804 * wn1HP + 408 * wn2HP) / 2048)
19
  HPout = ((wnHP - (wn1HP * 2) + wn2HP))
20
  HPout = HPout / 16
21
  
22
  wn2HP = wn1HP
23
  wn1HP = wnHP
24
endisr

von M. W. (rallini94)


Lesenswert?

Guten morgen,

ich habe gestern noch einmal eure Vorschläge ausprobiert. Folgendes kam 
dabei raus:

Das Entfernen der Statics brachte ein paar Taktzyklen, aber nicht so 
spürbare Optimierungen, wie das bei Hannäs klang.

Die neue ADC berechnung macht den Code auf jeden Fall übersichtlicher, 
brachte aber natürlich auch nur ein paar Zyklen, aber die war im 
Vergleich zur Filterung ja eh schnell.

Ich habe dann nochmal mit Matlab geguckt, ob int16 nicht auch reicht. 
Ich war mir da am Anfang nicht sicher, ob während der Berechnungen nicht 
vielleicht grössere Werte entstehen. Aber in Matlab schien es auch mit 
int16 gut zu funktionieren, also das ganze in C reingehauen und siehe 
da, die Ausführgeschwindigkeit ist drastisch gesunken, jetzt passt auch 
der BP mit in die ISR.

Momentan funktioniert das Gesamtsystem noch nicht, aber das wird 
vermutlich auch noch ein Hardware-Problem sein.

Auf jeden Fall nochmal vielen Dank an alle Unterstützer

von m.n. (Gast)


Lesenswert?

M. W. schrieb:
> Aber in Matlab schien es auch mit
> int16 gut zu funktionieren,

Ohne die engültigen Werte zu kennen, hätte ich an dieser Stelle aber 
erhebliche Bedenken, ob da nicht Überläufe stattfinden:

M. W. schrieb:
> (3746 x wn1TP - 1726 x wn2TP)

von M. W. (rallini94)


Lesenswert?

m.n. schrieb:
> M. W. schrieb:
>> Aber in Matlab schien es auch mit
>> int16 gut zu funktionieren,
>
> Ohne die engültigen Werte zu kennen, hätte ich an dieser Stelle aber
> erhebliche Bedenken, ob da nicht Überläufe stattfinden:
>
> M. W. schrieb:
>> (3746 x wn1TP - 1726 x wn2TP)

Das dachte ich auch, aber ich habe es mit Matlab rechnen lassen. Sowohl 
mit int16 als auch mit double als Datentyp, sahen die Ergebnisse im Plot 
gleich aus. Als Eingangssignal des Test habe ich einen Frequenzsweep 
genommen, sodass ich denke, dass ich nicht versehentlich ein Fall 
gefunden habe, der gerade so funktioniert, sondern dass das allgemein so 
klappt, oder denke ich falsch?

von Sascha (Gast)


Lesenswert?

Einfach mal dauerhaft 1024 auf den Filter geben und sich die 
Sprungantwort rausgeben lassen. Wenn das mit dem Matlab Plot 
übereinstimmt ist alles gut.

von m.n. (Gast)


Lesenswert?

M. W. schrieb:
> Das dachte ich auch, aber ich habe es mit Matlab rechnen lassen. Sowohl
> mit int16 als auch mit double als Datentyp, sahen die Ergebnisse im Plot
> gleich aus.

Mein Kopf sagt mir, daß das 1. Produkt überläuft, sobald wn1TP >= 9 ist. 
Vielleicht rechnet Matlab intern mit höherer Auflösung und gibt nur das 
Ergebnis als int16 aus. Mir wäre diese Lösung zu heiß!

Wenn Du nicht auf den ATmega328 angewiesen bist, es gibt gute+günstige 
Boards mit leistungsfähigen Prozessoren und gratis Entwicklungsumgebung, 
bei denen für Deinen Code (und möglichen Erweiterungen) keinerlei 
Einschränkungen bestehen. Im einfachsten Fall nimmt man ein Programm 
"blinky.c" und tippt die Filterfunktion einfach dazu. Dafür Bedarf es 
keiner großen Einarbeitungszeit und der Erfolg ist auch dann noch 
garantiert, wenn Deine Filter komplexer werden.

Mein Vorschlag.

Bitte melde dich an um einen Beitrag zu schreiben. Anmeldung ist kostenlos und dauert nur eine Minute.
Bestehender Account
Schon ein Account bei Google/GoogleMail? Keine Anmeldung erforderlich!
Mit Google-Account einloggen
Noch kein Account? Hier anmelden.