Re: [PATCH 3/4] drm: introduce DRM_CAP_ATOMIC_ASYNC_PAGE_FLIP

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

On Wed, Aug 24, 2022 at 03:08:55PM +0000, Simon Ser wrote:
> This new kernel capability indicates whether async page-flips are
> supported via the atomic uAPI. DRM clients can use it to check
> for support before feeding DRM_MODE_PAGE_FLIP_ASYNC to the kernel.

I think we'd need to clarify the semantics of the async flag
for atomic commits.

Eg. on Intel hw only pure page flips are possible async, if you do
anything else (change plane size/pos/scaling/etc.) you will need
to do a sync update. Technically not even all page flips (from the
uapi POV) might be possible as the exact scanout source address
is specified via two registers, only one of which can be update
async. So technically the two framebuffers might be laid out
just slightly differently which could prevent an async flip.
Also only some subset of planes actually support async flips.

And on hw where multiple planes support it on the same crtc, only one
plane can do it at a time. Well, more accurately we can only select
one plane at a time to give us the "flip done" interrupt. I guess
if the user wants to async flip multiple planes at the same time
we could do them serially as opposed to in parallel to make sure
all the flips actually happened before we signal completion of the
entire commit. Async flips of multiple planes probably won't
happen atomically anyway so doing them serially seems fine.

ATM in i915 we probably don't have sufficient state checks in
place to catch all the restrictions, and instead in part we rely
on the limited scope of the legacy async flip ioctl to make sure
the operation doesn't attempt something the hw can't do.

> Make it clear that DRM_CAP_ASYNC_PAGE_FLIP is for legacy uAPI only.
> 
> Signed-off-by: Simon Ser <contact@emersion.fr>
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Cc: Joshua Ashton <joshua@froggi.es>
> Cc: Melissa Wen <mwen@igalia.com>
> Cc: Alex Deucher <alexander.deucher@amd.com>
> Cc: Harry Wentland <hwentlan@amd.com>
> Cc: Nicholas Kazlauskas <nicholas.kazlauskas@amd.com>
> ---
>  drivers/gpu/drm/drm_ioctl.c |  5 +++++
>  include/uapi/drm/drm.h      | 10 +++++++++-
>  2 files changed, 14 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c
> index ca2a6e6101dc..5b1591e2b46c 100644
> --- a/drivers/gpu/drm/drm_ioctl.c
> +++ b/drivers/gpu/drm/drm_ioctl.c
> @@ -302,6 +302,11 @@ static int drm_getcap(struct drm_device *dev, void *data, struct drm_file *file_
>  	case DRM_CAP_CRTC_IN_VBLANK_EVENT:
>  		req->value = 1;
>  		break;
> +	case DRM_CAP_ATOMIC_ASYNC_PAGE_FLIP:
> +		req->value = drm_core_check_feature(dev, DRIVER_ATOMIC) &&
> +			     dev->mode_config.async_page_flip &&
> +			     !dev->mode_config.atomic_async_page_flip_not_supported;
> +		break;
>  	default:
>  		return -EINVAL;
>  	}
> diff --git a/include/uapi/drm/drm.h b/include/uapi/drm/drm.h
> index 642808520d92..b1962628ecda 100644
> --- a/include/uapi/drm/drm.h
> +++ b/include/uapi/drm/drm.h
> @@ -706,7 +706,8 @@ struct drm_gem_open {
>  /**
>   * DRM_CAP_ASYNC_PAGE_FLIP
>   *
> - * If set to 1, the driver supports &DRM_MODE_PAGE_FLIP_ASYNC.
> + * If set to 1, the driver supports &DRM_MODE_PAGE_FLIP_ASYNC for legacy
> + * page-flips.
>   */
>  #define DRM_CAP_ASYNC_PAGE_FLIP		0x7
>  /**
> @@ -767,6 +768,13 @@ struct drm_gem_open {
>   * Documentation/gpu/drm-mm.rst, section "DRM Sync Objects".
>   */
>  #define DRM_CAP_SYNCOBJ_TIMELINE	0x14
> +/**
> + * DRM_CAP_ATOMIC_ASYNC_PAGE_FLIP
> + *
> + * If set to 1, the driver supports &DRM_MODE_PAGE_FLIP_ASYNC for atomic
> + * commits.
> + */
> +#define DRM_CAP_ATOMIC_ASYNC_PAGE_FLIP	0x15
>  
>  /* DRM_IOCTL_GET_CAP ioctl argument type */
>  struct drm_get_cap {
> -- 
> 2.37.2
> 

-- 
Ville Syrjälä
Intel