On 10/30/2025 4:05 PM, Ville Syrjälä wrote:
On Thu, Oct 30, 2025 at 10:47:55AM +0530, Ankit Nautiyal wrote:
LOBF must be disabled if the number of lines within Window 1 is not greater
than ALPM_CTL[ALPM Entry Check]
Bspec:71041
Signed-off-by: Ankit Nautiyal <[email protected]>
---
drivers/gpu/drm/i915/display/intel_alpm.c | 13 +++++++++++++
1 file changed, 13 insertions(+)
diff --git a/drivers/gpu/drm/i915/display/intel_alpm.c
b/drivers/gpu/drm/i915/display/intel_alpm.c
index 1c240dd8d668..58cd0d2a4395 100644
--- a/drivers/gpu/drm/i915/display/intel_alpm.c
+++ b/drivers/gpu/drm/i915/display/intel_alpm.c
@@ -255,6 +255,7 @@ void intel_alpm_lobf_compute_config_late(struct intel_dp
*intel_dp,
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 window1;
if (intel_dp->alpm.lobf_disable_debug) {
drm_dbg_kms(display->drm, "LOBF is disabled by debug flag\n");
@@ -287,6 +288,18 @@ void intel_alpm_lobf_compute_config_late(struct intel_dp
*intel_dp,
if (!intel_alpm_compute_params(intel_dp, crtc_state))
return;
+ /*
+ * LOBF must be disabled if the number of lines within Window 1 is not
+ * greater than ALPM_CTL[ALPM Entry Check]
+ */
+ window1 = adjusted_mode->crtc_vtotal -
+ (adjusted_mode->crtc_vdisplay +
+ crtc_state->vrr.guardband +
+ crtc_state->set_context_latency);
vdisplay+guardband+scl isn't a value that means anything.
So the extra parentheses make this rather confusing.
Yes true, vdisplay+guardband+scl doesnt make any sense in itself. What I
intended was:
vblank = (adjusted_mode->crtc_vtotal - adjusted_mode->crtc_vdisplay);
window1 = vblank - (crtc_state->vrr.guardband +
crtc_state->set_context_latency);
This also doesn't account for the case where SCL would start already
in vertical active on PTL+. I know we don't do that currently, but we
should probably write this in a way that accounts for that so that we
don't have to rewrite this yet again.
Fair point. Then something like:
vblank = (adjusted_mode->crtc_vtotal - adjusted_mode->crtc_vdisplay);
scl_gb_region = min(vblank, crtc_state->vrr.guardband +
crtc_state->set_context_latency)
window1 = vblank - scl_gb_region;
Apparently this lobf stuff depends on actually having a window1,
which means it can only be used if we're using the VRR timing generator
with an optimized guardband. That means it never did anything (or it
did things wrong) until you enabled the optimized guardband.
Yeah right. Without optimized guardband, window1 would be always 0 so no
scope to use this.
It sort of looks like intel_alpm_{pre,post}_plane_update() might
allow us to enable this even for the 'vrr.enable==true &&
always_use_vrr_tg()==false' case. Either that or we should just
enable it for the always_use_vrr_tg()==true case. Whatever the
case, we definitely seem to be missing some kind of a VRR check
here.
I was just short of adding check for always_use_vrr_tg() &&
intel_vrr_is_fixed_rr()
Bspec says that lobf can be enabled only with fixed refresh rate and
mentions flipline = vmin =vmax.
I was not sure if its implicit to enable this only with VRR TG and not
with legacy timing generator.
Makes sense to replace the existing check with for flipline=vmin=vmax with:
intel_vrr_always_use_vrr_tg() && intel_vrr_is_fixed_rr()
I'm also 90% sure that alpm pre/post plane stuff isn't handling
DRRS/LRR/etc correctly...
Hmm... ｡oO( need to check how DRRS and LRR interact with ALPM pre/post
plane handling... )
Regards,
Ankit
+
+ if (window1 <= crtc_state->alpm_state.check_entry_lines)
+ 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
--
2.45.2
