All of lore.kernel.org
 help / color / mirror / Atom feed
* [igt-dev] [PATCH i-g-t] tests/kms_available_modes_crc limit tested fb formats
@ 2019-02-18 14:22 Juha-Pekka Heikkila
  2019-02-18 14:28 ` Ville Syrjälä
                   ` (3 more replies)
  0 siblings, 4 replies; 7+ messages in thread
From: Juha-Pekka Heikkila @ 2019-02-18 14:22 UTC (permalink / raw)
  To: igt-dev

This test is causing too much useless noise. Limit tested
fb formats to DRM_FORMAT_C8 and DRM_FORMAT_XBGR2101010 for now.
These two formats are currently not tested otherwise thus
they're left here for now. DRM_FORMAT_XBGR2101010 need to be
included into IGT supported formats and DRM_FORMAT_C8 test need
to be moved elsewhere, maybe into kms_plane.

Signed-off-by: Juha-Pekka Heikkila <juhapekka.heikkila@gmail.com>
---
 tests/kms_available_modes_crc.c | 217 +++++++++++++++-------------------------
 1 file changed, 83 insertions(+), 134 deletions(-)

diff --git a/tests/kms_available_modes_crc.c b/tests/kms_available_modes_crc.c
index 7ff385f..a51006b 100644
--- a/tests/kms_available_modes_crc.c
+++ b/tests/kms_available_modes_crc.c
@@ -101,17 +101,17 @@ static void generate_comparison_crc_list(data_t *data, igt_output_t *output)
 	igt_plane_set_fb(primary, &data->primary_fb);
 	igt_display_commit2(&data->display, data->commit);
 
+	igt_pipe_crc_drain(data->pipe_crc);
 	igt_pipe_crc_get_current(data->gfx_fd, data->pipe_crc, &data->cursor_crc);
 	igt_plane_set_fb(primary, NULL);
 	igt_display_commit2(&data->display, data->commit);
 
-	intel_gen(intel_get_drm_devid(data->gfx_fd)) < 9 ?
-		  igt_paint_color(cr, 0, 0, mode->hdisplay, mode->vdisplay, 1.0, 1.0, 1.0) :
-		  igt_paint_color_alpha(cr, 0, 0, mode->hdisplay, mode->vdisplay, 1.0, 1.0, 1.0, 1.0);
+	igt_paint_color(cr, 0, 0, mode->hdisplay, mode->vdisplay, 1.0, 1.0, 1.0);
 
 	igt_plane_set_fb(primary, &data->primary_fb);
 	igt_display_commit2(&data->display, data->commit);
 
+	igt_pipe_crc_drain(data->pipe_crc);
 	igt_pipe_crc_get_current(data->gfx_fd, data->pipe_crc, &data->fullscreen_crc);
 
 	cairo_destroy(cr);
@@ -122,48 +122,11 @@ static const struct {
 	uint32_t	fourcc;
 	char		zeropadding;
 	enum		{ BYTES_PP_1=1,
-				BYTES_PP_2=2,
-				BYTES_PP_4=4,
-				NV12,
-				P010,
-				SKIP4 } bpp;
+				BYTES_PP_4=4} bpp;
 	uint32_t	value;
 } fillers[] = {
 	{ DRM_FORMAT_C8, 0, BYTES_PP_1, 0xff},
-	{ DRM_FORMAT_RGB565, 0, BYTES_PP_2, 0xffff},
-	{ DRM_FORMAT_XRGB8888, 0, BYTES_PP_4, 0xffffffff},
-	{ DRM_FORMAT_XBGR8888, 0, BYTES_PP_4, 0xffffffff},
-
-	/*
-	 * following two are skipped because blending seems to work
-	 * incorrectly with exception of AR24 on cursor plane.
-	 * Test still creates the planes, just filling plane
-	 * and getting crc is skipped.
-	 */
-	{ DRM_FORMAT_ARGB8888, 0, SKIP4, 0xffffffff},
-	{ DRM_FORMAT_ABGR8888, 0, SKIP4, 0x00ffffff},
-
-	{ DRM_FORMAT_XRGB2101010, 0, BYTES_PP_4, 0xffffffff},
 	{ DRM_FORMAT_XBGR2101010, 0, BYTES_PP_4, 0xffffffff},
-
-	{ DRM_FORMAT_YUYV, 0, BYTES_PP_4, 0x80eb80eb},
-	{ DRM_FORMAT_YVYU, 0, BYTES_PP_4, 0x80eb80eb},
-	{ DRM_FORMAT_VYUY, 0, BYTES_PP_4, 0xeb80eb80},
-	{ DRM_FORMAT_UYVY, 0, BYTES_PP_4, 0xeb80eb80},
-
-	/*
-	 * (semi-)planar formats
-	 */
-	{ DRM_FORMAT_NV12, 0, NV12, 0x80eb},
-#ifdef DRM_FORMAT_P010
-	{ DRM_FORMAT_P010, 0, P010, 0x8000eb00},
-#endif
-#ifdef DRM_FORMAT_P012
-	{ DRM_FORMAT_P012, 0, P010, 0x8000eb00},
-#endif
-#ifdef DRM_FORMAT_P016
-	{ DRM_FORMAT_P016, 0, P010, 0x8000eb00},
-#endif
 	{ 0, 0, 0, 0 }
 };
 
@@ -175,10 +138,9 @@ static bool fill_in_fb(data_t *data, igt_output_t *output, igt_plane_t *plane,
 		       uint32_t format)
 {
 	signed i, c, writesize;
-	unsigned short* ptemp_16_buf;
 	unsigned int* ptemp_32_buf;
 
-	for( i = 0; fillers[i].fourcc != 0; i++ ) {
+	for( i = 0; i < ARRAY_SIZE(fillers)-1; i++ ) {
 		if( fillers[i].fourcc == format )
 			break;
 	}
@@ -190,53 +152,10 @@ static bool fill_in_fb(data_t *data, igt_output_t *output, igt_plane_t *plane,
 			ptemp_32_buf[c] = fillers[i].value;
 		writesize = data->size;
 		break;
-	case BYTES_PP_2:
-		ptemp_16_buf = (unsigned short*)data->buf;
-		for (c = 0; c < data->size/2; c++)
-			ptemp_16_buf[c] = (unsigned short)fillers[i].value;
-		writesize = data->size;
-		break;
 	case BYTES_PP_1:
 		memset((void *)data->buf, fillers[i].value, data->size);
 		writesize = data->size;
 		break;
-	case NV12:
-		memset((void *)data->buf, fillers[i].value&0xff,
-		       data->fb.offsets[1]);
-
-		memset((void *)(data->buf+data->fb.offsets[1]),
-		       (fillers[i].value>>8)&0xff,
-		       data->size - data->fb.offsets[1]);
-
-		writesize = data->size;
-		break;
-	case P010:
-		ptemp_16_buf = (unsigned short*)data->buf;
-		for (c = 0; c < data->size/2; c++)
-			ptemp_16_buf[c] = (unsigned short)fillers[i].value&0xffff;
-
-		ptemp_16_buf = (unsigned short*)(data->buf+data->size);
-		for (c = 0; c < data->size/2; c++)
-			ptemp_16_buf[c] = (unsigned short)(fillers[i].value>>16)&0xffff;
-
-		writesize = data->size+data->size/2;
-		break;
-	case SKIP4:
-		if (fillers[i].fourcc == DRM_FORMAT_ARGB8888 &&
-		    plane->type == DRM_PLANE_TYPE_CURSOR) {
-		/*
-		 * special for cursor plane where blending works correctly.
-		 */
-			ptemp_32_buf = (unsigned int*)data->buf;
-			for (c = 0; c < data->size/4; c++)
-				ptemp_32_buf[c] = fillers[i].value;
-			writesize = data->size;
-			break;
-		}
-		igt_info("Format %s CRC comparison skipped by design.\n",
-			 (char*)&fillers[i].fourcc);
-
-		return false;
 	default:
 		igt_info("Unsupported mode for test %s\n",
 			 (char*)&fillers[i].fourcc);
@@ -271,26 +190,20 @@ static bool setup_fb(data_t *data, igt_output_t *output, igt_plane_t *plane,
 		tiling = LOCAL_DRM_FORMAT_MOD_NONE;
 	}
 
-	for (i = 0; fillers[i].fourcc != 0; i++) {
-		if (fillers[i].fourcc == format)
+	for( i = 0; i < ARRAY_SIZE(fillers)-1; i++ ) {
+		if( fillers[i].fourcc == format )
 			break;
 	}
 
 	switch (fillers[i].bpp) {
-	case NV12:
 	case BYTES_PP_1:
 		bpp = 8;
 		break;
-
-	case P010:
-	case BYTES_PP_2:
-		bpp = 16;
-		break;
-
-	case SKIP4:
 	case BYTES_PP_4:
 		bpp = 32;
 		break;
+	default:
+		return false;
 	}
 
 	igt_get_fb_tile_size(data->gfx_fd, tiling, bpp,
@@ -299,17 +212,6 @@ static bool setup_fb(data_t *data, igt_output_t *output, igt_plane_t *plane,
 	data->fb.strides[0] = ALIGN(w * bpp / 8, tile_width);
 	gemsize = data->size = data->fb.strides[0] * ALIGN(h, tile_height);
 
-	if (fillers[i].bpp == P010 || fillers[i].bpp == NV12) {
-		data->fb.offsets[1] = data->size;
-		data->fb.strides[1] = data->fb.strides[0];
-		gemsize = data->size * 2;
-
-		if (fillers[i].bpp == NV12)
-			data->size += data->fb.strides[1] * ALIGN(h/2, tile_height);
-
-		num_planes = 2;
-	}
-
 	data->gem_handle = gem_create(data->gfx_fd, gemsize);
 	ret = __gem_set_tiling(data->gfx_fd, data->gem_handle,
 			       igt_fb_mod_to_tiling(tiling),
@@ -386,12 +288,23 @@ static bool prepare_crtc(data_t *data, igt_output_t *output,
 
 static int
 test_one_mode(data_t* data, igt_output_t *output, igt_plane_t* plane,
-	      int mode)
+	      int mode, enum pipe pipe)
 {
 	igt_crc_t current_crc;
 	signed rVal = 0;
 	bool do_crc;
-	char* crccompare[2];
+	int i;
+
+	/*
+	 * Limit tests only to those fb formats listed in fillers table
+	 */
+	for( i = 0; i < ARRAY_SIZE(fillers)-1; i++ ) {
+		if( fillers[i].fourcc == mode )
+			break;
+	}
+
+	if(fillers[i].bpp == 0)
+		return false;
 
 	if (prepare_crtc(data, output, plane, mode)){
 		/*
@@ -412,29 +325,30 @@ test_one_mode(data_t* data, igt_output_t *output, igt_plane_t* plane,
 			if (plane->type != DRM_PLANE_TYPE_CURSOR) {
 				if (!igt_check_crc_equal(&current_crc,
 					&data->fullscreen_crc)) {
-					crccompare[0] = igt_crc_to_string(&current_crc);
-					crccompare[1] = igt_crc_to_string(&data->fullscreen_crc);
-					igt_warn("crc mismatch. target %.8s, result %.8s.\n", crccompare[0], crccompare[1]);
-					free(crccompare[0]);
-					free(crccompare[1]);
+					igt_warn("crc mismatch. connector %s using pipe %s" \
+						" plane index %d mode %.4s\n",
+						igt_output_name(output),
+						kmstest_pipe_name(pipe),
+						plane->index,
+						(char*)&mode);
 					rVal++;
 				}
 			} else {
 				if (!igt_check_crc_equal(&current_crc,
 					&data->cursor_crc)) {
-					crccompare[0] = igt_crc_to_string(&current_crc);
-					crccompare[1] = igt_crc_to_string(&data->cursor_crc);
-					igt_warn("crc mismatch. target %.8s, result %.8s.\n", crccompare[0], crccompare[1]);
-					free(crccompare[0]);
-					free(crccompare[1]);
+					igt_warn("crc mismatch. connector %s using pipe %s" \
+						" plane index %d mode %.4s\n",
+						igt_output_name(output),
+						kmstest_pipe_name(pipe),
+						plane->index,
+						(char*)&mode);
 					rVal++;
 				}
 			}
 		}
-		remove_fb(data, output, plane);
-		return rVal;
 	}
-	return 1;
+	remove_fb(data, output, plane);
+	return rVal;
 }
 
 
@@ -445,14 +359,44 @@ test_available_modes(data_t* data)
 	igt_plane_t *plane;
 	int modeindex;
 	enum pipe pipe;
-	int invalids = 0;
+	int invalids = 0, i, lut_size;
 	drmModePlane *modePlane;
-	char planetype[3][8] = {"OVERLAY\0", "PRIMARY\0", "CURSOR\0" };
+
+	struct {
+	    uint16_t red;
+	    uint16_t green;
+	    uint16_t blue;
+	    uint16_t reserved;
+	} *lut = NULL;
 
 	for_each_pipe_with_valid_output(&data->display, pipe, output) {
 		igt_output_set_pipe(output, pipe);
 		igt_display_commit2(&data->display, data->commit);
 
+		if (igt_pipe_obj_has_prop(&data->display.pipes[pipe], IGT_CRTC_GAMMA_LUT_SIZE)) {
+			lut_size = igt_pipe_get_prop(&data->display, pipe,
+						     IGT_CRTC_GAMMA_LUT_SIZE);
+
+			lut = calloc(sizeof(*lut), lut_size);
+
+			for (i = 0; i < lut_size; i++) {
+				lut[i].red = (i * 0xffff / (lut_size - 1)) & 0x8000;
+				lut[i].green = (i * 0xffff / (lut_size - 1)) & 0x8000;
+				lut[i].blue = (i * 0xffff / (lut_size - 1)) & 0x8000;
+			}
+
+			igt_pipe_replace_prop_blob(&data->display, pipe,
+						   IGT_CRTC_GAMMA_LUT,
+						   lut, sizeof(*lut) * lut_size);
+			igt_display_commit2(&data->display, data->commit);
+
+			for (i = 0; i < lut_size; i++) {
+				lut[i].red = i * 0xffff / (lut_size - 1);
+				lut[i].green = i * 0xffff / (lut_size - 1);
+				lut[i].blue = i * 0xffff / (lut_size - 1);
+			}
+		}
+
 		data->pipe_crc = igt_pipe_crc_new(data->gfx_fd, pipe,
 						  INTEL_PIPE_CRC_SOURCE_AUTO);
 
@@ -467,29 +411,34 @@ test_available_modes(data_t* data)
 			modePlane = drmModeGetPlane(data->gfx_fd,
 						    plane->drm_plane->plane_id);
 
+			if (plane->type == DRM_PLANE_TYPE_CURSOR)
+				continue;
+
 			for (modeindex = 0;
 			     modeindex < modePlane->count_formats;
 			     modeindex++) {
 				data->format.dword = modePlane->formats[modeindex];
 
-				igt_info("Testing connector %s using pipe %s" \
-					 " plane index %d type %s mode %s\n",
-					 igt_output_name(output),
-					 kmstest_pipe_name(pipe),
-					 plane->index,
-					 planetype[plane->type],
-					 (char*)&data->format.name);
-
 				invalids += test_one_mode(data, output,
 							  plane,
-							  modePlane->formats[modeindex]);
+							  modePlane->formats[modeindex],
+							  pipe);
 			}
 			drmModeFreePlane(modePlane);
 		}
 		igt_pipe_crc_stop(data->pipe_crc);
 		igt_pipe_crc_free(data->pipe_crc);
-		igt_display_commit2(&data->display, data->commit);
+
+		if (lut != NULL) {
+			igt_pipe_replace_prop_blob(&data->display, pipe,
+						   IGT_CRTC_GAMMA_LUT,
+						   lut, sizeof(*lut) * lut_size);
+			free(lut);
+			lut = NULL;
+		}
+
 		igt_output_set_pipe(output, PIPE_NONE);
+		igt_display_commit2(&data->display, data->commit);
 	}
 	igt_assert(invalids == 0);
 }
-- 
2.7.4

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

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

* Re: [igt-dev] [PATCH i-g-t] tests/kms_available_modes_crc limit tested fb formats
  2019-02-18 14:22 [igt-dev] [PATCH i-g-t] tests/kms_available_modes_crc limit tested fb formats Juha-Pekka Heikkila
@ 2019-02-18 14:28 ` Ville Syrjälä
  2019-02-18 15:10 ` [igt-dev] ✓ Fi.CI.BAT: success for tests/kms_available_modes_crc limit tested fb formats (rev8) Patchwork
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 7+ messages in thread
From: Ville Syrjälä @ 2019-02-18 14:28 UTC (permalink / raw)
  To: Juha-Pekka Heikkila; +Cc: igt-dev

On Mon, Feb 18, 2019 at 04:22:07PM +0200, Juha-Pekka Heikkila wrote:
> This test is causing too much useless noise. Limit tested
> fb formats to DRM_FORMAT_C8 and DRM_FORMAT_XBGR2101010 for now.
> These two formats are currently not tested otherwise thus
> they're left here for now. DRM_FORMAT_XBGR2101010 need to be
> included into IGT supported formats and DRM_FORMAT_C8 test need
> to be moved elsewhere, maybe into kms_plane.
> 
> Signed-off-by: Juha-Pekka Heikkila <juhapekka.heikkila@gmail.com>

Acked-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

> ---
>  tests/kms_available_modes_crc.c | 217 +++++++++++++++-------------------------
>  1 file changed, 83 insertions(+), 134 deletions(-)
> 
> diff --git a/tests/kms_available_modes_crc.c b/tests/kms_available_modes_crc.c
> index 7ff385f..a51006b 100644
> --- a/tests/kms_available_modes_crc.c
> +++ b/tests/kms_available_modes_crc.c
> @@ -101,17 +101,17 @@ static void generate_comparison_crc_list(data_t *data, igt_output_t *output)
>  	igt_plane_set_fb(primary, &data->primary_fb);
>  	igt_display_commit2(&data->display, data->commit);
>  
> +	igt_pipe_crc_drain(data->pipe_crc);
>  	igt_pipe_crc_get_current(data->gfx_fd, data->pipe_crc, &data->cursor_crc);
>  	igt_plane_set_fb(primary, NULL);
>  	igt_display_commit2(&data->display, data->commit);
>  
> -	intel_gen(intel_get_drm_devid(data->gfx_fd)) < 9 ?
> -		  igt_paint_color(cr, 0, 0, mode->hdisplay, mode->vdisplay, 1.0, 1.0, 1.0) :
> -		  igt_paint_color_alpha(cr, 0, 0, mode->hdisplay, mode->vdisplay, 1.0, 1.0, 1.0, 1.0);
> +	igt_paint_color(cr, 0, 0, mode->hdisplay, mode->vdisplay, 1.0, 1.0, 1.0);
>  
>  	igt_plane_set_fb(primary, &data->primary_fb);
>  	igt_display_commit2(&data->display, data->commit);
>  
> +	igt_pipe_crc_drain(data->pipe_crc);
>  	igt_pipe_crc_get_current(data->gfx_fd, data->pipe_crc, &data->fullscreen_crc);
>  
>  	cairo_destroy(cr);
> @@ -122,48 +122,11 @@ static const struct {
>  	uint32_t	fourcc;
>  	char		zeropadding;
>  	enum		{ BYTES_PP_1=1,
> -				BYTES_PP_2=2,
> -				BYTES_PP_4=4,
> -				NV12,
> -				P010,
> -				SKIP4 } bpp;
> +				BYTES_PP_4=4} bpp;
>  	uint32_t	value;
>  } fillers[] = {
>  	{ DRM_FORMAT_C8, 0, BYTES_PP_1, 0xff},
> -	{ DRM_FORMAT_RGB565, 0, BYTES_PP_2, 0xffff},
> -	{ DRM_FORMAT_XRGB8888, 0, BYTES_PP_4, 0xffffffff},
> -	{ DRM_FORMAT_XBGR8888, 0, BYTES_PP_4, 0xffffffff},
> -
> -	/*
> -	 * following two are skipped because blending seems to work
> -	 * incorrectly with exception of AR24 on cursor plane.
> -	 * Test still creates the planes, just filling plane
> -	 * and getting crc is skipped.
> -	 */
> -	{ DRM_FORMAT_ARGB8888, 0, SKIP4, 0xffffffff},
> -	{ DRM_FORMAT_ABGR8888, 0, SKIP4, 0x00ffffff},
> -
> -	{ DRM_FORMAT_XRGB2101010, 0, BYTES_PP_4, 0xffffffff},
>  	{ DRM_FORMAT_XBGR2101010, 0, BYTES_PP_4, 0xffffffff},
> -
> -	{ DRM_FORMAT_YUYV, 0, BYTES_PP_4, 0x80eb80eb},
> -	{ DRM_FORMAT_YVYU, 0, BYTES_PP_4, 0x80eb80eb},
> -	{ DRM_FORMAT_VYUY, 0, BYTES_PP_4, 0xeb80eb80},
> -	{ DRM_FORMAT_UYVY, 0, BYTES_PP_4, 0xeb80eb80},
> -
> -	/*
> -	 * (semi-)planar formats
> -	 */
> -	{ DRM_FORMAT_NV12, 0, NV12, 0x80eb},
> -#ifdef DRM_FORMAT_P010
> -	{ DRM_FORMAT_P010, 0, P010, 0x8000eb00},
> -#endif
> -#ifdef DRM_FORMAT_P012
> -	{ DRM_FORMAT_P012, 0, P010, 0x8000eb00},
> -#endif
> -#ifdef DRM_FORMAT_P016
> -	{ DRM_FORMAT_P016, 0, P010, 0x8000eb00},
> -#endif
>  	{ 0, 0, 0, 0 }
>  };
>  
> @@ -175,10 +138,9 @@ static bool fill_in_fb(data_t *data, igt_output_t *output, igt_plane_t *plane,
>  		       uint32_t format)
>  {
>  	signed i, c, writesize;
> -	unsigned short* ptemp_16_buf;
>  	unsigned int* ptemp_32_buf;
>  
> -	for( i = 0; fillers[i].fourcc != 0; i++ ) {
> +	for( i = 0; i < ARRAY_SIZE(fillers)-1; i++ ) {
>  		if( fillers[i].fourcc == format )
>  			break;
>  	}
> @@ -190,53 +152,10 @@ static bool fill_in_fb(data_t *data, igt_output_t *output, igt_plane_t *plane,
>  			ptemp_32_buf[c] = fillers[i].value;
>  		writesize = data->size;
>  		break;
> -	case BYTES_PP_2:
> -		ptemp_16_buf = (unsigned short*)data->buf;
> -		for (c = 0; c < data->size/2; c++)
> -			ptemp_16_buf[c] = (unsigned short)fillers[i].value;
> -		writesize = data->size;
> -		break;
>  	case BYTES_PP_1:
>  		memset((void *)data->buf, fillers[i].value, data->size);
>  		writesize = data->size;
>  		break;
> -	case NV12:
> -		memset((void *)data->buf, fillers[i].value&0xff,
> -		       data->fb.offsets[1]);
> -
> -		memset((void *)(data->buf+data->fb.offsets[1]),
> -		       (fillers[i].value>>8)&0xff,
> -		       data->size - data->fb.offsets[1]);
> -
> -		writesize = data->size;
> -		break;
> -	case P010:
> -		ptemp_16_buf = (unsigned short*)data->buf;
> -		for (c = 0; c < data->size/2; c++)
> -			ptemp_16_buf[c] = (unsigned short)fillers[i].value&0xffff;
> -
> -		ptemp_16_buf = (unsigned short*)(data->buf+data->size);
> -		for (c = 0; c < data->size/2; c++)
> -			ptemp_16_buf[c] = (unsigned short)(fillers[i].value>>16)&0xffff;
> -
> -		writesize = data->size+data->size/2;
> -		break;
> -	case SKIP4:
> -		if (fillers[i].fourcc == DRM_FORMAT_ARGB8888 &&
> -		    plane->type == DRM_PLANE_TYPE_CURSOR) {
> -		/*
> -		 * special for cursor plane where blending works correctly.
> -		 */
> -			ptemp_32_buf = (unsigned int*)data->buf;
> -			for (c = 0; c < data->size/4; c++)
> -				ptemp_32_buf[c] = fillers[i].value;
> -			writesize = data->size;
> -			break;
> -		}
> -		igt_info("Format %s CRC comparison skipped by design.\n",
> -			 (char*)&fillers[i].fourcc);
> -
> -		return false;
>  	default:
>  		igt_info("Unsupported mode for test %s\n",
>  			 (char*)&fillers[i].fourcc);
> @@ -271,26 +190,20 @@ static bool setup_fb(data_t *data, igt_output_t *output, igt_plane_t *plane,
>  		tiling = LOCAL_DRM_FORMAT_MOD_NONE;
>  	}
>  
> -	for (i = 0; fillers[i].fourcc != 0; i++) {
> -		if (fillers[i].fourcc == format)
> +	for( i = 0; i < ARRAY_SIZE(fillers)-1; i++ ) {
> +		if( fillers[i].fourcc == format )
>  			break;
>  	}
>  
>  	switch (fillers[i].bpp) {
> -	case NV12:
>  	case BYTES_PP_1:
>  		bpp = 8;
>  		break;
> -
> -	case P010:
> -	case BYTES_PP_2:
> -		bpp = 16;
> -		break;
> -
> -	case SKIP4:
>  	case BYTES_PP_4:
>  		bpp = 32;
>  		break;
> +	default:
> +		return false;
>  	}
>  
>  	igt_get_fb_tile_size(data->gfx_fd, tiling, bpp,
> @@ -299,17 +212,6 @@ static bool setup_fb(data_t *data, igt_output_t *output, igt_plane_t *plane,
>  	data->fb.strides[0] = ALIGN(w * bpp / 8, tile_width);
>  	gemsize = data->size = data->fb.strides[0] * ALIGN(h, tile_height);
>  
> -	if (fillers[i].bpp == P010 || fillers[i].bpp == NV12) {
> -		data->fb.offsets[1] = data->size;
> -		data->fb.strides[1] = data->fb.strides[0];
> -		gemsize = data->size * 2;
> -
> -		if (fillers[i].bpp == NV12)
> -			data->size += data->fb.strides[1] * ALIGN(h/2, tile_height);
> -
> -		num_planes = 2;
> -	}
> -
>  	data->gem_handle = gem_create(data->gfx_fd, gemsize);
>  	ret = __gem_set_tiling(data->gfx_fd, data->gem_handle,
>  			       igt_fb_mod_to_tiling(tiling),
> @@ -386,12 +288,23 @@ static bool prepare_crtc(data_t *data, igt_output_t *output,
>  
>  static int
>  test_one_mode(data_t* data, igt_output_t *output, igt_plane_t* plane,
> -	      int mode)
> +	      int mode, enum pipe pipe)
>  {
>  	igt_crc_t current_crc;
>  	signed rVal = 0;
>  	bool do_crc;
> -	char* crccompare[2];
> +	int i;
> +
> +	/*
> +	 * Limit tests only to those fb formats listed in fillers table
> +	 */
> +	for( i = 0; i < ARRAY_SIZE(fillers)-1; i++ ) {
> +		if( fillers[i].fourcc == mode )
> +			break;
> +	}
> +
> +	if(fillers[i].bpp == 0)
> +		return false;
>  
>  	if (prepare_crtc(data, output, plane, mode)){
>  		/*
> @@ -412,29 +325,30 @@ test_one_mode(data_t* data, igt_output_t *output, igt_plane_t* plane,
>  			if (plane->type != DRM_PLANE_TYPE_CURSOR) {
>  				if (!igt_check_crc_equal(&current_crc,
>  					&data->fullscreen_crc)) {
> -					crccompare[0] = igt_crc_to_string(&current_crc);
> -					crccompare[1] = igt_crc_to_string(&data->fullscreen_crc);
> -					igt_warn("crc mismatch. target %.8s, result %.8s.\n", crccompare[0], crccompare[1]);
> -					free(crccompare[0]);
> -					free(crccompare[1]);
> +					igt_warn("crc mismatch. connector %s using pipe %s" \
> +						" plane index %d mode %.4s\n",
> +						igt_output_name(output),
> +						kmstest_pipe_name(pipe),
> +						plane->index,
> +						(char*)&mode);
>  					rVal++;
>  				}
>  			} else {
>  				if (!igt_check_crc_equal(&current_crc,
>  					&data->cursor_crc)) {
> -					crccompare[0] = igt_crc_to_string(&current_crc);
> -					crccompare[1] = igt_crc_to_string(&data->cursor_crc);
> -					igt_warn("crc mismatch. target %.8s, result %.8s.\n", crccompare[0], crccompare[1]);
> -					free(crccompare[0]);
> -					free(crccompare[1]);
> +					igt_warn("crc mismatch. connector %s using pipe %s" \
> +						" plane index %d mode %.4s\n",
> +						igt_output_name(output),
> +						kmstest_pipe_name(pipe),
> +						plane->index,
> +						(char*)&mode);
>  					rVal++;
>  				}
>  			}
>  		}
> -		remove_fb(data, output, plane);
> -		return rVal;
>  	}
> -	return 1;
> +	remove_fb(data, output, plane);
> +	return rVal;
>  }
>  
>  
> @@ -445,14 +359,44 @@ test_available_modes(data_t* data)
>  	igt_plane_t *plane;
>  	int modeindex;
>  	enum pipe pipe;
> -	int invalids = 0;
> +	int invalids = 0, i, lut_size;
>  	drmModePlane *modePlane;
> -	char planetype[3][8] = {"OVERLAY\0", "PRIMARY\0", "CURSOR\0" };
> +
> +	struct {
> +	    uint16_t red;
> +	    uint16_t green;
> +	    uint16_t blue;
> +	    uint16_t reserved;
> +	} *lut = NULL;
>  
>  	for_each_pipe_with_valid_output(&data->display, pipe, output) {
>  		igt_output_set_pipe(output, pipe);
>  		igt_display_commit2(&data->display, data->commit);
>  
> +		if (igt_pipe_obj_has_prop(&data->display.pipes[pipe], IGT_CRTC_GAMMA_LUT_SIZE)) {
> +			lut_size = igt_pipe_get_prop(&data->display, pipe,
> +						     IGT_CRTC_GAMMA_LUT_SIZE);
> +
> +			lut = calloc(sizeof(*lut), lut_size);
> +
> +			for (i = 0; i < lut_size; i++) {
> +				lut[i].red = (i * 0xffff / (lut_size - 1)) & 0x8000;
> +				lut[i].green = (i * 0xffff / (lut_size - 1)) & 0x8000;
> +				lut[i].blue = (i * 0xffff / (lut_size - 1)) & 0x8000;
> +			}
> +
> +			igt_pipe_replace_prop_blob(&data->display, pipe,
> +						   IGT_CRTC_GAMMA_LUT,
> +						   lut, sizeof(*lut) * lut_size);
> +			igt_display_commit2(&data->display, data->commit);
> +
> +			for (i = 0; i < lut_size; i++) {
> +				lut[i].red = i * 0xffff / (lut_size - 1);
> +				lut[i].green = i * 0xffff / (lut_size - 1);
> +				lut[i].blue = i * 0xffff / (lut_size - 1);
> +			}
> +		}
> +
>  		data->pipe_crc = igt_pipe_crc_new(data->gfx_fd, pipe,
>  						  INTEL_PIPE_CRC_SOURCE_AUTO);
>  
> @@ -467,29 +411,34 @@ test_available_modes(data_t* data)
>  			modePlane = drmModeGetPlane(data->gfx_fd,
>  						    plane->drm_plane->plane_id);
>  
> +			if (plane->type == DRM_PLANE_TYPE_CURSOR)
> +				continue;
> +
>  			for (modeindex = 0;
>  			     modeindex < modePlane->count_formats;
>  			     modeindex++) {
>  				data->format.dword = modePlane->formats[modeindex];
>  
> -				igt_info("Testing connector %s using pipe %s" \
> -					 " plane index %d type %s mode %s\n",
> -					 igt_output_name(output),
> -					 kmstest_pipe_name(pipe),
> -					 plane->index,
> -					 planetype[plane->type],
> -					 (char*)&data->format.name);
> -
>  				invalids += test_one_mode(data, output,
>  							  plane,
> -							  modePlane->formats[modeindex]);
> +							  modePlane->formats[modeindex],
> +							  pipe);
>  			}
>  			drmModeFreePlane(modePlane);
>  		}
>  		igt_pipe_crc_stop(data->pipe_crc);
>  		igt_pipe_crc_free(data->pipe_crc);
> -		igt_display_commit2(&data->display, data->commit);
> +
> +		if (lut != NULL) {
> +			igt_pipe_replace_prop_blob(&data->display, pipe,
> +						   IGT_CRTC_GAMMA_LUT,
> +						   lut, sizeof(*lut) * lut_size);
> +			free(lut);
> +			lut = NULL;
> +		}
> +
>  		igt_output_set_pipe(output, PIPE_NONE);
> +		igt_display_commit2(&data->display, data->commit);
>  	}
>  	igt_assert(invalids == 0);
>  }
> -- 
> 2.7.4
> 
> _______________________________________________
> 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

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

* [igt-dev] ✓ Fi.CI.BAT: success for tests/kms_available_modes_crc limit tested fb formats (rev8)
  2019-02-18 14:22 [igt-dev] [PATCH i-g-t] tests/kms_available_modes_crc limit tested fb formats Juha-Pekka Heikkila
  2019-02-18 14:28 ` Ville Syrjälä
@ 2019-02-18 15:10 ` Patchwork
  2019-02-18 18:15 ` [igt-dev] ✓ Fi.CI.IGT: " Patchwork
  2019-02-21  0:01 ` [igt-dev] [PATCH i-g-t] tests/kms_available_modes_crc limit tested fb formats Dhinakaran Pandiyan
  3 siblings, 0 replies; 7+ messages in thread
From: Patchwork @ 2019-02-18 15:10 UTC (permalink / raw)
  To: igt-dev

== Series Details ==

Series: tests/kms_available_modes_crc limit tested fb formats (rev8)
URL   : https://patchwork.freedesktop.org/series/56491/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_5626 -> IGTPW_2438
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  External URL: https://patchwork.freedesktop.org/api/1.0/series/56491/revisions/8/mbox/

Known issues
------------

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

### IGT changes ###

#### Issues hit ####

  * igt@pm_rpm@basic-rte:
    - fi-bsw-kefka:       PASS -> FAIL [fdo#108800]

  
#### Possible fixes ####

  * igt@kms_chamelium@hdmi-hpd-fast:
    - fi-kbl-7500u:       FAIL [fdo#109485] -> PASS

  * igt@kms_pipe_crc_basic@suspend-read-crc-pipe-b:
    - fi-blb-e6850:       INCOMPLETE [fdo#107718] -> PASS

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

  [fdo#107718]: https://bugs.freedesktop.org/show_bug.cgi?id=107718
  [fdo#108569]: https://bugs.freedesktop.org/show_bug.cgi?id=108569
  [fdo#108800]: https://bugs.freedesktop.org/show_bug.cgi?id=108800
  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [fdo#109278]: https://bugs.freedesktop.org/show_bug.cgi?id=109278
  [fdo#109485]: https://bugs.freedesktop.org/show_bug.cgi?id=109485


Participating hosts (47 -> 41)
------------------------------

  Additional (1): fi-bwr-2160 
  Missing    (7): fi-kbl-soraka fi-ilk-m540 fi-hsw-4200u fi-byt-squawks fi-bsw-cyan fi-icl-y fi-bdw-samus 


Build changes
-------------

    * IGT: IGT_4835 -> IGTPW_2438

  CI_DRM_5626: 40f27a9645a0cae536df6bc26e0d91766b47eb2f @ git://anongit.freedesktop.org/gfx-ci/linux
  IGTPW_2438: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_2438/
  IGT_4835: 784b3c535cb066dafb05d52170851b9dfb262f98 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools

== Logs ==

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

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

* [igt-dev] ✓ Fi.CI.IGT: success for tests/kms_available_modes_crc limit tested fb formats (rev8)
  2019-02-18 14:22 [igt-dev] [PATCH i-g-t] tests/kms_available_modes_crc limit tested fb formats Juha-Pekka Heikkila
  2019-02-18 14:28 ` Ville Syrjälä
  2019-02-18 15:10 ` [igt-dev] ✓ Fi.CI.BAT: success for tests/kms_available_modes_crc limit tested fb formats (rev8) Patchwork
@ 2019-02-18 18:15 ` Patchwork
  2019-02-21  0:01 ` [igt-dev] [PATCH i-g-t] tests/kms_available_modes_crc limit tested fb formats Dhinakaran Pandiyan
  3 siblings, 0 replies; 7+ messages in thread
From: Patchwork @ 2019-02-18 18:15 UTC (permalink / raw)
  To: igt-dev

== Series Details ==

Series: tests/kms_available_modes_crc limit tested fb formats (rev8)
URL   : https://patchwork.freedesktop.org/series/56491/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_5626_full -> IGTPW_2438_full
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  External URL: https://patchwork.freedesktop.org/api/1.0/series/56491/revisions/8/mbox/

Known issues
------------

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

### IGT changes ###

#### Issues hit ####

  * igt@gem_exec_big:
    - shard-hsw:          PASS -> TIMEOUT [fdo#107937]

  * igt@kms_atomic_transition@plane-toggle-modeset-transition:
    - shard-apl:          PASS -> INCOMPLETE [fdo#103927] +1

  * igt@kms_color@pipe-a-legacy-gamma:
    - shard-glk:          PASS -> FAIL [fdo#104782] / [fdo#108145]

  * igt@kms_cursor_crc@cursor-128x128-dpms:
    - shard-apl:          NOTRUN -> FAIL [fdo#103232]

  * igt@kms_cursor_crc@cursor-128x128-suspend:
    - shard-apl:          PASS -> FAIL [fdo#103191] / [fdo#103232]

  * igt@kms_cursor_crc@cursor-128x42-sliding:
    - shard-apl:          PASS -> FAIL [fdo#103232]

  * igt@kms_cursor_crc@cursor-64x64-sliding:
    - shard-kbl:          PASS -> FAIL [fdo#103232] +1

  * igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-draw-blt:
    - shard-kbl:          PASS -> FAIL [fdo#103167]

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

  * igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-draw-render:
    - shard-apl:          PASS -> FAIL [fdo#103167] +2

  * igt@kms_frontbuffer_tracking@fbc-1p-rte:
    - shard-glk:          PASS -> FAIL [fdo#103167] / [fdo#105682]

  * igt@kms_frontbuffer_tracking@fbc-2p-primscrn-spr-indfb-draw-mmap-cpu:
    - shard-glk:          PASS -> FAIL [fdo#103167] +7

  * igt@kms_plane@pixel-format-pipe-c-planes-source-clamping:
    - shard-glk:          PASS -> FAIL [fdo#108948]

  * igt@kms_plane_alpha_blend@pipe-c-alpha-7efc:
    - shard-kbl:          NOTRUN -> FAIL [fdo#108145] / [fdo#108590]

  * igt@kms_plane_multiple@atomic-pipe-b-tiling-x:
    - shard-glk:          PASS -> FAIL [fdo#103166]

  * igt@kms_universal_plane@universal-plane-pipe-a-functional:
    - shard-apl:          PASS -> FAIL [fdo#103166]

  * igt@kms_vblank@pipe-c-ts-continuation-dpms-suspend:
    - shard-apl:          PASS -> FAIL [fdo#104894] +2
    - shard-kbl:          PASS -> FAIL [fdo#104894] +2

  
#### Possible fixes ####

  * igt@gem_ctx_isolation@vcs0-s3:
    - shard-kbl:          INCOMPLETE [fdo#103665] -> PASS

  * igt@kms_available_modes_crc@available_mode_test_crc:
    - shard-kbl:          FAIL [fdo#106641] -> PASS

  * igt@kms_ccs@pipe-b-crc-sprite-planes-basic:
    - shard-apl:          FAIL [fdo#106510] / [fdo#108145] -> PASS
    - shard-kbl:          FAIL [fdo#107725] / [fdo#108145] -> PASS

  * igt@kms_cursor_crc@cursor-256x256-suspend:
    - shard-apl:          FAIL [fdo#103191] / [fdo#103232] -> PASS

  * igt@kms_cursor_crc@cursor-64x21-random:
    - shard-apl:          FAIL [fdo#103232] -> PASS +7

  * igt@kms_cursor_crc@cursor-alpha-opaque:
    - shard-kbl:          FAIL [fdo#109350] -> PASS

  * igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-draw-mmap-gtt:
    - shard-apl:          FAIL [fdo#103167] -> PASS
    - shard-kbl:          FAIL [fdo#103167] -> PASS

  * igt@kms_frontbuffer_tracking@fbc-2p-primscrn-spr-indfb-draw-pwrite:
    - shard-glk:          FAIL [fdo#103167] -> PASS

  * igt@kms_plane@pixel-format-pipe-b-planes-source-clamping:
    - shard-glk:          FAIL [fdo#108948] -> PASS

  * igt@kms_plane_alpha_blend@pipe-c-alpha-opaque-fb:
    - shard-kbl:          FAIL [fdo#108145] -> PASS
    - shard-apl:          FAIL [fdo#108145] -> PASS

  * igt@kms_plane_multiple@atomic-pipe-a-tiling-y:
    - shard-kbl:          FAIL [fdo#103166] -> PASS +1

  * igt@kms_plane_multiple@atomic-pipe-c-tiling-yf:
    - shard-apl:          FAIL [fdo#103166] -> PASS +8

  * igt@kms_rotation_crc@multiplane-rotation:
    - shard-kbl:          DMESG-FAIL [fdo#105763] -> PASS

  * igt@kms_rotation_crc@multiplane-rotation-cropping-top:
    - shard-kbl:          FAIL [fdo#109016] -> PASS

  * igt@kms_setmode@basic:
    - shard-apl:          FAIL [fdo#99912] -> PASS

  * igt@kms_universal_plane@universal-plane-pipe-c-functional:
    - shard-glk:          FAIL [fdo#103166] -> PASS +2

  * igt@kms_vblank@pipe-b-ts-continuation-modeset-hang:
    - shard-apl:          FAIL [fdo#104894] -> PASS +1

  * igt@pm_rc6_residency@rc6-accuracy:
    - shard-kbl:          {SKIP} [fdo#109271] -> PASS

  * igt@testdisplay:
    - shard-apl:          INCOMPLETE [fdo#103927] -> PASS

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

  [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#103665]: https://bugs.freedesktop.org/show_bug.cgi?id=103665
  [fdo#103927]: https://bugs.freedesktop.org/show_bug.cgi?id=103927
  [fdo#104782]: https://bugs.freedesktop.org/show_bug.cgi?id=104782
  [fdo#104894]: https://bugs.freedesktop.org/show_bug.cgi?id=104894
  [fdo#105411]: https://bugs.freedesktop.org/show_bug.cgi?id=105411
  [fdo#105682]: https://bugs.freedesktop.org/show_bug.cgi?id=105682
  [fdo#105763]: https://bugs.freedesktop.org/show_bug.cgi?id=105763
  [fdo#106510]: https://bugs.freedesktop.org/show_bug.cgi?id=106510
  [fdo#106641]: https://bugs.freedesktop.org/show_bug.cgi?id=106641
  [fdo#107725]: https://bugs.freedesktop.org/show_bug.cgi?id=107725
  [fdo#107937]: https://bugs.freedesktop.org/show_bug.cgi?id=107937
  [fdo#108145]: https://bugs.freedesktop.org/show_bug.cgi?id=108145
  [fdo#108590]: https://bugs.freedesktop.org/show_bug.cgi?id=108590
  [fdo#108948]: https://bugs.freedesktop.org/show_bug.cgi?id=108948
  [fdo#109016]: https://bugs.freedesktop.org/show_bug.cgi?id=109016
  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [fdo#109278]: https://bugs.freedesktop.org/show_bug.cgi?id=109278
  [fdo#109350]: https://bugs.freedesktop.org/show_bug.cgi?id=109350
  [fdo#99912]: https://bugs.freedesktop.org/show_bug.cgi?id=99912


Participating hosts (7 -> 5)
------------------------------

  Missing    (2): shard-skl shard-iclb 


Build changes
-------------

    * IGT: IGT_4835 -> IGTPW_2438
    * Piglit: piglit_4509 -> None

  CI_DRM_5626: 40f27a9645a0cae536df6bc26e0d91766b47eb2f @ git://anongit.freedesktop.org/gfx-ci/linux
  IGTPW_2438: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_2438/
  IGT_4835: 784b3c535cb066dafb05d52170851b9dfb262f98 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  piglit_4509: fdc5a4ca11124ab8413c7988896eec4c97336694 @ git://anongit.freedesktop.org/piglit

== Logs ==

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

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

* Re: [igt-dev] [PATCH i-g-t] tests/kms_available_modes_crc limit tested fb formats
  2019-02-18 14:22 [igt-dev] [PATCH i-g-t] tests/kms_available_modes_crc limit tested fb formats Juha-Pekka Heikkila
                   ` (2 preceding siblings ...)
  2019-02-18 18:15 ` [igt-dev] ✓ Fi.CI.IGT: " Patchwork
@ 2019-02-21  0:01 ` Dhinakaran Pandiyan
  2019-02-21 11:28   ` Juha-Pekka Heikkila
  3 siblings, 1 reply; 7+ messages in thread
From: Dhinakaran Pandiyan @ 2019-02-21  0:01 UTC (permalink / raw)
  To: Juha-Pekka Heikkila, igt-dev

On Mon, 2019-02-18 at 16:22 +0200, Juha-Pekka Heikkila wrote:
> This test is causing too much useless noise. Limit tested
> fb formats to DRM_FORMAT_C8 and DRM_FORMAT_XBGR2101010 for now.
> These two formats are currently not tested otherwise thus
> they're left here for now. DRM_FORMAT_XBGR2101010 need to be
> included into IGT supported formats and DRM_FORMAT_C8 test need
> to be moved elsewhere, maybe into kms_plane.

Thanks for doing this.

> 
> Signed-off-by: Juha-Pekka Heikkila <juhapekka.heikkila@gmail.com>
> ---
>  tests/kms_available_modes_crc.c | 217 +++++++++++++++---------------
> ----------
>  1 file changed, 83 insertions(+), 134 deletions(-)
> 
> diff --git a/tests/kms_available_modes_crc.c
> b/tests/kms_available_modes_crc.c
> index 7ff385f..a51006b 100644
> --- a/tests/kms_available_modes_crc.c
> +++ b/tests/kms_available_modes_crc.c
> @@ -101,17 +101,17 @@ static void generate_comparison_crc_list(data_t
> *data, igt_output_t *output)
>  	igt_plane_set_fb(primary, &data->primary_fb);
>  	igt_display_commit2(&data->display, data->commit);
>  
> +	igt_pipe_crc_drain(data->pipe_crc);
>  	igt_pipe_crc_get_current(data->gfx_fd, data->pipe_crc, &data-
> >cursor_crc);
>  	igt_plane_set_fb(primary, NULL);
>  	igt_display_commit2(&data->display, data->commit);
>  
> -	intel_gen(intel_get_drm_devid(data->gfx_fd)) < 9 ?
> -		  igt_paint_color(cr, 0, 0, mode->hdisplay, mode-
> >vdisplay, 1.0, 1.0, 1.0) :
> -		  igt_paint_color_alpha(cr, 0, 0, mode->hdisplay, mode-
> >vdisplay, 1.0, 1.0, 1.0, 1.0);
> +	igt_paint_color(cr, 0, 0, mode->hdisplay, mode->vdisplay, 1.0,
> 1.0, 1.0);
>  
>  	igt_plane_set_fb(primary, &data->primary_fb);
>  	igt_display_commit2(&data->display, data->commit);
>  
> +	igt_pipe_crc_drain(data->pipe_crc);
>  	igt_pipe_crc_get_current(data->gfx_fd, data->pipe_crc, &data-
> >fullscreen_crc);
>  
>  	cairo_destroy(cr);
> @@ -122,48 +122,11 @@ static const struct {
>  	uint32_t	fourcc;
>  	char		zeropadding;
>  	enum		{ BYTES_PP_1=1,
> -				BYTES_PP_2=2,
> -				BYTES_PP_4=4,
> -				NV12,
> -				P010,
> -				SKIP4 } bpp;
> +				BYTES_PP_4=4} bpp;
>  	uint32_t	value;
>  } fillers[] = {
>  	{ DRM_FORMAT_C8, 0, BYTES_PP_1, 0xff},
> -	{ DRM_FORMAT_RGB565, 0, BYTES_PP_2, 0xffff},
> -	{ DRM_FORMAT_XRGB8888, 0, BYTES_PP_4, 0xffffffff},
> -	{ DRM_FORMAT_XBGR8888, 0, BYTES_PP_4, 0xffffffff},
> -
> -	/*
> -	 * following two are skipped because blending seems to work
> -	 * incorrectly with exception of AR24 on cursor plane.
> -	 * Test still creates the planes, just filling plane
> -	 * and getting crc is skipped.
> -	 */
> -	{ DRM_FORMAT_ARGB8888, 0, SKIP4, 0xffffffff},
> -	{ DRM_FORMAT_ABGR8888, 0, SKIP4, 0x00ffffff},
> -
> -	{ DRM_FORMAT_XRGB2101010, 0, BYTES_PP_4, 0xffffffff},
>  	{ DRM_FORMAT_XBGR2101010, 0, BYTES_PP_4, 0xffffffff},
> -
> -	{ DRM_FORMAT_YUYV, 0, BYTES_PP_4, 0x80eb80eb},
> -	{ DRM_FORMAT_YVYU, 0, BYTES_PP_4, 0x80eb80eb},
> -	{ DRM_FORMAT_VYUY, 0, BYTES_PP_4, 0xeb80eb80},
> -	{ DRM_FORMAT_UYVY, 0, BYTES_PP_4, 0xeb80eb80},
> -
> -	/*
> -	 * (semi-)planar formats
> -	 */
> -	{ DRM_FORMAT_NV12, 0, NV12, 0x80eb},
> -#ifdef DRM_FORMAT_P010
> -	{ DRM_FORMAT_P010, 0, P010, 0x8000eb00},
> -#endif
> -#ifdef DRM_FORMAT_P012
> -	{ DRM_FORMAT_P012, 0, P010, 0x8000eb00},
> -#endif
> -#ifdef DRM_FORMAT_P016
> -	{ DRM_FORMAT_P016, 0, P010, 0x8000eb00},
> -#endif
>  	{ 0, 0, 0, 0 }
>  };
>  
> @@ -175,10 +138,9 @@ static bool fill_in_fb(data_t *data,
> igt_output_t *output, igt_plane_t *plane,
>  		       uint32_t format)
>  {
>  	signed i, c, writesize;
> -	unsigned short* ptemp_16_buf;
>  	unsigned int* ptemp_32_buf;
>  
> -	for( i = 0; fillers[i].fourcc != 0; i++ ) {
> +	for( i = 0; i < ARRAY_SIZE(fillers)-1; i++ ) {
>  		if( fillers[i].fourcc == format )
>  			break;
>  	}
> @@ -190,53 +152,10 @@ static bool fill_in_fb(data_t *data,
> igt_output_t *output, igt_plane_t *plane,
>  			ptemp_32_buf[c] = fillers[i].value;
>  		writesize = data->size;
>  		break;
> -	case BYTES_PP_2:
> -		ptemp_16_buf = (unsigned short*)data->buf;
> -		for (c = 0; c < data->size/2; c++)
> -			ptemp_16_buf[c] = (unsigned
> short)fillers[i].value;
> -		writesize = data->size;
> -		break;
>  	case BYTES_PP_1:
>  		memset((void *)data->buf, fillers[i].value, data-
> >size);
>  		writesize = data->size;
>  		break;
> -	case NV12:
> -		memset((void *)data->buf, fillers[i].value&0xff,
> -		       data->fb.offsets[1]);
> -
> -		memset((void *)(data->buf+data->fb.offsets[1]),
> -		       (fillers[i].value>>8)&0xff,
> -		       data->size - data->fb.offsets[1]);
> -
> -		writesize = data->size;
> -		break;
> -	case P010:
> -		ptemp_16_buf = (unsigned short*)data->buf;
> -		for (c = 0; c < data->size/2; c++)
> -			ptemp_16_buf[c] = (unsigned
> short)fillers[i].value&0xffff;
> -
> -		ptemp_16_buf = (unsigned short*)(data->buf+data->size);
> -		for (c = 0; c < data->size/2; c++)
> -			ptemp_16_buf[c] = (unsigned
> short)(fillers[i].value>>16)&0xffff;
> -
> -		writesize = data->size+data->size/2;
> -		break;
> -	case SKIP4:
> -		if (fillers[i].fourcc == DRM_FORMAT_ARGB8888 &&
> -		    plane->type == DRM_PLANE_TYPE_CURSOR) {
> -		/*
> -		 * special for cursor plane where blending works
> correctly.
> -		 */
> -			ptemp_32_buf = (unsigned int*)data->buf;
> -			for (c = 0; c < data->size/4; c++)
> -				ptemp_32_buf[c] = fillers[i].value;
> -			writesize = data->size;
> -			break;
> -		}
> -		igt_info("Format %s CRC comparison skipped by
> design.\n",
> -			 (char*)&fillers[i].fourcc);
> -
> -		return false;
>  	default:
>  		igt_info("Unsupported mode for test %s\n",
>  			 (char*)&fillers[i].fourcc);
> @@ -271,26 +190,20 @@ static bool setup_fb(data_t *data, igt_output_t
> *output, igt_plane_t *plane,
>  		tiling = LOCAL_DRM_FORMAT_MOD_NONE;
>  	}
>  
> -	for (i = 0; fillers[i].fourcc != 0; i++) {
> -		if (fillers[i].fourcc == format)
> +	for( i = 0; i < ARRAY_SIZE(fillers)-1; i++ ) {
> +		if( fillers[i].fourcc == format )
>  			break;
>  	}
>  
>  	switch (fillers[i].bpp) {
> -	case NV12:
>  	case BYTES_PP_1:
>  		bpp = 8;
>  		break;
> -
> -	case P010:
> -	case BYTES_PP_2:
> -		bpp = 16;
> -		break;
> -
> -	case SKIP4:
>  	case BYTES_PP_4:
>  		bpp = 32;
>  		break;
> +	default:
> +		return false;

Hmm, we should never hit this condition. Mark this as a failure so that
the test gets fixed?

>  	}
>  
>  	igt_get_fb_tile_size(data->gfx_fd, tiling, bpp,
> @@ -299,17 +212,6 @@ static bool setup_fb(data_t *data, igt_output_t
> *output, igt_plane_t *plane,
>  	data->fb.strides[0] = ALIGN(w * bpp / 8, tile_width);
>  	gemsize = data->size = data->fb.strides[0] * ALIGN(h,
> tile_height);
>  
> -	if (fillers[i].bpp == P010 || fillers[i].bpp == NV12) {
> -		data->fb.offsets[1] = data->size;
> -		data->fb.strides[1] = data->fb.strides[0];
> -		gemsize = data->size * 2;
> -
> -		if (fillers[i].bpp == NV12)
> -			data->size += data->fb.strides[1] * ALIGN(h/2,
> tile_height);
> -
> -		num_planes = 2;
> -	}
> -
>  	data->gem_handle = gem_create(data->gfx_fd, gemsize);
>  	ret = __gem_set_tiling(data->gfx_fd, data->gem_handle,
>  			       igt_fb_mod_to_tiling(tiling),
> @@ -386,12 +288,23 @@ static bool prepare_crtc(data_t *data,
> igt_output_t *output,
>  
>  static int
>  test_one_mode(data_t* data, igt_output_t *output, igt_plane_t*
> plane,
> -	      int mode)
> +	      int mode, enum pipe pipe)
s/mode/format
Comment applies to other places too.


>  {
>  	igt_crc_t current_crc;
>  	signed rVal = 0;
>  	bool do_crc;
> -	char* crccompare[2];
> +	int i;
> +
> +	/*
> +	 * Limit tests only to those fb formats listed in fillers table
> +	 */
> +	for( i = 0; i < ARRAY_SIZE(fillers)-1; i++ ) {
> +		if( fillers[i].fourcc == mode )
> +			break;
> +	}
> +
> +	if(fillers[i].bpp == 0)
> +		return false;
.bpp is always valid for the two formats that are tested, isn't it? In
that case .bpp == 0 is a test failure.

>  
>  	if (prepare_crtc(data, output, plane, mode)){
>  		/*
> @@ -412,29 +325,30 @@ test_one_mode(data_t* data, igt_output_t
> *output, igt_plane_t* plane,
>  			if (plane->type != DRM_PLANE_TYPE_CURSOR) {
>  				if (!igt_check_crc_equal(&current_crc,
>  					&data->fullscreen_crc)) {
> -					crccompare[0] =
> igt_crc_to_string(&current_crc);
> -					crccompare[1] =
> igt_crc_to_string(&data->fullscreen_crc);
> -					igt_warn("crc mismatch. target
> %.8s, result %.8s.\n", crccompare[0], crccompare[1]);
> -					free(crccompare[0]);
> -					free(crccompare[1]);
> +					igt_warn("crc mismatch.
> connector %s using pipe %s" \
> +						" plane index %d mode
> %.4s\n",
> +						igt_output_name(output)
> ,
> +						kmstest_pipe_name(pipe)
> ,
> +						plane->index,
> +						(char*)&mode);
>  					rVal++;
>  				}
>  			} else {
>  				if (!igt_check_crc_equal(&current_crc,
>  					&data->cursor_crc)) {
> -					crccompare[0] =
> igt_crc_to_string(&current_crc);
> -					crccompare[1] =
> igt_crc_to_string(&data->cursor_crc);
> -					igt_warn("crc mismatch. target
> %.8s, result %.8s.\n", crccompare[0], crccompare[1]);
> -					free(crccompare[0]);
> -					free(crccompare[1]);
> +					igt_warn("crc mismatch.
> connector %s using pipe %s" \
> +						" plane index %d mode
> %.4s\n",
> +						igt_output_name(output)
> ,
> +						kmstest_pipe_name(pipe)
> ,
> +						plane->index,
> +						(char*)&mode);
>  					rVal++;
>  				}
>  			}
>  		}
> -		remove_fb(data, output, plane);
> -		return rVal;
>  	}
> -	return 1;
> +	remove_fb(data, output, plane);
> +	return rVal;
>  }
>  
>  
> @@ -445,14 +359,44 @@ test_available_modes(data_t* data)
>  	igt_plane_t *plane;
>  	int modeindex;
>  	enum pipe pipe;
> -	int invalids = 0;
> +	int invalids = 0, i, lut_size;
>  	drmModePlane *modePlane;
> -	char planetype[3][8] = {"OVERLAY\0", "PRIMARY\0", "CURSOR\0" };
> +
> +	struct {
> +	    uint16_t red;
> +	    uint16_t green;
> +	    uint16_t blue;
> +	    uint16_t reserved;
> +	} *lut = NULL;
>  
>  	for_each_pipe_with_valid_output(&data->display, pipe, output) {
>  		igt_output_set_pipe(output, pipe);
>  		igt_display_commit2(&data->display, data->commit);
>  
> +		if (igt_pipe_obj_has_prop(&data->display.pipes[pipe],
> IGT_CRTC_GAMMA_LUT_SIZE)) {
> +			lut_size = igt_pipe_get_prop(&data->display,
> pipe,
> +						     IGT_CRTC_GAMMA_LUT
> _SIZE);
> +
> +			lut = calloc(sizeof(*lut), lut_size);
> +
> +			for (i = 0; i < lut_size; i++) {
> +				lut[i].red = (i * 0xffff / (lut_size -
> 1)) & 0x8000;
> +				lut[i].green = (i * 0xffff / (lut_size
> - 1)) & 0x8000;
> +				lut[i].blue = (i * 0xffff / (lut_size -
> 1)) & 0x8000;
> +			}
> +
> +			igt_pipe_replace_prop_blob(&data->display,
> pipe,
> +						   IGT_CRTC_GAMMA_LUT,
> +						   lut, sizeof(*lut) *
> lut_size);
> +			igt_display_commit2(&data->display, data-
> >commit);
> +
> +			for (i = 0; i < lut_size; i++) {
> +				lut[i].red = i * 0xffff / (lut_size -
> 1);
> +				lut[i].green = i * 0xffff / (lut_size -
> 1);
> +				lut[i].blue = i * 0xffff / (lut_size -
> 1);
> +			}
> +		}
> +
>  		data->pipe_crc = igt_pipe_crc_new(data->gfx_fd, pipe,
>  						  INTEL_PIPE_CRC_SOURCE
> _AUTO);
>  
> @@ -467,29 +411,34 @@ test_available_modes(data_t* data)
>  			modePlane = drmModeGetPlane(data->gfx_fd,
>  						    plane->drm_plane-
> >plane_id);
>  
> +			if (plane->type == DRM_PLANE_TYPE_CURSOR)
> +				continue;
> +
>  			for (modeindex = 0;
>  			     modeindex < modePlane->count_formats;
>  			     modeindex++) {
>  				data->format.dword = modePlane-
> >formats[modeindex];
>  
> -				igt_info("Testing connector %s using
> pipe %s" \
> -					 " plane index %d type %s mode
> %s\n",
> -					 igt_output_name(output),
> -					 kmstest_pipe_name(pipe),
> -					 plane->index,
> -					 planetype[plane->type],
> -					 (char*)&data->format.name);

Not sure why this was removed, perhaps convert it to igt_debug()? It's
hard to tell what is being tested now even with the --debug option.

If you want to address the comments in a separate patch, feel free to
add 
Reviewed-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>


> -
>  				invalids += test_one_mode(data, output,
>  							  plane,
> -							  modePlane-
> >formats[modeindex]);
> +							  modePlane-
> >formats[modeindex],
> +							  pipe);
>  			}
>  			drmModeFreePlane(modePlane);
>  		}
>  		igt_pipe_crc_stop(data->pipe_crc);
>  		igt_pipe_crc_free(data->pipe_crc);
> -		igt_display_commit2(&data->display, data->commit);
> +
> +		if (lut != NULL) {
> +			igt_pipe_replace_prop_blob(&data->display,
> pipe,
> +						   IGT_CRTC_GAMMA_LUT,
> +						   lut, sizeof(*lut) *
> lut_size);
> +			free(lut);
> +			lut = NULL;
> +		}
> +
>  		igt_output_set_pipe(output, PIPE_NONE);
> +		igt_display_commit2(&data->display, data->commit);
>  	}
>  	igt_assert(invalids == 0);
>  }

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

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

* Re: [igt-dev] [PATCH i-g-t] tests/kms_available_modes_crc limit tested fb formats
  2019-02-21  0:01 ` [igt-dev] [PATCH i-g-t] tests/kms_available_modes_crc limit tested fb formats Dhinakaran Pandiyan
@ 2019-02-21 11:28   ` Juha-Pekka Heikkila
  2019-02-21 22:04     ` Dhinakaran Pandiyan
  0 siblings, 1 reply; 7+ messages in thread
From: Juha-Pekka Heikkila @ 2019-02-21 11:28 UTC (permalink / raw)
  To: dhinakaran.pandiyan, igt-dev

On 21.2.2019 2.01, Dhinakaran Pandiyan wrote:
> On Mon, 2019-02-18 at 16:22 +0200, Juha-Pekka Heikkila wrote:
>> This test is causing too much useless noise. Limit tested
>> fb formats to DRM_FORMAT_C8 and DRM_FORMAT_XBGR2101010 for now.
>> These two formats are currently not tested otherwise thus
>> they're left here for now. DRM_FORMAT_XBGR2101010 need to be
>> included into IGT supported formats and DRM_FORMAT_C8 test need
>> to be moved elsewhere, maybe into kms_plane.
> 
> Thanks for doing this.
> 
>>
>> Signed-off-by: Juha-Pekka Heikkila <juhapekka.heikkila@gmail.com>
>> ---
>>   tests/kms_available_modes_crc.c | 217 +++++++++++++++---------------
>> ----------
>>   1 file changed, 83 insertions(+), 134 deletions(-)
>>
>> diff --git a/tests/kms_available_modes_crc.c
>> b/tests/kms_available_modes_crc.c
>> index 7ff385f..a51006b 100644
>> --- a/tests/kms_available_modes_crc.c
>> +++ b/tests/kms_available_modes_crc.c
>> @@ -101,17 +101,17 @@ static void generate_comparison_crc_list(data_t
>> *data, igt_output_t *output)
>>   	igt_plane_set_fb(primary, &data->primary_fb);
>>   	igt_display_commit2(&data->display, data->commit);
>>   
>> +	igt_pipe_crc_drain(data->pipe_crc);
>>   	igt_pipe_crc_get_current(data->gfx_fd, data->pipe_crc, &data-
>>> cursor_crc);
>>   	igt_plane_set_fb(primary, NULL);
>>   	igt_display_commit2(&data->display, data->commit);
>>   
>> -	intel_gen(intel_get_drm_devid(data->gfx_fd)) < 9 ?
>> -		  igt_paint_color(cr, 0, 0, mode->hdisplay, mode-
>>> vdisplay, 1.0, 1.0, 1.0) :
>> -		  igt_paint_color_alpha(cr, 0, 0, mode->hdisplay, mode-
>>> vdisplay, 1.0, 1.0, 1.0, 1.0);
>> +	igt_paint_color(cr, 0, 0, mode->hdisplay, mode->vdisplay, 1.0,
>> 1.0, 1.0);
>>   
>>   	igt_plane_set_fb(primary, &data->primary_fb);
>>   	igt_display_commit2(&data->display, data->commit);
>>   
>> +	igt_pipe_crc_drain(data->pipe_crc);
>>   	igt_pipe_crc_get_current(data->gfx_fd, data->pipe_crc, &data-
>>> fullscreen_crc);
>>   
>>   	cairo_destroy(cr);
>> @@ -122,48 +122,11 @@ static const struct {
>>   	uint32_t	fourcc;
>>   	char		zeropadding;
>>   	enum		{ BYTES_PP_1=1,
>> -				BYTES_PP_2=2,
>> -				BYTES_PP_4=4,
>> -				NV12,
>> -				P010,
>> -				SKIP4 } bpp;
>> +				BYTES_PP_4=4} bpp;
>>   	uint32_t	value;
>>   } fillers[] = {
>>   	{ DRM_FORMAT_C8, 0, BYTES_PP_1, 0xff},
>> -	{ DRM_FORMAT_RGB565, 0, BYTES_PP_2, 0xffff},
>> -	{ DRM_FORMAT_XRGB8888, 0, BYTES_PP_4, 0xffffffff},
>> -	{ DRM_FORMAT_XBGR8888, 0, BYTES_PP_4, 0xffffffff},
>> -
>> -	/*
>> -	 * following two are skipped because blending seems to work
>> -	 * incorrectly with exception of AR24 on cursor plane.
>> -	 * Test still creates the planes, just filling plane
>> -	 * and getting crc is skipped.
>> -	 */
>> -	{ DRM_FORMAT_ARGB8888, 0, SKIP4, 0xffffffff},
>> -	{ DRM_FORMAT_ABGR8888, 0, SKIP4, 0x00ffffff},
>> -
>> -	{ DRM_FORMAT_XRGB2101010, 0, BYTES_PP_4, 0xffffffff},
>>   	{ DRM_FORMAT_XBGR2101010, 0, BYTES_PP_4, 0xffffffff},
>> -
>> -	{ DRM_FORMAT_YUYV, 0, BYTES_PP_4, 0x80eb80eb},
>> -	{ DRM_FORMAT_YVYU, 0, BYTES_PP_4, 0x80eb80eb},
>> -	{ DRM_FORMAT_VYUY, 0, BYTES_PP_4, 0xeb80eb80},
>> -	{ DRM_FORMAT_UYVY, 0, BYTES_PP_4, 0xeb80eb80},
>> -
>> -	/*
>> -	 * (semi-)planar formats
>> -	 */
>> -	{ DRM_FORMAT_NV12, 0, NV12, 0x80eb},
>> -#ifdef DRM_FORMAT_P010
>> -	{ DRM_FORMAT_P010, 0, P010, 0x8000eb00},
>> -#endif
>> -#ifdef DRM_FORMAT_P012
>> -	{ DRM_FORMAT_P012, 0, P010, 0x8000eb00},
>> -#endif
>> -#ifdef DRM_FORMAT_P016
>> -	{ DRM_FORMAT_P016, 0, P010, 0x8000eb00},
>> -#endif
>>   	{ 0, 0, 0, 0 }
>>   };
>>   
>> @@ -175,10 +138,9 @@ static bool fill_in_fb(data_t *data,
>> igt_output_t *output, igt_plane_t *plane,
>>   		       uint32_t format)
>>   {
>>   	signed i, c, writesize;
>> -	unsigned short* ptemp_16_buf;
>>   	unsigned int* ptemp_32_buf;
>>   
>> -	for( i = 0; fillers[i].fourcc != 0; i++ ) {
>> +	for( i = 0; i < ARRAY_SIZE(fillers)-1; i++ ) {
>>   		if( fillers[i].fourcc == format )
>>   			break;
>>   	}
>> @@ -190,53 +152,10 @@ static bool fill_in_fb(data_t *data,
>> igt_output_t *output, igt_plane_t *plane,
>>   			ptemp_32_buf[c] = fillers[i].value;
>>   		writesize = data->size;
>>   		break;
>> -	case BYTES_PP_2:
>> -		ptemp_16_buf = (unsigned short*)data->buf;
>> -		for (c = 0; c < data->size/2; c++)
>> -			ptemp_16_buf[c] = (unsigned
>> short)fillers[i].value;
>> -		writesize = data->size;
>> -		break;
>>   	case BYTES_PP_1:
>>   		memset((void *)data->buf, fillers[i].value, data-
>>> size);
>>   		writesize = data->size;
>>   		break;
>> -	case NV12:
>> -		memset((void *)data->buf, fillers[i].value&0xff,
>> -		       data->fb.offsets[1]);
>> -
>> -		memset((void *)(data->buf+data->fb.offsets[1]),
>> -		       (fillers[i].value>>8)&0xff,
>> -		       data->size - data->fb.offsets[1]);
>> -
>> -		writesize = data->size;
>> -		break;
>> -	case P010:
>> -		ptemp_16_buf = (unsigned short*)data->buf;
>> -		for (c = 0; c < data->size/2; c++)
>> -			ptemp_16_buf[c] = (unsigned
>> short)fillers[i].value&0xffff;
>> -
>> -		ptemp_16_buf = (unsigned short*)(data->buf+data->size);
>> -		for (c = 0; c < data->size/2; c++)
>> -			ptemp_16_buf[c] = (unsigned
>> short)(fillers[i].value>>16)&0xffff;
>> -
>> -		writesize = data->size+data->size/2;
>> -		break;
>> -	case SKIP4:
>> -		if (fillers[i].fourcc == DRM_FORMAT_ARGB8888 &&
>> -		    plane->type == DRM_PLANE_TYPE_CURSOR) {
>> -		/*
>> -		 * special for cursor plane where blending works
>> correctly.
>> -		 */
>> -			ptemp_32_buf = (unsigned int*)data->buf;
>> -			for (c = 0; c < data->size/4; c++)
>> -				ptemp_32_buf[c] = fillers[i].value;
>> -			writesize = data->size;
>> -			break;
>> -		}
>> -		igt_info("Format %s CRC comparison skipped by
>> design.\n",
>> -			 (char*)&fillers[i].fourcc);
>> -
>> -		return false;
>>   	default:
>>   		igt_info("Unsupported mode for test %s\n",
>>   			 (char*)&fillers[i].fourcc);
>> @@ -271,26 +190,20 @@ static bool setup_fb(data_t *data, igt_output_t
>> *output, igt_plane_t *plane,
>>   		tiling = LOCAL_DRM_FORMAT_MOD_NONE;
>>   	}
>>   
>> -	for (i = 0; fillers[i].fourcc != 0; i++) {
>> -		if (fillers[i].fourcc == format)
>> +	for( i = 0; i < ARRAY_SIZE(fillers)-1; i++ ) {
>> +		if( fillers[i].fourcc == format )
>>   			break;
>>   	}
>>   
>>   	switch (fillers[i].bpp) {
>> -	case NV12:
>>   	case BYTES_PP_1:
>>   		bpp = 8;
>>   		break;
>> -
>> -	case P010:
>> -	case BYTES_PP_2:
>> -		bpp = 16;
>> -		break;
>> -
>> -	case SKIP4:
>>   	case BYTES_PP_4:
>>   		bpp = 32;
>>   		break;
>> +	default:
>> +		return false;
> 
> Hmm, we should never hit this condition. Mark this as a failure so that
> the test gets fixed?

Yes, this is good place for assert.

> 
>>   	}
>>   
>>   	igt_get_fb_tile_size(data->gfx_fd, tiling, bpp,
>> @@ -299,17 +212,6 @@ static bool setup_fb(data_t *data, igt_output_t
>> *output, igt_plane_t *plane,
>>   	data->fb.strides[0] = ALIGN(w * bpp / 8, tile_width);
>>   	gemsize = data->size = data->fb.strides[0] * ALIGN(h,
>> tile_height);
>>   
>> -	if (fillers[i].bpp == P010 || fillers[i].bpp == NV12) {
>> -		data->fb.offsets[1] = data->size;
>> -		data->fb.strides[1] = data->fb.strides[0];
>> -		gemsize = data->size * 2;
>> -
>> -		if (fillers[i].bpp == NV12)
>> -			data->size += data->fb.strides[1] * ALIGN(h/2,
>> tile_height);
>> -
>> -		num_planes = 2;
>> -	}
>> -
>>   	data->gem_handle = gem_create(data->gfx_fd, gemsize);
>>   	ret = __gem_set_tiling(data->gfx_fd, data->gem_handle,
>>   			       igt_fb_mod_to_tiling(tiling),
>> @@ -386,12 +288,23 @@ static bool prepare_crtc(data_t *data,
>> igt_output_t *output,
>>   
>>   static int
>>   test_one_mode(data_t* data, igt_output_t *output, igt_plane_t*
>> plane,
>> -	      int mode)
>> +	      int mode, enum pipe pipe)
> s/mode/format
> Comment applies to other places too.

While I agree I'm so-so about doing this change since it will follow 
into massive patch for test which I'm anyway wanting to throw away in 
the end.

> 
> 
>>   {
>>   	igt_crc_t current_crc;
>>   	signed rVal = 0;
>>   	bool do_crc;
>> -	char* crccompare[2];
>> +	int i;
>> +
>> +	/*
>> +	 * Limit tests only to those fb formats listed in fillers table
>> +	 */
>> +	for( i = 0; i < ARRAY_SIZE(fillers)-1; i++ ) {
>> +		if( fillers[i].fourcc == mode )
>> +			break;
>> +	}
>> +
>> +	if(fillers[i].bpp == 0)
>> +		return false;
> .bpp is always valid for the two formats that are tested, isn't it? In
> that case .bpp == 0 is a test failure.
> 

No, here will arrive query for all fb formats which kernel talk about. 
If bpp become zero this will go out from testing mode which is not 
supported. Ie. C8 or XBGR2101010 will go forward and here other formats 
are ditched.

>>   
>>   	if (prepare_crtc(data, output, plane, mode)){
>>   		/*
>> @@ -412,29 +325,30 @@ test_one_mode(data_t* data, igt_output_t
>> *output, igt_plane_t* plane,
>>   			if (plane->type != DRM_PLANE_TYPE_CURSOR) {
>>   				if (!igt_check_crc_equal(&current_crc,
>>   					&data->fullscreen_crc)) {
>> -					crccompare[0] =
>> igt_crc_to_string(&current_crc);
>> -					crccompare[1] =
>> igt_crc_to_string(&data->fullscreen_crc);
>> -					igt_warn("crc mismatch. target
>> %.8s, result %.8s.\n", crccompare[0], crccompare[1]);
>> -					free(crccompare[0]);
>> -					free(crccompare[1]);
>> +					igt_warn("crc mismatch.
>> connector %s using pipe %s" \
>> +						" plane index %d mode
>> %.4s\n",
>> +						igt_output_name(output)
>> ,
>> +						kmstest_pipe_name(pipe)
>> ,
>> +						plane->index,
>> +						(char*)&mode);
>>   					rVal++;
>>   				}
>>   			} else {
>>   				if (!igt_check_crc_equal(&current_crc,
>>   					&data->cursor_crc)) {
>> -					crccompare[0] =
>> igt_crc_to_string(&current_crc);
>> -					crccompare[1] =
>> igt_crc_to_string(&data->cursor_crc);
>> -					igt_warn("crc mismatch. target
>> %.8s, result %.8s.\n", crccompare[0], crccompare[1]);
>> -					free(crccompare[0]);
>> -					free(crccompare[1]);
>> +					igt_warn("crc mismatch.
>> connector %s using pipe %s" \
>> +						" plane index %d mode
>> %.4s\n",
>> +						igt_output_name(output)
>> ,
>> +						kmstest_pipe_name(pipe)
>> ,
>> +						plane->index,
>> +						(char*)&mode);
>>   					rVal++;
>>   				}
>>   			}
>>   		}
>> -		remove_fb(data, output, plane);
>> -		return rVal;
>>   	}
>> -	return 1;
>> +	remove_fb(data, output, plane);
>> +	return rVal;
>>   }
>>   
>>   
>> @@ -445,14 +359,44 @@ test_available_modes(data_t* data)
>>   	igt_plane_t *plane;
>>   	int modeindex;
>>   	enum pipe pipe;
>> -	int invalids = 0;
>> +	int invalids = 0, i, lut_size;
>>   	drmModePlane *modePlane;
>> -	char planetype[3][8] = {"OVERLAY\0", "PRIMARY\0", "CURSOR\0" };
>> +
>> +	struct {
>> +	    uint16_t red;
>> +	    uint16_t green;
>> +	    uint16_t blue;
>> +	    uint16_t reserved;
>> +	} *lut = NULL;
>>   
>>   	for_each_pipe_with_valid_output(&data->display, pipe, output) {
>>   		igt_output_set_pipe(output, pipe);
>>   		igt_display_commit2(&data->display, data->commit);
>>   
>> +		if (igt_pipe_obj_has_prop(&data->display.pipes[pipe],
>> IGT_CRTC_GAMMA_LUT_SIZE)) {
>> +			lut_size = igt_pipe_get_prop(&data->display,
>> pipe,
>> +						     IGT_CRTC_GAMMA_LUT
>> _SIZE);
>> +
>> +			lut = calloc(sizeof(*lut), lut_size);
>> +
>> +			for (i = 0; i < lut_size; i++) {
>> +				lut[i].red = (i * 0xffff / (lut_size -
>> 1)) & 0x8000;
>> +				lut[i].green = (i * 0xffff / (lut_size
>> - 1)) & 0x8000;
>> +				lut[i].blue = (i * 0xffff / (lut_size -
>> 1)) & 0x8000;
>> +			}
>> +
>> +			igt_pipe_replace_prop_blob(&data->display,
>> pipe,
>> +						   IGT_CRTC_GAMMA_LUT,
>> +						   lut, sizeof(*lut) *
>> lut_size);
>> +			igt_display_commit2(&data->display, data-
>>> commit);
>> +
>> +			for (i = 0; i < lut_size; i++) {
>> +				lut[i].red = i * 0xffff / (lut_size -
>> 1);
>> +				lut[i].green = i * 0xffff / (lut_size -
>> 1);
>> +				lut[i].blue = i * 0xffff / (lut_size -
>> 1);
>> +			}
>> +		}
>> +
>>   		data->pipe_crc = igt_pipe_crc_new(data->gfx_fd, pipe,
>>   						  INTEL_PIPE_CRC_SOURCE
>> _AUTO);
>>   
>> @@ -467,29 +411,34 @@ test_available_modes(data_t* data)
>>   			modePlane = drmModeGetPlane(data->gfx_fd,
>>   						    plane->drm_plane-
>>> plane_id);
>>   
>> +			if (plane->type == DRM_PLANE_TYPE_CURSOR)
>> +				continue;
>> +
>>   			for (modeindex = 0;
>>   			     modeindex < modePlane->count_formats;
>>   			     modeindex++) {
>>   				data->format.dword = modePlane-
>>> formats[modeindex];
>>   
>> -				igt_info("Testing connector %s using
>> pipe %s" \
>> -					 " plane index %d type %s mode
>> %s\n",
>> -					 igt_output_name(output),
>> -					 kmstest_pipe_name(pipe),
>> -					 plane->index,
>> -					 planetype[plane->type],
>> -					 (char*)&data->format.name);
> 
> Not sure why this was removed, perhaps convert it to igt_debug()? It's
> hard to tell what is being tested now even with the --debug option.

I did consider this as too verbose. Above where crcs are compared 
there's printed what failed with more detail than here.

> 
> If you want to address the comments in a separate patch, feel free to
> add
> Reviewed-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>

I'm going to have few CI runs with evolving this patch still.

I found timing error on this test, it is most likely what is causing the 
noise from this test in CI runs. I somehow never see it on any of my own 
computers but realized it from the logic and first attempt to mitigate 
it changed CI results for the better so I'll get that fixed.

/Juha-Pekka

> 
> 
>> -
>>   				invalids += test_one_mode(data, output,
>>   							  plane,
>> -							  modePlane-
>>> formats[modeindex]);
>> +							  modePlane-
>>> formats[modeindex],
>> +							  pipe);
>>   			}
>>   			drmModeFreePlane(modePlane);
>>   		}
>>   		igt_pipe_crc_stop(data->pipe_crc);
>>   		igt_pipe_crc_free(data->pipe_crc);
>> -		igt_display_commit2(&data->display, data->commit);
>> +
>> +		if (lut != NULL) {
>> +			igt_pipe_replace_prop_blob(&data->display,
>> pipe,
>> +						   IGT_CRTC_GAMMA_LUT,
>> +						   lut, sizeof(*lut) *
>> lut_size);
>> +			free(lut);
>> +			lut = NULL;
>> +		}
>> +
>>   		igt_output_set_pipe(output, PIPE_NONE);
>> +		igt_display_commit2(&data->display, data->commit);
>>   	}
>>   	igt_assert(invalids == 0);
>>   }
> 

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

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

* Re: [igt-dev] [PATCH i-g-t] tests/kms_available_modes_crc limit tested fb formats
  2019-02-21 11:28   ` Juha-Pekka Heikkila
@ 2019-02-21 22:04     ` Dhinakaran Pandiyan
  0 siblings, 0 replies; 7+ messages in thread
From: Dhinakaran Pandiyan @ 2019-02-21 22:04 UTC (permalink / raw)
  To: juhapekka.heikkila, igt-dev

On Thu, 2019-02-21 at 13:28 +0200, Juha-Pekka Heikkila wrote:
> On 21.2.2019 2.01, Dhinakaran Pandiyan wrote:
> > On Mon, 2019-02-18 at 16:22 +0200, Juha-Pekka Heikkila wrote:
> > > This test is causing too much useless noise. Limit tested
> > > fb formats to DRM_FORMAT_C8 and DRM_FORMAT_XBGR2101010 for now.
> > > These two formats are currently not tested otherwise thus
> > > they're left here for now. DRM_FORMAT_XBGR2101010 need to be
> > > included into IGT supported formats and DRM_FORMAT_C8 test need
> > > to be moved elsewhere, maybe into kms_plane.
> > 
> > Thanks for doing this.
> > 
> > > 
> > > Signed-off-by: Juha-Pekka Heikkila <juhapekka.heikkila@gmail.com>
> > > ---
> > >   tests/kms_available_modes_crc.c | 217 +++++++++++++++--------
> > > -------
> > > ----------
> > >   1 file changed, 83 insertions(+), 134 deletions(-)
> > > 
> > > diff --git a/tests/kms_available_modes_crc.c
> > > b/tests/kms_available_modes_crc.c
> > > index 7ff385f..a51006b 100644
> > > --- a/tests/kms_available_modes_crc.c
> > > +++ b/tests/kms_available_modes_crc.c
> > > @@ -101,17 +101,17 @@ static void
> > > generate_comparison_crc_list(data_t
> > > *data, igt_output_t *output)
> > >   	igt_plane_set_fb(primary, &data->primary_fb);
> > >   	igt_display_commit2(&data->display, data->commit);
> > >   
> > > +	igt_pipe_crc_drain(data->pipe_crc);
> > >   	igt_pipe_crc_get_current(data->gfx_fd, data->pipe_crc,
> > > &data-
> > > > cursor_crc);
> > > 
> > >   	igt_plane_set_fb(primary, NULL);
> > >   	igt_display_commit2(&data->display, data->commit);
> > >   
> > > -	intel_gen(intel_get_drm_devid(data->gfx_fd)) < 9 ?
> > > -		  igt_paint_color(cr, 0, 0, mode->hdisplay, mode-
> > > > vdisplay, 1.0, 1.0, 1.0) :
> > > 
> > > -		  igt_paint_color_alpha(cr, 0, 0, mode->hdisplay, mode-
> > > > vdisplay, 1.0, 1.0, 1.0, 1.0);
> > > 
> > > +	igt_paint_color(cr, 0, 0, mode->hdisplay, mode->vdisplay, 1.0,
> > > 1.0, 1.0);
> > >   
> > >   	igt_plane_set_fb(primary, &data->primary_fb);
> > >   	igt_display_commit2(&data->display, data->commit);
> > >   
> > > +	igt_pipe_crc_drain(data->pipe_crc);
> > >   	igt_pipe_crc_get_current(data->gfx_fd, data->pipe_crc,
> > > &data-
> > > > fullscreen_crc);
> > > 
> > >   
> > >   	cairo_destroy(cr);
> > > @@ -122,48 +122,11 @@ static const struct {
> > >   	uint32_t	fourcc;
> > >   	char		zeropadding;
> > >   	enum		{ BYTES_PP_1=1,
> > > -				BYTES_PP_2=2,
> > > -				BYTES_PP_4=4,
> > > -				NV12,
> > > -				P010,
> > > -				SKIP4 } bpp;
> > > +				BYTES_PP_4=4} bpp;
> > >   	uint32_t	value;
> > >   } fillers[] = {
> > >   	{ DRM_FORMAT_C8, 0, BYTES_PP_1, 0xff},
> > > -	{ DRM_FORMAT_RGB565, 0, BYTES_PP_2, 0xffff},
> > > -	{ DRM_FORMAT_XRGB8888, 0, BYTES_PP_4, 0xffffffff},
> > > -	{ DRM_FORMAT_XBGR8888, 0, BYTES_PP_4, 0xffffffff},
> > > -
> > > -	/*
> > > -	 * following two are skipped because blending seems to work
> > > -	 * incorrectly with exception of AR24 on cursor plane.
> > > -	 * Test still creates the planes, just filling plane
> > > -	 * and getting crc is skipped.
> > > -	 */
> > > -	{ DRM_FORMAT_ARGB8888, 0, SKIP4, 0xffffffff},
> > > -	{ DRM_FORMAT_ABGR8888, 0, SKIP4, 0x00ffffff},
> > > -
> > > -	{ DRM_FORMAT_XRGB2101010, 0, BYTES_PP_4, 0xffffffff},
> > >   	{ DRM_FORMAT_XBGR2101010, 0, BYTES_PP_4, 0xffffffff},
> > > -
> > > -	{ DRM_FORMAT_YUYV, 0, BYTES_PP_4, 0x80eb80eb},
> > > -	{ DRM_FORMAT_YVYU, 0, BYTES_PP_4, 0x80eb80eb},
> > > -	{ DRM_FORMAT_VYUY, 0, BYTES_PP_4, 0xeb80eb80},
> > > -	{ DRM_FORMAT_UYVY, 0, BYTES_PP_4, 0xeb80eb80},
> > > -
> > > -	/*
> > > -	 * (semi-)planar formats
> > > -	 */
> > > -	{ DRM_FORMAT_NV12, 0, NV12, 0x80eb},
> > > -#ifdef DRM_FORMAT_P010
> > > -	{ DRM_FORMAT_P010, 0, P010, 0x8000eb00},
> > > -#endif
> > > -#ifdef DRM_FORMAT_P012
> > > -	{ DRM_FORMAT_P012, 0, P010, 0x8000eb00},
> > > -#endif
> > > -#ifdef DRM_FORMAT_P016
> > > -	{ DRM_FORMAT_P016, 0, P010, 0x8000eb00},
> > > -#endif
> > >   	{ 0, 0, 0, 0 }
> > >   };
> > >   
> > > @@ -175,10 +138,9 @@ static bool fill_in_fb(data_t *data,
> > > igt_output_t *output, igt_plane_t *plane,
> > >   		       uint32_t format)
> > >   {
> > >   	signed i, c, writesize;
> > > -	unsigned short* ptemp_16_buf;
> > >   	unsigned int* ptemp_32_buf;
> > >   
> > > -	for( i = 0; fillers[i].fourcc != 0; i++ ) {
> > > +	for( i = 0; i < ARRAY_SIZE(fillers)-1; i++ ) {
> > >   		if( fillers[i].fourcc == format )
> > >   			break;
> > >   	}
> > > @@ -190,53 +152,10 @@ static bool fill_in_fb(data_t *data,
> > > igt_output_t *output, igt_plane_t *plane,
> > >   			ptemp_32_buf[c] = fillers[i].value;
> > >   		writesize = data->size;
> > >   		break;
> > > -	case BYTES_PP_2:
> > > -		ptemp_16_buf = (unsigned short*)data->buf;
> > > -		for (c = 0; c < data->size/2; c++)
> > > -			ptemp_16_buf[c] = (unsigned
> > > short)fillers[i].value;
> > > -		writesize = data->size;
> > > -		break;
> > >   	case BYTES_PP_1:
> > >   		memset((void *)data->buf, fillers[i].value,
> > > data-
> > > > size);
> > > 
> > >   		writesize = data->size;
> > >   		break;
> > > -	case NV12:
> > > -		memset((void *)data->buf, fillers[i].value&0xff,
> > > -		       data->fb.offsets[1]);
> > > -
> > > -		memset((void *)(data->buf+data->fb.offsets[1]),
> > > -		       (fillers[i].value>>8)&0xff,
> > > -		       data->size - data->fb.offsets[1]);
> > > -
> > > -		writesize = data->size;
> > > -		break;
> > > -	case P010:
> > > -		ptemp_16_buf = (unsigned short*)data->buf;
> > > -		for (c = 0; c < data->size/2; c++)
> > > -			ptemp_16_buf[c] = (unsigned
> > > short)fillers[i].value&0xffff;
> > > -
> > > -		ptemp_16_buf = (unsigned short*)(data->buf+data->size);
> > > -		for (c = 0; c < data->size/2; c++)
> > > -			ptemp_16_buf[c] = (unsigned
> > > short)(fillers[i].value>>16)&0xffff;
> > > -
> > > -		writesize = data->size+data->size/2;
> > > -		break;
> > > -	case SKIP4:
> > > -		if (fillers[i].fourcc == DRM_FORMAT_ARGB8888 &&
> > > -		    plane->type == DRM_PLANE_TYPE_CURSOR) {
> > > -		/*
> > > -		 * special for cursor plane where blending works
> > > correctly.
> > > -		 */
> > > -			ptemp_32_buf = (unsigned int*)data->buf;
> > > -			for (c = 0; c < data->size/4; c++)
> > > -				ptemp_32_buf[c] = fillers[i].value;
> > > -			writesize = data->size;
> > > -			break;
> > > -		}
> > > -		igt_info("Format %s CRC comparison skipped by
> > > design.\n",
> > > -			 (char*)&fillers[i].fourcc);
> > > -
> > > -		return false;
> > >   	default:
> > >   		igt_info("Unsupported mode for test %s\n",
> > >   			 (char*)&fillers[i].fourcc);
> > > @@ -271,26 +190,20 @@ static bool setup_fb(data_t *data,
> > > igt_output_t
> > > *output, igt_plane_t *plane,
> > >   		tiling = LOCAL_DRM_FORMAT_MOD_NONE;
> > >   	}
> > >   
> > > -	for (i = 0; fillers[i].fourcc != 0; i++) {
> > > -		if (fillers[i].fourcc == format)
> > > +	for( i = 0; i < ARRAY_SIZE(fillers)-1; i++ ) {
> > > +		if( fillers[i].fourcc == format )
> > >   			break;
> > >   	}
> > >   
> > >   	switch (fillers[i].bpp) {
> > > -	case NV12:
> > >   	case BYTES_PP_1:
> > >   		bpp = 8;
> > >   		break;
> > > -
> > > -	case P010:
> > > -	case BYTES_PP_2:
> > > -		bpp = 16;
> > > -		break;
> > > -
> > > -	case SKIP4:
> > >   	case BYTES_PP_4:
> > >   		bpp = 32;
> > >   		break;
> > > +	default:
> > > +		return false;
> > 
> > Hmm, we should never hit this condition. Mark this as a failure so
> > that
> > the test gets fixed?
> 
> Yes, this is good place for assert.
> 
> > 
> > >   	}
> > >   
> > >   	igt_get_fb_tile_size(data->gfx_fd, tiling, bpp,
> > > @@ -299,17 +212,6 @@ static bool setup_fb(data_t *data,
> > > igt_output_t
> > > *output, igt_plane_t *plane,
> > >   	data->fb.strides[0] = ALIGN(w * bpp / 8, tile_width);
> > >   	gemsize = data->size = data->fb.strides[0] * ALIGN(h,
> > > tile_height);
> > >   
> > > -	if (fillers[i].bpp == P010 || fillers[i].bpp == NV12) {
> > > -		data->fb.offsets[1] = data->size;
> > > -		data->fb.strides[1] = data->fb.strides[0];
> > > -		gemsize = data->size * 2;
> > > -
> > > -		if (fillers[i].bpp == NV12)
> > > -			data->size += data->fb.strides[1] * ALIGN(h/2,
> > > tile_height);
> > > -
> > > -		num_planes = 2;
> > > -	}
> > > -
> > >   	data->gem_handle = gem_create(data->gfx_fd, gemsize);
> > >   	ret = __gem_set_tiling(data->gfx_fd, data->gem_handle,
> > >   			       igt_fb_mod_to_tiling(tiling),
> > > @@ -386,12 +288,23 @@ static bool prepare_crtc(data_t *data,
> > > igt_output_t *output,
> > >   
> > >   static int
> > >   test_one_mode(data_t* data, igt_output_t *output, igt_plane_t*
> > > plane,
> > > -	      int mode)
> > > +	      int mode, enum pipe pipe)
> > 
> > s/mode/format
> > Comment applies to other places too.
> 
> While I agree I'm so-so about doing this change since it will follow 
> into massive patch for test which I'm anyway wanting to throw away
> in 
> the end.
Okay.

> 
> > 
> > 
> > >   {
> > >   	igt_crc_t current_crc;
> > >   	signed rVal = 0;
> > >   	bool do_crc;
> > > -	char* crccompare[2];
> > > +	int i;
> > > +
> > > +	/*
> > > +	 * Limit tests only to those fb formats listed in fillers table
> > > +	 */
> > > +	for( i = 0; i < ARRAY_SIZE(fillers)-1; i++ ) {
> > > +		if( fillers[i].fourcc == mode )
> > > +			break;
> > > +	}
> > > +
> > > +	if(fillers[i].bpp == 0)
> > > +		return false;

s/return false/return 0 since the function returns 'number' of crc
failures.

> > 
> > .bpp is always valid for the two formats that are tested, isn't it?
> > In
> > that case .bpp == 0 is a test failure.
> > 
> 
> No, here will arrive query for all fb formats which kernel talk
> about. 
> If bpp become zero this will go out from testing mode which is not 
> supported. Ie. C8 or XBGR2101010 will go forward and here other
> formats 
> are ditched.

You are right, my brain automatically filled in a

if (i == ARRAY_SIZE(fillers) - 1)
	return 0;

I feel checking for the loop index to have reached the limit seems more
natural since only two valid formats are tested. Also, we don't really
need the { 0, 0, 0, 0 } element in the fillers array anymore.
 
> 
> > >   
> > >   	if (prepare_crtc(data, output, plane, mode)){
> > >   		/*
> > > @@ -412,29 +325,30 @@ test_one_mode(data_t* data, igt_output_t
> > > *output, igt_plane_t* plane,
> > >   			if (plane->type !=
> > > DRM_PLANE_TYPE_CURSOR) {
> > >   				if
> > > (!igt_check_crc_equal(&current_crc,
> > >   					&data->fullscreen_crc)) 
> > > {
> > > -					crccompare[0] =
> > > igt_crc_to_string(&current_crc);
> > > -					crccompare[1] =
> > > igt_crc_to_string(&data->fullscreen_crc);
> > > -					igt_warn("crc mismatch. target
> > > %.8s, result %.8s.\n", crccompare[0], crccompare[1]);
> > > -					free(crccompare[0]);
> > > -					free(crccompare[1]);
> > > +					igt_warn("crc mismatch.
> > > connector %s using pipe %s" \
> > > +						" plane index %d mode
> > > %.4s\n",
> > > +						igt_output_name(output)
> > > ,
> > > +						kmstest_pipe_name(pipe)
> > > ,
> > > +						plane->index,
> > > +						(char*)&mode);
> > >   					rVal++;
> > >   				}
> > >   			} else {
> > >   				if
> > > (!igt_check_crc_equal(&current_crc,
> > >   					&data->cursor_crc)) {
> > > -					crccompare[0] =
> > > igt_crc_to_string(&current_crc);
> > > -					crccompare[1] =
> > > igt_crc_to_string(&data->cursor_crc);
> > > -					igt_warn("crc mismatch. target
> > > %.8s, result %.8s.\n", crccompare[0], crccompare[1]);
> > > -					free(crccompare[0]);
> > > -					free(crccompare[1]);
> > > +					igt_warn("crc mismatch.
> > > connector %s using pipe %s" \
> > > +						" plane index %d mode
> > > %.4s\n",
> > > +						igt_output_name(output)
> > > ,
> > > +						kmstest_pipe_name(pipe)
> > > ,
> > > +						plane->index,
> > > +						(char*)&mode);
> > >   					rVal++;
> > >   				}
> > >   			}
> > >   		}
> > > -		remove_fb(data, output, plane);
> > > -		return rVal;
> > >   	}
> > > -	return 1;
> > > +	remove_fb(data, output, plane);
> > > +	return rVal;
> > >   }
> > >   
> > >   
> > > @@ -445,14 +359,44 @@ test_available_modes(data_t* data)
> > >   	igt_plane_t *plane;
> > >   	int modeindex;
> > >   	enum pipe pipe;
> > > -	int invalids = 0;
> > > +	int invalids = 0, i, lut_size;
> > >   	drmModePlane *modePlane;
> > > -	char planetype[3][8] = {"OVERLAY\0", "PRIMARY\0", "CURSOR\0" };
> > > +
> > > +	struct {
> > > +	    uint16_t red;
> > > +	    uint16_t green;
> > > +	    uint16_t blue;
> > > +	    uint16_t reserved;
> > > +	} *lut = NULL;
> > >   
> > >   	for_each_pipe_with_valid_output(&data->display, pipe,
> > > output) {
> > >   		igt_output_set_pipe(output, pipe);
> > >   		igt_display_commit2(&data->display, data-
> > > >commit);
> > >   
> > > +		if (igt_pipe_obj_has_prop(&data->display.pipes[pipe],
> > > IGT_CRTC_GAMMA_LUT_SIZE)) {
> > > +			lut_size = igt_pipe_get_prop(&data->display,
> > > pipe,
> > > +						     IGT_CRTC_GAMMA_LUT
> > > _SIZE);
> > > +
> > > +			lut = calloc(sizeof(*lut), lut_size);
> > > +
> > > +			for (i = 0; i < lut_size; i++) {
> > > +				lut[i].red = (i * 0xffff / (lut_size -
> > > 1)) & 0x8000;
> > > +				lut[i].green = (i * 0xffff / (lut_size
> > > - 1)) & 0x8000;
> > > +				lut[i].blue = (i * 0xffff / (lut_size -
> > > 1)) & 0x8000;
> > > +			}
> > > +
> > > +			igt_pipe_replace_prop_blob(&data->display,
> > > pipe,
> > > +						   IGT_CRTC_GAMMA_LUT,
> > > +						   lut, sizeof(*lut) *
> > > lut_size);
> > > +			igt_display_commit2(&data->display, data-
> > > > commit);
> > > 
> > > +
> > > +			for (i = 0; i < lut_size; i++) {
> > > +				lut[i].red = i * 0xffff / (lut_size -
> > > 1);
> > > +				lut[i].green = i * 0xffff / (lut_size -
> > > 1);
> > > +				lut[i].blue = i * 0xffff / (lut_size -
> > > 1);
> > > +			}
> > > +		}
> > > +
> > >   		data->pipe_crc = igt_pipe_crc_new(data->gfx_fd, 
> > > pipe,
> > >   						  INTEL_PIPE_CR
> > > C_SOURCE
> > > _AUTO);
> > >   
> > > @@ -467,29 +411,34 @@ test_available_modes(data_t* data)
> > >   			modePlane = drmModeGetPlane(data-
> > > >gfx_fd,
> > >   						    plane-
> > > >drm_plane-
> > > > plane_id);
> > > 
> > >   
> > > +			if (plane->type == DRM_PLANE_TYPE_CURSOR)
> > > +				continue;
> > > +
> > >   			for (modeindex = 0;
> > >   			     modeindex < modePlane-
> > > >count_formats;
> > >   			     modeindex++) {
> > >   				data->format.dword = modePlane-
> > > > formats[modeindex];
> > > 
> > >   
> > > -				igt_info("Testing connector %s using
> > > pipe %s" \
> > > -					 " plane index %d type %s mode
> > > %s\n",
> > > -					 igt_output_name(output),
> > > -					 kmstest_pipe_name(pipe),
> > > -					 plane->index,
> > > -					 planetype[plane->type],
> > > -					 (char*)&data->format.name);
> > 
> > Not sure why this was removed, perhaps convert it to igt_debug()?
> > It's
> > hard to tell what is being tested now even with the --debug option.
> 
> I did consider this as too verbose. Above where crcs are compared 
> there's printed what failed with more detail than here.
> 
> > 
> > If you want to address the comments in a separate patch, feel free
> > to
> > add
> > Reviewed-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> 
> I'm going to have few CI runs with evolving this patch still.
> 
> I found timing error on this test, it is most likely what is causing
> the 
> noise from this test in CI runs. I somehow never see it on any of my
> own 
> computers but realized it from the logic and first attempt to
> mitigate 
> it changed CI results for the better so I'll get that fixed.
> 
> /Juha-Pekka
> 
> > 
> > 
> > > -
> > >   				invalids += test_one_mode(data,
> > > output,
> > >   							  plane
> > > ,
> > > -							  modePlane-
> > > > formats[modeindex]);
> > > 
> > > +							  modePlane-
> > > > formats[modeindex],
> > > 
> > > +							  pipe);
> > >   			}
> > >   			drmModeFreePlane(modePlane);
> > >   		}
> > >   		igt_pipe_crc_stop(data->pipe_crc);
> > >   		igt_pipe_crc_free(data->pipe_crc);
> > > -		igt_display_commit2(&data->display, data->commit);
> > > +
> > > +		if (lut != NULL) {
> > > +			igt_pipe_replace_prop_blob(&data->display,
> > > pipe,
> > > +						   IGT_CRTC_GAMMA_LUT,
> > > +						   lut, sizeof(*lut) *
> > > lut_size);
> > > +			free(lut);
> > > +			lut = NULL;
> > > +		}
> > > +
> > >   		igt_output_set_pipe(output, PIPE_NONE);
> > > +		igt_display_commit2(&data->display, data->commit);
> > >   	}
> > >   	igt_assert(invalids == 0);
> > >   }
> 
> 

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

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

end of thread, other threads:[~2019-02-21 22:04 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-02-18 14:22 [igt-dev] [PATCH i-g-t] tests/kms_available_modes_crc limit tested fb formats Juha-Pekka Heikkila
2019-02-18 14:28 ` Ville Syrjälä
2019-02-18 15:10 ` [igt-dev] ✓ Fi.CI.BAT: success for tests/kms_available_modes_crc limit tested fb formats (rev8) Patchwork
2019-02-18 18:15 ` [igt-dev] ✓ Fi.CI.IGT: " Patchwork
2019-02-21  0:01 ` [igt-dev] [PATCH i-g-t] tests/kms_available_modes_crc limit tested fb formats Dhinakaran Pandiyan
2019-02-21 11:28   ` Juha-Pekka Heikkila
2019-02-21 22:04     ` Dhinakaran Pandiyan

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.