diff mbox

drm/i915: edp resume/On time optimization.

Message ID 1450747132-29893-1-git-send-email-abhay.kumar@intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Kumar, Abhay Dec. 22, 2015, 1:18 a.m. UTC
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(-)

Comments

Kumar, Abhay Jan. 5, 2016, 9:14 p.m. UTC | #1
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
Ville Syrjälä Jan. 7, 2016, 6:15 p.m. UTC | #2
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
Kumar, Abhay Jan. 8, 2016, 2:27 a.m. UTC | #3
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
Ville Syrjälä Jan. 8, 2016, 12:55 p.m. UTC | #4
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
diff mbox

Patch

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;