Re: [PATCH v8 6/9] drm/i915/lobf: Update lobf if any change in dependent parameters

["Hogander, Jouni" <jouni.hogander@intel.com>]

On Wed, 2025-04-09 at 19:32 +0530, Animesh Manna wrote:
> For every commit the dependent condition for LOBF is checked
> and accordingly update has_lobf flag which will be used
> to update the ALPM_CTL register during commit.
> 
> v1: Initial version.
> v2: Avoid reading h/w register without has_lobf check. [Jani]
> v3: Update LOBF in post plane update instead of separate function.
> [Jouni]
> v4:
> - Add lobf disable print. [Jouni]
> - Simplify condition check for enabling/disabling lobf. [Jouni]
> 
> Signed-off-by: Animesh Manna <animesh.manna@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_alpm.c | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_alpm.c
> b/drivers/gpu/drm/i915/display/intel_alpm.c
> index 01949b90c0c3..4fe1914ff759 100644
> --- a/drivers/gpu/drm/i915/display/intel_alpm.c
> +++ b/drivers/gpu/drm/i915/display/intel_alpm.c
> @@ -362,6 +362,8 @@ static void lnl_alpm_configure(struct intel_dp
> *intel_dp,
>  	if (crtc_state->has_lobf) {
>  		alpm_ctl |= ALPM_CTL_LOBF_ENABLE;
>  		drm_dbg_kms(display->drm, "Link off between frames
> (LOBF) enabled\n");
> +	} else {
> +		drm_dbg_kms(display->drm, "Link off between frames
> (LOBF) disabled\n");
>  	}
>  
>  	alpm_ctl |= ALPM_CTL_ALPM_ENTRY_CHECK(intel_dp-
> >alpm_parameters.check_entry_lines);
> @@ -382,9 +384,12 @@ void intel_alpm_post_plane_update(struct
> intel_atomic_state *state,
>  	struct intel_display *display = to_intel_display(state);
>  	const struct intel_crtc_state *crtc_state =
>  		intel_atomic_get_new_crtc_state(state, crtc);
> +	const struct intel_crtc_state *old_crtc_state =
> +		intel_atomic_get_old_crtc_state(state, crtc);
>  	struct intel_encoder *encoder;
>  
> -	if (!crtc_state->has_lobf && !crtc_state->has_psr)
> +	if (crtc_state->has_lobf == old_crtc_state->has_lobf &&
> +	    !crtc_state->has_psr)

I'm still concerned on LOBF disable. If LOBF is about to be disabled
here. It means new mode with parameters incompatible with LOBF are
already applied at this point. LOBF is still enabled and about to be
disabled here. Adding intel_alpm_pre_plane_update with LOBF disable
would solve this.

BR,

Jouni Högander

>  		return;
>  
>  	for_each_intel_encoder_mask(display->drm, encoder,