Re: [PATCH 01/13] gpu: drm: sti: 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/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