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@xxxxxxxxx> Reviewed-by: Nicholas Kazlauskas <nicholas.kazlauskas@xxxxxxx> 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 > _______________________________________________ amd-gfx mailing list amd-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/amd-gfx