On Tue, May 17, 2016 at 10:48:02AM -0400, Robert Foss wrote: > > > On 2016-05-17 06:32 AM, Marius Vlad wrote: > >On Mon, May 16, 2016 at 09:38:32AM -0400, [email protected] wrote: > >>From: Robert Foss <[email protected]> > >> > >>Change __wait_for_vblank() to use kmstest_get_vbl_flag() helper function. > >> > >>Signed-off-by: Robert Foss <[email protected]> > >>--- > >> tests/kms_flip.c | 8 ++++---- > >> 1 file changed, 4 insertions(+), 4 deletions(-) > >> > >>diff --git a/tests/kms_flip.c b/tests/kms_flip.c > >>index eda2fcc..0ecca07 100644 > >>--- a/tests/kms_flip.c > >>+++ b/tests/kms_flip.c > >>@@ -481,15 +481,15 @@ static int __wait_for_vblank(unsigned int flags, int > >>crtc_idx, > >> { > >> drmVBlank wait_vbl; > >> int ret; > >>- unsigned crtc_idx_mask; > >>+ uint32_t pipe_id_flag; > >> bool event = !(flags & TEST_VBLANK_BLOCK); > >> > >> memset(&wait_vbl, 0, sizeof(wait_vbl)); > >> > >>- crtc_idx_mask = crtc_idx << DRM_VBLANK_HIGH_CRTC_SHIFT; > >>- igt_assert(!(crtc_idx_mask & ~DRM_VBLANK_HIGH_CRTC_MASK)); > >If crtc_idx is 1 (pipe B), crtc_idx_mask = (1 << DRM_VBLANK_HIGH_CRTC_SHIFT) > >= 2. > >>+ pipe_id_flag = kmstest_get_vbl_flag(crtc_idx); > >>+ igt_assert(!(pipe_id_flag & ~DRM_VBLANK_HIGH_CRTC_MASK)); > >If crtc_idx is 1 (pipe B), kmstest_get_vbl_flag(crtc_idx) = > >DRM_VBLANK_SECONDARY = 0x20000000 > > > >And the assertion fails as !(0x20000000 & ~0x0000003e) = 0. > > > >Should kmstest_get_vbl_flag() always return pipe_id << 1? > > From re-reading the pipe id parsing code in drm_irq.c drm_wait_vblank() > it would seem to me that the assertion is incorrect, specifically for the > case of _DRM_VBLANK_SECONDARY. > > Supplying the flag _DRM_VBLANK_SECONDARY and setting the high crtc id field > to 1 fboth seem to be valid ways to communicate the same thing. Indeed. > > flags = vblwait->request.type & _DRM_VBLANK_FLAGS_MASK; > high_pipe = (vblwait->request.type &_DRM_VBLANK_HIGH_CRTC_MASK); > if (high_pipe) > pipe = high_pipe >> _DRM_VBLANK_HIGH_CRTC_SHIFT; > else > pipe = flags & _DRM_VBLANK_SECONDARY ? 1 : 0; > > > Maybe adding correct and more thorough assertion to kmstest_get_vbl_flag() > and removing the failing assertion is the way forward. Sounds reasonable to me, though I assume that the assertion had/has some point in determining that you have a valid crtc_idx. > > > > >> > >>- wait_vbl.request.type = crtc_idx_mask; > >>+ wait_vbl.request.type = pipe_id_flag; > >> if (flags & TEST_VBLANK_ABSOLUTE) > >> wait_vbl.request.type |= DRM_VBLANK_ABSOLUTE; > >> else > >>-- > >>2.7.4 > >> > _______________________________________________ > Intel-gfx mailing list > [email protected] > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
signature.asc
Description: Digital signature
_______________________________________________ Intel-gfx mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/intel-gfx
