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

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

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.

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?

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx