Re: [PATCH v2 42/56] drm/i915/hdmi: Add read out for FRL state

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] drm/i915: unconditional extraction of FRL configuration fields causes state mismatches
--

commit 7fe3edceb823d24a4b2a0a493b7dfd96cc58cbdd
Author: Ankit Nautiyal <ankit.k.nautiyal@intel.com>

drm/i915/hdmi: Add read out for FRL state

Read out the FRL configuration and Data Flow Metering registers and fill the
FRL members in crtc_state. In subsequent changes these are added to the
state checker and compared bwteen two crtc_states in
intel_pipe_config_compare().

> diff --git a/drivers/gpu/drm/i915/display/intel_hdmi.c b/drivers/gpu/drm/i915/display/intel_hdmi.c
> index 69612ebe80f04..c5f25d2d6331c 100644
> --- a/drivers/gpu/drm/i915/display/intel_hdmi.c
> +++ b/drivers/gpu/drm/i915/display/intel_hdmi.c

[ ... ]

> @@ -4160,3 +4160,38 @@ void intel_hdmi_frl_cfg_write(const struct intel_crtc_state *crtc_state)
>  
>  	intel_de_rmw(display, TRANS_HDMI_FRL_CFG(display, cpu_trans), 0, val);
>  }
> +
> +static void intel_hdmi_frl_cfg_read(struct intel_crtc_state *crtc_state)
> +{
> +	struct intel_display *display = to_intel_display(crtc_state);
> +	enum transcoder cpu_trans = crtc_state->cpu_transcoder;
> +	u32 val;
> +
> +	val = intel_de_read(display, TRANS_HDMI_FRL_CFG(display, cpu_trans));
> +
> +	if (val & TRANS_HDMI_FRL_ENABLE)
> +		crtc_state->frl.enable = true;
> +
> +	crtc_state->frl.rsrc_sched_en =
> +		REG_FIELD_GET(TRANS_HDMI_R_B_SCHED_ENABLE_MASK, val);
> +	crtc_state->frl.active_char_buf_threshold =
> +		REG_FIELD_GET(TRANS_HDMI_ACTIVE_CHAR_BUF_THRESH_MASK, val);

[Severity: Medium]
Will this unconditional extraction cause state mismatch warnings during
fastset?

When FRL is disabled, it typically only clears the TRANS_HDMI_FRL_ENABLE bit,
potentially leaving stale non-zero values in the other bitfields of this
register. Because these fields are extracted and stored in the hardware state
without being guarded by a check for TRANS_HDMI_FRL_ENABLE, the hardware state
will contain these stale values even for non-FRL (TMDS) modes.

If these fields are then compared in intel_pipe_config_compare(), it may
trigger a state mismatch because the computed software state has these fields
properly zeroed out for TMDS modes.

> +}

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