Bug 107820

https://intel-gfx-ci.01.org/tree/drm-tip/drmtip_110/fi-kbl-7560u/igt@pm_backlight@fade.html

This has not seen lately, resolve?

Still around, needs to be debugged.

Analyzed the sequence of PM and backlight events in the dmesg log. From the dmesg log, it appears that the test is setting and reading the backlight brightness value even before the edp panel is turned on after system resume.

I made few changes to the test and ran some experimental tests. For example, with the following change mentioned below, the backlight brightness is set and read after the edp panel was turned on resulting in a successful test. It appears that adding some delay after system resume might suffice allowing the edp panel to be turned on before changing the brightness values. I need to find out what might be a good amount of delay to add after system resume. Or, if there is way to find if the edp panel is on or off, it can be used to make sure the edp panel is on before setting/reading the backlight brightness value.

diff --git a/tests/i915/i915_pm_backlight.c b/tests/i915/i915_pm_backlight.c
 index 054300f6..094ac3fa 100644
— a/tests/i915/i915_pm_backlight.c
 +++ b/tests/i915/i915_pm_backlight.c
 @@ -183,7 +183,9 @@ 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);
 -
 +       kmstest_set_connector_dpms(output->display->drm_fd,
 +                                  output->config.connector,
 +                                  DRM_MODE_DPMS_ON);
         test_fade(context);
  }

Posted an RFC with the changes in comment #4 :
https://patchwork.freedesktop.org/patch/292077/

In order to get some more information to respond to Chris Wilson's comments to the RFC patch, I made some more changes mentioned below to the test and ran the test. The test passed with these changes. From the experimental test I conclude that the system suspend is turning off the backlight and the system resume is properly turning on the backlight. But when the DPMS is turned off before the system suspend and resume, the DPMS off is turning off something that's preventing the system resume from turning on the backlight. Now I am investigating why turning off the DPMS is preventing the system resume from turning on the backlight.

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

I turned on the debug messages in DRM and i915 and ran the test with the changes mentioned in comment #6 and without those changes. I have been comparing the debug messages during the system resume step for both the cases. One difference I found out was if the crtc was active/inactive. When I ran the test without any changes, the crtc state was inactive during system resume. When I ran the test with the changes in comment #6, the crtc state was active during system resume.

I am trying to find if turning off the DPMS is setting the crtc state to inactive and that is causing the system resume step to not turn on the backlight.


With the changes in comment #6:
-------------------------------
[ 4341.507980] [drm:drm_atomic_helper_check_modeset [drm_kms_helper]] [CRTC:47:pipe A] needs all connectors, enable: y, active: y


Test without any changes:
-------------------------
[ 4341.507980] [drm:drm_atomic_helper_check_modeset [drm_kms_helper]] [CRTC:47:pipe A] needs all connectors, enable: y, active: n

Below are comments from the definition of "struct drm_crtc_state" which explain what active/inactive crtc states mean. If the CRTC state is inactive it implies CRTC is not actively displaying in which case it doesn't make sense to turn on the backlight. CRTC state should be active in order to turn on the backlight. The comments also imply if the user space turns off the DPMS, it has to be the user space that should turn on the DPMS and not the kernel.

/**
 * @active: Whether the CRTC is actively displaying (used for DPMS).
 * Implies that @enable is set. The driver must not release any shared
 * resources if @active is set to false but @enable still true, because e
 * userspace expects that a DPMS ON always succeeds.
 *
 * Hence drivers must not consult @active in their various
 * &drm_mode_config_funcs.atomic_check callback to reject an atomic
 * commit. They can consult it to aid in the computation of derived
 * hardware state, since even in the DPMS OFF state the display hardware
 * should be as much powered down as when the CRTC is completely
 * disabled through setting @enable to false.
 */
bool active;

https://patchwork.freedesktop.org/patch/296202/

The following commit has been pushed into the master branch of igt-gpu-tools


commit 8770e24fe4008404da769c16b7edac6142225ad7 (HEAD -> master)
Author: Harish Chegondi <harish.chegondi@intel.com>
Date:   Wed Apr 3 18:26:30 2019 -0700

    i915/pm_backlight: Do not turn off DPMS before system suspend

    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.")
    Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
    Reviewed-by: José Roberto de Souza <jose.souza@intel.com>

https://intel-gfx-ci.01.org/tree/drm-tip/IGT_4929/shard-iclb6/igt@i915_pm_backlight@fade_with_suspend.html