Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider: - [Medium] Spurious state mismatch warnings for dip.emp_as_sdp_tl due to unconditional hardware readout of stale register values. - [Medium] Unconditional fastset check for `dip.emp_as_sdp_tl` breaks seamless Link Refresh Rate (LRR) updates, forcing heavy full modesets. --
commit fdc12bb4795c43a7d04cbec4746e152e2a99a6d8 Author: Ankit Nautiyal <[email protected]> drm/i915/dip: Store and use AS SDP transmission line from crtc state This commit moves the computation of the Adaptive Sync SDP transmission line from programming time to the crtc state computation phase. It stores the value in the crtc state to be used when programming the EMP_AS_SDP_TL register, preparing for programmable transmission lines for other DP SDPs. > diff --git a/drivers/gpu/drm/i915/display/intel_ddi.c > b/drivers/gpu/drm/i915/display/intel_ddi.c > --- 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] Could this unconditional hardware readout capture stale values and cause spurious state mismatch warnings? intel_dip_sdp_transmission_line_get_config() unconditionally reads the EMP_AS_SDP_TL hardware register. If the encoder is HDMI, or if VRR was disabled, the computed software state leaves dip.emp_as_sdp_tl = 0. However, when VRR is disabled, intel_vrr_set_transcoder_timings() in drivers/gpu/drm/i915/display/intel_vrr.c returns early and never clears EMP_AS_SDP_TL. This leaves a stale non-zero value in the hardware register from a previous DP modeset. Could this mismatch cause intel_pipe_config_compare() to fail, incorrectly forcing an unnecessary full modeset instead of a fastset? > diff --git a/drivers/gpu/drm/i915/display/intel_dip.c > b/drivers/gpu/drm/i915/display/intel_dip.c > --- a/drivers/gpu/drm/i915/display/intel_dip.c > +++ b/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); > +} [Severity: Medium] Does computing and checking this transmission line state break seamless Link Refresh Rate (LRR) updates? LRR is designed to perform seamless refresh rate transitions without screen blanking. The intel_pipe_config_compare() function in intel_display.c ignores vrr.vsync_start differences during a fastset because it is explicitly guarded by an if (!fastset) condition. Because dip.emp_as_sdp_tl is computed directly from vrr.vsync_start, and its corresponding state check (PIPE_CONF_CHECK_I) runs outside the !fastset block, an LRR transition that naturally alters the vtotal and vsync_start will produce a mismatch. Will this erroneously fail the fastset check and force a heavy full modeset instead of a seamless transition? > + > +void intel_dip_sdp_transmission_line_get_config(struct intel_crtc_state > *crtc_state) > +{ > + crtc_state->dip.emp_as_sdp_tl = > intel_dip_read_emp_as_sdp_tl(crtc_state); > } -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=6
