From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from smtp.forwardemail.net (smtp.forwardemail.net [149.28.215.223]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 9F44A4EE873 for ; Fri, 4 Sep 2026 15:13:53 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=149.28.215.223 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1788534836; cv=none; b=EyJ4cMzv6VR7e96x2x9h8Oh1bk8f+tJ/JvQ+DQ0R8YiX9+XwEaGA3C5U+muSM4iok52FrOoiObv11VI/NZjXaXmjNfb3mlXG38zg/WC7bZviqglBqdgzYjIPTOT4pFW0nMdRodFnJ+dVDozCqf5t1FHCrxoa2/E46OxwQoIr1kw= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1788534836; c=relaxed/simple; bh=seRwgqmK7wkA4WaGmSlHoknG8zIwR/4ksaxu0H/F9qM=; h=Mime-Version:Content-Type:Date:Message-Id:Cc:Subject:From:To: References:In-Reply-To; b=HtZKec3t2Sw3Bzla7dd2qAQDVMn5RROTtlm0F7ibGTo78TBgfD3ksriO8G2rYOGERdj2w1AzxK5EwCNelYmTVB0c9Yu1SxbMsnAs+BheR1BRuJ6wN+qUCGid2nR3hL6516hRhy73rQgdq7Zprq/xIaHDiDoWsYYbE4tidT05JvE= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=ubuntu.com; spf=pass smtp.mailfrom=fe-bounces.ubuntu.com; dkim=pass (2048-bit key) header.d=ubuntu.com header.i=@ubuntu.com header.b=PUgTLeXT; arc=none smtp.client-ip=149.28.215.223 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=ubuntu.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=fe-bounces.ubuntu.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=ubuntu.com header.i=@ubuntu.com header.b="PUgTLeXT" DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=ubuntu.com; h=In-Reply-To: References: To: From: Subject: Cc: Message-Id: Date: Content-Type: Content-Transfer-Encoding: Mime-Version; q=dns/txt; s=fe-953a8a3ca9; t=1788534828; bh=J66fN78gjzDvd/CtvKdQYi/6n2qcde6bjwwV8TUBACo=; b=PUgTLeXT3u9g7rRLDLHZzgKdte+MpyB25XHNz/bZWTs6dEO/iOWhv5h2g4JlfuURkdEaC7iGv FAVolsEk8W6yKPpcA3W2MZyV8j8nyGx752pDhIg5/EZd0gQmbhiOgJy1vjpziFoullbNsxYyxT/ E75UPf8tZ01DAynL2IsSnq/dIwV9UDZPfP+g6t+r7PZA3Gqz7pYMh46RJwEG2cCC2DzVM1xH8UB YNdUg4QHRLnGk68H0AUbUuoUcMHkGEFpxkTJHsVUQJkAVhsvef6h7zVdGu/Lygo8/gn8XD0m7Di UzhukpuHXRzXzJyM3xj044PXuIbwHAolxDFSdbz3rUNQ== X-Forward-Email-ID: 6a9ae0044b8c51dfb697c95f X-Forward-Email-Sender: rfc822; jpeisach@ubuntu.com, smtp.forwardemail.net, 149.28.215.223 X-Forward-Email-Version: 2.14.0 X-Forward-Email-Website: https://forwardemail.net X-Complaints-To: abuse@forwardemail.net X-Report-Abuse: abuse@forwardemail.net X-Report-Abuse-To: abuse@forwardemail.net Precedence: bulk X-Mailing-List: linux-clk@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: Mime-Version: 1.0 Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset=UTF-8; format=Flowed Date: Fri, 04 Sep 2026 11:13:05 -0400 Message-Id: Cc: , , , , , , , , , "Andy Yan" , "Marek Szyprowski" , "Maud Spierings" , "Graham Markall" , "Icenowy Zheng" Subject: Re: [PATCH v3 00/19] drm: starfive: jh7110: Enable display subsystem From: "Joshua Peisach" To: "Michal Wilczynski" , "Vinod Koul" , "Neil Armstrong" , "Rob Herring" , "Krzysztof Kozlowski" , "Conor Dooley" , "Andrzej Hajda" , "Robert Foss" , "Laurent Pinchart" , "Jonas Karlman" , "Jernej Skrabec" , "Luca Ceresoli" , "David Airlie" , "Simona Vetter" , "Maarten Lankhorst" , "Maxime Ripard" , "Thomas Zimmermann" , "Lee Jones" , "Andy Yan" , "Philipp Zabel" , "Emil Renner Berthing" , "Hal Feng" , "Michael Turquette" , "Stephen Boyd" , "Brian Masney" , "Heiko Stuebner" , "Conor Dooley" , "Paul Walmsley" , "Palmer Dabbelt" , "Albert Ou" , "Alexandre Ghiti" , "Dominique Belhachemi" , "Brian Masney" , "Jerome Brunet" X-Mailer: aerc 0.21.0 References: <20260904-jh7110-clean-send-v3-0-484f9ae72715@samsung.com> In-Reply-To: <20260904-jh7110-clean-send-v3-0-484f9ae72715@samsung.com> On Fri Sep 4, 2026 at 9:27 AM EDT, Michal Wilczynski wrote: > This series enables the display subsystem on the StarFive JH7110. > > The dom_vout block holds the display controller (dc8200), the clock > generator (voutcrg) and the HDMI IP, all inside PD_VOUT. The HDMI IP is > a single register block containing both the controller and the PHY, and > it has a circular clock dependency with voutcrg: > > - the HDMI controller needs pclk/mclk/bclk from voutcrg > - voutcrg needs the pixel clock for its dc8200 pixel MUXes, and that > clock is generated by the HDMI PHY > > The loop only exists if the HDMI block is treated as one device. The > PHY's reference clock is xin24m, not a voutcrg output, so splitting the > node into a parent plus phy and controller children gives deferred probe > a linear order: hdmi-phy, then voutcrg, then hdmi-controller. > > The parent maps the register block and owns the regmap its two children > share. Everything in the region sits behind one NoC port whose clock and > reset gate access to it, inside PD_VOUT, so the vout subsystem node from > the RFC is back and owns those for as long as any child exists. > > Patch 10 adds a .mode_valid platform op to inno-hdmi. > inno_hdmi_bridge_mode_valid() checks the pixel clock against > hdmi->refclk, but that clock only exists where a "ref" clock is > described. The JH7110 gets its pixel clock from the PHY, so refclk is > NULL and the check was skipped: unsupported modes were advertised, the > modeset then "succeeded" because the atomic enable path cannot fail, and > the display stayed blank. > > Patches 15-17 drop the PHY duplication from the RFC. The JH7110 has the > same Innosilicon PHY as the RK3328, offset by 0x100 because it sits > behind the controller in the shared register block. Patch 15 factors out > the pre-PLL config format, table lookup, determine_rate, recalc_rate and > the pre-PLL programming; patch 16 moves Rockchip onto it; patch 17 adds > the JH7110 driver. Pixel clock tables, post-PLL and analog config stay > SoC specific. > > Patch 16 should be a no-op for Rockchip - same writes, same order, same > values - and RK3228, whose pre-PLL is at different addresses, keeps its > own register code and shares only the lookup. I have no Rockchip > hardware, so it is build tested only (arm and riscv). A Tested-by would > help. > > The dc8200 driver, th1520 reset controller and inno-hdmi bridge that the > RFC listed as prerequisites are all upstream now, so there are no > out-of-tree dependencies. > > Testing > =3D=3D=3D=3D=3D=3D=3D > > Tested on a VisionFive 2 v1.3B using modetest. > > All 42 modes the sink advertises work, with nothing in dmesg. Pixel > clocks run from 25.175 MHz (640x480@59.94) up to 297 MHz > (4096x2160@30), including 3840x2160 and the full 1920x1080 and 1280x720 > rate families. > > The four modes the RFC reported as broken work now too: 2560x1440@59.95, > 2048x1080@60.00, 2048x1080@24.00 and 720x400@70.08. > > Before patch 10, four of the advertised modes failed: 1680x1050@59.95 > (146.250 MHz), 1400x1050@59.98 (121.750), 1152x864@59.97 (81.768) and > 1280x768@60.35 (80.140). Those pixel clocks are not in the PHY pre-PLL > table, so clk_set_rate() returned -EINVAL and the screen stayed black > while userspace saw a successful modeset. They are rejected in > .mode_valid now; the other refresh rates of those resolutions still work. > > Every commit builds for riscv, and the Rockchip PHY also for arm. > > Notes > =3D=3D=3D=3D=3D > > The JH7110 has no central MAINTAINERS entry and maintainership is > fragmented, so patch 19 adds one for the display subsystem and I am > happy to help maintain it. The new PHY library lives under drivers/phy/, > already covered by the generic PHY framework entry. > > checkpatch warns "does MAINTAINERS need updating?" on the patches adding > files, because that entry comes in patch 19. > > Thanks to Icenowy Zheng for the dc8200 driver and for explaining how the > SoC and the display pipeline fit together. > > Thanks also to Dominique Belhachemi, who got rid of the vout-subsystem > wrapper and helped with the testing, to Maud Spierings for testing on a > Framework 13 panel, and to Graham Markall for testing > the JH7110 display patches independently and writing up the results: > https://big-grey.co.uk/2026/01/26/testing-starfive-jh7110-display-control= ler-patches/ > > Link to v1: https://lore.kernel.org/all/20251108-jh7110-clean-send-v1-0-0= 6bf43bb76b1@samsung.com/ > > --- > Changes in v3: > - Brought back the vout subsystem node and driver, now owning the NoC > bus clock, its reset and PD_VOUT for the whole region, with dc8200, > the HDMI block, the syscon and voutcrg as its children (Icenowy Zheng). > - Fixed a hard hang when the bridge is built as a module: the PHY's > .is_prepared read a register in the window gated by the controller's > system clock, so clk_disable_unused() wedged the CPU before the > controller had bound. The op is gone; the framework uses the software > prepare count instead. (Marek Szyprowski) > - The HDMI controller now programs the display mux in dom_vout_syscon > from the port graph rather than inheriting whatever the bootloader > left, with a phandle to the syscon (Icenowy Zheng). > - The register access clock is named "pclk" to match the existing > inno-hdmi binding, so the generic driver no longer picks up the pixel > clock. Previously it held the pre-PLL powered from probe and sized the > DDC divider from the wrong rate. > - Dropped the clk suffixes and the single-entry -names properties from > the bindings (Conor Dooley). mclk and bclk keep their names: per TRM > 5.3 they are the HDMI audio clocks, not module and bus clocks, so the > descriptions say that instead. > - Replaced patternProperties with plain properties in the hdmi-subsystem > binding (Conor Dooley). > - dc8200 gets an SoC specific compatible, and inherits dma-noncoherent > from the subsystem bus node, so it validates against verisilicon,dc. > - Added the pre-PLL entry for the Framework 13 panel and fixed two > devicetree whitespace nits (Maud Spierings). > - select REGMAP_MMIO, CLK_SET_RATE_NO_REPARENT on the dc8200 pixel MUXes > so clk_set_rate() cannot reroute them, and inno-hdmi register reads > return 0 instead of stack garbage when regmap_read() fails. > - phy: rockchip: dropped the local pre-PLL lookup wrapper and the 28 now > unused RK3328 pre-PLL macros, and restored the VCO debug output, this > time in the shared helper so both drivers get it (Jonas Karlman). > - Rebased onto v7.3-rc1. > - Link to v2: https://lore.kernel.org/r/20260828-jh7110-clean-send-v2-0-3= 31680c8b9d1@samsung.com > > Changes since the RFC: > - Dropped the vout-subsystem wrapper driver and its binding, along with > the patch relaxing the voutcrg binding; genpd handles PD_VOUT per > node. > - Renamed the compatible to starfive,jh7110-hdmi-subsystem, dropping > "mfd" as a Linux term (Conor Dooley). > - Absolute $refs in the bindings, unused example labels dropped, and the > examples deduplicated between parent and children (Conor Dooley). > - Added the .mode_valid platform operation (patch 7). > - Split the inno-hdmi rework into a mechanical probe/bind split (patch > 4) > and the regmap-from-parent change (patch 5). struct inno_hdmi is no > longer exported; no platform glue dereferences it. > - Replaced the duplicated PHY driver with a shared Innosilicon library > and moved Rockchip onto it (patches 11-13). > - Fixed pre-PLL lock detection, which masked the status read with the > register address instead of the lock bit. > - Fixed a pixel clock refcount underflow: enable returns early on > failure while disable tore down unconditionally. > - voutcrg patch reduced to adding CLK_SET_RATE_PARENT to the two dc8200 > pixel MUXes. > - Rebased onto v7.2. > > --- > Michal Wilczynski (19): > dt-bindings: phy: Add starfive,jh7110-inno-hdmi-phy > dt-bindings: display: bridge: Add starfive,jh7110-inno-hdmi-control= ler > dt-bindings: mfd: Add starfive,jh7110-hdmi-subsystem > dt-bindings: soc: starfive: Add starfive,jh7110-vout-syscon > dt-bindings: soc: starfive: Add starfive,jh7110-vout-subsystem > dt-bindings: display: verisilicon: Add starfive,jh7110-dc8200 > drm/bridge: inno-hdmi: Split probe out of bind > drm/bridge: inno-hdmi: Allow the register map to come from a parent > drm/bridge: inno-hdmi: Add .disable platform operation > drm/bridge: inno-hdmi: Add .mode_valid platform operation > soc: starfive: Add jh7110-hdmi-subsystem driver > soc: starfive: Add jh7110-vout-subsystem driver > clk: starfive: jh7110-vout: Allow pixel clock rate propagation > drm/bridge: starfive: Add JH7110 HDMI controller driver > phy: Add common Innosilicon HDMI PHY helpers > phy: rockchip: inno-hdmi: Use the common Innosilicon PHY helpers > phy: starfive: Add jh7110-inno-hdmi-phy driver > riscv: dts: starfive: jh7110: Update DT for display subsystem > MAINTAINERS: Add StarFive JH7110 display subsystem entry > > .../starfive,jh7110-inno-hdmi-controller.yaml | 121 +++++ > .../bindings/display/verisilicon,dc.yaml | 1 + > .../mfd/starfive,jh7110-hdmi-subsystem.yaml | 95 ++++ > .../phy/starfive,jh7110-inno-hdmi-phy.yaml | 49 ++ > .../soc/starfive/starfive,jh7110-syscon.yaml | 6 + > .../starfive/starfive,jh7110-vout-subsystem.yaml | 100 ++++ > MAINTAINERS | 13 + > arch/riscv/boot/dts/starfive/jh7110-common.dtsi | 121 ++++- > arch/riscv/boot/dts/starfive/jh7110.dtsi | 102 +++- > drivers/clk/starfive/clk-starfive-jh7110-vout.c | 6 +- > drivers/gpu/drm/bridge/Kconfig | 11 + > drivers/gpu/drm/bridge/Makefile | 1 + > drivers/gpu/drm/bridge/inno-hdmi.c | 84 ++- > drivers/gpu/drm/bridge/jh7110-inno-hdmi.c | 318 +++++++++++ > drivers/phy/Kconfig | 8 + > drivers/phy/Makefile | 1 + > drivers/phy/phy-inno-hdmi.c | 298 +++++++++++ > drivers/phy/rockchip/Kconfig | 1 + > drivers/phy/rockchip/phy-rockchip-inno-hdmi.c | 165 +----- > drivers/phy/starfive/Kconfig | 20 + > drivers/phy/starfive/Makefile | 1 + > drivers/phy/starfive/phy-jh7110-inno-hdmi.c | 579 +++++++++++++++= ++++++ > drivers/soc/Kconfig | 1 + > drivers/soc/Makefile | 1 + > drivers/soc/starfive/Kconfig | 43 ++ > drivers/soc/starfive/Makefile | 3 + > drivers/soc/starfive/jh7110-hdmi-subsystem.c | 74 +++ > drivers/soc/starfive/jh7110-vout-subsystem.c | 83 +++ > include/drm/bridge/inno_hdmi.h | 10 +- > include/linux/phy/inno-hdmi-phy.h | 85 +++ > 30 files changed, 2227 insertions(+), 174 deletions(-) > --- > base-commit: cee9395acd8043be0644b25c34bfa86623f2b935 > change-id: 20251031-jh7110-clean-send-7d2242118026 > > Best regards, So as a kernel newbie, and someone who happens to have this device, it's nice to see it here. It looks good to me, just a few questions: - One patch mentioned in a comment "the docs" - is there documentation for the device? - There are multiple pieces, like the hdmi and vout subsystem, and also the inno helpers. Should those be separate patches? I honestly don't have enough experience (or authority) to suggest doing so. I thought this would be great as a possible driver I could do to learn kernel dev.. looks like I was very wrong. Great work, and hopefully in the future I get my hands on some device that needs a driver to be written for it. I'm at university, once my board gets mailed from home I'll be able to test. For now, Reviewed-by: Joshua Peisach