* [PATCH] drm/amd/display: ensure async flips are only accepted for fast updates
@ 2023-08-04 15:56 Hamza Mahfooz
2023-08-04 17:29 ` Harry Wentland
0 siblings, 1 reply; 2+ messages in thread
From: Hamza Mahfooz @ 2023-08-04 15:56 UTC (permalink / raw
To: amd-gfx
Cc: Alan Liu, Marcus Seyfarth, Hamza Mahfooz, André Almeida,
Rodrigo Siqueira, Aurabindo Pillai, Stylon Wang,
Srinivasan Shanmugam, Leo Li, Melissa Wen, Hersen Wu, dri-devel,
David Tadokoro, Qingqing Zhuo, Pan, Xinhui, linux-kernel, stable,
Wayne Lin, Alex Deucher, Christian König
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
Fixes: 3f86b60691e6 ("drm/amd/display: only accept async flips for fast updates")
Tested-by: Marcus Seyfarth <m.seyfarth@gmail.com>
Signed-off-by: Hamza Mahfooz <hamza.mahfooz@amd.com>
---
.../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 25 ++++++++++++++++---
.../amd/display/amdgpu_dm/amdgpu_dm_crtc.c | 12 ---------
2 files changed, 21 insertions(+), 16 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..e561d99b3f40 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -8086,7 +8086,8 @@ 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);
@@ -10050,12 +10051,18 @@ 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,
false,
&lock_and_validation_needed,
&is_top_most_overlay);
+
if (ret) {
DRM_DEBUG_DRIVER("dm_update_plane_state() failed\n");
goto fail;
@@ -10069,6 +10076,7 @@ static int amdgpu_dm_atomic_check(struct drm_device *dev,
new_crtc_state,
false,
&lock_and_validation_needed);
+
if (ret) {
DRM_DEBUG_DRIVER("DISABLE: dm_update_crtc_state() failed\n");
goto fail;
@@ -10297,9 +10305,18 @@ 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);
- dm_new_crtc_state->update_type = lock_and_validation_needed ?
- UPDATE_TYPE_FULL :
- UPDATE_TYPE_FAST;
+ /*
+ * 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;
+ } else
+ dm_new_crtc_state->update_type = 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 related [flat|nested] 2+ messages in thread
* Re: [PATCH] drm/amd/display: ensure async flips are only accepted for fast updates
2023-08-04 15:56 [PATCH] drm/amd/display: ensure async flips are only accepted for fast updates Hamza Mahfooz
@ 2023-08-04 17:29 ` Harry Wentland
0 siblings, 0 replies; 2+ messages in thread
From: Harry Wentland @ 2023-08-04 17:29 UTC (permalink / raw
To: Hamza Mahfooz, amd-gfx
Cc: Alan Liu, dri-devel, André Almeida, Rodrigo Siqueira,
Aurabindo Pillai, Stylon Wang, Srinivasan Shanmugam, Leo Li,
Melissa Wen, Hersen Wu, Marcus Seyfarth, David Tadokoro,
Qingqing Zhuo, Pan, Xinhui, linux-kernel, stable, Wayne Lin,
Alex Deucher, Christian König
On 2023-08-04 11:56, 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
> Fixes: 3f86b60691e6 ("drm/amd/display: only accept async flips for fast updates")
> Tested-by: Marcus Seyfarth <m.seyfarth@gmail.com>
> Signed-off-by: Hamza Mahfooz <hamza.mahfooz@amd.com>
> ---
> .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 25 ++++++++++++++++---
> .../amd/display/amdgpu_dm/amdgpu_dm_crtc.c | 12 ---------
> 2 files changed, 21 insertions(+), 16 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..e561d99b3f40 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> @@ -8086,7 +8086,8 @@ 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);
> @@ -10050,12 +10051,18 @@ 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,
> false,
> &lock_and_validation_needed,
> &is_top_most_overlay);
> +
nit: extraneous newline
> if (ret) {
> DRM_DEBUG_DRIVER("dm_update_plane_state() failed\n");
> goto fail;
> @@ -10069,6 +10076,7 @@ static int amdgpu_dm_atomic_check(struct drm_device *dev,
> new_crtc_state,
> false,
> &lock_and_validation_needed);
> +
nit: extraneous newline
> if (ret) {
> DRM_DEBUG_DRIVER("DISABLE: dm_update_crtc_state() failed\n");
> goto fail;
> @@ -10297,9 +10305,18 @@ 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);
>
> - dm_new_crtc_state->update_type = lock_and_validation_needed ?
> - UPDATE_TYPE_FULL :
> - UPDATE_TYPE_FAST;
> + /*
> + * 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;
> + } else
nit: Add braces here as well
> + dm_new_crtc_state->update_type = UPDATE_TYPE_FAST;
If async_flip is false you'll be setting update_type to FAST here
uncoditionally.
You'll still need the lock_and_validation check here, i.e. this:
> dm_new_crtc_state->update_type = lock_and_validation_needed ?
> UPDATE_TYPE_FULL :
> UPDATE_TYPE_FAST;
Harry
> }
>
> /* 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 17:29 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-08-04 15:56 [PATCH] drm/amd/display: ensure async flips are only accepted for fast updates Hamza Mahfooz
2023-08-04 17:29 ` Harry Wentland
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).