[Intel-gfx] [PATCH i-g-t] tests/kms_fbcon_fbt: Report fbc_status on error

[Daniel Vetter]

On Wed, Oct 18, 2017 at 11:39:15PM -0200, Gabriel Krisman Bertazi wrote:
> knowing the assertion triggered on wait_until_enabled() is not that
> useful without knowing what exactly caused the failure.  It might be an
> user error, like too little stolen memory by the bios, or an actual
> issue in the kernel.  So, let's make life easier, particularly for the
> CI, by printing the status before failing out.
> 
> Case in point:
> 
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=101718
> 
> Signed-off-by: Gabriel Krisman Bertazi <krisman at collabora.co.uk>

The more igt-like (and simpler) pattern is to dump such debug info with
igt_debug. When an igt_assert fails it'll dump all that stuff for you, and
bonus, you can crank up the log level when debugging locally and watch it
all.

Care to redo the patch along those lines?

Aside: We currently don't dump the log buffer when we hit an igt_skip, but
that might be useful to do (at least in some cases), when we have
pass/skip flip-floppers in CI.
-Daniel

> ---
>  tests/kms_fbcon_fbt.c | 35 ++++++++++++++++++++++++++++++-----
>  1 file changed, 30 insertions(+), 5 deletions(-)
> 
> diff --git a/tests/kms_fbcon_fbt.c b/tests/kms_fbcon_fbt.c
> index d0090912694e..4027258ef1ba 100644
> --- a/tests/kms_fbcon_fbt.c
> +++ b/tests/kms_fbcon_fbt.c
> @@ -95,6 +95,14 @@ static bool connector_can_fbc(drmModeConnectorPtr connector)
>  	return true;
>  }
>  
> +static char *fbc_get_status(int fd)
> +{
> +	static char buf[128];
> +
> +	igt_debugfs_read(fd, "i915_fbc_status", buf);
> +	return buf;
> +}
> +
>  static bool fbc_is_enabled(int fd)
>  {
>  	char buf[128];
> @@ -160,6 +168,14 @@ static bool connector_can_psr(drmModeConnectorPtr connector)
>  	return (connector->connector_type == DRM_MODE_CONNECTOR_eDP);
>  }
>  
> +static char *psr_get_status(int fd)
> +{
> +	static char buf[256];
> +
> +	igt_debugfs_read(fd, "i915_edp_psr_status", buf);
> +	return buf;
> +}
> +
>  static bool psr_is_enabled(int fd)
>  {
>  	char buf[256];
> @@ -177,16 +193,19 @@ struct feature {
>  	bool (*supported_on_chipset)(int fd);
>  	bool (*wait_until_enabled)(int fd);
>  	bool (*connector_possible_fn)(drmModeConnectorPtr connector);
> +	char *(*get_status)(int fd);
>  	const char *param_name;
>  } fbc = {
>  	.supported_on_chipset = fbc_supported_on_chipset,
>  	.wait_until_enabled = fbc_wait_until_enabled,
>  	.connector_possible_fn = connector_can_fbc,
> +	.get_status = fbc_get_status,
>  	.param_name = "enable_fbc",
>  }, psr = {
>  	.supported_on_chipset = psr_supported_on_chipset,
>  	.wait_until_enabled = psr_wait_until_enabled,
>  	.connector_possible_fn = connector_can_psr,
> +	.get_status = psr_get_status,
>  	.param_name = "enable_psr",
>  };
>  
> @@ -196,6 +215,12 @@ static void disable_features(void)
>  	igt_set_module_param_int(psr.param_name, 0);
>  }
>  
> +#define feature_enabled_or_die(drm_info, feature) \
> +	igt_assert_f(feature->wait_until_enabled(drm_info.fd), "%s\n", feature->get_status(drm_info.fd));
> +
> +#define feature_disabled_or_die(drm_info, feature) \
> +	igt_assert_f(!feature->wait_until_enabled(drm_info.fd), "%s\n", feature->get_status(drm_info.fd));
> +
>  static void subtest(struct feature *feature, bool suspend)
>  {
>  	struct drm_info drm;
> @@ -210,17 +235,17 @@ static void subtest(struct feature *feature, bool suspend)
>  
>  	kmstest_unset_all_crtcs(drm.fd, drm.res);
>  	wait_user("Modes unset.");
> -	igt_assert(!feature->wait_until_enabled(drm.fd));
> +	feature_disabled_or_die(drm, feature);
>  
>  	set_mode_for_one_screen(&drm, &fb, feature->connector_possible_fn);
>  	wait_user("Screen set.");
> -	igt_assert(feature->wait_until_enabled(drm.fd));
> +	feature_enabled_or_die(drm, feature);
>  
>  	if (suspend) {
>  		igt_system_suspend_autoresume(SUSPEND_STATE_MEM,
>  					      SUSPEND_TEST_NONE);
>  		sleep(5);
> -		igt_assert(feature->wait_until_enabled(drm.fd));
> +		feature_enabled_or_die(drm, feature);
>  	}
>  
>  	igt_remove_fb(drm.fd, &fb);
> @@ -230,13 +255,13 @@ static void subtest(struct feature *feature, bool suspend)
>  	sleep(3);
>  
>  	wait_user("Back to fbcon.");
> -	igt_assert(!feature->wait_until_enabled(drm.fd));
> +	feature_disabled_or_die(drm, feature);
>  
>  	if (suspend) {
>  		igt_system_suspend_autoresume(SUSPEND_STATE_MEM,
>  					      SUSPEND_TEST_NONE);
>  		sleep(5);
> -		igt_assert(!feature->wait_until_enabled(drm.fd));
> +		feature_disabled_or_die(drm, feature);
>  	}
>  }
>  
> -- 
> 2.11.0
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch