[PATCH v3 1/2] dt-bindings: display: imx: add binding for i.MX8MP HDMI PVI

[PATCH v3 1/2] dt-bindings: display: imx: add binding for i.MX8MP HDMI PVI

[Lucas Stach]

Add binding for the i.MX8MP HDMI parallel video interface block.

Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
---
 .../display/imx/fsl,imx8mp-hdmi-pvi.yaml      | 80 +++++++++++++++++++
 1 file changed, 80 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/display/imx/fsl,imx8mp-hdmi-pvi.yaml

diff --git a/Documentation/devicetree/bindings/display/imx/fsl,imx8mp-hdmi-pvi.yaml b/Documentation/devicetree/bindings/display/imx/fsl,imx8mp-hdmi-pvi.yaml
new file mode 100644
index 000000000000..12855bb3ed4c
--- /dev/null
+++ b/Documentation/devicetree/bindings/display/imx/fsl,imx8mp-hdmi-pvi.yaml
@@ -0,0 +1,80 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/display/imx/fsl,imx8mp-hdmi-pvi.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Freescale i.MX8MP HDMI Parallel Video Interface
+
+maintainers:
+  - Lucas Stach <l.stach@pengutronix.de>
+
+description: |
+  The HDMI parallel video interface is a timing and sync generator block in the
+  i.MX8MP SoC, that sits between the video source and the HDMI TX controller.
+
+properties:
+  compatible:
+    const: fsl,imx8mp-hdmi-pvi
+
+  reg:
+    maxItems: 1
+
+  power-domains:
+    maxItems: 1
+
+  ports:
+    $ref: /schemas/graph.yaml#/properties/ports
+
+    properties:
+      port@0:
+        $ref: /schemas/graph.yaml#/properties/port
+        description: Input from the LCDIF controller.
+
+      port@1:
+        $ref: /schemas/graph.yaml#/properties/port
+        description: Output to the HDMI TX controller.
+
+    required:
+      - port@0
+      - port@1
+
+required:
+  - compatible
+  - reg
+  - interrupts
+  - power-domains
+  - ports
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/interrupt-controller/irq.h>
+    #include <dt-bindings/power/imx8mp-power.h>
+
+    display-bridge@32fc4000 {
+        compatible = "fsl,imx8mp-hdmi-pvi";
+        reg = <0x32fc4000 0x40>;
+        interrupts = <12 IRQ_TYPE_LEVEL_HIGH>;
+        power-domains = <&hdmi_blk_ctrl IMX8MP_HDMIBLK_PD_PVI>;
+
+        ports {
+            #address-cells = <1>;
+            #size-cells = <0>;
+
+            port@0 {
+                reg = <0>;
+                pvi_from_lcdif3: endpoint {
+                    remote-endpoint = <&lcdif3_to_pvi>;
+                };
+            };
+
+            port@1 {
+                reg = <1>;
+                pvi_to_hdmi_tx: endpoint {
+                    remote-endpoint = <&hdmi_tx_from_pvi>;
+                };
+            };
+        };
+    };
-- 
2.39.2

[PATCH v3 2/2] drm/bridge: imx: add driver for HDMI TX Parallel Video Interface

[Lucas Stach]

This IP block is found in the HDMI subsystem of the i.MX8MP SoC. It has a
full timing generator and can switch between different video sources. On
the i.MX8MP however the only supported source is the LCDIF. The block
just needs to be powered up and told about the polarity of the video
sync signals to act in bypass mode.

Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
Reviewed-by: Luca Ceresoli <luca.ceresoli@bootlin.com> (v2)
Tested-by: Marek Vasut <marex@denx.de> (v1)
Tested-by: Luca Ceresoli <luca.ceresoli@bootlin.com> (v2)
Tested-by: Richard Leitner <richard.leitner@skidata.com> (v2)
Tested-by: Frieder Schrempf <frieder.schrempf@kontron.de> (v2)
---
 drivers/gpu/drm/bridge/imx/Kconfig           |   7 +
 drivers/gpu/drm/bridge/imx/Makefile          |   1 +
 drivers/gpu/drm/bridge/imx/imx8mp-hdmi-pvi.c | 206 +++++++++++++++++++
 3 files changed, 214 insertions(+)
 create mode 100644 drivers/gpu/drm/bridge/imx/imx8mp-hdmi-pvi.c

diff --git a/drivers/gpu/drm/bridge/imx/Kconfig b/drivers/gpu/drm/bridge/imx/Kconfig
index 9fae28db6aa7..3a4e663d922a 100644
--- a/drivers/gpu/drm/bridge/imx/Kconfig
+++ b/drivers/gpu/drm/bridge/imx/Kconfig
@@ -3,6 +3,13 @@ if ARCH_MXC || COMPILE_TEST
 config DRM_IMX_LDB_HELPER
 	tristate
 
+config DRM_IMX8MP_HDMI_PVI
+	tristate "Freescale i.MX8MP HDMI PVI bridge support"
+	depends on OF
+	help
+	  Choose this to enable support for the internal HDMI TX Parallel
+	  Video Interface found on the Freescale i.MX8MP SoC.
+
 config DRM_IMX8QM_LDB
 	tristate "Freescale i.MX8QM LVDS display bridge"
 	depends on OF
diff --git a/drivers/gpu/drm/bridge/imx/Makefile b/drivers/gpu/drm/bridge/imx/Makefile
index 8e2ebf3399a1..be9b4f9adb50 100644
--- a/drivers/gpu/drm/bridge/imx/Makefile
+++ b/drivers/gpu/drm/bridge/imx/Makefile
@@ -1,4 +1,5 @@
 obj-$(CONFIG_DRM_IMX_LDB_HELPER) += imx-ldb-helper.o
+obj-$(CONFIG_DRM_IMX8MP_HDMI_PVI) += imx8mp-hdmi-pvi.o
 obj-$(CONFIG_DRM_IMX8QM_LDB) += imx8qm-ldb.o
 obj-$(CONFIG_DRM_IMX8QXP_LDB) += imx8qxp-ldb.o
 obj-$(CONFIG_DRM_IMX8QXP_PIXEL_COMBINER) += imx8qxp-pixel-combiner.o
diff --git a/drivers/gpu/drm/bridge/imx/imx8mp-hdmi-pvi.c b/drivers/gpu/drm/bridge/imx/imx8mp-hdmi-pvi.c
new file mode 100644
index 000000000000..5ccd70c98187
--- /dev/null
+++ b/drivers/gpu/drm/bridge/imx/imx8mp-hdmi-pvi.c
@@ -0,0 +1,206 @@
+// SPDX-License-Identifier: GPL-2.0+
+
+/*
+ * Copyright (C) 2022 Pengutronix, Lucas Stach <kernel@pengutronix.de>
+ */
+
+#include <drm/drm_atomic_helper.h>
+#include <drm/drm_bridge.h>
+#include <drm/drm_crtc.h>
+#include <linux/bitfield.h>
+#include <linux/io.h>
+#include <linux/module.h>
+#include <linux/of_device.h>
+#include <linux/of_graph.h>
+#include <linux/pm_runtime.h>
+
+#define HTX_PVI_CTRL			0x0
+#define  PVI_CTRL_OP_VSYNC_POL		BIT(18)
+#define  PVI_CTRL_OP_HSYNC_POL		BIT(17)
+#define  PVI_CTRL_OP_DE_POL		BIT(16)
+#define  PVI_CTRL_INP_VSYNC_POL		BIT(14)
+#define  PVI_CTRL_INP_HSYNC_POL		BIT(13)
+#define  PVI_CTRL_INP_DE_POL		BIT(12)
+#define  PVI_CTRL_MODE_MASK		GENMASK(2, 1)
+#define  PVI_CTRL_MODE_LCDIF		2
+#define  PVI_CTRL_EN			BIT(0)
+
+struct imx8mp_hdmi_pvi {
+	struct drm_bridge	bridge;
+	struct device		*dev;
+	struct drm_bridge	*next_bridge;
+	void __iomem		*regs;
+};
+
+static inline struct imx8mp_hdmi_pvi *
+to_imx8mp_hdmi_pvi(struct drm_bridge *bridge)
+{
+	return container_of(bridge, struct imx8mp_hdmi_pvi, bridge);
+}
+
+static int imx8mp_hdmi_pvi_bridge_attach(struct drm_bridge *bridge,
+					 enum drm_bridge_attach_flags flags)
+{
+	struct imx8mp_hdmi_pvi *pvi = to_imx8mp_hdmi_pvi(bridge);
+
+	return drm_bridge_attach(bridge->encoder, pvi->next_bridge,
+				 bridge, flags);
+}
+
+static void imx8mp_hdmi_pvi_bridge_enable(struct drm_bridge *bridge,
+					  struct drm_bridge_state *bridge_state)
+{
+	struct drm_atomic_state *state = bridge_state->base.state;
+	struct imx8mp_hdmi_pvi *pvi = to_imx8mp_hdmi_pvi(bridge);
+	struct drm_connector_state *conn_state;
+	const struct drm_display_mode *mode;
+	struct drm_crtc_state *crtc_state;
+	struct drm_connector *connector;
+	u32 bus_flags, val;
+
+	connector = drm_atomic_get_new_connector_for_encoder(state, bridge->encoder);
+	conn_state = drm_atomic_get_new_connector_state(state, connector);
+	crtc_state = drm_atomic_get_new_crtc_state(state, conn_state->crtc);
+
+	if (WARN_ON(pm_runtime_resume_and_get(pvi->dev)))
+		return;
+
+	mode = &crtc_state->adjusted_mode;
+
+	val = FIELD_PREP(PVI_CTRL_MODE_MASK, PVI_CTRL_MODE_LCDIF) | PVI_CTRL_EN;
+
+	if (mode->flags & DRM_MODE_FLAG_PVSYNC)
+		val |= PVI_CTRL_OP_VSYNC_POL | PVI_CTRL_INP_VSYNC_POL;
+
+	if (mode->flags & DRM_MODE_FLAG_PHSYNC)
+		val |= PVI_CTRL_OP_HSYNC_POL | PVI_CTRL_INP_HSYNC_POL;
+
+	if (pvi->next_bridge->timings)
+		bus_flags = pvi->next_bridge->timings->input_bus_flags;
+	else if (bridge_state)
+		bus_flags = bridge_state->input_bus_cfg.flags;
+
+	if (bus_flags & DRM_BUS_FLAG_DE_HIGH)
+		val |= PVI_CTRL_OP_DE_POL | PVI_CTRL_INP_DE_POL;
+
+	writel(val, pvi->regs + HTX_PVI_CTRL);
+}
+
+static void imx8mp_hdmi_pvi_bridge_disable(struct drm_bridge *bridge,
+					   struct drm_bridge_state *bridge_state)
+{
+	struct imx8mp_hdmi_pvi *pvi = to_imx8mp_hdmi_pvi(bridge);
+
+	writel(0x0, pvi->regs + HTX_PVI_CTRL);
+
+	pm_runtime_put(pvi->dev);
+}
+
+static u32 *
+imx8mp_hdmi_pvi_bridge_get_input_bus_fmts(struct drm_bridge *bridge,
+					  struct drm_bridge_state *bridge_state,
+					  struct drm_crtc_state *crtc_state,
+					  struct drm_connector_state *conn_state,
+					  u32 output_fmt,
+					  unsigned int *num_input_fmts)
+{
+	struct imx8mp_hdmi_pvi *pvi = to_imx8mp_hdmi_pvi(bridge);
+	struct drm_bridge *next_bridge = pvi->next_bridge;
+	struct drm_bridge_state *next_state;
+
+	if (!next_bridge->funcs->atomic_get_input_bus_fmts)
+		return 0;
+
+	next_state = drm_atomic_get_new_bridge_state(crtc_state->state,
+						     next_bridge);
+
+	return next_bridge->funcs->atomic_get_input_bus_fmts(next_bridge,
+							     next_state,
+							     crtc_state,
+							     conn_state,
+							     output_fmt,
+							     num_input_fmts);
+}
+
+static const struct drm_bridge_funcs imx_hdmi_pvi_bridge_funcs = {
+	.attach		= imx8mp_hdmi_pvi_bridge_attach,
+	.atomic_enable	= imx8mp_hdmi_pvi_bridge_enable,
+	.atomic_disable	= imx8mp_hdmi_pvi_bridge_disable,
+	.atomic_get_input_bus_fmts = imx8mp_hdmi_pvi_bridge_get_input_bus_fmts,
+	.atomic_duplicate_state = drm_atomic_helper_bridge_duplicate_state,
+	.atomic_destroy_state = drm_atomic_helper_bridge_destroy_state,
+	.atomic_reset = drm_atomic_helper_bridge_reset,
+};
+
+static int imx8mp_hdmi_pvi_probe(struct platform_device *pdev)
+{
+	struct device_node *remote;
+	struct imx8mp_hdmi_pvi *pvi;
+
+	pvi = devm_kzalloc(&pdev->dev, sizeof(*pvi), GFP_KERNEL);
+	if (!pvi)
+		return -ENOMEM;
+
+	platform_set_drvdata(pdev, pvi);
+	pvi->dev = &pdev->dev;
+
+	pvi->regs = devm_platform_ioremap_resource(pdev, 0);
+	if (IS_ERR(pvi->regs))
+		return PTR_ERR(pvi->regs);
+
+	/* Get the next bridge in the pipeline. */
+	remote = of_graph_get_remote_node(pdev->dev.of_node, 1, -1);
+	if (!remote)
+		return -EINVAL;
+
+	pvi->next_bridge = of_drm_find_bridge(remote);
+	of_node_put(remote);
+
+	if (!pvi->next_bridge)
+		return dev_err_probe(&pdev->dev, -EPROBE_DEFER,
+				     "could not find next bridge\n");
+
+	/* Register the bridge. */
+	pvi->bridge.funcs = &imx_hdmi_pvi_bridge_funcs;
+	pvi->bridge.of_node = pdev->dev.of_node;
+	pvi->bridge.timings = pvi->next_bridge->timings;
+
+	drm_bridge_add(&pvi->bridge);
+
+	pm_runtime_enable(&pdev->dev);
+
+	return 0;
+}
+
+static int imx8mp_hdmi_pvi_remove(struct platform_device *pdev)
+{
+	struct imx8mp_hdmi_pvi *pvi = platform_get_drvdata(pdev);
+
+	pm_runtime_disable(&pdev->dev);
+
+	drm_bridge_remove(&pvi->bridge);
+
+	return 0;
+}
+
+static const struct of_device_id imx8mp_hdmi_pvi_match[] = {
+	{
+		.compatible = "fsl,imx8mp-hdmi-pvi",
+	}, {
+		/* sentinel */
+	}
+};
+MODULE_DEVICE_TABLE(of, imx8mp_hdmi_pvi_match);
+
+static struct platform_driver imx8mp_hdmi_pvi_driver = {
+	.probe	= imx8mp_hdmi_pvi_probe,
+	.remove	= imx8mp_hdmi_pvi_remove,
+	.driver		= {
+		.name = "imx-hdmi-pvi",
+		.of_match_table	= imx8mp_hdmi_pvi_match,
+	},
+};
+module_platform_driver(imx8mp_hdmi_pvi_driver);
+
+MODULE_DESCRIPTION("i.MX8MP HDMI TX Parallel Video Interface bridge driver");
+MODULE_LICENSE("GPL");
-- 
2.39.2

Re: [PATCH v3 2/2] drm/bridge: imx: add driver for HDMI TX Parallel Video Interface

[Laurent Pinchart]

Hi Lucas,

Thank you for the patch.

On Wed, Sep 20, 2023 at 07:10:09PM +0200, Lucas Stach wrote:
> This IP block is found in the HDMI subsystem of the i.MX8MP SoC. It has a
> full timing generator and can switch between different video sources. On
> the i.MX8MP however the only supported source is the LCDIF. The block
> just needs to be powered up and told about the polarity of the video
> sync signals to act in bypass mode.
> 
> Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
> Reviewed-by: Luca Ceresoli <luca.ceresoli@bootlin.com> (v2)
> Tested-by: Marek Vasut <marex@denx.de> (v1)
> Tested-by: Luca Ceresoli <luca.ceresoli@bootlin.com> (v2)
> Tested-by: Richard Leitner <richard.leitner@skidata.com> (v2)
> Tested-by: Frieder Schrempf <frieder.schrempf@kontron.de> (v2)
> ---
>  drivers/gpu/drm/bridge/imx/Kconfig           |   7 +
>  drivers/gpu/drm/bridge/imx/Makefile          |   1 +
>  drivers/gpu/drm/bridge/imx/imx8mp-hdmi-pvi.c | 206 +++++++++++++++++++
>  3 files changed, 214 insertions(+)
>  create mode 100644 drivers/gpu/drm/bridge/imx/imx8mp-hdmi-pvi.c
> 
> diff --git a/drivers/gpu/drm/bridge/imx/Kconfig b/drivers/gpu/drm/bridge/imx/Kconfig
> index 9fae28db6aa7..3a4e663d922a 100644
> --- a/drivers/gpu/drm/bridge/imx/Kconfig
> +++ b/drivers/gpu/drm/bridge/imx/Kconfig
> @@ -3,6 +3,13 @@ if ARCH_MXC || COMPILE_TEST
>  config DRM_IMX_LDB_HELPER
>  	tristate
>  
> +config DRM_IMX8MP_HDMI_PVI
> +	tristate "Freescale i.MX8MP HDMI PVI bridge support"
> +	depends on OF
> +	help
> +	  Choose this to enable support for the internal HDMI TX Parallel
> +	  Video Interface found on the Freescale i.MX8MP SoC.
> +
>  config DRM_IMX8QM_LDB
>  	tristate "Freescale i.MX8QM LVDS display bridge"
>  	depends on OF
> diff --git a/drivers/gpu/drm/bridge/imx/Makefile b/drivers/gpu/drm/bridge/imx/Makefile
> index 8e2ebf3399a1..be9b4f9adb50 100644
> --- a/drivers/gpu/drm/bridge/imx/Makefile
> +++ b/drivers/gpu/drm/bridge/imx/Makefile
> @@ -1,4 +1,5 @@
>  obj-$(CONFIG_DRM_IMX_LDB_HELPER) += imx-ldb-helper.o
> +obj-$(CONFIG_DRM_IMX8MP_HDMI_PVI) += imx8mp-hdmi-pvi.o
>  obj-$(CONFIG_DRM_IMX8QM_LDB) += imx8qm-ldb.o
>  obj-$(CONFIG_DRM_IMX8QXP_LDB) += imx8qxp-ldb.o
>  obj-$(CONFIG_DRM_IMX8QXP_PIXEL_COMBINER) += imx8qxp-pixel-combiner.o
> diff --git a/drivers/gpu/drm/bridge/imx/imx8mp-hdmi-pvi.c b/drivers/gpu/drm/bridge/imx/imx8mp-hdmi-pvi.c
> new file mode 100644
> index 000000000000..5ccd70c98187
> --- /dev/null
> +++ b/drivers/gpu/drm/bridge/imx/imx8mp-hdmi-pvi.c
> @@ -0,0 +1,206 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +
> +/*
> + * Copyright (C) 2022 Pengutronix, Lucas Stach <kernel@pengutronix.de>
> + */
> +
> +#include <drm/drm_atomic_helper.h>
> +#include <drm/drm_bridge.h>
> +#include <drm/drm_crtc.h>
> +#include <linux/bitfield.h>
> +#include <linux/io.h>
> +#include <linux/module.h>
> +#include <linux/of_device.h>
> +#include <linux/of_graph.h>
> +#include <linux/pm_runtime.h>
> +
> +#define HTX_PVI_CTRL			0x0
> +#define  PVI_CTRL_OP_VSYNC_POL		BIT(18)
> +#define  PVI_CTRL_OP_HSYNC_POL		BIT(17)
> +#define  PVI_CTRL_OP_DE_POL		BIT(16)
> +#define  PVI_CTRL_INP_VSYNC_POL		BIT(14)
> +#define  PVI_CTRL_INP_HSYNC_POL		BIT(13)
> +#define  PVI_CTRL_INP_DE_POL		BIT(12)
> +#define  PVI_CTRL_MODE_MASK		GENMASK(2, 1)
> +#define  PVI_CTRL_MODE_LCDIF		2
> +#define  PVI_CTRL_EN			BIT(0)
> +
> +struct imx8mp_hdmi_pvi {
> +	struct drm_bridge	bridge;
> +	struct device		*dev;
> +	struct drm_bridge	*next_bridge;
> +	void __iomem		*regs;
> +};
> +
> +static inline struct imx8mp_hdmi_pvi *
> +to_imx8mp_hdmi_pvi(struct drm_bridge *bridge)
> +{
> +	return container_of(bridge, struct imx8mp_hdmi_pvi, bridge);
> +}
> +
> +static int imx8mp_hdmi_pvi_bridge_attach(struct drm_bridge *bridge,
> +					 enum drm_bridge_attach_flags flags)
> +{
> +	struct imx8mp_hdmi_pvi *pvi = to_imx8mp_hdmi_pvi(bridge);
> +
> +	return drm_bridge_attach(bridge->encoder, pvi->next_bridge,
> +				 bridge, flags);
> +}
> +
> +static void imx8mp_hdmi_pvi_bridge_enable(struct drm_bridge *bridge,
> +					  struct drm_bridge_state *bridge_state)
> +{
> +	struct drm_atomic_state *state = bridge_state->base.state;
> +	struct imx8mp_hdmi_pvi *pvi = to_imx8mp_hdmi_pvi(bridge);
> +	struct drm_connector_state *conn_state;
> +	const struct drm_display_mode *mode;
> +	struct drm_crtc_state *crtc_state;
> +	struct drm_connector *connector;
> +	u32 bus_flags, val;
> +
> +	connector = drm_atomic_get_new_connector_for_encoder(state, bridge->encoder);
> +	conn_state = drm_atomic_get_new_connector_state(state, connector);
> +	crtc_state = drm_atomic_get_new_crtc_state(state, conn_state->crtc);
> +
> +	if (WARN_ON(pm_runtime_resume_and_get(pvi->dev)))
> +		return;
> +
> +	mode = &crtc_state->adjusted_mode;
> +
> +	val = FIELD_PREP(PVI_CTRL_MODE_MASK, PVI_CTRL_MODE_LCDIF) | PVI_CTRL_EN;
> +
> +	if (mode->flags & DRM_MODE_FLAG_PVSYNC)
> +		val |= PVI_CTRL_OP_VSYNC_POL | PVI_CTRL_INP_VSYNC_POL;
> +
> +	if (mode->flags & DRM_MODE_FLAG_PHSYNC)
> +		val |= PVI_CTRL_OP_HSYNC_POL | PVI_CTRL_INP_HSYNC_POL;
> +
> +	if (pvi->next_bridge->timings)
> +		bus_flags = pvi->next_bridge->timings->input_bus_flags;
> +	else if (bridge_state)
> +		bus_flags = bridge_state->input_bus_cfg.flags;
> +
> +	if (bus_flags & DRM_BUS_FLAG_DE_HIGH)
> +		val |= PVI_CTRL_OP_DE_POL | PVI_CTRL_INP_DE_POL;
> +
> +	writel(val, pvi->regs + HTX_PVI_CTRL);
> +}
> +
> +static void imx8mp_hdmi_pvi_bridge_disable(struct drm_bridge *bridge,
> +					   struct drm_bridge_state *bridge_state)
> +{
> +	struct imx8mp_hdmi_pvi *pvi = to_imx8mp_hdmi_pvi(bridge);
> +
> +	writel(0x0, pvi->regs + HTX_PVI_CTRL);
> +
> +	pm_runtime_put(pvi->dev);
> +}
> +
> +static u32 *
> +imx8mp_hdmi_pvi_bridge_get_input_bus_fmts(struct drm_bridge *bridge,
> +					  struct drm_bridge_state *bridge_state,
> +					  struct drm_crtc_state *crtc_state,
> +					  struct drm_connector_state *conn_state,
> +					  u32 output_fmt,
> +					  unsigned int *num_input_fmts)
> +{
> +	struct imx8mp_hdmi_pvi *pvi = to_imx8mp_hdmi_pvi(bridge);
> +	struct drm_bridge *next_bridge = pvi->next_bridge;
> +	struct drm_bridge_state *next_state;
> +
> +	if (!next_bridge->funcs->atomic_get_input_bus_fmts)
> +		return 0;
> +
> +	next_state = drm_atomic_get_new_bridge_state(crtc_state->state,
> +						     next_bridge);
> +
> +	return next_bridge->funcs->atomic_get_input_bus_fmts(next_bridge,
> +							     next_state,
> +							     crtc_state,
> +							     conn_state,
> +							     output_fmt,
> +							     num_input_fmts);
> +}
> +
> +static const struct drm_bridge_funcs imx_hdmi_pvi_bridge_funcs = {
> +	.attach		= imx8mp_hdmi_pvi_bridge_attach,
> +	.atomic_enable	= imx8mp_hdmi_pvi_bridge_enable,
> +	.atomic_disable	= imx8mp_hdmi_pvi_bridge_disable,
> +	.atomic_get_input_bus_fmts = imx8mp_hdmi_pvi_bridge_get_input_bus_fmts,
> +	.atomic_duplicate_state = drm_atomic_helper_bridge_duplicate_state,
> +	.atomic_destroy_state = drm_atomic_helper_bridge_destroy_state,
> +	.atomic_reset = drm_atomic_helper_bridge_reset,
> +};
> +
> +static int imx8mp_hdmi_pvi_probe(struct platform_device *pdev)
> +{
> +	struct device_node *remote;
> +	struct imx8mp_hdmi_pvi *pvi;
> +
> +	pvi = devm_kzalloc(&pdev->dev, sizeof(*pvi), GFP_KERNEL);
> +	if (!pvi)
> +		return -ENOMEM;
> +
> +	platform_set_drvdata(pdev, pvi);
> +	pvi->dev = &pdev->dev;
> +
> +	pvi->regs = devm_platform_ioremap_resource(pdev, 0);
> +	if (IS_ERR(pvi->regs))
> +		return PTR_ERR(pvi->regs);
> +
> +	/* Get the next bridge in the pipeline. */
> +	remote = of_graph_get_remote_node(pdev->dev.of_node, 1, -1);
> +	if (!remote)
> +		return -EINVAL;
> +
> +	pvi->next_bridge = of_drm_find_bridge(remote);
> +	of_node_put(remote);
> +
> +	if (!pvi->next_bridge)
> +		return dev_err_probe(&pdev->dev, -EPROBE_DEFER,
> +				     "could not find next bridge\n");
> +
> +	/* Register the bridge. */
> +	pvi->bridge.funcs = &imx_hdmi_pvi_bridge_funcs;
> +	pvi->bridge.of_node = pdev->dev.of_node;
> +	pvi->bridge.timings = pvi->next_bridge->timings;
> +
> +	drm_bridge_add(&pvi->bridge);
> +
> +	pm_runtime_enable(&pdev->dev);

I would move this just before drm_bridge_add(). In theory, as soon as
the bridge is added, it could get used, so it's a good practice to
initialize everything before adding it.

> +
> +	return 0;
> +}
> +
> +static int imx8mp_hdmi_pvi_remove(struct platform_device *pdev)
> +{
> +	struct imx8mp_hdmi_pvi *pvi = platform_get_drvdata(pdev);
> +
> +	pm_runtime_disable(&pdev->dev);
> +
> +	drm_bridge_remove(&pvi->bridge);

And here you could flip the two as well for consistency.

With these minor changes,

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> +
> +	return 0;
> +}
> +
> +static const struct of_device_id imx8mp_hdmi_pvi_match[] = {
> +	{
> +		.compatible = "fsl,imx8mp-hdmi-pvi",
> +	}, {
> +		/* sentinel */
> +	}
> +};
> +MODULE_DEVICE_TABLE(of, imx8mp_hdmi_pvi_match);
> +
> +static struct platform_driver imx8mp_hdmi_pvi_driver = {
> +	.probe	= imx8mp_hdmi_pvi_probe,
> +	.remove	= imx8mp_hdmi_pvi_remove,
> +	.driver		= {
> +		.name = "imx-hdmi-pvi",
> +		.of_match_table	= imx8mp_hdmi_pvi_match,
> +	},
> +};
> +module_platform_driver(imx8mp_hdmi_pvi_driver);
> +
> +MODULE_DESCRIPTION("i.MX8MP HDMI TX Parallel Video Interface bridge driver");
> +MODULE_LICENSE("GPL");

-- 
Regards,

Laurent Pinchart

Re: [PATCH v3 2/2] drm/bridge: imx: add driver for HDMI TX Parallel Video Interface

[Adam Ford]

On Wed, Sep 20, 2023 at 3:57 PM Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
>
> Hi Lucas,
>
> Thank you for the patch.
>
> On Wed, Sep 20, 2023 at 07:10:09PM +0200, Lucas Stach wrote:
> > This IP block is found in the HDMI subsystem of the i.MX8MP SoC. It has a
> > full timing generator and can switch between different video sources. On
> > the i.MX8MP however the only supported source is the LCDIF. The block
> > just needs to be powered up and told about the polarity of the video
> > sync signals to act in bypass mode.
> >
> > Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
> > Reviewed-by: Luca Ceresoli <luca.ceresoli@bootlin.com> (v2)
> > Tested-by: Marek Vasut <marex@denx.de> (v1)
> > Tested-by: Luca Ceresoli <luca.ceresoli@bootlin.com> (v2)
> > Tested-by: Richard Leitner <richard.leitner@skidata.com> (v2)
> > Tested-by: Frieder Schrempf <frieder.schrempf@kontron.de> (v2)
> > ---
> >  drivers/gpu/drm/bridge/imx/Kconfig           |   7 +
> >  drivers/gpu/drm/bridge/imx/Makefile          |   1 +
> >  drivers/gpu/drm/bridge/imx/imx8mp-hdmi-pvi.c | 206 +++++++++++++++++++
> >  3 files changed, 214 insertions(+)
> >  create mode 100644 drivers/gpu/drm/bridge/imx/imx8mp-hdmi-pvi.c
> >
> > diff --git a/drivers/gpu/drm/bridge/imx/Kconfig b/drivers/gpu/drm/bridge/imx/Kconfig
> > index 9fae28db6aa7..3a4e663d922a 100644
> > --- a/drivers/gpu/drm/bridge/imx/Kconfig
> > +++ b/drivers/gpu/drm/bridge/imx/Kconfig
> > @@ -3,6 +3,13 @@ if ARCH_MXC || COMPILE_TEST
> >  config DRM_IMX_LDB_HELPER
> >       tristate
> >
> > +config DRM_IMX8MP_HDMI_PVI
> > +     tristate "Freescale i.MX8MP HDMI PVI bridge support"
> > +     depends on OF
> > +     help
> > +       Choose this to enable support for the internal HDMI TX Parallel
> > +       Video Interface found on the Freescale i.MX8MP SoC.
> > +
> >  config DRM_IMX8QM_LDB
> >       tristate "Freescale i.MX8QM LVDS display bridge"
> >       depends on OF
> > diff --git a/drivers/gpu/drm/bridge/imx/Makefile b/drivers/gpu/drm/bridge/imx/Makefile
> > index 8e2ebf3399a1..be9b4f9adb50 100644
> > --- a/drivers/gpu/drm/bridge/imx/Makefile
> > +++ b/drivers/gpu/drm/bridge/imx/Makefile
> > @@ -1,4 +1,5 @@
> >  obj-$(CONFIG_DRM_IMX_LDB_HELPER) += imx-ldb-helper.o
> > +obj-$(CONFIG_DRM_IMX8MP_HDMI_PVI) += imx8mp-hdmi-pvi.o
> >  obj-$(CONFIG_DRM_IMX8QM_LDB) += imx8qm-ldb.o
> >  obj-$(CONFIG_DRM_IMX8QXP_LDB) += imx8qxp-ldb.o
> >  obj-$(CONFIG_DRM_IMX8QXP_PIXEL_COMBINER) += imx8qxp-pixel-combiner.o
> > diff --git a/drivers/gpu/drm/bridge/imx/imx8mp-hdmi-pvi.c b/drivers/gpu/drm/bridge/imx/imx8mp-hdmi-pvi.c
> > new file mode 100644
> > index 000000000000..5ccd70c98187
> > --- /dev/null
> > +++ b/drivers/gpu/drm/bridge/imx/imx8mp-hdmi-pvi.c
> > @@ -0,0 +1,206 @@
> > +// SPDX-License-Identifier: GPL-2.0+
> > +
> > +/*
> > + * Copyright (C) 2022 Pengutronix, Lucas Stach <kernel@pengutronix.de>
> > + */
> > +
> > +#include <drm/drm_atomic_helper.h>
> > +#include <drm/drm_bridge.h>
> > +#include <drm/drm_crtc.h>
> > +#include <linux/bitfield.h>
> > +#include <linux/io.h>
> > +#include <linux/module.h>
> > +#include <linux/of_device.h>
> > +#include <linux/of_graph.h>
> > +#include <linux/pm_runtime.h>
> > +
> > +#define HTX_PVI_CTRL                 0x0
> > +#define  PVI_CTRL_OP_VSYNC_POL               BIT(18)
> > +#define  PVI_CTRL_OP_HSYNC_POL               BIT(17)
> > +#define  PVI_CTRL_OP_DE_POL          BIT(16)
> > +#define  PVI_CTRL_INP_VSYNC_POL              BIT(14)
> > +#define  PVI_CTRL_INP_HSYNC_POL              BIT(13)
> > +#define  PVI_CTRL_INP_DE_POL         BIT(12)
> > +#define  PVI_CTRL_MODE_MASK          GENMASK(2, 1)
> > +#define  PVI_CTRL_MODE_LCDIF         2
> > +#define  PVI_CTRL_EN                 BIT(0)
> > +
> > +struct imx8mp_hdmi_pvi {
> > +     struct drm_bridge       bridge;
> > +     struct device           *dev;
> > +     struct drm_bridge       *next_bridge;
> > +     void __iomem            *regs;
> > +};
> > +
> > +static inline struct imx8mp_hdmi_pvi *
> > +to_imx8mp_hdmi_pvi(struct drm_bridge *bridge)
> > +{
> > +     return container_of(bridge, struct imx8mp_hdmi_pvi, bridge);
> > +}
> > +
> > +static int imx8mp_hdmi_pvi_bridge_attach(struct drm_bridge *bridge,
> > +                                      enum drm_bridge_attach_flags flags)
> > +{
> > +     struct imx8mp_hdmi_pvi *pvi = to_imx8mp_hdmi_pvi(bridge);
> > +
> > +     return drm_bridge_attach(bridge->encoder, pvi->next_bridge,
> > +                              bridge, flags);
> > +}
> > +
> > +static void imx8mp_hdmi_pvi_bridge_enable(struct drm_bridge *bridge,
> > +                                       struct drm_bridge_state *bridge_state)
> > +{
> > +     struct drm_atomic_state *state = bridge_state->base.state;
> > +     struct imx8mp_hdmi_pvi *pvi = to_imx8mp_hdmi_pvi(bridge);
> > +     struct drm_connector_state *conn_state;
> > +     const struct drm_display_mode *mode;
> > +     struct drm_crtc_state *crtc_state;
> > +     struct drm_connector *connector;
> > +     u32 bus_flags, val;
> > +
> > +     connector = drm_atomic_get_new_connector_for_encoder(state, bridge->encoder);
> > +     conn_state = drm_atomic_get_new_connector_state(state, connector);
> > +     crtc_state = drm_atomic_get_new_crtc_state(state, conn_state->crtc);
> > +
> > +     if (WARN_ON(pm_runtime_resume_and_get(pvi->dev)))
> > +             return;
> > +
> > +     mode = &crtc_state->adjusted_mode;
> > +
> > +     val = FIELD_PREP(PVI_CTRL_MODE_MASK, PVI_CTRL_MODE_LCDIF) | PVI_CTRL_EN;
> > +
> > +     if (mode->flags & DRM_MODE_FLAG_PVSYNC)
> > +             val |= PVI_CTRL_OP_VSYNC_POL | PVI_CTRL_INP_VSYNC_POL;
> > +
> > +     if (mode->flags & DRM_MODE_FLAG_PHSYNC)
> > +             val |= PVI_CTRL_OP_HSYNC_POL | PVI_CTRL_INP_HSYNC_POL;
> > +
> > +     if (pvi->next_bridge->timings)
> > +             bus_flags = pvi->next_bridge->timings->input_bus_flags;
> > +     else if (bridge_state)
> > +             bus_flags = bridge_state->input_bus_cfg.flags;
> > +
> > +     if (bus_flags & DRM_BUS_FLAG_DE_HIGH)
> > +             val |= PVI_CTRL_OP_DE_POL | PVI_CTRL_INP_DE_POL;
> > +
> > +     writel(val, pvi->regs + HTX_PVI_CTRL);
> > +}
> > +
> > +static void imx8mp_hdmi_pvi_bridge_disable(struct drm_bridge *bridge,
> > +                                        struct drm_bridge_state *bridge_state)
> > +{
> > +     struct imx8mp_hdmi_pvi *pvi = to_imx8mp_hdmi_pvi(bridge);
> > +
> > +     writel(0x0, pvi->regs + HTX_PVI_CTRL);
> > +
> > +     pm_runtime_put(pvi->dev);
> > +}
> > +
> > +static u32 *
> > +imx8mp_hdmi_pvi_bridge_get_input_bus_fmts(struct drm_bridge *bridge,
> > +                                       struct drm_bridge_state *bridge_state,
> > +                                       struct drm_crtc_state *crtc_state,
> > +                                       struct drm_connector_state *conn_state,
> > +                                       u32 output_fmt,
> > +                                       unsigned int *num_input_fmts)
> > +{
> > +     struct imx8mp_hdmi_pvi *pvi = to_imx8mp_hdmi_pvi(bridge);
> > +     struct drm_bridge *next_bridge = pvi->next_bridge;
> > +     struct drm_bridge_state *next_state;
> > +
> > +     if (!next_bridge->funcs->atomic_get_input_bus_fmts)
> > +             return 0;
> > +
> > +     next_state = drm_atomic_get_new_bridge_state(crtc_state->state,
> > +                                                  next_bridge);
> > +
> > +     return next_bridge->funcs->atomic_get_input_bus_fmts(next_bridge,
> > +                                                          next_state,
> > +                                                          crtc_state,
> > +                                                          conn_state,
> > +                                                          output_fmt,
> > +                                                          num_input_fmts);
> > +}
> > +
> > +static const struct drm_bridge_funcs imx_hdmi_pvi_bridge_funcs = {
> > +     .attach         = imx8mp_hdmi_pvi_bridge_attach,
> > +     .atomic_enable  = imx8mp_hdmi_pvi_bridge_enable,
> > +     .atomic_disable = imx8mp_hdmi_pvi_bridge_disable,
> > +     .atomic_get_input_bus_fmts = imx8mp_hdmi_pvi_bridge_get_input_bus_fmts,
> > +     .atomic_duplicate_state = drm_atomic_helper_bridge_duplicate_state,
> > +     .atomic_destroy_state = drm_atomic_helper_bridge_destroy_state,
> > +     .atomic_reset = drm_atomic_helper_bridge_reset,
> > +};
> > +
> > +static int imx8mp_hdmi_pvi_probe(struct platform_device *pdev)
> > +{
> > +     struct device_node *remote;
> > +     struct imx8mp_hdmi_pvi *pvi;
> > +
> > +     pvi = devm_kzalloc(&pdev->dev, sizeof(*pvi), GFP_KERNEL);
> > +     if (!pvi)
> > +             return -ENOMEM;
> > +
> > +     platform_set_drvdata(pdev, pvi);
> > +     pvi->dev = &pdev->dev;
> > +
> > +     pvi->regs = devm_platform_ioremap_resource(pdev, 0);
> > +     if (IS_ERR(pvi->regs))
> > +             return PTR_ERR(pvi->regs);
> > +
> > +     /* Get the next bridge in the pipeline. */
> > +     remote = of_graph_get_remote_node(pdev->dev.of_node, 1, -1);
> > +     if (!remote)
> > +             return -EINVAL;
> > +
> > +     pvi->next_bridge = of_drm_find_bridge(remote);
> > +     of_node_put(remote);
> > +
> > +     if (!pvi->next_bridge)
> > +             return dev_err_probe(&pdev->dev, -EPROBE_DEFER,
> > +                                  "could not find next bridge\n");
> > +
> > +     /* Register the bridge. */
> > +     pvi->bridge.funcs = &imx_hdmi_pvi_bridge_funcs;
> > +     pvi->bridge.of_node = pdev->dev.of_node;
> > +     pvi->bridge.timings = pvi->next_bridge->timings;
> > +
> > +     drm_bridge_add(&pvi->bridge);
> > +
> > +     pm_runtime_enable(&pdev->dev);
>
> I would move this just before drm_bridge_add(). In theory, as soon as
> the bridge is added, it could get used, so it's a good practice to
> initialize everything before adding it.
>
> > +
> > +     return 0;
> > +}
> > +
> > +static int imx8mp_hdmi_pvi_remove(struct platform_device *pdev)
> > +{
> > +     struct imx8mp_hdmi_pvi *pvi = platform_get_drvdata(pdev);
> > +
> > +     pm_runtime_disable(&pdev->dev);
> > +
> > +     drm_bridge_remove(&pvi->bridge);
>
> And here you could flip the two as well for consistency.
>
> With these minor changes,
>
> Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

Lucas,

It's been a few months since there has been any action.  If you want,
I can help apply the suggestions that Laurent has and re-submit with
both of our names if you want.  It would be nice to get this
integrated.

adam
>
> > +
> > +     return 0;
> > +}
> > +
> > +static const struct of_device_id imx8mp_hdmi_pvi_match[] = {
> > +     {
> > +             .compatible = "fsl,imx8mp-hdmi-pvi",
> > +     }, {
> > +             /* sentinel */
> > +     }
> > +};
> > +MODULE_DEVICE_TABLE(of, imx8mp_hdmi_pvi_match);
> > +
> > +static struct platform_driver imx8mp_hdmi_pvi_driver = {
> > +     .probe  = imx8mp_hdmi_pvi_probe,
> > +     .remove = imx8mp_hdmi_pvi_remove,
> > +     .driver         = {
> > +             .name = "imx-hdmi-pvi",
> > +             .of_match_table = imx8mp_hdmi_pvi_match,
> > +     },
> > +};
> > +module_platform_driver(imx8mp_hdmi_pvi_driver);
> > +
> > +MODULE_DESCRIPTION("i.MX8MP HDMI TX Parallel Video Interface bridge driver");
> > +MODULE_LICENSE("GPL");
>
> --
> Regards,
>
> Laurent Pinchart

Re: [PATCH v3 2/2] drm/bridge: imx: add driver for HDMI TX Parallel Video Interface

[Fabio Estevam]

Hi Adam,

On Sun, Dec 10, 2023 at 2:35 PM Adam Ford <aford173@gmail.com> wrote:

> Lucas,
>
> It's been a few months since there has been any action.  If you want,
> I can help apply the suggestions that Laurent has and re-submit with
> both of our names if you want.  It would be nice to get this
> integrated.

It would be nice if you could re-submit the series.

Thanks

Re: [PATCH v3 2/2] drm/bridge: imx: add driver for HDMI TX Parallel Video Interface

[Laurent Pinchart]

On Fri, Dec 15, 2023 at 10:31:27AM -0300, Fabio Estevam wrote:
> On Sun, Dec 10, 2023 at 2:35 PM Adam Ford wrote:
> 
> > Lucas,
> >
> > It's been a few months since there has been any action.  If you want,
> > I can help apply the suggestions that Laurent has and re-submit with
> > both of our names if you want.  It would be nice to get this
> > integrated.
> 
> It would be nice if you could re-submit the series.

Yes, that would be nice. It shouldn't cause any issue, the patches will
retain Lucas' authorship.

-- 
Regards,

Laurent Pinchart

Re: [PATCH v3 2/2] drm/bridge: imx: add driver for HDMI TX Parallel Video Interface

[Adam Ford]

On Fri, Dec 15, 2023 at 8:23 AM Laurent Pinchart
<laurent.pinchart@ideasonboard.com> wrote:
>
> On Fri, Dec 15, 2023 at 10:31:27AM -0300, Fabio Estevam wrote:
> > On Sun, Dec 10, 2023 at 2:35 PM Adam Ford wrote:
> >
> > > Lucas,
> > >
> > > It's been a few months since there has been any action.  If you want,
> > > I can help apply the suggestions that Laurent has and re-submit with
> > > both of our names if you want.  It would be nice to get this
> > > integrated.
> >
> > It would be nice if you could re-submit the series.
>
> Yes, that would be nice. It shouldn't cause any issue, the patches will
> retain Lucas' authorship.

I started looking into this today, but there appears to be some
dependencies missing because the PVI is just one small portion of
this. The PVI needs to interact with the hdmi_blk_ctrl and the hdmi
transmitter itself.

It looks like there was at least one attempt to push the hdmi driver,
but we're also missing some hdmi power domain information, and the dri
patchwork lists a bunch of proposed patches for the lcdif driver.  I
haven't looked through them all, so I don't know if they are
necessary.  I found a git repo with Lucas' stuff, but it's based on
the 6.0 kernel, so it's fairly old.  Either way it seems like there is
more to the HDMI than just his one series.

adam
>
> --
> Regards,
>
> Laurent Pinchart

Re: [PATCH v3 2/2] drm/bridge: imx: add driver for HDMI TX Parallel Video Interface

[Fabio Estevam]

Hi Adam,

On Fri, Dec 15, 2023 at 1:40 PM Adam Ford <aford173@gmail.com> wrote:

> I started looking into this today, but there appears to be some
> dependencies missing because the PVI is just one small portion of
> this. The PVI needs to interact with the hdmi_blk_ctrl and the hdmi
> transmitter itself.
>
> It looks like there was at least one attempt to push the hdmi driver,
> but we're also missing some hdmi power domain information, and the dri
> patchwork lists a bunch of proposed patches for the lcdif driver.  I
> haven't looked through them all, so I don't know if they are
> necessary.  I found a git repo with Lucas' stuff, but it's based on
> the 6.0 kernel, so it's fairly old.  Either way it seems like there is
> more to the HDMI than just his one series.

Here is the whole patchset that I tested against 6.6:

https://patchwork.freedesktop.org/patch/485391/
https://patchwork.freedesktop.org/patch/485392/
https://patchwork.freedesktop.org/patch/485395/
https://patchwork.freedesktop.org/patch/515299/
https://patchwork.freedesktop.org/patch/515300/
https://patchwork.kernel.org/project/linux-arm-kernel/patch/20220406153402.1265474-12-l.stach@pengutronix.de/

Re: [PATCH v3 2/2] drm/bridge: imx: add driver for HDMI TX Parallel Video Interface

[Adam Ford]

On Fri, Dec 15, 2023 at 10:47 AM Fabio Estevam <festevam@gmail.com> wrote:
>
> Hi Adam,
>
> On Fri, Dec 15, 2023 at 1:40 PM Adam Ford <aford173@gmail.com> wrote:
>
> > I started looking into this today, but there appears to be some
> > dependencies missing because the PVI is just one small portion of
> > this. The PVI needs to interact with the hdmi_blk_ctrl and the hdmi
> > transmitter itself.
> >
> > It looks like there was at least one attempt to push the hdmi driver,
> > but we're also missing some hdmi power domain information, and the dri
> > patchwork lists a bunch of proposed patches for the lcdif driver.  I
> > haven't looked through them all, so I don't know if they are
> > necessary.  I found a git repo with Lucas' stuff, but it's based on
> > the 6.0 kernel, so it's fairly old.  Either way it seems like there is
> > more to the HDMI than just his one series.
>
> Here is the whole patchset that I tested against 6.6:
>
> https://patchwork.freedesktop.org/patch/485391/
> https://patchwork.freedesktop.org/patch/485392/
> https://patchwork.freedesktop.org/patch/485395/
> https://patchwork.freedesktop.org/patch/515299/
> https://patchwork.freedesktop.org/patch/515300/
> https://patchwork.kernel.org/project/linux-arm-kernel/patch/20220406153402.1265474-12-l.stach@pengutronix.de/

Thanks for the list.  I was able to successfully build the stable 6.6
branch with those patches applied and I have the HDMI working.
Unfortunately, I get build errors on the linux-next, so it's going to
take me a little time to sort through all of this.

I am thinking that it would be better to consolidate all these
together into one series instead of piecemealing it.  However, there
are some items that can be submitted right away without significantly
reworking them against linux-next.  Do people have a preference?

adam


adam

Re: [PATCH v3 2/2] drm/bridge: imx: add driver for HDMI TX Parallel Video Interface

[Fabio Estevam]

On Fri, Dec 15, 2023 at 4:01 PM Adam Ford <aford173@gmail.com> wrote:

> Thanks for the list.  I was able to successfully build the stable 6.6
> branch with those patches applied and I have the HDMI working.
> Unfortunately, I get build errors on the linux-next, so it's going to
> take me a little time to sort through all of this.

If you need help to figure this problem out, please let me know.

I haven't tried it against linux-next.

> I am thinking that it would be better to consolidate all these
> together into one series instead of piecemealing it.  However, there
> are some items that can be submitted right away without significantly
> reworking them against linux-next.  Do people have a preference?

I think it makes sense to re-submit the "easy ones" right away.

Re: [PATCH v3 2/2] drm/bridge: imx: add driver for HDMI TX Parallel Video Interface

[Laurent Pinchart]

On Fri, Dec 15, 2023 at 05:09:41PM -0300, Fabio Estevam wrote:
> On Fri, Dec 15, 2023 at 4:01 PM Adam Ford <aford173@gmail.com> wrote:
> 
> > Thanks for the list.  I was able to successfully build the stable 6.6
> > branch with those patches applied and I have the HDMI working.
> > Unfortunately, I get build errors on the linux-next, so it's going to
> > take me a little time to sort through all of this.
> 
> If you need help to figure this problem out, please let me know.
> 
> I haven't tried it against linux-next.
> 
> > I am thinking that it would be better to consolidate all these
> > together into one series instead of piecemealing it.  However, there
> > are some items that can be submitted right away without significantly
> > reworking them against linux-next.  Do people have a preference?
> 
> I think it makes sense to re-submit the "easy ones" right away.

Agreed. The more we can merge quickly, the easier it will become to
rebase and upstream the rest.

-- 
Regards,

Laurent Pinchart

Re: [PATCH v3 2/2] drm/bridge: imx: add driver for HDMI TX Parallel Video Interface

[Luca Ceresoli]

Hi,

On Mon, 18 Dec 2023 04:36:55 +0200
Laurent Pinchart <laurent.pinchart@ideasonboard.com> wrote:

> On Fri, Dec 15, 2023 at 05:09:41PM -0300, Fabio Estevam wrote:
> > On Fri, Dec 15, 2023 at 4:01 PM Adam Ford <aford173@gmail.com> wrote:
> >   
> > > Thanks for the list.  I was able to successfully build the stable 6.6
> > > branch with those patches applied and I have the HDMI working.
> > > Unfortunately, I get build errors on the linux-next, so it's going to
> > > take me a little time to sort through all of this.  
> > 
> > If you need help to figure this problem out, please let me know.
> > 
> > I haven't tried it against linux-next.
> >   
> > > I am thinking that it would be better to consolidate all these
> > > together into one series instead of piecemealing it.  However, there
> > > are some items that can be submitted right away without significantly
> > > reworking them against linux-next.  Do people have a preference?  
> > 
> > I think it makes sense to re-submit the "easy ones" right away.  
> 
> Agreed. The more we can merge quickly, the easier it will become to
> rebase and upstream the rest.

I agree as well, "release early, release often". These patches are
around since a long time and lots of people are using them already, and
tracking all of them from different threads is time-consuming. I will
happily test them early as soon as they are sent.

Luca

-- 
Luca Ceresoli, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

Re: [PATCH v3 2/2] drm/bridge: imx: add driver for HDMI TX Parallel Video Interface

[Alexander Stein]

Hi everybody,

Am Montag, 18. Dezember 2023, 09:48:49 CET schrieb Luca Ceresoli:
> Hi,
> 
> On Mon, 18 Dec 2023 04:36:55 +0200
> 
> Laurent Pinchart <laurent.pinchart@ideasonboard.com> wrote:
> > On Fri, Dec 15, 2023 at 05:09:41PM -0300, Fabio Estevam wrote:
> > > On Fri, Dec 15, 2023 at 4:01 PM Adam Ford <aford173@gmail.com> wrote:
> > > > Thanks for the list.  I was able to successfully build the stable 6.6
> > > > branch with those patches applied and I have the HDMI working.
> > > > Unfortunately, I get build errors on the linux-next, so it's going to
> > > > take me a little time to sort through all of this.
> > > 
> > > If you need help to figure this problem out, please let me know.
> > > 
> > > I haven't tried it against linux-next.
> > > 
> > > > I am thinking that it would be better to consolidate all these
> > > > together into one series instead of piecemealing it.  However, there
> > > > are some items that can be submitted right away without significantly
> > > > reworking them against linux-next.  Do people have a preference?
> > > 
> > > I think it makes sense to re-submit the "easy ones" right away.
> > 
> > Agreed. The more we can merge quickly, the easier it will become to
> > rebase and upstream the rest.
> 
> I agree as well, "release early, release often". These patches are
> around since a long time and lots of people are using them already, and
> tracking all of them from different threads is time-consuming. I will
> happily test them early as soon as they are sent.

I lost track of the series well, but I do remember I had to adjust the 
original series to get it running on linux-next.
Please keep me on CC so I can add my local changes if necessary.
I have a proof of concept for HDMI audio, which is based on the base HDMI 
support. I can continue on that after merge, but I'm lacking an idea how to 
add some kind of "bridge" into the audio pipeline.

Best regards
Alexander
-- 
TQ-Systems GmbH | Mühlstraße 2, Gut Delling | 82229 Seefeld, Germany
Amtsgericht München, HRB 105018
Geschäftsführer: Detlef Schneider, Rüdiger Stahl, Stefan Schneider
http://www.tq-group.com/

Re: [PATCH v3 2/2] drm/bridge: imx: add driver for HDMI TX Parallel Video Interface

[Adam Ford]

On Mon, Dec 18, 2023 at 2:59 AM Alexander Stein
<alexander.stein@ew.tq-group.com> wrote:
>
> Hi everybody,
>
> Am Montag, 18. Dezember 2023, 09:48:49 CET schrieb Luca Ceresoli:
> > Hi,
> >
> > On Mon, 18 Dec 2023 04:36:55 +0200
> >
> > Laurent Pinchart <laurent.pinchart@ideasonboard.com> wrote:
> > > On Fri, Dec 15, 2023 at 05:09:41PM -0300, Fabio Estevam wrote:
> > > > On Fri, Dec 15, 2023 at 4:01 PM Adam Ford <aford173@gmail.com> wrote:
> > > > > Thanks for the list.  I was able to successfully build the stable 6.6
> > > > > branch with those patches applied and I have the HDMI working.
> > > > > Unfortunately, I get build errors on the linux-next, so it's going to
> > > > > take me a little time to sort through all of this.
> > > >
> > > > If you need help to figure this problem out, please let me know.
> > > >
> > > > I haven't tried it against linux-next.
> > > >
> > > > > I am thinking that it would be better to consolidate all these
> > > > > together into one series instead of piecemealing it.  However, there
> > > > > are some items that can be submitted right away without significantly
> > > > > reworking them against linux-next.  Do people have a preference?
> > > >
> > > > I think it makes sense to re-submit the "easy ones" right away.
> > >
> > > Agreed. The more we can merge quickly, the easier it will become to
> > > rebase and upstream the rest.
> >
> > I agree as well, "release early, release often". These patches are
> > around since a long time and lots of people are using them already, and
> > tracking all of them from different threads is time-consuming. I will
> > happily test them early as soon as they are sent.
>
> I lost track of the series well, but I do remember I had to adjust the
> original series to get it running on linux-next.
> Please keep me on CC so I can add my local changes if necessary.

Fabio / Alexander,

I have a pending question to Peng regarding a change I pulled from NXP
[1] which moves the FDCC clock to the power domain and removes it from
the HDMI-TX driver.  I am hoping to get that answered soon.  If not, I
might just push the series again after a few more days.  In the
meantime, I have a git repo [2] if anyone wants to review stuff.
Marek Vasut sent me an offline patch to address some suspend/resume
issues, and I incorporated them into the series.  My suspend-resume
has been broken for a while, so I cannot test that.


> I have a proof of concept for HDMI audio, which is based on the base HDMI
> support. I can continue on that after merge, but I'm lacking an idea how to
> add some kind of "bridge" into the audio pipeline.

Maybe the git repo above will help.  It looks like the xcvr and
audio-tx drivers are there, but they appear to be dependent on custom
NXP sound card drivers which would be nice to replace with standard
audio drivers, so let me know if I can assist in any way.

>
> Best regards
> Alexander
> --
> TQ-Systems GmbH | Mühlstraße 2, Gut Delling | 82229 Seefeld, Germany
> Amtsgericht München, HRB 105018
> Geschäftsführer: Detlef Schneider, Rüdiger Stahl, Stefan Schneider
> http://www.tq-group.com/
>

[1] - https://patchwork.kernel.org/project/linux-pm/patch/20240106223951.387067-2-aford173@gmail.com/
[2] - https://github.com/aford173/linux/tree/for-6.9-imx8mp-hdmi

>

Re: [PATCH v3 2/2] drm/bridge: imx: add driver for HDMI TX Parallel Video Interface

[Rob Herring]

On Wed, Sep 20, 2023 at 12:10 PM Lucas Stach <l.stach@pengutronix.de> wrote:
>
> This IP block is found in the HDMI subsystem of the i.MX8MP SoC. It has a
> full timing generator and can switch between different video sources. On
> the i.MX8MP however the only supported source is the LCDIF. The block
> just needs to be powered up and told about the polarity of the video
> sync signals to act in bypass mode.
>
> Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
> Reviewed-by: Luca Ceresoli <luca.ceresoli@bootlin.com> (v2)
> Tested-by: Marek Vasut <marex@denx.de> (v1)
> Tested-by: Luca Ceresoli <luca.ceresoli@bootlin.com> (v2)
> Tested-by: Richard Leitner <richard.leitner@skidata.com> (v2)
> Tested-by: Frieder Schrempf <frieder.schrempf@kontron.de> (v2)
> ---
>  drivers/gpu/drm/bridge/imx/Kconfig           |   7 +
>  drivers/gpu/drm/bridge/imx/Makefile          |   1 +
>  drivers/gpu/drm/bridge/imx/imx8mp-hdmi-pvi.c | 206 +++++++++++++++++++
>  3 files changed, 214 insertions(+)
>  create mode 100644 drivers/gpu/drm/bridge/imx/imx8mp-hdmi-pvi.c
>
> diff --git a/drivers/gpu/drm/bridge/imx/Kconfig b/drivers/gpu/drm/bridge/imx/Kconfig
> index 9fae28db6aa7..3a4e663d922a 100644
> --- a/drivers/gpu/drm/bridge/imx/Kconfig
> +++ b/drivers/gpu/drm/bridge/imx/Kconfig
> @@ -3,6 +3,13 @@ if ARCH_MXC || COMPILE_TEST
>  config DRM_IMX_LDB_HELPER
>         tristate
>
> +config DRM_IMX8MP_HDMI_PVI
> +       tristate "Freescale i.MX8MP HDMI PVI bridge support"
> +       depends on OF
> +       help
> +         Choose this to enable support for the internal HDMI TX Parallel
> +         Video Interface found on the Freescale i.MX8MP SoC.
> +
>  config DRM_IMX8QM_LDB
>         tristate "Freescale i.MX8QM LVDS display bridge"
>         depends on OF
> diff --git a/drivers/gpu/drm/bridge/imx/Makefile b/drivers/gpu/drm/bridge/imx/Makefile
> index 8e2ebf3399a1..be9b4f9adb50 100644
> --- a/drivers/gpu/drm/bridge/imx/Makefile
> +++ b/drivers/gpu/drm/bridge/imx/Makefile
> @@ -1,4 +1,5 @@
>  obj-$(CONFIG_DRM_IMX_LDB_HELPER) += imx-ldb-helper.o
> +obj-$(CONFIG_DRM_IMX8MP_HDMI_PVI) += imx8mp-hdmi-pvi.o
>  obj-$(CONFIG_DRM_IMX8QM_LDB) += imx8qm-ldb.o
>  obj-$(CONFIG_DRM_IMX8QXP_LDB) += imx8qxp-ldb.o
>  obj-$(CONFIG_DRM_IMX8QXP_PIXEL_COMBINER) += imx8qxp-pixel-combiner.o
> diff --git a/drivers/gpu/drm/bridge/imx/imx8mp-hdmi-pvi.c b/drivers/gpu/drm/bridge/imx/imx8mp-hdmi-pvi.c
> new file mode 100644
> index 000000000000..5ccd70c98187
> --- /dev/null
> +++ b/drivers/gpu/drm/bridge/imx/imx8mp-hdmi-pvi.c
> @@ -0,0 +1,206 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +
> +/*
> + * Copyright (C) 2022 Pengutronix, Lucas Stach <kernel@pengutronix.de>
> + */
> +
> +#include <drm/drm_atomic_helper.h>
> +#include <drm/drm_bridge.h>
> +#include <drm/drm_crtc.h>
> +#include <linux/bitfield.h>
> +#include <linux/io.h>
> +#include <linux/module.h>
> +#include <linux/of_device.h>

You probably don't need this header and the implicit includes it makes
are dropped now in linux-next. Please check what you actually need and
make them explicit.

Rob

Re: [PATCH v3 1/2] dt-bindings: display: imx: add binding for i.MX8MP HDMI PVI

[Laurent Pinchart]

Hi Lucas,

Thank you for the patch.

On Wed, Sep 20, 2023 at 07:10:08PM +0200, Lucas Stach wrote:
> Add binding for the i.MX8MP HDMI parallel video interface block.
> 
> Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
> ---
>  .../display/imx/fsl,imx8mp-hdmi-pvi.yaml      | 80 +++++++++++++++++++
>  1 file changed, 80 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/display/imx/fsl,imx8mp-hdmi-pvi.yaml
> 
> diff --git a/Documentation/devicetree/bindings/display/imx/fsl,imx8mp-hdmi-pvi.yaml b/Documentation/devicetree/bindings/display/imx/fsl,imx8mp-hdmi-pvi.yaml
> new file mode 100644
> index 000000000000..12855bb3ed4c
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/display/imx/fsl,imx8mp-hdmi-pvi.yaml
> @@ -0,0 +1,80 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/display/imx/fsl,imx8mp-hdmi-pvi.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Freescale i.MX8MP HDMI Parallel Video Interface
> +
> +maintainers:
> +  - Lucas Stach <l.stach@pengutronix.de>
> +
> +description: |
> +  The HDMI parallel video interface is a timing and sync generator block in the
> +  i.MX8MP SoC, that sits between the video source and the HDMI TX controller.
> +
> +properties:
> +  compatible:
> +    const: fsl,imx8mp-hdmi-pvi
> +
> +  reg:
> +    maxItems: 1
> +
> +  power-domains:
> +    maxItems: 1
> +
> +  ports:
> +    $ref: /schemas/graph.yaml#/properties/ports
> +
> +    properties:
> +      port@0:
> +        $ref: /schemas/graph.yaml#/properties/port
> +        description: Input from the LCDIF controller.
> +
> +      port@1:
> +        $ref: /schemas/graph.yaml#/properties/port
> +        description: Output to the HDMI TX controller.
> +
> +    required:
> +      - port@0
> +      - port@1
> +
> +required:
> +  - compatible
> +  - reg
> +  - interrupts

The interrupts property is missing above. With this fixed,

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> +  - power-domains
> +  - ports
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +    #include <dt-bindings/interrupt-controller/irq.h>
> +    #include <dt-bindings/power/imx8mp-power.h>
> +
> +    display-bridge@32fc4000 {
> +        compatible = "fsl,imx8mp-hdmi-pvi";
> +        reg = <0x32fc4000 0x40>;
> +        interrupts = <12 IRQ_TYPE_LEVEL_HIGH>;
> +        power-domains = <&hdmi_blk_ctrl IMX8MP_HDMIBLK_PD_PVI>;
> +
> +        ports {
> +            #address-cells = <1>;
> +            #size-cells = <0>;
> +
> +            port@0 {
> +                reg = <0>;
> +                pvi_from_lcdif3: endpoint {
> +                    remote-endpoint = <&lcdif3_to_pvi>;
> +                };
> +            };
> +
> +            port@1 {
> +                reg = <1>;
> +                pvi_to_hdmi_tx: endpoint {
> +                    remote-endpoint = <&hdmi_tx_from_pvi>;
> +                };
> +            };
> +        };
> +    };

-- 
Regards,

Laurent Pinchart

Re: [PATCH v3 1/2] dt-bindings: display: imx: add binding for i.MX8MP HDMI PVI

[Rob Herring]

On Wed, 20 Sep 2023 19:10:08 +0200, Lucas Stach wrote:
> Add binding for the i.MX8MP HDMI parallel video interface block.
> 
> Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
> ---
>  .../display/imx/fsl,imx8mp-hdmi-pvi.yaml      | 80 +++++++++++++++++++
>  1 file changed, 80 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/display/imx/fsl,imx8mp-hdmi-pvi.yaml
> 

My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check'
on your patch (DT_CHECKER_FLAGS is new in v5.13):

yamllint warnings/errors:

dtschema/dtc warnings/errors:
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/display/imx/fsl,imx8mp-hdmi-pvi.example.dtb: display-bridge@32fc4000: 'interrupts' does not match any of the regexes: 'pinctrl-[0-9]+'
	from schema $id: http://devicetree.org/schemas/display/imx/fsl,imx8mp-hdmi-pvi.yaml#

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20230920171009.3193296-1-l.stach@pengutronix.de

The base for the series is generally the latest rc1. A different dependency
should be noted in *this* patch.

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:

pip3 install dtschema --upgrade

Please check and re-submit after running the above command yourself. Note
that DT_SCHEMA_FILES can be set to your schema file to speed up checking
your schema. However, it must be unset to test all examples with your schema.