Fixes for several lcd_1602_i2c.c issues

[ahmogit]

Re the following comment in lcd_toggle_enable() accompanying the value
of 600 us for DELAY_US:

"We cannot do this too quickly or things don't work"

Various datasheets I've come across for these 16x2 (and 20x4) modules from
3-4 different manufacturers show the Enable minimum active pulse dwell
(e.g. t_w,min, on p. 14 of [1]) as 250 ns or less. So that 600 us value is
excessive by more than three orders of magnitude.

The reason that the example code needs that huge value in order to "work"
properly is because the required delays (e.g. per [1], p. 17) between the
first three initialization commands are missing from lcd_init(). But the
time expended in lcd_toggle_enable(), due that artificially huge 600 us
DELAY_US is fortuitously compensating (partially) for the missing
delays in lcd_init() (since lcd_init() invokes lcd_toggle_enable()
via lcd_send_byte() on every write operation.)

Behavior observed with the present code, due to the above issues:

• Unreliable initialization: Malformed screen characters occasionally
  appear immediately after init. (Observed on approximately 5 % of
  inits of most 16x2 units that I have, depending on LCD vendor, and
  considerably more often (perhaps 10% - 20%) on the 20x4 modules,
  which have near-identical interface specs).

• Write times much slower than the LCD module is capable of.

• Write times scale poorly with I2C clock rate. (For example,
  increasing the I2C clock by a factor of 10 leads to only about
  20% reduction in write times; see examples below.)

To address these issues: If you insert explicit delays between the first
two writes in lcd_init() in order to satisfy the requirements in [1], e.g.

   [...]
   lcd_send_byte(0x03, LCD_COMMAND);
   sleep_ms(5);                        // Conservative
   lcd_send_byte(0x03, LCD_COMMAND);
   sleep_ms(5);                        // Conservative
   lcd_send_byte(0x03, LCD_COMMAND);
   [ ... ]

then you can reduce DELAY_US down to 1 us, or even eliminate it entirely,
since the I2C comm delays alone will easily meet the 250 ns t_w,min
requirement. The 5 ms example value above is even conservative; per [1],
only 4.1 ms is required between the first two init commands and 100 us
between the second and third.

These two mods eliminate the initialization reliability issue entirely,
and provide significantly improved per-character write times as well.
More importantly, the write times scale approximately in proportion to
I2C clock rate, as one would expect. (The original code scales poorly
with I2C clock rate because the huge 600 us delay in lcd_toggle_enable()
is incurred on every I2C write, i.e. a massive overhead on every write
operation.)

Timing examples, on Pico RP2040:

Original vs. fixed: I2C clock 100 kHz:

=================================================
examp_orig:
  I2C rate:                   100 kHz
    lcd_string(1 char):     4.894 ms
    lcd_string(15 chars):  73.103 ms
=================================================

=================================================
examp_fixed:
  I2C rate:                   100 kHz
    lcd_string(1 char):     1.294 ms
    lcd_string(15 chars):  19.194 ms
=================================================

Original vs. fixed: I2C clock 1 MHz:

=================================================
examp_orig:
  I2C rate:                  1000 kHz
    lcd_string(1 char):     3.763 ms
    lcd_string(15 chars):  56.201 ms
=================================================

=================================================
examp_fixed:
  I2C rate:                  1000 kHz
    lcd_string(1 char):     0.169 ms
    lcd_string(15 chars):   2.323 ms
=================================================

---

REFERENCES

[1] Typical spec sheet for one particular manufacturer (WaveShare)
of these ubiquitius 16x2 LCD modules:

     https://www.waveshare.com/datasheet/LCD_en_PDF/LCD1602.pdf

Datasheets for similar modules from other vendors give generally
similar figures for t_w,min and for the requred delays between
the initialization commands. So this WaveShare example datasheet
is not an outlier.

[lurch]

Apologies if this is a stupid question, but the datasheet that you're quoting is for a parallel-controlled LCD (e.g. https://uk.rs-online.com/web/p/lcd-monochrome-displays/2109029 ) but I believe the lcd_1602_i2c.c example code is for driving an I2C-controlled LCD (e.g. https://thepihut.com/products/lcd1602-i2c-module ).
Does the I2C chip add additional timing restrictions on top of what a "raw" parallel LCD would require?

[ahmogit]

@lurch Not a dumb question at all, happy to clarify.

Quick background and terminology:

All of the various "LCDxxxx" modules (xxxx = 1602 or 2004) that I've come
across on Ali/Amazon/eBay/etc usually comprise a base LCD board with
a piggybacked PCF8574 (or equivalent) I2C bridge module. The base LCD
board has a parallel interface, as you point out. The I2C bridge simply
provides a serial interface that enables the MPU programmer to exercise
the parallel control/data lines on the base LCD board via I2C writes.
And that is indeed what the example library code (lcd_1602_i2c.c) does,
as you say.

But the underlying timing requirements of the base LCD board are unaffected
by the presence of the I2C bridge: The base LCD board "doesn't know" whether
it's being driven directly via its parallel interface or via the parallel side of the I2C
bridge chip. As long as the timing requirements of the base LCD board are met,
it should perform per spec.

So the effect of the relatively slow I2C bridge is simply to reduce the rate
at which commands and display data can be issued to the base LCD, but it
does not change the underlying timing requirements of the base LCD board.

The problems with the original example code are related to the following
two timing requirements of the base LCD board:

(a) Minimum assertion time for the "E" ('enable') line.

(b) Minimum required delays between the issuance of the first three
initialization commands.

For the three LCD base vendors for which I have datasheets (Shenzen Eone
Electronics, CrystalFontz America, and WaveShare), requirement (a) is a few
hundred ns (140, 230, 450 ns), and requirement (b) is shown as 4.1 ms for
the first delay and 100 us for the second.

The two interrelated problems with the example code vis a vis the above
timing requirements are as follows:

(1) The initialization delay requirements (b) are not explicitly enforced
in lcd_init(). (And the incidental I2C transport delays -- something
like a few hundred us per command, even at the default (slowest)
RPi Pico I2C clock rate of 100 kHz -- are also not sufficient to
meet the 4.1 ms requirement part of (b)).

(2) The assertion time for the E line is being artificially extended
to 600 us in lcd_toggle_enable() [see DELAY_US #define], thus
exceeding (i.e. more than satisfying) the minimum requirement by
more than three orders of magnitude; yet that hugely-longer-than-
necessary E assertion is accompanied in the code by the curious
comment, "We cannot do this too quickly or things don't work".

The explanation as to why the example code actually works at all, despite
(1), is that the hugely long E assertion time in lcd_toggle_enable() is
inadvertently introducing total delays between the initialization commands
in lcd_init() which, along with the I2C delays, are evidently "close enough"
to the multi-ms initialization requirement (b) that lcd_init() happens to
work most of the time (but not 100% reliably, as I mentioned.)

And the explanation as to the curious "things don't work" comment -- which
after all, seems exactly backward from reality, given that DELAY_US = 600 us
is already three orders of magnitude longer than necessary -- is that if
DELAY_US is reduced much below the 600 us, then things indeed "don't work",
just as the comment says; but that is because cranking down DELAY_US eventually
reduces the delay introduced by the E assertion in lcd_toggle_enable() below
the value necessary to inadvertently barely-meet initialization requirement (b).

So the example code is, effectively, "working by accident".

A secondary effect of (2) is that the artificially long E delay slows all
commands and data transfers from the MPU to the I2C bridge, not only
the initialization commands, thus unnecessarily increasing the MPU
character-write latencies, as shown in the example runs comparing
the original to the repaired code.

The fixes for the above fortuitously-working situation are straightforward:
Insert explicit delays into lcd_init() that guarantee satisfaction of (b)
regardless of I2C clock rate, and then reduce DELAY_US to near-zero
(or just eliminate it completely, since the I2C delays, even at the fastest feasible
I2C clock rates, are more than sufficient to meet the few-hundred-ns
requirement (a)).

So there you go. Hope the above long-winded yappage answers your questions.

If you're still skeptical or curious, I'd suggest making up an original/fixed
pair of example sources, with the fixes as described in the original post,
and then scope the I2C SDA line with both sets of code. (Or, even better,
scope the base module "E" signal, it comes out as a solder pad at the side
of the board, you can solder-tack a pigtail onto it easily). Then, with a
bit of careful scope triggering, you can observe exactly what the timing looks
like going into the LCD base module itself.

One additional note that may have been confusing: In my original post,
I cited the datasheet for a WaveShare 1602 part, but then mistakenly quoted
t_w,min figures and page numbers that instead referred to this CrystalFontz
datasheet

https://www.alldatasheet.com/datasheet-pdf/download/1574132/CRYSTAIFONTZ/LCD-1602A.html

rather than the cited WaveShare datasheet. So if you were headscratching
as to why the referenced page numbers don't make sense, that's why, and
my apologies for that. But it makes no significant difference to the overall
story though, the numbers between the two vendors' datasheets are not
appreciably different.

[lurch]

Thanks for the superbly detailed explanation! ❤️

Would you like to submit a PR with your suggested changes against the develop branch?

[dmalnati]

I'm not a party to this but am compelled to say this is by far the best level of detail and overall explanation I've seen outside a work setting. Pro stuff, A+.

[ahmogit]

@lurch:

Would you like to submit a PR with your suggested changes against the develop branch?

Decline, sorry.