On 2023-08-04 14:20, Hamza Mahfooz wrote:
> We should be checking to see if async flips are supported in
> amdgpu_dm_atomic_check() (i.e. not dm_crtc_helper_atomic_check()). Also,
> async flipping isn't supported if a plane's framebuffer changes memory
> domains during an atomic commit. So, move the check from
> dm_crtc_helper_atomic_check() to amdgpu_dm_atomic_check() and check if
> the memory domain has changed in amdgpu_dm_atomic_check().
>
> Cc: sta...@vger.kernel.org
> Link: https://gitlab.freedesktop.org/drm/amd/-/issues/2733
> Fixes: 3f86b60691e6 ("drm/amd/display: only accept async flips for fast
> updates")
> Signed-off-by: Hamza Mahfooz <hamza.mahf...@amd.com>
Reviewed-by: Harry Wentland <harry.wentl...@amd.com>
Harry
> ---
> v2: link issue and revert back to the old way of setting update_type.
> ---
> .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 24 ++++++++++++++++---
> .../amd/display/amdgpu_dm/amdgpu_dm_crtc.c | 12 ----------
> 2 files changed, 21 insertions(+), 15 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 32fb551862b0..1d3afab5bc85 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> @@ -8086,10 +8086,12 @@ static void amdgpu_dm_commit_planes(struct
> drm_atomic_state *state,
> * fast updates.
> */
> if (crtc->state->async_flip &&
> - acrtc_state->update_type != UPDATE_TYPE_FAST)
> + (acrtc_state->update_type != UPDATE_TYPE_FAST ||
> + get_mem_type(old_plane_state->fb) != get_mem_type(fb)))
> drm_warn_once(state->dev,
> "[PLANE:%d:%s] async flip with non-fast
> update\n",
> plane->base.id, plane->name);
> +
> bundle->flip_addrs[planes_count].flip_immediate =
> crtc->state->async_flip &&
> acrtc_state->update_type == UPDATE_TYPE_FAST &&
> @@ -10050,6 +10052,11 @@ static int amdgpu_dm_atomic_check(struct drm_device
> *dev,
>
> /* Remove exiting planes if they are modified */
> for_each_oldnew_plane_in_state_reverse(state, plane, old_plane_state,
> new_plane_state, i) {
> + if (old_plane_state->fb && new_plane_state->fb &&
> + get_mem_type(old_plane_state->fb) !=
> + get_mem_type(new_plane_state->fb))
> + lock_and_validation_needed = true;
> +
> ret = dm_update_plane_state(dc, state, plane,
> old_plane_state,
> new_plane_state,
> @@ -10297,9 +10304,20 @@ static int amdgpu_dm_atomic_check(struct drm_device
> *dev,
> struct dm_crtc_state *dm_new_crtc_state =
> to_dm_crtc_state(new_crtc_state);
>
> + /*
> + * Only allow async flips for fast updates that don't change
> + * the FB pitch, the DCC state, rotation, etc.
> + */
> + if (new_crtc_state->async_flip && lock_and_validation_needed) {
> + drm_dbg_atomic(crtc->dev,
> + "[CRTC:%d:%s] async flips are only
> supported for fast updates\n",
> + crtc->base.id, crtc->name);
> + ret = -EINVAL;
> + goto fail;
> + }
> +
> dm_new_crtc_state->update_type = lock_and_validation_needed ?
> - UPDATE_TYPE_FULL :
> - UPDATE_TYPE_FAST;
> + UPDATE_TYPE_FULL : UPDATE_TYPE_FAST;
> }
>
> /* Must be success */
> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_crtc.c
> b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_crtc.c
> index 30d4c6fd95f5..440fc0869a34 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_crtc.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_crtc.c
> @@ -398,18 +398,6 @@ static int dm_crtc_helper_atomic_check(struct drm_crtc
> *crtc,
> return -EINVAL;
> }
>
> - /*
> - * Only allow async flips for fast updates that don't change the FB
> - * pitch, the DCC state, rotation, etc.
> - */
> - if (crtc_state->async_flip &&
> - dm_crtc_state->update_type != UPDATE_TYPE_FAST) {
> - drm_dbg_atomic(crtc->dev,
> - "[CRTC:%d:%s] async flips are only supported for
> fast updates\n",
> - crtc->base.id, crtc->name);
> - return -EINVAL;
> - }
> -
> /* In some use cases, like reset, no stream is attached */
> if (!dm_crtc_state->stream)
> return 0;
