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

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

diff --git a/drivers/gpu/drm/display/drm_bridge_connector.c b/drivers/gpu/drm/display/drm_bridge_connector.c
index 9f234bc647d5c0880d4c42aea130074b7fa54573..5b77fd59d79abddd419e611a7868b001857ccb37 100644
--- a/drivers/gpu/drm/display/drm_bridge_connector.c
+++ b/drivers/gpu/drm/display/drm_bridge_connector.c
@@ -9,6 +9,8 @@
 #include <linux/property.h>
 #include <linux/slab.h>
 
+#include <media/cec.h>
+
 #include <drm/drm_atomic_state_helper.h>
 #include <drm/drm_bridge.h>
 #include <drm/drm_bridge_connector.h>
@@ -497,6 +499,82 @@ 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;
+
+	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;
+
+	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;
+
+	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;
+
+	drm_bridge_cec_data_set(bridge, connector);
+
+	if (!bridge->funcs->hdmi_cec_init)
+		return 0;
+
+	return bridge->funcs->hdmi_cec_init(connector, bridge);
+}
+
+static void drm_bridge_connector_hdmi_cec_unregister(struct drm_connector *connector)
+{
+	struct drm_bridge_connector *bridge_connector =
+		to_drm_bridge_connector(connector);
+	struct drm_bridge *bridge;
+
+	bridge = bridge_connector->bridge_hdmi;
+
+	drm_bridge_cec_data_set(bridge, NULL);
+
+	drm_connector_hdmi_cec_unregister(connector);
+}
+
+static const struct drm_connector_hdmi_cec_adapter_ops drm_bridge_connector_hdmi_cec_ops = {
+	.base.unregister = drm_bridge_connector_hdmi_cec_unregister,
+	.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
  */
@@ -633,6 +711,21 @@ struct drm_connector *drm_bridge_connector_init(struct drm_device *drm,
 			if (ret)
 				return ERR_PTR(ret);
 		}
+
+		if (bridge->hdmi_cec_adapter_name) {
+			if (!bridge->funcs->hdmi_cec_enable ||
+			    !bridge->funcs->hdmi_cec_log_addr ||
+			    !bridge->funcs->hdmi_cec_transmit)
+				return ERR_PTR(-EINVAL);
+
+			ret = drm_connector_hdmi_cec_register(connector,
+							      &drm_bridge_connector_hdmi_cec_ops,
+							      bridge->hdmi_cec_adapter_name,
+							      bridge->hdmi_cec_available_las,
+							      bridge->hdmi_dev);
+			if (ret)
+				return ERR_PTR(ret);
+		}
 	} else {
 		ret = drmm_connector_init(drm, connector,
 					  &drm_bridge_connector_funcs,
diff --git a/include/drm/drm_bridge.h b/include/drm/drm_bridge.h
index df1d72c7e176c75585283684acc2ef2ffb2f8bff..b55e80a57758e8b652eac0cd01cb245a04e221f5 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;
@@ -729,6 +730,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);
+
 	/**
 	 * @debugfs_init:
 	 *
@@ -924,6 +935,16 @@ struct drm_bridge {
 	 */
 	bool hdmi_cec_notifier;
 
+	/**
+	 * @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

On Sun, Jan 26, 2025 at 03:29:13PM +0200, Dmitry Baryshkov wrote:
>  /* -----------------------------------------------------------------------------
>   * Bridge Connector Initialisation
>   */
> @@ -633,6 +711,21 @@ struct drm_connector *drm_bridge_connector_init(struct drm_device *drm,
>  			if (ret)
>  				return ERR_PTR(ret);
>  		}
> +
> +		if (bridge->hdmi_cec_adapter_name) {
> +			if (!bridge->funcs->hdmi_cec_enable ||
> +			    !bridge->funcs->hdmi_cec_log_addr ||
> +			    !bridge->funcs->hdmi_cec_transmit)
> +				return ERR_PTR(-EINVAL);
> +
> +			ret = drm_connector_hdmi_cec_register(connector,
> +							      &drm_bridge_connector_hdmi_cec_ops,
> +							      bridge->hdmi_cec_adapter_name,
> +							      bridge->hdmi_cec_available_las,
> +							      bridge->hdmi_dev);
> +			if (ret)
> +				return ERR_PTR(ret);
> +		}

Maybe we can use a different bridge feature flag to trigger the CEC code
support instead?

Maxime

On Tue, Jan 28, 2025 at 05:14:06PM +0100, Maxime Ripard wrote:
> On Sun, Jan 26, 2025 at 03:29:13PM +0200, Dmitry Baryshkov wrote:
> >  /* -----------------------------------------------------------------------------
> >   * Bridge Connector Initialisation
> >   */
> > @@ -633,6 +711,21 @@ struct drm_connector *drm_bridge_connector_init(struct drm_device *drm,
> >  			if (ret)
> >  				return ERR_PTR(ret);
> >  		}
> > +
> > +		if (bridge->hdmi_cec_adapter_name) {
> > +			if (!bridge->funcs->hdmi_cec_enable ||
> > +			    !bridge->funcs->hdmi_cec_log_addr ||
> > +			    !bridge->funcs->hdmi_cec_transmit)
> > +				return ERR_PTR(-EINVAL);
> > +
> > +			ret = drm_connector_hdmi_cec_register(connector,
> > +							      &drm_bridge_connector_hdmi_cec_ops,
> > +							      bridge->hdmi_cec_adapter_name,
> > +							      bridge->hdmi_cec_available_las,
> > +							      bridge->hdmi_dev);
> > +			if (ret)
> > +				return ERR_PTR(ret);
> > +		}
> 
> Maybe we can use a different bridge feature flag to trigger the CEC code
> support instead?

it is possible, but what is the possible usecase? DP drivers should be
using DP_AUX CEC instead. And I think there are no other kinds of
bridges which implement CEC support. Meson driver does something strange
by registering CEC notifier from meson_encoder_hdmi. I think instead
this should be moved to the DW HDMI bridge itself

-- 
With best wishes
Dmitry

On 29/01/2025 00:44, Dmitry Baryshkov wrote:
> On Tue, Jan 28, 2025 at 05:14:06PM +0100, Maxime Ripard wrote:
>> On Sun, Jan 26, 2025 at 03:29:13PM +0200, Dmitry Baryshkov wrote:
>>>   /* -----------------------------------------------------------------------------
>>>    * Bridge Connector Initialisation
>>>    */
>>> @@ -633,6 +711,21 @@ struct drm_connector *drm_bridge_connector_init(struct drm_device *drm,
>>>   			if (ret)
>>>   				return ERR_PTR(ret);
>>>   		}
>>> +
>>> +		if (bridge->hdmi_cec_adapter_name) {
>>> +			if (!bridge->funcs->hdmi_cec_enable ||
>>> +			    !bridge->funcs->hdmi_cec_log_addr ||
>>> +			    !bridge->funcs->hdmi_cec_transmit)
>>> +				return ERR_PTR(-EINVAL);
>>> +
>>> +			ret = drm_connector_hdmi_cec_register(connector,
>>> +							      &drm_bridge_connector_hdmi_cec_ops,
>>> +							      bridge->hdmi_cec_adapter_name,
>>> +							      bridge->hdmi_cec_available_las,
>>> +							      bridge->hdmi_dev);
>>> +			if (ret)
>>> +				return ERR_PTR(ret);
>>> +		}
>>
>> Maybe we can use a different bridge feature flag to trigger the CEC code
>> support instead?
> 
> it is possible, but what is the possible usecase? DP drivers should be
> using DP_AUX CEC instead. And I think there are no other kinds of
> bridges which implement CEC support. Meson driver does something strange
> by registering CEC notifier from meson_encoder_hdmi. I think instead
> this should be moved to the DW HDMI bridge itself
> 

It was done before bridge_connector has any support for CEC to keep the
functionnality, I'll be happy to switch to this.

Neil

On Wed, Jan 29, 2025 at 05:44:31PM +0100, Neil Armstrong wrote:
> On 29/01/2025 00:44, Dmitry Baryshkov wrote:
> > On Tue, Jan 28, 2025 at 05:14:06PM +0100, Maxime Ripard wrote:
> > > On Sun, Jan 26, 2025 at 03:29:13PM +0200, Dmitry Baryshkov wrote:
> > > >   /* -----------------------------------------------------------------------------
> > > >    * Bridge Connector Initialisation
> > > >    */
> > > > @@ -633,6 +711,21 @@ struct drm_connector *drm_bridge_connector_init(struct drm_device *drm,
> > > >   			if (ret)
> > > >   				return ERR_PTR(ret);
> > > >   		}
> > > > +
> > > > +		if (bridge->hdmi_cec_adapter_name) {
> > > > +			if (!bridge->funcs->hdmi_cec_enable ||
> > > > +			    !bridge->funcs->hdmi_cec_log_addr ||
> > > > +			    !bridge->funcs->hdmi_cec_transmit)
> > > > +				return ERR_PTR(-EINVAL);
> > > > +
> > > > +			ret = drm_connector_hdmi_cec_register(connector,
> > > > +							      &drm_bridge_connector_hdmi_cec_ops,
> > > > +							      bridge->hdmi_cec_adapter_name,
> > > > +							      bridge->hdmi_cec_available_las,
> > > > +							      bridge->hdmi_dev);
> > > > +			if (ret)
> > > > +				return ERR_PTR(ret);
> > > > +		}
> > > 
> > > Maybe we can use a different bridge feature flag to trigger the CEC code
> > > support instead?
> > 
> > it is possible, but what is the possible usecase? DP drivers should be
> > using DP_AUX CEC instead. And I think there are no other kinds of
> > bridges which implement CEC support. Meson driver does something strange
> > by registering CEC notifier from meson_encoder_hdmi. I think instead
> > this should be moved to the DW HDMI bridge itself
> 
> It was done before bridge_connector has any support for CEC to keep the
> functionnality, I'll be happy to switch to this.

Well... Converting dw-hdmi to use all HDMI API is going to be
interesting anyway:

- iMX8, iMX6 attach with flags=0, so dw-hdmi creates connector (with all
  its fanciness).
- Ingenic and RCAR-DU use simple drm_bridge_connector, so they will just
  get all new features.
- Meson uses drm_bridge_connector + additional bits on top of it (like
  CEC, max_bpc, HDR).

Either we end up with a monstruous commit that reworks dw-hdmi _and_
drop extra bits from meson driver at the same time or we need some
well-thought plan. For example, first make some of the features of Meson
optional and dependent on their generic counterparts being registered by
drm_bridge_connector, then convert dw-hdmi _and_ use
drm_bridge_connector internally, finally drop extra meson features now
consumed by the core.


-- 
With best wishes
Dmitry