PRIME Synchronization
Subsystems (20)
cover PRIME Synchronization
Hello all,
For a while now, I've been working to fix tearing with PRIME. I have a working solution that requires changes to DRM, libdrm, and the X server. I've also implemented sink support in the modesetting driver, and source support in the NVIDIA proprietary driver.
These DRM patches ultimately provide a new ioctl, DRM_IOCTL_PRIME_PAGE_FLIP, that sets up a fence on a PRIME member's DMA-BUF reservation object with a callback to schedule a flip to the associated fb.
With my synchronization patches to X, it is possible to export two shared buffers per crtc instead of just one. The sink driver uses DRM_IOCTL_PRIME_PAGE_FLIP to set up fences on each of these buffers. When the source driver is done presenting on a given buffer, it signals the associated fence, and the sink flips on the next vblank. In the page flip event handler, the sink driver uses the ioctl to set up another fence, and explicitly requests a present on the current back buffer using a new function in the X ABI. Thus, flips are driven by presents, presents are driven by flips, and the whole thing conforms to the sink's refresh rate.
Right now, the part that signals the fence is a vendor-specific ioctl, but I'm open to factoring it out into a core DRM ioctl and DRM driver function helper like DRM_IOCTL_PRIME_PAGE_FLIP if it would make implementing source support easier for open source drivers.
Due to the lack of asynchronous atomic modesetting support on i915 (the sink of most hybrid graphics systems,) I've implemented this on top of the old page_flip method. Perhaps an atomic modesetting / nuclear pageflip based method would be better, but it wouldn't be as viable without widespread asynchronous support. I'm willing to implement an atomic modesetting version if required, but it's less clear what the best implementation would be: perhaps a fence property that defers asynchronous commits. There are enough ways to do it that it probably warrants prior discussion. Nonetheless, this method is necessary at least as a fallback until i915 supports asynchronous atomic modesetting.
To give some greater context, I've uploaded my branches for DRM, libdrm, and the X server to Github. I'll move forward with upstreaming the libdrm and X changes if and when these DRM patches go in.
DRM Tree: https://github.com/GoinsWithTheWind/drm-prime-sync libdrm Tree: https://github.com/GoinsWithTheWind/libdrm-prime-sync X Tree: https://github.com/GoinsWithTheWind/xserver-prime-sync
Thanks,
Alex @ NVIDIA Linux Driver Team
agoins (6):
drm: factor out drm_mode_page_flip_ioctl()
drm: generalize drm_prime_lookup
drm: add fence context
drm: add drm_gem_prime_page_flip() helper
drm: add prime_page_flip() driver function
drm: add DRM_IOCTL_PRIME_PAGE_FLIP
drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 1 + drivers/gpu/drm/armada/armada_drv.c | 1 + drivers/gpu/drm/drm_crtc.c | 90 ++++++++---- drivers/gpu/drm/drm_internal.h | 2 + drivers/gpu/drm/drm_ioctl.c | 1 + drivers/gpu/drm/drm_prime.c | 213 ++++++++++++++++++++++++++-- drivers/gpu/drm/exynos/exynos_drm_drv.c | 1 + drivers/gpu/drm/i915/i915_drv.c | 1 + drivers/gpu/drm/imx/imx-drm-core.c | 1 + drivers/gpu/drm/msm/msm_drv.c | 1 + drivers/gpu/drm/nouveau/nouveau_drm.c | 1 + drivers/gpu/drm/omapdrm/omap_drv.c | 1 + drivers/gpu/drm/qxl/qxl_drv.c | 1 + drivers/gpu/drm/radeon/radeon_drv.c | 1 + drivers/gpu/drm/rcar-du/rcar_du_drv.c | 1 + drivers/gpu/drm/rockchip/rockchip_drm_drv.c | 1 + drivers/gpu/drm/shmobile/shmob_drm_drv.c | 1 + drivers/gpu/drm/sti/sti_drv.c | 1 + drivers/gpu/drm/tegra/drm.c | 1 + drivers/gpu/drm/udl/udl_drv.c | 1 + drivers/gpu/drm/vmwgfx/vmwgfx_drv.c | 1 + include/drm/drmP.h | 8 ++ include/drm/drm_crtc.h | 4 + include/uapi/drm/drm.h | 10 ++ 24 files changed, 303 insertions(+), 42 deletions(-)
Signature · 6 lines
1.9.1 _______________________________________________ dri-devel mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/dri-devel
PATCH 1/6 drm: factor out drm_mode_page_flip_ioctl()
From: agoins <[email protected]>
Factors contents of drm_mode_page_flip_ioctl() into drm_mode_page_flip(), allowing it to be callable from the kernel within DRM. Replace contents of drm_mode_page_flip_ioctl() with a call to drm_mode_page_flip().
--- drivers/gpu/drm/drm_crtc.c | 90 +++++++++++++++++++++++++++++++--------------- include/drm/drm_crtc.h | 4 +++ 2 files changed, 66 insertions(+), 28 deletions(-)
drivers/gpu/drm/drm_crtc.c +62 -28
@@ -5126,41 +5126,40 @@ out: }/** -* drm_mode_page_flip_ioctl - schedule an asynchronous fb update +* drm_mode_page_flip - schedule an asynchronous fb update * @dev: DRM device -* @data: ioctl data +* @crtc_id: CRTC to update +* @fb_id: FB to update to +* @flags: Flags modifying page flip behavior. +* @user_data: Returned as user_data field in in the vblank event struct * @file_priv: DRM file info * * This schedules an asynchronous update on a given CRTC, called page flip. * Optionally a drm event is generated to signal the completion of the event. * Generic drivers cannot assume that a pageflip with changed framebuffer * properties (including driver specific metadata like tiling layout) will work, -* but some drivers support e.g. pixel format changes through the pageflip -* ioctl. -* -* Called by the user via ioctl. +* but some drivers support e.g. pixel format changes through the pageflip. * * Returns: * Zero on success, negative errno on failure. */ -int drm_mode_page_flip_ioctl (struct drm_device *dev, -void *data,struct drm_file *file_priv) +int drm_mode_page_flip (struct drm_device *dev, +uint32_t crtc_id,uint32_t fb_id, +uint32_t flags,uint64_t user_data, +struct drm_file *file_priv) { -struct drm_mode_crtc_page_flip *page_flip = data;struct drm_crtc *crtc;struct drm_framebuffer *fb =NULL ;struct drm_pending_vblank_event *e =NULL ; -unsigned long flags; +unsigned long lock_flags;int ret = -EINVAL ; -if (page_flip->flags & ~DRM_MODE_PAGE_FLIP_FLAGS || - page_flip->reserved !=0 ) -return -EINVAL ; - -if ((page_flip->flags &DRM_MODE_PAGE_FLIP_ASYNC ) && !dev->mode_config.async_page_flip) +if ((flags &DRM_MODE_PAGE_FLIP_ASYNC ) && + !dev->mode_config.async_page_flip) {return -EINVAL ; + } - crtc =drm_crtc_find (dev, page_flip->crtc_id); + crtc =drm_crtc_find (dev, crtc_id);if (!crtc)return -ENOENT ;
@@ -5177,7 +5176,7 @@ int drm_mode_page_flip_ioctl(struct drm_device *dev,if (crtc->funcs->page_flip ==NULL ) goto out; - fb =drm_framebuffer_lookup (dev, page_flip->fb_id); + fb =drm_framebuffer_lookup (dev, fb_id);if (!fb) { ret = -ENOENT ; goto out;
@@ -5200,27 +5199,27 @@ int drm_mode_page_flip_ioctl(struct drm_device *dev, goto out; } -if (page_flip->flags &DRM_MODE_PAGE_FLIP_EVENT ) { +if (flags &DRM_MODE_PAGE_FLIP_EVENT ) { ret = -ENOMEM ; -spin_lock_irqsave (&dev->event_lock, flags); +spin_lock_irqsave (&dev->event_lock, lock_flags);if (file_priv->event_space <sizeof (e->event)) { -spin_unlock_irqrestore (&dev->event_lock, flags); +spin_unlock_irqrestore (&dev->event_lock, lock_flags); goto out; } file_priv->event_space -=sizeof (e->event); -spin_unlock_irqrestore (&dev->event_lock, flags); +spin_unlock_irqrestore (&dev->event_lock, lock_flags); e =kzalloc (sizeof (*e),GFP_KERNEL );if (e ==NULL ) { -spin_lock_irqsave (&dev->event_lock, flags); +spin_lock_irqsave (&dev->event_lock, lock_flags); file_priv->event_space +=sizeof (e->event); -spin_unlock_irqrestore (&dev->event_lock, flags); +spin_unlock_irqrestore (&dev->event_lock, lock_flags); goto out; } 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.user_data = user_data; e->base.event = &e->event.base; e->base.file_priv = file_priv; e->base.destroy =
@@ -5228,12 +5227,12 @@ int drm_mode_page_flip_ioctl(struct drm_device *dev, } crtc->primary->old_fb = crtc->primary->fb; - ret = crtc->funcs->page_flip (crtc, fb, e, page_flip->flags); + ret = crtc->funcs->page_flip (crtc, fb, e, flags);if (ret) { -if (page_flip->flags &DRM_MODE_PAGE_FLIP_EVENT ) { -spin_lock_irqsave (&dev->event_lock, flags); +if (flags &DRM_MODE_PAGE_FLIP_EVENT ) { +spin_lock_irqsave (&dev->event_lock, lock_flags); file_priv->event_space +=sizeof (e->event); -spin_unlock_irqrestore (&dev->event_lock, flags); +spin_unlock_irqrestore (&dev->event_lock, lock_flags);kfree (e); }/* Keep the old fb, don't unref it. */
@@ -5256,6 +5255,41 @@ out: }/** +* drm_mode_page_flip_ioctl - schedule an asynchronous fb update +* @dev: DRM device +* @data: ioctl data +* @file_priv: DRM file info +* +* This schedules an asynchronous update on a given CRTC, called page flip. +* Optionally a drm event is generated to signal the completion of the event. +* Generic drivers cannot assume that a pageflip with changed framebuffer +* properties (including driver specific metadata like tiling layout) will work, +* but some drivers support e.g. pixel format changes through the pageflip +* ioctl. +* +* Called by the user via ioctl. +* +* Returns: +* Zero on success, negative errno on failure. +*/ +int drm_mode_page_flip_ioctl (struct drm_device *dev, +void *data,struct drm_file *file_priv) +{ +struct drm_mode_crtc_page_flip *page_flip = data; + +if (page_flip->reserved !=0 ) +return -EINVAL ; + +if (page_flip->flags & ~DRM_MODE_PAGE_FLIP_FLAGS ) +return -EINVAL ; + +return drm_mode_page_flip (dev, + page_flip->crtc_id, page_flip->fb_id, + page_flip->flags, page_flip->user_data, + file_priv); +} + +/** * drm_mode_config_reset - call ->reset callbacks * @dev: drm device *
include/drm/drm_crtc.h +4 -0
@@ -1448,6 +1448,10 @@ extern enum hdmi_picture_aspect drm_get_cea_aspect_ratio(const u8 video_code);extern bool drm_detect_hdmi_monitor (struct edid *edid);extern bool drm_detect_monitor_audio (struct edid *edid);extern bool drm_rgb_quant_range_selectable (struct edid *edid); +extern int drm_mode_page_flip (struct drm_device *dev, +uint32_t crtc_id,uint32_t fb_id, +uint32_t flags,uint64_t user_data, +struct drm_file *file_priv);extern int drm_mode_page_flip_ioctl (struct drm_device *dev,void *data,struct drm_file *file_priv); extern int drm_add_modes_noedid(struct drm_connector *connector,
Signature · 6 lines
1.9.1 _______________________________________________ dri-devel mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/dri-devel
PATCH 2/6 drm: generalize drm_prime_lookup
In reply to Alex GoinsFrom: agoins <[email protected]>
Replace drm_prime_lookup_buf_by_handle() and drm_prime_lookup_buf_handle() with more generalized drm_prime_lookup_member_by_handle() and drm_prime_lookup_buf_member(), respectively. New implementations return the member itself rather than extracting a specific field, allowing other fields to be queried as well.
--- drivers/gpu/drm/drm_prime.c | 34 ++++++++++++++++++++-------------- 1 file changed, 20 insertions(+), 14 deletions(-)
drivers/gpu/drm/drm_prime.c +20 -14
@@ -87,32 +87,33 @@ static int drm_prime_add_buf_handle(struct drm_prime_file_private *prime_fpriv,return 0 ; } -static struct dma_buf *drm_prime_lookup_buf_by_handle (struct drm_prime_file_private *prime_fpriv, -uint32_t handle) +static struct drm_prime_member *drm_prime_lookup_member_by_handle ( +struct drm_prime_file_private *prime_fpriv, +uint32_t handle) {struct drm_prime_member *member;list_for_each_entry (member, &prime_fpriv->head, entry) {if (member->handle == handle) -return member->dma_buf; +return member; }return NULL ; } -static int drm_prime_lookup_buf_handle (struct drm_prime_file_private *prime_fpriv, -struct dma_buf *dma_buf, -uint32_t *handle) +static struct drm_prime_member *drm_prime_lookup_buf_member ( +struct drm_prime_file_private *prime_fpriv, +struct dma_buf *dma_buf) {struct drm_prime_member *member;list_for_each_entry (member, &prime_fpriv->head, entry) {if (member->dma_buf == dma_buf) { - *handle = member->handle; -return 0 ; +return member; } } -return -ENOENT ; + +return NULL ; } static int drm_gem_map_attach(struct dma_buf *dma_buf,
@@ -404,6 +405,7 @@ int drm_gem_prime_handle_to_fd(struct drm_device *dev, {struct drm_gem_object *obj;int ret =0 ; +struct drm_prime_member *member;struct dma_buf *dmabuf;mutex_lock (&file_priv->prime.lock);
@@ -413,8 +415,9 @@ int drm_gem_prime_handle_to_fd(struct drm_device *dev, goto out_unlock; } - dmabuf =drm_prime_lookup_buf_by_handle (&file_priv->prime, handle); -if (dmabuf) { + member =drm_prime_lookup_member_by_handle (&file_priv->prime, handle); +if (member) { + dmabuf = member->dma_buf;get_dma_buf (dmabuf); goto out_have_handle; }
@@ -564,6 +567,7 @@ int drm_gem_prime_fd_to_handle(struct drm_device *dev, uint32_t *handle) {struct dma_buf *dma_buf; +struct drm_prime_member *member;struct drm_gem_object *obj;int ret;
@@ -573,10 +577,12 @@ int drm_gem_prime_fd_to_handle(struct drm_device *dev,mutex_lock (&file_priv->prime.lock); - ret =drm_prime_lookup_buf_handle (&file_priv->prime, - dma_buf, handle); -if (ret ==0 ) + member =drm_prime_lookup_buf_member (&file_priv->prime, dma_buf); +if (member) { + ret =0 ; + *handle = member->handle; goto out_put; + }/* never seen this one, need to import */ mutex_lock (&dev->object_name_lock);
Signature · 6 lines
1.9.1 _______________________________________________ dri-devel mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/dri-devel
PATCH 3/6 drm: add fence context
In reply to Alex GoinsFrom: agoins <[email protected]>
Adds new struct drm_prime_fence_ctx as field of drm_prime_member, and initializes it when member is created. Used for keeping track of context id and seqno of fences to be generated by PRIME.
--- drivers/gpu/drm/drm_prime.c | 12 ++++++++++++ 1 file changed, 12 insertions(+)
drivers/gpu/drm/drm_prime.c +12 -0
@@ -28,6 +28,7 @@#include <linux/export.h> #include <linux/dma-buf.h> +#include <linux/reservation.h> #include <drm/drmP.h> #include <drm/drm_gem.h>
@@ -60,8 +61,14 @@* use the drm_gem_prime_{import,export} helpers. */ +struct drm_prime_fence_ctx { +uint32_t context; +uint32_t seqno; +}; +struct drm_prime_member {struct list_head entry; +struct drm_prime_fence_ctx fctx;struct dma_buf *dma_buf;uint32_t handle; };
@@ -83,6 +90,11 @@ static int drm_prime_add_buf_handle(struct drm_prime_file_private *prime_fpriv,get_dma_buf (dma_buf); member->dma_buf = dma_buf; member->handle = handle; + +/* Initialize fence context for PRIME flipping */ + member->fctx.context =fence_context_alloc (1 ); + member->fctx.seqno =0 ; +list_add (&member->entry, &prime_fpriv->head);return 0 ; }
Signature · 6 lines
1.9.1 _______________________________________________ dri-devel mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/dri-devel
PATCH 4/6 drm: add drm_gem_prime_page_flip() helper
In reply to Alex GoinsFrom: agoins <[email protected]>
Adds drm_gem_prime_page_flip(), a helper implementation of prime_page_flip() to be used by DRM drivers.
--- drivers/gpu/drm/drm_prime.c | 147 ++++++++++++++++++++++++++++++++++++++++++++ include/drm/drmP.h | 3 + 2 files changed, 150 insertions(+)
drivers/gpu/drm/drm_prime.c +147 -0
@@ -61,6 +61,11 @@* use the drm_gem_prime_{import,export} helpers. */ +struct drm_prime_fence { +struct fence base; +spinlock_t lock; +}; +struct drm_prime_fence_ctx {uint32_t context;uint32_t seqno;
@@ -78,6 +83,16 @@ struct drm_prime_attachment {enum dma_data_direction dir; }; +struct drm_gem_prime_page_flip_cb { +struct fence_cb base; +struct drm_device *dev; +uint32_t crtc_id; +uint32_t fb_id; +uint32_t flags; +uint64_t user_data; +struct drm_file *file_priv; +}; +static int drm_prime_add_buf_handle (struct drm_prime_file_private *prime_fpriv,struct dma_buf *dma_buf,uint32_t handle) {
@@ -316,6 +331,28 @@ static const struct dma_buf_ops drm_gem_prime_dmabuf_ops = { .vunmap = drm_gem_dmabuf_vunmap, }; +static const char *drm_gem_prime_get_driver_name (struct fence *fence) +{ +return "PRIME" ; +} + +static const char *drm_gem_prime_get_timeline_name (struct fence *fence) +{ +return "prime.swonly" ; +} + +static bool drm_gem_prime_enable_signaling (struct fence *fence) +{ +return true;/* SW signaling only */ +} + +static const struct fence_ops drm_gem_prime_fence_ops = { + .get_driver_name = drm_gem_prime_get_driver_name, + .get_timeline_name = drm_gem_prime_get_timeline_name, + .enable_signaling = drm_gem_prime_enable_signaling, + .wait = fence_default_wait, +}; +/** * DOC: PRIME Helpers *
@@ -680,6 +717,116 @@ int drm_prime_fd_to_handle_ioctl(struct drm_device *dev, void *data, args->fd, &args->handle); } +/* Callback used by drm_gem_prime_page_flip(). Is added to a fence such that +* when the fence is signaled, page flip is requested with given parameters. */ +static void drm_gem_prime_page_flip_cb (struct fence *fence, +struct fence_cb *cb) +{ +struct drm_gem_prime_page_flip_cb *page_flip_cb = + container_of(cb, struct drm_gem_prime_page_flip_cb, base); + +drm_mode_page_flip (page_flip_cb->dev, + page_flip_cb->crtc_id, + page_flip_cb->fb_id, + page_flip_cb->flags, + page_flip_cb->user_data, + page_flip_cb->file_priv); + +kfree (cb); +} + +/** +* drm_gem_prime_page_flip - +* helper library implementation of the page flip callback +* +* @dev: DRM device +* @file_priv: DRM file info +* @handle: buffer handle to fence +* @fb_id: FB to update to +* @crtc_id: CRTC to update +* @user_data: Returned as user_data field in in the vblank event struct +* @flags: Flags modifying page flip behavior, same as drm_mode_page_flip. +* +* This is the implementation of the prime_page_flip function for GEM drivers +* using the PRIME helpers. +*/ +int drm_gem_prime_page_flip (struct drm_device *dev, +struct drm_file *file_priv,uint32_t handle, +uint32_t fb_id,uint32_t crtc_id, +uint64_t user_data,uint32_t flags) +{ +int ret; + +struct dma_buf *dmabuf; +struct fence *oldfence; +struct drm_prime_fence *fence; +struct drm_gem_prime_page_flip_cb *page_flip_cb; +struct drm_prime_member *member; +struct drm_prime_fence_ctx *fctx; + +mutex_lock (&file_priv->prime.lock); + + member =drm_prime_lookup_member_by_handle (&file_priv->prime, + handle); +if (!member) { +/* Buffer is not a member of PRIME */ + ret = -EINVAL ; + goto out; + } + + dmabuf = member->dma_buf; + fctx = &member->fctx; + +/* Clear out old fence callbacks */ + oldfence =reservation_object_get_excl (dmabuf->resv); +if (oldfence) +fence_signal (oldfence); + +/* Create and initialize new fence */ + fence =kmalloc (sizeof (*fence),GFP_KERNEL ); +if (!fence) { + ret = -ENOMEM ; + goto out; + } + +spin_lock_init (&fence->lock); +fence_init (&fence->base, &drm_gem_prime_fence_ops, &fence->lock, + fctx->context, fctx->seqno++); + +/* Create and initialize new page flip callback */ + page_flip_cb =kmalloc (sizeof (*page_flip_cb),GFP_KERNEL ); +if (!page_flip_cb) { + ret = -ENOMEM ; +fence_free (&fence->base); + goto out; + } + + *page_flip_cb = (struct drm_gem_prime_page_flip_cb ) { + .dev = dev, + .crtc_id = crtc_id, + .fb_id = fb_id, + .flags = flags, + .user_data = user_data, + .file_priv = file_priv, + }; + + ret =fence_add_callback (&fence->base, &page_flip_cb->base, + drm_gem_prime_page_flip_cb); +if (ret) { +fence_free (&fence->base); +kfree (page_flip_cb); + goto out; + } + +/* Add new fence */ +reservation_object_add_excl_fence (dmabuf->resv, &fence->base); +fence_put (&fence->base);/* Reservation object has reference */ +out: +mutex_unlock (&file_priv->prime.lock); +return ret; +} +EXPORT_SYMBOL (drm_gem_prime_page_flip); +/** * drm_prime_pages_to_sg - converts a page array into an sg list * @pages: pointer to the array of page pointers to convert
include/drm/drmP.h +3 -0
@@ -1028,6 +1028,9 @@ extern struct drm_gem_object *drm_gem_prime_import(struct drm_device *dev, struct dma_buf *dma_buf);extern int drm_gem_prime_fd_to_handle (struct drm_device *dev,struct drm_file *file_priv,int prime_fd,uint32_t *handle); +extern int drm_gem_prime_page_flip (struct drm_device *dev, +struct drm_file *file_priv,uint32_t handle,uint32_t fb_id, +uint32_t crtc_id,uint64_t user_data,uint32_t flags);extern void drm_gem_dmabuf_release (struct dma_buf *dma_buf); extern int drm_prime_sg_to_page_addr_arrays(struct sg_table *sgt, struct page **pages,
Signature · 6 lines
1.9.1 _______________________________________________ dri-devel mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/dri-devel
PATCH 5/6 drm: add prime_page_flip() driver function
In reply to Alex GoinsFrom: agoins <[email protected]>
Adds prime_page_flip() to struct drm_driver, intended to be a function to schedule a flip in response to a fence being signaled.
Makes drivers use drm_gem_prime_page_flip() helper implementation.
--- drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 1 + drivers/gpu/drm/armada/armada_drv.c | 1 + drivers/gpu/drm/exynos/exynos_drm_drv.c | 1 + drivers/gpu/drm/i915/i915_drv.c | 1 + drivers/gpu/drm/imx/imx-drm-core.c | 1 + drivers/gpu/drm/msm/msm_drv.c | 1 + drivers/gpu/drm/nouveau/nouveau_drm.c | 1 + drivers/gpu/drm/omapdrm/omap_drv.c | 1 + drivers/gpu/drm/qxl/qxl_drv.c | 1 + drivers/gpu/drm/radeon/radeon_drv.c | 1 + drivers/gpu/drm/rcar-du/rcar_du_drv.c | 1 + drivers/gpu/drm/rockchip/rockchip_drm_drv.c | 1 + drivers/gpu/drm/shmobile/shmob_drm_drv.c | 1 + drivers/gpu/drm/sti/sti_drv.c | 1 + drivers/gpu/drm/tegra/drm.c | 1 + drivers/gpu/drm/udl/udl_drv.c | 1 + drivers/gpu/drm/vmwgfx/vmwgfx_drv.c | 1 + include/drm/drmP.h | 5 +++++ 18 files changed, 22 insertions(+)
drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c +1 -0
@@ -512,6 +512,7 @@ static struct drm_driver kms_driver = { .prime_handle_to_fd = drm_gem_prime_handle_to_fd, .prime_fd_to_handle = drm_gem_prime_fd_to_handle, + .prime_page_flip = drm_gem_prime_page_flip, .gem_prime_export = amdgpu_gem_prime_export, .gem_prime_import = drm_gem_prime_import, .gem_prime_pin = amdgpu_gem_prime_pin,
drivers/gpu/drm/armada/armada_drv.c +1 -0
@@ -203,6 +203,7 @@ static struct drm_driver armada_drm_driver = { .gem_free_object = armada_gem_free_object, .prime_handle_to_fd = drm_gem_prime_handle_to_fd, .prime_fd_to_handle = drm_gem_prime_fd_to_handle, + .prime_page_flip = drm_gem_prime_page_flip, .gem_prime_export = armada_gem_prime_export, .gem_prime_import = armada_gem_prime_import, .dumb_create = armada_gem_dumb_create,
drivers/gpu/drm/exynos/exynos_drm_drv.c +1 -0
@@ -459,6 +459,7 @@ static struct drm_driver exynos_drm_driver = { .dumb_destroy = drm_gem_dumb_destroy, .prime_handle_to_fd = drm_gem_prime_handle_to_fd, .prime_fd_to_handle = drm_gem_prime_fd_to_handle, + .prime_page_flip = drm_gem_prime_page_flip, .gem_prime_export = drm_gem_prime_export, .gem_prime_import = drm_gem_prime_import, .gem_prime_get_sg_table = exynos_drm_gem_prime_get_sg_table,
drivers/gpu/drm/i915/i915_drv.c +1 -0
@@ -1724,6 +1724,7 @@ static struct drm_driver driver = { .prime_handle_to_fd = drm_gem_prime_handle_to_fd, .prime_fd_to_handle = drm_gem_prime_fd_to_handle, + .prime_page_flip = drm_gem_prime_page_flip, .gem_prime_export = i915_gem_prime_export, .gem_prime_import = i915_gem_prime_import,
drivers/gpu/drm/imx/imx-drm-core.c +1 -0
@@ -480,6 +480,7 @@ static struct drm_driver imx_drm_driver = { .prime_handle_to_fd = drm_gem_prime_handle_to_fd, .prime_fd_to_handle = drm_gem_prime_fd_to_handle, + .prime_page_flip = drm_gem_prime_page_flip, .gem_prime_import = drm_gem_prime_import, .gem_prime_export = drm_gem_prime_export, .gem_prime_get_sg_table = drm_gem_cma_prime_get_sg_table,
drivers/gpu/drm/msm/msm_drv.c +1 -0
@@ -988,6 +988,7 @@ static struct drm_driver msm_driver = { .dumb_destroy = drm_gem_dumb_destroy, .prime_handle_to_fd = drm_gem_prime_handle_to_fd, .prime_fd_to_handle = drm_gem_prime_fd_to_handle, + .prime_page_flip = drm_gem_prime_page_flip, .gem_prime_export = drm_gem_prime_export, .gem_prime_import = drm_gem_prime_import, .gem_prime_pin = msm_gem_prime_pin,
drivers/gpu/drm/nouveau/nouveau_drm.c +1 -0
@@ -946,6 +946,7 @@ driver_stub = { .prime_handle_to_fd = drm_gem_prime_handle_to_fd, .prime_fd_to_handle = drm_gem_prime_fd_to_handle, + .prime_page_flip = drm_gem_prime_page_flip, .gem_prime_export = drm_gem_prime_export, .gem_prime_import = drm_gem_prime_import, .gem_prime_pin = nouveau_gem_prime_pin,
drivers/gpu/drm/omapdrm/omap_drv.c +1 -0
@@ -848,6 +848,7 @@ static struct drm_driver omap_drm_driver = {#endif .prime_handle_to_fd = drm_gem_prime_handle_to_fd, .prime_fd_to_handle = drm_gem_prime_fd_to_handle, + .prime_page_flip = drm_gem_prime_page_flip, .gem_prime_export = omap_gem_prime_export, .gem_prime_import = omap_gem_prime_import, .gem_free_object = omap_gem_free_object,
drivers/gpu/drm/qxl/qxl_drv.c +1 -0
@@ -247,6 +247,7 @@ static struct drm_driver qxl_driver = {#endif .prime_handle_to_fd = drm_gem_prime_handle_to_fd, .prime_fd_to_handle = drm_gem_prime_fd_to_handle, + .prime_page_flip = drm_gem_prime_page_flip, .gem_prime_export = drm_gem_prime_export, .gem_prime_import = drm_gem_prime_import, .gem_prime_pin = qxl_gem_prime_pin,
drivers/gpu/drm/radeon/radeon_drv.c +1 -0
@@ -598,6 +598,7 @@ static struct drm_driver kms_driver = { .prime_handle_to_fd = drm_gem_prime_handle_to_fd, .prime_fd_to_handle = drm_gem_prime_fd_to_handle, + .prime_page_flip = drm_gem_prime_page_flip, .gem_prime_export = radeon_gem_prime_export, .gem_prime_import = drm_gem_prime_import, .gem_prime_pin = radeon_gem_prime_pin,
drivers/gpu/drm/rcar-du/rcar_du_drv.c +1 -0
@@ -256,6 +256,7 @@ static struct drm_driver rcar_du_driver = { .gem_vm_ops = &drm_gem_cma_vm_ops, .prime_handle_to_fd = drm_gem_prime_handle_to_fd, .prime_fd_to_handle = drm_gem_prime_fd_to_handle, + .prime_page_flip = drm_gem_prime_page_flip, .gem_prime_import = drm_gem_prime_import, .gem_prime_export = drm_gem_prime_export, .gem_prime_get_sg_table = drm_gem_cma_prime_get_sg_table,
drivers/gpu/drm/rockchip/rockchip_drm_drv.c +1 -0
@@ -290,6 +290,7 @@ static struct drm_driver rockchip_drm_driver = { .dumb_destroy = drm_gem_dumb_destroy, .prime_handle_to_fd = drm_gem_prime_handle_to_fd, .prime_fd_to_handle = drm_gem_prime_fd_to_handle, + .prime_page_flip = drm_gem_prime_page_flip, .gem_prime_import = drm_gem_prime_import, .gem_prime_export = drm_gem_prime_export, .gem_prime_get_sg_table = rockchip_gem_prime_get_sg_table,
drivers/gpu/drm/shmobile/shmob_drm_drv.c +1 -0
@@ -276,6 +276,7 @@ static struct drm_driver shmob_drm_driver = { .gem_vm_ops = &drm_gem_cma_vm_ops, .prime_handle_to_fd = drm_gem_prime_handle_to_fd, .prime_fd_to_handle = drm_gem_prime_fd_to_handle, + .prime_page_flip = drm_gem_prime_page_flip, .gem_prime_import = drm_gem_prime_import, .gem_prime_export = drm_gem_prime_export, .gem_prime_get_sg_table = drm_gem_cma_prime_get_sg_table,
drivers/gpu/drm/sti/sti_drv.c +1 -0
@@ -207,6 +207,7 @@ static struct drm_driver sti_driver = { .prime_handle_to_fd = drm_gem_prime_handle_to_fd, .prime_fd_to_handle = drm_gem_prime_fd_to_handle, + .prime_page_flip = drm_gem_prime_page_flip, .gem_prime_export = sti_gem_prime_export, .gem_prime_import = drm_gem_prime_import, .gem_prime_get_sg_table = drm_gem_cma_prime_get_sg_table,
drivers/gpu/drm/tegra/drm.c +1 -0
@@ -942,6 +942,7 @@ static struct drm_driver tegra_drm_driver = { .prime_handle_to_fd = drm_gem_prime_handle_to_fd, .prime_fd_to_handle = drm_gem_prime_fd_to_handle, + .prime_page_flip = drm_gem_prime_page_flip, .gem_prime_export = tegra_gem_prime_export, .gem_prime_import = tegra_gem_prime_import,
drivers/gpu/drm/udl/udl_drv.c +1 -0
@@ -53,6 +53,7 @@ static struct drm_driver driver = { .prime_handle_to_fd = drm_gem_prime_handle_to_fd, .prime_fd_to_handle = drm_gem_prime_fd_to_handle, + .prime_page_flip = drm_gem_prime_page_flip, .gem_prime_export = udl_gem_prime_export, .gem_prime_import = udl_gem_prime_import,
drivers/gpu/drm/vmwgfx/vmwgfx_drv.c +1 -0
@@ -1509,6 +1509,7 @@ static struct drm_driver driver = { .prime_fd_to_handle = vmw_prime_fd_to_handle, .prime_handle_to_fd = vmw_prime_handle_to_fd, + .prime_page_flip = drm_gem_prime_page_flip, .fops = &vmwgfx_driver_fops, .name = VMWGFX_DRIVER_NAME,
include/drm/drmP.h +5 -0
@@ -593,6 +593,11 @@ struct drm_driver {/* import dmabuf -> GEM */ struct drm_gem_object * (*gem_prime_import)(struct drm_device *dev,struct dma_buf *dma_buf); +/* schedule flip to dmabuf on fence signal +* (see drm_gem_prime_page_flip() helper) */ +int (*prime_page_flip)(struct drm_device *dev,struct drm_file *file_priv, +uint32_t handle,uint32_t fb_id,uint32_t crtc_id, +uint64_t user_data,uint32_t flags);/* low-level interface used by drm_gem_prime_{import,export} */ int (*gem_prime_pin)(struct drm_gem_object *obj);void (*gem_prime_unpin)(struct drm_gem_object *obj);
Signature · 6 lines
1.9.1 _______________________________________________ dri-devel mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/dri-devel
PATCH 6/6 drm: add DRM_IOCTL_PRIME_PAGE_FLIP
In reply to Alex GoinsFrom: agoins <[email protected]>
Adds DRM_IOCTL_PRIME_PAGE_FLIP, a new PRIME ioctl that uses DRM driver function prime_page_flip() to request a DRM page flip in response to an exclusive fence being signaled on a PRIME DMA-BUF's associated reservation object.
drm_internal.h:
Add declaration for drm_prime_page_flip_ioctl()
drm_ioctl.c:
DRM_IOCTL_DEF DRM_IOCTL_PRIME_PAGE_FLIP.
drm_prime.c:
Define drm_prime_page_flip_ioctl().
drm.h:
Define struct drm_prime_page_flip. Parameter struct from DRM_IOCTL_PRIME_PAGE_FLIP.
Define DRM_IOCTL_PRIME_PAGE_FLIP.
--- drivers/gpu/drm/drm_internal.h | 2 ++ drivers/gpu/drm/drm_ioctl.c | 1 + drivers/gpu/drm/drm_prime.c | 20 ++++++++++++++++++++ include/uapi/drm/drm.h | 10 ++++++++++ 4 files changed, 33 insertions(+)
drivers/gpu/drm/drm_internal.h +2 -0
@@ -45,6 +45,8 @@ int drm_prime_handle_to_fd_ioctl(struct drm_device *dev, void *data, struct drm_file *file_priv);int drm_prime_fd_to_handle_ioctl (struct drm_device *dev,void *data,struct drm_file *file_priv); +int drm_prime_page_flip_ioctl (struct drm_device *dev,void *data, +struct drm_file *file_priv);void drm_prime_init_file_private (struct drm_prime_file_private *prime_fpriv);void drm_prime_destroy_file_private (struct drm_prime_file_private *prime_fpriv);
drivers/gpu/drm/drm_ioctl.c +1 -0
@@ -632,6 +632,7 @@ static const struct drm_ioctl_desc drm_ioctls[] = {DRM_IOCTL_DEF (DRM_IOCTL_PRIME_HANDLE_TO_FD , drm_prime_handle_to_fd_ioctl,DRM_AUTH |DRM_UNLOCKED |DRM_RENDER_ALLOW ),DRM_IOCTL_DEF (DRM_IOCTL_PRIME_FD_TO_HANDLE , drm_prime_fd_to_handle_ioctl,DRM_AUTH |DRM_UNLOCKED |DRM_RENDER_ALLOW ), +DRM_IOCTL_DEF (DRM_IOCTL_PRIME_PAGE_FLIP , drm_prime_page_flip_ioctl,DRM_MASTER |DRM_AUTH |DRM_UNLOCKED |DRM_RENDER_ALLOW ),DRM_IOCTL_DEF (DRM_IOCTL_MODE_GETPLANERESOURCES , drm_mode_getplane_res,DRM_CONTROL_ALLOW |DRM_UNLOCKED ), DRM_IOCTL_DEF(DRM_IOCTL_MODE_GETCRTC, drm_mode_getcrtc, DRM_CONTROL_ALLOW|DRM_UNLOCKED),
drivers/gpu/drm/drm_prime.c +20 -0
@@ -827,6 +827,26 @@ out: }EXPORT_SYMBOL (drm_gem_prime_page_flip); +int drm_prime_page_flip_ioctl (struct drm_device *dev, +void *data,struct drm_file *file_priv) +{ +struct drm_prime_page_flip *args = data; + +if (!drm_core_check_feature (dev,DRIVER_PRIME )) +return -EINVAL ; + +if (!dev->driver->prime_page_flip) +return -ENOSYS ; + +/* check flags are valid */ +if (args->flags & ~DRM_PRIME_PAGE_FLIP_FLAGS ) +return -EINVAL ; + +return dev->driver->prime_page_flip (dev, file_priv, + args->handle, args->fb_id, args->crtc_id, + args->user_data, args->flags); +} +/** * drm_prime_pages_to_sg - converts a page array into an sg list * @pages: pointer to the array of page pointers to convert
include/uapi/drm/drm.h +10 -0
@@ -679,6 +679,15 @@ struct drm_prime_handle {__s32 fd; }; +#define DRM_PRIME_PAGE_FLIP_FLAGS DRM_MODE_PAGE_FLIP_FLAGS +struct drm_prime_page_flip { +__u32 handle; +__u32 fb_id; +__u32 crtc_id; +__u32 flags; +__u64 user_data; +}; +#include <drm/drm_mode.h> #define DRM_IOCTL_BASE 'd'
@@ -738,6 +747,7 @@ struct drm_prime_handle {#define DRM_IOCTL_PRIME_HANDLE_TO_FD DRM_IOWR(0x2d, struct drm_prime_handle)#define DRM_IOCTL_PRIME_FD_TO_HANDLE DRM_IOWR(0x2e, struct drm_prime_handle) +#define DRM_IOCTL_PRIME_PAGE_FLIP DRM_IOWR(0x2f, struct drm_prime_page_flip)#define DRM_IOCTL_AGP_ACQUIRE DRM_IO( 0x30)#define DRM_IOCTL_AGP_RELEASE DRM_IO( 0x31)
Signature · 6 lines
1.9.1 _______________________________________________ dri-devel mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH drm 4/6] drm: add drm_gem_prime_page_flip() helper
In reply to Alex GoinsQuoted 131 lines · Alex Goins
From: agoins <[email protected]>
Adds
drm_gem_prime_page_flip(), a helper implementation ofprime_page_flip()to be used by DRM drivers.Signed-off-by Alex Goins <[email protected]>--- drivers/gpu/drm/drm_prime.c | 147 ++++++++++++++++++++++++++++++++++++++++++++ include/drm/drmP.h | 3 + 2 files changed, 150 insertions(+)drivers/gpu/drm/drm_prime.c +91
-0@@ -61,6 +61,11 @@* use the drm_gem_prime_{import,export} helpers. */ +struct drm_prime_fence { +struct fence base; +spinlock_t lock; +}; +struct drm_prime_fence_ctx {uint32_t context;uint32_t seqno;@@ -78,6 +83,16 @@ struct drm_prime_attachment {enum dma_data_direction dir; }; +struct drm_gem_prime_page_flip_cb { +struct fence_cb base; +struct drm_device *dev; +uint32_t crtc_id; +uint32_t fb_id; +uint32_t flags; +uint64_t user_data; +struct drm_file *file_priv; +}; +static int drm_prime_add_buf_handle (struct drm_prime_file_private *prime_fpriv,struct dma_buf *dma_buf,uint32_t handle) {@@ -316,6 +331,28 @@ static const struct dma_buf_ops drm_gem_prime_dmabuf_ops = { .vunmap = drm_gem_dmabuf_vunmap, }; +static const char *drm_gem_prime_get_driver_name (struct fence *fence) +{ +return "PRIME" ; +} + +static const char *drm_gem_prime_get_timeline_name (struct fence *fence) +{ +return "prime.swonly" ; +} + +static bool drm_gem_prime_enable_signaling (struct fence *fence) +{ +return true;/* SW signaling only */ +} + +static const struct fence_ops drm_gem_prime_fence_ops = { + .get_driver_name = drm_gem_prime_get_driver_name, + .get_timeline_name = drm_gem_prime_get_timeline_name, + .enable_signaling = drm_gem_prime_enable_signaling, + .wait = fence_default_wait, +}; +/** * DOC: PRIME Helpers * @@ -680,6 +717,116 @@ int drm_prime_fd_to_handle_ioctl(struct drm_device *dev, void *data, args->fd, &args->handle); } +/* Callback used by drm_gem_prime_page_flip(). Is added to a fence such that +* when the fence is signaled, page flip is requested with given parameters. */ +static void drm_gem_prime_page_flip_cb (struct fence *fence, +struct fence_cb *cb) +{ +struct drm_gem_prime_page_flip_cb *page_flip_cb = + container_of(cb, struct drm_gem_prime_page_flip_cb, base); + +drm_mode_page_flip (page_flip_cb->dev, + page_flip_cb->crtc_id, + page_flip_cb->fb_id, + page_flip_cb->flags, + page_flip_cb->user_data, + page_flip_cb->file_priv); + +kfree (cb); +} + +/** +* drm_gem_prime_page_flip - +* helper library implementation of the page flip callback +* +* @dev: DRM device +* @file_priv: DRM file info +* @handle: buffer handle to fence +* @fb_id: FB to update to +* @crtc_id: CRTC to update +* @user_data: Returned as user_data field in in the vblank event struct +* @flags: Flags modifying page flip behavior, same as drm_mode_page_flip. +* +* This is the implementation of the prime_page_flip function for GEM drivers +* using the PRIME helpers. +*/ +int drm_gem_prime_page_flip (struct drm_device *dev, +struct drm_file *file_priv,uint32_t handle, +uint32_t fb_id,uint32_t crtc_id, +uint64_t user_data,uint32_t flags) +{ +int ret; + +struct dma_buf *dmabuf; +struct fence *oldfence; +struct drm_prime_fence *fence; +struct drm_gem_prime_page_flip_cb *page_flip_cb; +struct drm_prime_member *member; +struct drm_prime_fence_ctx *fctx; + +mutex_lock (&file_priv->prime.lock); + + member =drm_prime_lookup_member_by_handle (&file_priv->prime, + handle); +if (!member) { +/* Buffer is not a member of PRIME */ + ret = -EINVAL ;
drivers/gpu/drm/drm_prime.c int drm_prime_fd_to_handle_ioctl(struct drm_device *dev, void *data, · line 783
Quoted 10 lines
drivers/gpu/drm/drm_prime.c +10
-0+ goto out; + } + + dmabuf = member->dma_buf; + fctx = &member->fctx; + +/* Clear out old fence callbacks */ + oldfence =reservation_object_get_excl (dmabuf->resv); +if (oldfence) +fence_signal (oldfence);
This looks really strange - you force-complete the fence already attached (which might be from any driver attached to this dma-buf)?
The creator of the fence is supposed to complete it when whatever async operation that is scheduled and which is using this dma-buf is done. So the way to go about this here is to take out the excl fence from the reservation object, and wait for it to complete.
Maarten Lankhorst had patches to do that for i915, but they just didn't go anywhere since i915 gem locking is a bit ... antique. But if your goal is to only fix up the page_flip path, and only for i915 as the display driver, then the only thing you need to do is:
- grab the reservation of the gem bo underlying the fb
- grab the exclusive fence (fence_get)
- wait for that fence to complete in the async worker (we already have the mmio_flip stuff for when you do the flip from the cpu)
- drop the fence once done with fence_put
Works as long as the producer correctly sets the exclusive fence (doesn't matter whether that's done with an explicit ioctl or at command submission). And that's already done by nouveau (well ttm) afaik.
No changes at all in userspace required.
Now if you want to do proper explicit fencing for kms, that's an entirely different story. For that we want something like android's hw composer in/out fences, and obviously for atomic (because interface extensions for legacy modeset ioctls just don't make sense). But that means we need the full stack, using open-source userspace, and that's a lot more pain.
For your limited use-case it really would be simplest to fix up the i915 mmio flip code to handle fences attached to dma-bufs, and teach the nvidia blob to attach a suitable exclusive fence somehow.
Wrt merge coordination: RSN (well that's the case since months) all the legacy pageflip code will be ripped out from i915 and replaced by async atomic updates). So except for a proof-of-concept I'd prefer if we just implement this in the atomic codepaths directly.
Cheers, Daniel
Quoted 70 lines
drivers/gpu/drm/drm_prime.c +46
-0+ +/* Create and initialize new fence */ + fence =kmalloc (sizeof (*fence),GFP_KERNEL ); +if (!fence) { + ret = -ENOMEM ; + goto out; + } + +spin_lock_init (&fence->lock); +fence_init (&fence->base, &drm_gem_prime_fence_ops, &fence->lock, + fctx->context, fctx->seqno++); + +/* Create and initialize new page flip callback */ + page_flip_cb =kmalloc (sizeof (*page_flip_cb),GFP_KERNEL ); +if (!page_flip_cb) { + ret = -ENOMEM ; +fence_free (&fence->base); + goto out; + } + + *page_flip_cb = (struct drm_gem_prime_page_flip_cb ) { + .dev = dev, + .crtc_id = crtc_id, + .fb_id = fb_id, + .flags = flags, + .user_data = user_data, + .file_priv = file_priv, + }; + + ret =fence_add_callback (&fence->base, &page_flip_cb->base, + drm_gem_prime_page_flip_cb); +if (ret) { +fence_free (&fence->base); +kfree (page_flip_cb); + goto out; + } + +/* Add new fence */ +reservation_object_add_excl_fence (dmabuf->resv, &fence->base); +fence_put (&fence->base);/* Reservation object has reference */ +out: +mutex_unlock (&file_priv->prime.lock); +return ret; +} +EXPORT_SYMBOL (drm_gem_prime_page_flip); +/** * drm_prime_pages_to_sg - converts a page array into an sg list * @pages: pointer to the array of page pointers to convert include/drm/drmP.h +3
-0@@ -1028,6 +1028,9 @@ extern struct drm_gem_object *drm_gem_prime_import(struct drm_device *dev, struct dma_buf *dma_buf);extern int drm_gem_prime_fd_to_handle (struct drm_device *dev,struct drm_file *file_priv,int prime_fd,uint32_t *handle); +extern int drm_gem_prime_page_flip (struct drm_device *dev, +struct drm_file *file_priv,uint32_t handle,uint32_t fb_id, +uint32_t crtc_id,uint64_t user_data,uint32_t flags);extern void drm_gem_dmabuf_release (struct dma_buf *dma_buf); extern int drm_prime_sg_to_page_addr_arrays(struct sg_table *sgt, struct page **pages,Signature · 6 lines
1.9.1 _______________________________________________ dri-devel mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/dri-devel
Signature · 7 lines
Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch _______________________________________________ dri-devel mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/dri-devel
RE: [PATCH drm 4/6] drm: add drm_gem_prime_page_flip() helper
Thanks for the quick feedback, Daniel!
Quoted 2 lines
This looks really strange - you force-complete the fence already attached (which might be from any driver attached to this dma-buf)?
The creator of the fence is supposed to complete it when whatever async operation that is scheduled and which is using this dma-buf is done. So the way to go about this here is to take out the excl fence from the reservation object, and wait for it to complete.
Yeah, true. I had just added that as a contingency - in reality this ioctl is never called (by my changes) on a buffer with an unsignaled fence. You're right that a wait would be better. I'll have it changed the revised patch set if we end up keeping the ioctl.
Quoted 1 line
But if your goal is to only fix up the
page_flippath, and only for i915 as the display driver
I'd be fine with the atomic modesetting path if i915 supported async atomic updates; really I just need a way to fence flips between two shared buffers. While i915 is the main target, we'd ideally like to support this with other devices/drivers as well.
Quoted 1 line
For your limited use-case it really would be simplest to fix up the i915 mmio flip code to handle fences attached to dma-bufs, and teach the nvidia blob to attach a suitable exclusive fence somehow.
This sounds like a pretty good solution - as long as the semantics stay the same as to who attaches the fence, transitioning from this to an atomic modesetting solution could be relatively painless. I'd need a way to query if a driver supports fencing on page_flip and/or async atomic modesetting in order to get graceful fallback on X.
Quoted 1 line
Wrt merge coordination: RSN (well that's the case since months) all the legacy pageflip code will be ripped out from i915 and replaced by async atomic updates). So except for a proof-of-concept I'd prefer if we just implement this in the atomic codepaths directly.
What kind of timeline are we looking at for i915 supporting async atomic updates? Will the legacy pageflip code be ripped out at the same time, or will there be a transition period? With the ability to query for fenced page_flip / fenced async atomic modesetting, I can probably get X to use the best available method. I should be able to test a fenced async atomic update implementation using dGPU + Tegra PRIME prior to i915 supporting it, but I'd be interested to hear from those more well versed on that path as to how they'd like it implemented.
Thanks,
Alex
_______________________________________________
dri-devel mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH drm 4/6] drm: add drm_gem_prime_page_flip() helper
Quoted 4 lines · Alexander Goins
Quoted 2 lines
This looks really strange - you force-complete the fence already attached (which might be from any driver attached to this dma-buf)?
The creator of the fence is supposed to complete it when whatever async operation that is scheduled and which is using this dma-buf is done. So the way to go about this here is to take out the excl fence from the reservation object, and wait for it to complete.Yeah, true. I had just added that as a contingency - in reality this ioctl is never called (by my changes) on a buffer with an unsignaled fence. You're right that a wait would be better. I'll have it changed the revised patch set if we end up keeping the ioctl.
It's not just that you should wait on the fence, there's no reason for a page_flip to attach an exclusive fence at all - display block at most reads from the buffer, we don't need to block anyone from reading (e.g. to do a front->back copy before rendering the next frame).
Quoted 3 lines
Quoted 1 line
But if your goal is to only fix up the
page_flippath, and only for i915 as the display driverI'd be fine with the atomic modesetting path if i915 supported async atomic updates; really I just need a way to fence flips between two shared buffers. While i915 is the main target, we'd ideally like to support this with other devices/drivers as well.
Every other driver supporting dma-buf import of foreign objects should just do the same. The only case you care about is probably tegra. Note that atomic helpers already are prepared for both implicit and explicit fences, there's plane_state->fence which they wait to complete on. The idea is to fill this out in your plane->prepare_fb hook.
Quoted 3 lines
Quoted 1 line
For your limited use-case it really would be simplest to fix up the i915 mmio flip code to handle fences attached to dma-bufs, and teach the nvidia blob to attach a suitable exclusive fence somehow.
This sounds like a pretty good solution - as long as the semantics stay the same as to who attaches the fence, transitioning from this to an atomic modesetting solution could be relatively painless. I'd need a way to query if a driver supports fencing on
page_flipand/or async atomic modesetting in order to get graceful fallback on X.
Feature flags for kernel bugs are kinda not something I like all that much. If you _really_ want it, we'll just backport the minimal fix and that's it. Otherwise I gues carry a list of drivers/kernels that work/dont in your X driver.
Quoted 3 lines
Quoted 1 line
Wrt merge coordination: RSN (well that's the case since months) all the legacy pageflip code will be ripped out from i915 and replaced by async atomic updates). So except for a proof-of-concept I'd prefer if we just implement this in the atomic codepaths directly.
What kind of timeline are we looking at for i915 supporting async atomic updates? Will the legacy pageflip code be ripped out at the same time, or will there be a transition period? With the ability to query for fenced
page_flip/ fenced async atomic modesetting, I can probably get X to use the best available method. I should be able to test a fenced async atomic update implementation using dGPU + Tegra PRIME prior to i915 supporting it, but I'd be interested to hear from those more well versed on that path as to how they'd like it implemented.
Legacy flip code will be ripped out as soon as async atomic works, and implemented using atomic modeset code using the usual helpers. But since it's a giantic task I don't want to add more features to the legacy flip code, that'll only make it slower to get to the goal. At least if you don't also implement the atomic side. So two patches:
1. for the mmio_flip code, which we could also maybe backport.
2. for the new atomic code, for future-proofing.
Both should be really minimal patches.
Cheers, Daniel
Signature · 7 lines
Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch _______________________________________________ dri-devel mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH drm 4/6] drm: add drm_gem_prime_page_flip() helper
In reply to Daniel VetterHi,
Quoted 10 lines · Daniel Vetter
Maarten Lankhorst had patches to do that for i915, but they just didn't go anywhere since i915 gem locking is a bit ... antique. But if your goal is to only fix up the
page_flippath, and only for i915 as the display driver, then the only thing you need to do is:
- grab the reservation of the gem bo underlying the fb
- grab the exclusive fence (fence_get)
- wait for that fence to complete in the async worker (we already have themmio_flipstuff for when you do the flip from the cpu)
- drop the fence once done withfence_put
Quoted 5 lines
Works as long as the producer correctly sets the exclusive fence (doesn't matter whether that's done with an explicit ioctl or at command submission). And that's already done by nouveau (well ttm) afaik.
No changes at all in userspace required.
Totally agree. For implicit sync, this is also how we (and ChromeOS) do it on ARM, e.g.:
https://git.collabora.com/cgit/user/daniels/linux.git/tree/drivers/gpu/drm/exynos/exynos_drm_fb.c?h=tmp/lfrb#n288
This was never upstreamed as the users (Exynos, Rockchip, et al) depended on Mali, but it does offer a perfect implementation of implicit synchronisation between multiple separate devices when you place reservation objects.
Quoted 5 lines
Now if you want to do proper explicit fencing for kms, that's an entirely different story. For that we want something like android's hw composer in/out fences, and obviously for atomic (because interface extensions for legacy modeset ioctls just don't make sense). But that means we need the full stack, using open-source userspace, and that's a lot more pain.
Totally agreed. New legacy API is pretty much a no-go - especially given the painful semantics they force you to implement - and wouldn't take any less time to roll out through the kernel and userspace, including working on tests, than it would to wait for Maarten Lankhorst's existing async-atomic work to land.
Cheers,
Daniel
_______________________________________________
dri-devel mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/dri-devel