Re: [PATCH] drm/i915: Suspend resume timing optimization.

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

On Wed, Dec 09, 2015 at 04:25:41PM +0530, Kumar, Shobhit wrote:
> On 12/08/2015 02:22 AM, Paulo Zanoni wrote:
> > 2015-12-07 18:28 GMT-02:00  <abhay.kumar@intel.com>:
> >> From: Abhay Kumar <abhay.kumar@intel.com>
> >>
> >> Moving 250ms from T12 timing to suspend path so that
> >> resume path will be faster.
> >
> > Can you please elaborate more on your motivation for this patch? I'm a
> > little confused. You're trying to make resume faster by making suspend
> > slower? What are your main arguments for this?
> >
> 
> Actually the display resume time is currently roughly around ~700ms for 
> eDP. The panel power sequence as per spec needs to be minimum 510ms . So 
> the t11_12 time is 600ms in our code. Question is how to optimize this. 
> What Abhay has tried is to move the wait for panel_power_cycle_delay in 
> the suspend path rather then in resume path when we check if panel has 
> power. Since this is jiffied based we end up waiting while going down 
> and by the time we come back jiffies have already expired, giving us 
> optimization in resume path. Will this work. Is it violates the spec in 
> any way ? Any other suggestions ?

I'm thinking this should work. However, it seems unlikely we would
really need any wait during suspend or resume since the panel will
likely be off for a longer period anyway while the machine is
suspended. So I wonder if we could look at the wall clock instead
during suspend? Eg. sample it during susend and resume,
and adjust last_power_cycle jiffies based on the difference.

> 
> >>
> >> Signed-off-by: Abhay Kumar <abhay.kumar@intel.com>
> >> ---
> >>   drivers/gpu/drm/i915/intel_ddi.c | 6 ++++++
> >>   1 file changed, 6 insertions(+)
> >>
> >> diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
> >> index 7f618cf..2679c9e 100644
> >> --- a/drivers/gpu/drm/i915/intel_ddi.c
> >> +++ b/drivers/gpu/drm/i915/intel_ddi.c
> >> @@ -2389,6 +2389,12 @@ static void intel_ddi_post_disable(struct intel_encoder *intel_encoder)
> >
> > Funcion intel_ddi_post_disable() doesn't only run on suspend
> > situations, yet your commit message suggests you're optimizing
> > suspend. Maybe this commit makes non-suspend modesets slower because
> > now we need to wait the panel power cycle earlier? Have you measured
> > the possible downsides?
> 
> Yes this is a problem. I again looked at the spec and found that t7 can 
> be 50, but VBT was default programming 200ms which is overriding our 
> driver initialized value. Correcting VBT gives back 150ms here. But yes 
> this will impact non-suspend modeset paths. Perhaps we should figure out 
> a way to avoid this and do only when relevant. A flag based check would 
> work ? I know it sounds hackish.
> 
> >
> >>                  intel_dp_sink_dpms(intel_dp, DRM_MODE_DPMS_OFF);
> >>                  intel_edp_panel_vdd_on(intel_dp);
> >>                  intel_edp_panel_off(intel_dp);
> >> +
> >> +               /* Give additional delay of 250 ms so that resume time will
> >> +                  be faster and also meets T12 delay.
> >> +               */
> >
> > The comment says 250ms, but the code doesn't. Also, there's a missing
> > '*' char in the comment.
> >
> >> +               wait_remaining_ms_from_jiffies(intel_dp->last_power_cycle,
> >> +                                      (intel_dp->panel_power_cycle_delay/2));
> >
> > Why wait half the panel power cycle? Why did you choose exactly this value?
> >
> 
> Actually I would want to wait out full panel power cycle delay. Brings 
> our resume time down to ~250ms. But ultimately if this is acceptable 
> solution, will still depend on suspend KPIs and we might have to 
> balance. But as far as I know suspend path has no strict KPI, so we 
> might get away with this.
> 
> Regards
> Shobhit
> 
> > Thanks,
> > Paulo
> >
> >>          }
> >>
> >>          if (IS_SKYLAKE(dev) || IS_KABYLAKE(dev))
> >> --
> >> 1.9.1
> >>
> >> _______________________________________________
> >> 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

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