Allow setting of CIRCUITPY_DISPLAY_LIMIT in settings.toml #10214
Conversation
| if (displays[i].epaper_display.base.type != &epaperdisplay_epaperdisplay_type || | ||
| displays[i].epaper_display.core.current_group != &circuitpython_splash) { | ||
| // Skip regular displays and those not showing the splash. | ||
| continue; | ||
| } |
There was a problem hiding this comment.
Do this after you've gotten the display object to inspect. Maybe add an actual function to get display X that can switch been the static array and the dynamic one.
There was a problem hiding this comment.
Do this after you've gotten the display object to inspect.
Thanks! moving it is much cleaner.
Maybe add an actual function to get display X that can switch been the static array and the dynamic one.
When you suggest an actual function do you mean the macro functions I'm using in the other modules? I left it out here because there were only a couple instances and thought it was easier to understand without the macro. Or are you suggesting I add a new common_hal function that returns a separate display object from the appropriate memory structure and use that instead of the macros?
main.c
Outdated
| #if CIRCUITPY_DISPLAYIO && CIRCUITPY_OS_GETENV && CIRCUITPY_SET_DISPLAY_LIMIT | ||
| // If number of displays has been overridden in settings.toml allocate memory and | ||
| // move existing displays | ||
| malloc_display_memory(); |
There was a problem hiding this comment.
Since you are doing this really early, can you just have one dynamically allocated array? It'll make all of the other accessing code simpler.
There was a problem hiding this comment.
That was my original attempt, however the boards that had displays defined as board.DISPLAY didn't like the dynamic memory allocation. I tried multiple pointer configurations to try and get the compile to work but didn't look into modifying how board.DISPLAY was defined.
I can go back and dig into the board.DISPLAY setup and try and get it to work with the dynamically allocated memory structure if you think that makes sense.
There was a problem hiding this comment.
Ah, right. board.DISPLAY is static.
I wonder if it'd be more fruitful to just try and allocate displays to the CP heap first and then move them over if needed.
There was a problem hiding this comment.
@tannewt I thought you mentioned somewhere else about replacing board.DISPLAY with something else in 10 or 11? Would board.display() or similar be helpful?
There was a problem hiding this comment.
I still don't have a working understanding of the complete CP memory model so I'm not sure how I would do that, is there a mechanism like port_malloc that would allocate heap space?
There was a problem hiding this comment.
Once this PR was set, I was going to take a run at getting the supervisor.runtime.display object to be settable but I have no idea what road blocks I might run into. It looked to me like getting the primary_display variable working was doable and the first step....
There was a problem hiding this comment.
These allocate into split heap chunks that are allocated with port_malloc
I'll give using m_malloc a try, will using the heap be compatible with board.DISPLAY or is there another advantage over the port_malloc approach?
Edit: I asked Claude and the AI seems to think the advantages are better memory management integration, consistency and error handling...
There was a problem hiding this comment.
I wonder if it'd be more fruitful to just try and allocate displays to the CP heap
m_malloc seems to complain (crash) if the VM is not running. I could allocate the m_malloc memory when the display is allocated in the displayio modules allocate_display and allocate_display_bus. I think I'd need to add a new structure to keep track of which displays had not yet had memory allocated as well. None of the dynamic displays would survive the VM but I could copy the primary display to the static memory before the VM shut down.
Is that what you had in mind?
There was a problem hiding this comment.
None of the dynamic displays would survive the VM but I could copy the primary display to the static memory before the VM shut down.
Yup, that's what I had in mind. I'd only expect it to be called from user code. displayio.DISPLAY and the picodvi init will use the single static allocation that is the display we also use for the terminal.
There was a problem hiding this comment.
Well the user code displays seem to be working with m_malloc now. I've held off working on moving displays to the static storage as I think that will be related to the primary display features and I thought it would be better to work on that in a follow up PR.
Set the maximum supported number of displayio displays. This value overrides the CIRCUITPY_DISPLAY_LIMIT compile-time flag.
Resubmitting earlier PR #10158 now against main rather than 9.x.x. This PR addresses #1760 but I think there are probably follow-up PRs needed before that issue can be considered closed.