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 thellstrom@vmware.com Reviewed-by: Sinclair Yeh syeh@vmware.com Reviewed-by: Deepak Rawat drawat@vmware.com
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;