[PATCH] drm: Fix timestamp docs for variable refresh properties.

[PATCH] drm: Fix timestamp docs for variable refresh properties.

[Mario Kleiner]

As discussed with Nicholas and Daniel Vetter (patchwork
link to discussion below), the VRR timestamping behaviour
produced utterly useless and bogus vblank/pageflip
timestamps. We have found a way to fix this and provide
sane behaviour.

As of Linux 5.2, the amdgpu driver will be able to
provide exactly the same vblank / pageflip timestamp
semantic in variable refresh rate mode as in standard
fixed refresh rate mode. This is achieved by deferring
core vblank handling (drm_crtc_handle_vblank()) until
the end of front porch, and also defer the sending of
pageflip completion events until end of front porch,
when we can safely compute correct pageflip/vblank
timestamps.

The same approach will be possible for other VRR
capable kms drivers, so we can actually have sane
and useful timestamps in VRR mode.

This patch removes the section of the docs that
describes the broken timestamp behaviour present
in Linux 5.0/5.1.

Fixes: ab7a664f7a2d ("drm: Document variable refresh properties")
Link: https://patchwork.freedesktop.org/patch/285333/
Signed-off-by: Mario Kleiner <mario.kleiner.de@gmail.com>
---
 drivers/gpu/drm/drm_connector.c | 6 ------
 1 file changed, 6 deletions(-)

diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
index 2355124849db..b34c3d38bf15 100644
--- a/drivers/gpu/drm/drm_connector.c
+++ b/drivers/gpu/drm/drm_connector.c
@@ -1416,12 +1416,6 @@ EXPORT_SYMBOL(drm_mode_create_scaling_mode_property);
  *
  *	The driver may place further restrictions within these minimum
  *	and maximum bounds.
- *
- *	The semantics for the vertical blank timestamp differ when
- *	variable refresh rate is active. The vertical blank timestamp
- *	is defined to be an estimate using the current mode's fixed
- *	refresh rate timings. The semantics for the page-flip event
- *	timestamp remain the same.
  */
 
 /**
-- 
2.20.1

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm: Fix timestamp docs for variable refresh properties.

[Daniel Vetter]

On Thu, Apr 18, 2019 at 08:01:57AM +0200, Mario Kleiner wrote:
> As discussed with Nicholas and Daniel Vetter (patchwork
> link to discussion below), the VRR timestamping behaviour
> produced utterly useless and bogus vblank/pageflip
> timestamps. We have found a way to fix this and provide
> sane behaviour.
> 
> As of Linux 5.2, the amdgpu driver will be able to
> provide exactly the same vblank / pageflip timestamp
> semantic in variable refresh rate mode as in standard
> fixed refresh rate mode. This is achieved by deferring
> core vblank handling (drm_crtc_handle_vblank()) until
> the end of front porch, and also defer the sending of
> pageflip completion events until end of front porch,
> when we can safely compute correct pageflip/vblank
> timestamps.
> 
> The same approach will be possible for other VRR
> capable kms drivers, so we can actually have sane
> and useful timestamps in VRR mode.
> 
> This patch removes the section of the docs that
> describes the broken timestamp behaviour present
> in Linux 5.0/5.1.
> 
> Fixes: ab7a664f7a2d ("drm: Document variable refresh properties")
> Link: https://patchwork.freedesktop.org/patch/285333/
> Signed-off-by: Mario Kleiner <mario.kleiner.de@gmail.com>
> ---
>  drivers/gpu/drm/drm_connector.c | 6 ------
>  1 file changed, 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
> index 2355124849db..b34c3d38bf15 100644
> --- a/drivers/gpu/drm/drm_connector.c
> +++ b/drivers/gpu/drm/drm_connector.c
> @@ -1416,12 +1416,6 @@ EXPORT_SYMBOL(drm_mode_create_scaling_mode_property);
>   *
>   *	The driver may place further restrictions within these minimum
>   *	and maximum bounds.
> - *
> - *	The semantics for the vertical blank timestamp differ when
> - *	variable refresh rate is active. The vertical blank timestamp
> - *	is defined to be an estimate using the current mode's fixed
> - *	refresh rate timings. The semantics for the page-flip event
> - *	timestamp remain the same.

Yay! Thanks for all your work to polish VRR in linux.

Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch>
>   */
>  
>  /**
> -- 
> 2.20.1
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm: Fix timestamp docs for variable refresh properties.

[Kazlauskas, Nicholas]

On 4/18/19 2:01 AM, Mario Kleiner wrote:
> As discussed with Nicholas and Daniel Vetter (patchwork
> link to discussion below), the VRR timestamping behaviour
> produced utterly useless and bogus vblank/pageflip
> timestamps. We have found a way to fix this and provide
> sane behaviour.
> 
> As of Linux 5.2, the amdgpu driver will be able to
> provide exactly the same vblank / pageflip timestamp
> semantic in variable refresh rate mode as in standard
> fixed refresh rate mode. This is achieved by deferring
> core vblank handling (drm_crtc_handle_vblank()) until
> the end of front porch, and also defer the sending of
> pageflip completion events until end of front porch,
> when we can safely compute correct pageflip/vblank
> timestamps.
> 
> The same approach will be possible for other VRR
> capable kms drivers, so we can actually have sane
> and useful timestamps in VRR mode.
> 
> This patch removes the section of the docs that
> describes the broken timestamp behaviour present
> in Linux 5.0/5.1.
> 
> Fixes: ab7a664f7a2d ("drm: Document variable refresh properties")
> Link: https://patchwork.freedesktop.org/patch/285333/
> Signed-off-by: Mario Kleiner <mario.kleiner.de@gmail.com>

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

Someone else can feel free to push this as I don't have commit rights 
for DRM.

Thanks!

Nicholas Kazlauskas

> ---
>   drivers/gpu/drm/drm_connector.c | 6 ------
>   1 file changed, 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
> index 2355124849db..b34c3d38bf15 100644
> --- a/drivers/gpu/drm/drm_connector.c
> +++ b/drivers/gpu/drm/drm_connector.c
> @@ -1416,12 +1416,6 @@ EXPORT_SYMBOL(drm_mode_create_scaling_mode_property);
>    *
>    *	The driver may place further restrictions within these minimum
>    *	and maximum bounds.
> - *
> - *	The semantics for the vertical blank timestamp differ when
> - *	variable refresh rate is active. The vertical blank timestamp
> - *	is defined to be an estimate using the current mode's fixed
> - *	refresh rate timings. The semantics for the page-flip event
> - *	timestamp remain the same.
>    */
>   
>   /**
> 

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm: Fix timestamp docs for variable refresh properties.

[Mario Kleiner]

Nag nag: The below documentation patch, acked-by Daniel and r-b'd by
Nicholas seems to not have made it into drm-next yet?

thanks,
-mario

On Thu, Apr 18, 2019 at 2:45 PM Kazlauskas, Nicholas
<Nicholas.Kazlauskas@amd.com> wrote:
>
> On 4/18/19 2:01 AM, Mario Kleiner wrote:
> > As discussed with Nicholas and Daniel Vetter (patchwork
> > link to discussion below), the VRR timestamping behaviour
> > produced utterly useless and bogus vblank/pageflip
> > timestamps. We have found a way to fix this and provide
> > sane behaviour.
> >
> > As of Linux 5.2, the amdgpu driver will be able to
> > provide exactly the same vblank / pageflip timestamp
> > semantic in variable refresh rate mode as in standard
> > fixed refresh rate mode. This is achieved by deferring
> > core vblank handling (drm_crtc_handle_vblank()) until
> > the end of front porch, and also defer the sending of
> > pageflip completion events until end of front porch,
> > when we can safely compute correct pageflip/vblank
> > timestamps.
> >
> > The same approach will be possible for other VRR
> > capable kms drivers, so we can actually have sane
> > and useful timestamps in VRR mode.
> >
> > This patch removes the section of the docs that
> > describes the broken timestamp behaviour present
> > in Linux 5.0/5.1.
> >
> > Fixes: ab7a664f7a2d ("drm: Document variable refresh properties")
> > Link: https://patchwork.freedesktop.org/patch/285333/
> > Signed-off-by: Mario Kleiner <mario.kleiner.de@gmail.com>
>
> Reviewed-by: Nicholas Kazlauskas <nicholas.kazlauskas@amd.com>
>
> Someone else can feel free to push this as I don't have commit rights
> for DRM.
>
> Thanks!
>
> Nicholas Kazlauskas
>
> > ---
> >   drivers/gpu/drm/drm_connector.c | 6 ------
> >   1 file changed, 6 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
> > index 2355124849db..b34c3d38bf15 100644
> > --- a/drivers/gpu/drm/drm_connector.c
> > +++ b/drivers/gpu/drm/drm_connector.c
> > @@ -1416,12 +1416,6 @@ EXPORT_SYMBOL(drm_mode_create_scaling_mode_property);
> >    *
> >    *  The driver may place further restrictions within these minimum
> >    *  and maximum bounds.
> > - *
> > - *   The semantics for the vertical blank timestamp differ when
> > - *   variable refresh rate is active. The vertical blank timestamp
> > - *   is defined to be an estimate using the current mode's fixed
> > - *   refresh rate timings. The semantics for the page-flip event
> > - *   timestamp remain the same.
> >    */
> >
> >   /**
> >
>
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH] drm: Fix timestamp docs for variable refresh properties.

[Alex Deucher]

On Tue, May 7, 2019 at 1:15 AM Mario Kleiner <mario.kleiner.de@gmail.com> wrote:
>
> Nag nag: The below documentation patch, acked-by Daniel and r-b'd by
> Nicholas seems to not have made it into drm-next yet?

Pushed to drm-misc-next-fixes

Thanks!

Alex

>
> thanks,
> -mario
>
> On Thu, Apr 18, 2019 at 2:45 PM Kazlauskas, Nicholas
> <Nicholas.Kazlauskas@amd.com> wrote:
> >
> > On 4/18/19 2:01 AM, Mario Kleiner wrote:
> > > As discussed with Nicholas and Daniel Vetter (patchwork
> > > link to discussion below), the VRR timestamping behaviour
> > > produced utterly useless and bogus vblank/pageflip
> > > timestamps. We have found a way to fix this and provide
> > > sane behaviour.
> > >
> > > As of Linux 5.2, the amdgpu driver will be able to
> > > provide exactly the same vblank / pageflip timestamp
> > > semantic in variable refresh rate mode as in standard
> > > fixed refresh rate mode. This is achieved by deferring
> > > core vblank handling (drm_crtc_handle_vblank()) until
> > > the end of front porch, and also defer the sending of
> > > pageflip completion events until end of front porch,
> > > when we can safely compute correct pageflip/vblank
> > > timestamps.
> > >
> > > The same approach will be possible for other VRR
> > > capable kms drivers, so we can actually have sane
> > > and useful timestamps in VRR mode.
> > >
> > > This patch removes the section of the docs that
> > > describes the broken timestamp behaviour present
> > > in Linux 5.0/5.1.
> > >
> > > Fixes: ab7a664f7a2d ("drm: Document variable refresh properties")
> > > Link: https://patchwork.freedesktop.org/patch/285333/
> > > Signed-off-by: Mario Kleiner <mario.kleiner.de@gmail.com>
> >
> > Reviewed-by: Nicholas Kazlauskas <nicholas.kazlauskas@amd.com>
> >
> > Someone else can feel free to push this as I don't have commit rights
> > for DRM.
> >
> > Thanks!
> >
> > Nicholas Kazlauskas
> >
> > > ---
> > >   drivers/gpu/drm/drm_connector.c | 6 ------
> > >   1 file changed, 6 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
> > > index 2355124849db..b34c3d38bf15 100644
> > > --- a/drivers/gpu/drm/drm_connector.c
> > > +++ b/drivers/gpu/drm/drm_connector.c
> > > @@ -1416,12 +1416,6 @@ EXPORT_SYMBOL(drm_mode_create_scaling_mode_property);
> > >    *
> > >    *  The driver may place further restrictions within these minimum
> > >    *  and maximum bounds.
> > > - *
> > > - *   The semantics for the vertical blank timestamp differ when
> > > - *   variable refresh rate is active. The vertical blank timestamp
> > > - *   is defined to be an estimate using the current mode's fixed
> > > - *   refresh rate timings. The semantics for the page-flip event
> > > - *   timestamp remain the same.
> > >    */
> > >
> > >   /**
> > >
> >
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel