On Thu, Feb 18, 2016 at 09:54:42AM +0100, Maarten Lankhorst wrote:
> There was a potential to crash in the following case:
> [ 49.985270] [drm:update_connector_routing] Updating routing for
> [CONNECTOR:48:DP-3]
> [ 49.985273] [drm:update_connector_routing] [CONNECTOR:48:DP-3] keeps
> [ENCODER:33:DP MST-33], now on [CRTC:21:crtc-0]
> [ 49.985275] [drm:update_connector_routing] Updating routing for
> [CONNECTOR:51:DP-4]
> [ 49.985278] [drm:steal_encoder] [ENCODER:33:DP MST-33] in use on
> [CRTC:21:crtc-0], stealing it
> [ 49.985281] [drm:update_connector_routing] [CONNECTOR:51:DP-4] using
> [ENCODER:33:DP MST-33] on [CRTC:21:crtc-0]
>
> This case is not allowed, similar to the previous case of 2 connectors newly
> assigned to the same encoder.
>
> Signed-off-by: Maarten Lankhorst <maarten.lankhorst at linux.intel.com>
What index something has is pretty arbitrary, i.e. I don't understand at
all what exactly you're trying to fix here, and what the problem is. Note
that this patch here seems to break the stealing-prevention logic: We're
supposed to steal the first time around, but not move an already stolen
encoder further (to make sure that all connectors in the current set can
be lit up).
-Daniel
> ---
> drivers/gpu/drm/drm_atomic_helper.c | 12 ++++++------
> 1 file changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_atomic_helper.c
> b/drivers/gpu/drm/drm_atomic_helper.c
> index fc1c06b1d8bb..fa708559542c 100644
> --- a/drivers/gpu/drm/drm_atomic_helper.c
> +++ b/drivers/gpu/drm/drm_atomic_helper.c
> @@ -88,18 +88,18 @@ drm_atomic_helper_plane_changed(struct drm_atomic_state
> *state,
>
> static bool
> check_pending_encoder_assignment(struct drm_atomic_state *state,
> - struct drm_encoder *new_encoder)
> + struct drm_encoder *new_encoder,
> + int conn_idx)
> {
> struct drm_connector *connector;
> struct drm_connector_state *conn_state;
> int i;
>
> for_each_connector_in_state(state, connector, conn_state, i) {
> - if (conn_state->best_encoder != new_encoder)
> - continue;
> + if (i >= conn_idx)
> + break;
>
> - /* encoder already assigned and we're trying to re-steal it! */
> - if (connector->state->best_encoder != conn_state->best_encoder)
> + if (conn_state->best_encoder == new_encoder)
> return false;
> }
>
> @@ -272,7 +272,7 @@ update_connector_routing(struct drm_atomic_state *state,
> return 0;
> }
>
> - if (!check_pending_encoder_assignment(state, new_encoder)) {
> + if (!check_pending_encoder_assignment(state, new_encoder, conn_idx)) {
> DRM_DEBUG_ATOMIC("Encoder for [CONNECTOR:%d:%s] already
> assigned\n",
> connector->base.id,
> connector->name);
> --
> 2.1.0
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
