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
>



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 6/6] drm/doc: Define KMS atomic state set

2023-07-04 Thread James Braden olin
Simulator cannot be changed

To join the meeting on Google Meet, click this link:
https://meet.google.com/ccm-jjwz-ehk

Or open Meet and enter this code: ccm-jjwz-ehk

On Mon, Jul 3, 2023, 11:33 AM André Almeida  wrote:

>
>
> Em 03/07/2023 05:38, Pekka Paalanen escreveu:
> > On Fri, 30 Jun 2023 23:09:17 -0300
> > André Almeida  wrote:
> >
> >> Specify how the atomic state is maintained between userspace and
> >> kernel, plus the special case for async flips.
> >>
> >> Signed-off-by: André Almeida 
>
> [...]
>
> >
> > If you want to take these and need my Sob, that would be
> > Signed-off-by: Pekka Paalanen 
> >
> >
>
> Thank you very much! Your version is way better than mine, I'll use it
> in my next version.
>
> > Thanks,
> > pq
>