| Message ID | 20241106123758.423584-4-heiko@sntech.de (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | arm64: dts: rockchip: add and enable DSI2 on rk3588 | expand |
Hi Heiko, On 11/6/24 1:37 PM, Heiko Stuebner wrote: > From: Heiko Stuebner <heiko.stuebner@cherry.de> > > This adds support for the video-demo-adapter for the Haikou devkit with > Tiger RK3588 SoM. > > The Video Demo adapter is an adapter connected to the fake PCIe slot > labeled "Video Connector" on the Haikou devkit. > > It's main feature is a Leadtek DSI-display with touchscreen. To drive these Well that and a camera :) Maybe we can provide the "official" name of that adapter so people can easily find it on our product page? DEVKIT ADDON CAM-TS-A01 https://embedded.cherry.de/product/development-kit/ > components a number of additional regulators are grouped on the adapter as > well as a PCA9670 gpio-expander to provide the needed additional gpio-lines. > > Signed-off-by: Heiko Stuebner <heiko.stuebner@cherry.de> > --- > arch/arm64/boot/dts/rockchip/Makefile | 1 + > .../rk3588-tiger-haikou-video-demo.dtso | 153 ++++++++++++++++++ > 2 files changed, 154 insertions(+) > create mode 100644 arch/arm64/boot/dts/rockchip/rk3588-tiger-haikou-video-demo.dtso > > diff --git a/arch/arm64/boot/dts/rockchip/Makefile b/arch/arm64/boot/dts/rockchip/Makefile > index 09423070c992..0c4ee6a767b8 100644 > --- a/arch/arm64/boot/dts/rockchip/Makefile > +++ b/arch/arm64/boot/dts/rockchip/Makefile > @@ -143,6 +143,7 @@ dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3588-rock-5b.dtb > dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3588-rock-5b-pcie-ep.dtbo > dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3588-rock-5b-pcie-srns.dtbo > dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3588-tiger-haikou.dtb > +dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3588-tiger-haikou-video-demo.dtbo > 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-haikou-video-demo.dtso b/arch/arm64/boot/dts/rockchip/rk3588-tiger-haikou-video-demo.dtso > new file mode 100644 > index 000000000000..c7416349e7d5 > --- /dev/null > +++ b/arch/arm64/boot/dts/rockchip/rk3588-tiger-haikou-video-demo.dtso > @@ -0,0 +1,153 @@ > +// SPDX-License-Identifier: (GPL-2.0+ OR MIT) > +/* > + * DT-overlay for the camera / DSI demo appliance for Haikou boards. > + * In the flavour for use with a Tiger system-on-module. Add the official name in the comment here (maybe a link too?). Aren't we missing a copyright notice here as well (I personally don't care :) )? If so, that would be Copyright (C) 2024 Cherry Embedded Solutions GmbH for us. > + */ > + > +/dts-v1/; > +/plugin/; > + > +#include <dt-bindings/gpio/gpio.h> > +#include <dt-bindings/interrupt-controller/irq.h> > +#include <dt-bindings/leds/common.h> > +#include <dt-bindings/pinctrl/rockchip.h> > +#include <dt-bindings/soc/rockchip,vop2.h> > + > +&{/} { > + video-adapter-leds { Please order the register-less-direct-children-nodes-of-root alphabetically. > + compatible = "gpio-leds"; > + status = "okay"; Isn't the default if omitted already "okay"? > + > + video-adapter-led { > + color = <LED_COLOR_ID_BLUE>; > + gpios = <&pca9670 7 GPIO_ACTIVE_HIGH>; > + label = "video-adapter-led"; > + linux,default-trigger = "none"; > + }; > + }; > + > + backlight: backlight { > + compatible = "pwm-backlight"; > + power-supply = <&dc_12v>; > + pwms = <&pwm0 0 25000 0>; > + }; > + > + hdmi-con { > + compatible = "hdmi-connector"; > + type = "a"; > + > + port { > + hdmi_con_in: endpoint { > + remote-endpoint = <&hdmi0_out_con>; > + }; > + }; > + }; > + Wrong copy-paste here I guess? No HDMI connector on that adapter :) > + vcc1v8_video: regulator-vcc1v8-video { > + compatible = "regulator-fixed"; > + regulator-name = "vcc1v8-video"; > + regulator-always-on; > + regulator-boot-on; > + regulator-min-microvolt = <1800000>; > + regulator-max-microvolt = <1800000>; > + vin-supply = <&vcc3v3_baseboard>; > + }; > + > + vcc2v8_video: regulator-vcc2v8-video { > + compatible = "regulator-fixed"; > + regulator-name = "vcc2v8-video"; > + regulator-always-on; > + regulator-boot-on; > + regulator-min-microvolt = <2800000>; > + regulator-max-microvolt = <2800000>; > + vin-supply = <&vcc3v3_baseboard>; > + }; > +}; > + > +&dsi0 { > + #address-cells = <1>; > + #size-cells = <0>; Shouldn't those be in the SoC dtsi? Is there any world where this would be different per board? > + status = "okay"; > + > + panel@0 { > + compatible = "leadtek,ltk050h3148w"; > + reg = <0>; > + backlight = <&backlight>; > + iovcc-supply = <&vcc1v8_video>; > + reset-gpios = <&pca9670 0 GPIO_ACTIVE_LOW>; > + vci-supply = <&vcc2v8_video>; > + > + port { > + mipi_panel_in: endpoint { > + remote-endpoint = <&dsi0_out_panel>; > + }; > + }; > + }; > +}; > + > +&dsi0_in { > + dsi0_in_vp3: endpoint { > + remote-endpoint = <&vp3_out_dsi0>; > + }; > +}; > + > +&dsi0_out { > + dsi0_out_panel: endpoint { > + remote-endpoint = <&mipi_panel_in>; > + }; > +}; > + > +&i2c6 { > + /* OV5675, GT911, DW9714 are limited to 400KHz */ > + clock-frequency = <400000>; > + #address-cells = <1>; > + #size-cells = <0>; > + Mmmm why the address and size cells properties here? They should already be part of the SoC dtsi no? > + touchscreen@14 { > + compatible = "goodix,gt911"; > + reg = <0x14>; > + interrupt-parent = <&gpio3>; > + interrupts = <RK_PC3 IRQ_TYPE_LEVEL_LOW>; > + irq-gpios = <&gpio3 RK_PC3 GPIO_ACTIVE_HIGH>; > + pinctrl-0 = <&touch_int>; > + pinctrl-names = "default"; > + reset-gpios = <&pca9670 1 GPIO_ACTIVE_HIGH>; So this is correct because of a cancelling mistake. The driver inverts the signal and here we invert it as well. c.f. https://lore.kernel.org/linux-rockchip/20221103-upstream-goodix-reset-v3-0-0975809eb183@theobroma-systems.com/ I haven't send a new version yet, I don't really know what should be done there. The main pain point being that this is probably a big patch that spans multiple maintainer trees at once? > + AVDD28-supply = <&vcc2v8_video>; > + VDDIO-supply = <&vcc3v3_baseboard>; > + }; > + > + pca9670: gpio@27 { > + compatible = "nxp,pca9670"; > + reg = <0x27>; > + gpio-controller; > + #gpio-cells = <2>; > + }; > +}; > + > +&mipidcphy0 { > + status = "okay"; > +}; > + > +&pinctrl { > + touch { > + touch_int: touch-int { > + rockchip,pins = <3 RK_PC3 RK_FUNC_GPIO &pcfg_pull_none>; > + }; > + }; > +}; > + > +&pwm0 { > + pinctrl-0 = <&pwm0m1_pins>; > + pinctrl-names = "default"; The other two pin muxes for PWM0 are either: - the pin used for CAN - the pin routed to an internal component (unexposed to Q7) and used as a GPIO so please move the pinctrl to the Tiger SoM DTSI. > + status = "okay"; > +}; > + > +&vp3 { > + #address-cells = <1>; > + #size-cells = <0>; > + Shouldn't those be in the SoC dtsi? Cheers, Quentin
Hi Quentin, Am Mittwoch, 6. November 2024, 14:18:49 CET schrieb Quentin Schulz: > On 11/6/24 1:37 PM, Heiko Stuebner wrote: > > diff --git a/arch/arm64/boot/dts/rockchip/Makefile b/arch/arm64/boot/dts/rockchip/Makefile > > index 09423070c992..0c4ee6a767b8 100644 > > --- a/arch/arm64/boot/dts/rockchip/Makefile > > +++ b/arch/arm64/boot/dts/rockchip/Makefile [...] > > +&dsi0 { > > + #address-cells = <1>; > > + #size-cells = <0>; > > Shouldn't those be in the SoC dtsi? Is there any world where this would > be different per board? It's more of a we need that here to _compile_ the dtbo, so one gets to duplicate them, as the dtbo is not compiled _against_ the dtb, hence dtc does not know about the parent #address-cells value. > > + status = "okay"; > > + > > + panel@0 { > > + compatible = "leadtek,ltk050h3148w"; > > + reg = <0>; > > + backlight = <&backlight>; > > + iovcc-supply = <&vcc1v8_video>; > > + reset-gpios = <&pca9670 0 GPIO_ACTIVE_LOW>; > > + vci-supply = <&vcc2v8_video>; > > + > > + port { > > + mipi_panel_in: endpoint { > > + remote-endpoint = <&dsi0_out_panel>; > > + }; > > + }; > > + }; > > +}; > > + > > +&dsi0_in { > > + dsi0_in_vp3: endpoint { > > + remote-endpoint = <&vp3_out_dsi0>; > > + }; > > +}; > > + > > +&dsi0_out { > > + dsi0_out_panel: endpoint { > > + remote-endpoint = <&mipi_panel_in>; > > + }; > > +}; > > + > > +&i2c6 { > > + /* OV5675, GT911, DW9714 are limited to 400KHz */ > > + clock-frequency = <400000>; > > + #address-cells = <1>; > > + #size-cells = <0>; > > + > > Mmmm why the address and size cells properties here? They should already > be part of the SoC dtsi no? same as above > > +&pwm0 { > > + pinctrl-0 = <&pwm0m1_pins>; > > + pinctrl-names = "default"; > > The other two pin muxes for PWM0 are either: > - the pin used for CAN > - the pin routed to an internal component (unexposed to Q7) and used as > a GPIO > > so please move the pinctrl to the Tiger SoM DTSI. the pwm0-pinctrl is actually already set in the tiger.dtsi, so I'll just remove it from here. > > + status = "okay"; > > +}; > > + > > +&vp3 { > > + #address-cells = <1>; > > + #size-cells = <0>; > > + > > Shouldn't those be in the SoC dtsi? same as dsi and i2c #adress-cells Heiko
diff --git a/arch/arm64/boot/dts/rockchip/Makefile b/arch/arm64/boot/dts/rockchip/Makefile index 09423070c992..0c4ee6a767b8 100644 --- a/arch/arm64/boot/dts/rockchip/Makefile +++ b/arch/arm64/boot/dts/rockchip/Makefile @@ -143,6 +143,7 @@ dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3588-rock-5b.dtb dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3588-rock-5b-pcie-ep.dtbo dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3588-rock-5b-pcie-srns.dtbo dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3588-tiger-haikou.dtb +dtb-$(CONFIG_ARCH_ROCKCHIP) += rk3588-tiger-haikou-video-demo.dtbo 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-haikou-video-demo.dtso b/arch/arm64/boot/dts/rockchip/rk3588-tiger-haikou-video-demo.dtso new file mode 100644 index 000000000000..c7416349e7d5 --- /dev/null +++ b/arch/arm64/boot/dts/rockchip/rk3588-tiger-haikou-video-demo.dtso @@ -0,0 +1,153 @@ +// SPDX-License-Identifier: (GPL-2.0+ OR MIT) +/* + * DT-overlay for the camera / DSI demo appliance for Haikou boards. + * In the flavour for use with a Tiger system-on-module. + */ + +/dts-v1/; +/plugin/; + +#include <dt-bindings/gpio/gpio.h> +#include <dt-bindings/interrupt-controller/irq.h> +#include <dt-bindings/leds/common.h> +#include <dt-bindings/pinctrl/rockchip.h> +#include <dt-bindings/soc/rockchip,vop2.h> + +&{/} { + video-adapter-leds { + compatible = "gpio-leds"; + status = "okay"; + + video-adapter-led { + color = <LED_COLOR_ID_BLUE>; + gpios = <&pca9670 7 GPIO_ACTIVE_HIGH>; + label = "video-adapter-led"; + linux,default-trigger = "none"; + }; + }; + + backlight: backlight { + compatible = "pwm-backlight"; + power-supply = <&dc_12v>; + pwms = <&pwm0 0 25000 0>; + }; + + hdmi-con { + compatible = "hdmi-connector"; + type = "a"; + + port { + hdmi_con_in: endpoint { + remote-endpoint = <&hdmi0_out_con>; + }; + }; + }; + + vcc1v8_video: regulator-vcc1v8-video { + compatible = "regulator-fixed"; + regulator-name = "vcc1v8-video"; + regulator-always-on; + regulator-boot-on; + regulator-min-microvolt = <1800000>; + regulator-max-microvolt = <1800000>; + vin-supply = <&vcc3v3_baseboard>; + }; + + vcc2v8_video: regulator-vcc2v8-video { + compatible = "regulator-fixed"; + regulator-name = "vcc2v8-video"; + regulator-always-on; + regulator-boot-on; + regulator-min-microvolt = <2800000>; + regulator-max-microvolt = <2800000>; + vin-supply = <&vcc3v3_baseboard>; + }; +}; + +&dsi0 { + #address-cells = <1>; + #size-cells = <0>; + status = "okay"; + + panel@0 { + compatible = "leadtek,ltk050h3148w"; + reg = <0>; + backlight = <&backlight>; + iovcc-supply = <&vcc1v8_video>; + reset-gpios = <&pca9670 0 GPIO_ACTIVE_LOW>; + vci-supply = <&vcc2v8_video>; + + port { + mipi_panel_in: endpoint { + remote-endpoint = <&dsi0_out_panel>; + }; + }; + }; +}; + +&dsi0_in { + dsi0_in_vp3: endpoint { + remote-endpoint = <&vp3_out_dsi0>; + }; +}; + +&dsi0_out { + dsi0_out_panel: endpoint { + remote-endpoint = <&mipi_panel_in>; + }; +}; + +&i2c6 { + /* OV5675, GT911, DW9714 are limited to 400KHz */ + clock-frequency = <400000>; + #address-cells = <1>; + #size-cells = <0>; + + touchscreen@14 { + compatible = "goodix,gt911"; + reg = <0x14>; + interrupt-parent = <&gpio3>; + interrupts = <RK_PC3 IRQ_TYPE_LEVEL_LOW>; + irq-gpios = <&gpio3 RK_PC3 GPIO_ACTIVE_HIGH>; + pinctrl-0 = <&touch_int>; + pinctrl-names = "default"; + reset-gpios = <&pca9670 1 GPIO_ACTIVE_HIGH>; + AVDD28-supply = <&vcc2v8_video>; + VDDIO-supply = <&vcc3v3_baseboard>; + }; + + pca9670: gpio@27 { + compatible = "nxp,pca9670"; + reg = <0x27>; + gpio-controller; + #gpio-cells = <2>; + }; +}; + +&mipidcphy0 { + status = "okay"; +}; + +&pinctrl { + touch { + touch_int: touch-int { + rockchip,pins = <3 RK_PC3 RK_FUNC_GPIO &pcfg_pull_none>; + }; + }; +}; + +&pwm0 { + pinctrl-0 = <&pwm0m1_pins>; + pinctrl-names = "default"; + status = "okay"; +}; + +&vp3 { + #address-cells = <1>; + #size-cells = <0>; + + vp3_out_dsi0: endpoint@ROCKCHIP_VOP2_EP_MIPI0 { + reg = <ROCKCHIP_VOP2_EP_MIPI0>; + remote-endpoint = <&dsi0_in_vp3>; + }; +};