Re: [igt-dev] [PATCH i-g-t 08/20] tests/psr: Merge PSR dpms and suspend variants.

[Rodrigo Vivi <rodrigo.vivi@intel.com>]

On Mon, Apr 16, 2018 at 02:46:14PM -0700, Pandiyan, Dhinakaran wrote:
> On Thu, 2018-04-12 at 13:20 +0200, Katarzyna Dec wrote:
> > On Tue, Apr 10, 2018 at 07:37:20PM -0700, Dhinakaran Pandiyan wrote:
> > > I don't see a big difference in what {dpms,suspend}_psr_exit and
> > > {dpms_off, suspend}_psr_active tests uniquely achieve. Combine them so
> > > that we have one dpms and one suspend test.
> > Could you please add more details why these tests can be merged?
> 
> 
> The reason is - the tests are pretty much the same. Combining them saves
> run time.

Agree.... Also, the other one was never reliable since we could never
assure the state of PSR when doing the operation.

So I'm in favor of this merge:
Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>

> 
> 
> > Is it based on documentation or your experience?
> > Kasia
> > > 
> > > Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> > > ---
> > >  tests/kms_psr_sink_crc.c | 34 +++-------------------------------
> > >  1 file changed, 3 insertions(+), 31 deletions(-)
> > > 
> > > diff --git a/tests/kms_psr_sink_crc.c b/tests/kms_psr_sink_crc.c
> > > index d9cce5ea..e04e0f6a 100644
> > > --- a/tests/kms_psr_sink_crc.c
> > > +++ b/tests/kms_psr_sink_crc.c
> > > @@ -522,51 +522,23 @@ int main(int argc, char *argv[])
> > >  		}
> > >  	}
> > >  
> > > -	igt_subtest_f("dpms_off_psr_active") {
> > > +	igt_subtest_f("dpms") {
> > >  		data.test_plane = DRM_PLANE_TYPE_PRIMARY;
> > >  		data.op = RENDER;
> > >  		setup_test_plane(&data);
> > >  		igt_assert(wait_psr_entry(&data));
> > > -
> > >  		dpms_off_on(data);
> > > -
> > > -		run_test(&data);
> > > -		test_cleanup(&data);
> > > -	}
> > > -
> > > -	igt_subtest_f("dpms_off_psr_exit") {
> > > -		data.test_plane = DRM_PLANE_TYPE_OVERLAY;
> > > -		data.op = PLANE_ONOFF;
> > > -		setup_test_plane(&data);
> > > -
> > > -		dpms_off_on(data);
> > > -
> > > -		igt_assert(wait_psr_entry(&data));
> > > -		run_test(&data);
> > > -		test_cleanup(&data);
> > > -	}
> > > -
> > > -	igt_subtest_f("suspend_psr_active") {
> > > -		data.test_plane = DRM_PLANE_TYPE_PRIMARY;
> > > -		data.op = PAGE_FLIP;
> > > -		setup_test_plane(&data);
> > > -		igt_assert(wait_psr_entry(&data));
> > > -
> > > -		igt_system_suspend_autoresume(SUSPEND_STATE_MEM,
> > > -					      SUSPEND_TEST_NONE);
> > > -
> > >  		run_test(&data);
> > >  		test_cleanup(&data);
> > >  	}
> > >  
> > > -	igt_subtest_f("suspend_psr_exit") {
> > > +	igt_subtest_f("suspend") {
> > >  		data.test_plane = DRM_PLANE_TYPE_CURSOR;
> > >  		data.op = PLANE_ONOFF;
> > >  		setup_test_plane(&data);
> > > -
> > > +		igt_assert(wait_psr_entry(&data));
> > >  		igt_system_suspend_autoresume(SUSPEND_STATE_MEM,
> > >  					      SUSPEND_TEST_NONE);
> > > -
> > >  		igt_assert(wait_psr_entry(&data));
> > >  		run_test(&data);
> > >  		test_cleanup(&data);
> > > -- 
> > > 2.14.1
> > > 
> > > _______________________________________________
> > > igt-dev mailing list
> > > igt-dev@lists.freedesktop.org
> > > https://lists.freedesktop.org/mailman/listinfo/igt-dev
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev