Re-implementation of ST7567 support for Duet 2 Maestro on v3.02 (#424)

* Adapted the latest version (2020-08-07) of v3.02-dev to include support
for a ST7565 minipanel display in the Duet 2 Maestro. An experimental
contrast parameter "C" (range 0-100) has been added to the M918 as well.
This builds on previous code where a ST7576 display was made to work on
a Duet 3 Mini board.

* Added final comments, TODO's and clarifications

* Added a default define to set the LCD_CS on the Maestro high permanently
on display init. LCD_CS functions as a gate pin for ST7567 displays on
the Maestro, and that doesn't have to happen with every chip select. In
this specific case, EXP_1 functions as the alternative LCD_CS, since the
original LCD_CS cannot provide an active low.

6 files changed, 78 insertions, 15 deletions

diff --git a/src/Display/Display.cpp b/src/Display/Display.cpp
index b0cc052a..fe95eb5e 100644
--- a/src/Display/Display.cpp
+++ b/src/Display/Display.cpp
@@ -145,9 +145,14 @@ void Display::ErrorBeep() noexcept
 
 void Display::InitDisplay(GCodeBuffer& gb, Lcd *newLcd, bool defaultCsPolarity) THROWS(GCodeException)
 {
-	newLcd->Init(LcdCSPin, LcdA0Pin, defaultCsPolarity, (gb.Seen('F')) ? gb.GetUIValue() : LcdSpiClockFrequency);
-	IoPort::SetPinMode(LcdBeepPin, OUTPUT_PWM_LOW);
-	newLcd->SetFont(SmallFontNumber);
+#ifdef SRC_DUETM_PINS_DUETM_H_
+	// Only for the Duet 2 Maestro since the LCD_CS gates the SPI0_SCK and SPI0_MOSI.
+	// NOTE: https://github.com/SchmartMaker/RepRapFirmware/tree/ST7565/src/Display did this on a higher level by using different display IDs
+	// NOTE: boundary checking is not implemented for the contrast (C) parameter yet
+	newLcd->Init(LcdCSAltPin, LcdA0Pin, defaultCsPolarity, (gb.Seen('F')) ? gb.GetUIValue() : LcdSpiClockFrequency, (gb.Seen('C')) ? gb.GetUIValue() : DefaultDisplayContrastRatio, LcdCSPin, true);
+#else
+	newLcd->Init(LcdCSPin, LcdA0Pin, defaultCsPolarity, (gb.Seen('F')) ? gb.GetUIValue() : LcdSpiClockFrequency, (gb.Seen('C')) ? gb.GetUIValue() : DefaultDisplayContrastRatio);
+#endif
 	IoPort::SetPinMode(LcdBeepPin, OUTPUT_PWM_LOW);
 	newLcd->SetFont(SmallFontNumber);
 
@@ -163,6 +168,8 @@ void Display::InitDisplay(GCodeBuffer& gb, Lcd *newLcd, bool defaultCsPolarity)
 
 GCodeResult Display::Configure(GCodeBuffer& gb, const StringRef& reply) THROWS(GCodeException)
 {
+	// BUG: calling M918 a number of times in succession seems to crash the firmware on the Maestro.
+	// This could be unreleased memory or something else.
 	bool seen = false;
 
 	if (gb.Seen('P'))
diff --git a/src/Display/Display.h b/src/Display/Display.h
index c15a255d..cba2d1cc 100644
--- a/src/Display/Display.h
+++ b/src/Display/Display.h
@@ -32,6 +32,8 @@ public:
 	bool IsPresent() const noexcept { return lcd != nullptr; }
 	void UpdatingFirmware() noexcept;
 
+	constexpr static uint8_t DefaultDisplayContrastRatio = 50;
+
 private:
 	void InitDisplay(GCodeBuffer& gb, Lcd *newLcd, bool defaultCsPolarity);
 
diff --git a/src/Display/Lcd/Lcd.cpp b/src/Display/Lcd/Lcd.cpp
index fcb25d76..6db5d6d1 100644
--- a/src/Display/Lcd/Lcd.cpp
+++ b/src/Display/Lcd/Lcd.cpp
@@ -23,13 +23,21 @@ Lcd::~Lcd()
 }
 
 // Initialise. a0Pin is only used by the ST7567.
-void Lcd::Init(Pin csPin, Pin p_a0Pin, bool csPolarity, uint32_t freq) noexcept
+void Lcd::Init(Pin csPin, Pin p_a0Pin, bool csPolarity, uint32_t freq, uint8_t displayContrastRatio, Pin gatePin, bool gatePinPolarity) noexcept
 {
+	// All this is SPI-display specific hardware initialisation, which prohibits I2C-display or UART-display support.
+	// NOTE: https://github.com/SchmartMaker/RepRapFirmware/tree/ST7565/src/Display did contain this abstraction
 	a0Pin = p_a0Pin;
+	this->gatePin = gatePin;
+	this->gatePinPolarity = gatePinPolarity;
+	this->displayContrastRatio = displayContrastRatio;
 	device.SetClockFrequency(freq);
 	device.SetCsPin(csPin);
 	device.SetCsPolarity(csPolarity);		// normally active high chip select for ST7920, active low for ST7567
 	pinMode(csPin, (csPolarity) ? OUTPUT_LOW : OUTPUT_HIGH);
+	if(gatePin != NoPin) {
+		pinMode(gatePin, (gatePinPolarity) ? OUTPUT_LOW : OUTPUT_HIGH);
+	}
 #ifdef __LPC17xx__
     device.sspChannel = LcdSpiChannel;
 #endif
diff --git a/src/Display/Lcd/Lcd.h b/src/Display/Lcd/Lcd.h
index 1a9ec27c..50d98784 100644
--- a/src/Display/Lcd/Lcd.h
+++ b/src/Display/Lcd/Lcd.h
@@ -40,7 +40,7 @@ public:
 	uint32_t GetSpiFrequency() const noexcept { return device.GetFrequency(); }
 
 	// Initialize the display
-	void Init(Pin csPin, Pin p_a0Pin, bool csPolarity, uint32_t freq) noexcept;
+	void Init(Pin csPin, Pin p_a0Pin, bool csPolarity, uint32_t freq, uint8_t displayContrastRatio, Pin gatePin = NoPin, bool gatePinPolarity = false) noexcept;
 
 	// Select the font to use for subsequent calls to write() in graphics mode
 	void SetFont(size_t newFont) noexcept;
@@ -152,6 +152,9 @@ protected:
 	SharedSpiClient device;
 	const PixelNumber numRows, numCols;
 	Pin a0Pin;
+	Pin gatePin;
+	uint8_t displayContrastRatio;
+	bool gatePinPolarity;
 	PixelNumber startRow, startCol, endRow, endCol;	// coordinates of the dirty rectangle
 	PixelNumber nextFlushRow;						// which row we need to flush next
 
diff --git a/src/Display/Lcd/ST7567/Lcd7567.cpp b/src/Display/Lcd/ST7567/Lcd7567.cpp
index 866fda0f..8984efec 100644
--- a/src/Display/Lcd/ST7567/Lcd7567.cpp
+++ b/src/Display/Lcd/ST7567/Lcd7567.cpp
@@ -12,6 +12,8 @@
 constexpr unsigned int TILE_WIDTH = 1;
 constexpr unsigned int TILE_HEIGHT = 8;
 
+#define KEEP_GATE_OPEN
+
 Lcd7567::Lcd7567(const LcdFont * const fnts[], size_t nFonts) noexcept
 	: Lcd(64, 128, fnts, nFonts, SpiMode::mode3)
 {
@@ -27,8 +29,17 @@ void Lcd7567::HardwareInit() noexcept
 {
 	pinMode(a0Pin, OUTPUT_LOW);				// set DC/A0 pin to be an output with initial LOW state so as to be in command mode (command: 0, data: 1)
 
-	device.Select();
-	delayMicroseconds(1);
+	// Post-reset wait of 6ms
+	delay(6);
+
+#ifdef KEEP_GATE_OPEN
+	if(gatePin != NoPin) {
+		digitalWrite(gatePin, gatePinPolarity);
+		delayMicroseconds(1);
+	}
+#endif
+
+	SelectDevice();
 
 	SendByte(SystemReset);					// 11100010 System reset
 	SendByte(DisplayOff);
@@ -40,17 +51,23 @@ void Lcd7567::HardwareInit() noexcept
 	SendByte(SetPowerControl | 0x07);		// 00101111 Set power control to enable XV0, V0 and VG charge pumps
 	SendByte(SetBoosterFirst);				// 11111000 Set booster ratio (2-byte command) to 4x
 	SendByte(0x00);
+
+	// David uses 0x06 for the resistor ratio, my display uses 0x03
 	SendByte(SetResistorRatio | 0x06);		// 00100011 Set Vlcd resistor ratio 1+Rb/Ra to 6.5 for the voltage regulator (contrast)
+
+	// Set display contrast
+	// NOTE: boundary checking is not implemented for the contrast parameter yet
+	uint8_t contrast = min<uint8_t>((64.0f * displayContrastRatio) / 100, 0b111111);
 	SendByte(SetVolumeFirst);				// 10000001 Set electronic volume (2-byte command) 6-bit contrast value
-	SendByte(0x18);							// contrast value, EA default: 0x016
+	SendByte(contrast);						// contrast value, EA default: 0x016
+
 	SendByte(SetStaticOff);					// 10101100 Set static indicator off
 
 	// Enable display
 	SendByte(PixelOff);						// 10100100 Set all pixel off
 	SendByte(DisplayOn);					// 10101111 Set display enable to on
 
-	delayMicroseconds(1);
-	device.Deselect();
+	DeselectDevice();
 }
 
 // Flush just some data, returning true if this needs to be called again
@@ -67,9 +84,7 @@ bool Lcd7567::FlushSome() noexcept
 		}
 
 		// Flush that row (which is 8 pixels high)
-		device.Select();
-		delayMicroseconds(1);
-
+		SelectDevice();
 		SetGraphicsAddress(nextFlushRow, startCol);
 		StartDataTransaction();
 
@@ -91,8 +106,7 @@ bool Lcd7567::FlushSome() noexcept
 		}
 
 		EndDataTransaction();
-		delayMicroseconds(1);
-		device.Deselect();
+		DeselectDevice();
 
 		// Check if there is still area to flush
 		if (startRow < endRow)
@@ -128,6 +142,32 @@ inline void Lcd7567::EndDataTransaction() noexcept
 	digitalWrite(a0Pin, false);
 }
 
+void Lcd7567::SelectDevice() noexcept
+{
+#ifndef KEEP_GATE_OPEN
+	if(gatePin != NoPin) {
+		digitalWrite(gatePin, gatePinPolarity);
+		delayMicroseconds(1);
+	}
+#endif
+
+	device.Select();
+	delayMicroseconds(1);
+}
+
+void Lcd7567::DeselectDevice() noexcept
+{
+	delayMicroseconds(1);
+	device.Deselect();
+
+#ifndef KEEP_GATE_OPEN
+	if(gatePin != NoPin) {
+		digitalWrite(gatePin, !gatePinPolarity);
+		delayMicroseconds(1);
+	}
+#endif
+}
+
 void Lcd7567::SendByte(uint8_t byteToSend) noexcept
 {
 	uint8_t data[1] = { byteToSend };
diff --git a/src/Display/Lcd/ST7567/Lcd7567.h b/src/Display/Lcd/ST7567/Lcd7567.h
index 085d22e8..ed916063 100644
--- a/src/Display/Lcd/ST7567/Lcd7567.h
+++ b/src/Display/Lcd/ST7567/Lcd7567.h
@@ -37,6 +37,7 @@ private:
 	constexpr PixelNumber GetTileWidth() const noexcept { return 1; };
 	constexpr PixelNumber GetTileHeight() const noexcept { return 8; };
 #else
+	// NOTE: these are remnants of a more efficient flush routine, not sure this will return
 	constexpr PixelNumber GetTileWidth() const noexcept { return 8; };
 	constexpr PixelNumber GetTileHeight() const noexcept { return 8; };
 #endif
@@ -49,6 +50,8 @@ private:
 	void StartDataTransaction() noexcept;
 	void EndDataTransaction() noexcept;
 	bool FlushRow() noexcept;
+	void SelectDevice() noexcept;
+	void DeselectDevice() noexcept;
 
 	constexpr static uint8_t SystemReset = 0xE2;				// 11100010 System reset
 	constexpr static uint8_t DisplayOff = 0xAE;					// 10101110 Set display enable to off