[Intel-gfx] [PATCH] Fixed the review issues for pm_rc6_residency IGT case
Daniel Vetter
daniel at ffwll.ch
Tue Jun 10 11:12:13 CEST 2014
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 at 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 at 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 at 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 at 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 at 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 at 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 at intel.com>
>
> 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 at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
More information about the Intel-gfx
mailing list