diff mbox series

[2/4] drm/amd/display: Rework vrr flip throttling for late vblank irq.

Message ID 20190318171952.24302-3-mario.kleiner.de@gmail.com (mailing list archive)
State New, archived
Headers show
Series [1/4] drm/amd/display: Prevent vblank irq disable while VRR is active. | expand

Commit Message

Mario Kleiner March 18, 2019, 5:19 p.m. UTC
For throttling to work correctly, we always need a baseline vblank
count last_flip_vblank that increments at start of front-porch.

This is the case for drm_crtc_vblank_count() in non-VRR mode, where
the vblank irq fires at start of front-porch and triggers DRM core
vblank handling, but it is no longer the case in VRR mode, where
core vblank handling is done later, after end of front-porch.

Therefore drm_crtc_vblank_count() is no longer useful for this.
We also can't use drm_crtc_accurate_vblank_count(), as that would
screw up vblank timestamps in VRR mode when called in front-porch.

To solve this, use the cooked hardware vblank counter returned by
amdgpu_get_vblank_counter_kms() instead, as that one is cooked to
always increment at start of front-porch, independent of when
vblank related irq's fire.

This patch allows vblank irq handling to happen anywhere within
vblank of even after it, without a negative impact on flip
throttling, so followup patches can shift the vblank core
handling trigger point wherever they need it.

Signed-off-by: Mario Kleiner <mario.kleiner.de@gmail.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h          |  2 +-
 drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 23 +++++++++++++----------
 2 files changed, 14 insertions(+), 11 deletions(-)

Comments

Kazlauskas, Nicholas March 18, 2019, 5:59 p.m. UTC | #1
On 3/18/19 1:19 PM, Mario Kleiner wrote:
> For throttling to work correctly, we always need a baseline vblank
> count last_flip_vblank that increments at start of front-porch.
> 
> This is the case for drm_crtc_vblank_count() in non-VRR mode, where
> the vblank irq fires at start of front-porch and triggers DRM core
> vblank handling, but it is no longer the case in VRR mode, where
> core vblank handling is done later, after end of front-porch.
> 
> Therefore drm_crtc_vblank_count() is no longer useful for this.
> We also can't use drm_crtc_accurate_vblank_count(), as that would
> screw up vblank timestamps in VRR mode when called in front-porch.
> 
> To solve this, use the cooked hardware vblank counter returned by
> amdgpu_get_vblank_counter_kms() instead, as that one is cooked to
> always increment at start of front-porch, independent of when
> vblank related irq's fire.
> 
> This patch allows vblank irq handling to happen anywhere within
> vblank of even after it, without a negative impact on flip
> throttling, so followup patches can shift the vblank core
> handling trigger point wherever they need it.
> 
> Signed-off-by: Mario Kleiner <mario.kleiner.de@gmail.com>

Reviewed-by: Nicholas Kazlauskas <nicholas.kazlauskas@amd.com>

Functionally drm_crtc_accurate_vblank_count is the same as 
amdgpu_get_vblank_counter_kms for querying purposes, so this works.

It's a nice cleanup for commit planes too since we no longer grab the 
DRM vblank count only to immediately subtract it off again.

> ---
>   drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h          |  2 +-
>   drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 23 +++++++++++++----------
>   2 files changed, 14 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h
> index 889e443..add238f 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h
> @@ -406,7 +406,7 @@ struct amdgpu_crtc {
>   	struct amdgpu_flip_work *pflip_works;
>   	enum amdgpu_flip_status pflip_status;
>   	int deferred_flip_completion;
> -	u64 last_flip_vblank;
> +	u32 last_flip_vblank;
>   	/* pll sharing */
>   	struct amdgpu_atom_ss ss;
>   	bool ss_enabled;
> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> index c1c3815..85e4f87 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> @@ -286,7 +286,7 @@ static void dm_pflip_high_irq(void *interrupt_params)
>   	}
>   
>   	/* Update to correct count(s) if racing with vblank irq */
> -	amdgpu_crtc->last_flip_vblank = drm_crtc_accurate_vblank_count(&amdgpu_crtc->base);
> +	drm_crtc_accurate_vblank_count(&amdgpu_crtc->base);
>   
>   	/* wake up userspace */
>   	if (amdgpu_crtc->event) {
> @@ -298,6 +298,14 @@ static void dm_pflip_high_irq(void *interrupt_params)
>   	} else
>   		WARN_ON(1);
>   
> +	/* Keep track of vblank of this flip for flip throttling. We use the
> +	 * cooked hw counter, as that one incremented at start of this vblank
> +	 * of pageflip completion, so last_flip_vblank is the forbidden count
> +	 * for queueing new pageflips if vsync + VRR is enabled.
> +	 */
> +	amdgpu_crtc->last_flip_vblank = amdgpu_get_vblank_counter_kms(adev->ddev,
> +							amdgpu_crtc->crtc_id);
> +
>   	amdgpu_crtc->pflip_status = AMDGPU_FLIP_NONE;
>   	spin_unlock_irqrestore(&adev->ddev->event_lock, flags);
>   
> @@ -4769,9 +4777,8 @@ static void amdgpu_dm_commit_planes(struct drm_atomic_state *state,
>   	unsigned long flags;
>   	struct amdgpu_bo *abo;
>   	uint64_t tiling_flags;
> -	uint32_t target, target_vblank;
> -	uint64_t last_flip_vblank;
> -	bool vrr_active = acrtc_state->freesync_config.state == VRR_STATE_ACTIVE_VARIABLE;
> +	uint32_t target_vblank, last_flip_vblank;
> +	bool vrr_active = amdgpu_dm_vrr_active(acrtc_state);
>   	bool pflip_present = false;
>   
>   	struct {
> @@ -4918,7 +4925,7 @@ static void amdgpu_dm_commit_planes(struct drm_atomic_state *state,
>   			 * clients using the GLX_OML_sync_control extension or
>   			 * DRI3/Present extension with defined target_msc.
>   			 */
> -			last_flip_vblank = drm_crtc_vblank_count(pcrtc);
> +			last_flip_vblank = amdgpu_get_vblank_counter_kms(dm->ddev, acrtc_attach->crtc_id);
>   		}
>   		else {
>   			/* For variable refresh rate mode only:
> @@ -4934,11 +4941,7 @@ static void amdgpu_dm_commit_planes(struct drm_atomic_state *state,
>   			spin_unlock_irqrestore(&pcrtc->dev->event_lock, flags);
>   		}
>   
> -		target = (uint32_t)last_flip_vblank + wait_for_vblank;
> -
> -		/* Prepare wait for target vblank early - before the fence-waits */
> -		target_vblank = target - (uint32_t)drm_crtc_vblank_count(pcrtc) +
> -				amdgpu_get_vblank_counter_kms(pcrtc->dev, acrtc_attach->crtc_id);
> +		target_vblank = last_flip_vblank + wait_for_vblank;
>   
>   		/*
>   		 * Wait until we're out of the vertical blank period before the one
>
diff mbox series

Patch

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h
index 889e443..add238f 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_mode.h
@@ -406,7 +406,7 @@  struct amdgpu_crtc {
 	struct amdgpu_flip_work *pflip_works;
 	enum amdgpu_flip_status pflip_status;
 	int deferred_flip_completion;
-	u64 last_flip_vblank;
+	u32 last_flip_vblank;
 	/* pll sharing */
 	struct amdgpu_atom_ss ss;
 	bool ss_enabled;
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
index c1c3815..85e4f87 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -286,7 +286,7 @@  static void dm_pflip_high_irq(void *interrupt_params)
 	}
 
 	/* Update to correct count(s) if racing with vblank irq */
-	amdgpu_crtc->last_flip_vblank = drm_crtc_accurate_vblank_count(&amdgpu_crtc->base);
+	drm_crtc_accurate_vblank_count(&amdgpu_crtc->base);
 
 	/* wake up userspace */
 	if (amdgpu_crtc->event) {
@@ -298,6 +298,14 @@  static void dm_pflip_high_irq(void *interrupt_params)
 	} else
 		WARN_ON(1);
 
+	/* Keep track of vblank of this flip for flip throttling. We use the
+	 * cooked hw counter, as that one incremented at start of this vblank
+	 * of pageflip completion, so last_flip_vblank is the forbidden count
+	 * for queueing new pageflips if vsync + VRR is enabled.
+	 */
+	amdgpu_crtc->last_flip_vblank = amdgpu_get_vblank_counter_kms(adev->ddev,
+							amdgpu_crtc->crtc_id);
+
 	amdgpu_crtc->pflip_status = AMDGPU_FLIP_NONE;
 	spin_unlock_irqrestore(&adev->ddev->event_lock, flags);
 
@@ -4769,9 +4777,8 @@  static void amdgpu_dm_commit_planes(struct drm_atomic_state *state,
 	unsigned long flags;
 	struct amdgpu_bo *abo;
 	uint64_t tiling_flags;
-	uint32_t target, target_vblank;
-	uint64_t last_flip_vblank;
-	bool vrr_active = acrtc_state->freesync_config.state == VRR_STATE_ACTIVE_VARIABLE;
+	uint32_t target_vblank, last_flip_vblank;
+	bool vrr_active = amdgpu_dm_vrr_active(acrtc_state);
 	bool pflip_present = false;
 
 	struct {
@@ -4918,7 +4925,7 @@  static void amdgpu_dm_commit_planes(struct drm_atomic_state *state,
 			 * clients using the GLX_OML_sync_control extension or
 			 * DRI3/Present extension with defined target_msc.
 			 */
-			last_flip_vblank = drm_crtc_vblank_count(pcrtc);
+			last_flip_vblank = amdgpu_get_vblank_counter_kms(dm->ddev, acrtc_attach->crtc_id);
 		}
 		else {
 			/* For variable refresh rate mode only:
@@ -4934,11 +4941,7 @@  static void amdgpu_dm_commit_planes(struct drm_atomic_state *state,
 			spin_unlock_irqrestore(&pcrtc->dev->event_lock, flags);
 		}
 
-		target = (uint32_t)last_flip_vblank + wait_for_vblank;
-
-		/* Prepare wait for target vblank early - before the fence-waits */
-		target_vblank = target - (uint32_t)drm_crtc_vblank_count(pcrtc) +
-				amdgpu_get_vblank_counter_kms(pcrtc->dev, acrtc_attach->crtc_id);
+		target_vblank = last_flip_vblank + wait_for_vblank;
 
 		/*
 		 * Wait until we're out of the vertical blank period before the one