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

[sashiko-bot@kernel.org]

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