On 12/11/2015 04:55 PM, Thulasimani, Sivakumar wrote:
On 12/10/2015 8:32 PM, Ville Syrjälä wrote:
On Thu, Dec 10, 2015 at 08:09:01PM +0530, Thulasimani, Sivakumar wrote:
On 12/10/2015 7:08 PM, Ville Syrjälä wrote:
On Thu, Dec 10, 2015 at 03:15:37PM +0200, Ville Syrjälä wrote:
On Thu, Dec 10, 2015 at 03:01:02PM +0530, Kumar, Shobhit wrote:
On 12/09/2015 09:35 PM, Ville Syrjälä wrote:
On Wed, Dec 09, 2015 at 08:59:26PM +0530, Shobhit Kumar wrote:
On Wed, Dec 9, 2015 at 8:34 PM, Chris Wilson
<chris@xxxxxxxxxxxxxxxxxx> wrote:
On Wed, Dec 09, 2015 at 08:07:10PM +0530, Shobhit Kumar wrote:
On Wed, Dec 9, 2015 at 7:27 PM, Ville Syrjälä
<ville.syrjala@xxxxxxxxxxxxxxx> wrote:
On Wed, Dec 09, 2015 at 06:51:48PM +0530, Shobhit Kumar wrote:
During resume, while turning the EDP panel power on, we need
not wait
blindly for panel_power_cycle_delay. Check if panel power
down sequence
in progress and then only wait. This improves our resume
time significantly.
Signed-off-by: Shobhit Kumar <shobhit.kumar@xxxxxxxxx>
---
drivers/gpu/drm/i915/intel_dp.c | 17 ++++++++++++++++-
1 file changed, 16 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/intel_dp.c
b/drivers/gpu/drm/i915/intel_dp.c
index f335c92..10ec669 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -617,6 +617,20 @@ static bool edp_have_panel_power(struct
intel_dp *intel_dp)
return (I915_READ(_pp_stat_reg(intel_dp)) & PP_ON)
!= 0;
}
+static bool edp_panel_off_seq(struct intel_dp *intel_dp)
+{
+ struct drm_device *dev = intel_dp_to_dev(intel_dp);
+ struct drm_i915_private *dev_priv = dev->dev_private;
+
+ lockdep_assert_held(&dev_priv->pps_mutex);
+
+ if (IS_VALLEYVIEW(dev) &&
+ intel_dp->pps_pipe == INVALID_PIPE)
+ return false;
+
+ return (I915_READ(_pp_stat_reg(intel_dp)) &
PP_SEQUENCE_POWER_DOWN) != 0;
+}
This doens't make sense to me. The power down cycle may have
completed just before, and so this would claim we don't have to
wait for the power_cycle_delay.
Not sure I understand your concern correctly. You are right,
power
down cycle may have completed just before and if it has then
we don't
need to wait. But in case the power down cycle is in progress
as per
internal state, then we need to wait for it to complete. This
will
happen for example in non-suspend disable path and will be
handled
correctly. In case of actual suspend/resume, this would have
successfully completed and will skip the wait as it is not needed
before enabling panel power.
+
static bool edp_have_panel_vdd(struct intel_dp *intel_dp)
{
struct drm_device *dev = intel_dp_to_dev(intel_dp);
@@ -2025,7 +2039,8 @@ static void edp_panel_on(struct
intel_dp *intel_dp)
port_name(dp_to_dig_port(intel_dp)->port)))
return;
- wait_panel_power_cycle(intel_dp);
+ if (edp_panel_off_seq(intel_dp))
+ wait_panel_power_cycle(intel_dp);
Looking in from the side, I have no idea what this is meant to
do. At
the very least you need your explanatory paragraph here which
would
include what exactly you are waiting for at the start of
edp_panel_on
(and please try and find a better name for edp_panel_off_seq()).
I will add a comment. Basically I am not additionally waiting, but
converting the wait which was already there to a conditional
wait. The
edp_panel_off_seq, checks if panel power down sequence is in
progress.
In that case we need to wait for the panel power cycle delay. If
it is
not in that sequence, there is no need to wait. I will make an
attempt
again on the naming in next patch update.
As far I remeber you need to wait for power_cycle_delay between
power
down cycle and power up cycle. You're trying to throw that wait away
entirely, unless the function happens get called while the power
down
Yes you are right and I realize I made a mistake in my patch which is
not checking PP_CYCLE_DELAY_ACTIVE bit.
cycle is still in progress. We should already optimize away
redundant
waits by tracking the end of the power down cycle with the jiffies
tracking.
Actually looking at the code the power_cycle_delay gets counted from
the start of the last power down cycle, so supposedly it's always at
least as long as the power down cycle, and typically it's quite a
bit
longer that that. But that doesn't change the fact that you can't
just skip it because the power down cycle delay happened to end
already.
So what we do now is:
1. initiate power down cycle
2. last_power_cycle=jiffies
3. wait for power down (I suppose this actually waits
until the power down delay has passed since that's
programmes into the PPS).
4. wait for power_cycle_delay from last_power_cycle
5. initiate power up cycle
I think with your patch step 4 would always be skipped since the
power down cycle has already ended, and then we fail to honor the
power cycle delay.
Yes, I agree. I missed checking for PP_CYCLE_DELAY_ACTIVE. Adding
that
check will take care of this scenario I guess ?
Nope. The vdd force bit doesn't respect the PPS state machine, so we
must do the waits manually instead. And in theory your patch wouldn't
do anything anyway since the sleep already takes into account when the
power cycle delay started.
The fact that we use msleep() may actually make those sleeps somewhat
longer, and maybe we should also think about switching to
usleep_range()
here.
Oh, and we may want to stop using the power sequencer forced delay,
at least for the power up delay since we could then speed up the power
up time by waiting for the long HPD instead. Clint even sent a patch
for
that, but I don't think it actually change the PPS delays. But maybe
the PPS won't really kick in due to our use of the vdd force bit during
panel power up?
The bahaviour of the PPS delays vs. the vdd force bit isn't actually
documented, so it would be something that could be studied a bit by
banging on the hardware. Probably best done on a machine where there's
no local panel to avoid damaging it if things go bad.
i think i updated about using hpd/edid read instead of waiting entire T3
delay for power on
during IRC chat sometime back to danvet. in short it does not work
across
all panels, it causes blooming effect on some panels resulting in panel
failing
to turn on, which will also affect panel life.
:( I was hoping we could use it for eDP, but I guess being an optimist
when it comes to hardware never pays off.
I wonder if we could at least start doing some of the AUX communication
sooner. That is, maybe we can do at least the DP_LINK_BW_SET and the
DP_DOWNSPREAD_CTRL DPCD writes after getting the HPD, and only then
wait for the end of the power on delay just before starting the
link training? And the same for any other AUX chatter when the panel
is otherwise powered off?
:) two parts to optimization of such DPCD aux communications
1) they can change during modeset so we can never be sure if the value
will be final
2) those are too small operations to help in any major way for power on.
Some more costly/time consuming operations we can optimize to "boot to
edp" are
1) avoid VDD & PPS on/0ff. i.e totally avoid doing any modeset op,
since eDP usually
has one or two modes it is highly likely that we will apply the same mode
brought up by BIOS, so any effort on our side is just redoing stuff.
2) if the above is not possible, we can remove as much of VDD off/on
in our flow as possible.(each off will take the entire Power down delay
time
and the next on will take power on delay time)
3) our dpcd read/write logic should be optimized, our whole Link training
on CHT is taking ~15ms, which should have been completed in under 10ms.
if we implement point 1 above for boot/s4 resume we can bring down modeset
time for edp alone scenario to few ms.
i don't want to keep talking but not doing anything, will finish my
compliance
activities (including upstreaming) and then will try to target each of
the above. :)
Thanks Siva for iterating few optimization points which I have been also
thinking are needed. But lets take the over all optimization
possibilities in another thread. My question to you guys in the current
context is -
1. Shall we keep the panel_power_cycle_delay as is but adjusting jiffies
based on wall clock as Ville suggested. This will ensure that we skip
the wait in suspend/resume scenario, or
2. Have the HW PPS based tracking for delays and also take care that VDD
force on is not called before panel power cycle is completely over. If
all do not agree or see issues with this approach, we can keep this for
later when we optimize vdd on/off sequences as suggested by siva and go
for now with approach 1.
Regards
Shobhit
regards,
Sivakumar
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
