Re: [igt-dev] [PATCH i-g-t] tests/kms_available_modes_crc: Fix handling of test comparisons

[Juha-Pekka Heikkila <juhapekka.heikkila@gmail.com>]

On 23.1.2019 21.50, Pandiyan, Dhinakaran wrote:
> On Wed, 2018-12-19 at 15:41 +0200, Juha-Pekka Heikkila wrote:
>> Fixed handling of single plane modes so used bpp never get to
>> be 0 and ease off crc comparison a bit with gamma tables as
>> idea of this test is to see chosen modes are able to initialize.
>>
>> Release output to PIPE_ANY at end of test per connector.
>>
>> Reduce verboseness if there's no errors.
>>
>> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=106641
>> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=106701
>> Signed-off-by: Juha-Pekka Heikkila <juhapekka.heikkila@gmail.com>
>> ---
>>   tests/kms_available_modes_crc.c | 169 ++++++++++++++++++++--------
>> ------------
>>   1 file changed, 86 insertions(+), 83 deletions(-)
>>
>> diff --git a/tests/kms_available_modes_crc.c
>> b/tests/kms_available_modes_crc.c
>> index 45a4775..04472d0 100644
>> --- a/tests/kms_available_modes_crc.c
>> +++ b/tests/kms_available_modes_crc.c
>> @@ -54,6 +54,12 @@ typedef struct {
>>   
>>   	igt_crc_t cursor_crc;
>>   	igt_crc_t fullscreen_crc;
>> +
>> +	/*
>> +	 * If there's unknown format setting up mode can fail
>> +	 * without failing entire test.
>> +	 */
>> +	bool allow_fail;
>>   } data_t;
>>   
>>   
>> @@ -120,7 +126,6 @@ static void generate_comparison_crc_list(data_t
>> *data, igt_output_t *output)
>>   
>>   static const struct {
>>   	uint32_t	fourcc;
>> -	char		zeropadding;
>>   	enum		{ BYTES_PP_1=1,
>>   				BYTES_PP_2=2,
>>   				BYTES_PP_4=4,
>> @@ -129,42 +134,33 @@ static const struct {
>>   				SKIP4 } 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},
>> +	{ DRM_FORMAT_C8, BYTES_PP_1, 0xff},
>> +	{ DRM_FORMAT_RGB565, BYTES_PP_2, 0xffff},
>> +	{ DRM_FORMAT_XRGB8888, BYTES_PP_4, 0xffffffff},
>> +	{ DRM_FORMAT_XBGR8888, BYTES_PP_4, 0xffffffff},
>> +	{ DRM_FORMAT_ARGB8888, BYTES_PP_4, 0xffffffff},
>> +	{ DRM_FORMAT_ABGR8888, BYTES_PP_4, 0x00ffffff},
>> +	{ DRM_FORMAT_XRGB2101010, BYTES_PP_4, 0xffffffff},
>> +	{ DRM_FORMAT_XBGR2101010, BYTES_PP_4, 0xffffffff},
>> +	{ DRM_FORMAT_YUYV, BYTES_PP_4, 0x80eb80eb},
>> +	{ DRM_FORMAT_YVYU, BYTES_PP_4, 0x80eb80eb},
>> +	{ DRM_FORMAT_VYUY, BYTES_PP_4, 0xeb80eb80},
>> +	{ DRM_FORMAT_UYVY, BYTES_PP_4, 0xeb80eb80},
>>   
>>   	/*
>>   	 * (semi-)planar formats
>>   	 */
>> -	{ DRM_FORMAT_NV12, 0, NV12, 0x80eb},
>> +	{ DRM_FORMAT_NV12, NV12, 0x80eb},
>>   #ifdef DRM_FORMAT_P010
>> -	{ DRM_FORMAT_P010, 0, P010, 0x8000eb00},
>> +	{ DRM_FORMAT_P010, P010, 0x8000eb00},
>>   #endif
>>   #ifdef DRM_FORMAT_P012
>> -	{ DRM_FORMAT_P012, 0, P010, 0x8000eb00},
>> +	{ DRM_FORMAT_P012, P010, 0x8000eb00},
>>   #endif
>>   #ifdef DRM_FORMAT_P016
>> -	{ DRM_FORMAT_P016, 0, P010, 0x8000eb00},
>> +	{ DRM_FORMAT_P016, P010, 0x8000eb00},
>>   #endif
>> -	{ 0, 0, 0, 0 }
>> +	{ 0, SKIP4, 0 }
>>   };
>>   
>>   /*
>> @@ -221,24 +217,9 @@ static bool fill_in_fb(data_t *data,
>> igt_output_t *output, igt_plane_t *plane,
>>   		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);
>> +		igt_info("Unsupported mode for testing CRC %.4s\n",
>> +			 (char *)&format);
>>   		return false;
>>   	}
>>   
>> @@ -261,6 +242,8 @@ static bool setup_fb(data_t *data, igt_output_t
>> *output, igt_plane_t *plane,
>>   	int bpp = 0;
>>   	int i;
>>   
>> +	data->allow_fail = false;
>> +
>>   	mode = igt_output_get_mode(output);
>>   	if (plane->type != DRM_PLANE_TYPE_CURSOR) {
>>   		w = mode->hdisplay;
>> @@ -289,6 +272,8 @@ static bool setup_fb(data_t *data, igt_output_t
>> *output, igt_plane_t *plane,
>>   		break;
>>   
>>   	case SKIP4:
>> +		data->allow_fail = true;
>> +		/* fall through */
>>   	case BYTES_PP_4:
>>   		bpp = 32;
>>   		break;
>> @@ -319,7 +304,7 @@ static bool setup_fb(data_t *data, igt_output_t
>> *output, igt_plane_t *plane,
>>   
>>   	if(ret < 0) {
>>   		igt_info("Creating fb for format %s failed, return code
>> %d\n",
>> -			 (char*)&data->format.name, ret);
>> +			 (char *)&data->format.name, ret);
>>   
>>   		return false;
>>   	}
>> @@ -380,13 +365,14 @@ 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)
>> +test_one_mode(data_t *data, igt_output_t *output, igt_plane_t
>> *plane,
>> +	      enum pipe pipe, int mode)
>>   {
>>   	igt_crc_t current_crc;
>>   	signed rVal = 0;
>>   	bool do_crc;
>> -	char* crccompare[2];
>> +	char *crccompare[2];
>> +	igt_crc_t *p_crc;
>>   
>>   	if (prepare_crtc(data, output, plane, mode)){
>>   		/*
>> @@ -404,32 +390,32 @@ test_one_mode(data_t* data, igt_output_t
>> *output, igt_plane_t* plane,
>>   		if (do_crc) {
>>   			igt_pipe_crc_get_current(data->gfx_fd, data-
>>> pipe_crc, &current_crc);
>>   
>> -			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]);
>> -					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]);
>> +			if (plane->type != DRM_PLANE_TYPE_CURSOR)
>> +				p_crc = &data->fullscreen_crc;
>> +			else
>> +				p_crc = &data->cursor_crc;
>> +
>> +			if (!igt_check_crc_equal(&current_crc, p_crc))
>> {
>> +				crccompare[0] =
>> igt_crc_to_string(&current_crc);
>> +				crccompare[1] =
>> igt_crc_to_string(p_crc);
>> +				igt_warn("crc mismatch on %s PIPE %s
>> plane %d format %.4s. target %.8s, result %.8s.\n",
>> +					 igt_output_name(output),
>> +					 kmstest_pipe_name(pipe),
>> +					 plane->index,
>> +					 (char *)&data->format.name,
>> +					 crccompare[0],
>> +					 crccompare[1]);
> 
> Can you please split this debug print into a separate patch? That way
> we can see which format/plane/pipe combination is failing on ICL while
> you are working on the gamma fixes?

This is what you were hoping for? 
https://patchwork.freedesktop.org/series/55727/


/Juha-Pekka


> 
> 
> 
>> +
>> +				free(crccompare[0]);
>> +				free(crccompare[1]);
>> +				if (!data->allow_fail)
>>   					rVal++;
>> -				}
>>   			}
>>   		}
>>   		remove_fb(data, output, plane);
>>   		return rVal;
>>   	}
>> -	return 1;
>> +	return data->allow_fail ? 0 : 1;
>>   }
>>   
>>   
>> @@ -440,14 +426,38 @@ test_available_modes(data_t* data)
>>   	igt_plane_t *plane;
>>   	int modeindex;
>>   	enum pipe pipe;
>> -	int invalids = 0;
>> +	int failed_crcs = 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;
>>   
>>   	for_each_pipe_with_valid_output(&data->display, pipe, output) {
>> +		igt_display_reset(&data->display);
>>   		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)) & 0xfc00;
>> +				lut[i].green = (i * 0xffff / (lut_size
>> - 1)) & 0xfc00;
>> +				lut[i].blue = (i * 0xffff / (lut_size -
>> 1)) & 0xfc00;
>> +			}
>> +
>> +			igt_pipe_replace_prop_blob(&data->display,
>> pipe,
>> +						   IGT_CRTC_GAMMA_LUT,
>> +						   lut, sizeof(*lut) *
>> lut_size);
>> +			free(lut);
>> +		}
>> +
>>   		data->pipe_crc = igt_pipe_crc_new(data->gfx_fd, pipe,
>>   						  INTEL_PIPE_CRC_SOURCE
>> _AUTO);
>>   
>> @@ -467,25 +477,18 @@ test_available_modes(data_t* data)
>>   			     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]);
>> +				failed_crcs += test_one_mode(data,
>> output,
>> +							     plane,
>> pipe,
>> +							     modePlane-
>>> formats[modeindex]);
>>   			}
>>   			drmModeFreePlane(modePlane);
>>   		}
>>   		igt_pipe_crc_stop(data->pipe_crc);
>>   		igt_pipe_crc_free(data->pipe_crc);
>> -		igt_display_commit2(&data->display, data->commit);
>> +
>> +		igt_output_set_pipe(output, PIPE_ANY);
>>   	}
>> -	igt_assert(invalids == 0);
>> +	igt_assert(failed_crcs == 0);
>>   }
>>   
>>   

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