Re: [igt-dev] [PATCH i-g-t 2/2] tests/kms_fbcon_fbt: Reduce execution time by not calling wait_until_enabled()

["Mun, Gwan-gyeong" <gwan-gyeong.mun@intel.com>]

On Wed, 2020-04-08 at 13:19 -0700, José Roberto de Souza wrote:
> After unset all CRTCs is expected that FBC and PSR is disabled,
> calling wait_until_enabled() will make it wait until timeout to it
> return false and pass the test.
> 
> So instead lets implement is_disabled() hook, as the
> kmstest_unset_all_crtcs() is a synchronous call, the features will
> be already disabled after it, so no need to do any wait.
> 
> Signed-off-by: José Roberto de Souza <jose.souza@intel.com>
> ---
>  lib/igt_psr.c         | 10 ++++++++++
>  lib/igt_psr.h         |  1 +
>  tests/kms_fbcon_fbt.c | 21 ++++++++++++++++++++-
>  3 files changed, 31 insertions(+), 1 deletion(-)
> 
> diff --git a/lib/igt_psr.c b/lib/igt_psr.c
> index 83ccacdd..f92eff6c 100644
> --- a/lib/igt_psr.c
> +++ b/lib/igt_psr.c
> @@ -25,6 +25,16 @@
>  #include "igt_sysfs.h"
>  #include <errno.h>
>  
> +bool psr_disabled_check(int debugfs_fd)
> +{
> +	char buf[PSR_STATUS_MAX_LEN];
> +
> +	igt_debugfs_simple_read(debugfs_fd, "i915_edp_psr_status", buf,
> +				sizeof(buf));
> +
> +	return strstr(buf, "PSR mode: disabled\n");
> +}
> +
>  static bool psr_active_check(int debugfs_fd, enum psr_mode mode)
>  {
>  	char buf[PSR_STATUS_MAX_LEN];
> diff --git a/lib/igt_psr.h b/lib/igt_psr.h
> index ca385736..02ce760b 100644
> --- a/lib/igt_psr.h
> +++ b/lib/igt_psr.h
> @@ -35,6 +35,7 @@ enum psr_mode {
>  	PSR_MODE_2
>  };
>  
> +bool psr_disabled_check(int debugfs_fd);
>  bool psr_wait_entry(int debugfs_fd, enum psr_mode mode);
>  bool psr_wait_update(int debugfs_fd, enum psr_mode mode);
>  bool psr_long_wait_update(int debugfs_fd, enum psr_mode mode);
> diff --git a/tests/kms_fbcon_fbt.c b/tests/kms_fbcon_fbt.c
> index 546dff99..143be3e3 100644
> --- a/tests/kms_fbcon_fbt.c
> +++ b/tests/kms_fbcon_fbt.c
> @@ -135,6 +135,14 @@ static bool fbc_wait_until_enabled(int
> debugfs_fd)
>  	return r;
>  }
>  
> +static bool fbc_is_disabled(int debugfs_fd)
> +{
> +	bool r = fbc_check_status(debugfs_fd, false);
> +
> +	fbc_print_status(debugfs_fd);
> +	return r;
> +}
> +
>  static bool fbc_wait_until_disabled(int debugfs_fd)
>  {
>  	bool r = igt_wait(fbc_check_status(debugfs_fd, false), 5000,
> 1);
> @@ -250,6 +258,14 @@ static bool psr_wait_until_enabled(int
> debugfs_fd)
>  	return r;
>  }
>  
> +static bool psr_is_disabled(int debugfs_fd)
> +{
> +	bool r = psr_disabled_check(debugfs_fd);
> +
> +	psr_print_status(debugfs_fd);
> +	return r;
> +}
> +
>  static bool psr_supported_on_chipset(int debugfs_fd)
>  {
>  	return psr_sink_support(debugfs_fd, PSR_MODE_1);
> @@ -280,18 +296,21 @@ static inline void psr_debugfs_enable(int
> debugfs_fd)
>  struct feature {
>  	bool (*supported_on_chipset)(int debugfs_fd);
>  	bool (*wait_until_enabled)(int debugfs_fd);
> +	bool (*is_disabled)(int debugfs_fd);
>  	bool (*wait_until_update)(struct drm_info *drm);
>  	bool (*connector_possible_fn)(drmModeConnectorPtr connector);
>  	void (*enable)(int debugfs_fd);
>  } fbc = {
>  	.supported_on_chipset = fbc_supported_on_chipset,
>  	.wait_until_enabled = fbc_wait_until_enabled,
> +	.is_disabled = fbc_is_disabled,
>  	.wait_until_update = fbc_wait_until_update,
>  	.connector_possible_fn = connector_can_fbc,
>  	.enable = fbc_modparam_enable,
>  }, psr = {
>  	.supported_on_chipset = psr_supported_on_chipset,
>  	.wait_until_enabled = psr_wait_until_enabled,
> +	.is_disabled = psr_is_disabled,
>  	.wait_until_update = psr_wait_until_update,
>  	.connector_possible_fn = connector_can_psr,
>  	.enable = psr_debugfs_enable,
> @@ -310,7 +329,7 @@ static void subtest(struct drm_info *drm, struct
> feature *feature, bool suspend)
>  
>  	kmstest_unset_all_crtcs(drm->fd, drm->res);
>  	wait_user("Modes unset.");
> -	igt_assert(!feature->wait_until_enabled(drm->debugfs_fd));
> +	igt_assert(feature->is_disabled(drm->debugfs_fd));
>  
>  	set_mode_for_one_screen(drm, &fb, feature-
> >connector_possible_fn);
>  	wait_user("Screen set.");

looks good to me.

Reviewed-by: Gwan-gyeong Mun <gwan-gyeong.mun@intel.com>
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev