* [PATCH] drm/amdgpu/dc: Stop dma_resv_lock inversion in commit_tail
@ 2020-07-27 21:30 ` Daniel Vetter
0 siblings, 0 replies; 16+ messages in thread
From: Daniel Vetter @ 2020-07-27 21:30 UTC (permalink / raw)
To: DRI Development
Cc: linux-rdma, Daniel Vetter, intel-gfx, Maarten Lankhorst, amd-gfx,
Chris Wilson, linaro-mm-sig, Daniel Vetter, Christian König,
linux-media
Trying to grab dma_resv_lock while in commit_tail before we've done
all the code that leads to the eventual signalling of the vblank event
(which can be a dma_fence) is deadlock-y. Don't do that.
Here the solution is easy because just grabbing locks to read
something races anyway. We don't need to bother, READ_ONCE is
equivalent. And avoids the locking issue.
v2: Also take into account tmz_surface boolean, plus just delete the
old code.
Cc: linux-media@vger.kernel.org
Cc: linaro-mm-sig@lists.linaro.org
Cc: linux-rdma@vger.kernel.org
Cc: amd-gfx@lists.freedesktop.org
Cc: intel-gfx@lists.freedesktop.org
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Cc: Christian König <christian.koenig@amd.com>
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
---
DC-folks, I think this split out patch from my series here
https://lore.kernel.org/dri-devel/20200707201229.472834-1-daniel.vetter@ffwll.ch/
should be ready for review/merging. I fixed it up a bit so that it's not
just a gross hack :-)
Cheers, Daniel
---
.../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 19 ++++++-------------
1 file changed, 6 insertions(+), 13 deletions(-)
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
index 21ec64fe5527..a20b62b1f2ef 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -6959,20 +6959,13 @@ static void amdgpu_dm_commit_planes(struct drm_atomic_state *state,
DRM_ERROR("Waiting for fences timed out!");
/*
- * TODO This might fail and hence better not used, wait
- * explicitly on fences instead
- * and in general should be called for
- * blocking commit to as per framework helpers
+ * We cannot reserve buffers here, which means the normal flag
+ * access functions don't work. Paper over this with READ_ONCE,
+ * but maybe the flags are invariant enough that not even that
+ * would be needed.
*/
- r = amdgpu_bo_reserve(abo, true);
- if (unlikely(r != 0))
- DRM_ERROR("failed to reserve buffer before flip\n");
-
- amdgpu_bo_get_tiling_flags(abo, &tiling_flags);
-
- tmz_surface = amdgpu_bo_encrypted(abo);
-
- amdgpu_bo_unreserve(abo);
+ tiling_flags = READ_ONCE(abo->tiling_flags);
+ tmz_surface = READ_ONCE(abo->flags) & AMDGPU_GEM_CREATE_ENCRYPTED;
fill_dc_plane_info_and_addr(
dm->adev, new_plane_state, tiling_flags,
--
2.27.0
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply related [flat|nested] 16+ messages in thread* [Intel-gfx] [PATCH] drm/amdgpu/dc: Stop dma_resv_lock inversion in commit_tail
@ 2020-07-27 21:30 ` Daniel Vetter
0 siblings, 0 replies; 16+ messages in thread
From: Daniel Vetter @ 2020-07-27 21:30 UTC (permalink / raw)
To: DRI Development
Cc: linux-rdma, Daniel Vetter, intel-gfx, amd-gfx, Chris Wilson,
linaro-mm-sig, Daniel Vetter, Christian König, linux-media
Trying to grab dma_resv_lock while in commit_tail before we've done
all the code that leads to the eventual signalling of the vblank event
(which can be a dma_fence) is deadlock-y. Don't do that.
Here the solution is easy because just grabbing locks to read
something races anyway. We don't need to bother, READ_ONCE is
equivalent. And avoids the locking issue.
v2: Also take into account tmz_surface boolean, plus just delete the
old code.
Cc: linux-media@vger.kernel.org
Cc: linaro-mm-sig@lists.linaro.org
Cc: linux-rdma@vger.kernel.org
Cc: amd-gfx@lists.freedesktop.org
Cc: intel-gfx@lists.freedesktop.org
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Cc: Christian König <christian.koenig@amd.com>
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
---
DC-folks, I think this split out patch from my series here
https://lore.kernel.org/dri-devel/20200707201229.472834-1-daniel.vetter@ffwll.ch/
should be ready for review/merging. I fixed it up a bit so that it's not
just a gross hack :-)
Cheers, Daniel
---
.../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 19 ++++++-------------
1 file changed, 6 insertions(+), 13 deletions(-)
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
index 21ec64fe5527..a20b62b1f2ef 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -6959,20 +6959,13 @@ static void amdgpu_dm_commit_planes(struct drm_atomic_state *state,
DRM_ERROR("Waiting for fences timed out!");
/*
- * TODO This might fail and hence better not used, wait
- * explicitly on fences instead
- * and in general should be called for
- * blocking commit to as per framework helpers
+ * We cannot reserve buffers here, which means the normal flag
+ * access functions don't work. Paper over this with READ_ONCE,
+ * but maybe the flags are invariant enough that not even that
+ * would be needed.
*/
- r = amdgpu_bo_reserve(abo, true);
- if (unlikely(r != 0))
- DRM_ERROR("failed to reserve buffer before flip\n");
-
- amdgpu_bo_get_tiling_flags(abo, &tiling_flags);
-
- tmz_surface = amdgpu_bo_encrypted(abo);
-
- amdgpu_bo_unreserve(abo);
+ tiling_flags = READ_ONCE(abo->tiling_flags);
+ tmz_surface = READ_ONCE(abo->flags) & AMDGPU_GEM_CREATE_ENCRYPTED;
fill_dc_plane_info_and_addr(
dm->adev, new_plane_state, tiling_flags,
--
2.27.0
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 16+ messages in thread* [PATCH] drm/amdgpu/dc: Stop dma_resv_lock inversion in commit_tail
@ 2020-07-27 21:30 ` Daniel Vetter
0 siblings, 0 replies; 16+ messages in thread
From: Daniel Vetter @ 2020-07-27 21:30 UTC (permalink / raw)
To: DRI Development
Cc: linux-rdma, Daniel Vetter, intel-gfx, amd-gfx, Chris Wilson,
linaro-mm-sig, Daniel Vetter, Christian König, linux-media
Trying to grab dma_resv_lock while in commit_tail before we've done
all the code that leads to the eventual signalling of the vblank event
(which can be a dma_fence) is deadlock-y. Don't do that.
Here the solution is easy because just grabbing locks to read
something races anyway. We don't need to bother, READ_ONCE is
equivalent. And avoids the locking issue.
v2: Also take into account tmz_surface boolean, plus just delete the
old code.
Cc: linux-media@vger.kernel.org
Cc: linaro-mm-sig@lists.linaro.org
Cc: linux-rdma@vger.kernel.org
Cc: amd-gfx@lists.freedesktop.org
Cc: intel-gfx@lists.freedesktop.org
Cc: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Cc: Christian König <christian.koenig@amd.com>
Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
---
DC-folks, I think this split out patch from my series here
https://lore.kernel.org/dri-devel/20200707201229.472834-1-daniel.vetter@ffwll.ch/
should be ready for review/merging. I fixed it up a bit so that it's not
just a gross hack :-)
Cheers, Daniel
---
.../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 19 ++++++-------------
1 file changed, 6 insertions(+), 13 deletions(-)
diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
index 21ec64fe5527..a20b62b1f2ef 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -6959,20 +6959,13 @@ static void amdgpu_dm_commit_planes(struct drm_atomic_state *state,
DRM_ERROR("Waiting for fences timed out!");
/*
- * TODO This might fail and hence better not used, wait
- * explicitly on fences instead
- * and in general should be called for
- * blocking commit to as per framework helpers
+ * We cannot reserve buffers here, which means the normal flag
+ * access functions don't work. Paper over this with READ_ONCE,
+ * but maybe the flags are invariant enough that not even that
+ * would be needed.
*/
- r = amdgpu_bo_reserve(abo, true);
- if (unlikely(r != 0))
- DRM_ERROR("failed to reserve buffer before flip\n");
-
- amdgpu_bo_get_tiling_flags(abo, &tiling_flags);
-
- tmz_surface = amdgpu_bo_encrypted(abo);
-
- amdgpu_bo_unreserve(abo);
+ tiling_flags = READ_ONCE(abo->tiling_flags);
+ tmz_surface = READ_ONCE(abo->flags) & AMDGPU_GEM_CREATE_ENCRYPTED;
fill_dc_plane_info_and_addr(
dm->adev, new_plane_state, tiling_flags,
--
2.27.0
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply related [flat|nested] 16+ messages in thread* [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for drm/amdgpu/dc: Stop dma_resv_lock inversion in commit_tail
2020-07-27 21:30 ` Daniel Vetter
` (2 preceding siblings ...)
(?)
@ 2020-07-27 21:40 ` Patchwork
-1 siblings, 0 replies; 16+ messages in thread
From: Patchwork @ 2020-07-27 21:40 UTC (permalink / raw)
To: Daniel Vetter; +Cc: intel-gfx
== Series Details ==
Series: drm/amdgpu/dc: Stop dma_resv_lock inversion in commit_tail
URL : https://patchwork.freedesktop.org/series/79937/
State : warning
== Summary ==
$ dim checkpatch origin/drm-tip
be92e429f9aa drm/amdgpu/dc: Stop dma_resv_lock inversion in commit_tail
-:60: WARNING:NO_AUTHOR_SIGN_OFF: Missing Signed-off-by: line by nominal patch author 'Daniel Vetter <daniel.vetter@ffwll.ch>'
total: 0 errors, 1 warnings, 0 checks, 26 lines checked
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 16+ messages in thread* [Intel-gfx] ✗ Fi.CI.SPARSE: warning for drm/amdgpu/dc: Stop dma_resv_lock inversion in commit_tail
2020-07-27 21:30 ` Daniel Vetter
` (3 preceding siblings ...)
(?)
@ 2020-07-27 21:40 ` Patchwork
-1 siblings, 0 replies; 16+ messages in thread
From: Patchwork @ 2020-07-27 21:40 UTC (permalink / raw)
To: Daniel Vetter; +Cc: intel-gfx
== Series Details ==
Series: drm/amdgpu/dc: Stop dma_resv_lock inversion in commit_tail
URL : https://patchwork.freedesktop.org/series/79937/
State : warning
== Summary ==
$ dim sparse --fast origin/drm-tip
Sparse version: v0.6.0
Fast mode used, each commit won't be checked separately.
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 16+ messages in thread* [Intel-gfx] ✓ Fi.CI.BAT: success for drm/amdgpu/dc: Stop dma_resv_lock inversion in commit_tail
2020-07-27 21:30 ` Daniel Vetter
` (4 preceding siblings ...)
(?)
@ 2020-07-27 22:01 ` Patchwork
-1 siblings, 0 replies; 16+ messages in thread
From: Patchwork @ 2020-07-27 22:01 UTC (permalink / raw)
To: Daniel Vetter; +Cc: intel-gfx
[-- Attachment #1.1: Type: text/plain, Size: 6853 bytes --]
== Series Details ==
Series: drm/amdgpu/dc: Stop dma_resv_lock inversion in commit_tail
URL : https://patchwork.freedesktop.org/series/79937/
State : success
== Summary ==
CI Bug Log - changes from CI_DRM_8799 -> Patchwork_18246
====================================================
Summary
-------
**SUCCESS**
No regressions found.
External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18246/index.html
Known issues
------------
Here are the changes found in Patchwork_18246 that come from known issues:
### IGT changes ###
#### Issues hit ####
* igt@i915_module_load@reload:
- fi-byt-j1900: [PASS][1] -> [DMESG-WARN][2] ([i915#1982]) +2 similar issues
[1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8799/fi-byt-j1900/igt@i915_module_load@reload.html
[2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18246/fi-byt-j1900/igt@i915_module_load@reload.html
* igt@kms_cursor_legacy@basic-flip-after-cursor-atomic:
- fi-icl-u2: [PASS][3] -> [DMESG-WARN][4] ([i915#1982]) +1 similar issue
[3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8799/fi-icl-u2/igt@kms_cursor_legacy@basic-flip-after-cursor-atomic.html
[4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18246/fi-icl-u2/igt@kms_cursor_legacy@basic-flip-after-cursor-atomic.html
* igt@prime_vgem@basic-write:
- fi-tgl-y: [PASS][5] -> [DMESG-WARN][6] ([i915#402]) +1 similar issue
[5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8799/fi-tgl-y/igt@prime_vgem@basic-write.html
[6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18246/fi-tgl-y/igt@prime_vgem@basic-write.html
#### Possible fixes ####
* igt@gem_exec_suspend@basic-s3:
- fi-tgl-u2: [FAIL][7] ([i915#1888]) -> [PASS][8]
[7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8799/fi-tgl-u2/igt@gem_exec_suspend@basic-s3.html
[8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18246/fi-tgl-u2/igt@gem_exec_suspend@basic-s3.html
* igt@gem_flink_basic@double-flink:
- fi-tgl-y: [DMESG-WARN][9] ([i915#402]) -> [PASS][10] +1 similar issue
[9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8799/fi-tgl-y/igt@gem_flink_basic@double-flink.html
[10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18246/fi-tgl-y/igt@gem_flink_basic@double-flink.html
* igt@i915_module_load@reload:
- {fi-tgl-dsi}: [DMESG-WARN][11] ([i915#1982]) -> [PASS][12] +1 similar issue
[11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8799/fi-tgl-dsi/igt@i915_module_load@reload.html
[12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18246/fi-tgl-dsi/igt@i915_module_load@reload.html
- fi-bxt-dsi: [DMESG-WARN][13] ([i915#1635] / [i915#1982]) -> [PASS][14]
[13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8799/fi-bxt-dsi/igt@i915_module_load@reload.html
[14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18246/fi-bxt-dsi/igt@i915_module_load@reload.html
* igt@i915_selftest@live@execlists:
- fi-cfl-8700k: [INCOMPLETE][15] ([i915#2089]) -> [PASS][16]
[15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8799/fi-cfl-8700k/igt@i915_selftest@live@execlists.html
[16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18246/fi-cfl-8700k/igt@i915_selftest@live@execlists.html
* igt@kms_pipe_crc_basic@read-crc-pipe-a-frame-sequence:
- fi-tgl-u2: [DMESG-WARN][17] ([i915#402]) -> [PASS][18]
[17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8799/fi-tgl-u2/igt@kms_pipe_crc_basic@read-crc-pipe-a-frame-sequence.html
[18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18246/fi-tgl-u2/igt@kms_pipe_crc_basic@read-crc-pipe-a-frame-sequence.html
#### Warnings ####
* igt@i915_module_load@reload:
- fi-tgl-u2: [DMESG-WARN][19] ([i915#402]) -> [DMESG-WARN][20] ([i915#1982])
[19]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8799/fi-tgl-u2/igt@i915_module_load@reload.html
[20]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18246/fi-tgl-u2/igt@i915_module_load@reload.html
* igt@i915_pm_rpm@module-reload:
- fi-kbl-x1275: [SKIP][21] ([fdo#109271]) -> [DMESG-FAIL][22] ([i915#62])
[21]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8799/fi-kbl-x1275/igt@i915_pm_rpm@module-reload.html
[22]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18246/fi-kbl-x1275/igt@i915_pm_rpm@module-reload.html
* igt@kms_force_connector_basic@force-edid:
- fi-kbl-x1275: [DMESG-WARN][23] ([i915#62] / [i915#92]) -> [DMESG-WARN][24] ([i915#62] / [i915#92] / [i915#95]) +8 similar issues
[23]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8799/fi-kbl-x1275/igt@kms_force_connector_basic@force-edid.html
[24]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18246/fi-kbl-x1275/igt@kms_force_connector_basic@force-edid.html
* igt@kms_pipe_crc_basic@read-crc-pipe-a-frame-sequence:
- fi-kbl-x1275: [DMESG-WARN][25] ([i915#62] / [i915#92] / [i915#95]) -> [DMESG-WARN][26] ([i915#62] / [i915#92]) +1 similar issue
[25]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8799/fi-kbl-x1275/igt@kms_pipe_crc_basic@read-crc-pipe-a-frame-sequence.html
[26]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18246/fi-kbl-x1275/igt@kms_pipe_crc_basic@read-crc-pipe-a-frame-sequence.html
{name}: This element is suppressed. This means it is ignored when computing
the status of the difference (SUCCESS, WARNING, or FAILURE).
[fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271
[i915#1635]: https://gitlab.freedesktop.org/drm/intel/issues/1635
[i915#1888]: https://gitlab.freedesktop.org/drm/intel/issues/1888
[i915#1982]: https://gitlab.freedesktop.org/drm/intel/issues/1982
[i915#2089]: https://gitlab.freedesktop.org/drm/intel/issues/2089
[i915#402]: https://gitlab.freedesktop.org/drm/intel/issues/402
[i915#62]: https://gitlab.freedesktop.org/drm/intel/issues/62
[i915#92]: https://gitlab.freedesktop.org/drm/intel/issues/92
[i915#95]: https://gitlab.freedesktop.org/drm/intel/issues/95
Participating hosts (46 -> 40)
------------------------------
Missing (6): fi-ilk-m540 fi-hsw-4200u fi-byt-squawks fi-bsw-cyan fi-byt-clapper fi-bdw-samus
Build changes
-------------
* Linux: CI_DRM_8799 -> Patchwork_18246
CI-20190529: 20190529
CI_DRM_8799: f8675332cfe857f95352e32cb329416941d90725 @ git://anongit.freedesktop.org/gfx-ci/linux
IGT_5749: 2fef871e791ceab7841b899691c443167550173d @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
Patchwork_18246: be92e429f9aa6760c8f355a4d4e4fbe1b82ee29a @ git://anongit.freedesktop.org/gfx-ci/linux
== Linux commits ==
be92e429f9aa drm/amdgpu/dc: Stop dma_resv_lock inversion in commit_tail
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18246/index.html
[-- Attachment #1.2: Type: text/html, Size: 8758 bytes --]
[-- Attachment #2: Type: text/plain, Size: 160 bytes --]
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 16+ messages in thread* [Intel-gfx] ✓ Fi.CI.IGT: success for drm/amdgpu/dc: Stop dma_resv_lock inversion in commit_tail
2020-07-27 21:30 ` Daniel Vetter
` (5 preceding siblings ...)
(?)
@ 2020-07-27 23:24 ` Patchwork
-1 siblings, 0 replies; 16+ messages in thread
From: Patchwork @ 2020-07-27 23:24 UTC (permalink / raw)
To: Daniel Vetter; +Cc: intel-gfx
[-- Attachment #1.1: Type: text/plain, Size: 16109 bytes --]
== Series Details ==
Series: drm/amdgpu/dc: Stop dma_resv_lock inversion in commit_tail
URL : https://patchwork.freedesktop.org/series/79937/
State : success
== Summary ==
CI Bug Log - changes from CI_DRM_8799_full -> Patchwork_18246_full
====================================================
Summary
-------
**SUCCESS**
No regressions found.
Known issues
------------
Here are the changes found in Patchwork_18246_full that come from known issues:
### IGT changes ###
#### Issues hit ####
* igt@gem_ctx_persistence@engines-mixed-process@vcs0:
- shard-skl: [PASS][1] -> [FAIL][2] ([i915#1528])
[1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8799/shard-skl9/igt@gem_ctx_persistence@engines-mixed-process@vcs0.html
[2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18246/shard-skl10/igt@gem_ctx_persistence@engines-mixed-process@vcs0.html
* igt@gem_mmap_gtt@fault-concurrent:
- shard-skl: [PASS][3] -> [DMESG-WARN][4] ([i915#2165])
[3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8799/shard-skl10/igt@gem_mmap_gtt@fault-concurrent.html
[4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18246/shard-skl4/igt@gem_mmap_gtt@fault-concurrent.html
* igt@kms_big_fb@yf-tiled-32bpp-rotate-270:
- shard-apl: [PASS][5] -> [DMESG-WARN][6] ([i915#1635] / [i915#1982])
[5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8799/shard-apl1/igt@kms_big_fb@yf-tiled-32bpp-rotate-270.html
[6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18246/shard-apl1/igt@kms_big_fb@yf-tiled-32bpp-rotate-270.html
* igt@kms_color@pipe-b-ctm-negative:
- shard-skl: [PASS][7] -> [DMESG-WARN][8] ([i915#1982]) +10 similar issues
[7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8799/shard-skl1/igt@kms_color@pipe-b-ctm-negative.html
[8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18246/shard-skl5/igt@kms_color@pipe-b-ctm-negative.html
* igt@kms_flip@flip-vs-expired-vblank@a-edp1:
- shard-skl: [PASS][9] -> [FAIL][10] ([i915#79])
[9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8799/shard-skl9/igt@kms_flip@flip-vs-expired-vblank@a-edp1.html
[10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18246/shard-skl10/igt@kms_flip@flip-vs-expired-vblank@a-edp1.html
* igt@kms_flip@flip-vs-suspend-interruptible@a-dp1:
- shard-kbl: [PASS][11] -> [DMESG-WARN][12] ([i915#180]) +8 similar issues
[11]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8799/shard-kbl7/igt@kms_flip@flip-vs-suspend-interruptible@a-dp1.html
[12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18246/shard-kbl3/igt@kms_flip@flip-vs-suspend-interruptible@a-dp1.html
* igt@kms_flip@flip-vs-suspend@a-edp1:
- shard-skl: [PASS][13] -> [INCOMPLETE][14] ([i915#198])
[13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8799/shard-skl7/igt@kms_flip@flip-vs-suspend@a-edp1.html
[14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18246/shard-skl9/igt@kms_flip@flip-vs-suspend@a-edp1.html
* igt@kms_flip_tiling@flip-yf-tiled:
- shard-kbl: [PASS][15] -> [DMESG-WARN][16] ([i915#1982])
[15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8799/shard-kbl1/igt@kms_flip_tiling@flip-yf-tiled.html
[16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18246/shard-kbl7/igt@kms_flip_tiling@flip-yf-tiled.html
* igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-indfb-msflip-blt:
- shard-tglb: [PASS][17] -> [DMESG-WARN][18] ([i915#1982])
[17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8799/shard-tglb1/igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-indfb-msflip-blt.html
[18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18246/shard-tglb7/igt@kms_frontbuffer_tracking@fbcpsr-1p-primscrn-indfb-msflip-blt.html
* igt@kms_hdr@bpc-switch-dpms:
- shard-skl: [PASS][19] -> [FAIL][20] ([i915#1188]) +1 similar issue
[19]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8799/shard-skl4/igt@kms_hdr@bpc-switch-dpms.html
[20]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18246/shard-skl9/igt@kms_hdr@bpc-switch-dpms.html
* igt@kms_psr@psr2_primary_blt:
- shard-iclb: [PASS][21] -> [SKIP][22] ([fdo#109441])
[21]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8799/shard-iclb2/igt@kms_psr@psr2_primary_blt.html
[22]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18246/shard-iclb8/igt@kms_psr@psr2_primary_blt.html
* igt@perf_pmu@semaphore-busy@rcs0:
- shard-kbl: [PASS][23] -> [FAIL][24] ([i915#1820])
[23]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8799/shard-kbl2/igt@perf_pmu@semaphore-busy@rcs0.html
[24]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18246/shard-kbl2/igt@perf_pmu@semaphore-busy@rcs0.html
#### Possible fixes ####
* igt@gem_eio@in-flight-suspend:
- shard-skl: [DMESG-WARN][25] ([i915#1982]) -> [PASS][26] +10 similar issues
[25]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8799/shard-skl3/igt@gem_eio@in-flight-suspend.html
[26]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18246/shard-skl5/igt@gem_eio@in-flight-suspend.html
* igt@gem_exec_whisper@basic-contexts:
- shard-glk: [DMESG-WARN][27] ([i915#118] / [i915#95]) -> [PASS][28]
[27]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8799/shard-glk8/igt@gem_exec_whisper@basic-contexts.html
[28]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18246/shard-glk4/igt@gem_exec_whisper@basic-contexts.html
* igt@gen9_exec_parse@allowed-single:
- shard-glk: [DMESG-WARN][29] ([i915#1436] / [i915#716]) -> [PASS][30]
[29]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8799/shard-glk5/igt@gen9_exec_parse@allowed-single.html
[30]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18246/shard-glk2/igt@gen9_exec_parse@allowed-single.html
* igt@kms_atomic_transition@1x-modeset-transitions-nonblocking-fencing:
- shard-tglb: [DMESG-WARN][31] ([i915#402]) -> [PASS][32]
[31]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8799/shard-tglb1/igt@kms_atomic_transition@1x-modeset-transitions-nonblocking-fencing.html
[32]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18246/shard-tglb7/igt@kms_atomic_transition@1x-modeset-transitions-nonblocking-fencing.html
* igt@kms_big_fb@linear-64bpp-rotate-0:
- shard-glk: [DMESG-FAIL][33] ([i915#118] / [i915#95]) -> [PASS][34]
[33]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8799/shard-glk8/igt@kms_big_fb@linear-64bpp-rotate-0.html
[34]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18246/shard-glk6/igt@kms_big_fb@linear-64bpp-rotate-0.html
* igt@kms_cursor_crc@pipe-c-cursor-suspend:
- shard-kbl: [DMESG-WARN][35] ([i915#180]) -> [PASS][36] +4 similar issues
[35]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8799/shard-kbl2/igt@kms_cursor_crc@pipe-c-cursor-suspend.html
[36]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18246/shard-kbl2/igt@kms_cursor_crc@pipe-c-cursor-suspend.html
* igt@kms_cursor_edge_walk@pipe-c-64x64-top-edge:
- shard-glk: [DMESG-WARN][37] ([i915#1982]) -> [PASS][38]
[37]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8799/shard-glk9/igt@kms_cursor_edge_walk@pipe-c-64x64-top-edge.html
[38]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18246/shard-glk1/igt@kms_cursor_edge_walk@pipe-c-64x64-top-edge.html
* igt@kms_cursor_legacy@2x-long-flip-vs-cursor-atomic:
- shard-glk: [FAIL][39] ([i915#72]) -> [PASS][40]
[39]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8799/shard-glk9/igt@kms_cursor_legacy@2x-long-flip-vs-cursor-atomic.html
[40]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18246/shard-glk1/igt@kms_cursor_legacy@2x-long-flip-vs-cursor-atomic.html
* igt@kms_flip_tiling@flip-changes-tiling-y:
- shard-skl: [FAIL][41] ([i915#699]) -> [PASS][42]
[41]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8799/shard-skl1/igt@kms_flip_tiling@flip-changes-tiling-y.html
[42]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18246/shard-skl5/igt@kms_flip_tiling@flip-changes-tiling-y.html
* igt@kms_hdr@bpc-switch-suspend:
- shard-skl: [FAIL][43] ([i915#1188]) -> [PASS][44]
[43]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8799/shard-skl6/igt@kms_hdr@bpc-switch-suspend.html
[44]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18246/shard-skl3/igt@kms_hdr@bpc-switch-suspend.html
* igt@kms_psr@psr2_sprite_plane_move:
- shard-iclb: [SKIP][45] ([fdo#109441]) -> [PASS][46] +2 similar issues
[45]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8799/shard-iclb4/igt@kms_psr@psr2_sprite_plane_move.html
[46]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18246/shard-iclb2/igt@kms_psr@psr2_sprite_plane_move.html
* igt@perf_pmu@module-unload:
- shard-apl: [DMESG-WARN][47] ([i915#1635] / [i915#1982]) -> [PASS][48]
[47]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8799/shard-apl4/igt@perf_pmu@module-unload.html
[48]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18246/shard-apl8/igt@perf_pmu@module-unload.html
#### Warnings ####
* igt@i915_pm_dc@dc3co-vpb-simulation:
- shard-iclb: [SKIP][49] ([i915#658]) -> [SKIP][50] ([i915#588])
[49]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8799/shard-iclb6/igt@i915_pm_dc@dc3co-vpb-simulation.html
[50]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18246/shard-iclb2/igt@i915_pm_dc@dc3co-vpb-simulation.html
* igt@kms_plane@plane-panning-bottom-right-suspend-pipe-b-planes:
- shard-kbl: [DMESG-WARN][51] ([i915#180]) -> [INCOMPLETE][52] ([i915#155])
[51]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8799/shard-kbl7/igt@kms_plane@plane-panning-bottom-right-suspend-pipe-b-planes.html
[52]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18246/shard-kbl4/igt@kms_plane@plane-panning-bottom-right-suspend-pipe-b-planes.html
* igt@kms_plane_alpha_blend@pipe-c-coverage-7efc:
- shard-skl: [FAIL][53] ([fdo#108145] / [i915#265]) -> [DMESG-FAIL][54] ([fdo#108145] / [i915#1982]) +2 similar issues
[53]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8799/shard-skl8/igt@kms_plane_alpha_blend@pipe-c-coverage-7efc.html
[54]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18246/shard-skl6/igt@kms_plane_alpha_blend@pipe-c-coverage-7efc.html
* igt@runner@aborted:
- shard-apl: ([FAIL][55], [FAIL][56], [FAIL][57], [FAIL][58], [FAIL][59], [FAIL][60], [FAIL][61], [FAIL][62], [FAIL][63], [FAIL][64], [FAIL][65], [FAIL][66], [FAIL][67]) ([i915#1610] / [i915#1635] / [i915#2110] / [i915#637]) -> [FAIL][68] ([i915#1635] / [i915#2110])
[55]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8799/shard-apl6/igt@runner@aborted.html
[56]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8799/shard-apl7/igt@runner@aborted.html
[57]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8799/shard-apl3/igt@runner@aborted.html
[58]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8799/shard-apl1/igt@runner@aborted.html
[59]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8799/shard-apl4/igt@runner@aborted.html
[60]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8799/shard-apl6/igt@runner@aborted.html
[61]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8799/shard-apl2/igt@runner@aborted.html
[62]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8799/shard-apl4/igt@runner@aborted.html
[63]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8799/shard-apl3/igt@runner@aborted.html
[64]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8799/shard-apl6/igt@runner@aborted.html
[65]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8799/shard-apl8/igt@runner@aborted.html
[66]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8799/shard-apl7/igt@runner@aborted.html
[67]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8799/shard-apl2/igt@runner@aborted.html
[68]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18246/shard-apl8/igt@runner@aborted.html
- shard-skl: ([FAIL][69], [FAIL][70], [FAIL][71], [FAIL][72], [FAIL][73], [FAIL][74], [FAIL][75], [FAIL][76], [FAIL][77], [FAIL][78], [FAIL][79]) ([i915#1436] / [i915#2110]) -> ([FAIL][80], [FAIL][81]) ([i915#1436] / [i915#1611] / [i915#2110])
[69]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8799/shard-skl3/igt@runner@aborted.html
[70]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8799/shard-skl9/igt@runner@aborted.html
[71]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8799/shard-skl5/igt@runner@aborted.html
[72]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8799/shard-skl10/igt@runner@aborted.html
[73]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8799/shard-skl2/igt@runner@aborted.html
[74]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8799/shard-skl8/igt@runner@aborted.html
[75]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8799/shard-skl4/igt@runner@aborted.html
[76]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8799/shard-skl3/igt@runner@aborted.html
[77]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8799/shard-skl9/igt@runner@aborted.html
[78]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8799/shard-skl6/igt@runner@aborted.html
[79]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_8799/shard-skl6/igt@runner@aborted.html
[80]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18246/shard-skl4/igt@runner@aborted.html
[81]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18246/shard-skl8/igt@runner@aborted.html
[fdo#108145]: https://bugs.freedesktop.org/show_bug.cgi?id=108145
[fdo#109441]: https://bugs.freedesktop.org/show_bug.cgi?id=109441
[i915#118]: https://gitlab.freedesktop.org/drm/intel/issues/118
[i915#1188]: https://gitlab.freedesktop.org/drm/intel/issues/1188
[i915#1436]: https://gitlab.freedesktop.org/drm/intel/issues/1436
[i915#1528]: https://gitlab.freedesktop.org/drm/intel/issues/1528
[i915#155]: https://gitlab.freedesktop.org/drm/intel/issues/155
[i915#1610]: https://gitlab.freedesktop.org/drm/intel/issues/1610
[i915#1611]: https://gitlab.freedesktop.org/drm/intel/issues/1611
[i915#1635]: https://gitlab.freedesktop.org/drm/intel/issues/1635
[i915#180]: https://gitlab.freedesktop.org/drm/intel/issues/180
[i915#1820]: https://gitlab.freedesktop.org/drm/intel/issues/1820
[i915#198]: https://gitlab.freedesktop.org/drm/intel/issues/198
[i915#1982]: https://gitlab.freedesktop.org/drm/intel/issues/1982
[i915#2110]: https://gitlab.freedesktop.org/drm/intel/issues/2110
[i915#2165]: https://gitlab.freedesktop.org/drm/intel/issues/2165
[i915#265]: https://gitlab.freedesktop.org/drm/intel/issues/265
[i915#402]: https://gitlab.freedesktop.org/drm/intel/issues/402
[i915#588]: https://gitlab.freedesktop.org/drm/intel/issues/588
[i915#637]: https://gitlab.freedesktop.org/drm/intel/issues/637
[i915#658]: https://gitlab.freedesktop.org/drm/intel/issues/658
[i915#699]: https://gitlab.freedesktop.org/drm/intel/issues/699
[i915#716]: https://gitlab.freedesktop.org/drm/intel/issues/716
[i915#72]: https://gitlab.freedesktop.org/drm/intel/issues/72
[i915#79]: https://gitlab.freedesktop.org/drm/intel/issues/79
[i915#95]: https://gitlab.freedesktop.org/drm/intel/issues/95
Participating hosts (12 -> 11)
------------------------------
Missing (1): pig-snb-2600
Build changes
-------------
* Linux: CI_DRM_8799 -> Patchwork_18246
CI-20190529: 20190529
CI_DRM_8799: f8675332cfe857f95352e32cb329416941d90725 @ git://anongit.freedesktop.org/gfx-ci/linux
IGT_5749: 2fef871e791ceab7841b899691c443167550173d @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
Patchwork_18246: be92e429f9aa6760c8f355a4d4e4fbe1b82ee29a @ git://anongit.freedesktop.org/gfx-ci/linux
piglit_4509: fdc5a4ca11124ab8413c7988896eec4c97336694 @ git://anongit.freedesktop.org/piglit
== Logs ==
For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_18246/index.html
[-- Attachment #1.2: Type: text/html, Size: 19068 bytes --]
[-- Attachment #2: Type: text/plain, Size: 160 bytes --]
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [PATCH] drm/amdgpu/dc: Stop dma_resv_lock inversion in commit_tail
2020-07-27 21:30 ` Daniel Vetter
(?)
(?)
@ 2020-07-28 6:56 ` Christian König
-1 siblings, 0 replies; 16+ messages in thread
From: Christian König @ 2020-07-28 6:56 UTC (permalink / raw)
To: Daniel Vetter, DRI Development
Cc: linux-media, linaro-mm-sig, linux-rdma, amd-gfx, intel-gfx,
Chris Wilson, Maarten Lankhorst, Daniel Vetter
Am 27.07.20 um 23:30 schrieb Daniel Vetter:
> Trying to grab dma_resv_lock while in commit_tail before we've done
> all the code that leads to the eventual signalling of the vblank event
> (which can be a dma_fence) is deadlock-y. Don't do that.
>
> Here the solution is easy because just grabbing locks to read
> something races anyway. We don't need to bother, READ_ONCE is
> equivalent. And avoids the locking issue.
>
> v2: Also take into account tmz_surface boolean, plus just delete the
> old code.
>
> Cc: linux-media@vger.kernel.org
> Cc: linaro-mm-sig@lists.linaro.org
> Cc: linux-rdma@vger.kernel.org
> Cc: amd-gfx@lists.freedesktop.org
> Cc: intel-gfx@lists.freedesktop.org
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Cc: Christian König <christian.koenig@amd.com>
> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> ---
> DC-folks, I think this split out patch from my series here
>
> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flore.kernel.org%2Fdri-devel%2F20200707201229.472834-1-daniel.vetter%40ffwll.ch%2F&data=02%7C01%7Cchristian.koenig%40amd.com%7C8a4f5736682a4b5c943e08d832747ab1%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637314823145521840&sdata=qd7Nrox62Lr%2FXWbJJFVskg9RYL4%2FoRVCFjR6rUDMA5E%3D&reserved=0
>
> should be ready for review/merging. I fixed it up a bit so that it's not
> just a gross hack :-)
>
> Cheers, Daniel
>
>
> ---
> .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 19 ++++++-------------
> 1 file changed, 6 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> index 21ec64fe5527..a20b62b1f2ef 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> @@ -6959,20 +6959,13 @@ static void amdgpu_dm_commit_planes(struct drm_atomic_state *state,
> DRM_ERROR("Waiting for fences timed out!");
>
> /*
> - * TODO This might fail and hence better not used, wait
> - * explicitly on fences instead
> - * and in general should be called for
> - * blocking commit to as per framework helpers
> + * We cannot reserve buffers here, which means the normal flag
> + * access functions don't work. Paper over this with READ_ONCE,
> + * but maybe the flags are invariant enough that not even that
> + * would be needed.
> */
> - r = amdgpu_bo_reserve(abo, true);
> - if (unlikely(r != 0))
> - DRM_ERROR("failed to reserve buffer before flip\n");
> -
> - amdgpu_bo_get_tiling_flags(abo, &tiling_flags);
> -
> - tmz_surface = amdgpu_bo_encrypted(abo);
> -
> - amdgpu_bo_unreserve(abo);
> + tiling_flags = READ_ONCE(abo->tiling_flags);
> + tmz_surface = READ_ONCE(abo->flags) & AMDGPU_GEM_CREATE_ENCRYPTED;
Yeah, the abo->flags are mostly fixed after creation, especially the
encrypted flag can't change or we corrupt page table tables. So that
should work fine.
Anybody who picks this up feel free to add an Reviewed-by: Christian
König <christian.koenig@amd.com>.
Regards,
Christian.
>
> fill_dc_plane_info_and_addr(
> dm->adev, new_plane_state, tiling_flags,
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [PATCH] drm/amdgpu/dc: Stop dma_resv_lock inversion in commit_tail
@ 2020-07-28 6:56 ` Christian König
0 siblings, 0 replies; 16+ messages in thread
From: Christian König @ 2020-07-28 6:56 UTC (permalink / raw)
To: Daniel Vetter, DRI Development
Cc: linux-rdma, intel-gfx, Maarten Lankhorst, amd-gfx, Chris Wilson,
linaro-mm-sig, Daniel Vetter, linux-media
Am 27.07.20 um 23:30 schrieb Daniel Vetter:
> Trying to grab dma_resv_lock while in commit_tail before we've done
> all the code that leads to the eventual signalling of the vblank event
> (which can be a dma_fence) is deadlock-y. Don't do that.
>
> Here the solution is easy because just grabbing locks to read
> something races anyway. We don't need to bother, READ_ONCE is
> equivalent. And avoids the locking issue.
>
> v2: Also take into account tmz_surface boolean, plus just delete the
> old code.
>
> Cc: linux-media@vger.kernel.org
> Cc: linaro-mm-sig@lists.linaro.org
> Cc: linux-rdma@vger.kernel.org
> Cc: amd-gfx@lists.freedesktop.org
> Cc: intel-gfx@lists.freedesktop.org
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Cc: Christian König <christian.koenig@amd.com>
> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> ---
> DC-folks, I think this split out patch from my series here
>
> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flore.kernel.org%2Fdri-devel%2F20200707201229.472834-1-daniel.vetter%40ffwll.ch%2F&data=02%7C01%7Cchristian.koenig%40amd.com%7C8a4f5736682a4b5c943e08d832747ab1%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637314823145521840&sdata=qd7Nrox62Lr%2FXWbJJFVskg9RYL4%2FoRVCFjR6rUDMA5E%3D&reserved=0
>
> should be ready for review/merging. I fixed it up a bit so that it's not
> just a gross hack :-)
>
> Cheers, Daniel
>
>
> ---
> .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 19 ++++++-------------
> 1 file changed, 6 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> index 21ec64fe5527..a20b62b1f2ef 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> @@ -6959,20 +6959,13 @@ static void amdgpu_dm_commit_planes(struct drm_atomic_state *state,
> DRM_ERROR("Waiting for fences timed out!");
>
> /*
> - * TODO This might fail and hence better not used, wait
> - * explicitly on fences instead
> - * and in general should be called for
> - * blocking commit to as per framework helpers
> + * We cannot reserve buffers here, which means the normal flag
> + * access functions don't work. Paper over this with READ_ONCE,
> + * but maybe the flags are invariant enough that not even that
> + * would be needed.
> */
> - r = amdgpu_bo_reserve(abo, true);
> - if (unlikely(r != 0))
> - DRM_ERROR("failed to reserve buffer before flip\n");
> -
> - amdgpu_bo_get_tiling_flags(abo, &tiling_flags);
> -
> - tmz_surface = amdgpu_bo_encrypted(abo);
> -
> - amdgpu_bo_unreserve(abo);
> + tiling_flags = READ_ONCE(abo->tiling_flags);
> + tmz_surface = READ_ONCE(abo->flags) & AMDGPU_GEM_CREATE_ENCRYPTED;
Yeah, the abo->flags are mostly fixed after creation, especially the
encrypted flag can't change or we corrupt page table tables. So that
should work fine.
Anybody who picks this up feel free to add an Reviewed-by: Christian
König <christian.koenig@amd.com>.
Regards,
Christian.
>
> fill_dc_plane_info_and_addr(
> dm->adev, new_plane_state, tiling_flags,
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [Intel-gfx] [PATCH] drm/amdgpu/dc: Stop dma_resv_lock inversion in commit_tail
@ 2020-07-28 6:56 ` Christian König
0 siblings, 0 replies; 16+ messages in thread
From: Christian König @ 2020-07-28 6:56 UTC (permalink / raw)
To: Daniel Vetter, DRI Development
Cc: linux-rdma, intel-gfx, amd-gfx, Chris Wilson, linaro-mm-sig,
Daniel Vetter, linux-media
Am 27.07.20 um 23:30 schrieb Daniel Vetter:
> Trying to grab dma_resv_lock while in commit_tail before we've done
> all the code that leads to the eventual signalling of the vblank event
> (which can be a dma_fence) is deadlock-y. Don't do that.
>
> Here the solution is easy because just grabbing locks to read
> something races anyway. We don't need to bother, READ_ONCE is
> equivalent. And avoids the locking issue.
>
> v2: Also take into account tmz_surface boolean, plus just delete the
> old code.
>
> Cc: linux-media@vger.kernel.org
> Cc: linaro-mm-sig@lists.linaro.org
> Cc: linux-rdma@vger.kernel.org
> Cc: amd-gfx@lists.freedesktop.org
> Cc: intel-gfx@lists.freedesktop.org
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Cc: Christian König <christian.koenig@amd.com>
> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> ---
> DC-folks, I think this split out patch from my series here
>
> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flore.kernel.org%2Fdri-devel%2F20200707201229.472834-1-daniel.vetter%40ffwll.ch%2F&data=02%7C01%7Cchristian.koenig%40amd.com%7C8a4f5736682a4b5c943e08d832747ab1%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637314823145521840&sdata=qd7Nrox62Lr%2FXWbJJFVskg9RYL4%2FoRVCFjR6rUDMA5E%3D&reserved=0
>
> should be ready for review/merging. I fixed it up a bit so that it's not
> just a gross hack :-)
>
> Cheers, Daniel
>
>
> ---
> .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 19 ++++++-------------
> 1 file changed, 6 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> index 21ec64fe5527..a20b62b1f2ef 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> @@ -6959,20 +6959,13 @@ static void amdgpu_dm_commit_planes(struct drm_atomic_state *state,
> DRM_ERROR("Waiting for fences timed out!");
>
> /*
> - * TODO This might fail and hence better not used, wait
> - * explicitly on fences instead
> - * and in general should be called for
> - * blocking commit to as per framework helpers
> + * We cannot reserve buffers here, which means the normal flag
> + * access functions don't work. Paper over this with READ_ONCE,
> + * but maybe the flags are invariant enough that not even that
> + * would be needed.
> */
> - r = amdgpu_bo_reserve(abo, true);
> - if (unlikely(r != 0))
> - DRM_ERROR("failed to reserve buffer before flip\n");
> -
> - amdgpu_bo_get_tiling_flags(abo, &tiling_flags);
> -
> - tmz_surface = amdgpu_bo_encrypted(abo);
> -
> - amdgpu_bo_unreserve(abo);
> + tiling_flags = READ_ONCE(abo->tiling_flags);
> + tmz_surface = READ_ONCE(abo->flags) & AMDGPU_GEM_CREATE_ENCRYPTED;
Yeah, the abo->flags are mostly fixed after creation, especially the
encrypted flag can't change or we corrupt page table tables. So that
should work fine.
Anybody who picks this up feel free to add an Reviewed-by: Christian
König <christian.koenig@amd.com>.
Regards,
Christian.
>
> fill_dc_plane_info_and_addr(
> dm->adev, new_plane_state, tiling_flags,
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [PATCH] drm/amdgpu/dc: Stop dma_resv_lock inversion in commit_tail
@ 2020-07-28 6:56 ` Christian König
0 siblings, 0 replies; 16+ messages in thread
From: Christian König @ 2020-07-28 6:56 UTC (permalink / raw)
To: Daniel Vetter, DRI Development
Cc: linux-rdma, intel-gfx, amd-gfx, Chris Wilson, linaro-mm-sig,
Daniel Vetter, linux-media
Am 27.07.20 um 23:30 schrieb Daniel Vetter:
> Trying to grab dma_resv_lock while in commit_tail before we've done
> all the code that leads to the eventual signalling of the vblank event
> (which can be a dma_fence) is deadlock-y. Don't do that.
>
> Here the solution is easy because just grabbing locks to read
> something races anyway. We don't need to bother, READ_ONCE is
> equivalent. And avoids the locking issue.
>
> v2: Also take into account tmz_surface boolean, plus just delete the
> old code.
>
> Cc: linux-media@vger.kernel.org
> Cc: linaro-mm-sig@lists.linaro.org
> Cc: linux-rdma@vger.kernel.org
> Cc: amd-gfx@lists.freedesktop.org
> Cc: intel-gfx@lists.freedesktop.org
> Cc: Chris Wilson <chris@chris-wilson.co.uk>
> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Cc: Christian König <christian.koenig@amd.com>
> Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> ---
> DC-folks, I think this split out patch from my series here
>
> https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flore.kernel.org%2Fdri-devel%2F20200707201229.472834-1-daniel.vetter%40ffwll.ch%2F&data=02%7C01%7Cchristian.koenig%40amd.com%7C8a4f5736682a4b5c943e08d832747ab1%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637314823145521840&sdata=qd7Nrox62Lr%2FXWbJJFVskg9RYL4%2FoRVCFjR6rUDMA5E%3D&reserved=0
>
> should be ready for review/merging. I fixed it up a bit so that it's not
> just a gross hack :-)
>
> Cheers, Daniel
>
>
> ---
> .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 19 ++++++-------------
> 1 file changed, 6 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> index 21ec64fe5527..a20b62b1f2ef 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> @@ -6959,20 +6959,13 @@ static void amdgpu_dm_commit_planes(struct drm_atomic_state *state,
> DRM_ERROR("Waiting for fences timed out!");
>
> /*
> - * TODO This might fail and hence better not used, wait
> - * explicitly on fences instead
> - * and in general should be called for
> - * blocking commit to as per framework helpers
> + * We cannot reserve buffers here, which means the normal flag
> + * access functions don't work. Paper over this with READ_ONCE,
> + * but maybe the flags are invariant enough that not even that
> + * would be needed.
> */
> - r = amdgpu_bo_reserve(abo, true);
> - if (unlikely(r != 0))
> - DRM_ERROR("failed to reserve buffer before flip\n");
> -
> - amdgpu_bo_get_tiling_flags(abo, &tiling_flags);
> -
> - tmz_surface = amdgpu_bo_encrypted(abo);
> -
> - amdgpu_bo_unreserve(abo);
> + tiling_flags = READ_ONCE(abo->tiling_flags);
> + tmz_surface = READ_ONCE(abo->flags) & AMDGPU_GEM_CREATE_ENCRYPTED;
Yeah, the abo->flags are mostly fixed after creation, especially the
encrypted flag can't change or we corrupt page table tables. So that
should work fine.
Anybody who picks this up feel free to add an Reviewed-by: Christian
König <christian.koenig@amd.com>.
Regards,
Christian.
>
> fill_dc_plane_info_and_addr(
> dm->adev, new_plane_state, tiling_flags,
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [PATCH] drm/amdgpu/dc: Stop dma_resv_lock inversion in commit_tail
2020-07-28 6:56 ` Christian König
(?)
(?)
@ 2020-07-29 22:10 ` Alex Deucher
-1 siblings, 0 replies; 16+ messages in thread
From: Alex Deucher @ 2020-07-29 22:10 UTC (permalink / raw)
To: Christian König
Cc: Daniel Vetter, DRI Development, linux-rdma,
Intel Graphics Development, Maarten Lankhorst, amd-gfx list,
Chris Wilson, moderated list:DMA BUFFER SHARING FRAMEWORK,
Daniel Vetter, linux-media
Applied. Thanks!
Alex
On Tue, Jul 28, 2020 at 2:56 AM Christian König
<christian.koenig@amd.com> wrote:
>
> Am 27.07.20 um 23:30 schrieb Daniel Vetter:
> > Trying to grab dma_resv_lock while in commit_tail before we've done
> > all the code that leads to the eventual signalling of the vblank event
> > (which can be a dma_fence) is deadlock-y. Don't do that.
> >
> > Here the solution is easy because just grabbing locks to read
> > something races anyway. We don't need to bother, READ_ONCE is
> > equivalent. And avoids the locking issue.
> >
> > v2: Also take into account tmz_surface boolean, plus just delete the
> > old code.
> >
> > Cc: linux-media@vger.kernel.org
> > Cc: linaro-mm-sig@lists.linaro.org
> > Cc: linux-rdma@vger.kernel.org
> > Cc: amd-gfx@lists.freedesktop.org
> > Cc: intel-gfx@lists.freedesktop.org
> > Cc: Chris Wilson <chris@chris-wilson.co.uk>
> > Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> > Cc: Christian König <christian.koenig@amd.com>
> > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> > ---
> > DC-folks, I think this split out patch from my series here
> >
> > https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flore.kernel.org%2Fdri-devel%2F20200707201229.472834-1-daniel.vetter%40ffwll.ch%2F&data=02%7C01%7Cchristian.koenig%40amd.com%7C8a4f5736682a4b5c943e08d832747ab1%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637314823145521840&sdata=qd7Nrox62Lr%2FXWbJJFVskg9RYL4%2FoRVCFjR6rUDMA5E%3D&reserved=0
> >
> > should be ready for review/merging. I fixed it up a bit so that it's not
> > just a gross hack :-)
> >
> > Cheers, Daniel
> >
> >
> > ---
> > .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 19 ++++++-------------
> > 1 file changed, 6 insertions(+), 13 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> > index 21ec64fe5527..a20b62b1f2ef 100644
> > --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> > +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> > @@ -6959,20 +6959,13 @@ static void amdgpu_dm_commit_planes(struct drm_atomic_state *state,
> > DRM_ERROR("Waiting for fences timed out!");
> >
> > /*
> > - * TODO This might fail and hence better not used, wait
> > - * explicitly on fences instead
> > - * and in general should be called for
> > - * blocking commit to as per framework helpers
> > + * We cannot reserve buffers here, which means the normal flag
> > + * access functions don't work. Paper over this with READ_ONCE,
> > + * but maybe the flags are invariant enough that not even that
> > + * would be needed.
> > */
> > - r = amdgpu_bo_reserve(abo, true);
> > - if (unlikely(r != 0))
> > - DRM_ERROR("failed to reserve buffer before flip\n");
> > -
> > - amdgpu_bo_get_tiling_flags(abo, &tiling_flags);
> > -
> > - tmz_surface = amdgpu_bo_encrypted(abo);
> > -
> > - amdgpu_bo_unreserve(abo);
> > + tiling_flags = READ_ONCE(abo->tiling_flags);
> > + tmz_surface = READ_ONCE(abo->flags) & AMDGPU_GEM_CREATE_ENCRYPTED;
>
> Yeah, the abo->flags are mostly fixed after creation, especially the
> encrypted flag can't change or we corrupt page table tables. So that
> should work fine.
>
> Anybody who picks this up feel free to add an Reviewed-by: Christian
> König <christian.koenig@amd.com>.
>
> Regards,
> Christian.
>
> >
> > fill_dc_plane_info_and_addr(
> > dm->adev, new_plane_state, tiling_flags,
>
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [PATCH] drm/amdgpu/dc: Stop dma_resv_lock inversion in commit_tail
@ 2020-07-29 22:10 ` Alex Deucher
0 siblings, 0 replies; 16+ messages in thread
From: Alex Deucher @ 2020-07-29 22:10 UTC (permalink / raw)
To: Christian König
Cc: linux-rdma, Daniel Vetter, Intel Graphics Development,
Maarten Lankhorst, amd-gfx list, Chris Wilson,
moderated list:DMA BUFFER SHARING FRAMEWORK, DRI Development,
Daniel Vetter, linux-media
Applied. Thanks!
Alex
On Tue, Jul 28, 2020 at 2:56 AM Christian König
<christian.koenig@amd.com> wrote:
>
> Am 27.07.20 um 23:30 schrieb Daniel Vetter:
> > Trying to grab dma_resv_lock while in commit_tail before we've done
> > all the code that leads to the eventual signalling of the vblank event
> > (which can be a dma_fence) is deadlock-y. Don't do that.
> >
> > Here the solution is easy because just grabbing locks to read
> > something races anyway. We don't need to bother, READ_ONCE is
> > equivalent. And avoids the locking issue.
> >
> > v2: Also take into account tmz_surface boolean, plus just delete the
> > old code.
> >
> > Cc: linux-media@vger.kernel.org
> > Cc: linaro-mm-sig@lists.linaro.org
> > Cc: linux-rdma@vger.kernel.org
> > Cc: amd-gfx@lists.freedesktop.org
> > Cc: intel-gfx@lists.freedesktop.org
> > Cc: Chris Wilson <chris@chris-wilson.co.uk>
> > Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> > Cc: Christian König <christian.koenig@amd.com>
> > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> > ---
> > DC-folks, I think this split out patch from my series here
> >
> > https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flore.kernel.org%2Fdri-devel%2F20200707201229.472834-1-daniel.vetter%40ffwll.ch%2F&data=02%7C01%7Cchristian.koenig%40amd.com%7C8a4f5736682a4b5c943e08d832747ab1%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637314823145521840&sdata=qd7Nrox62Lr%2FXWbJJFVskg9RYL4%2FoRVCFjR6rUDMA5E%3D&reserved=0
> >
> > should be ready for review/merging. I fixed it up a bit so that it's not
> > just a gross hack :-)
> >
> > Cheers, Daniel
> >
> >
> > ---
> > .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 19 ++++++-------------
> > 1 file changed, 6 insertions(+), 13 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> > index 21ec64fe5527..a20b62b1f2ef 100644
> > --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> > +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> > @@ -6959,20 +6959,13 @@ static void amdgpu_dm_commit_planes(struct drm_atomic_state *state,
> > DRM_ERROR("Waiting for fences timed out!");
> >
> > /*
> > - * TODO This might fail and hence better not used, wait
> > - * explicitly on fences instead
> > - * and in general should be called for
> > - * blocking commit to as per framework helpers
> > + * We cannot reserve buffers here, which means the normal flag
> > + * access functions don't work. Paper over this with READ_ONCE,
> > + * but maybe the flags are invariant enough that not even that
> > + * would be needed.
> > */
> > - r = amdgpu_bo_reserve(abo, true);
> > - if (unlikely(r != 0))
> > - DRM_ERROR("failed to reserve buffer before flip\n");
> > -
> > - amdgpu_bo_get_tiling_flags(abo, &tiling_flags);
> > -
> > - tmz_surface = amdgpu_bo_encrypted(abo);
> > -
> > - amdgpu_bo_unreserve(abo);
> > + tiling_flags = READ_ONCE(abo->tiling_flags);
> > + tmz_surface = READ_ONCE(abo->flags) & AMDGPU_GEM_CREATE_ENCRYPTED;
>
> Yeah, the abo->flags are mostly fixed after creation, especially the
> encrypted flag can't change or we corrupt page table tables. So that
> should work fine.
>
> Anybody who picks this up feel free to add an Reviewed-by: Christian
> König <christian.koenig@amd.com>.
>
> Regards,
> Christian.
>
> >
> > fill_dc_plane_info_and_addr(
> > dm->adev, new_plane_state, tiling_flags,
>
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [Intel-gfx] [PATCH] drm/amdgpu/dc: Stop dma_resv_lock inversion in commit_tail
@ 2020-07-29 22:10 ` Alex Deucher
0 siblings, 0 replies; 16+ messages in thread
From: Alex Deucher @ 2020-07-29 22:10 UTC (permalink / raw)
To: Christian König
Cc: linux-rdma, Daniel Vetter, Intel Graphics Development,
amd-gfx list, Chris Wilson,
moderated list:DMA BUFFER SHARING FRAMEWORK, DRI Development,
Daniel Vetter, linux-media
Applied. Thanks!
Alex
On Tue, Jul 28, 2020 at 2:56 AM Christian König
<christian.koenig@amd.com> wrote:
>
> Am 27.07.20 um 23:30 schrieb Daniel Vetter:
> > Trying to grab dma_resv_lock while in commit_tail before we've done
> > all the code that leads to the eventual signalling of the vblank event
> > (which can be a dma_fence) is deadlock-y. Don't do that.
> >
> > Here the solution is easy because just grabbing locks to read
> > something races anyway. We don't need to bother, READ_ONCE is
> > equivalent. And avoids the locking issue.
> >
> > v2: Also take into account tmz_surface boolean, plus just delete the
> > old code.
> >
> > Cc: linux-media@vger.kernel.org
> > Cc: linaro-mm-sig@lists.linaro.org
> > Cc: linux-rdma@vger.kernel.org
> > Cc: amd-gfx@lists.freedesktop.org
> > Cc: intel-gfx@lists.freedesktop.org
> > Cc: Chris Wilson <chris@chris-wilson.co.uk>
> > Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> > Cc: Christian König <christian.koenig@amd.com>
> > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> > ---
> > DC-folks, I think this split out patch from my series here
> >
> > https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flore.kernel.org%2Fdri-devel%2F20200707201229.472834-1-daniel.vetter%40ffwll.ch%2F&data=02%7C01%7Cchristian.koenig%40amd.com%7C8a4f5736682a4b5c943e08d832747ab1%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637314823145521840&sdata=qd7Nrox62Lr%2FXWbJJFVskg9RYL4%2FoRVCFjR6rUDMA5E%3D&reserved=0
> >
> > should be ready for review/merging. I fixed it up a bit so that it's not
> > just a gross hack :-)
> >
> > Cheers, Daniel
> >
> >
> > ---
> > .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 19 ++++++-------------
> > 1 file changed, 6 insertions(+), 13 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> > index 21ec64fe5527..a20b62b1f2ef 100644
> > --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> > +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> > @@ -6959,20 +6959,13 @@ static void amdgpu_dm_commit_planes(struct drm_atomic_state *state,
> > DRM_ERROR("Waiting for fences timed out!");
> >
> > /*
> > - * TODO This might fail and hence better not used, wait
> > - * explicitly on fences instead
> > - * and in general should be called for
> > - * blocking commit to as per framework helpers
> > + * We cannot reserve buffers here, which means the normal flag
> > + * access functions don't work. Paper over this with READ_ONCE,
> > + * but maybe the flags are invariant enough that not even that
> > + * would be needed.
> > */
> > - r = amdgpu_bo_reserve(abo, true);
> > - if (unlikely(r != 0))
> > - DRM_ERROR("failed to reserve buffer before flip\n");
> > -
> > - amdgpu_bo_get_tiling_flags(abo, &tiling_flags);
> > -
> > - tmz_surface = amdgpu_bo_encrypted(abo);
> > -
> > - amdgpu_bo_unreserve(abo);
> > + tiling_flags = READ_ONCE(abo->tiling_flags);
> > + tmz_surface = READ_ONCE(abo->flags) & AMDGPU_GEM_CREATE_ENCRYPTED;
>
> Yeah, the abo->flags are mostly fixed after creation, especially the
> encrypted flag can't change or we corrupt page table tables. So that
> should work fine.
>
> Anybody who picks this up feel free to add an Reviewed-by: Christian
> König <christian.koenig@amd.com>.
>
> Regards,
> Christian.
>
> >
> > fill_dc_plane_info_and_addr(
> > dm->adev, new_plane_state, tiling_flags,
>
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [PATCH] drm/amdgpu/dc: Stop dma_resv_lock inversion in commit_tail
@ 2020-07-29 22:10 ` Alex Deucher
0 siblings, 0 replies; 16+ messages in thread
From: Alex Deucher @ 2020-07-29 22:10 UTC (permalink / raw)
To: Christian König
Cc: linux-rdma, Daniel Vetter, Intel Graphics Development,
amd-gfx list, Chris Wilson,
moderated list:DMA BUFFER SHARING FRAMEWORK, DRI Development,
Daniel Vetter, linux-media
Applied. Thanks!
Alex
On Tue, Jul 28, 2020 at 2:56 AM Christian König
<christian.koenig@amd.com> wrote:
>
> Am 27.07.20 um 23:30 schrieb Daniel Vetter:
> > Trying to grab dma_resv_lock while in commit_tail before we've done
> > all the code that leads to the eventual signalling of the vblank event
> > (which can be a dma_fence) is deadlock-y. Don't do that.
> >
> > Here the solution is easy because just grabbing locks to read
> > something races anyway. We don't need to bother, READ_ONCE is
> > equivalent. And avoids the locking issue.
> >
> > v2: Also take into account tmz_surface boolean, plus just delete the
> > old code.
> >
> > Cc: linux-media@vger.kernel.org
> > Cc: linaro-mm-sig@lists.linaro.org
> > Cc: linux-rdma@vger.kernel.org
> > Cc: amd-gfx@lists.freedesktop.org
> > Cc: intel-gfx@lists.freedesktop.org
> > Cc: Chris Wilson <chris@chris-wilson.co.uk>
> > Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> > Cc: Christian König <christian.koenig@amd.com>
> > Signed-off-by: Daniel Vetter <daniel.vetter@intel.com>
> > ---
> > DC-folks, I think this split out patch from my series here
> >
> > https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flore.kernel.org%2Fdri-devel%2F20200707201229.472834-1-daniel.vetter%40ffwll.ch%2F&data=02%7C01%7Cchristian.koenig%40amd.com%7C8a4f5736682a4b5c943e08d832747ab1%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637314823145521840&sdata=qd7Nrox62Lr%2FXWbJJFVskg9RYL4%2FoRVCFjR6rUDMA5E%3D&reserved=0
> >
> > should be ready for review/merging. I fixed it up a bit so that it's not
> > just a gross hack :-)
> >
> > Cheers, Daniel
> >
> >
> > ---
> > .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 19 ++++++-------------
> > 1 file changed, 6 insertions(+), 13 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> > index 21ec64fe5527..a20b62b1f2ef 100644
> > --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> > +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> > @@ -6959,20 +6959,13 @@ static void amdgpu_dm_commit_planes(struct drm_atomic_state *state,
> > DRM_ERROR("Waiting for fences timed out!");
> >
> > /*
> > - * TODO This might fail and hence better not used, wait
> > - * explicitly on fences instead
> > - * and in general should be called for
> > - * blocking commit to as per framework helpers
> > + * We cannot reserve buffers here, which means the normal flag
> > + * access functions don't work. Paper over this with READ_ONCE,
> > + * but maybe the flags are invariant enough that not even that
> > + * would be needed.
> > */
> > - r = amdgpu_bo_reserve(abo, true);
> > - if (unlikely(r != 0))
> > - DRM_ERROR("failed to reserve buffer before flip\n");
> > -
> > - amdgpu_bo_get_tiling_flags(abo, &tiling_flags);
> > -
> > - tmz_surface = amdgpu_bo_encrypted(abo);
> > -
> > - amdgpu_bo_unreserve(abo);
> > + tiling_flags = READ_ONCE(abo->tiling_flags);
> > + tmz_surface = READ_ONCE(abo->flags) & AMDGPU_GEM_CREATE_ENCRYPTED;
>
> Yeah, the abo->flags are mostly fixed after creation, especially the
> encrypted flag can't change or we corrupt page table tables. So that
> should work fine.
>
> Anybody who picks this up feel free to add an Reviewed-by: Christian
> König <christian.koenig@amd.com>.
>
> Regards,
> Christian.
>
> >
> > fill_dc_plane_info_and_addr(
> > dm->adev, new_plane_state, tiling_flags,
>
> _______________________________________________
> amd-gfx mailing list
> amd-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/amd-gfx
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 16+ messages in thread