| Message ID | 20250130184518.22353-3-ville.syrjala@linux.intel.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | drm/i915/vrr: Fix DSB+VRR usage for PTL+ | expand |
On 1/31/2025 12:15 AM, Ville Syrjala wrote: > From: Ville Syrjälä <ville.syrjala@linux.intel.com> > > Currently we trigger the push send first, then follow it with > a "wait for safe window". That approach no longer works on > PTL+ because triggering the push send immediately ends the safe > window. On prior hardware the safe window extended past the > push being sent (presumably all the way to the pipe's delayed > vblank). > > In order to deal with the new hardware behaviour we must reverse > the order of these two operations: first wait for safe window, > then trigger the push. > > The only slight danger with this approach is that if we mess up > the vblank evasion around the vmax decision boundary the push > might get postponed until after the next frame's vactive. But > assuming we don't mess up the vblank evasion this approach is > completely safe. > > As a slight bonus we can perform the push after we've done the > LUT writes as well, meaning we no longer have to worry about > extending the vblank delay to provide enough time for LUT > programming. Instead we will now depend on the vblank evasion > at vmax decision boundary to guarantee this. > > However vblank delay (or framestart delay) is still the only > way to provide extra time for the LUT programming in the > non-VRR use cases. Let's assume we don't need anything extra > for now, but eventually we should come up with some proper > estimates on how long the LUT programming can take and > configure the vblank delay accordingly for the non-VRR use > cases. > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com> Reviewed-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com> > --- > drivers/gpu/drm/i915/display/intel_color.c | 2 ++ > drivers/gpu/drm/i915/display/intel_display.c | 12 ++---------- > 2 files changed, 4 insertions(+), 10 deletions(-) > > diff --git a/drivers/gpu/drm/i915/display/intel_color.c b/drivers/gpu/drm/i915/display/intel_color.c > index 8400a97f7e43..be2691a80227 100644 > --- a/drivers/gpu/drm/i915/display/intel_color.c > +++ b/drivers/gpu/drm/i915/display/intel_color.c > @@ -29,6 +29,7 @@ > #include "intel_de.h" > #include "intel_display_types.h" > #include "intel_dsb.h" > +#include "intel_vrr.h" > > struct intel_color_funcs { > int (*color_check)(struct intel_atomic_state *state, > @@ -1987,6 +1988,7 @@ void intel_color_prepare_commit(struct intel_atomic_state *state, > > display->funcs.color->load_luts(crtc_state); > > + intel_vrr_send_push(crtc_state->dsb_color_vblank, crtc_state); > intel_dsb_wait_vblank_delay(state, crtc_state->dsb_color_vblank); > intel_dsb_interrupt(crtc_state->dsb_color_vblank); > > diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c > index 60867b5b03ec..69dbb0eb5ca1 100644 > --- a/drivers/gpu/drm/i915/display/intel_display.c > +++ b/drivers/gpu/drm/i915/display/intel_display.c > @@ -2630,14 +2630,6 @@ static int intel_crtc_vblank_delay(const struct intel_crtc_state *crtc_state) > if (intel_crtc_needs_wa_14015401596(crtc_state)) > vblank_delay = max(vblank_delay, 1); > > - /* > - * Add a minimal vblank delay to make sure the push > - * doesn't race with the "wait for safe window" used > - * for frame completion with DSB. > - */ > - if (intel_vrr_possible(crtc_state)) > - vblank_delay = max(vblank_delay, 1); > - > return vblank_delay; > } > > @@ -7735,10 +7727,10 @@ static void intel_atomic_dsb_finish(struct intel_atomic_state *state, > intel_crtc_planes_update_arm(new_crtc_state->dsb_commit, > state, crtc); > > - intel_vrr_send_push(new_crtc_state->dsb_commit, new_crtc_state); > - > if (!new_crtc_state->dsb_color_vblank) { > intel_dsb_wait_vblanks(new_crtc_state->dsb_commit, 1); > + > + intel_vrr_send_push(new_crtc_state->dsb_commit, new_crtc_state); > intel_dsb_wait_vblank_delay(state, new_crtc_state->dsb_commit); > intel_dsb_interrupt(new_crtc_state->dsb_commit); > }
diff --git a/drivers/gpu/drm/i915/display/intel_color.c b/drivers/gpu/drm/i915/display/intel_color.c index 8400a97f7e43..be2691a80227 100644 --- a/drivers/gpu/drm/i915/display/intel_color.c +++ b/drivers/gpu/drm/i915/display/intel_color.c @@ -29,6 +29,7 @@ #include "intel_de.h" #include "intel_display_types.h" #include "intel_dsb.h" +#include "intel_vrr.h" struct intel_color_funcs { int (*color_check)(struct intel_atomic_state *state, @@ -1987,6 +1988,7 @@ void intel_color_prepare_commit(struct intel_atomic_state *state, display->funcs.color->load_luts(crtc_state); + intel_vrr_send_push(crtc_state->dsb_color_vblank, crtc_state); intel_dsb_wait_vblank_delay(state, crtc_state->dsb_color_vblank); intel_dsb_interrupt(crtc_state->dsb_color_vblank); diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c index 60867b5b03ec..69dbb0eb5ca1 100644 --- a/drivers/gpu/drm/i915/display/intel_display.c +++ b/drivers/gpu/drm/i915/display/intel_display.c @@ -2630,14 +2630,6 @@ static int intel_crtc_vblank_delay(const struct intel_crtc_state *crtc_state) if (intel_crtc_needs_wa_14015401596(crtc_state)) vblank_delay = max(vblank_delay, 1); - /* - * Add a minimal vblank delay to make sure the push - * doesn't race with the "wait for safe window" used - * for frame completion with DSB. - */ - if (intel_vrr_possible(crtc_state)) - vblank_delay = max(vblank_delay, 1); - return vblank_delay; } @@ -7735,10 +7727,10 @@ static void intel_atomic_dsb_finish(struct intel_atomic_state *state, intel_crtc_planes_update_arm(new_crtc_state->dsb_commit, state, crtc); - intel_vrr_send_push(new_crtc_state->dsb_commit, new_crtc_state); - if (!new_crtc_state->dsb_color_vblank) { intel_dsb_wait_vblanks(new_crtc_state->dsb_commit, 1); + + intel_vrr_send_push(new_crtc_state->dsb_commit, new_crtc_state); intel_dsb_wait_vblank_delay(state, new_crtc_state->dsb_commit); intel_dsb_interrupt(new_crtc_state->dsb_commit); }