Re: [PATCH 4/5] drm/i915/alpm: Simplify and align LOBF checks in pre/post plane update

[Nautiyal, Ankit K]

On 12/17/2025 7:19 PM, Hogander, Jouni wrote:

On Wed, 2025-11-19 at 19:21 +0530, Ankit Nautiyal wrote:

The pre_plane_update and post_plane_update hooks essentially
disable/enable lobf feature. Use the existing
_is_enabling/is_disabling
logic for this in the pre_plane_update and post_plane_update paths.

Also rename the helpers to intel_alpm_lobf_{en,dis}able().

Signed-off-by: Ankit Nautiyal <ankit.k.nauti...@intel.com>
---
  drivers/gpu/drm/i915/display/intel_alpm.c    | 45 ++++++------------
--
  drivers/gpu/drm/i915/display/intel_alpm.h    |  6 +--
  drivers/gpu/drm/i915/display/intel_display.c | 28 +++++++++++-
  3 files changed, 41 insertions(+), 38 deletions(-)

diff --git a/drivers/gpu/drm/i915/display/intel_alpm.c
b/drivers/gpu/drm/i915/display/intel_alpm.c
index 5cfb9345776a..abeb70fcf92c 100644
--- a/drivers/gpu/drm/i915/display/intel_alpm.c
+++ b/drivers/gpu/drm/i915/display/intel_alpm.c
@@ -447,25 +447,17 @@ void intel_alpm_port_configure(struct intel_dp
*intel_dp,
        intel_de_write(display, PORT_ALPM_LFPS_CTL(port),
lfps_ctl_val);
  }

-void intel_alpm_pre_plane_update(struct intel_atomic_state *state,

-                                struct intel_crtc *crtc)
+void intel_alpm_lobf_disable(const struct intel_crtc_state
*new_crtc_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);
-       enum transcoder cpu_transcoder = crtc_state->cpu_transcoder;
+       struct intel_display *display =
to_intel_display(new_crtc_state);
+       enum transcoder cpu_transcoder = new_crtc_state-

cpu_transcoder;

        struct intel_encoder *encoder;

  if (DISPLAY_VER(display) < 20)

                return;

You can remove this as well. intel_crtc_lobf_enabling is supposed to be
true only on display version >= 20.

Ok sure will remove this check.

- if (crtc_state->has_lobf || crtc_state->has_lobf ==

old_crtc_state->has_lobf)
-               return;
-
        for_each_intel_encoder_mask(display->drm, encoder,
-                                   crtc_state->uapi.encoder_mask) {
+                                   new_crtc_state-

uapi.encoder_mask) {

                struct intel_dp *intel_dp;

  if (!intel_encoder_is_dp(encoder))

@@ -476,12 +468,10 @@ void intel_alpm_pre_plane_update(struct
intel_atomic_state *state,
                if (!intel_dp_is_edp(intel_dp))
                        continue;

- if (old_crtc_state->has_lobf) {

-                       mutex_lock(&intel_dp->alpm.lock);
-                       intel_de_write(display, ALPM_CTL(display,
cpu_transcoder), 0);
-                       drm_dbg_kms(display->drm, "Link off between
frames (LOBF) disabled\n");
-                       mutex_unlock(&intel_dp->alpm.lock);
-               }
+               mutex_lock(&intel_dp->alpm.lock);
+               intel_de_write(display, ALPM_CTL(display,
cpu_transcoder), 0);
+               drm_dbg_kms(display->drm, "Link off between frames
(LOBF) disabled\n");
+               mutex_unlock(&intel_dp->alpm.lock);
        }
  }

@@ -502,22 +492,13 @@ void intel_alpm_enable_sink(struct intel_dp

*intel_dp,
        drm_dp_dpcd_writeb(&intel_dp->aux, DP_RECEIVER_ALPM_CONFIG,
val);
  }

-void intel_alpm_post_plane_update(struct intel_atomic_state *state,

-                                 struct intel_crtc *crtc)
+void intel_alpm_lobf_enable(const struct intel_crtc_state
*new_crtc_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_display *display =
to_intel_display(new_crtc_state);
        struct intel_encoder *encoder;

- if (crtc_state->has_psr || !crtc_state->has_lobf ||

-           crtc_state->has_lobf == old_crtc_state->has_lobf)
-               return;
-
        for_each_intel_encoder_mask(display->drm, encoder,
-                                   crtc_state->uapi.encoder_mask) {
+                                   new_crtc_state-

uapi.encoder_mask) {

                struct intel_dp *intel_dp;

  if (!intel_encoder_is_dp(encoder))

@@ -526,8 +507,8 @@ void intel_alpm_post_plane_update(struct
intel_atomic_state *state,
                intel_dp = enc_to_intel_dp(encoder);

  if (intel_dp_is_edp(intel_dp)) {

-                       intel_alpm_enable_sink(intel_dp,
crtc_state);
-                       intel_alpm_configure(intel_dp, crtc_state);
+                       intel_alpm_enable_sink(intel_dp,
new_crtc_state);
+                       intel_alpm_configure(intel_dp,
new_crtc_state);
                }
        }
  }
diff --git a/drivers/gpu/drm/i915/display/intel_alpm.h
b/drivers/gpu/drm/i915/display/intel_alpm.h
index 14dc49fee4c3..0b2460b90465 100644
--- a/drivers/gpu/drm/i915/display/intel_alpm.h
+++ b/drivers/gpu/drm/i915/display/intel_alpm.h
@@ -25,12 +25,10 @@ void intel_alpm_configure(struct intel_dp
*intel_dp,
                          const struct intel_crtc_state
*crtc_state);
  void intel_alpm_enable_sink(struct intel_dp *intel_dp,
                            const struct intel_crtc_state
*crtc_state);
-void intel_alpm_pre_plane_update(struct intel_atomic_state *state,
-                                struct intel_crtc *crtc);
+void intel_alpm_lobf_disable(const struct intel_crtc_state
*new_crtc_state);
  void intel_alpm_port_configure(struct intel_dp *intel_dp,
                               const struct intel_crtc_state
*crtc_state);
-void intel_alpm_post_plane_update(struct intel_atomic_state *state,
-                                 struct intel_crtc *crtc);
+void intel_alpm_lobf_enable(const struct intel_crtc_state
*new_crtc_state);
  void intel_alpm_lobf_debugfs_add(struct intel_connector *connector);
  bool intel_alpm_aux_wake_supported(struct intel_dp *intel_dp);
  bool intel_alpm_aux_less_wake_supported(struct intel_dp *intel_dp);
diff --git a/drivers/gpu/drm/i915/display/intel_display.c
b/drivers/gpu/drm/i915/display/intel_display.c
index 069967114bd9..ecbdc506a1fc 100644
--- a/drivers/gpu/drm/i915/display/intel_display.c
+++ b/drivers/gpu/drm/i915/display/intel_display.c
@@ -1009,6 +1009,27 @@ static bool intel_casf_disabling(const struct
intel_crtc_state *old_crtc_state,
        return is_disabling(hw.casf_params.casf_enable,
old_crtc_state, new_crtc_state);
  }

+static bool intel_crtc_lobf_enabling(const struct intel_crtc_state

*old_crtc_state,
+                                    const struct intel_crtc_state
*new_crtc_state)
+{
+       if (!new_crtc_state->hw.active)
+               return false;

Is it possible that new_crtc_state->hw.active == false &&
new_crtc_state->has_lobf == true

I dont think its possible.

+
+       if (!new_crtc_state->has_psr)
+               return false;

I don't understand why you are checking psr here? I should have been
checked already on compute phase?

hmm yes right this can be dropped here.

+
+       return is_enabling(has_lobf, old_crtc_state,
new_crtc_state);
+}
+
+static bool intel_crtc_lobf_disabling(const struct intel_crtc_state
*old_crtc_state,
+                                     const struct intel_crtc_state
*new_crtc_state)
+{
+       if (!new_crtc_state->hw.active)
+               return false;

No need to disable before everything else is disabled?

Sorry this should be :

       if (!old_crtc_state->hw.active)
                return false;

If pipe was already inactive we don't need to do anything.

Thanks for catching this.

I will update the patch with suggested changes.

Thanks & Regards,

Ankit

BR,

Jouni Högander

+
+       return is_disabling(has_lobf, old_crtc_state,
new_crtc_state);
+}
+
  #undef is_disabling
  #undef is_enabling

@@ -1056,7 +1077,8 @@ static void intel_post_plane_update(struct

intel_atomic_state *state,
                        adl_scaler_ecc_unmask(new_crtc_state);
        }

- intel_alpm_post_plane_update(state, crtc);

+       if (intel_crtc_lobf_enabling(old_crtc_state,
new_crtc_state))
+               intel_alpm_lobf_enable(new_crtc_state);

  intel_psr_post_plane_update(state, crtc);

  }
@@ -1153,7 +1175,9 @@ static void intel_pre_plane_update(struct
intel_atomic_state *state,
                intel_atomic_get_new_crtc_state(state, crtc);
        enum pipe pipe = crtc->pipe;

- intel_alpm_pre_plane_update(state, crtc);

+       if (intel_crtc_lobf_disabling(old_crtc_state,
new_crtc_state))
+               intel_alpm_lobf_disable(new_crtc_state);
+
        intel_psr_pre_plane_update(state, crtc);

  if (intel_crtc_vrr_disabling(state, crtc)) {