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

["Kumar, Abhay" <abhay.kumar@intel.com>]

[-- Attachment #1.1.1: Type: text/plain, Size: 2991 bytes --]

This is for Chrome os perspective which is using upstream kernel.  We have S3 suspend and resume time limit of 1sec.



Currently suspend of i915 driver takes 280ms and resume takes 600ms. To make resume faster and also follow edp spec this patch will move

Half of t12 time from resume path to suspend path.



Will take care of comment in next patchset.

[cid:image001.png@01D130F9.DFFC0300]







-----Original Message-----
From: Paulo Zanoni [mailto:przanoni@gmail.com]
Sent: Monday, December 7, 2015 12:52 PM
To: Kumar, Abhay
Cc: Intel Graphics Development
Subject: Re: [Intel-gfx] [PATCH] drm/i915: Suspend resume timing optimization.



2015-12-07 18:28 GMT-02:00  <abhay.kumar@intel.com<mailto:abhay.kumar@intel.com>>:

> From: Abhay Kumar <abhay.kumar@intel.com<mailto: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?



>

> Signed-off-by: Abhay Kumar <abhay.kumar@intel.com<mailto: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?



>                 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?



Thanks,

Paulo



>         }

>

>         if (IS_SKYLAKE(dev) || IS_KABYLAKE(dev))

> --

> 1.9.1

>

> _______________________________________________

> Intel-gfx mailing list

> Intel-gfx@lists.freedesktop.org<mailto:Intel-gfx@lists.freedesktop.org>

> http://lists.freedesktop.org/mailman/listinfo/intel-gfx







--

Paulo Zanoni

[-- Attachment #1.1.2: Type: text/html, Size: 9467 bytes --]

[-- Attachment #1.2: image001.png --]
[-- Type: image/png, Size: 53549 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx