-
-
Notifications
You must be signed in to change notification settings - Fork 82
NXT display support using pbio model #440
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
Bring LCD driver into a dedicated pbio driver. No feature change yet, only apply code formatting. Refs: pybricks/support#2425
This is needed on NXT to avoid damaging the display. Call it in pbdrv_deinit. Refs: pybricks/support#2425
Use a PBIO thread to handle the screen refresh instead of doing it in ISR. The driver provides the frame buffer which is used by the user. On refresh, format is converted to match the one used by the LCD driver. In the future, the user frame buffer may use the LCD driver format in order to improve memory usage. This will require pbio/image to handle this format. Refs: pybricks/support#2425
Remove unused debug display code. Use pbio image and driver instead of nxos display functions when appropriate. Refs: pybricks/support#2425
This is needed to allocate memory for images, without using the micropython allocator. Refs: pybricks/support#2425
|
Fantastic! Can't wait to try this out. |
| static void spi_write_command_byte(uint8_t command) { | ||
| spi_set_tx_mode(SPI_MODE_COMMAND); | ||
|
|
||
| // Wait for the transmit register to empty. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If I understand correctly, this blocking loop is only used during setup and the synchronous panic handler?
If so, then we should be OK, but it's worth adding a note about this to justify the blocking loop.
But if we are dropping the panic handler (see chat conversation), we could just as well make this non-blocking. We can use await_once + request_poll in the loop in that case so it is still very quick.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, your understanding is right. The loop should take something like 192 cycles (48 MHz / 2 MHz * 8).
await_once + request_poll seems a good solution, I was afraid to need an interrupt to do it right.
I also have a blocking 20 ms wait in deinit, is it correct?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, we'd want to make that non-blocking too.
We can use this pattern like we do in a few other places, so no additional processes are needed.
// this doesn't do deinit right away, but it ask the display process to gracefully exit at a useful spot, and then do deinit. The busy count is used so all processes can await deinit before actual power off.
void pbdrv_display_deinit(void) {
pbio_busy_count_up();
pbio_os_process_make_request(&pbdrv_display_nxt_process, PBIO_OS_PROCESS_REQUEST_TYPE_CANCEL);
}And then replace:
// Update the display with the user frame buffer, if changed.
for (;;) {
...
}
PBIO_OS_ASYNC_END(PBIO_SUCCESS);
}with
// Update the display with the user frame buffer, if changed.
while (pbdrv_display_nxt_process.request != PBIO_OS_PROCESS_REQUEST_TYPE_CANCEL) {
...
}
// ...
*AT91C_SPI_IDR = ~0;
*AT91C_SPI_PTCR = AT91C_PDC_TXTDIS;
spi_write_command_byte(RESET()); // could also be non blocking as discussed separately
PBIO_OS_AWAIT_MS(state, timer, 20); // instead of nx_systick_wait_ms(20);
pbio_busy_count_down();
PBIO_OS_ASYNC_END(PBIO_ERROR_CANCELLED);
}Use asynchronous code to avoid blocking. Thanks: Laurens for the code to do it. Refs: pybricks/support#2425
This migrates the LCD driver from nxos to pbio model. Handling is now done in pbio thread context and the driver is made compatible with pbio/image.
The nxos display functions are dropped, and the remaining code using them are updated to use pbio/image. If decision is taken to drop them, it can be done later.