[PATCH 0/7] drm/meson: rework encoders to pass ATTACH_NO_CONNECTOR

[PATCH 0/7] drm/meson: rework encoders to pass ATTACH_NO_CONNECTOR

[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 writting 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.

Neil Armstrong (7):
  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/bridge: synopsys: dw-hdmi: also allow interlace on bridge
  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    |  88 ++++
 drivers/gpu/drm/bridge/synopsys/dw-hdmi.c     |   1 +
 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 +-------------
 ...meson_venc_cvbs.c => meson_encoder_cvbs.c} | 198 ++++----
 ...meson_venc_cvbs.h => meson_encoder_cvbs.h} |   2 +-
 drivers/gpu/drm/meson/meson_encoder_hdmi.c    | 436 ++++++++++++++++++
 drivers/gpu/drm/meson/meson_encoder_hdmi.h    |  12 +
 10 files changed, 679 insertions(+), 476 deletions(-)
 rename drivers/gpu/drm/meson/{meson_venc_cvbs.c => meson_encoder_cvbs.c} (51%)
 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


base-commit: e4e737bb5c170df6135a127739a9e6148ee3da82
-- 
2.25.1

[PATCH 1/7] drm/bridge: display-connector: implement bus fmts callbacks

[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>
---
 drivers/gpu/drm/bridge/display-connector.c | 88 ++++++++++++++++++++++
 1 file changed, 88 insertions(+)

diff --git a/drivers/gpu/drm/bridge/display-connector.c b/drivers/gpu/drm/bridge/display-connector.c
index 05eb759da6fc..9697ac173157 100644
--- a/drivers/gpu/drm/bridge/display-connector.c
+++ b/drivers/gpu/drm/bridge/display-connector.c
@@ -14,6 +14,7 @@
 #include <linux/regulator/consumer.h>
 
 #include <drm/drm_bridge.h>
+#include <drm/drm_atomic_helper.h>
 #include <drm/drm_edid.h>
 
 struct display_connector {
@@ -87,10 +88,97 @@ 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 permits avoiding skipping the negociation 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 return fallback data like done in the bridge function:
+ * select_bus_fmt_recursive() when atomic_get_input_bus_fmts is not supported.
+ * This permits avoiding skipping the negociation 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

[PATCH 2/7] drm/meson: remove useless recursive components matching

[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>
---
 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 bc0d60df04ae..b53606d8825f 100644
--- a/drivers/gpu/drm/meson/meson_drv.c
+++ b/drivers/gpu/drm/meson/meson_drv.c
@@ -427,46 +427,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);
@@ -478,6 +438,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;
@@ -492,8 +459,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;
+		}
+
+		DRM_DEBUG_DRIVER("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

[PATCH 3/7] drm/meson: split out encoder from meson_dw_hdmi

[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, the encoder init uses the
simple kms helper and for now the bridge attach flags is 0.

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 | 359 +++++++++++++++++++++
 drivers/gpu/drm/meson/meson_encoder_hdmi.h |  12 +
 5 files changed, 396 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 b53606d8825f..0a28a8e29d63 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"
@@ -319,6 +320,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 2ed87cfdd735..c2480b3335ac 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,19 +702,12 @@ 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;
 	struct resource *res;
 	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");
@@ -1004,7 +723,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)) {
@@ -1076,28 +794,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;
@@ -1112,15 +813,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..c775a1ab5b1e
--- /dev/null
+++ b/drivers/gpu/drm/meson/meson_encoder_hdmi.c
@@ -0,0 +1,359 @@
+// 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_simple_kms_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_print.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;
+
+	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_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;
+
+	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_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;
+
+	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 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;
+
+	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(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_mode_set(struct drm_bridge *bridge,
+				   const struct drm_display_mode *mode,
+				   const struct drm_display_mode *adjusted_mode)
+{
+	struct meson_encoder_hdmi *encoder_hdmi = bridge_to_meson_encoder_hdmi(bridge);
+	struct meson_drm *priv = encoder_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 (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);
+
+	encoder_hdmi->output_bus_fmt = bridge_state->output_bus_cfg.format;
+
+	DRM_DEBUG_DRIVER("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,
+	.mode_set = meson_encoder_hdmi_mode_set,
+	.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;
+
+	DRM_DEBUG_DRIVER("\n");
+
+	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]
+	 */
+
+	DRM_DEBUG_DRIVER("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

[PATCH 4/7] drm/bridge: synopsys: dw-hdmi: also allow interlace on bridge

[Neil Armstrong]

Since we allow interlace on the encoder, also allow it on the bridge
so we can allow interlaced modes when using DRM_BRIDGE_ATTACH_NO_CONNECTOR.

Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
---
 drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
index f08d0fded61f..62ae63565d3a 100644
--- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
+++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
@@ -3413,6 +3413,7 @@ struct dw_hdmi *dw_hdmi_probe(struct platform_device *pdev,
 	hdmi->bridge.funcs = &dw_hdmi_bridge_funcs;
 	hdmi->bridge.ops = DRM_BRIDGE_OP_DETECT | DRM_BRIDGE_OP_EDID
 			 | DRM_BRIDGE_OP_HPD;
+	hdmi->bridge.interlace_allowed = true;
 #ifdef CONFIG_OF
 	hdmi->bridge.of_node = pdev->dev.of_node;
 #endif
-- 
2.25.1

[PATCH 5/7] drm/meson: encoder_hdmi: switch to bridge DRM_BRIDGE_ATTACH_NO_CONNECTOR

[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>
---
 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 c2480b3335ac..933ff63b1ecf 100644
--- a/drivers/gpu/drm/meson/meson_dw_hdmi.c
+++ b/drivers/gpu/drm/meson/meson_dw_hdmi.c
@@ -805,6 +805,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 c775a1ab5b1e..a4264587d89a 100644
--- a/drivers/gpu/drm/meson/meson_encoder_hdmi.c
+++ b/drivers/gpu/drm/meson/meson_encoder_hdmi.c
@@ -14,9 +14,12 @@
 #include <linux/regulator/consumer.h>
 #include <linux/reset.h>
 
+#include <media/cec-notifier.h>
+
 #include <drm/drm_atomic_helper.h>
 #include <drm/drm_simple_kms_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>
@@ -34,8 +37,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) \
@@ -50,6 +55,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);
@@ -284,12 +297,33 @@ 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,
 	.mode_set = meson_encoder_hdmi_mode_set,
+	.hpd_notify = meson_encoder_hdmi_hpd_notify,
 	.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,
@@ -300,6 +334,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;
 
@@ -326,6 +361,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);
 
@@ -342,17 +378,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;
+	}
+
 	DRM_DEBUG_DRIVER("HDMI encoder initialized\n");
 
 	return 0;
-- 
2.25.1

[PATCH 6/7] drm/meson: rename venc_cvbs to encoder_cvbs

[Neil Armstrong]

Rename the cvbs encoder to match the newly introduced meson_encoder_hdmi.

Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
---
 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 0a28a8e29d63..d07d302ce4a6 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"
@@ -308,7 +308,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

[PATCH 7/7] drm/meson: encoder_cvbs: switch to bridge with ATTACH_NO_CONNECTOR

[Neil Armstrong]

Drop the local connector and move all callback to bridge funcs in order
to leverage the generic CVBS diplay 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>
---
 drivers/gpu/drm/meson/meson_encoder_cvbs.c | 178 +++++++++------------
 1 file changed, 79 insertions(+), 99 deletions(-)

diff --git a/drivers/gpu/drm/meson/meson_encoder_cvbs.c b/drivers/gpu/drm/meson/meson_encoder_cvbs.c
index 01024c5f610c..0b974667cf55 100644
--- a/drivers/gpu/drm/meson/meson_encoder_cvbs.c
+++ b/drivers/gpu/drm/meson/meson_encoder_cvbs.c
@@ -13,6 +13,9 @@
 #include <linux/of_graph.h>
 
 #include <drm/drm_atomic_helper.h>
+#include <drm/drm_simple_kms_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>
@@ -30,14 +33,14 @@
 
 struct meson_encoder_cvbs {
 	struct drm_encoder	encoder;
-	struct drm_connector	connector;
+	struct drm_bridge	bridge;
+	struct drm_bridge	*next_bridge;
+	struct drm_connector	*connector;
 	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,21 +84,18 @@ 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 drm_display_mode *mode;
@@ -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,
-};
+	if (meson_cvbs_get_mode(mode))
+		return MODE_OK;
 
-/* Encoder */
-
-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,13 +176,13 @@ 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_mode_set(struct drm_bridge *bridge,
+				   const struct drm_display_mode *mode,
+				   const struct drm_display_mode *adjusted_mode)
 {
 	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);
+					bridge_to_meson_encoder_cvbs(bridge);
 	struct meson_drm *priv = meson_encoder_cvbs->priv;
 
 	if (meson_mode) {
@@ -218,76 +196,78 @@ 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,
+	.mode_set = meson_encoder_cvbs_mode_set,
+	.get_modes = meson_encoder_cvbs_get_modes,
+	.atomic_check = meson_encoder_cvbs_atomic_check,
 };
 
-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 */
+	meson_encoder_cvbs->connector = drm_bridge_connector_init(priv->drm,
+							&meson_encoder_cvbs->encoder);
+	if (IS_ERR(meson_encoder_cvbs->connector)) {
+		dev_err(priv->dev, "Unable to create CVBS bridge connector\n");
+		return PTR_ERR(meson_encoder_cvbs->connector);
+	}
+	drm_connector_attach_encoder(meson_encoder_cvbs->connector,
+				     &meson_encoder_cvbs->encoder);
 
 	return 0;
 }
-- 
2.25.1

Re: [PATCH 2/7] drm/meson: remove useless recursive components matching

[Neil Armstrong]

Hi Sam,

On 14/10/2021 19:49, Sam Ravnborg wrote:
> Hi Neil,
> 
> one comment below. Other than that
> Acked-by: Sam Ravnborg <sam@ravnborg.org>
> 
> 	Sam
> 
> On Thu, Oct 14, 2021 at 05:26:01PM +0200, Neil Armstrong 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>
>> ---
>>  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 bc0d60df04ae..b53606d8825f 100644
>> --- a/drivers/gpu/drm/meson/meson_drv.c
>> +++ b/drivers/gpu/drm/meson/meson_drv.c
>> @@ -427,46 +427,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);
>> @@ -478,6 +438,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;
>> @@ -492,8 +459,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;
>> +		}
>> +
>> +		DRM_DEBUG_DRIVER("parent %pOF remote match add %pOF parent %s\n",
>> +				  np, remote, dev_name(&pdev->dev));
> Avoid the deprecated logging functions.
> Use drm_dbg() or if there is no drm_device just dev_dbg().
> 
> I assume the driver uses DRM_xxx all over, so I understand if you keep
> it as-is.

Since it's in the probe function, I will move to dev_dbg().
I'll probably do a print cleanup all over the driver.

> 
>> +
>> +		component_match_add(&pdev->dev, &match, compare_of, remote);
>> +
>>  		of_node_put(remote);
>> +
>> +		++count;
>>  	}
>>  
>>  	if (count && !match)
>> -- 
>> 2.25.1

Thanks,
Neil

Re: [PATCH 3/7] drm/meson: split out encoder from meson_dw_hdmi

[Neil Armstrong]

Hi,

On 14/10/2021 20:07, Sam Ravnborg wrote:
> Hi Neil,
> 
> I did not verify all the code movements - but it looked correct from a
> quick glance.
> A few comments below, especially the use of mode_set() should be
> addressed as it is deprecated.

I was not sure about mode_set, will move to atomic_enable and address all the other comments,

Thanks,
Neil

> 
> 	Sam
> 
> On Thu, Oct 14, 2021 at 05:26:02PM +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, the encoder init uses the
>> simple kms helper and for now the bridge attach flags is 0.
>>
>> 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 | 359 +++++++++++++++++++++
>>  drivers/gpu/drm/meson/meson_encoder_hdmi.h |  12 +
>>  5 files changed, 396 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 b53606d8825f..0a28a8e29d63 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"
>> @@ -319,6 +320,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 2ed87cfdd735..c2480b3335ac 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,19 +702,12 @@ 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;
>>  	struct resource *res;
>>  	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");
>> @@ -1004,7 +723,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)) {
>> @@ -1076,28 +794,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;
>> @@ -1112,15 +813,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..c775a1ab5b1e
>> --- /dev/null
>> +++ b/drivers/gpu/drm/meson/meson_encoder_hdmi.c
>> @@ -0,0 +1,359 @@
>> +// 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_simple_kms_helper.h>
> alphabetic order.
>> +#include <drm/drm_bridge.h>
>> +#include <drm/drm_device.h>
>> +#include <drm/drm_edid.h>
>> +#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_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;
>> +
>> +	DRM_DEBUG_DRIVER("%s\n", priv->venc.hdmi_use_enci ? "VENCI" : "VENCP");
> Bridge drivers mainly uses dev_xxx logging.
> New drivers do not use the deprecated DRM_ logging.
> 
> This goes for all logging in this file.
> Remember to drop include of drm_print.h
> 
> 
>> +
>> +	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;
>> +
>> +	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_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;
>> +
>> +	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 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;
>> +
>> +	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(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_mode_set(struct drm_bridge *bridge,
>> +				   const struct drm_display_mode *mode,
>> +				   const struct drm_display_mode *adjusted_mode)
>> +{
>> +	struct meson_encoder_hdmi *encoder_hdmi = bridge_to_meson_encoder_hdmi(bridge);
>> +	struct meson_drm *priv = encoder_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 (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);
> In code similar to this in a previous patch kmalloc() was used.
> Both are OK, I just noticed that the code differs.
> 
>> +			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);
>> +
>> +	encoder_hdmi->output_bus_fmt = bridge_state->output_bus_cfg.format;
>> +
>> +	DRM_DEBUG_DRIVER("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,
>> +	.mode_set = meson_encoder_hdmi_mode_set,
> Use of mode_set is deprecated, new drivers shall set their mode in the
> atomic_enable operation.
>> +	.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;
>> +
>> +	DRM_DEBUG_DRIVER("\n");
>> +
>> +	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]
>> +	 */
>> +
>> +	DRM_DEBUG_DRIVER("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

Re: [PATCH 7/7] drm/meson: encoder_cvbs: switch to bridge with ATTACH_NO_CONNECTOR

[Neil Armstrong]

Hi Sam,

On 14/10/2021 20:15, Sam Ravnborg wrote:
> Hi Neil,
> 
> with include order fixed and the comment below considered:
> Acked-by: Sam Ravnborg <sam@ravnborg.org>
> 
> 	Sam
> 
> 
> On Thu, Oct 14, 2021 at 05:26:06PM +0200, Neil Armstrong wrote:
>> Drop the local connector and move all callback to bridge funcs in order
>> to leverage the generic CVBS diplay 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>
>> ---
>>  drivers/gpu/drm/meson/meson_encoder_cvbs.c | 178 +++++++++------------
>>  1 file changed, 79 insertions(+), 99 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/meson/meson_encoder_cvbs.c b/drivers/gpu/drm/meson/meson_encoder_cvbs.c
>> index 01024c5f610c..0b974667cf55 100644
>> --- a/drivers/gpu/drm/meson/meson_encoder_cvbs.c
>> +++ b/drivers/gpu/drm/meson/meson_encoder_cvbs.c
>> @@ -13,6 +13,9 @@
>>  #include <linux/of_graph.h>
>>  
>>  #include <drm/drm_atomic_helper.h>
>> +#include <drm/drm_simple_kms_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>
> alphabetic order.
> 
>> @@ -30,14 +33,14 @@
>>  
>>  struct meson_encoder_cvbs {
>>  	struct drm_encoder	encoder;
>> -	struct drm_connector	connector;
>> +	struct drm_bridge	bridge;
>> +	struct drm_bridge	*next_bridge;
>> +	struct drm_connector	*connector;
> Maybe drop this - see later.
> 
>>  	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,21 +84,18 @@ 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 drm_display_mode *mode;
>> @@ -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,
>> -};
>> +	if (meson_cvbs_get_mode(mode))
>> +		return MODE_OK;
>>  
>> -/* Encoder */
>> -
>> -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,13 +176,13 @@ 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_mode_set(struct drm_bridge *bridge,
>> +				   const struct drm_display_mode *mode,
>> +				   const struct drm_display_mode *adjusted_mode)
>>  {
>>  	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);
>> +					bridge_to_meson_encoder_cvbs(bridge);
>>  	struct meson_drm *priv = meson_encoder_cvbs->priv;
>>  
>>  	if (meson_mode) {
>> @@ -218,76 +196,78 @@ 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,
>> +	.mode_set = meson_encoder_cvbs_mode_set,
>> +	.get_modes = meson_encoder_cvbs_get_modes,
>> +	.atomic_check = meson_encoder_cvbs_atomic_check,
>>  };
>>  
>> -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 */
>> +	meson_encoder_cvbs->connector = drm_bridge_connector_init(priv->drm,
>> +							&meson_encoder_cvbs->encoder);
> I did not see other uses of meson_encoder_cvbs->connector, so if I am
> right a local variable can be used and the member dropped.

You're right, I'll drop this.

Thanks
Neil

> 
>> +	if (IS_ERR(meson_encoder_cvbs->connector)) {
>> +		dev_err(priv->dev, "Unable to create CVBS bridge connector\n");
>> +		return PTR_ERR(meson_encoder_cvbs->connector);
>> +	}
>> +	drm_connector_attach_encoder(meson_encoder_cvbs->connector,
>> +				     &meson_encoder_cvbs->encoder);
>>  
>>  	return 0;
>>  }
>> -- 
>> 2.25.1

Re: [PATCH 4/7] drm/bridge: synopsys: dw-hdmi: also allow interlace on bridge

[Neil Armstrong]

Hi Sam,

On 14/10/2021 20:08, Sam Ravnborg wrote:
> On Thu, Oct 14, 2021 at 05:26:03PM +0200, Neil Armstrong wrote:
>> Since we allow interlace on the encoder, also allow it on the bridge
>> so we can allow interlaced modes when using DRM_BRIDGE_ATTACH_NO_CONNECTOR.
>>
>> Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
> Acked-by: Sam Ravnborg <sam@ravnborg.org>

I applied it to drm-misc-next,

Thanks !

Neil

>> ---
>>  drivers/gpu/drm/bridge/synopsys/dw-hdmi.c | 1 +
>>  1 file changed, 1 insertion(+)
>>
>> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
>> index f08d0fded61f..62ae63565d3a 100644
>> --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
>> +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
>> @@ -3413,6 +3413,7 @@ struct dw_hdmi *dw_hdmi_probe(struct platform_device *pdev,
>>  	hdmi->bridge.funcs = &dw_hdmi_bridge_funcs;
>>  	hdmi->bridge.ops = DRM_BRIDGE_OP_DETECT | DRM_BRIDGE_OP_EDID
>>  			 | DRM_BRIDGE_OP_HPD;
>> +	hdmi->bridge.interlace_allowed = true;
>>  #ifdef CONFIG_OF
>>  	hdmi->bridge.of_node = pdev->dev.of_node;
>>  #endif
>> -- 
>> 2.25.1