intel-gfx.lists.freedesktop.org archive mirror
 help / color / mirror / Atom feed
From: "Ville Syrjälä" <ville.syrjala@linux.intel.com>
To: Rodrigo Vivi <rodrigo.vivi@intel.com>
Cc: intel-gfx@lists.freedesktop.org
Subject: Re: [PATCH 2/3] drm/i915: Assume eDP is always connected
Date: Thu, 19 Jul 2018 13:11:15 +0300	[thread overview]
Message-ID: <20180719101115.GZ5565@intel.com> (raw)
In-Reply-To: <20180718210042.GD5545@intel.com>

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@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@intel.com>

Thanks.

> 
> > 
> > > 
> > > > 
> > > > Signed-off-by: Ville Syrjälä <ville.syrjala@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@lists.freedesktop.org
> > > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> > 
> > -- 
> > Ville Syrjälä
> > Intel
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

  reply	other threads:[~2018-07-19 10:11 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-07-17 17:42 [PATCH 1/3] drm/i915: Nuke the LVDS lid notifier Ville Syrjala
2018-07-17 17:42 ` [PATCH 2/3] drm/i915: Assume eDP is always connected Ville Syrjala
2018-07-17 20:00   ` Rodrigo Vivi
2018-07-18 10:03     ` Ville Syrjälä
2018-07-18 21:00       ` Rodrigo Vivi
2018-07-19 10:11         ` Ville Syrjälä [this message]
2018-07-17 17:42 ` [PATCH 3/3] drm/i915: Remove intel_panel_detect() Ville Syrjala
2018-07-17 20:02   ` Rodrigo Vivi
2018-07-17 18:23 ` ✗ Fi.CI.CHECKPATCH: warning for series starting with [1/3] drm/i915: Nuke the LVDS lid notifier Patchwork
2018-07-17 18:25 ` ✗ Fi.CI.SPARSE: " Patchwork
2018-07-17 18:47 ` ✓ Fi.CI.BAT: success " Patchwork
2018-07-17 19:57 ` [Intel-gfx] [PATCH 1/3] " Rodrigo Vivi
2018-07-18  0:34 ` ✓ Fi.CI.IGT: success for series starting with [1/3] " Patchwork

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20180719101115.GZ5565@intel.com \
    --to=ville.syrjala@linux.intel.com \
    --cc=intel-gfx@lists.freedesktop.org \
    --cc=rodrigo.vivi@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).