[PATCH 00/19] atomic, remixed

[PATCH 00/19] atomic, remixed

[Daniel Vetter]

Hi all,

So I've figured instead of just talking I should draw up my ideas for atomic
helpers and related stuff in some draft patches. Major differences compared to
Rob's series:

- The software side state update is done synchronously, at least when using the
  helper code. Drivers are free to do whatever they want, as usual. This has a
  bunch of nice upsides:

  + All the tricky locking dances can be abolished, and if the ordering and
    synchronization is done correctly async worker threads don't need any
    modeset state locking at all. I'm fairly sure that Rob's scheme won't blow
    up, but I much prefer we don't need it at all.

  + In Rob's patches any atomic operation must wait for outstanding updates to
    complete. Otherwise the software state isn't committed yet. With the
    synchronous updates we can immediately proceed with the state construction
    and verification and will only block right before applying the state update.
    And even for that drivers can implement cancellation of the previous update.
    
    This has the nice benefit that check-only operations (which compositors need
    to do right after having completed the current frame to know how to render
    the next one) can be done without blocking for completion of the previous
    update.

- Internal interfaces for atomic updates don't need to go through properties,
  but can directly set the state. Drivers can always compare updates in their
  ->check hook later on, so we don't lose expressiveness in the interface. But
  the resulting code in the callers looks much better.

  In general I've ditched a lot of interfaces where drivers could overwrite
  default behaviour and boiled down the interface two state handling functions
  (duplicate+destroy), setting driver-private properties and check/commit.

- There is a clear helper separation between core code which should be used to
  use the atomic interface to drivers, and helper functions. Core never uses
  helper functions so that we can keep the clear separation we have in all other
  places in the kms api.

  There's a set of helpers sitting in-between to implement legacy interfaces on
  top of atomic. Those _only_ use the core atomic interfaces.

- I've added a new set of plane helpers. Mostly this was motivated to make the
  atomic helpers work on less stellar hardware where you can't just stream the
  state and then atomically commit with some GO bits. But those helpers should
  also be useful for more gradual transitions of drivers to the atomic
  interface. Since a requirement for atomic is to have universal plane support
  they can be used bare-bones just for that, without all the other atomic
  baggage (i.e. all the state tracking for crtcs/connectors).

- State tracking for connectors. i915 has piles of relevant connector
  properties, which we want to save/restore in atomic ops (e.g. hdmi metadata
  and stuff).

- fbdev panic locking is implemented with trylock instead of nolock.

- I've thought a bit about resume. See the proposed state handling for the
  default reset functions and the addition of a plane reset callback.

There's also a metric pile of stuff missing:

- It's completely untested (except for the parts touching currently used code)
  since I lack suitable hw. Converting i915 is just a bit too much of a pain for
  a quick w/e code draft ;-)

- I didn't add all the properties for things currently exposed through ioctl
  structures. Since the interface here doesn't force internal code to use
  properties for everything (which really makes things look much better) we
  don't have a need for that until we merge the actual atomic ioctl. So I left
  that out.

- I didn't write code for the fbdev helper to use atomic. The w/e ran out of
  time for that ...

- Kerneldoc is occasionally not updated or still missing.

Anyway I think this draft code is good enough to explain how I'd like to address
some of the concerns I have with the current atomic code. Comments, flames and
ideas highly welcome. And if anyone is insane enought to try this on real
hardware, that would certainly be interesting and I'm very much willing to help
out as much as possible ... i915 just really isn't suitable since it won't be
using the helpers, and all the other hw I have here doesn't support planes.

Cheers, Daniel

Daniel Vetter (18):
  drm: Add atomic driver interface definitions for objects
  drm: Add drm_plane/connector_index
  drm: Move modeset_lock_all helpers to drm_modeset_lock.[hc]
  drm: Handle legacy per-crtc locking with full acquire ctx
  drm: Global atomic state handling
  drm: Add atomic/plane helpers
  drm/irq: Implement a generic vblank_wait function
  drm/i915: Use generic vblank wait
  drm/plane-helper: transitional atomic plane helpers
  drm/crtc-helper: Transitional functions using atomic plane helpers
  drm: Atomic crtc/connector updates using crtc helper interfaces
  drm: Move ->old_fb from crtc to plane
  drm/atomic-helper: implementatations for legacy interfaces
  drm/plane-helper: Add async mode to prepare_fb
  drm/atomic-helpers: document how to implement async commit
  drm/atomic-helper: implement ->page_flip
  drm: trylock modest locking for fbdev panics
  drm/atomic-helpers: functions for state duplicate/destroy/reset

Ville Syrjälä (1):
  drm: Add drm_crtc_vblank_waitqueue()

 Documentation/DocBook/drm.tmpl       |    1 +
 drivers/gpu/drm/Makefile             |    4 +-
 drivers/gpu/drm/drm_atomic.c         |  529 ++++++++++++
 drivers/gpu/drm/drm_atomic_helper.c  | 1570 ++++++++++++++++++++++++++++++++++
 drivers/gpu/drm/drm_crtc.c           |  194 ++---
 drivers/gpu/drm/drm_crtc_helper.c    |  139 +++
 drivers/gpu/drm/drm_fb_helper.c      |   10 +-
 drivers/gpu/drm/drm_irq.c            |   45 +
 drivers/gpu/drm/drm_modeset_lock.c   |  204 ++++-
 drivers/gpu/drm/drm_plane_helper.c   |  181 +++-
 drivers/gpu/drm/i915/intel_display.c |   41 +-
 include/drm/drmP.h                   |   13 +
 include/drm/drm_atomic.h             |   63 ++
 include/drm/drm_atomic_helper.h      |   98 +++
 include/drm/drm_crtc.h               |  208 ++++-
 include/drm/drm_crtc_helper.h        |   13 +
 include/drm/drm_modeset_lock.h       |   16 +
 include/drm/drm_plane_helper.h       |   31 +
 18 files changed, 3188 insertions(+), 172 deletions(-)
 create mode 100644 drivers/gpu/drm/drm_atomic.c
 create mode 100644 drivers/gpu/drm/drm_atomic_helper.c
 create mode 100644 include/drm/drm_atomic.h
 create mode 100644 include/drm/drm_atomic_helper.h

-- 
2.0.1

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

[PATCH 01/19] drm: Add atomic driver interface definitions for objects

[Daniel Vetter]

Heavily based upon Rob Clark's atomic series.
- Dropped the connctor state from the crtc state, instead opting for a
  full-blown connector state. The only thing it has is the desired
  crtc, but drivers which have connector properties have now a
  data-structure to subclass.

- Rename create_state to duplicate_state. Especially for legacy ioctls
  we want updates on top of existing state, so we need a way to get at
  the current state. We need to be careful to clear the backpointers
  correctly though. We need to be careful though with the ->state
  backpointer we need while constructing the atomic state.

- Drop property values. Drivers with properties simply need to
  subclass the datastructures and track the decoded values in there. I
  also think that common properties (like rotation) should be decoded
  and stored in the core structures.

- Create a new set of ->atomic_set_prop functions, for smoother
  transitions from legacy to atomic operations.

- Pass the ->atomic_set_prop ioctl the right structure to avoid
  chasing pointers in drivers.

- Drop temporary boolean state for now until we resurrect them with
  the helper functions.

- Drop invert_dimensions. For now we don't need any checking since
  that's done by the higher-level legacy ioctls. But even then we
  should also add rotation/flip tracking to the core drm_crtc_state,
  not just whether the dimensions are inverted.

- Track crtc state with an enable/disable. That's equivalent to
  mode_valid, but a bit clearer that it means the entire crtc.

The global interface will follow in subsequent patches.

v2: We need to allow drivers to somehow set up the initial state and
clear it on resume. So add a plane->reset callback for that. Helpers
will be provided with default behaviour for all these.

Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/drm_crtc.c |   5 ++
 include/drm/drm_crtc.h     | 149 +++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 154 insertions(+)

diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
index 805240b11229..0f934a58702d 100644
--- a/drivers/gpu/drm/drm_crtc.c
+++ b/drivers/gpu/drm/drm_crtc.c
@@ -4623,9 +4623,14 @@ out:
 void drm_mode_config_reset(struct drm_device *dev)
 {
 	struct drm_crtc *crtc;
+	struct drm_crtc *plane;
 	struct drm_encoder *encoder;
 	struct drm_connector *connector;
 
+	list_for_each_entry(plane, &dev->mode_config.plane_list, head)
+		if (plane->funcs->reset)
+			plane->funcs->reset(plane);
+
 	list_for_each_entry(crtc, &dev->mode_config.crtc_list, head)
 		if (crtc->funcs->reset)
 			crtc->funcs->reset(crtc);
diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
index f1105d0da059..a1a8c694e765 100644
--- a/include/drm/drm_crtc.h
+++ b/include/drm/drm_crtc.h
@@ -228,6 +228,25 @@ struct drm_encoder;
 struct drm_pending_vblank_event;
 struct drm_plane;
 struct drm_bridge;
+struct drm_atomic_state;
+
+/**
+ * drm_crtc_state - mutable crtc state
+ * @mode_valid: a valid mode has been set
+ * @set_config: needs modeset (crtc->set_config())
+ * @connectors_change: the connector-ids array has changed
+ * @mode: current mode timings
+ * @event: pending pageflip event
+ */
+struct drm_crtc_state {
+	bool enable        : 1;
+
+	struct drm_display_mode mode;
+
+	struct drm_pending_vblank_event *event;
+
+	struct drm_atomic_state *state;
+};
 
 /**
  * drm_crtc_funcs - control CRTCs for a given device
@@ -291,6 +310,15 @@ struct drm_crtc_funcs {
 
 	int (*set_property)(struct drm_crtc *crtc,
 			    struct drm_property *property, uint64_t val);
+
+	/* atomic update handling */
+	struct drm_crtc_state *(*atomic_duplicate_state)(struct drm_crtc *crtc);
+	void (*atomic_destroy_state)(struct drm_crtc *crtc,
+				     struct drm_crtc_state *cstate);
+	int (*atomic_set_property)(struct drm_crtc *crtc,
+				   struct drm_crtc_state *state,
+				   struct drm_property *property,
+				   uint64_t val);
 };
 
 /**
@@ -375,8 +403,38 @@ struct drm_crtc {
 	void *helper_private;
 
 	struct drm_object_properties properties;
+
+	struct drm_crtc_state *state;
 };
 
+static inline struct drm_crtc_state *
+drm_crtc_duplicate_state(struct drm_crtc *crtc)
+{
+	if (crtc->funcs->atomic_duplicate_state)
+		return crtc->funcs->atomic_duplicate_state(crtc);
+	return kmemdup(crtc->state, sizeof(struct drm_crtc_state), GFP_KERNEL);
+}
+
+static inline void
+drm_crtc_destroy_state(struct drm_crtc *crtc,
+		       struct drm_crtc_state *cstate)
+{
+	if (crtc->funcs->atomic_destroy_state)
+		crtc->funcs->atomic_destroy_state(crtc, cstate);
+	else
+		kfree(cstate);
+}
+
+
+/**
+ * drm_connector_state - mutable connector state
+ * @crtc: crtc to connect connector to, NULL if disabled
+ */
+struct drm_connector_state {
+	struct drm_crtc *crtc;
+
+	struct drm_atomic_state *state;
+};
 
 /**
  * drm_connector_funcs - control connectors on a given device
@@ -413,6 +471,15 @@ struct drm_connector_funcs {
 			     uint64_t val);
 	void (*destroy)(struct drm_connector *connector);
 	void (*force)(struct drm_connector *connector);
+
+	/* atomic update handling */
+	struct drm_connector_state *(*atomic_duplicate_state)(struct drm_connector *connector);
+	void (*atomic_destroy_state)(struct drm_connector *connector,
+				     struct drm_connector_state *cstate);
+	int (*atomic_set_property)(struct drm_connector *connector,
+				   struct drm_connector_state *state,
+				   struct drm_property *property,
+				   uint64_t val);
 };
 
 /**
@@ -564,8 +631,59 @@ struct drm_connector {
 	unsigned bad_edid_counter;
 
 	struct dentry *debugfs_entry;
+
+	struct drm_connector_state *state;
 };
 
+static inline struct drm_connector_state *
+drm_connector_duplicate_state(struct drm_connector *connector)
+{
+	if (connector->funcs->atomic_duplicate_state)
+		return connector->funcs->atomic_duplicate_state(connector);
+	return kmemdup(connector->state,
+		       sizeof(struct drm_connector_state), GFP_KERNEL);
+}
+
+static inline void
+drm_connector_destroy_state(struct drm_connector *connector,
+			    struct drm_connector_state *cstate)
+{
+	if (connector->funcs->atomic_destroy_state)
+		connector->funcs->atomic_destroy_state(connector, cstate);
+	else
+		kfree(cstate);
+}
+
+/**
+ * drm_plane_state - mutable plane state
+ * @crtc: currently bound CRTC
+ * @fb: currently bound fb
+ * @crtc_x: left position of visible portion of plane on crtc
+ * @crtc_y: upper position of visible portion of plane on crtc
+ * @crtc_w: width of visible portion of plane on crtc
+ * @crtc_h: height of visible portion of plane on crtc
+ * @src_x: left position of visible portion of plane within
+ *	plane (in 16.16)
+ * @src_y: upper position of visible portion of plane within
+ *	plane (in 16.16)
+ * @src_w: width of visible portion of plane (in 16.16)
+ * @src_h: height of visible portion of plane (in 16.16)
+ */
+struct drm_plane_state {
+	struct drm_crtc *crtc;
+	struct drm_framebuffer *fb;
+
+	/* Signed dest location allows it to be partially off screen */
+	int32_t crtc_x, crtc_y;
+	uint32_t crtc_w, crtc_h;
+
+	/* Source values are 16.16 fixed point */
+	uint32_t src_x, src_y;
+	uint32_t src_h, src_w;
+	struct drm_atomic_state *state;
+};
+
+
 /**
  * drm_plane_funcs - driver plane control functions
  * @update_plane: update the plane configuration
@@ -582,9 +700,19 @@ struct drm_plane_funcs {
 			    uint32_t src_w, uint32_t src_h);
 	int (*disable_plane)(struct drm_plane *plane);
 	void (*destroy)(struct drm_plane *plane);
+	void (*reset)(struct drm_plane *plane);
 
 	int (*set_property)(struct drm_plane *plane,
 			    struct drm_property *property, uint64_t val);
+
+	/* atomic update handling */
+	struct drm_plane_state *(*atomic_duplicate_state)(struct drm_plane *plane);
+	void (*atomic_destroy_state)(struct drm_plane *plane,
+				     struct drm_plane_state *cstate);
+	int (*atomic_set_property)(struct drm_plane *plane,
+				   struct drm_plane_state *state,
+				   struct drm_property *property,
+				   uint64_t val);
 };
 
 enum drm_plane_type {
@@ -625,8 +753,29 @@ struct drm_plane {
 	struct drm_object_properties properties;
 
 	enum drm_plane_type type;
+
+	struct drm_plane_state *state;
 };
 
+static inline struct drm_plane_state *
+drm_plane_duplicate_state(struct drm_plane *plane)
+{
+	if (plane->funcs->atomic_duplicate_state)
+		return plane->funcs->atomic_duplicate_state(plane);
+	return kmemdup(plane->state, sizeof(struct drm_plane_state), GFP_KERNEL);
+}
+
+static inline void
+drm_plane_destroy_state(struct drm_plane *plane,
+			struct drm_plane_state *cstate)
+{
+	if (plane->funcs->atomic_destroy_state)
+		plane->funcs->atomic_destroy_state(plane, cstate);
+	else
+		kfree(cstate);
+}
+
+
 /**
  * drm_bridge_funcs - drm_bridge control functions
  * @mode_fixup: Try to fixup (or reject entirely) proposed mode for this bridge
-- 
2.0.1

[PATCH 02/19] drm: Add drm_plane/connector_index

[Daniel Vetter]

In the atomic state we'll have an array of states for crtcs, planes
and connectors and need to be able to at them by their index. We
already have a drm_crtc_index function so add the missing ones for
planes and connectors.

If it later on turns out that the list walking is too expensive we can
add the index to the relevant modeset objects.

Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/drm_crtc.c | 46 ++++++++++++++++++++++++++++++++++++++++++++++
 include/drm/drm_crtc.h     |  3 +++
 2 files changed, 49 insertions(+)

diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
index 0f934a58702d..fdea713767b4 100644
--- a/drivers/gpu/drm/drm_crtc.c
+++ b/drivers/gpu/drm/drm_crtc.c
@@ -937,6 +937,29 @@ void drm_connector_cleanup(struct drm_connector *connector)
 EXPORT_SYMBOL(drm_connector_cleanup);
 
 /**
+ * drm_plane_index - find the index of a registered CRTC
+ * @plane: CRTC to find index for
+ *
+ * Given a registered CRTC, return the index of that CRTC within a DRM
+ * device's list of CRTCs.
+ */
+unsigned int drm_connector_index(struct drm_connector *connector)
+{
+	unsigned int index = 0;
+	struct drm_connector *tmp;
+
+	list_for_each_entry(tmp, &connector->dev->mode_config.connector_list, head) {
+		if (tmp == connector)
+			return index;
+
+		index++;
+	}
+
+	BUG();
+}
+EXPORT_SYMBOL(drm_connector_index);
+
+/**
  * drm_connector_register - register a connector
  * @connector: the connector to register
  *
@@ -1239,6 +1262,29 @@ void drm_plane_cleanup(struct drm_plane *plane)
 EXPORT_SYMBOL(drm_plane_cleanup);
 
 /**
+ * drm_plane_index - find the index of a registered CRTC
+ * @plane: CRTC to find index for
+ *
+ * Given a registered CRTC, return the index of that CRTC within a DRM
+ * device's list of CRTCs.
+ */
+unsigned int drm_plane_index(struct drm_plane *plane)
+{
+	unsigned int index = 0;
+	struct drm_plane *tmp;
+
+	list_for_each_entry(tmp, &plane->dev->mode_config.plane_list, head) {
+		if (tmp == plane)
+			return index;
+
+		index++;
+	}
+
+	BUG();
+}
+EXPORT_SYMBOL(drm_plane_index);
+
+/**
  * drm_plane_force_disable - Forcibly disable a plane
  * @plane: plane to disable
  *
diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
index a1a8c694e765..d6546fd1e00c 100644
--- a/include/drm/drm_crtc.h
+++ b/include/drm/drm_crtc.h
@@ -680,6 +680,7 @@ struct drm_plane_state {
 	/* Source values are 16.16 fixed point */
 	uint32_t src_x, src_y;
 	uint32_t src_h, src_w;
+
 	struct drm_atomic_state *state;
 };
 
@@ -1052,6 +1053,7 @@ int drm_connector_register(struct drm_connector *connector);
 void drm_connector_unregister(struct drm_connector *connector);
 
 extern void drm_connector_cleanup(struct drm_connector *connector);
+extern unsigned int drm_connector_index(struct drm_connector *crtc);
 /* helper to unplug all connectors from sysfs for device */
 extern void drm_connector_unplug_all(struct drm_device *dev);
 
@@ -1091,6 +1093,7 @@ extern int drm_plane_init(struct drm_device *dev,
 			  const uint32_t *formats, uint32_t format_count,
 			  bool is_primary);
 extern void drm_plane_cleanup(struct drm_plane *plane);
+extern unsigned int drm_plane_index(struct drm_plane *crtc);
 extern void drm_plane_force_disable(struct drm_plane *plane);
 extern int drm_crtc_check_viewport(const struct drm_crtc *crtc,
 				   int x, int y,
-- 
2.0.1

[PATCH 03/19] drm: Move modeset_lock_all helpers to drm_modeset_lock.[hc]

[Daniel Vetter]

Somehow we've forgotten about this little bit of OCD.

Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/drm_crtc.c         | 95 --------------------------------------
 drivers/gpu/drm/drm_modeset_lock.c | 95 ++++++++++++++++++++++++++++++++++++++
 include/drm/drm_crtc.h             |  4 --
 include/drm/drm_modeset_lock.h     |  5 ++
 4 files changed, 100 insertions(+), 99 deletions(-)

diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
index fdea713767b4..acb3b3121f67 100644
--- a/drivers/gpu/drm/drm_crtc.c
+++ b/drivers/gpu/drm/drm_crtc.c
@@ -45,101 +45,6 @@ static struct drm_framebuffer *add_framebuffer_internal(struct drm_device *dev,
 							struct drm_mode_fb_cmd2 *r,
 							struct drm_file *file_priv);
 
-/**
- * drm_modeset_lock_all - take all modeset locks
- * @dev: drm device
- *
- * This function takes all modeset locks, suitable where a more fine-grained
- * scheme isn't (yet) implemented. Locks must be dropped with
- * drm_modeset_unlock_all.
- */
-void drm_modeset_lock_all(struct drm_device *dev)
-{
-	struct drm_mode_config *config = &dev->mode_config;
-	struct drm_modeset_acquire_ctx *ctx;
-	int ret;
-
-	ctx = kzalloc(sizeof(*ctx), GFP_KERNEL);
-	if (WARN_ON(!ctx))
-		return;
-
-	mutex_lock(&config->mutex);
-
-	drm_modeset_acquire_init(ctx, 0);
-
-retry:
-	ret = drm_modeset_lock(&config->connection_mutex, ctx);
-	if (ret)
-		goto fail;
-	ret = drm_modeset_lock_all_crtcs(dev, ctx);
-	if (ret)
-		goto fail;
-
-	WARN_ON(config->acquire_ctx);
-
-	/* now we hold the locks, so now that it is safe, stash the
-	 * ctx for drm_modeset_unlock_all():
-	 */
-	config->acquire_ctx = ctx;
-
-	drm_warn_on_modeset_not_all_locked(dev);
-
-	return;
-
-fail:
-	if (ret == -EDEADLK) {
-		drm_modeset_backoff(ctx);
-		goto retry;
-	}
-}
-EXPORT_SYMBOL(drm_modeset_lock_all);
-
-/**
- * drm_modeset_unlock_all - drop all modeset locks
- * @dev: device
- *
- * This function drop all modeset locks taken by drm_modeset_lock_all.
- */
-void drm_modeset_unlock_all(struct drm_device *dev)
-{
-	struct drm_mode_config *config = &dev->mode_config;
-	struct drm_modeset_acquire_ctx *ctx = config->acquire_ctx;
-
-	if (WARN_ON(!ctx))
-		return;
-
-	config->acquire_ctx = NULL;
-	drm_modeset_drop_locks(ctx);
-	drm_modeset_acquire_fini(ctx);
-
-	kfree(ctx);
-
-	mutex_unlock(&dev->mode_config.mutex);
-}
-EXPORT_SYMBOL(drm_modeset_unlock_all);
-
-/**
- * drm_warn_on_modeset_not_all_locked - check that all modeset locks are locked
- * @dev: device
- *
- * Useful as a debug assert.
- */
-void drm_warn_on_modeset_not_all_locked(struct drm_device *dev)
-{
-	struct drm_crtc *crtc;
-
-	/* Locking is currently fubar in the panic handler. */
-	if (oops_in_progress)
-		return;
-
-	list_for_each_entry(crtc, &dev->mode_config.crtc_list, head)
-		WARN_ON(!drm_modeset_is_locked(&crtc->mutex));
-
-	WARN_ON(!drm_modeset_is_locked(&dev->mode_config.connection_mutex));
-	WARN_ON(!mutex_is_locked(&dev->mode_config.mutex));
-}
-EXPORT_SYMBOL(drm_warn_on_modeset_not_all_locked);
-
 /* Avoid boilerplate.  I'm tired of typing. */
 #define DRM_ENUM_NAME_FN(fnname, list)				\
 	const char *fnname(int val)				\
diff --git a/drivers/gpu/drm/drm_modeset_lock.c b/drivers/gpu/drm/drm_modeset_lock.c
index 0dc57d5ecd10..73e6534fd0aa 100644
--- a/drivers/gpu/drm/drm_modeset_lock.c
+++ b/drivers/gpu/drm/drm_modeset_lock.c
@@ -57,6 +57,101 @@
 
 
 /**
+ * drm_modeset_lock_all - take all modeset locks
+ * @dev: drm device
+ *
+ * This function takes all modeset locks, suitable where a more fine-grained
+ * scheme isn't (yet) implemented. Locks must be dropped with
+ * drm_modeset_unlock_all.
+ */
+void drm_modeset_lock_all(struct drm_device *dev)
+{
+	struct drm_mode_config *config = &dev->mode_config;
+	struct drm_modeset_acquire_ctx *ctx;
+	int ret;
+
+	ctx = kzalloc(sizeof(*ctx), GFP_KERNEL);
+	if (WARN_ON(!ctx))
+		return;
+
+	mutex_lock(&config->mutex);
+
+	drm_modeset_acquire_init(ctx, 0);
+
+retry:
+	ret = drm_modeset_lock(&config->connection_mutex, ctx);
+	if (ret)
+		goto fail;
+	ret = drm_modeset_lock_all_crtcs(dev, ctx);
+	if (ret)
+		goto fail;
+
+	WARN_ON(config->acquire_ctx);
+
+	/* now we hold the locks, so now that it is safe, stash the
+	 * ctx for drm_modeset_unlock_all():
+	 */
+	config->acquire_ctx = ctx;
+
+	drm_warn_on_modeset_not_all_locked(dev);
+
+	return;
+
+fail:
+	if (ret == -EDEADLK) {
+		drm_modeset_backoff(ctx);
+		goto retry;
+	}
+}
+EXPORT_SYMBOL(drm_modeset_lock_all);
+
+/**
+ * drm_modeset_unlock_all - drop all modeset locks
+ * @dev: device
+ *
+ * This function drop all modeset locks taken by drm_modeset_lock_all.
+ */
+void drm_modeset_unlock_all(struct drm_device *dev)
+{
+	struct drm_mode_config *config = &dev->mode_config;
+	struct drm_modeset_acquire_ctx *ctx = config->acquire_ctx;
+
+	if (WARN_ON(!ctx))
+		return;
+
+	config->acquire_ctx = NULL;
+	drm_modeset_drop_locks(ctx);
+	drm_modeset_acquire_fini(ctx);
+
+	kfree(ctx);
+
+	mutex_unlock(&dev->mode_config.mutex);
+}
+EXPORT_SYMBOL(drm_modeset_unlock_all);
+
+/**
+ * drm_warn_on_modeset_not_all_locked - check that all modeset locks are locked
+ * @dev: device
+ *
+ * Useful as a debug assert.
+ */
+void drm_warn_on_modeset_not_all_locked(struct drm_device *dev)
+{
+	struct drm_crtc *crtc;
+
+	/* Locking is currently fubar in the panic handler. */
+	if (oops_in_progress)
+		return;
+
+	list_for_each_entry(crtc, &dev->mode_config.crtc_list, head)
+		WARN_ON(!drm_modeset_is_locked(&crtc->mutex));
+
+	WARN_ON(!drm_modeset_is_locked(&dev->mode_config.connection_mutex));
+	WARN_ON(!mutex_is_locked(&dev->mode_config.mutex));
+}
+EXPORT_SYMBOL(drm_warn_on_modeset_not_all_locked);
+
+/**
  * drm_modeset_acquire_init - initialize acquire context
  * @ctx: the acquire context
  * @flags: for future
diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
index d6546fd1e00c..bd305ce7b705 100644
--- a/include/drm/drm_crtc.h
+++ b/include/drm/drm_crtc.h
@@ -218,10 +218,6 @@ struct drm_property {
 	struct list_head enum_blob_list;
 };
 
-void drm_modeset_lock_all(struct drm_device *dev);
-void drm_modeset_unlock_all(struct drm_device *dev);
-void drm_warn_on_modeset_not_all_locked(struct drm_device *dev);
-
 struct drm_crtc;
 struct drm_connector;
 struct drm_encoder;
diff --git a/include/drm/drm_modeset_lock.h b/include/drm/drm_modeset_lock.h
index 402aa7a6a058..cf61e857bc06 100644
--- a/include/drm/drm_modeset_lock.h
+++ b/include/drm/drm_modeset_lock.h
@@ -120,6 +120,11 @@ int drm_modeset_lock_interruptible(struct drm_modeset_lock *lock,
 void drm_modeset_unlock(struct drm_modeset_lock *lock);
 
 struct drm_device;
+
+void drm_modeset_lock_all(struct drm_device *dev);
+void drm_modeset_unlock_all(struct drm_device *dev);
+void drm_warn_on_modeset_not_all_locked(struct drm_device *dev);
+
 int drm_modeset_lock_all_crtcs(struct drm_device *dev,
 		struct drm_modeset_acquire_ctx *ctx);
 
-- 
2.0.1

[PATCH 04/19] drm: Handle legacy per-crtc locking with full acquire ctx

[Daniel Vetter]

So drivers using the atomic interfaces expect that they can acquire
additional locks internal to the driver as-needed. Examples would be
locks to protect shared state like shared display PLLs.

Unfortunately the legacy ioctls assume that all locking is fully done
by the drm core. Now for those paths which grab all locks we already
have to keep around an acquire context in dev->mode_config. Helper
functions that implement legacy interfaces in terms of atomic support
can therefore grab this acquire contexts and reuse it.

The only interfaces left are the cursor and pageflip ioctls. So add
functions to grab the crtc lock these need using an acquire context
and preserve it for atomic drivers to reuse.

Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/drm_crtc.c         |  8 ++--
 drivers/gpu/drm/drm_modeset_lock.c | 86 ++++++++++++++++++++++++++++++++++++++
 include/drm/drm_crtc.h             |  6 +++
 include/drm/drm_modeset_lock.h     |  5 +++
 4 files changed, 101 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
index acb3b3121f67..e33aa3a3a8a5 100644
--- a/drivers/gpu/drm/drm_crtc.c
+++ b/drivers/gpu/drm/drm_crtc.c
@@ -2714,7 +2714,7 @@ static int drm_mode_cursor_common(struct drm_device *dev,
 	if (crtc->cursor)
 		return drm_mode_cursor_universal(crtc, req, file_priv);
 
-	drm_modeset_lock(&crtc->mutex, NULL);
+	drm_modeset_lock_crtc(crtc);
 	if (req->flags & DRM_MODE_CURSOR_BO) {
 		if (!crtc->funcs->cursor_set && !crtc->funcs->cursor_set2) {
 			ret = -ENXIO;
@@ -2738,7 +2738,7 @@ static int drm_mode_cursor_common(struct drm_device *dev,
 		}
 	}
 out:
-	drm_modeset_unlock(&crtc->mutex);
+	drm_modeset_unlock_crtc(crtc);
 
 	return ret;
 
@@ -4474,7 +4474,7 @@ int drm_mode_page_flip_ioctl(struct drm_device *dev,
 	if (!crtc)
 		return -ENOENT;
 
-	drm_modeset_lock(&crtc->mutex, NULL);
+	drm_modeset_lock_crtc(crtc);
 	if (crtc->primary->fb == NULL) {
 		/* The framebuffer is currently unbound, presumably
 		 * due to a hotplug event, that userspace has not
@@ -4558,7 +4558,7 @@ out:
 		drm_framebuffer_unreference(fb);
 	if (old_fb)
 		drm_framebuffer_unreference(old_fb);
-	drm_modeset_unlock(&crtc->mutex);
+	drm_modeset_unlock_crtc(crtc);
 
 	return ret;
 }
diff --git a/drivers/gpu/drm/drm_modeset_lock.c b/drivers/gpu/drm/drm_modeset_lock.c
index 73e6534fd0aa..412b396af740 100644
--- a/drivers/gpu/drm/drm_modeset_lock.c
+++ b/drivers/gpu/drm/drm_modeset_lock.c
@@ -130,6 +130,90 @@ void drm_modeset_unlock_all(struct drm_device *dev)
 EXPORT_SYMBOL(drm_modeset_unlock_all);
 
 /**
+ * drm_modeset_lock_crtc - lock crtc with hidden acquire ctx
+ * @crtc: drm crtc
+ *
+ * This function locks the given crtc using a hidden acquire context. This is
+ * necessary so that drivers internally using the atomic interfaces can grab
+ * furether locks with the lock acquire context.
+ */
+void drm_modeset_lock_crtc(struct drm_crtc *crtc)
+{
+	struct drm_modeset_acquire_ctx *ctx;
+	int ret;
+
+	ctx = kzalloc(sizeof(*ctx), GFP_KERNEL);
+	if (WARN_ON(!ctx))
+		return;
+
+	drm_modeset_acquire_init(ctx, 0);
+
+retry:
+	ret = drm_modeset_lock(&crtc->mutex, ctx);
+	if (ret)
+		goto fail;
+
+	WARN_ON(crtc->acquire_ctx);
+
+	/* now we hold the locks, so now that it is safe, stash the
+	 * ctx for drm_modeset_unlock_all():
+	 */
+	crtc->acquire_ctx = ctx;
+
+	return;
+
+fail:
+	if (ret == -EDEADLK) {
+		drm_modeset_backoff(ctx);
+		goto retry;
+	}
+}
+EXPORT_SYMBOL(drm_modeset_lock_crtc);
+
+/**
+ * drm_modeset_legacy_acquire_ctx - find acquire ctx for legacy ioctls
+ * crtc: drm crtc
+ *
+ * Legacy ioctl operations like cursor updates or page flips only have per-crtc
+ * locking, and store the acquire ctx in the corresponding crtc. All other
+ * legacy operations take all locks and use a global acquire context. This
+ * function grabs the right one.
+ */
+struct drm_modeset_acquire_ctx *
+drm_modeset_legacy_acquire_ctx(struct drm_crtc *crtc)
+{
+	if (crtc->acquire_ctx)
+		return crtc->acquire_ctx;
+
+	WARN_ON(!crtc->dev->mode_config.acquire_ctx);
+
+	return crtc->dev->mode_config.acquire_ctx;
+}
+EXPORT_SYMBOL(drm_modeset_legacy_acquire_ctx);
+
+/**
+ * drm_modeset_unlock_crtc - drop all modeset locks
+ * @crtc: drm crtc
+ *
+ * This drops the crtc lock acquire with drm_modeset_lock_crtc() and all other
+ * locks acquired through the hidden context.
+ */
+void drm_modeset_unlock_crtc(struct drm_crtc *crtc)
+{
+	struct drm_modeset_acquire_ctx *ctx = crtc->acquire_ctx;
+
+	if (WARN_ON(!ctx))
+		return;
+
+	crtc->acquire_ctx = NULL;
+	drm_modeset_drop_locks(ctx);
+	drm_modeset_acquire_fini(ctx);
+
+	kfree(ctx);
+}
+EXPORT_SYMBOL(drm_modeset_unlock_crtc);
+
+/**
  * drm_warn_on_modeset_not_all_locked - check that all modeset locks are locked
  * @dev: device
  *
@@ -336,6 +420,8 @@ int drm_modeset_lock_all_crtcs(struct drm_device *dev,
 		ret = drm_modeset_lock(&crtc->mutex, ctx);
 		if (ret)
 			return ret;
+
+		WARN_ON(crtc->acquire_ctx);
 	}
 
 	return 0;
diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
index bd305ce7b705..39465d716db6 100644
--- a/include/drm/drm_crtc.h
+++ b/include/drm/drm_crtc.h
@@ -401,6 +401,12 @@ struct drm_crtc {
 	struct drm_object_properties properties;
 
 	struct drm_crtc_state *state;
+
+	/*
+	 * For legacy crtc ioctls so that atomic drivers can get at the locking
+	 * acquire context.
+	 */
+	struct drm_modeset_acquire_ctx *acquire_ctx;
 };
 
 static inline struct drm_crtc_state *
diff --git a/include/drm/drm_modeset_lock.h b/include/drm/drm_modeset_lock.h
index cf61e857bc06..d38e1508f11a 100644
--- a/include/drm/drm_modeset_lock.h
+++ b/include/drm/drm_modeset_lock.h
@@ -120,10 +120,15 @@ int drm_modeset_lock_interruptible(struct drm_modeset_lock *lock,
 void drm_modeset_unlock(struct drm_modeset_lock *lock);
 
 struct drm_device;
+struct drm_crtc;
 
 void drm_modeset_lock_all(struct drm_device *dev);
 void drm_modeset_unlock_all(struct drm_device *dev);
+void drm_modeset_lock_crtc(struct drm_crtc *crtc);
+void drm_modeset_unlock_crtc(struct drm_crtc *crtc);
 void drm_warn_on_modeset_not_all_locked(struct drm_device *dev);
+struct drm_modeset_acquire_ctx *
+drm_modeset_legacy_acquire_ctx(struct drm_crtc *crtc);
 
 int drm_modeset_lock_all_crtcs(struct drm_device *dev,
 		struct drm_modeset_acquire_ctx *ctx);
-- 
2.0.1

[PATCH 05/19] drm: Global atomic state handling

[Daniel Vetter]

Some differences compared to Rob's patches again:
- Dropped the committed and checked booleans. Checking will be
  internally enforced by always calling ->atomic_check before
  ->atomic_commit. And async handling needs to be solved differently
  because the current scheme completely side-steps ww mutex deadlock
  avoidance (and so either reinvents a new deadlock avoidance wheel or
  like the current code just deadlocks).

- State for connectors needed to be added, since now they have a
  full-blown drm_connector_state (so that drivers have something to
  attach their own stuff to).

- Refcounting is gone. I plane to solve async updates differently,
  since the lock-passing scheme doesn't cut it (since it abuses ww
  mutexes). Essentially what we need for async is a simple ownership
  transfer from the caller to the driver. That doesn't need full-blown
  refcounting.

- The acquire ctx is a pointer. Real atomic callers should have that
  on their stack, legacy entry points need to put the right one
  (obtained by drm_modeset_legacy_acuire_ctx) in there.

- I've dropped all hooks except check/commit. All the begin/end
  handling is done by core functions and is the same.

- commit/check are just thin wrappers that ensure that ->check is
  always called.

- To help out with locking in the legacy implementations I've added a
  helper to just grab all locks in the backoff case.

v2: Add notices that check/commit can fail with EDEADLK.

v3:
- More consistent naming for state_alloc.
- Add state_clear which is needed for backoff and retry.

v4: Planes/connectors can switch between crtcs, and we need to be
careful that we grab the state (and locks) for both the old and new
crtc. Improve the interface functions to ensure this.

v5: Add functions to grab affected connectors for a crtc and to recompute
the crtc->enable state. This is useful for both helper and atomic ioctl
code when e.g. removing a connector.

Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/Makefile     |   2 +-
 drivers/gpu/drm/drm_atomic.c | 529 +++++++++++++++++++++++++++++++++++++++++++
 include/drm/drm_atomic.h     |  63 ++++++
 include/drm/drm_crtc.h       |  23 ++
 4 files changed, 616 insertions(+), 1 deletion(-)
 create mode 100644 drivers/gpu/drm/drm_atomic.c
 create mode 100644 include/drm/drm_atomic.h

diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
index 61d9e9c6bc10..82e922c02c6d 100644
--- a/drivers/gpu/drm/Makefile
+++ b/drivers/gpu/drm/Makefile
@@ -14,7 +14,7 @@ drm-y       :=	drm_auth.o drm_buffer.o drm_bufs.o drm_cache.o \
 		drm_info.o drm_debugfs.o drm_encoder_slave.o \
 		drm_trace_points.o drm_global.o drm_prime.o \
 		drm_rect.o drm_vma_manager.o drm_flip_work.o \
-		drm_modeset_lock.o
+		drm_modeset_lock.o drm_atomic.o
 
 drm-$(CONFIG_COMPAT) += drm_ioc32.o
 drm-$(CONFIG_DRM_GEM_CMA_HELPER) += drm_gem_cma_helper.o
diff --git a/drivers/gpu/drm/drm_atomic.c b/drivers/gpu/drm/drm_atomic.c
new file mode 100644
index 000000000000..29e77fe24e06
--- /dev/null
+++ b/drivers/gpu/drm/drm_atomic.c
@@ -0,0 +1,529 @@
+/*
+ * Copyright (C) 2014 Red Hat
+ * Copyright (C) 2014 Intel Corp.
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
+ * THE COPYRIGHT HOLDER(S) OR AUTHOR(S) BE LIABLE FOR ANY CLAIM, DAMAGES OR
+ * OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE,
+ * ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
+ * OTHER DEALINGS IN THE SOFTWARE.
+ *
+ * Authors:
+ * Rob Clark <robdclark@gmail.com>
+ * Daniel Vetter <daniel.vetter@ffwll.ch>
+ */
+
+
+#include <drm/drmP.h>
+#include <drm/drm_atomic.h>
+#include <drm/drm_plane_helper.h>
+
+static void kfree_state(struct drm_atomic_state *state)
+{
+	kfree(state->connectors);
+	kfree(state->connector_states);
+	kfree(state->crtcs);
+	kfree(state->crtc_states);
+	kfree(state->planes);
+	kfree(state->plane_states);
+	kfree(state);
+}
+
+/**
+ * drm_atomic_state_alloc - allocate atomic state
+ * @dev DRM device
+ *
+ * This allocates an empty atomic state to track updates.
+ */
+struct drm_atomic_state *
+drm_atomic_state_alloc(struct drm_device *dev)
+{
+	struct drm_atomic_state *state;
+
+	state = kzalloc(sizeof(*state), GFP_KERNEL);
+	if (!state)
+		return NULL;
+
+	state->crtcs = kcalloc(sizeof(*state->crtcs),
+			       dev->mode_config.num_crtc, GFP_KERNEL);
+	if (!state->crtcs)
+		goto fail;
+	state->crtc_states = kcalloc(sizeof(*state->crtc_states),
+				     dev->mode_config.num_crtc, GFP_KERNEL);
+	if (!state->crtc_states)
+		goto fail;
+	state->planes = kcalloc(sizeof(*state->planes),
+				dev->mode_config.num_total_plane, GFP_KERNEL);
+	if (!state->planes)
+		goto fail;
+	state->plane_states = kcalloc(sizeof(*state->plane_states),
+				      dev->mode_config.num_total_plane,
+				      GFP_KERNEL);
+	if (!state->plane_states)
+		goto fail;
+	state->connectors = kcalloc(sizeof(*state->connectors),
+				    dev->mode_config.num_connector,
+				    GFP_KERNEL);
+	if (!state->connectors)
+		goto fail;
+	state->connector_states = kcalloc(sizeof(*state->connector_states),
+					  dev->mode_config.num_connector,
+					  GFP_KERNEL);
+	if (!state->connector_states)
+		goto fail;
+
+	state->dev = dev;
+
+	return state;
+fail:
+	kfree_state(state);
+
+	return NULL;
+}
+EXPORT_SYMBOL(drm_atomic_state_alloc);
+
+/**
+ * drm_atomic_state_clear - clear state object
+ * @state: atomic state
+ *
+ * When the w/w mutex algorithm detects a deadlock we need to back off and drop
+ * all locks. So someone else could sneak in and change the current modeset
+ * configuration. Which means that all the state assembled in @state is no
+ * longer an atomic update to the current state, but to some arbitrary earlier
+ * state. Which would break assumptions the driver's ->atomic_check likely
+ * relies on.
+ *
+ * Hence we must clear all cached state and complete start over, using this
+ * function.
+ */
+void drm_atomic_state_clear(struct drm_atomic_state *state)
+{
+	struct drm_device *dev = state->dev;
+	int i;
+
+	for (i = 0; i < dev->mode_config.num_connector; i++) {
+		struct drm_connector *connector = state->connectors[i];
+
+		if (!connector)
+			continue;
+
+		connector->funcs->atomic_destroy_state(connector,
+						       state->connector_states[i]);
+	}
+
+	for (i = 0; i < dev->mode_config.num_crtc; i++) {
+		struct drm_crtc *crtc = state->crtcs[i];
+
+		if (!crtc)
+			continue;
+
+		crtc->funcs->atomic_destroy_state(crtc,
+						  state->crtc_states[i]);
+	}
+
+	for (i = 0; i < dev->mode_config.num_total_plane; i++) {
+		struct drm_plane *plane = state->planes[i];
+
+		if (!plane)
+			continue;
+
+		plane->funcs->atomic_destroy_state(plane,
+						   state->plane_states[i]);
+	}
+}
+EXPORT_SYMBOL(drm_atomic_state_clear);
+
+/**
+ * drm_atomic_state_free - free all memory for an atomic state
+ * @state: atomic state to deallocate
+ *
+ * This frees all memory associated with an atomic state, including all the
+ * per-object state for planes, crtcs and connectors.
+ */
+void drm_atomic_state_free(struct drm_atomic_state *state)
+{
+	drm_atomic_state_clear(state);
+
+	kfree_state(state);
+}
+EXPORT_SYMBOL(drm_atomic_state_free);
+
+/**
+ * drm_atomic_get_crtc_state - get crtc state
+ * @state: global atomic state object
+ * @crtc: crtc to get state object for
+ *
+ * This functions returns the crtc state for the given crtc, allocating it if
+ * need. It will also grab the relevant crtc lock to make sure that the state is
+ * consistent.
+ *
+ * Returns:
+ *
+ * Either the allocated state or the error code encoded into the pointer. When
+ * the error is EDEADLK then the w/w mutex code has detected a deadlock and the
+ * entire atomic sequence must be restarted. All other errors are fatal.
+ */
+struct drm_crtc_state *
+drm_atomic_get_crtc_state(struct drm_atomic_state *state,
+			  struct drm_crtc *crtc)
+{
+	int ret, index;
+	struct drm_crtc_state *crtc_state;
+
+	index = drm_crtc_index(crtc);
+
+	if (state->crtc_states[index])
+		return state->crtc_states[index];
+
+	ret = drm_modeset_lock(&crtc->mutex, state->acquire_ctx);
+	if (ret)
+		return ERR_PTR(ret);
+
+	crtc_state = crtc->funcs->atomic_duplicate_state(crtc);
+	if (!crtc_state)
+		return ERR_PTR(-ENOMEM);
+
+	state->crtc_states[index] = crtc_state;
+	state->crtcs[index] = crtc;
+
+	return crtc_state;
+}
+EXPORT_SYMBOL(drm_atomic_get_crtc_state);
+
+/**
+ * drm_atomic_get_plane_state - get plane state
+ * @state: global atomic state object
+ * @plane: plane to get state object for
+ *
+ * This functions returns the plane state for the given plane, allocating it if
+ * need. It will also grab the relevant plane lock to make sure that the state is
+ * consistent.
+ *
+ * Returns:
+ *
+ * Either the allocated state or the error code encoded into the pointer. When
+ * the error is EDEADLK then the w/w mutex code has detected a deadlock and the
+ * entire atomic sequence must be restarted. All other errors are fatal.
+ */
+struct drm_plane_state *
+drm_atomic_get_plane_state(struct drm_atomic_state *state,
+			  struct drm_plane *plane)
+{
+	int ret, index;
+	struct drm_plane_state *plane_state;
+
+	index = drm_plane_index(plane);
+
+	if (state->plane_states[index])
+		return state->plane_states[index];
+
+	/*
+	 * TODO: We currently don't have per-plane mutexes. So instead of trying
+	 * crazy tricks with deferring plane->crtc and hoping for the best just
+	 * grab all crtc locks. Once we have per-plane locks we must update this
+	 * to only take the plane mutex.
+	 */
+	ret = drm_modeset_lock_all_crtcs(state->dev, state->acquire_ctx);
+	if (ret)
+		return ERR_PTR(ret);
+
+	plane_state = plane->funcs->atomic_duplicate_state(plane);
+	if (!plane_state)
+		return ERR_PTR(-ENOMEM);
+
+	state->plane_states[index] = plane_state;
+	state->planes[index] = plane;
+
+	if (plane_state->crtc) {
+		struct drm_crtc_state *crtc_state;
+
+		crtc_state = drm_atomic_get_crtc_state(state,
+						       plane_state->crtc);
+		if (IS_ERR(crtc_state))
+			return ERR_PTR(PTR_ERR(crtc_state));
+	}
+
+	return plane_state;
+}
+EXPORT_SYMBOL(drm_atomic_get_plane_state);
+
+/**
+ * drm_atomic_get_connector_state - get connector state
+ * @state: global atomic state object
+ * @connector: connector to get state object for
+ *
+ * This functions returns the connector state for the given connector, allocating it if
+ * need. It will also grab the relevant connector lock to make sure that the state is
+ * consistent.
+ *
+ * Returns:
+ *
+ * Either the allocated state or the error code encoded into the pointer. When
+ * the error is EDEADLK then the w/w mutex code has detected a deadlock and the
+ * entire atomic sequence must be restarted. All other errors are fatal.
+ */
+struct drm_connector_state *
+drm_atomic_get_connector_state(struct drm_atomic_state *state,
+			  struct drm_connector *connector)
+{
+	int ret, index;
+	struct drm_mode_config *config = &connector->dev->mode_config;
+	struct drm_connector_state *connector_state;
+
+	index = drm_connector_index(connector);
+
+	if (state->connector_states[index])
+		return state->connector_states[index];
+
+	ret = drm_modeset_lock(&config->connection_mutex, state->acquire_ctx);
+	if (ret)
+		return ERR_PTR(ret);
+
+	connector_state = connector->funcs->atomic_duplicate_state(connector);
+	if (!connector_state)
+		return ERR_PTR(-ENOMEM);
+
+	state->connector_states[index] = connector_state;
+	state->connectors[index] = connector;
+
+	if (connector_state->crtc) {
+		struct drm_crtc_state *crtc_state;
+
+		crtc_state = drm_atomic_get_crtc_state(state,
+						       connector_state->crtc);
+		if (IS_ERR(crtc_state))
+			return ERR_PTR(PTR_ERR(crtc_state));
+	}
+
+	return connector_state;
+}
+EXPORT_SYMBOL(drm_atomic_get_connector_state);
+
+/**
+ * drm_atomic_set_crtc_for_plane - set crtc for plane
+ * @plane_state: atomic state object for the plane
+ * @crtc: crtc to use for the plane
+ *
+ * Returns:
+ * 0 on success or can fail with -EDEADLK or -ENOMEM.
+ */
+int
+drm_atomic_set_crtc_for_plane(struct drm_plane_state *plane_state,
+			      struct drm_crtc *crtc)
+{
+	struct drm_crtc_state *crtc_state;
+
+	crtc_state = drm_atomic_get_crtc_state(plane_state->state, crtc);
+	if (IS_ERR(crtc_state))
+		return PTR_ERR(crtc_state);
+
+	plane_state->crtc = crtc;
+
+	return 0;
+}
+EXPORT_SYMBOL(drm_atomic_set_crtc_for_plane);
+
+/**
+ * drm_atomic_set_crtc_for_connector - set crtc for connector
+ * @connector_state: atomic state object for the connector
+ * @crtc: crtc to use for the connector
+ *
+ * Returns:
+ * 0 on success or can fail with -EDEADLK or -ENOMEM.
+ */
+int
+drm_atomic_set_crtc_for_connector(struct drm_connector_state *conn_state,
+				  struct drm_crtc *crtc)
+{
+	struct drm_crtc_state *crtc_state;
+
+	crtc_state = drm_atomic_get_crtc_state(conn_state->state, crtc);
+	if (IS_ERR(crtc_state))
+		return PTR_ERR(crtc_state);
+
+	conn_state->crtc = crtc;
+
+	return 0;
+}
+EXPORT_SYMBOL(drm_atomic_set_crtc_for_connector);
+
+/**
+ * drm_atomic_add_affected_connectors - add connectors for crtc
+ * @state: atomic state
+ * @crtc: DRM crtc
+ *
+ * This functions walks the current configuration and adds all connectors
+ * currently using @crtc to the atomic configuration @state.
+ *
+ * Returns:
+ * 0 on success or can fail with -EDEADLK or -ENOMEM.
+ */
+int
+drm_atomic_add_affected_connectors(struct drm_atomic_state *state,
+				   struct drm_crtc *crtc)
+{
+	struct drm_mode_config *config = &state->dev->mode_config;
+	struct drm_connector *connector;
+	struct drm_connector_state *conn_state;
+	int ret;
+
+	ret = drm_modeset_lock(&config->connection_mutex, state->acquire_ctx);
+	if (ret)
+		return ret;
+
+	/*
+	 * Changed connectors are already in @state, so only need to look at the
+	 * current configuration.
+	 */
+	list_for_each_entry(connector, &config->connector_list, head) {
+		if (connector->state->crtc != crtc)
+			continue;
+
+		conn_state = drm_atomic_get_connector_state(state, connector);
+		if (IS_ERR(conn_state))
+			return PTR_ERR(conn_state);
+	}
+
+	return 0;
+}
+EXPORT_SYMBOL(drm_atomic_add_affected_connectors);
+
+/**
+ * drm_atomic_connectors_for_crtc - count number of connected outputs
+ * @state: atomic state
+ * @crtc: DRM crtc
+ *
+ * This function counts all connectors which will be connected to @crtc
+ * according to @state. Useful to recompute the enable state for @crtc.
+ */
+int
+drm_atomic_connectors_for_crtc(struct drm_atomic_state *state,
+			       struct drm_crtc *crtc)
+{
+	int nconnectors = state->dev->mode_config.num_connector;
+	int i, num_connected_connectors = 0;
+
+	for (i = 0; i < nconnectors; i++) {
+		struct drm_connector_state *conn_state;
+
+		conn_state = state->connector_states[i];
+
+		if (conn_state && conn_state->crtc == crtc)
+			num_connected_connectors++;
+	}
+
+	return num_connected_connectors;
+}
+EXPORT_SYMBOL(drm_atomic_connectors_for_crtc);
+
+/**
+ * drm_atomic_legacy_backoff - locking backoff for legacy ioctls
+ * @state: atomic state
+ *
+ * This function should be used by legacy entry points which don't understand
+ * -EDEADLK semantics. For simplicity this one will grab all modeset locks after
+ *  the slowpath completed.
+ */
+void drm_atomic_legacy_backoff(struct drm_atomic_state *state)
+{
+	int ret;
+
+retry:
+	drm_modeset_backoff(state->acquire_ctx);
+
+	ret = drm_modeset_lock(&state->dev->mode_config.connection_mutex,
+			       state->acquire_ctx);
+	if (ret)
+		goto retry;
+	ret = drm_modeset_lock_all_crtcs(state->dev,
+					 state->acquire_ctx);
+	if (ret)
+		goto retry;
+}
+EXPORT_SYMBOL(drm_atomic_legacy_backoff);
+
+/**
+ * drm_atomic_check_only - check whether a given config would work
+ * @state: atomic configuration to check
+ *
+ * Note that this function can return -EDEADLK if the driver needed to acquire
+ * more locks but encountered a deadlock. The caller must then do the usual w/w
+ * backoff dance and restart.
+ *
+ * Returns:
+ * 0 on success, negative error code on failure.
+ */
+int drm_atomic_check_only(struct drm_atomic_state *state)
+{
+	struct drm_mode_config *config = &state->dev->mode_config;
+
+	if (config->funcs->atomic_check)
+		return config->funcs->atomic_check(state->dev, state);
+	else
+		return 0;
+}
+EXPORT_SYMBOL(drm_atomic_check_only);
+
+/**
+ * drm_atomic_commit - commit configuration atomically
+ * @state: atomic configuration to check
+ *
+ * Note that this function can return -EDEADLK if the driver needed to acquire
+ * more locks but encountered a deadlock. The caller must then do the usual w/w
+ * backoff dance and restart.
+ *
+ * Returns:
+ * 0 on etuccess, negative error code on failure.
+ */
+int drm_atomic_commit(struct drm_atomic_state *state)
+{
+	struct drm_mode_config *config = &state->dev->mode_config;
+	int ret;
+
+	ret = drm_atomic_check_only(state);
+	if (ret)
+		return ret;
+
+	return config->funcs->atomic_commit(state->dev, state, false);
+}
+EXPORT_SYMBOL(drm_atomic_commit);
+
+/**
+ * drm_atomic_async_commit - atomic&async onfiguration commit
+ * @state: atomic configuration to check
+ *
+ * Note that this function can return -EDEADLK if the driver needed to acquire
+ * more locks but encountered a deadlock. The caller must then do the usual w/w
+ * backoff dance and restart.
+ *
+ * Also note that ownership of @state is transferred from the caller of this function
+ * to the function itself. The caller must not free or in any other way access
+ * @state.
+ *
+ * Returns:
+ * 0 on success, negative error code on failure.
+ */
+int drm_atomic_async_commit(struct drm_atomic_state *state)
+{
+	struct drm_mode_config *config = &state->dev->mode_config;
+	int ret;
+
+	ret = drm_atomic_check_only(state);
+	if (ret)
+		return ret;
+
+	return config->funcs->atomic_commit(state->dev, state, true);
+}
+EXPORT_SYMBOL(drm_atomic_async_commit);
diff --git a/include/drm/drm_atomic.h b/include/drm/drm_atomic.h
new file mode 100644
index 000000000000..753812034e71
--- /dev/null
+++ b/include/drm/drm_atomic.h
@@ -0,0 +1,63 @@
+/*
+ * Copyright (C) 2014 Red Hat
+ * Copyright (C) 2014 Intel Corp.
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
+ * THE COPYRIGHT HOLDER(S) OR AUTHOR(S) BE LIABLE FOR ANY CLAIM, DAMAGES OR
+ * OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE,
+ * ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
+ * OTHER DEALINGS IN THE SOFTWARE.
+ *
+ * Authors:
+ * Rob Clark <robdclark@gmail.com>
+ * Daniel Vetter <daniel.vetter@ffwll.ch>
+ */
+
+#ifndef DRM_ATOMIC_H_
+#define DRM_ATOMIC_H_
+
+struct drm_atomic_state *
+drm_atomic_state_alloc(struct drm_device *dev);
+void drm_atomic_state_clear(struct drm_atomic_state *state);
+void drm_atomic_state_free(struct drm_atomic_state *state);
+
+struct drm_crtc_state *
+drm_atomic_get_crtc_state(struct drm_atomic_state *state,
+			  struct drm_crtc *crtc);
+struct drm_plane_state *
+drm_atomic_get_plane_state(struct drm_atomic_state *state,
+			   struct drm_plane *plane);
+struct drm_connector_state *
+drm_atomic_get_connector_state(struct drm_atomic_state *state,
+			       struct drm_connector *connector);
+
+int drm_atomic_set_crtc_for_plane(struct drm_plane_state *plane_state,
+				  struct drm_crtc *crtc);
+int drm_atomic_set_crtc_for_connector(struct drm_connector_state *conn_state,
+				      struct drm_crtc *crtc);
+int
+drm_atomic_add_affected_connectors(struct drm_atomic_state *state,
+				   struct drm_crtc *crtc);
+int
+drm_atomic_connectors_for_crtc(struct drm_atomic_state *state,
+			       struct drm_crtc *crtc);
+
+void drm_atomic_legacy_backoff(struct drm_atomic_state *state);
+
+int drm_atomic_check_only(struct drm_atomic_state *state);
+int drm_atomic_commit(struct drm_atomic_state *state);
+int drm_atomic_async_commit(struct drm_atomic_state *state);
+
+#endif /* DRM_ATOMIC_H_ */
diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
index 39465d716db6..d79f2e0e8104 100644
--- a/include/drm/drm_crtc.h
+++ b/include/drm/drm_crtc.h
@@ -822,6 +822,23 @@ struct drm_bridge {
 };
 
 /**
+ * struct drm_atomic_state - the global state object for atomic updates
+ */
+struct drm_atomic_state {
+       struct drm_device *dev;
+       uint32_t flags;
+       struct drm_plane **planes;
+       struct drm_plane_state **plane_states;
+       struct drm_crtc **crtcs;
+       struct drm_crtc_state **crtc_states;
+       struct drm_connector **connectors;
+       struct drm_connector_state **connector_states;
+
+       struct drm_modeset_acquire_ctx *acquire_ctx;
+};
+
+
+/**
  * drm_mode_set - new values for a CRTC config change
  * @head: list management
  * @fb: framebuffer to use for new config
@@ -862,6 +879,12 @@ struct drm_mode_config_funcs {
 					     struct drm_file *file_priv,
 					     struct drm_mode_fb_cmd2 *mode_cmd);
 	void (*output_poll_changed)(struct drm_device *dev);
+
+	int (*atomic_check)(struct drm_device *dev,
+			    struct drm_atomic_state *a);
+	int (*atomic_commit)(struct drm_device *dev,
+			     struct drm_atomic_state *a,
+			     bool async);
 };
 
 /**
-- 
2.0.1

[PATCH 06/19] drm: Add atomic/plane helpers

[Daniel Vetter]

This is the first cut of atomic helper code. As-is it's only useful to
implement a pure atomic interface for plane updates.

Later patches will integrate this with the crtc helpers so that full
atomic updates are possible. We also need a pile of helpers to aid
drivers in transitioning from the legacy world to the shiny new atomic
age.

Finally we need helpers to implement legacy ioctls on top of the
atomic interface.

The design of the helpers is fairly simple, but has a unfortunate
large interface:
- We have ->atomic_check callbacks for crtcs and planes. The idea is
  that connectors don't need any checking, and if they do they can
  adjust the relevant crtc driver-private state. So no connector hooks
  should be needed. Also the crtc helpers integration will do the
  ->best_encoder checks, so no need for that.

- Framebuffer pinning needs to be done before we can commit to the hw
  state. This is especially important for async updates where we must
  pin all buffers before returning to userspace, so that really only
  hw failures can happen in the asynchronous worker.

  Hence we add ->prepare_fb and ->cleanup_fb hooks for this resources
  management.

- The actual atomic plane commit can't fail (except hw woes), so has
  void return type. It has three stages:
  1. Prepare all affected crtcs with crtc->atomic_begin. Drivers can
  use this to unset the GO bit or similar latches to prevent plane
  updates.
  2. Update plane state by looping over all changed planes and calling
  plane->atomic_update. Presuming the hardware is sane and has GO bits
  drivers can simply bash the state into the hardware in this
  function. Other drivers might use this to precompute hw state for
  the final step.
  3. Finally latch the update for the next vblank with
  crtc->atomic_flush. Note that this function doesn't need to wait for
  the vblank to happen even for the synchronous case.

v2: Clear drm_<obj>_state->state to NULL when swapping in state.

v3: Add TODO that we don't short-circuit plane updates for now. Likely
no one will care.

Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/Makefile            |   2 +-
 drivers/gpu/drm/drm_atomic_helper.c | 349 ++++++++++++++++++++++++++++++++++++
 include/drm/drm_atomic_helper.h     |  44 +++++
 include/drm/drm_crtc.h              |   5 +
 include/drm/drm_crtc_helper.h       |   6 +
 include/drm/drm_plane_helper.h      |  30 ++++
 6 files changed, 435 insertions(+), 1 deletion(-)
 create mode 100644 drivers/gpu/drm/drm_atomic_helper.c
 create mode 100644 include/drm/drm_atomic_helper.h

diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
index 82e922c02c6d..aae92c684f95 100644
--- a/drivers/gpu/drm/Makefile
+++ b/drivers/gpu/drm/Makefile
@@ -25,7 +25,7 @@ drm-$(CONFIG_OF) += drm_of.o
 drm-usb-y   := drm_usb.o
 
 drm_kms_helper-y := drm_crtc_helper.o drm_dp_helper.o drm_probe_helper.o \
-		drm_plane_helper.o drm_dp_mst_topology.o
+		drm_plane_helper.o drm_dp_mst_topology.o drm_atomic_helper.o
 drm_kms_helper-$(CONFIG_DRM_LOAD_EDID_FIRMWARE) += drm_edid_load.o
 drm_kms_helper-$(CONFIG_DRM_KMS_FB_HELPER) += drm_fb_helper.o
 drm_kms_helper-$(CONFIG_DRM_KMS_CMA_HELPER) += drm_fb_cma_helper.o
diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
new file mode 100644
index 000000000000..71e7025068f7
--- /dev/null
+++ b/drivers/gpu/drm/drm_atomic_helper.c
@@ -0,0 +1,349 @@
+/*
+ * Copyright (C) 2014 Red Hat
+ * Copyright (C) 2014 Intel Corp.
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
+ * THE COPYRIGHT HOLDER(S) OR AUTHOR(S) BE LIABLE FOR ANY CLAIM, DAMAGES OR
+ * OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE,
+ * ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
+ * OTHER DEALINGS IN THE SOFTWARE.
+ *
+ * Authors:
+ * Rob Clark <robdclark@gmail.com>
+ * Daniel Vetter <daniel.vetter@ffwll.ch>
+ */
+
+#include <drm/drmP.h>
+#include <drm/drm_atomic.h>
+#include <drm/drm_plane_helper.h>
+#include <drm/drm_crtc_helper.h>
+
+static void
+drm_atomic_helper_plane_changed(struct drm_atomic_state *state,
+				struct drm_plane_state *plane_state,
+				struct drm_plane *plane)
+{
+	struct drm_crtc_state *crtc_state;
+
+	/*
+	 * TODO: For now we don't bother optimizing out no-op changes - plane
+	 * updates should be fast.
+	 */
+
+	if (plane->crtc) {
+		crtc_state = state->crtc_states[drm_crtc_index(plane->crtc)];
+
+		if (WARN_ON(!crtc_state))
+			return;
+
+		crtc_state->planes_changed = true;
+	}
+
+	if (plane_state->crtc) {
+		crtc_state =
+			state->crtc_states[drm_crtc_index(plane_state->crtc)];
+
+		if (WARN_ON(!crtc_state))
+			return;
+
+		crtc_state->planes_changed = true;
+	}
+}
+
+/**
+ * drm_atomic_helper_check - validate state object
+ * @dev: DRM device
+ * @state: the driver state object
+ *
+ * Check the state object to see if the requested state is physically possible.
+ * Only crtcs and planes have check callbacks, so for any additional (global)
+ * checking that a driver needs it can simply wrap that around this function.
+ * Drivers without such needs can directly use this as their ->atomic_check()
+ * callback.
+ *
+ * RETURNS
+ * Zero for success or -errno
+ */
+int drm_atomic_helper_check(struct drm_device *dev,
+			    struct drm_atomic_state *state)
+{
+	int nplanes = dev->mode_config.num_total_plane;
+	int ncrtcs = dev->mode_config.num_crtc;
+	int i, ret = 0;
+
+	for (i = 0; i < nplanes; i++) {
+		struct drm_plane_helper_funcs *funcs;
+		struct drm_plane *plane = state->planes[i];
+		struct drm_plane_state *plane_state = state->plane_states[i];
+
+		if (!plane)
+			continue;
+
+		funcs = plane->helper_private;
+
+		drm_atomic_helper_plane_changed(state, plane_state, plane);
+
+		if (!funcs || !funcs->atomic_check)
+			continue;
+
+		ret = funcs->atomic_check(plane, plane_state);
+		if (ret)
+			return ret;
+	}
+
+	for (i = 0; i < ncrtcs; i++) {
+		struct drm_crtc_helper_funcs *funcs;
+		struct drm_crtc *crtc = state->crtcs[i];
+
+		if (!crtc)
+			continue;
+
+		funcs = crtc->helper_private;
+
+		if (!funcs || !funcs->atomic_check)
+			continue;
+
+		ret = funcs->atomic_check(crtc, state->crtc_states[i]);
+		if (ret)
+			return ret;
+	}
+
+	return ret;
+}
+EXPORT_SYMBOL(drm_atomic_helper_check);
+
+/**
+ * drm_atomic_helper_prepare_planes - prepare plane resources after commit
+ * @dev: DRM device
+ * @state: atomic state object with old state structures
+ *
+ * This function prepares plane state, specifically framebuffers, for the new
+ * configuration. If any failure is encountered this function will call
+ * ->cleanup_fb on any already successfully prepared framebuffer.
+ *
+ * Returns:
+ * 0 on success, negative error code on failure.
+ */
+int drm_atomic_helper_prepare_planes(struct drm_device *dev,
+				     struct drm_atomic_state *state)
+{
+	int nplanes = dev->mode_config.num_total_plane;
+	int ret, i;
+
+	for (i = 0; i < nplanes; i++) {
+		struct drm_plane_helper_funcs *funcs;
+		struct drm_plane *plane = state->planes[i];
+		struct drm_framebuffer *fb;
+
+		if (!plane)
+			continue;
+
+		funcs = plane->helper_private;
+
+		fb = state->plane_states[i]->fb;
+
+		if (fb && funcs->prepare_fb) {
+			ret = funcs->prepare_fb(plane, fb);
+			if (ret)
+				goto fail;
+		}
+	}
+
+	return 0;
+
+fail:
+	for (i--; i >= 0; i--) {
+		struct drm_plane_helper_funcs *funcs;
+		struct drm_plane *plane = state->planes[i];
+		struct drm_framebuffer *fb;
+
+		if (!plane)
+			continue;
+
+		funcs = plane->helper_private;
+
+		fb = state->plane_states[i]->fb;
+
+		if (fb && funcs->cleanup_fb)
+			funcs->cleanup_fb(plane, fb);
+
+	}
+
+	return ret;
+}
+EXPORT_SYMBOL(drm_atomic_helper_prepare_planes);
+
+/**
+ * drm_atomic_helper_commit_planes - commit plane state
+ * @dev: DRM device
+ * @state: atomic state
+ *
+ * This function commits the new plane state using the plane and atomic helper
+ * functions for planes and crtcs. It assumes that the atomic state has already
+ * been pushed into the relevant object state pointers, since this step can no
+ * longer fail.
+ */
+void drm_atomic_helper_commit_planes(struct drm_device *dev,
+				     struct drm_atomic_state *state)
+{
+	int nplanes = dev->mode_config.num_total_plane;
+	int ncrtcs = dev->mode_config.num_crtc;
+	int i;
+
+	for (i = 0; i < ncrtcs; i++) {
+		struct drm_crtc_helper_funcs *funcs;
+		struct drm_crtc *crtc = state->crtcs[i];
+
+		if (!crtc)
+			continue;
+
+		funcs = crtc->helper_private;
+
+		if (!funcs || !funcs->atomic_begin)
+			continue;
+
+		funcs->atomic_begin(crtc);
+	}
+
+	for (i = 0; i < nplanes; i++) {
+		struct drm_plane_helper_funcs *funcs;
+		struct drm_plane *plane = state->planes[i];
+
+		if (!plane)
+			continue;
+
+		funcs = plane->helper_private;
+
+		if (!funcs || !funcs->atomic_update)
+			continue;
+
+		funcs->atomic_update(plane);
+	}
+
+	for (i = 0; i < ncrtcs; i++) {
+		struct drm_crtc_helper_funcs *funcs;
+		struct drm_crtc *crtc = state->crtcs[i];
+
+		if (!crtc)
+			continue;
+
+		funcs = crtc->helper_private;
+
+		if (!funcs || !funcs->atomic_flush)
+			continue;
+
+		funcs->atomic_flush(crtc);
+	}
+}
+EXPORT_SYMBOL(drm_atomic_helper_commit_planes);
+
+/**
+ * drm_atomic_helper_cleanup_planes - cleanup plane resources after commit
+ * @dev: DRM device
+ * @state: atomic state object with old state structures
+ *
+ * This function cleans up plane state, specifically framebuffers, from the old
+ * configuration. Hence the old configuration must be perserved in @old_state to
+ * be able to call this function.
+ *
+ * This function must also be called on the new state when the atomic update
+ * fails at any point after calling drm_atomic_helper_prepare_planes().
+ */
+void drm_atomic_helper_cleanup_planes(struct drm_device *dev,
+				      struct drm_atomic_state *old_state)
+{
+	int nplanes = dev->mode_config.num_total_plane;
+	int i;
+
+	for (i = 0; i < nplanes; i++) {
+		struct drm_plane_helper_funcs *funcs;
+		struct drm_plane *plane = old_state->planes[i];
+		struct drm_framebuffer *old_fb;
+
+		if (!plane)
+			continue;
+
+		funcs = plane->helper_private;
+
+		old_fb = old_state->plane_states[i]->fb;
+
+		if (old_fb && funcs->cleanup_fb)
+			funcs->cleanup_fb(plane, old_fb);
+	}
+}
+EXPORT_SYMBOL(drm_atomic_helper_cleanup_planes);
+
+/**
+ * drm_atomic_helper_swap_state - store atomic state into current sw state
+ * @dev: DRM device
+ * @state: atomic state
+ *
+ * This function stores the atomic state into the current state pointers in all
+ * driver objects. It should be called after all failing steps have been done
+ * and succeeded, but before the actual hardware state is committed.
+ *
+ * For cleanup and error recovery the current state for all changed objects will
+ * be swaped into @state.
+ *
+ * With that sequence it fits perfectly into the plane prepare/cleanup sequence:
+ *
+ * 1. Call drm_atomic_helper_prepare_planes() with the staged atomic state.
+ *
+ * 2. Do any other steps that might fail, call drm_atomic_helper_prepare_planes
+ * with the staged atomic state.
+ *
+ * 3. Put the staged state into the current state pointers with this function.
+ *
+ * 4. Actually commit the hardware state.
+ *
+ * 5. Call drm_atomic_helper_cleanup_planes with @state, which since step 3
+ * contains the old state. Also do any other cleanup required with that state.
+ */
+void drm_atomic_helper_swap_state(struct drm_device *dev,
+				  struct drm_atomic_state *state)
+{
+	int i;
+
+	for (i = 0; i < dev->mode_config.num_connector; i++) {
+		struct drm_connector *connector = state->connectors[i];
+
+		if (!connector)
+			continue;
+
+		swap(state->connector_states[i], connector->state);
+		connector->state->state = NULL;
+	}
+
+	for (i = 0; i < dev->mode_config.num_crtc; i++) {
+		struct drm_crtc *crtc = state->crtcs[i];
+
+		if (!crtc)
+			continue;
+
+		swap(state->crtc_states[i], crtc->state);
+		crtc->state->state = NULL;
+	}
+
+	for (i = 0; i < dev->mode_config.num_total_plane; i++) {
+		struct drm_plane *plane = state->planes[i];
+
+		if (!plane)
+			continue;
+
+		swap(state->plane_states[i], plane->state);
+		plane->state->state = NULL;
+	}
+}
+EXPORT_SYMBOL(drm_atomic_helper_swap_state);
diff --git a/include/drm/drm_atomic_helper.h b/include/drm/drm_atomic_helper.h
new file mode 100644
index 000000000000..79938c62e7ad
--- /dev/null
+++ b/include/drm/drm_atomic_helper.h
@@ -0,0 +1,44 @@
+/*
+ * Copyright (C) 2014 Red Hat
+ * Copyright (C) 2014 Intel Corp.
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be included in
+ * all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT.  IN NO EVENT SHALL
+ * THE COPYRIGHT HOLDER(S) OR AUTHOR(S) BE LIABLE FOR ANY CLAIM, DAMAGES OR
+ * OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE,
+ * ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
+ * OTHER DEALINGS IN THE SOFTWARE.
+ *
+ * Authors:
+ * Rob Clark <robdclark@gmail.com>
+ * Daniel Vetter <daniel.vetter@ffwll.ch>
+ */
+
+#ifndef DRM_ATOMIC_HELPER_H_
+#define DRM_ATOMIC_HELPER_H_
+
+int drm_atomic_helper_check(struct drm_device *dev,
+			    struct drm_atomic_state *state);
+
+int drm_atomic_helper_prepare_planes(struct drm_device *dev,
+				     struct drm_atomic_state *state);
+void drm_atomic_helper_commit_planes(struct drm_device *dev,
+				     struct drm_atomic_state *state);
+void drm_atomic_helper_cleanup_planes(struct drm_device *dev,
+				      struct drm_atomic_state *old_state);
+
+void drm_atomic_helper_swap_state(struct drm_device *dev,
+				  struct drm_atomic_state *state);
+
+#endif /* DRM_ATOMIC_HELPER_H_ */
diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
index d79f2e0e8104..ff39aca5c266 100644
--- a/include/drm/drm_crtc.h
+++ b/include/drm/drm_crtc.h
@@ -237,6 +237,9 @@ struct drm_atomic_state;
 struct drm_crtc_state {
 	bool enable        : 1;
 
+	/* computed state bits used by helpers and drivers */
+	bool planes_changed : 1;
+
 	struct drm_display_mode mode;
 
 	struct drm_pending_vblank_event *event;
@@ -757,6 +760,8 @@ struct drm_plane {
 
 	enum drm_plane_type type;
 
+	void *helper_private;
+
 	struct drm_plane_state *state;
 };
 
diff --git a/include/drm/drm_crtc_helper.h b/include/drm/drm_crtc_helper.h
index a3d75fefd010..adec48b27aa5 100644
--- a/include/drm/drm_crtc_helper.h
+++ b/include/drm/drm_crtc_helper.h
@@ -81,6 +81,12 @@ struct drm_crtc_helper_funcs {
 
 	/* disable crtc when not in use - more explicit than dpms off */
 	void (*disable)(struct drm_crtc *crtc);
+
+	/* atomic helpers */
+	int (*atomic_check)(struct drm_crtc *crtc,
+			    struct drm_crtc_state *state);
+	void (*atomic_begin)(struct drm_crtc *crtc);
+	void (*atomic_flush)(struct drm_crtc *crtc);
 };
 
 /**
diff --git a/include/drm/drm_plane_helper.h b/include/drm/drm_plane_helper.h
index 52e6870534b2..a6f3a8ed3394 100644
--- a/include/drm/drm_plane_helper.h
+++ b/include/drm/drm_plane_helper.h
@@ -42,6 +42,28 @@
  * planes.
  */
 
+/**
+ * drm_plane_helper_funcs - helper operations for CRTCs
+ *
+ * The helper operations are called by the mid-layer CRTC helper.
+ */
+struct drm_plane_helper_funcs {
+	int (*prepare_fb)(struct drm_plane *plane,
+			  struct drm_framebuffer *fb);
+	void (*cleanup_fb)(struct drm_plane *plane,
+			   struct drm_framebuffer *fb);
+
+	int (*atomic_check)(struct drm_plane *plane,
+			    struct drm_plane_state *state);
+	void (*atomic_update)(struct drm_plane *plane);
+};
+
+static inline void drm_plane_helper_add(struct drm_plane *plane,
+					const struct drm_plane_helper_funcs *funcs)
+{
+	plane->helper_private = (void *)funcs;
+}
+
 extern int drm_plane_helper_check_update(struct drm_plane *plane,
 					 struct drm_crtc *crtc,
 					 struct drm_framebuffer *fb,
@@ -68,4 +90,12 @@ extern struct drm_plane *drm_primary_helper_create_plane(struct drm_device *dev,
 							 int num_formats);
 
 
+int drm_plane_helper_update(struct drm_plane *plane, struct drm_crtc *crtc,
+			    struct drm_framebuffer *fb,
+			    int crtc_x, int crtc_y,
+			    unsigned int crtc_w, unsigned int crtc_h,
+			    uint32_t src_x, uint32_t src_y,
+			    uint32_t src_w, uint32_t src_h);
+int drm_plane_helper_disable(struct drm_plane *plane);
+
 #endif
-- 
2.0.1

[PATCH 07/19] drm: Add drm_crtc_vblank_waitqueue()

[Daniel Vetter]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Add a small static inline helper to grab the vblank wait queue based on
the drm_crtc.

This is useful for drivers to do internal vblank waits using
wait_event() & co.

v2: Pimp commit message (Daniel)
    Add kernel doc (Daniel)

Suggested-by: Daniel Vetter <daniel@ffwll.ch>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 Documentation/DocBook/drm.tmpl |  1 +
 include/drm/drmP.h             | 11 +++++++++++
 2 files changed, 12 insertions(+)

diff --git a/Documentation/DocBook/drm.tmpl b/Documentation/DocBook/drm.tmpl
index 1d3756d3176c..972759489376 100644
--- a/Documentation/DocBook/drm.tmpl
+++ b/Documentation/DocBook/drm.tmpl
@@ -3400,6 +3400,7 @@ void (*disable_vblank) (struct drm_device *dev, int crtc);</synopsis>
     <sect2>
       <title>Vertical Blanking and Interrupt Handling Functions Reference</title>
 !Edrivers/gpu/drm/drm_irq.c
+!Iinclude/drm/drmP.h drm_crtc_vblank_waitqueue
     </sect2>
   </sect1>
 
diff --git a/include/drm/drmP.h b/include/drm/drmP.h
index 9b6a445f8602..06a673894c47 100644
--- a/include/drm/drmP.h
+++ b/include/drm/drmP.h
@@ -1372,6 +1372,17 @@ extern int drm_calc_vbltimestamp_from_scanoutpos(struct drm_device *dev,
 extern void drm_calc_timestamping_constants(struct drm_crtc *crtc,
 					    const struct drm_display_mode *mode);
 
+/**
+ * drm_crtc_vblank_waitqueue - get vblank waitqueue for the CRTC
+ * @crtc: which CRTC's vblank waitqueue to retrieve
+ *
+ * This function returns a pointer to the vblank waitqueue for the CRTC.
+ * Drivers can use this to implement vblank waits using wait_event() & co.
+ */
+static inline wait_queue_head_t *drm_crtc_vblank_waitqueue(struct drm_crtc *crtc)
+{
+	return &crtc->dev->vblank[drm_crtc_index(crtc)].queue;
+}
 
 /* Modesetting support */
 extern void drm_vblank_pre_modeset(struct drm_device *dev, int crtc);
-- 
2.0.1

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

[PATCH 08/19] drm/irq: Implement a generic vblank_wait function

[Daniel Vetter]

As usual in both a crtc index and a struct drm_crtc * version.

The function assumes that no one drivers their display below 10Hz, and
it will complain if the vblank wait takes longer than that.

v2: Also check dev->max_vblank_counter since some drivers register a
fake get_vblank_counter function.

Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/drm_irq.c | 45 +++++++++++++++++++++++++++++++++++++++++++++
 include/drm/drmP.h        |  2 ++
 2 files changed, 47 insertions(+)

diff --git a/drivers/gpu/drm/drm_irq.c b/drivers/gpu/drm/drm_irq.c
index 0de123afdb34..76024fdde452 100644
--- a/drivers/gpu/drm/drm_irq.c
+++ b/drivers/gpu/drm/drm_irq.c
@@ -999,6 +999,51 @@ void drm_crtc_vblank_put(struct drm_crtc *crtc)
 EXPORT_SYMBOL(drm_crtc_vblank_put);
 
 /**
+ * drm_vblank_wait - wait for one vblank
+ * @dev: DRM device
+ * @crtc: crtc index
+ *
+ * This waits for one vblank to pass on @crtc, using the irq driver interfaces.
+ * It is a failure to call this when the vblank irq for @crtc is disable, e.g.
+ * due to lack of driver support or because the crtc is off.
+ */
+void drm_vblank_wait(struct drm_device *dev, int crtc)
+{
+	int ret;
+	u32 last;
+
+	ret = drm_vblank_get(dev, crtc);
+	if (WARN_ON(ret))
+		return;
+
+	last = drm_vblank_count(dev, crtc);
+
+#define C (last != drm_vblank_count(dev, crtc))
+	ret = wait_event_timeout(dev->vblank[crtc].queue,
+				 C, msecs_to_jiffies(100));
+
+	WARN_ON(ret == 0);
+#undef C
+
+	drm_vblank_put(dev, crtc);
+}
+EXPORT_SYMBOL(drm_vblank_wait);
+
+/**
+ * drm_crtc_vblank_wait - wait for one vblank
+ * @crtc: DRM crtc
+ *
+ * This waits for one vblank to pass on @crtc, using the irq driver interfaces.
+ * It is a failure to call this when the vblank irq for @crtc is disable, e.g.
+ * due to lack of driver support or because the crtc is off.
+ */
+void drm_crtc_vblank_wait(struct drm_crtc *crtc)
+{
+	drm_vblank_wait(crtc->dev, drm_crtc_index(crtc));
+}
+EXPORT_SYMBOL(drm_crtc_vblank_wait);
+
+/**
  * drm_vblank_off - disable vblank events on a CRTC
  * @dev: DRM device
  * @crtc: CRTC in question
diff --git a/include/drm/drmP.h b/include/drm/drmP.h
index 06a673894c47..f72e5ef5f7b0 100644
--- a/include/drm/drmP.h
+++ b/include/drm/drmP.h
@@ -1355,6 +1355,8 @@ extern int drm_vblank_get(struct drm_device *dev, int crtc);
 extern void drm_vblank_put(struct drm_device *dev, int crtc);
 extern int drm_crtc_vblank_get(struct drm_crtc *crtc);
 extern void drm_crtc_vblank_put(struct drm_crtc *crtc);
+extern void drm_vblank_wait(struct drm_device *dev, int crtc);
+extern void drm_crtc_vblank_wait(struct drm_crtc *crtc);
 extern void drm_vblank_off(struct drm_device *dev, int crtc);
 extern void drm_vblank_on(struct drm_device *dev, int crtc);
 extern void drm_crtc_vblank_off(struct drm_crtc *crtc);
-- 
2.0.1

[PATCH 09/19] drm/i915: Use generic vblank wait

[Daniel Vetter]

This has the upside that it will no longer steal interrupts from the
interrutp handler on pre-g4x. Furthermore this will now scream properly
on all platforms if we don't have hw counters enabled.

Not yet tested.

Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/i915/intel_display.c | 41 +-----------------------------------
 1 file changed, 1 insertion(+), 40 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 99eb7cad62a8..d139970023ac 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -891,17 +891,6 @@ enum transcoder intel_pipe_to_cpu_transcoder(struct drm_i915_private *dev_priv,
 	return intel_crtc->config.cpu_transcoder;
 }
 
-static void g4x_wait_for_vblank(struct drm_device *dev, int pipe)
-{
-	struct drm_i915_private *dev_priv = dev->dev_private;
-	u32 frame, frame_reg = PIPE_FRMCOUNT_GM45(pipe);
-
-	frame = I915_READ(frame_reg);
-
-	if (wait_for(I915_READ_NOTRACE(frame_reg) != frame, 50))
-		WARN(1, "vblank wait timed out\n");
-}
-
 /**
  * intel_wait_for_vblank - wait for vblank on a given pipe
  * @dev: drm device
@@ -912,35 +901,7 @@ static void g4x_wait_for_vblank(struct drm_device *dev, int pipe)
  */
 void intel_wait_for_vblank(struct drm_device *dev, int pipe)
 {
-	struct drm_i915_private *dev_priv = dev->dev_private;
-	int pipestat_reg = PIPESTAT(pipe);
-
-	if (IS_G4X(dev) || INTEL_INFO(dev)->gen >= 5) {
-		g4x_wait_for_vblank(dev, pipe);
-		return;
-	}
-
-	/* Clear existing vblank status. Note this will clear any other
-	 * sticky status fields as well.
-	 *
-	 * This races with i915_driver_irq_handler() with the result
-	 * that either function could miss a vblank event.  Here it is not
-	 * fatal, as we will either wait upon the next vblank interrupt or
-	 * timeout.  Generally speaking intel_wait_for_vblank() is only
-	 * called during modeset at which time the GPU should be idle and
-	 * should *not* be performing page flips and thus not waiting on
-	 * vblanks...
-	 * Currently, the result of us stealing a vblank from the irq
-	 * handler is that a single frame will be skipped during swapbuffers.
-	 */
-	I915_WRITE(pipestat_reg,
-		   I915_READ(pipestat_reg) | PIPE_VBLANK_INTERRUPT_STATUS);
-
-	/* Wait for vblank interrupt bit to set */
-	if (wait_for(I915_READ(pipestat_reg) &
-		     PIPE_VBLANK_INTERRUPT_STATUS,
-		     50))
-		DRM_DEBUG_KMS("vblank wait timed out\n");
+	drm_vblank_wait(dev, pipe);
 }
 
 static bool pipe_dsl_stopped(struct drm_device *dev, enum pipe pipe)
-- 
2.0.1

[PATCH 10/19] drm/plane-helper: transitional atomic plane helpers

[Daniel Vetter]

Converting a driver to the atomic interface can be a daunting
undertaking. One of the prerequisites is to have full universal planes
support.

To make that transition a bit easier this pathc provides plane helpers
which use the new atomic helper callbacks just only for the plane
changes. This way the plane update functionality can be tested without
being forced to convert everything at once.

Of course a real atomic update capable driver will implement the
all plane properties through the atomic interface, so these helpers
are mostly transitional. But they can be used to enable proper
universal plane support, especially once the crtc helpers have also
been adapted.

v2: Use ->atomic_duplicate_state if available.

Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/drm_plane_helper.c | 181 ++++++++++++++++++++++++++++++++++++-
 1 file changed, 180 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/drm_plane_helper.c b/drivers/gpu/drm/drm_plane_helper.c
index 827ec1a3040b..7befbf017afe 100644
--- a/drivers/gpu/drm/drm_plane_helper.c
+++ b/drivers/gpu/drm/drm_plane_helper.c
@@ -27,7 +27,7 @@
 #include <drm/drmP.h>
 #include <drm/drm_plane_helper.h>
 #include <drm/drm_rect.h>
-#include <drm/drm_plane_helper.h>
+#include <drm/drm_crtc_helper.h>
 
 #define SUBPIXEL_MASK 0xffff
 
@@ -369,3 +369,182 @@ int drm_crtc_init(struct drm_device *dev, struct drm_crtc *crtc,
 	return drm_crtc_init_with_planes(dev, crtc, primary, NULL, funcs);
 }
 EXPORT_SYMBOL(drm_crtc_init);
+
+/**
+ * drm_primary_helper_update() - Helper for primary plane update
+ * @plane: plane object to update
+ * @crtc: owning CRTC of owning plane
+ * @fb: framebuffer to flip onto plane
+ * @crtc_x: x offset of primary plane on crtc
+ * @crtc_y: y offset of primary plane on crtc
+ * @crtc_w: width of primary plane rectangle on crtc
+ * @crtc_h: height of primary plane rectangle on crtc
+ * @src_x: x offset of @fb for panning
+ * @src_y: y offset of @fb for panning
+ * @src_w: width of source rectangle in @fb
+ * @src_h: height of source rectangle in @fb
+ *
+ * Provides a default plane update handler using the atomic plane update
+ * functions. It is fully left to the driver to check plane constraints and
+ * handle corner-cases like a fully occluded or otherwise invisible plane.
+ *
+ * This is useful for piecewise transitioning of a driver to the atomic helpers.
+ *
+ * RETURNS:
+ * Zero on success, error code on failure
+ */
+int drm_plane_helper_update(struct drm_plane *plane, struct drm_crtc *crtc,
+			    struct drm_framebuffer *fb,
+			    int crtc_x, int crtc_y,
+			    unsigned int crtc_w, unsigned int crtc_h,
+			    uint32_t src_x, uint32_t src_y,
+			    uint32_t src_w, uint32_t src_h)
+{
+	struct drm_plane_state *plane_state;
+	struct drm_plane_helper_funcs *plane_funcs;
+	struct drm_crtc_helper_funcs *crtc_funcs;
+	int ret;
+
+	if (plane->funcs->atomic_duplicate_state)
+		plane_state = plane->funcs->atomic_duplicate_state(plane);
+	else if (plane->state)
+		plane_state = kmemdup(plane->state, sizeof(*plane_state),
+				      GFP_KERNEL);
+	else
+		plane_state = kzalloc(sizeof(*plane_state), GFP_KERNEL);
+	if (!plane_state)
+		return -ENOMEM;
+
+	plane_state->crtc = crtc;
+	plane_state->fb = fb;
+	plane_state->crtc_x = crtc_x;
+	plane_state->crtc_y = crtc_y;
+	plane_state->crtc_h = crtc_h;
+	plane_state->crtc_w = crtc_w;
+	plane_state->src_x = src_x;
+	plane_state->src_y = src_y;
+	plane_state->src_h = src_h;
+	plane_state->src_w = src_w;
+
+	plane_funcs = plane->helper_private;
+
+	if (plane_funcs->atomic_check) {
+		ret = plane_funcs->atomic_check(plane, plane_state);
+		if (ret)
+			goto fail;
+	}
+
+	if (plane_funcs->prepare_fb) {
+		ret = plane_funcs->prepare_fb(plane, fb);
+		if (ret)
+			goto fail;
+	}
+
+	/* Point of no return, commit sw state. */
+	swap(plane->state, plane_state);
+	fb = plane_state->fb;
+
+	crtc_funcs = crtc->helper_private;
+
+	if (crtc_funcs && crtc_funcs->atomic_begin)
+		crtc_funcs->atomic_begin(crtc);
+
+	plane_funcs->atomic_update(plane);
+
+	if (crtc_funcs && crtc_funcs->atomic_flush)
+		crtc_funcs->atomic_flush(crtc);
+
+	/* There's no way to figure out whether the crtc is running. */
+	ret = drm_crtc_vblank_get(crtc);
+	if (ret == 0) {
+		drm_crtc_vblank_wait(crtc);
+		drm_crtc_vblank_put(crtc);
+	}
+
+	if (plane_funcs->cleanup_fb && fb)
+		plane_funcs->cleanup_fb(plane, fb);
+fail:
+	kfree(plane_state);
+
+	return ret;
+}
+EXPORT_SYMBOL(drm_plane_helper_update);
+
+/**
+ * drm_primary_helper_disable() - Helper for primary plane disable
+ * @plane: plane to disable
+ *
+ * Provides a default plane disable handler using the atomic plane update
+ * functions. It is fully left to the driver to check plane constraints and
+ * handle corner-cases like a fully occluded or otherwise invisible plane.
+ *
+ * This is useful for piecewise transitioning of a driver to the atomic helpers.
+ *
+ * RETURNS:
+ * Zero on success, error code on failure
+ */
+int drm_plane_helper_disable(struct drm_plane *plane)
+{
+	struct drm_plane_state *plane_state;
+	struct drm_plane_helper_funcs *plane_funcs;
+	struct drm_crtc_helper_funcs *crtc_funcs;
+	struct drm_framebuffer *fb = NULL;
+	struct drm_crtc *crtc = plane->crtc;
+	int ret;
+
+	if (WARN_ON(!crtc))
+		return -EINVAL;
+
+	if (plane->funcs->atomic_duplicate_state)
+		plane_state = plane->funcs->atomic_duplicate_state(plane);
+	else if (plane->state)
+		plane_state = kmemdup(plane->state, sizeof(*plane_state),
+				      GFP_KERNEL);
+	else
+		plane_state = kzalloc(sizeof(*plane_state), GFP_KERNEL);
+	if (!plane_state)
+		return -ENOMEM;
+
+	plane_funcs = plane->helper_private;
+
+	if (plane_funcs->atomic_check) {
+		ret = plane_funcs->atomic_check(plane, plane_state);
+		if (ret)
+			goto fail;
+	}
+
+	if (plane_funcs->prepare_fb) {
+		ret = plane_funcs->prepare_fb(plane, fb);
+		if (ret)
+			goto fail;
+	}
+
+	/* Point of no return, commit sw state. */
+	swap(plane->state, plane_state);
+	fb = plane_state->fb;
+
+	crtc_funcs = crtc->helper_private;
+
+	if (crtc_funcs && crtc_funcs->atomic_begin)
+		crtc_funcs->atomic_begin(crtc);
+
+	plane_funcs->atomic_update(plane);
+
+	if (crtc_funcs && crtc_funcs->atomic_flush)
+		crtc_funcs->atomic_flush(crtc);
+
+	/* There's no way to figure out whether the crtc is running. */
+	ret = drm_crtc_vblank_get(crtc);
+	if (ret == 0) {
+		drm_crtc_vblank_wait(crtc);
+		drm_crtc_vblank_put(crtc);
+	}
+
+	if (plane_funcs->cleanup_fb && fb)
+		plane_funcs->cleanup_fb(plane, fb);
+fail:
+	kfree(plane_state);
+
+	return ret;
+}
+EXPORT_SYMBOL(drm_plane_helper_disable);
-- 
2.0.1

[PATCH 11/19] drm/crtc-helper: Transitional functions using atomic plane helpers

[Daniel Vetter]

These two functions allow drivers to reuse their atomic plane helpers
functions for the primary plane to implement the interfaces required
by the crtc helpers for the legacy ->set_config callback.

This is purely transitional and won't be used once the driver is fully
converted. But it allows partial conversions to the atomic plane
helpers which are functional.

v2:
- Use ->atomic_duplicate_state if available.
- Don't forget to run crtc_funcs->atomic_check.

v3: Shift source coordinates correctly for 16.16 fixed point.

Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/drm_crtc_helper.c | 138 ++++++++++++++++++++++++++++++++++++++
 include/drm/drm_crtc.h            |   3 +
 include/drm/drm_crtc_helper.h     |   7 ++
 3 files changed, 148 insertions(+)

diff --git a/drivers/gpu/drm/drm_crtc_helper.c b/drivers/gpu/drm/drm_crtc_helper.c
index 6c65a0a28fbd..751dcc9ba163 100644
--- a/drivers/gpu/drm/drm_crtc_helper.c
+++ b/drivers/gpu/drm/drm_crtc_helper.c
@@ -38,6 +38,7 @@
 #include <drm/drm_fourcc.h>
 #include <drm/drm_crtc_helper.h>
 #include <drm/drm_fb_helper.h>
+#include <drm/drm_plane_helper.h>
 #include <drm/drm_edid.h>
 
 MODULE_AUTHOR("David Airlie, Jesse Barnes");
@@ -888,3 +889,140 @@ void drm_helper_resume_force_mode(struct drm_device *dev)
 	drm_modeset_unlock_all(dev);
 }
 EXPORT_SYMBOL(drm_helper_resume_force_mode);
+
+/**
+ * drm_helper_crtc_mode_set - mode_set implementation for atomic plane helpers
+ *
+ * This function implements a callback useable as the ->mode_set callback
+ * required by the crtc helpers. Besides the atomic plane helper functions for
+ * the primary plane the driver must also provide the ->mode_set_nofb callback
+ * to set up the crtc.
+ *
+ * This is a transitional helper useful for converting drivers to the atomic
+ * interfaces.
+ */
+int drm_helper_crtc_mode_set(struct drm_crtc *crtc, struct drm_display_mode *mode,
+			     struct drm_display_mode *adjusted_mode, int x, int y,
+			     struct drm_framebuffer *old_fb)
+{
+	struct drm_crtc_state *crtc_state;
+	struct drm_crtc_helper_funcs *crtc_funcs = crtc->helper_private;
+	int ret;
+
+	if (crtc->funcs->atomic_duplicate_state)
+		crtc_state = crtc->funcs->atomic_duplicate_state(crtc);
+	else if (crtc->state)
+		crtc_state = kmemdup(crtc->state, sizeof(*crtc_state),
+				     GFP_KERNEL);
+	else
+		crtc_state = kzalloc(sizeof(*crtc_state), GFP_KERNEL);
+	if (!crtc_state)
+		return -ENOMEM;
+
+	crtc_state->enable = true;
+	crtc_state->planes_changed = true;
+	drm_mode_copy(&crtc_state->mode, mode);
+	drm_mode_copy(&crtc_state->adjusted_mode, adjusted_mode);
+
+	if (crtc_funcs->atomic_check) {
+		ret = crtc_funcs->atomic_check(crtc, crtc_state);
+		if (ret) {
+			kfree(crtc_state);
+
+			return ret;
+		}
+	}
+
+	swap(crtc->state, crtc_state);
+
+	crtc_funcs->mode_set_nofb(crtc);
+
+	kfree(crtc_state);
+
+	return drm_helper_crtc_mode_set_base(crtc, x, y, old_fb);
+}
+EXPORT_SYMBOL(drm_helper_crtc_mode_set);
+
+/**
+ * drm_helper_crtc_mode_set_base - mode_set_base implementation for atomic plane helpers
+ *
+ * This function implements a callback useable as the ->mode_set_base used
+ * required by the crtc helpers. The driver must provide the atomic plane helper
+ * functions for the primary plane.
+ *
+ * This is a transitional helper useful for converting drivers to the atomic
+ * interfaces.
+ */
+int drm_helper_crtc_mode_set_base(struct drm_crtc *crtc, int x, int y,
+				  struct drm_framebuffer *old_fb)
+{
+	struct drm_plane_state *plane_state;
+	struct drm_plane_helper_funcs *plane_funcs;
+	struct drm_crtc_helper_funcs *crtc_funcs;
+	struct drm_plane *plane = crtc->primary;
+	int ret;
+
+	if (plane->funcs->atomic_duplicate_state)
+		plane_state = plane->funcs->atomic_duplicate_state(plane);
+	else if (plane->state)
+		plane_state = kmemdup(plane->state, sizeof(*plane_state),
+				      GFP_KERNEL);
+	else
+		plane_state = kzalloc(sizeof(*plane_state), GFP_KERNEL);
+	if (!plane_state)
+		return -ENOMEM;
+
+	plane_state->crtc = crtc;
+	plane_state->fb = crtc->primary->fb;
+	plane_state->crtc_x = 0;
+	plane_state->crtc_y = 0;
+	plane_state->crtc_h = crtc->mode.hdisplay;
+	plane_state->crtc_w = crtc->mode.vdisplay;
+	plane_state->src_x = x << 16;
+	plane_state->src_y = y << 16;
+	plane_state->src_h = crtc->mode.hdisplay << 16;
+	plane_state->src_w = crtc->mode.hdisplay << 16;
+
+	plane_funcs = plane->helper_private;
+
+	if (plane_funcs->atomic_check) {
+		ret = plane_funcs->atomic_check(plane, plane_state);
+		if (ret)
+			goto fail;
+	}
+
+	if (plane_funcs->prepare_fb) {
+		ret = plane_funcs->prepare_fb(plane, plane_state->fb);
+		if (ret)
+			goto fail;
+	}
+
+	/* Point of no return, commit sw state. */
+	swap(plane->state, plane_state);
+	plane_state->fb = old_fb;
+
+	crtc_funcs = crtc->helper_private;
+
+	if (crtc_funcs && crtc_funcs->atomic_begin)
+		crtc_funcs->atomic_begin(crtc);
+
+	plane_funcs->atomic_update(plane);
+
+	if (crtc_funcs && crtc_funcs->atomic_flush)
+		crtc_funcs->atomic_flush(crtc);
+
+	/* There's no way to figure out whether the crtc is running. */
+	ret = drm_crtc_vblank_get(crtc);
+	if (ret == 0) {
+		drm_crtc_vblank_wait(crtc);
+		drm_crtc_vblank_put(crtc);
+	}
+
+	if (plane_funcs->cleanup_fb && plane_state->fb)
+		plane_funcs->cleanup_fb(plane, plane_state->fb);
+fail:
+	kfree(plane_state);
+
+	return ret;
+}
+EXPORT_SYMBOL(drm_helper_crtc_mode_set_base);
diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
index ff39aca5c266..1dec9911abc7 100644
--- a/include/drm/drm_crtc.h
+++ b/include/drm/drm_crtc.h
@@ -240,6 +240,9 @@ struct drm_crtc_state {
 	/* computed state bits used by helpers and drivers */
 	bool planes_changed : 1;
 
+	/* adjusted_mode: for use by helpers and drivers */
+	struct drm_display_mode adjusted_mode;
+
 	struct drm_display_mode mode;
 
 	struct drm_pending_vblank_event *event;
diff --git a/include/drm/drm_crtc_helper.h b/include/drm/drm_crtc_helper.h
index adec48b27aa5..7adbb65ea8ae 100644
--- a/include/drm/drm_crtc_helper.h
+++ b/include/drm/drm_crtc_helper.h
@@ -68,6 +68,7 @@ struct drm_crtc_helper_funcs {
 	int (*mode_set)(struct drm_crtc *crtc, struct drm_display_mode *mode,
 			struct drm_display_mode *adjusted_mode, int x, int y,
 			struct drm_framebuffer *old_fb);
+	void (*mode_set_nofb)(struct drm_crtc *crtc);
 
 	/* Move the crtc on the current fb to the given position *optional* */
 	int (*mode_set_base)(struct drm_crtc *crtc, int x, int y,
@@ -167,6 +168,12 @@ static inline void drm_connector_helper_add(struct drm_connector *connector,
 
 extern void drm_helper_resume_force_mode(struct drm_device *dev);
 
+int drm_helper_crtc_mode_set(struct drm_crtc *crtc, struct drm_display_mode *mode,
+			     struct drm_display_mode *adjusted_mode, int x, int y,
+			     struct drm_framebuffer *old_fb);
+int drm_helper_crtc_mode_set_base(struct drm_crtc *crtc, int x, int y,
+				  struct drm_framebuffer *old_fb);
+
 /* drm_probe_helper.c */
 extern int drm_helper_probe_single_connector_modes(struct drm_connector
 						   *connector, uint32_t maxX,
-- 
2.0.1

[PATCH 12/19] drm: Atomic crtc/connector updates using crtc helper interfaces

[Daniel Vetter]

So this is finally the integration of the crtc helper interfaces into
the atomic helper functions.

In the check function we now have a few steps:

- First we update the output routing and figure out which crtcs need a
  full mode set. Suitable encoders are selected using ->best_encoder,
  with the same semantics as the crtc helpers of implicitly disabling
  all connectors currently using the encoder.

- Then we pull all other connectors into the state update which feed
  from a crtc which changes. This must be done do catch mode changes
  and similar updates - atomic updates are differences on top of the
  current state.

- Then we call all the various ->mode_fixup to compute the adjusted
  mode. Note that here we have a slight semantic difference compared
  to the crtc helpers: We have not yet updated the encoder->crtc link
  when calling the encoder's ->mode_fixup function. But that's a
  requirement when converting to atomic since we want to prepare the
  entire state completely contained with the over drm_atomic_state
  structure. So this must be carefully checked when converting drivers
  over to atomic helpers.

- Finally we do call the atomic_check functions on planes and crtcs.

The commit function is also quite a beast:

- The only step that can fail is done first, namely pinning the
  framebuffers. After that we cross the point of no return, an async
  commit would push all that into the worker thread.

- The disabling of encoders and connectors is a bit tricky, since
  depending upon the final state we need to select different crtc
  helper functions.

- Software tracking is a bit clarified compared to the crtc helpers:
  We commit the software state before starting to touch the hardware,
  like crtc helpers. But since we just swap them we still have the old
  state (i.e. the current hw state) around, which is really handy to
  write simple disable functions. So no more
  drm_crtc_helper_disable_all_unused_functions kind of fun because
  we're leaving unused crtcs/encoders behind. Everything gets shut
  down in-order now, which is one of the key differences of the i915
  helpers compared to crtc helpers and a really nice additional
  guarantee.

- Like with the plane helpers the atomic commit function waits for one
  vblank to pass before calling the framebuffer cleanup function.

Compared to Rob's helper approach there's a bunch of upsides:

- All the interfaces which can fail are called in the ->check hook
  (i.e. ->best_match and the various ->mode_fixup hooks). This means
  that drivers can just reuse those functions and don't need to move
  everything into ->atomic_check callbacks. If drivers have no need
  for additional constraint checking beyong their existing crtc
  helper callbacks they don't need to do anything.

- The actual commit operation is properly stage: First we prepare
  framebuffers, which can potentially still fail (due to memory
  exhausting). This is important for the async case, where this must
  be done synchronously to correctly return errors.

- The output configuration changes (done with crtc helper functions)
  and the plane update (using atomic plane helpers) are correctly
  interleaved: First we shut down any crtcs that need changing, then
  we update planes and finally we enable everything again. Hardware
  without GO bits must be more careful with ordering, which this
  sequence enables.

- Also for hardware with shared output resources (like display PLLs)
  we first must shut down the old configuration before we can enable
  the new one. Otherwise we can hit an impossible intermediate state
  where there's not enough PLLs (which is the point behind atomic
  updates).

v2:
- Ensure that users of ->check update crtc_state->enable correctly.
- Update the legacy state in crtc/plane structures. Eventually we want
  to remove that, but for now the drm core still expects this (especially
  the plane->fb pointer).

v3: A few changes for better async handling:

- Reorder the software side state commit so that it happens all before
  we touch the hardware. This way async support becomes very easy
  since we can punt all the actual hw touching to a worker thread. And
  as long as we synchronize with that thread (flushing or cancelling,
  depending upon what the driver can handle) before we commit the next
  software state there's no need for any locking in the worker thread
  at all. Which greatly simplifies things.

  And as long as we synchronize with all relevant threads we can have
  a lot of them (e.g. per-crtc for per-crtc updates) running in
  parallel.

- Expose pre/post plane commit steps separately. We need to expose the
  actual hw commit step anyway for drivers to be able to implement
  asynchronous commit workers. But if we expose pre/post and plane
  commit steps individually we allow drivers to selectively use atomic
  helpers.

- I've forgotten to call encoder/bridge ->mode_set functions, fix
  this.

Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/drm_atomic_helper.c | 592 ++++++++++++++++++++++++++++++++++++
 drivers/gpu/drm/drm_crtc_helper.c   |   1 +
 include/drm/drm_atomic_helper.h     |   8 +
 include/drm/drm_crtc.h              |   7 +
 4 files changed, 608 insertions(+)

diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
index 71e7025068f7..d5d2eb2908cc 100644
--- a/drivers/gpu/drm/drm_atomic_helper.c
+++ b/drivers/gpu/drm/drm_atomic_helper.c
@@ -29,6 +29,7 @@
 #include <drm/drm_atomic.h>
 #include <drm/drm_plane_helper.h>
 #include <drm/drm_crtc_helper.h>
+#include <drm/drm_atomic_helper.h>
 
 static void
 drm_atomic_helper_plane_changed(struct drm_atomic_state *state,
@@ -62,6 +63,254 @@ drm_atomic_helper_plane_changed(struct drm_atomic_state *state,
 	}
 }
 
+static int
+steal_encoder(struct drm_atomic_state *state,
+	      struct drm_encoder *encoder)
+{
+	struct drm_mode_config *config = &state->dev->mode_config;
+	struct drm_crtc_state *crtc_state;
+	struct drm_connector *connector;
+	struct drm_connector_state *connector_state;
+	/*
+	 * We can only steal an encoder coming from a connector, which means we
+	 * must already hold the connection_mutex.
+	 */
+	WARN_ON(!drm_modeset_is_locked(&config->connection_mutex));
+
+	if (!encoder->crtc)
+		return 0;
+
+	crtc_state = drm_atomic_get_crtc_state(state, encoder->crtc);
+	if (IS_ERR(crtc_state))
+		return PTR_ERR(crtc_state);
+
+	crtc_state->mode_changed = true;
+
+	list_for_each_entry(connector, &config->connector_list, head) {
+		if (connector->state->best_encoder != encoder)
+			continue;
+
+		connector_state = drm_atomic_get_connector_state(state,
+								 connector);
+		if (IS_ERR(connector_state))
+			return PTR_ERR(connector_state);
+
+		connector_state->crtc = NULL;
+		connector_state->best_encoder = NULL;
+	}
+
+	return 0;
+}
+
+static int
+update_connector_routing(struct drm_atomic_state *state, int conn_idx)
+{
+	struct drm_connector_helper_funcs *funcs;
+	struct drm_encoder *new_encoder;
+	struct drm_connector *connector;
+	struct drm_connector_state *connector_state;
+	struct drm_crtc *crtc;
+	struct drm_crtc_state *crtc_state;
+	int idx, ret;
+
+	connector = state->connectors[conn_idx];
+	connector_state = state->connector_states[conn_idx];
+
+	if (!connector)
+		return 0;
+
+	if (connector->state->crtc != connector_state->crtc) {
+		if (connector->state->crtc) {
+			idx = drm_crtc_index(connector->state->crtc);
+
+			crtc_state = state->crtc_states[idx];
+			crtc->state->mode_changed = true;
+		}
+
+		if (connector_state->crtc) {
+			idx = drm_crtc_index(connector_state->crtc);
+
+			crtc_state = state->crtc_states[idx];
+			crtc->state->mode_changed = true;
+		}
+	}
+
+	if (!connector_state->crtc)
+		return 0;
+
+	funcs = connector->helper_private;
+	new_encoder = funcs->best_encoder(connector);
+
+	if (!new_encoder) {
+		DRM_DEBUG_KMS("Now suitable encoder found\n");
+		return -EINVAL;
+	}
+
+	if (new_encoder->crtc) {
+		ret = steal_encoder(state, new_encoder);
+		if (ret) {
+			DRM_DEBUG_KMS("Encoder stealing failed\n");
+			return ret;
+		}
+	}
+
+	if (new_encoder != connector_state->best_encoder) {
+		connector_state->best_encoder = new_encoder;
+		idx = drm_crtc_index(connector_state->crtc);
+
+		crtc_state = state->crtc_states[idx];
+		crtc_state->mode_changed = true;
+	}
+
+	return 0;
+}
+
+static int
+mode_fixup(struct drm_atomic_state *state)
+{
+	int ncrtcs = state->dev->mode_config.num_crtc;
+	int nconnectors = state->dev->mode_config.num_connector;
+	struct drm_crtc_state *crtc_state;
+	struct drm_connector_state *conn_state;
+	int i;
+	bool ret;
+
+	for (i = 0; i < ncrtcs; i++) {
+		crtc_state = state->crtc_states[i];
+
+		if (!crtc_state || !crtc_state->mode_changed)
+			continue;
+
+		drm_mode_copy(&crtc_state->adjusted_mode, &crtc_state->mode);
+	}
+
+	for (i = 0; i < nconnectors; i++) {
+		struct drm_encoder_helper_funcs *funcs;
+		struct drm_encoder *encoder;
+
+		conn_state = state->connector_states[i];
+
+		if (!conn_state || !conn_state->best_encoder)
+			continue;
+
+		crtc_state =
+			state->crtc_states[drm_crtc_index(conn_state->crtc)];
+
+		/*
+		 * Each encoder has at most one connector (since we always steal
+		 * it away), so we won't call ->mode_fixup twice.
+		 */
+		encoder = conn_state->best_encoder;
+		funcs = encoder->helper_private;
+
+		if (encoder->bridge && encoder->bridge->funcs->mode_fixup) {
+			ret = encoder->bridge->funcs->mode_fixup(
+					encoder->bridge, &crtc_state->mode,
+					&crtc_state->adjusted_mode);
+			if (!ret) {
+				DRM_DEBUG_KMS("Bridge fixup failed\n");
+				return -EINVAL;
+			}
+		}
+
+
+		ret = funcs->mode_fixup(encoder, &crtc_state->mode,
+					&crtc_state->adjusted_mode);
+		if (!ret) {
+			DRM_DEBUG_KMS("Encoder fixup failed\n");
+			return -EINVAL;
+		}
+	}
+
+	for (i = 0; i < ncrtcs; i++) {
+		struct drm_crtc_helper_funcs *funcs;
+		struct drm_crtc *crtc;
+
+		crtc_state = state->crtc_states[i];
+		crtc = state->crtcs[i];
+
+		if (!crtc_state || !crtc_state->mode_changed)
+			continue;
+
+		funcs = crtc->helper_private;
+		ret = funcs->mode_fixup(crtc, &crtc_state->mode,
+					&crtc_state->adjusted_mode);
+		if (!ret) {
+			DRM_DEBUG_KMS("CRTC fixup failed\n");
+			return -EINVAL;
+		}
+	}
+
+	return 0;
+}
+
+static int
+drm_atomic_helper_check_prepare(struct drm_device *dev,
+				struct drm_atomic_state *state)
+{
+	int ncrtcs = dev->mode_config.num_crtc;
+	int nconnectors = dev->mode_config.num_connector;
+	struct drm_crtc *crtc;
+	struct drm_crtc_state *crtc_state;
+	int i, ret;
+
+	for (i = 0; i < ncrtcs; i++) {
+		crtc = state->crtcs[i];
+		crtc_state = state->crtc_states[i];
+
+		if (!crtc)
+			continue;
+
+		if (!drm_mode_equal(&crtc->state->mode, &crtc_state->mode))
+			crtc_state->mode_changed = true;
+
+		if (crtc->state->enable != crtc_state->enable)
+			crtc_state->mode_changed = true;
+	}
+
+	for (i = 0; i < nconnectors; i++) {
+		/*
+		 * This only sets crtc->mode_changed for routing changes,
+		 * drivers must set crtc->mode_changed themselves when connector
+		 * properties need to be updated.
+		 */
+		ret = update_connector_routing(state, i);
+		if (ret)
+			return ret;
+	}
+
+	/*
+	 * After all the routing has been prepared we need to add in any
+	 * connector which is itself unchanged, but who's crtc changes it's
+	 * configuration. This must be done before calling mode_fixup in case a
+	 * crtc only changed its mode but has the same set of connectors.
+	 */
+	for (i = 0; i < ncrtcs; i++) {
+		crtc = state->crtcs[i];
+		crtc_state = state->crtc_states[i];
+
+		if (!crtc)
+			continue;
+
+		if (crtc_state->mode_changed) {
+			bool has_connectors;
+
+			ret = drm_atomic_add_affected_connectors(state, crtc);
+			if (ret < 0)
+				return ret;
+
+			has_connectors = drm_atomic_connectors_for_crtc(state,
+									crtc);
+
+			WARN_ON(crtc_state->enable != has_connectors);
+		}
+	}
+
+	ret = mode_fixup(state);
+
+	return 0;
+}
+
 /**
  * drm_atomic_helper_check - validate state object
  * @dev: DRM device
@@ -83,6 +332,10 @@ int drm_atomic_helper_check(struct drm_device *dev,
 	int ncrtcs = dev->mode_config.num_crtc;
 	int i, ret = 0;
 
+	ret = drm_atomic_helper_check_prepare(dev, state);
+	if (ret)
+		return ret;
+
 	for (i = 0; i < nplanes; i++) {
 		struct drm_plane_helper_funcs *funcs;
 		struct drm_plane *plane = state->planes[i];
@@ -124,6 +377,344 @@ int drm_atomic_helper_check(struct drm_device *dev,
 }
 EXPORT_SYMBOL(drm_atomic_helper_check);
 
+static void
+disable_outputs(struct drm_device *dev, struct drm_atomic_state *old_state)
+{
+	int ncrtcs = old_state->dev->mode_config.num_crtc;
+	int nconnectors = old_state->dev->mode_config.num_connector;
+	int i;
+
+	for (i = 0; i < nconnectors; i++) {
+		struct drm_connector_state *old_conn_state;
+		struct drm_connector *connector;
+		struct drm_encoder_helper_funcs *funcs;
+		struct drm_encoder *encoder;
+
+		old_conn_state = old_state->connector_states[i];
+		connector = old_state->connectors[i];
+
+		/* Shut down everything that's in the changeset and currently
+		 * still on. So need to check the old, saved state. */
+		if (!old_conn_state || !old_conn_state->crtc)
+			continue;
+
+		encoder = connector->state->best_encoder;
+		funcs = encoder->helper_private;
+
+		if (encoder->bridge)
+			encoder->bridge->funcs->disable(encoder->bridge);
+
+		/* Right function depends upon target state. */
+		if (connector->state->crtc)
+			funcs->prepare(encoder);
+		else if (funcs->disable)
+			funcs->disable(encoder);
+		else
+			funcs->dpms(encoder, DRM_MODE_DPMS_OFF);
+
+		if (encoder->bridge)
+			encoder->bridge->funcs->post_disable(encoder->bridge);
+	}
+
+	for (i = 0; i < ncrtcs; i++) {
+		struct drm_crtc_helper_funcs *funcs;
+		struct drm_crtc *crtc;
+
+		crtc = old_state->crtcs[i];
+
+		/* Shut down everything that needs a full modeset. */
+		if (!crtc || !crtc->state->mode_changed)
+			continue;
+
+		funcs = crtc->helper_private;
+
+		/* Right function depends upon target state. */
+		if (crtc->state->enable)
+			funcs->prepare(crtc);
+		else if (funcs->disable)
+			funcs->disable(crtc);
+		else
+			funcs->dpms(crtc, DRM_MODE_DPMS_OFF);
+	}
+}
+
+static void
+set_routing_links(struct drm_device *dev, struct drm_atomic_state *old_state)
+{
+	int nconnectors = dev->mode_config.num_connector;
+	int ncrtcs = old_state->dev->mode_config.num_crtc;
+	int i;
+
+	/* clear out existing links */
+	for (i = 0; i < nconnectors; i++) {
+		struct drm_connector *connector;
+
+		connector = old_state->connectors[i];
+
+		if (!connector || !connector->encoder)
+			continue;
+
+		connector->encoder->crtc = NULL;
+		connector->encoder = NULL;
+	}
+
+	/* set new links */
+	for (i = 0; i < nconnectors; i++) {
+		struct drm_connector *connector;
+
+		connector = old_state->connectors[i];
+
+		if (!connector || !connector->state->crtc)
+			continue;
+
+		connector->encoder = connector->state->best_encoder;
+		connector->encoder->crtc = connector->state->crtc;
+	}
+
+	for (i = 0; i < ncrtcs; i++) {
+		struct drm_crtc *crtc;
+
+		crtc = old_state->crtcs[i];
+
+		/* Shut down everything that needs a full modeset. */
+		if (!crtc)
+			continue;
+
+		crtc->mode = crtc->state->mode;
+		crtc->enabled = crtc->state->enable;
+		crtc->x = crtc->primary->state->src_x >> 16;
+		crtc->y = crtc->primary->state->src_y >> 16;
+	}
+}
+
+static void
+crtc_set_mode(struct drm_device *dev, struct drm_atomic_state *old_state)
+{
+	int ncrtcs = old_state->dev->mode_config.num_crtc;
+	int nconnectors = old_state->dev->mode_config.num_connector;
+	int i;
+
+	for (i = 0; i < ncrtcs; i++) {
+		struct drm_crtc_helper_funcs *funcs;
+		struct drm_crtc *crtc;
+
+		crtc = old_state->crtcs[i];
+
+		if (!crtc || !crtc->state->mode_changed)
+			continue;
+
+		funcs = crtc->helper_private;
+
+		if (crtc->state->enable)
+			funcs->mode_set_nofb(crtc);
+	}
+
+	for (i = 0; i < nconnectors; i++) {
+		struct drm_connector *connector;
+		struct drm_crtc_state *new_crtc_state;
+		struct drm_encoder_helper_funcs *funcs;
+		struct drm_encoder *encoder;
+		struct drm_display_mode *mode, *adjusted_mode;
+
+		connector = old_state->connectors[i];
+
+		if (!connector || !connector->state->best_encoder)
+			continue;
+
+		encoder = connector->state->best_encoder;
+		funcs = encoder->helper_private;
+		new_crtc_state = connector->state->crtc->state;
+		mode = &new_crtc_state->mode;
+		adjusted_mode = &new_crtc_state->adjusted_mode;
+
+		funcs->mode_set(encoder, mode, adjusted_mode);
+
+		if (encoder->bridge && encoder->bridge->funcs->mode_set)
+			encoder->bridge->funcs->mode_set(encoder->bridge,
+							 mode, adjusted_mode);
+	}
+}
+
+/**
+ * drm_atomic_helper_commit_planes - modeset commit before plane updates
+ * @dev: DRM device
+ * @state: atomic state
+ *
+ * This function commits the modeset changes that need to be committed before
+ * updating planes. It shuts down all the outputs that need to be shut down and
+ * prepares them (if requried) with the new mode.
+ */
+void drm_atomic_helper_commit_pre_planes(struct drm_device *dev,
+					 struct drm_atomic_state *state)
+{
+	disable_outputs(dev, state);
+	set_routing_links(dev, state);
+	crtc_set_mode(dev, state);
+}
+EXPORT_SYMBOL(drm_atomic_helper_commit_pre_planes);
+
+/**
+ * drm_atomic_helper_commit_planes - commit plane state
+ * @dev: DRM device
+ * @state: atomic state
+ *
+ * This function commits the modeset changes that need to be committed after
+ * updating planes. It enables all the outputs with the new configuration which
+ * had to be turned off for the update.
+ */
+void drm_atomic_helper_commit_post_planes(struct drm_device *dev,
+					  struct drm_atomic_state *old_state)
+{
+	int ncrtcs = old_state->dev->mode_config.num_crtc;
+	int nconnectors = old_state->dev->mode_config.num_connector;
+	int i;
+
+	for (i = 0; i < ncrtcs; i++) {
+		struct drm_crtc_helper_funcs *funcs;
+		struct drm_crtc *crtc;
+
+		crtc = old_state->crtcs[i];
+
+		/* Need to filter out CRTCs where only planes change. */
+		if (!crtc || !crtc->state->mode_changed)
+			continue;
+
+		funcs = crtc->helper_private;
+
+		if (crtc->state->enable)
+			funcs->commit(crtc);
+	}
+
+	for (i = 0; i < nconnectors; i++) {
+		struct drm_connector *connector;
+		struct drm_encoder_helper_funcs *funcs;
+		struct drm_encoder *encoder;
+
+		connector = old_state->connectors[i];
+
+		if (!connector || !connector->state->best_encoder)
+			continue;
+
+		encoder = connector->state->best_encoder;
+		funcs = encoder->helper_private;
+
+		if (encoder->bridge)
+			encoder->bridge->funcs->pre_enable(encoder->bridge);
+
+		funcs->commit(encoder);
+
+		if (encoder->bridge)
+			encoder->bridge->funcs->enable(encoder->bridge);
+	}
+}
+EXPORT_SYMBOL(drm_atomic_helper_commit_post_planes);
+
+static void
+wait_for_vblanks(struct drm_device *dev, struct drm_atomic_state *old_state)
+{
+	struct drm_crtc *crtc;
+	struct drm_crtc_state *old_crtc_state;
+	int ncrtcs = old_state->dev->mode_config.num_crtc;
+	int i, ret;
+
+	for (i = 0; i < ncrtcs; i++) {
+		crtc = old_state->crtcs[i];
+
+		if (!crtc || !crtc->state->enable)
+			continue;
+
+		/* no one cares about the old state, so use it for tracking */
+		old_crtc_state = old_state->crtc_states[i];
+		old_crtc_state->enable = true;
+		ret = drm_crtc_vblank_get(crtc);
+		if (ret) {
+			old_crtc_state->enable = false;
+			continue;
+		}
+
+		old_crtc_state->last_vblank_count = drm_vblank_count(dev, i);
+	}
+
+	for (i = 0; i < ncrtcs; i++) {
+		crtc = old_state->crtcs[i];
+		old_crtc_state = old_state->crtc_states[i];
+
+		if (!crtc || !old_crtc_state->enable)
+			continue;
+
+#define C (old_crtc_state->last_vblank_count != drm_vblank_count(dev, i))
+		wait_event(dev->vblank[drm_crtc_index(crtc)].queue, C);
+
+#undef C
+
+		drm_crtc_vblank_put(crtc);
+	}
+}
+
+/**
+ * drm_atomic_helper_check - commit validated state object
+ * @dev: DRM device
+ * @state: the driver state object
+ * @async: asynchronous commit
+ *
+ * This function commits a with drm_atomic_helper_check() pre-validated state
+ * object. This can still fail when e.g. the framebuffer reservation fails. For
+ * now this doesn't implement asynchronous commits.
+ *
+ * RETURNS
+ * Zero for success or -errno.
+ */
+int drm_atomic_helper_commit(struct drm_device *dev,
+			     struct drm_atomic_state *state,
+			     bool async)
+{
+	int ret;
+
+	if (async)
+		return -EBUSY;
+
+	ret = drm_atomic_helper_prepare_planes(dev, state);
+	if (ret)
+		return ret;
+
+	/*
+	 * This is the point of no return - everything below never fails except
+	 * when the hw goes bonghits. Which means we can commit the new state on
+	 * the software side now.
+	 */
+
+	drm_atomic_helper_swap_state(dev, state);
+
+	/*
+	 * Everything below can be run asynchronously withou the need to grab
+	 * any modeset locks at all under one conditions: It must be guaranteed
+	 * that the asynchronous work has either been cancelled (if the driver
+	 * supports it, which at least requires that the framebuffers get
+	 * cleaned up with drm_atomic_helper_cleanup_planes()) or completed
+	 * before the new state gets committed on the software side with
+	 * drm_atomic_helper_swap_state().
+	 *
+	 * This scheme allows new atomic state updates to be prepared and
+	 * checked in parallel to the asynchronous completion of the previous
+	 * update. Which is important since compositors need to figure out the
+	 * composition of the next frame right after having submitted the
+	 * current layout.
+	 */
+
+	drm_atomic_helper_commit_pre_planes(dev, state);
+
+	drm_atomic_helper_commit_planes(dev, state);
+
+	drm_atomic_helper_commit_post_planes(dev, state);
+
+	wait_for_vblanks(dev, state);
+
+	drm_atomic_helper_cleanup_planes(dev, state);
+
+	return 0;
+}
+EXPORT_SYMBOL(drm_atomic_helper_commit);
+
 /**
  * drm_atomic_helper_prepare_planes - prepare plane resources after commit
  * @dev: DRM device
@@ -230,6 +821,7 @@ void drm_atomic_helper_commit_planes(struct drm_device *dev,
 			continue;
 
 		funcs->atomic_update(plane);
+		plane->fb = plane->state->fb;
 	}
 
 	for (i = 0; i < ncrtcs; i++) {
diff --git a/drivers/gpu/drm/drm_crtc_helper.c b/drivers/gpu/drm/drm_crtc_helper.c
index 751dcc9ba163..7e838104e72e 100644
--- a/drivers/gpu/drm/drm_crtc_helper.c
+++ b/drivers/gpu/drm/drm_crtc_helper.c
@@ -921,6 +921,7 @@ int drm_helper_crtc_mode_set(struct drm_crtc *crtc, struct drm_display_mode *mod
 
 	crtc_state->enable = true;
 	crtc_state->planes_changed = true;
+	crtc_state->mode_changed = true;
 	drm_mode_copy(&crtc_state->mode, mode);
 	drm_mode_copy(&crtc_state->adjusted_mode, adjusted_mode);
 
diff --git a/include/drm/drm_atomic_helper.h b/include/drm/drm_atomic_helper.h
index 79938c62e7ad..9781ce739e10 100644
--- a/include/drm/drm_atomic_helper.h
+++ b/include/drm/drm_atomic_helper.h
@@ -30,6 +30,14 @@
 
 int drm_atomic_helper_check(struct drm_device *dev,
 			    struct drm_atomic_state *state);
+int drm_atomic_helper_commit(struct drm_device *dev,
+			     struct drm_atomic_state *state,
+			     bool async);
+
+void drm_atomic_helper_commit_pre_planes(struct drm_device *dev,
+					 struct drm_atomic_state *state);
+void drm_atomic_helper_commit_post_planes(struct drm_device *dev,
+					  struct drm_atomic_state *old_state);
 
 int drm_atomic_helper_prepare_planes(struct drm_device *dev,
 				     struct drm_atomic_state *state);
diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
index 1dec9911abc7..59db2cd34b50 100644
--- a/include/drm/drm_crtc.h
+++ b/include/drm/drm_crtc.h
@@ -239,6 +239,10 @@ struct drm_crtc_state {
 
 	/* computed state bits used by helpers and drivers */
 	bool planes_changed : 1;
+	bool mode_changed : 1;
+
+	/* last_vblank_count: for vblank waits before cleanup */
+	u32 last_vblank_count;
 
 	/* adjusted_mode: for use by helpers and drivers */
 	struct drm_display_mode adjusted_mode;
@@ -441,6 +445,9 @@ drm_crtc_destroy_state(struct drm_crtc *crtc,
 struct drm_connector_state {
 	struct drm_crtc *crtc;
 
+	/* best_encoder: for use by helpers and drivers */
+	struct drm_encoder *best_encoder;
+
 	struct drm_atomic_state *state;
 };
 
-- 
2.0.1

[PATCH 13/19] drm: Move ->old_fb from crtc to plane

[Daniel Vetter]

Atomic implemenations for legacy ioctls must be able to drop locks.
Which doesn't cause havoc since we only do that while constructing
the new state, so no driver or hardware state change has happened.

The only troubling bit is the fb refcounting the core does - if
someone else has snuck in then it might potentially unref an
outdated framebuffer. To fix that move the old_fb temporary storage
into struct drm_plane for all ioctls, so that the atomic helpers can
update it.

Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/drm_crtc.c | 40 ++++++++++++++++++++++++----------------
 include/drm/drm_crtc.h     |  8 ++++----
 2 files changed, 28 insertions(+), 20 deletions(-)

diff --git a/drivers/gpu/drm/drm_crtc.c b/drivers/gpu/drm/drm_crtc.c
index e33aa3a3a8a5..ff705ea3f50e 100644
--- a/drivers/gpu/drm/drm_crtc.c
+++ b/drivers/gpu/drm/drm_crtc.c
@@ -1200,19 +1200,21 @@ EXPORT_SYMBOL(drm_plane_index);
  */
 void drm_plane_force_disable(struct drm_plane *plane)
 {
-	struct drm_framebuffer *old_fb = plane->fb;
 	int ret;
 
-	if (!old_fb)
+	if (!plane->fb)
 		return;
 
+	plane->old_fb = plane->fb;
 	ret = plane->funcs->disable_plane(plane);
 	if (ret) {
 		DRM_ERROR("failed to disable plane with busy fb\n");
+		plane->old_fb = NULL;
 		return;
 	}
 	/* disconnect the plane from the fb and crtc: */
-	__drm_framebuffer_unreference(old_fb);
+	__drm_framebuffer_unreference(plane->old_fb);
+	plane->old_fb = NULL;
 	plane->fb = NULL;
 	plane->crtc = NULL;
 }
@@ -2188,7 +2190,7 @@ static int setplane_internal(struct drm_plane *plane,
 			     uint32_t src_w, uint32_t src_h)
 {
 	struct drm_device *dev = plane->dev;
-	struct drm_framebuffer *old_fb = NULL;
+	struct drm_framebuffer *old_fb;
 	int ret = 0;
 	unsigned int fb_width, fb_height;
 	int i;
@@ -2196,14 +2198,16 @@ static int setplane_internal(struct drm_plane *plane,
 	/* No fb means shut it down */
 	if (!fb) {
 		drm_modeset_lock_all(dev);
-		old_fb = plane->fb;
+		plane->old_fb = plane->fb;
 		ret = plane->funcs->disable_plane(plane);
 		if (!ret) {
 			plane->crtc = NULL;
 			plane->fb = NULL;
 		} else {
-			old_fb = NULL;
+			plane->old_fb = NULL;
 		}
+		old_fb = plane->old_fb;
+		plane->old_fb = NULL;
 		drm_modeset_unlock_all(dev);
 		goto out;
 	}
@@ -2245,7 +2249,7 @@ static int setplane_internal(struct drm_plane *plane,
 	}
 
 	drm_modeset_lock_all(dev);
-	old_fb = plane->fb;
+	plane->old_fb = plane->fb;
 	ret = plane->funcs->update_plane(plane, crtc, fb,
 					 crtc_x, crtc_y, crtc_w, crtc_h,
 					 src_x, src_y, src_w, src_h);
@@ -2254,8 +2258,10 @@ static int setplane_internal(struct drm_plane *plane,
 		plane->fb = fb;
 		fb = NULL;
 	} else {
-		old_fb = NULL;
+		plane->old_fb = NULL;
 	}
+	old_fb = plane->old_fb;
+	plane->old_fb = NULL;
 	drm_modeset_unlock_all(dev);
 
 out:
@@ -2369,7 +2375,7 @@ int drm_mode_set_config_internal(struct drm_mode_set *set)
 	 * crtcs. Atomic modeset will have saner semantics ...
 	 */
 	list_for_each_entry(tmp, &crtc->dev->mode_config.crtc_list, head)
-		tmp->old_fb = tmp->primary->fb;
+		tmp->primary->old_fb = tmp->primary->fb;
 
 	fb = set->fb;
 
@@ -2382,8 +2388,9 @@ int drm_mode_set_config_internal(struct drm_mode_set *set)
 	list_for_each_entry(tmp, &crtc->dev->mode_config.crtc_list, head) {
 		if (tmp->primary->fb)
 			drm_framebuffer_reference(tmp->primary->fb);
-		if (tmp->old_fb)
-			drm_framebuffer_unreference(tmp->old_fb);
+		if (tmp->primary->old_fb)
+			drm_framebuffer_unreference(tmp->primary->old_fb);
+		tmp->primary->old_fb = NULL;
 	}
 
 	return ret;
@@ -4458,7 +4465,7 @@ int drm_mode_page_flip_ioctl(struct drm_device *dev,
 {
 	struct drm_mode_crtc_page_flip *page_flip = data;
 	struct drm_crtc *crtc;
-	struct drm_framebuffer *fb = NULL, *old_fb = NULL;
+	struct drm_framebuffer *fb = NULL;
 	struct drm_pending_vblank_event *e = NULL;
 	unsigned long flags;
 	int ret = -EINVAL;
@@ -4530,7 +4537,7 @@ int drm_mode_page_flip_ioctl(struct drm_device *dev,
 			(void (*) (struct drm_pending_event *)) kfree;
 	}
 
-	old_fb = crtc->primary->fb;
+	crtc->primary->old_fb = crtc->primary->fb;
 	ret = crtc->funcs->page_flip(crtc, fb, e, page_flip->flags);
 	if (ret) {
 		if (page_flip->flags & DRM_MODE_PAGE_FLIP_EVENT) {
@@ -4540,7 +4547,7 @@ int drm_mode_page_flip_ioctl(struct drm_device *dev,
 			kfree(e);
 		}
 		/* Keep the old fb, don't unref it. */
-		old_fb = NULL;
+		crtc->primary->old_fb = NULL;
 	} else {
 		/*
 		 * Warn if the driver hasn't properly updated the crtc->fb
@@ -4556,8 +4563,9 @@ int drm_mode_page_flip_ioctl(struct drm_device *dev,
 out:
 	if (fb)
 		drm_framebuffer_unreference(fb);
-	if (old_fb)
-		drm_framebuffer_unreference(old_fb);
+	if (crtc->primary->old_fb)
+		drm_framebuffer_unreference(crtc->primary->old_fb);
+	crtc->primary->old_fb = NULL;
 	drm_modeset_unlock_crtc(crtc);
 
 	return ret;
diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
index 59db2cd34b50..795d6b2dfd52 100644
--- a/include/drm/drm_crtc.h
+++ b/include/drm/drm_crtc.h
@@ -379,10 +379,6 @@ struct drm_crtc {
 	int cursor_x;
 	int cursor_y;
 
-	/* Temporary tracking of the old fb while a modeset is ongoing. Used
-	 * by drm_mode_set_config_internal to implement correct refcounting. */
-	struct drm_framebuffer *old_fb;
-
 	bool enabled;
 
 	/* Requested mode from modesetting. */
@@ -764,6 +760,10 @@ struct drm_plane {
 	struct drm_crtc *crtc;
 	struct drm_framebuffer *fb;
 
+	/* Temporary tracking of the old fb while a modeset is ongoing. Used
+	 * by drm_mode_set_config_internal to implement correct refcounting. */
+	struct drm_framebuffer *old_fb;
+
 	const struct drm_plane_funcs *funcs;
 
 	struct drm_object_properties properties;
-- 
2.0.1

[PATCH 14/19] drm/atomic-helper: implementatations for legacy interfaces

[Daniel Vetter]

Well, except page_flip since that requires async commit, which isn't
there yet.

For the functions which changes planes there's a bit of trickery
involved to keep the fb refcounting working. But otherwise fairly
straight-forward atomic updates.

The property setting functions are still a bit incomplete. Once we
have generic properties (e.g. rotation, but also all the properties
needed by the atomic ioctl) we need to filter those out and parse them
in the helper. Preferrably with the same function as used by the real
atomic ioctl implementation.

Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/drm_atomic_helper.c | 481 ++++++++++++++++++++++++++++++++++++
 include/drm/drm_atomic_helper.h     |  21 ++
 2 files changed, 502 insertions(+)

diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
index d5d2eb2908cc..1b13394cf020 100644
--- a/drivers/gpu/drm/drm_atomic_helper.c
+++ b/drivers/gpu/drm/drm_atomic_helper.c
@@ -939,3 +939,484 @@ void drm_atomic_helper_swap_state(struct drm_device *dev,
 	}
 }
 EXPORT_SYMBOL(drm_atomic_helper_swap_state);
+
+/**
+ * drm_atomic_helper_update_plane - Helper for primary plane update using atomic
+ * @plane: plane object to update
+ * @crtc: owning CRTC of owning plane
+ * @fb: framebuffer to flip onto plane
+ * @crtc_x: x offset of primary plane on crtc
+ * @crtc_y: y offset of primary plane on crtc
+ * @crtc_w: width of primary plane rectangle on crtc
+ * @crtc_h: height of primary plane rectangle on crtc
+ * @src_x: x offset of @fb for panning
+ * @src_y: y offset of @fb for panning
+ * @src_w: width of source rectangle in @fb
+ * @src_h: height of source rectangle in @fb
+ *
+ * Provides a default plane update handler using the atomic driver interface.
+ *
+ * RETURNS:
+ * Zero on success, error code on failure
+ */
+int drm_atomic_helper_update_plane(struct drm_plane *plane,
+				   struct drm_crtc *crtc,
+				   struct drm_framebuffer *fb,
+				   int crtc_x, int crtc_y,
+				   unsigned int crtc_w, unsigned int crtc_h,
+				   uint32_t src_x, uint32_t src_y,
+				   uint32_t src_w, uint32_t src_h)
+{
+	struct drm_atomic_state *state;
+	struct drm_plane_state *plane_state;
+	int ret = 0;
+
+	state = drm_atomic_state_alloc(plane->dev);
+	if (!state)
+		return -ENOMEM;
+
+	state->acquire_ctx = drm_modeset_legacy_acquire_ctx(crtc);
+retry:
+	plane_state = drm_atomic_get_plane_state(state, plane);
+	if (IS_ERR(plane_state)) {
+		ret = PTR_ERR(plane_state);
+		if (ret == -EDEADLK)
+			goto backoff;
+		else
+			goto fail;
+	}
+
+	plane_state->crtc = crtc;
+	plane_state->fb = fb;
+	plane_state->crtc_x = crtc_x;
+	plane_state->crtc_y = crtc_y;
+	plane_state->crtc_h = crtc_h;
+	plane_state->crtc_w = crtc_w;
+	plane_state->src_x = src_x;
+	plane_state->src_y = src_y;
+	plane_state->src_h = src_h;
+	plane_state->src_w = src_w;
+
+	ret = drm_atomic_commit(state);
+	if (ret == -EDEADLK)
+		goto backoff;
+fail:
+	drm_atomic_state_free(state);
+
+	return ret;
+backoff:
+	drm_atomic_legacy_backoff(state);
+	drm_atomic_state_clear(state);
+
+	/*
+	 * Someone might have exchanged the framebuffer while we dropped locks
+	 * in the backoff code. We need to fix up the fb refcount tracking the
+	 * core does for us.
+	 */
+	plane->old_fb = plane->fb;
+
+	goto retry;
+}
+EXPORT_SYMBOL(drm_atomic_helper_update_plane);
+
+/**
+ * drm_atomic_helper_disable_plane - Helper for primary plane disable using * atomic
+ * @plane: plane to disable
+ *
+ * Provides a default plane disablle handler using the atomic driver interface.
+ *
+ * RETURNS:
+ * Zero on success, error code on failure
+ */
+int drm_atomic_helper_disable_plane(struct drm_plane *plane)
+{
+	struct drm_atomic_state *state;
+	struct drm_plane_state *plane_state;
+	int ret = 0;
+
+	state = drm_atomic_state_alloc(plane->dev);
+	if (!state)
+		return -ENOMEM;
+
+	state->acquire_ctx = drm_modeset_legacy_acquire_ctx(plane->crtc);
+retry:
+	plane_state = drm_atomic_get_plane_state(state, plane);
+	if (IS_ERR(plane_state)) {
+		ret = PTR_ERR(plane_state);
+		if (ret == -EDEADLK)
+			goto backoff;
+		else
+			goto fail;
+	}
+
+	plane_state->crtc = NULL;
+	plane_state->fb = NULL;
+	plane_state->crtc_x = 0;
+	plane_state->crtc_y = 0;
+	plane_state->crtc_h = 0;
+	plane_state->crtc_w = 0;
+	plane_state->src_x = 0;
+	plane_state->src_y = 0;
+	plane_state->src_h = 0;
+	plane_state->src_w = 0;
+
+	ret = drm_atomic_commit(state);
+	if (ret == -EDEADLK)
+		goto backoff;
+fail:
+	drm_atomic_state_free(state);
+
+	return ret;
+backoff:
+	drm_atomic_legacy_backoff(state);
+	drm_atomic_state_clear(state);
+
+	/*
+	 * Someone might have exchanged the framebuffer while we dropped locks
+	 * in the backoff code. We need to fix up the fb refcount tracking the
+	 * core does for us.
+	 */
+	plane->old_fb = plane->fb;
+
+	goto retry;
+}
+EXPORT_SYMBOL(drm_atomic_helper_disable_plane);
+
+static int update_output_state(struct drm_atomic_state *state,
+			       struct drm_mode_set *set)
+{
+	struct drm_device *dev = set->crtc->dev;
+	struct drm_connector_state *conn_state;
+	struct drm_crtc_state *crtc_state;
+	struct drm_crtc *crtc;
+	int nconnectors = state->dev->mode_config.num_connector;
+	int ncrtcs = state->dev->mode_config.num_crtc;
+	int ret, i, j;
+
+	/* First grab all affected connector/crtc states. */
+	ret = drm_modeset_lock(&dev->mode_config.connection_mutex,
+			       state->acquire_ctx);
+	if (ret)
+		return ret;
+
+	for (i = 0; i < set->num_connectors; i++) {
+		conn_state = drm_atomic_get_connector_state(state,
+							    set->connectors[i]);
+		if (IS_ERR(conn_state))
+			return PTR_ERR(conn_state);
+	}
+
+	for (i = 0; i < ncrtcs; i++) {
+		crtc = state->crtcs[i];
+
+		if (!crtc)
+			continue;
+
+		ret = drm_atomic_add_affected_connectors(state, crtc);
+		if (ret)
+			return ret;
+	}
+
+	for (i = 0; i < nconnectors; i++) {
+		struct drm_connector *connector;
+
+		connector = state->connectors[i];
+		conn_state = state->connector_states[i];
+
+		if (!connector)
+			continue;
+
+		if (conn_state->crtc == crtc)
+			conn_state->crtc = NULL;
+
+		for (j = 0; j < set->num_connectors; j++) {
+			if (set->connectors[j] == connector) {
+				conn_state->crtc = crtc;
+				break;
+			}
+		}
+	}
+
+	for (i = 0; i < ncrtcs; i++) {
+		crtc = state->crtcs[i];
+		crtc_state = state->crtc_states[i];
+
+		if (!crtc)
+			continue;
+
+		crtc_state->enable =
+			drm_atomic_connectors_for_crtc(state, crtc);
+	}
+
+	return 0;
+}
+
+/**
+ * drm_atomic_helper_set_config - set a new config from userspace
+ * @set: mode set configuration
+ *
+ * Provides a default crtc set_config handler using the atomic driver interface.
+ *
+ * Returns:
+ * Returns 0 on success, negative errno numbers on failure.
+ */
+int drm_atomic_helper_set_config(struct drm_mode_set *set)
+{
+	struct drm_atomic_state *state;
+	struct drm_crtc *crtc = set->crtc;
+	struct drm_crtc_state *crtc_state;
+	struct drm_plane_state *primary_state;
+	int ret = 0;
+
+	state = drm_atomic_state_alloc(crtc->dev);
+	if (!state)
+		return -ENOMEM;
+
+	state->acquire_ctx = drm_modeset_legacy_acquire_ctx(crtc);
+retry:
+	crtc_state = drm_atomic_get_crtc_state(state, crtc);
+	if (IS_ERR(crtc_state)) {
+		ret = PTR_ERR(crtc_state);
+		if (ret == -EDEADLK)
+			goto backoff;
+		else
+			goto fail;
+	}
+
+	if (!set->mode) {
+		crtc_state->enable = false;
+		goto commit;
+	}
+
+	crtc_state->enable = true;
+	drm_mode_copy(&crtc_state->mode, set->mode);
+
+	primary_state = drm_atomic_get_plane_state(state, crtc->primary);
+	if (IS_ERR(primary_state)) {
+		ret = PTR_ERR(primary_state);
+		if (ret == -EDEADLK)
+			goto backoff;
+		else
+			goto fail;
+	}
+
+	primary_state->crtc = crtc;
+	primary_state->fb = set->fb;
+	primary_state->crtc_x = 0;
+	primary_state->crtc_y = 0;
+	primary_state->crtc_h = set->mode->hdisplay;
+	primary_state->crtc_w = set->mode->vdisplay;
+	primary_state->src_x = set->x << 16;
+	primary_state->src_y = set->y << 16;
+	primary_state->src_h = set->mode->hdisplay << 16;
+	primary_state->src_w = set->mode->hdisplay << 16;
+
+	ret = update_output_state(state, set);
+	if (ret) {
+		if (ret == -EDEADLK)
+			goto backoff;
+		else
+			goto fail;
+	}
+
+commit:
+	ret = drm_atomic_commit(state);
+	if (ret == -EDEADLK)
+		goto backoff;
+fail:
+	drm_atomic_state_free(state);
+
+	return ret;
+backoff:
+	drm_atomic_legacy_backoff(state);
+	drm_atomic_state_clear(state);
+
+	/*
+	 * Someone might have exchanged the framebuffer while we dropped locks
+	 * in the backoff code. We need to fix up the fb refcount tracking the
+	 * core does for us.
+	 */
+	crtc->primary->old_fb = crtc->primary->fb;
+
+	goto retry;
+}
+
+/**
+ * drm_atomic_helper_crtc_set_property - helper for crtc prorties
+ * @crtc: DRM crtc
+ * @prorty: DRM property
+ * @val: value of property
+ *
+ * Provides a default plane disablle handler using the atomic driver interface.
+ *
+ * RETURNS:
+ * Zero on success, error code on failure
+ */
+int
+drm_atomic_helper_crtc_set_property(struct drm_crtc *crtc,
+				    struct drm_property *property,
+				    uint64_t val)
+{
+	struct drm_atomic_state *state;
+	struct drm_crtc_state *crtc_state;
+	int ret = 0;
+
+	state = drm_atomic_state_alloc(crtc->dev);
+	if (!state)
+		return -ENOMEM;
+
+	/* ->set_property is always called with all locks held. */
+	state->acquire_ctx = crtc->dev->mode_config.acquire_ctx;
+retry:
+	crtc_state = drm_atomic_get_crtc_state(state, crtc);
+	if (IS_ERR(crtc_state)) {
+		ret = PTR_ERR(crtc_state);
+		if (ret == -EDEADLK)
+			goto backoff;
+		else
+			goto fail;
+	}
+
+	ret = crtc->funcs->atomic_set_property(crtc, crtc_state,
+					       property, val);
+	if (ret) {
+		if (ret == -EDEADLK)
+			goto backoff;
+		else
+			goto fail;
+	}
+
+	ret = drm_atomic_commit(state);
+	if (ret == -EDEADLK)
+		goto backoff;
+fail:
+	drm_atomic_state_free(state);
+
+	return ret;
+backoff:
+	drm_atomic_legacy_backoff(state);
+	drm_atomic_state_clear(state);
+
+	goto retry;
+}
+EXPORT_SYMBOL(drm_atomic_helper_crtc_set_property);
+
+/**
+ * drm_atomic_helper_plane_set_property - helper for plane prorties
+ * @plane: DRM plane
+ * @prorty: DRM property
+ * @val: value of property
+ *
+ * Provides a default plane disablle handler using the atomic driver interface.
+ *
+ * RETURNS:
+ * Zero on success, error code on failure
+ */
+int
+drm_atomic_helper_plane_set_property(struct drm_plane *plane,
+				    struct drm_property *property,
+				    uint64_t val)
+{
+	struct drm_atomic_state *state;
+	struct drm_plane_state *plane_state;
+	int ret = 0;
+
+	state = drm_atomic_state_alloc(plane->dev);
+	if (!state)
+		return -ENOMEM;
+
+	/* ->set_property is always called with all locks held. */
+	state->acquire_ctx = plane->dev->mode_config.acquire_ctx;
+retry:
+	plane_state = drm_atomic_get_plane_state(state, plane);
+	if (IS_ERR(plane_state)) {
+		ret = PTR_ERR(plane_state);
+		if (ret == -EDEADLK)
+			goto backoff;
+		else
+			goto fail;
+	}
+
+	ret = plane->funcs->atomic_set_property(plane, plane_state,
+					       property, val);
+	if (ret) {
+		if (ret == -EDEADLK)
+			goto backoff;
+		else
+			goto fail;
+	}
+
+	ret = drm_atomic_commit(state);
+	if (ret == -EDEADLK)
+		goto backoff;
+fail:
+	drm_atomic_state_free(state);
+
+	return ret;
+backoff:
+	drm_atomic_legacy_backoff(state);
+	drm_atomic_state_clear(state);
+
+	goto retry;
+}
+EXPORT_SYMBOL(drm_atomic_helper_plane_set_property);
+
+/**
+ * drm_atomic_helper_connector_set_property - helper for connector prorties
+ * @connector: DRM connector
+ * @prorty: DRM property
+ * @val: value of property
+ *
+ * Provides a default plane disablle handler using the atomic driver interface.
+ *
+ * RETURNS:
+ * Zero on success, error code on failure
+ */
+int
+drm_atomic_helper_connector_set_property(struct drm_connector *connector,
+				    struct drm_property *property,
+				    uint64_t val)
+{
+	struct drm_atomic_state *state;
+	struct drm_connector_state *connector_state;
+	int ret = 0;
+
+	state = drm_atomic_state_alloc(connector->dev);
+	if (!state)
+		return -ENOMEM;
+
+	/* ->set_property is always called with all locks held. */
+	state->acquire_ctx = connector->dev->mode_config.acquire_ctx;
+retry:
+	connector_state = drm_atomic_get_connector_state(state, connector);
+	if (IS_ERR(connector_state)) {
+		ret = PTR_ERR(connector_state);
+		if (ret == -EDEADLK)
+			goto backoff;
+		else
+			goto fail;
+	}
+
+	ret = connector->funcs->atomic_set_property(connector, connector_state,
+					       property, val);
+	if (ret) {
+		if (ret == -EDEADLK)
+			goto backoff;
+		else
+			goto fail;
+	}
+
+	ret = drm_atomic_commit(state);
+	if (ret == -EDEADLK)
+		goto backoff;
+fail:
+	drm_atomic_state_free(state);
+
+	return ret;
+backoff:
+	drm_atomic_legacy_backoff(state);
+	drm_atomic_state_clear(state);
+
+	goto retry;
+}
+EXPORT_SYMBOL(drm_atomic_helper_connector_set_property);
diff --git a/include/drm/drm_atomic_helper.h b/include/drm/drm_atomic_helper.h
index 9781ce739e10..8cd6fe7a48e5 100644
--- a/include/drm/drm_atomic_helper.h
+++ b/include/drm/drm_atomic_helper.h
@@ -49,4 +49,25 @@ void drm_atomic_helper_cleanup_planes(struct drm_device *dev,
 void drm_atomic_helper_swap_state(struct drm_device *dev,
 				  struct drm_atomic_state *state);
 
+/* implementations for legacy interfaces */
+int drm_atomic_helper_update_plane(struct drm_plane *plane,
+				   struct drm_crtc *crtc,
+				   struct drm_framebuffer *fb,
+				   int crtc_x, int crtc_y,
+				   unsigned int crtc_w, unsigned int crtc_h,
+				   uint32_t src_x, uint32_t src_y,
+				   uint32_t src_w, uint32_t src_h);
+int drm_atomic_helper_disable_plane(struct drm_plane *plane);
+int drm_atomic_helper_set_config(struct drm_mode_set *set);
+
+int drm_atomic_helper_crtc_set_property(struct drm_crtc *crtc,
+					struct drm_property *property,
+					uint64_t val);
+int drm_atomic_helper_plane_set_property(struct drm_plane *plane,
+					struct drm_property *property,
+					uint64_t val);
+int drm_atomic_helper_connector_set_property(struct drm_connector *connector,
+					struct drm_property *property,
+					uint64_t val);
+
 #endif /* DRM_ATOMIC_HELPER_H_ */
-- 
2.0.1

[PATCH 15/19] drm/plane-helper: Add async mode to prepare_fb

[Daniel Vetter]

Currently all helpers use ->prepare_fb for synchronous state updates,
so we need the driver callback to wait for any outstanding rendering.

But with async commit we really only want to reserve the framebuffer,
but not stall for rendering. That should be done in the asynchronous
worker.

So add a boolean for this.

Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/drm_atomic_helper.c | 12 +++++++++---
 drivers/gpu/drm/drm_crtc_helper.c   |  2 +-
 drivers/gpu/drm/drm_plane_helper.c  |  4 ++--
 include/drm/drm_atomic_helper.h     |  3 ++-
 include/drm/drm_plane_helper.h      |  3 ++-
 5 files changed, 16 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
index 1b13394cf020..9f7c45b91fe2 100644
--- a/drivers/gpu/drm/drm_atomic_helper.c
+++ b/drivers/gpu/drm/drm_atomic_helper.c
@@ -673,7 +673,7 @@ int drm_atomic_helper_commit(struct drm_device *dev,
 	if (async)
 		return -EBUSY;
 
-	ret = drm_atomic_helper_prepare_planes(dev, state);
+	ret = drm_atomic_helper_prepare_planes(dev, state, false);
 	if (ret)
 		return ret;
 
@@ -719,16 +719,22 @@ EXPORT_SYMBOL(drm_atomic_helper_commit);
  * drm_atomic_helper_prepare_planes - prepare plane resources after commit
  * @dev: DRM device
  * @state: atomic state object with old state structures
+ * @async: asynchronous commit
  *
  * This function prepares plane state, specifically framebuffers, for the new
  * configuration. If any failure is encountered this function will call
  * ->cleanup_fb on any already successfully prepared framebuffer.
  *
+ * If @async is true the driver callbacks should not wait for outstanding
+ * render, but instead ensure that the asynchronous commit work item is stalled
+ * sufficiently long.
+ *
  * Returns:
  * 0 on success, negative error code on failure.
  */
 int drm_atomic_helper_prepare_planes(struct drm_device *dev,
-				     struct drm_atomic_state *state)
+				     struct drm_atomic_state *state,
+				     bool async)
 {
 	int nplanes = dev->mode_config.num_total_plane;
 	int ret, i;
@@ -746,7 +752,7 @@ int drm_atomic_helper_prepare_planes(struct drm_device *dev,
 		fb = state->plane_states[i]->fb;
 
 		if (fb && funcs->prepare_fb) {
-			ret = funcs->prepare_fb(plane, fb);
+			ret = funcs->prepare_fb(plane, fb, async);
 			if (ret)
 				goto fail;
 		}
diff --git a/drivers/gpu/drm/drm_crtc_helper.c b/drivers/gpu/drm/drm_crtc_helper.c
index 7e838104e72e..1382842782f3 100644
--- a/drivers/gpu/drm/drm_crtc_helper.c
+++ b/drivers/gpu/drm/drm_crtc_helper.c
@@ -993,7 +993,7 @@ int drm_helper_crtc_mode_set_base(struct drm_crtc *crtc, int x, int y,
 	}
 
 	if (plane_funcs->prepare_fb) {
-		ret = plane_funcs->prepare_fb(plane, plane_state->fb);
+		ret = plane_funcs->prepare_fb(plane, plane_state->fb, false);
 		if (ret)
 			goto fail;
 	}
diff --git a/drivers/gpu/drm/drm_plane_helper.c b/drivers/gpu/drm/drm_plane_helper.c
index 7befbf017afe..6903f50012c5 100644
--- a/drivers/gpu/drm/drm_plane_helper.c
+++ b/drivers/gpu/drm/drm_plane_helper.c
@@ -435,7 +435,7 @@ int drm_plane_helper_update(struct drm_plane *plane, struct drm_crtc *crtc,
 	}
 
 	if (plane_funcs->prepare_fb) {
-		ret = plane_funcs->prepare_fb(plane, fb);
+		ret = plane_funcs->prepare_fb(plane, fb, false);
 		if (ret)
 			goto fail;
 	}
@@ -514,7 +514,7 @@ int drm_plane_helper_disable(struct drm_plane *plane)
 	}
 
 	if (plane_funcs->prepare_fb) {
-		ret = plane_funcs->prepare_fb(plane, fb);
+		ret = plane_funcs->prepare_fb(plane, fb, false);
 		if (ret)
 			goto fail;
 	}
diff --git a/include/drm/drm_atomic_helper.h b/include/drm/drm_atomic_helper.h
index 8cd6fe7a48e5..0f48e0ffb614 100644
--- a/include/drm/drm_atomic_helper.h
+++ b/include/drm/drm_atomic_helper.h
@@ -40,7 +40,8 @@ void drm_atomic_helper_commit_post_planes(struct drm_device *dev,
 					  struct drm_atomic_state *old_state);
 
 int drm_atomic_helper_prepare_planes(struct drm_device *dev,
-				     struct drm_atomic_state *state);
+				     struct drm_atomic_state *state,
+				     bool async);
 void drm_atomic_helper_commit_planes(struct drm_device *dev,
 				     struct drm_atomic_state *state);
 void drm_atomic_helper_cleanup_planes(struct drm_device *dev,
diff --git a/include/drm/drm_plane_helper.h b/include/drm/drm_plane_helper.h
index a6f3a8ed3394..48543149055b 100644
--- a/include/drm/drm_plane_helper.h
+++ b/include/drm/drm_plane_helper.h
@@ -49,7 +49,8 @@
  */
 struct drm_plane_helper_funcs {
 	int (*prepare_fb)(struct drm_plane *plane,
-			  struct drm_framebuffer *fb);
+			  struct drm_framebuffer *fb,
+			  bool async);
 	void (*cleanup_fb)(struct drm_plane *plane,
 			   struct drm_framebuffer *fb);
 
-- 
2.0.1

[PATCH 16/19] drm/atomic-helpers: document how to implement async commit

[Daniel Vetter]

No helper function to do it all yet provided since no driver has
support for driver core fences yet. Which we'd need to make the
implementation really generic.

Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/drm_atomic_helper.c | 37 +++++++++++++++++++++++++++++++++++++
 1 file changed, 37 insertions(+)

diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
index 9f7c45b91fe2..423f6bcec362 100644
--- a/drivers/gpu/drm/drm_atomic_helper.c
+++ b/drivers/gpu/drm/drm_atomic_helper.c
@@ -716,6 +716,43 @@ int drm_atomic_helper_commit(struct drm_device *dev,
 EXPORT_SYMBOL(drm_atomic_helper_commit);
 
 /**
+ * DOC: Implementing async commit
+ *
+ * For now the atomic helpers don't support async commit directly. If there is
+ * real need it could be added though, using the dma-buf fence infrastructure
+ * for generic synchronization with outstanding rendering.
+ *
+ * For now drivers have to implement async commit themselves, with the following
+ * sequence being the recommened one:
+ *
+ * 1. Run drm_atomic_helper_prepare_planes() first. This is the only function
+ * which commit needs to call which can fail, so we want to run it first and
+ * synchronously.
+ *
+ * 2. Synchronize with any outstanding async commit worker threads which might
+ * affect the new state update. This can be done by either cancelling or
+ * flushing the work items, depending upon whether the driver can deal with
+ * cancelled updates. Note that it is important to ensure that the framebuffer
+ * cleanup is still done when cancelling.
+ *
+ * For sufficient parallelism it is recommended to have a work item per crtc
+ * (for updates which don't touch global state) and a global one. Then we only
+ * need to synchronize with the crtc work items for changed crtcs and the global
+ * work item, which allows nice concurrent updates on disjoint sets of crtcs.
+ *
+ * 3. The software state is updated synchronously with
+ * drm_atomic_helper_swap_state. Doing this under the protection of all modeset
+ * locks means concurrent callers never see inconsistent state. And doing this
+ * while it's guranateed that no relevant async worker runs means that async
+ * workers do not need grab any locks. Actually they must not grab locks, for
+ * otherwise the work flushing will deadlock.
+ *
+ * 4. Schedule a work item to do all subsequent steps, using the split-out
+ * commit helpers: a) pre-plane commit b) plane commit c) post-plane commit and
+ * then cleaning up the framebuffers after one vblank has passed.
+ */
+
+/**
  * drm_atomic_helper_prepare_planes - prepare plane resources after commit
  * @dev: DRM device
  * @state: atomic state object with old state structures
-- 
2.0.1

[PATCH 17/19] drm/atomic-helper: implement ->page_flip

[Daniel Vetter]

Currently there is no way to implement async flips using atomic, that
essentially requires us to be able to cancel pending requests
mid-flight.

To be able to do that (and I guess we want this since vblank synced
updates whic opportunistically cancel still pending updates seem to be
wanted) we'd need to add a mandatory cancellation mode. Depending upon
the exact semantics we decide upon that could mean that userspace will
not get completion events, or will get them all stacked up.

So reject async updates for now. Also async updates usually means not
vblank synced at all, and I guess for drivers which want to support
this they should simply add a special pageflip handler (since usually
you need a special flip cmd to achieve this). That kind of async flip
is pretty much exclusively just used for games and benchmarks where
dropping just one frame means you'll get a headshot or something bad
like that ... And so slight amounts of tearing is acceptable.

Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/drm_atomic_helper.c | 84 +++++++++++++++++++++++++++++++++++++
 include/drm/drm_atomic_helper.h     |  5 +++
 2 files changed, 89 insertions(+)

diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
index 423f6bcec362..b1ef7a1e038c 100644
--- a/drivers/gpu/drm/drm_atomic_helper.c
+++ b/drivers/gpu/drm/drm_atomic_helper.c
@@ -1463,3 +1463,87 @@ backoff:
 	goto retry;
 }
 EXPORT_SYMBOL(drm_atomic_helper_connector_set_property);
+
+/**
+ * drm_atomic_helper_set_config - set a new config from userspace
+ * @crtc: DRM crtc
+ * @fb: DRM framebuffer
+ * @event: optional DRM event to signal upon completion
+ * @flags: flip flags for non-vblank sync'ed updates
+ *
+ * Provides a default page flip using the atomic driver interface.
+ *
+ * Note that for now so called async page flips (i.e. updates which are not
+ * synchronized to vblank) are not supported, since the atomic interfaces have
+ * no provisions for this yet.
+ *
+ * Returns:
+ * Returns 0 on success, negative errno numbers on failure.
+ */
+int drm_atomic_helper_page_flip(struct drm_crtc *crtc,
+				struct drm_framebuffer *fb,
+				struct drm_pending_vblank_event *event,
+				uint32_t flags)
+{
+	struct drm_plane *plane = crtc->primary;
+	struct drm_atomic_state *state;
+	struct drm_plane_state *plane_state;
+	struct drm_crtc_state *crtc_state;
+	int ret = 0;
+
+	if (flags & DRM_MODE_PAGE_FLIP_ASYNC)
+		return -EINVAL;
+
+	state = drm_atomic_state_alloc(plane->dev);
+	if (!state)
+		return -ENOMEM;
+
+	state->acquire_ctx = drm_modeset_legacy_acquire_ctx(crtc);
+retry:
+	crtc_state = drm_atomic_get_crtc_state(state, crtc);
+	if (IS_ERR(crtc_state)) {
+		ret = PTR_ERR(crtc_state);
+		if (ret == -EDEADLK)
+			goto backoff;
+		else
+			goto fail;
+	}
+	crtc_state->event = event;
+
+	plane_state = drm_atomic_get_plane_state(state, plane);
+	if (IS_ERR(plane_state)) {
+		ret = PTR_ERR(plane_state);
+		if (ret == -EDEADLK)
+			goto backoff;
+		else
+			goto fail;
+	}
+
+	plane_state->crtc = crtc;
+	plane_state->fb = fb;
+
+	ret = drm_atomic_async_commit(state);
+	if (ret == -EDEADLK)
+		goto backoff;
+
+	/* Driver takes ownership of state on successful async commit. */
+	if (ret == 0)
+		return 0;
+fail:
+	drm_atomic_state_free(state);
+
+	return ret;
+backoff:
+	drm_atomic_legacy_backoff(state);
+	drm_atomic_state_clear(state);
+
+	/*
+	 * Someone might have exchanged the framebuffer while we dropped locks
+	 * in the backoff code. We need to fix up the fb refcount tracking the
+	 * core does for us.
+	 */
+	plane->old_fb = plane->fb;
+
+	goto retry;
+}
+EXPORT_SYMBOL(drm_atomic_helper_page_flip);
diff --git a/include/drm/drm_atomic_helper.h b/include/drm/drm_atomic_helper.h
index 0f48e0ffb614..3400c2b7e6ae 100644
--- a/include/drm/drm_atomic_helper.h
+++ b/include/drm/drm_atomic_helper.h
@@ -70,5 +70,10 @@ int drm_atomic_helper_plane_set_property(struct drm_plane *plane,
 int drm_atomic_helper_connector_set_property(struct drm_connector *connector,
 					struct drm_property *property,
 					uint64_t val);
+int drm_atomic_helper_page_flip(struct drm_crtc *crtc,
+				struct drm_framebuffer *fb,
+				struct drm_pending_vblank_event *event,
+				uint32_t flags);
+
 
 #endif /* DRM_ATOMIC_HELPER_H_ */
-- 
2.0.1

[PATCH 18/19] drm: trylock modest locking for fbdev panics

[Daniel Vetter]

In the fbdev code we want to do trylocks only to avoid deadlocks and
other ugly issues. Thus far we've only grabbed the overall modeset
lock, but that already failed to exclude a pile of potential
concurrent operations. With proper atomic support this will be worse.

So add a trylock mode to the modeset locking code which attempts all
locks only with trylocks, if possible. We need to track this in the
locking functions themselves and can't restrict this to drivers since
driver-private w/w mutexes must be treated the same way.

There's still the issue that other driver private locks aren't handled
here at all, but well can't have everything. With this we will at
least not regress, even once atomic allows lots of concurrent kms
activity.

Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/drm_fb_helper.c    | 10 ++++----
 drivers/gpu/drm/drm_modeset_lock.c | 51 +++++++++++++++++++++++++++-----------
 include/drm/drm_modeset_lock.h     |  6 +++++
 3 files changed, 47 insertions(+), 20 deletions(-)

diff --git a/drivers/gpu/drm/drm_fb_helper.c b/drivers/gpu/drm/drm_fb_helper.c
index 3144db9dc0f1..66a438ab4e7f 100644
--- a/drivers/gpu/drm/drm_fb_helper.c
+++ b/drivers/gpu/drm/drm_fb_helper.c
@@ -419,11 +419,11 @@ static bool drm_fb_helper_force_kernel_mode(void)
 		if (dev->switch_power_state == DRM_SWITCH_POWER_OFF)
 			continue;
 
-		/* NOTE: we use lockless flag below to avoid grabbing other
-		 * modeset locks.  So just trylock the underlying mutex
-		 * directly:
+		/*
+		 * NOTE: Use trylock mode to avoid deadlocks and sleeping in
+		 * panic context.
 		 */
-		if (!mutex_trylock(&dev->mode_config.mutex)) {
+		if (!__drm_modeset_lock_all(dev, true)) {
 			error = true;
 			continue;
 		}
@@ -432,7 +432,7 @@ static bool drm_fb_helper_force_kernel_mode(void)
 		if (ret)
 			error = true;
 
-		mutex_unlock(&dev->mode_config.mutex);
+		drm_modeset_unlock_all(dev);
 	}
 	return error;
 }
diff --git a/drivers/gpu/drm/drm_modeset_lock.c b/drivers/gpu/drm/drm_modeset_lock.c
index 412b396af740..42ba9ca699ad 100644
--- a/drivers/gpu/drm/drm_modeset_lock.c
+++ b/drivers/gpu/drm/drm_modeset_lock.c
@@ -56,27 +56,27 @@
  */
 
 
-/**
- * drm_modeset_lock_all - take all modeset locks
- * @dev: drm device
- *
- * This function takes all modeset locks, suitable where a more fine-grained
- * scheme isn't (yet) implemented. Locks must be dropped with
- * drm_modeset_unlock_all.
- */
-void drm_modeset_lock_all(struct drm_device *dev)
+int __drm_modeset_lock_all(struct drm_device *dev,
+			   bool trylock)
 {
 	struct drm_mode_config *config = &dev->mode_config;
 	struct drm_modeset_acquire_ctx *ctx;
 	int ret;
 
-	ctx = kzalloc(sizeof(*ctx), GFP_KERNEL);
-	if (WARN_ON(!ctx))
-		return;
+	ctx = kzalloc(sizeof(*ctx),
+		      trylock ? GFP_ATOMIC : GFP_KERNEL);
+	if (!ctx)
+		return -ENOMEM;
 
-	mutex_lock(&config->mutex);
+	if (trylock) {
+		if (!mutex_trylock(&config->mutex))
+			return -EBUSY;
+	} else {
+		mutex_lock(&config->mutex);
+	}
 
 	drm_modeset_acquire_init(ctx, 0);
+	ctx->trylock_only = trylock;
 
 retry:
 	ret = drm_modeset_lock(&config->connection_mutex, ctx);
@@ -95,13 +95,29 @@ retry:
 
 	drm_warn_on_modeset_not_all_locked(dev);
 
-	return;
+	return 0;
 
 fail:
 	if (ret == -EDEADLK) {
 		drm_modeset_backoff(ctx);
 		goto retry;
 	}
+
+	return ret;
+}
+EXPORT_SYMBOL(__drm_modeset_lock_all);
+
+/**
+ * drm_modeset_lock_all - take all modeset locks
+ * @dev: drm device
+ *
+ * This function takes all modeset locks, suitable where a more fine-grained
+ * scheme isn't (yet) implemented. Locks must be dropped with
+ * drm_modeset_unlock_all.
+ */
+void drm_modeset_lock_all(struct drm_device *dev)
+{
+	WARN_ON(__drm_modeset_lock_all(dev, false) != 0);
 }
 EXPORT_SYMBOL(drm_modeset_lock_all);
 
@@ -287,7 +303,12 @@ static inline int modeset_lock(struct drm_modeset_lock *lock,
 
 	WARN_ON(ctx->contended);
 
-	if (interruptible && slow) {
+	if (ctx->trylock_only) {
+		if (!ww_mutex_trylock(&lock->mutex))
+			return -EBUSY;
+		else
+			return 0;
+	} else if (interruptible && slow) {
 		ret = ww_mutex_lock_slow_interruptible(&lock->mutex, &ctx->ww_ctx);
 	} else if (interruptible) {
 		ret = ww_mutex_lock_interruptible(&lock->mutex, &ctx->ww_ctx);
diff --git a/include/drm/drm_modeset_lock.h b/include/drm/drm_modeset_lock.h
index d38e1508f11a..a3f736d24382 100644
--- a/include/drm/drm_modeset_lock.h
+++ b/include/drm/drm_modeset_lock.h
@@ -53,6 +53,11 @@ struct drm_modeset_acquire_ctx {
 	 * list of held locks (drm_modeset_lock)
 	 */
 	struct list_head locked;
+
+	/**
+	 * Trylock mode, use only for panic handlers!
+	 */
+	bool trylock_only;
 };
 
 /**
@@ -123,6 +128,7 @@ struct drm_device;
 struct drm_crtc;
 
 void drm_modeset_lock_all(struct drm_device *dev);
+int __drm_modeset_lock_all(struct drm_device *dev, bool trylock);
 void drm_modeset_unlock_all(struct drm_device *dev);
 void drm_modeset_lock_crtc(struct drm_crtc *crtc);
 void drm_modeset_unlock_crtc(struct drm_crtc *crtc);
-- 
2.0.1

[PATCH 19/19] drm/atomic-helpers: functions for state duplicate/destroy/reset

[Daniel Vetter]

The atomic users and helpers assume that there is always a obj->state
structure around. Which means drivers need to somehow create that at
driver load time. Also it should obviously reset hardware state, so
needs to be reset upon resume.

Finally the destroy/duplicate_state functions are an awful lot of
boilerplate if the driver doesn't need anything beyond the default
state objects.

So add helper functions for all of this.

Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/drm_atomic_helper.c | 21 +++++++++++++++++++++
 include/drm/drm_atomic_helper.h     | 19 +++++++++++++++++++
 2 files changed, 40 insertions(+)

diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
index b1ef7a1e038c..9e188605f72f 100644
--- a/drivers/gpu/drm/drm_atomic_helper.c
+++ b/drivers/gpu/drm/drm_atomic_helper.c
@@ -1547,3 +1547,24 @@ backoff:
 	goto retry;
 }
 EXPORT_SYMBOL(drm_atomic_helper_page_flip);
+
+void drm_atomic_helper_crtc_reset(struct drm_crtc *crtc)
+{
+	kfree(crtc->state);
+	crtc->state = kzalloc(sizeof(*crtc->state), GFP_KERNEL);
+}
+EXPORT_SYMBOL(drm_atomic_helper_crtc_reset);
+
+struct drm_crtc_state *
+drm_atomic_helper_crtc_duplicate_state(struct drm_crtc *crtc)
+{
+	return kmemdup(crtc->state, sizeof(*crtc->state), GFP_KERNEL);
+}
+EXPORT_SYMBOL(drm_atomic_helper_crtc_duplicate_state);
+
+void drm_atomic_helper_crtc_destroy_state(struct drm_crtc *crtc,
+					  struct drm_crtc_state *state)
+{
+	kfree(state);
+}
+EXPORT_SYMBOL(drm_atomic_helper_crtc_destroy_state);
diff --git a/include/drm/drm_atomic_helper.h b/include/drm/drm_atomic_helper.h
index 3400c2b7e6ae..dccc44065abf 100644
--- a/include/drm/drm_atomic_helper.h
+++ b/include/drm/drm_atomic_helper.h
@@ -75,5 +75,24 @@ int drm_atomic_helper_page_flip(struct drm_crtc *crtc,
 				struct drm_pending_vblank_event *event,
 				uint32_t flags);
 
+/* default implementations for state handling */
+void drm_atomic_helper_crtc_reset(struct drm_crtc *crtc);
+struct drm_crtc_state *
+drm_atomic_helper_crtc_duplicate_state(struct drm_crtc *crtc);
+void drm_atomic_helper_crtc_destroy_state(struct drm_crtc *crtc,
+					  struct drm_crtc_state *state);
+
+void drm_atomic_helper_plane_reset(struct drm_plane *plane);
+struct drm_plane_state *
+drm_atomic_helper_plane_duplicate_state(struct drm_plane *plane);
+void drm_atomic_helper_plane_destroy_state(struct drm_plane *plane,
+					  struct drm_plane_state *state);
+
+void drm_atomic_helper_connector_reset(struct drm_connector *connector);
+struct drm_connector_state *
+drm_atomic_helper_connector_duplicate_state(struct drm_connector *connector);
+void drm_atomic_helper_connector_destroy_state(struct drm_connector *connector,
+					  struct drm_connector_state *state);
+
 
 #endif /* DRM_ATOMIC_HELPER_H_ */
-- 
2.0.1

Re: [PATCH 00/19] atomic, remixed

[Daniel Vetter]

On Sun, Jul 27, 2014 at 11:41:29PM +0200, Daniel Vetter wrote:
> - I've added a new set of plane helpers. Mostly this was motivated to make the
>   atomic helpers work on less stellar hardware where you can't just stream the
>   state and then atomically commit with some GO bits. But those helpers should
>   also be useful for more gradual transitions of drivers to the atomic
>   interface. Since a requirement for atomic is to have universal plane support
>   they can be used bare-bones just for that, without all the other atomic
>   baggage (i.e. all the state tracking for crtcs/connectors).

Maybe a short elaboration on how to smoothly transition to atomic is in
order:
1. Implement the plane helpers for the primary plane, use the with the
crtc helper set_config function through the provided ->mode_set_base and
->mode_set callbacks.

2. Expose a proper primary plane with the plan helpers and test it a bit
(since with primary planes we now allow an enabled crtc with the primary
plane off).

3. Convert all other planes (cursor, overlay) over to the primary plane
helpers. You should move any constraint checking for planes into the
atomic_check callback.

4. Roll out your atomic state handling functions for crtc/plane. Using the
provided default handlers in the helper library is probably the best.
Plane/crtc helpers will automatically switch to those when available. From
here on you can also start to track plane/crtc state in your own
subclassed structures.

5. Audit all your ->mode_fixup functions to make sure they don't depend
upon the connector->encoder and encoder->crtc links. That's the big
difference between atomic and crtc helpers.

6. Wire up the set_config implementation from the atomic helpers and test
it.

7. Go through all the connectors individually and convert them to atomic
state handling. If a connector has no driver-private properties then you
can just wire up the default functions and the atomic set_prop
implementation from the helpers. Otherwise you need to subclass the state
structure.

8. Add ->atomic_check functions for crtcs to check additional constraints
that need checking for global modesets. If you have global/shared
resources (e.g. plls) add a modeset_lock for them and grab it if you need
to change anything. Make sure you wire up the backoff logic correctly
(CONFIG_DEBUG_WW_MUTEX_SLOWPATH is your friend).

That's all - each step is fairly well-contained and can be tested on its
own.

Cheers, Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

Re: [PATCH 00/19] atomic, remixed

[Daniel Vetter]

On Sun, Jul 27, 2014 at 11:41:29PM +0200, Daniel Vetter wrote:
> Hi all,
> 
> So I've figured instead of just talking I should draw up my ideas for atomic
> helpers and related stuff in some draft patches. Major differences compared to
> Rob's series:
> 
> - The software side state update is done synchronously, at least when using the
>   helper code. Drivers are free to do whatever they want, as usual. This has a
>   bunch of nice upsides:
> 
>   + All the tricky locking dances can be abolished, and if the ordering and
>     synchronization is done correctly async worker threads don't need any
>     modeset state locking at all. I'm fairly sure that Rob's scheme won't blow
>     up, but I much prefer we don't need it at all.
> 
>   + In Rob's patches any atomic operation must wait for outstanding updates to
>     complete. Otherwise the software state isn't committed yet. With the
>     synchronous updates we can immediately proceed with the state construction
>     and verification and will only block right before applying the state update.
>     And even for that drivers can implement cancellation of the previous update.
>     
>     This has the nice benefit that check-only operations (which compositors need
>     to do right after having completed the current frame to know how to render
>     the next one) can be done without blocking for completion of the previous
>     update.
> 
> - Internal interfaces for atomic updates don't need to go through properties,
>   but can directly set the state. Drivers can always compare updates in their
>   ->check hook later on, so we don't lose expressiveness in the interface. But
>   the resulting code in the callers looks much better.
> 
>   In general I've ditched a lot of interfaces where drivers could overwrite
>   default behaviour and boiled down the interface two state handling functions
>   (duplicate+destroy), setting driver-private properties and check/commit.
> 
> - There is a clear helper separation between core code which should be used to
>   use the atomic interface to drivers, and helper functions. Core never uses
>   helper functions so that we can keep the clear separation we have in all other
>   places in the kms api.
> 
>   There's a set of helpers sitting in-between to implement legacy interfaces on
>   top of atomic. Those _only_ use the core atomic interfaces.
> 
> - I've added a new set of plane helpers. Mostly this was motivated to make the
>   atomic helpers work on less stellar hardware where you can't just stream the
>   state and then atomically commit with some GO bits. But those helpers should
>   also be useful for more gradual transitions of drivers to the atomic
>   interface. Since a requirement for atomic is to have universal plane support
>   they can be used bare-bones just for that, without all the other atomic
>   baggage (i.e. all the state tracking for crtcs/connectors).
> 
> - State tracking for connectors. i915 has piles of relevant connector
>   properties, which we want to save/restore in atomic ops (e.g. hdmi metadata
>   and stuff).
> 
> - fbdev panic locking is implemented with trylock instead of nolock.
> 
> - I've thought a bit about resume. See the proposed state handling for the
>   default reset functions and the addition of a plane reset callback.
> 
> There's also a metric pile of stuff missing:
> 
> - It's completely untested (except for the parts touching currently used code)
>   since I lack suitable hw. Converting i915 is just a bit too much of a pain for
>   a quick w/e code draft ;-)
> 
> - I didn't add all the properties for things currently exposed through ioctl
>   structures. Since the interface here doesn't force internal code to use
>   properties for everything (which really makes things look much better) we
>   don't have a need for that until we merge the actual atomic ioctl. So I left
>   that out.
> 
> - I didn't write code for the fbdev helper to use atomic. The w/e ran out of
>   time for that ...
> 
> - Kerneldoc is occasionally not updated or still missing.
> 
> Anyway I think this draft code is good enough to explain how I'd like to address
> some of the concerns I have with the current atomic code. Comments, flames and
> ideas highly welcome. And if anyone is insane enought to try this on real
> hardware, that would certainly be interesting and I'm very much willing to help
> out as much as possible ... i915 just really isn't suitable since it won't be
> using the helpers, and all the other hw I have here doesn't support planes.

The entire thing is available as a git tree at

	git://people.freedesktop.org/~danvet/drm colder-fusion

I've already pushed a bit more polish on top.
-Daniel

> 
> Cheers, Daniel
> 
> Daniel Vetter (18):
>   drm: Add atomic driver interface definitions for objects
>   drm: Add drm_plane/connector_index
>   drm: Move modeset_lock_all helpers to drm_modeset_lock.[hc]
>   drm: Handle legacy per-crtc locking with full acquire ctx
>   drm: Global atomic state handling
>   drm: Add atomic/plane helpers
>   drm/irq: Implement a generic vblank_wait function
>   drm/i915: Use generic vblank wait
>   drm/plane-helper: transitional atomic plane helpers
>   drm/crtc-helper: Transitional functions using atomic plane helpers
>   drm: Atomic crtc/connector updates using crtc helper interfaces
>   drm: Move ->old_fb from crtc to plane
>   drm/atomic-helper: implementatations for legacy interfaces
>   drm/plane-helper: Add async mode to prepare_fb
>   drm/atomic-helpers: document how to implement async commit
>   drm/atomic-helper: implement ->page_flip
>   drm: trylock modest locking for fbdev panics
>   drm/atomic-helpers: functions for state duplicate/destroy/reset
> 
> Ville Syrjälä (1):
>   drm: Add drm_crtc_vblank_waitqueue()
> 
>  Documentation/DocBook/drm.tmpl       |    1 +
>  drivers/gpu/drm/Makefile             |    4 +-
>  drivers/gpu/drm/drm_atomic.c         |  529 ++++++++++++
>  drivers/gpu/drm/drm_atomic_helper.c  | 1570 ++++++++++++++++++++++++++++++++++
>  drivers/gpu/drm/drm_crtc.c           |  194 ++---
>  drivers/gpu/drm/drm_crtc_helper.c    |  139 +++
>  drivers/gpu/drm/drm_fb_helper.c      |   10 +-
>  drivers/gpu/drm/drm_irq.c            |   45 +
>  drivers/gpu/drm/drm_modeset_lock.c   |  204 ++++-
>  drivers/gpu/drm/drm_plane_helper.c   |  181 +++-
>  drivers/gpu/drm/i915/intel_display.c |   41 +-
>  include/drm/drmP.h                   |   13 +
>  include/drm/drm_atomic.h             |   63 ++
>  include/drm/drm_atomic_helper.h      |   98 +++
>  include/drm/drm_crtc.h               |  208 ++++-
>  include/drm/drm_crtc_helper.h        |   13 +
>  include/drm/drm_modeset_lock.h       |   16 +
>  include/drm/drm_plane_helper.h       |   31 +
>  18 files changed, 3188 insertions(+), 172 deletions(-)
>  create mode 100644 drivers/gpu/drm/drm_atomic.c
>  create mode 100644 drivers/gpu/drm/drm_atomic_helper.c
>  create mode 100644 include/drm/drm_atomic.h
>  create mode 100644 include/drm/drm_atomic_helper.h
> 
> -- 
> 2.0.1
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

Re: [PATCH 00/19] atomic, remixed

[Rob Clark]

On Sun, Jul 27, 2014 at 5:41 PM, Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
> Hi all,
>
> So I've figured instead of just talking I should draw up my ideas for atomic
> helpers and related stuff in some draft patches. Major differences compared to
> Rob's series:
>
> - The software side state update is done synchronously, at least when using the
>   helper code. Drivers are free to do whatever they want, as usual. This has a
>   bunch of nice upsides:
>
>   + All the tricky locking dances can be abolished, and if the ordering and
>     synchronization is done correctly async worker threads don't need any
>     modeset state locking at all. I'm fairly sure that Rob's scheme won't blow
>     up, but I much prefer we don't need it at all.
>
>   + In Rob's patches any atomic operation must wait for outstanding updates to
>     complete. Otherwise the software state isn't committed yet. With the
>     synchronous updates we can immediately proceed with the state construction
>     and verification and will only block right before applying the state update.
>     And even for that drivers can implement cancellation of the previous update.

fwiw, the rough plan here for my version was to (since state was
refcnt'd) eventually allow things to chain up (ie. copying N+1'th
state from N'th state, rather than mode objects themselves) which
could be done without locks for check-only..

That said, I'm not super sold on the idea that userspace needs to be
able to queue up multiple frames ahead.. but it was something that
people ask for from time to time..

>     This has the nice benefit that check-only operations (which compositors need
>     to do right after having completed the current frame to know how to render
>     the next one) can be done without blocking for completion of the previous
>     update.
>
> - Internal interfaces for atomic updates don't need to go through properties,
>   but can directly set the state. Drivers can always compare updates in their
>   ->check hook later on, so we don't lose expressiveness in the interface. But
>   the resulting code in the callers looks much better.

hmm, in cases where drivers have to fwd one property to another object
or other sort of side effect, I think my way was nicer, imho ;-)

Sure, we can figure it out in check_hook later, but it is a bit
awkward.  And means we need to be careful about sequence of checks
(potentially having to re-check) of different objects.  Looks worse to
me ;-)

>   In general I've ditched a lot of interfaces where drivers could overwrite
>   default behaviour and boiled down the interface two state handling functions
>   (duplicate+destroy), setting driver-private properties and check/commit.
>
> - There is a clear helper separation between core code which should be used to
>   use the atomic interface to drivers, and helper functions. Core never uses
>   helper functions so that we can keep the clear separation we have in all other
>   places in the kms api.
>
>   There's a set of helpers sitting in-between to implement legacy interfaces on
>   top of atomic. Those _only_ use the core atomic interfaces.
>
> - I've added a new set of plane helpers. Mostly this was motivated to make the
>   atomic helpers work on less stellar hardware where you can't just stream the
>   state and then atomically commit with some GO bits. But those helpers should
>   also be useful for more gradual transitions of drivers to the atomic
>   interface. Since a requirement for atomic is to have universal plane support
>   they can be used bare-bones just for that, without all the other atomic
>   baggage (i.e. all the state tracking for crtcs/connectors).
>
> - State tracking for connectors. i915 has piles of relevant connector
>   properties, which we want to save/restore in atomic ops (e.g. hdmi metadata
>   and stuff).
>
> - fbdev panic locking is implemented with trylock instead of nolock.
>
> - I've thought a bit about resume. See the proposed state handling for the
>   default reset functions and the addition of a plane reset callback.
>
> There's also a metric pile of stuff missing:
>
> - It's completely untested (except for the parts touching currently used code)
>   since I lack suitable hw. Converting i915 is just a bit too much of a pain for
>   a quick w/e code draft ;-)
>
> - I didn't add all the properties for things currently exposed through ioctl
>   structures. Since the interface here doesn't force internal code to use
>   properties for everything (which really makes things look much better) we
>   don't have a need for that until we merge the actual atomic ioctl. So I left
>   that out.
>
> - I didn't write code for the fbdev helper to use atomic. The w/e ran out of
>   time for that ...
>
> - Kerneldoc is occasionally not updated or still missing.

Unless I'm missing something you also completely lost all the core
crtc and plane check code (except for what you left in legacy
codepaths.. tbh, this was one thing that I liked about my approach is
it unified all the core error checking for legacy and atomic paths..

Anyways, I'm still looking through it.. so may have some more comments later.

BR,
-R


> Anyway I think this draft code is good enough to explain how I'd like to address
> some of the concerns I have with the current atomic code. Comments, flames and
> ideas highly welcome. And if anyone is insane enought to try this on real
> hardware, that would certainly be interesting and I'm very much willing to help
> out as much as possible ... i915 just really isn't suitable since it won't be
> using the helpers, and all the other hw I have here doesn't support planes.
>
> Cheers, Daniel
>
> Daniel Vetter (18):
>   drm: Add atomic driver interface definitions for objects
>   drm: Add drm_plane/connector_index
>   drm: Move modeset_lock_all helpers to drm_modeset_lock.[hc]
>   drm: Handle legacy per-crtc locking with full acquire ctx
>   drm: Global atomic state handling
>   drm: Add atomic/plane helpers
>   drm/irq: Implement a generic vblank_wait function
>   drm/i915: Use generic vblank wait
>   drm/plane-helper: transitional atomic plane helpers
>   drm/crtc-helper: Transitional functions using atomic plane helpers
>   drm: Atomic crtc/connector updates using crtc helper interfaces
>   drm: Move ->old_fb from crtc to plane
>   drm/atomic-helper: implementatations for legacy interfaces
>   drm/plane-helper: Add async mode to prepare_fb
>   drm/atomic-helpers: document how to implement async commit
>   drm/atomic-helper: implement ->page_flip
>   drm: trylock modest locking for fbdev panics
>   drm/atomic-helpers: functions for state duplicate/destroy/reset
>
> Ville Syrjälä (1):
>   drm: Add drm_crtc_vblank_waitqueue()
>
>  Documentation/DocBook/drm.tmpl       |    1 +
>  drivers/gpu/drm/Makefile             |    4 +-
>  drivers/gpu/drm/drm_atomic.c         |  529 ++++++++++++
>  drivers/gpu/drm/drm_atomic_helper.c  | 1570 ++++++++++++++++++++++++++++++++++
>  drivers/gpu/drm/drm_crtc.c           |  194 ++---
>  drivers/gpu/drm/drm_crtc_helper.c    |  139 +++
>  drivers/gpu/drm/drm_fb_helper.c      |   10 +-
>  drivers/gpu/drm/drm_irq.c            |   45 +
>  drivers/gpu/drm/drm_modeset_lock.c   |  204 ++++-
>  drivers/gpu/drm/drm_plane_helper.c   |  181 +++-
>  drivers/gpu/drm/i915/intel_display.c |   41 +-
>  include/drm/drmP.h                   |   13 +
>  include/drm/drm_atomic.h             |   63 ++
>  include/drm/drm_atomic_helper.h      |   98 +++
>  include/drm/drm_crtc.h               |  208 ++++-
>  include/drm/drm_crtc_helper.h        |   13 +
>  include/drm/drm_modeset_lock.h       |   16 +
>  include/drm/drm_plane_helper.h       |   31 +
>  18 files changed, 3188 insertions(+), 172 deletions(-)
>  create mode 100644 drivers/gpu/drm/drm_atomic.c
>  create mode 100644 drivers/gpu/drm/drm_atomic_helper.c
>  create mode 100644 include/drm/drm_atomic.h
>  create mode 100644 include/drm/drm_atomic_helper.h
>
> --
> 2.0.1
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 00/19] atomic, remixed

[Daniel Vetter]

On Mon, Jul 28, 2014 at 1:17 PM, Rob Clark <robdclark@gmail.com> wrote:
> On Sun, Jul 27, 2014 at 5:41 PM, Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
>> Hi all,
>>
>> So I've figured instead of just talking I should draw up my ideas for atomic
>> helpers and related stuff in some draft patches. Major differences compared to
>> Rob's series:
>>
>> - The software side state update is done synchronously, at least when using the
>>   helper code. Drivers are free to do whatever they want, as usual. This has a
>>   bunch of nice upsides:
>>
>>   + All the tricky locking dances can be abolished, and if the ordering and
>>     synchronization is done correctly async worker threads don't need any
>>     modeset state locking at all. I'm fairly sure that Rob's scheme won't blow
>>     up, but I much prefer we don't need it at all.
>>
>>   + In Rob's patches any atomic operation must wait for outstanding updates to
>>     complete. Otherwise the software state isn't committed yet. With the
>>     synchronous updates we can immediately proceed with the state construction
>>     and verification and will only block right before applying the state update.
>>     And even for that drivers can implement cancellation of the previous update.
>
> fwiw, the rough plan here for my version was to (since state was
> refcnt'd) eventually allow things to chain up (ie. copying N+1'th
> state from N'th state, rather than mode objects themselves) which
> could be done without locks for check-only..
>
> That said, I'm not super sold on the idea that userspace needs to be
> able to queue up multiple frames ahead.. but it was something that
> people ask for from time to time..

We should still be able to queue up more than one update I think - the
drm-visible software state will always reflect the latest update, even
when it's not yet put into the hw. Internally the driver can keep a
linked-list (maybe we could move that to the official drm_atomic_state
even) to know where it is exactly with pushing out updates.

One issue though with either approach is canceling updates in-between.
I guess a lot of this will be highly driver-specific. In a way
canceling an entire queue is just a more generic version of the "how
can we cancel the current pending update" issue. So I think we should
postpone the queue problem until we have the normal cancel solved (for
lower-latency gaming).

Also I think queued updates will only be worth it for some very
special workloads like video display, when shutting down both the cpu
and gpu is actually possible and might be worth some power saving. And
hw engineers seem to flip-flop on whether that's worth it for video or
whether it's better to have a slow, but power-sipping special decoder
which feeds out frames constantly instead of the rush-stop-rush-stop
queued frames would need.

>>     This has the nice benefit that check-only operations (which compositors need
>>     to do right after having completed the current frame to know how to render
>>     the next one) can be done without blocking for completion of the previous
>>     update.
>>
>> - Internal interfaces for atomic updates don't need to go through properties,
>>   but can directly set the state. Drivers can always compare updates in their
>>   ->check hook later on, so we don't lose expressiveness in the interface. But
>>   the resulting code in the callers looks much better.
>
> hmm, in cases where drivers have to fwd one property to another object
> or other sort of side effect, I think my way was nicer, imho ;-)

Well for driver-private properties you can do that still. E.g. with
panel upscaling which you internally implement you could push that to
the crtc.

> Sure, we can figure it out in check_hook later, but it is a bit
> awkward.  And means we need to be careful about sequence of checks
> (potentially having to re-check) of different objects.  Looks worse to
> me ;-)

Otoh you can't just chase the connector_state->crtc link, since we
don't promise to deliver the state updates in any order. So that might
still change. I think in the end you really have to do all that
cross-object verification/propagation only in the ->check function,
and can't do it any earlier.

Also I hope that most hw really sets e.g. plane coordinates in their
plane functions, otherwise kms would be a serious misfit ;-) So
hopefully not too much forwarding of state required.

Wrt sequencing checks correctly: We have that problem any way, but the
upside is that the order in which the atomic_check helper callbacks
are run is well-defined. But the order in which ->atomic_set_prop is
run is totally abitrary. So you can't really rely on any ordering in
there (which rather seriously restricts what you can check).

>>   In general I've ditched a lot of interfaces where drivers could overwrite
>>   default behaviour and boiled down the interface two state handling functions
>>   (duplicate+destroy), setting driver-private properties and check/commit.
>>
>> - There is a clear helper separation between core code which should be used to
>>   use the atomic interface to drivers, and helper functions. Core never uses
>>   helper functions so that we can keep the clear separation we have in all other
>>   places in the kms api.
>>
>>   There's a set of helpers sitting in-between to implement legacy interfaces on
>>   top of atomic. Those _only_ use the core atomic interfaces.
>>
>> - I've added a new set of plane helpers. Mostly this was motivated to make the
>>   atomic helpers work on less stellar hardware where you can't just stream the
>>   state and then atomically commit with some GO bits. But those helpers should
>>   also be useful for more gradual transitions of drivers to the atomic
>>   interface. Since a requirement for atomic is to have universal plane support
>>   they can be used bare-bones just for that, without all the other atomic
>>   baggage (i.e. all the state tracking for crtcs/connectors).
>>
>> - State tracking for connectors. i915 has piles of relevant connector
>>   properties, which we want to save/restore in atomic ops (e.g. hdmi metadata
>>   and stuff).
>>
>> - fbdev panic locking is implemented with trylock instead of nolock.
>>
>> - I've thought a bit about resume. See the proposed state handling for the
>>   default reset functions and the addition of a plane reset callback.
>>
>> There's also a metric pile of stuff missing:
>>
>> - It's completely untested (except for the parts touching currently used code)
>>   since I lack suitable hw. Converting i915 is just a bit too much of a pain for
>>   a quick w/e code draft ;-)
>>
>> - I didn't add all the properties for things currently exposed through ioctl
>>   structures. Since the interface here doesn't force internal code to use
>>   properties for everything (which really makes things look much better) we
>>   don't have a need for that until we merge the actual atomic ioctl. So I left
>>   that out.
>>
>> - I didn't write code for the fbdev helper to use atomic. The w/e ran out of
>>   time for that ...
>>
>> - Kerneldoc is occasionally not updated or still missing.
>
> Unless I'm missing something you also completely lost all the core
> crtc and plane check code (except for what you left in legacy
> codepaths.. tbh, this was one thing that I liked about my approach is
> it unified all the core error checking for legacy and atomic paths..

Yeah, one downside of keeping the legacy interfaces as-is is that we
duplicate the checking a bit. But we already have that problem a bit
with primary plane helpers and solved it there by extracting the
checking done by the core so that drivers and helpers could reuse.

A few more of those and I think writing an appropriate
plane->atomic_check function should be fairly trivial. Current
->update_plane functions also duplicate a bunch of that. Also I think
we can reconsider this once we pull in the atomic ioctl. Then it
should be much clearer where the responsibilities for checking which
state lie. We'll probably move a lot down into helper functions for
drivers to use in the various ->atomic_check callbacks.

Cheers, Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

Re: [PATCH 16/19] drm/atomic-helpers: document how to implement async commit

[Rob Clark]

On Sun, Jul 27, 2014 at 5:41 PM, Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
> No helper function to do it all yet provided since no driver has
> support for driver core fences yet. Which we'd need to make the
> implementation really generic.
>
> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> ---
>  drivers/gpu/drm/drm_atomic_helper.c | 37 +++++++++++++++++++++++++++++++++++++
>  1 file changed, 37 insertions(+)
>
> diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
> index 9f7c45b91fe2..423f6bcec362 100644
> --- a/drivers/gpu/drm/drm_atomic_helper.c
> +++ b/drivers/gpu/drm/drm_atomic_helper.c
> @@ -716,6 +716,43 @@ int drm_atomic_helper_commit(struct drm_device *dev,
>  EXPORT_SYMBOL(drm_atomic_helper_commit);
>
>  /**
> + * DOC: Implementing async commit
> + *
> + * For now the atomic helpers don't support async commit directly. If there is
> + * real need it could be added though, using the dma-buf fence infrastructure
> + * for generic synchronization with outstanding rendering.
> + *
> + * For now drivers have to implement async commit themselves, with the following
> + * sequence being the recommened one:
> + *
> + * 1. Run drm_atomic_helper_prepare_planes() first. This is the only function
> + * which commit needs to call which can fail, so we want to run it first and
> + * synchronously.

Without some converted driver to look at for reference, I wasn't
completely sure whether prepare_fb/cleanup_fb was intended to do some
refcnt'ing, etc.   Or just driver internal pin/unpin?

> + * 2. Synchronize with any outstanding async commit worker threads which might
> + * affect the new state update. This can be done by either cancelling or
> + * flushing the work items, depending upon whether the driver can deal with
> + * cancelled updates. Note that it is important to ensure that the framebuffer
> + * cleanup is still done when cancelling.

maybe just an issue of wording..  but if an async update was writing
the state object, then we'd need to block earlier than commit.  I
don't think that is what you meant, because the state object data
structure synchronization you have relies on read-only access from the
async commit.  Which would make me expect it would be sufficient to
move the synchronization part into the async commit worker (ie. it
would only need to sync w/ other workers, ie if you had multiple
work-queues)?


> + *
> + * For sufficient parallelism it is recommended to have a work item per crtc
> + * (for updates which don't touch global state) and a global one. Then we only
> + * need to synchronize with the crtc work items for changed crtcs and the global
> + * work item, which allows nice concurrent updates on disjoint sets of crtcs.

s/work item/work queue/?

> + * 3. The software state is updated synchronously with
> + * drm_atomic_helper_swap_state. Doing this under the protection of all modeset
> + * locks means concurrent callers never see inconsistent state. And doing this
> + * while it's guranateed that no relevant async worker runs means that async
> + * workers do not need grab any locks. Actually they must not grab locks, for
> + * otherwise the work flushing will deadlock.
> + *
> + * 4. Schedule a work item to do all subsequent steps, using the split-out
> + * commit helpers: a) pre-plane commit b) plane commit c) post-plane commit and
> + * then cleaning up the framebuffers after one vblank has passed.

the *old* framebuffers..

I assume the atomic helpers don't really care how the driver does it,
as long as it does.  Somehow or another the driver would want to
schedule cleanup on the vblank of the *old* crtc, I guess.

BR,
-R


> + */
> +
> +/**
>   * drm_atomic_helper_prepare_planes - prepare plane resources after commit
>   * @dev: DRM device
>   * @state: atomic state object with old state structures
> --
> 2.0.1
>
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 16/19] drm/atomic-helpers: document how to implement async commit

[Daniel Vetter]

On Fri, Aug 1, 2014 at 1:52 PM, Rob Clark <robdclark@gmail.com> wrote:
>> + * 1. Run drm_atomic_helper_prepare_planes() first. This is the only function
>> + * which commit needs to call which can fail, so we want to run it first and
>> + * synchronously.
>
> Without some converted driver to look at for reference, I wasn't
> completely sure whether prepare_fb/cleanup_fb was intended to do some
> refcnt'ing, etc.   Or just driver internal pin/unpin?

Reference counting at the drm_framebuffer level is still done in the
core (and imo should stay like that with the real atomic ioctl). So
the prepare/cleanup_fb hook is just for driver-internal stuff like
pin/unpin or relocating to vram or whatever. For generic async support
I also think prepare_fb should set a plane_state->fence pointer if
some waiting is needed. Since not all drivers need this this callback
is optional.

>> + * 2. Synchronize with any outstanding async commit worker threads which might
>> + * affect the new state update. This can be done by either cancelling or
>> + * flushing the work items, depending upon whether the driver can deal with
>> + * cancelled updates. Note that it is important to ensure that the framebuffer
>> + * cleanup is still done when cancelling.
>
> maybe just an issue of wording..  but if an async update was writing
> the state object, then we'd need to block earlier than commit.  I
> don't think that is what you meant, because the state object data
> structure synchronization you have relies on read-only access from the
> async commit.  Which would make me expect it would be sufficient to
> move the synchronization part into the async commit worker (ie. it
> would only need to sync w/ other workers, ie if you had multiple
> work-queues)?

Yeah, wording is unfortunate here, need to improve it. There's two
reasons for the synchronization with workers here:
- We need to do it before swapping states since workers rely on the
read-only access. You're correct that we could postpone this step to
the async worker.
- We need to do it synchronously so that the next user of the atomic
interface (ioctl or fb helper) has the latest state and computes the
relative update correctly. We could make this work with an async
swap-states with some refcounting, but I wanted to avoid that
complexity. If drivers want an internal queue of state updates they
can simply do an aditional copy here on top of the plain swap and
shovel that copy into a list_head queue or something similar.

>> + *
>> + * For sufficient parallelism it is recommended to have a work item per crtc
>> + * (for updates which don't touch global state) and a global one. Then we only
>> + * need to synchronize with the crtc work items for changed crtcs and the global
>> + * work item, which allows nice concurrent updates on disjoint sets of crtcs.
>
> s/work item/work queue/?

work item is the important part here, since with a concurrent queue
you don't need more than one (iirc).

>> + * 3. The software state is updated synchronously with
>> + * drm_atomic_helper_swap_state. Doing this under the protection of all modeset
>> + * locks means concurrent callers never see inconsistent state. And doing this
>> + * while it's guranateed that no relevant async worker runs means that async
>> + * workers do not need grab any locks. Actually they must not grab locks, for
>> + * otherwise the work flushing will deadlock.
>> + *
>> + * 4. Schedule a work item to do all subsequent steps, using the split-out
>> + * commit helpers: a) pre-plane commit b) plane commit c) post-plane commit and
>> + * then cleaning up the framebuffers after one vblank has passed.
>
> the *old* framebuffers..

Yup, will clarify.

> I assume the atomic helpers don't really care how the driver does it,
> as long as it does.  Somehow or another the driver would want to
> schedule cleanup on the vblank of the *old* crtc, I guess.

Yeah. Synchronous atomic commit helper simply waits for one vblank on
all crtcs in the state update (which is probably a bit too much, but
meh for synchronous updates). The plane helper only with the new crtc
(presuming that you can't switch planes before the old crtc stopped
using it).

Cheers, Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch