Skip to content

drm: rockchip: dw_hdmi_qp: support inverted HDMI HPD - #559

Merged
rpardini merged 1 commit into
armbian:rk-6.1-rkr7.2from
mixtile-rockchip:core3588e-hpd-inverted
Sep 25, 2026
Merged

rpardini merged 1 commit into
armbian:rk-6.1-rkr7.2from
mixtile-rockchip:core3588e-hpd-inverted

Conversation

@evtest-hash

Copy link
Copy Markdown

Depends on #556 for the Core3588E HDMI HPD pinmux correction (hdmim0_tx0_hpd → hdmim1_tx0_hpd).

Problem

On the Mixtile Core3588E used with a Jetson-compatible carrier (tested with a Seeed A206), HDMI HPD is electrically inverted. The Rockchip driver therefore reports connected when no display is attached and disconnected when a display is plugged in.

Hardware

The Core3588E routes SODIMM pin 96 (DP1_HPD) directly to the RK3588 HDMI HPD input. The carrier provides the HPD level shifter.

The NVIDIA Jetson TX2 NX Product Design Guide (DG-10141-001_v1.1) explicitly allows either inverting or non-inverting HPD level shifters and states that the Jetson TX2 NX Developer Kit uses an inverting HPD level shifter.

Fix

Add an optional hpd-inverted boolean to the RK3588 HDMI QP driver.

When enabled, the HPD level read from GRF_SOC_STATUS1 is inverted before it is evaluated. Both RK3588 HPD handling paths apply the same polarity correction.

When hpd-inverted is absent, existing behaviour is unchanged.

Validation

Tested on Mixtile Core3588E + Seeed A206 with a 4K display:

  • unplugged: disconnected
  • plugged in: connected, EDID read successfully
  • unplugged again: disconnected

Real HDMI hotplug operation was verified.

Dependency

The Core3588E also requires the HPD pinmux correction from hdmim0_tx0_hpd to hdmim1_tx0_hpd. That change is handled separately in #556.

The two changes are independent:

Both are required for working HDMI on this board.

Both changes touch the same &hdmi0 node, so whichever merges second may show a small textual conflict there. The resolution is additive: keep the pinctrl configuration from #556 and hpd-inverted from this PR.

Regression

hpd-inverted is only enabled for the Core3588E. When the property is absent, existing RK3588 and other supported SoC paths retain their current behaviour.

@coderabbitai

coderabbitai Bot commented Sep 23, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

Note

Currently processing new changes in this PR. This may take a few minutes, please wait...

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Advanced

Run ID: 1e8e4c6d-222a-4d6a-be41-c51196f64b3e

📥 Commits

Reviewing files that changed from the base of the PR and between 9a62b63 and 0e846fa.

📒 Files selected for processing (1)
  • arch/arm64/boot/dts/rockchip/rk3588-mixtile-core3588e.dts
 ___________________________________________________________________________
< Two hard things: cache invalidation, naming things, and off-by-one jokes. >
 ---------------------------------------------------------------------------
  \
   \   \
        \ /\
        ( )
      .( o ).

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Advanced

Run ID: e2b56cc5-3c66-4d56-bdef-8cbdd43f54a3

📥 Commits

Reviewing files that changed from the base of the PR and between 428ab27 and 9a62b63.

📒 Files selected for processing (2)
  • arch/arm64/boot/dts/rockchip/rk3588-mixtile-core3588e.dts
  • drivers/gpu/drm/rockchip/dw_hdmi_qp-rockchip.c

Included review availability: Your plan provides up to 2 included reviews per hour; 1 remains after this review.


Walkthrough

The Core 3588E device tree sets hpd-inverted for HDMI0. The Rockchip HDMI driver reads this property into hpd_inverted. When the flag is set, the RK3588 interrupt and connector-status paths invert the selected port's LEVEL_INT bit before evaluating HPD status. Without the property, these paths retain their existing behavior.

Priority: ➖ Normal

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix

Merge Risk: ⚪ Minimal · up to 9a62b

The Core3588E configuration applies HPD polarity inversion consistently in the interrupt and connector-status paths. No concrete merge-blocking issue remains.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 3 functions across 1 files. (1 skipped: 1 … Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description check ✅ Passed The description clearly explains the inverted HDMI HPD problem, the driver and device-tree changes, validation results, dependency on the pinmux correction, and regression behavior.
Title check ✅ Passed The title clearly and concisely identifies the main change: adding inverted HDMI HPD support to the Rockchip DW HDMI QP driver.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Full details: Docstring Coverage

Explanation

Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 3 functions across 1 files. (1 skipped: 1 unsupported.)

  • Fix all pre-merge checks with AI
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@evtest-hash

Copy link
Copy Markdown
Author

@rpardini The Actions failure looks like a runner issue rather than a compile error: the arm64 step succeeded, the armhf pass stopped at drivers/base/regmap/regmap.o long before drivers/gpu/drm/, and there is no error: or ##[error] anywhere in the log — it just ends mid-build. Could you please rerun the failed job?

@rpardini

Copy link
Copy Markdown
Member

Hello again. Thanks for this.

When I did this board (which I don't have anymore), I used the "LEETOP carrier board" (as shown by https://www.cnx-software.com/2023/12/14/mixtile-core-3588e-development-kit-review-unboxing-first-boot/ ) -- from what I can remember HDMI worked fine. (It surely did on mainline kernel, which was done based on this).

Maybe we should have a derivative DT (#include the original, override just the needed property) for a different carrier?

it just ends mid-build. Could you please rerun the failed job?

Yeah those CIs fail a lot. I guess some infra problem. I've Re-run.

@evtest-hash

Copy link
Copy Markdown
Author

@rpardini Thanks — and thanks for re-running the job.

I think the carrier you used and the one I tested on are the same board. The review
you linked identifies it:

Markings show it's one of those LEETOP carrier boards we noted in some Jetson
Xavier NX mini PCs. The exact model used in this kit is the Leetop A206 carrier
board.

That A206 is what's on my desk, so we're looking at the same SoM on the same carrier.

Here's what I measure on it — SYS_GRF SOC_STATUS1 bit 16, the bit
dw_hdmi_rk3588_read_hpd() evaluates, against the cable:

state bit 16 connector
unplugged 1 disconnected, 0 modes
plugged in 0 connected, 47 modes

The bit tracks the cable inversely, so HPD is inverted on this carrier. With the
patch the connector state is correct both ways, EDID reads back over DDC, and
unplugging brings the VOP down on its own (vop disable intf:800,
Crtc atomic disable vp0) — so the threaded HPD path reacts too, not just
.detect.

On the derivative DT — I see the reasoning, but I don't think "#include the
original, override just this property" fits here, because the DT isn't
carrier-neutral apart from HPD. It already describes the A206 in a fair few places:
vcc12v_dcin for the DC jack, &sdmmc with card-detect, &pcie2x1l0 / &pcie3x4
for the M.2 slots, &edp1 for the DP connector, among others. Another carrier would
differ in most of those as well, so it'd want its own board DT rather than a
one-property override — and a derivative that overrode only HPD would imply the rest
is portable when it isn't. For now this module ships and is documented against the
A206 only.

Happy to split it properly if we pick up a second carrier — a SoM .dtsi plus one
.dts per carrier is the right shape for that, and I'll do it then.

I have the hardware here, so if there's anything you'd like checked, just say.

@rpardini rpardini left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, I see --

  1. indeed the DT represents the module + LEETOP carrier - I thought you had a different carrier here
  2. You changed the pinctrl on #556 so I guess it makes sense on the whole

@rpardini

Copy link
Copy Markdown
Member

I have the hardware here, so if there's anything you'd like checked, just say.

Well, it's a bit out there but if you could also check the mainline DTs (back in armbian/build) it would be great. I don't have the Core3588E anymore, but a friend over in the US does. I have Blade3 and the Edge2 here to test. Both would benefit immensely from your knowledge -- both me and Joshua Riek have fiddled with them over the years, getting great mainline support would be awesome. For the Blade3, also the mainline u-boot needs love, I never got it stable due to the USB-C/TPCM FUSB302 powering -- it bootloops 3 out of every 5 times. I currently "shove" 12V down the USB-C port for stable operation.

@evtest-hash

Copy link
Copy Markdown
Author

@rpardini On the mainline front — we're already on it, in parallel with the armbian/linux-rockchip DT work.

We've been bringing these boards up on mainline in a Yocto layer (linux-yocto 6.18, U-Boot 2026.07), written from the schematics and validated on hardware per peripheral, with the full status documented in the layer README:

Build images for all three are tagged as releases, if you'd like to test them directly: Edge 2, Blade 3, Core 3588E.

To be clear, this is just to show progress — it isn't going into this PR. The mainline changes are still being finalized on our end, and we'll submit them separately once they're ready.

On the Blade3 power issue: we currently negotiate the PD contract in the kernel (the Blade3 DT already describes the two FUSB302 controllers, with usbc1/PORT1 as the board's 5–20 V power input). We'll look at doing the same in U-Boot so the board doesn't stay at the 5 V default and brown out during boot — fixing that boot-time power issue is a priority for us.

@rpardini

Copy link
Copy Markdown
Member

Ooops, @evtest-hash -- apparently conflict surfaced after merging the other changes, could you take a look?

@evtest-hash

evtest-hash commented Sep 25, 2026 •

Copy link
Copy Markdown
Author

I checked the conflict. It is only a textual conflict in the Core3588E &hdmi0 node caused by #556 adding the correct HPD pinmux.

The changes are additive, so both should be kept. The resolved node should be:

&hdmi0 {
    pinctrl-names = "default";
    pinctrl-0 = <&hdmim0_tx0_cec
             &hdmim1_tx0_hpd
             &hdmim0_tx0_scl
             &hdmim0_tx0_sda>;
    hpd-inverted;
    status = "okay";
};

hdmim1_tx0_hpd comes from #556, while hpd-inverted is from this PR. There is no conflict in the driver changes.

@rpardini

Copy link
Copy Markdown
Member

Yep, thanks, still, this needs a rebase so we can merge.

…588E

On the Mixtile Core3588E used with a Jetson-compatible carrier board
(tested on a Seeed A206), HDMI hot plug detect is read with the wrong
polarity: the connector reports "connected" with nothing attached, and
"disconnected" once a display is plugged in.

The Core3588E passes SODIMM pin 96 straight through to the SoC with no
components in between, so the polarity is decided by the carrier, and on
Jetson-compatible carriers the HPD level shifter is inverting. This is
not a carrier defect. The NVIDIA Jetson TX2 NX Product Design Guide
(DG-10141-001_v1.1), figure 7-7 note 1, says:

  "HPD level shifter can be non-inverting or inverting. HPD level
   shifter on the Jetson TX2 NX Developer Kit is inverting."

The NVIDIA reference carrier implements HDMI HPD with a single NPN
common-emitter stage, which inverts; for DP/eDP, where the same guide
requires a non-inverting shifter, it uses two stages. This board shows
both sides of that split: &edp1 uses hpd-gpios with GPIO_ACTIVE_HIGH on
SODIMM pin 90, the non-inverting DP path, and needs no correction.

Add an optional "hpd-inverted" boolean. When present, the HPD level bit
is flipped straight after it is read from GRF_SOC_STATUS1, leaving all
downstream logic untouched. Both RK3588 HPD consumers need it: the
threaded HPD interrupt does not call read_hpd(), it evaluates the level
itself to set hpd_stat and to pick a direction-dependent debounce
(150 ms on connect, 20 ms on disconnect).

Measured on a Core3588E + A206. GRF_SOC_STATUS1 bit 16 tracks the cable
inversely, and the connector state is correct in both directions:

  unplugged    bit 16 = 1    disconnected     0 modes
  plugged in   bit 16 = 0    connected       47 modes, 3840x2160

Unplugging brings the VOP down on its own ("vop disable intf:800",
"Crtc atomic disable vp0") about two minutes before anything read sysfs,
so the interrupt path is exercised as well as the detect path.

Note this alone does not make HDMI work on the Core3588E: the board also
needs its HPD pin mux corrected from hdmim0_tx0_hpd (GPIO1_A5, the
rk3588s.dtsi default) to hdmim1_tx0_hpd (GPIO3_D4, where pin 96 lands),
which is handled separately.

Boards that do not set the property XOR the register value with 0, so
their behaviour is bit-identical to before. rk3588_hdmi_thread() and
dw_hdmi_rk3588_read_hpd() are RK3588-only; RK3538, RK3572 and RK3576 use
their own callbacks and are untouched.
@evtest-hash
evtest-hash force-pushed the core3588e-hpd-inverted branch from 9a62b63 to 0e846fa Compare September 25, 2026 13:19
@coderabbitai
coderabbitai Bot requested a review from rpardini September 25, 2026 13:21
@rpardini
rpardini merged commit f875369 into armbian:rk-6.1-rkr7.2 Sep 25, 2026
1 of 2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants