linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 0/6] drm/meson: rework encoders to pass ATTACH_NO_CONNECTOR
@ 2021-10-15 14:11 Neil Armstrong
  2021-10-15 14:11 ` [PATCH v2 1/6] drm/bridge: display-connector: implement bus fmts callbacks Neil Armstrong
                   ` (5 more replies)
  0 siblings, 6 replies; 16+ messages in thread
From: Neil Armstrong @ 2021-10-15 14:11 UTC (permalink / raw)
  To: daniel, Laurent.pinchart, sam
  Cc: martin.blumenstingl, dri-devel, linux-amlogic, linux-arm-kernel,
	linux-kernel, Neil Armstrong

This serie finnally reworks the drm/meson driver by extracting the encoders
in their own file and moves to bridge-only callbacks.

This permits passing the ATTACH_NO_CONNECTOR bridge attach flag and finally
use the CVBS & HDMI display-connector driver.

This will ease Martin Blumenstingl writing the HDMI transceiver driver for
the older Meson8/8b SoCs, and sets the proper architecture for the work in
progress MIPI-DSI support.

Finally, this serie will path the way to a removal of the device component
and use the drmm memory management.

Changes since v1 at [1];
 - patch 1: added sam's review tag, fixed include order, fixed doc wording
 - patch 2: added sam's ack tag, switched to dev_dbg()
 - patch 3: moved mode_set to atomic_enable, removed DRM_DEBUG, fixed include order
 - patch 4: added sam's ack tag & applied to drm-misc-next
 - patch 5 & 6: added sam's ack tag
 - patch 7: added sam's review tag, stopped saving connector, moved mode_set to atomic_enable,
 	added missing atomic state callbacks, fixed include order, switched to dev_dbg/dev_err

[1] https://lore.kernel.org/r/20211014152606.2289528-1-narmstrong@baylibre.com

Neil Armstrong (6):
  drm/bridge: display-connector: implement bus fmts callbacks
  drm/meson: remove useless recursive components matching
  drm/meson: split out encoder from meson_dw_hdmi
  drm/meson: encoder_hdmi: switch to bridge
    DRM_BRIDGE_ATTACH_NO_CONNECTOR
  drm/meson: rename venc_cvbs to encoder_cvbs
  drm/meson: encoder_cvbs: switch to bridge with ATTACH_NO_CONNECTOR

 drivers/gpu/drm/bridge/display-connector.c    |  86 ++++
 drivers/gpu/drm/meson/Kconfig                 |   2 +
 drivers/gpu/drm/meson/Makefile                |   3 +-
 drivers/gpu/drm/meson/meson_drv.c             |  71 ++-
 drivers/gpu/drm/meson/meson_dw_hdmi.c         | 342 +------------
 drivers/gpu/drm/meson/meson_encoder_cvbs.c    | 291 +++++++++++
 ...meson_venc_cvbs.h => meson_encoder_cvbs.h} |   2 +-
 drivers/gpu/drm/meson/meson_encoder_hdmi.c    | 452 ++++++++++++++++++
 drivers/gpu/drm/meson/meson_encoder_hdmi.h    |  12 +
 drivers/gpu/drm/meson/meson_venc_cvbs.c       | 293 ------------
 10 files changed, 894 insertions(+), 660 deletions(-)
 create mode 100644 drivers/gpu/drm/meson/meson_encoder_cvbs.c
 rename drivers/gpu/drm/meson/{meson_venc_cvbs.h => meson_encoder_cvbs.h} (92%)
 create mode 100644 drivers/gpu/drm/meson/meson_encoder_hdmi.c
 create mode 100644 drivers/gpu/drm/meson/meson_encoder_hdmi.h
 delete mode 100644 drivers/gpu/drm/meson/meson_venc_cvbs.c


base-commit: f6632721cd6231e1bf28b5317dcc7543e43359f7
-- 
2.25.1


^ permalink raw reply	[flat|nested] 16+ messages in thread

* [PATCH v2 1/6] drm/bridge: display-connector: implement bus fmts callbacks
  2021-10-15 14:11 [PATCH v2 0/6] drm/meson: rework encoders to pass ATTACH_NO_CONNECTOR Neil Armstrong
@ 2021-10-15 14:11 ` Neil Armstrong
  2021-10-15 14:11 ` [PATCH v2 2/6] drm/meson: remove useless recursive components matching Neil Armstrong
                   ` (4 subsequent siblings)
  5 siblings, 0 replies; 16+ messages in thread
From: Neil Armstrong @ 2021-10-15 14:11 UTC (permalink / raw)
  To: daniel, Laurent.pinchart, sam, robert.foss, jonas, jernej.skrabec
  Cc: martin.blumenstingl, dri-devel, linux-amlogic, linux-arm-kernel,
	linux-kernel, Neil Armstrong

Since this bridge is tied to the connector, it acts like a passthrough,
so concerning the output & input bus formats, either pass the bus formats from the
previous bridge or return fallback data like done in the bridge function:
drm_atomic_bridge_chain_select_bus_fmts() & select_bus_fmt_recursive.

This permits avoiding skipping the negociation if the remaining bridge chain has
all the bits in place.

Without this bus fmt negociation breaks on drm/meson HDMI pipeline when attaching
dw-hdmi with DRM_BRIDGE_ATTACH_NO_CONNECTOR, because the last bridge of the
display-connector doesn't implement buf fmt callbacks and MEDIA_BUS_FMT_FIXED
is used leading to select an unsupported default bus format from dw-hdmi.

Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
Reviewed-by: Sam Ravnborg <sam@ravnborg.org>
---
 drivers/gpu/drm/bridge/display-connector.c | 86 ++++++++++++++++++++++
 1 file changed, 86 insertions(+)

diff --git a/drivers/gpu/drm/bridge/display-connector.c b/drivers/gpu/drm/bridge/display-connector.c
index 847a0dce7f1d..d24f5b90feab 100644
--- a/drivers/gpu/drm/bridge/display-connector.c
+++ b/drivers/gpu/drm/bridge/display-connector.c
@@ -13,6 +13,7 @@
 #include <linux/platform_device.h>
 #include <linux/regulator/consumer.h>
 
+#include <drm/drm_atomic_helper.h>
 #include <drm/drm_bridge.h>
 #include <drm/drm_edid.h>
 
@@ -87,10 +88,95 @@ static struct edid *display_connector_get_edid(struct drm_bridge *bridge,
 	return drm_get_edid(connector, conn->bridge.ddc);
 }
 
+/*
+ * Since this bridge is tied to the connector, it acts like a passthrough,
+ * so concerning the output bus formats, either pass the bus formats from the
+ * previous bridge or return fallback data like done in the bridge function:
+ * drm_atomic_bridge_chain_select_bus_fmts().
+ * This supports negotiation if the bridge chain has all bits in place.
+ */
+static u32 *display_connector_get_output_bus_fmts(struct drm_bridge *bridge,
+					struct drm_bridge_state *bridge_state,
+					struct drm_crtc_state *crtc_state,
+					struct drm_connector_state *conn_state,
+					unsigned int *num_output_fmts)
+{
+	struct drm_bridge *prev_bridge = drm_bridge_get_prev_bridge(bridge);
+	struct drm_bridge_state *prev_bridge_state;
+
+	if (!prev_bridge || !prev_bridge->funcs->atomic_get_output_bus_fmts) {
+		struct drm_connector *conn = conn_state->connector;
+		u32 *out_bus_fmts;
+
+		*num_output_fmts = 1;
+		out_bus_fmts = kmalloc(sizeof(*out_bus_fmts), GFP_KERNEL);
+		if (!out_bus_fmts)
+			return NULL;
+
+		if (conn->display_info.num_bus_formats &&
+		    conn->display_info.bus_formats)
+			out_bus_fmts[0] = conn->display_info.bus_formats[0];
+		else
+			out_bus_fmts[0] = MEDIA_BUS_FMT_FIXED;
+
+		return out_bus_fmts;
+	}
+
+	prev_bridge_state = drm_atomic_get_new_bridge_state(crtc_state->state,
+							    prev_bridge);
+
+	return prev_bridge->funcs->atomic_get_output_bus_fmts(prev_bridge, prev_bridge_state,
+							      crtc_state, conn_state,
+							      num_output_fmts);
+}
+
+/*
+ * Since this bridge is tied to the connector, it acts like a passthrough,
+ * so concerning the input bus formats, either pass the bus formats from the
+ * previous bridge or MEDIA_BUS_FMT_FIXED (like select_bus_fmt_recursive())
+ * when atomic_get_input_bus_fmts is not supported.
+ * This supports negotiation if the bridge chain has all bits in place.
+ */
+static u32 *display_connector_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 drm_bridge *prev_bridge = drm_bridge_get_prev_bridge(bridge);
+	struct drm_bridge_state *prev_bridge_state;
+
+	if (!prev_bridge || !prev_bridge->funcs->atomic_get_input_bus_fmts) {
+		u32 *in_bus_fmts;
+
+		*num_input_fmts = 1;
+		in_bus_fmts = kmalloc(sizeof(*in_bus_fmts), GFP_KERNEL);
+		if (!in_bus_fmts)
+			return NULL;
+
+		in_bus_fmts[0] = MEDIA_BUS_FMT_FIXED;
+
+		return in_bus_fmts;
+	}
+
+	prev_bridge_state = drm_atomic_get_new_bridge_state(crtc_state->state,
+							    prev_bridge);
+
+	return prev_bridge->funcs->atomic_get_input_bus_fmts(prev_bridge, prev_bridge_state,
+							     crtc_state, conn_state, output_fmt,
+							     num_input_fmts);
+}
+
 static const struct drm_bridge_funcs display_connector_bridge_funcs = {
 	.attach = display_connector_attach,
 	.detect = display_connector_detect,
 	.get_edid = display_connector_get_edid,
+	.atomic_get_output_bus_fmts = display_connector_get_output_bus_fmts,
+	.atomic_get_input_bus_fmts = display_connector_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 irqreturn_t display_connector_hpd_irq(int irq, void *arg)
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v2 2/6] drm/meson: remove useless recursive components matching
  2021-10-15 14:11 [PATCH v2 0/6] drm/meson: rework encoders to pass ATTACH_NO_CONNECTOR Neil Armstrong
  2021-10-15 14:11 ` [PATCH v2 1/6] drm/bridge: display-connector: implement bus fmts callbacks Neil Armstrong
@ 2021-10-15 14:11 ` Neil Armstrong
  2021-10-15 21:59   ` Martin Blumenstingl
  2021-10-15 14:11 ` [PATCH v2 3/6] drm/meson: split out encoder from meson_dw_hdmi Neil Armstrong
                   ` (3 subsequent siblings)
  5 siblings, 1 reply; 16+ messages in thread
From: Neil Armstrong @ 2021-10-15 14:11 UTC (permalink / raw)
  To: daniel, Laurent.pinchart, sam
  Cc: martin.blumenstingl, dri-devel, linux-amlogic, linux-arm-kernel,
	linux-kernel, Neil Armstrong

The initial design was recursive to cover all port/endpoints, but only the first layer
of endpoints should be covered by the components list.
This also breaks the MIPI-DSI init/bridge attach sequence, thus only parse the
first endpoints instead of recursing.

Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
Acked-by: Sam Ravnborg <sam@ravnborg.org>
---
 drivers/gpu/drm/meson/meson_drv.c | 62 +++++++++++--------------------
 1 file changed, 21 insertions(+), 41 deletions(-)

diff --git a/drivers/gpu/drm/meson/meson_drv.c b/drivers/gpu/drm/meson/meson_drv.c
index 7f41a33592c8..97ebc07357bb 100644
--- a/drivers/gpu/drm/meson/meson_drv.c
+++ b/drivers/gpu/drm/meson/meson_drv.c
@@ -426,46 +426,6 @@ static int compare_of(struct device *dev, void *data)
 	return dev->of_node == data;
 }
 
-/* Possible connectors nodes to ignore */
-static const struct of_device_id connectors_match[] = {
-	{ .compatible = "composite-video-connector" },
-	{ .compatible = "svideo-connector" },
-	{ .compatible = "hdmi-connector" },
-	{ .compatible = "dvi-connector" },
-	{}
-};
-
-static int meson_probe_remote(struct platform_device *pdev,
-			      struct component_match **match,
-			      struct device_node *parent,
-			      struct device_node *remote)
-{
-	struct device_node *ep, *remote_node;
-	int count = 1;
-
-	/* If node is a connector, return and do not add to match table */
-	if (of_match_node(connectors_match, remote))
-		return 1;
-
-	component_match_add(&pdev->dev, match, compare_of, remote);
-
-	for_each_endpoint_of_node(remote, ep) {
-		remote_node = of_graph_get_remote_port_parent(ep);
-		if (!remote_node ||
-		    remote_node == parent || /* Ignore parent endpoint */
-		    !of_device_is_available(remote_node)) {
-			of_node_put(remote_node);
-			continue;
-		}
-
-		count += meson_probe_remote(pdev, match, remote, remote_node);
-
-		of_node_put(remote_node);
-	}
-
-	return count;
-}
-
 static void meson_drv_shutdown(struct platform_device *pdev)
 {
 	struct meson_drm *priv = dev_get_drvdata(&pdev->dev);
@@ -477,6 +437,13 @@ static void meson_drv_shutdown(struct platform_device *pdev)
 	drm_atomic_helper_shutdown(priv->drm);
 }
 
+/* Possible connectors nodes to ignore */
+static const struct of_device_id connectors_match[] = {
+	{ .compatible = "composite-video-connector" },
+	{ .compatible = "svideo-connector" },
+	{}
+};
+
 static int meson_drv_probe(struct platform_device *pdev)
 {
 	struct component_match *match = NULL;
@@ -491,8 +458,21 @@ static int meson_drv_probe(struct platform_device *pdev)
 			continue;
 		}
 
-		count += meson_probe_remote(pdev, &match, np, remote);
+		/* If an analog connector is detected, count it as an output */
+		if (of_match_node(connectors_match, remote)) {
+			++count;
+			of_node_put(remote);
+			continue;
+		}
+
+		dev_dbg(&pdev->dev, "parent %pOF remote match add %pOF parent %s\n",
+			np, remote, dev_name(&pdev->dev));
+
+		component_match_add(&pdev->dev, &match, compare_of, remote);
+
 		of_node_put(remote);
+
+		++count;
 	}
 
 	if (count && !match)
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v2 3/6] drm/meson: split out encoder from meson_dw_hdmi
  2021-10-15 14:11 [PATCH v2 0/6] drm/meson: rework encoders to pass ATTACH_NO_CONNECTOR Neil Armstrong
  2021-10-15 14:11 ` [PATCH v2 1/6] drm/bridge: display-connector: implement bus fmts callbacks Neil Armstrong
  2021-10-15 14:11 ` [PATCH v2 2/6] drm/meson: remove useless recursive components matching Neil Armstrong
@ 2021-10-15 14:11 ` Neil Armstrong
  2021-10-15 22:01   ` Martin Blumenstingl
  2021-10-18 13:58   ` Laurent Pinchart
  2021-10-15 14:11 ` [PATCH v2 4/6] drm/meson: encoder_hdmi: switch to bridge DRM_BRIDGE_ATTACH_NO_CONNECTOR Neil Armstrong
                   ` (2 subsequent siblings)
  5 siblings, 2 replies; 16+ messages in thread
From: Neil Armstrong @ 2021-10-15 14:11 UTC (permalink / raw)
  To: daniel, Laurent.pinchart, sam
  Cc: martin.blumenstingl, dri-devel, linux-amlogic, linux-arm-kernel,
	linux-kernel, Neil Armstrong

This moves all the non-DW-HDMI code where it should be:
an encoder in the drm/meson core driver.

The bridge functions are copied as-is, except:
- the encoder init uses the simple kms helper
- the mode_set has been moved to atomic_enable()
- debug prints are converted to dev_debg()

For now the bridge attach flags is 0, DRM_BRIDGE_ATTACH_NO_CONNECTOR
will be handled later.

The meson dw-hdmi glue is slighly fixed to live without the
encoder in the same driver.

Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
---
 drivers/gpu/drm/meson/Makefile             |   1 +
 drivers/gpu/drm/meson/meson_drv.c          |   5 +
 drivers/gpu/drm/meson/meson_dw_hdmi.c      | 341 ++-----------------
 drivers/gpu/drm/meson/meson_encoder_hdmi.c | 375 +++++++++++++++++++++
 drivers/gpu/drm/meson/meson_encoder_hdmi.h |  12 +
 5 files changed, 412 insertions(+), 322 deletions(-)
 create mode 100644 drivers/gpu/drm/meson/meson_encoder_hdmi.c
 create mode 100644 drivers/gpu/drm/meson/meson_encoder_hdmi.h

diff --git a/drivers/gpu/drm/meson/Makefile b/drivers/gpu/drm/meson/Makefile
index 28a519cdf66b..523fce45f16b 100644
--- a/drivers/gpu/drm/meson/Makefile
+++ b/drivers/gpu/drm/meson/Makefile
@@ -2,6 +2,7 @@
 meson-drm-y := meson_drv.o meson_plane.o meson_crtc.o meson_venc_cvbs.o
 meson-drm-y += meson_viu.o meson_vpp.o meson_venc.o meson_vclk.o meson_overlay.o
 meson-drm-y += meson_rdma.o meson_osd_afbcd.o
+meson-drm-y += meson_encoder_hdmi.o
 
 obj-$(CONFIG_DRM_MESON) += meson-drm.o
 obj-$(CONFIG_DRM_MESON_DW_HDMI) += meson_dw_hdmi.o
diff --git a/drivers/gpu/drm/meson/meson_drv.c b/drivers/gpu/drm/meson/meson_drv.c
index 97ebc07357bb..0978b440f336 100644
--- a/drivers/gpu/drm/meson/meson_drv.c
+++ b/drivers/gpu/drm/meson/meson_drv.c
@@ -32,6 +32,7 @@
 #include "meson_osd_afbcd.h"
 #include "meson_registers.h"
 #include "meson_venc_cvbs.h"
+#include "meson_encoder_hdmi.h"
 #include "meson_viu.h"
 #include "meson_vpp.h"
 #include "meson_rdma.h"
@@ -318,6 +319,10 @@ static int meson_drv_bind_master(struct device *dev, bool has_components)
 		}
 	}
 
+	ret = meson_encoder_hdmi_init(priv);
+	if (ret)
+		goto free_drm;
+
 	ret = meson_plane_create(priv);
 	if (ret)
 		goto free_drm;
diff --git a/drivers/gpu/drm/meson/meson_dw_hdmi.c b/drivers/gpu/drm/meson/meson_dw_hdmi.c
index 0afbd1e70bfc..fb540a503efe 100644
--- a/drivers/gpu/drm/meson/meson_dw_hdmi.c
+++ b/drivers/gpu/drm/meson/meson_dw_hdmi.c
@@ -22,14 +22,11 @@
 #include <drm/drm_probe_helper.h>
 #include <drm/drm_print.h>
 
-#include <linux/media-bus-format.h>
 #include <linux/videodev2.h>
 
 #include "meson_drv.h"
 #include "meson_dw_hdmi.h"
 #include "meson_registers.h"
-#include "meson_vclk.h"
-#include "meson_venc.h"
 
 #define DRIVER_NAME "meson-dw-hdmi"
 #define DRIVER_DESC "Amlogic Meson HDMI-TX DRM driver"
@@ -135,8 +132,6 @@ struct meson_dw_hdmi_data {
 };
 
 struct meson_dw_hdmi {
-	struct drm_encoder encoder;
-	struct drm_bridge bridge;
 	struct dw_hdmi_plat_data dw_plat_data;
 	struct meson_drm *priv;
 	struct device *dev;
@@ -148,12 +143,8 @@ struct meson_dw_hdmi {
 	struct regulator *hdmi_supply;
 	u32 irq_stat;
 	struct dw_hdmi *hdmi;
-	unsigned long output_bus_fmt;
+	struct drm_bridge *bridge;
 };
-#define encoder_to_meson_dw_hdmi(x) \
-	container_of(x, struct meson_dw_hdmi, encoder)
-#define bridge_to_meson_dw_hdmi(x) \
-	container_of(x, struct meson_dw_hdmi, bridge)
 
 static inline int dw_hdmi_is_compatible(struct meson_dw_hdmi *dw_hdmi,
 					const char *compat)
@@ -295,14 +286,14 @@ static inline void dw_hdmi_dwc_write_bits(struct meson_dw_hdmi *dw_hdmi,
 
 /* Setup PHY bandwidth modes */
 static void meson_hdmi_phy_setup_mode(struct meson_dw_hdmi *dw_hdmi,
-				      const struct drm_display_mode *mode)
+				      const struct drm_display_mode *mode,
+				      bool mode_is_420)
 {
 	struct meson_drm *priv = dw_hdmi->priv;
 	unsigned int pixel_clock = mode->clock;
 
 	/* For 420, pixel clock is half unlike venc clock */
-	if (dw_hdmi->output_bus_fmt == MEDIA_BUS_FMT_UYYVYY8_0_5X24)
-		pixel_clock /= 2;
+	if (mode_is_420) pixel_clock /= 2;
 
 	if (dw_hdmi_is_compatible(dw_hdmi, "amlogic,meson-gxl-dw-hdmi") ||
 	    dw_hdmi_is_compatible(dw_hdmi, "amlogic,meson-gxm-dw-hdmi")) {
@@ -374,68 +365,25 @@ static inline void meson_dw_hdmi_phy_reset(struct meson_dw_hdmi *dw_hdmi)
 	mdelay(2);
 }
 
-static void dw_hdmi_set_vclk(struct meson_dw_hdmi *dw_hdmi,
-			     const struct drm_display_mode *mode)
-{
-	struct meson_drm *priv = dw_hdmi->priv;
-	int vic = drm_match_cea_mode(mode);
-	unsigned int phy_freq;
-	unsigned int vclk_freq;
-	unsigned int venc_freq;
-	unsigned int hdmi_freq;
-
-	vclk_freq = mode->clock;
-
-	/* For 420, pixel clock is half unlike venc clock */
-	if (dw_hdmi->output_bus_fmt == MEDIA_BUS_FMT_UYYVYY8_0_5X24)
-		vclk_freq /= 2;
-
-	/* TMDS clock is pixel_clock * 10 */
-	phy_freq = vclk_freq * 10;
-
-	if (!vic) {
-		meson_vclk_setup(priv, MESON_VCLK_TARGET_DMT, phy_freq,
-				 vclk_freq, vclk_freq, vclk_freq, false);
-		return;
-	}
-
-	/* 480i/576i needs global pixel doubling */
-	if (mode->flags & DRM_MODE_FLAG_DBLCLK)
-		vclk_freq *= 2;
-
-	venc_freq = vclk_freq;
-	hdmi_freq = vclk_freq;
-
-	/* VENC double pixels for 1080i, 720p and YUV420 modes */
-	if (meson_venc_hdmi_venc_repeat(vic) ||
-	    dw_hdmi->output_bus_fmt == MEDIA_BUS_FMT_UYYVYY8_0_5X24)
-		venc_freq *= 2;
-
-	vclk_freq = max(venc_freq, hdmi_freq);
-
-	if (mode->flags & DRM_MODE_FLAG_DBLCLK)
-		venc_freq /= 2;
-
-	DRM_DEBUG_DRIVER("vclk:%d phy=%d venc=%d hdmi=%d enci=%d\n",
-		phy_freq, vclk_freq, venc_freq, hdmi_freq,
-		priv->venc.hdmi_use_enci);
-
-	meson_vclk_setup(priv, MESON_VCLK_TARGET_HDMI, phy_freq, vclk_freq,
-			 venc_freq, hdmi_freq, priv->venc.hdmi_use_enci);
-}
-
 static int dw_hdmi_phy_init(struct dw_hdmi *hdmi, void *data,
 			    const struct drm_display_info *display,
 			    const struct drm_display_mode *mode)
 {
 	struct meson_dw_hdmi *dw_hdmi = (struct meson_dw_hdmi *)data;
+	bool is_hdmi2_sink = display->hdmi.scdc.supported;
 	struct meson_drm *priv = dw_hdmi->priv;
 	unsigned int wr_clk =
 		readl_relaxed(priv->io_base + _REG(VPU_HDMI_SETTING));
+	bool mode_is_420 = false;
 
 	DRM_DEBUG_DRIVER("\"%s\" div%d\n", mode->name,
 			 mode->clock > 340000 ? 40 : 10);
 
+	if (drm_mode_is_420_only(display, mode) ||
+	    (!is_hdmi2_sink &&
+	     drm_mode_is_420_also(display, mode)))
+		mode_is_420 = true;
+
 	/* Enable clocks */
 	regmap_update_bits(priv->hhi, HHI_HDMI_CLK_CNTL, 0xffff, 0x100);
 
@@ -457,8 +405,7 @@ static int dw_hdmi_phy_init(struct dw_hdmi *hdmi, void *data,
 	dw_hdmi->data->top_write(dw_hdmi, HDMITX_TOP_BIST_CNTL, BIT(12));
 
 	/* TMDS pattern setup */
-	if (mode->clock > 340000 &&
-	    dw_hdmi->output_bus_fmt == MEDIA_BUS_FMT_YUV8_1X24) {
+	if (mode->clock > 340000 && !mode_is_420) {
 		dw_hdmi->data->top_write(dw_hdmi, HDMITX_TOP_TMDS_CLK_PTTN_01,
 				  0);
 		dw_hdmi->data->top_write(dw_hdmi, HDMITX_TOP_TMDS_CLK_PTTN_23,
@@ -476,7 +423,7 @@ static int dw_hdmi_phy_init(struct dw_hdmi *hdmi, void *data,
 	dw_hdmi->data->top_write(dw_hdmi, HDMITX_TOP_TMDS_CLK_PTTN_CNTL, 0x2);
 
 	/* Setup PHY parameters */
-	meson_hdmi_phy_setup_mode(dw_hdmi, mode);
+	meson_hdmi_phy_setup_mode(dw_hdmi, mode, mode_is_420);
 
 	/* Setup PHY */
 	regmap_update_bits(priv->hhi, HHI_HDMI_PHY_CNTL1,
@@ -622,214 +569,15 @@ static irqreturn_t dw_hdmi_top_thread_irq(int irq, void *dev_id)
 		dw_hdmi_setup_rx_sense(dw_hdmi->hdmi, hpd_connected,
 				       hpd_connected);
 
-		drm_helper_hpd_irq_event(dw_hdmi->encoder.dev);
+		drm_helper_hpd_irq_event(dw_hdmi->bridge->dev);
+		drm_bridge_hpd_notify(dw_hdmi->bridge,
+				      hpd_connected ? connector_status_connected
+						    : connector_status_disconnected);
 	}
 
 	return IRQ_HANDLED;
 }
 
-static enum drm_mode_status
-dw_hdmi_mode_valid(struct dw_hdmi *hdmi, void *data,
-		   const struct drm_display_info *display_info,
-		   const struct drm_display_mode *mode)
-{
-	struct meson_dw_hdmi *dw_hdmi = data;
-	struct meson_drm *priv = dw_hdmi->priv;
-	bool is_hdmi2_sink = display_info->hdmi.scdc.supported;
-	unsigned int phy_freq;
-	unsigned int vclk_freq;
-	unsigned int venc_freq;
-	unsigned int hdmi_freq;
-	int vic = drm_match_cea_mode(mode);
-	enum drm_mode_status status;
-
-	DRM_DEBUG_DRIVER("Modeline " DRM_MODE_FMT "\n", DRM_MODE_ARG(mode));
-
-	/* If sink does not support 540MHz, reject the non-420 HDMI2 modes */
-	if (display_info->max_tmds_clock &&
-	    mode->clock > display_info->max_tmds_clock &&
-	    !drm_mode_is_420_only(display_info, mode) &&
-	    !drm_mode_is_420_also(display_info, mode))
-		return MODE_BAD;
-
-	/* Check against non-VIC supported modes */
-	if (!vic) {
-		status = meson_venc_hdmi_supported_mode(mode);
-		if (status != MODE_OK)
-			return status;
-
-		return meson_vclk_dmt_supported_freq(priv, mode->clock);
-	/* Check against supported VIC modes */
-	} else if (!meson_venc_hdmi_supported_vic(vic))
-		return MODE_BAD;
-
-	vclk_freq = mode->clock;
-
-	/* For 420, pixel clock is half unlike venc clock */
-	if (drm_mode_is_420_only(display_info, mode) ||
-	    (!is_hdmi2_sink &&
-	     drm_mode_is_420_also(display_info, mode)))
-		vclk_freq /= 2;
-
-	/* TMDS clock is pixel_clock * 10 */
-	phy_freq = vclk_freq * 10;
-
-	/* 480i/576i needs global pixel doubling */
-	if (mode->flags & DRM_MODE_FLAG_DBLCLK)
-		vclk_freq *= 2;
-
-	venc_freq = vclk_freq;
-	hdmi_freq = vclk_freq;
-
-	/* VENC double pixels for 1080i, 720p and YUV420 modes */
-	if (meson_venc_hdmi_venc_repeat(vic) ||
-	    drm_mode_is_420_only(display_info, mode) ||
-	    (!is_hdmi2_sink &&
-	     drm_mode_is_420_also(display_info, mode)))
-		venc_freq *= 2;
-
-	vclk_freq = max(venc_freq, hdmi_freq);
-
-	if (mode->flags & DRM_MODE_FLAG_DBLCLK)
-		venc_freq /= 2;
-
-	dev_dbg(dw_hdmi->dev, "%s: vclk:%d phy=%d venc=%d hdmi=%d\n",
-		__func__, phy_freq, vclk_freq, venc_freq, hdmi_freq);
-
-	return meson_vclk_vic_supported_freq(priv, phy_freq, vclk_freq);
-}
-
-/* Encoder */
-
-static const u32 meson_dw_hdmi_out_bus_fmts[] = {
-	MEDIA_BUS_FMT_YUV8_1X24,
-	MEDIA_BUS_FMT_UYYVYY8_0_5X24,
-};
-
-static void meson_venc_hdmi_encoder_destroy(struct drm_encoder *encoder)
-{
-	drm_encoder_cleanup(encoder);
-}
-
-static const struct drm_encoder_funcs meson_venc_hdmi_encoder_funcs = {
-	.destroy        = meson_venc_hdmi_encoder_destroy,
-};
-
-static u32 *
-meson_venc_hdmi_encoder_get_inp_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)
-{
-	u32 *input_fmts = NULL;
-	int i;
-
-	*num_input_fmts = 0;
-
-	for (i = 0 ; i < ARRAY_SIZE(meson_dw_hdmi_out_bus_fmts) ; ++i) {
-		if (output_fmt == meson_dw_hdmi_out_bus_fmts[i]) {
-			*num_input_fmts = 1;
-			input_fmts = kcalloc(*num_input_fmts,
-					     sizeof(*input_fmts),
-					     GFP_KERNEL);
-			if (!input_fmts)
-				return NULL;
-
-			input_fmts[0] = output_fmt;
-
-			break;
-		}
-	}
-
-	return input_fmts;
-}
-
-static int meson_venc_hdmi_encoder_atomic_check(struct drm_bridge *bridge,
-					struct drm_bridge_state *bridge_state,
-					struct drm_crtc_state *crtc_state,
-					struct drm_connector_state *conn_state)
-{
-	struct meson_dw_hdmi *dw_hdmi = bridge_to_meson_dw_hdmi(bridge);
-
-	dw_hdmi->output_bus_fmt = bridge_state->output_bus_cfg.format;
-
-	DRM_DEBUG_DRIVER("output_bus_fmt %lx\n", dw_hdmi->output_bus_fmt);
-
-	return 0;
-}
-
-static void meson_venc_hdmi_encoder_disable(struct drm_bridge *bridge)
-{
-	struct meson_dw_hdmi *dw_hdmi = bridge_to_meson_dw_hdmi(bridge);
-	struct meson_drm *priv = dw_hdmi->priv;
-
-	DRM_DEBUG_DRIVER("\n");
-
-	writel_bits_relaxed(0x3, 0,
-			    priv->io_base + _REG(VPU_HDMI_SETTING));
-
-	writel_relaxed(0, priv->io_base + _REG(ENCI_VIDEO_EN));
-	writel_relaxed(0, priv->io_base + _REG(ENCP_VIDEO_EN));
-}
-
-static void meson_venc_hdmi_encoder_enable(struct drm_bridge *bridge)
-{
-	struct meson_dw_hdmi *dw_hdmi = bridge_to_meson_dw_hdmi(bridge);
-	struct meson_drm *priv = dw_hdmi->priv;
-
-	DRM_DEBUG_DRIVER("%s\n", priv->venc.hdmi_use_enci ? "VENCI" : "VENCP");
-
-	if (priv->venc.hdmi_use_enci)
-		writel_relaxed(1, priv->io_base + _REG(ENCI_VIDEO_EN));
-	else
-		writel_relaxed(1, priv->io_base + _REG(ENCP_VIDEO_EN));
-}
-
-static void meson_venc_hdmi_encoder_mode_set(struct drm_bridge *bridge,
-				   const struct drm_display_mode *mode,
-				   const struct drm_display_mode *adjusted_mode)
-{
-	struct meson_dw_hdmi *dw_hdmi = bridge_to_meson_dw_hdmi(bridge);
-	struct meson_drm *priv = dw_hdmi->priv;
-	int vic = drm_match_cea_mode(mode);
-	unsigned int ycrcb_map = VPU_HDMI_OUTPUT_CBYCR;
-	bool yuv420_mode = false;
-
-	DRM_DEBUG_DRIVER("\"%s\" vic %d\n", mode->name, vic);
-
-	if (dw_hdmi->output_bus_fmt == MEDIA_BUS_FMT_UYYVYY8_0_5X24) {
-		ycrcb_map = VPU_HDMI_OUTPUT_CRYCB;
-		yuv420_mode = true;
-	}
-
-	/* VENC + VENC-DVI Mode setup */
-	meson_venc_hdmi_mode_set(priv, vic, ycrcb_map, yuv420_mode, mode);
-
-	/* VCLK Set clock */
-	dw_hdmi_set_vclk(dw_hdmi, mode);
-
-	if (dw_hdmi->output_bus_fmt == MEDIA_BUS_FMT_UYYVYY8_0_5X24)
-		/* Setup YUV420 to HDMI-TX, no 10bit diphering */
-		writel_relaxed(2 | (2 << 2),
-			       priv->io_base + _REG(VPU_HDMI_FMT_CTRL));
-	else
-		/* Setup YUV444 to HDMI-TX, no 10bit diphering */
-		writel_relaxed(0, priv->io_base + _REG(VPU_HDMI_FMT_CTRL));
-}
-
-static const struct drm_bridge_funcs meson_venc_hdmi_encoder_bridge_funcs = {
-	.atomic_duplicate_state = drm_atomic_helper_bridge_duplicate_state,
-	.atomic_destroy_state = drm_atomic_helper_bridge_destroy_state,
-	.atomic_get_input_bus_fmts = meson_venc_hdmi_encoder_get_inp_bus_fmts,
-	.atomic_reset = drm_atomic_helper_bridge_reset,
-	.atomic_check = meson_venc_hdmi_encoder_atomic_check,
-	.enable	= meson_venc_hdmi_encoder_enable,
-	.disable = meson_venc_hdmi_encoder_disable,
-	.mode_set = meson_venc_hdmi_encoder_mode_set,
-};
-
 /* DW HDMI Regmap */
 
 static int meson_dw_hdmi_reg_read(void *context, unsigned int reg,
@@ -876,28 +624,6 @@ static const struct meson_dw_hdmi_data meson_dw_hdmi_g12a_data = {
 	.dwc_write = dw_hdmi_g12a_dwc_write,
 };
 
-static bool meson_hdmi_connector_is_available(struct device *dev)
-{
-	struct device_node *ep, *remote;
-
-	/* HDMI Connector is on the second port, first endpoint */
-	ep = of_graph_get_endpoint_by_regs(dev->of_node, 1, 0);
-	if (!ep)
-		return false;
-
-	/* If the endpoint node exists, consider it enabled */
-	remote = of_graph_get_remote_port(ep);
-	if (remote) {
-		of_node_put(ep);
-		return true;
-	}
-
-	of_node_put(ep);
-	of_node_put(remote);
-
-	return false;
-}
-
 static void meson_dw_hdmi_init(struct meson_dw_hdmi *meson_dw_hdmi)
 {
 	struct meson_drm *priv = meson_dw_hdmi->priv;
@@ -976,18 +702,11 @@ static int meson_dw_hdmi_bind(struct device *dev, struct device *master,
 	struct drm_device *drm = data;
 	struct meson_drm *priv = drm->dev_private;
 	struct dw_hdmi_plat_data *dw_plat_data;
-	struct drm_bridge *next_bridge;
-	struct drm_encoder *encoder;
 	int irq;
 	int ret;
 
 	DRM_DEBUG_DRIVER("\n");
 
-	if (!meson_hdmi_connector_is_available(dev)) {
-		dev_info(drm->dev, "HDMI Output connector not available\n");
-		return -ENODEV;
-	}
-
 	match = of_device_get_match_data(&pdev->dev);
 	if (!match) {
 		dev_err(&pdev->dev, "failed to get match data\n");
@@ -1003,7 +722,6 @@ static int meson_dw_hdmi_bind(struct device *dev, struct device *master,
 	meson_dw_hdmi->dev = dev;
 	meson_dw_hdmi->data = match;
 	dw_plat_data = &meson_dw_hdmi->dw_plat_data;
-	encoder = &meson_dw_hdmi->encoder;
 
 	meson_dw_hdmi->hdmi_supply = devm_regulator_get_optional(dev, "hdmi");
 	if (IS_ERR(meson_dw_hdmi->hdmi_supply)) {
@@ -1074,28 +792,11 @@ static int meson_dw_hdmi_bind(struct device *dev, struct device *master,
 		return ret;
 	}
 
-	/* Encoder */
-
-	ret = drm_encoder_init(drm, encoder, &meson_venc_hdmi_encoder_funcs,
-			       DRM_MODE_ENCODER_TMDS, "meson_hdmi");
-	if (ret) {
-		dev_err(priv->dev, "Failed to init HDMI encoder\n");
-		return ret;
-	}
-
-	meson_dw_hdmi->bridge.funcs = &meson_venc_hdmi_encoder_bridge_funcs;
-	drm_bridge_attach(encoder, &meson_dw_hdmi->bridge, NULL, 0);
-
-	encoder->possible_crtcs = BIT(0);
-
 	meson_dw_hdmi_init(meson_dw_hdmi);
 
-	DRM_DEBUG_DRIVER("encoder initialized\n");
-
 	/* Bridge / Connector */
 
 	dw_plat_data->priv_data = meson_dw_hdmi;
-	dw_plat_data->mode_valid = dw_hdmi_mode_valid;
 	dw_plat_data->phy_ops = &meson_dw_hdmi_phy_ops;
 	dw_plat_data->phy_name = "meson_dw_hdmi_phy";
 	dw_plat_data->phy_data = meson_dw_hdmi;
@@ -1110,15 +811,11 @@ static int meson_dw_hdmi_bind(struct device *dev, struct device *master,
 
 	platform_set_drvdata(pdev, meson_dw_hdmi);
 
-	meson_dw_hdmi->hdmi = dw_hdmi_probe(pdev,
-					    &meson_dw_hdmi->dw_plat_data);
+	meson_dw_hdmi->hdmi = dw_hdmi_probe(pdev, &meson_dw_hdmi->dw_plat_data);
 	if (IS_ERR(meson_dw_hdmi->hdmi))
 		return PTR_ERR(meson_dw_hdmi->hdmi);
 
-	next_bridge = of_drm_find_bridge(pdev->dev.of_node);
-	if (next_bridge)
-		drm_bridge_attach(encoder, next_bridge,
-				  &meson_dw_hdmi->bridge, 0);
+	meson_dw_hdmi->bridge = of_drm_find_bridge(pdev->dev.of_node);
 
 	DRM_DEBUG_DRIVER("HDMI controller initialized\n");
 
diff --git a/drivers/gpu/drm/meson/meson_encoder_hdmi.c b/drivers/gpu/drm/meson/meson_encoder_hdmi.c
new file mode 100644
index 000000000000..971da662c954
--- /dev/null
+++ b/drivers/gpu/drm/meson/meson_encoder_hdmi.c
@@ -0,0 +1,375 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright (C) 2016 BayLibre, SAS
+ * Author: Neil Armstrong <narmstrong@baylibre.com>
+ * Copyright (C) 2015 Amlogic, Inc. All rights reserved.
+ */
+
+#include <linux/clk.h>
+#include <linux/component.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/of_device.h>
+#include <linux/of_graph.h>
+#include <linux/regulator/consumer.h>
+#include <linux/reset.h>
+
+#include <drm/drm_atomic_helper.h>
+#include <drm/drm_bridge.h>
+#include <drm/drm_device.h>
+#include <drm/drm_edid.h>
+#include <drm/drm_probe_helper.h>
+#include <drm/drm_simple_kms_helper.h>
+
+#include <linux/media-bus-format.h>
+#include <linux/videodev2.h>
+
+#include "meson_drv.h"
+#include "meson_registers.h"
+#include "meson_vclk.h"
+#include "meson_venc.h"
+
+struct meson_encoder_hdmi {
+	struct drm_encoder encoder;
+	struct drm_bridge bridge;
+	struct drm_bridge *next_bridge;
+	struct meson_drm *priv;
+	unsigned long output_bus_fmt;
+};
+
+#define bridge_to_meson_encoder_hdmi(x) \
+	container_of(x, struct meson_encoder_hdmi, bridge)
+
+static int meson_encoder_hdmi_attach(struct drm_bridge *bridge,
+				     enum drm_bridge_attach_flags flags)
+{
+	struct meson_encoder_hdmi *encoder_hdmi = bridge_to_meson_encoder_hdmi(bridge);
+
+	return drm_bridge_attach(bridge->encoder, encoder_hdmi->next_bridge,
+				 &encoder_hdmi->bridge, flags);
+}
+
+static void meson_encoder_hdmi_enable(struct drm_bridge *bridge)
+{
+	struct meson_encoder_hdmi *encoder_hdmi = bridge_to_meson_encoder_hdmi(bridge);
+	struct meson_drm *priv = encoder_hdmi->priv;
+
+	dev_dbg(priv->dev, "%s\n", priv->venc.hdmi_use_enci ? "VENCI" : "VENCP");
+
+	if (priv->venc.hdmi_use_enci)
+		writel_relaxed(1, priv->io_base + _REG(ENCI_VIDEO_EN));
+	else
+		writel_relaxed(1, priv->io_base + _REG(ENCP_VIDEO_EN));
+}
+
+static void meson_encoder_hdmi_disable(struct drm_bridge *bridge)
+{
+	struct meson_encoder_hdmi *encoder_hdmi = bridge_to_meson_encoder_hdmi(bridge);
+	struct meson_drm *priv = encoder_hdmi->priv;
+
+	writel_bits_relaxed(0x3, 0,
+			    priv->io_base + _REG(VPU_HDMI_SETTING));
+
+	writel_relaxed(0, priv->io_base + _REG(ENCI_VIDEO_EN));
+	writel_relaxed(0, priv->io_base + _REG(ENCP_VIDEO_EN));
+}
+
+static void meson_encoder_hdmi_set_vclk(struct meson_encoder_hdmi *encoder_hdmi,
+					const struct drm_display_mode *mode)
+{
+	struct meson_drm *priv = encoder_hdmi->priv;
+	int vic = drm_match_cea_mode(mode);
+	unsigned int phy_freq;
+	unsigned int vclk_freq;
+	unsigned int venc_freq;
+	unsigned int hdmi_freq;
+
+	vclk_freq = mode->clock;
+
+	/* For 420, pixel clock is half unlike venc clock */
+	if (encoder_hdmi->output_bus_fmt == MEDIA_BUS_FMT_UYYVYY8_0_5X24)
+		vclk_freq /= 2;
+
+	/* TMDS clock is pixel_clock * 10 */
+	phy_freq = vclk_freq * 10;
+
+	if (!vic) {
+		meson_vclk_setup(priv, MESON_VCLK_TARGET_DMT, phy_freq,
+				 vclk_freq, vclk_freq, vclk_freq, false);
+		return;
+	}
+
+	/* 480i/576i needs global pixel doubling */
+	if (mode->flags & DRM_MODE_FLAG_DBLCLK)
+		vclk_freq *= 2;
+
+	venc_freq = vclk_freq;
+	hdmi_freq = vclk_freq;
+
+	/* VENC double pixels for 1080i, 720p and YUV420 modes */
+	if (meson_venc_hdmi_venc_repeat(vic) ||
+	    encoder_hdmi->output_bus_fmt == MEDIA_BUS_FMT_UYYVYY8_0_5X24)
+		venc_freq *= 2;
+
+	vclk_freq = max(venc_freq, hdmi_freq);
+
+	if (mode->flags & DRM_MODE_FLAG_DBLCLK)
+		venc_freq /= 2;
+
+	dev_dbg(priv->dev, "vclk:%d phy=%d venc=%d hdmi=%d enci=%d\n",
+		phy_freq, vclk_freq, venc_freq, hdmi_freq,
+		priv->venc.hdmi_use_enci);
+
+	meson_vclk_setup(priv, MESON_VCLK_TARGET_HDMI, phy_freq, vclk_freq,
+			 venc_freq, hdmi_freq, priv->venc.hdmi_use_enci);
+}
+
+static enum drm_mode_status meson_encoder_hdmi_mode_valid(struct drm_bridge *bridge,
+					const struct drm_display_info *display_info,
+					const struct drm_display_mode *mode)
+{
+	struct meson_encoder_hdmi *encoder_hdmi = bridge_to_meson_encoder_hdmi(bridge);
+	struct meson_drm *priv = encoder_hdmi->priv;
+	bool is_hdmi2_sink = display_info->hdmi.scdc.supported;
+	unsigned int phy_freq;
+	unsigned int vclk_freq;
+	unsigned int venc_freq;
+	unsigned int hdmi_freq;
+	int vic = drm_match_cea_mode(mode);
+	enum drm_mode_status status;
+
+	dev_dbg(priv->dev, "Modeline " DRM_MODE_FMT "\n", DRM_MODE_ARG(mode));
+
+	/* If sink does not support 540MHz, reject the non-420 HDMI2 modes */
+	if (display_info->max_tmds_clock &&
+	    mode->clock > display_info->max_tmds_clock &&
+	    !drm_mode_is_420_only(display_info, mode) &&
+	    !drm_mode_is_420_also(display_info, mode))
+		return MODE_BAD;
+
+	/* Check against non-VIC supported modes */
+	if (!vic) {
+		status = meson_venc_hdmi_supported_mode(mode);
+		if (status != MODE_OK)
+			return status;
+
+		return meson_vclk_dmt_supported_freq(priv, mode->clock);
+	/* Check against supported VIC modes */
+	} else if (!meson_venc_hdmi_supported_vic(vic))
+		return MODE_BAD;
+
+	vclk_freq = mode->clock;
+
+	/* For 420, pixel clock is half unlike venc clock */
+	if (drm_mode_is_420_only(display_info, mode) ||
+	    (!is_hdmi2_sink &&
+	     drm_mode_is_420_also(display_info, mode)))
+		vclk_freq /= 2;
+
+	/* TMDS clock is pixel_clock * 10 */
+	phy_freq = vclk_freq * 10;
+
+	/* 480i/576i needs global pixel doubling */
+	if (mode->flags & DRM_MODE_FLAG_DBLCLK)
+		vclk_freq *= 2;
+
+	venc_freq = vclk_freq;
+	hdmi_freq = vclk_freq;
+
+	/* VENC double pixels for 1080i, 720p and YUV420 modes */
+	if (meson_venc_hdmi_venc_repeat(vic) ||
+	    drm_mode_is_420_only(display_info, mode) ||
+	    (!is_hdmi2_sink &&
+	     drm_mode_is_420_also(display_info, mode)))
+		venc_freq *= 2;
+
+	vclk_freq = max(venc_freq, hdmi_freq);
+
+	if (mode->flags & DRM_MODE_FLAG_DBLCLK)
+		venc_freq /= 2;
+
+	dev_dbg(priv->dev, "%s: vclk:%d phy=%d venc=%d hdmi=%d\n",
+		__func__, phy_freq, vclk_freq, venc_freq, hdmi_freq);
+
+	return meson_vclk_vic_supported_freq(priv, phy_freq, vclk_freq);
+}
+
+static void meson_encoder_hdmi_atomic_enable(struct drm_bridge *bridge,
+					     struct drm_bridge_state *bridge_state)
+{
+	struct meson_encoder_hdmi *encoder_hdmi = bridge_to_meson_encoder_hdmi(bridge);
+	struct drm_atomic_state *state = bridge_state->base.state;
+	unsigned int ycrcb_map = VPU_HDMI_OUTPUT_CBYCR;
+	struct meson_drm *priv = encoder_hdmi->priv;
+	struct drm_connector_state *conn_state;
+	const struct drm_display_mode *mode;
+	struct drm_crtc_state *crtc_state;
+	struct drm_connector *connector;
+	bool yuv420_mode = false;
+	int vic;
+
+	connector = drm_atomic_get_new_connector_for_encoder(state, bridge->encoder);
+	if (WARN_ON(!connector))
+		return;
+
+	conn_state = drm_atomic_get_new_connector_state(state, connector);
+	if (WARN_ON(!conn_state))
+		return;
+
+	crtc_state = drm_atomic_get_new_crtc_state(state, conn_state->crtc);
+	if (WARN_ON(!crtc_state))
+		return;
+
+	mode = &crtc_state->adjusted_mode;
+
+	vic = drm_match_cea_mode(mode);
+
+	dev_dbg(priv->dev, "\"%s\" vic %d\n", mode->name, vic);
+
+	if (encoder_hdmi->output_bus_fmt == MEDIA_BUS_FMT_UYYVYY8_0_5X24) {
+		ycrcb_map = VPU_HDMI_OUTPUT_CRYCB;
+		yuv420_mode = true;
+	}
+
+	/* VENC + VENC-DVI Mode setup */
+	meson_venc_hdmi_mode_set(priv, vic, ycrcb_map, yuv420_mode, mode);
+
+	/* VCLK Set clock */
+	meson_encoder_hdmi_set_vclk(encoder_hdmi, mode);
+
+	if (encoder_hdmi->output_bus_fmt == MEDIA_BUS_FMT_UYYVYY8_0_5X24)
+		/* Setup YUV420 to HDMI-TX, no 10bit diphering */
+		writel_relaxed(2 | (2 << 2),
+			       priv->io_base + _REG(VPU_HDMI_FMT_CTRL));
+	else
+		/* Setup YUV444 to HDMI-TX, no 10bit diphering */
+		writel_relaxed(0, priv->io_base + _REG(VPU_HDMI_FMT_CTRL));
+}
+
+static const u32 meson_encoder_hdmi_out_bus_fmts[] = {
+	MEDIA_BUS_FMT_YUV8_1X24,
+	MEDIA_BUS_FMT_UYYVYY8_0_5X24,
+};
+
+static u32 *
+meson_encoder_hdmi_get_inp_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)
+{
+	u32 *input_fmts = NULL;
+	int i;
+
+	*num_input_fmts = 0;
+
+	for (i = 0 ; i < ARRAY_SIZE(meson_encoder_hdmi_out_bus_fmts) ; ++i) {
+		if (output_fmt == meson_encoder_hdmi_out_bus_fmts[i]) {
+			*num_input_fmts = 1;
+			input_fmts = kcalloc(*num_input_fmts,
+					     sizeof(*input_fmts),
+					     GFP_KERNEL);
+			if (!input_fmts)
+				return NULL;
+
+			input_fmts[0] = output_fmt;
+
+			break;
+		}
+	}
+
+	return input_fmts;
+}
+
+static int meson_encoder_hdmi_atomic_check(struct drm_bridge *bridge,
+					struct drm_bridge_state *bridge_state,
+					struct drm_crtc_state *crtc_state,
+					struct drm_connector_state *conn_state)
+{
+	struct meson_encoder_hdmi *encoder_hdmi = bridge_to_meson_encoder_hdmi(bridge);
+	struct drm_connector_state *old_conn_state =
+		drm_atomic_get_old_connector_state(conn_state->state, conn_state->connector);
+	struct meson_drm *priv = encoder_hdmi->priv;
+
+	encoder_hdmi->output_bus_fmt = bridge_state->output_bus_cfg.format;
+
+	dev_dbg(priv->dev, "output_bus_fmt %lx\n", encoder_hdmi->output_bus_fmt);
+
+	if (!drm_connector_atomic_hdr_metadata_equal(old_conn_state, conn_state))
+		crtc_state->mode_changed = true;
+
+	return 0;
+}
+
+static const struct drm_bridge_funcs meson_encoder_hdmi_bridge_funcs = {
+	.attach = meson_encoder_hdmi_attach,
+	.enable	= meson_encoder_hdmi_enable,
+	.disable = meson_encoder_hdmi_disable,
+	.mode_valid = meson_encoder_hdmi_mode_valid,
+	.atomic_enable = meson_encoder_hdmi_atomic_enable,
+	.atomic_get_input_bus_fmts = meson_encoder_hdmi_get_inp_bus_fmts,
+	.atomic_check = meson_encoder_hdmi_atomic_check,
+	.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,
+};
+
+int meson_encoder_hdmi_init(struct meson_drm *priv)
+{
+	struct meson_encoder_hdmi *meson_encoder_hdmi;
+	struct device_node *remote;
+	int ret;
+
+	meson_encoder_hdmi = devm_kzalloc(priv->dev, sizeof(*meson_encoder_hdmi), GFP_KERNEL);
+	if (!meson_encoder_hdmi)
+		return -ENOMEM;
+
+	/* HDMI Transceiver Bridge */
+	remote = of_graph_get_remote_node(priv->dev->of_node, 1, 0);
+	if (!remote) {
+		dev_err(priv->dev, "HDMI transceiver device is disabled");
+		return 0;
+	}
+
+	meson_encoder_hdmi->next_bridge = of_drm_find_bridge(remote);
+	if (!meson_encoder_hdmi->next_bridge) {
+		dev_err(priv->dev, "Failed to find HDMI transceiver bridge\n");
+		return -EPROBE_DEFER;
+	}
+
+	/* HDMI Encoder Bridge */
+	meson_encoder_hdmi->bridge.funcs = &meson_encoder_hdmi_bridge_funcs;
+	meson_encoder_hdmi->bridge.of_node = priv->dev->of_node;
+	meson_encoder_hdmi->bridge.type = DRM_MODE_CONNECTOR_HDMIA;
+
+	drm_bridge_add(&meson_encoder_hdmi->bridge);
+
+	meson_encoder_hdmi->priv = priv;
+
+	/* Encoder */
+	ret = drm_simple_encoder_init(priv->drm, &meson_encoder_hdmi->encoder,
+				      DRM_MODE_ENCODER_TMDS);
+	if (ret) {
+		dev_err(priv->dev, "Failed to init HDMI encoder: %d\n", ret);
+		return ret;
+	}
+
+	meson_encoder_hdmi->encoder.possible_crtcs = BIT(0);
+
+	/* Attach HDMI Encoder Bridge to Encoder */
+	ret = drm_bridge_attach(&meson_encoder_hdmi->encoder, &meson_encoder_hdmi->bridge, NULL, 0);
+	if (ret) {
+		dev_err(priv->dev, "Failed to attach bridge: %d\n", ret);
+		return ret;
+	}
+
+	/*
+	 * We should have now in place:
+	 * encoder->[hdmi encoder bridge]->[dw-hdmi bridge]->[dw-hdmi connector]
+	 */
+
+	dev_dbg(priv->dev, "HDMI encoder initialized\n");
+
+	return 0;
+}
diff --git a/drivers/gpu/drm/meson/meson_encoder_hdmi.h b/drivers/gpu/drm/meson/meson_encoder_hdmi.h
new file mode 100644
index 000000000000..ed19494f0956
--- /dev/null
+++ b/drivers/gpu/drm/meson/meson_encoder_hdmi.h
@@ -0,0 +1,12 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+/*
+ * Copyright (C) 2021 BayLibre, SAS
+ * Author: Neil Armstrong <narmstrong@baylibre.com>
+ */
+
+#ifndef __MESON_ENCODER_HDMI_H
+#define __MESON_ENCODER_HDMI_H
+
+int meson_encoder_hdmi_init(struct meson_drm *priv);
+
+#endif /* __MESON_ENCODER_HDMI_H */
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v2 4/6] drm/meson: encoder_hdmi: switch to bridge DRM_BRIDGE_ATTACH_NO_CONNECTOR
  2021-10-15 14:11 [PATCH v2 0/6] drm/meson: rework encoders to pass ATTACH_NO_CONNECTOR Neil Armstrong
                   ` (2 preceding siblings ...)
  2021-10-15 14:11 ` [PATCH v2 3/6] drm/meson: split out encoder from meson_dw_hdmi Neil Armstrong
@ 2021-10-15 14:11 ` Neil Armstrong
  2021-10-15 22:07   ` Martin Blumenstingl
  2021-10-15 14:11 ` [PATCH v2 5/6] drm/meson: rename venc_cvbs to encoder_cvbs Neil Armstrong
  2021-10-15 14:11 ` [PATCH v2 6/6] drm/meson: encoder_cvbs: switch to bridge with ATTACH_NO_CONNECTOR Neil Armstrong
  5 siblings, 1 reply; 16+ messages in thread
From: Neil Armstrong @ 2021-10-15 14:11 UTC (permalink / raw)
  To: daniel, Laurent.pinchart, sam
  Cc: martin.blumenstingl, dri-devel, linux-amlogic, linux-arm-kernel,
	linux-kernel, Neil Armstrong

This implements the necessary change to no more use the embedded
connector in dw-hdmi and use the dedicated bridge connector driver
by passing DRM_BRIDGE_ATTACH_NO_CONNECTOR to the bridge attach call.

The necessary connector properties are added to handle the same
functionalities as the embedded dw-hdmi connector, i.e. the HDR
metadata, the CEC notifier & other flags.

The dw-hdmi output_port is set to 1 in order to look for a connector
next bridge in order to get DRM_BRIDGE_ATTACH_NO_CONNECTOR working.

Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
Acked-by: Sam Ravnborg <sam@ravnborg.org>
---
 drivers/gpu/drm/meson/Kconfig              |  2 +
 drivers/gpu/drm/meson/meson_dw_hdmi.c      |  1 +
 drivers/gpu/drm/meson/meson_encoder_hdmi.c | 81 +++++++++++++++++++++-
 3 files changed, 82 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/meson/Kconfig b/drivers/gpu/drm/meson/Kconfig
index 9f9281dd49f8..a4e1ed96e5e8 100644
--- a/drivers/gpu/drm/meson/Kconfig
+++ b/drivers/gpu/drm/meson/Kconfig
@@ -6,9 +6,11 @@ config DRM_MESON
 	select DRM_KMS_HELPER
 	select DRM_KMS_CMA_HELPER
 	select DRM_GEM_CMA_HELPER
+	select DRM_DISPLAY_CONNECTOR
 	select VIDEOMODE_HELPERS
 	select REGMAP_MMIO
 	select MESON_CANVAS
+	select CEC_CORE if CEC_NOTIFIER
 
 config DRM_MESON_DW_HDMI
 	tristate "HDMI Synopsys Controller support for Amlogic Meson Display"
diff --git a/drivers/gpu/drm/meson/meson_dw_hdmi.c b/drivers/gpu/drm/meson/meson_dw_hdmi.c
index fb540a503efe..5cd2b2ebbbd3 100644
--- a/drivers/gpu/drm/meson/meson_dw_hdmi.c
+++ b/drivers/gpu/drm/meson/meson_dw_hdmi.c
@@ -803,6 +803,7 @@ static int meson_dw_hdmi_bind(struct device *dev, struct device *master,
 	dw_plat_data->input_bus_encoding = V4L2_YCBCR_ENC_709;
 	dw_plat_data->ycbcr_420_allowed = true;
 	dw_plat_data->disable_cec = true;
+	dw_plat_data->output_port = 1;
 
 	if (dw_hdmi_is_compatible(meson_dw_hdmi, "amlogic,meson-gxl-dw-hdmi") ||
 	    dw_hdmi_is_compatible(meson_dw_hdmi, "amlogic,meson-gxm-dw-hdmi") ||
diff --git a/drivers/gpu/drm/meson/meson_encoder_hdmi.c b/drivers/gpu/drm/meson/meson_encoder_hdmi.c
index 971da662c954..32f52f1c423b 100644
--- a/drivers/gpu/drm/meson/meson_encoder_hdmi.c
+++ b/drivers/gpu/drm/meson/meson_encoder_hdmi.c
@@ -14,8 +14,11 @@
 #include <linux/regulator/consumer.h>
 #include <linux/reset.h>
 
+#include <media/cec-notifier.h>
+
 #include <drm/drm_atomic_helper.h>
 #include <drm/drm_bridge.h>
+#include <drm/drm_bridge_connector.h>
 #include <drm/drm_device.h>
 #include <drm/drm_edid.h>
 #include <drm/drm_probe_helper.h>
@@ -33,8 +36,10 @@ struct meson_encoder_hdmi {
 	struct drm_encoder encoder;
 	struct drm_bridge bridge;
 	struct drm_bridge *next_bridge;
+	struct drm_connector *connector;
 	struct meson_drm *priv;
 	unsigned long output_bus_fmt;
+	struct cec_notifier *cec_notifier;
 };
 
 #define bridge_to_meson_encoder_hdmi(x) \
@@ -49,6 +54,14 @@ static int meson_encoder_hdmi_attach(struct drm_bridge *bridge,
 				 &encoder_hdmi->bridge, flags);
 }
 
+static void meson_encoder_hdmi_detach(struct drm_bridge *bridge)
+{
+	struct meson_encoder_hdmi *encoder_hdmi = bridge_to_meson_encoder_hdmi(bridge);
+
+	cec_notifier_conn_unregister(encoder_hdmi->cec_notifier);
+	encoder_hdmi->cec_notifier = NULL;
+}
+
 static void meson_encoder_hdmi_enable(struct drm_bridge *bridge)
 {
 	struct meson_encoder_hdmi *encoder_hdmi = bridge_to_meson_encoder_hdmi(bridge);
@@ -302,11 +315,32 @@ static int meson_encoder_hdmi_atomic_check(struct drm_bridge *bridge,
 	return 0;
 }
 
+static void meson_encoder_hdmi_hpd_notify(struct drm_bridge *bridge,
+					  enum drm_connector_status status)
+{
+	struct meson_encoder_hdmi *encoder_hdmi = bridge_to_meson_encoder_hdmi(bridge);
+	struct edid *edid;
+
+	if (!encoder_hdmi->cec_notifier)
+		return;
+
+	if (status == connector_status_connected) {
+		edid = drm_bridge_get_edid(encoder_hdmi->next_bridge, encoder_hdmi->connector);
+		if (!edid)
+			return;
+
+		cec_notifier_set_phys_addr_from_edid(encoder_hdmi->cec_notifier, edid);
+	} else
+		cec_notifier_phys_addr_invalidate(encoder_hdmi->cec_notifier);
+}
+
 static const struct drm_bridge_funcs meson_encoder_hdmi_bridge_funcs = {
 	.attach = meson_encoder_hdmi_attach,
+	.detach = meson_encoder_hdmi_detach,
 	.enable	= meson_encoder_hdmi_enable,
 	.disable = meson_encoder_hdmi_disable,
 	.mode_valid = meson_encoder_hdmi_mode_valid,
+	.hpd_notify = meson_encoder_hdmi_hpd_notify,
 	.atomic_enable = meson_encoder_hdmi_atomic_enable,
 	.atomic_get_input_bus_fmts = meson_encoder_hdmi_get_inp_bus_fmts,
 	.atomic_check = meson_encoder_hdmi_atomic_check,
@@ -318,6 +352,7 @@ static const struct drm_bridge_funcs meson_encoder_hdmi_bridge_funcs = {
 int meson_encoder_hdmi_init(struct meson_drm *priv)
 {
 	struct meson_encoder_hdmi *meson_encoder_hdmi;
+	struct platform_device *pdev;
 	struct device_node *remote;
 	int ret;
 
@@ -342,6 +377,7 @@ int meson_encoder_hdmi_init(struct meson_drm *priv)
 	meson_encoder_hdmi->bridge.funcs = &meson_encoder_hdmi_bridge_funcs;
 	meson_encoder_hdmi->bridge.of_node = priv->dev->of_node;
 	meson_encoder_hdmi->bridge.type = DRM_MODE_CONNECTOR_HDMIA;
+	meson_encoder_hdmi->bridge.interlace_allowed = true;
 
 	drm_bridge_add(&meson_encoder_hdmi->bridge);
 
@@ -358,17 +394,58 @@ int meson_encoder_hdmi_init(struct meson_drm *priv)
 	meson_encoder_hdmi->encoder.possible_crtcs = BIT(0);
 
 	/* Attach HDMI Encoder Bridge to Encoder */
-	ret = drm_bridge_attach(&meson_encoder_hdmi->encoder, &meson_encoder_hdmi->bridge, NULL, 0);
+	ret = drm_bridge_attach(&meson_encoder_hdmi->encoder, &meson_encoder_hdmi->bridge, NULL,
+				DRM_BRIDGE_ATTACH_NO_CONNECTOR);
 	if (ret) {
 		dev_err(priv->dev, "Failed to attach bridge: %d\n", ret);
 		return ret;
 	}
 
+	/* Initialize & attach Bridge Connector */
+	meson_encoder_hdmi->connector = drm_bridge_connector_init(priv->drm,
+							&meson_encoder_hdmi->encoder);
+	if (IS_ERR(meson_encoder_hdmi->connector)) {
+		dev_err(priv->dev, "Unable to create HDMI bridge connector\n");
+		return PTR_ERR(meson_encoder_hdmi->connector);
+	}
+	drm_connector_attach_encoder(meson_encoder_hdmi->connector,
+				     &meson_encoder_hdmi->encoder);
+
 	/*
 	 * We should have now in place:
-	 * encoder->[hdmi encoder bridge]->[dw-hdmi bridge]->[dw-hdmi connector]
+	 * encoder->[hdmi encoder bridge]->[dw-hdmi bridge]->[display connector bridge]->[display connector]
 	 */
 
+	/*
+	 * drm_connector_attach_max_bpc_property() requires the
+	 * connector to have a state.
+	 */
+	drm_atomic_helper_connector_reset(meson_encoder_hdmi->connector);
+
+	if (meson_vpu_is_compatible(priv, VPU_COMPATIBLE_GXL) ||
+	    meson_vpu_is_compatible(priv, VPU_COMPATIBLE_GXM) ||
+	    meson_vpu_is_compatible(priv, VPU_COMPATIBLE_G12A))
+		drm_connector_attach_hdr_output_metadata_property(meson_encoder_hdmi->connector);
+
+	drm_connector_attach_max_bpc_property(meson_encoder_hdmi->connector, 8, 8);
+
+	/* Handle this here until handled by drm_bridge_connector_init() */
+	meson_encoder_hdmi->connector->ycbcr_420_allowed = true;
+
+	pdev = of_find_device_by_node(remote);
+	if (pdev) {
+		struct cec_connector_info conn_info;
+		struct cec_notifier *notifier;
+
+		cec_fill_conn_info_from_drm(&conn_info, meson_encoder_hdmi->connector);
+
+		notifier = cec_notifier_conn_register(&pdev->dev, NULL, &conn_info);
+		if (!notifier)
+			return -ENOMEM;
+
+		meson_encoder_hdmi->cec_notifier = notifier;
+	}
+
 	dev_dbg(priv->dev, "HDMI encoder initialized\n");
 
 	return 0;
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v2 5/6] drm/meson: rename venc_cvbs to encoder_cvbs
  2021-10-15 14:11 [PATCH v2 0/6] drm/meson: rework encoders to pass ATTACH_NO_CONNECTOR Neil Armstrong
                   ` (3 preceding siblings ...)
  2021-10-15 14:11 ` [PATCH v2 4/6] drm/meson: encoder_hdmi: switch to bridge DRM_BRIDGE_ATTACH_NO_CONNECTOR Neil Armstrong
@ 2021-10-15 14:11 ` Neil Armstrong
  2021-10-15 22:07   ` Martin Blumenstingl
  2021-10-15 14:11 ` [PATCH v2 6/6] drm/meson: encoder_cvbs: switch to bridge with ATTACH_NO_CONNECTOR Neil Armstrong
  5 siblings, 1 reply; 16+ messages in thread
From: Neil Armstrong @ 2021-10-15 14:11 UTC (permalink / raw)
  To: daniel, Laurent.pinchart, sam
  Cc: martin.blumenstingl, dri-devel, linux-amlogic, linux-arm-kernel,
	linux-kernel, Neil Armstrong

Rename the cvbs encoder to match the newly introduced meson_encoder_hdmi.

Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
Acked-by: Sam Ravnborg <sam@ravnborg.org>
---
 drivers/gpu/drm/meson/Makefile                |  2 +-
 drivers/gpu/drm/meson/meson_drv.c             |  4 +-
 ...meson_venc_cvbs.c => meson_encoder_cvbs.c} | 78 +++++++++----------
 ...meson_venc_cvbs.h => meson_encoder_cvbs.h} |  2 +-
 4 files changed, 43 insertions(+), 43 deletions(-)
 rename drivers/gpu/drm/meson/{meson_venc_cvbs.c => meson_encoder_cvbs.c} (74%)
 rename drivers/gpu/drm/meson/{meson_venc_cvbs.h => meson_encoder_cvbs.h} (92%)

diff --git a/drivers/gpu/drm/meson/Makefile b/drivers/gpu/drm/meson/Makefile
index 523fce45f16b..3afa31bdc950 100644
--- a/drivers/gpu/drm/meson/Makefile
+++ b/drivers/gpu/drm/meson/Makefile
@@ -1,5 +1,5 @@
 # SPDX-License-Identifier: GPL-2.0-only
-meson-drm-y := meson_drv.o meson_plane.o meson_crtc.o meson_venc_cvbs.o
+meson-drm-y := meson_drv.o meson_plane.o meson_crtc.o meson_encoder_cvbs.o
 meson-drm-y += meson_viu.o meson_vpp.o meson_venc.o meson_vclk.o meson_overlay.o
 meson-drm-y += meson_rdma.o meson_osd_afbcd.o
 meson-drm-y += meson_encoder_hdmi.o
diff --git a/drivers/gpu/drm/meson/meson_drv.c b/drivers/gpu/drm/meson/meson_drv.c
index 0978b440f336..80f1d439841a 100644
--- a/drivers/gpu/drm/meson/meson_drv.c
+++ b/drivers/gpu/drm/meson/meson_drv.c
@@ -31,7 +31,7 @@
 #include "meson_plane.h"
 #include "meson_osd_afbcd.h"
 #include "meson_registers.h"
-#include "meson_venc_cvbs.h"
+#include "meson_encoder_cvbs.h"
 #include "meson_encoder_hdmi.h"
 #include "meson_viu.h"
 #include "meson_vpp.h"
@@ -307,7 +307,7 @@ static int meson_drv_bind_master(struct device *dev, bool has_components)
 
 	/* Encoder Initialization */
 
-	ret = meson_venc_cvbs_create(priv);
+	ret = meson_encoder_cvbs_init(priv);
 	if (ret)
 		goto free_drm;
 
diff --git a/drivers/gpu/drm/meson/meson_venc_cvbs.c b/drivers/gpu/drm/meson/meson_encoder_cvbs.c
similarity index 74%
rename from drivers/gpu/drm/meson/meson_venc_cvbs.c
rename to drivers/gpu/drm/meson/meson_encoder_cvbs.c
index f1747fde1fe0..01024c5f610c 100644
--- a/drivers/gpu/drm/meson/meson_venc_cvbs.c
+++ b/drivers/gpu/drm/meson/meson_encoder_cvbs.c
@@ -20,7 +20,7 @@
 
 #include "meson_registers.h"
 #include "meson_vclk.h"
-#include "meson_venc_cvbs.h"
+#include "meson_encoder_cvbs.h"
 
 /* HHI VDAC Registers */
 #define HHI_VDAC_CNTL0		0x2F4 /* 0xbd offset in data sheet */
@@ -28,16 +28,16 @@
 #define HHI_VDAC_CNTL1		0x2F8 /* 0xbe offset in data sheet */
 #define HHI_VDAC_CNTL1_G12A	0x2F0 /* 0xbe offset in data sheet */
 
-struct meson_venc_cvbs {
+struct meson_encoder_cvbs {
 	struct drm_encoder	encoder;
 	struct drm_connector	connector;
 	struct meson_drm	*priv;
 };
-#define encoder_to_meson_venc_cvbs(x) \
-	container_of(x, struct meson_venc_cvbs, encoder)
+#define encoder_to_meson_encoder_cvbs(x) \
+	container_of(x, struct meson_encoder_cvbs, encoder)
 
-#define connector_to_meson_venc_cvbs(x) \
-	container_of(x, struct meson_venc_cvbs, connector)
+#define connector_to_meson_encoder_cvbs(x) \
+	container_of(x, struct meson_encoder_cvbs, connector)
 
 /* Supported Modes */
 
@@ -140,16 +140,16 @@ struct drm_connector_helper_funcs meson_cvbs_connector_helper_funcs = {
 
 /* Encoder */
 
-static void meson_venc_cvbs_encoder_destroy(struct drm_encoder *encoder)
+static void meson_encoder_cvbs_encoder_destroy(struct drm_encoder *encoder)
 {
 	drm_encoder_cleanup(encoder);
 }
 
-static const struct drm_encoder_funcs meson_venc_cvbs_encoder_funcs = {
-	.destroy        = meson_venc_cvbs_encoder_destroy,
+static const struct drm_encoder_funcs meson_encoder_cvbs_encoder_funcs = {
+	.destroy        = meson_encoder_cvbs_encoder_destroy,
 };
 
-static int meson_venc_cvbs_encoder_atomic_check(struct drm_encoder *encoder,
+static int meson_encoder_cvbs_encoder_atomic_check(struct drm_encoder *encoder,
 					struct drm_crtc_state *crtc_state,
 					struct drm_connector_state *conn_state)
 {
@@ -159,11 +159,11 @@ static int meson_venc_cvbs_encoder_atomic_check(struct drm_encoder *encoder,
 	return -EINVAL;
 }
 
-static void meson_venc_cvbs_encoder_disable(struct drm_encoder *encoder)
+static void meson_encoder_cvbs_encoder_disable(struct drm_encoder *encoder)
 {
-	struct meson_venc_cvbs *meson_venc_cvbs =
-					encoder_to_meson_venc_cvbs(encoder);
-	struct meson_drm *priv = meson_venc_cvbs->priv;
+	struct meson_encoder_cvbs *meson_encoder_cvbs =
+					encoder_to_meson_encoder_cvbs(encoder);
+	struct meson_drm *priv = meson_encoder_cvbs->priv;
 
 	/* Disable CVBS VDAC */
 	if (meson_vpu_is_compatible(priv, VPU_COMPATIBLE_G12A)) {
@@ -175,11 +175,11 @@ static void meson_venc_cvbs_encoder_disable(struct drm_encoder *encoder)
 	}
 }
 
-static void meson_venc_cvbs_encoder_enable(struct drm_encoder *encoder)
+static void meson_encoder_cvbs_encoder_enable(struct drm_encoder *encoder)
 {
-	struct meson_venc_cvbs *meson_venc_cvbs =
-					encoder_to_meson_venc_cvbs(encoder);
-	struct meson_drm *priv = meson_venc_cvbs->priv;
+	struct meson_encoder_cvbs *meson_encoder_cvbs =
+					encoder_to_meson_encoder_cvbs(encoder);
+	struct meson_drm *priv = meson_encoder_cvbs->priv;
 
 	/* VDAC0 source is not from ATV */
 	writel_bits_relaxed(VENC_VDAC_SEL_ATV_DMD, 0,
@@ -198,14 +198,14 @@ static void meson_venc_cvbs_encoder_enable(struct drm_encoder *encoder)
 	}
 }
 
-static void meson_venc_cvbs_encoder_mode_set(struct drm_encoder *encoder,
+static void meson_encoder_cvbs_encoder_mode_set(struct drm_encoder *encoder,
 				   struct drm_display_mode *mode,
 				   struct drm_display_mode *adjusted_mode)
 {
 	const struct meson_cvbs_mode *meson_mode = meson_cvbs_get_mode(mode);
-	struct meson_venc_cvbs *meson_venc_cvbs =
-					encoder_to_meson_venc_cvbs(encoder);
-	struct meson_drm *priv = meson_venc_cvbs->priv;
+	struct meson_encoder_cvbs *meson_encoder_cvbs =
+					encoder_to_meson_encoder_cvbs(encoder);
+	struct meson_drm *priv = meson_encoder_cvbs->priv;
 
 	if (meson_mode) {
 		meson_venci_cvbs_mode_set(priv, meson_mode->enci);
@@ -219,14 +219,14 @@ static void meson_venc_cvbs_encoder_mode_set(struct drm_encoder *encoder,
 }
 
 static const struct drm_encoder_helper_funcs
-				meson_venc_cvbs_encoder_helper_funcs = {
-	.atomic_check	= meson_venc_cvbs_encoder_atomic_check,
-	.disable	= meson_venc_cvbs_encoder_disable,
-	.enable		= meson_venc_cvbs_encoder_enable,
-	.mode_set	= meson_venc_cvbs_encoder_mode_set,
+				meson_encoder_cvbs_encoder_helper_funcs = {
+	.atomic_check	= meson_encoder_cvbs_encoder_atomic_check,
+	.disable	= meson_encoder_cvbs_encoder_disable,
+	.enable		= meson_encoder_cvbs_encoder_enable,
+	.mode_set	= meson_encoder_cvbs_encoder_mode_set,
 };
 
-static bool meson_venc_cvbs_connector_is_available(struct meson_drm *priv)
+static bool meson_encoder_cvbs_connector_is_available(struct meson_drm *priv)
 {
 	struct device_node *remote;
 
@@ -238,27 +238,27 @@ static bool meson_venc_cvbs_connector_is_available(struct meson_drm *priv)
 	return true;
 }
 
-int meson_venc_cvbs_create(struct meson_drm *priv)
+int meson_encoder_cvbs_init(struct meson_drm *priv)
 {
 	struct drm_device *drm = priv->drm;
-	struct meson_venc_cvbs *meson_venc_cvbs;
+	struct meson_encoder_cvbs *meson_encoder_cvbs;
 	struct drm_connector *connector;
 	struct drm_encoder *encoder;
 	int ret;
 
-	if (!meson_venc_cvbs_connector_is_available(priv)) {
+	if (!meson_encoder_cvbs_connector_is_available(priv)) {
 		dev_info(drm->dev, "CVBS Output connector not available\n");
 		return 0;
 	}
 
-	meson_venc_cvbs = devm_kzalloc(priv->dev, sizeof(*meson_venc_cvbs),
+	meson_encoder_cvbs = devm_kzalloc(priv->dev, sizeof(*meson_encoder_cvbs),
 				       GFP_KERNEL);
-	if (!meson_venc_cvbs)
+	if (!meson_encoder_cvbs)
 		return -ENOMEM;
 
-	meson_venc_cvbs->priv = priv;
-	encoder = &meson_venc_cvbs->encoder;
-	connector = &meson_venc_cvbs->connector;
+	meson_encoder_cvbs->priv = priv;
+	encoder = &meson_encoder_cvbs->encoder;
+	connector = &meson_encoder_cvbs->connector;
 
 	/* Connector */
 
@@ -276,10 +276,10 @@ int meson_venc_cvbs_create(struct meson_drm *priv)
 
 	/* Encoder */
 
-	drm_encoder_helper_add(encoder, &meson_venc_cvbs_encoder_helper_funcs);
+	drm_encoder_helper_add(encoder, &meson_encoder_cvbs_encoder_helper_funcs);
 
-	ret = drm_encoder_init(drm, encoder, &meson_venc_cvbs_encoder_funcs,
-			       DRM_MODE_ENCODER_TVDAC, "meson_venc_cvbs");
+	ret = drm_encoder_init(drm, encoder, &meson_encoder_cvbs_encoder_funcs,
+			       DRM_MODE_ENCODER_TVDAC, "meson_encoder_cvbs");
 	if (ret) {
 		dev_err(priv->dev, "Failed to init CVBS encoder\n");
 		return ret;
diff --git a/drivers/gpu/drm/meson/meson_venc_cvbs.h b/drivers/gpu/drm/meson/meson_encoder_cvbs.h
similarity index 92%
rename from drivers/gpu/drm/meson/meson_venc_cvbs.h
rename to drivers/gpu/drm/meson/meson_encoder_cvbs.h
index ab7f76ba469c..61d9d183ce7f 100644
--- a/drivers/gpu/drm/meson/meson_venc_cvbs.h
+++ b/drivers/gpu/drm/meson/meson_encoder_cvbs.h
@@ -24,6 +24,6 @@ struct meson_cvbs_mode {
 /* Modes supported by the CVBS output */
 extern struct meson_cvbs_mode meson_cvbs_modes[MESON_CVBS_MODES_COUNT];
 
-int meson_venc_cvbs_create(struct meson_drm *priv);
+int meson_encoder_cvbs_init(struct meson_drm *priv);
 
 #endif /* __MESON_VENC_CVBS_H */
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* [PATCH v2 6/6] drm/meson: encoder_cvbs: switch to bridge with ATTACH_NO_CONNECTOR
  2021-10-15 14:11 [PATCH v2 0/6] drm/meson: rework encoders to pass ATTACH_NO_CONNECTOR Neil Armstrong
                   ` (4 preceding siblings ...)
  2021-10-15 14:11 ` [PATCH v2 5/6] drm/meson: rename venc_cvbs to encoder_cvbs Neil Armstrong
@ 2021-10-15 14:11 ` Neil Armstrong
  2021-10-15 22:07   ` Martin Blumenstingl
  2021-10-15 22:34   ` Martin Blumenstingl
  5 siblings, 2 replies; 16+ messages in thread
From: Neil Armstrong @ 2021-10-15 14:11 UTC (permalink / raw)
  To: daniel, Laurent.pinchart, sam
  Cc: martin.blumenstingl, dri-devel, linux-amlogic, linux-arm-kernel,
	linux-kernel, Neil Armstrong

Drop the local connector and move all callback to bridge funcs in order
to leverage the generic CVBS display connector.

This will also permit adding custom cvbs connectors for ADC based HPD
detection on some Amlogic SoC based boards.

Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
Acked-by: Sam Ravnborg <sam@ravnborg.org>
---
 drivers/gpu/drm/meson/meson_encoder_cvbs.c | 208 ++++++++++-----------
 1 file changed, 103 insertions(+), 105 deletions(-)

diff --git a/drivers/gpu/drm/meson/meson_encoder_cvbs.c b/drivers/gpu/drm/meson/meson_encoder_cvbs.c
index 01024c5f610c..5b9704d78cf9 100644
--- a/drivers/gpu/drm/meson/meson_encoder_cvbs.c
+++ b/drivers/gpu/drm/meson/meson_encoder_cvbs.c
@@ -13,10 +13,12 @@
 #include <linux/of_graph.h>
 
 #include <drm/drm_atomic_helper.h>
+#include <drm/drm_bridge.h>
+#include <drm/drm_bridge_connector.h>
 #include <drm/drm_device.h>
 #include <drm/drm_edid.h>
 #include <drm/drm_probe_helper.h>
-#include <drm/drm_print.h>
+#include <drm/drm_simple_kms_helper.h>
 
 #include "meson_registers.h"
 #include "meson_vclk.h"
@@ -30,14 +32,13 @@
 
 struct meson_encoder_cvbs {
 	struct drm_encoder	encoder;
-	struct drm_connector	connector;
+	struct drm_bridge	bridge;
+	struct drm_bridge	*next_bridge;
 	struct meson_drm	*priv;
 };
-#define encoder_to_meson_encoder_cvbs(x) \
-	container_of(x, struct meson_encoder_cvbs, encoder)
 
-#define connector_to_meson_encoder_cvbs(x) \
-	container_of(x, struct meson_encoder_cvbs, connector)
+#define bridge_to_meson_encoder_cvbs(x) \
+	container_of(x, struct meson_encoder_cvbs, bridge)
 
 /* Supported Modes */
 
@@ -81,32 +82,31 @@ meson_cvbs_get_mode(const struct drm_display_mode *req_mode)
 	return NULL;
 }
 
-/* Connector */
-
-static void meson_cvbs_connector_destroy(struct drm_connector *connector)
+static int meson_encoder_cvbs_attach(struct drm_bridge *bridge,
+				     enum drm_bridge_attach_flags flags)
 {
-	drm_connector_cleanup(connector);
-}
+	struct meson_encoder_cvbs *meson_encoder_cvbs =
+					bridge_to_meson_encoder_cvbs(bridge);
 
-static enum drm_connector_status
-meson_cvbs_connector_detect(struct drm_connector *connector, bool force)
-{
-	/* FIXME: Add load-detect or jack-detect if possible */
-	return connector_status_connected;
+	return drm_bridge_attach(bridge->encoder, meson_encoder_cvbs->next_bridge,
+				 &meson_encoder_cvbs->bridge, flags);
 }
 
-static int meson_cvbs_connector_get_modes(struct drm_connector *connector)
+static int meson_encoder_cvbs_get_modes(struct drm_bridge *bridge,
+					struct drm_connector *connector)
 {
-	struct drm_device *dev = connector->dev;
+	struct meson_encoder_cvbs *meson_encoder_cvbs =
+					bridge_to_meson_encoder_cvbs(bridge);
+	struct meson_drm *priv = meson_encoder_cvbs->priv;
 	struct drm_display_mode *mode;
 	int i;
 
 	for (i = 0; i < MESON_CVBS_MODES_COUNT; ++i) {
 		struct meson_cvbs_mode *meson_mode = &meson_cvbs_modes[i];
 
-		mode = drm_mode_duplicate(dev, &meson_mode->mode);
+		mode = drm_mode_duplicate(priv->drm, &meson_mode->mode);
 		if (!mode) {
-			DRM_ERROR("Failed to create a new display mode\n");
+			dev_err(priv->dev, "Failed to create a new display mode\n");
 			return 0;
 		}
 
@@ -116,40 +116,18 @@ static int meson_cvbs_connector_get_modes(struct drm_connector *connector)
 	return i;
 }
 
-static int meson_cvbs_connector_mode_valid(struct drm_connector *connector,
-					   struct drm_display_mode *mode)
+static int meson_encoder_cvbs_mode_valid(struct drm_bridge *bridge,
+					const struct drm_display_info *display_info,
+					const struct drm_display_mode *mode)
 {
-	/* Validate the modes added in get_modes */
-	return MODE_OK;
-}
-
-static const struct drm_connector_funcs meson_cvbs_connector_funcs = {
-	.detect			= meson_cvbs_connector_detect,
-	.fill_modes		= drm_helper_probe_single_connector_modes,
-	.destroy		= meson_cvbs_connector_destroy,
-	.reset			= drm_atomic_helper_connector_reset,
-	.atomic_duplicate_state	= drm_atomic_helper_connector_duplicate_state,
-	.atomic_destroy_state	= drm_atomic_helper_connector_destroy_state,
-};
-
-static const
-struct drm_connector_helper_funcs meson_cvbs_connector_helper_funcs = {
-	.get_modes	= meson_cvbs_connector_get_modes,
-	.mode_valid	= meson_cvbs_connector_mode_valid,
-};
-
-/* Encoder */
+	if (meson_cvbs_get_mode(mode))
+		return MODE_OK;
 
-static void meson_encoder_cvbs_encoder_destroy(struct drm_encoder *encoder)
-{
-	drm_encoder_cleanup(encoder);
+	return MODE_BAD;
 }
 
-static const struct drm_encoder_funcs meson_encoder_cvbs_encoder_funcs = {
-	.destroy        = meson_encoder_cvbs_encoder_destroy,
-};
-
-static int meson_encoder_cvbs_encoder_atomic_check(struct drm_encoder *encoder,
+static int meson_encoder_cvbs_atomic_check(struct drm_bridge *bridge,
+					struct drm_bridge_state *bridge_state,
 					struct drm_crtc_state *crtc_state,
 					struct drm_connector_state *conn_state)
 {
@@ -159,10 +137,10 @@ static int meson_encoder_cvbs_encoder_atomic_check(struct drm_encoder *encoder,
 	return -EINVAL;
 }
 
-static void meson_encoder_cvbs_encoder_disable(struct drm_encoder *encoder)
+static void meson_encoder_cvbs_disable(struct drm_bridge *bridge)
 {
 	struct meson_encoder_cvbs *meson_encoder_cvbs =
-					encoder_to_meson_encoder_cvbs(encoder);
+					bridge_to_meson_encoder_cvbs(bridge);
 	struct meson_drm *priv = meson_encoder_cvbs->priv;
 
 	/* Disable CVBS VDAC */
@@ -175,10 +153,10 @@ static void meson_encoder_cvbs_encoder_disable(struct drm_encoder *encoder)
 	}
 }
 
-static void meson_encoder_cvbs_encoder_enable(struct drm_encoder *encoder)
+static void meson_encoder_cvbs_enable(struct drm_bridge *bridge)
 {
 	struct meson_encoder_cvbs *meson_encoder_cvbs =
-					encoder_to_meson_encoder_cvbs(encoder);
+					bridge_to_meson_encoder_cvbs(bridge);
 	struct meson_drm *priv = meson_encoder_cvbs->priv;
 
 	/* VDAC0 source is not from ATV */
@@ -198,14 +176,30 @@ static void meson_encoder_cvbs_encoder_enable(struct drm_encoder *encoder)
 	}
 }
 
-static void meson_encoder_cvbs_encoder_mode_set(struct drm_encoder *encoder,
-				   struct drm_display_mode *mode,
-				   struct drm_display_mode *adjusted_mode)
+static void meson_encoder_cvbs_atomic_enable(struct drm_bridge *bridge,
+					     struct drm_bridge_state *bridge_state)
 {
-	const struct meson_cvbs_mode *meson_mode = meson_cvbs_get_mode(mode);
-	struct meson_encoder_cvbs *meson_encoder_cvbs =
-					encoder_to_meson_encoder_cvbs(encoder);
-	struct meson_drm *priv = meson_encoder_cvbs->priv;
+	struct meson_encoder_cvbs *encoder_cvbs = bridge_to_meson_encoder_cvbs(bridge);
+	struct drm_atomic_state *state = bridge_state->base.state;
+	struct meson_drm *priv = encoder_cvbs->priv;
+	const struct meson_cvbs_mode *meson_mode;
+	struct drm_connector_state *conn_state;
+	struct drm_crtc_state *crtc_state;
+	struct drm_connector *connector;
+
+	connector = drm_atomic_get_new_connector_for_encoder(state, bridge->encoder);
+	if (WARN_ON(!connector))
+		return;
+
+	conn_state = drm_atomic_get_new_connector_state(state, connector);
+	if (WARN_ON(!conn_state))
+		return;
+
+	crtc_state = drm_atomic_get_new_crtc_state(state, conn_state->crtc);
+	if (WARN_ON(!crtc_state))
+		return;
+
+	meson_mode = meson_cvbs_get_mode(&crtc_state->adjusted_mode);
 
 	if (meson_mode) {
 		meson_venci_cvbs_mode_set(priv, meson_mode->enci);
@@ -218,76 +212,80 @@ static void meson_encoder_cvbs_encoder_mode_set(struct drm_encoder *encoder,
 	}
 }
 
-static const struct drm_encoder_helper_funcs
-				meson_encoder_cvbs_encoder_helper_funcs = {
-	.atomic_check	= meson_encoder_cvbs_encoder_atomic_check,
-	.disable	= meson_encoder_cvbs_encoder_disable,
-	.enable		= meson_encoder_cvbs_encoder_enable,
-	.mode_set	= meson_encoder_cvbs_encoder_mode_set,
+static const struct drm_bridge_funcs meson_encoder_cvbs_bridge_funcs = {
+	.attach = meson_encoder_cvbs_attach,
+	.enable	= meson_encoder_cvbs_enable,
+	.disable = meson_encoder_cvbs_disable,
+	.mode_valid = meson_encoder_cvbs_mode_valid,
+	.get_modes = meson_encoder_cvbs_get_modes,
+	.atomic_enable = meson_encoder_cvbs_atomic_enable,
+	.atomic_check = meson_encoder_cvbs_atomic_check,
+	.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 bool meson_encoder_cvbs_connector_is_available(struct meson_drm *priv)
-{
-	struct device_node *remote;
-
-	remote = of_graph_get_remote_node(priv->dev->of_node, 0, 0);
-	if (!remote)
-		return false;
-
-	of_node_put(remote);
-	return true;
-}
-
 int meson_encoder_cvbs_init(struct meson_drm *priv)
 {
 	struct drm_device *drm = priv->drm;
 	struct meson_encoder_cvbs *meson_encoder_cvbs;
 	struct drm_connector *connector;
-	struct drm_encoder *encoder;
+	struct device_node *remote;
 	int ret;
 
-	if (!meson_encoder_cvbs_connector_is_available(priv)) {
+	meson_encoder_cvbs = devm_kzalloc(priv->dev, sizeof(*meson_encoder_cvbs), GFP_KERNEL);
+	if (!meson_encoder_cvbs)
+		return -ENOMEM;
+
+	/* CVBS Connector Bridge */
+	remote = of_graph_get_remote_node(priv->dev->of_node, 0, 0);
+	if (!remote) {
 		dev_info(drm->dev, "CVBS Output connector not available\n");
 		return 0;
 	}
 
-	meson_encoder_cvbs = devm_kzalloc(priv->dev, sizeof(*meson_encoder_cvbs),
-				       GFP_KERNEL);
-	if (!meson_encoder_cvbs)
-		return -ENOMEM;
+	meson_encoder_cvbs->next_bridge = of_drm_find_bridge(remote);
+	if (!meson_encoder_cvbs->next_bridge) {
+		dev_err(priv->dev, "Failed to find CVBS Connector bridge\n");
+		return -EPROBE_DEFER;
+	}
 
-	meson_encoder_cvbs->priv = priv;
-	encoder = &meson_encoder_cvbs->encoder;
-	connector = &meson_encoder_cvbs->connector;
+	/* CVBS Encoder Bridge */
+	meson_encoder_cvbs->bridge.funcs = &meson_encoder_cvbs_bridge_funcs;
+	meson_encoder_cvbs->bridge.of_node = priv->dev->of_node;
+	meson_encoder_cvbs->bridge.type = DRM_MODE_CONNECTOR_Composite;
+	meson_encoder_cvbs->bridge.ops = DRM_BRIDGE_OP_MODES;
+	meson_encoder_cvbs->bridge.interlace_allowed = true;
 
-	/* Connector */
+	drm_bridge_add(&meson_encoder_cvbs->bridge);
 
-	drm_connector_helper_add(connector,
-				 &meson_cvbs_connector_helper_funcs);
+	meson_encoder_cvbs->priv = priv;
 
-	ret = drm_connector_init(drm, connector, &meson_cvbs_connector_funcs,
-				 DRM_MODE_CONNECTOR_Composite);
+	/* Encoder */
+	ret = drm_simple_encoder_init(priv->drm, &meson_encoder_cvbs->encoder,
+				      DRM_MODE_ENCODER_TVDAC);
 	if (ret) {
-		dev_err(priv->dev, "Failed to init CVBS connector\n");
+		dev_err(priv->dev, "Failed to init CVBS encoder: %d\n", ret);
 		return ret;
 	}
 
-	connector->interlace_allowed = 1;
+	meson_encoder_cvbs->encoder.possible_crtcs = BIT(0);
 
-	/* Encoder */
-
-	drm_encoder_helper_add(encoder, &meson_encoder_cvbs_encoder_helper_funcs);
-
-	ret = drm_encoder_init(drm, encoder, &meson_encoder_cvbs_encoder_funcs,
-			       DRM_MODE_ENCODER_TVDAC, "meson_encoder_cvbs");
+	/* Attach CVBS Encoder Bridge to Encoder */
+	ret = drm_bridge_attach(&meson_encoder_cvbs->encoder, &meson_encoder_cvbs->bridge, NULL,
+				DRM_BRIDGE_ATTACH_NO_CONNECTOR);
 	if (ret) {
-		dev_err(priv->dev, "Failed to init CVBS encoder\n");
+		dev_err(priv->dev, "Failed to attach bridge: %d\n", ret);
 		return ret;
 	}
 
-	encoder->possible_crtcs = BIT(0);
-
-	drm_connector_attach_encoder(connector, encoder);
+	/* Initialize & attach Bridge Connector */
+	connector = drm_bridge_connector_init(priv->drm, &meson_encoder_cvbs->encoder);
+	if (IS_ERR(connector)) {
+		dev_err(priv->dev, "Unable to create CVBS bridge connector\n");
+		return PTR_ERR(connector);
+	}
+	drm_connector_attach_encoder(connector, &meson_encoder_cvbs->encoder);
 
 	return 0;
 }
-- 
2.25.1


^ permalink raw reply related	[flat|nested] 16+ messages in thread

* Re: [PATCH v2 2/6] drm/meson: remove useless recursive components matching
  2021-10-15 14:11 ` [PATCH v2 2/6] drm/meson: remove useless recursive components matching Neil Armstrong
@ 2021-10-15 21:59   ` Martin Blumenstingl
  0 siblings, 0 replies; 16+ messages in thread
From: Martin Blumenstingl @ 2021-10-15 21:59 UTC (permalink / raw)
  To: Neil Armstrong
  Cc: daniel, Laurent.pinchart, sam, dri-devel, linux-amlogic,
	linux-arm-kernel, linux-kernel

On Fri, Oct 15, 2021 at 4:11 PM Neil Armstrong <narmstrong@baylibre.com> wrote:
>
> The initial design was recursive to cover all port/endpoints, but only the first layer
> of endpoints should be covered by the components list.
> This also breaks the MIPI-DSI init/bridge attach sequence, thus only parse the
> first endpoints instead of recursing.
>
> Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
> Acked-by: Sam Ravnborg <sam@ravnborg.org>
Acked-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v2 3/6] drm/meson: split out encoder from meson_dw_hdmi
  2021-10-15 14:11 ` [PATCH v2 3/6] drm/meson: split out encoder from meson_dw_hdmi Neil Armstrong
@ 2021-10-15 22:01   ` Martin Blumenstingl
  2021-10-18 13:58   ` Laurent Pinchart
  1 sibling, 0 replies; 16+ messages in thread
From: Martin Blumenstingl @ 2021-10-15 22:01 UTC (permalink / raw)
  To: Neil Armstrong
  Cc: daniel, Laurent.pinchart, sam, dri-devel, linux-amlogic,
	linux-arm-kernel, linux-kernel

Hi Neil,

On Fri, Oct 15, 2021 at 4:11 PM Neil Armstrong <narmstrong@baylibre.com> wrote:
>
> This moves all the non-DW-HDMI code where it should be:
> an encoder in the drm/meson core driver.
>
> The bridge functions are copied as-is, except:
> - the encoder init uses the simple kms helper
> - the mode_set has been moved to atomic_enable()
> - debug prints are converted to dev_debg()
>
> For now the bridge attach flags is 0, DRM_BRIDGE_ATTACH_NO_CONNECTOR
> will be handled later.
>
> The meson dw-hdmi glue is slighly fixed to live without the
typo: slightly

> encoder in the same driver.
>
> Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
apart from that typo (please fix it up when applying the patch) this
is some great work!
it helps me a lot with HDMI support on the 32-bit SoCs
with that said:
Acked-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v2 4/6] drm/meson: encoder_hdmi: switch to bridge DRM_BRIDGE_ATTACH_NO_CONNECTOR
  2021-10-15 14:11 ` [PATCH v2 4/6] drm/meson: encoder_hdmi: switch to bridge DRM_BRIDGE_ATTACH_NO_CONNECTOR Neil Armstrong
@ 2021-10-15 22:07   ` Martin Blumenstingl
  2021-10-18  7:50     ` Neil Armstrong
  0 siblings, 1 reply; 16+ messages in thread
From: Martin Blumenstingl @ 2021-10-15 22:07 UTC (permalink / raw)
  To: Neil Armstrong
  Cc: daniel, Laurent.pinchart, sam, dri-devel, linux-amlogic,
	linux-arm-kernel, linux-kernel

Hi Neil,

On Fri, Oct 15, 2021 at 4:11 PM Neil Armstrong <narmstrong@baylibre.com> wrote:
>
> This implements the necessary change to no more use the embedded
> connector in dw-hdmi and use the dedicated bridge connector driver
> by passing DRM_BRIDGE_ATTACH_NO_CONNECTOR to the bridge attach call.
>
> The necessary connector properties are added to handle the same
> functionalities as the embedded dw-hdmi connector, i.e. the HDR
> metadata, the CEC notifier & other flags.
>
> The dw-hdmi output_port is set to 1 in order to look for a connector
> next bridge in order to get DRM_BRIDGE_ATTACH_NO_CONNECTOR working.
>
> Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
> Acked-by: Sam Ravnborg <sam@ravnborg.org>
another great piece which helps a lot with HDMI support for the 32-bit SoCs!
I have one question below - but regardless of the answer there this gets my:
Acked-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>

[...]
> +       pdev = of_find_device_by_node(remote);
I am wondering if we should use something like:
    encoder_hdmi->cec_notifier_pdev

> +       if (pdev) {
> +               struct cec_connector_info conn_info;
> +               struct cec_notifier *notifier;
> +
> +               cec_fill_conn_info_from_drm(&conn_info, meson_encoder_hdmi->connector);
> +
> +               notifier = cec_notifier_conn_register(&pdev->dev, NULL, &conn_info);
> +               if (!notifier)
> +                       return -ENOMEM;
> +
> +               meson_encoder_hdmi->cec_notifier = notifier;
> +       }
and then move this logic to meson_encoder_hdmi_attach()
This would be important if .detach() and .attach() can be called
multiple times (for example during suspend and resume). But I am not
sure if that's a supported use-case.


Best regards,
Martin

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v2 5/6] drm/meson: rename venc_cvbs to encoder_cvbs
  2021-10-15 14:11 ` [PATCH v2 5/6] drm/meson: rename venc_cvbs to encoder_cvbs Neil Armstrong
@ 2021-10-15 22:07   ` Martin Blumenstingl
  0 siblings, 0 replies; 16+ messages in thread
From: Martin Blumenstingl @ 2021-10-15 22:07 UTC (permalink / raw)
  To: Neil Armstrong
  Cc: daniel, Laurent.pinchart, sam, dri-devel, linux-amlogic,
	linux-arm-kernel, linux-kernel

On Fri, Oct 15, 2021 at 4:11 PM Neil Armstrong <narmstrong@baylibre.com> wrote:
>
> Rename the cvbs encoder to match the newly introduced meson_encoder_hdmi.
>
> Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
> Acked-by: Sam Ravnborg <sam@ravnborg.org>
Acked-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v2 6/6] drm/meson: encoder_cvbs: switch to bridge with ATTACH_NO_CONNECTOR
  2021-10-15 14:11 ` [PATCH v2 6/6] drm/meson: encoder_cvbs: switch to bridge with ATTACH_NO_CONNECTOR Neil Armstrong
@ 2021-10-15 22:07   ` Martin Blumenstingl
  2021-10-15 22:34   ` Martin Blumenstingl
  1 sibling, 0 replies; 16+ messages in thread
From: Martin Blumenstingl @ 2021-10-15 22:07 UTC (permalink / raw)
  To: Neil Armstrong
  Cc: daniel, Laurent.pinchart, sam, dri-devel, linux-amlogic,
	linux-arm-kernel, linux-kernel

On Fri, Oct 15, 2021 at 4:11 PM Neil Armstrong <narmstrong@baylibre.com> wrote:
>
> Drop the local connector and move all callback to bridge funcs in order
> to leverage the generic CVBS display connector.
>
> This will also permit adding custom cvbs connectors for ADC based HPD
> detection on some Amlogic SoC based boards.
>
> Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
> Acked-by: Sam Ravnborg <sam@ravnborg.org>
Acked-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v2 6/6] drm/meson: encoder_cvbs: switch to bridge with ATTACH_NO_CONNECTOR
  2021-10-15 14:11 ` [PATCH v2 6/6] drm/meson: encoder_cvbs: switch to bridge with ATTACH_NO_CONNECTOR Neil Armstrong
  2021-10-15 22:07   ` Martin Blumenstingl
@ 2021-10-15 22:34   ` Martin Blumenstingl
  2021-10-18  7:40     ` Neil Armstrong
  1 sibling, 1 reply; 16+ messages in thread
From: Martin Blumenstingl @ 2021-10-15 22:34 UTC (permalink / raw)
  To: Neil Armstrong, sam
  Cc: daniel, Laurent.pinchart, dri-devel, linux-amlogic,
	linux-arm-kernel, linux-kernel

Hi Neil, Hi Sam,

On Fri, Oct 15, 2021 at 4:11 PM Neil Armstrong <narmstrong@baylibre.com> wrote:
[...]
> +static const struct drm_bridge_funcs meson_encoder_cvbs_bridge_funcs = {
> +       .attach = meson_encoder_cvbs_attach,
> +       .enable = meson_encoder_cvbs_enable,
> +       .disable = meson_encoder_cvbs_disable,
> +       .mode_valid = meson_encoder_cvbs_mode_valid,
> +       .get_modes = meson_encoder_cvbs_get_modes,
> +       .atomic_enable = meson_encoder_cvbs_atomic_enable,
I did some testing on boards where u-boot doesn't initialize the video outputs.
It seems that meson_encoder_cvbs_enable() is never called with this patch.
meson_encoder_cvbs_atomic_enable() is called though.

From what I can see in drm_bridge.c [0] this is even expected.
Does this mean that we need to move all logic from .enable to
.atomic_enable (and same with .disable -> moving that logic to
.atomic_disable)?

The same comment applies to the HDMI patch.


Best regards,
Martin


[0] https://elixir.bootlin.com/linux/v5.15-rc5/source/drivers/gpu/drm/drm_bridge.c#L717

^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v2 6/6] drm/meson: encoder_cvbs: switch to bridge with ATTACH_NO_CONNECTOR
  2021-10-15 22:34   ` Martin Blumenstingl
@ 2021-10-18  7:40     ` Neil Armstrong
  0 siblings, 0 replies; 16+ messages in thread
From: Neil Armstrong @ 2021-10-18  7:40 UTC (permalink / raw)
  To: Martin Blumenstingl, sam
  Cc: daniel, Laurent.pinchart, dri-devel, linux-amlogic,
	linux-arm-kernel, linux-kernel

Hi,

On 16/10/2021 00:34, Martin Blumenstingl wrote:
> Hi Neil, Hi Sam,
> 
> On Fri, Oct 15, 2021 at 4:11 PM Neil Armstrong <narmstrong@baylibre.com> wrote:
> [...]
>> +static const struct drm_bridge_funcs meson_encoder_cvbs_bridge_funcs = {
>> +       .attach = meson_encoder_cvbs_attach,
>> +       .enable = meson_encoder_cvbs_enable,
>> +       .disable = meson_encoder_cvbs_disable,
>> +       .mode_valid = meson_encoder_cvbs_mode_valid,
>> +       .get_modes = meson_encoder_cvbs_get_modes,
>> +       .atomic_enable = meson_encoder_cvbs_atomic_enable,
> I did some testing on boards where u-boot doesn't initialize the video outputs.
> It seems that meson_encoder_cvbs_enable() is never called with this patch.
> meson_encoder_cvbs_atomic_enable() is called though.
> 
> From what I can see in drm_bridge.c [0] this is even expected.
> Does this mean that we need to move all logic from .enable to
> .atomic_enable (and same with .disable -> moving that logic to
> .atomic_disable)?
> 
> The same comment applies to the HDMI patch.

Good point, I'll fix that for both patches !

Neil

> 
> 
> Best regards,
> Martin
> 
> 
> [0] https://elixir.bootlin.com/linux/v5.15-rc5/source/drivers/gpu/drm/drm_bridge.c#L717
> 


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v2 4/6] drm/meson: encoder_hdmi: switch to bridge DRM_BRIDGE_ATTACH_NO_CONNECTOR
  2021-10-15 22:07   ` Martin Blumenstingl
@ 2021-10-18  7:50     ` Neil Armstrong
  0 siblings, 0 replies; 16+ messages in thread
From: Neil Armstrong @ 2021-10-18  7:50 UTC (permalink / raw)
  To: Martin Blumenstingl
  Cc: daniel, Laurent.pinchart, sam, dri-devel, linux-amlogic,
	linux-arm-kernel, linux-kernel

Hi,

On 16/10/2021 00:07, Martin Blumenstingl wrote:
> Hi Neil,
> 
> On Fri, Oct 15, 2021 at 4:11 PM Neil Armstrong <narmstrong@baylibre.com> wrote:
>>
>> This implements the necessary change to no more use the embedded
>> connector in dw-hdmi and use the dedicated bridge connector driver
>> by passing DRM_BRIDGE_ATTACH_NO_CONNECTOR to the bridge attach call.
>>
>> The necessary connector properties are added to handle the same
>> functionalities as the embedded dw-hdmi connector, i.e. the HDR
>> metadata, the CEC notifier & other flags.
>>
>> The dw-hdmi output_port is set to 1 in order to look for a connector
>> next bridge in order to get DRM_BRIDGE_ATTACH_NO_CONNECTOR working.
>>
>> Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
>> Acked-by: Sam Ravnborg <sam@ravnborg.org>
> another great piece which helps a lot with HDMI support for the 32-bit SoCs!
> I have one question below - but regardless of the answer there this gets my:
> Acked-by: Martin Blumenstingl <martin.blumenstingl@googlemail.com>
> 
> [...]
>> +       pdev = of_find_device_by_node(remote);
> I am wondering if we should use something like:
>     encoder_hdmi->cec_notifier_pdev
> 
>> +       if (pdev) {
>> +               struct cec_connector_info conn_info;
>> +               struct cec_notifier *notifier;
>> +
>> +               cec_fill_conn_info_from_drm(&conn_info, meson_encoder_hdmi->connector);
>> +
>> +               notifier = cec_notifier_conn_register(&pdev->dev, NULL, &conn_info);
>> +               if (!notifier)
>> +                       return -ENOMEM;
>> +
>> +               meson_encoder_hdmi->cec_notifier = notifier;
>> +       }
> and then move this logic to meson_encoder_hdmi_attach()

We can't because we create the connector after the attach.

> This would be important if .detach() and .attach() can be called
> multiple times (for example during suspend and resume). But I am not
> sure if that's a supported use-case.

Attach for now will only be done once at probe, and detach only a remove,
we don't change the bridge chaining while suspend/resume, we only reset the
pipeline state and put the old state back when resuming.

Neil

> 
> 
> Best regards,
> Martin
> 


^ permalink raw reply	[flat|nested] 16+ messages in thread

* Re: [PATCH v2 3/6] drm/meson: split out encoder from meson_dw_hdmi
  2021-10-15 14:11 ` [PATCH v2 3/6] drm/meson: split out encoder from meson_dw_hdmi Neil Armstrong
  2021-10-15 22:01   ` Martin Blumenstingl
@ 2021-10-18 13:58   ` Laurent Pinchart
  1 sibling, 0 replies; 16+ messages in thread
From: Laurent Pinchart @ 2021-10-18 13:58 UTC (permalink / raw)
  To: Neil Armstrong
  Cc: daniel, sam, martin.blumenstingl, dri-devel, linux-amlogic,
	linux-arm-kernel, linux-kernel

Hi Neil,

Thank you for the patch.

On Fri, Oct 15, 2021 at 04:11:04PM +0200, Neil Armstrong wrote:
> This moves all the non-DW-HDMI code where it should be:
> an encoder in the drm/meson core driver.
> 
> The bridge functions are copied as-is, except:
> - the encoder init uses the simple kms helper
> - the mode_set has been moved to atomic_enable()
> - debug prints are converted to dev_debg()
> 
> For now the bridge attach flags is 0, DRM_BRIDGE_ATTACH_NO_CONNECTOR
> will be handled later.
> 
> The meson dw-hdmi glue is slighly fixed to live without the
> encoder in the same driver.
> 
> Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
> ---
>  drivers/gpu/drm/meson/Makefile             |   1 +
>  drivers/gpu/drm/meson/meson_drv.c          |   5 +
>  drivers/gpu/drm/meson/meson_dw_hdmi.c      | 341 ++-----------------
>  drivers/gpu/drm/meson/meson_encoder_hdmi.c | 375 +++++++++++++++++++++
>  drivers/gpu/drm/meson/meson_encoder_hdmi.h |  12 +
>  5 files changed, 412 insertions(+), 322 deletions(-)
>  create mode 100644 drivers/gpu/drm/meson/meson_encoder_hdmi.c
>  create mode 100644 drivers/gpu/drm/meson/meson_encoder_hdmi.h
> 
> diff --git a/drivers/gpu/drm/meson/Makefile b/drivers/gpu/drm/meson/Makefile
> index 28a519cdf66b..523fce45f16b 100644
> --- a/drivers/gpu/drm/meson/Makefile
> +++ b/drivers/gpu/drm/meson/Makefile
> @@ -2,6 +2,7 @@
>  meson-drm-y := meson_drv.o meson_plane.o meson_crtc.o meson_venc_cvbs.o
>  meson-drm-y += meson_viu.o meson_vpp.o meson_venc.o meson_vclk.o meson_overlay.o
>  meson-drm-y += meson_rdma.o meson_osd_afbcd.o
> +meson-drm-y += meson_encoder_hdmi.o
>  
>  obj-$(CONFIG_DRM_MESON) += meson-drm.o
>  obj-$(CONFIG_DRM_MESON_DW_HDMI) += meson_dw_hdmi.o
> diff --git a/drivers/gpu/drm/meson/meson_drv.c b/drivers/gpu/drm/meson/meson_drv.c
> index 97ebc07357bb..0978b440f336 100644
> --- a/drivers/gpu/drm/meson/meson_drv.c
> +++ b/drivers/gpu/drm/meson/meson_drv.c
> @@ -32,6 +32,7 @@
>  #include "meson_osd_afbcd.h"
>  #include "meson_registers.h"
>  #include "meson_venc_cvbs.h"
> +#include "meson_encoder_hdmi.h"
>  #include "meson_viu.h"
>  #include "meson_vpp.h"
>  #include "meson_rdma.h"
> @@ -318,6 +319,10 @@ static int meson_drv_bind_master(struct device *dev, bool has_components)
>  		}
>  	}
>  
> +	ret = meson_encoder_hdmi_init(priv);
> +	if (ret)
> +		goto free_drm;
> +
>  	ret = meson_plane_create(priv);
>  	if (ret)
>  		goto free_drm;
> diff --git a/drivers/gpu/drm/meson/meson_dw_hdmi.c b/drivers/gpu/drm/meson/meson_dw_hdmi.c
> index 0afbd1e70bfc..fb540a503efe 100644
> --- a/drivers/gpu/drm/meson/meson_dw_hdmi.c
> +++ b/drivers/gpu/drm/meson/meson_dw_hdmi.c
> @@ -22,14 +22,11 @@
>  #include <drm/drm_probe_helper.h>
>  #include <drm/drm_print.h>
>  
> -#include <linux/media-bus-format.h>
>  #include <linux/videodev2.h>
>  
>  #include "meson_drv.h"
>  #include "meson_dw_hdmi.h"
>  #include "meson_registers.h"
> -#include "meson_vclk.h"
> -#include "meson_venc.h"
>  
>  #define DRIVER_NAME "meson-dw-hdmi"
>  #define DRIVER_DESC "Amlogic Meson HDMI-TX DRM driver"
> @@ -135,8 +132,6 @@ struct meson_dw_hdmi_data {
>  };
>  
>  struct meson_dw_hdmi {
> -	struct drm_encoder encoder;
> -	struct drm_bridge bridge;
>  	struct dw_hdmi_plat_data dw_plat_data;
>  	struct meson_drm *priv;
>  	struct device *dev;
> @@ -148,12 +143,8 @@ struct meson_dw_hdmi {
>  	struct regulator *hdmi_supply;
>  	u32 irq_stat;
>  	struct dw_hdmi *hdmi;
> -	unsigned long output_bus_fmt;
> +	struct drm_bridge *bridge;
>  };
> -#define encoder_to_meson_dw_hdmi(x) \
> -	container_of(x, struct meson_dw_hdmi, encoder)
> -#define bridge_to_meson_dw_hdmi(x) \
> -	container_of(x, struct meson_dw_hdmi, bridge)
>  
>  static inline int dw_hdmi_is_compatible(struct meson_dw_hdmi *dw_hdmi,
>  					const char *compat)
> @@ -295,14 +286,14 @@ static inline void dw_hdmi_dwc_write_bits(struct meson_dw_hdmi *dw_hdmi,
>  
>  /* Setup PHY bandwidth modes */
>  static void meson_hdmi_phy_setup_mode(struct meson_dw_hdmi *dw_hdmi,
> -				      const struct drm_display_mode *mode)
> +				      const struct drm_display_mode *mode,
> +				      bool mode_is_420)
>  {
>  	struct meson_drm *priv = dw_hdmi->priv;
>  	unsigned int pixel_clock = mode->clock;
>  
>  	/* For 420, pixel clock is half unlike venc clock */
> -	if (dw_hdmi->output_bus_fmt == MEDIA_BUS_FMT_UYYVYY8_0_5X24)
> -		pixel_clock /= 2;
> +	if (mode_is_420) pixel_clock /= 2;
>  
>  	if (dw_hdmi_is_compatible(dw_hdmi, "amlogic,meson-gxl-dw-hdmi") ||
>  	    dw_hdmi_is_compatible(dw_hdmi, "amlogic,meson-gxm-dw-hdmi")) {
> @@ -374,68 +365,25 @@ static inline void meson_dw_hdmi_phy_reset(struct meson_dw_hdmi *dw_hdmi)
>  	mdelay(2);
>  }
>  
> -static void dw_hdmi_set_vclk(struct meson_dw_hdmi *dw_hdmi,
> -			     const struct drm_display_mode *mode)
> -{
> -	struct meson_drm *priv = dw_hdmi->priv;
> -	int vic = drm_match_cea_mode(mode);
> -	unsigned int phy_freq;
> -	unsigned int vclk_freq;
> -	unsigned int venc_freq;
> -	unsigned int hdmi_freq;
> -
> -	vclk_freq = mode->clock;
> -
> -	/* For 420, pixel clock is half unlike venc clock */
> -	if (dw_hdmi->output_bus_fmt == MEDIA_BUS_FMT_UYYVYY8_0_5X24)
> -		vclk_freq /= 2;
> -
> -	/* TMDS clock is pixel_clock * 10 */
> -	phy_freq = vclk_freq * 10;
> -
> -	if (!vic) {
> -		meson_vclk_setup(priv, MESON_VCLK_TARGET_DMT, phy_freq,
> -				 vclk_freq, vclk_freq, vclk_freq, false);
> -		return;
> -	}
> -
> -	/* 480i/576i needs global pixel doubling */
> -	if (mode->flags & DRM_MODE_FLAG_DBLCLK)
> -		vclk_freq *= 2;
> -
> -	venc_freq = vclk_freq;
> -	hdmi_freq = vclk_freq;
> -
> -	/* VENC double pixels for 1080i, 720p and YUV420 modes */
> -	if (meson_venc_hdmi_venc_repeat(vic) ||
> -	    dw_hdmi->output_bus_fmt == MEDIA_BUS_FMT_UYYVYY8_0_5X24)
> -		venc_freq *= 2;
> -
> -	vclk_freq = max(venc_freq, hdmi_freq);
> -
> -	if (mode->flags & DRM_MODE_FLAG_DBLCLK)
> -		venc_freq /= 2;
> -
> -	DRM_DEBUG_DRIVER("vclk:%d phy=%d venc=%d hdmi=%d enci=%d\n",
> -		phy_freq, vclk_freq, venc_freq, hdmi_freq,
> -		priv->venc.hdmi_use_enci);
> -
> -	meson_vclk_setup(priv, MESON_VCLK_TARGET_HDMI, phy_freq, vclk_freq,
> -			 venc_freq, hdmi_freq, priv->venc.hdmi_use_enci);
> -}
> -
>  static int dw_hdmi_phy_init(struct dw_hdmi *hdmi, void *data,
>  			    const struct drm_display_info *display,
>  			    const struct drm_display_mode *mode)
>  {
>  	struct meson_dw_hdmi *dw_hdmi = (struct meson_dw_hdmi *)data;
> +	bool is_hdmi2_sink = display->hdmi.scdc.supported;
>  	struct meson_drm *priv = dw_hdmi->priv;
>  	unsigned int wr_clk =
>  		readl_relaxed(priv->io_base + _REG(VPU_HDMI_SETTING));
> +	bool mode_is_420 = false;
>  
>  	DRM_DEBUG_DRIVER("\"%s\" div%d\n", mode->name,
>  			 mode->clock > 340000 ? 40 : 10);
>  
> +	if (drm_mode_is_420_only(display, mode) ||
> +	    (!is_hdmi2_sink &&
> +	     drm_mode_is_420_also(display, mode)))
> +		mode_is_420 = true;
> +
>  	/* Enable clocks */
>  	regmap_update_bits(priv->hhi, HHI_HDMI_CLK_CNTL, 0xffff, 0x100);
>  
> @@ -457,8 +405,7 @@ static int dw_hdmi_phy_init(struct dw_hdmi *hdmi, void *data,
>  	dw_hdmi->data->top_write(dw_hdmi, HDMITX_TOP_BIST_CNTL, BIT(12));
>  
>  	/* TMDS pattern setup */
> -	if (mode->clock > 340000 &&
> -	    dw_hdmi->output_bus_fmt == MEDIA_BUS_FMT_YUV8_1X24) {
> +	if (mode->clock > 340000 && !mode_is_420) {
>  		dw_hdmi->data->top_write(dw_hdmi, HDMITX_TOP_TMDS_CLK_PTTN_01,
>  				  0);
>  		dw_hdmi->data->top_write(dw_hdmi, HDMITX_TOP_TMDS_CLK_PTTN_23,
> @@ -476,7 +423,7 @@ static int dw_hdmi_phy_init(struct dw_hdmi *hdmi, void *data,
>  	dw_hdmi->data->top_write(dw_hdmi, HDMITX_TOP_TMDS_CLK_PTTN_CNTL, 0x2);
>  
>  	/* Setup PHY parameters */
> -	meson_hdmi_phy_setup_mode(dw_hdmi, mode);
> +	meson_hdmi_phy_setup_mode(dw_hdmi, mode, mode_is_420);
>  
>  	/* Setup PHY */
>  	regmap_update_bits(priv->hhi, HHI_HDMI_PHY_CNTL1,
> @@ -622,214 +569,15 @@ static irqreturn_t dw_hdmi_top_thread_irq(int irq, void *dev_id)
>  		dw_hdmi_setup_rx_sense(dw_hdmi->hdmi, hpd_connected,
>  				       hpd_connected);
>  
> -		drm_helper_hpd_irq_event(dw_hdmi->encoder.dev);
> +		drm_helper_hpd_irq_event(dw_hdmi->bridge->dev);
> +		drm_bridge_hpd_notify(dw_hdmi->bridge,
> +				      hpd_connected ? connector_status_connected
> +						    : connector_status_disconnected);
>  	}
>  
>  	return IRQ_HANDLED;
>  }
>  
> -static enum drm_mode_status
> -dw_hdmi_mode_valid(struct dw_hdmi *hdmi, void *data,
> -		   const struct drm_display_info *display_info,
> -		   const struct drm_display_mode *mode)
> -{
> -	struct meson_dw_hdmi *dw_hdmi = data;
> -	struct meson_drm *priv = dw_hdmi->priv;
> -	bool is_hdmi2_sink = display_info->hdmi.scdc.supported;
> -	unsigned int phy_freq;
> -	unsigned int vclk_freq;
> -	unsigned int venc_freq;
> -	unsigned int hdmi_freq;
> -	int vic = drm_match_cea_mode(mode);
> -	enum drm_mode_status status;
> -
> -	DRM_DEBUG_DRIVER("Modeline " DRM_MODE_FMT "\n", DRM_MODE_ARG(mode));
> -
> -	/* If sink does not support 540MHz, reject the non-420 HDMI2 modes */
> -	if (display_info->max_tmds_clock &&
> -	    mode->clock > display_info->max_tmds_clock &&
> -	    !drm_mode_is_420_only(display_info, mode) &&
> -	    !drm_mode_is_420_also(display_info, mode))
> -		return MODE_BAD;
> -
> -	/* Check against non-VIC supported modes */
> -	if (!vic) {
> -		status = meson_venc_hdmi_supported_mode(mode);
> -		if (status != MODE_OK)
> -			return status;
> -
> -		return meson_vclk_dmt_supported_freq(priv, mode->clock);
> -	/* Check against supported VIC modes */
> -	} else if (!meson_venc_hdmi_supported_vic(vic))
> -		return MODE_BAD;
> -
> -	vclk_freq = mode->clock;
> -
> -	/* For 420, pixel clock is half unlike venc clock */
> -	if (drm_mode_is_420_only(display_info, mode) ||
> -	    (!is_hdmi2_sink &&
> -	     drm_mode_is_420_also(display_info, mode)))
> -		vclk_freq /= 2;
> -
> -	/* TMDS clock is pixel_clock * 10 */
> -	phy_freq = vclk_freq * 10;
> -
> -	/* 480i/576i needs global pixel doubling */
> -	if (mode->flags & DRM_MODE_FLAG_DBLCLK)
> -		vclk_freq *= 2;
> -
> -	venc_freq = vclk_freq;
> -	hdmi_freq = vclk_freq;
> -
> -	/* VENC double pixels for 1080i, 720p and YUV420 modes */
> -	if (meson_venc_hdmi_venc_repeat(vic) ||
> -	    drm_mode_is_420_only(display_info, mode) ||
> -	    (!is_hdmi2_sink &&
> -	     drm_mode_is_420_also(display_info, mode)))
> -		venc_freq *= 2;
> -
> -	vclk_freq = max(venc_freq, hdmi_freq);
> -
> -	if (mode->flags & DRM_MODE_FLAG_DBLCLK)
> -		venc_freq /= 2;
> -
> -	dev_dbg(dw_hdmi->dev, "%s: vclk:%d phy=%d venc=%d hdmi=%d\n",
> -		__func__, phy_freq, vclk_freq, venc_freq, hdmi_freq);
> -
> -	return meson_vclk_vic_supported_freq(priv, phy_freq, vclk_freq);
> -}
> -
> -/* Encoder */
> -
> -static const u32 meson_dw_hdmi_out_bus_fmts[] = {
> -	MEDIA_BUS_FMT_YUV8_1X24,
> -	MEDIA_BUS_FMT_UYYVYY8_0_5X24,
> -};
> -
> -static void meson_venc_hdmi_encoder_destroy(struct drm_encoder *encoder)
> -{
> -	drm_encoder_cleanup(encoder);
> -}
> -
> -static const struct drm_encoder_funcs meson_venc_hdmi_encoder_funcs = {
> -	.destroy        = meson_venc_hdmi_encoder_destroy,
> -};
> -
> -static u32 *
> -meson_venc_hdmi_encoder_get_inp_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)
> -{
> -	u32 *input_fmts = NULL;
> -	int i;
> -
> -	*num_input_fmts = 0;
> -
> -	for (i = 0 ; i < ARRAY_SIZE(meson_dw_hdmi_out_bus_fmts) ; ++i) {
> -		if (output_fmt == meson_dw_hdmi_out_bus_fmts[i]) {
> -			*num_input_fmts = 1;
> -			input_fmts = kcalloc(*num_input_fmts,
> -					     sizeof(*input_fmts),
> -					     GFP_KERNEL);
> -			if (!input_fmts)
> -				return NULL;
> -
> -			input_fmts[0] = output_fmt;
> -
> -			break;
> -		}
> -	}
> -
> -	return input_fmts;
> -}
> -
> -static int meson_venc_hdmi_encoder_atomic_check(struct drm_bridge *bridge,
> -					struct drm_bridge_state *bridge_state,
> -					struct drm_crtc_state *crtc_state,
> -					struct drm_connector_state *conn_state)
> -{
> -	struct meson_dw_hdmi *dw_hdmi = bridge_to_meson_dw_hdmi(bridge);
> -
> -	dw_hdmi->output_bus_fmt = bridge_state->output_bus_cfg.format;
> -
> -	DRM_DEBUG_DRIVER("output_bus_fmt %lx\n", dw_hdmi->output_bus_fmt);
> -
> -	return 0;
> -}
> -
> -static void meson_venc_hdmi_encoder_disable(struct drm_bridge *bridge)
> -{
> -	struct meson_dw_hdmi *dw_hdmi = bridge_to_meson_dw_hdmi(bridge);
> -	struct meson_drm *priv = dw_hdmi->priv;
> -
> -	DRM_DEBUG_DRIVER("\n");
> -
> -	writel_bits_relaxed(0x3, 0,
> -			    priv->io_base + _REG(VPU_HDMI_SETTING));
> -
> -	writel_relaxed(0, priv->io_base + _REG(ENCI_VIDEO_EN));
> -	writel_relaxed(0, priv->io_base + _REG(ENCP_VIDEO_EN));
> -}
> -
> -static void meson_venc_hdmi_encoder_enable(struct drm_bridge *bridge)
> -{
> -	struct meson_dw_hdmi *dw_hdmi = bridge_to_meson_dw_hdmi(bridge);
> -	struct meson_drm *priv = dw_hdmi->priv;
> -
> -	DRM_DEBUG_DRIVER("%s\n", priv->venc.hdmi_use_enci ? "VENCI" : "VENCP");
> -
> -	if (priv->venc.hdmi_use_enci)
> -		writel_relaxed(1, priv->io_base + _REG(ENCI_VIDEO_EN));
> -	else
> -		writel_relaxed(1, priv->io_base + _REG(ENCP_VIDEO_EN));
> -}
> -
> -static void meson_venc_hdmi_encoder_mode_set(struct drm_bridge *bridge,
> -				   const struct drm_display_mode *mode,
> -				   const struct drm_display_mode *adjusted_mode)
> -{
> -	struct meson_dw_hdmi *dw_hdmi = bridge_to_meson_dw_hdmi(bridge);
> -	struct meson_drm *priv = dw_hdmi->priv;
> -	int vic = drm_match_cea_mode(mode);
> -	unsigned int ycrcb_map = VPU_HDMI_OUTPUT_CBYCR;
> -	bool yuv420_mode = false;
> -
> -	DRM_DEBUG_DRIVER("\"%s\" vic %d\n", mode->name, vic);
> -
> -	if (dw_hdmi->output_bus_fmt == MEDIA_BUS_FMT_UYYVYY8_0_5X24) {
> -		ycrcb_map = VPU_HDMI_OUTPUT_CRYCB;
> -		yuv420_mode = true;
> -	}
> -
> -	/* VENC + VENC-DVI Mode setup */
> -	meson_venc_hdmi_mode_set(priv, vic, ycrcb_map, yuv420_mode, mode);
> -
> -	/* VCLK Set clock */
> -	dw_hdmi_set_vclk(dw_hdmi, mode);
> -
> -	if (dw_hdmi->output_bus_fmt == MEDIA_BUS_FMT_UYYVYY8_0_5X24)
> -		/* Setup YUV420 to HDMI-TX, no 10bit diphering */
> -		writel_relaxed(2 | (2 << 2),
> -			       priv->io_base + _REG(VPU_HDMI_FMT_CTRL));
> -	else
> -		/* Setup YUV444 to HDMI-TX, no 10bit diphering */
> -		writel_relaxed(0, priv->io_base + _REG(VPU_HDMI_FMT_CTRL));
> -}
> -
> -static const struct drm_bridge_funcs meson_venc_hdmi_encoder_bridge_funcs = {
> -	.atomic_duplicate_state = drm_atomic_helper_bridge_duplicate_state,
> -	.atomic_destroy_state = drm_atomic_helper_bridge_destroy_state,
> -	.atomic_get_input_bus_fmts = meson_venc_hdmi_encoder_get_inp_bus_fmts,
> -	.atomic_reset = drm_atomic_helper_bridge_reset,
> -	.atomic_check = meson_venc_hdmi_encoder_atomic_check,
> -	.enable	= meson_venc_hdmi_encoder_enable,
> -	.disable = meson_venc_hdmi_encoder_disable,
> -	.mode_set = meson_venc_hdmi_encoder_mode_set,
> -};
> -
>  /* DW HDMI Regmap */
>  
>  static int meson_dw_hdmi_reg_read(void *context, unsigned int reg,
> @@ -876,28 +624,6 @@ static const struct meson_dw_hdmi_data meson_dw_hdmi_g12a_data = {
>  	.dwc_write = dw_hdmi_g12a_dwc_write,
>  };
>  
> -static bool meson_hdmi_connector_is_available(struct device *dev)
> -{
> -	struct device_node *ep, *remote;
> -
> -	/* HDMI Connector is on the second port, first endpoint */
> -	ep = of_graph_get_endpoint_by_regs(dev->of_node, 1, 0);
> -	if (!ep)
> -		return false;
> -
> -	/* If the endpoint node exists, consider it enabled */
> -	remote = of_graph_get_remote_port(ep);
> -	if (remote) {
> -		of_node_put(ep);
> -		return true;
> -	}
> -
> -	of_node_put(ep);
> -	of_node_put(remote);
> -
> -	return false;
> -}
> -
>  static void meson_dw_hdmi_init(struct meson_dw_hdmi *meson_dw_hdmi)
>  {
>  	struct meson_drm *priv = meson_dw_hdmi->priv;
> @@ -976,18 +702,11 @@ static int meson_dw_hdmi_bind(struct device *dev, struct device *master,
>  	struct drm_device *drm = data;
>  	struct meson_drm *priv = drm->dev_private;
>  	struct dw_hdmi_plat_data *dw_plat_data;
> -	struct drm_bridge *next_bridge;
> -	struct drm_encoder *encoder;
>  	int irq;
>  	int ret;
>  
>  	DRM_DEBUG_DRIVER("\n");
>  
> -	if (!meson_hdmi_connector_is_available(dev)) {
> -		dev_info(drm->dev, "HDMI Output connector not available\n");
> -		return -ENODEV;
> -	}
> -
>  	match = of_device_get_match_data(&pdev->dev);
>  	if (!match) {
>  		dev_err(&pdev->dev, "failed to get match data\n");
> @@ -1003,7 +722,6 @@ static int meson_dw_hdmi_bind(struct device *dev, struct device *master,
>  	meson_dw_hdmi->dev = dev;
>  	meson_dw_hdmi->data = match;
>  	dw_plat_data = &meson_dw_hdmi->dw_plat_data;
> -	encoder = &meson_dw_hdmi->encoder;
>  
>  	meson_dw_hdmi->hdmi_supply = devm_regulator_get_optional(dev, "hdmi");
>  	if (IS_ERR(meson_dw_hdmi->hdmi_supply)) {
> @@ -1074,28 +792,11 @@ static int meson_dw_hdmi_bind(struct device *dev, struct device *master,
>  		return ret;
>  	}
>  
> -	/* Encoder */
> -
> -	ret = drm_encoder_init(drm, encoder, &meson_venc_hdmi_encoder_funcs,
> -			       DRM_MODE_ENCODER_TMDS, "meson_hdmi");
> -	if (ret) {
> -		dev_err(priv->dev, "Failed to init HDMI encoder\n");
> -		return ret;
> -	}
> -
> -	meson_dw_hdmi->bridge.funcs = &meson_venc_hdmi_encoder_bridge_funcs;
> -	drm_bridge_attach(encoder, &meson_dw_hdmi->bridge, NULL, 0);
> -
> -	encoder->possible_crtcs = BIT(0);
> -
>  	meson_dw_hdmi_init(meson_dw_hdmi);
>  
> -	DRM_DEBUG_DRIVER("encoder initialized\n");
> -
>  	/* Bridge / Connector */
>  
>  	dw_plat_data->priv_data = meson_dw_hdmi;
> -	dw_plat_data->mode_valid = dw_hdmi_mode_valid;
>  	dw_plat_data->phy_ops = &meson_dw_hdmi_phy_ops;
>  	dw_plat_data->phy_name = "meson_dw_hdmi_phy";
>  	dw_plat_data->phy_data = meson_dw_hdmi;
> @@ -1110,15 +811,11 @@ static int meson_dw_hdmi_bind(struct device *dev, struct device *master,
>  
>  	platform_set_drvdata(pdev, meson_dw_hdmi);
>  
> -	meson_dw_hdmi->hdmi = dw_hdmi_probe(pdev,
> -					    &meson_dw_hdmi->dw_plat_data);
> +	meson_dw_hdmi->hdmi = dw_hdmi_probe(pdev, &meson_dw_hdmi->dw_plat_data);
>  	if (IS_ERR(meson_dw_hdmi->hdmi))
>  		return PTR_ERR(meson_dw_hdmi->hdmi);
>  
> -	next_bridge = of_drm_find_bridge(pdev->dev.of_node);
> -	if (next_bridge)
> -		drm_bridge_attach(encoder, next_bridge,
> -				  &meson_dw_hdmi->bridge, 0);
> +	meson_dw_hdmi->bridge = of_drm_find_bridge(pdev->dev.of_node);
>  
>  	DRM_DEBUG_DRIVER("HDMI controller initialized\n");
>  
> diff --git a/drivers/gpu/drm/meson/meson_encoder_hdmi.c b/drivers/gpu/drm/meson/meson_encoder_hdmi.c
> new file mode 100644
> index 000000000000..971da662c954
> --- /dev/null
> +++ b/drivers/gpu/drm/meson/meson_encoder_hdmi.c
> @@ -0,0 +1,375 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + * Copyright (C) 2016 BayLibre, SAS
> + * Author: Neil Armstrong <narmstrong@baylibre.com>
> + * Copyright (C) 2015 Amlogic, Inc. All rights reserved.
> + */
> +
> +#include <linux/clk.h>
> +#include <linux/component.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/of_device.h>
> +#include <linux/of_graph.h>
> +#include <linux/regulator/consumer.h>
> +#include <linux/reset.h>
> +
> +#include <drm/drm_atomic_helper.h>
> +#include <drm/drm_bridge.h>
> +#include <drm/drm_device.h>
> +#include <drm/drm_edid.h>
> +#include <drm/drm_probe_helper.h>
> +#include <drm/drm_simple_kms_helper.h>
> +
> +#include <linux/media-bus-format.h>
> +#include <linux/videodev2.h>
> +
> +#include "meson_drv.h"
> +#include "meson_registers.h"
> +#include "meson_vclk.h"
> +#include "meson_venc.h"
> +
> +struct meson_encoder_hdmi {
> +	struct drm_encoder encoder;
> +	struct drm_bridge bridge;
> +	struct drm_bridge *next_bridge;
> +	struct meson_drm *priv;
> +	unsigned long output_bus_fmt;
> +};
> +
> +#define bridge_to_meson_encoder_hdmi(x) \
> +	container_of(x, struct meson_encoder_hdmi, bridge)
> +
> +static int meson_encoder_hdmi_attach(struct drm_bridge *bridge,
> +				     enum drm_bridge_attach_flags flags)
> +{
> +	struct meson_encoder_hdmi *encoder_hdmi = bridge_to_meson_encoder_hdmi(bridge);
> +
> +	return drm_bridge_attach(bridge->encoder, encoder_hdmi->next_bridge,
> +				 &encoder_hdmi->bridge, flags);
> +}
> +
> +static void meson_encoder_hdmi_enable(struct drm_bridge *bridge)
> +{
> +	struct meson_encoder_hdmi *encoder_hdmi = bridge_to_meson_encoder_hdmi(bridge);
> +	struct meson_drm *priv = encoder_hdmi->priv;
> +
> +	dev_dbg(priv->dev, "%s\n", priv->venc.hdmi_use_enci ? "VENCI" : "VENCP");
> +
> +	if (priv->venc.hdmi_use_enci)
> +		writel_relaxed(1, priv->io_base + _REG(ENCI_VIDEO_EN));
> +	else
> +		writel_relaxed(1, priv->io_base + _REG(ENCP_VIDEO_EN));
> +}
> +
> +static void meson_encoder_hdmi_disable(struct drm_bridge *bridge)
> +{
> +	struct meson_encoder_hdmi *encoder_hdmi = bridge_to_meson_encoder_hdmi(bridge);
> +	struct meson_drm *priv = encoder_hdmi->priv;
> +
> +	writel_bits_relaxed(0x3, 0,
> +			    priv->io_base + _REG(VPU_HDMI_SETTING));
> +
> +	writel_relaxed(0, priv->io_base + _REG(ENCI_VIDEO_EN));
> +	writel_relaxed(0, priv->io_base + _REG(ENCP_VIDEO_EN));
> +}
> +
> +static void meson_encoder_hdmi_set_vclk(struct meson_encoder_hdmi *encoder_hdmi,
> +					const struct drm_display_mode *mode)
> +{
> +	struct meson_drm *priv = encoder_hdmi->priv;
> +	int vic = drm_match_cea_mode(mode);
> +	unsigned int phy_freq;
> +	unsigned int vclk_freq;
> +	unsigned int venc_freq;
> +	unsigned int hdmi_freq;
> +
> +	vclk_freq = mode->clock;
> +
> +	/* For 420, pixel clock is half unlike venc clock */
> +	if (encoder_hdmi->output_bus_fmt == MEDIA_BUS_FMT_UYYVYY8_0_5X24)
> +		vclk_freq /= 2;
> +
> +	/* TMDS clock is pixel_clock * 10 */
> +	phy_freq = vclk_freq * 10;
> +
> +	if (!vic) {
> +		meson_vclk_setup(priv, MESON_VCLK_TARGET_DMT, phy_freq,
> +				 vclk_freq, vclk_freq, vclk_freq, false);
> +		return;
> +	}
> +
> +	/* 480i/576i needs global pixel doubling */
> +	if (mode->flags & DRM_MODE_FLAG_DBLCLK)
> +		vclk_freq *= 2;
> +
> +	venc_freq = vclk_freq;
> +	hdmi_freq = vclk_freq;
> +
> +	/* VENC double pixels for 1080i, 720p and YUV420 modes */
> +	if (meson_venc_hdmi_venc_repeat(vic) ||
> +	    encoder_hdmi->output_bus_fmt == MEDIA_BUS_FMT_UYYVYY8_0_5X24)
> +		venc_freq *= 2;
> +
> +	vclk_freq = max(venc_freq, hdmi_freq);
> +
> +	if (mode->flags & DRM_MODE_FLAG_DBLCLK)
> +		venc_freq /= 2;
> +
> +	dev_dbg(priv->dev, "vclk:%d phy=%d venc=%d hdmi=%d enci=%d\n",
> +		phy_freq, vclk_freq, venc_freq, hdmi_freq,
> +		priv->venc.hdmi_use_enci);
> +
> +	meson_vclk_setup(priv, MESON_VCLK_TARGET_HDMI, phy_freq, vclk_freq,
> +			 venc_freq, hdmi_freq, priv->venc.hdmi_use_enci);
> +}
> +
> +static enum drm_mode_status meson_encoder_hdmi_mode_valid(struct drm_bridge *bridge,
> +					const struct drm_display_info *display_info,
> +					const struct drm_display_mode *mode)
> +{
> +	struct meson_encoder_hdmi *encoder_hdmi = bridge_to_meson_encoder_hdmi(bridge);
> +	struct meson_drm *priv = encoder_hdmi->priv;
> +	bool is_hdmi2_sink = display_info->hdmi.scdc.supported;
> +	unsigned int phy_freq;
> +	unsigned int vclk_freq;
> +	unsigned int venc_freq;
> +	unsigned int hdmi_freq;
> +	int vic = drm_match_cea_mode(mode);
> +	enum drm_mode_status status;
> +
> +	dev_dbg(priv->dev, "Modeline " DRM_MODE_FMT "\n", DRM_MODE_ARG(mode));
> +
> +	/* If sink does not support 540MHz, reject the non-420 HDMI2 modes */
> +	if (display_info->max_tmds_clock &&
> +	    mode->clock > display_info->max_tmds_clock &&
> +	    !drm_mode_is_420_only(display_info, mode) &&
> +	    !drm_mode_is_420_also(display_info, mode))
> +		return MODE_BAD;
> +
> +	/* Check against non-VIC supported modes */
> +	if (!vic) {
> +		status = meson_venc_hdmi_supported_mode(mode);
> +		if (status != MODE_OK)
> +			return status;
> +
> +		return meson_vclk_dmt_supported_freq(priv, mode->clock);
> +	/* Check against supported VIC modes */
> +	} else if (!meson_venc_hdmi_supported_vic(vic))
> +		return MODE_BAD;
> +
> +	vclk_freq = mode->clock;
> +
> +	/* For 420, pixel clock is half unlike venc clock */
> +	if (drm_mode_is_420_only(display_info, mode) ||
> +	    (!is_hdmi2_sink &&
> +	     drm_mode_is_420_also(display_info, mode)))
> +		vclk_freq /= 2;
> +
> +	/* TMDS clock is pixel_clock * 10 */
> +	phy_freq = vclk_freq * 10;
> +
> +	/* 480i/576i needs global pixel doubling */
> +	if (mode->flags & DRM_MODE_FLAG_DBLCLK)
> +		vclk_freq *= 2;
> +
> +	venc_freq = vclk_freq;
> +	hdmi_freq = vclk_freq;
> +
> +	/* VENC double pixels for 1080i, 720p and YUV420 modes */
> +	if (meson_venc_hdmi_venc_repeat(vic) ||
> +	    drm_mode_is_420_only(display_info, mode) ||
> +	    (!is_hdmi2_sink &&
> +	     drm_mode_is_420_also(display_info, mode)))
> +		venc_freq *= 2;
> +
> +	vclk_freq = max(venc_freq, hdmi_freq);
> +
> +	if (mode->flags & DRM_MODE_FLAG_DBLCLK)
> +		venc_freq /= 2;
> +
> +	dev_dbg(priv->dev, "%s: vclk:%d phy=%d venc=%d hdmi=%d\n",
> +		__func__, phy_freq, vclk_freq, venc_freq, hdmi_freq);
> +
> +	return meson_vclk_vic_supported_freq(priv, phy_freq, vclk_freq);
> +}
> +
> +static void meson_encoder_hdmi_atomic_enable(struct drm_bridge *bridge,
> +					     struct drm_bridge_state *bridge_state)
> +{
> +	struct meson_encoder_hdmi *encoder_hdmi = bridge_to_meson_encoder_hdmi(bridge);
> +	struct drm_atomic_state *state = bridge_state->base.state;
> +	unsigned int ycrcb_map = VPU_HDMI_OUTPUT_CBYCR;
> +	struct meson_drm *priv = encoder_hdmi->priv;
> +	struct drm_connector_state *conn_state;
> +	const struct drm_display_mode *mode;
> +	struct drm_crtc_state *crtc_state;
> +	struct drm_connector *connector;
> +	bool yuv420_mode = false;
> +	int vic;
> +
> +	connector = drm_atomic_get_new_connector_for_encoder(state, bridge->encoder);
> +	if (WARN_ON(!connector))
> +		return;
> +
> +	conn_state = drm_atomic_get_new_connector_state(state, connector);
> +	if (WARN_ON(!conn_state))
> +		return;
> +
> +	crtc_state = drm_atomic_get_new_crtc_state(state, conn_state->crtc);
> +	if (WARN_ON(!crtc_state))
> +		return;
> +
> +	mode = &crtc_state->adjusted_mode;
> +
> +	vic = drm_match_cea_mode(mode);
> +
> +	dev_dbg(priv->dev, "\"%s\" vic %d\n", mode->name, vic);
> +
> +	if (encoder_hdmi->output_bus_fmt == MEDIA_BUS_FMT_UYYVYY8_0_5X24) {
> +		ycrcb_map = VPU_HDMI_OUTPUT_CRYCB;
> +		yuv420_mode = true;
> +	}
> +
> +	/* VENC + VENC-DVI Mode setup */
> +	meson_venc_hdmi_mode_set(priv, vic, ycrcb_map, yuv420_mode, mode);
> +
> +	/* VCLK Set clock */
> +	meson_encoder_hdmi_set_vclk(encoder_hdmi, mode);
> +
> +	if (encoder_hdmi->output_bus_fmt == MEDIA_BUS_FMT_UYYVYY8_0_5X24)
> +		/* Setup YUV420 to HDMI-TX, no 10bit diphering */
> +		writel_relaxed(2 | (2 << 2),
> +			       priv->io_base + _REG(VPU_HDMI_FMT_CTRL));
> +	else
> +		/* Setup YUV444 to HDMI-TX, no 10bit diphering */
> +		writel_relaxed(0, priv->io_base + _REG(VPU_HDMI_FMT_CTRL));
> +}
> +
> +static const u32 meson_encoder_hdmi_out_bus_fmts[] = {
> +	MEDIA_BUS_FMT_YUV8_1X24,
> +	MEDIA_BUS_FMT_UYYVYY8_0_5X24,
> +};
> +
> +static u32 *
> +meson_encoder_hdmi_get_inp_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)
> +{
> +	u32 *input_fmts = NULL;
> +	int i;
> +
> +	*num_input_fmts = 0;
> +
> +	for (i = 0 ; i < ARRAY_SIZE(meson_encoder_hdmi_out_bus_fmts) ; ++i) {
> +		if (output_fmt == meson_encoder_hdmi_out_bus_fmts[i]) {
> +			*num_input_fmts = 1;
> +			input_fmts = kcalloc(*num_input_fmts,
> +					     sizeof(*input_fmts),
> +					     GFP_KERNEL);
> +			if (!input_fmts)
> +				return NULL;
> +
> +			input_fmts[0] = output_fmt;
> +
> +			break;
> +		}
> +	}
> +
> +	return input_fmts;
> +}
> +
> +static int meson_encoder_hdmi_atomic_check(struct drm_bridge *bridge,
> +					struct drm_bridge_state *bridge_state,
> +					struct drm_crtc_state *crtc_state,
> +					struct drm_connector_state *conn_state)
> +{
> +	struct meson_encoder_hdmi *encoder_hdmi = bridge_to_meson_encoder_hdmi(bridge);
> +	struct drm_connector_state *old_conn_state =
> +		drm_atomic_get_old_connector_state(conn_state->state, conn_state->connector);
> +	struct meson_drm *priv = encoder_hdmi->priv;
> +
> +	encoder_hdmi->output_bus_fmt = bridge_state->output_bus_cfg.format;
> +
> +	dev_dbg(priv->dev, "output_bus_fmt %lx\n", encoder_hdmi->output_bus_fmt);
> +
> +	if (!drm_connector_atomic_hdr_metadata_equal(old_conn_state, conn_state))
> +		crtc_state->mode_changed = true;
> +
> +	return 0;
> +}
> +
> +static const struct drm_bridge_funcs meson_encoder_hdmi_bridge_funcs = {
> +	.attach = meson_encoder_hdmi_attach,
> +	.enable	= meson_encoder_hdmi_enable,
> +	.disable = meson_encoder_hdmi_disable,
> +	.mode_valid = meson_encoder_hdmi_mode_valid,
> +	.atomic_enable = meson_encoder_hdmi_atomic_enable,
> +	.atomic_get_input_bus_fmts = meson_encoder_hdmi_get_inp_bus_fmts,
> +	.atomic_check = meson_encoder_hdmi_atomic_check,
> +	.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,
> +};
> +
> +int meson_encoder_hdmi_init(struct meson_drm *priv)
> +{
> +	struct meson_encoder_hdmi *meson_encoder_hdmi;
> +	struct device_node *remote;
> +	int ret;
> +
> +	meson_encoder_hdmi = devm_kzalloc(priv->dev, sizeof(*meson_encoder_hdmi), GFP_KERNEL);
> +	if (!meson_encoder_hdmi)
> +		return -ENOMEM;
> +
> +	/* HDMI Transceiver Bridge */
> +	remote = of_graph_get_remote_node(priv->dev->of_node, 1, 0);
> +	if (!remote) {
> +		dev_err(priv->dev, "HDMI transceiver device is disabled");
> +		return 0;
> +	}
> +
> +	meson_encoder_hdmi->next_bridge = of_drm_find_bridge(remote);
> +	if (!meson_encoder_hdmi->next_bridge) {
> +		dev_err(priv->dev, "Failed to find HDMI transceiver bridge\n");
> +		return -EPROBE_DEFER;
> +	}
> +
> +	/* HDMI Encoder Bridge */
> +	meson_encoder_hdmi->bridge.funcs = &meson_encoder_hdmi_bridge_funcs;
> +	meson_encoder_hdmi->bridge.of_node = priv->dev->of_node;
> +	meson_encoder_hdmi->bridge.type = DRM_MODE_CONNECTOR_HDMIA;
> +
> +	drm_bridge_add(&meson_encoder_hdmi->bridge);
> +
> +	meson_encoder_hdmi->priv = priv;
> +
> +	/* Encoder */
> +	ret = drm_simple_encoder_init(priv->drm, &meson_encoder_hdmi->encoder,
> +				      DRM_MODE_ENCODER_TMDS);

Ideally this should be moved one level up, as the bridge side shouldn't
care about drm_encoder. Creating the encoder, attaching to the bridge
and creating the connector using the drm bridge connector helper all
belong together in the display controller driver outside of bridge code,
and once all encoders have been converted to the new model, it can be
done unconditionally without caring about the type of bridge.

This can be done on top of this series.

> +	if (ret) {
> +		dev_err(priv->dev, "Failed to init HDMI encoder: %d\n", ret);
> +		return ret;
> +	}
> +
> +	meson_encoder_hdmi->encoder.possible_crtcs = BIT(0);
> +
> +	/* Attach HDMI Encoder Bridge to Encoder */
> +	ret = drm_bridge_attach(&meson_encoder_hdmi->encoder, &meson_encoder_hdmi->bridge, NULL, 0);
> +	if (ret) {
> +		dev_err(priv->dev, "Failed to attach bridge: %d\n", ret);
> +		return ret;
> +	}
> +
> +	/*
> +	 * We should have now in place:
> +	 * encoder->[hdmi encoder bridge]->[dw-hdmi bridge]->[dw-hdmi connector]
> +	 */
> +
> +	dev_dbg(priv->dev, "HDMI encoder initialized\n");
> +
> +	return 0;
> +}
> diff --git a/drivers/gpu/drm/meson/meson_encoder_hdmi.h b/drivers/gpu/drm/meson/meson_encoder_hdmi.h
> new file mode 100644
> index 000000000000..ed19494f0956
> --- /dev/null
> +++ b/drivers/gpu/drm/meson/meson_encoder_hdmi.h
> @@ -0,0 +1,12 @@
> +/* SPDX-License-Identifier: GPL-2.0-or-later */
> +/*
> + * Copyright (C) 2021 BayLibre, SAS
> + * Author: Neil Armstrong <narmstrong@baylibre.com>
> + */
> +
> +#ifndef __MESON_ENCODER_HDMI_H
> +#define __MESON_ENCODER_HDMI_H
> +
> +int meson_encoder_hdmi_init(struct meson_drm *priv);
> +
> +#endif /* __MESON_ENCODER_HDMI_H */

-- 
Regards,

Laurent Pinchart

^ permalink raw reply	[flat|nested] 16+ messages in thread

end of thread, other threads:[~2021-10-18 14:02 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-10-15 14:11 [PATCH v2 0/6] drm/meson: rework encoders to pass ATTACH_NO_CONNECTOR Neil Armstrong
2021-10-15 14:11 ` [PATCH v2 1/6] drm/bridge: display-connector: implement bus fmts callbacks Neil Armstrong
2021-10-15 14:11 ` [PATCH v2 2/6] drm/meson: remove useless recursive components matching Neil Armstrong
2021-10-15 21:59   ` Martin Blumenstingl
2021-10-15 14:11 ` [PATCH v2 3/6] drm/meson: split out encoder from meson_dw_hdmi Neil Armstrong
2021-10-15 22:01   ` Martin Blumenstingl
2021-10-18 13:58   ` Laurent Pinchart
2021-10-15 14:11 ` [PATCH v2 4/6] drm/meson: encoder_hdmi: switch to bridge DRM_BRIDGE_ATTACH_NO_CONNECTOR Neil Armstrong
2021-10-15 22:07   ` Martin Blumenstingl
2021-10-18  7:50     ` Neil Armstrong
2021-10-15 14:11 ` [PATCH v2 5/6] drm/meson: rename venc_cvbs to encoder_cvbs Neil Armstrong
2021-10-15 22:07   ` Martin Blumenstingl
2021-10-15 14:11 ` [PATCH v2 6/6] drm/meson: encoder_cvbs: switch to bridge with ATTACH_NO_CONNECTOR Neil Armstrong
2021-10-15 22:07   ` Martin Blumenstingl
2021-10-15 22:34   ` Martin Blumenstingl
2021-10-18  7:40     ` Neil Armstrong

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).