Runtime Configuration of EVE_HAS_GT911?

[dstulken]

Rudolph-

A small library modification for your consideration.

As I'm sure you are familiar with, component shortages have thrown a wrench in many product lines this year. Matrix Orbital in particular seems to have been hit hard, and while looking for second sources, I've noticed that in some cases, equivalent displays from other manufacturers are quite similar- with compatible (or adaptable) pinouts, timings, and mechanical form factors. But the software hiccup of the hardcoded Goodix touch controller configuration makes these displays incompatible from a software perspective, unless a different software build is created for each display type used.

To avoid this, what I've tested locally is a simple change to EVE_commands.c and EVE_commands.h.

In EVE_commands.h, the init function changes from:
uint8_t EVE_init(void);
to
uint8_t EVE_init(bool hasGT911);

and in the revised EVE_init(bool hasGT911) function in EVE_commands.c, we simply take the

    #if defined (EVE_HAS_GT911)
    ...
    #endif

block and wrap it in a conditional:

   if (hasGT911)
   {
       #if defined (EVE_HAS_GT911)
       ...
       #endif
   }

Then for any "flexible" display configuration desired, "EVE_HAS_GT911" would be defined in the display profile in EVE_config.h (to make it "available"), and then this could either be used or not used as desired at runtime, as determined by the hasGT911 parameter when EVE_init is called, allowing both types of displays to be used without requiring separate software builds.

Thoughts?
Or is there a better way to do this?

Thanks in advance.

[RudolphRiedel]

Hmm, normally I would just add more profiles for different vendors and especially the "Resolution_xxx" blocks for standard timings allow very short configurations.
Look at the configurations for EVE_EVE3_35 and EVE_EVE3_35G for example, these are the same apart from the additional "EVE_HAS_GT911" in the second configuration.

But it sounds like you are using just the panels? So you are not using one of the modules that already includes a FT81x / BT81x?

There is a different way for this, with the version I put in the 5.x branch yesterday the define for the selected panel is gone from EVE_config,h to allow for configuration of the panel without the need to edit the file.
I could remove the EVE_HAS_GT911 define so it could be set externally if required but I am afraid that this would only lead to problem reports since the common user does not really need to know what a GT911 is in the first place.
But this also means you could roll your own configuration by setting the necessary defines yourself in your build-environment now without changing EVE_config,h.

Take this configuration for example:
#if defined (EVE_EVE3_50G) || defined (EVE_EVE3_70G)
#define Resolution_800x480

#define EVE_PCLK (2L)
#define EVE_PCLKPOL (1L)
#define EVE_SWIZZLE (0L)
#define EVE_CSPREAD (0L)
#define EVE_HAS_CRYSTAL
#define EVE_GEN 3
#define EVE_HAS_GT911
#endif

Instead of setting the define EVE_EVE3_50G) or EVE_EVE3_70G you could just set all the defines
in your build-environment to tweak the details without modifying EVE_config.h.

Also I could add generic profiles like this:

#if defined (EVE_GENERIC_800X480)
#define Resolution_800x480

#if !defined (EVE_PCLK)
#define EVE_PCLK (1L) /* 1 = use second PLL for pixel-clock in BT817 / BT818 */
#endif

#if !defined (EVE_PCLK_FREQ)
#define EVE_PCLK_FREQ (30000000L) /* EVE_PCLK needs to be set to 1 for this to take effect */
#endif

#if !defined (EVE_PCLKPOL)
#define EVE_PCLKPOL (1L)
#endif

#if !defined (EVE_SWIZZLE)
#define EVE_SWIZZLE (0L)
#endif

#if !defined (EVE_CSPREAD)
#define EVE_CSPREAD (0L)
#endif

#if !defined (EVE_GEN)
#define EVE_GEN 4
#endif

/* these need to be set externally if required:
#define EVE_HAS_CRYSTAL
#define EVE_HAS_GT911
*/
#endif

So you would only need to provided the defines that are different.
Or I could expand this to the other parameters as well, EVE_VSYNC0 and so on, leaving only EVE_HSIZE and EVE_VSIZE fixed as a match for the default values of the parameters.
So one could tweak EVE_VCYCLE for example from the outside.

[dstulken]

Thanks Rudolph.

Perhaps I didn't explain this well enough - It's not an issue of where in the code those #defines are located - it's an issue of the configuration defines being compile-time parameters that lock in the program behavior. The software after compilation is only compatible with one display - the display that was configured at compile time.

Instead, I'm proposing a change that lets one piece of the display configuration be runtime configurable, so that one software build can be used with multiple similar hardware displays, without requiring the software to be reconfigured (editing defines) and recompiled/reloaded.

For example, using the displays you mentioned-
If I had a device set up for the EVE_EVE3_35 display, and I connected an EVE_EVE3_35G instead, the touch wouldn't work, and it would take a new software load to permit it to function.
Likewise, if I had a device set up for the EVE_EVE3_35G, and I connected the EVE_EVE3_35 instead, it also wouldn't work and would require new software to be loaded to function.

But if that one differing parameter (EVE_HAS_GT911) was runtime modifiable, then a device's software could selectively enable it as needed - for example, set at startup based on a DIP switch value - so that the device could support multiple display types from a singular software load.

[RudolphRiedel]

I see, having one software to support more than one display makes more sense in this context.
This looks like a subset of: #6
I still do not like the idea of supporting multiple displays at once since that would break everything for everyone but I agree that this is a valid reason to change the configuration from beeing fixed at compile time to beeing configured at runtime.
Still, adding a parameter would make adding this a requirement, even most do not even need it.
What about adding an extra init function first and leaving everything else in place?

Like this: EVE_init_runtime(EVE_INIT_STRUCT_T st_eve_init)

Any maybe make this the only EVE_init() later?

[RudolphRiedel]

Hmm, I do have this now:

typedef struct
{
uint16_t hsize; /* valid range: 12 bits / 0-4095, Thd, length of the visible part of a line (in PCLKs) - active display width /
uint16_t vsize; / valid range: 12 bits / 0-4095, Tvd, number of visible lines (in lines) - active display height /
uint16_t hsync0; / valid range: 12 bits / 0-4095, Thf, Horizontal Front Porch /
uint16_t hsync1; / valid range: 12 bits / 0-4095, Tvf + Tvp, Vertical Front Porch plus Vsync Pulse width /
uint16_t hoffset; / valid range: 12 bits / 0-4095, Thf + Thp + Thb, length of non-visible part of line (in PCLK cycles) /
uint16_t hcycle; / valid range: 12 bits / 0-4095, Th, total length of line (visible and non-visible) (in PCLKs) /
uint16_t vsync0; / valid range: 12 bits / 0-4095, Tvf, Vertical Front Porch /
uint16_t vsync1; / valid range: 12 bits / 0-4095, Tvf + Tvp, Vertical Front Porch plus Vsync Pulse width /
uint16_t voffset; / valid range: 12 bits / 0-4095, Tvf + Tvp + Tvb Number of non-visible lines (in lines) /
uint16_t vcycle; / valid range: 12 bits / 0-4095, Tv, total number of lines (visible and non-visible) (in lines) /
uint8_t swizzle; / 4 bits, controls the arrangement of the output colour pins /
uint8_t pclkpol; / 1 bit, 0 = rising edge, 1 = falling edge /
uint8_t cspread; / helps with noise, when set to 1 fewer signals are changed simultaneously, reset-default: 1 /
uint8_t pclk; / pixel-clock divider, 0 = no PCLK output, 1 = use second PLL for pixel-clock in BT817 / BT818 /
uint32_t pclk_freq; / frequency in Hz for BT817 / BT818 to be used with EVE_cmd_pclkfreq() in order to write REG_PCLK_FREQ /
uint8_t pwm_duty; / valid range: 0-128, backlight PWM level, 0 = off, 128 = max /
uint8_t generation; / 2 = FT81x, 3 = BT815 / BT816, 4 = BT817 / BT818 */
bool has_crystal;
bool has_gt911;
} EVE_Display_Parameters_t;

But I just realized two things.
First "generation" better should remain a define to prevent running generation 4 code on generation 2 chips.
And EVE_GEN is not single-use like the others.

And "has_gt911" on it's own would mean that the binary blob for generation 2 GT911 support would always end up at in generation 2 binaries since the optimizer could not determine anymore if it is actually used or not.

Oh, annother thing, having EVE_HSIZE and EVE_VSIZE available globally also is rather nice.

This is getting a bit more complicated than I anticipated. :-)

[dstulken]

Thanks Rudolph.

I like your idea - I think it gives the software a lot more flexibility, and the use of the second version of the init function, in parallel to the original, makes sense and would be quite useful.

I had thought about the gt911 binary blob - the way I made my change in the first post should handle this. If a user didn't need gt911 support for any of their target displays, they would leave EVE_HAS_GT911 undefined and save on memory. But if they might need it, to be determined conditionally later at runtime, then they would define EVE_HAS_GT911 so it was available in the compiled executable.

I think you would be just fine to do something similar with your structure - keep it how you have it, but also keep the binary blob behind the #define statement. That way the users can leave it out if they know they won't need it, or include it if they do (or think they might).

For the EVE generation, that might also be somewhat self-solving? For example, if I want to provision a system for runtime selection between two different displays, one of which is EVE2, and one EVE4, I'm already going to be limiting myself to the EVE2 feature set for compatibility, otherwise I have to write two different versions of the user interface. If the EVE4 display will function as expected when the EVE2 generation is defined (I haven't tried this, but the datasheets seem to indicate it will work?), then your plan to leave EVE_GEN as a #define should be A-OK. The user would just set this to the oldest generation they planned to use, and it wouldn't otherwise be necessary to make any runtime changes to this.

One more wildcard question:
Do you know if there is any way to auto-detect the GT911 touch controller? Or to sense that or any other display parameters based on init feedback? Being able to manually specify dynamic configurations will be great - but auto-detection would be fantastic.