| Message ID | 20230804155613.117310-1-hamza.mahfooz@amd.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/amd/display: ensure async flips are only accepted for fast updates | expand |
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;
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;
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(-)