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

2023-07-05 Thread Michel Dänzer
On 7/4/23 19:06, Sebastian Wick wrote:
> On Sat, Jul 1, 2023 at 4:09 AM André Almeida  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.


-- 
Earthling Michel Dänzer|  https://redhat.com
Libre software enthusiast  | Mesa and Xwayland developer



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

2023-07-04 Thread Simon Ser
On Tuesday, July 4th, 2023 at 19:06, Sebastian Wick  
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.


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

2023-07-04 Thread Sebastian Wick
On Sat, Jul 1, 2023 at 4:09 AM André Almeida  wrote:
>
> From: Simon Ser 
>
> 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 
> Reviewed-by: Alex Deucher 
> Co-developed-by: André Almeida 
> Signed-off-by: André Almeida 
> ---
> 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
>