Skip to content
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

Attempt at S-PWM #1353

Open
wants to merge 24 commits into
base: master
Choose a base branch
from
Open

Conversation

greatballoflazers
Copy link

It is still work in progress. However this is proof of concept. More changes will come later. Needs more testing. Currently one of the parameters is hard coded and should be promoted to the command line options.

I have seen someone on a few forums mention S-PWM and I was curious so I took a swing at it. Seems like a pretty straight forward code change.

@greatballoflazers
Copy link
Author

Additional testing shows this to be slightly less stable. There is an improvement in specific cases however the impact is reduced by system stability. It helps for short chains which using max PWM bits. It helps in long chains using less PWM bits, however instability is much higher. I can neither recommend or recommend against this. Therefore I will leave this for review or additional testing.

This would likely be better on systems with better IO stability. It would appear that there is some kind of scheduler/kernel, memory conflict, IO arbitration, etc. causing reduction in lower PWM bits. Note no single core testing was performed. Testing was done on Pi 2. Again an improvement was noted.

Note in my testing, show refresh does not reflect the actual refresh. It is closer to the frame rate, which is mostly how it is in the original. Instability here causes frame rate to collapse potentially, in some use cases would be bad. The original is much somewhat more tolerant to this.

Changing seg_bits allows tuning of overhead. This would need to be a parameter moved to command line if this ever merged. This can be a little tricky but just fiddling without is also valid. Overall I found it is not worth touching lsb-nanoseconds very much. This is likely much of the same.

@greatballoflazers
Copy link
Author

@marcmerlin How does this work on your display?

@marcmerlin
Copy link
Contributor

@greatballoflazers sorry, I literally have negative time to test this. @hzeller is your man

@greatballoflazers
Copy link
Author

Added support for --led-seg-bits, however I only added it to the options. It is not added to canvas API.

Note PWM bits count down from 11, while S-PWM bits count up from 1. This gets a little weird but still works.

For example if you use 8 segment bits.

  • The 10 bit plane is divided into 4 sections
  • The 9 bit plane is divided into 2 sections
  • The 8 bit plane is divided into 1 section
  • The 7 and following bit planes is not divided.

In total there will be 8 sections for PWM bits greater than 3 PWM bits. S-PWM is supposed to give 8 times perceptive refresh boost in this case. However I have not verified this for myself. Lowering the segment bits will increase perceptive refresh but may cause instability.

Note standard brightness reduction via bit plane periods will still apply. Note this is not full S-PWM it is based on BCM.

@K0rkunc
Copy link

K0rkunc commented Oct 27, 2021

ok what is the advantage of having s-pwm

@greatballoflazers
Copy link
Author

ok what is the advantage of having s-pwm

It is supposed to increase the perceptive refresh rate. Which is usefully for reducing flicker and improving image appearance. It also is helpful if you want the image to be shown on camera or video. I have proven in certain cases this is accomplished however there is a possible increase in instability also in certain cases. It would be an option or tool to use to dial in operation, but may be considered advanced. I suspect this is used in receiver cards. It would likely work a little better in MCU or FPGA, but it may also have some benefit here also.

This is more commonly done on PWM LED drivers. However they do different things like use actual PWM instead of BCM. I want to say it is commonly implemented using a binary tree. Here is some mention of it for a memory mapped LED driver: (See section 3.6.2) https://www.ti.com/lit/an/slva645/slva645.pdf?ts=1635356532428

Due to CPU complexity this code base pretty much has to do BCM. Binary tree approach is probably more complicated than its worth. I cannot speak for the quality difference between using a binary tree or BCM time division vs PWM as I do not know. For all I know the receiver cards use binary tree implementation. This is low effort proof of concept. Have done testing enough to show it has positives and negatives.

The Pi is not likely stable enough to derive time from IO like a FPGA can. Instead it has to use BCM with a timer to accomplish mostly the same thing. With derived time you can do standard PWM, and when done on FPGA you do not have the massive CPU overhead. This makes a binary tree easier. The lower segments would have to be retransmitted constantly which again I am not sure is a good idea.

A binary tree may be possible on a MCU despite the CPU issue using a look up table. The idea here being a MCU can do IO with more stability. However the memory shortage is likely a huge issue. This would force you to move to low segment bits which will increase the row change overhead. Therefore I am not sure it makes sense to a binary tree on anything but hardware (PWM LED driver) or FPGA. Doing PWM over shift register is really hard for processors. That is what each segment would be with proper binary tree.

What could be done is a small binary tree conversion in set pixel. This however would still be implemented with BCM, so maybe the tree does something special. I do not know. However this library does not generally respect the lower bits and a binary tree basically works around them. So I do not recommend even bothering with this. Maybe it is worth it on MCU.

@K0rkunc
Copy link

K0rkunc commented Oct 27, 2021

ok what is the advantage of having s-pwm

It is supposed to increase the perceptive refresh rate. Which is usefully for reducing flicker and improving image appearance. It also is helpful if you want the image to be shown on camera or video. I have proven in certain cases this is accomplished however there is a possible increase in instability also in certain cases. It would be an option or tool to use to dial in operation, but may be considered advanced. I suspect this is used in receiver cards. It would likely work a little better in MCU or FPGA, but it may also have some benefit here also.

This is more commonly done on PWM LED drivers. However they do different things like use actual PWM instead of BCM. I want to say it is commonly implemented using a binary tree. Here is some mention of it for a memory mapped LED driver: (See section 3.6.2) https://www.ti.com/lit/an/slva645/slva645.pdf?ts=1635356532428

Due to CPU complexity this code base pretty much has to do BCM. Binary tree approach is probably more complicated than its worth. I cannot speak for the quality difference between using a binary tree or BCM time division vs PWM as I do not know. For all I know the receiver cards use binary tree implementation. This is low effort proof of concept. Have done testing enough to show it has positives and negatives.

The Pi is not likely stable enough to derive time from IO like a FPGA can. Instead it has to use BCM with a timer to accomplish mostly the same thing. With derived time you can do standard PWM, and when done on FPGA you do not have the massive CPU overhead. This makes a binary tree easier. The lower segments would have to be retransmitted constantly which again I am not sure is a good idea.

A binary tree may be possible on a MCU despite the CPU issue using a look up table. The idea here being a MCU can do IO with more stability. However the memory shortage is likely a huge issue. This would force you to move to low segment bits which will increase the row change overhead. Therefore I am not sure it makes sense to a binary tree on anything but hardware (PWM LED driver) or FPGA. Doing PWM over shift register is really hard for processors. That is what each segment would be with proper binary tree.

What could be done is a small binary tree conversion in set pixel. This however would still be implemented with BCM, so maybe the tree does something special. I do not know. However this library does not generally respect the lower bits and a binary tree basically works around them. So I do not recommend even bothering with this. Maybe it is worth it on MCU.

i want to try s-pwm i what files should i use =?

@greatballoflazers
Copy link
Author

Just download my fork. Then just build it as normal.

make -j 4

For a list of changed files see files changed. I change 7 files for this commit.

To change the number segment bits use --led-seg-bits=5. 8 is used as the default. Range of 1 to 11 is supported. (It uses
kBitPlanes to set the upper limit.)

@K0rkunc
Copy link

K0rkunc commented Oct 28, 2021

Just download my fork. Then just build it as normal.

make -j 4

For a list of changed files see files changed. I change 7 files for this commit.

To change the number segment bits use --led-seg-bits=5. 8 is used as the default. Range of 1 to 11 is supported. (It uses kBitPlanes to set the upper limit.)

I want to add directly to my own files, how do I do it?

Could you please take a look at the link below?
#1354

@hzeller
Copy link
Owner

hzeller commented Oct 30, 2021

Thanks for your contribution @greatballoflazers !

I am currently travelling, so I am a bit slow to respond. I think later next week I'll have time to have a closer look at your pull request.

include/led-matrix.h Outdated Show resolved Hide resolved
include/led-matrix-c.h Outdated Show resolved Hide resolved
@greatballoflazers
Copy link
Author

That compiler error does not happen on Pi 2. It does happen if I compile on x86.

@greatballoflazers
Copy link
Author

greatballoflazers commented Nov 28, 2021

Added in few changes for PWM drivers. Missing core logic from ESP32. Missing classes for driver commands and commandline selection logic. PWM driver logic currently does not support S-PWM logic.

What this adds is memory overhaul. I still need to look into set pixel logic.

@Xaver88
Copy link

Xaver88 commented May 12, 2022

@greatballoflazers
tried to compile your fork, but get many errors in framebuffer.cc
is there a working file?

@Xaver88
Copy link

Xaver88 commented May 12, 2022

all the errors at the fork of the download of greatballoflazers

pi@raspberrypi:~/rpi-rgb $ make -j 4
make -C ./lib
make[1]: Entering directory '/home/pi/rpi-rgb/lib'
g++ -I../include -W -Wall -Wextra -Wno-unused-parameter -O3 -g -fPIC -DDEFAULT_HARDWARE='"regular"' -fno-exceptions -std=c++11 -c -o framebuffer.o framebuffer.cc
In file included from framebuffer.cc:20:
framebuffer-internal.h:177:43: error: expected class-name before ‘{’ token
class PWMFramebuffer : public FrameBuffer {
^
framebuffer.cc:916:1: error: ‘PWMFrameBuffer’ does not name a type; did you mean ‘PWMFramebuffer’?
PWMFrameBuffer::PWMFramebuffer(int rows, int columns, int parallel,
^~~~~~~~~~~~~~
PWMFramebuffer
framebuffer.cc:924:6: error: ‘PWMFrameBuffer’ has not been declared
bool PWMFrameBuffer::SetPWMBits(uint8_t value) {
^~~~~~~~~~~~~~
framebuffer.cc: In function ‘bool rgb_matrix::internal::SetPWMBits(uint8_t)’:
framebuffer.cc:925:10: error: ‘FrameBuffer’ has not been declared
return FrameBuffer::SetPWMBits(value % (driver_bits_ + 1));
^~~~~~~~~~~
framebuffer.cc:925:43: error: ‘driver_bits_’ was not declared in this scope
return FrameBuffer::SetPWMBits(value % (driver_bits_ + 1));
^~~~~~~~~~~~
framebuffer.cc:925:43: note: suggested alternative: ‘gpio_bits_t’
return FrameBuffer::SetPWMBits(value % (driver_bits_ + 1));
^~~~~~~~~~~~
gpio_bits_t
framebuffer.cc: At global scope:
framebuffer.cc:928:6: error: ‘PWMFrameBuffer’ has not been declared
void PWMFrameBuffer::DumpToMatrix(GPIO *io, int pwm_bits_to_show) {
^~~~~~~~~~~~~~
framebuffer.cc:932:21: error: ‘PWMFrameBuffer’ has not been declared
inline gpio_bits_t PWMFrameBuffer::ValueAt(int double_row, int column, int bit) {
^~~~~~~~~~~~~~
framebuffer.cc: In function ‘gpio_bits_t
rgb_matrix::internal::ValueAt(int, int, int)’:
framebuffer.cc:933:11: error: ‘bitplane_buffer_’ was not declared in this scope
return &bitplane_buffer_[ double_row * (columns_ * kBitPlanes)
^~~~~~~~~~~~~~~~
framebuffer.cc:933:11: note: suggested alternative: ‘PWMFramebuffer’
return &bitplane_buffer_[ double_row * (columns_ * kBitPlanes)
^~~~~~~~~~~~~~~~
PWMFramebuffer
framebuffer.cc:933:43: error: ‘columns_’ was not declared in this scope
return &bitplane_buffer_[ double_row * (columns_ * kBitPlanes)
^~~~~~~~
framebuffer.cc:933:43: note: suggested alternative: ‘column’
return &bitplane_buffer_[ double_row * (columns_ * kBitPlanes)
^~~~~~~~
column
framebuffer.cc:933:54: error: ‘kBitPlanes’ was not declared in this scope
return &bitplane_buffer_[ double_row * (columns_ * kBitPlanes)
^~~~~~~~~~
make[1]: *** [Makefile:185: framebuffer.o] Error 1
make[1]: Leaving directory '/home/pi/rpi-rgb/lib'
make: *** [Makefile:15: lib/librgbmatrix.a] Error 2

@ledvinap
Copy link
Contributor

@greatballoflazers :
If I understand the code correctly, you split topmost (segment) bits into separate pulses.
PWM dithering is doing similar thing with lest significant bits, IMO with better results

@bluelasers
Copy link

I recommend closing this out due to patent concerns.

@ledvinap This was originally created for other purposes which do not really apply here.

@marcmerlin
Copy link
Contributor

I recommend assuming bluelasers is still David Tacher and to safely ignore him.
see #1468 (comment)

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.

7 participants