[Intel-gfx] [PATCH RESEND] drm: i915: Don't try detecting sinks on ports already in use

[Gabriel Krisman Bertazi]

Ville Syrjälä <ville.syrjala at linux.intel.com> writes:


>>  	mutex_lock(&dev->mode_config.mutex);
>>  	DRM_DEBUG_KMS("running encoder hotplug functions\n");
>> @@ -339,13 +340,18 @@ static void i915_hotplug_work_func(struct work_struct *work)
>>  		if (!intel_connector->encoder)
>>  			continue;
>>  		intel_encoder = intel_connector->encoder;
>> -		if (hpd_event_bits & (1 << intel_encoder->hpd_pin)) {
>> +		enc_hpd_pin_mask = (1 << intel_encoder->hpd_pin);
>> +		if (hpd_event_bits & enc_hpd_pin_mask) {
>>  			DRM_DEBUG_KMS("Connector %s (pin %i) received hotplug event.\n",
>>  				      connector->name, intel_encoder->hpd_pin);
>>  			if (intel_encoder->hot_plug)
>>  				intel_encoder->hot_plug(intel_encoder);
>> -			if (intel_hpd_irq_event(dev, connector))
>> +			if (intel_hpd_irq_event(dev, connector)) {
>>  				changed = true;
>> +				if (connector->status ==
>> +				    connector_status_connected)
>> +					hpd_event_bits &= ~(enc_hpd_pin_mask);
>
> This won't work correctly because now it will fail to update
> connector->status for the other connector. I admit that being fast
> enough to switch between DP<->HDMI directly while the HPD interrupt
> is enabled is perhaps a little unlikely, but it can certainly happen
> very easily if the interrupt has been disabled.

Hi Ville,

I see.  So, still racy when dealing with multiple connectors events on
the same HPD line.  Nice catch. I didn't see that.

> We alrady have other bugs in that area. Eg. if you manage to switch to a
> totally different display without noticing the disconnected state in
> between we'll skip sending the uevent. We should really be checking if
> the sink changed somehow (eg. EDID changed) and in that case send the
> uevent anyway.
>
> I also don't think this can have anything to do with vblank tests
> failing, unless there's some other problem involved that somehow
> triggers HPDs while the test is doing its measurements.

I thought about (blindly) reducing the overhead of
i915_hotplug_work_func, as mentioned by Chris in his second comment in
the report for bug 100215.  But I understand that the latest
reproduction of 100215 triggered a lot of timeouts directly from
userspace, and I didn't find any HPD occurrences there.

Anyway, correct me if I'm wrong, but I still understand that the amount
of time the DP detection takes is a little too much, and the fact that
it triggers whenever we connect anything to a shared port is very odd
behavior.  There are a few bugs reported where the DP detection timeouts
was pointed as the probable root cause.

I came up with two patches to workaround the issue but they both had
issues of their own.  I understand the first version I submitted is a
better approach than the second, and I think we could explore something
in that direction, once we address the race condition first pointed out
by Chris.  He suggested on an earlier email that we leverage the
knowledge about digital ports to improve the overall detection, but I'm
not sure how to start that.  I've been checking the specs and I don't
see how we could decide whether we have a DP device without doing the
dpcp dance.  Do you have any suggestions on that?  I'm new to i915, so
I'm not sure if I'm missing something, if that is the case, please point
it out. :)

-- 
Gabriel Krisman Bertazi