Hi Sebastian,
I run an RK3588 board with native DP out of usbdp_phy0 and have been
carrying parts of this series locally. Patches 3, 6 and 8 are on it and
they fixed real problems for me, so thanks for those.
The board is a custom RK3588 design, not upstream - DP0 is wired DP-only,
four lanes, no USB-C alt mode, so it is the native DP configuration your
changelog note refers to.
Two things that may be useful to you, one of them a correction to an
assumption in the v10 changelog.
1. System sleep is not broken on this platform
The changelog gives "system sleep is broken on recent Rockchip platforms"
as part of the reason for leaving the system sleep callbacks out. That is
not what I see here: S3 suspend and resume are reliable on this board.
What was broken was DP specifically, and only because the controller was
never reconfigured on the way back.
With no system sleep handling, after an S3 with a DisplayPort monitor
attached and working beforehand:
- DW_DP_HPD_STATUS returns a stale value that reads as asserted, and
does not change when the sink's mains lead is pulled and replaced.
The register is not backed by a configured controller.
- Every AUX transaction therefore clears the HPD guard and then times
out.
- The threaded HPD interrupt is never re-armed, so no hot-plug event is
delivered at all. Replugging the monitor produces silence.
- The connector settles at disconnected - the outcome of the failed
probe rather than a statement about the cable - and the display stays
dark until the machine is rebooted.
Unbinding and rebinding the DP device on the live machine recovers it
completely, with nothing touched at the monitor end.
2. Your runtime PM work already contains the fix
I had been carrying a local patch against the pre-series driver that
masks the interrupt on suspend and re-runs dw_dp_init_hw() plus
enable_irq() on resume. It has held for twelve deep-suspend cycles, with
the display coming back on its own every time.
Reading patch 14, that is what dw_dp_runtime_resume() already does -
clocks, hpd_sw_sel/hpd_sw_cfg, dw_dp_init_hw(), enable_irq(), and the
110 ms settle. And dw_dp_runtime_suspend() is the disable_irq() half.
So on top of your series the whole thing may be as small as adding the
system sleep pair to the ops you already have:
static const struct dev_pm_ops dw_dp_pm_ops = {
RUNTIME_PM_OPS(dw_dp_rockchip_runtime_suspend,
dw_dp_rockchip_runtime_resume, NULL)
SYSTEM_SLEEP_PM_OPS(pm_runtime_force_suspend,
pm_runtime_force_resume)
};
To be explicit about what I have and have not tested: those twelve cycles
were with my own hand-rolled callbacks against the pre-series driver, not
with the above on top of your series. I have not tested that, and my
patch is not worth sending - it collides with patch 15 and targets a
probe path you have since restructured.
What I can offer is hardware. This board reproduces the failure reliably
and S3 is dependable on it, so if you write the system sleep ops I am
happy to test them and report back.
One other offer, unrelated: this board also appears to be a reliable
source of AUX transaction timeouts, which is the path patch 6 addresses.
I have not captured a clean trace yet. If one would be useful, tell me
what you would want in it and I will produce it.
Karl Asseily
