Re: [PATCH] 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:
- [Medium] Stateless implementation of the CTA-861-H 2-second requirement causes indefinite transmission, premature termination, and spurious state mismatches.
--

commit 78df3126b776d1f0e5455dd977f44bfe09013697
Author: xiaolu <xiaolu@intel.com>

drm/i915/display: send null DRM infoframe when ending HDR transmission

This commit aims to fix color errors on SDR content when transitioning from
HDR to SDR by sending a null DRM infoframe for at least 2 seconds as required
by CTA-861-H.

> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
> index 6b60e0a1c8eef..7680594ae6702 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: Medium]
Could this introduce a regression where the zeroed DRM infoframe is either
transmitted indefinitely or terminated prematurely depending on subsequent
commits?

This logic in intel_dp_compute_hdr_metadata_infoframe_sdp() enables the
zeroed infoframe during the HDR to SDR modeset, but does not provide a
mechanism to disable it after 2 seconds. If the compositor does not perform
another commit, the infoframe transmission appears to continue indefinitely.

Conversely, if the compositor performs an unrelated atomic commit (for
example, updating a cursor plane) within 2 seconds of the HDR transition,
won't this immediately disable the infoframe? The recalculation of the state
would see no HDR metadata in the old state and skip setting the enable bit,
terminating the infoframe prematurely and violating the CTA-861-H requirement.

>  		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 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: Medium]
Does this face the same stateless implementation regression as the DP path?

Like in intel_dp_compute_hdr_metadata_infoframe_sdp(), this logic in
intel_hdmi_compute_drm_infoframe() indefinitely prolongs the transmission if
there are no further commits, or truncates it if rapid modesets occur within
the 2-second window.

>  		return true;
> +	}

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