* [PATCH] drm/i915: eDP HPD connected check to reduce T3 time
@ 2015-09-22 17:09 clinton.a.taylor
2015-09-22 17:27 ` Ville Syrjälä
0 siblings, 1 reply; 4+ messages in thread
From: clinton.a.taylor @ 2015-09-22 17:09 UTC (permalink / raw)
To: Intel-gfx
From: Clint Taylor <clinton.a.taylor@intel.com>
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@intel.com>
---
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);
+ }
}
return need_to_disable;
--
1.7.9.5
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] drm/i915: eDP HPD connected check to reduce T3 time
2015-09-22 17:09 [PATCH] drm/i915: eDP HPD connected check to reduce T3 time clinton.a.taylor
@ 2015-09-22 17:27 ` Ville Syrjälä
2015-09-22 21:34 ` Clint Taylor
0 siblings, 1 reply; 4+ messages in thread
From: Ville Syrjälä @ 2015-09-22 17:27 UTC (permalink / raw)
To: clinton.a.taylor; +Cc: Intel-gfx
On Tue, Sep 22, 2015 at 10:09:39AM -0700, clinton.a.taylor@intel.com wrote:
> From: Clint Taylor <clinton.a.taylor@intel.com>
>
> 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@intel.com>
> ---
> 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.
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?
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?
> }
>
> return need_to_disable;
> --
> 1.7.9.5
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] drm/i915: eDP HPD connected check to reduce T3 time
2015-09-22 17:27 ` Ville Syrjälä
@ 2015-09-22 21:34 ` Clint Taylor
2015-09-23 11:26 ` Ville Syrjälä
0 siblings, 1 reply; 4+ messages in thread
From: Clint Taylor @ 2015-09-22 21:34 UTC (permalink / raw)
To: Ville Syrjälä; +Cc: Intel-gfx
On 09/22/2015 10:27 AM, Ville Syrjälä wrote:
> On Tue, Sep 22, 2015 at 10:09:39AM -0700, clinton.a.taylor@intel.com wrote:
>> From: Clint Taylor <clinton.a.taylor@intel.com>
>>
>> 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@intel.com>
>> ---
>> 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.
>
> 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.
> 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.
>
>> }
>>
>> return need_to_disable;
>> --
>> 1.7.9.5
>>
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx@lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] drm/i915: eDP HPD connected check to reduce T3 time
2015-09-22 21:34 ` Clint Taylor
@ 2015-09-23 11:26 ` Ville Syrjälä
0 siblings, 0 replies; 4+ messages in thread
From: Ville Syrjälä @ 2015-09-23 11:26 UTC (permalink / raw)
To: Clint Taylor; +Cc: Intel-gfx
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@intel.com wrote:
> >> From: Clint Taylor <clinton.a.taylor@intel.com>
> >>
> >> 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@intel.com>
> >> ---
> >> 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@lists.freedesktop.org
> >> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> >
--
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2015-09-23 11:26 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2015-09-22 17:09 [PATCH] drm/i915: eDP HPD connected check to reduce T3 time clinton.a.taylor
2015-09-22 17:27 ` Ville Syrjälä
2015-09-22 21:34 ` Clint Taylor
2015-09-23 11:26 ` Ville Syrjälä
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox