On Wed, 2025-12-17 at 15:10 +0200, Hogander, Jouni wrote: > 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]>
Reviewed-by: Jouni Högander <[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; >
