[PATCH 0/5] drm/omap: hdmi: improve hdmi4 CEC, add CEC for hdmi5

[PATCH 0/5] drm/omap: hdmi: improve hdmi4 CEC, add CEC for hdmi5

[Hans Verkuil]

This series improves the drm_bridge support for CEC by introducing two
new bridge ops in the first patch, and using those in the second patch.

This makes it possible to call cec_s_conn_info() and set
CEC_CAP_CONNECTOR_INFO for the CEC adapter, so userspace can associate
the CEC adapter with the corresponding DRM connector.

The third patch simplifies CEC physical address handling by using the
cec_s_phys_addr_from_edid helper function that didn't exist when this
code was originally written.

The fourth patch adds CEC support to the OMAP5 driver and the last
patch adds the missing cec clock to the dra7 and omap5 device tree.

Tested with a Pandaboard and a Beagle X15 board.

Regards,

	Hans

Hans Verkuil (5):
  drm: drm_bridge: add cec_init/exit bridge ops
  drm/omap: hdmi4: switch to the cec bridge ops
  drm/omap: hdmi4: simplify CEC Phys Addr handling
  drm/omap: hdmi5: add CEC support
  ARM: dts: dra7/omap5: add cec clock

 arch/arm/boot/dts/dra7.dtsi              |   5 +-
 arch/arm/boot/dts/omap5.dtsi             |   5 +-
 drivers/gpu/drm/drm_bridge_connector.c   |  23 +++
 drivers/gpu/drm/omapdrm/dss/Kconfig      |   8 +
 drivers/gpu/drm/omapdrm/dss/Makefile     |   1 +
 drivers/gpu/drm/omapdrm/dss/hdmi.h       |   1 +
 drivers/gpu/drm/omapdrm/dss/hdmi4.c      |  41 ++---
 drivers/gpu/drm/omapdrm/dss/hdmi4_cec.c  |  12 +-
 drivers/gpu/drm/omapdrm/dss/hdmi4_cec.h  |  12 +-
 drivers/gpu/drm/omapdrm/dss/hdmi5.c      |  63 +++++--
 drivers/gpu/drm/omapdrm/dss/hdmi5_cec.c  | 201 +++++++++++++++++++++++
 drivers/gpu/drm/omapdrm/dss/hdmi5_cec.h  |  42 +++++
 drivers/gpu/drm/omapdrm/dss/hdmi5_core.c |  28 +++-
 drivers/gpu/drm/omapdrm/dss/hdmi5_core.h |  33 +++-
 include/drm/drm_bridge.h                 |  31 ++++
 15 files changed, 453 insertions(+), 53 deletions(-)
 create mode 100644 drivers/gpu/drm/omapdrm/dss/hdmi5_cec.c
 create mode 100644 drivers/gpu/drm/omapdrm/dss/hdmi5_cec.h

-- 
2.30.0

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 1/5] drm: drm_bridge: add cec_init/exit bridge ops

[Hans Verkuil]

Add bridge cec_init/exit ops. These ops will be responsible for
creating and destroying the CEC adapter for the bridge that supports
CEC.

Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
---
 drivers/gpu/drm/drm_bridge_connector.c | 23 +++++++++++++++++++
 include/drm/drm_bridge.h               | 31 ++++++++++++++++++++++++++
 2 files changed, 54 insertions(+)

diff --git a/drivers/gpu/drm/drm_bridge_connector.c b/drivers/gpu/drm/drm_bridge_connector.c
index 791379816837..2ff90f5e468c 100644
--- a/drivers/gpu/drm/drm_bridge_connector.c
+++ b/drivers/gpu/drm/drm_bridge_connector.c
@@ -84,6 +84,13 @@ struct drm_bridge_connector {
 	 * connector modes detection, if any (see &DRM_BRIDGE_OP_MODES).
 	 */
 	struct drm_bridge *bridge_modes;
+	/**
+	 * @bridge_cec:
+	 *
+	 * The last bridge in the chain (closest to the connector) that provides
+	 * cec adapter support, if any (see &DRM_BRIDGE_OP_CEC).
+	 */
+	struct drm_bridge *bridge_cec;
 };
 
 #define to_drm_bridge_connector(x) \
@@ -204,6 +211,11 @@ static void drm_bridge_connector_destroy(struct drm_connector *connector)
 	struct drm_bridge_connector *bridge_connector =
 		to_drm_bridge_connector(connector);
 
+	if (bridge_connector->bridge_cec) {
+		struct drm_bridge *cec = bridge_connector->bridge_cec;
+
+		cec->funcs->cec_exit(cec);
+	}
 	if (bridge_connector->bridge_hpd) {
 		struct drm_bridge *hpd = bridge_connector->bridge_hpd;
 
@@ -352,6 +364,8 @@ struct drm_connector *drm_bridge_connector_init(struct drm_device *drm,
 			bridge_connector->bridge_detect = bridge;
 		if (bridge->ops & DRM_BRIDGE_OP_MODES)
 			bridge_connector->bridge_modes = bridge;
+		if (bridge->ops & DRM_BRIDGE_OP_CEC)
+			bridge_connector->bridge_cec = bridge;
 
 		if (!drm_bridge_get_next_bridge(bridge))
 			connector_type = bridge->type;
@@ -374,6 +388,15 @@ struct drm_connector *drm_bridge_connector_init(struct drm_device *drm,
 	else if (bridge_connector->bridge_detect)
 		connector->polled = DRM_CONNECTOR_POLL_CONNECT
 				  | DRM_CONNECTOR_POLL_DISCONNECT;
+	if (bridge_connector->bridge_cec) {
+		struct drm_bridge *bridge = bridge_connector->bridge_cec;
+		int ret = bridge->funcs->cec_init(bridge, connector);
+
+		if (ret) {
+			drm_bridge_connector_destroy(connector);
+			return ERR_PTR(ret);
+		}
+	}
 
 	return connector;
 }
diff --git a/include/drm/drm_bridge.h b/include/drm/drm_bridge.h
index 2195daa289d2..4c83c2657e87 100644
--- a/include/drm/drm_bridge.h
+++ b/include/drm/drm_bridge.h
@@ -629,6 +629,30 @@ struct drm_bridge_funcs {
 	 * the DRM_BRIDGE_OP_HPD flag in their &drm_bridge->ops.
 	 */
 	void (*hpd_disable)(struct drm_bridge *bridge);
+
+	/**
+	 * @cec_init:
+	 *
+	 * Initialize the CEC adapter.
+	 *
+	 * This callback is optional and shall only be implemented by bridges
+	 * that support a CEC adapter. Bridges that implement it shall also
+	 * implement the @cec_exit callback and set the DRM_BRIDGE_OP_CEC flag
+	 * in their &drm_bridge->ops.
+	 */
+	int (*cec_init)(struct drm_bridge *bridge, struct drm_connector *conn);
+
+	/**
+	 * @cec_exit:
+	 *
+	 * Terminate the CEC adapter.
+	 *
+	 * This callback is optional and shall only be implemented by bridges
+	 * that support a CEC adapter. Bridges that implement it shall also
+	 * implement the @cec_init callback and set the DRM_BRIDGE_OP_CEC flag
+	 * in their &drm_bridge->ops.
+	 */
+	void (*cec_exit)(struct drm_bridge *bridge);
 };
 
 /**
@@ -698,6 +722,13 @@ enum drm_bridge_ops {
 	 * this flag shall implement the &drm_bridge_funcs->get_modes callback.
 	 */
 	DRM_BRIDGE_OP_MODES = BIT(3),
+	/**
+	 * @DRM_BRIDGE_OP_CEC: The bridge supports a CEC adapter.
+	 * Bridges that set this flag shall implement the
+	 * &drm_bridge_funcs->cec_init and &drm_bridge_funcs->cec_exit
+	 * callbacks.
+	 */
+	DRM_BRIDGE_OP_CEC = BIT(4),
 };
 
 /**
-- 
2.30.0

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 1/5] drm: drm_bridge: add cec_init/exit bridge ops

[Tomi Valkeinen]

On 11/02/2021 12:36, Hans Verkuil wrote:
> Add bridge cec_init/exit ops. These ops will be responsible for
> creating and destroying the CEC adapter for the bridge that supports
> CEC.
> 
> Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
> ---
>  drivers/gpu/drm/drm_bridge_connector.c | 23 +++++++++++++++++++
>  include/drm/drm_bridge.h               | 31 ++++++++++++++++++++++++++
>  2 files changed, 54 insertions(+)

Reviewed-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>

 Tomi

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 1/5] drm: drm_bridge: add cec_init/exit bridge ops

[Laurent Pinchart]

Hi Hans,

Thank you for the patch.

On Thu, Feb 11, 2021 at 11:36:59AM +0100, Hans Verkuil wrote:
> Add bridge cec_init/exit ops. These ops will be responsible for
> creating and destroying the CEC adapter for the bridge that supports
> CEC.
> 
> Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
> ---
>  drivers/gpu/drm/drm_bridge_connector.c | 23 +++++++++++++++++++
>  include/drm/drm_bridge.h               | 31 ++++++++++++++++++++++++++
>  2 files changed, 54 insertions(+)
> 
> diff --git a/drivers/gpu/drm/drm_bridge_connector.c b/drivers/gpu/drm/drm_bridge_connector.c
> index 791379816837..2ff90f5e468c 100644
> --- a/drivers/gpu/drm/drm_bridge_connector.c
> +++ b/drivers/gpu/drm/drm_bridge_connector.c
> @@ -84,6 +84,13 @@ struct drm_bridge_connector {
>  	 * connector modes detection, if any (see &DRM_BRIDGE_OP_MODES).
>  	 */
>  	struct drm_bridge *bridge_modes;
> +	/**
> +	 * @bridge_cec:
> +	 *
> +	 * The last bridge in the chain (closest to the connector) that provides
> +	 * cec adapter support, if any (see &DRM_BRIDGE_OP_CEC).
> +	 */
> +	struct drm_bridge *bridge_cec;
>  };
>  
>  #define to_drm_bridge_connector(x) \
> @@ -204,6 +211,11 @@ static void drm_bridge_connector_destroy(struct drm_connector *connector)
>  	struct drm_bridge_connector *bridge_connector =
>  		to_drm_bridge_connector(connector);
>  
> +	if (bridge_connector->bridge_cec) {
> +		struct drm_bridge *cec = bridge_connector->bridge_cec;
> +
> +		cec->funcs->cec_exit(cec);
> +	}
>  	if (bridge_connector->bridge_hpd) {
>  		struct drm_bridge *hpd = bridge_connector->bridge_hpd;
>  
> @@ -352,6 +364,8 @@ struct drm_connector *drm_bridge_connector_init(struct drm_device *drm,
>  			bridge_connector->bridge_detect = bridge;
>  		if (bridge->ops & DRM_BRIDGE_OP_MODES)
>  			bridge_connector->bridge_modes = bridge;
> +		if (bridge->ops & DRM_BRIDGE_OP_CEC)
> +			bridge_connector->bridge_cec = bridge;
>  
>  		if (!drm_bridge_get_next_bridge(bridge))
>  			connector_type = bridge->type;
> @@ -374,6 +388,15 @@ struct drm_connector *drm_bridge_connector_init(struct drm_device *drm,
>  	else if (bridge_connector->bridge_detect)
>  		connector->polled = DRM_CONNECTOR_POLL_CONNECT
>  				  | DRM_CONNECTOR_POLL_DISCONNECT;
> +	if (bridge_connector->bridge_cec) {
> +		struct drm_bridge *bridge = bridge_connector->bridge_cec;
> +		int ret = bridge->funcs->cec_init(bridge, connector);
> +
> +		if (ret) {
> +			drm_bridge_connector_destroy(connector);
> +			return ERR_PTR(ret);
> +		}
> +	}
>  
>  	return connector;
>  }
> diff --git a/include/drm/drm_bridge.h b/include/drm/drm_bridge.h
> index 2195daa289d2..4c83c2657e87 100644
> --- a/include/drm/drm_bridge.h
> +++ b/include/drm/drm_bridge.h
> @@ -629,6 +629,30 @@ struct drm_bridge_funcs {
>  	 * the DRM_BRIDGE_OP_HPD flag in their &drm_bridge->ops.
>  	 */
>  	void (*hpd_disable)(struct drm_bridge *bridge);
> +
> +	/**
> +	 * @cec_init:
> +	 *
> +	 * Initialize the CEC adapter.
> +	 *
> +	 * This callback is optional and shall only be implemented by bridges
> +	 * that support a CEC adapter. Bridges that implement it shall also
> +	 * implement the @cec_exit callback and set the DRM_BRIDGE_OP_CEC flag
> +	 * in their &drm_bridge->ops.
> +	 */
> +	int (*cec_init)(struct drm_bridge *bridge, struct drm_connector *conn);
> +
> +	/**
> +	 * @cec_exit:
> +	 *
> +	 * Terminate the CEC adapter.
> +	 *
> +	 * This callback is optional and shall only be implemented by bridges
> +	 * that support a CEC adapter. Bridges that implement it shall also
> +	 * implement the @cec_init callback and set the DRM_BRIDGE_OP_CEC flag
> +	 * in their &drm_bridge->ops.
> +	 */
> +	void (*cec_exit)(struct drm_bridge *bridge);

These are very ad-hoc operations. Would it make sense to have something
that could also be reused for other type of intiialization and cleanup
that require access to the drm_connector ?

>  };
>  
>  /**
> @@ -698,6 +722,13 @@ enum drm_bridge_ops {
>  	 * this flag shall implement the &drm_bridge_funcs->get_modes callback.
>  	 */
>  	DRM_BRIDGE_OP_MODES = BIT(3),
> +	/**
> +	 * @DRM_BRIDGE_OP_CEC: The bridge supports a CEC adapter.
> +	 * Bridges that set this flag shall implement the
> +	 * &drm_bridge_funcs->cec_init and &drm_bridge_funcs->cec_exit
> +	 * callbacks.
> +	 */
> +	DRM_BRIDGE_OP_CEC = BIT(4),
>  };
>  
>  /**

-- 
Regards,

Laurent Pinchart
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 1/5] drm: drm_bridge: add cec_init/exit bridge ops

[Hans Verkuil]

On 19/02/2021 13:02, Laurent Pinchart wrote:
> Hi Hans,
> 
> Thank you for the patch.
> 
> On Thu, Feb 11, 2021 at 11:36:59AM +0100, Hans Verkuil wrote:
>> Add bridge cec_init/exit ops. These ops will be responsible for
>> creating and destroying the CEC adapter for the bridge that supports
>> CEC.
>>
>> Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
>> ---
>>  drivers/gpu/drm/drm_bridge_connector.c | 23 +++++++++++++++++++
>>  include/drm/drm_bridge.h               | 31 ++++++++++++++++++++++++++
>>  2 files changed, 54 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/drm_bridge_connector.c b/drivers/gpu/drm/drm_bridge_connector.c
>> index 791379816837..2ff90f5e468c 100644
>> --- a/drivers/gpu/drm/drm_bridge_connector.c
>> +++ b/drivers/gpu/drm/drm_bridge_connector.c
>> @@ -84,6 +84,13 @@ struct drm_bridge_connector {
>>  	 * connector modes detection, if any (see &DRM_BRIDGE_OP_MODES).
>>  	 */
>>  	struct drm_bridge *bridge_modes;
>> +	/**
>> +	 * @bridge_cec:
>> +	 *
>> +	 * The last bridge in the chain (closest to the connector) that provides
>> +	 * cec adapter support, if any (see &DRM_BRIDGE_OP_CEC).
>> +	 */
>> +	struct drm_bridge *bridge_cec;
>>  };
>>  
>>  #define to_drm_bridge_connector(x) \
>> @@ -204,6 +211,11 @@ static void drm_bridge_connector_destroy(struct drm_connector *connector)
>>  	struct drm_bridge_connector *bridge_connector =
>>  		to_drm_bridge_connector(connector);
>>  
>> +	if (bridge_connector->bridge_cec) {
>> +		struct drm_bridge *cec = bridge_connector->bridge_cec;
>> +
>> +		cec->funcs->cec_exit(cec);
>> +	}
>>  	if (bridge_connector->bridge_hpd) {
>>  		struct drm_bridge *hpd = bridge_connector->bridge_hpd;
>>  
>> @@ -352,6 +364,8 @@ struct drm_connector *drm_bridge_connector_init(struct drm_device *drm,
>>  			bridge_connector->bridge_detect = bridge;
>>  		if (bridge->ops & DRM_BRIDGE_OP_MODES)
>>  			bridge_connector->bridge_modes = bridge;
>> +		if (bridge->ops & DRM_BRIDGE_OP_CEC)
>> +			bridge_connector->bridge_cec = bridge;
>>  
>>  		if (!drm_bridge_get_next_bridge(bridge))
>>  			connector_type = bridge->type;
>> @@ -374,6 +388,15 @@ struct drm_connector *drm_bridge_connector_init(struct drm_device *drm,
>>  	else if (bridge_connector->bridge_detect)
>>  		connector->polled = DRM_CONNECTOR_POLL_CONNECT
>>  				  | DRM_CONNECTOR_POLL_DISCONNECT;
>> +	if (bridge_connector->bridge_cec) {
>> +		struct drm_bridge *bridge = bridge_connector->bridge_cec;
>> +		int ret = bridge->funcs->cec_init(bridge, connector);
>> +
>> +		if (ret) {
>> +			drm_bridge_connector_destroy(connector);
>> +			return ERR_PTR(ret);
>> +		}
>> +	}
>>  
>>  	return connector;
>>  }
>> diff --git a/include/drm/drm_bridge.h b/include/drm/drm_bridge.h
>> index 2195daa289d2..4c83c2657e87 100644
>> --- a/include/drm/drm_bridge.h
>> +++ b/include/drm/drm_bridge.h
>> @@ -629,6 +629,30 @@ struct drm_bridge_funcs {
>>  	 * the DRM_BRIDGE_OP_HPD flag in their &drm_bridge->ops.
>>  	 */
>>  	void (*hpd_disable)(struct drm_bridge *bridge);
>> +
>> +	/**
>> +	 * @cec_init:
>> +	 *
>> +	 * Initialize the CEC adapter.
>> +	 *
>> +	 * This callback is optional and shall only be implemented by bridges
>> +	 * that support a CEC adapter. Bridges that implement it shall also
>> +	 * implement the @cec_exit callback and set the DRM_BRIDGE_OP_CEC flag
>> +	 * in their &drm_bridge->ops.
>> +	 */
>> +	int (*cec_init)(struct drm_bridge *bridge, struct drm_connector *conn);
>> +
>> +	/**
>> +	 * @cec_exit:
>> +	 *
>> +	 * Terminate the CEC adapter.
>> +	 *
>> +	 * This callback is optional and shall only be implemented by bridges
>> +	 * that support a CEC adapter. Bridges that implement it shall also
>> +	 * implement the @cec_init callback and set the DRM_BRIDGE_OP_CEC flag
>> +	 * in their &drm_bridge->ops.
>> +	 */
>> +	void (*cec_exit)(struct drm_bridge *bridge);
> 
> These are very ad-hoc operations. Would it make sense to have something
> that could also be reused for other type of intiialization and cleanup
> that require access to the drm_connector ?

I do not have a very strong opinion, to be honest.

How about this:

	/**
	 * @DRM_BRIDGE_OP_CONN: The bridge can do additional work when
	 * a drm_connector is created or destroyed, such as creating or
	 * removing a CEC adapter.
	 * &drm_bridge_funcs->conn_init and &drm_bridge_funcs->conn_exit
	 * callbacks.
	 */
	DRM_BRIDGE_OP_CONN = BIT(4),

Would that work better for you?

Regards,

	Hans

> 
>>  };
>>  
>>  /**
>> @@ -698,6 +722,13 @@ enum drm_bridge_ops {
>>  	 * this flag shall implement the &drm_bridge_funcs->get_modes callback.
>>  	 */
>>  	DRM_BRIDGE_OP_MODES = BIT(3),
>> +	/**
>> +	 * @DRM_BRIDGE_OP_CEC: The bridge supports a CEC adapter.
>> +	 * Bridges that set this flag shall implement the
>> +	 * &drm_bridge_funcs->cec_init and &drm_bridge_funcs->cec_exit
>> +	 * callbacks.
>> +	 */
>> +	DRM_BRIDGE_OP_CEC = BIT(4),
>>  };
>>  
>>  /**
> 

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 1/5] drm: drm_bridge: add cec_init/exit bridge ops

[Laurent Pinchart]

Hi Hans,

(CC'ing the DRM bridge maintainers and Daniel Vetter)

On Mon, Mar 01, 2021 at 11:56:28AM +0100, Hans Verkuil wrote:
> On 19/02/2021 13:02, Laurent Pinchart wrote:
> > On Thu, Feb 11, 2021 at 11:36:59AM +0100, Hans Verkuil wrote:
> >> Add bridge cec_init/exit ops. These ops will be responsible for
> >> creating and destroying the CEC adapter for the bridge that supports
> >> CEC.
> >>
> >> Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
> >> ---
> >>  drivers/gpu/drm/drm_bridge_connector.c | 23 +++++++++++++++++++
> >>  include/drm/drm_bridge.h               | 31 ++++++++++++++++++++++++++
> >>  2 files changed, 54 insertions(+)
> >>
> >> diff --git a/drivers/gpu/drm/drm_bridge_connector.c b/drivers/gpu/drm/drm_bridge_connector.c
> >> index 791379816837..2ff90f5e468c 100644
> >> --- a/drivers/gpu/drm/drm_bridge_connector.c
> >> +++ b/drivers/gpu/drm/drm_bridge_connector.c
> >> @@ -84,6 +84,13 @@ struct drm_bridge_connector {
> >>  	 * connector modes detection, if any (see &DRM_BRIDGE_OP_MODES).
> >>  	 */
> >>  	struct drm_bridge *bridge_modes;
> >> +	/**
> >> +	 * @bridge_cec:
> >> +	 *
> >> +	 * The last bridge in the chain (closest to the connector) that provides
> >> +	 * cec adapter support, if any (see &DRM_BRIDGE_OP_CEC).
> >> +	 */
> >> +	struct drm_bridge *bridge_cec;
> >>  };
> >>  
> >>  #define to_drm_bridge_connector(x) \
> >> @@ -204,6 +211,11 @@ static void drm_bridge_connector_destroy(struct drm_connector *connector)
> >>  	struct drm_bridge_connector *bridge_connector =
> >>  		to_drm_bridge_connector(connector);
> >>  
> >> +	if (bridge_connector->bridge_cec) {
> >> +		struct drm_bridge *cec = bridge_connector->bridge_cec;
> >> +
> >> +		cec->funcs->cec_exit(cec);
> >> +	}
> >>  	if (bridge_connector->bridge_hpd) {
> >>  		struct drm_bridge *hpd = bridge_connector->bridge_hpd;
> >>  
> >> @@ -352,6 +364,8 @@ struct drm_connector *drm_bridge_connector_init(struct drm_device *drm,
> >>  			bridge_connector->bridge_detect = bridge;
> >>  		if (bridge->ops & DRM_BRIDGE_OP_MODES)
> >>  			bridge_connector->bridge_modes = bridge;
> >> +		if (bridge->ops & DRM_BRIDGE_OP_CEC)
> >> +			bridge_connector->bridge_cec = bridge;
> >>  
> >>  		if (!drm_bridge_get_next_bridge(bridge))
> >>  			connector_type = bridge->type;
> >> @@ -374,6 +388,15 @@ struct drm_connector *drm_bridge_connector_init(struct drm_device *drm,
> >>  	else if (bridge_connector->bridge_detect)
> >>  		connector->polled = DRM_CONNECTOR_POLL_CONNECT
> >>  				  | DRM_CONNECTOR_POLL_DISCONNECT;
> >> +	if (bridge_connector->bridge_cec) {
> >> +		struct drm_bridge *bridge = bridge_connector->bridge_cec;
> >> +		int ret = bridge->funcs->cec_init(bridge, connector);
> >> +
> >> +		if (ret) {
> >> +			drm_bridge_connector_destroy(connector);
> >> +			return ERR_PTR(ret);
> >> +		}
> >> +	}
> >>  
> >>  	return connector;
> >>  }
> >> diff --git a/include/drm/drm_bridge.h b/include/drm/drm_bridge.h
> >> index 2195daa289d2..4c83c2657e87 100644
> >> --- a/include/drm/drm_bridge.h
> >> +++ b/include/drm/drm_bridge.h
> >> @@ -629,6 +629,30 @@ struct drm_bridge_funcs {
> >>  	 * the DRM_BRIDGE_OP_HPD flag in their &drm_bridge->ops.
> >>  	 */
> >>  	void (*hpd_disable)(struct drm_bridge *bridge);
> >> +
> >> +	/**
> >> +	 * @cec_init:
> >> +	 *
> >> +	 * Initialize the CEC adapter.
> >> +	 *
> >> +	 * This callback is optional and shall only be implemented by bridges
> >> +	 * that support a CEC adapter. Bridges that implement it shall also
> >> +	 * implement the @cec_exit callback and set the DRM_BRIDGE_OP_CEC flag
> >> +	 * in their &drm_bridge->ops.
> >> +	 */
> >> +	int (*cec_init)(struct drm_bridge *bridge, struct drm_connector *conn);
> >> +
> >> +	/**
> >> +	 * @cec_exit:
> >> +	 *
> >> +	 * Terminate the CEC adapter.
> >> +	 *
> >> +	 * This callback is optional and shall only be implemented by bridges
> >> +	 * that support a CEC adapter. Bridges that implement it shall also
> >> +	 * implement the @cec_init callback and set the DRM_BRIDGE_OP_CEC flag
> >> +	 * in their &drm_bridge->ops.
> >> +	 */
> >> +	void (*cec_exit)(struct drm_bridge *bridge);
> > 
> > These are very ad-hoc operations. Would it make sense to have something
> > that could also be reused for other type of intiialization and cleanup
> > that require access to the drm_connector ?
> 
> I do not have a very strong opinion, to be honest.
> 
> How about this:
> 
> 	/**
> 	 * @DRM_BRIDGE_OP_CONN: The bridge can do additional work when
> 	 * a drm_connector is created or destroyed, such as creating or
> 	 * removing a CEC adapter.
> 	 * &drm_bridge_funcs->conn_init and &drm_bridge_funcs->conn_exit
> 	 * callbacks.
> 	 */
> 	DRM_BRIDGE_OP_CONN = BIT(4),
> 
> Would that work better for you?

I like that better, it's more generic, but I then think we should drop
DRM_BRIDGE_OP_CONN. More than one bridge may need to be notified of
connector creation and deletion, so I'd loop over the bridges and call
all the ones that implement conn_init and conn_exit (which I'd rename to
spell out connector in full, but maybe there could also be better names
for init/exit as conn_init doesn't initialize the connector itself).

We could also merge the conn_init and conn_exit operations into a
connector_notify operation that would take an enum notification
parameter.

> >>  };
> >>  
> >>  /**
> >> @@ -698,6 +722,13 @@ enum drm_bridge_ops {
> >>  	 * this flag shall implement the &drm_bridge_funcs->get_modes callback.
> >>  	 */
> >>  	DRM_BRIDGE_OP_MODES = BIT(3),
> >> +	/**
> >> +	 * @DRM_BRIDGE_OP_CEC: The bridge supports a CEC adapter.
> >> +	 * Bridges that set this flag shall implement the
> >> +	 * &drm_bridge_funcs->cec_init and &drm_bridge_funcs->cec_exit
> >> +	 * callbacks.
> >> +	 */
> >> +	DRM_BRIDGE_OP_CEC = BIT(4),
> >>  };
> >>  
> >>  /**

-- 
Regards,

Laurent Pinchart
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 2/5] drm/omap: hdmi4: switch to the cec bridge ops

[Hans Verkuil]

Implement the new CEC bridge ops. This makes it possible to associate
a CEC adapter with a drm connector, which helps userspace determine
with cec device node belongs to which drm connector.

Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
---
 drivers/gpu/drm/omapdrm/dss/hdmi4.c     | 28 +++++++++++++++++--------
 drivers/gpu/drm/omapdrm/dss/hdmi4_cec.c |  8 ++++---
 drivers/gpu/drm/omapdrm/dss/hdmi4_cec.h |  7 ++++---
 3 files changed, 28 insertions(+), 15 deletions(-)

diff --git a/drivers/gpu/drm/omapdrm/dss/hdmi4.c b/drivers/gpu/drm/omapdrm/dss/hdmi4.c
index 8de41e74e8f8..765379380d4b 100644
--- a/drivers/gpu/drm/omapdrm/dss/hdmi4.c
+++ b/drivers/gpu/drm/omapdrm/dss/hdmi4.c
@@ -482,6 +482,21 @@ static struct edid *hdmi4_bridge_get_edid(struct drm_bridge *bridge,
 	return edid;
 }
 
+static int hdmi4_bridge_cec_init(struct drm_bridge *bridge,
+				 struct drm_connector *conn)
+{
+	struct omap_hdmi *hdmi = drm_bridge_to_hdmi(bridge);
+
+	return hdmi4_cec_init(hdmi->pdev, &hdmi->core, &hdmi->wp, conn);
+}
+
+static void hdmi4_bridge_cec_exit(struct drm_bridge *bridge)
+{
+	struct omap_hdmi *hdmi = drm_bridge_to_hdmi(bridge);
+
+	hdmi4_cec_uninit(&hdmi->core);
+}
+
 static const struct drm_bridge_funcs hdmi4_bridge_funcs = {
 	.attach = hdmi4_bridge_attach,
 	.mode_set = hdmi4_bridge_mode_set,
@@ -492,13 +507,15 @@ static const struct drm_bridge_funcs hdmi4_bridge_funcs = {
 	.atomic_disable = hdmi4_bridge_disable,
 	.hpd_notify = hdmi4_bridge_hpd_notify,
 	.get_edid = hdmi4_bridge_get_edid,
+	.cec_init = hdmi4_bridge_cec_init,
+	.cec_exit = hdmi4_bridge_cec_exit,
 };
 
 static void hdmi4_bridge_init(struct omap_hdmi *hdmi)
 {
 	hdmi->bridge.funcs = &hdmi4_bridge_funcs;
 	hdmi->bridge.of_node = hdmi->pdev->dev.of_node;
-	hdmi->bridge.ops = DRM_BRIDGE_OP_EDID;
+	hdmi->bridge.ops = DRM_BRIDGE_OP_EDID | DRM_BRIDGE_OP_CEC;
 	hdmi->bridge.type = DRM_MODE_CONNECTOR_HDMIA;
 
 	drm_bridge_add(&hdmi->bridge);
@@ -647,14 +664,10 @@ static int hdmi4_bind(struct device *dev, struct device *master, void *data)
 	if (r)
 		goto err_runtime_put;
 
-	r = hdmi4_cec_init(hdmi->pdev, &hdmi->core, &hdmi->wp);
-	if (r)
-		goto err_pll_uninit;
-
 	r = hdmi_audio_register(hdmi);
 	if (r) {
 		DSSERR("Registering HDMI audio failed\n");
-		goto err_cec_uninit;
+		goto err_pll_uninit;
 	}
 
 	hdmi->debugfs = dss_debugfs_create_file(dss, "hdmi", hdmi_dump_regs,
@@ -664,8 +677,6 @@ static int hdmi4_bind(struct device *dev, struct device *master, void *data)
 
 	return 0;
 
-err_cec_uninit:
-	hdmi4_cec_uninit(&hdmi->core);
 err_pll_uninit:
 	hdmi_pll_uninit(&hdmi->pll);
 err_runtime_put:
@@ -682,7 +693,6 @@ static void hdmi4_unbind(struct device *dev, struct device *master, void *data)
 	if (hdmi->audio_pdev)
 		platform_device_unregister(hdmi->audio_pdev);
 
-	hdmi4_cec_uninit(&hdmi->core);
 	hdmi_pll_uninit(&hdmi->pll);
 }
 
diff --git a/drivers/gpu/drm/omapdrm/dss/hdmi4_cec.c b/drivers/gpu/drm/omapdrm/dss/hdmi4_cec.c
index 43592c1cf081..64f5ccd0f11b 100644
--- a/drivers/gpu/drm/omapdrm/dss/hdmi4_cec.c
+++ b/drivers/gpu/drm/omapdrm/dss/hdmi4_cec.c
@@ -335,10 +335,10 @@ void hdmi4_cec_set_phys_addr(struct hdmi_core_data *core, u16 pa)
 }
 
 int hdmi4_cec_init(struct platform_device *pdev, struct hdmi_core_data *core,
-		  struct hdmi_wp_data *wp)
+		   struct hdmi_wp_data *wp, struct drm_connector *conn)
 {
-	const u32 caps = CEC_CAP_TRANSMIT | CEC_CAP_LOG_ADDRS |
-			 CEC_CAP_PASSTHROUGH | CEC_CAP_RC;
+	const u32 caps = CEC_CAP_DEFAULTS | CEC_CAP_CONNECTOR_INFO;
+	struct cec_connector_info conn_info;
 	int ret;
 
 	core->adap = cec_allocate_adapter(&hdmi_cec_adap_ops, core,
@@ -346,6 +346,8 @@ int hdmi4_cec_init(struct platform_device *pdev, struct hdmi_core_data *core,
 	ret = PTR_ERR_OR_ZERO(core->adap);
 	if (ret < 0)
 		return ret;
+	cec_fill_conn_info_from_drm(&conn_info, conn);
+	cec_s_conn_info(core->adap, &conn_info);
 	core->wp = wp;
 
 	/* Disable clock initially, hdmi_cec_adap_enable() manages it */
diff --git a/drivers/gpu/drm/omapdrm/dss/hdmi4_cec.h b/drivers/gpu/drm/omapdrm/dss/hdmi4_cec.h
index 0292337c97cc..b59a54c3040e 100644
--- a/drivers/gpu/drm/omapdrm/dss/hdmi4_cec.h
+++ b/drivers/gpu/drm/omapdrm/dss/hdmi4_cec.h
@@ -29,7 +29,7 @@ struct platform_device;
 void hdmi4_cec_set_phys_addr(struct hdmi_core_data *core, u16 pa);
 void hdmi4_cec_irq(struct hdmi_core_data *core);
 int hdmi4_cec_init(struct platform_device *pdev, struct hdmi_core_data *core,
-		  struct hdmi_wp_data *wp);
+		   struct hdmi_wp_data *wp, struct drm_connector *conn);
 void hdmi4_cec_uninit(struct hdmi_core_data *core);
 #else
 static inline void hdmi4_cec_set_phys_addr(struct hdmi_core_data *core, u16 pa)
@@ -41,8 +41,9 @@ static inline void hdmi4_cec_irq(struct hdmi_core_data *core)
 }
 
 static inline int hdmi4_cec_init(struct platform_device *pdev,
-				struct hdmi_core_data *core,
-				struct hdmi_wp_data *wp)
+				 struct hdmi_core_data *core,
+				 struct hdmi_wp_data *wp,
+				 struct drm_connector *conn)
 {
 	return 0;
 }
-- 
2.30.0

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 2/5] drm/omap: hdmi4: switch to the cec bridge ops

[Tomi Valkeinen]

On 11/02/2021 12:37, Hans Verkuil wrote:
> Implement the new CEC bridge ops. This makes it possible to associate
> a CEC adapter with a drm connector, which helps userspace determine
> with cec device node belongs to which drm connector.
> 
> Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
> ---
>  drivers/gpu/drm/omapdrm/dss/hdmi4.c     | 28 +++++++++++++++++--------
>  drivers/gpu/drm/omapdrm/dss/hdmi4_cec.c |  8 ++++---
>  drivers/gpu/drm/omapdrm/dss/hdmi4_cec.h |  7 ++++---
>  3 files changed, 28 insertions(+), 15 deletions(-)

Reviewed-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>

 Tomi

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 2/5] drm/omap: hdmi4: switch to the cec bridge ops

[Laurent Pinchart]

Hi Hans,

Thank you for the patch.

On Thu, Feb 11, 2021 at 11:37:00AM +0100, Hans Verkuil wrote:
> Implement the new CEC bridge ops. This makes it possible to associate
> a CEC adapter with a drm connector, which helps userspace determine
> with cec device node belongs to which drm connector.
> 
> Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
> ---
>  drivers/gpu/drm/omapdrm/dss/hdmi4.c     | 28 +++++++++++++++++--------
>  drivers/gpu/drm/omapdrm/dss/hdmi4_cec.c |  8 ++++---
>  drivers/gpu/drm/omapdrm/dss/hdmi4_cec.h |  7 ++++---
>  3 files changed, 28 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/gpu/drm/omapdrm/dss/hdmi4.c b/drivers/gpu/drm/omapdrm/dss/hdmi4.c
> index 8de41e74e8f8..765379380d4b 100644
> --- a/drivers/gpu/drm/omapdrm/dss/hdmi4.c
> +++ b/drivers/gpu/drm/omapdrm/dss/hdmi4.c
> @@ -482,6 +482,21 @@ static struct edid *hdmi4_bridge_get_edid(struct drm_bridge *bridge,
>  	return edid;
>  }
>  
> +static int hdmi4_bridge_cec_init(struct drm_bridge *bridge,
> +				 struct drm_connector *conn)
> +{
> +	struct omap_hdmi *hdmi = drm_bridge_to_hdmi(bridge);
> +
> +	return hdmi4_cec_init(hdmi->pdev, &hdmi->core, &hdmi->wp, conn);
> +}
> +
> +static void hdmi4_bridge_cec_exit(struct drm_bridge *bridge)
> +{
> +	struct omap_hdmi *hdmi = drm_bridge_to_hdmi(bridge);
> +
> +	hdmi4_cec_uninit(&hdmi->core);
> +}
> +
>  static const struct drm_bridge_funcs hdmi4_bridge_funcs = {
>  	.attach = hdmi4_bridge_attach,
>  	.mode_set = hdmi4_bridge_mode_set,
> @@ -492,13 +507,15 @@ static const struct drm_bridge_funcs hdmi4_bridge_funcs = {
>  	.atomic_disable = hdmi4_bridge_disable,
>  	.hpd_notify = hdmi4_bridge_hpd_notify,
>  	.get_edid = hdmi4_bridge_get_edid,
> +	.cec_init = hdmi4_bridge_cec_init,
> +	.cec_exit = hdmi4_bridge_cec_exit,
>  };
>  
>  static void hdmi4_bridge_init(struct omap_hdmi *hdmi)
>  {
>  	hdmi->bridge.funcs = &hdmi4_bridge_funcs;
>  	hdmi->bridge.of_node = hdmi->pdev->dev.of_node;
> -	hdmi->bridge.ops = DRM_BRIDGE_OP_EDID;
> +	hdmi->bridge.ops = DRM_BRIDGE_OP_EDID | DRM_BRIDGE_OP_CEC;
>  	hdmi->bridge.type = DRM_MODE_CONNECTOR_HDMIA;
>  
>  	drm_bridge_add(&hdmi->bridge);
> @@ -647,14 +664,10 @@ static int hdmi4_bind(struct device *dev, struct device *master, void *data)
>  	if (r)
>  		goto err_runtime_put;
>  
> -	r = hdmi4_cec_init(hdmi->pdev, &hdmi->core, &hdmi->wp);
> -	if (r)
> -		goto err_pll_uninit;

I'm wondering ifwe need to delay the creation of the CEC adapter until
the DRM connector is ready, or if we could only delay its registration ?
Catching errors related to adapter creation early could be nice, the
more error we have to handle at DRM bridge connector creation time, the
more complex the error handling will be for bridge support.

> -
>  	r = hdmi_audio_register(hdmi);
>  	if (r) {
>  		DSSERR("Registering HDMI audio failed\n");
> -		goto err_cec_uninit;
> +		goto err_pll_uninit;
>  	}
>  
>  	hdmi->debugfs = dss_debugfs_create_file(dss, "hdmi", hdmi_dump_regs,
> @@ -664,8 +677,6 @@ static int hdmi4_bind(struct device *dev, struct device *master, void *data)
>  
>  	return 0;
>  
> -err_cec_uninit:
> -	hdmi4_cec_uninit(&hdmi->core);
>  err_pll_uninit:
>  	hdmi_pll_uninit(&hdmi->pll);
>  err_runtime_put:
> @@ -682,7 +693,6 @@ static void hdmi4_unbind(struct device *dev, struct device *master, void *data)
>  	if (hdmi->audio_pdev)
>  		platform_device_unregister(hdmi->audio_pdev);
>  
> -	hdmi4_cec_uninit(&hdmi->core);
>  	hdmi_pll_uninit(&hdmi->pll);
>  }
>  
> diff --git a/drivers/gpu/drm/omapdrm/dss/hdmi4_cec.c b/drivers/gpu/drm/omapdrm/dss/hdmi4_cec.c
> index 43592c1cf081..64f5ccd0f11b 100644
> --- a/drivers/gpu/drm/omapdrm/dss/hdmi4_cec.c
> +++ b/drivers/gpu/drm/omapdrm/dss/hdmi4_cec.c
> @@ -335,10 +335,10 @@ void hdmi4_cec_set_phys_addr(struct hdmi_core_data *core, u16 pa)
>  }
>  
>  int hdmi4_cec_init(struct platform_device *pdev, struct hdmi_core_data *core,
> -		  struct hdmi_wp_data *wp)
> +		   struct hdmi_wp_data *wp, struct drm_connector *conn)
>  {
> -	const u32 caps = CEC_CAP_TRANSMIT | CEC_CAP_LOG_ADDRS |
> -			 CEC_CAP_PASSTHROUGH | CEC_CAP_RC;
> +	const u32 caps = CEC_CAP_DEFAULTS | CEC_CAP_CONNECTOR_INFO;
> +	struct cec_connector_info conn_info;
>  	int ret;
>  
>  	core->adap = cec_allocate_adapter(&hdmi_cec_adap_ops, core,
> @@ -346,6 +346,8 @@ int hdmi4_cec_init(struct platform_device *pdev, struct hdmi_core_data *core,
>  	ret = PTR_ERR_OR_ZERO(core->adap);
>  	if (ret < 0)
>  		return ret;
> +	cec_fill_conn_info_from_drm(&conn_info, conn);
> +	cec_s_conn_info(core->adap, &conn_info);
>  	core->wp = wp;
>  
>  	/* Disable clock initially, hdmi_cec_adap_enable() manages it */
> diff --git a/drivers/gpu/drm/omapdrm/dss/hdmi4_cec.h b/drivers/gpu/drm/omapdrm/dss/hdmi4_cec.h
> index 0292337c97cc..b59a54c3040e 100644
> --- a/drivers/gpu/drm/omapdrm/dss/hdmi4_cec.h
> +++ b/drivers/gpu/drm/omapdrm/dss/hdmi4_cec.h
> @@ -29,7 +29,7 @@ struct platform_device;
>  void hdmi4_cec_set_phys_addr(struct hdmi_core_data *core, u16 pa);
>  void hdmi4_cec_irq(struct hdmi_core_data *core);
>  int hdmi4_cec_init(struct platform_device *pdev, struct hdmi_core_data *core,
> -		  struct hdmi_wp_data *wp);
> +		   struct hdmi_wp_data *wp, struct drm_connector *conn);
>  void hdmi4_cec_uninit(struct hdmi_core_data *core);
>  #else
>  static inline void hdmi4_cec_set_phys_addr(struct hdmi_core_data *core, u16 pa)
> @@ -41,8 +41,9 @@ static inline void hdmi4_cec_irq(struct hdmi_core_data *core)
>  }
>  
>  static inline int hdmi4_cec_init(struct platform_device *pdev,
> -				struct hdmi_core_data *core,
> -				struct hdmi_wp_data *wp)
> +				 struct hdmi_core_data *core,
> +				 struct hdmi_wp_data *wp,
> +				 struct drm_connector *conn)
>  {
>  	return 0;
>  }

-- 
Regards,

Laurent Pinchart
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 2/5] drm/omap: hdmi4: switch to the cec bridge ops

[Hans Verkuil]

On 19/02/2021 13:07, Laurent Pinchart wrote:
> Hi Hans,
> 
> Thank you for the patch.
> 
> On Thu, Feb 11, 2021 at 11:37:00AM +0100, Hans Verkuil wrote:
>> Implement the new CEC bridge ops. This makes it possible to associate
>> a CEC adapter with a drm connector, which helps userspace determine
>> with cec device node belongs to which drm connector.
>>
>> Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
>> ---
>>  drivers/gpu/drm/omapdrm/dss/hdmi4.c     | 28 +++++++++++++++++--------
>>  drivers/gpu/drm/omapdrm/dss/hdmi4_cec.c |  8 ++++---
>>  drivers/gpu/drm/omapdrm/dss/hdmi4_cec.h |  7 ++++---
>>  3 files changed, 28 insertions(+), 15 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/omapdrm/dss/hdmi4.c b/drivers/gpu/drm/omapdrm/dss/hdmi4.c
>> index 8de41e74e8f8..765379380d4b 100644
>> --- a/drivers/gpu/drm/omapdrm/dss/hdmi4.c
>> +++ b/drivers/gpu/drm/omapdrm/dss/hdmi4.c
>> @@ -482,6 +482,21 @@ static struct edid *hdmi4_bridge_get_edid(struct drm_bridge *bridge,
>>  	return edid;
>>  }
>>  
>> +static int hdmi4_bridge_cec_init(struct drm_bridge *bridge,
>> +				 struct drm_connector *conn)
>> +{
>> +	struct omap_hdmi *hdmi = drm_bridge_to_hdmi(bridge);
>> +
>> +	return hdmi4_cec_init(hdmi->pdev, &hdmi->core, &hdmi->wp, conn);
>> +}
>> +
>> +static void hdmi4_bridge_cec_exit(struct drm_bridge *bridge)
>> +{
>> +	struct omap_hdmi *hdmi = drm_bridge_to_hdmi(bridge);
>> +
>> +	hdmi4_cec_uninit(&hdmi->core);
>> +}
>> +
>>  static const struct drm_bridge_funcs hdmi4_bridge_funcs = {
>>  	.attach = hdmi4_bridge_attach,
>>  	.mode_set = hdmi4_bridge_mode_set,
>> @@ -492,13 +507,15 @@ static const struct drm_bridge_funcs hdmi4_bridge_funcs = {
>>  	.atomic_disable = hdmi4_bridge_disable,
>>  	.hpd_notify = hdmi4_bridge_hpd_notify,
>>  	.get_edid = hdmi4_bridge_get_edid,
>> +	.cec_init = hdmi4_bridge_cec_init,
>> +	.cec_exit = hdmi4_bridge_cec_exit,
>>  };
>>  
>>  static void hdmi4_bridge_init(struct omap_hdmi *hdmi)
>>  {
>>  	hdmi->bridge.funcs = &hdmi4_bridge_funcs;
>>  	hdmi->bridge.of_node = hdmi->pdev->dev.of_node;
>> -	hdmi->bridge.ops = DRM_BRIDGE_OP_EDID;
>> +	hdmi->bridge.ops = DRM_BRIDGE_OP_EDID | DRM_BRIDGE_OP_CEC;
>>  	hdmi->bridge.type = DRM_MODE_CONNECTOR_HDMIA;
>>  
>>  	drm_bridge_add(&hdmi->bridge);
>> @@ -647,14 +664,10 @@ static int hdmi4_bind(struct device *dev, struct device *master, void *data)
>>  	if (r)
>>  		goto err_runtime_put;
>>  
>> -	r = hdmi4_cec_init(hdmi->pdev, &hdmi->core, &hdmi->wp);
>> -	if (r)
>> -		goto err_pll_uninit;
> 
> I'm wondering ifwe need to delay the creation of the CEC adapter until
> the DRM connector is ready, or if we could only delay its registration ?
> Catching errors related to adapter creation early could be nice, the
> more error we have to handle at DRM bridge connector creation time, the
> more complex the error handling will be for bridge support.

I feel that that is overkill, but if you really want this, just let me know.
Splitting it up would actually make it more complex for me since I would have
to check whether to call cec_unregister_adapter or cec_delete_adapter, depending
on whether the CEC registration was successful or not.

Regards,

	Hans

> 
>> -
>>  	r = hdmi_audio_register(hdmi);
>>  	if (r) {
>>  		DSSERR("Registering HDMI audio failed\n");
>> -		goto err_cec_uninit;
>> +		goto err_pll_uninit;
>>  	}
>>  
>>  	hdmi->debugfs = dss_debugfs_create_file(dss, "hdmi", hdmi_dump_regs,
>> @@ -664,8 +677,6 @@ static int hdmi4_bind(struct device *dev, struct device *master, void *data)
>>  
>>  	return 0;
>>  
>> -err_cec_uninit:
>> -	hdmi4_cec_uninit(&hdmi->core);
>>  err_pll_uninit:
>>  	hdmi_pll_uninit(&hdmi->pll);
>>  err_runtime_put:
>> @@ -682,7 +693,6 @@ static void hdmi4_unbind(struct device *dev, struct device *master, void *data)
>>  	if (hdmi->audio_pdev)
>>  		platform_device_unregister(hdmi->audio_pdev);
>>  
>> -	hdmi4_cec_uninit(&hdmi->core);
>>  	hdmi_pll_uninit(&hdmi->pll);
>>  }
>>  
>> diff --git a/drivers/gpu/drm/omapdrm/dss/hdmi4_cec.c b/drivers/gpu/drm/omapdrm/dss/hdmi4_cec.c
>> index 43592c1cf081..64f5ccd0f11b 100644
>> --- a/drivers/gpu/drm/omapdrm/dss/hdmi4_cec.c
>> +++ b/drivers/gpu/drm/omapdrm/dss/hdmi4_cec.c
>> @@ -335,10 +335,10 @@ void hdmi4_cec_set_phys_addr(struct hdmi_core_data *core, u16 pa)
>>  }
>>  
>>  int hdmi4_cec_init(struct platform_device *pdev, struct hdmi_core_data *core,
>> -		  struct hdmi_wp_data *wp)
>> +		   struct hdmi_wp_data *wp, struct drm_connector *conn)
>>  {
>> -	const u32 caps = CEC_CAP_TRANSMIT | CEC_CAP_LOG_ADDRS |
>> -			 CEC_CAP_PASSTHROUGH | CEC_CAP_RC;
>> +	const u32 caps = CEC_CAP_DEFAULTS | CEC_CAP_CONNECTOR_INFO;
>> +	struct cec_connector_info conn_info;
>>  	int ret;
>>  
>>  	core->adap = cec_allocate_adapter(&hdmi_cec_adap_ops, core,
>> @@ -346,6 +346,8 @@ int hdmi4_cec_init(struct platform_device *pdev, struct hdmi_core_data *core,
>>  	ret = PTR_ERR_OR_ZERO(core->adap);
>>  	if (ret < 0)
>>  		return ret;
>> +	cec_fill_conn_info_from_drm(&conn_info, conn);
>> +	cec_s_conn_info(core->adap, &conn_info);
>>  	core->wp = wp;
>>  
>>  	/* Disable clock initially, hdmi_cec_adap_enable() manages it */
>> diff --git a/drivers/gpu/drm/omapdrm/dss/hdmi4_cec.h b/drivers/gpu/drm/omapdrm/dss/hdmi4_cec.h
>> index 0292337c97cc..b59a54c3040e 100644
>> --- a/drivers/gpu/drm/omapdrm/dss/hdmi4_cec.h
>> +++ b/drivers/gpu/drm/omapdrm/dss/hdmi4_cec.h
>> @@ -29,7 +29,7 @@ struct platform_device;
>>  void hdmi4_cec_set_phys_addr(struct hdmi_core_data *core, u16 pa);
>>  void hdmi4_cec_irq(struct hdmi_core_data *core);
>>  int hdmi4_cec_init(struct platform_device *pdev, struct hdmi_core_data *core,
>> -		  struct hdmi_wp_data *wp);
>> +		   struct hdmi_wp_data *wp, struct drm_connector *conn);
>>  void hdmi4_cec_uninit(struct hdmi_core_data *core);
>>  #else
>>  static inline void hdmi4_cec_set_phys_addr(struct hdmi_core_data *core, u16 pa)
>> @@ -41,8 +41,9 @@ static inline void hdmi4_cec_irq(struct hdmi_core_data *core)
>>  }
>>  
>>  static inline int hdmi4_cec_init(struct platform_device *pdev,
>> -				struct hdmi_core_data *core,
>> -				struct hdmi_wp_data *wp)
>> +				 struct hdmi_core_data *core,
>> +				 struct hdmi_wp_data *wp,
>> +				 struct drm_connector *conn)
>>  {
>>  	return 0;
>>  }
> 

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 2/5] drm/omap: hdmi4: switch to the cec bridge ops

[Laurent Pinchart]

Hi Hans,

On Mon, Mar 01, 2021 at 12:07:56PM +0100, Hans Verkuil wrote:
> On 19/02/2021 13:07, Laurent Pinchart wrote:
> > On Thu, Feb 11, 2021 at 11:37:00AM +0100, Hans Verkuil wrote:
> >> Implement the new CEC bridge ops. This makes it possible to associate
> >> a CEC adapter with a drm connector, which helps userspace determine
> >> with cec device node belongs to which drm connector.
> >>
> >> Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
> >> ---
> >>  drivers/gpu/drm/omapdrm/dss/hdmi4.c     | 28 +++++++++++++++++--------
> >>  drivers/gpu/drm/omapdrm/dss/hdmi4_cec.c |  8 ++++---
> >>  drivers/gpu/drm/omapdrm/dss/hdmi4_cec.h |  7 ++++---
> >>  3 files changed, 28 insertions(+), 15 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/omapdrm/dss/hdmi4.c b/drivers/gpu/drm/omapdrm/dss/hdmi4.c
> >> index 8de41e74e8f8..765379380d4b 100644
> >> --- a/drivers/gpu/drm/omapdrm/dss/hdmi4.c
> >> +++ b/drivers/gpu/drm/omapdrm/dss/hdmi4.c
> >> @@ -482,6 +482,21 @@ static struct edid *hdmi4_bridge_get_edid(struct drm_bridge *bridge,
> >>  	return edid;
> >>  }
> >>  
> >> +static int hdmi4_bridge_cec_init(struct drm_bridge *bridge,
> >> +				 struct drm_connector *conn)
> >> +{
> >> +	struct omap_hdmi *hdmi = drm_bridge_to_hdmi(bridge);
> >> +
> >> +	return hdmi4_cec_init(hdmi->pdev, &hdmi->core, &hdmi->wp, conn);
> >> +}
> >> +
> >> +static void hdmi4_bridge_cec_exit(struct drm_bridge *bridge)
> >> +{
> >> +	struct omap_hdmi *hdmi = drm_bridge_to_hdmi(bridge);
> >> +
> >> +	hdmi4_cec_uninit(&hdmi->core);
> >> +}
> >> +
> >>  static const struct drm_bridge_funcs hdmi4_bridge_funcs = {
> >>  	.attach = hdmi4_bridge_attach,
> >>  	.mode_set = hdmi4_bridge_mode_set,
> >> @@ -492,13 +507,15 @@ static const struct drm_bridge_funcs hdmi4_bridge_funcs = {
> >>  	.atomic_disable = hdmi4_bridge_disable,
> >>  	.hpd_notify = hdmi4_bridge_hpd_notify,
> >>  	.get_edid = hdmi4_bridge_get_edid,
> >> +	.cec_init = hdmi4_bridge_cec_init,
> >> +	.cec_exit = hdmi4_bridge_cec_exit,
> >>  };
> >>  
> >>  static void hdmi4_bridge_init(struct omap_hdmi *hdmi)
> >>  {
> >>  	hdmi->bridge.funcs = &hdmi4_bridge_funcs;
> >>  	hdmi->bridge.of_node = hdmi->pdev->dev.of_node;
> >> -	hdmi->bridge.ops = DRM_BRIDGE_OP_EDID;
> >> +	hdmi->bridge.ops = DRM_BRIDGE_OP_EDID | DRM_BRIDGE_OP_CEC;
> >>  	hdmi->bridge.type = DRM_MODE_CONNECTOR_HDMIA;
> >>  
> >>  	drm_bridge_add(&hdmi->bridge);
> >> @@ -647,14 +664,10 @@ static int hdmi4_bind(struct device *dev, struct device *master, void *data)
> >>  	if (r)
> >>  		goto err_runtime_put;
> >>  
> >> -	r = hdmi4_cec_init(hdmi->pdev, &hdmi->core, &hdmi->wp);
> >> -	if (r)
> >> -		goto err_pll_uninit;
> > 
> > I'm wondering ifwe need to delay the creation of the CEC adapter until
> > the DRM connector is ready, or if we could only delay its registration ?
> > Catching errors related to adapter creation early could be nice, the
> > more error we have to handle at DRM bridge connector creation time, the
> > more complex the error handling will be for bridge support.
> 
> I feel that that is overkill, but if you really want this, just let me know.
> Splitting it up would actually make it more complex for me since I would have
> to check whether to call cec_unregister_adapter or cec_delete_adapter, depending
> on whether the CEC registration was successful or not.

I don't insist if you think it's not worth it.

> >> -
> >>  	r = hdmi_audio_register(hdmi);
> >>  	if (r) {
> >>  		DSSERR("Registering HDMI audio failed\n");
> >> -		goto err_cec_uninit;
> >> +		goto err_pll_uninit;
> >>  	}
> >>  
> >>  	hdmi->debugfs = dss_debugfs_create_file(dss, "hdmi", hdmi_dump_regs,
> >> @@ -664,8 +677,6 @@ static int hdmi4_bind(struct device *dev, struct device *master, void *data)
> >>  
> >>  	return 0;
> >>  
> >> -err_cec_uninit:
> >> -	hdmi4_cec_uninit(&hdmi->core);
> >>  err_pll_uninit:
> >>  	hdmi_pll_uninit(&hdmi->pll);
> >>  err_runtime_put:
> >> @@ -682,7 +693,6 @@ static void hdmi4_unbind(struct device *dev, struct device *master, void *data)
> >>  	if (hdmi->audio_pdev)
> >>  		platform_device_unregister(hdmi->audio_pdev);
> >>  
> >> -	hdmi4_cec_uninit(&hdmi->core);
> >>  	hdmi_pll_uninit(&hdmi->pll);
> >>  }
> >>  
> >> diff --git a/drivers/gpu/drm/omapdrm/dss/hdmi4_cec.c b/drivers/gpu/drm/omapdrm/dss/hdmi4_cec.c
> >> index 43592c1cf081..64f5ccd0f11b 100644
> >> --- a/drivers/gpu/drm/omapdrm/dss/hdmi4_cec.c
> >> +++ b/drivers/gpu/drm/omapdrm/dss/hdmi4_cec.c
> >> @@ -335,10 +335,10 @@ void hdmi4_cec_set_phys_addr(struct hdmi_core_data *core, u16 pa)
> >>  }
> >>  
> >>  int hdmi4_cec_init(struct platform_device *pdev, struct hdmi_core_data *core,
> >> -		  struct hdmi_wp_data *wp)
> >> +		   struct hdmi_wp_data *wp, struct drm_connector *conn)
> >>  {
> >> -	const u32 caps = CEC_CAP_TRANSMIT | CEC_CAP_LOG_ADDRS |
> >> -			 CEC_CAP_PASSTHROUGH | CEC_CAP_RC;
> >> +	const u32 caps = CEC_CAP_DEFAULTS | CEC_CAP_CONNECTOR_INFO;
> >> +	struct cec_connector_info conn_info;
> >>  	int ret;
> >>  
> >>  	core->adap = cec_allocate_adapter(&hdmi_cec_adap_ops, core,
> >> @@ -346,6 +346,8 @@ int hdmi4_cec_init(struct platform_device *pdev, struct hdmi_core_data *core,
> >>  	ret = PTR_ERR_OR_ZERO(core->adap);
> >>  	if (ret < 0)
> >>  		return ret;
> >> +	cec_fill_conn_info_from_drm(&conn_info, conn);
> >> +	cec_s_conn_info(core->adap, &conn_info);
> >>  	core->wp = wp;
> >>  
> >>  	/* Disable clock initially, hdmi_cec_adap_enable() manages it */
> >> diff --git a/drivers/gpu/drm/omapdrm/dss/hdmi4_cec.h b/drivers/gpu/drm/omapdrm/dss/hdmi4_cec.h
> >> index 0292337c97cc..b59a54c3040e 100644
> >> --- a/drivers/gpu/drm/omapdrm/dss/hdmi4_cec.h
> >> +++ b/drivers/gpu/drm/omapdrm/dss/hdmi4_cec.h
> >> @@ -29,7 +29,7 @@ struct platform_device;
> >>  void hdmi4_cec_set_phys_addr(struct hdmi_core_data *core, u16 pa);
> >>  void hdmi4_cec_irq(struct hdmi_core_data *core);
> >>  int hdmi4_cec_init(struct platform_device *pdev, struct hdmi_core_data *core,
> >> -		  struct hdmi_wp_data *wp);
> >> +		   struct hdmi_wp_data *wp, struct drm_connector *conn);
> >>  void hdmi4_cec_uninit(struct hdmi_core_data *core);
> >>  #else
> >>  static inline void hdmi4_cec_set_phys_addr(struct hdmi_core_data *core, u16 pa)
> >> @@ -41,8 +41,9 @@ static inline void hdmi4_cec_irq(struct hdmi_core_data *core)
> >>  }
> >>  
> >>  static inline int hdmi4_cec_init(struct platform_device *pdev,
> >> -				struct hdmi_core_data *core,
> >> -				struct hdmi_wp_data *wp)
> >> +				 struct hdmi_core_data *core,
> >> +				 struct hdmi_wp_data *wp,
> >> +				 struct drm_connector *conn)
> >>  {
> >>  	return 0;
> >>  }
> > 
> 

-- 
Regards,

Laurent Pinchart
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 3/5] drm/omap: hdmi4: simplify CEC Phys Addr handling

[Hans Verkuil]

Switch to using cec_s_phys_addr_from_edid() instead of a two-step process
of calling cec_get_edid_phys_addr() followed by cec_s_phys_addr().

Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
---
 drivers/gpu/drm/omapdrm/dss/hdmi4.c     | 13 ++-----------
 drivers/gpu/drm/omapdrm/dss/hdmi4_cec.c |  4 ++--
 drivers/gpu/drm/omapdrm/dss/hdmi4_cec.h |  5 +++--
 3 files changed, 7 insertions(+), 15 deletions(-)

diff --git a/drivers/gpu/drm/omapdrm/dss/hdmi4.c b/drivers/gpu/drm/omapdrm/dss/hdmi4.c
index 765379380d4b..ac142f870109 100644
--- a/drivers/gpu/drm/omapdrm/dss/hdmi4.c
+++ b/drivers/gpu/drm/omapdrm/dss/hdmi4.c
@@ -432,7 +432,7 @@ static void hdmi4_bridge_hpd_notify(struct drm_bridge *bridge,
 	struct omap_hdmi *hdmi = drm_bridge_to_hdmi(bridge);
 
 	if (status == connector_status_disconnected)
-		hdmi4_cec_set_phys_addr(&hdmi->core, CEC_PHYS_ADDR_INVALID);
+		hdmi4_cec_set_phys_addr(&hdmi->core, NULL);
 }
 
 static struct edid *hdmi4_bridge_get_edid(struct drm_bridge *bridge,
@@ -440,7 +440,6 @@ static struct edid *hdmi4_bridge_get_edid(struct drm_bridge *bridge,
 {
 	struct omap_hdmi *hdmi = drm_bridge_to_hdmi(bridge);
 	struct edid *edid = NULL;
-	unsigned int cec_addr;
 	bool need_enable;
 	int r;
 
@@ -466,15 +465,7 @@ static struct edid *hdmi4_bridge_get_edid(struct drm_bridge *bridge,
 	hdmi_runtime_put(hdmi);
 	mutex_unlock(&hdmi->lock);
 
-	if (edid && edid->extensions) {
-		unsigned int len = (edid->extensions + 1) * EDID_LENGTH;
-
-		cec_addr = cec_get_edid_phys_addr((u8 *)edid, len, NULL);
-	} else {
-		cec_addr = CEC_PHYS_ADDR_INVALID;
-	}
-
-	hdmi4_cec_set_phys_addr(&hdmi->core, cec_addr);
+	hdmi4_cec_set_phys_addr(&hdmi->core, edid);
 
 	if (need_enable)
 		hdmi4_core_disable(&hdmi->core);
diff --git a/drivers/gpu/drm/omapdrm/dss/hdmi4_cec.c b/drivers/gpu/drm/omapdrm/dss/hdmi4_cec.c
index 64f5ccd0f11b..68b4c84e1864 100644
--- a/drivers/gpu/drm/omapdrm/dss/hdmi4_cec.c
+++ b/drivers/gpu/drm/omapdrm/dss/hdmi4_cec.c
@@ -329,9 +329,9 @@ static const struct cec_adap_ops hdmi_cec_adap_ops = {
 	.adap_transmit = hdmi_cec_adap_transmit,
 };
 
-void hdmi4_cec_set_phys_addr(struct hdmi_core_data *core, u16 pa)
+void hdmi4_cec_set_phys_addr(struct hdmi_core_data *core, struct edid *edid)
 {
-	cec_s_phys_addr(core->adap, pa, false);
+	cec_s_phys_addr_from_edid(core->adap, edid);
 }
 
 int hdmi4_cec_init(struct platform_device *pdev, struct hdmi_core_data *core,
diff --git a/drivers/gpu/drm/omapdrm/dss/hdmi4_cec.h b/drivers/gpu/drm/omapdrm/dss/hdmi4_cec.h
index b59a54c3040e..16bf259643b7 100644
--- a/drivers/gpu/drm/omapdrm/dss/hdmi4_cec.h
+++ b/drivers/gpu/drm/omapdrm/dss/hdmi4_cec.h
@@ -26,13 +26,14 @@ struct platform_device;
 
 /* HDMI CEC funcs */
 #ifdef CONFIG_OMAP4_DSS_HDMI_CEC
-void hdmi4_cec_set_phys_addr(struct hdmi_core_data *core, u16 pa);
+void hdmi4_cec_set_phys_addr(struct hdmi_core_data *core, struct edid *edid);
 void hdmi4_cec_irq(struct hdmi_core_data *core);
 int hdmi4_cec_init(struct platform_device *pdev, struct hdmi_core_data *core,
 		   struct hdmi_wp_data *wp, struct drm_connector *conn);
 void hdmi4_cec_uninit(struct hdmi_core_data *core);
 #else
-static inline void hdmi4_cec_set_phys_addr(struct hdmi_core_data *core, u16 pa)
+static inline void hdmi4_cec_set_phys_addr(struct hdmi_core_data *core,
+					   struct edid *edid)
 {
 }
 
-- 
2.30.0

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 3/5] drm/omap: hdmi4: simplify CEC Phys Addr handling

[Tomi Valkeinen]

On 11/02/2021 12:37, Hans Verkuil wrote:
> Switch to using cec_s_phys_addr_from_edid() instead of a two-step process
> of calling cec_get_edid_phys_addr() followed by cec_s_phys_addr().
> 
> Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
> ---
>  drivers/gpu/drm/omapdrm/dss/hdmi4.c     | 13 ++-----------
>  drivers/gpu/drm/omapdrm/dss/hdmi4_cec.c |  4 ++--
>  drivers/gpu/drm/omapdrm/dss/hdmi4_cec.h |  5 +++--
>  3 files changed, 7 insertions(+), 15 deletions(-)

Reviewed-by: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>

 Tomi
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 4/5] drm/omap: hdmi5: add CEC support

[Hans Verkuil]

Add HDMI CEC support for OMAP5.

Many thanks to Tomi for helping out how to enable CEC for omap5.

Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
Thanks-to: Tomi Valkeinen <tomi.valkeinen@iki.fi>
---
 drivers/gpu/drm/omapdrm/dss/Kconfig      |   8 +
 drivers/gpu/drm/omapdrm/dss/Makefile     |   1 +
 drivers/gpu/drm/omapdrm/dss/hdmi.h       |   1 +
 drivers/gpu/drm/omapdrm/dss/hdmi5.c      |  63 +++++--
 drivers/gpu/drm/omapdrm/dss/hdmi5_cec.c  | 201 +++++++++++++++++++++++
 drivers/gpu/drm/omapdrm/dss/hdmi5_cec.h  |  42 +++++
 drivers/gpu/drm/omapdrm/dss/hdmi5_core.c |  28 +++-
 drivers/gpu/drm/omapdrm/dss/hdmi5_core.h |  33 +++-
 8 files changed, 358 insertions(+), 19 deletions(-)
 create mode 100644 drivers/gpu/drm/omapdrm/dss/hdmi5_cec.c
 create mode 100644 drivers/gpu/drm/omapdrm/dss/hdmi5_cec.h

diff --git a/drivers/gpu/drm/omapdrm/dss/Kconfig b/drivers/gpu/drm/omapdrm/dss/Kconfig
index e11b258a2294..67a1ba14703b 100644
--- a/drivers/gpu/drm/omapdrm/dss/Kconfig
+++ b/drivers/gpu/drm/omapdrm/dss/Kconfig
@@ -83,6 +83,14 @@ config OMAP5_DSS_HDMI
 	  Definition Multimedia Interface. See https://www.hdmi.org/ for HDMI
 	  specification.
 
+config OMAP5_DSS_HDMI_CEC
+	bool "Enable HDMI CEC support for OMAP5"
+	depends on OMAP5_DSS_HDMI
+	select CEC_CORE
+	default y
+	help
+	  When selected the HDMI transmitter will support the CEC feature.
+
 config OMAP2_DSS_SDI
 	bool "SDI support"
 	default n
diff --git a/drivers/gpu/drm/omapdrm/dss/Makefile b/drivers/gpu/drm/omapdrm/dss/Makefile
index f967e6948f2e..94fe0fa3b3c2 100644
--- a/drivers/gpu/drm/omapdrm/dss/Makefile
+++ b/drivers/gpu/drm/omapdrm/dss/Makefile
@@ -17,4 +17,5 @@ omapdss-$(CONFIG_OMAP2_DSS_HDMI_COMMON) += hdmi_common.o hdmi_wp.o hdmi_pll.o \
 omapdss-$(CONFIG_OMAP4_DSS_HDMI) += hdmi4.o hdmi4_core.o
 omapdss-$(CONFIG_OMAP4_DSS_HDMI_CEC) += hdmi4_cec.o
 omapdss-$(CONFIG_OMAP5_DSS_HDMI) += hdmi5.o hdmi5_core.o
+omapdss-$(CONFIG_OMAP5_DSS_HDMI_CEC) += hdmi5_cec.o
 ccflags-$(CONFIG_OMAP2_DSS_DEBUG) += -DDEBUG
diff --git a/drivers/gpu/drm/omapdrm/dss/hdmi.h b/drivers/gpu/drm/omapdrm/dss/hdmi.h
index c4a4e07f0b99..72d8ae441da6 100644
--- a/drivers/gpu/drm/omapdrm/dss/hdmi.h
+++ b/drivers/gpu/drm/omapdrm/dss/hdmi.h
@@ -261,6 +261,7 @@ struct hdmi_core_data {
 	struct hdmi_wp_data *wp;
 	unsigned int core_pwr_cnt;
 	struct cec_adapter *adap;
+	struct clk *cec_clk;
 };
 
 static inline void hdmi_write_reg(void __iomem *base_addr, const u32 idx,
diff --git a/drivers/gpu/drm/omapdrm/dss/hdmi5.c b/drivers/gpu/drm/omapdrm/dss/hdmi5.c
index 54e5cb5aa52d..b674d8ba173f 100644
--- a/drivers/gpu/drm/omapdrm/dss/hdmi5.c
+++ b/drivers/gpu/drm/omapdrm/dss/hdmi5.c
@@ -29,12 +29,14 @@
 #include <linux/of.h>
 #include <linux/of_graph.h>
 #include <sound/omap-hdmi-audio.h>
+#include <media/cec.h>
 
 #include <drm/drm_atomic.h>
 #include <drm/drm_atomic_state_helper.h>
 
 #include "omapdss.h"
 #include "hdmi5_core.h"
+#include "hdmi5_cec.h"
 #include "dss.h"
 
 static int hdmi_runtime_get(struct omap_hdmi *hdmi)
@@ -104,6 +106,10 @@ static irqreturn_t hdmi_irq_handler(int irq, void *data)
 	} else if (irqstatus & HDMI_IRQ_LINK_DISCONNECT) {
 		hdmi_wp_set_phy_pwr(wp, HDMI_PHYPWRCMD_LDOON);
 	}
+	if (irqstatus & HDMI_IRQ_CORE) {
+		hdmi5_cec_irq(&hdmi->core);
+		hdmi5_core_handle_irqs(&hdmi->core);
+	}
 
 	return IRQ_HANDLED;
 }
@@ -112,9 +118,12 @@ static int hdmi_power_on_core(struct omap_hdmi *hdmi)
 {
 	int r;
 
+	if (hdmi->core.core_pwr_cnt++)
+		return 0;
+
 	r = regulator_enable(hdmi->vdda_reg);
 	if (r)
-		return r;
+		goto err_reg_enable;
 
 	r = hdmi_runtime_get(hdmi);
 	if (r)
@@ -129,12 +138,17 @@ static int hdmi_power_on_core(struct omap_hdmi *hdmi)
 
 err_runtime_get:
 	regulator_disable(hdmi->vdda_reg);
+err_reg_enable:
+	hdmi->core.core_pwr_cnt--;
 
 	return r;
 }
 
 static void hdmi_power_off_core(struct omap_hdmi *hdmi)
 {
+	if (--hdmi->core.core_pwr_cnt)
+		return;
+
 	hdmi->core_enabled = false;
 
 	hdmi_runtime_put(hdmi);
@@ -168,7 +182,7 @@ static int hdmi_power_on_full(struct omap_hdmi *hdmi)
 		pc, &hdmi_cinfo);
 
 	/* disable and clear irqs */
-	hdmi_wp_clear_irqenable(&hdmi->wp, 0xffffffff);
+	hdmi_wp_clear_irqenable(&hdmi->wp, ~HDMI_IRQ_CORE);
 	hdmi_wp_set_irqstatus(&hdmi->wp,
 			hdmi_wp_get_irqstatus(&hdmi->wp));
 
@@ -225,7 +239,7 @@ static int hdmi_power_on_full(struct omap_hdmi *hdmi)
 
 static void hdmi_power_off_full(struct omap_hdmi *hdmi)
 {
-	hdmi_wp_clear_irqenable(&hdmi->wp, 0xffffffff);
+	hdmi_wp_clear_irqenable(&hdmi->wp, ~HDMI_IRQ_CORE);
 
 	hdmi_wp_video_stop(&hdmi->wp);
 
@@ -273,11 +287,11 @@ static void hdmi_stop_audio_stream(struct omap_hdmi *hd)
 	REG_FLD_MOD(hd->wp.base, HDMI_WP_SYSCONFIG, hd->wp_idlemode, 3, 2);
 }
 
-static int hdmi_core_enable(struct omap_hdmi *hdmi)
+int hdmi5_core_enable(struct omap_hdmi *hdmi)
 {
 	int r = 0;
 
-	DSSDBG("ENTER omapdss_hdmi_core_enable\n");
+	DSSDBG("ENTER %s\n", __func__);
 
 	mutex_lock(&hdmi->lock);
 
@@ -295,9 +309,9 @@ static int hdmi_core_enable(struct omap_hdmi *hdmi)
 	return r;
 }
 
-static void hdmi_core_disable(struct omap_hdmi *hdmi)
+void hdmi5_core_disable(struct omap_hdmi *hdmi)
 {
-	DSSDBG("Enter omapdss_hdmi_core_disable\n");
+	DSSDBG("ENTER %s\n", __func__);
 
 	mutex_lock(&hdmi->lock);
 
@@ -424,6 +438,15 @@ static void hdmi5_bridge_disable(struct drm_bridge *bridge,
 	mutex_unlock(&hdmi->lock);
 }
 
+static void hdmi5_bridge_hpd_notify(struct drm_bridge *bridge,
+				    enum drm_connector_status status)
+{
+	struct omap_hdmi *hdmi = drm_bridge_to_hdmi(bridge);
+
+	if (status == connector_status_disconnected)
+		hdmi5_cec_set_phys_addr(&hdmi->core, NULL);
+}
+
 static struct edid *hdmi5_bridge_get_edid(struct drm_bridge *bridge,
 					  struct drm_connector *connector)
 {
@@ -436,7 +459,7 @@ static struct edid *hdmi5_bridge_get_edid(struct drm_bridge *bridge,
 	need_enable = hdmi->core_enabled == false;
 
 	if (need_enable) {
-		r = hdmi_core_enable(hdmi);
+		r = hdmi5_core_enable(hdmi);
 		if (r)
 			return NULL;
 	}
@@ -460,12 +483,29 @@ static struct edid *hdmi5_bridge_get_edid(struct drm_bridge *bridge,
 	hdmi_runtime_put(hdmi);
 	mutex_unlock(&hdmi->lock);
 
+	hdmi5_cec_set_phys_addr(&hdmi->core, edid);
+
 	if (need_enable)
-		hdmi_core_disable(hdmi);
+		hdmi5_core_disable(hdmi);
 
 	return (struct edid *)edid;
 }
 
+static int hdmi5_bridge_cec_init(struct drm_bridge *bridge,
+				 struct drm_connector *conn)
+{
+	struct omap_hdmi *hdmi = drm_bridge_to_hdmi(bridge);
+
+	return hdmi5_cec_init(hdmi->pdev, &hdmi->core, &hdmi->wp, conn);
+}
+
+static void hdmi5_bridge_cec_exit(struct drm_bridge *bridge)
+{
+	struct omap_hdmi *hdmi = drm_bridge_to_hdmi(bridge);
+
+	hdmi5_cec_uninit(&hdmi->core);
+}
+
 static const struct drm_bridge_funcs hdmi5_bridge_funcs = {
 	.attach = hdmi5_bridge_attach,
 	.mode_set = hdmi5_bridge_mode_set,
@@ -474,14 +514,17 @@ static const struct drm_bridge_funcs hdmi5_bridge_funcs = {
 	.atomic_reset = drm_atomic_helper_bridge_reset,
 	.atomic_enable = hdmi5_bridge_enable,
 	.atomic_disable = hdmi5_bridge_disable,
+	.hpd_notify = hdmi5_bridge_hpd_notify,
 	.get_edid = hdmi5_bridge_get_edid,
+	.cec_init = hdmi5_bridge_cec_init,
+	.cec_exit = hdmi5_bridge_cec_exit,
 };
 
 static void hdmi5_bridge_init(struct omap_hdmi *hdmi)
 {
 	hdmi->bridge.funcs = &hdmi5_bridge_funcs;
 	hdmi->bridge.of_node = hdmi->pdev->dev.of_node;
-	hdmi->bridge.ops = DRM_BRIDGE_OP_EDID;
+	hdmi->bridge.ops = DRM_BRIDGE_OP_EDID | DRM_BRIDGE_OP_CEC;
 	hdmi->bridge.type = DRM_MODE_CONNECTOR_HDMIA;
 
 	drm_bridge_add(&hdmi->bridge);
diff --git a/drivers/gpu/drm/omapdrm/dss/hdmi5_cec.c b/drivers/gpu/drm/omapdrm/dss/hdmi5_cec.c
new file mode 100644
index 000000000000..26ef8f585b8d
--- /dev/null
+++ b/drivers/gpu/drm/omapdrm/dss/hdmi5_cec.c
@@ -0,0 +1,201 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * HDMI CEC
+ *
+ * Copyright 2019 Cisco Systems, Inc. and/or its affiliates. All rights reserved.
+ */
+#include <linux/interrupt.h>
+#include <linux/io.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/sched.h>
+#include <linux/slab.h>
+#include <linux/clk.h>
+
+#include "dss.h"
+#include "hdmi.h"
+#include "hdmi5_core.h"
+#include "hdmi5_cec.h"
+
+static int hdmi5_cec_log_addr(struct cec_adapter *adap, u8 logical_addr)
+{
+	struct hdmi_core_data *core = cec_get_drvdata(adap);
+	u8 v;
+
+	if (logical_addr == CEC_LOG_ADDR_INVALID) {
+		hdmi_write_reg(core->base, HDMI_CORE_CEC_ADDR_L, 0);
+		hdmi_write_reg(core->base, HDMI_CORE_CEC_ADDR_H, 0);
+		return 0;
+	}
+
+	if (logical_addr <= 7) {
+		v = hdmi_read_reg(core->base, HDMI_CORE_CEC_ADDR_L);
+		v |= 1 << logical_addr;
+		hdmi_write_reg(core->base, HDMI_CORE_CEC_ADDR_L, v);
+		v = hdmi_read_reg(core->base, HDMI_CORE_CEC_ADDR_H);
+		v |= 1 << 7;
+		hdmi_write_reg(core->base, HDMI_CORE_CEC_ADDR_H, v);
+	} else {
+		v = hdmi_read_reg(core->base, HDMI_CORE_CEC_ADDR_H);
+		v |= 1 << (logical_addr - 8);
+		v |= 1 << 7;
+		hdmi_write_reg(core->base, HDMI_CORE_CEC_ADDR_H, v);
+	}
+
+	return 0;
+}
+
+static int hdmi5_cec_transmit(struct cec_adapter *adap, u8 attempts,
+			      u32 signal_free_time, struct cec_msg *msg)
+{
+	struct hdmi_core_data *core = cec_get_drvdata(adap);
+	unsigned int i, ctrl;
+
+	switch (signal_free_time) {
+	case CEC_SIGNAL_FREE_TIME_RETRY:
+		ctrl = CEC_CTRL_RETRY;
+		break;
+	case CEC_SIGNAL_FREE_TIME_NEW_INITIATOR:
+	default:
+		ctrl = CEC_CTRL_NORMAL;
+		break;
+	case CEC_SIGNAL_FREE_TIME_NEXT_XFER:
+		ctrl = CEC_CTRL_IMMED;
+		break;
+	}
+
+	for (i = 0; i < msg->len; i++)
+		hdmi_write_reg(core->base,
+			       HDMI_CORE_CEC_TX_DATA0 + i * 4, msg->msg[i]);
+
+	hdmi_write_reg(core->base, HDMI_CORE_CEC_TX_CNT, msg->len);
+	hdmi_write_reg(core->base, HDMI_CORE_CEC_CTRL,
+		       ctrl | CEC_CTRL_START);
+
+	return 0;
+}
+
+void hdmi5_cec_irq(struct hdmi_core_data *core)
+{
+	struct cec_adapter *adap = core->adap;
+	unsigned int stat = hdmi_read_reg(core->base, HDMI_CORE_IH_CEC_STAT0);
+
+	if (stat == 0)
+		return;
+
+	hdmi_write_reg(core->base, HDMI_CORE_IH_CEC_STAT0, stat);
+
+	if (stat & CEC_STAT_ERROR_INIT)
+		cec_transmit_attempt_done(adap, CEC_TX_STATUS_ERROR);
+	else if (stat & CEC_STAT_DONE)
+		cec_transmit_attempt_done(adap, CEC_TX_STATUS_OK);
+	else if (stat & CEC_STAT_NACK)
+		cec_transmit_attempt_done(adap, CEC_TX_STATUS_NACK);
+
+	if (stat & CEC_STAT_EOM) {
+		struct cec_msg msg = {};
+		unsigned int len, i;
+
+		len = hdmi_read_reg(core->base, HDMI_CORE_CEC_RX_CNT);
+		if (len > sizeof(msg.msg))
+			len = sizeof(msg.msg);
+
+		for (i = 0; i < len; i++)
+			msg.msg[i] =
+				hdmi_read_reg(core->base,
+					      HDMI_CORE_CEC_RX_DATA0 + i * 4);
+
+		hdmi_write_reg(core->base, HDMI_CORE_CEC_LOCK, 0);
+
+		msg.len = len;
+		cec_received_msg(adap, &msg);
+	}
+}
+
+static int hdmi5_cec_enable(struct cec_adapter *adap, bool enable)
+{
+	struct hdmi_core_data *core = cec_get_drvdata(adap);
+	struct omap_hdmi *hdmi = container_of(core, struct omap_hdmi, core);
+	unsigned int irqs;
+	int err;
+
+	if (!enable) {
+		hdmi_write_reg(core->base, HDMI_CORE_CEC_MASK, ~0);
+		hdmi_write_reg(core->base, HDMI_CORE_IH_MUTE_CEC_STAT0, ~0);
+		hdmi_wp_clear_irqenable(core->wp, HDMI_IRQ_CORE);
+		hdmi_wp_set_irqstatus(core->wp, HDMI_IRQ_CORE);
+		REG_FLD_MOD(core->base, HDMI_CORE_MC_CLKDIS, 0x01, 5, 5);
+		hdmi5_core_disable(hdmi);
+		return 0;
+	}
+	err = hdmi5_core_enable(hdmi);
+	if (err)
+		return err;
+
+	REG_FLD_MOD(core->base, HDMI_CORE_MC_CLKDIS, 0x00, 5, 5);
+	hdmi_write_reg(core->base, HDMI_CORE_IH_I2CM_STAT0, ~0);
+	hdmi_write_reg(core->base, HDMI_CORE_IH_MUTE_I2CM_STAT0, ~0);
+	hdmi_write_reg(core->base, HDMI_CORE_CEC_CTRL, 0);
+	hdmi_write_reg(core->base, HDMI_CORE_IH_CEC_STAT0, ~0);
+	hdmi_write_reg(core->base, HDMI_CORE_CEC_LOCK, 0);
+	hdmi_write_reg(core->base, HDMI_CORE_CEC_TX_CNT, 0);
+
+	hdmi5_cec_log_addr(adap, CEC_LOG_ADDR_INVALID);
+
+	/* Enable HDMI core interrupts */
+	hdmi_wp_set_irqenable(core->wp, HDMI_IRQ_CORE);
+
+	irqs = CEC_STAT_ERROR_INIT | CEC_STAT_NACK | CEC_STAT_EOM |
+	       CEC_STAT_DONE;
+	hdmi_write_reg(core->base, HDMI_CORE_CEC_MASK, ~irqs);
+	hdmi_write_reg(core->base, HDMI_CORE_IH_MUTE_CEC_STAT0, ~irqs);
+	return 0;
+}
+
+static const struct cec_adap_ops hdmi5_cec_ops = {
+	.adap_enable = hdmi5_cec_enable,
+	.adap_log_addr = hdmi5_cec_log_addr,
+	.adap_transmit = hdmi5_cec_transmit,
+};
+
+void hdmi5_cec_set_phys_addr(struct hdmi_core_data *core, struct edid *edid)
+{
+	cec_s_phys_addr_from_edid(core->adap, edid);
+}
+
+int hdmi5_cec_init(struct platform_device *pdev, struct hdmi_core_data *core,
+		   struct hdmi_wp_data *wp, struct drm_connector *conn)
+{
+	const u32 caps = CEC_CAP_DEFAULTS | CEC_CAP_CONNECTOR_INFO;
+	struct cec_connector_info conn_info;
+	unsigned int ret;
+
+	core->cec_clk = devm_clk_get(&pdev->dev, "cec");
+	if (IS_ERR(core->cec_clk))
+		return PTR_ERR(core->cec_clk);
+	ret = clk_prepare_enable(core->cec_clk);
+	if (ret)
+		return ret;
+
+	core->adap = cec_allocate_adapter(&hdmi5_cec_ops, core,
+					  "omap5", caps, CEC_MAX_LOG_ADDRS);
+	ret = PTR_ERR_OR_ZERO(core->adap);
+	if (ret < 0)
+		return ret;
+	cec_fill_conn_info_from_drm(&conn_info, conn);
+	cec_s_conn_info(core->adap, &conn_info);
+	core->wp = wp;
+
+	ret = cec_register_adapter(core->adap, &pdev->dev);
+	if (ret < 0) {
+		cec_delete_adapter(core->adap);
+		return ret;
+	}
+	return 0;
+}
+
+void hdmi5_cec_uninit(struct hdmi_core_data *core)
+{
+	clk_disable_unprepare(core->cec_clk);
+	cec_unregister_adapter(core->adap);
+}
diff --git a/drivers/gpu/drm/omapdrm/dss/hdmi5_cec.h b/drivers/gpu/drm/omapdrm/dss/hdmi5_cec.h
new file mode 100644
index 000000000000..904541da46da
--- /dev/null
+++ b/drivers/gpu/drm/omapdrm/dss/hdmi5_cec.h
@@ -0,0 +1,42 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * HDMI header definition for OMAP5 HDMI CEC IP
+ *
+ * Copyright 2019 Cisco Systems, Inc. and/or its affiliates. All rights reserved.
+ */
+
+#ifndef _HDMI5_CEC_H_
+#define _HDMI5_CEC_H_
+
+/* HDMI CEC funcs */
+#ifdef CONFIG_OMAP5_DSS_HDMI_CEC
+void hdmi5_cec_set_phys_addr(struct hdmi_core_data *core,
+			     struct edid *edid);
+void hdmi5_cec_irq(struct hdmi_core_data *core);
+int hdmi5_cec_init(struct platform_device *pdev, struct hdmi_core_data *core,
+		   struct hdmi_wp_data *wp, struct drm_connector *conn);
+void hdmi5_cec_uninit(struct hdmi_core_data *core);
+#else
+static inline void hdmi5_cec_set_phys_addr(struct hdmi_core_data *core,
+					   struct edid *edid)
+{
+}
+
+static inline void hdmi5_cec_irq(struct hdmi_core_data *core)
+{
+}
+
+static inline int hdmi5_cec_init(struct platform_device *pdev,
+				 struct hdmi_core_data *core,
+				 struct hdmi_wp_data *wp,
+				 struct drm_connector *conn)
+{
+	return 0;
+}
+
+static inline void hdmi5_cec_uninit(struct hdmi_core_data *core)
+{
+}
+#endif
+
+#endif
diff --git a/drivers/gpu/drm/omapdrm/dss/hdmi5_core.c b/drivers/gpu/drm/omapdrm/dss/hdmi5_core.c
index 6cc2ad7a420c..13bc0f3d850b 100644
--- a/drivers/gpu/drm/omapdrm/dss/hdmi5_core.c
+++ b/drivers/gpu/drm/omapdrm/dss/hdmi5_core.c
@@ -229,6 +229,19 @@ void hdmi5_core_dump(struct hdmi_core_data *core, struct seq_file *s)
 	DUMPCORE(HDMI_CORE_I2CM_FS_SCL_LCNT_1_ADDR);
 	DUMPCORE(HDMI_CORE_I2CM_FS_SCL_LCNT_0_ADDR);
 	DUMPCORE(HDMI_CORE_I2CM_SDA_HOLD_ADDR);
+
+	DUMPCORE(HDMI_CORE_IH_CEC_STAT0);
+	DUMPCORE(HDMI_CORE_IH_MUTE_CEC_STAT0);
+	DUMPCORE(HDMI_CORE_CEC_CTRL);
+	DUMPCORE(HDMI_CORE_CEC_MASK);
+	DUMPCORE(HDMI_CORE_CEC_ADDR_L);
+	DUMPCORE(HDMI_CORE_CEC_ADDR_H);
+	DUMPCORE(HDMI_CORE_CEC_TX_CNT);
+	DUMPCORE(HDMI_CORE_CEC_RX_CNT);
+	DUMPCORE(HDMI_CORE_CEC_TX_DATA0);
+	DUMPCORE(HDMI_CORE_CEC_RX_DATA0);
+	DUMPCORE(HDMI_CORE_CEC_LOCK);
+	DUMPCORE(HDMI_CORE_CEC_WKUPCTRL);
 }
 
 static void hdmi_core_init(struct hdmi_core_vid_config *video_cfg,
@@ -513,8 +526,6 @@ static void hdmi_core_mask_interrupts(struct hdmi_core_data *core)
 	REG_FLD_MOD(base, HDMI_CORE_AUD_INT, 0x3, 3, 2);
 	REG_FLD_MOD(base, HDMI_CORE_AUD_GP_MASK, 0x3, 1, 0);
 
-	REG_FLD_MOD(base, HDMI_CORE_CEC_MASK, 0x7f, 6, 0);
-
 	REG_FLD_MOD(base, HDMI_CORE_I2CM_CTLINT, 0x1, 6, 6);
 	REG_FLD_MOD(base, HDMI_CORE_I2CM_CTLINT, 0x1, 2, 2);
 	REG_FLD_MOD(base, HDMI_CORE_I2CM_INT, 0x1, 2, 2);
@@ -532,8 +543,6 @@ static void hdmi_core_mask_interrupts(struct hdmi_core_data *core)
 
 	REG_FLD_MOD(base, HDMI_CORE_IH_AS_STAT0, 0x7, 2, 0);
 
-	REG_FLD_MOD(base, HDMI_CORE_IH_CEC_STAT0, 0x7f, 6, 0);
-
 	REG_FLD_MOD(base, HDMI_CORE_IH_I2CM_STAT0, 0x3, 1, 0);
 
 	REG_FLD_MOD(base, HDMI_CORE_IH_PHY_STAT0, 0xff, 7, 0);
@@ -549,13 +558,17 @@ int hdmi5_core_handle_irqs(struct hdmi_core_data *core)
 {
 	void __iomem *base = core->base;
 
+	/*
+	 * Clear all possible IRQ_CORE interrupts except for
+	 * HDMI_CORE_IH_I2CM_STAT0 (that interrupt is muted and
+	 * is handled by polling elsewhere) and HDMI_CORE_IH_CEC_STAT0
+	 * which is handled by the CEC code.
+	 */
 	REG_FLD_MOD(base, HDMI_CORE_IH_FC_STAT0, 0xff, 7, 0);
 	REG_FLD_MOD(base, HDMI_CORE_IH_FC_STAT1, 0xff, 7, 0);
 	REG_FLD_MOD(base, HDMI_CORE_IH_FC_STAT2, 0xff, 7, 0);
 	REG_FLD_MOD(base, HDMI_CORE_IH_AS_STAT0, 0xff, 7, 0);
 	REG_FLD_MOD(base, HDMI_CORE_IH_PHY_STAT0, 0xff, 7, 0);
-	REG_FLD_MOD(base, HDMI_CORE_IH_I2CM_STAT0, 0xff, 7, 0);
-	REG_FLD_MOD(base, HDMI_CORE_IH_CEC_STAT0, 0xff, 7, 0);
 	REG_FLD_MOD(base, HDMI_CORE_IH_VP_STAT0, 0xff, 7, 0);
 	REG_FLD_MOD(base, HDMI_CORE_IH_I2CMPHY_STAT0, 0xff, 7, 0);
 
@@ -879,5 +892,8 @@ int hdmi5_core_init(struct platform_device *pdev, struct hdmi_core_data *core)
 	if (IS_ERR(core->base))
 		return PTR_ERR(core->base);
 
+	REG_FLD_MOD(core->base, HDMI_CORE_CEC_MASK, 0x7f, 6, 0);
+	REG_FLD_MOD(core->base, HDMI_CORE_IH_CEC_STAT0, 0x7f, 6, 0);
+
 	return 0;
 }
diff --git a/drivers/gpu/drm/omapdrm/dss/hdmi5_core.h b/drivers/gpu/drm/omapdrm/dss/hdmi5_core.h
index 070cbf5fb57d..a83b634f6011 100644
--- a/drivers/gpu/drm/omapdrm/dss/hdmi5_core.h
+++ b/drivers/gpu/drm/omapdrm/dss/hdmi5_core.h
@@ -30,8 +30,18 @@
 #define HDMI_CORE_IH_PHY_STAT0			0x00410
 #define HDMI_CORE_IH_I2CM_STAT0			0x00414
 #define HDMI_CORE_IH_CEC_STAT0			0x00418
+#define CEC_STAT_DONE				BIT(0)
+#define CEC_STAT_EOM				BIT(1)
+#define CEC_STAT_NACK				BIT(2)
+#define CEC_STAT_ARBLOST			BIT(3)
+#define CEC_STAT_ERROR_INIT			BIT(4)
+#define CEC_STAT_ERROR_FOLL			BIT(5)
+#define CEC_STAT_WAKEUP				BIT(6)
+
 #define HDMI_CORE_IH_VP_STAT0			0x0041C
 #define HDMI_CORE_IH_I2CMPHY_STAT0		0x00420
+#define HDMI_CORE_IH_MUTE_I2CM_STAT0            0x00614
+#define HDMI_CORE_IH_MUTE_CEC_STAT0		0x00618
 #define HDMI_CORE_IH_MUTE			0x007FC
 
 /* HDMI Video Sampler */
@@ -233,9 +243,6 @@
 /* HDMI HDCP */
 #define HDMI_CORE_HDCP_MASK			0x14020
 
-/* HDMI CEC */
-#define HDMI_CORE_CEC_MASK			0x17408
-
 /* HDMI I2C Master */
 #define HDMI_CORE_I2CM_SLAVE			0x157C8
 #define HDMI_CORE_I2CM_ADDRESS			0x157CC
@@ -258,6 +265,24 @@
 #define HDMI_CORE_I2CM_FS_SCL_LCNT_0_ADDR	0x15810
 #define HDMI_CORE_I2CM_SDA_HOLD_ADDR		0x15814
 
+/* HDMI CEC */
+#define HDMI_CORE_CEC_CTRL			0x153C8
+#define CEC_CTRL_START				BIT(0)
+#define CEC_CTRL_FRAME_TYP			(3 << 1)
+#define CEC_CTRL_RETRY				(0 << 1)
+#define CEC_CTRL_NORMAL				(1 << 1)
+#define CEC_CTRL_IMMED				(2 << 1)
+
+#define HDMI_CORE_CEC_MASK			0x153D0
+#define HDMI_CORE_CEC_ADDR_L			0x153DC
+#define HDMI_CORE_CEC_ADDR_H			0x153E0
+#define HDMI_CORE_CEC_TX_CNT			0x153E4
+#define HDMI_CORE_CEC_RX_CNT			0x153E8
+#define HDMI_CORE_CEC_TX_DATA0			0x15408
+#define HDMI_CORE_CEC_RX_DATA0			0x15448
+#define HDMI_CORE_CEC_LOCK			0x15488
+#define HDMI_CORE_CEC_WKUPCTRL			0x1548C
+
 enum hdmi_core_packet_mode {
 	HDMI_PACKETMODERESERVEDVALUE = 0,
 	HDMI_PACKETMODE24BITPERPIXEL = 4,
@@ -290,6 +315,8 @@ int hdmi5_core_handle_irqs(struct hdmi_core_data *core);
 void hdmi5_configure(struct hdmi_core_data *core, struct hdmi_wp_data *wp,
 			struct hdmi_config *cfg);
 int hdmi5_core_init(struct platform_device *pdev, struct hdmi_core_data *core);
+int hdmi5_core_enable(struct omap_hdmi *hdmi);
+void hdmi5_core_disable(struct omap_hdmi *hdmi);
 
 int hdmi5_audio_config(struct hdmi_core_data *core, struct hdmi_wp_data *wp,
 			struct omap_dss_audio *audio, u32 pclk);
-- 
2.30.0

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 4/5] drm/omap: hdmi5: add CEC support

[Tomi Valkeinen]

Hi Hans,

On 11/02/2021 12:37, Hans Verkuil wrote:
> Add HDMI CEC support for OMAP5.
> 
> Many thanks to Tomi for helping out how to enable CEC for omap5.
> 
> Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
> Thanks-to: Tomi Valkeinen <tomi.valkeinen@iki.fi>
> ---
>  drivers/gpu/drm/omapdrm/dss/Kconfig      |   8 +
>  drivers/gpu/drm/omapdrm/dss/Makefile     |   1 +
>  drivers/gpu/drm/omapdrm/dss/hdmi.h       |   1 +
>  drivers/gpu/drm/omapdrm/dss/hdmi5.c      |  63 +++++--
>  drivers/gpu/drm/omapdrm/dss/hdmi5_cec.c  | 201 +++++++++++++++++++++++
>  drivers/gpu/drm/omapdrm/dss/hdmi5_cec.h  |  42 +++++
>  drivers/gpu/drm/omapdrm/dss/hdmi5_core.c |  28 +++-
>  drivers/gpu/drm/omapdrm/dss/hdmi5_core.h |  33 +++-
>  8 files changed, 358 insertions(+), 19 deletions(-)
>  create mode 100644 drivers/gpu/drm/omapdrm/dss/hdmi5_cec.c
>  create mode 100644 drivers/gpu/drm/omapdrm/dss/hdmi5_cec.h
> 
> diff --git a/drivers/gpu/drm/omapdrm/dss/Kconfig b/drivers/gpu/drm/omapdrm/dss/Kconfig
> index e11b258a2294..67a1ba14703b 100644
> --- a/drivers/gpu/drm/omapdrm/dss/Kconfig
> +++ b/drivers/gpu/drm/omapdrm/dss/Kconfig
> @@ -83,6 +83,14 @@ config OMAP5_DSS_HDMI
>  	  Definition Multimedia Interface. See https://www.hdmi.org/ for HDMI
>  	  specification.
>  
> +config OMAP5_DSS_HDMI_CEC
> +	bool "Enable HDMI CEC support for OMAP5"
> +	depends on OMAP5_DSS_HDMI
> +	select CEC_CORE
> +	default y
> +	help
> +	  When selected the HDMI transmitter will support the CEC feature.
> +
>  config OMAP2_DSS_SDI
>  	bool "SDI support"
>  	default n
> diff --git a/drivers/gpu/drm/omapdrm/dss/Makefile b/drivers/gpu/drm/omapdrm/dss/Makefile
> index f967e6948f2e..94fe0fa3b3c2 100644
> --- a/drivers/gpu/drm/omapdrm/dss/Makefile
> +++ b/drivers/gpu/drm/omapdrm/dss/Makefile
> @@ -17,4 +17,5 @@ omapdss-$(CONFIG_OMAP2_DSS_HDMI_COMMON) += hdmi_common.o hdmi_wp.o hdmi_pll.o \
>  omapdss-$(CONFIG_OMAP4_DSS_HDMI) += hdmi4.o hdmi4_core.o
>  omapdss-$(CONFIG_OMAP4_DSS_HDMI_CEC) += hdmi4_cec.o
>  omapdss-$(CONFIG_OMAP5_DSS_HDMI) += hdmi5.o hdmi5_core.o
> +omapdss-$(CONFIG_OMAP5_DSS_HDMI_CEC) += hdmi5_cec.o
>  ccflags-$(CONFIG_OMAP2_DSS_DEBUG) += -DDEBUG
> diff --git a/drivers/gpu/drm/omapdrm/dss/hdmi.h b/drivers/gpu/drm/omapdrm/dss/hdmi.h
> index c4a4e07f0b99..72d8ae441da6 100644
> --- a/drivers/gpu/drm/omapdrm/dss/hdmi.h
> +++ b/drivers/gpu/drm/omapdrm/dss/hdmi.h
> @@ -261,6 +261,7 @@ struct hdmi_core_data {
>  	struct hdmi_wp_data *wp;
>  	unsigned int core_pwr_cnt;
>  	struct cec_adapter *adap;
> +	struct clk *cec_clk;
>  };
>  
>  static inline void hdmi_write_reg(void __iomem *base_addr, const u32 idx,
> diff --git a/drivers/gpu/drm/omapdrm/dss/hdmi5.c b/drivers/gpu/drm/omapdrm/dss/hdmi5.c
> index 54e5cb5aa52d..b674d8ba173f 100644
> --- a/drivers/gpu/drm/omapdrm/dss/hdmi5.c
> +++ b/drivers/gpu/drm/omapdrm/dss/hdmi5.c
> @@ -29,12 +29,14 @@
>  #include <linux/of.h>
>  #include <linux/of_graph.h>
>  #include <sound/omap-hdmi-audio.h>
> +#include <media/cec.h>
>  
>  #include <drm/drm_atomic.h>
>  #include <drm/drm_atomic_state_helper.h>
>  
>  #include "omapdss.h"
>  #include "hdmi5_core.h"
> +#include "hdmi5_cec.h"
>  #include "dss.h"
>  
>  static int hdmi_runtime_get(struct omap_hdmi *hdmi)
> @@ -104,6 +106,10 @@ static irqreturn_t hdmi_irq_handler(int irq, void *data)
>  	} else if (irqstatus & HDMI_IRQ_LINK_DISCONNECT) {
>  		hdmi_wp_set_phy_pwr(wp, HDMI_PHYPWRCMD_LDOON);
>  	}

Empty line here, please.

> +	if (irqstatus & HDMI_IRQ_CORE) {
> +		hdmi5_cec_irq(&hdmi->core);
> +		hdmi5_core_handle_irqs(&hdmi->core);
> +	}

It's a bit odd to call two functions here. Would it work if
hdmi5_core_handle_irqs() would read and clear HDMI_CORE_IH_CEC_STAT0,
and call hdmi5_cec_irq() if the stat != 0 ?

And it would be nice if hdmi5_core.c would enable and disable core
interrupt, but maybe that can be left for later if the need ever comes
to handle other interrupts than cec.

>  
>  	return IRQ_HANDLED;
>  }
> @@ -112,9 +118,12 @@ static int hdmi_power_on_core(struct omap_hdmi *hdmi)
>  {
>  	int r;
>  
> +	if (hdmi->core.core_pwr_cnt++)
> +		return 0;
> +
>  	r = regulator_enable(hdmi->vdda_reg);
>  	if (r)
> -		return r;
> +		goto err_reg_enable;
>  
>  	r = hdmi_runtime_get(hdmi);
>  	if (r)
> @@ -129,12 +138,17 @@ static int hdmi_power_on_core(struct omap_hdmi *hdmi)
>  
>  err_runtime_get:
>  	regulator_disable(hdmi->vdda_reg);
> +err_reg_enable:
> +	hdmi->core.core_pwr_cnt--;
>  
>  	return r;
>  }
>  
>  static void hdmi_power_off_core(struct omap_hdmi *hdmi)
>  {
> +	if (--hdmi->core.core_pwr_cnt)
> +		return;
> +
>  	hdmi->core_enabled = false;
>  
>  	hdmi_runtime_put(hdmi);
> @@ -168,7 +182,7 @@ static int hdmi_power_on_full(struct omap_hdmi *hdmi)
>  		pc, &hdmi_cinfo);
>  
>  	/* disable and clear irqs */
> -	hdmi_wp_clear_irqenable(&hdmi->wp, 0xffffffff);
> +	hdmi_wp_clear_irqenable(&hdmi->wp, ~HDMI_IRQ_CORE);

I guess the point here is to not touch CORE interrupt, as hdmi5_cec.c
handles that? The line below will still clear the CORE interrupt status.

>  	hdmi_wp_set_irqstatus(&hdmi->wp,
>  			hdmi_wp_get_irqstatus(&hdmi->wp));
>  
> @@ -225,7 +239,7 @@ static int hdmi_power_on_full(struct omap_hdmi *hdmi)
>  
>  static void hdmi_power_off_full(struct omap_hdmi *hdmi)
>  {
> -	hdmi_wp_clear_irqenable(&hdmi->wp, 0xffffffff);
> +	hdmi_wp_clear_irqenable(&hdmi->wp, ~HDMI_IRQ_CORE);
>  
>  	hdmi_wp_video_stop(&hdmi->wp);
>  
> @@ -273,11 +287,11 @@ static void hdmi_stop_audio_stream(struct omap_hdmi *hd)
>  	REG_FLD_MOD(hd->wp.base, HDMI_WP_SYSCONFIG, hd->wp_idlemode, 3, 2);
>  }
>  
> -static int hdmi_core_enable(struct omap_hdmi *hdmi)
> +int hdmi5_core_enable(struct omap_hdmi *hdmi)
>  {
>  	int r = 0;
>  
> -	DSSDBG("ENTER omapdss_hdmi_core_enable\n");
> +	DSSDBG("ENTER %s\n", __func__);
>  
>  	mutex_lock(&hdmi->lock);
>  
> @@ -295,9 +309,9 @@ static int hdmi_core_enable(struct omap_hdmi *hdmi)
>  	return r;
>  }
>  
> -static void hdmi_core_disable(struct omap_hdmi *hdmi)
> +void hdmi5_core_disable(struct omap_hdmi *hdmi)
>  {
> -	DSSDBG("Enter omapdss_hdmi_core_disable\n");
> +	DSSDBG("ENTER %s\n", __func__);
>  
>  	mutex_lock(&hdmi->lock);
>  
> @@ -424,6 +438,15 @@ static void hdmi5_bridge_disable(struct drm_bridge *bridge,
>  	mutex_unlock(&hdmi->lock);
>  }
>  
> +static void hdmi5_bridge_hpd_notify(struct drm_bridge *bridge,
> +				    enum drm_connector_status status)
> +{
> +	struct omap_hdmi *hdmi = drm_bridge_to_hdmi(bridge);
> +
> +	if (status == connector_status_disconnected)
> +		hdmi5_cec_set_phys_addr(&hdmi->core, NULL);
> +}
> +
>  static struct edid *hdmi5_bridge_get_edid(struct drm_bridge *bridge,
>  					  struct drm_connector *connector)
>  {
> @@ -436,7 +459,7 @@ static struct edid *hdmi5_bridge_get_edid(struct drm_bridge *bridge,
>  	need_enable = hdmi->core_enabled == false;
>  
>  	if (need_enable) {
> -		r = hdmi_core_enable(hdmi);
> +		r = hdmi5_core_enable(hdmi);
>  		if (r)
>  			return NULL;
>  	}
> @@ -460,12 +483,29 @@ static struct edid *hdmi5_bridge_get_edid(struct drm_bridge *bridge,
>  	hdmi_runtime_put(hdmi);
>  	mutex_unlock(&hdmi->lock);
>  
> +	hdmi5_cec_set_phys_addr(&hdmi->core, edid);
> +
>  	if (need_enable)
> -		hdmi_core_disable(hdmi);
> +		hdmi5_core_disable(hdmi);
>  
>  	return (struct edid *)edid;
>  }
>  
> +static int hdmi5_bridge_cec_init(struct drm_bridge *bridge,
> +				 struct drm_connector *conn)
> +{
> +	struct omap_hdmi *hdmi = drm_bridge_to_hdmi(bridge);
> +
> +	return hdmi5_cec_init(hdmi->pdev, &hdmi->core, &hdmi->wp, conn);
> +}
> +
> +static void hdmi5_bridge_cec_exit(struct drm_bridge *bridge)
> +{
> +	struct omap_hdmi *hdmi = drm_bridge_to_hdmi(bridge);
> +
> +	hdmi5_cec_uninit(&hdmi->core);
> +}
> +
>  static const struct drm_bridge_funcs hdmi5_bridge_funcs = {
>  	.attach = hdmi5_bridge_attach,
>  	.mode_set = hdmi5_bridge_mode_set,
> @@ -474,14 +514,17 @@ static const struct drm_bridge_funcs hdmi5_bridge_funcs = {
>  	.atomic_reset = drm_atomic_helper_bridge_reset,
>  	.atomic_enable = hdmi5_bridge_enable,
>  	.atomic_disable = hdmi5_bridge_disable,
> +	.hpd_notify = hdmi5_bridge_hpd_notify,
>  	.get_edid = hdmi5_bridge_get_edid,
> +	.cec_init = hdmi5_bridge_cec_init,
> +	.cec_exit = hdmi5_bridge_cec_exit,
>  };
>  
>  static void hdmi5_bridge_init(struct omap_hdmi *hdmi)
>  {
>  	hdmi->bridge.funcs = &hdmi5_bridge_funcs;
>  	hdmi->bridge.of_node = hdmi->pdev->dev.of_node;
> -	hdmi->bridge.ops = DRM_BRIDGE_OP_EDID;
> +	hdmi->bridge.ops = DRM_BRIDGE_OP_EDID | DRM_BRIDGE_OP_CEC;
>  	hdmi->bridge.type = DRM_MODE_CONNECTOR_HDMIA;
>  
>  	drm_bridge_add(&hdmi->bridge);
> diff --git a/drivers/gpu/drm/omapdrm/dss/hdmi5_cec.c b/drivers/gpu/drm/omapdrm/dss/hdmi5_cec.c
> new file mode 100644
> index 000000000000..26ef8f585b8d
> --- /dev/null
> +++ b/drivers/gpu/drm/omapdrm/dss/hdmi5_cec.c
> @@ -0,0 +1,201 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * HDMI CEC
> + *
> + * Copyright 2019 Cisco Systems, Inc. and/or its affiliates. All rights reserved.
> + */
> +#include <linux/interrupt.h>
> +#include <linux/io.h>
> +#include <linux/module.h>
> +#include <linux/platform_device.h>
> +#include <linux/sched.h>
> +#include <linux/slab.h>
> +#include <linux/clk.h>
> +
> +#include "dss.h"
> +#include "hdmi.h"
> +#include "hdmi5_core.h"
> +#include "hdmi5_cec.h"
> +
> +static int hdmi5_cec_log_addr(struct cec_adapter *adap, u8 logical_addr)
> +{
> +	struct hdmi_core_data *core = cec_get_drvdata(adap);
> +	u8 v;
> +
> +	if (logical_addr == CEC_LOG_ADDR_INVALID) {
> +		hdmi_write_reg(core->base, HDMI_CORE_CEC_ADDR_L, 0);
> +		hdmi_write_reg(core->base, HDMI_CORE_CEC_ADDR_H, 0);

Empty line here

> +		return 0;
> +	}
> +
> +	if (logical_addr <= 7) {
> +		v = hdmi_read_reg(core->base, HDMI_CORE_CEC_ADDR_L);
> +		v |= 1 << logical_addr;
> +		hdmi_write_reg(core->base, HDMI_CORE_CEC_ADDR_L, v);
> +		v = hdmi_read_reg(core->base, HDMI_CORE_CEC_ADDR_H);
> +		v |= 1 << 7;
> +		hdmi_write_reg(core->base, HDMI_CORE_CEC_ADDR_H, v);
> +	} else {
> +		v = hdmi_read_reg(core->base, HDMI_CORE_CEC_ADDR_H);
> +		v |= 1 << (logical_addr - 8);
> +		v |= 1 << 7;
> +		hdmi_write_reg(core->base, HDMI_CORE_CEC_ADDR_H, v);
> +	}
> +
> +	return 0;
> +}
> +
> +static int hdmi5_cec_transmit(struct cec_adapter *adap, u8 attempts,
> +			      u32 signal_free_time, struct cec_msg *msg)
> +{
> +	struct hdmi_core_data *core = cec_get_drvdata(adap);
> +	unsigned int i, ctrl;
> +
> +	switch (signal_free_time) {
> +	case CEC_SIGNAL_FREE_TIME_RETRY:
> +		ctrl = CEC_CTRL_RETRY;
> +		break;
> +	case CEC_SIGNAL_FREE_TIME_NEW_INITIATOR:
> +	default:
> +		ctrl = CEC_CTRL_NORMAL;
> +		break;
> +	case CEC_SIGNAL_FREE_TIME_NEXT_XFER:
> +		ctrl = CEC_CTRL_IMMED;
> +		break;
> +	}
> +
> +	for (i = 0; i < msg->len; i++)
> +		hdmi_write_reg(core->base,
> +			       HDMI_CORE_CEC_TX_DATA0 + i * 4, msg->msg[i]);
> +
> +	hdmi_write_reg(core->base, HDMI_CORE_CEC_TX_CNT, msg->len);
> +	hdmi_write_reg(core->base, HDMI_CORE_CEC_CTRL,
> +		       ctrl | CEC_CTRL_START);
> +
> +	return 0;
> +}
> +
> +void hdmi5_cec_irq(struct hdmi_core_data *core)
> +{
> +	struct cec_adapter *adap = core->adap;
> +	unsigned int stat = hdmi_read_reg(core->base, HDMI_CORE_IH_CEC_STAT0);
> +
> +	if (stat == 0)
> +		return;
> +
> +	hdmi_write_reg(core->base, HDMI_CORE_IH_CEC_STAT0, stat);
> +
> +	if (stat & CEC_STAT_ERROR_INIT)
> +		cec_transmit_attempt_done(adap, CEC_TX_STATUS_ERROR);
> +	else if (stat & CEC_STAT_DONE)
> +		cec_transmit_attempt_done(adap, CEC_TX_STATUS_OK);
> +	else if (stat & CEC_STAT_NACK)
> +		cec_transmit_attempt_done(adap, CEC_TX_STATUS_NACK);
> +
> +	if (stat & CEC_STAT_EOM) {
> +		struct cec_msg msg = {};
> +		unsigned int len, i;
> +
> +		len = hdmi_read_reg(core->base, HDMI_CORE_CEC_RX_CNT);
> +		if (len > sizeof(msg.msg))
> +			len = sizeof(msg.msg);
> +
> +		for (i = 0; i < len; i++)
> +			msg.msg[i] =
> +				hdmi_read_reg(core->base,
> +					      HDMI_CORE_CEC_RX_DATA0 + i * 4);
> +
> +		hdmi_write_reg(core->base, HDMI_CORE_CEC_LOCK, 0);
> +
> +		msg.len = len;
> +		cec_received_msg(adap, &msg);
> +	}
> +}
> +
> +static int hdmi5_cec_enable(struct cec_adapter *adap, bool enable)
> +{
> +	struct hdmi_core_data *core = cec_get_drvdata(adap);
> +	struct omap_hdmi *hdmi = container_of(core, struct omap_hdmi, core);
> +	unsigned int irqs;
> +	int err;
> +
> +	if (!enable) {
> +		hdmi_write_reg(core->base, HDMI_CORE_CEC_MASK, ~0);
> +		hdmi_write_reg(core->base, HDMI_CORE_IH_MUTE_CEC_STAT0, ~0);
> +		hdmi_wp_clear_irqenable(core->wp, HDMI_IRQ_CORE);
> +		hdmi_wp_set_irqstatus(core->wp, HDMI_IRQ_CORE);
> +		REG_FLD_MOD(core->base, HDMI_CORE_MC_CLKDIS, 0x01, 5, 5);
> +		hdmi5_core_disable(hdmi);

Empty line.

> +		return 0;
> +	}

And here.

> +	err = hdmi5_core_enable(hdmi);
> +	if (err)
> +		return err;
> +
> +	REG_FLD_MOD(core->base, HDMI_CORE_MC_CLKDIS, 0x00, 5, 5);
> +	hdmi_write_reg(core->base, HDMI_CORE_IH_I2CM_STAT0, ~0);
> +	hdmi_write_reg(core->base, HDMI_CORE_IH_MUTE_I2CM_STAT0, ~0);
> +	hdmi_write_reg(core->base, HDMI_CORE_CEC_CTRL, 0);
> +	hdmi_write_reg(core->base, HDMI_CORE_IH_CEC_STAT0, ~0);
> +	hdmi_write_reg(core->base, HDMI_CORE_CEC_LOCK, 0);
> +	hdmi_write_reg(core->base, HDMI_CORE_CEC_TX_CNT, 0);
> +
> +	hdmi5_cec_log_addr(adap, CEC_LOG_ADDR_INVALID);
> +
> +	/* Enable HDMI core interrupts */
> +	hdmi_wp_set_irqenable(core->wp, HDMI_IRQ_CORE);
> +
> +	irqs = CEC_STAT_ERROR_INIT | CEC_STAT_NACK | CEC_STAT_EOM |
> +	       CEC_STAT_DONE;
> +	hdmi_write_reg(core->base, HDMI_CORE_CEC_MASK, ~irqs);
> +	hdmi_write_reg(core->base, HDMI_CORE_IH_MUTE_CEC_STAT0, ~irqs);

Empty line.

> +	return 0;
> +}
> +
> +static const struct cec_adap_ops hdmi5_cec_ops = {
> +	.adap_enable = hdmi5_cec_enable,
> +	.adap_log_addr = hdmi5_cec_log_addr,
> +	.adap_transmit = hdmi5_cec_transmit,
> +};

There's a chance of race with these and the drm originating hdmi code.
hdmi5_core_enable/disable is protected with a mutex, but a few of the
registers are touched without any common lock held. Did you go through
these and ensure there's no race? With some studying, I can't see
anything that might cause issues, so maybe it's fine.

> +void hdmi5_cec_set_phys_addr(struct hdmi_core_data *core, struct edid *edid)
> +{
> +	cec_s_phys_addr_from_edid(core->adap, edid);
> +}
> +
> +int hdmi5_cec_init(struct platform_device *pdev, struct hdmi_core_data *core,
> +		   struct hdmi_wp_data *wp, struct drm_connector *conn)
> +{
> +	const u32 caps = CEC_CAP_DEFAULTS | CEC_CAP_CONNECTOR_INFO;
> +	struct cec_connector_info conn_info;
> +	unsigned int ret;
> +
> +	core->cec_clk = devm_clk_get(&pdev->dev, "cec");
> +	if (IS_ERR(core->cec_clk))
> +		return PTR_ERR(core->cec_clk);
> +	ret = clk_prepare_enable(core->cec_clk);
> +	if (ret)
> +		return ret;
> +
> +	core->adap = cec_allocate_adapter(&hdmi5_cec_ops, core,
> +					  "omap5", caps, CEC_MAX_LOG_ADDRS);
> +	ret = PTR_ERR_OR_ZERO(core->adap);
> +	if (ret < 0)
> +		return ret;

Empty line.

> +	cec_fill_conn_info_from_drm(&conn_info, conn);
> +	cec_s_conn_info(core->adap, &conn_info);
> +	core->wp = wp;
> +
> +	ret = cec_register_adapter(core->adap, &pdev->dev);
> +	if (ret < 0) {
> +		cec_delete_adapter(core->adap);
> +		return ret;
> +	}

Empty line.

> +	return 0;
> +}
> +
> +void hdmi5_cec_uninit(struct hdmi_core_data *core)
> +{
> +	clk_disable_unprepare(core->cec_clk);
> +	cec_unregister_adapter(core->adap);
> +}
> diff --git a/drivers/gpu/drm/omapdrm/dss/hdmi5_cec.h b/drivers/gpu/drm/omapdrm/dss/hdmi5_cec.h
> new file mode 100644
> index 000000000000..904541da46da
> --- /dev/null
> +++ b/drivers/gpu/drm/omapdrm/dss/hdmi5_cec.h
> @@ -0,0 +1,42 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +/*
> + * HDMI header definition for OMAP5 HDMI CEC IP
> + *
> + * Copyright 2019 Cisco Systems, Inc. and/or its affiliates. All rights reserved.
> + */
> +
> +#ifndef _HDMI5_CEC_H_
> +#define _HDMI5_CEC_H_
> +
> +/* HDMI CEC funcs */
> +#ifdef CONFIG_OMAP5_DSS_HDMI_CEC
> +void hdmi5_cec_set_phys_addr(struct hdmi_core_data *core,
> +			     struct edid *edid);
> +void hdmi5_cec_irq(struct hdmi_core_data *core);
> +int hdmi5_cec_init(struct platform_device *pdev, struct hdmi_core_data *core,
> +		   struct hdmi_wp_data *wp, struct drm_connector *conn);
> +void hdmi5_cec_uninit(struct hdmi_core_data *core);
> +#else
> +static inline void hdmi5_cec_set_phys_addr(struct hdmi_core_data *core,
> +					   struct edid *edid)
> +{
> +}
> +
> +static inline void hdmi5_cec_irq(struct hdmi_core_data *core)
> +{
> +}
> +
> +static inline int hdmi5_cec_init(struct platform_device *pdev,
> +				 struct hdmi_core_data *core,
> +				 struct hdmi_wp_data *wp,
> +				 struct drm_connector *conn)
> +{
> +	return 0;
> +}
> +
> +static inline void hdmi5_cec_uninit(struct hdmi_core_data *core)
> +{
> +}
> +#endif
> +
> +#endif
> diff --git a/drivers/gpu/drm/omapdrm/dss/hdmi5_core.c b/drivers/gpu/drm/omapdrm/dss/hdmi5_core.c
> index 6cc2ad7a420c..13bc0f3d850b 100644
> --- a/drivers/gpu/drm/omapdrm/dss/hdmi5_core.c
> +++ b/drivers/gpu/drm/omapdrm/dss/hdmi5_core.c
> @@ -229,6 +229,19 @@ void hdmi5_core_dump(struct hdmi_core_data *core, struct seq_file *s)
>  	DUMPCORE(HDMI_CORE_I2CM_FS_SCL_LCNT_1_ADDR);
>  	DUMPCORE(HDMI_CORE_I2CM_FS_SCL_LCNT_0_ADDR);
>  	DUMPCORE(HDMI_CORE_I2CM_SDA_HOLD_ADDR);
> +
> +	DUMPCORE(HDMI_CORE_IH_CEC_STAT0);
> +	DUMPCORE(HDMI_CORE_IH_MUTE_CEC_STAT0);
> +	DUMPCORE(HDMI_CORE_CEC_CTRL);
> +	DUMPCORE(HDMI_CORE_CEC_MASK);
> +	DUMPCORE(HDMI_CORE_CEC_ADDR_L);
> +	DUMPCORE(HDMI_CORE_CEC_ADDR_H);
> +	DUMPCORE(HDMI_CORE_CEC_TX_CNT);
> +	DUMPCORE(HDMI_CORE_CEC_RX_CNT);
> +	DUMPCORE(HDMI_CORE_CEC_TX_DATA0);
> +	DUMPCORE(HDMI_CORE_CEC_RX_DATA0);
> +	DUMPCORE(HDMI_CORE_CEC_LOCK);
> +	DUMPCORE(HDMI_CORE_CEC_WKUPCTRL);
>  }
>  
>  static void hdmi_core_init(struct hdmi_core_vid_config *video_cfg,
> @@ -513,8 +526,6 @@ static void hdmi_core_mask_interrupts(struct hdmi_core_data *core)
>  	REG_FLD_MOD(base, HDMI_CORE_AUD_INT, 0x3, 3, 2);
>  	REG_FLD_MOD(base, HDMI_CORE_AUD_GP_MASK, 0x3, 1, 0);
>  
> -	REG_FLD_MOD(base, HDMI_CORE_CEC_MASK, 0x7f, 6, 0);
> -
>  	REG_FLD_MOD(base, HDMI_CORE_I2CM_CTLINT, 0x1, 6, 6);
>  	REG_FLD_MOD(base, HDMI_CORE_I2CM_CTLINT, 0x1, 2, 2);
>  	REG_FLD_MOD(base, HDMI_CORE_I2CM_INT, 0x1, 2, 2);
> @@ -532,8 +543,6 @@ static void hdmi_core_mask_interrupts(struct hdmi_core_data *core)
>  
>  	REG_FLD_MOD(base, HDMI_CORE_IH_AS_STAT0, 0x7, 2, 0);
>  
> -	REG_FLD_MOD(base, HDMI_CORE_IH_CEC_STAT0, 0x7f, 6, 0);
> -
>  	REG_FLD_MOD(base, HDMI_CORE_IH_I2CM_STAT0, 0x3, 1, 0);
>  
>  	REG_FLD_MOD(base, HDMI_CORE_IH_PHY_STAT0, 0xff, 7, 0);
> @@ -549,13 +558,17 @@ int hdmi5_core_handle_irqs(struct hdmi_core_data *core)
>  {
>  	void __iomem *base = core->base;
>  
> +	/*
> +	 * Clear all possible IRQ_CORE interrupts except for
> +	 * HDMI_CORE_IH_I2CM_STAT0 (that interrupt is muted and
> +	 * is handled by polling elsewhere) and HDMI_CORE_IH_CEC_STAT0
> +	 * which is handled by the CEC code.
> +	 */
>  	REG_FLD_MOD(base, HDMI_CORE_IH_FC_STAT0, 0xff, 7, 0);
>  	REG_FLD_MOD(base, HDMI_CORE_IH_FC_STAT1, 0xff, 7, 0);
>  	REG_FLD_MOD(base, HDMI_CORE_IH_FC_STAT2, 0xff, 7, 0);
>  	REG_FLD_MOD(base, HDMI_CORE_IH_AS_STAT0, 0xff, 7, 0);
>  	REG_FLD_MOD(base, HDMI_CORE_IH_PHY_STAT0, 0xff, 7, 0);
> -	REG_FLD_MOD(base, HDMI_CORE_IH_I2CM_STAT0, 0xff, 7, 0);
> -	REG_FLD_MOD(base, HDMI_CORE_IH_CEC_STAT0, 0xff, 7, 0);
>  	REG_FLD_MOD(base, HDMI_CORE_IH_VP_STAT0, 0xff, 7, 0);
>  	REG_FLD_MOD(base, HDMI_CORE_IH_I2CMPHY_STAT0, 0xff, 7, 0);
>  
> @@ -879,5 +892,8 @@ int hdmi5_core_init(struct platform_device *pdev, struct hdmi_core_data *core)
>  	if (IS_ERR(core->base))
>  		return PTR_ERR(core->base);
>  
> +	REG_FLD_MOD(core->base, HDMI_CORE_CEC_MASK, 0x7f, 6, 0);
> +	REG_FLD_MOD(core->base, HDMI_CORE_IH_CEC_STAT0, 0x7f, 6, 0);
> +
>  	return 0;
>  }
> diff --git a/drivers/gpu/drm/omapdrm/dss/hdmi5_core.h b/drivers/gpu/drm/omapdrm/dss/hdmi5_core.h
> index 070cbf5fb57d..a83b634f6011 100644
> --- a/drivers/gpu/drm/omapdrm/dss/hdmi5_core.h
> +++ b/drivers/gpu/drm/omapdrm/dss/hdmi5_core.h
> @@ -30,8 +30,18 @@
>  #define HDMI_CORE_IH_PHY_STAT0			0x00410
>  #define HDMI_CORE_IH_I2CM_STAT0			0x00414
>  #define HDMI_CORE_IH_CEC_STAT0			0x00418
> +#define CEC_STAT_DONE				BIT(0)
> +#define CEC_STAT_EOM				BIT(1)
> +#define CEC_STAT_NACK				BIT(2)
> +#define CEC_STAT_ARBLOST			BIT(3)
> +#define CEC_STAT_ERROR_INIT			BIT(4)
> +#define CEC_STAT_ERROR_FOLL			BIT(5)
> +#define CEC_STAT_WAKEUP				BIT(6)
> +
>  #define HDMI_CORE_IH_VP_STAT0			0x0041C
>  #define HDMI_CORE_IH_I2CMPHY_STAT0		0x00420
> +#define HDMI_CORE_IH_MUTE_I2CM_STAT0            0x00614
> +#define HDMI_CORE_IH_MUTE_CEC_STAT0		0x00618
>  #define HDMI_CORE_IH_MUTE			0x007FC
>  
>  /* HDMI Video Sampler */
> @@ -233,9 +243,6 @@
>  /* HDMI HDCP */
>  #define HDMI_CORE_HDCP_MASK			0x14020
>  
> -/* HDMI CEC */
> -#define HDMI_CORE_CEC_MASK			0x17408
> -
>  /* HDMI I2C Master */
>  #define HDMI_CORE_I2CM_SLAVE			0x157C8
>  #define HDMI_CORE_I2CM_ADDRESS			0x157CC
> @@ -258,6 +265,24 @@
>  #define HDMI_CORE_I2CM_FS_SCL_LCNT_0_ADDR	0x15810
>  #define HDMI_CORE_I2CM_SDA_HOLD_ADDR		0x15814
>  
> +/* HDMI CEC */
> +#define HDMI_CORE_CEC_CTRL			0x153C8
> +#define CEC_CTRL_START				BIT(0)
> +#define CEC_CTRL_FRAME_TYP			(3 << 1)
> +#define CEC_CTRL_RETRY				(0 << 1)
> +#define CEC_CTRL_NORMAL				(1 << 1)
> +#define CEC_CTRL_IMMED				(2 << 1)
> +
> +#define HDMI_CORE_CEC_MASK			0x153D0
> +#define HDMI_CORE_CEC_ADDR_L			0x153DC
> +#define HDMI_CORE_CEC_ADDR_H			0x153E0
> +#define HDMI_CORE_CEC_TX_CNT			0x153E4
> +#define HDMI_CORE_CEC_RX_CNT			0x153E8
> +#define HDMI_CORE_CEC_TX_DATA0			0x15408
> +#define HDMI_CORE_CEC_RX_DATA0			0x15448
> +#define HDMI_CORE_CEC_LOCK			0x15488
> +#define HDMI_CORE_CEC_WKUPCTRL			0x1548C
> +
>  enum hdmi_core_packet_mode {
>  	HDMI_PACKETMODERESERVEDVALUE = 0,
>  	HDMI_PACKETMODE24BITPERPIXEL = 4,
> @@ -290,6 +315,8 @@ int hdmi5_core_handle_irqs(struct hdmi_core_data *core);
>  void hdmi5_configure(struct hdmi_core_data *core, struct hdmi_wp_data *wp,
>  			struct hdmi_config *cfg);
>  int hdmi5_core_init(struct platform_device *pdev, struct hdmi_core_data *core);
> +int hdmi5_core_enable(struct omap_hdmi *hdmi);
> +void hdmi5_core_disable(struct omap_hdmi *hdmi);
>  
>  int hdmi5_audio_config(struct hdmi_core_data *core, struct hdmi_wp_data *wp,
>  			struct omap_dss_audio *audio, u32 pclk);
> 
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 4/5] drm/omap: hdmi5: add CEC support

[Hans Verkuil]

On 19/02/2021 12:09, Tomi Valkeinen wrote:
> Hi Hans,
> 
> On 11/02/2021 12:37, Hans Verkuil wrote:
>> Add HDMI CEC support for OMAP5.
>>
>> Many thanks to Tomi for helping out how to enable CEC for omap5.
>>
>> Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
>> Thanks-to: Tomi Valkeinen <tomi.valkeinen@iki.fi>
>> ---
>>  drivers/gpu/drm/omapdrm/dss/Kconfig      |   8 +
>>  drivers/gpu/drm/omapdrm/dss/Makefile     |   1 +
>>  drivers/gpu/drm/omapdrm/dss/hdmi.h       |   1 +
>>  drivers/gpu/drm/omapdrm/dss/hdmi5.c      |  63 +++++--
>>  drivers/gpu/drm/omapdrm/dss/hdmi5_cec.c  | 201 +++++++++++++++++++++++
>>  drivers/gpu/drm/omapdrm/dss/hdmi5_cec.h  |  42 +++++
>>  drivers/gpu/drm/omapdrm/dss/hdmi5_core.c |  28 +++-
>>  drivers/gpu/drm/omapdrm/dss/hdmi5_core.h |  33 +++-
>>  8 files changed, 358 insertions(+), 19 deletions(-)
>>  create mode 100644 drivers/gpu/drm/omapdrm/dss/hdmi5_cec.c
>>  create mode 100644 drivers/gpu/drm/omapdrm/dss/hdmi5_cec.h
>>
>> diff --git a/drivers/gpu/drm/omapdrm/dss/Kconfig b/drivers/gpu/drm/omapdrm/dss/Kconfig
>> index e11b258a2294..67a1ba14703b 100644
>> --- a/drivers/gpu/drm/omapdrm/dss/Kconfig
>> +++ b/drivers/gpu/drm/omapdrm/dss/Kconfig
>> @@ -83,6 +83,14 @@ config OMAP5_DSS_HDMI
>>  	  Definition Multimedia Interface. See https://www.hdmi.org/ for HDMI
>>  	  specification.
>>  
>> +config OMAP5_DSS_HDMI_CEC
>> +	bool "Enable HDMI CEC support for OMAP5"
>> +	depends on OMAP5_DSS_HDMI
>> +	select CEC_CORE
>> +	default y
>> +	help
>> +	  When selected the HDMI transmitter will support the CEC feature.
>> +
>>  config OMAP2_DSS_SDI
>>  	bool "SDI support"
>>  	default n
>> diff --git a/drivers/gpu/drm/omapdrm/dss/Makefile b/drivers/gpu/drm/omapdrm/dss/Makefile
>> index f967e6948f2e..94fe0fa3b3c2 100644
>> --- a/drivers/gpu/drm/omapdrm/dss/Makefile
>> +++ b/drivers/gpu/drm/omapdrm/dss/Makefile
>> @@ -17,4 +17,5 @@ omapdss-$(CONFIG_OMAP2_DSS_HDMI_COMMON) += hdmi_common.o hdmi_wp.o hdmi_pll.o \
>>  omapdss-$(CONFIG_OMAP4_DSS_HDMI) += hdmi4.o hdmi4_core.o
>>  omapdss-$(CONFIG_OMAP4_DSS_HDMI_CEC) += hdmi4_cec.o
>>  omapdss-$(CONFIG_OMAP5_DSS_HDMI) += hdmi5.o hdmi5_core.o
>> +omapdss-$(CONFIG_OMAP5_DSS_HDMI_CEC) += hdmi5_cec.o
>>  ccflags-$(CONFIG_OMAP2_DSS_DEBUG) += -DDEBUG
>> diff --git a/drivers/gpu/drm/omapdrm/dss/hdmi.h b/drivers/gpu/drm/omapdrm/dss/hdmi.h
>> index c4a4e07f0b99..72d8ae441da6 100644
>> --- a/drivers/gpu/drm/omapdrm/dss/hdmi.h
>> +++ b/drivers/gpu/drm/omapdrm/dss/hdmi.h
>> @@ -261,6 +261,7 @@ struct hdmi_core_data {
>>  	struct hdmi_wp_data *wp;
>>  	unsigned int core_pwr_cnt;
>>  	struct cec_adapter *adap;
>> +	struct clk *cec_clk;
>>  };
>>  
>>  static inline void hdmi_write_reg(void __iomem *base_addr, const u32 idx,
>> diff --git a/drivers/gpu/drm/omapdrm/dss/hdmi5.c b/drivers/gpu/drm/omapdrm/dss/hdmi5.c
>> index 54e5cb5aa52d..b674d8ba173f 100644
>> --- a/drivers/gpu/drm/omapdrm/dss/hdmi5.c
>> +++ b/drivers/gpu/drm/omapdrm/dss/hdmi5.c
>> @@ -29,12 +29,14 @@
>>  #include <linux/of.h>
>>  #include <linux/of_graph.h>
>>  #include <sound/omap-hdmi-audio.h>
>> +#include <media/cec.h>
>>  
>>  #include <drm/drm_atomic.h>
>>  #include <drm/drm_atomic_state_helper.h>
>>  
>>  #include "omapdss.h"
>>  #include "hdmi5_core.h"
>> +#include "hdmi5_cec.h"
>>  #include "dss.h"
>>  
>>  static int hdmi_runtime_get(struct omap_hdmi *hdmi)
>> @@ -104,6 +106,10 @@ static irqreturn_t hdmi_irq_handler(int irq, void *data)
>>  	} else if (irqstatus & HDMI_IRQ_LINK_DISCONNECT) {
>>  		hdmi_wp_set_phy_pwr(wp, HDMI_PHYPWRCMD_LDOON);
>>  	}
> 
> Empty line here, please.
> 
>> +	if (irqstatus & HDMI_IRQ_CORE) {
>> +		hdmi5_cec_irq(&hdmi->core);
>> +		hdmi5_core_handle_irqs(&hdmi->core);
>> +	}
> 
> It's a bit odd to call two functions here. Would it work if
> hdmi5_core_handle_irqs() would read and clear HDMI_CORE_IH_CEC_STAT0,
> and call hdmi5_cec_irq() if the stat != 0 ?

Makes sense, I'll do that.

> 
> And it would be nice if hdmi5_core.c would enable and disable core
> interrupt, but maybe that can be left for later if the need ever comes
> to handle other interrupts than cec.

I prefer to leave it as-is.

> 
>>  
>>  	return IRQ_HANDLED;
>>  }
>> @@ -112,9 +118,12 @@ static int hdmi_power_on_core(struct omap_hdmi *hdmi)
>>  {
>>  	int r;
>>  
>> +	if (hdmi->core.core_pwr_cnt++)
>> +		return 0;
>> +
>>  	r = regulator_enable(hdmi->vdda_reg);
>>  	if (r)
>> -		return r;
>> +		goto err_reg_enable;
>>  
>>  	r = hdmi_runtime_get(hdmi);
>>  	if (r)
>> @@ -129,12 +138,17 @@ static int hdmi_power_on_core(struct omap_hdmi *hdmi)
>>  
>>  err_runtime_get:
>>  	regulator_disable(hdmi->vdda_reg);
>> +err_reg_enable:
>> +	hdmi->core.core_pwr_cnt--;
>>  
>>  	return r;
>>  }
>>  
>>  static void hdmi_power_off_core(struct omap_hdmi *hdmi)
>>  {
>> +	if (--hdmi->core.core_pwr_cnt)
>> +		return;
>> +
>>  	hdmi->core_enabled = false;
>>  
>>  	hdmi_runtime_put(hdmi);
>> @@ -168,7 +182,7 @@ static int hdmi_power_on_full(struct omap_hdmi *hdmi)
>>  		pc, &hdmi_cinfo);
>>  
>>  	/* disable and clear irqs */
>> -	hdmi_wp_clear_irqenable(&hdmi->wp, 0xffffffff);
>> +	hdmi_wp_clear_irqenable(&hdmi->wp, ~HDMI_IRQ_CORE);
> 
> I guess the point here is to not touch CORE interrupt, as hdmi5_cec.c
> handles that? The line below will still clear the CORE interrupt status.
> 
>>  	hdmi_wp_set_irqstatus(&hdmi->wp,
>>  			hdmi_wp_get_irqstatus(&hdmi->wp));
>>  
>> @@ -225,7 +239,7 @@ static int hdmi_power_on_full(struct omap_hdmi *hdmi)
>>  
>>  static void hdmi_power_off_full(struct omap_hdmi *hdmi)
>>  {
>> -	hdmi_wp_clear_irqenable(&hdmi->wp, 0xffffffff);
>> +	hdmi_wp_clear_irqenable(&hdmi->wp, ~HDMI_IRQ_CORE);
>>  
>>  	hdmi_wp_video_stop(&hdmi->wp);
>>  
>> @@ -273,11 +287,11 @@ static void hdmi_stop_audio_stream(struct omap_hdmi *hd)
>>  	REG_FLD_MOD(hd->wp.base, HDMI_WP_SYSCONFIG, hd->wp_idlemode, 3, 2);
>>  }
>>  
>> -static int hdmi_core_enable(struct omap_hdmi *hdmi)
>> +int hdmi5_core_enable(struct omap_hdmi *hdmi)
>>  {
>>  	int r = 0;
>>  
>> -	DSSDBG("ENTER omapdss_hdmi_core_enable\n");
>> +	DSSDBG("ENTER %s\n", __func__);
>>  
>>  	mutex_lock(&hdmi->lock);
>>  
>> @@ -295,9 +309,9 @@ static int hdmi_core_enable(struct omap_hdmi *hdmi)
>>  	return r;
>>  }
>>  
>> -static void hdmi_core_disable(struct omap_hdmi *hdmi)
>> +void hdmi5_core_disable(struct omap_hdmi *hdmi)
>>  {
>> -	DSSDBG("Enter omapdss_hdmi_core_disable\n");
>> +	DSSDBG("ENTER %s\n", __func__);
>>  
>>  	mutex_lock(&hdmi->lock);
>>  
>> @@ -424,6 +438,15 @@ static void hdmi5_bridge_disable(struct drm_bridge *bridge,
>>  	mutex_unlock(&hdmi->lock);
>>  }
>>  
>> +static void hdmi5_bridge_hpd_notify(struct drm_bridge *bridge,
>> +				    enum drm_connector_status status)
>> +{
>> +	struct omap_hdmi *hdmi = drm_bridge_to_hdmi(bridge);
>> +
>> +	if (status == connector_status_disconnected)
>> +		hdmi5_cec_set_phys_addr(&hdmi->core, NULL);
>> +}
>> +
>>  static struct edid *hdmi5_bridge_get_edid(struct drm_bridge *bridge,
>>  					  struct drm_connector *connector)
>>  {
>> @@ -436,7 +459,7 @@ static struct edid *hdmi5_bridge_get_edid(struct drm_bridge *bridge,
>>  	need_enable = hdmi->core_enabled == false;
>>  
>>  	if (need_enable) {
>> -		r = hdmi_core_enable(hdmi);
>> +		r = hdmi5_core_enable(hdmi);
>>  		if (r)
>>  			return NULL;
>>  	}
>> @@ -460,12 +483,29 @@ static struct edid *hdmi5_bridge_get_edid(struct drm_bridge *bridge,
>>  	hdmi_runtime_put(hdmi);
>>  	mutex_unlock(&hdmi->lock);
>>  
>> +	hdmi5_cec_set_phys_addr(&hdmi->core, edid);
>> +
>>  	if (need_enable)
>> -		hdmi_core_disable(hdmi);
>> +		hdmi5_core_disable(hdmi);
>>  
>>  	return (struct edid *)edid;
>>  }
>>  
>> +static int hdmi5_bridge_cec_init(struct drm_bridge *bridge,
>> +				 struct drm_connector *conn)
>> +{
>> +	struct omap_hdmi *hdmi = drm_bridge_to_hdmi(bridge);
>> +
>> +	return hdmi5_cec_init(hdmi->pdev, &hdmi->core, &hdmi->wp, conn);
>> +}
>> +
>> +static void hdmi5_bridge_cec_exit(struct drm_bridge *bridge)
>> +{
>> +	struct omap_hdmi *hdmi = drm_bridge_to_hdmi(bridge);
>> +
>> +	hdmi5_cec_uninit(&hdmi->core);
>> +}
>> +
>>  static const struct drm_bridge_funcs hdmi5_bridge_funcs = {
>>  	.attach = hdmi5_bridge_attach,
>>  	.mode_set = hdmi5_bridge_mode_set,
>> @@ -474,14 +514,17 @@ static const struct drm_bridge_funcs hdmi5_bridge_funcs = {
>>  	.atomic_reset = drm_atomic_helper_bridge_reset,
>>  	.atomic_enable = hdmi5_bridge_enable,
>>  	.atomic_disable = hdmi5_bridge_disable,
>> +	.hpd_notify = hdmi5_bridge_hpd_notify,
>>  	.get_edid = hdmi5_bridge_get_edid,
>> +	.cec_init = hdmi5_bridge_cec_init,
>> +	.cec_exit = hdmi5_bridge_cec_exit,
>>  };
>>  
>>  static void hdmi5_bridge_init(struct omap_hdmi *hdmi)
>>  {
>>  	hdmi->bridge.funcs = &hdmi5_bridge_funcs;
>>  	hdmi->bridge.of_node = hdmi->pdev->dev.of_node;
>> -	hdmi->bridge.ops = DRM_BRIDGE_OP_EDID;
>> +	hdmi->bridge.ops = DRM_BRIDGE_OP_EDID | DRM_BRIDGE_OP_CEC;
>>  	hdmi->bridge.type = DRM_MODE_CONNECTOR_HDMIA;
>>  
>>  	drm_bridge_add(&hdmi->bridge);
>> diff --git a/drivers/gpu/drm/omapdrm/dss/hdmi5_cec.c b/drivers/gpu/drm/omapdrm/dss/hdmi5_cec.c
>> new file mode 100644
>> index 000000000000..26ef8f585b8d
>> --- /dev/null
>> +++ b/drivers/gpu/drm/omapdrm/dss/hdmi5_cec.c
>> @@ -0,0 +1,201 @@
>> +// SPDX-License-Identifier: GPL-2.0-only
>> +/*
>> + * HDMI CEC
>> + *
>> + * Copyright 2019 Cisco Systems, Inc. and/or its affiliates. All rights reserved.
>> + */
>> +#include <linux/interrupt.h>
>> +#include <linux/io.h>
>> +#include <linux/module.h>
>> +#include <linux/platform_device.h>
>> +#include <linux/sched.h>
>> +#include <linux/slab.h>
>> +#include <linux/clk.h>
>> +
>> +#include "dss.h"
>> +#include "hdmi.h"
>> +#include "hdmi5_core.h"
>> +#include "hdmi5_cec.h"
>> +
>> +static int hdmi5_cec_log_addr(struct cec_adapter *adap, u8 logical_addr)
>> +{
>> +	struct hdmi_core_data *core = cec_get_drvdata(adap);
>> +	u8 v;
>> +
>> +	if (logical_addr == CEC_LOG_ADDR_INVALID) {
>> +		hdmi_write_reg(core->base, HDMI_CORE_CEC_ADDR_L, 0);
>> +		hdmi_write_reg(core->base, HDMI_CORE_CEC_ADDR_H, 0);
> 
> Empty line here
> 
>> +		return 0;
>> +	}
>> +
>> +	if (logical_addr <= 7) {
>> +		v = hdmi_read_reg(core->base, HDMI_CORE_CEC_ADDR_L);
>> +		v |= 1 << logical_addr;
>> +		hdmi_write_reg(core->base, HDMI_CORE_CEC_ADDR_L, v);
>> +		v = hdmi_read_reg(core->base, HDMI_CORE_CEC_ADDR_H);
>> +		v |= 1 << 7;
>> +		hdmi_write_reg(core->base, HDMI_CORE_CEC_ADDR_H, v);
>> +	} else {
>> +		v = hdmi_read_reg(core->base, HDMI_CORE_CEC_ADDR_H);
>> +		v |= 1 << (logical_addr - 8);
>> +		v |= 1 << 7;
>> +		hdmi_write_reg(core->base, HDMI_CORE_CEC_ADDR_H, v);
>> +	}
>> +
>> +	return 0;
>> +}
>> +
>> +static int hdmi5_cec_transmit(struct cec_adapter *adap, u8 attempts,
>> +			      u32 signal_free_time, struct cec_msg *msg)
>> +{
>> +	struct hdmi_core_data *core = cec_get_drvdata(adap);
>> +	unsigned int i, ctrl;
>> +
>> +	switch (signal_free_time) {
>> +	case CEC_SIGNAL_FREE_TIME_RETRY:
>> +		ctrl = CEC_CTRL_RETRY;
>> +		break;
>> +	case CEC_SIGNAL_FREE_TIME_NEW_INITIATOR:
>> +	default:
>> +		ctrl = CEC_CTRL_NORMAL;
>> +		break;
>> +	case CEC_SIGNAL_FREE_TIME_NEXT_XFER:
>> +		ctrl = CEC_CTRL_IMMED;
>> +		break;
>> +	}
>> +
>> +	for (i = 0; i < msg->len; i++)
>> +		hdmi_write_reg(core->base,
>> +			       HDMI_CORE_CEC_TX_DATA0 + i * 4, msg->msg[i]);
>> +
>> +	hdmi_write_reg(core->base, HDMI_CORE_CEC_TX_CNT, msg->len);
>> +	hdmi_write_reg(core->base, HDMI_CORE_CEC_CTRL,
>> +		       ctrl | CEC_CTRL_START);
>> +
>> +	return 0;
>> +}
>> +
>> +void hdmi5_cec_irq(struct hdmi_core_data *core)
>> +{
>> +	struct cec_adapter *adap = core->adap;
>> +	unsigned int stat = hdmi_read_reg(core->base, HDMI_CORE_IH_CEC_STAT0);
>> +
>> +	if (stat == 0)
>> +		return;
>> +
>> +	hdmi_write_reg(core->base, HDMI_CORE_IH_CEC_STAT0, stat);
>> +
>> +	if (stat & CEC_STAT_ERROR_INIT)
>> +		cec_transmit_attempt_done(adap, CEC_TX_STATUS_ERROR);
>> +	else if (stat & CEC_STAT_DONE)
>> +		cec_transmit_attempt_done(adap, CEC_TX_STATUS_OK);
>> +	else if (stat & CEC_STAT_NACK)
>> +		cec_transmit_attempt_done(adap, CEC_TX_STATUS_NACK);
>> +
>> +	if (stat & CEC_STAT_EOM) {
>> +		struct cec_msg msg = {};
>> +		unsigned int len, i;
>> +
>> +		len = hdmi_read_reg(core->base, HDMI_CORE_CEC_RX_CNT);
>> +		if (len > sizeof(msg.msg))
>> +			len = sizeof(msg.msg);
>> +
>> +		for (i = 0; i < len; i++)
>> +			msg.msg[i] =
>> +				hdmi_read_reg(core->base,
>> +					      HDMI_CORE_CEC_RX_DATA0 + i * 4);
>> +
>> +		hdmi_write_reg(core->base, HDMI_CORE_CEC_LOCK, 0);
>> +
>> +		msg.len = len;
>> +		cec_received_msg(adap, &msg);
>> +	}
>> +}
>> +
>> +static int hdmi5_cec_enable(struct cec_adapter *adap, bool enable)
>> +{
>> +	struct hdmi_core_data *core = cec_get_drvdata(adap);
>> +	struct omap_hdmi *hdmi = container_of(core, struct omap_hdmi, core);
>> +	unsigned int irqs;
>> +	int err;
>> +
>> +	if (!enable) {
>> +		hdmi_write_reg(core->base, HDMI_CORE_CEC_MASK, ~0);
>> +		hdmi_write_reg(core->base, HDMI_CORE_IH_MUTE_CEC_STAT0, ~0);
>> +		hdmi_wp_clear_irqenable(core->wp, HDMI_IRQ_CORE);
>> +		hdmi_wp_set_irqstatus(core->wp, HDMI_IRQ_CORE);
>> +		REG_FLD_MOD(core->base, HDMI_CORE_MC_CLKDIS, 0x01, 5, 5);
>> +		hdmi5_core_disable(hdmi);
> 
> Empty line.
> 
>> +		return 0;
>> +	}
> 
> And here.
> 
>> +	err = hdmi5_core_enable(hdmi);
>> +	if (err)
>> +		return err;
>> +
>> +	REG_FLD_MOD(core->base, HDMI_CORE_MC_CLKDIS, 0x00, 5, 5);
>> +	hdmi_write_reg(core->base, HDMI_CORE_IH_I2CM_STAT0, ~0);
>> +	hdmi_write_reg(core->base, HDMI_CORE_IH_MUTE_I2CM_STAT0, ~0);
>> +	hdmi_write_reg(core->base, HDMI_CORE_CEC_CTRL, 0);
>> +	hdmi_write_reg(core->base, HDMI_CORE_IH_CEC_STAT0, ~0);
>> +	hdmi_write_reg(core->base, HDMI_CORE_CEC_LOCK, 0);
>> +	hdmi_write_reg(core->base, HDMI_CORE_CEC_TX_CNT, 0);
>> +
>> +	hdmi5_cec_log_addr(adap, CEC_LOG_ADDR_INVALID);
>> +
>> +	/* Enable HDMI core interrupts */
>> +	hdmi_wp_set_irqenable(core->wp, HDMI_IRQ_CORE);
>> +
>> +	irqs = CEC_STAT_ERROR_INIT | CEC_STAT_NACK | CEC_STAT_EOM |
>> +	       CEC_STAT_DONE;
>> +	hdmi_write_reg(core->base, HDMI_CORE_CEC_MASK, ~irqs);
>> +	hdmi_write_reg(core->base, HDMI_CORE_IH_MUTE_CEC_STAT0, ~irqs);
> 
> Empty line.
> 
>> +	return 0;
>> +}
>> +
>> +static const struct cec_adap_ops hdmi5_cec_ops = {
>> +	.adap_enable = hdmi5_cec_enable,
>> +	.adap_log_addr = hdmi5_cec_log_addr,
>> +	.adap_transmit = hdmi5_cec_transmit,
>> +};
> 
> There's a chance of race with these and the drm originating hdmi code.
> hdmi5_core_enable/disable is protected with a mutex, but a few of the
> registers are touched without any common lock held. Did you go through
> these and ensure there's no race? With some studying, I can't see
> anything that might cause issues, so maybe it's fine.
> 
>> +void hdmi5_cec_set_phys_addr(struct hdmi_core_data *core, struct edid *edid)
>> +{
>> +	cec_s_phys_addr_from_edid(core->adap, edid);
>> +}
>> +
>> +int hdmi5_cec_init(struct platform_device *pdev, struct hdmi_core_data *core,
>> +		   struct hdmi_wp_data *wp, struct drm_connector *conn)
>> +{
>> +	const u32 caps = CEC_CAP_DEFAULTS | CEC_CAP_CONNECTOR_INFO;
>> +	struct cec_connector_info conn_info;
>> +	unsigned int ret;
>> +
>> +	core->cec_clk = devm_clk_get(&pdev->dev, "cec");
>> +	if (IS_ERR(core->cec_clk))
>> +		return PTR_ERR(core->cec_clk);
>> +	ret = clk_prepare_enable(core->cec_clk);
>> +	if (ret)
>> +		return ret;
>> +
>> +	core->adap = cec_allocate_adapter(&hdmi5_cec_ops, core,
>> +					  "omap5", caps, CEC_MAX_LOG_ADDRS);
>> +	ret = PTR_ERR_OR_ZERO(core->adap);
>> +	if (ret < 0)
>> +		return ret;
> 
> Empty line.
> 
>> +	cec_fill_conn_info_from_drm(&conn_info, conn);
>> +	cec_s_conn_info(core->adap, &conn_info);
>> +	core->wp = wp;
>> +
>> +	ret = cec_register_adapter(core->adap, &pdev->dev);
>> +	if (ret < 0) {
>> +		cec_delete_adapter(core->adap);
>> +		return ret;
>> +	}
> 
> Empty line.

Added all these empty lines.

Regards,

	Hans

> 
>> +	return 0;
>> +}
>> +
>> +void hdmi5_cec_uninit(struct hdmi_core_data *core)
>> +{
>> +	clk_disable_unprepare(core->cec_clk);
>> +	cec_unregister_adapter(core->adap);
>> +}
>> diff --git a/drivers/gpu/drm/omapdrm/dss/hdmi5_cec.h b/drivers/gpu/drm/omapdrm/dss/hdmi5_cec.h
>> new file mode 100644
>> index 000000000000..904541da46da
>> --- /dev/null
>> +++ b/drivers/gpu/drm/omapdrm/dss/hdmi5_cec.h
>> @@ -0,0 +1,42 @@
>> +/* SPDX-License-Identifier: GPL-2.0-only */
>> +/*
>> + * HDMI header definition for OMAP5 HDMI CEC IP
>> + *
>> + * Copyright 2019 Cisco Systems, Inc. and/or its affiliates. All rights reserved.
>> + */
>> +
>> +#ifndef _HDMI5_CEC_H_
>> +#define _HDMI5_CEC_H_
>> +
>> +/* HDMI CEC funcs */
>> +#ifdef CONFIG_OMAP5_DSS_HDMI_CEC
>> +void hdmi5_cec_set_phys_addr(struct hdmi_core_data *core,
>> +			     struct edid *edid);
>> +void hdmi5_cec_irq(struct hdmi_core_data *core);
>> +int hdmi5_cec_init(struct platform_device *pdev, struct hdmi_core_data *core,
>> +		   struct hdmi_wp_data *wp, struct drm_connector *conn);
>> +void hdmi5_cec_uninit(struct hdmi_core_data *core);
>> +#else
>> +static inline void hdmi5_cec_set_phys_addr(struct hdmi_core_data *core,
>> +					   struct edid *edid)
>> +{
>> +}
>> +
>> +static inline void hdmi5_cec_irq(struct hdmi_core_data *core)
>> +{
>> +}
>> +
>> +static inline int hdmi5_cec_init(struct platform_device *pdev,
>> +				 struct hdmi_core_data *core,
>> +				 struct hdmi_wp_data *wp,
>> +				 struct drm_connector *conn)
>> +{
>> +	return 0;
>> +}
>> +
>> +static inline void hdmi5_cec_uninit(struct hdmi_core_data *core)
>> +{
>> +}
>> +#endif
>> +
>> +#endif
>> diff --git a/drivers/gpu/drm/omapdrm/dss/hdmi5_core.c b/drivers/gpu/drm/omapdrm/dss/hdmi5_core.c
>> index 6cc2ad7a420c..13bc0f3d850b 100644
>> --- a/drivers/gpu/drm/omapdrm/dss/hdmi5_core.c
>> +++ b/drivers/gpu/drm/omapdrm/dss/hdmi5_core.c
>> @@ -229,6 +229,19 @@ void hdmi5_core_dump(struct hdmi_core_data *core, struct seq_file *s)
>>  	DUMPCORE(HDMI_CORE_I2CM_FS_SCL_LCNT_1_ADDR);
>>  	DUMPCORE(HDMI_CORE_I2CM_FS_SCL_LCNT_0_ADDR);
>>  	DUMPCORE(HDMI_CORE_I2CM_SDA_HOLD_ADDR);
>> +
>> +	DUMPCORE(HDMI_CORE_IH_CEC_STAT0);
>> +	DUMPCORE(HDMI_CORE_IH_MUTE_CEC_STAT0);
>> +	DUMPCORE(HDMI_CORE_CEC_CTRL);
>> +	DUMPCORE(HDMI_CORE_CEC_MASK);
>> +	DUMPCORE(HDMI_CORE_CEC_ADDR_L);
>> +	DUMPCORE(HDMI_CORE_CEC_ADDR_H);
>> +	DUMPCORE(HDMI_CORE_CEC_TX_CNT);
>> +	DUMPCORE(HDMI_CORE_CEC_RX_CNT);
>> +	DUMPCORE(HDMI_CORE_CEC_TX_DATA0);
>> +	DUMPCORE(HDMI_CORE_CEC_RX_DATA0);
>> +	DUMPCORE(HDMI_CORE_CEC_LOCK);
>> +	DUMPCORE(HDMI_CORE_CEC_WKUPCTRL);
>>  }
>>  
>>  static void hdmi_core_init(struct hdmi_core_vid_config *video_cfg,
>> @@ -513,8 +526,6 @@ static void hdmi_core_mask_interrupts(struct hdmi_core_data *core)
>>  	REG_FLD_MOD(base, HDMI_CORE_AUD_INT, 0x3, 3, 2);
>>  	REG_FLD_MOD(base, HDMI_CORE_AUD_GP_MASK, 0x3, 1, 0);
>>  
>> -	REG_FLD_MOD(base, HDMI_CORE_CEC_MASK, 0x7f, 6, 0);
>> -
>>  	REG_FLD_MOD(base, HDMI_CORE_I2CM_CTLINT, 0x1, 6, 6);
>>  	REG_FLD_MOD(base, HDMI_CORE_I2CM_CTLINT, 0x1, 2, 2);
>>  	REG_FLD_MOD(base, HDMI_CORE_I2CM_INT, 0x1, 2, 2);
>> @@ -532,8 +543,6 @@ static void hdmi_core_mask_interrupts(struct hdmi_core_data *core)
>>  
>>  	REG_FLD_MOD(base, HDMI_CORE_IH_AS_STAT0, 0x7, 2, 0);
>>  
>> -	REG_FLD_MOD(base, HDMI_CORE_IH_CEC_STAT0, 0x7f, 6, 0);
>> -
>>  	REG_FLD_MOD(base, HDMI_CORE_IH_I2CM_STAT0, 0x3, 1, 0);
>>  
>>  	REG_FLD_MOD(base, HDMI_CORE_IH_PHY_STAT0, 0xff, 7, 0);
>> @@ -549,13 +558,17 @@ int hdmi5_core_handle_irqs(struct hdmi_core_data *core)
>>  {
>>  	void __iomem *base = core->base;
>>  
>> +	/*
>> +	 * Clear all possible IRQ_CORE interrupts except for
>> +	 * HDMI_CORE_IH_I2CM_STAT0 (that interrupt is muted and
>> +	 * is handled by polling elsewhere) and HDMI_CORE_IH_CEC_STAT0
>> +	 * which is handled by the CEC code.
>> +	 */
>>  	REG_FLD_MOD(base, HDMI_CORE_IH_FC_STAT0, 0xff, 7, 0);
>>  	REG_FLD_MOD(base, HDMI_CORE_IH_FC_STAT1, 0xff, 7, 0);
>>  	REG_FLD_MOD(base, HDMI_CORE_IH_FC_STAT2, 0xff, 7, 0);
>>  	REG_FLD_MOD(base, HDMI_CORE_IH_AS_STAT0, 0xff, 7, 0);
>>  	REG_FLD_MOD(base, HDMI_CORE_IH_PHY_STAT0, 0xff, 7, 0);
>> -	REG_FLD_MOD(base, HDMI_CORE_IH_I2CM_STAT0, 0xff, 7, 0);
>> -	REG_FLD_MOD(base, HDMI_CORE_IH_CEC_STAT0, 0xff, 7, 0);
>>  	REG_FLD_MOD(base, HDMI_CORE_IH_VP_STAT0, 0xff, 7, 0);
>>  	REG_FLD_MOD(base, HDMI_CORE_IH_I2CMPHY_STAT0, 0xff, 7, 0);
>>  
>> @@ -879,5 +892,8 @@ int hdmi5_core_init(struct platform_device *pdev, struct hdmi_core_data *core)
>>  	if (IS_ERR(core->base))
>>  		return PTR_ERR(core->base);
>>  
>> +	REG_FLD_MOD(core->base, HDMI_CORE_CEC_MASK, 0x7f, 6, 0);
>> +	REG_FLD_MOD(core->base, HDMI_CORE_IH_CEC_STAT0, 0x7f, 6, 0);
>> +
>>  	return 0;
>>  }
>> diff --git a/drivers/gpu/drm/omapdrm/dss/hdmi5_core.h b/drivers/gpu/drm/omapdrm/dss/hdmi5_core.h
>> index 070cbf5fb57d..a83b634f6011 100644
>> --- a/drivers/gpu/drm/omapdrm/dss/hdmi5_core.h
>> +++ b/drivers/gpu/drm/omapdrm/dss/hdmi5_core.h
>> @@ -30,8 +30,18 @@
>>  #define HDMI_CORE_IH_PHY_STAT0			0x00410
>>  #define HDMI_CORE_IH_I2CM_STAT0			0x00414
>>  #define HDMI_CORE_IH_CEC_STAT0			0x00418
>> +#define CEC_STAT_DONE				BIT(0)
>> +#define CEC_STAT_EOM				BIT(1)
>> +#define CEC_STAT_NACK				BIT(2)
>> +#define CEC_STAT_ARBLOST			BIT(3)
>> +#define CEC_STAT_ERROR_INIT			BIT(4)
>> +#define CEC_STAT_ERROR_FOLL			BIT(5)
>> +#define CEC_STAT_WAKEUP				BIT(6)
>> +
>>  #define HDMI_CORE_IH_VP_STAT0			0x0041C
>>  #define HDMI_CORE_IH_I2CMPHY_STAT0		0x00420
>> +#define HDMI_CORE_IH_MUTE_I2CM_STAT0            0x00614
>> +#define HDMI_CORE_IH_MUTE_CEC_STAT0		0x00618
>>  #define HDMI_CORE_IH_MUTE			0x007FC
>>  
>>  /* HDMI Video Sampler */
>> @@ -233,9 +243,6 @@
>>  /* HDMI HDCP */
>>  #define HDMI_CORE_HDCP_MASK			0x14020
>>  
>> -/* HDMI CEC */
>> -#define HDMI_CORE_CEC_MASK			0x17408
>> -
>>  /* HDMI I2C Master */
>>  #define HDMI_CORE_I2CM_SLAVE			0x157C8
>>  #define HDMI_CORE_I2CM_ADDRESS			0x157CC
>> @@ -258,6 +265,24 @@
>>  #define HDMI_CORE_I2CM_FS_SCL_LCNT_0_ADDR	0x15810
>>  #define HDMI_CORE_I2CM_SDA_HOLD_ADDR		0x15814
>>  
>> +/* HDMI CEC */
>> +#define HDMI_CORE_CEC_CTRL			0x153C8
>> +#define CEC_CTRL_START				BIT(0)
>> +#define CEC_CTRL_FRAME_TYP			(3 << 1)
>> +#define CEC_CTRL_RETRY				(0 << 1)
>> +#define CEC_CTRL_NORMAL				(1 << 1)
>> +#define CEC_CTRL_IMMED				(2 << 1)
>> +
>> +#define HDMI_CORE_CEC_MASK			0x153D0
>> +#define HDMI_CORE_CEC_ADDR_L			0x153DC
>> +#define HDMI_CORE_CEC_ADDR_H			0x153E0
>> +#define HDMI_CORE_CEC_TX_CNT			0x153E4
>> +#define HDMI_CORE_CEC_RX_CNT			0x153E8
>> +#define HDMI_CORE_CEC_TX_DATA0			0x15408
>> +#define HDMI_CORE_CEC_RX_DATA0			0x15448
>> +#define HDMI_CORE_CEC_LOCK			0x15488
>> +#define HDMI_CORE_CEC_WKUPCTRL			0x1548C
>> +
>>  enum hdmi_core_packet_mode {
>>  	HDMI_PACKETMODERESERVEDVALUE = 0,
>>  	HDMI_PACKETMODE24BITPERPIXEL = 4,
>> @@ -290,6 +315,8 @@ int hdmi5_core_handle_irqs(struct hdmi_core_data *core);
>>  void hdmi5_configure(struct hdmi_core_data *core, struct hdmi_wp_data *wp,
>>  			struct hdmi_config *cfg);
>>  int hdmi5_core_init(struct platform_device *pdev, struct hdmi_core_data *core);
>> +int hdmi5_core_enable(struct omap_hdmi *hdmi);
>> +void hdmi5_core_disable(struct omap_hdmi *hdmi);
>>  
>>  int hdmi5_audio_config(struct hdmi_core_data *core, struct hdmi_wp_data *wp,
>>  			struct omap_dss_audio *audio, u32 pclk);
>>

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 5/5] ARM: dts: dra7/omap5: add cec clock

[Hans Verkuil]

Add cec clock to the dra7 and omap5 device trees.

Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
---
 arch/arm/boot/dts/dra7.dtsi  | 5 +++--
 arch/arm/boot/dts/omap5.dtsi | 5 +++--
 2 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/arch/arm/boot/dts/dra7.dtsi b/arch/arm/boot/dts/dra7.dtsi
index ce1194744f84..efe579ddb324 100644
--- a/arch/arm/boot/dts/dra7.dtsi
+++ b/arch/arm/boot/dts/dra7.dtsi
@@ -879,8 +879,9 @@ hdmi: encoder@0 {
 						interrupts = <GIC_SPI 96 IRQ_TYPE_LEVEL_HIGH>;
 						status = "disabled";
 						clocks = <&dss_clkctrl DRA7_DSS_DSS_CORE_CLKCTRL 9>,
-							 <&dss_clkctrl DRA7_DSS_DSS_CORE_CLKCTRL 10>;
-						clock-names = "fck", "sys_clk";
+							 <&dss_clkctrl DRA7_DSS_DSS_CORE_CLKCTRL 10>,
+							 <&dss_clkctrl DRA7_DSS_DSS_CORE_CLKCTRL 11>;
+						clock-names = "fck", "sys_clk", "cec";
 						dmas = <&sdma_xbar 76>;
 						dma-names = "audio_tx";
 					};
diff --git a/arch/arm/boot/dts/omap5.dtsi b/arch/arm/boot/dts/omap5.dtsi
index 5f1a8bd13880..2bb1000aeae9 100644
--- a/arch/arm/boot/dts/omap5.dtsi
+++ b/arch/arm/boot/dts/omap5.dtsi
@@ -580,8 +580,9 @@ hdmi: encoder@0 {
 						interrupts = <GIC_SPI 101 IRQ_TYPE_LEVEL_HIGH>;
 						status = "disabled";
 						clocks = <&dss_clkctrl OMAP5_DSS_CORE_CLKCTRL 9>,
-							 <&dss_clkctrl OMAP5_DSS_CORE_CLKCTRL 10>;
-						clock-names = "fck", "sys_clk";
+							 <&dss_clkctrl OMAP5_DSS_CORE_CLKCTRL 10>,
+							 <&dss_clkctrl OMAP5_DSS_CORE_CLKCTRL 11>;
+						clock-names = "fck", "sys_clk", "cec";
 						dmas = <&sdma 76>;
 						dma-names = "audio_tx";
 					};
-- 
2.30.0

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 5/5] ARM: dts: dra7/omap5: add cec clock

[Tony Lindgren]

* Hans Verkuil <hverkuil-cisco@xs4all.nl> [210211 12:37]:
> Add cec clock to the dra7 and omap5 device trees.
> 
> Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>

This should merge just fine with what I'm planning to merge for v5.13,
probably best to apply this together with the driver changes:

Acked-by: Tony Lindgren <tony@atomide.com>

> ---
>  arch/arm/boot/dts/dra7.dtsi  | 5 +++--
>  arch/arm/boot/dts/omap5.dtsi | 5 +++--
>  2 files changed, 6 insertions(+), 4 deletions(-)
> 
> diff --git a/arch/arm/boot/dts/dra7.dtsi b/arch/arm/boot/dts/dra7.dtsi
> index ce1194744f84..efe579ddb324 100644
> --- a/arch/arm/boot/dts/dra7.dtsi
> +++ b/arch/arm/boot/dts/dra7.dtsi
> @@ -879,8 +879,9 @@ hdmi: encoder@0 {
>  						interrupts = <GIC_SPI 96 IRQ_TYPE_LEVEL_HIGH>;
>  						status = "disabled";
>  						clocks = <&dss_clkctrl DRA7_DSS_DSS_CORE_CLKCTRL 9>,
> -							 <&dss_clkctrl DRA7_DSS_DSS_CORE_CLKCTRL 10>;
> -						clock-names = "fck", "sys_clk";
> +							 <&dss_clkctrl DRA7_DSS_DSS_CORE_CLKCTRL 10>,
> +							 <&dss_clkctrl DRA7_DSS_DSS_CORE_CLKCTRL 11>;
> +						clock-names = "fck", "sys_clk", "cec";
>  						dmas = <&sdma_xbar 76>;
>  						dma-names = "audio_tx";
>  					};
> diff --git a/arch/arm/boot/dts/omap5.dtsi b/arch/arm/boot/dts/omap5.dtsi
> index 5f1a8bd13880..2bb1000aeae9 100644
> --- a/arch/arm/boot/dts/omap5.dtsi
> +++ b/arch/arm/boot/dts/omap5.dtsi
> @@ -580,8 +580,9 @@ hdmi: encoder@0 {
>  						interrupts = <GIC_SPI 101 IRQ_TYPE_LEVEL_HIGH>;
>  						status = "disabled";
>  						clocks = <&dss_clkctrl OMAP5_DSS_CORE_CLKCTRL 9>,
> -							 <&dss_clkctrl OMAP5_DSS_CORE_CLKCTRL 10>;
> -						clock-names = "fck", "sys_clk";
> +							 <&dss_clkctrl OMAP5_DSS_CORE_CLKCTRL 10>,
> +							 <&dss_clkctrl OMAP5_DSS_CORE_CLKCTRL 11>;
> +						clock-names = "fck", "sys_clk", "cec";
>  						dmas = <&sdma 76>;
>  						dma-names = "audio_tx";
>  					};
> -- 
> 2.30.0
> 
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 5/5] ARM: dts: dra7/omap5: add cec clock

[Tomi Valkeinen]

Hi Hans,

On 11/02/2021 12:37, Hans Verkuil wrote:
> Add cec clock to the dra7 and omap5 device trees.
> 
> Signed-off-by: Hans Verkuil <hverkuil-cisco@xs4all.nl>
> ---
>  arch/arm/boot/dts/dra7.dtsi  | 5 +++--
>  arch/arm/boot/dts/omap5.dtsi | 5 +++--
>  2 files changed, 6 insertions(+), 4 deletions(-)
> 
> diff --git a/arch/arm/boot/dts/dra7.dtsi b/arch/arm/boot/dts/dra7.dtsi
> index ce1194744f84..efe579ddb324 100644
> --- a/arch/arm/boot/dts/dra7.dtsi
> +++ b/arch/arm/boot/dts/dra7.dtsi
> @@ -879,8 +879,9 @@ hdmi: encoder@0 {
>  						interrupts = <GIC_SPI 96 IRQ_TYPE_LEVEL_HIGH>;
>  						status = "disabled";
>  						clocks = <&dss_clkctrl DRA7_DSS_DSS_CORE_CLKCTRL 9>,
> -							 <&dss_clkctrl DRA7_DSS_DSS_CORE_CLKCTRL 10>;
> -						clock-names = "fck", "sys_clk";
> +							 <&dss_clkctrl DRA7_DSS_DSS_CORE_CLKCTRL 10>,
> +							 <&dss_clkctrl DRA7_DSS_DSS_CORE_CLKCTRL 11>;
> +						clock-names = "fck", "sys_clk", "cec";
>  						dmas = <&sdma_xbar 76>;
>  						dma-names = "audio_tx";
>  					};
> diff --git a/arch/arm/boot/dts/omap5.dtsi b/arch/arm/boot/dts/omap5.dtsi
> index 5f1a8bd13880..2bb1000aeae9 100644
> --- a/arch/arm/boot/dts/omap5.dtsi
> +++ b/arch/arm/boot/dts/omap5.dtsi
> @@ -580,8 +580,9 @@ hdmi: encoder@0 {
>  						interrupts = <GIC_SPI 101 IRQ_TYPE_LEVEL_HIGH>;
>  						status = "disabled";
>  						clocks = <&dss_clkctrl OMAP5_DSS_CORE_CLKCTRL 9>,
> -							 <&dss_clkctrl OMAP5_DSS_CORE_CLKCTRL 10>;
> -						clock-names = "fck", "sys_clk";
> +							 <&dss_clkctrl OMAP5_DSS_CORE_CLKCTRL 10>,
> +							 <&dss_clkctrl OMAP5_DSS_CORE_CLKCTRL 11>;
> +						clock-names = "fck", "sys_clk", "cec";
>  						dmas = <&sdma 76>;
>  						dma-names = "audio_tx";
>  					};
> 

If I read right, the cec clock is mandatory after adding the CEC support
in the previous patch. So please move this patch before the patch 4, and
also update the bindings in ti,omap5-dss.txt to list the cec clock.

 Tomi
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 0/5] drm/omap: hdmi: improve hdmi4 CEC, add CEC for hdmi5

[H. Nikolaus Schaller]

Hi,

> Am 11.02.2021 um 11:36 schrieb Hans Verkuil <hverkuil-cisco@xs4all.nl>:
> 
> This series improves the drm_bridge support for CEC by introducing two
> new bridge ops in the first patch, and using those in the second patch.
> 
> This makes it possible to call cec_s_conn_info() and set
> CEC_CAP_CONNECTOR_INFO for the CEC adapter, so userspace can associate
> the CEC adapter with the corresponding DRM connector.
> 
> The third patch simplifies CEC physical address handling by using the
> cec_s_phys_addr_from_edid helper function that didn't exist when this
> code was originally written.
> 
> The fourth patch adds CEC support to the OMAP5 driver and the last
> patch adds the missing cec clock to the dra7 and omap5 device tree.
> 
> Tested with a Pandaboard and a Beagle X15 board.

Tested to have no adverse effect on Pyra (omap5432).
But I have not tested if CEC itself is working.

> 
> Regards,
> 
> 	Hans
> 
> Hans Verkuil (5):
>  drm: drm_bridge: add cec_init/exit bridge ops
>  drm/omap: hdmi4: switch to the cec bridge ops
>  drm/omap: hdmi4: simplify CEC Phys Addr handling
>  drm/omap: hdmi5: add CEC support
>  ARM: dts: dra7/omap5: add cec clock
> 
> arch/arm/boot/dts/dra7.dtsi              |   5 +-
> arch/arm/boot/dts/omap5.dtsi             |   5 +-
> drivers/gpu/drm/drm_bridge_connector.c   |  23 +++
> drivers/gpu/drm/omapdrm/dss/Kconfig      |   8 +
> drivers/gpu/drm/omapdrm/dss/Makefile     |   1 +

Merging with patch series by Tomi Valkeinen and Sebastian Reichel
for omapdrm/dsi will need to move the Kconfig and Makefile one level
up.

> drivers/gpu/drm/omapdrm/dss/hdmi.h       |   1 +
> drivers/gpu/drm/omapdrm/dss/hdmi4.c      |  41 ++---
> drivers/gpu/drm/omapdrm/dss/hdmi4_cec.c  |  12 +-
> drivers/gpu/drm/omapdrm/dss/hdmi4_cec.h  |  12 +-
> drivers/gpu/drm/omapdrm/dss/hdmi5.c      |  63 +++++--
> drivers/gpu/drm/omapdrm/dss/hdmi5_cec.c  | 201 +++++++++++++++++++++++
> drivers/gpu/drm/omapdrm/dss/hdmi5_cec.h  |  42 +++++
> drivers/gpu/drm/omapdrm/dss/hdmi5_core.c |  28 +++-
> drivers/gpu/drm/omapdrm/dss/hdmi5_core.h |  33 +++-
> include/drm/drm_bridge.h                 |  31 ++++
> 15 files changed, 453 insertions(+), 53 deletions(-)
> create mode 100644 drivers/gpu/drm/omapdrm/dss/hdmi5_cec.c
> create mode 100644 drivers/gpu/drm/omapdrm/dss/hdmi5_cec.h
> 
> -- 
> 2.30.0
> 

BR and thanks,
Nikolaus Schaller

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 0/5] drm/omap: hdmi: improve hdmi4 CEC, add CEC for hdmi5

[Tomi Valkeinen]

On 15/02/2021 13:11, H. Nikolaus Schaller wrote:
> Hi,
> 
>> Am 11.02.2021 um 11:36 schrieb Hans Verkuil <hverkuil-cisco@xs4all.nl>:
>>
>> This series improves the drm_bridge support for CEC by introducing two
>> new bridge ops in the first patch, and using those in the second patch.
>>
>> This makes it possible to call cec_s_conn_info() and set
>> CEC_CAP_CONNECTOR_INFO for the CEC adapter, so userspace can associate
>> the CEC adapter with the corresponding DRM connector.
>>
>> The third patch simplifies CEC physical address handling by using the
>> cec_s_phys_addr_from_edid helper function that didn't exist when this
>> code was originally written.
>>
>> The fourth patch adds CEC support to the OMAP5 driver and the last
>> patch adds the missing cec clock to the dra7 and omap5 device tree.
>>
>> Tested with a Pandaboard and a Beagle X15 board.
> 
> Tested to have no adverse effect on Pyra (omap5432).
> But I have not tested if CEC itself is working.

I tested on DRA76 EVM, but I don't have a CEC peripheral either.

> 
>>
>> Regards,
>>
>> 	Hans
>>
>> Hans Verkuil (5):
>>  drm: drm_bridge: add cec_init/exit bridge ops
>>  drm/omap: hdmi4: switch to the cec bridge ops
>>  drm/omap: hdmi4: simplify CEC Phys Addr handling
>>  drm/omap: hdmi5: add CEC support
>>  ARM: dts: dra7/omap5: add cec clock
>>
>> arch/arm/boot/dts/dra7.dtsi              |   5 +-
>> arch/arm/boot/dts/omap5.dtsi             |   5 +-
>> drivers/gpu/drm/drm_bridge_connector.c   |  23 +++
>> drivers/gpu/drm/omapdrm/dss/Kconfig      |   8 +
>> drivers/gpu/drm/omapdrm/dss/Makefile     |   1 +
> 
> Merging with patch series by Tomi Valkeinen and Sebastian Reichel
> for omapdrm/dsi will need to move the Kconfig and Makefile one level
> up.

Yes, this conflicts with drm-next due to the Kconfig and Makefile
changes. Should be trivial to fix.

 Tomi
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel