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

From: Jesse Barnes <jbarnes@virtuousgeek.org>
To: Daniel Vetter <daniel@ffwll.ch>
Cc: intel-gfx@lists.freedesktop.org
Subject: Re: [PATCH] drm/i915: Update intel_dp_hpd_pulse() to check link status for non-MST operation
Date: Mon, 09 Mar 2015 12:07:31 -0700	[thread overview]
Message-ID: <54FDEF73.20703@virtuousgeek.org> (raw)
In-Reply-To: <20150309172916.GW24485@phenom.ffwll.local>

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.

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