[Intel-gfx] [PATCH 2/3] drm/i915: Assume eDP is always connected

[Ville Syrjälä]

On Wed, Jul 18, 2018 at 02:00:42PM -0700, Rodrigo Vivi wrote:
> On Wed, Jul 18, 2018 at 01:03:59PM +0300, Ville Syrjälä wrote:
> > On Tue, Jul 17, 2018 at 01:00:25PM -0700, Rodrigo Vivi wrote:
> > > On Tue, Jul 17, 2018 at 08:42:15PM +0300, Ville Syrjala wrote:
> > > > From: Ville Syrjälä <ville.syrjala at linux.intel.com>
> > > > 
> > > > We never registered any kind of lid notifier for eDP, so looking at the
> > > > lid status is pretty much bonkers. Let's just consider eDP always
> > > > connected instead.
> > > 
> > > hm....
> > > I've seen in the past, specially on rvps, so many times that
> > > we detect edp or assume edp is there and it is not...
> > > that generates a flood of aux timeout reads...
> > > 
> > > I never investigate that, but I wonder if this change here
> > > couldn't make that case even more common... 
> > 
> > I don't know if RVPs would have any of the lid stuff implemented.
> > I somewhat doubt it.
> 
> I think they do... and with some hard toggles buttons,

Oh right, some do. I think I have one SKL with those switches on the
front. Don't think I've seen those buttons on any Atom RVPs though.

> 
> but anyways I believe we might safe,
> otherwise our CI would have complained about it right?

Not sure. Hmm. Actually none of this should matter for any machine that
doesn't actually have eDP. We don't check this stuff when we initialize
the encoder/connector so we'd still get the same amount of DPCD fail
as before.

> 
> > 
> > If dpcd reads fail we won't register edp so any timeouts and whatnot
> > should only be logged during driver load.
> 
> yeap, but I'm afraid that without any kind of check we would
> start seeing those aux failures everywhere... even on regular desktop
> units where we naver saw that kind of failure would start to having logs
> poluted.
> 
> But if CI is happy and you checked that this is not poluting the logs
> of platforms without eDP then the patch looks correct:
> 
> Reviewed-by: Rodrigo Vivi <rodrigo.vivi at intel.com>

Thanks.

> 
> > 
> > > 
> > > > 
> > > > Signed-off-by: Ville Syrjälä <ville.syrjala at linux.intel.com>
> > > > ---
> > > >  drivers/gpu/drm/i915/intel_dp.c | 11 ++---------
> > > >  1 file changed, 2 insertions(+), 9 deletions(-)
> > > > 
> > > > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> > > > index b45b08420c1f..61657b8b109c 100644
> > > > --- a/drivers/gpu/drm/i915/intel_dp.c
> > > > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > > > @@ -4532,14 +4532,7 @@ intel_dp_detect_dpcd(struct intel_dp *intel_dp)
> > > >  static enum drm_connector_status
> > > >  edp_detect(struct intel_dp *intel_dp)
> > > >  {
> > > > -	struct drm_i915_private *dev_priv = to_i915(intel_dp_to_dev(intel_dp));
> > > > -	enum drm_connector_status status;
> > > > -
> > > > -	status = intel_panel_detect(dev_priv);
> > > > -	if (status == connector_status_unknown)
> > > > -		status = connector_status_connected;
> > > > -
> > > > -	return status;
> > > > +	return connector_status_connected;
> > > >  }
> > > >  
> > > >  static bool ibx_digital_port_connected(struct intel_encoder *encoder)
> > > > @@ -4802,7 +4795,7 @@ intel_dp_long_pulse(struct intel_connector *connector)
> > > >  
> > > >  	intel_display_power_get(dev_priv, intel_dp->aux_power_domain);
> > > >  
> > > > -	/* Can't disconnect eDP, but you can close the lid... */
> > > > +	/* Can't disconnect eDP */
> > > >  	if (intel_dp_is_edp(intel_dp))
> > > >  		status = edp_detect(intel_dp);
> > > >  	else if (intel_digital_port_connected(&dp_to_dig_port(intel_dp)->base))
> > > > -- 
> > > > 2.16.4
> > > > 
> > > > _______________________________________________
> > > > Intel-gfx mailing list
> > > > Intel-gfx at lists.freedesktop.org
> > > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> > 
> > -- 
> > Ville Syrjälä
> > Intel
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx at lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel