[PATCH v3 0/4] Add support for iMX8MQ Display Controller Subsystem

[PATCH v3 0/4] Add support for iMX8MQ Display Controller Subsystem

[Laurentiu Palcu]

Hi,

This patchset adds initial DCSS support for iMX8MQ chip. Initial support
includes only graphics plane support (no video planes), no HDR10 capabilities,
no graphics decompression (only linear, tiled and super-tiled buffers allowed).

Support for the rest of the features will be added incrementally, in subsequent
patches.

The patchset was tested with both HDP driver (not yet upstreamed) and MIPI-DSI
(drivers already on the dri-devel ML).

Thanks,
Laurentiu

Changes in v3:
 * rebased to latest linux-next and made it compile as drmP.h was
   removed;
 * removed the patch adding the VIDEO2_PLL clock. It's already applied;
 * removed an unnecessary 50ms sleep in the dcss_dtg_sync_set();
 * fixed a a spurious hang reported by Lukas Hartmann and encountered
   by me several times;
 * mask DPR and DTG interrupts by default, as they may come enabled from
   U-boot;

Changes in v2:
 * Removed '0x' in node's unit-address both in DT and yaml;
 * Made the address region size lowercase, to be consistent;
 * Removed some left-over references to P010;
 * Added a Kconfig dependency of DRM && ARCH_MXC. This will also silence compilation
   issues reported by kbuild for other architectures;


Laurentiu Palcu (4):
  drm/imx: compile imx directory by default
  drm/imx: Add initial support for DCSS on iMX8MQ
  dt-bindings: display: imx: add bindings for DCSS
  arm64: dts: imx8mq: add DCSS node

 .../bindings/display/imx/nxp,imx8mq-dcss.yaml      |  86 +++
 arch/arm64/boot/dts/freescale/imx8mq.dtsi          |  25 +
 drivers/gpu/drm/Makefile                           |   2 +-
 drivers/gpu/drm/imx/Kconfig                        |   2 +
 drivers/gpu/drm/imx/Makefile                       |   1 +
 drivers/gpu/drm/imx/dcss/Kconfig                   |   8 +
 drivers/gpu/drm/imx/dcss/Makefile                  |   6 +
 drivers/gpu/drm/imx/dcss/dcss-blkctl.c             |  75 ++
 drivers/gpu/drm/imx/dcss/dcss-crtc.c               | 224 ++++++
 drivers/gpu/drm/imx/dcss/dcss-ctxld.c              | 447 +++++++++++
 drivers/gpu/drm/imx/dcss/dcss-dev.c                | 286 +++++++
 drivers/gpu/drm/imx/dcss/dcss-dev.h                | 195 +++++
 drivers/gpu/drm/imx/dcss/dcss-dpr.c                | 550 ++++++++++++++
 drivers/gpu/drm/imx/dcss/dcss-drv.c                | 181 +++++
 drivers/gpu/drm/imx/dcss/dcss-dtg.c                | 442 +++++++++++
 drivers/gpu/drm/imx/dcss/dcss-kms.c                | 322 ++++++++
 drivers/gpu/drm/imx/dcss/dcss-kms.h                |  52 ++
 drivers/gpu/drm/imx/dcss/dcss-plane.c              | 418 +++++++++++
 drivers/gpu/drm/imx/dcss/dcss-scaler.c             | 826 +++++++++++++++++++++
 drivers/gpu/drm/imx/dcss/dcss-ss.c                 | 179 +++++
 20 files changed, 4326 insertions(+), 1 deletion(-)
 create mode 100644 Documentation/devicetree/bindings/display/imx/nxp,imx8mq-dcss.yaml
 create mode 100644 drivers/gpu/drm/imx/dcss/Kconfig
 create mode 100644 drivers/gpu/drm/imx/dcss/Makefile
 create mode 100644 drivers/gpu/drm/imx/dcss/dcss-blkctl.c
 create mode 100644 drivers/gpu/drm/imx/dcss/dcss-crtc.c
 create mode 100644 drivers/gpu/drm/imx/dcss/dcss-ctxld.c
 create mode 100644 drivers/gpu/drm/imx/dcss/dcss-dev.c
 create mode 100644 drivers/gpu/drm/imx/dcss/dcss-dev.h
 create mode 100644 drivers/gpu/drm/imx/dcss/dcss-dpr.c
 create mode 100644 drivers/gpu/drm/imx/dcss/dcss-drv.c
 create mode 100644 drivers/gpu/drm/imx/dcss/dcss-dtg.c
 create mode 100644 drivers/gpu/drm/imx/dcss/dcss-kms.c
 create mode 100644 drivers/gpu/drm/imx/dcss/dcss-kms.h
 create mode 100644 drivers/gpu/drm/imx/dcss/dcss-plane.c
 create mode 100644 drivers/gpu/drm/imx/dcss/dcss-scaler.c
 create mode 100644 drivers/gpu/drm/imx/dcss/dcss-ss.c

-- 
2.7.4


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

[PATCH v3 3/4] dt-bindings: display: imx: add bindings for DCSS

[Laurentiu Palcu]

Add bindings for iMX8MQ Display Controller Subsystem.

Signed-off-by: Laurentiu Palcu <laurentiu.palcu@nxp.com>
Reviewed-by: Rob Herring <robh@kernel.org>
---
 .../bindings/display/imx/nxp,imx8mq-dcss.yaml      | 86 ++++++++++++++++++++++
 1 file changed, 86 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/display/imx/nxp,imx8mq-dcss.yaml

diff --git a/Documentation/devicetree/bindings/display/imx/nxp,imx8mq-dcss.yaml b/Documentation/devicetree/bindings/display/imx/nxp,imx8mq-dcss.yaml
new file mode 100644
index 00000000..efd2494
--- /dev/null
+++ b/Documentation/devicetree/bindings/display/imx/nxp,imx8mq-dcss.yaml
@@ -0,0 +1,86 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+# Copyright 2019 NXP
+%YAML 1.2
+---
+$id: "http://devicetree.org/schemas/display/imx/nxp,imx8mq-dcss.yaml#"
+$schema: "http://devicetree.org/meta-schemas/core.yaml#"
+
+title: iMX8MQ Display Controller Subsystem (DCSS)
+
+maintainers:
+  - Laurentiu Palcu <laurentiu.palcu@nxp.com>
+
+description:
+
+  The DCSS (display controller sub system) is used to source up to three
+  display buffers, compose them, and drive a display using HDMI 2.0a(with HDCP
+  2.2) or MIPI-DSI. The DCSS is intended to support up to 4kp60 displays. HDR10
+  image processing capabilities are included to provide a solution capable of
+  driving next generation high dynamic range displays.
+
+properties:
+  compatible:
+    const: nxp,imx8mq-dcss
+
+  reg:
+    maxItems: 2
+
+  interrupts:
+    maxItems: 3
+    items:
+      - description: Context loader completion and error interrupt
+      - description: DTG interrupt used to signal context loader trigger time
+      - description: DTG interrupt for Vblank
+
+  interrupt-names:
+    maxItems: 3
+    items:
+      - const: ctx_ld
+      - const: ctxld_kick
+      - const: vblank
+
+  clocks:
+    maxItems: 5
+    items:
+      - description: Display APB clock for all peripheral PIO access interfaces
+      - description: Display AXI clock needed by DPR, Scaler, RTRAM_CTRL
+      - description: RTRAM clock
+      - description: Pixel clock, can be driver either by HDMI phy clock or MIPI
+      - description: DTRC clock, needed by video decompressor
+
+  clock-names:
+    items:
+      - const: apb
+      - const: axi
+      - const: rtrm
+      - const: pix
+      - const: dtrc
+
+  port@0:
+    type: object
+    description: A port node pointing to a hdmi_in or mipi_in port node.
+
+examples:
+  - |
+    dcss: display-controller@32e00000 {
+        #address-cells = <1>;
+        #size-cells = <0>;
+        compatible = "nxp,imx8mq-dcss";
+        reg = <0x32e00000 0x2d000>, <0x32e2f000 0x1000>;
+        interrupts = <6>, <8>, <9>;
+        interrupt-names = "ctx_ld", "ctxld_kick", "vblank";
+        interrupt-parent = <&irqsteer>;
+        clocks = <&clk 248>, <&clk 247>, <&clk 249>,
+                 <&clk 254>,<&clk 122>;
+        clock-names = "apb", "axi", "rtrm", "pix", "dtrc";
+        assigned-clocks = <&clk 107>, <&clk 109>, <&clk 266>;
+        assigned-clock-parents = <&clk 78>, <&clk 78>, <&clk 3>;
+        assigned-clock-rates = <800000000>,
+                               <400000000>;
+        port@0 {
+            dcss_out: endpoint {
+                remote-endpoint = <&hdmi_in>;
+            };
+        };
+    };
+
-- 
2.7.4


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

[PATCH v3 4/4] arm64: dts: imx8mq: add DCSS node

[Laurentiu Palcu]

This patch adds the node for iMX8MQ Display Controller Subsystem.

Signed-off-by: Laurentiu Palcu <laurentiu.palcu@nxp.com>
---
 arch/arm64/boot/dts/freescale/imx8mq.dtsi | 25 +++++++++++++++++++++++++
 1 file changed, 25 insertions(+)

diff --git a/arch/arm64/boot/dts/freescale/imx8mq.dtsi b/arch/arm64/boot/dts/freescale/imx8mq.dtsi
index f6e840c..da7e485 100644
--- a/arch/arm64/boot/dts/freescale/imx8mq.dtsi
+++ b/arch/arm64/boot/dts/freescale/imx8mq.dtsi
@@ -981,6 +981,31 @@
 				interrupt-controller;
 				#interrupt-cells = <1>;
 			};
+
+			dcss: display-controller@32e00000 {
+				#address-cells = <1>;
+				#size-cells = <0>;
+				compatible = "nxp,imx8mq-dcss";
+				reg = <0x32e00000 0x2d000>, <0x32e2f000 0x1000>;
+				interrupts = <6>, <8>, <9>;
+				interrupt-names = "ctx_ld", "ctxld_kick", "vblank";
+				interrupt-parent = <&irqsteer>;
+				clocks = <&clk IMX8MQ_CLK_DISP_APB_ROOT>,
+					 <&clk IMX8MQ_CLK_DISP_AXI_ROOT>,
+					 <&clk IMX8MQ_CLK_DISP_RTRM_ROOT>,
+					 <&clk IMX8MQ_VIDEO2_PLL_OUT>,
+					 <&clk IMX8MQ_CLK_DISP_DTRC>;
+				clock-names = "apb", "axi", "rtrm", "pix", "dtrc";
+				assigned-clocks = <&clk IMX8MQ_CLK_DISP_AXI>,
+						  <&clk IMX8MQ_CLK_DISP_RTRM>,
+						  <&clk IMX8MQ_VIDEO2_PLL1_REF_SEL>;
+				assigned-clock-parents = <&clk IMX8MQ_SYS1_PLL_800M>,
+							 <&clk IMX8MQ_SYS1_PLL_800M>,
+							 <&clk IMX8MQ_CLK_27M>;
+				assigned-clock-rates = <800000000>,
+							   <400000000>;
+				status = "disabled";
+			};
 		};
 
 		gpu: gpu@38000000 {
-- 
2.7.4


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

Re: [PATCH v3 0/4] Add support for iMX8MQ Display Controller Subsystem

[Guido Günther]

Hi Laurentiu,
On Fri, Dec 06, 2019 at 11:52:37AM +0200, Laurentiu Palcu wrote:
> Hi,
> 
> This patchset adds initial DCSS support for iMX8MQ chip. Initial support
> includes only graphics plane support (no video planes), no HDR10 capabilities,
> no graphics decompression (only linear, tiled and super-tiled buffers allowed).
> 
> Support for the rest of the features will be added incrementally, in subsequent
> patches.
> 
> The patchset was tested with both HDP driver (not yet upstreamed) and MIPI-DSI
> (drivers already on the dri-devel ML).

I've been testing this with the HDP parts pulled out of NXPs vendor tree
for a while so

Tested-by: Guido Günther <agx@sigxcpu.org>

Cheers,
 -- Guido

> 
> Thanks,
> Laurentiu
> 
> Changes in v3:
>  * rebased to latest linux-next and made it compile as drmP.h was
>    removed;
>  * removed the patch adding the VIDEO2_PLL clock. It's already applied;
>  * removed an unnecessary 50ms sleep in the dcss_dtg_sync_set();
>  * fixed a a spurious hang reported by Lukas Hartmann and encountered
>    by me several times;
>  * mask DPR and DTG interrupts by default, as they may come enabled from
>    U-boot;
> 
> Changes in v2:
>  * Removed '0x' in node's unit-address both in DT and yaml;
>  * Made the address region size lowercase, to be consistent;
>  * Removed some left-over references to P010;
>  * Added a Kconfig dependency of DRM && ARCH_MXC. This will also silence compilation
>    issues reported by kbuild for other architectures;
> 
> 
> Laurentiu Palcu (4):
>   drm/imx: compile imx directory by default
>   drm/imx: Add initial support for DCSS on iMX8MQ
>   dt-bindings: display: imx: add bindings for DCSS
>   arm64: dts: imx8mq: add DCSS node
> 
>  .../bindings/display/imx/nxp,imx8mq-dcss.yaml      |  86 +++
>  arch/arm64/boot/dts/freescale/imx8mq.dtsi          |  25 +
>  drivers/gpu/drm/Makefile                           |   2 +-
>  drivers/gpu/drm/imx/Kconfig                        |   2 +
>  drivers/gpu/drm/imx/Makefile                       |   1 +
>  drivers/gpu/drm/imx/dcss/Kconfig                   |   8 +
>  drivers/gpu/drm/imx/dcss/Makefile                  |   6 +
>  drivers/gpu/drm/imx/dcss/dcss-blkctl.c             |  75 ++
>  drivers/gpu/drm/imx/dcss/dcss-crtc.c               | 224 ++++++
>  drivers/gpu/drm/imx/dcss/dcss-ctxld.c              | 447 +++++++++++
>  drivers/gpu/drm/imx/dcss/dcss-dev.c                | 286 +++++++
>  drivers/gpu/drm/imx/dcss/dcss-dev.h                | 195 +++++
>  drivers/gpu/drm/imx/dcss/dcss-dpr.c                | 550 ++++++++++++++
>  drivers/gpu/drm/imx/dcss/dcss-drv.c                | 181 +++++
>  drivers/gpu/drm/imx/dcss/dcss-dtg.c                | 442 +++++++++++
>  drivers/gpu/drm/imx/dcss/dcss-kms.c                | 322 ++++++++
>  drivers/gpu/drm/imx/dcss/dcss-kms.h                |  52 ++
>  drivers/gpu/drm/imx/dcss/dcss-plane.c              | 418 +++++++++++
>  drivers/gpu/drm/imx/dcss/dcss-scaler.c             | 826 +++++++++++++++++++++
>  drivers/gpu/drm/imx/dcss/dcss-ss.c                 | 179 +++++
>  20 files changed, 4326 insertions(+), 1 deletion(-)
>  create mode 100644 Documentation/devicetree/bindings/display/imx/nxp,imx8mq-dcss.yaml
>  create mode 100644 drivers/gpu/drm/imx/dcss/Kconfig
>  create mode 100644 drivers/gpu/drm/imx/dcss/Makefile
>  create mode 100644 drivers/gpu/drm/imx/dcss/dcss-blkctl.c
>  create mode 100644 drivers/gpu/drm/imx/dcss/dcss-crtc.c
>  create mode 100644 drivers/gpu/drm/imx/dcss/dcss-ctxld.c
>  create mode 100644 drivers/gpu/drm/imx/dcss/dcss-dev.c
>  create mode 100644 drivers/gpu/drm/imx/dcss/dcss-dev.h
>  create mode 100644 drivers/gpu/drm/imx/dcss/dcss-dpr.c
>  create mode 100644 drivers/gpu/drm/imx/dcss/dcss-drv.c
>  create mode 100644 drivers/gpu/drm/imx/dcss/dcss-dtg.c
>  create mode 100644 drivers/gpu/drm/imx/dcss/dcss-kms.c
>  create mode 100644 drivers/gpu/drm/imx/dcss/dcss-kms.h
>  create mode 100644 drivers/gpu/drm/imx/dcss/dcss-plane.c
>  create mode 100644 drivers/gpu/drm/imx/dcss/dcss-scaler.c
>  create mode 100644 drivers/gpu/drm/imx/dcss/dcss-ss.c
> 
> -- 
> 2.7.4
> 

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

Re: [PATCH v3 3/4] dt-bindings: display: imx: add bindings for DCSS

[Lucas Stach]

On Fr, 2019-12-06 at 11:52 +0200, Laurentiu Palcu wrote:
> Add bindings for iMX8MQ Display Controller Subsystem.
> 
> Signed-off-by: Laurentiu Palcu <laurentiu.palcu@nxp.com>
> Reviewed-by: Rob Herring <robh@kernel.org>
> ---
>  .../bindings/display/imx/nxp,imx8mq-dcss.yaml      | 86 ++++++++++++++++++++++
>  1 file changed, 86 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/display/imx/nxp,imx8mq-dcss.yaml
> 
> diff --git a/Documentation/devicetree/bindings/display/imx/nxp,imx8mq-dcss.yaml b/Documentation/devicetree/bindings/display/imx/nxp,imx8mq-dcss.yaml
> new file mode 100644
> index 00000000..efd2494
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/display/imx/nxp,imx8mq-dcss.yaml
> @@ -0,0 +1,86 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +# Copyright 2019 NXP
> +%YAML 1.2
> +---
> +$id: "http://devicetree.org/schemas/display/imx/nxp,imx8mq-dcss.yaml#"
> +$schema: "http://devicetree.org/meta-schemas/core.yaml#"
> +
> +title: iMX8MQ Display Controller Subsystem (DCSS)
> +
> +maintainers:
> +  - Laurentiu Palcu <laurentiu.palcu@nxp.com>
> +
> +description:
> +
> +  The DCSS (display controller sub system) is used to source up to three
> +  display buffers, compose them, and drive a display using HDMI 2.0a(with HDCP
> +  2.2) or MIPI-DSI.

HDMI 2.0a and MIPI_DSI are not really properties of the DCSS, but
rather the connected bridges. Maybe just drop them here?

>  The DCSS is intended to support up to 4kp60 displays. HDR10
> +  image processing capabilities are included to provide a solution capable of
> +  driving next generation high dynamic range displays.
> +
> +properties:
> +  compatible:
> +    const: nxp,imx8mq-dcss
> +
> +  reg:
> +    maxItems: 2
> +
> +  interrupts:
> +    maxItems: 3
> +    items:
> +      - description: Context loader completion and error interrupt
> +      - description: DTG interrupt used to signal context loader trigger time
> +      - description: DTG interrupt for Vblank
> +
> +  interrupt-names:
> +    maxItems: 3
> +    items:
> +      - const: ctx_ld

Can we make this just "ctxld" for a bit more consistency with the name
below?

> +      - const: ctxld_kick
> +      - const: vblank
> +
> +  clocks:
> +    maxItems: 5
> +    items:
> +      - description: Display APB clock for all peripheral PIO access interfaces
> +      - description: Display AXI clock needed by DPR, Scaler, RTRAM_CTRL
> +      - description: RTRAM clock
> +      - description: Pixel clock, can be driver either by HDMI phy clock or MIPI
> +      - description: DTRC clock, needed by video decompressor
> +
> +  clock-names:
> +    items:
> +      - const: apb
> +      - const: axi
> +      - const: rtrm
> +      - const: pix
> +      - const: dtrc
> +
> +  port@0:
> +    type: object
> +    description: A port node pointing to a hdmi_in or mipi_in port node.

"A port node pointing to the input port of a HDMI/DP or MIPI display
bridge".

> +
> +examples:
> +  - |
> +    dcss: display-controller@32e00000 {
> +        #address-cells = <1>;
> +        #size-cells = <0>;
> +        compatible = "nxp,imx8mq-dcss";
> +        reg = <0x32e00000 0x2d000>, <0x32e2f000 0x1000>;
> +        interrupts = <6>, <8>, <9>;
> +        interrupt-names = "ctx_ld", "ctxld_kick", "vblank";
> +        interrupt-parent = <&irqsteer>;
> +        clocks = <&clk 248>, <&clk 247>, <&clk 249>,
> +                 <&clk 254>,<&clk 122>;
> +        clock-names = "apb", "axi", "rtrm", "pix", "dtrc";
> +        assigned-clocks = <&clk 107>, <&clk 109>, <&clk 266>;
> +        assigned-clock-parents = <&clk 78>, <&clk 78>, <&clk 3>;
> +        assigned-clock-rates = <800000000>,
> +                               <400000000>;
> +        port@0 {
> +            dcss_out: endpoint {
> +                remote-endpoint = <&hdmi_in>;
> +            };
> +        };
> +    };
> +


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

Re: [PATCH v3 4/4] arm64: dts: imx8mq: add DCSS node

[Lucas Stach]

On Fr, 2019-12-06 at 11:52 +0200, Laurentiu Palcu wrote:
> This patch adds the node for iMX8MQ Display Controller Subsystem.
> 
> Signed-off-by: Laurentiu Palcu <laurentiu.palcu@nxp.com>
> ---
>  arch/arm64/boot/dts/freescale/imx8mq.dtsi | 25 +++++++++++++++++++++++++
>  1 file changed, 25 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/freescale/imx8mq.dtsi b/arch/arm64/boot/dts/freescale/imx8mq.dtsi
> index f6e840c..da7e485 100644
> --- a/arch/arm64/boot/dts/freescale/imx8mq.dtsi
> +++ b/arch/arm64/boot/dts/freescale/imx8mq.dtsi
> @@ -981,6 +981,31 @@
>  				interrupt-controller;
>  				#interrupt-cells = <1>;
>  			};
> +
> +			dcss: display-controller@32e00000 {

Node address is lower than the irqsteer node, so the dcss node should
be added before, not after the irqsteer node in the DT.

> +				#address-cells = <1>;
> +				#size-cells = <0>;
> +				compatible = "nxp,imx8mq-dcss";
> +				reg = <0x32e00000 0x2d000>, <0x32e2f000 0x1000>;
> +				interrupts = <6>, <8>, <9>;
> +				interrupt-names = "ctx_ld", "ctxld_kick", "vblank";
> +				interrupt-parent = <&irqsteer>;
> +				clocks = <&clk IMX8MQ_CLK_DISP_APB_ROOT>,
> +					 <&clk IMX8MQ_CLK_DISP_AXI_ROOT>,
> +					 <&clk IMX8MQ_CLK_DISP_RTRM_ROOT>,
> +					 <&clk IMX8MQ_VIDEO2_PLL_OUT>,
> +					 <&clk IMX8MQ_CLK_DISP_DTRC>;
> +				clock-names = "apb", "axi", "rtrm", "pix", "dtrc";
> +				assigned-clocks = <&clk IMX8MQ_CLK_DISP_AXI>,
> +						  <&clk IMX8MQ_CLK_DISP_RTRM>,
> +						  <&clk IMX8MQ_VIDEO2_PLL1_REF_SEL>;
> +				assigned-clock-parents = <&clk IMX8MQ_SYS1_PLL_800M>,
> +							 <&clk IMX8MQ_SYS1_PLL_800M>,
> +							 <&clk IMX8MQ_CLK_27M>;
> +				assigned-clock-rates = <800000000>,
> +							   <400000000>;

Second line is not aligned to the first one.

> +				status = "disabled";
> +			};
>  		};
>  
>  		gpu: gpu@38000000 {


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

Re: [PATCH v3 2/4] drm/imx: Add initial support for DCSS on iMX8MQ

[Lucas Stach]

Hi Laurentiu,

just a first drive-by comment, more in-depth review tomorrow.

On Fr, 2019-12-06 at 11:52 +0200, Laurentiu Palcu wrote:
> This adds initial support for iMX8MQ's Display Controller Subsystem (DCSS).
> Some of its capabilities include:
>  * 4K@60fps;
>  * HDR10;
>  * one graphics and 2 video pipelines;
>  * on-the-fly decompression of compressed video and graphics;
> 
> The reference manual can be found here:
> https://www.nxp.com/webapp/Download?colCode=IMX8MDQLQRM
> 
> The current patch adds only basic functionality: one primary plane for
> graphics, linear, tiled and super-tiled buffers support (no graphics
> decompression yet), no HDR10 and no video planes.
> 
> Video planes support and HDR10 will be added in subsequent patches once
> per-plane de-gamma/CSC/gamma support is in.
> 
> Signed-off-by: Laurentiu Palcu <laurentiu.palcu@nxp.com>
> ---
[...]
> diff --git a/drivers/gpu/drm/imx/dcss/Kconfig b/drivers/gpu/drm/imx/dcss/Kconfig
> new file mode 100644
> index 00000000..a189dac
> --- /dev/null
> +++ b/drivers/gpu/drm/imx/dcss/Kconfig
> @@ -0,0 +1,8 @@
> +config DRM_IMX_DCSS
> +	tristate "i.MX8MQ DCSS"
> +	select RESET_CONTROLLER
> +	select IMX_IRQSTEER

This driver has no build time dependency on the IRQSTEER driver. It
needs it at runtime, but those dependencies are normally not described
in Kconfig.

On the other hand this is missing a "select DRM_KMS_CMA_HELPER".

Regards,
Lucas


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

Re: [PATCH v3 2/4] drm/imx: Add initial support for DCSS on iMX8MQ

[Laurentiu Palcu]

Hi Lucas,

Thanks for taking some time to look over this series.

On Mon, Feb 24, 2020 at 06:27:25PM +0100, Lucas Stach wrote:
> Hi Laurentiu,
> 
> just a first drive-by comment, more in-depth review tomorrow.
> 
> On Fr, 2019-12-06 at 11:52 +0200, Laurentiu Palcu wrote:
> > This adds initial support for iMX8MQ's Display Controller Subsystem (DCSS).
> > Some of its capabilities include:
> >  * 4K@60fps;
> >  * HDR10;
> >  * one graphics and 2 video pipelines;
> >  * on-the-fly decompression of compressed video and graphics;
> > 
> > The reference manual can be found here:
> > https://www.nxp.com/webapp/Download?colCode=IMX8MDQLQRM
> > 
> > The current patch adds only basic functionality: one primary plane for
> > graphics, linear, tiled and super-tiled buffers support (no graphics
> > decompression yet), no HDR10 and no video planes.
> > 
> > Video planes support and HDR10 will be added in subsequent patches once
> > per-plane de-gamma/CSC/gamma support is in.
> > 
> > Signed-off-by: Laurentiu Palcu <laurentiu.palcu@nxp.com>
> > ---
> [...]
> > diff --git a/drivers/gpu/drm/imx/dcss/Kconfig b/drivers/gpu/drm/imx/dcss/Kconfig
> > new file mode 100644
> > index 00000000..a189dac
> > --- /dev/null
> > +++ b/drivers/gpu/drm/imx/dcss/Kconfig
> > @@ -0,0 +1,8 @@
> > +config DRM_IMX_DCSS
> > +	tristate "i.MX8MQ DCSS"
> > +	select RESET_CONTROLLER
> > +	select IMX_IRQSTEER
> 
> This driver has no build time dependency on the IRQSTEER driver. It
> needs it at runtime, but those dependencies are normally not described
> in Kconfig.
> 
> On the other hand this is missing a "select DRM_KMS_CMA_HELPER".

Oops, I guess I missed this. I'll add it in next revision. Waiting for
your in-depth review first.

Thanks,
laurentiu

> 
> Regards,
> Lucas
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Laurentiu
NXP

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

Re: [PATCH v3 3/4] dt-bindings: display: imx: add bindings for DCSS

[Laurentiu Palcu]

Hi Lucas,

On Mon, Feb 24, 2020 at 06:21:57PM +0100, Lucas Stach wrote:
> On Fr, 2019-12-06 at 11:52 +0200, Laurentiu Palcu wrote:
> > Add bindings for iMX8MQ Display Controller Subsystem.
> > 
> > Signed-off-by: Laurentiu Palcu <laurentiu.palcu@nxp.com>
> > Reviewed-by: Rob Herring <robh@kernel.org>
> > ---
> >  .../bindings/display/imx/nxp,imx8mq-dcss.yaml      | 86 ++++++++++++++++++++++
> >  1 file changed, 86 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/display/imx/nxp,imx8mq-dcss.yaml
> > 
> > diff --git a/Documentation/devicetree/bindings/display/imx/nxp,imx8mq-dcss.yaml b/Documentation/devicetree/bindings/display/imx/nxp,imx8mq-dcss.yaml
> > new file mode 100644
> > index 00000000..efd2494
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/display/imx/nxp,imx8mq-dcss.yaml
> > @@ -0,0 +1,86 @@
> > +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> > +# Copyright 2019 NXP
> > +%YAML 1.2
> > +---
> > +$id: "http://devicetree.org/schemas/display/imx/nxp,imx8mq-dcss.yaml#"
> > +$schema: "http://devicetree.org/meta-schemas/core.yaml#"
> > +
> > +title: iMX8MQ Display Controller Subsystem (DCSS)
> > +
> > +maintainers:
> > +  - Laurentiu Palcu <laurentiu.palcu@nxp.com>
> > +
> > +description:
> > +
> > +  The DCSS (display controller sub system) is used to source up to three
> > +  display buffers, compose them, and drive a display using HDMI 2.0a(with HDCP
> > +  2.2) or MIPI-DSI.
> 
> HDMI 2.0a and MIPI_DSI are not really properties of the DCSS, but
> rather the connected bridges. Maybe just drop them here?

I'm a bit confused... Drop what here?

> 
> >  The DCSS is intended to support up to 4kp60 displays. HDR10
> > +  image processing capabilities are included to provide a solution capable of
> > +  driving next generation high dynamic range displays.
> > +
> > +properties:
> > +  compatible:
> > +    const: nxp,imx8mq-dcss
> > +
> > +  reg:
> > +    maxItems: 2
> > +
> > +  interrupts:
> > +    maxItems: 3
> > +    items:
> > +      - description: Context loader completion and error interrupt
> > +      - description: DTG interrupt used to signal context loader trigger time
> > +      - description: DTG interrupt for Vblank
> > +
> > +  interrupt-names:
> > +    maxItems: 3
> > +    items:
> > +      - const: ctx_ld
> 
> Can we make this just "ctxld" for a bit more consistency with the name
> below?

Fair enough. Will change.

> 
> > +      - const: ctxld_kick
> > +      - const: vblank
> > +
> > +  clocks:
> > +    maxItems: 5
> > +    items:
> > +      - description: Display APB clock for all peripheral PIO access interfaces
> > +      - description: Display AXI clock needed by DPR, Scaler, RTRAM_CTRL
> > +      - description: RTRAM clock
> > +      - description: Pixel clock, can be driver either by HDMI phy clock or MIPI
> > +      - description: DTRC clock, needed by video decompressor
> > +
> > +  clock-names:
> > +    items:
> > +      - const: apb
> > +      - const: axi
> > +      - const: rtrm
> > +      - const: pix
> > +      - const: dtrc
> > +
> > +  port@0:
> > +    type: object
> > +    description: A port node pointing to a hdmi_in or mipi_in port node.
> 
> "A port node pointing to the input port of a HDMI/DP or MIPI display
> bridge".

Okay, your description's sounds better. I'll add it in the next revision.

Thanks,
laurentiu

> 
> > +
> > +examples:
> > +  - |
> > +    dcss: display-controller@32e00000 {
> > +        #address-cells = <1>;
> > +        #size-cells = <0>;
> > +        compatible = "nxp,imx8mq-dcss";
> > +        reg = <0x32e00000 0x2d000>, <0x32e2f000 0x1000>;
> > +        interrupts = <6>, <8>, <9>;
> > +        interrupt-names = "ctx_ld", "ctxld_kick", "vblank";
> > +        interrupt-parent = <&irqsteer>;
> > +        clocks = <&clk 248>, <&clk 247>, <&clk 249>,
> > +                 <&clk 254>,<&clk 122>;
> > +        clock-names = "apb", "axi", "rtrm", "pix", "dtrc";
> > +        assigned-clocks = <&clk 107>, <&clk 109>, <&clk 266>;
> > +        assigned-clock-parents = <&clk 78>, <&clk 78>, <&clk 3>;
> > +        assigned-clock-rates = <800000000>,
> > +                               <400000000>;
> > +        port@0 {
> > +            dcss_out: endpoint {
> > +                remote-endpoint = <&hdmi_in>;
> > +            };
> > +        };
> > +    };
> > +
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Laurentiu
NXP

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

Re: Re: [PATCH v3 4/4] arm64: dts: imx8mq: add DCSS node

[Laurentiu Palcu]

Hi Lucas,

On Mon, Feb 24, 2020 at 06:23:51PM +0100, Lucas Stach wrote:
> On Fr, 2019-12-06 at 11:52 +0200, Laurentiu Palcu wrote:
> > This patch adds the node for iMX8MQ Display Controller Subsystem.
> >
> > Signed-off-by: Laurentiu Palcu <laurentiu.palcu@nxp.com>
> > ---
> >  arch/arm64/boot/dts/freescale/imx8mq.dtsi | 25 +++++++++++++++++++++++++
> >  1 file changed, 25 insertions(+)
> >
> > diff --git a/arch/arm64/boot/dts/freescale/imx8mq.dtsi b/arch/arm64/boot/dts/freescale/imx8mq.dtsi
> > index f6e840c..da7e485 100644
> > --- a/arch/arm64/boot/dts/freescale/imx8mq.dtsi
> > +++ b/arch/arm64/boot/dts/freescale/imx8mq.dtsi
> > @@ -981,6 +981,31 @@
> >                               interrupt-controller;
> >                               #interrupt-cells = <1>;
> >                       };
> > +
> > +                     dcss: display-controller@32e00000 {
> 
> Node address is lower than the irqsteer node, so the dcss node should
> be added before, not after the irqsteer node in the DT.

That's correct, I'll move it.


> 
> > +                             #address-cells = <1>;
> > +                             #size-cells = <0>;
> > +                             compatible = "nxp,imx8mq-dcss";
> > +                             reg = <0x32e00000 0x2d000>, <0x32e2f000 0x1000>;
> > +                             interrupts = <6>, <8>, <9>;
> > +                             interrupt-names = "ctx_ld", "ctxld_kick", "vblank";
> > +                             interrupt-parent = <&irqsteer>;
> > +                             clocks = <&clk IMX8MQ_CLK_DISP_APB_ROOT>,
> > +                                      <&clk IMX8MQ_CLK_DISP_AXI_ROOT>,
> > +                                      <&clk IMX8MQ_CLK_DISP_RTRM_ROOT>,
> > +                                      <&clk IMX8MQ_VIDEO2_PLL_OUT>,
> > +                                      <&clk IMX8MQ_CLK_DISP_DTRC>;
> > +                             clock-names = "apb", "axi", "rtrm", "pix", "dtrc";
> > +                             assigned-clocks = <&clk IMX8MQ_CLK_DISP_AXI>,
> > +                                               <&clk IMX8MQ_CLK_DISP_RTRM>,
> > +                                               <&clk IMX8MQ_VIDEO2_PLL1_REF_SEL>;
> > +                             assigned-clock-parents = <&clk IMX8MQ_SYS1_PLL_800M>,
> > +                                                      <&clk IMX8MQ_SYS1_PLL_800M>,
> > +                                                      <&clk IMX8MQ_CLK_27M>;
> > +                             assigned-clock-rates = <800000000>,
> > +                                                        <400000000>;
> 
> Second line is not aligned to the first one.

ack

Thanks,
laurentiu

> 
> > +                             status = "disabled";
> > +                     };
> >               };
> >
> >               gpu: gpu@38000000 {
> 

-- 
Laurentiu
NXP

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

Re: [PATCH v3 3/4] dt-bindings: display: imx: add bindings for DCSS

[Philipp Zabel]

Hi Laurentiu,

On Fri, 2019-12-06 at 11:52 +0200, Laurentiu Palcu wrote:
> Add bindings for iMX8MQ Display Controller Subsystem.
> 
> Signed-off-by: Laurentiu Palcu <laurentiu.palcu@nxp.com>
> Reviewed-by: Rob Herring <robh@kernel.org>
> ---
>  .../bindings/display/imx/nxp,imx8mq-dcss.yaml      | 86 ++++++++++++++++++++++
>  1 file changed, 86 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/display/imx/nxp,imx8mq-dcss.yaml
> 
> diff --git a/Documentation/devicetree/bindings/display/imx/nxp,imx8mq-dcss.yaml b/Documentation/devicetree/bindings/display/imx/nxp,imx8mq-dcss.yaml
> new file mode 100644
> index 00000000..efd2494
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/display/imx/nxp,imx8mq-dcss.yaml
> @@ -0,0 +1,86 @@
> +# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
> +# Copyright 2019 NXP
> +%YAML 1.2
> +---
> +$id: "http://devicetree.org/schemas/display/imx/nxp,imx8mq-dcss.yaml#"
> +$schema: "http://devicetree.org/meta-schemas/core.yaml#"
> +
> +title: iMX8MQ Display Controller Subsystem (DCSS)
> +
> +maintainers:
> +  - Laurentiu Palcu <laurentiu.palcu@nxp.com>
> +
> +description:
> +
> +  The DCSS (display controller sub system) is used to source up to three
> +  display buffers, compose them, and drive a display using HDMI 2.0a(with HDCP
> +  2.2) or MIPI-DSI. The DCSS is intended to support up to 4kp60 displays. HDR10
> +  image processing capabilities are included to provide a solution capable of
> +  driving next generation high dynamic range displays.
> +
> +properties:
> +  compatible:
> +    const: nxp,imx8mq-dcss
> +
> +  reg:
> +    maxItems: 2
> +
> +  interrupts:
> +    maxItems: 3
> +    items:
> +      - description: Context loader completion and error interrupt
> +      - description: DTG interrupt used to signal context loader trigger time
> +      - description: DTG interrupt for Vblank
> +
> +  interrupt-names:
> +    maxItems: 3
> +    items:
> +      - const: ctx_ld
> +      - const: ctxld_kick
> +      - const: vblank
> +
> +  clocks:
> +    maxItems: 5
> +    items:
> +      - description: Display APB clock for all peripheral PIO access interfaces
> +      - description: Display AXI clock needed by DPR, Scaler, RTRAM_CTRL
> +      - description: RTRAM clock
> +      - description: Pixel clock, can be driver either by HDMI phy clock or MIPI
> +      - description: DTRC clock, needed by video decompressor
> +
> +  clock-names:
> +    items:
> +      - const: apb
> +      - const: axi
> +      - const: rtrm
> +      - const: pix
> +      - const: dtrc
> +
> +  port@0:

If there is just a single output port, I think the @0 unit address
should be dropped. Otherwise the port node needs to contain a "reg =
<0>;" property in the example below:

> +    type: object
> +    description: A port node pointing to a hdmi_in or mipi_in port node.
> +
> +examples:
> +  - |
> +    dcss: display-controller@32e00000 {
> +        #address-cells = <1>;
> +        #size-cells = <0>;

/soc@0/bus@32c00000/display-controller@32e00000: unnecessary #address-cells/#size-cells without "ranges" or child "reg" property

> +        compatible = "nxp,imx8mq-dcss";
> +        reg = <0x32e00000 0x2d000>, <0x32e2f000 0x1000>;
> +        interrupts = <6>, <8>, <9>;
> +        interrupt-names = "ctx_ld", "ctxld_kick", "vblank";
> +        interrupt-parent = <&irqsteer>;
> +        clocks = <&clk 248>, <&clk 247>, <&clk 249>,
> +                 <&clk 254>,<&clk 122>;
> +        clock-names = "apb", "axi", "rtrm", "pix", "dtrc";
> +        assigned-clocks = <&clk 107>, <&clk 109>, <&clk 266>;
> +        assigned-clock-parents = <&clk 78>, <&clk 78>, <&clk 3>;
> +        assigned-clock-rates = <800000000>,
> +                               <400000000>;
> +        port@0 {

/soc@0/bus@32c00000/display-controller@32e00000/port@0: node has a unit name, but no reg property

regards
Philipp

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

Re: [PATCH v3 2/4] drm/imx: Add initial support for DCSS on iMX8MQ

[Philipp Zabel]

Hi Laurentiu,

On Fri, 2020-03-06 at 11:58 +0200, Laurentiu Palcu wrote:
> On Wed, Feb 26, 2020 at 02:19:11PM +0100, Lucas Stach wrote:
[...]
> > > +/* This function will be called from interrupt context. */
> > > +void dcss_scaler_write_sclctrl(struct dcss_scaler *scl)
> > > +{
> > > +	int chnum;
> > > +
> > > +	for (chnum = 0; chnum < 3; chnum++) {
> > > +		struct dcss_scaler_ch *ch = &scl->ch[chnum];
> > > +
> > > +		if (ch->scaler_ctrl_chgd) {
> > > +			dcss_ctxld_write_irqsafe(scl->ctxld, scl->ctx_id,
> > > +						 ch->scaler_ctrl,
> > > +						 ch->base_ofs +
> > > +						 DCSS_SCALER_CTRL);
> > 
> > Why is this using the _irqsafe variant without any locking? Won't this
> > lead to potential internal state corruption? dcss_ctxld_write is using
> > the _irqsave locking variants, so it fine with being called from IRQ
> > context.
> 
> This is only called from __dcss_ctxld_enable() which is already protected
> by lock/unlock in dcss_ctxld_kick().

You could add a lockdep_assert_held() line to the top of this function
to make it clear this depends on the lock being held.

regards
Philipp

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

Re: [PATCH v3 2/4] drm/imx: Add initial support for DCSS on iMX8MQ

[Laurentiu Palcu]

Hi Philipp,

On Fri, Mar 06, 2020 at 11:20:51AM +0100, Philipp Zabel wrote:
> Hi Laurentiu,
> 
> On Fri, 2020-03-06 at 11:58 +0200, Laurentiu Palcu wrote:
> > On Wed, Feb 26, 2020 at 02:19:11PM +0100, Lucas Stach wrote:
> [...]
> > > > +/* This function will be called from interrupt context. */
> > > > +void dcss_scaler_write_sclctrl(struct dcss_scaler *scl)
> > > > +{
> > > > +	int chnum;
> > > > +
> > > > +	for (chnum = 0; chnum < 3; chnum++) {
> > > > +		struct dcss_scaler_ch *ch = &scl->ch[chnum];
> > > > +
> > > > +		if (ch->scaler_ctrl_chgd) {
> > > > +			dcss_ctxld_write_irqsafe(scl->ctxld, scl->ctx_id,
> > > > +						 ch->scaler_ctrl,
> > > > +						 ch->base_ofs +
> > > > +						 DCSS_SCALER_CTRL);
> > > 
> > > Why is this using the _irqsafe variant without any locking? Won't this
> > > lead to potential internal state corruption? dcss_ctxld_write is using
> > > the _irqsave locking variants, so it fine with being called from IRQ
> > > context.
> > 
> > This is only called from __dcss_ctxld_enable() which is already protected
> > by lock/unlock in dcss_ctxld_kick().
> 
> You could add a lockdep_assert_held() line to the top of this function
> to make it clear this depends on the lock being held.

Thanks for the suggestion. Will add a check.

Thanks,
laurentiu

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