-
Notifications
You must be signed in to change notification settings - Fork 5.1k
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
drm: rp1: rp1-dpi: Add "rgb_order" property (to match VC4 DPI) #6597
drm: rp1: rp1-dpi: Add "rgb_order" property (to match VC4 DPI) #6597
Conversation
I think this is correct, my only reticence (and the reason it's still a draft) is code size. |
@@ -353,6 +354,20 @@ static int rp1dpi_platform_probe(struct platform_device *pdev) | |||
if (ret) | |||
goto done_err; | |||
|
|||
dpi->rgb_order_override = -1; |
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.
My only comment is that this could potentially be defined as RP1DSPI_ORDER_UNCHANGED, and check for that by name instead of < 0
at line 423.
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.
It's copied from vc4_dpi.c (though in that instance the values were from a register field).
BTW I also toyed with encoding the lookup order more directly in the variable (values like 0x020100) to avoid the switch statement.
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.
vc4_dpi.c isn't always the best example, and there is no requirement to copy how it is implemented in the code, only the DT side.
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.
Added RP1DSI_ORDER_UNCHANGED
as an explicit value, and re-worked the lookup to use bit-shifts (which makes both object and source code slightly smaller. That last & 3
is superstitious since the values are internal).
Tested on RGB666_CPADHI (Mode 6) and RGB666 (Mode 5) HATs; kmstest with all supported 4CCs over all values of rgb_order. The colours seemed to be going to the right places as far as I could see. Also tested RGB565 (Mode 2) and RGB888 (Mode 7) albeit without appropriate hardware, and the output looked plausible. |
Tested on my hardware as well. Verified rgb888 with rgb-order of brg and grb work as expected. |
As on VC4, the OF property overrides the order implied by media bus format. Only 4 of the 6 possible orders are supported. New add-on hardware designs should not rely on this "legacy" feature. Signed-off-by: Nick Hollinghurst <nick.hollinghurst@raspberrypi.com>
7a24a97
to
0edb4d3
Compare
kernel: configs: add CONFIG_SND_SOC_WM8904=m See: raspberrypi/linux#6623 kernel: drm: rp1: rp1-dpi: Add rgb_order property (to match VC4 DPI) See: raspberrypi/linux#6597
kernel: configs: add CONFIG_SND_SOC_WM8904=m See: raspberrypi/linux#6623 kernel: drm: rp1: rp1-dpi: Add rgb_order property (to match VC4 DPI) See: raspberrypi/linux#6597
As on VC4, the OF property overrides the order implied by media bus format. Only 4 of the 6 possible orders are supported. New add-on hardware designs should not rely on this "legacy" feature.
This is is an attempt to deal with #6505.
Not all combinations of DRM pixel format, bus format and rgb_order have been tested yet.