[PATCH 00/31] Convert omapdrm to the atomic updates API

[PATCH 00/31] Convert omapdrm to the atomic updates API

[Laurent Pinchart]

Hello,

This patch set converts the omapdrm to the atomic update API in 31 small(ish)
and hopefully reviewable steps.

The series is based on the latest drm/next branch and depends on the pending
"drm: omapdrm: Store the rotation property in dev->mode_config" patch by
Daniel Vetter.

The omapdrm loses support for its custom fences in the process. This isn't
deemed to be an issue, as the custom fences API is used by the SGX driver
only, which requires out-of-tree patches anyway. Fences support will be
reimplemented at a later point on top of the atomic updates conversion using
the mainline fence API.

One known issue is a race condition between asynchronous commits and
drm_release() which can trigger a warning if a commit is applied between the
drm_events_release() call and the WARN_ON(!list_empty(&file_priv->event_list)).
Investigation is ongoing to find out the best way to fix the problem, the
problem seems not to be limited to omapdrm (but can be more or less difficult
to reproduce depending on the driver). Ideas would be welcome, I've CC'ed
maintainers of possibly affected drivers.

There's not much else to be said, details are explained in individual patches.

The patches can be found in my git tree at

	git://linuxtv.org/pinchartl/fbdev.git omapdrm/next

Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: Rob Clark <robdclark@gmail.com>
Cc: Thierry Reding <treding@nvidia.com>
Cc: Benjamin Gaignard <benjamin.gaignard@linaro.org>

Laurent Pinchart (31):
  drm: omapdrm: Store the rotation property in dev->mode_config
  drm: omapdrm: Apply settings synchronously
  drm: omapdrm: Rename omap_crtc_page_flip_locked to omap_crtc_page_flip
  drm: omapdrm: Rename omap_crtc page flip-related fields
  drm: omapdrm: Simplify IRQ registration
  drm: omapdrm: Cancel pending page flips when closing device
  drm: omapdrm: Rework page flip handling
  drm: omapdrm: Turn vblank on/off when enabling/disabling CRTC
  drm: omapdrm: Fix page flip race with CRTC disable
  drm: omapdrm: Clean up #include's
  drm: omapdrm: Rename CRTC DSS operations with an omap_crtc_dss_ prefix
  drm: omapdrm: Rework CRTC enable/disable for atomic updates
  drm: omapdrm: Implement encoder .disable() and .enable() operations
  drm: omapdrm: Wire up atomic state object scaffolding
  drm: omapdrm: Implement planes atomic operations
  drm: omapdrm: Handle primary plane config through atomic plane ops
  drm: omapdrm: Switch plane update to atomic helpers
  drm: omapdrm: Switch mode config to atomic helpers
  drm: omapdrm: Switch connector DPMS to atomic helpers
  drm: omapdrm: Replace encoder mode_fixup with atomic_check
  drm: omapdrm: Implement asynchronous commit support
  drm: omapdrm: Switch page flip to atomic helpers
  drm: omapdrm: Drop manual framebuffer pin handling
  drm: omapdrm: Switch crtc and plane set_property to atomic helpers
  drm: omapdrm: Move plane info and win out of the plane structure
  drm: omapdrm: Move crtc info out of the crtc structure
  drm: omapdrm: Remove omap_crtc enabled field
  drm: omapdrm: Remove omap_plane enabled field
  drm: omapdrm: Make the omap_crtc_flush function static
  drm: omapdrm: Don't get/put dispc in omap_crtc_flush()
  drm: omapdrm: omap_crtc_flush() isn't called with modeset locked

 drivers/gpu/drm/omapdrm/omap_connector.c  |  12 +-
 drivers/gpu/drm/omapdrm/omap_crtc.c       | 580 +++++++++++++-----------------
 drivers/gpu/drm/omapdrm/omap_debugfs.c    |   6 +-
 drivers/gpu/drm/omapdrm/omap_dmm_tiler.c  |  19 +-
 drivers/gpu/drm/omapdrm/omap_drv.c        | 162 ++++++++-
 drivers/gpu/drm/omapdrm/omap_drv.h        |  52 +--
 drivers/gpu/drm/omapdrm/omap_encoder.c    |  60 ++--
 drivers/gpu/drm/omapdrm/omap_fb.c         |   8 +-
 drivers/gpu/drm/omapdrm/omap_fbdev.c      |   6 +-
 drivers/gpu/drm/omapdrm/omap_gem.c        |   4 +-
 drivers/gpu/drm/omapdrm/omap_gem_dmabuf.c |   4 +-
 drivers/gpu/drm/omapdrm/omap_irq.c        | 106 ++----
 drivers/gpu/drm/omapdrm/omap_plane.c      | 426 +++++++++-------------
 13 files changed, 656 insertions(+), 789 deletions(-)

-- 
Regards,

Laurent Pinchart

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

[PATCH 01/31] drm: omapdrm: Store the rotation property in dev->mode_config

[Laurent Pinchart]

Rotation is a standard property, store it in
dev->mode_config.rotation_property. While at it, extract the properties
initialization code to a separate function instead of running it for
every plane.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 drivers/gpu/drm/omapdrm/omap_crtc.c  |  4 +---
 drivers/gpu/drm/omapdrm/omap_drv.c   | 31 ++++++++++++++++++++++++++++---
 drivers/gpu/drm/omapdrm/omap_drv.h   |  1 -
 drivers/gpu/drm/omapdrm/omap_plane.c | 27 ++++-----------------------
 4 files changed, 33 insertions(+), 30 deletions(-)

diff --git a/drivers/gpu/drm/omapdrm/omap_crtc.c b/drivers/gpu/drm/omapdrm/omap_crtc.c
index f456544..7a64765 100644
--- a/drivers/gpu/drm/omapdrm/omap_crtc.c
+++ b/drivers/gpu/drm/omapdrm/omap_crtc.c
@@ -646,9 +646,7 @@ static int omap_crtc_page_flip_locked(struct drm_crtc *crtc,
 static int omap_crtc_set_property(struct drm_crtc *crtc,
 		struct drm_property *property, uint64_t val)
 {
-	struct omap_drm_private *priv = crtc->dev->dev_private;
-
-	if (property == priv->rotation_prop) {
+	if (property == crtc->dev->mode_config.rotation_property) {
 		crtc->invert_dimensions =
 				!!(val & ((1LL << DRM_ROTATE_90) | (1LL << DRM_ROTATE_270)));
 	}
diff --git a/drivers/gpu/drm/omapdrm/omap_drv.c b/drivers/gpu/drm/omapdrm/omap_drv.c
index 94920d4..ce6a255 100644
--- a/drivers/gpu/drm/omapdrm/omap_drv.c
+++ b/drivers/gpu/drm/omapdrm/omap_drv.c
@@ -151,6 +151,27 @@ static int omap_modeset_create_crtc(struct drm_device *dev, int id,
 	return 0;
 }
 
+static int omap_modeset_init_properties(struct drm_device *dev)
+{
+	struct omap_drm_private *priv = dev->dev_private;
+
+	if (priv->has_dmm) {
+		dev->mode_config.rotation_property =
+			drm_mode_create_rotation_property(dev,
+				BIT(DRM_ROTATE_0) | BIT(DRM_ROTATE_90) |
+				BIT(DRM_ROTATE_180) | BIT(DRM_ROTATE_270) |
+				BIT(DRM_REFLECT_X) | BIT(DRM_REFLECT_Y));
+		if (!dev->mode_config.rotation_property)
+			return -ENOMEM;
+	}
+
+	priv->zorder_prop = drm_property_create_range(dev, 0, "zorder", 0, 3);
+	if (!priv->zorder_prop)
+		return -ENOMEM;
+
+	return 0;
+}
+
 static int omap_modeset_init(struct drm_device *dev)
 {
 	struct omap_drm_private *priv = dev->dev_private;
@@ -165,6 +186,10 @@ static int omap_modeset_init(struct drm_device *dev)
 
 	omap_drm_irq_install(dev);
 
+	ret = omap_modeset_init_properties(dev);
+	if (ret < 0)
+		return ret;
+
 	/*
 	 * We usually don't want to create a CRTC for each manager, at least
 	 * not until we have a way to expose private planes to userspace.
@@ -583,7 +608,7 @@ static void dev_lastclose(struct drm_device *dev)
 
 	DBG("lastclose: dev=%p", dev);
 
-	if (priv->rotation_prop) {
+	if (dev->mode_config.rotation_property) {
 		/* need to restore default rotation state.. not sure
 		 * if there is a cleaner way to restore properties to
 		 * default state?  Maybe a flag that properties should
@@ -592,12 +617,12 @@ static void dev_lastclose(struct drm_device *dev)
 		 */
 		for (i = 0; i < priv->num_crtcs; i++) {
 			drm_object_property_set_value(&priv->crtcs[i]->base,
-					priv->rotation_prop, 0);
+					dev->mode_config.rotation_property, 0);
 		}
 
 		for (i = 0; i < priv->num_planes; i++) {
 			drm_object_property_set_value(&priv->planes[i]->base,
-					priv->rotation_prop, 0);
+					dev->mode_config.rotation_property, 0);
 		}
 	}
 
diff --git a/drivers/gpu/drm/omapdrm/omap_drv.h b/drivers/gpu/drm/omapdrm/omap_drv.h
index b31c79f..a42a11c 100644
--- a/drivers/gpu/drm/omapdrm/omap_drv.h
+++ b/drivers/gpu/drm/omapdrm/omap_drv.h
@@ -114,7 +114,6 @@ struct omap_drm_private {
 	bool has_dmm;
 
 	/* properties: */
-	struct drm_property *rotation_prop;
 	struct drm_property *zorder_prop;
 
 	/* irq handling: */
diff --git a/drivers/gpu/drm/omapdrm/omap_plane.c b/drivers/gpu/drm/omapdrm/omap_plane.c
index 1c6b63f..a1c9c08 100644
--- a/drivers/gpu/drm/omapdrm/omap_plane.c
+++ b/drivers/gpu/drm/omapdrm/omap_plane.c
@@ -297,33 +297,14 @@ void omap_plane_install_properties(struct drm_plane *plane,
 {
 	struct drm_device *dev = plane->dev;
 	struct omap_drm_private *priv = dev->dev_private;
-	struct drm_property *prop;
 
 	if (priv->has_dmm) {
-		prop = priv->rotation_prop;
-		if (!prop) {
-			prop = drm_mode_create_rotation_property(dev,
-								 BIT(DRM_ROTATE_0) |
-								 BIT(DRM_ROTATE_90) |
-								 BIT(DRM_ROTATE_180) |
-								 BIT(DRM_ROTATE_270) |
-								 BIT(DRM_REFLECT_X) |
-								 BIT(DRM_REFLECT_Y));
-			if (prop == NULL)
-				return;
-			priv->rotation_prop = prop;
-		}
+		struct drm_property *prop = dev->mode_config.rotation_property;
+
 		drm_object_attach_property(obj, prop, 0);
 	}
 
-	prop = priv->zorder_prop;
-	if (!prop) {
-		prop = drm_property_create_range(dev, 0, "zorder", 0, 3);
-		if (prop == NULL)
-			return;
-		priv->zorder_prop = prop;
-	}
-	drm_object_attach_property(obj, prop, 0);
+	drm_object_attach_property(obj, priv->zorder_prop, 0);
 }
 
 int omap_plane_set_property(struct drm_plane *plane,
@@ -333,7 +314,7 @@ int omap_plane_set_property(struct drm_plane *plane,
 	struct omap_drm_private *priv = plane->dev->dev_private;
 	int ret = -EINVAL;
 
-	if (property == priv->rotation_prop) {
+	if (property == plane->dev->mode_config.rotation_property) {
 		DBG("%s: rotation: %02x", omap_plane->name, (uint32_t)val);
 		omap_plane->win.rotation = val;
 		ret = omap_plane_apply(plane);
-- 
2.0.5

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

[PATCH 02/31] drm: omapdrm: Apply settings synchronously

[Laurent Pinchart]

The omapdrm driver implements a mechanism to apply new settings (due to
plane update, plane disable, plane property set, CRTC mode set or CRTC
DPMS) asynchronously. While this improves performance, it adds a level
of complexity that makes transition to the atomic update API close to
impossible. Furthermore the atomic update API requires part of the apply
operations to be synchronous (such as pinning the framebuffers), so the
current implementation needs to be changed.

Simplify the CRTC and plane code by making updates synchronous to
prepare for the switch to the atomic update API. Asynchronous update
will be implemented in a second step.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 drivers/gpu/drm/omapdrm/omap_crtc.c  | 269 +++++++++++++----------------------
 drivers/gpu/drm/omapdrm/omap_drv.c   |   5 -
 drivers/gpu/drm/omapdrm/omap_drv.h   |  23 +--
 drivers/gpu/drm/omapdrm/omap_plane.c | 211 +++++++++++----------------
 4 files changed, 183 insertions(+), 325 deletions(-)

diff --git a/drivers/gpu/drm/omapdrm/omap_crtc.c b/drivers/gpu/drm/omapdrm/omap_crtc.c
index 7a64765..e5fb8c6 100644
--- a/drivers/gpu/drm/omapdrm/omap_crtc.c
+++ b/drivers/gpu/drm/omapdrm/omap_crtc.c
@@ -17,6 +17,8 @@
  * this program.  If not, see <http://www.gnu.org/licenses/>.
  */
 
+#include <linux/completion.h>
+
 #include "omap_drv.h"
 
 #include <drm/drm_mode.h>
@@ -46,36 +48,33 @@ struct omap_crtc {
 	struct omap_video_timings timings;
 	bool enabled;
 
-	struct omap_drm_apply apply;
-
-	struct omap_drm_irq apply_irq;
+	struct omap_drm_irq vblank_irq;
 	struct omap_drm_irq error_irq;
 
-	/* list of in-progress apply's: */
-	struct list_head pending_applies;
-
-	/* list of queued apply's: */
-	struct list_head queued_applies;
-
-	/* for handling queued and in-progress applies: */
-	struct work_struct apply_work;
+	/* list of framebuffers to unpin */
+	struct list_head pending_unpins;
 
 	/* if there is a pending flip, these will be non-null: */
 	struct drm_pending_vblank_event *event;
 	struct drm_framebuffer *old_fb;
 
+	struct completion completion;
+
 	/* for handling page flips without caring about what
 	 * the callback is called from.  Possibly we should just
 	 * make omap_gem always call the cb from the worker so
 	 * we don't have to care about this..
-	 *
-	 * XXX maybe fold into apply_work??
 	 */
 	struct work_struct page_flip_work;
 
 	bool ignore_digit_sync_lost;
 };
 
+struct omap_framebuffer_unpin {
+	struct list_head list;
+	struct drm_framebuffer *fb;
+};
+
 /* -----------------------------------------------------------------------------
  * Helper Functions
  */
@@ -142,7 +141,7 @@ static void omap_crtc_start_update(struct omap_overlay_manager *mgr)
 {
 }
 
-/* Called only from CRTC pre_apply and suspend/resume handlers. */
+/* Called only from omap_crtc_setup and suspend/resume handlers. */
 static void omap_crtc_set_enabled(struct drm_crtc *crtc, bool enable)
 {
 	struct drm_device *dev = crtc->dev;
@@ -261,7 +260,7 @@ static const struct dss_mgr_ops mgr_ops = {
 };
 
 /* -----------------------------------------------------------------------------
- * Apply Logic
+ * Setup and Flush
  */
 
 static void omap_crtc_error_irq(struct omap_drm_irq *irq, uint32_t irqstatus)
@@ -278,121 +277,93 @@ static void omap_crtc_error_irq(struct omap_drm_irq *irq, uint32_t irqstatus)
 	DRM_ERROR_RATELIMITED("%s: errors: %08x\n", omap_crtc->name, irqstatus);
 }
 
-static void omap_crtc_apply_irq(struct omap_drm_irq *irq, uint32_t irqstatus)
+static void omap_crtc_vblank_irq(struct omap_drm_irq *irq, uint32_t irqstatus)
 {
 	struct omap_crtc *omap_crtc =
-			container_of(irq, struct omap_crtc, apply_irq);
-	struct drm_crtc *crtc = &omap_crtc->base;
+			container_of(irq, struct omap_crtc, vblank_irq);
+	struct drm_device *dev = omap_crtc->base.dev;
+	unsigned long flags;
 
-	if (!dispc_mgr_go_busy(omap_crtc->channel)) {
-		struct omap_drm_private *priv =
-				crtc->dev->dev_private;
-		DBG("%s: apply done", omap_crtc->name);
-		__omap_irq_unregister(crtc->dev, &omap_crtc->apply_irq);
-		queue_work(priv->wq, &omap_crtc->apply_work);
-	}
+	if (dispc_mgr_go_busy(omap_crtc->channel))
+		return;
+
+	DBG("%s: apply done", omap_crtc->name);
+	__omap_irq_unregister(dev, &omap_crtc->vblank_irq);
+
+	spin_lock_irqsave(&dev->event_lock, flags);
+
+	/* wakeup userspace */
+	if (omap_crtc->event)
+		drm_send_vblank_event(dev, omap_crtc->pipe, omap_crtc->event);
+
+	omap_crtc->event = NULL;
+	omap_crtc->old_fb = NULL;
+
+	spin_unlock_irqrestore(&dev->event_lock, flags);
+
+	complete(&omap_crtc->completion);
 }
 
-static void apply_worker(struct work_struct *work)
+int omap_crtc_flush(struct drm_crtc *crtc)
 {
-	struct omap_crtc *omap_crtc =
-			container_of(work, struct omap_crtc, apply_work);
-	struct drm_crtc *crtc = &omap_crtc->base;
-	struct drm_device *dev = crtc->dev;
-	struct omap_drm_apply *apply, *n;
-	bool need_apply;
+	struct omap_crtc *omap_crtc = to_omap_crtc(crtc);
+	struct omap_framebuffer_unpin *fb, *next;
 
-	/*
-	 * Synchronize everything on mode_config.mutex, to keep
-	 * the callbacks and list modification all serialized
-	 * with respect to modesetting ioctls from userspace.
-	 */
-	drm_modeset_lock(&crtc->mutex, NULL);
-	dispc_runtime_get();
+	DBG("%s: GO", omap_crtc->name);
 
-	/*
-	 * If we are still pending a previous update, wait.. when the
-	 * pending update completes, we get kicked again.
-	 */
-	if (omap_crtc->apply_irq.registered)
-		goto out;
-
-	/* finish up previous apply's: */
-	list_for_each_entry_safe(apply, n,
-			&omap_crtc->pending_applies, pending_node) {
-		apply->post_apply(apply);
-		list_del(&apply->pending_node);
-	}
+	WARN_ON(!drm_modeset_is_locked(&crtc->mutex));
+	WARN_ON(omap_crtc->vblank_irq.registered);
 
-	need_apply = !list_empty(&omap_crtc->queued_applies);
+	dispc_runtime_get();
 
-	/* then handle the next round of of queued apply's: */
-	list_for_each_entry_safe(apply, n,
-			&omap_crtc->queued_applies, queued_node) {
-		apply->pre_apply(apply);
-		list_del(&apply->queued_node);
-		apply->queued = false;
-		list_add_tail(&apply->pending_node,
-				&omap_crtc->pending_applies);
-	}
+	if (dispc_mgr_is_enabled(omap_crtc->channel)) {
+		dispc_mgr_go(omap_crtc->channel);
+		omap_irq_register(crtc->dev, &omap_crtc->vblank_irq);
 
-	if (need_apply) {
-		enum omap_channel channel = omap_crtc->channel;
+		WARN_ON(!wait_for_completion_timeout(&omap_crtc->completion,
+						     msecs_to_jiffies(100)));
+		reinit_completion(&omap_crtc->completion);
+	}
 
-		DBG("%s: GO", omap_crtc->name);
+	dispc_runtime_put();
 
-		if (dispc_mgr_is_enabled(channel)) {
-			dispc_mgr_go(channel);
-			omap_irq_register(dev, &omap_crtc->apply_irq);
-		} else {
-			struct omap_drm_private *priv = dev->dev_private;
-			queue_work(priv->wq, &omap_crtc->apply_work);
-		}
+	/* Unpin and unreference pending framebuffers. */
+	list_for_each_entry_safe(fb, next, &omap_crtc->pending_unpins, list) {
+		omap_framebuffer_unpin(fb->fb);
+		drm_framebuffer_unreference(fb->fb);
+		list_del(&fb->list);
+		kfree(fb);
 	}
 
-out:
-	dispc_runtime_put();
-	drm_modeset_unlock(&crtc->mutex);
+	return 0;
 }
 
-int omap_crtc_apply(struct drm_crtc *crtc,
-		struct omap_drm_apply *apply)
+int omap_crtc_queue_unpin(struct drm_crtc *crtc, struct drm_framebuffer *fb)
 {
 	struct omap_crtc *omap_crtc = to_omap_crtc(crtc);
+	struct omap_framebuffer_unpin *unpin;
 
-	WARN_ON(!drm_modeset_is_locked(&crtc->mutex));
-
-	/* no need to queue it again if it is already queued: */
-	if (apply->queued)
-		return 0;
-
-	apply->queued = true;
-	list_add_tail(&apply->queued_node, &omap_crtc->queued_applies);
+	unpin = kzalloc(sizeof(*unpin), GFP_KERNEL);
+	if (!unpin)
+		return -ENOMEM;
 
-	/*
-	 * If there are no currently pending updates, then go ahead and
-	 * kick the worker immediately, otherwise it will run again when
-	 * the current update finishes.
-	 */
-	if (list_empty(&omap_crtc->pending_applies)) {
-		struct omap_drm_private *priv = crtc->dev->dev_private;
-		queue_work(priv->wq, &omap_crtc->apply_work);
-	}
+	unpin->fb = fb;
+	list_add_tail(&unpin->list, &omap_crtc->pending_unpins);
 
 	return 0;
 }
 
-static void omap_crtc_pre_apply(struct omap_drm_apply *apply)
+static void omap_crtc_setup(struct drm_crtc *crtc)
 {
-	struct omap_crtc *omap_crtc =
-			container_of(apply, struct omap_crtc, apply);
-	struct drm_crtc *crtc = &omap_crtc->base;
+	struct omap_crtc *omap_crtc = to_omap_crtc(crtc);
 	struct omap_drm_private *priv = crtc->dev->dev_private;
 	struct drm_encoder *encoder = NULL;
 	unsigned int i;
 
 	DBG("%s: enabled=%d", omap_crtc->name, omap_crtc->enabled);
 
+	dispc_runtime_get();
+
 	for (i = 0; i < priv->num_encoders; i++) {
 		if (priv->encoders[i]->crtc == crtc) {
 			encoder = priv->encoders[i];
@@ -416,30 +387,8 @@ static void omap_crtc_pre_apply(struct omap_drm_apply *apply)
 			omap_encoder_set_enabled(encoder, true);
 		}
 	}
-}
-
-static void omap_crtc_post_apply(struct omap_drm_apply *apply)
-{
-	/* nothing needed for post-apply */
-}
 
-void omap_crtc_flush(struct drm_crtc *crtc)
-{
-	struct omap_crtc *omap_crtc = to_omap_crtc(crtc);
-	int loops = 0;
-
-	while (!list_empty(&omap_crtc->pending_applies) ||
-		!list_empty(&omap_crtc->queued_applies) ||
-		omap_crtc->event || omap_crtc->old_fb) {
-
-		if (++loops > 10) {
-			dev_err(crtc->dev->dev,
-				"omap_crtc_flush() timeout\n");
-			break;
-		}
-
-		schedule_timeout_uninterruptible(msecs_to_jiffies(20));
-	}
+	dispc_runtime_put();
 }
 
 /* -----------------------------------------------------------------------------
@@ -452,7 +401,7 @@ static void omap_crtc_destroy(struct drm_crtc *crtc)
 
 	DBG("%s", omap_crtc->name);
 
-	WARN_ON(omap_crtc->apply_irq.registered);
+	WARN_ON(omap_crtc->vblank_irq.registered);
 	omap_irq_unregister(crtc->dev, &omap_crtc->error_irq);
 
 	drm_crtc_cleanup(crtc);
@@ -469,17 +418,21 @@ static void omap_crtc_dpms(struct drm_crtc *crtc, int mode)
 
 	DBG("%s: %d", omap_crtc->name, mode);
 
-	if (enabled != omap_crtc->enabled) {
-		omap_crtc->enabled = enabled;
-		omap_crtc_apply(crtc, &omap_crtc->apply);
+	if (enabled == omap_crtc->enabled)
+		return;
 
-		/* Enable/disable all planes associated with the CRTC. */
-		for (i = 0; i < priv->num_planes; i++) {
-			struct drm_plane *plane = priv->planes[i];
-			if (plane->crtc == crtc)
-				WARN_ON(omap_plane_set_enable(plane, enabled));
-		}
+	/* Enable/disable all planes associated with the CRTC. */
+	for (i = 0; i < priv->num_planes; i++) {
+		struct drm_plane *plane = priv->planes[i];
+
+		if (plane->crtc == crtc)
+			WARN_ON(omap_plane_set_enable(plane, enabled));
 	}
+
+	omap_crtc->enabled = enabled;
+
+	omap_crtc_setup(crtc);
+	omap_crtc_flush(crtc);
 }
 
 static bool omap_crtc_mode_fixup(struct drm_crtc *crtc,
@@ -518,8 +471,7 @@ static int omap_crtc_mode_set(struct drm_crtc *crtc,
 
 	return omap_plane_mode_set(crtc->primary, crtc, crtc->primary->fb,
 				   0, 0, mode->hdisplay, mode->vdisplay,
-				   x, y, mode->hdisplay, mode->vdisplay,
-				   NULL, NULL);
+				   x, y, mode->hdisplay, mode->vdisplay);
 }
 
 static void omap_crtc_prepare(struct drm_crtc *crtc)
@@ -541,36 +493,15 @@ static int omap_crtc_mode_set_base(struct drm_crtc *crtc, int x, int y,
 {
 	struct drm_plane *plane = crtc->primary;
 	struct drm_display_mode *mode = &crtc->mode;
+	int ret;
 
-	return omap_plane_mode_set(plane, crtc, crtc->primary->fb,
-				   0, 0, mode->hdisplay, mode->vdisplay,
-				   x, y, mode->hdisplay, mode->vdisplay,
-				   NULL, NULL);
-}
-
-static void vblank_cb(void *arg)
-{
-	struct drm_crtc *crtc = arg;
-	struct drm_device *dev = crtc->dev;
-	struct omap_crtc *omap_crtc = to_omap_crtc(crtc);
-	unsigned long flags;
-	struct drm_framebuffer *fb;
-
-	spin_lock_irqsave(&dev->event_lock, flags);
-
-	/* wakeup userspace */
-	if (omap_crtc->event)
-		drm_send_vblank_event(dev, omap_crtc->pipe, omap_crtc->event);
-
-	fb = omap_crtc->old_fb;
-
-	omap_crtc->event = NULL;
-	omap_crtc->old_fb = NULL;
-
-	spin_unlock_irqrestore(&dev->event_lock, flags);
+	ret = omap_plane_mode_set(plane, crtc, crtc->primary->fb,
+				  0, 0, mode->hdisplay, mode->vdisplay,
+				  x, y, mode->hdisplay, mode->vdisplay);
+	if (ret < 0)
+		return ret;
 
-	if (fb)
-		drm_framebuffer_unreference(fb);
+	return omap_crtc_flush(crtc);
 }
 
 static void page_flip_worker(struct work_struct *work)
@@ -584,12 +515,13 @@ static void page_flip_worker(struct work_struct *work)
 	drm_modeset_lock(&crtc->mutex, NULL);
 	omap_plane_mode_set(crtc->primary, crtc, crtc->primary->fb,
 			    0, 0, mode->hdisplay, mode->vdisplay,
-			    crtc->x, crtc->y, mode->hdisplay, mode->vdisplay,
-			    vblank_cb, crtc);
+			    crtc->x, crtc->y, mode->hdisplay, mode->vdisplay);
+	omap_crtc_flush(crtc);
 	drm_modeset_unlock(&crtc->mutex);
 
 	bo = omap_framebuffer_bo(crtc->primary->fb, 0);
 	drm_gem_object_unreference_unlocked(bo);
+	drm_framebuffer_unreference(crtc->primary->fb);
 }
 
 static void page_flip_cb(void *arg)
@@ -709,20 +641,17 @@ struct drm_crtc *omap_crtc_init(struct drm_device *dev,
 	crtc = &omap_crtc->base;
 
 	INIT_WORK(&omap_crtc->page_flip_work, page_flip_worker);
-	INIT_WORK(&omap_crtc->apply_work, apply_worker);
 
-	INIT_LIST_HEAD(&omap_crtc->pending_applies);
-	INIT_LIST_HEAD(&omap_crtc->queued_applies);
+	INIT_LIST_HEAD(&omap_crtc->pending_unpins);
 
-	omap_crtc->apply.pre_apply  = omap_crtc_pre_apply;
-	omap_crtc->apply.post_apply = omap_crtc_post_apply;
+	init_completion(&omap_crtc->completion);
 
 	omap_crtc->channel = channel;
 	omap_crtc->name = channel_names[channel];
 	omap_crtc->pipe = id;
 
-	omap_crtc->apply_irq.irqmask = pipe2vbl(crtc);
-	omap_crtc->apply_irq.irq = omap_crtc_apply_irq;
+	omap_crtc->vblank_irq.irqmask = pipe2vbl(crtc);
+	omap_crtc->vblank_irq.irq = omap_crtc_vblank_irq;
 
 	omap_crtc->error_irq.irqmask =
 			dispc_mgr_get_sync_lost_irq(channel);
diff --git a/drivers/gpu/drm/omapdrm/omap_drv.c b/drivers/gpu/drm/omapdrm/omap_drv.c
index ce6a255..bf02121 100644
--- a/drivers/gpu/drm/omapdrm/omap_drv.c
+++ b/drivers/gpu/drm/omapdrm/omap_drv.c
@@ -550,7 +550,6 @@ static int dev_load(struct drm_device *dev, unsigned long flags)
 static int dev_unload(struct drm_device *dev)
 {
 	struct omap_drm_private *priv = dev->dev_private;
-	int i;
 
 	DBG("unload: dev=%p", dev);
 
@@ -559,10 +558,6 @@ static int dev_unload(struct drm_device *dev)
 	if (priv->fbdev)
 		omap_fbdev_free(dev);
 
-	/* flush crtcs so the fbs get released */
-	for (i = 0; i < priv->num_crtcs; i++)
-		omap_crtc_flush(priv->crtcs[i]);
-
 	omap_modeset_free(dev);
 	omap_gem_deinit(dev);
 
diff --git a/drivers/gpu/drm/omapdrm/omap_drv.h b/drivers/gpu/drm/omapdrm/omap_drv.h
index a42a11c..6c0cb46 100644
--- a/drivers/gpu/drm/omapdrm/omap_drv.h
+++ b/drivers/gpu/drm/omapdrm/omap_drv.h
@@ -50,21 +50,6 @@ struct omap_drm_window {
 	uint32_t src_w, src_h;
 };
 
-/* Once GO bit is set, we can't make further updates to shadowed registers
- * until the GO bit is cleared.  So various parts in the kms code that need
- * to update shadowed registers queue up a pair of callbacks, pre_apply
- * which is called before setting GO bit, and post_apply that is called
- * after GO bit is cleared.  The crtc manages the queuing, and everyone
- * else goes thru omap_crtc_apply() using these callbacks so that the
- * code which has to deal w/ GO bit state is centralized.
- */
-struct omap_drm_apply {
-	struct list_head pending_node, queued_node;
-	bool queued;
-	void (*pre_apply)(struct omap_drm_apply *apply);
-	void (*post_apply)(struct omap_drm_apply *apply);
-};
-
 /* For transiently registering for different DSS irqs that various parts
  * of the KMS code need during setup/configuration.  We these are not
  * necessarily the same as what drm_vblank_get/put() are requesting, and
@@ -153,13 +138,12 @@ void omap_fbdev_free(struct drm_device *dev);
 
 const struct omap_video_timings *omap_crtc_timings(struct drm_crtc *crtc);
 enum omap_channel omap_crtc_channel(struct drm_crtc *crtc);
-int omap_crtc_apply(struct drm_crtc *crtc,
-		struct omap_drm_apply *apply);
+int omap_crtc_flush(struct drm_crtc *crtc);
+int omap_crtc_queue_unpin(struct drm_crtc *crtc, struct drm_framebuffer *fb);
 void omap_crtc_pre_init(void);
 void omap_crtc_pre_uninit(void);
 struct drm_crtc *omap_crtc_init(struct drm_device *dev,
 		struct drm_plane *plane, enum omap_channel channel, int id);
-void omap_crtc_flush(struct drm_crtc *crtc);
 
 struct drm_plane *omap_plane_init(struct drm_device *dev,
 		int id, enum drm_plane_type type);
@@ -169,8 +153,7 @@ int omap_plane_mode_set(struct drm_plane *plane,
 			int crtc_x, int crtc_y,
 			unsigned int crtc_w, unsigned int crtc_h,
 			unsigned int src_x, unsigned int src_y,
-			unsigned int src_w, unsigned int src_h,
-			void (*fxn)(void *), void *arg);
+			unsigned int src_w, unsigned int src_h);
 void omap_plane_install_properties(struct drm_plane *plane,
 		struct drm_mode_object *obj);
 int omap_plane_set_property(struct drm_plane *plane,
diff --git a/drivers/gpu/drm/omapdrm/omap_plane.c b/drivers/gpu/drm/omapdrm/omap_plane.c
index a1c9c08..99663ec 100644
--- a/drivers/gpu/drm/omapdrm/omap_plane.c
+++ b/drivers/gpu/drm/omapdrm/omap_plane.c
@@ -17,8 +17,6 @@
  * this program.  If not, see <http://www.gnu.org/licenses/>.
  */
 
-#include "drm_flip_work.h"
-
 #include "omap_drv.h"
 #include "omap_dmm_tiler.h"
 
@@ -32,11 +30,6 @@
  * plane funcs
  */
 
-struct callback {
-	void (*fxn)(void *);
-	void *arg;
-};
-
 #define to_omap_plane(x) container_of(x, struct omap_plane, base)
 
 struct omap_plane {
@@ -44,7 +37,6 @@ struct omap_plane {
 	int id;  /* TODO rename omap_plane -> omap_plane_id in omapdss so I can use the enum */
 	const char *name;
 	struct omap_overlay_info info;
-	struct omap_drm_apply apply;
 
 	/* position/orientation of scanout within the fb: */
 	struct omap_drm_window win;
@@ -57,91 +49,66 @@ struct omap_plane {
 	uint32_t formats[32];
 
 	struct omap_drm_irq error_irq;
-
-	/* for deferring bo unpin's until next post_apply(): */
-	struct drm_flip_work unpin_work;
-
-	// XXX maybe get rid of this and handle vblank in crtc too?
-	struct callback apply_done_cb;
 };
 
-static void omap_plane_unpin_worker(struct drm_flip_work *work, void *val)
-{
-	struct omap_plane *omap_plane =
-			container_of(work, struct omap_plane, unpin_work);
-	struct drm_device *dev = omap_plane->base.dev;
-
-	/*
-	 * omap_framebuffer_pin/unpin are always called from priv->wq,
-	 * so there's no need for locking here.
-	 */
-	omap_framebuffer_unpin(val);
-	mutex_lock(&dev->mode_config.mutex);
-	drm_framebuffer_unreference(val);
-	mutex_unlock(&dev->mode_config.mutex);
-}
-
 /* update which fb (if any) is pinned for scanout */
-static int omap_plane_update_pin(struct drm_plane *plane,
-				 struct drm_framebuffer *fb)
+static int omap_plane_update_pin(struct drm_plane *plane)
 {
 	struct omap_plane *omap_plane = to_omap_plane(plane);
 	struct drm_framebuffer *pinned_fb = omap_plane->pinned_fb;
+	struct drm_framebuffer *fb = omap_plane->enabled ? plane->fb : NULL;
+	int ret = 0;
 
-	if (pinned_fb != fb) {
-		int ret = 0;
-
-		DBG("%p -> %p", pinned_fb, fb);
+	if (pinned_fb == fb)
+		return 0;
 
-		if (fb) {
-			drm_framebuffer_reference(fb);
-			ret = omap_framebuffer_pin(fb);
-		}
+	DBG("%p -> %p", pinned_fb, fb);
 
-		if (pinned_fb)
-			drm_flip_work_queue(&omap_plane->unpin_work, pinned_fb);
+	if (fb) {
+		drm_framebuffer_reference(fb);
+		ret = omap_framebuffer_pin(fb);
+	}
 
-		if (ret) {
-			dev_err(plane->dev->dev, "could not swap %p -> %p\n",
-					omap_plane->pinned_fb, fb);
-			drm_framebuffer_unreference(fb);
-			omap_plane->pinned_fb = NULL;
-			return ret;
-		}
+	if (pinned_fb)
+		omap_crtc_queue_unpin(plane->crtc, pinned_fb);
 
-		omap_plane->pinned_fb = fb;
+	if (ret) {
+		dev_err(plane->dev->dev, "could not swap %p -> %p\n",
+				omap_plane->pinned_fb, fb);
+		drm_framebuffer_unreference(fb);
+		omap_plane->pinned_fb = NULL;
+		return ret;
 	}
 
+	omap_plane->pinned_fb = fb;
+
 	return 0;
 }
 
-static void omap_plane_pre_apply(struct omap_drm_apply *apply)
+static int omap_plane_setup(struct omap_plane *omap_plane)
 {
-	struct omap_plane *omap_plane =
-			container_of(apply, struct omap_plane, apply);
-	struct omap_drm_window *win = &omap_plane->win;
+	struct omap_overlay_info *info = &omap_plane->info;
 	struct drm_plane *plane = &omap_plane->base;
 	struct drm_device *dev = plane->dev;
-	struct omap_overlay_info *info = &omap_plane->info;
 	struct drm_crtc *crtc = plane->crtc;
-	enum omap_channel channel;
-	bool enabled = omap_plane->enabled && crtc;
 	int ret;
 
-	DBG("%s, enabled=%d", omap_plane->name, enabled);
+	DBG("%s, enabled=%d", omap_plane->name, omap_plane->enabled);
 
 	/* if fb has changed, pin new fb: */
-	omap_plane_update_pin(plane, enabled ? plane->fb : NULL);
+	ret = omap_plane_update_pin(plane);
+	if (ret)
+		return ret;
+
+	dispc_runtime_get();
 
-	if (!enabled) {
+	if (!omap_plane->enabled) {
 		dispc_ovl_enable(omap_plane->id, false);
-		return;
+		goto done;
 	}
 
-	channel = omap_crtc_channel(crtc);
-
 	/* update scanout: */
-	omap_framebuffer_update_scanout(plane->fb, win, info);
+	omap_framebuffer_update_scanout(plane->fb, &omap_plane->win, info);
 
 	DBG("%dx%d -> %dx%d (%d)", info->width, info->height,
 			info->out_width, info->out_height,
@@ -149,43 +116,22 @@ static void omap_plane_pre_apply(struct omap_drm_apply *apply)
 	DBG("%d,%d %pad %pad", info->pos_x, info->pos_y,
 			&info->paddr, &info->p_uv_addr);
 
-	dispc_ovl_set_channel_out(omap_plane->id, channel);
+	dispc_ovl_set_channel_out(omap_plane->id,
+				  omap_crtc_channel(crtc));
 
 	/* and finally, update omapdss: */
 	ret = dispc_ovl_setup(omap_plane->id, info, false,
 			      omap_crtc_timings(crtc), false);
 	if (ret) {
 		dev_err(dev->dev, "dispc_ovl_setup failed: %d\n", ret);
-		return;
+		goto done;
 	}
 
 	dispc_ovl_enable(omap_plane->id, true);
-}
-
-static void omap_plane_post_apply(struct omap_drm_apply *apply)
-{
-	struct omap_plane *omap_plane =
-			container_of(apply, struct omap_plane, apply);
-	struct drm_plane *plane = &omap_plane->base;
-	struct omap_drm_private *priv = plane->dev->dev_private;
-	struct callback cb;
-
-	cb = omap_plane->apply_done_cb;
-	omap_plane->apply_done_cb.fxn = NULL;
-
-	drm_flip_work_commit(&omap_plane->unpin_work, priv->wq);
-
-	if (cb.fxn)
-		cb.fxn(cb.arg);
-}
 
-static int omap_plane_apply(struct drm_plane *plane)
-{
-	if (plane->crtc) {
-		struct omap_plane *omap_plane = to_omap_plane(plane);
-		return omap_crtc_apply(plane->crtc, &omap_plane->apply);
-	}
-	return 0;
+done:
+	dispc_runtime_put();
+	return ret;
 }
 
 int omap_plane_mode_set(struct drm_plane *plane,
@@ -193,8 +139,7 @@ int omap_plane_mode_set(struct drm_plane *plane,
 			int crtc_x, int crtc_y,
 			unsigned int crtc_w, unsigned int crtc_h,
 			unsigned int src_x, unsigned int src_y,
-			unsigned int src_w, unsigned int src_h,
-			void (*fxn)(void *), void *arg)
+			unsigned int src_w, unsigned int src_h)
 {
 	struct omap_plane *omap_plane = to_omap_plane(plane);
 	struct omap_drm_window *win = &omap_plane->win;
@@ -209,17 +154,18 @@ int omap_plane_mode_set(struct drm_plane *plane,
 	win->src_w = src_w;
 	win->src_h = src_h;
 
-	if (fxn) {
-		/* omap_crtc should ensure that a new page flip
-		 * isn't permitted while there is one pending:
-		 */
-		BUG_ON(omap_plane->apply_done_cb.fxn);
+	return omap_plane_setup(omap_plane);
+}
 
-		omap_plane->apply_done_cb.fxn = fxn;
-		omap_plane->apply_done_cb.arg = arg;
-	}
+int omap_plane_set_enable(struct drm_plane *plane, bool enable)
+{
+	struct omap_plane *omap_plane = to_omap_plane(plane);
 
-	return omap_plane_apply(plane);
+	if (enable == omap_plane->enabled)
+		return 0;
+
+	omap_plane->enabled = enable;
+	return omap_plane_setup(omap_plane);
 }
 
 static int omap_plane_update(struct drm_plane *plane,
@@ -230,6 +176,8 @@ static int omap_plane_update(struct drm_plane *plane,
 		uint32_t src_w, uint32_t src_h)
 {
 	struct omap_plane *omap_plane = to_omap_plane(plane);
+	int ret;
+
 	omap_plane->enabled = true;
 
 	/* omap_plane_mode_set() takes adjusted src */
@@ -248,10 +196,14 @@ static int omap_plane_update(struct drm_plane *plane,
 	plane->crtc = crtc;
 
 	/* src values are in Q16 fixed point, convert to integer: */
-	return omap_plane_mode_set(plane, crtc, fb,
-			crtc_x, crtc_y, crtc_w, crtc_h,
-			src_x >> 16, src_y >> 16, src_w >> 16, src_h >> 16,
-			NULL, NULL);
+	ret = omap_plane_mode_set(plane, crtc, fb,
+				  crtc_x, crtc_y, crtc_w, crtc_h,
+				  src_x >> 16, src_y >> 16,
+				  src_w >> 16, src_h >> 16);
+	if (ret < 0)
+		return ret;
+
+	return omap_crtc_flush(plane->crtc);
 }
 
 static int omap_plane_disable(struct drm_plane *plane)
@@ -262,7 +214,14 @@ static int omap_plane_disable(struct drm_plane *plane)
 	omap_plane->info.zorder = plane->type == DRM_PLANE_TYPE_PRIMARY
 				? 0 : omap_plane->id;
 
-	return omap_plane_set_enable(plane, false);
+	if (!omap_plane->enabled)
+		return 0;
+
+	/* Disabling a plane never fails. */
+	omap_plane->enabled = false;
+	omap_plane_setup(omap_plane);
+
+	return omap_crtc_flush(plane->crtc);
 }
 
 static void omap_plane_destroy(struct drm_plane *plane)
@@ -275,22 +234,9 @@ static void omap_plane_destroy(struct drm_plane *plane)
 
 	drm_plane_cleanup(plane);
 
-	drm_flip_work_cleanup(&omap_plane->unpin_work);
-
 	kfree(omap_plane);
 }
 
-int omap_plane_set_enable(struct drm_plane *plane, bool enable)
-{
-	struct omap_plane *omap_plane = to_omap_plane(plane);
-
-	if (enable == omap_plane->enabled)
-		return 0;
-
-	omap_plane->enabled = enable;
-	return omap_plane_apply(plane);
-}
-
 /* helper to install properties which are common to planes and crtcs */
 void omap_plane_install_properties(struct drm_plane *plane,
 		struct drm_mode_object *obj)
@@ -312,19 +258,30 @@ int omap_plane_set_property(struct drm_plane *plane,
 {
 	struct omap_plane *omap_plane = to_omap_plane(plane);
 	struct omap_drm_private *priv = plane->dev->dev_private;
-	int ret = -EINVAL;
+	int ret;
 
 	if (property == plane->dev->mode_config.rotation_property) {
 		DBG("%s: rotation: %02x", omap_plane->name, (uint32_t)val);
 		omap_plane->win.rotation = val;
-		ret = omap_plane_apply(plane);
 	} else if (property == priv->zorder_prop) {
 		DBG("%s: zorder: %02x", omap_plane->name, (uint32_t)val);
 		omap_plane->info.zorder = val;
-		ret = omap_plane_apply(plane);
+	} else {
+		return -EINVAL;
 	}
 
-	return ret;
+	/*
+	 * We're done if the plane is disabled, properties will be applied the
+	 * next time it becomes enabled.
+	 */
+	if (!omap_plane->enabled)
+		return 0;
+
+	ret = omap_plane_setup(omap_plane);
+	if (ret < 0)
+		return ret;
+
+	return omap_crtc_flush(plane->crtc);
 }
 
 static const struct drm_plane_funcs omap_plane_funcs = {
@@ -372,9 +329,6 @@ struct drm_plane *omap_plane_init(struct drm_device *dev,
 	if (!omap_plane)
 		return ERR_PTR(-ENOMEM);
 
-	drm_flip_work_init(&omap_plane->unpin_work,
-			"unpin", omap_plane_unpin_worker);
-
 	omap_plane->nformats = omap_framebuffer_get_formats(
 			omap_plane->formats, ARRAY_SIZE(omap_plane->formats),
 			dss_feat_get_supported_color_modes(id));
@@ -383,9 +337,6 @@ struct drm_plane *omap_plane_init(struct drm_device *dev,
 
 	plane = &omap_plane->base;
 
-	omap_plane->apply.pre_apply  = omap_plane_pre_apply;
-	omap_plane->apply.post_apply = omap_plane_post_apply;
-
 	omap_plane->error_irq.irqmask = error_irqs[id];
 	omap_plane->error_irq.irq = omap_plane_error_irq;
 	omap_irq_register(dev, &omap_plane->error_irq);
-- 
2.0.5

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

[PATCH 03/31] drm: omapdrm: Rename omap_crtc_page_flip_locked to omap_crtc_page_flip

[Laurent Pinchart]

The operation is called page_flip, rename its implementation
accordingly.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 drivers/gpu/drm/omapdrm/omap_crtc.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/omapdrm/omap_crtc.c b/drivers/gpu/drm/omapdrm/omap_crtc.c
index e5fb8c6..7ef9147 100644
--- a/drivers/gpu/drm/omapdrm/omap_crtc.c
+++ b/drivers/gpu/drm/omapdrm/omap_crtc.c
@@ -534,10 +534,10 @@ static void page_flip_cb(void *arg)
 	queue_work(priv->wq, &omap_crtc->page_flip_work);
 }
 
-static int omap_crtc_page_flip_locked(struct drm_crtc *crtc,
-		 struct drm_framebuffer *fb,
-		 struct drm_pending_vblank_event *event,
-		 uint32_t page_flip_flags)
+static int omap_crtc_page_flip(struct drm_crtc *crtc,
+			       struct drm_framebuffer *fb,
+			       struct drm_pending_vblank_event *event,
+			       uint32_t page_flip_flags)
 {
 	struct drm_device *dev = crtc->dev;
 	struct omap_crtc *omap_crtc = to_omap_crtc(crtc);
@@ -589,7 +589,7 @@ static int omap_crtc_set_property(struct drm_crtc *crtc,
 static const struct drm_crtc_funcs omap_crtc_funcs = {
 	.set_config = drm_crtc_helper_set_config,
 	.destroy = omap_crtc_destroy,
-	.page_flip = omap_crtc_page_flip_locked,
+	.page_flip = omap_crtc_page_flip,
 	.set_property = omap_crtc_set_property,
 };
 
-- 
2.0.5

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

[PATCH 04/31] drm: omapdrm: Rename omap_crtc page flip-related fields

[Laurent Pinchart]

The old_fb field is only used to indicate whether a page flip is
pending. Turn it into a bool named flip_pending. Rename event and
page_flip_work to flip_event and flip_work for consistency.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 drivers/gpu/drm/omapdrm/omap_crtc.c | 47 ++++++++++++++++++++-----------------
 1 file changed, 26 insertions(+), 21 deletions(-)

diff --git a/drivers/gpu/drm/omapdrm/omap_crtc.c b/drivers/gpu/drm/omapdrm/omap_crtc.c
index 7ef9147..85129d5 100644
--- a/drivers/gpu/drm/omapdrm/omap_crtc.c
+++ b/drivers/gpu/drm/omapdrm/omap_crtc.c
@@ -54,19 +54,22 @@ struct omap_crtc {
 	/* list of framebuffers to unpin */
 	struct list_head pending_unpins;
 
-	/* if there is a pending flip, these will be non-null: */
-	struct drm_pending_vblank_event *event;
-	struct drm_framebuffer *old_fb;
+	/*
+	 * The flip_pending flag indicates if a page flip has been queued and
+	 * hasn't completed yet. The flip event, if any, is stored in
+	 * flip_event.
+	 *
+	 * The flip_work work queue handles page flip requests without caring
+	 * about what context the GEM async callback is called from. Possibly we
+	 * should just make omap_gem always call the cb from the worker so we
+	 * don't have to care about this.
+	 */
+	bool flip_pending;
+	struct drm_pending_vblank_event *flip_event;
+	struct work_struct flip_work;
 
 	struct completion completion;
 
-	/* for handling page flips without caring about what
-	 * the callback is called from.  Possibly we should just
-	 * make omap_gem always call the cb from the worker so
-	 * we don't have to care about this..
-	 */
-	struct work_struct page_flip_work;
-
 	bool ignore_digit_sync_lost;
 };
 
@@ -293,11 +296,12 @@ static void omap_crtc_vblank_irq(struct omap_drm_irq *irq, uint32_t irqstatus)
 	spin_lock_irqsave(&dev->event_lock, flags);
 
 	/* wakeup userspace */
-	if (omap_crtc->event)
-		drm_send_vblank_event(dev, omap_crtc->pipe, omap_crtc->event);
+	if (omap_crtc->flip_event)
+		drm_send_vblank_event(dev, omap_crtc->pipe,
+				      omap_crtc->flip_event);
 
-	omap_crtc->event = NULL;
-	omap_crtc->old_fb = NULL;
+	omap_crtc->flip_event = NULL;
+	omap_crtc->flip_pending = false;
 
 	spin_unlock_irqrestore(&dev->event_lock, flags);
 
@@ -507,7 +511,7 @@ static int omap_crtc_mode_set_base(struct drm_crtc *crtc, int x, int y,
 static void page_flip_worker(struct work_struct *work)
 {
 	struct omap_crtc *omap_crtc =
-			container_of(work, struct omap_crtc, page_flip_work);
+			container_of(work, struct omap_crtc, flip_work);
 	struct drm_crtc *crtc = &omap_crtc->base;
 	struct drm_display_mode *mode = &crtc->mode;
 	struct drm_gem_object *bo;
@@ -531,7 +535,7 @@ static void page_flip_cb(void *arg)
 	struct omap_drm_private *priv = crtc->dev->dev_private;
 
 	/* avoid assumptions about what ctxt we are called from: */
-	queue_work(priv->wq, &omap_crtc->page_flip_work);
+	queue_work(priv->wq, &omap_crtc->flip_work);
 }
 
 static int omap_crtc_page_flip(struct drm_crtc *crtc,
@@ -550,15 +554,16 @@ static int omap_crtc_page_flip(struct drm_crtc *crtc,
 
 	spin_lock_irqsave(&dev->event_lock, flags);
 
-	if (omap_crtc->old_fb) {
+	if (omap_crtc->flip_pending) {
 		spin_unlock_irqrestore(&dev->event_lock, flags);
 		dev_err(dev->dev, "already a pending flip\n");
 		return -EBUSY;
 	}
 
-	omap_crtc->event = event;
-	omap_crtc->old_fb = primary->fb = fb;
-	drm_framebuffer_reference(omap_crtc->old_fb);
+	omap_crtc->flip_event = event;
+	omap_crtc->flip_pending = true;
+	primary->fb = fb;
+	drm_framebuffer_reference(fb);
 
 	spin_unlock_irqrestore(&dev->event_lock, flags);
 
@@ -640,7 +645,7 @@ struct drm_crtc *omap_crtc_init(struct drm_device *dev,
 
 	crtc = &omap_crtc->base;
 
-	INIT_WORK(&omap_crtc->page_flip_work, page_flip_worker);
+	INIT_WORK(&omap_crtc->flip_work, page_flip_worker);
 
 	INIT_LIST_HEAD(&omap_crtc->pending_unpins);
 
-- 
2.0.5

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

[PATCH 05/31] drm: omapdrm: Simplify IRQ registration

[Laurent Pinchart]

The omapdrm can't use drm_irq_install() and drm_irq_uninstall() as it
delegates IRQ handling to the omapdss driver. However, the code still
declares IRQ-related operations used by the DRM IRQ helpers, and calls
them indirectly.

Simplify the implementation by calling the functions directly or
inlining them. The irq_enabled checks can then also be simplified as
the call stacks guarantees that omap_drm_irq_install() and
omap_drm_irq_uninstall() will never run concurrently.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 drivers/gpu/drm/omapdrm/omap_drv.c |   7 +--
 drivers/gpu/drm/omapdrm/omap_drv.h |   6 +--
 drivers/gpu/drm/omapdrm/omap_irq.c | 102 +++++++++----------------------------
 3 files changed, 25 insertions(+), 90 deletions(-)

diff --git a/drivers/gpu/drm/omapdrm/omap_drv.c b/drivers/gpu/drm/omapdrm/omap_drv.c
index bf02121..47fb99b 100644
--- a/drivers/gpu/drm/omapdrm/omap_drv.c
+++ b/drivers/gpu/drm/omapdrm/omap_drv.c
@@ -656,8 +656,7 @@ static const struct file_operations omapdriver_fops = {
 };
 
 static struct drm_driver omap_drm_driver = {
-	.driver_features = DRIVER_HAVE_IRQ | DRIVER_MODESET | DRIVER_GEM
-			 | DRIVER_PRIME,
+	.driver_features = DRIVER_MODESET | DRIVER_GEM  | DRIVER_PRIME,
 	.load = dev_load,
 	.unload = dev_unload,
 	.open = dev_open,
@@ -668,10 +667,6 @@ static struct drm_driver omap_drm_driver = {
 	.get_vblank_counter = drm_vblank_count,
 	.enable_vblank = omap_irq_enable_vblank,
 	.disable_vblank = omap_irq_disable_vblank,
-	.irq_preinstall = omap_irq_preinstall,
-	.irq_postinstall = omap_irq_postinstall,
-	.irq_uninstall = omap_irq_uninstall,
-	.irq_handler = omap_irq_handler,
 #ifdef CONFIG_DEBUG_FS
 	.debugfs_init = omap_debugfs_init,
 	.debugfs_cleanup = omap_debugfs_cleanup,
diff --git a/drivers/gpu/drm/omapdrm/omap_drv.h b/drivers/gpu/drm/omapdrm/omap_drv.h
index 6c0cb46..f1005b4 100644
--- a/drivers/gpu/drm/omapdrm/omap_drv.h
+++ b/drivers/gpu/drm/omapdrm/omap_drv.h
@@ -122,15 +122,11 @@ int omap_gem_resume(struct device *dev);
 
 int omap_irq_enable_vblank(struct drm_device *dev, int crtc_id);
 void omap_irq_disable_vblank(struct drm_device *dev, int crtc_id);
-irqreturn_t omap_irq_handler(int irq, void *arg);
-void omap_irq_preinstall(struct drm_device *dev);
-int omap_irq_postinstall(struct drm_device *dev);
-void omap_irq_uninstall(struct drm_device *dev);
 void __omap_irq_register(struct drm_device *dev, struct omap_drm_irq *irq);
 void __omap_irq_unregister(struct drm_device *dev, struct omap_drm_irq *irq);
 void omap_irq_register(struct drm_device *dev, struct omap_drm_irq *irq);
 void omap_irq_unregister(struct drm_device *dev, struct omap_drm_irq *irq);
-int omap_drm_irq_uninstall(struct drm_device *dev);
+void omap_drm_irq_uninstall(struct drm_device *dev);
 int omap_drm_irq_install(struct drm_device *dev);
 
 struct drm_fb_helper *omap_fbdev_init(struct drm_device *dev);
diff --git a/drivers/gpu/drm/omapdrm/omap_irq.c b/drivers/gpu/drm/omapdrm/omap_irq.c
index 3eb097e..803aff0 100644
--- a/drivers/gpu/drm/omapdrm/omap_irq.c
+++ b/drivers/gpu/drm/omapdrm/omap_irq.c
@@ -187,7 +187,7 @@ void omap_irq_disable_vblank(struct drm_device *dev, int crtc_id)
 	dispc_runtime_put();
 }
 
-irqreturn_t omap_irq_handler(int irq, void *arg)
+static irqreturn_t omap_irq_handler(int irq, void *arg)
 {
 	struct drm_device *dev = (struct drm_device *) arg;
 	struct omap_drm_private *priv = dev->dev_private;
@@ -222,23 +222,29 @@ irqreturn_t omap_irq_handler(int irq, void *arg)
 	return IRQ_HANDLED;
 }
 
-void omap_irq_preinstall(struct drm_device *dev)
-{
-	DBG("dev=%p", dev);
-	dispc_runtime_get();
-	dispc_clear_irqstatus(0xffffffff);
-	dispc_runtime_put();
-}
+/*
+ * We need a special version, instead of just using drm_irq_install(),
+ * because we need to register the irq via omapdss.  Once omapdss and
+ * omapdrm are merged together we can assign the dispc hwmod data to
+ * ourselves and drop these and just use drm_irq_{install,uninstall}()
+ */
 
-int omap_irq_postinstall(struct drm_device *dev)
+int omap_drm_irq_install(struct drm_device *dev)
 {
 	struct omap_drm_private *priv = dev->dev_private;
 	struct omap_drm_irq *error_handler = &priv->error_handler;
-
-	DBG("dev=%p", dev);
+	int ret;
 
 	INIT_LIST_HEAD(&priv->irq_list);
 
+	dispc_runtime_get();
+	dispc_clear_irqstatus(0xffffffff);
+	dispc_runtime_put();
+
+	ret = dispc_request_irq(omap_irq_handler, dev);
+	if (ret < 0)
+		return ret;
+
 	error_handler->irq = omap_irq_error_handler;
 	error_handler->irqmask = DISPC_IRQ_OCP_ERR;
 
@@ -249,76 +255,22 @@ int omap_irq_postinstall(struct drm_device *dev)
 
 	omap_irq_register(dev, error_handler);
 
-	return 0;
-}
-
-void omap_irq_uninstall(struct drm_device *dev)
-{
-	DBG("dev=%p", dev);
-	// TODO prolly need to call drm_irq_uninstall() somewhere too
-}
-
-/*
- * We need a special version, instead of just using drm_irq_install(),
- * because we need to register the irq via omapdss.  Once omapdss and
- * omapdrm are merged together we can assign the dispc hwmod data to
- * ourselves and drop these and just use drm_irq_{install,uninstall}()
- */
-
-int omap_drm_irq_install(struct drm_device *dev)
-{
-	int ret;
-
-	mutex_lock(&dev->struct_mutex);
-
-	if (dev->irq_enabled) {
-		mutex_unlock(&dev->struct_mutex);
-		return -EBUSY;
-	}
 	dev->irq_enabled = true;
-	mutex_unlock(&dev->struct_mutex);
-
-	/* Before installing handler */
-	if (dev->driver->irq_preinstall)
-		dev->driver->irq_preinstall(dev);
-
-	ret = dispc_request_irq(dev->driver->irq_handler, dev);
 
-	if (ret < 0) {
-		mutex_lock(&dev->struct_mutex);
-		dev->irq_enabled = false;
-		mutex_unlock(&dev->struct_mutex);
-		return ret;
-	}
-
-	/* After installing handler */
-	if (dev->driver->irq_postinstall)
-		ret = dev->driver->irq_postinstall(dev);
-
-	if (ret < 0) {
-		mutex_lock(&dev->struct_mutex);
-		dev->irq_enabled = false;
-		mutex_unlock(&dev->struct_mutex);
-		dispc_free_irq(dev);
-	}
-
-	return ret;
+	return 0;
 }
 
-int omap_drm_irq_uninstall(struct drm_device *dev)
+void omap_drm_irq_uninstall(struct drm_device *dev)
 {
 	unsigned long irqflags;
-	bool irq_enabled;
 	int i;
 
-	mutex_lock(&dev->struct_mutex);
-	irq_enabled = dev->irq_enabled;
+	if (!dev->irq_enabled)
+		return;
+
 	dev->irq_enabled = false;
-	mutex_unlock(&dev->struct_mutex);
 
-	/*
-	 * Wake up any waiters so they don't hang.
-	 */
+	/* Wake up any waiters so they don't hang. */
 	if (dev->num_crtcs) {
 		spin_lock_irqsave(&dev->vbl_lock, irqflags);
 		for (i = 0; i < dev->num_crtcs; i++) {
@@ -330,13 +282,5 @@ int omap_drm_irq_uninstall(struct drm_device *dev)
 		spin_unlock_irqrestore(&dev->vbl_lock, irqflags);
 	}
 
-	if (!irq_enabled)
-		return -EINVAL;
-
-	if (dev->driver->irq_uninstall)
-		dev->driver->irq_uninstall(dev);
-
 	dispc_free_irq(dev);
-
-	return 0;
 }
-- 
2.0.5

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

[PATCH 06/31] drm: omapdrm: Cancel pending page flips when closing device

[Laurent Pinchart]

Pending page flips must be cancelled when closing the device, otherwise
their completion at next vblank will result in nasty effects, including
possible oopses due to resources required to complete the page flip
being freed.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 drivers/gpu/drm/omapdrm/omap_crtc.c | 21 ++++++++++++++++++++-
 drivers/gpu/drm/omapdrm/omap_drv.c  |  6 ++++++
 drivers/gpu/drm/omapdrm/omap_drv.h  |  1 +
 3 files changed, 27 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/omapdrm/omap_crtc.c b/drivers/gpu/drm/omapdrm/omap_crtc.c
index 85129d5..a60f4e4 100644
--- a/drivers/gpu/drm/omapdrm/omap_crtc.c
+++ b/drivers/gpu/drm/omapdrm/omap_crtc.c
@@ -263,9 +263,28 @@ static const struct dss_mgr_ops mgr_ops = {
 };
 
 /* -----------------------------------------------------------------------------
- * Setup and Flush
+ * Setup, Flush and Page Flip
  */
 
+void omap_crtc_cancel_page_flip(struct drm_crtc *crtc, struct drm_file *file)
+{
+	struct omap_crtc *omap_crtc = to_omap_crtc(crtc);
+	struct drm_device *dev = crtc->dev;
+	unsigned long flags;
+
+	spin_lock_irqsave(&dev->event_lock, flags);
+
+	/* Only complete events queued for our file handle. */
+	if (omap_crtc->flip_event &&
+	    file == omap_crtc->flip_event->base.file_priv) {
+		drm_send_vblank_event(dev, omap_crtc->pipe,
+				      omap_crtc->flip_event);
+		omap_crtc->flip_event = NULL;
+	}
+
+	spin_unlock_irqrestore(&dev->event_lock, flags);
+}
+
 static void omap_crtc_error_irq(struct omap_drm_irq *irq, uint32_t irqstatus)
 {
 	struct omap_crtc *omap_crtc =
diff --git a/drivers/gpu/drm/omapdrm/omap_drv.c b/drivers/gpu/drm/omapdrm/omap_drv.c
index 47fb99b..cf1b37e 100644
--- a/drivers/gpu/drm/omapdrm/omap_drv.c
+++ b/drivers/gpu/drm/omapdrm/omap_drv.c
@@ -630,7 +630,13 @@ static void dev_lastclose(struct drm_device *dev)
 
 static void dev_preclose(struct drm_device *dev, struct drm_file *file)
 {
+	struct omap_drm_private *priv = dev->dev_private;
+	unsigned int i;
+
 	DBG("preclose: dev=%p", dev);
+
+	for (i = 0; i < priv->num_crtcs; ++i)
+		omap_crtc_cancel_page_flip(priv->crtcs[i], file);
 }
 
 static void dev_postclose(struct drm_device *dev, struct drm_file *file)
diff --git a/drivers/gpu/drm/omapdrm/omap_drv.h b/drivers/gpu/drm/omapdrm/omap_drv.h
index f1005b4..774b9f6 100644
--- a/drivers/gpu/drm/omapdrm/omap_drv.h
+++ b/drivers/gpu/drm/omapdrm/omap_drv.h
@@ -136,6 +136,7 @@ const struct omap_video_timings *omap_crtc_timings(struct drm_crtc *crtc);
 enum omap_channel omap_crtc_channel(struct drm_crtc *crtc);
 int omap_crtc_flush(struct drm_crtc *crtc);
 int omap_crtc_queue_unpin(struct drm_crtc *crtc, struct drm_framebuffer *fb);
+void omap_crtc_cancel_page_flip(struct drm_crtc *crtc, struct drm_file *file);
 void omap_crtc_pre_init(void);
 void omap_crtc_pre_uninit(void);
 struct drm_crtc *omap_crtc_init(struct drm_device *dev,
-- 
2.0.5

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

[PATCH 07/31] drm: omapdrm: Rework page flip handling

[Laurent Pinchart]

To implement proper vblank control the driver will need to wait for page
flip completion before disabling the vblank interrupt. This is made
complex by the page flip implementation which queues and submits page
flips to the hardware in two separate steps between which DRM locks are
released. We thus need to avoid waiting on a page flip that has been
queued but not submitted as submission and wait are covered by the same
lock.

Rework page flip handling as a first step by splitting the flip_pending
boolean variable into an enumerated state and moving between states
based on flip queue, submission and completion. The CANCELLED state will
be used in a second step.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 drivers/gpu/drm/omapdrm/omap_crtc.c | 84 ++++++++++++++++++++++++++++---------
 1 file changed, 64 insertions(+), 20 deletions(-)

diff --git a/drivers/gpu/drm/omapdrm/omap_crtc.c b/drivers/gpu/drm/omapdrm/omap_crtc.c
index a60f4e4..c086f72 100644
--- a/drivers/gpu/drm/omapdrm/omap_crtc.c
+++ b/drivers/gpu/drm/omapdrm/omap_crtc.c
@@ -28,6 +28,13 @@
 
 #define to_omap_crtc(x) container_of(x, struct omap_crtc, base)
 
+enum omap_page_flip_state {
+	OMAP_PAGE_FLIP_IDLE,
+	OMAP_PAGE_FLIP_WAIT,
+	OMAP_PAGE_FLIP_QUEUED,
+	OMAP_PAGE_FLIP_CANCELLED,
+};
+
 struct omap_crtc {
 	struct drm_crtc base;
 
@@ -55,16 +62,19 @@ struct omap_crtc {
 	struct list_head pending_unpins;
 
 	/*
-	 * The flip_pending flag indicates if a page flip has been queued and
-	 * hasn't completed yet. The flip event, if any, is stored in
-	 * flip_event.
+	 * flip_state flag indicates the current page flap state: IDLE if no
+	 * page queue has been submitted, WAIT when waiting for GEM async
+	 * completion, QUEUED when the page flip has been queued to the hardware
+	 * or CANCELLED when the CRTC is turned off before the flip gets queued
+	 * to the hardware. The flip event, if any, is stored in flip_event. The
+	 * flip_wait wait queue is used to wait for page flip completion.
 	 *
 	 * The flip_work work queue handles page flip requests without caring
 	 * about what context the GEM async callback is called from. Possibly we
 	 * should just make omap_gem always call the cb from the worker so we
 	 * don't have to care about this.
 	 */
-	bool flip_pending;
+	enum omap_page_flip_state flip_state;
 	struct drm_pending_vblank_event *flip_event;
 	struct work_struct flip_work;
 
@@ -285,6 +295,22 @@ void omap_crtc_cancel_page_flip(struct drm_crtc *crtc, struct drm_file *file)
 	spin_unlock_irqrestore(&dev->event_lock, flags);
 }
 
+/* Must be called with dev->event_lock locked. */
+static void omap_crtc_complete_page_flip(struct drm_crtc *crtc,
+					 enum omap_page_flip_state state)
+{
+	struct omap_crtc *omap_crtc = to_omap_crtc(crtc);
+	struct drm_device *dev = crtc->dev;
+
+	if (omap_crtc->flip_event) {
+		drm_send_vblank_event(dev, omap_crtc->pipe,
+				      omap_crtc->flip_event);
+		omap_crtc->flip_event = NULL;
+	}
+
+	omap_crtc->flip_state = state;
+}
+
 static void omap_crtc_error_irq(struct omap_drm_irq *irq, uint32_t irqstatus)
 {
 	struct omap_crtc *omap_crtc =
@@ -312,16 +338,9 @@ static void omap_crtc_vblank_irq(struct omap_drm_irq *irq, uint32_t irqstatus)
 	DBG("%s: apply done", omap_crtc->name);
 	__omap_irq_unregister(dev, &omap_crtc->vblank_irq);
 
-	spin_lock_irqsave(&dev->event_lock, flags);
-
 	/* wakeup userspace */
-	if (omap_crtc->flip_event)
-		drm_send_vblank_event(dev, omap_crtc->pipe,
-				      omap_crtc->flip_event);
-
-	omap_crtc->flip_event = NULL;
-	omap_crtc->flip_pending = false;
-
+	spin_lock_irqsave(&dev->event_lock, flags);
+	omap_crtc_complete_page_flip(&omap_crtc->base, OMAP_PAGE_FLIP_IDLE);
 	spin_unlock_irqrestore(&dev->event_lock, flags);
 
 	complete(&omap_crtc->completion);
@@ -533,16 +552,41 @@ static void page_flip_worker(struct work_struct *work)
 			container_of(work, struct omap_crtc, flip_work);
 	struct drm_crtc *crtc = &omap_crtc->base;
 	struct drm_display_mode *mode = &crtc->mode;
+	struct drm_device *dev = crtc->dev;
+	struct drm_framebuffer *fb;
 	struct drm_gem_object *bo;
+	unsigned long flags;
+	bool queue_flip;
 
 	drm_modeset_lock(&crtc->mutex, NULL);
-	omap_plane_mode_set(crtc->primary, crtc, crtc->primary->fb,
-			    0, 0, mode->hdisplay, mode->vdisplay,
-			    crtc->x, crtc->y, mode->hdisplay, mode->vdisplay);
-	omap_crtc_flush(crtc);
+
+	spin_lock_irqsave(&dev->event_lock, flags);
+	/*
+	 * The page flip could have been cancelled while waiting for the GEM
+	 * async operation to complete. Don't queue the flip in that case.
+	 */
+	if (omap_crtc->flip_state == OMAP_PAGE_FLIP_WAIT) {
+		omap_crtc->flip_state = OMAP_PAGE_FLIP_QUEUED;
+		queue_flip = true;
+	} else {
+		omap_crtc->flip_state = OMAP_PAGE_FLIP_IDLE;
+		queue_flip = false;
+	}
+	spin_unlock_irqrestore(&dev->event_lock, flags);
+
+	fb = crtc->primary->fb;
+
+	if (queue_flip) {
+		omap_plane_mode_set(crtc->primary, crtc, fb,
+				    0, 0, mode->hdisplay, mode->vdisplay,
+				    crtc->x, crtc->y, mode->hdisplay,
+				    mode->vdisplay);
+		omap_crtc_flush(crtc);
+	}
+
 	drm_modeset_unlock(&crtc->mutex);
 
-	bo = omap_framebuffer_bo(crtc->primary->fb, 0);
+	bo = omap_framebuffer_bo(fb, 0);
 	drm_gem_object_unreference_unlocked(bo);
 	drm_framebuffer_unreference(crtc->primary->fb);
 }
@@ -573,14 +617,14 @@ static int omap_crtc_page_flip(struct drm_crtc *crtc,
 
 	spin_lock_irqsave(&dev->event_lock, flags);
 
-	if (omap_crtc->flip_pending) {
+	if (omap_crtc->flip_state != OMAP_PAGE_FLIP_IDLE) {
 		spin_unlock_irqrestore(&dev->event_lock, flags);
 		dev_err(dev->dev, "already a pending flip\n");
 		return -EBUSY;
 	}
 
 	omap_crtc->flip_event = event;
-	omap_crtc->flip_pending = true;
+	omap_crtc->flip_state = OMAP_PAGE_FLIP_WAIT;
 	primary->fb = fb;
 	drm_framebuffer_reference(fb);
 
-- 
2.0.5

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

[PATCH 08/31] drm: omapdrm: Turn vblank on/off when enabling/disabling CRTC

[Laurent Pinchart]

The DRM core vblank handling mechanism requires drivers to forcefully
turn vblank reporting off when disabling the CRTC, and to restore the
vblank reporting status when enabling the CRTC.

Implement this using the drm_crtc_vblank_on/off helpers. When disabling
vblank we must first wait for page flips to complete, so implement page
flip completion wait as well.

Finally, drm_crtc_vblank_off() must be called at startup to synchronize
the state of the vblank core code with the hardware, which is initially
disabled. An interesting side effect is that the .disable_vblank()
operation will now be called for the first time with the CRTC disabled
and the DISPC runtime suspended. The dispc_runtime_get() call in
.disable_vblank() is supposed to take care of that, but the operation is
called with a spinlock held, which prevents it from sleeping.

To fix that move DISPC runtime PM handling out of the vblank operations
to the CRTC code, ensuring that the display controller will always be
powered when enabling or disabling vblank interrupts.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 drivers/gpu/drm/omapdrm/omap_crtc.c | 78 +++++++++++++++++++++++++++++++++++--
 drivers/gpu/drm/omapdrm/omap_drv.c  |  5 +++
 drivers/gpu/drm/omapdrm/omap_irq.c  |  4 --
 3 files changed, 79 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/omapdrm/omap_crtc.c b/drivers/gpu/drm/omapdrm/omap_crtc.c
index c086f72..1076bc0 100644
--- a/drivers/gpu/drm/omapdrm/omap_crtc.c
+++ b/drivers/gpu/drm/omapdrm/omap_crtc.c
@@ -76,6 +76,7 @@ struct omap_crtc {
 	 */
 	enum omap_page_flip_state flip_state;
 	struct drm_pending_vblank_event *flip_event;
+	wait_queue_head_t flip_wait;
 	struct work_struct flip_work;
 
 	struct completion completion;
@@ -309,6 +310,61 @@ static void omap_crtc_complete_page_flip(struct drm_crtc *crtc,
 	}
 
 	omap_crtc->flip_state = state;
+
+	if (state == OMAP_PAGE_FLIP_IDLE)
+		wake_up(&omap_crtc->flip_wait);
+}
+
+static bool omap_crtc_page_flip_pending(struct drm_crtc *crtc)
+{
+	struct omap_crtc *omap_crtc = to_omap_crtc(crtc);
+	struct drm_device *dev = crtc->dev;
+	unsigned long flags;
+	bool pending;
+
+	spin_lock_irqsave(&dev->event_lock, flags);
+	pending = omap_crtc->flip_state != OMAP_PAGE_FLIP_IDLE;
+	spin_unlock_irqrestore(&dev->event_lock, flags);
+
+	return pending;
+}
+
+static void omap_crtc_wait_page_flip(struct drm_crtc *crtc)
+{
+	struct omap_crtc *omap_crtc = to_omap_crtc(crtc);
+	struct drm_device *dev = crtc->dev;
+	bool cancelled = false;
+	unsigned long flags;
+
+	/*
+	 * If we're still waiting for the GEM async operation to complete just
+	 * cancel the page flip, as we're holding the CRTC mutex preventing the
+	 * page flip work handler from queueing the page flip.
+	 *
+	 * We can't release the reference to the frame buffer here as the async
+	 * operation doesn't keep its own reference to the buffer. We'll just
+	 * let the page flip work queue handle that.
+	 */
+	spin_lock_irqsave(&dev->event_lock, flags);
+	if (omap_crtc->flip_state == OMAP_PAGE_FLIP_WAIT) {
+		omap_crtc_complete_page_flip(crtc, OMAP_PAGE_FLIP_CANCELLED);
+		cancelled = true;
+	}
+	spin_unlock_irqrestore(&dev->event_lock, flags);
+
+	if (cancelled)
+		return;
+
+	if (wait_event_timeout(omap_crtc->flip_wait,
+			       !omap_crtc_page_flip_pending(crtc),
+			       msecs_to_jiffies(50)))
+		return;
+
+	dev_warn(crtc->dev->dev, "page flip timeout!\n");
+
+	spin_lock_irqsave(&dev->event_lock, flags);
+	omap_crtc_complete_page_flip(crtc, OMAP_PAGE_FLIP_IDLE);
+	spin_unlock_irqrestore(&dev->event_lock, flags);
 }
 
 static void omap_crtc_error_irq(struct omap_drm_irq *irq, uint32_t irqstatus)
@@ -455,26 +511,39 @@ static void omap_crtc_dpms(struct drm_crtc *crtc, int mode)
 {
 	struct omap_drm_private *priv = crtc->dev->dev_private;
 	struct omap_crtc *omap_crtc = to_omap_crtc(crtc);
-	bool enabled = (mode == DRM_MODE_DPMS_ON);
+	bool enable = (mode == DRM_MODE_DPMS_ON);
 	int i;
 
 	DBG("%s: %d", omap_crtc->name, mode);
 
-	if (enabled == omap_crtc->enabled)
+	if (enable == omap_crtc->enabled)
 		return;
 
+	if (!enable) {
+		omap_crtc_wait_page_flip(crtc);
+		dispc_runtime_get();
+		drm_crtc_vblank_off(crtc);
+		dispc_runtime_put();
+	}
+
 	/* Enable/disable all planes associated with the CRTC. */
 	for (i = 0; i < priv->num_planes; i++) {
 		struct drm_plane *plane = priv->planes[i];
 
 		if (plane->crtc == crtc)
-			WARN_ON(omap_plane_set_enable(plane, enabled));
+			WARN_ON(omap_plane_set_enable(plane, enable));
 	}
 
-	omap_crtc->enabled = enabled;
+	omap_crtc->enabled = enable;
 
 	omap_crtc_setup(crtc);
 	omap_crtc_flush(crtc);
+
+	if (enable) {
+		dispc_runtime_get();
+		drm_crtc_vblank_on(crtc);
+		dispc_runtime_put();
+	}
 }
 
 static bool omap_crtc_mode_fixup(struct drm_crtc *crtc,
@@ -709,6 +778,7 @@ struct drm_crtc *omap_crtc_init(struct drm_device *dev,
 	crtc = &omap_crtc->base;
 
 	INIT_WORK(&omap_crtc->flip_work, page_flip_worker);
+	init_waitqueue_head(&omap_crtc->flip_wait);
 
 	INIT_LIST_HEAD(&omap_crtc->pending_unpins);
 
diff --git a/drivers/gpu/drm/omapdrm/omap_drv.c b/drivers/gpu/drm/omapdrm/omap_drv.c
index cf1b37e..e81fbc0 100644
--- a/drivers/gpu/drm/omapdrm/omap_drv.c
+++ b/drivers/gpu/drm/omapdrm/omap_drv.c
@@ -502,6 +502,7 @@ static int dev_load(struct drm_device *dev, unsigned long flags)
 {
 	struct omap_drm_platform_data *pdata = dev->dev->platform_data;
 	struct omap_drm_private *priv;
+	unsigned int i;
 	int ret;
 
 	DBG("load: dev=%p", dev);
@@ -529,10 +530,14 @@ static int dev_load(struct drm_device *dev, unsigned long flags)
 		return ret;
 	}
 
+	/* Initialize vblank handling, start with all CRTCs disabled. */
 	ret = drm_vblank_init(dev, priv->num_crtcs);
 	if (ret)
 		dev_warn(dev->dev, "could not init vblank\n");
 
+	for (i = 0; i < priv->num_crtcs; i++)
+		drm_crtc_vblank_off(priv->crtcs[i]);
+
 	priv->fbdev = omap_fbdev_init(dev);
 	if (!priv->fbdev) {
 		dev_warn(dev->dev, "omap_fbdev_init failed\n");
diff --git a/drivers/gpu/drm/omapdrm/omap_irq.c b/drivers/gpu/drm/omapdrm/omap_irq.c
index 803aff0..249c0330 100644
--- a/drivers/gpu/drm/omapdrm/omap_irq.c
+++ b/drivers/gpu/drm/omapdrm/omap_irq.c
@@ -152,12 +152,10 @@ int omap_irq_enable_vblank(struct drm_device *dev, int crtc_id)
 
 	DBG("dev=%p, crtc=%d", dev, crtc_id);
 
-	dispc_runtime_get();
 	spin_lock_irqsave(&list_lock, flags);
 	priv->vblank_mask |= pipe2vbl(crtc);
 	omap_irq_update(dev);
 	spin_unlock_irqrestore(&list_lock, flags);
-	dispc_runtime_put();
 
 	return 0;
 }
@@ -179,12 +177,10 @@ void omap_irq_disable_vblank(struct drm_device *dev, int crtc_id)
 
 	DBG("dev=%p, crtc=%d", dev, crtc_id);
 
-	dispc_runtime_get();
 	spin_lock_irqsave(&list_lock, flags);
 	priv->vblank_mask &= ~pipe2vbl(crtc);
 	omap_irq_update(dev);
 	spin_unlock_irqrestore(&list_lock, flags);
-	dispc_runtime_put();
 }
 
 static irqreturn_t omap_irq_handler(int irq, void *arg)
-- 
2.0.5

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

[PATCH 09/31] drm: omapdrm: Fix page flip race with CRTC disable

[Laurent Pinchart]

We can't rely on crtc->primary->fb in the page flip worker, as a racing
CRTC disable (due for instance to an explicit framebuffer deletion from
userspace) would set that field to NULL before the worker gets a change
to run. Store the framebuffer queued for page flip in a new field of
omap_crtc instead, and hold a reference to it.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 drivers/gpu/drm/omapdrm/omap_crtc.c | 24 +++++++++++++++++++-----
 1 file changed, 19 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/omapdrm/omap_crtc.c b/drivers/gpu/drm/omapdrm/omap_crtc.c
index 1076bc0..68abc4d 100644
--- a/drivers/gpu/drm/omapdrm/omap_crtc.c
+++ b/drivers/gpu/drm/omapdrm/omap_crtc.c
@@ -66,7 +66,8 @@ struct omap_crtc {
 	 * page queue has been submitted, WAIT when waiting for GEM async
 	 * completion, QUEUED when the page flip has been queued to the hardware
 	 * or CANCELLED when the CRTC is turned off before the flip gets queued
-	 * to the hardware. The flip event, if any, is stored in flip_event. The
+	 * to the hardware. The flip event, if any, is stored in flip_event, and
+	 * the framebuffer queued for page flip is stored in flip_fb. The
 	 * flip_wait wait queue is used to wait for page flip completion.
 	 *
 	 * The flip_work work queue handles page flip requests without caring
@@ -76,6 +77,7 @@ struct omap_crtc {
 	 */
 	enum omap_page_flip_state flip_state;
 	struct drm_pending_vblank_event *flip_event;
+	struct drm_framebuffer *flip_fb;
 	wait_queue_head_t flip_wait;
 	struct work_struct flip_work;
 
@@ -630,6 +632,7 @@ static void page_flip_worker(struct work_struct *work)
 	drm_modeset_lock(&crtc->mutex, NULL);
 
 	spin_lock_irqsave(&dev->event_lock, flags);
+
 	/*
 	 * The page flip could have been cancelled while waiting for the GEM
 	 * async operation to complete. Don't queue the flip in that case.
@@ -641,9 +644,11 @@ static void page_flip_worker(struct work_struct *work)
 		omap_crtc->flip_state = OMAP_PAGE_FLIP_IDLE;
 		queue_flip = false;
 	}
-	spin_unlock_irqrestore(&dev->event_lock, flags);
 
-	fb = crtc->primary->fb;
+	fb = omap_crtc->flip_fb;
+	omap_crtc->flip_fb = NULL;
+
+	spin_unlock_irqrestore(&dev->event_lock, flags);
 
 	if (queue_flip) {
 		omap_plane_mode_set(crtc->primary, crtc, fb,
@@ -657,7 +662,7 @@ static void page_flip_worker(struct work_struct *work)
 
 	bo = omap_framebuffer_bo(fb, 0);
 	drm_gem_object_unreference_unlocked(bo);
-	drm_framebuffer_unreference(crtc->primary->fb);
+	drm_framebuffer_unreference(fb);
 }
 
 static void page_flip_cb(void *arg)
@@ -692,10 +697,19 @@ static int omap_crtc_page_flip(struct drm_crtc *crtc,
 		return -EBUSY;
 	}
 
+	/*
+	 * Store a reference to the framebuffer queued for page flip in the CRTC
+	 * private structure. We can't rely on crtc->primary->fb in the page
+	 * flip worker, as a racing CRTC disable (due for instance to an
+	 * explicit framebuffer deletion from userspace) would set that field to
+	 * NULL before the worker gets a change to run.
+	 */
+	drm_framebuffer_reference(fb);
+	omap_crtc->flip_fb = fb;
 	omap_crtc->flip_event = event;
 	omap_crtc->flip_state = OMAP_PAGE_FLIP_WAIT;
+
 	primary->fb = fb;
-	drm_framebuffer_reference(fb);
 
 	spin_unlock_irqrestore(&dev->event_lock, flags);
 
-- 
2.0.5

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

[PATCH 10/31] drm: omapdrm: Clean up #include's

[Laurent Pinchart]

Use the <...> include style instead of "..." for DRM headers and sort
the headers alphabetically to ease detection of duplicates.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 drivers/gpu/drm/omapdrm/omap_connector.c  |  6 +++---
 drivers/gpu/drm/omapdrm/omap_crtc.c       |  8 ++++----
 drivers/gpu/drm/omapdrm/omap_debugfs.c    |  6 +++---
 drivers/gpu/drm/omapdrm/omap_dmm_tiler.c  | 19 ++++++++++---------
 drivers/gpu/drm/omapdrm/omap_drv.c        |  6 +++---
 drivers/gpu/drm/omapdrm/omap_drv.h        |  8 ++++----
 drivers/gpu/drm/omapdrm/omap_encoder.c    | 10 ++++------
 drivers/gpu/drm/omapdrm/omap_fb.c         |  8 ++++----
 drivers/gpu/drm/omapdrm/omap_fbdev.c      |  6 +++---
 drivers/gpu/drm/omapdrm/omap_gem.c        |  4 ++--
 drivers/gpu/drm/omapdrm/omap_gem_dmabuf.c |  4 ++--
 drivers/gpu/drm/omapdrm/omap_plane.c      |  2 +-
 12 files changed, 43 insertions(+), 44 deletions(-)

diff --git a/drivers/gpu/drm/omapdrm/omap_connector.c b/drivers/gpu/drm/omapdrm/omap_connector.c
index b41965c..71081b6 100644
--- a/drivers/gpu/drm/omapdrm/omap_connector.c
+++ b/drivers/gpu/drm/omapdrm/omap_connector.c
@@ -17,10 +17,10 @@
  * this program.  If not, see <http://www.gnu.org/licenses/>.
  */
 
-#include "omap_drv.h"
+#include <drm/drm_crtc.h>
+#include <drm/drm_crtc_helper.h>
 
-#include "drm_crtc.h"
-#include "drm_crtc_helper.h"
+#include "omap_drv.h"
 
 /*
  * connector funcs
diff --git a/drivers/gpu/drm/omapdrm/omap_crtc.c b/drivers/gpu/drm/omapdrm/omap_crtc.c
index 68abc4d..6840ed5 100644
--- a/drivers/gpu/drm/omapdrm/omap_crtc.c
+++ b/drivers/gpu/drm/omapdrm/omap_crtc.c
@@ -19,12 +19,12 @@
 
 #include <linux/completion.h>
 
-#include "omap_drv.h"
-
+#include <drm/drm_crtc.h>
+#include <drm/drm_crtc_helper.h>
 #include <drm/drm_mode.h>
 #include <drm/drm_plane_helper.h>
-#include "drm_crtc.h"
-#include "drm_crtc_helper.h"
+
+#include "omap_drv.h"
 
 #define to_omap_crtc(x) container_of(x, struct omap_crtc, base)
 
diff --git a/drivers/gpu/drm/omapdrm/omap_debugfs.c b/drivers/gpu/drm/omapdrm/omap_debugfs.c
index d4c04d6..ee91a25 100644
--- a/drivers/gpu/drm/omapdrm/omap_debugfs.c
+++ b/drivers/gpu/drm/omapdrm/omap_debugfs.c
@@ -17,12 +17,12 @@
  * this program.  If not, see <http://www.gnu.org/licenses/>.
  */
 
+#include <drm/drm_crtc.h>
+#include <drm/drm_fb_helper.h>
+
 #include "omap_drv.h"
 #include "omap_dmm_tiler.h"
 
-#include "drm_fb_helper.h"
-
-
 #ifdef CONFIG_DEBUG_FS
 
 static int gem_show(struct seq_file *m, void *arg)
diff --git a/drivers/gpu/drm/omapdrm/omap_dmm_tiler.c b/drivers/gpu/drm/omapdrm/omap_dmm_tiler.c
index 042038e..f2daad8 100644
--- a/drivers/gpu/drm/omapdrm/omap_dmm_tiler.c
+++ b/drivers/gpu/drm/omapdrm/omap_dmm_tiler.c
@@ -15,21 +15,22 @@
  * of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
  * GNU General Public License for more details.
  */
+
+#include <linux/completion.h>
+#include <linux/delay.h>
+#include <linux/dma-mapping.h>
+#include <linux/errno.h>
 #include <linux/init.h>
+#include <linux/interrupt.h>
+#include <linux/list.h>
+#include <linux/mm.h>
 #include <linux/module.h>
 #include <linux/platform_device.h> /* platform_device() */
-#include <linux/errno.h>
 #include <linux/sched.h>
-#include <linux/wait.h>
-#include <linux/interrupt.h>
-#include <linux/dma-mapping.h>
 #include <linux/slab.h>
-#include <linux/vmalloc.h>
-#include <linux/delay.h>
-#include <linux/mm.h>
 #include <linux/time.h>
-#include <linux/list.h>
-#include <linux/completion.h>
+#include <linux/vmalloc.h>
+#include <linux/wait.h>
 
 #include "omap_dmm_tiler.h"
 #include "omap_dmm_priv.h"
diff --git a/drivers/gpu/drm/omapdrm/omap_drv.c b/drivers/gpu/drm/omapdrm/omap_drv.c
index e81fbc0..dbd3046 100644
--- a/drivers/gpu/drm/omapdrm/omap_drv.c
+++ b/drivers/gpu/drm/omapdrm/omap_drv.c
@@ -17,11 +17,11 @@
  * this program.  If not, see <http://www.gnu.org/licenses/>.
  */
 
-#include "omap_drv.h"
+#include <drm/drm_crtc_helper.h>
+#include <drm/drm_fb_helper.h>
 
-#include "drm_crtc_helper.h"
-#include "drm_fb_helper.h"
 #include "omap_dmm_tiler.h"
+#include "omap_drv.h"
 
 #define DRIVER_NAME		MODULE_NAME
 #define DRIVER_DESC		"OMAP DRM"
diff --git a/drivers/gpu/drm/omapdrm/omap_drv.h b/drivers/gpu/drm/omapdrm/omap_drv.h
index 774b9f6..6448fa8 100644
--- a/drivers/gpu/drm/omapdrm/omap_drv.h
+++ b/drivers/gpu/drm/omapdrm/omap_drv.h
@@ -20,15 +20,15 @@
 #ifndef __OMAP_DRV_H__
 #define __OMAP_DRV_H__
 
-#include <video/omapdss.h>
 #include <linux/module.h>
+#include <linux/platform_data/omap_drm.h>
 #include <linux/types.h>
+#include <video/omapdss.h>
+
 #include <drm/drmP.h>
 #include <drm/drm_crtc_helper.h>
-#include <drm/omap_drm.h>
 #include <drm/drm_gem.h>
-#include <linux/platform_data/omap_drm.h>
-
+#include <drm/omap_drm.h>
 
 #define DBG(fmt, ...) DRM_DEBUG(fmt"\n", ##__VA_ARGS__)
 #define VERB(fmt, ...) if (0) DRM_DEBUG(fmt, ##__VA_ARGS__) /* verbose debug */
diff --git a/drivers/gpu/drm/omapdrm/omap_encoder.c b/drivers/gpu/drm/omapdrm/omap_encoder.c
index 7445fb1..ab6ed49 100644
--- a/drivers/gpu/drm/omapdrm/omap_encoder.c
+++ b/drivers/gpu/drm/omapdrm/omap_encoder.c
@@ -17,16 +17,14 @@
  * this program.  If not, see <http://www.gnu.org/licenses/>.
  */
 
+#include <linux/list.h>
+
+#include <drm/drm_crtc.h>
+#include <drm/drm_crtc_helper.h>
 #include <drm/drm_edid.h>
 
 #include "omap_drv.h"
 
-#include "drm_crtc.h"
-#include "drm_crtc_helper.h"
-
-#include <linux/list.h>
-
-
 /*
  * encoder funcs
  */
diff --git a/drivers/gpu/drm/omapdrm/omap_fb.c b/drivers/gpu/drm/omapdrm/omap_fb.c
index b2c1a29cc..e505140 100644
--- a/drivers/gpu/drm/omapdrm/omap_fb.c
+++ b/drivers/gpu/drm/omapdrm/omap_fb.c
@@ -17,11 +17,11 @@
  * this program.  If not, see <http://www.gnu.org/licenses/>.
  */
 
-#include "omap_drv.h"
-#include "omap_dmm_tiler.h"
+#include <drm/drm_crtc.h>
+#include <drm/drm_crtc_helper.h>
 
-#include "drm_crtc.h"
-#include "drm_crtc_helper.h"
+#include "omap_dmm_tiler.h"
+#include "omap_drv.h"
 
 /*
  * framebuffer funcs
diff --git a/drivers/gpu/drm/omapdrm/omap_fbdev.c b/drivers/gpu/drm/omapdrm/omap_fbdev.c
index 950cd33..23b5a84 100644
--- a/drivers/gpu/drm/omapdrm/omap_fbdev.c
+++ b/drivers/gpu/drm/omapdrm/omap_fbdev.c
@@ -17,10 +17,10 @@
  * this program.  If not, see <http://www.gnu.org/licenses/>.
  */
 
-#include "omap_drv.h"
+#include <drm/drm_crtc.h>
+#include <drm/drm_fb_helper.h>
 
-#include "drm_crtc.h"
-#include "drm_fb_helper.h"
+#include "omap_drv.h"
 
 MODULE_PARM_DESC(ywrap, "Enable ywrap scrolling (omap44xx and later, default 'y')");
 static bool ywrap_enabled = true;
diff --git a/drivers/gpu/drm/omapdrm/omap_gem.c b/drivers/gpu/drm/omapdrm/omap_gem.c
index e9718b9..2ab7780 100644
--- a/drivers/gpu/drm/omapdrm/omap_gem.c
+++ b/drivers/gpu/drm/omapdrm/omap_gem.c
@@ -17,9 +17,9 @@
  * this program.  If not, see <http://www.gnu.org/licenses/>.
  */
 
-
-#include <linux/spinlock.h>
 #include <linux/shmem_fs.h>
+#include <linux/spinlock.h>
+
 #include <drm/drm_vma_manager.h>
 
 #include "omap_drv.h"
diff --git a/drivers/gpu/drm/omapdrm/omap_gem_dmabuf.c b/drivers/gpu/drm/omapdrm/omap_gem_dmabuf.c
index b46dabd..0039dd7 100644
--- a/drivers/gpu/drm/omapdrm/omap_gem_dmabuf.c
+++ b/drivers/gpu/drm/omapdrm/omap_gem_dmabuf.c
@@ -17,10 +17,10 @@
  * this program.  If not, see <http://www.gnu.org/licenses/>.
  */
 
-#include "omap_drv.h"
-
 #include <linux/dma-buf.h>
 
+#include "omap_drv.h"
+
 static struct sg_table *omap_gem_map_dma_buf(
 		struct dma_buf_attachment *attachment,
 		enum dma_data_direction dir)
diff --git a/drivers/gpu/drm/omapdrm/omap_plane.c b/drivers/gpu/drm/omapdrm/omap_plane.c
index 99663ec..57d5b03 100644
--- a/drivers/gpu/drm/omapdrm/omap_plane.c
+++ b/drivers/gpu/drm/omapdrm/omap_plane.c
@@ -17,8 +17,8 @@
  * this program.  If not, see <http://www.gnu.org/licenses/>.
  */
 
-#include "omap_drv.h"
 #include "omap_dmm_tiler.h"
+#include "omap_drv.h"
 
 /* some hackery because omapdss has an 'enum omap_plane' (which would be
  * better named omap_plane_id).. and compiler seems unhappy about having
-- 
2.0.5

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

[PATCH 11/31] drm: omapdrm: Rename CRTC DSS operations with an omap_crtc_dss_ prefix

[Laurent Pinchart]

The omap_crtc_enable() and omap_crtc_disable() DSS operations functions
will clash with the new CRTC enable and disable helpers. Rename them to
omap_crtc_dss_*, as well as the other DSS operations for consistency.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 drivers/gpu/drm/omapdrm/omap_crtc.c | 36 ++++++++++++++++++------------------
 1 file changed, 18 insertions(+), 18 deletions(-)

diff --git a/drivers/gpu/drm/omapdrm/omap_crtc.c b/drivers/gpu/drm/omapdrm/omap_crtc.c
index 6840ed5..94c1bb8 100644
--- a/drivers/gpu/drm/omapdrm/omap_crtc.c
+++ b/drivers/gpu/drm/omapdrm/omap_crtc.c
@@ -131,7 +131,7 @@ enum omap_channel omap_crtc_channel(struct drm_crtc *crtc)
 static struct omap_crtc *omap_crtcs[8];
 
 /* we can probably ignore these until we support command-mode panels: */
-static int omap_crtc_connect(struct omap_overlay_manager *mgr,
+static int omap_crtc_dss_connect(struct omap_overlay_manager *mgr,
 		struct omap_dss_device *dst)
 {
 	if (mgr->output)
@@ -146,14 +146,14 @@ static int omap_crtc_connect(struct omap_overlay_manager *mgr,
 	return 0;
 }
 
-static void omap_crtc_disconnect(struct omap_overlay_manager *mgr,
+static void omap_crtc_dss_disconnect(struct omap_overlay_manager *mgr,
 		struct omap_dss_device *dst)
 {
 	mgr->output->manager = NULL;
 	mgr->output = NULL;
 }
 
-static void omap_crtc_start_update(struct omap_overlay_manager *mgr)
+static void omap_crtc_dss_start_update(struct omap_overlay_manager *mgr)
 {
 }
 
@@ -215,7 +215,7 @@ static void omap_crtc_set_enabled(struct drm_crtc *crtc, bool enable)
 }
 
 
-static int omap_crtc_enable(struct omap_overlay_manager *mgr)
+static int omap_crtc_dss_enable(struct omap_overlay_manager *mgr)
 {
 	struct omap_crtc *omap_crtc = omap_crtcs[mgr->id];
 
@@ -227,14 +227,14 @@ static int omap_crtc_enable(struct omap_overlay_manager *mgr)
 	return 0;
 }
 
-static void omap_crtc_disable(struct omap_overlay_manager *mgr)
+static void omap_crtc_dss_disable(struct omap_overlay_manager *mgr)
 {
 	struct omap_crtc *omap_crtc = omap_crtcs[mgr->id];
 
 	omap_crtc_set_enabled(&omap_crtc->base, false);
 }
 
-static void omap_crtc_set_timings(struct omap_overlay_manager *mgr,
+static void omap_crtc_dss_set_timings(struct omap_overlay_manager *mgr,
 		const struct omap_video_timings *timings)
 {
 	struct omap_crtc *omap_crtc = omap_crtcs[mgr->id];
@@ -242,7 +242,7 @@ static void omap_crtc_set_timings(struct omap_overlay_manager *mgr,
 	omap_crtc->timings = *timings;
 }
 
-static void omap_crtc_set_lcd_config(struct omap_overlay_manager *mgr,
+static void omap_crtc_dss_set_lcd_config(struct omap_overlay_manager *mgr,
 		const struct dss_lcd_mgr_config *config)
 {
 	struct omap_crtc *omap_crtc = omap_crtcs[mgr->id];
@@ -250,29 +250,29 @@ static void omap_crtc_set_lcd_config(struct omap_overlay_manager *mgr,
 	dispc_mgr_set_lcd_config(omap_crtc->channel, config);
 }
 
-static int omap_crtc_register_framedone_handler(
+static int omap_crtc_dss_register_framedone(
 		struct omap_overlay_manager *mgr,
 		void (*handler)(void *), void *data)
 {
 	return 0;
 }
 
-static void omap_crtc_unregister_framedone_handler(
+static void omap_crtc_dss_unregister_framedone(
 		struct omap_overlay_manager *mgr,
 		void (*handler)(void *), void *data)
 {
 }
 
 static const struct dss_mgr_ops mgr_ops = {
-	.connect = omap_crtc_connect,
-	.disconnect = omap_crtc_disconnect,
-	.start_update = omap_crtc_start_update,
-	.enable = omap_crtc_enable,
-	.disable = omap_crtc_disable,
-	.set_timings = omap_crtc_set_timings,
-	.set_lcd_config = omap_crtc_set_lcd_config,
-	.register_framedone_handler = omap_crtc_register_framedone_handler,
-	.unregister_framedone_handler = omap_crtc_unregister_framedone_handler,
+	.connect = omap_crtc_dss_connect,
+	.disconnect = omap_crtc_dss_disconnect,
+	.start_update = omap_crtc_dss_start_update,
+	.enable = omap_crtc_dss_enable,
+	.disable = omap_crtc_dss_disable,
+	.set_timings = omap_crtc_dss_set_timings,
+	.set_lcd_config = omap_crtc_dss_set_lcd_config,
+	.register_framedone_handler = omap_crtc_dss_register_framedone,
+	.unregister_framedone_handler = omap_crtc_dss_unregister_framedone,
 };
 
 /* -----------------------------------------------------------------------------
-- 
2.0.5

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

[PATCH 12/31] drm: omapdrm: Rework CRTC enable/disable for atomic updates

[Laurent Pinchart]

When using atomic updates the CRTC .enable() and .disable() helper
operations are preferred over the (then legacy) .prepare() and .commit()
operations. Implement .enable() and rework .disable() to not depend on
DPMS, easing DPMS removal later on.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 drivers/gpu/drm/omapdrm/omap_crtc.c | 83 ++++++++++++++++++++++++++-----------
 1 file changed, 59 insertions(+), 24 deletions(-)

diff --git a/drivers/gpu/drm/omapdrm/omap_crtc.c b/drivers/gpu/drm/omapdrm/omap_crtc.c
index 94c1bb8..0359a67 100644
--- a/drivers/gpu/drm/omapdrm/omap_crtc.c
+++ b/drivers/gpu/drm/omapdrm/omap_crtc.c
@@ -509,50 +509,70 @@ static void omap_crtc_destroy(struct drm_crtc *crtc)
 	kfree(omap_crtc);
 }
 
-static void omap_crtc_dpms(struct drm_crtc *crtc, int mode)
+static bool omap_crtc_mode_fixup(struct drm_crtc *crtc,
+		const struct drm_display_mode *mode,
+		struct drm_display_mode *adjusted_mode)
+{
+	return true;
+}
+
+static void omap_crtc_enable(struct drm_crtc *crtc)
 {
 	struct omap_drm_private *priv = crtc->dev->dev_private;
 	struct omap_crtc *omap_crtc = to_omap_crtc(crtc);
-	bool enable = (mode == DRM_MODE_DPMS_ON);
-	int i;
+	unsigned int i;
 
-	DBG("%s: %d", omap_crtc->name, mode);
+	DBG("%s", omap_crtc->name);
 
-	if (enable == omap_crtc->enabled)
+	if (omap_crtc->enabled)
 		return;
 
-	if (!enable) {
-		omap_crtc_wait_page_flip(crtc);
-		dispc_runtime_get();
-		drm_crtc_vblank_off(crtc);
-		dispc_runtime_put();
-	}
-
-	/* Enable/disable all planes associated with the CRTC. */
+	/* Enable all planes associated with the CRTC. */
 	for (i = 0; i < priv->num_planes; i++) {
 		struct drm_plane *plane = priv->planes[i];
 
 		if (plane->crtc == crtc)
-			WARN_ON(omap_plane_set_enable(plane, enable));
+			WARN_ON(omap_plane_set_enable(plane, true));
 	}
 
-	omap_crtc->enabled = enable;
+	omap_crtc->enabled = true;
 
 	omap_crtc_setup(crtc);
 	omap_crtc_flush(crtc);
 
-	if (enable) {
-		dispc_runtime_get();
-		drm_crtc_vblank_on(crtc);
-		dispc_runtime_put();
-	}
+	dispc_runtime_get();
+	drm_crtc_vblank_on(crtc);
+	dispc_runtime_put();
 }
 
-static bool omap_crtc_mode_fixup(struct drm_crtc *crtc,
-		const struct drm_display_mode *mode,
-		struct drm_display_mode *adjusted_mode)
+static void omap_crtc_disable(struct drm_crtc *crtc)
 {
-	return true;
+	struct omap_drm_private *priv = crtc->dev->dev_private;
+	struct omap_crtc *omap_crtc = to_omap_crtc(crtc);
+	unsigned int i;
+
+	DBG("%s", omap_crtc->name);
+
+	if (!omap_crtc->enabled)
+		return;
+
+	omap_crtc_wait_page_flip(crtc);
+	dispc_runtime_get();
+	drm_crtc_vblank_off(crtc);
+	dispc_runtime_put();
+
+	/* Disable all planes associated with the CRTC. */
+	for (i = 0; i < priv->num_planes; i++) {
+		struct drm_plane *plane = priv->planes[i];
+
+		if (plane->crtc == crtc)
+			WARN_ON(omap_plane_set_enable(plane, false));
+	}
+
+	omap_crtc->enabled = false;
+
+	omap_crtc_setup(crtc);
+	omap_crtc_flush(crtc);
 }
 
 static int omap_crtc_mode_set(struct drm_crtc *crtc,
@@ -587,6 +607,19 @@ static int omap_crtc_mode_set(struct drm_crtc *crtc,
 				   x, y, mode->hdisplay, mode->vdisplay);
 }
 
+static void omap_crtc_dpms(struct drm_crtc *crtc, int mode)
+{
+	struct omap_crtc *omap_crtc = to_omap_crtc(crtc);
+	bool enable = (mode == DRM_MODE_DPMS_ON);
+
+	DBG("%s: %d", omap_crtc->name, mode);
+
+	if (enable)
+		omap_crtc_enable(crtc);
+	else
+		omap_crtc_disable(crtc);
+}
+
 static void omap_crtc_prepare(struct drm_crtc *crtc)
 {
 	struct omap_crtc *omap_crtc = to_omap_crtc(crtc);
@@ -751,6 +784,8 @@ static const struct drm_crtc_helper_funcs omap_crtc_helper_funcs = {
 	.prepare = omap_crtc_prepare,
 	.commit = omap_crtc_commit,
 	.mode_set_base = omap_crtc_mode_set_base,
+	.disable = omap_crtc_disable,
+	.enable = omap_crtc_enable,
 };
 
 /* -----------------------------------------------------------------------------
-- 
2.0.5

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

[PATCH 13/31] drm: omapdrm: Implement encoder .disable() and .enable() operations

[Laurent Pinchart]

The operations are required by the atomic helpers, implement them.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 drivers/gpu/drm/omapdrm/omap_encoder.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/drivers/gpu/drm/omapdrm/omap_encoder.c b/drivers/gpu/drm/omapdrm/omap_encoder.c
index ab6ed49..0734527 100644
--- a/drivers/gpu/drm/omapdrm/omap_encoder.c
+++ b/drivers/gpu/drm/omapdrm/omap_encoder.c
@@ -124,12 +124,22 @@ static void omap_encoder_commit(struct drm_encoder *encoder)
 {
 }
 
+static void omap_encoder_disable(struct drm_encoder *encoder)
+{
+}
+
+static void omap_encoder_enable(struct drm_encoder *encoder)
+{
+}
+
 static const struct drm_encoder_helper_funcs omap_encoder_helper_funcs = {
 	.dpms = omap_encoder_dpms,
 	.mode_fixup = omap_encoder_mode_fixup,
 	.mode_set = omap_encoder_mode_set,
 	.prepare = omap_encoder_prepare,
 	.commit = omap_encoder_commit,
+	.disable = omap_encoder_disable,
+	.enable = omap_encoder_enable,
 };
 
 /*
-- 
2.0.5

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

[PATCH 14/31] drm: omapdrm: Wire up atomic state object scaffolding

[Laurent Pinchart]

Hook up the default .reset(), .atomic_duplicate_state() and
.atomic_free_state() helpers to ensure that state objects are properly
created and destroyed, and call drm_mode_config_reset() at init time to
create the initial state objects.

Framebuffer reference count also gets maintained automatically by the
transitional helpers except for the legacy page flip operation. Maintain
it explicitly there.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 drivers/gpu/drm/omapdrm/omap_connector.c | 4 ++++
 drivers/gpu/drm/omapdrm/omap_crtc.c      | 6 ++++++
 drivers/gpu/drm/omapdrm/omap_drv.c       | 2 ++
 drivers/gpu/drm/omapdrm/omap_plane.c     | 5 +++++
 4 files changed, 17 insertions(+)

diff --git a/drivers/gpu/drm/omapdrm/omap_connector.c b/drivers/gpu/drm/omapdrm/omap_connector.c
index 71081b6..47781cce 100644
--- a/drivers/gpu/drm/omapdrm/omap_connector.c
+++ b/drivers/gpu/drm/omapdrm/omap_connector.c
@@ -17,6 +17,7 @@
  * this program.  If not, see <http://www.gnu.org/licenses/>.
  */
 
+#include <drm/drm_atomic_helper.h>
 #include <drm/drm_crtc.h>
 #include <drm/drm_crtc_helper.h>
 
@@ -260,9 +261,12 @@ struct drm_encoder *omap_connector_attached_encoder(
 
 static const struct drm_connector_funcs omap_connector_funcs = {
 	.dpms = drm_helper_connector_dpms,
+	.reset = drm_atomic_helper_connector_reset,
 	.detect = omap_connector_detect,
 	.fill_modes = drm_helper_probe_single_connector_modes,
 	.destroy = omap_connector_destroy,
+	.atomic_duplicate_state = drm_atomic_helper_connector_duplicate_state,
+	.atomic_destroy_state = drm_atomic_helper_connector_destroy_state,
 };
 
 static const struct drm_connector_helper_funcs omap_connector_helper_funcs = {
diff --git a/drivers/gpu/drm/omapdrm/omap_crtc.c b/drivers/gpu/drm/omapdrm/omap_crtc.c
index 0359a67..5f4f5ad 100644
--- a/drivers/gpu/drm/omapdrm/omap_crtc.c
+++ b/drivers/gpu/drm/omapdrm/omap_crtc.c
@@ -19,6 +19,8 @@
 
 #include <linux/completion.h>
 
+#include <drm/drm_atomic.h>
+#include <drm/drm_atomic_helper.h>
 #include <drm/drm_crtc.h>
 #include <drm/drm_crtc_helper.h>
 #include <drm/drm_mode.h>
@@ -742,6 +744,7 @@ static int omap_crtc_page_flip(struct drm_crtc *crtc,
 	omap_crtc->flip_event = event;
 	omap_crtc->flip_state = OMAP_PAGE_FLIP_WAIT;
 
+	drm_atomic_set_fb_for_plane(primary->state, fb);
 	primary->fb = fb;
 
 	spin_unlock_irqrestore(&dev->event_lock, flags);
@@ -771,10 +774,13 @@ static int omap_crtc_set_property(struct drm_crtc *crtc,
 }
 
 static const struct drm_crtc_funcs omap_crtc_funcs = {
+	.reset = drm_atomic_helper_crtc_reset,
 	.set_config = drm_crtc_helper_set_config,
 	.destroy = omap_crtc_destroy,
 	.page_flip = omap_crtc_page_flip,
 	.set_property = omap_crtc_set_property,
+	.atomic_duplicate_state = drm_atomic_helper_crtc_duplicate_state,
+	.atomic_destroy_state = drm_atomic_helper_crtc_destroy_state,
 };
 
 static const struct drm_crtc_helper_funcs omap_crtc_helper_funcs = {
diff --git a/drivers/gpu/drm/omapdrm/omap_drv.c b/drivers/gpu/drm/omapdrm/omap_drv.c
index dbd3046..5bf4b2b 100644
--- a/drivers/gpu/drm/omapdrm/omap_drv.c
+++ b/drivers/gpu/drm/omapdrm/omap_drv.c
@@ -350,6 +350,8 @@ static int omap_modeset_init(struct drm_device *dev)
 
 	dev->mode_config.funcs = &omap_mode_config_funcs;
 
+	drm_mode_config_reset(dev);
+
 	return 0;
 }
 
diff --git a/drivers/gpu/drm/omapdrm/omap_plane.c b/drivers/gpu/drm/omapdrm/omap_plane.c
index 57d5b03..533dcc1 100644
--- a/drivers/gpu/drm/omapdrm/omap_plane.c
+++ b/drivers/gpu/drm/omapdrm/omap_plane.c
@@ -17,6 +17,8 @@
  * this program.  If not, see <http://www.gnu.org/licenses/>.
  */
 
+#include <drm/drm_atomic_helper.h>
+
 #include "omap_dmm_tiler.h"
 #include "omap_drv.h"
 
@@ -287,8 +289,11 @@ int omap_plane_set_property(struct drm_plane *plane,
 static const struct drm_plane_funcs omap_plane_funcs = {
 	.update_plane = omap_plane_update,
 	.disable_plane = omap_plane_disable,
+	.reset = drm_atomic_helper_plane_reset,
 	.destroy = omap_plane_destroy,
 	.set_property = omap_plane_set_property,
+	.atomic_duplicate_state = drm_atomic_helper_plane_duplicate_state,
+	.atomic_destroy_state = drm_atomic_helper_plane_destroy_state,
 };
 
 static void omap_plane_error_irq(struct omap_drm_irq *irq, uint32_t irqstatus)
-- 
2.0.5

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

[PATCH 15/31] drm: omapdrm: Implement planes atomic operations

[Laurent Pinchart]

Implement the CRTC .atomic_begin() and .atomic_flush() operations, the
plane .atomic_check(), .atomic_update() and operations, and use the
transitional atomic helpers to implement the plane update and disable
operations on top of the new atomic operations.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 drivers/gpu/drm/omapdrm/omap_crtc.c  |  14 ++++
 drivers/gpu/drm/omapdrm/omap_plane.c | 127 ++++++++++++++++++++++-------------
 2 files changed, 93 insertions(+), 48 deletions(-)

diff --git a/drivers/gpu/drm/omapdrm/omap_crtc.c b/drivers/gpu/drm/omapdrm/omap_crtc.c
index 5f4f5ad..277cad1 100644
--- a/drivers/gpu/drm/omapdrm/omap_crtc.c
+++ b/drivers/gpu/drm/omapdrm/omap_crtc.c
@@ -652,6 +652,18 @@ static int omap_crtc_mode_set_base(struct drm_crtc *crtc, int x, int y,
 	return omap_crtc_flush(crtc);
 }
 
+static void omap_crtc_atomic_begin(struct drm_crtc *crtc)
+{
+	dispc_runtime_get();
+}
+
+static void omap_crtc_atomic_flush(struct drm_crtc *crtc)
+{
+	omap_crtc_flush(crtc);
+
+	dispc_runtime_put();
+}
+
 static void page_flip_worker(struct work_struct *work)
 {
 	struct omap_crtc *omap_crtc =
@@ -792,6 +804,8 @@ static const struct drm_crtc_helper_funcs omap_crtc_helper_funcs = {
 	.mode_set_base = omap_crtc_mode_set_base,
 	.disable = omap_crtc_disable,
 	.enable = omap_crtc_enable,
+	.atomic_begin = omap_crtc_atomic_begin,
+	.atomic_flush = omap_crtc_atomic_flush,
 };
 
 /* -----------------------------------------------------------------------------
diff --git a/drivers/gpu/drm/omapdrm/omap_plane.c b/drivers/gpu/drm/omapdrm/omap_plane.c
index 533dcc1..7587bac 100644
--- a/drivers/gpu/drm/omapdrm/omap_plane.c
+++ b/drivers/gpu/drm/omapdrm/omap_plane.c
@@ -18,6 +18,7 @@
  */
 
 #include <drm/drm_atomic_helper.h>
+#include <drm/drm_plane_helper.h>
 
 #include "omap_dmm_tiler.h"
 #include "omap_drv.h"
@@ -87,30 +88,23 @@ static int omap_plane_update_pin(struct drm_plane *plane)
 	return 0;
 }
 
-static int omap_plane_setup(struct omap_plane *omap_plane)
+static int __omap_plane_setup(struct omap_plane *omap_plane,
+			      struct drm_crtc *crtc,
+			      struct drm_framebuffer *fb)
 {
 	struct omap_overlay_info *info = &omap_plane->info;
-	struct drm_plane *plane = &omap_plane->base;
-	struct drm_device *dev = plane->dev;
-	struct drm_crtc *crtc = plane->crtc;
+	struct drm_device *dev = omap_plane->base.dev;
 	int ret;
 
 	DBG("%s, enabled=%d", omap_plane->name, omap_plane->enabled);
 
-	/* if fb has changed, pin new fb: */
-	ret = omap_plane_update_pin(plane);
-	if (ret)
-		return ret;
-
-	dispc_runtime_get();
-
 	if (!omap_plane->enabled) {
 		dispc_ovl_enable(omap_plane->id, false);
-		goto done;
+		return 0;
 	}
 
 	/* update scanout: */
-	omap_framebuffer_update_scanout(plane->fb, &omap_plane->win, info);
+	omap_framebuffer_update_scanout(fb, &omap_plane->win, info);
 
 	DBG("%dx%d -> %dx%d (%d)", info->width, info->height,
 			info->out_width, info->out_height,
@@ -126,13 +120,27 @@ static int omap_plane_setup(struct omap_plane *omap_plane)
 			      omap_crtc_timings(crtc), false);
 	if (ret) {
 		dev_err(dev->dev, "dispc_ovl_setup failed: %d\n", ret);
-		goto done;
+		return ret;
 	}
 
 	dispc_ovl_enable(omap_plane->id, true);
 
-done:
+	return 0;
+}
+
+static int omap_plane_setup(struct omap_plane *omap_plane)
+{
+	struct drm_plane *plane = &omap_plane->base;
+	int ret;
+
+	ret = omap_plane_update_pin(plane);
+	if (ret < 0)
+		return ret;
+
+	dispc_runtime_get();
+	ret = __omap_plane_setup(omap_plane, plane->crtc, plane->fb);
 	dispc_runtime_put();
+
 	return ret;
 }
 
@@ -170,45 +178,62 @@ int omap_plane_set_enable(struct drm_plane *plane, bool enable)
 	return omap_plane_setup(omap_plane);
 }
 
-static int omap_plane_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)
+static int omap_plane_prepare_fb(struct drm_plane *plane,
+				 struct drm_framebuffer *fb,
+				 const struct drm_plane_state *new_state)
+{
+	return omap_framebuffer_pin(fb);
+}
+
+static void omap_plane_cleanup_fb(struct drm_plane *plane,
+				  struct drm_framebuffer *fb,
+				  const struct drm_plane_state *old_state)
+{
+	omap_framebuffer_unpin(fb);
+}
+
+static void omap_plane_atomic_update(struct drm_plane *plane,
+				     struct drm_plane_state *old_state)
 {
 	struct omap_plane *omap_plane = to_omap_plane(plane);
-	int ret;
+	struct omap_drm_window *win = &omap_plane->win;
+	struct drm_plane_state *state = plane->state;
+	uint32_t src_w;
+	uint32_t src_h;
 
-	omap_plane->enabled = true;
+	if (!state->fb || !state->crtc)
+		return;
 
-	/* omap_plane_mode_set() takes adjusted src */
+	/* omap_framebuffer_update_scanout() takes adjusted src */
 	switch (omap_plane->win.rotation & 0xf) {
 	case BIT(DRM_ROTATE_90):
 	case BIT(DRM_ROTATE_270):
-		swap(src_w, src_h);
+		src_w = state->src_h;
+		src_h = state->src_w;
+		break;
+	default:
+		src_w = state->src_w;
+		src_h = state->src_h;
 		break;
 	}
 
-	/*
-	 * We don't need to take a reference to the framebuffer as the DRM core
-	 * has already done so for the purpose of setting plane->fb.
-	 */
-	plane->fb = fb;
-	plane->crtc = crtc;
-
-	/* src values are in Q16 fixed point, convert to integer: */
-	ret = omap_plane_mode_set(plane, crtc, fb,
-				  crtc_x, crtc_y, crtc_w, crtc_h,
-				  src_x >> 16, src_y >> 16,
-				  src_w >> 16, src_h >> 16);
-	if (ret < 0)
-		return ret;
+	/* src values are in Q16 fixed point, convert to integer. */
+	win->crtc_x = state->crtc_x;
+	win->crtc_y = state->crtc_y;
+	win->crtc_w = state->crtc_w;
+	win->crtc_h = state->crtc_h;
 
-	return omap_crtc_flush(plane->crtc);
+	win->src_x = state->src_x >> 16;
+	win->src_y = state->src_y >> 16;
+	win->src_w = src_w >> 16;
+	win->src_h = src_h >> 16;
+
+	omap_plane->enabled = true;
+	__omap_plane_setup(omap_plane, state->crtc, state->fb);
 }
 
-static int omap_plane_disable(struct drm_plane *plane)
+static void omap_plane_atomic_disable(struct drm_plane *plane,
+				      struct drm_plane_state *old_state)
 {
 	struct omap_plane *omap_plane = to_omap_plane(plane);
 
@@ -217,15 +242,19 @@ static int omap_plane_disable(struct drm_plane *plane)
 				? 0 : omap_plane->id;
 
 	if (!omap_plane->enabled)
-		return 0;
+		return;
 
-	/* Disabling a plane never fails. */
 	omap_plane->enabled = false;
-	omap_plane_setup(omap_plane);
-
-	return omap_crtc_flush(plane->crtc);
+	__omap_plane_setup(omap_plane, NULL, NULL);
 }
 
+static const struct drm_plane_helper_funcs omap_plane_helper_funcs = {
+	.prepare_fb = omap_plane_prepare_fb,
+	.cleanup_fb = omap_plane_cleanup_fb,
+	.atomic_update = omap_plane_atomic_update,
+	.atomic_disable = omap_plane_atomic_disable,
+};
+
 static void omap_plane_destroy(struct drm_plane *plane)
 {
 	struct omap_plane *omap_plane = to_omap_plane(plane);
@@ -287,8 +316,8 @@ int omap_plane_set_property(struct drm_plane *plane,
 }
 
 static const struct drm_plane_funcs omap_plane_funcs = {
-	.update_plane = omap_plane_update,
-	.disable_plane = omap_plane_disable,
+	.update_plane = drm_plane_helper_update,
+	.disable_plane = drm_plane_helper_disable,
 	.reset = drm_atomic_helper_plane_reset,
 	.destroy = omap_plane_destroy,
 	.set_property = omap_plane_set_property,
@@ -352,6 +381,8 @@ struct drm_plane *omap_plane_init(struct drm_device *dev,
 	if (ret < 0)
 		goto error;
 
+	drm_plane_helper_add(plane, &omap_plane_helper_funcs);
+
 	omap_plane_install_properties(plane, &plane->base);
 
 	/* get our starting configuration, set defaults for parameters
-- 
2.0.5

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

[PATCH 16/31] drm: omapdrm: Handle primary plane config through atomic plane ops

[Laurent Pinchart]

Use the new CRTC atomic transitional helpers drm_helper_crtc_mode_set()
and drm_helper_crtc_mode_set_base() to implement the CRTC .mode_set and
.mode_set_base operations. This delegates primary plane configuration to
the plane .atomic_update and .atomic_disable operations, removing
duplicate code from the CRTC implementation.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 drivers/gpu/drm/omapdrm/omap_crtc.c | 69 +++++++++++++++----------------------
 1 file changed, 27 insertions(+), 42 deletions(-)

diff --git a/drivers/gpu/drm/omapdrm/omap_crtc.c b/drivers/gpu/drm/omapdrm/omap_crtc.c
index 277cad1..9630fec 100644
--- a/drivers/gpu/drm/omapdrm/omap_crtc.c
+++ b/drivers/gpu/drm/omapdrm/omap_crtc.c
@@ -571,42 +571,42 @@ static void omap_crtc_disable(struct drm_crtc *crtc)
 			WARN_ON(omap_plane_set_enable(plane, false));
 	}
 
+	/*
+	 * HACK: Unpin the primary plane frame buffer if we're disabled without
+	 * going through full mode set.
+	 * HACK: The legacy set config helper drm_crtc_helper_set_config() that
+	 * we still use calls the .disable() operation directly when called with
+	 * a NULL frame buffer (for instance from drm_fb_release()). As a result
+	 * the CRTC is disabled without going through a full mode set, and the
+	 * primary plane' framebuffer is kept pin. Unpin it manually here until
+	 * we switch to the atomic set config helper.
+	 */
+	if (crtc->primary->fb) {
+		struct drm_plane_helper_funcs *funcs;
+
+		funcs = crtc->primary->helper_private;
+		funcs->cleanup_fb(crtc->primary, crtc->primary->fb, NULL);
+	}
+
 	omap_crtc->enabled = false;
 
 	omap_crtc_setup(crtc);
 	omap_crtc_flush(crtc);
 }
 
-static int omap_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)
+static void omap_crtc_mode_set_nofb(struct drm_crtc *crtc)
 {
 	struct omap_crtc *omap_crtc = to_omap_crtc(crtc);
-
-	mode = adjusted_mode;
+	struct drm_display_mode *mode = &crtc->state->adjusted_mode;
 
 	DBG("%s: set mode: %d:\"%s\" %d %d %d %d %d %d %d %d %d %d 0x%x 0x%x",
-			omap_crtc->name, mode->base.id, mode->name,
-			mode->vrefresh, mode->clock,
-			mode->hdisplay, mode->hsync_start,
-			mode->hsync_end, mode->htotal,
-			mode->vdisplay, mode->vsync_start,
-			mode->vsync_end, mode->vtotal,
-			mode->type, mode->flags);
+	    omap_crtc->name, mode->base.id, mode->name,
+	    mode->vrefresh, mode->clock,
+	    mode->hdisplay, mode->hsync_start, mode->hsync_end, mode->htotal,
+	    mode->vdisplay, mode->vsync_start, mode->vsync_end, mode->vtotal,
+	    mode->type, mode->flags);
 
 	copy_timings_drm_to_omap(&omap_crtc->timings, mode);
-
-	/*
-	 * The primary plane CRTC can be reset if the plane is disabled directly
-	 * through the universal plane API. Set it again here.
-	 */
-	crtc->primary->crtc = crtc;
-
-	return omap_plane_mode_set(crtc->primary, crtc, crtc->primary->fb,
-				   0, 0, mode->hdisplay, mode->vdisplay,
-				   x, y, mode->hdisplay, mode->vdisplay);
 }
 
 static void omap_crtc_dpms(struct drm_crtc *crtc, int mode)
@@ -636,22 +636,6 @@ static void omap_crtc_commit(struct drm_crtc *crtc)
 	omap_crtc_dpms(crtc, DRM_MODE_DPMS_ON);
 }
 
-static int omap_crtc_mode_set_base(struct drm_crtc *crtc, int x, int y,
-		struct drm_framebuffer *old_fb)
-{
-	struct drm_plane *plane = crtc->primary;
-	struct drm_display_mode *mode = &crtc->mode;
-	int ret;
-
-	ret = omap_plane_mode_set(plane, crtc, crtc->primary->fb,
-				  0, 0, mode->hdisplay, mode->vdisplay,
-				  x, y, mode->hdisplay, mode->vdisplay);
-	if (ret < 0)
-		return ret;
-
-	return omap_crtc_flush(crtc);
-}
-
 static void omap_crtc_atomic_begin(struct drm_crtc *crtc)
 {
 	dispc_runtime_get();
@@ -798,10 +782,11 @@ static const struct drm_crtc_funcs omap_crtc_funcs = {
 static const struct drm_crtc_helper_funcs omap_crtc_helper_funcs = {
 	.dpms = omap_crtc_dpms,
 	.mode_fixup = omap_crtc_mode_fixup,
-	.mode_set = omap_crtc_mode_set,
+	.mode_set = drm_helper_crtc_mode_set,
 	.prepare = omap_crtc_prepare,
 	.commit = omap_crtc_commit,
-	.mode_set_base = omap_crtc_mode_set_base,
+	.mode_set_nofb = omap_crtc_mode_set_nofb,
+	.mode_set_base = drm_helper_crtc_mode_set_base,
 	.disable = omap_crtc_disable,
 	.enable = omap_crtc_enable,
 	.atomic_begin = omap_crtc_atomic_begin,
-- 
2.0.5

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

[PATCH 17/31] drm: omapdrm: Switch plane update to atomic helpers

[Laurent Pinchart]

This removes the legacy plane update code. Wire up the default atomic
check and atomic commit mode config helpers as needed by the plane
update atomic helpers.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 drivers/gpu/drm/omapdrm/omap_drv.c   | 3 +++
 drivers/gpu/drm/omapdrm/omap_plane.c | 4 ++--
 2 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/omapdrm/omap_drv.c b/drivers/gpu/drm/omapdrm/omap_drv.c
index 5bf4b2b..ec0ae42 100644
--- a/drivers/gpu/drm/omapdrm/omap_drv.c
+++ b/drivers/gpu/drm/omapdrm/omap_drv.c
@@ -17,6 +17,7 @@
  * this program.  If not, see <http://www.gnu.org/licenses/>.
  */
 
+#include <drm/drm_atomic_helper.h>
 #include <drm/drm_crtc_helper.h>
 #include <drm/drm_fb_helper.h>
 
@@ -58,6 +59,8 @@ static void omap_fb_output_poll_changed(struct drm_device *dev)
 static const struct drm_mode_config_funcs omap_mode_config_funcs = {
 	.fb_create = omap_framebuffer_create,
 	.output_poll_changed = omap_fb_output_poll_changed,
+	.atomic_check = drm_atomic_helper_check,
+	.atomic_commit = drm_atomic_helper_commit,
 };
 
 static int get_connector_type(struct omap_dss_device *dssdev)
diff --git a/drivers/gpu/drm/omapdrm/omap_plane.c b/drivers/gpu/drm/omapdrm/omap_plane.c
index 7587bac..fcc5d96 100644
--- a/drivers/gpu/drm/omapdrm/omap_plane.c
+++ b/drivers/gpu/drm/omapdrm/omap_plane.c
@@ -316,8 +316,8 @@ int omap_plane_set_property(struct drm_plane *plane,
 }
 
 static const struct drm_plane_funcs omap_plane_funcs = {
-	.update_plane = drm_plane_helper_update,
-	.disable_plane = drm_plane_helper_disable,
+	.update_plane = drm_atomic_helper_update_plane,
+	.disable_plane = drm_atomic_helper_disable_plane,
 	.reset = drm_atomic_helper_plane_reset,
 	.destroy = omap_plane_destroy,
 	.set_property = omap_plane_set_property,
-- 
2.0.5

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

[PATCH 18/31] drm: omapdrm: Switch mode config to atomic helpers

[Laurent Pinchart]

This removes the legacy mode config code. The CRTC and encoder prepare
and commit operations are not used anymore, remove them.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 drivers/gpu/drm/omapdrm/omap_crtc.c    | 37 +---------------------------------
 drivers/gpu/drm/omapdrm/omap_encoder.c | 10 ---------
 2 files changed, 1 insertion(+), 46 deletions(-)

diff --git a/drivers/gpu/drm/omapdrm/omap_crtc.c b/drivers/gpu/drm/omapdrm/omap_crtc.c
index 9630fec..68bf38b 100644
--- a/drivers/gpu/drm/omapdrm/omap_crtc.c
+++ b/drivers/gpu/drm/omapdrm/omap_crtc.c
@@ -571,23 +571,6 @@ static void omap_crtc_disable(struct drm_crtc *crtc)
 			WARN_ON(omap_plane_set_enable(plane, false));
 	}
 
-	/*
-	 * HACK: Unpin the primary plane frame buffer if we're disabled without
-	 * going through full mode set.
-	 * HACK: The legacy set config helper drm_crtc_helper_set_config() that
-	 * we still use calls the .disable() operation directly when called with
-	 * a NULL frame buffer (for instance from drm_fb_release()). As a result
-	 * the CRTC is disabled without going through a full mode set, and the
-	 * primary plane' framebuffer is kept pin. Unpin it manually here until
-	 * we switch to the atomic set config helper.
-	 */
-	if (crtc->primary->fb) {
-		struct drm_plane_helper_funcs *funcs;
-
-		funcs = crtc->primary->helper_private;
-		funcs->cleanup_fb(crtc->primary, crtc->primary->fb, NULL);
-	}
-
 	omap_crtc->enabled = false;
 
 	omap_crtc_setup(crtc);
@@ -622,20 +605,6 @@ static void omap_crtc_dpms(struct drm_crtc *crtc, int mode)
 		omap_crtc_disable(crtc);
 }
 
-static void omap_crtc_prepare(struct drm_crtc *crtc)
-{
-	struct omap_crtc *omap_crtc = to_omap_crtc(crtc);
-	DBG("%s", omap_crtc->name);
-	omap_crtc_dpms(crtc, DRM_MODE_DPMS_OFF);
-}
-
-static void omap_crtc_commit(struct drm_crtc *crtc)
-{
-	struct omap_crtc *omap_crtc = to_omap_crtc(crtc);
-	DBG("%s", omap_crtc->name);
-	omap_crtc_dpms(crtc, DRM_MODE_DPMS_ON);
-}
-
 static void omap_crtc_atomic_begin(struct drm_crtc *crtc)
 {
 	dispc_runtime_get();
@@ -771,7 +740,7 @@ static int omap_crtc_set_property(struct drm_crtc *crtc,
 
 static const struct drm_crtc_funcs omap_crtc_funcs = {
 	.reset = drm_atomic_helper_crtc_reset,
-	.set_config = drm_crtc_helper_set_config,
+	.set_config = drm_atomic_helper_set_config,
 	.destroy = omap_crtc_destroy,
 	.page_flip = omap_crtc_page_flip,
 	.set_property = omap_crtc_set_property,
@@ -782,11 +751,7 @@ static const struct drm_crtc_funcs omap_crtc_funcs = {
 static const struct drm_crtc_helper_funcs omap_crtc_helper_funcs = {
 	.dpms = omap_crtc_dpms,
 	.mode_fixup = omap_crtc_mode_fixup,
-	.mode_set = drm_helper_crtc_mode_set,
-	.prepare = omap_crtc_prepare,
-	.commit = omap_crtc_commit,
 	.mode_set_nofb = omap_crtc_mode_set_nofb,
-	.mode_set_base = drm_helper_crtc_mode_set_base,
 	.disable = omap_crtc_disable,
 	.enable = omap_crtc_enable,
 	.atomic_begin = omap_crtc_atomic_begin,
diff --git a/drivers/gpu/drm/omapdrm/omap_encoder.c b/drivers/gpu/drm/omapdrm/omap_encoder.c
index 0734527..96459f7 100644
--- a/drivers/gpu/drm/omapdrm/omap_encoder.c
+++ b/drivers/gpu/drm/omapdrm/omap_encoder.c
@@ -116,14 +116,6 @@ static void omap_encoder_mode_set(struct drm_encoder *encoder,
 	}
 }
 
-static void omap_encoder_prepare(struct drm_encoder *encoder)
-{
-}
-
-static void omap_encoder_commit(struct drm_encoder *encoder)
-{
-}
-
 static void omap_encoder_disable(struct drm_encoder *encoder)
 {
 }
@@ -136,8 +128,6 @@ static const struct drm_encoder_helper_funcs omap_encoder_helper_funcs = {
 	.dpms = omap_encoder_dpms,
 	.mode_fixup = omap_encoder_mode_fixup,
 	.mode_set = omap_encoder_mode_set,
-	.prepare = omap_encoder_prepare,
-	.commit = omap_encoder_commit,
 	.disable = omap_encoder_disable,
 	.enable = omap_encoder_enable,
 };
-- 
2.0.5

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

[PATCH 19/31] drm: omapdrm: Switch connector DPMS to atomic helpers

[Laurent Pinchart]

The atomic connector DPMS helper implements the connector DPMS operation
using atomic commit, removing the need for DPMS helper operations on
CRTCs and encoders.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 drivers/gpu/drm/omapdrm/omap_connector.c |  2 +-
 drivers/gpu/drm/omapdrm/omap_crtc.c      | 14 --------------
 drivers/gpu/drm/omapdrm/omap_encoder.c   | 26 +++++++++-----------------
 3 files changed, 10 insertions(+), 32 deletions(-)

diff --git a/drivers/gpu/drm/omapdrm/omap_connector.c b/drivers/gpu/drm/omapdrm/omap_connector.c
index 47781cce..d081c4d 100644
--- a/drivers/gpu/drm/omapdrm/omap_connector.c
+++ b/drivers/gpu/drm/omapdrm/omap_connector.c
@@ -260,7 +260,7 @@ struct drm_encoder *omap_connector_attached_encoder(
 }
 
 static const struct drm_connector_funcs omap_connector_funcs = {
-	.dpms = drm_helper_connector_dpms,
+	.dpms = drm_atomic_helper_connector_dpms,
 	.reset = drm_atomic_helper_connector_reset,
 	.detect = omap_connector_detect,
 	.fill_modes = drm_helper_probe_single_connector_modes,
diff --git a/drivers/gpu/drm/omapdrm/omap_crtc.c b/drivers/gpu/drm/omapdrm/omap_crtc.c
index 68bf38b..5216fb0 100644
--- a/drivers/gpu/drm/omapdrm/omap_crtc.c
+++ b/drivers/gpu/drm/omapdrm/omap_crtc.c
@@ -592,19 +592,6 @@ static void omap_crtc_mode_set_nofb(struct drm_crtc *crtc)
 	copy_timings_drm_to_omap(&omap_crtc->timings, mode);
 }
 
-static void omap_crtc_dpms(struct drm_crtc *crtc, int mode)
-{
-	struct omap_crtc *omap_crtc = to_omap_crtc(crtc);
-	bool enable = (mode == DRM_MODE_DPMS_ON);
-
-	DBG("%s: %d", omap_crtc->name, mode);
-
-	if (enable)
-		omap_crtc_enable(crtc);
-	else
-		omap_crtc_disable(crtc);
-}
-
 static void omap_crtc_atomic_begin(struct drm_crtc *crtc)
 {
 	dispc_runtime_get();
@@ -749,7 +736,6 @@ static const struct drm_crtc_funcs omap_crtc_funcs = {
 };
 
 static const struct drm_crtc_helper_funcs omap_crtc_helper_funcs = {
-	.dpms = omap_crtc_dpms,
 	.mode_fixup = omap_crtc_mode_fixup,
 	.mode_set_nofb = omap_crtc_mode_set_nofb,
 	.disable = omap_crtc_disable,
diff --git a/drivers/gpu/drm/omapdrm/omap_encoder.c b/drivers/gpu/drm/omapdrm/omap_encoder.c
index 96459f7..2aeb41f 100644
--- a/drivers/gpu/drm/omapdrm/omap_encoder.c
+++ b/drivers/gpu/drm/omapdrm/omap_encoder.c
@@ -62,22 +62,6 @@ static const struct drm_encoder_funcs omap_encoder_funcs = {
 	.destroy = omap_encoder_destroy,
 };
 
-/*
- * The CRTC drm_crtc_helper_set_mode() doesn't really give us the right
- * order.. the easiest way to work around this for now is to make all
- * the encoder-helper's no-op's and have the omap_crtc code take care
- * of the sequencing and call us in the right points.
- *
- * Eventually to handle connecting CRTCs to different encoders properly,
- * either the CRTC helpers need to change or we need to replace
- * drm_crtc_helper_set_mode(), but lets wait until atomic-modeset for
- * that.
- */
-
-static void omap_encoder_dpms(struct drm_encoder *encoder, int mode)
-{
-}
-
 static bool omap_encoder_mode_fixup(struct drm_encoder *encoder,
 				  const struct drm_display_mode *mode,
 				  struct drm_display_mode *adjusted_mode)
@@ -116,6 +100,15 @@ static void omap_encoder_mode_set(struct drm_encoder *encoder,
 	}
 }
 
+/*
+ * The CRTC drm_crtc_helper_set_mode() didn't really give us the right order.
+ * The easiest way to work around this was to make all the encoder-helper's
+ * no-op's and have the omap_crtc code take care of the sequencing and call
+ * us in the right points.
+ *
+ * FIXME: Revisit this after switching to atomic updates completely.
+ */
+
 static void omap_encoder_disable(struct drm_encoder *encoder)
 {
 }
@@ -125,7 +118,6 @@ static void omap_encoder_enable(struct drm_encoder *encoder)
 }
 
 static const struct drm_encoder_helper_funcs omap_encoder_helper_funcs = {
-	.dpms = omap_encoder_dpms,
 	.mode_fixup = omap_encoder_mode_fixup,
 	.mode_set = omap_encoder_mode_set,
 	.disable = omap_encoder_disable,
-- 
2.0.5

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

[PATCH 20/31] drm: omapdrm: Replace encoder mode_fixup with atomic_check

[Laurent Pinchart]

The encoder .mode_fixup() operation is legacy, atomic updates uses the
new .atomic_check() operation. Convert the encoder driver.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 drivers/gpu/drm/omapdrm/omap_encoder.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/omapdrm/omap_encoder.c b/drivers/gpu/drm/omapdrm/omap_encoder.c
index 2aeb41f..54847ed 100644
--- a/drivers/gpu/drm/omapdrm/omap_encoder.c
+++ b/drivers/gpu/drm/omapdrm/omap_encoder.c
@@ -62,13 +62,6 @@ static const struct drm_encoder_funcs omap_encoder_funcs = {
 	.destroy = omap_encoder_destroy,
 };
 
-static bool omap_encoder_mode_fixup(struct drm_encoder *encoder,
-				  const struct drm_display_mode *mode,
-				  struct drm_display_mode *adjusted_mode)
-{
-	return true;
-}
-
 static void omap_encoder_mode_set(struct drm_encoder *encoder,
 				struct drm_display_mode *mode,
 				struct drm_display_mode *adjusted_mode)
@@ -117,11 +110,18 @@ static void omap_encoder_enable(struct drm_encoder *encoder)
 {
 }
 
+static int omap_encoder_atomic_check(struct drm_encoder *encoder,
+				     struct drm_crtc_state *crtc_state,
+				     struct drm_connector_state *conn_state)
+{
+	return 0;
+}
+
 static const struct drm_encoder_helper_funcs omap_encoder_helper_funcs = {
-	.mode_fixup = omap_encoder_mode_fixup,
 	.mode_set = omap_encoder_mode_set,
 	.disable = omap_encoder_disable,
 	.enable = omap_encoder_enable,
+	.atomic_check = omap_encoder_atomic_check,
 };
 
 /*
-- 
2.0.5

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

[PATCH 21/31] drm: omapdrm: Implement asynchronous commit support

[Laurent Pinchart]

Implement a custom .atomic_commit() handler that supports asynchronous
commits using a work queue. This can be used for userspace-driven
asynchronous commits, as well as for an atomic page flip implementation.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 drivers/gpu/drm/omapdrm/omap_drv.c | 99 +++++++++++++++++++++++++++++++++++++-
 drivers/gpu/drm/omapdrm/omap_drv.h |  7 +++
 2 files changed, 105 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/omapdrm/omap_drv.c b/drivers/gpu/drm/omapdrm/omap_drv.c
index ec0ae42..14c8714 100644
--- a/drivers/gpu/drm/omapdrm/omap_drv.c
+++ b/drivers/gpu/drm/omapdrm/omap_drv.c
@@ -17,6 +17,9 @@
  * this program.  If not, see <http://www.gnu.org/licenses/>.
  */
 
+#include <linux/wait.h>
+
+#include <drm/drm_atomic.h>
 #include <drm/drm_atomic_helper.h>
 #include <drm/drm_crtc_helper.h>
 #include <drm/drm_fb_helper.h>
@@ -56,11 +59,104 @@ static void omap_fb_output_poll_changed(struct drm_device *dev)
 		drm_fb_helper_hotplug_event(priv->fbdev);
 }
 
+struct omap_atomic_state_commit {
+	struct work_struct work;
+	struct drm_device *dev;
+	struct drm_atomic_state *state;
+	u32 crtcs;
+};
+
+static void omap_atomic_complete(struct omap_atomic_state_commit *commit)
+{
+	struct drm_device *dev = commit->dev;
+	struct omap_drm_private *priv = dev->dev_private;
+	struct drm_atomic_state *old_state = commit->state;
+
+	/* Apply the atomic update. */
+	drm_atomic_helper_commit_modeset_disables(dev, old_state);
+	drm_atomic_helper_commit_planes(dev, old_state);
+	drm_atomic_helper_commit_modeset_enables(dev, old_state);
+
+	drm_atomic_helper_wait_for_vblanks(dev, old_state);
+
+	drm_atomic_helper_cleanup_planes(dev, old_state);
+
+	drm_atomic_state_free(old_state);
+
+	/* Complete the commit, wake up any waiter. */
+	spin_lock(&priv->commit.wait.lock);
+	priv->commit.pending &= ~commit->crtcs;
+	wake_up_all_locked(&priv->commit.wait);
+	spin_unlock(&priv->commit.wait.lock);
+
+	kfree(commit);
+}
+
+static void omap_atomic_work(struct work_struct *work)
+{
+	struct omap_atomic_state_commit *commit =
+		container_of(work, struct omap_atomic_state_commit, work);
+
+	omap_atomic_complete(commit);
+}
+
+static int omap_atomic_commit(struct drm_device *dev,
+			      struct drm_atomic_state *state, bool async)
+{
+	struct omap_drm_private *priv = dev->dev_private;
+	struct omap_atomic_state_commit *commit;
+	unsigned int i;
+	int ret;
+
+	ret = drm_atomic_helper_prepare_planes(dev, state);
+	if (ret)
+		return ret;
+
+	/* Allocate the commit object. */
+	commit = kzalloc(sizeof(*commit), GFP_KERNEL);
+	if (commit == NULL)
+		return -ENOMEM;
+
+	INIT_WORK(&commit->work, omap_atomic_work);
+	commit->dev = dev;
+	commit->state = state;
+
+	/* Wait until all affected CRTCs have completed previous commits and
+	 * mark them as pending.
+	 */
+	for (i = 0; i < dev->mode_config.num_crtc; ++i) {
+		if (state->crtcs[i])
+			commit->crtcs |= 1 << drm_crtc_index(state->crtcs[i]);
+	}
+
+	spin_lock(&priv->commit.wait.lock);
+	ret = wait_event_interruptible_locked(priv->commit.wait,
+			!(priv->commit.pending & commit->crtcs));
+	if (ret == 0)
+		priv->commit.pending |= commit->crtcs;
+	spin_unlock(&priv->commit.wait.lock);
+
+	if (ret) {
+		kfree(commit);
+		return ret;
+	}
+
+	/* Swap the state, this is the point of no return. */
+	drm_atomic_helper_swap_state(dev, state);
+
+	if (async)
+		schedule_work(&commit->work);
+	else
+		omap_atomic_complete(commit);
+
+	return 0;
+}
+
 static const struct drm_mode_config_funcs omap_mode_config_funcs = {
 	.fb_create = omap_framebuffer_create,
 	.output_poll_changed = omap_fb_output_poll_changed,
 	.atomic_check = drm_atomic_helper_check,
-	.atomic_commit = drm_atomic_helper_commit,
+	.atomic_commit = omap_atomic_commit,
 };
 
 static int get_connector_type(struct omap_dss_device *dssdev)
@@ -521,6 +617,7 @@ static int dev_load(struct drm_device *dev, unsigned long flags)
 	dev->dev_private = priv;
 
 	priv->wq = alloc_ordered_workqueue("omapdrm", 0);
+	init_waitqueue_head(&priv->commit.wait);
 
 	spin_lock_init(&priv->list_lock);
 	INIT_LIST_HEAD(&priv->obj_list);
diff --git a/drivers/gpu/drm/omapdrm/omap_drv.h b/drivers/gpu/drm/omapdrm/omap_drv.h
index 6448fa8..a4737c0 100644
--- a/drivers/gpu/drm/omapdrm/omap_drv.h
+++ b/drivers/gpu/drm/omapdrm/omap_drv.h
@@ -23,6 +23,7 @@
 #include <linux/module.h>
 #include <linux/platform_data/omap_drm.h>
 #include <linux/types.h>
+#include <linux/wait.h>
 #include <video/omapdss.h>
 
 #include <drm/drmP.h>
@@ -105,6 +106,12 @@ struct omap_drm_private {
 	struct list_head irq_list;    /* list of omap_drm_irq */
 	uint32_t vblank_mask;         /* irq bits set for userspace vblank */
 	struct omap_drm_irq error_handler;
+
+	/* atomic commit */
+	struct {
+		wait_queue_head_t wait;
+		u32 pending;
+	} commit;
 };
 
 
-- 
2.0.5

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

[PATCH 22/31] drm: omapdrm: Switch page flip to atomic helpers

[Laurent Pinchart]

The atomic page flip helper implements the page flip operation using
asynchronous commits.

As the legacy page flip was the last caller of omap_plane_mode_set(),
remove the function.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 drivers/gpu/drm/omapdrm/omap_crtc.c  | 222 +++++++----------------------------
 drivers/gpu/drm/omapdrm/omap_drv.h   |   6 -
 drivers/gpu/drm/omapdrm/omap_plane.c |  23 ----
 3 files changed, 41 insertions(+), 210 deletions(-)

diff --git a/drivers/gpu/drm/omapdrm/omap_crtc.c b/drivers/gpu/drm/omapdrm/omap_crtc.c
index 5216fb0..d82b8ea 100644
--- a/drivers/gpu/drm/omapdrm/omap_crtc.c
+++ b/drivers/gpu/drm/omapdrm/omap_crtc.c
@@ -30,13 +30,6 @@
 
 #define to_omap_crtc(x) container_of(x, struct omap_crtc, base)
 
-enum omap_page_flip_state {
-	OMAP_PAGE_FLIP_IDLE,
-	OMAP_PAGE_FLIP_WAIT,
-	OMAP_PAGE_FLIP_QUEUED,
-	OMAP_PAGE_FLIP_CANCELLED,
-};
-
 struct omap_crtc {
 	struct drm_crtc base;
 
@@ -63,25 +56,9 @@ struct omap_crtc {
 	/* list of framebuffers to unpin */
 	struct list_head pending_unpins;
 
-	/*
-	 * flip_state flag indicates the current page flap state: IDLE if no
-	 * page queue has been submitted, WAIT when waiting for GEM async
-	 * completion, QUEUED when the page flip has been queued to the hardware
-	 * or CANCELLED when the CRTC is turned off before the flip gets queued
-	 * to the hardware. The flip event, if any, is stored in flip_event, and
-	 * the framebuffer queued for page flip is stored in flip_fb. The
-	 * flip_wait wait queue is used to wait for page flip completion.
-	 *
-	 * The flip_work work queue handles page flip requests without caring
-	 * about what context the GEM async callback is called from. Possibly we
-	 * should just make omap_gem always call the cb from the worker so we
-	 * don't have to care about this.
-	 */
-	enum omap_page_flip_state flip_state;
-	struct drm_pending_vblank_event *flip_event;
-	struct drm_framebuffer *flip_fb;
+	/* pending event */
+	struct drm_pending_vblank_event *event;
 	wait_queue_head_t flip_wait;
-	struct work_struct flip_work;
 
 	struct completion completion;
 
@@ -284,39 +261,46 @@ static const struct dss_mgr_ops mgr_ops = {
 void omap_crtc_cancel_page_flip(struct drm_crtc *crtc, struct drm_file *file)
 {
 	struct omap_crtc *omap_crtc = to_omap_crtc(crtc);
+	struct drm_pending_vblank_event *event;
 	struct drm_device *dev = crtc->dev;
 	unsigned long flags;
 
+	/* Destroy the pending vertical blanking event associated with the
+	 * pending page flip, if any, and disable vertical blanking interrupts.
+	 */
+
 	spin_lock_irqsave(&dev->event_lock, flags);
 
-	/* Only complete events queued for our file handle. */
-	if (omap_crtc->flip_event &&
-	    file == omap_crtc->flip_event->base.file_priv) {
-		drm_send_vblank_event(dev, omap_crtc->pipe,
-				      omap_crtc->flip_event);
-		omap_crtc->flip_event = NULL;
+	event = omap_crtc->event;
+	omap_crtc->event = NULL;
+
+	if (event && event->base.file_priv == file) {
+		event->base.destroy(&event->base);
+		drm_crtc_vblank_put(crtc);
 	}
 
 	spin_unlock_irqrestore(&dev->event_lock, flags);
 }
 
-/* Must be called with dev->event_lock locked. */
-static void omap_crtc_complete_page_flip(struct drm_crtc *crtc,
-					 enum omap_page_flip_state state)
+static void omap_crtc_complete_page_flip(struct drm_crtc *crtc)
 {
 	struct omap_crtc *omap_crtc = to_omap_crtc(crtc);
+	struct drm_pending_vblank_event *event;
 	struct drm_device *dev = crtc->dev;
+	unsigned long flags;
 
-	if (omap_crtc->flip_event) {
-		drm_send_vblank_event(dev, omap_crtc->pipe,
-				      omap_crtc->flip_event);
-		omap_crtc->flip_event = NULL;
-	}
+	spin_lock_irqsave(&dev->event_lock, flags);
 
-	omap_crtc->flip_state = state;
+	event = omap_crtc->event;
+	omap_crtc->event = NULL;
 
-	if (state == OMAP_PAGE_FLIP_IDLE)
+	if (event) {
+		drm_crtc_send_vblank_event(crtc, event);
 		wake_up(&omap_crtc->flip_wait);
+		drm_crtc_vblank_put(crtc);
+	}
+
+	spin_unlock_irqrestore(&dev->event_lock, flags);
 }
 
 static bool omap_crtc_page_flip_pending(struct drm_crtc *crtc)
@@ -327,7 +311,7 @@ static bool omap_crtc_page_flip_pending(struct drm_crtc *crtc)
 	bool pending;
 
 	spin_lock_irqsave(&dev->event_lock, flags);
-	pending = omap_crtc->flip_state != OMAP_PAGE_FLIP_IDLE;
+	pending = omap_crtc->event != NULL;
 	spin_unlock_irqrestore(&dev->event_lock, flags);
 
 	return pending;
@@ -336,28 +320,6 @@ static bool omap_crtc_page_flip_pending(struct drm_crtc *crtc)
 static void omap_crtc_wait_page_flip(struct drm_crtc *crtc)
 {
 	struct omap_crtc *omap_crtc = to_omap_crtc(crtc);
-	struct drm_device *dev = crtc->dev;
-	bool cancelled = false;
-	unsigned long flags;
-
-	/*
-	 * If we're still waiting for the GEM async operation to complete just
-	 * cancel the page flip, as we're holding the CRTC mutex preventing the
-	 * page flip work handler from queueing the page flip.
-	 *
-	 * We can't release the reference to the frame buffer here as the async
-	 * operation doesn't keep its own reference to the buffer. We'll just
-	 * let the page flip work queue handle that.
-	 */
-	spin_lock_irqsave(&dev->event_lock, flags);
-	if (omap_crtc->flip_state == OMAP_PAGE_FLIP_WAIT) {
-		omap_crtc_complete_page_flip(crtc, OMAP_PAGE_FLIP_CANCELLED);
-		cancelled = true;
-	}
-	spin_unlock_irqrestore(&dev->event_lock, flags);
-
-	if (cancelled)
-		return;
 
 	if (wait_event_timeout(omap_crtc->flip_wait,
 			       !omap_crtc_page_flip_pending(crtc),
@@ -366,9 +328,7 @@ static void omap_crtc_wait_page_flip(struct drm_crtc *crtc)
 
 	dev_warn(crtc->dev->dev, "page flip timeout!\n");
 
-	spin_lock_irqsave(&dev->event_lock, flags);
-	omap_crtc_complete_page_flip(crtc, OMAP_PAGE_FLIP_IDLE);
-	spin_unlock_irqrestore(&dev->event_lock, flags);
+	omap_crtc_complete_page_flip(crtc);
 }
 
 static void omap_crtc_error_irq(struct omap_drm_irq *irq, uint32_t irqstatus)
@@ -390,7 +350,6 @@ static void omap_crtc_vblank_irq(struct omap_drm_irq *irq, uint32_t irqstatus)
 	struct omap_crtc *omap_crtc =
 			container_of(irq, struct omap_crtc, vblank_irq);
 	struct drm_device *dev = omap_crtc->base.dev;
-	unsigned long flags;
 
 	if (dispc_mgr_go_busy(omap_crtc->channel))
 		return;
@@ -399,9 +358,7 @@ static void omap_crtc_vblank_irq(struct omap_drm_irq *irq, uint32_t irqstatus)
 	__omap_irq_unregister(dev, &omap_crtc->vblank_irq);
 
 	/* wakeup userspace */
-	spin_lock_irqsave(&dev->event_lock, flags);
-	omap_crtc_complete_page_flip(&omap_crtc->base, OMAP_PAGE_FLIP_IDLE);
-	spin_unlock_irqrestore(&dev->event_lock, flags);
+	omap_crtc_complete_page_flip(&omap_crtc->base);
 
 	complete(&omap_crtc->completion);
 }
@@ -594,124 +551,28 @@ static void omap_crtc_mode_set_nofb(struct drm_crtc *crtc)
 
 static void omap_crtc_atomic_begin(struct drm_crtc *crtc)
 {
-	dispc_runtime_get();
-}
-
-static void omap_crtc_atomic_flush(struct drm_crtc *crtc)
-{
-	omap_crtc_flush(crtc);
-
-	dispc_runtime_put();
-}
-
-static void page_flip_worker(struct work_struct *work)
-{
-	struct omap_crtc *omap_crtc =
-			container_of(work, struct omap_crtc, flip_work);
-	struct drm_crtc *crtc = &omap_crtc->base;
-	struct drm_display_mode *mode = &crtc->mode;
-	struct drm_device *dev = crtc->dev;
-	struct drm_framebuffer *fb;
-	struct drm_gem_object *bo;
-	unsigned long flags;
-	bool queue_flip;
-
-	drm_modeset_lock(&crtc->mutex, NULL);
-
-	spin_lock_irqsave(&dev->event_lock, flags);
-
-	/*
-	 * The page flip could have been cancelled while waiting for the GEM
-	 * async operation to complete. Don't queue the flip in that case.
-	 */
-	if (omap_crtc->flip_state == OMAP_PAGE_FLIP_WAIT) {
-		omap_crtc->flip_state = OMAP_PAGE_FLIP_QUEUED;
-		queue_flip = true;
-	} else {
-		omap_crtc->flip_state = OMAP_PAGE_FLIP_IDLE;
-		queue_flip = false;
-	}
-
-	fb = omap_crtc->flip_fb;
-	omap_crtc->flip_fb = NULL;
-
-	spin_unlock_irqrestore(&dev->event_lock, flags);
-
-	if (queue_flip) {
-		omap_plane_mode_set(crtc->primary, crtc, fb,
-				    0, 0, mode->hdisplay, mode->vdisplay,
-				    crtc->x, crtc->y, mode->hdisplay,
-				    mode->vdisplay);
-		omap_crtc_flush(crtc);
-	}
-
-	drm_modeset_unlock(&crtc->mutex);
-
-	bo = omap_framebuffer_bo(fb, 0);
-	drm_gem_object_unreference_unlocked(bo);
-	drm_framebuffer_unreference(fb);
-}
-
-static void page_flip_cb(void *arg)
-{
-	struct drm_crtc *crtc = arg;
+	struct drm_pending_vblank_event *event = crtc->state->event;
 	struct omap_crtc *omap_crtc = to_omap_crtc(crtc);
-	struct omap_drm_private *priv = crtc->dev->dev_private;
-
-	/* avoid assumptions about what ctxt we are called from: */
-	queue_work(priv->wq, &omap_crtc->flip_work);
-}
-
-static int omap_crtc_page_flip(struct drm_crtc *crtc,
-			       struct drm_framebuffer *fb,
-			       struct drm_pending_vblank_event *event,
-			       uint32_t page_flip_flags)
-{
 	struct drm_device *dev = crtc->dev;
-	struct omap_crtc *omap_crtc = to_omap_crtc(crtc);
-	struct drm_plane *primary = crtc->primary;
-	struct drm_gem_object *bo;
 	unsigned long flags;
 
-	DBG("%d -> %d (event=%p)", primary->fb ? primary->fb->base.id : -1,
-			fb->base.id, event);
+	dispc_runtime_get();
 
-	spin_lock_irqsave(&dev->event_lock, flags);
+	if (event) {
+		WARN_ON(omap_crtc->event);
+		WARN_ON(drm_crtc_vblank_get(crtc) != 0);
 
-	if (omap_crtc->flip_state != OMAP_PAGE_FLIP_IDLE) {
+		spin_lock_irqsave(&dev->event_lock, flags);
+		omap_crtc->event = event;
 		spin_unlock_irqrestore(&dev->event_lock, flags);
-		dev_err(dev->dev, "already a pending flip\n");
-		return -EBUSY;
 	}
+}
 
-	/*
-	 * Store a reference to the framebuffer queued for page flip in the CRTC
-	 * private structure. We can't rely on crtc->primary->fb in the page
-	 * flip worker, as a racing CRTC disable (due for instance to an
-	 * explicit framebuffer deletion from userspace) would set that field to
-	 * NULL before the worker gets a change to run.
-	 */
-	drm_framebuffer_reference(fb);
-	omap_crtc->flip_fb = fb;
-	omap_crtc->flip_event = event;
-	omap_crtc->flip_state = OMAP_PAGE_FLIP_WAIT;
-
-	drm_atomic_set_fb_for_plane(primary->state, fb);
-	primary->fb = fb;
-
-	spin_unlock_irqrestore(&dev->event_lock, flags);
-
-	/*
-	 * Hold a reference temporarily until the crtc is updated
-	 * and takes the reference to the bo.  This avoids it
-	 * getting freed from under us:
-	 */
-	bo = omap_framebuffer_bo(fb, 0);
-	drm_gem_object_reference(bo);
-
-	omap_gem_op_async(bo, OMAP_GEM_READ, page_flip_cb, crtc);
+static void omap_crtc_atomic_flush(struct drm_crtc *crtc)
+{
+	omap_crtc_flush(crtc);
 
-	return 0;
+	dispc_runtime_put();
 }
 
 static int omap_crtc_set_property(struct drm_crtc *crtc,
@@ -729,7 +590,7 @@ static const struct drm_crtc_funcs omap_crtc_funcs = {
 	.reset = drm_atomic_helper_crtc_reset,
 	.set_config = drm_atomic_helper_set_config,
 	.destroy = omap_crtc_destroy,
-	.page_flip = omap_crtc_page_flip,
+	.page_flip = drm_atomic_helper_page_flip,
 	.set_property = omap_crtc_set_property,
 	.atomic_duplicate_state = drm_atomic_helper_crtc_duplicate_state,
 	.atomic_destroy_state = drm_atomic_helper_crtc_destroy_state,
@@ -782,7 +643,6 @@ struct drm_crtc *omap_crtc_init(struct drm_device *dev,
 
 	crtc = &omap_crtc->base;
 
-	INIT_WORK(&omap_crtc->flip_work, page_flip_worker);
 	init_waitqueue_head(&omap_crtc->flip_wait);
 
 	INIT_LIST_HEAD(&omap_crtc->pending_unpins);
diff --git a/drivers/gpu/drm/omapdrm/omap_drv.h b/drivers/gpu/drm/omapdrm/omap_drv.h
index a4737c0..54aff0b 100644
--- a/drivers/gpu/drm/omapdrm/omap_drv.h
+++ b/drivers/gpu/drm/omapdrm/omap_drv.h
@@ -152,12 +152,6 @@ struct drm_crtc *omap_crtc_init(struct drm_device *dev,
 struct drm_plane *omap_plane_init(struct drm_device *dev,
 		int id, enum drm_plane_type type);
 int omap_plane_set_enable(struct drm_plane *plane, bool enable);
-int omap_plane_mode_set(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,
-			unsigned int src_x, unsigned int src_y,
-			unsigned int src_w, unsigned int src_h);
 void omap_plane_install_properties(struct drm_plane *plane,
 		struct drm_mode_object *obj);
 int omap_plane_set_property(struct drm_plane *plane,
diff --git a/drivers/gpu/drm/omapdrm/omap_plane.c b/drivers/gpu/drm/omapdrm/omap_plane.c
index fcc5d96..117fa37 100644
--- a/drivers/gpu/drm/omapdrm/omap_plane.c
+++ b/drivers/gpu/drm/omapdrm/omap_plane.c
@@ -144,29 +144,6 @@ static int omap_plane_setup(struct omap_plane *omap_plane)
 	return ret;
 }
 
-int omap_plane_mode_set(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,
-			unsigned int src_x, unsigned int src_y,
-			unsigned int src_w, unsigned int src_h)
-{
-	struct omap_plane *omap_plane = to_omap_plane(plane);
-	struct omap_drm_window *win = &omap_plane->win;
-
-	win->crtc_x = crtc_x;
-	win->crtc_y = crtc_y;
-	win->crtc_w = crtc_w;
-	win->crtc_h = crtc_h;
-
-	win->src_x = src_x;
-	win->src_y = src_y;
-	win->src_w = src_w;
-	win->src_h = src_h;
-
-	return omap_plane_setup(omap_plane);
-}
-
 int omap_plane_set_enable(struct drm_plane *plane, bool enable)
 {
 	struct omap_plane *omap_plane = to_omap_plane(plane);
-- 
2.0.5

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

[PATCH 23/31] drm: omapdrm: Drop manual framebuffer pin handling

[Laurent Pinchart]

Since the removal of omap_plane_mode_set(), framebuffers are now pinned
exclusively through the plane .prepare_fb() and .cleanup_fb() operations
as the remaining callers of omap_plane_setup() don't modify the
framebuffer. Remove the manual pin/unpin infrastructure.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 drivers/gpu/drm/omapdrm/omap_crtc.c  | 35 ------------------------------
 drivers/gpu/drm/omapdrm/omap_drv.h   |  1 -
 drivers/gpu/drm/omapdrm/omap_plane.c | 41 ------------------------------------
 3 files changed, 77 deletions(-)

diff --git a/drivers/gpu/drm/omapdrm/omap_crtc.c b/drivers/gpu/drm/omapdrm/omap_crtc.c
index d82b8ea..516da56 100644
--- a/drivers/gpu/drm/omapdrm/omap_crtc.c
+++ b/drivers/gpu/drm/omapdrm/omap_crtc.c
@@ -53,9 +53,6 @@ struct omap_crtc {
 	struct omap_drm_irq vblank_irq;
 	struct omap_drm_irq error_irq;
 
-	/* list of framebuffers to unpin */
-	struct list_head pending_unpins;
-
 	/* pending event */
 	struct drm_pending_vblank_event *event;
 	wait_queue_head_t flip_wait;
@@ -65,11 +62,6 @@ struct omap_crtc {
 	bool ignore_digit_sync_lost;
 };
 
-struct omap_framebuffer_unpin {
-	struct list_head list;
-	struct drm_framebuffer *fb;
-};
-
 /* -----------------------------------------------------------------------------
  * Helper Functions
  */
@@ -366,7 +358,6 @@ static void omap_crtc_vblank_irq(struct omap_drm_irq *irq, uint32_t irqstatus)
 int omap_crtc_flush(struct drm_crtc *crtc)
 {
 	struct omap_crtc *omap_crtc = to_omap_crtc(crtc);
-	struct omap_framebuffer_unpin *fb, *next;
 
 	DBG("%s: GO", omap_crtc->name);
 
@@ -386,29 +377,6 @@ int omap_crtc_flush(struct drm_crtc *crtc)
 
 	dispc_runtime_put();
 
-	/* Unpin and unreference pending framebuffers. */
-	list_for_each_entry_safe(fb, next, &omap_crtc->pending_unpins, list) {
-		omap_framebuffer_unpin(fb->fb);
-		drm_framebuffer_unreference(fb->fb);
-		list_del(&fb->list);
-		kfree(fb);
-	}
-
-	return 0;
-}
-
-int omap_crtc_queue_unpin(struct drm_crtc *crtc, struct drm_framebuffer *fb)
-{
-	struct omap_crtc *omap_crtc = to_omap_crtc(crtc);
-	struct omap_framebuffer_unpin *unpin;
-
-	unpin = kzalloc(sizeof(*unpin), GFP_KERNEL);
-	if (!unpin)
-		return -ENOMEM;
-
-	unpin->fb = fb;
-	list_add_tail(&unpin->list, &omap_crtc->pending_unpins);
-
 	return 0;
 }
 
@@ -644,9 +612,6 @@ struct drm_crtc *omap_crtc_init(struct drm_device *dev,
 	crtc = &omap_crtc->base;
 
 	init_waitqueue_head(&omap_crtc->flip_wait);
-
-	INIT_LIST_HEAD(&omap_crtc->pending_unpins);
-
 	init_completion(&omap_crtc->completion);
 
 	omap_crtc->channel = channel;
diff --git a/drivers/gpu/drm/omapdrm/omap_drv.h b/drivers/gpu/drm/omapdrm/omap_drv.h
index 54aff0b..db973b0 100644
--- a/drivers/gpu/drm/omapdrm/omap_drv.h
+++ b/drivers/gpu/drm/omapdrm/omap_drv.h
@@ -142,7 +142,6 @@ void omap_fbdev_free(struct drm_device *dev);
 const struct omap_video_timings *omap_crtc_timings(struct drm_crtc *crtc);
 enum omap_channel omap_crtc_channel(struct drm_crtc *crtc);
 int omap_crtc_flush(struct drm_crtc *crtc);
-int omap_crtc_queue_unpin(struct drm_crtc *crtc, struct drm_framebuffer *fb);
 void omap_crtc_cancel_page_flip(struct drm_crtc *crtc, struct drm_file *file);
 void omap_crtc_pre_init(void);
 void omap_crtc_pre_uninit(void);
diff --git a/drivers/gpu/drm/omapdrm/omap_plane.c b/drivers/gpu/drm/omapdrm/omap_plane.c
index 117fa37..7813e48 100644
--- a/drivers/gpu/drm/omapdrm/omap_plane.c
+++ b/drivers/gpu/drm/omapdrm/omap_plane.c
@@ -45,49 +45,12 @@ struct omap_plane {
 	struct omap_drm_window win;
 	bool enabled;
 
-	/* last fb that we pinned: */
-	struct drm_framebuffer *pinned_fb;
-
 	uint32_t nformats;
 	uint32_t formats[32];
 
 	struct omap_drm_irq error_irq;
 };
 
-/* update which fb (if any) is pinned for scanout */
-static int omap_plane_update_pin(struct drm_plane *plane)
-{
-	struct omap_plane *omap_plane = to_omap_plane(plane);
-	struct drm_framebuffer *pinned_fb = omap_plane->pinned_fb;
-	struct drm_framebuffer *fb = omap_plane->enabled ? plane->fb : NULL;
-	int ret = 0;
-
-	if (pinned_fb == fb)
-		return 0;
-
-	DBG("%p -> %p", pinned_fb, fb);
-
-	if (fb) {
-		drm_framebuffer_reference(fb);
-		ret = omap_framebuffer_pin(fb);
-	}
-
-	if (pinned_fb)
-		omap_crtc_queue_unpin(plane->crtc, pinned_fb);
-
-	if (ret) {
-		dev_err(plane->dev->dev, "could not swap %p -> %p\n",
-				omap_plane->pinned_fb, fb);
-		drm_framebuffer_unreference(fb);
-		omap_plane->pinned_fb = NULL;
-		return ret;
-	}
-
-	omap_plane->pinned_fb = fb;
-
-	return 0;
-}
-
 static int __omap_plane_setup(struct omap_plane *omap_plane,
 			      struct drm_crtc *crtc,
 			      struct drm_framebuffer *fb)
@@ -133,10 +96,6 @@ static int omap_plane_setup(struct omap_plane *omap_plane)
 	struct drm_plane *plane = &omap_plane->base;
 	int ret;
 
-	ret = omap_plane_update_pin(plane);
-	if (ret < 0)
-		return ret;
-
 	dispc_runtime_get();
 	ret = __omap_plane_setup(omap_plane, plane->crtc, plane->fb);
 	dispc_runtime_put();
-- 
2.0.5

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

[PATCH 24/31] drm: omapdrm: Switch crtc and plane set_property to atomic helpers

[Laurent Pinchart]

Allow setting up plane properties atomically using the plane
set_property atomic helper. The properties are now stored in the plane
state (requiring subclassing it) and applied when updating the planes.

The CRTC exposes the properties of its primary plane for legacy reason.
We can't get rid of that API, so simply delegate it to the primary
plane.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 drivers/gpu/drm/omapdrm/omap_crtc.c  |  45 +++++++--
 drivers/gpu/drm/omapdrm/omap_drv.h   |   2 -
 drivers/gpu/drm/omapdrm/omap_plane.c | 179 +++++++++++++++++++++++------------
 3 files changed, 155 insertions(+), 71 deletions(-)

diff --git a/drivers/gpu/drm/omapdrm/omap_crtc.c b/drivers/gpu/drm/omapdrm/omap_crtc.c
index 516da56..9fc7364 100644
--- a/drivers/gpu/drm/omapdrm/omap_crtc.c
+++ b/drivers/gpu/drm/omapdrm/omap_crtc.c
@@ -541,17 +541,44 @@ static void omap_crtc_atomic_flush(struct drm_crtc *crtc)
 	omap_crtc_flush(crtc);
 
 	dispc_runtime_put();
+
+	crtc->invert_dimensions = !!(crtc->primary->state->rotation &
+				    (BIT(DRM_ROTATE_90) | BIT(DRM_ROTATE_270)));
 }
 
-static int omap_crtc_set_property(struct drm_crtc *crtc,
-		struct drm_property *property, uint64_t val)
+static int omap_crtc_atomic_set_property(struct drm_crtc *crtc,
+					 struct drm_crtc_state *state,
+					 struct drm_property *property,
+					 uint64_t val)
 {
-	if (property == crtc->dev->mode_config.rotation_property) {
-		crtc->invert_dimensions =
-				!!(val & ((1LL << DRM_ROTATE_90) | (1LL << DRM_ROTATE_270)));
-	}
+	struct drm_plane_state *plane_state;
+	struct drm_plane *plane = crtc->primary;
+
+	/*
+	 * Delegate property set to the primary plane. Get the plane state and
+	 * set the property directly.
+	 */
+
+	plane_state = drm_atomic_get_plane_state(state->state, plane);
+	if (!plane_state)
+		return -EINVAL;
+
+	return drm_atomic_plane_set_property(plane, plane_state, property, val);
+}
 
-	return omap_plane_set_property(crtc->primary, property, val);
+static int omap_crtc_atomic_get_property(struct drm_crtc *crtc,
+					 const struct drm_crtc_state *state,
+					 struct drm_property *property,
+					 uint64_t *val)
+{
+	/*
+	 * Delegate property get to the primary plane. The
+	 * drm_atomic_plane_get_property() function isn't exported, but can be
+	 * called through drm_object_property_get_value() as that will call
+	 * drm_atomic_get_property() for atomic drivers.
+	 */
+	return drm_object_property_get_value(&crtc->primary->base, property,
+					     val);
 }
 
 static const struct drm_crtc_funcs omap_crtc_funcs = {
@@ -559,9 +586,11 @@ static const struct drm_crtc_funcs omap_crtc_funcs = {
 	.set_config = drm_atomic_helper_set_config,
 	.destroy = omap_crtc_destroy,
 	.page_flip = drm_atomic_helper_page_flip,
-	.set_property = omap_crtc_set_property,
+	.set_property = drm_atomic_helper_crtc_set_property,
 	.atomic_duplicate_state = drm_atomic_helper_crtc_duplicate_state,
 	.atomic_destroy_state = drm_atomic_helper_crtc_destroy_state,
+	.atomic_set_property = omap_crtc_atomic_set_property,
+	.atomic_get_property = omap_crtc_atomic_get_property,
 };
 
 static const struct drm_crtc_helper_funcs omap_crtc_helper_funcs = {
diff --git a/drivers/gpu/drm/omapdrm/omap_drv.h b/drivers/gpu/drm/omapdrm/omap_drv.h
index db973b0..bef3002 100644
--- a/drivers/gpu/drm/omapdrm/omap_drv.h
+++ b/drivers/gpu/drm/omapdrm/omap_drv.h
@@ -153,8 +153,6 @@ struct drm_plane *omap_plane_init(struct drm_device *dev,
 int omap_plane_set_enable(struct drm_plane *plane, bool enable);
 void omap_plane_install_properties(struct drm_plane *plane,
 		struct drm_mode_object *obj);
-int omap_plane_set_property(struct drm_plane *plane,
-		struct drm_property *property, uint64_t val);
 
 struct drm_encoder *omap_encoder_init(struct drm_device *dev,
 		struct omap_dss_device *dssdev);
diff --git a/drivers/gpu/drm/omapdrm/omap_plane.c b/drivers/gpu/drm/omapdrm/omap_plane.c
index 7813e48..0d93468 100644
--- a/drivers/gpu/drm/omapdrm/omap_plane.c
+++ b/drivers/gpu/drm/omapdrm/omap_plane.c
@@ -51,10 +51,22 @@ struct omap_plane {
 	struct omap_drm_irq error_irq;
 };
 
-static int __omap_plane_setup(struct omap_plane *omap_plane,
-			      struct drm_crtc *crtc,
-			      struct drm_framebuffer *fb)
+struct omap_plane_state {
+	struct drm_plane_state base;
+
+	unsigned int zorder;
+};
+
+static inline struct omap_plane_state *
+to_omap_plane_state(struct drm_plane_state *state)
 {
+	return container_of(state, struct omap_plane_state, base);
+}
+
+static int omap_plane_setup(struct omap_plane *omap_plane)
+{
+	struct drm_plane_state *state = omap_plane->base.state;
+	struct omap_plane_state *omap_state = to_omap_plane_state(state);
 	struct omap_overlay_info *info = &omap_plane->info;
 	struct drm_device *dev = omap_plane->base.dev;
 	int ret;
@@ -66,8 +78,10 @@ static int __omap_plane_setup(struct omap_plane *omap_plane,
 		return 0;
 	}
 
+	info->zorder = omap_state->zorder;
+
 	/* update scanout: */
-	omap_framebuffer_update_scanout(fb, &omap_plane->win, info);
+	omap_framebuffer_update_scanout(state->fb, &omap_plane->win, info);
 
 	DBG("%dx%d -> %dx%d (%d)", info->width, info->height,
 			info->out_width, info->out_height,
@@ -76,11 +90,11 @@ static int __omap_plane_setup(struct omap_plane *omap_plane,
 			&info->paddr, &info->p_uv_addr);
 
 	dispc_ovl_set_channel_out(omap_plane->id,
-				  omap_crtc_channel(crtc));
+				  omap_crtc_channel(state->crtc));
 
 	/* and finally, update omapdss: */
 	ret = dispc_ovl_setup(omap_plane->id, info, false,
-			      omap_crtc_timings(crtc), false);
+			      omap_crtc_timings(state->crtc), false);
 	if (ret) {
 		dev_err(dev->dev, "dispc_ovl_setup failed: %d\n", ret);
 		return ret;
@@ -91,27 +105,21 @@ static int __omap_plane_setup(struct omap_plane *omap_plane,
 	return 0;
 }
 
-static int omap_plane_setup(struct omap_plane *omap_plane)
-{
-	struct drm_plane *plane = &omap_plane->base;
-	int ret;
-
-	dispc_runtime_get();
-	ret = __omap_plane_setup(omap_plane, plane->crtc, plane->fb);
-	dispc_runtime_put();
-
-	return ret;
-}
-
 int omap_plane_set_enable(struct drm_plane *plane, bool enable)
 {
 	struct omap_plane *omap_plane = to_omap_plane(plane);
+	int ret;
 
 	if (enable == omap_plane->enabled)
 		return 0;
 
 	omap_plane->enabled = enable;
-	return omap_plane_setup(omap_plane);
+
+	dispc_runtime_get();
+	ret = omap_plane_setup(omap_plane);
+	dispc_runtime_put();
+
+	return ret;
 }
 
 static int omap_plane_prepare_fb(struct drm_plane *plane,
@@ -140,8 +148,10 @@ static void omap_plane_atomic_update(struct drm_plane *plane,
 	if (!state->fb || !state->crtc)
 		return;
 
+	win->rotation = state->rotation;
+
 	/* omap_framebuffer_update_scanout() takes adjusted src */
-	switch (omap_plane->win.rotation & 0xf) {
+	switch (state->rotation & 0xf) {
 	case BIT(DRM_ROTATE_90):
 	case BIT(DRM_ROTATE_270):
 		src_w = state->src_h;
@@ -165,23 +175,24 @@ static void omap_plane_atomic_update(struct drm_plane *plane,
 	win->src_h = src_h >> 16;
 
 	omap_plane->enabled = true;
-	__omap_plane_setup(omap_plane, state->crtc, state->fb);
+	omap_plane_setup(omap_plane);
 }
 
 static void omap_plane_atomic_disable(struct drm_plane *plane,
 				      struct drm_plane_state *old_state)
 {
+	struct omap_plane_state *omap_state = to_omap_plane_state(plane->state);
 	struct omap_plane *omap_plane = to_omap_plane(plane);
 
-	omap_plane->win.rotation = BIT(DRM_ROTATE_0);
-	omap_plane->info.zorder = plane->type == DRM_PLANE_TYPE_PRIMARY
-				? 0 : omap_plane->id;
+	plane->state->rotation = BIT(DRM_ROTATE_0);
+	omap_state->zorder = plane->type == DRM_PLANE_TYPE_PRIMARY
+			   ? 0 : omap_plane->id;
 
 	if (!omap_plane->enabled)
 		return;
 
 	omap_plane->enabled = false;
-	__omap_plane_setup(omap_plane, NULL, NULL);
+	omap_plane_setup(omap_plane);
 }
 
 static const struct drm_plane_helper_funcs omap_plane_helper_funcs = {
@@ -191,6 +202,33 @@ static const struct drm_plane_helper_funcs omap_plane_helper_funcs = {
 	.atomic_disable = omap_plane_atomic_disable,
 };
 
+static void omap_plane_reset(struct drm_plane *plane)
+{
+	struct omap_plane *omap_plane = to_omap_plane(plane);
+	struct omap_plane_state *omap_state;
+
+	if (plane->state && plane->state->fb)
+		drm_framebuffer_unreference(plane->state->fb);
+
+	kfree(plane->state);
+	plane->state = NULL;
+
+	omap_state = kzalloc(sizeof(*omap_state), GFP_KERNEL);
+	if (omap_state == NULL)
+		return;
+
+	/*
+	 * Set defaults depending on whether we are a primary or overlay
+	 * plane.
+	 */
+	omap_state->zorder = plane->type == DRM_PLANE_TYPE_PRIMARY
+			   ? 0 : omap_plane->id;
+	omap_state->base.rotation = BIT(DRM_ROTATE_0);
+
+	plane->state = &omap_state->base;
+	plane->state->plane = plane;
+}
+
 static void omap_plane_destroy(struct drm_plane *plane)
 {
 	struct omap_plane *omap_plane = to_omap_plane(plane);
@@ -220,45 +258,74 @@ void omap_plane_install_properties(struct drm_plane *plane,
 	drm_object_attach_property(obj, priv->zorder_prop, 0);
 }
 
-int omap_plane_set_property(struct drm_plane *plane,
-		struct drm_property *property, uint64_t val)
+static struct drm_plane_state *
+omap_plane_atomic_duplicate_state(struct drm_plane *plane)
+{
+	struct omap_plane_state *omap_state;
+
+	omap_state = kmemdup(to_omap_plane_state(plane->state),
+			      sizeof(*omap_state), GFP_KERNEL);
+	if (!omap_state)
+		return NULL;
+
+	if (omap_state->base.fb)
+		drm_framebuffer_reference(omap_state->base.fb);
+
+	return &omap_state->base;
+}
+
+static void omap_plane_atomic_destroy_state(struct drm_plane *plane,
+					    struct drm_plane_state *state)
+{
+	if (state->fb)
+		drm_framebuffer_unreference(state->fb);
+
+	kfree(to_omap_plane_state(state));
+}
+
+static int omap_plane_atomic_set_property(struct drm_plane *plane,
+					  struct drm_plane_state *state,
+					  struct drm_property *property,
+					  uint64_t val)
 {
-	struct omap_plane *omap_plane = to_omap_plane(plane);
 	struct omap_drm_private *priv = plane->dev->dev_private;
-	int ret;
+	struct omap_plane_state *omap_state = to_omap_plane_state(state);
 
-	if (property == plane->dev->mode_config.rotation_property) {
-		DBG("%s: rotation: %02x", omap_plane->name, (uint32_t)val);
-		omap_plane->win.rotation = val;
-	} else if (property == priv->zorder_prop) {
-		DBG("%s: zorder: %02x", omap_plane->name, (uint32_t)val);
-		omap_plane->info.zorder = val;
-	} else {
+	if (property == priv->zorder_prop)
+		omap_state->zorder = val;
+	else
 		return -EINVAL;
-	}
 
-	/*
-	 * We're done if the plane is disabled, properties will be applied the
-	 * next time it becomes enabled.
-	 */
-	if (!omap_plane->enabled)
-		return 0;
+	return 0;
+}
 
-	ret = omap_plane_setup(omap_plane);
-	if (ret < 0)
-		return ret;
+static int omap_plane_atomic_get_property(struct drm_plane *plane,
+					  const struct drm_plane_state *state,
+					  struct drm_property *property,
+					  uint64_t *val)
+{
+	struct omap_drm_private *priv = plane->dev->dev_private;
+	const struct omap_plane_state *omap_state =
+		container_of(state, const struct omap_plane_state, base);
+
+	if (property == priv->zorder_prop)
+		*val = omap_state->zorder;
+	else
+		return -EINVAL;
 
-	return omap_crtc_flush(plane->crtc);
+	return 0;
 }
 
 static const struct drm_plane_funcs omap_plane_funcs = {
 	.update_plane = drm_atomic_helper_update_plane,
 	.disable_plane = drm_atomic_helper_disable_plane,
-	.reset = drm_atomic_helper_plane_reset,
+	.reset = omap_plane_reset,
 	.destroy = omap_plane_destroy,
-	.set_property = omap_plane_set_property,
-	.atomic_duplicate_state = drm_atomic_helper_plane_duplicate_state,
-	.atomic_destroy_state = drm_atomic_helper_plane_destroy_state,
+	.set_property = drm_atomic_helper_plane_set_property,
+	.atomic_duplicate_state = omap_plane_atomic_duplicate_state,
+	.atomic_destroy_state = omap_plane_atomic_destroy_state,
+	.atomic_set_property = omap_plane_atomic_set_property,
+	.atomic_get_property = omap_plane_atomic_get_property,
 };
 
 static void omap_plane_error_irq(struct omap_drm_irq *irq, uint32_t irqstatus)
@@ -330,16 +397,6 @@ struct drm_plane *omap_plane_init(struct drm_device *dev,
 	info->global_alpha = 0xff;
 	info->mirror = 0;
 
-	/* Set defaults depending on whether we are a CRTC or overlay
-	 * layer.
-	 * TODO add ioctl to give userspace an API to change this.. this
-	 * will come in a subsequent patch.
-	 */
-	if (type == DRM_PLANE_TYPE_PRIMARY)
-		omap_plane->info.zorder = 0;
-	else
-		omap_plane->info.zorder = id;
-
 	return plane;
 
 error:
-- 
2.0.5

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

[PATCH 25/31] drm: omapdrm: Move plane info and win out of the plane structure

[Laurent Pinchart]

The plane info and win structures are only used to setup the plane
through the DSS API. Move them from the plane structure to local
variables in omap_plane_setup().

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 drivers/gpu/drm/omapdrm/omap_plane.c | 92 ++++++++++++++++--------------------
 1 file changed, 41 insertions(+), 51 deletions(-)

diff --git a/drivers/gpu/drm/omapdrm/omap_plane.c b/drivers/gpu/drm/omapdrm/omap_plane.c
index 0d93468..ac6b72c 100644
--- a/drivers/gpu/drm/omapdrm/omap_plane.c
+++ b/drivers/gpu/drm/omapdrm/omap_plane.c
@@ -39,10 +39,7 @@ struct omap_plane {
 	struct drm_plane base;
 	int id;  /* TODO rename omap_plane -> omap_plane_id in omapdss so I can use the enum */
 	const char *name;
-	struct omap_overlay_info info;
 
-	/* position/orientation of scanout within the fb: */
-	struct omap_drm_window win;
 	bool enabled;
 
 	uint32_t nformats;
@@ -67,8 +64,9 @@ static int omap_plane_setup(struct omap_plane *omap_plane)
 {
 	struct drm_plane_state *state = omap_plane->base.state;
 	struct omap_plane_state *omap_state = to_omap_plane_state(state);
-	struct omap_overlay_info *info = &omap_plane->info;
 	struct drm_device *dev = omap_plane->base.dev;
+	struct omap_overlay_info info;
+	struct omap_drm_window win;
 	int ret;
 
 	DBG("%s, enabled=%d", omap_plane->name, omap_plane->enabled);
@@ -78,22 +76,53 @@ static int omap_plane_setup(struct omap_plane *omap_plane)
 		return 0;
 	}
 
-	info->zorder = omap_state->zorder;
+	memset(&info, 0, sizeof(info));
+	info.rotation_type = OMAP_DSS_ROT_DMA;
+	info.rotation = OMAP_DSS_ROT_0;
+	info.global_alpha = 0xff;
+	info.mirror = 0;
+	info.zorder = omap_state->zorder;
+
+	memset(&win, 0, sizeof(win));
+	win.rotation = state->rotation;
+	win.crtc_x = state->crtc_x;
+	win.crtc_y = state->crtc_y;
+	win.crtc_w = state->crtc_w;
+	win.crtc_h = state->crtc_h;
+
+	/*
+	 * src values are in Q16 fixed point, convert to integer.
+	 * omap_framebuffer_update_scanout() takes adjusted src.
+	 */
+	win.src_x = state->src_x >> 16;
+	win.src_y = state->src_y >> 16;
+
+	switch (state->rotation & 0xf) {
+	case BIT(DRM_ROTATE_90):
+	case BIT(DRM_ROTATE_270):
+		win.src_w = state->src_h >> 16;
+		win.src_h = state->src_w >> 16;
+		break;
+	default:
+		win.src_w = state->src_w >> 16;
+		win.src_h = state->src_h >> 16;
+		break;
+	}
 
 	/* update scanout: */
-	omap_framebuffer_update_scanout(state->fb, &omap_plane->win, info);
+	omap_framebuffer_update_scanout(state->fb, &win, &info);
 
-	DBG("%dx%d -> %dx%d (%d)", info->width, info->height,
-			info->out_width, info->out_height,
-			info->screen_width);
-	DBG("%d,%d %pad %pad", info->pos_x, info->pos_y,
-			&info->paddr, &info->p_uv_addr);
+	DBG("%dx%d -> %dx%d (%d)", info.width, info.height,
+			info.out_width, info.out_height,
+			info.screen_width);
+	DBG("%d,%d %pad %pad", info.pos_x, info.pos_y,
+			&info.paddr, &info.p_uv_addr);
 
 	dispc_ovl_set_channel_out(omap_plane->id,
 				  omap_crtc_channel(state->crtc));
 
 	/* and finally, update omapdss: */
-	ret = dispc_ovl_setup(omap_plane->id, info, false,
+	ret = dispc_ovl_setup(omap_plane->id, &info, false,
 			      omap_crtc_timings(state->crtc), false);
 	if (ret) {
 		dev_err(dev->dev, "dispc_ovl_setup failed: %d\n", ret);
@@ -140,40 +169,11 @@ static void omap_plane_atomic_update(struct drm_plane *plane,
 				     struct drm_plane_state *old_state)
 {
 	struct omap_plane *omap_plane = to_omap_plane(plane);
-	struct omap_drm_window *win = &omap_plane->win;
 	struct drm_plane_state *state = plane->state;
-	uint32_t src_w;
-	uint32_t src_h;
 
 	if (!state->fb || !state->crtc)
 		return;
 
-	win->rotation = state->rotation;
-
-	/* omap_framebuffer_update_scanout() takes adjusted src */
-	switch (state->rotation & 0xf) {
-	case BIT(DRM_ROTATE_90):
-	case BIT(DRM_ROTATE_270):
-		src_w = state->src_h;
-		src_h = state->src_w;
-		break;
-	default:
-		src_w = state->src_w;
-		src_h = state->src_h;
-		break;
-	}
-
-	/* src values are in Q16 fixed point, convert to integer. */
-	win->crtc_x = state->crtc_x;
-	win->crtc_y = state->crtc_y;
-	win->crtc_w = state->crtc_w;
-	win->crtc_h = state->crtc_h;
-
-	win->src_x = state->src_x >> 16;
-	win->src_y = state->src_y >> 16;
-	win->src_w = src_w >> 16;
-	win->src_h = src_h >> 16;
-
 	omap_plane->enabled = true;
 	omap_plane_setup(omap_plane);
 }
@@ -357,7 +357,6 @@ struct drm_plane *omap_plane_init(struct drm_device *dev,
 	struct omap_drm_private *priv = dev->dev_private;
 	struct drm_plane *plane;
 	struct omap_plane *omap_plane;
-	struct omap_overlay_info *info;
 	int ret;
 
 	DBG("%s: type=%d", plane_names[id], type);
@@ -388,15 +387,6 @@ struct drm_plane *omap_plane_init(struct drm_device *dev,
 
 	omap_plane_install_properties(plane, &plane->base);
 
-	/* get our starting configuration, set defaults for parameters
-	 * we don't currently use, etc:
-	 */
-	info = &omap_plane->info;
-	info->rotation_type = OMAP_DSS_ROT_DMA;
-	info->rotation = OMAP_DSS_ROT_0;
-	info->global_alpha = 0xff;
-	info->mirror = 0;
-
 	return plane;
 
 error:
-- 
2.0.5

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

[PATCH 26/31] drm: omapdrm: Move crtc info out of the crtc structure

[Laurent Pinchart]

The crtc info structure is only used to setup the crtc through the DSS
API. Move it from the crtc structure to local variables in
omap_crtc_dss_enable().

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 drivers/gpu/drm/omapdrm/omap_crtc.c | 18 ++++++++----------
 1 file changed, 8 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/omapdrm/omap_crtc.c b/drivers/gpu/drm/omapdrm/omap_crtc.c
index 9fc7364..2986a60 100644
--- a/drivers/gpu/drm/omapdrm/omap_crtc.c
+++ b/drivers/gpu/drm/omapdrm/omap_crtc.c
@@ -36,7 +36,6 @@ struct omap_crtc {
 	const char *name;
 	int pipe;
 	enum omap_channel channel;
-	struct omap_overlay_manager_info info;
 	struct drm_encoder *current_encoder;
 
 	/*
@@ -189,8 +188,15 @@ static void omap_crtc_set_enabled(struct drm_crtc *crtc, bool enable)
 static int omap_crtc_dss_enable(struct omap_overlay_manager *mgr)
 {
 	struct omap_crtc *omap_crtc = omap_crtcs[mgr->id];
+	struct omap_overlay_manager_info info;
 
-	dispc_mgr_setup(omap_crtc->channel, &omap_crtc->info);
+	memset(&info, 0, sizeof(info));
+	info.default_color = 0x00000000;
+	info.trans_key = 0x00000000;
+	info.trans_key_type = OMAP_DSS_COLOR_KEY_GFX_DST;
+	info.trans_enabled = false;
+
+	dispc_mgr_setup(omap_crtc->channel, &info);
 	dispc_mgr_set_timings(omap_crtc->channel,
 			&omap_crtc->timings);
 	omap_crtc_set_enabled(&omap_crtc->base, true);
@@ -629,7 +635,6 @@ struct drm_crtc *omap_crtc_init(struct drm_device *dev,
 {
 	struct drm_crtc *crtc = NULL;
 	struct omap_crtc *omap_crtc;
-	struct omap_overlay_manager_info *info;
 	int ret;
 
 	DBG("%s", channel_names[channel]);
@@ -658,13 +663,6 @@ struct drm_crtc *omap_crtc_init(struct drm_device *dev,
 	/* temporary: */
 	omap_crtc->mgr = omap_dss_get_overlay_manager(channel);
 
-	/* TODO: fix hard-coded setup.. add properties! */
-	info = &omap_crtc->info;
-	info->default_color = 0x00000000;
-	info->trans_key = 0x00000000;
-	info->trans_key_type = OMAP_DSS_COLOR_KEY_GFX_DST;
-	info->trans_enabled = false;
-
 	ret = drm_crtc_init_with_planes(dev, crtc, plane, NULL,
 					&omap_crtc_funcs);
 	if (ret < 0) {
-- 
2.0.5

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

[PATCH 27/31] drm: omapdrm: Remove omap_crtc enabled field

[Laurent Pinchart]

The field tracks the CRTC state to avoid double-enable or -disable. As
the DRM atomic core guarantees that the CRTC enable and disable
functions won't be called on an already enabled or disabled CRTC, such
tracking isn't needed. Remove the enabled field.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 drivers/gpu/drm/omapdrm/omap_crtc.c | 32 +++++++++-----------------------
 1 file changed, 9 insertions(+), 23 deletions(-)

diff --git a/drivers/gpu/drm/omapdrm/omap_crtc.c b/drivers/gpu/drm/omapdrm/omap_crtc.c
index 2986a60..d02b54d 100644
--- a/drivers/gpu/drm/omapdrm/omap_crtc.c
+++ b/drivers/gpu/drm/omapdrm/omap_crtc.c
@@ -47,7 +47,6 @@ struct omap_crtc {
 	struct omap_overlay_manager *mgr;
 
 	struct omap_video_timings timings;
-	bool enabled;
 
 	struct omap_drm_irq vblank_irq;
 	struct omap_drm_irq error_irq;
@@ -127,7 +126,7 @@ static void omap_crtc_dss_start_update(struct omap_overlay_manager *mgr)
 {
 }
 
-/* Called only from omap_crtc_setup and suspend/resume handlers. */
+/* Called only from omap_crtc_encoder_setup and suspend/resume handlers. */
 static void omap_crtc_set_enabled(struct drm_crtc *crtc, bool enable)
 {
 	struct drm_device *dev = crtc->dev;
@@ -386,14 +385,14 @@ int omap_crtc_flush(struct drm_crtc *crtc)
 	return 0;
 }
 
-static void omap_crtc_setup(struct drm_crtc *crtc)
+static void omap_crtc_encoder_setup(struct drm_crtc *crtc, bool enable)
 {
 	struct omap_crtc *omap_crtc = to_omap_crtc(crtc);
 	struct omap_drm_private *priv = crtc->dev->dev_private;
 	struct drm_encoder *encoder = NULL;
 	unsigned int i;
 
-	DBG("%s: enabled=%d", omap_crtc->name, omap_crtc->enabled);
+	DBG("%s: enable=%d", omap_crtc->name, enable);
 
 	dispc_runtime_get();
 
@@ -409,14 +408,11 @@ static void omap_crtc_setup(struct drm_crtc *crtc)
 
 	omap_crtc->current_encoder = encoder;
 
-	if (!omap_crtc->enabled) {
-		if (encoder)
-			omap_encoder_set_enabled(encoder, false);
-	} else {
-		if (encoder) {
-			omap_encoder_set_enabled(encoder, false);
+	if (encoder) {
+		omap_encoder_set_enabled(encoder, false);
+		if (enable) {
 			omap_encoder_update(encoder, omap_crtc->mgr,
-					&omap_crtc->timings);
+					    &omap_crtc->timings);
 			omap_encoder_set_enabled(encoder, true);
 		}
 	}
@@ -457,9 +453,6 @@ static void omap_crtc_enable(struct drm_crtc *crtc)
 
 	DBG("%s", omap_crtc->name);
 
-	if (omap_crtc->enabled)
-		return;
-
 	/* Enable all planes associated with the CRTC. */
 	for (i = 0; i < priv->num_planes; i++) {
 		struct drm_plane *plane = priv->planes[i];
@@ -468,9 +461,7 @@ static void omap_crtc_enable(struct drm_crtc *crtc)
 			WARN_ON(omap_plane_set_enable(plane, true));
 	}
 
-	omap_crtc->enabled = true;
-
-	omap_crtc_setup(crtc);
+	omap_crtc_encoder_setup(crtc, true);
 	omap_crtc_flush(crtc);
 
 	dispc_runtime_get();
@@ -486,9 +477,6 @@ static void omap_crtc_disable(struct drm_crtc *crtc)
 
 	DBG("%s", omap_crtc->name);
 
-	if (!omap_crtc->enabled)
-		return;
-
 	omap_crtc_wait_page_flip(crtc);
 	dispc_runtime_get();
 	drm_crtc_vblank_off(crtc);
@@ -502,9 +490,7 @@ static void omap_crtc_disable(struct drm_crtc *crtc)
 			WARN_ON(omap_plane_set_enable(plane, false));
 	}
 
-	omap_crtc->enabled = false;
-
-	omap_crtc_setup(crtc);
+	omap_crtc_encoder_setup(crtc, false);
 	omap_crtc_flush(crtc);
 }
 
-- 
2.0.5

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

[PATCH 28/31] drm: omapdrm: Remove omap_plane enabled field

[Laurent Pinchart]

The field tracks the plane state to avoid double-enable or -disable.
This isn't required anymore, as

- the DRM atomic core guarantees that the plane atomic_update and
  atomic_disable functions will never be called on an enabled/disabled
  plane

- the CRTC enable/disable operations that enable/disable the plane are
  already guarded against double enable/disable

We can thus remove the enabled field completely. The
omap_plane_set_enable() function then becomes a wrapper around
omap_plane_setup() which can be called directly.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 drivers/gpu/drm/omapdrm/omap_crtc.c  | 11 +++++----
 drivers/gpu/drm/omapdrm/omap_drv.h   |  2 +-
 drivers/gpu/drm/omapdrm/omap_plane.c | 45 +++++++-----------------------------
 3 files changed, 16 insertions(+), 42 deletions(-)

diff --git a/drivers/gpu/drm/omapdrm/omap_crtc.c b/drivers/gpu/drm/omapdrm/omap_crtc.c
index d02b54d..5d0356f 100644
--- a/drivers/gpu/drm/omapdrm/omap_crtc.c
+++ b/drivers/gpu/drm/omapdrm/omap_crtc.c
@@ -453,19 +453,21 @@ static void omap_crtc_enable(struct drm_crtc *crtc)
 
 	DBG("%s", omap_crtc->name);
 
+	dispc_runtime_get();
+
 	/* Enable all planes associated with the CRTC. */
 	for (i = 0; i < priv->num_planes; i++) {
 		struct drm_plane *plane = priv->planes[i];
 
 		if (plane->crtc == crtc)
-			WARN_ON(omap_plane_set_enable(plane, true));
+			WARN_ON(omap_plane_setup(plane));
 	}
 
 	omap_crtc_encoder_setup(crtc, true);
 	omap_crtc_flush(crtc);
 
-	dispc_runtime_get();
 	drm_crtc_vblank_on(crtc);
+
 	dispc_runtime_put();
 }
 
@@ -480,18 +482,19 @@ static void omap_crtc_disable(struct drm_crtc *crtc)
 	omap_crtc_wait_page_flip(crtc);
 	dispc_runtime_get();
 	drm_crtc_vblank_off(crtc);
-	dispc_runtime_put();
 
 	/* Disable all planes associated with the CRTC. */
 	for (i = 0; i < priv->num_planes; i++) {
 		struct drm_plane *plane = priv->planes[i];
 
 		if (plane->crtc == crtc)
-			WARN_ON(omap_plane_set_enable(plane, false));
+			WARN_ON(omap_plane_setup(plane));
 	}
 
 	omap_crtc_encoder_setup(crtc, false);
 	omap_crtc_flush(crtc);
+
+	dispc_runtime_put();
 }
 
 static void omap_crtc_mode_set_nofb(struct drm_crtc *crtc)
diff --git a/drivers/gpu/drm/omapdrm/omap_drv.h b/drivers/gpu/drm/omapdrm/omap_drv.h
index bef3002..56b8ee6 100644
--- a/drivers/gpu/drm/omapdrm/omap_drv.h
+++ b/drivers/gpu/drm/omapdrm/omap_drv.h
@@ -150,7 +150,7 @@ struct drm_crtc *omap_crtc_init(struct drm_device *dev,
 
 struct drm_plane *omap_plane_init(struct drm_device *dev,
 		int id, enum drm_plane_type type);
-int omap_plane_set_enable(struct drm_plane *plane, bool enable);
+int omap_plane_setup(struct drm_plane *plane);
 void omap_plane_install_properties(struct drm_plane *plane,
 		struct drm_mode_object *obj);
 
diff --git a/drivers/gpu/drm/omapdrm/omap_plane.c b/drivers/gpu/drm/omapdrm/omap_plane.c
index ac6b72c..a3c5d1f 100644
--- a/drivers/gpu/drm/omapdrm/omap_plane.c
+++ b/drivers/gpu/drm/omapdrm/omap_plane.c
@@ -40,8 +40,6 @@ struct omap_plane {
 	int id;  /* TODO rename omap_plane -> omap_plane_id in omapdss so I can use the enum */
 	const char *name;
 
-	bool enabled;
-
 	uint32_t nformats;
 	uint32_t formats[32];
 
@@ -60,18 +58,19 @@ to_omap_plane_state(struct drm_plane_state *state)
 	return container_of(state, struct omap_plane_state, base);
 }
 
-static int omap_plane_setup(struct omap_plane *omap_plane)
+int omap_plane_setup(struct drm_plane *plane)
 {
-	struct drm_plane_state *state = omap_plane->base.state;
+	struct omap_plane *omap_plane = to_omap_plane(plane);
+	struct drm_plane_state *state = plane->state;
 	struct omap_plane_state *omap_state = to_omap_plane_state(state);
-	struct drm_device *dev = omap_plane->base.dev;
+	struct drm_device *dev = plane->dev;
 	struct omap_overlay_info info;
 	struct omap_drm_window win;
 	int ret;
 
-	DBG("%s, enabled=%d", omap_plane->name, omap_plane->enabled);
+	DBG("%s, crtc=%p fb=%p", omap_plane->name, state->crtc, state->fb);
 
-	if (!omap_plane->enabled) {
+	if (!state->crtc) {
 		dispc_ovl_enable(omap_plane->id, false);
 		return 0;
 	}
@@ -134,23 +133,6 @@ static int omap_plane_setup(struct omap_plane *omap_plane)
 	return 0;
 }
 
-int omap_plane_set_enable(struct drm_plane *plane, bool enable)
-{
-	struct omap_plane *omap_plane = to_omap_plane(plane);
-	int ret;
-
-	if (enable == omap_plane->enabled)
-		return 0;
-
-	omap_plane->enabled = enable;
-
-	dispc_runtime_get();
-	ret = omap_plane_setup(omap_plane);
-	dispc_runtime_put();
-
-	return ret;
-}
-
 static int omap_plane_prepare_fb(struct drm_plane *plane,
 				 struct drm_framebuffer *fb,
 				 const struct drm_plane_state *new_state)
@@ -168,14 +150,7 @@ static void omap_plane_cleanup_fb(struct drm_plane *plane,
 static void omap_plane_atomic_update(struct drm_plane *plane,
 				     struct drm_plane_state *old_state)
 {
-	struct omap_plane *omap_plane = to_omap_plane(plane);
-	struct drm_plane_state *state = plane->state;
-
-	if (!state->fb || !state->crtc)
-		return;
-
-	omap_plane->enabled = true;
-	omap_plane_setup(omap_plane);
+	omap_plane_setup(plane);
 }
 
 static void omap_plane_atomic_disable(struct drm_plane *plane,
@@ -188,11 +163,7 @@ static void omap_plane_atomic_disable(struct drm_plane *plane,
 	omap_state->zorder = plane->type == DRM_PLANE_TYPE_PRIMARY
 			   ? 0 : omap_plane->id;
 
-	if (!omap_plane->enabled)
-		return;
-
-	omap_plane->enabled = false;
-	omap_plane_setup(omap_plane);
+	omap_plane_setup(plane);
 }
 
 static const struct drm_plane_helper_funcs omap_plane_helper_funcs = {
-- 
2.0.5

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

[PATCH 29/31] drm: omapdrm: Make the omap_crtc_flush function static

[Laurent Pinchart]

The function isn't used outside of its compilation unit, make it static.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 drivers/gpu/drm/omapdrm/omap_crtc.c | 2 +-
 drivers/gpu/drm/omapdrm/omap_drv.h  | 1 -
 2 files changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/omapdrm/omap_crtc.c b/drivers/gpu/drm/omapdrm/omap_crtc.c
index 5d0356f..cc12dbf 100644
--- a/drivers/gpu/drm/omapdrm/omap_crtc.c
+++ b/drivers/gpu/drm/omapdrm/omap_crtc.c
@@ -360,7 +360,7 @@ static void omap_crtc_vblank_irq(struct omap_drm_irq *irq, uint32_t irqstatus)
 	complete(&omap_crtc->completion);
 }
 
-int omap_crtc_flush(struct drm_crtc *crtc)
+static int omap_crtc_flush(struct drm_crtc *crtc)
 {
 	struct omap_crtc *omap_crtc = to_omap_crtc(crtc);
 
diff --git a/drivers/gpu/drm/omapdrm/omap_drv.h b/drivers/gpu/drm/omapdrm/omap_drv.h
index 56b8ee6..b105394 100644
--- a/drivers/gpu/drm/omapdrm/omap_drv.h
+++ b/drivers/gpu/drm/omapdrm/omap_drv.h
@@ -141,7 +141,6 @@ void omap_fbdev_free(struct drm_device *dev);
 
 const struct omap_video_timings *omap_crtc_timings(struct drm_crtc *crtc);
 enum omap_channel omap_crtc_channel(struct drm_crtc *crtc);
-int omap_crtc_flush(struct drm_crtc *crtc);
 void omap_crtc_cancel_page_flip(struct drm_crtc *crtc, struct drm_file *file);
 void omap_crtc_pre_init(void);
 void omap_crtc_pre_uninit(void);
-- 
2.0.5

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

[PATCH 30/31] drm: omapdrm: Don't get/put dispc in omap_crtc_flush()

[Laurent Pinchart]

The omap_crtc_flush() function is always called with a reference to the
dispc held. Remove unnecessary calls to dispc_runtime_get() and
dispc_runtime_put().

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 drivers/gpu/drm/omapdrm/omap_crtc.c | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/drivers/gpu/drm/omapdrm/omap_crtc.c b/drivers/gpu/drm/omapdrm/omap_crtc.c
index cc12dbf..189c2cb 100644
--- a/drivers/gpu/drm/omapdrm/omap_crtc.c
+++ b/drivers/gpu/drm/omapdrm/omap_crtc.c
@@ -369,8 +369,6 @@ static int omap_crtc_flush(struct drm_crtc *crtc)
 	WARN_ON(!drm_modeset_is_locked(&crtc->mutex));
 	WARN_ON(omap_crtc->vblank_irq.registered);
 
-	dispc_runtime_get();
-
 	if (dispc_mgr_is_enabled(omap_crtc->channel)) {
 		dispc_mgr_go(omap_crtc->channel);
 		omap_irq_register(crtc->dev, &omap_crtc->vblank_irq);
@@ -380,8 +378,6 @@ static int omap_crtc_flush(struct drm_crtc *crtc)
 		reinit_completion(&omap_crtc->completion);
 	}
 
-	dispc_runtime_put();
-
 	return 0;
 }
 
-- 
2.0.5

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

[PATCH 31/31] drm: omapdrm: omap_crtc_flush() isn't called with modeset locked

[Laurent Pinchart]

When performing asynchronous atomic updates the modeset lock isn't taken
around the callers of omap_crtc_flush(). This isn't an issue though, as
access to the CRTC is properly serialized. Just drop the warning.

Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
---
 drivers/gpu/drm/omapdrm/omap_crtc.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/gpu/drm/omapdrm/omap_crtc.c b/drivers/gpu/drm/omapdrm/omap_crtc.c
index 189c2cb..4472862 100644
--- a/drivers/gpu/drm/omapdrm/omap_crtc.c
+++ b/drivers/gpu/drm/omapdrm/omap_crtc.c
@@ -366,7 +366,6 @@ static int omap_crtc_flush(struct drm_crtc *crtc)
 
 	DBG("%s: GO", omap_crtc->name);
 
-	WARN_ON(!drm_modeset_is_locked(&crtc->mutex));
 	WARN_ON(omap_crtc->vblank_irq.registered);
 
 	if (dispc_mgr_is_enabled(omap_crtc->channel)) {
-- 
2.0.5

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

Re: [PATCH 00/31] Convert omapdrm to the atomic updates API

[Daniel Vetter]

On Thu, Apr 16, 2015 at 01:09:17AM +0300, Laurent Pinchart wrote:
> Hello,
> 
> This patch set converts the omapdrm to the atomic update API in 31 small(ish)
> and hopefully reviewable steps.
> 
> The series is based on the latest drm/next branch and depends on the pending
> "drm: omapdrm: Store the rotation property in dev->mode_config" patch by
> Daniel Vetter.
> 
> The omapdrm loses support for its custom fences in the process. This isn't
> deemed to be an issue, as the custom fences API is used by the SGX driver
> only, which requires out-of-tree patches anyway. Fences support will be
> reimplemented at a later point on top of the atomic updates conversion using
> the mainline fence API.
> 
> One known issue is a race condition between asynchronous commits and
> drm_release() which can trigger a warning if a commit is applied between the
> drm_events_release() call and the WARN_ON(!list_empty(&file_priv->event_list)).
> Investigation is ongoing to find out the best way to fix the problem, the
> problem seems not to be limited to omapdrm (but can be more or less difficult
> to reproduce depending on the driver). Ideas would be welcome, I've CC'ed
> maintainers of possibly affected drivers.

Known issue, every driver has to currently hand-roll their pending events
code, and many don't :( One of my goals with rewriting drm_irq.c was to
make it possible to fix this up, but somehow that gets pushed down the
priority list forever ...
-Daniel

> 
> There's not much else to be said, details are explained in individual patches.
> 
> The patches can be found in my git tree at
> 
> 	git://linuxtv.org/pinchartl/fbdev.git omapdrm/next
> 
> Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> Cc: Rob Clark <robdclark@gmail.com>
> Cc: Thierry Reding <treding@nvidia.com>
> Cc: Benjamin Gaignard <benjamin.gaignard@linaro.org>
> 
> Laurent Pinchart (31):
>   drm: omapdrm: Store the rotation property in dev->mode_config
>   drm: omapdrm: Apply settings synchronously
>   drm: omapdrm: Rename omap_crtc_page_flip_locked to omap_crtc_page_flip
>   drm: omapdrm: Rename omap_crtc page flip-related fields
>   drm: omapdrm: Simplify IRQ registration
>   drm: omapdrm: Cancel pending page flips when closing device
>   drm: omapdrm: Rework page flip handling
>   drm: omapdrm: Turn vblank on/off when enabling/disabling CRTC
>   drm: omapdrm: Fix page flip race with CRTC disable
>   drm: omapdrm: Clean up #include's
>   drm: omapdrm: Rename CRTC DSS operations with an omap_crtc_dss_ prefix
>   drm: omapdrm: Rework CRTC enable/disable for atomic updates
>   drm: omapdrm: Implement encoder .disable() and .enable() operations
>   drm: omapdrm: Wire up atomic state object scaffolding
>   drm: omapdrm: Implement planes atomic operations
>   drm: omapdrm: Handle primary plane config through atomic plane ops
>   drm: omapdrm: Switch plane update to atomic helpers
>   drm: omapdrm: Switch mode config to atomic helpers
>   drm: omapdrm: Switch connector DPMS to atomic helpers
>   drm: omapdrm: Replace encoder mode_fixup with atomic_check
>   drm: omapdrm: Implement asynchronous commit support
>   drm: omapdrm: Switch page flip to atomic helpers
>   drm: omapdrm: Drop manual framebuffer pin handling
>   drm: omapdrm: Switch crtc and plane set_property to atomic helpers
>   drm: omapdrm: Move plane info and win out of the plane structure
>   drm: omapdrm: Move crtc info out of the crtc structure
>   drm: omapdrm: Remove omap_crtc enabled field
>   drm: omapdrm: Remove omap_plane enabled field
>   drm: omapdrm: Make the omap_crtc_flush function static
>   drm: omapdrm: Don't get/put dispc in omap_crtc_flush()
>   drm: omapdrm: omap_crtc_flush() isn't called with modeset locked
> 
>  drivers/gpu/drm/omapdrm/omap_connector.c  |  12 +-
>  drivers/gpu/drm/omapdrm/omap_crtc.c       | 580 +++++++++++++-----------------
>  drivers/gpu/drm/omapdrm/omap_debugfs.c    |   6 +-
>  drivers/gpu/drm/omapdrm/omap_dmm_tiler.c  |  19 +-
>  drivers/gpu/drm/omapdrm/omap_drv.c        | 162 ++++++++-
>  drivers/gpu/drm/omapdrm/omap_drv.h        |  52 +--
>  drivers/gpu/drm/omapdrm/omap_encoder.c    |  60 ++--
>  drivers/gpu/drm/omapdrm/omap_fb.c         |   8 +-
>  drivers/gpu/drm/omapdrm/omap_fbdev.c      |   6 +-
>  drivers/gpu/drm/omapdrm/omap_gem.c        |   4 +-
>  drivers/gpu/drm/omapdrm/omap_gem_dmabuf.c |   4 +-
>  drivers/gpu/drm/omapdrm/omap_irq.c        | 106 ++----
>  drivers/gpu/drm/omapdrm/omap_plane.c      | 426 +++++++++-------------
>  13 files changed, 656 insertions(+), 789 deletions(-)
> 
> -- 
> Regards,
> 
> Laurent Pinchart
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 00/31] Convert omapdrm to the atomic updates API

[Tomi Valkeinen]

[-- Attachment #1.1: Type: text/plain, Size: 7244 bytes --]

Hi,

On 16/04/15 01:09, Laurent Pinchart wrote:
> Hello,
> 
> This patch set converts the omapdrm to the atomic update API in 31 small(ish)
> and hopefully reviewable steps.

Thanks! I'll start reviewing the patches later this week.

> The series is based on the latest drm/next branch and depends on the pending
> "drm: omapdrm: Store the rotation property in dev->mode_config" patch by
> Daniel Vetter.
> 
> The omapdrm loses support for its custom fences in the process. This isn't
> deemed to be an issue, as the custom fences API is used by the SGX driver
> only, which requires out-of-tree patches anyway. Fences support will be
> reimplemented at a later point on top of the atomic updates conversion using
> the mainline fence API.
> 
> One known issue is a race condition between asynchronous commits and
> drm_release() which can trigger a warning if a commit is applied between the
> drm_events_release() call and the WARN_ON(!list_empty(&file_priv->event_list)).
> Investigation is ongoing to find out the best way to fix the problem, the
> problem seems not to be limited to omapdrm (but can be more or less difficult
> to reproduce depending on the driver). Ideas would be welcome, I've CC'ed
> maintainers of possibly affected drivers.
> 
> There's not much else to be said, details are explained in individual patches.
> 
> The patches can be found in my git tree at
> 
> 	git://linuxtv.org/pinchartl/fbdev.git omapdrm/next


I ran a few quick tests with your branch, on Pandaboard.

Something related to vblanks/timing is broken, and I can only get half
the fps. The "db" test app from my omapdrm-tests shows this easily.

I also got a few warnings (below), but that was before I applied your
"drm: omapdrm: Support unlinking page flip events prematurely" patch.
After that I didn't see those anymore.

I didn't see any other issues, but my tests were quite quick and simple.

 Tomi


[  190.434112] ------------[ cut here ]------------
[  190.439575] WARNING: CPU: 0 PID: 175 at
drivers/gpu/drm/omapdrm/omap_gem.c:1292 omap_gem_free_object+0x260/0x278
[omapdrm]()
[  190.451385] Modules linked in: omapdrm drm_kms_helper drm cfbfillrect
cfbimgblt cfbcopyarea panel_nec_nl8048hl11 panel_sony_acx565akm
panel_lgphilips_lb035q02 panel_dsi_cm pan
el_dpi connector_analog_tv connector_hdmi connector_dvi
encoder_tpd12s015 encoder_tfp410 omapdss
[  190.475860] CPU: 0 PID: 175 Comm: producer Not tainted
4.0.0-rc7-00759-g7290d47d0475 #4
[  190.485198] Hardware name: Generic OMAP4 (Flattened Device Tree)
[  190.485626] Backtrace:
[  190.493469] [<c00137dc>] (dump_backtrace) from [<c00139f0>]
(show_stack+0x18/0x1c)
[  190.502075]  r6:c0a07598 r5:c0a07598 r4:00000000 r3:00000000
[  190.505676] [<c00139d8>] (show_stack) from [<c068bf8c>]
(dump_stack+0xa4/0xd4)
[  190.515686] [<c068bee8>] (dump_stack) from [<c0045084>]
(warn_slowpath_common+0x84/0xc0)
[  190.524230]  r6:0000050c r5:bf145890 r4:00000000 r3:ed148b80
[  190.526794] [<c0045000>] (warn_slowpath_common) from [<c0045164>]
(warn_slowpath_null+0x24/0x2c)
[  190.539459]  r8:edac6678 r7:edf9eb98 r6:ee6b7800 r5:ed1b689c r4:ee713b80
[  190.545288] [<c0045140>] (warn_slowpath_null) from [<bf145890>]
(omap_gem_free_object+0x260/0x278 [omapdrm])
[  190.556945] [<bf145630>] (omap_gem_free_object [omapdrm]) from
[<bf0ae0bc>] (drm_gem_object_free+0x30/0x38 [drm])
[  190.567993]  r9:00000010 r8:edac6678 r7:edf9eb98 r6:ee2c2b10
r5:ee6b7834 r4:ee713b80
[  190.575134] [<bf0ae08c>] (drm_gem_object_free [drm]) from
[<bf146154>] (omap_gem_dmabuf_release+0xa8/0xb4 [omapdrm])
[  190.587432] [<bf1460ac>] (omap_gem_dmabuf_release [omapdrm]) from
[<c04211e0>] (dma_buf_release+0x54/0x17c)
[  190.597717]  r5:00000000 r4:ee60e800
[  190.601501] [<c042118c>] (dma_buf_release) from [<c016ba6c>]
(__fput+0x9c/0x1ec)
[  190.609283]  r9:00000010 r8:edac6678 r7:edf9eb98 r6:ee2c2b10
r5:edac6678 r4:ee6fce80
[  190.614654] [<c016b9d0>] (__fput) from [<c016bc24>] (____fput+0x10/0x14)
[  190.619750]  r10:ed148b80 r9:00000001 r8:00000000 r7:ed148b80
r6:c0a53738 r5:ed02ce40
[  190.632812]  r4:ed149084
[  190.635467] [<c016bc14>] (____fput) from [<c006099c>]
(task_work_run+0xbc/0x100)
[  190.643432] [<c00608e0>] (task_work_run) from [<c0047b20>]
(do_exit+0x390/0xb00)
[  190.651214]  r8:ebc16658 r7:ebc16600 r6:b6f75750 r5:c0a402c4
r4:ed14909c r3:0000003c
[  190.654876] [<c0047790>] (do_exit) from [<c0048314>]
(do_group_exit+0x48/0xc4)
[  190.667144]  r7:000000f8
[  190.670074] [<c00482cc>] (do_group_exit) from [<c00483a8>]
(__wake_up_parent+0x0/0x28)
[  190.678466]  r6:b6f75750 r5:00000000 r4:00000001 r3:ffffffff
[  190.684753] [<c0048390>] (SyS_exit_group) from [<c000f880>]
(ret_fast_syscall+0x0/0x4c)
[  190.693237] ---[ end trace 0f3a4a8e4c514be3 ]---




[  226.385437] ------------[ cut here ]------------
[  226.385437] WARNING: CPU: 0 PID: 185 at
drivers/gpu/drm/drm_fops.c:459 drm_release+0x418/0x464 [drm]()
[  226.385437] Modules linked in: omapdrm drm_kms_helper drm cfbfillrect
cfbimgblt cfbcopyarea panel_nec_nl8048hl11 panel_sony_acx565akm
panel_lgphilips_lb035q02 panel_dsi_cm pan
el_dpi connector_analog_tv connector_hdmi connector_dvi
encoder_tpd12s015 encoder_tfp410 omapdss
[  226.425781] CPU: 0 PID: 185 Comm: consumer Tainted: G        W
4.0.0-rc7-00759-g7290d47d0475 #4
[  226.435668] Hardware name: Generic OMAP4 (Flattened Device Tree)
[  226.441986] Backtrace:
[  226.442413] [<c00137dc>] (dump_backtrace) from [<c00139f0>]
(show_stack+0x18/0x1c)
[  226.452545]  r6:c0a07598 r5:c0a07598 r4:00000000 r3:00000000
[  226.456451] [<c00139d8>] (show_stack) from [<c068bf8c>]
(dump_stack+0xa4/0xd4)
[  226.466156] [<c068bee8>] (dump_stack) from [<c0045084>]
(warn_slowpath_common+0x84/0xc0)
[  226.474700]  r6:000001cb r5:bf0ad8ac r4:00000000 r3:e380a280
[  226.475097] [<c0045000>] (warn_slowpath_common) from [<c0045164>]
(warn_slowpath_null+0x24/0x2c)
[  226.489959]  r8:e57c52f4 r7:ee6b7834 r6:ee6b7800 r5:e57c52f8 r4:e57c5200
[  226.494720] [<c0045140>] (warn_slowpath_null) from [<bf0ad8ac>]
(drm_release+0x418/0x464 [drm])
[  226.506378] [<bf0ad494>] (drm_release [drm]) from [<c016ba6c>]
(__fput+0x9c/0x1ec)
[  226.506896]  r10:00000000 r9:00000008 r8:ed34d838 r7:edf3c240
r6:ee698910 r5:ed34d838
[  226.520416]  r4:ee762d40
[  226.525451] [<c016b9d0>] (__fput) from [<c016bc24>] (____fput+0x10/0x14)
[  226.525573]  r10:e380a280 r9:00000001 r8:00000000 r7:e380a280
r6:c0a53738 r5:ee6f4680
[  226.535339]  r4:e380a784
[  226.543457] [<c016bc14>] (____fput) from [<c006099c>]
(task_work_run+0xbc/0x100)
[  226.551239] [<c00608e0>] (task_work_run) from [<c0047b20>]
(do_exit+0x390/0xb00)
[  226.559020]  r8:e57c5658 r7:e57c5600 r6:b6f46750 r5:c0a402c4
r4:e380a79c r3:0000003d
[  226.567230] [<c0047790>] (do_exit) from [<c0048314>]
(do_group_exit+0x48/0xc4)
[  226.574829]  r7:000000f8
[  226.575164] [<c00482cc>] (do_group_exit) from [<c00483a8>]
(__wake_up_parent+0x0/0x28)
[  226.585845]  r6:b6f46750 r5:00000001 r4:00000001 r3:ffffffff
[  226.586181] [<c0048390>] (SyS_exit_group) from [<c000f880>]
(ret_fast_syscall+0x0/0x4c)
[  226.600250] ---[ end trace 0f3a4a8e4c514be4 ]---





[-- Attachment #1.2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

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