Skip to content

Missing Color Order in KMS DPI Overlay #6155

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

Closed
othermod opened this issue May 10, 2024 · 8 comments
Closed

Missing Color Order in KMS DPI Overlay #6155

othermod opened this issue May 10, 2024 · 8 comments

Comments

@othermod
Copy link

Describe the bug

The vc4-kms-dpi-generic-overlay.dts overlay is missing some capability that previously existed for changing the color order of DPI output. It gives only bgr888 and rgb888 as options. I developed a product using red instead of green in the middle group, and I'm currently unable to use the KMS DPI overlay.

The previous DPI method gave 4 color orders for RGB
rgb_order:
1: DPI_RGB_ORDER_RGB
2: DPI_RGB_ORDER_BGR
3: DPI_RGB_ORDER_GRB
4: DPI_RGB_ORDER_BRG

It is possible to bring back this capability that previously existed?

Steps to reproduce the behaviour

Use the KMS DPI overlay on a product (such as my PSPi 6) that makes use of the previously available DPI color orders that use red in the center group instead of green.

Device (s)

Raspberry Pi CM4

System

Raspberry Pi OS Bookworm

Logs

No response

Additional context

No response

@6by9
Copy link
Contributor

6by9 commented May 10, 2024

Possible but painful.

All the formats are represented by MEDIA_BUS_FMT_xxx defines in the kernel from https://github.com/torvalds/linux/blob/master/include/uapi/linux/media-bus-format.h#L37.
To add GRB and BRG means new defines for each bit depth of interest, so potentially 10 new formats when you consider all the supported modes.

As it's a set of defines shared with mainline Linux, we either have to upstream the defines (a moderate pain), or somehow handle the potential for downstream to have extra defines that aren't in mainline without creating merge conflicts(*) and account for what happens should someone actually upstream the formats properly.

I'm tempted to do it as a bit of a hack via a DT override on the dpi driver node that tells it to ignore the component order from the MEDIA_BUS_FMT and adopt the specified value. Doing so won't be acceptable to mainline though.

@pelwell what's your feeling? #6156 is a quick patch for the override option.

(*) Probably take some values like 0x1f00 - 0x1f0a which won't be used for many years.

@othermod
Copy link
Author

Thank you. I appreciate anything you are willing to do. I tried working this out on my own without luck.
The KMS overlay also appears to lack the ability to disable HSYNC and VSYNC (using DE mode on an LCD, and using gpio2 and gpio3 for I2C). I got around this by just setting the pins manually after bootup. I can create a separate issue later on if it becomes a problem. Just wanted to bring it up while you're considering options.

@6by9
Copy link
Contributor

6by9 commented May 10, 2024

The KMS overlay also appears to lack the ability to disable HSYNC and VSYNC (using DE mode on an LCD, and using gpio2 and gpio3 for I2C). I got around this by just setting the pins manually after bootup. I can create a separate issue later on if it becomes a problem. Just wanted to bring it up while you're considering options.

Again it's more tedious than anything complex, although disabling H & V sync sounds like a weird display device (how do you resync the frame start?!).

https://github.com/raspberrypi/linux/blob/rpi-6.6.y/arch/arm/boot/dts/broadcom/bcm270x.dtsi#L222-L268 lists out the sets of GPIOs assigned to DPI for each of the options. We have defined blocks with GPIOs 0 (PCK) & 1 (DE) disabled, but not with 2 & 3 disabled. Those can be defined easily enough, but you then compound up the number of overrides on the overlay eg rgb888 becomes
rgb888
rgb888_no_de_pck
rgb888_no_vsync
rgb888_no_hsync
rgb888_no_hv_sync
and any other permutation you can think of.

TBH it's at that point you should be creating a PR to add your panel to the vc4-kms-dpi-panel overlay, as then your panel name can select any of a much larger options. The generic option can only go so far.

@othermod
Copy link
Author

othermod commented May 10, 2024

Again it's more tedious than anything complex, although disabling H & V sync sounds like a weird display device (how do you resync the frame start?!).

DE

It uses DE going low for a period and then going high to signal a new frame. I use lots of 4.3" 480x272 and 800x480 displays, and I've never found an LCD that doesn't support the mode. The usually just require HSYNC and VSYNC be tied to GND to set it.

I'll work on getting an understanding of the code in the two links you sent. I had trouble getting this overlay to compile locally for testing. I've compiled some in the past for audio and DPI, but those were a bit simpler and my skill level is somewhere between novice and competent.

@othermod
Copy link
Author

Is it possible to do something like the magic number/dpi_output_format method that was used previously, and just stick with a single generic overlay that everyone can use? It might make the overlay lengthy, but it would remove the need for each special situation to make a different overlay and cause the overlay list to get bigger and bigger.

@6by9
Copy link
Contributor

6by9 commented May 10, 2024

I'll work on getting an understanding of the code in the two links you sent. I had trouble getting this overlay to compile locally for testing. I've compiled some in the past for audio and DPI, but those were a bit simpler and my skill level is somewhere between novice and competent.

The overlay uses defines included from the main kernel tree for things like GPIO_ACTIVE_HIGH.
The easiest route is probably to follow the main docs for building the kernel, but instead of make -j4 zImage modules dtbs, you only want make -j4 dtbs

Is it possible to do something like the magic number/dpi_output_format method that was used previously, and just stick with a single generic overlay that everyone can use? It might make the overlay lengthy, but it would remove the need for each special situation to make a different overlay and cause the overlay list to get bigger and bigger.

Not really. The kernel wants explicit configuration blocks, and spread across a number of DT nodes / kernel subsystems.

vc4-kms-dpi-panel is there as a wrapper for defined panels, and it least it then makes it controlled. I don't know if you're making panels for commercial products or for fun, but having random config.txt runes generally doesn't scale well, and is error prone.
I'll be quite sad if I start to see products being released where the documentation says "add dtoverlay=vc4-kms-dpi-generic,<long list of params>, instead of dtoverlay=vc4-kms-dpi-panel,<vendor_panel>.

Generic overlays will always have limitations, which is why product specific overlays are there.

@othermod
Copy link
Author

othermod commented May 10, 2024

The easiest route is probably to follow the main docs for building the kernel, but instead of make -j4 zImage modules dtbs, you only want make -j4 dtbs

Thanks. That compiled them. I assume I still won't be able to change the color order though, unless your RGB order override works.

I don't know if you're making panels for commercial products or for fun

I'm not making panels. I make a board for the Playstation Portable that lets you put a Pi Zero or CM4 (and hopefully CM5 soon) in it , and the project got a little more popular than I expected. I'm just a random dude who likes 90s games.
https://github.com/othermod/PSPi-Version-6

@othermod
Copy link
Author

I updated the kernel with rpi-update, and the color order works perfectly. Thank you so much.

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

No branches or pull requests

2 participants