All of lore.kernel.org
 help / color / mirror / Atom feed
* [igt-dev] [i-g-t,v3 0/8] Fix some bugs in amd_freesync_video_mode
@ 2023-07-06  9:09 Tom Chung
  2023-07-06  9:09 ` [igt-dev] [i-g-t, v3 1/8] tests/amdgpu/amd_freesync_video_mode: Adjust indents and spacing Tom Chung
                   ` (8 more replies)
  0 siblings, 9 replies; 10+ messages in thread
From: Tom Chung @ 2023-07-06  9:09 UTC (permalink / raw)
  To: igt-dev; +Cc: christian.koenig

Fix several bugs in tests/amdgpu/amd_freesync_video_mode.c

v2: Fix some errors that report from checkpatch.pl.
v3: Add amd_freesync_video_mode to meson.build in alphabetical order.

Tom Chung (8):
  tests/amdgpu/amd_freesync_video_mode: Adjust indents and spacing
  tests/amdgpu/amd_freesync_video_mode: Fix memory leak and corruption
  tests/amdgpu/amd_freesync_video_mode: Fix wrong resolution setting
    during the test
  tests/amdgpu/amd_freesync_video_mode: Add some code from kms_vrr to
    resolve tearing issue
  tests/amdgpu/amd_freesync_video_mode: Add/remove some test progress
    messages
  tests/amdgpu/amd_freesync_video_mode: Move the vrr setting and display
    reset
  tests/amdgpu/amd_freesync_video_mode: Fix wrong resolution setting for
    some panel
  tests/amdgpu/amd_freesync_video_mode: Add amd_freesync_video_mode to
    meson.build

 tests/amdgpu/amd_freesync_video_mode.c | 677 +++++++++++++------------
 tests/amdgpu/meson.build               |   1 +
 2 files changed, 357 insertions(+), 321 deletions(-)

-- 
2.25.1

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [igt-dev] [i-g-t, v3 1/8] tests/amdgpu/amd_freesync_video_mode: Adjust indents and spacing
  2023-07-06  9:09 [igt-dev] [i-g-t,v3 0/8] Fix some bugs in amd_freesync_video_mode Tom Chung
@ 2023-07-06  9:09 ` Tom Chung
  2023-07-06  9:09 ` [igt-dev] [i-g-t, v3 2/8] tests/amdgpu/amd_freesync_video_mode: Fix memory leak and corruption Tom Chung
                   ` (7 subsequent siblings)
  8 siblings, 0 replies; 10+ messages in thread
From: Tom Chung @ 2023-07-06  9:09 UTC (permalink / raw)
  To: igt-dev; +Cc: christian.koenig

Replace spaces with tabs. Adjust indents and spacing and fix some
other errors report from checkpatch.pl.

Signed-off-by: Tom Chung <chiahsuan.chung@amd.com>
---
 tests/amdgpu/amd_freesync_video_mode.c | 533 ++++++++++++-------------
 1 file changed, 263 insertions(+), 270 deletions(-)

diff --git a/tests/amdgpu/amd_freesync_video_mode.c b/tests/amdgpu/amd_freesync_video_mode.c
index 579d24436..f38d7ce0c 100644
--- a/tests/amdgpu/amd_freesync_video_mode.c
+++ b/tests/amdgpu/amd_freesync_video_mode.c
@@ -42,7 +42,7 @@
  * rate. Finally, the userspace can also derive an appropriate mode for
  * a particular refresh rate based on the FreeSync Mode and add it to the
  * connector’s mode list.
-*/
+ */
 IGT_TEST_DESCRIPTION("This tests transition between normal and FreeSync-Video"
 		     "modes and measures the FPS to ensure vblank events are"
 		     "happening at the expected rate.");
@@ -66,23 +66,23 @@ typedef struct data {
 	int		count_modes;
 
 	uint32_t	preferred_mode_index;
-        uint32_t	base_mode_index;
+	uint32_t	base_mode_index;
 	uint32_t	hdisplay;
 	uint32_t	vdisplay;
 } data_t;
 
 struct fsv_sprite {
-        uint32_t        w;
+	uint32_t    w;
 	uint32_t	h;
-        uint32_t        *data;
+	uint32_t    *data;
 };
 static struct fsv_sprite cicle_sprite;
 
 enum {
-        FSV_PREFERRED_MODE,
-        FSV_BASE_MODE,
-        FSV_FREESYNC_VIDEO_MODE,
-        FSV_NON_FREESYNC_VIDEO_MODE,
+	FSV_PREFERRED_MODE,
+	FSV_BASE_MODE,
+	FSV_FREESYNC_VIDEO_MODE,
+	FSV_NON_FREESYNC_VIDEO_MODE,
 };
 
 enum {
@@ -93,10 +93,10 @@ enum {
 };
 
 enum {
-	SCENE_BASE_MODE_TO_VARIOUS_FSV_MODE ,
-	SCENE_LOWER_FSV_MODE_TO_HIGHER_FSV_MODE ,
-	SCENE_NON_FSV_MODE_TO_FSV_MODE ,
-	SCENE_BASE_MODE_TO_CUSTUM_MODE ,
+	SCENE_BASE_MODE_TO_VARIOUS_FSV_MODE,
+	SCENE_LOWER_FSV_MODE_TO_HIGHER_FSV_MODE,
+	SCENE_NON_FSV_MODE_TO_FSV_MODE,
+	SCENE_BASE_MODE_TO_CUSTUM_MODE,
 	SCENE_NON_FSV_MODE_TO_NON_FSV_MODE,
 
 	SCENE_COUNT,
@@ -136,6 +136,7 @@ static uint64_t get_kernel_event_ns(data_t *data, uint32_t event)
 static uint64_t get_time_ns(void)
 {
 	struct timespec ts;
+
 	memset(&ts, 0, sizeof(ts));
 	errno = 0;
 
@@ -149,82 +150,76 @@ static uint64_t get_time_ns(void)
 }
 
 static void fbmem_draw_rect(
-		uint32_t *fbmem,
-		uint32_t stride,
-		uint32_t x,
-		uint32_t y,
-		uint32_t w,
-		uint32_t h,
-		uint32_t color)
+	uint32_t *fbmem,
+	uint32_t stride,
+	uint32_t x,
+	uint32_t y,
+	uint32_t w,
+	uint32_t h,
+	uint32_t color)
 {
-        uint32_t offset = y * stride + x;
-
-        for (uint32_t j = 0; j < h; j++) {
-                for (uint32_t i = 0; i < w; i++) {
-                        fbmem[offset + i] = color;
-                }
-                offset += stride;
-        }
+	uint32_t offset = y * stride + x;
+
+	for (uint32_t j = 0; j < h; j++) {
+		for (uint32_t i = 0; i < w; i++)
+			fbmem[offset + i] = color;
+		offset += stride;
+	}
 }
 
 static void fbmem_draw_smpte_pattern(uint32_t *fbmem, int width, int height)
 {
 	uint32_t x, y;
-        uint32_t colors_top[] = {
-                MK_COLOR(192, 192, 192), /* grey */
-                MK_COLOR(192, 192, 0),   /* yellow */
-                MK_COLOR(0, 192, 192),   /* cyan */
-                MK_COLOR(0, 192, 0),     /* green */
-                MK_COLOR(192, 0, 192),   /* magenta */
-                MK_COLOR(192, 0, 0),     /* red */
-                MK_COLOR(0, 0, 192),     /* blue */
-        };
-        uint32_t colors_middle[] = {
-                MK_COLOR(0, 0, 192),     /* blue */
-                MK_COLOR(19, 19, 19),    /* black */
-                MK_COLOR(192, 0, 192),   /* magenta */
-                MK_COLOR(19, 19, 19),    /* black */
-                MK_COLOR(0, 192, 192),   /* cyan */
-                MK_COLOR(19, 19, 19),    /* black */
-                MK_COLOR(192, 192, 192), /* grey */
-        };
-        uint32_t colors_bottom[] = {
-                MK_COLOR(0, 33, 76),     /* in-phase */
-                MK_COLOR(255, 255, 255), /* super white */
-                MK_COLOR(50, 0, 106),    /* quadrature */
-                MK_COLOR(19, 19, 19),    /* black */
-                MK_COLOR(9, 9, 9),       /* 3.5% */
-                MK_COLOR(19, 19, 19),    /* 7.5% */
-                MK_COLOR(29, 29, 29),    /* 11.5% */
-                MK_COLOR(19, 19, 19),    /* black */
-        };
-
-        for (y = 0; y < height * 6 / 9; ++y) {
-                for (x = 0; x < width; ++x)
-                        fbmem[x] =
-                                colors_top[x * 7 / width];
-                fbmem += width;
-        }
-
-        for (; y < height * 7 / 9; ++y) {
-                for (x = 0; x < width; ++x)
-                        fbmem[x] =
-                                colors_middle[x * 7 / width];
-                fbmem += width;
-        }
-
-        for (; y < height; ++y) {
-                for (x = 0; x < width * 5 / 7; ++x)
-                        fbmem[x] =
-                                colors_bottom[x * 4 / (width * 5 / 7)];
-                for (; x < width * 6 / 7; ++x)
-                        fbmem[x] =
-                                colors_bottom[(x - width * 5 / 7) * 3
-                                              / (width / 7) + 4];
-                for (; x < width; ++x)
-                        fbmem[x] = colors_bottom[7];
-                fbmem += width;
-        }
+	uint32_t colors_top[] = {
+		MK_COLOR(192, 192, 192), /* grey */
+		MK_COLOR(192, 192, 0),   /* yellow */
+		MK_COLOR(0, 192, 192),   /* cyan */
+		MK_COLOR(0, 192, 0),     /* green */
+		MK_COLOR(192, 0, 192),   /* magenta */
+		MK_COLOR(192, 0, 0),     /* red */
+		MK_COLOR(0, 0, 192),     /* blue */
+	};
+	uint32_t colors_middle[] = {
+		MK_COLOR(0, 0, 192),     /* blue */
+		MK_COLOR(19, 19, 19),    /* black */
+		MK_COLOR(192, 0, 192),   /* magenta */
+		MK_COLOR(19, 19, 19),    /* black */
+		MK_COLOR(0, 192, 192),   /* cyan */
+		MK_COLOR(19, 19, 19),    /* black */
+		MK_COLOR(192, 192, 192), /* grey */
+	};
+	uint32_t colors_bottom[] = {
+		MK_COLOR(0, 33, 76),     /* in-phase */
+		MK_COLOR(255, 255, 255), /* super white */
+		MK_COLOR(50, 0, 106),    /* quadrature */
+		MK_COLOR(19, 19, 19),    /* black */
+		MK_COLOR(9, 9, 9),       /* 3.5% */
+		MK_COLOR(19, 19, 19),    /* 7.5% */
+		MK_COLOR(29, 29, 29),    /* 11.5% */
+		MK_COLOR(19, 19, 19),    /* black */
+	};
+
+	for (y = 0; y < height * 6 / 9; ++y) {
+		for (x = 0; x < width; ++x)
+			fbmem[x] = colors_top[x * 7 / width];
+		fbmem += width;
+	}
+
+	for (; y < height * 7 / 9; ++y) {
+		for (x = 0; x < width; ++x)
+			fbmem[x] = colors_middle[x * 7 / width];
+		fbmem += width;
+	}
+
+	for (; y < height; ++y) {
+		for (x = 0; x < width * 5 / 7; ++x)
+			fbmem[x] = colors_bottom[x * 4 / (width * 5 / 7)];
+		for (; x < width * 6 / 7; ++x)
+			fbmem[x] = colors_bottom[(x - width * 5 / 7) * 3 / (width / 7) + 4];
+		for (; x < width; ++x)
+			fbmem[x] = colors_bottom[7];
+		fbmem += width;
+	}
 }
 
 static void sprite_init(
@@ -232,13 +227,13 @@ static void sprite_init(
 		uint32_t w,
 		uint32_t h)
 {
-        igt_assert(sprite);
+	igt_assert(sprite);
 
-        sprite->data = (uint32_t *)malloc(w * h * BYTES_PER_PIXEL);
-        igt_assert(sprite->data);
+	sprite->data = (uint32_t *)malloc(w * h * BYTES_PER_PIXEL);
+	igt_assert(sprite->data);
 
-        sprite->w = w;
-        sprite->h = h;
+	sprite->w = w;
+	sprite->h = h;
 }
 
 static void sprite_paste(
@@ -248,14 +243,14 @@ static void sprite_paste(
 		uint32_t x,
 		uint32_t y)
 {
-        uint32_t fb_offset = y * fb_stride + x;
-        uint32_t sprite_offset = 0;
-
-        for (int j = 0; j < sprite->h; j++) {
-                memcpy(fbmem + fb_offset, sprite->data + sprite_offset, sprite->w * 4);
-                sprite_offset += sprite->w;
-                fb_offset += fb_stride;
-        }
+	uint32_t fb_offset = y * fb_stride + x;
+	uint32_t sprite_offset = 0;
+
+	for (int j = 0; j < sprite->h; j++) {
+		memcpy(fbmem + fb_offset, sprite->data + sprite_offset, sprite->w * 4);
+		sprite_offset += sprite->w;
+		fb_offset += fb_stride;
+	}
 }
 
 static void sprite_draw_rect(
@@ -266,16 +261,15 @@ static void sprite_draw_rect(
 		uint32_t h,
 		uint32_t color)
 {
-        uint32_t offset = y * sprite->w + x;
-        uint32_t *addr = (uint32_t *)sprite->data;
-
-        for (uint32_t j = 0; j < h; j++) {
-                addr = (uint32_t *)(sprite->data + offset);
-                for (uint32_t i = 0; i < w; i++) {
-                        addr[i] = color;
-                }
-                offset += sprite->w;
-        }
+	uint32_t offset = y * sprite->w + x;
+	uint32_t *addr = (uint32_t *)sprite->data;
+
+	for (uint32_t j = 0; j < h; j++) {
+		addr = (uint32_t *)(sprite->data + offset);
+		for (uint32_t i = 0; i < w; i++)
+			addr[i] = color;
+		offset += sprite->w;
+	}
 }
 
 /* drawing horizontal line in the sprite */
@@ -287,9 +281,9 @@ static void sprite_draw_hline(
 		uint32_t color)
 {
 	uint32_t offset = y1 * sprite->w;
-        for (int x = x1 ; x < x2; x++) {
-                sprite->data[offset + x] = color;
-        }
+
+	for (int x = x1 ; x < x2; x++)
+		sprite->data[offset + x] = color;
 }
 
 /* drawing filled circle with Bresenham's algorithm */
@@ -300,155 +294,157 @@ static void sprite_draw_circle(
 		uint32_t radius,
 		uint32_t color)
 {
-        int offsetx = 0, offsety = radius, d = radius -1;
-
-        while (offsety >= offsetx) {
-                sprite_draw_hline(sprite, x - offsety, y + offsetx,
-                                x + offsety, color);
-                sprite_draw_hline(sprite, x - offsetx, y + offsety,
-                                x + offsetx, color);
-                sprite_draw_hline(sprite, x - offsetx, y - offsety,
-                                x + offsetx, color);
-                sprite_draw_hline(sprite, x - offsety, y - offsetx,
-                                x + offsety, color);
-
-                if (d >= 2 * offsetx) {
-                        d -= 2 * offsetx + 1;
-                        offsetx += 1;
-                } else if (d < 2 * (radius - offsety)) {
-                        d += 2 * offsety - 1;
-                        offsety -= 1;
-                } else {
-                        d += 2 * (offsety - offsetx - 1);
-                        offsety -= 1;
-                        offsetx += 1;
-                }
-        }
+	int offsetx = 0, offsety = radius, d = radius - 1;
+
+	while (offsety >= offsetx) {
+		sprite_draw_hline(sprite, x - offsety, y + offsetx,
+				  x + offsety, color);
+		sprite_draw_hline(sprite, x - offsetx, y + offsety,
+				  x + offsetx, color);
+		sprite_draw_hline(sprite, x - offsetx, y - offsety,
+				  x + offsetx, color);
+		sprite_draw_hline(sprite, x - offsety, y - offsetx,
+				  x + offsety, color);
+
+		if (d >= 2 * offsetx) {
+			d -= 2 * offsetx + 1;
+			offsetx += 1;
+		} else if (d < 2 * (radius - offsety)) {
+			d += 2 * offsety - 1;
+			offsety -= 1;
+		} else {
+			d += 2 * (offsety - offsetx - 1);
+			offsety -= 1;
+			offsetx += 1;
+		}
+	}
 }
 
 static void sprite_anim_init(void)
 {
-        memset(&cicle_sprite, 0, sizeof(cicle_sprite));
-        sprite_init(&cicle_sprite, 100, 100);
+	memset(&cicle_sprite, 0, sizeof(cicle_sprite));
+	sprite_init(&cicle_sprite, 100, 100);
 
-        sprite_draw_rect(&cicle_sprite, 0, 0, 100, 100, MK_COLOR(128, 128, 128));
+	sprite_draw_rect(&cicle_sprite, 0, 0, 100, 100, MK_COLOR(128, 128, 128));
 	/* draw filled circle with center (50, 50), radius 50. */
-        sprite_draw_circle(&cicle_sprite, 50, 50, 50, MK_COLOR(0, 0, 255));
+	sprite_draw_circle(&cicle_sprite, 50, 50, 50, MK_COLOR(0, 0, 255));
 }
 
 static void sprite_anim(data_t *data, uint32_t *addr)
 {
-        struct timeval tv1, tv2, tv_delta;
-        uint64_t frame_ns = get_time_ns();
-        double now = frame_ns / (double)NSECS_PER_SEC;
+	struct timeval tv1, tv2, tv_delta;
+	uint64_t frame_ns = get_time_ns();
+	double now = frame_ns / (double)NSECS_PER_SEC;
 
-        gettimeofday(&tv1, NULL);
+	gettimeofday(&tv1, NULL);
 
-        fbmem_draw_rect(addr, data->hdisplay, 0, 0,
+	fbmem_draw_rect(addr, data->hdisplay, 0, 0,
 			data->hdisplay, data->vdisplay, MK_COLOR(128, 128, 128));
 	/* red rectangle for checking tearing effect*/
-        if (data->front) {
-                fbmem_draw_rect(addr, data->hdisplay, 0, 0,
+	if (data->front) {
+		fbmem_draw_rect(addr, data->hdisplay, 0, 0,
 			30, data->vdisplay, MK_COLOR(191, 0, 0));
-        }
+	}
 
 	/* draw 16 filled circles */
-        for (int i = 0; i < 16; ++i) {
-                double tv = now + i * 0.25;
-                float x, y;
-                x = data->hdisplay - 10.0f - 118.0f * i - 100.0f;
-                y = data->vdisplay * 0.5f + cos(tv) * data->vdisplay * 0.35;
-                sprite_paste(addr, data->hdisplay, &cicle_sprite, (uint32_t)x, (uint32_t)y);
-        }
-
-        gettimeofday(&tv2, NULL);
-        timersub(&tv2, &tv1, &tv_delta);
-
-        igt_debug("time of drawing: %ld ms\n", tv_delta.tv_usec / 1000);
+	for (int i = 0; i < 16; ++i) {
+		double tv = now + i * 0.25;
+		float x, y;
+
+		x = data->hdisplay - 10.0f - 118.0f * i - 100.0f;
+		y = data->vdisplay * 0.5f + cos(tv) * data->vdisplay * 0.35;
+		sprite_paste(addr, data->hdisplay, &cicle_sprite, (uint32_t)x, (uint32_t)y);
+	}
+
+	gettimeofday(&tv2, NULL);
+	timersub(&tv2, &tv1, &tv_delta);
+
+	igt_debug("time of drawing: %ld ms\n", tv_delta.tv_usec / 1000);
 }
 
 /*----------------------------------------------------------------------------*/
 
 /* The freesync video modes is derived from the base mode(the mode with the
-   highest clock rate, and has the same resolution with preferred mode) by
-   amdgpu driver. They have the same clock rate with base mode, and the
-   type of mode has been set as DRM_MODE_TYPE_DRIVER"
-*/
+ * highest clock rate, and has the same resolution with preferred mode) by
+ * amdgpu driver. They have the same clock rate with base mode, and the
+ * type of mode has been set as DRM_MODE_TYPE_DRIVER"
+ */
 static bool is_freesync_video_mode(data_t *data, drmModeModeInfo *mode)
 {
-        drmModeModeInfo *base_mode = &data->modes[data->base_mode_index];
-        uint32_t bm_clock = base_mode->clock;
-
-        if (    mode->hdisplay == data->hdisplay &&
-                mode->vdisplay == data->vdisplay &&
-                mode->clock == bm_clock &&
-		mode->type & DRM_MODE_TYPE_DRIVER) {
-                return true;
-        }
+	drmModeModeInfo *base_mode = &data->modes[data->base_mode_index];
+	uint32_t bm_clock = base_mode->clock;
+
+	if (mode->hdisplay == data->hdisplay &&
+	    mode->vdisplay == data->vdisplay &&
+	    mode->clock == bm_clock &&
+	    mode->type & DRM_MODE_TYPE_DRIVER) {
+		return true;
+	}
 
-        return false;
+	return false;
 }
 
-static drmModeModeInfo* select_mode(
-        data_t *data,
-        uint32_t mode_type,
-        int refresh_rate)
+static drmModeModeInfo *select_mode(
+	data_t *data,
+	uint32_t mode_type,
+	int refresh_rate)
 {
 	int i;
-        int index;
-        drmModeModeInfo *mode = NULL;
+	int index;
+	drmModeModeInfo *mode = NULL;
+
 	igt_debug("select_mode: type=%d, refresh_rate=%d\n", mode_type, refresh_rate);
 
-        switch (mode_type) {
-        case FSV_BASE_MODE:
-                index = data->base_mode_index;
-                mode = &data->modes[index];
-                break;
-
-        case FSV_PREFERRED_MODE:
-                index = data->preferred_mode_index;
-                mode = &data->modes[index];
-                break;
-
-        case FSV_FREESYNC_VIDEO_MODE:
-                for (i = 0; i < data->count_modes; i++) {
-                        mode = &data->modes[i];
-                        if (    mode->vrefresh == refresh_rate &&
-                                is_freesync_video_mode(data, mode)) {
-                                break;
-                        }
-                }
+	switch (mode_type) {
+	case FSV_BASE_MODE:
+		index = data->base_mode_index;
+		mode = &data->modes[index];
+		break;
+
+	case FSV_PREFERRED_MODE:
+		index = data->preferred_mode_index;
+		mode = &data->modes[index];
+		break;
+
+	case FSV_FREESYNC_VIDEO_MODE:
+		for (i = 0; i < data->count_modes; i++) {
+			mode = &data->modes[i];
+			if (mode->vrefresh == refresh_rate &&
+			    is_freesync_video_mode(data, mode)) {
+				break;
+			}
+		}
 		if (i == data->count_modes)
 			mode = NULL;
-                break;
-
-        case FSV_NON_FREESYNC_VIDEO_MODE:
-                for (i = 0; i < data->count_modes; i++) {
-                        mode = &data->modes[i];
-                        if (    mode->vrefresh == refresh_rate &&
-                                !is_freesync_video_mode(data, mode)) {
-                                break;
-                        }
-                }
+		break;
+
+	case FSV_NON_FREESYNC_VIDEO_MODE:
+		for (i = 0; i < data->count_modes; i++) {
+			mode = &data->modes[i];
+			if (mode->vrefresh == refresh_rate &&
+			    !is_freesync_video_mode(data, mode)) {
+				break;
+			}
+		}
 		if (i == data->count_modes)
 			mode = NULL;
-                break;
+		break;
 
-        default:
-                igt_assert("Cannot find mode with specified rate and type.");
-                break;
-        }
+	default:
+		igt_assert("Cannot find mode with specified rate and type.");
+		break;
+	}
 
 	if (mode) {
 		igt_info("selected mode:\n");
 		kmstest_dump_mode(mode);
 	}
 
-        return mode;
+	return mode;
 }
 
 static int prepare_custom_mode(
-        data_t *data,
+	data_t *data,
 	drmModeModeInfo *custom_mode,
 	uint32_t refresh_rate)
 {
@@ -477,10 +473,8 @@ static int prepare_custom_mode(
 	den = refresh_rate * 1000 * (unsigned long long)base_mode->htotal;
 	target_vtotal = num / den;
 	target_vtotal_diff = target_vtotal - base_mode->vtotal;
-	igt_debug("num=%lu, den=%lu, " \
-                  "target_vtotal=%lu, target_vtotal_diff=%lu, base_mode->vtotal=%d\n",
-		  num, den, target_vtotal, target_vtotal_diff, base_mode->vtotal
-		);
+	igt_debug("num=%lu, den=%lu, target_vtotal=%lu, target_vtotal_diff=%lu, base_mode->vtotal=%d\n",
+		num, den, target_vtotal, target_vtotal_diff, base_mode->vtotal);
 
 	/* Check for illegal modes */
 	if (base_mode->vsync_start + target_vtotal_diff < base_mode->vdisplay ||
@@ -680,7 +674,8 @@ flip_and_measure(
 	return total_flip ? ((total_pass * 100) / total_flip) : 0;
 }
 
-static void init_data(data_t *data, igt_output_t *output) {
+static void init_data(data_t *data, igt_output_t *output)
+{
 	int i;
 	uint32_t pm_hdisplay, pm_vdisplay, max_clk = 0;
 	drmModeModeInfo *preferred_mode;
@@ -699,36 +694,38 @@ static void init_data(data_t *data, igt_output_t *output) {
 	}
 
 	/* searching the preferred mode */
-        for (i = 0; i < connector->count_modes; i++) {
-                drmModeModeInfo *mode = &connector->modes[i];
+	for (i = 0; i < connector->count_modes; i++) {
+		drmModeModeInfo *mode = &connector->modes[i];
 
-                if (mode->type & DRM_MODE_TYPE_PREFERRED) {
-                        data->preferred_mode_index = i;
+		if (mode->type & DRM_MODE_TYPE_PREFERRED) {
+			data->preferred_mode_index = i;
 			data->hdisplay = mode->hdisplay;
 			data->vdisplay = mode->vdisplay;
 			pm_hdisplay = preferred_mode->hdisplay;
 			pm_vdisplay = preferred_mode->vdisplay;
 			break;
-                }
-        }
-
-        /* searching the base mode; */
-        for (i = 0; i < connector->count_modes; i++) {
-                drmModeModeInfo *mode = &connector->modes[i];
-                if (mode->hdisplay == pm_hdisplay && mode->vdisplay == pm_vdisplay) {
-                        if (mode->clock > max_clk) {
-                                max_clk = mode->clock;
-                                data->base_mode_index = i;
-                        }
-                }
-        }
-        igt_info("preferred=%d, base=%d\n", data->preferred_mode_index, data->base_mode_index);
-
-        for (i = 0; i < connector->count_modes; i++) {
-                drmModeModeInfo *mode = &connector->modes[i];
-                if (is_freesync_video_mode(data, mode))
-                        igt_debug("mode[%d] is freesync video mode.\n", i);
-        }
+		}
+	}
+
+	/* searching the base mode; */
+	for (i = 0; i < connector->count_modes; i++) {
+		drmModeModeInfo *mode = &connector->modes[i];
+
+		if (mode->hdisplay == pm_hdisplay && mode->vdisplay == pm_vdisplay) {
+			if (mode->clock > max_clk) {
+				max_clk = mode->clock;
+				data->base_mode_index = i;
+			}
+		}
+	}
+	igt_info("preferred=%d, base=%d\n", data->preferred_mode_index, data->base_mode_index);
+
+	for (i = 0; i < connector->count_modes; i++) {
+		drmModeModeInfo *mode = &connector->modes[i];
+
+		if (is_freesync_video_mode(data, mode))
+			igt_debug("mode[%d] is freesync video mode.\n", i);
+	}
 
 	data->range = get_vrr_range(data, output);
 }
@@ -758,20 +755,20 @@ mode_transition(data_t *data, enum pipe pipe, igt_output_t *output, uint32_t sce
 	sprite_anim_init();
 
 	igt_info("stage-1:\n");
-	switch(scene) {
-        case SCENE_BASE_MODE_TO_VARIOUS_FSV_MODE:
+	switch (scene) {
+	case SCENE_BASE_MODE_TO_VARIOUS_FSV_MODE:
 		mode_start = select_mode(data, FSV_BASE_MODE, 0);
-                mode_playback  = select_mode(data, FSV_FREESYNC_VIDEO_MODE, 60);
+		mode_playback  = select_mode(data, FSV_FREESYNC_VIDEO_MODE, 60);
 		break;
-        case SCENE_LOWER_FSV_MODE_TO_HIGHER_FSV_MODE:
+	case SCENE_LOWER_FSV_MODE_TO_HIGHER_FSV_MODE:
 		mode_start = select_mode(data, FSV_FREESYNC_VIDEO_MODE, 60);
-                mode_playback = select_mode(data, FSV_FREESYNC_VIDEO_MODE, 120);
+		mode_playback = select_mode(data, FSV_FREESYNC_VIDEO_MODE, 120);
 		break;
-        case SCENE_NON_FSV_MODE_TO_FSV_MODE:
+	case SCENE_NON_FSV_MODE_TO_FSV_MODE:
 		mode_start = select_mode(data, FSV_NON_FREESYNC_VIDEO_MODE, 60);
-                mode_playback = select_mode(data, FSV_FREESYNC_VIDEO_MODE, 60);
+		mode_playback = select_mode(data, FSV_FREESYNC_VIDEO_MODE, 60);
 		break;
-        case SCENE_BASE_MODE_TO_CUSTUM_MODE:
+	case SCENE_BASE_MODE_TO_CUSTUM_MODE:
 		mode_start = select_mode(data, FSV_BASE_MODE, 0);
 		prepare_custom_mode(data, &mode_custom, 72);
 		mode_playback = &mode_custom;
@@ -787,13 +784,13 @@ mode_transition(data_t *data, enum pipe pipe, igt_output_t *output, uint32_t sce
 	igt_assert_f(mode_start && mode_playback,
 			"Failure on selecting mode with given type and refresh rate.\n");
 	prepare_test(data, output, pipe, mode_start);
-	interval = nsec_per_frame(mode_start->vrefresh) ;
+	interval = nsec_per_frame(mode_start->vrefresh);
 	set_vrr_on_pipe(data, pipe, 1);
 	result = flip_and_measure(data, output, pipe, interval, TEST_DURATION_NS, ANIM_TYPE_SMPTE);
 
 	igt_info("stage-2: simple animation as video playback\n");
 	prepare_test(data, output, pipe, mode_playback);
-	interval = nsec_per_frame(mode_playback->vrefresh) ;
+	interval = nsec_per_frame(mode_playback->vrefresh);
 	result = flip_and_measure(data, output, pipe, interval, TEST_DURATION_NS, ANIM_TYPE_CIRCLE_WAVE);
 	igt_assert_f(result > 90, "Target refresh rate not meet(result=%d%%\n", result);
 
@@ -824,9 +821,9 @@ run_test(data_t *data, uint32_t scene)
 		igt_skip("No vrr capable outputs found.\n");
 }
 
-igt_main
-{
+igt_main {
 	data_t data = {};
+
 	memset(&data, 0, sizeof(data));
 
 	igt_fixture {
@@ -841,30 +838,26 @@ igt_main
 	}
 
 	/* Expectation: Modeset happens instantaneously without blanking */
-        igt_describe("Test switch from base freesync mode to " \
-                     "various freesync video modes");
-        igt_subtest("freesync-base-to-various")
-		run_test(&data, SCENE_BASE_MODE_TO_VARIOUS_FSV_MODE);
+	igt_describe("Test switch from base freesync mode to various freesync video modes");
+	igt_subtest("freesync-base-to-various")
+	run_test(&data, SCENE_BASE_MODE_TO_VARIOUS_FSV_MODE);
 
 	/* Expectation: Modeset happens instantaneously without blanking */
-        igt_describe("Test switching from lower refresh freesync mode to " \
-                     "another freesync mode with higher refresh rate");
-        igt_subtest("freesync-lower-to-higher")
-		run_test(&data, SCENE_LOWER_FSV_MODE_TO_HIGHER_FSV_MODE);
+	igt_describe("Test switching from lower refresh freesync mode to another freesync mode with higher refresh rate");
+	igt_subtest("freesync-lower-to-higher")
+	run_test(&data, SCENE_LOWER_FSV_MODE_TO_HIGHER_FSV_MODE);
 
 	/* Expectation: Full modeset is triggered. */
-        igt_describe("Test switching from non preferred video mode to " \
-                     "one of freesync video mode");
-        igt_subtest("freesync-non-preferred-to-freesync")
-		run_test(&data, SCENE_NON_FSV_MODE_TO_FSV_MODE);
+	igt_describe("Test switching from non preferred video mode to one of freesync video mode");
+	igt_subtest("freesync-non-preferred-to-freesync")
+	run_test(&data, SCENE_NON_FSV_MODE_TO_FSV_MODE);
 
 	/* Expectation: Modeset happens instantaneously without blanking */
-        igt_describe("Add custom mode through xrandr based on " \
-                     "base freesync mode and apply the new mode");
-        igt_subtest("freesync-custom-mode")
-		run_test(&data, SCENE_BASE_MODE_TO_CUSTUM_MODE);
+	igt_describe("Add custom mode through xrandr based on base freesync mode and apply the new mode");
+	igt_subtest("freesync-custom-mode")
+	run_test(&data, SCENE_BASE_MODE_TO_CUSTUM_MODE);
 
-        igt_info("end of test\n");
+	igt_info("end of test\n");
 
 	igt_fixture {
 		igt_display_fini(&data.display);
-- 
2.25.1

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [igt-dev] [i-g-t, v3 2/8] tests/amdgpu/amd_freesync_video_mode: Fix memory leak and corruption
  2023-07-06  9:09 [igt-dev] [i-g-t,v3 0/8] Fix some bugs in amd_freesync_video_mode Tom Chung
  2023-07-06  9:09 ` [igt-dev] [i-g-t, v3 1/8] tests/amdgpu/amd_freesync_video_mode: Adjust indents and spacing Tom Chung
@ 2023-07-06  9:09 ` Tom Chung
  2023-07-06  9:09 ` [igt-dev] [i-g-t, v3 3/8] tests/amdgpu/amd_freesync_video_mode: Fix wrong resolution setting during the test Tom Chung
                   ` (6 subsequent siblings)
  8 siblings, 0 replies; 10+ messages in thread
From: Tom Chung @ 2023-07-06  9:09 UTC (permalink / raw)
  To: igt-dev; +Cc: christian.koenig

[Why]
There are some memory leak and corruption during the test.

[How]
1. Memory allocated for cicle_sprite.data is too small for drawing the circle.
   Adjust the circle radius from 50 to 48 to fit the cicle_sprite.data size.
2. Pointer preferred_mode does not point to a correct location.
   Use the data->hdisplay and data->vdisplay instead of it.
3. Fix some code for boundary issues.
4. Add some error check for potential memory allocate failed.
5. Free some allocated memory and resources before exit test.
6. Use memcpy() to copy the data->modes data.

Signed-off-by: Tom Chung <chiahsuan.chung@amd.com>
---
 tests/amdgpu/amd_freesync_video_mode.c | 61 +++++++++++++++++---------
 1 file changed, 40 insertions(+), 21 deletions(-)

diff --git a/tests/amdgpu/amd_freesync_video_mode.c b/tests/amdgpu/amd_freesync_video_mode.c
index f38d7ce0c..79e3e493c 100644
--- a/tests/amdgpu/amd_freesync_video_mode.c
+++ b/tests/amdgpu/amd_freesync_video_mode.c
@@ -326,8 +326,8 @@ static void sprite_anim_init(void)
 	sprite_init(&cicle_sprite, 100, 100);
 
 	sprite_draw_rect(&cicle_sprite, 0, 0, 100, 100, MK_COLOR(128, 128, 128));
-	/* draw filled circle with center (50, 50), radius 50. */
-	sprite_draw_circle(&cicle_sprite, 50, 50, 50, MK_COLOR(0, 0, 255));
+	/* draw filled circle with center (50, 50), radius 48. */
+	sprite_draw_circle(&cicle_sprite, 50, 50, 48, MK_COLOR(0, 0, 255));
 }
 
 static void sprite_anim(data_t *data, uint32_t *addr)
@@ -414,7 +414,7 @@ static drmModeModeInfo *select_mode(
 				break;
 			}
 		}
-		if (i == data->count_modes)
+		if (i >= data->count_modes)
 			mode = NULL;
 		break;
 
@@ -426,7 +426,7 @@ static drmModeModeInfo *select_mode(
 				break;
 			}
 		}
-		if (i == data->count_modes)
+		if (i >= data->count_modes)
 			mode = NULL;
 		break;
 
@@ -506,7 +506,7 @@ static uint64_t nsec_per_frame(uint64_t refresh)
 static range_t
 get_vrr_range(data_t *data, igt_output_t *output)
 {
-	char buf[256];
+	char buf[256] = {0};
 	char *start_loc;
 	int fd, res;
 	range_t range;
@@ -557,19 +557,25 @@ static void prepare_test(
 
 	/* Prepare resources */
 	if (!data->fb_initialized) {
-		igt_create_fb(data->drm_fd, mode->hdisplay, mode->vdisplay,
-				DRM_FORMAT_XRGB8888, DRM_FORMAT_MOD_LINEAR, &data->fbs[0]);
+		int fb_id;
+
+		fb_id = igt_create_fb(data->drm_fd, mode->hdisplay, mode->vdisplay,
+			DRM_FORMAT_XRGB8888, DRM_FORMAT_MOD_LINEAR, &data->fbs[0]);
+		igt_assert(fb_id);
+
+		fb_id = igt_create_fb(data->drm_fd, mode->hdisplay, mode->vdisplay,
+			DRM_FORMAT_XRGB8888, DRM_FORMAT_MOD_LINEAR, &data->fbs[1]);
+		igt_assert(fb_id);
 
-		igt_create_fb(data->drm_fd, mode->hdisplay, mode->vdisplay,
-				DRM_FORMAT_XRGB8888, DRM_FORMAT_MOD_LINEAR, &data->fbs[1]);
 		data->fb_mem[0] = igt_fb_map_buffer(data->drm_fd, &data->fbs[0]);
 		data->fb_mem[1] = igt_fb_map_buffer(data->drm_fd, &data->fbs[1]);
+
+		fbmem_draw_smpte_pattern(data->fb_mem[0], data->hdisplay, data->vdisplay);
+		fbmem_draw_smpte_pattern(data->fb_mem[1], data->hdisplay, data->vdisplay);
+
 		data->fb_initialized = true;
 	}
 
-	fbmem_draw_smpte_pattern(data->fb_mem[0], data->hdisplay, data->vdisplay);
-	fbmem_draw_smpte_pattern(data->fb_mem[1], data->hdisplay, data->vdisplay);
-
 	/* Take care of any required modesetting before the test begins. */
 	data->primary = igt_output_get_plane_type(output, DRM_PLANE_TYPE_PRIMARY);
 	igt_plane_set_fb(data->primary, &data->fbs[0]);
@@ -677,21 +683,21 @@ flip_and_measure(
 static void init_data(data_t *data, igt_output_t *output)
 {
 	int i;
-	uint32_t pm_hdisplay, pm_vdisplay, max_clk = 0;
-	drmModeModeInfo *preferred_mode;
+	uint32_t max_clk = 0;
 	drmModeConnector *connector;
 
 	connector = data->connector = output->config.connector;
 	data->count_modes = connector->count_modes;
 	data->modes = (drmModeModeInfo *)malloc(sizeof(drmModeModeInfo) * data->count_modes);
+	igt_assert(data->modes);
+	memcpy(data->modes, connector->modes, sizeof(drmModeModeInfo) * data->count_modes);
 
-	for (i = 0; i < data->count_modes; i++) {
-		data->modes[i] = connector->modes[i];
 #ifdef FSV_DEBUG
+	for (i = 0; i < data->count_modes; i++) {
 		igt_info("mode %d:", i);
 		kmstest_dump_mode(&data->modes[i]);
-#endif
 	}
+#endif
 
 	/* searching the preferred mode */
 	for (i = 0; i < connector->count_modes; i++) {
@@ -701,8 +707,6 @@ static void init_data(data_t *data, igt_output_t *output)
 			data->preferred_mode_index = i;
 			data->hdisplay = mode->hdisplay;
 			data->vdisplay = mode->vdisplay;
-			pm_hdisplay = preferred_mode->hdisplay;
-			pm_vdisplay = preferred_mode->vdisplay;
 			break;
 		}
 	}
@@ -711,7 +715,7 @@ static void init_data(data_t *data, igt_output_t *output)
 	for (i = 0; i < connector->count_modes; i++) {
 		drmModeModeInfo *mode = &connector->modes[i];
 
-		if (mode->hdisplay == pm_hdisplay && mode->vdisplay == pm_vdisplay) {
+		if (mode->hdisplay == data->hdisplay && mode->vdisplay == data->vdisplay) {
 			if (mode->clock > max_clk) {
 				max_clk = mode->clock;
 				data->base_mode_index = i;
@@ -742,11 +746,22 @@ static void finish_test(data_t *data, enum pipe pipe, igt_output_t *output)
 	igt_fb_unmap_buffer(&data->fbs[0], data->fb_mem[0]);
 	igt_remove_fb(data->drm_fd, &data->fbs[1]);
 	igt_remove_fb(data->drm_fd, &data->fbs[0]);
+
+	data->fb_initialized = false;
+	if (data->modes) {
+		free(data->modes);
+		data->modes = NULL;
+	}
+	if (cicle_sprite.data) {
+		free(cicle_sprite.data);
+		memset(&cicle_sprite, 0, sizeof(cicle_sprite));
+	}
 }
 
 static void
 mode_transition(data_t *data, enum pipe pipe, igt_output_t *output, uint32_t scene)
 {
+	int ret;
 	uint32_t result;
 	uint64_t interval;
 	drmModeModeInfo *mode_start = NULL, *mode_playback = NULL, mode_custom;
@@ -770,7 +785,8 @@ mode_transition(data_t *data, enum pipe pipe, igt_output_t *output, uint32_t sce
 		break;
 	case SCENE_BASE_MODE_TO_CUSTUM_MODE:
 		mode_start = select_mode(data, FSV_BASE_MODE, 0);
-		prepare_custom_mode(data, &mode_custom, 72);
+		ret = prepare_custom_mode(data, &mode_custom, 72);
+		igt_assert_eq(ret, 0);
 		mode_playback = &mode_custom;
 		break;
 	case SCENE_NON_FSV_MODE_TO_NON_FSV_MODE:
@@ -861,5 +877,8 @@ igt_main {
 
 	igt_fixture {
 		igt_display_fini(&data.display);
+		close(data.drm_fd);
+		if (data.modes)
+			free(data.modes);
 	}
 }
-- 
2.25.1

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [igt-dev] [i-g-t, v3 3/8] tests/amdgpu/amd_freesync_video_mode: Fix wrong resolution setting during the test
  2023-07-06  9:09 [igt-dev] [i-g-t,v3 0/8] Fix some bugs in amd_freesync_video_mode Tom Chung
  2023-07-06  9:09 ` [igt-dev] [i-g-t, v3 1/8] tests/amdgpu/amd_freesync_video_mode: Adjust indents and spacing Tom Chung
  2023-07-06  9:09 ` [igt-dev] [i-g-t, v3 2/8] tests/amdgpu/amd_freesync_video_mode: Fix memory leak and corruption Tom Chung
@ 2023-07-06  9:09 ` Tom Chung
  2023-07-06  9:09 ` [igt-dev] [i-g-t, v3 4/8] tests/amdgpu/amd_freesync_video_mode: Add some code from kms_vrr to resolve tearing issue Tom Chung
                   ` (5 subsequent siblings)
  8 siblings, 0 replies; 10+ messages in thread
From: Tom Chung @ 2023-07-06  9:09 UTC (permalink / raw)
  To: igt-dev; +Cc: christian.koenig

[Why]
Some monitors may have different resolution for non-Freesync video mode and it may
cause the IGT test set to a wrong resolution during the test.

[How]
Check the resolution and use the same one during select the video mode.

Signed-off-by: Tom Chung <chiahsuan.chung@amd.com>
---
 tests/amdgpu/amd_freesync_video_mode.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/tests/amdgpu/amd_freesync_video_mode.c b/tests/amdgpu/amd_freesync_video_mode.c
index 79e3e493c..cc5b92a81 100644
--- a/tests/amdgpu/amd_freesync_video_mode.c
+++ b/tests/amdgpu/amd_freesync_video_mode.c
@@ -421,10 +421,11 @@ static drmModeModeInfo *select_mode(
 	case FSV_NON_FREESYNC_VIDEO_MODE:
 		for (i = 0; i < data->count_modes; i++) {
 			mode = &data->modes[i];
-			if (mode->vrefresh == refresh_rate &&
-			    !is_freesync_video_mode(data, mode)) {
+			if (mode->hdisplay == data->hdisplay &&
+				mode->vdisplay == data->vdisplay &&
+				mode->vrefresh == refresh_rate &&
+				!is_freesync_video_mode(data, mode))
 				break;
-			}
 		}
 		if (i >= data->count_modes)
 			mode = NULL;
-- 
2.25.1

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [igt-dev] [i-g-t, v3 4/8] tests/amdgpu/amd_freesync_video_mode: Add some code from kms_vrr to resolve tearing issue
  2023-07-06  9:09 [igt-dev] [i-g-t,v3 0/8] Fix some bugs in amd_freesync_video_mode Tom Chung
                   ` (2 preceding siblings ...)
  2023-07-06  9:09 ` [igt-dev] [i-g-t, v3 3/8] tests/amdgpu/amd_freesync_video_mode: Fix wrong resolution setting during the test Tom Chung
@ 2023-07-06  9:09 ` Tom Chung
  2023-07-06  9:09 ` [igt-dev] [i-g-t, v3 5/8] tests/amdgpu/amd_freesync_video_mode: Add/remove some test progress messages Tom Chung
                   ` (4 subsequent siblings)
  8 siblings, 0 replies; 10+ messages in thread
From: Tom Chung @ 2023-07-06  9:09 UTC (permalink / raw)
  To: igt-dev; +Cc: christian.koenig

[Why]
There is a tearing video issue during the stage-2 animation test.

[How]
1. Refer to the kms_vrr IGT code, it needs some delay before the the next flip and
   add some warmup time before do the flip and measure to make the test more accurate.
2. Change the target refresh rate percentage to 75% to align with the kms_vrr test.

Signed-off-by: Tom Chung <chiahsuan.chung@amd.com>
---
 tests/amdgpu/amd_freesync_video_mode.c | 22 ++++++++++++++++++----
 1 file changed, 18 insertions(+), 4 deletions(-)

diff --git a/tests/amdgpu/amd_freesync_video_mode.c b/tests/amdgpu/amd_freesync_video_mode.c
index cc5b92a81..ebbb0b4e2 100644
--- a/tests/amdgpu/amd_freesync_video_mode.c
+++ b/tests/amdgpu/amd_freesync_video_mode.c
@@ -26,7 +26,7 @@
 #include <signal.h>
 
 #define NSECS_PER_SEC		(1000000000ull)
-#define TEST_DURATION_NS	(10 * NSECS_PER_SEC)
+#define TEST_DURATION_NS	(8 * NSECS_PER_SEC)
 
 #define BYTES_PER_PIXEL         4
 #define MK_COLOR(r, g, b)	((0 << 24) | (r << 16) | (g << 8) | b)
@@ -638,7 +638,7 @@ flip_and_measure(
 	igt_info("interval_ns=%lu\n", interval_ns);
 
 	for (;;) {
-		uint64_t event_ns;
+		uint64_t event_ns, wait_ns;
 		int64_t diff_ns;
 
 		data->front = !data->front;
@@ -673,6 +673,19 @@ flip_and_measure(
 
 		if (event_ns - start_ns > duration_ns)
 			break;
+
+		/*
+		 * Burn CPU until next timestamp, sleeping isn't accurate enough.
+		 * The target timestamp is based on the delta b/w event timestamps
+		 * and whatever the time left to reach the expected refresh rate.
+		 */
+		diff_ns = event_ns - target_ns;
+		wait_ns = ((diff_ns + interval_ns - 1) / interval_ns) * interval_ns;
+		wait_ns -= diff_ns;
+		target_ns = event_ns + wait_ns;
+
+		while (get_time_ns() < target_ns - 10)
+			;
 	}
 
 	igt_info("Completed %u flips, %u were in threshold for (%llu Hz) %"PRIu64"ns.\n",
@@ -808,9 +821,10 @@ mode_transition(data_t *data, enum pipe pipe, igt_output_t *output, uint32_t sce
 	igt_info("stage-2: simple animation as video playback\n");
 	prepare_test(data, output, pipe, mode_playback);
 	interval = nsec_per_frame(mode_playback->vrefresh);
+	/* Do a short run with VRR before measure to make sure we measure in a stable state */
+	result = flip_and_measure(data, output, pipe, interval, 2 * NSECS_PER_SEC, ANIM_TYPE_CIRCLE_WAVE);
 	result = flip_and_measure(data, output, pipe, interval, TEST_DURATION_NS, ANIM_TYPE_CIRCLE_WAVE);
-	igt_assert_f(result > 90, "Target refresh rate not meet(result=%d%%\n", result);
-
+	igt_assert_f(result > 75, "Target refresh rate not meet 75%% (result=%d%%\n", result);
 	finish_test(data, pipe, output);
 }
 
-- 
2.25.1

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [igt-dev] [i-g-t, v3 5/8] tests/amdgpu/amd_freesync_video_mode: Add/remove some test progress messages
  2023-07-06  9:09 [igt-dev] [i-g-t,v3 0/8] Fix some bugs in amd_freesync_video_mode Tom Chung
                   ` (3 preceding siblings ...)
  2023-07-06  9:09 ` [igt-dev] [i-g-t, v3 4/8] tests/amdgpu/amd_freesync_video_mode: Add some code from kms_vrr to resolve tearing issue Tom Chung
@ 2023-07-06  9:09 ` Tom Chung
  2023-07-06  9:09 ` [igt-dev] [i-g-t, v3 6/8] tests/amdgpu/amd_freesync_video_mode: Move the vrr setting and display reset Tom Chung
                   ` (3 subsequent siblings)
  8 siblings, 0 replies; 10+ messages in thread
From: Tom Chung @ 2023-07-06  9:09 UTC (permalink / raw)
  To: igt-dev; +Cc: christian.koenig

1. Add/remove some test progress messages.
2. Change the variable name "range" to "vrr_range" to make the code clear.
3. Switch to igt_subtest_with_dynamic.
   It can get more progress messages during the test.

Signed-off-by: Tom Chung <chiahsuan.chung@amd.com>
---
 tests/amdgpu/amd_freesync_video_mode.c | 45 +++++++++++++++-----------
 1 file changed, 26 insertions(+), 19 deletions(-)

diff --git a/tests/amdgpu/amd_freesync_video_mode.c b/tests/amdgpu/amd_freesync_video_mode.c
index ebbb0b4e2..30f2479ff 100644
--- a/tests/amdgpu/amd_freesync_video_mode.c
+++ b/tests/amdgpu/amd_freesync_video_mode.c
@@ -59,7 +59,7 @@ typedef struct data {
 	uint32_t	*fb_mem[2];
 	int		front;
 	bool		fb_initialized;
-	range_t		range;
+	range_t		vrr_range;
 
 	drmModeConnector *connector;
 	drmModeModeInfo *modes;
@@ -432,14 +432,15 @@ static drmModeModeInfo *select_mode(
 		break;
 
 	default:
-		igt_assert("Cannot find mode with specified rate and type.");
+		igt_skip("Cannot find the specified mode type.\n");
 		break;
 	}
 
 	if (mode) {
 		igt_info("selected mode:\n");
 		kmstest_dump_mode(mode);
-	}
+	} else
+		igt_skip("Cannot find the mode with specified rate and type.\n");
 
 	return mode;
 }
@@ -463,10 +464,10 @@ static int prepare_custom_mode(
 		return -1;
 	}
 
-	if (refresh_rate < data->range.min ||
-			refresh_rate > data->range.max) {
+	if (refresh_rate < data->vrr_range.min ||
+			refresh_rate > data->vrr_range.max) {
 		igt_warn("The given refresh rate(%u) should be between the rage of: min=%d, max=%d\n",
-				refresh_rate, data->range.min, data->range.max);
+				refresh_rate, data->vrr_range.min, data->vrr_range.max);
 		return -1;
 	}
 
@@ -525,6 +526,7 @@ get_vrr_range(data_t *data, igt_output_t *output)
 
 	igt_assert(start_loc = strstr(buf, "Max: "));
 	igt_assert_eq(sscanf(start_loc, "Max: %u", &range.max), 1);
+	igt_info("VRR Range: %u-%u Hz\n", range.min, range.max);
 
 	return range;
 }
@@ -635,7 +637,7 @@ flip_and_measure(
 	do_flip(data);
 	start_ns = last_event_ns = target_ns = get_kernel_event_ns(data,
 							DRM_EVENT_FLIP_COMPLETE);
-	igt_info("interval_ns=%lu\n", interval_ns);
+	igt_debug("interval_ns=%lu\n", interval_ns);
 
 	for (;;) {
 		uint64_t event_ns, wait_ns;
@@ -688,8 +690,9 @@ flip_and_measure(
 			;
 	}
 
-	igt_info("Completed %u flips, %u were in threshold for (%llu Hz) %"PRIu64"ns.\n",
-		 total_flip, total_pass, (NSECS_PER_SEC / interval_ns), interval_ns);
+	igt_info("Completed %u flips, %u were in threshold (Rate: %u%%) for (%llu Hz) %"PRIu64"ns.\n",
+		total_flip, total_pass, total_flip ? ((total_pass * 100) / total_flip) : 0,
+		(NSECS_PER_SEC / interval_ns), interval_ns);
 
 	return total_flip ? ((total_pass * 100) / total_flip) : 0;
 }
@@ -736,7 +739,6 @@ static void init_data(data_t *data, igt_output_t *output)
 			}
 		}
 	}
-	igt_info("preferred=%d, base=%d\n", data->preferred_mode_index, data->base_mode_index);
 
 	for (i = 0; i < connector->count_modes; i++) {
 		drmModeModeInfo *mode = &connector->modes[i];
@@ -745,7 +747,7 @@ static void init_data(data_t *data, igt_output_t *output)
 			igt_debug("mode[%d] is freesync video mode.\n", i);
 	}
 
-	data->range = get_vrr_range(data, output);
+	data->vrr_range = get_vrr_range(data, output);
 }
 
 static void finish_test(data_t *data, enum pipe pipe, igt_output_t *output)
@@ -783,7 +785,6 @@ mode_transition(data_t *data, enum pipe pipe, igt_output_t *output, uint32_t sce
 	init_data(data, output);
 	sprite_anim_init();
 
-	igt_info("stage-1:\n");
 	switch (scene) {
 	case SCENE_BASE_MODE_TO_VARIOUS_FSV_MODE:
 		mode_start = select_mode(data, FSV_BASE_MODE, 0);
@@ -813,12 +814,14 @@ mode_transition(data_t *data, enum pipe pipe, igt_output_t *output, uint32_t sce
 	}
 	igt_assert_f(mode_start && mode_playback,
 			"Failure on selecting mode with given type and refresh rate.\n");
+
+	igt_info("stage-1: fps:%d\n", mode_start->vrefresh);
 	prepare_test(data, output, pipe, mode_start);
 	interval = nsec_per_frame(mode_start->vrefresh);
-	set_vrr_on_pipe(data, pipe, 1);
+	set_vrr_on_pipe(data, pipe, true);
 	result = flip_and_measure(data, output, pipe, interval, TEST_DURATION_NS, ANIM_TYPE_SMPTE);
 
-	igt_info("stage-2: simple animation as video playback\n");
+	igt_info("stage-2: simple animation as video playback fps:%d\n", mode_playback->vrefresh);
 	prepare_test(data, output, pipe, mode_playback);
 	interval = nsec_per_frame(mode_playback->vrefresh);
 	/* Do a short run with VRR before measure to make sure we measure in a stable state */
@@ -828,6 +831,7 @@ mode_transition(data_t *data, enum pipe pipe, igt_output_t *output, uint32_t sce
 	finish_test(data, pipe, output);
 }
 
+/* Runs tests on outputs that are VRR capable. */
 static void
 run_test(data_t *data, uint32_t scene)
 {
@@ -837,11 +841,14 @@ run_test(data_t *data, uint32_t scene)
 	for_each_connected_output(&data->display, output) {
 		enum pipe pipe;
 
-		if (!has_vrr(output))
+		if (!has_vrr(output)) {
+			igt_info("%s is not a vrr capable output. Skip it.\n", output->name);
 			continue;
+		}
 
 		for_each_pipe(&data->display, pipe)
 			if (igt_pipe_connector_valid(pipe, output)) {
+				igt_dynamic_f("pipe-%s-%s", kmstest_pipe_name(pipe), output->name)
 				mode_transition(data, pipe, output, scene);
 				found = true;
 				break;
@@ -870,22 +877,22 @@ igt_main {
 
 	/* Expectation: Modeset happens instantaneously without blanking */
 	igt_describe("Test switch from base freesync mode to various freesync video modes");
-	igt_subtest("freesync-base-to-various")
+	igt_subtest_with_dynamic("freesync-base-to-various")
 	run_test(&data, SCENE_BASE_MODE_TO_VARIOUS_FSV_MODE);
 
 	/* Expectation: Modeset happens instantaneously without blanking */
 	igt_describe("Test switching from lower refresh freesync mode to another freesync mode with higher refresh rate");
-	igt_subtest("freesync-lower-to-higher")
+	igt_subtest_with_dynamic("freesync-lower-to-higher")
 	run_test(&data, SCENE_LOWER_FSV_MODE_TO_HIGHER_FSV_MODE);
 
 	/* Expectation: Full modeset is triggered. */
 	igt_describe("Test switching from non preferred video mode to one of freesync video mode");
-	igt_subtest("freesync-non-preferred-to-freesync")
+	igt_subtest_with_dynamic("freesync-non-preferred-to-freesync")
 	run_test(&data, SCENE_NON_FSV_MODE_TO_FSV_MODE);
 
 	/* Expectation: Modeset happens instantaneously without blanking */
 	igt_describe("Add custom mode through xrandr based on base freesync mode and apply the new mode");
-	igt_subtest("freesync-custom-mode")
+	igt_subtest_with_dynamic("freesync-custom-mode")
 	run_test(&data, SCENE_BASE_MODE_TO_CUSTUM_MODE);
 
 	igt_info("end of test\n");
-- 
2.25.1

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [igt-dev] [i-g-t, v3 6/8] tests/amdgpu/amd_freesync_video_mode: Move the vrr setting and display reset
  2023-07-06  9:09 [igt-dev] [i-g-t,v3 0/8] Fix some bugs in amd_freesync_video_mode Tom Chung
                   ` (4 preceding siblings ...)
  2023-07-06  9:09 ` [igt-dev] [i-g-t, v3 5/8] tests/amdgpu/amd_freesync_video_mode: Add/remove some test progress messages Tom Chung
@ 2023-07-06  9:09 ` Tom Chung
  2023-07-06  9:09 ` [igt-dev] [i-g-t, v3 7/8] tests/amdgpu/amd_freesync_video_mode: Fix wrong resolution setting for some panel Tom Chung
                   ` (2 subsequent siblings)
  8 siblings, 0 replies; 10+ messages in thread
From: Tom Chung @ 2023-07-06  9:09 UTC (permalink / raw)
  To: igt-dev; +Cc: christian.koenig

[Why]
We don't need to do the display reset and vrr enable/disable in the middle of the test.

[How]
Move the display reset output and vrr enable/disable setting to the begin/end of the test.

Signed-off-by: Tom Chung <chiahsuan.chung@amd.com>
---
 tests/amdgpu/amd_freesync_video_mode.c | 20 +++++++-------------
 1 file changed, 7 insertions(+), 13 deletions(-)

diff --git a/tests/amdgpu/amd_freesync_video_mode.c b/tests/amdgpu/amd_freesync_video_mode.c
index 30f2479ff..bd3642121 100644
--- a/tests/amdgpu/amd_freesync_video_mode.c
+++ b/tests/amdgpu/amd_freesync_video_mode.c
@@ -552,12 +552,6 @@ static void prepare_test(
 		enum pipe pipe,
 		drmModeModeInfo *mode)
 {
-	/* Reset output */
-	igt_display_reset(&data->display);
-	igt_output_set_pipe(output, pipe);
-
-	igt_output_override_mode(output, mode);
-
 	/* Prepare resources */
 	if (!data->fb_initialized) {
 		int fb_id;
@@ -579,15 +573,11 @@ static void prepare_test(
 		data->fb_initialized = true;
 	}
 
+	/* set output mode */
+	igt_output_override_mode(output, mode);
 	/* Take care of any required modesetting before the test begins. */
 	data->primary = igt_output_get_plane_type(output, DRM_PLANE_TYPE_PRIMARY);
 	igt_plane_set_fb(data->primary, &data->fbs[0]);
-
-	/* Clear vrr_enabled state before enabling it, because
-	 * it might be left enabled if the previous test fails.
-	 */
-	igt_pipe_set_prop_value(&data->display, pipe, IGT_CRTC_VRR_ENABLED, 0);
-
 	igt_display_commit2(&data->display, COMMIT_ATOMIC);
 }
 
@@ -752,7 +742,6 @@ static void init_data(data_t *data, igt_output_t *output)
 
 static void finish_test(data_t *data, enum pipe pipe, igt_output_t *output)
 {
-	set_vrr_on_pipe(data, pipe, 0);
 	igt_plane_set_fb(data->primary, NULL);
 	igt_output_set_pipe(output, PIPE_NONE);
 	igt_output_override_mode(output, NULL);
@@ -828,6 +817,8 @@ mode_transition(data_t *data, enum pipe pipe, igt_output_t *output, uint32_t sce
 	result = flip_and_measure(data, output, pipe, interval, 2 * NSECS_PER_SEC, ANIM_TYPE_CIRCLE_WAVE);
 	result = flip_and_measure(data, output, pipe, interval, TEST_DURATION_NS, ANIM_TYPE_CIRCLE_WAVE);
 	igt_assert_f(result > 75, "Target refresh rate not meet 75%% (result=%d%%\n", result);
+	set_vrr_on_pipe(data, pipe, false);
+
 	finish_test(data, pipe, output);
 }
 
@@ -848,6 +839,9 @@ run_test(data_t *data, uint32_t scene)
 
 		for_each_pipe(&data->display, pipe)
 			if (igt_pipe_connector_valid(pipe, output)) {
+				igt_display_reset(&data->display);
+				igt_output_set_pipe(output, pipe);
+
 				igt_dynamic_f("pipe-%s-%s", kmstest_pipe_name(pipe), output->name)
 				mode_transition(data, pipe, output, scene);
 				found = true;
-- 
2.25.1

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [igt-dev] [i-g-t, v3 7/8] tests/amdgpu/amd_freesync_video_mode: Fix wrong resolution setting for some panel
  2023-07-06  9:09 [igt-dev] [i-g-t,v3 0/8] Fix some bugs in amd_freesync_video_mode Tom Chung
                   ` (5 preceding siblings ...)
  2023-07-06  9:09 ` [igt-dev] [i-g-t, v3 6/8] tests/amdgpu/amd_freesync_video_mode: Move the vrr setting and display reset Tom Chung
@ 2023-07-06  9:09 ` Tom Chung
  2023-07-06  9:09 ` [igt-dev] [i-g-t, v3 8/8] tests/amdgpu/amd_freesync_video_mode: Add amd_freesync_video_mode to meson.build Tom Chung
  2023-07-06 10:44 ` [igt-dev] ✗ Fi.CI.BUILD: failure for Fix some bugs in amd_freesync_video_mode (rev2) Patchwork
  8 siblings, 0 replies; 10+ messages in thread
From: Tom Chung @ 2023-07-06  9:09 UTC (permalink / raw)
  To: igt-dev; +Cc: christian.koenig

[Why]
Some eDP panel VRR range only has 48-60Hz and it will cause some of the test failed.

[How]
Use the VRR range fps to select the mode for test.

Signed-off-by: Tom Chung <chiahsuan.chung@amd.com>
---
 tests/amdgpu/amd_freesync_video_mode.c | 25 ++++++++++++++++---------
 1 file changed, 16 insertions(+), 9 deletions(-)

diff --git a/tests/amdgpu/amd_freesync_video_mode.c b/tests/amdgpu/amd_freesync_video_mode.c
index bd3642121..77ddd85cd 100644
--- a/tests/amdgpu/amd_freesync_video_mode.c
+++ b/tests/amdgpu/amd_freesync_video_mode.c
@@ -421,11 +421,18 @@ static drmModeModeInfo *select_mode(
 	case FSV_NON_FREESYNC_VIDEO_MODE:
 		for (i = 0; i < data->count_modes; i++) {
 			mode = &data->modes[i];
-			if (mode->hdisplay == data->hdisplay &&
-				mode->vdisplay == data->vdisplay &&
-				mode->vrefresh == refresh_rate &&
-				!is_freesync_video_mode(data, mode))
-				break;
+			if (refresh_rate > 0) {
+				if (mode->hdisplay == data->hdisplay &&
+					mode->vdisplay == data->vdisplay &&
+					mode->vrefresh == refresh_rate &&
+					!is_freesync_video_mode(data, mode))
+					break;
+			} else {
+				if (mode->hdisplay == data->hdisplay &&
+					mode->vdisplay == data->vdisplay &&
+					!is_freesync_video_mode(data, mode))
+					break;
+			}
 		}
 		if (i >= data->count_modes)
 			mode = NULL;
@@ -780,16 +787,16 @@ mode_transition(data_t *data, enum pipe pipe, igt_output_t *output, uint32_t sce
 		mode_playback  = select_mode(data, FSV_FREESYNC_VIDEO_MODE, 60);
 		break;
 	case SCENE_LOWER_FSV_MODE_TO_HIGHER_FSV_MODE:
-		mode_start = select_mode(data, FSV_FREESYNC_VIDEO_MODE, 60);
-		mode_playback = select_mode(data, FSV_FREESYNC_VIDEO_MODE, 120);
+		mode_start = select_mode(data, FSV_FREESYNC_VIDEO_MODE, data->vrr_range.min);
+		mode_playback = select_mode(data, FSV_FREESYNC_VIDEO_MODE, data->vrr_range.max);
 		break;
 	case SCENE_NON_FSV_MODE_TO_FSV_MODE:
-		mode_start = select_mode(data, FSV_NON_FREESYNC_VIDEO_MODE, 60);
+		mode_start = select_mode(data, FSV_NON_FREESYNC_VIDEO_MODE, 0);
 		mode_playback = select_mode(data, FSV_FREESYNC_VIDEO_MODE, 60);
 		break;
 	case SCENE_BASE_MODE_TO_CUSTUM_MODE:
 		mode_start = select_mode(data, FSV_BASE_MODE, 0);
-		ret = prepare_custom_mode(data, &mode_custom, 72);
+		ret = prepare_custom_mode(data, &mode_custom, data->vrr_range.min);
 		igt_assert_eq(ret, 0);
 		mode_playback = &mode_custom;
 		break;
-- 
2.25.1

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [igt-dev] [i-g-t, v3 8/8] tests/amdgpu/amd_freesync_video_mode: Add amd_freesync_video_mode to meson.build
  2023-07-06  9:09 [igt-dev] [i-g-t,v3 0/8] Fix some bugs in amd_freesync_video_mode Tom Chung
                   ` (6 preceding siblings ...)
  2023-07-06  9:09 ` [igt-dev] [i-g-t, v3 7/8] tests/amdgpu/amd_freesync_video_mode: Fix wrong resolution setting for some panel Tom Chung
@ 2023-07-06  9:09 ` Tom Chung
  2023-07-06 10:44 ` [igt-dev] ✗ Fi.CI.BUILD: failure for Fix some bugs in amd_freesync_video_mode (rev2) Patchwork
  8 siblings, 0 replies; 10+ messages in thread
From: Tom Chung @ 2023-07-06  9:09 UTC (permalink / raw)
  To: igt-dev; +Cc: christian.koenig

Add amd_freesync_video_mode to meson.build file.

Signed-off-by: Tom Chung <chiahsuan.chung@amd.com>
---
 tests/amdgpu/meson.build | 1 +
 1 file changed, 1 insertion(+)

diff --git a/tests/amdgpu/meson.build b/tests/amdgpu/meson.build
index ce081c35a..9f1165a5b 100644
--- a/tests/amdgpu/meson.build
+++ b/tests/amdgpu/meson.build
@@ -11,6 +11,7 @@ if libdrm_amdgpu.found()
 			  'amd_cp_dma_misc',
 			  'amd_color',
 			  'amd_cs_nop',
+			  'amd_freesync_video_mode',
 			  'amd_hotplug',
 			  'amd_info',
 			  'amd_prime',
-- 
2.25.1

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [igt-dev] ✗ Fi.CI.BUILD: failure for Fix some bugs in amd_freesync_video_mode (rev2)
  2023-07-06  9:09 [igt-dev] [i-g-t,v3 0/8] Fix some bugs in amd_freesync_video_mode Tom Chung
                   ` (7 preceding siblings ...)
  2023-07-06  9:09 ` [igt-dev] [i-g-t, v3 8/8] tests/amdgpu/amd_freesync_video_mode: Add amd_freesync_video_mode to meson.build Tom Chung
@ 2023-07-06 10:44 ` Patchwork
  8 siblings, 0 replies; 10+ messages in thread
From: Patchwork @ 2023-07-06 10:44 UTC (permalink / raw)
  To: Tom Chung; +Cc: igt-dev

== Series Details ==

Series: Fix some bugs in amd_freesync_video_mode (rev2)
URL   : https://patchwork.freedesktop.org/series/120072/
State : failure

== Summary ==

Applying: tests/amdgpu/amd_freesync_video_mode: Adjust indents and spacing
Applying: tests/amdgpu/amd_freesync_video_mode: Fix memory leak and corruption
Patch failed at 0002 tests/amdgpu/amd_freesync_video_mode: Fix memory leak and corruption
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".


^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2023-07-06 10:44 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-07-06  9:09 [igt-dev] [i-g-t,v3 0/8] Fix some bugs in amd_freesync_video_mode Tom Chung
2023-07-06  9:09 ` [igt-dev] [i-g-t, v3 1/8] tests/amdgpu/amd_freesync_video_mode: Adjust indents and spacing Tom Chung
2023-07-06  9:09 ` [igt-dev] [i-g-t, v3 2/8] tests/amdgpu/amd_freesync_video_mode: Fix memory leak and corruption Tom Chung
2023-07-06  9:09 ` [igt-dev] [i-g-t, v3 3/8] tests/amdgpu/amd_freesync_video_mode: Fix wrong resolution setting during the test Tom Chung
2023-07-06  9:09 ` [igt-dev] [i-g-t, v3 4/8] tests/amdgpu/amd_freesync_video_mode: Add some code from kms_vrr to resolve tearing issue Tom Chung
2023-07-06  9:09 ` [igt-dev] [i-g-t, v3 5/8] tests/amdgpu/amd_freesync_video_mode: Add/remove some test progress messages Tom Chung
2023-07-06  9:09 ` [igt-dev] [i-g-t, v3 6/8] tests/amdgpu/amd_freesync_video_mode: Move the vrr setting and display reset Tom Chung
2023-07-06  9:09 ` [igt-dev] [i-g-t, v3 7/8] tests/amdgpu/amd_freesync_video_mode: Fix wrong resolution setting for some panel Tom Chung
2023-07-06  9:09 ` [igt-dev] [i-g-t, v3 8/8] tests/amdgpu/amd_freesync_video_mode: Add amd_freesync_video_mode to meson.build Tom Chung
2023-07-06 10:44 ` [igt-dev] ✗ Fi.CI.BUILD: failure for Fix some bugs in amd_freesync_video_mode (rev2) Patchwork

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.