[igt-dev] [PATCH i-g-t 1/2] [CI] tests/pm_backlight.c: Fix brightness failure with system suspend

[Jani Nikula]

On Thu, 18 Oct 2018, Jani Nikula <jani.nikula at linux.intel.com> wrote:
> On Wed, 17 Oct 2018, Daniel Vetter <daniel.vetter at ffwll.ch> wrote:
>> On Wed, Oct 17, 2018 at 11:44 AM Chris Wilson <chris at chris-wilson.co.uk> wrote:
>>>
>>> Quoting Jyoti Yadav (2018-10-17 10:13:20)
>>> > This patch tries to fix the faliure occuring during subtest
>>> > "fade_with_suspend". Failure is occuring because, brightness
>>> > value is being checked before driver initializes fully after resume.
>>> >
>>> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=107847
>>> > Signed-off-by: Jyoti Yadav <jyoti.r.yadav at intel.com>
>>> > ---
>>> >  tests/pm_backlight.c | 1 +
>>> >  1 file changed, 1 insertion(+)
>>> >
>>> > diff --git a/tests/pm_backlight.c b/tests/pm_backlight.c
>>> > index 32808cd..ec047b3 100644
>>> > --- a/tests/pm_backlight.c
>>> > +++ b/tests/pm_backlight.c
>>> > @@ -183,6 +183,7 @@ test_fade_with_suspend(struct context *context, igt_output_t *output)
>>> >         igt_require(igt_wait_for_pm_status(IGT_RUNTIME_PM_STATUS_SUSPENDED));
>>> >
>>> >         igt_system_suspend_autoresume(SUSPEND_STATE_MEM, SUSPEND_TEST_NONE);
>>> > +       igt_require(igt_wait_for_pm_status(IGT_RUNTIME_PM_STATUS_ACTIVE));
>>>
>>> Why would the device be active on system resume if it was asleep
>>> beforehand? In fact, that sounds like the opposite of what we would like
>>> ;)
>>
>> This also looks like papering over a kernel bug I think. Or a test
>> bug. Doing an igt_require after we've done a rather expensive
>> operation already (suspend/resume cycle) is definitely not correct -
>> we should know what should work, not dynamically skip the test if it
>> turns out it didn't work.
>
> It's a test bug.
>
> How about just
>
> diff --git a/tests/pm_backlight.c b/tests/pm_backlight.c
> index 32808cdf6ca4..702ee046d5cb 100644
> --- a/tests/pm_backlight.c
> +++ b/tests/pm_backlight.c
> @@ -177,9 +177,6 @@ test_fade_with_suspend(struct context *context, igt_output_t *output)
>  {
>  	igt_require(igt_setup_runtime_pm());
>  
> -	kmstest_set_connector_dpms(output->display->drm_fd,
> -				   output->config.connector,
> -				   DRM_MODE_DPMS_OFF);
> -  	igt_require(igt_wait_for_pm_status(IGT_RUNTIME_PM_STATUS_SUSPENDED));
>  
>  	igt_system_suspend_autoresume(SUSPEND_STATE_MEM, SUSPEND_TEST_NONE);

I sneaked in another "-" line there ;)

BR,
Jani.

-- 
Jani Nikula, Intel Open Source Graphics Center