Hi,
Right now, if an atomic commit sends multiple vblank events it is not possible to distiguish for each crtc each event is for in userspace. This series changes that be repurposing the reserved field in struct drm_event_vblank.
Would this be a sane thing to do?
Thanks, Ander
drivers/gpu/drm/drm_atomic.c | 7 +++++-- drivers/gpu/drm/drm_crtc.c | 1 + drivers/gpu/drm/drm_ioctl.c | 3 +++ include/uapi/drm/drm.h | 3 ++- 4 files changed, 11 insertions(+), 3 deletions(-)
With the atomic API, it is possible that a single commit affects multiple crtcs. If the user requests an event with that commit, one event will be sent for each CRTC, but it is not possible to distinguish which crtc an event is for in user space. To solve this, the reserved field in struct drm_vblank_event is repurposed to include the crtc_id which the event is for.
The DRM_CAP_CRTC_IN_VBLANK_EVENT is added to allow userspace to query if the crtc field will be set properly.
Signed-off-by: Ander Conselvan de Oliveira ander.conselvan.de.oliveira@intel.com --- drivers/gpu/drm/drm_atomic.c | 7 +++++-- drivers/gpu/drm/drm_crtc.c | 1 + drivers/gpu/drm/drm_ioctl.c | 3 +++ include/uapi/drm/drm.h | 3 ++- 4 files changed, 11 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c index 1066e4b..2a76d10 100644 --- a/drivers/gpu/drm/drm_atomic.c +++ b/drivers/gpu/drm/drm_atomic.c @@ -1304,7 +1304,8 @@ EXPORT_SYMBOL(drm_atomic_async_commit); */
static struct drm_pending_vblank_event *create_vblank_event( - struct drm_device *dev, struct drm_file *file_priv, uint64_t user_data) + struct drm_device *dev, struct drm_file *file_priv, + uint64_t user_data, struct drm_crtc *crtc) { struct drm_pending_vblank_event *e = NULL; unsigned long flags; @@ -1328,6 +1329,7 @@ static struct drm_pending_vblank_event *create_vblank_event( e->event.base.type = DRM_EVENT_FLIP_COMPLETE; e->event.base.length = sizeof e->event; e->event.user_data = user_data; + e->event.crtc_id = crtc->base.id; e->base.event = &e->event.base; e->base.file_priv = file_priv; e->base.destroy = (void (*) (struct drm_pending_event *)) kfree; @@ -1529,7 +1531,8 @@ retry: for_each_crtc_in_state(state, crtc, crtc_state, i) { struct drm_pending_vblank_event *e;
- e = create_vblank_event(dev, file_priv, arg->user_data); + e = create_vblank_event(dev, file_priv, arg->user_data, + crtc); if (!e) { ret = -ENOMEM; goto out; diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c index 33d877c..98fe624 100644 --- a/drivers/gpu/drm/drm_crtc.c +++ b/drivers/gpu/drm/drm_crtc.c @@ -5212,6 +5212,7 @@ int drm_mode_page_flip_ioctl(struct drm_device *dev, e->event.base.type = DRM_EVENT_FLIP_COMPLETE; e->event.base.length = sizeof(e->event); e->event.user_data = page_flip->user_data; + e->event.crtc_id = crtc->base.id; e->base.event = &e->event.base; e->base.file_priv = file_priv; e->base.destroy = diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c index b1d303f..152aeba 100644 --- a/drivers/gpu/drm/drm_ioctl.c +++ b/drivers/gpu/drm/drm_ioctl.c @@ -312,6 +312,9 @@ static int drm_getcap(struct drm_device *dev, void *data, struct drm_file *file_ case DRM_CAP_ADDFB2_MODIFIERS: req->value = dev->mode_config.allow_fb_modifiers; break; + case DRM_CAP_CRTC_IN_VBLANK_EVENT: + req->value = 1; + break; default: return -EINVAL; } diff --git a/include/uapi/drm/drm.h b/include/uapi/drm/drm.h index 3801584..ecd9e96 100644 --- a/include/uapi/drm/drm.h +++ b/include/uapi/drm/drm.h @@ -631,6 +631,7 @@ struct drm_gem_open { #define DRM_CAP_CURSOR_WIDTH 0x8 #define DRM_CAP_CURSOR_HEIGHT 0x9 #define DRM_CAP_ADDFB2_MODIFIERS 0x10 +#define DRM_CAP_CRTC_IN_VBLANK_EVENT 0x11
/** DRM_IOCTL_GET_CAP ioctl argument type */ struct drm_get_cap { @@ -826,7 +827,7 @@ struct drm_event_vblank { __u32 tv_sec; __u32 tv_usec; __u32 sequence; - __u32 reserved; + __u32 crtc_id; };
/* typedef area */
Signed-off-by: Ander Conselvan de Oliveira ander.conselvan.de.oliveira@intel.com
--- include/drm/drm.h | 2 +- xf86drm.h | 9 ++++++++- xf86drmMode.c | 26 +++++++++++++++++--------- 3 files changed, 26 insertions(+), 11 deletions(-)
diff --git a/include/drm/drm.h b/include/drm/drm.h index 167b7b8..81f8af1 100644 --- a/include/drm/drm.h +++ b/include/drm/drm.h @@ -806,7 +806,7 @@ struct drm_event_vblank { __u32 tv_sec; __u32 tv_usec; __u32 sequence; - __u32 reserved; + __u32 crtc_id; };
#define DRM_CAP_DUMB_BUFFER 0x1 diff --git a/xf86drm.h b/xf86drm.h index 360e04a..6eef8ac 100644 --- a/xf86drm.h +++ b/xf86drm.h @@ -726,7 +726,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
typedef struct _drmEventContext {
@@ -746,6 +746,13 @@ typedef struct _drmEventContext { unsigned int tv_usec, void *user_data);
+ void (*page_flip_handler2)(int fd, + unsigned int sequence, + unsigned int tv_sec, + unsigned int tv_usec, + unsigned int crtc_id, + void *user_data); + } drmEventContext, *drmEventContextPtr;
extern int drmHandleEvent(int fd, drmEventContextPtr evctx); diff --git a/xf86drmMode.c b/xf86drmMode.c index fc19504..a9119a2 100644 --- a/xf86drmMode.c +++ b/xf86drmMode.c @@ -879,7 +879,8 @@ int drmHandleEvent(int fd, drmEventContextPtr evctx) int len, i; struct drm_event *e; struct drm_event_vblank *vblank; - + void *user_data; + /* The DRM read semantics guarantees that we always get only * complete events. */
@@ -905,15 +906,22 @@ int drmHandleEvent(int fd, drmEventContextPtr evctx) U642VOID (vblank->user_data)); break; case DRM_EVENT_FLIP_COMPLETE: - if (evctx->version < 2 || - evctx->page_flip_handler == NULL) - break; vblank = (struct drm_event_vblank *) e; - evctx->page_flip_handler(fd, - vblank->sequence, - vblank->tv_sec, - vblank->tv_usec, - U642VOID (vblank->user_data)); + user_data = U642VOID (vblank->user_data); + + if (evctx->version >= 3 && evctx->page_flip_handler2) + evctx->page_flip_handler2(fd, + vblank->sequence, + vblank->tv_sec, + vblank->tv_usec, + vblank->crtc_id, + user_data); + else if (evctx->version == 2 && evctx->page_flip_handler) + evctx->page_flip_handler(fd, + vblank->sequence, + vblank->tv_sec, + vblank->tv_usec, + user_data); break; default: break;
With the atomic API, it is possible that a single commit affects multiple crtcs. If the user requests an event with that commit, one event will be sent for each CRTC, but it is not possible to distinguish which crtc an event is for in user space. To solve this, the reserved field in struct drm_vblank_event is repurposed to include the crtc_id which the event is for.
The DRM_CAP_CRTC_IN_VBLANK_EVENT is added to allow userspace to query if the crtc field will be set properly.
Signed-off-by: Ander Conselvan de Oliveira ander.conselvan.de.oliveira@intel.com --- drivers/gpu/drm/drm_atomic.c | 7 +++++-- drivers/gpu/drm/drm_crtc.c | 1 + drivers/gpu/drm/drm_ioctl.c | 3 +++ include/uapi/drm/drm.h | 3 ++- 4 files changed, 11 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c index 1066e4b..2a76d10 100644 --- a/drivers/gpu/drm/drm_atomic.c +++ b/drivers/gpu/drm/drm_atomic.c @@ -1304,7 +1304,8 @@ EXPORT_SYMBOL(drm_atomic_async_commit); */
static struct drm_pending_vblank_event *create_vblank_event( - struct drm_device *dev, struct drm_file *file_priv, uint64_t user_data) + struct drm_device *dev, struct drm_file *file_priv, + uint64_t user_data, struct drm_crtc *crtc) { struct drm_pending_vblank_event *e = NULL; unsigned long flags; @@ -1328,6 +1329,7 @@ static struct drm_pending_vblank_event *create_vblank_event( e->event.base.type = DRM_EVENT_FLIP_COMPLETE; e->event.base.length = sizeof e->event; e->event.user_data = user_data; + e->event.crtc_id = crtc->base.id; e->base.event = &e->event.base; e->base.file_priv = file_priv; e->base.destroy = (void (*) (struct drm_pending_event *)) kfree; @@ -1529,7 +1531,8 @@ retry: for_each_crtc_in_state(state, crtc, crtc_state, i) { struct drm_pending_vblank_event *e;
- e = create_vblank_event(dev, file_priv, arg->user_data); + e = create_vblank_event(dev, file_priv, arg->user_data, + crtc); if (!e) { ret = -ENOMEM; goto out; diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c index 33d877c..98fe624 100644 --- a/drivers/gpu/drm/drm_crtc.c +++ b/drivers/gpu/drm/drm_crtc.c @@ -5212,6 +5212,7 @@ int drm_mode_page_flip_ioctl(struct drm_device *dev, e->event.base.type = DRM_EVENT_FLIP_COMPLETE; e->event.base.length = sizeof(e->event); e->event.user_data = page_flip->user_data; + e->event.crtc_id = crtc->base.id; e->base.event = &e->event.base; e->base.file_priv = file_priv; e->base.destroy = diff --git a/drivers/gpu/drm/drm_ioctl.c b/drivers/gpu/drm/drm_ioctl.c index b1d303f..152aeba 100644 --- a/drivers/gpu/drm/drm_ioctl.c +++ b/drivers/gpu/drm/drm_ioctl.c @@ -312,6 +312,9 @@ static int drm_getcap(struct drm_device *dev, void *data, struct drm_file *file_ case DRM_CAP_ADDFB2_MODIFIERS: req->value = dev->mode_config.allow_fb_modifiers; break; + case DRM_CAP_CRTC_IN_VBLANK_EVENT: + req->value = 1; + break; default: return -EINVAL; } diff --git a/include/uapi/drm/drm.h b/include/uapi/drm/drm.h index 3801584..ecd9e96 100644 --- a/include/uapi/drm/drm.h +++ b/include/uapi/drm/drm.h @@ -631,6 +631,7 @@ struct drm_gem_open { #define DRM_CAP_CURSOR_WIDTH 0x8 #define DRM_CAP_CURSOR_HEIGHT 0x9 #define DRM_CAP_ADDFB2_MODIFIERS 0x10 +#define DRM_CAP_CRTC_IN_VBLANK_EVENT 0x11
/** DRM_IOCTL_GET_CAP ioctl argument type */ struct drm_get_cap { @@ -826,7 +827,7 @@ struct drm_event_vblank { __u32 tv_sec; __u32 tv_usec; __u32 sequence; - __u32 reserved; + __u32 crtc_id; };
/* typedef area */
On Mon, Aug 17, 2015 at 04:21:22PM +0300, Ander Conselvan de Oliveira wrote:
Hi,
Right now, if an atomic commit sends multiple vblank events it is not possible to distiguish for each crtc each event is for in userspace. This series changes that be repurposing the reserved field in struct drm_event_vblank.
Would this be a sane thing to do?
Mostly just needs userspace since atm Weston has a per-crtc draw-loop and doesn't seem to care about events for big modesets. -Daniel
dri-devel@lists.freedesktop.org
-
Ander Conselvan de Oliveira -
Daniel Vetter