[Intel-gfx] [PATCH] drm/i915/gt: Include a tell-tale for engine parking

[Intel-gfx] [PATCH] drm/i915/gt: Include a tell-tale for engine parking

[Chris Wilson]

We have two trace messages that rely on the function name for
distinction. However, if gcc inlines the function, the two traces end up
with the same function name and are indistinguishable. Add a different
message to each to clarify which one we hit, i.e. which phase of engine
parking we are processing.

Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/gt/intel_engine_pm.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/gt/intel_engine_pm.c b/drivers/gpu/drm/i915/gt/intel_engine_pm.c
index ea90ab3e396e..b6cf284e3a2d 100644
--- a/drivers/gpu/drm/i915/gt/intel_engine_pm.c
+++ b/drivers/gpu/drm/i915/gt/intel_engine_pm.c
@@ -112,7 +112,7 @@ __queue_and_release_pm(struct i915_request *rq,
 {
 	struct intel_gt_timelines *timelines = &engine->gt->timelines;
 
-	ENGINE_TRACE(engine, "\n");
+	ENGINE_TRACE(engine, "parking\n");
 
 	/*
 	 * We have to serialise all potential retirement paths with our
@@ -249,7 +249,7 @@ static int __engine_park(struct intel_wakeref *wf)
 	if (!switch_to_kernel_context(engine))
 		return -EBUSY;
 
-	ENGINE_TRACE(engine, "\n");
+	ENGINE_TRACE(engine, "parked\n");
 
 	call_idle_barriers(engine); /* cleanup after wedging */
 
-- 
2.25.0

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

[Intel-gfx] ✗ Fi.CI.BAT: failure for drm/i915/gt: Include a tell-tale for engine parking

[Patchwork]

== Series Details ==

Series: drm/i915/gt: Include a tell-tale for engine parking
URL   : https://patchwork.freedesktop.org/series/72392/
State : failure

== Summary ==

CI Bug Log - changes from CI_DRM_7790 -> Patchwork_16209
====================================================

Summary
-------

  **FAILURE**

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

  External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16209/index.html

Possible new issues
-------------------

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

### IGT changes ###

#### Possible regressions ####

  * igt@i915_selftest@live_workarounds:
    - fi-skl-6600u:       NOTRUN -> [DMESG-FAIL][1]
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16209/fi-skl-6600u/igt@i915_selftest@live_workarounds.html

  
Known issues
------------

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

### IGT changes ###

#### Issues hit ####

  * igt@gem_exec_suspend@basic-s0:
    - fi-skl-6700k2:      [PASS][2] -> [FAIL][3] ([fdo#103375]) +1 similar issue
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7790/fi-skl-6700k2/igt@gem_exec_suspend@basic-s0.html
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16209/fi-skl-6700k2/igt@gem_exec_suspend@basic-s0.html

  * igt@gem_exec_suspend@basic-s4-devices:
    - fi-skl-6700k2:      [PASS][4] -> [INCOMPLETE][5] ([i915#69])
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7790/fi-skl-6700k2/igt@gem_exec_suspend@basic-s4-devices.html
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16209/fi-skl-6700k2/igt@gem_exec_suspend@basic-s4-devices.html

  * igt@i915_selftest@live_blt:
    - fi-bsw-n3050:       [PASS][6] -> [DMESG-FAIL][7] ([i915#723])
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7790/fi-bsw-n3050/igt@i915_selftest@live_blt.html
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16209/fi-bsw-n3050/igt@i915_selftest@live_blt.html
    - fi-ivb-3770:        [PASS][8] -> [DMESG-FAIL][9] ([i915#725])
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7790/fi-ivb-3770/igt@i915_selftest@live_blt.html
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16209/fi-ivb-3770/igt@i915_selftest@live_blt.html

  * igt@kms_chamelium@dp-hpd-fast:
    - fi-icl-u2:          [PASS][10] -> [DMESG-WARN][11] ([i915#289])
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7790/fi-icl-u2/igt@kms_chamelium@dp-hpd-fast.html
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16209/fi-icl-u2/igt@kms_chamelium@dp-hpd-fast.html

  
#### Possible fixes ####

  * igt@gem_close_race@basic-threads:
    - fi-byt-n2820:       [INCOMPLETE][12] ([i915#45]) -> [PASS][13]
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7790/fi-byt-n2820/igt@gem_close_race@basic-threads.html
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16209/fi-byt-n2820/igt@gem_close_race@basic-threads.html

  * igt@i915_module_load@reload-with-fault-injection:
    - fi-skl-lmem:        [INCOMPLETE][14] ([i915#671]) -> [PASS][15]
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7790/fi-skl-lmem/igt@i915_module_load@reload-with-fault-injection.html
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16209/fi-skl-lmem/igt@i915_module_load@reload-with-fault-injection.html

  * igt@i915_selftest@live_blt:
    - fi-hsw-peppy:       [DMESG-FAIL][16] ([i915#553] / [i915#725]) -> [PASS][17]
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7790/fi-hsw-peppy/igt@i915_selftest@live_blt.html
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16209/fi-hsw-peppy/igt@i915_selftest@live_blt.html

  * igt@i915_selftest@live_gem_contexts:
    - fi-cfl-8700k:       [INCOMPLETE][18] ([i915#424]) -> [PASS][19]
   [18]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7790/fi-cfl-8700k/igt@i915_selftest@live_gem_contexts.html
   [19]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16209/fi-cfl-8700k/igt@i915_selftest@live_gem_contexts.html

  * igt@kms_frontbuffer_tracking@basic:
    - fi-hsw-peppy:       [DMESG-WARN][20] ([i915#44]) -> [PASS][21]
   [20]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7790/fi-hsw-peppy/igt@kms_frontbuffer_tracking@basic.html
   [21]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16209/fi-hsw-peppy/igt@kms_frontbuffer_tracking@basic.html

  * igt@kms_pipe_crc_basic@suspend-read-crc-pipe-a:
    - fi-icl-dsi:         [DMESG-WARN][22] ([i915#109]) -> [PASS][23]
   [22]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7790/fi-icl-dsi/igt@kms_pipe_crc_basic@suspend-read-crc-pipe-a.html
   [23]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16209/fi-icl-dsi/igt@kms_pipe_crc_basic@suspend-read-crc-pipe-a.html

  
#### Warnings ####

  * igt@i915_selftest@live_blt:
    - fi-hsw-4770r:       [DMESG-FAIL][24] ([i915#770]) -> [DMESG-FAIL][25] ([i915#553] / [i915#725])
   [24]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7790/fi-hsw-4770r/igt@i915_selftest@live_blt.html
   [25]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_16209/fi-hsw-4770r/igt@i915_selftest@live_blt.html

  
  [fdo#103375]: https://bugs.freedesktop.org/show_bug.cgi?id=103375
  [i915#109]: https://gitlab.freedesktop.org/drm/intel/issues/109
  [i915#289]: https://gitlab.freedesktop.org/drm/intel/issues/289
  [i915#424]: https://gitlab.freedesktop.org/drm/intel/issues/424
  [i915#44]: https://gitlab.freedesktop.org/drm/intel/issues/44
  [i915#45]: https://gitlab.freedesktop.org/drm/intel/issues/45
  [i915#553]: https://gitlab.freedesktop.org/drm/intel/issues/553
  [i915#671]: https://gitlab.freedesktop.org/drm/intel/issues/671
  [i915#69]: https://gitlab.freedesktop.org/drm/intel/issues/69
  [i915#723]: https://gitlab.freedesktop.org/drm/intel/issues/723
  [i915#725]: https://gitlab.freedesktop.org/drm/intel/issues/725
  [i915#770]: https://gitlab.freedesktop.org/drm/intel/issues/770


Participating hosts (39 -> 38)
------------------------------

  Additional (5): fi-glk-dsi fi-cfl-guc fi-kbl-guc fi-icl-u3 fi-skl-6600u 
  Missing    (6): fi-kbl-soraka fi-cml-u2 fi-byt-squawks fi-bsw-cyan fi-elk-e7500 fi-snb-2600 


Build changes
-------------

  * CI: CI-20190529 -> None
  * Linux: CI_DRM_7790 -> Patchwork_16209

  CI-20190529: 20190529
  CI_DRM_7790: 854ee1f2aa71a3aad01bd4e45f9e9c4cf9ad47f9 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_5377: 1e6cb3e75925cf623df04f78430ae9299632ec3f @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_16209: 71554eb2501eead4bbd9b147bbd7b98f9b3700d1 @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

71554eb2501e drm/i915/gt: Include a tell-tale for engine parking

== Logs ==

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

Re: [Intel-gfx] [PATCH] drm/i915/gt: Include a tell-tale for engine parking

[Tvrtko Ursulin]

On 22/01/2020 12:41, Chris Wilson wrote:
> We have two trace messages that rely on the function name for
> distinction. However, if gcc inlines the function, the two traces end up
> with the same function name and are indistinguishable. Add a different
> message to each to clarify which one we hit, i.e. which phase of engine
> parking we are processing.
> 
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> ---
>   drivers/gpu/drm/i915/gt/intel_engine_pm.c | 4 ++--
>   1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/gt/intel_engine_pm.c b/drivers/gpu/drm/i915/gt/intel_engine_pm.c
> index ea90ab3e396e..b6cf284e3a2d 100644
> --- a/drivers/gpu/drm/i915/gt/intel_engine_pm.c
> +++ b/drivers/gpu/drm/i915/gt/intel_engine_pm.c
> @@ -112,7 +112,7 @@ __queue_and_release_pm(struct i915_request *rq,
>   {
>   	struct intel_gt_timelines *timelines = &engine->gt->timelines;
>   
> -	ENGINE_TRACE(engine, "\n");
> +	ENGINE_TRACE(engine, "parking\n");
>   
>   	/*
>   	 * We have to serialise all potential retirement paths with our
> @@ -249,7 +249,7 @@ static int __engine_park(struct intel_wakeref *wf)
>   	if (!switch_to_kernel_context(engine))
>   		return -EBUSY;
>   
> -	ENGINE_TRACE(engine, "\n");
> +	ENGINE_TRACE(engine, "parked\n");
>   
>   	call_idle_barriers(engine); /* cleanup after wedging */
>   
> 

Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>

Regards,

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

Re: [Intel-gfx] [PATCH] drm/i915/gt: Include a tell-tale for engine parking

[Mika Kuoppala]

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

> We have two trace messages that rely on the function name for
> distinction. However, if gcc inlines the function, the two traces end up
> with the same function name and are indistinguishable. Add a different
> message to each to clarify which one we hit, i.e. which phase of engine
> parking we are processing.
>
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> ---
>  drivers/gpu/drm/i915/gt/intel_engine_pm.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gt/intel_engine_pm.c b/drivers/gpu/drm/i915/gt/intel_engine_pm.c
> index ea90ab3e396e..b6cf284e3a2d 100644
> --- a/drivers/gpu/drm/i915/gt/intel_engine_pm.c
> +++ b/drivers/gpu/drm/i915/gt/intel_engine_pm.c
> @@ -112,7 +112,7 @@ __queue_and_release_pm(struct i915_request *rq,
>  {
>  	struct intel_gt_timelines *timelines = &engine->gt->timelines;
>  
> -	ENGINE_TRACE(engine, "\n");
> +	ENGINE_TRACE(engine, "parking\n");
>  
>  	/*
>  	 * We have to serialise all potential retirement paths with our
> @@ -249,7 +249,7 @@ static int __engine_park(struct intel_wakeref *wf)
>  	if (!switch_to_kernel_context(engine))
>  		return -EBUSY;
>  
> -	ENGINE_TRACE(engine, "\n");
> +	ENGINE_TRACE(engine, "parked\n");

Reading the functions, the exact spots are a mystery for me still
as of why in these exact lines. Like the 'parked' would mean it
is parked already, which it seems not to.

However, what comes to the commit message and to
immediate problem and fixing it,

Reviewed-by: Mika Kuoppala <mika.kuoppala@linux.intel.com>

>  
>  	call_idle_barriers(engine); /* cleanup after wedging */
>  
> -- 
> 2.25.0
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH] drm/i915/gt: Include a tell-tale for engine parking

[Chris Wilson]

Quoting Mika Kuoppala (2020-01-22 14:39:19)
> Chris Wilson <chris@chris-wilson.co.uk> writes:
> 
> > We have two trace messages that rely on the function name for
> > distinction. However, if gcc inlines the function, the two traces end up
> > with the same function name and are indistinguishable. Add a different
> > message to each to clarify which one we hit, i.e. which phase of engine
> > parking we are processing.
> >
> > Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
> > ---
> >  drivers/gpu/drm/i915/gt/intel_engine_pm.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/gt/intel_engine_pm.c b/drivers/gpu/drm/i915/gt/intel_engine_pm.c
> > index ea90ab3e396e..b6cf284e3a2d 100644
> > --- a/drivers/gpu/drm/i915/gt/intel_engine_pm.c
> > +++ b/drivers/gpu/drm/i915/gt/intel_engine_pm.c
> > @@ -112,7 +112,7 @@ __queue_and_release_pm(struct i915_request *rq,
> >  {
> >       struct intel_gt_timelines *timelines = &engine->gt->timelines;
> >  
> > -     ENGINE_TRACE(engine, "\n");
> > +     ENGINE_TRACE(engine, "parking\n");
> >  
> >       /*
> >        * We have to serialise all potential retirement paths with our
> > @@ -249,7 +249,7 @@ static int __engine_park(struct intel_wakeref *wf)
> >       if (!switch_to_kernel_context(engine))
> >               return -EBUSY;
> >  
> > -     ENGINE_TRACE(engine, "\n");
> > +     ENGINE_TRACE(engine, "parked\n");
> 
> Reading the functions, the exact spots are a mystery for me still
> as of why in these exact lines. Like the 'parked' would mean it
> is parked already, which it seems not to.

True, but 'park' was too similar to 'parking', whereas 'parked' at least
conveyed that 'parking' was over, which was the important concept that I
wanted clarity over in the debug traces.

They are mere labels, if you have a better idea, ... :)
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx