[igt-dev] [RFC i-g-t 1/1] i915/pm_backlight: turn on dpms before backlight fade out and fade in

[igt-dev] [RFC i-g-t 1/1] i915/pm_backlight: turn on dpms before backlight fade out and fade in

[Harish Chegondi]

backlight fade with suspend test turns off dpms which turns off
the edp backlight and panel. Then it does a runtime suspend,
system suspend and resume. After resume, it does a fade out and
fade in of the backlight brightness. From the dmesg logs of the
ci tests it appears that the test is setting the brightness
even before the edp panel and backlight are turned on resuilting
in the brightness values written and read to be different.
Turn on the dpms which turns on the edp panel and backlight
before backlight fade out and fade in. With this change the
fade_with_suspend test passes.

Cc: Jyoti Yadav <jyoti.r.yadav@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
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
---
 tests/i915/i915_pm_backlight.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/tests/i915/i915_pm_backlight.c b/tests/i915/i915_pm_backlight.c
index 054300f6..ca640eca 100644
--- a/tests/i915/i915_pm_backlight.c
+++ b/tests/i915/i915_pm_backlight.c
@@ -184,6 +184,9 @@ test_fade_with_suspend(struct context *context, igt_output_t *output)
 
 	igt_system_suspend_autoresume(SUSPEND_STATE_MEM, SUSPEND_TEST_NONE);
 
+	kmstest_set_connector_dpms(output->display->drm_fd,
+				   output->config.connector,
+				   DRM_MODE_DPMS_ON);
 	test_fade(context);
 }
 
-- 
2.20.1

_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

[igt-dev] ✓ Fi.CI.BAT: success for series starting with [RFC,i-g-t,1/1] i915/pm_backlight: turn on dpms before backlight fade out and fade in

[Patchwork]

== Series Details ==

Series: series starting with [RFC,i-g-t,1/1] i915/pm_backlight: turn on dpms before backlight fade out and fade in
URL   : https://patchwork.freedesktop.org/series/57976/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_5743 -> IGTPW_2617
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  External URL: https://patchwork.freedesktop.org/api/1.0/series/57976/revisions/1/mbox/

Known issues
------------

  Here are the changes found in IGTPW_2617 that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@amdgpu/amd_cs_nop@sync-gfx0:
    - fi-kbl-7567u:       NOTRUN -> SKIP [fdo#109271] +33

  * igt@gem_exec_basic@gtt-bsd:
    - fi-bwr-2160:        NOTRUN -> SKIP [fdo#109271] +103

  * igt@gem_exec_suspend@basic-s3:
    - fi-icl-u2:          PASS -> FAIL [fdo#103375]

  * igt@gem_exec_suspend@basic-s4-devices:
    - fi-icl-u2:          PASS -> DMESG-WARN [fdo#109638]

  * igt@kms_busy@basic-flip-c:
    - fi-bwr-2160:        NOTRUN -> SKIP [fdo#109271] / [fdo#109278]

  * igt@kms_force_connector_basic@force-load-detect:
    - fi-kbl-7560u:       NOTRUN -> SKIP [fdo#109271] +33

  * igt@prime_vgem@basic-fence-flip:
    - fi-gdg-551:         PASS -> FAIL [fdo#103182]

  
#### Possible fixes ####

  * igt@gem_ctx_create@basic-files:
    - fi-kbl-7560u:       INCOMPLETE [fdo#103665] -> PASS

  * igt@kms_frontbuffer_tracking@basic:
    - fi-icl-u3:          FAIL [fdo#103167] -> PASS

  
  [fdo#103167]: https://bugs.freedesktop.org/show_bug.cgi?id=103167
  [fdo#103182]: https://bugs.freedesktop.org/show_bug.cgi?id=103182
  [fdo#103375]: https://bugs.freedesktop.org/show_bug.cgi?id=103375
  [fdo#103665]: https://bugs.freedesktop.org/show_bug.cgi?id=103665
  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [fdo#109278]: https://bugs.freedesktop.org/show_bug.cgi?id=109278
  [fdo#109638]: https://bugs.freedesktop.org/show_bug.cgi?id=109638


Participating hosts (47 -> 35)
------------------------------

  Additional (2): fi-kbl-7567u fi-bwr-2160 
  Missing    (14): fi-kbl-soraka fi-hsw-4770r fi-ilk-m540 fi-hsw-4200u fi-byt-j1900 fi-byt-squawks fi-bsw-cyan fi-hsw-4770 fi-elk-e7500 fi-bsw-kefka fi-skl-lmem fi-blb-e6850 fi-byt-clapper fi-bdw-samus 


Build changes
-------------

    * IGT: IGT_4884 -> IGTPW_2617

  CI_DRM_5743: e167a588f3614da02999797b9816744e39e31ad7 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGTPW_2617: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_2617/
  IGT_4884: c46051337b972f8b5a302afb6f603df06fea527d @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_2617/
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [igt-dev] [RFC i-g-t 1/1] i915/pm_backlight: turn on dpms before backlight fade out and fade in

[Chris Wilson]

Quoting Harish Chegondi (2019-03-14 06:41:48)
> backlight fade with suspend test turns off dpms which turns off
> the edp backlight and panel. Then it does a runtime suspend,
> system suspend and resume. After resume, it does a fade out and
> fade in of the backlight brightness. From the dmesg logs of the
> ci tests it appears that the test is setting the brightness
> even before the edp panel and backlight are turned on resuilting
> in the brightness values written and read to be different.
> Turn on the dpms which turns on the edp panel and backlight
> before backlight fade out and fade in. With this change the
> fade_with_suspend test passes.

But is it legal for the kernel to that? Is the kernel meant to restore
the previous configuration upon resume or leave it to userspace? What
about for fbcon?

If it is meant to leave it to userspace, why is the connector still even
enabled?
-Chris
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

[igt-dev] ✓ Fi.CI.IGT: success for series starting with [RFC,i-g-t,1/1] i915/pm_backlight: turn on dpms before backlight fade out and fade in

[Patchwork]

== Series Details ==

Series: series starting with [RFC,i-g-t,1/1] i915/pm_backlight: turn on dpms before backlight fade out and fade in
URL   : https://patchwork.freedesktop.org/series/57976/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_5743_full -> IGTPW_2617_full
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  External URL: https://patchwork.freedesktop.org/api/1.0/series/57976/revisions/1/mbox/

Known issues
------------

  Here are the changes found in IGTPW_2617_full that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@gem_ctx_param@invalid-param-set:
    - shard-hsw:          NOTRUN -> FAIL [fdo#109674]

  * igt@gem_exec_schedule@preempt-bsd2:
    - shard-apl:          NOTRUN -> SKIP [fdo#109271] +4

  * igt@kms_atomic_transition@4x-modeset-transitions-fencing:
    - shard-kbl:          NOTRUN -> SKIP [fdo#109271] / [fdo#109278] +3

  * igt@kms_busy@extended-modeset-hang-newfb-with-reset-render-b:
    - shard-snb:          PASS -> DMESG-WARN [fdo#107956]

  * igt@kms_busy@extended-pageflip-modeset-hang-oldfb-render-a:
    - shard-glk:          NOTRUN -> DMESG-WARN [fdo#107956]

  * igt@kms_busy@extended-pageflip-modeset-hang-oldfb-render-b:
    - shard-glk:          PASS -> DMESG-WARN [fdo#107956]

  * igt@kms_ccs@pipe-c-crc-primary-basic:
    - shard-hsw:          NOTRUN -> SKIP [fdo#109271] +30

  * igt@kms_color@pipe-a-ctm-max:
    - shard-glk:          NOTRUN -> FAIL [fdo#108147]

  * igt@kms_color@pipe-b-ctm-max:
    - shard-apl:          PASS -> FAIL [fdo#108147]
    - shard-kbl:          PASS -> FAIL [fdo#108147]

  * igt@kms_color@pipe-c-degamma:
    - shard-glk:          NOTRUN -> FAIL [fdo#104782]

  * igt@kms_cursor_crc@cursor-128x128-suspend:
    - shard-glk:          NOTRUN -> FAIL [fdo#103232] +2
    - shard-apl:          PASS -> FAIL [fdo#103191] / [fdo#103232]

  * igt@kms_cursor_crc@cursor-128x42-onscreen:
    - shard-apl:          PASS -> FAIL [fdo#103232] +2

  * igt@kms_cursor_crc@cursor-64x64-suspend:
    - shard-kbl:          PASS -> FAIL [fdo#103191] / [fdo#103232]

  * igt@kms_cursor_legacy@2x-long-nonblocking-modeset-vs-cursor-atomic:
    - shard-glk:          PASS -> FAIL [fdo#105454] / [fdo#106509]

  * igt@kms_cursor_legacy@cursor-vs-flip-toggle:
    - shard-hsw:          PASS -> FAIL [fdo#103355]

  * igt@kms_flip@flip-vs-panning:
    - shard-glk:          PASS -> INCOMPLETE [fdo#103359] / [k.org#198133]

  * igt@kms_frontbuffer_tracking@fbc-1p-primscrn-cur-indfb-draw-mmap-wc:
    - shard-kbl:          PASS -> FAIL [fdo#103167] +1

  * igt@kms_frontbuffer_tracking@fbc-1p-primscrn-cur-indfb-draw-pwrite:
    - shard-apl:          PASS -> FAIL [fdo#103167] +4

  * igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-draw-blt:
    - shard-glk:          NOTRUN -> FAIL [fdo#103167]

  * igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-onoff:
    - shard-glk:          PASS -> FAIL [fdo#103167] +5

  * igt@kms_frontbuffer_tracking@psr-1p-offscren-pri-shrfb-draw-blt:
    - shard-kbl:          NOTRUN -> SKIP [fdo#109271] +12

  * igt@kms_frontbuffer_tracking@psr-rgb101010-draw-pwrite:
    - shard-glk:          NOTRUN -> SKIP [fdo#109271] +57

  * igt@kms_pipe_crc_basic@suspend-read-crc-pipe-d:
    - shard-glk:          NOTRUN -> SKIP [fdo#109271] / [fdo#109278] +5

  * igt@kms_plane_alpha_blend@pipe-a-alpha-opaque-fb:
    - shard-apl:          PASS -> FAIL [fdo#108145]
    - shard-glk:          PASS -> FAIL [fdo#108145]
    - shard-kbl:          PASS -> FAIL [fdo#108145]

  * igt@kms_plane_alpha_blend@pipe-b-constant-alpha-max:
    - shard-kbl:          NOTRUN -> FAIL [fdo#108145]

  * igt@kms_rotation_crc@multiplane-rotation:
    - shard-kbl:          PASS -> FAIL [fdo#109016]

  * igt@kms_rotation_crc@multiplane-rotation-cropping-bottom:
    - shard-kbl:          PASS -> DMESG-FAIL [fdo#105763]

  * igt@kms_setmode@basic:
    - shard-apl:          PASS -> FAIL [fdo#99912]

  * igt@kms_universal_plane@universal-plane-gen9-features-pipe-e:
    - shard-hsw:          NOTRUN -> SKIP [fdo#109271] / [fdo#109278] +4

  * igt@kms_vblank@pipe-c-ts-continuation-suspend:
    - shard-apl:          PASS -> FAIL [fdo#104894] +2

  
#### Possible fixes ####

  * igt@gem_persistent_relocs@forked-thrash-inactive:
    - shard-hsw:          INCOMPLETE [fdo#103540] -> PASS

  * igt@kms_atomic_transition@1x-modeset-transitions-nonblocking-fencing:
    - shard-apl:          FAIL [fdo#109660] -> PASS
    - shard-kbl:          FAIL [fdo#109660] -> PASS

  * igt@kms_busy@extended-modeset-hang-newfb-render-a:
    - shard-hsw:          DMESG-WARN [fdo#107956] -> PASS
    - shard-kbl:          DMESG-WARN [fdo#107956] -> PASS +2
    - shard-snb:          DMESG-WARN [fdo#107956] -> PASS

  * igt@kms_color@pipe-b-legacy-gamma:
    - shard-glk:          FAIL [fdo#104782] -> PASS
    - shard-apl:          FAIL [fdo#104782] -> PASS
    - shard-kbl:          FAIL [fdo#104782] -> PASS

  * igt@kms_cursor_crc@cursor-64x64-sliding:
    - shard-apl:          FAIL [fdo#103232] -> PASS +1
    - shard-kbl:          FAIL [fdo#103232] -> PASS

  * igt@kms_cursor_crc@cursor-alpha-opaque:
    - shard-glk:          FAIL [fdo#109350] -> PASS

  * igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-fullscreen:
    - shard-apl:          FAIL [fdo#103167] -> PASS +1

  * igt@kms_frontbuffer_tracking@fbc-1p-primscrn-spr-indfb-move:
    - shard-glk:          FAIL [fdo#103167] -> PASS +1
    - shard-kbl:          FAIL [fdo#103167] -> PASS +1

  * {igt@kms_plane@pixel-format-pipe-b-planes}:
    - shard-glk:          SKIP [fdo#109271] -> PASS

  * {igt@kms_plane_multiple@atomic-pipe-b-tiling-none}:
    - shard-glk:          FAIL [fdo#110037] -> PASS +2

  * {igt@kms_plane_multiple@atomic-pipe-b-tiling-x}:
    - shard-apl:          FAIL [fdo#110037] -> PASS +1

  * {igt@kms_plane_multiple@atomic-pipe-b-tiling-yf}:
    - shard-kbl:          FAIL [fdo#110037] -> PASS +1

  * igt@kms_rotation_crc@multiplane-rotation-cropping-top:
    - shard-kbl:          FAIL [fdo#109016] -> PASS

  * igt@kms_setmode@basic:
    - shard-kbl:          FAIL [fdo#99912] -> PASS

  * {igt@kms_universal_plane@universal-plane-pipe-b-functional}:
    - shard-apl:          FAIL [fdo#103166] -> PASS +2

  * {igt@kms_universal_plane@universal-plane-pipe-c-functional}:
    - shard-glk:          FAIL [fdo#103166] -> PASS

  * igt@kms_vblank@pipe-a-ts-continuation-suspend:
    - shard-hsw:          FAIL [fdo#104894] -> PASS

  * igt@kms_vblank@pipe-b-ts-continuation-modeset-rpm:
    - shard-apl:          FAIL [fdo#104894] -> PASS +1

  * igt@perf@oa-exponents:
    - shard-kbl:          FAIL [fdo#105483] -> PASS

  
#### Warnings ####

  * igt@kms_plane_scaling@pipe-c-scaler-with-rotation:
    - shard-glk:          FAIL [fdo#110098] -> SKIP [fdo#109271] / [fdo#109278]

  
  {name}: This element is suppressed. This means it is ignored when computing
          the status of the difference (SUCCESS, WARNING, or FAILURE).

  [fdo#103166]: https://bugs.freedesktop.org/show_bug.cgi?id=103166
  [fdo#103167]: https://bugs.freedesktop.org/show_bug.cgi?id=103167
  [fdo#103191]: https://bugs.freedesktop.org/show_bug.cgi?id=103191
  [fdo#103232]: https://bugs.freedesktop.org/show_bug.cgi?id=103232
  [fdo#103355]: https://bugs.freedesktop.org/show_bug.cgi?id=103355
  [fdo#103359]: https://bugs.freedesktop.org/show_bug.cgi?id=103359
  [fdo#103540]: https://bugs.freedesktop.org/show_bug.cgi?id=103540
  [fdo#104782]: https://bugs.freedesktop.org/show_bug.cgi?id=104782
  [fdo#104894]: https://bugs.freedesktop.org/show_bug.cgi?id=104894
  [fdo#105454]: https://bugs.freedesktop.org/show_bug.cgi?id=105454
  [fdo#105483]: https://bugs.freedesktop.org/show_bug.cgi?id=105483
  [fdo#105763]: https://bugs.freedesktop.org/show_bug.cgi?id=105763
  [fdo#106509]: https://bugs.freedesktop.org/show_bug.cgi?id=106509
  [fdo#107956]: https://bugs.freedesktop.org/show_bug.cgi?id=107956
  [fdo#108145]: https://bugs.freedesktop.org/show_bug.cgi?id=108145
  [fdo#108147]: https://bugs.freedesktop.org/show_bug.cgi?id=108147
  [fdo#109016]: https://bugs.freedesktop.org/show_bug.cgi?id=109016
  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [fdo#109278]: https://bugs.freedesktop.org/show_bug.cgi?id=109278
  [fdo#109350]: https://bugs.freedesktop.org/show_bug.cgi?id=109350
  [fdo#109660]: https://bugs.freedesktop.org/show_bug.cgi?id=109660
  [fdo#109674]: https://bugs.freedesktop.org/show_bug.cgi?id=109674
  [fdo#110037]: https://bugs.freedesktop.org/show_bug.cgi?id=110037
  [fdo#110038]: https://bugs.freedesktop.org/show_bug.cgi?id=110038
  [fdo#110098]: https://bugs.freedesktop.org/show_bug.cgi?id=110098
  [fdo#99912]: https://bugs.freedesktop.org/show_bug.cgi?id=99912
  [k.org#198133]: https://bugzilla.kernel.org/show_bug.cgi?id=198133


Participating hosts (10 -> 5)
------------------------------

  Missing    (5): shard-skl pig-hsw-4770r pig-glk-j5005 shard-iclb pig-skl-6260u 


Build changes
-------------

    * IGT: IGT_4884 -> IGTPW_2617
    * Piglit: piglit_4509 -> None

  CI_DRM_5743: e167a588f3614da02999797b9816744e39e31ad7 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGTPW_2617: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_2617/
  IGT_4884: c46051337b972f8b5a302afb6f603df06fea527d @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  piglit_4509: fdc5a4ca11124ab8413c7988896eec4c97336694 @ git://anongit.freedesktop.org/piglit

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_2617/
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [igt-dev] [RFC i-g-t 1/1] i915/pm_backlight: turn on dpms before backlight fade out and fade in

[Chegondi, Harish]

[-- Attachment #1.1: Type: text/plain, Size: 1644 bytes --]

On Thu, 2019-03-14 at 07:46 +0000, Chris Wilson wrote:
> Quoting Harish Chegondi (2019-03-14 06:41:48)
> > backlight fade with suspend test turns off dpms which turns off
> > the edp backlight and panel. Then it does a runtime suspend,
> > system suspend and resume. After resume, it does a fade out and
> > fade in of the backlight brightness. From the dmesg logs of the
> > ci tests it appears that the test is setting the brightness
> > even before the edp panel and backlight are turned on resuilting
> > in the brightness values written and read to be different.
> > Turn on the dpms which turns on the edp panel and backlight
> > before backlight fade out and fade in. With this change the
> > fade_with_suspend test passes.

Chris,

My commit message was confusing. I will redo the commit message in the
next version of my patch.

> 
> But is it legal for the kernel to that? Is the kernel meant to
> restore
> the previous configuration upon resume or leave it to userspace? What
> about for fbcon?

Yes, the kernel is meant to restore the previous configuration upon
resume. In this test, before system suspend, DPMS is turned off which
sets the crtc state to inactive and edp backlight to off. After system
resume, the crtc state should still be inactive and dpms and edp
backlight should still be off. After system resume, before setting the
backlight brighness, the test should turn on the DPMS. Turning on the
DPMS would set the CRTC state to active and turn on the backlight.

-Harish.

> 
> If it is meant to leave it to userspace, why is the connector still
> even
> enabled?
> -Chris



[-- Attachment #1.2: smime.p7s --]
[-- Type: application/x-pkcs7-signature, Size: 3274 bytes --]

[-- Attachment #2: Type: text/plain, Size: 153 bytes --]

_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [igt-dev] [RFC i-g-t 1/1] i915/pm_backlight: turn on dpms before backlight fade out and fade in

[Chris Wilson]

Quoting Chegondi, Harish (2019-04-03 01:47:09)
> On Thu, 2019-03-14 at 07:46 +0000, Chris Wilson wrote:
> > Quoting Harish Chegondi (2019-03-14 06:41:48)
> > > backlight fade with suspend test turns off dpms which turns off
> > > the edp backlight and panel. Then it does a runtime suspend,
> > > system suspend and resume. After resume, it does a fade out and
> > > fade in of the backlight brightness. From the dmesg logs of the
> > > ci tests it appears that the test is setting the brightness
> > > even before the edp panel and backlight are turned on resuilting
> > > in the brightness values written and read to be different.
> > > Turn on the dpms which turns on the edp panel and backlight
> > > before backlight fade out and fade in. With this change the
> > > fade_with_suspend test passes.
> 
> Chris,
> 
> My commit message was confusing. I will redo the commit message in the
> next version of my patch.
> 
> > 
> > But is it legal for the kernel to that? Is the kernel meant to
> > restore
> > the previous configuration upon resume or leave it to userspace? What
> > about for fbcon?
> 
> Yes, the kernel is meant to restore the previous configuration upon
> resume.

Are you sure?

We send a hotplug to userspace to tell them to restore the configuration
as they see fit (because the configuration will often change). If fbcon
is active, it restores the fbcon screen which can just be the consequence
of handling its hotplug event.
-Chris
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [igt-dev] [RFC i-g-t 1/1] i915/pm_backlight: turn on dpms before backlight fade out and fade in

[Daniel Vetter]

On Wed, Apr 03, 2019 at 08:44:57AM +0100, Chris Wilson wrote:
> Quoting Chegondi, Harish (2019-04-03 01:47:09)
> > On Thu, 2019-03-14 at 07:46 +0000, Chris Wilson wrote:
> > > Quoting Harish Chegondi (2019-03-14 06:41:48)
> > > > backlight fade with suspend test turns off dpms which turns off
> > > > the edp backlight and panel. Then it does a runtime suspend,
> > > > system suspend and resume. After resume, it does a fade out and
> > > > fade in of the backlight brightness. From the dmesg logs of the
> > > > ci tests it appears that the test is setting the brightness
> > > > even before the edp panel and backlight are turned on resuilting
> > > > in the brightness values written and read to be different.
> > > > Turn on the dpms which turns on the edp panel and backlight
> > > > before backlight fade out and fade in. With this change the
> > > > fade_with_suspend test passes.
> > 
> > Chris,
> > 
> > My commit message was confusing. I will redo the commit message in the
> > next version of my patch.
> > 
> > > 
> > > But is it legal for the kernel to that? Is the kernel meant to
> > > restore
> > > the previous configuration upon resume or leave it to userspace? What
> > > about for fbcon?
> > 
> > Yes, the kernel is meant to restore the previous configuration upon
> > resume.
> 
> Are you sure?
> 
> We send a hotplug to userspace to tell them to restore the configuration
> as they see fit (because the configuration will often change). If fbcon
> is active, it restores the fbcon screen which can just be the consequence
> of handling its hotplug event.

Bunch of comments from irc discussion:
- kernel is supposed to restore
- but since the test does a dpms off, we do need to do a dpms on
- but that's silly, because defacto that means we test nothing

Worse:
- the test doesn't shut up fbcon, so uncontrolled env
- it also doesn't set up a mode explicitly, so again uncontrolled env

Rodrigo shouldn't have r-b'ed this imo. There's a lot more work needed
here than duct-taping a few more commands on top, I think starting with
a) what exactly are we trying to test here (testing fancy effects isn't
useful in the context of CI, no one is looking)
b) have we set up the right starting conditions to actually run that test

Cheers, Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [igt-dev] [RFC i-g-t 1/1] i915/pm_backlight: turn on dpms before backlight fade out and fade in

[Daniel Vetter]

On Wed, Apr 03, 2019 at 04:34:10PM +0200, Daniel Vetter wrote:
> On Wed, Apr 03, 2019 at 08:44:57AM +0100, Chris Wilson wrote:
> > Quoting Chegondi, Harish (2019-04-03 01:47:09)
> > > On Thu, 2019-03-14 at 07:46 +0000, Chris Wilson wrote:
> > > > Quoting Harish Chegondi (2019-03-14 06:41:48)
> > > > > backlight fade with suspend test turns off dpms which turns off
> > > > > the edp backlight and panel. Then it does a runtime suspend,
> > > > > system suspend and resume. After resume, it does a fade out and
> > > > > fade in of the backlight brightness. From the dmesg logs of the
> > > > > ci tests it appears that the test is setting the brightness
> > > > > even before the edp panel and backlight are turned on resuilting
> > > > > in the brightness values written and read to be different.
> > > > > Turn on the dpms which turns on the edp panel and backlight
> > > > > before backlight fade out and fade in. With this change the
> > > > > fade_with_suspend test passes.
> > > 
> > > Chris,
> > > 
> > > My commit message was confusing. I will redo the commit message in the
> > > next version of my patch.
> > > 
> > > > 
> > > > But is it legal for the kernel to that? Is the kernel meant to
> > > > restore
> > > > the previous configuration upon resume or leave it to userspace? What
> > > > about for fbcon?
> > > 
> > > Yes, the kernel is meant to restore the previous configuration upon
> > > resume.
> > 
> > Are you sure?
> > 
> > We send a hotplug to userspace to tell them to restore the configuration
> > as they see fit (because the configuration will often change). If fbcon
> > is active, it restores the fbcon screen which can just be the consequence
> > of handling its hotplug event.
> 
> Bunch of comments from irc discussion:
> - kernel is supposed to restore
> - but since the test does a dpms off, we do need to do a dpms on
> - but that's silly, because defacto that means we test nothing
> 
> Worse:
> - the test doesn't shut up fbcon, so uncontrolled env
> - it also doesn't set up a mode explicitly, so again uncontrolled env

Correction: I was blind, it's all there in the igt_fixture. Apologies.

> Rodrigo shouldn't have r-b'ed this imo. There's a lot more work needed
> here than duct-taping a few more commands on top, I think starting with
> a) what exactly are we trying to test here (testing fancy effects isn't
> useful in the context of CI, no one is looking)
> b) have we set up the right starting conditions to actually run that test

For the test itself I think we should instead drop the dpms off. There's
not much to test really if we suspend with the display off. Or at least we
should check both ways I think, but suspending with display on is a lot
more interesting. And for that case the kernel should resume the display
for us again, not explicit dpms on needed.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [igt-dev] [RFC i-g-t 1/1] i915/pm_backlight: turn on dpms before backlight fade out and fade in

[Chegondi, Harish]

[-- Attachment #1.1: Type: text/plain, Size: 3599 bytes --]

On Wed, 2019-04-03 at 18:10 +0200, Daniel Vetter wrote:
> On Wed, Apr 03, 2019 at 04:34:10PM +0200, Daniel Vetter wrote:
> > On Wed, Apr 03, 2019 at 08:44:57AM +0100, Chris Wilson wrote:
> > > Quoting Chegondi, Harish (2019-04-03 01:47:09)
> > > > On Thu, 2019-03-14 at 07:46 +0000, Chris Wilson wrote:
> > > > > Quoting Harish Chegondi (2019-03-14 06:41:48)
> > > > > > backlight fade with suspend test turns off dpms which turns
> > > > > > off
> > > > > > the edp backlight and panel. Then it does a runtime
> > > > > > suspend,
> > > > > > system suspend and resume. After resume, it does a fade out
> > > > > > and
> > > > > > fade in of the backlight brightness. From the dmesg logs of
> > > > > > the
> > > > > > ci tests it appears that the test is setting the brightness
> > > > > > even before the edp panel and backlight are turned on
> > > > > > resuilting
> > > > > > in the brightness values written and read to be different.
> > > > > > Turn on the dpms which turns on the edp panel and backlight
> > > > > > before backlight fade out and fade in. With this change the
> > > > > > fade_with_suspend test passes.
> > > > 
> > > > Chris,
> > > > 
> > > > My commit message was confusing. I will redo the commit message
> > > > in the
> > > > next version of my patch.
> > > > 
> > > > > But is it legal for the kernel to that? Is the kernel meant
> > > > > to
> > > > > restore
> > > > > the previous configuration upon resume or leave it to
> > > > > userspace? What
> > > > > about for fbcon?
> > > > 
> > > > Yes, the kernel is meant to restore the previous configuration
> > > > upon
> > > > resume.
> > > 
> > > Are you sure?
> > > 
> > > We send a hotplug to userspace to tell them to restore the
> > > configuration
> > > as they see fit (because the configuration will often change). If
> > > fbcon
> > > is active, it restores the fbcon screen which can just be the
> > > consequence
> > > of handling its hotplug event.
> > 
> > Bunch of comments from irc discussion:
> > - kernel is supposed to restore
> > - but since the test does a dpms off, we do need to do a dpms on
> > - but that's silly, because defacto that means we test nothing
> > 
> > Worse:
> > - the test doesn't shut up fbcon, so uncontrolled env
> > - it also doesn't set up a mode explicitly, so again uncontrolled
> > env
> 
> Correction: I was blind, it's all there in the igt_fixture.
> Apologies.
> 
> > Rodrigo shouldn't have r-b'ed this imo. There's a lot more work
> > needed
> > here than duct-taping a few more commands on top, I think starting
> > with
> > a) what exactly are we trying to test here (testing fancy effects
> > isn't
> > useful in the context of CI, no one is looking)
> > b) have we set up the right starting conditions to actually run
> > that test
> 
> For the test itself I think we should instead drop the dpms off.
> There's
> not much to test really if we suspend with the display off. Or at
> least we
> should check both ways I think, but suspending with display on is a
> lot
> more interesting. And for that case the kernel should resume the
> display
> for us again, not explicit dpms on needed.
> -Daniel

During my investigation, I already tried removing DPMS off and verified
that the system suspend is turning off the backlight and the system
resume is properly turning on the backlight. I captured the test
changes and the observation in the fdo:

https://bugs.freedesktop.org/show_bug.cgi?id=107820#c6

I will send out this patch.

Thanks for your feedback.

-Harish.

[-- Attachment #1.2: smime.p7s --]
[-- Type: application/x-pkcs7-signature, Size: 3274 bytes --]

[-- Attachment #2: Type: text/plain, Size: 153 bytes --]

_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

[igt-dev] [PATCH v2 1/1] i915/pm_backlight: Do not turn off DPMS before system suspend

[Harish Chegondi]

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.")
---
 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_SUSPENDED));
-
 	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

[igt-dev] ✓ Fi.CI.BAT: success for series starting with [v2,1/1] i915/pm_backlight: Do not turn off DPMS before system suspend (rev2)

[Patchwork]

== Series Details ==

Series: series starting with [v2,1/1] i915/pm_backlight: Do not turn off DPMS before system suspend (rev2)
URL   : https://patchwork.freedesktop.org/series/57976/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_5869 -> IGTPW_2782
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  External URL: https://patchwork.freedesktop.org/api/1.0/series/57976/revisions/2/mbox/

Known issues
------------

  Here are the changes found in IGTPW_2782 that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@gem_exec_basic@gtt-bsd2:
    - fi-byt-clapper:     NOTRUN -> SKIP [fdo#109271] +57

  * igt@i915_selftest@live_contexts:
    - fi-bdw-gvtdvm:      PASS -> DMESG-FAIL [fdo#110235 ]

  * igt@i915_selftest@live_uncore:
    - fi-ivb-3770:        PASS -> DMESG-FAIL [fdo#110210]

  * igt@kms_busy@basic-flip-a:
    - fi-bsw-n3050:       NOTRUN -> SKIP [fdo#109271] / [fdo#109278] +1

  * igt@kms_busy@basic-flip-c:
    - fi-byt-clapper:     NOTRUN -> SKIP [fdo#109271] / [fdo#109278]

  * igt@kms_chamelium@hdmi-crc-fast:
    - fi-bsw-n3050:       NOTRUN -> SKIP [fdo#109271] +62

  * igt@kms_frontbuffer_tracking@basic:
    - fi-byt-clapper:     NOTRUN -> FAIL [fdo#103167]

  * igt@kms_psr@primary_mmap_gtt:
    - fi-blb-e6850:       NOTRUN -> SKIP [fdo#109271] +27

  * igt@runner@aborted:
    - fi-bxt-dsi:         NOTRUN -> FAIL [fdo#109516]

  
#### Possible fixes ####

  * igt@kms_pipe_crc_basic@suspend-read-crc-pipe-b:
    - fi-blb-e6850:       INCOMPLETE [fdo#107718] -> PASS

  * igt@prime_vgem@basic-fence-flip:
    - fi-ilk-650:         FAIL [fdo#104008] -> PASS

  
  [fdo#103167]: https://bugs.freedesktop.org/show_bug.cgi?id=103167
  [fdo#104008]: https://bugs.freedesktop.org/show_bug.cgi?id=104008
  [fdo#107718]: https://bugs.freedesktop.org/show_bug.cgi?id=107718
  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [fdo#109278]: https://bugs.freedesktop.org/show_bug.cgi?id=109278
  [fdo#109516]: https://bugs.freedesktop.org/show_bug.cgi?id=109516
  [fdo#110210]: https://bugs.freedesktop.org/show_bug.cgi?id=110210
  [fdo#110235 ]: https://bugs.freedesktop.org/show_bug.cgi?id=110235 


Participating hosts (47 -> 41)
------------------------------

  Additional (3): fi-bxt-dsi fi-byt-clapper fi-bsw-n3050 
  Missing    (9): fi-ilk-m540 fi-hsw-4200u fi-hsw-peppy fi-byt-squawks fi-bsw-cyan fi-ctg-p8600 fi-pnv-d510 fi-icl-y fi-bdw-samus 


Build changes
-------------

    * IGT: IGT_4926 -> IGTPW_2782

  CI_DRM_5869: 03f8f3298b90c7f80da6a98c3eb8413d7aeaa52b @ git://anongit.freedesktop.org/gfx-ci/linux
  IGTPW_2782: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_2782/
  IGT_4926: c9a9cf357b6b2a304623790bf8dae797e12888a8 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_2782/
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [igt-dev] [PATCH v2 1/1] i915/pm_backlight: Do not turn off DPMS before system suspend

[Daniel Vetter]

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>

> ---
>  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_SUSPENDED));
> -
>  	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

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

[igt-dev] ✓ Fi.CI.IGT: success for series starting with [v2,1/1] i915/pm_backlight: Do not turn off DPMS before system suspend (rev2)

[Patchwork]

== Series Details ==

Series: series starting with [v2,1/1] i915/pm_backlight: Do not turn off DPMS before system suspend (rev2)
URL   : https://patchwork.freedesktop.org/series/57976/
State : success

== Summary ==

CI Bug Log - changes from CI_DRM_5869_full -> IGTPW_2782_full
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  External URL: https://patchwork.freedesktop.org/api/1.0/series/57976/revisions/2/mbox/

Known issues
------------

  Here are the changes found in IGTPW_2782_full that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@gem_create@create-clear:
    - shard-snb:          NOTRUN -> INCOMPLETE [fdo#105411]

  * igt@gem_exec_schedule@preempt-other-chain-blt:
    - shard-snb:          NOTRUN -> SKIP [fdo#109271] +114

  * igt@kms_atomic_transition@plane-all-modeset-transition-fencing:
    - shard-apl:          PASS -> INCOMPLETE [fdo#103927]

  * igt@kms_busy@extended-modeset-hang-newfb-render-a:
    - shard-kbl:          PASS -> DMESG-WARN [fdo#110222]

  * igt@kms_busy@extended-modeset-hang-newfb-render-f:
    - shard-snb:          NOTRUN -> SKIP [fdo#109271] / [fdo#109278] +14

  * igt@kms_busy@extended-modeset-hang-newfb-with-reset-render-a:
    - shard-snb:          NOTRUN -> DMESG-WARN [fdo#110222]

  * igt@kms_busy@extended-pageflip-hang-newfb-render-f:
    - shard-apl:          NOTRUN -> SKIP [fdo#109271] / [fdo#109278]

  * igt@kms_busy@extended-pageflip-modeset-hang-oldfb-render-b:
    - shard-kbl:          NOTRUN -> DMESG-WARN [fdo#110222]

  * igt@kms_content_protection@legacy:
    - shard-kbl:          NOTRUN -> FAIL [fdo#108739] / [fdo#110321]

  * igt@kms_cursor_crc@cursor-128x128-onscreen:
    - shard-kbl:          PASS -> FAIL [fdo#103232]
    - shard-apl:          PASS -> FAIL [fdo#103232]

  * igt@kms_cursor_legacy@2x-long-flip-vs-cursor-atomic:
    - shard-glk:          PASS -> FAIL [fdo#104873]

  * igt@kms_flip@flip-vs-suspend-interruptible:
    - shard-hsw:          PASS -> INCOMPLETE [fdo#103540]

  * igt@kms_frontbuffer_tracking@psr-2p-pri-indfb-multidraw:
    - shard-apl:          NOTRUN -> SKIP [fdo#109271] +12

  * igt@kms_frontbuffer_tracking@psr-2p-primscrn-shrfb-pgflip-blt:
    - shard-kbl:          NOTRUN -> SKIP [fdo#109271] +8

  * igt@kms_frontbuffer_tracking@psr-2p-scndscrn-spr-indfb-onoff:
    - shard-glk:          NOTRUN -> SKIP [fdo#109271] +4

  * igt@kms_pipe_crc_basic@read-crc-pipe-d:
    - shard-kbl:          NOTRUN -> SKIP [fdo#109271] / [fdo#109278]

  * igt@kms_plane_scaling@pipe-c-scaler-with-rotation:
    - shard-glk:          PASS -> SKIP [fdo#109271] / [fdo#109278]

  * igt@kms_rotation_crc@multiplane-rotation:
    - shard-kbl:          PASS -> INCOMPLETE [fdo#103665]

  * igt@kms_setmode@basic:
    - shard-apl:          PASS -> FAIL [fdo#99912]

  * igt@tools_test@tools_test:
    - shard-glk:          PASS -> SKIP [fdo#109271]

  
#### Possible fixes ####

  * igt@gem_eio@in-flight-suspend:
    - shard-kbl:          INCOMPLETE [fdo#103665] -> PASS

  * igt@kms_busy@extended-modeset-hang-newfb-render-b:
    - shard-kbl:          DMESG-WARN [fdo#110222] -> PASS +2
    - shard-snb:          DMESG-WARN [fdo#110222] -> PASS

  * igt@kms_cursor_crc@cursor-64x64-sliding:
    - shard-glk:          FAIL [fdo#103232] -> PASS +1

  * igt@kms_flip@dpms-vs-vblank-race:
    - shard-glk:          FAIL [fdo#103060] -> PASS

  * igt@kms_vblank@pipe-c-ts-continuation-modeset-hang:
    - shard-apl:          FAIL [fdo#104894] -> PASS

  
  [fdo#103060]: https://bugs.freedesktop.org/show_bug.cgi?id=103060
  [fdo#103232]: https://bugs.freedesktop.org/show_bug.cgi?id=103232
  [fdo#103540]: https://bugs.freedesktop.org/show_bug.cgi?id=103540
  [fdo#103665]: https://bugs.freedesktop.org/show_bug.cgi?id=103665
  [fdo#103927]: https://bugs.freedesktop.org/show_bug.cgi?id=103927
  [fdo#104873]: https://bugs.freedesktop.org/show_bug.cgi?id=104873
  [fdo#104894]: https://bugs.freedesktop.org/show_bug.cgi?id=104894
  [fdo#105411]: https://bugs.freedesktop.org/show_bug.cgi?id=105411
  [fdo#108739]: https://bugs.freedesktop.org/show_bug.cgi?id=108739
  [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
  [fdo#109278]: https://bugs.freedesktop.org/show_bug.cgi?id=109278
  [fdo#110222]: https://bugs.freedesktop.org/show_bug.cgi?id=110222
  [fdo#110321]: https://bugs.freedesktop.org/show_bug.cgi?id=110321
  [fdo#99912]: https://bugs.freedesktop.org/show_bug.cgi?id=99912


Participating hosts (10 -> 5)
------------------------------

  Missing    (5): shard-skl pig-hsw-4770r pig-glk-j5005 shard-iclb pig-skl-6260u 


Build changes
-------------

    * IGT: IGT_4926 -> IGTPW_2782
    * Piglit: piglit_4509 -> None

  CI_DRM_5869: 03f8f3298b90c7f80da6a98c3eb8413d7aeaa52b @ git://anongit.freedesktop.org/gfx-ci/linux
  IGTPW_2782: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_2782/
  IGT_4926: c9a9cf357b6b2a304623790bf8dae797e12888a8 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  piglit_4509: fdc5a4ca11124ab8413c7988896eec4c97336694 @ git://anongit.freedesktop.org/piglit

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_2782/
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [igt-dev] [PATCH v2 1/1] i915/pm_backlight: Do not turn off DPMS before system suspend

[Souza, Jose]

[-- Attachment #1.1: Type: text/plain, Size: 2034 bytes --]

On Wed, 2019-04-03 at 18:26 -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.

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

And going to push it.

> 
> 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.")
> ---
>  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);

[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[-- Attachment #2: Type: text/plain, Size: 153 bytes --]

_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

Re: [igt-dev] [PATCH v2 1/1] i915/pm_backlight: Do not turn off DPMS before system suspend

[Chegondi, Harish]

[-- Attachment #1.1: Type: text/plain, Size: 2864 bytes --]

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

[-- Attachment #1.2: smime.p7s --]
[-- Type: application/x-pkcs7-signature, Size: 3274 bytes --]

[-- Attachment #2: Type: text/plain, Size: 153 bytes --]

_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev