Re: [RFC] [PATCH 00/14] HPD/connector-polling rework

[Alex Deucher <alexdeucher@gmail.com>]

On Thu, Oct 4, 2012 at 1:16 PM, Daniel Vetter <daniel@ffwll.ch> wrote:
> On Thu, Oct 04, 2012 at 12:07:01PM -0400, Alex Deucher wrote:
>> On Thu, May 24, 2012 at 3:26 PM, Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
>> > Hi all,
>> >
>> > I've got fed up with our sorry state of connector detection and rampant edid re
>> > and rere-reading.
>> >
>> > This patch series lays the groundwork in the drm helpers so that drivers can
>> > avoid all this madness (at least on working hw) and properly cache the edid.
>> >
>> > With the additional changes for drm/i915, the edid is now read _once_ per plug
>> > event (or at boot-up/resume time). A further step would be to integrate the
>> > hotplug handling into the driver itself and only call ->detect on the connectors
>> > for which the irq handler received a hotplug event.
>> >
>> > By adding POLL_FORCE drivers can get back the old behaviour of calling ->detect
>> > every time probe_single_connector is called from userspace. I've splattered that
>> > over all drivers where I've thought it might be required.
>> >
>> > Note though that setting this doesn't avoid all regressions - the regular output
>> > poll work will still ignore any connectors with POLL_HPD set. If a driver/hw
>> > with broken hpd got away due to this, this will break stuff. But that should be
>> > easy to fix by admitting the defeat and setting POLL_CONNECT|DISCONNECT
>> > directly.
>> >
>> > If other people want to convert over their drivers, the following steps are
>> > required:
>> > - Ensure that the connector status is unknown every time the driver could have
>> >   missed a hpd event (e.g. after resume). drm_mode_config_reset will do that for
>> >   you.
>> > - Drop the POLL_FORCE flag for connectors where hdp is fully reliable.
>> > - Implement edid caching - that's a nice way to figure out whether hpd is
>> >   actually reliable, because if it isn't, this step will ensure that you get bug
>> >   reports because the the edid won't ever get updated ;-)
>> > - Optionally teach the driver some smarts about which specific connectors
>> >   actually got a hotplug event. Mostly useful on cheap hw (like intel's) that
>> >   can't distinguish between hdmi and dp without trying some aux channel
>> >   transfers.
>> >
>> > As you can guess from the patch series, I've discovered the hard way that i915
>> > sdvo support is totally broken. Tested on most of the intel machines I have and
>> > also quickly on my radeon hd5000.
>> >
>> > Comments, flames, ideas and test reports highly welcome.
>>
>> This set looks good to me.  Do you have plans to revive this?  At
>> least the common drm ones are much better than what we currently have.
>
> The issue I've meanwhile discovered with hpd handling is that we really
> need the smarts in the driver to only do probing on native hdmi/dp outputs
> if we have a hpd irq for that specific port: Since we have 2 encoders for
> the same port, if e.g. dp is enabled and we do edid probing on the hdmi
> encoder/connector the display might get unhappy, which can happen even
> with these patches applied e.g. when pluggin in a 2nd display.
>
> So the new plan (for 3.8 hopefully) is to revamp the i915-internal hpd
> handling and keep on using the crtc helper stuff. For connectors with
> reliable hpd we'd simply return the tracked stated in ->detect without
> touching the hw at all. So I don't think we need any changes in the
> hotplug/polling helper code. And looking again at these patches, they're a
> rather decent kludge and it's probably better to do this all in the
> drivers. Since the helper code sets the force parameter to false for all
> the background polling, you can just return any driver-internal cached
> state. And when force=true you could invalidate that cached state or do
> some additional (more expensive) detection.

Well the current code skips all hotplug event propagation if the poll
option is disabled (even for hpd capable connectors) which is fail so
the first few patches still seem like a win.  I guess you are not
planning to use any of the current common hpd code?  If so, do you
mind if we pull at least the first few patches in in the interim for
other drivers?

Alex

>
> In short: No plans to revive this, but certainly plans to improve the
> drm/i915 hpd handling.
>
> Cheers, Daniel
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> +41 (0) 79 365 57 48 - http://blog.ffwll.ch