Re: [PATCH v6 16/22] drm: bridge: dw_hdmi: Use display_info is_hdmi and has_audio

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Direct, lockless access to `connector->display_info` from the `atomic_enable` path creates a race condition with concurrent EDID probes, which can result in incorrectly initializing the display in DVI mode with wrong audio or SCDC settings.
--

commit 339e5d6c4cc3c0ee74a9425b8a94dabb6a044d69
Author: Jonas Karlman <jonas@kwiboo.se>

drm: bridge: dw_hdmi: Use display_info is_hdmi and has_audio

This commit removes the private sink_is_hdmi and sink_has_audio tracking in
dw_hdmi, and switches to using display->is_hdmi and display->has_audio
from connector->display_info directly instead.

> diff --git a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> index 1402b3963ae12..e9c4e24c090ca 100644
> --- a/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
> +++ b/drivers/gpu/drm/bridge/synopsys/dw-hdmi.c
[ ... ]
> @@ -2053,7 +2047,7 @@ static void hdmi_av_composer(struct dw_hdmi *hdmi,
>  		HDMI_FC_INVIDCONF_IN_I_P_INTERLACED :
>  		HDMI_FC_INVIDCONF_IN_I_P_PROGRESSIVE;
>  
> -	inv_val |= hdmi->sink_is_hdmi ?
> +	inv_val |= display->is_hdmi ?

Is it safe to read connector->display_info directly in the atomic_enable path?

Since dw_hdmi_poweron() and hdmi_av_composer() are called from the bridge's
atomic_enable callback, they can run asynchronously for non-blocking commits
after the DRM modeset locks have been released.

If a concurrent EDID probe triggered by userspace (e.g., via
DRM_IOCTL_MODE_GETCONNECTOR) calls drm_edid_connector_update() on another CPU,
it starts by calling drm_reset_display_info() which clears display_info.is_hdmi
and has_audio to false until the EDID is fully re-parsed.

If this code reads the display_info fields during that window, won't it observe
transient zeroes and incorrectly program the hardware for DVI mode, as well as
disabling SCDC scrambling and audio?

Would it be better to capture these properties into an atomic state structure
during atomic_check() and read them safely from the state object during
atomic_enable()?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260516183838.2024991-1-jonas@kwiboo.se?part=16