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),

};

/**

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; }

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; }

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);

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