Re: [PATCH] drm/i915: Do not reset detect_done flag in intel_dp_detect

[Manasi Navare <manasi.d.navare@intel.com>]

On Wed, Dec 07, 2016 at 04:05:56PM +0200, Ville Syrjälä wrote:
> On Tue, Dec 06, 2016 at 04:43:51PM -0800, Manasi Navare wrote:
> > The detect_done flag was introduced in the commit
> > 7d23e3c37bb3fc6952dc84007ee60cb533fd2d5c in order to avoid
> > multiple detects on hotplug where intel_dp_long_pulse()
> > was called from HPD handler as well as in intel_dp_detect.
> > So this detect_done flag was required to make sure intel_dp_detect()
> > did not call long pulse handler again if it was already been called
> > from HPD handler. However commit 1015811609c0328b5ed670d07748591b837e74eb
> > differs the long hpd handling entirely until the hotplug work runs to
> > avoid the double long hpd handling the "detect_done" flag is trying
> > to prevent.
> 
> That sentence doesn't parse here. Anyways, the flag indeed is now a nop
> and your patch is pretty much the same what I did here:
> https://patchwork.freedesktop.org/patch/101476/
>

I looked at this patch and yes it is the same thing except that it keeps
the name of the flag unchanged plus previous patch is too old and wont
apply cleanly unless you rebase it. I will add the reseting of this flag in the
resume case. 
Thoughts?

> > 
> > So now we do not need to reset the detect_done flag to false in
> > intel_dp_detect. It will be reset in the intel_dp_hpd_pulse so
> > that intel_dp_detect does a full detect. However if the .detect
> > gets called during mode enumeration then we do not need to do a
> > full detect. This patch avoids the WARNS_ONS during connected boot
> > case when it calls intel_dp_check_link_status() due to multiple
> > detects
> 
> How exactly does it do that? Also we shouldn't sweep that under the rug
> anyway. Instead someone should actually fix the problem that causes the
> WARN.
>

Actually an additional call to intel_dp_long_pulse from intel_dp_detect()
because we are not resetting the detect_done flag, makes the function
call intel_dp_check_link_status() in the case when intel_dp->lane_count is 
0 and throws the WARN about that. So by resetting the flag we avoid this
unwanted call to intel_dp_long_pulse and that gets rid of the warning.

Manasi
 
> > and also avoids DP compliance failures. It avoids doing
> > a full detect every single time on .detect().
> > 
> > Cc: Ville Syrjala <ville.syrjala@linux.intel.com>
> > Cc: Daniel Vetter <daniel.vetter@ffwll.ch>
> > Cc: Jani Nikula <jani.nikula@linux.intel.com>
> > Signed-off-by: Manasi Navare <manasi.d.navare@intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_dp.c | 2 --
> >  1 file changed, 2 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> > index db75bb9..9c9277e 100644
> > --- a/drivers/gpu/drm/i915/intel_dp.c
> > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > @@ -4470,8 +4470,6 @@ static bool intel_digital_port_connected(struct drm_i915_private *dev_priv,
> >  	if (!intel_dp->detect_done)
> >  		status = intel_dp_long_pulse(intel_dp->attached_connector);
> >  
> > -	intel_dp->detect_done = false;
> > -
> >  	return status;
> >  }
> >  
> > -- 
> > 1.9.1
> 
> -- 
> Ville Syrjälä
> Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx