| Message ID | 20230621202459.979661-2-andrealmeid@igalia.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | amd/display: only accept async flips for fast updates | expand |
On 2023-06-21 16:24, André Almeida wrote: > From: Simon Ser <contact@emersion.fr> > > Up until now, amdgpu was silently degrading to vsync when > user-space requested an async flip but the hardware didn't support > it. > > The hardware doesn't support immediate flips when the update changes > the FB pitch, the DCC state, the rotation, enables or disables CRTCs > or planes, etc. This is reflected in the dm_crtc_state.update_type > field: UPDATE_TYPE_FAST means that immediate flip is supported. > > Silently degrading async flips to vsync is not the expected behavior > from a uAPI point-of-view. Xorg expects async flips to fail if > unsupported, to be able to fall back to a blit. i915 already behaves > this way. > > This patch aligns amdgpu with uAPI expectations and returns a failure > when an async flip is not possible. > > Signed-off-by: Simon Ser <contact@emersion.fr> > Reviewed-by: André Almeida <andrealmeid@igalia.com> > Reviewed-by: Alex Deucher <alexander.deucher@amd.com> > Signed-off-by: André Almeida <andrealmeid@igalia.com> Reviewed-by: Harry Wentland <harry.wentland@amd.com> Harry > --- > drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 8 ++++++++ > .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_crtc.c | 12 ++++++++++++ > 2 files changed, 20 insertions(+) > > 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 514f6785a020..1d9b84e5835f 100644 > --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c > +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c > @@ -8136,7 +8136,15 @@ static void amdgpu_dm_commit_planes(struct drm_atomic_state *state, > * Only allow immediate flips for fast updates that don't > * change memory domain, FB pitch, DCC state, rotation or > * mirroring. > + * > + * dm_crtc_helper_atomic_check() only accepts async flips with > + * fast updates. > */ > + if (crtc->state->async_flip && > + acrtc_state->update_type != UPDATE_TYPE_FAST) > + 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 && > 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 440fc0869a34..30d4c6fd95f5 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,6 +398,18 @@ 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;
On 6/21/23 16:24, André Almeida wrote: > From: Simon Ser <contact@emersion.fr> > > Up until now, amdgpu was silently degrading to vsync when > user-space requested an async flip but the hardware didn't support > it. > > The hardware doesn't support immediate flips when the update changes > the FB pitch, the DCC state, the rotation, enables or disables CRTCs > or planes, etc. This is reflected in the dm_crtc_state.update_type > field: UPDATE_TYPE_FAST means that immediate flip is supported. > > Silently degrading async flips to vsync is not the expected behavior > from a uAPI point-of-view. Xorg expects async flips to fail if > unsupported, to be able to fall back to a blit. i915 already behaves > this way. > > This patch aligns amdgpu with uAPI expectations and returns a failure > when an async flip is not possible. > > Signed-off-by: Simon Ser <contact@emersion.fr> > Reviewed-by: André Almeida <andrealmeid@igalia.com> > Reviewed-by: Alex Deucher <alexander.deucher@amd.com> > Signed-off-by: André Almeida <andrealmeid@igalia.com> Applied, thanks! > --- > drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 8 ++++++++ > .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_crtc.c | 12 ++++++++++++ > 2 files changed, 20 insertions(+) > > 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 514f6785a020..1d9b84e5835f 100644 > --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c > +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c > @@ -8136,7 +8136,15 @@ static void amdgpu_dm_commit_planes(struct drm_atomic_state *state, > * Only allow immediate flips for fast updates that don't > * change memory domain, FB pitch, DCC state, rotation or > * mirroring. > + * > + * dm_crtc_helper_atomic_check() only accepts async flips with > + * fast updates. > */ > + if (crtc->state->async_flip && > + acrtc_state->update_type != UPDATE_TYPE_FAST) > + 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 && > 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 440fc0869a34..30d4c6fd95f5 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,6 +398,18 @@ 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 514f6785a020..1d9b84e5835f 100644 --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c @@ -8136,7 +8136,15 @@ static void amdgpu_dm_commit_planes(struct drm_atomic_state *state, * Only allow immediate flips for fast updates that don't * change memory domain, FB pitch, DCC state, rotation or * mirroring. + * + * dm_crtc_helper_atomic_check() only accepts async flips with + * fast updates. */ + if (crtc->state->async_flip && + acrtc_state->update_type != UPDATE_TYPE_FAST) + 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 && 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 440fc0869a34..30d4c6fd95f5 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,6 +398,18 @@ 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;