Re: [Intel-gfx] [v2] drm/i915/pps: improve eDP power on flow

2022-11-02 Thread Lee, Shawn C
On Wednesday, November 2, 2022 6:19 PM, Jani Nikula 
 wrote:
>On Tue, 01 Nov 2022, "Lee, Shawn C"  wrote:
>> On Tue, Nov. 1, 2022, 1:43 p.m, Jani Nikula  
>> wrote:
>>>On Tue, 01 Nov 2022, "Lee, Shawn C"  wrote:
 On Tuesday, November 1, 2022 5:53 PM, Jani Nikula 
  wrote:
>On Mon, 24 Oct 2022, Lee Shawn C  wrote:
>> A panel power off cycle delay always append before turn eDP on.
>> Driver should check last_power_on and last_backlight_off before 
>> insert this delay. If these values are the same, it means eDP was 
>> off until now and driver can bypass power off cycle delay to save 
>> some times to speed up eDP power on sequence.
>>
>> v2: fix commit messages
>
>There are more changes here than what the changelog says, but the previous 
>review comments were not answered [1].
>

 I'm sorry that lose the question in [1]. 

 "But someone else may have turned it off just before we were handed 
 control, we have no idea."
 This is the situation from Ville's comment. Agree that we don't know when 
 panel will be powered off.
 In my opinion, eDP panel should not be turned off before i915 take it 
 over. If it was turned on or off by standard contorl (ex: modeset).
 last_power_on and last_backlight_off would not be the same. So this change 
 should be safe.
>>>
>>>I think it's pretty much a hard requirement we respect the panel 
>>>delays at i915 probe. If we don't know, we don't know, and we can't 
>>>make assumptions.
>>>
>>>If the goal is to speed up boot, you should ensure 1) the pre-os 
>>>enables the display, and 2) i915 can take over the display without a 
>>>modeset and a panel power cycle.
>>>
>>
>> After boot into kernel. It seems there are two cases we will see. 
>> 1) If eDP display did not enable at pre-os stage, this patch can save power 
>> cycle time. 
>> 2) If eDP display was enabled at pre-os, because of cdclk was setting to max 
>> freq always.
>>i915 driver will trigger modeset to reduce cdclk freq and run power 
>> off/on cycle.
>>At this case power cycle delay will not be ignored.
>
>In case 2, the effort should probably be spent on hardware take over using the 
>same cdclk as it was.
>I thought this was already the case, but maybe I'm wrong and/or there are 
>corner cases.
>

When cdclk was the same, it means driver will not trigger modeset and keep the 
setting from pre-os.
If so, this behavior sound like fastboot mode enabled.

>> So this patch can only benefit at case #1 (eDP did not enable at 
>> pre-os stage). And it is what we need. :)
>
>I understand a typical T12 min (i.e. from Vcc power down to up again) could 
>be, say, 500 ms and it's a long time to wait. Especially if the wait happens 
>in output init which is all serial and synchronous in probe.
>
>However, you're basically asking us to potentially violate panel timings. It 
>just doesn't strike me as an obviusly good idea.
>

>From my point of view, pre-os initialization already take 2~3 seconds (panel 
>power is off). Kernel log in below shows the first time driver try to enable 
>eDP pwoer in case 1.

[0.957991] i915 :00:02.0: [drm:intel_pps_vdd_on_unlocked] Turning 
[ENCODER:235:DDI A/PHY A] VDD on
[0.958021] i915 :00:02.0: [drm:wait_panel_power_cycle] Wait for panel 
power cycle

eDP panel power never turn on before time [0.958021]. So the panel power 
already off over 2 (pre-os) + 1 (kernel) seconds at least.
In my opinion, 3 seconds already over the power cycle delay setting. That's why 
i'm thinking maybe we don't need this additional 600ms delay in this case.

>It might be a good idea to file an issue at fdo gitlab [1] and attach a dmesg 
>with drm.debug=14 from boot to at least the first modeset so we can actually 
>see what the delays are and where the time is spent.

Here is the gitlab issue and you can find kernel log in it. Thanks!
https://gitlab.freedesktop.org/drm/intel/-/issues/7417

Best regards,
Shawn

>
>
>BR,
>Jani.
>
>
>[1] https://gitlab.freedesktop.org/drm/intel/issues/new
>
>
>
>
>>
>> Best regards,
>> Shawn
>>
>>>
>>>BR,
>>>Jani.
>>>
>>>

 Best regards,
 Shawn

>
>BR,
>Jani.
>
>
>[1] https://lore.kernel.org/r/y1afudawfvtac...@intel.com
>
>>
>> Cc: Shankar Uma 
>> Cc: Jani Nikula 
>> Cc: Ville Syrjälä 
>> Signed-off-by: Lee Shawn C 
>> ---
>>  drivers/gpu/drm/i915/display/intel_pps.c | 9 -
>>  1 file changed, 8 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/i915/display/intel_pps.c
>> b/drivers/gpu/drm/i915/display/intel_pps.c
>> index 21944f5bf3a8..290473ec70d5 100644
>> --- a/drivers/gpu/drm/i915/display/intel_pps.c
>> +++ b/drivers/gpu/drm/i915/display/intel_pps.c
>> @@ -509,6 +509,13 @@ static void wait_panel_power_cycle(struct intel_dp 
>> *intel_dp)
>>  ktime_t panel_power_on_time;
>>  s64 panel_power_off_duration;

Re: [Intel-gfx] [v2] drm/i915/pps: improve eDP power on flow

2022-11-02 Thread Jani Nikula
On Tue, 01 Nov 2022, "Lee, Shawn C"  wrote:
> On Tue, Nov. 1, 2022, 1:43 p.m, Jani Nikula  
> wrote:
>>On Tue, 01 Nov 2022, "Lee, Shawn C"  wrote:
>>> On Tuesday, November 1, 2022 5:53 PM, Jani Nikula 
>>>  wrote:
On Mon, 24 Oct 2022, Lee Shawn C  wrote:
> A panel power off cycle delay always append before turn eDP on.
> Driver should check last_power_on and last_backlight_off before insert 
> this delay. If these values are the same, it means eDP was off until 
> now and driver can bypass power off cycle delay to save some times to 
> speed up eDP power on sequence.
>
> v2: fix commit messages

There are more changes here than what the changelog says, but the previous 
review comments were not answered [1].

>>>
>>> I'm sorry that lose the question in [1]. 
>>>
>>> "But someone else may have turned it off just before we were handed 
>>> control, we have no idea."
>>> This is the situation from Ville's comment. Agree that we don't know when 
>>> panel will be powered off.
>>> In my opinion, eDP panel should not be turned off before i915 take it over. 
>>> If it was turned on or off by standard contorl (ex: modeset).
>>> last_power_on and last_backlight_off would not be the same. So this change 
>>> should be safe.
>>
>>I think it's pretty much a hard requirement we respect the panel delays
>>at i915 probe. If we don't know, we don't know, and we can't make
>>assumptions.
>>
>>If the goal is to speed up boot, you should ensure 1) the pre-os enables
>>the display, and 2) i915 can take over the display without a modeset and
>>a panel power cycle.
>>
>
> After boot into kernel. It seems there are two cases we will see. 
> 1) If eDP display did not enable at pre-os stage, this patch can save power 
> cycle time. 
> 2) If eDP display was enabled at pre-os, because of cdclk was setting to max 
> freq always.
>i915 driver will trigger modeset to reduce cdclk freq and run power off/on 
> cycle.
>At this case power cycle delay will not be ignored.

In case 2, the effort should probably be spent on hardware take over
using the same cdclk as it was. I thought this was already the case, but
maybe I'm wrong and/or there are corner cases.

> So this patch can only benefit at case #1 (eDP did not enable at pre-os 
> stage). And it is what we need. :)

I understand a typical T12 min (i.e. from Vcc power down to up again)
could be, say, 500 ms and it's a long time to wait. Especially if the
wait happens in output init which is all serial and synchronous in
probe.

However, you're basically asking us to potentially violate panel
timings. It just doesn't strike me as an obviusly good idea.

It might be a good idea to file an issue at fdo gitlab [1] and attach a
dmesg with drm.debug=14 from boot to at least the first modeset so we
can actually see what the delays are and where the time is spent.


BR,
Jani.


[1] https://gitlab.freedesktop.org/drm/intel/issues/new




>
> Best regards,
> Shawn
>
>>
>>BR,
>>Jani.
>>
>>
>>>
>>> Best regards,
>>> Shawn
>>>

BR,
Jani.


[1] https://lore.kernel.org/r/y1afudawfvtac...@intel.com

>
> Cc: Shankar Uma 
> Cc: Jani Nikula 
> Cc: Ville Syrjälä 
> Signed-off-by: Lee Shawn C 
> ---
>  drivers/gpu/drm/i915/display/intel_pps.c | 9 -
>  1 file changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_pps.c 
> b/drivers/gpu/drm/i915/display/intel_pps.c
> index 21944f5bf3a8..290473ec70d5 100644
> --- a/drivers/gpu/drm/i915/display/intel_pps.c
> +++ b/drivers/gpu/drm/i915/display/intel_pps.c
> @@ -509,6 +509,13 @@ static void wait_panel_power_cycle(struct intel_dp 
> *intel_dp)
>   ktime_t panel_power_on_time;
>   s64 panel_power_off_duration;
>  
> + /* When last_power_on equal to last_backlight_off, it means driver did 
> not
> +  * turn on or off eDP panel so far. So we can bypass power cycle delay 
> to
> +  * save some times here.
> +  */
> + if (intel_dp->pps.last_power_on == intel_dp->pps.last_backlight_off)
> + return;
> +
>   drm_dbg_kms(>drm, "Wait for panel power cycle\n");
>  
>   /* take the difference of current time and panel power off time @@ 
> -1100,7 +1107,7 @@ static void pps_init_timestamps(struct intel_dp 
> *intel_dp)  {
>   intel_dp->pps.panel_power_off_time = ktime_get_boottime();
>   intel_dp->pps.last_power_on = jiffies;
> - intel_dp->pps.last_backlight_off = jiffies;
> + intel_dp->pps.last_backlight_off = intel_dp->pps.last_power_on;
>  }
>  
>  static void

--
Jani Nikula, Intel Open Source Graphics Center

-- 
Jani Nikula, Intel Open Source Graphics Center


Re: [Intel-gfx] [v2] drm/i915/pps: improve eDP power on flow

2022-11-01 Thread Lee, Shawn C
On Tue, Nov. 1, 2022, 1:43 p.m, Jani Nikula  wrote:
>On Tue, 01 Nov 2022, "Lee, Shawn C"  wrote:
>> On Tuesday, November 1, 2022 5:53 PM, Jani Nikula 
>>  wrote:
>>>On Mon, 24 Oct 2022, Lee Shawn C  wrote:
 A panel power off cycle delay always append before turn eDP on.
 Driver should check last_power_on and last_backlight_off before 
 insert this delay. If these values are the same, it means eDP was 
 off until now and driver can bypass power off cycle delay to save 
 some times to speed up eDP power on sequence.

 v2: fix commit messages
>>>
>>>There are more changes here than what the changelog says, but the previous 
>>>review comments were not answered [1].
>>>
>>
>> I'm sorry that lose the question in [1]. 
>>
>> "But someone else may have turned it off just before we were handed control, 
>> we have no idea."
>> This is the situation from Ville's comment. Agree that we don't know when 
>> panel will be powered off.
>> In my opinion, eDP panel should not be turned off before i915 take it over. 
>> If it was turned on or off by standard contorl (ex: modeset).
>> last_power_on and last_backlight_off would not be the same. So this change 
>> should be safe.
>
>I think it's pretty much a hard requirement we respect the panel delays 
>at i915 probe. If we don't know, we don't know, and we can't make 
>assumptions.
>
>If the goal is to speed up boot, you should ensure 1) the pre-os 
>enables the display, and 2) i915 can take over the display without a 
>modeset and a panel power cycle.
>

After boot into kernel. It seems there are two cases we will see. 
1) If eDP display did not enable at pre-os stage, this patch can save power 
cycle time. 
2) If eDP display was enabled at pre-os, because of cdclk was setting to max 
freq always.
   i915 driver will trigger modeset to reduce cdclk freq and run power off/on 
cycle.
   At this case power cycle delay will not be ignored.

So this patch can only benefit at case #1 (eDP did not enable at pre-os stage). 
And it is what we need. :)

Best regards,
Shawn

>
>BR,
>Jani.
>
>
>>
>> Best regards,
>> Shawn
>>
>>>
>>>BR,
>>>Jani.
>>>
>>>
>>>[1] https://lore.kernel.org/r/y1afudawfvtac...@intel.com
>>>

 Cc: Shankar Uma 
 Cc: Jani Nikula 
 Cc: Ville Syrjälä 
 Signed-off-by: Lee Shawn C 
 ---
  drivers/gpu/drm/i915/display/intel_pps.c | 9 -
  1 file changed, 8 insertions(+), 1 deletion(-)

 diff --git a/drivers/gpu/drm/i915/display/intel_pps.c
 b/drivers/gpu/drm/i915/display/intel_pps.c
 index 21944f5bf3a8..290473ec70d5 100644
 --- a/drivers/gpu/drm/i915/display/intel_pps.c
 +++ b/drivers/gpu/drm/i915/display/intel_pps.c
 @@ -509,6 +509,13 @@ static void wait_panel_power_cycle(struct intel_dp 
 *intel_dp)
ktime_t panel_power_on_time;
s64 panel_power_off_duration;
  
 +  /* When last_power_on equal to last_backlight_off, it means driver did 
 not
 +   * turn on or off eDP panel so far. So we can bypass power cycle delay 
 to
 +   * save some times here.
 +   */
 +  if (intel_dp->pps.last_power_on == intel_dp->pps.last_backlight_off)
 +  return;
 +
drm_dbg_kms(>drm, "Wait for panel power cycle\n");
  
/* take the difference of current time and panel power off time 
 @@
 -1100,7 +1107,7 @@ static void pps_init_timestamps(struct intel_dp
 *intel_dp)  {
intel_dp->pps.panel_power_off_time = ktime_get_boottime();
intel_dp->pps.last_power_on = jiffies;
 -  intel_dp->pps.last_backlight_off = jiffies;
 +  intel_dp->pps.last_backlight_off = intel_dp->pps.last_power_on;
  }
  
  static void
>>>
>>>--
>>>Jani Nikula, Intel Open Source Graphics Center


Re: [Intel-gfx] [v2] drm/i915/pps: improve eDP power on flow

2022-11-01 Thread Lee, Shawn C
On Tue, Nov. 1, 2022, 1:43 p.m, Jani Nikula  wrote:
>On Tue, 01 Nov 2022, "Lee, Shawn C"  wrote:
>> On Tuesday, November 1, 2022 5:53 PM, Jani Nikula 
>>  wrote:
>>>On Mon, 24 Oct 2022, Lee Shawn C  wrote:
 A panel power off cycle delay always append before turn eDP on.
 Driver should check last_power_on and last_backlight_off before insert 
 this delay. If these values are the same, it means eDP was off until 
 now and driver can bypass power off cycle delay to save some times to 
 speed up eDP power on sequence.

 v2: fix commit messages
>>>
>>>There are more changes here than what the changelog says, but the previous 
>>>review comments were not answered [1].
>>>
>>
>> I'm sorry that lose the question in [1]. 
>>
>> "But someone else may have turned it off just before we were handed control, 
>> we have no idea."
>> This is the situation from Ville's comment. Agree that we don't know when 
>> panel will be powered off.
>> In my opinion, eDP panel should not be turned off before i915 take it over. 
>> If it was turned on or off by standard contorl (ex: modeset).
>> last_power_on and last_backlight_off would not be the same. So this change 
>> should be safe.
>
>I think it's pretty much a hard requirement we respect the panel delays
>at i915 probe. If we don't know, we don't know, and we can't make
>assumptions.
>
>If the goal is to speed up boot, you should ensure 1) the pre-os enables
>the display, and 2) i915 can take over the display without a modeset and
>a panel power cycle.
>

After boot into kernel. It seems there are two cases we will see. 
1) If eDP display did not enable at pre-os stage, this patch can save power 
cycle time. 
2) If eDP display was enabled at pre-os, because of cdclk was setting to max 
freq always.
   i915 driver will trigger modeset to reduce cdclk freq and run power off/on 
cycle.
   At this case power cycle delay will not be ignored.

So this patch can only benefit at case #1 (eDP did not enable at pre-os stage). 
And it is what we need. :)

Best regards,
Shawn

>
>BR,
>Jani.
>
>
>>
>> Best regards,
>> Shawn
>>
>>>
>>>BR,
>>>Jani.
>>>
>>>
>>>[1] https://lore.kernel.org/r/y1afudawfvtac...@intel.com
>>>

 Cc: Shankar Uma 
 Cc: Jani Nikula 
 Cc: Ville Syrjälä 
 Signed-off-by: Lee Shawn C 
 ---
  drivers/gpu/drm/i915/display/intel_pps.c | 9 -
  1 file changed, 8 insertions(+), 1 deletion(-)

 diff --git a/drivers/gpu/drm/i915/display/intel_pps.c 
 b/drivers/gpu/drm/i915/display/intel_pps.c
 index 21944f5bf3a8..290473ec70d5 100644
 --- a/drivers/gpu/drm/i915/display/intel_pps.c
 +++ b/drivers/gpu/drm/i915/display/intel_pps.c
 @@ -509,6 +509,13 @@ static void wait_panel_power_cycle(struct intel_dp 
 *intel_dp)
ktime_t panel_power_on_time;
s64 panel_power_off_duration;
  
 +  /* When last_power_on equal to last_backlight_off, it means driver did 
 not
 +   * turn on or off eDP panel so far. So we can bypass power cycle delay 
 to
 +   * save some times here.
 +   */
 +  if (intel_dp->pps.last_power_on == intel_dp->pps.last_backlight_off)
 +  return;
 +
drm_dbg_kms(>drm, "Wait for panel power cycle\n");
  
/* take the difference of current time and panel power off time @@ 
 -1100,7 +1107,7 @@ static void pps_init_timestamps(struct intel_dp 
 *intel_dp)  {
intel_dp->pps.panel_power_off_time = ktime_get_boottime();
intel_dp->pps.last_power_on = jiffies;
 -  intel_dp->pps.last_backlight_off = jiffies;
 +  intel_dp->pps.last_backlight_off = intel_dp->pps.last_power_on;
  }
  
  static void
>>>
>>>--
>>>Jani Nikula, Intel Open Source Graphics Center


Re: [Intel-gfx] [v2] drm/i915/pps: improve eDP power on flow

2022-11-01 Thread Jani Nikula
On Tue, 01 Nov 2022, "Lee, Shawn C"  wrote:
> On Tuesday, November 1, 2022 5:53 PM, Jani Nikula 
>  wrote:
>>On Mon, 24 Oct 2022, Lee Shawn C  wrote:
>>> A panel power off cycle delay always append before turn eDP on.
>>> Driver should check last_power_on and last_backlight_off before insert 
>>> this delay. If these values are the same, it means eDP was off until 
>>> now and driver can bypass power off cycle delay to save some times to 
>>> speed up eDP power on sequence.
>>>
>>> v2: fix commit messages
>>
>>There are more changes here than what the changelog says, but the previous 
>>review comments were not answered [1].
>>
>
> I'm sorry that lose the question in [1]. 
>
> "But someone else may have turned it off just before we were handed control, 
> we have no idea."
> This is the situation from Ville's comment. Agree that we don't know when 
> panel will be powered off.
> In my opinion, eDP panel should not be turned off before i915 take it over. 
> If it was turned on or off by standard contorl (ex: modeset).
> last_power_on and last_backlight_off would not be the same. So this change 
> should be safe.

I think it's pretty much a hard requirement we respect the panel delays
at i915 probe. If we don't know, we don't know, and we can't make
assumptions.

If the goal is to speed up boot, you should ensure 1) the pre-os enables
the display, and 2) i915 can take over the display without a modeset and
a panel power cycle.


BR,
Jani.


>
> Best regards,
> Shawn
>
>>
>>BR,
>>Jani.
>>
>>
>>[1] https://lore.kernel.org/r/y1afudawfvtac...@intel.com
>>
>>>
>>> Cc: Shankar Uma 
>>> Cc: Jani Nikula 
>>> Cc: Ville Syrjälä 
>>> Signed-off-by: Lee Shawn C 
>>> ---
>>>  drivers/gpu/drm/i915/display/intel_pps.c | 9 -
>>>  1 file changed, 8 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/gpu/drm/i915/display/intel_pps.c 
>>> b/drivers/gpu/drm/i915/display/intel_pps.c
>>> index 21944f5bf3a8..290473ec70d5 100644
>>> --- a/drivers/gpu/drm/i915/display/intel_pps.c
>>> +++ b/drivers/gpu/drm/i915/display/intel_pps.c
>>> @@ -509,6 +509,13 @@ static void wait_panel_power_cycle(struct intel_dp 
>>> *intel_dp)
>>> ktime_t panel_power_on_time;
>>> s64 panel_power_off_duration;
>>>  
>>> +   /* When last_power_on equal to last_backlight_off, it means driver did 
>>> not
>>> +* turn on or off eDP panel so far. So we can bypass power cycle delay 
>>> to
>>> +* save some times here.
>>> +*/
>>> +   if (intel_dp->pps.last_power_on == intel_dp->pps.last_backlight_off)
>>> +   return;
>>> +
>>> drm_dbg_kms(>drm, "Wait for panel power cycle\n");
>>>  
>>> /* take the difference of current time and panel power off time @@ 
>>> -1100,7 +1107,7 @@ static void pps_init_timestamps(struct intel_dp 
>>> *intel_dp)  {
>>> intel_dp->pps.panel_power_off_time = ktime_get_boottime();
>>> intel_dp->pps.last_power_on = jiffies;
>>> -   intel_dp->pps.last_backlight_off = jiffies;
>>> +   intel_dp->pps.last_backlight_off = intel_dp->pps.last_power_on;
>>>  }
>>>  
>>>  static void
>>
>>--
>>Jani Nikula, Intel Open Source Graphics Center

-- 
Jani Nikula, Intel Open Source Graphics Center


Re: [Intel-gfx] [v2] drm/i915/pps: improve eDP power on flow

2022-11-01 Thread Lee, Shawn C


On Tuesday, November 1, 2022 5:53 PM, Jani Nikula  
wrote:
>On Mon, 24 Oct 2022, Lee Shawn C  wrote:
>> A panel power off cycle delay always append before turn eDP on.
>> Driver should check last_power_on and last_backlight_off before insert 
>> this delay. If these values are the same, it means eDP was off until 
>> now and driver can bypass power off cycle delay to save some times to 
>> speed up eDP power on sequence.
>>
>> v2: fix commit messages
>
>There are more changes here than what the changelog says, but the previous 
>review comments were not answered [1].
>

I'm sorry that lose the question in [1]. 

"But someone else may have turned it off just before we were handed control, we 
have no idea."
This is the situation from Ville's comment. Agree that we don't know when panel 
will be powered off.
In my opinion, eDP panel should not be turned off before i915 take it over. If 
it was turned on or off by standard contorl (ex: modeset).
last_power_on and last_backlight_off would not be the same. So this change 
should be safe.

Best regards,
Shawn

>
>BR,
>Jani.
>
>
>[1] https://lore.kernel.org/r/y1afudawfvtac...@intel.com
>
>>
>> Cc: Shankar Uma 
>> Cc: Jani Nikula 
>> Cc: Ville Syrjälä 
>> Signed-off-by: Lee Shawn C 
>> ---
>>  drivers/gpu/drm/i915/display/intel_pps.c | 9 -
>>  1 file changed, 8 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/i915/display/intel_pps.c 
>> b/drivers/gpu/drm/i915/display/intel_pps.c
>> index 21944f5bf3a8..290473ec70d5 100644
>> --- a/drivers/gpu/drm/i915/display/intel_pps.c
>> +++ b/drivers/gpu/drm/i915/display/intel_pps.c
>> @@ -509,6 +509,13 @@ static void wait_panel_power_cycle(struct intel_dp 
>> *intel_dp)
>>  ktime_t panel_power_on_time;
>>  s64 panel_power_off_duration;
>>  
>> +/* When last_power_on equal to last_backlight_off, it means driver did 
>> not
>> + * turn on or off eDP panel so far. So we can bypass power cycle delay 
>> to
>> + * save some times here.
>> + */
>> +if (intel_dp->pps.last_power_on == intel_dp->pps.last_backlight_off)
>> +return;
>> +
>>  drm_dbg_kms(>drm, "Wait for panel power cycle\n");
>>  
>>  /* take the difference of current time and panel power off time @@ 
>> -1100,7 +1107,7 @@ static void pps_init_timestamps(struct intel_dp 
>> *intel_dp)  {
>>  intel_dp->pps.panel_power_off_time = ktime_get_boottime();
>>  intel_dp->pps.last_power_on = jiffies;
>> -intel_dp->pps.last_backlight_off = jiffies;
>> +intel_dp->pps.last_backlight_off = intel_dp->pps.last_power_on;
>>  }
>>  
>>  static void
>
>--
>Jani Nikula, Intel Open Source Graphics Center


Re: [Intel-gfx] [v2] drm/i915/pps: improve eDP power on flow

2022-11-01 Thread Jani Nikula
On Mon, 24 Oct 2022, Lee Shawn C  wrote:
> A panel power off cycle delay always append before turn eDP on.
> Driver should check last_power_on and last_backlight_off before
> insert this delay. If these values are the same, it means eDP
> was off until now and driver can bypass power off cycle delay
> to save some times to speed up eDP power on sequence.
>
> v2: fix commit messages

There are more changes here than what the changelog says, but the
previous review comments were not answered [1].


BR,
Jani.


[1] https://lore.kernel.org/r/y1afudawfvtac...@intel.com

>
> Cc: Shankar Uma 
> Cc: Jani Nikula 
> Cc: Ville Syrjälä 
> Signed-off-by: Lee Shawn C 
> ---
>  drivers/gpu/drm/i915/display/intel_pps.c | 9 -
>  1 file changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_pps.c 
> b/drivers/gpu/drm/i915/display/intel_pps.c
> index 21944f5bf3a8..290473ec70d5 100644
> --- a/drivers/gpu/drm/i915/display/intel_pps.c
> +++ b/drivers/gpu/drm/i915/display/intel_pps.c
> @@ -509,6 +509,13 @@ static void wait_panel_power_cycle(struct intel_dp 
> *intel_dp)
>   ktime_t panel_power_on_time;
>   s64 panel_power_off_duration;
>  
> + /* When last_power_on equal to last_backlight_off, it means driver did 
> not
> +  * turn on or off eDP panel so far. So we can bypass power cycle delay 
> to
> +  * save some times here.
> +  */
> + if (intel_dp->pps.last_power_on == intel_dp->pps.last_backlight_off)
> + return;
> +
>   drm_dbg_kms(>drm, "Wait for panel power cycle\n");
>  
>   /* take the difference of current time and panel power off time
> @@ -1100,7 +1107,7 @@ static void pps_init_timestamps(struct intel_dp 
> *intel_dp)
>  {
>   intel_dp->pps.panel_power_off_time = ktime_get_boottime();
>   intel_dp->pps.last_power_on = jiffies;
> - intel_dp->pps.last_backlight_off = jiffies;
> + intel_dp->pps.last_backlight_off = intel_dp->pps.last_power_on;
>  }
>  
>  static void

-- 
Jani Nikula, Intel Open Source Graphics Center


[Intel-gfx] [v2] drm/i915/pps: improve eDP power on flow

2022-10-24 Thread Lee Shawn C
A panel power off cycle delay always append before turn eDP on.
Driver should check last_power_on and last_backlight_off before
insert this delay. If these values are the same, it means eDP
was off until now and driver can bypass power off cycle delay
to save some times to speed up eDP power on sequence.

v2: fix commit messages

Cc: Shankar Uma 
Cc: Jani Nikula 
Cc: Ville Syrjälä 
Signed-off-by: Lee Shawn C 
---
 drivers/gpu/drm/i915/display/intel_pps.c | 9 -
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/display/intel_pps.c 
b/drivers/gpu/drm/i915/display/intel_pps.c
index 21944f5bf3a8..290473ec70d5 100644
--- a/drivers/gpu/drm/i915/display/intel_pps.c
+++ b/drivers/gpu/drm/i915/display/intel_pps.c
@@ -509,6 +509,13 @@ static void wait_panel_power_cycle(struct intel_dp 
*intel_dp)
ktime_t panel_power_on_time;
s64 panel_power_off_duration;
 
+   /* When last_power_on equal to last_backlight_off, it means driver did 
not
+* turn on or off eDP panel so far. So we can bypass power cycle delay 
to
+* save some times here.
+*/
+   if (intel_dp->pps.last_power_on == intel_dp->pps.last_backlight_off)
+   return;
+
drm_dbg_kms(>drm, "Wait for panel power cycle\n");
 
/* take the difference of current time and panel power off time
@@ -1100,7 +1107,7 @@ static void pps_init_timestamps(struct intel_dp *intel_dp)
 {
intel_dp->pps.panel_power_off_time = ktime_get_boottime();
intel_dp->pps.last_power_on = jiffies;
-   intel_dp->pps.last_backlight_off = jiffies;
+   intel_dp->pps.last_backlight_off = intel_dp->pps.last_power_on;
 }
 
 static void
-- 
2.17.1