On Wed, Feb 24, 2016 at 09:37:31AM +0100, Maarten Lankhorst wrote:
> Instead of failing with -EINVAL when conflicting encoders are found,
> the legacy set_config will disable other connectors when encoders
> conflict.
>
> With the cleanup to update_output_state this is a lot easier to
> implement. set_config only adds connectors to the state that are
> modified, and because of the previous commit that calls
> add_affected_connectors only on set->crtc it means any connector not
> part of the modeset can be stolen from. We disable the connector in
> that case, and possibly the crtc if no connectors are left.
>
> Atomic modeset itself still doesn't allow encoder stealing, the
> results would be too unpredictable.
Shouldn't this be reworked to say something like "With this we can rework
the atomic core to not allow stealing"?
>
> Signed-off-by: Maarten Lankhorst <maarten.lankhorst at linux.intel.com>
> ---
> drivers/gpu/drm/drm_atomic_helper.c | 69
> +++++++++++++++++++++++++++++++++++++
> include/drm/drm_crtc.h | 2 ++
> 2 files changed, 71 insertions(+)
>
> diff --git a/drivers/gpu/drm/drm_atomic_helper.c
> b/drivers/gpu/drm/drm_atomic_helper.c
> index e89a5da27463..3543c7fcd072 100644
> --- a/drivers/gpu/drm/drm_atomic_helper.c
> +++ b/drivers/gpu/drm/drm_atomic_helper.c
> @@ -86,6 +86,68 @@ drm_atomic_helper_plane_changed(struct drm_atomic_state
> *state,
> }
> }
>
> +static int disable_conflicting_connectors(struct drm_atomic_state *state)
> +{
> + struct drm_connector_state *conn_state;
> + struct drm_connector *connector;
> + struct drm_encoder *encoder;
> + unsigned encoder_mask = 0;
> + int i, ret;
> +
> + for_each_connector_in_state(state, connector, conn_state, i) {
> + const struct drm_connector_helper_funcs *funcs =
> connector->helper_private;
> + struct drm_encoder *new_encoder;
> +
> + if (!conn_state->crtc)
> + continue;
> +
> + if (funcs->atomic_best_encoder)
> + new_encoder = funcs->atomic_best_encoder(connector,
> conn_state);
> + else
> + new_encoder = funcs->best_encoder(connector);
> +
> + if (new_encoder)
> + encoder_mask |= 1 << drm_encoder_index(new_encoder);
> + }
> +
> + drm_for_each_connector(connector, state->dev) {
> + struct drm_crtc_state *crtc_state;
> +
> + if (drm_atomic_get_existing_connector_state(state, connector))
> + continue;
> +
> + encoder = connector->state->best_encoder;
> + if (!encoder || !(encoder_mask & (1 <<
> drm_encoder_index(encoder))))
> + continue;
> +
> + conn_state = drm_atomic_get_connector_state(state, connector);
> + if (IS_ERR(conn_state))
> + return PTR_ERR(conn_state);
> +
> + DRM_DEBUG_ATOMIC("[ENCODER:%d:%s] in use on [CRTC:%d:%s],
> disabling [CONNECTOR:%d:%s]\n",
> + encoder->base.id, encoder->name,
> + conn_state->crtc->base.id,
> conn_state->crtc->name,
> + connector->base.id, connector->name);
> +
> + crtc_state = drm_atomic_get_existing_crtc_state(state,
> conn_state->crtc);
> +
> + ret = drm_atomic_set_crtc_for_connector(conn_state, NULL);
> + if (ret)
> + return ret;
> +
> + if (!crtc_state->connector_mask) {
> + ret = drm_atomic_set_mode_prop_for_crtc(crtc_state,
> + NULL);
> + if (ret < 0)
> + return ret;
> +
> + crtc_state->active = false;
> + }
> + }
> +
> + return 0;
> +}
> +
> static bool
> check_pending_encoder_assignment(struct drm_atomic_state *state,
> struct drm_encoder *new_encoder)
> @@ -449,6 +511,12 @@ drm_atomic_helper_check_modeset(struct drm_device *dev,
> }
> }
>
> + if (state->legacy_set_config) {
> + ret = disable_conflicting_connectors(state);
> + if (ret)
> + return ret;
> + }
> +
> for_each_connector_in_state(state, connector, connector_state, i) {
> /*
> * This only sets crtc->mode_changed for routing changes,
> @@ -1797,6 +1865,7 @@ int drm_atomic_helper_set_config(struct drm_mode_set
> *set)
> if (!state)
> return -ENOMEM;
>
> + state->legacy_set_config = true;
> state->acquire_ctx = drm_modeset_legacy_acquire_ctx(crtc);
> retry:
> ret = __drm_atomic_helper_set_config(set, state);
> diff --git a/include/drm/drm_crtc.h b/include/drm/drm_crtc.h
> index 7fad193dc645..9a946df27f07 100644
> --- a/include/drm/drm_crtc.h
> +++ b/include/drm/drm_crtc.h
> @@ -1677,6 +1677,7 @@ struct drm_bridge {
> * @dev: parent DRM device
> * @allow_modeset: allow full modeset
> * @legacy_cursor_update: hint to enforce legacy cursor IOCTL semantics
> + * @legacy_set_config: Disable conflicting encoders instead of failing with
> -EINVAL.
> * @planes: pointer to array of plane pointers
> * @plane_states: pointer to array of plane states pointers
> * @crtcs: pointer to array of CRTC pointers
> @@ -1690,6 +1691,7 @@ struct drm_atomic_state {
> struct drm_device *dev;
> bool allow_modeset : 1;
> bool legacy_cursor_update : 1;
> + bool legacy_set_config : 1;
> struct drm_plane **planes;
> struct drm_plane_state **plane_states;
> struct drm_crtc **crtcs;
> --
> 2.1.0
>
> _______________________________________________
> dri-devel mailing list
> dri-devel at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
