Re: [Intel-gfx] [PATCH v2 2/2] drm/i915: Wait for pending modesets to complete before trying link training, v2.

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Op 14-06-17 om 15:44 schreef Ville Syrjälä:
> On Wed, Jun 14, 2017 at 11:08:42AM +0200, Maarten Lankhorst wrote:
>> The nonblocking modeset tests were failing on systems with a DP output,
>> because lijnk training could occur during the modeset.
>>
>> With normal modesets we're protected by the connection_mutex lock,
>> but nonblocking modesets drop locks before completing. To get around
>> that, check if the most recent crtc_state has a modeset, and if this
>> is the case, wait for all outstanding atomic updates on that crtc
>> to complete.
>>
>> This fixes the following spew when running nonblocking tests:
>> [ 1061.653778] ------------[ cut here ]------------
>> [ 1061.653781] WARNING: CPU: 1 PID: 8165 at
>> drivers/gpu/drm/drm_irq.c:1217 drm_wait_one_vblank+0x165/0x1a0
>> [ 1061.653782] vblank not available on crtc 0, ret=-22
>> [ 1061.653783] Modules linked in: vgem snd_hda_intel i915
>> snd_hda_codec_hdmi snd_hda_codec_realtek snd_hda_codec_generic
>> x86_pkg_temp_thermal intel_powerclamp coretemp crct10dif_pclmul
>> crc32_pclmul ghash_clmulni_intel snd_hda_codec snd_hwdep snd_hda_core
>> snd_pcm mei_me mei e1000e ptp pps_core i2c_hid [last unloaded: vgem]
>> [ 1061.653796] CPU: 1 PID: 8165 Comm: kworker/1:3 Tainted: G     U  W
>> 4.10.0-rc1-CI-Custom_114+ #1
>> [ 1061.653797] Hardware name: GIGABYTE GB-BKi5A-7200/MFLP5AP-00, BIOS F1
>> 07/27/2016
>> [ 1061.653812] Workqueue: events i915_hpd_poll_init_work [i915]
>> [ 1061.653813] Call Trace:
>> [ 1061.653816]  dump_stack+0x67/0x92
>> [ 1061.653818]  __warn+0xc6/0xe0
>> [ 1061.653819]  warn_slowpath_fmt+0x4a/0x50
>> [ 1061.653820]  ? drm_vblank_get+0x74/0xc0
>> [ 1061.653821]  drm_wait_one_vblank+0x165/0x1a0
>> [ 1061.653823]  ? drm_dp_dpcd_write+0x16/0x20
>> [ 1061.653837]  ? intel_dp_set_link_train+0x44/0xe0 [i915]
>> [ 1061.653851]  intel_dp_check_link_status+0x113/0x1a0 [i915]
>> [ 1061.653864]  intel_dp_detect+0x5a1/0xa40 [i915]
>> [ 1061.653866]  drm_helper_hpd_irq_event+0x8d/0x110
>> [ 1061.653880]  i915_hpd_poll_init_work+0xab/0xd0 [i915]
>> [ 1061.653882]  process_one_work+0x1f4/0x6d0
>> [ 1061.653883]  ? process_one_work+0x16e/0x6d0
>> [ 1061.653885]  worker_thread+0x49/0x4a0
>> [ 1061.653886]  kthread+0x107/0x140
>> [ 1061.653888]  ? process_one_work+0x6d0/0x6d0
>> [ 1061.653889]  ? kthread_create_on_node+0x40/0x40
>> [ 1061.653891]  ret_from_fork+0x27/0x40
>> [ 1061.653892] ---[ end trace f6037618668d4cbc ]---
>> [ 1061.656353] [drm:intel_dp_set_idle_link_train [i915]] *ERROR* Timed out waiting for DP idle patterns
>>
>> Changes since v1:
>> - intel_dp_detect() now holds connection_mutex and has an acquire_ctx,
>>   rewrite patch to take advantage of it.
>>
>> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@xxxxxxxxxxxxxxx>
>> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=99272
>> Testcase: kms_atomic_transitions.1x-modeset-transitions-nonblocking
>> ---
>>  drivers/gpu/drm/i915/intel_dp.c | 72 ++++++++++++++++++++++++++++++++++-------
>>  1 file changed, 60 insertions(+), 12 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
>> index 64fa774c855b..c7a66883e453 100644
>> --- a/drivers/gpu/drm/i915/intel_dp.c
>> +++ b/drivers/gpu/drm/i915/intel_dp.c
>> @@ -4243,21 +4243,32 @@ 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_device *dev = intel_dp_to_dev(intel_dp);
>>  	u8 link_status[DP_LINK_STATUS_SIZE];
>> +	struct intel_connector *intel_connector = intel_dp->attached_connector;
>> +	struct drm_crtc *crtc;
>>  
>> -	WARN_ON(!drm_modeset_is_locked(&dev->mode_config.connection_mutex));
>> -
>> -	if (!intel_dp_get_link_status(intel_dp, link_status)) {
>> -		DRM_ERROR("Failed to get link status\n");
>> +	crtc = intel_connector->base.state->crtc;
>> +	if (!crtc)
>>  		return;
>> -	}
>>  
>> -	if (!intel_encoder->base.crtc)
>> +	WARN_ON(!drm_modeset_is_locked(&crtc->mutex));
>> +
>> +	if (!crtc->state->active)
>>  		return;
>>  
>> -	if (!to_intel_crtc(intel_encoder->base.crtc)->active)
>> +	if (drm_atomic_crtc_needs_modeset(crtc->state)) {
>> +		int ret;
>> +
>> +		/* wait for atomic modeset to complete */
>> +		ret = drm_atomic_helper_wait_for_hw_done(crtc);
>> +		if (ret < 0)
>> +			DRM_ERROR("Waiting for hw_done timed out\n");
>> +	}
>> +
>> +	if (!intel_dp_get_link_status(intel_dp, link_status)) {
>> +		DRM_ERROR("Failed to get link status\n");
>>  		return;
>> +	}
>>  
>>  	/*
>>  	 * Validate the cached values of intel_dp->link_rate and
>> @@ -4291,7 +4302,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;
>> @@ -4331,9 +4341,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 */
>> @@ -4735,8 +4744,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;
>>  
>> @@ -5028,10 +5048,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);
> Please no modeset locks in the dig_work. Hmm. Looks like we already have
> it there though :( I thought we had moved the link retraining fully into
> the hoplug_work, but I guess not. Can you try to do that instead?
I see you've had a series to fix this, what happened to it?
>> +			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.11.0
>>
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
>> https://lists.freedesktop.org/mailman/listinfo/intel-gfx


_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel




[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux