Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider: - [High] The DP controller hardware is not initialized before the eDP panel probe is triggered, causing synchronous AUX transfers to fail or hang. - [High] The custom EDID read function violates the I2C EEPROM protocol by omitting the Middle-of-Transaction (MOT) bit, breaking EDID reads on sinks that require a repeated START. --
--- Patch [14]: [PATCH RESEND 14/17] riscv: dts: spacemit: k3: add display nodes --- commit 9b9ee39195ad90a76f056f9bddbdabb8377b67ec Author: Cody Kang <[email protected]> riscv: dts: spacemit: k3: add display nodes Add the SoC-level display nodes: the two Saturn DPUs, the two DP/eDP controllers with their PHY children. > diff --git a/arch/riscv/boot/dts/spacemit/k3-display.dtsi > b/arch/riscv/boot/dts/spacemit/k3-display.dtsi > new file mode 100644 > index 0000000000000..4892ee6acda22 > --- /dev/null > +++ b/arch/riscv/boot/dts/spacemit/k3-display.dtsi > @@ -0,0 +1,156 @@ [ ... ] > +&soc { [ ... ] > + edp0: edp@cac84000 { > + compatible = "spacemit,k3-inno-dp"; [Severity: High] Does enabling this device node cause the eDP panel probe to fail? Looking at inno_dp_probe() in drivers/gpu/drm/spacemit/spacemit_inno_dp.c, it populates the eDP panel device on the AUX bus: ret = devm_of_dp_aux_populate_bus(&dp->aux, spacemit_dp_edp_link_panel); The panel driver probes synchronously and attempts an AUX transfer to read the EDID. However, the DP controller's hardware registers (including the AUX clock divider DP_AUX_CTRL_CLK_DIV and soft resets) are not initialized until later: spacemit_inno_dp.c:spacemit_dp_bind() { ... ret = spacemit_dp_dev_init(dp); ... } The uninitialized controller ignores the transfer or the AUX state machine does not start, causing an unavoidable timeout. > + reg = <0x0 0xcac84000 0x0 0x4000>; [ ... ] > + dp1: dp@cac88000 { > + compatible = "spacemit,k3-inno-dp"; [Severity: High] Will EDID reads fail for sinks connected to this node? When reading EDID, the driver splits the transaction into a write and a read: spacemit_inno_dp.c:spacemit_dp_conn_get_edid_block() { ... ret = spacemit_dp_aux_i2c_write(dp, DDC_ADDR, &edid_offset, 1); ... } It calls spacemit_dp_aux_i2c_write() which hardcodes the request: spacemit_inno_dp.c:spacemit_dp_aux_i2c_write() { ... .request = DP_AUX_I2C_WRITE, ... } The missing MOT (Middle-of-Transaction) bit guarantees an I2C STOP condition after writing the offset address. This resets the internal address pointer of many standard EEPROMs, causing the subsequent read to repeatedly fetch block 0 instead of the requested extension block. > + reg = <0x0 0xcac88000 0x0 0x4000>; -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=14
