On Wed, 2025-11-19 at 19:21 +0530, Ankit Nautiyal wrote:
> Currently intel_alpm_lobf_compute_config() tries to account for
> guardband +SCL requirements during encoder->compute_config() phase,
> even before guardband is computed.
> Also, LOBF depends on crtc_state->has_psr which can be modified in
> encoder->compute_config_late().
>
> Account for lobf requirements while optimizing the guardband and add
> checks for final guardband in encoder->compute_config_late() phase
> after
> the guardband and the final state of crtc_state->has_psr are already
> computed.
>
> Use crtc_state->vrr.guardband and crtc_state->set_context_latency for
> the computation and add more documentation for the dependency of
> first
> sdp position, guardband, set context latency and wake lines.
>
> v2: Add helper to use min guardband required for lobf.
>
> Bspec:71041
> Signed-off-by: Ankit Nautiyal <[email protected]>
> ---
> drivers/gpu/drm/i915/display/intel_alpm.c | 63 +++++++++++++++++---
> --
> drivers/gpu/drm/i915/display/intel_alpm.h | 3 ++
> drivers/gpu/drm/i915/display/intel_dp.c | 2 +
> drivers/gpu/drm/i915/display/intel_intel.c | 0
> drivers/gpu/drm/i915/display/intel_vrr.c | 2 +
> 5 files changed, 56 insertions(+), 14 deletions(-)
> create mode 100644 drivers/gpu/drm/i915/display/intel_intel.c
>
> diff --git a/drivers/gpu/drm/i915/display/intel_alpm.c
> b/drivers/gpu/drm/i915/display/intel_alpm.c
> index 6372f533f65b..98cbf5dde73b 100644
> --- a/drivers/gpu/drm/i915/display/intel_alpm.c
> +++ b/drivers/gpu/drm/i915/display/intel_alpm.c
> @@ -15,6 +15,7 @@
> #include "intel_dp_aux.h"
> #include "intel_psr.h"
> #include "intel_psr_regs.h"
> +#include "intel_vrr.h"
>
> #define SILENCE_PERIOD_MIN_TIME 80
> #define SILENCE_PERIOD_MAX_TIME 180
> @@ -248,14 +249,58 @@ bool intel_alpm_compute_params(struct intel_dp
> *intel_dp,
> return true;
> }
>
> +int intel_alpm_lobf_min_guardband(struct intel_crtc_state
> *crtc_state)
> +{
> + struct drm_display_mode *adjusted_mode = &crtc_state-
> >hw.adjusted_mode;
> + int first_sdp_position = adjusted_mode->crtc_vtotal -
> + adjusted_mode->crtc_vsync_start;
> + int waketime_in_lines = max(crtc_state-
> >alpm_state.io_wake_lines,
> + crtc_state-
> >alpm_state.aux_less_wake_lines);
I think we should fix this to use proper value instead of just max. At
this point FIXME would be enough.
> +
> + if (!crtc_state->has_lobf)
> + return 0;
> +
> + return first_sdp_position + waketime_in_lines + crtc_state-
> >set_context_latency;
> +}
> +
> +void intel_alpm_lobf_compute_config_late(struct intel_dp *intel_dp,
> + struct intel_crtc_state
> *crtc_state)
> +{
> + struct drm_display_mode *adjusted_mode = &crtc_state-
> >hw.adjusted_mode;
> + int waketime_in_lines, first_sdp_position;
> +
> + if (!crtc_state->has_lobf)
> + return;
> +
> + /*
> + * LOBF can only be enabled if the time from the start of
> the SCL+Guardband
> + * window to the position of the first SDP is greater than
> the time it takes
> + * to wake the main link.
> + *
> + * Position of first sdp : vsync_start
> + * start of scl + guardband : vtotal - (scl + guardband)
> + * time in lines to wake main link : waketime_in_lines
> + *
> + * Position of first sdp - start of (scl + guardband) > time
> in lines to wake main link
> + * vsync_start - (vtotal - (scl + guardband)) >
> waketime_in_lines
> + * vsync_start - vtotal + scl + guardband >
> waketime_in_lines
> + * scl + guardband > waketime_in_lines + (vtotal -
> vsync_start)
> + */
> + first_sdp_position = adjusted_mode->crtc_vtotal -
> adjusted_mode->crtc_vsync_start;
> + if (intel_alpm_aux_less_wake_supported(intel_dp))
> + waketime_in_lines = crtc_state-
> >alpm_state.io_wake_lines;
> + else
> + waketime_in_lines = crtc_state-
> >alpm_state.aux_less_wake_lines;
> +
> + crtc_state->has_lobf = (crtc_state->set_context_latency +
> crtc_state->vrr.guardband) >
> + (first_sdp_position +
> waketime_in_lines);
Now if crtc_state->has_lobf is switching to false at this point we
still have lobf guardband requirement included in our optimized
guardband. Is that ok?
BR,
Jouni Högander
> +}
> +
> void intel_alpm_lobf_compute_config(struct intel_dp *intel_dp,
> struct intel_crtc_state
> *crtc_state,
> struct drm_connector_state
> *conn_state)
> {
> struct intel_display *display = to_intel_display(intel_dp);
> - struct drm_display_mode *adjusted_mode = &crtc_state-
> >hw.adjusted_mode;
> - int waketime_in_lines, first_sdp_position;
> - int context_latency, guardband;
>
> if (intel_dp->alpm.lobf_disable_debug) {
> drm_dbg_kms(display->drm, "LOBF is disabled by debug
> flag\n");
> @@ -288,17 +333,7 @@ void intel_alpm_lobf_compute_config(struct
> intel_dp *intel_dp,
> if (!intel_alpm_compute_params(intel_dp, crtc_state))
> return;
>
> - context_latency = adjusted_mode->crtc_vblank_start -
> adjusted_mode->crtc_vdisplay;
> - guardband = adjusted_mode->crtc_vtotal -
> - adjusted_mode->crtc_vdisplay - context_latency;
> - first_sdp_position = adjusted_mode->crtc_vtotal -
> adjusted_mode->crtc_vsync_start;
> - if (intel_alpm_aux_less_wake_supported(intel_dp))
> - waketime_in_lines = crtc_state-
> >alpm_state.io_wake_lines;
> - else
> - waketime_in_lines = crtc_state-
> >alpm_state.aux_less_wake_lines;
> -
> - crtc_state->has_lobf = (context_latency + guardband) >
> - (first_sdp_position + waketime_in_lines);
> + crtc_state->has_lobf = true;
> }
>
> static void lnl_alpm_configure(struct intel_dp *intel_dp,
> diff --git a/drivers/gpu/drm/i915/display/intel_alpm.h
> b/drivers/gpu/drm/i915/display/intel_alpm.h
> index 53599b464dea..14dc49fee4c3 100644
> --- a/drivers/gpu/drm/i915/display/intel_alpm.h
> +++ b/drivers/gpu/drm/i915/display/intel_alpm.h
> @@ -38,4 +38,7 @@ bool intel_alpm_is_alpm_aux_less(struct intel_dp
> *intel_dp,
> const struct intel_crtc_state
> *crtc_state);
> void intel_alpm_disable(struct intel_dp *intel_dp);
> bool intel_alpm_get_error(struct intel_dp *intel_dp);
> +void intel_alpm_lobf_compute_config_late(struct intel_dp *intel_dp,
> + struct intel_crtc_state
> *crtc_state);
> +int intel_alpm_lobf_min_guardband(struct intel_crtc_state
> *crtc_state);
> #endif
> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c
> b/drivers/gpu/drm/i915/display/intel_dp.c
> index 0ec82fcbcf48..782e981bbc89 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> @@ -7020,6 +7020,8 @@ int intel_dp_compute_config_late(struct
> intel_encoder *encoder,
> if (ret)
> return ret;
>
> + intel_alpm_lobf_compute_config_late(intel_dp, crtc_state);
> +
> return 0;
> }
>
> diff --git a/drivers/gpu/drm/i915/display/intel_intel.c
> b/drivers/gpu/drm/i915/display/intel_intel.c
> new file mode 100644
> index 000000000000..e69de29bb2d1
> diff --git a/drivers/gpu/drm/i915/display/intel_vrr.c
> b/drivers/gpu/drm/i915/display/intel_vrr.c
> index b92c42fde937..fcecdf3dc78c 100644
> --- a/drivers/gpu/drm/i915/display/intel_vrr.c
> +++ b/drivers/gpu/drm/i915/display/intel_vrr.c
> @@ -6,6 +6,7 @@
>
> #include <drm/drm_print.h>
>
> +#include "intel_alpm.h"
> #include "intel_de.h"
> #include "intel_display_regs.h"
> #include "intel_display_types.h"
> @@ -451,6 +452,7 @@ int intel_vrr_compute_optimized_guardband(struct
> intel_crtc_state *crtc_state)
> if (intel_crtc_has_dp_encoder(crtc_state)) {
> guardband = max(guardband,
> intel_psr_min_guardband(crtc_state));
> guardband = max(guardband,
> intel_dp_sdp_min_guardband(crtc_state, true));
> + guardband = max(guardband,
> intel_alpm_lobf_min_guardband(crtc_state));
> }
>
> return guardband;
