espressif / esp-idf

Espressif IoT Development Framework. Official development framework for Espressif SoCs.
Apache License 2.0
12.95k stars 7.1k forks source link

Making the LCD panel API more practical for stateless graphics libraries (IDFGH-8701) #10143

Open codewitch-honey-crisis opened 1 year ago

codewitch-honey-crisis commented 1 year ago

Is your feature request related to a problem?

I have some gripes about the design of the LCD panel API

It really only works well with LVGL. Libraries like TFT_eSPI, htcw_gfx, Lovyan_GFX etc cannot use this API because whoever designed it seems to have only considered LVGL.

Now I like LVGL - i'm a contributor to it in fact, but here's the problem.

LVGL is stateful, meaning you have widgets on screen that exist around in memory. This is very much unlike say TFT_eSPI or htcw_gfx where there are no widgets really.

But because of this, it's easy for LVGL to take a group of widgets that overlap a rectangle on the screen, draw that to a bitmap and then send it to the LCD panel API

With the other libraries this doesn't make any sense, and can't be duplicated efficiently.

The really unfortunate part of this is it could easily support both stateful and stateless graphics libraries quite easily with the addition of a few methods

Describe the solution you'd like.

  1. I'd like a method to do a fast fill of a rectangular window in a single color without requiring memory allocation

  2. A method to read back from the frame buffer

I understand that 2 is slow over SPI, however it's also a critical feature for stateless graphics libraries in order to facilitate alpha blending and anti-aliasing. It works when used effectively, even over SPI and several graphics libraries do it in practice.

Describe alternatives you've considered.

I usually bitbang the parallel i8080 or use traditional SPI calls (under Arduino, or the IDF) and/or go to the SPI registers directly to communicate with LCD panels. I've basically given up using anything but LVGL on the S3 though because of the above, and that's really unfortunate.

Additional context.

No response

atanisoft commented 1 year ago
  1. I'd like a method to do a fast fill of a rectangular window in a single color without requiring memory allocation

This is likely doable but how would you pass in the color data?

  1. A method to read back from the frame buffer

I understand that 2 is slow over SPI, however it's also a critical feature for stateless graphics libraries in order to facilitate alpha blending and anti-aliasing. It works when used effectively, even over SPI and several graphics libraries do it in practice.

It could certainly be slow but there is also the problem with some controllers that don't properly handle putting MISO into a floating state when it is not actively selected via the CS pin.

I don't know how feasible it would be to have a display buffer in the driver implementation that could be used to avoid a roundtrip to the display, very likely it would depend on: PSRAM, display pixel counts and color depth (1bit, 16bit, 18bit, 24bit, etc).

codewitch-honey-crisis commented 1 year ago

I would think if the color data size is not fixed to say, 16 bits for example, it would take a pointer and a size, but that's for one pixel unit, like a 16 bit word. Not sure as I don't have the api in front of me, but I think that should cover it.

It could certainly be slow but there is also the problem with some controllers that don't properly handle putting MISO into a floating state when it is not actively selected via the CS pin.

I don't know what the ramifications of that are. I do know readback works on ST7789s and ST7735s via the MOSI (yes MOSI) line via an SDA Read. Maybe that's why they don't mess with MISO at all. the ILI9XXXX series typically have a MISO line for readback but I'm not sure about the floating issue.

Can I ask a dumb question? Does it really matter? Consider that if it doesn't work with devices that don't handle MISO properly. Then they wouldn't handle MISO properly and wouldn't read. But the ones that did would still work. I'm not sure why a few devices not working is an argument for not having the feature, so I feel like I'm missing something about what you wrote.

If you're saying doing it would actually break working devices in some cases, maybe it can be enabled/disabled.

I don't know how feasible it would be to have a display buffer in the driver implementation that could be used to avoid a roundtrip to the display, very likely it would depend on: PSRAM, display pixel counts and color depth (1bit, 16bit, 18bit, 24bit, etc).

Not realistic, because firmware that uses existing stateless graphics libraries will already buffer in their own code to avoid readbacks when necessary. I do it in htcw_gfx in performance critical code for example.

atanisoft commented 1 year ago

Consider that if it doesn't work with devices that don't handle MISO properly. Then they wouldn't handle MISO properly and wouldn't read.

I'm not against adding this sort of functionality by any means, the problem though is on the display side potentially. In the case of ILIXXXX devices, if they do not properly handle floating state of the MISO/MOSI (depends on sp32/display viewpoint) when CS is not asserted it CAN cause problems for other devices on the same bus (such as touch controllers that also use SPI).

If you're saying doing it would actually break working devices in some cases, maybe it can be enabled/disabled.

Depending on the device yes, it could potentially influence other devices on the same bus (SPI touch controllers as example). I think in this case it could be an optional API that may or may not be supported by the driver implementation.

firmware that uses existing stateless graphics libraries will already buffer in their own code to avoid readbacks when necessary.

Agreed, it's also a rather large chunk of memory that would be "permanently" allocated (very likely in PSRAM due to size).

I would think if the color data size is not fixed to say, 16 bits for example, it would take a pointer and a size, but that's for one pixel unit, like a 16 bit word. Not sure as I don't have the api in front of me, but I think that should cover it.

Perhaps something like:

esp_err_t esp_lcd_clear_to_color(esp_lcd_panel_handle_t panel, uint8_t red, uint8_t green, uint8_t blue)

In the case of monochrome displays it could use logic in the driver to "turn on" the pixel when any of the color parameters are non-zero. Displays that support color could convert the color parameters into the most efficient format for the display.

codewitch-honey-crisis commented 1 year ago

That could work, but is the biggest pixel you can display 24-bits for any potential device? That's why i was considering something like this.

esp_err_t esp_lcd_fill_rect(uint16_t x, uint16_t y, uint16_t width, uint16_t height, void* color, size_t color_size);

For an ILI9341, you'd just do

uint16_t col = TFT_PURPLE;
esp_lcd_fill_rect(0,0,10,10,&col, sizeof(col));

Like that Umm one thing about the widths and heights above. Oddly, in other methods I believe it's actually width-1, and height-1, (but not x2 and y2!) - it would be worth looking into and making sure that this oddness is carried over to this method for consistency.

atanisoft commented 1 year ago

but is the biggest pixel you can display 24-bits for any potential device?

I believe there maybe some displays that support alpha channel so an additional parameter for alpha would likely be good to have.

esp_err_t esp_lcd_fill_rect(uint16_t x, uint16_t y, uint16_t width, uint16_t height, void* color, size_t color_size);

The only issue with this would be determining color data format (RGB565, RGB888, RGB8888, etc). This is where the splitting of color components may be better as it eliminates any doubts.

Oddly, in other methods I believe it's actually width-1, and height-1, (but not x2 and y2!)

X1 and Y1 are passed as-is but X2 and Y2 are reduced by 1 when calculating the display pixel memory, at least for most drivers that I've checked.

it would be worth looking into and making sure that this oddness is carried over to this method for consistency.

Yes, the API would need to be consistent in usage of X/Y coordinates. I can see there potentially being two APIs:

  1. One to clear the full screen to a single color (what I had proposed)
  2. One to clear a section of the screen to a single color (what you have proposed).

I think both make sense and the full panel one could internally leverage a "tiling" approach using the second API to minimize the memory requirements for the pixel data buffer. Both APIs could also leverage the draw_bitmap API internally to reduce code duplication and ensure consistency in X/Y coordinates.

codewitch-honey-crisis commented 1 year ago

Oh wow, so you actually need to do internal mallocs anyway? I guess that must be a hardware thing where you're doing everything DMA based? Traditionally, I would just go to the SPI registers, or bitbang an i8080 interface to do the fill operation without any allocations. This might be a problem.

htcw_gfx will spit out most primitives using at the very least, a Run Length Encoded scheme wherein each line of say, a filled polygon is rendered at a time using the fill operation.

The problem with that is it will be whipping the heap extremely hard given any usual graphics operations, which are often composed of many fill operations. The other issue I see, is complexity, like if the fill rectangle was too big for SRAM, you'd have to do it in sections.

Oh man. I'm suddenly wondering how realistic any of this is.

atanisoft commented 1 year ago

so you actually need to do internal mallocs anyway? I guess that must be a hardware thing where you're doing everything DMA based?

Mostly depends on the driver implementation. Most have DMA buffers allocated up front and re-used, but some include allocation inside draw_bitmap for color conversions (32bit -> 18/24 bit as example).

Most drivers only transfer one row (or column) of pixels as a single transaction, but it depends on how the driver is used.

The other issue I see, is complexity, like if the fill rectangle was too big for SRAM, you'd have to do it in sections.

For esp_lcd_clear_to_color (full screen clear) it would likely be best to use a block size of 16x16 or 32x32 inside the driver implementation based on what makes the most sense for memory usage. For the user provided X/Y coords it likely would need to be broken into blocks for efficiency reasons but may not be fully required.

it will be whipping the heap extremely hard

That is a possibility, but it is not strictly limited to esp_lcd.

codewitch-honey-crisis commented 1 year ago

That is a possibility, but it is not strictly limited to esp_lcd.

While it's not, prior to this API being available, when TFT_eSPI and htcw_gfx were devised they did not malloc at all for these operations, so there was no heap whipping.

I am comparing the present LCD API with these libraries in terms of my back of the napkin performance concerns.

Now, I've tried bit banging i8080 and going to the SPI registers on an S3 vs using the LCD panel API, and I get significantly better > 30% if I recall framerates using the LCD panel API method - both using LVGL on top. However, since it was using LVGL, the fast fill mechanisms and such weren't being employed - it was all bitmap transfers either way.

