From 900e242adc7c9accef14a505fee86ffba61fadd2 Mon Sep 17 00:00:00 2001 From: Jorj Bauer Date: Wed, 2 Feb 2022 07:17:05 -0500 Subject: [PATCH] dma working again w/ dynamic objects --- README.md | 7 +++--- apple/appledisplay.cpp | 1 - physicaldisplay.cpp | 18 -------------- teensy/ILI9341_wrap.cpp | 14 +++++++---- teensy/ILI9341_wrap.h | 1 - teensy/RA8875_t4.cpp | 39 ++++++++++++++++++++++++------ teensy/RA8875_t4.h | 11 ++++++--- teensy/basedisplay.h | 3 --- teensy/teensy-display.cpp | 51 +++++++++++++++++++++++++++------------ teensy/teensy-display.h | 1 - teensy/teensy.ino | 1 + 11 files changed, 87 insertions(+), 60 deletions(-) diff --git a/README.md b/README.md index 7cac263..4f48f72 100644 --- a/README.md +++ b/README.md @@ -85,10 +85,9 @@ You'll also need the ILI9341_t3n library from https://github.com/KurtE/ILI9341_t3n/ -I'm using it at tag f1bfb81825c60e39e011e502fe5c39a04305e1dc - not -because that tag is special, but because that's when I checked out the -repo. I haven't tested newer code and if you have problems, you'll -want to roll back to that tag. +As of this writing, the master branch does not work for Aiie; but the +branch "dma_new_fix" is fine. I'd recommend checking out that branch +if it exists. # Running (on the Teensy) diff --git a/apple/appledisplay.cpp b/apple/appledisplay.cpp index 145f05b..962948e 100644 --- a/apple/appledisplay.cpp +++ b/apple/appledisplay.cpp @@ -460,7 +460,6 @@ void AppleDisplay::redraw40ColumnText(uint8_t startingY) // Only draw onscreen locations if (row >= startingY && col <= 39 && row <= 23) { const uint8_t *cptr = xlateChar(mmu->readDirect(addr, 0), &invert); - for (uint8_t y2 = 0; y2<8; y2++) { uint8_t d = *(cptr + y2); for (uint8_t x2 = 0; x2 < 7; x2++) { diff --git a/physicaldisplay.cpp b/physicaldisplay.cpp index 520d820..33a7620 100644 --- a/physicaldisplay.cpp +++ b/physicaldisplay.cpp @@ -5,24 +5,6 @@ #include "font.h" -const uint16_t loresPixelColors[16] = { 0x0000, // 0 black - 0xC006, // 1 magenta - 0x0010, // 2 dark blue - 0xA1B5, // 3 purple - 0x0480, // 4 dark green - 0x6B4D, // 5 dark grey - 0x1B9F, // 6 med blue - 0x0DFD, // 7 light blue - 0x92A5, // 8 brown - 0xF8C5, // 9 orange - 0x9555, // 10 light gray - 0xFCF2, // 11 pink - 0x07E0, // 12 green - 0xFFE0, // 13 yellow - 0x87F0, // 14 aqua - 0xFFFF // 15 white -}; - void PhysicalDisplay::drawCharacter(uint8_t mode, uint16_t x, uint16_t y, char c) { int8_t xsize = 8, diff --git a/teensy/ILI9341_wrap.cpp b/teensy/ILI9341_wrap.cpp index 3ece15d..195f8db 100644 --- a/teensy/ILI9341_wrap.cpp +++ b/teensy/ILI9341_wrap.cpp @@ -60,18 +60,19 @@ bool ILI9341_Wrap::updateScreenAsync(bool update_cont) void ILI9341_Wrap::drawPixel(int16_t x, int16_t y, uint16_t color) { + if (x>=320 || y>=240) + return; + frame_buffer[y*ILI9341_WIDTH+x] = color; } -void ILI9341_Wrap::drawPixel(int16_t x, int16_t y, uint8_t color) -{ - frame_buffer[y*ILI9341_WIDTH+x] = _332To565(color); -} - // The 9341 is half the width we need, so this jumps through hoops to // reduce the resolution in a way that's reasonable by blending pixels void ILI9341_Wrap::cacheApplePixel(uint16_t x, uint16_t y, uint16_t color) { + if (x>=560 || y>=192) + return; + if (x&1) { uint16_t origColor =frame_buffer[(y+SCREENINSET_9341_Y)*ILI9341_WIDTH+(x>>1)+SCREENINSET_9341_X]; if (g_displayType == m_blackAndWhite) { @@ -98,6 +99,9 @@ void ILI9341_Wrap::cacheApplePixel(uint16_t x, uint16_t y, uint16_t color) void ILI9341_Wrap::cacheDoubleWideApplePixel(uint16_t x, uint16_t y, uint16_t color16) { + if (x>=280 || y>=192) + return; + frame_buffer[(y+SCREENINSET_9341_Y)*ILI9341_WIDTH + (x) + SCREENINSET_9341_X] = color16; } diff --git a/teensy/ILI9341_wrap.h b/teensy/ILI9341_wrap.h index 54e9ed8..07e460c 100644 --- a/teensy/ILI9341_wrap.h +++ b/teensy/ILI9341_wrap.h @@ -27,7 +27,6 @@ class ILI9341_Wrap : public BaseDisplay { virtual bool updateScreenAsync(bool update_cont = false); virtual void drawPixel(int16_t x, int16_t y, uint16_t color); - virtual void drawPixel(int16_t x, int16_t y, uint8_t color); virtual void cacheApplePixel(uint16_t x, uint16_t y, uint16_t color); virtual void ILI9341_Wrap::cacheDoubleWideApplePixel(uint16_t x, uint16_t y, uint16_t color16); diff --git a/teensy/RA8875_t4.cpp b/teensy/RA8875_t4.cpp index 0f797bf..43129f3 100644 --- a/teensy/RA8875_t4.cpp +++ b/teensy/RA8875_t4.cpp @@ -87,6 +87,10 @@ * https://github-wiki-see.page/m/TeensyUser/doc/wiki/Memory-Mapping */ +// Static DMA objects that we need in RAM1 +DMASetting _dmasettings[12]; +DMAChannel _dmatx; + // at 8bpp, each pixel is 1 byte #define COUNT_PIXELS_WRITE (RA8875_WIDTH * RA8875_HEIGHT) @@ -282,6 +286,8 @@ void RA8875_t4::_initializeTFT() void RA8875_t4::setFrameBuffer(uint8_t *frame_buffer) { + Serial.print("fb 0x"); + Serial.println((uint32_t)frame_buffer, HEX); _pfbtft = frame_buffer; _dma_state &= ~RA8875_DMA_INIT; } @@ -382,7 +388,10 @@ bool RA8875_t4::updateScreenAsync(bool update_cont) _dma_state &= ~RA8875_DMA_CONT; } _dma_state |= RA8875_DMA_ACTIVE; - + + // Make sure the dma settings are flushed. Otherwise bad things happen. + if ((uint32_t)_dmasettings >= 0x20200000u) + arm_dcache_flush(_dmasettings, sizeof(DMASetting)*12); // FIXME constant return true; } @@ -399,18 +408,27 @@ uint8_t _color16To8bpp(uint16_t color) { void RA8875_t4::drawPixel(int16_t x, int16_t y, uint16_t color) { - // FIXME: bounds checking + if (x>=800 || y>=480) { + Serial.print("^ "); + Serial.print(x); + Serial.print(" "); + Serial.println(y); + return; + } + _pfbtft[y*RA8875_WIDTH+x] = _color16To8bpp(color); } -void RA8875_t4::drawPixel(int16_t x, int16_t y, uint8_t color) -{ - // FIXME: bounds checking - _pfbtft[y*RA8875_WIDTH+x] = color; -} - void RA8875_t4::cacheApplePixel(uint16_t x, uint16_t y, uint16_t color) { + if (x>=560 || y>=192) { + Serial.print("! "); + Serial.print(x); + Serial.print(" "); + Serial.println(y); + return; + } + // The 8875 display doubles vertically uint c8 = _565To332(color); for (int yoff=0; yoff<2; yoff++) { @@ -420,6 +438,11 @@ void RA8875_t4::cacheApplePixel(uint16_t x, uint16_t y, uint16_t color) void RA8875_t4::cacheDoubleWideApplePixel(uint16_t x, uint16_t y, uint16_t color16) { + if (x>=280 || y>=192) { + Serial.println("@"); + return; + } + // The RA8875 doubles Apple's pixels. for (int yoff=0; yoff<2; yoff++) { for (int xoff=0; xoff<2; xoff++) { diff --git a/teensy/RA8875_t4.h b/teensy/RA8875_t4.h index 95b503c..09a486c 100644 --- a/teensy/RA8875_t4.h +++ b/teensy/RA8875_t4.h @@ -37,7 +37,6 @@ class RA8875_t4 : public BaseDisplay { virtual bool updateScreenAsync(bool update_cont = false); virtual void drawPixel(int16_t x, int16_t y, uint16_t color); - virtual void drawPixel(int16_t x, int16_t y, uint8_t color); virtual void cacheApplePixel(uint16_t x, uint16_t y, uint16_t color16); virtual void cacheDoubleWideApplePixel(uint16_t x, uint16_t y, uint16_t color16); @@ -81,9 +80,13 @@ private: uint32_t _spi_clock_read; uint32_t _clock; // current clock, used in starting transactions (b/c we have to slow down sometimes) - // DMA stuff - DMASetting _dmasettings[12]; - DMAChannel _dmatx; + // DMA stuff. The _dmasettings[] and _dmatx can't be member + // variables. If they are, then the object will work if it's + // statically allocated; but dynamically allocated RA8875_t4 objects + // would be malloc()'d in RAM2, which is a problem for DMA. + // So instead they're static globals in the module. + // DMASetting _dmasettings[12]; + // DMAChannel _dmatx; uint32_t _spi_fcr_save; uint8_t *_pfbtft; volatile uint8_t _dma_state; diff --git a/teensy/basedisplay.h b/teensy/basedisplay.h index e107f84..b4ec116 100644 --- a/teensy/basedisplay.h +++ b/teensy/basedisplay.h @@ -1,8 +1,6 @@ #ifndef __BASE_DISPLAY_H #define __BASE_DISPLAY_H -const uint16_t loresPixelColors[16]; - #define RGBto565(r,g,b) ((((r) & 0xF8) << 8) | (((g) & 0xFC) << 3) | ((b) >> 3)) #define _565toR(c) ( ((c) & 0xF800) >> 8 ) #define _565toG(c) ( ((c) & 0x07E0) >> 3 ) @@ -30,7 +28,6 @@ class BaseDisplay { virtual bool updateScreenAsync(bool update_cont = false) = 0; virtual void drawPixel(int16_t x, int16_t y, uint16_t color) = 0; - virtual void drawPixel(int16_t x, int16_t y, uint8_t color) = 0; // Apple interface methods virtual void cacheApplePixel(uint16_t x, uint16_t y, uint16_t color) = 0; diff --git a/teensy/teensy-display.cpp b/teensy/teensy-display.cpp index c24f200..a180437 100644 --- a/teensy/teensy-display.cpp +++ b/teensy/teensy-display.cpp @@ -25,6 +25,24 @@ uint16_t *dmaBuffer16 = NULL; #define PIN_MISO 1 #define PIN_SCK 27 +const uint16_t loresPixelColors[16] = { 0x0000, // 0 black + 0xC006, // 1 magenta + 0x0010, // 2 dark blue + 0xA1B5, // 3 purple + 0x0480, // 4 dark green + 0x6B4D, // 5 dark grey + 0x1B9F, // 6 med blue + 0x0DFD, // 7 light blue + 0x92A5, // 8 brown + 0xF8C5, // 9 orange + 0x9555, // 10 light gray + 0xFCF2, // 11 pink + 0x07E0, // 12 green + 0xFFE0, // 13 yellow + 0x87F0, // 14 aqua + 0xFFFF // 15 white +}; + TeensyDisplay::TeensyDisplay() { driveIndicator[0] = driveIndicator[1] = false; @@ -38,11 +56,15 @@ TeensyDisplay::TeensyDisplay() pinMode(11, INPUT); digitalWrite(11, HIGH); // turn on pull-up - if (digitalRead(11)) { + if (!digitalRead(11)) { // Default: use older, smaller but faster, ILI display if pin 11 is not connected to ground Serial.println(" using ILI9341 display"); - dmaBuffer16 = (uint16_t *)malloc((320*240)*2+32); // malloc() happens in the DMAMEM area (RAM2) FIXME *** CONSTANTS -- and does the +32 align properly? + dmaBuffer16 = (uint16_t *)malloc((320*240)*2+32); // malloc() happens in the DMAMEM area (RAM2) + // And we have to be sure dmaBuffer16 is 32-byte aligned for DMA purposes + // so we intentionally alloc'd an extra 32 bytes in order to shift here + dmaBuffer16 = (uint16_t *)(((uintptr_t)dmaBuffer16 + 32) & + ~((uintptr_t)(31))); tft = new ILI9341_Wrap(PIN_CS, PIN_RST, PIN_MOSI, PIN_SCK, PIN_MISO, PIN_DC); @@ -60,8 +82,12 @@ TeensyDisplay::TeensyDisplay() // If someone grounded pin 11, then use the new RA8875 display Serial.println(" using RA8875 display"); - dmaBuffer = (uint8_t *)malloc(800*480+32); // malloc() happens in the DMAMEM area (RAM2) FIXME *** CONSTANTS -- and does the +32 align properly? - + dmaBuffer = (uint8_t *)malloc(800*480+32); // malloc() happens in the DMAMEM area (RAM2) + // And we have to be sure dmaBuffer is 32-byte aligned for DMA purposes + // so we intentionally alloc'd an extra 32 bytes in order to shift here + dmaBuffer = (uint8_t *)(((uintptr_t)dmaBuffer + 32) & + ~((uintptr_t)(31))); + tft = new RA8875_t4(PIN_CS, PIN_RST, PIN_MOSI, PIN_SCK, PIN_MISO); // Load the 8875 images @@ -79,16 +105,17 @@ TeensyDisplay::TeensyDisplay() tft->setFrameBuffer((uint8_t *)dmaBuffer); } - Serial.print("before "); - Serial.println((uint32_t)tft); - Serial.print("after "); - Serial.println((uint32_t)tft); tft->fillWindow(); - Serial.println("finished filling"); } TeensyDisplay::~TeensyDisplay() { + /* FIXME: we mucked with these after alloc to align them, so we can't free them from their offset addresses; need to keep track of the original malloc'd address instead + if (dmaBuffer) + free(dmaBuffer); + if (dmaBuffer16) + free(dmaBuffer16); + */ } // Take one of the abstracted image constants, figure out which one it @@ -125,7 +152,6 @@ void TeensyDisplay::drawImageOfSizeAt(const uint8_t *img, uint16_t wherex, uint16_t wherey) { uint8_t r, g, b; - uint8_t *p = img; for (uint16_t y=0; ycacheApplePixel(x,y,loresPixelColors[color]); } -void TeensyDisplay::cacheDoubleWidePixel(uint16_t x, uint16_t y, uint16_t color16) -{ - tft->cacheDoubleWideApplePixel(x, y, color16); -} - // "DoubleWide" means "please double the X because I'm in low-res // width mode". void TeensyDisplay::cacheDoubleWidePixel(uint16_t x, uint16_t y, uint8_t color) diff --git a/teensy/teensy-display.h b/teensy/teensy-display.h index 9011072..612c6cf 100644 --- a/teensy/teensy-display.h +++ b/teensy/teensy-display.h @@ -24,7 +24,6 @@ class TeensyDisplay : public PhysicalDisplay { virtual void drawUIImage(uint8_t imageIdx); virtual void drawImageOfSizeAt(const uint8_t *img, uint16_t sizex, uint16_t sizey, uint16_t wherex, uint16_t wherey); - void cacheDoubleWidePixel(uint16_t x, uint16_t y, uint16_t color16); virtual void cacheDoubleWidePixel(uint16_t x, uint16_t y, uint8_t color); virtual void cachePixel(uint16_t x, uint16_t y, uint8_t color); diff --git a/teensy/teensy.ino b/teensy/teensy.ino index 8a55035..63a20ba 100644 --- a/teensy/teensy.ino +++ b/teensy/teensy.ino @@ -358,6 +358,7 @@ void runDisplay(uint32_t now) } if (!g_biosInterrupt) { + // FIXME this needs some love. It could be efficient, but parts are removed, so it's doing duplicative work. g_ui->blit(); g_vm->vmdisplay->lockDisplay(); if (g_vm->vmdisplay->needsRedraw()) { // necessary for the VM to redraw