Graphics Corrupted on 12864 LCD with MEGA2560/RAMPS/12864LCD #21495
Comments
|
|
|
Hi. Could you please provide your config files? Otherwise, diagnostics will be very difficult. |
|
Configuration.zip |
|
I guess this could be a delay issue similar to #21264 and fixed for different boards in PRs #21346 and #21425. I would suggest to try setting #define BOARD_ST7920_DELAY_1 DELAY_NS(125)
#define BOARD_ST7920_DELAY_2 DELAY_NS(63)
#define BOARD_ST7920_DELAY_3 DELAY_NS(125)in |
|
It does correct the issue. |
|
I guess we can now see where this is going. Time to visit all configs, maybe? |
|
In Configuration_adv.h, this could do them all .. |
|
Nope there are many that it would miss based on conditionals ... |
|
This is only required on really bad slow lcd's |
The ST7920 LCD driver is not bad it's just old, but there are 10's of thousands of them out there in active use. |
|
its not the driver. its the actual lcd module. My REPRAP_DISCOUNT_FULL_GRAPHIC_SMART_CONTROLLER on ramps still works as it always has, without needing delays when on a mega2560. They say they are the same chipset. |
|
The scope is the only real way to know if the signal time is not correct or marginally close. I can set it up a full test, but in my experiences it's not usually the driving chip .. it's the driving signal and noise. Nothing changed on the hardware I have, then suddenly the display goes south after some AVR code changing along with lots of others hitting the same behavior. I think it's more likely to be tight code timing issues than a bad display LCD chip. I have only once in my career observed poor tolerances in a batch of LCD chips and it was intermittent in nature. Based of a quick check to the clock specs at https://www.crystalfontz.com/controllers/Sitronix/ST7920/ I am betting on marginal timing coming out of the code. |
|
I can't remember which PR or when the change happened, the delays were sped up by something like 50%. This would mean all the current LCD timings are incorrect (some worse than others) since those weren't updated & likely why there are reports & PRs trickling in to update LCD timings. |
|
wasn't that was for stm32, not avr |
It didn't apply as broadly as I had hoped: #20901 |
|
As detailed in this post, it looks like a switch from GCC 5.4 to 7.3 in the platformio toolchain for AVR also changed the machine code, resulting in some delays being one cycle shorter than before. Also, based on what I gathered from that thread, the original delays on some AVR boards were tight even before that change in the toolchain. I would be open to make a PR for the RAMS 1.4 board affected here, but I am not sure where the default delays for this are set. Does this case apply for the board Marlin/Marlin/src/lcd/dogm/ultralcd_st7920_u8glib_rrd_AVR.cpp Lines 60 to 63 in 600ef1e ? |
|
Did some research and its clear that the timing is not correct for the mega2560. If the CPU is at the common 16MHZ one instruction cycle would be 62.5ns and with SPI that means we are very marginal when pushing a data byte. The spec for the ST7290 is a minimum 400ns SCLK cycle, we are frequently below that minimum. The fact that worked in the past is based on the chip working faster than spec in most cases!
#elif MB(RAMBO) test condition and loop = 62.5ns This is code for the u8glib_rrd_AVR and will likely fail with many ST7920 chips. So the theory is we can add a delay of 150ns on any of 1,2 or 3 and it should function correctly at that point. So that's what I tested: Existing code: #elif F_CPU == 16000000 test 1 test 2 test 3 The issue is that we are not within the documented spec of 400ns now that the compiler has improved optimizations. |
|
If you look at all the conditionals and different values, one thing becomes clear: a DELAY_NS(100) for example is not the same amount of nanoseconds (i.e. 100 one would hope) on each board. In that case the "_NS" moniker is misleading. If on the other hand, the delays actually ARE nanoseconds (reasonably accurate or minumums), they could be the same on each board. I know this came about historically, but there is a chance to get this cleaned up. |
|
I believe thats correct. The resolution is 62.5ns in this case if the value is 63 you will get a delay of 125ns , in the tests I performed we are getting 186.5ns. |
|
150 ns would effectively lead to 187.5 ns delays + communication command times. If we want to stay in spec, we would rather use #define CPU_ST7920_DELAY_1 DELAY_NS(125)
#define CPU_ST7920_DELAY_2 DELAY_NS(0)
#define CPU_ST7920_DELAY_3 DELAY_NS(125) right? That should add up to 437.5 ns per full cycle (assuming each command between the delays takes 62.5 ns). Anything problematic with this? |
|
Based on the ST7920 specs and how the compiler works with the DELAY_NS() macro there should be no need to condition on what board or F_CPU is used. The code should just define the delays based on the target device spec. One delay definition for all of them. e.g. ... |
|
I have an ST32 (MKS) board handy, It should work with the ST7920, I will give it a poke later and test the theory. |
I don't see any issues with it. |
|
Passes OK on MEGA2560/RAMPS1.4/12864LCD |
|
In any case I suggest writing ST7920_SWSPI_SND_8BIT() in assembly so we have greater control of the timing, at least for AVRs. This code might change with another compiler update or different flags, and the timing will be off again. If we write it in asm, we're sure that it won't change anymore. |
|
@ramiropolla Good point. It would also be a somewhat redundant re-implementation of the delay functions, but I get the reasoning. If assembler is requested, I can't supply the PR because I don't know the language. |
I think it would not be in the best interests of the community to do one driver in assembler to have more control over the small potentially negative impact of compiler improvements. Simplicity would keep the code maintained and serviceable. An error in the previous code is the root cause and not the compiler improvements fault. |
How did you get to that conclusion? What is the error in the previous code? (I genuinely want to know. did you read this comment?) |
|
I checked the timing on my scope is how I know the code is in error. The original code does not follow the specifications for the minimum cycle time of SCLK. Even without the compiler improvements the original code generated an SCLK time shorter than the spec. |
|
Can this be closed now that a couple related PRs have been merged? |
|
The final required delay should be #define CPU_ST7920_DELAY_1 DELAY_NS(125) This is now resolved. |
|
This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs. |
Did you test the latest
bugfix-2.0.xcode?Yes, and the problem still exists.
Bug Description
Random bits draw on status and Menu's
Bug Timeline
On or slightly before Mar 29/2021
Expected behavior
No random bits flying around.
Actual behavior
Random bits flying around.
Steps to Reproduce
Clone buxfix-2.0.x
Using RAMPS MEGA2560 based controller:
edit Configuration.h to include #define REPRAP_DISCOUNT_FULL_GRAPHIC_SMART_CONTROLLER
Version of Marlin Firmware
bugfix-2.0.x
Printer model
N/A
Electronics
No response
Add-ons
No response
Your Slicer
No response
Host Software
No response
The text was updated successfully, but these errors were encountered: