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

[PATCHv2 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

Changes in v2 (based on sashiko reviews):

- missed one incorrect cec_notifier_set_phys_addr call in sti_hdmi.c
- cec_notifier_cec_adap_register didn't call cec_phys_addr_invalidate
  if an earlier cec_notifier_phys_addr_invalidate call indicated that
  there is no EDID.
- sending the initial state change event didn't set the
  CEC_EVENT_FL_STATE_EDID_PRESENT flag if needed.

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                |  4 +-
 drivers/media/cec/core/cec-adap.c             | 51 ++++++++++++++++---
 drivers/media/cec/core/cec-api.c              | 13 +++--
 drivers/media/cec/core/cec-core.c             |  4 +-
 drivers/media/cec/core/cec-notifier.c         | 26 +++++++++-
 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, 191 insertions(+), 67 deletions(-)

-- 
2.53.0

[PATCHv2 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 | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/sti/sti_hdmi.c b/drivers/gpu/drm/sti/sti_hdmi.c
index 7ba9e0b7ba6c..12e05505bbde 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);
 }
 
 /*
@@ -1068,7 +1068,7 @@ sti_hdmi_connector_detect(struct drm_connector *connector, bool force)
 	}
 
 	DRM_DEBUG_DRIVER("hdmi cable disconnected\n");
-	cec_notifier_set_phys_addr(hdmi->notifier, CEC_PHYS_ADDR_INVALID);
+	cec_notifier_phys_addr_invalidate(hdmi->notifier);
 	return connector_status_disconnected;
 }
 
-- 
2.53.0

Re: [PATCHv2 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: sti_hdmi_connector_get_modes() sets CEC phys addr when !drm_edid
--

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

gpu: drm: sti: call cec_notifier_phys_addr_invalidate if no EDID

This commit replaces calls to cec_notifier_set_phys_addr() passing
CEC_PHYS_ADDR_INVALID with cec_notifier_phys_addr_invalidate() in the STI
HDMI driver so the CEC framework can distinguish between missing EDIDs and
EDIDs without a physical address.

> diff --git a/drivers/gpu/drm/sti/sti_hdmi.c b/drivers/gpu/drm/sti/sti_hdmi.c
> index 7ba9e0b7ba6c0..12e05505bbdef 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);
>  }
>  
>  /*
> @@ -1068,7 +1068,7 @@ sti_hdmi_connector_detect(struct drm_connector *connector, bool force)
>  	}
>  
>  	DRM_DEBUG_DRIVER("hdmi cable disconnected\n");
> -	cec_notifier_set_phys_addr(hdmi->notifier, CEC_PHYS_ADDR_INVALID);
> +	cec_notifier_phys_addr_invalidate(hdmi->notifier);
>  	return connector_status_disconnected;
>  }

[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."

However, in drivers/gpu/drm/sti/sti_hdmi.c, sti_hdmi_connector_get_modes()
still calls cec_notifier_set_phys_addr() unconditionally before checking
whether drm_edid is NULL:

drivers/gpu/drm/sti/sti_hdmi.c:sti_hdmi_connector_get_modes() {
	...
	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_read() returns NULL, drm_edid_connector_update(connector, NULL)
resets connector->display_info.source_physical_address to
CEC_PHYS_ADDR_INVALID, and sti_hdmi_connector_get_modes() then passes
CEC_PHYS_ADDR_INVALID to cec_notifier_set_phys_addr() (a pattern introduced
in commit f7945d9fa8b7 ("drm/sti/sti_hdmi: convert to struct drm_edid") to
invalidate the physical address on EDID read failure).

Should sti_hdmi_connector_get_modes() also check !drm_edid first and call
cec_notifier_phys_addr_invalidate(hdmi->notifier) on the failure path before
calling cec_notifier_set_phys_addr()?

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

[PATCHv2 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

Re: [PATCHv2 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() with no EDID
--

commit f0dacf81d55b2fd1b8fee4812c0ecf88da4b0a39
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 exynos_hdmi 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 cec_notifier_set_phys_addr() should be called if
an EDID is present and cec_notifier_phys_addr_invalidate() if not, so the CEC
framework can distinguish between CEC_PHYS_ADDR_INVALID due to a missing EDID
versus an EDID without a physical address.

While this updates the disconnect path in hdmi_detect(), hdmi_get_modes() in
drivers/gpu/drm/exynos/exynos_hdmi.c still calls cec_notifier_set_phys_addr()
before checking whether drm_edid is NULL:

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

When drm_edid_read_ddc() returns NULL during mode probing via
drm_helper_probe_single_connector_modes() -> hdmi_get_modes(),
drm_edid_connector_update() resets info->source_physical_address to
CEC_PHYS_ADDR_INVALID, and hdmi_get_modes() passes that to
cec_notifier_set_phys_addr() even though no EDID was read.

Should hdmi_get_modes() also check for a NULL drm_edid first and call
cec_notifier_phys_addr_invalidate(hdata->notifier) on the no_edid path instead
of cec_notifier_set_phys_addr()?

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

[PATCHv2 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

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

[Tomi Valkeinen]

Hi,

On 03/11/2026 09:31, 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

> 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)
>   {
>   }

[PATCHv2 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

[PATCHv2 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..ae3628767ac5 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 || update_has_edid) {
+		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

[PATCHv2 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.

Also update cec_notifier_cec_adap_register(): it has to remember if
the last set physical address was from an EDID or because there was
a disconnect.

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

diff --git a/drivers/media/cec/core/cec-notifier.c b/drivers/media/cec/core/cec-notifier.c
index 8cf1474d0361..5a4588487ea3 100644
--- a/drivers/media/cec/core/cec-notifier.c
+++ b/drivers/media/cec/core/cec-notifier.c
@@ -29,6 +29,7 @@ struct cec_notifier {
 	struct cec_adapter *cec_adap;
 
 	u16 phys_addr;
+	bool edid_present;
 };
 
 static LIST_HEAD(cec_notifiers);
@@ -113,6 +114,7 @@ cec_notifier_conn_register(struct device *hdmi_dev, const char *port_name,
 
 	mutex_lock(&n->lock);
 	n->phys_addr = CEC_PHYS_ADDR_INVALID;
+	n->edid_present = false;
 	if (conn_info)
 		n->conn_info = *conn_info;
 	else
@@ -135,6 +137,7 @@ void cec_notifier_conn_unregister(struct cec_notifier *n)
 	mutex_lock(&n->lock);
 	memset(&n->conn_info, 0, sizeof(n->conn_info));
 	n->phys_addr = CEC_PHYS_ADDR_INVALID;
+	n->edid_present = false;
 	if (n->cec_adap) {
 		if (!n->cec_adap->adap_controls_phys_addr)
 			cec_phys_addr_invalidate(n->cec_adap);
@@ -162,8 +165,12 @@ cec_notifier_cec_adap_register(struct device *hdmi_dev, const char *port_name,
 	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);
+	if (!adap->adap_controls_phys_addr) {
+		if (n->phys_addr == CEC_PHYS_ADDR_INVALID && !n->edid_present)
+			cec_phys_addr_invalidate(adap);
+		else
+			cec_s_phys_addr(adap, n->phys_addr, false);
+	}
 	mutex_unlock(&n->lock);
 	return n;
 }
@@ -190,6 +197,7 @@ void cec_notifier_set_phys_addr(struct cec_notifier *n, u16 pa)
 
 	mutex_lock(&n->lock);
 	n->phys_addr = pa;
+	n->edid_present = true;
 	if (n->cec_adap && !n->cec_adap->adap_controls_phys_addr)
 		cec_s_phys_addr(n->cec_adap, n->phys_addr, false);
 	mutex_unlock(&n->lock);
@@ -216,6 +224,20 @@ 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;
+	n->edid_present = false;
+	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

[PATCHv2 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

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

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] media: uapi: cec.h: unimplemented EDID capability and flag stripped in core
- [Low] media: uapi: cec.h: missing docs cause broken Sphinx xrefs for EDID macros
--

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

media: uapi: cec.h: add CEC_CAP_HAS_EDID_PRESENT/CEC_LOG_ADDRS_FL_EDID_PRESENT

This commit adds the CEC_CAP_HAS_EDID_PRESENT capability and the
CEC_LOG_ADDRS_FL_EDID_PRESENT logical address flag to the CEC UAPI header
to indicate if an EDID is present.

> diff --git a/include/uapi/linux/cec.h b/include/uapi/linux/cec.h
> index fdfc97a6e4ec0..8f7c7a4cb505e 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)

[Severity: Low]
Does adding CEC_CAP_HAS_EDID_PRESENT and CEC_LOG_ADDRS_FL_EDID_PRESENT
without corresponding documentation entries introduce broken Sphinx
cross-reference warnings during make htmldocs?

Documentation/userspace-api/media/cec/cec-header.rst includes
include/uapi/linux/cec.h using the kernel-include directive:

.. kernel-include:: include/uapi/linux/cec.h
    :generate-cross-refs:
    :exception-file: cec.h.rst.exceptions
    :toc:
    :warn-broken:

ParseDataStructs.parse_file() in
tools/lib/python/kdoc/parse_data_structs.py generates :ref:
cross-references for every #define macro in include/uapi/linux/cec.h
unless the symbol is listed in
Documentation/userspace-api/media/cec/cec.h.rst.exceptions.

Because CEC_CAP_HAS_EDID_PRESENT and CEC_LOG_ADDRS_FL_EDID_PRESENT are not
documented in Documentation/userspace-api/media/cec/cec-ioc-adap-g-caps.rst
and Documentation/userspace-api/media/cec/cec-ioc-adap-g-log-addrs.rst,
check_missing_refs() in Documentation/sphinx/kernel_include.py reports
unresolved references for std:ref:cec-cap-has-edid-present and
std:ref:cec-log-addrs-fl-edid-present.

>  /**
>   * 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)

[Severity: Medium]
How are CEC_CAP_HAS_EDID_PRESENT and CEC_LOG_ADDRS_FL_EDID_PRESENT
intended to be used when neither is referenced in the kernel and
cec_adap_s_log_addrs() strips the flag?

When userspace calls ioctl(fd, CEC_ADAP_S_LOG_ADDRS, &log_addrs),
drivers/media/cec/core/cec-api.c:cec_adap_s_log_addrs() masks out
CEC_LOG_ADDRS_FL_EDID_PRESENT before calling __cec_s_log_addrs():

	log_addrs.flags &= CEC_LOG_ADDRS_FL_ALLOW_UNREG_FALLBACK |
			   CEC_LOG_ADDRS_FL_ALLOW_RC_PASSTHRU |
			   CEC_LOG_ADDRS_FL_CDC_ONLY;

In addition, __cec_s_log_addrs() in drivers/media/cec/core/cec-adap.c
overwrites adap->log_addrs = *log_addrs without setting or preserving
CEC_LOG_ADDRS_FL_EDID_PRESENT, and no core or driver code sets
CEC_CAP_HAS_EDID_PRESENT in adap->capabilities.

Should the in-kernel implementation in drivers/media/cec/ accompany these
UAPI definitions so that the flag is not silently cleared?

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

[PATCHv2 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

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

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] media: cec: docs reference undefined EDID_PRESENT uAPI macros
--

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

Documentation: media: document EDID_PRESENT cap and flag.

This commit adds documentation for the CEC_CAP_HAS_EDID_PRESENT capability
and the CEC_LOG_ADDRS_FL_EDID_PRESENT flag to the CEC uAPI documentation.

> 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 1d5248979a6da..abd62ac64541b 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 8397bf5737988..2088793b1506c 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.

[Severity: Medium]
Are the corresponding uAPI definitions and kernel implementation for
CEC_CAP_HAS_EDID_PRESENT and CEC_LOG_ADDRS_FL_EDID_PRESENT missing from the
tree?

In include/uapi/linux/cec.h, the capability definitions currently stop at
CEC_CAP_REPLY_VENDOR_ID:

    /* CEC_ADAP_G_CONNECTOR_INFO is available */
    #define CEC_CAP_CONNECTOR_INFO	(1 << 8)
    /* CEC_MSG_FL_REPLY_VENDOR_ID is available */
    #define CEC_CAP_REPLY_VENDOR_ID	(1 << 9)

And the struct cec_log_addrs flag definitions stop at
CEC_LOG_ADDRS_FL_CONFIG_FAILED:

    /* CDC-Only device: supports only CDC messages */
    #define CEC_LOG_ADDRS_FL_CDC_ONLY		(1 << 2)
    /* Configuration failed */
    #define CEC_LOG_ADDRS_FL_CONFIG_FAILED		(1 << 3)

Neither symbol appears to be defined in include/uapi/linux/cec.h or handled
anywhere in drivers/media/cec/ (where cec-api.c still masks log_addrs.flags
to CEC_LOG_ADDRS_FL_ALLOW_UNREG_FALLBACK | CEC_LOG_ADDRS_FL_ALLOW_RC_PASSTHRU
| CEC_LOG_ADDRS_FL_CDC_ONLY). Because
Documentation/userspace-api/media/cec/cec-header.rst also parses
include/uapi/linux/cec.h to generate cross-references, the generated header
documentation is out of sync with these ioctl pages.

Also, in Documentation/userspace-api/media/cec/cec-ioc-adap-g-log-addrs.rst,
should the description clarify whether CEC_LOG_ADDRS_FL_EDID_PRESENT is a
read-only flag set by the driver or accepted by CEC_ADAP_S_LOG_ADDRS?

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

[PATCHv2 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 | 49 ++++++++++++++++++++++++++-----
 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, 63 insertions(+), 23 deletions(-)

diff --git a/drivers/media/cec/core/cec-adap.c b/drivers/media/cec/core/cec-adap.c
index ee8a200b9803..1a7b79916e47 100644
--- a/drivers/media/cec/core/cec-adap.c
+++ b/drivers/media/cec/core/cec-adap.c
@@ -1727,18 +1727,35 @@ 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;
+	} else {
+		edid_present = false;
+	}
 	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 +1784,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 +1810,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 +2002,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

Re: [PATCHv2 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:
- [High] media: cec: missing definitions for CEC_CAP_HAS_EDID_PRESENT and CEC_LOG_ADDRS_FL_EDID_PRESENT
- [Medium] media: cec: cec_notifier falsely sets CEC_LOG_ADDRS_FL_EDID_PRESENT via cec_s_phys_addr()
--

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

media: cec: core: add support for EDID_PRESENT

This commit adds tracking for whether an EDID is present when updating or
invalidating the CEC physical address, setting CEC_LOG_ADDRS_FL_EDID_PRESENT
in cec_s_phys_addr() and clearing it in cec_phys_addr_invalidate().

> diff --git a/drivers/media/cec/core/cec-adap.c b/drivers/media/cec/core/cec-adap.c
> index ee8a200b9803f..1a7b79916e47d 100644
> --- a/drivers/media/cec/core/cec-adap.c
> +++ b/drivers/media/cec/core/cec-adap.c
> @@ -1727,18 +1727,35 @@ 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;
> +	} else {
> +		edid_present = false;
> +	}

[Severity: High]
Were the header changes defining CEC_CAP_HAS_EDID_PRESENT and
CEC_LOG_ADDRS_FL_EDID_PRESENT in include/uapi/linux/cec.h accidentally omitted
from this commit?

In drivers/media/cec/core/cec-adap.c, __cec_s_phys_addr() and
__cec_s_log_addrs() reference CEC_LOG_ADDRS_FL_EDID_PRESENT and
CEC_CAP_HAS_EDID_PRESENT, and cec_allocate_adapter() in
drivers/media/cec/core/cec-core.c also uses CEC_CAP_HAS_EDID_PRESENT, causing
the build to fail with undeclared identifier errors when CONFIG_CEC_CORE is
enabled.

[ ... ]

> @@ -1767,11 +1784,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]
Does passing edid_present = true unconditionally in cec_s_phys_addr() break
EDID presence tracking for drivers using cec_notifier?

In include/media/cec-notifier.h, cec_notifier_phys_addr_invalidate() still
calls cec_notifier_set_phys_addr(n, CEC_PHYS_ADDR_INVALID):

include/media/cec-notifier.h:
static inline void cec_notifier_phys_addr_invalidate(struct cec_notifier *n)
{
	cec_notifier_set_phys_addr(n, CEC_PHYS_ADDR_INVALID);
}

And in drivers/media/cec/core/cec-notifier.c, cec_notifier_set_phys_addr() and
cec_notifier_cec_adap_register() both call cec_s_phys_addr():

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

cec_notifier_set_phys_addr()
	n->phys_addr = pa;
	if (n->cec_adap && !n->cec_adap->adap_controls_phys_addr)
		cec_s_phys_addr(n->cec_adap, n->phys_addr, false);

As a result, when an adapter is registered via
cec_notifier_cec_adap_register() before any display is connected, when
cec_notifier_set_phys_addr_from_edid(n, NULL) is called, or when HPD goes low
and cec_notifier_phys_addr_invalidate() is called, cec_s_phys_addr() sets
CEC_LOG_ADDRS_FL_EDID_PRESENT instead of clearing it.

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

[PATCHv2 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

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

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] media: cec: CEC_EVENT_FL_STATE_EDID_PRESENT is unimplemented and undocumented
--

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

media: uapi: cec.h: add CEC_EVENT_FL_STATE_EDID_PRESENT flag

This commit adds the CEC_EVENT_FL_STATE_EDID_PRESENT event flag to
include/uapi/linux/cec.h to report whether the EDID is present.

> diff --git a/include/uapi/linux/cec.h b/include/uapi/linux/cec.h
> index fdfc97a6e4ec0..051125a446933 100644
> --- a/include/uapi/linux/cec.h
> +++ b/include/uapi/linux/cec.h
> @@ -453,6 +453,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)

[Severity: Medium]
Is part of this change missing from the commit?

CEC_EVENT_FL_STATE_EDID_PRESENT is added to include/uapi/linux/cec.h,
but nothing in the kernel ever references or sets it when queuing
CEC_EVENT_STATE_CHANGE events. Specifically, neither
cec_post_state_event() in drivers/media/cec/core/cec-adap.c:

    static void cec_post_state_event(struct cec_adapter *adap)
    {
            struct cec_event ev = {
                    .event = CEC_EVENT_STATE_CHANGE,
            };

nor cec_open() in drivers/media/cec/core/cec-api.c:

    struct cec_event ev = {
            .event = CEC_EVENT_STATE_CHANGE,
            .flags = CEC_EVENT_FL_INITIAL_STATE,
    };

sets CEC_EVENT_FL_STATE_EDID_PRESENT, so userspace applications
dequeuing struct cec_event via ioctl CEC_DQEVENT after opening a CEC
device or receiving an adapter state change notification will always
observe the flag as 0 even when a valid EDID is present.

In addition, Documentation/userspace-api/media/cec/cec-header.rst
parses include/uapi/linux/cec.h with :generate-cross-refs:,
:exception-file: cec.h.rst.exceptions, and :warn-broken:. Because
CEC_EVENT_FL_STATE_EDID_PRESENT is neither documented in the
cec-event-flags table in
Documentation/userspace-api/media/cec/cec-ioc-dqevent.rst nor listed in
Documentation/userspace-api/media/cec/cec.h.rst.exceptions, does
running make htmldocs emit a broken cross-reference warning for
cec-event-fl-state-edid-present?

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

[PATCHv2 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

[PATCHv2 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 ++
 drivers/media/cec/core/cec-api.c  | 10 ++++++----
 2 files changed, 8 insertions(+), 4 deletions(-)

diff --git a/drivers/media/cec/core/cec-adap.c b/drivers/media/cec/core/cec-adap.c
index 1a7b79916e47..52a56cabedcf 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);
 }
 
diff --git a/drivers/media/cec/core/cec-api.c b/drivers/media/cec/core/cec-api.c
index 1ba080d5a123..77778ff737a6 100644
--- a/drivers/media/cec/core/cec-api.c
+++ b/drivers/media/cec/core/cec-api.c
@@ -560,10 +560,7 @@ static int cec_open(struct inode *inode, struct file *filp)
 	 * Initial events that are automatically sent when the cec device is
 	 * opened.
 	 */
-	struct cec_event ev = {
-		.event = CEC_EVENT_STATE_CHANGE,
-		.flags = CEC_EVENT_FL_INITIAL_STATE,
-	};
+	struct cec_event ev = {};
 	unsigned int i;
 	int err;
 
@@ -589,12 +586,17 @@ static int cec_open(struct inode *inode, struct file *filp)
 	filp->private_data = fh;
 
 	/* Queue up initial state events */
+	ev.event = CEC_EVENT_STATE_CHANGE;
 	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;
+	ev.flags = CEC_EVENT_FL_INITIAL_STATE;
+	if (adap->log_addrs.flags & CEC_LOG_ADDRS_FL_EDID_PRESENT)
+		ev.flags |= CEC_EVENT_FL_STATE_EDID_PRESENT;
 	cec_queue_event_fh(fh, &ev, 0);
 #ifdef CONFIG_CEC_PIN
+	ev.flags = CEC_EVENT_FL_INITIAL_STATE;
 	if (adap->pin && adap->pin->ops->read_hpd &&
 	    !adap->devnode.unregistered) {
 		err = adap->pin->ops->read_hpd(adap);
-- 
2.53.0

Re: [PATCHv2 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:
- [High] media: cec: undeclared and unset EDID_PRESENT flags break build
--

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

media: cec: core: implement CEC_EVENT_FL_STATE_EDID_PRESENT

This commit checks for CEC_LOG_ADDRS_FL_EDID_PRESENT in
adap->log_addrs.flags and sets CEC_EVENT_FL_STATE_EDID_PRESENT on
CEC_EVENT_STATE_CHANGE events in cec_post_state_event() and cec_open().

> diff --git a/drivers/media/cec/core/cec-adap.c b/drivers/media/cec/core/cec-adap.c
> index ee8a200b9803f..c02d036ee221e 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);
>  }
>  
> diff --git a/drivers/media/cec/core/cec-api.c b/drivers/media/cec/core/cec-api.c
> index a491d7fc8a71a..a2ceea5522d46 100644
> --- a/drivers/media/cec/core/cec-api.c
> +++ b/drivers/media/cec/core/cec-api.c
> @@ -559,10 +559,7 @@ static int cec_open(struct inode *inode, struct file *filp)
>  	 * Initial events that are automatically sent when the cec device is
>  	 * opened.
>  	 */
> -	struct cec_event ev = {
> -		.event = CEC_EVENT_STATE_CHANGE,
> -		.flags = CEC_EVENT_FL_INITIAL_STATE,
> -	};
> +	struct cec_event ev = {};
>  	unsigned int i;
>  	int err;
>  
> @@ -588,12 +585,17 @@ static int cec_open(struct inode *inode, struct file *filp)
>  	filp->private_data = fh;
>  
>  	/* Queue up initial state events */
> +	ev.event = CEC_EVENT_STATE_CHANGE;
>  	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;
> +	ev.flags = CEC_EVENT_FL_INITIAL_STATE;
> +	if (adap->log_addrs.flags & CEC_LOG_ADDRS_FL_EDID_PRESENT)
> +		ev.flags |= CEC_EVENT_FL_STATE_EDID_PRESENT;

[Severity: High]
Are the header definitions and flag updates for
CEC_LOG_ADDRS_FL_EDID_PRESENT and CEC_EVENT_FL_STATE_EDID_PRESENT missing
from this patch?

In both cec_post_state_event() in drivers/media/cec/core/cec-adap.c and
cec_open() in drivers/media/cec/core/cec-api.c, the build fails with
undeclared identifier errors because neither CEC_LOG_ADDRS_FL_EDID_PRESENT
nor CEC_EVENT_FL_STATE_EDID_PRESENT is defined in include/uapi/linux/cec.h:

#define CEC_EVENT_FL_INITIAL_STATE	(1 << 0)
#define CEC_EVENT_FL_DROPPED_EVENTS	(1 << 1)

In addition, nothing in the tree sets CEC_LOG_ADDRS_FL_EDID_PRESENT in
adap->log_addrs.flags, and cec_adap_s_log_addrs() in
drivers/media/cec/core/cec-api.c strips all other flags before
__cec_s_log_addrs() updates adap->log_addrs:

	log_addrs.flags &= CEC_LOG_ADDRS_FL_ALLOW_UNREG_FALLBACK |
			   CEC_LOG_ADDRS_FL_ALLOW_RC_PASSTHRU |
			   CEC_LOG_ADDRS_FL_CDC_ONLY;

Should include/uapi/linux/cec.h define these flags, and should the CEC core
set or preserve CEC_LOG_ADDRS_FL_EDID_PRESENT when EDID is present?

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

[PATCHv2 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: [PATCHv2 00/13] media/drm: cec: add EDID_PRESENT

[Hans Verkuil]

Hi all,

On 03/11/2026 08:31, Hans Verkuil wrote:
> 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.

I'll prep a v3 for this. The code is OK, but the flag names are wrong. This
is not about whether an EDID is present, but whether the Hotplug Detect is
present. I'll rename 'EDID' to 'HPD' for the v3.

Regards,

	Hans

> 
> 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
> 
> Changes in v2 (based on sashiko reviews):
> 
> - missed one incorrect cec_notifier_set_phys_addr call in sti_hdmi.c
> - cec_notifier_cec_adap_register didn't call cec_phys_addr_invalidate
>   if an earlier cec_notifier_phys_addr_invalidate call indicated that
>   there is no EDID.
> - sending the initial state change event didn't set the
>   CEC_EVENT_FL_STATE_EDID_PRESENT flag if needed.
> 
> 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                |  4 +-
>  drivers/media/cec/core/cec-adap.c             | 51 ++++++++++++++++---
>  drivers/media/cec/core/cec-api.c              | 13 +++--
>  drivers/media/cec/core/cec-core.c             |  4 +-
>  drivers/media/cec/core/cec-notifier.c         | 26 +++++++++-
>  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, 191 insertions(+), 67 deletions(-)
>