Re: [igt-dev] [PATCH i-g-t 4/6] tests/fbcon_fbt: Avoid opening debugfs dir repeatedly

["Souza, Jose" <jose.souza@intel.com>]

On Wed, 2018-08-29 at 14:57 -0700, Dhinakaran Pandiyan wrote:
> Make use of igt_debugfs_simple_read() to open debugfs dir just once.

Reviewed-by: José Roberto de Souza <jose.souza@intel.com>

But please consider do this renames in this patch and in the previous,
to make the code more easy to understand: fbc_supported_on_chipset(int
fd) to fbc_supported_on_chipset(int debugfs_fd)

> 
> Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> ---
>  tests/kms_fbcon_fbt.c | 34 ++++++++++++++++++++--------------
>  1 file changed, 20 insertions(+), 14 deletions(-)
> 
> diff --git a/tests/kms_fbcon_fbt.c b/tests/kms_fbcon_fbt.c
> index 280ddbd7..e9376cde 100644
> --- a/tests/kms_fbcon_fbt.c
> +++ b/tests/kms_fbcon_fbt.c
> @@ -39,6 +39,7 @@ static bool do_wait_user = false;
>  
>  struct drm_info {
>  	int fd;
> +	int debugfs_fd;
>  	drmModeResPtr res;
>  	drmModeConnectorPtr connectors[MAX_CONNECTORS];
>  };
> @@ -58,6 +59,7 @@ static void setup_drm(struct drm_info *drm)
>  	int i;
>  
>  	drm->fd = drm_open_driver_master(DRIVER_INTEL);
> +	drm->debugfs_fd = igt_debugfs_dir(drm->fd);
>  
>  	drm->res = drmModeGetResources(drm->fd);
>  	igt_assert(drm->res->count_connectors <= MAX_CONNECTORS);
> @@ -85,9 +87,11 @@ static void teardown_drm(struct drm_info *drm)
>  static bool fbc_supported_on_chipset(int fd)
>  {
>  	char buf[128];
> +	int ret;
>  
> -	igt_debugfs_read(fd, "i915_fbc_status", buf);
> -	if (*buf == '\0') /* !HAS_FBC -> -ENODEV*/
> +	ret = igt_debugfs_simple_read(fd, "i915_fbc_status",
> +				      buf, sizeof(buf));
> +	if (ret < 0)
>  		return false;
>  
>  	return !strstr(buf, "FBC unsupported on this chipset\n");
> @@ -102,7 +106,7 @@ static void fbc_print_status(int fd)
>  {
>  	static char buf[128];
>  
> -	igt_debugfs_read(fd, "i915_fbc_status", buf);
> +	igt_debugfs_simple_read(fd, "i915_fbc_status", buf,
> sizeof(buf));
>  	igt_debug("FBC status: %s\n", buf);
>  }
>  
> @@ -110,7 +114,7 @@ static bool fbc_is_enabled(int fd)
>  {
>  	char buf[128];
>  
> -	igt_debugfs_read(fd, "i915_fbc_status", buf);
> +	igt_debugfs_simple_read(fd, "i915_fbc_status", buf,
> sizeof(buf));
>  	return strstr(buf, "FBC enabled\n");
>  }
>  
> @@ -163,9 +167,11 @@ static void set_mode_for_one_screen(struct
> drm_info *drm, struct igt_fb *fb,
>  static bool psr_supported_on_chipset(int fd)
>  {
>  	char buf[256];
> +	int ret;
>  
> -	igt_debugfs_read(fd, "i915_edp_psr_status", buf);
> -	if (*buf == '\0') /* !HAS_PSR -> -ENODEV*/
> +	ret = igt_debugfs_simple_read(fd, "i915_edp_psr_status",
> +				      buf, sizeof(buf));
> +	if (ret < 0)
>  		return false;
>  
>  	return strstr(buf, "Sink_Support: yes\n");
> @@ -180,7 +186,7 @@ static void psr_print_status(int fd)
>  {
>  	static char buf[256];
>  
> -	igt_debugfs_read(fd, "i915_edp_psr_status", buf);
> +	igt_debugfs_simple_read(fd, "i915_edp_psr_status", buf,
> sizeof(buf));
>  	igt_debug("PSR status: %s\n", buf);
>  }
>  
> @@ -188,7 +194,7 @@ static bool psr_is_enabled(int fd)
>  {
>  	char buf[256];
>  
> -	igt_debugfs_read(fd, "i915_edp_psr_status", buf);
> +	igt_debugfs_simple_read(fd, "i915_edp_psr_status", buf,
> sizeof(buf));
>  	return strstr(buf, "\nHW Enabled & Active bit: yes\n");
>  }
>  
> @@ -229,24 +235,24 @@ static void subtest(struct feature *feature,
> bool suspend)
>  
>  	setup_drm(&drm);
>  
> -	igt_require(feature->supported_on_chipset(drm.fd));
> +	igt_require(feature->supported_on_chipset(drm.debugfs_fd));
>  
>  	disable_features();
>  	igt_set_module_param_int(feature->param_name, 1);
>  
>  	kmstest_unset_all_crtcs(drm.fd, drm.res);
>  	wait_user("Modes unset.");
> -	igt_assert(!feature->wait_until_enabled(drm.fd));
> +	igt_assert(!feature->wait_until_enabled(drm.debugfs_fd));
>  
>  	set_mode_for_one_screen(&drm, &fb, feature-
> >connector_possible_fn);
>  	wait_user("Screen set.");
> -	igt_assert(feature->wait_until_enabled(drm.fd));
> +	igt_assert(feature->wait_until_enabled(drm.debugfs_fd));
>  
>  	if (suspend) {
>  		igt_system_suspend_autoresume(SUSPEND_STATE_MEM,
>  					      SUSPEND_TEST_NONE);
>  		sleep(5);
> -		igt_assert(feature->wait_until_enabled(drm.fd));
> +		igt_assert(feature-
> >wait_until_enabled(drm.debugfs_fd));
>  	}
>  
>  	igt_remove_fb(drm.fd, &fb);
> @@ -256,13 +262,13 @@ static void subtest(struct feature *feature,
> bool suspend)
>  	sleep(3);
>  
>  	wait_user("Back to fbcon.");
> -	igt_assert(!feature->wait_until_enabled(drm.fd));
> +	igt_assert(!feature->wait_until_enabled(drm.debugfs_fd));
>  
>  	if (suspend) {
>  		igt_system_suspend_autoresume(SUSPEND_STATE_MEM,
>  					      SUSPEND_TEST_NONE);
>  		sleep(5);
> -		igt_assert(!feature->wait_until_enabled(drm.fd));
> +		igt_assert(!feature-
> >wait_until_enabled(drm.debugfs_fd));
>  	}
>  }
>  
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev