[mipi][mipi_spi] SWRESET handling improved (#17504)

Co-authored-by: pre-commit-ci-lite[bot] <117423508+pre-commit-ci-lite[bot]@users.noreply.github.com>

esphome/components/mipi/__init__.py

@@ -26,6 +26,7 @@ from esphome.const import (
     CONF_OFFSET_HEIGHT,
     CONF_OFFSET_WIDTH,
     CONF_PAGES,
+    CONF_RESET_PIN,
     CONF_ROTATION,
     CONF_SWAP_XY,
     CONF_TRANSFORM,
@@ -601,12 +602,15 @@ class DriverChip:
         """
         return self.get_default(f"no_{command.lower()}", False)
 
-    def get_sequence(self, config, add_madctl=True) -> tuple[int, ...]:
+    def get_sequence(self, config, add_madctl=True, add_reset=False) -> tuple[int, ...]:
         """
         Create the init sequence for the display.
         Use the default sequence from the model, if any, and append any custom sequence provided in the config.
         Append SLPOUT (if not already in the sequence) and DISPON to the end of the sequence
         MADCTL will be set if add_madctl is True
+        If add_reset is True, a reset is prepended: a software reset when no reset pin
+        is configured (and the model doesn't skip it), followed by a settling delay that
+        both a software and a hardware reset require.
         Returns the init sequence
         """
         sequence = list(self.initsequence or ())
@@ -615,6 +619,15 @@ class DriverChip:
         # Ensure each command is a tuple
         sequence = [x if isinstance(x, tuple) else (x,) for x in sequence]
 
+        if add_reset:
+            reset: list = []
+            # A software reset is only needed when there is no hardware reset pin.
+            if CONF_RESET_PIN not in config and not self.skip_command("SWRESET"):
+                reset.append((SWRESET,))
+            # Both a software and a hardware reset need a settling delay before further commands.
+            reset.append(delay(10))
+            sequence = reset + sequence
+
         # Set pixel format if not already in the custom sequence
         pixel_mode = config[CONF_PIXEL_MODE]
         if not isinstance(pixel_mode, int):
@@ -635,8 +648,13 @@ class DriverChip:
             sequence.append((BRIGHTNESS, brightness))
         # Add a SLPOUT command if required.
         if not self.skip_command("SLPOUT"):
+            # A zero delay will delay until 120ms after reset
+            sequence.append(delay(0))
             sequence.append((SLPOUT,))
+            sequence.append(delay(10))
         sequence.append((DISPON,))
+        # Add a delay here because additional commands may be added after this at runtime.
+        sequence.append(delay(10))
 
         # Flatten the sequence into a list of bytes, with the length of each command
         # or the delay flag inserted where needed

esphome/components/mipi_spi/display.py

@@ -397,7 +397,7 @@ def get_instance(config):
 async def to_code(config):
     model = MODELS[config[CONF_MODEL]]
     var_id = config[CONF_ID]
-    init_sequence = model.get_sequence(config, False)
+    init_sequence = model.get_sequence(config, add_madctl=False, add_reset=True)
     var_id.type, templateargs = get_instance(config)
     var = cg.new_Pvariable(var_id, TemplateArguments(*templateargs))
     cg.add(var.set_init_sequence(init_sequence))

esphome/components/mipi_spi/mipi_spi.h

@@ -13,6 +13,8 @@ constexpr static const char *const TAG = "display.mipi_spi";
 
 // Maximum bytes to log for commands (truncated if larger)
 static constexpr size_t MIPI_SPI_MAX_CMD_LOG_BYTES = 64;
+
+// Command codes for MIPI SPI displays. Not all currently used, kept here for reference.
 static constexpr uint8_t SW_RESET_CMD = 0x01;
 static constexpr uint8_t SLEEP_OUT = 0x11;
 static constexpr uint8_t NORON = 0x13;
@@ -151,14 +153,11 @@ class MipiSpi : public display::Display,
       this->reset_pin_->digital_write(false);
       delay(5);
       this->reset_pin_->digital_write(true);
-    } else {
-      // no reset pin, send software reset command
-      this->write_command_(SW_RESET_CMD);
+      // required delay after reset is already in the init sequence, don't duplicate
     }
 
     // need to know when the display is ready for SLPOUT command - will be 120ms after reset
     auto when = millis() + 120;
-    delay(10);
     size_t index = 0;
     auto &vec = this->init_sequence_;
     while (index != vec.size()) {
@@ -170,6 +169,9 @@ class MipiSpi : public display::Display,
       uint8_t cmd = vec[index++];
       uint8_t x = vec[index++];
       if (x == DELAY_FLAG) {
+        if (cmd == 0) {
+          cmd = clamp_at_least((int) (when - millis()), 0);
+        }
         esph_log_d(TAG, "Delay %dms", cmd);
         delay(cmd);
       } else {
@@ -179,24 +181,9 @@ class MipiSpi : public display::Display,
           this->mark_failed();
           return;
         }
-        switch (cmd) {
-          case SLEEP_OUT: {
-            // are we ready, boots?
-            int duration = when - millis();
-            if (duration > 0) {
-              esph_log_d(TAG, "Sleep %dms", duration);
-              delay(duration);
-            }
-          } break;
-
-          default:
-            break;
-        }
         const auto *ptr = vec.data() + index;
         this->write_command_(cmd, ptr, num_args);
         index += num_args;
-        if (cmd == SLEEP_OUT)
-          delay(10);
       }
     }
     this->reset_params_();

esphome/components/mipi_spi/models/jc.py

@@ -13,6 +13,7 @@ AXS15231 = DriverChip(
     transforms={CONF_MIRROR_X, CONF_MIRROR_Y},
     color_order=MODE_RGB,
     bus_mode=TYPE_QUAD,
+    no_swreset=True,
     initsequence=(
         (0xBB, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x5A, 0xA5),
         (0xC1, 0x33),

tests/component_tests/mipi_spi/test_init.py

@@ -361,7 +361,8 @@ def test_native_generation(
         "mipi_spi::MipiSpiBuffer<uint16_t, mipi_spi::PIXEL_MODE_16, true, mipi_spi::PIXEL_MODE_16, mipi_spi::BUS_TYPE_QUAD, 360, 360, 0, 1, 0, 0, 0, true, 1, 1>()"
         in main_cpp
     )
-    assert "set_init_sequence({240, 1, 8, 242" in main_cpp
+    # A 10ms post-reset delay ({10, 255}) is prepended ahead of the model commands.
+    assert "set_init_sequence({10, 255, 240, 1, 8, 242" in main_cpp
     assert "show_test_card();" in main_cpp
     assert "set_write_only(true);" in main_cpp
 
@@ -377,6 +378,76 @@ def test_lvgl_generation(
         "mipi_spi::MipiSpi<uint16_t, mipi_spi::PIXEL_MODE_16, true, mipi_spi::PIXEL_MODE_16, mipi_spi::BUS_TYPE_SINGLE, 128, 160, 0, 0, 0, 0, 0, true>();"
         in main_cpp
     )
-    assert "set_init_sequence({177, 3, 1, 44, 45, 178" in main_cpp
+    # A 10ms post-reset delay ({10, 255}) is prepended ahead of the model commands.
+    assert "set_init_sequence({10, 255, 177, 3, 1, 44, 45, 178" in main_cpp
     assert "show_test_card();" not in main_cpp
     assert "set_auto_clear(false);" in main_cpp
+
+
+# A 10ms delay (flattened to {10, 0xFF}, where 0xFF is the delay marker byte) is
+# always prepended to the init sequence, since both a software and a hardware reset
+# need to settle before further commands. A custom model has no reset_pin default
+# and does not set no_swreset, so when no reset pin is configured the SWRESET command
+# ({1, 0}: command 0x01 with no parameters) is prepended ahead of that delay.
+_SWRESET_YAML = """
+esphome:
+  name: swreset-test
+esp32:
+  board: esp32-s3-devkitc-1
+  framework:
+    type: esp-idf
+spi:
+  clk_pin: 1
+  mosi_pin: 2
+display:
+  - platform: mipi_spi
+    model: custom
+    id: {display_id}
+    dc_pin: 4
+    cs_pin: 8
+    dimensions:
+      width: 320
+      height: 240
+    init_sequence:
+      - [0xA0, 0x01]
+{reset_line}
+"""
+
+
+def test_swreset_prepended_without_reset_pin(
+    generate_main: Callable[[str | Path], str],
+    tmp_path: Path,
+) -> None:
+    """A model with no reset pin (and no no_swreset) gets SWRESET prepended."""
+    yaml_file = tmp_path / "swreset.yaml"
+    yaml_file.write_text(
+        _SWRESET_YAML.format(display_id="swreset_display", reset_line="")
+    )
+
+    main_cpp = generate_main(yaml_file)
+
+    # SWRESET ({1, 0}) followed by a 10ms delay ({10, 255}) is inserted ahead of
+    # the model's own commands.
+    assert "swreset_display->set_init_sequence({1, 0, 10, 255, 160, 1, 1," in main_cpp
+
+
+def test_swreset_not_prepended_with_reset_pin(
+    generate_main: Callable[[str | Path], str],
+    tmp_path: Path,
+) -> None:
+    """A hardware reset pin performs the reset, so SWRESET must not be prepended.
+
+    The post-reset delay is still required, so the sequence starts with the delay.
+    """
+    yaml_file = tmp_path / "hwreset.yaml"
+    yaml_file.write_text(
+        _SWRESET_YAML.format(
+            display_id="hwreset_display", reset_line="    reset_pin: 5"
+        )
+    )
+
+    main_cpp = generate_main(yaml_file)
+
+    # The delay ({10, 255}) is still present, but no leading SWRESET ({1, 0}).
+    assert "hwreset_display->set_init_sequence({10, 255, 160, 1, 1," in main_cpp
+    assert "hwreset_display->set_init_sequence({1, 0," not in main_cpp