On Tue, Sep 22, 2015 at 02:34:02PM -0700, Clint Taylor wrote:
> On 09/22/2015 10:27 AM, Ville Syrjälä wrote:
> > On Tue, Sep 22, 2015 at 10:09:39AM -0700, clinton.a.taylor@xxxxxxxxx wrote:
> >> From: Clint Taylor <clinton.a.taylor@xxxxxxxxx>
> >>
> >> To reduce eDP T3 time check for digital port connected instead of
> >> msleep. Maintain VBT time if HPD is not asserted on the port.
> >>
> >> Current eDP T3 time is an msleep for the panel_power_up time specified
> >> in VBT. The eDP specification allows maximum T3 time of 200ms.
> >> Typically panels raise HPD from 70ms-105ms and are ready for AUX traffic
> >> and training. Reading HPD will reduce power-on and resume times by over
> >> 100ms on systems with eDP HPD connected.
> >>
> >> Signed-off-by: Clint Taylor <clinton.a.taylor@xxxxxxxxx>
> >> ---
> >> drivers/gpu/drm/i915/intel_dp.c | 13 ++++++++++++-
> >> 1 file changed, 12 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> >> index 77e4115..7caf3ab 100644
> >> --- a/drivers/gpu/drm/i915/intel_dp.c
> >> +++ b/drivers/gpu/drm/i915/intel_dp.c
> >> @@ -129,6 +129,8 @@ static void edp_panel_vdd_off(struct intel_dp *intel_dp, bool sync);
> >> static void vlv_init_panel_power_sequencer(struct intel_dp *intel_dp);
> >> static void vlv_steal_power_sequencer(struct drm_device *dev,
> >> enum pipe pipe);
> >> +static bool intel_digital_port_connected(struct drm_i915_private *dev_priv,
> >> + struct intel_digital_port *port);
> >>
> >> static unsigned int intel_dp_unused_lane_mask(int lane_count)
> >> {
> >> @@ -1772,6 +1774,7 @@ static bool edp_panel_vdd_on(struct intel_dp *intel_dp)
> >> u32 pp;
> >> u32 pp_stat_reg, pp_ctrl_reg;
> >> bool need_to_disable = !intel_dp->want_panel_vdd;
> >> + int i, step = 0;
> >>
> >> lockdep_assert_held(&dev_priv->pps_mutex);
> >>
> >> @@ -1809,7 +1812,15 @@ static bool edp_panel_vdd_on(struct intel_dp *intel_dp)
> >> if (!edp_have_panel_power(intel_dp)) {
> >> DRM_DEBUG_KMS("eDP port %c panel power wasn't enabled\n",
> >> port_name(intel_dig_port->port));
> >> - msleep(intel_dp->panel_power_up_delay);
> >> + step = intel_dp->panel_power_up_delay / 10;
> >> + for (i=0; i < intel_dp->panel_power_up_delay; i+=step) {
> >> + if (intel_digital_port_connected(dev_priv, intel_dig_port)) {
> >> + DRM_DEBUG_KMS("Port %c HPD detected\n",
> >> + port_name(intel_dig_port->port));
> >> + break;
> >> + }
> >> + msleep(10);
> >> + }
> >
> > We have the HPD irq wired up for all ports now, so we could even do
> > this without polling.
>
> I thought about using the HPD irq though I really didn't want to get
> into full HPD handling of the eDP port since only the rise of HPD is of
> interest.
Well, since we have it all wired up already, should be mostly a matter
of adding a waitqueue, wake it up from hpd_pulse, and use
wait_event_timeout() to wait for it.
> > There is a slight concern that what if the HPD line is noisy (happened
> > on BSW RVP due wrong pullup settings at least) we could start link
> > training before the panel is powered up. But I have no good solution for
> > that, other than maybe blacklisting bad machines, which would require
> > that we detect them somehow in the first place. Maybe we should have
> > some kind of check in case the HPD is raised suspiciously soon, we would
> > fall back to the full msleep method?
> >
>
> If for some reason the HPD line asserts early we could always retry AUX
> transactions until we get a good ACK before starting link training. VBT
> has a field for "T3 optimization" that when enabled just polls the AUX
> until the panel becomes ready instead of waiting for T3 time. retrying
> AUX on a spurious HPD would be the equivalent. We would need a sane way
> to give up after the expected T3 time.
Hmm. Yeah perhaps just making really sure AUX works before we continue
would be a good way.
> > Another thing I didn't think about fully is how the power sequencer fits
> > into this sort of thing. It seems we always enable the vdd force override
> > before starting the panel on sequence, so i suppose we should be able
> > to program the panel power on delay to 0 (or whatever the min is), and
> > just rely on the driver to do the delays?
> >
> The power panel sequencer is actually causing additional delays during
> coldboot and resume. The current optimization in the driver for
> power_panel_cycle T12 delay is not being realized as we still wait for
> the panel power sequencer to complete its cycle before training the
> panel. That is my next level of optimization that I was going to work on
> next. My initial prototypes are causing spikes in the output voltage
> which could damage the panel. If the power on delays were 0 that may
> help. Thanks for the idea.
OK.
>
> >
> >> }
> >>
> >> return need_to_disable;
> >> --
> >> 1.7.9.5
> >>
> >> _______________________________________________
> >> Intel-gfx mailing list
> >> Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
> >> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> >
--
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
