[PATCH] drm/i915: Do not use iowait while waiting for the GPU

[PATCH] drm/i915: Do not use iowait while waiting for the GPU

[Chris Wilson]

A recent trend for cpufreq is to boost the CPU frequencies for
iowaiters, in particularly to benefit high frequency I/O. We do the same
and boost the GPU clocks to try and minimise time spent waiting for the
GPU. However, as the igfx and CPU share the same TDP, boosting the CPU
frequency will result in the GPU being throttled and its frequency being
reduced. Thus declaring iowait negatively impacts on GPU throughput.

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=107410
References: 52ccc4314293 ("cpufreq: intel_pstate: HWP boost performance on IO wakeup")
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Cc: Eero Tamminen <eero.t.tamminen@intel.com>
Cc: Francisco Jerez <currojerez@riseup.net>
---
 drivers/gpu/drm/i915/i915_request.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_request.c b/drivers/gpu/drm/i915/i915_request.c
index 5c2c93cbab12..7ef7ade12073 100644
--- a/drivers/gpu/drm/i915/i915_request.c
+++ b/drivers/gpu/drm/i915/i915_request.c
@@ -1330,7 +1330,7 @@ long i915_request_wait(struct i915_request *rq,
 			goto complete;
 		}
 
-		timeout = io_schedule_timeout(timeout);
+		timeout = schedule_timeout(timeout);
 	} while (1);
 
 	GEM_BUG_ON(!intel_wait_has_seqno(&wait));
-- 
2.18.0

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915: Do not use iowait while waiting for the GPU

[Chris Wilson]

Quoting Chris Wilson (2018-07-27 19:43:12)
> A recent trend for cpufreq is to boost the CPU frequencies for
> iowaiters, in particularly to benefit high frequency I/O. We do the same
> and boost the GPU clocks to try and minimise time spent waiting for the
> GPU. However, as the igfx and CPU share the same TDP, boosting the CPU
> frequency will result in the GPU being throttled and its frequency being
> reduced. Thus declaring iowait negatively impacts on GPU throughput.
> 
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=107410
> References: 52ccc4314293 ("cpufreq: intel_pstate: HWP boost performance on IO wakeup")
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> Cc: Eero Tamminen <eero.t.tamminen@intel.com>
> Cc: Francisco Jerez <currojerez@riseup.net>
> ---
>  drivers/gpu/drm/i915/i915_request.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_request.c b/drivers/gpu/drm/i915/i915_request.c
> index 5c2c93cbab12..7ef7ade12073 100644
> --- a/drivers/gpu/drm/i915/i915_request.c
> +++ b/drivers/gpu/drm/i915/i915_request.c
> @@ -1330,7 +1330,7 @@ long i915_request_wait(struct i915_request *rq,
>                         goto complete;
>                 }
>  
> -               timeout = io_schedule_timeout(timeout);
> +               timeout = schedule_timeout(timeout);

Wrong one...
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✗ Fi.CI.CHECKPATCH: warning for drm/i915: Do not use iowait while waiting for the GPU

[Patchwork]

== Series Details ==

Series: drm/i915: Do not use iowait while waiting for the GPU
URL   : https://patchwork.freedesktop.org/series/47363/
State : warning

== Summary ==

$ dim checkpatch origin/drm-tip
9323b39a7901 drm/i915: Do not use iowait while waiting for the GPU
-:14: WARNING:COMMIT_LOG_LONG_LINE: Possible unwrapped commit description (prefer a maximum 75 chars per line)
#14: 
References: 52ccc4314293 ("cpufreq: intel_pstate: HWP boost performance on IO wakeup")

-:14: ERROR:GIT_COMMIT_ID: Please use git commit description style 'commit <12+ chars of sha1> ("<title line>")' - ie: 'commit 52ccc4314293 ("cpufreq: intel_pstate: HWP boost performance on IO wakeup")'
#14: 
References: 52ccc4314293 ("cpufreq: intel_pstate: HWP boost performance on IO wakeup")

total: 1 errors, 1 warnings, 0 checks, 8 lines checked

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH] drm/i915: Do not use iowait while waiting for the GPU

[Chris Wilson]

A recent trend for cpufreq is to boost the CPU frequencies for
iowaiters, in particularly to benefit high frequency I/O. We do the same
and boost the GPU clocks to try and minimise time spent waiting for the
GPU. However, as the igfx and CPU share the same TDP, boosting the CPU
frequency will result in the GPU being throttled and its frequency being
reduced. Thus declaring iowait negatively impacts on GPU throughput.

v2: Both sleeps!

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=107410
References: 52ccc4314293 ("cpufreq: intel_pstate: HWP boost performance on IO wakeup")
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>
Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
Cc: Eero Tamminen <eero.t.tamminen@intel.com>
Cc: Francisco Jerez <currojerez@riseup.net>
---
 drivers/gpu/drm/i915/i915_request.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_request.c b/drivers/gpu/drm/i915/i915_request.c
index 5c2c93cbab12..21950c1c3885 100644
--- a/drivers/gpu/drm/i915/i915_request.c
+++ b/drivers/gpu/drm/i915/i915_request.c
@@ -1330,7 +1330,7 @@ long i915_request_wait(struct i915_request *rq,
 			goto complete;
 		}
 
-		timeout = io_schedule_timeout(timeout);
+		timeout = schedule_timeout(timeout);
 	} while (1);
 
 	GEM_BUG_ON(!intel_wait_has_seqno(&wait));
@@ -1363,7 +1363,7 @@ long i915_request_wait(struct i915_request *rq,
 			break;
 		}
 
-		timeout = io_schedule_timeout(timeout);
+		timeout = schedule_timeout(timeout);
 
 		if (intel_wait_complete(&wait) &&
 		    intel_wait_check_request(&wait, rq))
-- 
2.18.0

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✓ Fi.CI.BAT: success for drm/i915: Do not use iowait while waiting for the GPU

[Patchwork]

== Series Details ==

Series: drm/i915: Do not use iowait while waiting for the GPU
URL   : https://patchwork.freedesktop.org/series/47363/
State : success

== Summary ==

= CI Bug Log - changes from CI_DRM_4566 -> Patchwork_9795 =

== Summary - SUCCESS ==

  No regressions found.

  External URL: https://patchwork.freedesktop.org/api/1.0/series/47363/revisions/1/mbox/

== Known issues ==

  Here are the changes found in Patchwork_9795 that come from known issues:

  === IGT changes ===

    ==== Issues hit ====

    igt@drv_selftest@live_hangcheck:
      fi-skl-guc:         PASS -> DMESG-FAIL (fdo#107174)

    igt@kms_frontbuffer_tracking@basic:
      fi-hsw-peppy:       NOTRUN -> DMESG-FAIL (fdo#106103, fdo#102614)

    igt@prime_vgem@basic-fence-flip:
      fi-ilk-650:         PASS -> FAIL (fdo#104008)

    
    ==== Possible fixes ====

    igt@kms_chamelium@dp-edid-read:
      fi-kbl-7500u:       FAIL (fdo#103841) -> PASS

    
    ==== Warnings ====

    {igt@kms_psr@primary_page_flip}:
      fi-cnl-psr:         DMESG-FAIL (fdo#107372) -> DMESG-WARN (fdo#107372)

    
  {name}: This element is suppressed. This means it is ignored when computing
          the status of the difference (SUCCESS, WARNING, or FAILURE).

  fdo#102614 https://bugs.freedesktop.org/show_bug.cgi?id=102614
  fdo#103841 https://bugs.freedesktop.org/show_bug.cgi?id=103841
  fdo#104008 https://bugs.freedesktop.org/show_bug.cgi?id=104008
  fdo#106103 https://bugs.freedesktop.org/show_bug.cgi?id=106103
  fdo#107174 https://bugs.freedesktop.org/show_bug.cgi?id=107174
  fdo#107372 https://bugs.freedesktop.org/show_bug.cgi?id=107372


== Participating hosts (52 -> 47) ==

  Additional (1): fi-hsw-peppy 
  Missing    (6): fi-ilk-m540 fi-hsw-4200u fi-byt-squawks fi-bsw-cyan fi-ctg-p8600 fi-byt-clapper 


== Build changes ==

    * Linux: CI_DRM_4566 -> Patchwork_9795

  CI_DRM_4566: 57e37535b29e6a90d36b0705968cdebff24c71ed @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4580: f1c868dae24056ebc27e4f3c197724ce9b956a8a @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_9795: 9323b39a79014311227b059c3f086340cdf7f9ff @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

9323b39a7901 drm/i915: Do not use iowait while waiting for the GPU

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_9795/issues.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✗ Fi.CI.CHECKPATCH: warning for drm/i915: Do not use iowait while waiting for the GPU (rev2)

[Patchwork]

== Series Details ==

Series: drm/i915: Do not use iowait while waiting for the GPU (rev2)
URL   : https://patchwork.freedesktop.org/series/47363/
State : warning

== Summary ==

$ dim checkpatch origin/drm-tip
0fb47183378a drm/i915: Do not use iowait while waiting for the GPU
-:16: WARNING:COMMIT_LOG_LONG_LINE: Possible unwrapped commit description (prefer a maximum 75 chars per line)
#16: 
References: 52ccc4314293 ("cpufreq: intel_pstate: HWP boost performance on IO wakeup")

-:16: ERROR:GIT_COMMIT_ID: Please use git commit description style 'commit <12+ chars of sha1> ("<title line>")' - ie: 'commit 52ccc4314293 ("cpufreq: intel_pstate: HWP boost performance on IO wakeup")'
#16: 
References: 52ccc4314293 ("cpufreq: intel_pstate: HWP boost performance on IO wakeup")

total: 1 errors, 1 warnings, 0 checks, 16 lines checked

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✓ Fi.CI.BAT: success for drm/i915: Do not use iowait while waiting for the GPU (rev2)

[Patchwork]

== Series Details ==

Series: drm/i915: Do not use iowait while waiting for the GPU (rev2)
URL   : https://patchwork.freedesktop.org/series/47363/
State : success

== Summary ==

= CI Bug Log - changes from CI_DRM_4567 -> Patchwork_9797 =

== Summary - WARNING ==

  Minor unknown changes coming with Patchwork_9797 need to be verified
  manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in Patchwork_9797, please notify your bug team to allow them
  to document this new failure mode, which will reduce false positives in CI.

  External URL: https://patchwork.freedesktop.org/api/1.0/series/47363/revisions/2/mbox/

== Possible new issues ==

  Here are the unknown changes that may have been introduced in Patchwork_9797:

  === IGT changes ===

    ==== Warnings ====

    igt@drv_selftest@live_guc:
      fi-kbl-7560u:       PASS -> SKIP +1

    
== Known issues ==

  Here are the changes found in Patchwork_9797 that come from known issues:

  === IGT changes ===

    ==== Issues hit ====

    igt@drv_selftest@live_hangcheck:
      fi-kbl-7560u:       PASS -> DMESG-FAIL (fdo#106947, fdo#106560)

    igt@kms_chamelium@dp-edid-read:
      fi-kbl-7500u:       PASS -> FAIL (fdo#103841)

    igt@kms_pipe_crc_basic@suspend-read-crc-pipe-a:
      fi-snb-2600:        PASS -> INCOMPLETE (fdo#105411)

    
    ==== Possible fixes ====

    igt@prime_vgem@basic-fence-flip:
      fi-ilk-650:         FAIL (fdo#104008) -> PASS

    
    ==== Warnings ====

    {igt@kms_psr@primary_page_flip}:
      fi-cnl-psr:         DMESG-FAIL (fdo#107372) -> DMESG-WARN (fdo#107372)

    
  {name}: This element is suppressed. This means it is ignored when computing
          the status of the difference (SUCCESS, WARNING, or FAILURE).

  fdo#103841 https://bugs.freedesktop.org/show_bug.cgi?id=103841
  fdo#104008 https://bugs.freedesktop.org/show_bug.cgi?id=104008
  fdo#105411 https://bugs.freedesktop.org/show_bug.cgi?id=105411
  fdo#106560 https://bugs.freedesktop.org/show_bug.cgi?id=106560
  fdo#106947 https://bugs.freedesktop.org/show_bug.cgi?id=106947
  fdo#107372 https://bugs.freedesktop.org/show_bug.cgi?id=107372


== Participating hosts (53 -> 47) ==

  Missing    (6): fi-ilk-m540 fi-hsw-4200u fi-byt-squawks fi-bsw-cyan fi-ctg-p8600 fi-byt-clapper 


== Build changes ==

    * Linux: CI_DRM_4567 -> Patchwork_9797

  CI_DRM_4567: f1c26a5ef3fc573651e536a694ab505fb7c6a0e2 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4580: f1c868dae24056ebc27e4f3c197724ce9b956a8a @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_9797: 0fb47183378ab49a76358b78e9653fc576996f66 @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

0fb47183378a drm/i915: Do not use iowait while waiting for the GPU

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_9797/issues.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915: Do not use iowait while waiting for the GPU

[Francisco Jerez]

[-- Attachment #1.1.1: Type: text/plain, Size: 3411 bytes --]

Chris Wilson <chris@chris-wilson.co.uk> writes:

> A recent trend for cpufreq is to boost the CPU frequencies for
> iowaiters, in particularly to benefit high frequency I/O. We do the same
> and boost the GPU clocks to try and minimise time spent waiting for the
> GPU. However, as the igfx and CPU share the same TDP, boosting the CPU
> frequency will result in the GPU being throttled and its frequency being
> reduced. Thus declaring iowait negatively impacts on GPU throughput.
>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=107410
> References: 52ccc4314293 ("cpufreq: intel_pstate: HWP boost performance on IO wakeup")

This patch causes up to ~13% performance regressions (with significance
5%) on several latency-sensitive tests on my BXT:

 jxrendermark/rendering-test=Linear Gradient Blend/rendering-size=128x128:     XXX ±35.69% x53 -> XXX ±32.57% x61 d=-13.52% ±31.88% p=2.58%
 jxrendermark/rendering-test=Transformed Blit Bilinear/rendering-size=128x128: XXX ±3.51% x21 ->  XXX ±3.77% x21   d=-12.08% ±3.41% p=0.00%
 gtkperf/gtk-test=GtkComboBox:                                                 XXX ±1.90% x19 ->  XXX ±1.59% x20    d=-4.74% ±1.71% p=0.00%
 x11perf/test=500px Compositing From Pixmap To Window:                         XXX ±2.35% x21 ->  XXX ±1.73% x21    d=-2.69% ±2.04% p=0.01%
 qgears2/render-backend=XRender Extension/test-mode=Text:                      XXX ±0.38% x21 ->  XXX ±0.40% x25    d=-2.20% ±0.38% p=0.00%
 x11perf/test=500px Compositing From Pixmap To Window:                         XXX ±2.78% x53 ->  XXX ±2.27% x61    d=-1.77% ±2.50% p=0.03%

It's unsurprising to see latency-sensitive workloads relying on the
lower latency offered by io_schedule_timeout(), since the CPUFREQ
governor will have substantial downward bias without it, in response to
the intermittent CPU usage pattern of those benchmarks.

We could possibly have the best from both worlds if the CPUFREQ governor
didn't attempt to EPP-boost the CPU frequency on IOWAIT while the system
is heavily IO-bound, since the occurrence of both conditions
simultaneously indicates the CPU workload is also likely to be IO-bound
and its performance will remain unchanged while boosting the CPU
frequency, so it can only pessimize the performance of the system.  This
could be achieved by using the statistic implemented here [1].  I think
the offending patch should probably be reverted for the time being...

[1] https://patchwork.kernel.org/patch/10312259/

> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>
> Cc: Joonas Lahtinen <joonas.lahtinen@linux.intel.com>
> Cc: Eero Tamminen <eero.t.tamminen@intel.com>
> Cc: Francisco Jerez <currojerez@riseup.net>
> ---
>  drivers/gpu/drm/i915/i915_request.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_request.c b/drivers/gpu/drm/i915/i915_request.c
> index 5c2c93cbab12..7ef7ade12073 100644
> --- a/drivers/gpu/drm/i915/i915_request.c
> +++ b/drivers/gpu/drm/i915/i915_request.c
> @@ -1330,7 +1330,7 @@ long i915_request_wait(struct i915_request *rq,
>  			goto complete;
>  		}
>  
> -		timeout = io_schedule_timeout(timeout);
> +		timeout = schedule_timeout(timeout);
>  	} while (1);
>  
>  	GEM_BUG_ON(!intel_wait_has_seqno(&wait));
> -- 
> 2.18.0

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 227 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915: Do not use iowait while waiting for the GPU

[Chris Wilson]

Quoting Francisco Jerez (2018-07-28 06:20:12)
> Chris Wilson <chris@chris-wilson.co.uk> writes:
> 
> > A recent trend for cpufreq is to boost the CPU frequencies for
> > iowaiters, in particularly to benefit high frequency I/O. We do the same
> > and boost the GPU clocks to try and minimise time spent waiting for the
> > GPU. However, as the igfx and CPU share the same TDP, boosting the CPU
> > frequency will result in the GPU being throttled and its frequency being
> > reduced. Thus declaring iowait negatively impacts on GPU throughput.
> >
> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=107410
> > References: 52ccc4314293 ("cpufreq: intel_pstate: HWP boost performance on IO wakeup")
> 
> This patch causes up to ~13% performance regressions (with significance
> 5%) on several latency-sensitive tests on my BXT:
> 
>  jxrendermark/rendering-test=Linear Gradient Blend/rendering-size=128x128:     XXX ±35.69% x53 -> XXX ±32.57% x61 d=-13.52% ±31.88% p=2.58%

Curious, as this is just a bunch of composites and as with the others,
should never be latency sensitive (at least under bare X11).
Fwiw, I double checked this result:

Broxton J3455, jxrend -num $(for i in $(seq 1 100); do echo 12 128; done)
x noio-1.txt
+ io-1.txt
+------------------------------------------------------------------------+
|                 +                                                      |
|                 +                                                      |
|                 *                                                      |
|                +*x                                                     |
|              + +***+                                                   |
|              + +***++                                                  |
|              + ****+*   +                                              |
|             ++x******  x+   x                                          |
|       xx    **x*******+x* xx*                                          |
|   + + xx*xx+***********x**x***x x+                                     |
|x x+** x**x****************x***x***+  x + x x                    ++    +|
|           |_______MA_______|                                           |
|          |________MA__________|                                        |
+------------------------------------------------------------------------+
    N           Min           Max        Median           Avg        Stddev
x 100     16109.095     16211.579     16152.497      16154.87     19.270749
+ 100      16116.47     16274.973     16152.365     16156.954     25.304398
No difference proven at 95.0% confidence

Your variance is much, much higher, are you still using the original
jxrendermark that doesn't wait for rendering completion?
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915: Do not use iowait while waiting for the GPU

[Francisco Jerez]

[-- Attachment #1.1.1: Type: text/plain, Size: 3352 bytes --]

Chris Wilson <chris@chris-wilson.co.uk> writes:

> Quoting Francisco Jerez (2018-07-28 06:20:12)
>> Chris Wilson <chris@chris-wilson.co.uk> writes:
>> 
>> > A recent trend for cpufreq is to boost the CPU frequencies for
>> > iowaiters, in particularly to benefit high frequency I/O. We do the same
>> > and boost the GPU clocks to try and minimise time spent waiting for the
>> > GPU. However, as the igfx and CPU share the same TDP, boosting the CPU
>> > frequency will result in the GPU being throttled and its frequency being
>> > reduced. Thus declaring iowait negatively impacts on GPU throughput.
>> >
>> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=107410
>> > References: 52ccc4314293 ("cpufreq: intel_pstate: HWP boost performance on IO wakeup")
>> 
>> This patch causes up to ~13% performance regressions (with significance
>> 5%) on several latency-sensitive tests on my BXT:
>> 
>>  jxrendermark/rendering-test=Linear Gradient Blend/rendering-size=128x128:     XXX ±35.69% x53 -> XXX ±32.57% x61 d=-13.52% ±31.88% p=2.58%
>

The jxrendermark Linear Gradient Blend test-case had probably the
smallest effect size of all the regressions I noticed...  Can you take a
look at any of the other ones instead?

> Curious, as this is just a bunch of composites and as with the others,
> should never be latency sensitive (at least under bare X11).

They are largely latency-sensitive due to the poor pipelining they seem
to achieve between their GPU rendering work and the X11 thread.

> Fwiw, I double checked this result:
>
> Broxton J3455, jxrend -num $(for i in $(seq 1 100); do echo 12 128; done)
> x noio-1.txt
> + io-1.txt
> +------------------------------------------------------------------------+
> |                 +                                                      |
> |                 +                                                      |
> |                 *                                                      |
> |                +*x                                                     |
> |              + +***+                                                   |
> |              + +***++                                                  |
> |              + ****+*   +                                              |
> |             ++x******  x+   x                                          |
> |       xx    **x*******+x* xx*                                          |
> |   + + xx*xx+***********x**x***x x+                                     |
> |x x+** x**x****************x***x***+  x + x x                    ++    +|
> |           |_______MA_______|                                           |
> |          |________MA__________|                                        |
> +------------------------------------------------------------------------+
>     N           Min           Max        Median           Avg        Stddev
> x 100     16109.095     16211.579     16152.497      16154.87     19.270749
> + 100      16116.47     16274.973     16152.365     16156.954     25.304398
> No difference proven at 95.0% confidence
>
> Your variance is much, much higher, are you still using the original
> jxrendermark that doesn't wait for rendering completion?

I bet, but the other regressing benchmarks shouldn't be affected.

> -Chris

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 227 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915: Do not use iowait while waiting for the GPU

[Chris Wilson]

Quoting Francisco Jerez (2018-07-28 21:18:50)
> Chris Wilson <chris@chris-wilson.co.uk> writes:
> 
> > Quoting Francisco Jerez (2018-07-28 06:20:12)
> >> Chris Wilson <chris@chris-wilson.co.uk> writes:
> >> 
> >> > A recent trend for cpufreq is to boost the CPU frequencies for
> >> > iowaiters, in particularly to benefit high frequency I/O. We do the same
> >> > and boost the GPU clocks to try and minimise time spent waiting for the
> >> > GPU. However, as the igfx and CPU share the same TDP, boosting the CPU
> >> > frequency will result in the GPU being throttled and its frequency being
> >> > reduced. Thus declaring iowait negatively impacts on GPU throughput.
> >> >
> >> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=107410
> >> > References: 52ccc4314293 ("cpufreq: intel_pstate: HWP boost performance on IO wakeup")
> >> 
> >> This patch causes up to ~13% performance regressions (with significance
> >> 5%) on several latency-sensitive tests on my BXT:
> >> 
> >>  jxrendermark/rendering-test=Linear Gradient Blend/rendering-size=128x128:     XXX ±35.69% x53 -> XXX ±32.57% x61 d=-13.52% ±31.88% p=2.58%
> >
> 
> The jxrendermark Linear Gradient Blend test-case had probably the
> smallest effect size of all the regressions I noticed...  Can you take a
> look at any of the other ones instead?

It was the biggest in the list, was it not? I didn't observe anything of
note in a quick look at x11perf, but didn't let it run for a good sample
size. They didn't seem to be as relevant as jxrendermark so I went and
dug that out.
 
> > Curious, as this is just a bunch of composites and as with the others,
> > should never be latency sensitive (at least under bare X11).
> 
> They are largely latency-sensitive due to the poor pipelining they seem
> to achieve between their GPU rendering work and the X11 thread.

Only the X11 thread is touching the GPU, and in the cases I looked at
it, we were either waiting for the ring to drain or on throttling.
Synchronisation with the GPU was only for draining the queue on timing,
and the cpu was able to stay ahead during the benchmark.

Off the top of my head, for X to be latency sensitive you need to mix
client and Xserver rendering, along the lines of Paint; GetImage, in the
extreme becoming gem_sync. Adding a compositor is also interesting for
the context switching will prevent us merging requests (but that all
depends on the frequency of compositor updates ofc), and we would
need more CPU and require reasonably low latency (less than the next
request) to keep the GPU busy. However, that is driven directly off
interrupts, iowait isn't a factor -- but your hook could still be useful
to provide pm_qos.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915: Do not use iowait while waiting for the GPU

[Francisco Jerez]

[-- Attachment #1.1.1: Type: text/plain, Size: 3167 bytes --]

Chris Wilson <chris@chris-wilson.co.uk> writes:

> Quoting Francisco Jerez (2018-07-28 21:18:50)
>> Chris Wilson <chris@chris-wilson.co.uk> writes:
>> 
>> > Quoting Francisco Jerez (2018-07-28 06:20:12)
>> >> Chris Wilson <chris@chris-wilson.co.uk> writes:
>> >> 
>> >> > A recent trend for cpufreq is to boost the CPU frequencies for
>> >> > iowaiters, in particularly to benefit high frequency I/O. We do the same
>> >> > and boost the GPU clocks to try and minimise time spent waiting for the
>> >> > GPU. However, as the igfx and CPU share the same TDP, boosting the CPU
>> >> > frequency will result in the GPU being throttled and its frequency being
>> >> > reduced. Thus declaring iowait negatively impacts on GPU throughput.
>> >> >
>> >> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=107410
>> >> > References: 52ccc4314293 ("cpufreq: intel_pstate: HWP boost performance on IO wakeup")
>> >> 
>> >> This patch causes up to ~13% performance regressions (with significance
>> >> 5%) on several latency-sensitive tests on my BXT:
>> >> 
>> >>  jxrendermark/rendering-test=Linear Gradient Blend/rendering-size=128x128:     XXX ±35.69% x53 -> XXX ±32.57% x61 d=-13.52% ±31.88% p=2.58%
>> >
>> 
>> The jxrendermark Linear Gradient Blend test-case had probably the
>> smallest effect size of all the regressions I noticed...  Can you take a
>> look at any of the other ones instead?
>
> It was the biggest in the list, was it not? I didn't observe anything of
> note in a quick look at x11perf, but didn't let it run for a good sample
> size. They didn't seem to be as relevant as jxrendermark so I went and
> dug that out.
>

That was the biggest regression in absolute value, but the smallest in
effect size (roughly 0.4 standard deviations).

>> > Curious, as this is just a bunch of composites and as with the others,
>> > should never be latency sensitive (at least under bare X11).
>> 
>> They are largely latency-sensitive due to the poor pipelining they seem
>> to achieve between their GPU rendering work and the X11 thread.
>
> Only the X11 thread is touching the GPU, and in the cases I looked at
> it, we were either waiting for the ring to drain or on throttling.
> Synchronisation with the GPU was only for draining the queue on timing,
> and the cpu was able to stay ahead during the benchmark.
>

Apparently the CPU doesn't get ahead enough for the GPU to be
consistently loaded, which prevents us from hiding the latency of the
CPU computation even in those cases.

> Off the top of my head, for X to be latency sensitive you need to mix
> client and Xserver rendering, along the lines of Paint; GetImage, in the
> extreme becoming gem_sync. Adding a compositor is also interesting for
> the context switching will prevent us merging requests (but that all
> depends on the frequency of compositor updates ofc), and we would
> need more CPU and require reasonably low latency (less than the next
> request) to keep the GPU busy. However, that is driven directly off
> interrupts, iowait isn't a factor -- but your hook could still be useful
> to provide pm_qos.
> -Chris

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 227 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915: Do not use iowait while waiting for the GPU

[Chris Wilson]

Quoting Francisco Jerez (2018-07-29 20:29:42)
> Chris Wilson <chris@chris-wilson.co.uk> writes:
> 
> > Quoting Francisco Jerez (2018-07-28 21:18:50)
> >> Chris Wilson <chris@chris-wilson.co.uk> writes:
> >> 
> >> > Quoting Francisco Jerez (2018-07-28 06:20:12)
> >> >> Chris Wilson <chris@chris-wilson.co.uk> writes:
> >> >> 
> >> >> > A recent trend for cpufreq is to boost the CPU frequencies for
> >> >> > iowaiters, in particularly to benefit high frequency I/O. We do the same
> >> >> > and boost the GPU clocks to try and minimise time spent waiting for the
> >> >> > GPU. However, as the igfx and CPU share the same TDP, boosting the CPU
> >> >> > frequency will result in the GPU being throttled and its frequency being
> >> >> > reduced. Thus declaring iowait negatively impacts on GPU throughput.
> >> >> >
> >> >> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=107410
> >> >> > References: 52ccc4314293 ("cpufreq: intel_pstate: HWP boost performance on IO wakeup")
> >> >> 
> >> >> This patch causes up to ~13% performance regressions (with significance
> >> >> 5%) on several latency-sensitive tests on my BXT:
> >> >> 
> >> >>  jxrendermark/rendering-test=Linear Gradient Blend/rendering-size=128x128:     XXX ±35.69% x53 -> XXX ±32.57% x61 d=-13.52% ±31.88% p=2.58%
> >> >
> >> 
> >> The jxrendermark Linear Gradient Blend test-case had probably the
> >> smallest effect size of all the regressions I noticed...  Can you take a
> >> look at any of the other ones instead?
> >
> > It was the biggest in the list, was it not? I didn't observe anything of
> > note in a quick look at x11perf, but didn't let it run for a good sample
> > size. They didn't seem to be as relevant as jxrendermark so I went and
> > dug that out.
> >
> 
> That was the biggest regression in absolute value, but the smallest in
> effect size (roughly 0.4 standard deviations).

d=-13.52% wasn't the delta between the two runs?

Sorry, but it appears to be redacted beyond my comprehension.

> >> > Curious, as this is just a bunch of composites and as with the others,
> >> > should never be latency sensitive (at least under bare X11).
> >> 
> >> They are largely latency-sensitive due to the poor pipelining they seem
> >> to achieve between their GPU rendering work and the X11 thread.
> >
> > Only the X11 thread is touching the GPU, and in the cases I looked at
> > it, we were either waiting for the ring to drain or on throttling.
> > Synchronisation with the GPU was only for draining the queue on timing,
> > and the cpu was able to stay ahead during the benchmark.
> >
> 
> Apparently the CPU doesn't get ahead enough for the GPU to be
> consistently loaded, which prevents us from hiding the latency of the
> CPU computation even in those cases.

The curse of reproducibility. On my bxt, I don't see the issue, so we
have a significant difference in setup.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915: Do not use iowait while waiting for the GPU

[Francisco Jerez]

[-- Attachment #1.1.1: Type: text/plain, Size: 3133 bytes --]

Chris Wilson <chris@chris-wilson.co.uk> writes:

> Quoting Francisco Jerez (2018-07-29 20:29:42)
>> Chris Wilson <chris@chris-wilson.co.uk> writes:
>> 
>> > Quoting Francisco Jerez (2018-07-28 21:18:50)
>> >> Chris Wilson <chris@chris-wilson.co.uk> writes:
>> >> 
>> >> > Quoting Francisco Jerez (2018-07-28 06:20:12)
>> >> >> Chris Wilson <chris@chris-wilson.co.uk> writes:
>> >> >> 
>> >> >> > A recent trend for cpufreq is to boost the CPU frequencies for
>> >> >> > iowaiters, in particularly to benefit high frequency I/O. We do the same
>> >> >> > and boost the GPU clocks to try and minimise time spent waiting for the
>> >> >> > GPU. However, as the igfx and CPU share the same TDP, boosting the CPU
>> >> >> > frequency will result in the GPU being throttled and its frequency being
>> >> >> > reduced. Thus declaring iowait negatively impacts on GPU throughput.
>> >> >> >
>> >> >> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=107410
>> >> >> > References: 52ccc4314293 ("cpufreq: intel_pstate: HWP boost performance on IO wakeup")
>> >> >> 
>> >> >> This patch causes up to ~13% performance regressions (with significance
>> >> >> 5%) on several latency-sensitive tests on my BXT:
>> >> >> 
>> >> >>  jxrendermark/rendering-test=Linear Gradient Blend/rendering-size=128x128:     XXX ±35.69% x53 -> XXX ±32.57% x61 d=-13.52% ±31.88% p=2.58%
>> >> >
>> >> 
>> >> The jxrendermark Linear Gradient Blend test-case had probably the
>> >> smallest effect size of all the regressions I noticed...  Can you take a
>> >> look at any of the other ones instead?
>> >
>> > It was the biggest in the list, was it not? I didn't observe anything of
>> > note in a quick look at x11perf, but didn't let it run for a good sample
>> > size. They didn't seem to be as relevant as jxrendermark so I went and
>> > dug that out.
>> >
>> 
>> That was the biggest regression in absolute value, but the smallest in
>> effect size (roughly 0.4 standard deviations).
>
> d=-13.52% wasn't the delta between the two runs?
>

It is, less than half of 31.88% which is the pooled standard deviation.

> Sorry, but it appears to be redacted beyond my comprehension.
>
>> >> > Curious, as this is just a bunch of composites and as with the others,
>> >> > should never be latency sensitive (at least under bare X11).
>> >> 
>> >> They are largely latency-sensitive due to the poor pipelining they seem
>> >> to achieve between their GPU rendering work and the X11 thread.
>> >
>> > Only the X11 thread is touching the GPU, and in the cases I looked at
>> > it, we were either waiting for the ring to drain or on throttling.
>> > Synchronisation with the GPU was only for draining the queue on timing,
>> > and the cpu was able to stay ahead during the benchmark.
>> >
>> 
>> Apparently the CPU doesn't get ahead enough for the GPU to be
>> consistently loaded, which prevents us from hiding the latency of the
>> CPU computation even in those cases.
>
> The curse of reproducibility. On my bxt, I don't see the issue, so we
> have a significant difference in setup.
> -Chris

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 227 bytes --]

[-- Attachment #2: Type: text/plain, Size: 160 bytes --]

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915: Do not use iowait while waiting for the GPU

[Chris Wilson]

Quoting Francisco Jerez (2018-07-28 06:20:12)
> Chris Wilson <chris@chris-wilson.co.uk> writes:
> 
> > A recent trend for cpufreq is to boost the CPU frequencies for
> > iowaiters, in particularly to benefit high frequency I/O. We do the same
> > and boost the GPU clocks to try and minimise time spent waiting for the
> > GPU. However, as the igfx and CPU share the same TDP, boosting the CPU
> > frequency will result in the GPU being throttled and its frequency being
> > reduced. Thus declaring iowait negatively impacts on GPU throughput.
> >
> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=107410
> > References: 52ccc4314293 ("cpufreq: intel_pstate: HWP boost performance on IO wakeup")
> 
> This patch causes up to ~13% performance regressions (with significance
> 5%) on several latency-sensitive tests on my BXT:
> 
>  jxrendermark/rendering-test=Linear Gradient Blend/rendering-size=128x128:     XXX ±35.69% x53 -> XXX ±32.57% x61 d=-13.52% ±31.88% p=2.58%
>  jxrendermark/rendering-test=Transformed Blit Bilinear/rendering-size=128x128: XXX ±3.51% x21 ->  XXX ±3.77% x21   d=-12.08% ±3.41% p=0.00%
>  gtkperf/gtk-test=GtkComboBox:                                                 XXX ±1.90% x19 ->  XXX ±1.59% x20    d=-4.74% ±1.71% p=0.00%
>  x11perf/test=500px Compositing From Pixmap To Window:                         XXX ±2.35% x21 ->  XXX ±1.73% x21    d=-2.69% ±2.04% p=0.01%
>  qgears2/render-backend=XRender Extension/test-mode=Text:                      XXX ±0.38% x21 ->  XXX ±0.40% x25    d=-2.20% ±0.38% p=0.00%
>  x11perf/test=500px Compositing From Pixmap To Window:                         XXX ±2.78% x53 ->  XXX ±2.27% x61    d=-1.77% ±2.50% p=0.03%
> 
> It's unsurprising to see latency-sensitive workloads relying on the
> lower latency offered by io_schedule_timeout(), since the CPUFREQ
> governor will have substantial downward bias without it, in response to
> the intermittent CPU usage pattern of those benchmarks.

Fwiw, I have a better example, gem_sync --run store-default. This test
waits on a short batch,

with io_schedule_timeout:
Completed 987136 cycles: 152.092 us

with schedule_timeout:
Completed 157696 cycles: 956.403 us

Though note that for a no-op batch, we see no difference as the sleep is
short enough, both take on average 52us. But microbenchmarks be micro.
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx