Re: [PATCH v6 06/13] drm/i915/dip: Store and use AS SDP transmission line from crtc state

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] Spurious state mismatch warning due to un-cleared EMP_AS_SDP_TL register for non-VRR and HDMI modesets.
- [Low] AS SDP transmission line is computed unconditionally for DP, contradicting the commit message claim.
--

commit 058f1f7445f91002d16b429b35d8a7cd903b86af
Author: Ankit Nautiyal <ankit.k.nautiyal@intel.com>

drm/i915/dip: Store and use AS SDP transmission line from crtc state

Computes and stores the Adaptive Sync SDP transmission line in the
crtc state rather than computing it at programming time.

> diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c b/drivers/gpu/drm/i915/display/intel_ddi.c
> index 9b3b526e5e55b..dacb4b7588a02 100644
> --- a/drivers/gpu/drm/i915/display/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/display/intel_ddi.c
[ ... ]
> @@ -4235,6 +4236,7 @@ static void intel_ddi_get_config(struct intel_encoder *encoder,
>  	intel_read_dp_sdp(encoder, pipe_config, HDMI_PACKET_TYPE_GAMUT_METADATA);
>  	intel_read_dp_sdp(encoder, pipe_config, DP_SDP_VSC);
>  	intel_read_dp_sdp(encoder, pipe_config, DP_SDP_ADAPTIVE_SYNC);
> +	intel_dip_sdp_transmission_line_get_config(pipe_config);

[Severity: Medium]
Will this lead to a state mismatch warning on non-VRR or HDMI modesets?

When transitioning from a DP monitor with VRR enabled to an HDMI monitor or DP
with VRR disabled, intel_vrr_possible() returns false. This causes
intel_vrr_set_transcoder_timings() to return early without calling
intel_dip_write_emp_as_sdp_tl():

drivers/gpu/drm/i915/display/intel_vrr.c:intel_vrr_set_transcoder_timings() {
	if (!intel_vrr_possible(crtc_state)) {
		intel_de_write(display,
			       TRANS_VRR_CTL(display, cpu_transcoder), 0);
		return;
	}
	...
	intel_dip_write_emp_as_sdp_tl(crtc_state);
}

Since intel_dip_write_emp_as_sdp_tl() is skipped, the EMP_AS_SDP_TL hardware
register is left with a stale non-zero value.

Will intel_pipe_config_compare() now throw a state mismatch error because the
software state is computed as 0 for HDMI, but the hardware register retains
the stale VRR value read here?

>  
>  	intel_audio_codec_get_config(encoder, pipe_config);
>  }
[ ... ]
> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
> index 0cd5e6b5034cf..8c3a324e164ec 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
[ ... ]
> @@ -7324,6 +7325,8 @@ int intel_dp_sdp_compute_config_late(struct intel_crtc_state *crtc_state)
>  		return -EINVAL;
>  	}
>  
> +	intel_dip_sdp_tl_compute_config_late(crtc_state);

[Severity: Low]
Does this execute unconditionally for DP, contradicting the commit message?

The commit message states: "we must configure the AS SDP transmission line at
this point when AS SDP is enabled in crtc_state."

However, this is called without checking if DP_SDP_ADAPTIVE_SYNC is actually
present in crtc_state->infoframes.enable, which populates the software state
variable with vsync_start even when AS SDP is not enabled. Should this have a
conditional check?

> +
>  	return 0;
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260913145649.3240904-1-ankit.k.nautiyal@intel.com?part=6