Re: [PATCH] drm/i915: Update intel_dp_hpd_pulse() to check link status for non-MST operation

[Jesse Barnes <jbarnes@virtuousgeek.org>]

On 03/09/2015 02:04 PM, Ville Syrjälä wrote:
> On Mon, Mar 09, 2015 at 12:07:31PM -0700, Jesse Barnes wrote:
>> On 03/09/2015 10:29 AM, Daniel Vetter wrote:
>>> On Mon, Mar 09, 2015 at 08:34:49AM -0700, Jesse Barnes wrote:
>>>> On 03/06/2015 08:34 AM, Daniel Vetter wrote:
>>>>> On Thu, Mar 05, 2015 at 11:22:19AM -0700, Todd Previte wrote:
>>>>>> +	} else {
>>>>>> +		/* SST mode - handle short/long pulses here */
>>>>>> +		drm_modeset_lock(&dev->mode_config.connection_mutex, NULL);
>>>>>> +		/* Clear compliance testing flags/data here to prevent
>>>>>> +		 * false detection in userspace */
>>>>>> +		intel_dp->compliance_test_data = 0;
>>>>>> +		intel_dp->compliance_testing_active = 0;
>>>>>> +		/* For a long pulse in SST mode, disable the main link */
>>>>>> +		if (long_hpd) {
>>>>>> +			I915_WRITE(DP_TP_CTL(intel_dig_port->port),
>>>>>> +					      ~DP_TP_CTL_ENABLE);
>>>>>> +		}
>>>>>
>>>>> Disabling the  main link should be done in userspace. All long pulse
>>>>> requests should be forwarded to userspace as a hotplug event. Userspace
>>>>> can then react to that hotplug appropriately. This way we can again
>>>>> exercise the normal operation of all our dp code.
>>>>
>>>> What's your concern here?  Do you want to make sure we get coverage on
>>>> dp_link_down()?  It looks like that might be safe to use here instead of
>>>> flipping the disable bit directly.  Or did you want to go through the
>>>> whole pipe/port shutdown sequence as well?  If so, I think the dpms
>>>> tests will already cover that, separate from simple compliance.
>>>
>>> This is likely to upset the state checker, we've already had some fun with
>>> killing the hard dp pipe disable that the hdp code occasionally did. Well,
>>> still have. The other reason is that dp compliance testing with
>>> special-case code is somewhat pointless, except when the compliance test
>>> contracts what real-world experience forces us to do. For these exceptions
>>> I'd like that we fully understand them and also document them. Disabling
>>> the link on a full hot-unplug is something we can (and most DE actually
>>> do) do.
>>
>> If we end up hitting the checker while testing, then yeah it would spew.
>>
>> But I thought this was mainly about testing the DP code, making sure we
>> can up/down links, train at different parameters, etc, not about going
>> through full mode sets all the time...
>>
>> But either way, I agree we should be documenting this behavior so we
>> don't get stuck trying to figure it out later.
> 
> I don't think we should be killing the port like this. It'll also kill
> the pipe on some platforms and then we get all kinds of pipe stuck
> warnings. So I think we'd definitely want a more graceful shutdown of
> things.

Does that affect current platforms?  Or just CTG and ILK?  I can guess
BYT & BSW might be affected, but I haven't tested.  As long as we just
up/down the port w/o anything else it might be able to work...

> I thought we actually discussed about going to the other direction, ie.
> potentially allowing the link to brought up without the pipe and
> enabling/disabling the pipe independently while the link remains up and
> running?

I guess I was thinking the reverse: that bringing up the port w/o a pipe
driving it would be more likely to cause problems, but I guess we'll
need testing.

Depending on what we find, we could change the logic to accommodate the
platforms we want to test (HSW+ and BYT+ I think, though we could limit
it to even newer ones if those are too tough to handle).

Jesse
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx