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