On 2021-11-16 15:10, Alex Deucher wrote:
On Tue, Nov 16, 2021 at 3:09 AM Christian König ckoenig.leichtzumerken@gmail.com wrote:
Am 16.11.21 um 09:00 schrieb Lang Yu:
On Tue, Nov 16, 2021 at 08:14:08AM +0100, Christian KKKnig wrote:
Am 16.11.21 um 04:27 schrieb Lang Yu:
On Mon, Nov 15, 2021 at 01:04:15PM +0100, Michel DDDnzer wrote:
[SNIP] > Though a single call to dce_v*_0_crtc_do_set_base() will > only pin the BO, I found it will be unpinned in next call to > dce_v*_0_crtc_do_set_base(). Yeah, that's the normal case when the new BO is different from the old one.
To catch the case I described, try something like
diff --git a/drivers/gpu/drm/amd/amdgpu/dce_v11_0.c b/drivers/gpu/drm/amd/amdgpu/dce_v11_0.c
index 18a7b3bd633b..5726bd87a355 100644
--- a/drivers/gpu/drm/amd/amdgpu/dce_v11_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/dce_v11_0.c
@@ -1926,6 +1926,7 @@ static int dce_v11_0_crtc_do_set_base(struct drm_crtc *crtc,
return r; if (!atomic) {
WARN_ON_ONCE(target_fb == fb); r = amdgpu_bo_pin(abo, AMDGPU_GEM_DOMAIN_VRAM); if (unlikely(r != 0)) { amdgpu_bo_unreserve(abo);I did some tests, the warning can be triggered.
pin/unpin operations in *_crtc_do_set_base() and amdgpu_display_crtc_page_flip_target() are mixed.
Ok sounds like we narrowed down the root cause pretty well.
Question is now how can we fix this? Just not pin the BO when target_fb == fb?
That worked. I did a few simple tests and didn't observe ttm_bo_release warnings any more.
The pin/unpin logic,
1, fist crtc_mode_set, dce_v*_0_crtc_do_set_base() pins crtc->primary->fb(new), old_fb is NULL.
2, second crtc_mode_set, dce_v*_0_crtc_do_set_base() pins crtc->primary->fb(new), unpins old fb.
3, amdgpu_display_crtc_page_flip_target() pin/unpin operations.
4, third crtc_mode_set, dce_v*_0_crtc_do_set_base() pins crtc->primary->fb(new), unpins old fb (it is pinned in last call to amdgpu_display_crtc_page_flip_target)
5, amdgpu_display_crtc_page_flip_target() pin/unpin operations.
.....
x, reboot, amdgpu_display_suspend_helper() is called, the last pinned fb was unpinned.
And I didn't observe amdgpu_bo_unpin() in dce_v*_0_crtc_disable() is called.
If the logic is wrong, please correct me.
I can't fully judge because I'm not that deep with my head in the old display code, but from a ten mile high view it sounds sane to me. Michel what do you think?
Sounds good to me.
Would be nice to address the other issue identified in this thread as well:
The pin in amdgpu_display_crtc_page_flip_target is guarded by if (!adev->enable_virtual_display), but the corresponding unpins in amdgpu_display_unpin_work_func & dce_v*_0_crtc_disable aren't. This probably results in pin count underflows with virtual display.