Bug 71631 - [Bisected]igt/pc8 fails
Summary: [Bisected]igt/pc8 fails
Status: CLOSED FIXED
Alias: None
Product: DRI
Classification: Unclassified
Component: DRM/Intel (show other bugs)
Version: unspecified
Hardware: All Linux (All)
: high major
Assignee: Paulo Zanoni
QA Contact: Intel GFX Bugs mailing list
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-11-15 06:18 UTC by lu hua
Modified: 2017-10-06 14:42 UTC (History)
2 users (show)

See Also:
i915 platform:
i915 features:


Attachments
dmesg (97.45 KB, text/plain)
2013-11-15 06:18 UTC, lu hua
no flags Details

Description lu hua 2013-11-15 06:18:11 UTC
Created attachment 89245 [details]
dmesg

System Environment:
--------------------------
Platform: Pineview/GM45/Ironlake/Sandybridge/Ivybridge/Haswell/Baytrail
Kernel: (drm-intel-nightly)0bb2335025daa5c51b0dc2ec7d181319cc899ce6

Bug detailed description:
-------------------------
It fails on all platforms with -queued, -nightly and -fixes kernel.

Bisect shows:a4e96a95ca6468a7ece9fff1c8e1949691439ed4 is the first bad commit.
commit a4e96a95ca6468a7ece9fff1c8e1949691439ed4
Author:     Paulo Zanoni <paulo.r.zanoni@intel.com>
AuthorDate: Tue Nov 12 13:24:38 2013 -0200
Commit:     Paulo Zanoni <paulo.r.zanoni@intel.com>
CommitDate: Thu Nov 14 19:59:09 2013 -0200

    tests/pm_pc8: add support for runtime PM

    We try to detect if we have runtime PM or if we just have PC8. In case
    there's runtime PM, the functions that wait will wait for the runtime
    PM status reported by the sysfs file instead of waiting for PC8
    residencies to move.

    Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>

run: ./pm_pc8 --run-subtest i2c
output:
Test assertion failure function setup_runtime_pm, file pm_pc8.c:606:
Failed assertion: fd >= 0
Can't open /sys/devices/pci0000:00/0000:00:02.0/power/autosuspend_delay_ms
Subtest rte: FAIL
Subtest drm-resources-equal: FAIL
Subtest modeset-lpsp: FAIL
Subtest modeset-non-lpsp: FAIL
Subtest gem-mmap-cpu: FAIL
Subtest gem-mmap-gtt: FAIL
Subtest gem-pread: FAIL
Subtest gem-execbuf: FAIL
Subtest i2c: FAIL
Subtest pc8-residency: FAIL
Subtest debugfs-read: FAIL
Subtest debugfs-forcewake-user: FAIL
Subtest sysfs-read: FAIL
Subtest modeset-lpsp-stress: FAIL
Subtest modeset-non-lpsp-stress: FAIL
Subtest modeset-lpsp-stress-no-wait: FAIL
Subtest modeset-non-lpsp-stress-no-wait: FAIL
Subtest modeset-pc8-residency-stress: FAIL
Subtest modeset-stress-extra-wait: FAIL
Subtest gem-execbuf-stress: FAIL
Subtest gem-execbuf-stress: FAIL
Subtest gem-execbuf-stress: FAIL


Reproduce steps:
-------------------------
1. ./pm_pc8 --run-subtest i2c
Comment 1 Paulo Zanoni 2013-11-18 12:32:47 UTC
When you get an error like this, please analyze the error message and try to discover why it happened. In our case, the error message is:
"Can't open /sys/devices/pci0000:00/0000:00:02.0/power/autosuspend_delay_ms"

- Do you have that file on your system?
- Did you check its permissions?
- Did you run the PC8 test as root?

Thanks,
Paulo
Comment 2 lu hua 2013-11-20 07:43:10 UTC
> - Do you have that file on your system?
file: /sys/devices/pci0000:00/0000:00:02.0/power/autosuspend_delay_ms doesn't exists.
Comment 3 Chris Wilson 2013-11-20 11:01:52 UTC
diff --git a/tests/pm_pc8.c b/tests/pm_pc8.c
index efeae21..15127d4 100644
--- a/tests/pm_pc8.c
+++ b/tests/pm_pc8.c
@@ -602,15 +602,14 @@ static void setup_runtime_pm(void)
         * suite goes faster and we have a higher probability of triggering race
         * conditions. */
        fd = open(POWER_DIR "/autosuspend_delay_ms", O_WRONLY);
-       igt_assert_f(fd >= 0,
-                    "Can't open " POWER_DIR "/autosuspend_delay_ms\n");
+       if (fd != -1) {
+               /* If we fail to write to the file, it means this system doesn't support
+                * runtime PM. */
+               size = write(fd, "0\n", 2);
+               has_runtime_pm = (size == 2);
 
-       /* If we fail to write to the file, it means this system doesn't support
-        * runtime PM. */
-       size = write(fd, "0\n", 2);
-       has_runtime_pm = (size == 2);
-
-       close(fd);
+               close(fd);
+       }
 
        if (!has_runtime_pm)
                return;

should make it correctly skip the test
Comment 4 lu hua 2013-11-21 05:27:29 UTC
(In reply to comment #3)
> diff --git a/tests/pm_pc8.c b/tests/pm_pc8.c
> index efeae21..15127d4 100644
> --- a/tests/pm_pc8.c
> +++ b/tests/pm_pc8.c
> @@ -602,15 +602,14 @@ static void setup_runtime_pm(void)
>          * suite goes faster and we have a higher probability of triggering
> race
>          * conditions. */
>         fd = open(POWER_DIR "/autosuspend_delay_ms", O_WRONLY);
> -       igt_assert_f(fd >= 0,
> -                    "Can't open " POWER_DIR "/autosuspend_delay_ms\n");
> +       if (fd != -1) {
> +               /* If we fail to write to the file, it means this system
> doesn't support
> +                * runtime PM. */
> +               size = write(fd, "0\n", 2);
> +               has_runtime_pm = (size == 2);
>  
> -       /* If we fail to write to the file, it means this system doesn't
> support
> -        * runtime PM. */
> -       size = write(fd, "0\n", 2);
> -       has_runtime_pm = (size == 2);
> -
> -       close(fd);
> +               close(fd);
> +       }
>  
>         if (!has_runtime_pm)
>                 return;
> 
> should make it correctly skip the test


Fixed by this patch.
Comment 5 Daniel Vetter 2013-11-21 08:47:26 UTC
Imo the real bug here is that QA's kernel build doesn't have runtime pm enabled. If we just skip paulo's nice runtime pm tests that's not good.
Comment 6 Paulo Zanoni 2013-11-21 16:58:24 UTC
(In reply to comment #5)
> Imo the real bug here is that QA's kernel build doesn't have runtime pm
> enabled. If we just skip paulo's nice runtime pm tests that's not good.

Yeah, that's my guess too. Even my SNB machine has that file, so I got surprised by QA's report.

Still, our test should work on cases where there's just PC8 support without runtime D3 (i.e., current drm-intel-nightly), so just forcing igt_assert on everything is not a good idea.
Comment 7 Paulo Zanoni 2013-11-21 17:04:30 UTC
(In reply to comment #6)
> (In reply to comment #5)
> > Imo the real bug here is that QA's kernel build doesn't have runtime pm
> > enabled. If we just skip paulo's nice runtime pm tests that's not good.
> 
> Yeah, that's my guess too. Even my SNB machine has that file, so I got
> surprised by QA's report.
> 
> Still, our test should work on cases where there's just PC8 support without
> runtime D3 (i.e., current drm-intel-nightly), so just forcing igt_assert on
> everything is not a good idea.

Our current options:

1 - Do *not* patch pm_pc8.c, so we'll still fail the test. This way we can make sure QA's Kernels will always have runtime PM enabled. Maybe we could change the commit message and say "please check if your Kernel has runtime PM support". Then we'd close the bug once QA has correctly configured their machines.

2 - Apply Chris's patch and SKIP the test in case the machine does not have runtime PM support on the Kernel. This will benefit people who only have PC8 support and don't want to bother with runtime PM support.

I'd vote for option 1, especially since in the future PC8 and RPM will be a single feature, so we'll always require the autosuspend_delay_ms file to exist.
Comment 8 lu hua 2013-11-25 03:18:59 UTC
enable the Kernel Runtime PM support(CONFIG_PM_RUNTIME=y)
It still fails.
file: /sys/devices/pci0000:00/0000:00:02.0/power/autosuspend_delay_ms doesn't exists.
Comment 9 lu hua 2013-11-25 09:04:11 UTC
(In reply to comment #8)
> enable the Kernel Runtime PM support(CONFIG_PM_RUNTIME=y)
> It still fails.
> file: /sys/devices/pci0000:00/0000:00:02.0/power/autosuspend_delay_ms
> doesn't exists.

Sorry, enable the kernel runtime PM support, It works well. 
Close it.
Comment 10 lu hua 2013-11-25 09:04:29 UTC
Verified.Fixed.
Comment 11 Elizabeth 2017-10-06 14:42:04 UTC
Closing old verified.


Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.