| Message ID | 20190318171952.24302-5-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 |
On 3/18/19 1:19 PM, Mario Kleiner wrote: > We want vblank counts and timestamps of flip completion as sent > in pageflip completion events to be consistent with the vblank > count and timestamp of the vblank of flip completion, like in non > VRR mode. > > In VRR mode, drm_update_vblank_count() - and thereby vblank > count and timestamp updates - must be delayed until after the > end of front-porch of each vblank, as it is only safe to > calculate vblank timestamps outside of the front-porch, when > we actually know when the vblank will end or has ended. > > The function drm_update_vblank_count() which updates timestamps > and counts gets called by drm_crtc_accurate_vblank_count() or by > drm_crtc_handle_vblank(). > > Therefore we must make sure that pageflip events for a completed > flip are only sent out after drm_crtc_accurate_vblank_count() or > drm_crtc_handle_vblank() is executed, after end of front-porch > for the vblank of flip completion. > > Two cases: > > a) Pageflip irq handler executes inside front-porch: > In this case we must defer sending pageflip events until > drm_crtc_handle_vblank() executes after end of front-porch, > and thereby calculates proper vblank count and timestamp. > Iow. the pflip irq handler must just arm a pageflip event > to be sent out by drm_crtc_handle_vblank() later on. > > b) Pageflip irq handler executes after end of front-porch, e.g., > after flip completion in back-porch or due to a massively > delayed handler invocation into the active scanout of the new > frame. In this case we can call drm_crtc_accurate_vblank_count() > to safely force calculation of a proper vblank count and > timestamp, and must send the pageflip completion event > ourselves from the pageflip irq handler. > > This is the same behaviour as needed for standard fixed refresh > rate mode. > > To decide from within pageflip handler if we are in case a) or b), > we check the current scanout position against the boundary of > front-porch. In non-VRR mode we just do what we did in the past. > > Signed-off-by: Mario Kleiner <mario.kleiner.de@gmail.com> Reviewed-by: Nicholas Kazlauskas <nicholas.kazlauskas@amd.com> This patch looks fine to me for the most part, but it's still pending on the other patches in the series. > --- > drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 68 ++++++++++++++++++----- > 1 file changed, 55 insertions(+), 13 deletions(-) > > 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 555d9e9f..499284d 100644 > --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c > +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c > @@ -263,6 +263,10 @@ static void dm_pflip_high_irq(void *interrupt_params) > struct common_irq_params *irq_params = interrupt_params; > struct amdgpu_device *adev = irq_params->adev; > unsigned long flags; > + struct drm_pending_vblank_event *e; > + struct dm_crtc_state *acrtc_state; > + uint32_t vpos, hpos, v_blank_start, v_blank_end; > + bool vrr_active; > > amdgpu_crtc = get_crtc_by_otg_inst(adev, irq_params->irq_src - IRQ_TYPE_PFLIP); > > @@ -285,18 +289,57 @@ static void dm_pflip_high_irq(void *interrupt_params) > return; > } > > - /* Update to correct count(s) if racing with vblank irq */ > - drm_crtc_accurate_vblank_count(&amdgpu_crtc->base); > + /* page flip completed. */ > + e = amdgpu_crtc->event; > + amdgpu_crtc->event = NULL; > > - /* wake up userspace */ > - if (amdgpu_crtc->event) { > - drm_crtc_send_vblank_event(&amdgpu_crtc->base, amdgpu_crtc->event); > + if (!e) > + WARN_ON(1); > > - /* page flip completed. clean up */ > - amdgpu_crtc->event = NULL; > + acrtc_state = to_dm_crtc_state(amdgpu_crtc->base.state); > + vrr_active = amdgpu_dm_vrr_active(acrtc_state); > + > + /* Fixed refresh rate, or VRR scanout position outside front-porch? */ > + if (!vrr_active || > + !dc_stream_get_scanoutpos(acrtc_state->stream, &v_blank_start, > + &v_blank_end, &hpos, &vpos) || > + (vpos < v_blank_start)) { > + /* Update to correct count and vblank timestamp if racing with > + * vblank irq. This also updates to the correct vblank timestamp > + * even in VRR mode, as scanout is past the front-porch atm. > + */ > + drm_crtc_accurate_vblank_count(&amdgpu_crtc->base); > > - } else > - WARN_ON(1); > + /* Wake up userspace by sending the pageflip event with proper > + * count and timestamp of vblank of flip completion. > + */ > + if (e) { > + drm_crtc_send_vblank_event(&amdgpu_crtc->base, e); > + > + /* Event sent, so done with vblank for this flip */ > + drm_crtc_vblank_put(&amdgpu_crtc->base); > + } > + } else if (e) { > + /* VRR active and inside front-porch: vblank count and > + * timestamp for pageflip event will only be up to date after > + * drm_crtc_handle_vblank() has been executed from late vblank > + * irq handler after start of back-porch (vline 0). We queue the > + * pageflip event for send-out by drm_crtc_handle_vblank() with > + * updated timestamp and count, once it runs after us. > + * > + * We need to open-code this instead of using the helper > + * drm_crtc_arm_vblank_event(), as that helper would > + * call drm_crtc_accurate_vblank_count(), which we must > + * not call in VRR mode while we are in front-porch! > + */ > + > + /* sequence will be replaced by real count during send-out. */ > + e->sequence = drm_crtc_vblank_count(&amdgpu_crtc->base); > + e->pipe = amdgpu_crtc->crtc_id; > + > + list_add_tail(&e->base.link, &adev->ddev->vblank_event_list); > + e = NULL; I guess drm_crtc_vblank_off handles sending any leftover events here if there are any if the IRQ gets disabled incorrectly. I wonder why we never just used this list in the first place for pageflip vblank event handling instead of the locking and NULL set. Though I suppose it's more useful to use this now for VRR vblank handling. > + } > > /* 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 > @@ -309,10 +352,9 @@ static void dm_pflip_high_irq(void *interrupt_params) > amdgpu_crtc->pflip_status = AMDGPU_FLIP_NONE; > spin_unlock_irqrestore(&adev->ddev->event_lock, flags); > > - DRM_DEBUG_DRIVER("%s - crtc :%d[%p], pflip_stat:AMDGPU_FLIP_NONE\n", > - __func__, amdgpu_crtc->crtc_id, amdgpu_crtc); > - > - drm_crtc_vblank_put(&amdgpu_crtc->base); > + DRM_DEBUG_DRIVER("crtc:%d[%p], pflip_stat:AMDGPU_FLIP_NONE, vrr[%d]-fp %d\n", > + amdgpu_crtc->crtc_id, amdgpu_crtc, > + vrr_active, (int) !e); The debug output for this is a little strange, but I guess it makes it makes as much sense as the original did. I'm glad the __func__ is gone at least. > } > > static void dm_vupdate_high_irq(void *interrupt_params) >
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 555d9e9f..499284d 100644 --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c @@ -263,6 +263,10 @@ static void dm_pflip_high_irq(void *interrupt_params) struct common_irq_params *irq_params = interrupt_params; struct amdgpu_device *adev = irq_params->adev; unsigned long flags; + struct drm_pending_vblank_event *e; + struct dm_crtc_state *acrtc_state; + uint32_t vpos, hpos, v_blank_start, v_blank_end; + bool vrr_active; amdgpu_crtc = get_crtc_by_otg_inst(adev, irq_params->irq_src - IRQ_TYPE_PFLIP); @@ -285,18 +289,57 @@ static void dm_pflip_high_irq(void *interrupt_params) return; } - /* Update to correct count(s) if racing with vblank irq */ - drm_crtc_accurate_vblank_count(&amdgpu_crtc->base); + /* page flip completed. */ + e = amdgpu_crtc->event; + amdgpu_crtc->event = NULL; - /* wake up userspace */ - if (amdgpu_crtc->event) { - drm_crtc_send_vblank_event(&amdgpu_crtc->base, amdgpu_crtc->event); + if (!e) + WARN_ON(1); - /* page flip completed. clean up */ - amdgpu_crtc->event = NULL; + acrtc_state = to_dm_crtc_state(amdgpu_crtc->base.state); + vrr_active = amdgpu_dm_vrr_active(acrtc_state); + + /* Fixed refresh rate, or VRR scanout position outside front-porch? */ + if (!vrr_active || + !dc_stream_get_scanoutpos(acrtc_state->stream, &v_blank_start, + &v_blank_end, &hpos, &vpos) || + (vpos < v_blank_start)) { + /* Update to correct count and vblank timestamp if racing with + * vblank irq. This also updates to the correct vblank timestamp + * even in VRR mode, as scanout is past the front-porch atm. + */ + drm_crtc_accurate_vblank_count(&amdgpu_crtc->base); - } else - WARN_ON(1); + /* Wake up userspace by sending the pageflip event with proper + * count and timestamp of vblank of flip completion. + */ + if (e) { + drm_crtc_send_vblank_event(&amdgpu_crtc->base, e); + + /* Event sent, so done with vblank for this flip */ + drm_crtc_vblank_put(&amdgpu_crtc->base); + } + } else if (e) { + /* VRR active and inside front-porch: vblank count and + * timestamp for pageflip event will only be up to date after + * drm_crtc_handle_vblank() has been executed from late vblank + * irq handler after start of back-porch (vline 0). We queue the + * pageflip event for send-out by drm_crtc_handle_vblank() with + * updated timestamp and count, once it runs after us. + * + * We need to open-code this instead of using the helper + * drm_crtc_arm_vblank_event(), as that helper would + * call drm_crtc_accurate_vblank_count(), which we must + * not call in VRR mode while we are in front-porch! + */ + + /* sequence will be replaced by real count during send-out. */ + e->sequence = drm_crtc_vblank_count(&amdgpu_crtc->base); + e->pipe = amdgpu_crtc->crtc_id; + + list_add_tail(&e->base.link, &adev->ddev->vblank_event_list); + e = NULL; + } /* 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 @@ -309,10 +352,9 @@ static void dm_pflip_high_irq(void *interrupt_params) amdgpu_crtc->pflip_status = AMDGPU_FLIP_NONE; spin_unlock_irqrestore(&adev->ddev->event_lock, flags); - DRM_DEBUG_DRIVER("%s - crtc :%d[%p], pflip_stat:AMDGPU_FLIP_NONE\n", - __func__, amdgpu_crtc->crtc_id, amdgpu_crtc); - - drm_crtc_vblank_put(&amdgpu_crtc->base); + DRM_DEBUG_DRIVER("crtc:%d[%p], pflip_stat:AMDGPU_FLIP_NONE, vrr[%d]-fp %d\n", + amdgpu_crtc->crtc_id, amdgpu_crtc, + vrr_active, (int) !e); } static void dm_vupdate_high_irq(void *interrupt_params)
We want vblank counts and timestamps of flip completion as sent in pageflip completion events to be consistent with the vblank count and timestamp of the vblank of flip completion, like in non VRR mode. In VRR mode, drm_update_vblank_count() - and thereby vblank count and timestamp updates - must be delayed until after the end of front-porch of each vblank, as it is only safe to calculate vblank timestamps outside of the front-porch, when we actually know when the vblank will end or has ended. The function drm_update_vblank_count() which updates timestamps and counts gets called by drm_crtc_accurate_vblank_count() or by drm_crtc_handle_vblank(). Therefore we must make sure that pageflip events for a completed flip are only sent out after drm_crtc_accurate_vblank_count() or drm_crtc_handle_vblank() is executed, after end of front-porch for the vblank of flip completion. Two cases: a) Pageflip irq handler executes inside front-porch: In this case we must defer sending pageflip events until drm_crtc_handle_vblank() executes after end of front-porch, and thereby calculates proper vblank count and timestamp. Iow. the pflip irq handler must just arm a pageflip event to be sent out by drm_crtc_handle_vblank() later on. b) Pageflip irq handler executes after end of front-porch, e.g., after flip completion in back-porch or due to a massively delayed handler invocation into the active scanout of the new frame. In this case we can call drm_crtc_accurate_vblank_count() to safely force calculation of a proper vblank count and timestamp, and must send the pageflip completion event ourselves from the pageflip irq handler. This is the same behaviour as needed for standard fixed refresh rate mode. To decide from within pageflip handler if we are in case a) or b), we check the current scanout position against the boundary of front-porch. In non-VRR mode we just do what we did in the past. Signed-off-by: Mario Kleiner <mario.kleiner.de@gmail.com> --- drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 68 ++++++++++++++++++----- 1 file changed, 55 insertions(+), 13 deletions(-)