[PATCH v2 0/6] Add Synopsys DesignWare HDMI RX Controller

[PATCH v2 0/6] Add Synopsys DesignWare HDMI RX Controller

[Shreeya Patel]

This series implements support for the Synopsys DesignWare
HDMI RX Controller, being compliant with standard HDMI 1.4b
and HDMI 2.0.

Features that are currently supported by the HDMI RX driver
have been tested on rock5b board using a HDMI to micro-HDMI cable.
It is recommended to use a good quality cable as there were
multiple issues seen during testing the driver.

Please note the below information :-
* While testing the driver on rock5b we noticed that the binary BL31
from Rockchip contains some unknown code to get the HDMI-RX PHY
access working. With TF-A BL31, the HDMI-RX PHY doesn't work as
expected since there are no interrupts seen for rk_hdmirx-hdmi
leading to some failures in the driver [0].
* We have tested the working of OBS studio with HDMIRX driver and
there were no issues seen.

[0] https://gitlab.collabora.com/hardware-enablement/rockchip-3588/trusted-firmware-a/-/issues/1

To test the HDMI RX Controller driver, following example commands can be used :-

root@debian-rockchip-rock5b-rk3588:~# v4l2-ctl --verbose -d /dev/video0 \
--set-fmt-video=width=1920,height=1080,pixelformat='BGR3' --stream-mmap=4 \
--stream-skip=3 --stream-count=100 --stream-to=/home/hdmiin4k.raw --stream-poll

root@debian-rockchip-rock5b-rk3588:~# ffmpeg -f rawvideo -vcodec rawvideo \
-s 1920x1080 -r 60 -pix_fmt bgr24 -i /home/hdmiin4k.raw output.mkv


Following is the v4l2-compliance test result :-

root@debian-rockchip-rock5b-rk3588:~# v4l2-compliance -d /dev/video0
v4l2-compliance 1.27.0-5174, 64 bits, 64-bit time_t
v4l2-compliance SHA: d700deb14368 2024-01-18 12:19:05

Compliance test for snps_hdmirx device /dev/video0:

Driver Info:
        Driver name      : snps_hdmirx
        Card type        : snps_hdmirx
        Bus info         : platform: snps_hdmirx
        Driver version   : 6.8.0
        Capabilities     : 0x84201000
                Video Capture Multiplanar
                Streaming
                Extended Pix Format
                Device Capabilities
        Device Caps      : 0x04201000
                Video Capture Multiplanar
                Streaming
                Extended Pix Format

Required ioctls:
        test VIDIOC_QUERYCAP: OK
        test invalid ioctls: OK

Allow for multiple opens:
        test second /dev/video0 open: OK
        test VIDIOC_QUERYCAP: OK
        test VIDIOC_G/S_PRIORITY: OK
        test for unlimited opens: OK

Debug ioctls:
        test VIDIOC_DBG_G/S_REGISTER: OK (Not Supported)
        test VIDIOC_LOG_STATUS: OK

Input ioctls:
        test VIDIOC_G/S_TUNER/ENUM_FREQ_BANDS: OK (Not Supported)
        test VIDIOC_G/S_FREQUENCY: OK (Not Supported)
        test VIDIOC_S_HW_FREQ_SEEK: OK (Not Supported)
        test VIDIOC_ENUMAUDIO: OK (Not Supported)
        test VIDIOC_G/S/ENUMINPUT: OK
        test VIDIOC_G/S_AUDIO: OK (Not Supported)
        Inputs: 1 Audio Inputs: 0 Tuners: 0

Output ioctls:
        test VIDIOC_G/S_MODULATOR: OK (Not Supported)
        test VIDIOC_G/S_FREQUENCY: OK (Not Supported)
        test VIDIOC_ENUMAUDOUT: OK (Not Supported)
        test VIDIOC_G/S/ENUMOUTPUT: OK (Not Supported)
        test VIDIOC_G/S_AUDOUT: OK (Not Supported)
        Outputs: 0 Audio Outputs: 0 Modulators: 0

Input/Output configuration ioctls:
        test VIDIOC_ENUM/G/S/QUERY_STD: OK (Not Supported)
        test VIDIOC_ENUM/G/S/QUERY_DV_TIMINGS: OK
        test VIDIOC_DV_TIMINGS_CAP: OK
        test VIDIOC_G/S_EDID: OK

Control ioctls (Input 0):
        test VIDIOC_QUERY_EXT_CTRL/QUERYMENU: OK
        test VIDIOC_QUERYCTRL: OK
        test VIDIOC_G/S_CTRL: OK
        test VIDIOC_G/S/TRY_EXT_CTRLS: OK
        test VIDIOC_(UN)SUBSCRIBE_EVENT/DQEVENT: OK
        test VIDIOC_G/S_JPEGCOMP: OK (Not Supported)
        Standard Controls: 2 Private Controls: 0

Format ioctls (Input 0):
        test VIDIOC_ENUM_FMT/FRAMESIZES/FRAMEINTERVALS: OK
        test VIDIOC_G/S_PARM: OK
        test VIDIOC_G_FBUF: OK (Not Supported)
        test VIDIOC_G_FMT: OK
        test VIDIOC_TRY_FMT: OK
        test VIDIOC_S_FMT: OK
        test VIDIOC_G_SLICED_VBI_CAP: OK (Not Supported)
        test Cropping: OK (Not Supported)
        test Composing: OK (Not Supported)
        test Scaling: OK (Not Supported)

Codec ioctls (Input 0):
        test VIDIOC_(TRY_)ENCODER_CMD: OK (Not Supported)
        test VIDIOC_G_ENC_INDEX: OK (Not Supported)
        test VIDIOC_(TRY_)DECODER_CMD: OK (Not Supported)

Buffer ioctls (Input 0):
        test VIDIOC_REQBUFS/CREATE_BUFS/QUERYBUF: OK
        test CREATE_BUFS maximum buffers: OK
        test VIDIOC_EXPBUF: OK
        test Requests: OK (Not Supported)

Total for snps_hdmirx device /dev/video0: 46, Succeeded: 46, Failed: 0, Warnings: 0

Changes in v2 :-
  - Fix checkpatch --strict warnings
  - Move the dt-binding include file changes in a separate patch
  - Add a description for the hardware in the dt-bindings file
  - Rename resets, vo1 grf and HPD properties
  - Add a proper description for grf and vo1-grf phandles in the
    bindings
  - Rename the HDMI RX node name to hdmi-receiver
  - Include gpio header file in binding example to fix the
    dt_binding_check failure
  - Move hdmirx_cma node to the rk3588.dtsi file
  - Add an entry to MAINTAINERS file for the HDMIRX driver

Shreeya Patel (6):
  dt-bindings: reset: Define reset id used for HDMI Receiver
  clk: rockchip: rst-rk3588: Add reset line for HDMI Receiver
  dt-bindings: media: Document HDMI RX Controller
  arm64: dts: rockchip: Add device tree support for HDMI RX Controller
  media: platform: synopsys: Add support for hdmi input driver
  MAINTAINERS: Add entry for Synopsys DesignWare HDMI RX Driver

 .../bindings/media/snps,dw-hdmi-rx.yaml       |  132 +
 MAINTAINERS                                   |    8 +
 .../boot/dts/rockchip/rk3588-pinctrl.dtsi     |   41 +
 arch/arm64/boot/dts/rockchip/rk3588.dtsi      |   55 +
 drivers/clk/rockchip/rst-rk3588.c             |    1 +
 drivers/media/platform/Kconfig                |    1 +
 drivers/media/platform/Makefile               |    1 +
 drivers/media/platform/synopsys/Kconfig       |    3 +
 drivers/media/platform/synopsys/Makefile      |    2 +
 .../media/platform/synopsys/hdmirx/Kconfig    |   18 +
 .../media/platform/synopsys/hdmirx/Makefile   |    4 +
 .../platform/synopsys/hdmirx/snps_hdmirx.c    | 2856 +++++++++++++++++
 .../platform/synopsys/hdmirx/snps_hdmirx.h    |  393 +++
 .../synopsys/hdmirx/snps_hdmirx_cec.c         |  289 ++
 .../synopsys/hdmirx/snps_hdmirx_cec.h         |   46 +
 .../dt-bindings/reset/rockchip,rk3588-cru.h   |    2 +
 16 files changed, 3852 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/media/snps,dw-hdmi-rx.yaml
 create mode 100644 drivers/media/platform/synopsys/Kconfig
 create mode 100644 drivers/media/platform/synopsys/Makefile
 create mode 100644 drivers/media/platform/synopsys/hdmirx/Kconfig
 create mode 100644 drivers/media/platform/synopsys/hdmirx/Makefile
 create mode 100644 drivers/media/platform/synopsys/hdmirx/snps_hdmirx.c
 create mode 100644 drivers/media/platform/synopsys/hdmirx/snps_hdmirx.h
 create mode 100644 drivers/media/platform/synopsys/hdmirx/snps_hdmirx_cec.c
 create mode 100644 drivers/media/platform/synopsys/hdmirx/snps_hdmirx_cec.h

-- 
2.39.2


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v2 1/6] dt-bindings: reset: Define reset id used for HDMI Receiver

[Shreeya Patel]

Add reset id used for HDMI Receiver in RK3588 SoCs

Signed-off-by: Shreeya Patel <shreeya.patel@collabora.com>
---
Changes in v2 :-
  - Move the dt-binding include file changes in a separate patch
  - Improve the subject and commit message description

 include/dt-bindings/reset/rockchip,rk3588-cru.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/include/dt-bindings/reset/rockchip,rk3588-cru.h b/include/dt-bindings/reset/rockchip,rk3588-cru.h
index d4264db2a07f..e2fe4bd5f7f0 100644
--- a/include/dt-bindings/reset/rockchip,rk3588-cru.h
+++ b/include/dt-bindings/reset/rockchip,rk3588-cru.h
@@ -751,4 +751,6 @@
 #define SRST_P_TRNG_CHK			658
 #define SRST_TRNG_S			659
 
+#define SRST_A_HDMIRX_BIU		660
+
 #endif
-- 
2.39.2


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v2 2/6] clk: rockchip: rst-rk3588: Add reset line for HDMI Receiver

[Shreeya Patel]

Export hdmirx_biu reset line required by the Synopsys
DesignWare HDMIRX Controller.

Signed-off-by: Shreeya Patel <shreeya.patel@collabora.com>
---
Changes in v2 :-
  - Improve the subject line and commit message description.

 drivers/clk/rockchip/rst-rk3588.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/clk/rockchip/rst-rk3588.c b/drivers/clk/rockchip/rst-rk3588.c
index e855bb8d5413..c4ebc01f1c9c 100644
--- a/drivers/clk/rockchip/rst-rk3588.c
+++ b/drivers/clk/rockchip/rst-rk3588.c
@@ -577,6 +577,7 @@ static const int rk3588_register_offset[] = {
 
 	/* SOFTRST_CON59 */
 	RK3588_CRU_RESET_OFFSET(SRST_A_HDCP1_BIU, 59, 6),
+	RK3588_CRU_RESET_OFFSET(SRST_A_HDMIRX_BIU, 59, 7),
 	RK3588_CRU_RESET_OFFSET(SRST_A_VO1_BIU, 59, 8),
 	RK3588_CRU_RESET_OFFSET(SRST_H_VOP1_BIU, 59, 9),
 	RK3588_CRU_RESET_OFFSET(SRST_H_VOP1_S_BIU, 59, 10),
-- 
2.39.2


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v2 3/6] dt-bindings: media: Document HDMI RX Controller

[Shreeya Patel]

Document bindings for the Synopsys DesignWare HDMI RX Controller.

Reviewed-by: Dmitry Osipenko <dmitry.osipenko@collabora.com>
Signed-off-by: Shreeya Patel <shreeya.patel@collabora.com>
---
Changes in v2 :-
  - Add a description for the hardware
  - Rename resets, vo1 grf and HPD properties
  - Add a proper description for grf and vo1-grf phandles
  - Rename the HDMI Input node name to hdmi-receiver
  - Improve the subject line
  - Include gpio header file in example to fix dt_binding_check failure

 .../bindings/media/snps,dw-hdmi-rx.yaml       | 132 ++++++++++++++++++
 1 file changed, 132 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/media/snps,dw-hdmi-rx.yaml

