[1/2] drm/vmwgfx: Fix up the implicit display unit handling

Commit Message

Thomas Hellstrom Oct. 4, 2018, 10:38 p.m. UTC

Make the connector is_implicit property immutable.
As far as we know, no user-space application is writing to it.

Also move the verification that all implicit display units scan out
from the same framebuffer to atomic_check().

Signed-off-by: Thomas Hellstrom <[e-mail]>
Reviewed-by: Sinclair Yeh <[e-mail]>
Reviewed-by: Deepak Rawat <[e-mail]>
---
 drivers/gpu/drm/vmwgfx/vmwgfx_drv.h  |   2 -
 drivers/gpu/drm/vmwgfx/vmwgfx_kms.c  | 230 ++++++++++++++---------------------
 drivers/gpu/drm/vmwgfx/vmwgfx_kms.h  |  16 +--
 drivers/gpu/drm/vmwgfx/vmwgfx_ldu.c  |   6 +-
 drivers/gpu/drm/vmwgfx/vmwgfx_scrn.c |  38 +-----
 drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c |  43 +------
 6 files changed, 102 insertions(+), 233 deletions(-)

Comments

Ville Syrjälä Oct. 5, 2018, 10:04 a.m. UTC | #1

On Thu, Oct 04, 2018 at 10:38:17PM +0000, Thomas Hellstrom wrote:
> Make the connector is_implicit property immutable.
> As far as we know, no user-space application is writing to it.
> 
> Also move the verification that all implicit display units scan out
> from the same framebuffer to atomic_check().
> 
> Signed-off-by: Thomas Hellstrom <[e-mail]>
> Reviewed-by: Sinclair Yeh <[e-mail]>
> Reviewed-by: Deepak Rawat <[e-mail]>
> ---
>  drivers/gpu/drm/vmwgfx/vmwgfx_drv.h  |   2 -
>  drivers/gpu/drm/vmwgfx/vmwgfx_kms.c  | 230 ++++++++++++++---------------------
>  drivers/gpu/drm/vmwgfx/vmwgfx_kms.h  |  16 +--
>  drivers/gpu/drm/vmwgfx/vmwgfx_ldu.c  |   6 +-
>  drivers/gpu/drm/vmwgfx/vmwgfx_scrn.c |  38 +-----
>  drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c |  43 +------
>  6 files changed, 102 insertions(+), 233 deletions(-)
> 
> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h
> index 59f614225bcd..ea2c22d92357 100644
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h
> @@ -486,8 +486,6 @@ struct vmw_private {
>  	struct vmw_overlay *overlay_priv;
>  	struct drm_property *hotplug_mode_update_property;
>  	struct drm_property *implicit_placement_property;
> -	unsigned num_implicit;
> -	struct vmw_framebuffer *implicit_fb;
>  	struct mutex global_kms_state_mutex;
>  	spinlock_t cursor_lock;
>  	struct drm_atomic_state *suspend_state;
> diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> index 05fb16733c5c..ccaec2cbabd2 100644
> --- a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
> @@ -456,21 +456,8 @@ int vmw_du_primary_plane_atomic_check(struct drm_plane *plane,
>  		struct drm_crtc *crtc = state->crtc;
>  		struct vmw_connector_state *vcs;
>  		struct vmw_display_unit *du = vmw_crtc_to_du(crtc);
> -		struct vmw_private *dev_priv = vmw_priv(crtc->dev);
> -		struct vmw_framebuffer *vfb = vmw_framebuffer_to_vfb(new_fb);
>  
>  		vcs = vmw_connector_state_to_vcs(du->connector.state);
> -
> -		/* Only one active implicit framebuffer at a time. */
> -		mutex_lock(&dev_priv->global_kms_state_mutex);
> -		if (vcs->is_implicit && dev_priv->implicit_fb &&
> -		    !(dev_priv->num_implicit == 1 && du->active_implicit)
> -		    && dev_priv->implicit_fb != vfb) {
> -			DRM_ERROR("Multiple implicit framebuffers "
> -				  "not supported.\n");
> -			ret = -EINVAL;
> -		}
> -		mutex_unlock(&dev_priv->global_kms_state_mutex);
>  	}
>  
>  
> @@ -1566,6 +1553,88 @@ static int vmw_kms_check_display_memory(struct drm_device *dev,
>  	return 0;
>  }
>  
> +/**
> + * vmw_crtc_state_and_lock - Return new or current crtc state with locked
> + * crtc mutex
> + * @state: The atomic state pointer containing the new atomic state
> + * @crtc: The crtc
> + *
> + * This function returns the new crtc state if it's part of the state update.
> + * Otherwise returns the current crtc state. It also makes sure that the
> + * crtc mutex is locked.
> + *
> + * Returns: A valid crtc state pointer or NULL. It may also return a
> + * pointer error, in particular -EDEADLK if locking needs to be rerun.
> + */
> +static struct drm_crtc_state *
> +vmw_crtc_state_and_lock(struct drm_atomic_state *state, struct drm_crtc *crtc)
> +{
> +	struct drm_crtc_state *crtc_state;
> +
> +	crtc_state = drm_atomic_get_new_crtc_state(state, crtc);
> +	if (crtc_state) {
> +		lockdep_assert_held(&crtc->mutex.mutex.base);
> +	} else {
> +		int ret = drm_modeset_lock(&crtc->mutex, state->acquire_ctx);
> +
> +		if (ret != 0 && ret != -EALREADY)
> +			return ERR_PTR(ret);
> +
> +		crtc_state = crtc->state;
> +	}
> +
> +	return crtc_state;
> +}
> +
> +/**
> + * vmw_kms_check_implicit - Verify that all implicit display units scan out
> + * from the same fb after the new state is committed.
> + * @dev: The drm_device.
> + * @state: The new state to be checked.
> + *
> + * Returns:
> + *   Zero on success,
> + *   -EINVAL on invalid state,
> + *   -EDEADLK if modeset locking needs to be rerun.
> + */
> +static int vmw_kms_check_implicit(struct drm_device *dev,
> +				  struct drm_atomic_state *state)
> +{
> +	struct drm_framebuffer *implicit_fb = NULL;
> +	struct drm_crtc *crtc;
> +	struct drm_crtc_state *crtc_state;
> +	struct drm_plane_state *plane_state;
> +
> +	drm_for_each_crtc(crtc, dev) {
> +		struct vmw_display_unit *du = vmw_crtc_to_du(crtc);
> +
> +		if (!du->is_implicit)
> +			continue;
> +
> +		crtc_state = vmw_crtc_state_and_lock(state, crtc);
> +		if (IS_ERR(crtc_state))
> +			return PTR_ERR(crtc_state);
> +
> +		if (!crtc_state || !crtc_state->enable)
> +			continue;
> +
> +		/*
> +		 * Can't move primary planes across crtcs, so this is OK.
> +		 * It also means we don't need to take the plane mutex.
> +		 */
> +		plane_state = du->primary.state;
> +		if (plane_state->crtc != crtc)
> +			continue;
> +
> +		if (!implicit_fb)
> +			implicit_fb = plane_state->fb;
> +		else if (implicit_fb != plane_state->fb)
> +			return -EINVAL;
> +	}
> +
> +	return 0;
> +}
> +
>  /**
>   * vmw_kms_check_topology - Validates topology in drm_atomic_state
>   * @dev: DRM device
> @@ -1683,6 +1752,10 @@ vmw_kms_atomic_check_modeset(struct drm_device *dev,
>  	if (ret)
>  		return ret;
>  
> +	ret = vmw_kms_check_implicit(dev, state);
> +	if (ret)
> +		return ret;
> +
>  	if (!state->allow_modeset)
>  		return ret;
>  
> @@ -2277,13 +2350,7 @@ int vmw_du_connector_set_property(struct drm_connector *connector,
>  				  struct drm_property *property,
>  				  uint64_t val)
>  {
> -	struct vmw_display_unit *du = vmw_connector_to_du(connector);
> -	struct vmw_private *dev_priv = vmw_priv(connector->dev);
> -
> -	if (property == dev_priv->implicit_placement_property)
> -		du->is_implicit = val;
> -
> -	return 0;
> +	return -EINVAL;
>  }
>  
>  
> @@ -2302,25 +2369,7 @@ vmw_du_connector_atomic_set_property(struct drm_connector *connector,
>  				     struct drm_property *property,
>  				     uint64_t val)
>  {
> -	struct vmw_private *dev_priv = vmw_priv(connector->dev);
> -	struct vmw_connector_state *vcs = vmw_connector_state_to_vcs(state);
> -	struct vmw_display_unit *du = vmw_connector_to_du(connector);
> -
> -
> -	if (property == dev_priv->implicit_placement_property) {
> -		vcs->is_implicit = val;
> -
> -		/*
> -		 * We should really be doing a drm_atomic_commit() to
> -		 * commit the new state, but since this doesn't cause
> -		 * an immedate state change, this is probably ok
> -		 */
> -		du->is_implicit = vcs->is_implicit;
> -	} else {
> -		return -EINVAL;
> -	}
> -
> -	return 0;
> +	return -EINVAL;
>  }

No need to keep the empty functions.

>  
>  
> @@ -2339,10 +2388,9 @@ vmw_du_connector_atomic_get_property(struct drm_connector *connector,
>  				     uint64_t *val)
>  {
>  	struct vmw_private *dev_priv = vmw_priv(connector->dev);
> -	struct vmw_connector_state *vcs = vmw_connector_state_to_vcs(state);
>  
>  	if (property == dev_priv->implicit_placement_property)
> -		*val = vcs->is_implicit;
> +		*val = vmw_connector_to_du(connector)->is_implicit;

This codepath will never be take with immutable props. So you can nuke
this and you must do the appropriate drm_property_set_value() somewhere
to make the prop value match .is_implicit.

>  	else {
>  		DRM_ERROR("Invalid Property %s\n", property->name);
>  		return -EINVAL;

Thomas Hellstrom Oct. 5, 2018, 6:32 p.m. UTC | #2

Thanks for reviewing, Ville.

I'll fix those up.

/Thomas


On 10/05/2018 12:04 PM, Ville Syrjälä wrote:
...

Patch

diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h
index 59f614225bcd..ea2c22d92357 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_drv.h
@@ -486,8 +486,6 @@  struct vmw_private {
 	struct vmw_overlay *overlay_priv;
 	struct drm_property *hotplug_mode_update_property;
 	struct drm_property *implicit_placement_property;
-	unsigned num_implicit;
-	struct vmw_framebuffer *implicit_fb;
 	struct mutex global_kms_state_mutex;
 	spinlock_t cursor_lock;
 	struct drm_atomic_state *suspend_state;
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
index 05fb16733c5c..ccaec2cbabd2 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.c
@@ -456,21 +456,8 @@  int vmw_du_primary_plane_atomic_check(struct drm_plane *plane,
 		struct drm_crtc *crtc = state->crtc;
 		struct vmw_connector_state *vcs;
 		struct vmw_display_unit *du = vmw_crtc_to_du(crtc);
-		struct vmw_private *dev_priv = vmw_priv(crtc->dev);
-		struct vmw_framebuffer *vfb = vmw_framebuffer_to_vfb(new_fb);
 
 		vcs = vmw_connector_state_to_vcs(du->connector.state);
-
-		/* Only one active implicit framebuffer at a time. */
-		mutex_lock(&dev_priv->global_kms_state_mutex);
-		if (vcs->is_implicit && dev_priv->implicit_fb &&
-		    !(dev_priv->num_implicit == 1 && du->active_implicit)
-		    && dev_priv->implicit_fb != vfb) {
-			DRM_ERROR("Multiple implicit framebuffers "
-				  "not supported.\n");
-			ret = -EINVAL;
-		}
-		mutex_unlock(&dev_priv->global_kms_state_mutex);
 	}
 
 
@@ -1566,6 +1553,88 @@  static int vmw_kms_check_display_memory(struct drm_device *dev,
 	return 0;
 }
 
+/**
+ * vmw_crtc_state_and_lock - Return new or current crtc state with locked
+ * crtc mutex
+ * @state: The atomic state pointer containing the new atomic state
+ * @crtc: The crtc
+ *
+ * This function returns the new crtc state if it's part of the state update.
+ * Otherwise returns the current crtc state. It also makes sure that the
+ * crtc mutex is locked.
+ *
+ * Returns: A valid crtc state pointer or NULL. It may also return a
+ * pointer error, in particular -EDEADLK if locking needs to be rerun.
+ */
+static struct drm_crtc_state *
+vmw_crtc_state_and_lock(struct drm_atomic_state *state, struct drm_crtc *crtc)
+{
+	struct drm_crtc_state *crtc_state;
+
+	crtc_state = drm_atomic_get_new_crtc_state(state, crtc);
+	if (crtc_state) {
+		lockdep_assert_held(&crtc->mutex.mutex.base);
+	} else {
+		int ret = drm_modeset_lock(&crtc->mutex, state->acquire_ctx);
+
+		if (ret != 0 && ret != -EALREADY)
+			return ERR_PTR(ret);
+
+		crtc_state = crtc->state;
+	}
+
+	return crtc_state;
+}
+
+/**
+ * vmw_kms_check_implicit - Verify that all implicit display units scan out
+ * from the same fb after the new state is committed.
+ * @dev: The drm_device.
+ * @state: The new state to be checked.
+ *
+ * Returns:
+ *   Zero on success,
+ *   -EINVAL on invalid state,
+ *   -EDEADLK if modeset locking needs to be rerun.
+ */
+static int vmw_kms_check_implicit(struct drm_device *dev,
+				  struct drm_atomic_state *state)
+{
+	struct drm_framebuffer *implicit_fb = NULL;
+	struct drm_crtc *crtc;
+	struct drm_crtc_state *crtc_state;
+	struct drm_plane_state *plane_state;
+
+	drm_for_each_crtc(crtc, dev) {
+		struct vmw_display_unit *du = vmw_crtc_to_du(crtc);
+
+		if (!du->is_implicit)
+			continue;
+
+		crtc_state = vmw_crtc_state_and_lock(state, crtc);
+		if (IS_ERR(crtc_state))
+			return PTR_ERR(crtc_state);
+
+		if (!crtc_state || !crtc_state->enable)
+			continue;
+
+		/*
+		 * Can't move primary planes across crtcs, so this is OK.
+		 * It also means we don't need to take the plane mutex.
+		 */
+		plane_state = du->primary.state;
+		if (plane_state->crtc != crtc)
+			continue;
+
+		if (!implicit_fb)
+			implicit_fb = plane_state->fb;
+		else if (implicit_fb != plane_state->fb)
+			return -EINVAL;
+	}
+
+	return 0;
+}
+
 /**
  * vmw_kms_check_topology - Validates topology in drm_atomic_state
  * @dev: DRM device
@@ -1683,6 +1752,10 @@  vmw_kms_atomic_check_modeset(struct drm_device *dev,
 	if (ret)
 		return ret;
 
+	ret = vmw_kms_check_implicit(dev, state);
+	if (ret)
+		return ret;
+
 	if (!state->allow_modeset)
 		return ret;
 
@@ -2277,13 +2350,7 @@  int vmw_du_connector_set_property(struct drm_connector *connector,
 				  struct drm_property *property,
 				  uint64_t val)
 {
-	struct vmw_display_unit *du = vmw_connector_to_du(connector);
-	struct vmw_private *dev_priv = vmw_priv(connector->dev);
-
-	if (property == dev_priv->implicit_placement_property)
-		du->is_implicit = val;
-
-	return 0;
+	return -EINVAL;
 }
 
 
@@ -2302,25 +2369,7 @@  vmw_du_connector_atomic_set_property(struct drm_connector *connector,
 				     struct drm_property *property,
 				     uint64_t val)
 {
-	struct vmw_private *dev_priv = vmw_priv(connector->dev);
-	struct vmw_connector_state *vcs = vmw_connector_state_to_vcs(state);
-	struct vmw_display_unit *du = vmw_connector_to_du(connector);
-
-
-	if (property == dev_priv->implicit_placement_property) {
-		vcs->is_implicit = val;
-
-		/*
-		 * We should really be doing a drm_atomic_commit() to
-		 * commit the new state, but since this doesn't cause
-		 * an immedate state change, this is probably ok
-		 */
-		du->is_implicit = vcs->is_implicit;
-	} else {
-		return -EINVAL;
-	}
-
-	return 0;
+	return -EINVAL;
 }
 
 
@@ -2339,10 +2388,9 @@  vmw_du_connector_atomic_get_property(struct drm_connector *connector,
 				     uint64_t *val)
 {
 	struct vmw_private *dev_priv = vmw_priv(connector->dev);
-	struct vmw_connector_state *vcs = vmw_connector_state_to_vcs(state);
 
 	if (property == dev_priv->implicit_placement_property)
-		*val = vcs->is_implicit;
+		*val = vmw_connector_to_du(connector)->is_implicit;
 	else {
 		DRM_ERROR("Invalid Property %s\n", property->name);
 		return -EINVAL;
@@ -2723,120 +2771,24 @@  int vmw_kms_fbdev_init_data(struct vmw_private *dev_priv,
 	return ret;
 }
 
-/**
- * vmw_kms_del_active - unregister a crtc binding to the implicit framebuffer
- *
- * @dev_priv: Pointer to a device private struct.
- * @du: The display unit of the crtc.
- */
-void vmw_kms_del_active(struct vmw_private *dev_priv,
-			struct vmw_display_unit *du)
-{
-	mutex_lock(&dev_priv->global_kms_state_mutex);
-	if (du->active_implicit) {
-		if (--(dev_priv->num_implicit) == 0)
-			dev_priv->implicit_fb = NULL;
-		du->active_implicit = false;
-	}
-	mutex_unlock(&dev_priv->global_kms_state_mutex);
-}
-
-/**
- * vmw_kms_add_active - register a crtc binding to an implicit framebuffer
- *
- * @vmw_priv: Pointer to a device private struct.
- * @du: The display unit of the crtc.
- * @vfb: The implicit framebuffer
- *
- * Registers a binding to an implicit framebuffer.
- */
-void vmw_kms_add_active(struct vmw_private *dev_priv,
-			struct vmw_display_unit *du,
-			struct vmw_framebuffer *vfb)
-{
-	mutex_lock(&dev_priv->global_kms_state_mutex);
-	WARN_ON_ONCE(!dev_priv->num_implicit && dev_priv->implicit_fb);
-
-	if (!du->active_implicit && du->is_implicit) {
-		dev_priv->implicit_fb = vfb;
-		du->active_implicit = true;
-		dev_priv->num_implicit++;
-	}
-	mutex_unlock(&dev_priv->global_kms_state_mutex);
-}
-
-/**
- * vmw_kms_screen_object_flippable - Check whether we can page-flip a crtc.
- *
- * @dev_priv: Pointer to device-private struct.
- * @crtc: The crtc we want to flip.
- *
- * Returns true or false depending whether it's OK to flip this crtc
- * based on the criterion that we must not have more than one implicit
- * frame-buffer at any one time.
- */
-bool vmw_kms_crtc_flippable(struct vmw_private *dev_priv,
-			    struct drm_crtc *crtc)
-{
-	struct vmw_display_unit *du = vmw_crtc_to_du(crtc);
-	bool ret;
-
-	mutex_lock(&dev_priv->global_kms_state_mutex);
-	ret = !du->is_implicit || dev_priv->num_implicit == 1;
-	mutex_unlock(&dev_priv->global_kms_state_mutex);
-
-	return ret;
-}
-
-/**
- * vmw_kms_update_implicit_fb - Update the implicit fb.
- *
- * @dev_priv: Pointer to device-private struct.
- * @crtc: The crtc the new implicit frame-buffer is bound to.
- */
-void vmw_kms_update_implicit_fb(struct vmw_private *dev_priv,
-				struct drm_crtc *crtc)
-{
-	struct vmw_display_unit *du = vmw_crtc_to_du(crtc);
-	struct drm_plane *plane = crtc->primary;
-	struct vmw_framebuffer *vfb;
-
-	mutex_lock(&dev_priv->global_kms_state_mutex);
-
-	if (!du->is_implicit)
-		goto out_unlock;
-
-	vfb = vmw_framebuffer_to_vfb(plane->state->fb);
-	WARN_ON_ONCE(dev_priv->num_implicit != 1 &&
-		     dev_priv->implicit_fb != vfb);
-
-	dev_priv->implicit_fb = vfb;
-out_unlock:
-	mutex_unlock(&dev_priv->global_kms_state_mutex);
-}
-
 /**
  * vmw_kms_create_implicit_placement_proparty - Set up the implicit placement
  * property.
  *
  * @dev_priv: Pointer to a device private struct.
- * @immutable: Whether the property is immutable.
  *
  * Sets up the implicit placement property unless it's already set up.
  */
 void
-vmw_kms_create_implicit_placement_property(struct vmw_private *dev_priv,
-					   bool immutable)
+vmw_kms_create_implicit_placement_property(struct vmw_private *dev_priv)
 {
 	if (dev_priv->implicit_placement_property)
 		return;
 
 	dev_priv->implicit_placement_property =
 		drm_property_create_range(dev_priv->dev,
-					  immutable ?
-					  DRM_MODE_PROP_IMMUTABLE : 0,
+					  DRM_MODE_PROP_IMMUTABLE,
 					  "implicit_placement", 0, 1);
-
 }
 
 
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.h b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.h
index 76ec570c0684..979056330266 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_kms.h
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_kms.h
@@ -191,8 +191,6 @@  struct vmw_plane_state {
 struct vmw_connector_state {
 	struct drm_connector_state base;
 
-	bool is_implicit;
-
 	/**
 	 * @gui_x:
 	 *
@@ -254,7 +252,6 @@  struct vmw_display_unit {
 	int gui_x;
 	int gui_y;
 	bool is_implicit;
-	bool active_implicit;
 	int set_gui_x;
 	int set_gui_y;
 };
@@ -334,17 +331,8 @@  int vmw_kms_fbdev_init_data(struct vmw_private *dev_priv,
 			    struct drm_crtc **p_crtc,
 			    struct drm_display_mode **p_mode);
 void vmw_guess_mode_timing(struct drm_display_mode *mode);
-void vmw_kms_del_active(struct vmw_private *dev_priv,
-			struct vmw_display_unit *du);
-void vmw_kms_add_active(struct vmw_private *dev_priv,
-			struct vmw_display_unit *du,
-			struct vmw_framebuffer *vfb);
-bool vmw_kms_crtc_flippable(struct vmw_private *dev_priv,
-			    struct drm_crtc *crtc);
-void vmw_kms_update_implicit_fb(struct vmw_private *dev_priv,
-				struct drm_crtc *crtc);
-void vmw_kms_create_implicit_placement_property(struct vmw_private *dev_priv,
-						bool immutable);
+void vmw_kms_update_implicit_fb(struct vmw_private *dev_priv);
+void vmw_kms_create_implicit_placement_property(struct vmw_private *dev_priv);
 
 /* Universal Plane Helpers */
 void vmw_du_primary_plane_destroy(struct drm_plane *plane);
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_ldu.c b/drivers/gpu/drm/vmwgfx/vmwgfx_ldu.c
index 723578117191..558bc8cf1510 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_ldu.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_ldu.c
@@ -417,7 +417,6 @@  static int vmw_ldu_init(struct vmw_private *dev_priv, unsigned unit)
 
 	drm_plane_helper_add(cursor, &vmw_ldu_cursor_plane_helper_funcs);
 
-
 	vmw_du_connector_reset(connector);
 	ret = drm_connector_init(dev, connector, &vmw_legacy_connector_funcs,
 				 DRM_MODE_CONNECTOR_VIRTUAL);
@@ -428,8 +427,6 @@  static int vmw_ldu_init(struct vmw_private *dev_priv, unsigned unit)
 
 	drm_connector_helper_add(connector, &vmw_ldu_connector_helper_funcs);
 	connector->status = vmw_du_connector_detect(connector, true);
-	vmw_connector_state_to_vcs(connector->state)->is_implicit = true;
-
 
 	ret = drm_encoder_init(dev, encoder, &vmw_legacy_encoder_funcs,
 			       DRM_MODE_ENCODER_VIRTUAL, NULL);
@@ -448,7 +445,6 @@  static int vmw_ldu_init(struct vmw_private *dev_priv, unsigned unit)
 		goto err_free_encoder;
 	}
 
-
 	vmw_du_crtc_reset(crtc);
 	ret = drm_crtc_init_with_planes(dev, crtc, &ldu->base.primary,
 					&ldu->base.cursor,
@@ -514,7 +510,7 @@  int vmw_kms_ldu_init_display(struct vmw_private *dev_priv)
 	if (ret != 0)
 		goto err_free;
 
-	vmw_kms_create_implicit_placement_property(dev_priv, true);
+	vmw_kms_create_implicit_placement_property(dev_priv);
 
 	if (dev_priv->capabilities & SVGA_CAP_MULTIMON)
 		for (i = 0; i < VMWGFX_NUM_DISPLAY_UNITS; ++i)
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_scrn.c b/drivers/gpu/drm/vmwgfx/vmwgfx_scrn.c
index 53316b1bda3d..dcab2ecaeecb 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_scrn.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_scrn.c
@@ -241,28 +241,20 @@  static void vmw_sou_crtc_mode_set_nofb(struct drm_crtc *crtc)
 		sou->buffer = vps->bo;
 		sou->buffer_size = vps->bo_size;
 
-		if (sou->base.is_implicit) {
-			x = crtc->x;
-			y = crtc->y;
-		} else {
-			conn_state = sou->base.connector.state;
-			vmw_conn_state = vmw_connector_state_to_vcs(conn_state);
-
-			x = vmw_conn_state->gui_x;
-			y = vmw_conn_state->gui_y;
-		}
+		conn_state = sou->base.connector.state;
+		vmw_conn_state = vmw_connector_state_to_vcs(conn_state);
+
+		x = vmw_conn_state->gui_x;
+		y = vmw_conn_state->gui_y;
 
 		ret = vmw_sou_fifo_create(dev_priv, sou, x, y, &crtc->mode);
 		if (ret)
 			DRM_ERROR("Failed to define Screen Object %dx%d\n",
 				  crtc->x, crtc->y);
 
-		vmw_kms_add_active(dev_priv, &sou->base, vfb);
 	} else {
 		sou->buffer = NULL;
 		sou->buffer_size = 0;
-
-		vmw_kms_del_active(dev_priv, &sou->base);
 	}
 }
 
@@ -323,21 +315,14 @@  static int vmw_sou_crtc_page_flip(struct drm_crtc *crtc,
 				  uint32_t flags,
 				  struct drm_modeset_acquire_ctx *ctx)
 {
-	struct vmw_private *dev_priv = vmw_priv(crtc->dev);
 	int ret;
 
-	if (!vmw_kms_crtc_flippable(dev_priv, crtc))
-		return -EINVAL;
-
 	ret = drm_atomic_helper_page_flip(crtc, new_fb, event, flags, ctx);
 	if (ret) {
 		DRM_ERROR("Page flip error %d.\n", ret);
 		return ret;
 	}
 
-	if (vmw_crtc_to_du(crtc)->is_implicit)
-		vmw_kms_update_implicit_fb(dev_priv, crtc);
-
 	return ret;
 }
 
@@ -640,7 +625,6 @@  static int vmw_sou_init(struct vmw_private *dev_priv, unsigned unit)
 	primary = &sou->base.primary;
 	cursor = &sou->base.cursor;
 
-	sou->base.active_implicit = false;
 	sou->base.pref_active = (unit == 0);
 	sou->base.pref_width = dev_priv->initial_width;
 	sou->base.pref_height = dev_priv->initial_height;
@@ -693,8 +677,6 @@  static int vmw_sou_init(struct vmw_private *dev_priv, unsigned unit)
 
 	drm_connector_helper_add(connector, &vmw_sou_connector_helper_funcs);
 	connector->status = vmw_du_connector_detect(connector, true);
-	vmw_connector_state_to_vcs(connector->state)->is_implicit = false;
-
 
 	ret = drm_encoder_init(dev, encoder, &vmw_screen_object_encoder_funcs,
 			       DRM_MODE_ENCODER_VIRTUAL, NULL);
@@ -733,12 +715,6 @@  static int vmw_sou_init(struct vmw_private *dev_priv, unsigned unit)
 				   dev->mode_config.suggested_x_property, 0);
 	drm_object_attach_property(&connector->base,
 				   dev->mode_config.suggested_y_property, 0);
-	if (dev_priv->implicit_placement_property)
-		drm_object_attach_property
-			(&connector->base,
-			 dev_priv->implicit_placement_property,
-			 sou->base.is_implicit);
-
 	return 0;
 
 err_free_unregister:
@@ -764,15 +740,11 @@  int vmw_kms_sou_init_display(struct vmw_private *dev_priv)
 	}
 
 	ret = -ENOMEM;
-	dev_priv->num_implicit = 0;
-	dev_priv->implicit_fb = NULL;
 
 	ret = drm_vblank_init(dev, VMWGFX_NUM_DISPLAY_UNITS);
 	if (unlikely(ret != 0))
 		return ret;
 
-	vmw_kms_create_implicit_placement_property(dev_priv, false);
-
 	for (i = 0; i < VMWGFX_NUM_DISPLAY_UNITS; ++i)
 		vmw_sou_init(dev_priv, i);
 
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c b/drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c
index d3a9eba12b0e..e0a7275254e5 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_stdu.c
@@ -396,13 +396,8 @@  static void vmw_stdu_crtc_mode_set_nofb(struct drm_crtc *crtc)
 	if (!crtc->state->enable)
 		return;
 
-	if (stdu->base.is_implicit) {
-		x = crtc->x;
-		y = crtc->y;
-	} else {
-		x = vmw_conn_state->gui_x;
-		y = vmw_conn_state->gui_y;
-	}
+	x = vmw_conn_state->gui_x;
+	y = vmw_conn_state->gui_y;
 
 	vmw_svga_enable(dev_priv);
 	ret = vmw_stdu_define_st(dev_priv, stdu, &crtc->mode, x, y);
@@ -417,27 +412,9 @@  static void vmw_stdu_crtc_helper_prepare(struct drm_crtc *crtc)
 {
 }
 
-
 static void vmw_stdu_crtc_atomic_enable(struct drm_crtc *crtc,
 					struct drm_crtc_state *old_state)
 {
-	struct drm_plane_state *plane_state = crtc->primary->state;
-	struct vmw_private *dev_priv;
-	struct vmw_screen_target_display_unit *stdu;
-	struct vmw_framebuffer *vfb;
-	struct drm_framebuffer *fb;
-
-
-	stdu     = vmw_crtc_to_stdu(crtc);
-	dev_priv = vmw_priv(crtc->dev);
-	fb       = plane_state->fb;
-
-	vfb = (fb) ? vmw_framebuffer_to_vfb(fb) : NULL;
-
-	if (vfb)
-		vmw_kms_add_active(dev_priv, &stdu->base, vfb);
-	else
-		vmw_kms_del_active(dev_priv, &stdu->base);
 }
 
 static void vmw_stdu_crtc_atomic_disable(struct drm_crtc *crtc,
@@ -497,11 +474,10 @@  static int vmw_stdu_crtc_page_flip(struct drm_crtc *crtc,
 				   struct drm_modeset_acquire_ctx *ctx)
 
 {
-	struct vmw_private *dev_priv = vmw_priv(crtc->dev);
 	struct vmw_screen_target_display_unit *stdu = vmw_crtc_to_stdu(crtc);
 	int ret;
 
-	if (!stdu->defined || !vmw_kms_crtc_flippable(dev_priv, crtc))
+	if (!stdu->defined)
 		return -EINVAL;
 
 	ret = drm_atomic_helper_page_flip(crtc, new_fb, event, flags, ctx);
@@ -1457,11 +1433,6 @@  static int vmw_stdu_init(struct vmw_private *dev_priv, unsigned unit)
 	stdu->base.pref_active = (unit == 0);
 	stdu->base.pref_width  = dev_priv->initial_width;
 	stdu->base.pref_height = dev_priv->initial_height;
-
-	/*
-	 * Remove this after enabling atomic because property values can
-	 * only exist in a state object
-	 */
 	stdu->base.is_implicit = false;
 
 	/* Initialize primary plane */
@@ -1506,7 +1477,6 @@  static int vmw_stdu_init(struct vmw_private *dev_priv, unsigned unit)
 
 	drm_connector_helper_add(connector, &vmw_stdu_connector_helper_funcs);
 	connector->status = vmw_du_connector_detect(connector, false);
-	vmw_connector_state_to_vcs(connector->state)->is_implicit = false;
 
 	ret = drm_encoder_init(dev, encoder, &vmw_stdu_encoder_funcs,
 			       DRM_MODE_ENCODER_VIRTUAL, NULL);
@@ -1544,11 +1514,6 @@  static int vmw_stdu_init(struct vmw_private *dev_priv, unsigned unit)
 				   dev->mode_config.suggested_x_property, 0);
 	drm_object_attach_property(&connector->base,
 				   dev->mode_config.suggested_y_property, 0);
-	if (dev_priv->implicit_placement_property)
-		drm_object_attach_property
-			(&connector->base,
-			 dev_priv->implicit_placement_property,
-			 stdu->base.is_implicit);
 	return 0;
 
 err_free_unregister:
@@ -1642,8 +1607,6 @@  int vmw_kms_stdu_init_display(struct vmw_private *dev_priv)
 		dev->mode_config.max_height = 8192;
 	}
 
-	vmw_kms_create_implicit_placement_property(dev_priv, false);
-
 	for (i = 0; i < VMWGFX_NUM_DISPLAY_UNITS; ++i) {
 		ret = vmw_stdu_init(dev_priv, i);