diff mbox series

[v2,1/8] drm/i915/dsb: Move the +1 usec adjustment into dsb_wait_usec()

Message ID 20250207223159.14132-2-ville.syrjala@linux.intel.com (mailing list archive)
State New
Headers show
Series drm/i915/vrr: Fix DSB+VRR usage for PTL+ | expand

Commit Message

Ville Syrjälä Feb. 7, 2025, 10:31 p.m. UTC 
From: Ville Syrjälä <ville.syrjala@linux.intel.com>

The "wait usec" DSB command doesn't quite seem to able to
guarantee that it always waits at least the specified
amount of usecs. Some of that could be just because it
supposedly just does some kind of dumb timestamp comparison
internally. But I also see cases where two hardware timestamps
sampled on each side of the "wait usec" command come out one
less than expected. So it looks like we always need at least a
+1 to guarantee that we never wait less than specified. Always
apply that adjustment in dsb_wait_usec().

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/display/intel_dsb.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

Comments

Nautiyal, Ankit K Feb. 11, 2025, 8:58 a.m. UTC | #1 
On 2/8/2025 4:01 AM, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> The "wait usec" DSB command doesn't quite seem to able to
> guarantee that it always waits at least the specified
> amount of usecs. Some of that could be just because it
> supposedly just does some kind of dumb timestamp comparison
> internally. But I also see cases where two hardware timestamps
> sampled on each side of the "wait usec" command come out one
> less than expected. So it looks like we always need at least a
> +1 to guarantee that we never wait less than specified. Always
> apply that adjustment in dsb_wait_usec().
>
> 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_dsb.c | 5 +++--
>   1 file changed, 3 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_dsb.c b/drivers/gpu/drm/i915/display/intel_dsb.c
> index 2f2812c23972..f8bd6fad0c87 100644
> --- a/drivers/gpu/drm/i915/display/intel_dsb.c
> +++ b/drivers/gpu/drm/i915/display/intel_dsb.c
> @@ -369,7 +369,8 @@ void intel_dsb_interrupt(struct intel_dsb *dsb)
>   
>   void intel_dsb_wait_usec(struct intel_dsb *dsb, int count)
>   {
> -	intel_dsb_emit(dsb, count,
> +	/* +1 to make sure we never wait less time than asked for */
> +	intel_dsb_emit(dsb, count + 1,
>   		       DSB_OPCODE_WAIT_USEC << DSB_OPCODE_SHIFT);
>   }
>   
> @@ -622,7 +623,7 @@ void intel_dsb_wait_vblank_delay(struct intel_atomic_state *state,
>   	const struct intel_crtc_state *crtc_state =
>   		intel_pre_commit_crtc_state(state, crtc);
>   	int usecs = intel_scanlines_to_usecs(&crtc_state->hw.adjusted_mode,
> -					     dsb_vblank_delay(state, crtc)) + 1;
> +					     dsb_vblank_delay(state, crtc));
>   
>   	intel_dsb_wait_usec(dsb, usecs);
>   }
diff mbox series

Patch

diff --git a/drivers/gpu/drm/i915/display/intel_dsb.c b/drivers/gpu/drm/i915/display/intel_dsb.c
index 2f2812c23972..f8bd6fad0c87 100644
--- a/drivers/gpu/drm/i915/display/intel_dsb.c
+++ b/drivers/gpu/drm/i915/display/intel_dsb.c
@@ -369,7 +369,8 @@  void intel_dsb_interrupt(struct intel_dsb *dsb)
 
 void intel_dsb_wait_usec(struct intel_dsb *dsb, int count)
 {
-	intel_dsb_emit(dsb, count,
+	/* +1 to make sure we never wait less time than asked for */
+	intel_dsb_emit(dsb, count + 1,
 		       DSB_OPCODE_WAIT_USEC << DSB_OPCODE_SHIFT);
 }
 
@@ -622,7 +623,7 @@  void intel_dsb_wait_vblank_delay(struct intel_atomic_state *state,
 	const struct intel_crtc_state *crtc_state =
 		intel_pre_commit_crtc_state(state, crtc);
 	int usecs = intel_scanlines_to_usecs(&crtc_state->hw.adjusted_mode,
-					     dsb_vblank_delay(state, crtc)) + 1;
+					     dsb_vblank_delay(state, crtc));
 
 	intel_dsb_wait_usec(dsb, usecs);
 }