[PATCH] Fixed the review issues for pm_rc6_residency IGT case

[PATCH] Fixed the review issues for pm_rc6_residency IGT case

[Wendy Wang]

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@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

Re: [PATCH] Fixed the review issues for pm_rc6_residency IGT case

[Daniel Vetter]

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@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@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

Re: [PATCH] Fixed the review issues for pm_rc6_residency IGT case

[Ben Widawsky]

Hi Wendy. Daniel has reverted your original commit here:
commit 35554a1bcaaea55c1cfa88c0176c58d2fb3b8013
Author: Daniel Vetter <daniel.vetter@ffwll.ch>
Date:   Tue Jun 10 11:05:16 2014 +0200

    Revert "Add rc6_residency_counter subtest"

Note that I absolutely do not agree with the decision to revert your
patch as was stated in the commit message. I am not sure how Daniel got
the impression that I thought this was "in order."

Can you please resubmit the patch based on the latest intel-gpu-tools?

-- 
Ben Widawsky, Intel Open Source Technology Center

Re: [PATCH] Fixed the review issues for pm_rc6_residency IGT case

[Jesse Barnes]

On Mon, 16 Jun 2014 11:43:30 -0700
Ben Widawsky <benjamin.widawsky@intel.com> wrote:

> Hi Wendy. Daniel has reverted your original commit here:
> commit 35554a1bcaaea55c1cfa88c0176c58d2fb3b8013
> Author: Daniel Vetter <daniel.vetter@ffwll.ch>
> Date:   Tue Jun 10 11:05:16 2014 +0200
> 
>     Revert "Add rc6_residency_counter subtest"
> 
> Note that I absolutely do not agree with the decision to revert your
> patch as was stated in the commit message. I am not sure how Daniel got
> the impression that I thought this was "in order."
> 
> Can you please resubmit the patch based on the latest intel-gpu-tools?

I also made that clear when Daniel and I discussed it.  I simply don't
understand why a revert was necessary, especially given that we had an
incremental patch to address many of the comments.  Was the test
breaking i-g-t runs (i.e. preventing tests from running)?  Was it
somehow crashing and causing false reports?

-- 
Jesse Barnes, Intel Open Source Technology Center

Re: [PATCH] Fixed the review issues for pm_rc6_residency IGT case

[Daniel Vetter]

On Mon, Jun 16, 2014 at 10:38 PM, Jesse Barnes <jbarnes@virtuousgeek.org> wrote:
> On Mon, 16 Jun 2014 11:43:30 -0700
> Ben Widawsky <benjamin.widawsky@intel.com> wrote:
>
>> Hi Wendy. Daniel has reverted your original commit here:
>> commit 35554a1bcaaea55c1cfa88c0176c58d2fb3b8013
>> Author: Daniel Vetter <daniel.vetter@ffwll.ch>
>> Date:   Tue Jun 10 11:05:16 2014 +0200
>>
>>     Revert "Add rc6_residency_counter subtest"
>>
>> Note that I absolutely do not agree with the decision to revert your
>> patch as was stated in the commit message. I am not sure how Daniel got
>> the impression that I thought this was "in order."
>>
>> Can you please resubmit the patch based on the latest intel-gpu-tools?
>
> I also made that clear when Daniel and I discussed it.  I simply don't
> understand why a revert was necessary, especially given that we had an
> incremental patch to address many of the comments.  Was the test
> breaking i-g-t runs (i.e. preventing tests from running)?  Was it
> somehow crashing and causing false reports?

Ok, I've reverted the revert since people are too unhappy with it.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

Re: [PATCH] Fixed the review issues for pm_rc6_residency IGT case

[Jesse Barnes]

On Mon, 16 Jun 2014 23:55:24 +0200
Daniel Vetter <daniel@ffwll.ch> wrote:

> On Mon, Jun 16, 2014 at 10:38 PM, Jesse Barnes <jbarnes@virtuousgeek.org> wrote:
> > On Mon, 16 Jun 2014 11:43:30 -0700
> > Ben Widawsky <benjamin.widawsky@intel.com> wrote:
> >
> >> Hi Wendy. Daniel has reverted your original commit here:
> >> commit 35554a1bcaaea55c1cfa88c0176c58d2fb3b8013
> >> Author: Daniel Vetter <daniel.vetter@ffwll.ch>
> >> Date:   Tue Jun 10 11:05:16 2014 +0200
> >>
> >>     Revert "Add rc6_residency_counter subtest"
> >>
> >> Note that I absolutely do not agree with the decision to revert your
> >> patch as was stated in the commit message. I am not sure how Daniel got
> >> the impression that I thought this was "in order."
> >>
> >> Can you please resubmit the patch based on the latest intel-gpu-tools?
> >
> > I also made that clear when Daniel and I discussed it.  I simply don't
> > understand why a revert was necessary, especially given that we had an
> > incremental patch to address many of the comments.  Was the test
> > breaking i-g-t runs (i.e. preventing tests from running)?  Was it
> > somehow crashing and causing false reports?
> 
> Ok, I've reverted the revert since people are too unhappy with it.

Fine, but that doesn't address why we needed the revert in the first
place.  Until we have some clear explanation of that and some criteria,
this will just happen all over again down the road.

So, what is revert-worthy in i-g-t?  Open review items?  Requests for
change?  False test failures?  False test passes?  Crashing tests?  I'd
vote for the latter 3 myself; did this fall into any of those
categories?

Thanks,
-- 
Jesse Barnes, Intel Open Source Technology Center

Re: [PATCH] Fixed the review issues for pm_rc6_residency IGT case

[Daniel Vetter]

On Tue, Jun 17, 2014 at 12:03 AM, Jesse Barnes <jbarnes@virtuousgeek.org> wrote:
> So, what is revert-worthy in i-g-t?  Open review items?  Requests for
> change?  False test failures?  False test passes?  Crashing tests?  I'd
> vote for the latter 3 myself; did this fall into any of those
> categories?

Imo this isn't about when to revert, but about the responsibilities of
the committer. Especially for new contributors that imo includes full
review and making sure the test has a bit polish applied and uses all
the latest igt infrastructure. My impression was that Ben didn't do
that so I asked him whether he'll follow up. From irc I've gotten the
impression that he won't do that work (maybe that was a
misunderstanding) and that I should go ahead.

That I reverted the patch is just personal choice - for polishing
patches I much prefer one clean patch that gets revised than
committing early and then applying a few fixup patches. It's simply
the process I'm used to from the kernel side.

I don't mind at all if other people do this differently, as long as it
gets done. Which in this case here didn't look like. And if you've
read my first mail in this thread there's still work left to do here.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

Re: [PATCH] Fixed the review issues for pm_rc6_residency IGT case

[Daniel Vetter]

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@intel.com>

Patch applied, thanks.
-Daniel

> 
> 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@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