Re: [PATCH v3] 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 intended to send a null DRM infoframe for 2 seconds fails to enforce the CTA-861-H requirement, either transmitting the infoframe indefinitely during SDR operation or stopping prematurely on subsequent modesets, and forces an unnecessary full modeset.
--

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

This commit modifies the DP and HDMI display paths to send a zeroed DRM
infoframe when transitioning from HDR to SDR. This intends to fulfill the
CTA-861-H requirement to send this null infoframe for at least 2 seconds
rather than abruptly stopping HDR metadata transmission.

> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
> index 0cd5e6b5034cf..f71e2a021663a 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 fulfill the 2-second CTA-861-H requirement mentioned in the
comment? The zeroed infoframe is enabled here when transitioning from HDR to
SDR, but it doesn't appear to be disabled by a timer after 2 seconds. Can
this lead to indefinite transmission of the null infoframe during SDR
operation if no subsequent commits occur?

Furthermore, because this logic strictly relies on
old_conn_state->hdr_output_metadata, won't the very next atomic commit
involving the connector evaluate this as false? That would drop the DRM
infoframe enable bit, causing intel_pipe_config_compare() to flag a mismatch
in infoframes.enable. Will this force an unexpected full modeset on the
subsequent commit?

>  		return;
> +	}

[ ... ]

> diff --git a/drivers/gpu/drm/i915/display/intel_hdmi.c b/drivers/gpu/drm/i915/display/intel_hdmi.c
> index 9b637e38a1a51..bdacad8ea30a3 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]
Similarly for the HDMI path in intel_hdmi_compute_drm_infoframe(), does this
suffer from the same indefinite transmission and fastset mismatch issues on
subsequent atomic commits as the DP path?

>  		return true;
> +	}

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