Re: [PATCH v4] drm/i915/display: send null DRM infoframe when ending HDR transmission

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] The logic to send a zeroed DRM infoframe fails to enforce the CTA-861-H 2-second duration, risks premature termination, and causes unintended full modesets on subsequent atomic commits.
--

commit 1fa58c4612bfbba739489ee34fd7b6bc9eeeb05e
Author: Xiao Lu <xiaolu.xie@intel.com>
drm/i915/display: send null DRM infoframe when ending HDR transmission

This commit updates the driver to send a null DRM infoframe (EOTF=0) when
transitioning from HDR to SDR to comply with CTA-861-H. This prevents 
DP-to-HDMI converters from latching previous HDR metadata on SDR content.

> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
> index 113d767e62e9c..d5ed96ba7e7ba 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
[ ... ]
> @@ -3231,14 +3231,29 @@ intel_dp_in_hdr_mode(const struct drm_connector_state *conn_state)
>  static void
>  intel_dp_compute_hdr_metadata_infoframe_sdp(struct intel_dp *intel_dp,
>  					    struct intel_crtc_state *crtc_state,
> -					    const struct drm_connector_state *conn_state)
> +					    const struct drm_connector_state *conn_state,
> +					    const struct drm_connector_state *old_conn_state)
>  {
>  	struct intel_display *display = to_intel_display(intel_dp);
>  	int ret;
>  	struct hdmi_drm_infoframe *drm_infoframe = &crtc_state->infoframes.drm.drm;
>  
> -	if (!conn_state->hdr_output_metadata)
> +	if (!conn_state->hdr_output_metadata) {
> +		/*
> +		 * CTA-861-H requires ending HDR metadata transmission by
> +		 * sending a DRM infoframe with EOTF=0 and all fields zero
> +		 * for at least 2 seconds, rather than abruptly stopping.
> +		 * Abruptly stopping causes DP-to-HDMI converters to latch
> +		 * the previous HDR metadata and forward it to the HDMI sink,
> +		 * resulting in color errors on SDR content.
> +		 */
> +		if (old_conn_state && old_conn_state->hdr_output_metadata) {
> +			hdmi_drm_infoframe_init(drm_infoframe);
> +			crtc_state->infoframes.enable |=
> +				intel_hdmi_infoframe_enable(HDMI_PACKET_TYPE_GAMUT_METADATA);
> +		}

[Severity: High]
Does this logic correctly enforce the 2-second transmission requirement? 

Since this ties the lifecycle of the zeroed DRM infoframe to atomic commit 
boundaries rather than time, it appears the transmission could be terminated 
prematurely if userspace performs a subsequent atomic commit (such as a 
cursor update) within 2 seconds. In that case, both old_conn_state->hdr 
and conn_state->hdr would be false, so the DRM bit in 
crtc_state->infoframes.enable would be computed as cleared.

Additionally, could this cause an unintended full modeset? 

When that subsequent commit occurs, intel_pipe_config_compare() will check 
the active hardware state (which still has the DRM bit set) against the new 
state (which clears it). The mismatch might force a full screen blanking 
for a simple SDR cursor update.

>  		return;
> +	}
>  
>  	ret = drm_hdmi_infoframe_set_hdr_metadata(drm_infoframe, conn_state);
[ ... ]
> diff --git a/drivers/gpu/drm/i915/display/intel_hdmi.c b/drivers/gpu/drm/i915/display/intel_hdmi.c
> index 8a019d3574df2..9fb9be302d35b 100644
> --- a/drivers/gpu/drm/i915/display/intel_hdmi.c
> +++ b/drivers/gpu/drm/i915/display/intel_hdmi.c
[ ... ]
> @@ -856,8 +856,25 @@ intel_hdmi_compute_drm_infoframe(struct intel_encoder *encoder,
>  	if (!crtc_state->has_infoframe)
>  		return true;
>  
> -	if (!conn_state->hdr_output_metadata)
> +	if (!conn_state->hdr_output_metadata) {
> +		const struct drm_connector_state *old_conn_state =
> +			drm_atomic_get_old_connector_state(conn_state->state,
> +							   conn_state->connector);
> +		/*
> +		 * CTA-861-H requires ending HDR metadata transmission by
> +		 * sending a DRM infoframe with EOTF=0 and all fields zero
> +		 * for at least 2 seconds, rather than abruptly stopping.
> +		 * Abruptly stopping causes DP-to-HDMI converters to latch
> +		 * the previous HDR metadata and forward it to the HDMI sink,
> +		 * resulting in color errors on SDR content.
> +		 */
> +		if (old_conn_state && old_conn_state->hdr_output_metadata) {
> +			hdmi_drm_infoframe_init(frame);
> +			crtc_state->infoframes.enable |=
> +				intel_hdmi_infoframe_enable(HDMI_INFOFRAME_TYPE_DRM);
> +		}

[Severity: High]
The native HDMI path in intel_hdmi_compute_drm_infoframe() appears to share 
the same lifecycle vulnerability as the DP path. 

Without a timer or workqueue to manage the 2-second timeout, do we risk 
prematurely aborting the infoframe and triggering unintended modesets on 
subsequent SDR commits?

>  		return true;
> +	}
>  
>  	crtc_state->infoframes.enable |=
>  		intel_hdmi_infoframe_enable(HDMI_INFOFRAME_TYPE_DRM);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260902030411.258603-1-xiaolu.xie@intel.com?part=1