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 file was updated to include the Suggested labels
Suggested reviewers
📜 Recent review detailsConfiguration used: CodeRabbit UI ⛔ Files ignored due to path filters (18)
📒 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... |
|
Tested on Orange Pi 4 with the help of an external Type-C Hub. I noticed though that, when the Hub is connected to Type-C and HDMI is in use, USB ports of the Hub don't work. |
Please elaborate. What kind of USB device(pen drive, wireless mouse, usb 2.0 or 3.0) was tested with? Is dmesg showing anything indicating the detection of USB device when it's plugged? Here's an example dmesg log: The glue driver will print current USB extcon status in dmesg with prefix When DP HPD event is synced with extcon, something like this is printed: HDMI works at a frequency that will interfere with 2.4G radios if not shielded properly. If usb works alone and then not when HDMI is plugged, I suspect that HDMI is interfering with your wireless keyboard/mouse. If so, it's the hub's quality issue. Some devices may be more resistant to the interference though. If DP/HDMI works, I think at least USB 3.0 should work, if equipped on the hub. For once, my hub's USB ports stopped working, but I can't reproduce the problem. |
github-actions
bot
removed
the
Ready to merge
hyx0329
force-pushed
the
rk3399-usbc
branch
2 times, most recently
from
7ee69dc to
bd723d8
Compare
|
Thanks to @paolosabatino's report, we know at least OPi4 can support DP output. But for some reason, when the HDMI(DP output) is in use, the hub's usb ports don't work. As far as I know, even if 4-lane DP is used and USB3 fails, USB2(on rk3399's side) shouldn't fail, which is the old behavior. I pushed the changes to OPi4/LTS's dts anyway. Maybe I miss something. Edit: oh I found it, it's PD(Power Delivery). When external PD power supply is connected, the hub's usb ports may not recognized. |
|
@hyx0329 thanks for the answer. For the sake of completeness, I attach here a dmesg log of the session. The various USB attach/detach messages are due to me attaching and detaching the USB device to the board USB ports. The USB ports of the Hub were completely silent. Anyway I will try to find a way to power the board without PD and report if the USB ports work along DP/HDMI when power is not delivered by Type-C. Something I also noticed is that, when detaching the HDMI monitor from the Hub and attaching to the board HDMI, the monitor was still enumerated as a connected display on DP-1. I don't know if it was a problem of the KDE Plasma utility or a driver/dts issue, but I then got two monitors, one attached to |
|
Loading a gadget driver(like g_serial) seems fix the PD hub problem. Out of coincidence, I left the g_serial module loaded after some tests and connected the powered hub to my tinkerboard 2s after that. Then I found dmesg printing a lot about new USB devices. Problem solved :) Loading one gadget driver(g_serial, g_ncm, etc.) before connecting those powered hubs will somehow fix the detection issue. If the gadget driver is loaded with a config in modules-load.d, powered hubs connected before booting will also work. I think it's something like this(on superuser), that the downstream hub is not reset or something. But I couldn't find a way to test. By the way, if |
|
@hyx0329 understand, thanks for delving into this. In the meantime I'm having bad time finding a proper power connector for the Opi4LTS, because it can't be powered via GPIO headers and the power connector is an unusual 1.7mm x 4.75mm or so. I guess that, since if |
I can sent you some of these if you want - have lots of cables and PSUs doing nothing. |
|
@paolosabatino well the situation is, the new implementation/extcon driver requires changes to device trees, otherwise at least the displayport output won't work. I think I'll keep On tinkerboard 2 and nanopc t4, the type-c port doesn't actually support power input and thus hubs are tend to be used without a power supply, so I would like to keep A second option is to load I won't mind if this pr is postponed or closed. I just do my best to fix problems :) |
Thanks, but I am sure I have one of them somewhere I cut from an old DELL PDA adapter. Just can't find it. Anyway, @hyx0329 has found that the issue is software based, probably some reset or some register to initialize to make the USB devices working, hence there isn't the necessity to test the external hub without PD anymore. It would have been a real nice thing to get HDMI/DP + PD + USB via Type-C on rk3399, since the hardware seems to support the whole package, there is just that annoying workaround to load a usb gadget. In my opinion, this PR could be merged when @hyx0329 says it is ready: if I understand this correctly, it should not change the behaviour to any board other than those which gets |
github-actions
bot
removed
the
Ready to merge
Improvements: - type-c DP on rk3399 works with both orientations - type-c USB 3.0 on rk3399 works with both orientations, with minor issues 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. Tested on tinkerboard 2s. This patch contains other minor fixes for tinker2's device tree. The 2 patches adding dp support for nanopc t4 and pinebook pro are also updated. The device trees of Orange Pi 4 / 4 LTS are also updated to match the new implementation.
|
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 :) |
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: