On 4/12/19 9:58 AM, Helen Koike wrote: > Add atomic_async_{check,update} hooks in drm_plane_helper_funcs. > These hooks are called when userspace requests asyncronous page flip in > the atomic api through the flag DRM_MODE_PAGE_FLIP_ASYNC. > > Update those hooks in the drivers that implement async functions, except > for amdgpu who handles the flag in the normal path, and rockchip who > doesn't support async page flip. > > Signed-off-by: Helen Koike <helen.ko...@collabora.com> > > --- > Hi, > > This patch is an attempt to expose the implementation that already exist > for true async page flips updates through atomic api when the > DRM_MODE_PAGE_FLIP_ASYNC is used. > > In this commit I'm re-introducing the atomic_async_{check,update} hooks. > I know it is a bit weird to remove them first and them re-add them, but > I did this in the last commit to avoid any state of inconsistency > between commits, as rockchip doesn't support async page flip and they were > being used as amend. > So I reverted to amend first and then re-introced async again > tied to the DRM_MODE_PAGE_FLIP_ASYNC flag (I also think this is easier > to read). > > To use async, it is required to have > dev->mode_config.async_page_flip = true; > but I see that only amdgpu and vc4 have this, so this patch won't take > effect on mdp5. > Nouveau and radeon also have this flag, but they don't implement the > async hooks yet. > > Please let me know what you think. > > Thanks > Helen > > Changes in v3: None > Changes in v2: None > Changes in v1: None > > .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 5 +++ > drivers/gpu/drm/drm_atomic_helper.c | 31 ++++++++++++---- > drivers/gpu/drm/drm_atomic_uapi.c | 3 +- > drivers/gpu/drm/msm/disp/mdp5/mdp5_plane.c | 2 + > drivers/gpu/drm/rockchip/rockchip_drm_vop.c | 4 ++ > drivers/gpu/drm/vc4/vc4_plane.c | 2 + > include/drm/drm_atomic.h | 2 + > include/drm/drm_atomic_helper.h | 9 +++-- > include/drm/drm_modeset_helper_vtables.h | 37 +++++++++++++++++++ > 9 files changed, 83 insertions(+), 12 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 711e7715e112..bb8a5f1997d6 100644 > --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c > +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c > @@ -3785,6 +3785,11 @@ static const struct drm_plane_helper_funcs > dm_plane_helper_funcs = { > */ > .atomic_amend_check = dm_plane_atomic_async_check, > .atomic_amend_update = dm_plane_atomic_async_update > + /* > + * Note: amdgpu takes care of DRM_MODE_PAGE_FLIP_ASYNC flag in the > + * normal commit path, thus .atomic_async_check and .atomic_async_update > + * are not provided here. > + */ > }; > > /* > diff --git a/drivers/gpu/drm/drm_atomic_helper.c > b/drivers/gpu/drm/drm_atomic_helper.c > index 9b0df08836c3..bfcf88359de5 100644 > --- a/drivers/gpu/drm/drm_atomic_helper.c > +++ b/drivers/gpu/drm/drm_atomic_helper.c > @@ -947,16 +947,31 @@ int drm_atomic_helper_check(struct drm_device *dev, > if (ret) > return ret; > > + /* > + * If async page flip was explicitly requested, but it is not possible, > + * return error instead of falling back to a normal commit. > + * If async_amend_check returns -EOPNOTSUPP, it means > + * ->atomic_async_update hook doesn't exist, so fallback to normal > + * commit and let the driver handle DRM_MODE_PAGE_FLIP_ASYNC flag > + * through normal commit code path. > + */ > + if (state->async_update) { > + ret = drm_atomic_helper_async_amend_check(dev, state, false); > + state->async_update = !ret; > + return !ret || ret == -EOPNOTSUPP ? 0 : ret; > + } > + > /* > * If amend was explicitly requested, but it is not possible, > * return error instead of falling back to a normal commit. > */ > if (state->amend_update) > - return drm_atomic_helper_amend_check(dev, state); > + return drm_atomic_helper_async_amend_check(dev, state, true); > > /* Legacy mode falls back to a normal commit if amend isn't possible. */ > if (state->legacy_cursor_update) > - state->amend_update = !drm_atomic_helper_amend_check(dev, > state); > + state->amend_update = > + !drm_atomic_helper_async_amend_check(dev, state, true); > > return 0; > } > @@ -1651,8 +1666,9 @@ static void commit_work(struct work_struct *work) > * if not. Note that error just mean it can't be committed in amend mode, if > it > * fails the commit should be treated like a normal commit. > */ > -int drm_atomic_helper_amend_check(struct drm_device *dev, > - struct drm_atomic_state *state) > +int drm_atomic_helper_async_amend_check(struct drm_device *dev, > + struct drm_atomic_state *state, > + bool amend) > { > struct drm_crtc *crtc; > struct drm_crtc_state *crtc_state; > @@ -1695,7 +1711,7 @@ int drm_atomic_helper_amend_check(struct drm_device > *dev, > return -EINVAL; >
Sorry, I forgot a modif here: - if (!funcs->atomic_amend_update) - return -EINVAL; + if ((amend && !funcs->atomic_amend_update) || + !funcs->atomic_async_update) + return -EOPNOTSUPP; I need to return -EOPNOTSUPP so I can know if async should fallback to the normal async path, this is required for amdgpu as it handles the PAGE_FLIP_ASYNC flag it self. I'll correct this in the next version after your comments. You can also see the series on https://gitlab.collabora.com/koike/linux/commits/drm/amend/uapi Thanks > /* Only allow amend update for cursor type planes. */ > - if (plane->type != DRM_PLANE_TYPE_CURSOR) > + if (amend && plane->type != DRM_PLANE_TYPE_CURSOR) > return -EINVAL; > > /* > @@ -1707,9 +1723,10 @@ int drm_atomic_helper_amend_check(struct drm_device > *dev, > !try_wait_for_completion(&old_plane_state->commit->hw_done)) > return -EBUSY; > > - return funcs->atomic_amend_check(plane, new_plane_state); > + return amend ? funcs->atomic_amend_check(plane, new_plane_state) : > + funcs->atomic_async_check(plane, new_plane_state); > } > -EXPORT_SYMBOL(drm_atomic_helper_amend_check); > +EXPORT_SYMBOL(drm_atomic_helper_async_amend_check); > > /** > * drm_atomic_helper_amend_commit - commit state in amend mode > diff --git a/drivers/gpu/drm/drm_atomic_uapi.c > b/drivers/gpu/drm/drm_atomic_uapi.c > index d1962cdea602..1d9a6142218e 100644 > --- a/drivers/gpu/drm/drm_atomic_uapi.c > +++ b/drivers/gpu/drm/drm_atomic_uapi.c > @@ -1312,8 +1312,9 @@ int drm_mode_atomic_ioctl(struct drm_device *dev, > drm_modeset_acquire_init(&ctx, DRM_MODESET_ACQUIRE_INTERRUPTIBLE); > state->acquire_ctx = &ctx; > state->allow_modeset = !!(arg->flags & DRM_MODE_ATOMIC_ALLOW_MODESET); > + state->async_update = !!(arg->flags & DRM_MODE_PAGE_FLIP_ASYNC); > /* async takes precedence over amend */ > - state->amend_update = arg->flags & DRM_MODE_PAGE_FLIP_ASYNC ? 0 : > + state->amend_update = state->async_update ? 0 : > !!(arg->flags & DRM_MODE_ATOMIC_AMEND); > > retry: > diff --git a/drivers/gpu/drm/msm/disp/mdp5/mdp5_plane.c > b/drivers/gpu/drm/msm/disp/mdp5/mdp5_plane.c > index 814e8230cec6..e3b2a2c74852 100644 > --- a/drivers/gpu/drm/msm/disp/mdp5/mdp5_plane.c > +++ b/drivers/gpu/drm/msm/disp/mdp5/mdp5_plane.c > @@ -531,6 +531,8 @@ static const struct drm_plane_helper_funcs > mdp5_plane_helper_funcs = { > .cleanup_fb = mdp5_plane_cleanup_fb, > .atomic_check = mdp5_plane_atomic_check, > .atomic_update = mdp5_plane_atomic_update, > + .atomic_async_check = mdp5_plane_atomic_async_check, > + .atomic_async_update = mdp5_plane_atomic_async_update, > /* > * FIXME: ideally, instead of hooking async updates to amend, > * we could avoid tearing by modifying the pending commit. > diff --git a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c > b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c > index 216ad76118dc..c2f7201e52a9 100644 > --- a/drivers/gpu/drm/rockchip/rockchip_drm_vop.c > +++ b/drivers/gpu/drm/rockchip/rockchip_drm_vop.c > @@ -954,6 +954,10 @@ static const struct drm_plane_helper_funcs > plane_helper_funcs = { > .atomic_disable = vop_plane_atomic_disable, > .atomic_amend_check = vop_plane_atomic_amend_check, > .atomic_amend_update = vop_plane_atomic_amend_update, > + /* > + * Note: rockchip doesn't support async page flip, thus > + * .atomic_async_update and .atomic_async_check are not provided. > + */ > .prepare_fb = drm_gem_fb_prepare_fb, > }; > > diff --git a/drivers/gpu/drm/vc4/vc4_plane.c b/drivers/gpu/drm/vc4/vc4_plane.c > index ea560000222d..24a9befe89e6 100644 > --- a/drivers/gpu/drm/vc4/vc4_plane.c > +++ b/drivers/gpu/drm/vc4/vc4_plane.c > @@ -1175,6 +1175,8 @@ static const struct drm_plane_helper_funcs > vc4_plane_helper_funcs = { > */ > .atomic_amend_check = vc4_plane_atomic_async_check, > .atomic_amend_update = vc4_plane_atomic_async_update, > + .atomic_async_check = vc4_plane_atomic_async_check, > + .atomic_async_update = vc4_plane_atomic_async_update, > }; > > static void vc4_plane_destroy(struct drm_plane *plane) > diff --git a/include/drm/drm_atomic.h b/include/drm/drm_atomic.h > index b1ced069ffc1..05756a09e762 100644 > --- a/include/drm/drm_atomic.h > +++ b/include/drm/drm_atomic.h > @@ -300,6 +300,7 @@ struct __drm_private_objs_state { > * @ref: count of all references to this state (will not be freed until zero) > * @dev: parent DRM device > * @legacy_cursor_update: hint to enforce legacy cursor IOCTL semantics > + * @async_update: hint for asyncronous page flip update > * @amend_update: hint for amend plane update > * @planes: pointer to array of structures with per-plane data > * @crtcs: pointer to array of CRTC pointers > @@ -328,6 +329,7 @@ struct drm_atomic_state { > */ > bool allow_modeset : 1; > bool legacy_cursor_update : 1; > + bool async_update : 1; > bool amend_update : 1; > /** > * @duplicated: > diff --git a/include/drm/drm_atomic_helper.h b/include/drm/drm_atomic_helper.h > index 8ce0594ccfb9..39e57d559a30 100644 > --- a/include/drm/drm_atomic_helper.h > +++ b/include/drm/drm_atomic_helper.h > @@ -55,10 +55,11 @@ void drm_atomic_helper_commit_tail_rpm(struct > drm_atomic_state *state); > int drm_atomic_helper_commit(struct drm_device *dev, > struct drm_atomic_state *state, > bool nonblock); > -int drm_atomic_helper_amend_check(struct drm_device *dev, > - struct drm_atomic_state *state); > -void drm_atomic_helper_amend_commit(struct drm_device *dev, > - struct drm_atomic_state *state); > +int drm_atomic_helper_async_amend_check(struct drm_device *dev, > + struct drm_atomic_state *state, > + bool amend); > +void drm_atomic_helper_async_amend_commit(struct drm_device *dev, > + struct drm_atomic_state *state); > > int drm_atomic_helper_wait_for_fences(struct drm_device *dev, > struct drm_atomic_state *state, > diff --git a/include/drm/drm_modeset_helper_vtables.h > b/include/drm/drm_modeset_helper_vtables.h > index d92e62dd76c4..c2863d62f160 100644 > --- a/include/drm/drm_modeset_helper_vtables.h > +++ b/include/drm/drm_modeset_helper_vtables.h > @@ -1135,6 +1135,43 @@ struct drm_plane_helper_funcs { > void (*atomic_disable)(struct drm_plane *plane, > struct drm_plane_state *old_state); > > + /** > + * @atomic_async_check: > + * > + * Drivers should set this function pointer to check if a page flip can > + * be performed asynchronously, i.e., immediately without waiting for a > + * vblank. > + * > + * This hook is called by drm_atomic_async_check() to establish if a > + * given update can be committed in async mode, that is, if it can > + * jump ahead of the state currently queued for update. > + * > + * RETURNS: > + * > + * Return 0 on success and any error returned indicates that the update > + * can not be applied in asynd mode. > + */ > + int (*atomic_async_check)(struct drm_plane *plane, > + struct drm_plane_state *state); > + > + /** > + * @atomic_async_update: > + * > + * Drivers should set this function pointer to perform asynchronous page > + * flips through the atomic api, i.e. not vblank synchronized. > + * > + * Note that unlike &drm_plane_helper_funcs.atomic_update this hook > + * takes the new &drm_plane_state as parameter. When doing async_update > + * drivers shouldn't replace the &drm_plane_state but update the > + * current one with the new plane configurations in the new > + * plane_state. > + * > + * FIXME: > + * - It only works for single plane updates > + */ > + void (*atomic_async_update)(struct drm_plane *plane, > + struct drm_plane_state *new_state); > + > /** > * @atomic_amend_check: > * > _______________________________________________ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx