Re: [PATCH RFC v3 4/4] drm/atomic: hook atomic_async_{check,update} with PAGE_FLIP_ASYNC flag
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
>
> ---
> 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,
> +
[PATCH RFC v3 4/4] drm/atomic: hook atomic_async_{check, update} with PAGE_FLIP_ASYNC flag
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
---
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 @@
