[igt-dev] [PATCH v3] tests/kms_vblank: Turn on hardware before testing invalid vblank.

[igt-dev] [PATCH v3] tests/kms_vblank: Turn on hardware before testing invalid vblank.

[Mark Yacoub]

From: Mark Yacoub <markyacoub@google.com>

[Why]
Before any hardware is on, the vblank is off and its ref counter is in
an initialized state as each driver handles toggling it differently.
Ioctl DRM_IOCTL_WAIT_VBLANK could return 0 such as on i915, or an invalid
integer that doesn't mean much, such as on Zork running Kernel 5.4 due
to the kernel workaround that increments the vblank ref count to prevent
a get from enabling the interrupt.

[How]
For invalid_subtest(), active the CRTCs to turn the hardware on so
DRM_IOCTL_WAIT_VBLANK returns something meaningful.

Changes since v3:
Turn on only 1 output instead of all.

Changes since v2:
1. Updated Signed-off-by.

Changes since v1:
1. Update Commit message
2. Rename variable p to pipe_number

Signed-off-by: Mark Yacoub <markyacoub@chromium.org>
---
 tests/kms_vblank.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/tests/kms_vblank.c b/tests/kms_vblank.c
index 93b01eba..19284257 100644
--- a/tests/kms_vblank.c
+++ b/tests/kms_vblank.c
@@ -475,8 +475,13 @@ static void invalid_subtest(data_t *data, int fd)
 {
 	union drm_wait_vblank vbl;
 	unsigned long valid_flags;
+	igt_display_t* display = &data->display;
+	igt_output_t* output;
 
-	igt_display_require_output_on_pipe(&data->display, 0);
+	igt_display_require_output_on_pipe(display, 0);
+	igt_require(display->n_outputs);
+	output = &display->outputs[0];
+	prepare_crtc(data, fd, output);
 
 	/* First check all is well with a simple query */
 	memset(&vbl, 0, sizeof(vbl));
@@ -511,6 +516,8 @@ static void invalid_subtest(data_t *data, int fd)
 	vbl.request.type |= _DRM_VBLANK_SECONDARY;
 	vbl.request.type |= _DRM_VBLANK_FLAGS_MASK;
 	igt_assert_eq(wait_vblank(fd, &vbl), -EINVAL);
+
+	cleanup_crtc(data, fd, output);
 }
 
 igt_main
-- 
2.32.0.288.g62a8d224e6-goog

_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [igt-dev] [PATCH v3] tests/kms_vblank: Turn on hardware before testing invalid vblank.

[Mark Yacoub]

Thanks!
I was trying to find a neater way to grab the output that I wanna turn
on. Let me know if this is good or there is a better solution.
I pushed the change:
https://patchwork.freedesktop.org/patch/440342/?series=90382&rev=6

On Mon, Jun 21, 2021 at 6:53 AM Petri Latvala <petri.latvala@intel.com> wrote:
>
> On Thu, Jun 17, 2021 at 03:20:32PM +0000, Mark Yacoub wrote:
> > From: Mark Yacoub <markyacoub@google.com>
> >
> > [Why]
> > Before any hardware is on, the vblank is off and its ref counter is in
> > an initialized state as each driver handles toggling it differently.
> > Ioctl DRM_IOCTL_WAIT_VBLANK could return 0 such as on i915, or an invalid
> > integer that doesn't mean much, such as on Zork running Kernel 5.4 due
> > to the kernel workaround that increments the vblank ref count to prevent
> > a get from enabling the interrupt.
> >
> > [How]
> > For invalid_subtest(), active the CRTCs to turn the hardware on so
> > DRM_IOCTL_WAIT_VBLANK returns something meaningful.
> >
> > Changes since v2:
> > 1. Updated Signed-off-by.
> >
> > Changes since v1:
> > 1. Update Commit message
> > 2. Rename variable p to pipe_number
> >
> > Signed-off-by: Mark Yacoub <markyacoub@chromium.org>
> > ---
> >  tests/kms_vblank.c | 11 ++++++++++-
> >  1 file changed, 10 insertions(+), 1 deletion(-)
> >
> > diff --git a/tests/kms_vblank.c b/tests/kms_vblank.c
> > index 93b01eba..988794c0 100644
> > --- a/tests/kms_vblank.c
> > +++ b/tests/kms_vblank.c
> > @@ -475,8 +475,14 @@ static void invalid_subtest(data_t *data, int fd)
> >  {
> >       union drm_wait_vblank vbl;
> >       unsigned long valid_flags;
> > +     igt_display_t* display = &data->display;
> > +     enum pipe pipe_number = 0;
> > +     igt_output_t* output;
> >
> > -     igt_display_require_output_on_pipe(&data->display, 0);
> > +     igt_display_require_output_on_pipe(display, pipe_number);
> > +     data->pipe = pipe_number;
> > +     for_each_valid_output_on_pipe(display, pipe_number, output)
> > +             prepare_crtc(data, fd, output);
>
> Do you really need to fire up and reset all the outputs that pipe can
> drive? As opposed to just one.
>
>
> --
> Petri Latvala
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

[igt-dev] [PATCH v3] tests/kms_vblank: Turn on hardware before testing invalid vblank.

[Mark Yacoub]

From: Mark Yacoub <markyacoub@google.com>

[Why]
Before any hardware is on, the vblank is off and its ref counter is in
an initialized state as each driver handles toggling it differently.
Ioctl DRM_IOCTL_WAIT_VBLANK could return 0 such as on i915, or an invalid
integer that doesn't mean much, such as on Zork running Kernel 5.4 due
to the kernel workaround that increments the vblank ref count to prevent
a get from enabling the interrupt.

[How]
For invalid_subtest(), active the CRTCs to turn the hardware on so
DRM_IOCTL_WAIT_VBLANK returns something meaningful.

Changes since v3:
Turn on only 1 output instead of all.

Changes since v2:
1. Updated Signed-off-by.

Changes since v1:
1. Update Commit message
2. Rename variable p to pipe_number

Signed-off-by: Mark Yacoub <markyacoub@chromium.org>
Change-Id: Id70072ea797a74d3e9563ef8feb5901daa6b511a
---
 tests/kms_vblank.c | 21 ++++++++++++++-------
 1 file changed, 14 insertions(+), 7 deletions(-)

diff --git a/tests/kms_vblank.c b/tests/kms_vblank.c
index 93b01eba..5e87610f 100644
--- a/tests/kms_vblank.c
+++ b/tests/kms_vblank.c
@@ -475,8 +475,13 @@ static void invalid_subtest(data_t *data, int fd)
 {
 	union drm_wait_vblank vbl;
 	unsigned long valid_flags;
+	igt_display_t* display = &data->display;
+	igt_output_t* output;
 
-	igt_display_require_output_on_pipe(&data->display, 0);
+	igt_display_require_output_on_pipe(display, 0);
+	igt_require(display->n_outputs);
+	output = &display->outputs[0];
+	prepare_crtc(data, fd, output);
 
 	/* First check all is well with a simple query */
 	memset(&vbl, 0, sizeof(vbl));
@@ -511,6 +516,8 @@ static void invalid_subtest(data_t *data, int fd)
 	vbl.request.type |= _DRM_VBLANK_SECONDARY;
 	vbl.request.type |= _DRM_VBLANK_FLAGS_MASK;
 	igt_assert_eq(wait_vblank(fd, &vbl), -EINVAL);
+
+	cleanup_crtc(data, fd, output);
 }
 
 igt_main
@@ -529,11 +536,11 @@ igt_main
 	igt_subtest("invalid")
 		invalid_subtest(&data, fd);
 
-	igt_describe("check the Vblank and flip events works with given crtc id");
-	igt_subtest("crtc-id")
-		crtc_id_subtest(&data, fd);
+	// igt_describe("check the Vblank and flip events works with given crtc id");
+	// igt_subtest("crtc-id")
+	// 	crtc_id_subtest(&data, fd);
 
-	for_each_pipe_static(data.pipe)
-		igt_subtest_group
-			run_subtests_for_pipe(&data);
+	// for_each_pipe_static(data.pipe)
+	// 	igt_subtest_group
+	// 		run_subtests_for_pipe(&data);
 }
-- 
2.32.0.288.g62a8d224e6-goog

_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [igt-dev] [PATCH v3] tests/kms_vblank: Turn on hardware before testing invalid vblank.

[Petri Latvala]

On Thu, Jun 17, 2021 at 03:20:32PM +0000, Mark Yacoub wrote:
> From: Mark Yacoub <markyacoub@google.com>
> 
> [Why]
> Before any hardware is on, the vblank is off and its ref counter is in
> an initialized state as each driver handles toggling it differently.
> Ioctl DRM_IOCTL_WAIT_VBLANK could return 0 such as on i915, or an invalid
> integer that doesn't mean much, such as on Zork running Kernel 5.4 due
> to the kernel workaround that increments the vblank ref count to prevent
> a get from enabling the interrupt.
> 
> [How]
> For invalid_subtest(), active the CRTCs to turn the hardware on so
> DRM_IOCTL_WAIT_VBLANK returns something meaningful.
> 
> Changes since v2:
> 1. Updated Signed-off-by.
> 
> Changes since v1:
> 1. Update Commit message
> 2. Rename variable p to pipe_number
> 
> Signed-off-by: Mark Yacoub <markyacoub@chromium.org>
> ---
>  tests/kms_vblank.c | 11 ++++++++++-
>  1 file changed, 10 insertions(+), 1 deletion(-)
> 
> diff --git a/tests/kms_vblank.c b/tests/kms_vblank.c
> index 93b01eba..988794c0 100644
> --- a/tests/kms_vblank.c
> +++ b/tests/kms_vblank.c
> @@ -475,8 +475,14 @@ static void invalid_subtest(data_t *data, int fd)
>  {
>  	union drm_wait_vblank vbl;
>  	unsigned long valid_flags;
> +	igt_display_t* display = &data->display;
> +	enum pipe pipe_number = 0;
> +	igt_output_t* output;
>  
> -	igt_display_require_output_on_pipe(&data->display, 0);
> +	igt_display_require_output_on_pipe(display, pipe_number);
> +	data->pipe = pipe_number;
> +	for_each_valid_output_on_pipe(display, pipe_number, output)
> +		prepare_crtc(data, fd, output);

Do you really need to fire up and reset all the outputs that pipe can
drive? As opposed to just one.


-- 
Petri Latvala
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

[igt-dev] [PATCH v3] tests/kms_vblank: Turn on hardware before testing invalid vblank.

[Mark Yacoub]

From: Mark Yacoub <markyacoub@google.com>

[Why]
Before any hardware is on, the vblank is off and its ref counter is in
an initialized state as each driver handles toggling it differently.
Ioctl DRM_IOCTL_WAIT_VBLANK could return 0 such as on i915, or an invalid
integer that doesn't mean much, such as on Zork running Kernel 5.4 due
to the kernel workaround that increments the vblank ref count to prevent
a get from enabling the interrupt.

[How]
For invalid_subtest(), active the CRTCs to turn the hardware on so
DRM_IOCTL_WAIT_VBLANK returns something meaningful.

Changes since v2:
1. Updated Signed-off-by.

Changes since v1:
1. Update Commit message
2. Rename variable p to pipe_number

Signed-off-by: Mark Yacoub <markyacoub@chromium.org>
---
 tests/kms_vblank.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/tests/kms_vblank.c b/tests/kms_vblank.c
index 93b01eba..988794c0 100644
--- a/tests/kms_vblank.c
+++ b/tests/kms_vblank.c
@@ -475,8 +475,14 @@ static void invalid_subtest(data_t *data, int fd)
 {
 	union drm_wait_vblank vbl;
 	unsigned long valid_flags;
+	igt_display_t* display = &data->display;
+	enum pipe pipe_number = 0;
+	igt_output_t* output;
 
-	igt_display_require_output_on_pipe(&data->display, 0);
+	igt_display_require_output_on_pipe(display, pipe_number);
+	data->pipe = pipe_number;
+	for_each_valid_output_on_pipe(display, pipe_number, output)
+		prepare_crtc(data, fd, output);
 
 	/* First check all is well with a simple query */
 	memset(&vbl, 0, sizeof(vbl));
@@ -511,6 +517,9 @@ static void invalid_subtest(data_t *data, int fd)
 	vbl.request.type |= _DRM_VBLANK_SECONDARY;
 	vbl.request.type |= _DRM_VBLANK_FLAGS_MASK;
 	igt_assert_eq(wait_vblank(fd, &vbl), -EINVAL);
+
+	for_each_valid_output_on_pipe(display, pipe_number, output)
+		cleanup_crtc(data, fd, output);
 }
 
 igt_main
-- 
2.32.0.288.g62a8d224e6-goog

_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev