pw_display_driver_ili9341: Simplify command writing

Created DisplayDriverILI9341::WriteCommand() to simplify display
initialization and improve performance as the data and optional
payload can be written in the same transaction.

Change-Id: I33cd9e192e4f0d6c52934e00b6ba3576bba63e91
Reviewed-on: https://pigweed-review.googlesource.com/c/pigweed/experimental/+/112870
Commit-Queue: Chris Mumford <cmumford@google.com>
Reviewed-by: Anthony DiGirolamo <tonymd@google.com>
diff --git a/pw_display_driver_ili9341/display_driver.cc b/pw_display_driver_ili9341/display_driver.cc
index b17c25f..349c50e 100644
--- a/pw_display_driver_ili9341/display_driver.cc
+++ b/pw_display_driver_ili9341/display_driver.cc
@@ -58,28 +58,18 @@
   }
 }
 
-Status DisplayDriverILI9341::SendByte(Device::Transaction& transaction,
-                                      std::byte val) {
-  SetMode(Mode::kData);
-  std::byte buff[1]{val};
-  return transaction.Write(buff);
-}
-
-Status DisplayDriverILI9341::SendShort(Device::Transaction& transaction,
-                                       uint16_t val) {
-  SetMode(Mode::kData);
-  std::byte buff[2]{
-      static_cast<std::byte>(val >> 8),
-      static_cast<std::byte>(val),
-  };
-  return transaction.Write(buff);
-}
-
-Status DisplayDriverILI9341::SendCommand(Device::Transaction& transaction,
-                                         uint8_t cmd) {
+Status DisplayDriverILI9341::WriteCommand(Device::Transaction& transaction,
+                                          const Command& command) {
   SetMode(Mode::kCommand);
-  std::byte buff[1]{static_cast<std::byte>(cmd)};
-  return transaction.Write(buff);
+  std::byte buff[1]{static_cast<std::byte>(command.command)};
+  auto s = transaction.Write(buff);
+  if (!s.ok())
+    return s;
+
+  if (command.command_data.empty())
+    return OkStatus();
+  SetMode(Mode::kData);
+  return transaction.Write(command.command_data);
 }
 
 Status DisplayDriverILI9341::Init() {
@@ -91,159 +81,199 @@
       spi_device_.StartTransaction(ChipSelectBehavior::kPerWriteRead);
 
   // ?
-  SendCommand(transaction, 0xEF);
-  SendByte(transaction, std::byte{0x03});
-  SendByte(transaction, std::byte{0x80});
-  SendByte(transaction, std::byte{0x02});
+  WriteCommand(transaction,
+               {0xEF,
+                std::array<std::byte, 3>{
+                    std::byte{0x03},
+                    std::byte{0x80},
+                    std::byte{0x02},
+                }});
 
   // ?
-  SendCommand(transaction, 0xCF);
-  SendByte(transaction, std::byte{0x00});
-  SendByte(transaction, std::byte{0xC1});
-  SendByte(transaction, std::byte{0x30});
+  WriteCommand(transaction,
+               {0xCF,
+                std::array<std::byte, 3>{
+                    std::byte{0x00},
+                    std::byte{0xC1},
+                    std::byte{0x30},
+                }});
 
   // ?
-  SendCommand(transaction, 0xED);
-  SendByte(transaction, std::byte{0x64});
-  SendByte(transaction, std::byte{0x03});
-  SendByte(transaction, std::byte{0x12});
-  SendByte(transaction, std::byte{0x81});
+  WriteCommand(transaction,
+               {0xED,
+                std::array<std::byte, 4>{
+                    std::byte{0x64},
+                    std::byte{0x03},
+                    std::byte{0x12},
+                    std::byte{0x81},
+                }});
 
   // ?
-  SendCommand(transaction, 0xE8);
-  SendByte(transaction, std::byte{0x85});
-  SendByte(transaction, std::byte{0x00});
-  SendByte(transaction, std::byte{0x78});
+  WriteCommand(transaction,
+               {0xE8,
+                std::array<std::byte, 3>{
+                    std::byte{0x85},
+                    std::byte{0x00},
+                    std::byte{0x78},
+                }});
 
   // ?
-  SendCommand(transaction, 0xCB);
-  SendByte(transaction, std::byte{0x39});
-  SendByte(transaction, std::byte{0x2C});
-  SendByte(transaction, std::byte{0x00});
-  SendByte(transaction, std::byte{0x34});
-  SendByte(transaction, std::byte{0x02});
+  WriteCommand(transaction,
+               {0xCB,
+                std::array<std::byte, 5>{
+                    std::byte{0x39},
+                    std::byte{0x2C},
+                    std::byte{0x00},
+                    std::byte{0x34},
+                    std::byte{0x02},
+                }});
 
   // ?
-  SendCommand(transaction, 0xF7);
-  SendByte(transaction, std::byte{0x20});
+  WriteCommand(transaction, {0xF7, std::array<std::byte, 1>{std::byte{0x20}}});
 
   // ?
-  SendCommand(transaction, 0xEA);
-  SendByte(transaction, std::byte{0x00});
-  SendByte(transaction, std::byte{0x00});
+  WriteCommand(transaction,
+               {0xEA,
+                std::array<std::byte, 2>{
+                    std::byte{0x00},
+                    std::byte{0x00},
+                }});
 
   // Power control
-  SendCommand(transaction, 0xC0);
-  SendByte(transaction, std::byte{0x23});
+  WriteCommand(transaction, {0xC0, std::array<std::byte, 1>{std::byte{0x23}}});
 
   // Power control
-  SendCommand(transaction, 0xC1);
-  SendByte(transaction, std::byte{0x10});
+  WriteCommand(transaction, {0xC1, std::array<std::byte, 1>{std::byte{0x10}}});
 
   // VCM control
-  SendCommand(transaction, 0xC5);
-  SendByte(transaction, std::byte{0x3e});
-  SendByte(transaction, std::byte{0x28});
+  WriteCommand(transaction,
+               {0xC5,
+                std::array<std::byte, 2>{
+                    std::byte{0x3e},
+                    std::byte{0x28},
+                }});
 
   // VCM control
-  SendCommand(transaction, 0xC7);
-  SendByte(transaction, std::byte{0x86});
+  WriteCommand(transaction, {0xC7, std::array<std::byte, 1>{std::byte{0x86}}});
 
-  SendCommand(transaction, ILI9341_MADCTL);
-  // Rotation
-  // SendByte(MADCTL_MX | MADCTL_BGR); // 0
-  // SendByte(MADCTL_MV | MADCTL_BGR); // 1 landscape
-  // SendByte(MADCTL_MY | MADCTL_BGR); // 2
-  SendByte(transaction,
-           MADCTL_MX | MADCTL_MY | MADCTL_MV | MADCTL_BGR);  // 3 landscape
+  constexpr std::byte kMode0 = MADCTL_MX | MADCTL_BGR;
+  constexpr std::byte kMode1 = MADCTL_MV | MADCTL_BGR;
+  constexpr std::byte kMode2 = MADCTL_MY | MADCTL_BGR;
+  constexpr std::byte kMode3 = MADCTL_MX | MADCTL_MY | MADCTL_MV | MADCTL_BGR;
+  WriteCommand(transaction, {ILI9341_MADCTL, std::array<std::byte, 1>{kMode3}});
 
-  SendCommand(transaction, ILI9341_PIXEL_FORMAT_SET);
-  SendByte(transaction, std::byte{0x55});  // 16 bits / pixel
-  // SendByte(0x36);  // 18 bits / pixel
+  constexpr std::byte kPixelFormat16bits = std::byte(0x55);
+  constexpr std::byte kPixelFormat18bits = std::byte(0x36);
+  WriteCommand(
+      transaction,
+      {ILI9341_PIXEL_FORMAT_SET, std::array<std::byte, 1>{kPixelFormat16bits}});
 
   // Frame Control (Normal Mode)
-  SendCommand(transaction, 0xB1);
-  SendByte(transaction, std::byte{0x00});  // division ratio
-  SendByte(transaction, std::byte{0x1F});  // 61 Hz
-  // SendByte(0x1B);  // 70 Hz - default
-  // SendByte(0x18);  // 79 Hz
-  // SendByte(0x10);  // 119 Hz
+  constexpr std::byte kFrameRate61 = std::byte{0x1F};
+  constexpr std::byte kFrameRate70 = std::byte{0x1B};
+  constexpr std::byte kFrameRate79 = std::byte{0x18};
+  constexpr std::byte kFrameRate119 = std::byte{0x10};
+  WriteCommand(transaction,
+               {0xB1,
+                std::array<std::byte, 2>{
+                    std::byte{0x00},  // division ratio
+                    kFrameRate61,
+                }});
 
   // Display Function Control
-  SendCommand(transaction, 0xB6);
-  SendByte(transaction, std::byte{0x08});
-  SendByte(transaction, std::byte{0x82});
-  SendByte(transaction, std::byte{0x27});
+  WriteCommand(transaction,
+               {0xB6,
+                std::array<std::byte, 3>{
+                    std::byte{0x08},
+                    std::byte{0x82},
+                    std::byte{0x27},
+                }});
 
   // Gamma Function Disable?
-  SendCommand(transaction, 0xF2);
-  SendByte(transaction, std::byte{0x00});
+  WriteCommand(transaction, {0xF2, std::array<std::byte, 1>{std::byte{0x00}}});
 
   // Gamma Set
-  SendCommand(transaction, 0x26);
-  SendByte(transaction, std::byte{0x01});
+  WriteCommand(transaction, {0x26, std::array<std::byte, 1>{std::byte{0x01}}});
 
   // Positive Gamma Correction
-  SendCommand(transaction, 0xE0);
-  SendByte(transaction, std::byte{0x0F});
-  SendByte(transaction, std::byte{0x31});
-  SendByte(transaction, std::byte{0x2B});
-  SendByte(transaction, std::byte{0x0C});
-  SendByte(transaction, std::byte{0x0E});
-  SendByte(transaction, std::byte{0x08});
-  SendByte(transaction, std::byte{0x4E});
-  SendByte(transaction, std::byte{0xF1});
-  SendByte(transaction, std::byte{0x37});
-  SendByte(transaction, std::byte{0x07});
-  SendByte(transaction, std::byte{0x10});
-  SendByte(transaction, std::byte{0x03});
-  SendByte(transaction, std::byte{0x0E});
-  SendByte(transaction, std::byte{0x09});
-  SendByte(transaction, std::byte{0x00});
+  WriteCommand(transaction,
+               {0xE0,
+                std::array<std::byte, 15>{
+                    std::byte{0x0F},
+                    std::byte{0x31},
+                    std::byte{0x2B},
+                    std::byte{0x0C},
+                    std::byte{0x0E},
+                    std::byte{0x08},
+                    std::byte{0x4E},
+                    std::byte{0xF1},
+                    std::byte{0x37},
+                    std::byte{0x07},
+                    std::byte{0x10},
+                    std::byte{0x03},
+                    std::byte{0x0E},
+                    std::byte{0x09},
+                    std::byte{0x00},
+                }});
 
   // Negative Gamma Correction
-  SendCommand(transaction, 0xE1);
-  SendByte(transaction, std::byte{0x00});
-  SendByte(transaction, std::byte{0x0E});
-  SendByte(transaction, std::byte{0x14});
-  SendByte(transaction, std::byte{0x03});
-  SendByte(transaction, std::byte{0x11});
-  SendByte(transaction, std::byte{0x07});
-  SendByte(transaction, std::byte{0x31});
-  SendByte(transaction, std::byte{0xC1});
-  SendByte(transaction, std::byte{0x48});
-  SendByte(transaction, std::byte{0x08});
-  SendByte(transaction, std::byte{0x0F});
-  SendByte(transaction, std::byte{0x0C});
-  SendByte(transaction, std::byte{0x31});
-  SendByte(transaction, std::byte{0x36});
-  SendByte(transaction, std::byte{0x0F});
+  WriteCommand(transaction,
+               {0xE1,
+                std::array<std::byte, 15>{
+                    std::byte{0x00},
+                    std::byte{0x0E},
+                    std::byte{0x14},
+                    std::byte{0x03},
+                    std::byte{0x11},
+                    std::byte{0x07},
+                    std::byte{0x31},
+                    std::byte{0xC1},
+                    std::byte{0x48},
+                    std::byte{0x08},
+                    std::byte{0x0F},
+                    std::byte{0x0C},
+                    std::byte{0x31},
+                    std::byte{0x36},
+                    std::byte{0x0F},
+                }});
 
   // Exit Sleep
-  SendCommand(transaction, 0x11);
-
+  WriteCommand(transaction, {0x11, std::array<std::byte, 0>{}});
   pw::spin_delay::WaitMillis(100);
 
   // Display On
-  SendCommand(transaction, 0x29);
-
+  WriteCommand(transaction, {0x29, std::array<std::byte, 0>{}});
   pw::spin_delay::WaitMillis(100);
 
   // Normal display mode on
-  SendCommand(transaction, 0x13);
+  WriteCommand(transaction, {0x13, std::array<std::byte, 0>{}});
 
   // Setup drawing full framebuffers
 
   // Landscape drawing Column Address Set
-  SendCommand(transaction, 0x2A);
-  SendShort(transaction, 0);
-  SendShort(transaction, kDisplayWidth - 1);
+  constexpr uint16_t kMaxColumn = kDisplayWidth - 1;
+  WriteCommand(transaction,
+               {0x2A,
+                std::array<std::byte, 4>{
+                    std::byte{0x0},
+                    std::byte{0x0},
+                    std::byte{kMaxColumn >> 8},    // high byte of short.
+                    std::byte{kMaxColumn & 0xff},  // low byte of short.
+                }});
+
   // Page Address Set
-  SendCommand(transaction, 0x2B);
-  SendShort(transaction, 0);
-  SendShort(transaction, kDisplayHeight - 1);
+  constexpr uint16_t kMaxRow = kDisplayHeight - 1;
+  WriteCommand(transaction,
+               {0x2B,
+                std::array<std::byte, 4>{
+                    std::byte{0x0},
+                    std::byte{0x0},
+                    std::byte{kMaxRow >> 8},    // high byte of short.
+                    std::byte{kMaxRow & 0xff},  // low byte of short.
+                }});
+
   pw::spin_delay::WaitMillis(10);
-  SendCommand(transaction, 0x2C);
+  WriteCommand(transaction, {0x2C, std::array<std::byte, 0>{}});
 
   SetMode(Mode::kData);
   pw::spin_delay::WaitMillis(100);
diff --git a/pw_display_driver_ili9341/public/pw_display_driver_ili9341/display_driver.h b/pw_display_driver_ili9341/public/pw_display_driver_ili9341/display_driver.h
index 804d085..6698524 100644
--- a/pw_display_driver_ili9341/public/pw_display_driver_ili9341/display_driver.h
+++ b/pw_display_driver_ili9341/public/pw_display_driver_ili9341/display_driver.h
@@ -75,17 +75,20 @@
     kCommand,
   };
 
+  // A command and optional data to write to the ILI9341.
+  struct Command {
+    uint8_t command;
+    ConstByteSpan command_data;
+  };
+
   // Toggle the reset GPIO line to reset the display controller.
   Status Reset();
 
   // Set the command/data mode of the display controller.
   void SetMode(Mode mode);
-  // Send a byte to the display controller.
-  Status SendByte(pw::spi::Device::Transaction& transaction, std::byte val);
-  // Send a short to the display controller.
-  Status SendShort(pw::spi::Device::Transaction& transaction, uint16_t val);
-  // Send a command to the display controller.
-  Status SendCommand(pw::spi::Device::Transaction& transaction, uint8_t cmd);
+  // Write the command to the display controller.
+  Status WriteCommand(pw::spi::Device::Transaction& transaction,
+                      const Command& command);
 
   pw::digital_io::DigitalOut& data_cmd_gpio_;  // Pin to specify D/CX mode.
   pw::digital_io::DigitalOut* reset_gpio_;     // Optional to reset controller.