Hi Wendy, I think it's better to squash this into the original patch so I've reverted your patch from i-g-t for now. Also chatted with Ben on irc and he's ok with that. More comments after I've read your test more carefully: - Please don't use abort() or exit() anywhere in your test. Use the igt macros and functions instead so that the test result handling is done properly. We have rather good igt documentation now, I suggest http://people.freedesktop.org/~danvet/igt/intel-gpu-tools-i-g-t-core.html as a starting point. - The new subtest doesn't really test anything new, but only adds more informational output afaics. I think it's better to just merge that into the existing basic subtest for rc6 residency. You could also use igt_debug() to print optional diagnostical information. See the above documentation for how to use it. - When we convert an existing simple testcase into a test with subtests we call the basic subtest "basic". This way it's easier for QA to track regressions and test status across such reorganizations. Also for subtests there's no reason to repeat the overall testname so here you can drop the "rc6_residency" part since that's already included in the "pm_rc6_residency" name. On a quick glance your fixup here looks good. Please cc me on the next revision so that I can work together with you to polish this patch. Thanks, Daniel On Mon, Jun 09, 2014 at 04:36:47PM +0800, Wendy Wang wrote: > Why need add rc6_residency_counter subtest case: > RC6 feature support residency counter,from power consumption aspect, > the counter closer to 1,the better.If the counter is < 0.9, the residency > is not good and will impact power consumption value, if the counter is > 1, > sysfs file is inaccurate. > > Attach the test result message: > root@x-bdw05:/GFX/Test/Intel_gpu_tools/intel-gpu-tools/tests# ./pm_rc6_residency > IGT-Version: 1.6-g9a70e29 (x86_64) (Linux: 3.15.0-rc7_drm-intel-nightly_0a37b5_20140604+ x86_64) > Subtest rc6-residency-check: SUCCESS > This machine doesn't support rc6pp > This machine doesn't support rc6p > The residency counter : 0.987000 > This machine entry rc6 state. > Subtest rc6-residency-counter: SUCCESS > > root@x-bdw05:/GFX/Test/Intel_gpu_tools/intel-gpu-tools/tests# ./pm_rc6_residency --run-subtest rc6-residency-counter > IGT-Version: 1.6-g9a70e29 (x86_64) (Linux: 3.15.0-rc7_drm-intel-nightly_0a37b5_20140604+ x86_64) > This machine doesn't support rc6pp > This machine doesn't support rc6p > The residency counter : 0.987000 > This machine entry rc6 state. > Subtest rc6-residency-counter: SUCCESS > > root@x-bdw05:/GFX/Test/Intel_gpu_tools/intel-gpu-tools/tests# ./pm_rc6_residency --run-subtest rc6-residency-check > IGT-Version: 1.6-g9a70e29 (x86_64) (Linux: 3.15.0-rc7_drm-intel-nightly_0a37b5_20140604+ x86_64) > Subtest rc6-residency-check: SUCCESS > > root@x-bdw05:/GFX/Test/Intel_gpu_tools/intel-gpu-tools/tests# ./pm_rc6_residency --list > rc6-residency-check > rc6-residency-counter > > Run as non-root > [haha@x-pk home]$ ./pm_rc6_residency > IGT-Version: 1.6-g18d2130 (x86_64) (Linux: 3.13.0-rc3_drm-intel-nightly_639e4d_20131210+ x86_64) > No intel gpu found > Subtest rc6-residency-check: SKIP > Subtest rc6-residency-counter: SKIP > > Run on non-intel platform > [root@x-pk5 home]# ./pm_rc6_residency > IGT-Version: 1.6-g18d2130 (x86_64) (Linux: 3.13.0-rc3_drm-intel-nightly_639e4d_20131210+ x86_64) > Test requirement not met in function read_rc6_residency, file pm_rc6_residency.c:77: > Last errno: 2, No such file or directory > Test requirement: (!(file)) > Subtest rc6-residency-check: SKIP > Subtest rc6-residency-counter: SKIP > > Signed-off-by: Wendy Wang <wendy.wang@xxxxxxxxx> > > diff --git a/tests/Makefile.sources b/tests/Makefile.sources > index e4c23b3..214c055 100644 > --- a/tests/Makefile.sources > +++ b/tests/Makefile.sources > @@ -72,6 +72,7 @@ TESTS_progs_M = \ > pm_lpsp \ > pm_rpm \ > pm_rps \ > + pm_rc6_residency \ > prime_self_import \ > template \ > $(NULL) > @@ -138,7 +139,6 @@ TESTS_progs = \ > kms_sink_crc_basic \ > kms_fence_pin_leak \ > pm_psr \ > - pm_rc6_residency \ > prime_udl \ > $(NULL) > > diff --git a/tests/pm_rc6_residency.c b/tests/pm_rc6_residency.c > index 550e3ad..d364369 100644 > --- a/tests/pm_rc6_residency.c > +++ b/tests/pm_rc6_residency.c > @@ -38,7 +38,6 @@ > #define SLEEP_DURATION 3000 // in milliseconds > #define RC6_FUDGE 900 // in milliseconds > > - > static unsigned int readit(const char *path) > { > unsigned int ret; > @@ -60,6 +59,7 @@ static unsigned int readit(const char *path) > > static void read_rc6_residency( int value[], const char *name_of_rc6_residency[]) > { > + unsigned int i; > const int device = drm_get_card(); > char *path ; > int ret; > @@ -72,32 +72,33 @@ static void read_rc6_residency( int value[], const char *name_of_rc6_residency[] > ret = asprintf(&path, "/sys/class/drm/card%d/power/rc6_enable", device); > igt_assert(ret != -1); > > - file = fopen(path, "r");//open > + file = fopen(path, "r"); > igt_require(file); > > /* claim success if no rc6 enabled. */ > if (readit(path) == 0) > igt_success(); > > - for(unsigned int i = 0; i < 6; i++) > + for(i = 0; i < 6; i++) > { > if(i == 3) > sleep(SLEEP_DURATION / 1000); > - ret = asprintf(&path, "/sys/class/drm/card%d/power/%s_residency_ms",device,name_of_rc6_residency[i % 3] ); > + ret = asprintf(&path, "/sys/class/drm/card%d/power/%s_residency_ms",device,name_of_rc6_residency[i % 3]); > igt_assert(ret != -1); > value[i] = readit(path); > } > free(path); > } > > -static void rc6_residency_counter(int value[],const char * name_of_rc6_residency[]) > +static void rc6_residency_counter(int value[],const char *name_of_rc6_residency[]) > { > + int flag; > unsigned int flag_counter,flag_support; > - double counter_result = 0; > + double counter_result = 0; > flag_counter = 0; > flag_support = 0; > > - for(int flag = NUMBER_OF_RC6_RESIDENCY-1; flag >= 0 ; flag --) > + for(flag = NUMBER_OF_RC6_RESIDENCY-1; flag >= 0; flag --) > { > unsigned int tmp_counter, tmp_support; > double counter; > @@ -124,11 +125,10 @@ static void rc6_residency_counter(int value[],const char * name_of_rc6_residency > flag_support = flag_support << 1; > } > > - printf("The residency counter : %f \n", counter_result); > + printf("The residency counter: %f \n", counter_result); > > - igt_assert_f(flag_counter != 0 , "The RC6 residency counter is not good.\n"); > - igt_assert_f(flag_support != 0 , "This machine doesn't support any RC6 state!\n"); > - igt_assert_f(counter_result <=1 , "Debug files must be wrong \n"); > + igt_skip_on_f(flag_support == 0 , "This machine didn't entry any RC6 state.\n"); > + igt_assert_f((flag_counter != 0) && (counter_result <=1) , "Sysfs RC6 residency counter is inaccurate.\n"); > > printf("This machine entry %s state.\n", name_of_rc6_residency[(flag_counter / 2) - 1]); > } > @@ -148,22 +148,23 @@ static void rc6_residency_check(int value[]) > > igt_main > { > - int value[6]; > int fd; > - const char * name_of_rc6_residency[3]={"rc6","rc6p","rc6pp"}; > + int value[6]; > + const char *name_of_rc6_residency[3]={"rc6","rc6p","rc6pp"}; > > igt_skip_on_simulation(); > > /* Use drm_open_any to verify device existence */ > - fd = drm_open_any(); > - close(fd); > + igt_fixture { > + fd = drm_open_any(); > + close(fd); > > - read_rc6_residency(value, name_of_rc6_residency); > + read_rc6_residency(value, name_of_rc6_residency); > + } > > igt_subtest("rc6-residency-check") > rc6_residency_check(value); > > igt_subtest("rc6-residency-counter") > rc6_residency_counter(value, name_of_rc6_residency); > - > } > -- > 1.8.3.1 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@xxxxxxxxxxxxxxxxxxxxx > http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/intel-gfx
