diff mbox series

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

Message ID 1535690345-408-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. 31, 2018, 4:39 a.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.

v2 : Optimize the code to avoid code redundancy. (Rodrigo)

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

Comments

Rodrigo Vivi Aug. 31, 2018, 2:09 p.m. UTC | #1 
On Fri, Aug 31, 2018 at 12:39:05AM -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.
> 
> v2 : Optimize the code to avoid code redundancy. (Rodrigo)
> 
> Signed-off-by: Jyoti Yadav <jyoti.r.yadav@intel.com>

Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>

and pushed...

Thanks,
Rodrigo.

> ---
>  tests/pm_backlight.c | 30 ++++++++++++++++++++++++++++--
>  1 file changed, 28 insertions(+), 2 deletions(-)
> 
> diff --git a/tests/pm_backlight.c b/tests/pm_backlight.c
> index a695f90..8b5c79d 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,38 @@ static void test_fade(struct context *context)
>  		nanosleep(&ts, NULL);
>  	}
>  }
> +static void test_fade_with_dpms(struct context *context, igt_output_t *output)
> +{
> +	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));
> +	test_fade(context);
> +}
> +static void test_fade_with_suspend(struct context *context, igt_output_t *output)
> +{
> +	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);
> +	test_fade(context);
> +}
>  
>  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 +207,6 @@ igt_main
>  		for_each_pipe_with_valid_output(&display, pipe, output) {
>  			if (strcmp(name + 6, output->name))
>  				continue;
> -
>  			found = true;
>  			break;
>  		}
> @@ -205,6 +224,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 +233,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 +244,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
Jani Nikula Sept. 28, 2018, 1:37 p.m. UTC | #2 
On Fri, 31 Aug 2018, Rodrigo Vivi <rodrigo.vivi@intel.com> wrote:
> On Fri, Aug 31, 2018 at 12:39:05AM -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.
>> 
>> v2 : Optimize the code to avoid code redundancy. (Rodrigo)
>> 
>> Signed-off-by: Jyoti Yadav <jyoti.r.yadav@intel.com>
>
> Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
>
> and pushed...

Jyoti, please sign up on [1] so we can involve you in bug reports.

This patch, or commit 377752242995 ("tests/pm_backlight.c : Brightness
test with DPMS and System suspend."), appears to cause bug [2]. The
suspend test goes on to use the sysfs interface before the backlight has
been re-enabled after resume, failing the test.

BR,
Jani.

[1] https://bugs.freedesktop.org

[2] https://bugs.freedesktop.org/show_bug.cgi?id=107847


>
> Thanks,
> Rodrigo.
>
>> ---
>>  tests/pm_backlight.c | 30 ++++++++++++++++++++++++++++--
>>  1 file changed, 28 insertions(+), 2 deletions(-)
>> 
>> diff --git a/tests/pm_backlight.c b/tests/pm_backlight.c
>> index a695f90..8b5c79d 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,38 @@ static void test_fade(struct context *context)
>>  		nanosleep(&ts, NULL);
>>  	}
>>  }
>> +static void test_fade_with_dpms(struct context *context, igt_output_t *output)
>> +{
>> +	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));
>> +	test_fade(context);
>> +}
>> +static void test_fade_with_suspend(struct context *context, igt_output_t *output)
>> +{
>> +	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);
>> +	test_fade(context);
>> +}
>>  
>>  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 +207,6 @@ igt_main
>>  		for_each_pipe_with_valid_output(&display, pipe, output) {
>>  			if (strcmp(name + 6, output->name))
>>  				continue;
>> -
>>  			found = true;
>>  			break;
>>  		}
>> @@ -205,6 +224,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 +233,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 +244,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
> _______________________________________________
> 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..8b5c79d 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,38 @@  static void test_fade(struct context *context)
 		nanosleep(&ts, NULL);
 	}
 }
+static void test_fade_with_dpms(struct context *context, igt_output_t *output)
+{
+	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));
+	test_fade(context);
+}
+static void test_fade_with_suspend(struct context *context, igt_output_t *output)
+{
+	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);
+	test_fade(context);
+}
 
 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 +207,6 @@  igt_main
 		for_each_pipe_with_valid_output(&display, pipe, output) {
 			if (strcmp(name + 6, output->name))
 				continue;
-
 			found = true;
 			break;
 		}
@@ -205,6 +224,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 +233,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 +244,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);
 	}
 }