diff mbox

drm/omap: use omapdss low level API

Message ID 1353024058-11504-1-git-send-email-robdclark@gmail.com
State New
Headers show

Commit Message

Rob Clark Nov. 16, 2012, midnight UTC 
This patch changes the omapdrm KMS to bypass the omapdss "compat"
layer and use the core omapdss API directly.  This solves some layering
issues that would cause unpin confusion vs GO bit status, because we
would not know whether a particular pageflip or overlay update has hit
the screen or not.  Now instead we explicitly manage the GO bits in
dispc and handle the vblank/framedone interrupts ourself so that we
always know which buffers are being scanned out at any given time, and
so on.

As an added bonus, we no longer leave the last overlay buffer pinned
when the display is disabled, and have been able to add the previously
missing vblank event handling.

Signed-off-by: Rob Clark <robdclark@gmail.com>
---
Note: this patch applies on top of staging-next plus the "OMAPDSS:
create compat layer" patch series:

http://comments.gmane.org/gmane.linux.ports.arm.omap/89435

 drivers/staging/omapdrm/Makefile         |   1 +
 drivers/staging/omapdrm/TODO             |   3 -
 drivers/staging/omapdrm/omap_connector.c | 111 +-------
 drivers/staging/omapdrm/omap_crtc.c      | 472 +++++++++++++++++++++++++++++--
 drivers/staging/omapdrm/omap_drv.c       | 439 +++++-----------------------
 drivers/staging/omapdrm/omap_drv.h       | 140 +++++++--
 drivers/staging/omapdrm/omap_encoder.c   | 132 ++++-----
 drivers/staging/omapdrm/omap_irq.c       | 322 +++++++++++++++++++++
 drivers/staging/omapdrm/omap_plane.c     | 452 +++++++++++------------------
 9 files changed, 1207 insertions(+), 865 deletions(-)
 create mode 100644 drivers/staging/omapdrm/omap_irq.c

Comments

Archit Taneja Nov. 16, 2012, 6:44 a.m. UTC | #1 
On Friday 16 November 2012 05:30 AM, Rob Clark wrote:
> This patch changes the omapdrm KMS to bypass the omapdss "compat"
> layer and use the core omapdss API directly.  This solves some layering
> issues that would cause unpin confusion vs GO bit status, because we
> would not know whether a particular pageflip or overlay update has hit
> the screen or not.  Now instead we explicitly manage the GO bits in
> dispc and handle the vblank/framedone interrupts ourself so that we
> always know which buffers are being scanned out at any given time, and
> so on.
>
> As an added bonus, we no longer leave the last overlay buffer pinned
> when the display is disabled, and have been able to add the previously
> missing vblank event handling.
>
> Signed-off-by: Rob Clark <robdclark@gmail.com>
> ---
> Note: this patch applies on top of staging-next plus the "OMAPDSS:
> create compat layer" patch series:
>
> http://comments.gmane.org/gmane.linux.ports.arm.omap/89435
>
>   drivers/staging/omapdrm/Makefile         |   1 +
>   drivers/staging/omapdrm/TODO             |   3 -
>   drivers/staging/omapdrm/omap_connector.c | 111 +-------
>   drivers/staging/omapdrm/omap_crtc.c      | 472 +++++++++++++++++++++++++++++--
>   drivers/staging/omapdrm/omap_drv.c       | 439 +++++-----------------------
>   drivers/staging/omapdrm/omap_drv.h       | 140 +++++++--
>   drivers/staging/omapdrm/omap_encoder.c   | 132 ++++-----
>   drivers/staging/omapdrm/omap_irq.c       | 322 +++++++++++++++++++++
>   drivers/staging/omapdrm/omap_plane.c     | 452 +++++++++++------------------
>   9 files changed, 1207 insertions(+), 865 deletions(-)
>   create mode 100644 drivers/staging/omapdrm/omap_irq.c
>
> diff --git a/drivers/staging/omapdrm/Makefile b/drivers/staging/omapdrm/Makefile
> index 1ca0e00..d85e058 100644
> --- a/drivers/staging/omapdrm/Makefile
> +++ b/drivers/staging/omapdrm/Makefile
> @@ -5,6 +5,7 @@
>
>   ccflags-y := -Iinclude/drm -Werror
>   omapdrm-y := omap_drv.o \
> +	omap_irq.o \
>   	omap_debugfs.o \
>   	omap_crtc.o \
>   	omap_plane.o \
> diff --git a/drivers/staging/omapdrm/TODO b/drivers/staging/omapdrm/TODO
> index 938c788..abeeb00 100644
> --- a/drivers/staging/omapdrm/TODO
> +++ b/drivers/staging/omapdrm/TODO
> @@ -17,9 +17,6 @@ TODO
>   . Revisit GEM sync object infrastructure.. TTM has some framework for this
>     already.  Possibly this could be refactored out and made more common?
>     There should be some way to do this with less wheel-reinvention.
> -. Review DSS vs KMS mismatches.  The omap_dss_device is sort of part encoder,
> -  part connector.  Which results in a bit of duct tape to fwd calls from
> -  encoder to connector.  Possibly this could be done a bit better.
>   . Solve PM sequencing on resume.  DMM/TILER must be reloaded before any
>     access is made from any component in the system.  Which means on suspend
>     CRTC's should be disabled, and on resume the LUT should be reprogrammed
> diff --git a/drivers/staging/omapdrm/omap_connector.c b/drivers/staging/omapdrm/omap_connector.c
> index 91edb3f..4cc9ee7 100644
> --- a/drivers/staging/omapdrm/omap_connector.c
> +++ b/drivers/staging/omapdrm/omap_connector.c
> @@ -31,9 +31,10 @@
>   struct omap_connector {
>   	struct drm_connector base;
>   	struct omap_dss_device *dssdev;
> +	struct drm_encoder *encoder;
>   };
>
> -static inline void copy_timings_omap_to_drm(struct drm_display_mode *mode,
> +void copy_timings_omap_to_drm(struct drm_display_mode *mode,
>   		struct omap_video_timings *timings)
>   {
>   	mode->clock = timings->pixel_clock;
> @@ -64,7 +65,7 @@ static inline void copy_timings_omap_to_drm(struct drm_display_mode *mode,
>   		mode->flags |= DRM_MODE_FLAG_NVSYNC;
>   }
>
> -static inline void copy_timings_drm_to_omap(struct omap_video_timings *timings,
> +void copy_timings_drm_to_omap(struct omap_video_timings *timings,
>   		struct drm_display_mode *mode)
>   {
>   	timings->pixel_clock = mode->clock;
> @@ -96,48 +97,7 @@ static inline void copy_timings_drm_to_omap(struct omap_video_timings *timings,
>   	timings->sync_pclk_edge = OMAPDSS_DRIVE_SIG_OPPOSITE_EDGES;
>   }
>
> -static void omap_connector_dpms(struct drm_connector *connector, int mode)
> -{
> -	struct omap_connector *omap_connector = to_omap_connector(connector);
> -	struct omap_dss_device *dssdev = omap_connector->dssdev;
> -	int old_dpms;
> -
> -	DBG("%s: %d", dssdev->name, mode);
> -
> -	old_dpms = connector->dpms;
> -
> -	/* from off to on, do from crtc to connector */
> -	if (mode < old_dpms)
> -		drm_helper_connector_dpms(connector, mode);
> -
> -	if (mode == DRM_MODE_DPMS_ON) {
> -		/* store resume info for suspended displays */
> -		switch (dssdev->state) {
> -		case OMAP_DSS_DISPLAY_SUSPENDED:
> -			dssdev->activate_after_resume = true;
> -			break;
> -		case OMAP_DSS_DISPLAY_DISABLED: {
> -			int ret = dssdev->driver->enable(dssdev);
> -			if (ret) {
> -				DBG("%s: failed to enable: %d",
> -						dssdev->name, ret);
> -				dssdev->driver->disable(dssdev);
> -			}
> -			break;
> -		}
> -		default:
> -			break;
> -		}
> -	} else {
> -		/* TODO */
> -	}
> -
> -	/* from on to off, do from connector to crtc */
> -	if (mode > old_dpms)
> -		drm_helper_connector_dpms(connector, mode);
> -}
> -
> -enum drm_connector_status omap_connector_detect(
> +static enum drm_connector_status omap_connector_detect(
>   		struct drm_connector *connector, bool force)
>   {
>   	struct omap_connector *omap_connector = to_omap_connector(connector);
> @@ -164,8 +124,6 @@ static void omap_connector_destroy(struct drm_connector *connector)
>   	struct omap_connector *omap_connector = to_omap_connector(connector);
>   	struct omap_dss_device *dssdev = omap_connector->dssdev;
>
> -	dssdev->driver->disable(dssdev);
> -
>   	DBG("%s", omap_connector->dssdev->name);
>   	drm_sysfs_connector_remove(connector);
>   	drm_connector_cleanup(connector);
> @@ -261,36 +219,12 @@ static int omap_connector_mode_valid(struct drm_connector *connector,
>   struct drm_encoder *omap_connector_attached_encoder(
>   		struct drm_connector *connector)
>   {
> -	int i;
>   	struct omap_connector *omap_connector = to_omap_connector(connector);
> -
> -	for (i = 0; i < DRM_CONNECTOR_MAX_ENCODER; i++) {
> -		struct drm_mode_object *obj;
> -
> -		if (connector->encoder_ids[i] == 0)
> -			break;
> -
> -		obj = drm_mode_object_find(connector->dev,
> -				connector->encoder_ids[i],
> -				DRM_MODE_OBJECT_ENCODER);
> -
> -		if (obj) {
> -			struct drm_encoder *encoder = obj_to_encoder(obj);
> -			struct omap_overlay_manager *mgr =
> -					omap_encoder_get_manager(encoder);
> -			DBG("%s: found %s", omap_connector->dssdev->name,
> -					mgr->name);
> -			return encoder;
> -		}
> -	}
> -
> -	DBG("%s: no encoder", omap_connector->dssdev->name);
> -
> -	return NULL;
> +	return omap_connector->encoder;
>   }
>
>   static const struct drm_connector_funcs omap_connector_funcs = {
> -	.dpms = omap_connector_dpms,
> +	.dpms = drm_helper_connector_dpms,
>   	.detect = omap_connector_detect,
>   	.fill_modes = drm_helper_probe_single_connector_modes,
>   	.destroy = omap_connector_destroy,
> @@ -302,34 +236,6 @@ static const struct drm_connector_helper_funcs omap_connector_helper_funcs = {
>   	.best_encoder = omap_connector_attached_encoder,
>   };
>
> -/* called from encoder when mode is set, to propagate settings to the dssdev */
> -void omap_connector_mode_set(struct drm_connector *connector,
> -		struct drm_display_mode *mode)
> -{
> -	struct drm_device *dev = connector->dev;
> -	struct omap_connector *omap_connector = to_omap_connector(connector);
> -	struct omap_dss_device *dssdev = omap_connector->dssdev;
> -	struct omap_dss_driver *dssdrv = dssdev->driver;
> -	struct omap_video_timings timings = {0};
> -
> -	copy_timings_drm_to_omap(&timings, mode);
> -
> -	DBG("%s: set mode: %d:\"%s\" %d %d %d %d %d %d %d %d %d %d 0x%x 0x%x",
> -			omap_connector->dssdev->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);
> -
> -	if (dssdrv->check_timings(dssdev, &timings)) {
> -		dev_err(dev->dev, "could not set timings\n");
> -		return;
> -	}
> -
> -	dssdrv->set_timings(dssdev, &timings);
> -}
> -
>   /* flush an area of the framebuffer (in case of manual update display that
>    * is not automatically flushed)
>    */
> @@ -344,7 +250,8 @@ void omap_connector_flush(struct drm_connector *connector,
>
>   /* initialize connector */
>   struct drm_connector *omap_connector_init(struct drm_device *dev,
> -		int connector_type, struct omap_dss_device *dssdev)
> +		int connector_type, struct omap_dss_device *dssdev,
> +		struct drm_encoder *encoder)
>   {
>   	struct drm_connector *connector = NULL;
>   	struct omap_connector *omap_connector;
> @@ -360,6 +267,8 @@ struct drm_connector *omap_connector_init(struct drm_device *dev,
>   	}
>
>   	omap_connector->dssdev = dssdev;
> +	omap_connector->encoder = encoder;
> +
>   	connector = &omap_connector->base;
>
>   	drm_connector_init(dev, connector, &omap_connector_funcs,
> diff --git a/drivers/staging/omapdrm/omap_crtc.c b/drivers/staging/omapdrm/omap_crtc.c
> index 3e2c736..e3496e5 100644
> --- a/drivers/staging/omapdrm/omap_crtc.c
> +++ b/drivers/staging/omapdrm/omap_crtc.c
> @@ -28,19 +28,131 @@
>   struct omap_crtc {
>   	struct drm_crtc base;
>   	struct drm_plane *plane;
> +
>   	const char *name;
> -	int id;
> +	int pipe;
> +	enum omap_channel channel;
> +	struct omap_overlay_manager_info info;
> +
> +	/*
> +	 * Temporary: eventually this will go away, but it is needed
> +	 * for now to keep the output's happy.  (They only need
> +	 * mgr->id.)  Eventually this will be replaced w/ something
> +	 * more common-panel-framework-y
> +	 */
> +	struct omap_overlay_manager mgr;
> +
> +	struct omap_video_timings timings;
> +	bool enabled;
> +	bool full_update;
> +
> +	struct omap_drm_apply apply;
> +
> +	struct omap_drm_irq apply_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;
>
>   	/* if there is a pending flip, these will be non-null: */
>   	struct drm_pending_vblank_event *event;
>   	struct drm_framebuffer *old_fb;
> +
> +	/* 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;
>   };
>
> +/*
> + * Manager-ops, callbacks from output when they need to configure
> + * the upstream part of the video pipe.
> + *
> + * Most of these we can ignore until we add support for command-mode
> + * panels.. for video-mode the crtc-helpers already do an adequate
> + * job of sequencing the setup of the video pipe in the proper order
> + */
> +
> +/* we can probably ignore these until we support command-mode panels: */
> +static void omap_crtc_start_update(struct omap_overlay_manager *mgr)
> +{
> +}
> +
> +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_set_timings(struct omap_overlay_manager *mgr,
> +		const struct omap_video_timings *timings)
> +{
> +	struct omap_crtc *omap_crtc = container_of(mgr, struct omap_crtc, mgr);
> +	DBG("%s", omap_crtc->name);
> +	omap_crtc->timings = *timings;
> +	omap_crtc->full_update = true;
> +}
> +
> +static void omap_crtc_set_lcd_config(struct omap_overlay_manager *mgr,
> +		const struct dss_lcd_mgr_config *config)
> +{
> +	struct omap_crtc *omap_crtc = container_of(mgr, struct omap_crtc, mgr);
> +	DBG("%s", omap_crtc->name);
> +	dispc_mgr_set_lcd_config(omap_crtc->channel, config);

Maybe you should move this dispc write also to omap_crtc_pre_apply, the 
same way it's done for timings. We'll also be confident about having the 
clocks required if this is called in pre_apply.

> +}
> +
> +static int omap_crtc_register_framedone_handler(
> +		struct omap_overlay_manager *mgr,
> +		void (*handler)(void *), void *data)
> +{
> +	return 0;
> +}
> +
> +static void omap_crtc_unregister_framedone_handler(
> +		struct omap_overlay_manager *mgr,
> +		void (*handler)(void *), void *data)
> +{
> +}
> +
> +static const struct dss_mgr_ops mgr_ops = {
> +		.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,
> +};
> +
> +/*
> + * CRTC funcs:
> + */
> +
>   static void omap_crtc_destroy(struct drm_crtc *crtc)
>   {
>   	struct omap_crtc *omap_crtc = to_omap_crtc(crtc);
> +
> +	DBG("%s", omap_crtc->name);
> +
> +	WARN_ON(omap_crtc->apply_irq.registered);
> +	omap_irq_unregister(crtc->dev, &omap_crtc->error_irq);
> +
>   	omap_crtc->plane->funcs->destroy(omap_crtc->plane);
>   	drm_crtc_cleanup(crtc);
> +
>   	kfree(omap_crtc);
>   }
>
> @@ -48,14 +160,25 @@ 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);
>   	int i;
>
> -	WARN_ON(omap_plane_dpms(omap_crtc->plane, mode));
> +	DBG("%s: %d", omap_crtc->name, mode);
> +
> +	if (enabled != omap_crtc->enabled) {
> +		omap_crtc->enabled = enabled;
> +		omap_crtc->full_update = true;
> +		omap_crtc_apply(crtc, &omap_crtc->apply);
>
> -	for (i = 0; i < priv->num_planes; i++) {
> -		struct drm_plane *plane = priv->planes[i];
> -		if (plane->crtc == crtc)
> -			WARN_ON(omap_plane_dpms(plane, mode));
> +		/* also enable our private plane: */
> +		WARN_ON(omap_plane_dpms(omap_crtc->plane, mode));
> +
> +		/* and any attached overlay planes: */
> +		for (i = 0; i < priv->num_planes; i++) {
> +			struct drm_plane *plane = priv->planes[i];
> +			if (plane->crtc == crtc)
> +				WARN_ON(omap_plane_dpms(plane, mode));
> +		}
>   	}
>   }
>
> @@ -73,12 +196,26 @@ static int omap_crtc_mode_set(struct drm_crtc *crtc,
>   		struct drm_framebuffer *old_fb)
>   {
>   	struct omap_crtc *omap_crtc = to_omap_crtc(crtc);
> -	struct drm_plane *plane = omap_crtc->plane;
>
> -	return omap_plane_mode_set(plane, crtc, crtc->fb,
> +	mode = 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);
> +
> +	copy_timings_drm_to_omap(&omap_crtc->timings, mode);
> +	omap_crtc->full_update = true;
> +
> +	return omap_plane_mode_set(omap_crtc->plane, crtc, crtc->fb,
>   			0, 0, mode->hdisplay, mode->vdisplay,
>   			x << 16, y << 16,
> -			mode->hdisplay << 16, mode->vdisplay << 16);
> +			mode->hdisplay << 16, mode->vdisplay << 16,
> +			NULL, NULL);
>   }
>
>   static void omap_crtc_prepare(struct drm_crtc *crtc)
> @@ -102,10 +239,11 @@ static int omap_crtc_mode_set_base(struct drm_crtc *crtc, int x, int y,
>   	struct drm_plane *plane = omap_crtc->plane;
>   	struct drm_display_mode *mode = &crtc->mode;
>
> -	return plane->funcs->update_plane(plane, crtc, crtc->fb,
> +	return omap_plane_mode_set(plane, crtc, crtc->fb,
>   			0, 0, mode->hdisplay, mode->vdisplay,
>   			x << 16, y << 16,
> -			mode->hdisplay << 16, mode->vdisplay << 16);
> +			mode->hdisplay << 16, mode->vdisplay << 16,
> +			NULL, NULL);
>   }
>
>   static void omap_crtc_load_lut(struct drm_crtc *crtc)
> @@ -123,7 +261,7 @@ static void vblank_cb(void *arg)
>
>   	/* wakeup userspace */
>   	if (omap_crtc->event)
> -		drm_send_vblank_event(dev, -1, omap_crtc->event);
> +		drm_send_vblank_event(dev, omap_crtc->pipe, omap_crtc->event);
>
>   	omap_crtc->event = NULL;
>   	omap_crtc->old_fb = NULL;
> @@ -131,25 +269,37 @@ static void vblank_cb(void *arg)
>   	spin_unlock_irqrestore(&dev->event_lock, flags);
>   }
>
> -static void page_flip_cb(void *arg)
> +static void page_flip_worker(struct work_struct *work)
>   {
> -	struct drm_crtc *crtc = arg;
> -	struct omap_crtc *omap_crtc = to_omap_crtc(crtc);
> -	struct drm_framebuffer *old_fb = omap_crtc->old_fb;
> +	struct omap_crtc *omap_crtc =
> +			container_of(work, struct omap_crtc, page_flip_work);
> +	struct drm_crtc *crtc = &omap_crtc->base;
> +	struct drm_device *dev = crtc->dev;
> +	struct drm_display_mode *mode = &crtc->mode;
>   	struct drm_gem_object *bo;
>
> -	omap_crtc_mode_set_base(crtc, crtc->x, crtc->y, old_fb);
> -
> -	/* really we'd like to setup the callback atomically w/ setting the
> -	 * new scanout buffer to avoid getting stuck waiting an extra vblank
> -	 * cycle.. for now go for correctness and later figure out speed..
> -	 */
> -	omap_plane_on_endwin(omap_crtc->plane, vblank_cb, crtc);
> +	mutex_lock(&dev->mode_config.mutex);
> +	omap_plane_mode_set(omap_crtc->plane, crtc, crtc->fb,
> +			0, 0, mode->hdisplay, mode->vdisplay,
> +			crtc->x << 16, crtc->y << 16,
> +			mode->hdisplay << 16, mode->vdisplay << 16,
> +			vblank_cb, crtc);
> +	mutex_unlock(&dev->mode_config.mutex);
>
>   	bo = omap_framebuffer_bo(crtc->fb, 0);
>   	drm_gem_object_unreference_unlocked(bo);
>   }
>
> +static void page_flip_cb(void *arg)
> +{
> +	struct drm_crtc *crtc = arg;
> +	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->page_flip_work);
> +}
> +
>   static int omap_crtc_page_flip_locked(struct drm_crtc *crtc,
>   		 struct drm_framebuffer *fb,
>   		 struct drm_pending_vblank_event *event)
> @@ -158,14 +308,14 @@ static int omap_crtc_page_flip_locked(struct drm_crtc *crtc,
>   	struct omap_crtc *omap_crtc = to_omap_crtc(crtc);
>   	struct drm_gem_object *bo;
>
> -	DBG("%d -> %d", crtc->fb ? crtc->fb->base.id : -1, fb->base.id);
> +	DBG("%d -> %d (event=%p)", crtc->fb ? crtc->fb->base.id : -1,
> +			fb->base.id, event);
>
>   	if (omap_crtc->old_fb) {
>   		dev_err(dev->dev, "already a pending flip\n");
>   		return -EINVAL;
>   	}
>
> -	omap_crtc->old_fb = crtc->fb;
>   	omap_crtc->event = event;
>   	crtc->fb = fb;
>
> @@ -213,14 +363,244 @@ static const struct drm_crtc_helper_funcs omap_crtc_helper_funcs = {
>   	.load_lut = omap_crtc_load_lut,
>   };
>
> +const struct omap_video_timings *omap_crtc_timings(struct drm_crtc *crtc)
> +{
> +	struct omap_crtc *omap_crtc = to_omap_crtc(crtc);
> +	return &omap_crtc->timings;
> +}
> +
> +enum omap_channel omap_crtc_channel(struct drm_crtc *crtc)
> +{
> +	struct omap_crtc *omap_crtc = to_omap_crtc(crtc);
> +	return omap_crtc->channel;
> +}
> +
> +static void omap_crtc_error_irq(struct omap_drm_irq *irq, uint32_t irqstatus)
> +{
> +	struct omap_crtc *omap_crtc =
> +			container_of(irq, struct omap_crtc, error_irq);
> +	struct drm_crtc *crtc = &omap_crtc->base;
> +	DRM_ERROR("%s: errors: %08x\n", omap_crtc->name, irqstatus);
> +	/* avoid getting in a flood, unregister the irq until next vblank */
> +	omap_irq_unregister(crtc->dev, &omap_crtc->error_irq);
> +}
> +
> +static void omap_crtc_apply_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;
> +
> +	if (!omap_crtc->error_irq.registered)
> +		omap_irq_register(crtc->dev, &omap_crtc->error_irq);
> +
> +	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);
> +	}
> +}
> +
> +static void apply_worker(struct work_struct *work)
> +{
> +	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;
> +
> +	/*
> +	 * Synchronize everything on mode_config.mutex, to keep
> +	 * the callbacks and list modification all serialized
> +	 * with respect to modesetting ioctls from userspace.
> +	 */
> +	mutex_lock(&dev->mode_config.mutex);
> +	dispc_runtime_get();
> +
> +	/*
> +	 * 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);
> +	}
> +
> +	need_apply = !list_empty(&omap_crtc->queued_applies);
> +
> +	/* 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 (need_apply) {
> +		enum omap_channel channel = omap_crtc->channel;
> +
> +		DBG("%s: GO", omap_crtc->name);
> +
> +		if (dispc_mgr_is_enabled(channel)) {
> +			omap_irq_register(dev, &omap_crtc->apply_irq);
> +			dispc_mgr_go(channel);
> +		} else if (!dispc_mgr_go_busy(channel)) {

I'm not clear about this. Why would GO be set in the first place if the 
manager isn't enabled? This could be replaced with a simple 'else'

> +			struct omap_drm_private *priv = dev->dev_private;
> +			queue_work(priv->wq, &omap_crtc->apply_work);
> +		}
> +	}
> +
> +out:
> +	dispc_runtime_put();
> +	mutex_unlock(&dev->mode_config.mutex);
> +}
> +
> +int omap_crtc_apply(struct drm_crtc *crtc,
> +		struct omap_drm_apply *apply)
> +{
> +	struct omap_crtc *omap_crtc = to_omap_crtc(crtc);
> +	struct drm_device *dev = crtc->dev;
> +
> +	WARN_ON(!mutex_is_locked(&dev->mode_config.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);
> +
> +	/*
> +	 * 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);
> +	}
> +
> +	return 0;
> +}
> +
> +/* called only from apply */
> +static void set_enabled(struct drm_crtc *crtc, bool enable)
> +{
> +	struct drm_device *dev = crtc->dev;
> +	struct omap_crtc *omap_crtc = to_omap_crtc(crtc);
> +	enum omap_channel channel = omap_crtc->channel;
> +	struct omap_irq_wait *wait = NULL;
> +
> +	if (dispc_mgr_is_enabled(channel) == enable)
> +		return;
> +
> +	/* ignore sync-lost irqs during enable/disable */
> +	omap_irq_unregister(crtc->dev, &omap_crtc->error_irq);
> +
> +	if (dispc_mgr_get_framedone_irq(channel)) {
> +		if (!enable) {
> +			wait = omap_irq_wait_init(dev,
> +					dispc_mgr_get_framedone_irq(channel), 1);
> +		}
> +	} else {
> +		/*
> +		 * When we disable digit output, we need to wait until fields
> +		 * are done.  Otherwise the DSS is still working, and turning
> +		 * off the clocks prevents DSS from going to OFF mode. And when
> +		 * enabling, we need to wait for the extra sync losts
> +		 */
> +		wait = omap_irq_wait_init(dev,
> +				dispc_mgr_get_vsync_irq(channel), 2);
> +	}
> +
> +	dispc_mgr_enable(channel, enable);
> +
> +	if (wait) {
> +		int ret = omap_irq_wait(dev, wait, msecs_to_jiffies(100));
> +		if (ret) {
> +			dev_err(dev->dev, "%s: timeout waiting for %s\n",
> +					omap_crtc->name, enable ? "enable" : "disable");
> +		}
> +	}
> +
> +	omap_irq_register(crtc->dev, &omap_crtc->error_irq);
> +}
> +
> +static void omap_crtc_pre_apply(struct omap_drm_apply *apply)
> +{
> +	struct omap_crtc *omap_crtc =
> +			container_of(apply, struct omap_crtc, apply);
> +	struct drm_crtc *crtc = &omap_crtc->base;
> +	struct drm_encoder *encoder = NULL;
> +
> +	DBG("%s: enabled=%d, full=%d", omap_crtc->name,
> +			omap_crtc->enabled, omap_crtc->full_update);
> +
> +	if (omap_crtc->full_update) {

If I get it right, full_update refers to manager properties that 
previously used to propogate downstream from the output driver to dispc, 
i.e, things like timings and so on. and the ones which aren't 
full_update are upstream properties like transparency keys, 
default_color and so on?

> +		struct omap_drm_private *priv = crtc->dev->dev_private;
> +		int i;
> +		for (i = 0; i < priv->num_encoders; i++) {
> +			if (priv->encoders[i]->crtc == crtc) {
> +				encoder = priv->encoders[i];
> +				break;
> +			}
> +		}
> +	}
> +
> +	if (!omap_crtc->enabled) {
> +		set_enabled(&omap_crtc->base, false);
> +		if (encoder)
> +			omap_encoder_set_enabled(encoder, false);
> +	} else {
> +		if (encoder) {
> +			omap_encoder_set_enabled(encoder, false);
> +			omap_encoder_update(encoder, &omap_crtc->mgr,
> +					&omap_crtc->timings);
> +			omap_encoder_set_enabled(encoder, true);
> +			omap_crtc->full_update = false;
> +		}
> +
> +		dispc_mgr_setup(omap_crtc->channel, &omap_crtc->info);
> +		dispc_mgr_set_timings(omap_crtc->channel,
> +				&omap_crtc->timings);
> +		set_enabled(&omap_crtc->base, true);
> +	}
> +
> +	omap_crtc->full_update = false;
> +}
> +
><

snip
>
> -static int omap_modeset_init(struct drm_device *dev)
> -{
> -	const struct omap_drm_platform_data *pdata = dev->dev->platform_data;
> -	struct omap_kms_platform_data *kms_pdata = NULL;
> -	struct omap_drm_private *priv = dev->dev_private;
> -	struct omap_dss_device *dssdev = NULL;
> -	int i, j;
> -	unsigned int connected_connectors = 0;
> +		encoder = omap_encoder_init(dev, dssdev);
>
> -	drm_mode_config_init(dev);
> -
> -	if (pdata && pdata->kms_pdata) {
> -		kms_pdata = pdata->kms_pdata;
> -
> -		/* if platform data is provided by the board file, use it to
> -		 * control which overlays, managers, and devices we own.
> -		 */
> -		for (i = 0; i < kms_pdata->mgr_cnt; i++) {
> -			struct omap_overlay_manager *mgr =
> -				omap_dss_get_overlay_manager(
> -						kms_pdata->mgr_ids[i]);
> -			create_encoder(dev, mgr);
> -		}
> -
> -		for (i = 0; i < kms_pdata->dev_cnt; i++) {
> -			struct omap_dss_device *dssdev =
> -				omap_dss_find_device(
> -					(void *)kms_pdata->dev_names[i],
> -					match_dev_name);
> -			if (!dssdev) {
> -				dev_warn(dev->dev, "no such dssdev: %s\n",
> -						kms_pdata->dev_names[i]);
> -				continue;
> -			}
> -			create_connector(dev, dssdev);
> +		if (!encoder) {
> +			dev_err(dev->dev, "could not create encoder: %s\n",
> +					dssdev->name);
> +			return -ENOMEM;
>   		}
>
> -		connected_connectors = detect_connectors(dev);
> +		connector = omap_connector_init(dev,
> +				get_connector_type(dssdev), dssdev, encoder);
>
> -		j = 0;
> -		for (i = 0; i < kms_pdata->ovl_cnt; i++) {
> -			struct omap_overlay *ovl =
> -				omap_dss_get_overlay(kms_pdata->ovl_ids[i]);
> -			create_crtc(dev, ovl, &j, connected_connectors);
> +		if (!connector) {
> +			dev_err(dev->dev, "could not create connector: %s\n",
> +					dssdev->name);
> +			return -ENOMEM;
>   		}
>
> -		for (i = 0; i < kms_pdata->pln_cnt; i++) {
> -			struct omap_overlay *ovl =
> -				omap_dss_get_overlay(kms_pdata->pln_ids[i]);
> -			create_plane(dev, ovl, (1 << priv->num_crtcs) - 1);
> -		}
> -	} else {
> -		/* otherwise just grab up to CONFIG_DRM_OMAP_NUM_CRTCS and try
> -		 * to make educated guesses about everything else
> -		 */
> -		int max_overlays = min(omap_dss_get_num_overlays(), num_crtc);
> +		BUG_ON(priv->num_encoders >= ARRAY_SIZE(priv->encoders));
> +		BUG_ON(priv->num_connectors >= ARRAY_SIZE(priv->connectors));
>
> -		for (i = 0; i < omap_dss_get_num_overlay_managers(); i++)
> -			create_encoder(dev, omap_dss_get_overlay_manager(i));
> -
> -		for_each_dss_dev(dssdev) {
> -			create_connector(dev, dssdev);
> -		}
> +		priv->encoders[priv->num_encoders++] = encoder;
> +		priv->connectors[priv->num_connectors++] = connector;
>
> -		connected_connectors = detect_connectors(dev);
> +		drm_mode_connector_attach_encoder(connector, encoder);
>
> -		j = 0;
> -		for (i = 0; i < max_overlays; i++) {
> -			create_crtc(dev, omap_dss_get_overlay(i),
> -					&j, connected_connectors);
> -		}
> -
> -		/* use any remaining overlays as drm planes */
> -		for (; i < omap_dss_get_num_overlays(); i++) {
> -			struct omap_overlay *ovl = omap_dss_get_overlay(i);
> -			create_plane(dev, ovl, (1 << priv->num_crtcs) - 1);
> +		/* figure out which crtc's we can connect the encoder to: */
> +		encoder->possible_crtcs = 0;
> +		for (id = 0; id < priv->num_crtcs; id++) {
> +			enum omap_display_type supported_displays =
> +					dss_feat_get_supported_displays(pipe2chan(id));

We could use the newer version here: dss_feat_get_supported_outputs(), 
this will tell what all outputs a manager can connect to.

> +			if (supported_displays & dssdev->type)
> +				encoder->possible_crtcs |= (1 << id);
>   		}
>   	}
>
> -	/* for now keep the mapping of CRTCs and encoders static.. */
> -	for (i = 0; i < priv->num_encoders; i++) {
> -		struct drm_encoder *encoder = priv->encoders[i];
> -		struct omap_overlay_manager *mgr =
> -				omap_encoder_get_manager(encoder);
> -
> -		encoder->possible_crtcs = (1 << priv->num_crtcs) - 1;
> -
> -		DBG("%s: possible_crtcs=%08x", mgr->name,
> -					encoder->possible_crtcs);
> -	}
> -
> -	dump_video_chains();
> -
>   	dev->mode_config.min_width = 32;
>   	dev->mode_config.min_height = 32;
>
> @@ -450,7 +229,7 @@ static int ioctl_gem_new(struct drm_device *dev, void *data,
>   		struct drm_file *file_priv)
>   {
>   	struct drm_omap_gem_new *args = data;
> -	DBG("%p:%p: size=0x%08x, flags=%08x", dev, file_priv,
> +	VERB("%p:%p: size=0x%08x, flags=%08x", dev, file_priv,
>   			args->size.bytes, args->flags);
>   	return omap_gem_new_handle(dev, file_priv, args->size,
>   			args->flags, &args->handle);
> @@ -510,7 +289,7 @@ static int ioctl_gem_info(struct drm_device *dev, void *data,
>   	struct drm_gem_object *obj;
>   	int ret = 0;
>
> -	DBG("%p:%p: handle=%d", dev, file_priv, args->handle);
> +	VERB("%p:%p: handle=%d", dev, file_priv, args->handle);
>
>   	obj = drm_gem_object_lookup(dev, file_priv, args->handle);
>   	if (!obj)
> @@ -565,14 +344,6 @@ static int dev_load(struct drm_device *dev, unsigned long flags)
>
>   	dev->dev_private = priv;
>
> -	ret = omapdss_compat_init();
> -	if (ret) {
> -		dev_err(dev->dev, "coult not init omapdss\n");
> -		dev->dev_private = NULL;
> -		kfree(priv);
> -		return ret;
> -	}
> -

How can omapdss_compat_init/ininit already exist in the driver, and 
removed in this patch?

<snip>
Archit
Archit Taneja Nov. 16, 2012, 6:53 a.m. UTC | #2 
On Friday 16 November 2012 12:14 PM, Archit Taneja wrote:
> On Friday 16 November 2012 05:30 AM, Rob Clark wrote:
      if (!obj)
>> @@ -565,14 +344,6 @@ static int dev_load(struct drm_device *dev,
>> unsigned long flags)
>>
>>       dev->dev_private = priv;
>>
>> -    ret = omapdss_compat_init();
>> -    if (ret) {
>> -        dev_err(dev->dev, "coult not init omapdss\n");
>> -        dev->dev_private = NULL;
>> -        kfree(priv);
>> -        return ret;
>> -    }
>> -
>
> How can omapdss_compat_init/ininit already exist in the driver, and
> removed in this patch?

Had a look at Tomi's series, please ignore this one.

Archit
Greg KH Nov. 16, 2012, 12:19 p.m. UTC | #3 
On Thu, Nov 15, 2012 at 06:00:58PM -0600, Rob Clark wrote:
> This patch changes the omapdrm KMS to bypass the omapdss "compat"
> layer and use the core omapdss API directly.  This solves some layering
> issues that would cause unpin confusion vs GO bit status, because we
> would not know whether a particular pageflip or overlay update has hit
> the screen or not.  Now instead we explicitly manage the GO bits in
> dispc and handle the vblank/framedone interrupts ourself so that we
> always know which buffers are being scanned out at any given time, and
> so on.
> 
> As an added bonus, we no longer leave the last overlay buffer pinned
> when the display is disabled, and have been able to add the previously
> missing vblank event handling.
> 
> Signed-off-by: Rob Clark <robdclark@gmail.com>
> ---
> Note: this patch applies on top of staging-next plus the "OMAPDSS:
> create compat layer" patch series:
> 
> http://comments.gmane.org/gmane.linux.ports.arm.omap/89435

Hm, I can't take that patch set in staging-next, so how should this go
in?

thanks,

greg k-h
Rob Clark Nov. 16, 2012, 1:59 p.m. UTC | #4 
On Fri, Nov 16, 2012 at 12:44 AM, Archit Taneja <archit@ti.com> wrote:
> On Friday 16 November 2012 05:30 AM, Rob Clark wrote:
>>
>> +static void omap_crtc_set_lcd_config(struct omap_overlay_manager *mgr,
>> +               const struct dss_lcd_mgr_config *config)
>> +{
>> +       struct omap_crtc *omap_crtc = container_of(mgr, struct omap_crtc,
>> mgr);
>> +       DBG("%s", omap_crtc->name);
>> +       dispc_mgr_set_lcd_config(omap_crtc->channel, config);
>
>
> Maybe you should move this dispc write also to omap_crtc_pre_apply, the same
> way it's done for timings. We'll also be confident about having the clocks
> required if this is called in pre_apply.
>

That was the first thing I had tried, although the order doesn't
really work out well and lcd config ends up getting set on the n+1'th
apply.  Currently this is only called indirectly from
omap_encoder_set_enabled() -> dssdrv->enable() which always happens on
apply worker.  In fact no dispc or dssdev fxn that changes state is
called outside of apply worker.  (Only things like detect and reading
edid are happening outside of apply worker.)

When we get to the point of having more sophisticated panel drivers,
these callbacks from the panel are probably going to need some sort of
if (is_in_apply_worker()) sort of check.


>> +static void apply_worker(struct work_struct *work)
>> +{
>> +       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;
>> +
>> +       /*
>> +        * Synchronize everything on mode_config.mutex, to keep
>> +        * the callbacks and list modification all serialized
>> +        * with respect to modesetting ioctls from userspace.
>> +        */
>> +       mutex_lock(&dev->mode_config.mutex);
>> +       dispc_runtime_get();
>> +
>> +       /*
>> +        * 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);
>> +       }
>> +
>> +       need_apply = !list_empty(&omap_crtc->queued_applies);
>> +
>> +       /* 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 (need_apply) {
>> +               enum omap_channel channel = omap_crtc->channel;
>> +
>> +               DBG("%s: GO", omap_crtc->name);
>> +
>> +               if (dispc_mgr_is_enabled(channel)) {
>> +                       omap_irq_register(dev, &omap_crtc->apply_irq);
>> +                       dispc_mgr_go(channel);
>> +               } else if (!dispc_mgr_go_busy(channel)) {
>
>
> I'm not clear about this. Why would GO be set in the first place if the
> manager isn't enabled? This could be replaced with a simple 'else'
>

Yeah, that extra if should be redundant

>
>> +static void omap_crtc_pre_apply(struct omap_drm_apply *apply)
>> +{
>> +       struct omap_crtc *omap_crtc =
>> +                       container_of(apply, struct omap_crtc, apply);
>> +       struct drm_crtc *crtc = &omap_crtc->base;
>> +       struct drm_encoder *encoder = NULL;
>> +
>> +       DBG("%s: enabled=%d, full=%d", omap_crtc->name,
>> +                       omap_crtc->enabled, omap_crtc->full_update);
>> +
>> +       if (omap_crtc->full_update) {
>
>
> If I get it right, full_update refers to manager properties that previously
> used to propogate downstream from the output driver to dispc, i.e, things
> like timings and so on. and the ones which aren't full_update are upstream
> properties like transparency keys, default_color and so on?
>

Well, it basically means power or timings have changed.  So it means
closer to "full modeset" vs "pageflip".

But this function does deal with one mismatch between DRM and DSS.. in
DRM, everything gets setup in crtc -> downstream order, whereas
omapdss to accommodate more sophisticated panels does things in the
reverse order.  So the crtc here propagates timing/power state change
to the encoder (which may turn into mgr op callbacks), rather than
relying on the encoder-helper fxn ptrs called from KMS.

And in fact other DRM drivers will need the same thing eventually if
they are to support the common panel/display framework.  So I think
eventually a new / alternate set of crtc helpers (at least
drm_crtc_helper_set_{config,mode})()) will be needed.  But I think it
would be easier to introduce after the atomic modeset changes.

>
>>
>> -static int omap_modeset_init(struct drm_device *dev)
>> -{
>> -       const struct omap_drm_platform_data *pdata =
>> dev->dev->platform_data;
>> -       struct omap_kms_platform_data *kms_pdata = NULL;
>> -       struct omap_drm_private *priv = dev->dev_private;
>> -       struct omap_dss_device *dssdev = NULL;
>> -       int i, j;
>> -       unsigned int connected_connectors = 0;
>> +               encoder = omap_encoder_init(dev, dssdev);
>>
>> -       drm_mode_config_init(dev);
>> -
>> -       if (pdata && pdata->kms_pdata) {
>> -               kms_pdata = pdata->kms_pdata;
>> -
>> -               /* if platform data is provided by the board file, use it
>> to
>> -                * control which overlays, managers, and devices we own.
>> -                */
>> -               for (i = 0; i < kms_pdata->mgr_cnt; i++) {
>> -                       struct omap_overlay_manager *mgr =
>> -                               omap_dss_get_overlay_manager(
>> -                                               kms_pdata->mgr_ids[i]);
>> -                       create_encoder(dev, mgr);
>> -               }
>> -
>> -               for (i = 0; i < kms_pdata->dev_cnt; i++) {
>> -                       struct omap_dss_device *dssdev =
>> -                               omap_dss_find_device(
>> -                                       (void *)kms_pdata->dev_names[i],
>> -                                       match_dev_name);
>> -                       if (!dssdev) {
>> -                               dev_warn(dev->dev, "no such dssdev: %s\n",
>> -                                               kms_pdata->dev_names[i]);
>> -                               continue;
>> -                       }
>> -                       create_connector(dev, dssdev);
>> +               if (!encoder) {
>> +                       dev_err(dev->dev, "could not create encoder:
>> %s\n",
>> +                                       dssdev->name);
>> +                       return -ENOMEM;
>>                 }
>>
>> -               connected_connectors = detect_connectors(dev);
>> +               connector = omap_connector_init(dev,
>> +                               get_connector_type(dssdev), dssdev,
>> encoder);
>>
>> -               j = 0;
>> -               for (i = 0; i < kms_pdata->ovl_cnt; i++) {
>> -                       struct omap_overlay *ovl =
>> -
>> omap_dss_get_overlay(kms_pdata->ovl_ids[i]);
>> -                       create_crtc(dev, ovl, &j, connected_connectors);
>> +               if (!connector) {
>> +                       dev_err(dev->dev, "could not create connector:
>> %s\n",
>> +                                       dssdev->name);
>> +                       return -ENOMEM;
>>                 }
>>
>> -               for (i = 0; i < kms_pdata->pln_cnt; i++) {
>> -                       struct omap_overlay *ovl =
>> -
>> omap_dss_get_overlay(kms_pdata->pln_ids[i]);
>> -                       create_plane(dev, ovl, (1 << priv->num_crtcs) -
>> 1);
>> -               }
>> -       } else {
>> -               /* otherwise just grab up to CONFIG_DRM_OMAP_NUM_CRTCS and
>> try
>> -                * to make educated guesses about everything else
>> -                */
>> -               int max_overlays = min(omap_dss_get_num_overlays(),
>> num_crtc);
>> +               BUG_ON(priv->num_encoders >= ARRAY_SIZE(priv->encoders));
>> +               BUG_ON(priv->num_connectors >=
>> ARRAY_SIZE(priv->connectors));
>>
>> -               for (i = 0; i < omap_dss_get_num_overlay_managers(); i++)
>> -                       create_encoder(dev,
>> omap_dss_get_overlay_manager(i));
>> -
>> -               for_each_dss_dev(dssdev) {
>> -                       create_connector(dev, dssdev);
>> -               }
>> +               priv->encoders[priv->num_encoders++] = encoder;
>> +               priv->connectors[priv->num_connectors++] = connector;
>>
>> -               connected_connectors = detect_connectors(dev);
>> +               drm_mode_connector_attach_encoder(connector, encoder);
>>
>> -               j = 0;
>> -               for (i = 0; i < max_overlays; i++) {
>> -                       create_crtc(dev, omap_dss_get_overlay(i),
>> -                                       &j, connected_connectors);
>> -               }
>> -
>> -               /* use any remaining overlays as drm planes */
>> -               for (; i < omap_dss_get_num_overlays(); i++) {
>> -                       struct omap_overlay *ovl =
>> omap_dss_get_overlay(i);
>> -                       create_plane(dev, ovl, (1 << priv->num_crtcs) -
>> 1);
>> +               /* figure out which crtc's we can connect the encoder to:
>> */
>> +               encoder->possible_crtcs = 0;
>> +               for (id = 0; id < priv->num_crtcs; id++) {
>> +                       enum omap_display_type supported_displays =
>> +
>> dss_feat_get_supported_displays(pipe2chan(id));
>
>
> We could use the newer version here: dss_feat_get_supported_outputs(), this
> will tell what all outputs a manager can connect to.

ok, I guess I started on this before that fxn existed

BR,
-R
Tomi Valkeinen Nov. 19, 2012, 8:10 a.m. UTC | #5 
On 2012-11-16 14:19, Greg KH wrote:
> On Thu, Nov 15, 2012 at 06:00:58PM -0600, Rob Clark wrote:
>> This patch changes the omapdrm KMS to bypass the omapdss "compat"
>> layer and use the core omapdss API directly.  This solves some layering
>> issues that would cause unpin confusion vs GO bit status, because we
>> would not know whether a particular pageflip or overlay update has hit
>> the screen or not.  Now instead we explicitly manage the GO bits in
>> dispc and handle the vblank/framedone interrupts ourself so that we
>> always know which buffers are being scanned out at any given time, and
>> so on.
>>
>> As an added bonus, we no longer leave the last overlay buffer pinned
>> when the display is disabled, and have been able to add the previously
>> missing vblank event handling.
>>
>> Signed-off-by: Rob Clark <robdclark@gmail.com>
>> ---
>> Note: this patch applies on top of staging-next plus the "OMAPDSS:
>> create compat layer" patch series:
>>
>> http://comments.gmane.org/gmane.linux.ports.arm.omap/89435
> 
> Hm, I can't take that patch set in staging-next, so how should this go
> in?

I wonder, could the omapdrm part go in for -rc2? It's "only" a driver in
staging. I think that would be the easiest way. Perhaps it'd be even
possible to split the patch into two, of which the first half would not
depend on omapdss, but would prepare omapdrm for the change, thus making
the patch for -rc2 smaller.

Otherwise it's more tricky... Either wait for 3.9, or get omapdss,
omapfb and omapdrm merged via the same tree. But which that would be?
omapdss and omapfb usually go through fbdev tree. I don't know what its
maintainer thinks of merging drm driver. And if the omapdrm depends on
new drm changes, I guess fbdev is out of the question.

 Tomi
diff mbox

Patch

diff --git a/drivers/staging/omapdrm/Makefile b/drivers/staging/omapdrm/Makefile
index 1ca0e00..d85e058 100644
--- a/drivers/staging/omapdrm/Makefile
+++ b/drivers/staging/omapdrm/Makefile
@@ -5,6 +5,7 @@ 
 
 ccflags-y := -Iinclude/drm -Werror
 omapdrm-y := omap_drv.o \
+	omap_irq.o \
 	omap_debugfs.o \
 	omap_crtc.o \
 	omap_plane.o \
diff --git a/drivers/staging/omapdrm/TODO b/drivers/staging/omapdrm/TODO
index 938c788..abeeb00 100644
--- a/drivers/staging/omapdrm/TODO
+++ b/drivers/staging/omapdrm/TODO
@@ -17,9 +17,6 @@  TODO
 . Revisit GEM sync object infrastructure.. TTM has some framework for this
   already.  Possibly this could be refactored out and made more common?
   There should be some way to do this with less wheel-reinvention.
-. Review DSS vs KMS mismatches.  The omap_dss_device is sort of part encoder,
-  part connector.  Which results in a bit of duct tape to fwd calls from
-  encoder to connector.  Possibly this could be done a bit better.
 . Solve PM sequencing on resume.  DMM/TILER must be reloaded before any
   access is made from any component in the system.  Which means on suspend
   CRTC's should be disabled, and on resume the LUT should be reprogrammed
diff --git a/drivers/staging/omapdrm/omap_connector.c b/drivers/staging/omapdrm/omap_connector.c
index 91edb3f..4cc9ee7 100644
--- a/drivers/staging/omapdrm/omap_connector.c
+++ b/drivers/staging/omapdrm/omap_connector.c
@@ -31,9 +31,10 @@ 
 struct omap_connector {
 	struct drm_connector base;
 	struct omap_dss_device *dssdev;
+	struct drm_encoder *encoder;
 };
 
-static inline void copy_timings_omap_to_drm(struct drm_display_mode *mode,
+void copy_timings_omap_to_drm(struct drm_display_mode *mode,
 		struct omap_video_timings *timings)
 {
 	mode->clock = timings->pixel_clock;
@@ -64,7 +65,7 @@  static inline void copy_timings_omap_to_drm(struct drm_display_mode *mode,
 		mode->flags |= DRM_MODE_FLAG_NVSYNC;
 }
 
-static inline void copy_timings_drm_to_omap(struct omap_video_timings *timings,
+void copy_timings_drm_to_omap(struct omap_video_timings *timings,
 		struct drm_display_mode *mode)
 {
 	timings->pixel_clock = mode->clock;
@@ -96,48 +97,7 @@  static inline void copy_timings_drm_to_omap(struct omap_video_timings *timings,
 	timings->sync_pclk_edge = OMAPDSS_DRIVE_SIG_OPPOSITE_EDGES;
 }
 
-static void omap_connector_dpms(struct drm_connector *connector, int mode)
-{
-	struct omap_connector *omap_connector = to_omap_connector(connector);
-	struct omap_dss_device *dssdev = omap_connector->dssdev;
-	int old_dpms;
-
-	DBG("%s: %d", dssdev->name, mode);
-
-	old_dpms = connector->dpms;
-
-	/* from off to on, do from crtc to connector */
-	if (mode < old_dpms)
-		drm_helper_connector_dpms(connector, mode);
-
-	if (mode == DRM_MODE_DPMS_ON) {
-		/* store resume info for suspended displays */
-		switch (dssdev->state) {
-		case OMAP_DSS_DISPLAY_SUSPENDED:
-			dssdev->activate_after_resume = true;
-			break;
-		case OMAP_DSS_DISPLAY_DISABLED: {
-			int ret = dssdev->driver->enable(dssdev);
-			if (ret) {
-				DBG("%s: failed to enable: %d",
-						dssdev->name, ret);
-				dssdev->driver->disable(dssdev);
-			}
-			break;
-		}
-		default:
-			break;
-		}
-	} else {
-		/* TODO */
-	}
-
-	/* from on to off, do from connector to crtc */
-	if (mode > old_dpms)
-		drm_helper_connector_dpms(connector, mode);
-}
-
-enum drm_connector_status omap_connector_detect(
+static enum drm_connector_status omap_connector_detect(
 		struct drm_connector *connector, bool force)
 {
 	struct omap_connector *omap_connector = to_omap_connector(connector);
@@ -164,8 +124,6 @@  static void omap_connector_destroy(struct drm_connector *connector)
 	struct omap_connector *omap_connector = to_omap_connector(connector);
 	struct omap_dss_device *dssdev = omap_connector->dssdev;
 
-	dssdev->driver->disable(dssdev);
-
 	DBG("%s", omap_connector->dssdev->name);
 	drm_sysfs_connector_remove(connector);
 	drm_connector_cleanup(connector);
@@ -261,36 +219,12 @@  static int omap_connector_mode_valid(struct drm_connector *connector,
 struct drm_encoder *omap_connector_attached_encoder(
 		struct drm_connector *connector)
 {
-	int i;
 	struct omap_connector *omap_connector = to_omap_connector(connector);
-
-	for (i = 0; i < DRM_CONNECTOR_MAX_ENCODER; i++) {
-		struct drm_mode_object *obj;
-
-		if (connector->encoder_ids[i] == 0)
-			break;
-
-		obj = drm_mode_object_find(connector->dev,
-				connector->encoder_ids[i],
-				DRM_MODE_OBJECT_ENCODER);
-
-		if (obj) {
-			struct drm_encoder *encoder = obj_to_encoder(obj);
-			struct omap_overlay_manager *mgr =
-					omap_encoder_get_manager(encoder);
-			DBG("%s: found %s", omap_connector->dssdev->name,
-					mgr->name);
-			return encoder;
-		}
-	}
-
-	DBG("%s: no encoder", omap_connector->dssdev->name);
-
-	return NULL;
+	return omap_connector->encoder;
 }
 
 static const struct drm_connector_funcs omap_connector_funcs = {
-	.dpms = omap_connector_dpms,
+	.dpms = drm_helper_connector_dpms,
 	.detect = omap_connector_detect,
 	.fill_modes = drm_helper_probe_single_connector_modes,
 	.destroy = omap_connector_destroy,
@@ -302,34 +236,6 @@  static const struct drm_connector_helper_funcs omap_connector_helper_funcs = {
 	.best_encoder = omap_connector_attached_encoder,
 };
 
-/* called from encoder when mode is set, to propagate settings to the dssdev */
-void omap_connector_mode_set(struct drm_connector *connector,
-		struct drm_display_mode *mode)
-{
-	struct drm_device *dev = connector->dev;
-	struct omap_connector *omap_connector = to_omap_connector(connector);
-	struct omap_dss_device *dssdev = omap_connector->dssdev;
-	struct omap_dss_driver *dssdrv = dssdev->driver;
-	struct omap_video_timings timings = {0};
-
-	copy_timings_drm_to_omap(&timings, mode);
-
-	DBG("%s: set mode: %d:\"%s\" %d %d %d %d %d %d %d %d %d %d 0x%x 0x%x",
-			omap_connector->dssdev->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);
-
-	if (dssdrv->check_timings(dssdev, &timings)) {
-		dev_err(dev->dev, "could not set timings\n");
-		return;
-	}
-
-	dssdrv->set_timings(dssdev, &timings);
-}
-
 /* flush an area of the framebuffer (in case of manual update display that
  * is not automatically flushed)
  */
@@ -344,7 +250,8 @@  void omap_connector_flush(struct drm_connector *connector,
 
 /* initialize connector */
 struct drm_connector *omap_connector_init(struct drm_device *dev,
-		int connector_type, struct omap_dss_device *dssdev)
+		int connector_type, struct omap_dss_device *dssdev,
+		struct drm_encoder *encoder)
 {
 	struct drm_connector *connector = NULL;
 	struct omap_connector *omap_connector;
@@ -360,6 +267,8 @@  struct drm_connector *omap_connector_init(struct drm_device *dev,
 	}
 
 	omap_connector->dssdev = dssdev;
+	omap_connector->encoder = encoder;
+
 	connector = &omap_connector->base;
 
 	drm_connector_init(dev, connector, &omap_connector_funcs,
diff --git a/drivers/staging/omapdrm/omap_crtc.c b/drivers/staging/omapdrm/omap_crtc.c
index 3e2c736..e3496e5 100644
--- a/drivers/staging/omapdrm/omap_crtc.c
+++ b/drivers/staging/omapdrm/omap_crtc.c
@@ -28,19 +28,131 @@ 
 struct omap_crtc {
 	struct drm_crtc base;
 	struct drm_plane *plane;
+
 	const char *name;
-	int id;
+	int pipe;
+	enum omap_channel channel;
+	struct omap_overlay_manager_info info;
+
+	/*
+	 * Temporary: eventually this will go away, but it is needed
+	 * for now to keep the output's happy.  (They only need
+	 * mgr->id.)  Eventually this will be replaced w/ something
+	 * more common-panel-framework-y
+	 */
+	struct omap_overlay_manager mgr;
+
+	struct omap_video_timings timings;
+	bool enabled;
+	bool full_update;
+
+	struct omap_drm_apply apply;
+
+	struct omap_drm_irq apply_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;
 
 	/* if there is a pending flip, these will be non-null: */
 	struct drm_pending_vblank_event *event;
 	struct drm_framebuffer *old_fb;
+
+	/* 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;
 };
 
+/*
+ * Manager-ops, callbacks from output when they need to configure
+ * the upstream part of the video pipe.
+ *
+ * Most of these we can ignore until we add support for command-mode
+ * panels.. for video-mode the crtc-helpers already do an adequate
+ * job of sequencing the setup of the video pipe in the proper order
+ */
+
+/* we can probably ignore these until we support command-mode panels: */
+static void omap_crtc_start_update(struct omap_overlay_manager *mgr)
+{
+}
+
+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_set_timings(struct omap_overlay_manager *mgr,
+		const struct omap_video_timings *timings)
+{
+	struct omap_crtc *omap_crtc = container_of(mgr, struct omap_crtc, mgr);
+	DBG("%s", omap_crtc->name);
+	omap_crtc->timings = *timings;
+	omap_crtc->full_update = true;
+}
+
+static void omap_crtc_set_lcd_config(struct omap_overlay_manager *mgr,
+		const struct dss_lcd_mgr_config *config)
+{
+	struct omap_crtc *omap_crtc = container_of(mgr, struct omap_crtc, mgr);
+	DBG("%s", omap_crtc->name);
+	dispc_mgr_set_lcd_config(omap_crtc->channel, config);
+}
+
+static int omap_crtc_register_framedone_handler(
+		struct omap_overlay_manager *mgr,
+		void (*handler)(void *), void *data)
+{
+	return 0;
+}
+
+static void omap_crtc_unregister_framedone_handler(
+		struct omap_overlay_manager *mgr,
+		void (*handler)(void *), void *data)
+{
+}
+
+static const struct dss_mgr_ops mgr_ops = {
+		.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,
+};
+
+/*
+ * CRTC funcs:
+ */
+
 static void omap_crtc_destroy(struct drm_crtc *crtc)
 {
 	struct omap_crtc *omap_crtc = to_omap_crtc(crtc);
+
+	DBG("%s", omap_crtc->name);
+
+	WARN_ON(omap_crtc->apply_irq.registered);
+	omap_irq_unregister(crtc->dev, &omap_crtc->error_irq);
+
 	omap_crtc->plane->funcs->destroy(omap_crtc->plane);
 	drm_crtc_cleanup(crtc);
+
 	kfree(omap_crtc);
 }
 
@@ -48,14 +160,25 @@  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);
 	int i;
 
-	WARN_ON(omap_plane_dpms(omap_crtc->plane, mode));
+	DBG("%s: %d", omap_crtc->name, mode);
+
+	if (enabled != omap_crtc->enabled) {
+		omap_crtc->enabled = enabled;
+		omap_crtc->full_update = true;
+		omap_crtc_apply(crtc, &omap_crtc->apply);
 
-	for (i = 0; i < priv->num_planes; i++) {
-		struct drm_plane *plane = priv->planes[i];
-		if (plane->crtc == crtc)
-			WARN_ON(omap_plane_dpms(plane, mode));
+		/* also enable our private plane: */
+		WARN_ON(omap_plane_dpms(omap_crtc->plane, mode));
+
+		/* and any attached overlay planes: */
+		for (i = 0; i < priv->num_planes; i++) {
+			struct drm_plane *plane = priv->planes[i];
+			if (plane->crtc == crtc)
+				WARN_ON(omap_plane_dpms(plane, mode));
+		}
 	}
 }
 
@@ -73,12 +196,26 @@  static int omap_crtc_mode_set(struct drm_crtc *crtc,
 		struct drm_framebuffer *old_fb)
 {
 	struct omap_crtc *omap_crtc = to_omap_crtc(crtc);
-	struct drm_plane *plane = omap_crtc->plane;
 
-	return omap_plane_mode_set(plane, crtc, crtc->fb,
+	mode = 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);
+
+	copy_timings_drm_to_omap(&omap_crtc->timings, mode);
+	omap_crtc->full_update = true;
+
+	return omap_plane_mode_set(omap_crtc->plane, crtc, crtc->fb,
 			0, 0, mode->hdisplay, mode->vdisplay,
 			x << 16, y << 16,
-			mode->hdisplay << 16, mode->vdisplay << 16);
+			mode->hdisplay << 16, mode->vdisplay << 16,
+			NULL, NULL);
 }
 
 static void omap_crtc_prepare(struct drm_crtc *crtc)
@@ -102,10 +239,11 @@  static int omap_crtc_mode_set_base(struct drm_crtc *crtc, int x, int y,
 	struct drm_plane *plane = omap_crtc->plane;
 	struct drm_display_mode *mode = &crtc->mode;
 
-	return plane->funcs->update_plane(plane, crtc, crtc->fb,
+	return omap_plane_mode_set(plane, crtc, crtc->fb,
 			0, 0, mode->hdisplay, mode->vdisplay,
 			x << 16, y << 16,
-			mode->hdisplay << 16, mode->vdisplay << 16);
+			mode->hdisplay << 16, mode->vdisplay << 16,
+			NULL, NULL);
 }
 
 static void omap_crtc_load_lut(struct drm_crtc *crtc)
@@ -123,7 +261,7 @@  static void vblank_cb(void *arg)
 
 	/* wakeup userspace */
 	if (omap_crtc->event)
-		drm_send_vblank_event(dev, -1, omap_crtc->event);
+		drm_send_vblank_event(dev, omap_crtc->pipe, omap_crtc->event);
 
 	omap_crtc->event = NULL;
 	omap_crtc->old_fb = NULL;
@@ -131,25 +269,37 @@  static void vblank_cb(void *arg)
 	spin_unlock_irqrestore(&dev->event_lock, flags);
 }
 
-static void page_flip_cb(void *arg)
+static void page_flip_worker(struct work_struct *work)
 {
-	struct drm_crtc *crtc = arg;
-	struct omap_crtc *omap_crtc = to_omap_crtc(crtc);
-	struct drm_framebuffer *old_fb = omap_crtc->old_fb;
+	struct omap_crtc *omap_crtc =
+			container_of(work, struct omap_crtc, page_flip_work);
+	struct drm_crtc *crtc = &omap_crtc->base;
+	struct drm_device *dev = crtc->dev;
+	struct drm_display_mode *mode = &crtc->mode;
 	struct drm_gem_object *bo;
 
-	omap_crtc_mode_set_base(crtc, crtc->x, crtc->y, old_fb);
-
-	/* really we'd like to setup the callback atomically w/ setting the
-	 * new scanout buffer to avoid getting stuck waiting an extra vblank
-	 * cycle.. for now go for correctness and later figure out speed..
-	 */
-	omap_plane_on_endwin(omap_crtc->plane, vblank_cb, crtc);
+	mutex_lock(&dev->mode_config.mutex);
+	omap_plane_mode_set(omap_crtc->plane, crtc, crtc->fb,
+			0, 0, mode->hdisplay, mode->vdisplay,
+			crtc->x << 16, crtc->y << 16,
+			mode->hdisplay << 16, mode->vdisplay << 16,
+			vblank_cb, crtc);
+	mutex_unlock(&dev->mode_config.mutex);
 
 	bo = omap_framebuffer_bo(crtc->fb, 0);
 	drm_gem_object_unreference_unlocked(bo);
 }
 
+static void page_flip_cb(void *arg)
+{
+	struct drm_crtc *crtc = arg;
+	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->page_flip_work);
+}
+
 static int omap_crtc_page_flip_locked(struct drm_crtc *crtc,
 		 struct drm_framebuffer *fb,
 		 struct drm_pending_vblank_event *event)
@@ -158,14 +308,14 @@  static int omap_crtc_page_flip_locked(struct drm_crtc *crtc,
 	struct omap_crtc *omap_crtc = to_omap_crtc(crtc);
 	struct drm_gem_object *bo;
 
-	DBG("%d -> %d", crtc->fb ? crtc->fb->base.id : -1, fb->base.id);
+	DBG("%d -> %d (event=%p)", crtc->fb ? crtc->fb->base.id : -1,
+			fb->base.id, event);
 
 	if (omap_crtc->old_fb) {
 		dev_err(dev->dev, "already a pending flip\n");
 		return -EINVAL;
 	}
 
-	omap_crtc->old_fb = crtc->fb;
 	omap_crtc->event = event;
 	crtc->fb = fb;
 
@@ -213,14 +363,244 @@  static const struct drm_crtc_helper_funcs omap_crtc_helper_funcs = {
 	.load_lut = omap_crtc_load_lut,
 };
 
+const struct omap_video_timings *omap_crtc_timings(struct drm_crtc *crtc)
+{
+	struct omap_crtc *omap_crtc = to_omap_crtc(crtc);
+	return &omap_crtc->timings;
+}
+
+enum omap_channel omap_crtc_channel(struct drm_crtc *crtc)
+{
+	struct omap_crtc *omap_crtc = to_omap_crtc(crtc);
+	return omap_crtc->channel;
+}
+
+static void omap_crtc_error_irq(struct omap_drm_irq *irq, uint32_t irqstatus)
+{
+	struct omap_crtc *omap_crtc =
+			container_of(irq, struct omap_crtc, error_irq);
+	struct drm_crtc *crtc = &omap_crtc->base;
+	DRM_ERROR("%s: errors: %08x\n", omap_crtc->name, irqstatus);
+	/* avoid getting in a flood, unregister the irq until next vblank */
+	omap_irq_unregister(crtc->dev, &omap_crtc->error_irq);
+}
+
+static void omap_crtc_apply_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;
+
+	if (!omap_crtc->error_irq.registered)
+		omap_irq_register(crtc->dev, &omap_crtc->error_irq);
+
+	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);
+	}
+}
+
+static void apply_worker(struct work_struct *work)
+{
+	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;
+
+	/*
+	 * Synchronize everything on mode_config.mutex, to keep
+	 * the callbacks and list modification all serialized
+	 * with respect to modesetting ioctls from userspace.
+	 */
+	mutex_lock(&dev->mode_config.mutex);
+	dispc_runtime_get();
+
+	/*
+	 * 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);
+	}
+
+	need_apply = !list_empty(&omap_crtc->queued_applies);
+
+	/* 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 (need_apply) {
+		enum omap_channel channel = omap_crtc->channel;
+
+		DBG("%s: GO", omap_crtc->name);
+
+		if (dispc_mgr_is_enabled(channel)) {
+			omap_irq_register(dev, &omap_crtc->apply_irq);
+			dispc_mgr_go(channel);
+		} else if (!dispc_mgr_go_busy(channel)) {
+			struct omap_drm_private *priv = dev->dev_private;
+			queue_work(priv->wq, &omap_crtc->apply_work);
+		}
+	}
+
+out:
+	dispc_runtime_put();
+	mutex_unlock(&dev->mode_config.mutex);
+}
+
+int omap_crtc_apply(struct drm_crtc *crtc,
+		struct omap_drm_apply *apply)
+{
+	struct omap_crtc *omap_crtc = to_omap_crtc(crtc);
+	struct drm_device *dev = crtc->dev;
+
+	WARN_ON(!mutex_is_locked(&dev->mode_config.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);
+
+	/*
+	 * 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);
+	}
+
+	return 0;
+}
+
+/* called only from apply */
+static void set_enabled(struct drm_crtc *crtc, bool enable)
+{
+	struct drm_device *dev = crtc->dev;
+	struct omap_crtc *omap_crtc = to_omap_crtc(crtc);
+	enum omap_channel channel = omap_crtc->channel;
+	struct omap_irq_wait *wait = NULL;
+
+	if (dispc_mgr_is_enabled(channel) == enable)
+		return;
+
+	/* ignore sync-lost irqs during enable/disable */
+	omap_irq_unregister(crtc->dev, &omap_crtc->error_irq);
+
+	if (dispc_mgr_get_framedone_irq(channel)) {
+		if (!enable) {
+			wait = omap_irq_wait_init(dev,
+					dispc_mgr_get_framedone_irq(channel), 1);
+		}
+	} else {
+		/*
+		 * When we disable digit output, we need to wait until fields
+		 * are done.  Otherwise the DSS is still working, and turning
+		 * off the clocks prevents DSS from going to OFF mode. And when
+		 * enabling, we need to wait for the extra sync losts
+		 */
+		wait = omap_irq_wait_init(dev,
+				dispc_mgr_get_vsync_irq(channel), 2);
+	}
+
+	dispc_mgr_enable(channel, enable);
+
+	if (wait) {
+		int ret = omap_irq_wait(dev, wait, msecs_to_jiffies(100));
+		if (ret) {
+			dev_err(dev->dev, "%s: timeout waiting for %s\n",
+					omap_crtc->name, enable ? "enable" : "disable");
+		}
+	}
+
+	omap_irq_register(crtc->dev, &omap_crtc->error_irq);
+}
+
+static void omap_crtc_pre_apply(struct omap_drm_apply *apply)
+{
+	struct omap_crtc *omap_crtc =
+			container_of(apply, struct omap_crtc, apply);
+	struct drm_crtc *crtc = &omap_crtc->base;
+	struct drm_encoder *encoder = NULL;
+
+	DBG("%s: enabled=%d, full=%d", omap_crtc->name,
+			omap_crtc->enabled, omap_crtc->full_update);
+
+	if (omap_crtc->full_update) {
+		struct omap_drm_private *priv = crtc->dev->dev_private;
+		int i;
+		for (i = 0; i < priv->num_encoders; i++) {
+			if (priv->encoders[i]->crtc == crtc) {
+				encoder = priv->encoders[i];
+				break;
+			}
+		}
+	}
+
+	if (!omap_crtc->enabled) {
+		set_enabled(&omap_crtc->base, false);
+		if (encoder)
+			omap_encoder_set_enabled(encoder, false);
+	} else {
+		if (encoder) {
+			omap_encoder_set_enabled(encoder, false);
+			omap_encoder_update(encoder, &omap_crtc->mgr,
+					&omap_crtc->timings);
+			omap_encoder_set_enabled(encoder, true);
+			omap_crtc->full_update = false;
+		}
+
+		dispc_mgr_setup(omap_crtc->channel, &omap_crtc->info);
+		dispc_mgr_set_timings(omap_crtc->channel,
+				&omap_crtc->timings);
+		set_enabled(&omap_crtc->base, true);
+	}
+
+	omap_crtc->full_update = false;
+}
+
+static void omap_crtc_post_apply(struct omap_drm_apply *apply)
+{
+	/* nothing needed for post-apply */
+}
+
+static const char *channel_names[] = {
+		[OMAP_DSS_CHANNEL_LCD] = "lcd",
+		[OMAP_DSS_CHANNEL_DIGIT] = "tv",
+		[OMAP_DSS_CHANNEL_LCD2] = "lcd2",
+};
+
 /* initialize crtc */
 struct drm_crtc *omap_crtc_init(struct drm_device *dev,
-		struct omap_overlay *ovl, int id)
+		struct drm_plane *plane, enum omap_channel channel, int id)
 {
 	struct drm_crtc *crtc = NULL;
-	struct omap_crtc *omap_crtc = kzalloc(sizeof(*omap_crtc), GFP_KERNEL);
+	struct omap_crtc *omap_crtc;
+	struct omap_overlay_manager_info *info;
 
-	DBG("%s", ovl->name);
+	DBG("%s", channel_names[channel]);
+
+	omap_crtc = kzalloc(sizeof(*omap_crtc), GFP_KERNEL);
 
 	if (!omap_crtc) {
 		dev_err(dev->dev, "could not allocate CRTC\n");
@@ -229,10 +609,40 @@  struct drm_crtc *omap_crtc_init(struct drm_device *dev,
 
 	crtc = &omap_crtc->base;
 
-	omap_crtc->plane = omap_plane_init(dev, ovl, (1 << id), true);
+	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);
+
+	omap_crtc->apply.pre_apply  = omap_crtc_pre_apply;
+	omap_crtc->apply.post_apply = omap_crtc_post_apply;
+
+	omap_crtc->apply_irq.irqmask = pipe2vbl(id);
+	omap_crtc->apply_irq.irq = omap_crtc_apply_irq;
+
+	omap_crtc->error_irq.irqmask =
+			dispc_mgr_get_sync_lost_irq(channel);
+	omap_crtc->error_irq.irq = omap_crtc_error_irq;
+	omap_irq_register(dev, &omap_crtc->error_irq);
+
+	omap_crtc->channel = channel;
+	omap_crtc->plane = plane;
 	omap_crtc->plane->crtc = crtc;
-	omap_crtc->name = ovl->name;
-	omap_crtc->id = id;
+	omap_crtc->name = channel_names[channel];
+	omap_crtc->pipe = id;
+
+	/* temporary: */
+	omap_crtc->mgr.id = channel;
+
+	dss_install_mgr_ops(&mgr_ops);
+
+	/* 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;
 
 	drm_crtc_init(dev, crtc, &omap_crtc_funcs);
 	drm_crtc_helper_add(crtc, &omap_crtc_helper_funcs);
diff --git a/drivers/staging/omapdrm/omap_drv.c b/drivers/staging/omapdrm/omap_drv.c
index 84943e5..b3be80d 100644
--- a/drivers/staging/omapdrm/omap_drv.c
+++ b/drivers/staging/omapdrm/omap_drv.c
@@ -74,320 +74,99 @@  static int get_connector_type(struct omap_dss_device *dssdev)
 	}
 }
 
-#if 0 /* enable when dss2 supports hotplug */
-static int omap_drm_notifier(struct notifier_block *nb,
-		unsigned long evt, void *arg)
-{
-	switch (evt) {
-	case OMAP_DSS_SIZE_CHANGE:
-	case OMAP_DSS_HOTPLUG_CONNECT:
-	case OMAP_DSS_HOTPLUG_DISCONNECT: {
-		struct drm_device *dev = drm_device;
-		DBG("hotplug event: evt=%d, dev=%p", evt, dev);
-		if (dev)
-			drm_sysfs_hotplug_event(dev);
-
-		return NOTIFY_OK;
-	}
-	default:  /* don't care about other events for now */
-		return NOTIFY_DONE;
-	}
-}
-#endif
-
-static void dump_video_chains(void)
-{
-	int i;
-
-	DBG("dumping video chains: ");
-	for (i = 0; i < omap_dss_get_num_overlays(); i++) {
-		struct omap_overlay *ovl = omap_dss_get_overlay(i);
-		struct omap_overlay_manager *mgr = ovl->manager;
-		struct omap_dss_device *dssdev = mgr ?
-					mgr->get_device(mgr) : NULL;
-		if (dssdev) {
-			DBG("%d: %s -> %s -> %s", i, ovl->name, mgr->name,
-						dssdev->name);
-		} else if (mgr) {
-			DBG("%d: %s -> %s", i, ovl->name, mgr->name);
-		} else {
-			DBG("%d: %s", i, ovl->name);
-		}
-	}
-}
-
-/* create encoders for each manager */
-static int create_encoder(struct drm_device *dev,
-		struct omap_overlay_manager *mgr)
-{
-	struct omap_drm_private *priv = dev->dev_private;
-	struct drm_encoder *encoder = omap_encoder_init(dev, mgr);
-
-	if (!encoder) {
-		dev_err(dev->dev, "could not create encoder: %s\n",
-				mgr->name);
-		return -ENOMEM;
-	}
-
-	BUG_ON(priv->num_encoders >= ARRAY_SIZE(priv->encoders));
-
-	priv->encoders[priv->num_encoders++] = encoder;
-
-	return 0;
-}
-
-/* create connectors for each display device */
-static int create_connector(struct drm_device *dev,
-		struct omap_dss_device *dssdev)
+static int omap_modeset_init(struct drm_device *dev)
 {
 	struct omap_drm_private *priv = dev->dev_private;
-	static struct notifier_block *notifier;
-	struct drm_connector *connector;
-	int j;
-
-	if (!dssdev->driver) {
-		dev_warn(dev->dev, "%s has no driver.. skipping it\n",
-				dssdev->name);
-		return 0;
-	}
+	struct omap_dss_device *dssdev = NULL;
+	int num_ovls = dss_feat_get_num_ovls();
+	int id;
 
-	if (!(dssdev->driver->get_timings ||
-				dssdev->driver->read_edid)) {
-		dev_warn(dev->dev, "%s driver does not support "
-			"get_timings or read_edid.. skipping it!\n",
-			dssdev->name);
-		return 0;
-	}
+	drm_mode_config_init(dev);
 
-	connector = omap_connector_init(dev,
-			get_connector_type(dssdev), dssdev);
+	omap_drm_irq_install(dev);
 
-	if (!connector) {
-		dev_err(dev->dev, "could not create connector: %s\n",
-				dssdev->name);
-		return -ENOMEM;
-	}
-
-	BUG_ON(priv->num_connectors >= ARRAY_SIZE(priv->connectors));
+	/*
+	 * Create private planes and CRTCs for the last NUM_CRTCs overlay
+	 * plus manager:
+	 */
+	for (id = 0; id < min(num_crtc, num_ovls); id++) {
+		struct drm_plane *plane;
+		struct drm_crtc *crtc;
 
-	priv->connectors[priv->num_connectors++] = connector;
+		plane = omap_plane_init(dev, id, true);
+		crtc = omap_crtc_init(dev, plane, pipe2chan(id), id);
 
-#if 0 /* enable when dss2 supports hotplug */
-	notifier = kzalloc(sizeof(struct notifier_block), GFP_KERNEL);
-	notifier->notifier_call = omap_drm_notifier;
-	omap_dss_add_notify(dssdev, notifier);
-#else
-	notifier = NULL;
-#endif
+		BUG_ON(priv->num_crtcs >= ARRAY_SIZE(priv->crtcs));
+		priv->crtcs[id] = crtc;
+		priv->num_crtcs++;
 
-	for (j = 0; j < priv->num_encoders; j++) {
-		struct omap_overlay_manager *mgr =
-			omap_encoder_get_manager(priv->encoders[j]);
-		if (mgr->get_device(mgr) == dssdev) {
-			drm_mode_connector_attach_encoder(connector,
-					priv->encoders[j]);
-		}
+		priv->planes[id] = plane;
+		priv->num_planes++;
 	}
 
-	return 0;
-}
-
-/* create up to max_overlays CRTCs mapping to overlays.. by default,
- * connect the overlays to different managers/encoders, giving priority
- * to encoders connected to connectors with a detected connection
- */
-static int create_crtc(struct drm_device *dev, struct omap_overlay *ovl,
-		int *j, unsigned int connected_connectors)
-{
-	struct omap_drm_private *priv = dev->dev_private;
-	struct omap_overlay_manager *mgr = NULL;
-	struct drm_crtc *crtc;
-
-	/* find next best connector, ones with detected connection first
+	/*
+	 * Create normal planes for the remaining overlays:
 	 */
-	while (*j < priv->num_connectors && !mgr) {
-		if (connected_connectors & (1 << *j)) {
-			struct drm_encoder *encoder =
-				omap_connector_attached_encoder(
-						priv->connectors[*j]);
-			if (encoder)
-				mgr = omap_encoder_get_manager(encoder);
+	for (; id < num_ovls; id++) {
+		struct drm_plane *plane = omap_plane_init(dev, id, false);
 
-		}
-		(*j)++;
+		BUG_ON(priv->num_planes >= ARRAY_SIZE(priv->planes));
+		priv->planes[priv->num_planes++] = plane;
 	}
 
-	/* if we couldn't find another connected connector, lets start
-	 * looking at the unconnected connectors:
-	 *
-	 * note: it might not be immediately apparent, but thanks to
-	 * the !mgr check in both this loop and the one above, the only
-	 * way to enter this loop is with *j == priv->num_connectors,
-	 * so idx can never go negative.
-	 */
-	while (*j < 2 * priv->num_connectors && !mgr) {
-		int idx = *j - priv->num_connectors;
-		if (!(connected_connectors & (1 << idx))) {
-			struct drm_encoder *encoder =
-				omap_connector_attached_encoder(
-						priv->connectors[idx]);
-			if (encoder)
-				mgr = omap_encoder_get_manager(encoder);
+	for_each_dss_dev(dssdev) {
+		struct drm_connector *connector;
+		struct drm_encoder *encoder;
 
+		if (!dssdev->driver) {
+			dev_warn(dev->dev, "%s has no driver.. skipping it\n",
+					dssdev->name);
+			return 0;
 		}
-		(*j)++;
-	}
-
-	crtc = omap_crtc_init(dev, ovl, priv->num_crtcs);
-
-	if (!crtc) {
-		dev_err(dev->dev, "could not create CRTC: %s\n",
-				ovl->name);
-		return -ENOMEM;
-	}
 
-	BUG_ON(priv->num_crtcs >= ARRAY_SIZE(priv->crtcs));
-
-	priv->crtcs[priv->num_crtcs++] = crtc;
-
-	return 0;
-}
-
-static int create_plane(struct drm_device *dev, struct omap_overlay *ovl,
-		unsigned int possible_crtcs)
-{
-	struct omap_drm_private *priv = dev->dev_private;
-	struct drm_plane *plane =
-			omap_plane_init(dev, ovl, possible_crtcs, false);
-
-	if (!plane) {
-		dev_err(dev->dev, "could not create plane: %s\n",
-				ovl->name);
-		return -ENOMEM;
-	}
-
-	BUG_ON(priv->num_planes >= ARRAY_SIZE(priv->planes));
-
-	priv->planes[priv->num_planes++] = plane;
-
-	return 0;
-}
-
-static int match_dev_name(struct omap_dss_device *dssdev, void *data)
-{
-	return !strcmp(dssdev->name, data);
-}
-
-static unsigned int detect_connectors(struct drm_device *dev)
-{
-	struct omap_drm_private *priv = dev->dev_private;
-	unsigned int connected_connectors = 0;
-	int i;
-
-	for (i = 0; i < priv->num_connectors; i++) {
-		struct drm_connector *connector = priv->connectors[i];
-		if (omap_connector_detect(connector, true) ==
-				connector_status_connected) {
-			connected_connectors |= (1 << i);
+		if (!(dssdev->driver->get_timings ||
+					dssdev->driver->read_edid)) {
+			dev_warn(dev->dev, "%s driver does not support "
+				"get_timings or read_edid.. skipping it!\n",
+				dssdev->name);
+			return 0;
 		}
-	}
-
-	return connected_connectors;
-}
 
-static int omap_modeset_init(struct drm_device *dev)
-{
-	const struct omap_drm_platform_data *pdata = dev->dev->platform_data;
-	struct omap_kms_platform_data *kms_pdata = NULL;
-	struct omap_drm_private *priv = dev->dev_private;
-	struct omap_dss_device *dssdev = NULL;
-	int i, j;
-	unsigned int connected_connectors = 0;
+		encoder = omap_encoder_init(dev, dssdev);
 
-	drm_mode_config_init(dev);
-
-	if (pdata && pdata->kms_pdata) {
-		kms_pdata = pdata->kms_pdata;
-
-		/* if platform data is provided by the board file, use it to
-		 * control which overlays, managers, and devices we own.
-		 */
-		for (i = 0; i < kms_pdata->mgr_cnt; i++) {
-			struct omap_overlay_manager *mgr =
-				omap_dss_get_overlay_manager(
-						kms_pdata->mgr_ids[i]);
-			create_encoder(dev, mgr);
-		}
-
-		for (i = 0; i < kms_pdata->dev_cnt; i++) {
-			struct omap_dss_device *dssdev =
-				omap_dss_find_device(
-					(void *)kms_pdata->dev_names[i],
-					match_dev_name);
-			if (!dssdev) {
-				dev_warn(dev->dev, "no such dssdev: %s\n",
-						kms_pdata->dev_names[i]);
-				continue;
-			}
-			create_connector(dev, dssdev);
+		if (!encoder) {
+			dev_err(dev->dev, "could not create encoder: %s\n",
+					dssdev->name);
+			return -ENOMEM;
 		}
 
-		connected_connectors = detect_connectors(dev);
+		connector = omap_connector_init(dev,
+				get_connector_type(dssdev), dssdev, encoder);
 
-		j = 0;
-		for (i = 0; i < kms_pdata->ovl_cnt; i++) {
-			struct omap_overlay *ovl =
-				omap_dss_get_overlay(kms_pdata->ovl_ids[i]);
-			create_crtc(dev, ovl, &j, connected_connectors);
+		if (!connector) {
+			dev_err(dev->dev, "could not create connector: %s\n",
+					dssdev->name);
+			return -ENOMEM;
 		}
 
-		for (i = 0; i < kms_pdata->pln_cnt; i++) {
-			struct omap_overlay *ovl =
-				omap_dss_get_overlay(kms_pdata->pln_ids[i]);
-			create_plane(dev, ovl, (1 << priv->num_crtcs) - 1);
-		}
-	} else {
-		/* otherwise just grab up to CONFIG_DRM_OMAP_NUM_CRTCS and try
-		 * to make educated guesses about everything else
-		 */
-		int max_overlays = min(omap_dss_get_num_overlays(), num_crtc);
+		BUG_ON(priv->num_encoders >= ARRAY_SIZE(priv->encoders));
+		BUG_ON(priv->num_connectors >= ARRAY_SIZE(priv->connectors));
 
-		for (i = 0; i < omap_dss_get_num_overlay_managers(); i++)
-			create_encoder(dev, omap_dss_get_overlay_manager(i));
-
-		for_each_dss_dev(dssdev) {
-			create_connector(dev, dssdev);
-		}
+		priv->encoders[priv->num_encoders++] = encoder;
+		priv->connectors[priv->num_connectors++] = connector;
 
-		connected_connectors = detect_connectors(dev);
+		drm_mode_connector_attach_encoder(connector, encoder);
 
-		j = 0;
-		for (i = 0; i < max_overlays; i++) {
-			create_crtc(dev, omap_dss_get_overlay(i),
-					&j, connected_connectors);
-		}
-
-		/* use any remaining overlays as drm planes */
-		for (; i < omap_dss_get_num_overlays(); i++) {
-			struct omap_overlay *ovl = omap_dss_get_overlay(i);
-			create_plane(dev, ovl, (1 << priv->num_crtcs) - 1);
+		/* figure out which crtc's we can connect the encoder to: */
+		encoder->possible_crtcs = 0;
+		for (id = 0; id < priv->num_crtcs; id++) {
+			enum omap_display_type supported_displays =
+					dss_feat_get_supported_displays(pipe2chan(id));
+			if (supported_displays & dssdev->type)
+				encoder->possible_crtcs |= (1 << id);
 		}
 	}
 
-	/* for now keep the mapping of CRTCs and encoders static.. */
-	for (i = 0; i < priv->num_encoders; i++) {
-		struct drm_encoder *encoder = priv->encoders[i];
-		struct omap_overlay_manager *mgr =
-				omap_encoder_get_manager(encoder);
-
-		encoder->possible_crtcs = (1 << priv->num_crtcs) - 1;
-
-		DBG("%s: possible_crtcs=%08x", mgr->name,
-					encoder->possible_crtcs);
-	}
-
-	dump_video_chains();
-
 	dev->mode_config.min_width = 32;
 	dev->mode_config.min_height = 32;
 
@@ -450,7 +229,7 @@  static int ioctl_gem_new(struct drm_device *dev, void *data,
 		struct drm_file *file_priv)
 {
 	struct drm_omap_gem_new *args = data;
-	DBG("%p:%p: size=0x%08x, flags=%08x", dev, file_priv,
+	VERB("%p:%p: size=0x%08x, flags=%08x", dev, file_priv,
 			args->size.bytes, args->flags);
 	return omap_gem_new_handle(dev, file_priv, args->size,
 			args->flags, &args->handle);
@@ -510,7 +289,7 @@  static int ioctl_gem_info(struct drm_device *dev, void *data,
 	struct drm_gem_object *obj;
 	int ret = 0;
 
-	DBG("%p:%p: handle=%d", dev, file_priv, args->handle);
+	VERB("%p:%p: handle=%d", dev, file_priv, args->handle);
 
 	obj = drm_gem_object_lookup(dev, file_priv, args->handle);
 	if (!obj)
@@ -565,14 +344,6 @@  static int dev_load(struct drm_device *dev, unsigned long flags)
 
 	dev->dev_private = priv;
 
-	ret = omapdss_compat_init();
-	if (ret) {
-		dev_err(dev->dev, "coult not init omapdss\n");
-		dev->dev_private = NULL;
-		kfree(priv);
-		return ret;
-	}
-
 	priv->wq = alloc_ordered_workqueue("omapdrm", 0);
 
 	INIT_LIST_HEAD(&priv->obj_list);
@@ -584,10 +355,13 @@  static int dev_load(struct drm_device *dev, unsigned long flags)
 		dev_err(dev->dev, "omap_modeset_init failed: ret=%d\n", ret);
 		dev->dev_private = NULL;
 		kfree(priv);
-		omapdss_compat_uninit();
 		return ret;
 	}
 
+	ret = drm_vblank_init(dev, priv->num_crtcs);
+	if (ret)
+		dev_warn(dev->dev, "could not init vblank\n");
+
 	priv->fbdev = omap_fbdev_init(dev);
 	if (!priv->fbdev) {
 		dev_warn(dev->dev, "omap_fbdev_init failed\n");
@@ -596,10 +370,6 @@  static int dev_load(struct drm_device *dev, unsigned long flags)
 
 	drm_kms_helper_poll_init(dev);
 
-	ret = drm_vblank_init(dev, priv->num_crtcs);
-	if (ret)
-		dev_warn(dev->dev, "could not init vblank\n");
-
 	return 0;
 }
 
@@ -609,8 +379,9 @@  static int dev_unload(struct drm_device *dev)
 
 	DBG("unload: dev=%p", dev);
 
-	drm_vblank_cleanup(dev);
 	drm_kms_helper_poll_fini(dev);
+	drm_vblank_cleanup(dev);
+	omap_drm_irq_uninstall(dev);
 
 	omap_fbdev_free(dev);
 	omap_modeset_free(dev);
@@ -619,8 +390,6 @@  static int dev_unload(struct drm_device *dev)
 	flush_workqueue(priv->wq);
 	destroy_workqueue(priv->wq);
 
-	omapdss_compat_uninit();
-
 	kfree(dev->dev_private);
 	dev->dev_private = NULL;
 
@@ -680,7 +449,9 @@  static void dev_lastclose(struct drm_device *dev)
 		}
 	}
 
+	mutex_lock(&dev->mode_config.mutex);
 	ret = drm_fb_helper_restore_fbdev_mode(priv->fbdev);
+	mutex_unlock(&dev->mode_config.mutex);
 	if (ret)
 		DBG("failed to restore crtc mode");
 }
@@ -695,60 +466,6 @@  static void dev_postclose(struct drm_device *dev, struct drm_file *file)
 	DBG("postclose: dev=%p, file=%p", dev, file);
 }
 
-/**
- * enable_vblank - enable vblank interrupt events
- * @dev: DRM device
- * @crtc: which irq to enable
- *
- * Enable vblank interrupts for @crtc.  If the device doesn't have
- * a hardware vblank counter, this routine should be a no-op, since
- * interrupts will have to stay on to keep the count accurate.
- *
- * RETURNS
- * Zero on success, appropriate errno if the given @crtc's vblank
- * interrupt cannot be enabled.
- */
-static int dev_enable_vblank(struct drm_device *dev, int crtc)
-{
-	DBG("enable_vblank: dev=%p, crtc=%d", dev, crtc);
-	return 0;
-}
-
-/**
- * disable_vblank - disable vblank interrupt events
- * @dev: DRM device
- * @crtc: which irq to enable
- *
- * Disable vblank interrupts for @crtc.  If the device doesn't have
- * a hardware vblank counter, this routine should be a no-op, since
- * interrupts will have to stay on to keep the count accurate.
- */
-static void dev_disable_vblank(struct drm_device *dev, int crtc)
-{
-	DBG("disable_vblank: dev=%p, crtc=%d", dev, crtc);
-}
-
-static irqreturn_t dev_irq_handler(DRM_IRQ_ARGS)
-{
-	return IRQ_HANDLED;
-}
-
-static void dev_irq_preinstall(struct drm_device *dev)
-{
-	DBG("irq_preinstall: dev=%p", dev);
-}
-
-static int dev_irq_postinstall(struct drm_device *dev)
-{
-	DBG("irq_postinstall: dev=%p", dev);
-	return 0;
-}
-
-static void dev_irq_uninstall(struct drm_device *dev)
-{
-	DBG("irq_uninstall: dev=%p", dev);
-}
-
 static const struct vm_operations_struct omap_gem_vm_ops = {
 	.fault = omap_gem_fault,
 	.open = drm_gem_vm_open,
@@ -778,12 +495,12 @@  static struct drm_driver omap_drm_driver = {
 		.preclose = dev_preclose,
 		.postclose = dev_postclose,
 		.get_vblank_counter = drm_vblank_count,
-		.enable_vblank = dev_enable_vblank,
-		.disable_vblank = dev_disable_vblank,
-		.irq_preinstall = dev_irq_preinstall,
-		.irq_postinstall = dev_irq_postinstall,
-		.irq_uninstall = dev_irq_uninstall,
-		.irq_handler = dev_irq_handler,
+		.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/staging/omapdrm/omap_drv.h b/drivers/staging/omapdrm/omap_drv.h
index 1d4aea5..cd1f22b 100644
--- a/drivers/staging/omapdrm/omap_drv.h
+++ b/drivers/staging/omapdrm/omap_drv.h
@@ -28,6 +28,7 @@ 
 #include <linux/platform_data/omap_drm.h>
 #include "omap_drm.h"
 
+
 #define DBG(fmt, ...) DRM_DEBUG(fmt"\n", ##__VA_ARGS__)
 #define VERB(fmt, ...) if (0) DRM_DEBUG(fmt, ##__VA_ARGS__) /* verbose debug */
 
@@ -39,6 +40,51 @@ 
  */
 #define MAX_MAPPERS 2
 
+/* parameters which describe (unrotated) coordinates of scanout within a fb: */
+struct omap_drm_window {
+	uint32_t rotation;
+	int32_t  crtc_x, crtc_y;		/* signed because can be offscreen */
+	uint32_t crtc_w, crtc_h;
+	uint32_t src_x, src_y;
+	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
+ * the hysteresis in drm_vblank_put() is not necessarily desirable for
+ * internal housekeeping related irq usage.
+ */
+struct omap_drm_irq {
+	struct list_head node;
+	uint32_t irqmask;
+	bool registered;
+	void (*irq)(struct omap_drm_irq *irq, uint32_t irqstatus);
+};
+
+/* For KMS code that needs to wait for a certain # of IRQs:
+ */
+struct omap_irq_wait;
+struct omap_irq_wait * omap_irq_wait_init(struct drm_device *dev,
+		uint32_t irqmask, int count);
+int omap_irq_wait(struct drm_device *dev, struct omap_irq_wait *wait,
+		unsigned long timeout);
+
 struct omap_drm_private {
 	uint32_t omaprev;
 
@@ -58,6 +104,7 @@  struct omap_drm_private {
 
 	struct workqueue_struct *wq;
 
+	/* list of GEM objects: */
 	struct list_head obj_list;
 
 	bool has_dmm;
@@ -65,6 +112,11 @@  struct omap_drm_private {
 	/* properties: */
 	struct drm_property *rotation_prop;
 	struct drm_property *zorder_prop;
+
+	/* irq handling: */
+	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;
 };
 
 /* this should probably be in drm-core to standardize amongst drivers */
@@ -75,15 +127,6 @@  struct omap_drm_private {
 #define DRM_REFLECT_X	4
 #define DRM_REFLECT_Y	5
 
-/* parameters which describe (unrotated) coordinates of scanout within a fb: */
-struct omap_drm_window {
-	uint32_t rotation;
-	int32_t  crtc_x, crtc_y;		/* signed because can be offscreen */
-	uint32_t crtc_w, crtc_h;
-	uint32_t src_x, src_y;
-	uint32_t src_w, src_h;
-};
-
 #ifdef CONFIG_DEBUG_FS
 int omap_debugfs_init(struct drm_minor *minor);
 void omap_debugfs_cleanup(struct drm_minor *minor);
@@ -92,23 +135,36 @@  void omap_gem_describe(struct drm_gem_object *obj, struct seq_file *m);
 void omap_gem_describe_objects(struct list_head *list, struct seq_file *m);
 #endif
 
+int omap_irq_enable_vblank(struct drm_device *dev, int crtc);
+void omap_irq_disable_vblank(struct drm_device *dev, int crtc);
+irqreturn_t omap_irq_handler(DRM_IRQ_ARGS);
+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);
+int 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);
 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);
 struct drm_crtc *omap_crtc_init(struct drm_device *dev,
-		struct omap_overlay *ovl, int id);
+		struct drm_plane *plane, enum omap_channel channel, int id);
 
 struct drm_plane *omap_plane_init(struct drm_device *dev,
-		struct omap_overlay *ovl, unsigned int possible_crtcs,
-		bool priv);
+		int plane_id, bool private_plane);
 int omap_plane_dpms(struct drm_plane *plane, int mode);
 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,
 		uint32_t src_x, uint32_t src_y,
-		uint32_t src_w, uint32_t src_h);
-void omap_plane_on_endwin(struct drm_plane *plane,
+		uint32_t src_w, uint32_t src_h,
 		void (*fxn)(void *), void *arg);
 void omap_plane_install_properties(struct drm_plane *plane,
 		struct drm_mode_object *obj);
@@ -116,21 +172,25 @@  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_overlay_manager *mgr);
-struct omap_overlay_manager *omap_encoder_get_manager(
+		struct omap_dss_device *dssdev);
+int omap_encoder_set_enabled(struct drm_encoder *encoder, bool enabled);
+int omap_encoder_update(struct drm_encoder *encoder,
+		struct omap_overlay_manager *mgr,
+		struct omap_video_timings *timings);
+
+struct drm_connector *omap_connector_init(struct drm_device *dev,
+		int connector_type, struct omap_dss_device *dssdev,
 		struct drm_encoder *encoder);
 struct drm_encoder *omap_connector_attached_encoder(
 		struct drm_connector *connector);
-enum drm_connector_status omap_connector_detect(
-		struct drm_connector *connector, bool force);
-
-struct drm_connector *omap_connector_init(struct drm_device *dev,
-		int connector_type, struct omap_dss_device *dssdev);
-void omap_connector_mode_set(struct drm_connector *connector,
-		struct drm_display_mode *mode);
 void omap_connector_flush(struct drm_connector *connector,
 		int x, int y, int w, int h);
 
+void copy_timings_omap_to_drm(struct drm_display_mode *mode,
+		struct omap_video_timings *timings);
+void copy_timings_drm_to_omap(struct omap_video_timings *timings,
+		struct drm_display_mode *mode);
+
 uint32_t omap_framebuffer_get_formats(uint32_t *pixel_formats,
 		uint32_t max_formats, enum omap_color_mode supported_modes);
 struct drm_framebuffer *omap_framebuffer_create(struct drm_device *dev,
@@ -207,6 +267,40 @@  static inline int align_pitch(int pitch, int width, int bpp)
 	return ALIGN(pitch, 8 * bytespp);
 }
 
+static inline enum omap_channel pipe2chan(int pipe)
+{
+	int num_mgrs = dss_feat_get_num_mgrs();
+
+	/*
+	 * 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.  Otherwise there would not be enough video
+	 * pipes left for drm planes.  The higher #'d managers tend
+	 * to have more features so start in reverse order.
+	 */
+	return num_mgrs - pipe - 1;
+}
+
+/* map crtc to vblank mask */
+static inline uint32_t pipe2vbl(int crtc)
+{
+	enum omap_channel channel = pipe2chan(crtc);
+	return dispc_mgr_get_vsync_irq(channel);
+}
+
+static inline int crtc2pipe(struct drm_device *dev, struct drm_crtc *crtc)
+{
+	struct omap_drm_private *priv = dev->dev_private;
+	int i;
+
+	for (i = 0; i < ARRAY_SIZE(priv->crtcs); i++)
+		if (priv->crtcs[i] == crtc)
+			return i;
+
+	BUG();  /* bogus CRTC ptr */
+	return -1;
+}
+
 /* should these be made into common util helpers?
  */
 
diff --git a/drivers/staging/omapdrm/omap_encoder.c b/drivers/staging/omapdrm/omap_encoder.c
index 5341d5e..e053160 100644
--- a/drivers/staging/omapdrm/omap_encoder.c
+++ b/drivers/staging/omapdrm/omap_encoder.c
@@ -22,37 +22,56 @@ 
 #include "drm_crtc.h"
 #include "drm_crtc_helper.h"
 
+#include <linux/list.h>
+
+
 /*
  * encoder funcs
  */
 
 #define to_omap_encoder(x) container_of(x, struct omap_encoder, base)
 
+/* The encoder and connector both map to same dssdev.. the encoder
+ * handles the 'active' parts, ie. anything the modifies the state
+ * of the hw, and the connector handles the 'read-only' parts, like
+ * detecting connection and reading edid.
+ */
 struct omap_encoder {
 	struct drm_encoder base;
-	struct omap_overlay_manager *mgr;
+	struct omap_dss_device *dssdev;
 };
 
 static void omap_encoder_destroy(struct drm_encoder *encoder)
 {
 	struct omap_encoder *omap_encoder = to_omap_encoder(encoder);
-	DBG("%s", omap_encoder->mgr->name);
 	drm_encoder_cleanup(encoder);
 	kfree(omap_encoder);
 }
 
+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)
 {
-	struct omap_encoder *omap_encoder = to_omap_encoder(encoder);
-	DBG("%s: %d", omap_encoder->mgr->name, mode);
 }
 
 static bool omap_encoder_mode_fixup(struct drm_encoder *encoder,
 				  const struct drm_display_mode *mode,
 				  struct drm_display_mode *adjusted_mode)
 {
-	struct omap_encoder *omap_encoder = to_omap_encoder(encoder);
-	DBG("%s", omap_encoder->mgr->name);
 	return true;
 }
 
@@ -60,47 +79,16 @@  static void omap_encoder_mode_set(struct drm_encoder *encoder,
 				struct drm_display_mode *mode,
 				struct drm_display_mode *adjusted_mode)
 {
-	struct omap_encoder *omap_encoder = to_omap_encoder(encoder);
-	struct drm_device *dev = encoder->dev;
-	struct omap_drm_private *priv = dev->dev_private;
-	int i;
-
-	mode = adjusted_mode;
-
-	DBG("%s: set mode: %dx%d", omap_encoder->mgr->name,
-			mode->hdisplay, mode->vdisplay);
-
-	for (i = 0; i < priv->num_connectors; i++) {
-		struct drm_connector *connector = priv->connectors[i];
-		if (connector->encoder == encoder)
-			omap_connector_mode_set(connector, mode);
-
-	}
 }
 
 static void omap_encoder_prepare(struct drm_encoder *encoder)
 {
-	struct omap_encoder *omap_encoder = to_omap_encoder(encoder);
-	struct drm_encoder_helper_funcs *encoder_funcs =
-				encoder->helper_private;
-	DBG("%s", omap_encoder->mgr->name);
-	encoder_funcs->dpms(encoder, DRM_MODE_DPMS_OFF);
 }
 
 static void omap_encoder_commit(struct drm_encoder *encoder)
 {
-	struct omap_encoder *omap_encoder = to_omap_encoder(encoder);
-	struct drm_encoder_helper_funcs *encoder_funcs =
-				encoder->helper_private;
-	DBG("%s", omap_encoder->mgr->name);
-	omap_encoder->mgr->apply(omap_encoder->mgr);
-	encoder_funcs->dpms(encoder, DRM_MODE_DPMS_ON);
 }
 
-static const struct drm_encoder_funcs omap_encoder_funcs = {
-	.destroy = omap_encoder_destroy,
-};
-
 static const struct drm_encoder_helper_funcs omap_encoder_helper_funcs = {
 	.dpms = omap_encoder_dpms,
 	.mode_fixup = omap_encoder_mode_fixup,
@@ -109,23 +97,54 @@  static const struct drm_encoder_helper_funcs omap_encoder_helper_funcs = {
 	.commit = omap_encoder_commit,
 };
 
-struct omap_overlay_manager *omap_encoder_get_manager(
-		struct drm_encoder *encoder)
+/*
+ * Instead of relying on the helpers for modeset, the omap_crtc code
+ * calls these functions in the proper sequence.
+ */
+
+int omap_encoder_set_enabled(struct drm_encoder *encoder, bool enabled)
 {
 	struct omap_encoder *omap_encoder = to_omap_encoder(encoder);
-	return omap_encoder->mgr;
+	struct omap_dss_device *dssdev = omap_encoder->dssdev;
+	struct omap_dss_driver *dssdrv = dssdev->driver;
+
+	if (enabled) {
+		return dssdrv->enable(dssdev);
+	} else {
+		dssdrv->disable(dssdev);
+		return 0;
+	}
+}
+
+int omap_encoder_update(struct drm_encoder *encoder,
+		struct omap_overlay_manager *mgr,
+		struct omap_video_timings *timings)
+{
+	struct drm_device *dev = encoder->dev;
+	struct omap_encoder *omap_encoder = to_omap_encoder(encoder);
+	struct omap_dss_device *dssdev = omap_encoder->dssdev;
+	struct omap_dss_driver *dssdrv = dssdev->driver;
+	int ret;
+
+	dssdev->output->manager = mgr;
+
+	ret = dssdrv->check_timings(dssdev, timings);
+	if (ret) {
+		dev_err(dev->dev, "could not set timings: %d\n", ret);
+		return ret;
+	}
+
+	dssdrv->set_timings(dssdev, timings);
+
+	return 0;
 }
 
 /* initialize encoder */
 struct drm_encoder *omap_encoder_init(struct drm_device *dev,
-		struct omap_overlay_manager *mgr)
+		struct omap_dss_device *dssdev)
 {
 	struct drm_encoder *encoder = NULL;
 	struct omap_encoder *omap_encoder;
-	struct omap_overlay_manager_info info;
-	int ret;
-
-	DBG("%s", mgr->name);
 
 	omap_encoder = kzalloc(sizeof(*omap_encoder), GFP_KERNEL);
 	if (!omap_encoder) {
@@ -133,33 +152,14 @@  struct drm_encoder *omap_encoder_init(struct drm_device *dev,
 		goto fail;
 	}
 
-	omap_encoder->mgr = mgr;
+	omap_encoder->dssdev = dssdev;
+
 	encoder = &omap_encoder->base;
 
 	drm_encoder_init(dev, encoder, &omap_encoder_funcs,
 			 DRM_MODE_ENCODER_TMDS);
 	drm_encoder_helper_add(encoder, &omap_encoder_helper_funcs);
 
-	mgr->get_manager_info(mgr, &info);
-
-	/* TODO: fix hard-coded setup.. */
-	info.default_color = 0x00000000;
-	info.trans_key = 0x00000000;
-	info.trans_key_type = OMAP_DSS_COLOR_KEY_GFX_DST;
-	info.trans_enabled = false;
-
-	ret = mgr->set_manager_info(mgr, &info);
-	if (ret) {
-		dev_err(dev->dev, "could not set manager info\n");
-		goto fail;
-	}
-
-	ret = mgr->apply(mgr);
-	if (ret) {
-		dev_err(dev->dev, "could not apply\n");
-		goto fail;
-	}
-
 	return encoder;
 
 fail:
diff --git a/drivers/staging/omapdrm/omap_irq.c b/drivers/staging/omapdrm/omap_irq.c
new file mode 100644
index 0000000..2629ba7
--- /dev/null
+++ b/drivers/staging/omapdrm/omap_irq.c
@@ -0,0 +1,322 @@ 
+/*
+ * drivers/staging/omapdrm/omap_irq.c
+ *
+ * Copyright (C) 2012 Texas Instruments
+ * Author: Rob Clark <rob.clark@linaro.org>
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 as published by
+ * the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE.  See the GNU General Public License for
+ * more details.
+ *
+ * You should have received a copy of the GNU General Public License along with
+ * this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include "omap_drv.h"
+
+static DEFINE_SPINLOCK(list_lock);
+
+static void omap_irq_error_handler(struct omap_drm_irq *irq,
+		uint32_t irqstatus)
+{
+	DRM_ERROR("errors: %08x\n", irqstatus);
+}
+
+/* call with list_lock and dispc runtime held */
+static void omap_irq_update(struct drm_device *dev)
+{
+	struct omap_drm_private *priv = dev->dev_private;
+	struct omap_drm_irq *irq;
+	uint32_t irqmask = priv->vblank_mask;
+
+	BUG_ON(!spin_is_locked(&list_lock));
+
+	list_for_each_entry(irq, &priv->irq_list, node)
+		irqmask |= irq->irqmask;
+
+	DBG("irqmask=%08x", irqmask);
+
+	dispc_write_irqenable(irqmask);
+	dispc_read_irqenable();        /* flush posted write */
+}
+
+void omap_irq_register(struct drm_device *dev, struct omap_drm_irq *irq)
+{
+	struct omap_drm_private *priv = dev->dev_private;
+	unsigned long flags;
+
+	dispc_runtime_get();
+	spin_lock_irqsave(&list_lock, flags);
+
+	if (!WARN_ON(irq->registered)) {
+		irq->registered = true;
+		list_add(&irq->node, &priv->irq_list);
+		omap_irq_update(dev);
+	}
+
+	spin_unlock_irqrestore(&list_lock, flags);
+	dispc_runtime_put();
+}
+
+void omap_irq_unregister(struct drm_device *dev, struct omap_drm_irq *irq)
+{
+	unsigned long flags;
+
+	dispc_runtime_get();
+	spin_lock_irqsave(&list_lock, flags);
+
+	if (!WARN_ON(!irq->registered)) {
+		irq->registered = false;
+		list_del(&irq->node);
+		omap_irq_update(dev);
+	}
+
+	spin_unlock_irqrestore(&list_lock, flags);
+	dispc_runtime_put();
+}
+
+struct omap_irq_wait {
+	struct omap_drm_irq irq;
+	int count;
+};
+
+static DECLARE_WAIT_QUEUE_HEAD(wait_event);
+
+static void wait_irq(struct omap_drm_irq *irq, uint32_t irqstatus)
+{
+	struct omap_irq_wait *wait =
+			container_of(irq, struct omap_irq_wait, irq);
+	wait->count--;
+	wake_up_all(&wait_event);
+}
+
+struct omap_irq_wait * omap_irq_wait_init(struct drm_device *dev,
+		uint32_t irqmask, int count)
+{
+	struct omap_irq_wait *wait = kzalloc(sizeof(*wait), GFP_KERNEL);
+	wait->irq.irq = wait_irq;
+	wait->irq.irqmask = irqmask;
+	wait->count = count;
+	omap_irq_register(dev, &wait->irq);
+	return wait;
+}
+
+int omap_irq_wait(struct drm_device *dev, struct omap_irq_wait *wait,
+		unsigned long timeout)
+{
+	int ret = wait_event_timeout(wait_event, (wait->count <= 0), timeout);
+	omap_irq_unregister(dev, &wait->irq);
+	kfree(wait);
+	if (ret == 0)
+		return -1;
+	return 0;
+}
+
+/**
+ * enable_vblank - enable vblank interrupt events
+ * @dev: DRM device
+ * @crtc: which irq to enable
+ *
+ * Enable vblank interrupts for @crtc.  If the device doesn't have
+ * a hardware vblank counter, this routine should be a no-op, since
+ * interrupts will have to stay on to keep the count accurate.
+ *
+ * RETURNS
+ * Zero on success, appropriate errno if the given @crtc's vblank
+ * interrupt cannot be enabled.
+ */
+int omap_irq_enable_vblank(struct drm_device *dev, int crtc)
+{
+	struct omap_drm_private *priv = dev->dev_private;
+	unsigned long flags;
+
+	DBG("dev=%p, crtc=%d", dev, crtc);
+
+	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;
+}
+
+/**
+ * disable_vblank - disable vblank interrupt events
+ * @dev: DRM device
+ * @crtc: which irq to enable
+ *
+ * Disable vblank interrupts for @crtc.  If the device doesn't have
+ * a hardware vblank counter, this routine should be a no-op, since
+ * interrupts will have to stay on to keep the count accurate.
+ */
+void omap_irq_disable_vblank(struct drm_device *dev, int crtc)
+{
+	struct omap_drm_private *priv = dev->dev_private;
+	unsigned long flags;
+
+	DBG("dev=%p, crtc=%d", dev, crtc);
+
+	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();
+}
+
+irqreturn_t omap_irq_handler(DRM_IRQ_ARGS)
+{
+	struct drm_device *dev = (struct drm_device *) arg;
+	struct omap_drm_private *priv = dev->dev_private;
+	struct omap_drm_irq *handler, *n;
+	unsigned long flags;
+	unsigned int id;
+	u32 irqstatus;
+
+	irqstatus = dispc_read_irqstatus();
+	dispc_clear_irqstatus(irqstatus);
+	dispc_read_irqstatus();        /* flush posted write */
+
+	VERB("irqs: %08x", irqstatus);
+
+	for (id = 0; id < priv->num_crtcs; id++)
+		if (irqstatus & pipe2vbl(id))
+			drm_handle_vblank(dev, id);
+
+	spin_lock_irqsave(&list_lock, flags);
+	list_for_each_entry_safe(handler, n, &priv->irq_list, node) {
+		if (handler->irqmask & irqstatus) {
+			spin_unlock_irqrestore(&list_lock, flags);
+			handler->irq(handler, handler->irqmask & irqstatus);
+			spin_lock_irqsave(&list_lock, flags);
+		}
+	}
+	spin_unlock_irqrestore(&list_lock, flags);
+
+	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();
+}
+
+int omap_irq_postinstall(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);
+
+	INIT_LIST_HEAD(&priv->irq_list);
+
+	error_handler->irq = omap_irq_error_handler;
+	error_handler->irqmask = DISPC_IRQ_OCP_ERR;
+
+	/* for now ignore DISPC_IRQ_SYNC_LOST_DIGIT.. really I think
+	 * we just need to ignore it while enabling tv-out
+	 */
+	error_handler->irqmask &= ~DISPC_IRQ_SYNC_LOST_DIGIT;
+
+	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 = 1;
+	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 = 0;
+		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 = 0;
+		mutex_unlock(&dev->struct_mutex);
+		dispc_free_irq(dev);
+	}
+
+	return ret;
+}
+
+int omap_drm_irq_uninstall(struct drm_device *dev)
+{
+	unsigned long irqflags;
+	int irq_enabled, i;
+
+	mutex_lock(&dev->struct_mutex);
+	irq_enabled = dev->irq_enabled;
+	dev->irq_enabled = 0;
+	mutex_unlock(&dev->struct_mutex);
+
+	/*
+	 * 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++) {
+			DRM_WAKEUP(&dev->vbl_queue[i]);
+			dev->vblank_enabled[i] = 0;
+			dev->last_vblank[i] =
+				dev->driver->get_vblank_counter(dev, i);
+		}
+		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;
+}
diff --git a/drivers/staging/omapdrm/omap_plane.c b/drivers/staging/omapdrm/omap_plane.c
index 2a8e5ba..bb989d7 100644
--- a/drivers/staging/omapdrm/omap_plane.c
+++ b/drivers/staging/omapdrm/omap_plane.c
@@ -41,12 +41,14 @@  struct callback {
 
 struct omap_plane {
 	struct drm_plane base;
-	struct omap_overlay *ovl;
+	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;
-
+	bool enabled;
 
 	/* last fb that we pinned: */
 	struct drm_framebuffer *pinned_fb;
@@ -54,189 +56,15 @@  struct omap_plane {
 	uint32_t nformats;
 	uint32_t formats[32];
 
-	/* for synchronizing access to unpins fifo */
-	struct mutex unpin_mutex;
+	struct omap_drm_irq error_irq;
 
-	/* set of bo's pending unpin until next END_WIN irq */
+	/* set of bo's pending unpin until next post_apply() */
 	DECLARE_KFIFO_PTR(unpin_fifo, struct drm_gem_object *);
-	int num_unpins, pending_num_unpins;
-
-	/* for deferred unpin when we need to wait for scanout complete irq */
-	struct work_struct work;
-
-	/* callback on next endwin irq */
-	struct callback endwin;
-};
 
-/* map from ovl->id to the irq we are interested in for scanout-done */
-static const uint32_t id2irq[] = {
-		[OMAP_DSS_GFX]    = DISPC_IRQ_GFX_END_WIN,
-		[OMAP_DSS_VIDEO1] = DISPC_IRQ_VID1_END_WIN,
-		[OMAP_DSS_VIDEO2] = DISPC_IRQ_VID2_END_WIN,
-		[OMAP_DSS_VIDEO3] = DISPC_IRQ_VID3_END_WIN,
+	// XXX maybe get rid of this and handle vblank in crtc too?
+	struct callback apply_done_cb;
 };
 
-static void dispc_isr(void *arg, uint32_t mask)
-{
-	struct drm_plane *plane = arg;
-	struct omap_plane *omap_plane = to_omap_plane(plane);
-	struct omap_drm_private *priv = plane->dev->dev_private;
-
-	omap_dispc_unregister_isr(dispc_isr, plane,
-			id2irq[omap_plane->ovl->id]);
-
-	queue_work(priv->wq, &omap_plane->work);
-}
-
-static void unpin_worker(struct work_struct *work)
-{
-	struct omap_plane *omap_plane =
-			container_of(work, struct omap_plane, work);
-	struct callback endwin;
-
-	mutex_lock(&omap_plane->unpin_mutex);
-	DBG("unpinning %d of %d", omap_plane->num_unpins,
-			omap_plane->num_unpins + omap_plane->pending_num_unpins);
-	while (omap_plane->num_unpins > 0) {
-		struct drm_gem_object *bo = NULL;
-		int ret = kfifo_get(&omap_plane->unpin_fifo, &bo);
-		WARN_ON(!ret);
-		omap_gem_put_paddr(bo);
-		drm_gem_object_unreference_unlocked(bo);
-		omap_plane->num_unpins--;
-	}
-	endwin = omap_plane->endwin;
-	omap_plane->endwin.fxn = NULL;
-	mutex_unlock(&omap_plane->unpin_mutex);
-
-	if (endwin.fxn)
-		endwin.fxn(endwin.arg);
-}
-
-static void install_irq(struct drm_plane *plane)
-{
-	struct omap_plane *omap_plane = to_omap_plane(plane);
-	struct omap_overlay *ovl = omap_plane->ovl;
-	int ret;
-
-	ret = omap_dispc_register_isr(dispc_isr, plane, id2irq[ovl->id]);
-
-	/*
-	 * omapdss has upper limit on # of registered irq handlers,
-	 * which we shouldn't hit.. but if we do the limit should
-	 * be raised or bad things happen:
-	 */
-	WARN_ON(ret == -EBUSY);
-}
-
-/* push changes down to dss2 */
-static int commit(struct drm_plane *plane)
-{
-	struct drm_device *dev = plane->dev;
-	struct omap_plane *omap_plane = to_omap_plane(plane);
-	struct omap_overlay *ovl = omap_plane->ovl;
-	struct omap_overlay_info *info = &omap_plane->info;
-	int ret;
-
-	DBG("%s", ovl->name);
-	DBG("%dx%d -> %dx%d (%d)", info->width, info->height, info->out_width,
-			info->out_height, info->screen_width);
-	DBG("%d,%d %08x %08x", info->pos_x, info->pos_y,
-			info->paddr, info->p_uv_addr);
-
-	/* NOTE: do we want to do this at all here, or just wait
-	 * for dpms(ON) since other CRTC's may not have their mode
-	 * set yet, so fb dimensions may still change..
-	 */
-	ret = ovl->set_overlay_info(ovl, info);
-	if (ret) {
-		dev_err(dev->dev, "could not set overlay info\n");
-		return ret;
-	}
-
-	mutex_lock(&omap_plane->unpin_mutex);
-	omap_plane->num_unpins += omap_plane->pending_num_unpins;
-	omap_plane->pending_num_unpins = 0;
-	mutex_unlock(&omap_plane->unpin_mutex);
-
-	/* our encoder doesn't necessarily get a commit() after this, in
-	 * particular in the dpms() and mode_set_base() cases, so force the
-	 * manager to update:
-	 *
-	 * could this be in the encoder somehow?
-	 */
-	if (ovl->manager) {
-		ret = ovl->manager->apply(ovl->manager);
-		if (ret) {
-			dev_err(dev->dev, "could not apply settings\n");
-			return ret;
-		}
-
-		/*
-		 * NOTE: really this should be atomic w/ mgr->apply() but
-		 * omapdss does not expose such an API
-		 */
-		if (omap_plane->num_unpins > 0)
-			install_irq(plane);
-
-	} else {
-		struct omap_drm_private *priv = dev->dev_private;
-		queue_work(priv->wq, &omap_plane->work);
-	}
-
-
-	if (ovl->is_enabled(ovl)) {
-		omap_framebuffer_flush(plane->fb, info->pos_x, info->pos_y,
-				info->out_width, info->out_height);
-	}
-
-	return 0;
-}
-
-/* when CRTC that we are attached to has potentially changed, this checks
- * if we are attached to proper manager, and if necessary updates.
- */
-static void update_manager(struct drm_plane *plane)
-{
-	struct omap_drm_private *priv = plane->dev->dev_private;
-	struct omap_plane *omap_plane = to_omap_plane(plane);
-	struct omap_overlay *ovl = omap_plane->ovl;
-	struct omap_overlay_manager *mgr = NULL;
-	int i;
-
-	if (plane->crtc) {
-		for (i = 0; i < priv->num_encoders; i++) {
-			struct drm_encoder *encoder = priv->encoders[i];
-			if (encoder->crtc == plane->crtc) {
-				mgr = omap_encoder_get_manager(encoder);
-				break;
-			}
-		}
-	}
-
-	if (ovl->manager != mgr) {
-		bool enabled = ovl->is_enabled(ovl);
-
-		/* don't switch things around with enabled overlays: */
-		if (enabled)
-			omap_plane_dpms(plane, DRM_MODE_DPMS_OFF);
-
-		if (ovl->manager) {
-			DBG("disconnecting %s from %s", ovl->name,
-					ovl->manager->name);
-			ovl->unset_manager(ovl);
-		}
-
-		if (mgr) {
-			DBG("connecting %s to %s", ovl->name, mgr->name);
-			ovl->set_manager(ovl, mgr);
-		}
-
-		if (enabled && mgr)
-			omap_plane_dpms(plane, DRM_MODE_DPMS_ON);
-	}
-}
-
 static void unpin(void *arg, struct drm_gem_object *bo)
 {
 	struct drm_plane *plane = arg;
@@ -244,7 +72,6 @@  static void unpin(void *arg, struct drm_gem_object *bo)
 
 	if (kfifo_put(&omap_plane->unpin_fifo,
 			(const struct drm_gem_object **)&bo)) {
-		omap_plane->pending_num_unpins++;
 		/* also hold a ref so it isn't free'd while pinned */
 		drm_gem_object_reference(bo);
 	} else {
@@ -264,13 +91,19 @@  static int update_pin(struct drm_plane *plane, struct drm_framebuffer *fb)
 
 		DBG("%p -> %p", pinned_fb, fb);
 
-		mutex_lock(&omap_plane->unpin_mutex);
+		if (fb)
+			drm_framebuffer_reference(fb);
+
 		ret = omap_framebuffer_replace(pinned_fb, fb, plane, unpin);
-		mutex_unlock(&omap_plane->unpin_mutex);
+
+		if (pinned_fb)
+			drm_framebuffer_unreference(pinned_fb);
 
 		if (ret) {
 			dev_err(plane->dev->dev, "could not swap %p -> %p\n",
 					omap_plane->pinned_fb, fb);
+			if (fb)
+				drm_framebuffer_unreference(fb);
 			omap_plane->pinned_fb = NULL;
 			return ret;
 		}
@@ -281,31 +114,90 @@  static int update_pin(struct drm_plane *plane, struct drm_framebuffer *fb)
 	return 0;
 }
 
-/* update parameters that are dependent on the framebuffer dimensions and
- * position within the fb that this plane scans out from. This is called
- * when framebuffer or x,y base may have changed.
- */
-static void update_scanout(struct drm_plane *plane)
+static void omap_plane_pre_apply(struct omap_drm_apply *apply)
 {
-	struct omap_plane *omap_plane = to_omap_plane(plane);
-	struct omap_overlay_info *info = &omap_plane->info;
+	struct omap_plane *omap_plane =
+			container_of(apply, struct omap_plane, apply);
 	struct omap_drm_window *win = &omap_plane->win;
+	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;
+	bool ilace, replication;
 	int ret;
 
-	ret = update_pin(plane, plane->fb);
-	if (ret) {
-		dev_err(plane->dev->dev,
-			"could not pin fb: %d\n", ret);
-		omap_plane_dpms(plane, DRM_MODE_DPMS_OFF);
+	DBG("%s, enabled=%d", omap_plane->name, enabled);
+
+	/* if fb has changed, pin new fb: */
+	update_pin(plane, enabled ? plane->fb : NULL);
+
+	if (!enabled) {
+		dispc_ovl_enable(omap_plane->id, false);
 		return;
 	}
 
+	channel = omap_crtc_channel(crtc);
+
+	/* update scanout: */
 	omap_framebuffer_update_scanout(plane->fb, win, info);
 
-	DBG("%s: %d,%d: %08x %08x (%d)", omap_plane->ovl->name,
-			win->src_x, win->src_y,
-			(u32)info->paddr, (u32)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 %08x %08x", info->pos_x, info->pos_y,
+			info->paddr, info->p_uv_addr);
+
+	/* TODO: */
+	ilace = false;
+	replication = false;
+
+	/* and finally, update omapdss: */
+	ret = dispc_ovl_setup(omap_plane->id, info,
+			replication, omap_crtc_timings(crtc), false);
+	if (ret) {
+		dev_err(dev->dev, "dispc_ovl_setup failed: %d\n", ret);
+		return;
+	}
+
+	dispc_ovl_enable(omap_plane->id, true);
+	dispc_ovl_set_channel_out(omap_plane->id, channel);
+}
+
+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_overlay_info *info = &omap_plane->info;
+	struct drm_gem_object *bo = NULL;
+	struct callback cb;
+
+	cb = omap_plane->apply_done_cb;
+	omap_plane->apply_done_cb.fxn = NULL;
+
+	while (kfifo_get(&omap_plane->unpin_fifo, &bo)) {
+		omap_gem_put_paddr(bo);
+		drm_gem_object_unreference_unlocked(bo);
+	}
+
+	if (cb.fxn)
+		cb.fxn(cb.arg);
+
+	if (omap_plane->enabled) {
+		omap_framebuffer_flush(plane->fb, info->pos_x, info->pos_y,
+				info->out_width, info->out_height);
+	}
+}
+
+static int 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;
 }
 
 int omap_plane_mode_set(struct drm_plane *plane,
@@ -313,7 +205,8 @@  int omap_plane_mode_set(struct drm_plane *plane,
 		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)
+		uint32_t src_w, uint32_t src_h,
+		void (*fxn)(void *), void *arg)
 {
 	struct omap_plane *omap_plane = to_omap_plane(plane);
 	struct omap_drm_window *win = &omap_plane->win;
@@ -329,17 +222,20 @@  int omap_plane_mode_set(struct drm_plane *plane,
 	win->src_w = src_w >> 16;
 	win->src_h = src_h >> 16;
 
-	/* note: this is done after this fxn returns.. but if we need
-	 * to do a commit/update_scanout, etc before this returns we
-	 * need the current value.
-	 */
+	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);
+
+		omap_plane->apply_done_cb.fxn = fxn;
+		omap_plane->apply_done_cb.arg = arg;
+	}
+
 	plane->fb = fb;
 	plane->crtc = crtc;
 
-	update_scanout(plane);
-	update_manager(plane);
-
-	return 0;
+	return apply(plane);
 }
 
 static int omap_plane_update(struct drm_plane *plane,
@@ -349,9 +245,12 @@  static int omap_plane_update(struct drm_plane *plane,
 		uint32_t src_x, uint32_t src_y,
 		uint32_t src_w, uint32_t src_h)
 {
-	omap_plane_mode_set(plane, crtc, fb, crtc_x, crtc_y, crtc_w, crtc_h,
-			src_x, src_y, src_w, src_h);
-	return omap_plane_dpms(plane, DRM_MODE_DPMS_ON);
+	struct omap_plane *omap_plane = to_omap_plane(plane);
+	omap_plane->enabled = true;
+	return omap_plane_mode_set(plane, crtc, fb,
+			crtc_x, crtc_y, crtc_w, crtc_h,
+			src_x, src_y, src_w, src_h,
+			NULL, NULL);
 }
 
 static int omap_plane_disable(struct drm_plane *plane)
@@ -364,48 +263,32 @@  static int omap_plane_disable(struct drm_plane *plane)
 static void omap_plane_destroy(struct drm_plane *plane)
 {
 	struct omap_plane *omap_plane = to_omap_plane(plane);
-	DBG("%s", omap_plane->ovl->name);
+
+	DBG("%s", omap_plane->name);
+
+	omap_irq_unregister(plane->dev, &omap_plane->error_irq);
+
 	omap_plane_disable(plane);
 	drm_plane_cleanup(plane);
-	WARN_ON(omap_plane->pending_num_unpins + omap_plane->num_unpins > 0);
+
+	WARN_ON(!kfifo_is_empty(&omap_plane->unpin_fifo));
 	kfifo_free(&omap_plane->unpin_fifo);
+
 	kfree(omap_plane);
 }
 
 int omap_plane_dpms(struct drm_plane *plane, int mode)
 {
 	struct omap_plane *omap_plane = to_omap_plane(plane);
-	struct omap_overlay *ovl = omap_plane->ovl;
-	int r;
+	bool enabled = (mode == DRM_MODE_DPMS_ON);
+	int ret = 0;
 
-	DBG("%s: %d", omap_plane->ovl->name, mode);
-
-	if (mode == DRM_MODE_DPMS_ON) {
-		update_scanout(plane);
-		r = commit(plane);
-		if (!r)
-			r = ovl->enable(ovl);
-	} else {
-		struct omap_drm_private *priv = plane->dev->dev_private;
-		r = ovl->disable(ovl);
-		update_pin(plane, NULL);
-		queue_work(priv->wq, &omap_plane->work);
+	if (enabled != omap_plane->enabled) {
+		omap_plane->enabled = enabled;
+		ret = apply(plane);
 	}
 
-	return r;
-}
-
-void omap_plane_on_endwin(struct drm_plane *plane,
-		void (*fxn)(void *), void *arg)
-{
-	struct omap_plane *omap_plane = to_omap_plane(plane);
-
-	mutex_lock(&omap_plane->unpin_mutex);
-	omap_plane->endwin.fxn = fxn;
-	omap_plane->endwin.arg = arg;
-	mutex_unlock(&omap_plane->unpin_mutex);
-
-	install_irq(plane);
+	return ret;
 }
 
 /* helper to install properties which are common to planes and crtcs */
@@ -454,25 +337,13 @@  int omap_plane_set_property(struct drm_plane *plane,
 	int ret = -EINVAL;
 
 	if (property == priv->rotation_prop) {
-		struct omap_overlay *ovl = omap_plane->ovl;
-
-		DBG("%s: rotation: %02x", ovl->name, (uint32_t)val);
+		DBG("%s: rotation: %02x", omap_plane->name, (uint32_t)val);
 		omap_plane->win.rotation = val;
-
-		if (ovl->is_enabled(ovl))
-			ret = omap_plane_dpms(plane, DRM_MODE_DPMS_ON);
-		else
-			ret = 0;
+		ret = apply(plane);
 	} else if (property == priv->zorder_prop) {
-		struct omap_overlay *ovl = omap_plane->ovl;
-
-		DBG("%s: zorder: %d", ovl->name, (uint32_t)val);
+		DBG("%s: zorder: %02x", omap_plane->name, (uint32_t)val);
 		omap_plane->info.zorder = val;
-
-		if (ovl->is_enabled(ovl))
-			ret = omap_plane_dpms(plane, DRM_MODE_DPMS_ON);
-		else
-			ret = 0;
+		ret = apply(plane);
 	}
 
 	return ret;
@@ -485,20 +356,38 @@  static const struct drm_plane_funcs omap_plane_funcs = {
 		.set_property = omap_plane_set_property,
 };
 
+static void omap_plane_error_irq(struct omap_drm_irq *irq, uint32_t irqstatus)
+{
+	struct omap_plane *omap_plane =
+			container_of(irq, struct omap_plane, error_irq);
+	DRM_ERROR("%s: errors: %08x\n", omap_plane->name, irqstatus);
+}
+
+static const char *plane_names[] = {
+		[OMAP_DSS_GFX] = "gfx",
+		[OMAP_DSS_VIDEO1] = "vid1",
+		[OMAP_DSS_VIDEO2] = "vid2",
+		[OMAP_DSS_VIDEO3] = "vid3",
+};
+
+static const uint32_t error_irqs[] = {
+		[OMAP_DSS_GFX] = DISPC_IRQ_GFX_FIFO_UNDERFLOW,
+		[OMAP_DSS_VIDEO1] = DISPC_IRQ_VID1_FIFO_UNDERFLOW,
+		[OMAP_DSS_VIDEO2] = DISPC_IRQ_VID2_FIFO_UNDERFLOW,
+		[OMAP_DSS_VIDEO3] = DISPC_IRQ_VID3_FIFO_UNDERFLOW,
+};
+
 /* initialize plane */
 struct drm_plane *omap_plane_init(struct drm_device *dev,
-		struct omap_overlay *ovl, unsigned int possible_crtcs,
-		bool priv)
+		int id, bool private_plane)
 {
+	struct omap_drm_private *priv = dev->dev_private;
 	struct drm_plane *plane = NULL;
 	struct omap_plane *omap_plane;
+	struct omap_overlay_info *info;
 	int ret;
 
-	DBG("%s: possible_crtcs=%08x, priv=%d", ovl->name,
-			possible_crtcs, priv);
-
-	/* friendly reminder to update table for future hw: */
-	WARN_ON(ovl->id >= ARRAY_SIZE(id2irq));
+	DBG("%s: priv=%d", plane_names[id], private_plane);
 
 	omap_plane = kzalloc(sizeof(*omap_plane), GFP_KERNEL);
 	if (!omap_plane) {
@@ -506,47 +395,50 @@  struct drm_plane *omap_plane_init(struct drm_device *dev,
 		goto fail;
 	}
 
-	mutex_init(&omap_plane->unpin_mutex);
-
 	ret = kfifo_alloc(&omap_plane->unpin_fifo, 16, GFP_KERNEL);
 	if (ret) {
 		dev_err(dev->dev, "could not allocate unpin FIFO\n");
 		goto fail;
 	}
 
-	INIT_WORK(&omap_plane->work, unpin_worker);
-
 	omap_plane->nformats = omap_framebuffer_get_formats(
 			omap_plane->formats, ARRAY_SIZE(omap_plane->formats),
-			ovl->supported_modes);
-	omap_plane->ovl = ovl;
+			dss_feat_get_supported_color_modes(id));
+	omap_plane->id = id;
+	omap_plane->name = plane_names[id];
+
 	plane = &omap_plane->base;
 
-	drm_plane_init(dev, plane, possible_crtcs, &omap_plane_funcs,
-			omap_plane->formats, omap_plane->nformats, priv);
+	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);
+
+	drm_plane_init(dev, plane, (1 << priv->num_crtcs) - 1, &omap_plane_funcs,
+			omap_plane->formats, omap_plane->nformats, private_plane);
 
 	omap_plane_install_properties(plane, &plane->base);
 
 	/* get our starting configuration, set defaults for parameters
 	 * we don't currently use, etc:
 	 */
-	ovl->get_overlay_info(ovl, &omap_plane->info);
-	omap_plane->info.rotation_type = OMAP_DSS_ROT_DMA;
-	omap_plane->info.rotation = OMAP_DSS_ROT_0;
-	omap_plane->info.global_alpha = 0xff;
-	omap_plane->info.mirror = 0;
+	info = &omap_plane->info;
+	info->rotation_type = OMAP_DSS_ROT_DMA;
+	info->rotation = OMAP_DSS_ROT_0;
+	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 (priv)
+	if (private_plane)
 		omap_plane->info.zorder = 0;
 	else
-		omap_plane->info.zorder = ovl->id;
-
-	update_manager(plane);
+		omap_plane->info.zorder = id;
 
 	return plane;