diff mbox

[01/11] drm/vmwgfx: Add universal plane support

Message ID 1490652064-44817-2-git-send-email-syeh@vmware.com (mailing list archive)
State New, archived
Headers show

Commit Message

Sinclair Yeh March 27, 2017, 10 p.m. UTC
Universal support is prerequisite for atomic mode set.

Explicitly create planes for the cursor and the primary FB.  With
a functional cursor plane, the DRM will no longer use the legacy
cursor_set2 and cursor_move entry points.

Signed-off-by: Sinclair Yeh <syeh@vmware.com>
Signed-off-by: Thomas Hellstrom <thellstrom@vmware.com>
Reviewed-by: Thomas Hellstrom <thellstrom@vmware.com>
---
 drivers/gpu/drm/vmwgfx/vmwgfx_drv.c  |   1 +
 drivers/gpu/drm/vmwgfx/vmwgfx_drv.h  |   1 +
 drivers/gpu/drm/vmwgfx/vmwgfx_kms.c  | 271 +++++++++++++++++------------------
 drivers/gpu/drm/vmwgfx/vmwgfx_kms.h  |  39 +++--
 drivers/gpu/drm/vmwgfx/vmwgfx_ldu.c  | 108 ++++++++++----
 drivers/gpu/drm/vmwgfx/vmwgfx_scrn.c |  87 +++++++++--
 drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c |  89 ++++++++++--
 7 files changed, 398 insertions(+), 198 deletions(-)

Comments

Daniel Vetter March 28, 2017, 7:07 a.m. UTC | #1
On Mon, Mar 27, 2017 at 03:00:54PM -0700, Sinclair Yeh wrote:
> Universal support is prerequisite for atomic mode set.
> 
> Explicitly create planes for the cursor and the primary FB.  With
> a functional cursor plane, the DRM will no longer use the legacy
> cursor_set2 and cursor_move entry points.
> 
> Signed-off-by: Sinclair Yeh <syeh@vmware.com>
> Signed-off-by: Thomas Hellstrom <thellstrom@vmware.com>
> Reviewed-by: Thomas Hellstrom <thellstrom@vmware.com>

So per our discussion I think it'd be good to only have 1 global cursor
plane instead of the fake per-crtc one. At least it sounded like that
would reflect the virtual hw more accurately.
-Daniel

> ---
>  drivers/gpu/drm/vmwgfx/vmwgfx_drv.c  |   1 +
>  drivers/gpu/drm/vmwgfx/vmwgfx_drv.h  |   1 +
>  drivers/gpu/drm/vmwgfx/vmwgfx_kms.c  | 271 +++++++++++++++++------------------
>  drivers/gpu/drm/vmwgfx/vmwgfx_kms.h  |  39 +++--
>  drivers/gpu/drm/vmwgfx/vmwgfx_ldu.c  | 108 ++++++++++----
>  drivers/gpu/drm/vmwgfx/vmwgfx_scrn.c |  87 +++++++++--
>  drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c |  89 ++++++++++--
>  7 files changed, 398 insertions(+), 198 deletions(-)
> 
> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
> index 541a588..45d711e 100644
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
> @@ -645,6 +645,7 @@ static int vmw_driver_load(struct drm_device *dev, unsigned long chipset)
>  	spin_lock_init(&dev_priv->waiter_lock);
>  	spin_lock_init(&dev_priv->cap_lock);
>  	spin_lock_init(&dev_priv->svga_lock);
> +	spin_lock_init(&dev_priv->cursor_lock);
>  
>  	for (i = vmw_res_context; i < vmw_res_max; ++i) {
>  		idr_init(&dev_priv->res_idr[i]);
> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h
> index 9ae4477..ef0181c 100644
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h
> @@ -414,6 +414,7 @@ struct vmw_private {
>  	unsigned num_implicit;
>  	struct vmw_framebuffer *implicit_fb;
>  	struct mutex global_kms_state_mutex;
> +	spinlock_t cursor_lock;
>  
>  	/*
>  	 * Context and surface management.
> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> index 6bcba56..c9f5dda 100644
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> @@ -33,10 +33,9 @@
>  
>  void vmw_du_cleanup(struct vmw_display_unit *du)
>  {
> -	if (du->cursor_surface)
> -		vmw_surface_unreference(&du->cursor_surface);
> -	if (du->cursor_dmabuf)
> -		vmw_dmabuf_unreference(&du->cursor_dmabuf);
> +	drm_plane_cleanup(&du->primary);
> +	drm_plane_cleanup(&du->cursor);
> +
>  	drm_connector_unregister(&du->connector);
>  	drm_crtc_cleanup(&du->crtc);
>  	drm_encoder_cleanup(&du->encoder);
> @@ -47,9 +46,9 @@ void vmw_du_cleanup(struct vmw_display_unit *du)
>   * Display Unit Cursor functions
>   */
>  
> -int vmw_cursor_update_image(struct vmw_private *dev_priv,
> -			    u32 *image, u32 width, u32 height,
> -			    u32 hotspotX, u32 hotspotY)
> +static int vmw_cursor_update_image(struct vmw_private *dev_priv,
> +				   u32 *image, u32 width, u32 height,
> +				   u32 hotspotX, u32 hotspotY)
>  {
>  	struct {
>  		u32 cmd;
> @@ -83,10 +82,10 @@ int vmw_cursor_update_image(struct vmw_private *dev_priv,
>  	return 0;
>  }
>  
> -int vmw_cursor_update_dmabuf(struct vmw_private *dev_priv,
> -			     struct vmw_dma_buffer *dmabuf,
> -			     u32 width, u32 height,
> -			     u32 hotspotX, u32 hotspotY)
> +static int vmw_cursor_update_dmabuf(struct vmw_private *dev_priv,
> +				    struct vmw_dma_buffer *dmabuf,
> +				    u32 width, u32 height,
> +				    u32 hotspotX, u32 hotspotY)
>  {
>  	struct ttm_bo_kmap_obj map;
>  	unsigned long kmap_offset;
> @@ -120,145 +119,22 @@ int vmw_cursor_update_dmabuf(struct vmw_private *dev_priv,
>  }
>  
>  
> -void vmw_cursor_update_position(struct vmw_private *dev_priv,
> -				bool show, int x, int y)
> +static void vmw_cursor_update_position(struct vmw_private *dev_priv,
> +				       bool show, int x, int y)
>  {
>  	u32 *fifo_mem = dev_priv->mmio_virt;
>  	uint32_t count;
>  
> +	spin_lock(&dev_priv->cursor_lock);
>  	vmw_mmio_write(show ? 1 : 0, fifo_mem + SVGA_FIFO_CURSOR_ON);
>  	vmw_mmio_write(x, fifo_mem + SVGA_FIFO_CURSOR_X);
>  	vmw_mmio_write(y, fifo_mem + SVGA_FIFO_CURSOR_Y);
>  	count = vmw_mmio_read(fifo_mem + SVGA_FIFO_CURSOR_COUNT);
>  	vmw_mmio_write(++count, fifo_mem + SVGA_FIFO_CURSOR_COUNT);
> +	spin_unlock(&dev_priv->cursor_lock);
>  }
>  
>  
> -/*
> - * vmw_du_crtc_cursor_set2 - Driver cursor_set2 callback.
> - */
> -int vmw_du_crtc_cursor_set2(struct drm_crtc *crtc, struct drm_file *file_priv,
> -			    uint32_t handle, uint32_t width, uint32_t height,
> -			    int32_t hot_x, int32_t hot_y)
> -{
> -	struct vmw_private *dev_priv = vmw_priv(crtc->dev);
> -	struct vmw_display_unit *du = vmw_crtc_to_du(crtc);
> -	struct vmw_surface *surface = NULL;
> -	struct vmw_dma_buffer *dmabuf = NULL;
> -	s32 hotspot_x, hotspot_y;
> -	int ret;
> -
> -	/*
> -	 * FIXME: Unclear whether there's any global state touched by the
> -	 * cursor_set function, especially vmw_cursor_update_position looks
> -	 * suspicious. For now take the easy route and reacquire all locks. We
> -	 * can do this since the caller in the drm core doesn't check anything
> -	 * which is protected by any looks.
> -	 */
> -	drm_modeset_unlock_crtc(crtc);
> -	drm_modeset_lock_all(dev_priv->dev);
> -	hotspot_x = hot_x + du->hotspot_x;
> -	hotspot_y = hot_y + du->hotspot_y;
> -
> -	/* A lot of the code assumes this */
> -	if (handle && (width != 64 || height != 64)) {
> -		ret = -EINVAL;
> -		goto out;
> -	}
> -
> -	if (handle) {
> -		struct ttm_object_file *tfile = vmw_fpriv(file_priv)->tfile;
> -
> -		ret = vmw_user_lookup_handle(dev_priv, tfile,
> -					     handle, &surface, &dmabuf);
> -		if (ret) {
> -			DRM_ERROR("failed to find surface or dmabuf: %i\n", ret);
> -			ret = -EINVAL;
> -			goto out;
> -		}
> -	}
> -
> -	/* need to do this before taking down old image */
> -	if (surface && !surface->snooper.image) {
> -		DRM_ERROR("surface not suitable for cursor\n");
> -		vmw_surface_unreference(&surface);
> -		ret = -EINVAL;
> -		goto out;
> -	}
> -
> -	/* takedown old cursor */
> -	if (du->cursor_surface) {
> -		vmw_surface_unreference(&du->cursor_surface);
> -	}
> -	if (du->cursor_dmabuf)
> -		vmw_dmabuf_unreference(&du->cursor_dmabuf);
> -
> -	/* setup new image */
> -	ret = 0;
> -	if (surface) {
> -		/* vmw_user_surface_lookup takes one reference */
> -		du->cursor_surface = surface;
> -
> -		du->cursor_age = du->cursor_surface->snooper.age;
> -		ret = vmw_cursor_update_image(dev_priv, surface->snooper.image,
> -					      64, 64, hotspot_x, hotspot_y);
> -	} else if (dmabuf) {
> -		/* vmw_user_surface_lookup takes one reference */
> -		du->cursor_dmabuf = dmabuf;
> -
> -		ret = vmw_cursor_update_dmabuf(dev_priv, dmabuf, width, height,
> -					       hotspot_x, hotspot_y);
> -	} else {
> -		vmw_cursor_update_position(dev_priv, false, 0, 0);
> -		goto out;
> -	}
> -
> -	if (!ret) {
> -		vmw_cursor_update_position(dev_priv, true,
> -					   du->cursor_x + hotspot_x,
> -					   du->cursor_y + hotspot_y);
> -		du->core_hotspot_x = hot_x;
> -		du->core_hotspot_y = hot_y;
> -	}
> -
> -out:
> -	drm_modeset_unlock_all(dev_priv->dev);
> -	drm_modeset_lock_crtc(crtc, crtc->cursor);
> -
> -	return ret;
> -}
> -
> -int vmw_du_crtc_cursor_move(struct drm_crtc *crtc, int x, int y)
> -{
> -	struct vmw_private *dev_priv = vmw_priv(crtc->dev);
> -	struct vmw_display_unit *du = vmw_crtc_to_du(crtc);
> -	bool shown = du->cursor_surface || du->cursor_dmabuf ? true : false;
> -
> -	du->cursor_x = x + du->set_gui_x;
> -	du->cursor_y = y + du->set_gui_y;
> -
> -	/*
> -	 * FIXME: Unclear whether there's any global state touched by the
> -	 * cursor_set function, especially vmw_cursor_update_position looks
> -	 * suspicious. For now take the easy route and reacquire all locks. We
> -	 * can do this since the caller in the drm core doesn't check anything
> -	 * which is protected by any looks.
> -	 */
> -	drm_modeset_unlock_crtc(crtc);
> -	drm_modeset_lock_all(dev_priv->dev);
> -
> -	vmw_cursor_update_position(dev_priv, shown,
> -				   du->cursor_x + du->hotspot_x +
> -				   du->core_hotspot_x,
> -				   du->cursor_y + du->hotspot_y +
> -				   du->core_hotspot_y);
> -
> -	drm_modeset_unlock_all(dev_priv->dev);
> -	drm_modeset_lock_crtc(crtc, crtc->cursor);
> -
> -	return 0;
> -}
> -
>  void vmw_kms_cursor_snoop(struct vmw_surface *srf,
>  			  struct ttm_object_file *tfile,
>  			  struct ttm_buffer_object *bo,
> @@ -393,6 +269,125 @@ void vmw_kms_cursor_post_execbuf(struct vmw_private *dev_priv)
>  	mutex_unlock(&dev->mode_config.mutex);
>  }
>  
> +
> +
> +/**
> + * vmw_du_cursor_plane_update() - Update cursor image and location
> + *
> + * @plane: plane object to update
> + * @crtc: owning CRTC of @plane
> + * @fb: framebuffer to flip onto plane
> + * @crtc_x: x offset of plane on crtc
> + * @crtc_y: y offset of plane on crtc
> + * @crtc_w: width of plane rectangle on crtc
> + * @crtc_h: height of plane rectangle on crtc
> + * @src_x: Not used
> + * @src_y: Not used
> + * @src_w: Not used
> + * @src_h: Not used
> + *
> + *
> + * RETURNS:
> + * Zero on success, error code on failure
> + */
> +int vmw_du_cursor_plane_update(struct drm_plane *plane,
> +			       struct drm_crtc *crtc,
> +			       struct drm_framebuffer *fb,
> +			       int crtc_x, int crtc_y,
> +			       unsigned int crtc_w,
> +			       unsigned int crtc_h,
> +			       uint32_t src_x, uint32_t src_y,
> +			       uint32_t src_w, uint32_t src_h)
> +{
> +	struct vmw_private *dev_priv = vmw_priv(crtc->dev);
> +	struct vmw_display_unit *du = vmw_crtc_to_du(crtc);
> +	struct vmw_surface *surface = NULL;
> +	struct vmw_dma_buffer *dmabuf = NULL;
> +	s32 hotspot_x, hotspot_y;
> +	int ret;
> +
> +	hotspot_x = du->hotspot_x + fb->hot_x;
> +	hotspot_y = du->hotspot_y + fb->hot_y;
> +
> +	/* A lot of the code assumes this */
> +	if (crtc_w != 64 || crtc_h != 64) {
> +		ret = -EINVAL;
> +		goto out;
> +	}
> +
> +	if (vmw_framebuffer_to_vfb(fb)->dmabuf)
> +		dmabuf = vmw_framebuffer_to_vfbd(fb)->buffer;
> +	else
> +		surface = vmw_framebuffer_to_vfbs(fb)->surface;
> +
> +	if (surface && !surface->snooper.image) {
> +		DRM_ERROR("surface not suitable for cursor\n");
> +		ret = -EINVAL;
> +		goto out;
> +	}
> +
> +	/* setup new image */
> +	ret = 0;
> +	if (surface) {
> +		/* vmw_user_surface_lookup takes one reference */
> +		du->cursor_surface = surface;
> +
> +		du->cursor_age = du->cursor_surface->snooper.age;
> +
> +		ret = vmw_cursor_update_image(dev_priv, surface->snooper.image,
> +					      64, 64, hotspot_x, hotspot_y);
> +	} else if (dmabuf) {
> +		/* vmw_user_surface_lookup takes one reference */
> +		du->cursor_dmabuf = dmabuf;
> +
> +		ret = vmw_cursor_update_dmabuf(dev_priv, dmabuf, crtc_w, crtc_h,
> +					       hotspot_x, hotspot_y);
> +	} else {
> +		vmw_cursor_update_position(dev_priv, false, 0, 0);
> +		goto out;
> +	}
> +
> +	if (!ret) {
> +		du->cursor_x = crtc_x + du->set_gui_x;
> +		du->cursor_y = crtc_y + du->set_gui_y;
> +
> +		vmw_cursor_update_position(dev_priv, true,
> +					   du->cursor_x + hotspot_x,
> +					   du->cursor_y + hotspot_y);
> +	}
> +
> +out:
> +	return ret;
> +}
> +
> +
> +int vmw_du_cursor_plane_disable(struct drm_plane *plane)
> +{
> +	if (plane->fb) {
> +		drm_framebuffer_unreference(plane->fb);
> +		plane->fb = NULL;
> +	}
> +
> +	return -EINVAL;
> +}
> +
> +
> +void vmw_du_cursor_plane_destroy(struct drm_plane *plane)
> +{
> +	vmw_cursor_update_position(plane->dev->dev_private, false, 0, 0);
> +
> +	drm_plane_cleanup(plane);
> +}
> +
> +
> +void vmw_du_primary_plane_destroy(struct drm_plane *plane)
> +{
> +	drm_plane_cleanup(plane);
> +
> +	/* Planes are static in our case so we don't free it */
> +}
> +
> +
>  /*
>   * Generic framebuffer code
>   */
> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.h b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.h
> index cb36e1d..e400bfb 100644
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.h
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.h
> @@ -33,6 +33,8 @@
>  #include <drm/drm_encoder.h>
>  #include "vmwgfx_drv.h"
>  
> +
> +
>  /**
>   * struct vmw_kms_dirty - closure structure for the vmw_kms_helper_dirty
>   * function.
> @@ -125,19 +127,17 @@ struct vmw_framebuffer_dmabuf {
>  };
>  
>  
> -/*
> - * Basic cursor manipulation
> - */
> -int vmw_cursor_update_image(struct vmw_private *dev_priv,
> -			    u32 *image, u32 width, u32 height,
> -			    u32 hotspotX, u32 hotspotY);
> -int vmw_cursor_update_dmabuf(struct vmw_private *dev_priv,
> -			     struct vmw_dma_buffer *dmabuf,
> -			     u32 width, u32 height,
> -			     u32 hotspotX, u32 hotspotY);
> -void vmw_cursor_update_position(struct vmw_private *dev_priv,
> -				bool show, int x, int y);
> +static const uint32_t vmw_primary_plane_formats[] = {
> +	DRM_FORMAT_XRGB1555,
> +	DRM_FORMAT_RGB565,
> +	DRM_FORMAT_RGB888,
> +	DRM_FORMAT_XRGB8888,
> +	DRM_FORMAT_ARGB8888,
> +};
>  
> +static const uint32_t vmw_cursor_plane_formats[] = {
> +	DRM_FORMAT_ARGB8888,
> +};
>  
>  /**
>   * Base class display unit.
> @@ -150,6 +150,8 @@ struct vmw_display_unit {
>  	struct drm_crtc crtc;
>  	struct drm_encoder encoder;
>  	struct drm_connector connector;
> +	struct drm_plane primary;
> +	struct drm_plane cursor;
>  
>  	struct vmw_surface *cursor_surface;
>  	struct vmw_dma_buffer *cursor_dmabuf;
> @@ -270,6 +272,19 @@ void vmw_kms_update_implicit_fb(struct vmw_private *dev_priv,
>  void vmw_kms_create_implicit_placement_property(struct vmw_private *dev_priv,
>  						bool immutable);
>  
> +/* Universal Plane Helpers */
> +void vmw_du_primary_plane_destroy(struct drm_plane *plane);
> +void vmw_du_cursor_plane_destroy(struct drm_plane *plane);
> +int vmw_du_cursor_plane_disable(struct drm_plane *plane);
> +int vmw_du_cursor_plane_update(struct drm_plane *plane,
> +			       struct drm_crtc *crtc,
> +			       struct drm_framebuffer *fb,
> +			       int crtc_x, int crtc_y,
> +			       unsigned int crtc_w,
> +			       unsigned int crtc_h,
> +			       uint32_t src_x, uint32_t src_y,
> +			       uint32_t src_w, uint32_t src_h);
> +
>  
>  /*
>   * Legacy display unit functions - vmwgfx_ldu.c
> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_ldu.c b/drivers/gpu/drm/vmwgfx/vmwgfx_ldu.c
> index 3806148..3efcbe5 100644
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_ldu.c
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_ldu.c
> @@ -75,10 +75,9 @@ static int vmw_ldu_commit_list(struct vmw_private *dev_priv)
>  {
>  	struct vmw_legacy_display *lds = dev_priv->ldu_priv;
>  	struct vmw_legacy_display_unit *entry;
> -	struct vmw_display_unit *du = NULL;
>  	struct drm_framebuffer *fb = NULL;
>  	struct drm_crtc *crtc = NULL;
> -	int i = 0, ret;
> +	int i = 0;
>  
>  	/* If there is no display topology the host just assumes
>  	 * that the guest will set the same layout as the host.
> @@ -132,25 +131,6 @@ static int vmw_ldu_commit_list(struct vmw_private *dev_priv)
>  
>  	lds->last_num_active = lds->num_active;
>  
> -
> -	/* Find the first du with a cursor. */
> -	list_for_each_entry(entry, &lds->active, active) {
> -		du = &entry->base;
> -
> -		if (!du->cursor_dmabuf)
> -			continue;
> -
> -		ret = vmw_cursor_update_dmabuf(dev_priv,
> -					       du->cursor_dmabuf,
> -					       64, 64,
> -					       du->hotspot_x,
> -					       du->hotspot_y);
> -		if (ret == 0)
> -			break;
> -
> -		DRM_ERROR("Could not update cursor image\n");
> -	}
> -
>  	return 0;
>  }
>  
> @@ -298,8 +278,6 @@ static int vmw_ldu_crtc_set_config(struct drm_mode_set *set)
>  }
>  
>  static const struct drm_crtc_funcs vmw_legacy_crtc_funcs = {
> -	.cursor_set2 = vmw_du_crtc_cursor_set2,
> -	.cursor_move = vmw_du_crtc_cursor_move,
>  	.gamma_set = vmw_du_crtc_gamma_set,
>  	.destroy = vmw_ldu_crtc_destroy,
>  	.set_config = vmw_ldu_crtc_set_config,
> @@ -336,6 +314,23 @@ static const struct drm_connector_funcs vmw_legacy_connector_funcs = {
>  	.destroy = vmw_ldu_connector_destroy,
>  };
>  
> +/*
> + * Legacy Display Plane Functions
> + */
> +
> +static const struct drm_plane_funcs vmw_ldu_plane_funcs = {
> +	.update_plane = drm_primary_helper_update,
> +	.disable_plane = drm_primary_helper_disable,
> +	.destroy = vmw_du_primary_plane_destroy,
> +};
> +
> +static const struct drm_plane_funcs vmw_ldu_cursor_funcs = {
> +	.update_plane = vmw_du_cursor_plane_update,
> +	.disable_plane = vmw_du_cursor_plane_disable,
> +	.destroy = vmw_du_cursor_plane_destroy,
> +};
> +
> +
>  static int vmw_ldu_init(struct vmw_private *dev_priv, unsigned unit)
>  {
>  	struct vmw_legacy_display_unit *ldu;
> @@ -343,6 +338,7 @@ static int vmw_ldu_init(struct vmw_private *dev_priv, unsigned unit)
>  	struct drm_connector *connector;
>  	struct drm_encoder *encoder;
>  	struct drm_crtc *crtc;
> +	int ret;
>  
>  	ldu = kzalloc(sizeof(*ldu), GFP_KERNEL);
>  	if (!ldu)
> @@ -361,19 +357,61 @@ static int vmw_ldu_init(struct vmw_private *dev_priv, unsigned unit)
>  	ldu->base.pref_mode = NULL;
>  	ldu->base.is_implicit = true;
>  
> -	drm_connector_init(dev, connector, &vmw_legacy_connector_funcs,
> -			   DRM_MODE_CONNECTOR_VIRTUAL);
> +	/* Initialize primary plane */
> +	ret = drm_universal_plane_init(dev, &ldu->base.primary,
> +				       0, &vmw_ldu_plane_funcs,
> +				       vmw_primary_plane_formats,
> +				       ARRAY_SIZE(vmw_primary_plane_formats),
> +				       DRM_PLANE_TYPE_PRIMARY, NULL);
> +	if (ret) {
> +		DRM_ERROR("Failed to initialize primary plane");
> +		goto err_free;
> +	}
> +
> +	/* Initialize cursor plane */
> +	ret = drm_universal_plane_init(dev, &ldu->base.cursor,
> +			0, &vmw_ldu_cursor_funcs,
> +			vmw_cursor_plane_formats,
> +			ARRAY_SIZE(vmw_cursor_plane_formats),
> +			DRM_PLANE_TYPE_CURSOR, NULL);
> +	if (ret) {
> +		DRM_ERROR("Failed to initialize cursor plane");
> +		drm_plane_cleanup(&ldu->base.primary);
> +		goto err_free;
> +	}
> +
> +	ret = drm_connector_init(dev, connector, &vmw_legacy_connector_funcs,
> +				 DRM_MODE_CONNECTOR_VIRTUAL);
> +	if (ret) {
> +		DRM_ERROR("Failed to initialize connector\n");
> +		goto err_free;
> +	}
>  	connector->status = vmw_du_connector_detect(connector, true);
>  
> -	drm_encoder_init(dev, encoder, &vmw_legacy_encoder_funcs,
> -			 DRM_MODE_ENCODER_VIRTUAL, NULL);
> -	drm_mode_connector_attach_encoder(connector, encoder);
> +	ret = drm_encoder_init(dev, encoder, &vmw_legacy_encoder_funcs,
> +			       DRM_MODE_ENCODER_VIRTUAL, NULL);
> +	if (ret) {
> +		DRM_ERROR("Failed to initialize encoder\n");
> +		goto err_free_connector;
> +	}
> +
> +	(void) drm_mode_connector_attach_encoder(connector, encoder);
>  	encoder->possible_crtcs = (1 << unit);
>  	encoder->possible_clones = 0;
>  
> -	(void) drm_connector_register(connector);
> +	ret = drm_connector_register(connector);
> +	if (ret) {
> +		DRM_ERROR("Failed to register connector\n");
> +		goto err_free_encoder;
> +	}
>  
> -	drm_crtc_init(dev, crtc, &vmw_legacy_crtc_funcs);
> +	ret = drm_crtc_init_with_planes(dev, crtc, &ldu->base.primary,
> +					&ldu->base.cursor,
> +					&vmw_legacy_crtc_funcs, NULL);
> +	if (ret) {
> +		DRM_ERROR("Failed to initialize CRTC\n");
> +		goto err_free_unregister;
> +	}
>  
>  	drm_mode_crtc_set_gamma_size(crtc, 256);
>  
> @@ -390,6 +428,16 @@ static int vmw_ldu_init(struct vmw_private *dev_priv, unsigned unit)
>  			 1);
>  
>  	return 0;
> +
> +err_free_unregister:
> +	drm_connector_unregister(connector);
> +err_free_encoder:
> +	drm_encoder_cleanup(encoder);
> +err_free_connector:
> +	drm_connector_cleanup(connector);
> +err_free:
> +	kfree(ldu);
> +	return ret;
>  }
>  
>  int vmw_kms_ldu_init_display(struct vmw_private *dev_priv)
> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_scrn.c b/drivers/gpu/drm/vmwgfx/vmwgfx_scrn.c
> index d4268ef..8ffccb8 100644
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_scrn.c
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_scrn.c
> @@ -459,8 +459,6 @@ static int vmw_sou_crtc_page_flip(struct drm_crtc *crtc,
>  }
>  
>  static const struct drm_crtc_funcs vmw_screen_object_crtc_funcs = {
> -	.cursor_set2 = vmw_du_crtc_cursor_set2,
> -	.cursor_move = vmw_du_crtc_cursor_move,
>  	.gamma_set = vmw_du_crtc_gamma_set,
>  	.destroy = vmw_sou_crtc_destroy,
>  	.set_config = vmw_sou_crtc_set_config,
> @@ -497,6 +495,23 @@ static const struct drm_connector_funcs vmw_sou_connector_funcs = {
>  	.destroy = vmw_sou_connector_destroy,
>  };
>  
> +/*
> + * Screen Object Display Plane Functions
> + */
> +
> +static const struct drm_plane_funcs vmw_sou_plane_funcs = {
> +	.update_plane = drm_primary_helper_update,
> +	.disable_plane = drm_primary_helper_disable,
> +	.destroy = vmw_du_primary_plane_destroy,
> +};
> +
> +static const struct drm_plane_funcs vmw_sou_cursor_funcs = {
> +	.update_plane = vmw_du_cursor_plane_update,
> +	.disable_plane = vmw_du_cursor_plane_disable,
> +	.destroy = vmw_du_cursor_plane_destroy,
> +};
> +
> +
>  static int vmw_sou_init(struct vmw_private *dev_priv, unsigned unit)
>  {
>  	struct vmw_screen_object_unit *sou;
> @@ -504,6 +519,7 @@ static int vmw_sou_init(struct vmw_private *dev_priv, unsigned unit)
>  	struct drm_connector *connector;
>  	struct drm_encoder *encoder;
>  	struct drm_crtc *crtc;
> +	int ret;
>  
>  	sou = kzalloc(sizeof(*sou), GFP_KERNEL);
>  	if (!sou)
> @@ -521,19 +537,62 @@ static int vmw_sou_init(struct vmw_private *dev_priv, unsigned unit)
>  	sou->base.pref_mode = NULL;
>  	sou->base.is_implicit = false;
>  
> -	drm_connector_init(dev, connector, &vmw_sou_connector_funcs,
> -			   DRM_MODE_CONNECTOR_VIRTUAL);
> +	/* Initialize primary plane */
> +	ret = drm_universal_plane_init(dev, &sou->base.primary,
> +				       0, &vmw_sou_plane_funcs,
> +				       vmw_primary_plane_formats,
> +				       ARRAY_SIZE(vmw_primary_plane_formats),
> +				       DRM_PLANE_TYPE_PRIMARY, NULL);
> +	if (ret) {
> +		DRM_ERROR("Failed to initialize primary plane");
> +		goto err_free;
> +	}
> +
> +	/* Initialize cursor plane */
> +	ret = drm_universal_plane_init(dev, &sou->base.cursor,
> +			0, &vmw_sou_cursor_funcs,
> +			vmw_cursor_plane_formats,
> +			ARRAY_SIZE(vmw_cursor_plane_formats),
> +			DRM_PLANE_TYPE_CURSOR, NULL);
> +	if (ret) {
> +		DRM_ERROR("Failed to initialize cursor plane");
> +		drm_plane_cleanup(&sou->base.primary);
> +		goto err_free;
> +	}
> +
> +	ret = drm_connector_init(dev, connector, &vmw_sou_connector_funcs,
> +				 DRM_MODE_CONNECTOR_VIRTUAL);
> +	if (ret) {
> +		DRM_ERROR("Failed to initialize connector\n");
> +		goto err_free;
> +	}
> +
>  	connector->status = vmw_du_connector_detect(connector, true);
>  
> -	drm_encoder_init(dev, encoder, &vmw_screen_object_encoder_funcs,
> -			 DRM_MODE_ENCODER_VIRTUAL, NULL);
> -	drm_mode_connector_attach_encoder(connector, encoder);
> +	ret = drm_encoder_init(dev, encoder, &vmw_screen_object_encoder_funcs,
> +			       DRM_MODE_ENCODER_VIRTUAL, NULL);
> +	if (ret) {
> +		DRM_ERROR("Failed to initialize encoder\n");
> +		goto err_free_connector;
> +	}
> +
> +	(void) drm_mode_connector_attach_encoder(connector, encoder);
>  	encoder->possible_crtcs = (1 << unit);
>  	encoder->possible_clones = 0;
>  
> -	(void) drm_connector_register(connector);
> +	ret = drm_connector_register(connector);
> +	if (ret) {
> +		DRM_ERROR("Failed to register connector\n");
> +		goto err_free_encoder;
> +	}
>  
> -	drm_crtc_init(dev, crtc, &vmw_screen_object_crtc_funcs);
> +	ret = drm_crtc_init_with_planes(dev, crtc, &sou->base.primary,
> +					&sou->base.cursor,
> +					&vmw_screen_object_crtc_funcs, NULL);
> +	if (ret) {
> +		DRM_ERROR("Failed to initialize CRTC\n");
> +		goto err_free_unregister;
> +	}
>  
>  	drm_mode_crtc_set_gamma_size(crtc, 256);
>  
> @@ -550,6 +609,16 @@ static int vmw_sou_init(struct vmw_private *dev_priv, unsigned unit)
>  			 sou->base.is_implicit);
>  
>  	return 0;
> +
> +err_free_unregister:
> +	drm_connector_unregister(connector);
> +err_free_encoder:
> +	drm_encoder_cleanup(encoder);
> +err_free_connector:
> +	drm_connector_cleanup(connector);
> +err_free:
> +	kfree(sou);
> +	return ret;
>  }
>  
>  int vmw_kms_sou_init_display(struct vmw_private *dev_priv)
> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c b/drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c
> index b27cd18..4d9dd1b 100644
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c
> @@ -1015,8 +1015,6 @@ int vmw_kms_stdu_surface_dirty(struct vmw_private *dev_priv,
>   *  Screen Target CRTC dispatch table
>   */
>  static const struct drm_crtc_funcs vmw_stdu_crtc_funcs = {
> -	.cursor_set2 = vmw_du_crtc_cursor_set2,
> -	.cursor_move = vmw_du_crtc_cursor_move,
>  	.gamma_set = vmw_du_crtc_gamma_set,
>  	.destroy = vmw_stdu_crtc_destroy,
>  	.set_config = vmw_stdu_crtc_set_config,
> @@ -1081,6 +1079,23 @@ static const struct drm_connector_funcs vmw_stdu_connector_funcs = {
>  
>  
>  
> +/******************************************************************************
> + * Screen Target Display Plane Functions
> + *****************************************************************************/
> +
> +static const struct drm_plane_funcs vmw_stdu_plane_funcs = {
> +	.update_plane = drm_primary_helper_update,
> +	.disable_plane = drm_primary_helper_disable,
> +	.destroy = vmw_du_primary_plane_destroy,
> +};
> +
> +static const struct drm_plane_funcs vmw_stdu_cursor_funcs = {
> +	.update_plane = vmw_du_cursor_plane_update,
> +	.disable_plane = vmw_du_cursor_plane_disable,
> +	.destroy = vmw_du_cursor_plane_destroy,
> +};
> +
> +
>  /**
>   * vmw_stdu_init - Sets up a Screen Target Display Unit
>   *
> @@ -1097,7 +1112,9 @@ static int vmw_stdu_init(struct vmw_private *dev_priv, unsigned unit)
>  	struct drm_device *dev = dev_priv->dev;
>  	struct drm_connector *connector;
>  	struct drm_encoder *encoder;
> +	struct drm_plane *primary, *cursor;
>  	struct drm_crtc *crtc;
> +	int    ret;
>  
>  
>  	stdu = kzalloc(sizeof(*stdu), GFP_KERNEL);
> @@ -1108,25 +1125,69 @@ static int vmw_stdu_init(struct vmw_private *dev_priv, unsigned unit)
>  	crtc = &stdu->base.crtc;
>  	encoder = &stdu->base.encoder;
>  	connector = &stdu->base.connector;
> +	primary = &stdu->base.primary;
> +	cursor = &stdu->base.cursor;
>  
>  	stdu->base.pref_active = (unit == 0);
>  	stdu->base.pref_width  = dev_priv->initial_width;
>  	stdu->base.pref_height = dev_priv->initial_height;
>  	stdu->base.is_implicit = false;
>  
> -	drm_connector_init(dev, connector, &vmw_stdu_connector_funcs,
> -			   DRM_MODE_CONNECTOR_VIRTUAL);
> +	/* Initialize primary plane */
> +	ret = drm_universal_plane_init(dev, primary,
> +				       0, &vmw_stdu_plane_funcs,
> +				       vmw_primary_plane_formats,
> +				       ARRAY_SIZE(vmw_primary_plane_formats),
> +				       DRM_PLANE_TYPE_PRIMARY, NULL);
> +	if (ret) {
> +		DRM_ERROR("Failed to initialize primary plane");
> +		goto err_free;
> +	}
> +
> +	/* Initialize cursor plane */
> +	ret = drm_universal_plane_init(dev, cursor,
> +			0, &vmw_stdu_cursor_funcs,
> +			vmw_cursor_plane_formats,
> +			ARRAY_SIZE(vmw_cursor_plane_formats),
> +			DRM_PLANE_TYPE_CURSOR, NULL);
> +	if (ret) {
> +		DRM_ERROR("Failed to initialize cursor plane");
> +		drm_plane_cleanup(&stdu->base.primary);
> +		goto err_free;
> +	}
> +
> +	ret = drm_connector_init(dev, connector, &vmw_stdu_connector_funcs,
> +				 DRM_MODE_CONNECTOR_VIRTUAL);
> +	if (ret) {
> +		DRM_ERROR("Failed to initialize connector\n");
> +		goto err_free;
> +	}
>  	connector->status = vmw_du_connector_detect(connector, false);
>  
> -	drm_encoder_init(dev, encoder, &vmw_stdu_encoder_funcs,
> -			 DRM_MODE_ENCODER_VIRTUAL, NULL);
> -	drm_mode_connector_attach_encoder(connector, encoder);
> +	ret = drm_encoder_init(dev, encoder, &vmw_stdu_encoder_funcs,
> +			       DRM_MODE_ENCODER_VIRTUAL, NULL);
> +	if (ret) {
> +		DRM_ERROR("Failed to initialize encoder\n");
> +		goto err_free_connector;
> +	}
> +
> +	(void) drm_mode_connector_attach_encoder(connector, encoder);
>  	encoder->possible_crtcs = (1 << unit);
>  	encoder->possible_clones = 0;
>  
> -	(void) drm_connector_register(connector);
> +	ret = drm_connector_register(connector);
> +	if (ret) {
> +		DRM_ERROR("Failed to register connector\n");
> +		goto err_free_encoder;
> +	}
>  
> -	drm_crtc_init(dev, crtc, &vmw_stdu_crtc_funcs);
> +	ret = drm_crtc_init_with_planes(dev, crtc, &stdu->base.primary,
> +					&stdu->base.cursor,
> +					&vmw_stdu_crtc_funcs, NULL);
> +	if (ret) {
> +		DRM_ERROR("Failed to initialize CRTC\n");
> +		goto err_free_unregister;
> +	}
>  
>  	drm_mode_crtc_set_gamma_size(crtc, 256);
>  
> @@ -1142,6 +1203,16 @@ static int vmw_stdu_init(struct vmw_private *dev_priv, unsigned unit)
>  			 dev_priv->implicit_placement_property,
>  			 stdu->base.is_implicit);
>  	return 0;
> +
> +err_free_unregister:
> +	drm_connector_unregister(connector);
> +err_free_encoder:
> +	drm_encoder_cleanup(encoder);
> +err_free_connector:
> +	drm_connector_cleanup(connector);
> +err_free:
> +	kfree(stdu);
> +	return ret;
>  }
>  
>  
> -- 
> 2.7.4
>
diff mbox

Patch

diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
index 541a588..45d711e 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.c
@@ -645,6 +645,7 @@  static int vmw_driver_load(struct drm_device *dev, unsigned long chipset)
 	spin_lock_init(&dev_priv->waiter_lock);
 	spin_lock_init(&dev_priv->cap_lock);
 	spin_lock_init(&dev_priv->svga_lock);
+	spin_lock_init(&dev_priv->cursor_lock);
 
 	for (i = vmw_res_context; i < vmw_res_max; ++i) {
 		idr_init(&dev_priv->res_idr[i]);
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h
index 9ae4477..ef0181c 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h
@@ -414,6 +414,7 @@  struct vmw_private {
 	unsigned num_implicit;
 	struct vmw_framebuffer *implicit_fb;
 	struct mutex global_kms_state_mutex;
+	spinlock_t cursor_lock;
 
 	/*
 	 * Context and surface management.
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
index 6bcba56..c9f5dda 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
@@ -33,10 +33,9 @@ 
 
 void vmw_du_cleanup(struct vmw_display_unit *du)
 {
-	if (du->cursor_surface)
-		vmw_surface_unreference(&du->cursor_surface);
-	if (du->cursor_dmabuf)
-		vmw_dmabuf_unreference(&du->cursor_dmabuf);
+	drm_plane_cleanup(&du->primary);
+	drm_plane_cleanup(&du->cursor);
+
 	drm_connector_unregister(&du->connector);
 	drm_crtc_cleanup(&du->crtc);
 	drm_encoder_cleanup(&du->encoder);
@@ -47,9 +46,9 @@  void vmw_du_cleanup(struct vmw_display_unit *du)
  * Display Unit Cursor functions
  */
 
-int vmw_cursor_update_image(struct vmw_private *dev_priv,
-			    u32 *image, u32 width, u32 height,
-			    u32 hotspotX, u32 hotspotY)
+static int vmw_cursor_update_image(struct vmw_private *dev_priv,
+				   u32 *image, u32 width, u32 height,
+				   u32 hotspotX, u32 hotspotY)
 {
 	struct {
 		u32 cmd;
@@ -83,10 +82,10 @@  int vmw_cursor_update_image(struct vmw_private *dev_priv,
 	return 0;
 }
 
-int vmw_cursor_update_dmabuf(struct vmw_private *dev_priv,
-			     struct vmw_dma_buffer *dmabuf,
-			     u32 width, u32 height,
-			     u32 hotspotX, u32 hotspotY)
+static int vmw_cursor_update_dmabuf(struct vmw_private *dev_priv,
+				    struct vmw_dma_buffer *dmabuf,
+				    u32 width, u32 height,
+				    u32 hotspotX, u32 hotspotY)
 {
 	struct ttm_bo_kmap_obj map;
 	unsigned long kmap_offset;
@@ -120,145 +119,22 @@  int vmw_cursor_update_dmabuf(struct vmw_private *dev_priv,
 }
 
 
-void vmw_cursor_update_position(struct vmw_private *dev_priv,
-				bool show, int x, int y)
+static void vmw_cursor_update_position(struct vmw_private *dev_priv,
+				       bool show, int x, int y)
 {
 	u32 *fifo_mem = dev_priv->mmio_virt;
 	uint32_t count;
 
+	spin_lock(&dev_priv->cursor_lock);
 	vmw_mmio_write(show ? 1 : 0, fifo_mem + SVGA_FIFO_CURSOR_ON);
 	vmw_mmio_write(x, fifo_mem + SVGA_FIFO_CURSOR_X);
 	vmw_mmio_write(y, fifo_mem + SVGA_FIFO_CURSOR_Y);
 	count = vmw_mmio_read(fifo_mem + SVGA_FIFO_CURSOR_COUNT);
 	vmw_mmio_write(++count, fifo_mem + SVGA_FIFO_CURSOR_COUNT);
+	spin_unlock(&dev_priv->cursor_lock);
 }
 
 
-/*
- * vmw_du_crtc_cursor_set2 - Driver cursor_set2 callback.
- */
-int vmw_du_crtc_cursor_set2(struct drm_crtc *crtc, struct drm_file *file_priv,
-			    uint32_t handle, uint32_t width, uint32_t height,
-			    int32_t hot_x, int32_t hot_y)
-{
-	struct vmw_private *dev_priv = vmw_priv(crtc->dev);
-	struct vmw_display_unit *du = vmw_crtc_to_du(crtc);
-	struct vmw_surface *surface = NULL;
-	struct vmw_dma_buffer *dmabuf = NULL;
-	s32 hotspot_x, hotspot_y;
-	int ret;
-
-	/*
-	 * FIXME: Unclear whether there's any global state touched by the
-	 * cursor_set function, especially vmw_cursor_update_position looks
-	 * suspicious. For now take the easy route and reacquire all locks. We
-	 * can do this since the caller in the drm core doesn't check anything
-	 * which is protected by any looks.
-	 */
-	drm_modeset_unlock_crtc(crtc);
-	drm_modeset_lock_all(dev_priv->dev);
-	hotspot_x = hot_x + du->hotspot_x;
-	hotspot_y = hot_y + du->hotspot_y;
-
-	/* A lot of the code assumes this */
-	if (handle && (width != 64 || height != 64)) {
-		ret = -EINVAL;
-		goto out;
-	}
-
-	if (handle) {
-		struct ttm_object_file *tfile = vmw_fpriv(file_priv)->tfile;
-
-		ret = vmw_user_lookup_handle(dev_priv, tfile,
-					     handle, &surface, &dmabuf);
-		if (ret) {
-			DRM_ERROR("failed to find surface or dmabuf: %i\n", ret);
-			ret = -EINVAL;
-			goto out;
-		}
-	}
-
-	/* need to do this before taking down old image */
-	if (surface && !surface->snooper.image) {
-		DRM_ERROR("surface not suitable for cursor\n");
-		vmw_surface_unreference(&surface);
-		ret = -EINVAL;
-		goto out;
-	}
-
-	/* takedown old cursor */
-	if (du->cursor_surface) {
-		vmw_surface_unreference(&du->cursor_surface);
-	}
-	if (du->cursor_dmabuf)
-		vmw_dmabuf_unreference(&du->cursor_dmabuf);
-
-	/* setup new image */
-	ret = 0;
-	if (surface) {
-		/* vmw_user_surface_lookup takes one reference */
-		du->cursor_surface = surface;
-
-		du->cursor_age = du->cursor_surface->snooper.age;
-		ret = vmw_cursor_update_image(dev_priv, surface->snooper.image,
-					      64, 64, hotspot_x, hotspot_y);
-	} else if (dmabuf) {
-		/* vmw_user_surface_lookup takes one reference */
-		du->cursor_dmabuf = dmabuf;
-
-		ret = vmw_cursor_update_dmabuf(dev_priv, dmabuf, width, height,
-					       hotspot_x, hotspot_y);
-	} else {
-		vmw_cursor_update_position(dev_priv, false, 0, 0);
-		goto out;
-	}
-
-	if (!ret) {
-		vmw_cursor_update_position(dev_priv, true,
-					   du->cursor_x + hotspot_x,
-					   du->cursor_y + hotspot_y);
-		du->core_hotspot_x = hot_x;
-		du->core_hotspot_y = hot_y;
-	}
-
-out:
-	drm_modeset_unlock_all(dev_priv->dev);
-	drm_modeset_lock_crtc(crtc, crtc->cursor);
-
-	return ret;
-}
-
-int vmw_du_crtc_cursor_move(struct drm_crtc *crtc, int x, int y)
-{
-	struct vmw_private *dev_priv = vmw_priv(crtc->dev);
-	struct vmw_display_unit *du = vmw_crtc_to_du(crtc);
-	bool shown = du->cursor_surface || du->cursor_dmabuf ? true : false;
-
-	du->cursor_x = x + du->set_gui_x;
-	du->cursor_y = y + du->set_gui_y;
-
-	/*
-	 * FIXME: Unclear whether there's any global state touched by the
-	 * cursor_set function, especially vmw_cursor_update_position looks
-	 * suspicious. For now take the easy route and reacquire all locks. We
-	 * can do this since the caller in the drm core doesn't check anything
-	 * which is protected by any looks.
-	 */
-	drm_modeset_unlock_crtc(crtc);
-	drm_modeset_lock_all(dev_priv->dev);
-
-	vmw_cursor_update_position(dev_priv, shown,
-				   du->cursor_x + du->hotspot_x +
-				   du->core_hotspot_x,
-				   du->cursor_y + du->hotspot_y +
-				   du->core_hotspot_y);
-
-	drm_modeset_unlock_all(dev_priv->dev);
-	drm_modeset_lock_crtc(crtc, crtc->cursor);
-
-	return 0;
-}
-
 void vmw_kms_cursor_snoop(struct vmw_surface *srf,
 			  struct ttm_object_file *tfile,
 			  struct ttm_buffer_object *bo,
@@ -393,6 +269,125 @@  void vmw_kms_cursor_post_execbuf(struct vmw_private *dev_priv)
 	mutex_unlock(&dev->mode_config.mutex);
 }
 
+
+
+/**
+ * vmw_du_cursor_plane_update() - Update cursor image and location
+ *
+ * @plane: plane object to update
+ * @crtc: owning CRTC of @plane
+ * @fb: framebuffer to flip onto plane
+ * @crtc_x: x offset of plane on crtc
+ * @crtc_y: y offset of plane on crtc
+ * @crtc_w: width of plane rectangle on crtc
+ * @crtc_h: height of plane rectangle on crtc
+ * @src_x: Not used
+ * @src_y: Not used
+ * @src_w: Not used
+ * @src_h: Not used
+ *
+ *
+ * RETURNS:
+ * Zero on success, error code on failure
+ */
+int vmw_du_cursor_plane_update(struct drm_plane *plane,
+			       struct drm_crtc *crtc,
+			       struct drm_framebuffer *fb,
+			       int crtc_x, int crtc_y,
+			       unsigned int crtc_w,
+			       unsigned int crtc_h,
+			       uint32_t src_x, uint32_t src_y,
+			       uint32_t src_w, uint32_t src_h)
+{
+	struct vmw_private *dev_priv = vmw_priv(crtc->dev);
+	struct vmw_display_unit *du = vmw_crtc_to_du(crtc);
+	struct vmw_surface *surface = NULL;
+	struct vmw_dma_buffer *dmabuf = NULL;
+	s32 hotspot_x, hotspot_y;
+	int ret;
+
+	hotspot_x = du->hotspot_x + fb->hot_x;
+	hotspot_y = du->hotspot_y + fb->hot_y;
+
+	/* A lot of the code assumes this */
+	if (crtc_w != 64 || crtc_h != 64) {
+		ret = -EINVAL;
+		goto out;
+	}
+
+	if (vmw_framebuffer_to_vfb(fb)->dmabuf)
+		dmabuf = vmw_framebuffer_to_vfbd(fb)->buffer;
+	else
+		surface = vmw_framebuffer_to_vfbs(fb)->surface;
+
+	if (surface && !surface->snooper.image) {
+		DRM_ERROR("surface not suitable for cursor\n");
+		ret = -EINVAL;
+		goto out;
+	}
+
+	/* setup new image */
+	ret = 0;
+	if (surface) {
+		/* vmw_user_surface_lookup takes one reference */
+		du->cursor_surface = surface;
+
+		du->cursor_age = du->cursor_surface->snooper.age;
+
+		ret = vmw_cursor_update_image(dev_priv, surface->snooper.image,
+					      64, 64, hotspot_x, hotspot_y);
+	} else if (dmabuf) {
+		/* vmw_user_surface_lookup takes one reference */
+		du->cursor_dmabuf = dmabuf;
+
+		ret = vmw_cursor_update_dmabuf(dev_priv, dmabuf, crtc_w, crtc_h,
+					       hotspot_x, hotspot_y);
+	} else {
+		vmw_cursor_update_position(dev_priv, false, 0, 0);
+		goto out;
+	}
+
+	if (!ret) {
+		du->cursor_x = crtc_x + du->set_gui_x;
+		du->cursor_y = crtc_y + du->set_gui_y;
+
+		vmw_cursor_update_position(dev_priv, true,
+					   du->cursor_x + hotspot_x,
+					   du->cursor_y + hotspot_y);
+	}
+
+out:
+	return ret;
+}
+
+
+int vmw_du_cursor_plane_disable(struct drm_plane *plane)
+{
+	if (plane->fb) {
+		drm_framebuffer_unreference(plane->fb);
+		plane->fb = NULL;
+	}
+
+	return -EINVAL;
+}
+
+
+void vmw_du_cursor_plane_destroy(struct drm_plane *plane)
+{
+	vmw_cursor_update_position(plane->dev->dev_private, false, 0, 0);
+
+	drm_plane_cleanup(plane);
+}
+
+
+void vmw_du_primary_plane_destroy(struct drm_plane *plane)
+{
+	drm_plane_cleanup(plane);
+
+	/* Planes are static in our case so we don't free it */
+}
+
+
 /*
  * Generic framebuffer code
  */
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.h b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.h
index cb36e1d..e400bfb 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.h
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.h
@@ -33,6 +33,8 @@ 
 #include <drm/drm_encoder.h>
 #include "vmwgfx_drv.h"
 
+
+
 /**
  * struct vmw_kms_dirty - closure structure for the vmw_kms_helper_dirty
  * function.
@@ -125,19 +127,17 @@  struct vmw_framebuffer_dmabuf {
 };
 
 
-/*
- * Basic cursor manipulation
- */
-int vmw_cursor_update_image(struct vmw_private *dev_priv,
-			    u32 *image, u32 width, u32 height,
-			    u32 hotspotX, u32 hotspotY);
-int vmw_cursor_update_dmabuf(struct vmw_private *dev_priv,
-			     struct vmw_dma_buffer *dmabuf,
-			     u32 width, u32 height,
-			     u32 hotspotX, u32 hotspotY);
-void vmw_cursor_update_position(struct vmw_private *dev_priv,
-				bool show, int x, int y);
+static const uint32_t vmw_primary_plane_formats[] = {
+	DRM_FORMAT_XRGB1555,
+	DRM_FORMAT_RGB565,
+	DRM_FORMAT_RGB888,
+	DRM_FORMAT_XRGB8888,
+	DRM_FORMAT_ARGB8888,
+};
 
+static const uint32_t vmw_cursor_plane_formats[] = {
+	DRM_FORMAT_ARGB8888,
+};
 
 /**
  * Base class display unit.
@@ -150,6 +150,8 @@  struct vmw_display_unit {
 	struct drm_crtc crtc;
 	struct drm_encoder encoder;
 	struct drm_connector connector;
+	struct drm_plane primary;
+	struct drm_plane cursor;
 
 	struct vmw_surface *cursor_surface;
 	struct vmw_dma_buffer *cursor_dmabuf;
@@ -270,6 +272,19 @@  void vmw_kms_update_implicit_fb(struct vmw_private *dev_priv,
 void vmw_kms_create_implicit_placement_property(struct vmw_private *dev_priv,
 						bool immutable);
 
+/* Universal Plane Helpers */
+void vmw_du_primary_plane_destroy(struct drm_plane *plane);
+void vmw_du_cursor_plane_destroy(struct drm_plane *plane);
+int vmw_du_cursor_plane_disable(struct drm_plane *plane);
+int vmw_du_cursor_plane_update(struct drm_plane *plane,
+			       struct drm_crtc *crtc,
+			       struct drm_framebuffer *fb,
+			       int crtc_x, int crtc_y,
+			       unsigned int crtc_w,
+			       unsigned int crtc_h,
+			       uint32_t src_x, uint32_t src_y,
+			       uint32_t src_w, uint32_t src_h);
+
 
 /*
  * Legacy display unit functions - vmwgfx_ldu.c
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_ldu.c b/drivers/gpu/drm/vmwgfx/vmwgfx_ldu.c
index 3806148..3efcbe5 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_ldu.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_ldu.c
@@ -75,10 +75,9 @@  static int vmw_ldu_commit_list(struct vmw_private *dev_priv)
 {
 	struct vmw_legacy_display *lds = dev_priv->ldu_priv;
 	struct vmw_legacy_display_unit *entry;
-	struct vmw_display_unit *du = NULL;
 	struct drm_framebuffer *fb = NULL;
 	struct drm_crtc *crtc = NULL;
-	int i = 0, ret;
+	int i = 0;
 
 	/* If there is no display topology the host just assumes
 	 * that the guest will set the same layout as the host.
@@ -132,25 +131,6 @@  static int vmw_ldu_commit_list(struct vmw_private *dev_priv)
 
 	lds->last_num_active = lds->num_active;
 
-
-	/* Find the first du with a cursor. */
-	list_for_each_entry(entry, &lds->active, active) {
-		du = &entry->base;
-
-		if (!du->cursor_dmabuf)
-			continue;
-
-		ret = vmw_cursor_update_dmabuf(dev_priv,
-					       du->cursor_dmabuf,
-					       64, 64,
-					       du->hotspot_x,
-					       du->hotspot_y);
-		if (ret == 0)
-			break;
-
-		DRM_ERROR("Could not update cursor image\n");
-	}
-
 	return 0;
 }
 
@@ -298,8 +278,6 @@  static int vmw_ldu_crtc_set_config(struct drm_mode_set *set)
 }
 
 static const struct drm_crtc_funcs vmw_legacy_crtc_funcs = {
-	.cursor_set2 = vmw_du_crtc_cursor_set2,
-	.cursor_move = vmw_du_crtc_cursor_move,
 	.gamma_set = vmw_du_crtc_gamma_set,
 	.destroy = vmw_ldu_crtc_destroy,
 	.set_config = vmw_ldu_crtc_set_config,
@@ -336,6 +314,23 @@  static const struct drm_connector_funcs vmw_legacy_connector_funcs = {
 	.destroy = vmw_ldu_connector_destroy,
 };
 
+/*
+ * Legacy Display Plane Functions
+ */
+
+static const struct drm_plane_funcs vmw_ldu_plane_funcs = {
+	.update_plane = drm_primary_helper_update,
+	.disable_plane = drm_primary_helper_disable,
+	.destroy = vmw_du_primary_plane_destroy,
+};
+
+static const struct drm_plane_funcs vmw_ldu_cursor_funcs = {
+	.update_plane = vmw_du_cursor_plane_update,
+	.disable_plane = vmw_du_cursor_plane_disable,
+	.destroy = vmw_du_cursor_plane_destroy,
+};
+
+
 static int vmw_ldu_init(struct vmw_private *dev_priv, unsigned unit)
 {
 	struct vmw_legacy_display_unit *ldu;
@@ -343,6 +338,7 @@  static int vmw_ldu_init(struct vmw_private *dev_priv, unsigned unit)
 	struct drm_connector *connector;
 	struct drm_encoder *encoder;
 	struct drm_crtc *crtc;
+	int ret;
 
 	ldu = kzalloc(sizeof(*ldu), GFP_KERNEL);
 	if (!ldu)
@@ -361,19 +357,61 @@  static int vmw_ldu_init(struct vmw_private *dev_priv, unsigned unit)
 	ldu->base.pref_mode = NULL;
 	ldu->base.is_implicit = true;
 
-	drm_connector_init(dev, connector, &vmw_legacy_connector_funcs,
-			   DRM_MODE_CONNECTOR_VIRTUAL);
+	/* Initialize primary plane */
+	ret = drm_universal_plane_init(dev, &ldu->base.primary,
+				       0, &vmw_ldu_plane_funcs,
+				       vmw_primary_plane_formats,
+				       ARRAY_SIZE(vmw_primary_plane_formats),
+				       DRM_PLANE_TYPE_PRIMARY, NULL);
+	if (ret) {
+		DRM_ERROR("Failed to initialize primary plane");
+		goto err_free;
+	}
+
+	/* Initialize cursor plane */
+	ret = drm_universal_plane_init(dev, &ldu->base.cursor,
+			0, &vmw_ldu_cursor_funcs,
+			vmw_cursor_plane_formats,
+			ARRAY_SIZE(vmw_cursor_plane_formats),
+			DRM_PLANE_TYPE_CURSOR, NULL);
+	if (ret) {
+		DRM_ERROR("Failed to initialize cursor plane");
+		drm_plane_cleanup(&ldu->base.primary);
+		goto err_free;
+	}
+
+	ret = drm_connector_init(dev, connector, &vmw_legacy_connector_funcs,
+				 DRM_MODE_CONNECTOR_VIRTUAL);
+	if (ret) {
+		DRM_ERROR("Failed to initialize connector\n");
+		goto err_free;
+	}
 	connector->status = vmw_du_connector_detect(connector, true);
 
-	drm_encoder_init(dev, encoder, &vmw_legacy_encoder_funcs,
-			 DRM_MODE_ENCODER_VIRTUAL, NULL);
-	drm_mode_connector_attach_encoder(connector, encoder);
+	ret = drm_encoder_init(dev, encoder, &vmw_legacy_encoder_funcs,
+			       DRM_MODE_ENCODER_VIRTUAL, NULL);
+	if (ret) {
+		DRM_ERROR("Failed to initialize encoder\n");
+		goto err_free_connector;
+	}
+
+	(void) drm_mode_connector_attach_encoder(connector, encoder);
 	encoder->possible_crtcs = (1 << unit);
 	encoder->possible_clones = 0;
 
-	(void) drm_connector_register(connector);
+	ret = drm_connector_register(connector);
+	if (ret) {
+		DRM_ERROR("Failed to register connector\n");
+		goto err_free_encoder;
+	}
 
-	drm_crtc_init(dev, crtc, &vmw_legacy_crtc_funcs);
+	ret = drm_crtc_init_with_planes(dev, crtc, &ldu->base.primary,
+					&ldu->base.cursor,
+					&vmw_legacy_crtc_funcs, NULL);
+	if (ret) {
+		DRM_ERROR("Failed to initialize CRTC\n");
+		goto err_free_unregister;
+	}
 
 	drm_mode_crtc_set_gamma_size(crtc, 256);
 
@@ -390,6 +428,16 @@  static int vmw_ldu_init(struct vmw_private *dev_priv, unsigned unit)
 			 1);
 
 	return 0;
+
+err_free_unregister:
+	drm_connector_unregister(connector);
+err_free_encoder:
+	drm_encoder_cleanup(encoder);
+err_free_connector:
+	drm_connector_cleanup(connector);
+err_free:
+	kfree(ldu);
+	return ret;
 }
 
 int vmw_kms_ldu_init_display(struct vmw_private *dev_priv)
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_scrn.c b/drivers/gpu/drm/vmwgfx/vmwgfx_scrn.c
index d4268ef..8ffccb8 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_scrn.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_scrn.c
@@ -459,8 +459,6 @@  static int vmw_sou_crtc_page_flip(struct drm_crtc *crtc,
 }
 
 static const struct drm_crtc_funcs vmw_screen_object_crtc_funcs = {
-	.cursor_set2 = vmw_du_crtc_cursor_set2,
-	.cursor_move = vmw_du_crtc_cursor_move,
 	.gamma_set = vmw_du_crtc_gamma_set,
 	.destroy = vmw_sou_crtc_destroy,
 	.set_config = vmw_sou_crtc_set_config,
@@ -497,6 +495,23 @@  static const struct drm_connector_funcs vmw_sou_connector_funcs = {
 	.destroy = vmw_sou_connector_destroy,
 };
 
+/*
+ * Screen Object Display Plane Functions
+ */
+
+static const struct drm_plane_funcs vmw_sou_plane_funcs = {
+	.update_plane = drm_primary_helper_update,
+	.disable_plane = drm_primary_helper_disable,
+	.destroy = vmw_du_primary_plane_destroy,
+};
+
+static const struct drm_plane_funcs vmw_sou_cursor_funcs = {
+	.update_plane = vmw_du_cursor_plane_update,
+	.disable_plane = vmw_du_cursor_plane_disable,
+	.destroy = vmw_du_cursor_plane_destroy,
+};
+
+
 static int vmw_sou_init(struct vmw_private *dev_priv, unsigned unit)
 {
 	struct vmw_screen_object_unit *sou;
@@ -504,6 +519,7 @@  static int vmw_sou_init(struct vmw_private *dev_priv, unsigned unit)
 	struct drm_connector *connector;
 	struct drm_encoder *encoder;
 	struct drm_crtc *crtc;
+	int ret;
 
 	sou = kzalloc(sizeof(*sou), GFP_KERNEL);
 	if (!sou)
@@ -521,19 +537,62 @@  static int vmw_sou_init(struct vmw_private *dev_priv, unsigned unit)
 	sou->base.pref_mode = NULL;
 	sou->base.is_implicit = false;
 
-	drm_connector_init(dev, connector, &vmw_sou_connector_funcs,
-			   DRM_MODE_CONNECTOR_VIRTUAL);
+	/* Initialize primary plane */
+	ret = drm_universal_plane_init(dev, &sou->base.primary,
+				       0, &vmw_sou_plane_funcs,
+				       vmw_primary_plane_formats,
+				       ARRAY_SIZE(vmw_primary_plane_formats),
+				       DRM_PLANE_TYPE_PRIMARY, NULL);
+	if (ret) {
+		DRM_ERROR("Failed to initialize primary plane");
+		goto err_free;
+	}
+
+	/* Initialize cursor plane */
+	ret = drm_universal_plane_init(dev, &sou->base.cursor,
+			0, &vmw_sou_cursor_funcs,
+			vmw_cursor_plane_formats,
+			ARRAY_SIZE(vmw_cursor_plane_formats),
+			DRM_PLANE_TYPE_CURSOR, NULL);
+	if (ret) {
+		DRM_ERROR("Failed to initialize cursor plane");
+		drm_plane_cleanup(&sou->base.primary);
+		goto err_free;
+	}
+
+	ret = drm_connector_init(dev, connector, &vmw_sou_connector_funcs,
+				 DRM_MODE_CONNECTOR_VIRTUAL);
+	if (ret) {
+		DRM_ERROR("Failed to initialize connector\n");
+		goto err_free;
+	}
+
 	connector->status = vmw_du_connector_detect(connector, true);
 
-	drm_encoder_init(dev, encoder, &vmw_screen_object_encoder_funcs,
-			 DRM_MODE_ENCODER_VIRTUAL, NULL);
-	drm_mode_connector_attach_encoder(connector, encoder);
+	ret = drm_encoder_init(dev, encoder, &vmw_screen_object_encoder_funcs,
+			       DRM_MODE_ENCODER_VIRTUAL, NULL);
+	if (ret) {
+		DRM_ERROR("Failed to initialize encoder\n");
+		goto err_free_connector;
+	}
+
+	(void) drm_mode_connector_attach_encoder(connector, encoder);
 	encoder->possible_crtcs = (1 << unit);
 	encoder->possible_clones = 0;
 
-	(void) drm_connector_register(connector);
+	ret = drm_connector_register(connector);
+	if (ret) {
+		DRM_ERROR("Failed to register connector\n");
+		goto err_free_encoder;
+	}
 
-	drm_crtc_init(dev, crtc, &vmw_screen_object_crtc_funcs);
+	ret = drm_crtc_init_with_planes(dev, crtc, &sou->base.primary,
+					&sou->base.cursor,
+					&vmw_screen_object_crtc_funcs, NULL);
+	if (ret) {
+		DRM_ERROR("Failed to initialize CRTC\n");
+		goto err_free_unregister;
+	}
 
 	drm_mode_crtc_set_gamma_size(crtc, 256);
 
@@ -550,6 +609,16 @@  static int vmw_sou_init(struct vmw_private *dev_priv, unsigned unit)
 			 sou->base.is_implicit);
 
 	return 0;
+
+err_free_unregister:
+	drm_connector_unregister(connector);
+err_free_encoder:
+	drm_encoder_cleanup(encoder);
+err_free_connector:
+	drm_connector_cleanup(connector);
+err_free:
+	kfree(sou);
+	return ret;
 }
 
 int vmw_kms_sou_init_display(struct vmw_private *dev_priv)
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c b/drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c
index b27cd18..4d9dd1b 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c
@@ -1015,8 +1015,6 @@  int vmw_kms_stdu_surface_dirty(struct vmw_private *dev_priv,
  *  Screen Target CRTC dispatch table
  */
 static const struct drm_crtc_funcs vmw_stdu_crtc_funcs = {
-	.cursor_set2 = vmw_du_crtc_cursor_set2,
-	.cursor_move = vmw_du_crtc_cursor_move,
 	.gamma_set = vmw_du_crtc_gamma_set,
 	.destroy = vmw_stdu_crtc_destroy,
 	.set_config = vmw_stdu_crtc_set_config,
@@ -1081,6 +1079,23 @@  static const struct drm_connector_funcs vmw_stdu_connector_funcs = {
 
 
 
+/******************************************************************************
+ * Screen Target Display Plane Functions
+ *****************************************************************************/
+
+static const struct drm_plane_funcs vmw_stdu_plane_funcs = {
+	.update_plane = drm_primary_helper_update,
+	.disable_plane = drm_primary_helper_disable,
+	.destroy = vmw_du_primary_plane_destroy,
+};
+
+static const struct drm_plane_funcs vmw_stdu_cursor_funcs = {
+	.update_plane = vmw_du_cursor_plane_update,
+	.disable_plane = vmw_du_cursor_plane_disable,
+	.destroy = vmw_du_cursor_plane_destroy,
+};
+
+
 /**
  * vmw_stdu_init - Sets up a Screen Target Display Unit
  *
@@ -1097,7 +1112,9 @@  static int vmw_stdu_init(struct vmw_private *dev_priv, unsigned unit)
 	struct drm_device *dev = dev_priv->dev;
 	struct drm_connector *connector;
 	struct drm_encoder *encoder;
+	struct drm_plane *primary, *cursor;
 	struct drm_crtc *crtc;
+	int    ret;
 
 
 	stdu = kzalloc(sizeof(*stdu), GFP_KERNEL);
@@ -1108,25 +1125,69 @@  static int vmw_stdu_init(struct vmw_private *dev_priv, unsigned unit)
 	crtc = &stdu->base.crtc;
 	encoder = &stdu->base.encoder;
 	connector = &stdu->base.connector;
+	primary = &stdu->base.primary;
+	cursor = &stdu->base.cursor;
 
 	stdu->base.pref_active = (unit == 0);
 	stdu->base.pref_width  = dev_priv->initial_width;
 	stdu->base.pref_height = dev_priv->initial_height;
 	stdu->base.is_implicit = false;
 
-	drm_connector_init(dev, connector, &vmw_stdu_connector_funcs,
-			   DRM_MODE_CONNECTOR_VIRTUAL);
+	/* Initialize primary plane */
+	ret = drm_universal_plane_init(dev, primary,
+				       0, &vmw_stdu_plane_funcs,
+				       vmw_primary_plane_formats,
+				       ARRAY_SIZE(vmw_primary_plane_formats),
+				       DRM_PLANE_TYPE_PRIMARY, NULL);
+	if (ret) {
+		DRM_ERROR("Failed to initialize primary plane");
+		goto err_free;
+	}
+
+	/* Initialize cursor plane */
+	ret = drm_universal_plane_init(dev, cursor,
+			0, &vmw_stdu_cursor_funcs,
+			vmw_cursor_plane_formats,
+			ARRAY_SIZE(vmw_cursor_plane_formats),
+			DRM_PLANE_TYPE_CURSOR, NULL);
+	if (ret) {
+		DRM_ERROR("Failed to initialize cursor plane");
+		drm_plane_cleanup(&stdu->base.primary);
+		goto err_free;
+	}
+
+	ret = drm_connector_init(dev, connector, &vmw_stdu_connector_funcs,
+				 DRM_MODE_CONNECTOR_VIRTUAL);
+	if (ret) {
+		DRM_ERROR("Failed to initialize connector\n");
+		goto err_free;
+	}
 	connector->status = vmw_du_connector_detect(connector, false);
 
-	drm_encoder_init(dev, encoder, &vmw_stdu_encoder_funcs,
-			 DRM_MODE_ENCODER_VIRTUAL, NULL);
-	drm_mode_connector_attach_encoder(connector, encoder);
+	ret = drm_encoder_init(dev, encoder, &vmw_stdu_encoder_funcs,
+			       DRM_MODE_ENCODER_VIRTUAL, NULL);
+	if (ret) {
+		DRM_ERROR("Failed to initialize encoder\n");
+		goto err_free_connector;
+	}
+
+	(void) drm_mode_connector_attach_encoder(connector, encoder);
 	encoder->possible_crtcs = (1 << unit);
 	encoder->possible_clones = 0;
 
-	(void) drm_connector_register(connector);
+	ret = drm_connector_register(connector);
+	if (ret) {
+		DRM_ERROR("Failed to register connector\n");
+		goto err_free_encoder;
+	}
 
-	drm_crtc_init(dev, crtc, &vmw_stdu_crtc_funcs);
+	ret = drm_crtc_init_with_planes(dev, crtc, &stdu->base.primary,
+					&stdu->base.cursor,
+					&vmw_stdu_crtc_funcs, NULL);
+	if (ret) {
+		DRM_ERROR("Failed to initialize CRTC\n");
+		goto err_free_unregister;
+	}
 
 	drm_mode_crtc_set_gamma_size(crtc, 256);
 
@@ -1142,6 +1203,16 @@  static int vmw_stdu_init(struct vmw_private *dev_priv, unsigned unit)
 			 dev_priv->implicit_placement_property,
 			 stdu->base.is_implicit);
 	return 0;
+
+err_free_unregister:
+	drm_connector_unregister(connector);
+err_free_encoder:
+	drm_encoder_cleanup(encoder);
+err_free_connector:
+	drm_connector_cleanup(connector);
+err_free:
+	kfree(stdu);
+	return ret;
 }