intel-gfx.lists.freedesktop.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] drm/i915/guc: Skip suspend/resume GuC action on platforms w/o GuC submission
@ 2019-10-28 21:25 don.hiatt
  2019-10-28 21:25 ` [Intel-gfx] " don.hiatt
                   ` (3 more replies)
  0 siblings, 4 replies; 20+ messages in thread
From: don.hiatt @ 2019-10-28 21:25 UTC (permalink / raw)
  To: intel-gfx

From: Don Hiatt <don.hiatt@intel.com>

On some platforms (e.g. KBL) that do not support GuC submission, but
the user enabled the GuC communication (e.g for HuC authentication)
calling the GuC EXIT_S_STATE action results in lose of ability to
enter RC6. We can remove the GuC suspend/remove entirely as we do
not need to save the GuC submission status.

v2: Do not suspend/resume the GuC on platforms that do not support
    Guc Submission.

Signed-off-by: Don Hiatt <don.hiatt@intel.com>
---
 drivers/gpu/drm/i915/gt/uc/intel_uc.c | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/drivers/gpu/drm/i915/gt/uc/intel_uc.c b/drivers/gpu/drm/i915/gt/uc/intel_uc.c
index 3fdbc935d155..04031564f0b1 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_uc.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_uc.c
@@ -572,10 +572,19 @@ void intel_uc_runtime_suspend(struct intel_uc *uc)
 	if (!intel_guc_is_running(guc))
 		return;
 
+	/*
+	 * If GuC communciation is enabled but submission is not supported,
+	 * we do not need to suspend the GuC but we do need to disable the
+	 * GuC communication on suspend.
+	 */
+	if (!guc->submission_supported)
+		goto guc_disable_comm;
+
 	err = intel_guc_suspend(guc);
 	if (err)
 		DRM_DEBUG_DRIVER("Failed to suspend GuC, err=%d", err);
 
+guc_disable_comm:
 	guc_disable_communication(guc);
 }
 
@@ -605,6 +614,14 @@ static int __uc_resume(struct intel_uc *uc, bool enable_communication)
 	if (enable_communication)
 		guc_enable_communication(guc);
 
+	/*
+	 * If GuC communciation is enabled but submission is not supported,
+	 * we do not need to resume the GuC but we do need to enable the
+	 * GuC communication on resume (above).
+	 */
+	if (!guc->submission_supported)
+		return 0;
+
 	err = intel_guc_resume(guc);
 	if (err) {
 		DRM_DEBUG_DRIVER("Failed to resume GuC, err=%d", err);
-- 
2.20.1

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

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [Intel-gfx] [PATCH] drm/i915/guc: Skip suspend/resume GuC action on platforms w/o GuC submission
  2019-10-28 21:25 [PATCH] drm/i915/guc: Skip suspend/resume GuC action on platforms w/o GuC submission don.hiatt
@ 2019-10-28 21:25 ` don.hiatt
  2019-10-29  0:58 ` ✗ Fi.CI.BAT: failure for " Patchwork
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 20+ messages in thread
From: don.hiatt @ 2019-10-28 21:25 UTC (permalink / raw)
  To: intel-gfx

From: Don Hiatt <don.hiatt@intel.com>

On some platforms (e.g. KBL) that do not support GuC submission, but
the user enabled the GuC communication (e.g for HuC authentication)
calling the GuC EXIT_S_STATE action results in lose of ability to
enter RC6. We can remove the GuC suspend/remove entirely as we do
not need to save the GuC submission status.

v2: Do not suspend/resume the GuC on platforms that do not support
    Guc Submission.

Signed-off-by: Don Hiatt <don.hiatt@intel.com>
---
 drivers/gpu/drm/i915/gt/uc/intel_uc.c | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/drivers/gpu/drm/i915/gt/uc/intel_uc.c b/drivers/gpu/drm/i915/gt/uc/intel_uc.c
index 3fdbc935d155..04031564f0b1 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_uc.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_uc.c
@@ -572,10 +572,19 @@ void intel_uc_runtime_suspend(struct intel_uc *uc)
 	if (!intel_guc_is_running(guc))
 		return;
 
+	/*
+	 * If GuC communciation is enabled but submission is not supported,
+	 * we do not need to suspend the GuC but we do need to disable the
+	 * GuC communication on suspend.
+	 */
+	if (!guc->submission_supported)
+		goto guc_disable_comm;
+
 	err = intel_guc_suspend(guc);
 	if (err)
 		DRM_DEBUG_DRIVER("Failed to suspend GuC, err=%d", err);
 
+guc_disable_comm:
 	guc_disable_communication(guc);
 }
 
@@ -605,6 +614,14 @@ static int __uc_resume(struct intel_uc *uc, bool enable_communication)
 	if (enable_communication)
 		guc_enable_communication(guc);
 
+	/*
+	 * If GuC communciation is enabled but submission is not supported,
+	 * we do not need to resume the GuC but we do need to enable the
+	 * GuC communication on resume (above).
+	 */
+	if (!guc->submission_supported)
+		return 0;
+
 	err = intel_guc_resume(guc);
 	if (err) {
 		DRM_DEBUG_DRIVER("Failed to resume GuC, err=%d", err);
-- 
2.20.1

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

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* ✗ Fi.CI.BAT: failure for drm/i915/guc: Skip suspend/resume GuC action on platforms w/o GuC submission
  2019-10-28 21:25 [PATCH] drm/i915/guc: Skip suspend/resume GuC action on platforms w/o GuC submission don.hiatt
  2019-10-28 21:25 ` [Intel-gfx] " don.hiatt
@ 2019-10-29  0:58 ` Patchwork
  2019-10-29  0:58   ` [Intel-gfx] " Patchwork
  2019-10-29 12:33 ` [PATCH] " Michal Wajdeczko
  2019-10-30  7:53 ` ✗ Fi.CI.BUILD: failure for drm/i915/guc: Skip suspend/resume GuC action on platforms w/o GuC submission (rev2) Patchwork
  3 siblings, 1 reply; 20+ messages in thread
From: Patchwork @ 2019-10-29  0:58 UTC (permalink / raw)
  To: don.hiatt; +Cc: intel-gfx

== Series Details ==

Series: drm/i915/guc: Skip suspend/resume GuC action on platforms w/o GuC submission
URL   : https://patchwork.freedesktop.org/series/68685/
State : failure

== Summary ==

CI Bug Log - changes from CI_DRM_7206 -> Patchwork_15037
====================================================

Summary
-------

  **FAILURE**

  Serious unknown changes coming with Patchwork_15037 absolutely need to be
  verified manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in Patchwork_15037, 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_15037/index.html

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

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

### IGT changes ###

#### Possible regressions ####

  * igt@i915_selftest@live_blt:
    - fi-kbl-8809g:       [PASS][1] -> [TIMEOUT][2]
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7206/fi-kbl-8809g/igt@i915_selftest@live_blt.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15037/fi-kbl-8809g/igt@i915_selftest@live_blt.html
    - fi-bxt-dsi:         NOTRUN -> [TIMEOUT][3]
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15037/fi-bxt-dsi/igt@i915_selftest@live_blt.html
    - fi-skl-lmem:        [PASS][4] -> [TIMEOUT][5]
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7206/fi-skl-lmem/igt@i915_selftest@live_blt.html
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15037/fi-skl-lmem/igt@i915_selftest@live_blt.html
    - fi-cfl-8700k:       [PASS][6] -> [TIMEOUT][7]
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7206/fi-cfl-8700k/igt@i915_selftest@live_blt.html
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15037/fi-cfl-8700k/igt@i915_selftest@live_blt.html
    - fi-skl-guc:         [PASS][8] -> [TIMEOUT][9]
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7206/fi-skl-guc/igt@i915_selftest@live_blt.html
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15037/fi-skl-guc/igt@i915_selftest@live_blt.html
    - fi-cfl-guc:         [PASS][10] -> [TIMEOUT][11]
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7206/fi-cfl-guc/igt@i915_selftest@live_blt.html
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15037/fi-cfl-guc/igt@i915_selftest@live_blt.html
    - fi-skl-iommu:       [PASS][12] -> [TIMEOUT][13]
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7206/fi-skl-iommu/igt@i915_selftest@live_blt.html
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15037/fi-skl-iommu/igt@i915_selftest@live_blt.html

  * igt@runner@aborted:
    - fi-bxt-dsi:         NOTRUN -> [FAIL][14]
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15037/fi-bxt-dsi/igt@runner@aborted.html
    - fi-cfl-8700k:       NOTRUN -> [FAIL][15]
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15037/fi-cfl-8700k/igt@runner@aborted.html
    - fi-kbl-8809g:       NOTRUN -> [FAIL][16]
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15037/fi-kbl-8809g/igt@runner@aborted.html

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

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

### IGT changes ###

#### Issues hit ####

  * igt@gem_flink_basic@flink-lifetime:
    - fi-icl-u3:          [PASS][17] -> [DMESG-WARN][18] ([fdo#107724]) +1 similar issue
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7206/fi-icl-u3/igt@gem_flink_basic@flink-lifetime.html
   [18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15037/fi-icl-u3/igt@gem_flink_basic@flink-lifetime.html

  
#### Possible fixes ####

  * igt@gem_ctx_create@basic-files:
    - {fi-icl-dsi}:       [INCOMPLETE][19] ([fdo#107713] / [fdo#109100]) -> [PASS][20]
   [19]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7206/fi-icl-dsi/igt@gem_ctx_create@basic-files.html
   [20]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15037/fi-icl-dsi/igt@gem_ctx_create@basic-files.html

  * igt@gem_ctx_switch@legacy-render:
    - fi-bxt-dsi:         [INCOMPLETE][21] ([fdo#103927] / [fdo#111381]) -> [PASS][22]
   [21]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7206/fi-bxt-dsi/igt@gem_ctx_switch@legacy-render.html
   [22]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15037/fi-bxt-dsi/igt@gem_ctx_switch@legacy-render.html

  * igt@gem_mmap_gtt@basic-read-write-distinct:
    - fi-icl-u3:          [DMESG-WARN][23] ([fdo#107724]) -> [PASS][24] +2 similar issues
   [23]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7206/fi-icl-u3/igt@gem_mmap_gtt@basic-read-write-distinct.html
   [24]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15037/fi-icl-u3/igt@gem_mmap_gtt@basic-read-write-distinct.html

  * igt@i915_selftest@live_blt:
    - fi-kbl-r:           [TIMEOUT][25] -> [PASS][26]
   [25]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7206/fi-kbl-r/igt@i915_selftest@live_blt.html
   [26]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15037/fi-kbl-r/igt@i915_selftest@live_blt.html
    - fi-icl-u2:          [TIMEOUT][27] -> [PASS][28]
   [27]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7206/fi-icl-u2/igt@i915_selftest@live_blt.html
   [28]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15037/fi-icl-u2/igt@i915_selftest@live_blt.html
    - {fi-icl-u4}:        [TIMEOUT][29] -> [PASS][30]
   [29]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7206/fi-icl-u4/igt@i915_selftest@live_blt.html
   [30]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15037/fi-icl-u4/igt@i915_selftest@live_blt.html

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

  [fdo#103927]: https://bugs.freedesktop.org/show_bug.cgi?id=103927
  [fdo#107713]: https://bugs.freedesktop.org/show_bug.cgi?id=107713
  [fdo#107724]: https://bugs.freedesktop.org/show_bug.cgi?id=107724
  [fdo#109100]: https://bugs.freedesktop.org/show_bug.cgi?id=109100
  [fdo#110566]: https://bugs.freedesktop.org/show_bug.cgi?id=110566
  [fdo#111381]: https://bugs.freedesktop.org/show_bug.cgi?id=111381


Participating hosts (50 -> 42)
------------------------------

  Missing    (8): fi-ilk-m540 fi-hsw-4200u fi-byt-squawks fi-bsw-cyan fi-apl-guc fi-icl-y fi-byt-clapper fi-bdw-samus 


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

  * CI: CI-20190529 -> None
  * Linux: CI_DRM_7206 -> Patchwork_15037

  CI-20190529: 20190529
  CI_DRM_7206: 197ff4c24c6762ac5bbd8c5364d6f840fb9929c5 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_5248: 81e55f1f97d73e48f00caa7e4fb98295023c5afa @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_15037: bacfbda2a742316927daa8d7054efce567a83b45 @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

bacfbda2a742 drm/i915/guc: Skip suspend/resume GuC action on platforms w/o GuC submission

== Logs ==

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

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [Intel-gfx] ✗ Fi.CI.BAT: failure for drm/i915/guc: Skip suspend/resume GuC action on platforms w/o GuC submission
  2019-10-29  0:58 ` ✗ Fi.CI.BAT: failure for " Patchwork
@ 2019-10-29  0:58   ` Patchwork
  0 siblings, 0 replies; 20+ messages in thread
From: Patchwork @ 2019-10-29  0:58 UTC (permalink / raw)
  To: don.hiatt; +Cc: intel-gfx

== Series Details ==

Series: drm/i915/guc: Skip suspend/resume GuC action on platforms w/o GuC submission
URL   : https://patchwork.freedesktop.org/series/68685/
State : failure

== Summary ==

CI Bug Log - changes from CI_DRM_7206 -> Patchwork_15037
====================================================

Summary
-------

  **FAILURE**

  Serious unknown changes coming with Patchwork_15037 absolutely need to be
  verified manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in Patchwork_15037, 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_15037/index.html

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

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

### IGT changes ###

#### Possible regressions ####

  * igt@i915_selftest@live_blt:
    - fi-kbl-8809g:       [PASS][1] -> [TIMEOUT][2]
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7206/fi-kbl-8809g/igt@i915_selftest@live_blt.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15037/fi-kbl-8809g/igt@i915_selftest@live_blt.html
    - fi-bxt-dsi:         NOTRUN -> [TIMEOUT][3]
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15037/fi-bxt-dsi/igt@i915_selftest@live_blt.html
    - fi-skl-lmem:        [PASS][4] -> [TIMEOUT][5]
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7206/fi-skl-lmem/igt@i915_selftest@live_blt.html
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15037/fi-skl-lmem/igt@i915_selftest@live_blt.html
    - fi-cfl-8700k:       [PASS][6] -> [TIMEOUT][7]
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7206/fi-cfl-8700k/igt@i915_selftest@live_blt.html
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15037/fi-cfl-8700k/igt@i915_selftest@live_blt.html
    - fi-skl-guc:         [PASS][8] -> [TIMEOUT][9]
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7206/fi-skl-guc/igt@i915_selftest@live_blt.html
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15037/fi-skl-guc/igt@i915_selftest@live_blt.html
    - fi-cfl-guc:         [PASS][10] -> [TIMEOUT][11]
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7206/fi-cfl-guc/igt@i915_selftest@live_blt.html
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15037/fi-cfl-guc/igt@i915_selftest@live_blt.html
    - fi-skl-iommu:       [PASS][12] -> [TIMEOUT][13]
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7206/fi-skl-iommu/igt@i915_selftest@live_blt.html
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15037/fi-skl-iommu/igt@i915_selftest@live_blt.html

  * igt@runner@aborted:
    - fi-bxt-dsi:         NOTRUN -> [FAIL][14]
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15037/fi-bxt-dsi/igt@runner@aborted.html
    - fi-cfl-8700k:       NOTRUN -> [FAIL][15]
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15037/fi-cfl-8700k/igt@runner@aborted.html
    - fi-kbl-8809g:       NOTRUN -> [FAIL][16]
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15037/fi-kbl-8809g/igt@runner@aborted.html

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

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

### IGT changes ###

#### Issues hit ####

  * igt@gem_flink_basic@flink-lifetime:
    - fi-icl-u3:          [PASS][17] -> [DMESG-WARN][18] ([fdo#107724]) +1 similar issue
   [17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7206/fi-icl-u3/igt@gem_flink_basic@flink-lifetime.html
   [18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15037/fi-icl-u3/igt@gem_flink_basic@flink-lifetime.html

  
#### Possible fixes ####

  * igt@gem_ctx_create@basic-files:
    - {fi-icl-dsi}:       [INCOMPLETE][19] ([fdo#107713] / [fdo#109100]) -> [PASS][20]
   [19]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7206/fi-icl-dsi/igt@gem_ctx_create@basic-files.html
   [20]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15037/fi-icl-dsi/igt@gem_ctx_create@basic-files.html

  * igt@gem_ctx_switch@legacy-render:
    - fi-bxt-dsi:         [INCOMPLETE][21] ([fdo#103927] / [fdo#111381]) -> [PASS][22]
   [21]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7206/fi-bxt-dsi/igt@gem_ctx_switch@legacy-render.html
   [22]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15037/fi-bxt-dsi/igt@gem_ctx_switch@legacy-render.html

  * igt@gem_mmap_gtt@basic-read-write-distinct:
    - fi-icl-u3:          [DMESG-WARN][23] ([fdo#107724]) -> [PASS][24] +2 similar issues
   [23]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7206/fi-icl-u3/igt@gem_mmap_gtt@basic-read-write-distinct.html
   [24]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15037/fi-icl-u3/igt@gem_mmap_gtt@basic-read-write-distinct.html

  * igt@i915_selftest@live_blt:
    - fi-kbl-r:           [TIMEOUT][25] -> [PASS][26]
   [25]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7206/fi-kbl-r/igt@i915_selftest@live_blt.html
   [26]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15037/fi-kbl-r/igt@i915_selftest@live_blt.html
    - fi-icl-u2:          [TIMEOUT][27] -> [PASS][28]
   [27]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7206/fi-icl-u2/igt@i915_selftest@live_blt.html
   [28]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15037/fi-icl-u2/igt@i915_selftest@live_blt.html
    - {fi-icl-u4}:        [TIMEOUT][29] -> [PASS][30]
   [29]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_7206/fi-icl-u4/igt@i915_selftest@live_blt.html
   [30]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_15037/fi-icl-u4/igt@i915_selftest@live_blt.html

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

  [fdo#103927]: https://bugs.freedesktop.org/show_bug.cgi?id=103927
  [fdo#107713]: https://bugs.freedesktop.org/show_bug.cgi?id=107713
  [fdo#107724]: https://bugs.freedesktop.org/show_bug.cgi?id=107724
  [fdo#109100]: https://bugs.freedesktop.org/show_bug.cgi?id=109100
  [fdo#110566]: https://bugs.freedesktop.org/show_bug.cgi?id=110566
  [fdo#111381]: https://bugs.freedesktop.org/show_bug.cgi?id=111381


Participating hosts (50 -> 42)
------------------------------

  Missing    (8): fi-ilk-m540 fi-hsw-4200u fi-byt-squawks fi-bsw-cyan fi-apl-guc fi-icl-y fi-byt-clapper fi-bdw-samus 


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

  * CI: CI-20190529 -> None
  * Linux: CI_DRM_7206 -> Patchwork_15037

  CI-20190529: 20190529
  CI_DRM_7206: 197ff4c24c6762ac5bbd8c5364d6f840fb9929c5 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_5248: 81e55f1f97d73e48f00caa7e4fb98295023c5afa @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_15037: bacfbda2a742316927daa8d7054efce567a83b45 @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

bacfbda2a742 drm/i915/guc: Skip suspend/resume GuC action on platforms w/o GuC submission

== Logs ==

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

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH] drm/i915/guc: Skip suspend/resume GuC action on platforms w/o GuC submission
  2019-10-28 21:25 [PATCH] drm/i915/guc: Skip suspend/resume GuC action on platforms w/o GuC submission don.hiatt
  2019-10-28 21:25 ` [Intel-gfx] " don.hiatt
  2019-10-29  0:58 ` ✗ Fi.CI.BAT: failure for " Patchwork
@ 2019-10-29 12:33 ` Michal Wajdeczko
  2019-10-29 12:33   ` [Intel-gfx] " Michal Wajdeczko
                     ` (2 more replies)
  2019-10-30  7:53 ` ✗ Fi.CI.BUILD: failure for drm/i915/guc: Skip suspend/resume GuC action on platforms w/o GuC submission (rev2) Patchwork
  3 siblings, 3 replies; 20+ messages in thread
From: Michal Wajdeczko @ 2019-10-29 12:33 UTC (permalink / raw)
  To: intel-gfx, don.hiatt

On Mon, 28 Oct 2019 22:25:27 +0100, <don.hiatt@intel.com> wrote:

> From: Don Hiatt <don.hiatt@intel.com>
>
> On some platforms (e.g. KBL) that do not support GuC submission, but
> the user enabled the GuC communication (e.g for HuC authentication)
> calling the GuC EXIT_S_STATE action results in lose of ability to
> enter RC6. We can remove the GuC suspend/remove entirely as we do
> not need to save the GuC submission status.
>
> v2: Do not suspend/resume the GuC on platforms that do not support
>     Guc Submission.
>
> Signed-off-by: Don Hiatt <don.hiatt@intel.com>
> ---
>  drivers/gpu/drm/i915/gt/uc/intel_uc.c | 17 +++++++++++++++++
>  1 file changed, 17 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_uc.c  
> b/drivers/gpu/drm/i915/gt/uc/intel_uc.c
> index 3fdbc935d155..04031564f0b1 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_uc.c
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_uc.c
> @@ -572,10 +572,19 @@ void intel_uc_runtime_suspend(struct intel_uc *uc)
>  	if (!intel_guc_is_running(guc))
>  		return;
> +	/*
> +	 * If GuC communciation is enabled but submission is not supported,

typo

> +	 * we do not need to suspend the GuC but we do need to disable the
> +	 * GuC communication on suspend.
> +	 */
> +	if (!guc->submission_supported)

Using submission_supported flag directly can be tricky, as today it
is always set to false, but in the future it may indicate either that
submission is supported by the driver/fw and/or enabled by modparam.

There is no guarantee that it will reflect actual runtime status,
as even supported/unblocked guc submission may fallback to execlists.

We may need something like intel_guc_submission_is_active() that will
reflect actual mode of submission currently used by the driver.

> +		goto guc_disable_comm;

and maybe we can move above logic to intel_guc_suspend()
to do not introduce extra goto's ?

> +
>  	err = intel_guc_suspend(guc);
>  	if (err)
>  		DRM_DEBUG_DRIVER("Failed to suspend GuC, err=%d", err);
> +guc_disable_comm:
>  	guc_disable_communication(guc);
>  }
> @@ -605,6 +614,14 @@ static int __uc_resume(struct intel_uc *uc, bool  
> enable_communication)
>  	if (enable_communication)
>  		guc_enable_communication(guc);
> +	/*
> +	 * If GuC communciation is enabled but submission is not supported,

typo

> +	 * we do not need to resume the GuC but we do need to enable the
> +	 * GuC communication on resume (above).
> +	 */
> +	if (!guc->submission_supported)
> +		return 0;

see suspend case comment

> +
>  	err = intel_guc_resume(guc);
>  	if (err) {
>  		DRM_DEBUG_DRIVER("Failed to resume GuC, err=%d", err);

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

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [Intel-gfx] [PATCH] drm/i915/guc: Skip suspend/resume GuC action on platforms w/o GuC submission
  2019-10-29 12:33 ` [PATCH] " Michal Wajdeczko
@ 2019-10-29 12:33   ` Michal Wajdeczko
  2019-10-29 21:15   ` Hiatt, Don
  2019-10-30  7:25   ` Tomas Janousek
  2 siblings, 0 replies; 20+ messages in thread
From: Michal Wajdeczko @ 2019-10-29 12:33 UTC (permalink / raw)
  To: intel-gfx, don.hiatt

On Mon, 28 Oct 2019 22:25:27 +0100, <don.hiatt@intel.com> wrote:

> From: Don Hiatt <don.hiatt@intel.com>
>
> On some platforms (e.g. KBL) that do not support GuC submission, but
> the user enabled the GuC communication (e.g for HuC authentication)
> calling the GuC EXIT_S_STATE action results in lose of ability to
> enter RC6. We can remove the GuC suspend/remove entirely as we do
> not need to save the GuC submission status.
>
> v2: Do not suspend/resume the GuC on platforms that do not support
>     Guc Submission.
>
> Signed-off-by: Don Hiatt <don.hiatt@intel.com>
> ---
>  drivers/gpu/drm/i915/gt/uc/intel_uc.c | 17 +++++++++++++++++
>  1 file changed, 17 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_uc.c  
> b/drivers/gpu/drm/i915/gt/uc/intel_uc.c
> index 3fdbc935d155..04031564f0b1 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_uc.c
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_uc.c
> @@ -572,10 +572,19 @@ void intel_uc_runtime_suspend(struct intel_uc *uc)
>  	if (!intel_guc_is_running(guc))
>  		return;
> +	/*
> +	 * If GuC communciation is enabled but submission is not supported,

typo

> +	 * we do not need to suspend the GuC but we do need to disable the
> +	 * GuC communication on suspend.
> +	 */
> +	if (!guc->submission_supported)

Using submission_supported flag directly can be tricky, as today it
is always set to false, but in the future it may indicate either that
submission is supported by the driver/fw and/or enabled by modparam.

There is no guarantee that it will reflect actual runtime status,
as even supported/unblocked guc submission may fallback to execlists.

We may need something like intel_guc_submission_is_active() that will
reflect actual mode of submission currently used by the driver.

> +		goto guc_disable_comm;

and maybe we can move above logic to intel_guc_suspend()
to do not introduce extra goto's ?

> +
>  	err = intel_guc_suspend(guc);
>  	if (err)
>  		DRM_DEBUG_DRIVER("Failed to suspend GuC, err=%d", err);
> +guc_disable_comm:
>  	guc_disable_communication(guc);
>  }
> @@ -605,6 +614,14 @@ static int __uc_resume(struct intel_uc *uc, bool  
> enable_communication)
>  	if (enable_communication)
>  		guc_enable_communication(guc);
> +	/*
> +	 * If GuC communciation is enabled but submission is not supported,

typo

> +	 * we do not need to resume the GuC but we do need to enable the
> +	 * GuC communication on resume (above).
> +	 */
> +	if (!guc->submission_supported)
> +		return 0;

see suspend case comment

> +
>  	err = intel_guc_resume(guc);
>  	if (err) {
>  		DRM_DEBUG_DRIVER("Failed to resume GuC, err=%d", err);

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

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH] drm/i915/guc: Skip suspend/resume GuC action on platforms w/o GuC submission
  2019-10-29 12:33 ` [PATCH] " Michal Wajdeczko
  2019-10-29 12:33   ` [Intel-gfx] " Michal Wajdeczko
@ 2019-10-29 21:15   ` Hiatt, Don
  2019-10-29 21:15     ` [Intel-gfx] " Hiatt, Don
  2019-10-30  7:25   ` Tomas Janousek
  2 siblings, 1 reply; 20+ messages in thread
From: Hiatt, Don @ 2019-10-29 21:15 UTC (permalink / raw)
  To: Wajdeczko, Michal, intel-gfx

> From: Wajdeczko, Michal <Michal.Wajdeczko@intel.com>
> Sent: Tuesday, October 29, 2019 5:33 AM
> To: intel-gfx@lists.freedesktop.org; Hiatt, Don <don.hiatt@intel.com>
> Subject: Re: [Intel-gfx] [PATCH] drm/i915/guc: Skip suspend/resume GuC action
> on platforms w/o GuC submission
> 
> On Mon, 28 Oct 2019 22:25:27 +0100, <don.hiatt@intel.com> wrote:
> 
> > From: Don Hiatt <don.hiatt@intel.com>
> >
> > On some platforms (e.g. KBL) that do not support GuC submission, but
> > the user enabled the GuC communication (e.g for HuC authentication)
> > calling the GuC EXIT_S_STATE action results in lose of ability to
> > enter RC6. We can remove the GuC suspend/remove entirely as we do
> > not need to save the GuC submission status.
> >
> > v2: Do not suspend/resume the GuC on platforms that do not support
> >     Guc Submission.
> >
> > Signed-off-by: Don Hiatt <don.hiatt@intel.com>
> > ---
> >  drivers/gpu/drm/i915/gt/uc/intel_uc.c | 17 +++++++++++++++++
> >  1 file changed, 17 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/i915/gt/uc/intel_uc.c
> > b/drivers/gpu/drm/i915/gt/uc/intel_uc.c
> > index 3fdbc935d155..04031564f0b1 100644
> > --- a/drivers/gpu/drm/i915/gt/uc/intel_uc.c
> > +++ b/drivers/gpu/drm/i915/gt/uc/intel_uc.c
> > @@ -572,10 +572,19 @@ void intel_uc_runtime_suspend(struct intel_uc *uc)
> >  	if (!intel_guc_is_running(guc))
> >  		return;
> > +	/*
> > +	 * If GuC communciation is enabled but submission is not supported,
> 
> typo
> 
> > +	 * we do not need to suspend the GuC but we do need to disable the
> > +	 * GuC communication on suspend.
> > +	 */
> > +	if (!guc->submission_supported)
> 
> Using submission_supported flag directly can be tricky, as today it
> is always set to false, but in the future it may indicate either that
> submission is supported by the driver/fw and/or enabled by modparam.
> 
> There is no guarantee that it will reflect actual runtime status,
> as even supported/unblocked guc submission may fallback to execlists.
> 
> We may need something like intel_guc_submission_is_active() that will
> reflect actual mode of submission currently used by the driver.

Hi Michal,

I looked at your patch wrt checking the set_default_submission vfunc but
as that is for the engine, and here I only have access to the intel_guc struct.
I'm not sure just where I can know what the default submission is and then
flag it somewhere that I can then check here in the suspend/resume. I'll keep
looking (sorry, I'm very new to this code). 😊

Thanks,

don






> 
> > +		goto guc_disable_comm;
> 
> and maybe we can move above logic to intel_guc_suspend()
> to do not introduce extra goto's ?
> 
> > +
> >  	err = intel_guc_suspend(guc);
> >  	if (err)
> >  		DRM_DEBUG_DRIVER("Failed to suspend GuC, err=%d", err);
> > +guc_disable_comm:
> >  	guc_disable_communication(guc);
> >  }
> > @@ -605,6 +614,14 @@ static int __uc_resume(struct intel_uc *uc, bool
> > enable_communication)
> >  	if (enable_communication)
> >  		guc_enable_communication(guc);
> > +	/*
> > +	 * If GuC communciation is enabled but submission is not supported,
> 
> typo
> 
> > +	 * we do not need to resume the GuC but we do need to enable the
> > +	 * GuC communication on resume (above).
> > +	 */
> > +	if (!guc->submission_supported)
> > +		return 0;
> 
> see suspend case comment
> 
> > +
> >  	err = intel_guc_resume(guc);
> >  	if (err) {
> >  		DRM_DEBUG_DRIVER("Failed to resume GuC, err=%d", err);
> 
> Thanks,
> Michal
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [Intel-gfx] [PATCH] drm/i915/guc: Skip suspend/resume GuC action on platforms w/o GuC submission
  2019-10-29 21:15   ` Hiatt, Don
@ 2019-10-29 21:15     ` Hiatt, Don
  0 siblings, 0 replies; 20+ messages in thread
From: Hiatt, Don @ 2019-10-29 21:15 UTC (permalink / raw)
  To: Wajdeczko, Michal, intel-gfx

> From: Wajdeczko, Michal <Michal.Wajdeczko@intel.com>
> Sent: Tuesday, October 29, 2019 5:33 AM
> To: intel-gfx@lists.freedesktop.org; Hiatt, Don <don.hiatt@intel.com>
> Subject: Re: [Intel-gfx] [PATCH] drm/i915/guc: Skip suspend/resume GuC action
> on platforms w/o GuC submission
> 
> On Mon, 28 Oct 2019 22:25:27 +0100, <don.hiatt@intel.com> wrote:
> 
> > From: Don Hiatt <don.hiatt@intel.com>
> >
> > On some platforms (e.g. KBL) that do not support GuC submission, but
> > the user enabled the GuC communication (e.g for HuC authentication)
> > calling the GuC EXIT_S_STATE action results in lose of ability to
> > enter RC6. We can remove the GuC suspend/remove entirely as we do
> > not need to save the GuC submission status.
> >
> > v2: Do not suspend/resume the GuC on platforms that do not support
> >     Guc Submission.
> >
> > Signed-off-by: Don Hiatt <don.hiatt@intel.com>
> > ---
> >  drivers/gpu/drm/i915/gt/uc/intel_uc.c | 17 +++++++++++++++++
> >  1 file changed, 17 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/i915/gt/uc/intel_uc.c
> > b/drivers/gpu/drm/i915/gt/uc/intel_uc.c
> > index 3fdbc935d155..04031564f0b1 100644
> > --- a/drivers/gpu/drm/i915/gt/uc/intel_uc.c
> > +++ b/drivers/gpu/drm/i915/gt/uc/intel_uc.c
> > @@ -572,10 +572,19 @@ void intel_uc_runtime_suspend(struct intel_uc *uc)
> >  	if (!intel_guc_is_running(guc))
> >  		return;
> > +	/*
> > +	 * If GuC communciation is enabled but submission is not supported,
> 
> typo
> 
> > +	 * we do not need to suspend the GuC but we do need to disable the
> > +	 * GuC communication on suspend.
> > +	 */
> > +	if (!guc->submission_supported)
> 
> Using submission_supported flag directly can be tricky, as today it
> is always set to false, but in the future it may indicate either that
> submission is supported by the driver/fw and/or enabled by modparam.
> 
> There is no guarantee that it will reflect actual runtime status,
> as even supported/unblocked guc submission may fallback to execlists.
> 
> We may need something like intel_guc_submission_is_active() that will
> reflect actual mode of submission currently used by the driver.

Hi Michal,

I looked at your patch wrt checking the set_default_submission vfunc but
as that is for the engine, and here I only have access to the intel_guc struct.
I'm not sure just where I can know what the default submission is and then
flag it somewhere that I can then check here in the suspend/resume. I'll keep
looking (sorry, I'm very new to this code). 😊

Thanks,

don






> 
> > +		goto guc_disable_comm;
> 
> and maybe we can move above logic to intel_guc_suspend()
> to do not introduce extra goto's ?
> 
> > +
> >  	err = intel_guc_suspend(guc);
> >  	if (err)
> >  		DRM_DEBUG_DRIVER("Failed to suspend GuC, err=%d", err);
> > +guc_disable_comm:
> >  	guc_disable_communication(guc);
> >  }
> > @@ -605,6 +614,14 @@ static int __uc_resume(struct intel_uc *uc, bool
> > enable_communication)
> >  	if (enable_communication)
> >  		guc_enable_communication(guc);
> > +	/*
> > +	 * If GuC communciation is enabled but submission is not supported,
> 
> typo
> 
> > +	 * we do not need to resume the GuC but we do need to enable the
> > +	 * GuC communication on resume (above).
> > +	 */
> > +	if (!guc->submission_supported)
> > +		return 0;
> 
> see suspend case comment
> 
> > +
> >  	err = intel_guc_resume(guc);
> >  	if (err) {
> >  		DRM_DEBUG_DRIVER("Failed to resume GuC, err=%d", err);
> 
> Thanks,
> Michal
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [PATCH] drm/i915/guc: Skip suspend/resume GuC action on platforms w/o GuC submission
  2019-10-29 12:33 ` [PATCH] " Michal Wajdeczko
  2019-10-29 12:33   ` [Intel-gfx] " Michal Wajdeczko
  2019-10-29 21:15   ` Hiatt, Don
@ 2019-10-30  7:25   ` Tomas Janousek
  2019-10-30  7:25     ` [Intel-gfx] " Tomas Janousek
  2 siblings, 1 reply; 20+ messages in thread
From: Tomas Janousek @ 2019-10-30  7:25 UTC (permalink / raw)
  To: Michal Wajdeczko; +Cc: intel-gfx

On Tue, Oct 29, 2019 at 01:33:22PM +0100, Michal Wajdeczko wrote:
> On Mon, 28 Oct 2019 22:25:27 +0100, <don.hiatt@intel.com> wrote:
> > +	 * we do not need to suspend the GuC but we do need to disable the
> > +	 * GuC communication on suspend.
> > +	 */
> > +	if (!guc->submission_supported)
> 
> Using submission_supported flag directly can be tricky, as today it
> is always set to false, but in the future it may indicate either that
> submission is supported by the driver/fw and/or enabled by modparam.
> 
> There is no guarantee that it will reflect actual runtime status,
> as even supported/unblocked guc submission may fallback to execlists.
> 
> We may need something like intel_guc_submission_is_active() that will
> reflect actual mode of submission currently used by the driver.

What about this:

diff --git a/drivers/gpu/drm/i915/intel_uc.c b/drivers/gpu/drm/i915/intel_uc.c
index ae45651ac73c..acda38a9fec5 100644
--- a/drivers/gpu/drm/i915/intel_uc.c
+++ b/drivers/gpu/drm/i915/intel_uc.c
@@ -522,10 +522,19 @@ void intel_uc_runtime_suspend(struct drm_i915_private *i915)
 	if (!intel_guc_is_loaded(guc))
 		return;
 
+	/*
+	 * If GuC communication is enabled but submission is not supported,
+	 * we do not need to suspend the GuC but we do need to disable the
+	 * GuC communication on suspend.
+	 */
+	if (!USES_GUC_SUBMISSION(i915))
+		goto guc_disable_comm;
+
 	err = intel_guc_suspend(guc);
 	if (err)
 		DRM_DEBUG_DRIVER("Failed to suspend GuC, err=%d", err);
 
+guc_disable_comm:
 	guc_disable_communication(guc);
 }
 
@@ -551,6 +560,14 @@ int intel_uc_resume(struct drm_i915_private *i915)
 
 	guc_enable_communication(guc);
 
+	/*
+	 * If GuC communication is enabled but submission is not supported,
+	 * we do not need to resume the GuC but we do need to enable the
+	 * GuC communication on resume (above).
+	 */
+	if (!USES_GUC_SUBMISSION(i915))
+		return 0;
+
 	err = intel_guc_resume(guc);
 	if (err) {
 		DRM_DEBUG_DRIVER("Failed to resume GuC, err=%d", err);

This is what the backport to stable-5.3 will look like anyway as there's no
submission_supported there. The name "USES_GUC_SUBMISSION" does suggest it
might reflect actual runtime status... :-)

-- 
Tomáš Janoušek, a.k.a. Pivník, a.k.a. Liskni_si, http://work.lisk.in/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* Re: [Intel-gfx] [PATCH] drm/i915/guc: Skip suspend/resume GuC action on platforms w/o GuC submission
  2019-10-30  7:25   ` Tomas Janousek
@ 2019-10-30  7:25     ` Tomas Janousek
  0 siblings, 0 replies; 20+ messages in thread
From: Tomas Janousek @ 2019-10-30  7:25 UTC (permalink / raw)
  To: Michal Wajdeczko; +Cc: intel-gfx

On Tue, Oct 29, 2019 at 01:33:22PM +0100, Michal Wajdeczko wrote:
> On Mon, 28 Oct 2019 22:25:27 +0100, <don.hiatt@intel.com> wrote:
> > +	 * we do not need to suspend the GuC but we do need to disable the
> > +	 * GuC communication on suspend.
> > +	 */
> > +	if (!guc->submission_supported)
> 
> Using submission_supported flag directly can be tricky, as today it
> is always set to false, but in the future it may indicate either that
> submission is supported by the driver/fw and/or enabled by modparam.
> 
> There is no guarantee that it will reflect actual runtime status,
> as even supported/unblocked guc submission may fallback to execlists.
> 
> We may need something like intel_guc_submission_is_active() that will
> reflect actual mode of submission currently used by the driver.

What about this:

diff --git a/drivers/gpu/drm/i915/intel_uc.c b/drivers/gpu/drm/i915/intel_uc.c
index ae45651ac73c..acda38a9fec5 100644
--- a/drivers/gpu/drm/i915/intel_uc.c
+++ b/drivers/gpu/drm/i915/intel_uc.c
@@ -522,10 +522,19 @@ void intel_uc_runtime_suspend(struct drm_i915_private *i915)
 	if (!intel_guc_is_loaded(guc))
 		return;
 
+	/*
+	 * If GuC communication is enabled but submission is not supported,
+	 * we do not need to suspend the GuC but we do need to disable the
+	 * GuC communication on suspend.
+	 */
+	if (!USES_GUC_SUBMISSION(i915))
+		goto guc_disable_comm;
+
 	err = intel_guc_suspend(guc);
 	if (err)
 		DRM_DEBUG_DRIVER("Failed to suspend GuC, err=%d", err);
 
+guc_disable_comm:
 	guc_disable_communication(guc);
 }
 
@@ -551,6 +560,14 @@ int intel_uc_resume(struct drm_i915_private *i915)
 
 	guc_enable_communication(guc);
 
+	/*
+	 * If GuC communication is enabled but submission is not supported,
+	 * we do not need to resume the GuC but we do need to enable the
+	 * GuC communication on resume (above).
+	 */
+	if (!USES_GUC_SUBMISSION(i915))
+		return 0;
+
 	err = intel_guc_resume(guc);
 	if (err) {
 		DRM_DEBUG_DRIVER("Failed to resume GuC, err=%d", err);

This is what the backport to stable-5.3 will look like anyway as there's no
submission_supported there. The name "USES_GUC_SUBMISSION" does suggest it
might reflect actual runtime status... :-)

-- 
Tomáš Janoušek, a.k.a. Pivník, a.k.a. Liskni_si, http://work.lisk.in/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* ✗ Fi.CI.BUILD: failure for drm/i915/guc: Skip suspend/resume GuC action on platforms w/o GuC submission (rev2)
  2019-10-28 21:25 [PATCH] drm/i915/guc: Skip suspend/resume GuC action on platforms w/o GuC submission don.hiatt
                   ` (2 preceding siblings ...)
  2019-10-29 12:33 ` [PATCH] " Michal Wajdeczko
@ 2019-10-30  7:53 ` Patchwork
  2019-10-30  7:53   ` [Intel-gfx] " Patchwork
  3 siblings, 1 reply; 20+ messages in thread
From: Patchwork @ 2019-10-30  7:53 UTC (permalink / raw)
  To: Tomas Janousek; +Cc: intel-gfx

== Series Details ==

Series: drm/i915/guc: Skip suspend/resume GuC action on platforms w/o GuC submission (rev2)
URL   : https://patchwork.freedesktop.org/series/68685/
State : failure

== Summary ==

Applying: drm/i915/guc: Skip suspend/resume GuC action on platforms w/o GuC submission
Using index info to reconstruct a base tree...
A	drivers/gpu/drm/i915/intel_uc.c
Falling back to patching base and 3-way merge...
CONFLICT (modify/delete): drivers/gpu/drm/i915/intel_uc.c deleted in HEAD and modified in drm/i915/guc: Skip suspend/resume GuC action on platforms w/o GuC submission. Version drm/i915/guc: Skip suspend/resume GuC action on platforms w/o GuC submission of drivers/gpu/drm/i915/intel_uc.c left in tree.
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch' to see the failed patch
Patch failed at 0001 drm/i915/guc: Skip suspend/resume GuC action on platforms w/o GuC submission
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".

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

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [Intel-gfx] ✗ Fi.CI.BUILD: failure for drm/i915/guc: Skip suspend/resume GuC action on platforms w/o GuC submission (rev2)
  2019-10-30  7:53 ` ✗ Fi.CI.BUILD: failure for drm/i915/guc: Skip suspend/resume GuC action on platforms w/o GuC submission (rev2) Patchwork
@ 2019-10-30  7:53   ` Patchwork
  0 siblings, 0 replies; 20+ messages in thread
From: Patchwork @ 2019-10-30  7:53 UTC (permalink / raw)
  To: Tomas Janousek; +Cc: intel-gfx

== Series Details ==

Series: drm/i915/guc: Skip suspend/resume GuC action on platforms w/o GuC submission (rev2)
URL   : https://patchwork.freedesktop.org/series/68685/
State : failure

== Summary ==

Applying: drm/i915/guc: Skip suspend/resume GuC action on platforms w/o GuC submission
Using index info to reconstruct a base tree...
A	drivers/gpu/drm/i915/intel_uc.c
Falling back to patching base and 3-way merge...
CONFLICT (modify/delete): drivers/gpu/drm/i915/intel_uc.c deleted in HEAD and modified in drm/i915/guc: Skip suspend/resume GuC action on platforms w/o GuC submission. Version drm/i915/guc: Skip suspend/resume GuC action on platforms w/o GuC submission of drivers/gpu/drm/i915/intel_uc.c left in tree.
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch' to see the failed patch
Patch failed at 0001 drm/i915/guc: Skip suspend/resume GuC action on platforms w/o GuC submission
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".

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

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [Intel-gfx] [PATCH] drm/i915/guc: Skip suspend/resume GuC action on platforms w/o GuC submission
  2019-11-16 10:04 ` Chris Wilson
@ 2019-11-16 10:04   ` Chris Wilson
  0 siblings, 0 replies; 20+ messages in thread
From: Chris Wilson @ 2019-11-16 10:04 UTC (permalink / raw)
  To: don.hiatt, intel-gfx; +Cc: S . Zharkoff, KiteStramuort, Tomas Janousek

Quoting don.hiatt@intel.com (2019-11-15 23:15:38)
> From: Don Hiatt <don.hiatt@intel.com>
> 
> On some platforms (e.g. KBL) that do not support GuC submission, but
> the user enabled the GuC communication (e.g for HuC authentication)
> calling the GuC EXIT_S_STATE action results in lose of ability to
> enter RC6. We can remove the GuC suspend/resume entirely as we do
> not need to save the GuC submission status.
> 
> Add intel_guc_submission_is_enabled() function to determine if
> GuC submission is active.
> 
> v2: Do not suspend/resume the GuC on platforms that do not support
>     Guc Submission.
> v3: Fix typo, move suspend logic to remove goto.
> v4: Use intel_guc_submission_is_enabled() to check GuC submission
>     status.
> v5: No need to look at engine to determine if submission is enabled.
>     Squash fix + intel_guc_submission_is_enabled() patch into one.
> v6: Move resume check into intel_guc_resume() for symmetry.
>     Fix commit Fixes tag.
> 
> Reported-by: KiteStramuort <kitestramuort@autistici.org>
> Reported-by: S. Zharkoff <s.zharkoff@gmail.com>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=111594
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=111623
> Fixes: ffd5ce22faa4 ("drm/i915/guc: Updates for GuC 32.0.3 firmware")
> Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
> Cc: Daniele Ceralo Spurio <daniele.ceraolospurio@intel.com>
> Cc: Stuart Summers <stuart.summers@intel.com>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Tested-by: Tomas Janousek <tomi@nomi.cz>
> Signed-off-by: Don Hiatt <don.hiatt@intel.com>

This fixes a rather nasty bug; looks simple enough to be backport
friendly and any further bikesheds can be applied along with guc
refactoring.

Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [Intel-gfx] [PATCH] drm/i915/guc: Skip suspend/resume GuC action on platforms w/o GuC submission
  2019-11-15 23:15 don.hiatt
@ 2019-11-15 23:15 ` don.hiatt
  2019-11-16 10:04 ` Chris Wilson
  1 sibling, 0 replies; 20+ messages in thread
From: don.hiatt @ 2019-11-15 23:15 UTC (permalink / raw)
  To: intel-gfx; +Cc: S . Zharkoff, KiteStramuort, Tomas Janousek

From: Don Hiatt <don.hiatt@intel.com>

On some platforms (e.g. KBL) that do not support GuC submission, but
the user enabled the GuC communication (e.g for HuC authentication)
calling the GuC EXIT_S_STATE action results in lose of ability to
enter RC6. We can remove the GuC suspend/resume entirely as we do
not need to save the GuC submission status.

Add intel_guc_submission_is_enabled() function to determine if
GuC submission is active.

v2: Do not suspend/resume the GuC on platforms that do not support
    Guc Submission.
v3: Fix typo, move suspend logic to remove goto.
v4: Use intel_guc_submission_is_enabled() to check GuC submission
    status.
v5: No need to look at engine to determine if submission is enabled.
    Squash fix + intel_guc_submission_is_enabled() patch into one.
v6: Move resume check into intel_guc_resume() for symmetry.
    Fix commit Fixes tag.

Reported-by: KiteStramuort <kitestramuort@autistici.org>
Reported-by: S. Zharkoff <s.zharkoff@gmail.com>
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=111594
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=111623
Fixes: ffd5ce22faa4 ("drm/i915/guc: Updates for GuC 32.0.3 firmware")
Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
Cc: Daniele Ceralo Spurio <daniele.ceraolospurio@intel.com>
Cc: Stuart Summers <stuart.summers@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Tested-by: Tomas Janousek <tomi@nomi.cz>
Signed-off-by: Don Hiatt <don.hiatt@intel.com>
---
 drivers/gpu/drm/i915/gt/uc/intel_guc.c | 15 +++++++++++++++
 drivers/gpu/drm/i915/i915_drv.h        |  6 ++++++
 2 files changed, 21 insertions(+)

diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc.c b/drivers/gpu/drm/i915/gt/uc/intel_guc.c
index 019ae6486e8d..3ee4a4e7689d 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc.c
@@ -553,6 +553,13 @@ int intel_guc_suspend(struct intel_guc *guc)
 		GUC_POWER_D1, /* any value greater than GUC_POWER_D0 */
 	};
 
+	/*
+	 * If GuC communication is enabled but submission is not supported,
+	 * we do not need to suspend the GuC.
+	 */
+	if (!intel_guc_submission_is_enabled(guc))
+		return 0;
+
 	/*
 	 * The ENTER_S_STATE action queues the save/restore operation in GuC FW
 	 * and then returns, so waiting on the H2G is not enough to guarantee
@@ -610,6 +617,14 @@ int intel_guc_resume(struct intel_guc *guc)
 		GUC_POWER_D0,
 	};
 
+	/*
+	 * If GuC communication is enabled but submission is not supported,
+	 * we do not need to resume the GuC but we do need to enable the
+	 * GuC communication on resume (above).
+	 */
+	if (!intel_guc_submission_is_enabled(guc))
+		return 0;
+
 	return intel_guc_send(guc, action, ARRAY_SIZE(action));
 }
 
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 7bc1d8c102b2..2b879472e760 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2044,4 +2044,10 @@ i915_coherent_map_type(struct drm_i915_private *i915)
 	return HAS_LLC(i915) ? I915_MAP_WB : I915_MAP_WC;
 }
 
+static inline bool intel_guc_submission_is_enabled(struct intel_guc *guc)
+{
+	return intel_guc_is_submission_supported(guc) &&
+		intel_guc_is_running(guc);
+}
+
 #endif
-- 
2.20.1

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

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* Re: [Intel-gfx] [PATCH] drm/i915/guc: Skip suspend/resume GuC action on platforms w/o GuC submission
  2019-11-15 23:15       ` Hiatt, Don
@ 2019-11-15 23:15         ` Hiatt, Don
  0 siblings, 0 replies; 20+ messages in thread
From: Hiatt, Don @ 2019-11-15 23:15 UTC (permalink / raw)
  To: Ceraolo Spurio, Daniele, intel-gfx
  Cc: S . Zharkoff, KiteStramuort, Tomas Janousek

> From: Ceraolo Spurio, Daniele <daniele.ceraolospurio@intel.com>
> Sent: Friday, November 15, 2019 3:10 PM
> To: Hiatt, Don <don.hiatt@intel.com>; intel-gfx@lists.freedesktop.org
> Cc: Ewins, Jon <jon.ewins@intel.com>; KiteStramuort
> <kitestramuort@autistici.org>; S . Zharkoff <s.zharkoff@gmail.com>;
> Wajdeczko, Michal <Michal.Wajdeczko@intel.com>; Summers, Stuart
> <stuart.summers@intel.com>; Chris Wilson <chris@chris-wilson.co.uk>; Tomas
> Janousek <tomi@nomi.cz>
> Subject: Re: [PATCH] drm/i915/guc: Skip suspend/resume GuC action on
> platforms w/o GuC submission
> 
> 
> 
> On 11/15/2019 2:22 PM, Hiatt, Don wrote:
> >
> >> From: Ceraolo Spurio, Daniele <daniele.ceraolospurio@intel.com>
> >> Sent: Friday, November 15, 2019 2:02 PM
> >> To: Hiatt, Don <don.hiatt@intel.com>; intel-gfx@lists.freedesktop.org
> >> Cc: Ewins, Jon <jon.ewins@intel.com>; KiteStramuort
> >> <kitestramuort@autistici.org>; S . Zharkoff <s.zharkoff@gmail.com>;
> >> Wajdeczko, Michal <Michal.Wajdeczko@intel.com>; Summers, Stuart
> >> <stuart.summers@intel.com>; Chris Wilson <chris@chris-wilson.co.uk>;
> Tomas
> >> Janousek <tomi@nomi.cz>
> >> Subject: Re: [PATCH] drm/i915/guc: Skip suspend/resume GuC action on
> >> platforms w/o GuC submission
> >>
> >>
> >>
> >> On 11/15/19 10:20 AM, don.hiatt@intel.com wrote:
> >>> From: Don Hiatt <don.hiatt@intel.com>
> >>>
> >>> On some platforms (e.g. KBL) that do not support GuC submission, but
> >>> the user enabled the GuC communication (e.g for HuC authentication)
> >>> calling the GuC EXIT_S_STATE action results in lose of ability to
> >>> enter RC6. We can remove the GuC suspend/resume entirely as we do
> >>> not need to save the GuC submission status.
> >>>
> >>> Add intel_guc_submission_is_enabled() function to determine if
> >>> GuC submission is active.
> >>>
> >>> v2: Do not suspend/resume the GuC on platforms that do not support
> >>>       Guc Submission.
> >>> v3: Fix typo, move suspend logic to remove goto.
> >>> v4: Use intel_guc_submission_is_enabled() to check GuC submission
> >>>       status.
> >>> v5: No need to look at engine to determine if submission is enabled.
> >>>       Squash fix + intel_guc_submission_is_enabled() patch into one.
> >>>
> >>> Reported-by: KiteStramuort <kitestramuort@autistici.org>
> >>> Reported-by: S. Zharkoff <s.zharkoff@gmail.com>
> >>> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=111594
> >>> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=111623
> >>> Fixes: f774f0964919 ("drm/i915/guc: Turn on GuC/HuC auto mode")
> >> I don't think this Fixes tag is appropriate since we already reverted
> >> that patch.
> >>
> > Can you suggest an appropriate tag? Chris suggest "guc/huc enabling by
> default" patch
> > but I couldn't find it and this one seemed similar.
> 
> Do we want to backport this even if guc is off by default and taints on
> enabling? if we do, then we probably need to go all the way back to when
> we introduced the new binaries that show the problem ("drm/i915/guc:
> Updates for GuC 32.0.3 firmware").
> 
Yes, we do as the original reports was with the user specifically enabling
with the driver parameters.

Thanks a lot! The update patch will be posted in a second.

> Daniele
> 
> >
> >>> Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
> >>> Cc: Daniele Ceralo Spurio <daniele.ceraolospurio@intel.com>
> >>> Cc: Stuart Summers <stuart.summers@intel.com>
> >>> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> >>> Tested-by: Tomas Janousek <tomi@nomi.cz>
> >>> Signed-off-by: Don Hiatt <don.hiatt@intel.com>
> >>> ---
> >>>    drivers/gpu/drm/i915/gt/uc/intel_guc.c | 7 +++++++
> >>>    drivers/gpu/drm/i915/gt/uc/intel_uc.c  | 8 ++++++++
> >>>    drivers/gpu/drm/i915/i915_drv.h        | 6 ++++++
> >>>    3 files changed, 21 insertions(+)
> >>>
> >>> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc.c
> >> b/drivers/gpu/drm/i915/gt/uc/intel_guc.c
> >>> index 019ae6486e8d..92d9305c0d73 100644
> >>> --- a/drivers/gpu/drm/i915/gt/uc/intel_guc.c
> >>> +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc.c
> >>> @@ -553,6 +553,13 @@ int intel_guc_suspend(struct intel_guc *guc)
> >>>    		GUC_POWER_D1, /* any value greater than GUC_POWER_D0
> >> */
> >>>    	};
> >>>
> >>> +	/*
> >>> +	 * If GuC communication is enabled but submission is not supported,
> >>> +	 * we do not need to suspend the GuC.
> >>> +	 */
> >>> +	if (!intel_guc_submission_is_enabled(guc))
> >>> +		return 0;
> >>> +
> >>>    	/*
> >>>    	 * The ENTER_S_STATE action queues the save/restore operation in GuC
> >> FW
> >>>    	 * and then returns, so waiting on the H2G is not enough to guarantee
> >>> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_uc.c
> >> b/drivers/gpu/drm/i915/gt/uc/intel_uc.c
> >>> index 629b19377a29..4dd43b99a334 100644
> >>> --- a/drivers/gpu/drm/i915/gt/uc/intel_uc.c
> >>> +++ b/drivers/gpu/drm/i915/gt/uc/intel_uc.c
> >>> @@ -605,6 +605,14 @@ static int __uc_resume(struct intel_uc *uc, bool
> >> enable_communication)
> >>>    	if (enable_communication)
> >>>    		guc_enable_communication(guc);
> >>>
> >>> +	/*
> >>> +	 * If GuC communication is enabled but submission is not supported,
> >>> +	 * we do not need to resume the GuC but we do need to enable the
> >>> +	 * GuC communication on resume (above).
> >>> +	 */
> >>> +	if (!intel_guc_submission_is_enabled(guc))
> >>> +		return 0;
> >>> +
> >> I would put this check inside intel_guc_resume(), for symmetry with the
> >> one you put inside intel_guc_suspend().
> >>
> > Will do.
> >
> >> Daniele
> >>
> >>>    	err = intel_guc_resume(guc);
> >>>    	if (err) {
> >>>    		DRM_DEBUG_DRIVER("Failed to resume GuC, err=%d", err);
> >>> diff --git a/drivers/gpu/drm/i915/i915_drv.h
> >> b/drivers/gpu/drm/i915/i915_drv.h
> >>> index 7bc1d8c102b2..2b879472e760 100644
> >>> --- a/drivers/gpu/drm/i915/i915_drv.h
> >>> +++ b/drivers/gpu/drm/i915/i915_drv.h
> >>> @@ -2044,4 +2044,10 @@ i915_coherent_map_type(struct
> drm_i915_private
> >> *i915)
> >>>    	return HAS_LLC(i915) ? I915_MAP_WB : I915_MAP_WC;
> >>>    }
> >>>
> >>> +static inline bool intel_guc_submission_is_enabled(struct intel_guc *guc)
> >>> +{
> >>> +	return intel_guc_is_submission_supported(guc) &&
> >>> +		intel_guc_is_running(guc);
> >>> +}
> >>> +
> >>>    #endif
> >>>

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

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [Intel-gfx] [PATCH] drm/i915/guc: Skip suspend/resume GuC action on platforms w/o GuC submission
  2019-11-15 23:10     ` Daniele Ceraolo Spurio
@ 2019-11-15 23:10       ` Daniele Ceraolo Spurio
  2019-11-15 23:15       ` Hiatt, Don
  1 sibling, 0 replies; 20+ messages in thread
From: Daniele Ceraolo Spurio @ 2019-11-15 23:10 UTC (permalink / raw)
  To: Hiatt, Don, intel-gfx; +Cc: S . Zharkoff, KiteStramuort, Tomas Janousek



On 11/15/2019 2:22 PM, Hiatt, Don wrote:
>
>> From: Ceraolo Spurio, Daniele <daniele.ceraolospurio@intel.com>
>> Sent: Friday, November 15, 2019 2:02 PM
>> To: Hiatt, Don <don.hiatt@intel.com>; intel-gfx@lists.freedesktop.org
>> Cc: Ewins, Jon <jon.ewins@intel.com>; KiteStramuort
>> <kitestramuort@autistici.org>; S . Zharkoff <s.zharkoff@gmail.com>;
>> Wajdeczko, Michal <Michal.Wajdeczko@intel.com>; Summers, Stuart
>> <stuart.summers@intel.com>; Chris Wilson <chris@chris-wilson.co.uk>; Tomas
>> Janousek <tomi@nomi.cz>
>> Subject: Re: [PATCH] drm/i915/guc: Skip suspend/resume GuC action on
>> platforms w/o GuC submission
>>
>>
>>
>> On 11/15/19 10:20 AM, don.hiatt@intel.com wrote:
>>> From: Don Hiatt <don.hiatt@intel.com>
>>>
>>> On some platforms (e.g. KBL) that do not support GuC submission, but
>>> the user enabled the GuC communication (e.g for HuC authentication)
>>> calling the GuC EXIT_S_STATE action results in lose of ability to
>>> enter RC6. We can remove the GuC suspend/resume entirely as we do
>>> not need to save the GuC submission status.
>>>
>>> Add intel_guc_submission_is_enabled() function to determine if
>>> GuC submission is active.
>>>
>>> v2: Do not suspend/resume the GuC on platforms that do not support
>>>       Guc Submission.
>>> v3: Fix typo, move suspend logic to remove goto.
>>> v4: Use intel_guc_submission_is_enabled() to check GuC submission
>>>       status.
>>> v5: No need to look at engine to determine if submission is enabled.
>>>       Squash fix + intel_guc_submission_is_enabled() patch into one.
>>>
>>> Reported-by: KiteStramuort <kitestramuort@autistici.org>
>>> Reported-by: S. Zharkoff <s.zharkoff@gmail.com>
>>> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=111594
>>> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=111623
>>> Fixes: f774f0964919 ("drm/i915/guc: Turn on GuC/HuC auto mode")
>> I don't think this Fixes tag is appropriate since we already reverted
>> that patch.
>>
> Can you suggest an appropriate tag? Chris suggest "guc/huc enabling by default" patch
> but I couldn't find it and this one seemed similar.

Do we want to backport this even if guc is off by default and taints on 
enabling? if we do, then we probably need to go all the way back to when 
we introduced the new binaries that show the problem ("drm/i915/guc: 
Updates for GuC 32.0.3 firmware").

Daniele

>
>>> Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
>>> Cc: Daniele Ceralo Spurio <daniele.ceraolospurio@intel.com>
>>> Cc: Stuart Summers <stuart.summers@intel.com>
>>> Cc: Chris Wilson <chris@chris-wilson.co.uk>
>>> Tested-by: Tomas Janousek <tomi@nomi.cz>
>>> Signed-off-by: Don Hiatt <don.hiatt@intel.com>
>>> ---
>>>    drivers/gpu/drm/i915/gt/uc/intel_guc.c | 7 +++++++
>>>    drivers/gpu/drm/i915/gt/uc/intel_uc.c  | 8 ++++++++
>>>    drivers/gpu/drm/i915/i915_drv.h        | 6 ++++++
>>>    3 files changed, 21 insertions(+)
>>>
>>> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc.c
>> b/drivers/gpu/drm/i915/gt/uc/intel_guc.c
>>> index 019ae6486e8d..92d9305c0d73 100644
>>> --- a/drivers/gpu/drm/i915/gt/uc/intel_guc.c
>>> +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc.c
>>> @@ -553,6 +553,13 @@ int intel_guc_suspend(struct intel_guc *guc)
>>>    		GUC_POWER_D1, /* any value greater than GUC_POWER_D0
>> */
>>>    	};
>>>
>>> +	/*
>>> +	 * If GuC communication is enabled but submission is not supported,
>>> +	 * we do not need to suspend the GuC.
>>> +	 */
>>> +	if (!intel_guc_submission_is_enabled(guc))
>>> +		return 0;
>>> +
>>>    	/*
>>>    	 * The ENTER_S_STATE action queues the save/restore operation in GuC
>> FW
>>>    	 * and then returns, so waiting on the H2G is not enough to guarantee
>>> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_uc.c
>> b/drivers/gpu/drm/i915/gt/uc/intel_uc.c
>>> index 629b19377a29..4dd43b99a334 100644
>>> --- a/drivers/gpu/drm/i915/gt/uc/intel_uc.c
>>> +++ b/drivers/gpu/drm/i915/gt/uc/intel_uc.c
>>> @@ -605,6 +605,14 @@ static int __uc_resume(struct intel_uc *uc, bool
>> enable_communication)
>>>    	if (enable_communication)
>>>    		guc_enable_communication(guc);
>>>
>>> +	/*
>>> +	 * If GuC communication is enabled but submission is not supported,
>>> +	 * we do not need to resume the GuC but we do need to enable the
>>> +	 * GuC communication on resume (above).
>>> +	 */
>>> +	if (!intel_guc_submission_is_enabled(guc))
>>> +		return 0;
>>> +
>> I would put this check inside intel_guc_resume(), for symmetry with the
>> one you put inside intel_guc_suspend().
>>
> Will do.
>
>> Daniele
>>
>>>    	err = intel_guc_resume(guc);
>>>    	if (err) {
>>>    		DRM_DEBUG_DRIVER("Failed to resume GuC, err=%d", err);
>>> diff --git a/drivers/gpu/drm/i915/i915_drv.h
>> b/drivers/gpu/drm/i915/i915_drv.h
>>> index 7bc1d8c102b2..2b879472e760 100644
>>> --- a/drivers/gpu/drm/i915/i915_drv.h
>>> +++ b/drivers/gpu/drm/i915/i915_drv.h
>>> @@ -2044,4 +2044,10 @@ i915_coherent_map_type(struct drm_i915_private
>> *i915)
>>>    	return HAS_LLC(i915) ? I915_MAP_WB : I915_MAP_WC;
>>>    }
>>>
>>> +static inline bool intel_guc_submission_is_enabled(struct intel_guc *guc)
>>> +{
>>> +	return intel_guc_is_submission_supported(guc) &&
>>> +		intel_guc_is_running(guc);
>>> +}
>>> +
>>>    #endif
>>>

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

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [Intel-gfx] [PATCH] drm/i915/guc: Skip suspend/resume GuC action on platforms w/o GuC submission
  2019-11-15 22:22   ` Hiatt, Don
@ 2019-11-15 22:22     ` Hiatt, Don
  2019-11-15 23:10     ` Daniele Ceraolo Spurio
  1 sibling, 0 replies; 20+ messages in thread
From: Hiatt, Don @ 2019-11-15 22:22 UTC (permalink / raw)
  To: Ceraolo Spurio, Daniele, intel-gfx
  Cc: S . Zharkoff, KiteStramuort, Tomas Janousek



> From: Ceraolo Spurio, Daniele <daniele.ceraolospurio@intel.com>
> Sent: Friday, November 15, 2019 2:02 PM
> To: Hiatt, Don <don.hiatt@intel.com>; intel-gfx@lists.freedesktop.org
> Cc: Ewins, Jon <jon.ewins@intel.com>; KiteStramuort
> <kitestramuort@autistici.org>; S . Zharkoff <s.zharkoff@gmail.com>;
> Wajdeczko, Michal <Michal.Wajdeczko@intel.com>; Summers, Stuart
> <stuart.summers@intel.com>; Chris Wilson <chris@chris-wilson.co.uk>; Tomas
> Janousek <tomi@nomi.cz>
> Subject: Re: [PATCH] drm/i915/guc: Skip suspend/resume GuC action on
> platforms w/o GuC submission
> 
> 
> 
> On 11/15/19 10:20 AM, don.hiatt@intel.com wrote:
> > From: Don Hiatt <don.hiatt@intel.com>
> >
> > On some platforms (e.g. KBL) that do not support GuC submission, but
> > the user enabled the GuC communication (e.g for HuC authentication)
> > calling the GuC EXIT_S_STATE action results in lose of ability to
> > enter RC6. We can remove the GuC suspend/resume entirely as we do
> > not need to save the GuC submission status.
> >
> > Add intel_guc_submission_is_enabled() function to determine if
> > GuC submission is active.
> >
> > v2: Do not suspend/resume the GuC on platforms that do not support
> >      Guc Submission.
> > v3: Fix typo, move suspend logic to remove goto.
> > v4: Use intel_guc_submission_is_enabled() to check GuC submission
> >      status.
> > v5: No need to look at engine to determine if submission is enabled.
> >      Squash fix + intel_guc_submission_is_enabled() patch into one.
> >
> > Reported-by: KiteStramuort <kitestramuort@autistici.org>
> > Reported-by: S. Zharkoff <s.zharkoff@gmail.com>
> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=111594
> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=111623
> > Fixes: f774f0964919 ("drm/i915/guc: Turn on GuC/HuC auto mode")
> 
> I don't think this Fixes tag is appropriate since we already reverted
> that patch.
> 

Can you suggest an appropriate tag? Chris suggest "guc/huc enabling by default" patch
but I couldn't find it and this one seemed similar.


> > Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
> > Cc: Daniele Ceralo Spurio <daniele.ceraolospurio@intel.com>
> > Cc: Stuart Summers <stuart.summers@intel.com>
> > Cc: Chris Wilson <chris@chris-wilson.co.uk>
> > Tested-by: Tomas Janousek <tomi@nomi.cz>
> > Signed-off-by: Don Hiatt <don.hiatt@intel.com>
> > ---
> >   drivers/gpu/drm/i915/gt/uc/intel_guc.c | 7 +++++++
> >   drivers/gpu/drm/i915/gt/uc/intel_uc.c  | 8 ++++++++
> >   drivers/gpu/drm/i915/i915_drv.h        | 6 ++++++
> >   3 files changed, 21 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc.c
> b/drivers/gpu/drm/i915/gt/uc/intel_guc.c
> > index 019ae6486e8d..92d9305c0d73 100644
> > --- a/drivers/gpu/drm/i915/gt/uc/intel_guc.c
> > +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc.c
> > @@ -553,6 +553,13 @@ int intel_guc_suspend(struct intel_guc *guc)
> >   		GUC_POWER_D1, /* any value greater than GUC_POWER_D0
> */
> >   	};
> >
> > +	/*
> > +	 * If GuC communication is enabled but submission is not supported,
> > +	 * we do not need to suspend the GuC.
> > +	 */
> > +	if (!intel_guc_submission_is_enabled(guc))
> > +		return 0;
> > +
> >   	/*
> >   	 * The ENTER_S_STATE action queues the save/restore operation in GuC
> FW
> >   	 * and then returns, so waiting on the H2G is not enough to guarantee
> > diff --git a/drivers/gpu/drm/i915/gt/uc/intel_uc.c
> b/drivers/gpu/drm/i915/gt/uc/intel_uc.c
> > index 629b19377a29..4dd43b99a334 100644
> > --- a/drivers/gpu/drm/i915/gt/uc/intel_uc.c
> > +++ b/drivers/gpu/drm/i915/gt/uc/intel_uc.c
> > @@ -605,6 +605,14 @@ static int __uc_resume(struct intel_uc *uc, bool
> enable_communication)
> >   	if (enable_communication)
> >   		guc_enable_communication(guc);
> >
> > +	/*
> > +	 * If GuC communication is enabled but submission is not supported,
> > +	 * we do not need to resume the GuC but we do need to enable the
> > +	 * GuC communication on resume (above).
> > +	 */
> > +	if (!intel_guc_submission_is_enabled(guc))
> > +		return 0;
> > +
> 
> I would put this check inside intel_guc_resume(), for symmetry with the
> one you put inside intel_guc_suspend().
> 

Will do.

> Daniele
> 
> >   	err = intel_guc_resume(guc);
> >   	if (err) {
> >   		DRM_DEBUG_DRIVER("Failed to resume GuC, err=%d", err);
> > diff --git a/drivers/gpu/drm/i915/i915_drv.h
> b/drivers/gpu/drm/i915/i915_drv.h
> > index 7bc1d8c102b2..2b879472e760 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.h
> > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > @@ -2044,4 +2044,10 @@ i915_coherent_map_type(struct drm_i915_private
> *i915)
> >   	return HAS_LLC(i915) ? I915_MAP_WB : I915_MAP_WC;
> >   }
> >
> > +static inline bool intel_guc_submission_is_enabled(struct intel_guc *guc)
> > +{
> > +	return intel_guc_is_submission_supported(guc) &&
> > +		intel_guc_is_running(guc);
> > +}
> > +
> >   #endif
> >
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 20+ messages in thread

* Re: [Intel-gfx] [PATCH] drm/i915/guc: Skip suspend/resume GuC action on platforms w/o GuC submission
  2019-11-15 22:02 ` Daniele Ceraolo Spurio
@ 2019-11-15 22:02   ` Daniele Ceraolo Spurio
  2019-11-15 22:22   ` Hiatt, Don
  1 sibling, 0 replies; 20+ messages in thread
From: Daniele Ceraolo Spurio @ 2019-11-15 22:02 UTC (permalink / raw)
  To: don.hiatt, intel-gfx; +Cc: S . Zharkoff, KiteStramuort, Tomas Janousek



On 11/15/19 10:20 AM, don.hiatt@intel.com wrote:
> From: Don Hiatt <don.hiatt@intel.com>
> 
> On some platforms (e.g. KBL) that do not support GuC submission, but
> the user enabled the GuC communication (e.g for HuC authentication)
> calling the GuC EXIT_S_STATE action results in lose of ability to
> enter RC6. We can remove the GuC suspend/resume entirely as we do
> not need to save the GuC submission status.
> 
> Add intel_guc_submission_is_enabled() function to determine if
> GuC submission is active.
> 
> v2: Do not suspend/resume the GuC on platforms that do not support
>      Guc Submission.
> v3: Fix typo, move suspend logic to remove goto.
> v4: Use intel_guc_submission_is_enabled() to check GuC submission
>      status.
> v5: No need to look at engine to determine if submission is enabled.
>      Squash fix + intel_guc_submission_is_enabled() patch into one.
> 
> Reported-by: KiteStramuort <kitestramuort@autistici.org>
> Reported-by: S. Zharkoff <s.zharkoff@gmail.com>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=111594
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=111623
> Fixes: f774f0964919 ("drm/i915/guc: Turn on GuC/HuC auto mode")

I don't think this Fixes tag is appropriate since we already reverted 
that patch.

> Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
> Cc: Daniele Ceralo Spurio <daniele.ceraolospurio@intel.com>
> Cc: Stuart Summers <stuart.summers@intel.com>
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Tested-by: Tomas Janousek <tomi@nomi.cz>
> Signed-off-by: Don Hiatt <don.hiatt@intel.com>
> ---
>   drivers/gpu/drm/i915/gt/uc/intel_guc.c | 7 +++++++
>   drivers/gpu/drm/i915/gt/uc/intel_uc.c  | 8 ++++++++
>   drivers/gpu/drm/i915/i915_drv.h        | 6 ++++++
>   3 files changed, 21 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc.c b/drivers/gpu/drm/i915/gt/uc/intel_guc.c
> index 019ae6486e8d..92d9305c0d73 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_guc.c
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc.c
> @@ -553,6 +553,13 @@ int intel_guc_suspend(struct intel_guc *guc)
>   		GUC_POWER_D1, /* any value greater than GUC_POWER_D0 */
>   	};
>   
> +	/*
> +	 * If GuC communication is enabled but submission is not supported,
> +	 * we do not need to suspend the GuC.
> +	 */
> +	if (!intel_guc_submission_is_enabled(guc))
> +		return 0;
> +
>   	/*
>   	 * The ENTER_S_STATE action queues the save/restore operation in GuC FW
>   	 * and then returns, so waiting on the H2G is not enough to guarantee
> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_uc.c b/drivers/gpu/drm/i915/gt/uc/intel_uc.c
> index 629b19377a29..4dd43b99a334 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_uc.c
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_uc.c
> @@ -605,6 +605,14 @@ static int __uc_resume(struct intel_uc *uc, bool enable_communication)
>   	if (enable_communication)
>   		guc_enable_communication(guc);
>   
> +	/*
> +	 * If GuC communication is enabled but submission is not supported,
> +	 * we do not need to resume the GuC but we do need to enable the
> +	 * GuC communication on resume (above).
> +	 */
> +	if (!intel_guc_submission_is_enabled(guc))
> +		return 0;
> +

I would put this check inside intel_guc_resume(), for symmetry with the 
one you put inside intel_guc_suspend().

Daniele

>   	err = intel_guc_resume(guc);
>   	if (err) {
>   		DRM_DEBUG_DRIVER("Failed to resume GuC, err=%d", err);
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 7bc1d8c102b2..2b879472e760 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -2044,4 +2044,10 @@ i915_coherent_map_type(struct drm_i915_private *i915)
>   	return HAS_LLC(i915) ? I915_MAP_WB : I915_MAP_WC;
>   }
>   
> +static inline bool intel_guc_submission_is_enabled(struct intel_guc *guc)
> +{
> +	return intel_guc_is_submission_supported(guc) &&
> +		intel_guc_is_running(guc);
> +}
> +
>   #endif
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 20+ messages in thread

* [Intel-gfx] [PATCH] drm/i915/guc: Skip suspend/resume GuC action on platforms w/o GuC submission
  2019-11-15 18:20 don.hiatt
@ 2019-11-15 18:20 ` don.hiatt
  2019-11-15 22:02 ` Daniele Ceraolo Spurio
  1 sibling, 0 replies; 20+ messages in thread
From: don.hiatt @ 2019-11-15 18:20 UTC (permalink / raw)
  To: intel-gfx; +Cc: S . Zharkoff, KiteStramuort, Tomas Janousek

From: Don Hiatt <don.hiatt@intel.com>

On some platforms (e.g. KBL) that do not support GuC submission, but
the user enabled the GuC communication (e.g for HuC authentication)
calling the GuC EXIT_S_STATE action results in lose of ability to
enter RC6. We can remove the GuC suspend/resume entirely as we do
not need to save the GuC submission status.

Add intel_guc_submission_is_enabled() function to determine if
GuC submission is active.

v2: Do not suspend/resume the GuC on platforms that do not support
    Guc Submission.
v3: Fix typo, move suspend logic to remove goto.
v4: Use intel_guc_submission_is_enabled() to check GuC submission
    status.
v5: No need to look at engine to determine if submission is enabled.
    Squash fix + intel_guc_submission_is_enabled() patch into one.

Reported-by: KiteStramuort <kitestramuort@autistici.org>
Reported-by: S. Zharkoff <s.zharkoff@gmail.com>
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=111594
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=111623
Fixes: f774f0964919 ("drm/i915/guc: Turn on GuC/HuC auto mode")
Cc: Michal Wajdeczko <michal.wajdeczko@intel.com>
Cc: Daniele Ceralo Spurio <daniele.ceraolospurio@intel.com>
Cc: Stuart Summers <stuart.summers@intel.com>
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Tested-by: Tomas Janousek <tomi@nomi.cz>
Signed-off-by: Don Hiatt <don.hiatt@intel.com>
---
 drivers/gpu/drm/i915/gt/uc/intel_guc.c | 7 +++++++
 drivers/gpu/drm/i915/gt/uc/intel_uc.c  | 8 ++++++++
 drivers/gpu/drm/i915/i915_drv.h        | 6 ++++++
 3 files changed, 21 insertions(+)

diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc.c b/drivers/gpu/drm/i915/gt/uc/intel_guc.c
index 019ae6486e8d..92d9305c0d73 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc.c
@@ -553,6 +553,13 @@ int intel_guc_suspend(struct intel_guc *guc)
 		GUC_POWER_D1, /* any value greater than GUC_POWER_D0 */
 	};
 
+	/*
+	 * If GuC communication is enabled but submission is not supported,
+	 * we do not need to suspend the GuC.
+	 */
+	if (!intel_guc_submission_is_enabled(guc))
+		return 0;
+
 	/*
 	 * The ENTER_S_STATE action queues the save/restore operation in GuC FW
 	 * and then returns, so waiting on the H2G is not enough to guarantee
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_uc.c b/drivers/gpu/drm/i915/gt/uc/intel_uc.c
index 629b19377a29..4dd43b99a334 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_uc.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_uc.c
@@ -605,6 +605,14 @@ static int __uc_resume(struct intel_uc *uc, bool enable_communication)
 	if (enable_communication)
 		guc_enable_communication(guc);
 
+	/*
+	 * If GuC communication is enabled but submission is not supported,
+	 * we do not need to resume the GuC but we do need to enable the
+	 * GuC communication on resume (above).
+	 */
+	if (!intel_guc_submission_is_enabled(guc))
+		return 0;
+
 	err = intel_guc_resume(guc);
 	if (err) {
 		DRM_DEBUG_DRIVER("Failed to resume GuC, err=%d", err);
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 7bc1d8c102b2..2b879472e760 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2044,4 +2044,10 @@ i915_coherent_map_type(struct drm_i915_private *i915)
 	return HAS_LLC(i915) ? I915_MAP_WB : I915_MAP_WC;
 }
 
+static inline bool intel_guc_submission_is_enabled(struct intel_guc *guc)
+{
+	return intel_guc_is_submission_supported(guc) &&
+		intel_guc_is_running(guc);
+}
+
 #endif
-- 
2.20.1

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

^ permalink raw reply related	[flat|nested] 20+ messages in thread

* [Intel-gfx] [PATCH] drm/i915/guc: Skip suspend/resume GuC action on platforms w/o GuC submission
  2019-10-30 21:22 [PATCH] drm/i915/guc: Skip suspend/resume GuC action on platforms w/o GuC submission don.hiatt
@ 2019-10-30 21:22 ` don.hiatt
  0 siblings, 0 replies; 20+ messages in thread
From: don.hiatt @ 2019-10-30 21:22 UTC (permalink / raw)
  To: intel-gfx

From: Don Hiatt <don.hiatt@intel.com>

On some platforms (e.g. KBL) that do not support GuC submission, but
the user enabled the GuC communication (e.g for HuC authentication)
calling the GuC EXIT_S_STATE action results in lose of ability to
enter RC6. We can remove the GuC suspend/resume entirely as we do
not need to save the GuC submission status.

v2: Do not suspend/resume the GuC on platforms that do not support
    Guc Submission.
v3: Fix typo, move suspend logic to remove goto.

Signed-off-by: Don Hiatt <don.hiatt@intel.com>
---
 drivers/gpu/drm/i915/gt/uc/intel_guc.c | 7 +++++++
 drivers/gpu/drm/i915/gt/uc/intel_uc.c  | 8 ++++++++
 2 files changed, 15 insertions(+)

diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc.c b/drivers/gpu/drm/i915/gt/uc/intel_guc.c
index f12959182182..070c3db8513a 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc.c
@@ -587,6 +587,13 @@ int intel_guc_suspend(struct intel_guc *guc)
 		GUC_POWER_D1, /* any value greater than GUC_POWER_D0 */
 	};
 
+	/*
+	 * If GuC communication is enabled but submission is not supported,
+	 * we do not need to suspend the GuC.
+	 */
+	if (!guc->submission_supported)
+		return 0;
+
 	/*
 	 * The ENTER_S_STATE action queues the save/restore operation in GuC FW
 	 * and then returns, so waiting on the H2G is not enough to guarantee
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_uc.c b/drivers/gpu/drm/i915/gt/uc/intel_uc.c
index 629b19377a29..e0b59dfbb901 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_uc.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_uc.c
@@ -605,6 +605,14 @@ static int __uc_resume(struct intel_uc *uc, bool enable_communication)
 	if (enable_communication)
 		guc_enable_communication(guc);
 
+	/*
+	 * If GuC communication is enabled but submission is not supported,
+	 * we do not need to resume the GuC but we do need to enable the
+	 * GuC communication on resume (above).
+	 */
+	if (!guc->submission_supported)
+		return 0;
+
 	err = intel_guc_resume(guc);
 	if (err) {
 		DRM_DEBUG_DRIVER("Failed to resume GuC, err=%d", err);
-- 
2.20.1

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

^ permalink raw reply related	[flat|nested] 20+ messages in thread

end of thread, other threads:[~2019-11-16 10:05 UTC | newest]

Thread overview: 20+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-10-28 21:25 [PATCH] drm/i915/guc: Skip suspend/resume GuC action on platforms w/o GuC submission don.hiatt
2019-10-28 21:25 ` [Intel-gfx] " don.hiatt
2019-10-29  0:58 ` ✗ Fi.CI.BAT: failure for " Patchwork
2019-10-29  0:58   ` [Intel-gfx] " Patchwork
2019-10-29 12:33 ` [PATCH] " Michal Wajdeczko
2019-10-29 12:33   ` [Intel-gfx] " Michal Wajdeczko
2019-10-29 21:15   ` Hiatt, Don
2019-10-29 21:15     ` [Intel-gfx] " Hiatt, Don
2019-10-30  7:25   ` Tomas Janousek
2019-10-30  7:25     ` [Intel-gfx] " Tomas Janousek
2019-10-30  7:53 ` ✗ Fi.CI.BUILD: failure for drm/i915/guc: Skip suspend/resume GuC action on platforms w/o GuC submission (rev2) Patchwork
2019-10-30  7:53   ` [Intel-gfx] " Patchwork
2019-10-30 21:22 [PATCH] drm/i915/guc: Skip suspend/resume GuC action on platforms w/o GuC submission don.hiatt
2019-10-30 21:22 ` [Intel-gfx] " don.hiatt
2019-11-15 18:20 don.hiatt
2019-11-15 18:20 ` [Intel-gfx] " don.hiatt
2019-11-15 22:02 ` Daniele Ceraolo Spurio
2019-11-15 22:02   ` [Intel-gfx] " Daniele Ceraolo Spurio
2019-11-15 22:22   ` Hiatt, Don
2019-11-15 22:22     ` [Intel-gfx] " Hiatt, Don
2019-11-15 23:10     ` Daniele Ceraolo Spurio
2019-11-15 23:10       ` [Intel-gfx] " Daniele Ceraolo Spurio
2019-11-15 23:15       ` Hiatt, Don
2019-11-15 23:15         ` [Intel-gfx] " Hiatt, Don
2019-11-15 23:15 don.hiatt
2019-11-15 23:15 ` [Intel-gfx] " don.hiatt
2019-11-16 10:04 ` Chris Wilson
2019-11-16 10:04   ` [Intel-gfx] " Chris Wilson

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).