On Thu, 2019-04-04 at 10:12 +0200, Daniel Vetter wrote:
> On Wed, Apr 03, 2019 at 06:26:30PM -0700, Harish Chegondi wrote:
> > backlight fade with suspend test turns off dpms which turns off the
> > edp
> > backlight. Then it does a system suspend and resume. After resume,
> > the edp backlight would still be off, but the test sets the
> > brightness
> > value and reads it back. Since the edp backlight is off, the
> > brightness
> > values written and read are different causing the test to fail.
> > 
> > Do not turn off the DPMS before suspend so that after system
> > resume,
> > the edp backlight would be on and setting the brightness value
> > would
> > be successful.
> > 
> > v2: Remove "DPMS off" before system suspend instead of adding
> >     "DPMS on" after system resume.
> > 
> > Cc: Jyoti Yadav <jyoti.r.yadav@intel.com>
> > Cc: Chris Wilson <chris@chris-wilson.co.uk>
> > Cc: Daniel Vetter <daniel.vetter@intel.com>
> > Cc: Clinton Taylor <clinton.a.taylor@intel.com>
> > Signed-off-by: Harish Chegondi <harish.chegondi@intel.com>
> > References: https://bugs.freedesktop.org/show_bug.cgi?id=107820
> > Fixes: 377752242995 ("Brightness test with DPMS and System
> > suspend.")
> 
> I think this makes sense. Still not sure what exactly the test tries
> to
> verify ...
> 
> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> 
Thanks for the review !

I pasted few lines of the commit message from the commit that added
this test which may explain the intention of the test author.

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.

> ---
> >  tests/i915/i915_pm_backlight.c | 7 -------
> >  1 file changed, 7 deletions(-)
> > 
> > diff --git a/tests/i915/i915_pm_backlight.c
> > b/tests/i915/i915_pm_backlight.c
> > index 054300f6..4c1bff5b 100644
> > --- a/tests/i915/i915_pm_backlight.c
> > +++ b/tests/i915/i915_pm_backlight.c
> > @@ -175,13 +175,6 @@ test_fade_with_dpms(struct context *context,
> > igt_output_t *output)
> >  static void
> >  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_SUSPEN
> > DED));
> > -
> >  	igt_system_suspend_autoresume(SUSPEND_STATE_MEM,
> > SUSPEND_TEST_NONE);
> >  
> >  	test_fade(context);
> > -- 
> > 2.20.1
> > 
> > _______________________________________________
> > igt-dev mailing list
> > igt-dev@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/igt-dev