Re: [PATCH 2/2] arm64: dts: rockchip: add RK3588 DP carrier from Theobroma Systems

[Quentin Schulz <quentin.schulz@cherry.de>]

Hi Heiko,

On 7/16/25 8:50 PM, Heiko Stuebner wrote:
> From: Heiko Stuebner <heiko.stuebner@cherry.de>
> 
> The DisplayPort carrier is a very simple baseboard only providing serial,
> ethernet and a displayport output.
> 
> But its main functionality is that it routes the Analogix eDP controller
> to this DisplayPort output, which allows to test that controller simply
> by hooking it up to a suitable monitor.
> 
> The Analogix-DP controller supports eDP 1.3 and DP 1.2, so can drive
> both eDP displays as well as full DP monitors. It does not support DP+
> so passive DP-to-HDMI adapters won't work.
> 
> Signed-off-by: Heiko Stuebner <heiko.stuebner@cherry.de>
> ---
>   arch/arm64/boot/dts/rockchip/Makefile         |   1 +
>   .../rk3588-tiger-displayport-carrier.dts      | 118 ++++++++++++++++++
>   2 files changed, 119 insertions(+)
>   create mode 100644 arch/arm64/boot/dts/rockchip/rk3588-tiger-displayport-carrier.dts
> 
> diff --git a/arch/arm64/boot/dts/rockchip/Makefile b/arch/arm64/boot/dts/rockchip/Makefile
> index 7946bec10670..0c915679fde6 100644
> --- a/arch/arm64/boot/dts/rockchip/Makefile
> +++ b/arch/arm64/boot/dts/rockchip/Makefile
> @@ -178,6 +178,7 @@ dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3588-rock-5b-pcie-srns.dtbo
>   dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3588-rock-5b-plus.dtb
>   dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3588-tiger-haikou.dtb
>   dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3588-tiger-haikou-video-demo.dtbo
> +dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3588-tiger-displayport-carrier.dtb

Please sort this alphabetically.

>   dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3588-toybrick-x0.dtb
>   dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3588-turing-rk1.dtb
>   dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3588s-coolpi-4b.dtb
> diff --git a/arch/arm64/boot/dts/rockchip/rk3588-tiger-displayport-carrier.dts b/arch/arm64/boot/dts/rockchip/rk3588-tiger-displayport-carrier.dts
> new file mode 100644
> index 000000000000..5bd79877194a
> --- /dev/null
> +++ b/arch/arm64/boot/dts/rockchip/rk3588-tiger-displayport-carrier.dts
> @@ -0,0 +1,118 @@
> +// SPDX-License-Identifier: (GPL-2.0+ OR MIT)
> +/*
> + * Copyright (c) 2024 Cherry Embedded Solutions GmbH
> + */
> +
> +/dts-v1/;
> +#include <dt-bindings/soc/rockchip,vop2.h>
> +#include "rk3588-tiger.dtsi"
> +
> +/ {
> +	model = "Theobroma Systems RK3588-Q7 SoM on Tiger Displayport Carrier v1";
> +	compatible = "tsd,rk3588-tiger-displayport-carrier", "tsd,rk3588-tiger", "rockchip,rk3588";
> +

Please add ethernet0 to /aliases?

> +	chosen {
> +		stdout-path = "serial2:115200n8";
> +	};
> +
> +	dp-connector {
> +		compatible = "dp-connector";
> +		dp-pwr-supply = <&vcc3v3_baseboard>;
> +		label = "dp0";
> +		type = "full-size";

Please order alphabetically the two properties above.

> +
> +		/* Q7_DP_HPD# but Q7_HDMI_HPD# could be used too */
> +		hpd-gpios = <&gpio4 RK_PB5 GPIO_ACTIVE_LOW>;
> +		pinctrl-0 = <&edp0_hpd_l>;
> +		pinctrl-names = "default";
> +
> +		port {
> +			dp_con_in: endpoint {
> +				remote-endpoint = <&edp0_out_con>;
> +			};
> +		};
> +	};
> +
> +	vcc3v3_baseboard: regulator-vcc3v3-baseboard {
> +		compatible = "regulator-fixed";
> +		regulator-name = "vcc3v3_baseboard";
> +		regulator-always-on;
> +		regulator-boot-on;
> +		regulator-min-microvolt = <3300000>;
> +		regulator-max-microvolt = <3300000>;
> +		vin-supply = <&vcc5v0_baseboard>;
> +	};
> +
> +	vcc5v0_baseboard: regulator-vcc5v0-baseboard {
> +		compatible = "regulator-fixed";
> +		regulator-name = "vcc5v0_baseboard";
> +		regulator-always-on;
> +		regulator-boot-on;
> +		regulator-min-microvolt = <5000000>;
> +		regulator-max-microvolt = <5000000>;
> +	};
> +};
> +
> +&edp0 {
> +	/*
> +	 * Note that this board doesn't respect Q7 standard as Q7 has
> +	 * lane 2 of HDMI on the same pin as lane 0 of DP and lane 0 of HDMI
> +	 * on lane 2 of DP but this board uses HDMI lane numbering instead of
> +	 * DP lane numbering for its DP lanes.
> +	 *
> +	 * It should be possible to match Q7 standard if required by requesting
> +	 * the eDPTX controller to swap lanes with:
> +	 *
> +	 * data-lanes = <2 1 0 3>;

I don't think that's true currently?

The driver only seems to be supporting swapping the lanes, c.f. 
analogix_dp_lane_swap() which routes it to either 0 1 2 3 or 3 2 1 0 but 
no mix like the one we would need. Also, this function is never called 
with anything but "do not swap" argument. So we would need to fix the 
driver to support that.

Also, this is the kind of information we should have in Tiger DTSI and 
not some "random" baseboard DTS which will actually not make use of it. 
We should have data-lanes = <2 1 0 3>; in Tiger DTSI actually but 
considering 1) the driver doesn't support that, 2) we don't have HW to 
validate it works; I guess we'll just leave it at that.

> +	 */
> +

Aren't we missing the data-lanes property here? I don't see a default 
and I would assume we should be providing one?

Documentation/devicetree/bindings/media/video-interfaces.yaml specifies 
it either starts at 0 or 1, depending if there's a clock lane or not. As 
far as I could tell, DP has 4 data lanes and the AUX lane as well, but 
nothing clock-specific so I assume starting at 0 would be correct?

On a side note, is the audio output on eDP TX controller already 
supported like on HDMI? If so, we should add the edp0-sound node like in 
our downstream fork. But looking at the driver, I don't think we 
implement any dp_audio_* callbacks from drm_bridge_funcs which I assume 
would be the way to implement that.

Cheers,
Quentin