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

[Chris Chiu]

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;
> >
>