Re: [PATCH 4/4] igt/gem_stolen: Verify contents of stolen-backed objects across hibernation

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




Hi,

On 02/12/15 09:24, ankitprasad.r.sharma@xxxxxxxxx wrote:
From: Ankitprasad Sharma <ankitprasad.r.sharma@xxxxxxxxx>

This patch verifies if the contents of the stolen backed object were
preserved across hibernation. This is to validate kernel changes related
to moving stolen-backed objects to shmem on hibernation.

Signed-off-by: Ankitprasad Sharma <ankitprasad.r.sharma@xxxxxxxxx>

Please Cc me if it is expected I keep reviewing this.

---
  tests/gem_stolen.c | 86 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
  1 file changed, 86 insertions(+)

diff --git a/tests/gem_stolen.c b/tests/gem_stolen.c
index 3374716..1f13fb0 100644
--- a/tests/gem_stolen.c
+++ b/tests/gem_stolen.c
@@ -290,6 +290,89 @@ static void stolen_fill_purge_test(int fd)
  		gem_close(fd, handle[i]);
  }

+static void stolen_hibernate(int fd)
+{
+	drm_intel_bo *bo;
+	drm_intel_bo *src, *dest;
+	int obj_count = 0, i = 0;
+	int _ret = 0, j = 0;

Don't need to init _ret and j.

+	uint32_t handle[MAX_OBJECTS], src_handle;
+	uint32_t *virt;
+
+	gem_require_stolen_support(fd);
+
+	src_handle = gem_create(fd, SIZE);
+	igt_assert(!src_handle);

gem_create already asserts this, but isn't it also reversed? Does it run and work?

+	src = gem_handle_to_libdrm_bo(bufmgr, fd,
+				     "bo", src_handle);
+	igt_assert(src != NULL);
+
+	_ret = drm_intel_gem_bo_map_gtt(src);
+	igt_assert(!_ret);

I think igt_assert_eq(ret, 0) is a preferred way nowadays.

Also a question, what is the special significance of _ret vs just normal ret in this test?

+
+	virt = src->virtual;
+	for (j = 0; j < SIZE/DWORD_SIZE; j++)
+		virt[j] = DATA;

memset?

+
+	drm_intel_bo_unmap(src);
+	/* Exhaust Stolen space */
+	do {
+		handle[i] = __gem_create_stolen(fd, SIZE);
+		if (handle[i] != 0) {

Maybe it would be more readable and with less indentation:

for (i = 0; i < MAX_OBJECTS; i++) {
	handle[i] = __gem_create_stolen(...);
	if (!handle[i])
		break;

	...

	obj_count++;
}

+			bo = gem_handle_to_libdrm_bo(bufmgr, fd,
+						     "verify_bo", handle[i]);
+			igt_assert(bo != NULL);
+			_ret = drm_intel_gem_bo_map_gtt(bo);
+			igt_assert(!_ret);
+
+			virt = bo->virtual;
+			for (j = 0; j < SIZE/DWORD_SIZE; j++)
+				igt_assert(!virt[j]);

Again, people will probably want igt_assert_eq.

+
+			drm_intel_bo_unmap(bo);
+			drm_intel_bo_unreference(bo);
+
+			obj_count++;
+		}
+
+		i++;
+	} while (handle[i-1] && i < MAX_OBJECTS);
+
+	igt_assert(obj_count > 0);
+
+	for (i = 0; i < obj_count; i++) {
+		dest = gem_handle_to_libdrm_bo(bufmgr, fd,
+					       "dst_bo", handle[i]);
+		igt_assert(dest != NULL);
+		intel_copy_bo(batch, dest, src, SIZE);
+		drm_intel_bo_unreference(dest);
+	}

Probably worth doing a verification step after each blit. So after resume is not the first time you are checking it.

+	drm_intel_bo_unreference(src);
+
+	igt_system_hibernate_autoresume();
+	/* Check if the object's memory contents are intact
+	 * across hibernation.
+	 */
+	for (i = 0; i < obj_count; i++) {
+		bo = gem_handle_to_libdrm_bo(bufmgr, fd,
+					     "verify_bo", handle[i]);
+		igt_assert(bo != NULL);
+		_ret = drm_intel_gem_bo_map_gtt(bo);
+		igt_assert(!_ret);
+		virt = bo->virtual;
+		for (j = 0; j < SIZE/DWORD_SIZE; j++)
+			igt_assert_eq(virt[j], DATA);
+
+		drm_intel_bo_unmap(bo);
+		drm_intel_bo_unreference(bo);
+	}

Would it be interesting to fill the bos with some more elaborate pattern to potentially catch more potential corruption types?

Perhaps incrementing dwords would simply do?

+	gem_close(fd, src_handle);
+	for (i = 0; i < obj_count; i++)
+		gem_close(fd, handle[i]);
+}
+
  static void
  stolen_no_mmap(int fd)
  {
@@ -353,6 +436,9 @@ igt_main
  	igt_subtest("stolen-fill-purge")
  		stolen_fill_purge_test(fd);

+	igt_subtest("stolen-hibernate")
+		stolen_hibernate(fd);
+
  	igt_fixture {
  		intel_batchbuffer_free(batch);
  		drm_intel_bufmgr_destroy(bufmgr);


Regards,

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




[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]
  Powered by Linux