Re: [PATCH v2 14/15] riscv: dts: starfive: jh7110: Update DT for display subsystem

[Michal Wilczynski]

On 8/28/26 16:06, Icenowy Zheng wrote:
> 在 2026-08-28五的 15:47 +0200，Michal Wilczynski写道：
>> Activate the display subsystem by adding the dc8200, hdmi and voutcrg
>> nodes as direct soc children. Each references the PD_VOUT power
>> domain
>> through power-domains, so genpd powers the domain on demand, and the
>> HDMI-PHY -> voutcrg -> HDMI-controller probe order is resolved by
>> deferred probe.
>>
>> The monolithic hdmi node is modelled as a container with hdmi_phy
>> and hdmi_controller children. This lets the PHY (clock provider,
>> depending only on xin24m) and the controller (clock consumer) probe
>> independently, breaking the circular clock dependency with voutcrg.
>>
>> voutcrg consumes the pixel clock from the &hdmi_phy node instead of
>> the
>> old fixed-clock, and dc8200 gets its pixel clocks from voutcrg's
>> MUXes
>> and is marked dma-noncoherent for framebuffer DMA. The old, incorrect
>> hdmitx0-pixel-clock fixed-clock node is removed.
>>
>> Co-developed-by: Dominique Belhachemi <db@xxxxxxxxxx>
>> Signed-off-by: Dominique Belhachemi <db@xxxxxxxxxx>
>> Signed-off-by: Michal Wilczynski <m.wilczynski@xxxxxxxxxxx>
>> +		compatible = "fixed-clock";
>> +		#clock-cells = <0>;
>> +		clock-frequency = <24000000>;
>> +		clock-output-names = "xin24m";
>> +	};
>> +
>>  	soc {
>>  		compatible = "simple-bus";
>>  		interrupt-parent = <&plic>;
>> @@ -1199,22 +1200,92 @@ csi_phy: phy@19820000 {
>>  			#phy-cells = <0>;
>>  		};
>>  
>> +		dc8200: display@29400000 {
>> +			compatible = "verisilicon,dc";
>> +			reg = <0x0 0x29400000 0x0 0x2800>;
>> +			interrupts = <95>;
>> +			dma-noncoherent;
>> +
>> +			power-domains = <&pwrc JH7110_PD_VOUT>;
>> +
>> +			clocks = <&voutcrg
>> JH7110_VOUTCLK_DC8200_CORE>,
>> +				<&voutcrg
>> JH7110_VOUTCLK_DC8200_AXI>,
>> +				<&voutcrg
>> JH7110_VOUTCLK_DC8200_AHB>,
>> +				<&voutcrg
>> JH7110_VOUTCLK_DC8200_PIX0>,
>> +				<&voutcrg
>> JH7110_VOUTCLK_DC8200_PIX1>;
>> +			clock-names = "core", "axi", "ahb", "pix0",
>> "pix1";
>> +
>> +			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";
>> +			reg = <0x0 0x29590000 0x0 0x4000>;
>> +
>> +			/*
>> +			 * This MFD's PHY child is the first device
>> in the vout
>> +			 * region to touch registers, so it owns the
>> NoC display
>> +			 * bus clock + reset that gate access to the
>> region.
>> +			 * PD_VOUT is powered on by genpd via power-
>> domains.
>> +			 */
>> +			power-domains = <&pwrc JH7110_PD_VOUT>;
>> +			clocks = <&syscrg
>> JH7110_SYSCLK_NOC_BUS_DISP_AXI>;
>> +			resets = <&syscrg
>> JH7110_SYSRST_NOC_BUS_DISP_AXI>;
> 
> I think the NOC clocks/resets should be also needed by the DC8200
> either?
> 
> Although patching the DC8200 driver to add support for these
> clocks/resets looks a little dirty...
> 
> Maybe it's still good to have a vout subsys instead of a hdmi subsys...

Yeah that was the idea as it was modeling hardware correctly, however
indirectly it works thanks to the probing order. The problem with the
extra vout subsys as shown in the RFC was that an extra driver and extra
dt-binding were needed which seemed to complicate things a bit. But it
does seem more correct in a sense that it's modeling the relationship
correctly so I'll bring it back for v3.


Best regards,
-- 
Michal Wilczynski <m.wilczynski@xxxxxxxxxxx>

> 
> Thanks,
> Icenowy
>