From: Gustavo Padovan <[email protected]>
Hi,
New version of the DRM fences patches with all comments on v7 adressed. Please
refer to the cover letter[1] in a previous version to check for more details.
The changes since the last version can be seen in commit message on each patch.
Robert Foss managed to port Android's drm_hwcomposer to the new HWC2 API and
added support to fences. Current patches can be seen here:
https://git.collabora.com/cgit/user/robertfoss/drm_hwcomposer.git/log/?h=hwc2_fence_v1
He ran AOSP on top of padovan/fences kernel branch with full fence support on
qemu/virgl and msm db410c. That means we already have a working open source
userspace using the explicit fencing implementation.
Also i-g-t testing are available with all tests suggested in v7 included:
https://git.collabora.com/cgit/user/padovan/intel-gpu-tools.git/log/
Please review!
Gustavo
[1] https://www.mail-archive.com/[email protected]/msg1253822.html
---
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 | 256 +++++++++++++++++++++++++++++-------
drivers/gpu/drm/drm_atomic_helper.c | 5 +
drivers/gpu/drm/drm_crtc.c | 45 +++++++
drivers/gpu/drm/drm_plane.c | 1 +
include/drm/drm_atomic.h | 1 +
include/drm/drm_crtc.h | 56 ++++++++
7 files changed, 320 insertions(+), 45 deletions(-)
--
2.5.5
From: Gustavo Padovan <[email protected]>
There is now a new property called IN_FENCE_FD attached to every plane
state that receives sync_file fds from userspace via the atomic commit
IOCTL.
The fd is then translated to a fence (that may be a fence_array
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
v4: Comment from Maarten Lankhorst
- allow set fence with no related fb
v5: rename FENCE_FD to IN_FENCE_FD
v6: Comments by Daniel Vetter:
- rename plane_state->in_fence back to "fence"
- re-introduce WARN_ON if fence set but no fb
- rebase after fence -> dma_fence rename
v7: Comments by Brian Starkey
- set state->fence to NULL when duplicating the state
- fail if IN_FENCE_FD was already set
Signed-off-by: Gustavo Padovan <[email protected]>
---
drivers/gpu/drm/Kconfig | 1 +
drivers/gpu/drm/drm_atomic.c | 14 ++++++++++++++
drivers/gpu/drm/drm_atomic_helper.c | 5 +++++
drivers/gpu/drm/drm_crtc.c | 6 ++++++
drivers/gpu/drm/drm_plane.c | 1 +
include/drm/drm_crtc.h | 5 +++++
6 files changed, 32 insertions(+)
diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
index 25e8e37..360cb92 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 b096c16..8e26ad1 100644
--- a/drivers/gpu/drm/drm_atomic.c
+++ b/drivers/gpu/drm/drm_atomic.c
@@ -31,6 +31,7 @@
#include <drm/drm_mode.h>
#include <drm/drm_plane_helper.h>
#include <drm/drm_print.h>
+#include <linux/sync_file.h>
#include "drm_crtc_internal.h"
@@ -709,6 +710,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_in_fence_fd) {
+ if (state->fence)
+ return -EINVAL;
+
+ if (U642I64(val) == -1)
+ return 0;
+
+ 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);
@@ -770,6 +782,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_in_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) {
diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
index 75ad01d..caed870 100644
--- a/drivers/gpu/drm/drm_atomic_helper.c
+++ b/drivers/gpu/drm/drm_atomic_helper.c
@@ -3076,6 +3076,8 @@ void __drm_atomic_helper_plane_duplicate_state(struct drm_plane *plane,
if (state->fb)
drm_framebuffer_reference(state->fb);
+
+ state->fence = NULL;
}
EXPORT_SYMBOL(__drm_atomic_helper_plane_duplicate_state);
@@ -3114,6 +3116,9 @@ void __drm_atomic_helper_plane_destroy_state(struct drm_plane_state *state)
{
if (state->fb)
drm_framebuffer_unreference(state->fb);
+
+ if (state->fence)
+ dma_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 ce274ed..154e040 100644
--- a/drivers/gpu/drm/drm_crtc.c
+++ b/drivers/gpu/drm/drm_crtc.c
@@ -397,6 +397,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,
+ "IN_FENCE_FD", -1, INT_MAX);
+ if (!prop)
+ return -ENOMEM;
+ dev->mode_config.prop_in_fence_fd = prop;
+
prop = drm_property_create_object(dev, DRM_MODE_PROP_ATOMIC,
"CRTC_ID", DRM_MODE_OBJECT_CRTC);
if (!prop)
diff --git a/drivers/gpu/drm/drm_plane.c b/drivers/gpu/drm/drm_plane.c
index 249c0ae..3957ef8 100644
--- a/drivers/gpu/drm/drm_plane.c
+++ b/drivers/gpu/drm/drm_plane.c
@@ -137,6 +137,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_in_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);
diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
index 8cca2a8..11780a9 100644
--- a/include/drm/drm_crtc.h
+++ b/include/drm/drm_crtc.h
@@ -1214,6 +1214,11 @@ struct drm_mode_config {
*/
struct drm_property *prop_fb_id;
/**
+ * @prop_in_fence_fd: Sync File fd representing the incoming fences
+ * for a Plane.
+ */
+ struct drm_property *prop_in_fence_fd;
+ /**
* @prop_crtc_id: Default atomic plane property to specify the
* &drm_crtc.
*/
--
2.5.5
From: Gustavo Padovan <[email protected]>
Support DRM out-fences by creating a sync_file with a fence for each CRTC
that sets the OUT_FENCE_PTR property.
We use the out_fence pointer received in the OUT_FENCE_PTR prop to send
the sync_file fd back to userspace.
The sync_file and fd are allocated/created before commit, but the
fd_install operation only happens after we know that commit succeed.
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.
v4: Create OUT_FENCE_PTR properties and remove old approach.
v5: Comments by Brian Starkey:
- Remove extra fence_get() in atomic_ioctl()
- Check ret before iterating on the crtc_state
- check ret before fd_install
- set fence_state to NULL at the beginning
- check fence_state->out_fence_ptr before put_user()
- change order of fput() and put_unused_fd() on failure
- Add access_ok() check to the out_fence_ptr received
- Rebase after fence -> dma_fence rename
- Store out_fence_ptr in the drm_atomic_state
- Split crtc_setup_out_fence()
- return -1 as out_fence with TEST_ONLY flag
v6: Comments by Daniel Vetter
- Add prepare/unprepare_crtc_signaling()
- move struct drm_out_fence_state to drm_atomic.c
- mark get_crtc_fence() as static
Comments by Brian Starkey
- proper set fence_ptr fence_state array
- isolate fence_idx increment
- improve error handling
v7: Comments by Daniel Vetter
- remove prefix from internal functions
- make out_fence_ptr an s64 pointer
- degrade DRM_INFO to DRM_DEBUG_ATOMIC when put_user fail
- fix doc issues
- filter out OUT_FENCE_PTR == NULL and do fail in this case
- add complete_crtc_signalling()
- krealloc fence_state on demand
Comment by Brian Starkey
- remove unused crtc_state arg from get_out_fence()
Signed-off-by: Gustavo Padovan <[email protected]>
---
drivers/gpu/drm/drm_atomic.c | 242 +++++++++++++++++++++++++++++++++++--------
drivers/gpu/drm/drm_crtc.c | 8 ++
include/drm/drm_atomic.h | 1 +
include/drm/drm_crtc.h | 6 ++
4 files changed, 212 insertions(+), 45 deletions(-)
diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
index 8e26ad1..cd39f13 100644
--- a/drivers/gpu/drm/drm_atomic.c
+++ b/drivers/gpu/drm/drm_atomic.c
@@ -290,6 +290,23 @@ drm_atomic_get_crtc_state(struct drm_atomic_state *state,
}
EXPORT_SYMBOL(drm_atomic_get_crtc_state);
+static void set_out_fence_for_crtc(struct drm_atomic_state *state,
+ struct drm_crtc *crtc, u64 __user *fence_ptr)
+{
+ state->crtcs[drm_crtc_index(crtc)].out_fence_ptr = fence_ptr;
+}
+
+static u64 __user * get_out_fence_for_crtc(struct drm_atomic_state *state,
+ struct drm_crtc *crtc)
+{
+ u64 __user *fence_ptr;
+
+ fence_ptr = state->crtcs[drm_crtc_index(crtc)].out_fence_ptr;
+ state->crtcs[drm_crtc_index(crtc)].out_fence_ptr = NULL;
+
+ return fence_ptr;
+}
+
/**
* drm_atomic_set_mode_for_crtc - set mode for CRTC
* @state: the CRTC whose incoming state to update
@@ -491,6 +508,16 @@ int drm_atomic_crtc_set_property(struct drm_crtc *crtc,
&replaced);
state->color_mgmt_changed |= replaced;
return ret;
+ } else if (property == config->prop_out_fence_ptr) {
+ s64 __user *fence_ptr = u64_to_user_ptr(val);
+
+ if (!fence_ptr)
+ return 0;
+
+ if (!access_ok(VERIFY_WRITE, fence_ptr, sizeof(*fence_ptr)))
+ return -EFAULT;
+
+ set_out_fence_for_crtc(state->state, crtc, fence_ptr);
} else if (crtc->funcs->atomic_set_property)
return crtc->funcs->atomic_set_property(crtc, state, property, val);
else
@@ -533,6 +560,8 @@ drm_atomic_crtc_get_property(struct drm_crtc *crtc,
*val = (state->ctm) ? state->ctm->base.id : 0;
else if (property == config->gamma_lut_property)
*val = (state->gamma_lut) ? state->gamma_lut->base.id : 0;
+ else if (property == config->prop_out_fence_ptr)
+ *val = 0;
else if (crtc->funcs->atomic_get_property)
return crtc->funcs->atomic_get_property(crtc, state, property, val);
else
@@ -1659,11 +1688,9 @@ int drm_atomic_debugfs_init(struct drm_minor *minor)
*/
static struct drm_pending_vblank_event *create_vblank_event(
- struct drm_device *dev, struct drm_file *file_priv,
- struct dma_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)
@@ -1673,17 +1700,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;
}
@@ -1788,6 +1804,166 @@ void drm_atomic_clean_old_fb(struct drm_device *dev,
}
EXPORT_SYMBOL(drm_atomic_clean_old_fb);
+static struct dma_fence *get_crtc_fence(struct drm_crtc *crtc)
+{
+ struct dma_fence *fence;
+
+ fence = kzalloc(sizeof(*fence), GFP_KERNEL);
+ if (!fence)
+ return NULL;
+
+ dma_fence_init(fence, &drm_crtc_fence_ops, &crtc->fence_lock,
+ crtc->fence_context, ++crtc->fence_seqno);
+
+ return fence;
+}
+
+struct drm_out_fence_state {
+ s64 __user *out_fence_ptr;
+ struct sync_file *sync_file;
+ int fd;
+};
+
+static int setup_out_fence(struct drm_out_fence_state *fence_state,
+ struct dma_fence *fence)
+{
+ fence_state->fd = get_unused_fd_flags(O_CLOEXEC);
+ if (fence_state->fd < 0)
+ return fence_state->fd;
+
+ if (put_user(fence_state->fd, fence_state->out_fence_ptr))
+ return -EFAULT;
+
+ fence_state->sync_file = sync_file_create(fence);
+ if(!fence_state->sync_file)
+ return -ENOMEM;
+
+ return 0;
+}
+
+static int prepare_crtc_signaling(struct drm_device *dev,
+ struct drm_atomic_state *state,
+ struct drm_mode_atomic *arg,
+ struct drm_file *file_priv,
+ struct drm_out_fence_state **fence_state,
+ unsigned int *num_fences)
+{
+ struct drm_crtc *crtc;
+ struct drm_crtc_state *crtc_state;
+ int i, ret;
+
+ for_each_crtc_in_state(state, crtc, crtc_state, i) {
+ u64 __user *fence_ptr;
+
+ fence_ptr = get_out_fence_for_crtc(crtc_state->state, crtc);
+
+ if (arg->flags & DRM_MODE_ATOMIC_TEST_ONLY && fence_ptr) {
+ if (put_user(-1, fence_ptr))
+ return -EFAULT;
+
+ continue;
+ }
+
+ if (arg->flags & DRM_MODE_PAGE_FLIP_EVENT || fence_ptr) {
+ struct drm_pending_vblank_event *e;
+
+ e = create_vblank_event(dev, arg->user_data);
+ if (!e)
+ return -ENOMEM;
+
+ crtc_state->event = e;
+ }
+
+ if (arg->flags & DRM_MODE_PAGE_FLIP_EVENT) {
+ 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;
+ return ret;
+ }
+ }
+
+ if (fence_ptr) {
+ struct dma_fence *fence;
+ struct drm_out_fence_state *f;
+
+ f = krealloc(*fence_state, sizeof(**fence_state) *
+ (*num_fences + 1), GFP_KERNEL);
+ if (!f)
+ return -ENOMEM;
+
+ f[*num_fences].out_fence_ptr = fence_ptr;
+
+ fence = get_crtc_fence(crtc);
+ if (!fence)
+ return -ENOMEM;
+
+ ret = setup_out_fence(&f[*num_fences], fence);
+ if (ret) {
+ dma_fence_put(fence);
+ return ret;
+ }
+
+ (*num_fences)++;
+
+ *fence_state = f;
+ crtc_state->event->base.fence = fence;
+ }
+ }
+
+ return 0;
+}
+
+static void complete_crtc_signaling(struct drm_device *dev,
+ struct drm_atomic_state *state,
+ struct drm_out_fence_state *fence_state,
+ unsigned int num_fences, int ret)
+{
+ struct drm_crtc *crtc;
+ struct drm_crtc_state *crtc_state;
+ int i;
+
+ if (!ret) {
+ for (i = 0; i < num_fences; i++)
+ fd_install(fence_state[i].fd,
+ fence_state[i].sync_file->file);
+ return;
+ }
+
+ if (!fence_state)
+ return;
+
+ for_each_crtc_in_state(state, crtc, crtc_state, i) {
+ /*
+ * TEST_ONLY and PAGE_FLIP_EVENT are mutually
+ * exclusive, if they weren't, this code should be
+ * called on success for TEST_ONLY too.
+ */
+ if (crtc_state->event)
+ drm_event_cancel_free(dev, &crtc_state->event->base);
+ }
+
+ for (i = 0; i < num_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);
+
+ /* If this fails log error to the user */
+ if (fence_state[i].out_fence_ptr &&
+ put_user(-1, fence_state[i].out_fence_ptr))
+ DRM_DEBUG_ATOMIC("Couldn't clear out_fence_ptr\n");
+ }
+
+ kfree(fence_state);
+}
+
int drm_mode_atomic_ioctl(struct drm_device *dev,
void *data, struct drm_file *file_priv)
{
@@ -1800,11 +1976,10 @@ int drm_mode_atomic_ioctl(struct drm_device *dev,
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;
+ unsigned int i, j, num_fences = 0;
/* disallow for drivers not supporting atomic: */
if (!drm_core_check_feature(dev, DRIVER_ATOMIC))
@@ -1919,20 +2094,10 @@ int drm_mode_atomic_ioctl(struct drm_device *dev,
drm_mode_object_unreference(obj);
}
- if (arg->flags & DRM_MODE_PAGE_FLIP_EVENT) {
- 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);
- if (!e) {
- ret = -ENOMEM;
- goto out;
- }
-
- crtc_state->event = e;
- }
- }
+ ret = prepare_crtc_signaling(dev, state, arg, file_priv, &fence_state,
+ &num_fences);
+ if (ret)
+ goto out;
if (arg->flags & DRM_MODE_ATOMIC_TEST_ONLY) {
/*
@@ -1952,20 +2117,7 @@ int drm_mode_atomic_ioctl(struct drm_device *dev,
out:
drm_atomic_clean_old_fb(dev, plane_mask, ret);
- if (ret && arg->flags & DRM_MODE_PAGE_FLIP_EVENT) {
- /*
- * TEST_ONLY and PAGE_FLIP_EVENT are mutually exclusive,
- * if they weren't, this code should be called on success
- * for TEST_ONLY too.
- */
-
- for_each_crtc_in_state(state, crtc, crtc_state, i) {
- if (!crtc_state->event)
- continue;
-
- drm_event_cancel_free(dev, &crtc_state->event->base);
- }
- }
+ complete_crtc_signaling(dev, state, fence_state, num_fences, ret);
if (ret == -EDEADLK) {
drm_atomic_state_clear(state);
diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
index 9b9881b..e8bc1ae 100644
--- a/drivers/gpu/drm/drm_crtc.c
+++ b/drivers/gpu/drm/drm_crtc.c
@@ -274,6 +274,8 @@ int drm_crtc_init_with_planes(struct drm_device *dev, struct drm_crtc *crtc,
if (drm_core_check_feature(dev, DRIVER_ATOMIC)) {
drm_object_attach_property(&crtc->base, config->prop_active, 0);
drm_object_attach_property(&crtc->base, config->prop_mode_id, 0);
+ drm_object_attach_property(&crtc->base,
+ config->prop_out_fence_ptr, 0);
}
return 0;
@@ -434,6 +436,12 @@ static int drm_mode_create_standard_properties(struct drm_device *dev)
return -ENOMEM;
dev->mode_config.prop_in_fence_fd = prop;
+ prop = drm_property_create_range(dev, DRM_MODE_PROP_ATOMIC,
+ "OUT_FENCE_PTR", 0, U64_MAX);
+ if (!prop)
+ return -ENOMEM;
+ dev->mode_config.prop_out_fence_ptr = 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_atomic.h b/include/drm/drm_atomic.h
index 331bb10..c0eaec7 100644
--- a/include/drm/drm_atomic.h
+++ b/include/drm/drm_atomic.h
@@ -144,6 +144,7 @@ struct __drm_crtcs_state {
struct drm_crtc *ptr;
struct drm_crtc_state *state;
struct drm_crtc_commit *commit;
+ s64 __user *out_fence_ptr;
};
struct __drm_connnectors_state {
diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
index 0870de1..44571bc 100644
--- a/include/drm/drm_crtc.h
+++ b/include/drm/drm_crtc.h
@@ -1264,6 +1264,12 @@ struct drm_mode_config {
*/
struct drm_property *prop_in_fence_fd;
/**
+ * @prop_out_fence_ptr: Sync File fd pointer representing the
+ * outgoing fences for a CRTC. Userspace should provide a pointer to a
+ * value of type s64, and then cast that pointer to u64.
+ */
+ struct drm_property *prop_out_fence_ptr;
+ /**
* @prop_crtc_id: Default atomic plane property to specify the
* &drm_crtc.
*/
--
2.5.5
From: Gustavo Padovan <[email protected]>
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
v4: Comments by Brian Starkey
- Use even more meaninful name for the crtc timeline
- add doc for timeline_name
Comment by Daniel Vetter
- use in-line style for comments
- rebase after fence -> dma_fence rename
v5: Comment by Daniel Vetter
- Add doc for drm_crtc_fence_ops
Signed-off-by: Gustavo Padovan <[email protected]>
Reviewed-by: Daniel Vetter <[email protected]>
---
drivers/gpu/drm/drm_crtc.c | 31 +++++++++++++++++++++++++++++++
include/drm/drm_crtc.h | 45 +++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 76 insertions(+)
diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
index 154e040..9b9881b 100644
--- a/drivers/gpu/drm/drm_crtc.c
+++ b/drivers/gpu/drm/drm_crtc.c
@@ -165,6 +165,32 @@ static void drm_crtc_crc_fini(struct drm_crtc *crtc)
#endif
}
+static const char *drm_crtc_fence_get_driver_name(struct dma_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 dma_fence *fence)
+{
+ struct drm_crtc *crtc = fence_to_crtc(fence);
+
+ return crtc->timeline_name;
+}
+
+static bool drm_crtc_fence_enable_signaling(struct dma_fence *fence)
+{
+ return true;
+}
+
+const struct dma_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 = dma_fence_default_wait,
+};
+
/**
* drm_crtc_init_with_planes - Initialise a new CRTC object with
* specified primary and cursor planes.
@@ -222,6 +248,11 @@ int drm_crtc_init_with_planes(struct drm_device *dev, struct drm_crtc *crtc,
return -ENOMEM;
}
+ crtc->fence_context = dma_fence_context_alloc(1);
+ spin_lock_init(&crtc->fence_lock);
+ snprintf(crtc->timeline_name, sizeof(crtc->timeline_name),
+ "CRTC:%d-%s", crtc->base.id, crtc->name);
+
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 11780a9..0870de1 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/dma-fence.h>
#include <uapi/drm/drm_mode.h>
#include <uapi/drm/drm_fourcc.h>
#include <drm/drm_modeset_lock.h>
@@ -739,9 +741,52 @@ struct drm_crtc {
*/
struct drm_crtc_crc crc;
#endif
+
+ /**
+ * @fence_context:
+ *
+ * timeline context used for fence operations.
+ */
+ unsigned int fence_context;
+
+ /**
+ * @fence_lock:
+ *
+ * spinlock to protect the fences in the fence_context.
+ */
+
+ spinlock_t fence_lock;
+ /**
+ * @fence_seqno:
+ *
+ * Seqno variable used as monotonic counter for the fences
+ * created on the CRTC's timeline.
+ */
+ unsigned long fence_seqno;
+
+ /**
+ * @timeline_name:
+ *
+ * The name of the CRTC's fence timeline.
+ */
+ char timeline_name[32];
};
/**
+ * dma_crtc_fence_ops - fence ops for the drm_crtc timeline
+ *
+ * It contains the dma_fence_ops that should be called by the dma_fence
+ * code. CRTC core should use this ops when initializing fences.
+ */
+extern const struct dma_fence_ops drm_crtc_fence_ops;
+
+static inline struct drm_crtc *fence_to_crtc(struct dma_fence *fence)
+{
+ BUG_ON(fence->ops != &drm_crtc_fence_ops);
+ return container_of(fence->lock, struct drm_crtc, fence_lock);
+}
+
+/**
* struct drm_mode_set - new values for a CRTC config change
* @fb: framebuffer to use for new config
* @crtc: CRTC whose configuration we're about to change
--
2.5.5
Hi Gustavo,
On Fri, Nov 11, 2016 at 02:16:07PM +0900, Gustavo Padovan wrote:
>From: Gustavo Padovan <[email protected]>
>
>There is now a new property called IN_FENCE_FD attached to every plane
>state that receives sync_file fds from userspace via the atomic commit
>IOCTL.
>
>The fd is then translated to a fence (that may be a fence_array
>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
>
>v4: Comment from Maarten Lankhorst
> - allow set fence with no related fb
>
>v5: rename FENCE_FD to IN_FENCE_FD
>
>v6: Comments by Daniel Vetter:
> - rename plane_state->in_fence back to "fence"
> - re-introduce WARN_ON if fence set but no fb
>
> - rebase after fence -> dma_fence rename
>
>v7: Comments by Brian Starkey
> - set state->fence to NULL when duplicating the state
> - fail if IN_FENCE_FD was already set
>
>Signed-off-by: Gustavo Padovan <[email protected]>
lgtm,
Reviewed-by: Brian Starkey <[email protected]>
>---
> drivers/gpu/drm/Kconfig | 1 +
> drivers/gpu/drm/drm_atomic.c | 14 ++++++++++++++
> drivers/gpu/drm/drm_atomic_helper.c | 5 +++++
> drivers/gpu/drm/drm_crtc.c | 6 ++++++
> drivers/gpu/drm/drm_plane.c | 1 +
> include/drm/drm_crtc.h | 5 +++++
> 6 files changed, 32 insertions(+)
>
>diff --git a/drivers/gpu/drm/Kconfig b/drivers/gpu/drm/Kconfig
>index 25e8e37..360cb92 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 b096c16..8e26ad1 100644
>--- a/drivers/gpu/drm/drm_atomic.c
>+++ b/drivers/gpu/drm/drm_atomic.c
>@@ -31,6 +31,7 @@
> #include <drm/drm_mode.h>
> #include <drm/drm_plane_helper.h>
> #include <drm/drm_print.h>
>+#include <linux/sync_file.h>
>
> #include "drm_crtc_internal.h"
>
>@@ -709,6 +710,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_in_fence_fd) {
>+ if (state->fence)
>+ return -EINVAL;
>+
>+ if (U642I64(val) == -1)
>+ return 0;
>+
>+ 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);
>@@ -770,6 +782,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_in_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) {
>diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
>index 75ad01d..caed870 100644
>--- a/drivers/gpu/drm/drm_atomic_helper.c
>+++ b/drivers/gpu/drm/drm_atomic_helper.c
>@@ -3076,6 +3076,8 @@ void __drm_atomic_helper_plane_duplicate_state(struct drm_plane *plane,
>
> if (state->fb)
> drm_framebuffer_reference(state->fb);
>+
>+ state->fence = NULL;
> }
> EXPORT_SYMBOL(__drm_atomic_helper_plane_duplicate_state);
>
>@@ -3114,6 +3116,9 @@ void __drm_atomic_helper_plane_destroy_state(struct drm_plane_state *state)
> {
> if (state->fb)
> drm_framebuffer_unreference(state->fb);
>+
>+ if (state->fence)
>+ dma_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 ce274ed..154e040 100644
>--- a/drivers/gpu/drm/drm_crtc.c
>+++ b/drivers/gpu/drm/drm_crtc.c
>@@ -397,6 +397,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,
>+ "IN_FENCE_FD", -1, INT_MAX);
>+ if (!prop)
>+ return -ENOMEM;
>+ dev->mode_config.prop_in_fence_fd = prop;
>+
> prop = drm_property_create_object(dev, DRM_MODE_PROP_ATOMIC,
> "CRTC_ID", DRM_MODE_OBJECT_CRTC);
> if (!prop)
>diff --git a/drivers/gpu/drm/drm_plane.c b/drivers/gpu/drm/drm_plane.c
>index 249c0ae..3957ef8 100644
>--- a/drivers/gpu/drm/drm_plane.c
>+++ b/drivers/gpu/drm/drm_plane.c
>@@ -137,6 +137,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_in_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);
>diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
>index 8cca2a8..11780a9 100644
>--- a/include/drm/drm_crtc.h
>+++ b/include/drm/drm_crtc.h
>@@ -1214,6 +1214,11 @@ struct drm_mode_config {
> */
> struct drm_property *prop_fb_id;
> /**
>+ * @prop_in_fence_fd: Sync File fd representing the incoming fences
>+ * for a Plane.
>+ */
>+ struct drm_property *prop_in_fence_fd;
>+ /**
> * @prop_crtc_id: Default atomic plane property to specify the
> * &drm_crtc.
> */
>--
>2.5.5
>
Hi Gustavo,
On Fri, Nov 11, 2016 at 02:16:09PM +0900, Gustavo Padovan wrote:
>From: Gustavo Padovan <[email protected]>
>
>Support DRM out-fences by creating a sync_file with a fence for each CRTC
>that sets the OUT_FENCE_PTR property.
>
>We use the out_fence pointer received in the OUT_FENCE_PTR prop to send
>the sync_file fd back to userspace.
>
>The sync_file and fd are allocated/created before commit, but the
>fd_install operation only happens after we know that commit succeed.
>
>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.
>
>v4: Create OUT_FENCE_PTR properties and remove old approach.
>
>v5: Comments by Brian Starkey:
> - Remove extra fence_get() in atomic_ioctl()
> - Check ret before iterating on the crtc_state
> - check ret before fd_install
> - set fence_state to NULL at the beginning
> - check fence_state->out_fence_ptr before put_user()
> - change order of fput() and put_unused_fd() on failure
>
> - Add access_ok() check to the out_fence_ptr received
> - Rebase after fence -> dma_fence rename
> - Store out_fence_ptr in the drm_atomic_state
> - Split crtc_setup_out_fence()
> - return -1 as out_fence with TEST_ONLY flag
>
>v6: Comments by Daniel Vetter
> - Add prepare/unprepare_crtc_signaling()
> - move struct drm_out_fence_state to drm_atomic.c
> - mark get_crtc_fence() as static
>
> Comments by Brian Starkey
> - proper set fence_ptr fence_state array
> - isolate fence_idx increment
>
> - improve error handling
>
>v7: Comments by Daniel Vetter
> - remove prefix from internal functions
> - make out_fence_ptr an s64 pointer
> - degrade DRM_INFO to DRM_DEBUG_ATOMIC when put_user fail
> - fix doc issues
> - filter out OUT_FENCE_PTR == NULL and do fail in this case
Do *not* fail in this case?
> - add complete_crtc_signalling()
> - krealloc fence_state on demand
>
> Comment by Brian Starkey
> - remove unused crtc_state arg from get_out_fence()
>
>Signed-off-by: Gustavo Padovan <[email protected]>
From an integration with writeback point of view, this looks fine to
me - I can add writeback to this easily.
A few comments below though:
>---
> drivers/gpu/drm/drm_atomic.c | 242 +++++++++++++++++++++++++++++++++++--------
> drivers/gpu/drm/drm_crtc.c | 8 ++
> include/drm/drm_atomic.h | 1 +
> include/drm/drm_crtc.h | 6 ++
> 4 files changed, 212 insertions(+), 45 deletions(-)
>
>diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
>index 8e26ad1..cd39f13 100644
>--- a/drivers/gpu/drm/drm_atomic.c
>+++ b/drivers/gpu/drm/drm_atomic.c
>@@ -290,6 +290,23 @@ drm_atomic_get_crtc_state(struct drm_atomic_state *state,
> }
> EXPORT_SYMBOL(drm_atomic_get_crtc_state);
>
>+static void set_out_fence_for_crtc(struct drm_atomic_state *state,
>+ struct drm_crtc *crtc, u64 __user *fence_ptr)
>+{
>+ state->crtcs[drm_crtc_index(crtc)].out_fence_ptr = fence_ptr;
>+}
>+
>+static u64 __user * get_out_fence_for_crtc(struct drm_atomic_state *state,
>+ struct drm_crtc *crtc)
>+{
>+ u64 __user *fence_ptr;
>+
>+ fence_ptr = state->crtcs[drm_crtc_index(crtc)].out_fence_ptr;
>+ state->crtcs[drm_crtc_index(crtc)].out_fence_ptr = NULL;
>+
>+ return fence_ptr;
>+}
>+
You missed a couple of s/u64/s64/ in the two functions above.
> /**
> * drm_atomic_set_mode_for_crtc - set mode for CRTC
> * @state: the CRTC whose incoming state to update
>@@ -491,6 +508,16 @@ int drm_atomic_crtc_set_property(struct drm_crtc *crtc,
> &replaced);
> state->color_mgmt_changed |= replaced;
> return ret;
>+ } else if (property == config->prop_out_fence_ptr) {
>+ s64 __user *fence_ptr = u64_to_user_ptr(val);
>+
>+ if (!fence_ptr)
>+ return 0;
>+
>+ if (!access_ok(VERIFY_WRITE, fence_ptr, sizeof(*fence_ptr)))
>+ return -EFAULT;
>+
>+ set_out_fence_for_crtc(state->state, crtc, fence_ptr);
> } else if (crtc->funcs->atomic_set_property)
> return crtc->funcs->atomic_set_property(crtc, state, property, val);
> else
>@@ -533,6 +560,8 @@ drm_atomic_crtc_get_property(struct drm_crtc *crtc,
> *val = (state->ctm) ? state->ctm->base.id : 0;
> else if (property == config->gamma_lut_property)
> *val = (state->gamma_lut) ? state->gamma_lut->base.id : 0;
>+ else if (property == config->prop_out_fence_ptr)
>+ *val = 0;
> else if (crtc->funcs->atomic_get_property)
> return crtc->funcs->atomic_get_property(crtc, state, property, val);
> else
>@@ -1659,11 +1688,9 @@ int drm_atomic_debugfs_init(struct drm_minor *minor)
> */
>
> static struct drm_pending_vblank_event *create_vblank_event(
>- struct drm_device *dev, struct drm_file *file_priv,
>- struct dma_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)
>@@ -1673,17 +1700,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;
> }
>
>@@ -1788,6 +1804,166 @@ void drm_atomic_clean_old_fb(struct drm_device *dev,
> }
> EXPORT_SYMBOL(drm_atomic_clean_old_fb);
>
>+static struct dma_fence *get_crtc_fence(struct drm_crtc *crtc)
>+{
>+ struct dma_fence *fence;
>+
>+ fence = kzalloc(sizeof(*fence), GFP_KERNEL);
>+ if (!fence)
>+ return NULL;
>+
>+ dma_fence_init(fence, &drm_crtc_fence_ops, &crtc->fence_lock,
>+ crtc->fence_context, ++crtc->fence_seqno);
>+
>+ return fence;
>+}
>+
>+struct drm_out_fence_state {
>+ s64 __user *out_fence_ptr;
>+ struct sync_file *sync_file;
>+ int fd;
>+};
>+
>+static int setup_out_fence(struct drm_out_fence_state *fence_state,
>+ struct dma_fence *fence)
>+{
>+ fence_state->fd = get_unused_fd_flags(O_CLOEXEC);
>+ if (fence_state->fd < 0)
>+ return fence_state->fd;
>+
>+ if (put_user(fence_state->fd, fence_state->out_fence_ptr))
>+ return -EFAULT;
>+
>+ fence_state->sync_file = sync_file_create(fence);
>+ if(!fence_state->sync_file)
>+ return -ENOMEM;
>+
>+ return 0;
>+}
>+
>+static int prepare_crtc_signaling(struct drm_device *dev,
>+ struct drm_atomic_state *state,
>+ struct drm_mode_atomic *arg,
>+ struct drm_file *file_priv,
>+ struct drm_out_fence_state **fence_state,
>+ unsigned int *num_fences)
>+{
>+ struct drm_crtc *crtc;
>+ struct drm_crtc_state *crtc_state;
>+ int i, ret;
>+
>+ for_each_crtc_in_state(state, crtc, crtc_state, i) {
>+ u64 __user *fence_ptr;
>+
>+ fence_ptr = get_out_fence_for_crtc(crtc_state->state, crtc);
>+
>+ if (arg->flags & DRM_MODE_ATOMIC_TEST_ONLY && fence_ptr) {
>+ if (put_user(-1, fence_ptr))
>+ return -EFAULT;
>+
>+ continue;
>+ }
>+
>+ if (arg->flags & DRM_MODE_PAGE_FLIP_EVENT || fence_ptr) {
>+ struct drm_pending_vblank_event *e;
>+
>+ e = create_vblank_event(dev, arg->user_data);
>+ if (!e)
>+ return -ENOMEM;
>+
>+ crtc_state->event = e;
>+ }
>+
>+ if (arg->flags & DRM_MODE_PAGE_FLIP_EVENT) {
>+ 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;
>+ return ret;
>+ }
>+ }
>+
>+ if (fence_ptr) {
>+ struct dma_fence *fence;
>+ struct drm_out_fence_state *f;
>+
>+ f = krealloc(*fence_state, sizeof(**fence_state) *
>+ (*num_fences + 1), GFP_KERNEL);
>+ if (!f)
>+ return -ENOMEM;
>+
>+ f[*num_fences].out_fence_ptr = fence_ptr;
>+
>+ fence = get_crtc_fence(crtc);
>+ if (!fence)
>+ return -ENOMEM;
>+
>+ ret = setup_out_fence(&f[*num_fences], fence);
>+ if (ret) {
>+ dma_fence_put(fence);
>+ return ret;
>+ }
Looks to me that the cleanup path doesn't work here. If
setup_out_fence fails, you bail out before incrementing num_fences,
and so the cleanup loop will be an iteration short.
>+
>+ (*num_fences)++;
>+
>+ *fence_state = f;
I think this is problematic too. If I understand krealloc right, when
you bail out before here and f != *fence_state, then the old
*fence_state will have been already freed, and f is going to leak.
>+ crtc_state->event->base.fence = fence;
>+ }
>+ }
>+
>+ return 0;
>+}
>+
>+static void complete_crtc_signaling(struct drm_device *dev,
>+ struct drm_atomic_state *state,
>+ struct drm_out_fence_state *fence_state,
>+ unsigned int num_fences, int ret)
>+{
>+ struct drm_crtc *crtc;
>+ struct drm_crtc_state *crtc_state;
>+ int i;
>+
>+ if (!ret) {
>+ for (i = 0; i < num_fences; i++)
>+ fd_install(fence_state[i].fd,
>+ fence_state[i].sync_file->file);
>+ return;
>+ }
>+
>+ if (!fence_state)
>+ return;
>+
>+ for_each_crtc_in_state(state, crtc, crtc_state, i) {
>+ /*
>+ * TEST_ONLY and PAGE_FLIP_EVENT are mutually
>+ * exclusive, if they weren't, this code should be
>+ * called on success for TEST_ONLY too.
>+ */
>+ if (crtc_state->event)
>+ drm_event_cancel_free(dev, &crtc_state->event->base);
>+ }
I think this event cleanup needs to be before the !fence_state check;
userspace can ask for events but no fences, and if that fails then the
events need to be freed.
Cheers,
-Brian
>+
>+ for (i = 0; i < num_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);
>+
>+ /* If this fails log error to the user */
>+ if (fence_state[i].out_fence_ptr &&
>+ put_user(-1, fence_state[i].out_fence_ptr))
>+ DRM_DEBUG_ATOMIC("Couldn't clear out_fence_ptr\n");
>+ }
>+
>+ kfree(fence_state);
>+}
>+
> int drm_mode_atomic_ioctl(struct drm_device *dev,
> void *data, struct drm_file *file_priv)
> {
>@@ -1800,11 +1976,10 @@ int drm_mode_atomic_ioctl(struct drm_device *dev,
> 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;
>+ unsigned int i, j, num_fences = 0;
>
> /* disallow for drivers not supporting atomic: */
> if (!drm_core_check_feature(dev, DRIVER_ATOMIC))
>@@ -1919,20 +2094,10 @@ int drm_mode_atomic_ioctl(struct drm_device *dev,
> drm_mode_object_unreference(obj);
> }
>
>- if (arg->flags & DRM_MODE_PAGE_FLIP_EVENT) {
>- 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);
>- if (!e) {
>- ret = -ENOMEM;
>- goto out;
>- }
>-
>- crtc_state->event = e;
>- }
>- }
>+ ret = prepare_crtc_signaling(dev, state, arg, file_priv, &fence_state,
>+ &num_fences);
>+ if (ret)
>+ goto out;
>
> if (arg->flags & DRM_MODE_ATOMIC_TEST_ONLY) {
> /*
>@@ -1952,20 +2117,7 @@ int drm_mode_atomic_ioctl(struct drm_device *dev,
> out:
> drm_atomic_clean_old_fb(dev, plane_mask, ret);
>
>- if (ret && arg->flags & DRM_MODE_PAGE_FLIP_EVENT) {
>- /*
>- * TEST_ONLY and PAGE_FLIP_EVENT are mutually exclusive,
>- * if they weren't, this code should be called on success
>- * for TEST_ONLY too.
>- */
>-
>- for_each_crtc_in_state(state, crtc, crtc_state, i) {
>- if (!crtc_state->event)
>- continue;
>-
>- drm_event_cancel_free(dev, &crtc_state->event->base);
>- }
>- }
>+ complete_crtc_signaling(dev, state, fence_state, num_fences, ret);
>
> if (ret == -EDEADLK) {
> drm_atomic_state_clear(state);
>diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
>index 9b9881b..e8bc1ae 100644
>--- a/drivers/gpu/drm/drm_crtc.c
>+++ b/drivers/gpu/drm/drm_crtc.c
>@@ -274,6 +274,8 @@ int drm_crtc_init_with_planes(struct drm_device *dev, struct drm_crtc *crtc,
> if (drm_core_check_feature(dev, DRIVER_ATOMIC)) {
> drm_object_attach_property(&crtc->base, config->prop_active, 0);
> drm_object_attach_property(&crtc->base, config->prop_mode_id, 0);
>+ drm_object_attach_property(&crtc->base,
>+ config->prop_out_fence_ptr, 0);
> }
>
> return 0;
>@@ -434,6 +436,12 @@ static int drm_mode_create_standard_properties(struct drm_device *dev)
> return -ENOMEM;
> dev->mode_config.prop_in_fence_fd = prop;
>
>+ prop = drm_property_create_range(dev, DRM_MODE_PROP_ATOMIC,
>+ "OUT_FENCE_PTR", 0, U64_MAX);
>+ if (!prop)
>+ return -ENOMEM;
>+ dev->mode_config.prop_out_fence_ptr = 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_atomic.h b/include/drm/drm_atomic.h
>index 331bb10..c0eaec7 100644
>--- a/include/drm/drm_atomic.h
>+++ b/include/drm/drm_atomic.h
>@@ -144,6 +144,7 @@ struct __drm_crtcs_state {
> struct drm_crtc *ptr;
> struct drm_crtc_state *state;
> struct drm_crtc_commit *commit;
>+ s64 __user *out_fence_ptr;
> };
>
> struct __drm_connnectors_state {
>diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
>index 0870de1..44571bc 100644
>--- a/include/drm/drm_crtc.h
>+++ b/include/drm/drm_crtc.h
>@@ -1264,6 +1264,12 @@ struct drm_mode_config {
> */
> struct drm_property *prop_in_fence_fd;
> /**
>+ * @prop_out_fence_ptr: Sync File fd pointer representing the
>+ * outgoing fences for a CRTC. Userspace should provide a pointer to a
>+ * value of type s64, and then cast that pointer to u64.
>+ */
>+ struct drm_property *prop_out_fence_ptr;
>+ /**
> * @prop_crtc_id: Default atomic plane property to specify the
> * &drm_crtc.
> */
>--
>2.5.5
>
Hi Brian,
2016-11-11 Brian Starkey <[email protected]>:
> Hi Gustavo,
>
> On Fri, Nov 11, 2016 at 02:16:09PM +0900, Gustavo Padovan wrote:
> > From: Gustavo Padovan <[email protected]>
> >
> > Support DRM out-fences by creating a sync_file with a fence for each CRTC
> > that sets the OUT_FENCE_PTR property.
> >
> > We use the out_fence pointer received in the OUT_FENCE_PTR prop to send
> > the sync_file fd back to userspace.
> >
> > The sync_file and fd are allocated/created before commit, but the
> > fd_install operation only happens after we know that commit succeed.
> >
> > 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.
> >
> > v4: Create OUT_FENCE_PTR properties and remove old approach.
> >
> > v5: Comments by Brian Starkey:
> > - Remove extra fence_get() in atomic_ioctl()
> > - Check ret before iterating on the crtc_state
> > - check ret before fd_install
> > - set fence_state to NULL at the beginning
> > - check fence_state->out_fence_ptr before put_user()
> > - change order of fput() and put_unused_fd() on failure
> >
> > - Add access_ok() check to the out_fence_ptr received
> > - Rebase after fence -> dma_fence rename
> > - Store out_fence_ptr in the drm_atomic_state
> > - Split crtc_setup_out_fence()
> > - return -1 as out_fence with TEST_ONLY flag
> >
> > v6: Comments by Daniel Vetter
> > - Add prepare/unprepare_crtc_signaling()
> > - move struct drm_out_fence_state to drm_atomic.c
> > - mark get_crtc_fence() as static
> >
> > Comments by Brian Starkey
> > - proper set fence_ptr fence_state array
> > - isolate fence_idx increment
> >
> > - improve error handling
> >
> > v7: Comments by Daniel Vetter
> > - remove prefix from internal functions
> > - make out_fence_ptr an s64 pointer
> > - degrade DRM_INFO to DRM_DEBUG_ATOMIC when put_user fail
> > - fix doc issues
> > - filter out OUT_FENCE_PTR == NULL and do fail in this case
>
> Do *not* fail in this case?
>
> > - add complete_crtc_signalling()
> > - krealloc fence_state on demand
> >
> > Comment by Brian Starkey
> > - remove unused crtc_state arg from get_out_fence()
> >
> > Signed-off-by: Gustavo Padovan <[email protected]>
>
> From an integration with writeback point of view, this looks fine to
> me - I can add writeback to this easily.
>
> A few comments below though:
>
> > ---
> > drivers/gpu/drm/drm_atomic.c | 242 +++++++++++++++++++++++++++++++++++--------
> > drivers/gpu/drm/drm_crtc.c | 8 ++
> > include/drm/drm_atomic.h | 1 +
> > include/drm/drm_crtc.h | 6 ++
> > 4 files changed, 212 insertions(+), 45 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
> > index 8e26ad1..cd39f13 100644
> > --- a/drivers/gpu/drm/drm_atomic.c
> > +++ b/drivers/gpu/drm/drm_atomic.c
> > @@ -290,6 +290,23 @@ drm_atomic_get_crtc_state(struct drm_atomic_state *state,
> > }
> > EXPORT_SYMBOL(drm_atomic_get_crtc_state);
> >
> > +static void set_out_fence_for_crtc(struct drm_atomic_state *state,
> > + struct drm_crtc *crtc, u64 __user *fence_ptr)
> > +{
> > + state->crtcs[drm_crtc_index(crtc)].out_fence_ptr = fence_ptr;
> > +}
> > +
> > +static u64 __user * get_out_fence_for_crtc(struct drm_atomic_state *state,
> > + struct drm_crtc *crtc)
> > +{
> > + u64 __user *fence_ptr;
> > +
> > + fence_ptr = state->crtcs[drm_crtc_index(crtc)].out_fence_ptr;
> > + state->crtcs[drm_crtc_index(crtc)].out_fence_ptr = NULL;
> > +
> > + return fence_ptr;
> > +}
> > +
>
> You missed a couple of s/u64/s64/ in the two functions above.
>
> > /**
> > * drm_atomic_set_mode_for_crtc - set mode for CRTC
> > * @state: the CRTC whose incoming state to update
> > @@ -491,6 +508,16 @@ int drm_atomic_crtc_set_property(struct drm_crtc *crtc,
> > &replaced);
> > state->color_mgmt_changed |= replaced;
> > return ret;
> > + } else if (property == config->prop_out_fence_ptr) {
> > + s64 __user *fence_ptr = u64_to_user_ptr(val);
> > +
> > + if (!fence_ptr)
> > + return 0;
> > +
> > + if (!access_ok(VERIFY_WRITE, fence_ptr, sizeof(*fence_ptr)))
> > + return -EFAULT;
> > +
> > + set_out_fence_for_crtc(state->state, crtc, fence_ptr);
> > } else if (crtc->funcs->atomic_set_property)
> > return crtc->funcs->atomic_set_property(crtc, state, property, val);
> > else
> > @@ -533,6 +560,8 @@ drm_atomic_crtc_get_property(struct drm_crtc *crtc,
> > *val = (state->ctm) ? state->ctm->base.id : 0;
> > else if (property == config->gamma_lut_property)
> > *val = (state->gamma_lut) ? state->gamma_lut->base.id : 0;
> > + else if (property == config->prop_out_fence_ptr)
> > + *val = 0;
> > else if (crtc->funcs->atomic_get_property)
> > return crtc->funcs->atomic_get_property(crtc, state, property, val);
> > else
> > @@ -1659,11 +1688,9 @@ int drm_atomic_debugfs_init(struct drm_minor *minor)
> > */
> >
> > static struct drm_pending_vblank_event *create_vblank_event(
> > - struct drm_device *dev, struct drm_file *file_priv,
> > - struct dma_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)
> > @@ -1673,17 +1700,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;
> > }
> >
> > @@ -1788,6 +1804,166 @@ void drm_atomic_clean_old_fb(struct drm_device *dev,
> > }
> > EXPORT_SYMBOL(drm_atomic_clean_old_fb);
> >
> > +static struct dma_fence *get_crtc_fence(struct drm_crtc *crtc)
> > +{
> > + struct dma_fence *fence;
> > +
> > + fence = kzalloc(sizeof(*fence), GFP_KERNEL);
> > + if (!fence)
> > + return NULL;
> > +
> > + dma_fence_init(fence, &drm_crtc_fence_ops, &crtc->fence_lock,
> > + crtc->fence_context, ++crtc->fence_seqno);
> > +
> > + return fence;
> > +}
> > +
> > +struct drm_out_fence_state {
> > + s64 __user *out_fence_ptr;
> > + struct sync_file *sync_file;
> > + int fd;
> > +};
> > +
> > +static int setup_out_fence(struct drm_out_fence_state *fence_state,
> > + struct dma_fence *fence)
> > +{
> > + fence_state->fd = get_unused_fd_flags(O_CLOEXEC);
> > + if (fence_state->fd < 0)
> > + return fence_state->fd;
> > +
> > + if (put_user(fence_state->fd, fence_state->out_fence_ptr))
> > + return -EFAULT;
> > +
> > + fence_state->sync_file = sync_file_create(fence);
> > + if(!fence_state->sync_file)
> > + return -ENOMEM;
> > +
> > + return 0;
> > +}
> > +
> > +static int prepare_crtc_signaling(struct drm_device *dev,
> > + struct drm_atomic_state *state,
> > + struct drm_mode_atomic *arg,
> > + struct drm_file *file_priv,
> > + struct drm_out_fence_state **fence_state,
> > + unsigned int *num_fences)
> > +{
> > + struct drm_crtc *crtc;
> > + struct drm_crtc_state *crtc_state;
> > + int i, ret;
> > +
> > + for_each_crtc_in_state(state, crtc, crtc_state, i) {
> > + u64 __user *fence_ptr;
> > +
> > + fence_ptr = get_out_fence_for_crtc(crtc_state->state, crtc);
> > +
> > + if (arg->flags & DRM_MODE_ATOMIC_TEST_ONLY && fence_ptr) {
> > + if (put_user(-1, fence_ptr))
> > + return -EFAULT;
> > +
> > + continue;
> > + }
> > +
> > + if (arg->flags & DRM_MODE_PAGE_FLIP_EVENT || fence_ptr) {
> > + struct drm_pending_vblank_event *e;
> > +
> > + e = create_vblank_event(dev, arg->user_data);
> > + if (!e)
> > + return -ENOMEM;
> > +
> > + crtc_state->event = e;
> > + }
> > +
> > + if (arg->flags & DRM_MODE_PAGE_FLIP_EVENT) {
> > + 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;
> > + return ret;
> > + }
> > + }
> > +
> > + if (fence_ptr) {
> > + struct dma_fence *fence;
> > + struct drm_out_fence_state *f;
> > +
> > + f = krealloc(*fence_state, sizeof(**fence_state) *
> > + (*num_fences + 1), GFP_KERNEL);
> > + if (!f)
> > + return -ENOMEM;
> > +
> > + f[*num_fences].out_fence_ptr = fence_ptr;
> > +
> > + fence = get_crtc_fence(crtc);
> > + if (!fence)
> > + return -ENOMEM;
> > +
> > + ret = setup_out_fence(&f[*num_fences], fence);
> > + if (ret) {
> > + dma_fence_put(fence);
> > + return ret;
> > + }
>
> Looks to me that the cleanup path doesn't work here. If
> setup_out_fence fails, you bail out before incrementing num_fences,
> and so the cleanup loop will be an iteration short.
>
> > +
> > + (*num_fences)++;
> > +
> > + *fence_state = f;
>
> I think this is problematic too. If I understand krealloc right, when
> you bail out before here and f != *fence_state, then the old
> *fence_state will have been already freed, and f is going to leak.
Yes, it is broken, I missed that. I think for fence_state I can just
move the assignment a few lines above and for num_fences one way is to
increment it before returning on error.
>
> > + crtc_state->event->base.fence = fence;
> > + }
> > + }
> > +
> > + return 0;
> > +}
> > +
> > +static void complete_crtc_signaling(struct drm_device *dev,
> > + struct drm_atomic_state *state,
> > + struct drm_out_fence_state *fence_state,
> > + unsigned int num_fences, int ret)
> > +{
> > + struct drm_crtc *crtc;
> > + struct drm_crtc_state *crtc_state;
> > + int i;
> > +
> > + if (!ret) {
> > + for (i = 0; i < num_fences; i++)
> > + fd_install(fence_state[i].fd,
> > + fence_state[i].sync_file->file);
> > + return;
> > + }
> > +
> > + if (!fence_state)
> > + return;
> > +
> > + for_each_crtc_in_state(state, crtc, crtc_state, i) {
> > + /*
> > + * TEST_ONLY and PAGE_FLIP_EVENT are mutually
> > + * exclusive, if they weren't, this code should be
> > + * called on success for TEST_ONLY too.
> > + */
> > + if (crtc_state->event)
> > + drm_event_cancel_free(dev, &crtc_state->event->base);
> > + }
>
> I think this event cleanup needs to be before the !fence_state check;
> userspace can ask for events but no fences, and if that fails then the
> events need to be freed.
Yes. In the previous version this was the correct flow, but with v8 the
clean up actually needs to happen before.
Gustavo
Subject was supposed to be v9! sorry about that.
2016-11-11 Gustavo Padovan <[email protected]>:
> From: Gustavo Padovan <[email protected]>
>
> Hi,
>
> Another iteration after Brian comments. Please refer to the cover letter[1] in
> a previous version to check for more details.
>
> The changes since the last version can be seen in commit message on each patch.
>
> Robert Foss managed to port Android's drm_hwcomposer to the new HWC2 API and
> added support to fences. Current patches can be seen here:
>
> https://git.collabora.com/cgit/user/robertfoss/drm_hwcomposer.git/log/?h=hwc2_fence_v1
>
> He ran AOSP on top of padovan/fences kernel branch with full fence support on
> qemu/virgl and msm db410c. That means we already have a working open source
> userspace using the explicit fencing implementation.
>
> Also i-g-t testing are available with all tests suggested in v7 included:
>
> https://git.collabora.com/cgit/user/padovan/intel-gpu-tools.git/log/
>
> Please review!
>
> Gustavo
>
> [1] https://www.mail-archive.com/[email protected]/msg1253822.html
>
> ---
> 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 | 257 +++++++++++++++++++++++++++++-------
> drivers/gpu/drm/drm_atomic_helper.c | 5 +
> drivers/gpu/drm/drm_crtc.c | 45 +++++++
> drivers/gpu/drm/drm_plane.c | 1 +
> include/drm/drm_atomic.h | 1 +
> include/drm/drm_crtc.h | 56 ++++++++
> 7 files changed, 321 insertions(+), 45 deletions(-)
>
> --
> 2.5.5
>
On Fri, Nov 11, 2016 at 12:16 AM, Gustavo Padovan <[email protected]> wrote:
> From: Gustavo Padovan <[email protected]>
>
> 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
>
> v4: Comments by Brian Starkey
> - Use even more meaninful name for the crtc timeline
> - add doc for timeline_name
> Comment by Daniel Vetter
> - use in-line style for comments
>
> - rebase after fence -> dma_fence rename
>
> v5: Comment by Daniel Vetter
> - Add doc for drm_crtc_fence_ops
>
> Signed-off-by: Gustavo Padovan <[email protected]>
> Reviewed-by: Daniel Vetter <[email protected]>
Reviewed-by: Sean Paul <[email protected]>
> ---
> drivers/gpu/drm/drm_crtc.c | 31 +++++++++++++++++++++++++++++++
> include/drm/drm_crtc.h | 45 +++++++++++++++++++++++++++++++++++++++++++++
> 2 files changed, 76 insertions(+)
>
> diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
> index 154e040..9b9881b 100644
> --- a/drivers/gpu/drm/drm_crtc.c
> +++ b/drivers/gpu/drm/drm_crtc.c
> @@ -165,6 +165,32 @@ static void drm_crtc_crc_fini(struct drm_crtc *crtc)
> #endif
> }
>
> +static const char *drm_crtc_fence_get_driver_name(struct dma_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 dma_fence *fence)
> +{
> + struct drm_crtc *crtc = fence_to_crtc(fence);
> +
> + return crtc->timeline_name;
> +}
> +
> +static bool drm_crtc_fence_enable_signaling(struct dma_fence *fence)
> +{
> + return true;
> +}
> +
> +const struct dma_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 = dma_fence_default_wait,
> +};
> +
> /**
> * drm_crtc_init_with_planes - Initialise a new CRTC object with
> * specified primary and cursor planes.
> @@ -222,6 +248,11 @@ int drm_crtc_init_with_planes(struct drm_device *dev, struct drm_crtc *crtc,
> return -ENOMEM;
> }
>
> + crtc->fence_context = dma_fence_context_alloc(1);
> + spin_lock_init(&crtc->fence_lock);
> + snprintf(crtc->timeline_name, sizeof(crtc->timeline_name),
> + "CRTC:%d-%s", crtc->base.id, crtc->name);
> +
> 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 11780a9..0870de1 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/dma-fence.h>
> #include <uapi/drm/drm_mode.h>
> #include <uapi/drm/drm_fourcc.h>
> #include <drm/drm_modeset_lock.h>
> @@ -739,9 +741,52 @@ struct drm_crtc {
> */
> struct drm_crtc_crc crc;
> #endif
> +
> + /**
> + * @fence_context:
> + *
> + * timeline context used for fence operations.
> + */
> + unsigned int fence_context;
> +
> + /**
> + * @fence_lock:
> + *
> + * spinlock to protect the fences in the fence_context.
> + */
> +
> + spinlock_t fence_lock;
> + /**
> + * @fence_seqno:
> + *
> + * Seqno variable used as monotonic counter for the fences
> + * created on the CRTC's timeline.
> + */
> + unsigned long fence_seqno;
> +
> + /**
> + * @timeline_name:
> + *
> + * The name of the CRTC's fence timeline.
> + */
> + char timeline_name[32];
> };
>
> /**
> + * dma_crtc_fence_ops - fence ops for the drm_crtc timeline
> + *
> + * It contains the dma_fence_ops that should be called by the dma_fence
> + * code. CRTC core should use this ops when initializing fences.
> + */
> +extern const struct dma_fence_ops drm_crtc_fence_ops;
> +
> +static inline struct drm_crtc *fence_to_crtc(struct dma_fence *fence)
> +{
> + BUG_ON(fence->ops != &drm_crtc_fence_ops);
> + return container_of(fence->lock, struct drm_crtc, fence_lock);
> +}
> +
> +/**
> * struct drm_mode_set - new values for a CRTC config change
> * @fb: framebuffer to use for new config
> * @crtc: CRTC whose configuration we're about to change
> --
> 2.5.5
>