[Intel-gfx] [PATCH] drm/i915: Fix the HDMI hot plug disconnection failure (v4)

[Guang Bai]

On Fri, 4 Jan 2019 12:02:34 +0800
Chris Chiu <chiu at endlessm.com> wrote:

> On Thu, Jan 3, 2019 at 1:50 AM Guang Bai <guang.bai at intel.com> wrote:
> >
> > On Wed, 2 Jan 2019 17:29:46 +0800
> > Chris Chiu <chiu at endlessm.com> wrote:
> >  
> > > Happy New Year.
> > > Sorry for bothering you guys again, I don't really want to make
> > > myself a nuisance.
> > > Is there any better idea for fixing this issue?  
> > I've already back ported this change into the kernel 4.18.17 and
> > sent it to our customer for integration test - So far so good.
> > Thanks,
> > -Guang  
> 
> Thanks, Guang.
> Can I expect to see it in next kernel release? Or we need to wait
> until more positive results coming?

- I'm not sure if and when my changes will pass the review and merged
  into the upstream kernel. I'm stuck on re-testing my fix with all
  those claimed failures from HDMI port live status checking happened
  sometimes before. I don't have any background info on those failures
  simply because I stepped into DRM/i915 area in recent one or two
  years after working on other OS/platform's gfx driver for more than
  two decades
- My customer can't wait for this long, endless review time-period and
  just go ahead on proactive integration testing. They'll update us the
  results in a couple of days
Thanks,
-Guang
> 
> > >
> > > Chris
> > >
> > > On Mon, Dec 3, 2018 at 6:38 PM Chris Chiu <chiu at endlessm.com>
> > > wrote:  
> > > >
> > > > On Fri, Nov 30, 2018 at 1:15 AM Guang Bai <guang.bai at intel.com>
> > > > wrote:  
> > > > >
> > > > > On Thu, 29 Nov 2018 10:17:49 +0200
> > > > > Jani Nikula <jani.nikula at intel.com> wrote:
> > > > >  
> > > > > > On Wed, 28 Nov 2018, Guang Bai <guang.bai at intel.com>
> > > > > > wrote:  
> > > > > > > On some GEN9 platforms, slowly unplugging (wiggling) the
> > > > > > > HDMI cable makes the kernel to believe the HDMI display
> > > > > > > is still connected. This is because the HDMI DDC lines are
> > > > > > > disconnected a little bit later after the hot-plug
> > > > > > > interrupt triggered thus an immediate edid fetch can be
> > > > > > > made. This problem has been identified by more than one
> > > > > > > customer recently. Use digital port live states to
> > > > > > > authorize the edid read at HDMI detection point will
> > > > > > > ensure most of the display related software states
> > > > > > > updated and rest of them will be renewed accordingly when
> > > > > > > the port is connected.
> > > > > > >
> > > > > > > v2: Fix the formatting issue
> > > > > > > v3: Use digital port states to authorize the edid read
> > > > > > > v4: Add comments on issue histories and rationale of the
> > > > > > > fix (Chris W)  
> > > > > >
> > > > > > You're not answering Chris Wilson's question.
> > > > > >
> > > > > > Why do you think the problems we've historically had with
> > > > > > live status are no longer a problem? We've tried and
> > > > > > reverted live status checks at least twice before because
> > > > > > of regressions. Why do you think this time there won't be
> > > > > > regressions? Why do you think this patch makes forward
> > > > > > progress?  
> > > > > Jani,
> > > > > I'm still new to kernel developments compared with all of you
> > > > > working in this area for many years - Haven't got any
> > > > > feedbacks on how exactly the HDMI live statue *not* fit for
> > > > > HDMI hot-plug related port status checking, neither had time
> > > > > to track all upstream bugzilla, plus not working directly
> > > > > with Intel OTC teams
> > > > > - What are those failing cases/regressions you mentioned
> > > > > above?
> > > > > - what were the kernel versions related with those
> > > > > developments?
> > > > > - Given the fact i915 architecture and implementation are
> > > > > constantly evolving - Should we re-visit those issues with
> > > > > current kernel implementation?
> > > > > - Fundamentally, do you think the edid fetch is still *valid*
> > > > > when the HDMI is unplugged (status either from PCH or DE)? Or
> > > > > other platform configurations may present more complexities
> > > > > such as kvm switches are used along with HDMI?
> > > > > Again, if you could provide me more historical issue details,
> > > > > I'd like to have some reviews/re-investigation for those
> > > > > cases with current 4.20 kernel.
> > > > > Thanks,
> > > > > -Guang  
> > > >
> > > > Hi Jani,
> > > >     I don't know the history and what kind of painful regression
> > > > that you had run into. Could you maybe provide a test plan or
> > > > some test cases for the regression verification? I can follow
> > > > steps to try to verify whether if the patch can work on all
> > > > cases.
> > > >
> > > > Chris
> > > >  
> > > > > >
> > > > > > I've *repeatedly* said from the beginning that I am very
> > > > > > sceptical of using live status because we've been burned by
> > > > > > it so many times before. I don't much care to repeat this
> > > > > > anymore.
> > > > > >
> > > > > >
> > > > > > BR,
> > > > > > Jani.
> > > > > >
> > > > > >  
> > > > > > >
> > > > > > > Cc: Jani Nikula <jani.nikula at intel.com>
> > > > > > > Cc: Chris Chiu <chiu at endlessm.com>
> > > > > > > Cc: Chris Wilson <chris at chris-wilson.co.uk>
> > > > > > > Signed-off-by: Guang Bai <guang.bai at intel.com>
> > > > > > > ---
> > > > > > >  drivers/gpu/drm/i915/intel_hdmi.c | 2 +-
> > > > > > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > > > > >
> > > > > > > diff --git a/drivers/gpu/drm/i915/intel_hdmi.c
> > > > > > > b/drivers/gpu/drm/i915/intel_hdmi.c index e2c6a2b..8cf7c78
> > > > > > > 100644 --- a/drivers/gpu/drm/i915/intel_hdmi.c
> > > > > > > +++ b/drivers/gpu/drm/i915/intel_hdmi.c
> > > > > > > @@ -1929,7 +1929,7 @@ intel_hdmi_detect(struct
> > > > > > > drm_connector *connector, bool force)
> > > > > > >     intel_display_power_get(dev_priv, POWER_DOMAIN_GMBUS);
> > > > > > >
> > > > > > > -   if (IS_ICELAKE(dev_priv) &&
> > > > > > > +   if ((IS_ICELAKE(dev_priv) || IS_GEN9_BC(dev_priv)) &&
> > > > > > >         !intel_digital_port_connected(encoder))
> > > > > > >             goto out;  
> > > > > >  
> > > > >  
> >