Thread (14 messages) 14 messages, 5 authors, 2026-01-14

Re: [PATCH 4/4] arm64: dts: rockchip: Add overlay for FriendlyElec HD702E

From: Doug Anderson <dianders@chromium.org>
Date: 2026-01-05 20:28:42
Also in: dri-devel, linux-rockchip

Hi,

On Mon, Jan 5, 2026 at 11:39 AM Robin Murphy [off-list ref] wrote:
quoted
quoted
+&{/} {
+       pwm_bl: backlight {
nit: up to you, but I happened to notice that other rk3399 boards just
use the label "backlight:" instead of "pwm_bl:"
Sure, if you prefer.
Totally up to you--I was just comparing what you had to what other
rk3399 boards had and the fact that this was different made it take 15
seconds longer to find the PWM backlight on the bother boards. :-P

quoted
quoted
+               compatible = "pwm-backlight";
+               pwms = <&pwm0 0 25000 0>;
40kHz is awfully fast for a PWM backlight. Are you sure you need it that fast?
I guess I just copied this from somewhere without too much thought, but
double-checking the schematic[1] now, the backlight driver where this
signal ends up is apparently a SY7200A, whose datasheet says "And the
recommend dimming frequency range is 20kHz~1MHz." So relative to that
range it doesn't seem too bad!
OK, cool! I just know in the past that I've seen cases where people
copied the PWM values from elsewhere and ended up wildly out of spec
without really noticing. :-) Sounds like you're good.

quoted
quoted
+&edp {
+       force-hpd;
+       status = "okay";
+
+       aux-bus {
+               edp-panel {
+                       compatible = "friendlyarm,hd702e";
As per my response in your driver patch, any chance this can just be
"edp-panel"?
Per the cover letter, I did try...
quoted
quoted
+                       backlight = <&pwm_bl>;
+                       no-hpd;
+                       power-supply = <&vcc12v0_sys>;
While not strictly required, it seems highly likely that you want
"hpd-absent-delay-ms". It's highly unlikely that you would have
"no-hpd" plus a "power-supply" but no hardcoded delay to wait here. I
haven't seen panels that turn on and respond instantly.
...but even with both delays bumped up and up and up to a ridiculous
2000ms it still didn't work. It reads the EDID as all-zeros then fails
to probe due to a lack of modes. Whereas with the hard-coded mode, the
display lights up immediately upon probe, so I don't think it's a timing
issue. My working theory is that there's some fundamental ordering issue
based on the comments in analogix_dp_detect_hpd() about aux not working
until HPD is forced at the controller end, and from a brief skim of the
history, quite possibly related to f2600d08d4e8 ("drm/bridge:
analogix_dp: Improve panel on time") which maybe prevents that happening?

By that point it's well beyond my expertise, hence my conclusion that
since I'm not *adding* the legacy panel data, just rearranging what's
already upstream to finally put it to proper use, that's just about
acceptable :)
OK. It's unfortunate, but fair enough. :-) It would certainly be
interesting to know what eventually makes the EDID read correctly, but
I agree that it's a bit much to ask you to debug that.

FWIW, I took a _quick_ gander at the schematics you sent, and I
actually wonder if it's the "backlight" enable signal that you need.
Depending on the stuffing options, maybe that connects to the PWRDN
pin? If I trust the stuffing options in the schematic that GPIO
doesn't connect to anything at all, but I know stuffing options on
schematics aren't always right... In any case, it's also OK to just do
what you're doing...

The power-supply entry is really just for cleanliness, to avoid a "dummy
regulator" message - the screen module has all its own regulation on
board, which didn't seem worth modelling in detail as it's all fixed and
always-on, but the source is the board's main 12V input, so that much
*is* true.
Fair enough.

quoted
quoted
+&i2c4 {
+       #address-cells = <1>;
+       #size-cells = <0>;
The base dts already specifies #address-cells and #size-cells, right?
Indeed, but dtc doesn't know that when compiling the .dtbo in isolation.
Oh, right.

quoted
quoted
+       touchscreen@5d {
+               compatible = "goodix,gt9271";
+               reg = <0x5d>;
+               interrupt-parent = <&gpio1>;
+               interrupts = <RK_PC4 IRQ_TYPE_EDGE_FALLING>;
+               irq-gpios = <&gpio1 RK_PC4 GPIO_ACTIVE_HIGH>;
+               reset-gpios = <&gpio1 RK_PB5 GPIO_ACTIVE_HIGH>;
There's no power supply here, so I'll assume it follows the power
supply of the panel? You probably want to be a "panel-follower" then,
right? AKA you'd want to add a "panel = " node here to refer to your
edp-panel.

Oh, except that the goodix driver you're using doesn't support
panel-follower. That's annoying. I guess you'd have to add support for
that (see history around "is_panel_follower" in "i2c-hid-core.c")?
Without it then I assume you'll just be lucky that things work? ...or
you'd need to mark the power supply as always-on?
Yeah, as above it is in fact always-on anyway - somehow I missed that
the goodix driver is getting regulators too. The 12V input is stepped
down to a main VDD_3.3V rail from which everything else is derived, so
if I add a fixed-regulator node for that which is sufficiently close to
the truth for both these and the panel supply, would that be clear enough?
Sure, it's fine from my point of view. Really pretty much all of the
dts is up to Heiko. I'm mostly here because of the eDP-panel stuff.
;-)
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help