[PATCH i-g-t 1/6] lib/igt_vmwgfx: Add vmwgfx device

[PATCH i-g-t 1/6] lib/igt_vmwgfx: Add vmwgfx device

[Deepak Rawat]

Add DRIVER_VMWGFX to represent vmwgfx device for running igt tests.

v2: Don't remove second virtio_gpu

Signed-off-by: Deepak Rawat <drawat@vmware.com>
---
 lib/drmtest.c | 8 ++++++++
 lib/drmtest.h | 3 +++
 2 files changed, 11 insertions(+)

diff --git a/lib/drmtest.c b/lib/drmtest.c
index fee9d33a..9d013a00 100644
--- a/lib/drmtest.c
+++ b/lib/drmtest.c
@@ -105,6 +105,11 @@ bool is_i915_device(int fd)
 	return __is_device(fd, "i915");
 }
 
+bool is_vmwgfx_device(int fd)
+{
+	return __is_device(fd, "vmwg");
+}
+
 static bool has_known_intel_chipset(int fd)
 {
 	struct drm_i915_getparam gp;
@@ -206,6 +211,7 @@ static const struct module {
 	{ DRIVER_VGEM, "vgem" },
 	{ DRIVER_VIRTIO, "virtio-gpu" },
 	{ DRIVER_VIRTIO, "virtio_gpu" },
+	{ DRIVER_VMWGFX, "vmwgfx" },
 	{}
 };
 
@@ -348,6 +354,8 @@ static const char *chipset_to_str(int chipset)
 		return "virtio";
 	case DRIVER_AMDGPU:
 		return "amdgpu";
+	case DRIVER_VMWGFX:
+		return "vmwgfx";
 	case DRIVER_ANY:
 		return "any";
 	default:
diff --git a/lib/drmtest.h b/lib/drmtest.h
index 949865ee..0213fb51 100644
--- a/lib/drmtest.h
+++ b/lib/drmtest.h
@@ -43,6 +43,7 @@
 #define DRIVER_VGEM	(1 << 2)
 #define DRIVER_VIRTIO	(1 << 3)
 #define DRIVER_AMDGPU	(1 << 4)
+#define DRIVER_VMWGFX	(1 << 5)
 /*
  * Exclude DRVER_VGEM from DRIVER_ANY since if you run on a system
  * with vgem as well as a supported driver, you can end up with a
@@ -80,6 +81,8 @@ void igt_require_intel(int fd);
 
 bool is_i915_device(int fd);
 
+bool is_vmwgfx_device(int fd);
+
 /**
  * do_or_die:
  * @x: command
-- 
2.17.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[igt-dev] [PATCH i-g-t 1/6] lib/igt_vmwgfx: Add vmwgfx device

[Deepak Rawat]

Add DRIVER_VMWGFX to represent vmwgfx device for running igt tests.

v2: Don't remove second virtio_gpu

Signed-off-by: Deepak Rawat <drawat@vmware.com>
---
 lib/drmtest.c | 8 ++++++++
 lib/drmtest.h | 3 +++
 2 files changed, 11 insertions(+)

diff --git a/lib/drmtest.c b/lib/drmtest.c
index fee9d33a..9d013a00 100644
--- a/lib/drmtest.c
+++ b/lib/drmtest.c
@@ -105,6 +105,11 @@ bool is_i915_device(int fd)
 	return __is_device(fd, "i915");
 }
 
+bool is_vmwgfx_device(int fd)
+{
+	return __is_device(fd, "vmwg");
+}
+
 static bool has_known_intel_chipset(int fd)
 {
 	struct drm_i915_getparam gp;
@@ -206,6 +211,7 @@ static const struct module {
 	{ DRIVER_VGEM, "vgem" },
 	{ DRIVER_VIRTIO, "virtio-gpu" },
 	{ DRIVER_VIRTIO, "virtio_gpu" },
+	{ DRIVER_VMWGFX, "vmwgfx" },
 	{}
 };
 
@@ -348,6 +354,8 @@ static const char *chipset_to_str(int chipset)
 		return "virtio";
 	case DRIVER_AMDGPU:
 		return "amdgpu";
+	case DRIVER_VMWGFX:
+		return "vmwgfx";
 	case DRIVER_ANY:
 		return "any";
 	default:
diff --git a/lib/drmtest.h b/lib/drmtest.h
index 949865ee..0213fb51 100644
--- a/lib/drmtest.h
+++ b/lib/drmtest.h
@@ -43,6 +43,7 @@
 #define DRIVER_VGEM	(1 << 2)
 #define DRIVER_VIRTIO	(1 << 3)
 #define DRIVER_AMDGPU	(1 << 4)
+#define DRIVER_VMWGFX	(1 << 5)
 /*
  * Exclude DRVER_VGEM from DRIVER_ANY since if you run on a system
  * with vgem as well as a supported driver, you can end up with a
@@ -80,6 +81,8 @@ void igt_require_intel(int fd);
 
 bool is_i915_device(int fd);
 
+bool is_vmwgfx_device(int fd);
+
 /**
  * do_or_die:
  * @x: command
-- 
2.17.1

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

[PATCH i-g-t 2/6] lib/igt_fb: Call dumb_destroy ioctl in case of dumb buffers

[Deepak Rawat]

vmwgfx does not support GEM interface so calling gem_close on vmwgfx
results in error.

v2: Use drmIoctl with error when ioctl() failed.

v3: Seperate ioctl wrapper.

Signed-off-by: Deepak Rawat <drawat@vmware.com>
---
 lib/igt_fb.c  |  5 ++++-
 lib/igt_kms.c | 22 ++++++++++++++++++++++
 lib/igt_kms.h |  1 +
 3 files changed, 27 insertions(+), 1 deletion(-)

diff --git a/lib/igt_fb.c b/lib/igt_fb.c
index 35be2e88..335ece69 100644
--- a/lib/igt_fb.c
+++ b/lib/igt_fb.c
@@ -2121,7 +2121,10 @@ void igt_remove_fb(int fd, struct igt_fb *fb)
 
 	cairo_surface_destroy(fb->cairo_surface);
 	do_or_die(drmModeRmFB(fd, fb->fb_id));
-	gem_close(fd, fb->gem_handle);
+	if (fb->is_dumb)
+		kmstest_dumb_destroy(fd, fb->gem_handle);
+	else
+		gem_close(fd, fb->gem_handle);
 	fb->fb_id = 0;
 }
 
diff --git a/lib/igt_kms.c b/lib/igt_kms.c
index b2cbaa11..6e499f48 100644
--- a/lib/igt_kms.c
+++ b/lib/igt_kms.c
@@ -641,6 +641,28 @@ void *kmstest_dumb_map_buffer(int fd, uint32_t handle, uint64_t size,
 	return ptr;
 }
 
+static int __kmstest_dumb_destroy(int fd, uint32_t handle)
+{
+	struct drm_mode_destroy_dumb arg = { handle };
+	int err = 0;
+
+	if (drmIoctl(fd, DRM_IOCTL_MODE_DESTROY_DUMB, &arg))
+		err = -errno;
+
+	errno = 0;
+	return err;
+}
+
+/**
+ * kmstest_dumb_destroy:
+ * @fd: Opened drm file descriptor
+ * @handle: Offset in the file referred to by fd
+ */
+void kmstest_dumb_destroy(int fd, uint32_t handle)
+{
+	igt_assert_eq(__kmstest_dumb_destroy(fd, handle), 0);
+}
+
 /*
  * Returns: the previous mode, or KD_GRAPHICS if no /dev/tty0 was
  * found and nothing was done.
diff --git a/lib/igt_kms.h b/lib/igt_kms.h
index 38fa944e..cbfcced9 100644
--- a/lib/igt_kms.h
+++ b/lib/igt_kms.h
@@ -222,6 +222,7 @@ uint32_t kmstest_dumb_create(int fd, int width, int height, int bpp,
 
 void *kmstest_dumb_map_buffer(int fd, uint32_t handle, uint64_t size,
 			      unsigned prot);
+void kmstest_dumb_destroy(int fd, uint32_t handle);
 void kmstest_wait_for_pageflip(int fd);
 unsigned int kmstest_get_vblank(int fd, int pipe, unsigned int flags);
 void igt_assert_plane_visible(int fd, enum pipe pipe, bool visibility);
-- 
2.17.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH i-g-t 2/6] lib/igt_fb: Call dumb_destroy ioctl in case of dumb buffers

[Deepak Rawat]

vmwgfx does not support GEM interface so calling gem_close on vmwgfx
results in error.

v2: Use drmIoctl with error when ioctl() failed.

v3: Seperate ioctl wrapper.

Signed-off-by: Deepak Rawat <drawat@vmware.com>
---
 lib/igt_fb.c  |  5 ++++-
 lib/igt_kms.c | 22 ++++++++++++++++++++++
 lib/igt_kms.h |  1 +
 3 files changed, 27 insertions(+), 1 deletion(-)

diff --git a/lib/igt_fb.c b/lib/igt_fb.c
index 35be2e88..335ece69 100644
--- a/lib/igt_fb.c
+++ b/lib/igt_fb.c
@@ -2121,7 +2121,10 @@ void igt_remove_fb(int fd, struct igt_fb *fb)
 
 	cairo_surface_destroy(fb->cairo_surface);
 	do_or_die(drmModeRmFB(fd, fb->fb_id));
-	gem_close(fd, fb->gem_handle);
+	if (fb->is_dumb)
+		kmstest_dumb_destroy(fd, fb->gem_handle);
+	else
+		gem_close(fd, fb->gem_handle);
 	fb->fb_id = 0;
 }
 
diff --git a/lib/igt_kms.c b/lib/igt_kms.c
index b2cbaa11..6e499f48 100644
--- a/lib/igt_kms.c
+++ b/lib/igt_kms.c
@@ -641,6 +641,28 @@ void *kmstest_dumb_map_buffer(int fd, uint32_t handle, uint64_t size,
 	return ptr;
 }
 
+static int __kmstest_dumb_destroy(int fd, uint32_t handle)
+{
+	struct drm_mode_destroy_dumb arg = { handle };
+	int err = 0;
+
+	if (drmIoctl(fd, DRM_IOCTL_MODE_DESTROY_DUMB, &arg))
+		err = -errno;
+
+	errno = 0;
+	return err;
+}
+
+/**
+ * kmstest_dumb_destroy:
+ * @fd: Opened drm file descriptor
+ * @handle: Offset in the file referred to by fd
+ */
+void kmstest_dumb_destroy(int fd, uint32_t handle)
+{
+	igt_assert_eq(__kmstest_dumb_destroy(fd, handle), 0);
+}
+
 /*
  * Returns: the previous mode, or KD_GRAPHICS if no /dev/tty0 was
  * found and nothing was done.
diff --git a/lib/igt_kms.h b/lib/igt_kms.h
index 38fa944e..cbfcced9 100644
--- a/lib/igt_kms.h
+++ b/lib/igt_kms.h
@@ -222,6 +222,7 @@ uint32_t kmstest_dumb_create(int fd, int width, int height, int bpp,
 
 void *kmstest_dumb_map_buffer(int fd, uint32_t handle, uint64_t size,
 			      unsigned prot);
+void kmstest_dumb_destroy(int fd, uint32_t handle);
 void kmstest_wait_for_pageflip(int fd);
 unsigned int kmstest_get_vblank(int fd, int pipe, unsigned int flags);
 void igt_assert_plane_visible(int fd, enum pipe pipe, bool visibility);
-- 
2.17.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH i-g-t 3/6] lib/igt_fb: Check for cairo surface success

[Deepak Rawat]

For vmwgfx cairo surface creation fails due to stride mismatch, add a
igt_require_f() for surface.

v2: Check for surface creation failure.

Signed-off-by: Deepak Rawat <drawat@vmware.com>
---
 lib/igt_fb.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/lib/igt_fb.c b/lib/igt_fb.c
index 335ece69..1bb6d324 100644
--- a/lib/igt_fb.c
+++ b/lib/igt_fb.c
@@ -1433,6 +1433,10 @@ static void create_cairo_surface__gtt(int fd, struct igt_fb *fb)
 		cairo_image_surface_create_for_data(ptr,
 						    drm_format_to_cairo(fb->drm_format),
 						    fb->width, fb->height, fb->strides[0]);
+	igt_require_f(cairo_surface_status(fb->cairo_surface) == CAIRO_STATUS_SUCCESS,
+		      "Unable to create a cairo surface: %s\n",
+		      cairo_status_to_string(cairo_surface_status(fb->cairo_surface)));
+
 	fb->domain = I915_GEM_DOMAIN_GTT;
 
 	cairo_surface_set_user_data(fb->cairo_surface,
-- 
2.17.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH i-g-t 3/6] lib/igt_fb: Check for cairo surface success

[Deepak Rawat]

For vmwgfx cairo surface creation fails due to stride mismatch, add a
igt_require_f() for surface.

v2: Check for surface creation failure.

Signed-off-by: Deepak Rawat <drawat@vmware.com>
---
 lib/igt_fb.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/lib/igt_fb.c b/lib/igt_fb.c
index 335ece69..1bb6d324 100644
--- a/lib/igt_fb.c
+++ b/lib/igt_fb.c
@@ -1433,6 +1433,10 @@ static void create_cairo_surface__gtt(int fd, struct igt_fb *fb)
 		cairo_image_surface_create_for_data(ptr,
 						    drm_format_to_cairo(fb->drm_format),
 						    fb->width, fb->height, fb->strides[0]);
+	igt_require_f(cairo_surface_status(fb->cairo_surface) == CAIRO_STATUS_SUCCESS,
+		      "Unable to create a cairo surface: %s\n",
+		      cairo_status_to_string(cairo_surface_status(fb->cairo_surface)));
+
 	fb->domain = I915_GEM_DOMAIN_GTT;
 
 	cairo_surface_set_user_data(fb->cairo_surface,
-- 
2.17.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH i-g-t 4/6] lib: Don't call igt_require_fb_modifiers() when no modifier

[Deepak Rawat]

vmwgfx doesn't support fb modifier so skip igt_require_fb_modifiers()
when modifier are not passed.

Signed-off-by: Deepak Rawat <drawat@vmware.com>
---
 lib/ioctl_wrappers.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/lib/ioctl_wrappers.c b/lib/ioctl_wrappers.c
index 0929c43f..3a11cb6e 100644
--- a/lib/ioctl_wrappers.c
+++ b/lib/ioctl_wrappers.c
@@ -1678,7 +1678,10 @@ int __kms_addfb(int fd, uint32_t handle,
 	struct drm_mode_fb_cmd2 f;
 	int ret, i;
 
-	igt_require_fb_modifiers(fd);
+	if (modifier)
+		igt_require_fb_modifiers(fd);
+	else
+		flags &= ~DRM_MODE_FB_MODIFIERS;
 
 	memset(&f, 0, sizeof(f));
 
-- 
2.17.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[igt-dev] [PATCH i-g-t 4/6] lib: Don't call igt_require_fb_modifiers() when no modifier

[Deepak Rawat]

vmwgfx doesn't support fb modifier so skip igt_require_fb_modifiers()
when modifier are not passed.

Signed-off-by: Deepak Rawat <drawat@vmware.com>
---
 lib/ioctl_wrappers.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/lib/ioctl_wrappers.c b/lib/ioctl_wrappers.c
index 0929c43f..3a11cb6e 100644
--- a/lib/ioctl_wrappers.c
+++ b/lib/ioctl_wrappers.c
@@ -1678,7 +1678,10 @@ int __kms_addfb(int fd, uint32_t handle,
 	struct drm_mode_fb_cmd2 f;
 	int ret, i;
 
-	igt_require_fb_modifiers(fd);
+	if (modifier)
+		igt_require_fb_modifiers(fd);
+	else
+		flags &= ~DRM_MODE_FB_MODIFIERS;
 
 	memset(&f, 0, sizeof(f));
 
-- 
2.17.1

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

[PATCH i-g-t 5/6] tests/kms_atomic: Add a new test case for FB_DAMAGE_CLIPS plane property

[Deepak Rawat]

Some simple test cases to use FB_DAMAGE_CLIPS plane property.

Cc: ville.syrjala@linux.intel.com
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Pekka Paalanen <ppaalanen@gmail.com>
Cc: Daniel Stone <daniel@fooishbar.org>
Signed-off-by: Deepak Rawat <drawat@vmware.com>
---
 lib/igt_kms.c      |   1 +
 lib/igt_kms.h      |   1 +
 tests/kms_atomic.c | 260 +++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 262 insertions(+)

diff --git a/lib/igt_kms.c b/lib/igt_kms.c
index 6e499f48..7a3ce2f6 100644
--- a/lib/igt_kms.c
+++ b/lib/igt_kms.c
@@ -177,6 +177,7 @@ const char * const igt_plane_prop_names[IGT_NUM_PLANE_PROPS] = {
 	[IGT_PLANE_COLOR_RANGE] = "COLOR_RANGE",
 	[IGT_PLANE_PIXEL_BLEND_MODE] = "pixel blend mode",
 	[IGT_PLANE_ALPHA] = "alpha",
+	[IGT_PLANE_FB_DAMAGE_CLIPS] = "FB_DAMAGE_CLIPS",
 };
 
 const char * const igt_crtc_prop_names[IGT_NUM_CRTC_PROPS] = {
diff --git a/lib/igt_kms.h b/lib/igt_kms.h
index cbfcced9..e1c9a6a5 100644
--- a/lib/igt_kms.h
+++ b/lib/igt_kms.h
@@ -269,6 +269,7 @@ enum igt_atomic_plane_properties {
        IGT_PLANE_COLOR_RANGE,
        IGT_PLANE_PIXEL_BLEND_MODE,
        IGT_PLANE_ALPHA,
+       IGT_PLANE_FB_DAMAGE_CLIPS,
        IGT_NUM_PLANE_PROPS
 };
 
diff --git a/tests/kms_atomic.c b/tests/kms_atomic.c
index 3aad2d9a..20dfc978 100644
--- a/tests/kms_atomic.c
+++ b/tests/kms_atomic.c
@@ -56,6 +56,24 @@
 
 IGT_TEST_DESCRIPTION("Test atomic modesetting API");
 
+/* signed32 drm_mode_rect declared here for use with drm damage for page-flip */
+struct damage_rect {
+	int x1;
+	int y1;
+	int x2;
+	int y2;
+};
+
+static inline int damage_rect_width(struct damage_rect *r)
+{
+	return r->x2 - r->x1;
+}
+
+static inline int damage_rect_height(struct damage_rect *r)
+{
+	return r->y2 - r->y1;
+}
+
 enum kms_atomic_check_relax {
 	ATOMIC_RELAX_NONE = 0,
 	CRTC_RELAX_MODE = (1 << 0),
@@ -835,6 +853,240 @@ static void atomic_invalid_params(igt_pipe_t *pipe,
 	do_ioctl_err(display->drm_fd, DRM_IOCTL_MODE_ATOMIC, &ioc, EFAULT);
 }
 
+static void atomic_plane_damage(igt_pipe_t *pipe, igt_plane_t *plane, struct igt_fb *fb)
+{
+	struct damage_rect *damage;
+	struct igt_fb fb_1;
+	struct igt_fb fb_2;
+	cairo_t *cr_1;
+	cairo_t *cr_2;
+
+	damage = malloc(sizeof(*damage) * 2);
+	igt_assert(damage);
+
+	/* Color fb with white rect at center */
+	igt_create_color_fb(pipe->display->drm_fd, fb->width, fb->height,
+			    fb->drm_format, I915_TILING_NONE, 0.2, 0.2, 0.2,
+			    &fb_1);
+	cr_1 = igt_get_cairo_ctx(pipe->display->drm_fd, &fb_1);
+	igt_paint_color(cr_1, fb->width/4, fb->height/4, fb->width/2,
+			fb->height/2, 1.0, 1.0, 1.0);
+	igt_put_cairo_ctx(pipe->display->drm_fd, &fb_1, cr_1);
+
+	/*
+	 * Flip the primary plane to new color fb using atomic API and check the
+	 * state.
+	 */
+	igt_plane_set_fb(plane, &fb_1);
+	crtc_commit(pipe, plane, COMMIT_ATOMIC, ATOMIC_RELAX_NONE);
+
+	/*
+	 * Change the color of top left clip from center and issue plane update
+	 * with damage and verify the state.
+	 */
+	damage[0].x1 = 0;
+	damage[0].y1 = 0;
+	damage[0].x2 = fb->width/2;
+	damage[0].y2 = fb->height/2;
+
+	cr_1 = igt_get_cairo_ctx(pipe->display->drm_fd, &fb_1);
+	igt_paint_color(cr_1, damage[0].x1, damage[0].y1,
+			damage_rect_width(&damage[0]),
+			damage_rect_height(&damage[0]), 1.0, 0, 0);
+	igt_put_cairo_ctx(pipe->display->drm_fd, &fb_1, cr_1);
+
+	igt_plane_set_fb(plane, &fb_1);
+	igt_plane_replace_prop_blob(plane, IGT_PLANE_FB_DAMAGE_CLIPS, damage,
+				    sizeof(*damage));
+	crtc_commit(pipe, plane, COMMIT_ATOMIC, ATOMIC_RELAX_NONE);
+
+	/*
+	 * Change the color of top left and bottom right clip from center and
+	 * issue plane update with damage and verify the state.
+	 */
+	damage[0].x1 = 0;
+	damage[0].y1 = 0;
+	damage[0].x2 = fb->width/2;
+	damage[0].y2 = fb->height/2;
+
+	damage[1].x1 = fb->width/2;
+	damage[1].y1 = fb->height/2;
+	damage[1].x2 = fb->width;
+	damage[1].y2 = fb->height;
+
+	cr_1 = igt_get_cairo_ctx(pipe->display->drm_fd, &fb_1);
+	igt_paint_color(cr_1, damage[0].x1, damage[0].y1,
+			damage_rect_width(&damage[0]),
+			damage_rect_height(&damage[0]), 1.0, 0, 1.0);
+	igt_paint_color(cr_1, damage[1].x1, damage[1].y1,
+			damage_rect_width(&damage[1]),
+			damage_rect_height(&damage[1]), 0, 0, 1.0);
+	igt_put_cairo_ctx(pipe->display->drm_fd, &fb_1, cr_1);
+
+	igt_plane_set_fb(plane, &fb_1);
+	igt_plane_replace_prop_blob(plane, IGT_PLANE_FB_DAMAGE_CLIPS, damage,
+				    sizeof(*damage) * 2);
+	crtc_commit(pipe, plane, COMMIT_ATOMIC, ATOMIC_RELAX_NONE);
+
+	/*
+	 * Issue a plane update with damage on a seperate fb with damage to
+	 * upper right clip from center.
+	 */
+	igt_create_color_fb(pipe->display->drm_fd, fb->width, fb->height,
+			    fb->drm_format, I915_TILING_NONE, 0.2, 0.2, 0.2,
+			    &fb_2);
+
+	damage[0].x1 = fb->width/2;
+	damage[0].y1 = 0;
+	damage[0].x2 = fb->width;
+	damage[0].y2 = fb->height/2;
+
+	cr_2 = igt_get_cairo_ctx(pipe->display->drm_fd, &fb_2);
+	cr_1 = igt_get_cairo_ctx(pipe->display->drm_fd, &fb_1);
+	cairo_set_source_surface(cr_2, fb_1.cairo_surface, 0, 0);
+	cairo_paint(cr_2);
+	igt_paint_color(cr_2, damage[0].x1, damage[0].y1,
+			damage_rect_width(&damage[0]),
+			damage_rect_height(&damage[0]), 0, 1.0, 0);
+	igt_put_cairo_ctx(pipe->display->drm_fd, &fb_1, cr_1);
+	igt_put_cairo_ctx(pipe->display->drm_fd, &fb_2, cr_2);
+	igt_plane_set_fb(plane, &fb_2);
+	igt_plane_replace_prop_blob(plane, IGT_PLANE_FB_DAMAGE_CLIPS, damage,
+				    sizeof(*damage));
+	crtc_commit(pipe, plane, COMMIT_ATOMIC, ATOMIC_RELAX_NONE);
+
+	/*
+	 * Issue plane update with damage with a clip outside of plane src.
+	 * NOTE: This will result in no update on plane as damage is outside, so
+	 * will see no change on the screen.
+	 */
+	/* Reszie fb_1 to be bigger than plane */
+	igt_remove_fb(pipe->display->drm_fd, &fb_1);
+	igt_create_color_fb(pipe->display->drm_fd, fb->width * 2, fb->height,
+			    fb->drm_format, I915_TILING_NONE, 0.2, 0.2, 0.2,
+			    &fb_1);
+
+	damage[0].x1 = fb->width;
+	damage[0].y1 = 0;
+	damage[0].x2 = fb->width + fb->width/2;
+	damage[0].y2 = fb->height/2;
+
+	cr_1 = igt_get_cairo_ctx(pipe->display->drm_fd, &fb_1);
+	cr_2 = igt_get_cairo_ctx(pipe->display->drm_fd, &fb_2);
+	cairo_set_source_surface(cr_1, fb_2.cairo_surface, 0, 0);
+	cairo_paint(cr_1);
+	igt_paint_color(cr_1, damage[0].x1, damage[0].y1,
+			damage_rect_width(&damage[0]),
+			damage_rect_height(&damage[0]), 0, 1.0, 0);
+	igt_put_cairo_ctx(pipe->display->drm_fd, &fb_2, cr_2);
+	igt_put_cairo_ctx(pipe->display->drm_fd, &fb_1, cr_1);
+	igt_plane_set_fb(plane, &fb_1);
+	igt_plane_set_size(plane, fb->width, fb->height);
+	igt_fb_set_position(&fb_1, plane, 0, 0);
+	igt_fb_set_size(&fb_1, plane, fb->width, fb->height);
+	igt_plane_replace_prop_blob(plane, IGT_PLANE_FB_DAMAGE_CLIPS, damage,
+				    sizeof(*damage));
+	crtc_commit(pipe, plane, COMMIT_ATOMIC, ATOMIC_RELAX_NONE);
+
+	/*
+	 * Issue a plane update with damage with a clip that overlap with plane
+	 * src (Top right from center extending outside src in below case).
+	 * NOTE: Here drm core should take care of intersecting the clip to
+	 * plane src.
+	 */
+	damage[0].x1 = fb->width/2;
+	damage[0].y1 = 0;
+	damage[0].x2 = fb->width/2 + fb->width;
+	damage[0].y2 = fb->height/2;
+
+	cr_1 = igt_get_cairo_ctx(pipe->display->drm_fd, &fb_1);
+	igt_paint_color(cr_1, damage[0].x1, damage[0].y1,
+			damage_rect_width(&damage[0]),
+			damage_rect_height(&damage[0]), 1.0, 1.0, 0);
+	igt_put_cairo_ctx(pipe->display->drm_fd, &fb_1, cr_1);
+	igt_plane_set_fb(plane, &fb_1);
+	igt_plane_set_size(plane, fb->width, fb->height);
+	igt_fb_set_position(&fb_1, plane, 0, 0);
+	igt_fb_set_size(&fb_1, plane, fb->width, fb->height);
+	igt_plane_replace_prop_blob(plane, IGT_PLANE_FB_DAMAGE_CLIPS, damage,
+				    sizeof(*damage));
+	crtc_commit(pipe, plane, COMMIT_ATOMIC, ATOMIC_RELAX_NONE);
+
+	/*
+	 * Issue a plane update with damage with two clips one inside plane src
+	 * and one outside
+	 * NOTE: This will result in plane update with clip inside plane src.
+	 */
+	damage[0].x1 = 0;
+	damage[0].y1 = fb->height/2;
+	damage[0].x2 = fb->width/2;
+	damage[0].y2 = fb->height;
+
+	damage[1].x1 = fb->width + fb->width/2;
+	damage[1].y1 = fb->height/2;
+	damage[1].x2 = fb->width * 2;
+	damage[1].y2 = fb->height;
+
+	cr_1 = igt_get_cairo_ctx(pipe->display->drm_fd, &fb_1);
+	igt_paint_color(cr_1, damage[0].x1, damage[0].y1,
+			damage_rect_width(&damage[0]),
+			damage_rect_height(&damage[0]), 0, 1.0, 1.0);
+	igt_paint_color(cr_1, damage[1].x1, damage[1].y1,
+			damage_rect_width(&damage[1]),
+			damage_rect_height(&damage[1]), 0, 1.0, 0);
+	igt_put_cairo_ctx(pipe->display->drm_fd, &fb_1, cr_1);
+	igt_plane_set_fb(plane, &fb_1);
+	igt_plane_set_size(plane, fb->width, fb->height);
+	igt_fb_set_position(&fb_1, plane, 0, 0);
+	igt_fb_set_size(&fb_1, plane, fb->width, fb->height);
+	igt_plane_replace_prop_blob(plane, IGT_PLANE_FB_DAMAGE_CLIPS, damage,
+				    sizeof(*damage) * 2);
+	crtc_commit(pipe, plane, COMMIT_ATOMIC, ATOMIC_RELAX_NONE);
+
+	/*
+	 * Issue a plane update with overlapping damage clips. White rect in
+	 * center overlap partially with top left red rect.
+	 * NOTE: Drm core does not error for overlapping damage clips so if any
+	 * driver does not support overlapping should have their own
+	 * validations.
+	 */
+	damage[0].x1 = 0;
+	damage[0].y1 = 0;
+	damage[0].x2 = fb->width/2;
+	damage[0].y2 = fb->height/2;
+
+	damage[1].x1 = fb->width/4;
+	damage[1].y1 = fb->height/4;
+	damage[1].x2 = fb->width/4 + fb->width/2;
+	damage[1].y2 = fb->height/4 + fb->height/2;
+
+	cr_1 = igt_get_cairo_ctx(pipe->display->drm_fd, &fb_1);
+	igt_paint_color(cr_1, damage[0].x1, damage[0].y1,
+			damage_rect_width(&damage[0]),
+			damage_rect_height(&damage[0]), 1.0, 0, 0);
+	igt_paint_color(cr_1, damage[1].x1, damage[1].y1,
+			damage_rect_width(&damage[1]),
+			damage_rect_height(&damage[1]), 1.0, 1.0, 1.0);
+	igt_put_cairo_ctx(pipe->display->drm_fd, &fb_1, cr_1);
+	igt_plane_set_fb(plane, &fb_1);
+	igt_plane_set_size(plane, fb->width, fb->height);
+	igt_fb_set_position(&fb_1, plane, 0, 0);
+	igt_fb_set_size(&fb_1, plane, fb->width, fb->height);
+	igt_plane_replace_prop_blob(plane, IGT_PLANE_FB_DAMAGE_CLIPS, damage,
+				    sizeof(*damage) * 2);
+	crtc_commit(pipe, plane, COMMIT_ATOMIC, ATOMIC_RELAX_NONE);
+
+	/* Restore the primary plane */
+	igt_plane_set_fb(plane, fb);
+	plane_commit(plane, COMMIT_ATOMIC, ATOMIC_RELAX_NONE);
+
+	/* Remove the fb created for this test */
+	igt_remove_fb(pipe->display->drm_fd, &fb_1);
+	igt_remove_fb(pipe->display->drm_fd, &fb_2);
+
+	free(damage);
+}
+
 static void atomic_setup(igt_display_t *display, enum pipe pipe, igt_output_t *output, igt_plane_t *primary, struct igt_fb *fb)
 {
 	igt_output_set_pipe(output, pipe);
@@ -950,6 +1202,14 @@ igt_main
 		atomic_invalid_params(pipe_obj, primary, output, &fb);
 	}
 
+	igt_subtest("atomic_plane_damage") {
+		igt_require(igt_plane_has_prop(primary, IGT_PLANE_FB_DAMAGE_CLIPS));
+
+		atomic_setup(&display, pipe, output, primary, &fb);
+
+		atomic_plane_damage(pipe_obj, primary, &fb);
+	}
+
 	igt_fixture {
 		atomic_clear(&display, pipe, primary, output);
 		igt_remove_fb(display.drm_fd, &fb);
-- 
2.17.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH i-g-t 5/6] tests/kms_atomic: Add a new test case for FB_DAMAGE_CLIPS plane property

[Deepak Rawat]

Some simple test cases to use FB_DAMAGE_CLIPS plane property.

Cc: ville.syrjala@linux.intel.com
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Pekka Paalanen <ppaalanen@gmail.com>
Cc: Daniel Stone <daniel@fooishbar.org>
Signed-off-by: Deepak Rawat <drawat@vmware.com>
---
 lib/igt_kms.c      |   1 +
 lib/igt_kms.h      |   1 +
 tests/kms_atomic.c | 260 +++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 262 insertions(+)

diff --git a/lib/igt_kms.c b/lib/igt_kms.c
index 6e499f48..7a3ce2f6 100644
--- a/lib/igt_kms.c
+++ b/lib/igt_kms.c
@@ -177,6 +177,7 @@ const char * const igt_plane_prop_names[IGT_NUM_PLANE_PROPS] = {
 	[IGT_PLANE_COLOR_RANGE] = "COLOR_RANGE",
 	[IGT_PLANE_PIXEL_BLEND_MODE] = "pixel blend mode",
 	[IGT_PLANE_ALPHA] = "alpha",
+	[IGT_PLANE_FB_DAMAGE_CLIPS] = "FB_DAMAGE_CLIPS",
 };
 
 const char * const igt_crtc_prop_names[IGT_NUM_CRTC_PROPS] = {
diff --git a/lib/igt_kms.h b/lib/igt_kms.h
index cbfcced9..e1c9a6a5 100644
--- a/lib/igt_kms.h
+++ b/lib/igt_kms.h
@@ -269,6 +269,7 @@ enum igt_atomic_plane_properties {
        IGT_PLANE_COLOR_RANGE,
        IGT_PLANE_PIXEL_BLEND_MODE,
        IGT_PLANE_ALPHA,
+       IGT_PLANE_FB_DAMAGE_CLIPS,
        IGT_NUM_PLANE_PROPS
 };
 
diff --git a/tests/kms_atomic.c b/tests/kms_atomic.c
index 3aad2d9a..20dfc978 100644
--- a/tests/kms_atomic.c
+++ b/tests/kms_atomic.c
@@ -56,6 +56,24 @@
 
 IGT_TEST_DESCRIPTION("Test atomic modesetting API");
 
+/* signed32 drm_mode_rect declared here for use with drm damage for page-flip */
+struct damage_rect {
+	int x1;
+	int y1;
+	int x2;
+	int y2;
+};
+
+static inline int damage_rect_width(struct damage_rect *r)
+{
+	return r->x2 - r->x1;
+}
+
+static inline int damage_rect_height(struct damage_rect *r)
+{
+	return r->y2 - r->y1;
+}
+
 enum kms_atomic_check_relax {
 	ATOMIC_RELAX_NONE = 0,
 	CRTC_RELAX_MODE = (1 << 0),
@@ -835,6 +853,240 @@ static void atomic_invalid_params(igt_pipe_t *pipe,
 	do_ioctl_err(display->drm_fd, DRM_IOCTL_MODE_ATOMIC, &ioc, EFAULT);
 }
 
+static void atomic_plane_damage(igt_pipe_t *pipe, igt_plane_t *plane, struct igt_fb *fb)
+{
+	struct damage_rect *damage;
+	struct igt_fb fb_1;
+	struct igt_fb fb_2;
+	cairo_t *cr_1;
+	cairo_t *cr_2;
+
+	damage = malloc(sizeof(*damage) * 2);
+	igt_assert(damage);
+
+	/* Color fb with white rect at center */
+	igt_create_color_fb(pipe->display->drm_fd, fb->width, fb->height,
+			    fb->drm_format, I915_TILING_NONE, 0.2, 0.2, 0.2,
+			    &fb_1);
+	cr_1 = igt_get_cairo_ctx(pipe->display->drm_fd, &fb_1);
+	igt_paint_color(cr_1, fb->width/4, fb->height/4, fb->width/2,
+			fb->height/2, 1.0, 1.0, 1.0);
+	igt_put_cairo_ctx(pipe->display->drm_fd, &fb_1, cr_1);
+
+	/*
+	 * Flip the primary plane to new color fb using atomic API and check the
+	 * state.
+	 */
+	igt_plane_set_fb(plane, &fb_1);
+	crtc_commit(pipe, plane, COMMIT_ATOMIC, ATOMIC_RELAX_NONE);
+
+	/*
+	 * Change the color of top left clip from center and issue plane update
+	 * with damage and verify the state.
+	 */
+	damage[0].x1 = 0;
+	damage[0].y1 = 0;
+	damage[0].x2 = fb->width/2;
+	damage[0].y2 = fb->height/2;
+
+	cr_1 = igt_get_cairo_ctx(pipe->display->drm_fd, &fb_1);
+	igt_paint_color(cr_1, damage[0].x1, damage[0].y1,
+			damage_rect_width(&damage[0]),
+			damage_rect_height(&damage[0]), 1.0, 0, 0);
+	igt_put_cairo_ctx(pipe->display->drm_fd, &fb_1, cr_1);
+
+	igt_plane_set_fb(plane, &fb_1);
+	igt_plane_replace_prop_blob(plane, IGT_PLANE_FB_DAMAGE_CLIPS, damage,
+				    sizeof(*damage));
+	crtc_commit(pipe, plane, COMMIT_ATOMIC, ATOMIC_RELAX_NONE);
+
+	/*
+	 * Change the color of top left and bottom right clip from center and
+	 * issue plane update with damage and verify the state.
+	 */
+	damage[0].x1 = 0;
+	damage[0].y1 = 0;
+	damage[0].x2 = fb->width/2;
+	damage[0].y2 = fb->height/2;
+
+	damage[1].x1 = fb->width/2;
+	damage[1].y1 = fb->height/2;
+	damage[1].x2 = fb->width;
+	damage[1].y2 = fb->height;
+
+	cr_1 = igt_get_cairo_ctx(pipe->display->drm_fd, &fb_1);
+	igt_paint_color(cr_1, damage[0].x1, damage[0].y1,
+			damage_rect_width(&damage[0]),
+			damage_rect_height(&damage[0]), 1.0, 0, 1.0);
+	igt_paint_color(cr_1, damage[1].x1, damage[1].y1,
+			damage_rect_width(&damage[1]),
+			damage_rect_height(&damage[1]), 0, 0, 1.0);
+	igt_put_cairo_ctx(pipe->display->drm_fd, &fb_1, cr_1);
+
+	igt_plane_set_fb(plane, &fb_1);
+	igt_plane_replace_prop_blob(plane, IGT_PLANE_FB_DAMAGE_CLIPS, damage,
+				    sizeof(*damage) * 2);
+	crtc_commit(pipe, plane, COMMIT_ATOMIC, ATOMIC_RELAX_NONE);
+
+	/*
+	 * Issue a plane update with damage on a seperate fb with damage to
+	 * upper right clip from center.
+	 */
+	igt_create_color_fb(pipe->display->drm_fd, fb->width, fb->height,
+			    fb->drm_format, I915_TILING_NONE, 0.2, 0.2, 0.2,
+			    &fb_2);
+
+	damage[0].x1 = fb->width/2;
+	damage[0].y1 = 0;
+	damage[0].x2 = fb->width;
+	damage[0].y2 = fb->height/2;
+
+	cr_2 = igt_get_cairo_ctx(pipe->display->drm_fd, &fb_2);
+	cr_1 = igt_get_cairo_ctx(pipe->display->drm_fd, &fb_1);
+	cairo_set_source_surface(cr_2, fb_1.cairo_surface, 0, 0);
+	cairo_paint(cr_2);
+	igt_paint_color(cr_2, damage[0].x1, damage[0].y1,
+			damage_rect_width(&damage[0]),
+			damage_rect_height(&damage[0]), 0, 1.0, 0);
+	igt_put_cairo_ctx(pipe->display->drm_fd, &fb_1, cr_1);
+	igt_put_cairo_ctx(pipe->display->drm_fd, &fb_2, cr_2);
+	igt_plane_set_fb(plane, &fb_2);
+	igt_plane_replace_prop_blob(plane, IGT_PLANE_FB_DAMAGE_CLIPS, damage,
+				    sizeof(*damage));
+	crtc_commit(pipe, plane, COMMIT_ATOMIC, ATOMIC_RELAX_NONE);
+
+	/*
+	 * Issue plane update with damage with a clip outside of plane src.
+	 * NOTE: This will result in no update on plane as damage is outside, so
+	 * will see no change on the screen.
+	 */
+	/* Reszie fb_1 to be bigger than plane */
+	igt_remove_fb(pipe->display->drm_fd, &fb_1);
+	igt_create_color_fb(pipe->display->drm_fd, fb->width * 2, fb->height,
+			    fb->drm_format, I915_TILING_NONE, 0.2, 0.2, 0.2,
+			    &fb_1);
+
+	damage[0].x1 = fb->width;
+	damage[0].y1 = 0;
+	damage[0].x2 = fb->width + fb->width/2;
+	damage[0].y2 = fb->height/2;
+
+	cr_1 = igt_get_cairo_ctx(pipe->display->drm_fd, &fb_1);
+	cr_2 = igt_get_cairo_ctx(pipe->display->drm_fd, &fb_2);
+	cairo_set_source_surface(cr_1, fb_2.cairo_surface, 0, 0);
+	cairo_paint(cr_1);
+	igt_paint_color(cr_1, damage[0].x1, damage[0].y1,
+			damage_rect_width(&damage[0]),
+			damage_rect_height(&damage[0]), 0, 1.0, 0);
+	igt_put_cairo_ctx(pipe->display->drm_fd, &fb_2, cr_2);
+	igt_put_cairo_ctx(pipe->display->drm_fd, &fb_1, cr_1);
+	igt_plane_set_fb(plane, &fb_1);
+	igt_plane_set_size(plane, fb->width, fb->height);
+	igt_fb_set_position(&fb_1, plane, 0, 0);
+	igt_fb_set_size(&fb_1, plane, fb->width, fb->height);
+	igt_plane_replace_prop_blob(plane, IGT_PLANE_FB_DAMAGE_CLIPS, damage,
+				    sizeof(*damage));
+	crtc_commit(pipe, plane, COMMIT_ATOMIC, ATOMIC_RELAX_NONE);
+
+	/*
+	 * Issue a plane update with damage with a clip that overlap with plane
+	 * src (Top right from center extending outside src in below case).
+	 * NOTE: Here drm core should take care of intersecting the clip to
+	 * plane src.
+	 */
+	damage[0].x1 = fb->width/2;
+	damage[0].y1 = 0;
+	damage[0].x2 = fb->width/2 + fb->width;
+	damage[0].y2 = fb->height/2;
+
+	cr_1 = igt_get_cairo_ctx(pipe->display->drm_fd, &fb_1);
+	igt_paint_color(cr_1, damage[0].x1, damage[0].y1,
+			damage_rect_width(&damage[0]),
+			damage_rect_height(&damage[0]), 1.0, 1.0, 0);
+	igt_put_cairo_ctx(pipe->display->drm_fd, &fb_1, cr_1);
+	igt_plane_set_fb(plane, &fb_1);
+	igt_plane_set_size(plane, fb->width, fb->height);
+	igt_fb_set_position(&fb_1, plane, 0, 0);
+	igt_fb_set_size(&fb_1, plane, fb->width, fb->height);
+	igt_plane_replace_prop_blob(plane, IGT_PLANE_FB_DAMAGE_CLIPS, damage,
+				    sizeof(*damage));
+	crtc_commit(pipe, plane, COMMIT_ATOMIC, ATOMIC_RELAX_NONE);
+
+	/*
+	 * Issue a plane update with damage with two clips one inside plane src
+	 * and one outside
+	 * NOTE: This will result in plane update with clip inside plane src.
+	 */
+	damage[0].x1 = 0;
+	damage[0].y1 = fb->height/2;
+	damage[0].x2 = fb->width/2;
+	damage[0].y2 = fb->height;
+
+	damage[1].x1 = fb->width + fb->width/2;
+	damage[1].y1 = fb->height/2;
+	damage[1].x2 = fb->width * 2;
+	damage[1].y2 = fb->height;
+
+	cr_1 = igt_get_cairo_ctx(pipe->display->drm_fd, &fb_1);
+	igt_paint_color(cr_1, damage[0].x1, damage[0].y1,
+			damage_rect_width(&damage[0]),
+			damage_rect_height(&damage[0]), 0, 1.0, 1.0);
+	igt_paint_color(cr_1, damage[1].x1, damage[1].y1,
+			damage_rect_width(&damage[1]),
+			damage_rect_height(&damage[1]), 0, 1.0, 0);
+	igt_put_cairo_ctx(pipe->display->drm_fd, &fb_1, cr_1);
+	igt_plane_set_fb(plane, &fb_1);
+	igt_plane_set_size(plane, fb->width, fb->height);
+	igt_fb_set_position(&fb_1, plane, 0, 0);
+	igt_fb_set_size(&fb_1, plane, fb->width, fb->height);
+	igt_plane_replace_prop_blob(plane, IGT_PLANE_FB_DAMAGE_CLIPS, damage,
+				    sizeof(*damage) * 2);
+	crtc_commit(pipe, plane, COMMIT_ATOMIC, ATOMIC_RELAX_NONE);
+
+	/*
+	 * Issue a plane update with overlapping damage clips. White rect in
+	 * center overlap partially with top left red rect.
+	 * NOTE: Drm core does not error for overlapping damage clips so if any
+	 * driver does not support overlapping should have their own
+	 * validations.
+	 */
+	damage[0].x1 = 0;
+	damage[0].y1 = 0;
+	damage[0].x2 = fb->width/2;
+	damage[0].y2 = fb->height/2;
+
+	damage[1].x1 = fb->width/4;
+	damage[1].y1 = fb->height/4;
+	damage[1].x2 = fb->width/4 + fb->width/2;
+	damage[1].y2 = fb->height/4 + fb->height/2;
+
+	cr_1 = igt_get_cairo_ctx(pipe->display->drm_fd, &fb_1);
+	igt_paint_color(cr_1, damage[0].x1, damage[0].y1,
+			damage_rect_width(&damage[0]),
+			damage_rect_height(&damage[0]), 1.0, 0, 0);
+	igt_paint_color(cr_1, damage[1].x1, damage[1].y1,
+			damage_rect_width(&damage[1]),
+			damage_rect_height(&damage[1]), 1.0, 1.0, 1.0);
+	igt_put_cairo_ctx(pipe->display->drm_fd, &fb_1, cr_1);
+	igt_plane_set_fb(plane, &fb_1);
+	igt_plane_set_size(plane, fb->width, fb->height);
+	igt_fb_set_position(&fb_1, plane, 0, 0);
+	igt_fb_set_size(&fb_1, plane, fb->width, fb->height);
+	igt_plane_replace_prop_blob(plane, IGT_PLANE_FB_DAMAGE_CLIPS, damage,
+				    sizeof(*damage) * 2);
+	crtc_commit(pipe, plane, COMMIT_ATOMIC, ATOMIC_RELAX_NONE);
+
+	/* Restore the primary plane */
+	igt_plane_set_fb(plane, fb);
+	plane_commit(plane, COMMIT_ATOMIC, ATOMIC_RELAX_NONE);
+
+	/* Remove the fb created for this test */
+	igt_remove_fb(pipe->display->drm_fd, &fb_1);
+	igt_remove_fb(pipe->display->drm_fd, &fb_2);
+
+	free(damage);
+}
+
 static void atomic_setup(igt_display_t *display, enum pipe pipe, igt_output_t *output, igt_plane_t *primary, struct igt_fb *fb)
 {
 	igt_output_set_pipe(output, pipe);
@@ -950,6 +1202,14 @@ igt_main
 		atomic_invalid_params(pipe_obj, primary, output, &fb);
 	}
 
+	igt_subtest("atomic_plane_damage") {
+		igt_require(igt_plane_has_prop(primary, IGT_PLANE_FB_DAMAGE_CLIPS));
+
+		atomic_setup(&display, pipe, output, primary, &fb);
+
+		atomic_plane_damage(pipe_obj, primary, &fb);
+	}
+
 	igt_fixture {
 		atomic_clear(&display, pipe, primary, output);
 		igt_remove_fb(display.drm_fd, &fb);
-- 
2.17.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH i-g-t 6/6] tests/plane_damage: Integrate kernel selftest test-drm_damage_helper

[Deepak Rawat]

Call kernel selftest module test-drm_damage_helper from igt.

v2:
- Add test alphabetically.
- Add test to meson build.

Cc: ville.syrjala@linux.intel.com
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Pekka Paalanen <ppaalanen@gmail.com>
Cc: Daniel Stone <daniel@fooishbar.org>
Signed-off-by: Deepak Rawat <drawat@vmware.com>
---
 tests/Makefile.sources    |  1 +
 tests/drm_plane_damage.c  | 10 ++++++++++
 tests/igt_command_line.sh |  2 +-
 tests/meson.build         |  1 +
 4 files changed, 13 insertions(+), 1 deletion(-)
 create mode 100644 tests/drm_plane_damage.c

diff --git a/tests/Makefile.sources b/tests/Makefile.sources
index 001f1a2b..eae5f39e 100644
--- a/tests/Makefile.sources
+++ b/tests/Makefile.sources
@@ -32,6 +32,7 @@ TESTS_progs = \
 	debugfs_test \
 	drm_import_export \
 	drm_mm \
+	drm_plane_damage \
 	drm_read \
 	drv_getparams_basic \
 	drv_hangman \
diff --git a/tests/drm_plane_damage.c b/tests/drm_plane_damage.c
new file mode 100644
index 00000000..c2b793cc
--- /dev/null
+++ b/tests/drm_plane_damage.c
@@ -0,0 +1,10 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#include "igt.h"
+#include "igt_kmod.h"
+
+IGT_TEST_DESCRIPTION("Basic sanity check of DRM's plane damage helper iterator.");
+
+igt_main
+{
+	igt_kselftests("test-drm_damage_helper", NULL, NULL, NULL);
+}
diff --git a/tests/igt_command_line.sh b/tests/igt_command_line.sh
index 8285ba62..621b2cbd 100755
--- a/tests/igt_command_line.sh
+++ b/tests/igt_command_line.sh
@@ -90,7 +90,7 @@ check_test ()
 		# Subtest enumeration of kernel selftest launchers depends
 		# on the running kernel. If selftests are not enabled,
 		# they will output nothing and exit with 0.
-		if [ "$testname" != "drv_selftest" -a "$testname" != "drm_mm" ]; then
+		if [ "$testname" != "drv_selftest" -a "$testname" != "drm_mm" -a "$testname" != "drm_plane_damage" ]; then
 			fail $test
 		fi
 	fi
diff --git a/tests/meson.build b/tests/meson.build
index 697ff515..5acd7aa2 100644
--- a/tests/meson.build
+++ b/tests/meson.build
@@ -9,6 +9,7 @@ test_progs = [
 	'debugfs_test',
 	'drm_import_export',
 	'drm_mm',
+	'drm_plane_damage',
 	'drm_read',
 	'drv_getparams_basic',
 	'drv_hangman',
-- 
2.17.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[Intel-gfx] [PATCH i-g-t 6/6] tests/plane_damage: Integrate kernel selftest test-drm_damage_helper

[Deepak Rawat]

Call kernel selftest module test-drm_damage_helper from igt.

v2:
- Add test alphabetically.
- Add test to meson build.

Cc: ville.syrjala@linux.intel.com
Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Pekka Paalanen <ppaalanen@gmail.com>
Cc: Daniel Stone <daniel@fooishbar.org>
Signed-off-by: Deepak Rawat <drawat@vmware.com>
---
 tests/Makefile.sources    |  1 +
 tests/drm_plane_damage.c  | 10 ++++++++++
 tests/igt_command_line.sh |  2 +-
 tests/meson.build         |  1 +
 4 files changed, 13 insertions(+), 1 deletion(-)
 create mode 100644 tests/drm_plane_damage.c

diff --git a/tests/Makefile.sources b/tests/Makefile.sources
index 001f1a2b..eae5f39e 100644
--- a/tests/Makefile.sources
+++ b/tests/Makefile.sources
@@ -32,6 +32,7 @@ TESTS_progs = \
 	debugfs_test \
 	drm_import_export \
 	drm_mm \
+	drm_plane_damage \
 	drm_read \
 	drv_getparams_basic \
 	drv_hangman \
diff --git a/tests/drm_plane_damage.c b/tests/drm_plane_damage.c
new file mode 100644
index 00000000..c2b793cc
--- /dev/null
+++ b/tests/drm_plane_damage.c
@@ -0,0 +1,10 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#include "igt.h"
+#include "igt_kmod.h"
+
+IGT_TEST_DESCRIPTION("Basic sanity check of DRM's plane damage helper iterator.");
+
+igt_main
+{
+	igt_kselftests("test-drm_damage_helper", NULL, NULL, NULL);
+}
diff --git a/tests/igt_command_line.sh b/tests/igt_command_line.sh
index 8285ba62..621b2cbd 100755
--- a/tests/igt_command_line.sh
+++ b/tests/igt_command_line.sh
@@ -90,7 +90,7 @@ check_test ()
 		# Subtest enumeration of kernel selftest launchers depends
 		# on the running kernel. If selftests are not enabled,
 		# they will output nothing and exit with 0.
-		if [ "$testname" != "drv_selftest" -a "$testname" != "drm_mm" ]; then
+		if [ "$testname" != "drv_selftest" -a "$testname" != "drm_mm" -a "$testname" != "drm_plane_damage" ]; then
 			fail $test
 		fi
 	fi
diff --git a/tests/meson.build b/tests/meson.build
index 697ff515..5acd7aa2 100644
--- a/tests/meson.build
+++ b/tests/meson.build
@@ -9,6 +9,7 @@ test_progs = [
 	'debugfs_test',
 	'drm_import_export',
 	'drm_mm',
+	'drm_plane_damage',
 	'drm_read',
 	'drv_getparams_basic',
 	'drv_hangman',
-- 
2.17.1

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [igt-dev] [PATCH i-g-t 1/6] lib/igt_vmwgfx: Add vmwgfx device

[Daniel Vetter]

On Wed, Oct 10, 2018 at 05:20:59PM -0700, Deepak Rawat wrote:
> Add DRIVER_VMWGFX to represent vmwgfx device for running igt tests.
> 
> v2: Don't remove second virtio_gpu
> 
> Signed-off-by: Deepak Rawat <drawat@vmware.com>
> ---
>  lib/drmtest.c | 8 ++++++++
>  lib/drmtest.h | 3 +++
>  2 files changed, 11 insertions(+)
> 
> diff --git a/lib/drmtest.c b/lib/drmtest.c
> index fee9d33a..9d013a00 100644
> --- a/lib/drmtest.c
> +++ b/lib/drmtest.c
> @@ -105,6 +105,11 @@ bool is_i915_device(int fd)
>  	return __is_device(fd, "i915");
>  }
>  
> +bool is_vmwgfx_device(int fd)
> +{
> +	return __is_device(fd, "vmwg");
> +}
> +
>  static bool has_known_intel_chipset(int fd)
>  {
>  	struct drm_i915_getparam gp;
> @@ -206,6 +211,7 @@ static const struct module {
>  	{ DRIVER_VGEM, "vgem" },
>  	{ DRIVER_VIRTIO, "virtio-gpu" },
>  	{ DRIVER_VIRTIO, "virtio_gpu" },
> +	{ DRIVER_VMWGFX, "vmwgfx" },
>  	{}
>  };
>  
> @@ -348,6 +354,8 @@ static const char *chipset_to_str(int chipset)
>  		return "virtio";
>  	case DRIVER_AMDGPU:
>  		return "amdgpu";
> +	case DRIVER_VMWGFX:
> +		return "vmwgfx";
>  	case DRIVER_ANY:
>  		return "any";
>  	default:
> diff --git a/lib/drmtest.h b/lib/drmtest.h
> index 949865ee..0213fb51 100644
> --- a/lib/drmtest.h
> +++ b/lib/drmtest.h
> @@ -43,6 +43,7 @@
>  #define DRIVER_VGEM	(1 << 2)
>  #define DRIVER_VIRTIO	(1 << 3)
>  #define DRIVER_AMDGPU	(1 << 4)
> +#define DRIVER_VMWGFX	(1 << 5)

This seems not needed? For pure generic kms tests I think it'd be great if
we don't have to sprinkle driver-specific checks all over. Which you seem
to achive in your series here.

So not clear why this here is needed?
-Daniel

>  /*
>   * Exclude DRVER_VGEM from DRIVER_ANY since if you run on a system
>   * with vgem as well as a supported driver, you can end up with a
> @@ -80,6 +81,8 @@ void igt_require_intel(int fd);
>  
>  bool is_i915_device(int fd);
>  
> +bool is_vmwgfx_device(int fd);
> +
>  /**
>   * do_or_die:
>   * @x: command
> -- 
> 2.17.1
> 
> _______________________________________________
> igt-dev mailing list
> igt-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/igt-dev

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

Re: [igt-dev] [PATCH i-g-t 1/6] lib/igt_vmwgfx: Add vmwgfx device

[Daniel Vetter]

On Wed, Oct 10, 2018 at 05:20:59PM -0700, Deepak Rawat wrote:
> Add DRIVER_VMWGFX to represent vmwgfx device for running igt tests.
> 
> v2: Don't remove second virtio_gpu
> 
> Signed-off-by: Deepak Rawat <drawat@vmware.com>
> ---
>  lib/drmtest.c | 8 ++++++++
>  lib/drmtest.h | 3 +++
>  2 files changed, 11 insertions(+)
> 
> diff --git a/lib/drmtest.c b/lib/drmtest.c
> index fee9d33a..9d013a00 100644
> --- a/lib/drmtest.c
> +++ b/lib/drmtest.c
> @@ -105,6 +105,11 @@ bool is_i915_device(int fd)
>  	return __is_device(fd, "i915");
>  }
>  
> +bool is_vmwgfx_device(int fd)
> +{
> +	return __is_device(fd, "vmwg");
> +}
> +
>  static bool has_known_intel_chipset(int fd)
>  {
>  	struct drm_i915_getparam gp;
> @@ -206,6 +211,7 @@ static const struct module {
>  	{ DRIVER_VGEM, "vgem" },
>  	{ DRIVER_VIRTIO, "virtio-gpu" },
>  	{ DRIVER_VIRTIO, "virtio_gpu" },
> +	{ DRIVER_VMWGFX, "vmwgfx" },
>  	{}
>  };
>  
> @@ -348,6 +354,8 @@ static const char *chipset_to_str(int chipset)
>  		return "virtio";
>  	case DRIVER_AMDGPU:
>  		return "amdgpu";
> +	case DRIVER_VMWGFX:
> +		return "vmwgfx";
>  	case DRIVER_ANY:
>  		return "any";
>  	default:
> diff --git a/lib/drmtest.h b/lib/drmtest.h
> index 949865ee..0213fb51 100644
> --- a/lib/drmtest.h
> +++ b/lib/drmtest.h
> @@ -43,6 +43,7 @@
>  #define DRIVER_VGEM	(1 << 2)
>  #define DRIVER_VIRTIO	(1 << 3)
>  #define DRIVER_AMDGPU	(1 << 4)
> +#define DRIVER_VMWGFX	(1 << 5)

This seems not needed? For pure generic kms tests I think it'd be great if
we don't have to sprinkle driver-specific checks all over. Which you seem
to achive in your series here.

So not clear why this here is needed?
-Daniel

>  /*
>   * Exclude DRVER_VGEM from DRIVER_ANY since if you run on a system
>   * with vgem as well as a supported driver, you can end up with a
> @@ -80,6 +81,8 @@ void igt_require_intel(int fd);
>  
>  bool is_i915_device(int fd);
>  
> +bool is_vmwgfx_device(int fd);
> +
>  /**
>   * do_or_die:
>   * @x: command
> -- 
> 2.17.1
> 
> _______________________________________________
> igt-dev mailing list
> igt-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/igt-dev

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

Re: [PATCH i-g-t 6/6] tests/plane_damage: Integrate kernel selftest test-drm_damage_helper

[Daniel Vetter]

On Wed, Oct 10, 2018 at 05:21:04PM -0700, Deepak Rawat wrote:
> Call kernel selftest module test-drm_damage_helper from igt.
> 
> v2:
> - Add test alphabetically.
> - Add test to meson build.
> 
> Cc: ville.syrjala@linux.intel.com
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Cc: Pekka Paalanen <ppaalanen@gmail.com>
> Cc: Daniel Stone <daniel@fooishbar.org>
> Signed-off-by: Deepak Rawat <drawat@vmware.com>
> ---
>  tests/Makefile.sources    |  1 +
>  tests/drm_plane_damage.c  | 10 ++++++++++
>  tests/igt_command_line.sh |  2 +-
>  tests/meson.build         |  1 +
>  4 files changed, 13 insertions(+), 1 deletion(-)
>  create mode 100644 tests/drm_plane_damage.c
> 
> diff --git a/tests/Makefile.sources b/tests/Makefile.sources
> index 001f1a2b..eae5f39e 100644
> --- a/tests/Makefile.sources
> +++ b/tests/Makefile.sources
> @@ -32,6 +32,7 @@ TESTS_progs = \
>  	debugfs_test \
>  	drm_import_export \
>  	drm_mm \
> +	drm_plane_damage \
>  	drm_read \
>  	drv_getparams_basic \
>  	drv_hangman \
> diff --git a/tests/drm_plane_damage.c b/tests/drm_plane_damage.c
> new file mode 100644
> index 00000000..c2b793cc
> --- /dev/null
> +++ b/tests/drm_plane_damage.c
> @@ -0,0 +1,10 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +#include "igt.h"
> +#include "igt_kmod.h"
> +
> +IGT_TEST_DESCRIPTION("Basic sanity check of DRM's plane damage helper iterator.");
> +
> +igt_main
> +{
> +	igt_kselftests("test-drm_damage_helper", NULL, NULL, NULL);
> +}
> diff --git a/tests/igt_command_line.sh b/tests/igt_command_line.sh
> index 8285ba62..621b2cbd 100755
> --- a/tests/igt_command_line.sh
> +++ b/tests/igt_command_line.sh
> @@ -90,7 +90,7 @@ check_test ()
>  		# Subtest enumeration of kernel selftest launchers depends
>  		# on the running kernel. If selftests are not enabled,
>  		# they will output nothing and exit with 0.
> -		if [ "$testname" != "drv_selftest" -a "$testname" != "drm_mm" ]; then
> +		if [ "$testname" != "drv_selftest" -a "$testname" != "drm_mm" -a "$testname" != "drm_plane_damage" ]; then
>  			fail $test
>  		fi
>  	fi
> diff --git a/tests/meson.build b/tests/meson.build
> index 697ff515..5acd7aa2 100644
> --- a/tests/meson.build
> +++ b/tests/meson.build
> @@ -9,6 +9,7 @@ test_progs = [
>  	'debugfs_test',
>  	'drm_import_export',
>  	'drm_mm',
> +	'drm_plane_damage',

For future proofing I think it'd be much better if we call this drm_kms or
similar. The individual subtest results will be all exposed, but there's a
bit a problem when we always have to upgrade both igt and the kernel at
the same time. At least with the current CI infrastructure.

I'm also asking the ARM folks to type selftests for the new block_* format
description stuff, so this will come in handy real soon.
-Daniel

>  	'drm_read',
>  	'drv_getparams_basic',
>  	'drv_hangman',
> -- 
> 2.17.1
> 

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

Re: [Intel-gfx] [PATCH i-g-t 6/6] tests/plane_damage: Integrate kernel selftest test-drm_damage_helper

[Daniel Vetter]

On Wed, Oct 10, 2018 at 05:21:04PM -0700, Deepak Rawat wrote:
> Call kernel selftest module test-drm_damage_helper from igt.
> 
> v2:
> - Add test alphabetically.
> - Add test to meson build.
> 
> Cc: ville.syrjala@linux.intel.com
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Cc: Pekka Paalanen <ppaalanen@gmail.com>
> Cc: Daniel Stone <daniel@fooishbar.org>
> Signed-off-by: Deepak Rawat <drawat@vmware.com>
> ---
>  tests/Makefile.sources    |  1 +
>  tests/drm_plane_damage.c  | 10 ++++++++++
>  tests/igt_command_line.sh |  2 +-
>  tests/meson.build         |  1 +
>  4 files changed, 13 insertions(+), 1 deletion(-)
>  create mode 100644 tests/drm_plane_damage.c
> 
> diff --git a/tests/Makefile.sources b/tests/Makefile.sources
> index 001f1a2b..eae5f39e 100644
> --- a/tests/Makefile.sources
> +++ b/tests/Makefile.sources
> @@ -32,6 +32,7 @@ TESTS_progs = \
>  	debugfs_test \
>  	drm_import_export \
>  	drm_mm \
> +	drm_plane_damage \
>  	drm_read \
>  	drv_getparams_basic \
>  	drv_hangman \
> diff --git a/tests/drm_plane_damage.c b/tests/drm_plane_damage.c
> new file mode 100644
> index 00000000..c2b793cc
> --- /dev/null
> +++ b/tests/drm_plane_damage.c
> @@ -0,0 +1,10 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +#include "igt.h"
> +#include "igt_kmod.h"
> +
> +IGT_TEST_DESCRIPTION("Basic sanity check of DRM's plane damage helper iterator.");
> +
> +igt_main
> +{
> +	igt_kselftests("test-drm_damage_helper", NULL, NULL, NULL);
> +}
> diff --git a/tests/igt_command_line.sh b/tests/igt_command_line.sh
> index 8285ba62..621b2cbd 100755
> --- a/tests/igt_command_line.sh
> +++ b/tests/igt_command_line.sh
> @@ -90,7 +90,7 @@ check_test ()
>  		# Subtest enumeration of kernel selftest launchers depends
>  		# on the running kernel. If selftests are not enabled,
>  		# they will output nothing and exit with 0.
> -		if [ "$testname" != "drv_selftest" -a "$testname" != "drm_mm" ]; then
> +		if [ "$testname" != "drv_selftest" -a "$testname" != "drm_mm" -a "$testname" != "drm_plane_damage" ]; then
>  			fail $test
>  		fi
>  	fi
> diff --git a/tests/meson.build b/tests/meson.build
> index 697ff515..5acd7aa2 100644
> --- a/tests/meson.build
> +++ b/tests/meson.build
> @@ -9,6 +9,7 @@ test_progs = [
>  	'debugfs_test',
>  	'drm_import_export',
>  	'drm_mm',
> +	'drm_plane_damage',

For future proofing I think it'd be much better if we call this drm_kms or
similar. The individual subtest results will be all exposed, but there's a
bit a problem when we always have to upgrade both igt and the kernel at
the same time. At least with the current CI infrastructure.

I'm also asking the ARM folks to type selftests for the new block_* format
description stuff, so this will come in handy real soon.
-Daniel

>  	'drm_read',
>  	'drv_getparams_basic',
>  	'drv_hangman',
> -- 
> 2.17.1
> 

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

Re: [igt-dev] [PATCH i-g-t 1/6] lib/igt_vmwgfx: Add vmwgfx device

[Petri Latvala]

On Thu, Oct 11, 2018 at 11:01:48AM +0200, Daniel Vetter wrote:
> On Wed, Oct 10, 2018 at 05:20:59PM -0700, Deepak Rawat wrote:
> > Add DRIVER_VMWGFX to represent vmwgfx device for running igt tests.
> > 
> > v2: Don't remove second virtio_gpu
> > 
> > Signed-off-by: Deepak Rawat <drawat@vmware.com>
> > ---
> >  lib/drmtest.c | 8 ++++++++
> >  lib/drmtest.h | 3 +++
> >  2 files changed, 11 insertions(+)
> > 
> > diff --git a/lib/drmtest.c b/lib/drmtest.c
> > index fee9d33a..9d013a00 100644
> > --- a/lib/drmtest.c
> > +++ b/lib/drmtest.c
> > @@ -105,6 +105,11 @@ bool is_i915_device(int fd)
> >  	return __is_device(fd, "i915");
> >  }
> >  
> > +bool is_vmwgfx_device(int fd)
> > +{
> > +	return __is_device(fd, "vmwg");
> > +}
> > +
> >  static bool has_known_intel_chipset(int fd)
> >  {
> >  	struct drm_i915_getparam gp;
> > @@ -206,6 +211,7 @@ static const struct module {
> >  	{ DRIVER_VGEM, "vgem" },
> >  	{ DRIVER_VIRTIO, "virtio-gpu" },
> >  	{ DRIVER_VIRTIO, "virtio_gpu" },
> > +	{ DRIVER_VMWGFX, "vmwgfx" },
> >  	{}
> >  };
> >  
> > @@ -348,6 +354,8 @@ static const char *chipset_to_str(int chipset)
> >  		return "virtio";
> >  	case DRIVER_AMDGPU:
> >  		return "amdgpu";
> > +	case DRIVER_VMWGFX:
> > +		return "vmwgfx";
> >  	case DRIVER_ANY:
> >  		return "any";
> >  	default:
> > diff --git a/lib/drmtest.h b/lib/drmtest.h
> > index 949865ee..0213fb51 100644
> > --- a/lib/drmtest.h
> > +++ b/lib/drmtest.h
> > @@ -43,6 +43,7 @@
> >  #define DRIVER_VGEM	(1 << 2)
> >  #define DRIVER_VIRTIO	(1 << 3)
> >  #define DRIVER_AMDGPU	(1 << 4)
> > +#define DRIVER_VMWGFX	(1 << 5)
> 
> This seems not needed? For pure generic kms tests I think it'd be great if
> we don't have to sprinkle driver-specific checks all over. Which you seem
> to achive in your series here.
> 
> So not clear why this here is needed?


Loading the .ko needs the name in this array:

> > @@ -206,6 +211,7 @@ static const struct module {
> >  	{ DRIVER_VGEM, "vgem" },
> >  	{ DRIVER_VIRTIO, "virtio-gpu" },
> >  	{ DRIVER_VIRTIO, "virtio_gpu" },
> > +	{ DRIVER_VMWGFX, "vmwgfx" },



-- 
Petri Latvala




> -Daniel
> 
> >  /*
> >   * Exclude DRVER_VGEM from DRIVER_ANY since if you run on a system
> >   * with vgem as well as a supported driver, you can end up with a
> > @@ -80,6 +81,8 @@ void igt_require_intel(int fd);
> >  
> >  bool is_i915_device(int fd);
> >  
> > +bool is_vmwgfx_device(int fd);
> > +
> >  /**
> >   * do_or_die:
> >   * @x: command
> > -- 
> > 2.17.1
> > 
> > _______________________________________________
> > igt-dev mailing list
> > igt-dev@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/igt-dev
> 
> -- 
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch
> _______________________________________________
> igt-dev mailing list
> igt-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/igt-dev
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [igt-dev] [PATCH i-g-t 1/6] lib/igt_vmwgfx: Add vmwgfx device

[Petri Latvala]

On Thu, Oct 11, 2018 at 11:01:48AM +0200, Daniel Vetter wrote:
> On Wed, Oct 10, 2018 at 05:20:59PM -0700, Deepak Rawat wrote:
> > Add DRIVER_VMWGFX to represent vmwgfx device for running igt tests.
> > 
> > v2: Don't remove second virtio_gpu
> > 
> > Signed-off-by: Deepak Rawat <drawat@vmware.com>
> > ---
> >  lib/drmtest.c | 8 ++++++++
> >  lib/drmtest.h | 3 +++
> >  2 files changed, 11 insertions(+)
> > 
> > diff --git a/lib/drmtest.c b/lib/drmtest.c
> > index fee9d33a..9d013a00 100644
> > --- a/lib/drmtest.c
> > +++ b/lib/drmtest.c
> > @@ -105,6 +105,11 @@ bool is_i915_device(int fd)
> >  	return __is_device(fd, "i915");
> >  }
> >  
> > +bool is_vmwgfx_device(int fd)
> > +{
> > +	return __is_device(fd, "vmwg");
> > +}
> > +
> >  static bool has_known_intel_chipset(int fd)
> >  {
> >  	struct drm_i915_getparam gp;
> > @@ -206,6 +211,7 @@ static const struct module {
> >  	{ DRIVER_VGEM, "vgem" },
> >  	{ DRIVER_VIRTIO, "virtio-gpu" },
> >  	{ DRIVER_VIRTIO, "virtio_gpu" },
> > +	{ DRIVER_VMWGFX, "vmwgfx" },
> >  	{}
> >  };
> >  
> > @@ -348,6 +354,8 @@ static const char *chipset_to_str(int chipset)
> >  		return "virtio";
> >  	case DRIVER_AMDGPU:
> >  		return "amdgpu";
> > +	case DRIVER_VMWGFX:
> > +		return "vmwgfx";
> >  	case DRIVER_ANY:
> >  		return "any";
> >  	default:
> > diff --git a/lib/drmtest.h b/lib/drmtest.h
> > index 949865ee..0213fb51 100644
> > --- a/lib/drmtest.h
> > +++ b/lib/drmtest.h
> > @@ -43,6 +43,7 @@
> >  #define DRIVER_VGEM	(1 << 2)
> >  #define DRIVER_VIRTIO	(1 << 3)
> >  #define DRIVER_AMDGPU	(1 << 4)
> > +#define DRIVER_VMWGFX	(1 << 5)
> 
> This seems not needed? For pure generic kms tests I think it'd be great if
> we don't have to sprinkle driver-specific checks all over. Which you seem
> to achive in your series here.
> 
> So not clear why this here is needed?


Loading the .ko needs the name in this array:

> > @@ -206,6 +211,7 @@ static const struct module {
> >  	{ DRIVER_VGEM, "vgem" },
> >  	{ DRIVER_VIRTIO, "virtio-gpu" },
> >  	{ DRIVER_VIRTIO, "virtio_gpu" },
> > +	{ DRIVER_VMWGFX, "vmwgfx" },



-- 
Petri Latvala




> -Daniel
> 
> >  /*
> >   * Exclude DRVER_VGEM from DRIVER_ANY since if you run on a system
> >   * with vgem as well as a supported driver, you can end up with a
> > @@ -80,6 +81,8 @@ void igt_require_intel(int fd);
> >  
> >  bool is_i915_device(int fd);
> >  
> > +bool is_vmwgfx_device(int fd);
> > +
> >  /**
> >   * do_or_die:
> >   * @x: command
> > -- 
> > 2.17.1
> > 
> > _______________________________________________
> > igt-dev mailing list
> > igt-dev@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/igt-dev
> 
> -- 
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch
> _______________________________________________
> igt-dev mailing list
> igt-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/igt-dev
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

[igt-dev] ✓ Fi.CI.BAT: success for series starting with [i-g-t,1/6] lib/igt_vmwgfx: Add vmwgfx device

[Patchwork]

== Series Details ==

Series: series starting with [i-g-t,1/6] lib/igt_vmwgfx: Add vmwgfx device
URL   : https://patchwork.freedesktop.org/series/50841/
State : success

== Summary ==

= CI Bug Log - changes from CI_DRM_4958 -> IGTPW_1938 =

== Summary - SUCCESS ==

  No regressions found.

  External URL: https://patchwork.freedesktop.org/api/1.0/series/50841/revisions/1/mbox/

== Known issues ==

  Here are the changes found in IGTPW_1938 that come from known issues:

  === IGT changes ===

    ==== Issues hit ====

    igt@kms_flip@basic-flip-vs-dpms:
      fi-hsw-4770r:       PASS -> DMESG-WARN (fdo#105602)

    
    ==== Possible fixes ====

    igt@kms_frontbuffer_tracking@basic:
      fi-byt-clapper:     FAIL (fdo#103167) -> PASS

    igt@pm_rpm@basic-pci-d3-state:
      fi-skl-6600u:       FAIL (fdo#107707) -> PASS

    igt@prime_vgem@basic-fence-flip:
      fi-ilk-650:         FAIL (fdo#104008) -> PASS

    
  fdo#103167 https://bugs.freedesktop.org/show_bug.cgi?id=103167
  fdo#104008 https://bugs.freedesktop.org/show_bug.cgi?id=104008
  fdo#105602 https://bugs.freedesktop.org/show_bug.cgi?id=105602
  fdo#107707 https://bugs.freedesktop.org/show_bug.cgi?id=107707


== Participating hosts (48 -> 41) ==

  Missing    (7): fi-ilk-m540 fi-hsw-4200u fi-byt-squawks fi-bsw-cyan fi-snb-2520m fi-ctg-p8600 fi-byt-n2820 


== Build changes ==

    * IGT: IGT_4672 -> IGTPW_1938

  CI_DRM_4958: 9990e1665029dc2ef4a9c0632b8a2f516263e595 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGTPW_1938: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_1938/
  IGT_4672: 4497591d2572831a9f07fd9e48a2571bfcffe354 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools



== Testlist changes ==

+igt@kms_atomic@atomic_plane_damage

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_1938/issues.html
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [igt-dev] [PATCH i-g-t 1/6] lib/igt_vmwgfx: Add vmwgfx device

[Daniel Vetter]

On Thu, Oct 11, 2018 at 12:11:23PM +0300, Petri Latvala wrote:
> On Thu, Oct 11, 2018 at 11:01:48AM +0200, Daniel Vetter wrote:
> > On Wed, Oct 10, 2018 at 05:20:59PM -0700, Deepak Rawat wrote:
> > > Add DRIVER_VMWGFX to represent vmwgfx device for running igt tests.
> > > 
> > > v2: Don't remove second virtio_gpu
> > > 
> > > Signed-off-by: Deepak Rawat <drawat@vmware.com>
> > > ---
> > >  lib/drmtest.c | 8 ++++++++
> > >  lib/drmtest.h | 3 +++
> > >  2 files changed, 11 insertions(+)
> > > 
> > > diff --git a/lib/drmtest.c b/lib/drmtest.c
> > > index fee9d33a..9d013a00 100644
> > > --- a/lib/drmtest.c
> > > +++ b/lib/drmtest.c
> > > @@ -105,6 +105,11 @@ bool is_i915_device(int fd)
> > >  	return __is_device(fd, "i915");
> > >  }
> > >  
> > > +bool is_vmwgfx_device(int fd)
> > > +{
> > > +	return __is_device(fd, "vmwg");
> > > +}
> > > +
> > >  static bool has_known_intel_chipset(int fd)
> > >  {
> > >  	struct drm_i915_getparam gp;
> > > @@ -206,6 +211,7 @@ static const struct module {
> > >  	{ DRIVER_VGEM, "vgem" },
> > >  	{ DRIVER_VIRTIO, "virtio-gpu" },
> > >  	{ DRIVER_VIRTIO, "virtio_gpu" },
> > > +	{ DRIVER_VMWGFX, "vmwgfx" },
> > >  	{}
> > >  };
> > >  
> > > @@ -348,6 +354,8 @@ static const char *chipset_to_str(int chipset)
> > >  		return "virtio";
> > >  	case DRIVER_AMDGPU:
> > >  		return "amdgpu";
> > > +	case DRIVER_VMWGFX:
> > > +		return "vmwgfx";
> > >  	case DRIVER_ANY:
> > >  		return "any";
> > >  	default:
> > > diff --git a/lib/drmtest.h b/lib/drmtest.h
> > > index 949865ee..0213fb51 100644
> > > --- a/lib/drmtest.h
> > > +++ b/lib/drmtest.h
> > > @@ -43,6 +43,7 @@
> > >  #define DRIVER_VGEM	(1 << 2)
> > >  #define DRIVER_VIRTIO	(1 << 3)
> > >  #define DRIVER_AMDGPU	(1 << 4)
> > > +#define DRIVER_VMWGFX	(1 << 5)
> > 
> > This seems not needed? For pure generic kms tests I think it'd be great if
> > we don't have to sprinkle driver-specific checks all over. Which you seem
> > to achive in your series here.
> > 
> > So not clear why this here is needed?
> 
> 
> Loading the .ko needs the name in this array:
> 
> > > @@ -206,6 +211,7 @@ static const struct module {
> > >  	{ DRIVER_VGEM, "vgem" },
> > >  	{ DRIVER_VIRTIO, "virtio-gpu" },
> > >  	{ DRIVER_VIRTIO, "virtio_gpu" },
> > > +	{ DRIVER_VMWGFX, "vmwgfx" },

I guess I don't quite understand why we do that then. Adding driver flags
for every kms driver under the sun isn't a maintainable thing going
forward. And there's definitely people using igt who don't have their
special flag+module autoloader.

vgem and vkms needs it, but that's about it. Everything else sounds like
papering over CI infrastructure mishaps to me ...
-Daniel

> 
> 
> 
> -- 
> Petri Latvala
> 
> 
> 
> 
> > -Daniel
> > 
> > >  /*
> > >   * Exclude DRVER_VGEM from DRIVER_ANY since if you run on a system
> > >   * with vgem as well as a supported driver, you can end up with a
> > > @@ -80,6 +81,8 @@ void igt_require_intel(int fd);
> > >  
> > >  bool is_i915_device(int fd);
> > >  
> > > +bool is_vmwgfx_device(int fd);
> > > +
> > >  /**
> > >   * do_or_die:
> > >   * @x: command
> > > -- 
> > > 2.17.1
> > > 
> > > _______________________________________________
> > > igt-dev mailing list
> > > igt-dev@lists.freedesktop.org
> > > https://lists.freedesktop.org/mailman/listinfo/igt-dev
> > 
> > -- 
> > Daniel Vetter
> > Software Engineer, Intel Corporation
> > http://blog.ffwll.ch
> > _______________________________________________
> > igt-dev mailing list
> > igt-dev@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/igt-dev

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

Re: [igt-dev] [PATCH i-g-t 1/6] lib/igt_vmwgfx: Add vmwgfx device

[Daniel Vetter]

On Thu, Oct 11, 2018 at 12:11:23PM +0300, Petri Latvala wrote:
> On Thu, Oct 11, 2018 at 11:01:48AM +0200, Daniel Vetter wrote:
> > On Wed, Oct 10, 2018 at 05:20:59PM -0700, Deepak Rawat wrote:
> > > Add DRIVER_VMWGFX to represent vmwgfx device for running igt tests.
> > > 
> > > v2: Don't remove second virtio_gpu
> > > 
> > > Signed-off-by: Deepak Rawat <drawat@vmware.com>
> > > ---
> > >  lib/drmtest.c | 8 ++++++++
> > >  lib/drmtest.h | 3 +++
> > >  2 files changed, 11 insertions(+)
> > > 
> > > diff --git a/lib/drmtest.c b/lib/drmtest.c
> > > index fee9d33a..9d013a00 100644
> > > --- a/lib/drmtest.c
> > > +++ b/lib/drmtest.c
> > > @@ -105,6 +105,11 @@ bool is_i915_device(int fd)
> > >  	return __is_device(fd, "i915");
> > >  }
> > >  
> > > +bool is_vmwgfx_device(int fd)
> > > +{
> > > +	return __is_device(fd, "vmwg");
> > > +}
> > > +
> > >  static bool has_known_intel_chipset(int fd)
> > >  {
> > >  	struct drm_i915_getparam gp;
> > > @@ -206,6 +211,7 @@ static const struct module {
> > >  	{ DRIVER_VGEM, "vgem" },
> > >  	{ DRIVER_VIRTIO, "virtio-gpu" },
> > >  	{ DRIVER_VIRTIO, "virtio_gpu" },
> > > +	{ DRIVER_VMWGFX, "vmwgfx" },
> > >  	{}
> > >  };
> > >  
> > > @@ -348,6 +354,8 @@ static const char *chipset_to_str(int chipset)
> > >  		return "virtio";
> > >  	case DRIVER_AMDGPU:
> > >  		return "amdgpu";
> > > +	case DRIVER_VMWGFX:
> > > +		return "vmwgfx";
> > >  	case DRIVER_ANY:
> > >  		return "any";
> > >  	default:
> > > diff --git a/lib/drmtest.h b/lib/drmtest.h
> > > index 949865ee..0213fb51 100644
> > > --- a/lib/drmtest.h
> > > +++ b/lib/drmtest.h
> > > @@ -43,6 +43,7 @@
> > >  #define DRIVER_VGEM	(1 << 2)
> > >  #define DRIVER_VIRTIO	(1 << 3)
> > >  #define DRIVER_AMDGPU	(1 << 4)
> > > +#define DRIVER_VMWGFX	(1 << 5)
> > 
> > This seems not needed? For pure generic kms tests I think it'd be great if
> > we don't have to sprinkle driver-specific checks all over. Which you seem
> > to achive in your series here.
> > 
> > So not clear why this here is needed?
> 
> 
> Loading the .ko needs the name in this array:
> 
> > > @@ -206,6 +211,7 @@ static const struct module {
> > >  	{ DRIVER_VGEM, "vgem" },
> > >  	{ DRIVER_VIRTIO, "virtio-gpu" },
> > >  	{ DRIVER_VIRTIO, "virtio_gpu" },
> > > +	{ DRIVER_VMWGFX, "vmwgfx" },

I guess I don't quite understand why we do that then. Adding driver flags
for every kms driver under the sun isn't a maintainable thing going
forward. And there's definitely people using igt who don't have their
special flag+module autoloader.

vgem and vkms needs it, but that's about it. Everything else sounds like
papering over CI infrastructure mishaps to me ...
-Daniel

> 
> 
> 
> -- 
> Petri Latvala
> 
> 
> 
> 
> > -Daniel
> > 
> > >  /*
> > >   * Exclude DRVER_VGEM from DRIVER_ANY since if you run on a system
> > >   * with vgem as well as a supported driver, you can end up with a
> > > @@ -80,6 +81,8 @@ void igt_require_intel(int fd);
> > >  
> > >  bool is_i915_device(int fd);
> > >  
> > > +bool is_vmwgfx_device(int fd);
> > > +
> > >  /**
> > >   * do_or_die:
> > >   * @x: command
> > > -- 
> > > 2.17.1
> > > 
> > > _______________________________________________
> > > igt-dev mailing list
> > > igt-dev@lists.freedesktop.org
> > > https://lists.freedesktop.org/mailman/listinfo/igt-dev
> > 
> > -- 
> > Daniel Vetter
> > Software Engineer, Intel Corporation
> > http://blog.ffwll.ch
> > _______________________________________________
> > igt-dev mailing list
> > igt-dev@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/igt-dev

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

[igt-dev] ✓ Fi.CI.IGT: success for series starting with [i-g-t,1/6] lib/igt_vmwgfx: Add vmwgfx device

[Patchwork]

== Series Details ==

Series: series starting with [i-g-t,1/6] lib/igt_vmwgfx: Add vmwgfx device
URL   : https://patchwork.freedesktop.org/series/50841/
State : success

== Summary ==

= CI Bug Log - changes from IGT_4672_full -> IGTPW_1938_full =

== Summary - WARNING ==

  Minor unknown changes coming with IGTPW_1938_full need to be verified
  manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in IGTPW_1938_full, please notify your bug team to allow them
  to document this new failure mode, which will reduce false positives in CI.

  External URL: https://patchwork.freedesktop.org/api/1.0/series/50841/revisions/1/mbox/

== Possible new issues ==

  Here are the unknown changes that may have been introduced in IGTPW_1938_full:

  === IGT changes ===

    ==== Warnings ====

    igt@perf_pmu@rc6:
      shard-kbl:          PASS -> SKIP

    igt@pm_rc6_residency@rc6-accuracy:
      shard-snb:          PASS -> SKIP

    
== Known issues ==

  Here are the changes found in IGTPW_1938_full that come from known issues:

  === IGT changes ===

    ==== Issues hit ====

    igt@drv_suspend@shrink:
      shard-glk:          PASS -> INCOMPLETE (k.org#198133, fdo#106886, fdo#103359)

    igt@gem_ctx_switch@basic-all-light:
      shard-kbl:          PASS -> INCOMPLETE (fdo#103665) +1

    igt@gem_tiled_blits@interruptible:
      shard-apl:          PASS -> INCOMPLETE (fdo#103927)

    igt@kms_available_modes_crc@available_mode_test_crc:
      shard-snb:          PASS -> FAIL (fdo#106641)

    igt@kms_color@pipe-a-degamma:
      shard-apl:          PASS -> FAIL (fdo#108145, fdo#104782)
      shard-kbl:          PASS -> FAIL (fdo#108145, fdo#104782)

    igt@kms_cursor_crc@cursor-128x42-random:
      shard-glk:          PASS -> FAIL (fdo#103232)

    igt@kms_cursor_crc@cursor-256x256-sliding:
      shard-kbl:          PASS -> FAIL (fdo#103232) +1

    igt@kms_cursor_crc@cursor-256x85-onscreen:
      shard-hsw:          PASS -> INCOMPLETE (fdo#103540) +1

    igt@kms_cursor_crc@cursor-64x21-random:
      shard-apl:          PASS -> FAIL (fdo#103232) +3

    igt@kms_draw_crc@draw-method-xrgb8888-mmap-cpu-xtiled:
      shard-glk:          PASS -> FAIL (fdo#107791)

    igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-draw-mmap-wc:
      shard-kbl:          PASS -> FAIL (fdo#103167)

    igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-draw-pwrite:
      shard-apl:          PASS -> FAIL (fdo#103167) +4

    igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-onoff:
      shard-glk:          PASS -> FAIL (fdo#103167) +6

    {igt@kms_plane_alpha_blend@pipe-a-constant-alpha-max}:
      shard-glk:          PASS -> FAIL (fdo#108145) +1
      shard-kbl:          PASS -> FAIL (fdo#108145)
      shard-apl:          PASS -> FAIL (fdo#108145)

    igt@kms_plane_multiple@atomic-pipe-a-tiling-x:
      shard-apl:          PASS -> FAIL (fdo#103166) +1

    igt@kms_rotation_crc@primary-rotation-180:
      shard-snb:          NOTRUN -> FAIL (fdo#103925)

    igt@perf@blocking:
      shard-hsw:          PASS -> FAIL (fdo#102252)

    igt@pm_rps@reset:
      shard-apl:          PASS -> FAIL (fdo#102250)

    
    ==== Possible fixes ====

    igt@kms_ccs@pipe-b-crc-sprite-planes-basic:
      shard-glk:          FAIL (fdo#108145) -> PASS

    igt@kms_cursor_crc@cursor-128x42-onscreen:
      shard-apl:          FAIL (fdo#103232) -> PASS +1

    igt@kms_cursor_crc@cursor-256x256-sliding:
      shard-glk:          FAIL (fdo#103232) -> PASS +3

    igt@kms_cursor_crc@cursor-64x64-sliding:
      shard-kbl:          FAIL (fdo#103232) -> PASS

    igt@kms_cursor_crc@cursor-64x64-suspend:
      shard-glk:          INCOMPLETE (k.org#198133, fdo#103359) -> PASS
      shard-apl:          FAIL (fdo#103232, fdo#103191) -> PASS

    igt@kms_cursor_legacy@2x-long-cursor-vs-flip-atomic:
      shard-hsw:          FAIL (fdo#105767) -> PASS

    igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-draw-mmap-gtt:
      shard-snb:          INCOMPLETE (fdo#105411) -> PASS

    igt@kms_frontbuffer_tracking@fbc-2p-primscrn-shrfb-pgflip-blt:
      shard-glk:          FAIL (fdo#103167) -> PASS

    igt@kms_plane@plane-position-covered-pipe-a-planes:
      shard-glk:          FAIL (fdo#103166) -> PASS +1

    igt@kms_plane_multiple@atomic-pipe-b-tiling-x:
      shard-apl:          FAIL (fdo#103166) -> PASS +2

    igt@kms_setmode@basic:
      shard-kbl:          FAIL (fdo#99912) -> PASS

    
  {name}: This element is suppressed. This means it is ignored when computing
          the status of the difference (SUCCESS, WARNING, or FAILURE).

  fdo#102250 https://bugs.freedesktop.org/show_bug.cgi?id=102250
  fdo#102252 https://bugs.freedesktop.org/show_bug.cgi?id=102252
  fdo#103166 https://bugs.freedesktop.org/show_bug.cgi?id=103166
  fdo#103167 https://bugs.freedesktop.org/show_bug.cgi?id=103167
  fdo#103191 https://bugs.freedesktop.org/show_bug.cgi?id=103191
  fdo#103232 https://bugs.freedesktop.org/show_bug.cgi?id=103232
  fdo#103359 https://bugs.freedesktop.org/show_bug.cgi?id=103359
  fdo#103540 https://bugs.freedesktop.org/show_bug.cgi?id=103540
  fdo#103665 https://bugs.freedesktop.org/show_bug.cgi?id=103665
  fdo#103925 https://bugs.freedesktop.org/show_bug.cgi?id=103925
  fdo#103927 https://bugs.freedesktop.org/show_bug.cgi?id=103927
  fdo#104782 https://bugs.freedesktop.org/show_bug.cgi?id=104782
  fdo#105411 https://bugs.freedesktop.org/show_bug.cgi?id=105411
  fdo#105767 https://bugs.freedesktop.org/show_bug.cgi?id=105767
  fdo#106641 https://bugs.freedesktop.org/show_bug.cgi?id=106641
  fdo#106886 https://bugs.freedesktop.org/show_bug.cgi?id=106886
  fdo#107791 https://bugs.freedesktop.org/show_bug.cgi?id=107791
  fdo#108145 https://bugs.freedesktop.org/show_bug.cgi?id=108145
  fdo#99912 https://bugs.freedesktop.org/show_bug.cgi?id=99912
  k.org#198133 https://bugzilla.kernel.org/show_bug.cgi?id=198133


== Participating hosts (6 -> 5) ==

  Missing    (1): shard-skl 


== Build changes ==

    * IGT: IGT_4672 -> IGTPW_1938
    * Linux: CI_DRM_4952 -> CI_DRM_4958

  CI_DRM_4952: a62e43ba13605a478b22307ea1790d48aea029a6 @ git://anongit.freedesktop.org/gfx-ci/linux
  CI_DRM_4958: 9990e1665029dc2ef4a9c0632b8a2f516263e595 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGTPW_1938: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_1938/
  IGT_4672: 4497591d2572831a9f07fd9e48a2571bfcffe354 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_1938/shards.html
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [igt-dev] [PATCH i-g-t 4/6] lib: Don't call igt_require_fb_modifiers() when no modifier

[Ville Syrjälä]

On Wed, Oct 10, 2018 at 05:21:02PM -0700, Deepak Rawat wrote:
> vmwgfx doesn't support fb modifier so skip igt_require_fb_modifiers()
> when modifier are not passed.
> 
> Signed-off-by: Deepak Rawat <drawat@vmware.com>
> ---
>  lib/ioctl_wrappers.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/lib/ioctl_wrappers.c b/lib/ioctl_wrappers.c
> index 0929c43f..3a11cb6e 100644
> --- a/lib/ioctl_wrappers.c
> +++ b/lib/ioctl_wrappers.c
> @@ -1678,7 +1678,10 @@ int __kms_addfb(int fd, uint32_t handle,
>  	struct drm_mode_fb_cmd2 f;
>  	int ret, i;
>  
> -	igt_require_fb_modifiers(fd);
> +	if (modifier)
> +		igt_require_fb_modifiers(fd);
> +	else
> +		flags &= ~DRM_MODE_FB_MODIFIERS;

This would theoretically change the behviour for i915 at least. Without
the modifiers flag the kernel will pick the modifier for us based on 
the bo tiling, which in theory might not be what we wanted. But at least
igt_fb should be fine with that.

Maybe it would be better to just not pass the flags from the caller at
all, and instead have __kms_addfb() check if the driver has modifiers
or not and set the flag based on that?

>  
>  	memset(&f, 0, sizeof(f));
>  
> -- 
> 2.17.1
> 
> _______________________________________________
> igt-dev mailing list
> igt-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/igt-dev

-- 
Ville Syrjälä
Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [igt-dev] [PATCH i-g-t 4/6] lib: Don't call igt_require_fb_modifiers() when no modifier

[Ville Syrjälä]

On Wed, Oct 10, 2018 at 05:21:02PM -0700, Deepak Rawat wrote:
> vmwgfx doesn't support fb modifier so skip igt_require_fb_modifiers()
> when modifier are not passed.
> 
> Signed-off-by: Deepak Rawat <drawat@vmware.com>
> ---
>  lib/ioctl_wrappers.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/lib/ioctl_wrappers.c b/lib/ioctl_wrappers.c
> index 0929c43f..3a11cb6e 100644
> --- a/lib/ioctl_wrappers.c
> +++ b/lib/ioctl_wrappers.c
> @@ -1678,7 +1678,10 @@ int __kms_addfb(int fd, uint32_t handle,
>  	struct drm_mode_fb_cmd2 f;
>  	int ret, i;
>  
> -	igt_require_fb_modifiers(fd);
> +	if (modifier)
> +		igt_require_fb_modifiers(fd);
> +	else
> +		flags &= ~DRM_MODE_FB_MODIFIERS;

This would theoretically change the behviour for i915 at least. Without
the modifiers flag the kernel will pick the modifier for us based on 
the bo tiling, which in theory might not be what we wanted. But at least
igt_fb should be fine with that.

Maybe it would be better to just not pass the flags from the caller at
all, and instead have __kms_addfb() check if the driver has modifiers
or not and set the flag based on that?

>  
>  	memset(&f, 0, sizeof(f));
>  
> -- 
> 2.17.1
> 
> _______________________________________________
> igt-dev mailing list
> igt-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/igt-dev

-- 
Ville Syrjälä
Intel
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [igt-dev] [PATCH i-g-t 3/6] lib/igt_fb: Check for cairo surface success

[Ville Syrjälä]

On Wed, Oct 10, 2018 at 05:21:01PM -0700, Deepak Rawat wrote:
> For vmwgfx cairo surface creation fails due to stride mismatch, add a
> igt_require_f() for surface.

Hmm. What kind of pixel format are you using?

It seems to me cairo should be happy with just a 4 byte aligned
stride. How bad would it be to change vmw_dumb_create() to give
you that? Having working cairo stuff should make igt_fb much more
useful for you in general.

Alternative would to not use dumb buffers I suppose, but then
we'd need to make sure the igt_fb size calculations are OK for
vmwgfx. Currently igt_fb assumes Intel hw in the sense that
linear buffers will get a 64byte aligned stride.

> 
> v2: Check for surface creation failure.
> 
> Signed-off-by: Deepak Rawat <drawat@vmware.com>
> ---
>  lib/igt_fb.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/lib/igt_fb.c b/lib/igt_fb.c
> index 335ece69..1bb6d324 100644
> --- a/lib/igt_fb.c
> +++ b/lib/igt_fb.c
> @@ -1433,6 +1433,10 @@ static void create_cairo_surface__gtt(int fd, struct igt_fb *fb)
>  		cairo_image_surface_create_for_data(ptr,
>  						    drm_format_to_cairo(fb->drm_format),
>  						    fb->width, fb->height, fb->strides[0]);
> +	igt_require_f(cairo_surface_status(fb->cairo_surface) == CAIRO_STATUS_SUCCESS,
> +		      "Unable to create a cairo surface: %s\n",
> +		      cairo_status_to_string(cairo_surface_status(fb->cairo_surface)));
> +
>  	fb->domain = I915_GEM_DOMAIN_GTT;
>  
>  	cairo_surface_set_user_data(fb->cairo_surface,
> -- 
> 2.17.1
> 
> _______________________________________________
> igt-dev mailing list
> igt-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/igt-dev

-- 
Ville Syrjälä
Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [igt-dev] [PATCH i-g-t 3/6] lib/igt_fb: Check for cairo surface success

[Ville Syrjälä]

On Wed, Oct 10, 2018 at 05:21:01PM -0700, Deepak Rawat wrote:
> For vmwgfx cairo surface creation fails due to stride mismatch, add a
> igt_require_f() for surface.

Hmm. What kind of pixel format are you using?

It seems to me cairo should be happy with just a 4 byte aligned
stride. How bad would it be to change vmw_dumb_create() to give
you that? Having working cairo stuff should make igt_fb much more
useful for you in general.

Alternative would to not use dumb buffers I suppose, but then
we'd need to make sure the igt_fb size calculations are OK for
vmwgfx. Currently igt_fb assumes Intel hw in the sense that
linear buffers will get a 64byte aligned stride.

> 
> v2: Check for surface creation failure.
> 
> Signed-off-by: Deepak Rawat <drawat@vmware.com>
> ---
>  lib/igt_fb.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/lib/igt_fb.c b/lib/igt_fb.c
> index 335ece69..1bb6d324 100644
> --- a/lib/igt_fb.c
> +++ b/lib/igt_fb.c
> @@ -1433,6 +1433,10 @@ static void create_cairo_surface__gtt(int fd, struct igt_fb *fb)
>  		cairo_image_surface_create_for_data(ptr,
>  						    drm_format_to_cairo(fb->drm_format),
>  						    fb->width, fb->height, fb->strides[0]);
> +	igt_require_f(cairo_surface_status(fb->cairo_surface) == CAIRO_STATUS_SUCCESS,
> +		      "Unable to create a cairo surface: %s\n",
> +		      cairo_status_to_string(cairo_surface_status(fb->cairo_surface)));
> +
>  	fb->domain = I915_GEM_DOMAIN_GTT;
>  
>  	cairo_surface_set_user_data(fb->cairo_surface,
> -- 
> 2.17.1
> 
> _______________________________________________
> igt-dev mailing list
> igt-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/igt-dev

-- 
Ville Syrjälä
Intel
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [igt-dev] [PATCH i-g-t 1/6] lib/igt_vmwgfx: Add vmwgfx device

[Deepak Singh Rawat]

> 
> On Wed, Oct 10, 2018 at 05:20:59PM -0700, Deepak Rawat wrote:
> > Add DRIVER_VMWGFX to represent vmwgfx device for running igt tests.
> >
> > v2: Don't remove second virtio_gpu
> >
> > Signed-off-by: Deepak Rawat <drawat@vmware.com>
> > ---
> >  lib/drmtest.c | 8 ++++++++
> >  lib/drmtest.h | 3 +++
> >  2 files changed, 11 insertions(+)
> >
> > diff --git a/lib/drmtest.c b/lib/drmtest.c
> > index fee9d33a..9d013a00 100644
> > --- a/lib/drmtest.c
> > +++ b/lib/drmtest.c
> > @@ -105,6 +105,11 @@ bool is_i915_device(int fd)
> >  	return __is_device(fd, "i915");
> >  }
> >
> > +bool is_vmwgfx_device(int fd)
> > +{
> > +	return __is_device(fd, "vmwg");
> > +}
> > +
> >  static bool has_known_intel_chipset(int fd)
> >  {
> >  	struct drm_i915_getparam gp;
> > @@ -206,6 +211,7 @@ static const struct module {
> >  	{ DRIVER_VGEM, "vgem" },
> >  	{ DRIVER_VIRTIO, "virtio-gpu" },
> >  	{ DRIVER_VIRTIO, "virtio_gpu" },
> > +	{ DRIVER_VMWGFX, "vmwgfx" },
> >  	{}
> >  };
> >
> > @@ -348,6 +354,8 @@ static const char *chipset_to_str(int chipset)
> >  		return "virtio";
> >  	case DRIVER_AMDGPU:
> >  		return "amdgpu";
> > +	case DRIVER_VMWGFX:
> > +		return "vmwgfx";
> >  	case DRIVER_ANY:
> >  		return "any";
> >  	default:
> > diff --git a/lib/drmtest.h b/lib/drmtest.h
> > index 949865ee..0213fb51 100644
> > --- a/lib/drmtest.h
> > +++ b/lib/drmtest.h
> > @@ -43,6 +43,7 @@
> >  #define DRIVER_VGEM	(1 << 2)
> >  #define DRIVER_VIRTIO	(1 << 3)
> >  #define DRIVER_AMDGPU	(1 << 4)
> > +#define DRIVER_VMWGFX	(1 << 5)
> 
> This seems not needed? For pure generic kms tests I think it'd be great if
> we don't have to sprinkle driver-specific checks all over. Which you seem
> to achive in your series here.
> 
> So not clear why this here is needed?
> -Daniel

Hi Daniel,

Thanks for the review. You are right for kms tests having vmwgfx driver type
is not needed but I added vmwgfx device type because I plan to add more
vmwgfx test cases, and since vmwgfx buffer allocation is private ioctl
having a separate device type might be needed.

I can drop this until I have some vmwgfx specific test cases.

> 
> >  /*
> >   * Exclude DRVER_VGEM from DRIVER_ANY since if you run on a system
> >   * with vgem as well as a supported driver, you can end up with a
> > @@ -80,6 +81,8 @@ void igt_require_intel(int fd);
> >
> >  bool is_i915_device(int fd);
> >
> > +bool is_vmwgfx_device(int fd);
> > +
> >  /**
> >   * do_or_die:
> >   * @x: command
> > --
> > 2.17.1
> >
> > _______________________________________________
> > igt-dev mailing list
> > igt-dev@lists.freedesktop.org
> >
> https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.fr
> eedesktop.org%2Fmailman%2Flistinfo%2Figt-
> dev&amp;data=02%7C01%7Cdrawat%40vmware.com%7C5486d67be73647b1
> 1d2708d62f58307d%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C1%7C0%7C6
> 36748453147406904&amp;sdata=QcT4VTQth8GuXgjZejCsnFPDsMFbtqrUOW
> uSDr8zbG8%3D&amp;reserved=0
> 
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> https://na01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fblog.ff
> wll.ch&amp;data=02%7C01%7Cdrawat%40vmware.com%7C5486d67be73647
> b11d2708d62f58307d%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C1%7C0%7
> C636748453147406904&amp;sdata=4SKi8P5PeyCAUsoZh%2BsYFC%2FU2RiEx5
> qp2gPP7bPr3Bo%3D&amp;reserved=0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [igt-dev] [PATCH i-g-t 1/6] lib/igt_vmwgfx: Add vmwgfx device

[Deepak Singh Rawat]

> 
> On Wed, Oct 10, 2018 at 05:20:59PM -0700, Deepak Rawat wrote:
> > Add DRIVER_VMWGFX to represent vmwgfx device for running igt tests.
> >
> > v2: Don't remove second virtio_gpu
> >
> > Signed-off-by: Deepak Rawat <drawat@vmware.com>
> > ---
> >  lib/drmtest.c | 8 ++++++++
> >  lib/drmtest.h | 3 +++
> >  2 files changed, 11 insertions(+)
> >
> > diff --git a/lib/drmtest.c b/lib/drmtest.c
> > index fee9d33a..9d013a00 100644
> > --- a/lib/drmtest.c
> > +++ b/lib/drmtest.c
> > @@ -105,6 +105,11 @@ bool is_i915_device(int fd)
> >  	return __is_device(fd, "i915");
> >  }
> >
> > +bool is_vmwgfx_device(int fd)
> > +{
> > +	return __is_device(fd, "vmwg");
> > +}
> > +
> >  static bool has_known_intel_chipset(int fd)
> >  {
> >  	struct drm_i915_getparam gp;
> > @@ -206,6 +211,7 @@ static const struct module {
> >  	{ DRIVER_VGEM, "vgem" },
> >  	{ DRIVER_VIRTIO, "virtio-gpu" },
> >  	{ DRIVER_VIRTIO, "virtio_gpu" },
> > +	{ DRIVER_VMWGFX, "vmwgfx" },
> >  	{}
> >  };
> >
> > @@ -348,6 +354,8 @@ static const char *chipset_to_str(int chipset)
> >  		return "virtio";
> >  	case DRIVER_AMDGPU:
> >  		return "amdgpu";
> > +	case DRIVER_VMWGFX:
> > +		return "vmwgfx";
> >  	case DRIVER_ANY:
> >  		return "any";
> >  	default:
> > diff --git a/lib/drmtest.h b/lib/drmtest.h
> > index 949865ee..0213fb51 100644
> > --- a/lib/drmtest.h
> > +++ b/lib/drmtest.h
> > @@ -43,6 +43,7 @@
> >  #define DRIVER_VGEM	(1 << 2)
> >  #define DRIVER_VIRTIO	(1 << 3)
> >  #define DRIVER_AMDGPU	(1 << 4)
> > +#define DRIVER_VMWGFX	(1 << 5)
> 
> This seems not needed? For pure generic kms tests I think it'd be great if
> we don't have to sprinkle driver-specific checks all over. Which you seem
> to achive in your series here.
> 
> So not clear why this here is needed?
> -Daniel

Hi Daniel,

Thanks for the review. You are right for kms tests having vmwgfx driver type
is not needed but I added vmwgfx device type because I plan to add more
vmwgfx test cases, and since vmwgfx buffer allocation is private ioctl
having a separate device type might be needed.

I can drop this until I have some vmwgfx specific test cases.

> 
> >  /*
> >   * Exclude DRVER_VGEM from DRIVER_ANY since if you run on a system
> >   * with vgem as well as a supported driver, you can end up with a
> > @@ -80,6 +81,8 @@ void igt_require_intel(int fd);
> >
> >  bool is_i915_device(int fd);
> >
> > +bool is_vmwgfx_device(int fd);
> > +
> >  /**
> >   * do_or_die:
> >   * @x: command
> > --
> > 2.17.1
> >
> > _______________________________________________
> > igt-dev mailing list
> > igt-dev@lists.freedesktop.org
> >
> https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.fr
> eedesktop.org%2Fmailman%2Flistinfo%2Figt-
> dev&amp;data=02%7C01%7Cdrawat%40vmware.com%7C5486d67be73647b1
> 1d2708d62f58307d%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C1%7C0%7C6
> 36748453147406904&amp;sdata=QcT4VTQth8GuXgjZejCsnFPDsMFbtqrUOW
> uSDr8zbG8%3D&amp;reserved=0
> 
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> https://na01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fblog.ff
> wll.ch&amp;data=02%7C01%7Cdrawat%40vmware.com%7C5486d67be73647
> b11d2708d62f58307d%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C1%7C0%7
> C636748453147406904&amp;sdata=4SKi8P5PeyCAUsoZh%2BsYFC%2FU2RiEx5
> qp2gPP7bPr3Bo%3D&amp;reserved=0
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [PATCH i-g-t 6/6] tests/plane_damage: Integrate kernel selftest test-drm_damage_helper

[Deepak Singh Rawat]

> > diff --git a/tests/meson.build b/tests/meson.build
> > index 697ff515..5acd7aa2 100644
> > --- a/tests/meson.build
> > +++ b/tests/meson.build
> > @@ -9,6 +9,7 @@ test_progs = [
> >  	'debugfs_test',
> >  	'drm_import_export',
> >  	'drm_mm',
> > +	'drm_plane_damage',
> 
> For future proofing I think it'd be much better if we call this drm_kms or
> similar. The individual subtest results will be all exposed, but there's a
> bit a problem when we always have to upgrade both igt and the kernel at
> the same time. At least with the current CI infrastructure.

Do you mean drm_kms_plane_damage? IIUC this is to allow running test
from run-test.sh? I suppose in that case it should be named
kms_plane_damage, because all other kms test starts with kms_*.

> 
> I'm also asking the ARM folks to type selftests for the new block_* format
> description stuff, so this will come in handy real soon.
> -Daniel
> 
> >  	'drm_read',
> >  	'drv_getparams_basic',
> >  	'drv_hangman',
> > --
> > 2.17.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH i-g-t 6/6] tests/plane_damage: Integrate kernel selftest test-drm_damage_helper

[Deepak Singh Rawat]

> > diff --git a/tests/meson.build b/tests/meson.build
> > index 697ff515..5acd7aa2 100644
> > --- a/tests/meson.build
> > +++ b/tests/meson.build
> > @@ -9,6 +9,7 @@ test_progs = [
> >  	'debugfs_test',
> >  	'drm_import_export',
> >  	'drm_mm',
> > +	'drm_plane_damage',
> 
> For future proofing I think it'd be much better if we call this drm_kms or
> similar. The individual subtest results will be all exposed, but there's a
> bit a problem when we always have to upgrade both igt and the kernel at
> the same time. At least with the current CI infrastructure.

Do you mean drm_kms_plane_damage? IIUC this is to allow running test
from run-test.sh? I suppose in that case it should be named
kms_plane_damage, because all other kms test starts with kms_*.

> 
> I'm also asking the ARM folks to type selftests for the new block_* format
> description stuff, so this will come in handy real soon.
> -Daniel
> 
> >  	'drm_read',
> >  	'drv_getparams_basic',
> >  	'drv_hangman',
> > --
> > 2.17.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH i-g-t 6/6] tests/plane_damage: Integrate kernel selftest test-drm_damage_helper

[Daniel Vetter]

On Thu, Oct 11, 2018 at 03:23:58PM +0000, Deepak Singh Rawat wrote:
> > > diff --git a/tests/meson.build b/tests/meson.build
> > > index 697ff515..5acd7aa2 100644
> > > --- a/tests/meson.build
> > > +++ b/tests/meson.build
> > > @@ -9,6 +9,7 @@ test_progs = [
> > >  	'debugfs_test',
> > >  	'drm_import_export',
> > >  	'drm_mm',
> > > +	'drm_plane_damage',
> > 
> > For future proofing I think it'd be much better if we call this drm_kms or
> > similar. The individual subtest results will be all exposed, but there's a
> > bit a problem when we always have to upgrade both igt and the kernel at
> > the same time. At least with the current CI infrastructure.
> 
> Do you mean drm_kms_plane_damage? IIUC this is to allow running test
> from run-test.sh? I suppose in that case it should be named
> kms_plane_damage, because all other kms test starts with kms_*.

Ah yes, sticking to the kms_ prefix is a good idea.  kms_selftests is what
I'd recommend. For both the igt wrapper here, and the kernel module.

That way it's a natural place to add all kinds of kms self tests in the
future, without the need to go through the basic scaffolding steps again.

Cheers, Daniel
> 
> > 
> > I'm also asking the ARM folks to type selftests for the new block_* format
> > description stuff, so this will come in handy real soon.
> > -Daniel
> > 
> > >  	'drm_read',
> > >  	'drv_getparams_basic',
> > >  	'drv_hangman',
> > > --
> > > 2.17.1

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

Re: [Intel-gfx] [PATCH i-g-t 6/6] tests/plane_damage: Integrate kernel selftest test-drm_damage_helper

[Daniel Vetter]

On Thu, Oct 11, 2018 at 03:23:58PM +0000, Deepak Singh Rawat wrote:
> > > diff --git a/tests/meson.build b/tests/meson.build
> > > index 697ff515..5acd7aa2 100644
> > > --- a/tests/meson.build
> > > +++ b/tests/meson.build
> > > @@ -9,6 +9,7 @@ test_progs = [
> > >  	'debugfs_test',
> > >  	'drm_import_export',
> > >  	'drm_mm',
> > > +	'drm_plane_damage',
> > 
> > For future proofing I think it'd be much better if we call this drm_kms or
> > similar. The individual subtest results will be all exposed, but there's a
> > bit a problem when we always have to upgrade both igt and the kernel at
> > the same time. At least with the current CI infrastructure.
> 
> Do you mean drm_kms_plane_damage? IIUC this is to allow running test
> from run-test.sh? I suppose in that case it should be named
> kms_plane_damage, because all other kms test starts with kms_*.

Ah yes, sticking to the kms_ prefix is a good idea.  kms_selftests is what
I'd recommend. For both the igt wrapper here, and the kernel module.

That way it's a natural place to add all kinds of kms self tests in the
future, without the need to go through the basic scaffolding steps again.

Cheers, Daniel
> 
> > 
> > I'm also asking the ARM folks to type selftests for the new block_* format
> > description stuff, so this will come in handy real soon.
> > -Daniel
> > 
> > >  	'drm_read',
> > >  	'drv_getparams_basic',
> > >  	'drv_hangman',
> > > --
> > > 2.17.1

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

Re: [igt-dev] [PATCH i-g-t 1/6] lib/igt_vmwgfx: Add vmwgfx device

[Daniel Vetter]

On Thu, Oct 11, 2018 at 03:17:01PM +0000, Deepak Singh Rawat wrote:
> > 
> > On Wed, Oct 10, 2018 at 05:20:59PM -0700, Deepak Rawat wrote:
> > > Add DRIVER_VMWGFX to represent vmwgfx device for running igt tests.
> > >
> > > v2: Don't remove second virtio_gpu
> > >
> > > Signed-off-by: Deepak Rawat <drawat@vmware.com>
> > > ---
> > >  lib/drmtest.c | 8 ++++++++
> > >  lib/drmtest.h | 3 +++
> > >  2 files changed, 11 insertions(+)
> > >
> > > diff --git a/lib/drmtest.c b/lib/drmtest.c
> > > index fee9d33a..9d013a00 100644
> > > --- a/lib/drmtest.c
> > > +++ b/lib/drmtest.c
> > > @@ -105,6 +105,11 @@ bool is_i915_device(int fd)
> > >  	return __is_device(fd, "i915");
> > >  }
> > >
> > > +bool is_vmwgfx_device(int fd)
> > > +{
> > > +	return __is_device(fd, "vmwg");
> > > +}
> > > +
> > >  static bool has_known_intel_chipset(int fd)
> > >  {
> > >  	struct drm_i915_getparam gp;
> > > @@ -206,6 +211,7 @@ static const struct module {
> > >  	{ DRIVER_VGEM, "vgem" },
> > >  	{ DRIVER_VIRTIO, "virtio-gpu" },
> > >  	{ DRIVER_VIRTIO, "virtio_gpu" },
> > > +	{ DRIVER_VMWGFX, "vmwgfx" },
> > >  	{}
> > >  };
> > >
> > > @@ -348,6 +354,8 @@ static const char *chipset_to_str(int chipset)
> > >  		return "virtio";
> > >  	case DRIVER_AMDGPU:
> > >  		return "amdgpu";
> > > +	case DRIVER_VMWGFX:
> > > +		return "vmwgfx";
> > >  	case DRIVER_ANY:
> > >  		return "any";
> > >  	default:
> > > diff --git a/lib/drmtest.h b/lib/drmtest.h
> > > index 949865ee..0213fb51 100644
> > > --- a/lib/drmtest.h
> > > +++ b/lib/drmtest.h
> > > @@ -43,6 +43,7 @@
> > >  #define DRIVER_VGEM	(1 << 2)
> > >  #define DRIVER_VIRTIO	(1 << 3)
> > >  #define DRIVER_AMDGPU	(1 << 4)
> > > +#define DRIVER_VMWGFX	(1 << 5)
> > 
> > This seems not needed? For pure generic kms tests I think it'd be great if
> > we don't have to sprinkle driver-specific checks all over. Which you seem
> > to achive in your series here.
> > 
> > So not clear why this here is needed?
> > -Daniel
> 
> Hi Daniel,
> 
> Thanks for the review. You are right for kms tests having vmwgfx driver type
> is not needed but I added vmwgfx device type because I plan to add more
> vmwgfx test cases, and since vmwgfx buffer allocation is private ioctl
> having a separate device type might be needed.

Oh, this sounds awesome. And yes, for private ioctl tests vmwgfx is
obviuosly very much welcome!

> I can drop this until I have some vmwgfx specific test cases.

Sounds like a good plan to me.
-Daniel

> 
> > 
> > >  /*
> > >   * Exclude DRVER_VGEM from DRIVER_ANY since if you run on a system
> > >   * with vgem as well as a supported driver, you can end up with a
> > > @@ -80,6 +81,8 @@ void igt_require_intel(int fd);
> > >
> > >  bool is_i915_device(int fd);
> > >
> > > +bool is_vmwgfx_device(int fd);
> > > +
> > >  /**
> > >   * do_or_die:
> > >   * @x: command
> > > --
> > > 2.17.1
> > >
> > > _______________________________________________
> > > igt-dev mailing list
> > > igt-dev@lists.freedesktop.org
> > >
> > https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.fr
> > eedesktop.org%2Fmailman%2Flistinfo%2Figt-
> > dev&amp;data=02%7C01%7Cdrawat%40vmware.com%7C5486d67be73647b1
> > 1d2708d62f58307d%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C1%7C0%7C6
> > 36748453147406904&amp;sdata=QcT4VTQth8GuXgjZejCsnFPDsMFbtqrUOW
> > uSDr8zbG8%3D&amp;reserved=0
> > 
> > --
> > Daniel Vetter
> > Software Engineer, Intel Corporation
> > https://na01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fblog.ff
> > wll.ch&amp;data=02%7C01%7Cdrawat%40vmware.com%7C5486d67be73647
> > b11d2708d62f58307d%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C1%7C0%7
> > C636748453147406904&amp;sdata=4SKi8P5PeyCAUsoZh%2BsYFC%2FU2RiEx5
> > qp2gPP7bPr3Bo%3D&amp;reserved=0

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

Re: [igt-dev] [PATCH i-g-t 1/6] lib/igt_vmwgfx: Add vmwgfx device

[Daniel Vetter]

On Thu, Oct 11, 2018 at 03:17:01PM +0000, Deepak Singh Rawat wrote:
> > 
> > On Wed, Oct 10, 2018 at 05:20:59PM -0700, Deepak Rawat wrote:
> > > Add DRIVER_VMWGFX to represent vmwgfx device for running igt tests.
> > >
> > > v2: Don't remove second virtio_gpu
> > >
> > > Signed-off-by: Deepak Rawat <drawat@vmware.com>
> > > ---
> > >  lib/drmtest.c | 8 ++++++++
> > >  lib/drmtest.h | 3 +++
> > >  2 files changed, 11 insertions(+)
> > >
> > > diff --git a/lib/drmtest.c b/lib/drmtest.c
> > > index fee9d33a..9d013a00 100644
> > > --- a/lib/drmtest.c
> > > +++ b/lib/drmtest.c
> > > @@ -105,6 +105,11 @@ bool is_i915_device(int fd)
> > >  	return __is_device(fd, "i915");
> > >  }
> > >
> > > +bool is_vmwgfx_device(int fd)
> > > +{
> > > +	return __is_device(fd, "vmwg");
> > > +}
> > > +
> > >  static bool has_known_intel_chipset(int fd)
> > >  {
> > >  	struct drm_i915_getparam gp;
> > > @@ -206,6 +211,7 @@ static const struct module {
> > >  	{ DRIVER_VGEM, "vgem" },
> > >  	{ DRIVER_VIRTIO, "virtio-gpu" },
> > >  	{ DRIVER_VIRTIO, "virtio_gpu" },
> > > +	{ DRIVER_VMWGFX, "vmwgfx" },
> > >  	{}
> > >  };
> > >
> > > @@ -348,6 +354,8 @@ static const char *chipset_to_str(int chipset)
> > >  		return "virtio";
> > >  	case DRIVER_AMDGPU:
> > >  		return "amdgpu";
> > > +	case DRIVER_VMWGFX:
> > > +		return "vmwgfx";
> > >  	case DRIVER_ANY:
> > >  		return "any";
> > >  	default:
> > > diff --git a/lib/drmtest.h b/lib/drmtest.h
> > > index 949865ee..0213fb51 100644
> > > --- a/lib/drmtest.h
> > > +++ b/lib/drmtest.h
> > > @@ -43,6 +43,7 @@
> > >  #define DRIVER_VGEM	(1 << 2)
> > >  #define DRIVER_VIRTIO	(1 << 3)
> > >  #define DRIVER_AMDGPU	(1 << 4)
> > > +#define DRIVER_VMWGFX	(1 << 5)
> > 
> > This seems not needed? For pure generic kms tests I think it'd be great if
> > we don't have to sprinkle driver-specific checks all over. Which you seem
> > to achive in your series here.
> > 
> > So not clear why this here is needed?
> > -Daniel
> 
> Hi Daniel,
> 
> Thanks for the review. You are right for kms tests having vmwgfx driver type
> is not needed but I added vmwgfx device type because I plan to add more
> vmwgfx test cases, and since vmwgfx buffer allocation is private ioctl
> having a separate device type might be needed.

Oh, this sounds awesome. And yes, for private ioctl tests vmwgfx is
obviuosly very much welcome!

> I can drop this until I have some vmwgfx specific test cases.

Sounds like a good plan to me.
-Daniel

> 
> > 
> > >  /*
> > >   * Exclude DRVER_VGEM from DRIVER_ANY since if you run on a system
> > >   * with vgem as well as a supported driver, you can end up with a
> > > @@ -80,6 +81,8 @@ void igt_require_intel(int fd);
> > >
> > >  bool is_i915_device(int fd);
> > >
> > > +bool is_vmwgfx_device(int fd);
> > > +
> > >  /**
> > >   * do_or_die:
> > >   * @x: command
> > > --
> > > 2.17.1
> > >
> > > _______________________________________________
> > > igt-dev mailing list
> > > igt-dev@lists.freedesktop.org
> > >
> > https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.fr
> > eedesktop.org%2Fmailman%2Flistinfo%2Figt-
> > dev&amp;data=02%7C01%7Cdrawat%40vmware.com%7C5486d67be73647b1
> > 1d2708d62f58307d%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C1%7C0%7C6
> > 36748453147406904&amp;sdata=QcT4VTQth8GuXgjZejCsnFPDsMFbtqrUOW
> > uSDr8zbG8%3D&amp;reserved=0
> > 
> > --
> > Daniel Vetter
> > Software Engineer, Intel Corporation
> > https://na01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fblog.ff
> > wll.ch&amp;data=02%7C01%7Cdrawat%40vmware.com%7C5486d67be73647
> > b11d2708d62f58307d%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C1%7C0%7
> > C636748453147406904&amp;sdata=4SKi8P5PeyCAUsoZh%2BsYFC%2FU2RiEx5
> > qp2gPP7bPr3Bo%3D&amp;reserved=0

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

Re: [igt-dev] [PATCH i-g-t 1/6] lib/igt_vmwgfx: Add vmwgfx device

[Deepak Singh Rawat]

> > > This seems not needed? For pure generic kms tests I think it'd be great if
> > > we don't have to sprinkle driver-specific checks all over. Which you seem
> > > to achive in your series here.
> > >
> > > So not clear why this here is needed?
> > > -Daniel
> >
> > Hi Daniel,
> >
> > Thanks for the review. You are right for kms tests having vmwgfx driver
> type
> > is not needed but I added vmwgfx device type because I plan to add more
> > vmwgfx test cases, and since vmwgfx buffer allocation is private ioctl
> > having a separate device type might be needed.
> 
> Oh, this sounds awesome. And yes, for private ioctl tests vmwgfx is
> obviuosly very much welcome!
> 
> > I can drop this until I have some vmwgfx specific test cases.
> 
> Sounds like a good plan to me.
> -Daniel

I saw latest email from Perti mentioning that this might be needed for loading
ko. I am really not sure about that but I will test without vmwgfx as a separate
driver type and see if things work.

Thanks,
Deepak


_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [igt-dev] [PATCH i-g-t 1/6] lib/igt_vmwgfx: Add vmwgfx device

[Deepak Singh Rawat]

> > > This seems not needed? For pure generic kms tests I think it'd be great if
> > > we don't have to sprinkle driver-specific checks all over. Which you seem
> > > to achive in your series here.
> > >
> > > So not clear why this here is needed?
> > > -Daniel
> >
> > Hi Daniel,
> >
> > Thanks for the review. You are right for kms tests having vmwgfx driver
> type
> > is not needed but I added vmwgfx device type because I plan to add more
> > vmwgfx test cases, and since vmwgfx buffer allocation is private ioctl
> > having a separate device type might be needed.
> 
> Oh, this sounds awesome. And yes, for private ioctl tests vmwgfx is
> obviuosly very much welcome!
> 
> > I can drop this until I have some vmwgfx specific test cases.
> 
> Sounds like a good plan to me.
> -Daniel

I saw latest email from Perti mentioning that this might be needed for loading
ko. I am really not sure about that but I will test without vmwgfx as a separate
driver type and see if things work.

Thanks,
Deepak


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

Re: [igt-dev] [PATCH i-g-t 4/6] lib: Don't call igt_require_fb_modifiers() when no modifier

[Deepak Singh Rawat]

> On Wed, Oct 10, 2018 at 05:21:02PM -0700, Deepak Rawat wrote:
> > vmwgfx doesn't support fb modifier so skip igt_require_fb_modifiers()
> > when modifier are not passed.
> >
> > Signed-off-by: Deepak Rawat <drawat@vmware.com>
> > ---
> >  lib/ioctl_wrappers.c | 5 ++++-
> >  1 file changed, 4 insertions(+), 1 deletion(-)
> >
> > diff --git a/lib/ioctl_wrappers.c b/lib/ioctl_wrappers.c
> > index 0929c43f..3a11cb6e 100644
> > --- a/lib/ioctl_wrappers.c
> > +++ b/lib/ioctl_wrappers.c
> > @@ -1678,7 +1678,10 @@ int __kms_addfb(int fd, uint32_t handle,
> >  	struct drm_mode_fb_cmd2 f;
> >  	int ret, i;
> >
> > -	igt_require_fb_modifiers(fd);
> > +	if (modifier)
> > +		igt_require_fb_modifiers(fd);
> > +	else
> > +		flags &= ~DRM_MODE_FB_MODIFIERS;
> 
> This would theoretically change the behviour for i915 at least. Without
> the modifiers flag the kernel will pick the modifier for us based on
> the bo tiling, which in theory might not be what we wanted. But at least
> igt_fb should be fine with that.
> 
> Maybe it would be better to just not pass the flags from the caller at
> all, and instead have __kms_addfb() check if the driver has modifiers
> or not and set the flag based on that?
> 

Thanks Ville for the review. I thought of checking for modifiers support
in __kms_addfb() but I discarded the idea as it looked an overkill to me,
checking each time __kms_addfb() is called. May be static variable will
be a good idea? And yes resetting the flag from caller looks a better way.

Thanks,
Deepak
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [igt-dev] [PATCH i-g-t 4/6] lib: Don't call igt_require_fb_modifiers() when no modifier

[Deepak Singh Rawat]

> On Wed, Oct 10, 2018 at 05:21:02PM -0700, Deepak Rawat wrote:
> > vmwgfx doesn't support fb modifier so skip igt_require_fb_modifiers()
> > when modifier are not passed.
> >
> > Signed-off-by: Deepak Rawat <drawat@vmware.com>
> > ---
> >  lib/ioctl_wrappers.c | 5 ++++-
> >  1 file changed, 4 insertions(+), 1 deletion(-)
> >
> > diff --git a/lib/ioctl_wrappers.c b/lib/ioctl_wrappers.c
> > index 0929c43f..3a11cb6e 100644
> > --- a/lib/ioctl_wrappers.c
> > +++ b/lib/ioctl_wrappers.c
> > @@ -1678,7 +1678,10 @@ int __kms_addfb(int fd, uint32_t handle,
> >  	struct drm_mode_fb_cmd2 f;
> >  	int ret, i;
> >
> > -	igt_require_fb_modifiers(fd);
> > +	if (modifier)
> > +		igt_require_fb_modifiers(fd);
> > +	else
> > +		flags &= ~DRM_MODE_FB_MODIFIERS;
> 
> This would theoretically change the behviour for i915 at least. Without
> the modifiers flag the kernel will pick the modifier for us based on
> the bo tiling, which in theory might not be what we wanted. But at least
> igt_fb should be fine with that.
> 
> Maybe it would be better to just not pass the flags from the caller at
> all, and instead have __kms_addfb() check if the driver has modifiers
> or not and set the flag based on that?
> 

Thanks Ville for the review. I thought of checking for modifiers support
in __kms_addfb() but I discarded the idea as it looked an overkill to me,
checking each time __kms_addfb() is called. May be static variable will
be a good idea? And yes resetting the flag from caller looks a better way.

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

Re: [igt-dev] [PATCH i-g-t 3/6] lib/igt_fb: Check for cairo surface success

[Deepak Singh Rawat]

> 
> On Wed, Oct 10, 2018 at 05:21:01PM -0700, Deepak Rawat wrote:
> > For vmwgfx cairo surface creation fails due to stride mismatch, add a
> > igt_require_f() for surface.
> 
> Hmm. What kind of pixel format are you using?
> 
> It seems to me cairo should be happy with just a 4 byte aligned
> stride. How bad would it be to change vmw_dumb_create() to give
> you that? Having working cairo stuff should make igt_fb much more
> useful for you in general.

I think pixel format was SVGA3D_R5G6B5. For vmwgfx pitch is simply
width * bpp and for only some old length framebuffer, surface creation
was failing. I didn't pursued this in much detail.

Yes I plan to add vmwgfx private ioctl for buffer allocation to igt and
I think that would solve this.

> 
> Alternative would to not use dumb buffers I suppose, but then
> we'd need to make sure the igt_fb size calculations are OK for
> vmwgfx. Currently igt_fb assumes Intel hw in the sense that
> linear buffers will get a 64byte aligned stride.
> 
> >
> > v2: Check for surface creation failure.
> >
> > Signed-off-by: Deepak Rawat <drawat@vmware.com>
> > ---
> >  lib/igt_fb.c | 4 ++++
> >  1 file changed, 4 insertions(+)
> >
> > diff --git a/lib/igt_fb.c b/lib/igt_fb.c
> > index 335ece69..1bb6d324 100644
> > --- a/lib/igt_fb.c
> > +++ b/lib/igt_fb.c
> > @@ -1433,6 +1433,10 @@ static void create_cairo_surface__gtt(int fd,
> struct igt_fb *fb)
> >  		cairo_image_surface_create_for_data(ptr,
> >  						    drm_format_to_cairo(fb-
> >drm_format),
> >  						    fb->width, fb->height, fb-
> >strides[0]);
> > +	igt_require_f(cairo_surface_status(fb->cairo_surface) ==
> CAIRO_STATUS_SUCCESS,
> > +		      "Unable to create a cairo surface: %s\n",
> > +		      cairo_status_to_string(cairo_surface_status(fb-
> >cairo_surface)));
> > +
> >  	fb->domain = I915_GEM_DOMAIN_GTT;
> >
> >  	cairo_surface_set_user_data(fb->cairo_surface,
> > --
> > 2.17.1
> >
> > _______________________________________________
> > igt-dev mailing list
> > igt-dev@lists.freedesktop.org
> >
> https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.fr
> eedesktop.org%2Fmailman%2Flistinfo%2Figt-
> dev&amp;data=02%7C01%7Cdrawat%40vmware.com%7C55643c7eddb14e60
> aa3c08d62f8b500c%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C1%7C0%7C63
> 6748672729440089&amp;sdata=pE7NX1LoOz5qhBZhp%2F2wArWHvKdSF0ctx
> TXyNW5%2Fcig%3D&amp;reserved=0
> 
> --
> Ville Syrjälä
> Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [igt-dev] [PATCH i-g-t 3/6] lib/igt_fb: Check for cairo surface success

[Deepak Singh Rawat]

> 
> On Wed, Oct 10, 2018 at 05:21:01PM -0700, Deepak Rawat wrote:
> > For vmwgfx cairo surface creation fails due to stride mismatch, add a
> > igt_require_f() for surface.
> 
> Hmm. What kind of pixel format are you using?
> 
> It seems to me cairo should be happy with just a 4 byte aligned
> stride. How bad would it be to change vmw_dumb_create() to give
> you that? Having working cairo stuff should make igt_fb much more
> useful for you in general.

I think pixel format was SVGA3D_R5G6B5. For vmwgfx pitch is simply
width * bpp and for only some old length framebuffer, surface creation
was failing. I didn't pursued this in much detail.

Yes I plan to add vmwgfx private ioctl for buffer allocation to igt and
I think that would solve this.

> 
> Alternative would to not use dumb buffers I suppose, but then
> we'd need to make sure the igt_fb size calculations are OK for
> vmwgfx. Currently igt_fb assumes Intel hw in the sense that
> linear buffers will get a 64byte aligned stride.
> 
> >
> > v2: Check for surface creation failure.
> >
> > Signed-off-by: Deepak Rawat <drawat@vmware.com>
> > ---
> >  lib/igt_fb.c | 4 ++++
> >  1 file changed, 4 insertions(+)
> >
> > diff --git a/lib/igt_fb.c b/lib/igt_fb.c
> > index 335ece69..1bb6d324 100644
> > --- a/lib/igt_fb.c
> > +++ b/lib/igt_fb.c
> > @@ -1433,6 +1433,10 @@ static void create_cairo_surface__gtt(int fd,
> struct igt_fb *fb)
> >  		cairo_image_surface_create_for_data(ptr,
> >  						    drm_format_to_cairo(fb-
> >drm_format),
> >  						    fb->width, fb->height, fb-
> >strides[0]);
> > +	igt_require_f(cairo_surface_status(fb->cairo_surface) ==
> CAIRO_STATUS_SUCCESS,
> > +		      "Unable to create a cairo surface: %s\n",
> > +		      cairo_status_to_string(cairo_surface_status(fb-
> >cairo_surface)));
> > +
> >  	fb->domain = I915_GEM_DOMAIN_GTT;
> >
> >  	cairo_surface_set_user_data(fb->cairo_surface,
> > --
> > 2.17.1
> >
> > _______________________________________________
> > igt-dev mailing list
> > igt-dev@lists.freedesktop.org
> >
> https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.fr
> eedesktop.org%2Fmailman%2Flistinfo%2Figt-
> dev&amp;data=02%7C01%7Cdrawat%40vmware.com%7C55643c7eddb14e60
> aa3c08d62f8b500c%7Cb39138ca3cee4b4aa4d6cd83d9dd62f0%7C1%7C0%7C63
> 6748672729440089&amp;sdata=pE7NX1LoOz5qhBZhp%2F2wArWHvKdSF0ctx
> TXyNW5%2Fcig%3D&amp;reserved=0
> 
> --
> Ville Syrjälä
> Intel
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [igt-dev] [PATCH i-g-t 4/6] lib: Don't call igt_require_fb_modifiers() when no modifier

[Daniel Vetter]

On Thu, Oct 11, 2018 at 03:38:11PM +0000, Deepak Singh Rawat wrote:
> 
> > On Wed, Oct 10, 2018 at 05:21:02PM -0700, Deepak Rawat wrote:
> > > vmwgfx doesn't support fb modifier so skip igt_require_fb_modifiers()
> > > when modifier are not passed.
> > >
> > > Signed-off-by: Deepak Rawat <drawat@vmware.com>
> > > ---
> > >  lib/ioctl_wrappers.c | 5 ++++-
> > >  1 file changed, 4 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/lib/ioctl_wrappers.c b/lib/ioctl_wrappers.c
> > > index 0929c43f..3a11cb6e 100644
> > > --- a/lib/ioctl_wrappers.c
> > > +++ b/lib/ioctl_wrappers.c
> > > @@ -1678,7 +1678,10 @@ int __kms_addfb(int fd, uint32_t handle,
> > >  	struct drm_mode_fb_cmd2 f;
> > >  	int ret, i;
> > >
> > > -	igt_require_fb_modifiers(fd);
> > > +	if (modifier)
> > > +		igt_require_fb_modifiers(fd);
> > > +	else
> > > +		flags &= ~DRM_MODE_FB_MODIFIERS;
> > 
> > This would theoretically change the behviour for i915 at least. Without
> > the modifiers flag the kernel will pick the modifier for us based on
> > the bo tiling, which in theory might not be what we wanted. But at least
> > igt_fb should be fine with that.
> > 
> > Maybe it would be better to just not pass the flags from the caller at
> > all, and instead have __kms_addfb() check if the driver has modifiers
> > or not and set the flag based on that?
> > 
> 
> Thanks Ville for the review. I thought of checking for modifiers support
> in __kms_addfb() but I discarded the idea as it looked an overkill to me,
> checking each time __kms_addfb() is called. May be static variable will
> be a good idea? And yes resetting the flag from caller looks a better way.

I think simply removing the modifier flag iff the driver doesn't support
modifiers _and_ the modifer is 0 would be ok. And yes probably better to
do that in the higher-level calls that care. Just converting igt_fb should
catch a lot of tests.

There's 2 more crc tests you might care about, but vmwgfx doesn't have crc
support so not really relevant. And maybe by then you do have modifier
support :-)
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [igt-dev] [PATCH i-g-t 4/6] lib: Don't call igt_require_fb_modifiers() when no modifier

[Daniel Vetter]

On Thu, Oct 11, 2018 at 03:38:11PM +0000, Deepak Singh Rawat wrote:
> 
> > On Wed, Oct 10, 2018 at 05:21:02PM -0700, Deepak Rawat wrote:
> > > vmwgfx doesn't support fb modifier so skip igt_require_fb_modifiers()
> > > when modifier are not passed.
> > >
> > > Signed-off-by: Deepak Rawat <drawat@vmware.com>
> > > ---
> > >  lib/ioctl_wrappers.c | 5 ++++-
> > >  1 file changed, 4 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/lib/ioctl_wrappers.c b/lib/ioctl_wrappers.c
> > > index 0929c43f..3a11cb6e 100644
> > > --- a/lib/ioctl_wrappers.c
> > > +++ b/lib/ioctl_wrappers.c
> > > @@ -1678,7 +1678,10 @@ int __kms_addfb(int fd, uint32_t handle,
> > >  	struct drm_mode_fb_cmd2 f;
> > >  	int ret, i;
> > >
> > > -	igt_require_fb_modifiers(fd);
> > > +	if (modifier)
> > > +		igt_require_fb_modifiers(fd);
> > > +	else
> > > +		flags &= ~DRM_MODE_FB_MODIFIERS;
> > 
> > This would theoretically change the behviour for i915 at least. Without
> > the modifiers flag the kernel will pick the modifier for us based on
> > the bo tiling, which in theory might not be what we wanted. But at least
> > igt_fb should be fine with that.
> > 
> > Maybe it would be better to just not pass the flags from the caller at
> > all, and instead have __kms_addfb() check if the driver has modifiers
> > or not and set the flag based on that?
> > 
> 
> Thanks Ville for the review. I thought of checking for modifiers support
> in __kms_addfb() but I discarded the idea as it looked an overkill to me,
> checking each time __kms_addfb() is called. May be static variable will
> be a good idea? And yes resetting the flag from caller looks a better way.

I think simply removing the modifier flag iff the driver doesn't support
modifiers _and_ the modifer is 0 would be ok. And yes probably better to
do that in the higher-level calls that care. Just converting igt_fb should
catch a lot of tests.

There's 2 more crc tests you might care about, but vmwgfx doesn't have crc
support so not really relevant. And maybe by then you do have modifier
support :-)
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev