On Thu, 2015-08-27 at 13:13 +0200, Maarten Lankhorst wrote: > connector->encoder is initialized as NULL. Fix this by setting it in > during pre enable. MST connectors are not read out during initial hw > readout, and have no fixed encoder mappings. So it's harmless to > return false when the connector has never been assigned to an encoder. > > Signed-off-by: Maarten Lankhorst <maarten.lankhorst@xxxxxxxxxxxxxxx> > --- > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > index 738178a0ac96..e3922d973db0 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -6283,7 +6283,7 @@ static void intel_connector_check_state(struct intel_connector *connector) > connector->base.name); > > if (connector->get_hw_state(connector)) { > - struct drm_encoder *encoder = &connector->encoder->base; > + struct intel_encoder *encoder = connector->encoder; > struct drm_connector_state *conn_state = connector->base.state; > > I915_STATE_WARN(!crtc, > @@ -6295,13 +6295,13 @@ static void intel_connector_check_state(struct intel_connector *connector) > I915_STATE_WARN(!crtc->state->active, > "connector is active, but attached crtc isn't\n"); > > - if (!encoder) > + if (!encoder || encoder->type == INTEL_OUTPUT_DP_MST) > return; > > - I915_STATE_WARN(conn_state->best_encoder != encoder, > + I915_STATE_WARN(conn_state->best_encoder != &encoder->base, > "atomic encoder doesn't match attached encoder\n"); > > - I915_STATE_WARN(conn_state->crtc != encoder->crtc, > + I915_STATE_WARN(conn_state->crtc != encoder->base.crtc, > "attached encoder crtc differs from connector crtc\n"); > } else { > I915_STATE_WARN(crtc && crtc->state->active, > diff --git a/drivers/gpu/drm/i915/intel_dp_mst.c b/drivers/gpu/drm/i915/intel_dp_mst.c > index ebf205462631..d606721b1788 100644 > --- a/drivers/gpu/drm/i915/intel_dp_mst.c > +++ b/drivers/gpu/drm/i915/intel_dp_mst.c > @@ -159,6 +159,11 @@ static void intel_mst_pre_enable_dp(struct intel_encoder *encoder) > return; > } > > + /* MST encoders are bound to a crtc, not to a connector, > + * force the mapping here for get_hw_state. > + */ > + found->encoder = encoder; > + > DRM_DEBUG_KMS("%d\n", intel_dp->active_mst_links); > intel_mst->port = found->port; > > @@ -392,7 +397,7 @@ static const struct drm_encoder_funcs intel_dp_mst_enc_funcs = { > > static bool intel_dp_mst_get_hw_state(struct intel_connector *connector) > { > - if (connector->encoder) { > + if (connector->encoder && connector->base.state->crtc) { Is this here to cover the case where we disable this connector and assign the encoder to a different connector? I guess this should work since if we disable the connector than crtc will be null, and we'll assign a proper encoder when re-enabling. But now I'm wondering if it would make sense to set connector->encoder back to NULL in post_disable. Ander > enum pipe pipe; > if (!connector->encoder->get_hw_state(connector->encoder, &pipe)) > return false; > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@xxxxxxxxxxxxxxxxxxxxx > http://lists.freedesktop.org/mailman/listinfo/intel-gfx _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/intel-gfx