diff --git a/Documentation/devicetree/bindings/media/snps,dw-hdmi-rx.yaml b/Documentation/devicetree/bindings/media/snps,dw-hdmi-rx.yaml
new file mode 100644
index 000000000000..ea9f728c11b3
--- /dev/null
+++ b/Documentation/devicetree/bindings/media/snps,dw-hdmi-rx.yaml
@@ -0,0 +1,132 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+# Device Tree bindings for Synopsys DesignWare HDMI RX Controller
+
+---
+$id: http://devicetree.org/schemas/media/snps,dw-hdmi-rx.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Synopsys DesignWare HDMI RX Controller
+
+maintainers:
+  - Shreeya Patel <shreeya.patel@collabora.com>
+
+description:
+  Synopsys DesignWare HDMI Input Controller preset on RK3588 SoCs
+  allowing devices to receive and decode high-resolution video streams
+  from external sources like media players, cameras, laptops, etc.
+
+properties:
+  compatible:
+    items:
+      - const: rockchip,rk3588-hdmirx-ctrler
+      - const: snps,dw-hdmi-rx
+
+  reg:
+    maxItems: 1
+
+  interrupts:
+    maxItems: 3
+
+  interrupt-names:
+    items:
+      - const: cec
+      - const: hdmi
+      - const: dma
+
+  clocks:
+    maxItems: 7
+
+  clock-names:
+    items:
+      - const: aclk
+      - const: audio
+      - const: cr_para
+      - const: pclk
+      - const: ref
+      - const: hclk_s_hdmirx
+      - const: hclk_vo1
+
+  power-domains:
+    maxItems: 1
+
+  resets:
+    maxItems: 4
+
+  reset-names:
+    items:
+      - const: axi
+      - const: apb
+      - const: ref
+      - const: biu
+
+  memory-region:
+    maxItems: 1
+
+  hpd-gpios:
+    description: GPIO specifier for HPD.
+    maxItems: 1
+
+  rockchip,grf:
+    $ref: /schemas/types.yaml#/definitions/phandle
+    description:
+      The phandle of the syscon node for the general register file
+      containing HDMIRX PHY status bits.
+
+  rockchip,vo1-grf:
+    $ref: /schemas/types.yaml#/definitions/phandle
+    description:
+      The phandle of the syscon node for the Video Output GRF register
+      to enable EDID transfer through SDAIN and SCLIN.
+
+required:
+  - compatible
+  - reg
+  - interrupts
+  - interrupt-names
+  - clocks
+  - clock-names
+  - power-domains
+  - resets
+  - pinctrl-0
+  - hpd-gpios
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/clock/rockchip,rk3588-cru.h>
+    #include <dt-bindings/gpio/gpio.h>
+    #include <dt-bindings/interrupt-controller/arm-gic.h>
+    #include <dt-bindings/interrupt-controller/irq.h>
+    #include <dt-bindings/power/rk3588-power.h>
+    #include <dt-bindings/reset/rockchip,rk3588-cru.h>
+    hdmi_receiver: hdmi-receiver@fdee0000 {
+      compatible = "rockchip,rk3588-hdmirx-ctrler", "snps,dw-hdmi-rx";
+      reg = <0xfdee0000 0x6000>;
+      interrupts = <GIC_SPI 177 IRQ_TYPE_LEVEL_HIGH 0>,
+                   <GIC_SPI 436 IRQ_TYPE_LEVEL_HIGH 0>,
+                   <GIC_SPI 179 IRQ_TYPE_LEVEL_HIGH 0>;
+      interrupt-names = "cec", "hdmi", "dma";
+      clocks = <&cru ACLK_HDMIRX>,
+               <&cru CLK_HDMIRX_AUD>,
+               <&cru CLK_CR_PARA>,
+               <&cru PCLK_HDMIRX>,
+               <&cru CLK_HDMIRX_REF>,
+               <&cru PCLK_S_HDMIRX>,
+               <&cru HCLK_VO1>;
+      clock-names = "aclk",
+                    "audio",
+                    "cr_para",
+                    "pclk",
+                    "ref",
+                    "hclk_s_hdmirx",
+                    "hclk_vo1";
+      power-domains = <&power RK3588_PD_VO1>;
+      resets = <&cru SRST_A_HDMIRX>, <&cru SRST_P_HDMIRX>,
+               <&cru SRST_HDMIRX_REF>, <&cru SRST_A_HDMIRX_BIU>;
+      reset-names = "axi", "apb", "ref", "biu";
+      pinctrl-0 = <&hdmim1_rx_cec &hdmim1_rx_hpdin &hdmim1_rx_scl &hdmim1_rx_sda &hdmirx_5v_detection>;
+      pinctrl-names = "default";
+      memory-region = <&hdmirx_cma>;
+      hpd-gpios = <&gpio1 22 GPIO_ACTIVE_LOW>;
+    };
-- 
2.39.2


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v2 4/6] arm64: dts: rockchip: Add device tree support for HDMI RX Controller

[Shreeya Patel]

Add device tree support for Synopsys DesignWare HDMI RX
Controller.

Reviewed-by: Dmitry Osipenko <dmitry.osipenko@collabora.com>
Tested-by: Dmitry Osipenko <dmitry.osipenko@collabora.com>
Co-developed-by: Dingxian Wen <shawn.wen@rock-chips.com>
Signed-off-by: Dingxian Wen <shawn.wen@rock-chips.com>
Signed-off-by: Shreeya Patel <shreeya.patel@collabora.com>
---
Changes in v2 :-
  - Fix some of the checkpatch errors and warnings
  - Rename resets, vo1-grf and HPD
  - Move hdmirx_cma node to the rk3588.dtsi file

 .../boot/dts/rockchip/rk3588-pinctrl.dtsi     | 41 ++++++++++++++
 arch/arm64/boot/dts/rockchip/rk3588.dtsi      | 55 +++++++++++++++++++
 2 files changed, 96 insertions(+)

diff --git a/arch/arm64/boot/dts/rockchip/rk3588-pinctrl.dtsi b/arch/arm64/boot/dts/rockchip/rk3588-pinctrl.dtsi
index 244c66faa161..4fbe194d96b1 100644
--- a/arch/arm64/boot/dts/rockchip/rk3588-pinctrl.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3588-pinctrl.dtsi
@@ -169,6 +169,47 @@ hdmim0_tx1_sda: hdmim0-tx1-sda {
 				/* hdmim0_tx1_sda */
 				<2 RK_PB4 4 &pcfg_pull_none>;
 		};
+
+		/omit-if-no-ref/
+		hdmim1_rx: hdmim1-rx {
+			rockchip,pins =
+				/* hdmim1_rx_cec */
+				<3 RK_PD1 5 &pcfg_pull_none>,
+				/* hdmim1_rx_scl */
+				<3 RK_PD2 5 &pcfg_pull_none_smt>,
+				/* hdmim1_rx_sda */
+				<3 RK_PD3 5 &pcfg_pull_none_smt>,
+				/* hdmim1_rx_hpdin */
+				<3 RK_PD4 5 &pcfg_pull_none>;
+		};
+
+		/omit-if-no-ref/
+		hdmim1_rx_cec: hdmim1-rx-cec {
+			rockchip,pins =
+				/* hdmim1_rx_cec */
+				<3 RK_PD1 5 &pcfg_pull_none>;
+		};
+
+		/omit-if-no-ref/
+		hdmim1_rx_hpdin: hdmim1-rx-hpdin {
+			rockchip,pins =
+				/* hdmim1_rx_hpdin */
+				<3 RK_PD4 5 &pcfg_pull_none>;
+		};
+
+		/omit-if-no-ref/
+		hdmim1_rx_scl: hdmim1-rx-scl {
+			rockchip,pins =
+				/* hdmim1_rx_scl */
+				<3 RK_PD2 5 &pcfg_pull_none>;
+		};
+
+		/omit-if-no-ref/
+		hdmim1_rx_sda: hdmim1-rx-sda {
+			rockchip,pins =
+				/* hdmim1_rx_sda */
+				<3 RK_PD3 5 &pcfg_pull_none>;
+		};
 	};
 
 	i2c0 {
diff --git a/arch/arm64/boot/dts/rockchip/rk3588.dtsi b/arch/arm64/boot/dts/rockchip/rk3588.dtsi
index 5519c1430cb7..8adb98b99701 100644
--- a/arch/arm64/boot/dts/rockchip/rk3588.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3588.dtsi
@@ -7,6 +7,24 @@
 #include "rk3588-pinctrl.dtsi"
 
 / {
+	reserved-memory {
+		#address-cells = <2>;
+		#size-cells = <2>;
+		ranges;
+		/*
+		 * The 4k HDMI capture controller works only with 32bit
+		 * phys addresses and doesn't support IOMMU. HDMI RX CMA
+		 * must be reserved below 4GB.
+		 */
+		hdmirx_cma: hdmirx_cma {
+			compatible = "shared-dma-pool";
+			alloc-ranges = <0x0 0x0 0x0 0xffffffff>;
+			size = <0x0 (160 * 0x100000)>; /* 160MiB */
+			no-map;
+			status = "disabled";
+		};
+	};
+
 	pcie30_phy_grf: syscon@fd5b8000 {
 		compatible = "rockchip,rk3588-pcie3-phy-grf", "syscon";
 		reg = <0x0 0xfd5b8000 0x0 0x10000>;
@@ -85,6 +103,38 @@ i2s10_8ch: i2s@fde00000 {
 		status = "disabled";
 	};
 
+	hdmi_receiver: hdmi-receiver@fdee0000 {
+		compatible = "rockchip,rk3588-hdmirx-ctrler", "snps,dw-hdmi-rx";
+		reg = <0x0 0xfdee0000 0x0 0x6000>;
+		power-domains = <&power RK3588_PD_VO1>;
+		rockchip,grf = <&sys_grf>;
+		rockchip,vo1-grf = <&vo1_grf>;
+		interrupts = <GIC_SPI 177 IRQ_TYPE_LEVEL_HIGH 0>,
+			     <GIC_SPI 436 IRQ_TYPE_LEVEL_HIGH 0>,
+			     <GIC_SPI 179 IRQ_TYPE_LEVEL_HIGH 0>;
+		interrupt-names = "cec", "hdmi", "dma";
+		clocks = <&cru ACLK_HDMIRX>,
+			 <&cru CLK_HDMIRX_AUD>,
+			 <&cru CLK_CR_PARA>,
+			 <&cru PCLK_HDMIRX>,
+			 <&cru CLK_HDMIRX_REF>,
+			 <&cru PCLK_S_HDMIRX>,
+			 <&cru HCLK_VO1>;
+		clock-names = "aclk",
+			      "audio",
+			      "cr_para",
+			      "pclk",
+			      "ref",
+			      "hclk_s_hdmirx",
+			      "hclk_vo1";
+		resets = <&cru SRST_A_HDMIRX>, <&cru SRST_P_HDMIRX>,
+			 <&cru SRST_HDMIRX_REF>, <&cru SRST_A_HDMIRX_BIU>;
+		reset-names = "axi", "apb", "ref", "biu";
+		pinctrl-0 = <&hdmim1_rx>;
+		pinctrl-names = "default";
+		status = "disabled";
+	};
+
 	pcie3x4: pcie@fe150000 {
 		compatible = "rockchip,rk3588-pcie", "rockchip,rk3568-pcie";
 		#address-cells = <3>;
@@ -339,3 +389,8 @@ pcie30phy: phy@fee80000 {
 		status = "disabled";
 	};
 };
+
+&hdmirx_cma {
+	status = "okay";
+};
+
-- 
2.39.2


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v2 6/6] MAINTAINERS: Add entry for Synopsys DesignWare HDMI RX Driver

[Shreeya Patel]

Add an entry for Synopsys DesignWare HDMI Receiver Controller
Driver.

Signed-off-by: Shreeya Patel <shreeya.patel@collabora.com>
---
Changes in v2 :-
  - Add a patch for MAINTAINERS file changes

 MAINTAINERS | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 897fbf1b0701..26ff6a1cb24a 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -21427,6 +21427,14 @@ F:	drivers/net/pcs/pcs-xpcs.c
 F:	drivers/net/pcs/pcs-xpcs.h
 F:	include/linux/pcs/pcs-xpcs.h
 
+SYNOPSYS DESIGNWARE HDMI RX CONTROLLER DRIVER
+M:	Shreeya Patel <shreeya.patel@collabora.com
+L:	linux-media@vger.kernel.org
+L:	kernel@collabora.com
+S:	Maintained
+F:	Documentation/devicetree/bindings/media/snps,dw-hdmi-rx.yaml
+F:	drivers/media/platform/synopsys/hdmirx/*
+
 SYNOPSYS DESIGNWARE I2C DRIVER
 M:	Jarkko Nikula <jarkko.nikula@linux.intel.com>
 R:	Andy Shevchenko <andriy.shevchenko@linux.intel.com>
-- 
2.39.2


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v2 4/6] arm64: dts: rockchip: Add device tree support for HDMI RX Controller

[Heiko Stübner]

Hi,


Am Dienstag, 5. März 2024, 13:36:46 CET schrieb Shreeya Patel:
> Add device tree support for Synopsys DesignWare HDMI RX
> Controller.
> 
> Reviewed-by: Dmitry Osipenko <dmitry.osipenko@collabora.com>
> Tested-by: Dmitry Osipenko <dmitry.osipenko@collabora.com>
> Co-developed-by: Dingxian Wen <shawn.wen@rock-chips.com>
> Signed-off-by: Dingxian Wen <shawn.wen@rock-chips.com>
> Signed-off-by: Shreeya Patel <shreeya.patel@collabora.com>
> ---
> Changes in v2 :-
>   - Fix some of the checkpatch errors and warnings
>   - Rename resets, vo1-grf and HPD
>   - Move hdmirx_cma node to the rk3588.dtsi file
> 
>  .../boot/dts/rockchip/rk3588-pinctrl.dtsi     | 41 ++++++++++++++
>  arch/arm64/boot/dts/rockchip/rk3588.dtsi      | 55 +++++++++++++++++++
>  2 files changed, 96 insertions(+)

> diff --git a/arch/arm64/boot/dts/rockchip/rk3588.dtsi b/arch/arm64/boot/dts/rockchip/rk3588.dtsi
> index 5519c1430cb7..8adb98b99701 100644
> --- a/arch/arm64/boot/dts/rockchip/rk3588.dtsi
> +++ b/arch/arm64/boot/dts/rockchip/rk3588.dtsi
> @@ -7,6 +7,24 @@
>  #include "rk3588-pinctrl.dtsi"
>  
>  / {
> +	reserved-memory {
> +		#address-cells = <2>;
> +		#size-cells = <2>;
> +		ranges;

add blank line here

> +		/*
> +		 * The 4k HDMI capture controller works only with 32bit
> +		 * phys addresses and doesn't support IOMMU. HDMI RX CMA
> +		 * must be reserved below 4GB.
> +		 */
> +		hdmirx_cma: hdmirx_cma {

phandles use "_", but node-names "-"

> +			compatible = "shared-dma-pool";
> +			alloc-ranges = <0x0 0x0 0x0 0xffffffff>;
> +			size = <0x0 (160 * 0x100000)>; /* 160MiB */

The comment above that node, could elaborate where the value of 160MB
originates from. I assume it is to hold n-times of 4K frames or whatever,
but it would be helpful for people to be able to read that.


> +			no-map;
> +			status = "disabled";
> +		};
> +	};
> +
>  	pcie30_phy_grf: syscon@fd5b8000 {
>  		compatible = "rockchip,rk3588-pcie3-phy-grf", "syscon";
>  		reg = <0x0 0xfd5b8000 0x0 0x10000>;
> @@ -85,6 +103,38 @@ i2s10_8ch: i2s@fde00000 {
>  		status = "disabled";
>  	};
>  
> +	hdmi_receiver: hdmi-receiver@fdee0000 {

Maybe rename the label to "hdmirx:" ... that way in a board enabling the
cma region, both nodes would stay close to each other?


> +		compatible = "rockchip,rk3588-hdmirx-ctrler", "snps,dw-hdmi-rx";
> +		reg = <0x0 0xfdee0000 0x0 0x6000>;
> +		power-domains = <&power RK3588_PD_VO1>;
> +		rockchip,grf = <&sys_grf>;
> +		rockchip,vo1-grf = <&vo1_grf>;
> +		interrupts = <GIC_SPI 177 IRQ_TYPE_LEVEL_HIGH 0>,
> +			     <GIC_SPI 436 IRQ_TYPE_LEVEL_HIGH 0>,
> +			     <GIC_SPI 179 IRQ_TYPE_LEVEL_HIGH 0>;
> +		interrupt-names = "cec", "hdmi", "dma";
> +		clocks = <&cru ACLK_HDMIRX>,
> +			 <&cru CLK_HDMIRX_AUD>,
> +			 <&cru CLK_CR_PARA>,
> +			 <&cru PCLK_HDMIRX>,
> +			 <&cru CLK_HDMIRX_REF>,
> +			 <&cru PCLK_S_HDMIRX>,
> +			 <&cru HCLK_VO1>;
> +		clock-names = "aclk",
> +			      "audio",
> +			      "cr_para",
> +			      "pclk",
> +			      "ref",
> +			      "hclk_s_hdmirx",
> +			      "hclk_vo1";

the driver uses of_reserved_mem_device_init(), so doesn't this node need
a "memory-region = <&hdmirx_cma>; or similar?


> +		resets = <&cru SRST_A_HDMIRX>, <&cru SRST_P_HDMIRX>,
> +			 <&cru SRST_HDMIRX_REF>, <&cru SRST_A_HDMIRX_BIU>;
> +		reset-names = "axi", "apb", "ref", "biu";
> +		pinctrl-0 = <&hdmim1_rx>;
> +		pinctrl-names = "default";
> +		status = "disabled";
> +	};
> +
>  	pcie3x4: pcie@fe150000 {
>  		compatible = "rockchip,rk3588-pcie", "rockchip,rk3568-pcie";
>  		#address-cells = <3>;
> @@ -339,3 +389,8 @@ pcie30phy: phy@fee80000 {
>  		status = "disabled";
>  	};
>  };
> +
> +&hdmirx_cma {
> +	status = "okay";
> +};

I'd assume a board that enables &hdmi_receiver would also enable hdmirx_cma
and not the soc dtsi for _all_ boards?


Thanks
Heiko



_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v2 1/6] dt-bindings: reset: Define reset id used for HDMI Receiver

[Rob Herring]

On Tue, 05 Mar 2024 18:06:43 +0530, Shreeya Patel wrote:
> Add reset id used for HDMI Receiver in RK3588 SoCs
> 
> Signed-off-by: Shreeya Patel <shreeya.patel@collabora.com>
> ---
> Changes in v2 :-
>   - Move the dt-binding include file changes in a separate patch
>   - Improve the subject and commit message description
> 
>  include/dt-bindings/reset/rockchip,rk3588-cru.h | 2 ++
>  1 file changed, 2 insertions(+)
> 

Acked-by: Rob Herring <robh@kernel.org>


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v2 3/6] dt-bindings: media: Document HDMI RX Controller

[Rob Herring]

On Tue, 05 Mar 2024 18:06:45 +0530, Shreeya Patel wrote:
> Document bindings for the Synopsys DesignWare HDMI RX Controller.
> 
> Reviewed-by: Dmitry Osipenko <dmitry.osipenko@collabora.com>
> Signed-off-by: Shreeya Patel <shreeya.patel@collabora.com>
> ---
> Changes in v2 :-
>   - Add a description for the hardware
>   - Rename resets, vo1 grf and HPD properties
>   - Add a proper description for grf and vo1-grf phandles
>   - Rename the HDMI Input node name to hdmi-receiver
>   - Improve the subject line
>   - Include gpio header file in example to fix dt_binding_check failure
> 
>  .../bindings/media/snps,dw-hdmi-rx.yaml       | 132 ++++++++++++++++++
>  1 file changed, 132 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/media/snps,dw-hdmi-rx.yaml
> 

Reviewed-by: Rob Herring <robh@kernel.org>


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v2 4/6] arm64: dts: rockchip: Add device tree support for HDMI RX Controller

[Shreeya Patel]

On Tuesday, March 05, 2024 19:41 IST, Heiko Stübner <heiko@sntech.de> wrote:

> Hi,
> 

Hi Heiko,

> 
> Am Dienstag, 5. März 2024, 13:36:46 CET schrieb Shreeya Patel:
> > Add device tree support for Synopsys DesignWare HDMI RX
> > Controller.
> > 
> > Reviewed-by: Dmitry Osipenko <dmitry.osipenko@collabora.com>
> > Tested-by: Dmitry Osipenko <dmitry.osipenko@collabora.com>
> > Co-developed-by: Dingxian Wen <shawn.wen@rock-chips.com>
> > Signed-off-by: Dingxian Wen <shawn.wen@rock-chips.com>
> > Signed-off-by: Shreeya Patel <shreeya.patel@collabora.com>
> > ---
> > Changes in v2 :-
> >   - Fix some of the checkpatch errors and warnings
> >   - Rename resets, vo1-grf and HPD
> >   - Move hdmirx_cma node to the rk3588.dtsi file
> > 
> >  .../boot/dts/rockchip/rk3588-pinctrl.dtsi     | 41 ++++++++++++++
> >  arch/arm64/boot/dts/rockchip/rk3588.dtsi      | 55 +++++++++++++++++++
> >  2 files changed, 96 insertions(+)
> 
> > diff --git a/arch/arm64/boot/dts/rockchip/rk3588.dtsi b/arch/arm64/boot/dts/rockchip/rk3588.dtsi
> > index 5519c1430cb7..8adb98b99701 100644
> > --- a/arch/arm64/boot/dts/rockchip/rk3588.dtsi
> > +++ b/arch/arm64/boot/dts/rockchip/rk3588.dtsi
> > @@ -7,6 +7,24 @@
> >  #include "rk3588-pinctrl.dtsi"
> >  
> >  / {
> > +	reserved-memory {
> > +		#address-cells = <2>;
> > +		#size-cells = <2>;
> > +		ranges;
> 
> add blank line here
> 
> > +		/*
> > +		 * The 4k HDMI capture controller works only with 32bit
> > +		 * phys addresses and doesn't support IOMMU. HDMI RX CMA
> > +		 * must be reserved below 4GB.
> > +		 */
> > +		hdmirx_cma: hdmirx_cma {
> 
> phandles use "_", but node-names "-"
> 
> > +			compatible = "shared-dma-pool";
> > +			alloc-ranges = <0x0 0x0 0x0 0xffffffff>;
> > +			size = <0x0 (160 * 0x100000)>; /* 160MiB */
> 
> The comment above that node, could elaborate where the value of 160MB
> originates from. I assume it is to hold n-times of 4K frames or whatever,
> but it would be helpful for people to be able to read that.
> 

right, we did the following calculation to come up with this value :-
3840 * 2160 * 4 (bytes/pix) * 2 (frames/buffer) / 1000 / 1000 = 66M
and then we do the 2x times of this value to be on the safer side
and support all practical use-cases.

I'll add some more details to the comment in v3.

> 
> > +			no-map;
> > +			status = "disabled";
> > +		};
> > +	};
> > +
> >  	pcie30_phy_grf: syscon@fd5b8000 {
> >  		compatible = "rockchip,rk3588-pcie3-phy-grf", "syscon";
> >  		reg = <0x0 0xfd5b8000 0x0 0x10000>;
> > @@ -85,6 +103,38 @@ i2s10_8ch: i2s@fde00000 {
> >  		status = "disabled";
> >  	};
> >  
> > +	hdmi_receiver: hdmi-receiver@fdee0000 {
> 
> Maybe rename the label to "hdmirx:" ... that way in a board enabling the
> cma region, both nodes would stay close to each other?
> 

Umm we already have receiver in the name so I am not sure if adding rx will be
a good idea. I was trying to keep it consistent with the names used in other device tree files.
In case you still feel otherwise then do let me know, I'll make the change.

> 
> > +		compatible = "rockchip,rk3588-hdmirx-ctrler", "snps,dw-hdmi-rx";
> > +		reg = <0x0 0xfdee0000 0x0 0x6000>;
> > +		power-domains = <&power RK3588_PD_VO1>;
> > +		rockchip,grf = <&sys_grf>;
> > +		rockchip,vo1-grf = <&vo1_grf>;
> > +		interrupts = <GIC_SPI 177 IRQ_TYPE_LEVEL_HIGH 0>,
> > +			     <GIC_SPI 436 IRQ_TYPE_LEVEL_HIGH 0>,
> > +			     <GIC_SPI 179 IRQ_TYPE_LEVEL_HIGH 0>;
> > +		interrupt-names = "cec", "hdmi", "dma";
> > +		clocks = <&cru ACLK_HDMIRX>,
> > +			 <&cru CLK_HDMIRX_AUD>,
> > +			 <&cru CLK_CR_PARA>,
> > +			 <&cru PCLK_HDMIRX>,
> > +			 <&cru CLK_HDMIRX_REF>,
> > +			 <&cru PCLK_S_HDMIRX>,
> > +			 <&cru HCLK_VO1>;
> > +		clock-names = "aclk",
> > +			      "audio",
> > +			      "cr_para",
> > +			      "pclk",
> > +			      "ref",
> > +			      "hclk_s_hdmirx",
> > +			      "hclk_vo1";
> 
> the driver uses of_reserved_mem_device_init(), so doesn't this node need
> a "memory-region = <&hdmirx_cma>; or similar?
> 

yes, we should have the memory-region property here. My bad, I'll correct this in v3.

> 
> > +		resets = <&cru SRST_A_HDMIRX>, <&cru SRST_P_HDMIRX>,
> > +			 <&cru SRST_HDMIRX_REF>, <&cru SRST_A_HDMIRX_BIU>;
> > +		reset-names = "axi", "apb", "ref", "biu";
> > +		pinctrl-0 = <&hdmim1_rx>;
> > +		pinctrl-names = "default";
> > +		status = "disabled";
> > +	};
> > +
> >  	pcie3x4: pcie@fe150000 {
> >  		compatible = "rockchip,rk3588-pcie", "rockchip,rk3568-pcie";
> >  		#address-cells = <3>;
> > @@ -339,3 +389,8 @@ pcie30phy: phy@fee80000 {
> >  		status = "disabled";
> >  	};
> >  };
> > +
> > +&hdmirx_cma {
> > +	status = "okay";
> > +};
> 
> I'd assume a board that enables &hdmi_receiver would also enable hdmirx_cma
> and not the soc dtsi for _all_ boards?
> 

Actually this node should be in the rock-5b.dts file instead of here.
v1 had it correct but I made a mistake in v2 :(
Thanks for pointing this out, I'll fix this and send a v3 soon.


Thanks,
Shreeya Patel

> 
> Thanks
> Heiko
> 
> 
> _______________________________________________
> Kernel mailing list -- kernel@mailman.collabora.com
> To unsubscribe send an email to kernel-leave@mailman.collabora.com
> This list is managed by https://mailman.collabora.com


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v2 4/6] arm64: dts: rockchip: Add device tree support for HDMI RX Controller

[Heiko Stübner]

Hi again :-)

Am Dienstag, 5. März 2024, 20:05:02 CET schrieb Shreeya Patel:
> On Tuesday, March 05, 2024 19:41 IST, Heiko Stübner <heiko@sntech.de> wrote:
> > Am Dienstag, 5. März 2024, 13:36:46 CET schrieb Shreeya Patel:
> > > Add device tree support for Synopsys DesignWare HDMI RX
> > > Controller.
> > > 
> > > Reviewed-by: Dmitry Osipenko <dmitry.osipenko@collabora.com>
> > > Tested-by: Dmitry Osipenko <dmitry.osipenko@collabora.com>
> > > Co-developed-by: Dingxian Wen <shawn.wen@rock-chips.com>
> > > Signed-off-by: Dingxian Wen <shawn.wen@rock-chips.com>
> > > Signed-off-by: Shreeya Patel <shreeya.patel@collabora.com>
> > > ---
> > > Changes in v2 :-
> > >   - Fix some of the checkpatch errors and warnings
> > >   - Rename resets, vo1-grf and HPD
> > >   - Move hdmirx_cma node to the rk3588.dtsi file
> > > 
> > >  .../boot/dts/rockchip/rk3588-pinctrl.dtsi     | 41 ++++++++++++++
> > >  arch/arm64/boot/dts/rockchip/rk3588.dtsi      | 55 +++++++++++++++++++
> > >  2 files changed, 96 insertions(+)
> > 
> > > diff --git a/arch/arm64/boot/dts/rockchip/rk3588.dtsi b/arch/arm64/boot/dts/rockchip/rk3588.dtsi
> > > index 5519c1430cb7..8adb98b99701 100644
> > > --- a/arch/arm64/boot/dts/rockchip/rk3588.dtsi
> > > +++ b/arch/arm64/boot/dts/rockchip/rk3588.dtsi
> > > @@ -7,6 +7,24 @@
> > >  #include "rk3588-pinctrl.dtsi"
> > >  
> > >  / {
> > > +	reserved-memory {
> > > +		#address-cells = <2>;
> > > +		#size-cells = <2>;
> > > +		ranges;
> > 
> > add blank line here
> > 
> > > +		/*
> > > +		 * The 4k HDMI capture controller works only with 32bit
> > > +		 * phys addresses and doesn't support IOMMU. HDMI RX CMA
> > > +		 * must be reserved below 4GB.
> > > +		 */
> > > +		hdmirx_cma: hdmirx_cma {
> > 
> > phandles use "_", but node-names "-"
> > 
> > > +			compatible = "shared-dma-pool";
> > > +			alloc-ranges = <0x0 0x0 0x0 0xffffffff>;
> > > +			size = <0x0 (160 * 0x100000)>; /* 160MiB */
> > 
> > The comment above that node, could elaborate where the value of 160MB
> > originates from. I assume it is to hold n-times of 4K frames or whatever,
> > but it would be helpful for people to be able to read that.
> > 
> 
> right, we did the following calculation to come up with this value :-
> 3840 * 2160 * 4 (bytes/pix) * 2 (frames/buffer) / 1000 / 1000 = 66M
> and then we do the 2x times of this value to be on the safer side
> and support all practical use-cases.
> 
> I'll add some more details to the comment in v3.

thanks, that will be helpful for me and everybody reading the dts later on

> 
> > 
> > > +			no-map;
> > > +			status = "disabled";
> > > +		};
> > > +	};
> > > +
> > >  	pcie30_phy_grf: syscon@fd5b8000 {
> > >  		compatible = "rockchip,rk3588-pcie3-phy-grf", "syscon";
> > >  		reg = <0x0 0xfd5b8000 0x0 0x10000>;
> > > @@ -85,6 +103,38 @@ i2s10_8ch: i2s@fde00000 {
> > >  		status = "disabled";
> > >  	};
> > >  
> > > +	hdmi_receiver: hdmi-receiver@fdee0000 {
> > 
> > Maybe rename the label to "hdmirx:" ... that way in a board enabling the
> > cma region, both nodes would stay close to each other?
> > 
> 
> Umm we already have receiver in the name so I am not sure if adding rx will be
> a good idea. I was trying to keep it consistent with the names used in other device tree files.
> In case you still feel otherwise then do let me know, I'll make the change.

I'm somewhat partial to the actual name, I was more getting at similar
names to keep things together.

General sorting rules are that &foo phandles are sorted alphabetically
in board devicetrees.

So having

&hdmirx {
	status = "okay";
};

&hdmirx_cma {
	status = "okay";
};

in the board dt, makes them stay together automatically ;-)

So if it's hdmirx + hdmirx_cma or hdmi_receiver + hdmi_receiver_cma
doesn't matter that much, just that they share a common basename.


I really want to stay away from allowing special rules for things as much
as possible, because that becomes a neverending story, so it's
alphabetical sorting.

But nothing prevents us from naming phandles in an intelligent way ;-) .


Thanks
Heiko

> > > +		compatible = "rockchip,rk3588-hdmirx-ctrler", "snps,dw-hdmi-rx";
> > > +		reg = <0x0 0xfdee0000 0x0 0x6000>;
> > > +		power-domains = <&power RK3588_PD_VO1>;
> > > +		rockchip,grf = <&sys_grf>;
> > > +		rockchip,vo1-grf = <&vo1_grf>;
> > > +		interrupts = <GIC_SPI 177 IRQ_TYPE_LEVEL_HIGH 0>,
> > > +			     <GIC_SPI 436 IRQ_TYPE_LEVEL_HIGH 0>,
> > > +			     <GIC_SPI 179 IRQ_TYPE_LEVEL_HIGH 0>;
> > > +		interrupt-names = "cec", "hdmi", "dma";
> > > +		clocks = <&cru ACLK_HDMIRX>,
> > > +			 <&cru CLK_HDMIRX_AUD>,
> > > +			 <&cru CLK_CR_PARA>,
> > > +			 <&cru PCLK_HDMIRX>,
> > > +			 <&cru CLK_HDMIRX_REF>,
> > > +			 <&cru PCLK_S_HDMIRX>,
> > > +			 <&cru HCLK_VO1>;
> > > +		clock-names = "aclk",
> > > +			      "audio",
> > > +			      "cr_para",
> > > +			      "pclk",
> > > +			      "ref",
> > > +			      "hclk_s_hdmirx",
> > > +			      "hclk_vo1";
> > 
> > the driver uses of_reserved_mem_device_init(), so doesn't this node need
> > a "memory-region = <&hdmirx_cma>; or similar?
> > 
> 
> yes, we should have the memory-region property here. My bad, I'll correct this in v3.
> 
> > 
> > > +		resets = <&cru SRST_A_HDMIRX>, <&cru SRST_P_HDMIRX>,
> > > +			 <&cru SRST_HDMIRX_REF>, <&cru SRST_A_HDMIRX_BIU>;
> > > +		reset-names = "axi", "apb", "ref", "biu";
> > > +		pinctrl-0 = <&hdmim1_rx>;
> > > +		pinctrl-names = "default";
> > > +		status = "disabled";
> > > +	};
> > > +
> > >  	pcie3x4: pcie@fe150000 {
> > >  		compatible = "rockchip,rk3588-pcie", "rockchip,rk3568-pcie";
> > >  		#address-cells = <3>;
> > > @@ -339,3 +389,8 @@ pcie30phy: phy@fee80000 {
> > >  		status = "disabled";
> > >  	};
> > >  };
> > > +
> > > +&hdmirx_cma {
> > > +	status = "okay";
> > > +};
> > 
> > I'd assume a board that enables &hdmi_receiver would also enable hdmirx_cma
> > and not the soc dtsi for _all_ boards?
> > 
> 
> Actually this node should be in the rock-5b.dts file instead of here.
> v1 had it correct but I made a mistake in v2 :(
> Thanks for pointing this out, I'll fix this and send a v3 soon.





_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v2 4/6] arm64: dts: rockchip: Add device tree support for HDMI RX Controller

[Shreeya Patel]

On Wednesday, March 06, 2024 01:50 IST, Heiko Stübner <heiko@sntech.de> wrote:

> Hi again :-)
> 
> Am Dienstag, 5. März 2024, 20:05:02 CET schrieb Shreeya Patel:
> > On Tuesday, March 05, 2024 19:41 IST, Heiko Stübner <heiko@sntech.de> wrote:
> > > Am Dienstag, 5. März 2024, 13:36:46 CET schrieb Shreeya Patel:
> > > > Add device tree support for Synopsys DesignWare HDMI RX
> > > > Controller.
> > > > 
> > > > Reviewed-by: Dmitry Osipenko <dmitry.osipenko@collabora.com>
> > > > Tested-by: Dmitry Osipenko <dmitry.osipenko@collabora.com>
> > > > Co-developed-by: Dingxian Wen <shawn.wen@rock-chips.com>
> > > > Signed-off-by: Dingxian Wen <shawn.wen@rock-chips.com>
> > > > Signed-off-by: Shreeya Patel <shreeya.patel@collabora.com>
> > > > ---
> > > > Changes in v2 :-
> > > >   - Fix some of the checkpatch errors and warnings
> > > >   - Rename resets, vo1-grf and HPD
> > > >   - Move hdmirx_cma node to the rk3588.dtsi file
> > > > 
> > > >  .../boot/dts/rockchip/rk3588-pinctrl.dtsi     | 41 ++++++++++++++
> > > >  arch/arm64/boot/dts/rockchip/rk3588.dtsi      | 55 +++++++++++++++++++
> > > >  2 files changed, 96 insertions(+)
> > > 
> > > > diff --git a/arch/arm64/boot/dts/rockchip/rk3588.dtsi b/arch/arm64/boot/dts/rockchip/rk3588.dtsi
> > > > index 5519c1430cb7..8adb98b99701 100644
> > > > --- a/arch/arm64/boot/dts/rockchip/rk3588.dtsi
> > > > +++ b/arch/arm64/boot/dts/rockchip/rk3588.dtsi
> > > > @@ -7,6 +7,24 @@
> > > >  #include "rk3588-pinctrl.dtsi"
> > > >  
> > > >  / {
> > > > +	reserved-memory {
> > > > +		#address-cells = <2>;
> > > > +		#size-cells = <2>;
> > > > +		ranges;
> > > 
> > > add blank line here
> > > 
> > > > +		/*
> > > > +		 * The 4k HDMI capture controller works only with 32bit
> > > > +		 * phys addresses and doesn't support IOMMU. HDMI RX CMA
> > > > +		 * must be reserved below 4GB.
> > > > +		 */
> > > > +		hdmirx_cma: hdmirx_cma {
> > > 
> > > phandles use "_", but node-names "-"
> > > 
> > > > +			compatible = "shared-dma-pool";
> > > > +			alloc-ranges = <0x0 0x0 0x0 0xffffffff>;
> > > > +			size = <0x0 (160 * 0x100000)>; /* 160MiB */
> > > 
> > > The comment above that node, could elaborate where the value of 160MB
> > > originates from. I assume it is to hold n-times of 4K frames or whatever,
> > > but it would be helpful for people to be able to read that.
> > > 
> > 
> > right, we did the following calculation to come up with this value :-
> > 3840 * 2160 * 4 (bytes/pix) * 2 (frames/buffer) / 1000 / 1000 = 66M
> > and then we do the 2x times of this value to be on the safer side
> > and support all practical use-cases.
> > 
> > I'll add some more details to the comment in v3.
> 
> thanks, that will be helpful for me and everybody reading the dts later on
> 
> > 
> > > 
> > > > +			no-map;
> > > > +			status = "disabled";
> > > > +		};
> > > > +	};
> > > > +
> > > >  	pcie30_phy_grf: syscon@fd5b8000 {
> > > >  		compatible = "rockchip,rk3588-pcie3-phy-grf", "syscon";
> > > >  		reg = <0x0 0xfd5b8000 0x0 0x10000>;
> > > > @@ -85,6 +103,38 @@ i2s10_8ch: i2s@fde00000 {
> > > >  		status = "disabled";
> > > >  	};
> > > >  
> > > > +	hdmi_receiver: hdmi-receiver@fdee0000 {
> > > 
> > > Maybe rename the label to "hdmirx:" ... that way in a board enabling the
> > > cma region, both nodes would stay close to each other?
> > > 
> > 
> > Umm we already have receiver in the name so I am not sure if adding rx will be
> > a good idea. I was trying to keep it consistent with the names used in other device tree files.
> > In case you still feel otherwise then do let me know, I'll make the change.
> 
> I'm somewhat partial to the actual name, I was more getting at similar
> names to keep things together.
> 
> General sorting rules are that &foo phandles are sorted alphabetically
> in board devicetrees.
> 
> So having
> 
> &hdmirx {
> 	status = "okay";
> };
> 
> &hdmirx_cma {
> 	status = "okay";
> };
> 
> in the board dt, makes them stay together automatically ;-)
> 
> So if it's hdmirx + hdmirx_cma or hdmi_receiver + hdmi_receiver_cma
> doesn't matter that much, just that they share a common basename.
> 
> 
> I really want to stay away from allowing special rules for things as much
> as possible, because that becomes a neverending story, so it's
> alphabetical sorting.
> 
> But nothing prevents us from naming phandles in an intelligent way ;-) .
> 

Makes sense to me, I'll use hdmi_receiver + hdmi_receiver_cma combination
to keep it consistent.

Thanks,
Shreeya Patel

> 
> Thanks
> Heiko
>


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel