[PATCH v5 00/11] drm/display: generic HDMI CEC helpers

[PATCH v5 00/11] drm/display: generic HDMI CEC helpers

[Dmitry Baryshkov]

Currently it is next to impossible to implement CEC handling for the
setup using drm_bridges and drm_bridge_connector: bridges don't have a
hold of the connector at the proper time to be able to route CEC events.

At the same time it not very easy and obvious to get the CEC physical
address handling correctly. Drivers handle it at various places, ending
up with the slight differences in behaviour.

Follow up the HDMI Connector and HDMI Audio series and implement generic
HDMI CEC set of helpers that link into the HDMI Connector and
drm_bridge_connector frameworks and provide a way to implement CEC
handling for HDMI bridges in an easy yet standard way.

Notes:
- the patchset was only compile-tested
- being an RFC some of the API functions and structures are left
  undocumented
- although the patchset provides drm_bridge / drm_bridge_connector API
  for working with CEC, there is no actual bridge that uses the API
  (yet)

- I'm pretty unhappy with the drm_bridge integration code, we end up
  duplicating wrappers for CEC functions instead of reusing the
  drm_connector wrapping functions. An easy way would be to map
  drm_bridge to the corresponding drm_connector, but that would
  either require a state or storing drm_connector inside a drm_bridge.
  Current code stores cec_adapter instead.

Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
---
Changes in v5:
- Fixed the check in adv7511_bridge_hdmi_tmds_char_rate_valid().
- Major rework of HDMI CEC to follow 'helpers' design. Implemented
  generic data structures that can be used for both CEC notifiers and
  CEC adapters (Maxime).
- Link to v4: https://lore.kernel.org/r/20250202-drm-hdmi-connector-cec-v4-0-a71620a54d50@linaro.org

Changes in v4:
- Rebased on top of drm-misc-next + commit 78a5acf5433d ("drm/display:
  hdmi: Do not read EDID on disconnected connectors")
- Moved 'select DRM_DISPLAY_HDMI_CEC_HELPER' under the
  DRM_I2C_ADV7511_CEC symbol
- Fixed documentation for @hdmi_audio_i2s_formats to describe default
  behaviour.
- Split drm_connector_cleanup() patch from the patch adding CEC-related
  data structures (Maxime).
- Documented that CEC mutex protects all data fields in struct
  drm_connector_cec (Maxime).
- Improved drm_connector_cec_funcs.unregister() documentation.
- Documented struct drm_connector_hdmi_cec_adapter_ops fields. Added a
  note to the commit message on the difference between it and struct
  drm_connector_cec_funcs (Maxime).
- Fixed Kconfig dependencies.
- Link to v3: https://lore.kernel.org/r/20250126-drm-hdmi-connector-cec-v3-0-5b5b2d4956da@linaro.org

Changes in v3:
- Moved default available_las setting to
  drm_connector_hdmi_cec_register(), allowing drivers to pass 0 instead
  of CEC_MAX_LOG_ADDRS.
- Reworked drm_bridge interface to store opaque pointer and interpret it
  as drm_connector in CEC helpers code.
- Fixed EINVAL checks in drm_connector_hdmi_cec_register().
- Added the adv7511 patch, demonstrating CEC helpers for the DRM
  bridges.
- Link to v2: https://lore.kernel.org/r/20250110-drm-hdmi-connector-cec-v2-0-9067c8f34663@linaro.org

Changes in v2:
- Refactored CEC funcs, adding more wrappers to provide more consistent
  interface (Maxime)
- Removed export of drm_connector_cec_unregister() (Maxime)
- Restored and rephrased comment in vc4_hdmi (Maxime)
- Squashed vc4 patches
- Link to v1: https://lore.kernel.org/r/20241225-drm-hdmi-connector-cec-v1-0-b80380c67221@linaro.org

---
Dmitry Baryshkov (11):
      drm/bridge: move private data to the end of the struct
      drm/bridge: allow limiting I2S formats
      drm/connector: add CEC-related fields
      drm/connector: unregister CEC data
      drm/display: move CEC_CORE selection to DRM_DISPLAY_HELPER
      drm/display: add CEC helpers code
      drm/display: hdmi-state-helper: handle CEC physical address
      drm/vc4: hdmi: switch to generic CEC helpers
      drm/display: bridge-connector: hook in CEC notifier support
      drm/display: bridge-connector: handle CEC adapters
      drm/bridge: adv7511: switch to the HDMI connector helpers

 drivers/gpu/drm/bridge/adv7511/Kconfig             |   5 +-
 drivers/gpu/drm/bridge/adv7511/adv7511.h           |  52 ++--
 drivers/gpu/drm/bridge/adv7511/adv7511_audio.c     |  77 +----
 drivers/gpu/drm/bridge/adv7511/adv7511_cec.c       |  57 ++--
 drivers/gpu/drm/bridge/adv7511/adv7511_drv.c       | 345 +++++++++------------
 drivers/gpu/drm/bridge/adv7511/adv7533.c           |   4 -
 drivers/gpu/drm/display/Kconfig                    |  13 +-
 drivers/gpu/drm/display/Makefile                   |   4 +
 drivers/gpu/drm/display/drm_bridge_connector.c     | 108 +++++++
 drivers/gpu/drm/display/drm_hdmi_audio_helper.c    |   3 +
 drivers/gpu/drm/display/drm_hdmi_cec_helper.c      | 175 +++++++++++
 .../gpu/drm/display/drm_hdmi_cec_notifier_helper.c |  60 ++++
 drivers/gpu/drm/display/drm_hdmi_state_helper.c    |   7 +-
 drivers/gpu/drm/drm_connector.c                    |  51 +++
 drivers/gpu/drm/vc4/Kconfig                        |   1 +
 drivers/gpu/drm/vc4/vc4_hdmi.c                     | 140 ++++-----
 drivers/gpu/drm/vc4/vc4_hdmi.h                     |   1 -
 include/drm/display/drm_hdmi_audio_helper.h        |   1 +
 include/drm/display/drm_hdmi_cec_helper.h          |  74 +++++
 include/drm/drm_bridge.h                           |  76 ++++-
 include/drm/drm_connector.h                        |  56 ++++
 21 files changed, 879 insertions(+), 431 deletions(-)
---
base-commit: 231adeda9f674ece664b09b75b68a4c023180fb4
change-id: 20241223-drm-hdmi-connector-cec-34902ce847b7

Best regards,
-- 
Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>

[PATCH v5 01/11] drm/bridge: move private data to the end of the struct

[Dmitry Baryshkov]

WHen adding HDMI fields I didn't notice the private: declaration for HPD
fields. Move private fields to the end of the struct drm_bride to have
clear distinction between private and public fields.

Reviewed-by: Maxime Ripard <mripard@kernel.org>
Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
---
 include/drm/drm_bridge.h | 31 ++++++++++++++++---------------
 1 file changed, 16 insertions(+), 15 deletions(-)

diff --git a/include/drm/drm_bridge.h b/include/drm/drm_bridge.h
index 4e418a29a9ff9d014d6ac0910a5d9bcf7118195e..286f6fb3fe2b80f237db85dc8459430dc82337e2 100644
--- a/include/drm/drm_bridge.h
+++ b/include/drm/drm_bridge.h
@@ -977,21 +977,6 @@ struct drm_bridge {
 	 * @ddc: Associated I2C adapter for DDC access, if any.
 	 */
 	struct i2c_adapter *ddc;
-	/** private: */
-	/**
-	 * @hpd_mutex: Protects the @hpd_cb and @hpd_data fields.
-	 */
-	struct mutex hpd_mutex;
-	/**
-	 * @hpd_cb: Hot plug detection callback, registered with
-	 * drm_bridge_hpd_enable().
-	 */
-	void (*hpd_cb)(void *data, enum drm_connector_status status);
-	/**
-	 * @hpd_data: Private data passed to the Hot plug detection callback
-	 * @hpd_cb.
-	 */
-	void *hpd_data;
 
 	/**
 	 * @vendor: Vendor of the product to be used for the SPD InfoFrame
@@ -1043,6 +1028,22 @@ struct drm_bridge {
 	 * not used.
 	 */
 	int hdmi_audio_dai_port;
+
+	/** private: */
+	/**
+	 * @hpd_mutex: Protects the @hpd_cb and @hpd_data fields.
+	 */
+	struct mutex hpd_mutex;
+	/**
+	 * @hpd_cb: Hot plug detection callback, registered with
+	 * drm_bridge_hpd_enable().
+	 */
+	void (*hpd_cb)(void *data, enum drm_connector_status status);
+	/**
+	 * @hpd_data: Private data passed to the Hot plug detection callback
+	 * @hpd_cb.
+	 */
+	void *hpd_data;
 };
 
 static inline struct drm_bridge *

-- 
2.39.5

[PATCH v5 02/11] drm/bridge: allow limiting I2S formats

[Dmitry Baryshkov]

By default HDMI codec registers all formats supported on the I2S bus.
Allow bridges (and connectors) to limit the list of the PCM formats
supported by the HDMI codec.

Reviewed-by: Maxime Ripard <mripard@kernel.org>
Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
---
 drivers/gpu/drm/display/drm_bridge_connector.c  | 1 +
 drivers/gpu/drm/display/drm_hdmi_audio_helper.c | 3 +++
 drivers/gpu/drm/vc4/vc4_hdmi.c                  | 2 +-
 include/drm/display/drm_hdmi_audio_helper.h     | 1 +
 include/drm/drm_bridge.h                        | 8 ++++++++
 5 files changed, 14 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/display/drm_bridge_connector.c b/drivers/gpu/drm/display/drm_bridge_connector.c
index 7d2e499ea5dec2f710c1c67323bf9e6b177d3c9e..381a0f9d4259bf9f72d3a292b7dcc82e45c61bae 100644
--- a/drivers/gpu/drm/display/drm_bridge_connector.c
+++ b/drivers/gpu/drm/display/drm_bridge_connector.c
@@ -717,6 +717,7 @@ struct drm_connector *drm_bridge_connector_init(struct drm_device *drm,
 		ret = drm_connector_hdmi_audio_init(connector, dev,
 						    &drm_bridge_connector_hdmi_audio_funcs,
 						    bridge->hdmi_audio_max_i2s_playback_channels,
+						    bridge->hdmi_audio_i2s_formats,
 						    bridge->hdmi_audio_spdif_playback,
 						    bridge->hdmi_audio_dai_port);
 		if (ret)
diff --git a/drivers/gpu/drm/display/drm_hdmi_audio_helper.c b/drivers/gpu/drm/display/drm_hdmi_audio_helper.c
index 05afc9f0bdd6b6f00d74223a9d8875e6d16aea5f..21c93bdd8648cf70e691dbf0c92fae5823fd1828 100644
--- a/drivers/gpu/drm/display/drm_hdmi_audio_helper.c
+++ b/drivers/gpu/drm/display/drm_hdmi_audio_helper.c
@@ -142,6 +142,7 @@ static const struct hdmi_codec_ops drm_connector_hdmi_audio_ops = {
  * @hdmi_codec_dev: device to be used as a parent for the HDMI Codec
  * @funcs: callbacks for this HDMI Codec
  * @max_i2s_playback_channels: maximum number of playback I2S channels
+ * @i2s_formats: set of I2S formats (use 0 for a bus-specific set)
  * @spdif_playback: set if HDMI codec has S/PDIF playback port
  * @dai_port: sound DAI port, -1 if it is not enabled
  *
@@ -154,6 +155,7 @@ int drm_connector_hdmi_audio_init(struct drm_connector *connector,
 				  struct device *hdmi_codec_dev,
 				  const struct drm_connector_hdmi_audio_funcs *funcs,
 				  unsigned int max_i2s_playback_channels,
+				  u64 i2s_formats,
 				  bool spdif_playback,
 				  int dai_port)
 {
@@ -161,6 +163,7 @@ int drm_connector_hdmi_audio_init(struct drm_connector *connector,
 		.ops = &drm_connector_hdmi_audio_ops,
 		.max_i2s_channels = max_i2s_playback_channels,
 		.i2s = !!max_i2s_playback_channels,
+		.i2s_formats = i2s_formats,
 		.spdif = spdif_playback,
 		.no_i2s_capture = true,
 		.no_spdif_capture = true,
diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c
index a29a6ef266f9a5952af53030a9a2d313e2ecdfce..4797ed1c21f47992fe4d497d904ee31c824cd449 100644
--- a/drivers/gpu/drm/vc4/vc4_hdmi.c
+++ b/drivers/gpu/drm/vc4/vc4_hdmi.c
@@ -562,7 +562,7 @@ static int vc4_hdmi_connector_init(struct drm_device *dev,
 
 	ret = drm_connector_hdmi_audio_init(connector, dev->dev,
 					    &vc4_hdmi_audio_funcs,
-					    8, false, -1);
+					    8, 0, false, -1);
 	if (ret)
 		return ret;
 
diff --git a/include/drm/display/drm_hdmi_audio_helper.h b/include/drm/display/drm_hdmi_audio_helper.h
index c9a6faef4109f20ba79b610a9d5e8d5980efe2d1..44d910bdc72dd2fdbbe7ada65b67080d4a41e88b 100644
--- a/include/drm/display/drm_hdmi_audio_helper.h
+++ b/include/drm/display/drm_hdmi_audio_helper.h
@@ -14,6 +14,7 @@ int drm_connector_hdmi_audio_init(struct drm_connector *connector,
 				  struct device *hdmi_codec_dev,
 				  const struct drm_connector_hdmi_audio_funcs *funcs,
 				  unsigned int max_i2s_playback_channels,
+				  u64 i2s_formats,
 				  bool spdif_playback,
 				  int sound_dai_port);
 void drm_connector_hdmi_audio_plugged_notify(struct drm_connector *connector,
diff --git a/include/drm/drm_bridge.h b/include/drm/drm_bridge.h
index 286f6fb3fe2b80f237db85dc8459430dc82337e2..db0d374d863b0b1f774d395743f1e29bb84e8937 100644
--- a/include/drm/drm_bridge.h
+++ b/include/drm/drm_bridge.h
@@ -1016,6 +1016,14 @@ struct drm_bridge {
 	 */
 	int hdmi_audio_max_i2s_playback_channels;
 
+	/**
+	 * @hdmi_audio_i2s_formats: supported I2S formats, optional. The
+	 * default is to allow all formats supported by the corresponding I2S
+	 * bus driver. This is only used for bridges setting
+	 * @DRM_BRIDGE_OP_HDMI_AUDIO or @DRM_BRIDGE_OP_DP_AUDIO.
+	 */
+	u64 hdmi_audio_i2s_formats;
+
 	/**
 	 * @hdmi_audio_spdif_playback: set if this bridge has S/PDIF playback
 	 * port for @DRM_BRIDGE_OP_HDMI_AUDIO or @DRM_BRIDGE_OP_DP_AUDIO.

-- 
2.39.5

[PATCH v5 03/11] drm/connector: add CEC-related fields

[Dmitry Baryshkov]

As a preparation to adding HDMI CEC helper code, add CEC-related fields
to the struct drm_connector. The callbacks abstract CEC infrastructure
in order to support CEC adapters and CEC notifiers in a universal way.

Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
---
 drivers/gpu/drm/drm_connector.c | 42 +++++++++++++++++++++++++++++++
 include/drm/drm_connector.h     | 56 +++++++++++++++++++++++++++++++++++++++++
 2 files changed, 98 insertions(+)

diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
index 48b08c9611a7bc70e4d849ff33ecf1c9de3cf0ae..ba08fbd973829e49ea977251c4f0fb6d96ade631 100644
--- a/drivers/gpu/drm/drm_connector.c
+++ b/drivers/gpu/drm/drm_connector.c
@@ -279,6 +279,7 @@ static int drm_connector_init_only(struct drm_device *dev,
 	INIT_LIST_HEAD(&connector->probed_modes);
 	INIT_LIST_HEAD(&connector->modes);
 	mutex_init(&connector->mutex);
+	mutex_init(&connector->cec.mutex);
 	mutex_init(&connector->eld_mutex);
 	mutex_init(&connector->edid_override_mutex);
 	mutex_init(&connector->hdmi.infoframes.lock);
@@ -701,6 +702,47 @@ static void drm_mode_remove(struct drm_connector *connector,
 	drm_mode_destroy(connector->dev, mode);
 }
 
+/**
+ * drm_connector_cec_phys_addr_invalidate - invalidate CEC physical address
+ * @connector: connector undergoing CEC operation
+ *
+ * Invalidated CEC physical address set for this DRM connector.
+ */
+void drm_connector_cec_phys_addr_invalidate(struct drm_connector *connector)
+{
+	mutex_lock(&connector->cec.mutex);
+
+	if (connector->cec.funcs &&
+	    connector->cec.funcs->phys_addr_invalidate)
+		connector->cec.funcs->phys_addr_invalidate(connector);
+
+	mutex_unlock(&connector->cec.mutex);
+}
+EXPORT_SYMBOL(drm_connector_cec_phys_addr_invalidate);
+
+
+/**
+ * drm_connector_cec_phys_addr_set - propagate CEC physical address
+ * @connector: connector undergoing CEC operation
+ *
+ * Propagate CEC physical address from the display_info to this DRM connector.
+ */
+void drm_connector_cec_phys_addr_set(struct drm_connector *connector)
+{
+	u16 addr;
+
+	mutex_lock(&connector->cec.mutex);
+
+	addr = connector->display_info.source_physical_address;
+
+	if (connector->cec.funcs &&
+	    connector->cec.funcs->phys_addr_set)
+		connector->cec.funcs->phys_addr_set(connector, addr);
+
+	mutex_unlock(&connector->cec.mutex);
+}
+EXPORT_SYMBOL(drm_connector_cec_phys_addr_set);
+
 /**
  * drm_connector_cleanup - cleans up an initialised connector
  * @connector: connector to cleanup
diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
index f13d597370a30dc1b14c630ee00145256052ba56..5f47df9a586a3e0acc26204b86ee7242acad7403 100644
--- a/include/drm/drm_connector.h
+++ b/include/drm/drm_connector.h
@@ -1191,6 +1191,37 @@ struct drm_connector_hdmi_audio_funcs {
 			   bool enable, int direction);
 };
 
+void drm_connector_cec_phys_addr_invalidate(struct drm_connector *connector);
+void drm_connector_cec_phys_addr_set(struct drm_connector *connector);
+
+/**
+ * struct drm_connector_cec_funcs - drm_hdmi_connector control functions
+ */
+struct drm_connector_cec_funcs {
+	/**
+	 * @phys_addr_invalidate: mark CEC physical address as invalid
+	 *
+	 * The callback to mark CEC physical address as invalid, abstracting
+	 * the operation.
+	 */
+	void (*phys_addr_invalidate)(struct drm_connector *connector);
+
+	/**
+	 * @phys_addr_set: set CEC physical address
+	 *
+	 * The callback to set CEC physical address, abstracting the operation.
+	 */
+	void (*phys_addr_set)(struct drm_connector *connector, u16 addr);
+
+	/**
+	 * @unregister: unregister CEC adapter / notifier.
+	 *
+	 * The callback to unregister CEC adapter or notifier, abstracting the
+	 * operation.
+	 */
+	void (*unregister)(struct drm_connector *connector);
+};
+
 /**
  * struct drm_connector_hdmi_funcs - drm_hdmi_connector control functions
  */
@@ -1832,6 +1863,26 @@ struct drm_connector_hdmi {
 	} infoframes;
 };
 
+/**
+ * struct drm_connector_cec - DRM Connector CEC-related structure
+ */
+struct drm_connector_cec {
+	/**
+	 * @mutex: protects all fields in this structure.
+	 */
+	struct mutex mutex;
+
+	/**
+	 * @funcs: CEC Control Functions
+	 */
+	const struct drm_connector_cec_funcs *funcs;
+
+	/**
+	 * @data: CEC implementation-specific data
+	 */
+	void *data;
+};
+
 /**
  * struct drm_connector - central DRM connector control structure
  *
@@ -2253,6 +2304,11 @@ struct drm_connector {
 	 * @hdmi_audio: HDMI codec properties and non-DRM state.
 	 */
 	struct drm_connector_hdmi_audio hdmi_audio;
+
+	/**
+	 * @cec: CEC-related data.
+	 */
+	struct drm_connector_cec cec;
 };
 
 #define obj_to_connector(x) container_of(x, struct drm_connector, base)

-- 
2.39.5

[PATCH v5 04/11] drm/connector: unregister CEC data

[Dmitry Baryshkov]

In order to make sure that CEC adapters or notifiers are unregistered
and CEC-related data is properly destroyed make drm_connector_cleanup()
call CEC's unregister() callback.

Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
---
 drivers/gpu/drm/drm_connector.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
index ba08fbd973829e49ea977251c4f0fb6d96ade631..ae9c02ef9ab102db03c2824683ece37cfbcd3300 100644
--- a/drivers/gpu/drm/drm_connector.c
+++ b/drivers/gpu/drm/drm_connector.c
@@ -743,6 +743,13 @@ void drm_connector_cec_phys_addr_set(struct drm_connector *connector)
 }
 EXPORT_SYMBOL(drm_connector_cec_phys_addr_set);
 
+static void drm_connector_cec_unregister(struct drm_connector *connector)
+{
+	if (connector->cec.funcs &&
+	    connector->cec.funcs->unregister)
+		connector->cec.funcs->unregister(connector);
+}
+
 /**
  * drm_connector_cleanup - cleans up an initialised connector
  * @connector: connector to cleanup
@@ -763,6 +770,8 @@ void drm_connector_cleanup(struct drm_connector *connector)
 
 	platform_device_unregister(connector->hdmi_audio.codec_pdev);
 
+	drm_connector_cec_unregister(connector);
+
 	if (connector->privacy_screen) {
 		drm_privacy_screen_put(connector->privacy_screen);
 		connector->privacy_screen = NULL;

-- 
2.39.5

[PATCH v5 05/11] drm/display: move CEC_CORE selection to DRM_DISPLAY_HELPER

[Dmitry Baryshkov]

THe Kconfig symbol DRM_DISPLAY_DP_AUX_CEC is a boolean which simply
toggles whether DP_AUX_CEC support should be built into the
drm_display_helper (which can be eithera module or built-in into the
kernel). If DRM_DISPLAY_DP_AUX_CEC is selected, then CEC_CORE is
selected to be built-in into the kernel even if DRM_DISPLAY_HELPER is
selected to be built as a module. Move CEC_CORE selection to the latter
symbol in order to allow it to be built as a module.

Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
---
 drivers/gpu/drm/display/Kconfig | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/display/Kconfig b/drivers/gpu/drm/display/Kconfig
index 8d22b7627d41f7bc015decf24ae02a05bc00f055..3666e791d6d6eba58f095d7fb691de1fd0b95ed3 100644
--- a/drivers/gpu/drm/display/Kconfig
+++ b/drivers/gpu/drm/display/Kconfig
@@ -8,6 +8,7 @@ config DRM_DISPLAY_DP_AUX_BUS
 config DRM_DISPLAY_HELPER
 	tristate
 	depends on DRM
+	select CEC_CORE if DRM_DISPLAY_DP_AUX_CEC
 	help
 	  DRM helpers for display adapters.
 
@@ -23,7 +24,6 @@ config DRM_BRIDGE_CONNECTOR
 config DRM_DISPLAY_DP_AUX_CEC
 	bool "Enable DisplayPort CEC-Tunneling-over-AUX HDMI support"
 	select DRM_DISPLAY_DP_HELPER
-	select CEC_CORE
 	help
 	  Choose this option if you want to enable HDMI CEC support for
 	  DisplayPort/USB-C to HDMI adapters.

-- 
2.39.5

[PATCH v5 06/11] drm/display: add CEC helpers code

[Dmitry Baryshkov]

Add generic CEC helpers to be used by HDMI drivers. Both notifier and
and adapter are supported for registration. Once registered, the driver
can call common set of functions to update physical address, to
invalidate it or to unregister CEC data. Unlike drm_connector_cec_funcs
(which provides interface common to all implementations, including, but
not limited to the CEC adapter, CEC notifier, CEC pin-based adapter,
etc) the struct drm_connector_hdmi_cec_adapter_ops provides callbacks
specific to the CEC adapter implementations.

Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
---
 drivers/gpu/drm/display/Kconfig                    |  12 +-
 drivers/gpu/drm/display/Makefile                   |   4 +
 drivers/gpu/drm/display/drm_hdmi_cec_helper.c      | 175 +++++++++++++++++++++
 .../gpu/drm/display/drm_hdmi_cec_notifier_helper.c |  60 +++++++
 include/drm/display/drm_hdmi_cec_helper.h          |  74 +++++++++
 5 files changed, 324 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/display/Kconfig b/drivers/gpu/drm/display/Kconfig
index 3666e791d6d6eba58f095d7fb691de1fd0b95ed3..6376ea01ec3093a72de25064e31223d2c9868ed7 100644
--- a/drivers/gpu/drm/display/Kconfig
+++ b/drivers/gpu/drm/display/Kconfig
@@ -8,7 +8,7 @@ config DRM_DISPLAY_DP_AUX_BUS
 config DRM_DISPLAY_HELPER
 	tristate
 	depends on DRM
-	select CEC_CORE if DRM_DISPLAY_DP_AUX_CEC
+	select CEC_CORE if DRM_DISPLAY_DP_AUX_CEC || DRM_DISPLAY_HDMI_CEC_HELPER || CEC_NOTIFIER
 	help
 	  DRM helpers for display adapters.
 
@@ -82,6 +82,16 @@ config DRM_DISPLAY_HDMI_AUDIO_HELPER
 	  DRM display helpers for HDMI Audio functionality (generic HDMI Codec
 	  implementation).
 
+config DRM_DISPLAY_HDMI_CEC_HELPER
+	bool
+	help
+	  DRM display helpers for HDMI CEC implementation.
+
+config DRM_DISPLAY_HDMI_CEC_NOTIFIER_HELPER
+	def_bool CEC_NOTIFIER
+	help
+	  DRM display helpers for HDMI CEC notifiers implementation.
+
 config DRM_DISPLAY_HDMI_HELPER
 	bool
 	help
diff --git a/drivers/gpu/drm/display/Makefile b/drivers/gpu/drm/display/Makefile
index b17879b957d5401721396e247fa346387cf6c48a..0ff4a1ad0222078bf495175915007f1b1f903296 100644
--- a/drivers/gpu/drm/display/Makefile
+++ b/drivers/gpu/drm/display/Makefile
@@ -16,6 +16,10 @@ drm_display_helper-$(CONFIG_DRM_DISPLAY_DSC_HELPER) += \
 drm_display_helper-$(CONFIG_DRM_DISPLAY_HDCP_HELPER) += drm_hdcp_helper.o
 drm_display_helper-$(CONFIG_DRM_DISPLAY_HDMI_AUDIO_HELPER) += \
 	drm_hdmi_audio_helper.o
+drm_display_helper-$(CONFIG_DRM_DISPLAY_HDMI_CEC_HELPER) += \
+	drm_hdmi_cec_helper.o
+drm_display_helper-$(CONFIG_DRM_DISPLAY_HDMI_CEC_NOTIFIER_HELPER) += \
+	drm_hdmi_cec_notifier_helper.o
 drm_display_helper-$(CONFIG_DRM_DISPLAY_HDMI_HELPER) += \
 	drm_hdmi_helper.o \
 	drm_scdc_helper.o
diff --git a/drivers/gpu/drm/display/drm_hdmi_cec_helper.c b/drivers/gpu/drm/display/drm_hdmi_cec_helper.c
new file mode 100644
index 0000000000000000000000000000000000000000..85076ea92d95ad95fb7b2d6c9db69c290e09a8cf
--- /dev/null
+++ b/drivers/gpu/drm/display/drm_hdmi_cec_helper.c
@@ -0,0 +1,175 @@
+// SPDX-License-Identifier: MIT
+/*
+ * Copyright (c) 2024 Linaro Ltd
+ */
+
+#include <drm/drm_bridge.h>
+#include <drm/drm_connector.h>
+#include <drm/display/drm_hdmi_cec_helper.h>
+
+#include <linux/mutex.h>
+
+#include <media/cec.h>
+
+struct drm_connector_hdmi_cec_data {
+	struct cec_adapter *adapter;
+	const struct drm_connector_hdmi_cec_funcs *funcs;
+};
+
+static int drm_connector_hdmi_cec_adap_enable(struct cec_adapter *adap, bool enable)
+{
+	struct drm_connector *connector = cec_get_drvdata(adap);
+	struct drm_connector_hdmi_cec_data *data = connector->cec.data;
+
+	return data->funcs->enable(connector, enable);
+}
+
+static int drm_connector_hdmi_cec_adap_log_addr(struct cec_adapter *adap, u8 logical_addr)
+{
+	struct drm_connector *connector = cec_get_drvdata(adap);
+	struct drm_connector_hdmi_cec_data *data = connector->cec.data;
+
+	return data->funcs->log_addr(connector, logical_addr);
+}
+
+static int drm_connector_hdmi_cec_adap_transmit(struct cec_adapter *adap, u8 attempts,
+						u32 signal_free_time, struct cec_msg *msg)
+{
+	struct drm_connector *connector = cec_get_drvdata(adap);
+	struct drm_connector_hdmi_cec_data *data = connector->cec.data;
+
+	return data->funcs->transmit(connector, attempts, signal_free_time, msg);
+}
+
+static const struct cec_adap_ops drm_connector_hdmi_cec_adap_ops = {
+	.adap_enable = drm_connector_hdmi_cec_adap_enable,
+	.adap_log_addr = drm_connector_hdmi_cec_adap_log_addr,
+	.adap_transmit = drm_connector_hdmi_cec_adap_transmit,
+};
+
+static void drm_connector_hdmi_cec_adapter_phys_addr_invalidate(struct drm_connector *connector)
+{
+	struct drm_connector_hdmi_cec_data *data = connector->cec.data;
+
+	cec_phys_addr_invalidate(data->adapter);
+}
+
+static void drm_connector_hdmi_cec_adapter_phys_addr_set(struct drm_connector *connector,
+							 u16 addr)
+{
+	struct drm_connector_hdmi_cec_data *data = connector->cec.data;
+
+	cec_s_phys_addr(data->adapter, addr, false);
+}
+
+static void drm_connector_hdmi_cec_adapter_unregister(struct drm_connector *connector)
+{
+	struct drm_connector_hdmi_cec_data *data = connector->cec.data;
+
+	cec_delete_adapter(data->adapter);
+
+	if (data->funcs->uninit)
+		data->funcs->uninit(connector);
+
+	kfree(data);
+	connector->cec.data = NULL;
+}
+
+static struct drm_connector_cec_funcs drm_connector_hdmi_cec_adapter_funcs = {
+	.phys_addr_invalidate = drm_connector_hdmi_cec_adapter_phys_addr_invalidate,
+	.phys_addr_set = drm_connector_hdmi_cec_adapter_phys_addr_set,
+	.unregister = drm_connector_hdmi_cec_adapter_unregister,
+};
+
+int drm_connector_hdmi_cec_register(struct drm_connector *connector,
+				    const struct drm_connector_hdmi_cec_funcs *funcs,
+				    const char *name,
+				    u8 available_las,
+				    struct device *dev)
+{
+	struct drm_connector_hdmi_cec_data *data;
+	struct cec_connector_info conn_info;
+	struct cec_adapter *cec_adap;
+	int ret;
+
+	if (!funcs->init || !funcs->enable || !funcs->log_addr || !funcs->transmit)
+		return -EINVAL;
+
+	data = kzalloc(sizeof(*data), GFP_KERNEL);
+	if (!data)
+		return -ENOMEM;
+
+	data->funcs = funcs;
+
+	cec_adap = cec_allocate_adapter(&drm_connector_hdmi_cec_adap_ops, connector, name,
+					CEC_CAP_DEFAULTS | CEC_CAP_CONNECTOR_INFO,
+					available_las ? : CEC_MAX_LOG_ADDRS);
+	ret = PTR_ERR_OR_ZERO(cec_adap);
+	if (ret < 0)
+		goto err_free;
+
+	cec_fill_conn_info_from_drm(&conn_info, connector);
+	cec_s_conn_info(cec_adap, &conn_info);
+
+	data->adapter = cec_adap;
+
+	mutex_lock(&connector->cec.mutex);
+
+	connector->cec.data = data;
+	connector->cec.funcs = &drm_connector_hdmi_cec_adapter_funcs;
+
+	ret = funcs->init(connector);
+	if (ret < 0)
+		goto err_delete_adapter;
+
+	ret = cec_register_adapter(cec_adap, dev);
+	if (ret < 0)
+		goto err_delete_adapter;
+
+	mutex_unlock(&connector->cec.mutex);
+
+	return 0;
+
+err_delete_adapter:
+	cec_delete_adapter(cec_adap);
+
+	connector->cec.data = NULL;
+
+	mutex_unlock(&connector->cec.mutex);
+
+err_free:
+	kfree(data);
+
+	return ret;
+}
+EXPORT_SYMBOL(drm_connector_hdmi_cec_register);
+
+void drm_connector_hdmi_cec_received_msg(struct drm_connector *connector,
+					 struct cec_msg *msg)
+{
+	struct drm_connector_hdmi_cec_data *data = connector->cec.data;
+
+	cec_received_msg(data->adapter, msg);
+}
+EXPORT_SYMBOL(drm_connector_hdmi_cec_received_msg);
+
+void drm_connector_hdmi_cec_transmit_attempt_done(struct drm_connector *connector,
+						  u8 status)
+{
+	struct drm_connector_hdmi_cec_data *data = connector->cec.data;
+
+	cec_transmit_attempt_done(data->adapter, status);
+}
+EXPORT_SYMBOL(drm_connector_hdmi_cec_transmit_attempt_done);
+
+void drm_connector_hdmi_cec_transmit_done(struct drm_connector *connector,
+					  u8 status,
+					  u8 arb_lost_cnt, u8 nack_cnt,
+					  u8 low_drive_cnt, u8 error_cnt)
+{
+	struct drm_connector_hdmi_cec_data *data = connector->cec.data;
+
+	cec_transmit_done(data->adapter, status,
+			  arb_lost_cnt, nack_cnt, low_drive_cnt, error_cnt);
+}
+EXPORT_SYMBOL(drm_connector_hdmi_cec_transmit_done);
diff --git a/drivers/gpu/drm/display/drm_hdmi_cec_notifier_helper.c b/drivers/gpu/drm/display/drm_hdmi_cec_notifier_helper.c
new file mode 100644
index 0000000000000000000000000000000000000000..d87d30ec3fbb09d25eeb8ea382a77564f1071734
--- /dev/null
+++ b/drivers/gpu/drm/display/drm_hdmi_cec_notifier_helper.c
@@ -0,0 +1,60 @@
+// SPDX-License-Identifier: MIT
+/*
+ * Copyright (c) 2024 Linaro Ltd
+ */
+
+#include <drm/drm_bridge.h>
+#include <drm/drm_connector.h>
+#include <drm/display/drm_hdmi_cec_helper.h>
+
+#include <linux/mutex.h>
+
+#include <media/cec.h>
+#include <media/cec-notifier.h>
+
+static void drm_connector_hdmi_cec_notifier_phys_addr_invalidate(struct drm_connector *connector)
+{
+	cec_notifier_phys_addr_invalidate(connector->cec.data);
+}
+
+static void drm_connector_hdmi_cec_notifier_phys_addr_set(struct drm_connector *connector,
+							  u16 addr)
+{
+	cec_notifier_set_phys_addr(connector->cec.data, addr);
+}
+
+static void drm_connector_hdmi_cec_notifier_unregister(struct drm_connector *connector)
+{
+	cec_notifier_conn_unregister(connector->cec.data);
+	connector->cec.data = NULL;
+}
+
+static const struct drm_connector_cec_funcs drm_connector_cec_notifier_funcs = {
+	.phys_addr_invalidate = drm_connector_hdmi_cec_notifier_phys_addr_invalidate,
+	.phys_addr_set = drm_connector_hdmi_cec_notifier_phys_addr_set,
+	.unregister = drm_connector_hdmi_cec_notifier_unregister,
+};
+
+int drm_connector_hdmi_cec_notifier_register(struct drm_connector *connector,
+					     const char *port_name,
+					     struct device *dev)
+{
+	struct cec_connector_info conn_info;
+	struct cec_notifier *notifier;
+
+	cec_fill_conn_info_from_drm(&conn_info, connector);
+
+	notifier = cec_notifier_conn_register(dev, port_name, &conn_info);
+	if (!notifier)
+		return -ENOMEM;
+
+	mutex_lock(&connector->cec.mutex);
+
+	connector->cec.data = notifier;
+	connector->cec.funcs = &drm_connector_cec_notifier_funcs;
+
+	mutex_unlock(&connector->cec.mutex);
+
+	return 0;
+}
+EXPORT_SYMBOL(drm_connector_hdmi_cec_notifier_register);
diff --git a/include/drm/display/drm_hdmi_cec_helper.h b/include/drm/display/drm_hdmi_cec_helper.h
new file mode 100644
index 0000000000000000000000000000000000000000..39bb6d12acb35f539a4a6cd1b61ce97bf4e063ab
--- /dev/null
+++ b/include/drm/display/drm_hdmi_cec_helper.h
@@ -0,0 +1,74 @@
+/* SPDX-License-Identifier: MIT */
+
+#ifndef DRM_DISPLAY_HDMI_CEC_HELPER
+#define DRM_DISPLAY_HDMI_CEC_HELPER
+
+#include <drm/drm_connector.h>
+
+#include <linux/types.h>
+
+struct drm_connector;
+
+struct cec_msg;
+struct device;
+
+struct drm_connector_hdmi_cec_funcs {
+	/**
+	 * @init: perform hardware-specific initialization before registering the CEC adapter
+	 */
+	int (*init)(struct drm_connector *connector);
+
+	/**
+	 * @uninit: perform hardware-specific teardown for the CEC adapter
+	 */
+	void (*uninit)(struct drm_connector *connector);
+
+	/**
+	 * @enable: enable or disable CEC adapter
+	 */
+	int (*enable)(struct drm_connector *connector, bool enable);
+
+	/**
+	 * @log_addr: set adapter's logical address, can be called multiple
+	 * times if adapter supports several LAs
+	 */
+	int (*log_addr)(struct drm_connector *connector, u8 logical_addr);
+
+	/**
+	 * @transmit: start transmission of the specified CEC message
+	 */
+	int (*transmit)(struct drm_connector *connector, u8 attempts,
+			u32 signal_free_time, struct cec_msg *msg);
+};
+
+int drm_connector_hdmi_cec_register(struct drm_connector *connector,
+				    const struct drm_connector_hdmi_cec_funcs *funcs,
+				    const char *name,
+				    u8 available_las,
+				    struct device *dev);
+
+void drm_connector_hdmi_cec_received_msg(struct drm_connector *connector,
+					 struct cec_msg *msg);
+
+void drm_connector_hdmi_cec_transmit_done(struct drm_connector *connector,
+					  u8 status,
+					  u8 arb_lost_cnt, u8 nack_cnt,
+					  u8 low_drive_cnt, u8 error_cnt);
+
+void drm_connector_hdmi_cec_transmit_attempt_done(struct drm_connector *connector,
+						  u8 status);
+
+#if IS_ENABLED(CONFIG_DRM_DISPLAY_HDMI_CEC_NOTIFIER_HELPER)
+int drm_connector_hdmi_cec_notifier_register(struct drm_connector *connector,
+					     const char *port_name,
+					     struct device *dev);
+#else
+static inline int drm_connector_hdmi_cec_notifier_register(struct drm_connector *connector,
+							   const char *port_name,
+							   struct device *dev)
+{
+	return 0;
+}
+#endif
+
+#endif

-- 
2.39.5

[PATCH v5 07/11] drm/display: hdmi-state-helper: handle CEC physical address

[Dmitry Baryshkov]

Call HDMI CEC helpers in order to update physical address of the
adapter.

Reviewed-by: Maxime Ripard <mripard@kernel.org>
Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
---
 drivers/gpu/drm/display/drm_hdmi_state_helper.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/display/drm_hdmi_state_helper.c b/drivers/gpu/drm/display/drm_hdmi_state_helper.c
index d9d9948b29e9d5ef9bc9cc9108b3ace4aca2e3ae..bae7aa624f7db61cc7d5ff7a86a413938963543f 100644
--- a/drivers/gpu/drm/display/drm_hdmi_state_helper.c
+++ b/drivers/gpu/drm/display/drm_hdmi_state_helper.c
@@ -6,6 +6,7 @@
 #include <drm/drm_print.h>
 
 #include <drm/display/drm_hdmi_audio_helper.h>
+#include <drm/display/drm_hdmi_cec_helper.h>
 #include <drm/display/drm_hdmi_helper.h>
 #include <drm/display/drm_hdmi_state_helper.h>
 
@@ -1081,9 +1082,10 @@ drm_atomic_helper_connector_hdmi_update(struct drm_connector *connector,
 	const struct drm_edid *drm_edid;
 
 	if (status == connector_status_disconnected) {
-		// TODO: also handle CEC and scramber, HDMI sink disconnected.
+		// TODO: also handle scramber, HDMI sink disconnected.
 		drm_connector_hdmi_audio_plugged_notify(connector, false);
 		drm_edid_connector_update(connector, NULL);
+		drm_connector_cec_phys_addr_invalidate(connector);
 		return;
 	}
 
@@ -1097,8 +1099,9 @@ drm_atomic_helper_connector_hdmi_update(struct drm_connector *connector,
 	drm_edid_free(drm_edid);
 
 	if (status == connector_status_connected) {
-		// TODO: also handle CEC and scramber, HDMI sink is now connected.
+		// TODO: also handle scramber, HDMI sink is now connected.
 		drm_connector_hdmi_audio_plugged_notify(connector, true);
+		drm_connector_cec_phys_addr_set(connector);
 	}
 }
 

-- 
2.39.5

[PATCH v5 08/11] drm/vc4: hdmi: switch to generic CEC helpers

[Dmitry Baryshkov]

Switch VC4 driver to using CEC helpers code, simplifying hotplug and
registration / cleanup. The existing vc4_hdmi_cec_release() is kept for
now.

Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
---
 drivers/gpu/drm/vc4/Kconfig    |   1 +
 drivers/gpu/drm/vc4/vc4_hdmi.c | 138 ++++++++++++++++-------------------------
 drivers/gpu/drm/vc4/vc4_hdmi.h |   1 -
 3 files changed, 56 insertions(+), 84 deletions(-)

diff --git a/drivers/gpu/drm/vc4/Kconfig b/drivers/gpu/drm/vc4/Kconfig
index 123ab0ce178157c3b39466f87c7ac39c8470f329..bb8c40be325033632d3e94db87a16b03554ad3af 100644
--- a/drivers/gpu/drm/vc4/Kconfig
+++ b/drivers/gpu/drm/vc4/Kconfig
@@ -35,6 +35,7 @@ config DRM_VC4_HDMI_CEC
 	bool "Broadcom VC4 HDMI CEC Support"
 	depends on DRM_VC4
 	select CEC_CORE
+	select DRM_DISPLAY_HDMI_CEC_HELPER
 	help
 	  Choose this option if you have a Broadcom VC4 GPU
 	  and want to use CEC.
diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c
index 4797ed1c21f47992fe4d497d904ee31c824cd449..3bc7c4bd496b60ccb0c7dce8b41e5cd9f01b18ab 100644
--- a/drivers/gpu/drm/vc4/vc4_hdmi.c
+++ b/drivers/gpu/drm/vc4/vc4_hdmi.c
@@ -32,6 +32,7 @@
  */
 
 #include <drm/display/drm_hdmi_audio_helper.h>
+#include <drm/display/drm_hdmi_cec_helper.h>
 #include <drm/display/drm_hdmi_helper.h>
 #include <drm/display/drm_hdmi_state_helper.h>
 #include <drm/display/drm_scdc_helper.h>
@@ -375,14 +376,6 @@ static void vc4_hdmi_handle_hotplug(struct vc4_hdmi *vc4_hdmi,
 
 	drm_atomic_helper_connector_hdmi_hotplug(connector, status);
 
-	if (status == connector_status_disconnected) {
-		cec_phys_addr_invalidate(vc4_hdmi->cec_adap);
-		return;
-	}
-
-	cec_s_phys_addr(vc4_hdmi->cec_adap,
-			connector->display_info.source_physical_address, false);
-
 	if (status != connector_status_connected)
 		return;
 
@@ -2378,8 +2371,8 @@ static irqreturn_t vc4_cec_irq_handler_rx_thread(int irq, void *priv)
 	struct vc4_hdmi *vc4_hdmi = priv;
 
 	if (vc4_hdmi->cec_rx_msg.len)
-		cec_received_msg(vc4_hdmi->cec_adap,
-				 &vc4_hdmi->cec_rx_msg);
+		drm_connector_hdmi_cec_received_msg(&vc4_hdmi->connector,
+						    &vc4_hdmi->cec_rx_msg);
 
 	return IRQ_HANDLED;
 }
@@ -2389,15 +2382,17 @@ static irqreturn_t vc4_cec_irq_handler_tx_thread(int irq, void *priv)
 	struct vc4_hdmi *vc4_hdmi = priv;
 
 	if (vc4_hdmi->cec_tx_ok) {
-		cec_transmit_done(vc4_hdmi->cec_adap, CEC_TX_STATUS_OK,
-				  0, 0, 0, 0);
+		drm_connector_hdmi_cec_transmit_done(&vc4_hdmi->connector,
+						     CEC_TX_STATUS_OK,
+						     0, 0, 0, 0);
 	} else {
 		/*
 		 * This CEC implementation makes 1 retry, so if we
 		 * get a NACK, then that means it made 2 attempts.
 		 */
-		cec_transmit_done(vc4_hdmi->cec_adap, CEC_TX_STATUS_NACK,
-				  0, 2, 0, 0);
+		drm_connector_hdmi_cec_transmit_done(&vc4_hdmi->connector,
+						     CEC_TX_STATUS_NACK,
+						     0, 2, 0, 0);
 	}
 	return IRQ_HANDLED;
 }
@@ -2554,9 +2549,9 @@ static irqreturn_t vc4_cec_irq_handler(int irq, void *priv)
 	return ret;
 }
 
-static int vc4_hdmi_cec_enable(struct cec_adapter *adap)
+static int vc4_hdmi_cec_enable(struct drm_connector *connector)
 {
-	struct vc4_hdmi *vc4_hdmi = cec_get_drvdata(adap);
+	struct vc4_hdmi *vc4_hdmi = connector_to_vc4_hdmi(connector);
 	struct drm_device *drm = vc4_hdmi->connector.dev;
 	/* clock period in microseconds */
 	const u32 usecs = 1000000 / CEC_CLOCK_FREQ;
@@ -2621,9 +2616,9 @@ static int vc4_hdmi_cec_enable(struct cec_adapter *adap)
 	return 0;
 }
 
-static int vc4_hdmi_cec_disable(struct cec_adapter *adap)
+static int vc4_hdmi_cec_disable(struct drm_connector *connector)
 {
-	struct vc4_hdmi *vc4_hdmi = cec_get_drvdata(adap);
+	struct vc4_hdmi *vc4_hdmi = connector_to_vc4_hdmi(connector);
 	struct drm_device *drm = vc4_hdmi->connector.dev;
 	unsigned long flags;
 	int idx;
@@ -2657,17 +2652,17 @@ static int vc4_hdmi_cec_disable(struct cec_adapter *adap)
 	return 0;
 }
 
-static int vc4_hdmi_cec_adap_enable(struct cec_adapter *adap, bool enable)
+static int vc4_hdmi_cec_adap_enable(struct drm_connector *connector, bool enable)
 {
 	if (enable)
-		return vc4_hdmi_cec_enable(adap);
+		return vc4_hdmi_cec_enable(connector);
 	else
-		return vc4_hdmi_cec_disable(adap);
+		return vc4_hdmi_cec_disable(connector);
 }
 
-static int vc4_hdmi_cec_adap_log_addr(struct cec_adapter *adap, u8 log_addr)
+static int vc4_hdmi_cec_adap_log_addr(struct drm_connector *connector, u8 log_addr)
 {
-	struct vc4_hdmi *vc4_hdmi = cec_get_drvdata(adap);
+	struct vc4_hdmi *vc4_hdmi = connector_to_vc4_hdmi(connector);
 	struct drm_device *drm = vc4_hdmi->connector.dev;
 	unsigned long flags;
 	int idx;
@@ -2693,10 +2688,10 @@ static int vc4_hdmi_cec_adap_log_addr(struct cec_adapter *adap, u8 log_addr)
 	return 0;
 }
 
-static int vc4_hdmi_cec_adap_transmit(struct cec_adapter *adap, u8 attempts,
+static int vc4_hdmi_cec_adap_transmit(struct drm_connector *connector, u8 attempts,
 				      u32 signal_free_time, struct cec_msg *msg)
 {
-	struct vc4_hdmi *vc4_hdmi = cec_get_drvdata(adap);
+	struct vc4_hdmi *vc4_hdmi = connector_to_vc4_hdmi(connector);
 	struct drm_device *dev = vc4_hdmi->connector.dev;
 	unsigned long flags;
 	u32 val;
@@ -2739,84 +2734,66 @@ static int vc4_hdmi_cec_adap_transmit(struct cec_adapter *adap, u8 attempts,
 	return 0;
 }
 
-static const struct cec_adap_ops vc4_hdmi_cec_adap_ops = {
-	.adap_enable = vc4_hdmi_cec_adap_enable,
-	.adap_log_addr = vc4_hdmi_cec_adap_log_addr,
-	.adap_transmit = vc4_hdmi_cec_adap_transmit,
-};
-
-static void vc4_hdmi_cec_release(void *ptr)
-{
-	struct vc4_hdmi *vc4_hdmi = ptr;
-
-	cec_unregister_adapter(vc4_hdmi->cec_adap);
-	vc4_hdmi->cec_adap = NULL;
-}
-
-static int vc4_hdmi_cec_init(struct vc4_hdmi *vc4_hdmi)
+static int vc4_hdmi_cec_init(struct drm_connector *connector)
 {
-	struct cec_connector_info conn_info;
+	struct vc4_hdmi *vc4_hdmi = connector_to_vc4_hdmi(connector);
 	struct platform_device *pdev = vc4_hdmi->pdev;
 	struct device *dev = &pdev->dev;
 	int ret;
 
-	if (!of_property_present(dev->of_node, "interrupts")) {
-		dev_warn(dev, "'interrupts' DT property is missing, no CEC\n");
-		return 0;
-	}
-
-	vc4_hdmi->cec_adap = cec_allocate_adapter(&vc4_hdmi_cec_adap_ops,
-						  vc4_hdmi,
-						  vc4_hdmi->variant->card_name,
-						  CEC_CAP_DEFAULTS |
-						  CEC_CAP_CONNECTOR_INFO, 1);
-	ret = PTR_ERR_OR_ZERO(vc4_hdmi->cec_adap);
-	if (ret < 0)
-		return ret;
-
-	cec_fill_conn_info_from_drm(&conn_info, &vc4_hdmi->connector);
-	cec_s_conn_info(vc4_hdmi->cec_adap, &conn_info);
-
 	if (vc4_hdmi->variant->external_irq_controller) {
 		ret = devm_request_threaded_irq(dev, platform_get_irq_byname(pdev, "cec-rx"),
 						vc4_cec_irq_handler_rx_bare,
 						vc4_cec_irq_handler_rx_thread, 0,
 						"vc4 hdmi cec rx", vc4_hdmi);
 		if (ret)
-			goto err_delete_cec_adap;
+			return ret;
 
 		ret = devm_request_threaded_irq(dev, platform_get_irq_byname(pdev, "cec-tx"),
 						vc4_cec_irq_handler_tx_bare,
 						vc4_cec_irq_handler_tx_thread, 0,
 						"vc4 hdmi cec tx", vc4_hdmi);
 		if (ret)
-			goto err_delete_cec_adap;
+			return ret;
 	} else {
 		ret = devm_request_threaded_irq(dev, platform_get_irq(pdev, 0),
 						vc4_cec_irq_handler,
 						vc4_cec_irq_handler_thread, 0,
 						"vc4 hdmi cec", vc4_hdmi);
 		if (ret)
-			goto err_delete_cec_adap;
+			return ret;
 	}
 
-	ret = cec_register_adapter(vc4_hdmi->cec_adap, &pdev->dev);
-	if (ret < 0)
-		goto err_delete_cec_adap;
+	return 0;
+}
+
+static const struct drm_connector_hdmi_cec_adapter_ops vc4_hdmi_cec_adap_ops = {
+	.base.unregister = drm_connector_hdmi_cec_unregister,
+	.init = vc4_hdmi_cec_init,
+	.enable = vc4_hdmi_cec_adap_enable,
+	.log_addr = vc4_hdmi_cec_adap_log_addr,
+	.transmit = vc4_hdmi_cec_adap_transmit,
+};
+
+static int vc4_hdmi_cec_register(struct vc4_hdmi *vc4_hdmi)
+{
+	struct platform_device *pdev = vc4_hdmi->pdev;
+	struct device *dev = &pdev->dev;
+
+	if (!of_property_present(dev->of_node, "interrupts")) {
+		dev_warn(dev, "'interrupts' DT property is missing, no CEC\n");
+		return 0;
+	}
 
 	/*
-	 * NOTE: Strictly speaking, we should probably use a DRM-managed
-	 * registration there to avoid removing the CEC adapter by the
-	 * time the DRM driver doesn't have any user anymore.
+	 * NOTE: the CEC adapter will be unregistered from
+	 * drm_connector_cleanup(), which is called from drm_dev_unplug()
+	 * during device unbind.
 	 *
 	 * However, the CEC framework already cleans up the CEC adapter
 	 * only when the last user has closed its file descriptor, so we
 	 * don't need to handle it in DRM.
 	 *
-	 * By the time the device-managed hook is executed, we will give
-	 * up our reference to the CEC adapter and therefore don't
-	 * really care when it's actually freed.
-	 *
 	 * There's still a problematic sequence: if we unregister our
 	 * CEC adapter, but the userspace keeps a handle on the CEC
 	 * adapter but not the DRM device for some reason. In such a
@@ -2827,19 +2804,14 @@ static int vc4_hdmi_cec_init(struct vc4_hdmi *vc4_hdmi)
 	 * the CEC framework already handles this too, by calling
 	 * cec_is_registered() in cec_ioctl() and cec_poll().
 	 */
-	ret = devm_add_action_or_reset(dev, vc4_hdmi_cec_release, vc4_hdmi);
-	if (ret)
-		return ret;
-
-	return 0;
-
-err_delete_cec_adap:
-	cec_delete_adapter(vc4_hdmi->cec_adap);
-
-	return ret;
+	return drm_connector_hdmi_cec_register(&vc4_hdmi->connector,
+					       &vc4_hdmi_cec_adap_ops,
+					       vc4_hdmi->variant->card_name,
+					       1,
+					       &pdev->dev);
 }
 #else
-static int vc4_hdmi_cec_init(struct vc4_hdmi *vc4_hdmi)
+static int vc4_hdmi_cec_register(struct vc4_hdmi *vc4_hdmi)
 {
 	return 0;
 }
@@ -3244,7 +3216,7 @@ static int vc4_hdmi_bind(struct device *dev, struct device *master, void *data)
 	if (ret)
 		goto err_put_runtime_pm;
 
-	ret = vc4_hdmi_cec_init(vc4_hdmi);
+	ret = vc4_hdmi_cec_register(vc4_hdmi);
 	if (ret)
 		goto err_put_runtime_pm;
 
diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.h b/drivers/gpu/drm/vc4/vc4_hdmi.h
index a31157c99bee6b33527c4b558fc72fff65d2a278..8d069718df00d9afc13aadbb12648e2bb75a1721 100644
--- a/drivers/gpu/drm/vc4/vc4_hdmi.h
+++ b/drivers/gpu/drm/vc4/vc4_hdmi.h
@@ -147,7 +147,6 @@ struct vc4_hdmi {
 	 */
 	bool disable_wifi_frequencies;
 
-	struct cec_adapter *cec_adap;
 	struct cec_msg cec_rx_msg;
 	bool cec_tx_ok;
 	bool cec_irq_was_rx;

-- 
2.39.5

[PATCH v5 09/11] drm/display: bridge-connector: hook in CEC notifier support

[Dmitry Baryshkov]

Allow HDMI DRM bridges to create CEC notifier. Physical address is
handled automatically by drm_atomic_helper_connector_hdmi_hotplug()
being called from .detect() path.

Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
---
 drivers/gpu/drm/display/Kconfig                |  1 +
 drivers/gpu/drm/display/drm_bridge_connector.c | 24 ++++++++++++++++++++++++
 include/drm/drm_bridge.h                       | 11 +++++++++++
 3 files changed, 36 insertions(+)

diff --git a/drivers/gpu/drm/display/Kconfig b/drivers/gpu/drm/display/Kconfig
index 6376ea01ec3093a72de25064e31223d2c9868ed7..df09cf9a8ca19ea894d6f2fad68c0b191e81e3d0 100644
--- a/drivers/gpu/drm/display/Kconfig
+++ b/drivers/gpu/drm/display/Kconfig
@@ -17,6 +17,7 @@ if DRM_DISPLAY_HELPER
 config DRM_BRIDGE_CONNECTOR
 	bool
 	select DRM_DISPLAY_HDMI_AUDIO_HELPER
+	select DRM_DISPLAY_HDMI_CEC_HELPER
 	select DRM_DISPLAY_HDMI_STATE_HELPER
 	help
 	  DRM connector implementation terminating DRM bridge chains.
diff --git a/drivers/gpu/drm/display/drm_bridge_connector.c b/drivers/gpu/drm/display/drm_bridge_connector.c
index 381a0f9d4259bf9f72d3a292b7dcc82e45c61bae..daf6117268d9d3bb0c0c12b5094e79ad13cf72dd 100644
--- a/drivers/gpu/drm/display/drm_bridge_connector.c
+++ b/drivers/gpu/drm/display/drm_bridge_connector.c
@@ -20,6 +20,7 @@
 #include <drm/drm_print.h>
 #include <drm/drm_probe_helper.h>
 #include <drm/display/drm_hdmi_audio_helper.h>
+#include <drm/display/drm_hdmi_cec_helper.h>
 #include <drm/display/drm_hdmi_helper.h>
 #include <drm/display/drm_hdmi_state_helper.h>
 
@@ -113,6 +114,13 @@ struct drm_bridge_connector {
 	 * &DRM_BRIDGE_OP_DP_AUDIO).
 	 */
 	struct drm_bridge *bridge_dp_audio;
+	/**
+	 * @bridge_hdmi_cec:
+	 *
+	 * The bridge in the chain that implements CEC support, if any (see
+	 * DRM_BRIDGE_OP_HDMI_CEC_NOTIFIER).
+	 */
+	struct drm_bridge *bridge_hdmi_cec;
 };
 
 #define to_drm_bridge_connector(x) \
@@ -662,6 +670,13 @@ struct drm_connector *drm_bridge_connector_init(struct drm_device *drm,
 			bridge_connector->bridge_dp_audio = bridge;
 		}
 
+		if (bridge->ops & DRM_BRIDGE_OP_HDMI_CEC_NOTIFIER) {
+			if (bridge_connector->bridge_hdmi_cec)
+				return ERR_PTR(-EBUSY);
+
+			bridge_connector->bridge_hdmi_cec = bridge;
+		}
+
 		if (!drm_bridge_get_next_bridge(bridge))
 			connector_type = bridge->type;
 
@@ -724,6 +739,15 @@ struct drm_connector *drm_bridge_connector_init(struct drm_device *drm,
 			return ERR_PTR(ret);
 	}
 
+	if (bridge_connector->bridge_hdmi_cec &&
+	    bridge_connector->bridge_hdmi_cec->ops & DRM_BRIDGE_OP_HDMI_CEC_NOTIFIER) {
+		ret = drm_connector_hdmi_cec_notifier_register(connector,
+							       NULL,
+							       bridge->hdmi_cec_dev);
+		if (ret)
+			return ERR_PTR(ret);
+	}
+
 	drm_connector_helper_add(connector, &drm_bridge_connector_helper_funcs);
 
 	if (bridge_connector->bridge_hpd)
diff --git a/include/drm/drm_bridge.h b/include/drm/drm_bridge.h
index db0d374d863b0b1f774d395743f1e29bb84e8937..0e5f6a007d536215bd50e11846433290c2060b9c 100644
--- a/include/drm/drm_bridge.h
+++ b/include/drm/drm_bridge.h
@@ -907,6 +907,11 @@ enum drm_bridge_ops {
 	 * flag.
 	 */
 	DRM_BRIDGE_OP_DP_AUDIO = BIT(6),
+	/**
+	 * @DRM_BRIDGE_OP_HDMI_CEC_NOTIFIER: The bridge requires CEC notifier
+	 * to be present.
+	 */
+	DRM_BRIDGE_OP_HDMI_CEC_NOTIFIER = BIT(7),
 };
 
 /**
@@ -1003,6 +1008,12 @@ struct drm_bridge {
 	 */
 	unsigned int max_bpc;
 
+	/**
+	 * @hdmi_cec_dev: device to be used as a containing device for CEC
+	 * functions.
+	 */
+	struct device *hdmi_cec_dev;
+
 	/**
 	 * @hdmi_audio_dev: device to be used as a parent for the HDMI Codec if
 	 * either of @DRM_BRIDGE_OP_HDMI_AUDIO or @DRM_BRIDGE_OP_DP_AUDIO is set.

-- 
2.39.5

[PATCH v5 10/11] drm/display: bridge-connector: handle CEC adapters

[Dmitry Baryshkov]

Implement necessary glue code to let DRM bridge drivers to implement CEC
adapters support.

Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
---
 drivers/gpu/drm/display/drm_bridge_connector.c | 83 ++++++++++++++++++++++++++
 include/drm/drm_bridge.h                       | 26 ++++++++
 2 files changed, 109 insertions(+)

diff --git a/drivers/gpu/drm/display/drm_bridge_connector.c b/drivers/gpu/drm/display/drm_bridge_connector.c
index daf6117268d9d3bb0c0c12b5094e79ad13cf72dd..54b49be7837a174e0a25eec3395fd10d9c487c9e 100644
--- a/drivers/gpu/drm/display/drm_bridge_connector.c
+++ b/drivers/gpu/drm/display/drm_bridge_connector.c
@@ -554,6 +554,66 @@ 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_cec_enable(struct drm_connector *connector, bool enable)
+{
+	struct drm_bridge_connector *bridge_connector =
+		to_drm_bridge_connector(connector);
+	struct drm_bridge *bridge;
+
+	bridge = bridge_connector->bridge_hdmi_cec;
+
+	return bridge->funcs->hdmi_cec_enable(bridge, enable);
+}
+
+static int drm_bridge_connector_hdmi_cec_log_addr(struct drm_connector *connector, u8 logical_addr)
+{
+	struct drm_bridge_connector *bridge_connector =
+		to_drm_bridge_connector(connector);
+	struct drm_bridge *bridge;
+
+	bridge = bridge_connector->bridge_hdmi_cec;
+
+	return bridge->funcs->hdmi_cec_log_addr(bridge, logical_addr);
+}
+
+static int drm_bridge_connector_hdmi_cec_transmit(struct drm_connector *connector,
+						  u8 attempts,
+						  u32 signal_free_time,
+						  struct cec_msg *msg)
+{
+	struct drm_bridge_connector *bridge_connector =
+		to_drm_bridge_connector(connector);
+	struct drm_bridge *bridge;
+
+	bridge = bridge_connector->bridge_hdmi_cec;
+
+	return bridge->funcs->hdmi_cec_transmit(bridge, attempts,
+						signal_free_time,
+						msg);
+}
+
+static int drm_bridge_connector_hdmi_cec_init(struct drm_connector *connector)
+{
+	struct drm_bridge_connector *bridge_connector =
+		to_drm_bridge_connector(connector);
+	struct drm_bridge *bridge;
+
+	bridge = bridge_connector->bridge_hdmi_cec;
+
+	if (!bridge->funcs->hdmi_cec_init)
+		return 0;
+
+	return bridge->funcs->hdmi_cec_init(connector, bridge);
+}
+
+static const struct drm_connector_hdmi_cec_funcs drm_bridge_connector_hdmi_cec_funcs = {
+	.init = drm_bridge_connector_hdmi_cec_init,
+	.enable = drm_bridge_connector_hdmi_cec_enable,
+	.log_addr = drm_bridge_connector_hdmi_cec_log_addr,
+	.transmit = drm_bridge_connector_hdmi_cec_transmit,
+};
+
+
 /* -----------------------------------------------------------------------------
  * Bridge Connector Initialisation
  */
@@ -677,6 +737,18 @@ struct drm_connector *drm_bridge_connector_init(struct drm_device *drm,
 			bridge_connector->bridge_hdmi_cec = bridge;
 		}
 
+		if (bridge->ops & DRM_BRIDGE_OP_HDMI_CEC_ADAPTER) {
+			if (bridge_connector->bridge_hdmi_cec)
+				return ERR_PTR(-EBUSY);
+
+			bridge_connector->bridge_hdmi_cec = bridge;
+
+			if (!bridge->funcs->hdmi_cec_enable ||
+			    !bridge->funcs->hdmi_cec_log_addr ||
+			    !bridge->funcs->hdmi_cec_transmit)
+				return ERR_PTR(-EINVAL);
+		}
+
 		if (!drm_bridge_get_next_bridge(bridge))
 			connector_type = bridge->type;
 
@@ -748,6 +820,17 @@ struct drm_connector *drm_bridge_connector_init(struct drm_device *drm,
 			return ERR_PTR(ret);
 	}
 
+	if (bridge_connector->bridge_hdmi_cec &&
+	    bridge_connector->bridge_hdmi_cec->ops & DRM_BRIDGE_OP_HDMI_CEC_ADAPTER) {
+		ret = drm_connector_hdmi_cec_register(connector,
+						      &drm_bridge_connector_hdmi_cec_funcs,
+						      bridge->hdmi_cec_adapter_name,
+						      bridge->hdmi_cec_available_las,
+						      bridge->hdmi_cec_dev);
+		if (ret)
+			return ERR_PTR(ret);
+	}
+
 	drm_connector_helper_add(connector, &drm_bridge_connector_helper_funcs);
 
 	if (bridge_connector->bridge_hpd)
diff --git a/include/drm/drm_bridge.h b/include/drm/drm_bridge.h
index 0e5f6a007d536215bd50e11846433290c2060b9c..cc9f7df38102e3c43913b35312f0ed5c4d8a7bd0 100644
--- a/include/drm/drm_bridge.h
+++ b/include/drm/drm_bridge.h
@@ -32,6 +32,7 @@
 #include <drm/drm_mode_object.h>
 #include <drm/drm_modes.h>
 
+struct cec_msg;
 struct device_node;
 
 struct drm_bridge;
@@ -737,6 +738,16 @@ struct drm_bridge_funcs {
 				      struct drm_bridge *bridge,
 				      bool enable, int direction);
 
+	int (*hdmi_cec_init)(struct drm_connector *connector,
+			     struct drm_bridge *bridge);
+
+	int (*hdmi_cec_enable)(struct drm_bridge *bridge, bool enable);
+
+	int (*hdmi_cec_log_addr)(struct drm_bridge *bridge, u8 logical_addr);
+
+	int (*hdmi_cec_transmit)(struct drm_bridge *bridge, u8 attempts,
+				 u32 signal_free_time, struct cec_msg *msg);
+
 	/**
 	 * @dp_audio_startup:
 	 *
@@ -912,6 +923,11 @@ enum drm_bridge_ops {
 	 * to be present.
 	 */
 	DRM_BRIDGE_OP_HDMI_CEC_NOTIFIER = BIT(7),
+	/**
+	 * @DRM_BRIDGE_OP_HDMI_CEC_ADAPTER: The bridge requires CEC notifier
+	 * to be present.
+	 */
+	DRM_BRIDGE_OP_HDMI_CEC_ADAPTER = BIT(8),
 };
 
 /**
@@ -1048,6 +1064,16 @@ struct drm_bridge {
 	 */
 	int hdmi_audio_dai_port;
 
+	/**
+	 * @hdmi_cec_adapter_name: the name of the adapter to register
+	 */
+	const char *hdmi_cec_adapter_name;
+
+	/**
+	 * @hdmi_cec_available_las: number of logical addresses, CEC_MAX_LOG_ADDRS if unset
+	 */
+	u8 hdmi_cec_available_las;
+
 	/** private: */
 	/**
 	 * @hpd_mutex: Protects the @hpd_cb and @hpd_data fields.

-- 
2.39.5

[PATCH v5 11/11] drm/bridge: adv7511: switch to the HDMI connector helpers

[Dmitry Baryshkov]

Rewrite the ADV7511 driver to use implementation provided by the DRM
HDMI connector framework, including the Audio and CEC bits. Drop the
in-bridge connector support and use drm_bridge_connector if the host
requires the connector to be provided by the bridge.

Note: currently only AVI InfoFrames are supported. Existing driver
doesn't support programming any other InfoFrames directly and Audio
InfoFrame seems to be programmed using individual bits and pieces rather
than programming it directly.

Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
---
 drivers/gpu/drm/bridge/adv7511/Kconfig         |   5 +-
 drivers/gpu/drm/bridge/adv7511/adv7511.h       |  52 ++--
 drivers/gpu/drm/bridge/adv7511/adv7511_audio.c |  77 +-----
 drivers/gpu/drm/bridge/adv7511/adv7511_cec.c   |  57 ++--
 drivers/gpu/drm/bridge/adv7511/adv7511_drv.c   | 345 +++++++++++--------------
 drivers/gpu/drm/bridge/adv7511/adv7533.c       |   4 -
 6 files changed, 212 insertions(+), 328 deletions(-)

diff --git a/drivers/gpu/drm/bridge/adv7511/Kconfig b/drivers/gpu/drm/bridge/adv7511/Kconfig
index f46a5e26b5dd640670afa21802f9019d5c7439fb..59a5256ce8a6e16dfbf1848a7c85ac7d709a68ed 100644
--- a/drivers/gpu/drm/bridge/adv7511/Kconfig
+++ b/drivers/gpu/drm/bridge/adv7511/Kconfig
@@ -5,6 +5,9 @@ config DRM_I2C_ADV7511
 	select DRM_KMS_HELPER
 	select REGMAP_I2C
 	select DRM_MIPI_DSI
+	select DRM_DISPLAY_HELPER
+	select DRM_BRIDGE_CONNECTOR
+	select DRM_DISPLAY_HDMI_STATE_HELPER
 	help
 	  Support for the Analog Devices ADV7511(W)/13/33/35 HDMI encoders.
 
@@ -19,7 +22,7 @@ config DRM_I2C_ADV7511_AUDIO
 config DRM_I2C_ADV7511_CEC
 	bool "ADV7511/33/35 HDMI CEC driver"
 	depends on DRM_I2C_ADV7511
-	select CEC_CORE
+	select DRM_DISPLAY_HDMI_CEC_HELPER
 	default y
 	help
 	  When selected the HDMI transmitter will support the CEC feature.
diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511.h b/drivers/gpu/drm/bridge/adv7511/adv7511.h
index ec0b7f3d889c4eedeb1d80369fd2a160cd0e2968..90c9a3da2406d16c8988548a87053c122a332f31 100644
--- a/drivers/gpu/drm/bridge/adv7511/adv7511.h
+++ b/drivers/gpu/drm/bridge/adv7511/adv7511.h
@@ -313,16 +313,11 @@ enum adv7511_csc_scaling {
  * @csc_enable:			Whether to enable color space conversion
  * @csc_scaling_factor:		Color space conversion scaling factor
  * @csc_coefficents:		Color space conversion coefficents
- * @hdmi_mode:			Whether to use HDMI or DVI output mode
- * @avi_infoframe:		HDMI infoframe
  */
 struct adv7511_video_config {
 	bool csc_enable;
 	enum adv7511_csc_scaling csc_scaling_factor;
 	const uint16_t *csc_coefficents;
-
-	bool hdmi_mode;
-	struct hdmi_avi_infoframe avi_infoframe;
 };
 
 enum adv7511_type {
@@ -337,6 +332,7 @@ struct adv7511_chip_info {
 	enum adv7511_type type;
 	unsigned int max_mode_clock_khz;
 	unsigned int max_lane_freq_khz;
+	const char *name;
 	const char * const *supply_names;
 	unsigned int num_supplies;
 	unsigned int reg_cec_offset;
@@ -371,7 +367,7 @@ struct adv7511 {
 	struct work_struct hpd_work;
 
 	struct drm_bridge bridge;
-	struct drm_connector connector;
+	struct drm_connector *cec_connector;
 
 	bool embedded_sync;
 	enum adv7511_sync_polarity vsync_polarity;
@@ -389,9 +385,7 @@ struct adv7511 {
 	bool use_timing_gen;
 
 	const struct adv7511_chip_info *info;
-	struct platform_device *audio_pdev;
 
-	struct cec_adapter *cec_adap;
 	u8   cec_addr[ADV7511_MAX_ADDRS];
 	u8   cec_valid_addrs;
 	bool cec_enabled_adap;
@@ -399,16 +393,24 @@ struct adv7511 {
 	u32 cec_clk_freq;
 };
 
+static inline struct adv7511 *bridge_to_adv7511(struct drm_bridge *bridge)
+{
+	return container_of(bridge, struct adv7511, bridge);
+}
+
 #ifdef CONFIG_DRM_I2C_ADV7511_CEC
-int adv7511_cec_init(struct device *dev, struct adv7511 *adv7511);
+int adv7511_cec_init(struct drm_connector *connector,
+		     struct drm_bridge *bridge);
+int adv7511_cec_enable(struct drm_bridge *bridge, bool enable);
+int adv7511_cec_log_addr(struct drm_bridge *bridge, u8 addr);
+int adv7511_cec_transmit(struct drm_bridge *bridge, u8 attempts,
+			      u32 signal_free_time, struct cec_msg *msg);
 int adv7511_cec_irq_process(struct adv7511 *adv7511, unsigned int irq1);
 #else
-static inline int adv7511_cec_init(struct device *dev, struct adv7511 *adv7511)
-{
-	regmap_write(adv7511->regmap, ADV7511_REG_CEC_CTRL,
-		     ADV7511_CEC_CTRL_POWER_DOWN);
-	return 0;
-}
+#define adv7511_cec_init NULL
+#define adv7511_cec_enable NULL
+#define adv7511_cec_log_addr NULL
+#define adv7511_cec_transmit NULL
 #endif
 
 void adv7533_dsi_power_on(struct adv7511 *adv);
@@ -421,16 +423,18 @@ int adv7533_attach_dsi(struct adv7511 *adv);
 int adv7533_parse_dt(struct device_node *np, struct adv7511 *adv);
 
 #ifdef CONFIG_DRM_I2C_ADV7511_AUDIO
-int adv7511_audio_init(struct device *dev, struct adv7511 *adv7511);
-void adv7511_audio_exit(struct adv7511 *adv7511);
+int adv7511_hdmi_audio_startup(struct drm_connector *connector,
+			       struct drm_bridge *bridge);
+void adv7511_hdmi_audio_shutdown(struct drm_connector *connector,
+				 struct drm_bridge *bridge);
+int adv7511_hdmi_audio_prepare(struct drm_connector *connector,
+			       struct drm_bridge *bridge,
+			       struct hdmi_codec_daifmt *fmt,
+			       struct hdmi_codec_params *hparms);
 #else /*CONFIG_DRM_I2C_ADV7511_AUDIO */
-static inline int adv7511_audio_init(struct device *dev, struct adv7511 *adv7511)
-{
-	return 0;
-}
-static inline void adv7511_audio_exit(struct adv7511 *adv7511)
-{
-}
+#define adv7511_hdmi_audio_startup NULL
+#define adv7511_hdmi_audio_shutdown NULL
+#define adv7511_hdmi_audio_prepare NULL
 #endif /* CONFIG_DRM_I2C_ADV7511_AUDIO */
 
 #endif /* __DRM_I2C_ADV7511_H__ */
diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511_audio.c b/drivers/gpu/drm/bridge/adv7511/adv7511_audio.c
index 1ff8c815ec798445ec771f549eca8a06a99ff64d..915c3b96721626c6af5d454c0bf7f53e37ff25af 100644
--- a/drivers/gpu/drm/bridge/adv7511/adv7511_audio.c
+++ b/drivers/gpu/drm/bridge/adv7511/adv7511_audio.c
@@ -55,11 +55,12 @@ static int adv7511_update_cts_n(struct adv7511 *adv7511)
 	return 0;
 }
 
-static int adv7511_hdmi_hw_params(struct device *dev, void *data,
-				  struct hdmi_codec_daifmt *fmt,
-				  struct hdmi_codec_params *hparms)
+int adv7511_hdmi_audio_prepare(struct drm_connector *connector,
+			       struct drm_bridge *bridge,
+			       struct hdmi_codec_daifmt *fmt,
+			       struct hdmi_codec_params *hparms)
 {
-	struct adv7511 *adv7511 = dev_get_drvdata(dev);
+	struct adv7511 *adv7511 = bridge_to_adv7511(bridge);
 	unsigned int audio_source, i2s_format = 0;
 	unsigned int invert_clock;
 	unsigned int rate;
@@ -167,9 +168,10 @@ static int adv7511_hdmi_hw_params(struct device *dev, void *data,
 	return 0;
 }
 
-static int audio_startup(struct device *dev, void *data)
+int adv7511_hdmi_audio_startup(struct drm_connector *connector,
+			       struct drm_bridge *bridge)
 {
-	struct adv7511 *adv7511 = dev_get_drvdata(dev);
+	struct adv7511 *adv7511 = bridge_to_adv7511(bridge);
 
 	regmap_update_bits(adv7511->regmap, ADV7511_REG_AUDIO_CONFIG,
 				BIT(7), 0);
@@ -204,69 +206,12 @@ static int audio_startup(struct device *dev, void *data)
 	return 0;
 }
 
-static void audio_shutdown(struct device *dev, void *data)
+void adv7511_hdmi_audio_shutdown(struct drm_connector *connector,
+				 struct drm_bridge *bridge)
 {
-	struct adv7511 *adv7511 = dev_get_drvdata(dev);
+	struct adv7511 *adv7511 = bridge_to_adv7511(bridge);
 
 	if (adv7511->audio_source == ADV7511_AUDIO_SOURCE_SPDIF)
 		regmap_update_bits(adv7511->regmap, ADV7511_REG_AUDIO_CONFIG,
 				   BIT(7), 0);
 }
-
-static int adv7511_hdmi_i2s_get_dai_id(struct snd_soc_component *component,
-					struct device_node *endpoint,
-					void *data)
-{
-	struct of_endpoint of_ep;
-	int ret;
-
-	ret = of_graph_parse_endpoint(endpoint, &of_ep);
-	if (ret < 0)
-		return ret;
-
-	/*
-	 * HDMI sound should be located as reg = <2>
-	 * Then, it is sound port 0
-	 */
-	if (of_ep.port == 2)
-		return 0;
-
-	return -EINVAL;
-}
-
-static const struct hdmi_codec_ops adv7511_codec_ops = {
-	.hw_params	= adv7511_hdmi_hw_params,
-	.audio_shutdown = audio_shutdown,
-	.audio_startup	= audio_startup,
-	.get_dai_id	= adv7511_hdmi_i2s_get_dai_id,
-};
-
-static const struct hdmi_codec_pdata codec_data = {
-	.ops = &adv7511_codec_ops,
-	.i2s_formats = (SNDRV_PCM_FMTBIT_S16_LE | SNDRV_PCM_FMTBIT_S20_3LE |
-			SNDRV_PCM_FMTBIT_S24_3LE | SNDRV_PCM_FMTBIT_S24_LE |
-			SNDRV_PCM_FMTBIT_IEC958_SUBFRAME_LE),
-	.max_i2s_channels = 2,
-	.i2s = 1,
-	.no_i2s_capture = 1,
-	.spdif = 1,
-	.no_spdif_capture = 1,
-};
-
-int adv7511_audio_init(struct device *dev, struct adv7511 *adv7511)
-{
-	adv7511->audio_pdev = platform_device_register_data(dev,
-					HDMI_CODEC_DRV_NAME,
-					PLATFORM_DEVID_AUTO,
-					&codec_data,
-					sizeof(codec_data));
-	return PTR_ERR_OR_ZERO(adv7511->audio_pdev);
-}
-
-void adv7511_audio_exit(struct adv7511 *adv7511)
-{
-	if (adv7511->audio_pdev) {
-		platform_device_unregister(adv7511->audio_pdev);
-		adv7511->audio_pdev = NULL;
-	}
-}
diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511_cec.c b/drivers/gpu/drm/bridge/adv7511/adv7511_cec.c
index 2e9c88a2b5ed44ef0cd417c553ea7873d00e4a14..822265426f58a6887941522e86cddac9d0151371 100644
--- a/drivers/gpu/drm/bridge/adv7511/adv7511_cec.c
+++ b/drivers/gpu/drm/bridge/adv7511/adv7511_cec.c
@@ -12,6 +12,8 @@
 
 #include <media/cec.h>
 
+#include <drm/display/drm_hdmi_cec_helper.h>
+
 #include "adv7511.h"
 
 static const u8 ADV7511_REG_CEC_RX_FRAME_HDR[] = {
@@ -44,8 +46,8 @@ static void adv_cec_tx_raw_status(struct adv7511 *adv7511, u8 tx_raw_status)
 		return;
 
 	if (tx_raw_status & ADV7511_INT1_CEC_TX_ARBIT_LOST) {
-		cec_transmit_attempt_done(adv7511->cec_adap,
-					  CEC_TX_STATUS_ARB_LOST);
+		drm_connector_hdmi_cec_transmit_attempt_done(adv7511->cec_connector,
+							     CEC_TX_STATUS_ARB_LOST);
 		return;
 	}
 	if (tx_raw_status & ADV7511_INT1_CEC_TX_RETRY_TIMEOUT) {
@@ -72,12 +74,14 @@ static void adv_cec_tx_raw_status(struct adv7511 *adv7511, u8 tx_raw_status)
 			if (low_drive_cnt)
 				status |= CEC_TX_STATUS_LOW_DRIVE;
 		}
-		cec_transmit_done(adv7511->cec_adap, status,
-				  0, nack_cnt, low_drive_cnt, err_cnt);
+		drm_connector_hdmi_cec_transmit_done(adv7511->cec_connector, status,
+						     0, nack_cnt, low_drive_cnt,
+						     err_cnt);
 		return;
 	}
 	if (tx_raw_status & ADV7511_INT1_CEC_TX_READY) {
-		cec_transmit_attempt_done(adv7511->cec_adap, CEC_TX_STATUS_OK);
+		drm_connector_hdmi_cec_transmit_attempt_done(adv7511->cec_connector,
+							     CEC_TX_STATUS_OK);
 		return;
 	}
 }
@@ -116,7 +120,7 @@ static void adv7511_cec_rx(struct adv7511 *adv7511, int rx_buf)
 	regmap_update_bits(adv7511->regmap_cec,
 			   ADV7511_REG_CEC_RX_BUFFERS + offset, BIT(rx_buf), 0);
 
-	cec_received_msg(adv7511->cec_adap, &msg);
+	drm_connector_hdmi_cec_received_msg(adv7511->cec_connector, &msg);
 }
 
 int adv7511_cec_irq_process(struct adv7511 *adv7511, unsigned int irq1)
@@ -179,9 +183,9 @@ int adv7511_cec_irq_process(struct adv7511 *adv7511, unsigned int irq1)
 	return IRQ_HANDLED;
 }
 
-static int adv7511_cec_adap_enable(struct cec_adapter *adap, bool enable)
+int adv7511_cec_enable(struct drm_bridge *bridge, bool enable)
 {
-	struct adv7511 *adv7511 = cec_get_drvdata(adap);
+	struct adv7511 *adv7511 = bridge_to_adv7511(bridge);
 	unsigned int offset = adv7511->info->reg_cec_offset;
 
 	if (adv7511->i2c_cec == NULL)
@@ -225,9 +229,9 @@ static int adv7511_cec_adap_enable(struct cec_adapter *adap, bool enable)
 	return 0;
 }
 
-static int adv7511_cec_adap_log_addr(struct cec_adapter *adap, u8 addr)
+int adv7511_cec_log_addr(struct drm_bridge *bridge, u8 addr)
 {
-	struct adv7511 *adv7511 = cec_get_drvdata(adap);
+	struct adv7511 *adv7511 = bridge_to_adv7511(bridge);
 	unsigned int offset = adv7511->info->reg_cec_offset;
 	unsigned int i, free_idx = ADV7511_MAX_ADDRS;
 
@@ -293,10 +297,10 @@ static int adv7511_cec_adap_log_addr(struct cec_adapter *adap, u8 addr)
 	return 0;
 }
 
-static int adv7511_cec_adap_transmit(struct cec_adapter *adap, u8 attempts,
-				     u32 signal_free_time, struct cec_msg *msg)
+int adv7511_cec_transmit(struct drm_bridge *bridge, u8 attempts,
+			 u32 signal_free_time, struct cec_msg *msg)
 {
-	struct adv7511 *adv7511 = cec_get_drvdata(adap);
+	struct adv7511 *adv7511 = bridge_to_adv7511(bridge);
 	unsigned int offset = adv7511->info->reg_cec_offset;
 	u8 len = msg->len;
 	unsigned int i;
@@ -328,12 +332,6 @@ static int adv7511_cec_adap_transmit(struct cec_adapter *adap, u8 attempts,
 	return 0;
 }
 
-static const struct cec_adap_ops adv7511_cec_adap_ops = {
-	.adap_enable = adv7511_cec_adap_enable,
-	.adap_log_addr = adv7511_cec_adap_log_addr,
-	.adap_transmit = adv7511_cec_adap_transmit,
-};
-
 static int adv7511_cec_parse_dt(struct device *dev, struct adv7511 *adv7511)
 {
 	adv7511->cec_clk = devm_clk_get(dev, "cec");
@@ -348,20 +346,18 @@ static int adv7511_cec_parse_dt(struct device *dev, struct adv7511 *adv7511)
 	return 0;
 }
 
-int adv7511_cec_init(struct device *dev, struct adv7511 *adv7511)
+int adv7511_cec_init(struct drm_connector *connector,
+		     struct drm_bridge *bridge)
 {
+	struct adv7511 *adv7511 = bridge_to_adv7511(bridge);
+	struct device *dev = &adv7511->i2c_main->dev;
 	unsigned int offset = adv7511->info->reg_cec_offset;
 	int ret = adv7511_cec_parse_dt(dev, adv7511);
 
 	if (ret)
 		goto err_cec_parse_dt;
 
-	adv7511->cec_adap = cec_allocate_adapter(&adv7511_cec_adap_ops,
-		adv7511, dev_name(dev), CEC_CAP_DEFAULTS, ADV7511_MAX_ADDRS);
-	if (IS_ERR(adv7511->cec_adap)) {
-		ret = PTR_ERR(adv7511->cec_adap);
-		goto err_cec_alloc;
-	}
+	adv7511->cec_connector = connector;
 
 	regmap_write(adv7511->regmap, ADV7511_REG_CEC_CTRL, 0);
 	/* cec soft reset */
@@ -378,17 +374,8 @@ int adv7511_cec_init(struct device *dev, struct adv7511 *adv7511)
 		     ADV7511_REG_CEC_CLK_DIV + offset,
 		     ((adv7511->cec_clk_freq / 750000) - 1) << 2);
 
-	ret = cec_register_adapter(adv7511->cec_adap, dev);
-	if (ret)
-		goto err_cec_register;
 	return 0;
 
-err_cec_register:
-	cec_delete_adapter(adv7511->cec_adap);
-	adv7511->cec_adap = NULL;
-err_cec_alloc:
-	dev_info(dev, "Initializing CEC failed with error %d, disabling CEC\n",
-		 ret);
 err_cec_parse_dt:
 	regmap_write(adv7511->regmap, ADV7511_REG_CEC_CTRL,
 		     ADV7511_CEC_CTRL_POWER_DOWN);
diff --git a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
index 1257009e850c1b20184cfaea5b6a4440e75e10d7..8b7548448615f84db796467ffd70fe11554bb681 100644
--- a/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
+++ b/drivers/gpu/drm/bridge/adv7511/adv7511_drv.c
@@ -12,14 +12,17 @@
 #include <linux/of.h>
 #include <linux/slab.h>
 
-#include <media/cec.h>
+#include <sound/pcm.h>
 
 #include <drm/drm_atomic.h>
 #include <drm/drm_atomic_helper.h>
+#include <drm/drm_bridge_connector.h>
 #include <drm/drm_edid.h>
 #include <drm/drm_of.h>
 #include <drm/drm_print.h>
 #include <drm/drm_probe_helper.h>
+#include <drm/display/drm_hdmi_helper.h>
+#include <drm/display/drm_hdmi_state_helper.h>
 
 #include "adv7511.h"
 
@@ -203,62 +206,37 @@ static const uint16_t adv7511_csc_ycbcr_to_rgb[] = {
 
 static void adv7511_set_config_csc(struct adv7511 *adv7511,
 				   struct drm_connector *connector,
-				   bool rgb, bool hdmi_mode)
+				   bool rgb)
 {
 	struct adv7511_video_config config;
 	bool output_format_422, output_format_ycbcr;
 	unsigned int mode;
-	uint8_t infoframe[17];
-
-	config.hdmi_mode = hdmi_mode;
-
-	hdmi_avi_infoframe_init(&config.avi_infoframe);
-
-	config.avi_infoframe.scan_mode = HDMI_SCAN_MODE_UNDERSCAN;
 
 	if (rgb) {
 		config.csc_enable = false;
-		config.avi_infoframe.colorspace = HDMI_COLORSPACE_RGB;
+		output_format_422 = false;
+		output_format_ycbcr = false;
 	} else {
 		config.csc_scaling_factor = ADV7511_CSC_SCALING_4;
 		config.csc_coefficents = adv7511_csc_ycbcr_to_rgb;
 
 		if ((connector->display_info.color_formats &
 		     DRM_COLOR_FORMAT_YCBCR422) &&
-		    config.hdmi_mode) {
+		    connector->display_info.is_hdmi) {
 			config.csc_enable = false;
-			config.avi_infoframe.colorspace =
-				HDMI_COLORSPACE_YUV422;
-		} else {
-			config.csc_enable = true;
-			config.avi_infoframe.colorspace = HDMI_COLORSPACE_RGB;
-		}
-	}
-
-	if (config.hdmi_mode) {
-		mode = ADV7511_HDMI_CFG_MODE_HDMI;
-
-		switch (config.avi_infoframe.colorspace) {
-		case HDMI_COLORSPACE_YUV444:
-			output_format_422 = false;
-			output_format_ycbcr = true;
-			break;
-		case HDMI_COLORSPACE_YUV422:
 			output_format_422 = true;
 			output_format_ycbcr = true;
-			break;
-		default:
+		} else {
+			config.csc_enable = true;
 			output_format_422 = false;
 			output_format_ycbcr = false;
-			break;
 		}
-	} else {
-		mode = ADV7511_HDMI_CFG_MODE_DVI;
-		output_format_422 = false;
-		output_format_ycbcr = false;
 	}
 
-	adv7511_packet_disable(adv7511, ADV7511_PACKET_ENABLE_AVI_INFOFRAME);
+	if (connector->display_info.is_hdmi)
+		mode = ADV7511_HDMI_CFG_MODE_HDMI;
+	else
+		mode = ADV7511_HDMI_CFG_MODE_DVI;
 
 	adv7511_set_colormap(adv7511, config.csc_enable,
 			     config.csc_coefficents,
@@ -269,15 +247,6 @@ static void adv7511_set_config_csc(struct adv7511 *adv7511,
 
 	regmap_update_bits(adv7511->regmap, ADV7511_REG_HDCP_HDMI_CFG,
 			   ADV7511_HDMI_CFG_MODE_MASK, mode);
-
-	hdmi_avi_infoframe_pack(&config.avi_infoframe, infoframe,
-				sizeof(infoframe));
-
-	/* The AVI infoframe id is not configurable */
-	regmap_bulk_write(adv7511->regmap, ADV7511_REG_AVI_INFOFRAME_VERSION,
-			  infoframe + 1, sizeof(infoframe) - 1);
-
-	adv7511_packet_enable(adv7511, ADV7511_PACKET_ENABLE_AVI_INFOFRAME);
 }
 
 static void adv7511_set_link_config(struct adv7511 *adv7511,
@@ -446,22 +415,16 @@ static void adv7511_hpd_work(struct work_struct *work)
 	 * restore its state.
 	 */
 	if (status == connector_status_connected &&
-	    adv7511->connector.status == connector_status_disconnected &&
+	    adv7511->status == connector_status_disconnected &&
 	    adv7511->powered) {
 		regcache_mark_dirty(adv7511->regmap);
 		adv7511_power_on(adv7511);
 	}
 
-	if (adv7511->connector.status != status) {
-		adv7511->connector.status = status;
+	if (adv7511->status != status) {
+		adv7511->status = status;
 
-		if (adv7511->connector.dev) {
-			if (status == connector_status_disconnected)
-				cec_phys_addr_invalidate(adv7511->cec_adap);
-			drm_kms_helper_hotplug_event(adv7511->connector.dev);
-		} else {
-			drm_bridge_hpd_notify(&adv7511->bridge, status);
-		}
+		drm_bridge_hpd_notify(&adv7511->bridge, status);
 	}
 }
 
@@ -636,45 +599,11 @@ static const struct drm_edid *adv7511_edid_read(struct adv7511 *adv7511,
 	if (!adv7511->powered)
 		__adv7511_power_off(adv7511);
 
-	if (drm_edid) {
-		/*
-		 * FIXME: The CEC physical address should be set using
-		 * cec_s_phys_addr(adap,
-		 * connector->display_info.source_physical_address, false) from
-		 * a path that has read the EDID and called
-		 * drm_edid_connector_update().
-		 */
-		const struct edid *edid = drm_edid_raw(drm_edid);
-
-		adv7511_set_config_csc(adv7511, connector, adv7511->rgb,
-				       drm_detect_hdmi_monitor(edid));
-
-		cec_s_phys_addr_from_edid(adv7511->cec_adap, edid);
-	} else {
-		cec_s_phys_addr_from_edid(adv7511->cec_adap, NULL);
-	}
-
 	return drm_edid;
 }
 
-static int adv7511_get_modes(struct adv7511 *adv7511,
-			     struct drm_connector *connector)
-{
-	const struct drm_edid *drm_edid;
-	unsigned int count;
-
-	drm_edid = adv7511_edid_read(adv7511, connector);
-
-	drm_edid_connector_update(connector, drm_edid);
-	count = drm_edid_connector_add_modes(connector);
-
-	drm_edid_free(drm_edid);
-
-	return count;
-}
-
 static enum drm_connector_status
-adv7511_detect(struct adv7511 *adv7511, struct drm_connector *connector)
+adv7511_detect(struct adv7511 *adv7511)
 {
 	enum drm_connector_status status;
 	unsigned int val;
@@ -699,8 +628,6 @@ adv7511_detect(struct adv7511 *adv7511, struct drm_connector *connector)
 	if (status == connector_status_connected && hpd && adv7511->powered) {
 		regcache_mark_dirty(adv7511->regmap);
 		adv7511_power_on(adv7511);
-		if (connector)
-			adv7511_get_modes(adv7511, connector);
 		if (adv7511->status == connector_status_connected)
 			status = connector_status_disconnected;
 	} else {
@@ -719,17 +646,7 @@ adv7511_detect(struct adv7511 *adv7511, struct drm_connector *connector)
 	return status;
 }
 
-static enum drm_mode_status adv7511_mode_valid(struct adv7511 *adv7511,
-			      const struct drm_display_mode *mode)
-{
-	if (mode->clock > 165000)
-		return MODE_CLOCK_HIGH;
-
-	return MODE_OK;
-}
-
 static void adv7511_mode_set(struct adv7511 *adv7511,
-			     const struct drm_display_mode *mode,
 			     const struct drm_display_mode *adj_mode)
 {
 	unsigned int low_refresh_rate;
@@ -800,11 +717,11 @@ static void adv7511_mode_set(struct adv7511 *adv7511,
 			vsync_polarity = 1;
 	}
 
-	if (drm_mode_vrefresh(mode) <= 24)
+	if (drm_mode_vrefresh(adj_mode) <= 24)
 		low_refresh_rate = ADV7511_LOW_REFRESH_RATE_24HZ;
-	else if (drm_mode_vrefresh(mode) <= 25)
+	else if (drm_mode_vrefresh(adj_mode) <= 25)
 		low_refresh_rate = ADV7511_LOW_REFRESH_RATE_25HZ;
-	else if (drm_mode_vrefresh(mode) <= 30)
+	else if (drm_mode_vrefresh(adj_mode) <= 30)
 		low_refresh_rate = ADV7511_LOW_REFRESH_RATE_30HZ;
 	else
 		low_refresh_rate = ADV7511_LOW_REFRESH_RATE_NONE;
@@ -826,77 +743,21 @@ static void adv7511_mode_set(struct adv7511 *adv7511,
 	 * supposed to give better results.
 	 */
 
-	adv7511->f_tmds = mode->clock;
-}
-
-/* -----------------------------------------------------------------------------
- * DRM Connector Operations
- */
-
-static struct adv7511 *connector_to_adv7511(struct drm_connector *connector)
-{
-	return container_of(connector, struct adv7511, connector);
+	adv7511->f_tmds = adj_mode->clock;
 }
 
-static int adv7511_connector_get_modes(struct drm_connector *connector)
-{
-	struct adv7511 *adv = connector_to_adv7511(connector);
-
-	return adv7511_get_modes(adv, connector);
-}
-
-static enum drm_mode_status
-adv7511_connector_mode_valid(struct drm_connector *connector,
-			     const struct drm_display_mode *mode)
-{
-	struct adv7511 *adv = connector_to_adv7511(connector);
-
-	return adv7511_mode_valid(adv, mode);
-}
-
-static struct drm_connector_helper_funcs adv7511_connector_helper_funcs = {
-	.get_modes = adv7511_connector_get_modes,
-	.mode_valid = adv7511_connector_mode_valid,
-};
-
-static enum drm_connector_status
-adv7511_connector_detect(struct drm_connector *connector, bool force)
-{
-	struct adv7511 *adv = connector_to_adv7511(connector);
-
-	return adv7511_detect(adv, connector);
-}
-
-static const struct drm_connector_funcs adv7511_connector_funcs = {
-	.fill_modes = drm_helper_probe_single_connector_modes,
-	.detect = adv7511_connector_detect,
-	.destroy = drm_connector_cleanup,
-	.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 int adv7511_connector_init(struct adv7511 *adv)
 {
 	struct drm_bridge *bridge = &adv->bridge;
-	int ret;
-
-	if (adv->i2c_main->irq)
-		adv->connector.polled = DRM_CONNECTOR_POLL_HPD;
-	else
-		adv->connector.polled = DRM_CONNECTOR_POLL_CONNECT |
-				DRM_CONNECTOR_POLL_DISCONNECT;
+	struct drm_connector *connector;
 
-	ret = drm_connector_init(bridge->dev, &adv->connector,
-				 &adv7511_connector_funcs,
-				 DRM_MODE_CONNECTOR_HDMIA);
-	if (ret < 0) {
+	connector = drm_bridge_connector_init(bridge->dev, bridge->encoder);
+	if (IS_ERR(connector)) {
 		DRM_ERROR("Failed to initialize connector with drm\n");
-		return ret;
+		return PTR_ERR(connector);
 	}
-	drm_connector_helper_add(&adv->connector,
-				 &adv7511_connector_helper_funcs);
-	drm_connector_attach_encoder(&adv->connector, bridge->encoder);
+
+	drm_connector_attach_encoder(connector, bridge->encoder);
 
 	return 0;
 }
@@ -905,7 +766,7 @@ static int adv7511_connector_init(struct adv7511 *adv)
  * DRM Bridge Operations
  */
 
-static struct adv7511 *bridge_to_adv7511(struct drm_bridge *bridge)
+static const struct adv7511 *bridge_to_adv7511_const(const struct drm_bridge *bridge)
 {
 	return container_of(bridge, struct adv7511, bridge);
 }
@@ -914,8 +775,29 @@ static void adv7511_bridge_atomic_enable(struct drm_bridge *bridge,
 					 struct drm_atomic_state *state)
 {
 	struct adv7511 *adv = bridge_to_adv7511(bridge);
+	struct drm_connector *connector;
+	struct drm_connector_state *conn_state;
+	struct drm_crtc_state *crtc_state;
 
 	adv7511_power_on(adv);
+
+	connector = drm_atomic_get_new_connector_for_encoder(state, bridge->encoder);
+	if (WARN_ON(!connector))
+		return;
+
+	conn_state = drm_atomic_get_new_connector_state(state, connector);
+	if (WARN_ON(!conn_state))
+		return;
+
+	crtc_state = drm_atomic_get_new_crtc_state(state, conn_state->crtc);
+	if (WARN_ON(!crtc_state))
+		return;
+
+	adv7511_set_config_csc(adv, connector, adv->rgb);
+
+	adv7511_mode_set(adv, &crtc_state->adjusted_mode);
+
+	drm_atomic_helper_connector_hdmi_update_infoframes(connector, state);
 }
 
 static void adv7511_bridge_atomic_disable(struct drm_bridge *bridge,
@@ -926,13 +808,17 @@ static void adv7511_bridge_atomic_disable(struct drm_bridge *bridge,
 	adv7511_power_off(adv);
 }
 
-static void adv7511_bridge_mode_set(struct drm_bridge *bridge,
-				    const struct drm_display_mode *mode,
-				    const struct drm_display_mode *adj_mode)
+static enum drm_mode_status
+adv7511_bridge_hdmi_tmds_char_rate_valid(const struct drm_bridge *bridge,
+					 const struct drm_display_mode *mode,
+					 unsigned long long tmds_rate)
 {
-	struct adv7511 *adv = bridge_to_adv7511(bridge);
+	const struct adv7511 *adv = bridge_to_adv7511_const(bridge);
 
-	adv7511_mode_set(adv, mode, adj_mode);
+	if (tmds_rate > 1000ULL * adv->info->max_mode_clock_khz)
+		return MODE_CLOCK_HIGH;
+
+	return MODE_OK;
 }
 
 static enum drm_mode_status adv7511_bridge_mode_valid(struct drm_bridge *bridge,
@@ -941,10 +827,10 @@ static enum drm_mode_status adv7511_bridge_mode_valid(struct drm_bridge *bridge,
 {
 	struct adv7511 *adv = bridge_to_adv7511(bridge);
 
-	if (adv->info->has_dsi)
-		return adv7533_mode_valid(adv, mode);
-	else
-		return adv7511_mode_valid(adv, mode);
+	if (!adv->info->has_dsi)
+		return MODE_OK;
+
+	return adv7533_mode_valid(adv, mode);
 }
 
 static int adv7511_bridge_attach(struct drm_bridge *bridge,
@@ -978,7 +864,7 @@ static enum drm_connector_status adv7511_bridge_detect(struct drm_bridge *bridge
 {
 	struct adv7511 *adv = bridge_to_adv7511(bridge);
 
-	return adv7511_detect(adv, NULL);
+	return adv7511_detect(adv);
 }
 
 static const struct drm_edid *adv7511_bridge_edid_read(struct drm_bridge *bridge,
@@ -989,28 +875,71 @@ static const struct drm_edid *adv7511_bridge_edid_read(struct drm_bridge *bridge
 	return adv7511_edid_read(adv, connector);
 }
 
-static void adv7511_bridge_hpd_notify(struct drm_bridge *bridge,
-				      enum drm_connector_status status)
+static int adv7511_bridge_hdmi_clear_infoframe(struct drm_bridge *bridge,
+					       enum hdmi_infoframe_type type)
 {
-	struct adv7511 *adv = bridge_to_adv7511(bridge);
+	struct adv7511 *adv7511 = bridge_to_adv7511(bridge);
+
+	switch (type) {
+	case HDMI_INFOFRAME_TYPE_AVI:
+		adv7511_packet_disable(adv7511, ADV7511_PACKET_ENABLE_AVI_INFOFRAME);
+		break;
+	default:
+		drm_dbg_driver(adv7511->bridge.dev, "Unsupported HDMI InfoFrame %x\n", type);
+		break;
+	}
+
+	return 0;
+}
+
+static int adv7511_bridge_hdmi_write_infoframe(struct drm_bridge *bridge,
+					       enum hdmi_infoframe_type type,
+					       const u8 *buffer, size_t len)
+{
+	struct adv7511 *adv7511 = bridge_to_adv7511(bridge);
+
+	adv7511_bridge_hdmi_clear_infoframe(bridge, type);
+
+	switch (type) {
+	case HDMI_INFOFRAME_TYPE_AVI:
+		/* The AVI infoframe id is not configurable */
+		regmap_bulk_write(adv7511->regmap, ADV7511_REG_AVI_INFOFRAME_VERSION,
+				  buffer + 1, len - 1);
 
-	if (status == connector_status_disconnected)
-		cec_phys_addr_invalidate(adv->cec_adap);
+		adv7511_packet_enable(adv7511, ADV7511_PACKET_ENABLE_AVI_INFOFRAME);
+		break;
+	default:
+		drm_dbg_driver(adv7511->bridge.dev, "Unsupported HDMI InfoFrame %x\n", type);
+		break;
+	}
+
+	return 0;
 }
 
 static const struct drm_bridge_funcs adv7511_bridge_funcs = {
-	.mode_set = adv7511_bridge_mode_set,
 	.mode_valid = adv7511_bridge_mode_valid,
 	.attach = adv7511_bridge_attach,
 	.detect = adv7511_bridge_detect,
 	.edid_read = adv7511_bridge_edid_read,
-	.hpd_notify = adv7511_bridge_hpd_notify,
 
 	.atomic_enable = adv7511_bridge_atomic_enable,
 	.atomic_disable = adv7511_bridge_atomic_disable,
 	.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,
+
+	.hdmi_tmds_char_rate_valid = adv7511_bridge_hdmi_tmds_char_rate_valid,
+	.hdmi_clear_infoframe = adv7511_bridge_hdmi_clear_infoframe,
+	.hdmi_write_infoframe = adv7511_bridge_hdmi_write_infoframe,
+
+	.hdmi_audio_startup = adv7511_hdmi_audio_startup,
+	.hdmi_audio_prepare = adv7511_hdmi_audio_prepare,
+	.hdmi_audio_shutdown = adv7511_hdmi_audio_shutdown,
+
+	.hdmi_cec_init = adv7511_cec_init,
+	.hdmi_cec_enable = adv7511_cec_enable,
+	.hdmi_cec_log_addr = adv7511_cec_log_addr,
+	.hdmi_cec_transmit = adv7511_cec_transmit,
 };
 
 /* -----------------------------------------------------------------------------
@@ -1323,22 +1252,44 @@ static int adv7511_probe(struct i2c_client *i2c)
 	if (adv7511->info->link_config)
 		adv7511_set_link_config(adv7511, &link_config);
 
-	ret = adv7511_cec_init(dev, adv7511);
-	if (ret)
-		goto err_unregister_cec;
+	regmap_write(adv7511->regmap, ADV7511_REG_CEC_CTRL,
+		     ADV7511_CEC_CTRL_POWER_DOWN);
 
 	adv7511->bridge.funcs = &adv7511_bridge_funcs;
-	adv7511->bridge.ops = DRM_BRIDGE_OP_DETECT | DRM_BRIDGE_OP_EDID;
+	adv7511->bridge.ops = DRM_BRIDGE_OP_DETECT |
+		DRM_BRIDGE_OP_EDID |
+		DRM_BRIDGE_OP_HDMI |
+		DRM_BRIDGE_OP_HDMI_AUDIO |
+		DRM_BRIDGE_OP_HDMI_CEC_ADAPTER;
 	if (adv7511->i2c_main->irq)
 		adv7511->bridge.ops |= DRM_BRIDGE_OP_HPD;
 
+	adv7511->bridge.vendor = "Analog";
+	adv7511->bridge.product = adv7511->info->name;
+
+#ifdef CONFIG_DRM_I2C_ADV7511_AUDIO
+	adv7511->bridge.hdmi_audio_dev = dev;
+	adv7511->bridge.hdmi_audio_max_i2s_playback_channels = 2;
+	adv7511->bridge.hdmi_audio_i2s_formats = (SNDRV_PCM_FMTBIT_S16_LE |
+						  SNDRV_PCM_FMTBIT_S20_3LE |
+						  SNDRV_PCM_FMTBIT_S24_3LE |
+						  SNDRV_PCM_FMTBIT_S24_LE |
+						  SNDRV_PCM_FMTBIT_IEC958_SUBFRAME_LE),
+	adv7511->bridge.hdmi_audio_spdif_playback = 1;
+	adv7511->bridge.hdmi_audio_dai_port = 2;
+#endif
+
+#ifdef CONFIG_DRM_I2C_ADV7511_CEC
+	adv7511->bridge.hdmi_cec_dev = dev;
+	adv7511->bridge.hdmi_cec_adapter_name = dev_name(dev);
+	adv7511->bridge.hdmi_cec_available_las = ADV7511_MAX_ADDRS;
+#endif
+
 	adv7511->bridge.of_node = dev->of_node;
 	adv7511->bridge.type = DRM_MODE_CONNECTOR_HDMIA;
 
 	drm_bridge_add(&adv7511->bridge);
 
-	adv7511_audio_init(dev, adv7511);
-
 	if (i2c->irq) {
 		init_waitqueue_head(&adv7511->wq);
 
@@ -1360,10 +1311,7 @@ static int adv7511_probe(struct i2c_client *i2c)
 	return 0;
 
 err_unregister_audio:
-	adv7511_audio_exit(adv7511);
 	drm_bridge_remove(&adv7511->bridge);
-err_unregister_cec:
-	cec_unregister_adapter(adv7511->cec_adap);
 	i2c_unregister_device(adv7511->i2c_cec);
 	clk_disable_unprepare(adv7511->cec_clk);
 err_i2c_unregister_packet:
@@ -1388,9 +1336,6 @@ static void adv7511_remove(struct i2c_client *i2c)
 
 	drm_bridge_remove(&adv7511->bridge);
 
-	adv7511_audio_exit(adv7511);
-
-	cec_unregister_adapter(adv7511->cec_adap);
 	i2c_unregister_device(adv7511->i2c_cec);
 	clk_disable_unprepare(adv7511->cec_clk);
 
@@ -1400,6 +1345,8 @@ static void adv7511_remove(struct i2c_client *i2c)
 
 static const struct adv7511_chip_info adv7511_chip_info = {
 	.type = ADV7511,
+	.name = "ADV7511",
+	.max_mode_clock_khz = 165000,
 	.supply_names = adv7511_supply_names,
 	.num_supplies = ARRAY_SIZE(adv7511_supply_names),
 	.link_config = true,
@@ -1407,6 +1354,7 @@ static const struct adv7511_chip_info adv7511_chip_info = {
 
 static const struct adv7511_chip_info adv7533_chip_info = {
 	.type = ADV7533,
+	.name = "ADV7533",
 	.max_mode_clock_khz = 80000,
 	.max_lane_freq_khz = 800000,
 	.supply_names = adv7533_supply_names,
@@ -1417,6 +1365,7 @@ static const struct adv7511_chip_info adv7533_chip_info = {
 
 static const struct adv7511_chip_info adv7535_chip_info = {
 	.type = ADV7535,
+	.name = "ADV7535",
 	.max_mode_clock_khz = 148500,
 	.max_lane_freq_khz = 891000,
 	.supply_names = adv7533_supply_names,
diff --git a/drivers/gpu/drm/bridge/adv7511/adv7533.c b/drivers/gpu/drm/bridge/adv7511/adv7533.c
index 122ad91e8a3293de1839cad061cd858d8046b675..b12d422343fc139e8d9b59a2ded60ce08ce43dc8 100644
--- a/drivers/gpu/drm/bridge/adv7511/adv7533.c
+++ b/drivers/gpu/drm/bridge/adv7511/adv7533.c
@@ -106,10 +106,6 @@ enum drm_mode_status adv7533_mode_valid(struct adv7511 *adv,
 	struct mipi_dsi_device *dsi = adv->dsi;
 	u8 bpp = mipi_dsi_pixel_format_to_bpp(dsi->format);
 
-	/* Check max clock for either 7533 or 7535 */
-	if (mode->clock > adv->info->max_mode_clock_khz)
-		return MODE_CLOCK_HIGH;
-
 	/* Check max clock for each lane */
 	if (mode->clock * bpp > adv->info->max_lane_freq_khz * adv->num_dsi_lanes)
 		return MODE_CLOCK_HIGH;

-- 
2.39.5

Re: [PATCH v5 05/11] drm/display: move CEC_CORE selection to DRM_DISPLAY_HELPER

[Maxime Ripard]

On Mon, 7 Apr 2025 18:11:02 +0300, Dmitry Baryshkov wrote:
> THe Kconfig symbol DRM_DISPLAY_DP_AUX_CEC is a boolean which simply
> toggles whether DP_AUX_CEC support should be built into the
> drm_display_helper (which can be eithera module or built-in into the
> kernel). If DRM_DISPLAY_DP_AUX_CEC is selected, then CEC_CORE is
> selected to be built-in into the kernel even if DRM_DISPLAY_HELPER is
> 
> [ ... ]

Reviewed-by: Maxime Ripard <mripard@kernel.org>

Thanks!
Maxime

Re: [PATCH v5 08/11] drm/vc4: hdmi: switch to generic CEC helpers

[Maxime Ripard]

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

On Mon, Apr 07, 2025 at 06:11:05PM +0300, Dmitry Baryshkov wrote:
> +static const struct drm_connector_hdmi_cec_adapter_ops vc4_hdmi_cec_adap_ops = {
> +	.base.unregister = drm_connector_hdmi_cec_unregister,
> +	.init = vc4_hdmi_cec_init,
> +	.enable = vc4_hdmi_cec_adap_enable,
> +	.log_addr = vc4_hdmi_cec_adap_log_addr,
> +	.transmit = vc4_hdmi_cec_adap_transmit,
> +};

Did you compile it? it looks like drm_connector_hdmi_cec_adapter_ops has been renamed, and it
doesn't have a base.unregister field anymore?

Maxime

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

Re: [PATCH v5 04/11] drm/connector: unregister CEC data

[Maxime Ripard]

On Mon, 7 Apr 2025 18:11:01 +0300, Dmitry Baryshkov wrote:
> In order to make sure that CEC adapters or notifiers are unregistered
> and CEC-related data is properly destroyed make drm_connector_cleanup()
> call CEC's unregister() callback.
> 
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> 
> [ ... ]

Reviewed-by: Maxime Ripard <mripard@kernel.org>

Thanks!
Maxime

Re: [PATCH v5 04/11] drm/connector: unregister CEC data

[Maxime Ripard]

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

Hi,

On Mon, Apr 07, 2025 at 06:11:01PM +0300, Dmitry Baryshkov wrote:
> In order to make sure that CEC adapters or notifiers are unregistered
> and CEC-related data is properly destroyed make drm_connector_cleanup()
> call CEC's unregister() callback.
> 
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
> ---
>  drivers/gpu/drm/drm_connector.c | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
> index ba08fbd973829e49ea977251c4f0fb6d96ade631..ae9c02ef9ab102db03c2824683ece37cfbcd3300 100644
> --- a/drivers/gpu/drm/drm_connector.c
> +++ b/drivers/gpu/drm/drm_connector.c
> @@ -743,6 +743,13 @@ void drm_connector_cec_phys_addr_set(struct drm_connector *connector)
>  }
>  EXPORT_SYMBOL(drm_connector_cec_phys_addr_set);
>  
> +static void drm_connector_cec_unregister(struct drm_connector *connector)
> +{
> +	if (connector->cec.funcs &&
> +	    connector->cec.funcs->unregister)
> +		connector->cec.funcs->unregister(connector);
> +}
> +
>  /**
>   * drm_connector_cleanup - cleans up an initialised connector
>   * @connector: connector to cleanup
> @@ -763,6 +770,8 @@ void drm_connector_cleanup(struct drm_connector *connector)
>  
>  	platform_device_unregister(connector->hdmi_audio.codec_pdev);
>  
> +	drm_connector_cec_unregister(connector);
> +

Actually, since we know that the HDMI connector is drm-managed, why
can't we make the call to connector->cec.funcs->unregister a drm-managed
action registered by drm_connector_hdmi_cec_register?

Maxime

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

Re: [PATCH v5 03/11] drm/connector: add CEC-related fields

[Maxime Ripard]

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

Hi,

On Mon, Apr 07, 2025 at 06:11:00PM +0300, Dmitry Baryshkov wrote:
> +/**
> + * struct drm_connector_cec - DRM Connector CEC-related structure
> + */
> +struct drm_connector_cec {
> +	/**
> +	 * @mutex: protects all fields in this structure.
> +	 */
> +	struct mutex mutex;
> +
> +	/**
> +	 * @funcs: CEC Control Functions
> +	 */
> +	const struct drm_connector_cec_funcs *funcs;
> +
> +	/**
> +	 * @data: CEC implementation-specific data
> +	 */
> +	void *data;

Is there a reason we don't just skip that data? The only user I'm seeing
so far are the helpers, and they only put the cec_adapter pointer in
there.

Can't we pass the connector to CEC and make the adapter part of drm_connector_cec?

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

Re: [PATCH v5 06/11] drm/display: add CEC helpers code

[Maxime Ripard]

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

On Mon, Apr 07, 2025 at 06:11:03PM +0300, Dmitry Baryshkov wrote:
> +static void drm_connector_hdmi_cec_adapter_unregister(struct drm_connector *connector)
> +{
> +	struct drm_connector_hdmi_cec_data *data = connector->cec.data;
> +
> +	cec_delete_adapter(data->adapter);
> +
> +	if (data->funcs->uninit)
> +		data->funcs->uninit(connector);
> +
> +	kfree(data);
> +	connector->cec.data = NULL;
> +}
>
> [...]
>
> +int drm_connector_hdmi_cec_register(struct drm_connector *connector,
> +				    const struct drm_connector_hdmi_cec_funcs *funcs,
> +				    const char *name,
> +				    u8 available_las,
> +				    struct device *dev)
> +{
> +	struct drm_connector_hdmi_cec_data *data;
> +	struct cec_connector_info conn_info;
> +	struct cec_adapter *cec_adap;
> +	int ret;
> +
> +	if (!funcs->init || !funcs->enable || !funcs->log_addr || !funcs->transmit)
> +		return -EINVAL;
> +
> +	data = kzalloc(sizeof(*data), GFP_KERNEL);
> +	if (!data)
> +		return -ENOMEM;
> +
> +	data->funcs = funcs;
> +
> +	cec_adap = cec_allocate_adapter(&drm_connector_hdmi_cec_adap_ops, connector, name,
> +					CEC_CAP_DEFAULTS | CEC_CAP_CONNECTOR_INFO,
> +					available_las ? : CEC_MAX_LOG_ADDRS);
> +	ret = PTR_ERR_OR_ZERO(cec_adap);
> +	if (ret < 0)
> +		goto err_free;
> +
> +	cec_fill_conn_info_from_drm(&conn_info, connector);
> +	cec_s_conn_info(cec_adap, &conn_info);
> +
> +	data->adapter = cec_adap;
> +
> +	mutex_lock(&connector->cec.mutex);
> +
> +	connector->cec.data = data;
> +	connector->cec.funcs = &drm_connector_hdmi_cec_adapter_funcs;
> +
> +	ret = funcs->init(connector);
> +	if (ret < 0)
> +		goto err_delete_adapter;
> +
> +	ret = cec_register_adapter(cec_adap, dev);
> +	if (ret < 0)
> +		goto err_delete_adapter;

I'm a bit concerned about the respective lifetimes of CEC adapters and
DRM connectors.

When you register the CEC adapter, its associated structure is
kzalloc'd, and freed when the DRM connector is freed (so when nobody has
any reference to it anymore: either when the device is torn down, or a
DP-MST hotplug scenario).

The CEC adapter however will only be freed when its own users will close
their file descriptor. So we can have a scenario when the CEC adapter is
still live but the DRM connector has been unregistered. Thus, the CEC
adapter data will have been kfree'd.

You might consider safe because $REASONS, but those need to be properly
detailed and explained.

That's another reason why I think that just putting the connector
pointer as data is better: connectors are refcounted, so we know those
aren't an issue.

Maxime>

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

Re: [PATCH v5 09/11] drm/display: bridge-connector: hook in CEC notifier support

[Maxime Ripard]

On Mon, 7 Apr 2025 18:11:06 +0300, Dmitry Baryshkov wrote:
> Allow HDMI DRM bridges to create CEC notifier. Physical address is
> handled automatically by drm_atomic_helper_connector_hdmi_hotplug()
> being called from .detect() path.
> 
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> 
> [ ... ]

Reviewed-by: Maxime Ripard <mripard@kernel.org>

Thanks!
Maxime

Re: [PATCH v5 10/11] drm/display: bridge-connector: handle CEC adapters

[Maxime Ripard]

On Mon, 7 Apr 2025 18:11:07 +0300, Dmitry Baryshkov wrote:
> Implement necessary glue code to let DRM bridge drivers to implement CEC
> adapters support.
> 
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
> 
> [ ... ]

Reviewed-by: Maxime Ripard <mripard@kernel.org>

Thanks!
Maxime

Re: [PATCH v5 04/11] drm/connector: unregister CEC data

[Dmitry Baryshkov]

On 14/04/2025 17:47, Maxime Ripard wrote:
> Hi,
> 
> On Mon, Apr 07, 2025 at 06:11:01PM +0300, Dmitry Baryshkov wrote:
>> In order to make sure that CEC adapters or notifiers are unregistered
>> and CEC-related data is properly destroyed make drm_connector_cleanup()
>> call CEC's unregister() callback.
>>
>> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
>> Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
>> ---
>>   drivers/gpu/drm/drm_connector.c | 9 +++++++++
>>   1 file changed, 9 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
>> index ba08fbd973829e49ea977251c4f0fb6d96ade631..ae9c02ef9ab102db03c2824683ece37cfbcd3300 100644
>> --- a/drivers/gpu/drm/drm_connector.c
>> +++ b/drivers/gpu/drm/drm_connector.c
>> @@ -743,6 +743,13 @@ void drm_connector_cec_phys_addr_set(struct drm_connector *connector)
>>   }
>>   EXPORT_SYMBOL(drm_connector_cec_phys_addr_set);
>>   
>> +static void drm_connector_cec_unregister(struct drm_connector *connector)
>> +{
>> +	if (connector->cec.funcs &&
>> +	    connector->cec.funcs->unregister)
>> +		connector->cec.funcs->unregister(connector);
>> +}
>> +
>>   /**
>>    * drm_connector_cleanup - cleans up an initialised connector
>>    * @connector: connector to cleanup
>> @@ -763,6 +770,8 @@ void drm_connector_cleanup(struct drm_connector *connector)
>>   
>>   	platform_device_unregister(connector->hdmi_audio.codec_pdev);
>>   
>> +	drm_connector_cec_unregister(connector);
>> +
> 
> Actually, since we know that the HDMI connector is drm-managed, why
> can't we make the call to connector->cec.funcs->unregister a drm-managed
> action registered by drm_connector_hdmi_cec_register?

I haven't settled yet in my mind whether we can/should also use this 
infrastructure for drm_dp_cec management. So, at this point, I'd prefer 
to keep a non-managed unregister function. Once we settle on something 
for drm_dp_cec, we can switch to drmm.

-- 
With best wishes
Dmitry

Re: [PATCH v5 08/11] drm/vc4: hdmi: switch to generic CEC helpers

[Dmitry Baryshkov]

On 14/04/2025 17:41, Maxime Ripard wrote:
> On Mon, Apr 07, 2025 at 06:11:05PM +0300, Dmitry Baryshkov wrote:
>> +static const struct drm_connector_hdmi_cec_adapter_ops vc4_hdmi_cec_adap_ops = {
>> +	.base.unregister = drm_connector_hdmi_cec_unregister,
>> +	.init = vc4_hdmi_cec_init,
>> +	.enable = vc4_hdmi_cec_adap_enable,
>> +	.log_addr = vc4_hdmi_cec_adap_log_addr,
>> +	.transmit = vc4_hdmi_cec_adap_transmit,
>> +};
> 
> Did you compile it? it looks like drm_connector_hdmi_cec_adapter_ops has been renamed, and it
> doesn't have a base.unregister field anymore?

It seems, I've changed my build tree config and this got disabled. I'll 
make sure that it is enabled for the next iteration.

> 
> Maxime


-- 
With best wishes
Dmitry

Re: [PATCH v5 03/11] drm/connector: add CEC-related fields

[Dmitry Baryshkov]

On 14/04/2025 17:52, Maxime Ripard wrote:
> Hi,
> 
> On Mon, Apr 07, 2025 at 06:11:00PM +0300, Dmitry Baryshkov wrote:
>> +/**
>> + * struct drm_connector_cec - DRM Connector CEC-related structure
>> + */
>> +struct drm_connector_cec {
>> +	/**
>> +	 * @mutex: protects all fields in this structure.
>> +	 */
>> +	struct mutex mutex;
>> +
>> +	/**
>> +	 * @funcs: CEC Control Functions
>> +	 */
>> +	const struct drm_connector_cec_funcs *funcs;
>> +
>> +	/**
>> +	 * @data: CEC implementation-specific data
>> +	 */
>> +	void *data;
> 
> Is there a reason we don't just skip that data? The only user I'm seeing
> so far are the helpers, and they only put the cec_adapter pointer in
> there.
> 
> Can't we pass the connector to CEC and make the adapter part of drm_connector_cec?

It will be either cec_notifier or cec_adapter + 
drm_connector_hdmi_cec_funcs. Initially I sketched a union here, but 
then I thought that a void pointer makes more sense. It allows us to 
make CEC data helper-specific. For example, cec-pin might store platform 
callbacks here. DP CEC might need to store AUX pointer, etc.

-- 
With best wishes
Dmitry

Re: [PATCH v5 06/11] drm/display: add CEC helpers code

[Dmitry Baryshkov]

On 14/04/2025 17:58, Maxime Ripard wrote:
> On Mon, Apr 07, 2025 at 06:11:03PM +0300, Dmitry Baryshkov wrote:
>> +static void drm_connector_hdmi_cec_adapter_unregister(struct drm_connector *connector)
>> +{
>> +	struct drm_connector_hdmi_cec_data *data = connector->cec.data;
>> +
>> +	cec_delete_adapter(data->adapter);
>> +
>> +	if (data->funcs->uninit)
>> +		data->funcs->uninit(connector);
>> +
>> +	kfree(data);
>> +	connector->cec.data = NULL;
>> +}
>>
>> [...]
>>
>> +int drm_connector_hdmi_cec_register(struct drm_connector *connector,
>> +				    const struct drm_connector_hdmi_cec_funcs *funcs,
>> +				    const char *name,
>> +				    u8 available_las,
>> +				    struct device *dev)
>> +{
>> +	struct drm_connector_hdmi_cec_data *data;
>> +	struct cec_connector_info conn_info;
>> +	struct cec_adapter *cec_adap;
>> +	int ret;
>> +
>> +	if (!funcs->init || !funcs->enable || !funcs->log_addr || !funcs->transmit)
>> +		return -EINVAL;
>> +
>> +	data = kzalloc(sizeof(*data), GFP_KERNEL);
>> +	if (!data)
>> +		return -ENOMEM;
>> +
>> +	data->funcs = funcs;
>> +
>> +	cec_adap = cec_allocate_adapter(&drm_connector_hdmi_cec_adap_ops, connector, name,
>> +					CEC_CAP_DEFAULTS | CEC_CAP_CONNECTOR_INFO,
>> +					available_las ? : CEC_MAX_LOG_ADDRS);
>> +	ret = PTR_ERR_OR_ZERO(cec_adap);
>> +	if (ret < 0)
>> +		goto err_free;
>> +
>> +	cec_fill_conn_info_from_drm(&conn_info, connector);
>> +	cec_s_conn_info(cec_adap, &conn_info);
>> +
>> +	data->adapter = cec_adap;
>> +
>> +	mutex_lock(&connector->cec.mutex);
>> +
>> +	connector->cec.data = data;
>> +	connector->cec.funcs = &drm_connector_hdmi_cec_adapter_funcs;
>> +
>> +	ret = funcs->init(connector);
>> +	if (ret < 0)
>> +		goto err_delete_adapter;
>> +
>> +	ret = cec_register_adapter(cec_adap, dev);
>> +	if (ret < 0)
>> +		goto err_delete_adapter;
> 
> I'm a bit concerned about the respective lifetimes of CEC adapters and
> DRM connectors.
> 
> When you register the CEC adapter, its associated structure is
> kzalloc'd, and freed when the DRM connector is freed (so when nobody has
> any reference to it anymore: either when the device is torn down, or a
> DP-MST hotplug scenario).
> 
> The CEC adapter however will only be freed when its own users will close
> their file descriptor. So we can have a scenario when the CEC adapter is
> still live but the DRM connector has been unregistered. Thus, the CEC
> adapter data will have been kfree'd.

If I understand correctly, CEC core will handle this thanks to the 
cec_is_registered() calls in the important places. Nevertheless it's 
worth adding a comment and a set of drm_dev_enter() / _exit() calls.

> You might consider safe because $REASONS, but those need to be properly
> detailed and explained.
> 
> That's another reason why I think that just putting the connector
> pointer as data is better: connectors are refcounted, so we know those
> aren't an issue.

Not quite. CEC adapter itself doesn't have a refcount on the connector. 
And if add one, we'd create a loop, preventing connector from being 
unregistered.

-- 
With best wishes
Dmitry

Re: [PATCH v5 03/11] drm/connector: add CEC-related fields

[Dmitry Baryshkov]

On Tue, Apr 15, 2025 at 12:10:06PM +0300, Dmitry Baryshkov wrote:
> On 14/04/2025 17:52, Maxime Ripard wrote:
> > Hi,
> > 
> > On Mon, Apr 07, 2025 at 06:11:00PM +0300, Dmitry Baryshkov wrote:
> > > +/**
> > > + * struct drm_connector_cec - DRM Connector CEC-related structure
> > > + */
> > > +struct drm_connector_cec {
> > > +	/**
> > > +	 * @mutex: protects all fields in this structure.
> > > +	 */
> > > +	struct mutex mutex;
> > > +
> > > +	/**
> > > +	 * @funcs: CEC Control Functions
> > > +	 */
> > > +	const struct drm_connector_cec_funcs *funcs;
> > > +
> > > +	/**
> > > +	 * @data: CEC implementation-specific data
> > > +	 */
> > > +	void *data;
> > 
> > Is there a reason we don't just skip that data? The only user I'm seeing
> > so far are the helpers, and they only put the cec_adapter pointer in
> > there.
> > 
> > Can't we pass the connector to CEC and make the adapter part of drm_connector_cec?
> 
> It will be either cec_notifier or cec_adapter +
> drm_connector_hdmi_cec_funcs. Initially I sketched a union here, but then I
> thought that a void pointer makes more sense. It allows us to make CEC data
> helper-specific. For example, cec-pin might store platform callbacks here.
> DP CEC might need to store AUX pointer, etc.

Maxime, gracious ping. I'd like to resolve these pending items.

As I wrote, I think a void pointer makes more sense. Another option
might be to have a union of corresponding per-backend data. WDYT?

-- 
With best wishes
Dmitry

Re: [PATCH v5 03/11] drm/connector: add CEC-related fields

[Maxime Ripard]

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

On Tue, Apr 15, 2025 at 12:10:06PM +0300, Dmitry Baryshkov wrote:
> On 14/04/2025 17:52, Maxime Ripard wrote:
> > Hi,
> > 
> > On Mon, Apr 07, 2025 at 06:11:00PM +0300, Dmitry Baryshkov wrote:
> > > +/**
> > > + * struct drm_connector_cec - DRM Connector CEC-related structure
> > > + */
> > > +struct drm_connector_cec {
> > > +	/**
> > > +	 * @mutex: protects all fields in this structure.
> > > +	 */
> > > +	struct mutex mutex;
> > > +
> > > +	/**
> > > +	 * @funcs: CEC Control Functions
> > > +	 */
> > > +	const struct drm_connector_cec_funcs *funcs;
> > > +
> > > +	/**
> > > +	 * @data: CEC implementation-specific data
> > > +	 */
> > > +	void *data;
> > 
> > Is there a reason we don't just skip that data? The only user I'm seeing
> > so far are the helpers, and they only put the cec_adapter pointer in
> > there.
> > 
> > Can't we pass the connector to CEC and make the adapter part of drm_connector_cec?
> 
> It will be either cec_notifier or cec_adapter +
> drm_connector_hdmi_cec_funcs. Initially I sketched a union here, but then I
> thought that a void pointer makes more sense. It allows us to make CEC data
> helper-specific. For example, cec-pin might store platform callbacks here.
> DP CEC might need to store AUX pointer, etc.

Ah I see, that makes sense.

Thanks!
Maxime

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

Re: [PATCH v5 04/11] drm/connector: unregister CEC data

[Maxime Ripard]

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

On Tue, Apr 15, 2025 at 12:03:23PM +0300, Dmitry Baryshkov wrote:
> On 14/04/2025 17:47, Maxime Ripard wrote:
> > Hi,
> > 
> > On Mon, Apr 07, 2025 at 06:11:01PM +0300, Dmitry Baryshkov wrote:
> > > In order to make sure that CEC adapters or notifiers are unregistered
> > > and CEC-related data is properly destroyed make drm_connector_cleanup()
> > > call CEC's unregister() callback.
> > > 
> > > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> > > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
> > > ---
> > >   drivers/gpu/drm/drm_connector.c | 9 +++++++++
> > >   1 file changed, 9 insertions(+)
> > > 
> > > diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
> > > index ba08fbd973829e49ea977251c4f0fb6d96ade631..ae9c02ef9ab102db03c2824683ece37cfbcd3300 100644
> > > --- a/drivers/gpu/drm/drm_connector.c
> > > +++ b/drivers/gpu/drm/drm_connector.c
> > > @@ -743,6 +743,13 @@ void drm_connector_cec_phys_addr_set(struct drm_connector *connector)
> > >   }
> > >   EXPORT_SYMBOL(drm_connector_cec_phys_addr_set);
> > > +static void drm_connector_cec_unregister(struct drm_connector *connector)
> > > +{
> > > +	if (connector->cec.funcs &&
> > > +	    connector->cec.funcs->unregister)
> > > +		connector->cec.funcs->unregister(connector);
> > > +}
> > > +
> > >   /**
> > >    * drm_connector_cleanup - cleans up an initialised connector
> > >    * @connector: connector to cleanup
> > > @@ -763,6 +770,8 @@ void drm_connector_cleanup(struct drm_connector *connector)
> > >   	platform_device_unregister(connector->hdmi_audio.codec_pdev);
> > > +	drm_connector_cec_unregister(connector);
> > > +
> > 
> > Actually, since we know that the HDMI connector is drm-managed, why
> > can't we make the call to connector->cec.funcs->unregister a drm-managed
> > action registered by drm_connector_hdmi_cec_register?
> 
> I haven't settled yet in my mind whether we can/should also use this
> infrastructure for drm_dp_cec management. So, at this point, I'd prefer to
> keep a non-managed unregister function. Once we settle on something for
> drm_dp_cec, we can switch to drmm.

I'd rather do the opposite. Let's go for drmm for now, and if we need to
change it for DP, we can always change it.

"Nothing is so permanent as a temporary solution", so I'd rather have
the natural and consistent one for now :)

Maxime

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

Re: [PATCH v5 06/11] drm/display: add CEC helpers code

[Maxime Ripard]

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

On Tue, Apr 15, 2025 at 07:01:25PM +0300, Dmitry Baryshkov wrote:
> On 14/04/2025 17:58, Maxime Ripard wrote:
> > On Mon, Apr 07, 2025 at 06:11:03PM +0300, Dmitry Baryshkov wrote:
> > > +static void drm_connector_hdmi_cec_adapter_unregister(struct drm_connector *connector)
> > > +{
> > > +	struct drm_connector_hdmi_cec_data *data = connector->cec.data;
> > > +
> > > +	cec_delete_adapter(data->adapter);
> > > +
> > > +	if (data->funcs->uninit)
> > > +		data->funcs->uninit(connector);
> > > +
> > > +	kfree(data);
> > > +	connector->cec.data = NULL;
> > > +}
> > > 
> > > [...]
> > > 
> > > +int drm_connector_hdmi_cec_register(struct drm_connector *connector,
> > > +				    const struct drm_connector_hdmi_cec_funcs *funcs,
> > > +				    const char *name,
> > > +				    u8 available_las,
> > > +				    struct device *dev)
> > > +{
> > > +	struct drm_connector_hdmi_cec_data *data;
> > > +	struct cec_connector_info conn_info;
> > > +	struct cec_adapter *cec_adap;
> > > +	int ret;
> > > +
> > > +	if (!funcs->init || !funcs->enable || !funcs->log_addr || !funcs->transmit)
> > > +		return -EINVAL;
> > > +
> > > +	data = kzalloc(sizeof(*data), GFP_KERNEL);
> > > +	if (!data)
> > > +		return -ENOMEM;
> > > +
> > > +	data->funcs = funcs;
> > > +
> > > +	cec_adap = cec_allocate_adapter(&drm_connector_hdmi_cec_adap_ops, connector, name,
> > > +					CEC_CAP_DEFAULTS | CEC_CAP_CONNECTOR_INFO,
> > > +					available_las ? : CEC_MAX_LOG_ADDRS);
> > > +	ret = PTR_ERR_OR_ZERO(cec_adap);
> > > +	if (ret < 0)
> > > +		goto err_free;
> > > +
> > > +	cec_fill_conn_info_from_drm(&conn_info, connector);
> > > +	cec_s_conn_info(cec_adap, &conn_info);
> > > +
> > > +	data->adapter = cec_adap;
> > > +
> > > +	mutex_lock(&connector->cec.mutex);
> > > +
> > > +	connector->cec.data = data;
> > > +	connector->cec.funcs = &drm_connector_hdmi_cec_adapter_funcs;
> > > +
> > > +	ret = funcs->init(connector);
> > > +	if (ret < 0)
> > > +		goto err_delete_adapter;
> > > +
> > > +	ret = cec_register_adapter(cec_adap, dev);
> > > +	if (ret < 0)
> > > +		goto err_delete_adapter;
> > 
> > I'm a bit concerned about the respective lifetimes of CEC adapters and
> > DRM connectors.
> > 
> > When you register the CEC adapter, its associated structure is
> > kzalloc'd, and freed when the DRM connector is freed (so when nobody has
> > any reference to it anymore: either when the device is torn down, or a
> > DP-MST hotplug scenario).
> > 
> > The CEC adapter however will only be freed when its own users will close
> > their file descriptor. So we can have a scenario when the CEC adapter is
> > still live but the DRM connector has been unregistered. Thus, the CEC
> > adapter data will have been kfree'd.
> 
> If I understand correctly, CEC core will handle this thanks to the
> cec_is_registered() calls in the important places. Nevertheless it's worth
> adding a comment and a set of drm_dev_enter() / _exit() calls.

Yep, it definitely deserves a comment.

> > You might consider safe because $REASONS, but those need to be properly
> > detailed and explained.
> > 
> > That's another reason why I think that just putting the connector
> > pointer as data is better: connectors are refcounted, so we know those
> > aren't an issue.
> 
> Not quite. CEC adapter itself doesn't have a refcount on the connector. And
> if add one, we'd create a loop, preventing connector from being
> unregistered.

ACK
Maxime

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

Re: [PATCH v5 04/11] drm/connector: unregister CEC data

[Dmitry Baryshkov]

On Tue, 29 Apr 2025 at 18:35, Maxime Ripard <mripard@kernel.org> wrote:
>
> On Tue, Apr 15, 2025 at 12:03:23PM +0300, Dmitry Baryshkov wrote:
> > On 14/04/2025 17:47, Maxime Ripard wrote:
> > > Hi,
> > >
> > > On Mon, Apr 07, 2025 at 06:11:01PM +0300, Dmitry Baryshkov wrote:
> > > > In order to make sure that CEC adapters or notifiers are unregistered
> > > > and CEC-related data is properly destroyed make drm_connector_cleanup()
> > > > call CEC's unregister() callback.
> > > >
> > > > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
> > > > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com>
> > > > ---
> > > >   drivers/gpu/drm/drm_connector.c | 9 +++++++++
> > > >   1 file changed, 9 insertions(+)
> > > >
> > > > diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
> > > > index ba08fbd973829e49ea977251c4f0fb6d96ade631..ae9c02ef9ab102db03c2824683ece37cfbcd3300 100644
> > > > --- a/drivers/gpu/drm/drm_connector.c
> > > > +++ b/drivers/gpu/drm/drm_connector.c
> > > > @@ -743,6 +743,13 @@ void drm_connector_cec_phys_addr_set(struct drm_connector *connector)
> > > >   }
> > > >   EXPORT_SYMBOL(drm_connector_cec_phys_addr_set);
> > > > +static void drm_connector_cec_unregister(struct drm_connector *connector)
> > > > +{
> > > > + if (connector->cec.funcs &&
> > > > +     connector->cec.funcs->unregister)
> > > > +         connector->cec.funcs->unregister(connector);
> > > > +}
> > > > +
> > > >   /**
> > > >    * drm_connector_cleanup - cleans up an initialised connector
> > > >    * @connector: connector to cleanup
> > > > @@ -763,6 +770,8 @@ void drm_connector_cleanup(struct drm_connector *connector)
> > > >           platform_device_unregister(connector->hdmi_audio.codec_pdev);
> > > > + drm_connector_cec_unregister(connector);
> > > > +
> > >
> > > Actually, since we know that the HDMI connector is drm-managed, why
> > > can't we make the call to connector->cec.funcs->unregister a drm-managed
> > > action registered by drm_connector_hdmi_cec_register?
> >
> > I haven't settled yet in my mind whether we can/should also use this
> > infrastructure for drm_dp_cec management. So, at this point, I'd prefer to
> > keep a non-managed unregister function. Once we settle on something for
> > drm_dp_cec, we can switch to drmm.
>
> I'd rather do the opposite. Let's go for drmm for now, and if we need to
> change it for DP, we can always change it.
>
> "Nothing is so permanent as a temporary solution", so I'd rather have
> the natural and consistent one for now :)

Ack, I'll change that for the next version.

-- 
With best wishes
Dmitry

Re: [PATCH v5 06/11] drm/display: add CEC helpers code

[Jani Nikula]

On Mon, 07 Apr 2025, Dmitry Baryshkov <dmitry.baryshkov@oss.qualcomm.com> wrote:
> diff --git a/include/drm/display/drm_hdmi_cec_helper.h b/include/drm/display/drm_hdmi_cec_helper.h
> new file mode 100644
> index 0000000000000000000000000000000000000000..39bb6d12acb35f539a4a6cd1b61ce97bf4e063ab
> --- /dev/null
> +++ b/include/drm/display/drm_hdmi_cec_helper.h
> @@ -0,0 +1,74 @@
> +/* SPDX-License-Identifier: MIT */
> +
> +#ifndef DRM_DISPLAY_HDMI_CEC_HELPER
> +#define DRM_DISPLAY_HDMI_CEC_HELPER
> +
> +#include <drm/drm_connector.h>

Is there anything in this file that requires that include?

> +
> +#include <linux/types.h>
> +
> +struct drm_connector;
> +
> +struct cec_msg;
> +struct device;
> +
> +struct drm_connector_hdmi_cec_funcs {
> +	/**
> +	 * @init: perform hardware-specific initialization before registering the CEC adapter
> +	 */
> +	int (*init)(struct drm_connector *connector);
> +
> +	/**
> +	 * @uninit: perform hardware-specific teardown for the CEC adapter
> +	 */
> +	void (*uninit)(struct drm_connector *connector);
> +
> +	/**
> +	 * @enable: enable or disable CEC adapter
> +	 */
> +	int (*enable)(struct drm_connector *connector, bool enable);
> +
> +	/**
> +	 * @log_addr: set adapter's logical address, can be called multiple
> +	 * times if adapter supports several LAs
> +	 */
> +	int (*log_addr)(struct drm_connector *connector, u8 logical_addr);
> +
> +	/**
> +	 * @transmit: start transmission of the specified CEC message
> +	 */
> +	int (*transmit)(struct drm_connector *connector, u8 attempts,
> +			u32 signal_free_time, struct cec_msg *msg);
> +};
> +
> +int drm_connector_hdmi_cec_register(struct drm_connector *connector,
> +				    const struct drm_connector_hdmi_cec_funcs *funcs,
> +				    const char *name,
> +				    u8 available_las,
> +				    struct device *dev);
> +
> +void drm_connector_hdmi_cec_received_msg(struct drm_connector *connector,
> +					 struct cec_msg *msg);
> +
> +void drm_connector_hdmi_cec_transmit_done(struct drm_connector *connector,
> +					  u8 status,
> +					  u8 arb_lost_cnt, u8 nack_cnt,
> +					  u8 low_drive_cnt, u8 error_cnt);
> +
> +void drm_connector_hdmi_cec_transmit_attempt_done(struct drm_connector *connector,
> +						  u8 status);
> +
> +#if IS_ENABLED(CONFIG_DRM_DISPLAY_HDMI_CEC_NOTIFIER_HELPER)
> +int drm_connector_hdmi_cec_notifier_register(struct drm_connector *connector,
> +					     const char *port_name,
> +					     struct device *dev);
> +#else
> +static inline int drm_connector_hdmi_cec_notifier_register(struct drm_connector *connector,
> +							   const char *port_name,
> +							   struct device *dev)
> +{
> +	return 0;
> +}
> +#endif
> +
> +#endif

-- 
Jani Nikula, Intel