Re: [PATCH v4 19/20] riscv: dts: starfive: jh7110: Update DT for display subsystem
From: sashiko-bot@kernel.org
Date: 2026-09-15 15:49:27
Also in:
dri-devel, linux-clk, linux-devicetree
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider: - [High] dts: CMA allocation failure on 1GB boards - [Medium] dts: missing 'status = "disabled"' in SoC DTSI for display IPs - [Low] dts: redundant reg property on display-subsystem bus node -- commit d9fbb93d7964da0ec9e48356030bc459050f3218 Author: Michal Wilczynski [off-list ref] riscv: dts: starfive: jh7110: Update DT for display subsystem This commit activates the display subsystem on the JH7110 SoC by describing the video output subsystem, HDMI block, and voutcrg in the device tree. It restructures the HDMI node into a container with phy and controller children to break a circular clock dependency. Additionally, the CMA pool is expanded and constrained to memory addressable by the display controller.
quoted hunk ↗ jump to hunk
diff --git a/arch/riscv/boot/dts/starfive/jh7110-common.dtsi b/arch/riscv/boot/dts/starfive/jh7110-common.dtsi index a7a1c09a2c907..a4c3daf712a63 100644 --- a/arch/riscv/boot/dts/starfive/jh7110-common.dtsi +++ b/arch/riscv/boot/dts/starfive/jh7110-common.dtsi@@ -33,6 +33,27 @@ memory@40000000 { bootph-pre-ram; }; + reserved-memory { + #address-cells = <2>; + #size-cells = <2>; + ranges; + + /* + * The display controller allocates its framebuffers from CMA, + * and the default pool is too small for them. Keep the pool + * inside the range the controller can address; it fits in the + * memory every VisionFive 2 variant has. + */ + linux,cma { + compatible = "shared-dma-pool"; + reusable; + size = <0x0 0x20000000>; + alignment = <0x0 0x1000>; + alloc-ranges = <0x0 0x70000000 0x0 0x20000000>;
[Severity: High] Will this unconditionally fail to allocate the CMA pool on 1GB boards? On boards like the 1GB Milk-V Mars, physical RAM starts at 0x40000000 and ends at 0x80000000. With alloc-ranges starting at 0x70000000, there is only 256MB of physical memory available within the requested bounds, making a 512MB allocation mathematically impossible.
+ linux,cma-default; + }; + }; +
[ ... ]
quoted hunk ↗ jump to hunk
diff --git a/arch/riscv/boot/dts/starfive/jh7110.dtsi b/arch/riscv/boot/dts/starfive/jh7110.dtsi index 9c3e4598747eb..bc2470963520e 100644 --- a/arch/riscv/boot/dts/starfive/jh7110.dtsi +++ b/arch/riscv/boot/dts/starfive/jh7110.dtsi
[ ... ]
quoted hunk ↗ jump to hunk
@@ -1199,22 +1200,84 @@ csi_phy: phy@19820000 { #phy-cells = <0>; }; - voutcrg: clock-controller@295c0000 { - compatible = "starfive,jh7110-voutcrg"; - reg = <0x0 0x295c0000 0x0 0x10000>; - clocks = <&syscrg JH7110_SYSCLK_VOUT_SRC>, - <&syscrg JH7110_SYSCLK_VOUT_TOP_AHB>, - <&syscrg JH7110_SYSCLK_VOUT_TOP_AXI>, - <&syscrg JH7110_SYSCLK_VOUT_TOP_HDMITX0_MCLK>, - <&syscrg JH7110_SYSCLK_I2STX0_BCLK>, - <&hdmitx0_pixelclk>; - clock-names = "vout_src", "vout_top_ahb", - "vout_top_axi", "vout_top_hdmitx0_mclk", - "i2stx0_bclk", "hdmitx0_pixelclk"; - resets = <&syscrg JH7110_SYSRST_VOUT_TOP_SRC>; - #clock-cells = <1>; - #reset-cells = <1>; + vout_subsystem: display-subsystem@29400000 { + compatible = "starfive,jh7110-vout-subsystem"; + reg = <0x0 0x29400000 0x0 0x200000>; + ranges;
[Severity: Low] Is this reg property redundant on the bus node? It assigns a memory region to a bus node that does not own configuration registers, simply to represent the aggregated bounds of its children. The dc8200 child claims the exact same starting physical registers.
+ #address-cells = <2>;
+ #size-cells = <2>;
+ clocks = <&syscrg JH7110_SYSCLK_NOC_BUS_DISP_AXI>;
+ dma-noncoherent;
power-domains = <&pwrc JH7110_PD_VOUT>;
+ resets = <&syscrg JH7110_SYSRST_NOC_BUS_DISP_AXI>;
+
+ dc8200: display@29400000 {
+ compatible = "starfive,jh7110-dc8200", "verisilicon,dc";[Severity: Medium] Should this SoC-level IP definition include status = "disabled"? Custom boards that include jh7110.dtsi but do not include jh7110-common.dtsi will attempt to probe this IP by default. Headless boards lacking physical HDMI connectors and necessary board-level pinmuxing may experience probe errors or unwanted resource consumption.
+ reg = <0x0 0x29400000 0x0 0x2800>; + clocks = <&voutcrg JH7110_VOUTCLK_DC8200_CORE>, + <&voutcrg JH7110_VOUTCLK_DC8200_AXI>,
[ ... ]
+ resets = <&voutcrg JH7110_VOUTRST_DC8200_CORE>,
+ <&voutcrg JH7110_VOUTRST_DC8200_AXI>,
+ <&voutcrg JH7110_VOUTRST_DC8200_AHB>;
+ reset-names = "core", "axi", "ahb";
+ };
+
+ hdmi_subsystem: hdmi@29590000 {
+ compatible = "starfive,jh7110-hdmi-subsystem";[Severity: Medium] Should the hdmi_subsystem IP definition also be disabled by default at the SoC level? Like the dc8200 node above, enabling this by default in the SoC devicetree may cause unintended probe attempts on boards that do not utilize the display subsystem.
+ reg = <0x0 0x29590000 0x0 0x4000>;
+
+ hdmi_controller: controller {[ ... ] -- Sashiko AI review · https://sashiko.dev/#/patchset/20260915-jh7110-clean-send-v4-0-f0e4fd6f2cc8@samsung.com?part=19 -- linux-phy mailing list linux-phy@lists.infradead.org https://lists.infradead.org/mailman/listinfo/linux-phy