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. 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. 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. Lastly I'm not too happy about how complicated the re-detect avoidance logic works with your two kernel patches + the userspace change. Originally I've thought it doesn't solve the edid re-reading issue, but after carefully re-reading things I stand corrected: - output poll work or hpd handler call just ->detect from the helpers interface - if a change has happened it'll eventually call down to the fb helper (note that we might end up with kms only drivers who punt on this eventually). Thanks to the new force parameter it'll forego the ->detect and only call ->get_modes. - Then userspace makes sure that none of ->fill_modes will get called, which avoids both the ->detect and the ->get_modes. So it's pretty tricky, and also doesn't avoid re-reading the EDID if both ->detect and ->get_modes does that. Also I don't like that fbdev gets special treatment interface-wise compared to userspace (it can set force=false, which userspace can't). I've very much like to avoid this, our kms interface should be good enough for both userspace and legacy fbdev. 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. Cheers, Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch
