Re: [PATCH v2 0/7] Add DSI support for RK3128
Am 09.05.24 um 14:43 schrieb Alex Bee: Hi Heiko Am 09.05.24 um 14:21 schrieb Heiko Stübner: Hi Alex, Am Donnerstag, 9. Mai 2024, 14:07:08 CEST schrieb Alex Bee: This series aims to add support for the DesignWare MIPI DSI controller and the Innoslicon D-PHY found in RK3128 SoCs. The code additions are rather tiny: It only need some code in the Rockchip dw-mipi-dsi glue layer for this SoC, add support for an additional clock and do some changes in the SoC's clock driver. Support for the phy was already added when the Innosilicon D-PHY driver was initially submitted. I tested it with a 800x1280 DSI panel where all 4 lanes that are supported are used. changes in v2: To improve power-efficiency when the DSI controller is not in use, I dropped the patch which made hclk_vio_h2p a critical clock and instead added support for an AHB clock to the DSI controller driver and updated the bindings and the addition to the SoC DT accordingly. The naming already suggests that hclk_vio_h2p is not a clock-part of the actual dsi controller, but more an internal thing inside the clock controller. At least naming and perceived functionality would suggest a chain of hclk_vio -> hclk_vio_h2p -> pclk_mipi I personally wouldn't give to much on naming when it comes to Rockchip CRUs. Actually looking at "Fig. 2-5 Chip Clock Architecture Diagram 4" of RK312x its: ... -> hclk_vio -> hclk_h2p (clock in question) -> pclk_mipi (DSI APB clock) -> hclk_rga -> hclk_vop Also there is no other display output path (HDMI, LVDS) which requires this clock to be enabled. They all work when it's disabled. That really makes me think it's just the AHB clock line for the DSI controller. Maybe Andy can share some details? Anyway: I just looked at the "MIPI Controller architecture" part of the TRM - there is not even AHB clock line, only APB. So I revert the change with the additional clock, make the h2p-clock critical again and resend. Alex In any case, I really don't see hclk_vio_h2p to be in the realm of the actual DSI controller, but more a part of clock-controller / interconnect. Similar to the NIU clocks for the interconnect. rk3588 actually tries to implement this already and while the gate-link clocks are described as "recent", I think this definitly the same concept used a most/all older Rockchip SoCs, just nobody cared about that till now ;-) [0] . So TL;DR I'd really prefer to not leak CRU-details into the DSI controller. Heiko [0] Which reminds me that I should look at Sebastian's make GATE-LINK actually-work-patch.
Re: [PATCH v2 0/7] Add DSI support for RK3128
Hi Heiko Am 09.05.24 um 14:21 schrieb Heiko Stübner: Hi Alex, Am Donnerstag, 9. Mai 2024, 14:07:08 CEST schrieb Alex Bee: This series aims to add support for the DesignWare MIPI DSI controller and the Innoslicon D-PHY found in RK3128 SoCs. The code additions are rather tiny: It only need some code in the Rockchip dw-mipi-dsi glue layer for this SoC, add support for an additional clock and do some changes in the SoC's clock driver. Support for the phy was already added when the Innosilicon D-PHY driver was initially submitted. I tested it with a 800x1280 DSI panel where all 4 lanes that are supported are used. changes in v2: To improve power-efficiency when the DSI controller is not in use, I dropped the patch which made hclk_vio_h2p a critical clock and instead added support for an AHB clock to the DSI controller driver and updated the bindings and the addition to the SoC DT accordingly. The naming already suggests that hclk_vio_h2p is not a clock-part of the actual dsi controller, but more an internal thing inside the clock controller. At least naming and perceived functionality would suggest a chain of hclk_vio -> hclk_vio_h2p -> pclk_mipi I personally wouldn't give to much on naming when it comes to Rockchip CRUs. Actually looking at "Fig. 2-5 Chip Clock Architecture Diagram 4" of RK312x its: ... -> hclk_vio -> hclk_h2p (clock in question) -> pclk_mipi (DSI APB clock) -> hclk_rga -> hclk_vop Also there is no other display output path (HDMI, LVDS) which requires this clock to be enabled. They all work when it's disabled. That really makes me think it's just the AHB clock line for the DSI controller. Maybe Andy can share some details? Alex In any case, I really don't see hclk_vio_h2p to be in the realm of the actual DSI controller, but more a part of clock-controller / interconnect. Similar to the NIU clocks for the interconnect. rk3588 actually tries to implement this already and while the gate-link clocks are described as "recent", I think this definitly the same concept used a most/all older Rockchip SoCs, just nobody cared about that till now ;-) [0] . So TL;DR I'd really prefer to not leak CRU-details into the DSI controller. Heiko [0] Which reminds me that I should look at Sebastian's make GATE-LINK actually-work-patch.
Re: [PATCH v2 0/7] Add DSI support for RK3128
Hi Alex, Am Donnerstag, 9. Mai 2024, 14:07:08 CEST schrieb Alex Bee: > This series aims to add support for the DesignWare MIPI DSI controller and > the Innoslicon D-PHY found in RK3128 SoCs. The code additions are rather > tiny: It only need some code in the Rockchip dw-mipi-dsi glue layer for > this SoC, add support for an additional clock and do some changes in the > SoC's clock driver. Support for the phy was already added when the > Innosilicon D-PHY driver was initially submitted. I tested it with a > 800x1280 DSI panel where all 4 lanes that are supported are used. > > changes in v2: > To improve power-efficiency when the DSI controller is not in use, I > dropped the patch which made hclk_vio_h2p a critical clock and instead > added support for an AHB clock to the DSI controller driver and updated > the bindings and the addition to the SoC DT accordingly. The naming already suggests that hclk_vio_h2p is not a clock-part of the actual dsi controller, but more an internal thing inside the clock controller. At least naming and perceived functionality would suggest a chain of hclk_vio -> hclk_vio_h2p -> pclk_mipi In any case, I really don't see hclk_vio_h2p to be in the realm of the actual DSI controller, but more a part of clock-controller / interconnect. Similar to the NIU clocks for the interconnect. rk3588 actually tries to implement this already and while the gate-link clocks are described as "recent", I think this definitly the same concept used a most/all older Rockchip SoCs, just nobody cared about that till now ;-) [0] . So TL;DR I'd really prefer to not leak CRU-details into the DSI controller. Heiko [0] Which reminds me that I should look at Sebastian's make GATE-LINK actually-work-patch.
[PATCH v2 0/7] Add DSI support for RK3128
This series aims to add support for the DesignWare MIPI DSI controller and the Innoslicon D-PHY found in RK3128 SoCs. The code additions are rather tiny: It only need some code in the Rockchip dw-mipi-dsi glue layer for this SoC, add support for an additional clock and do some changes in the SoC's clock driver. Support for the phy was already added when the Innosilicon D-PHY driver was initially submitted. I tested it with a 800x1280 DSI panel where all 4 lanes that are supported are used. changes in v2: To improve power-efficiency when the DSI controller is not in use, I dropped the patch which made hclk_vio_h2p a critical clock and instead added support for an AHB clock to the DSI controller driver and updated the bindings and the addition to the SoC DT accordingly. Link to v1: https://lore.kernel.org/linux-kernel/20240506194343.282119-1-knaerz...@gmail.com/ Please see individual patches for details about the changes. Alex Bee (7): dt-bindings: display: rockchip,dw-mipi-dsi: Document RK3128 DSI dt-bindings: clock: rk3128: Add PCLK_MIPIPHY clk: rockchip: rk3128: Export PCLK_MIPIPHY drm/rockchip: dsi: Support optional AHB clock drm/rockchip: dsi: Add support for RK3128 ARM: dts: rockchip: Add D-PHY for RK3128 ARM: dts: rockchip: Add DSI for RK3128 .../rockchip/rockchip,dw-mipi-dsi.yaml| 25 +- arch/arm/boot/dts/rockchip/rk3128.dtsi| 50 +++ drivers/clk/rockchip/clk-rk3128.c | 2 +- .../gpu/drm/rockchip/dw-mipi-dsi-rockchip.c | 45 + include/dt-bindings/clock/rk3128-cru.h| 1 + 5 files changed, 121 insertions(+), 2 deletions(-) -- 2.43.2
