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

[Pierre-Eric Pelloux-Prayer <pierre-eric.pelloux-prayer@amd.com>]

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,
>