Hallo zusammen,
ich sitze aktuell an einem Projekt, bei dem ich mit einem Atmega328P @
16MHz verschieden codierte Daten am Pin PD3 (INT1 Interrupt Pin)
einlesen möchte.
Auf diesem (DATA) Pin findet entweder eine 2-Wire Kommunikation statt,
dann werte ich auf einem anderen Pin noch das dazugehörige CLK Signal
aus.
Oder aber die Kommunikation findet ASK Moduliert nur auf dem DATA Pin
statt.
Im angehängten Bild sieht man den Mitschnitt einer 2-Wire Kommunikation.
CH2 ist CLK, CH3 DATA und CH4 ist ein Port vom Atmega, den ich zu
Debugzwecken nutze.
Bei der steigenden Flanke vom DATA Pin möchte ich in einer ISR erstmal
mit der Auswertung des ASK Signals beginnen. Dabei soll aber sofort beim
Eintritt in die ISR abgefragt werden, ob der Pegel des CLK Signals 0
ist. Andernfalls breche ich die ASK Demodulation ab, denn dann haben wir
eine 2-Wire Übertragung.
Mein Problem:
Der Prolog der ISR dauert so lange, dass ich den High-Pegel des CLK
Signals verpasse.
Anbei der Code der ISR:
1
ISR(INT1_vect){
2
uint16_ttemp_ctr;
3
toggle_pinC0;// Toggelt Pin CH4
4
if(!get_lv_PinD(SCK_PIN))
5
{
6
do_something();
7
}
8
else{
9
sniffer_state=SNIFFER_STATE_RESET;
10
}
11
TCNT1=0;
12
sei();
13
}
Wird folgendermaßen übersetzt:
1
ISR(INT1_vect){
2
00000116 PUSH R1 Push register on stack
3
00000117 PUSH R0 Push register on stack
4
00000118 IN R0,0x3F In from I/O location
5
00000119 PUSH R0 Push register on stack
6
0000011A CLR R1 Clear Register
7
0000011B PUSH R18 Push register on stack
8
0000011C PUSH R19 Push register on stack
9
0000011D PUSH R20 Push register on stack
10
0000011E PUSH R21 Push register on stack
11
0000011F PUSH R22 Push register on stack
12
00000120 PUSH R23 Push register on stack
13
00000121 PUSH R24 Push register on stack
14
00000122 PUSH R25 Push register on stack
15
00000123 PUSH R26 Push register on stack
16
00000124 PUSH R27 Push register on stack
17
00000125 PUSH R28 Push register on stack
18
00000126 PUSH R30 Push register on stack
19
00000127 PUSH R31 Push register on stack
20
toggle_pinC0;
21
00000128 IN R25,0x08 In from I/O location
22
00000129 LDI R24,0x01 Load immediate
23
0000012A EOR R24,R25 Exclusive OR
24
0000012B OUT 0x08,R24 Out to I/O location
25
if(!get_lv_PinD(SCK_PIN))
26
0000012C SBIC 0x09,6 Skip if bit in I/O register cleared
27
0000012D RJMP PC+0x0033 Relative jump
Der "Push-Prolog" braucht also ca. 18 Takte. Im Oszi Bild sieht man im
oberen Signal, wo ca. die Stelle ist wo der CLK Pegel ausgewertet würde.
Dort ist der Pegel aber schon wieder 0.
Ist es irgendwie möglich, den Prolog in die If-Abfrage zu verschieben?
Oder was wäre hier die beste Vorgehensweise, ohne gleich die ganze ISR
in Assembler zu schreiben?
Vielen Dank schonmal für eure Tipps.
Ohne handassemblieren wüsste ich keine Option den Prolog zu verschieben.
Allerdings kannst du ihn kürzer gestalten, indem du sowenig Verarbeitung
wie möglich in der ISR machst oder dir register für deine ISR
reservierst.
beim GCC meine ich war eh eine ganze Latte an registern ungenutzt (nagel
mich nicht fest, ob es irgendwo bei R15 war, oder bei R7...) die kannst
du in der ISR nutzen und der compiler muss nicht so viel kontext
abspeichern.
z.B. sowas:
1
registeruint16_ttemp_ctr=asm("R7");/* bei der Zeile müsste ich auch nochmal nachforschen wie das genau ging und welche register "frei" sind */
Philipp schrieb:> Atmega328P @ 16MHz
Da ist noch Potenzial. Für den Mega328p garantiert der Hersteller 20Mhz.
Philipp schrieb:> INT1 Interrupt Pin
Prüfe, ob du die Aufgaben nicht weitgehend asynchron in Hardware lösen
kannst!
Variante 1: Du setzt in der ISR nur ein Flag anstatt do_something und
ruft sie stattdessen aus der main heraus auf.
Variante 2: Du stellst sicher, dass do_something geinlined werden kann
und für den Compiler ersichtlich wenige register verwendet. Im Moment
fügt der Compiler die ganzen push und pop hinzu, weil er nicht weiß, wie
viele Register do_something verwenden wird und er deshalb alle Register,
die die Funktion verwenden/ohne Sicherung ändern darf ("call
clobbered"), einmal für die ISR sichern muss.
https://gcc.gnu.org/wiki/avr-gcc#Register_Layouthttps://gcc.gnu.org/wiki/avr-gcc#Calling_Convention
Variante 3: Du machst eine naked ISR in der du via inline Assembler
zunächst nur die Mindestanzahl Register sicherst, die für die
Flankenerkennung notwendig sind, die Flanke erkennst, und dann manuell
die verbleibenden Register sicherst und do_something aufrufst.
Weshalb setzt du eigentlich sei() am Ende der ISR? Scheint mir sinnlos.
Dein toogle_pinC0 ist langsamer als nötig. Es macht in/ldi/eor/out (4
Takte, 2 Register + SREG). Der ATmega328P kann aber auch über das PINx
Register den Ausgang toggeln:
https://ww1.microchip.com/downloads/en/DeviceDoc/ATmega48A-PA-88A-PA-168A-PA-328-P-DS-DS40002061B.pdf
S.84:
> The port input pins I/O location is read only, while the data> register and the data direction register are read/write. However,> writing a logic one to a bit in the PINx register, will result> in a toggle in the corresponding bit in the data register.
Du könntest also stattdessen ldi/out (2 Takte, 1 Register) oder sogar
sbi (2 Takte, 0 Register) nutzen.
Philipp schrieb:> Ist es irgendwie möglich, den Prolog in die If-Abfrage zu verschieben?> Oder was wäre hier die beste Vorgehensweise, ohne gleich die ganze ISR> in Assembler zu schreiben?>> Vielen Dank schonmal für eure Tipps.
Manchmal kann man ISR so vereinfachen, daß das ohne Register und SREG
geht. Z.B. man ändert ein Bit in IO-Bereich unter 0x20. Dann geht es
wunderschön mit NAKED. Als Beispiel:
Du solltest dir man das Input Capture Feature ansehen. Vor einiger Zeit
habe ich mal einen Artikel dazu geschrieben, ist zufällig gerade der
Artikel der Woche:
https://www.mikrocontroller.net/articles/High-Speed_capture_mit_ATmega_Timer
Damit hast du deutlich mehr Zeit um auf ein Signal zu reagieren und
weißt trotzdem genau, wann es eingetreten ist.
Eine Möglichkeit wäre, den Input Capture für das Clock Signal zu
verwenden.
Michael
Philipp schrieb:> do_something();
Servus,
wieviele Register verwendet do_something()?
Die müssen alle noch auf den Stack und wieder hergestellt werden.
Solange wir nicht erfahren, was do_something() so anstellt, kann man
nichts sagen.
Das sei(); am Ende ist (zumindest beim IAR, ich weiß nicht wie es beim
gcc ist) überflüssig, weil ein kluger Compiler am Ende des ISR kein RET
sondern ein RETI setzt, welcher die ISR automatisch wieder aktiviert.
Meiner Erfahrung nach macht der gcc ISR()`s deutlich weniger effizient
als z.B. der IAR.
Rudi schrieb:> Variante 3:
Variante 4:
Du läßt die ISR nach Assembler übersetzen und optimierst dann das
erzeugte *.S File.
Daß Funktionsaufrufe in ISRs böse sind, wurde ja schon gesagt.
Peter D. schrieb:> Variante 4:> Du läßt die ISR nach Assembler übersetzen und optimierst dann das> erzeugte *.S File.
Da gehört aber auch Mut und Selbstbewusstsein dazu:
"Ich weiß es besser als der Compiler".
Ich würde eher versuchen über Compileroptionen und besseren C-Code eine
bessere Lösung zu finden.
Der Robs schrieb:> Da gehört aber auch Mut und Selbstbewusstsein dazu:> "Ich weiß es besser als der Compiler".
Einfach nur das Instructionset aufschlagen, dann kann man schon
entscheiden, welche Instruktionen verschoben werden können.
Der Code ist ja schon vollständig und auch die Syntax für die benutzten
globalen Variablen ist schon fertig.
R0, R1 muß man nicht sichern, wenn man sie nicht verändert. Das war
einfach nur ein unglückliche Vereinbarung, bevor es MUL und LPM/SPM gab.
Danke erstmal für die zahlreichen Tipps!
Um mal vorweg das "Geheimnis" um die do_something(); Funktion zu lüften
(dachte das interessiert nicht und lenkt nur vom Hauptproblem ab).
Do_something() ist nicht mal ein Funktionsaufruf, sondern nur eine
If-Abfrage die abhängig der Anzahl der Pulse die Pulsdauer abfragt. Den
genauen Code werde ich heute Abend wenn ich wieder am PC bin
nachreichen.
Rudi (Gast) schrieb:
> Variante 2: Du stellst sicher, dass do_something geinlined werden kann> und für den Compiler ersichtlich wenige register verwendet. Im Moment> fügt der Compiler die ganzen push und pop hinzu, weil er nicht weiß, wie> viele Register do_something verwenden wird und er deshalb alle Register,> die die Funktion verwenden/ohne Sicherung ändern darf ("call> clobbered"), einmal für die ISR sichern muss.
Der Compiler kennt den code in do_something() schon. Der Code verwendet
wirklich einige Register, das push/pop dieser ist also gerechtfertigt.
F.M. schrieb:
> Philipp schrieb:> > Atmega328P @ 16MHz>> Da ist noch Potenzial. Für den Mega328p garantiert der Hersteller 20Mhz.
Ich benutze ein Arduino Board. Da möchte ich ungerne etwas ändern, denn
mein Code soll auch von anderen Arduino Usern benutzt werden können.
Außerdem wird das Problem mit 4 MHz mehr auch nicht behoben sein.
F.M. schrieb:
> Philipp schrieb:> > INT1 Interrupt Pin>> Prüfe, ob du die Aufgaben nicht weitgehend asynchron in Hardware lösen> kannst!
Könne ich wahrscheinlich schon. Wäre aber wesentlich komplizierter und
unsauberer. Hätte dann auch das Risiko, dass ich im Interrupt erkannte
Flanken in der Main Loop verpasse, wenn diese nicht schnell genug läuft.
Ich würde mich gerne auf die Lösung konzentrieren, den PIN in der ISR
früher auszulesen.
Rudi (Gast) schrieb:
> Weshalb setzt du eigentlich sei() am Ende der ISR? Scheint mir sinnlos.
Dachte immer dass man das bräuchte. Werde ich dann in Zukunft weglassen.
Rudi (Gast) schrieb:
> Dein toogle_pinC0 ist langsamer als nötig. Es macht in/ldi/eor/out (4> Takte, 2 Register + SREG). Der ATmega328P kann aber auch über das PINx> Register den Ausgang toggeln:
Super Hinweis, auch das wusste ich nicht. Werde ich abändern.
Aber das "Toggle" ist nur zu Debug Zwecken da. Habe auch schon versuch,
das in das If zu verschieben, was natürlich nichts gebracht hat. Es ist
der Prolog, der einfach viel zu viel Zeit verschwendet.
Maxim B. (max182) schrieb:
> Manchmal kann man ISR so vereinfachen, daß das ohne Register und SREG> geht. Z.B. man ändert ein Bit in IO-Bereich unter 0x20. Dann geht es> wunderschön mit NAKED. Als Beispiel:
Gebe dir Recht. Aber wie oben beschrieben finde ich es sauberer, die
Verarbeitung in der ISR zu machen. Wenn ansonsten die Main loop mit
niedriger Frequenz als die ISR läuft, entstehen hier Fehler. Aber ich
mache mir nochmal Gedanken darüber , und werde es vielleicht am Schluss
doch so umsetzen.
Michael D. (nospam2000) schrieb:
> Du solltest dir man das Input Capture Feature ansehen. Vor einiger Zeit> habe ich mal einen Artikel dazu geschrieben, ist zufällig gerade der> Artikel der Woche:> https://www.mikrocontroller.net/articles/High-Speed_capture_mit_ATmega_Timer
Gute Idee! Ich kannte zwar das Input-Capture Feature, hatte aber nicht
mehr daran gedacht.
Das Problem ist aber, dass ich zum Zeitpunkt der Flanke gleichzeitig
noch das Level eines anderen Pins abfragen muss. Da sehe ich noch
nicht, wie mir das Input-Capture dabei helfen kann. Der CLK Pin hat auch
keine Input Capture Funktionalität (glaube das sind nur Pin INT0 und
INT1, hab aber gerade kein Datenblatt hier).
Peter D. (peda) schrieb:
> Variante 4:> Du läßt die ISR nach Assembler übersetzen und optimierst dann das> erzeugte *.S File.>> Daß Funktionsaufrufe in ISRs böse sind, wurde ja schon gesagt.
Denke das ist die beste Idee! Das probiere ich heute Abend mal.
Philipp schrieb:> Das Problem ist aber, dass ich zum Zeitpunkt der Flanke gleichzeitig> noch das Level eines anderen Pins abfragen muss. Da sehe ich noch> nicht, wie mir das Input-Capture dabei helfen kann.
Ein externes D-FlipFlop würde die Sache wesentlich entspannter gestalten
;-)
Stimmt natürlich.
Aber mein Anspruch ist es, soweit wie möglich ohne zusätzliche Hardware
auszukommen. Der Atmega ist ja grundsätzlich schon in der Lage, die
Aufgabe zu erfüllen. Ich sehe nicht ein, nur wegen den Eigenheiten des
Compilers anzufangen, zusätzliche Hardware aufzubauen ;-)
Philipp schrieb:> Der Compiler kennt den code in do_something() schon. Der Code verwendet> wirklich einige Register, das push/pop dieser ist also gerechtfertigt.
Das sind aber doch eine ganze Menge. Kompilierst du mit Optimierung?
Oliver
TIFR1=(1<<OCF1A);// clear OCF1A flag (which is always set before), otherwise interrupt will directly fire as soon as enabled. Clearing is done by writing 1.
28
TCCR1B|=(1<<WGM12);
29
TIMSK1|=(1<<OCIE1A);
30
}
31
else{
32
TIMSK1&=~(1<<OCIE1A);
33
TCCR1B&=~(1<<WGM12);
34
}
35
}
Compiler ist der avr-gcc Version 4.4.7, wenn ich das richtig sehe.
Compiliert wird mit -O1
Philipp schrieb:> Rudi (Gast) schrieb:>> Weshalb setzt du eigentlich sei() am Ende der ISR? Scheint mir sinnlos.>> Dachte immer dass man das bräuchte.
SEI() vor dem ISR-Ende hat den Vorteil, dass Interrupts vor dem POP-en
der Prozesserregister sofort wieder zuschlagen können. Stehen Interrupts
ohne Unterbrechung an, gibt's 'nen schönen Stacküberlauf mit
irrationalem Verhalten und nachfolgendem Reset. So wird verhindert, dass
der Prozessor mit Interrupts überlastet wird. :D
Hab den Code jetzt mal umgeschrieben, so dass die ISR nur ganz kurz ist:
1
ISR(INT1_vect){
2
if(!get_lv_PinD(SCK_PIN)){
3
TCNT1_diff_isr=TCNT1;
4
TCNT1=0;
5
toggle_pinC0;
6
lv_dout_new_edge=1;
7
}
8
else
9
{
10
toggle_pinC0;
11
sniffer_state=SNIFFER_STATE_RESET;
12
}
13
}
Dies hat den Prolog zwar deutlich verkürzt, er ist aber leider immer
noch recht lang.
Hier der ASM Code:
1
ISR(INT1_vect){
2
0000010C PUSH R1 Push register on stack
3
0000010D PUSH R0 Push register on stack
4
0000010E IN R0,0x3F In from I/O location
5
0000010F PUSH R0 Push register on stack
6
00000110 CLR R1 Clear Register
7
00000111 PUSH R24 Push register on stack
8
00000112 PUSH R25 Push register on stack
9
00000113 PUSH R30 Push register on stack
10
00000114 PUSH R31 Push register on stack
11
if(!get_lv_PinD(SCK_PIN)){
12
00000115 SBIC 0x09,6 Skip if bit in I/O register cleared
13
00000116 RJMP PC+0x0010 Relative jump
14
TCNT1_diff_isr = TCNT1;
15
00000117 LDI R30,0x84 Load immediate
16
00000118 LDI R31,0x00 Load immediate
17
00000119 LDD R24,Z+0 Load indirect with displacement
18
0000011A LDD R25,Z+1 Load indirect with displacement
19
0000011B STS 0x01C1,R25 Store direct to data space
20
0000011D STS 0x01C0,R24 Store direct to data space
21
TCNT1 = 0;
22
0000011F STD Z+1,R1 Store indirect with displacement
23
00000120 STD Z+0,R1 Store indirect with displacement
24
toggle_pinC0;
25
00000121 SBI 0x06,0 Set bit in I/O register
26
lv_dout_new_edge = 1;
27
00000122 LDI R24,0x01 Load immediate
28
00000123 STS 0x01CD,R24 Store direct to data space
29
00000125 RJMP PC+0x0005 Relative jump
30
toggle_pinC0;
31
00000126 SBI 0x06,0 Set bit in I/O register
32
sniffer_state = SNIFFER_STATE_RESET;
33
00000127 LDI R24,0x01 Load immediate
34
00000128 STS 0x0101,R24 Store direct to data space
35
}
Aber Tatsache ist, jetzt wird die CLK Flanke erkannt! Aber ganz knapp,
ich schätze 2 oder 3 Takte später wäre die Flanke schon vorbei. Das ist
natürlich alles andere als sicher.
Für die Robustheit sollte ich doch die ISR in inline ASM schreiben.
Philipp schrieb:> Compiler ist der avr-gcc Version 4.4.7, wenn ich das richtig sehe.> Compiliert wird mit -O1
-O1 ist zwar ganz nett, aber immer noch recht mies. Nimm besser -O2.
Peter D. schrieb:> Daß Funktionsaufrufe in ISRs böse sind, wurde ja schon gesagt.
Mit LTO kann man das Problem entschärfen. Dann weiß der Compiler auch,
welche Register er pushen muss Das könnte den Prolog verkürzen.
Allerdings würde ich LTO frühestens ab gcc 4.7.2 einsetzen.
Wichtig dabei: Auch der Linker muss dann mit derselben Optimierung -O...
wie der Compiler aufgerufen werden. Bei späteren gcc-Versionen kann man
sich das dann wieder sparen.
Wow, sogar gleich ein .S File für mich geschrieben!
Vielen Dank Falk für die großartige Hilfe! Ist gar nicht so kompliziert
wie ich es mir vorgestellt hatte ;-)
Ein paar kleine Fehler habe ich noch ausgebessert. Hier nochmal der
funktionierende Code:
Mit der Konstante ASSEMBLERFILE habe ich in meinem Include-File die
Enums "ausgeblendet", sonst meldet der Präprozessor im .S file einen
Fehler.
Wen es interessiert: Die komplette ISR ist jetzt nach ca 2.2us
durchgelaufen, der SCK Pegel wird frühstmöglich erkannt. Das nenne ich
mal Erfolg!
Abschließend nochmal danke an Alle für eure große Hilfe!!!
Musste doch noch etwas verbessern:
Das TCNT1 Register ist 16 bit breit. Dementsprechend muss auch der
Zugriff geändert werden (low-Byte, high-Byte). Bitte beachten wer diesen
Code als Beispiel nehmen will!
Philipp schrieb:> Mit der Konstante ASSEMBLERFILE habe ich in meinem Include-File die> Enums "ausgeblendet", sonst meldet der Präprozessor im .S file einen> Fehler.
Dafür gibt es schon ein #define vom Compiler
Philipp schrieb:> Ist gar nicht so kompliziert> wie ich es mir vorgestellt hatte ;-)
Nur ist das dann GCC-Assembler. Nach AVR-Assembler aus AVR Studio wirkt
GCC-Assembler enttäuschend.