Skip to content

Conversation

@jepler
Copy link
Contributor

@jepler jepler commented Mar 13, 2025

The original post_addr_delay (the delay after changing the ABCD(E) address lines) was pretty long, for two reasons: First, the initial timing numbers were chosen conservatively without regard to performance; and second, because they were originally chosen assuming a high PIO clock rate but we lowered the PIO clock to 2.7MHz.

Together this made the delay a huge 185us!

The new value of 5 is about 1.8us instead; this gives a nice increase in FPS from 88 to 120 on my 128x128 (4 lane, 6/2 plane) setup, without any visible artifacts.

This also updates piolib & gets rid of a workaround that was needed in an older version for large transfers. This also requires a newer kernel, but even one from mid January should suffice.

Lastly, it enables 3 (& 5) temporal planes instead of just the 0/(1)/2/4 settings. 5+ temporal planes don't seem that useful, but 2 & 3 look good. 18 bit color with 3 temporal planes gets to 150fps on my 128x128 matrix.

jepler added 6 commits March 13, 2025 09:34
The original post_addr_delay (the delay after changing the ABCD(E)
address lines) was pretty long, for two reasons: First, the initial
timing numbers were chosen conservatively without regard to
performance; and second, because they were originally chosen
assuming a high PIO clock rate but we lowered the PIO clock to
2.7MHz.

Together this made the delay a huge 185us!

The new value of 5 is about 1.8us instead; this gives a nice
increase in FPS from 88 to 120 on my 128x128 (4 lane, 6/2 plane)
setup, without any visible artifacts.
This diff looks bigger than it is because we'd previously
reformatted piolib sources via pre-commit.
it's not needed with updated piolib
By doing a bit more math we can create a sensible schedule for
these situations.  A 6/3 schedule gives a very nice 150fps with
temporal shimmer at 50Hz
@jepler jepler requested a review from FoamyGuy March 13, 2025 15:44
Copy link
Contributor

@FoamyGuy FoamyGuy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks great to me. I tested mirroring pico8, doom, and snes rom with this branch using a combination of xdisplay_mirror and virtual display. All are looking great. I noticed this change appears to have resolved an issue I was seeing sometimes where certain pixels (typically a few in a row, sometimes longer rows) would be "extra bright" compared to the rest of the image.

One question about the added print. Looks good to me otherwise.
Thanks for this improvement @jepler!

schedule sched;
for (int j = 0; j < n_real_planes; j++) {
int k = 1 << (n_temporal_planes + n_real_planes - j - 1);
printf("n_real_planes=%d n_temporal_planes=%d plane=%d k=%d\n",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this print statement added and left enabled on purpose? I don't really have an opinion on keeping it or not, but saw some repeated printouts from it and wanted to double check that it was intentional

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nope, it should be deleted.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

and now it is.

@jepler jepler requested a review from FoamyGuy March 13, 2025 20:34
@jepler
Copy link
Contributor Author

jepler commented Mar 13, 2025

It's most likely the updated piolib that fixes uneven brightness on certain lines. when multiple transmit calls were being used, a row of pixels might have been illuminated during the transmission gap, leaving it on longer than it should have been.

Copy link
Contributor

@FoamyGuy FoamyGuy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you!

@FoamyGuy FoamyGuy merged commit f80ae1e into main Mar 13, 2025
14 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants