[PATCH v2 0/3] drm/bridge: reuse DRM HDMI Audio helpers for DisplayPort bridges

[PATCH v2 0/3] drm/bridge: reuse DRM HDMI Audio helpers for DisplayPort bridges

[Dmitry Baryshkov]

A lot of DisplayPort bridges use HDMI Codec in order to provide audio
support. Present DRM HDMI Audio support has been written with the HDMI
and in particular DRM HDMI Connector framework support, however those
audio helpers can be easily reused for DisplayPort drivers too.

Patches by Hermes Wu that targeted implementing HDMI Audio support in
the iTE IT6506 driver pointed out the necessity of allowing one to use
generic audio helpers for DisplayPort drivers, as otherwise each driver
has to manually (and correctly) implement the get_eld() and plugged_cb
support.

Implement necessary integration in drm_bridge_connector and provide an
example implementation in the msm/dp driver.

The plan is to land core parts via the drm-misc-next tree and msm patch
via the msm-next tree.

Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
---
Changes in v2:
- Added drm_connector_attach_dp_subconnector_property() patches
- Link to v1: https://lore.kernel.org/r/20250206-dp-hdmi-audio-v1-0-8aa14a8c0d4d@linaro.org

---
Dmitry Baryshkov (3):
      drm/display: bridge-connector: add DisplayPort bridges
      drm/display: bridge_connector: add DisplayPort subconnector property
      drm/msm/dp: reuse generic HDMI codec implementation

 drivers/gpu/drm/display/drm_bridge_connector.c |  68 ++++++++++---
 drivers/gpu/drm/msm/Kconfig                    |   1 +
 drivers/gpu/drm/msm/dp/dp_audio.c              | 131 +++----------------------
 drivers/gpu/drm/msm/dp/dp_audio.h              |  27 ++---
 drivers/gpu/drm/msm/dp/dp_display.c            |  31 ++----
 drivers/gpu/drm/msm/dp/dp_display.h            |   6 --
 drivers/gpu/drm/msm/dp/dp_drm.c                |  11 ++-
 include/drm/drm_bridge.h                       |  14 ++-
 8 files changed, 101 insertions(+), 188 deletions(-)
---
base-commit: ed58d103e6da15a442ff87567898768dc3a66987
change-id: 20250206-dp-hdmi-audio-15d9fdbebb9f

Best regards,
-- 
Dmitry Baryshkov <dmitry.baryshkov@linaro.org>

[PATCH v2 1/3] drm/display: bridge-connector: add DisplayPort bridges

[Dmitry Baryshkov]

DRM HDMI Codec framework is useful not only for the HDMI bridges, but
also for the DisplayPort bridges. Add new DRM_BRIDGE_OP_DisplayPort
define in order to distinguish DP bridges. Create HDMI codec device
automatically for DP bridges which have declared audio support.

Note, unlike HDMI devices, which already have a framework to handle HPD
notifications in a standard way, DP drivers don't (yet?) have such a
framework. As such it is necessary to manually call
drm_connector_hdmi_audio_plugged_notify(). This requirement hopefully
can be lifted later on, if/when DRM framework gets better DisplayPort
ports support in the core layer.

Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
---
 drivers/gpu/drm/display/drm_bridge_connector.c | 66 ++++++++++++++++++++------
 include/drm/drm_bridge.h                       | 14 +++++-
 2 files changed, 65 insertions(+), 15 deletions(-)

diff --git a/drivers/gpu/drm/display/drm_bridge_connector.c b/drivers/gpu/drm/display/drm_bridge_connector.c
index 30c736fc0067e31a97db242e5b16ea8a5b4cf359..5e031395b801f9a1371dcb4ac09f3da23e4615dd 100644
--- a/drivers/gpu/drm/display/drm_bridge_connector.c
+++ b/drivers/gpu/drm/display/drm_bridge_connector.c
@@ -98,6 +98,13 @@ struct drm_bridge_connector {
 	 * HDMI connector infrastructure, if any (see &DRM_BRIDGE_OP_HDMI).
 	 */
 	struct drm_bridge *bridge_hdmi;
+	/**
+	 * @bridge_dp:
+	 *
+	 * The bridge in the chain that implements necessary support for the
+	 * DisplayPort connector infrastructure, if any (see &DRM_BRIDGE_OP_DisplayPort).
+	 */
+	struct drm_bridge *bridge_dp;
 };
 
 #define to_drm_bridge_connector(x) \
@@ -496,6 +503,25 @@ static const struct drm_connector_hdmi_audio_funcs drm_bridge_connector_hdmi_aud
 	.mute_stream = drm_bridge_connector_audio_mute_stream,
 };
 
+static int drm_bridge_connector_hdmi_audio_init(struct drm_connector *connector,
+						struct drm_bridge *bridge)
+{
+	if (!bridge->hdmi_audio_max_i2s_playback_channels &&
+	    !bridge->hdmi_audio_spdif_playback)
+		return 0;
+
+	if (!bridge->funcs->hdmi_audio_prepare ||
+	    !bridge->funcs->hdmi_audio_shutdown)
+		return -EINVAL;
+
+	return drm_connector_hdmi_audio_init(connector,
+					     bridge->hdmi_audio_dev,
+					     &drm_bridge_connector_hdmi_audio_funcs,
+					     bridge->hdmi_audio_max_i2s_playback_channels,
+					     bridge->hdmi_audio_spdif_playback,
+					     bridge->hdmi_audio_dai_port);
+}
+
 /* -----------------------------------------------------------------------------
  * Bridge Connector Initialisation
  */
@@ -564,6 +590,8 @@ struct drm_connector *drm_bridge_connector_init(struct drm_device *drm,
 		if (bridge->ops & DRM_BRIDGE_OP_HDMI) {
 			if (bridge_connector->bridge_hdmi)
 				return ERR_PTR(-EBUSY);
+			if (bridge_connector->bridge_dp)
+				return ERR_PTR(-EINVAL);
 			if (!bridge->funcs->hdmi_write_infoframe ||
 			    !bridge->funcs->hdmi_clear_infoframe)
 				return ERR_PTR(-EINVAL);
@@ -576,6 +604,16 @@ struct drm_connector *drm_bridge_connector_init(struct drm_device *drm,
 				max_bpc = bridge->max_bpc;
 		}
 
+		if (bridge->ops & DRM_BRIDGE_OP_DisplayPort) {
+			if (bridge_connector->bridge_dp)
+				return ERR_PTR(-EBUSY);
+			if (bridge_connector->bridge_hdmi)
+				return ERR_PTR(-EINVAL);
+
+			bridge_connector->bridge_dp = bridge;
+
+		}
+
 		if (!drm_bridge_get_next_bridge(bridge))
 			connector_type = bridge->type;
 
@@ -612,21 +650,21 @@ struct drm_connector *drm_bridge_connector_init(struct drm_device *drm,
 		if (ret)
 			return ERR_PTR(ret);
 
-		if (bridge->hdmi_audio_max_i2s_playback_channels ||
-		    bridge->hdmi_audio_spdif_playback) {
-			if (!bridge->funcs->hdmi_audio_prepare ||
-			    !bridge->funcs->hdmi_audio_shutdown)
-				return ERR_PTR(-EINVAL);
+		ret = drm_bridge_connector_hdmi_audio_init(connector, bridge);
+		if (ret)
+			return ERR_PTR(ret);
+	} else if (bridge_connector->bridge_dp) {
+		bridge = bridge_connector->bridge_dp;
 
-			ret = drm_connector_hdmi_audio_init(connector,
-							    bridge->hdmi_audio_dev,
-							    &drm_bridge_connector_hdmi_audio_funcs,
-							    bridge->hdmi_audio_max_i2s_playback_channels,
-							    bridge->hdmi_audio_spdif_playback,
-							    bridge->hdmi_audio_dai_port);
-			if (ret)
-				return ERR_PTR(ret);
-		}
+		ret = drmm_connector_init(drm, connector,
+					  &drm_bridge_connector_funcs,
+					  connector_type, ddc);
+		if (ret)
+			return ERR_PTR(ret);
+
+		ret = drm_bridge_connector_hdmi_audio_init(connector, bridge);
+		if (ret)
+			return ERR_PTR(ret);
 	} else {
 		ret = drmm_connector_init(drm, connector,
 					  &drm_bridge_connector_funcs,
diff --git a/include/drm/drm_bridge.h b/include/drm/drm_bridge.h
index 496dbbd2ad7edff7f091adfbe62de1e33ef0cf07..40f37444426b1b8ded25da9ba9e2963f18ad6267 100644
--- a/include/drm/drm_bridge.h
+++ b/include/drm/drm_bridge.h
@@ -811,9 +811,21 @@ enum drm_bridge_ops {
 	 *
 	 * Note: currently there can be at most one bridge in a chain that sets
 	 * this bit. This is to simplify corresponding glue code in connector
-	 * drivers.
+	 * drivers. Having both HDMI and DisplayPort bridges in the same bridge
+	 * chain is also not allowed.
 	 */
 	DRM_BRIDGE_OP_HDMI = BIT(4),
+	/**
+	 * @DRM_BRIDGE_OP_DisplayPort: The bridge provides DisplayPort connector
+	 * operations. Currently this is limited to the optional HDMI codec
+	 * support.
+	 *
+	 * Note: currently there can be at most one bridge in a chain that sets
+	 * this bit. This is to simplify corresponding glue code in connector
+	 * drivers. Having both HDMI and DisplayPort bridges in the same bridge
+	 * chain is also not allowed.
+	 */
+	DRM_BRIDGE_OP_DisplayPort = BIT(5),
 };
 
 /**

-- 
2.39.5

[PATCH v2 2/3] drm/display: bridge_connector: add DisplayPort subconnector property

[Dmitry Baryshkov]

Create the DisplayPort subconnector property for DP connectors managed
through drm_bridge_connector, removing the need to create it manually by
the drivers.

Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
---
 drivers/gpu/drm/display/drm_bridge_connector.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/gpu/drm/display/drm_bridge_connector.c b/drivers/gpu/drm/display/drm_bridge_connector.c
index 5e031395b801f9a1371dcb4ac09f3da23e4615dd..df9e6b46b40454385f7023310327c5c99d5c6a5a 100644
--- a/drivers/gpu/drm/display/drm_bridge_connector.c
+++ b/drivers/gpu/drm/display/drm_bridge_connector.c
@@ -662,6 +662,8 @@ struct drm_connector *drm_bridge_connector_init(struct drm_device *drm,
 		if (ret)
 			return ERR_PTR(ret);
 
+		drm_connector_attach_dp_subconnector_property(connector);
+
 		ret = drm_bridge_connector_hdmi_audio_init(connector, bridge);
 		if (ret)
 			return ERR_PTR(ret);

-- 
2.39.5

[PATCH v2 3/3] drm/msm/dp: reuse generic HDMI codec implementation

[Dmitry Baryshkov]

The MSM DisplayPort driver implements several HDMI codec functions
in the driver, e.g. it manually manages HDMI codec device registration,
returning ELD and plugged_cb support. In order to reduce code
duplication reuse drm_hdmi_audio_* helpers and drm_bridge_connector
integration.

As a part of this change, also drop the call to
drm_connector_attach_dp_subconnector_property(), it is now being handled
by the drm_bridge_connector.

Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
---
 drivers/gpu/drm/msm/Kconfig         |   1 +
 drivers/gpu/drm/msm/dp/dp_audio.c   | 131 ++++--------------------------------
 drivers/gpu/drm/msm/dp/dp_audio.h   |  27 ++------
 drivers/gpu/drm/msm/dp/dp_display.c |  31 ++-------
 drivers/gpu/drm/msm/dp/dp_display.h |   6 --
 drivers/gpu/drm/msm/dp/dp_drm.c     |  11 ++-
 6 files changed, 34 insertions(+), 173 deletions(-)

diff --git a/drivers/gpu/drm/msm/Kconfig b/drivers/gpu/drm/msm/Kconfig
index 7ec833b6d8292f8cb26cfe5582812f2754cd4d35..fe36a3bcfe03994952d1b5e1b423e923e3e3b014 100644
--- a/drivers/gpu/drm/msm/Kconfig
+++ b/drivers/gpu/drm/msm/Kconfig
@@ -104,6 +104,7 @@ config DRM_MSM_DPU
 config DRM_MSM_DP
 	bool "Enable DisplayPort support in MSM DRM driver"
 	depends on DRM_MSM
+	select DRM_DISPLAY_HDMI_AUDIO_HELPER
 	select RATIONAL
 	default y
 	help
diff --git a/drivers/gpu/drm/msm/dp/dp_audio.c b/drivers/gpu/drm/msm/dp/dp_audio.c
index 70fdc9fe228a7149546accd8479a9e4397f3d5dd..f8bfb908f9b4bf93ad5480f0785e3aed23dde160 100644
--- a/drivers/gpu/drm/msm/dp/dp_audio.c
+++ b/drivers/gpu/drm/msm/dp/dp_audio.c
@@ -13,13 +13,13 @@
 
 #include "dp_catalog.h"
 #include "dp_audio.h"
+#include "dp_drm.h"
 #include "dp_panel.h"
 #include "dp_reg.h"
 #include "dp_display.h"
 #include "dp_utils.h"
 
 struct msm_dp_audio_private {
-	struct platform_device *audio_pdev;
 	struct platform_device *pdev;
 	struct drm_device *drm_dev;
 	struct msm_dp_catalog *catalog;
@@ -160,24 +160,11 @@ static void msm_dp_audio_enable(struct msm_dp_audio_private *audio, bool enable)
 	msm_dp_catalog_audio_enable(catalog, enable);
 }
 
-static struct msm_dp_audio_private *msm_dp_audio_get_data(struct platform_device *pdev)
+static struct msm_dp_audio_private *msm_dp_audio_get_data(struct msm_dp *msm_dp_display)
 {
 	struct msm_dp_audio *msm_dp_audio;
-	struct msm_dp *msm_dp_display;
-
-	if (!pdev) {
-		DRM_ERROR("invalid input\n");
-		return ERR_PTR(-ENODEV);
-	}
-
-	msm_dp_display = platform_get_drvdata(pdev);
-	if (!msm_dp_display) {
-		DRM_ERROR("invalid input\n");
-		return ERR_PTR(-ENODEV);
-	}
 
 	msm_dp_audio = msm_dp_display->msm_dp_audio;
-
 	if (!msm_dp_audio) {
 		DRM_ERROR("invalid msm_dp_audio data\n");
 		return ERR_PTR(-EINVAL);
@@ -186,68 +173,16 @@ static struct msm_dp_audio_private *msm_dp_audio_get_data(struct platform_device
 	return container_of(msm_dp_audio, struct msm_dp_audio_private, msm_dp_audio);
 }
 
-static int msm_dp_audio_hook_plugged_cb(struct device *dev, void *data,
-		hdmi_codec_plugged_cb fn,
-		struct device *codec_dev)
-{
-
-	struct platform_device *pdev;
-	struct msm_dp *msm_dp_display;
-
-	pdev = to_platform_device(dev);
-	if (!pdev) {
-		pr_err("invalid input\n");
-		return -ENODEV;
-	}
-
-	msm_dp_display = platform_get_drvdata(pdev);
-	if (!msm_dp_display) {
-		pr_err("invalid input\n");
-		return -ENODEV;
-	}
-
-	return msm_dp_display_set_plugged_cb(msm_dp_display, fn, codec_dev);
-}
-
-static int msm_dp_audio_get_eld(struct device *dev,
-	void *data, uint8_t *buf, size_t len)
-{
-	struct platform_device *pdev;
-	struct msm_dp *msm_dp_display;
-
-	pdev = to_platform_device(dev);
-
-	if (!pdev) {
-		DRM_ERROR("invalid input\n");
-		return -ENODEV;
-	}
-
-	msm_dp_display = platform_get_drvdata(pdev);
-	if (!msm_dp_display) {
-		DRM_ERROR("invalid input\n");
-		return -ENODEV;
-	}
-
-	mutex_lock(&msm_dp_display->connector->eld_mutex);
-	memcpy(buf, msm_dp_display->connector->eld,
-		min(sizeof(msm_dp_display->connector->eld), len));
-	mutex_unlock(&msm_dp_display->connector->eld_mutex);
-
-	return 0;
-}
-
-int msm_dp_audio_hw_params(struct device *dev,
-	void *data,
-	struct hdmi_codec_daifmt *daifmt,
-	struct hdmi_codec_params *params)
+int msm_dp_audio_prepare(struct drm_connector *connector,
+			 struct drm_bridge *bridge,
+			 struct hdmi_codec_daifmt *daifmt,
+			 struct hdmi_codec_params *params)
 {
 	int rc = 0;
 	struct msm_dp_audio_private *audio;
-	struct platform_device *pdev;
 	struct msm_dp *msm_dp_display;
 
-	pdev = to_platform_device(dev);
-	msm_dp_display = platform_get_drvdata(pdev);
+	msm_dp_display = to_dp_bridge(bridge)->msm_dp_display;
 
 	/*
 	 * there could be cases where sound card can be opened even
@@ -262,7 +197,7 @@ int msm_dp_audio_hw_params(struct device *dev,
 		goto end;
 	}
 
-	audio = msm_dp_audio_get_data(pdev);
+	audio = msm_dp_audio_get_data(msm_dp_display);
 	if (IS_ERR(audio)) {
 		rc = PTR_ERR(audio);
 		goto end;
@@ -281,15 +216,14 @@ int msm_dp_audio_hw_params(struct device *dev,
 	return rc;
 }
 
-static void msm_dp_audio_shutdown(struct device *dev, void *data)
+void msm_dp_audio_shutdown(struct drm_connector *connector,
+			   struct drm_bridge *bridge)
 {
 	struct msm_dp_audio_private *audio;
-	struct platform_device *pdev;
 	struct msm_dp *msm_dp_display;
 
-	pdev = to_platform_device(dev);
-	msm_dp_display = platform_get_drvdata(pdev);
-	audio = msm_dp_audio_get_data(pdev);
+	msm_dp_display = to_dp_bridge(bridge)->msm_dp_display;
+	audio = msm_dp_audio_get_data(msm_dp_display);
 	if (IS_ERR(audio)) {
 		DRM_ERROR("failed to get audio data\n");
 		return;
@@ -311,47 +245,6 @@ static void msm_dp_audio_shutdown(struct device *dev, void *data)
 	msm_dp_display_signal_audio_complete(msm_dp_display);
 }
 
-static const struct hdmi_codec_ops msm_dp_audio_codec_ops = {
-	.hw_params = msm_dp_audio_hw_params,
-	.audio_shutdown = msm_dp_audio_shutdown,
-	.get_eld = msm_dp_audio_get_eld,
-	.hook_plugged_cb = msm_dp_audio_hook_plugged_cb,
-};
-
-static struct hdmi_codec_pdata codec_data = {
-	.ops = &msm_dp_audio_codec_ops,
-	.max_i2s_channels = 8,
-	.i2s = 1,
-};
-
-void msm_dp_unregister_audio_driver(struct device *dev, struct msm_dp_audio *msm_dp_audio)
-{
-	struct msm_dp_audio_private *audio_priv;
-
-	audio_priv = container_of(msm_dp_audio, struct msm_dp_audio_private, msm_dp_audio);
-
-	if (audio_priv->audio_pdev) {
-		platform_device_unregister(audio_priv->audio_pdev);
-		audio_priv->audio_pdev = NULL;
-	}
-}
-
-int msm_dp_register_audio_driver(struct device *dev,
-		struct msm_dp_audio *msm_dp_audio)
-{
-	struct msm_dp_audio_private *audio_priv;
-
-	audio_priv = container_of(msm_dp_audio,
-			struct msm_dp_audio_private, msm_dp_audio);
-
-	audio_priv->audio_pdev = platform_device_register_data(dev,
-						HDMI_CODEC_DRV_NAME,
-						PLATFORM_DEVID_AUTO,
-						&codec_data,
-						sizeof(codec_data));
-	return PTR_ERR_OR_ZERO(audio_priv->audio_pdev);
-}
-
 struct msm_dp_audio *msm_dp_audio_get(struct platform_device *pdev,
 			struct msm_dp_catalog *catalog)
 {
diff --git a/drivers/gpu/drm/msm/dp/dp_audio.h b/drivers/gpu/drm/msm/dp/dp_audio.h
index beea34cbab77f31b33873297dc454a9cee446240..58fc14693e48bff2b57ef7278983e5f21ee80ac7 100644
--- a/drivers/gpu/drm/msm/dp/dp_audio.h
+++ b/drivers/gpu/drm/msm/dp/dp_audio.h
@@ -35,23 +35,6 @@ struct msm_dp_audio {
 struct msm_dp_audio *msm_dp_audio_get(struct platform_device *pdev,
 			struct msm_dp_catalog *catalog);
 
-/**
- * msm_dp_register_audio_driver()
- *
- * Registers DP device with hdmi_codec interface.
- *
- * @dev: DP device instance.
- * @msm_dp_audio: an instance of msm_dp_audio module.
- *
- *
- * Returns the error code in case of failure, otherwise
- * zero on success.
- */
-int msm_dp_register_audio_driver(struct device *dev,
-		struct msm_dp_audio *msm_dp_audio);
-
-void msm_dp_unregister_audio_driver(struct device *dev, struct msm_dp_audio *msm_dp_audio);
-
 /**
  * msm_dp_audio_put()
  *
@@ -61,10 +44,12 @@ void msm_dp_unregister_audio_driver(struct device *dev, struct msm_dp_audio *msm
  */
 void msm_dp_audio_put(struct msm_dp_audio *msm_dp_audio);
 
-int msm_dp_audio_hw_params(struct device *dev,
-	void *data,
-	struct hdmi_codec_daifmt *daifmt,
-	struct hdmi_codec_params *params);
+int msm_dp_audio_prepare(struct drm_connector *connector,
+			 struct drm_bridge *bridge,
+			 struct hdmi_codec_daifmt *daifmt,
+			 struct hdmi_codec_params *params);
+void msm_dp_audio_shutdown(struct drm_connector *connector,
+			   struct drm_bridge *bridge);
 
 #endif /* _DP_AUDIO_H_ */
 
diff --git a/drivers/gpu/drm/msm/dp/dp_display.c b/drivers/gpu/drm/msm/dp/dp_display.c
index 24dd37f1682bf5016bb0efbeb44489061deff060..aa19dfd267da97a6bb626a1d4e773f82aef571b5 100644
--- a/drivers/gpu/drm/msm/dp/dp_display.c
+++ b/drivers/gpu/drm/msm/dp/dp_display.c
@@ -12,6 +12,7 @@
 #include <linux/phy/phy.h>
 #include <linux/delay.h>
 #include <drm/display/drm_dp_aux_bus.h>
+#include <drm/display/drm_hdmi_audio_helper.h>
 #include <drm/drm_edid.h>
 
 #include "msm_drv.h"
@@ -287,13 +288,6 @@ static int msm_dp_display_bind(struct device *dev, struct device *master,
 		goto end;
 	}
 
-
-	rc = msm_dp_register_audio_driver(dev, dp->audio);
-	if (rc) {
-		DRM_ERROR("Audio registration Dp failed\n");
-		goto end;
-	}
-
 	rc = msm_dp_hpd_event_thread_start(dp);
 	if (rc) {
 		DRM_ERROR("Event thread create failed\n");
@@ -315,7 +309,6 @@ static void msm_dp_display_unbind(struct device *dev, struct device *master,
 
 	of_dp_aux_depopulate_bus(dp->aux);
 
-	msm_dp_unregister_audio_driver(dev, dp->audio);
 	msm_dp_aux_unregister(dp->aux);
 	dp->drm_dev = NULL;
 	dp->aux->drm_dev = NULL;
@@ -351,6 +344,8 @@ static int msm_dp_display_send_hpd_notification(struct msm_dp_display_private *d
 	/* reset video pattern flag on disconnect */
 	if (!hpd) {
 		dp->panel->video_test = false;
+
+		// FIXME: when reworking HPD use bridge->ops & DRM_BRIDGE_OP_DisplayPort
 		if (!dp->msm_dp_display.is_edp)
 			drm_dp_set_subconnector_property(dp->msm_dp_display.connector,
 							 connector_status_disconnected,
@@ -379,6 +374,7 @@ static int msm_dp_display_process_hpd_high(struct msm_dp_display_private *dp)
 
 	msm_dp_link_process_request(dp->link);
 
+	// FIXME: when reworking HPD use bridge->ops & DRM_BRIDGE_OP_DisplayPort
 	if (!dp->msm_dp_display.is_edp)
 		drm_dp_set_subconnector_property(connector,
 						 connector_status_connected,
@@ -611,9 +607,9 @@ static void msm_dp_display_handle_plugged_change(struct msm_dp *msm_dp_display,
 			struct msm_dp_display_private, msm_dp_display);
 
 	/* notify audio subsystem only if sink supports audio */
-	if (msm_dp_display->plugged_cb && msm_dp_display->codec_dev &&
-			dp->audio_supported)
-		msm_dp_display->plugged_cb(msm_dp_display->codec_dev, plugged);
+	if (dp->audio_supported)
+		drm_connector_hdmi_audio_plugged_notify(msm_dp_display->connector,
+							plugged);
 }
 
 static int msm_dp_hpd_unplug_handle(struct msm_dp_display_private *dp, u32 data)
@@ -892,19 +888,6 @@ static int msm_dp_display_disable(struct msm_dp_display_private *dp)
 	return 0;
 }
 
-int msm_dp_display_set_plugged_cb(struct msm_dp *msm_dp_display,
-		hdmi_codec_plugged_cb fn, struct device *codec_dev)
-{
-	bool plugged;
-
-	msm_dp_display->plugged_cb = fn;
-	msm_dp_display->codec_dev = codec_dev;
-	plugged = msm_dp_display->link_ready;
-	msm_dp_display_handle_plugged_change(msm_dp_display, plugged);
-
-	return 0;
-}
-
 /**
  * msm_dp_bridge_mode_valid - callback to determine if specified mode is valid
  * @bridge: Pointer to drm bridge structure
diff --git a/drivers/gpu/drm/msm/dp/dp_display.h b/drivers/gpu/drm/msm/dp/dp_display.h
index ecbc2d92f546a346ee53adcf1b060933e4f54317..cc6e2cab36e9c0b1527ff292e547cbb4d69fd95c 100644
--- a/drivers/gpu/drm/msm/dp/dp_display.h
+++ b/drivers/gpu/drm/msm/dp/dp_display.h
@@ -7,7 +7,6 @@
 #define _DP_DISPLAY_H_
 
 #include "dp_panel.h"
-#include <sound/hdmi-codec.h>
 #include "disp/msm_disp_snapshot.h"
 
 #define DP_MAX_PIXEL_CLK_KHZ	675000
@@ -15,7 +14,6 @@
 struct msm_dp {
 	struct drm_device *drm_dev;
 	struct platform_device *pdev;
-	struct device *codec_dev;
 	struct drm_connector *connector;
 	struct drm_bridge *next_bridge;
 	bool link_ready;
@@ -25,14 +23,10 @@ struct msm_dp {
 	bool is_edp;
 	bool internal_hpd;
 
-	hdmi_codec_plugged_cb plugged_cb;
-
 	struct msm_dp_audio *msm_dp_audio;
 	bool psr_supported;
 };
 
-int msm_dp_display_set_plugged_cb(struct msm_dp *msm_dp_display,
-		hdmi_codec_plugged_cb fn, struct device *codec_dev);
 int msm_dp_display_get_modes(struct msm_dp *msm_dp_display);
 bool msm_dp_display_check_video_test(struct msm_dp *msm_dp_display);
 int msm_dp_display_get_test_bpp(struct msm_dp *msm_dp_display);
diff --git a/drivers/gpu/drm/msm/dp/dp_drm.c b/drivers/gpu/drm/msm/dp/dp_drm.c
index d3e241ea6941615b8e274dd17426c2f8557f09b5..37dc0ba607f28e5ab65bb5d8ebe8ade0660be287 100644
--- a/drivers/gpu/drm/msm/dp/dp_drm.c
+++ b/drivers/gpu/drm/msm/dp/dp_drm.c
@@ -11,6 +11,7 @@
 
 #include "msm_drv.h"
 #include "msm_kms.h"
+#include "dp_audio.h"
 #include "dp_drm.h"
 
 /**
@@ -113,6 +114,9 @@ static const struct drm_bridge_funcs msm_dp_bridge_ops = {
 	.hpd_disable  = msm_dp_bridge_hpd_disable,
 	.hpd_notify   = msm_dp_bridge_hpd_notify,
 	.debugfs_init = msm_dp_bridge_debugfs_init,
+
+	.hdmi_audio_prepare = msm_dp_audio_prepare,
+	.hdmi_audio_shutdown = msm_dp_audio_shutdown,
 };
 
 static int msm_edp_bridge_atomic_check(struct drm_bridge *drm_bridge,
@@ -319,9 +323,13 @@ int msm_dp_bridge_init(struct msm_dp *msm_dp_display, struct drm_device *dev,
 	 */
 	if (!msm_dp_display->is_edp) {
 		bridge->ops =
+			DRM_BRIDGE_OP_DisplayPort |
 			DRM_BRIDGE_OP_DETECT |
 			DRM_BRIDGE_OP_HPD |
 			DRM_BRIDGE_OP_MODES;
+		bridge->hdmi_audio_dev = &msm_dp_display->pdev->dev;
+		bridge->hdmi_audio_max_i2s_playback_channels = 8;
+		bridge->hdmi_audio_dai_port = -1;
 	}
 
 	rc = devm_drm_bridge_add(dev->dev, bridge);
@@ -361,9 +369,6 @@ struct drm_connector *msm_dp_drm_connector_init(struct msm_dp *msm_dp_display,
 	if (IS_ERR(connector))
 		return connector;
 
-	if (!msm_dp_display->is_edp)
-		drm_connector_attach_dp_subconnector_property(connector);
-
 	drm_connector_attach_encoder(connector, encoder);
 
 	return connector;

-- 
2.39.5

Re: [PATCH v2 1/3] drm/display: bridge-connector: add DisplayPort bridges

[Laurent Pinchart]

Hi Dmitry,

Thank you for the patch.

On Sun, Feb 09, 2025 at 03:41:18PM +0200, Dmitry Baryshkov wrote:
> DRM HDMI Codec framework is useful not only for the HDMI bridges, but
> also for the DisplayPort bridges. Add new DRM_BRIDGE_OP_DisplayPort
> define in order to distinguish DP bridges. Create HDMI codec device
> automatically for DP bridges which have declared audio support.
> 
> Note, unlike HDMI devices, which already have a framework to handle HPD
> notifications in a standard way, DP drivers don't (yet?) have such a
> framework. As such it is necessary to manually call
> drm_connector_hdmi_audio_plugged_notify(). This requirement hopefully
> can be lifted later on, if/when DRM framework gets better DisplayPort
> ports support in the core layer.
> 
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> ---
>  drivers/gpu/drm/display/drm_bridge_connector.c | 66 ++++++++++++++++++++------
>  include/drm/drm_bridge.h                       | 14 +++++-
>  2 files changed, 65 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/gpu/drm/display/drm_bridge_connector.c b/drivers/gpu/drm/display/drm_bridge_connector.c
> index 30c736fc0067e31a97db242e5b16ea8a5b4cf359..5e031395b801f9a1371dcb4ac09f3da23e4615dd 100644
> --- a/drivers/gpu/drm/display/drm_bridge_connector.c
> +++ b/drivers/gpu/drm/display/drm_bridge_connector.c
> @@ -98,6 +98,13 @@ struct drm_bridge_connector {
>  	 * HDMI connector infrastructure, if any (see &DRM_BRIDGE_OP_HDMI).
>  	 */
>  	struct drm_bridge *bridge_hdmi;
> +	/**
> +	 * @bridge_dp:
> +	 *
> +	 * The bridge in the chain that implements necessary support for the
> +	 * DisplayPort connector infrastructure, if any (see &DRM_BRIDGE_OP_DisplayPort).
> +	 */
> +	struct drm_bridge *bridge_dp;
>  };
>  
>  #define to_drm_bridge_connector(x) \
> @@ -496,6 +503,25 @@ static const struct drm_connector_hdmi_audio_funcs drm_bridge_connector_hdmi_aud
>  	.mute_stream = drm_bridge_connector_audio_mute_stream,
>  };
>  
> +static int drm_bridge_connector_hdmi_audio_init(struct drm_connector *connector,
> +						struct drm_bridge *bridge)
> +{
> +	if (!bridge->hdmi_audio_max_i2s_playback_channels &&
> +	    !bridge->hdmi_audio_spdif_playback)
> +		return 0;
> +
> +	if (!bridge->funcs->hdmi_audio_prepare ||
> +	    !bridge->funcs->hdmi_audio_shutdown)
> +		return -EINVAL;
> +
> +	return drm_connector_hdmi_audio_init(connector,
> +					     bridge->hdmi_audio_dev,
> +					     &drm_bridge_connector_hdmi_audio_funcs,
> +					     bridge->hdmi_audio_max_i2s_playback_channels,
> +					     bridge->hdmi_audio_spdif_playback,
> +					     bridge->hdmi_audio_dai_port);
> +}
> +
>  /* -----------------------------------------------------------------------------
>   * Bridge Connector Initialisation
>   */
> @@ -564,6 +590,8 @@ struct drm_connector *drm_bridge_connector_init(struct drm_device *drm,
>  		if (bridge->ops & DRM_BRIDGE_OP_HDMI) {
>  			if (bridge_connector->bridge_hdmi)
>  				return ERR_PTR(-EBUSY);
> +			if (bridge_connector->bridge_dp)
> +				return ERR_PTR(-EINVAL);
>  			if (!bridge->funcs->hdmi_write_infoframe ||
>  			    !bridge->funcs->hdmi_clear_infoframe)
>  				return ERR_PTR(-EINVAL);
> @@ -576,6 +604,16 @@ struct drm_connector *drm_bridge_connector_init(struct drm_device *drm,
>  				max_bpc = bridge->max_bpc;
>  		}
>  
> +		if (bridge->ops & DRM_BRIDGE_OP_DisplayPort) {
> +			if (bridge_connector->bridge_dp)
> +				return ERR_PTR(-EBUSY);
> +			if (bridge_connector->bridge_hdmi)
> +				return ERR_PTR(-EINVAL);
> +
> +			bridge_connector->bridge_dp = bridge;
> +
> +		}
> +
>  		if (!drm_bridge_get_next_bridge(bridge))
>  			connector_type = bridge->type;
>  
> @@ -612,21 +650,21 @@ struct drm_connector *drm_bridge_connector_init(struct drm_device *drm,
>  		if (ret)
>  			return ERR_PTR(ret);
>  
> -		if (bridge->hdmi_audio_max_i2s_playback_channels ||
> -		    bridge->hdmi_audio_spdif_playback) {
> -			if (!bridge->funcs->hdmi_audio_prepare ||
> -			    !bridge->funcs->hdmi_audio_shutdown)
> -				return ERR_PTR(-EINVAL);
> +		ret = drm_bridge_connector_hdmi_audio_init(connector, bridge);
> +		if (ret)
> +			return ERR_PTR(ret);
> +	} else if (bridge_connector->bridge_dp) {
> +		bridge = bridge_connector->bridge_dp;
>  
> -			ret = drm_connector_hdmi_audio_init(connector,
> -							    bridge->hdmi_audio_dev,
> -							    &drm_bridge_connector_hdmi_audio_funcs,
> -							    bridge->hdmi_audio_max_i2s_playback_channels,
> -							    bridge->hdmi_audio_spdif_playback,
> -							    bridge->hdmi_audio_dai_port);
> -			if (ret)
> -				return ERR_PTR(ret);
> -		}
> +		ret = drmm_connector_init(drm, connector,
> +					  &drm_bridge_connector_funcs,
> +					  connector_type, ddc);
> +		if (ret)
> +			return ERR_PTR(ret);
> +
> +		ret = drm_bridge_connector_hdmi_audio_init(connector, bridge);
> +		if (ret)
> +			return ERR_PTR(ret);
>  	} else {
>  		ret = drmm_connector_init(drm, connector,
>  					  &drm_bridge_connector_funcs,
> diff --git a/include/drm/drm_bridge.h b/include/drm/drm_bridge.h
> index 496dbbd2ad7edff7f091adfbe62de1e33ef0cf07..40f37444426b1b8ded25da9ba9e2963f18ad6267 100644
> --- a/include/drm/drm_bridge.h
> +++ b/include/drm/drm_bridge.h
> @@ -811,9 +811,21 @@ enum drm_bridge_ops {
>  	 *
>  	 * Note: currently there can be at most one bridge in a chain that sets
>  	 * this bit. This is to simplify corresponding glue code in connector
> -	 * drivers.
> +	 * drivers. Having both HDMI and DisplayPort bridges in the same bridge
> +	 * chain is also not allowed.
>  	 */
>  	DRM_BRIDGE_OP_HDMI = BIT(4),
> +	/**
> +	 * @DRM_BRIDGE_OP_DisplayPort: The bridge provides DisplayPort connector
> +	 * operations. Currently this is limited to the optional HDMI codec
> +	 * support.
> +	 *
> +	 * Note: currently there can be at most one bridge in a chain that sets
> +	 * this bit. This is to simplify corresponding glue code in connector
> +	 * drivers. Having both HDMI and DisplayPort bridges in the same bridge
> +	 * chain is also not allowed.
> +	 */
> +	DRM_BRIDGE_OP_DisplayPort = BIT(5),

The OP bits are not supposed to describe tbe type of bridge, but the
operations it implements. I see quite a bit of duplication between HDMI
and DisplayPort in this patch. Can we have a single bit named after the
feature that you want to support ? The bridge_hdmi and bridge_dp fields
should also be merged into a single one.

>  };
>  
>  /**
> 

-- 
Regards,

Laurent Pinchart

Re: [PATCH v2 1/3] drm/display: bridge-connector: add DisplayPort bridges

[Dmitry Baryshkov]

On Fri, Feb 14, 2025 at 01:57:45AM +0200, Laurent Pinchart wrote:
> Hi Dmitry,
> 
> Thank you for the patch.
> 
> On Sun, Feb 09, 2025 at 03:41:18PM +0200, Dmitry Baryshkov wrote:
> > DRM HDMI Codec framework is useful not only for the HDMI bridges, but
> > also for the DisplayPort bridges. Add new DRM_BRIDGE_OP_DisplayPort
> > define in order to distinguish DP bridges. Create HDMI codec device
> > automatically for DP bridges which have declared audio support.
> > 
> > Note, unlike HDMI devices, which already have a framework to handle HPD
> > notifications in a standard way, DP drivers don't (yet?) have such a
> > framework. As such it is necessary to manually call
> > drm_connector_hdmi_audio_plugged_notify(). This requirement hopefully
> > can be lifted later on, if/when DRM framework gets better DisplayPort
> > ports support in the core layer.
> > 
> > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> > ---
> >  drivers/gpu/drm/display/drm_bridge_connector.c | 66 ++++++++++++++++++++------
> >  include/drm/drm_bridge.h                       | 14 +++++-
> >  2 files changed, 65 insertions(+), 15 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/display/drm_bridge_connector.c b/drivers/gpu/drm/display/drm_bridge_connector.c
> > index 30c736fc0067e31a97db242e5b16ea8a5b4cf359..5e031395b801f9a1371dcb4ac09f3da23e4615dd 100644
> > --- a/drivers/gpu/drm/display/drm_bridge_connector.c
> > +++ b/drivers/gpu/drm/display/drm_bridge_connector.c
> > @@ -98,6 +98,13 @@ struct drm_bridge_connector {
> >  	 * HDMI connector infrastructure, if any (see &DRM_BRIDGE_OP_HDMI).
> >  	 */
> >  	struct drm_bridge *bridge_hdmi;
> > +	/**
> > +	 * @bridge_dp:
> > +	 *
> > +	 * The bridge in the chain that implements necessary support for the
> > +	 * DisplayPort connector infrastructure, if any (see &DRM_BRIDGE_OP_DisplayPort).
> > +	 */
> > +	struct drm_bridge *bridge_dp;
> >  };
> >  
> >  #define to_drm_bridge_connector(x) \
> > @@ -496,6 +503,25 @@ static const struct drm_connector_hdmi_audio_funcs drm_bridge_connector_hdmi_aud
> >  	.mute_stream = drm_bridge_connector_audio_mute_stream,
> >  };
> >  
> > +static int drm_bridge_connector_hdmi_audio_init(struct drm_connector *connector,
> > +						struct drm_bridge *bridge)
> > +{
> > +	if (!bridge->hdmi_audio_max_i2s_playback_channels &&
> > +	    !bridge->hdmi_audio_spdif_playback)
> > +		return 0;
> > +
> > +	if (!bridge->funcs->hdmi_audio_prepare ||
> > +	    !bridge->funcs->hdmi_audio_shutdown)
> > +		return -EINVAL;
> > +
> > +	return drm_connector_hdmi_audio_init(connector,
> > +					     bridge->hdmi_audio_dev,
> > +					     &drm_bridge_connector_hdmi_audio_funcs,
> > +					     bridge->hdmi_audio_max_i2s_playback_channels,
> > +					     bridge->hdmi_audio_spdif_playback,
> > +					     bridge->hdmi_audio_dai_port);
> > +}
> > +
> >  /* -----------------------------------------------------------------------------
> >   * Bridge Connector Initialisation
> >   */
> > @@ -564,6 +590,8 @@ struct drm_connector *drm_bridge_connector_init(struct drm_device *drm,
> >  		if (bridge->ops & DRM_BRIDGE_OP_HDMI) {
> >  			if (bridge_connector->bridge_hdmi)
> >  				return ERR_PTR(-EBUSY);
> > +			if (bridge_connector->bridge_dp)
> > +				return ERR_PTR(-EINVAL);
> >  			if (!bridge->funcs->hdmi_write_infoframe ||
> >  			    !bridge->funcs->hdmi_clear_infoframe)
> >  				return ERR_PTR(-EINVAL);
> > @@ -576,6 +604,16 @@ struct drm_connector *drm_bridge_connector_init(struct drm_device *drm,
> >  				max_bpc = bridge->max_bpc;
> >  		}
> >  
> > +		if (bridge->ops & DRM_BRIDGE_OP_DisplayPort) {
> > +			if (bridge_connector->bridge_dp)
> > +				return ERR_PTR(-EBUSY);
> > +			if (bridge_connector->bridge_hdmi)
> > +				return ERR_PTR(-EINVAL);
> > +
> > +			bridge_connector->bridge_dp = bridge;
> > +
> > +		}
> > +
> >  		if (!drm_bridge_get_next_bridge(bridge))
> >  			connector_type = bridge->type;
> >  
> > @@ -612,21 +650,21 @@ struct drm_connector *drm_bridge_connector_init(struct drm_device *drm,
> >  		if (ret)
> >  			return ERR_PTR(ret);
> >  
> > -		if (bridge->hdmi_audio_max_i2s_playback_channels ||
> > -		    bridge->hdmi_audio_spdif_playback) {
> > -			if (!bridge->funcs->hdmi_audio_prepare ||
> > -			    !bridge->funcs->hdmi_audio_shutdown)
> > -				return ERR_PTR(-EINVAL);
> > +		ret = drm_bridge_connector_hdmi_audio_init(connector, bridge);
> > +		if (ret)
> > +			return ERR_PTR(ret);
> > +	} else if (bridge_connector->bridge_dp) {
> > +		bridge = bridge_connector->bridge_dp;
> >  
> > -			ret = drm_connector_hdmi_audio_init(connector,
> > -							    bridge->hdmi_audio_dev,
> > -							    &drm_bridge_connector_hdmi_audio_funcs,
> > -							    bridge->hdmi_audio_max_i2s_playback_channels,
> > -							    bridge->hdmi_audio_spdif_playback,
> > -							    bridge->hdmi_audio_dai_port);
> > -			if (ret)
> > -				return ERR_PTR(ret);
> > -		}
> > +		ret = drmm_connector_init(drm, connector,
> > +					  &drm_bridge_connector_funcs,
> > +					  connector_type, ddc);
> > +		if (ret)
> > +			return ERR_PTR(ret);
> > +
> > +		ret = drm_bridge_connector_hdmi_audio_init(connector, bridge);
> > +		if (ret)
> > +			return ERR_PTR(ret);
> >  	} else {
> >  		ret = drmm_connector_init(drm, connector,
> >  					  &drm_bridge_connector_funcs,
> > diff --git a/include/drm/drm_bridge.h b/include/drm/drm_bridge.h
> > index 496dbbd2ad7edff7f091adfbe62de1e33ef0cf07..40f37444426b1b8ded25da9ba9e2963f18ad6267 100644
> > --- a/include/drm/drm_bridge.h
> > +++ b/include/drm/drm_bridge.h
> > @@ -811,9 +811,21 @@ enum drm_bridge_ops {
> >  	 *
> >  	 * Note: currently there can be at most one bridge in a chain that sets
> >  	 * this bit. This is to simplify corresponding glue code in connector
> > -	 * drivers.
> > +	 * drivers. Having both HDMI and DisplayPort bridges in the same bridge
> > +	 * chain is also not allowed.
> >  	 */
> >  	DRM_BRIDGE_OP_HDMI = BIT(4),
> > +	/**
> > +	 * @DRM_BRIDGE_OP_DisplayPort: The bridge provides DisplayPort connector
> > +	 * operations. Currently this is limited to the optional HDMI codec
> > +	 * support.
> > +	 *
> > +	 * Note: currently there can be at most one bridge in a chain that sets
> > +	 * this bit. This is to simplify corresponding glue code in connector
> > +	 * drivers. Having both HDMI and DisplayPort bridges in the same bridge
> > +	 * chain is also not allowed.
> > +	 */
> > +	DRM_BRIDGE_OP_DisplayPort = BIT(5),
> 
> The OP bits are not supposed to describe tbe type of bridge, but the
> operations it implements. I see quite a bit of duplication between HDMI
> and DisplayPort in this patch. Can we have a single bit named after the
> feature that you want to support ? The bridge_hdmi and bridge_dp fields
> should also be merged into a single one.

In this case these ops actually describe the set of ops implemented by
the bridge. DRM_BRIDGE_OP_HDMI implements hdmi_tmds_char_rate_valid(),
hdmi_write_infoframe(), hdmi_clear_infoframe() and hdmi_audio_*()
callbacks. It is impossible to just use HDMI helpers for any
DRM_MODE_CONNECTOR_HDMIA bridge as they lack required callbacks.
At the same time it is perfectly legic to have a
DRM_MODE_CONNECTOR_HDMIA bridge which doesn't set DRM_BRIDGE_OP_HDMI:
this bridge chain will not use HDMI Connector / HDMI State helpers, but
it should be fine otherwise.

DRM_BRIDGE_OP_DisplayPort bridges currently implement hdmi_audio_*(),
but I have long-term plans for that set of ops.

It is not quite possible to merge bridge_hdmi and bridge_dp fields:
for bridges, which implement DRM_BRIDGE_OP_HDMI, drm_bridge_connector
call various drm_atomic_helper_connector_hdmi_*() functions. For
DisplayPort there is no corresponding functionality (yet).

-- 
With best wishes
Dmitry

Re: [PATCH v2 1/3] drm/display: bridge-connector: add DisplayPort bridges

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 7796 bytes --]

On Fri, Feb 14, 2025 at 02:32:56AM +0200, Dmitry Baryshkov wrote:
> On Fri, Feb 14, 2025 at 01:57:45AM +0200, Laurent Pinchart wrote:
> > Hi Dmitry,
> > 
> > Thank you for the patch.
> > 
> > On Sun, Feb 09, 2025 at 03:41:18PM +0200, Dmitry Baryshkov wrote:
> > > DRM HDMI Codec framework is useful not only for the HDMI bridges, but
> > > also for the DisplayPort bridges. Add new DRM_BRIDGE_OP_DisplayPort
> > > define in order to distinguish DP bridges. Create HDMI codec device
> > > automatically for DP bridges which have declared audio support.
> > > 
> > > Note, unlike HDMI devices, which already have a framework to handle HPD
> > > notifications in a standard way, DP drivers don't (yet?) have such a
> > > framework. As such it is necessary to manually call
> > > drm_connector_hdmi_audio_plugged_notify(). This requirement hopefully
> > > can be lifted later on, if/when DRM framework gets better DisplayPort
> > > ports support in the core layer.
> > > 
> > > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> > > ---
> > >  drivers/gpu/drm/display/drm_bridge_connector.c | 66 ++++++++++++++++++++------
> > >  include/drm/drm_bridge.h                       | 14 +++++-
> > >  2 files changed, 65 insertions(+), 15 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/display/drm_bridge_connector.c b/drivers/gpu/drm/display/drm_bridge_connector.c
> > > index 30c736fc0067e31a97db242e5b16ea8a5b4cf359..5e031395b801f9a1371dcb4ac09f3da23e4615dd 100644
> > > --- a/drivers/gpu/drm/display/drm_bridge_connector.c
> > > +++ b/drivers/gpu/drm/display/drm_bridge_connector.c
> > > @@ -98,6 +98,13 @@ struct drm_bridge_connector {
> > >  	 * HDMI connector infrastructure, if any (see &DRM_BRIDGE_OP_HDMI).
> > >  	 */
> > >  	struct drm_bridge *bridge_hdmi;
> > > +	/**
> > > +	 * @bridge_dp:
> > > +	 *
> > > +	 * The bridge in the chain that implements necessary support for the
> > > +	 * DisplayPort connector infrastructure, if any (see &DRM_BRIDGE_OP_DisplayPort).
> > > +	 */
> > > +	struct drm_bridge *bridge_dp;
> > >  };
> > >  
> > >  #define to_drm_bridge_connector(x) \
> > > @@ -496,6 +503,25 @@ static const struct drm_connector_hdmi_audio_funcs drm_bridge_connector_hdmi_aud
> > >  	.mute_stream = drm_bridge_connector_audio_mute_stream,
> > >  };
> > >  
> > > +static int drm_bridge_connector_hdmi_audio_init(struct drm_connector *connector,
> > > +						struct drm_bridge *bridge)
> > > +{
> > > +	if (!bridge->hdmi_audio_max_i2s_playback_channels &&
> > > +	    !bridge->hdmi_audio_spdif_playback)
> > > +		return 0;
> > > +
> > > +	if (!bridge->funcs->hdmi_audio_prepare ||
> > > +	    !bridge->funcs->hdmi_audio_shutdown)
> > > +		return -EINVAL;
> > > +
> > > +	return drm_connector_hdmi_audio_init(connector,
> > > +					     bridge->hdmi_audio_dev,
> > > +					     &drm_bridge_connector_hdmi_audio_funcs,
> > > +					     bridge->hdmi_audio_max_i2s_playback_channels,
> > > +					     bridge->hdmi_audio_spdif_playback,
> > > +					     bridge->hdmi_audio_dai_port);
> > > +}
> > > +
> > >  /* -----------------------------------------------------------------------------
> > >   * Bridge Connector Initialisation
> > >   */
> > > @@ -564,6 +590,8 @@ struct drm_connector *drm_bridge_connector_init(struct drm_device *drm,
> > >  		if (bridge->ops & DRM_BRIDGE_OP_HDMI) {
> > >  			if (bridge_connector->bridge_hdmi)
> > >  				return ERR_PTR(-EBUSY);
> > > +			if (bridge_connector->bridge_dp)
> > > +				return ERR_PTR(-EINVAL);
> > >  			if (!bridge->funcs->hdmi_write_infoframe ||
> > >  			    !bridge->funcs->hdmi_clear_infoframe)
> > >  				return ERR_PTR(-EINVAL);
> > > @@ -576,6 +604,16 @@ struct drm_connector *drm_bridge_connector_init(struct drm_device *drm,
> > >  				max_bpc = bridge->max_bpc;
> > >  		}
> > >  
> > > +		if (bridge->ops & DRM_BRIDGE_OP_DisplayPort) {
> > > +			if (bridge_connector->bridge_dp)
> > > +				return ERR_PTR(-EBUSY);
> > > +			if (bridge_connector->bridge_hdmi)
> > > +				return ERR_PTR(-EINVAL);
> > > +
> > > +			bridge_connector->bridge_dp = bridge;
> > > +
> > > +		}
> > > +
> > >  		if (!drm_bridge_get_next_bridge(bridge))
> > >  			connector_type = bridge->type;
> > >  
> > > @@ -612,21 +650,21 @@ struct drm_connector *drm_bridge_connector_init(struct drm_device *drm,
> > >  		if (ret)
> > >  			return ERR_PTR(ret);
> > >  
> > > -		if (bridge->hdmi_audio_max_i2s_playback_channels ||
> > > -		    bridge->hdmi_audio_spdif_playback) {
> > > -			if (!bridge->funcs->hdmi_audio_prepare ||
> > > -			    !bridge->funcs->hdmi_audio_shutdown)
> > > -				return ERR_PTR(-EINVAL);
> > > +		ret = drm_bridge_connector_hdmi_audio_init(connector, bridge);
> > > +		if (ret)
> > > +			return ERR_PTR(ret);
> > > +	} else if (bridge_connector->bridge_dp) {
> > > +		bridge = bridge_connector->bridge_dp;
> > >  
> > > -			ret = drm_connector_hdmi_audio_init(connector,
> > > -							    bridge->hdmi_audio_dev,
> > > -							    &drm_bridge_connector_hdmi_audio_funcs,
> > > -							    bridge->hdmi_audio_max_i2s_playback_channels,
> > > -							    bridge->hdmi_audio_spdif_playback,
> > > -							    bridge->hdmi_audio_dai_port);
> > > -			if (ret)
> > > -				return ERR_PTR(ret);
> > > -		}
> > > +		ret = drmm_connector_init(drm, connector,
> > > +					  &drm_bridge_connector_funcs,
> > > +					  connector_type, ddc);
> > > +		if (ret)
> > > +			return ERR_PTR(ret);
> > > +
> > > +		ret = drm_bridge_connector_hdmi_audio_init(connector, bridge);
> > > +		if (ret)
> > > +			return ERR_PTR(ret);
> > >  	} else {
> > >  		ret = drmm_connector_init(drm, connector,
> > >  					  &drm_bridge_connector_funcs,
> > > diff --git a/include/drm/drm_bridge.h b/include/drm/drm_bridge.h
> > > index 496dbbd2ad7edff7f091adfbe62de1e33ef0cf07..40f37444426b1b8ded25da9ba9e2963f18ad6267 100644
> > > --- a/include/drm/drm_bridge.h
> > > +++ b/include/drm/drm_bridge.h
> > > @@ -811,9 +811,21 @@ enum drm_bridge_ops {
> > >  	 *
> > >  	 * Note: currently there can be at most one bridge in a chain that sets
> > >  	 * this bit. This is to simplify corresponding glue code in connector
> > > -	 * drivers.
> > > +	 * drivers. Having both HDMI and DisplayPort bridges in the same bridge
> > > +	 * chain is also not allowed.
> > >  	 */
> > >  	DRM_BRIDGE_OP_HDMI = BIT(4),
> > > +	/**
> > > +	 * @DRM_BRIDGE_OP_DisplayPort: The bridge provides DisplayPort connector
> > > +	 * operations. Currently this is limited to the optional HDMI codec
> > > +	 * support.
> > > +	 *
> > > +	 * Note: currently there can be at most one bridge in a chain that sets
> > > +	 * this bit. This is to simplify corresponding glue code in connector
> > > +	 * drivers. Having both HDMI and DisplayPort bridges in the same bridge
> > > +	 * chain is also not allowed.
> > > +	 */
> > > +	DRM_BRIDGE_OP_DisplayPort = BIT(5),
> > 
> > The OP bits are not supposed to describe tbe type of bridge, but the
> > operations it implements. I see quite a bit of duplication between HDMI
> > and DisplayPort in this patch. Can we have a single bit named after the
> > feature that you want to support ? The bridge_hdmi and bridge_dp fields
> > should also be merged into a single one.
> 
> In this case these ops actually describe the set of ops implemented by
> the bridge. DRM_BRIDGE_OP_HDMI implements hdmi_tmds_char_rate_valid(),
> hdmi_write_infoframe(), hdmi_clear_infoframe()

Those are required for HDMI bridges (using the common infrastructure).

> and hdmi_audio_*() callbacks.

But those are certainly not required.

If the OP enum is meant to list what a bridge implements, then we need
to have a different enum for video, audio, and CEC.

Maxime

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 273 bytes --]