diff mbox

[i-g-t,1/2] tests/kms_setmode: Request the intiial vbl count with RELATIVE instead of ABSOLUTE

Message ID 20171009163350.27086-1-ville.syrjala@linux.intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Ville Syrjälä Oct. 9, 2017, 4:33 p.m. UTC
From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Asking for the initial vblank count by specifying and absolute vblank count of 0
doesn't make much sense. Switch to a relative query instead.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 tests/kms_setmode.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Daniel Vetter Oct. 10, 2017, 9:16 a.m. UTC | #1
On Mon, Oct 09, 2017 at 07:33:49PM +0300, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Asking for the initial vblank count by specifying and absolute vblank count of 0
> doesn't make much sense. Switch to a relative query instead.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  tests/kms_setmode.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/tests/kms_setmode.c b/tests/kms_setmode.c
> index 206d360607bb..ed5d97442255 100644
> --- a/tests/kms_setmode.c
> +++ b/tests/kms_setmode.c
> @@ -431,7 +431,7 @@ static void check_timings(int crtc_idx, const drmModeModeInfo *kmode)
>  
>  	memset(&wait, 0, sizeof(wait));
>  	wait.request.type = kmstest_get_vbl_flag(crtc_idx);
> -	wait.request.type |= DRM_VBLANK_ABSOLUTE | DRM_VBLANK_NEXTONMISS;
> +	wait.request.type |= DRM_VBLANK_RELATIVE | DRM_VBLANK_NEXTONMISS;

Looking at drm_wait_vblank_is_query() in drm_vblank.c you also want to
drop NEXTONMISS. With that:

Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch> on both.

Cheers, Daniel
>  	do_or_die(drmWaitVBlank(drm_fd, &wait));
>  
>  	last_seq = wait.reply.sequence;
> -- 
> 2.13.5
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
Ville Syrjälä Oct. 10, 2017, 9:24 a.m. UTC | #2
On Tue, Oct 10, 2017 at 11:16:23AM +0200, Daniel Vetter wrote:
> On Mon, Oct 09, 2017 at 07:33:49PM +0300, Ville Syrjala wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > Asking for the initial vblank count by specifying and absolute vblank count of 0
> > doesn't make much sense. Switch to a relative query instead.
> > 
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> >  tests/kms_setmode.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/tests/kms_setmode.c b/tests/kms_setmode.c
> > index 206d360607bb..ed5d97442255 100644
> > --- a/tests/kms_setmode.c
> > +++ b/tests/kms_setmode.c
> > @@ -431,7 +431,7 @@ static void check_timings(int crtc_idx, const drmModeModeInfo *kmode)
> >  
> >  	memset(&wait, 0, sizeof(wait));
> >  	wait.request.type = kmstest_get_vbl_flag(crtc_idx);
> > -	wait.request.type |= DRM_VBLANK_ABSOLUTE | DRM_VBLANK_NEXTONMISS;
> > +	wait.request.type |= DRM_VBLANK_RELATIVE | DRM_VBLANK_NEXTONMISS;
> 
> Looking at drm_wait_vblank_is_query() in drm_vblank.c you also want to
> drop NEXTONMISS.

Hmm. I think we may want to line things up to a vblank boundary here
(but I'd have to re-read the code to confirm that). Without the
NEXTONMISS we would just query the current count and continue
immediately. Alternative I guess we could drop the NEXTONMISS but
instead wait for seq+1.

> With that:
> 
> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch> on both.
> 
> Cheers, Daniel
> >  	do_or_die(drmWaitVBlank(drm_fd, &wait));
> >  
> >  	last_seq = wait.reply.sequence;
> > -- 
> > 2.13.5
> > 
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> -- 
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch
Daniel Vetter Oct. 10, 2017, 12:25 p.m. UTC | #3
On Tue, Oct 10, 2017 at 12:24:06PM +0300, Ville Syrjälä wrote:
> On Tue, Oct 10, 2017 at 11:16:23AM +0200, Daniel Vetter wrote:
> > On Mon, Oct 09, 2017 at 07:33:49PM +0300, Ville Syrjala wrote:
> > > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > 
> > > Asking for the initial vblank count by specifying and absolute vblank count of 0
> > > doesn't make much sense. Switch to a relative query instead.
> > > 
> > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > ---
> > >  tests/kms_setmode.c | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > 
> > > diff --git a/tests/kms_setmode.c b/tests/kms_setmode.c
> > > index 206d360607bb..ed5d97442255 100644
> > > --- a/tests/kms_setmode.c
> > > +++ b/tests/kms_setmode.c
> > > @@ -431,7 +431,7 @@ static void check_timings(int crtc_idx, const drmModeModeInfo *kmode)
> > >  
> > >  	memset(&wait, 0, sizeof(wait));
> > >  	wait.request.type = kmstest_get_vbl_flag(crtc_idx);
> > > -	wait.request.type |= DRM_VBLANK_ABSOLUTE | DRM_VBLANK_NEXTONMISS;
> > > +	wait.request.type |= DRM_VBLANK_RELATIVE | DRM_VBLANK_NEXTONMISS;
> > 
> > Looking at drm_wait_vblank_is_query() in drm_vblank.c you also want to
> > drop NEXTONMISS.
> 
> Hmm. I think we may want to line things up to a vblank boundary here
> (but I'd have to re-read the code to confirm that). Without the
> NEXTONMISS we would just query the current count and continue
> immediately. Alternative I guess we could drop the NEXTONMISS but
> instead wait for seq+1.

NEXTONMISS means you don't actually query, but wait until that vblank
happened (as a blocking ioctl, you only get the event when asking for it).

I didn't really look into what the tests wants, just went with your
description of what you want to do ...
-Daniel


> 
> > With that:
> > 
> > Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch> on both.
> > 
> > Cheers, Daniel
> > >  	do_or_die(drmWaitVBlank(drm_fd, &wait));
> > >  
> > >  	last_seq = wait.reply.sequence;
> > > -- 
> > > 2.13.5
> > > 
> > > _______________________________________________
> > > Intel-gfx mailing list
> > > Intel-gfx@lists.freedesktop.org
> > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> > 
> > -- 
> > Daniel Vetter
> > Software Engineer, Intel Corporation
> > http://blog.ffwll.ch
> 
> -- 
> Ville Syrjälä
> Intel OTC
diff mbox

Patch

diff --git a/tests/kms_setmode.c b/tests/kms_setmode.c
index 206d360607bb..ed5d97442255 100644
--- a/tests/kms_setmode.c
+++ b/tests/kms_setmode.c
@@ -431,7 +431,7 @@  static void check_timings(int crtc_idx, const drmModeModeInfo *kmode)
 
 	memset(&wait, 0, sizeof(wait));
 	wait.request.type = kmstest_get_vbl_flag(crtc_idx);
-	wait.request.type |= DRM_VBLANK_ABSOLUTE | DRM_VBLANK_NEXTONMISS;
+	wait.request.type |= DRM_VBLANK_RELATIVE | DRM_VBLANK_NEXTONMISS;
 	do_or_die(drmWaitVBlank(drm_fd, &wait));
 
 	last_seq = wait.reply.sequence;