diff mbox series

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

Message ID 20230701020917.143394-2-andrealmeid@igalia.com (mailing list archive)
State New, archived
Headers show
Series drm: Add support for atomic async page-flip | expand

Commit Message

André Almeida July 1, 2023, 2:09 a.m. UTC 
From: Simon Ser <contact@emersion.fr>

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@emersion.fr>
Reviewed-by: Alex Deucher <alexander.deucher@amd.com>
Co-developed-by: André Almeida <andrealmeid@igalia.com>
Signed-off-by: André Almeida <andrealmeid@igalia.com>
---
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(-)

Comments

Sebastian Wick July 4, 2023, 5:06 p.m. UTC | #1 
On Sat, Jul 1, 2023 at 4:09 AM André Almeida <andrealmeid@igalia.com> wrote:
>
> From: Simon Ser <contact@emersion.fr>
>
> 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@emersion.fr>
> Reviewed-by: Alex Deucher <alexander.deucher@amd.com>
> Co-developed-by: André Almeida <andrealmeid@igalia.com>
> Signed-off-by: André Almeida <andrealmeid@igalia.com>
> ---
> 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
>
Simon Ser July 4, 2023, 5:57 p.m. UTC | #2 
On Tuesday, July 4th, 2023 at 19:06, Sebastian Wick <sebastian.wick@redhat.com> wrote:

> > + * 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.

This is not a new case, i915 already does this with the legacy API to
address some hw issues. Sadly I don't think we can do anything about
it.
Michel Dänzer July 5, 2023, 10 a.m. UTC | #3 
On 7/4/23 19:06, Sebastian Wick wrote:
> On Sat, Jul 1, 2023 at 4:09 AM André Almeida <andrealmeid@igalia.com> wrote:
>>
>> @@ -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?

Should be the former. DRM_MODE_PAGE_FLIP_ASYNC just means the flip may complete outside of vertical blank, it doesn't change anything else.
diff mbox series

Patch

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.
  */
 #define DRM_MODE_PAGE_FLIP_ASYNC 0x02
 #define DRM_MODE_PAGE_FLIP_TARGET_ABSOLUTE 0x4