From d21e64e9a29f62ebc826d21e1c413f67d3c56820 Mon Sep 17 00:00:00 2001 From: Dominic Fischer <14130965+Dominaezzz@users.noreply.github.com> Date: Wed, 27 Aug 2025 08:46:05 +0100 Subject: [PATCH] Align `I8080` driver pin configurations with latest guidelines (#3997) --- esp-hal/CHANGELOG.md | 1 + esp-hal/MIGRATING-1.0.0-rc.0.md | 32 +++ esp-hal/src/lcd_cam/lcd/i8080.rs | 278 +++++++++++--------------- hil-test/tests/lcd_cam_i8080.rs | 47 +---- hil-test/tests/lcd_cam_i8080_async.rs | 6 +- qa-test/src/bin/lcd_i8080.rs | 25 +-- 6 files changed, 166 insertions(+), 223 deletions(-) diff --git a/esp-hal/CHANGELOG.md b/esp-hal/CHANGELOG.md index fe5a70d0b..4866bc572 100644 --- a/esp-hal/CHANGELOG.md +++ b/esp-hal/CHANGELOG.md @@ -20,6 +20,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - `aes::{AesBackend, AesContext, dma::AesDmaBackend}`: Work-queue based AES driver (#3880, #3897) - `aes::cipher_modes`, `aes::CipherState` for constructing `AesContext`s (#3895) - `aes::dma::DmaCipherState` so that `AesDma` can properly support cipher modes that require state (IV, nonce, etc.) (#3897) +- Align `I8080` driver pin configurations with latest guidelines (#3997) - Expose cache line configuration (#3946) - ESP32: Expose `psram_vaddr_mode` via `PsramConfig` (#3990) diff --git a/esp-hal/MIGRATING-1.0.0-rc.0.md b/esp-hal/MIGRATING-1.0.0-rc.0.md index 946fd01ee..b3801eb22 100644 --- a/esp-hal/MIGRATING-1.0.0-rc.0.md +++ b/esp-hal/MIGRATING-1.0.0-rc.0.md @@ -153,3 +153,35 @@ let peripherals = esp_hal::init( }) ); ``` + +## `I8080` driver pin configuration changes + +```diff +- let tx_pins = TxEightBits::new( +- peripherals.GPIO9, +- peripherals.GPIO46, +- peripherals.GPIO3, +- peripherals.GPIO8, +- peripherals.GPIO18, +- peripherals.GPIO17, +- peripherals.GPIO16, +- peripherals.GPIO15, +- ); ++ let mut i8080 = I8080::new( + lcd_cam.lcd, + peripherals.DMA_CH0, +- tx_pins, + config, + )? +- .with_ctrl_pins(peripherals.GPIO0, peripherals.GPIO47); ++ .with_dc(peripherals.GPIO0) ++ .with_wrx(peripherals.GPIO47) ++ .with_data0(peripherals.GPIO9) ++ .with_data1(peripherals.GPIO46) ++ .with_data2(peripherals.GPIO3) ++ .with_data3(peripherals.GPIO8) ++ .with_data4(peripherals.GPIO18) ++ .with_data5(peripherals.GPIO17) ++ .with_data6(peripherals.GPIO16) ++ .with_data7(peripherals.GPIO15); +``` \ No newline at end of file diff --git a/esp-hal/src/lcd_cam/lcd/i8080.rs b/esp-hal/src/lcd_cam/lcd/i8080.rs index 72271a19b..78309b2d1 100644 --- a/esp-hal/src/lcd_cam/lcd/i8080.rs +++ b/esp-hal/src/lcd_cam/lcd/i8080.rs @@ -16,28 +16,27 @@ //! //! ```rust, no_run //! # {before_snippet} -//! # use esp_hal::lcd_cam::{LcdCam, lcd::i8080::{Config, I8080, TxEightBits}}; +//! # use esp_hal::lcd_cam::{LcdCam, lcd::i8080::{Config, I8080}}; //! # use esp_hal::dma_tx_buffer; //! # use esp_hal::dma::DmaTxBuf; //! //! # let mut dma_buf = dma_tx_buffer!(32678)?; //! -//! let tx_pins = TxEightBits::new( -//! peripherals.GPIO9, -//! peripherals.GPIO46, -//! peripherals.GPIO3, -//! peripherals.GPIO8, -//! peripherals.GPIO18, -//! peripherals.GPIO17, -//! peripherals.GPIO16, -//! peripherals.GPIO15, -//! ); //! let lcd_cam = LcdCam::new(peripherals.LCD_CAM); //! //! let config = Config::default().with_frequency(Rate::from_mhz(20)); //! -//! let mut i8080 = I8080::new(lcd_cam.lcd, peripherals.DMA_CH0, tx_pins, config)? -//! .with_ctrl_pins(peripherals.GPIO0, peripherals.GPIO47); +//! let mut i8080 = I8080::new(lcd_cam.lcd, peripherals.DMA_CH0, config)? +//! .with_dc(peripherals.GPIO0) +//! .with_wrx(peripherals.GPIO47) +//! .with_data0(peripherals.GPIO9) +//! .with_data1(peripherals.GPIO46) +//! .with_data2(peripherals.GPIO3) +//! .with_data3(peripherals.GPIO8) +//! .with_data4(peripherals.GPIO18) +//! .with_data5(peripherals.GPIO17) +//! .with_data6(peripherals.GPIO16) +//! .with_data7(peripherals.GPIO15); //! //! dma_buf.fill(&[0x55]); //! let transfer = i8080.send(0x3Au8, 0, dma_buf)?; // RGB565 @@ -57,11 +56,7 @@ use crate::{ DriverMode, clock::Clocks, dma::{ChannelTx, DmaError, DmaPeripheral, DmaTxBuffer, PeripheralTxChannel, TxChannelFor}, - gpio::{ - OutputConfig, - OutputSignal, - interconnect::{self, PeripheralOutput}, - }, + gpio::{OutputConfig, OutputSignal, interconnect::PeripheralOutput}, lcd_cam::{ BitOrder, ByteOrder, @@ -102,7 +97,6 @@ where pub fn new( lcd: Lcd<'d, Dm>, channel: impl TxChannelFor>, - mut pins: impl TxPins, config: Config, ) -> Result { let tx_channel = ChannelTx::new(channel.degrade()); @@ -115,7 +109,6 @@ where }; this.apply_config(&config)?; - pins.configure(); Ok(this) } @@ -275,19 +268,21 @@ where self } - /// Configures the control pins for the I8080 interface. - pub fn with_ctrl_pins( - self, - dc: impl PeripheralOutput<'d>, - wrx: impl PeripheralOutput<'d>, - ) -> Self { + /// Associates a DC pin with the I8080 interface. + pub fn with_dc(self, dc: impl PeripheralOutput<'d>) -> Self { let dc = dc.into(); - let wrx = wrx.into(); dc.apply_output_config(&OutputConfig::default()); dc.set_output_enable(true); OutputSignal::LCD_DC.connect_to(&dc); + self + } + + /// Associates a WRX pin with the I8080 interface. + pub fn with_wrx(self, wrx: impl PeripheralOutput<'d>) -> Self { + let wrx = wrx.into(); + wrx.apply_output_config(&OutputConfig::default()); wrx.set_output_enable(true); OutputSignal::LCD_PCLK.connect_to(&wrx); @@ -295,6 +290,96 @@ where self } + fn with_data_pin(self, signal: OutputSignal, pin: impl PeripheralOutput<'d>) -> Self { + let pin = pin.into(); + + pin.apply_output_config(&OutputConfig::default()); + pin.set_output_enable(true); + signal.connect_to(&pin); + + self + } + + /// Associate a DATA 0 pin with the I8080 interface. + pub fn with_data0(self, pin: impl PeripheralOutput<'d>) -> Self { + self.with_data_pin(OutputSignal::LCD_DATA_0, pin) + } + + /// Associate a DATA 1 pin with the I8080 interface. + pub fn with_data1(self, pin: impl PeripheralOutput<'d>) -> Self { + self.with_data_pin(OutputSignal::LCD_DATA_1, pin) + } + + /// Associate a DATA 2 pin with the I8080 interface. + pub fn with_data2(self, pin: impl PeripheralOutput<'d>) -> Self { + self.with_data_pin(OutputSignal::LCD_DATA_2, pin) + } + + /// Associate a DATA 3 pin with the I8080 interface. + pub fn with_data3(self, pin: impl PeripheralOutput<'d>) -> Self { + self.with_data_pin(OutputSignal::LCD_DATA_3, pin) + } + + /// Associate a DATA 4 pin with the I8080 interface. + pub fn with_data4(self, pin: impl PeripheralOutput<'d>) -> Self { + self.with_data_pin(OutputSignal::LCD_DATA_4, pin) + } + + /// Associate a DATA 5 pin with the I8080 interface. + pub fn with_data5(self, pin: impl PeripheralOutput<'d>) -> Self { + self.with_data_pin(OutputSignal::LCD_DATA_5, pin) + } + + /// Associate a DATA 6 pin with the I8080 interface. + pub fn with_data6(self, pin: impl PeripheralOutput<'d>) -> Self { + self.with_data_pin(OutputSignal::LCD_DATA_6, pin) + } + + /// Associate a DATA 7 pin with the I8080 interface. + pub fn with_data7(self, pin: impl PeripheralOutput<'d>) -> Self { + self.with_data_pin(OutputSignal::LCD_DATA_7, pin) + } + + /// Associate a DATA 8 pin with the I8080 interface. + pub fn with_data8(self, pin: impl PeripheralOutput<'d>) -> Self { + self.with_data_pin(OutputSignal::LCD_DATA_8, pin) + } + + /// Associate a DATA 9 pin with the I8080 interface. + pub fn with_data9(self, pin: impl PeripheralOutput<'d>) -> Self { + self.with_data_pin(OutputSignal::LCD_DATA_9, pin) + } + + /// Associate a DATA 10 pin with the I8080 interface. + pub fn with_data10(self, pin: impl PeripheralOutput<'d>) -> Self { + self.with_data_pin(OutputSignal::LCD_DATA_10, pin) + } + + /// Associate a DATA 11 pin with the I8080 interface. + pub fn with_data11(self, pin: impl PeripheralOutput<'d>) -> Self { + self.with_data_pin(OutputSignal::LCD_DATA_11, pin) + } + + /// Associate a DATA 12 pin with the I8080 interface. + pub fn with_data12(self, pin: impl PeripheralOutput<'d>) -> Self { + self.with_data_pin(OutputSignal::LCD_DATA_12, pin) + } + + /// Associate a DATA 13 pin with the I8080 interface. + pub fn with_data13(self, pin: impl PeripheralOutput<'d>) -> Self { + self.with_data_pin(OutputSignal::LCD_DATA_13, pin) + } + + /// Associate a DATA 14 pin with the I8080 interface. + pub fn with_data14(self, pin: impl PeripheralOutput<'d>) -> Self { + self.with_data_pin(OutputSignal::LCD_DATA_14, pin) + } + + /// Associate a DATA 15 pin with the I8080 interface. + pub fn with_data15(self, pin: impl PeripheralOutput<'d>) -> Self { + self.with_data_pin(OutputSignal::LCD_DATA_15, pin) + } + /// Sends a command and data to the LCD using DMA. /// /// Passing a `Command` will make this an 8-bit transfer and a @@ -613,142 +698,3 @@ impl From for Command { Command::One(value) } } - -/// Represents a group of 8 output pins configured for 8-bit parallel data -/// transmission. -pub struct TxEightBits<'d> { - pins: [interconnect::OutputSignal<'d>; 8], -} - -impl<'d> TxEightBits<'d> { - #[expect(clippy::too_many_arguments)] - /// Creates a new `TxEightBits` instance with the provided output pins. - pub fn new( - pin_0: impl PeripheralOutput<'d>, - pin_1: impl PeripheralOutput<'d>, - pin_2: impl PeripheralOutput<'d>, - pin_3: impl PeripheralOutput<'d>, - pin_4: impl PeripheralOutput<'d>, - pin_5: impl PeripheralOutput<'d>, - pin_6: impl PeripheralOutput<'d>, - pin_7: impl PeripheralOutput<'d>, - ) -> Self { - Self { - pins: [ - pin_0.into(), - pin_1.into(), - pin_2.into(), - pin_3.into(), - pin_4.into(), - pin_5.into(), - pin_6.into(), - pin_7.into(), - ], - } - } -} - -impl TxPins for TxEightBits<'_> { - fn configure(&mut self) { - const SIGNALS: [OutputSignal; 8] = [ - OutputSignal::LCD_DATA_0, - OutputSignal::LCD_DATA_1, - OutputSignal::LCD_DATA_2, - OutputSignal::LCD_DATA_3, - OutputSignal::LCD_DATA_4, - OutputSignal::LCD_DATA_5, - OutputSignal::LCD_DATA_6, - OutputSignal::LCD_DATA_7, - ]; - - for (pin, signal) in self.pins.iter().zip(SIGNALS.into_iter()) { - pin.apply_output_config(&OutputConfig::default()); - pin.set_output_enable(true); - signal.connect_to(pin); - } - } -} - -/// Represents a group of 16 output pins configured for 16-bit parallel data -/// transmission. -pub struct TxSixteenBits<'d> { - pins: [interconnect::OutputSignal<'d>; 16], -} - -impl<'d> TxSixteenBits<'d> { - #[expect(clippy::too_many_arguments)] - /// Creates a new `TxSixteenBits` instance with the provided output pins. - pub fn new( - pin_0: impl PeripheralOutput<'d>, - pin_1: impl PeripheralOutput<'d>, - pin_2: impl PeripheralOutput<'d>, - pin_3: impl PeripheralOutput<'d>, - pin_4: impl PeripheralOutput<'d>, - pin_5: impl PeripheralOutput<'d>, - pin_6: impl PeripheralOutput<'d>, - pin_7: impl PeripheralOutput<'d>, - pin_8: impl PeripheralOutput<'d>, - pin_9: impl PeripheralOutput<'d>, - pin_10: impl PeripheralOutput<'d>, - pin_11: impl PeripheralOutput<'d>, - pin_12: impl PeripheralOutput<'d>, - pin_13: impl PeripheralOutput<'d>, - pin_14: impl PeripheralOutput<'d>, - pin_15: impl PeripheralOutput<'d>, - ) -> Self { - Self { - pins: [ - pin_0.into(), - pin_1.into(), - pin_2.into(), - pin_3.into(), - pin_4.into(), - pin_5.into(), - pin_6.into(), - pin_7.into(), - pin_8.into(), - pin_9.into(), - pin_10.into(), - pin_11.into(), - pin_12.into(), - pin_13.into(), - pin_14.into(), - pin_15.into(), - ], - } - } -} - -impl TxPins for TxSixteenBits<'_> { - fn configure(&mut self) { - const SIGNALS: [OutputSignal; 16] = [ - OutputSignal::LCD_DATA_0, - OutputSignal::LCD_DATA_1, - OutputSignal::LCD_DATA_2, - OutputSignal::LCD_DATA_3, - OutputSignal::LCD_DATA_4, - OutputSignal::LCD_DATA_5, - OutputSignal::LCD_DATA_6, - OutputSignal::LCD_DATA_7, - OutputSignal::LCD_DATA_8, - OutputSignal::LCD_DATA_9, - OutputSignal::LCD_DATA_10, - OutputSignal::LCD_DATA_11, - OutputSignal::LCD_DATA_12, - OutputSignal::LCD_DATA_13, - OutputSignal::LCD_DATA_14, - OutputSignal::LCD_DATA_15, - ]; - - for (pin, signal) in self.pins.iter_mut().zip(SIGNALS.into_iter()) { - pin.apply_output_config(&OutputConfig::default()); - pin.set_output_enable(true); - signal.connect_to(pin); - } - } -} - -#[doc(hidden)] -pub trait TxPins { - fn configure(&mut self); -} diff --git a/hil-test/tests/lcd_cam_i8080.rs b/hil-test/tests/lcd_cam_i8080.rs index 95ad7e956..ed69e3033 100644 --- a/hil-test/tests/lcd_cam_i8080.rs +++ b/hil-test/tests/lcd_cam_i8080.rs @@ -14,7 +14,7 @@ use esp_hal::{ lcd_cam::{ BitOrder, LcdCam, - lcd::i8080::{Command, Config, I8080, TxEightBits, TxSixteenBits}, + lcd::i8080::{Command, Config, I8080}, }, pcnt::{ Pcnt, @@ -75,12 +75,9 @@ mod tests { #[test] fn test_i8080_8bit(ctx: Context<'static>) { - let pins = TxEightBits::new(NoPin, NoPin, NoPin, NoPin, NoPin, NoPin, NoPin, NoPin); - let i8080 = I8080::new( ctx.lcd_cam.lcd, ctx.dma, - pins, Config::default().with_frequency(Rate::from_mhz(20)), ) .unwrap(); @@ -129,26 +126,17 @@ mod tests { .channel0 .set_input_mode(EdgeMode::Hold, EdgeMode::Increment); - let pins = TxEightBits::new( - unit0_signal, - unit1_signal, - unit2_signal, - unit3_signal, - NoPin, - NoPin, - NoPin, - NoPin, - ); - let mut i8080 = I8080::new( ctx.lcd_cam.lcd, ctx.dma, - pins, Config::default().with_frequency(Rate::from_mhz(20)), ) .unwrap() .with_cs(cs_signal) - .with_ctrl_pins(NoPin, NoPin); + .with_data0(unit0_signal) + .with_data1(unit1_signal) + .with_data2(unit2_signal) + .with_data3(unit3_signal); // explicitly drop the camera half to see if it disables clocks (unexpectedly, // I8080 should keep it alive) @@ -243,34 +231,17 @@ mod tests { .channel0 .set_input_mode(EdgeMode::Hold, EdgeMode::Increment); - let pins = TxSixteenBits::new( - NoPin, - NoPin, - NoPin, - unit0_signal, - NoPin, - NoPin, - NoPin, - unit1_signal, - NoPin, - NoPin, - NoPin, - unit2_signal, - NoPin, - NoPin, - NoPin, - unit3_signal, - ); - let mut i8080 = I8080::new( ctx.lcd_cam.lcd, ctx.dma, - pins, Config::default().with_frequency(Rate::from_mhz(20)), ) .unwrap() .with_cs(cs_signal) - .with_ctrl_pins(NoPin, NoPin); + .with_data3(unit0_signal) + .with_data7(unit1_signal) + .with_data11(unit2_signal) + .with_data15(unit3_signal); // This is to make the test values look more intuitive. i8080.set_bit_order(BitOrder::Inverted); diff --git a/hil-test/tests/lcd_cam_i8080_async.rs b/hil-test/tests/lcd_cam_i8080_async.rs index 22ff19267..ebcaf4aa3 100644 --- a/hil-test/tests/lcd_cam_i8080_async.rs +++ b/hil-test/tests/lcd_cam_i8080_async.rs @@ -10,10 +10,9 @@ use esp_hal::{ Async, dma::DmaTxBuf, dma_buffers, - gpio::NoPin, lcd_cam::{ LcdCam, - lcd::i8080::{Command, Config, I8080, TxEightBits}, + lcd::i8080::{Command, Config, I8080}, }, peripherals::DMA_CH0, time::Rate, @@ -50,12 +49,9 @@ mod tests { #[test] async fn test_i8080_8bit(ctx: Context<'static>) { - let pins = TxEightBits::new(NoPin, NoPin, NoPin, NoPin, NoPin, NoPin, NoPin, NoPin); - let i8080 = I8080::new( ctx.lcd_cam.lcd, ctx.dma, - pins, Config::default().with_frequency(Rate::from_mhz(20)), ) .unwrap(); diff --git a/qa-test/src/bin/lcd_i8080.rs b/qa-test/src/bin/lcd_i8080.rs index 1b492c0b9..5e0d630f0 100644 --- a/qa-test/src/bin/lcd_i8080.rs +++ b/qa-test/src/bin/lcd_i8080.rs @@ -31,7 +31,7 @@ use esp_hal::{ gpio::{Input, InputConfig, Level, Output, OutputConfig, Pull}, lcd_cam::{ LcdCam, - lcd::i8080::{Config, I8080, TxEightBits}, + lcd::i8080::{Config, I8080}, }, main, time::Rate, @@ -59,26 +59,23 @@ fn main() -> ! { let mut reset = Output::new(lcd_reset, Level::Low, OutputConfig::default()); let tear_effect = Input::new(lcd_te, InputConfig::default().with_pull(Pull::None)); - let tx_pins = TxEightBits::new( - peripherals.GPIO9, - peripherals.GPIO46, - peripherals.GPIO3, - peripherals.GPIO8, - peripherals.GPIO18, - peripherals.GPIO17, - peripherals.GPIO16, - peripherals.GPIO15, - ); - let lcd_cam = LcdCam::new(peripherals.LCD_CAM); let i8080 = I8080::new( lcd_cam.lcd, peripherals.DMA_CH0, - tx_pins, Config::default().with_frequency(Rate::from_mhz(20)), ) .unwrap() - .with_ctrl_pins(lcd_rs, lcd_wr); + .with_dc(lcd_rs) + .with_wrx(lcd_wr) + .with_data0(peripherals.GPIO9) + .with_data1(peripherals.GPIO46) + .with_data2(peripherals.GPIO3) + .with_data3(peripherals.GPIO8) + .with_data4(peripherals.GPIO18) + .with_data5(peripherals.GPIO17) + .with_data6(peripherals.GPIO16) + .with_data7(peripherals.GPIO15); // Note: This isn't provided in the HAL since different drivers may require // different considerations, like how to manage the CS pin, the CD pin,