| Message ID | 20211117143347.314294-11-s.hauer@pengutronix.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | arm64: dts: rockchip: enable vop2 and hdmi tx on quartz64a | expand |
On Wed, Nov 17, 2021 at 09:19:38AM -0600, Rob Herring wrote: > On Wed, Nov 17, 2021 at 8:34 AM Sascha Hauer <s.hauer@pengutronix.de> wrote: > > > > This enabled the VOP2 display controller along with hdmi and the > > required port routes which is enough to get a picture out of the > > hdmi port of the board. > > > > Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de> > > --- > > .../boot/dts/rockchip/rk3568-evb1-v10.dts | 24 +++++++++++++++++++ > > 1 file changed, 24 insertions(+) > > > > diff --git a/arch/arm64/boot/dts/rockchip/rk3568-evb1-v10.dts b/arch/arm64/boot/dts/rockchip/rk3568-evb1-v10.dts > > index 184e2aa2416af..156e001492173 100644 > > --- a/arch/arm64/boot/dts/rockchip/rk3568-evb1-v10.dts > > +++ b/arch/arm64/boot/dts/rockchip/rk3568-evb1-v10.dts > > @@ -106,6 +106,12 @@ &gmac1m1_rgmii_clk > > status = "okay"; > > }; > > > > +&hdmi { > > + status = "okay"; > > + avdd-0v9-supply = <&vdda0v9_image>; > > + avdd-1v8-supply = <&vcca1v8_image>; > > +}; > > + > > &i2c0 { > > status = "okay"; > > > > @@ -390,3 +396,21 @@ &sdmmc0 { > > &uart2 { > > status = "okay"; > > }; > > + > > +&vop { > > + status = "okay"; > > + assigned-clocks = <&cru DCLK_VOP0>, <&cru DCLK_VOP1>; > > + assigned-clock-parents = <&pmucru PLL_HPLL>, <&cru PLL_VPLL>; > > +}; > > + > > +&vop_mmu { > > + status = "okay"; > > +}; > > + > > +&hdmi_in_vp0 { > > + status = "okay"; > > +}; > > + > > +&vp0_out_hdmi { > > + status = "okay"; > > +}; > > You can accomplish the same thing already with: > > &vp0_out_hdmi { > remote-endpoint = <&hdmi_in_vp0>; > }; My idea was to describe all possible connections in the dtsi file and let the board dts writer only en/disable the needed connections. When the connections are specified in the dts file then writing it is more difficult and error prone. > > or: > > &vp0_out_hdmi { > /delete-property/ remote-endpoint; > }; With this I have to change all connections that I don't need. With status = "okay" I have to change all connections that I actually do need, which will be much easier to read and write. I'll stick to the status = "okay" method for the next round, maybe I can still convince you ;) If it's the 'status' property you don't like being used when it's not a device that is enabled/disabled, then every other name would be fine with me as well. Sascha
Hi Sascha, Am Donnerstag, 2. Dezember 2021, 16:34:49 CET schrieb Sascha Hauer: > On Wed, Nov 17, 2021 at 09:19:38AM -0600, Rob Herring wrote: > > On Wed, Nov 17, 2021 at 8:34 AM Sascha Hauer <s.hauer@pengutronix.de> wrote: > > > > > > This enabled the VOP2 display controller along with hdmi and the > > > required port routes which is enough to get a picture out of the > > > hdmi port of the board. > > > > > > Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de> > > > --- > > > .../boot/dts/rockchip/rk3568-evb1-v10.dts | 24 +++++++++++++++++++ > > > 1 file changed, 24 insertions(+) > > > > > > diff --git a/arch/arm64/boot/dts/rockchip/rk3568-evb1-v10.dts b/arch/arm64/boot/dts/rockchip/rk3568-evb1-v10.dts > > > index 184e2aa2416af..156e001492173 100644 > > > --- a/arch/arm64/boot/dts/rockchip/rk3568-evb1-v10.dts > > > +++ b/arch/arm64/boot/dts/rockchip/rk3568-evb1-v10.dts > > > @@ -106,6 +106,12 @@ &gmac1m1_rgmii_clk > > > status = "okay"; > > > }; > > > > > > +&hdmi { > > > + status = "okay"; > > > + avdd-0v9-supply = <&vdda0v9_image>; > > > + avdd-1v8-supply = <&vcca1v8_image>; > > > +}; > > > + > > > &i2c0 { > > > status = "okay"; > > > > > > @@ -390,3 +396,21 @@ &sdmmc0 { > > > &uart2 { > > > status = "okay"; > > > }; > > > + > > > +&vop { > > > + status = "okay"; > > > + assigned-clocks = <&cru DCLK_VOP0>, <&cru DCLK_VOP1>; > > > + assigned-clock-parents = <&pmucru PLL_HPLL>, <&cru PLL_VPLL>; > > > +}; > > > + > > > +&vop_mmu { > > > + status = "okay"; > > > +}; > > > + > > > +&hdmi_in_vp0 { > > > + status = "okay"; > > > +}; > > > + > > > +&vp0_out_hdmi { > > > + status = "okay"; > > > +}; > > > > You can accomplish the same thing already with: > > > > &vp0_out_hdmi { > > remote-endpoint = <&hdmi_in_vp0>; > > }; > > My idea was to describe all possible connections in the dtsi file and > let the board dts writer only en/disable the needed connections. When > the connections are specified in the dts file then writing it is more > difficult and error prone. > > > > > or: > > > > &vp0_out_hdmi { > > /delete-property/ remote-endpoint; > > }; > > With this I have to change all connections that I don't need. With > status = "okay" I have to change all connections that I actually do > need, which will be much easier to read and write. > > I'll stick to the status = "okay" method for the next round, maybe I can > still convince you ;) > > If it's the 'status' property you don't like being used when it's not a > device that is enabled/disabled, then every other name would be fine > with me as well. hmm, we do have code in the rockchip drm-driver to find out if the device at the end of a graph-connection is disabled or not [0] , So on previous Rockchip socs, there are already all connections established, and the driver weeds out the disabled ones. So I'm wondering what is missing to use that in a vop2 context? Heiko [0] https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/gpu/drm/rockchip/rockchip_drm_drv.c#n274
Hi Heiko, On Thu, Dec 02, 2021 at 04:41:17PM +0100, Heiko Stübner wrote: > Hi Sascha, > > Am Donnerstag, 2. Dezember 2021, 16:34:49 CET schrieb Sascha Hauer: > > On Wed, Nov 17, 2021 at 09:19:38AM -0600, Rob Herring wrote: > > > On Wed, Nov 17, 2021 at 8:34 AM Sascha Hauer <s.hauer@pengutronix.de> wrote: > > > > > > > > This enabled the VOP2 display controller along with hdmi and the > > > > required port routes which is enough to get a picture out of the > > > > hdmi port of the board. > > > > > > > > Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de> > > > > --- > > > > .../boot/dts/rockchip/rk3568-evb1-v10.dts | 24 +++++++++++++++++++ > > > > 1 file changed, 24 insertions(+) > > > > > > > > diff --git a/arch/arm64/boot/dts/rockchip/rk3568-evb1-v10.dts b/arch/arm64/boot/dts/rockchip/rk3568-evb1-v10.dts > > > > index 184e2aa2416af..156e001492173 100644 > > > > --- a/arch/arm64/boot/dts/rockchip/rk3568-evb1-v10.dts > > > > +++ b/arch/arm64/boot/dts/rockchip/rk3568-evb1-v10.dts > > > > @@ -106,6 +106,12 @@ &gmac1m1_rgmii_clk > > > > status = "okay"; > > > > }; > > > > > > > > +&hdmi { > > > > + status = "okay"; > > > > + avdd-0v9-supply = <&vdda0v9_image>; > > > > + avdd-1v8-supply = <&vcca1v8_image>; > > > > +}; > > > > + > > > > &i2c0 { > > > > status = "okay"; > > > > > > > > @@ -390,3 +396,21 @@ &sdmmc0 { > > > > &uart2 { > > > > status = "okay"; > > > > }; > > > > + > > > > +&vop { > > > > + status = "okay"; > > > > + assigned-clocks = <&cru DCLK_VOP0>, <&cru DCLK_VOP1>; > > > > + assigned-clock-parents = <&pmucru PLL_HPLL>, <&cru PLL_VPLL>; > > > > +}; > > > > + > > > > +&vop_mmu { > > > > + status = "okay"; > > > > +}; > > > > + > > > > +&hdmi_in_vp0 { > > > > + status = "okay"; > > > > +}; > > > > + > > > > +&vp0_out_hdmi { > > > > + status = "okay"; > > > > +}; > > > > > > You can accomplish the same thing already with: > > > > > > &vp0_out_hdmi { > > > remote-endpoint = <&hdmi_in_vp0>; > > > }; > > > > My idea was to describe all possible connections in the dtsi file and > > let the board dts writer only en/disable the needed connections. When > > the connections are specified in the dts file then writing it is more > > difficult and error prone. > > > > > > > > or: > > > > > > &vp0_out_hdmi { > > > /delete-property/ remote-endpoint; > > > }; > > > > With this I have to change all connections that I don't need. With > > status = "okay" I have to change all connections that I actually do > > need, which will be much easier to read and write. > > > > I'll stick to the status = "okay" method for the next round, maybe I can > > still convince you ;) > > > > If it's the 'status' property you don't like being used when it's not a > > device that is enabled/disabled, then every other name would be fine > > with me as well. > > hmm, we do have code in the rockchip drm-driver to find out > if the device at the end of a graph-connection is disabled or not [0] , > So on previous Rockchip socs, there are already all connections > established, and the driver weeds out the disabled ones. > > So I'm wondering what is missing to use that in a vop2 context? The vop2 has three video ports (crtcs) instead of only one. All three are described in the device tree and each of them has a of_graph connection to the different encoders, so something like: vp0 <-> hdmi vp0 <-> mipi vp1 <-> hdmi vp1 <-> mipi vp2 <-> hdmi vp2 <-> mipi Enabling just vp0 <-> hdmi means only the first video port is can do hdmi. Different constraints in the clock tree (hdmi reference clock is hardwired to hpll, not enough PLLs to put all video ports on independent ones) prevent us from just allowing all connections. Sascha
diff --git a/arch/arm64/boot/dts/rockchip/rk3568-evb1-v10.dts b/arch/arm64/boot/dts/rockchip/rk3568-evb1-v10.dts index 184e2aa2416af..156e001492173 100644 --- a/arch/arm64/boot/dts/rockchip/rk3568-evb1-v10.dts +++ b/arch/arm64/boot/dts/rockchip/rk3568-evb1-v10.dts @@ -106,6 +106,12 @@ &gmac1m1_rgmii_clk status = "okay"; }; +&hdmi { + status = "okay"; + avdd-0v9-supply = <&vdda0v9_image>; + avdd-1v8-supply = <&vcca1v8_image>; +}; + &i2c0 { status = "okay"; @@ -390,3 +396,21 @@ &sdmmc0 { &uart2 { status = "okay"; }; + +&vop { + status = "okay"; + assigned-clocks = <&cru DCLK_VOP0>, <&cru DCLK_VOP1>; + assigned-clock-parents = <&pmucru PLL_HPLL>, <&cru PLL_VPLL>; +}; + +&vop_mmu { + status = "okay"; +}; + +&hdmi_in_vp0 { + status = "okay"; +}; + +&vp0_out_hdmi { + status = "okay"; +};
This enabled the VOP2 display controller along with hdmi and the required port routes which is enough to get a picture out of the hdmi port of the board. Signed-off-by: Sascha Hauer <s.hauer@pengutronix.de> --- .../boot/dts/rockchip/rk3568-evb1-v10.dts | 24 +++++++++++++++++++ 1 file changed, 24 insertions(+)