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

["Nautiyal, Ankit K" <ankit.k.nautiyal@intel.com>]

On 9/15/2026 10:52 PM, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Medium] drm/i915/dip: Spurious state mismatch for EMP_AS_SDP_TL on HDMI fastsets
> --
>
> commit 33aef03ce9383e1676d66a385c5b27a724956776
> Author: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
> Subject: drm/i915/dip: Store and use AS SDP transmission line from crtc state
>
> The driver computes and stores the Adaptive Sync SDP transmission line in the
> crtc state and uses it when programming the EMP_AS_SDP_TL register. This
> prepares the ground for supporting programmable transmission lines for
> additional DP SDPs.
>
>> 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 trigger a spurious state mismatch for EMP_AS_SDP_TL on HDMI fastsets?
>
> If a non-DP (e.g., HDMI) connection inherits a non-zero EMP_AS_SDP_TL value
> from a BIOS handover or a previous DP VRR mode, the hardware state
> (hw_crtc_state) will reflect this non-zero value.
>
> This happens because intel_dip_sdp_transmission_line_get_config()
> unconditionally reads the register here via intel_dip_read_emp_as_sdp_tl()
> without verifying if the encoder is DP:
>
> drivers/gpu/drm/i915/display/intel_dip.c:intel_dip_read_emp_as_sdp_tl() {
>      ...
>      val = intel_de_read(display, EMP_AS_SDP_TL(display, cpu_transcoder));
>      return REG_FIELD_GET(EMP_AS_SDP_DB_TL_MASK, val);
> }
>
> Meanwhile, the software state (sw_crtc_state) expects 0 for HDMI since
> intel_dip_sdp_tl_compute_config_late() is only called for DP.
>
> Because the dip.emp_as_sdp_tl comparison is guarded by if (!fastset) in
> intel_pipe_config_compare(), the discrepancy is ignored during the fastset
> compatibility check, allowing a fastset to proceed.
>
> The fastset skips reprogramming the VRR transcoder timings, leaving the
> hardware register at its stale non-zero value.


intel_vrr_set_transcoder_timings() is set from both modeset and fastset 
path and is written for all platforms that support VRR && have  
EMP_AS_SDP_TL.

intel_vrr_possible() is basically checking flipline that is always set 
for the platforms that have VRR and EMP_AS_SDP_TL, so there is no early 
return from intel_vrr_set_transcoder_timings() either.

Regards,

Ankit

>
> During post-commit hardware verification in verify_crtc_state(),
> intel_pipe_config_compare() is called with fastset = false, which strictly
> checks the field. This causes a drm_WARN backtrace when comparing the expected
> 0 against the stale non-zero hardware value.
>