On 4/2/19 10:41 AM, Kazlauskas, Nicholas wrote: > On 4/2/19 10:28 AM, Christian König wrote: >> Don't block others while waiting for the fences to finish, concurrent >> submission is perfectly valid in this case and holding the lock can >> prevent killed applications from terminating. >> >> Signed-off-by: Christian König <christian.koenig@xxxxxxx> > > Reviewed-by: Nicholas Kazlauskas <nicholas.kazlauskas@xxxxxxx> > > I do recall having the discussion about this a while back with you and > Andrey. > > I highly doubt that the flags would ever really change here (if they're > even capable) between the wait and the lock so I think this is fine. > > I think there's a bigger problem with that and how they could > potentially change between atomic check and atomic commit tail anyway > (and doing something bad after we think it's passed validation). > > But this is mostly a theoretical concern I think rather than a practical > one. > > Thanks. > > Nicholas Kazlauskas ...and waiting for the fence isn't really analogous to ensuring that they can't change in the first place I suppose. This looks fine. Nicholas Kazlauskas > > >> --- >> drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 13 ++++++++----- >> 1 file changed, 8 insertions(+), 5 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 697af51d403a..9426e701a7b6 100644 >> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c >> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c >> @@ -5129,23 +5129,26 @@ static void amdgpu_dm_commit_planes(struct drm_atomic_state *state, >> continue; >> } >> >> + abo = gem_to_amdgpu_bo(fb->obj[0]); >> + >> + /* Wait for all fences on this FB */ >> + r = reservation_object_wait_timeout_rcu(abo->tbo.resv, true, >> + false, >> + MAX_SCHEDULE_TIMEOUT); >> + WARN_ON(r < 0); >> + >> /* >> * 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 >> */ >> - abo = gem_to_amdgpu_bo(fb->obj[0]); >> r = amdgpu_bo_reserve(abo, true); >> if (unlikely(r != 0)) { >> DRM_ERROR("failed to reserve buffer before flip\n"); >> WARN_ON(1); >> } >> >> - /* Wait for all fences on this FB */ >> - WARN_ON(reservation_object_wait_timeout_rcu(abo->tbo.resv, true, false, >> - MAX_SCHEDULE_TIMEOUT) < 0); >> - >> amdgpu_bo_get_tiling_flags(abo, &tiling_flags); >> >> amdgpu_bo_unreserve(abo); >> > > _______________________________________________ > amd-gfx mailing list > amd-gfx@xxxxxxxxxxxxxxxxxxxxx > https://lists.freedesktop.org/mailman/listinfo/amd-gfx > _______________________________________________ amd-gfx mailing list amd-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/amd-gfx