From: Gustavo Padovan gustavo.padovan@collabora.co.uk
Hi,
Currently the Linux Kernel only have an implicit fencing mechanism where the fence are attached directly to buffers and userspace is unaware of the inner kernel workings. However by enabling explicit fencing, where fences travels all the way up the userspace we can provide insightful information to compositors so smarter decisions when scheduling framebuffers.
For that we adapted the Android Sync Framework[1], a explicit fencing mechanism that helps the userspace handles fences directly. It has the concept of sync_file (called sync_fence in Android) that expose the driver's fences to userspace via file descriptors, than can then be shared between process.
With explicit fencing we have a global mechanism that optimizes the flow of buffers between consumers and producers, avoid a lot of waiting. So instead of waiting for a buffer to be processed by the GPU before sending it to DRM/KMS in an Atomic IOCTL we can get a sync_file fd from the GPU driver at the moment we submit the buffer processing. The compositor then passes these fds to DRM/KMS in a Atomic Commit request, that will not be displayed until the fences signal, i.e, the GPU finished processing the buffer and it is ready to be displayed.
In DRM/KMS the fences we wait on before displaying a buffer are called *in-fences*. Vice-versa, we have *out-fences*, to sychronize the return of buffers to GPU (producer) to be processed again. When DRM/KMS receives an atomic request with a special flag set it generates one fence per-crtc and attach it to a per-crtc sync_file. It then returns the array of sync_file fds to userspace as an atomic_ioctl out arg. With the fences available userspace can forward these fences to the GPU, where it will wait the fence to signal before starting to process on the shared buffer again.
Explicit fencing with the Sync Framework provides better traceability and debuggabilty of the kernel and allows efficient buffer suballocation by Mesa/Vulkan.
DRM/KMS explicit fences are opt-in, as the default will still be implicit fencing. To enable explicit in-fences one just need to pass a sync_file fd in the FENCE_FD plane property. *In-fences are per-plane*, i.e., per framebuffer.
For out-fences, just enabling DRM_MODE_ATOMIC_OUT_FENCE flag is enough. *Out-fences are per-crtc*.
In-fences ---------
In the first discussions on #dri-devel on IRC we decided to hide the Sync Framework from DRM drivers to reduce complexity, so as soon we get the fd via FENCE_FD plane property we convert the sync_file fd to a struct fence.
Then we just use the already in place fence support to wait on those fences. Once the producer calls fence_signal() for all fences on wait we can proceed with the atomic commit and scanout the framebuffers.
Out-fences ----------
Passing the DRM_MODE_ATOMIC_OUT_FENCE flag to an atomic request enables out-fences. The kernel then creates a fence, attach it to a sync_file and install this file on a unused fd. The process is repeated for each crtc. Userspace get the fence back as an array of per-crtc sync_file fds.
DRM core use the already in place drm_event infrastructure to help signal fences, we've added a fence pointer to struct drm_pending_event. On vblank we just call fence_signal() to signal that the buffer related to this fence is *now* on the screen. Note that this is exactly the opposite behaviour from Android, where the fences are signaled when they are not on display anymore, so free to be reused.
No changes are required to DRM drivers to have out-fences support, apart from atomic support of course.
Kernel tree -----------
For those who want all patches on this RFC are in my tree. The tree includes a few other patches necessary to run it, like the interruptible wait_for_fences patch that I sent last week to the mainling list:
https://git.kernel.org/cgit/linux/kernel/git/padovan/linux.git/log/?h=fences
Testing -------
For testing it Rob Clark add fences support to mesa and kmscube, Rob has patch for the Mesa core native fence work and freedeno and I have added support of virgl as well:
git://github.com/freedreno/libdrm.git fences
git://git.collabora.com/git/user/padovan/mesa.git fences
git://github.com/robclark/kmscube.git atomic-fence
Changes in RFC v4 -----------------
Rebased against latest drm-misc
Regards,
Gustavo ---
[1] https://source.android.com/devices/graphics/implement.html#vsync
Gustavo Padovan (3): drm/fence: add in-fences support drm/fence: add fence timeline to drm_crtc drm/fence: add out-fences support
drivers/gpu/drm/Kconfig | 1 + drivers/gpu/drm/drm_atomic.c | 233 ++++++++++++++++++++++++++++++++---- drivers/gpu/drm/drm_atomic_helper.c | 3 + drivers/gpu/drm/drm_crtc.c | 38 ++++++ include/drm/drm_crtc.h | 33 +++++ include/uapi/drm/drm_mode.h | 15 ++- 6 files changed, 301 insertions(+), 22 deletions(-)
From: Gustavo Padovan gustavo.padovan@collabora.co.uk
There is now a new property called FENCE_FD attached to every plane state that receives the sync_file fd from userspace via the atomic commit IOCTL.
The fd is then translated to a fence (that may be a fence_collection subclass or just a normal fence) and then used by DRM to fence_wait() for all fences in the sync_file to signal. So it only commits when all framebuffers are ready to scanout.
v2: Comments by Daniel Vetter: - remove set state->fence = NULL in destroy phase - accept fence -1 as valid and just return 0 - do not call fence_get() - sync_file_fences_get() already calls it - fence_put() if state->fence is already set, in case userspace set the property more than once.
v3: WARN_ON if fence is set but state has no FB
Signed-off-by: Gustavo Padovan gustavo.padovan@collabora.co.uk --- drivers/gpu/drm/Kconfig | 1 + drivers/gpu/drm/drm_atomic.c | 19 +++++++++++++++++++ drivers/gpu/drm/drm_atomic_helper.c | 3 +++ drivers/gpu/drm/drm_crtc.c | 7 +++++++ include/drm/drm_crtc.h | 4 ++++ 5 files changed, 34 insertions(+)
diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig index c02be6a..07f9c60 100644 --- a/drivers/gpu/drm/Kconfig +++ b/drivers/gpu/drm/Kconfig @@ -12,6 +12,7 @@ menuconfig DRM select I2C select I2C_ALGOBIT select DMA_SHARED_BUFFER + select SYNC_FILE help Kernel-level support for the Direct Rendering Infrastructure (DRI) introduced in XFree86 4.0. If you say Y here, you need to select diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c index 5cb2e22..9e6c4e7 100644 --- a/drivers/gpu/drm/drm_atomic.c +++ b/drivers/gpu/drm/drm_atomic.c @@ -30,6 +30,7 @@ #include <drm/drm_atomic.h> #include <drm/drm_mode.h> #include <drm/drm_plane_helper.h> +#include <linux/sync_file.h>
#include "drm_crtc_internal.h"
@@ -690,6 +691,17 @@ int drm_atomic_plane_set_property(struct drm_plane *plane, drm_atomic_set_fb_for_plane(state, fb); if (fb) drm_framebuffer_unreference(fb); + } else if (property == config->prop_fence_fd) { + if (U642I64(val) == -1) + return 0; + + if (state->fence) + fence_put(state->fence); + + state->fence = sync_file_get_fence(val); + if (!state->fence) + return -EINVAL; + } else if (property == config->prop_crtc_id) { struct drm_crtc *crtc = drm_crtc_find(dev, val); return drm_atomic_set_crtc_for_plane(state, crtc); @@ -749,6 +761,8 @@ drm_atomic_plane_get_property(struct drm_plane *plane,
if (property == config->prop_fb_id) { *val = (state->fb) ? state->fb->base.id : 0; + } else if (property == config->prop_fence_fd) { + *val = -1; } else if (property == config->prop_crtc_id) { *val = (state->crtc) ? state->crtc->base.id : 0; } else if (property == config->prop_crtc_x) { @@ -824,6 +838,11 @@ static int drm_atomic_plane_check(struct drm_plane *plane, return -EINVAL; }
+ if (WARN_ON(state->fence && !state->fb)) { + DRM_DEBUG_ATOMIC("in-fence set but no FB\n"); + return -EINVAL; + } + /* if disabled, we don't care about the rest of the state: */ if (!state->crtc) return 0; diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c index dff2389..f817452 100644 --- a/drivers/gpu/drm/drm_atomic_helper.c +++ b/drivers/gpu/drm/drm_atomic_helper.c @@ -3156,6 +3156,9 @@ void __drm_atomic_helper_plane_destroy_state(struct drm_plane_state *state) { if (state->fb) drm_framebuffer_unreference(state->fb); + + if (state->fence) + fence_put(state->fence); } EXPORT_SYMBOL(__drm_atomic_helper_plane_destroy_state);
diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c index b95c48ac..6eaeb73 100644 --- a/drivers/gpu/drm/drm_crtc.c +++ b/drivers/gpu/drm/drm_crtc.c @@ -338,6 +338,7 @@ int drm_universal_plane_init(struct drm_device *dev, struct drm_plane *plane,
if (drm_core_check_feature(dev, DRIVER_ATOMIC)) { drm_object_attach_property(&plane->base, config->prop_fb_id, 0); + drm_object_attach_property(&plane->base, config->prop_fence_fd, -1); drm_object_attach_property(&plane->base, config->prop_crtc_id, 0); drm_object_attach_property(&plane->base, config->prop_crtc_x, 0); drm_object_attach_property(&plane->base, config->prop_crtc_y, 0); @@ -610,6 +611,12 @@ static int drm_mode_create_standard_properties(struct drm_device *dev) return -ENOMEM; dev->mode_config.prop_fb_id = prop;
+ prop = drm_property_create_signed_range(dev, DRM_MODE_PROP_ATOMIC, + "FENCE_FD", -1, INT_MAX); + if (!prop) + return -ENOMEM; + dev->mode_config.prop_fence_fd = prop; + prop = drm_property_create_object(dev, DRM_MODE_PROP_ATOMIC, "CRTC_ID", DRM_MODE_OBJECT_CRTC); if (!prop) diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h index 3d33c90..2dc7e79 100644 --- a/include/drm/drm_crtc.h +++ b/include/drm/drm_crtc.h @@ -1809,6 +1809,10 @@ struct drm_mode_config { */ struct drm_property *prop_fb_id; /** + * @prop_fence_fd: Sync File fd reprenseting the fences for this plane + */ + struct drm_property *prop_fence_fd; + /** * @prop_crtc_id: Default atomic plane property to specify the * &drm_crtc. */
Op 31-08-16 om 21:07 schreef Gustavo Padovan:
From: Gustavo Padovan gustavo.padovan@collabora.co.uk
There is now a new property called FENCE_FD attached to every plane state that receives the sync_file fd from userspace via the atomic commit IOCTL.
The fd is then translated to a fence (that may be a fence_collection subclass or just a normal fence) and then used by DRM to fence_wait() for all fences in the sync_file to signal. So it only commits when all framebuffers are ready to scanout.
v2: Comments by Daniel Vetter:
- remove set state->fence = NULL in destroy phase
- accept fence -1 as valid and just return 0
- do not call fence_get() - sync_file_fences_get() already calls it
- fence_put() if state->fence is already set, in case userspace
set the property more than once.
v3: WARN_ON if fence is set but state has no FB
Signed-off-by: Gustavo Padovan gustavo.padovan@collabora.co.uk
drivers/gpu/drm/Kconfig | 1 + drivers/gpu/drm/drm_atomic.c | 19 +++++++++++++++++++ drivers/gpu/drm/drm_atomic_helper.c | 3 +++ drivers/gpu/drm/drm_crtc.c | 7 +++++++ include/drm/drm_crtc.h | 4 ++++ 5 files changed, 34 insertions(+)
diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig index c02be6a..07f9c60 100644 --- a/drivers/gpu/drm/Kconfig +++ b/drivers/gpu/drm/Kconfig @@ -12,6 +12,7 @@ menuconfig DRM select I2C select I2C_ALGOBIT select DMA_SHARED_BUFFER
- select SYNC_FILE help Kernel-level support for the Direct Rendering Infrastructure (DRI) introduced in XFree86 4.0. If you say Y here, you need to select
diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c index 5cb2e22..9e6c4e7 100644 --- a/drivers/gpu/drm/drm_atomic.c +++ b/drivers/gpu/drm/drm_atomic.c @@ -30,6 +30,7 @@ #include <drm/drm_atomic.h> #include <drm/drm_mode.h> #include <drm/drm_plane_helper.h> +#include <linux/sync_file.h>
#include "drm_crtc_internal.h"
@@ -690,6 +691,17 @@ int drm_atomic_plane_set_property(struct drm_plane *plane, drm_atomic_set_fb_for_plane(state, fb); if (fb) drm_framebuffer_unreference(fb);
- } else if (property == config->prop_fence_fd) {
if (U642I64(val) == -1)return 0;if (state->fence)fence_put(state->fence);state->fence = sync_file_get_fence(val);if (!state->fence)return -EINVAL;@@ -749,6 +761,8 @@ drm_atomic_plane_get_property(struct drm_plane *plane,
if (property == config->prop_fb_id) { *val = (state->fb) ? state->fb->base.id : 0;
- } else if (property == config->prop_fence_fd) {
*val = -1;@@ -824,6 +838,11 @@ static int drm_atomic_plane_check(struct drm_plane *plane, return -EINVAL; }
- if (WARN_ON(state->fence && !state->fb)) {
DRM_DEBUG_ATOMIC("in-fence set but no FB\n");return -EINVAL;- }
Why is this a error? Could be useful to fence a modeset disable.
It might make more sense to put the fence inside the crtc state, not the plane state. Updates are done per crtc and moving planes between multiple crtc's inside a single commit is not allowed. I'd like to know what others think of that.
I'm not sure this patch is tested, looks like plane_duplicate_state is not clearing ->fence, probably resulting in WARNs.
diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c index dff2389..f817452 100644 --- a/drivers/gpu/drm/drm_atomic_helper.c +++ b/drivers/gpu/drm/drm_atomic_helper.c @@ -3156,6 +3156,9 @@ void __drm_atomic_helper_plane_destroy_state(struct drm_plane_state *state) { if (state->fb) drm_framebuffer_unreference(state->fb);
- if (state->fence)
fence_put(state->fence);} EXPORT_SYMBOL(__drm_atomic_helper_plane_destroy_state);
diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c index b95c48ac..6eaeb73 100644 --- a/drivers/gpu/drm/drm_crtc.c +++ b/drivers/gpu/drm/drm_crtc.c @@ -338,6 +338,7 @@ int drm_universal_plane_init(struct drm_device *dev, struct drm_plane *plane,
if (drm_core_check_feature(dev, DRIVER_ATOMIC)) { drm_object_attach_property(&plane->base, config->prop_fb_id, 0);
drm_object_attach_property(&plane->base, config->prop_fence_fd, -1);@@ -610,6 +611,12 @@ static int drm_mode_create_standard_properties(struct drm_device *dev) return -ENOMEM; dev->mode_config.prop_fb_id = prop;
- prop = drm_property_create_signed_range(dev, DRM_MODE_PROP_ATOMIC,
"FENCE_FD", -1, INT_MAX);- if (!prop)
return -ENOMEM;- dev->mode_config.prop_fence_fd = prop;
- prop = drm_property_create_object(dev, DRM_MODE_PROP_ATOMIC, "CRTC_ID", DRM_MODE_OBJECT_CRTC); if (!prop)
diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h index 3d33c90..2dc7e79 100644 --- a/include/drm/drm_crtc.h +++ b/include/drm/drm_crtc.h @@ -1809,6 +1809,10 @@ struct drm_mode_config { */ struct drm_property *prop_fb_id; /**
* @prop_fence_fd: Sync File fd reprenseting the fences for this plane*/- struct drm_property *prop_fence_fd;
- /**
*/
- @prop_crtc_id: Default atomic plane property to specify the
- &drm_crtc.
Hi Maarten,
2016-09-01 Maarten Lankhorst maarten.lankhorst@linux.intel.com:
Op 31-08-16 om 21:07 schreef Gustavo Padovan:
From: Gustavo Padovan gustavo.padovan@collabora.co.uk
There is now a new property called FENCE_FD attached to every plane state that receives the sync_file fd from userspace via the atomic commit IOCTL.
The fd is then translated to a fence (that may be a fence_collection subclass or just a normal fence) and then used by DRM to fence_wait() for all fences in the sync_file to signal. So it only commits when all framebuffers are ready to scanout.
v2: Comments by Daniel Vetter:
- remove set state->fence = NULL in destroy phase
- accept fence -1 as valid and just return 0
- do not call fence_get() - sync_file_fences_get() already calls it
- fence_put() if state->fence is already set, in case userspace
set the property more than once.
v3: WARN_ON if fence is set but state has no FB
Signed-off-by: Gustavo Padovan gustavo.padovan@collabora.co.uk
drivers/gpu/drm/Kconfig | 1 + drivers/gpu/drm/drm_atomic.c | 19 +++++++++++++++++++ drivers/gpu/drm/drm_atomic_helper.c | 3 +++ drivers/gpu/drm/drm_crtc.c | 7 +++++++ include/drm/drm_crtc.h | 4 ++++ 5 files changed, 34 insertions(+)
diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig index c02be6a..07f9c60 100644 --- a/drivers/gpu/drm/Kconfig +++ b/drivers/gpu/drm/Kconfig @@ -12,6 +12,7 @@ menuconfig DRM select I2C select I2C_ALGOBIT select DMA_SHARED_BUFFER
- select SYNC_FILE help Kernel-level support for the Direct Rendering Infrastructure (DRI) introduced in XFree86 4.0. If you say Y here, you need to select
diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c index 5cb2e22..9e6c4e7 100644 --- a/drivers/gpu/drm/drm_atomic.c +++ b/drivers/gpu/drm/drm_atomic.c @@ -30,6 +30,7 @@ #include <drm/drm_atomic.h> #include <drm/drm_mode.h> #include <drm/drm_plane_helper.h> +#include <linux/sync_file.h>
#include "drm_crtc_internal.h"
@@ -690,6 +691,17 @@ int drm_atomic_plane_set_property(struct drm_plane *plane, drm_atomic_set_fb_for_plane(state, fb); if (fb) drm_framebuffer_unreference(fb);
- } else if (property == config->prop_fence_fd) {
if (U642I64(val) == -1)return 0;if (state->fence)fence_put(state->fence);state->fence = sync_file_get_fence(val);if (!state->fence)return -EINVAL;@@ -749,6 +761,8 @@ drm_atomic_plane_get_property(struct drm_plane *plane,
if (property == config->prop_fb_id) { *val = (state->fb) ? state->fb->base.id : 0;
- } else if (property == config->prop_fence_fd) {
*val = -1;@@ -824,6 +838,11 @@ static int drm_atomic_plane_check(struct drm_plane *plane, return -EINVAL; }
- if (WARN_ON(state->fence && !state->fb)) {
DRM_DEBUG_ATOMIC("in-fence set but no FB\n");return -EINVAL;- }
Why is this a error? Could be useful to fence a modeset disable.
Yes. I didn't envision that use case. I'll change that for the next version.
It might make more sense to put the fence inside the crtc state, not the plane state. Updates are done per crtc and moving planes between multiple crtc's inside a single commit is not allowed. I'd like to know what others think of that.
I'm not sure this patch is tested, looks like plane_duplicate_state is not clearing ->fence, probably resulting in WARNs.
It is tested, but I'n not seeing any warning there. Which WARNs are you talking about? And why do we need to clear fence there? we don't clean anything else on duplicate.
Gustavo
From: Gustavo Padovan gustavo.padovan@collabora.co.uk
Create one timeline context for each CRTC to be able to handle out-fences and signal them. It adds a few members to struct drm_crtc: fence_context, where we store the context we get from fence_context_alloc(), the fence seqno and the fence lock, that we pass in fence_init() to be used by the fence.
v2: Comment by Daniel Stone: - add BUG_ON() to fence_to_crtc() macro
v3: Comment by Ville Syrjälä - Use more meaningful name as crtc timeline name
Signed-off-by: Gustavo Padovan gustavo.padovan@collabora.co.uk --- drivers/gpu/drm/drm_crtc.c | 31 +++++++++++++++++++++++++++++++ include/drm/drm_crtc.h | 19 +++++++++++++++++++ 2 files changed, 50 insertions(+)
diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c index 6eaeb73..28e49a5 100644 --- a/drivers/gpu/drm/drm_crtc.c +++ b/drivers/gpu/drm/drm_crtc.c @@ -141,6 +141,32 @@ static void drm_crtc_unregister_all(struct drm_device *dev) } }
+static const char *drm_crtc_fence_get_driver_name(struct fence *fence) +{ + struct drm_crtc *crtc = fence_to_crtc(fence); + + return crtc->dev->driver->name; +} + +static const char *drm_crtc_fence_get_timeline_name(struct fence *fence) +{ + struct drm_crtc *crtc = fence_to_crtc(fence); + + return crtc->timeline_name; +} + +static bool drm_crtc_fence_enable_signaling(struct fence *fence) +{ + return true; +} + +const struct fence_ops drm_crtc_fence_ops = { + .get_driver_name = drm_crtc_fence_get_driver_name, + .get_timeline_name = drm_crtc_fence_get_timeline_name, + .enable_signaling = drm_crtc_fence_enable_signaling, + .wait = fence_default_wait, +}; + /** * drm_crtc_init_with_planes - Initialise a new CRTC object with * specified primary and cursor planes. @@ -194,6 +220,11 @@ int drm_crtc_init_with_planes(struct drm_device *dev, struct drm_crtc *crtc, return -ENOMEM; }
+ crtc->fence_context = fence_context_alloc(1); + spin_lock_init(&crtc->fence_lock); + snprintf(crtc->timeline_name, sizeof(crtc->timeline_name), + "drm_crtc-%d", crtc->base.id); + crtc->base.properties = &crtc->properties;
list_add_tail(&crtc->head, &config->crtc_list); diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h index 2dc7e79..e92fcb2 100644 --- a/include/drm/drm_crtc.h +++ b/include/drm/drm_crtc.h @@ -32,6 +32,8 @@ #include <linux/fb.h> #include <linux/hdmi.h> #include <linux/media-bus-format.h> +#include <linux/srcu.h> +#include <linux/fence.h> #include <uapi/drm/drm_mode.h> #include <uapi/drm/drm_fourcc.h> #include <drm/drm_modeset_lock.h> @@ -542,6 +544,9 @@ struct drm_crtc_funcs { * @gamma_store: gamma ramp values * @helper_private: mid-layer private data * @properties: property tracking for this CRTC + * @fence_context: context for fence signalling + * @fence_lock: fence lock for the fence context + * @fence_seqno: seqno variable to create fences * * Each CRTC may have one or more connectors associated with it. This structure * allows the CRTC to be controlled. @@ -634,8 +639,22 @@ struct drm_crtc { * context. */ struct drm_modeset_acquire_ctx *acquire_ctx; + + /* fence timelines info for DRM out-fences */ + unsigned int fence_context; + spinlock_t fence_lock; + unsigned long fence_seqno; + char timeline_name[32]; };
+extern const struct fence_ops drm_crtc_fence_ops; + +static inline struct drm_crtc *fence_to_crtc(struct fence *fence) +{ + BUG_ON(fence->ops != &drm_crtc_fence_ops); + return container_of(fence->lock, struct drm_crtc, fence_lock); +} + /** * struct drm_plane_state - mutable plane state * @plane: backpointer to the plane
From: Gustavo Padovan gustavo.padovan@collabora.co.uk
Support DRM out-fences creating a sync_file with a fence for each crtc update with the DRM_MODE_ATOMIC_OUT_FENCE flag.
We then send an struct drm_out_fences array with the out-fences fds back in the drm_atomic_ioctl() as an out arg in the out_fences_ptr field.
struct drm_out_fences { __u32 crtc_id; __u32 fd; };
v2: Comment by Rob Clark: - Squash commit that adds DRM_MODE_ATOMIC_OUT_FENCE flag here.
Comment by Daniel Vetter: - Add clean up code for out_fences
v3: Comments by Daniel Vetter: - create DRM_MODE_ATOMIC_EVENT_MASK - userspace should fill out_fences_ptr with the crtc_ids for which it wants fences back.
Signed-off-by: Gustavo Padovan gustavo.padovan@collabora.co.uk --- drivers/gpu/drm/drm_atomic.c | 214 ++++++++++++++++++++++++++++++++++++++----- include/drm/drm_crtc.h | 10 ++ include/uapi/drm/drm_mode.h | 15 ++- 3 files changed, 217 insertions(+), 22 deletions(-)
diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c index 9e6c4e7..8732c3d 100644 --- a/drivers/gpu/drm/drm_atomic.c +++ b/drivers/gpu/drm/drm_atomic.c @@ -1483,11 +1483,9 @@ EXPORT_SYMBOL(drm_atomic_nonblocking_commit); */
static struct drm_pending_vblank_event *create_vblank_event( - struct drm_device *dev, struct drm_file *file_priv, - struct fence *fence, uint64_t user_data) + struct drm_device *dev, uint64_t user_data) { struct drm_pending_vblank_event *e = NULL; - int ret;
e = kzalloc(sizeof *e, GFP_KERNEL); if (!e) @@ -1497,17 +1495,6 @@ static struct drm_pending_vblank_event *create_vblank_event( e->event.base.length = sizeof(e->event); e->event.user_data = user_data;
- if (file_priv) { - ret = drm_event_reserve_init(dev, file_priv, &e->base, - &e->event.base); - if (ret) { - kfree(e); - return NULL; - } - } - - e->base.fence = fence; - return e; }
@@ -1612,6 +1599,147 @@ void drm_atomic_clean_old_fb(struct drm_device *dev, } EXPORT_SYMBOL(drm_atomic_clean_old_fb);
+static struct drm_out_fence_state *get_out_fence(struct drm_device *dev, + struct drm_atomic_state *state, + uint32_t __user *out_fences_ptr, + uint64_t count_out_fences, + uint64_t user_data) +{ + struct drm_crtc *crtc; + struct drm_crtc_state *crtc_state; + struct drm_out_fences *out_fences; + struct drm_out_fence_state *fence_state; + int i, ret; + + if (count_out_fences > dev->mode_config.num_crtc) + return ERR_PTR(-EINVAL); + + out_fences = kcalloc(count_out_fences, sizeof(*out_fences), + GFP_KERNEL); + if (!out_fences) + return ERR_PTR(-ENOMEM); + + if (copy_from_user(out_fences, out_fences_ptr, + count_out_fences * sizeof(*out_fences))) { + kfree(out_fences); + return ERR_PTR(-EFAULT); + } + + fence_state = kcalloc(count_out_fences, sizeof(*fence_state), + GFP_KERNEL); + if (!fence_state) { + kfree(out_fences); + return ERR_PTR(-ENOMEM); + } + + for (i = 0 ; i < count_out_fences ; i++) + fence_state[i].fd = -1; + + for (i = 0 ; i < count_out_fences ; i++) { + struct fence *fence; + + if (out_fences[i].fd != -1) { + ret = -EINVAL; + goto out; + } + + crtc = drm_crtc_find(dev, out_fences[i].crtc_id); + if (!crtc) { + ret = -ENOMEM; + goto out; + } + + crtc_state = drm_atomic_get_crtc_state(state, crtc); + if (IS_ERR(crtc_state)) { + ret = PTR_ERR(crtc_state); + goto out; + } + + fence = kzalloc(sizeof(*fence), GFP_KERNEL); + if (!fence) { + ret = -ENOMEM; + goto out; + } + + fence_init(fence, &drm_crtc_fence_ops, &crtc->fence_lock, + crtc->fence_context, ++crtc->fence_seqno); + + fence_state[i].fd = get_unused_fd_flags(O_CLOEXEC); + if (fence_state[i].fd < 0) { + fence_put(fence); + ret = fence_state[i].fd; + goto out; + } + + fence_state[i].sync_file = sync_file_create(fence); + if(!fence_state[i].sync_file) { + fence_put(fence); + ret = -ENOMEM; + goto out; + } + + crtc_state->event->base.fence = fence; + + out_fences[i].fd = fence_state[i].fd; + } + + if (copy_to_user(out_fences_ptr, out_fences, + count_out_fences * sizeof(*out_fences))) { + ret = -EFAULT; + goto out; + } + + kfree(out_fences); + + return fence_state; + +out: + for (i = 0 ; i < count_out_fences ; i++) { + if (fence_state[i].sync_file) + fput(fence_state[i].sync_file->file); + if (fence_state[i].fd >= 0) + put_unused_fd(fence_state[i].fd); + } + + kfree(fence_state); + kfree(out_fences); + + return ERR_PTR(ret); +} + +static void install_out_fence(struct drm_atomic_state *state, + uint64_t count_out_fences, + struct drm_out_fence_state *fence_state) +{ + struct drm_crtc_state *crtc_state; + struct drm_crtc *crtc; + int i; + + for (i = 0 ; i < count_out_fences ; i++) { + if (fence_state[i].sync_file) + fd_install(fence_state[i].fd, + fence_state[i].sync_file->file); + } + + for_each_crtc_in_state(state, crtc, crtc_state, i) { + if (crtc_state->event->base.fence) + fence_get(crtc_state->event->base.fence); + } +} + +static void release_out_fence(uint64_t count_out_fences, + struct drm_out_fence_state *state) +{ + int i; + + for (i = 0 ; i < count_out_fences ; i++) { + if (state->sync_file) + fput(state->sync_file->file); + if (state->fd >= 0) + put_unused_fd(state->fd); + } +} + int drm_mode_atomic_ioctl(struct drm_device *dev, void *data, struct drm_file *file_priv) { @@ -1620,12 +1748,14 @@ int drm_mode_atomic_ioctl(struct drm_device *dev, uint32_t __user *count_props_ptr = (uint32_t __user *)(unsigned long)(arg->count_props_ptr); uint32_t __user *props_ptr = (uint32_t __user *)(unsigned long)(arg->props_ptr); uint64_t __user *prop_values_ptr = (uint64_t __user *)(unsigned long)(arg->prop_values_ptr); + uint32_t __user *out_fences_ptr = (uint32_t __user *)(unsigned long)(arg->out_fences_ptr); unsigned int copied_objs, copied_props; struct drm_atomic_state *state; struct drm_modeset_acquire_ctx ctx; struct drm_plane *plane; struct drm_crtc *crtc; struct drm_crtc_state *crtc_state; + struct drm_out_fence_state *fence_state = NULL; unsigned plane_mask; int ret = 0; unsigned int i, j; @@ -1651,9 +1781,12 @@ int drm_mode_atomic_ioctl(struct drm_device *dev, !dev->mode_config.async_page_flip) return -EINVAL;
+ if ((arg->flags & DRM_MODE_ATOMIC_OUT_FENCE) && !arg->count_out_fences) + return -EINVAL; + /* can't test and expect an event at the same time. */ if ((arg->flags & DRM_MODE_ATOMIC_TEST_ONLY) && - (arg->flags & DRM_MODE_PAGE_FLIP_EVENT)) + (arg->flags & DRM_MODE_ATOMIC_EVENT_MASK)) return -EINVAL;
drm_modeset_acquire_init(&ctx, 0); @@ -1743,12 +1876,11 @@ retry: drm_mode_object_unreference(obj); }
- if (arg->flags & DRM_MODE_PAGE_FLIP_EVENT) { + if (arg->flags & DRM_MODE_ATOMIC_EVENT_MASK) { for_each_crtc_in_state(state, crtc, crtc_state, i) { struct drm_pending_vblank_event *e;
- e = create_vblank_event(dev, file_priv, NULL, - arg->user_data); + e = create_vblank_event(dev, arg->user_data); if (!e) { ret = -ENOMEM; goto out; @@ -1758,16 +1890,48 @@ retry: } }
+ if (arg->flags & DRM_MODE_PAGE_FLIP_EVENT) { + for_each_crtc_in_state(state, crtc, crtc_state, i) { + struct drm_pending_vblank_event *e = crtc_state->event; + + if (!file_priv) + continue; + + ret = drm_event_reserve_init(dev, file_priv, &e->base, + &e->event.base); + if (ret) { + kfree(e); + crtc_state->event = NULL; + goto out; + } + } + } + + if (arg->flags & DRM_MODE_ATOMIC_OUT_FENCE) { + fence_state = get_out_fence(dev, state, out_fences_ptr, + arg->count_out_fences, + arg->user_data); + if (IS_ERR(fence_state)) { + ret = PTR_ERR(fence_state); + goto out; + } + } + if (arg->flags & DRM_MODE_ATOMIC_TEST_ONLY) { /* * Unlike commit, check_only does not clean up state. * Below we call drm_atomic_state_free for it. */ ret = drm_atomic_check_only(state); - } else if (arg->flags & DRM_MODE_ATOMIC_NONBLOCK) { - ret = drm_atomic_nonblocking_commit(state); } else { - ret = drm_atomic_commit(state); + if (arg->flags & DRM_MODE_ATOMIC_OUT_FENCE) + install_out_fence(state, arg->count_out_fences, + fence_state); + + if (arg->flags & DRM_MODE_ATOMIC_NONBLOCK) + ret = drm_atomic_nonblocking_commit(state); + else + ret = drm_atomic_commit(state); }
out: @@ -1788,6 +1952,14 @@ out: } }
+ if ((arg->flags & DRM_MODE_ATOMIC_OUT_FENCE) && + !IS_ERR_OR_NULL(fence_state)) { + if (ret) + release_out_fence(arg->count_out_fences, fence_state); + + kfree(fence_state); + } + if (ret == -EDEADLK) { drm_atomic_state_clear(state); drm_modeset_backoff(&ctx); diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h index e92fcb2..010a0f9 100644 --- a/include/drm/drm_crtc.h +++ b/include/drm/drm_crtc.h @@ -656,6 +656,16 @@ static inline struct drm_crtc *fence_to_crtc(struct fence *fence) }
/** + * struct drm_out_fence_state - store out_fence data for clean up + * @sync_file: sync_file related to the out_fence + * @fd: file descriptor to installed on the sync_file. + */ +struct drm_out_fence_state { + struct sync_file *sync_file; + int fd; +}; + +/** * struct drm_plane_state - mutable plane state * @plane: backpointer to the plane * @crtc: currently bound CRTC, NULL if disabled diff --git a/include/uapi/drm/drm_mode.h b/include/uapi/drm/drm_mode.h index 49a7265..ce71ad5 100644 --- a/include/uapi/drm/drm_mode.h +++ b/include/uapi/drm/drm_mode.h @@ -588,13 +588,24 @@ struct drm_mode_destroy_dumb { #define DRM_MODE_ATOMIC_TEST_ONLY 0x0100 #define DRM_MODE_ATOMIC_NONBLOCK 0x0200 #define DRM_MODE_ATOMIC_ALLOW_MODESET 0x0400 +#define DRM_MODE_ATOMIC_OUT_FENCE 0x0800
#define DRM_MODE_ATOMIC_FLAGS (\ DRM_MODE_PAGE_FLIP_EVENT |\ DRM_MODE_PAGE_FLIP_ASYNC |\ DRM_MODE_ATOMIC_TEST_ONLY |\ DRM_MODE_ATOMIC_NONBLOCK |\ - DRM_MODE_ATOMIC_ALLOW_MODESET) + DRM_MODE_ATOMIC_ALLOW_MODESET |\ + DRM_MODE_ATOMIC_OUT_FENCE) + +#define DRM_MODE_ATOMIC_EVENT_MASK (\ + DRM_MODE_PAGE_FLIP_EVENT |\ + DRM_MODE_ATOMIC_OUT_FENCE) + +struct drm_out_fences { + __u32 crtc_id; + __u32 fd; +};
struct drm_mode_atomic { __u32 flags; @@ -605,6 +616,8 @@ struct drm_mode_atomic { __u64 prop_values_ptr; __u64 reserved; __u64 user_data; + __u64 count_out_fences; + __u64 out_fences_ptr; };
/**
dri-devel@lists.freedesktop.org
-
Gustavo Padovan -
Maarten Lankhorst