From: "Kazlauskas, Nicholas" <Nicholas.Kazlauskas-5C7GfCeVMHo@public.gmane.org>
To: Mario Kleiner
<mario.kleiner.de-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>,
"amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org"
<amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org>
Cc: "dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org"
<dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org>
Subject: Re: [PATCH 4/4] drm/amd/display: Make pageflip event delivery compatible with VRR.
Date: Tue, 19 Mar 2019 13:05:34 +0000 [thread overview]
Message-ID: <e8dca2a6-49fd-1050-5021-9a8293e3da6c@amd.com> (raw)
In-Reply-To: <20190318171952.24302-5-mario.kleiner.de-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
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)
>
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
next prev parent reply other threads:[~2019-03-19 13:05 UTC|newest]
Thread overview: 18+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-03-18 17:19 AMD Freesync patches for proper vblank and pageflip timestamping in VRR mode Mario Kleiner
2019-03-18 17:19 ` [PATCH 1/4] drm/amd/display: Prevent vblank irq disable while VRR is active Mario Kleiner
[not found] ` <20190318171952.24302-2-mario.kleiner.de-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2019-03-18 17:29 ` Kazlauskas, Nicholas
2019-03-20 8:14 ` Mario Kleiner
[not found] ` <20190318171952.24302-1-mario.kleiner.de-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2019-03-18 17:19 ` [PATCH 2/4] drm/amd/display: Rework vrr flip throttling for late vblank irq Mario Kleiner
2019-03-18 17:59 ` Kazlauskas, Nicholas
2019-03-18 17:19 ` [PATCH 4/4] drm/amd/display: Make pageflip event delivery compatible with VRR Mario Kleiner
[not found] ` <20190318171952.24302-5-mario.kleiner.de-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2019-03-19 13:05 ` Kazlauskas, Nicholas [this message]
2019-03-18 17:19 ` [PATCH 3/4] drm/amd/display: In VRR mode, do DRM core vblank handling at end of vblank Mario Kleiner
[not found] ` <20190318171952.24302-4-mario.kleiner.de-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2019-03-19 7:17 ` Paul Menzel
2019-03-19 13:23 ` Kazlauskas, Nicholas
[not found] ` <8ef78169-1893-0aee-9cb1-cce4054ebbcc-5C7GfCeVMHo@public.gmane.org>
2019-03-19 13:32 ` Kazlauskas, Nicholas
2019-03-20 7:51 ` Mario Kleiner
2019-03-20 12:53 ` Kazlauskas, Nicholas
[not found] ` <b00cc1c5-f710-6c75-cdd2-c9ad30c633aa-5C7GfCeVMHo@public.gmane.org>
2019-03-21 9:39 ` Mario Kleiner
2019-03-21 15:38 ` Wentland, Harry
[not found] ` <929bad72-fb12-661e-d7e3-ac837d32c01c-5C7GfCeVMHo@public.gmane.org>
2019-03-21 15:48 ` Kazlauskas, Nicholas
2019-03-22 20:04 AMD Freesync patches v2 Mario Kleiner
[not found] ` <20190322200428.4008-1-mario.kleiner.de-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2019-03-22 20:04 ` [PATCH 4/4] drm/amd/display: Make pageflip event delivery compatible with VRR Mario Kleiner
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=e8dca2a6-49fd-1050-5021-9a8293e3da6c@amd.com \
--to=nicholas.kazlauskas-5c7gfcevmho@public.gmane.org \
--cc=amd-gfx-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org \
--cc=dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW@public.gmane.org \
--cc=mario.kleiner.de-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).