So with that said, if I'm eating all that performance win up in mallocs it makes this whole endeavor far less worthwhile. I guess we wouldn't know without implementing and testing it. =(

I don't have a proper build environment to build these latest IDF bits, but I could submit a template project (i use platformio) that has a boilerplate setup in it that can be benchmarked, if it would help.

I just fear releasing these extra methods into the API without actually gaining anything.

atanisoft commented 1 year ago

when TFT_eSPI and htcw_gfx were devised they did not malloc at all for these operations, so there was no heap whipping.

For TFT_eSPI it appears to use a buffer provided by the caller for DMA purposes, up to 32kb.

if I'm eating all that performance win up in mallocs it makes this whole endeavor far less worthwhile. I guess we wouldn't know without implementing and testing it. =(

LVGL ESP32 drivers for most displays internally malloc/free inside the draw_bitmap API. It doesn't appear to introduce any performance bottlenecks that I've observed so far.

In the case of full screen "set to color" it could introduce heap thrashing IF it allocated multiple temporary buffers (most of which would be XYcolor-bit-depth). In this case it could be optimized to use a single buffer and reuse that for sending the data to the display (perhaps bypassing draw_bitmap which may internally allocate/convert color data as well).

I don't have a proper build environment to build these latest IDF bits, but I could submit a template project (i use platformio) that has a boilerplate setup in it that can be benchmarked, if it would help.

If you have a sample project to compare options I can do some testing on my side (I have ILI9341 and ILI9488 displays on hand). I don't use PIO but can convert to a compatible IDF project.

codewitch-honey-crisis commented 1 year ago

For TFT_eSPI it appears to use a buffer provided by the caller for DMA purposes, up to 32kb.

That's only when using DMA, which isn't typically used very often with these libraries. You'll note TFT_eSPI doesn't even initialize SPI master, until you actually tell it to. LCD Panel api appears heavily DMA oriented. The libraries I'm comparing them to support DMA, but it's only used for maybe 1/3 of the operations, if that in most real world applications. And the fast filling and such does not use DMA at all, unless Bodmer has changed it (i know htcw_gfx still doesn't unless you tell it to try). It's used for bitmap transfers.

If you have a sample project to compare options I can do some testing on my side (I have ILI9341 and ILI9488 displays on hand). I don't use PIO but can convert to a compatible IDF project.

I've got some things to take care of but I will put one together today and stick it on github once I can. They'll both be setup using an S3. I won't be able to test 16-bit i8080 using this method as TFT_eSPI and htcw_gfx drivers don't currently support it, but I will set up something using 8-bit i8080 and SPI side by side using each method. I have to wrap my around the best way to put it together still but i should have something today barring disasters.

Thanks!

codewitch-honey-crisis commented 1 year ago

Hey, I just noticed something in the code I wrote awhile back for ILI9488 panels for the LCD panel IO api

I'm not mallocing. my ili9488 code at github

static esp_err_t panel_ili9488_draw_bitmap(esp_lcd_panel_t *panel, int x_start, int y_start, int x_end, int y_end, const void *color_data)
{
    ili9488_panel_t *ili9488 = __containerof(panel, ili9488_panel_t, base);
    //assert((x_start < x_end) && (y_start < y_end) && "start position must be smaller than end position");
    esp_lcd_panel_io_handle_t io = ili9488->io;
    x_start += ili9488->x_gap;
    x_end += ili9488->x_gap;
    y_start += ili9488->y_gap;
    y_end += ili9488->y_gap;
    // define an area of frame memory where MCU can access
    uint8_t data1[4];
    data1[0]=(x_start>>8)&0xFF;
    data1[1]=x_start&0xFF;
    data1[2]=((x_end-1)>>8)&0xFF;
    data1[3]=(x_end-1)&0xFF;
    esp_lcd_panel_io_tx_param(io, 0x2A, data1, 4);
    uint8_t data2[4];
    data2[0]=(y_start>>8)&0xFF;
    data2[1]=y_start&0xFF;
    data2[2]=((y_end-1)>>8)&0xFF;
    data2[3]=(y_end-1)&0xFF;
    esp_lcd_panel_io_tx_param(io, 0x2B, data2, 4);

    // transfer frame buffer
    size_t len = (x_end - x_start) * (y_end - y_start) * ili9488->bits_per_pixel / 8;
    esp_lcd_panel_io_tx_color(io, 0x2C, color_data, len);

    return ESP_OK;
}
atanisoft commented 1 year ago

I'm not mallocing. my ili9488 code at github

You are using 16 bit color mode, if you enable 18bit color here it will not render correctly on the display (note that 18-bit color depth requires CONFIG_LV_COLOR_DEPTH_32)

My driver is registered here and lives here. I've tested both 16-bit and 18-bit with no noticable differences. Note that the draw_bitmap method only allocates for 18-bit color since input data would be 32-bit width).

In my tests I'm also using the basic 4-wire SPI and not 8/16 width modes (mostly due to displays I have only using the 4-wire mode).

I've got some things to take care of but I will put one together today and stick it on github once I can. They'll both be setup using an S3.

Sounds good, I have a couple S3 with one currently connected to an ILI9488 4-wire SPI interface (more like 5 wire due to DC pin). I have a WIP XPT2046 driver as well which will be published once I've finished testing.

codewitch-honey-crisis commented 1 year ago

Ah yes. Indeed. htcw_gfx drivers do malloc when doing the pixel format conversion from 18-bit (padded 24-bit) mode, and when doing readbacks for the same reason (pixel format is always 18-bit on reads). I usually just accept that ILI9488 over SPI is slow. It's true with all the libraries. Honestly, I don't know why they designed it to be 18-bit only over SPI. You'd think it would be the other way around. /gripe.

I had to put out some proverbial fires today so I haven't gotten around to the benchmark project. I'm starting that now but I realized all of htcw_gfx driver code for the i8080 is arduino only so I'm going to have to do some porting.

I apologize for the delay, but I'll get it to you as soon as I can.

I'll write this for SPI and 8-bit i8080 ILI9488.

Adding, if it helps, I've written an XPT2046 touch driver, and also the common capacitive FT6336 driver used on higher end screens. There's code here. It's for arduino but shouldn't be a problem to port [htcw_f76636] (https://github.com/codewitch-honey-crisis/htcw_ft6336/blob/master/include/ft6336.hpp) - it should actually work with several chips in that series but that's the one i tested it with

atanisoft commented 1 year ago

Honestly, I don't know why they designed it to be 18-bit only over SPI. You'd think it would be the other way around.

It certainly is a bit silly. There is supposed to be a 16-bit mode but I haven't been successful with making it display anything meaningful when running via SPI. Most likely the i80 interface would be faster than this by quite a bit.

I'm starting that now but I realized all of htcw_gfx driver code for the i8080 is arduino only so I'm going to have to do some porting.

esp_lcd offers a few i8080 options, so it's possible you wouldn't need to port too much over. However, it should be feasible to use ESP-IDF APIs (including my ili9488 driver) within Arduino without too much hassle, it likely be using IDF v4.4.2 though due to arduino-esp32 (and in turn PIO) using a pre-compiled version of IDF.

if it helps, I've written an XPT2046 touch driver

Thanks, I'll take a look at it. The API will adhere to the esp_lcd_touch APIs which mostly needs an API for read_data and get_xy.

codewitch-honey-crisis commented 1 year ago

esp_lcd offers a few i8080 options, so it's possible you wouldn't need to port too much over

My goal was to create a side-by-side comparison of what I could between the LCD api and something like an htcw_gfx ili9488 driver. To do that I will need to port.

See my thought was to hand you off something you could bang on to add fast fill and such to the LCD API bits and then run a comparison with the manual bit banging/SPI Master way of doing things (I don't use the SPI registers under the ESP-IDF because the SPI Master API is less suitable for that than the Arduino SPI api due to the way transaction boundaries are set in SPI Master)**

** a separate gripe of mine about the ESP-IDF is how allocation happy it is. There's too much overhead for SPI transactions under the ESP-IDF / SPI Master - one of the reasons I want to move to the LCD API to begin with.

atanisoft commented 1 year ago

create a side-by-side comparison of what I could between the LCD api and something like an htcw_gfx ili9488 driver

Sounds good, looking forward to seeing it and testing it (note I only have SPI displays so...)

add fast fill and such to the LCD API bits

Perhaps it would make sense to create a component that provides GFX primitives similar to the BASIC DRAW API in Adafruit-GFX-Library (or similar libraries) that will internally call esp_lcd APIs to transfer data to the displays?

I don't use the SPI registers under the ESP-IDF because the SPI Master API is less suitable for that than the Arduino SPI api due to the way transaction boundaries are set in SPI Master

Might be worthwhile to explore these differences a bit further as the Arduino SPI APIs are directly poking values into hardware registers which is abstracted in ESP-IDF to the HAL/HAL-LL APIs. There has been an effort for some time to move away from directly accessing hardware in Arduino-esp32 over to ESP-IDF APIs (HAL or driver interfaces), but it may not have started on the SPI interface yet.

one of the reasons I want to move to the LCD API to begin with.

The LCD APIs internally will use SPI (with transactions) if using esp_lcd_new_panel_io_spi which is used by may of the inexpensive LCDs on the market and will use SPI transactions / DMA internally if supported. If esp_lcd_new_i80_bus is used it will use DMA and likely will be considerably faster due to parallel data transfer capabilities.

codewitch-honey-crisis commented 1 year ago

Sounds good, looking forward to seeing it and testing it (note I only have SPI displays so...)

Oh, well that saves me some doing then. Still, this is dragging on longer than I expected, and I have a couple more things to do for work in a couple of hours. I have not punted this project, it's just more work than I thought, and life is getting in the way.

**Perhaps it would make sense to create a component that provides GFX primitives similar to the BASIC DRAW API in Adafruit-GFX-Library (or similar libraries) that will internally call esp_lcd APIs to transfer data to the displays?

Not in this case I think. The thing is, the way htcw_gfx works is it knows nothing about IoT. All the line drawing and primitive drawing, alpha blending, etc is all done in pure C++ code with no knowledge of any hardware, or any framework.

The drivers are what tie it in - and the drivers are what are relevant here. htcw_gfx interfaces using various methods as outlined here

There are drivers for Arduino, for ESP-IDF, for DirectX on PC, etc, and each of those for various display hardware.

So it's at the driver intersection that this stuff is all really relevant. Stuff like the line, shape, and text drawing are not relevant to this op.

Rest assured that htcw_gfx is already used in professional production code, over SPI on ILI9XXXX devices, so it's "fast enough" for the real world. That has been established. TFT_eSPI even moreso.

The LCD APIs internally will use SPI (with transactions) if using esp_lcd_new_panel_io_spi which is used by may of the inexpensive LCDs on the market and will use SPI transactions

I figured this, but what would be telling for me was does the esp_lcd_new_panel_io_spi() use the SPI Master API or does it go underneath it? And the reason I ask that is because the SPI Master overhead is prohibitive for the types of operations you need to do typically with an LCD display controller. By prohibitive I mean I get 30% better framerates under Arduino because I can go to the registers directly and start and stop transactions without having to memcpy under Arduino.

If esp_lcd_new_panel_io_spi uses SPI Master rather than going underneath it/alongside it, then I will not be using it for driving SPI displays in my production code (except perhaps with DMA where additional overhead is reasonable to expect). It's not performant enough. I guess I'll take a look at how it's implemented.

atanisoft commented 1 year ago

does the esp_lcd_new_panel_io_spi() use the SPI Master API or does it go underneath it?

Yes, it internally does use SPI master: https://github.com/espressif/esp-idf/blob/master/components/esp_lcd/src/esp_lcd_panel_io_spi.c

All the line drawing and primitive drawing, alpha blending, etc is all done in pure C++ code with no knowledge of any hardware, or any framework.

Same for the esp_lcd framework, it's an abstract API that can interface with lower level drivers that implement transport and device specific interfacing. The question though is can a basic primitives interface be layered on top of the existing esp_lcd API.

codewitch-honey-crisis commented 1 year ago

The question though is can a basic primitives interface be layered on top of the existing esp_lcd API.

Edit: I think I misunderstood you. So let me try again.

The short answer is, with the methods I requested in the OP, most any library should be able to efficiently do all these things using this API.

Here's my previous response: I thought you were talking about implementing drawing primitives within the LCD framework itself. My mistake. Leaving it for posterity. :)

I guess to answer your question, I don't see why not, but I question the wisdom of doing it that way. It seems like scope creep. Libraries like LVGL and htcw_gfx already do these things - they just need something to interface with to transfer the graphics results down the last mile to the display.

The only reason for the additional methods I requested at the start was performance, just to be clear. I feel like implementing these primitives within the IDF is heavy handed at best, but then, I don't work for Espressif and am not a contributor to the ESP-IDF in any capacity. Obviously, I don't determine scope, but it just feels out of scope to me.

The only advantage I can see in doing so is it allows a library to potentially be written that takes advantage of the hardware acceleration features of the RA8875 controller - the only IoT display controller I've ever used that has hardware accelerated drawing primitives, like ellipses, lines, and text.

If you really do want to implement primitives, tackle text first, and I think you'll see why it's problematic quickly.

LVGL and htcw_gfx support things like TrueType vector fonts as well as raster fonts. That's non-trivial to implement. You'll need extra coordinate primitives like sizes and rectangles to measure text effectively, possibly do DPI conversions to convert to actual pixels if you want to support that (it requires plugging in the physical size of the display), etc.

And if you want to consider everything except doing text, you're still effectively requiring people to use a separate drawing library that does do text, for almost any real world application, with all the duplicated effort that entails.

Basically, then you have your own graphics library. At that point you're in direct competition with libraries like LVGL that have their own wysiwyg visual designer software and hundreds of contributors, and work on many many different devices.

At least if I understand your question.

Maybe I totally misunderstand what you're trying to do though. I feel like I might.

atanisoft commented 1 year ago

with the methods I requested in the OP, most any library should be able to efficiently do all these things using this API.

So, from your initial request:

  1. A method to send a single color to a pixel range (x1,x2,y1,y2)
  2. A method to read back the value from a pixel range (x1,x2,y1,y2) as color values.

One piece that will need to be defined is "pixel color", would it be MONOCHROME, RGB565, RGBA8888, RGB666, etc.. Most likely it would be RGB565 as that is a common value for 16-bit color displays and MONOCHROME for 1-bit displays (inexpensive OLEDs). There may be other definitions that I'm not thinking of.

However, if we ignore the "pixel color" definition... The first method could be accomplished with the existing draw_bitmap method provided that the data is in a format that the underlying driver supports (or converts from).

The second method (read back) is a bit more complicated and from a driver perspective I'm not sure how it should be handled. There is a breaking API change from IDF v4.4 and IDF v5.0, introducing another method could be another when moving up to v5.1. It would likely need to be optional (much like esp_lcd_touch uses) to allow drivers to work between the supported IDF versions without too many breaking changes.

The only reason for the additional methods I requested at the start was performance, just to be clear.

Performance would be heavily dependent on the IO layer (i80 / parallel being faster than four wire SPI as example).

codewitch-honey-crisis commented 1 year ago

The first method could be accomplished with the existing draw_bitmap method provided that the data is in a format that the underlying driver supports (or converts from).

The only reason I don't use LCD panel API as is for doing exactly what you're talking about at a higher level is it's not even remotely sensical to do this

// len is the size in bytes)
void* buf = malloc(len);
// fill to black
memset(buf,0,len);
draw_bitmap(x1,y1,x2,y2,buf,len);

The memory allocation is completely unnecessary. It's simply not acceptable, at least in my code. Fill is called in critical codepaths. 50 times per letter on a font for example. I will forgo using the LCD API altogether if I have to do that, even if I know it is doing that under the covers when I call its fill method.

codewitch-honey-crisis commented 1 year ago

I found a problem. I cannot get readbacks to work under the ESP-IDF using SPI Master. I forgot that this has always been a problem for me (I've tried before, over a year ago but forgot) They work fine under Arduino.

Short of someone maybe producing some code to do a successful readback from an ILI9488 using SPI Master I am currently stuck.

But even if I could, there's a deeper problem with it.

An ILI9341 will typically support 40MHz writes and 20MHz reads. An ST7735 will typically support 20MHz writes, but only 4MHz reads.

SPI Master, being not well designed for LCD devices, does not let you change the SPI clock speed on a per transaction basis, meaning if I want to read back I have to operate the entire device at the read speed.

Because of a software limitation.

Maybe this just isn't practical. This is why I use the Arduino framework when I'm creating professional (paid) projects with the ESP32 line, BTW.

atanisoft commented 1 year ago

SPI Master, being not well designed for LCD devices, does not let you change the SPI clock speed on a per transaction basis, meaning if I want to read back I have to operate the entire device at the read speed.

Is it possible to use one spi_device_handle_t for read and one for write (I haven't tested this)?

codewitch-honey-crisis commented 1 year ago

I haven't tried that. I wouldn't be surprised if it fails due to the same pin assignments being used for "both" devices. I could give it a look. Edit: I got errors trying to do this.

I have considered unregistering the SPI device and registering it but I don't know what the overhead for that looks like. I haven't pursued checking that because the whole thing smells to me.

I have also considered spelunking through SPI master and recreating the code it uses to set the hardware clock speed.

I should also point out this makes the RA8875 based touch panel displays useless under the esp-idf because they have a slaved SPI device embedded into the RA8875 that operates at 10MHz while the display hardware operates at 20MHz and is already almost too slow at that. Since the device is slaved, it is not a separate device instance - it is something you query by writing a register over SPI, switching to 10MHz and then reading, all while keeping your CS line for that "compound" or "composite" device active.

I really feel like the design of SPI Master could have been more battle tested. That's why I was hoping the LCD Panel API went behind it.