display.c <> Blocking delay

[stecor7001]

Hello all,

Just a quick observation - in display.c, line74-76 (function display_writechar) there is a test to check if the display buffer is full, and if so, the system waits 1ms then retries enqueue-ing additional info in the display buffer..

However, dequeue operations are performed outside of interrupt context (in function display_tick, which in turn is called from clock() which itself gets called in the main loop), thus there is no chance that the display buffer would empty while waiting.

Proposed solutions: 1/ If we're willing to wait 1ms, that equates to 20x 50us (which is about the time taken to write a char to the display: 10us for bit mangling + 40us wait between instructions). So we could call display_tick 10x instead, which allows for some context changes (lost time) plus ensures that the display buffer is being cleared. 2/ Do away with the check entirely. (personal preference is to use solution 1 not remove the check)

Not sure it's worth switching display ops to interrupt context since this is a non-critical subsystem.

Please let me know what you think.

Thanks!

/snip/ display.c /snip/ 64 This code is identical for all display buses and display types, because it 65 just queues up the character. 66
67 In case the buffer is full already it waits for a millisecond to allow 68 data to be sent to the display, then it tries again. If it still fails then, 69 it drops the character. This way we're fairly protected against data loss, 70 still we guarantee to not hang forever. 71 */ 72 void display_writechar(uint8_t data) { 73

74 if ( ! buf_canwrite(display)) { 75 delay_ms(1); 76 } 77 if (buf_canwrite(display)) { 78 buf_push(display, data); 79 } 80 }

[Wurstnase]

However, dequeue operations are performed outside of interrupt context (in function display_tick, which in turn is called from clock() which itself gets called in the main loop), thus there is no chance that the display buffer would empty while waiting.

That makes sense.

I think we should only wait until we can write in the buffer again. So worst case, the interface is busy and the display_tick() will return immediately.

What do you think?

void display_writechar(uint8_t data) {
  uint8_t looptick = 0;
  while ( ! buf_canwrite(display) && looptick < 10) {
    display_tick();
    delay_us(50);
    looptick++;
  }
  if (buf_canwrite(display)) {
    buf_push(display, data);
  }
}

[stecor7001]

Yep, makes sense, thank you! However: delay_us(50) may not be needed here - in my version I had to add it to the HD44780 driver file, since without it, on an unloaded system, the display gets corrupted (clock() gets called too often). I am still thinking about a better solution, but for now, I think it's acceptable to wait 40us - blocking mode - after each display update... if you have a better approach, please let me know. (an option is to throttle back display_tick() - to max 1call/ms).

I think you're the lucky owner of a really quick display. Mine corrupts with the current Teacup branch(and I did check the specs on HD44780 - it does say 37us char load time, so... it is in spec).

By the way, I think you (and I mean the maintainers, you obviously included and Triffid as well) did a great job with the code! It's really easy to understand and work with. Thanks!!!

Also, on another note, whoever designed the boards was definitely NOT a programmer! How can one use different port pins on the same LCD connector??? The amount of bit-mangling to adjust to this... can we call it error ? ... it is unbelievable. Same goes for motor pins, extruders, everything.... why?

[Wurstnase]

I don't have a HD44780. I only experiment a bit with a SSD1306.

It's maybe a better idea to wait 50µs and display_tick() afterwards. The idea behind this is only to block until we can write again in the buffer, and wait a short time while display could be busy.

[stecor7001]

Yes - fully agree. You might make it 40us though instead of 50 (datasheet says 37us).

Thank you!