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@xxxxxxxxxx> Reviewed-by: Sinclair Yeh <syeh@xxxxxxxxxx> Reviewed-by: Deepak Rawat <drawat@xxxxxxxxxx> --- 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; } @@ -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); -- 2.14.4 _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel