Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit b13cc8dd authored by Brian Starkey's avatar Brian Starkey Committed by Liviu Dudau
Browse files

drm: writeback: Add out-fences for writeback connectors 



Add the WRITEBACK_OUT_FENCE_PTR property to writeback connectors, to
enable userspace to get a fence which will signal once the writeback is
complete. It is not allowed to request an out-fence without a
framebuffer attached to the connector.

A timeline is added to drm_writeback_connector for use by the writeback
out-fences.

In the case of a commit failure or DRM_MODE_ATOMIC_TEST_ONLY, the fence
is set to -1.

Changes from v2:
 - Rebase onto Gustavo Padovan's v9 explicit sync series
 - Change out_fence_ptr type to s32 __user *
 - Set *out_fence_ptr to -1 in drm_atomic_connector_set_property
 - Store fence in drm_writeback_job
 Gustavo Padovan:
 - Move out_fence_ptr out of connector_state
 - Signal fence from drm_writeback_signal_completion instead of
   in driver directly

Changes from v3:
 - Rebase onto commit 7e9081c5 ("drm/fence: fix memory overwrite
   when setting out_fence fd") (change out_fence_ptr to s32 __user *,
   for real this time.)
 - Update documentation around WRITEBACK_OUT_FENCE_PTR

Signed-off-by: default avatarBrian Starkey <brian.starkey@arm.com>
[rebased and fixed conflicts]
Signed-off-by: default avatarMihail Atanassov <mihail.atanassov@arm.com>
Signed-off-by: default avatarLiviu Dudau <liviu.dudau@arm.com>
Reviewed-by: default avatarEric Anholt <eric@anholt.net>
Reviewed-by: default avatarSean Paul <seanpaul@chromium.org>
Link: https://patchwork.freedesktop.org/patch/229036/
parent 935774cd

drivers/gpu/drm/drm_atomic.c

+90 −9
Original line number Diff line number Diff line
@@ -318,6 +318,35 @@ static s32 __user *get_out_fence_for_crtc(struct drm_atomic_state *state, 
	return fence_ptr;

}



static int set_out_fence_for_connector(struct drm_atomic_state *state,

					struct drm_connector *connector,

					s32 __user *fence_ptr)

{

	unsigned int index = drm_connector_index(connector);



	if (!fence_ptr)

		return 0;



	if (put_user(-1, fence_ptr))

		return -EFAULT;



	state->connectors[index].out_fence_ptr = fence_ptr;



	return 0;

}



static s32 __user *get_out_fence_for_connector(struct drm_atomic_state *state,

					       struct drm_connector *connector)

{

	unsigned int index = drm_connector_index(connector);

	s32 __user *fence_ptr;



	fence_ptr = state->connectors[index].out_fence_ptr;

	state->connectors[index].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
@@ -727,6 +756,12 @@ static int drm_atomic_connector_check(struct drm_connector *connector, 
		return -EINVAL;

	}



	if (writeback_job->out_fence && !writeback_job->fb) {

		DRM_DEBUG_ATOMIC("[CONNECTOR:%d:%s] requesting out-fence without framebuffer\n",

				 connector->base.id, connector->name);

		return -EINVAL;

	}



	return 0;

}
@@ -1367,6 +1402,11 @@ static int drm_atomic_connector_set_property(struct drm_connector *connector, 
		if (fb)

			drm_framebuffer_put(fb);

		return ret;

	} else if (property == config->writeback_out_fence_ptr_property) {

		s32 __user *fence_ptr = u64_to_user_ptr(val);



		return set_out_fence_for_connector(state->state, connector,

						   fence_ptr);

	} else if (connector->funcs->atomic_set_property) {

		return connector->funcs->atomic_set_property(connector,

				state, property, val);
@@ -1456,6 +1496,8 @@ drm_atomic_connector_get_property(struct drm_connector *connector, 
	} else if (property == config->writeback_fb_id_property) {

		/* Writeback framebuffer is one-shot, write and forget */

		*val = 0;

	} else if (property == config->writeback_out_fence_ptr_property) {

		*val = 0;

	} else if (connector->funcs->atomic_get_property) {

		return connector->funcs->atomic_get_property(connector,

				state, property, val);
@@ -2292,7 +2334,7 @@ static int setup_out_fence(struct drm_out_fence_state *fence_state, 
	return 0;

}



static int prepare_crtc_signaling(struct drm_device *dev,

static int prepare_signaling(struct drm_device *dev,

				  struct drm_atomic_state *state,

				  struct drm_mode_atomic *arg,

				  struct drm_file *file_priv,
@@ -2301,6 +2343,8 @@ static int prepare_crtc_signaling(struct drm_device *dev, 
{

	struct drm_crtc *crtc;

	struct drm_crtc_state *crtc_state;

	struct drm_connector *conn;

	struct drm_connector_state *conn_state;

	int i, c = 0, ret;



	if (arg->flags & DRM_MODE_ATOMIC_TEST_ONLY)
@@ -2366,6 +2410,43 @@ static int prepare_crtc_signaling(struct drm_device *dev, 
		c++;

	}



	for_each_new_connector_in_state(state, conn, conn_state, i) {

		struct drm_writeback_job *job;

		struct drm_out_fence_state *f;

		struct dma_fence *fence;

		s32 __user *fence_ptr;



		fence_ptr = get_out_fence_for_connector(state, conn);

		if (!fence_ptr)

			continue;



		job = drm_atomic_get_writeback_job(conn_state);

		if (!job)

			return -ENOMEM;



		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 = drm_writeback_get_out_fence((struct drm_writeback_connector *)conn);

		if (!fence)

			return -ENOMEM;



		ret = setup_out_fence(&f[(*num_fences)++], fence);

		if (ret) {

			dma_fence_put(fence);

			return ret;

		}



		job->out_fence = fence;

	}



	/*

	 * Having this flag means user mode pends on event which will never

	 * reach due to lack of at least one CRTC for signaling
@@ -2376,7 +2457,7 @@ static int prepare_crtc_signaling(struct drm_device *dev, 
	return 0;

}



static void complete_crtc_signaling(struct drm_device *dev,

static void complete_signaling(struct drm_device *dev,

			       struct drm_atomic_state *state,

			       struct drm_out_fence_state *fence_state,

			       unsigned int num_fences,
@@ -2550,7 +2631,7 @@ int drm_mode_atomic_ioctl(struct drm_device *dev, 
		drm_mode_object_put(obj);

	}



	ret = prepare_crtc_signaling(dev, state, arg, file_priv, &fence_state,

	ret = prepare_signaling(dev, state, arg, file_priv, &fence_state,

				&num_fences);

	if (ret)

		goto out;
@@ -2567,7 +2648,7 @@ int drm_mode_atomic_ioctl(struct drm_device *dev, 
	}



out:

	complete_crtc_signaling(dev, state, fence_state, num_fences, !ret);

	complete_signaling(dev, state, fence_state, num_fences, !ret);



	if (ret == -EDEADLK) {

		drm_atomic_state_clear(state);

drivers/gpu/drm/drm_writeback.c

+107 −2
Original line number Diff line number Diff line
@@ -14,6 +14,7 @@ 
#include <drm/drm_property.h>

#include <drm/drm_writeback.h>

#include <drm/drmP.h>

#include <linux/dma-fence.h>



/**

 * DOC: overview
@@ -31,6 +32,16 @@ 
 * framebuffer applies only to a single commit (see below). A framebuffer may

 * not be attached while the CRTC is off.

 *

 * Unlike with planes, when a writeback framebuffer is removed by userspace DRM

 * makes no attempt to remove it from active use by the connector. This is

 * because no method is provided to abort a writeback operation, and in any

 * case making a new commit whilst a writeback is ongoing is undefined (see

 * WRITEBACK_OUT_FENCE_PTR below). As soon as the current writeback is finished,

 * the framebuffer will automatically no longer be in active use. As it will

 * also have already been removed from the framebuffer list, there will be no

 * way for any userspace application to retrieve a reference to it in the

 * intervening period.

 *

 * Writeback connectors have some additional properties, which userspace

 * can use to query and control them:

 *
@@ -47,8 +58,54 @@ 
 *	data is an array of u32 DRM_FORMAT_* fourcc values.

 *	Userspace can use this blob to find out what pixel formats are supported

 *	by the connector's writeback engine.

 *

 *  "WRITEBACK_OUT_FENCE_PTR":

 *	Userspace can use this property to provide a pointer for the kernel to

 *	fill with a sync_file file descriptor, which will signal once the

 *	writeback is finished. The value should be the address of a 32-bit

 *	signed integer, cast to a u64.

 *	Userspace should wait for this fence to signal before making another

 *	commit affecting any of the same CRTCs, Planes or Connectors.

 *	**Failure to do so will result in undefined behaviour.**

 *	For this reason it is strongly recommended that all userspace

 *	applications making use of writeback connectors *always* retrieve an

 *	out-fence for the commit and use it appropriately.

 *	From userspace, this property will always read as zero.

 */



#define fence_to_wb_connector(x) container_of(x->lock, \

					      struct drm_writeback_connector, \

					      fence_lock)



static const char *drm_writeback_fence_get_driver_name(struct dma_fence *fence)

{

	struct drm_writeback_connector *wb_connector =

		fence_to_wb_connector(fence);



	return wb_connector->base.dev->driver->name;

}



static const char *

drm_writeback_fence_get_timeline_name(struct dma_fence *fence)

{

	struct drm_writeback_connector *wb_connector =

		fence_to_wb_connector(fence);



	return wb_connector->timeline_name;

}



static bool drm_writeback_fence_enable_signaling(struct dma_fence *fence)

{

	return true;

}



static const struct dma_fence_ops drm_writeback_fence_ops = {

	.get_driver_name = drm_writeback_fence_get_driver_name,

	.get_timeline_name = drm_writeback_fence_get_timeline_name,

	.enable_signaling = drm_writeback_fence_enable_signaling,

	.wait = dma_fence_default_wait,

};



static int create_writeback_properties(struct drm_device *dev)

{

	struct drm_property *prop;
@@ -72,6 +129,15 @@ static int create_writeback_properties(struct drm_device *dev) 
		dev->mode_config.writeback_pixel_formats_property = prop;

	}



	if (!dev->mode_config.writeback_out_fence_ptr_property) {

		prop = drm_property_create_range(dev, DRM_MODE_PROP_ATOMIC,

						 "WRITEBACK_OUT_FENCE_PTR", 0,

						 U64_MAX);

		if (!prop)

			return -ENOMEM;

		dev->mode_config.writeback_out_fence_ptr_property = prop;

	}



	return 0;

}
@@ -141,6 +207,15 @@ int drm_writeback_connector_init(struct drm_device *dev, 
	INIT_LIST_HEAD(&wb_connector->job_queue);

	spin_lock_init(&wb_connector->job_lock);



	wb_connector->fence_context = dma_fence_context_alloc(1);

	spin_lock_init(&wb_connector->fence_lock);

	snprintf(wb_connector->timeline_name,

		 sizeof(wb_connector->timeline_name),

		 "CONNECTOR:%d-%s", connector->base.id, connector->name);



	drm_object_attach_property(&connector->base,

				   config->writeback_out_fence_ptr_property, 0);



	drm_object_attach_property(&connector->base,

				   config->writeback_fb_id_property, 0);
@@ -210,6 +285,7 @@ static void cleanup_work(struct work_struct *work) 
/**

 * drm_writeback_signal_completion - Signal the completion of a writeback job

 * @wb_connector: The writeback connector whose job is complete

 * @status: Status code to set in the writeback out_fence (0 for success)

 *

 * Drivers should call this to signal the completion of a previously queued

 * writeback job. It should be called as soon as possible after the hardware
@@ -223,7 +299,8 @@ static void cleanup_work(struct work_struct *work) 
 * See also: drm_writeback_queue_job()

 */

void

drm_writeback_signal_completion(struct drm_writeback_connector *wb_connector)

drm_writeback_signal_completion(struct drm_writeback_connector *wb_connector,

				int status)

{

	unsigned long flags;

	struct drm_writeback_job *job;
@@ -232,8 +309,15 @@ drm_writeback_signal_completion(struct drm_writeback_connector *wb_connector) 
	job = list_first_entry_or_null(&wb_connector->job_queue,

				       struct drm_writeback_job,

				       list_entry);

	if (job)

	if (job) {

		list_del(&job->list_entry);

		if (job->out_fence) {

			if (status)

				dma_fence_set_error(job->out_fence, status);

			dma_fence_signal(job->out_fence);

			dma_fence_put(job->out_fence);

		}

	}

	spin_unlock_irqrestore(&wb_connector->job_lock, flags);



	if (WARN_ON(!job))
@@ -243,3 +327,24 @@ drm_writeback_signal_completion(struct drm_writeback_connector *wb_connector) 
	queue_work(system_long_wq, &job->cleanup_work);

}

EXPORT_SYMBOL(drm_writeback_signal_completion);



struct dma_fence *

drm_writeback_get_out_fence(struct drm_writeback_connector *wb_connector)

{

	struct dma_fence *fence;



	if (WARN_ON(wb_connector->base.connector_type !=

		    DRM_MODE_CONNECTOR_WRITEBACK))

		return NULL;



	fence = kzalloc(sizeof(*fence), GFP_KERNEL);

	if (!fence)

		return NULL;



	dma_fence_init(fence, &drm_writeback_fence_ops,

		       &wb_connector->fence_lock, wb_connector->fence_context,

		       ++wb_connector->fence_seqno);



	return fence;

}

EXPORT_SYMBOL(drm_writeback_get_out_fence);

include/drm/drm_atomic.h

+8 −0
Original line number Diff line number Diff line
@@ -160,6 +160,14 @@ struct __drm_crtcs_state { 
struct __drm_connnectors_state {

	struct drm_connector *ptr;

	struct drm_connector_state *state, *old_state, *new_state;

	/**

	 * @out_fence_ptr:

	 *

	 * User-provided pointer which the kernel uses to return a sync_file

	 * file descriptor. Used by writeback connectors to signal completion of

	 * the writeback.

	 */

	s32 __user *out_fence_ptr;

};



struct drm_private_obj;

include/drm/drm_connector.h

+4 −4
Original line number Diff line number Diff line
@@ -441,10 +441,10 @@ struct drm_connector_state { 
	/**

	 * @writeback_job: Writeback job for writeback connectors

	 *

	 * Holds the framebuffer for a writeback connector. As the writeback

	 * completion may be asynchronous to the normal commit cycle, the

	 * writeback job lifetime is managed separately from the normal atomic

	 * state by this object.

	 * Holds the framebuffer and out-fence for a writeback connector. As

	 * the writeback completion may be asynchronous to the normal commit

	 * cycle, the writeback job lifetime is managed separately from the

	 * normal atomic state by this object.

	 *

	 * See also: drm_writeback_queue_job() and

	 * drm_writeback_signal_completion()

include/drm/drm_mode_config.h

+8 −0
Original line number Diff line number Diff line
@@ -798,6 +798,14 @@ struct drm_mode_config { 
	 * See also: drm_writeback_connector_init()

	 */

	struct drm_property *writeback_pixel_formats_property;

	/**

	 * @writeback_out_fence_ptr_property: Property for writeback connectors,

	 * fd pointer representing the outgoing fences for a writeback

	 * connector. Userspace should provide a pointer to a value of type s32,

	 * and then cast that pointer to u64.

	 * See also: drm_writeback_connector_init()

	 */

	struct drm_property *writeback_out_fence_ptr_property;



	/* dumb ioctl parameters */

	uint32_t preferred_depth, prefer_shadow;