Hi,
On Mon, Jan 5, 2026 at 11:39 AM Robin Murphy <[email protected]> wrote:
>
> >> +&{/} {
> >> + 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
> >> + 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.
> >> +&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...
>
> >> + 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.
> >> +&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.
> >> + 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.
;-)
