drivers: display: Add LED-Strip matrix display driver #68614

soburi · 2024-02-06T10:43:22Z

This type of product is available from Adafruit's Adafruit NeoPixel Shield for Arduino.
But electrically, it is just an LED strip, so it is a general-purpose product.
https://www.adafruit.com/product/1430

There is no problem in merging this patch itself, but
Changes may occur depending on the progress of #68514 and #67610.

I verify this PR with 16x16 WS2812 matrix with following overlay.

samples/drivers/display/boards/rpi_pico.overlay

#include <zephyr/dt-bindings/led/led.h>
#include <dt-bindings/pinctrl/rpi-pico-rp2040-pinctrl.h>

/ {
        aliases {
                led-strip = &ws2812;
        };
};

&pinctrl {
        ws2812_pio0_default: ws2812_pio0_default {
                ws2812 {
                        pinmux = <PIO0_P21>;
                };
        };
};

&pio0 {
        status = "okay";

        pio-ws2812 {
                compatible = "worldsemi,ws2812-rpi_pico-pio";
                status = "okay";
                pinctrl-0 = <&ws2812_pio0_default>;
                pinctrl-names = "default";
                bit-waveform = <3>, <3>, <4>;

                ws2812: ws2812 {
                        status = "okay";
                        output-pin = <21>;
                        chain-length = <255>;
                        color-mapping = <LED_COLOR_ID_GREEN
                                         LED_COLOR_ID_RED
                                         LED_COLOR_ID_BLUE>;
                        reset-delay = <280>;
                        frequency = <800000>;
                };

        };
};

/ {
        chosen {
                zephyr,display = &led_strip_matrix;
        };

        led_strip_matrix: led-strip-matrix {
                compatible = "led-strip-matrix";
                status = "okay";

                led-strip = <&ws2812>;
                width = <16>;
                height = <16>;
                start-from-right;
        };
};

samples/drivers/display/boards/rpi_pico.conf

CONFIG_WS2812_STRIP_RPI_PICO_PIO=y
CONFIG_LOG=y
CONFIG_LED_STRIP=y
CONFIG_LED_STRIP_LOG_LEVEL_DBG=y
CONFIG_WS2812_STRIP=y

dts/bindings/display/led-strip-matrix.yaml

simonguinot · 2024-02-06T15:29:51Z

This is fun :)

fabiobaltieri

Cool, can you also add a build case in tests/drivers/build_all/display/?

drivers/display/Kconfig.led_strip_matrix

drivers/display/display_led_strip_matrix.c

fabiobaltieri · 2024-02-08T12:13:22Z

@soburi give it a rebase, the display test was broken and the format changed

soburi · 2024-02-08T12:16:55Z

@fabiobaltieri

@soburi give it a rebase, the display test was broken and the format changed

Thank you for the notification.
I'm correcting the points raised in the review now, so I will address it together.

fabiobaltieri

Cool! Just a nitpicky comments about variable names from me.

drivers/display/display_led_strip_matrix.c

soburi · 2024-02-28T23:15:07Z

@kartben
Could you take a look, please?

simonguinot

Hi @soburi,

Thanks for this nice driver. I will definitely use it !

Please find few comments and questions below. I'll try to review the code next week.

dts/bindings/display/led-strip-matrix.yaml

simonguinot · 2024-03-01T16:05:31Z

dts/bindings/display/led-strip-matrix.yaml

+        [ 7][ 6][ 5][ 4]
+        [11][10][ 9][ 8]
+        [15][14][13][12]
+


Should you also handle the case where the first LED is at the bottom of the display matrix ? start-from-bottom ?

Added start-from-bottom.

simonguinot · 2024-03-01T16:22:35Z

dts/bindings/display/led-strip-matrix.yaml

+
+      * Modules are arranged circulative from top left to bottom right.
+        [M0][M1]
+        [M2][M3]


And what about supporting a serpentine layout of modules ? :)

Is it common to have a large grid/array of LED assembled from several smaller modules ?

Since I was expecting a module like this that the user could connect with the connector, I didn't give much thought to the arrangement of the modules. However, the serpentine arrangement can have shorter wiring.
The placement can now be specified using the same rules as for LEDs.

tests/drivers/build_all/display/app.overlay

drivers/display/display_led_strip_matrix.c

simonguinot · 2024-03-01T17:57:51Z

dts/bindings/display/led-strip-matrix.yaml

+    description: |
+      Specify the LED strip that is the substance of the matrix.
+      If more than one element is specified, split the matrix into multiple strips.
+      The amount of LEDs must equal the [width * height] value.


Maybe it is worth mentioning that the phandles of LED strip controllers must be ordered based on their position in the "flattened" chain of pixels, and that the strip controller must be chained "simply" (i.e. all LEDs of controller 1, all LED of controller 2, etc).

Added explanation. Could you check, please?

kartben

this is very cool stuff! ❤️

kartben · 2024-03-04T18:29:24Z

dts/bindings/display/led-strip-matrix.yaml

+
+  width:
+    description: |
+      Specifies the overall `width` of the matrix.


nit to be consistent with height:

Suggested change

Specifies the overall `width` of the matrix.

Specifies the overall width of the matrix.

kartben · 2024-03-04T21:54:38Z

drivers/display/display_led_strip_matrix.c

+	caps->supported_pixel_formats = PIXEL_FORMAT_ARGB_8888;
+	caps->current_pixel_format = PIXEL_FORMAT_ARGB_8888;


btw was there a reason for not using PIXEL_FORMAT_RGB_888? It doesn't look like alpha channel can be used in any meaningful way so having 24-bit per colour instead of 32 could save quite a bit of memory.

I changed to be able to select pixel format on devicetree.

kartben

on a second thought, adding back a request for change regarding my latest comment about pixel format.

kartben · 2024-03-05T08:28:35Z

dts/bindings/display/led-strip-matrix.yaml

+      This property only accept PANEL_PIXEL_FORMAT_RGB_888 and PANEL_PIXEL_FORMAT_RRGB_8888.
+      If not set this property, the behavior is same as selecting PANEL_PIXEL_FORMAT_RGB_888.


Suggested change

This property only accept PANEL_PIXEL_FORMAT_RGB_888 and PANEL_PIXEL_FORMAT_RRGB_8888.

If not set this property, the behavior is same as selecting PANEL_PIXEL_FORMAT_RGB_888.

This property only accepts PANEL_PIXEL_FORMAT_RGB_888 and PANEL_PIXEL_FORMAT_RRGB_8888.

If this property is not set, the behavior is same as selecting PANEL_PIXEL_FORMAT_RGB_888.

btw why not just use PANEL_PIXEL_FORMAT_RGB_888 (1) as a default value here?

I didn't deeply consider it. It's easier to understand if you explicitly set the default value, so I've modified it that way.

Adds a driver for a display of LED strips arranged in a grid. Signed-off-by: TOKITA Hiroshi <[email protected]>

soburi force-pushed the led-strip-matrix-driver branch 2 times, most recently from d4a7fcb to 14b73c7 Compare February 6, 2024 10:50

soburi marked this pull request as ready for review February 6, 2024 11:54

zephyrbot added area: Devicetree Binding PR modifies or adds a Device Tree binding area: Display labels Feb 6, 2024

zephyrbot requested review from danieldegrasse, galak and jfischer-no February 6, 2024 11:54

zephyrbot assigned galak Feb 6, 2024

soburi requested review from simonguinot, kartben and fabiobaltieri February 6, 2024 11:55

kartben reviewed Feb 6, 2024

View reviewed changes

dts/bindings/display/led-strip-matrix.yaml Show resolved Hide resolved

dts/bindings/display/led-strip-matrix.yaml Outdated Show resolved Hide resolved

dts/bindings/display/led-strip-matrix.yaml Outdated Show resolved Hide resolved

fabiobaltieri assigned kartben and jfischer-no and unassigned galak Feb 6, 2024

fabiobaltieri requested review from bbilas and Mani-Sadhasivam February 6, 2024 16:14

fabiobaltieri reviewed Feb 6, 2024

View reviewed changes

soburi force-pushed the led-strip-matrix-driver branch from 14b73c7 to edd43aa Compare February 8, 2024 06:30

soburi force-pushed the led-strip-matrix-driver branch 4 times, most recently from 82fbd38 to 7d9f1a6 Compare February 8, 2024 23:19

soburi changed the title ~~drivers: display: Add LED-Strip matrix display driver.~~ drivers: display: Add LED-Strip matrix display driver Feb 8, 2024

soburi requested review from kartben and fabiobaltieri February 9, 2024 00:14

fabiobaltieri reviewed Feb 9, 2024

View reviewed changes

drivers/display/display_led_strip_matrix.c Outdated Show resolved Hide resolved

soburi requested a review from fabiobaltieri February 9, 2024 21:21

soburi force-pushed the led-strip-matrix-driver branch from 7d9f1a6 to 6a96534 Compare February 9, 2024 21:26

fabiobaltieri previously approved these changes Feb 9, 2024

View reviewed changes

simonguinot mentioned this pull request Mar 1, 2024

boards: shields: Adding Adafruit 5x5 Neopixel Grid BFF #67610

Merged

simonguinot reviewed Mar 1, 2024

View reviewed changes

soburi dismissed fabiobaltieri’s stale review via 79897c3 March 3, 2024 05:33

soburi force-pushed the led-strip-matrix-driver branch from 6a96534 to 79897c3 Compare March 3, 2024 05:33

soburi requested review from fabiobaltieri and simonguinot March 3, 2024 09:07

fabiobaltieri previously approved these changes Mar 4, 2024

View reviewed changes

kartben approved these changes Mar 4, 2024

View reviewed changes

kartben reviewed Mar 4, 2024

View reviewed changes

kartben requested changes Mar 4, 2024

View reviewed changes

soburi dismissed fabiobaltieri’s stale review via 5c635a1 March 5, 2024 01:13

soburi force-pushed the led-strip-matrix-driver branch 2 times, most recently from 5c635a1 to 7d6239f Compare March 5, 2024 01:17

soburi requested review from kartben and fabiobaltieri March 5, 2024 02:52

kartben reviewed Mar 5, 2024

View reviewed changes

drivers: display: Add LED-Strip matrix display driver

3ad79a5

Adds a driver for a display of LED strips arranged in a grid. Signed-off-by: TOKITA Hiroshi <[email protected]>

soburi force-pushed the led-strip-matrix-driver branch from 7d6239f to 3ad79a5 Compare March 5, 2024 09:53

soburi requested a review from kartben March 5, 2024 10:39

kartben approved these changes Mar 5, 2024

View reviewed changes

fabiobaltieri approved these changes Mar 5, 2024

View reviewed changes

nashif merged commit ca520f8 into zephyrproject-rtos:main Mar 5, 2024
21 checks passed

soburi deleted the led-strip-matrix-driver branch March 5, 2024 21:10

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

drivers: display: Add LED-Strip matrix display driver #68614

drivers: display: Add LED-Strip matrix display driver #68614

soburi commented Feb 6, 2024 •

edited

Loading

simonguinot commented Feb 6, 2024

fabiobaltieri left a comment

fabiobaltieri commented Feb 8, 2024

soburi commented Feb 8, 2024

fabiobaltieri left a comment

soburi commented Feb 28, 2024

simonguinot left a comment

simonguinot Mar 1, 2024

soburi Mar 3, 2024 •

edited

Loading

simonguinot Mar 1, 2024

soburi Mar 3, 2024 •

edited

Loading

simonguinot Mar 1, 2024 •

edited

Loading

soburi Mar 3, 2024

kartben left a comment

kartben Mar 4, 2024

soburi Mar 5, 2024

kartben Mar 4, 2024

soburi Mar 5, 2024

kartben left a comment

kartben Mar 5, 2024

kartben Mar 5, 2024

soburi Mar 5, 2024

	Specifies the overall `width` of the matrix.
	Specifies the overall width of the matrix.

		caps->supported_pixel_formats = PIXEL_FORMAT_ARGB_8888;
		caps->current_pixel_format = PIXEL_FORMAT_ARGB_8888;

		This property only accept PANEL_PIXEL_FORMAT_RGB_888 and PANEL_PIXEL_FORMAT_RRGB_8888.
		If not set this property, the behavior is same as selecting PANEL_PIXEL_FORMAT_RGB_888.

drivers: display: Add LED-Strip matrix display driver #68614

drivers: display: Add LED-Strip matrix display driver #68614

Conversation

soburi commented Feb 6, 2024 • edited Loading

simonguinot commented Feb 6, 2024

fabiobaltieri left a comment

Choose a reason for hiding this comment

fabiobaltieri commented Feb 8, 2024

soburi commented Feb 8, 2024

fabiobaltieri left a comment

Choose a reason for hiding this comment

soburi commented Feb 28, 2024

simonguinot left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

soburi Mar 3, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

soburi Mar 3, 2024 • edited Loading

Choose a reason for hiding this comment

simonguinot Mar 1, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kartben left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kartben left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

soburi commented Feb 6, 2024 •

edited

Loading

soburi Mar 3, 2024 •

edited

Loading

soburi Mar 3, 2024 •

edited

Loading

simonguinot Mar 1, 2024 •

edited

Loading