Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider: - [Medium] drm/i915/display: Incomplete FRL state verification missing tb_threshold_min and required_rate - [Medium] drm/i915: spurious WARN_ON in state checker due to uncleared FRL config bits --
commit 5c7a8a2a5c09104f1fb54f2b4082592bd27316bb Author: Ankit Nautiyal <[email protected]> drm/i915/display: Add FRL members to the state checker This commit introduces hardware state verification for FRL (Fixed Rate Link) by adding several FRL configuration members to the state checker. > diff --git a/drivers/gpu/drm/i915/display/intel_display.c > b/drivers/gpu/drm/i915/display/intel_display.c > --- a/drivers/gpu/drm/i915/display/intel_display.c > +++ b/drivers/gpu/drm/i915/display/intel_display.c > @@ -5626,6 +5626,14 @@ intel_pipe_config_compare(const struct > intel_crtc_state *current_config, > > PIPE_CONF_CHECK_I(set_context_latency); > > + PIPE_CONF_CHECK_BOOL(frl.enable); > + PIPE_CONF_CHECK_BOOL(frl.rsrc_sched_en); > + PIPE_CONF_CHECK_I(frl.required_lanes); > + PIPE_CONF_CHECK_I(frl.tb_actual); > + PIPE_CONF_CHECK_I(frl.active_char_buf_threshold); [Severity: Medium] Will checking frl.rsrc_sched_en and frl.active_char_buf_threshold trigger spurious WARN_ON state mismatches due to uncleared hardware bits? Looking at intel_hdmi_frl_cfg_write() in intel_hdmi.c, it appears to use intel_de_rmw() with a clear mask of 0: intel_hdmi.c:intel_hdmi_frl_cfg_write() { ... val = TRANS_HDMI_R_B_SCHED_ENABLE(crtc_state->frl.rsrc_sched_en); val |= TRANS_HDMI_ACTIVE_CHAR_BUF_THRESH(crtc_state->frl.active_char_buf_threshold); ... intel_de_rmw(display, TRANS_HDMI_FRL_CFG(display, cpu_trans), 0, val); } Since the clear mask is 0, preexisting bits from BIOS or prior configurations will persist in the hardware register. Will the state checker trip over these leftover bits when it compares them against the software state? > + PIPE_CONF_CHECK_I(frl.link_m); > + PIPE_CONF_CHECK_I(frl.link_n); [Severity: Medium] Should frl.tb_threshold_min and frl.required_rate also be verified here? The driver actively populates tb_threshold_min from hardware during state readout in intel_hdmi_frl_dfm_read(): intel_hdmi_frl_dfm.c:intel_hdmi_frl_dfm_read() { ... val = intel_de_read(display, TRANS_HDMI_FRL_DFMTHRSH(display, cpu_trans)); crtc_state->frl.tb_threshold_min = REG_FIELD_GET(TB_MIN_THESHOLD_MASK, val); ... } Similarly, frl.required_rate is populated during hardware readout in intel_hdmi_frl_get_config(). Without adding these fields to the state checker, can hardware state corruption or mismatches for the minimum threshold and required rate go undetected? > + > #undef PIPE_CONF_CHECK_X > #undef PIPE_CONF_CHECK_I > #undef PIPE_CONF_CHECK_LLI -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=46
