[PATCH 00/13] media/drm: cec: add EDID_PRESENT

[PATCH 00/13] media/drm: cec: add EDID_PRESENT

[Hans Verkuil]

Using the media CEC framework the application receives events when the
Physical and/or Logical Addresses change. However, if the Physical Address
changes to f.f.f.f (Invalid), it does not know whether that is because the
HDMI link is disconnected, or because the EDID does not provide a valid
Physical Address. That can happen because the display does not support CEC,
as is the case for DisplayPort inputs or PC monitors.

This is important information to have but this is currently missing.

This series adds support for that.

Currently drivers set the physical address either with cec_s_phys_addr() or
with cec_phys_addr_invalidate(), where the second is just a helper
function that calls cec_s_phys_addr with CEC_PHYS_ADDR_INVALID. There
are also cec_notifier_ counterparts.

Change the behavior so that cec_s_phys_addr() is called whenever there
is an EDID. In that case CEC_PHYS_ADDR_INVALID means that the EDID is
present but does not contain a valid PA. And cec_phys_addr_invalidate()
is only called when the video link is disconnected.

Most CEC drivers already did that, except for the few that are fixed in the
first 6 patches.

The remaining patches implement the new functionality where the CEC
framework actually reports whether or not the EDID was present, based
on whether cec_s_phys_addr or cec_phys_addr_invalidate was called.

The drm patches can either be queued up in the drm subsystem independently
of the remaining patches, or I can take them through the media subsytem
as part of the series, but then I need an Acked/Reviewed-by.

Regards,

	Hans

Hans Verkuil (13):
  gpu: drm: sti: call cec_notifier_phys_addr_invalidate if no EDID
  gpu: drm: exynos: call cec_notifier_phys_addr_invalidate if no EDID
  gpu: drm: omapdrm: call cec_s_phys_addr only if EDID is present
  media: i2c: adv7511-v4l2: call cec_s_phys_addr only if EDID is present
  media: cec: extron-da-hd-4k-plus: call cec_s_phys_addr only if EDID is
    present
  media: cec: core: improve cec_notifier_phys_addr_invalidate
  media: uapi: cec.h: add
    CEC_CAP_HAS_EDID_PRESENT/CEC_LOG_ADDRS_FL_EDID_PRESENT
  Documentation: media: document EDID_PRESENT cap and flag.
  media: cec: core: add support for EDID_PRESENT
  media: uapi: cec.h: add CEC_EVENT_FL_STATE_EDID_PRESENT flag
  Documentation: media: cec-ioc-dqevent.rst: document STATE_EDID_PRESENT
    flag
  media: cec: core: implement CEC_EVENT_FL_STATE_EDID_PRESENT
  Documentation: media: document cec_phys_addr_invalidate

 Documentation/driver-api/media/cec-core.rst   | 10 +++-
 .../media/cec/cec-ioc-adap-g-caps.rst         | 10 ++++
 .../media/cec/cec-ioc-adap-g-log-addrs.rst    |  9 ++++
 .../media/cec/cec-ioc-dqevent.rst             | 10 ++++
 drivers/gpu/drm/exynos/exynos_hdmi.c          |  2 +-
 drivers/gpu/drm/omapdrm/dss/hdmi4.c           |  7 ++-
 drivers/gpu/drm/omapdrm/dss/hdmi4_cec.c       |  5 ++
 drivers/gpu/drm/omapdrm/dss/hdmi4_cec.h       |  5 ++
 drivers/gpu/drm/sti/sti_hdmi.c                |  2 +-
 drivers/media/cec/core/cec-adap.c             | 49 ++++++++++++++++---
 drivers/media/cec/core/cec-api.c              |  3 +-
 drivers/media/cec/core/cec-core.c             |  4 +-
 drivers/media/cec/core/cec-notifier.c         | 13 +++++
 drivers/media/cec/core/cec-priv.h             |  3 +-
 .../extron-da-hd-4k-plus.c                    |  8 ++-
 drivers/media/i2c/adv7511-v4l2.c              |  2 +-
 include/media/cec-notifier.h                  | 33 +++++++------
 include/media/cec.h                           | 27 +++++-----
 include/uapi/linux/cec.h                      | 29 ++++++-----
 19 files changed, 171 insertions(+), 60 deletions(-)

-- 
2.53.0

[PATCH 01/13] gpu: drm: sti: call cec_notifier_phys_addr_invalidate if no EDID

[Hans Verkuil]

If an EDID is present, then call cec_notifier_set_phys_addr. If not, then
call cec_notifier_phys_addr_invalidate. This helps the CEC framework
differentiate between CEC_PHYS_ADDR_INVALID due to no EDID, or due to an
EDID without a Physical Address.

This driver is one of the few that was using cec_notifier_set_phys_addr
on disconnect instead of cec_notifier_phys_addr_invalidate.

Signed-off-by: Hans Verkuil <hverkuil+cisco@kernel.org>
Cc: Alain Volmat <alain.volmat@foss.st.com>
Cc: Maxime Ripard <mripard@kernel.org>
---
 drivers/gpu/drm/sti/sti_hdmi.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/sti/sti_hdmi.c b/drivers/gpu/drm/sti/sti_hdmi.c
index 7ba9e0b7ba6c..d392140df235 100644
--- a/drivers/gpu/drm/sti/sti_hdmi.c
+++ b/drivers/gpu/drm/sti/sti_hdmi.c
@@ -790,7 +790,7 @@ static void sti_hdmi_disable(struct drm_bridge *bridge,
 
 	hdmi->enabled = false;
 
-	cec_notifier_set_phys_addr(hdmi->notifier, CEC_PHYS_ADDR_INVALID);
+	cec_notifier_phys_addr_invalidate(hdmi->notifier);
 }
 
 /*
-- 
2.53.0

[PATCH 02/13] gpu: drm: exynos: call cec_notifier_phys_addr_invalidate if no EDID

[Hans Verkuil]

If an EDID is present, then call cec_notifier_set_phys_addr. If not, then
call cec_notifier_phys_addr_invalidate. This helps the CEC framework
differentiate between CEC_PHYS_ADDR_INVALID due to no EDID, or due to an
EDID without a Physical Address.

This driver is one of the few that was using cec_notifier_set_phys_addr
on disconnect instead of cec_notifier_phys_addr_invalidate.

Signed-off-by: Hans Verkuil <hverkuil+cisco@kernel.org>
Cc: Inki Dae <inki.dae@samsung.com>
Cc: Kyungmin Park <kyungmin.park@samsung.com>
Cc: Krzysztof Kozlowski <krzk@kernel.org>
Cc: Peter Griffin <peter.griffin@linaro.org>
Cc: Alim Akhtar <alim.akhtar@samsung.com>
Cc: Maxime Ripard <mripard@kernel.org>
---
 drivers/gpu/drm/exynos/exynos_hdmi.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/exynos/exynos_hdmi.c b/drivers/gpu/drm/exynos/exynos_hdmi.c
index 055dc04f9b3e..d03f4e531e05 100644
--- a/drivers/gpu/drm/exynos/exynos_hdmi.c
+++ b/drivers/gpu/drm/exynos/exynos_hdmi.c
@@ -857,7 +857,7 @@ static enum drm_connector_status hdmi_detect(struct drm_connector *connector,
 	if (gpiod_get_value(hdata->hpd_gpio))
 		return connector_status_connected;
 
-	cec_notifier_set_phys_addr(hdata->notifier, CEC_PHYS_ADDR_INVALID);
+	cec_notifier_phys_addr_invalidate(hdata->notifier);
 	return connector_status_disconnected;
 }
 
-- 
2.53.0

[PATCH 03/13] gpu: drm: omapdrm: call cec_s_phys_addr only if EDID is present

[Hans Verkuil]

If an EDID is present, then call cec_s_phys_addr. If not, then
call cec_phys_addr_invalidate. This helps the CEC framework differentiate
between CEC_PHYS_ADDR_INVALID due to no EDID, or due to an EDID without
a Physical Address.

This driver is one of the few that was doing this inconsistently.

Signed-off-by: Hans Verkuil <hverkuil+cisco@kernel.org>
Cc: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
Cc: Maxime Ripard <mripard@kernel.org>
---
 drivers/gpu/drm/omapdrm/dss/hdmi4.c     | 7 +++----
 drivers/gpu/drm/omapdrm/dss/hdmi4_cec.c | 5 +++++
 drivers/gpu/drm/omapdrm/dss/hdmi4_cec.h | 5 +++++
 3 files changed, 13 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/omapdrm/dss/hdmi4.c b/drivers/gpu/drm/omapdrm/dss/hdmi4.c
index bd6c4d2dcdc8..056bede2ec8a 100644
--- a/drivers/gpu/drm/omapdrm/dss/hdmi4.c
+++ b/drivers/gpu/drm/omapdrm/dss/hdmi4.c
@@ -439,7 +439,7 @@ static void hdmi4_bridge_hpd_notify(struct drm_bridge *bridge,
 		if (hdmi_runtime_get(hdmi))
 			return;
 
-		hdmi4_cec_set_phys_addr(&hdmi->core, CEC_PHYS_ADDR_INVALID);
+		hdmi4_cec_phys_addr_invalidate(&hdmi->core);
 		hdmi_runtime_put(hdmi);
 	}
 }
@@ -487,12 +487,11 @@ static const struct drm_edid *hdmi4_bridge_edid_read(struct drm_bridge *bridge,
 		unsigned int len = (edid->extensions + 1) * EDID_LENGTH;
 
 		cec_addr = cec_get_edid_phys_addr((u8 *)edid, len, NULL);
+		hdmi4_cec_set_phys_addr(&hdmi->core, cec_addr);
 	} else {
-		cec_addr = CEC_PHYS_ADDR_INVALID;
+		hdmi4_cec_phys_addr_invalidate(&hdmi->core);
 	}
 
-	hdmi4_cec_set_phys_addr(&hdmi->core, cec_addr);
-
 	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 852987e67e40..60d0d85dccad 100644
--- a/drivers/gpu/drm/omapdrm/dss/hdmi4_cec.c
+++ b/drivers/gpu/drm/omapdrm/dss/hdmi4_cec.c
@@ -322,6 +322,11 @@ void hdmi4_cec_set_phys_addr(struct hdmi_core_data *core, u16 pa)
 	cec_s_phys_addr(core->adap, pa, false);
 }
 
+void hdmi4_cec_phys_addr_invalidate(struct hdmi_core_data *core)
+{
+	cec_phys_addr_invalidate(core->adap);
+}
+
 int hdmi4_cec_init(struct platform_device *pdev, struct hdmi_core_data *core,
 		  struct hdmi_wp_data *wp)
 {
diff --git a/drivers/gpu/drm/omapdrm/dss/hdmi4_cec.h b/drivers/gpu/drm/omapdrm/dss/hdmi4_cec.h
index 1ca5b5ca8a99..234690dd3a1e 100644
--- a/drivers/gpu/drm/omapdrm/dss/hdmi4_cec.h
+++ b/drivers/gpu/drm/omapdrm/dss/hdmi4_cec.h
@@ -15,6 +15,7 @@ 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_phys_addr_invalidate(struct hdmi_core_data *core);
 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);
@@ -24,6 +25,10 @@ static inline void hdmi4_cec_set_phys_addr(struct hdmi_core_data *core, u16 pa)
 {
 }
 
+static inline void hdmi4_cec_phys_addr_invalidate(struct hdmi_core_data *core)
+{
+}
+
 static inline void hdmi4_cec_irq(struct hdmi_core_data *core)
 {
 }
-- 
2.53.0

[PATCH 04/13] media: i2c: adv7511-v4l2: call cec_s_phys_addr only if EDID is present

[Hans Verkuil]

If an EDID is present, then call cec_s_phys_addr. If not, then
call cec_phys_addr_invalidate. This helps the CEC framework differentiate
between CEC_PHYS_ADDR_INVALID due to no EDID, or due to an EDID without
a Physical Address.

Signed-off-by: Hans Verkuil <hverkuil+cisco@kernel.org>
---
 drivers/media/i2c/adv7511-v4l2.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/media/i2c/adv7511-v4l2.c b/drivers/media/i2c/adv7511-v4l2.c
index 27e0e2c31dbd..1cc87049c6be 100644
--- a/drivers/media/i2c/adv7511-v4l2.c
+++ b/drivers/media/i2c/adv7511-v4l2.c
@@ -1466,7 +1466,7 @@ static void adv7511_notify_no_edid(struct v4l2_subdev *sd)
 	ed.present = false;
 	ed.segment = adv7511_rd(sd, 0xc4);
 	ed.phys_addr = CEC_PHYS_ADDR_INVALID;
-	cec_s_phys_addr(state->cec_adap, ed.phys_addr, false);
+	cec_phys_addr_invalidate(state->cec_adap);
 	v4l2_subdev_notify(sd, ADV7511_EDID_DETECT, (void *)&ed);
 	v4l2_ctrl_s_ctrl(state->have_edid0_ctrl, 0x0);
 }
-- 
2.53.0

[PATCH 05/13] media: cec: extron-da-hd-4k-plus: call cec_s_phys_addr only if EDID is present

[Hans Verkuil]

If an EDID is present, then call cec_s_phys_addr. If not, then
call cec_phys_addr_invalidate. This helps the CEC framework differentiate
between CEC_PHYS_ADDR_INVALID due to no EDID, or due to an EDID without
a Physical Address.

Signed-off-by: Hans Verkuil <hverkuil+cisco@kernel.org>
---
 .../cec/usb/extron-da-hd-4k-plus/extron-da-hd-4k-plus.c   | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/media/cec/usb/extron-da-hd-4k-plus/extron-da-hd-4k-plus.c b/drivers/media/cec/usb/extron-da-hd-4k-plus/extron-da-hd-4k-plus.c
index 1f8ed7ff31e5..0a0724fd89c0 100644
--- a/drivers/media/cec/usb/extron-da-hd-4k-plus/extron-da-hd-4k-plus.c
+++ b/drivers/media/cec/usb/extron-da-hd-4k-plus/extron-da-hd-4k-plus.c
@@ -643,8 +643,12 @@ static void extron_irq_work_handler(struct work_struct *work)
 			extron_read_edid(port);
 		}
 	}
-	if (update_pa)
-		cec_s_phys_addr(port->adap, pa, false);
+	if (update_pa) {
+		if (has_edid)
+			cec_s_phys_addr(port->adap, pa, false);
+		else
+			cec_phys_addr_invalidate(port->adap);
+	}
 }
 
 static void extron_process_received(struct extron_port *port, const char *data)
-- 
2.53.0

[PATCH 06/13] media: cec: core: improve cec_notifier_phys_addr_invalidate

[Hans Verkuil]

cec_notifier_phys_addr_invalidate must call cec_phys_addr_invalidate
instead of cec_s_phys_addr. This ensures that the CEC framework knowns
that there is no EDID.

Signed-off-by: Hans Verkuil <hverkuil+cisco@kernel.org>
---
 drivers/media/cec/core/cec-notifier.c | 13 +++++++++++
 include/media/cec-notifier.h          | 33 +++++++++++++++------------
 2 files changed, 32 insertions(+), 14 deletions(-)

diff --git a/drivers/media/cec/core/cec-notifier.c b/drivers/media/cec/core/cec-notifier.c
index 8cf1474d0361..2814ef7d4804 100644
--- a/drivers/media/cec/core/cec-notifier.c
+++ b/drivers/media/cec/core/cec-notifier.c
@@ -216,6 +216,19 @@ void cec_notifier_set_phys_addr_from_edid(struct cec_notifier *n,
 }
 EXPORT_SYMBOL_GPL(cec_notifier_set_phys_addr_from_edid);
 
+void cec_notifier_phys_addr_invalidate(struct cec_notifier *n)
+{
+	if (!n)
+		return;
+
+	mutex_lock(&n->lock);
+	n->phys_addr = CEC_PHYS_ADDR_INVALID;
+	if (n->cec_adap && !n->cec_adap->adap_controls_phys_addr)
+		cec_phys_addr_invalidate(n->cec_adap);
+	mutex_unlock(&n->lock);
+}
+EXPORT_SYMBOL_GPL(cec_notifier_phys_addr_invalidate);
+
 struct device *cec_notifier_parse_hdmi_phandle(struct device *dev)
 {
 	struct platform_device *hdmi_pdev;
diff --git a/include/media/cec-notifier.h b/include/media/cec-notifier.h
index b1c839734124..15e9bcb5b887 100644
--- a/include/media/cec-notifier.h
+++ b/include/media/cec-notifier.h
@@ -80,7 +80,10 @@ void cec_notifier_cec_adap_unregister(struct cec_notifier *n,
  * @n: the CEC notifier
  * @pa: the CEC physical address
  *
- * Set a new CEC physical address.
+ * Set a new CEC physical address. @pa may be f.f.f.f, indicating that
+ * the EDID does not contain a valid Physical Address.
+ * Use @cec_notifier_phys_addr_invalidate() when there is no EDID at all,
+ * i.e. there is no video sink connected.
  * Does nothing if @n == NULL.
  */
 void cec_notifier_set_phys_addr(struct cec_notifier *n, u16 pa);
@@ -96,6 +99,17 @@ void cec_notifier_set_phys_addr(struct cec_notifier *n, u16 pa);
 void cec_notifier_set_phys_addr_from_edid(struct cec_notifier *n,
 					  const struct edid *edid);
 
+/**
+ * cec_notifier_phys_addr_invalidate() - set the physical address to INVALID
+ *
+ * @n: the CEC notifier
+ *
+ * This is a simple helper function to invalidate the physical
+ * address when the video sink disappears.
+ * Does nothing if @n == NULL.
+ */
+void cec_notifier_phys_addr_invalidate(struct cec_notifier *n);
+
 /**
  * cec_notifier_parse_hdmi_phandle - find the hdmi device from "hdmi-phandle"
  * @dev: the device with the "hdmi-phandle" device tree property
@@ -143,6 +157,10 @@ static inline void cec_notifier_set_phys_addr_from_edid(struct cec_notifier *n,
 {
 }
 
+static inline void cec_notifier_phys_addr_invalidate(struct cec_notifier *n)
+{
+}
+
 static inline struct device *cec_notifier_parse_hdmi_phandle(struct device *dev)
 {
 	return ERR_PTR(-ENODEV);
@@ -150,17 +168,4 @@ static inline struct device *cec_notifier_parse_hdmi_phandle(struct device *dev)
 
 #endif
 
-/**
- * cec_notifier_phys_addr_invalidate() - set the physical address to INVALID
- *
- * @n: the CEC notifier
- *
- * This is a simple helper function to invalidate the physical
- * address. Does nothing if @n == NULL.
- */
-static inline void cec_notifier_phys_addr_invalidate(struct cec_notifier *n)
-{
-	cec_notifier_set_phys_addr(n, CEC_PHYS_ADDR_INVALID);
-}
-
 #endif
-- 
2.53.0

[PATCH 07/13] media: uapi: cec.h: add CEC_CAP_HAS_EDID_PRESENT/CEC_LOG_ADDRS_FL_EDID_PRESENT

[Hans Verkuil]

Add a capability and a new flag that is used to indicate if an
EDID is present.

Signed-off-by: Hans Verkuil <hverkuil+cisco@kernel.org>
---
 include/uapi/linux/cec.h | 28 +++++++++++++++-------------
 1 file changed, 15 insertions(+), 13 deletions(-)

diff --git a/include/uapi/linux/cec.h b/include/uapi/linux/cec.h
index fdfc97a6e4ec..8f7c7a4cb505 100644
--- a/include/uapi/linux/cec.h
+++ b/include/uapi/linux/cec.h
@@ -325,27 +325,27 @@ static inline int cec_is_unconfigured(__u16 log_addr_mask)
 #define CEC_MODE_FOLLOWER_MSK		0xf0
 
 /* Userspace has to configure the physical address */
-#define CEC_CAP_PHYS_ADDR	(1 << 0)
+#define CEC_CAP_PHYS_ADDR		(1 << 0)
 /* Userspace has to configure the logical addresses */
-#define CEC_CAP_LOG_ADDRS	(1 << 1)
+#define CEC_CAP_LOG_ADDRS		(1 << 1)
 /* Userspace can transmit messages (and thus become follower as well) */
-#define CEC_CAP_TRANSMIT	(1 << 2)
-/*
- * Passthrough all messages instead of processing them.
- */
-#define CEC_CAP_PASSTHROUGH	(1 << 3)
+#define CEC_CAP_TRANSMIT		(1 << 2)
+/* Passthrough all messages instead of processing them. */
+#define CEC_CAP_PASSTHROUGH		(1 << 3)
 /* Supports remote control */
-#define CEC_CAP_RC		(1 << 4)
+#define CEC_CAP_RC			(1 << 4)
 /* Hardware can monitor all messages, not just directed and broadcast. */
-#define CEC_CAP_MONITOR_ALL	(1 << 5)
+#define CEC_CAP_MONITOR_ALL		(1 << 5)
 /* Hardware can use CEC only if the HDMI HPD pin is high. */
-#define CEC_CAP_NEEDS_HPD	(1 << 6)
+#define CEC_CAP_NEEDS_HPD		(1 << 6)
 /* Hardware can monitor CEC pin transitions */
-#define CEC_CAP_MONITOR_PIN	(1 << 7)
+#define CEC_CAP_MONITOR_PIN		(1 << 7)
 /* CEC_ADAP_G_CONNECTOR_INFO is available */
-#define CEC_CAP_CONNECTOR_INFO	(1 << 8)
+#define CEC_CAP_CONNECTOR_INFO		(1 << 8)
 /* CEC_MSG_FL_REPLY_VENDOR_ID is available */
-#define CEC_CAP_REPLY_VENDOR_ID	(1 << 9)
+#define CEC_CAP_REPLY_VENDOR_ID		(1 << 9)
+/* CEC_LOG_ADDRS_FL_EDID_PRESENT is available */
+#define CEC_CAP_HAS_EDID_PRESENT	(1 << 10)
 
 /**
  * struct cec_caps - CEC capabilities structure.
@@ -405,6 +405,8 @@ struct cec_log_addrs {
 #define CEC_LOG_ADDRS_FL_CDC_ONLY		(1 << 2)
 /* Configuration failed */
 #define CEC_LOG_ADDRS_FL_CONFIG_FAILED		(1 << 3)
+/* EDID is present */
+#define CEC_LOG_ADDRS_FL_EDID_PRESENT		(1 << 4)
 
 /**
  * struct cec_drm_connector_info - tells which drm connector is
-- 
2.53.0

[PATCH 08/13] Documentation: media: document EDID_PRESENT cap and flag.

[Hans Verkuil]

Document CEC_CAP_HAS_EDID_PRESENT and CEC_LOG_ADDRS_FL_EDID_PRESENT.

Signed-off-by: Hans Verkuil <hverkuil+cisco@kernel.org>
---
 .../userspace-api/media/cec/cec-ioc-adap-g-caps.rst      | 9 +++++++++
 .../userspace-api/media/cec/cec-ioc-adap-g-log-addrs.rst | 9 +++++++++
 2 files changed, 18 insertions(+)

diff --git a/Documentation/userspace-api/media/cec/cec-ioc-adap-g-caps.rst b/Documentation/userspace-api/media/cec/cec-ioc-adap-g-caps.rst
index 1d5248979a6d..abd62ac64541 100644
--- a/Documentation/userspace-api/media/cec/cec-ioc-adap-g-caps.rst
+++ b/Documentation/userspace-api/media/cec/cec-ioc-adap-g-caps.rst
@@ -143,6 +143,15 @@ returns the information to the application. The ioctl never fails.
       - 0x00000200
       - If this capability is set, then
         :ref:`CEC_MSG_FL_REPLY_VENDOR_ID <cec-msg-flags>` can be used.
+    * .. _`CEC-CAP-HAS-EDID-PRESENT`:
+
+      - ``CEC_CAP_HAS_EDID_PRESENT``
+      - 0x00000400
+      - If this capability is set, then
+        :ref:`CEC_LOG_ADDRS_FL_EDID_PRESENT <cec-log-addrs-flags>` is valid.
+	This makes it possible to tell apart the case where the Physical Address
+	is f.f.f.f because no EDID is present, and the case where it is f.f.f.f
+	because the EDID does not provide a valid Physical Address.
 
 Return Value
 ============
diff --git a/Documentation/userspace-api/media/cec/cec-ioc-adap-g-log-addrs.rst b/Documentation/userspace-api/media/cec/cec-ioc-adap-g-log-addrs.rst
index 8397bf573798..2088793b1506 100644
--- a/Documentation/userspace-api/media/cec/cec-ioc-adap-g-log-addrs.rst
+++ b/Documentation/userspace-api/media/cec/cec-ioc-adap-g-log-addrs.rst
@@ -192,6 +192,15 @@ logical address types are already defined will return with error ``EBUSY``.
       - If this flag is set, then the CEC device failed to claim a free logical
         address and is in the unconfigured state. This can never happen if
 	``CEC_LOG_ADDRS_FL_ALLOW_UNREG_FALLBACK`` was set as well.
+    * .. _`CEC-LOG-ADDRS-FL-EDID-PRESENT`:
+
+      - ``CEC_LOG_ADDRS_FL_EDID_PRESENT``
+      - 16
+      - If this flag is set, then an EDID is present. This helps tell apart the
+        case where the Physical Address is f.f.f.f because no EDID is present,
+	and the case where it is f.f.f.f because the EDID does not provide a
+	valid Physical Address. This flag can only be used if
+	``CEC_CAP_HAS_EDID_PRESENT`` is set in the adapter capabilities.
 
 .. tabularcolumns:: |p{7.8cm}|p{1.0cm}|p{8.5cm}|
 
-- 
2.53.0

[PATCH 09/13] media: cec: core: add support for EDID_PRESENT

[Hans Verkuil]

There can be two reasons for the CEC Physical Address to be invalid:
either there is no EDID, or there is an EDID, but it does not contain
a Physical Address, or the Physical Address is set to f.f.f.f in the EDID.

For debugging it is very useful to know why the PA is invalid.

Add support for reporting this to userspace by changing the behavior of
two CEC functions: cec_s_phys_addr is called with the PA read from the
EDID (or CEC_PHYS_ADDR_INVALID if the EDID has no PA or the PA is f.f.f.f),
and cec_phys_addr_invalidate is only called if the HPD goes low, so the
EDID is gone.

The first function sets the new CEC_LOG_ADDRS_FL_EDID_PRESENT flag, and
the second function removes it.

Signed-off-by: Hans Verkuil <hverkuil+cisco@kernel.org>
---
 drivers/media/cec/core/cec-adap.c | 47 ++++++++++++++++++++++++++-----
 drivers/media/cec/core/cec-api.c  |  3 +-
 drivers/media/cec/core/cec-core.c |  4 ++-
 drivers/media/cec/core/cec-priv.h |  3 +-
 include/media/cec.h               | 27 +++++++++---------
 5 files changed, 61 insertions(+), 23 deletions(-)

diff --git a/drivers/media/cec/core/cec-adap.c b/drivers/media/cec/core/cec-adap.c
index ee8a200b9803..06a596a0ede3 100644
--- a/drivers/media/cec/core/cec-adap.c
+++ b/drivers/media/cec/core/cec-adap.c
@@ -1727,18 +1727,33 @@ int cec_adap_enable(struct cec_adapter *adap)
  *
  * This function is called with adap->lock held.
  */
-void __cec_s_phys_addr(struct cec_adapter *adap, u16 phys_addr, bool block)
+void __cec_s_phys_addr(struct cec_adapter *adap, bool edid_present,
+		       u16 phys_addr, bool block)
 {
 	bool becomes_invalid = phys_addr == CEC_PHYS_ADDR_INVALID;
 	bool is_invalid = adap->phys_addr == CEC_PHYS_ADDR_INVALID;
+	bool edid_was_present =
+		adap->log_addrs.flags & CEC_LOG_ADDRS_FL_EDID_PRESENT;
 
-	if (phys_addr == adap->phys_addr)
-		return;
+	if (adap->capabilities & CEC_CAP_HAS_EDID_PRESENT) {
+		if (edid_present)
+			adap->log_addrs.flags |= CEC_LOG_ADDRS_FL_EDID_PRESENT;
+		else
+			adap->log_addrs.flags &= ~CEC_LOG_ADDRS_FL_EDID_PRESENT;
+	}
 	if (!becomes_invalid && adap->devnode.unregistered)
 		return;
+	if (phys_addr == adap->phys_addr) {
+		if (edid_was_present != edid_present) {
+			dprintk(1, "EDID is %spresent\n",
+				edid_present ? "" : "not ");
+			cec_post_state_event(adap);
+		}
+		return;
+	}
 
-	dprintk(1, "new physical address %x.%x.%x.%x\n",
-		cec_phys_addr_exp(phys_addr));
+	dprintk(1, "new physical address %x.%x.%x.%x, EDID is %spresent\n",
+		cec_phys_addr_exp(phys_addr), edid_present ? "" : "not ");
 	if (becomes_invalid || !is_invalid) {
 		adap->phys_addr = CEC_PHYS_ADDR_INVALID;
 		cec_adap_unconfigure(adap);
@@ -1767,11 +1782,22 @@ void cec_s_phys_addr(struct cec_adapter *adap, u16 phys_addr, bool block)
 		return;
 
 	mutex_lock(&adap->lock);
-	__cec_s_phys_addr(adap, phys_addr, block);
+	__cec_s_phys_addr(adap, true, phys_addr, block);
 	mutex_unlock(&adap->lock);
 }
 EXPORT_SYMBOL_GPL(cec_s_phys_addr);
 
+void cec_phys_addr_invalidate(struct cec_adapter *adap)
+{
+	if (IS_ERR_OR_NULL(adap))
+		return;
+
+	mutex_lock(&adap->lock);
+	__cec_s_phys_addr(adap, false, CEC_PHYS_ADDR_INVALID, false);
+	mutex_unlock(&adap->lock);
+}
+EXPORT_SYMBOL_GPL(cec_phys_addr_invalidate);
+
 /*
  * Note: In the drm subsystem, prefer calling (if possible):
  *
@@ -1782,10 +1808,15 @@ void cec_s_phys_addr_from_edid(struct cec_adapter *adap,
 {
 	u16 pa = CEC_PHYS_ADDR_INVALID;
 
+	if (IS_ERR_OR_NULL(adap))
+		return;
+
 	if (edid && edid->extensions)
 		pa = cec_get_edid_phys_addr((const u8 *)edid,
 				EDID_LENGTH * (edid->extensions + 1), NULL);
-	cec_s_phys_addr(adap, pa, false);
+	mutex_lock(&adap->lock);
+	__cec_s_phys_addr(adap, edid, pa, false);
+	mutex_unlock(&adap->lock);
 }
 EXPORT_SYMBOL_GPL(cec_s_phys_addr_from_edid);
 
@@ -1969,6 +2000,8 @@ int __cec_s_log_addrs(struct cec_adapter *adap,
 	}
 
 	log_addrs->log_addr_mask = adap->log_addrs.log_addr_mask;
+	log_addrs->flags |=
+		adap->log_addrs.flags & CEC_LOG_ADDRS_FL_EDID_PRESENT;
 	adap->log_addrs = *log_addrs;
 	err = cec_adap_enable(adap);
 	if (!err && adap->phys_addr != CEC_PHYS_ADDR_INVALID)
diff --git a/drivers/media/cec/core/cec-api.c b/drivers/media/cec/core/cec-api.c
index a491d7fc8a71..1ba080d5a123 100644
--- a/drivers/media/cec/core/cec-api.c
+++ b/drivers/media/cec/core/cec-api.c
@@ -136,7 +136,8 @@ static long cec_adap_s_phys_addr(struct cec_adapter *adap, struct cec_fh *fh,
 	if (cec_is_busy(adap, fh))
 		err = -EBUSY;
 	else
-		__cec_s_phys_addr(adap, phys_addr, block);
+		__cec_s_phys_addr(adap, phys_addr != CEC_PHYS_ADDR_INVALID,
+				  phys_addr, block);
 	mutex_unlock(&adap->lock);
 	return err;
 }
diff --git a/drivers/media/cec/core/cec-core.c b/drivers/media/cec/core/cec-core.c
index 230f8a413e74..9767f7c55b46 100644
--- a/drivers/media/cec/core/cec-core.c
+++ b/drivers/media/cec/core/cec-core.c
@@ -178,7 +178,7 @@ static void cec_devnode_unregister(struct cec_adapter *adap)
 	mutex_unlock(&devnode->lock);
 
 	mutex_lock(&adap->lock);
-	__cec_s_phys_addr(adap, CEC_PHYS_ADDR_INVALID, false);
+	__cec_s_phys_addr(adap, false, CEC_PHYS_ADDR_INVALID, false);
 	__cec_s_log_addrs(adap, NULL, false);
 	// Disable the adapter (since adap->devnode.unregistered is true)
 	cec_adap_enable(adap);
@@ -279,6 +279,8 @@ struct cec_adapter *cec_allocate_adapter(const struct cec_adap_ops *ops,
 	adap->log_addrs.cec_version = CEC_OP_CEC_VERSION_2_0;
 	adap->log_addrs.vendor_id = CEC_VENDOR_ID_NONE;
 	adap->capabilities = caps | CEC_CAP_REPLY_VENDOR_ID;
+	if (!(caps & CEC_CAP_PHYS_ADDR))
+		adap->capabilities |= CEC_CAP_HAS_EDID_PRESENT;
 	if (debug_phys_addr)
 		adap->capabilities |= CEC_CAP_PHYS_ADDR;
 	adap->needs_hpd = caps & CEC_CAP_NEEDS_HPD;
diff --git a/drivers/media/cec/core/cec-priv.h b/drivers/media/cec/core/cec-priv.h
index ce42a37c4ac0..8ccf9ac868ba 100644
--- a/drivers/media/cec/core/cec-priv.h
+++ b/drivers/media/cec/core/cec-priv.h
@@ -46,7 +46,8 @@ void cec_monitor_pin_cnt_dec(struct cec_adapter *adap);
 int cec_adap_status(struct seq_file *file, void *priv);
 int cec_thread_func(void *_adap);
 int cec_adap_enable(struct cec_adapter *adap);
-void __cec_s_phys_addr(struct cec_adapter *adap, u16 phys_addr, bool block);
+void __cec_s_phys_addr(struct cec_adapter *adap, bool edid_present,
+		       u16 phys_addr, bool block);
 int __cec_s_log_addrs(struct cec_adapter *adap,
 		      struct cec_log_addrs *log_addrs, bool block);
 int cec_transmit_msg_fh(struct cec_adapter *adap, struct cec_msg *msg,
diff --git a/include/media/cec.h b/include/media/cec.h
index 4c8b0950b377..bfdc2987f7b1 100644
--- a/include/media/cec.h
+++ b/include/media/cec.h
@@ -381,6 +381,16 @@ void cec_s_conn_info(struct cec_adapter *adap,
 int cec_transmit_msg(struct cec_adapter *adap, struct cec_msg *msg,
 		     bool block);
 
+/**
+ * cec_phys_addr_invalidate() - set the physical address to INVALID
+ *
+ * @adap:	the CEC adapter
+ *
+ * This is a simple helper function to invalidate the physical
+ * address when the EDID is lost.
+ */
+void cec_phys_addr_invalidate(struct cec_adapter *adap);
+
 /* Called by the adapter */
 void cec_transmit_done_ts(struct cec_adapter *adap, u8 status,
 			  u8 arb_lost_cnt, u8 nack_cnt, u8 low_drive_cnt,
@@ -491,6 +501,10 @@ static inline void cec_s_phys_addr_from_edid(struct cec_adapter *adap,
 {
 }
 
+static inline void cec_phys_addr_invalidate(struct cec_adapter *adap)
+{
+}
+
 static inline u16 cec_get_edid_phys_addr(const u8 *edid, unsigned int size,
 					 unsigned int *offset)
 {
@@ -513,19 +527,6 @@ cec_fill_conn_info_from_drm(struct cec_connector_info *conn_info,
 
 #endif
 
-/**
- * cec_phys_addr_invalidate() - set the physical address to INVALID
- *
- * @adap:	the CEC adapter
- *
- * This is a simple helper function to invalidate the physical
- * address.
- */
-static inline void cec_phys_addr_invalidate(struct cec_adapter *adap)
-{
-	cec_s_phys_addr(adap, CEC_PHYS_ADDR_INVALID, false);
-}
-
 /**
  * cec_get_edid_spa_location() - find location of the Source Physical Address
  *
-- 
2.53.0

[PATCH 10/13] media: uapi: cec.h: add CEC_EVENT_FL_STATE_EDID_PRESENT flag

[Hans Verkuil]

The CEC_EVENT_FL_STATE_EDID_PRESENT will report whether the EDID is
present or not.

Signed-off-by: Hans Verkuil <hverkuil+cisco@kernel.org>
---
 include/uapi/linux/cec.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/include/uapi/linux/cec.h b/include/uapi/linux/cec.h
index 8f7c7a4cb505..8324d208c1d5 100644
--- a/include/uapi/linux/cec.h
+++ b/include/uapi/linux/cec.h
@@ -455,6 +455,7 @@ struct cec_connector_info {
 
 #define CEC_EVENT_FL_INITIAL_STATE	(1 << 0)
 #define CEC_EVENT_FL_DROPPED_EVENTS	(1 << 1)
+#define CEC_EVENT_FL_STATE_EDID_PRESENT	(1 << 2)
 
 /**
  * struct cec_event_state_change - used when the CEC adapter changes state.
-- 
2.53.0

[PATCH 11/13] Documentation: media: cec-ioc-dqevent.rst: document STATE_EDID_PRESENT flag

[Hans Verkuil]

Document the CEC_EVENT_FL_STATE_EDID_PRESENT event flag.

Signed-off-by: Hans Verkuil <hverkuil+cisco@kernel.org>
---
 .../userspace-api/media/cec/cec-ioc-adap-g-caps.rst    |  3 ++-
 .../userspace-api/media/cec/cec-ioc-dqevent.rst        | 10 ++++++++++
 2 files changed, 12 insertions(+), 1 deletion(-)

diff --git a/Documentation/userspace-api/media/cec/cec-ioc-adap-g-caps.rst b/Documentation/userspace-api/media/cec/cec-ioc-adap-g-caps.rst
index abd62ac64541..49bd90d9392f 100644
--- a/Documentation/userspace-api/media/cec/cec-ioc-adap-g-caps.rst
+++ b/Documentation/userspace-api/media/cec/cec-ioc-adap-g-caps.rst
@@ -148,7 +148,8 @@ returns the information to the application. The ioctl never fails.
       - ``CEC_CAP_HAS_EDID_PRESENT``
       - 0x00000400
       - If this capability is set, then
-        :ref:`CEC_LOG_ADDRS_FL_EDID_PRESENT <cec-log-addrs-flags>` is valid.
+        :ref:`CEC_LOG_ADDRS_FL_EDID_PRESENT <cec-log-addrs-flags>` and
+        :ref:`CEC_EVENT_FL_STATE_EDID_PRESENT <cec-event-flags>` are valid.
 	This makes it possible to tell apart the case where the Physical Address
 	is f.f.f.f because no EDID is present, and the case where it is f.f.f.f
 	because the EDID does not provide a valid Physical Address.
diff --git a/Documentation/userspace-api/media/cec/cec-ioc-dqevent.rst b/Documentation/userspace-api/media/cec/cec-ioc-dqevent.rst
index 71d6a9e81f75..188600dff3c7 100644
--- a/Documentation/userspace-api/media/cec/cec-ioc-dqevent.rst
+++ b/Documentation/userspace-api/media/cec/cec-ioc-dqevent.rst
@@ -224,6 +224,16 @@ it is guaranteed that the state did change in between the two events.
       - 2
       - Set if one or more events of the given event type have been dropped.
         This is an indication that the application cannot keep up.
+    * .. _`CEC-EVENT-FL-STATE-EDID-PRESENT`:
+
+      - ``CEC_EVENT_FL_STATE_EDID_PRESENT``
+      - 4
+      - Only set for the ``CEC_EVENT_STATE_CHANGE`` event type.
+        If this flag is set, then an EDID is present. This helps tell apart the
+        case where the Physical Address is f.f.f.f because no EDID is present,
+        and the case where it is f.f.f.f because the EDID does not provide a
+        valid Physical Address. This flag can only be used if
+        ``CEC_CAP_HAS_EDID_PRESENT`` is set in the adapter capabilities.
 
 
 Return Value
-- 
2.53.0

[PATCH 12/13] media: cec: core: implement CEC_EVENT_FL_STATE_EDID_PRESENT

[Hans Verkuil]

Set CEC_EVENT_FL_STATE_EDID_PRESENT if needed when posting the state
change event.

Signed-off-by: Hans Verkuil <hverkuil+cisco@kernel.org>
---
 drivers/media/cec/core/cec-adap.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/media/cec/core/cec-adap.c b/drivers/media/cec/core/cec-adap.c
index 06a596a0ede3..c0de5835fea0 100644
--- a/drivers/media/cec/core/cec-adap.c
+++ b/drivers/media/cec/core/cec-adap.c
@@ -323,6 +323,8 @@ static void cec_post_state_event(struct cec_adapter *adap)
 	ev.state_change.log_addr_mask = adap->log_addrs.log_addr_mask;
 	ev.state_change.have_conn_info =
 		adap->conn_info.type != CEC_CONNECTOR_TYPE_NO_CONNECTOR;
+	if (adap->log_addrs.flags & CEC_LOG_ADDRS_FL_EDID_PRESENT)
+		ev.flags |= CEC_EVENT_FL_STATE_EDID_PRESENT;
 	cec_queue_event(adap, &ev);
 }
 
-- 
2.53.0

[PATCH 13/13] Documentation: media: document cec_phys_addr_invalidate

[Hans Verkuil]

This function was not document, add it. Also clarify
cec_adap_s_phys_addr, explaining that the PA comes from the EDID.

Signed-off-by: Hans Verkuil <hverkuil+cisco@kernel.org>
---
 Documentation/driver-api/media/cec-core.rst | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/Documentation/driver-api/media/cec-core.rst b/Documentation/driver-api/media/cec-core.rst
index f1ffdec388f3..10927833992c 100644
--- a/Documentation/driver-api/media/cec-core.rst
+++ b/Documentation/driver-api/media/cec-core.rst
@@ -437,7 +437,8 @@ transmitted, otherwise just queue it and return.
 .. c:function::
    void cec_s_phys_addr(struct cec_adapter *adap, u16 phys_addr, bool block);
 
-Change the physical address. This function will set adap->phys_addr and
+Change the physical address. Use cec_s_phys_addr() to set the Physical Address
+as read from an EDID. This function will set adap->phys_addr and
 send an event if it has changed. If cec_s_log_addrs() has been called and
 the physical address has become valid, then the CEC framework will start
 claiming the logical addresses. If block is true, then this function won't
@@ -449,6 +450,13 @@ then the CEC adapter will be disabled. If you change a valid physical address
 to another valid physical address, then this function will first set the
 address to CEC_PHYS_ADDR_INVALID before enabling the new physical address.
 
+.. c:function::
+   void cec_phys_addr_invalidate(struct cec_adapter *adap);
+
+Invalidates the Physical Address when the EDID was lost, e.g. if the HDMI
+cable was unplugged. Do not use this to report an invalid Physical Address
+that you read from an EDID, use cec_s_phys_addr() for that.
+
 .. c:function::
    void cec_s_phys_addr_from_edid(struct cec_adapter *adap, \
 				  const struct edid *edid);
-- 
2.53.0

Re: [PATCH 01/13] gpu: drm: sti: call cec_notifier_phys_addr_invalidate if no EDID

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] drm/sti: incomplete cec_notifier_phys_addr_invalidate() conversion
--

commit 349b68331fa85963a4002deda235cb948b105b27
Author: Hans Verkuil <hverkuil+cisco@kernel.org>

gpu: drm: sti: call cec_notifier_phys_addr_invalidate if no EDID

This commit replaces a cec_notifier_set_phys_addr() call with
cec_notifier_phys_addr_invalidate() in sti_hdmi_disable().

> diff --git a/drivers/gpu/drm/sti/sti_hdmi.c b/drivers/gpu/drm/sti/sti_hdmi.c
> index 7ba9e0b7ba6c0..d392140df2357 100644
> --- a/drivers/gpu/drm/sti/sti_hdmi.c
> +++ b/drivers/gpu/drm/sti/sti_hdmi.c
> @@ -790,7 +790,7 @@ static void sti_hdmi_disable(struct drm_bridge *bridge,
>  
>  	hdmi->enabled = false;
>  
> -	cec_notifier_set_phys_addr(hdmi->notifier, CEC_PHYS_ADDR_INVALID);
> +	cec_notifier_phys_addr_invalidate(hdmi->notifier);
>  }

[Severity: Medium]
The commit message states:

  If an EDID is present, then call cec_notifier_set_phys_addr. If not, then
  call cec_notifier_phys_addr_invalidate. This helps the CEC framework
  differentiate between CEC_PHYS_ADDR_INVALID due to no EDID, or due to an
  EDID without a Physical Address.

  This driver is one of the few that was using cec_notifier_set_phys_addr
  on disconnect instead of cec_notifier_phys_addr_invalidate.

However, this patch only updates sti_hdmi_disable().

In drivers/gpu/drm/sti/sti_hdmi.c, sti_hdmi_connector_detect() still calls
cec_notifier_set_phys_addr() on cable disconnect when hdmi->hpd is false:

	DRM_DEBUG_DRIVER("hdmi cable disconnected\n");
	cec_notifier_set_phys_addr(hdmi->notifier, CEC_PHYS_ADDR_INVALID);
	return connector_status_disconnected;

Additionally, sti_hdmi_connector_get_modes() unconditionally calls
cec_notifier_set_phys_addr() before checking whether drm_edid is NULL:

	drm_edid = drm_edid_read(connector);

	drm_edid_connector_update(connector, drm_edid);

	cec_notifier_set_phys_addr(hdmi->notifier,
				   connector->display_info.source_physical_address);

	if (!drm_edid)
		goto fail;

When drm_edid is NULL, drm_edid_connector_update() sets
source_physical_address to CEC_PHYS_ADDR_INVALID, so
cec_notifier_set_phys_addr(hdmi->notifier, CEC_PHYS_ADDR_INVALID) is still
called when no EDID is present.

Should sti_hdmi_connector_detect() and sti_hdmi_connector_get_modes() also be
updated to call cec_notifier_phys_addr_invalidate() when the cable is
disconnected or when drm_edid is NULL?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/cover.1793615824.git.hverkuil+cisco@kernel.org?part=1

Re: [PATCH 02/13] gpu: drm: exynos: call cec_notifier_phys_addr_invalidate if no EDID

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] drm/exynos: hdmi_get_modes() calls cec_notifier_set_phys_addr() when no EDID
--

commit 66acd3ff42feaf8689b6b9eff4b13a5139f4e6bb
Author: Hans Verkuil <hverkuil+cisco@kernel.org>

gpu: drm: exynos: call cec_notifier_phys_addr_invalidate if no EDID

This commit updates hdmi_detect() in the Exynos HDMI driver to call
cec_notifier_phys_addr_invalidate() instead of cec_notifier_set_phys_addr()
with CEC_PHYS_ADDR_INVALID when the connector is disconnected.

> diff --git a/drivers/gpu/drm/exynos/exynos_hdmi.c b/drivers/gpu/drm/exynos/exynos_hdmi.c
> index 055dc04f9b3e5..d03f4e531e05e 100644
> --- a/drivers/gpu/drm/exynos/exynos_hdmi.c
> +++ b/drivers/gpu/drm/exynos/exynos_hdmi.c
> @@ -857,7 +857,7 @@ static enum drm_connector_status hdmi_detect(struct drm_connector *connector,
>  	if (gpiod_get_value(hdata->hpd_gpio))
>  		return connector_status_connected;
>  
> -	cec_notifier_set_phys_addr(hdata->notifier, CEC_PHYS_ADDR_INVALID);
> +	cec_notifier_phys_addr_invalidate(hdata->notifier);

[Severity: Medium]
The commit message notes that if an EDID is present, the driver should call
cec_notifier_set_phys_addr(), and if not, it should call
cec_notifier_phys_addr_invalidate().

However, this patch only updates the HPD disconnect path in hdmi_detect().
When HPD is asserted (or probing is forced) and
drm_helper_probe_single_connector_modes() calls hdmi_get_modes(), if reading
the EDID via drm_edid_read_ddc() returns NULL, drm_edid_connector_update()
sets info->source_physical_address to CEC_PHYS_ADDR_INVALID and
hdmi_get_modes() still calls cec_notifier_set_phys_addr() before checking
drm_edid:

drivers/gpu/drm/exynos/exynos_hdmi.c:hdmi_get_modes() {
	...
	drm_edid = drm_edid_read_ddc(connector, hdata->ddc_adpt);

	ret = drm_edid_connector_update(connector, drm_edid);
	if (ret)
		return 0;

	cec_notifier_set_phys_addr(hdata->notifier, info->source_physical_address);

	if (!drm_edid)
		goto no_edid;
	...
no_edid:
	return drm_add_modes_noedid(connector, 640, 480);
}

Should hdmi_get_modes() also be updated so that cec_notifier_set_phys_addr()
is only called when drm_edid is non-NULL, and
cec_notifier_phys_addr_invalidate() is called in the no_edid path when no
EDID could be read?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/cover.1793615824.git.hverkuil+cisco@kernel.org?part=2

Re: [PATCH 06/13] media: cec: core: improve cec_notifier_phys_addr_invalidate

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] media: cec: cec_notifier loses no-EDID state and calls cec_s_phys_addr() on adap register
--

commit bae9e1de47a9aeb702b139c748d1a200e761edc7
Author: Hans Verkuil <hverkuil+cisco@kernel.org>

media: cec: core: improve cec_notifier_phys_addr_invalidate

This commit moves cec_notifier_phys_addr_invalidate() into cec-notifier.c
and changes it to call cec_phys_addr_invalidate() instead of
cec_notifier_set_phys_addr() so the CEC framework knows when no EDID is
present.

> diff --git a/drivers/media/cec/core/cec-notifier.c b/drivers/media/cec/core/cec-notifier.c
> index 8cf1474d0361c..2814ef7d48045 100644
> --- a/drivers/media/cec/core/cec-notifier.c
> +++ b/drivers/media/cec/core/cec-notifier.c
> @@ -216,6 +216,19 @@ void cec_notifier_set_phys_addr_from_edid(struct cec_notifier *n,
>  }
>  EXPORT_SYMBOL_GPL(cec_notifier_set_phys_addr_from_edid);
>  
> +void cec_notifier_phys_addr_invalidate(struct cec_notifier *n)
> +{
> +	if (!n)
> +		return;
> +
> +	mutex_lock(&n->lock);
> +	n->phys_addr = CEC_PHYS_ADDR_INVALID;
> +	if (n->cec_adap && !n->cec_adap->adap_controls_phys_addr)
> +		cec_phys_addr_invalidate(n->cec_adap);
> +	mutex_unlock(&n->lock);
> +}
> +EXPORT_SYMBOL_GPL(cec_notifier_phys_addr_invalidate);

[Severity: Medium]
What happens if cec_notifier_conn_register() or
cec_notifier_phys_addr_invalidate() is called before the CEC adapter
registers via cec_notifier_cec_adap_register()?

When n->cec_adap is NULL, cec_notifier_phys_addr_invalidate() only sets
n->phys_addr = CEC_PHYS_ADDR_INVALID, because struct cec_notifier does not
track whether an EDID is present. This leaves the notifier state
indistinguishable from cec_notifier_set_phys_addr(n, CEC_PHYS_ADDR_INVALID).

When the CEC adapter subsequently registers in
cec_notifier_cec_adap_register(), it unconditionally calls
cec_s_phys_addr() with n->phys_addr:

drivers/media/cec/core/cec-notifier.c:cec_notifier_cec_adap_register() {
	...
	mutex_lock(&n->lock);
	n->cec_adap = adap;
	adap->conn_info = n->conn_info;
	adap->notifier = n;
	if (!adap->adap_controls_phys_addr)
		cec_s_phys_addr(adap, n->phys_addr, false);
	mutex_unlock(&n->lock);
	...
}

Doesn't calling cec_s_phys_addr(adap, CEC_PHYS_ADDR_INVALID, false) instead
of cec_phys_addr_invalidate(adap) tell the CEC framework that an EDID is
present (with physical address f.f.f.f) even when no sink is connected at
registration time?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/cover.1793615824.git.hverkuil+cisco@kernel.org?part=6

Re: [PATCH 05/13] media: cec: extron-da-hd-4k-plus: call cec_s_phys_addr only if EDID is present

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] media: cec: extron-da-hd-4k-plus: lost PA update when gated only on update_pa
--

commit 83f5e1abc765867f0cdd4fc7af830331a61a4349
Author: Hans Verkuil <hverkuil+cisco@kernel.org>

media: cec: extron-da-hd-4k-plus: call cec_s_phys_addr only if EDID is present

This commit updates extron_irq_work_handler() to call cec_s_phys_addr() when
an EDID is present and cec_phys_addr_invalidate() when no EDID is present, so
the CEC framework can distinguish between a missing EDID and an EDID without a
physical address.

> diff --git a/drivers/media/cec/usb/extron-da-hd-4k-plus/extron-da-hd-4k-plus.c b/drivers/media/cec/usb/extron-da-hd-4k-plus/extron-da-hd-4k-plus.c
> index 1f8ed7ff31e52..0a0724fd89c0c 100644
> --- a/drivers/media/cec/usb/extron-da-hd-4k-plus/extron-da-hd-4k-plus.c
> +++ b/drivers/media/cec/usb/extron-da-hd-4k-plus/extron-da-hd-4k-plus.c
> @@ -643,8 +643,12 @@ static void extron_irq_work_handler(struct work_struct *work)
>  			extron_read_edid(port);
>  		}
>  	}
> -	if (update_pa)
> -		cec_s_phys_addr(port->adap, pa, false);
> +	if (update_pa) {
> +		if (has_edid)
> +			cec_s_phys_addr(port->adap, pa, false);
> +		else
> +			cec_phys_addr_invalidate(port->adap);
> +	}

[Severity: High]
Can gating this block solely on update_pa cause physical address updates or
EDID-presence transitions to be lost when Pceco and Hdcp notifications are
processed in separate extron_irq_work_handler() runs?

port->phys_addr and port->has_edid are updated by independent asynchronous
messages via extron_phys_addr_change() and extron_process_edid_change() ->
extron_port_edid_change(). In extron_phys_addr_change(), update_phys_addr is
only set when port->phys_addr changes:

	if (!port->update_phys_addr && port->phys_addr != pa) {
		update = true;
		port->update_phys_addr = true;
	}
	port->phys_addr = pa;

During extron_setup(), CEC on output ports is enabled before extron->is_ready
is set and before WHDCP queries initial EDID presence:

	err = extron_send_and_wait(extron, NULL, "WO20*CCEC", "CcecO20");
	if (err)
		return err;
	...
	extron->is_ready = true;

	/* Query HDCP and Signal states, used to update the initial state */
	err = extron_send_and_wait(extron, NULL, "WHDCP", "Hdcp");

If Pceco arrives for a connected output sink while port->has_edid is still
false (either during extron_setup() before WHDCP or on hotplug before Hdcp),
the first run of extron_irq_work_handler() sees update_pa == true and
has_edid == false, clears port->update_phys_addr, and calls
cec_phys_addr_invalidate(port->adap). When WHDCP or Hdcp later sets
port->has_edid = true and port->update_has_edid = true, the next run of
extron_irq_work_handler() sees update_pa == false (and port->phys_addr already
equals pa), leaving port->adap stuck at CEC_PHYS_ADDR_INVALID.

Similarly, on hot-unplug, if Pceco with 0xffff is processed before Hdcp clears
port->has_edid, the first work run sees update_pa == true and has_edid == true
and calls cec_s_phys_addr(port->adap, CEC_PHYS_ADDR_INVALID, false), while the
subsequent work run for update_has_edid == true sees update_pa == false and
never calls cec_phys_addr_invalidate(port->adap). Likewise, connecting or
disconnecting a DVI sink whose EDID has no physical address keeps pa at
CEC_PHYS_ADDR_INVALID, so port->phys_addr != pa is false and update_pa is
never set when update_has_edid triggers.

Should this check also run when update_has_edid is true, for example:

	if (update_pa || update_has_edid) {
		if (has_edid)
			cec_s_phys_addr(port->adap, pa, false);
		else
			cec_phys_addr_invalidate(port->adap);
	}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/cover.1793615824.git.hverkuil+cisco@kernel.org?part=5

Re: [PATCH 09/13] media: cec: core: add support for EDID_PRESENT

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] media: cec: spurious state events in __cec_s_phys_addr() without cap
- [Medium] media: cec: cec_s_phys_addr() falsely sets EDID_PRESENT without EDID
--

commit 1fe777ffdbc4971735bf4636dac9ebebeba91665
Author: Hans Verkuil <hverkuil+cisco@kernel.org>

media: cec: core: add support for EDID_PRESENT

This commit adds support for reporting whether an EDID is present when the
CEC physical address is set or invalidated. It updates cec_s_phys_addr() to
set CEC_LOG_ADDRS_FL_EDID_PRESENT and changes cec_phys_addr_invalidate() to
clear it when the EDID is lost.

> diff --git a/drivers/media/cec/core/cec-adap.c b/drivers/media/cec/core/cec-adap.c
> index ee8a200b9803f..06a596a0ede3e 100644
> --- a/drivers/media/cec/core/cec-adap.c
> +++ b/drivers/media/cec/core/cec-adap.c
> @@ -1727,18 +1727,33 @@ int cec_adap_enable(struct cec_adapter *adap)
>   *
>   * This function is called with adap->lock held.
>   */
> -void __cec_s_phys_addr(struct cec_adapter *adap, u16 phys_addr, bool block)
> +void __cec_s_phys_addr(struct cec_adapter *adap, bool edid_present,
> +		       u16 phys_addr, bool block)
>  {
>  	bool becomes_invalid = phys_addr == CEC_PHYS_ADDR_INVALID;
>  	bool is_invalid = adap->phys_addr == CEC_PHYS_ADDR_INVALID;
> +	bool edid_was_present =
> +		adap->log_addrs.flags & CEC_LOG_ADDRS_FL_EDID_PRESENT;
>  
> -	if (phys_addr == adap->phys_addr)
> -		return;
> +	if (adap->capabilities & CEC_CAP_HAS_EDID_PRESENT) {
> +		if (edid_present)
> +			adap->log_addrs.flags |= CEC_LOG_ADDRS_FL_EDID_PRESENT;
> +		else
> +			adap->log_addrs.flags &= ~CEC_LOG_ADDRS_FL_EDID_PRESENT;
> +	}
>  	if (!becomes_invalid && adap->devnode.unregistered)
>  		return;
> +	if (phys_addr == adap->phys_addr) {
> +		if (edid_was_present != edid_present) {
> +			dprintk(1, "EDID is %spresent\n",
> +				edid_present ? "" : "not ");
> +			cec_post_state_event(adap);
> +		}
> +		return;
> +	}

[Severity: Medium]
Can this queue spurious CEC_EVENT_STATE_CHANGE events when an adapter does
not have CEC_CAP_HAS_EDID_PRESENT?

In cec_allocate_adapter(), adapters allocated with CEC_CAP_PHYS_ADDR do not
get CEC_CAP_HAS_EDID_PRESENT:

drivers/media/cec/core/cec-core.c:cec_allocate_adapter()
	if (!(caps & CEC_CAP_PHYS_ADDR))
		adap->capabilities |= CEC_CAP_HAS_EDID_PRESENT;

Because __cec_s_phys_addr() only sets CEC_LOG_ADDRS_FL_EDID_PRESENT in
adap->log_addrs.flags when CEC_CAP_HAS_EDID_PRESENT is set, edid_was_present
remains permanently false for those adapters.

However, cec_adap_s_phys_addr() (for the CEC_ADAP_S_PHYS_ADDR ioctl) and
cec_s_phys_addr() still pass edid_present = true when setting a valid
physical address. Whenever the same valid physical address is set again on
such an adapter (phys_addr == adap->phys_addr), edid_was_present !=
edid_present evaluates to true and cec_post_state_event(adap) is called even
though neither adap->phys_addr nor adap->log_addrs.flags changed.

Should edid_present be forced to false when !(adap->capabilities &
CEC_CAP_HAS_EDID_PRESENT), or should the edid_was_present != edid_present
check be gated on CEC_CAP_HAS_EDID_PRESENT?

>  
> -	dprintk(1, "new physical address %x.%x.%x.%x\n",
> -		cec_phys_addr_exp(phys_addr));
> +	dprintk(1, "new physical address %x.%x.%x.%x, EDID is %spresent\n",
> +		cec_phys_addr_exp(phys_addr), edid_present ? "" : "not ");
>  	if (becomes_invalid || !is_invalid) {
>  		adap->phys_addr = CEC_PHYS_ADDR_INVALID;
>  		cec_adap_unconfigure(adap);
> @@ -1767,11 +1782,22 @@ void cec_s_phys_addr(struct cec_adapter *adap, u16 phys_addr, bool block)
>  		return;
>  
>  	mutex_lock(&adap->lock);
> -	__cec_s_phys_addr(adap, phys_addr, block);
> +	__cec_s_phys_addr(adap, true, phys_addr, block);
>  	mutex_unlock(&adap->lock);
>  }
>  EXPORT_SYMBOL_GPL(cec_s_phys_addr);

[Severity: Medium]
Won't passing true unconditionally here cause existing callers that pass
CEC_PHYS_ADDR_INVALID when no EDID is present to falsely report
CEC_LOG_ADDRS_FL_EDID_PRESENT?

For example, in drivers/media/cec/core/cec-notifier.c, struct cec_notifier
only tracks n->phys_addr (which starts as CEC_PHYS_ADDR_INVALID when no
sink is connected) and does not track whether an EDID is present. When a
CEC adapter registers with a notifier while disconnected,
cec_notifier_cec_adap_register() calls cec_s_phys_addr() with
CEC_PHYS_ADDR_INVALID:

drivers/media/cec/core/cec-notifier.c:cec_notifier_cec_adap_register()
	if (!adap->adap_controls_phys_addr)
		cec_s_phys_addr(adap, n->phys_addr, false);

Similarly, cec_notifier_set_phys_addr_from_edid() calls
cec_notifier_set_phys_addr(n, CEC_PHYS_ADDR_INVALID) -> cec_s_phys_addr()
even when edid is NULL:

drivers/media/cec/core/cec-notifier.c:cec_notifier_set_phys_addr_from_edid()
	if (edid && edid->extensions)
		pa = cec_get_edid_phys_addr((const u8 *)edid,
				EDID_LENGTH * (edid->extensions + 1), NULL);
	cec_notifier_set_phys_addr(n, pa);

And drm_dp_cec_attach() in drivers/gpu/drm/display/drm_dp_cec.c also calls
cec_s_phys_addr(aux->cec.adap, source_physical_address, false) when
called from drm_dp_cec_register_connector() with no EDID and
source_physical_address set to CEC_PHYS_ADDR_INVALID.

In all of those paths, __cec_s_phys_addr() will now set
CEC_LOG_ADDRS_FL_EDID_PRESENT and post a state change event even though no
EDID is present. Do cec_notifier and drm_dp_cec need to be updated to track
or invalidate EDID presence accordingly?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/cover.1793615824.git.hverkuil+cisco@kernel.org?part=9

Re: [PATCH 12/13] media: cec: core: implement CEC_EVENT_FL_STATE_EDID_PRESENT

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] media: cec: missing CEC_EVENT_FL_STATE_EDID_PRESENT in cec_open()
--

commit 46a7f75b89b8be5b867c7e7aadb50668719e461a
Author: Hans Verkuil <hverkuil+cisco@kernel.org>

media: cec: core: implement CEC_EVENT_FL_STATE_EDID_PRESENT

This commit sets CEC_EVENT_FL_STATE_EDID_PRESENT on CEC_EVENT_STATE_CHANGE
events in cec_post_state_event() when CEC_LOG_ADDRS_FL_EDID_PRESENT is set
in adap->log_addrs.flags.

> diff --git a/drivers/media/cec/core/cec-adap.c b/drivers/media/cec/core/cec-adap.c
> index 06a596a0ede3e..c0de5835fea01 100644
> --- a/drivers/media/cec/core/cec-adap.c
> +++ b/drivers/media/cec/core/cec-adap.c
> @@ -323,6 +323,8 @@ static void cec_post_state_event(struct cec_adapter *adap)
>  	ev.state_change.log_addr_mask = adap->log_addrs.log_addr_mask;
>  	ev.state_change.have_conn_info =
>  		adap->conn_info.type != CEC_CONNECTOR_TYPE_NO_CONNECTOR;
> +	if (adap->log_addrs.flags & CEC_LOG_ADDRS_FL_EDID_PRESENT)
> +		ev.flags |= CEC_EVENT_FL_STATE_EDID_PRESENT;
>  	cec_queue_event(adap, &ev);
>  }

[Severity: Medium]
Should the initial CEC_EVENT_STATE_CHANGE event queued in cec_open() also
set CEC_EVENT_FL_STATE_EDID_PRESENT when an EDID is already present?

In drivers/media/cec/core/cec-api.c, cec_open() constructs the initial state
event directly:

drivers/media/cec/core/cec-api.c:cec_open() {
	struct cec_event ev = {
		.event = CEC_EVENT_STATE_CHANGE,
		.flags = CEC_EVENT_FL_INITIAL_STATE,
	};
	...
	/* Queue up initial state events */
	ev.state_change.phys_addr = adap->phys_addr;
	ev.state_change.log_addr_mask = adap->log_addrs.log_addr_mask;
	ev.state_change.have_conn_info =
		adap->conn_info.type != CEC_CONNECTOR_TYPE_NO_CONNECTOR;
	cec_queue_event_fh(fh, &ev, 0);
}

When userspace opens a CEC device node while adap->log_addrs.flags has
CEC_LOG_ADDRS_FL_EDID_PRESENT set and dequeues the initial state event via
CEC_DQEVENT, ev.flags will be missing CEC_EVENT_FL_STATE_EDID_PRESENT.

Also note that under CONFIG_CEC_PIN, cec_open() reuses the same ev variable
to queue initial CEC_EVENT_PIN_HPD_HIGH/LOW and CEC_EVENT_PIN_5V_HIGH/LOW
events, so if CEC_EVENT_FL_STATE_EDID_PRESENT is set on ev.flags for the
initial state change event, it would need to be cleared before queueing the
pin events.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/cover.1793615824.git.hverkuil+cisco@kernel.org?part=12

Re: [PATCH 03/13] gpu: drm: omapdrm: call cec_s_phys_addr only if EDID is present

[Tomi Valkeinen]

Hi,

On 02/11/2026 12:36, Hans Verkuil wrote:
> If an EDID is present, then call cec_s_phys_addr. If not, then
> call cec_phys_addr_invalidate. This helps the CEC framework differentiate
> between CEC_PHYS_ADDR_INVALID due to no EDID, or due to an EDID without
> a Physical Address.
> 
> This driver is one of the few that was doing this inconsistently.
> 
> Signed-off-by: Hans Verkuil <hverkuil+cisco@kernel.org>
> Cc: Tomi Valkeinen <tomi.valkeinen@ideasonboard.com>
> Cc: Maxime Ripard <mripard@kernel.org>
> ---
>   drivers/gpu/drm/omapdrm/dss/hdmi4.c     | 7 +++----
>   drivers/gpu/drm/omapdrm/dss/hdmi4_cec.c | 5 +++++
>   drivers/gpu/drm/omapdrm/dss/hdmi4_cec.h | 5 +++++
>   3 files changed, 13 insertions(+), 4 deletions(-)
> 

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

  Tomi