Skip to content

Commit

Permalink
Re-implement readRect to avoid RX FIFO overflow
Browse files Browse the repository at this point in the history
(Ported from ILI9341_t3, with appropriate changes to use SPIN)

This fixes PaulStoffregen/ILI9341_t3#29

The implementation of readRect had an issue where it would not correctly read
the last pixel from the screen. This was caused by an overflow in the RX FIFO
while waiting for the EOQ flag in the last transmitted byte. The loop only
counted transmitted bytes, so it would fill up the TX FIFO and not wait until
the transfer actually happened. Waiting for EOQ competed the queued transfers
and overflowed the RX FIFO since nothing was reading from it.

The new implementation explicitly tracks the number of RX and TX bytes to
ensure that all bytes are correctly transmitted and received. A few ideas
have been borrowed from KurtE/ILI9341_t3n as well:
* Each byte is received independently and stored in a 3-byte array, which is
  converted and written to the buffer once all 3 colors have been received.
* The TX FIFO is kept full throughout the function to avoid the delay between
  the dummy byte and the first pixel byte. Instead of using EOQ for the dummy
  byte, the results from queued commands and the dummy byte are skipped.
  • Loading branch information
mudaltsov committed Dec 3, 2016
1 parent 2e6a104 commit 8ae598a
Showing 1 changed file with 34 additions and 44 deletions.
78 changes: 34 additions & 44 deletions ILI9341_t3n.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -800,65 +800,55 @@ void ILI9341_t3n::readRect(int16_t x, int16_t y, int16_t w, int16_t h, uint16_t

if (_miso == 0xff) return; // bail if not valid miso

uint8_t dummy __attribute__((unused));
// uint8_t r,g,b;
uint8_t rgb[3]; // read into an array...
uint8_t rgb_index = 0;
uint32_t c = w * h;
uint8_t rgb[3]; // RGB bytes received from the display
uint8_t rgbIdx = 0;
uint32_t txCount = w * h * 3; // number of bytes we will transmit to the display
uint32_t rxCount = txCount; // number of bytes we will receive back from the display

_pspin->beginTransaction(SPISettings(ILI9341_SPICLOCK_READ, MSBFIRST, SPI_MODE0));

setAddr(x, y, x+w-1, y+h-1);
writecommand_cont(ILI9341_RAMRD); // read from RAM
_pspin->waitTransmitComplete();
c *= 3; // number of bytes we will transmit to the display
// First remove all queued up receive entries.
while ((_pkinetisk_spi->SR & 0xf0)) {
dummy = _pkinetisk_spi->POPR; // Read a DUMMY byte but only once
}
_pkinetisk_spi->PUSHR = 0 | (pcs_data << 16) | SPI_PUSHR_CTAS(0)| SPI_PUSHR_CONT | SPI_PUSHR_EOQ;

// transmit a DUMMY byte before the color bytes
_pkinetisk_spi->PUSHR = 0 | (pcs_data << 16) | SPI_PUSHR_CTAS(0)| SPI_PUSHR_CONT;

while ((_pkinetisk_spi->SR & SPI_SR_EOQF) == 0) {
// maybe keep queue with something in it, while waiting for that EOQF
if ((_pkinetisk_spi->SR & (15 << 12)) == 0) {
_pkinetisk_spi->PUSHR = READ_PIXEL_PUSH_BYTE | (pcs_data << 16) | SPI_PUSHR_CTAS(0)| SPI_PUSHR_CONT;
c--;
}
}
// skip values returned by the queued up transfers and the current in-flight transfer
uint32_t sr = _pkinetisk_spi->SR;
uint8_t skipCount = ((sr >> 4) & 0xF) + ((sr >> 12) & 0xF) + 1;

_pkinetisk_spi->SR = SPI_SR_EOQF; // make sure it is clear
while ((_pkinetisk_spi->SR & 0xf0)) {
dummy = _pkinetisk_spi->POPR; // Read a DUMMY byte but only once
}
while (c--) {
if (c) {
_pkinetisk_spi->PUSHR = READ_PIXEL_PUSH_BYTE | (pcs_data << 16) | SPI_PUSHR_CTAS(0)| SPI_PUSHR_CONT;
} else {
_pkinetisk_spi->PUSHR = READ_PIXEL_PUSH_BYTE | (pcs_data << 16) | SPI_PUSHR_CTAS(0)| SPI_PUSHR_EOQ;
while (txCount || rxCount) {
// transmit another byte if possible
if (txCount && ((_pkinetisk_spi->SR & 0xF000) >> 12) < _pspin->sizeFIFO()) {
txCount--;
if (txCount) {
_pkinetisk_spi->PUSHR = READ_PIXEL_PUSH_BYTE | (pcs_data << 16) | SPI_PUSHR_CTAS(0)| SPI_PUSHR_CONT;
} else {
_pkinetisk_spi->PUSHR = READ_PIXEL_PUSH_BYTE | (pcs_data << 16) | SPI_PUSHR_CTAS(0)| SPI_PUSHR_EOQ;
}
}

// If last byte wait until all have come in...
if (c == 0) {
while ((_pkinetisk_spi->SR & SPI_SR_EOQF) == 0) ;
_pkinetisk_spi->SR = SPI_SR_EOQF; // make sure it is clear
}
while ((_pkinetisk_spi->SR & 0xf0) > 0x00) { // do we have at least 3 bytes in queue if so extract...
rgb[rgb_index++] = _pkinetisk_spi->POPR; // Read in the next byte of the color
if (rgb_index == 3) {
*pcolors++ = color565(rgb[0],rgb[1],rgb[2]);
rgb_index = 0; // set index back to 0...
// receive another byte if possible, and either skip it or store the color
if (rxCount && (_pkinetisk_spi->SR & 0xF0)) {
rgb[rgbIdx] = _pkinetisk_spi->POPR;

if (skipCount) {
skipCount--;
} else {
rxCount--;
rgbIdx++;
if (rgbIdx == 3) {
rgbIdx = 0;
*pcolors++ = color565(rgb[0], rgb[1], rgb[2]);
}
}
}
// like waitFiroNotFull but does not pop our return queue
if (_pspin->sizeFIFO() >= 4)
while ((_pkinetisk_spi->SR & (15 << 12)) > (3 << 12)) ;
else
while ((_pkinetisk_spi->SR & (15 << 12)) > (0 << 12)) ;

}

// wait for End of Queue
while ((_pkinetisk_spi->SR & SPI_SR_EOQF) == 0) ;
_pkinetisk_spi->SR = SPI_SR_EOQF; // make sure it is clear

endSPITransaction();
}

Expand Down

0 comments on commit 8ae598a

Please sign in to comment.