All of lore.kernel.org
 help / color / mirror / Atom feed
From: Martin Peres <martin.peres@linux.intel.com>
To: Chris Wilson <chris@chris-wilson.co.uk>, intel-gfx@lists.freedesktop.org
Cc: igt-dev@lists.freedesktop.org
Subject: Re: [igt-dev] [PATCH i-g-t] i915/i915_pm_rpm: Only check for suspend failures after each debugfs entry
Date: Mon, 24 Feb 2020 11:16:02 +0200	[thread overview]
Message-ID: <efbae310-47f4-d774-ccac-31ee30a0f96b@linux.intel.com> (raw)
In-Reply-To: <20200221135620.2794706-1-chris@chris-wilson.co.uk>

On 2020-02-21 15:56, Chris Wilson wrote:
> Since we check before and then after each debugfs entry, we do not need
> to check before each time as well. We will error out as soon as it does
> fail, at all other times we know the system to be idle.
> 
> No impact on runtime for glk (which apparently is one of the better
> behaving systems).
> 
> v2: Assert that we are currently suspended prior to opening the file,
> i.e. that nothing else is waking up the device behind our backs.
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Martin Peres <martin.peres@linux.intel.com>

This changes the purpose of this test, but checking that nothing is
waking the driver in the background sound like a good test and could
explain the terribly-slow runtime on some platforms!

Reviewed-by: Martin Peres <martin.peres@linux.intel.com>

Please inform me before merging the patch, so as I can suppress the test
ahead of the merge.

> ---
>  tests/i915/i915_pm_rpm.c | 10 +++++++++-
>  1 file changed, 9 insertions(+), 1 deletion(-)
> 
> diff --git a/tests/i915/i915_pm_rpm.c b/tests/i915/i915_pm_rpm.c
> index 0c2821122..36416a9db 100644
> --- a/tests/i915/i915_pm_rpm.c
> +++ b/tests/i915/i915_pm_rpm.c
> @@ -183,6 +183,14 @@ static enum pc8_status get_pc8_status(void)
>  		return PC8_DISABLED;
>  }
>  
> +static bool is_suspended(void)
> +{
> +	if (has_pc8 && !has_runtime_pm)
> +		return get_pc8_status() == PC8_ENABLED;
> +	else
> +		return igt_get_runtime_pm_status() == IGT_RUNTIME_PM_STATUS_SUSPENDED;
> +}
> +
>  static bool wait_for_pc8_status(enum pc8_status status)
>  {
>  	return igt_wait(get_pc8_status() == status, 10000, 100);
> @@ -932,7 +940,7 @@ static int read_entry(const char *filepath,
>  	int fd;
>  	int rc;
>  
> -	igt_assert_f(wait_for_suspended(), "Before opening: %s (%s)\n",
> +	igt_assert_f(is_suspended(), "Before opening: %s (%s)\n",
>  		     filepath + pathinfo->base, filepath);
>  
>  	fd = open(filepath, O_RDONLY | O_NONBLOCK);
> 
_______________________________________________
igt-dev mailing list
igt-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/igt-dev

WARNING: multiple messages have this Message-ID (diff)
From: Martin Peres <martin.peres@linux.intel.com>
To: Chris Wilson <chris@chris-wilson.co.uk>, intel-gfx@lists.freedesktop.org
Cc: igt-dev@lists.freedesktop.org
Subject: Re: [Intel-gfx] [PATCH i-g-t] i915/i915_pm_rpm: Only check for suspend failures after each debugfs entry
Date: Mon, 24 Feb 2020 11:16:02 +0200	[thread overview]
Message-ID: <efbae310-47f4-d774-ccac-31ee30a0f96b@linux.intel.com> (raw)
In-Reply-To: <20200221135620.2794706-1-chris@chris-wilson.co.uk>

On 2020-02-21 15:56, Chris Wilson wrote:
> Since we check before and then after each debugfs entry, we do not need
> to check before each time as well. We will error out as soon as it does
> fail, at all other times we know the system to be idle.
> 
> No impact on runtime for glk (which apparently is one of the better
> behaving systems).
> 
> v2: Assert that we are currently suspended prior to opening the file,
> i.e. that nothing else is waking up the device behind our backs.
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Martin Peres <martin.peres@linux.intel.com>

This changes the purpose of this test, but checking that nothing is
waking the driver in the background sound like a good test and could
explain the terribly-slow runtime on some platforms!

Reviewed-by: Martin Peres <martin.peres@linux.intel.com>

Please inform me before merging the patch, so as I can suppress the test
ahead of the merge.

> ---
>  tests/i915/i915_pm_rpm.c | 10 +++++++++-
>  1 file changed, 9 insertions(+), 1 deletion(-)
> 
> diff --git a/tests/i915/i915_pm_rpm.c b/tests/i915/i915_pm_rpm.c
> index 0c2821122..36416a9db 100644
> --- a/tests/i915/i915_pm_rpm.c
> +++ b/tests/i915/i915_pm_rpm.c
> @@ -183,6 +183,14 @@ static enum pc8_status get_pc8_status(void)
>  		return PC8_DISABLED;
>  }
>  
> +static bool is_suspended(void)
> +{
> +	if (has_pc8 && !has_runtime_pm)
> +		return get_pc8_status() == PC8_ENABLED;
> +	else
> +		return igt_get_runtime_pm_status() == IGT_RUNTIME_PM_STATUS_SUSPENDED;
> +}
> +
>  static bool wait_for_pc8_status(enum pc8_status status)
>  {
>  	return igt_wait(get_pc8_status() == status, 10000, 100);
> @@ -932,7 +940,7 @@ static int read_entry(const char *filepath,
>  	int fd;
>  	int rc;
>  
> -	igt_assert_f(wait_for_suspended(), "Before opening: %s (%s)\n",
> +	igt_assert_f(is_suspended(), "Before opening: %s (%s)\n",
>  		     filepath + pathinfo->base, filepath);
>  
>  	fd = open(filepath, O_RDONLY | O_NONBLOCK);
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

  reply	other threads:[~2020-02-24  9:16 UTC|newest]

Thread overview: 19+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-02-20 17:41 [igt-dev] [PATCH i-g-t] i915/i915_pm_rpm: Only check for suspend failures after each debugfs entry Chris Wilson
2020-02-20 17:41 ` [Intel-gfx] " Chris Wilson
2020-02-20 19:56 ` [igt-dev] ✓ Fi.CI.BAT: success for " Patchwork
2020-02-21  7:33 ` [igt-dev] [PATCH i-g-t] " Martin Peres
2020-02-21  7:33   ` [Intel-gfx] " Martin Peres
2020-02-21  8:21   ` [igt-dev] " Chris Wilson
2020-02-21  8:21     ` [Intel-gfx] " Chris Wilson
2020-02-21  8:28     ` [igt-dev] " Martin Peres
2020-02-21  8:28       ` [Intel-gfx] " Martin Peres
2020-02-21 13:55       ` [igt-dev] " Chris Wilson
2020-02-21 13:55         ` [Intel-gfx] " Chris Wilson
2020-02-21 13:56 ` Chris Wilson
2020-02-24  9:16   ` Martin Peres [this message]
2020-02-24  9:16     ` Martin Peres
2020-02-24  9:59     ` [igt-dev] " Chris Wilson
2020-02-24  9:59       ` [Intel-gfx] " Chris Wilson
2020-02-21 15:38 ` [igt-dev] ✓ Fi.CI.BAT: success for i915/i915_pm_rpm: Only check for suspend failures after each debugfs entry (rev2) Patchwork
2020-02-23  4:27 ` [igt-dev] ✓ Fi.CI.IGT: success for i915/i915_pm_rpm: Only check for suspend failures after each debugfs entry Patchwork
2020-02-24 10:52 ` [igt-dev] ✓ Fi.CI.IGT: success for i915/i915_pm_rpm: Only check for suspend failures after each debugfs entry (rev2) Patchwork

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=efbae310-47f4-d774-ccac-31ee30a0f96b@linux.intel.com \
    --to=martin.peres@linux.intel.com \
    --cc=chris@chris-wilson.co.uk \
    --cc=igt-dev@lists.freedesktop.org \
    --cc=intel-gfx@lists.freedesktop.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.