[Intel-gfx] [PATCH 5/5] tests/kms_psr_sink_crc: Add manual mode.

Daniel Vetter daniel at ffwll.ch
Mon Dec 8 01:33:02 PST 2014


On Fri, Dec 05, 2014 at 08:14:24PM -0500, Rodrigo Vivi wrote:
> Sink CRC is the most reliable way to test PSR. However in some platforms
> apparently auto generated packages force panel to keep calculating CRC invalidating
> our current sink crc check over debugfs.
> 
> So, this manual test help us to find possible gaps on this platforms where we cannot
> trust on sink crc checks.
> 
> Signed-off-by: Rodrigo Vivi <rodrigo.vivi at intel.com>
> ---
>  tests/kms_psr_sink_crc.c | 74 ++++++++++++++++++++++++++++++++++++++++++++----
>  1 file changed, 68 insertions(+), 6 deletions(-)
> 
> diff --git a/tests/kms_psr_sink_crc.c b/tests/kms_psr_sink_crc.c
> index 155c25f..3853031 100644
> --- a/tests/kms_psr_sink_crc.c
> +++ b/tests/kms_psr_sink_crc.c
> @@ -26,6 +26,7 @@
>  #include <stdbool.h>
>  #include <stdio.h>
>  #include <string.h>
> +#include <termios.h>
>  
>  #include "ioctl_wrappers.h"
>  #include "drmtest.h"
> @@ -37,6 +38,7 @@
>  #include "igt_aux.h"
>  
>  bool running_with_psr_disabled;
> +bool manual;
>  
>  #define CRC_BLACK "000000000000"
>  
> @@ -244,6 +246,9 @@ static void get_sink_crc(data_t *data, char *crc) {
>  	int ret;
>  	FILE *file;
>  
> +	if (manual)
> +		return;
> +
>  	file = igt_debugfs_fopen("i915_sink_crc_eDP1", "r");
>  	igt_require(file);
>  
> @@ -271,6 +276,9 @@ static bool is_green(char *crc)
>  	unsigned int rh, gh, bh, mask;
>  	int ret;
>  
> +	if (manual)
> +		return false;
> +
>  	sscanf(color_mask, "%4x", &mask);
>  
>  	memcpy(rs, &crc[0], 4);
> @@ -293,6 +301,31 @@ static bool is_green(char *crc)
>  		(bh & mask) == 0);
>  }
>  
> +static void assert(bool condition, const char *debug_msg)

This collides with the libc assert() macro.

> +{
> +	if (manual) {
> +		struct termios oldt, newt;
> +		char c;
> +
> +		igt_info("%s? [Y/n]: ", debug_msg);
> +
> +		tcgetattr(STDIN_FILENO, &oldt);
> +		newt = oldt;
> +		newt.c_lflag &= ~(ICANON);
> +		tcsetattr(STDIN_FILENO, TCSANOW, &newt);
> +		c = getchar();
> +		if (c != '\n')
> +			igt_info("\n");
> +		tcsetattr(STDIN_FILENO, TCSANOW, &oldt);
> +
> +		if (c == 'n' || c == 'N')
> +			igt_fail(-1);

tbh not sure whether this is all that useful to wait for keypresses and
then make the test fail/pass. You need to manually run the testcase
anyway. Imo better to just use igt_debug_wait_for_keypress. That one also
magically skips if the option hasn't been set. And yeah we might good to
add a new common option like --interactive-debug for it instead of just
the env variable. You could even reuse that here then (by exporting
an igt_interactive_debug boolean or so).

> +	} else {
> +		igt_debug("%s\n", debug_msg);
> +		igt_assert(condition);

igt_assert_f. Also I'd just use igt_assert_f(manual || condition) instead,
simplifies the control flow.

> +	}
> +}
> +
>  static void test_crc(data_t *data)
>  {
>  	uint32_t handle = data->fb_white.gem_handle;
> @@ -300,18 +333,19 @@ static void test_crc(data_t *data)
>  	void *ptr;
>  	char ref_crc[12];
>  	char crc[12];
> +	char manual_debug[50] = "";

Just make this a const char * point and assign strings to it. No need for
strcpy.

>  
>  	igt_plane_set_fb(data->primary, &data->fb_green);
>  	igt_display_commit(&data->display);
>  
>  	/* Confirm that screen became Green */
>  	get_sink_crc(data, ref_crc);
> -	igt_assert(is_green(ref_crc));
> +	assert(is_green(ref_crc), "screen GREEN");
>  
>  	/* Confirm screen stays Green after PSR got active */
>  	igt_assert(wait_psr_entry(data, 10));
>  	get_sink_crc(data, ref_crc);
> -	igt_assert(is_green(ref_crc));
> +	assert(is_green(ref_crc), "screen GREEN");
>  
>  	/* Setting a secondary fb/plane */
>  	switch (data->test_plane) {
> @@ -325,7 +359,10 @@ static void test_crc(data_t *data)
>  	/* Confirm it is not Green anymore */
>  	igt_assert(wait_psr_entry(data, 10));
>  	get_sink_crc(data, ref_crc);
> -	igt_assert(!is_green(ref_crc));
> +	if (data->test_plane == PRIMARY)
> +		assert(!is_green(ref_crc), "screen WHITE");
> +	else
> +		assert(!is_green(ref_crc), "GREEN background with WHITE box");
>  
>  	switch (data->op) {
>  	case PAGE_FLIP:
> @@ -333,7 +370,8 @@ static void test_crc(data_t *data)
>  		igt_assert(drmModePageFlip(data->drm_fd, data->crtc_id,
>  					   data->fb_green.fb_id, 0, NULL) == 0);
>  		get_sink_crc(data, crc);
> -		igt_assert(is_green(crc));
> +		assert(is_green(crc), "screen GREEN");
> +		strcpy(manual_debug, "still GREEN");
>  		break;
>  	case MMAP_GTT:
>  		ptr = gem_mmap__gtt(data->drm_fd, handle, data->mod_size,
> @@ -342,6 +380,8 @@ static void test_crc(data_t *data)
>  			       I915_GEM_DOMAIN_GTT, I915_GEM_DOMAIN_GTT);
>  		memset(ptr, 0xcc, data->mod_size);
>  		munmap(ptr, data->mod_size);
> +		strcpy(manual_debug,
> +		       "BLACK or TRANSPARENT mark on top of plane in test");
>  		break;
>  	case MMAP_GTT_WAITING:
>  		ptr = gem_mmap__gtt(data->drm_fd, handle, data->mod_size,
> @@ -352,7 +392,11 @@ static void test_crc(data_t *data)
>  		/* Printing white on white so the screen shouldn't change */
>  		memset(ptr, 0xff, data->mod_size);
>  		get_sink_crc(data, crc);
> -		igt_assert(strcmp(ref_crc, crc) == 0);
> +		if (data->test_plane == PRIMARY)
> +			assert(strcmp(ref_crc, crc) == 0, "screen WHITE");
> +		else
> +			assert(strcmp(ref_crc, crc) == 0,
> +			       "GREEN background with WHITE box");
>  
>  		igt_info("Waiting 10s...\n");
>  		sleep(10);
> @@ -360,6 +404,8 @@ static void test_crc(data_t *data)
>  		/* Now lets print black to change the screen */
>  		memset(ptr, 0, data->mod_size);
>  		munmap(ptr, data->mod_size);
> +		strcpy(manual_debug,
> +		       "BLACK or TRANSPARENT mark on top of plane in test");
>  		break;
>  	case MMAP_CPU:
>  		ptr = gem_mmap__cpu(data->drm_fd, handle, 0, data->mod_size, PROT_WRITE);
> @@ -368,26 +414,34 @@ static void test_crc(data_t *data)
>  		memset(ptr, 0, data->mod_size);
>  		munmap(ptr, data->mod_size);
>  		gem_sw_finish(data->drm_fd, handle);
> +		strcpy(manual_debug,
> +		       "BLACK or TRANSPARENT mark on top of plane in test");
>  		break;
>  	case BLT:
>  		fill_blt(data, handle, 0);
> +		strcpy(manual_debug,
> +		       "BLACK or TRANSPARENT mark on top of plane in test");
>  		break;
>  	case RENDER:
>  		fill_render(data, handle, 0);
> +		strcpy(manual_debug,
> +		       "BLACK or TRANSPARENT mark on top of plane in test");
>  		break;
>  	case PLANE_MOVE:
>  		/* Only in use when testing Sprite and Cursor */
>  		igt_plane_set_position(test_plane, 500, 500);
>  		igt_display_commit(&data->display);
> +		strcpy(manual_debug, "White box moved to 500x500");
>  		break;
>  	case PLANE_ONOFF:
>  		/* Only in use when testing Sprite and Cursor */
>  		igt_plane_set_fb(test_plane, NULL);
>  		igt_display_commit(&data->display);
> +		strcpy(manual_debug, "screen GREEN");
>  		break;
>  	}
>  	get_sink_crc(data, crc);
> -	igt_assert(strcmp(ref_crc, crc) != 0);
> +	assert(strcmp(ref_crc, crc) != 0, manual_debug);
>  }
>  
>  static void test_cleanup(data_t *data) {
> @@ -480,6 +534,9 @@ static int opt_handler(int opt, int opt_index)
>  	case 'n':
>  		running_with_psr_disabled = true;
>  		break;
> +	case 'm':
> +		manual = true;
> +		break;
>  	default:
>  		igt_assert(0);
>  	}
> @@ -493,6 +550,7 @@ int main(int argc, char *argv[])
>  	       "  --no-psr\tRun test without PSR to check the CRC test logic.";
>  	static struct option long_options[] = {
>  		{"no-psr", 0, 0, 'n'},
> +		{"manual", 0, 0, 'm'},
>  		{ 0, 0, 0, 0 }
>  	};
>  	data_t data = {};
> @@ -507,6 +565,10 @@ int main(int argc, char *argv[])
>  		kmstest_set_vt_graphics_mode();
>  		data.devid = intel_get_drm_devid(data.drm_fd);
>  
> +		if ((IS_VALLEYVIEW(data.devid) || IS_CHERRYVIEW(data.devid)) &&
> +		    !manual)
> +			igt_skip("Sink CRC is unreliable on this platform when PSR is enabled. Test available only on manual mode. Run again with --manual\n");
> +
>  		igt_skip_on(!psr_enabled(&data));
>  
>  		data.bufmgr = drm_intel_bufmgr_gem_init(data.drm_fd, 4096);
> -- 
> 1.9.3
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch


More information about the Intel-gfx mailing list