* [PATCH i-g-t] tests/intel/kms_pwrite_crc: Create basic subtest
@ 2026-04-20 15:07 Kamil Konieczny
2026-04-21 3:28 ` Karthik B S
0 siblings, 1 reply; 3+ messages in thread
From: Kamil Konieczny @ 2026-04-20 15:07 UTC (permalink / raw)
To: igt-dev; +Cc: Kamil Konieczny, Karthik B S
Create a basic subtests which allows to properly cleanup after
any failure or a skip. Also while at this, move an igt header
to proper place.
Cc: Karthik B S <karthik.b.s@intel.com>
Signed-off-by: Kamil Konieczny <kamil.konieczny@linux.intel.com>
---
tests/intel/kms_pwrite_crc.c | 39 +++++++++++++++++++++++-------------
1 file changed, 25 insertions(+), 14 deletions(-)
diff --git a/tests/intel/kms_pwrite_crc.c b/tests/intel/kms_pwrite_crc.c
index bd536007c..92f5292dd 100644
--- a/tests/intel/kms_pwrite_crc.c
+++ b/tests/intel/kms_pwrite_crc.c
@@ -30,15 +30,16 @@
* Mega feature: General Display Features
*/
-#include "igt.h"
#include <errno.h>
#include <limits.h>
#include <stdbool.h>
#include <stdio.h>
#include <string.h>
+#include "igt.h"
+
/**
- * SUBTEST:
+ * SUBTEST: basic
* Description: Use the display CRC support to validate pwrite to an already
* uncached future scanout buffer.
*/
@@ -192,22 +193,32 @@ static void run_test(data_t *data)
igt_skip("no valid crtc/connector combinations found\n");
}
-static data_t data;
+static data_t data = { };
-int igt_simple_main()
+int igt_main()
{
- data.drm_fd = drm_open_driver_master(DRIVER_INTEL);
- kmstest_set_vt_graphics_mode();
+ igt_fixture() {
+ data.drm_fd = -1;
+ data.drm_fd = drm_open_driver_master(DRIVER_INTEL);
+ kmstest_set_vt_graphics_mode();
- igt_display_require(&data.display, data.drm_fd);
- igt_display_require_output(&data.display);
- igt_require_pipe_crc(data.drm_fd);
+ igt_display_require(&data.display, data.drm_fd);
+ igt_display_require_output(&data.display);
+ igt_require_pipe_crc(data.drm_fd);
- data.devid = intel_get_drm_devid(data.drm_fd);
- data.pipe_crc = NULL;
+ data.devid = intel_get_drm_devid(data.drm_fd);
+ data.pipe_crc = NULL;
+ }
- run_test(&data);
+ igt_describe("Use the display CRC support to validate pwrite "
+ "to an already uncached future scanout buffer.");
+ igt_subtest("basic")
+ run_test(&data);
- igt_display_fini(&data.display);
- drm_close_driver(data.drm_fd);
+ igt_fixture() {
+ if (data.drm_fd != -1) {
+ igt_display_fini(&data.display);
+ drm_close_driver(data.drm_fd);
+ }
+ }
}
--
2.53.0
^ permalink raw reply related [flat|nested] 3+ messages in thread* Re: [PATCH i-g-t] tests/intel/kms_pwrite_crc: Create basic subtest
2026-04-20 15:07 [PATCH i-g-t] tests/intel/kms_pwrite_crc: Create basic subtest Kamil Konieczny
@ 2026-04-21 3:28 ` Karthik B S
2026-04-21 12:05 ` Kamil Konieczny
0 siblings, 1 reply; 3+ messages in thread
From: Karthik B S @ 2026-04-21 3:28 UTC (permalink / raw)
To: Kamil Konieczny, igt-dev
Hi Kamil,
On 4/20/2026 8:37 PM, Kamil Konieczny wrote:
> Create a basic subtests which allows to properly cleanup after
> any failure or a skip. Also while at this, move an igt header
> to proper place.
>
> Cc: Karthik B S <karthik.b.s@intel.com>
> Signed-off-by: Kamil Konieczny <kamil.konieczny@linux.intel.com>
> ---
> tests/intel/kms_pwrite_crc.c | 39 +++++++++++++++++++++++-------------
> 1 file changed, 25 insertions(+), 14 deletions(-)
>
> diff --git a/tests/intel/kms_pwrite_crc.c b/tests/intel/kms_pwrite_crc.c
> index bd536007c..92f5292dd 100644
> --- a/tests/intel/kms_pwrite_crc.c
> +++ b/tests/intel/kms_pwrite_crc.c
> @@ -30,15 +30,16 @@
> * Mega feature: General Display Features
> */
>
> -#include "igt.h"
> #include <errno.h>
> #include <limits.h>
> #include <stdbool.h>
> #include <stdio.h>
> #include <string.h>
>
> +#include "igt.h"
> +
> /**
> - * SUBTEST:
> + * SUBTEST: basic
> * Description: Use the display CRC support to validate pwrite to an already
> * uncached future scanout buffer.
> */
> @@ -192,22 +193,32 @@ static void run_test(data_t *data)
> igt_skip("no valid crtc/connector combinations found\n");
> }
>
> -static data_t data;
> +static data_t data = { };
>
> -int igt_simple_main()
> +int igt_main()
> {
> - data.drm_fd = drm_open_driver_master(DRIVER_INTEL);
> - kmstest_set_vt_graphics_mode();
> + igt_fixture() {
> + data.drm_fd = -1;
This is redundant with the next line?
> + data.drm_fd = drm_open_driver_master(DRIVER_INTEL);
> + kmstest_set_vt_graphics_mode();
>
> - igt_display_require(&data.display, data.drm_fd);
> - igt_display_require_output(&data.display);
> - igt_require_pipe_crc(data.drm_fd);
> + igt_display_require(&data.display, data.drm_fd);
> + igt_display_require_output(&data.display);
> + igt_require_pipe_crc(data.drm_fd);
>
> - data.devid = intel_get_drm_devid(data.drm_fd);
> - data.pipe_crc = NULL;
> + data.devid = intel_get_drm_devid(data.drm_fd);
> + data.pipe_crc = NULL;
> + }
>
> - run_test(&data);
> + igt_describe("Use the display CRC support to validate pwrite "
> + "to an already uncached future scanout buffer.");
> + igt_subtest("basic")
> + run_test(&data);
>
> - igt_display_fini(&data.display);
> - drm_close_driver(data.drm_fd);
> + igt_fixture() {
> + if (data.drm_fd != -1) {
Similar redundancy here? If drm_fd = -1, the test will be skipped in the
first fixture itself and this check is not needed?
With these removed the patch LGTM,
Reviewed-by: Karthik B S <karthik.b.s@intel.com>
> + igt_display_fini(&data.display);
> + drm_close_driver(data.drm_fd);
> + }
> + }
> }
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH i-g-t] tests/intel/kms_pwrite_crc: Create basic subtest
2026-04-21 3:28 ` Karthik B S
@ 2026-04-21 12:05 ` Kamil Konieczny
0 siblings, 0 replies; 3+ messages in thread
From: Kamil Konieczny @ 2026-04-21 12:05 UTC (permalink / raw)
To: Karthik B S; +Cc: igt-dev
Hi Karthik,
On 2026-04-21 at 08:58:02 +0530, Karthik B S wrote:
> Hi Kamil,
>
> On 4/20/2026 8:37 PM, Kamil Konieczny wrote:
> > Create a basic subtests which allows to properly cleanup after
> > any failure or a skip. Also while at this, move an igt header
> > to proper place.
> >
> > Cc: Karthik B S <karthik.b.s@intel.com>
> > Signed-off-by: Kamil Konieczny <kamil.konieczny@linux.intel.com>
> > ---
> > tests/intel/kms_pwrite_crc.c | 39 +++++++++++++++++++++++-------------
> > 1 file changed, 25 insertions(+), 14 deletions(-)
> >
> > diff --git a/tests/intel/kms_pwrite_crc.c b/tests/intel/kms_pwrite_crc.c
> > index bd536007c..92f5292dd 100644
> > --- a/tests/intel/kms_pwrite_crc.c
> > +++ b/tests/intel/kms_pwrite_crc.c
> > @@ -30,15 +30,16 @@
> > * Mega feature: General Display Features
> > */
> > -#include "igt.h"
> > #include <errno.h>
> > #include <limits.h>
> > #include <stdbool.h>
> > #include <stdio.h>
> > #include <string.h>
> > +#include "igt.h"
> > +
> > /**
> > - * SUBTEST:
> > + * SUBTEST: basic
> > * Description: Use the display CRC support to validate pwrite to an already
> > * uncached future scanout buffer.
> > */
> > @@ -192,22 +193,32 @@ static void run_test(data_t *data)
> > igt_skip("no valid crtc/connector combinations found\n");
> > }
> > -static data_t data;
> > +static data_t data = { };
> > -int igt_simple_main()
> > +int igt_main()
> > {
> > - data.drm_fd = drm_open_driver_master(DRIVER_INTEL);
> > - kmstest_set_vt_graphics_mode();
> > + igt_fixture() {
> > + data.drm_fd = -1;
>
> This is redundant with the next line?
>
> > + data.drm_fd = drm_open_driver_master(DRIVER_INTEL);
> > + kmstest_set_vt_graphics_mode();
> > - igt_display_require(&data.display, data.drm_fd);
> > - igt_display_require_output(&data.display);
> > - igt_require_pipe_crc(data.drm_fd);
> > + igt_display_require(&data.display, data.drm_fd);
> > + igt_display_require_output(&data.display);
> > + igt_require_pipe_crc(data.drm_fd);
> > - data.devid = intel_get_drm_devid(data.drm_fd);
> > - data.pipe_crc = NULL;
> > + data.devid = intel_get_drm_devid(data.drm_fd);
> > + data.pipe_crc = NULL;
> > + }
> > - run_test(&data);
> > + igt_describe("Use the display CRC support to validate pwrite "
> > + "to an already uncached future scanout buffer.");
> > + igt_subtest("basic")
> > + run_test(&data);
> > - igt_display_fini(&data.display);
> > - drm_close_driver(data.drm_fd);
> > + igt_fixture() {
> > + if (data.drm_fd != -1) {
>
> Similar redundancy here? If drm_fd = -1, the test will be skipped in the
> first fixture itself and this check is not needed?
You are right, I will remove this and one at first fixture above.
>
> With these removed the patch LGTM,
>
> Reviewed-by: Karthik B S <karthik.b.s@intel.com>
>
Thank you, I will send v2 soon.
Regards,
Kamil
> > + igt_display_fini(&data.display);
> > + drm_close_driver(data.drm_fd);
> > + }
> > + }
> > }
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2026-04-21 12:06 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-04-20 15:07 [PATCH i-g-t] tests/intel/kms_pwrite_crc: Create basic subtest Kamil Konieczny
2026-04-21 3:28 ` Karthik B S
2026-04-21 12:05 ` Kamil Konieczny
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox