Re: [RFC v1 1/3] drm/i915/dip: Send HDMI EMPs early in V. Blank when VRR is in range

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] drm/i915/dip: unreachable HDMI branch in intel_dip_get_as_sdp_transmission_line()
--

commit d831d02efdf03e1121867448c9abebc827f0c335
Author: Alan Stead <alan.stead@intel.com>

drm/i915/dip: Send HDMI EMPs early in V. Blank when VRR is in range

This commit updates intel_dip_get_as_sdp_transmission_line() so that HDMI
outputs with VRR in range send EMPs on the second line of V. Blank within
the SCL + guardband region rather than at the start of the VSYNC pulse.

> diff --git a/drivers/gpu/drm/i915/display/intel_dip.c b/drivers/gpu/drm/i915/display/intel_dip.c
> index f447150ff7bc0..831881e6c15ec 100644
> --- a/drivers/gpu/drm/i915/display/intel_dip.c
> +++ b/drivers/gpu/drm/i915/display/intel_dip.c
> @@ -19,6 +19,25 @@ static int intel_dip_get_as_sdp_transmission_line(const struct intel_crtc_state
>  	if (!HAS_EMP_AS_SDP_TL(display))
>  		return 0;
>  
> +	/*
> +	 * HDMI 2.1 expects the EMPs (such as the VTEM used for VRR) within
> +	 * the first half of V. Blank, so send them on the 2nd line of the
> +	 * V. Blank rather than at the start of the VSYNC pulse. The line
> +	 * must also stay within the SCL + guardband region.
> +	 */
> +	if (intel_crtc_has_type(crtc_state, INTEL_OUTPUT_HDMI) &&
> +	    crtc_state->vrr.in_range) {
> +		const struct drm_display_mode *mode = &crtc_state->hw.adjusted_mode;
> +		int max_tl = crtc_state->set_context_latency +
> +			crtc_state->vrr.guardband - 1;
> +		int tl = mode->crtc_vtotal - mode->crtc_vdisplay - 1;
> +
> +		if (max_tl > 0)
> +			tl = min(tl, max_tl);
> +
> +		return tl;
> +	}

[Severity: High]
Can this HDMI branch in intel_dip_get_as_sdp_transmission_line() ever be
reached?

Looking at the call chain, intel_dip_get_as_sdp_transmission_line() is only
called from intel_dip_sdp_tl_compute_config_late():

drivers/gpu/drm/i915/display/intel_dip.c:
void intel_dip_sdp_tl_compute_config_late(struct intel_crtc_state *crtc_state)
{
	crtc_state->dip.emp_as_sdp_tl = intel_dip_get_as_sdp_transmission_line(crtc_state);

	intel_dip_cmn_sdp_tl_compute_config_late(crtc_state);
}

And intel_dip_sdp_tl_compute_config_late() is only called from
intel_dp_compute_config_late() -> intel_dp_sdp_compute_config_late(), which
is gated on intel_crtc_has_dp_encoder(crtc_state) in
intel_ddi_compute_config_late():

drivers/gpu/drm/i915/display/intel_ddi.c:
static int intel_ddi_compute_config_late(struct intel_atomic_state *state,
					 struct intel_encoder *encoder,
					 struct intel_crtc_state *crtc_state,
					 struct drm_connector_state *conn_state)
{
	...
	if (intel_crtc_has_dp_encoder(crtc_state))
		ret = intel_dp_compute_config_late(encoder, crtc_state, conn_state);

Because DDI encoders are not cloneable, intel_crtc_has_dp_encoder(crtc_state)
is false whenever intel_crtc_has_type(crtc_state, INTEL_OUTPUT_HDMI) is true,
leaving crtc_state->dip.emp_as_sdp_tl at 0 for HDMI outputs.

In addition, if intel_dip_sdp_tl_compute_config_late() were called for HDMI,
non-VRR HDMI modes (!crtc_state->vrr.in_range) would fall through to return
crtc_state->vrr.vsync_start instead of 0, and
intel_dip_cmn_sdp_tl_compute_config_late() would populate
crtc_state->dip.cmn_sdp_tl and the SDP stagger fields that are only
programmed on DP in mtl_ddi_pre_enable_dp(), causing state verification
mismatches in intel_pipe_config_compare() on platforms with
HAS_COMMON_SDP_TL(display).

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261030203314.1085262-1-alan.stead@intel.com?part=1