Duet3D / RepRapFirmware

OO C++ RepRap Firmware
GNU General Public License v3.0
932 stars 533 forks source link

[Bug]: MB6HC (maybe 6XD too) - LED strips connected to io ports set first LED incorrectly, always green #996

Closed droftarts closed 3 months ago

droftarts commented 3 months ago

Duet Forum Discussion Thread

https://forum.duet3d.com/topic/35653/rgb-rgbw-led-malfunction-3-5-1

Which Duet products are you using?

Firmware Version

RRF 3.5.1

Duet Web Control Version

DWC 3.5.1

Are you using a Single Board Computer (RaspberryPi) with your Duet?

Please upload the results of sending M122 in the gcode console.

M122 Report

Please upload the content of your config.g file.

; LED Strips
M950 E0 C"io8.out" T1 ; configure LED strip #0
M950 E1 C"io7.out" T2 ; configure LED strip #1

Please upload the content of any other releveant macro files.

A range of LED values was tested:

M150 E1 R0 U255 B0 W0 P40 S6 F0
M150 E1 R0 U0 B0 W0 P40 S6 F0
M150 E1 R0 U0 B255 W255 P40 S6 F0
M150 E0 R0 U0 B0 P40 S6 F0
M150 E0 R0 U0 B255 P40 S6 F0
M150 E0 R0 U0 B255 P255 S6 F0
M150 E0 R0 U0 B0 P255 S6 F0
M150 E0 R0 U255 B0 P255 S6 F0
M150 E0 R255 U0 B0 P255 S6 F0

Details specific to your printer.

Bench test only

Links to additional info.

No response

What happened?

Following the forum thread, I tested RGB and RGBW LEDs on 6HC io ports in RRF 3.5.1, and see the same as the user, ie first LED is stuck green. I went back and tested on 3.5.0-rc.1 (works correctly), rc.2 (works correctly), and rc.3 (first LED stuck green) too. It seems there was a regression in function between rc.2 and rc.3. I also tested in 3.5.1 on Duet 3 Mini 5+, and LEDs on io port work correctly, so this seems to be specific to 6HC, and possibly 6XD (I don't have one to test).

dc42 commented 3 months ago

Issue was caused by the SAME70 cache behaviour and instruction reordering by the compiler. This sometimes led to the first pulse being too long when it was supposed to be a 0 (causing the reported symptom of too much green on the first LED), and sometimes led to the second pulse being delayed and shortened when it was supposed to be a 1 (causing too little green). Added ISB instructions to prevent the re-ordering, an alignment attribute to force the start of the function to be aligned on a cache line boundary, and some NOPs to shift the critical loop bodies to the middle of cache line boundaries.