Op 08-11-17 om 14:04 schreef Ville Syrjälä: > On Wed, Nov 08, 2017 at 01:57:50PM +0100, Daniel Vetter wrote: >> Two bits: >> - check actual atomic state, the legacy stuff can only be looked at >> from within the atomic_commit_tail function, since it's only >> protected by ordering and not by any locks. >> >> - Make sure we don't wreak the work an ongoing nonblocking commit is >> doing. > I still think we need to move the retraining to the hotplug work. But lets not block a fix for that. :) Reviewed-by: Maarten Lankhorst <maarten.lankhorst@xxxxxxxxxxxxxxx> >> v2: We need the crtc lock too, because a plane update might change it >> without having to acquire the connection_mutex (Maarten). Use >> Maarten's changes for this locking, while keeping the logic that uses >> the connection->commit->hw_done signal for syncing with nonblocking >> commits. >> >> Cc: Manasi Navare <manasi.d.navare@xxxxxxxxx> >> Cc: Maarten Lankhorst <maarten.lankhorst@xxxxxxxxxxxxxxx> >> Cc: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx> >> References: https://bugs.freedesktop.org/show_bug.cgi?id=103336 >> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=99272 >> Signed-off-by: Daniel Vetter <daniel.vetter@xxxxxxxxx> >> --- >> drivers/gpu/drm/i915/intel_dp.c | 56 ++++++++++++++++++++++++++++++++++++----- >> 1 file changed, 50 insertions(+), 6 deletions(-) >> >> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c >> index d27c0145ac91..7cd7ab4fb431 100644 >> --- a/drivers/gpu/drm/i915/intel_dp.c >> +++ b/drivers/gpu/drm/i915/intel_dp.c >> @@ -4319,6 +4319,8 @@ static void >> intel_dp_check_link_status(struct intel_dp *intel_dp) >> { >> struct intel_encoder *intel_encoder = &dp_to_dig_port(intel_dp)->base; >> + struct drm_connector_state *conn_state = >> + intel_dp->attached_connector->base.state; >> struct drm_device *dev = intel_dp_to_dev(intel_dp); >> u8 link_status[DP_LINK_STATUS_SIZE]; >> >> @@ -4329,10 +4331,15 @@ intel_dp_check_link_status(struct intel_dp *intel_dp) >> return; >> } >> >> - if (!intel_encoder->base.crtc) >> + if (!conn_state->crtc) >> return; >> >> - if (!to_intel_crtc(intel_encoder->base.crtc)->active) >> + WARN_ON(!drm_modeset_is_locked(&conn_state->crtc->mutex)); >> + >> + if (!conn_state->crtc->state->active) >> + return; >> + >> + if (!try_wait_for_completion(&conn_state->commit->hw_done)) >> return; >> >> /* >> @@ -4368,7 +4375,6 @@ intel_dp_check_link_status(struct intel_dp *intel_dp) >> static bool >> intel_dp_short_pulse(struct intel_dp *intel_dp) >> { >> - struct drm_device *dev = intel_dp_to_dev(intel_dp); >> struct intel_encoder *intel_encoder = &dp_to_dig_port(intel_dp)->base; >> u8 sink_irq_vector = 0; >> u8 old_sink_count = intel_dp->sink_count; >> @@ -4408,9 +4414,8 @@ intel_dp_short_pulse(struct intel_dp *intel_dp) >> DRM_DEBUG_DRIVER("CP or sink specific irq unhandled\n"); >> } >> >> - drm_modeset_lock(&dev->mode_config.connection_mutex, NULL); >> intel_dp_check_link_status(intel_dp); >> - drm_modeset_unlock(&dev->mode_config.connection_mutex); >> + >> if (intel_dp->compliance.test_type == DP_TEST_LINK_TRAINING) { >> DRM_DEBUG_KMS("Link Training Compliance Test requested\n"); >> /* Send a Hotplug Uevent to userspace to start modeset */ >> @@ -4860,8 +4865,19 @@ intel_dp_detect(struct drm_connector *connector, >> connector->base.id, connector->name); >> >> /* If full detect is not performed yet, do a full detect */ >> - if (!intel_dp->detect_done) >> + if (!intel_dp->detect_done) { >> + struct drm_crtc *crtc; >> + int ret; >> + >> + crtc = connector->state->crtc; >> + if (crtc) { >> + ret = drm_modeset_lock(&crtc->mutex, ctx); >> + if (ret) >> + return ret; >> + } >> + >> status = intel_dp_long_pulse(intel_dp->attached_connector); >> + } >> >> intel_dp->detect_done = false; >> >> @@ -5146,10 +5162,38 @@ intel_dp_hpd_pulse(struct intel_digital_port *intel_dig_port, bool long_hpd) >> } >> >> if (!intel_dp->is_mst) { >> + struct drm_modeset_acquire_ctx ctx; >> + struct drm_connector *connector = &intel_dp->attached_connector->base; >> + struct drm_crtc *crtc; >> + int iret; >> + >> + drm_modeset_acquire_init(&ctx, 0); >> +retry: >> + iret = drm_modeset_lock(&dev->mode_config.connection_mutex, &ctx); >> + if (iret) >> + goto err; >> + >> + crtc = connector->state->crtc; >> + if (crtc) { >> + iret = drm_modeset_lock(&crtc->mutex, &ctx); >> + if (iret) >> + goto err; >> + } >> + >> if (!intel_dp_short_pulse(intel_dp)) { >> intel_dp->detect_done = false; >> goto put_power; >> } >> + >> +err: >> + if (iret == -EDEADLK) { >> + drm_modeset_backoff(&ctx); >> + goto retry; >> + } >> + >> + drm_modeset_drop_locks(&ctx); >> + drm_modeset_acquire_fini(&ctx); >> + WARN(iret, "Acquiring modeset locks failed with %i\n", iret); >> } >> >> ret = IRQ_HANDLED; >> -- >> 2.15.0 _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx
