[PATCH] drm/radeon: Retry DDC probing on DVI on failure if we got an HPD interrupt

[Lyude]

Ping
Do we have any consensus on the best way of handling this situation in the
driver?

On Mon, 2015-11-23 at 14:20 +0000, Deucher, Alexander wrote:
> > -----Original Message-----
> > From: Lyude [mailto:cpaul at redhat.com]
> > Sent: Sunday, November 22, 2015 9:45 PM
> > To: Koenig, Christian; Daniel Stone
> > Cc: Deucher, Alexander; David Airlie; dri-devel; Linux Kernel Mailing List;
> > Jerome Glisse; Benjamin Tissoires
> > Subject: Re: [PATCH] drm/radeon: Retry DDC probing on DVI on failure if we
> > got an HPD interrupt
> > 
> > On Sat, 2015-11-21 at 16:22 +0100, Christian König wrote:
> > > On 21.11.2015 15:49, Daniel Stone wrote:
> > > > Hi,
> > > > 
> > > > On 21 November 2015 at 14:22, Christian König <christian.koenig at amd
> > > > .com> wrote:
> > > > > On 20.11.2015 16:52, cpaul at redhat.com wrote:
> > > > > > This is somewhat rare on most cards (depending on what angle
> > > > > > you plug
> > > > > > the DVI connector in), but on some cards it happens constantly.
> > > > > > The
> > > > > > Radeon R5 on the machine used for testing this patch for
> > > > > > instance, runs
> > > > > > into this issue just about every time I try to hotplug a DVI
> > > > > > monitor and
> > > > > > as a result hotplugging almost never works.
> > > > > > 
> > > > > > Rescheduling the hotplug work for a second when we run into an
> > > > > > HPD
> > > > > > signal with a failing DDC probe usually gives enough time for
> > > > > > the rest
> > > > > > of the connector's pins to make contact, and fixes this issue.
> > > > > > 
> > > > > > Signed-off-by: Stephen Chandler Paul <cpaul at redhat.com>
> > > > > 
> > > > > Yeah, that's something I always wondered a about bit as well.
> > > > > 
> > > > > Debouncing is something very common done in electronics, but as
> > > > > far as I
> > > > > know the HPD pins don't necessary have an RC circuit so we might
> > > > > need to
> > > > > handle this case in software here.
> > > > > 
> > > > > A delay of something between 10-30ms between the last HPD
> > > > > interrupt and
> > > > > further processing of the signal doesn't sounds like such a bad
> > > > > idea.
> > Unfortunately the delay needed to make hotplugging work on the system
> > mentioned in the commit log can actually be over 700ms.
> > > > > 
> > > > > Retrying on the other hand doesn't necessarily improve the
> > > > > situation cause
> > > > > the delay introduced by this might not be enough.
> > Yeah, but I would think it would make sense to retry here so long as we
> > back off after a certain time. This would also have the benefit of
> > skipping this delay on systems that don't need it.
> > > > > 
> > > > > So I would rather vote for a fixed delay between an HPD interrupt
> > > > > and
> > > > > actually starting to process anything.
> > > > Yes-ish. Debouncing is useful, and ignoring buggy devices (e.g.
> > > > those
> > > > on marginal power) which send you HPD storms as well. But DP relies
> > > > on
> > > > 'short HPD' pulses which can be as brief as 2ms. So attempting to
> > > > totally debounce all HPD won't work.
> > > Well the discussion so far was about HPD on DVI only.
> > > 
> > > I'm not so deep into DP, but why should it uses HPD pulses of less
> > > than 2ms?
> > This is part of the DP spec iirc. This being said though, the issue
> > here with the HPD signal coming before the connector is ready only
> > happens on DVI. I haven't ever run into this issue with any HDMI cables
> > or DP cables, so I'm against imposing this on all connectors.
> > 
> > One of the solutions I've been thinking about with this: In
> > radeon_dvi_detect(), if we get a real hotplug signal retry the DDC
> > probe until at least a second has passed, after which we back off and
> > assume the port is disconnected.
> 
> FWIW, there are registers to adjust how long the hpd needs to be asserted
> before the hpd connection and short pulse interrupts are triggered.  See
> DC_HPDx_CONTROL.  Maybe adjusting them would help.  We currently just write
> the default value, but it might be better to RMW the value in case there is a
> special golden value set by the vbios at init time.
> 
> Alex
> 
> > > 
> > > Regards,
> > > Christian.
> > > 
> > > > 
> > > > Cheers,
> > > > Daniel
> > > 
> > --
> > Cheers,
> > 	Lyude
> 
-- 
Cheers,
	Lyude