Re: [PATCH] drm/amd/display: assign fb_location only if bo is pinned

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





On 10/24/2017 11:43 AM, Michel Dänzer wrote:
[ Adding the dri-devel list ]

On 24/10/17 04:58 PM, Andrey Grodzovsky wrote:
On 10/24/2017 10:36 AM, S, Shirish wrote:
On 10/24/2017 7:48 PM, Andrey Grodzovsky wrote:
On 10/24/2017 09:51 AM, S, Shirish wrote:
From: Shirish S <shirish.s@xxxxxxx>

On some systems amdgpu_bo_gpu_offset seems to be
called before the BO is pinned, leading to page faults
as the GPU address is still subject to change.

This patch fixes the above issue.

Signed-off-by: Shirish S <shirish.s@xxxxxxx>
---
   drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 7 +++++--
   1 file changed, 5 insertions(+), 2 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 974ee97..b251b7e 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -1786,8 +1786,11 @@ static int get_fb_info(const struct
amdgpu_framebuffer *amdgpu_fb,
           return r;
       }
   -    if (fb_location)
-        *fb_location = amdgpu_bo_gpu_offset(rbo);
+    r = amdgpu_bo_pin(rbo, AMDGPU_GEM_DOMAIN_VRAM, fb_location);
+    if (unlikely(r != 0)) {
+        amdgpu_bo_unreserve(rbo);
+        return -EINVAL;
+    }
Where the unpin would happen then ? Today DAL calls pin from
dm_plane_helper_prepare_fb while the unpin from
dm_plane_helper_cleanup_fb.

Good point, am trying to address the issue Michel is facing, if this
patch works, we can think of unpinning the bo appropriately.
Ideally it should be at the end of amdgpu_dm_atomic_check(). Can you
think of any better place?
Sounds right, you have to unpin within atomic_check since UMD might call
check without commit sometimes.
Not sure it's appropriate to pin BOs from atomic_check.

In general, BOs should be pinned just before the display hardware starts
accessing them, and unpinned just after it stops accessing them.

Not sure what other option (unless moving fill_plane_attributes_from_fb to commit) since new fb parameters are needed in check to build the new plane state. Before we did upstream refactoring get_fb_info was always called called from atomic_commit so it was within pin/unpin scope. Moving it out into check we missed the problem Shirish just found. We can't do pin in beginning of check and unpin in the end of commit because as i said, sometimes check is called w\o commit, and as much as I remember we can't do it in caller of check/commit sequence without changing DRM code.

Thanks,
Andrey




_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel




[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux