[PATCH] drm/i915: edp resume/On time optimization.

[PATCH] drm/i915: edp resume/On time optimization.

[abhay.kumar]

From: Abhay Kumar <abhay.kumar@intel.com>

Make resume/on codepath not to wait for panel_power_cycle_delay(t11_t12)
if this time is already spent in suspend/poweron time.

v2: Use CLOCK_BOOTTIME and remove jiffies for panel power cycle
    delay calculation(Ville).

v3: Addressing Ville review comment.

Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Abhay Kumar <abhay.kumar@intel.com>
---
 drivers/gpu/drm/i915/intel_dp.c  | 19 ++++++++++++++-----
 drivers/gpu/drm/i915/intel_drv.h |  2 +-
 2 files changed, 15 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 796e3d3..d0885bc 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -1812,12 +1812,21 @@ static void wait_panel_off(struct intel_dp *intel_dp)
 
 static void wait_panel_power_cycle(struct intel_dp *intel_dp)
 {
+	static ktime_t panel_power_on_time;
+	s64 panel_power_off_duration;
+
 	DRM_DEBUG_KMS("Wait for panel power cycle\n");
 
+	/* take the difference of currrent time and panel power off time
+	 * and then make panel wait for t11_t12 if needed. */
+	panel_power_on_time = ktime_get_boottime();
+	panel_power_off_duration = ktime_ms_delta(panel_power_on_time, intel_dp->panel_power_off_time);
+
 	/* When we disable the VDD override bit last we have to do the manual
 	 * wait. */
-	wait_remaining_ms_from_jiffies(intel_dp->last_power_cycle,
-				       intel_dp->panel_power_cycle_delay);
+	if (panel_power_off_duration < ((s64) intel_dp->panel_power_cycle_delay))
+		wait_remaining_ms_from_jiffies(jiffies,
+				       (intel_dp->panel_power_cycle_delay - panel_power_off_duration));
 
 	wait_panel_status(intel_dp, IDLE_CYCLE_MASK, IDLE_CYCLE_VALUE);
 }
@@ -1969,7 +1978,7 @@ static void edp_panel_vdd_off_sync(struct intel_dp *intel_dp)
 	I915_READ(pp_stat_reg), I915_READ(pp_ctrl_reg));
 
 	if ((pp & POWER_TARGET_ON) == 0)
-		intel_dp->last_power_cycle = jiffies;
+		intel_dp->panel_power_off_time = ktime_get_boottime();
 
 	power_domain = intel_display_port_aux_power_domain(intel_encoder);
 	intel_display_power_put(dev_priv, power_domain);
@@ -2118,7 +2127,7 @@ static void edp_panel_off(struct intel_dp *intel_dp)
 	I915_WRITE(pp_ctrl_reg, pp);
 	POSTING_READ(pp_ctrl_reg);
 
-	intel_dp->last_power_cycle = jiffies;
+	intel_dp->panel_power_off_time = ktime_get_boottime();
 	wait_panel_off(intel_dp);
 
 	/* We got a reference when we enabled the VDD. */
@@ -5122,7 +5131,7 @@ intel_dp_add_properties(struct intel_dp *intel_dp, struct drm_connector *connect
 
 static void intel_dp_init_panel_power_timestamps(struct intel_dp *intel_dp)
 {
-	intel_dp->last_power_cycle = jiffies;
+	intel_dp->panel_power_off_time = ktime_get_boottime();
 	intel_dp->last_power_on = jiffies;
 	intel_dp->last_backlight_off = jiffies;
 }
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index bdfe403..06b37b8 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -793,9 +793,9 @@ struct intel_dp {
 	int backlight_off_delay;
 	struct delayed_work panel_vdd_work;
 	bool want_panel_vdd;
-	unsigned long last_power_cycle;
 	unsigned long last_power_on;
 	unsigned long last_backlight_off;
+	ktime_t panel_power_off_time;
 
 	struct notifier_block edp_notifier;
 
-- 
1.9.1

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

✗ warning: Fi.CI.BAT

[Patchwork]

== Summary ==

Built on a90796840c30dac6d9907439bf98d1d08046c49d drm-intel-nightly: 2016y-01m-11d-17h-22m-54s UTC integration manifest

Test kms_pipe_crc_basic:
        Subgroup read-crc-pipe-b-frame-sequence:
                pass       -> DMESG-WARN (byt-nuc)
Test pm_rpm:
        Subgroup basic-rte:
                dmesg-warn -> PASS       (byt-nuc) UNSTABLE

bdw-nuci7        total:138  pass:129  dwarn:0   dfail:0   fail:0   skip:9  
bdw-ultra        total:138  pass:132  dwarn:0   dfail:0   fail:0   skip:6  
bsw-nuc-2        total:141  pass:115  dwarn:2   dfail:0   fail:0   skip:24 
byt-nuc          total:141  pass:123  dwarn:3   dfail:0   fail:0   skip:15 
hsw-brixbox      total:141  pass:134  dwarn:0   dfail:0   fail:0   skip:7  
hsw-gt2          total:141  pass:137  dwarn:0   dfail:0   fail:0   skip:4  
ilk-hp8440p      total:141  pass:101  dwarn:3   dfail:0   fail:0   skip:37 
ivb-t430s        total:135  pass:122  dwarn:3   dfail:4   fail:0   skip:6  
skl-i5k-2        total:141  pass:132  dwarn:1   dfail:0   fail:0   skip:8  
skl-i7k-2        total:141  pass:132  dwarn:1   dfail:0   fail:0   skip:8  
snb-dellxps      total:141  pass:122  dwarn:5   dfail:0   fail:0   skip:14 
snb-x220t        total:141  pass:122  dwarn:5   dfail:0   fail:1   skip:13 

Results at /archive/results/CI_IGT_test/Patchwork_1141/

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

Re: [PATCH] drm/i915: edp resume/On time optimization.

[Ville Syrjälä]

On Mon, Jan 11, 2016 at 02:55:37PM -0800, abhay.kumar@intel.com wrote:
> From: Abhay Kumar <abhay.kumar@intel.com>
> 
> Make resume/on codepath not to wait for panel_power_cycle_delay(t11_t12)
> if this time is already spent in suspend/poweron time.
> 
> v2: Use CLOCK_BOOTTIME and remove jiffies for panel power cycle
>     delay calculation(Ville).
> 
> v3: Addressing Ville review comment.

That's not a very good changelog.

> 
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Signed-off-by: Abhay Kumar <abhay.kumar@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_dp.c  | 19 ++++++++++++++-----
>  drivers/gpu/drm/i915/intel_drv.h |  2 +-
>  2 files changed, 15 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 796e3d3..d0885bc 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -1812,12 +1812,21 @@ static void wait_panel_off(struct intel_dp *intel_dp)
>  
>  static void wait_panel_power_cycle(struct intel_dp *intel_dp)
>  {
> +	static ktime_t panel_power_on_time;
> +	s64 panel_power_off_duration;
> +
>  	DRM_DEBUG_KMS("Wait for panel power cycle\n");
>  
> +	/* take the difference of currrent time and panel power off time
> +	 * and then make panel wait for t11_t12 if needed. */
> +	panel_power_on_time = ktime_get_boottime();
> +	panel_power_off_duration = ktime_ms_delta(panel_power_on_time, intel_dp->panel_power_off_time);
> +
>  	/* When we disable the VDD override bit last we have to do the manual
>  	 * wait. */
> -	wait_remaining_ms_from_jiffies(intel_dp->last_power_cycle,
> -				       intel_dp->panel_power_cycle_delay);
> +	if (panel_power_off_duration < ((s64) intel_dp->panel_power_cycle_delay))

Some usless parens here.

> +		wait_remaining_ms_from_jiffies(jiffies,
> +				       (intel_dp->panel_power_cycle_delay - panel_power_off_duration));

ditto

Otherwise it looks like it should do what we want, so with the minor
bikesheds sorted this is
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

>  
>  	wait_panel_status(intel_dp, IDLE_CYCLE_MASK, IDLE_CYCLE_VALUE);
>  }
> @@ -1969,7 +1978,7 @@ static void edp_panel_vdd_off_sync(struct intel_dp *intel_dp)
>  	I915_READ(pp_stat_reg), I915_READ(pp_ctrl_reg));
>  
>  	if ((pp & POWER_TARGET_ON) == 0)
> -		intel_dp->last_power_cycle = jiffies;
> +		intel_dp->panel_power_off_time = ktime_get_boottime();
>  
>  	power_domain = intel_display_port_aux_power_domain(intel_encoder);
>  	intel_display_power_put(dev_priv, power_domain);
> @@ -2118,7 +2127,7 @@ static void edp_panel_off(struct intel_dp *intel_dp)
>  	I915_WRITE(pp_ctrl_reg, pp);
>  	POSTING_READ(pp_ctrl_reg);
>  
> -	intel_dp->last_power_cycle = jiffies;
> +	intel_dp->panel_power_off_time = ktime_get_boottime();
>  	wait_panel_off(intel_dp);
>  
>  	/* We got a reference when we enabled the VDD. */
> @@ -5122,7 +5131,7 @@ intel_dp_add_properties(struct intel_dp *intel_dp, struct drm_connector *connect
>  
>  static void intel_dp_init_panel_power_timestamps(struct intel_dp *intel_dp)
>  {
> -	intel_dp->last_power_cycle = jiffies;
> +	intel_dp->panel_power_off_time = ktime_get_boottime();
>  	intel_dp->last_power_on = jiffies;
>  	intel_dp->last_backlight_off = jiffies;
>  }
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index bdfe403..06b37b8 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -793,9 +793,9 @@ struct intel_dp {
>  	int backlight_off_delay;
>  	struct delayed_work panel_vdd_work;
>  	bool want_panel_vdd;
> -	unsigned long last_power_cycle;
>  	unsigned long last_power_on;
>  	unsigned long last_backlight_off;
> +	ktime_t panel_power_off_time;
>  
>  	struct notifier_block edp_notifier;
>  
> -- 
> 1.9.1

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

[PATCH] drm/i915: edp resume/On time optimization.

[abhay.kumar]

From: Abhay Kumar <abhay.kumar@intel.com>

Make resume/on codepath not to wait for panel_power_cycle_delay(t11_t12)
if this time is already spent in suspend/poweron time.

v2: Use CLOCK_BOOTTIME and remove jiffies for panel power cycle
    delay calculation(Ville).

Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Abhay Kumar <abhay.kumar@intel.com>
---
 drivers/gpu/drm/i915/intel_ddi.c |  3 +++
 drivers/gpu/drm/i915/intel_dp.c  | 22 ++++++++++++++--------
 drivers/gpu/drm/i915/intel_drv.h |  2 +-
 3 files changed, 18 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index e6408e5..480697d 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -2395,6 +2395,9 @@ static void intel_ddi_post_disable(struct intel_encoder *intel_encoder)
 		intel_dp_sink_dpms(intel_dp, DRM_MODE_DPMS_OFF);
 		intel_edp_panel_vdd_on(intel_dp);
 		intel_edp_panel_off(intel_dp);
+
+		/* storing panel power off time */
+		intel_dp->panel_power_off_time = ktime_get_with_offset(TK_OFFS_BOOT);
 	}
 
 	if (IS_SKYLAKE(dev) || IS_KABYLAKE(dev))
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 796e3d3..c813605 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -38,7 +38,6 @@
 #include "intel_drv.h"
 #include <drm/i915_drm.h>
 #include "i915_drv.h"
-
 #define DP_LINK_CHECK_TIMEOUT	(10 * 1000)
 
 /* Compliance test status bits  */
@@ -1812,13 +1811,22 @@ static void wait_panel_off(struct intel_dp *intel_dp)
 
 static void wait_panel_power_cycle(struct intel_dp *intel_dp)
 {
+	ktime_t panel_power_on_time;
+	u32 panel_power_off_duration;
+
 	DRM_DEBUG_KMS("Wait for panel power cycle\n");
 
-	/* When we disable the VDD override bit last we have to do the manual
-	 * wait. */
-	wait_remaining_ms_from_jiffies(intel_dp->last_power_cycle,
-				       intel_dp->panel_power_cycle_delay);
+        /* take the diffrence of currrent time and panel power off time
+           and then make panel wait for t11_t12 if needed */
+	panel_power_on_time = ktime_get_with_offset(TK_OFFS_BOOT);
+	panel_power_off_duration = (panel_power_on_time.tv64 - intel_dp->panel_power_off_time.tv64);
+	panel_power_off_duration = panel_power_off_duration / 1000000;
 
+	/* When we disable the VDD override bit last we have to do the manual
+	 * wait */
+	if (panel_power_off_duration < intel_dp->panel_power_cycle_delay)
+		wait_remaining_ms_from_jiffies(jiffies,
+				       (intel_dp->panel_power_cycle_delay - panel_power_off_duration));
 	wait_panel_status(intel_dp, IDLE_CYCLE_MASK, IDLE_CYCLE_VALUE);
 }
 
@@ -1969,7 +1977,7 @@ static void edp_panel_vdd_off_sync(struct intel_dp *intel_dp)
 	I915_READ(pp_stat_reg), I915_READ(pp_ctrl_reg));
 
 	if ((pp & POWER_TARGET_ON) == 0)
-		intel_dp->last_power_cycle = jiffies;
+		intel_dp->panel_power_off_time = ktime_get_with_offset(TK_OFFS_BOOT);
 
 	power_domain = intel_display_port_aux_power_domain(intel_encoder);
 	intel_display_power_put(dev_priv, power_domain);
@@ -2118,7 +2126,6 @@ static void edp_panel_off(struct intel_dp *intel_dp)
 	I915_WRITE(pp_ctrl_reg, pp);
 	POSTING_READ(pp_ctrl_reg);
 
-	intel_dp->last_power_cycle = jiffies;
 	wait_panel_off(intel_dp);
 
 	/* We got a reference when we enabled the VDD. */
@@ -5122,7 +5129,6 @@ intel_dp_add_properties(struct intel_dp *intel_dp, struct drm_connector *connect
 
 static void intel_dp_init_panel_power_timestamps(struct intel_dp *intel_dp)
 {
-	intel_dp->last_power_cycle = jiffies;
 	intel_dp->last_power_on = jiffies;
 	intel_dp->last_backlight_off = jiffies;
 }
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index d523ebb..84ad134 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -765,9 +765,9 @@ struct intel_dp {
 	int backlight_off_delay;
 	struct delayed_work panel_vdd_work;
 	bool want_panel_vdd;
-	unsigned long last_power_cycle;
 	unsigned long last_power_on;
 	unsigned long last_backlight_off;
+	ktime_t panel_power_off_time;
 
 	struct notifier_block edp_notifier;
 
-- 
1.9.1

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

Re: [PATCH] drm/i915: edp resume/On time optimization.

[Kumar, Abhay]

Make resume/on codepath not to wait for panel_power_cycle_delay(t11_t12) if this time is already spent in suspend/poweron time.

v2: Use CLOCK_BOOTTIME and remove jiffies for panel power cycle
    delay calculation(Ville).

Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
Signed-off-by: Abhay Kumar <abhay.kumar@intel.com>
---
 drivers/gpu/drm/i915/intel_ddi.c |  3 +++  drivers/gpu/drm/i915/intel_dp.c  | 22 ++++++++++++++--------  drivers/gpu/drm/i915/intel_drv.h |  2 +-
 3 files changed, 18 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index e6408e5..480697d 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -2395,6 +2395,9 @@ static void intel_ddi_post_disable(struct intel_encoder *intel_encoder)
 		intel_dp_sink_dpms(intel_dp, DRM_MODE_DPMS_OFF);
 		intel_edp_panel_vdd_on(intel_dp);
 		intel_edp_panel_off(intel_dp);
+
+		/* storing panel power off time */
+		intel_dp->panel_power_off_time = ktime_get_with_offset(TK_OFFS_BOOT);
 	}
 
 	if (IS_SKYLAKE(dev) || IS_KABYLAKE(dev)) diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c index 796e3d3..c813605 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -38,7 +38,6 @@
 #include "intel_drv.h"
 #include <drm/i915_drm.h>
 #include "i915_drv.h"
-
 #define DP_LINK_CHECK_TIMEOUT	(10 * 1000)
 
 /* Compliance test status bits  */
@@ -1812,13 +1811,22 @@ static void wait_panel_off(struct intel_dp *intel_dp)
 
 static void wait_panel_power_cycle(struct intel_dp *intel_dp)  {
+	ktime_t panel_power_on_time;
+	u32 panel_power_off_duration;
+
 	DRM_DEBUG_KMS("Wait for panel power cycle\n");
 
-	/* When we disable the VDD override bit last we have to do the manual
-	 * wait. */
-	wait_remaining_ms_from_jiffies(intel_dp->last_power_cycle,
-				       intel_dp->panel_power_cycle_delay);
+        /* take the diffrence of currrent time and panel power off time
+           and then make panel wait for t11_t12 if needed */
+	panel_power_on_time = ktime_get_with_offset(TK_OFFS_BOOT);
+	panel_power_off_duration = (panel_power_on_time.tv64 - intel_dp->panel_power_off_time.tv64);
+	panel_power_off_duration = panel_power_off_duration / 1000000;
 
+	/* When we disable the VDD override bit last we have to do the manual
+	 * wait */
+	if (panel_power_off_duration < intel_dp->panel_power_cycle_delay)
+		wait_remaining_ms_from_jiffies(jiffies,
+				       (intel_dp->panel_power_cycle_delay - 
+panel_power_off_duration));
 	wait_panel_status(intel_dp, IDLE_CYCLE_MASK, IDLE_CYCLE_VALUE);  }
 
@@ -1969,7 +1977,7 @@ static void edp_panel_vdd_off_sync(struct intel_dp *intel_dp)
 	I915_READ(pp_stat_reg), I915_READ(pp_ctrl_reg));
 
 	if ((pp & POWER_TARGET_ON) == 0)
-		intel_dp->last_power_cycle = jiffies;
+		intel_dp->panel_power_off_time = ktime_get_with_offset(TK_OFFS_BOOT);
 
 	power_domain = intel_display_port_aux_power_domain(intel_encoder);
 	intel_display_power_put(dev_priv, power_domain); @@ -2118,7 +2126,6 @@ static void edp_panel_off(struct intel_dp *intel_dp)
 	I915_WRITE(pp_ctrl_reg, pp);
 	POSTING_READ(pp_ctrl_reg);
 
-	intel_dp->last_power_cycle = jiffies;
 	wait_panel_off(intel_dp);
 
 	/* We got a reference when we enabled the VDD. */ @@ -5122,7 +5129,6 @@ intel_dp_add_properties(struct intel_dp *intel_dp, struct drm_connector *connect
 
 static void intel_dp_init_panel_power_timestamps(struct intel_dp *intel_dp)  {
-	intel_dp->last_power_cycle = jiffies;
 	intel_dp->last_power_on = jiffies;
 	intel_dp->last_backlight_off = jiffies;  } diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index d523ebb..84ad134 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -765,9 +765,9 @@ struct intel_dp {
 	int backlight_off_delay;
 	struct delayed_work panel_vdd_work;
 	bool want_panel_vdd;
-	unsigned long last_power_cycle;
 	unsigned long last_power_on;
 	unsigned long last_backlight_off;
+	ktime_t panel_power_off_time;
 
 	struct notifier_block edp_notifier;


Ville,
 
 Is this patch is coming close to what you wanted?

Regards,
Abhay Kumar

 
--
1.9.1

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

Re: [PATCH] drm/i915: edp resume/On time optimization.

[Daniel Vetter]

On Tue, Jan 05, 2016 at 01:30:53AM +0000, Kumar, Abhay wrote:
> Ville,
>  
>  Is this patch is coming close to what you wanted?

Please don't bottom-post but not quote properly - no one will ever find
your comment and assume you accidentally sent out the patch twice. If you
have to use a broken mailer that can't quote, then top-post. But please
just install something that does work (I personally use Thunderbird on
Windows, it getst the job done). Anything else just doesn't work.

Thanks, Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915: edp resume/On time optimization.

[Kumar, Abhay]

On 1/5/2016 3:04 AM, Daniel Vetter wrote:
> On Tue, Jan 05, 2016 at 01:30:53AM +0000, Kumar, Abhay wrote:
>> Ville,
>>   
>>   Is this patch is coming close to what you wanted?
> Please don't bottom-post but not quote properly - no one will ever find
> your comment and assume you accidentally sent out the patch twice. If you
> have to use a broken mailer that can't quote, then top-post. But please
> just install something that does work (I personally use Thunderbird on
> Windows, it getst the job done). Anything else just doesn't work.
>
> Thanks, Daniel
Thanks Daniel. I used wrong email client. Will take care of this from 
next time for bottom post.

Regards,
Abhay Kumar

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

Re: [PATCH] drm/i915: edp resume/On time optimization.

[Kumar, Abhay]

On 12/21/2015 5:18 PM, Kumar, Abhay wrote:
> From: Abhay Kumar <abhay.kumar@intel.com>
>
> Make resume/on codepath not to wait for panel_power_cycle_delay(t11_t12)
> if this time is already spent in suspend/poweron time.
>
> v2: Use CLOCK_BOOTTIME and remove jiffies for panel power cycle
>      delay calculation(Ville).
>
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Signed-off-by: Abhay Kumar <abhay.kumar@intel.com>
> ---
>   drivers/gpu/drm/i915/intel_ddi.c |  3 +++
>   drivers/gpu/drm/i915/intel_dp.c  | 22 ++++++++++++++--------
>   drivers/gpu/drm/i915/intel_drv.h |  2 +-
>   3 files changed, 18 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
> index e6408e5..480697d 100644
> --- a/drivers/gpu/drm/i915/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/intel_ddi.c
> @@ -2395,6 +2395,9 @@ static void intel_ddi_post_disable(struct intel_encoder *intel_encoder)
>   		intel_dp_sink_dpms(intel_dp, DRM_MODE_DPMS_OFF);
>   		intel_edp_panel_vdd_on(intel_dp);
>   		intel_edp_panel_off(intel_dp);
> +
> +		/* storing panel power off time */
> +		intel_dp->panel_power_off_time = ktime_get_with_offset(TK_OFFS_BOOT);
>   	}
>   
>   	if (IS_SKYLAKE(dev) || IS_KABYLAKE(dev))
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 796e3d3..c813605 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -38,7 +38,6 @@
>   #include "intel_drv.h"
>   #include <drm/i915_drm.h>
>   #include "i915_drv.h"
> -
>   #define DP_LINK_CHECK_TIMEOUT	(10 * 1000)
>   
>   /* Compliance test status bits  */
> @@ -1812,13 +1811,22 @@ static void wait_panel_off(struct intel_dp *intel_dp)
>   
>   static void wait_panel_power_cycle(struct intel_dp *intel_dp)
>   {
> +	ktime_t panel_power_on_time;
> +	u32 panel_power_off_duration;
> +
>   	DRM_DEBUG_KMS("Wait for panel power cycle\n");
>   
> -	/* When we disable the VDD override bit last we have to do the manual
> -	 * wait. */
> -	wait_remaining_ms_from_jiffies(intel_dp->last_power_cycle,
> -				       intel_dp->panel_power_cycle_delay);
> +        /* take the diffrence of currrent time and panel power off time
> +           and then make panel wait for t11_t12 if needed */
> +	panel_power_on_time = ktime_get_with_offset(TK_OFFS_BOOT);
> +	panel_power_off_duration = (panel_power_on_time.tv64 - intel_dp->panel_power_off_time.tv64);
> +	panel_power_off_duration = panel_power_off_duration / 1000000;
>   
> +	/* When we disable the VDD override bit last we have to do the manual
> +	 * wait */
> +	if (panel_power_off_duration < intel_dp->panel_power_cycle_delay)
> +		wait_remaining_ms_from_jiffies(jiffies,
> +				       (intel_dp->panel_power_cycle_delay - panel_power_off_duration));
>   	wait_panel_status(intel_dp, IDLE_CYCLE_MASK, IDLE_CYCLE_VALUE);
>   }
>   
> @@ -1969,7 +1977,7 @@ static void edp_panel_vdd_off_sync(struct intel_dp *intel_dp)
>   	I915_READ(pp_stat_reg), I915_READ(pp_ctrl_reg));
>   
>   	if ((pp & POWER_TARGET_ON) == 0)
> -		intel_dp->last_power_cycle = jiffies;
> +		intel_dp->panel_power_off_time = ktime_get_with_offset(TK_OFFS_BOOT);
>   
>   	power_domain = intel_display_port_aux_power_domain(intel_encoder);
>   	intel_display_power_put(dev_priv, power_domain);
> @@ -2118,7 +2126,6 @@ static void edp_panel_off(struct intel_dp *intel_dp)
>   	I915_WRITE(pp_ctrl_reg, pp);
>   	POSTING_READ(pp_ctrl_reg);
>   
> -	intel_dp->last_power_cycle = jiffies;
>   	wait_panel_off(intel_dp);
>   
>   	/* We got a reference when we enabled the VDD. */
> @@ -5122,7 +5129,6 @@ intel_dp_add_properties(struct intel_dp *intel_dp, struct drm_connector *connect
>   
>   static void intel_dp_init_panel_power_timestamps(struct intel_dp *intel_dp)
>   {
> -	intel_dp->last_power_cycle = jiffies;
>   	intel_dp->last_power_on = jiffies;
>   	intel_dp->last_backlight_off = jiffies;
>   }
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index d523ebb..84ad134 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -765,9 +765,9 @@ struct intel_dp {
>   	int backlight_off_delay;
>   	struct delayed_work panel_vdd_work;
>   	bool want_panel_vdd;
> -	unsigned long last_power_cycle;
>   	unsigned long last_power_on;
>   	unsigned long last_backlight_off;
> +	ktime_t panel_power_off_time;
>   
>   	struct notifier_block edp_notifier;
>   

Is this close to what you wanted?  thoughts?
Pardon me for pinging again as last time send with wrong email client.

Regards,
Abhay Kumar

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

Re: [PATCH] drm/i915: edp resume/On time optimization.

[Ville Syrjälä]

On Mon, Dec 21, 2015 at 05:18:52PM -0800, abhay.kumar@intel.com wrote:
> From: Abhay Kumar <abhay.kumar@intel.com>
> 
> Make resume/on codepath not to wait for panel_power_cycle_delay(t11_t12)
> if this time is already spent in suspend/poweron time.
> 
> v2: Use CLOCK_BOOTTIME and remove jiffies for panel power cycle
>     delay calculation(Ville).

The approach seems reasonable enough to me. There are a few issues with
the patch though, see below.

> 
> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> Signed-off-by: Abhay Kumar <abhay.kumar@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_ddi.c |  3 +++
>  drivers/gpu/drm/i915/intel_dp.c  | 22 ++++++++++++++--------
>  drivers/gpu/drm/i915/intel_drv.h |  2 +-
>  3 files changed, 18 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
> index e6408e5..480697d 100644
> --- a/drivers/gpu/drm/i915/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/intel_ddi.c
> @@ -2395,6 +2395,9 @@ static void intel_ddi_post_disable(struct intel_encoder *intel_encoder)
>  		intel_dp_sink_dpms(intel_dp, DRM_MODE_DPMS_OFF);
>  		intel_edp_panel_vdd_on(intel_dp);
>  		intel_edp_panel_off(intel_dp);
> +
> +		/* storing panel power off time */

The comment seems rather pointless.

> +		intel_dp->panel_power_off_time = ktime_get_with_offset(TK_OFFS_BOOT);

There appears to be a wrapper for this: ktime_get_boottime().

Not sure why you're adding this here anyway. Should be enough to just
replace the places where we currently sample jiffies to sample the boot
clock AFAICS.

>  	}
>  
>  	if (IS_SKYLAKE(dev) || IS_KABYLAKE(dev))
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 796e3d3..c813605 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -38,7 +38,6 @@
>  #include "intel_drv.h"
>  #include <drm/i915_drm.h>
>  #include "i915_drv.h"
> -

Spurious change.

>  #define DP_LINK_CHECK_TIMEOUT	(10 * 1000)
>  
>  /* Compliance test status bits  */
> @@ -1812,13 +1811,22 @@ static void wait_panel_off(struct intel_dp *intel_dp)
>  
>  static void wait_panel_power_cycle(struct intel_dp *intel_dp)
>  {
> +	ktime_t panel_power_on_time;
> +	u32 panel_power_off_duration;
> +
>  	DRM_DEBUG_KMS("Wait for panel power cycle\n");
>  
> -	/* When we disable the VDD override bit last we have to do the manual
> -	 * wait. */
> -	wait_remaining_ms_from_jiffies(intel_dp->last_power_cycle,
> -				       intel_dp->panel_power_cycle_delay);
> +        /* take the diffrence of currrent time and panel power off time
> +           and then make panel wait for t11_t12 if needed */

Indent fail. Also the comment isn't in proper format.

> +	panel_power_on_time = ktime_get_with_offset(TK_OFFS_BOOT);
> +	panel_power_off_duration = (panel_power_on_time.tv64 - intel_dp->panel_power_off_time.tv64);
> +	panel_power_off_duration = panel_power_off_duration / 1000000;

ktime_ms_delta() perhaps?

>  
> +	/* When we disable the VDD override bit last we have to do the manual
> +	 * wait */

This comment formatting is a bit wonky too. Maybe polish it up while at
it.

> +	if (panel_power_off_duration < intel_dp->panel_power_cycle_delay)
> +		wait_remaining_ms_from_jiffies(jiffies,
> +				       (intel_dp->panel_power_cycle_delay - panel_power_off_duration));
>  	wait_panel_status(intel_dp, IDLE_CYCLE_MASK, IDLE_CYCLE_VALUE);
>  }
>  
> @@ -1969,7 +1977,7 @@ static void edp_panel_vdd_off_sync(struct intel_dp *intel_dp)
>  	I915_READ(pp_stat_reg), I915_READ(pp_ctrl_reg));
>  
>  	if ((pp & POWER_TARGET_ON) == 0)
> -		intel_dp->last_power_cycle = jiffies;
> +		intel_dp->panel_power_off_time = ktime_get_with_offset(TK_OFFS_BOOT);
>  
>  	power_domain = intel_display_port_aux_power_domain(intel_encoder);
>  	intel_display_power_put(dev_priv, power_domain);
> @@ -2118,7 +2126,6 @@ static void edp_panel_off(struct intel_dp *intel_dp)
>  	I915_WRITE(pp_ctrl_reg, pp);
>  	POSTING_READ(pp_ctrl_reg);
>  
> -	intel_dp->last_power_cycle = jiffies;

Removing this doens't seem correct. Instead we should sample the clock
here as well.

>  	wait_panel_off(intel_dp);
>  
>  	/* We got a reference when we enabled the VDD. */
> @@ -5122,7 +5129,6 @@ intel_dp_add_properties(struct intel_dp *intel_dp, struct drm_connector *connect
>  
>  static void intel_dp_init_panel_power_timestamps(struct intel_dp *intel_dp)
>  {
> -	intel_dp->last_power_cycle = jiffies;

and I suppose here too.

>  	intel_dp->last_power_on = jiffies;
>  	intel_dp->last_backlight_off = jiffies;
>  }
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index d523ebb..84ad134 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -765,9 +765,9 @@ struct intel_dp {
>  	int backlight_off_delay;
>  	struct delayed_work panel_vdd_work;
>  	bool want_panel_vdd;
> -	unsigned long last_power_cycle;
>  	unsigned long last_power_on;
>  	unsigned long last_backlight_off;
> +	ktime_t panel_power_off_time;
>  
>  	struct notifier_block edp_notifier;
>  
> -- 
> 1.9.1

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

Re: [PATCH] drm/i915: edp resume/On time optimization.

[Kumar, Abhay]

On 1/7/2016 10:15 AM, Ville Syrjälä wrote:
> On Mon, Dec 21, 2015 at 05:18:52PM -0800, abhay.kumar@intel.com wrote:
>> From: Abhay Kumar <abhay.kumar@intel.com>
>>
>> Make resume/on codepath not to wait for panel_power_cycle_delay(t11_t12)
>> if this time is already spent in suspend/poweron time.
>>
>> v2: Use CLOCK_BOOTTIME and remove jiffies for panel power cycle
>>      delay calculation(Ville).
> The approach seems reasonable enough to me. There are a few issues with
> the patch though, see below.
>
>> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
>> Signed-off-by: Abhay Kumar <abhay.kumar@intel.com>
>> ---
>>   drivers/gpu/drm/i915/intel_ddi.c |  3 +++
>>   drivers/gpu/drm/i915/intel_dp.c  | 22 ++++++++++++++--------
>>   drivers/gpu/drm/i915/intel_drv.h |  2 +-
>>   3 files changed, 18 insertions(+), 9 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
>> index e6408e5..480697d 100644
>> --- a/drivers/gpu/drm/i915/intel_ddi.c
>> +++ b/drivers/gpu/drm/i915/intel_ddi.c
>> @@ -2395,6 +2395,9 @@ static void intel_ddi_post_disable(struct intel_encoder *intel_encoder)
>>   		intel_dp_sink_dpms(intel_dp, DRM_MODE_DPMS_OFF);
>>   		intel_edp_panel_vdd_on(intel_dp);
>>   		intel_edp_panel_off(intel_dp);
>> +
>> +		/* storing panel power off time */
> The comment seems rather pointless.
>
>> +		intel_dp->panel_power_off_time = ktime_get_with_offset(TK_OFFS_BOOT);
> There appears to be a wrapper for this: ktime_get_boottime().
>
> Not sure why you're adding this here anyway. Should be enough to just
> replace the places where we currently sample jiffies to sample the boot
> clock AFAICS.
>
>>   	}
>>   
>>   	if (IS_SKYLAKE(dev) || IS_KABYLAKE(dev))
>> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
>> index 796e3d3..c813605 100644
>> --- a/drivers/gpu/drm/i915/intel_dp.c
>> +++ b/drivers/gpu/drm/i915/intel_dp.c
>> @@ -38,7 +38,6 @@
>>   #include "intel_drv.h"
>>   #include <drm/i915_drm.h>
>>   #include "i915_drv.h"
>> -
> Spurious change.
>
>>   #define DP_LINK_CHECK_TIMEOUT	(10 * 1000)
>>   
>>   /* Compliance test status bits  */
>> @@ -1812,13 +1811,22 @@ static void wait_panel_off(struct intel_dp *intel_dp)
>>   
>>   static void wait_panel_power_cycle(struct intel_dp *intel_dp)
>>   {
>> +	ktime_t panel_power_on_time;
>> +	u32 panel_power_off_duration;
>> +
>>   	DRM_DEBUG_KMS("Wait for panel power cycle\n");
>>   
>> -	/* When we disable the VDD override bit last we have to do the manual
>> -	 * wait. */
>> -	wait_remaining_ms_from_jiffies(intel_dp->last_power_cycle,
>> -				       intel_dp->panel_power_cycle_delay);
>> +        /* take the diffrence of currrent time and panel power off time
>> +           and then make panel wait for t11_t12 if needed */
> Indent fail. Also the comment isn't in proper format.
>
>> +	panel_power_on_time = ktime_get_with_offset(TK_OFFS_BOOT);
>> +	panel_power_off_duration = (panel_power_on_time.tv64 - intel_dp->panel_power_off_time.tv64);
>> +	panel_power_off_duration = panel_power_off_duration / 1000000;
> ktime_ms_delta() perhaps?
sure.
>
>>   
>> +	/* When we disable the VDD override bit last we have to do the manual
>> +	 * wait */
> This comment formatting is a bit wonky too. Maybe polish it up while at
> it.
>
>> +	if (panel_power_off_duration < intel_dp->panel_power_cycle_delay)
>> +		wait_remaining_ms_from_jiffies(jiffies,
>> +				       (intel_dp->panel_power_cycle_delay - panel_power_off_duration));
>>   	wait_panel_status(intel_dp, IDLE_CYCLE_MASK, IDLE_CYCLE_VALUE);
>>   }
>>   
>> @@ -1969,7 +1977,7 @@ static void edp_panel_vdd_off_sync(struct intel_dp *intel_dp)
>>   	I915_READ(pp_stat_reg), I915_READ(pp_ctrl_reg));
>>   
>>   	if ((pp & POWER_TARGET_ON) == 0)
>> -		intel_dp->last_power_cycle = jiffies;
>> +		intel_dp->panel_power_off_time = ktime_get_with_offset(TK_OFFS_BOOT);
>>   
>>   	power_domain = intel_display_port_aux_power_domain(intel_encoder);
>>   	intel_display_power_put(dev_priv, power_domain);
>> @@ -2118,7 +2126,6 @@ static void edp_panel_off(struct intel_dp *intel_dp)
>>   	I915_WRITE(pp_ctrl_reg, pp);
>>   	POSTING_READ(pp_ctrl_reg);
>>   
>> -	intel_dp->last_power_cycle = jiffies;
> Removing this doens't seem correct. Instead we should sample the clock
> here as well.
Do we really need "last_power_cycle" ? As the ktime_get_bootime() will 
always calculate the boot time in fresh boot from zero and we only need 
to track the delta time when we go to suspend and resume.
this is the reason i removed last_power_cycle sampling and also 
initialization. Please let me know if this is ok and make sense?

>
>>   	wait_panel_off(intel_dp);
>>   
>>   	/* We got a reference when we enabled the VDD. */
>> @@ -5122,7 +5129,6 @@ intel_dp_add_properties(struct intel_dp *intel_dp, struct drm_connector *connect
>>   
>>   static void intel_dp_init_panel_power_timestamps(struct intel_dp *intel_dp)
>>   {
>> -	intel_dp->last_power_cycle = jiffies;
> and I suppose here too.
>
>>   	intel_dp->last_power_on = jiffies;
>>   	intel_dp->last_backlight_off = jiffies;
>>   }
>> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
>> index d523ebb..84ad134 100644
>> --- a/drivers/gpu/drm/i915/intel_drv.h
>> +++ b/drivers/gpu/drm/i915/intel_drv.h
>> @@ -765,9 +765,9 @@ struct intel_dp {
>>   	int backlight_off_delay;
>>   	struct delayed_work panel_vdd_work;
>>   	bool want_panel_vdd;
>> -	unsigned long last_power_cycle;
>>   	unsigned long last_power_on;
>>   	unsigned long last_backlight_off;
>> +	ktime_t panel_power_off_time;
>>   
>>   	struct notifier_block edp_notifier;
>>   
>> -- 
>> 1.9.1

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

Re: [PATCH] drm/i915: edp resume/On time optimization.

[Ville Syrjälä]

On Thu, Jan 07, 2016 at 06:27:24PM -0800, Kumar, Abhay wrote:
> 
> 
> On 1/7/2016 10:15 AM, Ville Syrjälä wrote:
> > On Mon, Dec 21, 2015 at 05:18:52PM -0800, abhay.kumar@intel.com wrote:
> >> From: Abhay Kumar <abhay.kumar@intel.com>
> >>
> >> Make resume/on codepath not to wait for panel_power_cycle_delay(t11_t12)
> >> if this time is already spent in suspend/poweron time.
> >>
> >> v2: Use CLOCK_BOOTTIME and remove jiffies for panel power cycle
> >>      delay calculation(Ville).
> > The approach seems reasonable enough to me. There are a few issues with
> > the patch though, see below.
> >
> >> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >> Signed-off-by: Abhay Kumar <abhay.kumar@intel.com>
> >> ---
> >>   drivers/gpu/drm/i915/intel_ddi.c |  3 +++
> >>   drivers/gpu/drm/i915/intel_dp.c  | 22 ++++++++++++++--------
> >>   drivers/gpu/drm/i915/intel_drv.h |  2 +-
> >>   3 files changed, 18 insertions(+), 9 deletions(-)
> >>
> >> diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
> >> index e6408e5..480697d 100644
> >> --- a/drivers/gpu/drm/i915/intel_ddi.c
> >> +++ b/drivers/gpu/drm/i915/intel_ddi.c
> >> @@ -2395,6 +2395,9 @@ static void intel_ddi_post_disable(struct intel_encoder *intel_encoder)
> >>   		intel_dp_sink_dpms(intel_dp, DRM_MODE_DPMS_OFF);
> >>   		intel_edp_panel_vdd_on(intel_dp);
> >>   		intel_edp_panel_off(intel_dp);
> >> +
> >> +		/* storing panel power off time */
> > The comment seems rather pointless.
> >
> >> +		intel_dp->panel_power_off_time = ktime_get_with_offset(TK_OFFS_BOOT);
> > There appears to be a wrapper for this: ktime_get_boottime().
> >
> > Not sure why you're adding this here anyway. Should be enough to just
> > replace the places where we currently sample jiffies to sample the boot
> > clock AFAICS.
> >
> >>   	}
> >>   
> >>   	if (IS_SKYLAKE(dev) || IS_KABYLAKE(dev))
> >> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> >> index 796e3d3..c813605 100644
> >> --- a/drivers/gpu/drm/i915/intel_dp.c
> >> +++ b/drivers/gpu/drm/i915/intel_dp.c
> >> @@ -38,7 +38,6 @@
> >>   #include "intel_drv.h"
> >>   #include <drm/i915_drm.h>
> >>   #include "i915_drv.h"
> >> -
> > Spurious change.
> >
> >>   #define DP_LINK_CHECK_TIMEOUT	(10 * 1000)
> >>   
> >>   /* Compliance test status bits  */
> >> @@ -1812,13 +1811,22 @@ static void wait_panel_off(struct intel_dp *intel_dp)
> >>   
> >>   static void wait_panel_power_cycle(struct intel_dp *intel_dp)
> >>   {
> >> +	ktime_t panel_power_on_time;
> >> +	u32 panel_power_off_duration;
> >> +
> >>   	DRM_DEBUG_KMS("Wait for panel power cycle\n");
> >>   
> >> -	/* When we disable the VDD override bit last we have to do the manual
> >> -	 * wait. */
> >> -	wait_remaining_ms_from_jiffies(intel_dp->last_power_cycle,
> >> -				       intel_dp->panel_power_cycle_delay);
> >> +        /* take the diffrence of currrent time and panel power off time
> >> +           and then make panel wait for t11_t12 if needed */
> > Indent fail. Also the comment isn't in proper format.
> >
> >> +	panel_power_on_time = ktime_get_with_offset(TK_OFFS_BOOT);
> >> +	panel_power_off_duration = (panel_power_on_time.tv64 - intel_dp->panel_power_off_time.tv64);
> >> +	panel_power_off_duration = panel_power_off_duration / 1000000;
> > ktime_ms_delta() perhaps?
> sure.
> >
> >>   
> >> +	/* When we disable the VDD override bit last we have to do the manual
> >> +	 * wait */
> > This comment formatting is a bit wonky too. Maybe polish it up while at
> > it.
> >
> >> +	if (panel_power_off_duration < intel_dp->panel_power_cycle_delay)
> >> +		wait_remaining_ms_from_jiffies(jiffies,
> >> +				       (intel_dp->panel_power_cycle_delay - panel_power_off_duration));
> >>   	wait_panel_status(intel_dp, IDLE_CYCLE_MASK, IDLE_CYCLE_VALUE);
> >>   }
> >>   
> >> @@ -1969,7 +1977,7 @@ static void edp_panel_vdd_off_sync(struct intel_dp *intel_dp)
> >>   	I915_READ(pp_stat_reg), I915_READ(pp_ctrl_reg));
> >>   
> >>   	if ((pp & POWER_TARGET_ON) == 0)
> >> -		intel_dp->last_power_cycle = jiffies;
> >> +		intel_dp->panel_power_off_time = ktime_get_with_offset(TK_OFFS_BOOT);
> >>   
> >>   	power_domain = intel_display_port_aux_power_domain(intel_encoder);
> >>   	intel_display_power_put(dev_priv, power_domain);
> >> @@ -2118,7 +2126,6 @@ static void edp_panel_off(struct intel_dp *intel_dp)
> >>   	I915_WRITE(pp_ctrl_reg, pp);
> >>   	POSTING_READ(pp_ctrl_reg);
> >>   
> >> -	intel_dp->last_power_cycle = jiffies;
> > Removing this doens't seem correct. Instead we should sample the clock
> > here as well.
> Do we really need "last_power_cycle" ? As the ktime_get_bootime() will 
> always calculate the boot time in fresh boot from zero and we only need 
> to track the delta time when we go to suspend and resume.

We need to track if from when the last power cycle was initiated.

> this is the reason i removed last_power_cycle sampling and also 
> initialization. Please let me know if this is ok and make sense?
> 
> >
> >>   	wait_panel_off(intel_dp);
> >>   
> >>   	/* We got a reference when we enabled the VDD. */
> >> @@ -5122,7 +5129,6 @@ intel_dp_add_properties(struct intel_dp *intel_dp, struct drm_connector *connect
> >>   
> >>   static void intel_dp_init_panel_power_timestamps(struct intel_dp *intel_dp)
> >>   {
> >> -	intel_dp->last_power_cycle = jiffies;
> > and I suppose here too.
> >
> >>   	intel_dp->last_power_on = jiffies;
> >>   	intel_dp->last_backlight_off = jiffies;
> >>   }
> >> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> >> index d523ebb..84ad134 100644
> >> --- a/drivers/gpu/drm/i915/intel_drv.h
> >> +++ b/drivers/gpu/drm/i915/intel_drv.h
> >> @@ -765,9 +765,9 @@ struct intel_dp {
> >>   	int backlight_off_delay;
> >>   	struct delayed_work panel_vdd_work;
> >>   	bool want_panel_vdd;
> >> -	unsigned long last_power_cycle;
> >>   	unsigned long last_power_on;
> >>   	unsigned long last_backlight_off;
> >> +	ktime_t panel_power_off_time;
> >>   
> >>   	struct notifier_block edp_notifier;
> >>   
> >> -- 
> >> 1.9.1

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

[PATCH] drm/i915: edp resume/On time optimization.

[abhay.kumar]

From: Abhay Kumar <abhay.kumar@intel.com>

Make resume/on codepath not to wait for panel_power_cycle_delay(t11_t12)
if this time is already spent in suspend/poweron time.

Change-Id: Ied0f10f82776af8e6e8ff561bb4e5c0ce1dad4b3
Signed-off-by: Abhay Kumar <abhay.kumar@intel.com>
---
 drivers/gpu/drm/i915/intel_ddi.c |  3 +++
 drivers/gpu/drm/i915/intel_dp.c  | 22 ++++++++++++++--------
 drivers/gpu/drm/i915/intel_drv.h |  2 +-
 3 files changed, 18 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index cbabcb4..fe99d72 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -2347,6 +2347,9 @@ static void intel_ddi_post_disable(struct intel_encoder *intel_encoder)
 		intel_dp_sink_dpms(intel_dp, DRM_MODE_DPMS_OFF);
 		intel_edp_panel_vdd_on(intel_dp);
 		intel_edp_panel_off(intel_dp);
+
+		/* storing panel power off time */
+		intel_dp->panel_power_off_time = ktime_get_with_offset(TK_OFFS_BOOT);
 	}
 
 	if (IS_SKYLAKE(dev))
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index acda70e..845944d 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -38,7 +38,6 @@
 #include "intel_drv.h"
 #include <drm/i915_drm.h>
 #include "i915_drv.h"
-
 #define DP_LINK_CHECK_TIMEOUT	(10 * 1000)
 
 /* Compliance test status bits  */
@@ -1654,13 +1653,22 @@ static void wait_panel_off(struct intel_dp *intel_dp)
 
 static void wait_panel_power_cycle(struct intel_dp *intel_dp)
 {
+	ktime_t panel_power_on_time;
+	u32 panel_power_off_duration;
+
 	DRM_DEBUG_KMS("Wait for panel power cycle\n");
 
-	/* When we disable the VDD override bit last we have to do the manual
-	 * wait. */
-	wait_remaining_ms_from_jiffies(intel_dp->last_power_cycle,
-				       intel_dp->panel_power_cycle_delay);
+        /* take the diffrence of currrent time and panel power off time
+           and then make panel wait for t11_t12 if needed */
+	panel_power_on_time = ktime_get_with_offset(TK_OFFS_BOOT);
+	panel_power_off_duration = (panel_power_on_time.tv64 - intel_dp->panel_power_off_time.tv64);
+	panel_power_off_duration = panel_power_off_duration / 1000000;
 
+	/* When we disable the VDD override bit last we have to do the manual
+	 * wait */
+	if (panel_power_off_duration < intel_dp->panel_power_cycle_delay)
+		wait_remaining_ms_from_jiffies(jiffies,
+				       (intel_dp->panel_power_cycle_delay - panel_power_off_duration));
 	wait_panel_status(intel_dp, IDLE_CYCLE_MASK, IDLE_CYCLE_VALUE);
 }
 
@@ -1811,7 +1819,7 @@ static void edp_panel_vdd_off_sync(struct intel_dp *intel_dp)
 	I915_READ(pp_stat_reg), I915_READ(pp_ctrl_reg));
 
 	if ((pp & POWER_TARGET_ON) == 0)
-		intel_dp->last_power_cycle = jiffies;
+		intel_dp->panel_power_off_time = ktime_get_with_offset(TK_OFFS_BOOT);
 
 	power_domain = intel_display_port_power_domain(intel_encoder);
 	intel_display_power_put(dev_priv, power_domain);
@@ -1960,7 +1968,6 @@ static void edp_panel_off(struct intel_dp *intel_dp)
 	I915_WRITE(pp_ctrl_reg, pp);
 	POSTING_READ(pp_ctrl_reg);
 
-	intel_dp->last_power_cycle = jiffies;
 	wait_panel_off(intel_dp);
 
 	/* We got a reference when we enabled the VDD. */
@@ -5196,7 +5203,6 @@ intel_dp_add_properties(struct intel_dp *intel_dp, struct drm_connector *connect
 
 static void intel_dp_init_panel_power_timestamps(struct intel_dp *intel_dp)
 {
-	intel_dp->last_power_cycle = jiffies;
 	intel_dp->last_power_on = jiffies;
 	intel_dp->last_backlight_off = jiffies;
 }
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index d44f2f5..ef82e8f 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -722,9 +722,9 @@ struct intel_dp {
 	int backlight_off_delay;
 	struct delayed_work panel_vdd_work;
 	bool want_panel_vdd;
-	unsigned long last_power_cycle;
 	unsigned long last_power_on;
 	unsigned long last_backlight_off;
+	ktime_t panel_power_off_time;
 
 	struct notifier_block edp_notifier;
 
-- 
1.9.1

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

Re: [PATCH] drm/i915: edp resume/On time optimization.

[Kumar, Abhay]

Changed the implementation using boottime and removed jiffies. Please review and let us know if this is close.

-----Original Message-----
From: Kumar, Abhay 
Sent: Friday, December 18, 2015 11:55 AM
To: Intel-gfx@lists.freedesktop.org
Cc: Kumar, Abhay
Subject: [PATCH] drm/i915: edp resume/On time optimization.

From: Abhay Kumar <abhay.kumar@intel.com>

Make resume/on codepath not to wait for panel_power_cycle_delay(t11_t12) if this time is already spent in suspend/poweron time.

Change-Id: Ied0f10f82776af8e6e8ff561bb4e5c0ce1dad4b3
Signed-off-by: Abhay Kumar <abhay.kumar@intel.com>
---
 drivers/gpu/drm/i915/intel_ddi.c |  3 +++  drivers/gpu/drm/i915/intel_dp.c  | 22 ++++++++++++++--------  drivers/gpu/drm/i915/intel_drv.h |  2 +-
 3 files changed, 18 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index cbabcb4..fe99d72 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -2347,6 +2347,9 @@ static void intel_ddi_post_disable(struct intel_encoder *intel_encoder)
 		intel_dp_sink_dpms(intel_dp, DRM_MODE_DPMS_OFF);
 		intel_edp_panel_vdd_on(intel_dp);
 		intel_edp_panel_off(intel_dp);
+
+		/* storing panel power off time */
+		intel_dp->panel_power_off_time = ktime_get_with_offset(TK_OFFS_BOOT);
 	}
 
 	if (IS_SKYLAKE(dev))
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c index acda70e..845944d 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -38,7 +38,6 @@
 #include "intel_drv.h"
 #include <drm/i915_drm.h>
 #include "i915_drv.h"
-
 #define DP_LINK_CHECK_TIMEOUT	(10 * 1000)
 
 /* Compliance test status bits  */
@@ -1654,13 +1653,22 @@ static void wait_panel_off(struct intel_dp *intel_dp)
 
 static void wait_panel_power_cycle(struct intel_dp *intel_dp)  {
+	ktime_t panel_power_on_time;
+	u32 panel_power_off_duration;
+
 	DRM_DEBUG_KMS("Wait for panel power cycle\n");
 
-	/* When we disable the VDD override bit last we have to do the manual
-	 * wait. */
-	wait_remaining_ms_from_jiffies(intel_dp->last_power_cycle,
-				       intel_dp->panel_power_cycle_delay);
+        /* take the diffrence of currrent time and panel power off time
+           and then make panel wait for t11_t12 if needed */
+	panel_power_on_time = ktime_get_with_offset(TK_OFFS_BOOT);
+	panel_power_off_duration = (panel_power_on_time.tv64 - intel_dp->panel_power_off_time.tv64);
+	panel_power_off_duration = panel_power_off_duration / 1000000;
 
+	/* When we disable the VDD override bit last we have to do the manual
+	 * wait */
+	if (panel_power_off_duration < intel_dp->panel_power_cycle_delay)
+		wait_remaining_ms_from_jiffies(jiffies,
+				       (intel_dp->panel_power_cycle_delay - 
+panel_power_off_duration));
 	wait_panel_status(intel_dp, IDLE_CYCLE_MASK, IDLE_CYCLE_VALUE);  }
 
@@ -1811,7 +1819,7 @@ static void edp_panel_vdd_off_sync(struct intel_dp *intel_dp)
 	I915_READ(pp_stat_reg), I915_READ(pp_ctrl_reg));
 
 	if ((pp & POWER_TARGET_ON) == 0)
-		intel_dp->last_power_cycle = jiffies;
+		intel_dp->panel_power_off_time = ktime_get_with_offset(TK_OFFS_BOOT);
 
 	power_domain = intel_display_port_power_domain(intel_encoder);
 	intel_display_power_put(dev_priv, power_domain); @@ -1960,7 +1968,6 @@ static void edp_panel_off(struct intel_dp *intel_dp)
 	I915_WRITE(pp_ctrl_reg, pp);
 	POSTING_READ(pp_ctrl_reg);
 
-	intel_dp->last_power_cycle = jiffies;
 	wait_panel_off(intel_dp);
 
 	/* We got a reference when we enabled the VDD. */ @@ -5196,7 +5203,6 @@ intel_dp_add_properties(struct intel_dp *intel_dp, struct drm_connector *connect
 
 static void intel_dp_init_panel_power_timestamps(struct intel_dp *intel_dp)  {
-	intel_dp->last_power_cycle = jiffies;
 	intel_dp->last_power_on = jiffies;
 	intel_dp->last_backlight_off = jiffies;  } diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index d44f2f5..ef82e8f 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -722,9 +722,9 @@ struct intel_dp {
 	int backlight_off_delay;
 	struct delayed_work panel_vdd_work;
 	bool want_panel_vdd;
-	unsigned long last_power_cycle;
 	unsigned long last_power_on;
 	unsigned long last_backlight_off;
+	ktime_t panel_power_off_time;
 
 	struct notifier_block edp_notifier;
 
--
1.9.1

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

Re: [PATCH] drm/i915: edp resume/On time optimization.

[Daniel Vetter]

On Mon, Dec 21, 2015 at 05:33:41AM +0000, Kumar, Abhay wrote:
> Changed the implementation using boottime and removed jiffies. Please review and let us know if this is close.

Comments like these should be part of the patch submission itself,
including a note about which reviewer made the suggestion and adding
everyone who commented to the cc list. So something like this at the
bottom:

v2:
- Change .... (Ville).
- Use boootime instead of jiffies (Ville).

Cc: Ville .... (full mail address here).
Singed-off-by: ...

Without either the in-patch changelog or the cc it's much harder for
reviewers to follow along, especially when they're commenting on 10+ patch
series at the same time.

Please resubmit with that added.

Thanks, Daniel

> 
> -----Original Message-----
> From: Kumar, Abhay 
> Sent: Friday, December 18, 2015 11:55 AM
> To: Intel-gfx@lists.freedesktop.org
> Cc: Kumar, Abhay
> Subject: [PATCH] drm/i915: edp resume/On time optimization.
> 
> From: Abhay Kumar <abhay.kumar@intel.com>
> 
> Make resume/on codepath not to wait for panel_power_cycle_delay(t11_t12) if this time is already spent in suspend/poweron time.
> 
> Change-Id: Ied0f10f82776af8e6e8ff561bb4e5c0ce1dad4b3
> Signed-off-by: Abhay Kumar <abhay.kumar@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_ddi.c |  3 +++  drivers/gpu/drm/i915/intel_dp.c  | 22 ++++++++++++++--------  drivers/gpu/drm/i915/intel_drv.h |  2 +-
>  3 files changed, 18 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
> index cbabcb4..fe99d72 100644
> --- a/drivers/gpu/drm/i915/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/intel_ddi.c
> @@ -2347,6 +2347,9 @@ static void intel_ddi_post_disable(struct intel_encoder *intel_encoder)
>  		intel_dp_sink_dpms(intel_dp, DRM_MODE_DPMS_OFF);
>  		intel_edp_panel_vdd_on(intel_dp);
>  		intel_edp_panel_off(intel_dp);
> +
> +		/* storing panel power off time */
> +		intel_dp->panel_power_off_time = ktime_get_with_offset(TK_OFFS_BOOT);
>  	}
>  
>  	if (IS_SKYLAKE(dev))
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c index acda70e..845944d 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -38,7 +38,6 @@
>  #include "intel_drv.h"
>  #include <drm/i915_drm.h>
>  #include "i915_drv.h"
> -
>  #define DP_LINK_CHECK_TIMEOUT	(10 * 1000)
>  
>  /* Compliance test status bits  */
> @@ -1654,13 +1653,22 @@ static void wait_panel_off(struct intel_dp *intel_dp)
>  
>  static void wait_panel_power_cycle(struct intel_dp *intel_dp)  {
> +	ktime_t panel_power_on_time;
> +	u32 panel_power_off_duration;
> +
>  	DRM_DEBUG_KMS("Wait for panel power cycle\n");
>  
> -	/* When we disable the VDD override bit last we have to do the manual
> -	 * wait. */
> -	wait_remaining_ms_from_jiffies(intel_dp->last_power_cycle,
> -				       intel_dp->panel_power_cycle_delay);
> +        /* take the diffrence of currrent time and panel power off time
> +           and then make panel wait for t11_t12 if needed */
> +	panel_power_on_time = ktime_get_with_offset(TK_OFFS_BOOT);
> +	panel_power_off_duration = (panel_power_on_time.tv64 - intel_dp->panel_power_off_time.tv64);
> +	panel_power_off_duration = panel_power_off_duration / 1000000;
>  
> +	/* When we disable the VDD override bit last we have to do the manual
> +	 * wait */
> +	if (panel_power_off_duration < intel_dp->panel_power_cycle_delay)
> +		wait_remaining_ms_from_jiffies(jiffies,
> +				       (intel_dp->panel_power_cycle_delay - 
> +panel_power_off_duration));
>  	wait_panel_status(intel_dp, IDLE_CYCLE_MASK, IDLE_CYCLE_VALUE);  }
>  
> @@ -1811,7 +1819,7 @@ static void edp_panel_vdd_off_sync(struct intel_dp *intel_dp)
>  	I915_READ(pp_stat_reg), I915_READ(pp_ctrl_reg));
>  
>  	if ((pp & POWER_TARGET_ON) == 0)
> -		intel_dp->last_power_cycle = jiffies;
> +		intel_dp->panel_power_off_time = ktime_get_with_offset(TK_OFFS_BOOT);
>  
>  	power_domain = intel_display_port_power_domain(intel_encoder);
>  	intel_display_power_put(dev_priv, power_domain); @@ -1960,7 +1968,6 @@ static void edp_panel_off(struct intel_dp *intel_dp)
>  	I915_WRITE(pp_ctrl_reg, pp);
>  	POSTING_READ(pp_ctrl_reg);
>  
> -	intel_dp->last_power_cycle = jiffies;
>  	wait_panel_off(intel_dp);
>  
>  	/* We got a reference when we enabled the VDD. */ @@ -5196,7 +5203,6 @@ intel_dp_add_properties(struct intel_dp *intel_dp, struct drm_connector *connect
>  
>  static void intel_dp_init_panel_power_timestamps(struct intel_dp *intel_dp)  {
> -	intel_dp->last_power_cycle = jiffies;
>  	intel_dp->last_power_on = jiffies;
>  	intel_dp->last_backlight_off = jiffies;  } diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index d44f2f5..ef82e8f 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -722,9 +722,9 @@ struct intel_dp {
>  	int backlight_off_delay;
>  	struct delayed_work panel_vdd_work;
>  	bool want_panel_vdd;
> -	unsigned long last_power_cycle;
>  	unsigned long last_power_on;
>  	unsigned long last_backlight_off;
> +	ktime_t panel_power_off_time;
>  
>  	struct notifier_block edp_notifier;
>  
> --
> 1.9.1
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915: edp resume/On time optimization.

[Kumar, Abhay]

Thanks Daniel. Will push today with proper comment.

-----Original Message-----
From: Daniel Vetter [mailto:daniel.vetter@ffwll.ch] On Behalf Of Daniel Vetter
Sent: Monday, December 21, 2015 7:58 AM
To: Kumar, Abhay
Cc: Intel-gfx@lists.freedesktop.org; Syrjala, Ville; Paulo Zanoni
Subject: Re: [Intel-gfx] [PATCH] drm/i915: edp resume/On time optimization.

On Mon, Dec 21, 2015 at 05:33:41AM +0000, Kumar, Abhay wrote:
> Changed the implementation using boottime and removed jiffies. Please review and let us know if this is close.

Comments like these should be part of the patch submission itself, including a note about which reviewer made the suggestion and adding everyone who commented to the cc list. So something like this at the
bottom:

v2:
- Change .... (Ville).
- Use boootime instead of jiffies (Ville).

Cc: Ville .... (full mail address here).
Singed-off-by: ...

Without either the in-patch changelog or the cc it's much harder for reviewers to follow along, especially when they're commenting on 10+ patch series at the same time.

Please resubmit with that added.

Thanks, Daniel

> 
> -----Original Message-----
> From: Kumar, Abhay
> Sent: Friday, December 18, 2015 11:55 AM
> To: Intel-gfx@lists.freedesktop.org
> Cc: Kumar, Abhay
> Subject: [PATCH] drm/i915: edp resume/On time optimization.
> 
> From: Abhay Kumar <abhay.kumar@intel.com>
> 
> Make resume/on codepath not to wait for panel_power_cycle_delay(t11_t12) if this time is already spent in suspend/poweron time.
> 
> Change-Id: Ied0f10f82776af8e6e8ff561bb4e5c0ce1dad4b3
> Signed-off-by: Abhay Kumar <abhay.kumar@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_ddi.c |  3 +++  
> drivers/gpu/drm/i915/intel_dp.c  | 22 ++++++++++++++--------  
> drivers/gpu/drm/i915/intel_drv.h |  2 +-
>  3 files changed, 18 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_ddi.c 
> b/drivers/gpu/drm/i915/intel_ddi.c
> index cbabcb4..fe99d72 100644
> --- a/drivers/gpu/drm/i915/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/intel_ddi.c
> @@ -2347,6 +2347,9 @@ static void intel_ddi_post_disable(struct intel_encoder *intel_encoder)
>  		intel_dp_sink_dpms(intel_dp, DRM_MODE_DPMS_OFF);
>  		intel_edp_panel_vdd_on(intel_dp);
>  		intel_edp_panel_off(intel_dp);
> +
> +		/* storing panel power off time */
> +		intel_dp->panel_power_off_time = 
> +ktime_get_with_offset(TK_OFFS_BOOT);
>  	}
>  
>  	if (IS_SKYLAKE(dev))
> diff --git a/drivers/gpu/drm/i915/intel_dp.c 
> b/drivers/gpu/drm/i915/intel_dp.c index acda70e..845944d 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -38,7 +38,6 @@
>  #include "intel_drv.h"
>  #include <drm/i915_drm.h>
>  #include "i915_drv.h"
> -
>  #define DP_LINK_CHECK_TIMEOUT	(10 * 1000)
>  
>  /* Compliance test status bits  */
> @@ -1654,13 +1653,22 @@ static void wait_panel_off(struct intel_dp 
> *intel_dp)
>  
>  static void wait_panel_power_cycle(struct intel_dp *intel_dp)  {
> +	ktime_t panel_power_on_time;
> +	u32 panel_power_off_duration;
> +
>  	DRM_DEBUG_KMS("Wait for panel power cycle\n");
>  
> -	/* When we disable the VDD override bit last we have to do the manual
> -	 * wait. */
> -	wait_remaining_ms_from_jiffies(intel_dp->last_power_cycle,
> -				       intel_dp->panel_power_cycle_delay);
> +        /* take the diffrence of currrent time and panel power off time
> +           and then make panel wait for t11_t12 if needed */
> +	panel_power_on_time = ktime_get_with_offset(TK_OFFS_BOOT);
> +	panel_power_off_duration = (panel_power_on_time.tv64 - intel_dp->panel_power_off_time.tv64);
> +	panel_power_off_duration = panel_power_off_duration / 1000000;
>  
> +	/* When we disable the VDD override bit last we have to do the manual
> +	 * wait */
> +	if (panel_power_off_duration < intel_dp->panel_power_cycle_delay)
> +		wait_remaining_ms_from_jiffies(jiffies,
> +				       (intel_dp->panel_power_cycle_delay - 
> +panel_power_off_duration));
>  	wait_panel_status(intel_dp, IDLE_CYCLE_MASK, IDLE_CYCLE_VALUE);  }
>  
> @@ -1811,7 +1819,7 @@ static void edp_panel_vdd_off_sync(struct intel_dp *intel_dp)
>  	I915_READ(pp_stat_reg), I915_READ(pp_ctrl_reg));
>  
>  	if ((pp & POWER_TARGET_ON) == 0)
> -		intel_dp->last_power_cycle = jiffies;
> +		intel_dp->panel_power_off_time = 
> +ktime_get_with_offset(TK_OFFS_BOOT);
>  
>  	power_domain = intel_display_port_power_domain(intel_encoder);
>  	intel_display_power_put(dev_priv, power_domain); @@ -1960,7 +1968,6 @@ static void edp_panel_off(struct intel_dp *intel_dp)
>  	I915_WRITE(pp_ctrl_reg, pp);
>  	POSTING_READ(pp_ctrl_reg);
>  
> -	intel_dp->last_power_cycle = jiffies;
>  	wait_panel_off(intel_dp);
>  
>  	/* We got a reference when we enabled the VDD. */ @@ -5196,7 +5203,6 
> @@ intel_dp_add_properties(struct intel_dp *intel_dp, struct 
> drm_connector *connect
>  
>  static void intel_dp_init_panel_power_timestamps(struct intel_dp *intel_dp)  {
> -	intel_dp->last_power_cycle = jiffies;
>  	intel_dp->last_power_on = jiffies;
>  	intel_dp->last_backlight_off = jiffies;  } diff --git 
> a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index d44f2f5..ef82e8f 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -722,9 +722,9 @@ struct intel_dp {
>  	int backlight_off_delay;
>  	struct delayed_work panel_vdd_work;
>  	bool want_panel_vdd;
> -	unsigned long last_power_cycle;
>  	unsigned long last_power_on;
>  	unsigned long last_backlight_off;
> +	ktime_t panel_power_off_time;
>  
>  	struct notifier_block edp_notifier;
>  
> --
> 1.9.1
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH] drm/i915: edp resume/On time optimization.

[abhay.kumar]

From: Abhay Kumar <abhay.kumar@intel.com>

Make resume codepath not to wait for panel_power_cycle_delay(t11_t12)
if this time is already spent in suspend/poweron time.

Change-Id: Ied0f10f82776af8e6e8ff561bb4e5c0ce1dad4b3
Signed-off-by: Abhay Kumar <abhay.kumar@intel.com>
---
 drivers/gpu/drm/i915/intel_ddi.c |  3 +++
 drivers/gpu/drm/i915/intel_dp.c  | 21 +++++++++++++--------
 drivers/gpu/drm/i915/intel_drv.h |  2 +-
 3 files changed, 17 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index cbabcb4..fe99d72 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -2347,6 +2347,9 @@ static void intel_ddi_post_disable(struct intel_encoder *intel_encoder)
 		intel_dp_sink_dpms(intel_dp, DRM_MODE_DPMS_OFF);
 		intel_edp_panel_vdd_on(intel_dp);
 		intel_edp_panel_off(intel_dp);
+
+		/* storing panel power off time */
+		intel_dp->panel_power_off_time = ktime_get_with_offset(TK_OFFS_BOOT);
 	}
 
 	if (IS_SKYLAKE(dev))
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index acda70e..509da67 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -38,7 +38,6 @@
 #include "intel_drv.h"
 #include <drm/i915_drm.h>
 #include "i915_drv.h"
-
 #define DP_LINK_CHECK_TIMEOUT	(10 * 1000)
 
 /* Compliance test status bits  */
@@ -1654,13 +1653,21 @@ static void wait_panel_off(struct intel_dp *intel_dp)
 
 static void wait_panel_power_cycle(struct intel_dp *intel_dp)
 {
+	ktime_t panel_power_on_time;
+	u32 panel_power_off_duration;
+
 	DRM_DEBUG_KMS("Wait for panel power cycle\n");
 
-	/* When we disable the VDD override bit last we have to do the manual
-	 * wait. */
-	wait_remaining_ms_from_jiffies(intel_dp->last_power_cycle,
-				       intel_dp->panel_power_cycle_delay);
+        /* take the diffrence of currrent time and panel power off time
+           and then make panel wait for t11_t12 if needed */
+        panel_power_on_time = ktime_get_with_offset(TK_OFFS_BOOT);
+        panel_power_off_duration = (panel_power_on_time.tv64 - intel_dp->panel_power_off_time.tv64) / 1000000;
 
+	/* When we disable the VDD override bit last we have to do the manual
+	 * wait */
+	if (panel_power_off_duration < intel_dp->panel_power_cycle_delay)
+		wait_remaining_ms_from_jiffies(jiffies,
+				       (intel_dp->panel_power_cycle_delay - panel_power_off_duration));
 	wait_panel_status(intel_dp, IDLE_CYCLE_MASK, IDLE_CYCLE_VALUE);
 }
 
@@ -1811,7 +1818,7 @@ static void edp_panel_vdd_off_sync(struct intel_dp *intel_dp)
 	I915_READ(pp_stat_reg), I915_READ(pp_ctrl_reg));
 
 	if ((pp & POWER_TARGET_ON) == 0)
-		intel_dp->last_power_cycle = jiffies;
+		intel_dp->panel_power_off_time = ktime_get_with_offset(TK_OFFS_BOOT);
 
 	power_domain = intel_display_port_power_domain(intel_encoder);
 	intel_display_power_put(dev_priv, power_domain);
@@ -1960,7 +1967,6 @@ static void edp_panel_off(struct intel_dp *intel_dp)
 	I915_WRITE(pp_ctrl_reg, pp);
 	POSTING_READ(pp_ctrl_reg);
 
-	intel_dp->last_power_cycle = jiffies;
 	wait_panel_off(intel_dp);
 
 	/* We got a reference when we enabled the VDD. */
@@ -5196,7 +5202,6 @@ intel_dp_add_properties(struct intel_dp *intel_dp, struct drm_connector *connect
 
 static void intel_dp_init_panel_power_timestamps(struct intel_dp *intel_dp)
 {
-	intel_dp->last_power_cycle = jiffies;
 	intel_dp->last_power_on = jiffies;
 	intel_dp->last_backlight_off = jiffies;
 }
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index d44f2f5..ef82e8f 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -722,9 +722,9 @@ struct intel_dp {
 	int backlight_off_delay;
 	struct delayed_work panel_vdd_work;
 	bool want_panel_vdd;
-	unsigned long last_power_cycle;
 	unsigned long last_power_on;
 	unsigned long last_backlight_off;
+	ktime_t panel_power_off_time;
 
 	struct notifier_block edp_notifier;
 
-- 
1.9.1

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

Re: [PATCH] drm/i915: edp resume/On time optimization.

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 819 bytes --]

Hi Abhay,

[auto build test ERROR on v4.4-rc3]
[also build test ERROR on next-20151217]
[cannot apply to drm-intel/for-linux-next v4.4-rc5 v4.4-rc4]

url:    https://github.com/0day-ci/linux/commits/abhay-kumar-intel-com/drm-i915-edp-resume-On-time-optimization/20151218-143043
config: i386-randconfig-i0-201550 (attached as .config)
reproduce:
        # save the attached .config to linux build tree
        make ARCH=i386 

All errors (new ones prefixed by >>):

   drivers/built-in.o: In function `wait_panel_power_cycle':
>> intel_dp.c:(.text+0x306552): undefined reference to `__divdi3'
   intel_dp.c:(.text+0x3065b9): undefined reference to `__divdi3'

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/octet-stream, Size: 24404 bytes --]

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

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

[PATCH] drm/i915: edp resume/On time optimization.

[abhay.kumar]

From: Abhay Kumar <abhay.kumar@intel.com>

Make resume codepath not to wait for panel_power_cycle_delay(t11_t12)
if this time is already spent in suspend/poweron time.

Signed-off-by: Abhay Kumar <abhay.kumar@intel.com>
---
 drivers/gpu/drm/i915/intel_ddi.c |  3 +++
 drivers/gpu/drm/i915/intel_dp.c  | 18 ++++++++++++++++++
 drivers/gpu/drm/i915/intel_drv.h |  2 ++
 3 files changed, 23 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index f00a3c9..d2a5a89 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -2395,6 +2395,9 @@ static void intel_ddi_post_disable(struct intel_encoder *intel_encoder)
 		intel_dp_sink_dpms(intel_dp, DRM_MODE_DPMS_OFF);
 		intel_edp_panel_vdd_on(intel_dp);
 		intel_edp_panel_off(intel_dp);
+
+		/* storing panel power off time */
+		do_gettimeofday(&intel_dp->panel_power_off_timestamp);
 	}
 
 	if (IS_SKYLAKE(dev) || IS_KABYLAKE(dev))
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 0f1eb96..1ca01b1 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -2032,6 +2032,9 @@ static void edp_panel_on(struct intel_dp *intel_dp)
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	u32 pp;
 	i915_reg_t pp_ctrl_reg;
+	u32 panel_power_off_duration;
+	u32 temp_power_cycle_delay;
+
 
 	lockdep_assert_held(&dev_priv->pps_mutex);
 
@@ -2045,8 +2048,22 @@ static void edp_panel_on(struct intel_dp *intel_dp)
 		 "eDP port %c panel power already on\n",
 		 port_name(dp_to_dig_port(intel_dp)->port)))
 		return;
+	/* taking the diffrence of currrent time and panel power off time
+	   and then make panel to wait for T12 if needed */
+	do_gettimeofday(&intel_dp->panel_power_on_timestamp);
+
+	panel_power_off_duration  = (intel_dp->panel_power_on_timestamp.tv_sec-intel_dp->panel_power_off_timestamp.tv_sec) * 1000000 +  intel_dp->panel_power_on_timestamp.tv_usec-intel_dp->panel_power_off_timestamp.tv_usec;
+	panel_power_off_duration = panel_power_off_duration / 1000 ;
+	temp_power_cycle_delay = intel_dp->panel_power_cycle_delay;
+
+	if(panel_power_off_duration >= intel_dp->panel_power_cycle_delay) {
+		intel_dp->panel_power_cycle_delay = 0;
+	} else {
+		intel_dp->panel_power_cycle_delay = intel_dp->panel_power_cycle_delay - panel_power_off_duration;
+	}
 
 	wait_panel_power_cycle(intel_dp);
+	intel_dp->panel_power_cycle_delay = temp_power_cycle_delay;
 
 	pp_ctrl_reg = _pp_ctrl_reg(intel_dp);
 	pp = ironlake_get_pp_control(intel_dp);
@@ -5127,6 +5144,7 @@ static void intel_dp_init_panel_power_timestamps(struct intel_dp *intel_dp)
 	intel_dp->last_power_cycle = jiffies;
 	intel_dp->last_power_on = jiffies;
 	intel_dp->last_backlight_off = jiffies;
+	do_gettimeofday(&intel_dp->panel_power_off_timestamp);
 }
 
 static void
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 76dfa28..66ed2cb 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -769,6 +769,8 @@ struct intel_dp {
 	unsigned long last_power_cycle;
 	unsigned long last_power_on;
 	unsigned long last_backlight_off;
+	struct timeval panel_power_off_timestamp;
+	struct timeval panel_power_on_timestamp;
 
 	struct notifier_block edp_notifier;
 
-- 
1.9.1

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

Re: [PATCH] drm/i915: edp resume/On time optimization.

[Kumar, Abhay]

Is this something close to what we wanted to optimize for edp resume time and using wall clock.

-----Original Message-----
From: Kumar, Abhay 
Sent: Tuesday, December 15, 2015 2:17 PM
To: Intel-gfx@lists.freedesktop.org
Cc: Kumar, Abhay
Subject: [PATCH] drm/i915: edp resume/On time optimization.

From: Abhay Kumar <abhay.kumar@intel.com>

Make resume codepath not to wait for panel_power_cycle_delay(t11_t12) if this time is already spent in suspend/poweron time.

Signed-off-by: Abhay Kumar <abhay.kumar@intel.com>
---
 drivers/gpu/drm/i915/intel_ddi.c |  3 +++  drivers/gpu/drm/i915/intel_dp.c  | 18 ++++++++++++++++++  drivers/gpu/drm/i915/intel_drv.h |  2 ++
 3 files changed, 23 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index f00a3c9..d2a5a89 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -2395,6 +2395,9 @@ static void intel_ddi_post_disable(struct intel_encoder *intel_encoder)
 		intel_dp_sink_dpms(intel_dp, DRM_MODE_DPMS_OFF);
 		intel_edp_panel_vdd_on(intel_dp);
 		intel_edp_panel_off(intel_dp);
+
+		/* storing panel power off time */
+		do_gettimeofday(&intel_dp->panel_power_off_timestamp);
 	}
 
 	if (IS_SKYLAKE(dev) || IS_KABYLAKE(dev)) diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c index 0f1eb96..1ca01b1 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -2032,6 +2032,9 @@ static void edp_panel_on(struct intel_dp *intel_dp)
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	u32 pp;
 	i915_reg_t pp_ctrl_reg;
+	u32 panel_power_off_duration;
+	u32 temp_power_cycle_delay;
+
 
 	lockdep_assert_held(&dev_priv->pps_mutex);
 
@@ -2045,8 +2048,22 @@ static void edp_panel_on(struct intel_dp *intel_dp)
 		 "eDP port %c panel power already on\n",
 		 port_name(dp_to_dig_port(intel_dp)->port)))
 		return;
+	/* taking the diffrence of currrent time and panel power off time
+	   and then make panel to wait for T12 if needed */
+	do_gettimeofday(&intel_dp->panel_power_on_timestamp);
+
+	panel_power_off_duration  = (intel_dp->panel_power_on_timestamp.tv_sec-intel_dp->panel_power_off_timestamp.tv_sec) * 1000000 +  intel_dp->panel_power_on_timestamp.tv_usec-intel_dp->panel_power_off_timestamp.tv_usec;
+	panel_power_off_duration = panel_power_off_duration / 1000 ;
+	temp_power_cycle_delay = intel_dp->panel_power_cycle_delay;
+
+	if(panel_power_off_duration >= intel_dp->panel_power_cycle_delay) {
+		intel_dp->panel_power_cycle_delay = 0;
+	} else {
+		intel_dp->panel_power_cycle_delay = intel_dp->panel_power_cycle_delay - panel_power_off_duration;
+	}
 
 	wait_panel_power_cycle(intel_dp);
+	intel_dp->panel_power_cycle_delay = temp_power_cycle_delay;
 
 	pp_ctrl_reg = _pp_ctrl_reg(intel_dp);
 	pp = ironlake_get_pp_control(intel_dp);
@@ -5127,6 +5144,7 @@ static void intel_dp_init_panel_power_timestamps(struct intel_dp *intel_dp)
 	intel_dp->last_power_cycle = jiffies;
 	intel_dp->last_power_on = jiffies;
 	intel_dp->last_backlight_off = jiffies;
+	do_gettimeofday(&intel_dp->panel_power_off_timestamp);
 }
 
 static void
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 76dfa28..66ed2cb 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -769,6 +769,8 @@ struct intel_dp {
 	unsigned long last_power_cycle;
 	unsigned long last_power_on;
 	unsigned long last_backlight_off;
+	struct timeval panel_power_off_timestamp;
+	struct timeval panel_power_on_timestamp;
 
 	struct notifier_block edp_notifier;
 
--
1.9.1

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

Re: [PATCH] drm/i915: edp resume/On time optimization.

[Kumar, Shobhit]

On 12/16/2015 03:46 AM, abhay.kumar@intel.com wrote:
> From: Abhay Kumar <abhay.kumar@intel.com>
>
> Make resume codepath not to wait for panel_power_cycle_delay(t11_t12)
> if this time is already spent in suspend/poweron time.
>
> Signed-off-by: Abhay Kumar <abhay.kumar@intel.com>
> ---
>   drivers/gpu/drm/i915/intel_ddi.c |  3 +++
>   drivers/gpu/drm/i915/intel_dp.c  | 18 ++++++++++++++++++
>   drivers/gpu/drm/i915/intel_drv.h |  2 ++
>   3 files changed, 23 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
> index f00a3c9..d2a5a89 100644
> --- a/drivers/gpu/drm/i915/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/intel_ddi.c
> @@ -2395,6 +2395,9 @@ static void intel_ddi_post_disable(struct intel_encoder *intel_encoder)
>   		intel_dp_sink_dpms(intel_dp, DRM_MODE_DPMS_OFF);
>   		intel_edp_panel_vdd_on(intel_dp);
>   		intel_edp_panel_off(intel_dp);
> +
> +		/* storing panel power off time */
> +		do_gettimeofday(&intel_dp->panel_power_off_timestamp);

Store this in edp_panel_off where "last_power_cycle" is initialized. 
This would be generic optimization and not specific only to ddi.

>   	}
>
>   	if (IS_SKYLAKE(dev) || IS_KABYLAKE(dev))
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 0f1eb96..1ca01b1 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -2032,6 +2032,9 @@ static void edp_panel_on(struct intel_dp *intel_dp)
>   	struct drm_i915_private *dev_priv = dev->dev_private;
>   	u32 pp;
>   	i915_reg_t pp_ctrl_reg;
> +	u32 panel_power_off_duration;
> +	u32 temp_power_cycle_delay;
> +
>
>   	lockdep_assert_held(&dev_priv->pps_mutex);
>
> @@ -2045,8 +2048,22 @@ static void edp_panel_on(struct intel_dp *intel_dp)
>   		 "eDP port %c panel power already on\n",
>   		 port_name(dp_to_dig_port(intel_dp)->port)))
>   		return;
> +	/* taking the diffrence of currrent time and panel power off time
> +	   and then make panel to wait for T12 if needed */
> +	do_gettimeofday(&intel_dp->panel_power_on_timestamp);
> +
> +	panel_power_off_duration  = (intel_dp->panel_power_on_timestamp.tv_sec-intel_dp->panel_power_off_timestamp.tv_sec) * 1000000 +  intel_dp->panel_power_on_timestamp.tv_usec-intel_dp->panel_power_off_timestamp.tv_usec;
> +	panel_power_off_duration = panel_power_off_duration / 1000 ;
> +	temp_power_cycle_delay = intel_dp->panel_power_cycle_delay;
> +
> +	if(panel_power_off_duration >= intel_dp->panel_power_cycle_delay) {
> +		intel_dp->panel_power_cycle_delay = 0;
> +	} else {
> +		intel_dp->panel_power_cycle_delay = intel_dp->panel_power_cycle_delay - panel_power_off_duration;
> +	}
>

Moving all adjustment inside wait_panel_power_cycle looks better to me. 
Also checkout timeval_to_jiffies to simplify the code.

>   	wait_panel_power_cycle(intel_dp);
> +	intel_dp->panel_power_cycle_delay = temp_power_cycle_delay;
>
>   	pp_ctrl_reg = _pp_ctrl_reg(intel_dp);
>   	pp = ironlake_get_pp_control(intel_dp);
> @@ -5127,6 +5144,7 @@ static void intel_dp_init_panel_power_timestamps(struct intel_dp *intel_dp)
>   	intel_dp->last_power_cycle = jiffies;
>   	intel_dp->last_power_on = jiffies;
>   	intel_dp->last_backlight_off = jiffies;
> +	do_gettimeofday(&intel_dp->panel_power_off_timestamp);
>   }
>
>   static void
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 76dfa28..66ed2cb 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -769,6 +769,8 @@ struct intel_dp {
>   	unsigned long last_power_cycle;
>   	unsigned long last_power_on;
>   	unsigned long last_backlight_off;
> +	struct timeval panel_power_off_timestamp;
> +	struct timeval panel_power_on_timestamp;

panel_power_on_timestamp is not needed to be stored in the structure. 
What we need is current time which can be locally obtained and consumed 
in wait_panel_power_cycle

Regards
Shobhit
>
>   	struct notifier_block edp_notifier;
>
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH] drm/i915: edp resume/On time optimization.

[Ville Syrjälä]

On Tue, Dec 15, 2015 at 02:16:38PM -0800, abhay.kumar@intel.com wrote:
> From: Abhay Kumar <abhay.kumar@intel.com>
> 
> Make resume codepath not to wait for panel_power_cycle_delay(t11_t12)
> if this time is already spent in suspend/poweron time.
> 
> Signed-off-by: Abhay Kumar <abhay.kumar@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_ddi.c |  3 +++
>  drivers/gpu/drm/i915/intel_dp.c  | 18 ++++++++++++++++++
>  drivers/gpu/drm/i915/intel_drv.h |  2 ++
>  3 files changed, 23 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
> index f00a3c9..d2a5a89 100644
> --- a/drivers/gpu/drm/i915/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/intel_ddi.c
> @@ -2395,6 +2395,9 @@ static void intel_ddi_post_disable(struct intel_encoder *intel_encoder)
>  		intel_dp_sink_dpms(intel_dp, DRM_MODE_DPMS_OFF);
>  		intel_edp_panel_vdd_on(intel_dp);
>  		intel_edp_panel_off(intel_dp);
> +
> +		/* storing panel power off time */
> +		do_gettimeofday(&intel_dp->panel_power_off_timestamp);

I think what we want to use is CLOCK_BOOTTIME. It's like
MONOTONIC, except it counts the suspend time too.

Initially I figured we'd use REALTIME, and only do the adjustment around
suspend/resume. But actually BOOTTIME should be perfectly safe to use
all the time (changing the system time doesn't affect it). So maybe we just
want to convert the power cycle delay handling entirely over to using
the BOOTTIME clock instead of jiffies?

>  	}
>  
>  	if (IS_SKYLAKE(dev) || IS_KABYLAKE(dev))
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 0f1eb96..1ca01b1 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -2032,6 +2032,9 @@ static void edp_panel_on(struct intel_dp *intel_dp)
>  	struct drm_i915_private *dev_priv = dev->dev_private;
>  	u32 pp;
>  	i915_reg_t pp_ctrl_reg;
> +	u32 panel_power_off_duration;
> +	u32 temp_power_cycle_delay;
> +
>  
>  	lockdep_assert_held(&dev_priv->pps_mutex);
>  
> @@ -2045,8 +2048,22 @@ static void edp_panel_on(struct intel_dp *intel_dp)
>  		 "eDP port %c panel power already on\n",
>  		 port_name(dp_to_dig_port(intel_dp)->port)))
>  		return;
> +	/* taking the diffrence of currrent time and panel power off time
> +	   and then make panel to wait for T12 if needed */
> +	do_gettimeofday(&intel_dp->panel_power_on_timestamp);
> +
> +	panel_power_off_duration  = (intel_dp->panel_power_on_timestamp.tv_sec-intel_dp->panel_power_off_timestamp.tv_sec) * 1000000 +  intel_dp->panel_power_on_timestamp.tv_usec-intel_dp->panel_power_off_timestamp.tv_usec;
> +	panel_power_off_duration = panel_power_off_duration / 1000 ;
> +	temp_power_cycle_delay = intel_dp->panel_power_cycle_delay;
> +
> +	if(panel_power_off_duration >= intel_dp->panel_power_cycle_delay) {
> +		intel_dp->panel_power_cycle_delay = 0;
> +	} else {
> +		intel_dp->panel_power_cycle_delay = intel_dp->panel_power_cycle_delay - panel_power_off_duration;
> +	}
>  
>  	wait_panel_power_cycle(intel_dp);
> +	intel_dp->panel_power_cycle_delay = temp_power_cycle_delay;
>  
>  	pp_ctrl_reg = _pp_ctrl_reg(intel_dp);
>  	pp = ironlake_get_pp_control(intel_dp);
> @@ -5127,6 +5144,7 @@ static void intel_dp_init_panel_power_timestamps(struct intel_dp *intel_dp)
>  	intel_dp->last_power_cycle = jiffies;
>  	intel_dp->last_power_on = jiffies;
>  	intel_dp->last_backlight_off = jiffies;
> +	do_gettimeofday(&intel_dp->panel_power_off_timestamp);
>  }
>  
>  static void
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 76dfa28..66ed2cb 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -769,6 +769,8 @@ struct intel_dp {
>  	unsigned long last_power_cycle;
>  	unsigned long last_power_on;
>  	unsigned long last_backlight_off;
> +	struct timeval panel_power_off_timestamp;
> +	struct timeval panel_power_on_timestamp;
>  
>  	struct notifier_block edp_notifier;
>  
> -- 
> 1.9.1
> 
> _______________________________________________
> 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

Re: [PATCH] drm/i915: edp resume/On time optimization.

[Kumar, Abhay]

Sure. In next patch set will get rid of jiffies altogether and will use getboottime() instead of do_gettimeofday() for panel_power_cycle_delay.

Does this make sense?


-----Original Message-----
From: Ville Syrjälä [mailto:ville.syrjala@linux.intel.com] 
Sent: Wednesday, December 16, 2015 2:56 AM
To: Kumar, Abhay
Cc: Intel-gfx@lists.freedesktop.org
Subject: Re: [Intel-gfx] [PATCH] drm/i915: edp resume/On time optimization.

On Tue, Dec 15, 2015 at 02:16:38PM -0800, abhay.kumar@intel.com wrote:
> From: Abhay Kumar <abhay.kumar@intel.com>
> 
> Make resume codepath not to wait for panel_power_cycle_delay(t11_t12) 
> if this time is already spent in suspend/poweron time.
> 
> Signed-off-by: Abhay Kumar <abhay.kumar@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_ddi.c |  3 +++  
> drivers/gpu/drm/i915/intel_dp.c  | 18 ++++++++++++++++++  
> drivers/gpu/drm/i915/intel_drv.h |  2 ++
>  3 files changed, 23 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/intel_ddi.c 
> b/drivers/gpu/drm/i915/intel_ddi.c
> index f00a3c9..d2a5a89 100644
> --- a/drivers/gpu/drm/i915/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/intel_ddi.c
> @@ -2395,6 +2395,9 @@ static void intel_ddi_post_disable(struct intel_encoder *intel_encoder)
>  		intel_dp_sink_dpms(intel_dp, DRM_MODE_DPMS_OFF);
>  		intel_edp_panel_vdd_on(intel_dp);
>  		intel_edp_panel_off(intel_dp);
> +
> +		/* storing panel power off time */
> +		do_gettimeofday(&intel_dp->panel_power_off_timestamp);

I think what we want to use is CLOCK_BOOTTIME. It's like MONOTONIC, except it counts the suspend time too.

Initially I figured we'd use REALTIME, and only do the adjustment around suspend/resume. But actually BOOTTIME should be perfectly safe to use all the time (changing the system time doesn't affect it). So maybe we just want to convert the power cycle delay handling entirely over to using the BOOTTIME clock instead of jiffies?

>  	}
>  
>  	if (IS_SKYLAKE(dev) || IS_KABYLAKE(dev)) diff --git 
> a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c 
> index 0f1eb96..1ca01b1 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -2032,6 +2032,9 @@ static void edp_panel_on(struct intel_dp *intel_dp)
>  	struct drm_i915_private *dev_priv = dev->dev_private;
>  	u32 pp;
>  	i915_reg_t pp_ctrl_reg;
> +	u32 panel_power_off_duration;
> +	u32 temp_power_cycle_delay;
> +
>  
>  	lockdep_assert_held(&dev_priv->pps_mutex);
>  
> @@ -2045,8 +2048,22 @@ static void edp_panel_on(struct intel_dp *intel_dp)
>  		 "eDP port %c panel power already on\n",
>  		 port_name(dp_to_dig_port(intel_dp)->port)))
>  		return;
> +	/* taking the diffrence of currrent time and panel power off time
> +	   and then make panel to wait for T12 if needed */
> +	do_gettimeofday(&intel_dp->panel_power_on_timestamp);
> +
> +	panel_power_off_duration  = (intel_dp->panel_power_on_timestamp.tv_sec-intel_dp->panel_power_off_timestamp.tv_sec) * 1000000 +  intel_dp->panel_power_on_timestamp.tv_usec-intel_dp->panel_power_off_timestamp.tv_usec;
> +	panel_power_off_duration = panel_power_off_duration / 1000 ;
> +	temp_power_cycle_delay = intel_dp->panel_power_cycle_delay;
> +
> +	if(panel_power_off_duration >= intel_dp->panel_power_cycle_delay) {
> +		intel_dp->panel_power_cycle_delay = 0;
> +	} else {
> +		intel_dp->panel_power_cycle_delay = intel_dp->panel_power_cycle_delay - panel_power_off_duration;
> +	}
>  
>  	wait_panel_power_cycle(intel_dp);
> +	intel_dp->panel_power_cycle_delay = temp_power_cycle_delay;
>  
>  	pp_ctrl_reg = _pp_ctrl_reg(intel_dp);
>  	pp = ironlake_get_pp_control(intel_dp);
> @@ -5127,6 +5144,7 @@ static void intel_dp_init_panel_power_timestamps(struct intel_dp *intel_dp)
>  	intel_dp->last_power_cycle = jiffies;
>  	intel_dp->last_power_on = jiffies;
>  	intel_dp->last_backlight_off = jiffies;
> +	do_gettimeofday(&intel_dp->panel_power_off_timestamp);
>  }
>  
>  static void
> diff --git a/drivers/gpu/drm/i915/intel_drv.h 
> b/drivers/gpu/drm/i915/intel_drv.h
> index 76dfa28..66ed2cb 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -769,6 +769,8 @@ struct intel_dp {
>  	unsigned long last_power_cycle;
>  	unsigned long last_power_on;
>  	unsigned long last_backlight_off;
> +	struct timeval panel_power_off_timestamp;
> +	struct timeval panel_power_on_timestamp;
>  
>  	struct notifier_block edp_notifier;
>  
> --
> 1.9.1
> 
> _______________________________________________
> 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

Re: [PATCH] drm/i915: edp resume/On time optimization.

[Ville Syrjälä]

On Thu, Dec 17, 2015 at 01:04:25AM +0000, Kumar, Abhay wrote:
> Sure. In next patch set will get rid of jiffies altogether and will use getboottime() instead of do_gettimeofday() for panel_power_cycle_delay.
> 
> Does this make sense?

I can't say for sure until I see a patch, but that's my current thinking
yes.

> 
> 
> -----Original Message-----
> From: Ville Syrjälä [mailto:ville.syrjala@linux.intel.com] 
> Sent: Wednesday, December 16, 2015 2:56 AM
> To: Kumar, Abhay
> Cc: Intel-gfx@lists.freedesktop.org
> Subject: Re: [Intel-gfx] [PATCH] drm/i915: edp resume/On time optimization.
> 
> On Tue, Dec 15, 2015 at 02:16:38PM -0800, abhay.kumar@intel.com wrote:
> > From: Abhay Kumar <abhay.kumar@intel.com>
> > 
> > Make resume codepath not to wait for panel_power_cycle_delay(t11_t12) 
> > if this time is already spent in suspend/poweron time.
> > 
> > Signed-off-by: Abhay Kumar <abhay.kumar@intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_ddi.c |  3 +++  
> > drivers/gpu/drm/i915/intel_dp.c  | 18 ++++++++++++++++++  
> > drivers/gpu/drm/i915/intel_drv.h |  2 ++
> >  3 files changed, 23 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_ddi.c 
> > b/drivers/gpu/drm/i915/intel_ddi.c
> > index f00a3c9..d2a5a89 100644
> > --- a/drivers/gpu/drm/i915/intel_ddi.c
> > +++ b/drivers/gpu/drm/i915/intel_ddi.c
> > @@ -2395,6 +2395,9 @@ static void intel_ddi_post_disable(struct intel_encoder *intel_encoder)
> >  		intel_dp_sink_dpms(intel_dp, DRM_MODE_DPMS_OFF);
> >  		intel_edp_panel_vdd_on(intel_dp);
> >  		intel_edp_panel_off(intel_dp);
> > +
> > +		/* storing panel power off time */
> > +		do_gettimeofday(&intel_dp->panel_power_off_timestamp);
> 
> I think what we want to use is CLOCK_BOOTTIME. It's like MONOTONIC, except it counts the suspend time too.
> 
> Initially I figured we'd use REALTIME, and only do the adjustment around suspend/resume. But actually BOOTTIME should be perfectly safe to use all the time (changing the system time doesn't affect it). So maybe we just want to convert the power cycle delay handling entirely over to using the BOOTTIME clock instead of jiffies?
> 
> >  	}
> >  
> >  	if (IS_SKYLAKE(dev) || IS_KABYLAKE(dev)) diff --git 
> > a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c 
> > index 0f1eb96..1ca01b1 100644
> > --- a/drivers/gpu/drm/i915/intel_dp.c
> > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > @@ -2032,6 +2032,9 @@ static void edp_panel_on(struct intel_dp *intel_dp)
> >  	struct drm_i915_private *dev_priv = dev->dev_private;
> >  	u32 pp;
> >  	i915_reg_t pp_ctrl_reg;
> > +	u32 panel_power_off_duration;
> > +	u32 temp_power_cycle_delay;
> > +
> >  
> >  	lockdep_assert_held(&dev_priv->pps_mutex);
> >  
> > @@ -2045,8 +2048,22 @@ static void edp_panel_on(struct intel_dp *intel_dp)
> >  		 "eDP port %c panel power already on\n",
> >  		 port_name(dp_to_dig_port(intel_dp)->port)))
> >  		return;
> > +	/* taking the diffrence of currrent time and panel power off time
> > +	   and then make panel to wait for T12 if needed */
> > +	do_gettimeofday(&intel_dp->panel_power_on_timestamp);
> > +
> > +	panel_power_off_duration  = (intel_dp->panel_power_on_timestamp.tv_sec-intel_dp->panel_power_off_timestamp.tv_sec) * 1000000 +  intel_dp->panel_power_on_timestamp.tv_usec-intel_dp->panel_power_off_timestamp.tv_usec;
> > +	panel_power_off_duration = panel_power_off_duration / 1000 ;
> > +	temp_power_cycle_delay = intel_dp->panel_power_cycle_delay;
> > +
> > +	if(panel_power_off_duration >= intel_dp->panel_power_cycle_delay) {
> > +		intel_dp->panel_power_cycle_delay = 0;
> > +	} else {
> > +		intel_dp->panel_power_cycle_delay = intel_dp->panel_power_cycle_delay - panel_power_off_duration;
> > +	}
> >  
> >  	wait_panel_power_cycle(intel_dp);
> > +	intel_dp->panel_power_cycle_delay = temp_power_cycle_delay;
> >  
> >  	pp_ctrl_reg = _pp_ctrl_reg(intel_dp);
> >  	pp = ironlake_get_pp_control(intel_dp);
> > @@ -5127,6 +5144,7 @@ static void intel_dp_init_panel_power_timestamps(struct intel_dp *intel_dp)
> >  	intel_dp->last_power_cycle = jiffies;
> >  	intel_dp->last_power_on = jiffies;
> >  	intel_dp->last_backlight_off = jiffies;
> > +	do_gettimeofday(&intel_dp->panel_power_off_timestamp);
> >  }
> >  
> >  static void
> > diff --git a/drivers/gpu/drm/i915/intel_drv.h 
> > b/drivers/gpu/drm/i915/intel_drv.h
> > index 76dfa28..66ed2cb 100644
> > --- a/drivers/gpu/drm/i915/intel_drv.h
> > +++ b/drivers/gpu/drm/i915/intel_drv.h
> > @@ -769,6 +769,8 @@ struct intel_dp {
> >  	unsigned long last_power_cycle;
> >  	unsigned long last_power_on;
> >  	unsigned long last_backlight_off;
> > +	struct timeval panel_power_off_timestamp;
> > +	struct timeval panel_power_on_timestamp;
> >  
> >  	struct notifier_block edp_notifier;
> >  
> > --
> > 1.9.1
> > 
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> --
> Ville Syrjälä
> Intel OTC

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