* [PATCH v2] drm/amd/display: ensure async flips are only accepted for fast updates
@ 2023-08-04 18:20 Hamza Mahfooz
2023-08-04 18:24 ` Harry Wentland
0 siblings, 1 reply; 2+ messages in thread
From: Hamza Mahfooz @ 2023-08-04 18:20 UTC (permalink / raw)
To: amd-gfx
Cc: Hamza Mahfooz, stable, Harry Wentland, Leo Li, Rodrigo Siqueira,
Alex Deucher, Christian König, Pan, Xinhui, David Airlie,
Daniel Vetter, Qingqing Zhuo, Aurabindo Pillai, Hersen Wu,
Srinivasan Shanmugam, Stylon Wang, Wayne Lin, Alan Liu,
Melissa Wen, Simon Ser, David Tadokoro, André Almeida,
dri-devel, linux-kernel
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: stable@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.mahfooz@amd.com>
---
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;
--
2.41.0
^ permalink raw reply [flat|nested] 2+ messages in thread* Re: [PATCH v2] drm/amd/display: ensure async flips are only accepted for fast updates
2023-08-04 18:20 [PATCH v2] drm/amd/display: ensure async flips are only accepted for fast updates Hamza Mahfooz
@ 2023-08-04 18:24 ` Harry Wentland
0 siblings, 0 replies; 2+ messages in thread
From: Harry Wentland @ 2023-08-04 18:24 UTC (permalink / raw)
To: Hamza Mahfooz, amd-gfx
Cc: stable, Leo Li, Rodrigo Siqueira, Alex Deucher,
Christian König, Pan, Xinhui, David Airlie, Daniel Vetter,
Qingqing Zhuo, Aurabindo Pillai, Hersen Wu, Srinivasan Shanmugam,
Stylon Wang, Wayne Lin, Alan Liu, Melissa Wen, Simon Ser,
David Tadokoro, André Almeida, dri-devel, linux-kernel
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: stable@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.mahfooz@amd.com>
Reviewed-by: Harry Wentland <harry.wentland@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;
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2023-08-04 18:24 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-08-04 18:20 [PATCH v2] drm/amd/display: ensure async flips are only accepted for fast updates Hamza Mahfooz
2023-08-04 18:24 ` Harry Wentland
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®