Re: [PATCH 13/18] drm/amdgpu/dc: Stop dma_resv_lock inversion in commit_tail

Pierre-Eric Pelloux-Prayer
2020-06-05T08:30:25Z

Hi Daniel,

On 04/06/2020 10:12, Daniel Vetter wrote:
[...]
> @@ -6910,7 +6910,11 @@ static void amdgpu_dm_commit_planes(struct drm_atomic_state *state,
> * explicitly on fences instead
> * and in general should be called for
> * blocking commit to as per framework helpers
> + *
> + * Yes, this deadlocks, since you're calling dma_resv_lock in a
> + * path that leads to a dma_fence_signal(). Don't do that.
> */
> +#if 0
> r = amdgpu_bo_reserve(abo, true);
> if (unlikely(r != 0))
> DRM_ERROR("failed to reserve buffer before flip\n");
> @@ -6920,6 +6924,12 @@ static void amdgpu_dm_commit_planes(struct drm_atomic_state *state,
> tmz_surface = amdgpu_bo_encrypted(abo);
>
> amdgpu_bo_unreserve(abo);
> +#endif
> + /*
> + * this races anyway, so READ_ONCE isn't any better or worse
> + * than the stuff above. Except the stuff above can deadlock.
> + */
> + tiling_flags = READ_ONCE(abo->tiling_flags);

With this change "tmz_surface" won't be initialized properly.
Adding the following line should fix it:

tmz_surface = READ_ONCE(abo->flags) & AMDGPU_GEM_CREATE_ENCRYPTED;

Pierre-Eric

>
> fill_dc_plane_info_and_addr(
> dm->adev, new_plane_state, tiling_flags,
>