diff mbox series

[igt-dev,i-g-t] tests/pm_backlight.c : Brightness test with DPMS and System suspend.

Message ID 1535026588-12495-1-git-send-email-jyoti.r.yadav@intel.com (mailing list archive)
State New, archived
Headers show
Series [igt-dev,i-g-t] tests/pm_backlight.c : Brightness test with DPMS and System suspend. | expand

Commit Message

Yadav, Jyoti R Aug. 23, 2018, 12:16 p.m. UTC 
From: Jyoti <jyoti.r.yadav@intel.com>

BIOS programs few of PWM related registers during initial boot.
But during System suspend those registers are cleared.
This test aim to check whether display programs those registers properly after
system resume.
Also checks brightness programming during DPMS ON/OFF cycle to check backlight
programming is done properly from display side.

Signed-off-by: Jyoti Yadav <jyoti.r.yadav@intel.com>
---
 tests/pm_backlight.c | 52 ++++++++++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 50 insertions(+), 2 deletions(-)

Comments

Rodrigo Vivi Aug. 30, 2018, 12:40 a.m. UTC | #1 
On Thu, Aug 23, 2018 at 08:16:28AM -0400, Jyoti Yadav wrote:
> From: Jyoti <jyoti.r.yadav@intel.com>
> 
> BIOS programs few of PWM related registers during initial boot.
> But during System suspend those registers are cleared.
> This test aim to check whether display programs those registers properly after
> system resume.
> Also checks brightness programming during DPMS ON/OFF cycle to check backlight
> programming is done properly from display side.
> 
> Signed-off-by: Jyoti Yadav <jyoti.r.yadav@intel.com>
> ---
>  tests/pm_backlight.c | 52 ++++++++++++++++++++++++++++++++++++++++++++++++++--
>  1 file changed, 50 insertions(+), 2 deletions(-)
> 
> diff --git a/tests/pm_backlight.c b/tests/pm_backlight.c
> index a695f90..6f6d727 100644
> --- a/tests/pm_backlight.c
> +++ b/tests/pm_backlight.c
> @@ -47,6 +47,7 @@ struct context {
>  #define FADESPEED 100 /* milliseconds between steps */
>  
>  IGT_TEST_DESCRIPTION("Basic backlight sysfs test");
> +static int8_t *pm_data = NULL;
>  
>  static int backlight_read(int *result, const char *fname)
>  {
> @@ -150,19 +151,60 @@ static void test_fade(struct context *context)
>  		nanosleep(&ts, NULL);
>  	}
>  }
> +static void test_fade_with_dpms(struct context *context, igt_output_t *output)

nip: what about test_fade_after_dpms_on_off() ?

but with is not wrong or bad, just my poor brain first thought you
would try fade test between off and on, what doesn't make any sense I know...

> +{
> +	int i;
> +	static const struct timespec ts = { .tv_sec = 0, .tv_nsec = FADESPEED*1000000 };
> +	bool has_runtime_pm;
> +	has_runtime_pm = igt_setup_runtime_pm();
> +	igt_info("Runtime PM support: %d\n", has_runtime_pm);
> +	igt_assert(has_runtime_pm);
> +	kmstest_set_connector_dpms(output->display->drm_fd, output->config.connector, DRM_MODE_DPMS_OFF);
> +	igt_assert(igt_wait_for_pm_status(IGT_RUNTIME_PM_STATUS_SUSPENDED));
> +	kmstest_set_connector_dpms(output->display->drm_fd, output->config.connector, DRM_MODE_DPMS_ON);
> +	igt_assert(igt_wait_for_pm_status(IGT_RUNTIME_PM_STATUS_ACTIVE));
> +
> +	/* Fade out, then in */
> +	for (i = context->max; i > 0; i -= context->max / FADESTEPS) {
> +		test_and_verify(context, i);
> +		nanosleep(&ts, NULL);
> +	}
> +	for (i = 0; i <= context->max; i += context->max / FADESTEPS) {
> +		test_and_verify(context, i);
> +		nanosleep(&ts, NULL);
> +	}

I think this function should only include the dpms part and then we call

test_fade(context) directly instead of duplicating the code...

> +}
> +static void test_fade_with_suspend(struct context *context, igt_output_t *output)
> +{
> +	int i;
> +	static const struct timespec ts = { .tv_sec = 0, .tv_nsec = FADESPEED*1000000 };
> +	kmstest_set_connector_dpms(output->display->drm_fd, output->config.connector, DRM_MODE_DPMS_OFF);
> +	igt_assert(igt_wait_for_pm_status(IGT_RUNTIME_PM_STATUS_SUSPENDED));
> +	igt_system_suspend_autoresume(SUSPEND_STATE_MEM, SUSPEND_TEST_NONE);
> +
> +	/* Fade out, then in */
> +	for (i = context->max; i > 0; i -= context->max / FADESTEPS) {
> +		test_and_verify(context, i);
> +		nanosleep(&ts, NULL);
> +	}
> +	for (i = 0; i <= context->max; i += context->max / FADESTEPS) {
> +		test_and_verify(context, i);
> +		nanosleep(&ts, NULL);
> +	}

same here...

besides the code duplication everything else lgtm

Thanks,
Rodrigo.

> +}
>  
>  igt_main
>  {
>  	struct context context = {0};
>  	int old;
>  	igt_display_t display;
> +	igt_output_t *output;
>  	struct igt_fb fb;
>  
>  	igt_skip_on_simulation();
>  
>  	igt_fixture {
>  		enum pipe pipe;
> -		igt_output_t *output;
>  		bool found = false;
>  		char full_name[32] = {};
>  		char *name;
> @@ -187,7 +229,6 @@ igt_main
>  		for_each_pipe_with_valid_output(&display, pipe, output) {
>  			if (strcmp(name + 6, output->name))
>  				continue;
> -
>  			found = true;
>  			break;
>  		}
> @@ -205,6 +246,7 @@ igt_main
>  		igt_plane_set_fb(primary, &fb);
>  
>  		igt_display_commit2(&display, display.is_atomic ? COMMIT_ATOMIC : COMMIT_LEGACY);
> +		pm_data = igt_pm_enable_sata_link_power_management();
>  	}
>  
>  	igt_subtest("basic-brightness")
> @@ -213,6 +255,10 @@ igt_main
>  		test_bad_brightness(&context);
>  	igt_subtest("fade")
>  		test_fade(&context);
> +	igt_subtest("fade_with_dpms")
> +		test_fade_with_dpms(&context, output);
> +	igt_subtest("fade_with_suspend")
> +		test_fade_with_suspend(&context, output);
>  
>  	igt_fixture {
>  		/* Restore old brightness */
> @@ -220,6 +266,8 @@ igt_main
>  
>  		igt_display_fini(&display);
>  		igt_remove_fb(display.drm_fd, &fb);
> +		igt_pm_restore_sata_link_power_management(pm_data);
> +		free(pm_data);
>  		close(display.drm_fd);
>  	}
>  }
> -- 
> 1.9.1
> 
> _______________________________________________
> igt-dev mailing list
> igt-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/igt-dev
diff mbox series

Patch

diff --git a/tests/pm_backlight.c b/tests/pm_backlight.c
index a695f90..6f6d727 100644
--- a/tests/pm_backlight.c
+++ b/tests/pm_backlight.c
@@ -47,6 +47,7 @@  struct context {
 #define FADESPEED 100 /* milliseconds between steps */
 
 IGT_TEST_DESCRIPTION("Basic backlight sysfs test");
+static int8_t *pm_data = NULL;
 
 static int backlight_read(int *result, const char *fname)
 {
@@ -150,19 +151,60 @@  static void test_fade(struct context *context)
 		nanosleep(&ts, NULL);
 	}
 }
+static void test_fade_with_dpms(struct context *context, igt_output_t *output)
+{
+	int i;
+	static const struct timespec ts = { .tv_sec = 0, .tv_nsec = FADESPEED*1000000 };
+	bool has_runtime_pm;
+	has_runtime_pm = igt_setup_runtime_pm();
+	igt_info("Runtime PM support: %d\n", has_runtime_pm);
+	igt_assert(has_runtime_pm);
+	kmstest_set_connector_dpms(output->display->drm_fd, output->config.connector, DRM_MODE_DPMS_OFF);
+	igt_assert(igt_wait_for_pm_status(IGT_RUNTIME_PM_STATUS_SUSPENDED));
+	kmstest_set_connector_dpms(output->display->drm_fd, output->config.connector, DRM_MODE_DPMS_ON);
+	igt_assert(igt_wait_for_pm_status(IGT_RUNTIME_PM_STATUS_ACTIVE));
+
+	/* Fade out, then in */
+	for (i = context->max; i > 0; i -= context->max / FADESTEPS) {
+		test_and_verify(context, i);
+		nanosleep(&ts, NULL);
+	}
+	for (i = 0; i <= context->max; i += context->max / FADESTEPS) {
+		test_and_verify(context, i);
+		nanosleep(&ts, NULL);
+	}
+}
+static void test_fade_with_suspend(struct context *context, igt_output_t *output)
+{
+	int i;
+	static const struct timespec ts = { .tv_sec = 0, .tv_nsec = FADESPEED*1000000 };
+	kmstest_set_connector_dpms(output->display->drm_fd, output->config.connector, DRM_MODE_DPMS_OFF);
+	igt_assert(igt_wait_for_pm_status(IGT_RUNTIME_PM_STATUS_SUSPENDED));
+	igt_system_suspend_autoresume(SUSPEND_STATE_MEM, SUSPEND_TEST_NONE);
+
+	/* Fade out, then in */
+	for (i = context->max; i > 0; i -= context->max / FADESTEPS) {
+		test_and_verify(context, i);
+		nanosleep(&ts, NULL);
+	}
+	for (i = 0; i <= context->max; i += context->max / FADESTEPS) {
+		test_and_verify(context, i);
+		nanosleep(&ts, NULL);
+	}
+}
 
 igt_main
 {
 	struct context context = {0};
 	int old;
 	igt_display_t display;
+	igt_output_t *output;
 	struct igt_fb fb;
 
 	igt_skip_on_simulation();
 
 	igt_fixture {
 		enum pipe pipe;
-		igt_output_t *output;
 		bool found = false;
 		char full_name[32] = {};
 		char *name;
@@ -187,7 +229,6 @@  igt_main
 		for_each_pipe_with_valid_output(&display, pipe, output) {
 			if (strcmp(name + 6, output->name))
 				continue;
-
 			found = true;
 			break;
 		}
@@ -205,6 +246,7 @@  igt_main
 		igt_plane_set_fb(primary, &fb);
 
 		igt_display_commit2(&display, display.is_atomic ? COMMIT_ATOMIC : COMMIT_LEGACY);
+		pm_data = igt_pm_enable_sata_link_power_management();
 	}
 
 	igt_subtest("basic-brightness")
@@ -213,6 +255,10 @@  igt_main
 		test_bad_brightness(&context);
 	igt_subtest("fade")
 		test_fade(&context);
+	igt_subtest("fade_with_dpms")
+		test_fade_with_dpms(&context, output);
+	igt_subtest("fade_with_suspend")
+		test_fade_with_suspend(&context, output);
 
 	igt_fixture {
 		/* Restore old brightness */
@@ -220,6 +266,8 @@  igt_main
 
 		igt_display_fini(&display);
 		igt_remove_fb(display.drm_fd, &fb);
+		igt_pm_restore_sata_link_power_management(pm_data);
+		free(pm_data);
 		close(display.drm_fd);
 	}
 }