Re: [PATCH libdrm 2/2] Add CRTC ID to vblank event

From: Emil Velikov <emil.l.velikov@gmail.com>
To: Daniel Stone <daniels@collabora.com>
Cc: Maarten Lankhorst <maarten.lankhorst@intel.com>,
	ML dri-devel <dri-devel@lists.freedesktop.org>
Subject: Re: [PATCH libdrm 2/2] Add CRTC ID to vblank event
Date: Tue, 4 Apr 2017 18:12:19 +0100	[thread overview]
Message-ID: <CACvgo53seGPO2tCf2243ZDUb6dU7Jk+Wbq0B9=3ALWn=e6ftaQ@mail.gmail.com> (raw)
In-Reply-To: <20170404165221.28240-1-daniels@collabora.com>

Hi Daniel,

Zero objections against the patch, just a couple of small notes.

On 4 April 2017 at 17:52, Daniel Stone <daniels@collabora.com> wrote:
> From: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>
>
> When using the atomic API, one request can span multiple CRTCs, however
> one event is generated per CRTC. As we cannot disambiguate the CRTC with
> user data (since we only have one piece of user data to pass in), newer
> kernels can include the CRTC ID in the page flip event.
>
> Add a new vfunc to dispatch vblank events carrying a CRTC ID to clients
> who negotiate a higher interface version.
>
> [daniels: Rebased, include new cap, call page_flip_handler if it is set
>           but page_flip_handler2 isn't even on newer contexts, write a
>           commit message.]
>
> Signed-off-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>
> Signed-off-by: Daniel Stone <daniels@collabora.com>
> Cc: Maarten Lankhorst <maarten.lankhorst@intel.com>
> ---
>  include/drm/drm.h |  3 ++-
>  xf86drm.h         |  9 ++++++++-
>  xf86drmMode.c     | 24 ++++++++++++++++--------
>  3 files changed, 26 insertions(+), 10 deletions(-)
>
> diff --git a/include/drm/drm.h b/include/drm/drm.h
> index 09d4262f..c4492372 100644
> --- a/include/drm/drm.h
> +++ b/include/drm/drm.h
> @@ -641,6 +641,7 @@ struct drm_gem_open {
>  #define DRM_CAP_CURSOR_HEIGHT          0x9
>  #define DRM_CAP_ADDFB2_MODIFIERS       0x10
>  #define DRM_CAP_PAGE_FLIP_TARGET       0x11
> +#define DRM_CAP_CRTC_IN_VBLANK_EVENT   0x12
>
>  /** DRM_IOCTL_GET_CAP ioctl argument type */
>  struct drm_get_cap {
> @@ -846,7 +847,7 @@ struct drm_event_vblank {
>         __u32 tv_sec;
>         __u32 tv_usec;
>         __u32 sequence;
> -       __u32 reserved;
> +       __u32 crtc_id;
>  };
>
Please split these to a separate commit as mentioned in
include/drm/README "When and how to update these files" ?

>  /* typedef area */
> diff --git a/xf86drm.h b/xf86drm.h
> index 0d927018..d75ca8ce 100644
> --- a/xf86drm.h
> +++ b/xf86drm.h
> @@ -728,7 +728,7 @@ extern void drmMsg(const char *format, ...) DRM_PRINTFLIKE(1, 2);
>  extern int drmSetMaster(int fd);
>  extern int drmDropMaster(int fd);
>
> -#define DRM_EVENT_CONTEXT_VERSION 2
> +#define DRM_EVENT_CONTEXT_VERSION 3
>
We really want to fix weston to _not_ use this define, but to set the
version it actually supports/implements.
I'll send a patch for that in a minute.

-Emil
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel