Re: [RFC v1 3/6] drm: Add a capability flag to support additional flip completion signalling

[Pekka Paalanen <ppaalanen@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 3112 bytes --]

On Mon, 13 Sep 2021 16:35:26 -0700
Vivek Kasireddy <vivek.kasireddy@intel.com> wrote:

> If a driver supports this capability, it means that there would be an
> additional signalling mechanism for a page flip completion in addition
> to out_fence or DRM_MODE_PAGE_FLIP_EVENT.
> 
> This capability may only be relevant for Virtual KMS drivers and is currently
> used only by virtio-gpu. Also, it can provide a potential solution for:
> https://gitlab.freedesktop.org/wayland/weston/-/issues/514
> 
> Signed-off-by: Vivek Kasireddy <vivek.kasireddy@intel.com>
> ---
>  drivers/gpu/drm/drm_ioctl.c   | 3 +++
>  include/drm/drm_mode_config.h | 8 ++++++++
>  include/uapi/drm/drm.h        | 1 +
>  3 files changed, 12 insertions(+)
> 
> diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c
> index 8b8744dcf691..8a420844f8bc 100644
> --- a/drivers/gpu/drm/drm_ioctl.c
> +++ b/drivers/gpu/drm/drm_ioctl.c
> @@ -302,6 +302,9 @@ 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_RELEASE_FENCE:
> +		req->value = dev->mode_config.release_fence;
> +		break;

Hi Vivek,

is this actually necessary?

I would think that userspace figures out the existence of the release
fence capability by seeing that the KMS property "RELEASE_FENCE_PTR"
either exists or not.

However, would we not need a client cap instead?

If a KMS driver knows that userspace is aware of "RELEASE_FENCE_PTR"
and will use it when necessary, then the KMS driver can send the
pageflip completion without waiting for the host OS to signal the old
buffer as free for re-use.

If the KMS driver does not know that userspace can handle pageflip
completing "too early", then it has no choice but to wait until the old
buffer is really free before signalling pageflip completion.

Wouldn't that make sense?


Otherwise, this proposal sounds fine to me.


Thanks,
pq


>  	default:
>  		return -EINVAL;
>  	}
> diff --git a/include/drm/drm_mode_config.h b/include/drm/drm_mode_config.h
> index 12b964540069..944bebf359d7 100644
> --- a/include/drm/drm_mode_config.h
> +++ b/include/drm/drm_mode_config.h
> @@ -935,6 +935,14 @@ struct drm_mode_config {
>  	 */
>  	bool normalize_zpos;
>  
> +	/**
> +	 * @release_fence:
> +	 *
> +	 * If this option is set, it means there would be an additional signalling
> +	 * mechanism for a page flip completion.
> +	 */
> +	bool release_fence;
> +
>  	/**
>  	 * @modifiers_property: Plane property to list support modifier/format
>  	 * combination.
> diff --git a/include/uapi/drm/drm.h b/include/uapi/drm/drm.h
> index 3b810b53ba8b..8b8985f65581 100644
> --- a/include/uapi/drm/drm.h
> +++ b/include/uapi/drm/drm.h
> @@ -767,6 +767,7 @@ struct drm_gem_open {
>   * Documentation/gpu/drm-mm.rst, section "DRM Sync Objects".
>   */
>  #define DRM_CAP_SYNCOBJ_TIMELINE	0x14
> +#define DRM_CAP_RELEASE_FENCE		0x15
>  
>  /* DRM_IOCTL_GET_CAP ioctl argument type */
>  struct drm_get_cap {


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]