[Intel-gfx] [PATCH] drm/i915: Add a hotplug connector property

[Chris Wilson]

On Sat, Jun 08, 2013 at 02:34:20PM +0200, Daniel Vetter wrote:
> On Sat, Jun 8, 2013 at 9:08 AM, Chris Wilson <chris at chris-wilson.co.uk> wrote:
> > On Wed, Jun 05, 2013 at 10:59:23PM +0100, Chris Wilson wrote:
> >> It is useful for userspace to know when it may be able to skip a forced
> >> detection cycle as the connector maintains an accurate status. It also
> >> provides status feedback to the user of the hotplug storm detection,
> >> and the ability to override the method used for detecting changes in
> >> connection status.
> >
> > Please review this in the context of the user being able to manually
> > override the hotplug detection method for an individual connector. In
> > that role this makes a lot of sense as it should improve the user
> > experience in quite a few situations.
> 
> The problem I see is that userspace _can't_ trust the kernel like
> this, i.e. the optimization you do in
> 
> http://cgit.freedesktop.org/~ickle/xf86-video-intel/commit/?h=hotplug-property&id=995173e5e5adf8297957155cceab28bdf928022b
> 
> to not force the kernel into the full ->fill_modes dance will break
> systems. We do have a bunch of machines on record where we claim to
> have full hpd support, but it doesn't seem to do anything useful.

It is these machines that require the per-connector quirk to turn off
hotplug detection anyway. And there are users that need to turn off all
hotplug detection (hardware and polling) on certain connectors whilst
plugged into their kvm.

I'm not happy with the ddx portion as it breaks the RandR expectation of
->detect performing an explicit probe, which is exposed over the
protocol. RandR added the ability to query the existing modes, so I am
going to have to live with the current interface and keep on hoping that
everyone migrates over to the cheap query.
 
> But it's not just machines where hotplug flat-out does nothing, hw
> hotplug is also pretty racy. We've tried to plug them a bit, but due
> to the completely broken machines that regressed again.

If the hpd itself is racy, the detect is likely to be as well. It's a
no-win situation.
 
> The other thing that irks me is that we give the user the option to
> override stuff. Imo stuff _really_ should just work, and it sounds
> like we have to do the same retraining exercise with "please drop
> poll=0 from your boot options" as with disabling modeset. So up to a
> certain catastrophic level I prefer pitchforks in front of my house
> over people just quirking each and every machine themselves.

Those who suffer are already pretty irate. I take the opposite view, in
that we should not quirk in the kernel unless there is a user switch as
well. What's is good for one machine, is likely required for many more -
so making that option available makes it easier to test, and gives users
control before we can get a quirked kernel to them (if it is even
possible to add a quirk for their particular machine).

> Lastly I'm not too happy about how complicated the re-detect avoidance
> logic works with your two kernel patches + the userspace change.

I think the root cause here is that fill_modes should not be doing a
detect, but that detect does the the fill_modes. I.e. the EDID and
probed mode list for a connector should be considered static for the
lifetime of the connection.

The complication in userspace is a separate issue in trying to avoid
doing a forced detection because the RandR interface doesn't distinguish
between returning the latest information after a hotplug uevent versus
doing an explicit probe. (So yes, it is a repetition of the same problem
we have in the kernel code.)

[snip]

> Finally a pure process thing on top: I'd like to confirm that Egbert's
> hotplug storm stuff is solid before we frob our detect code a bit
> more. Which means 3.10 should get out the door first and we need to
> ping the various bug reporters a bit.

In hindsight, I would have liked the hotplug property in with
Egbert's automatic storm mitigation.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre