diff mbox

[06/18] tests/drv_suspend: mark sysfs tests as basic

Message ID 1439497901-14310-6-git-send-email-jbarnes@virtuousgeek.org (mailing list archive)
State New, archived
Headers show

Commit Message

Jesse Barnes Aug. 13, 2015, 8:31 p.m. UTC
debugfs may not be mounted, but sysfs should always be restored after
suspend or hibernate.

Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
---
 tests/drv_suspend.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Comments

Daniel Vetter Aug. 14, 2015, 12:29 p.m. UTC | #1
On Thu, Aug 13, 2015 at 01:31:29PM -0700, Jesse Barnes wrote:
> debugfs may not be mounted, but sysfs should always be restored after
> suspend or hibernate.
> 
> Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>

We already have a suspend/resume testcase in kms_pipc_crc_basic. Do we
have enough budget for this one?
> ---
>  tests/drv_suspend.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/tests/drv_suspend.c b/tests/drv_suspend.c
> index d67a794..60ca8e3 100644
> --- a/tests/drv_suspend.c
> +++ b/tests/drv_suspend.c
> @@ -193,7 +193,7 @@ igt_main
>  	igt_subtest("debugfs-reader")
>  		test_debugfs_reader(false);
>  
> -	igt_subtest("sysfs-reader")
> +	igt_subtest("basic-sysfs-reader")
>  		test_sysfs_reader(false);
>  
>  	igt_subtest("forcewake")
> @@ -208,7 +208,7 @@ igt_main
>  	igt_subtest("debugfs-reader-hibernate")
>  		test_debugfs_reader(true);
>  
> -	igt_subtest("sysfs-reader-hibernate")
> +	igt_subtest("basic-sysfs-reader-hibernate")
>  		test_sysfs_reader(true);

Hibernate is a giantic can of worms. We have a big pile of issues here
that we never fixed. Otoh not testing hibernate is definitely an
oversight. Assuming we have the testing budget:

Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch>
>  
>  	igt_subtest("forcewake-hibernate")
> -- 
> 1.9.1
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Jesse Barnes Aug. 14, 2015, 3:29 p.m. UTC | #2
On 08/14/2015 05:29 AM, Daniel Vetter wrote:
> On Thu, Aug 13, 2015 at 01:31:29PM -0700, Jesse Barnes wrote:
>> debugfs may not be mounted, but sysfs should always be restored after
>> suspend or hibernate.
>>
>> Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
> 
> We already have a suspend/resume testcase in kms_pipc_crc_basic. Do we
> have enough budget for this one?

Yeah I thought about getting rid of the suspend/resume ones in pipe_crc (marking them as non-basic), since I really just want the one set of tests.  Any preference?

Jesse
Daniel Vetter Aug. 14, 2015, 4:01 p.m. UTC | #3
On Fri, Aug 14, 2015 at 08:29:40AM -0700, Jesse Barnes wrote:
> On 08/14/2015 05:29 AM, Daniel Vetter wrote:
> > On Thu, Aug 13, 2015 at 01:31:29PM -0700, Jesse Barnes wrote:
> >> debugfs may not be mounted, but sysfs should always be restored after
> >> suspend or hibernate.
> >>
> >> Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
> > 
> > We already have a suspend/resume testcase in kms_pipc_crc_basic. Do we
> > have enough budget for this one?
> 
> Yeah I thought about getting rid of the suspend/resume ones in pipe_crc
> (marking them as non-basic), since I really just want the one set of
> tests.  Any preference?

Imo crc is such a basic (hah!) validation tool that we really should try
to run all the corner cases. Otherwise we have ugly situations where some
tests randomyl fail, depending upon when they've been run after system
suspend/resume or not. And with all our troubles we need to reboot a few
times for a full run, so this is likely.

This actually happened (that's why we have these tests) and resulted in
lots and lots of wtf until resolved. So yeah I think for crc we really
want them all, even if it's a bit expensive, since if we don't there's a
good chance for lots of flaky test results. And that's the prime problem
we have with igt.
-Daniel
Jesse Barnes Aug. 14, 2015, 4:10 p.m. UTC | #4
On 08/14/2015 09:01 AM, Daniel Vetter wrote:
> On Fri, Aug 14, 2015 at 08:29:40AM -0700, Jesse Barnes wrote:
>> On 08/14/2015 05:29 AM, Daniel Vetter wrote:
>>> On Thu, Aug 13, 2015 at 01:31:29PM -0700, Jesse Barnes wrote:
>>>> debugfs may not be mounted, but sysfs should always be restored after
>>>> suspend or hibernate.
>>>>
>>>> Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
>>>
>>> We already have a suspend/resume testcase in kms_pipc_crc_basic. Do we
>>> have enough budget for this one?
>>
>> Yeah I thought about getting rid of the suspend/resume ones in pipe_crc
>> (marking them as non-basic), since I really just want the one set of
>> tests.  Any preference?
> 
> Imo crc is such a basic (hah!) validation tool that we really should try
> to run all the corner cases. Otherwise we have ugly situations where some
> tests randomyl fail, depending upon when they've been run after system
> suspend/resume or not. And with all our troubles we need to reboot a few
> times for a full run, so this is likely.
> 
> This actually happened (that's why we have these tests) and resulted in
> lots and lots of wtf until resolved. So yeah I think for crc we really
> want them all, even if it's a bit expensive, since if we don't there's a
> good chance for lots of flaky test results. And that's the prime problem
> we have with igt.

Ok sounds good, I'll drop the drv_suspend test then.

Jesse
diff mbox

Patch

diff --git a/tests/drv_suspend.c b/tests/drv_suspend.c
index d67a794..60ca8e3 100644
--- a/tests/drv_suspend.c
+++ b/tests/drv_suspend.c
@@ -193,7 +193,7 @@  igt_main
 	igt_subtest("debugfs-reader")
 		test_debugfs_reader(false);
 
-	igt_subtest("sysfs-reader")
+	igt_subtest("basic-sysfs-reader")
 		test_sysfs_reader(false);
 
 	igt_subtest("forcewake")
@@ -208,7 +208,7 @@  igt_main
 	igt_subtest("debugfs-reader-hibernate")
 		test_debugfs_reader(true);
 
-	igt_subtest("sysfs-reader-hibernate")
+	igt_subtest("basic-sysfs-reader-hibernate")
 		test_sysfs_reader(true);
 
 	igt_subtest("forcewake-hibernate")