rockchip64-6.15: Fix DP alt mode on some rk3399 boards #8271
Conversation
hyx0329
requested review from
ColorfulRhino,
JohnTheCoolingFan,
SuperKali,
TRSx80,
TheSnowfield,
Tonymac32,
ahoneybun,
amazingfate,
brentr,
catalinii,
clee,
efectn,
igorpecovnik,
joekhoobyar,
krachlatte,
lanefu,
paolosabatino,
prahal,
rpardini and
schwar3kat
as code owners
WalkthroughThe linux-rockchip64-edge kernel configuration was modified to add the Suggested labels
Suggested reviewers
📜 Recent review detailsConfiguration used: CodeRabbit UI ⛔ Files ignored due to path filters (16)
📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
✨ Finishing Touches🧪 Generate Unit Tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
github-actions
bot
added
size/large
|
Looks interesting; perhaps I can find some time during this weekend to give a check on Opi4 LTS. I'm only dubious about what the ~20 new patches for rk3399 do, but anyway... |
github-actions
bot
removed
the
Ready to merge
|
I think this will be a "partial" fix. With a few more tests with different devices & configurations, 2 major flaws are found:
I couldn't find the root cause of them. Loading a gadget driver only works around the first flaw. If the behavior change and the workaround are acceptable, I think it's okay to merge. I'll look into it if the workaround should be included by default for each affected boards. Behavior change and comparisonOnly on RK3399
The old behavior can still be preserved with some tweaks to the device tree, if it's desired. Affected boardsAs far as I know, only boards mentioned in this pr are affected. Other RK3399 boards are not affected because their type-c ports are either fully functional or not working at all, if they exist. Other non-RK3399 boards should be not affected. To be affected, RK3399 boards will have to leave |
|
Thanks @hyx0329 for the summary overview table, I have a couple of questions though. This sentence confuses me a bit:
I thought the "old implementation" in the mainline kernel - regarding the RK3399 SoC in general - is limited and/or partially broken; First of all, the patches in this PR either fix and expand the functionalities for the RK3399 SoC, then the corrections to the device trees declare the hardware in the right way to make those functionalities available to the kernel. About the sentence:
do you mean that if the USB device has some kind of power supply of its own won't work anymore? Thanks for any answer, this whole PR is definitely intersting but the risk of breakage and the patches it carries along (that will require maintenance by armbian maintainers, in the future) are my main concerns, so I would like to understand as much as I can. |
The "old implementation" contains 2 parts: existing drivers in mainline kernel, and the patch
But upstream misses the extcon bridge(shared state) connecting all those components. The patch The main reason that I created this PR is to make type-c work in both orientations, which should make it more comfortable to use. The patches in this PR mainly bring these things:
As you can see, there are still a few less relevant patches. Some are prerequisites, and others are minor improvements. And yes the device trees are then updated to make those functionalities available. The functions have to be explictly specified per capable board. Because dwc3 is a common driver, it can affect a wide range of boards if things go wrong.
I think I made a mistake. Now I would say smarter devices which can negotiate data role or power role through USB-C may encounter issues. What I observed:
I thought the reason was power, but the fact seems to be their ability to negotiate data role and power role. I just tested a simple battery powered RP2040 gadget.
Understood. That is being responsible :) |
|
My tests on my Orange PI 4 LTS. Test hardware is:
Patched kernelUsing a powered USB Hub to power the board:
Using the Hub without power:
Direct connection of a USB device to type-c port:
Kernel without patchesUsing a powered USB Hub to power the board:
Direct connection of a USB device to type-c port:
@hyx0329 I don't know if these tests report the same behaviour you experienced, it looks a bit odd to me that the case "Patched kernel + Hub without power" the Type-C port is practically non-working and "Patched kernel + Power Hub" makes both HDMI and USB devices work too. Also the patches change the way the type-c port work when there is a powered Hub connected to the port; if a non-OTG device is directly connected to the port, it behaves substantially the same as before. I see a possible regression for people who use the Type-C port with a Hub to power the device and connects one or more USB devices to the Hub; in such case, if At the moment, the |
|
Things become quite complicated. I'm not confident with the changes now. Anyway, here's my report. Devices:
Kernel without patchesAll results of kernel w/o patches are expected, except that I thought USB 2.0 should always work when dr_mode is set to "host". I haven't done a fresh test yet. Kernel with patchesIf something like In the process, I met once that the port became unresponsive after multiple plugs and unplugs. dr_mode = "otg"
dr_mode = "host"It's not expected to work properly though.
During testing, I found that if the a simple device is connected after a first connection to a hub, it may not be detected at all, while the hub will still work. It seems not affecting those USB2-only devices like yubikeys, and devices connected through a USB 2.0 cable. It is possibly related to the internal state of the driver/phy hardware. Maybe I didn't fully understand the old behavior. About mixed dr_mode
For those with type-c ports, From my understanding, a full-featured type-c port on a rk3399 board consists a USB 2.0 OTG peripheral and a set of USB 3.0 / DP multi-function peripherals: Because of the hardware design, if we need DP output through type-c port, we need to reconfigure the type-c phy( Because of the driver design and possibly other reasons I don't understand, the child USB3 phy of tcphy is consumed by dwc3 driver, therefore the reset of the phy should be performed by dwc3 driver to maintain consistent state. Currently I'm not sure whether it's feasible to reconfigure the tcphy outside dwc3 driver. If no extcon/usb-role-switch is defined for |
|
I made another round of tests, this time I guess I was more rigorous and systematic than in the first round. Hardware:
In the table Unpatched is kernel 6.16-rc3 and Patched is kernel 6.15.3 + USB patches. Direct connection
Connection via Unpowered Hub
Connection via Powered Hub (powering the board via PD)
This looks more reasonable to me than the previous test and, most important, there seems to be no regressions and in some cases some gains. I guess my USB3 Type-C hub has some role in this (especially the unpowered case), but the direct connection also has its own issues. Direct connection also passes through an adapter which I don't know it is really suitable to handle OTG to Host mode (but I hope so, those things are usually made to attach USB sticks to phones...) |
|
After a quick comparison on the device trees, it looks to me that the opi4lts device tree misses these lines: https://github.com/armbian/build/pull/8271/files#diff-b72b6fb9641cade670e223a16c0ecdd89dad4a06d460762c20e9a3bdad6248c4L451-L454 I'm pretty sure the very first tests I did, I included those changes in my device tree; in the latest test instead I used the device tree coming from this PR. Perhaps this can explain the differences? I will fix the device tree and give a chance to another round of checks. |
Actually I did suspect those changes were the cause, but my tests didn't support it. |
Tried again with fixed DT, nothing changed. Now the thing becomes really puzzling 🤔 |
|
I could have a check with direct Type-C to HDMI cable and the display still does not get detected in orientation. The only way I could get any display output with I guess at this point the board is not really capable, could it be? |
I'm not sure. The schematic shows the board should have all required bits. And since it's possible to have DP and USB3 work in some conditions, there should be a way to make it work. When I have this line, the type-c port no longer works. Normally type-c port are not powered until negotiated(via CC pins pulls and PD messages). Maybe you want to check that. |
This could indeed be important and I didn't notice that while I was reviewing the device tree. I will remove the property and check again soon, thanks! |
|
@hyx0329 that's, the
Some other nice things are that the Type-C Hub hotplugging and Hub HDMI hot-plugging work very well! I noticed the same issue as yours when I use PD via Type-C Hub: USB device does not get recognized, altough I have to admit that this problem also happens on my daily job laptop... also, looking at my previous tests were USB devices were working with PD over Type-C and This is the regulator summary with PD, HDMI and USB device plugged (but not detected) in the Type-C Hub: root@orangepi4-lts:/sys/kernel/debug/regulator# cat regulator_summary
regulator use open bypass opmode voltage current min max
---------------------------------------------------------------------------------------
regulator-dummy 4 3 0 unknown 0mV 0mA 0mV 0mV
ff940000.hdmi-avdd-1v8 1 0mA 0mV 0mV
ff940000.hdmi-avdd-0v9 1 0mA 0mV 0mV
vdd_log 1 0 0 unknown 1400mV 0mA 800mV 1400mV
vcc_sys 3 2 0 unknown 0mV 0mA 0mV 0mV
vcc3v3_sys 18 17 0 unknown 3300mV 0mA 3300mV 3300mV
vdd_center 1 0 0 unknown 900mV 0mA 750mV 1350mV
vdd_cpu_l 2 1 0 unknown 825mV 0mA 750mV 1350mV
cpu0-cpu 1 0mA 825mV 1250mV
vcc_ddr 1 0 0 unknown 3300mV 0mA 0mV 0mV
vcc1v8 2 1 0 unknown 1800mV 0mA 1800mV 1800mV
ff100000.saradc-vref 1 0mA 0mV 0mV
vcc1v8_dvp 1 1 0 unknown 1800mV 0mA 1800mV 1800mV
ff770000.syscon:io-domains-bt656 0 0mA 0mV 0mV
vcc3v0_touch 1 0 0 unknown 3000mV 0mA 3000mV 3000mV
vcc1v8_pmu 1 0 0 unknown 1800mV 0mA 1800mV 1800mV
vcc_sdio 2 2 0 unknown 1800mV 0mA 1800mV 3000mV
fe320000.mmc-vqmmc 1 0mA 1800mV 1950mV
ff770000.syscon:io-domains-sdmmc 0 0mA 0mV 0mV
vcca3v0_codec 1 0 0 unknown 3000mV 0mA 3000mV 3000mV
vcc_1v5 1 0 0 unknown 1500mV 0mA 1500mV 1500mV
vcca1v8_codec 1 1 0 unknown 1800mV 0mA 1800mV 1800mV
ff770000.syscon:io-domains-audio 0 0mA 0mV 0mV
vcc_3v0 1 2 0 unknown 3000mV 0mA 3000mV 3000mV
ff770000.syscon:io-domains-gpio1830 0 0mA 0mV 0mV
ff320000.syscon:io-domains-pmu1830 0 0mA 0mV 0mV
vcc3v3_s3 2 1 0 unknown 3300mV 0mA 0mV 0mV
fe300000.ethernet-phy 1 0mA 0mV 0mV
vcc3v3_s0 1 0 0 unknown 3300mV 0mA 0mV 0mV
vdd_cpu_b 2 1 0 normal 825mV 0mA 712mV 1500mV
cpu4-cpu 1 0mA 825mV 1250mV
vdd_gpu 2 1 0 normal 875mV 0mA 712mV 1500mV
ff9a0000.gpu-mali 1 0mA 875mV 1150mV
vcc3v0_sd 2 1 0 unknown 3000mV 0mA 3000mV 3000mV
fe320000.mmc-vmmc 1 0mA 3000mV 3100mV
vcc5v0_sys 3 3 0 unknown 5000mV 0mA 5000mV 5000mV
vbus-5v 0 1 0 unknown 5000mV 0mA 5000mV 5000mV
4-0022-vbus 0 0mA 0mV 0mV
usb3_vbus 2 1 0 unknown 5000mV 0mA 5000mV 5000mV
phy-ff770000.syscon:usb2phy@e450.11-phy 2 0mA 0mV 0mV
usb_vbus 2 1 0 unknown 5000mV 0mA 5000mV 5000mV
phy-ff770000.syscon:usb2phy@e460.7-phy 2 0mA 0mV 0mV
vcc3v3_pcie 1 0 0 unknown 0mV 0mA 0mV 0mV By the way, everything looks great, the tests with Anyway thanks for the great work! 👍 👍 👍 |
The old method carried along with board-pbp-add-dp-alt-mode.patch only makes typec work in one(normal) orientation. This patch introduces a proper extcon driver and makes the workaround cleaner, so orientation switch is working. Improvements: - type-c DP on rk3399 works with both orientations - type-c USB 3.0 on rk3399 works with both orientations, with minor issues, see below Caveats: - Powered USB-C hubs may be not recognized, and can be worked around by loading a gadget driver, or manually toggling the mode once for each connection. - Some dual-role devices(phone, tablet) may be not recognized. Affected boards: - TinkerBoard 2/2S - Pinebook Pro - NanoPC T4 - Orange Pi 4 - Orange Pi 4 LTS Tested on tinkerboard 2s. This patch contains other minor fixes for tinker2's device tree, including adding a missing fan node, adding color labels to leds. The 2 patches adding dp support for nanopc t4 and pinebook pro are also updated accordingly. The device trees of Orange Pi 4 / 4 LTS are also updated to match the new implementation.
coderabbitai
bot
added
05
github-actions
bot
removed
the
Ready to merge
igorpecovnik
added
Ready to merge
|
Merged! |
Description
Fix Type-C DP altmode on some rk3399 boards. Currently only on rockchip64-6.15.
The old implementation is tricky, and only works in normal orientation. If the plug is flipped, there will be only USB2.0 connectivity.
With the proposed changes, the type-c ports on related rk3399 boards should work in both orientations. A few patches from Megous are picked(some of them already present in sunxi64 kernel patches). The tricky part is how to notify rk3399 cdn-dp driver about DP HPD(hot plug detection), and a workaround is written.
A caveat is that the c port has to be OTG mode, otherwise it will behave very similar to the old implementation if DP is enabled.
I'm not sure if orange pi 4 / 4 LTS support this, so there's no change to their dts yet.
Apart from the work above, the missing fan node is added to tinkerboard 2's dts.
list of changes
CONFIG_TYPEC_EXTCON=mHow Has This Been Tested?
Checklist: