Re: [PATCH v4 1/6] drm: allow DRM_MODE_PAGE_FLIP_ASYNC for atomic commits

[Sebastian Wick]

On Sat, Jul 1, 2023 at 4:09 AM André Almeida <andrealmeid@xxxxxxxxxx> wrote:
>
> From: Simon Ser <contact@xxxxxxxxxxx>
>
> If the driver supports it, allow user-space to supply the
> DRM_MODE_PAGE_FLIP_ASYNC flag to request an async page-flip.
> Set drm_crtc_state.async_flip accordingly.
>
> Document that drivers will reject atomic commits if an async
> flip isn't possible. This allows user-space to fall back to
> something else. For instance, Xorg falls back to a blit.
> Another option is to wait as close to the next vblank as
> possible before performing the page-flip to reduce latency.
>
> Signed-off-by: Simon Ser <contact@xxxxxxxxxxx>
> Reviewed-by: Alex Deucher <alexander.deucher@xxxxxxx>
> Co-developed-by: André Almeida <andrealmeid@xxxxxxxxxx>
> Signed-off-by: André Almeida <andrealmeid@xxxxxxxxxx>
> ---
> v4: no changes
> ---
>  drivers/gpu/drm/drm_atomic_uapi.c | 28 +++++++++++++++++++++++++---
>  include/uapi/drm/drm_mode.h       |  9 +++++++++
>  2 files changed, 34 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_atomic_uapi.c b/drivers/gpu/drm/drm_atomic_uapi.c
> index d867e7f9f2cd..dfd4cf7169df 100644
> --- a/drivers/gpu/drm/drm_atomic_uapi.c
> +++ b/drivers/gpu/drm/drm_atomic_uapi.c
> @@ -1286,6 +1286,18 @@ static void complete_signaling(struct drm_device *dev,
>         kfree(fence_state);
>  }
>
> +static void
> +set_async_flip(struct drm_atomic_state *state)
> +{
> +       struct drm_crtc *crtc;
> +       struct drm_crtc_state *crtc_state;
> +       int i;
> +
> +       for_each_new_crtc_in_state(state, crtc, crtc_state, i) {
> +               crtc_state->async_flip = true;
> +       }
> +}
> +
>  int drm_mode_atomic_ioctl(struct drm_device *dev,
>                           void *data, struct drm_file *file_priv)
>  {
> @@ -1326,9 +1338,16 @@ int drm_mode_atomic_ioctl(struct drm_device *dev,
>         }
>
>         if (arg->flags & DRM_MODE_PAGE_FLIP_ASYNC) {
> -               drm_dbg_atomic(dev,
> -                              "commit failed: invalid flag DRM_MODE_PAGE_FLIP_ASYNC\n");
> -               return -EINVAL;
> +               if (!dev->mode_config.async_page_flip) {
> +                       drm_dbg_atomic(dev,
> +                                      "commit failed: DRM_MODE_PAGE_FLIP_ASYNC not supported\n");
> +                       return -EINVAL;
> +               }
> +               if (dev->mode_config.atomic_async_page_flip_not_supported) {
> +                       drm_dbg_atomic(dev,
> +                                      "commit failed: DRM_MODE_PAGE_FLIP_ASYNC not supported with atomic\n");
> +                       return -EINVAL;
> +               }
>         }
>
>         /* can't test and expect an event at the same time. */
> @@ -1426,6 +1445,9 @@ int drm_mode_atomic_ioctl(struct drm_device *dev,
>         if (ret)
>                 goto out;
>
> +       if (arg->flags & DRM_MODE_PAGE_FLIP_ASYNC)
> +               set_async_flip(state);
> +
>         if (arg->flags & DRM_MODE_ATOMIC_TEST_ONLY) {
>                 ret = drm_atomic_check_only(state);
>         } else if (arg->flags & DRM_MODE_ATOMIC_NONBLOCK) {
> diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h
> index 46becedf5b2f..56342ba2c11a 100644
> --- a/include/uapi/drm/drm_mode.h
> +++ b/include/uapi/drm/drm_mode.h
> @@ -949,6 +949,15 @@ struct hdr_output_metadata {
>   * Request that the page-flip is performed as soon as possible, ie. with no
>   * delay due to waiting for vblank. This may cause tearing to be visible on
>   * the screen.
> + *
> + * When used with atomic uAPI, the driver will return an error if the hardware
> + * doesn't support performing an asynchronous page-flip for this update.
> + * User-space should handle this, e.g. by falling back to a regular page-flip.
> + *
> + * Note, some hardware might need to perform one last synchronous page-flip
> + * before being able to switch to asynchronous page-flips. As an exception,
> + * the driver will return success even though that first page-flip is not
> + * asynchronous.

What would happen if one commits another async KMS update before the
first page flip? Does one receive EAGAIN, does it amend the previous
commit? What happens to the timing feedback?

This seems really risky to include tbh. I would prefer if we would not
add such special cases for now.

>   */
>  #define DRM_MODE_PAGE_FLIP_ASYNC 0x02
>  #define DRM_MODE_PAGE_FLIP_TARGET_ABSOLUTE 0x4
> --
> 2.41.0
>