All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v10 0/3] drm: add explicit fencing
@ 2016-11-14  1:59 Gustavo Padovan
  2016-11-14  1:59   ` Gustavo Padovan
                   ` (2 more replies)
  0 siblings, 3 replies; 12+ messages in thread
From: Gustavo Padovan @ 2016-11-14  1:59 UTC (permalink / raw)
  To: dri-devel
  Cc: linux-kernel, Daniel Stone, Daniel Vetter, Rob Clark,
	Greg Hackmann, John Harrison, laurent.pinchart, seanpaul,
	marcheu, m.chehab, Maarten Lankhorst, Brian Starkey,
	Gustavo Padovan

From: Gustavo Padovan <gustavo.padovan@collabora.co.uk>

Hi,

Another iteration after comments on v8 and v9. Please refer to the cover
letter[1] in a previous version to check for more details.

The only changes in this series are in patch 3/3, see commit message for details.

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/linux-kernel@vger.kernel.org/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        | 255 +++++++++++++++++++++++++++++-------
 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, 319 insertions(+), 45 deletions(-)

-- 
2.5.5

^ permalink raw reply	[flat|nested] 12+ messages in thread
* [PATCH v10 1/3] drm/fence: add in-fences support
  2016-11-14  1:59 [PATCH v10 0/3] drm: add explicit fencing Gustavo Padovan
@ 2016-11-14  1:59   ` Gustavo Padovan
  2016-11-14  1:59   ` Gustavo Padovan
  2016-11-14  1:59   ` Gustavo Padovan
  2 siblings, 0 replies; 12+ messages in thread
From: Gustavo Padovan @ 2016-11-14  1:59 UTC (permalink / raw)
  To: dri-devel
  Cc: linux-kernel, Daniel Stone, Daniel Vetter, Rob Clark,
	Greg Hackmann, John Harrison, laurent.pinchart, seanpaul,
	marcheu, m.chehab, Maarten Lankhorst, Brian Starkey,
	Gustavo Padovan

From: Gustavo Padovan <gustavo.padovan@collabora.co.uk>

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 <gustavo.padovan@collabora.co.uk>
Reviewed-by: Brian Starkey <brian.starkey@arm.com>
Reviewed-by: Sean Paul <seanpaul@chromium.org>
Tested-by: Robert Foss <robert.foss@collabora.com>
---
 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 863cdca..95fc041 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 57e0a6e9..3ad780a 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"
 
@@ -712,6 +713,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);
@@ -773,6 +785,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 5007796..0b16587 100644
--- a/drivers/gpu/drm/drm_atomic_helper.c
+++ b/drivers/gpu/drm/drm_atomic_helper.c
@@ -3072,6 +3072,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);
 
@@ -3110,6 +3112,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 5745464..f6d1b38 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 2ba0c22..419ac31 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

^ permalink raw reply related	[flat|nested] 12+ messages in thread
* [PATCH v10 1/3] drm/fence: add in-fences support
@ 2016-11-14  1:59   ` Gustavo Padovan
  0 siblings, 0 replies; 12+ messages in thread
From: Gustavo Padovan @ 2016-11-14  1:59 UTC (permalink / raw)
  To: dri-devel
  Cc: marcheu, Daniel Stone, seanpaul, Daniel Vetter, linux-kernel,
	laurent.pinchart, Gustavo Padovan, John Harrison, m.chehab

From: Gustavo Padovan <gustavo.padovan@collabora.co.uk>

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 <gustavo.padovan@collabora.co.uk>
Reviewed-by: Brian Starkey <brian.starkey@arm.com>
Reviewed-by: Sean Paul <seanpaul@chromium.org>
Tested-by: Robert Foss <robert.foss@collabora.com>
---
 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 863cdca..95fc041 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 57e0a6e9..3ad780a 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"
 
@@ -712,6 +713,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);
@@ -773,6 +785,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 5007796..0b16587 100644
--- a/drivers/gpu/drm/drm_atomic_helper.c
+++ b/drivers/gpu/drm/drm_atomic_helper.c
@@ -3072,6 +3072,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);
 
@@ -3110,6 +3112,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 5745464..f6d1b38 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 2ba0c22..419ac31 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

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

^ permalink raw reply related	[flat|nested] 12+ messages in thread
* [PATCH v10 2/3] drm/fence: add fence timeline to drm_crtc
  2016-11-14  1:59 [PATCH v10 0/3] drm: add explicit fencing Gustavo Padovan
@ 2016-11-14  1:59   ` Gustavo Padovan
  2016-11-14  1:59   ` Gustavo Padovan
  2016-11-14  1:59   ` Gustavo Padovan
  2 siblings, 0 replies; 12+ messages in thread
From: Gustavo Padovan @ 2016-11-14  1:59 UTC (permalink / raw)
  To: dri-devel
  Cc: linux-kernel, Daniel Stone, Daniel Vetter, Rob Clark,
	Greg Hackmann, John Harrison, laurent.pinchart, seanpaul,
	marcheu, m.chehab, Maarten Lankhorst, Brian Starkey,
	Gustavo Padovan

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

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 <gustavo.padovan@collabora.co.uk>
Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Reviewed-by: Sean Paul <seanpaul@chromium.org>
Tested-by: Robert Foss <robert.foss@collabora.com>
---
 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 f6d1b38..20ddaff 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

^ permalink raw reply related	[flat|nested] 12+ messages in thread
* [PATCH v10 2/3] drm/fence: add fence timeline to drm_crtc
@ 2016-11-14  1:59   ` Gustavo Padovan
  0 siblings, 0 replies; 12+ messages in thread
From: Gustavo Padovan @ 2016-11-14  1:59 UTC (permalink / raw)
  To: dri-devel
  Cc: marcheu, Daniel Stone, seanpaul, Daniel Vetter, linux-kernel,
	laurent.pinchart, Gustavo Padovan, John Harrison, m.chehab

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

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 <gustavo.padovan@collabora.co.uk>
Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Reviewed-by: Sean Paul <seanpaul@chromium.org>
Tested-by: Robert Foss <robert.foss@collabora.com>
---
 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 f6d1b38..20ddaff 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

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

^ permalink raw reply related	[flat|nested] 12+ messages in thread
* [PATCH v10 3/3] drm/fence: add out-fences support
  2016-11-14  1:59 [PATCH v10 0/3] drm: add explicit fencing Gustavo Padovan
@ 2016-11-14  1:59   ` Gustavo Padovan
  2016-11-14  1:59   ` Gustavo Padovan
  2016-11-14  1:59   ` Gustavo Padovan
  2 siblings, 0 replies; 12+ messages in thread
From: Gustavo Padovan @ 2016-11-14  1:59 UTC (permalink / raw)
  To: dri-devel
  Cc: linux-kernel, Daniel Stone, Daniel Vetter, Rob Clark,
	Greg Hackmann, John Harrison, laurent.pinchart, seanpaul,
	marcheu, m.chehab, Maarten Lankhorst, Brian Starkey,
	Gustavo Padovan

From: Gustavo Padovan <gustavo.padovan@collabora.co.uk>

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 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()

v8: Comment by Brian Starkey
	- cancel events before check for !fence_state
	- convert a few lefovers u64 types for out_fence_ptr
	- fix memleak by assign fence_state earlier after realloc
	- proper accout num_fences in case of error

v9: Comment by Brian Starkey
	- memset last position of fence_state after krealloc
    Comments by Sean Paul
	- pass install_fds in complete_crtc_signaling() instead of ret

     - put_user(-1, fence_ptr) when decoding props

Signed-off-by: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
---
 drivers/gpu/drm/drm_atomic.c | 241 +++++++++++++++++++++++++++++++++++--------
 drivers/gpu/drm/drm_crtc.c   |   8 ++
 include/drm/drm_atomic.h     |   1 +
 include/drm/drm_crtc.h       |   6 ++
 4 files changed, 211 insertions(+), 45 deletions(-)

diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
index 3ad780a..27e5c0a 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, s64 __user *fence_ptr)
+{
+	state->crtcs[drm_crtc_index(crtc)].out_fence_ptr = fence_ptr;
+}
+
+static s64 __user * get_out_fence_for_crtc(struct drm_atomic_state *state,
+					   struct drm_crtc *crtc)
+{
+	s64 __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
@@ -494,6 +511,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 (put_user(-1, 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
@@ -536,6 +563,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
@@ -1664,11 +1693,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)
@@ -1678,17 +1705,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;
 }
 
@@ -1793,6 +1809,165 @@ 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;
+
+	if (arg->flags & DRM_MODE_ATOMIC_TEST_ONLY)
+		return 0;
+
+	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_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;
+
+			memset(&f[*num_fences], 0, sizeof(*f));
+
+			f[*num_fences].out_fence_ptr = fence_ptr;
+			*fence_state = f;
+
+			fence = get_crtc_fence(crtc);
+			if (!fence) {
+				(*num_fences)++;
+				return -ENOMEM;
+			}
+
+			ret = setup_out_fence(&f[(*num_fences)++], fence);
+			if (ret) {
+				dma_fence_put(fence);
+				return ret;
+			}
+
+			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,
+				     bool install_fds)
+{
+	struct drm_crtc *crtc;
+	struct drm_crtc_state *crtc_state;
+	int i;
+
+	if (install_fds) {
+		for (i = 0; i < num_fences; i++)
+			fd_install(fence_state[i].fd,
+				   fence_state[i].sync_file->file);
+		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);
+	}
+
+	if (!fence_state)
+		return;
+
+	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)
 {
@@ -1805,11 +1980,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))
@@ -1924,20 +2098,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) {
 		/*
@@ -1957,20 +2121,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 20ddaff..cf2423d 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

^ permalink raw reply related	[flat|nested] 12+ messages in thread
* [PATCH v10 3/3] drm/fence: add out-fences support
@ 2016-11-14  1:59   ` Gustavo Padovan
  0 siblings, 0 replies; 12+ messages in thread
From: Gustavo Padovan @ 2016-11-14  1:59 UTC (permalink / raw)
  To: dri-devel
  Cc: marcheu, Daniel Stone, seanpaul, Daniel Vetter, linux-kernel,
	laurent.pinchart, Gustavo Padovan, John Harrison, m.chehab

From: Gustavo Padovan <gustavo.padovan@collabora.co.uk>

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 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()

v8: Comment by Brian Starkey
	- cancel events before check for !fence_state
	- convert a few lefovers u64 types for out_fence_ptr
	- fix memleak by assign fence_state earlier after realloc
	- proper accout num_fences in case of error

v9: Comment by Brian Starkey
	- memset last position of fence_state after krealloc
    Comments by Sean Paul
	- pass install_fds in complete_crtc_signaling() instead of ret

     - put_user(-1, fence_ptr) when decoding props

Signed-off-by: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
---
 drivers/gpu/drm/drm_atomic.c | 241 +++++++++++++++++++++++++++++++++++--------
 drivers/gpu/drm/drm_crtc.c   |   8 ++
 include/drm/drm_atomic.h     |   1 +
 include/drm/drm_crtc.h       |   6 ++
 4 files changed, 211 insertions(+), 45 deletions(-)

diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
index 3ad780a..27e5c0a 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, s64 __user *fence_ptr)
+{
+	state->crtcs[drm_crtc_index(crtc)].out_fence_ptr = fence_ptr;
+}
+
+static s64 __user * get_out_fence_for_crtc(struct drm_atomic_state *state,
+					   struct drm_crtc *crtc)
+{
+	s64 __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
@@ -494,6 +511,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 (put_user(-1, 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
@@ -536,6 +563,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
@@ -1664,11 +1693,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)
@@ -1678,17 +1705,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;
 }
 
@@ -1793,6 +1809,165 @@ 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;
+
+	if (arg->flags & DRM_MODE_ATOMIC_TEST_ONLY)
+		return 0;
+
+	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_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;
+
+			memset(&f[*num_fences], 0, sizeof(*f));
+
+			f[*num_fences].out_fence_ptr = fence_ptr;
+			*fence_state = f;
+
+			fence = get_crtc_fence(crtc);
+			if (!fence) {
+				(*num_fences)++;
+				return -ENOMEM;
+			}
+
+			ret = setup_out_fence(&f[(*num_fences)++], fence);
+			if (ret) {
+				dma_fence_put(fence);
+				return ret;
+			}
+
+			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,
+				     bool install_fds)
+{
+	struct drm_crtc *crtc;
+	struct drm_crtc_state *crtc_state;
+	int i;
+
+	if (install_fds) {
+		for (i = 0; i < num_fences; i++)
+			fd_install(fence_state[i].fd,
+				   fence_state[i].sync_file->file);
+		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);
+	}
+
+	if (!fence_state)
+		return;
+
+	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)
 {
@@ -1805,11 +1980,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))
@@ -1924,20 +2098,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) {
 		/*
@@ -1957,20 +2121,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 20ddaff..cf2423d 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

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

^ permalink raw reply related	[flat|nested] 12+ messages in thread
* Re: [PATCH v10 3/3] drm/fence: add out-fences support
  2016-11-14  1:59   ` Gustavo Padovan
  (?)
@ 2016-11-14 10:35   ` Brian Starkey
  -1 siblings, 0 replies; 12+ messages in thread
From: Brian Starkey @ 2016-11-14 10:35 UTC (permalink / raw)
  To: Gustavo Padovan
  Cc: dri-devel, linux-kernel, Daniel Stone, Daniel Vetter, Rob Clark,
	Greg Hackmann, John Harrison, laurent.pinchart, seanpaul,
	marcheu, m.chehab, Maarten Lankhorst, Gustavo Padovan

Hi,

On Mon, Nov 14, 2016 at 10:59:56AM +0900, Gustavo Padovan wrote:
>From: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
>
>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 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()
>
>v8: Comment by Brian Starkey
>	- cancel events before check for !fence_state
>	- convert a few lefovers u64 types for out_fence_ptr
>	- fix memleak by assign fence_state earlier after realloc
>	- proper accout num_fences in case of error
>
>v9: Comment by Brian Starkey
>	- memset last position of fence_state after krealloc
>    Comments by Sean Paul
>	- pass install_fds in complete_crtc_signaling() instead of ret
>
>     - put_user(-1, fence_ptr) when decoding props
>
>Signed-off-by: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
>---
> drivers/gpu/drm/drm_atomic.c | 241 +++++++++++++++++++++++++++++++++++--------
> drivers/gpu/drm/drm_crtc.c   |   8 ++
> include/drm/drm_atomic.h     |   1 +
> include/drm/drm_crtc.h       |   6 ++
> 4 files changed, 211 insertions(+), 45 deletions(-)
>
>diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
>index 3ad780a..27e5c0a 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, s64 __user *fence_ptr)
>+{
>+	state->crtcs[drm_crtc_index(crtc)].out_fence_ptr = fence_ptr;
>+}
>+
>+static s64 __user * get_out_fence_for_crtc(struct drm_atomic_state *state,
>+					   struct drm_crtc *crtc)
>+{
>+	s64 __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
>@@ -494,6 +511,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 (put_user(-1, 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
>@@ -536,6 +563,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
>@@ -1664,11 +1693,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)
>@@ -1678,17 +1705,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;
> }
>
>@@ -1793,6 +1809,165 @@ 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;
>+
>+	if (arg->flags & DRM_MODE_ATOMIC_TEST_ONLY)
>+		return 0;
>+
>+	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_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;
>+
>+			memset(&f[*num_fences], 0, sizeof(*f));
>+
>+			f[*num_fences].out_fence_ptr = fence_ptr;
>+			*fence_state = f;
>+
>+			fence = get_crtc_fence(crtc);
>+			if (!fence) {
>+				(*num_fences)++;

You shouldn't need this increment anymore, now that they all get set
to -1 at the start. I don't mind either way though.

>+				return -ENOMEM;
>+			}
>+
>+			ret = setup_out_fence(&f[(*num_fences)++], fence);
>+			if (ret) {
>+				dma_fence_put(fence);
>+				return ret;
>+			}
>+
>+			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,
>+				     bool install_fds)
>+{
>+	struct drm_crtc *crtc;
>+	struct drm_crtc_state *crtc_state;
>+	int i;
>+
>+	if (install_fds) {
>+		for (i = 0; i < num_fences; i++)
>+			fd_install(fence_state[i].fd,
>+				   fence_state[i].sync_file->file);
>+		return;

Need to kfree(fence_state) before returning here? Must have got lost
somewhere in the v6 refactor.

With that addressed:
Reviewed-by: Brian Starkey <brian.starkey@arm.com>

Cheers,
-Brian

>+	}
>+
>+	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);
>+	}
>+
>+	if (!fence_state)
>+		return;
>+
>+	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)
> {
>@@ -1805,11 +1980,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))
>@@ -1924,20 +2098,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) {
> 		/*
>@@ -1957,20 +2121,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 20ddaff..cf2423d 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
>

^ permalink raw reply	[flat|nested] 12+ messages in thread
* Re: [PATCH v10 3/3] drm/fence: add out-fences support
  2016-11-14  1:59   ` Gustavo Padovan
@ 2016-11-14 14:57     ` Brian Starkey
  -1 siblings, 0 replies; 12+ messages in thread
From: Brian Starkey @ 2016-11-14 14:57 UTC (permalink / raw)
  To: Gustavo Padovan
  Cc: dri-devel, linux-kernel, Daniel Stone, Daniel Vetter, Rob Clark,
	Greg Hackmann, John Harrison, laurent.pinchart, seanpaul,
	marcheu, m.chehab, Maarten Lankhorst, Gustavo Padovan,
	ville.syrjala

Hi Gustavo,

I was just writing some internal docs, and it occurred to me that the
out-fence implementation here doesn't seem to match what we discussed
with Ville a few weeks back (which had completely slipped my mind).

Did the idea of returning -1 fences for multiple commits within a
frame get dropped? I didn't see any discussion further than that
thread on v5 from October:
http://lkml.iu.edu/hypermail/linux/kernel/1610.2/04727.html

Cheers,
Brian

On Mon, Nov 14, 2016 at 10:59:56AM +0900, Gustavo Padovan wrote:
>From: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
>
>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 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()
>
>v8: Comment by Brian Starkey
>	- cancel events before check for !fence_state
>	- convert a few lefovers u64 types for out_fence_ptr
>	- fix memleak by assign fence_state earlier after realloc
>	- proper accout num_fences in case of error
>
>v9: Comment by Brian Starkey
>	- memset last position of fence_state after krealloc
>    Comments by Sean Paul
>	- pass install_fds in complete_crtc_signaling() instead of ret
>
>     - put_user(-1, fence_ptr) when decoding props
>
>Signed-off-by: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
>---
> drivers/gpu/drm/drm_atomic.c | 241 +++++++++++++++++++++++++++++++++++--------
> drivers/gpu/drm/drm_crtc.c   |   8 ++
> include/drm/drm_atomic.h     |   1 +
> include/drm/drm_crtc.h       |   6 ++
> 4 files changed, 211 insertions(+), 45 deletions(-)
>
>diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
>index 3ad780a..27e5c0a 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, s64 __user *fence_ptr)
>+{
>+	state->crtcs[drm_crtc_index(crtc)].out_fence_ptr = fence_ptr;
>+}
>+
>+static s64 __user * get_out_fence_for_crtc(struct drm_atomic_state *state,
>+					   struct drm_crtc *crtc)
>+{
>+	s64 __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
>@@ -494,6 +511,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 (put_user(-1, 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
>@@ -536,6 +563,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
>@@ -1664,11 +1693,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)
>@@ -1678,17 +1705,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;
> }
>
>@@ -1793,6 +1809,165 @@ 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;
>+
>+	if (arg->flags & DRM_MODE_ATOMIC_TEST_ONLY)
>+		return 0;
>+
>+	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_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;
>+
>+			memset(&f[*num_fences], 0, sizeof(*f));
>+
>+			f[*num_fences].out_fence_ptr = fence_ptr;
>+			*fence_state = f;
>+
>+			fence = get_crtc_fence(crtc);
>+			if (!fence) {
>+				(*num_fences)++;
>+				return -ENOMEM;
>+			}
>+
>+			ret = setup_out_fence(&f[(*num_fences)++], fence);
>+			if (ret) {
>+				dma_fence_put(fence);
>+				return ret;
>+			}
>+
>+			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,
>+				     bool install_fds)
>+{
>+	struct drm_crtc *crtc;
>+	struct drm_crtc_state *crtc_state;
>+	int i;
>+
>+	if (install_fds) {
>+		for (i = 0; i < num_fences; i++)
>+			fd_install(fence_state[i].fd,
>+				   fence_state[i].sync_file->file);
>+		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);
>+	}
>+
>+	if (!fence_state)
>+		return;
>+
>+	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)
> {
>@@ -1805,11 +1980,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))
>@@ -1924,20 +2098,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) {
> 		/*
>@@ -1957,20 +2121,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 20ddaff..cf2423d 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
>

^ permalink raw reply	[flat|nested] 12+ messages in thread
* Re: [PATCH v10 3/3] drm/fence: add out-fences support
@ 2016-11-14 14:57     ` Brian Starkey
  0 siblings, 0 replies; 12+ messages in thread
From: Brian Starkey @ 2016-11-14 14:57 UTC (permalink / raw)
  To: Gustavo Padovan
  Cc: marcheu, Daniel Stone, seanpaul, Daniel Vetter, linux-kernel,
	dri-devel, m.chehab, Gustavo Padovan, John Harrison,
	laurent.pinchart

Hi Gustavo,

I was just writing some internal docs, and it occurred to me that the
out-fence implementation here doesn't seem to match what we discussed
with Ville a few weeks back (which had completely slipped my mind).

Did the idea of returning -1 fences for multiple commits within a
frame get dropped? I didn't see any discussion further than that
thread on v5 from October:
http://lkml.iu.edu/hypermail/linux/kernel/1610.2/04727.html

Cheers,
Brian

On Mon, Nov 14, 2016 at 10:59:56AM +0900, Gustavo Padovan wrote:
>From: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
>
>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 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()
>
>v8: Comment by Brian Starkey
>	- cancel events before check for !fence_state
>	- convert a few lefovers u64 types for out_fence_ptr
>	- fix memleak by assign fence_state earlier after realloc
>	- proper accout num_fences in case of error
>
>v9: Comment by Brian Starkey
>	- memset last position of fence_state after krealloc
>    Comments by Sean Paul
>	- pass install_fds in complete_crtc_signaling() instead of ret
>
>     - put_user(-1, fence_ptr) when decoding props
>
>Signed-off-by: Gustavo Padovan <gustavo.padovan@collabora.co.uk>
>---
> drivers/gpu/drm/drm_atomic.c | 241 +++++++++++++++++++++++++++++++++++--------
> drivers/gpu/drm/drm_crtc.c   |   8 ++
> include/drm/drm_atomic.h     |   1 +
> include/drm/drm_crtc.h       |   6 ++
> 4 files changed, 211 insertions(+), 45 deletions(-)
>
>diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
>index 3ad780a..27e5c0a 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, s64 __user *fence_ptr)
>+{
>+	state->crtcs[drm_crtc_index(crtc)].out_fence_ptr = fence_ptr;
>+}
>+
>+static s64 __user * get_out_fence_for_crtc(struct drm_atomic_state *state,
>+					   struct drm_crtc *crtc)
>+{
>+	s64 __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
>@@ -494,6 +511,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 (put_user(-1, 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
>@@ -536,6 +563,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
>@@ -1664,11 +1693,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)
>@@ -1678,17 +1705,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;
> }
>
>@@ -1793,6 +1809,165 @@ 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;
>+
>+	if (arg->flags & DRM_MODE_ATOMIC_TEST_ONLY)
>+		return 0;
>+
>+	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_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;
>+
>+			memset(&f[*num_fences], 0, sizeof(*f));
>+
>+			f[*num_fences].out_fence_ptr = fence_ptr;
>+			*fence_state = f;
>+
>+			fence = get_crtc_fence(crtc);
>+			if (!fence) {
>+				(*num_fences)++;
>+				return -ENOMEM;
>+			}
>+
>+			ret = setup_out_fence(&f[(*num_fences)++], fence);
>+			if (ret) {
>+				dma_fence_put(fence);
>+				return ret;
>+			}
>+
>+			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,
>+				     bool install_fds)
>+{
>+	struct drm_crtc *crtc;
>+	struct drm_crtc_state *crtc_state;
>+	int i;
>+
>+	if (install_fds) {
>+		for (i = 0; i < num_fences; i++)
>+			fd_install(fence_state[i].fd,
>+				   fence_state[i].sync_file->file);
>+		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);
>+	}
>+
>+	if (!fence_state)
>+		return;
>+
>+	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)
> {
>@@ -1805,11 +1980,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))
>@@ -1924,20 +2098,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) {
> 		/*
>@@ -1957,20 +2121,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 20ddaff..cf2423d 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
>
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply	[flat|nested] 12+ messages in thread
* Re: [PATCH v10 3/3] drm/fence: add out-fences support
  2016-11-14  1:59   ` Gustavo Padovan
                     ` (2 preceding siblings ...)
  (?)
@ 2016-11-14 17:09   ` Robert Foss
  -1 siblings, 0 replies; 12+ messages in thread
From: Robert Foss @ 2016-11-14 17:09 UTC (permalink / raw)
  To: Gustavo Padovan, dri-devel
  Cc: marcheu, Daniel Stone, seanpaul, Daniel Vetter, linux-kernel,
	laurent.pinchart, Gustavo Padovan, John Harrison, m.chehab

Tested on db410c running android + drm_hwcomposer, confirmed to be
working.

Tested-by: Robert Foss <robert.foss@collabora.com>

^ permalink raw reply	[flat|nested] 12+ messages in thread
* Re: [PATCH v10 3/3] drm/fence: add out-fences support
  2016-11-14 14:57     ` Brian Starkey
  (?)
@ 2016-11-14 20:08     ` Daniel Vetter
  -1 siblings, 0 replies; 12+ messages in thread
From: Daniel Vetter @ 2016-11-14 20:08 UTC (permalink / raw)
  To: Brian Starkey
  Cc: Gustavo Padovan, dri-devel, Linux Kernel Mailing List,
	Daniel Stone, Rob Clark, Greg Hackmann, John Harrison,
	Laurent Pinchart, Sean Paul, Stéphane Marchesin, m.chehab,
	Maarten Lankhorst, Gustavo Padovan, Syrjala, Ville

On Mon, Nov 14, 2016 at 3:57 PM, Brian Starkey <brian.starkey@arm.com> wrote:
> I was just writing some internal docs, and it occurred to me that the
> out-fence implementation here doesn't seem to match what we discussed
> with Ville a few weeks back (which had completely slipped my mind).
>
> Did the idea of returning -1 fences for multiple commits within a
> frame get dropped? I didn't see any discussion further than that
> thread on v5 from October:
> http://lkml.iu.edu/hypermail/linux/kernel/1610.2/04727.html

Atm we only support a queue depth of 1, and not faster than vblank.
This was just discussions to make sure we're not drawing ourselves
into a corner with this uabi.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

^ permalink raw reply	[flat|nested] 12+ messages in thread
end of thread, other threads:[~2016-11-14 20:08 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-11-14  1:59 [PATCH v10 0/3] drm: add explicit fencing Gustavo Padovan
2016-11-14  1:59 ` [PATCH v10 1/3] drm/fence: add in-fences support Gustavo Padovan
2016-11-14  1:59   ` Gustavo Padovan
2016-11-14  1:59 ` [PATCH v10 2/3] drm/fence: add fence timeline to drm_crtc Gustavo Padovan
2016-11-14  1:59   ` Gustavo Padovan
2016-11-14  1:59 ` [PATCH v10 3/3] drm/fence: add out-fences support Gustavo Padovan
2016-11-14  1:59   ` Gustavo Padovan
2016-11-14 10:35   ` Brian Starkey
2016-11-14 14:57   ` Brian Starkey
2016-11-14 14:57     ` Brian Starkey
2016-11-14 20:08     ` Daniel Vetter
2016-11-14 17:09   ` Robert Foss

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.