| Message ID | 20221024064004.1879-1-shawn.c.lee@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/i915/pps: improve eDP power on flow. | expand |
On Mon, Oct 24, 2022 at 02:40:04PM +0800, Lee Shawn C wrote: > Driver always apply panel power off cycle delay before eDP enable. > If eDP display was enabled at pre-os stage, driver would always > trigger modeset to optimize cdclk setting after boot into kernel. > So last_power_on and last_backlight_off value will be updated. > > We can check last_power_on and last_backlight_off before insert > panel power cycle delay. If these values are the same, it means eDP > was off until now and driver should bypass this delay to save > some times to speed up eDP power on sequence. > > Cc: Shankar Uma <uma.shankar@intel.com> > Cc: Jani Nikula <jani.nikula@linux.intel.com> > Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> > Signed-off-by: Lee Shawn C <shawn.c.lee@intel.com> > --- > drivers/gpu/drm/i915/display/intel_pps.c | 13 +++++++++++-- > 1 file changed, 11 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/i915/display/intel_pps.c b/drivers/gpu/drm/i915/display/intel_pps.c > index 21944f5bf3a8..f3485a4fbfd0 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. But someone else may have turned it off just before we were handed control, we have no idea. The pessimistic estimate is the safe one. Also I don't understand what this has to do with the story in the comit message. In that story you say eDP was already on, so the power cycle delay is not relevant at all. /me confused. > 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(&i915->drm, "Wait for panel power cycle\n"); > > /* take the difference of current time and panel power off time > @@ -1098,9 +1105,11 @@ bool intel_pps_have_panel_power_or_vdd(struct intel_dp *intel_dp) > > static void pps_init_timestamps(struct intel_dp *intel_dp) > { > + unsigned long tmp_jiffies = jiffies; > + > 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_power_on = tmp_jiffies; > + intel_dp->pps.last_backlight_off = tmp_jiffies; > } > > static void > -- > 2.17.1
On Monday, October 24, 2022 9:17 PM, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote: >On Mon, Oct 24, 2022 at 02:40:04PM +0800, Lee Shawn C wrote: >> Driver always apply panel power off cycle delay before eDP enable. >> If eDP display was enabled at pre-os stage, driver would always >> trigger modeset to optimize cdclk setting after boot into kernel. >> So last_power_on and last_backlight_off value will be updated. >> >> We can check last_power_on and last_backlight_off before insert panel >> power cycle delay. If these values are the same, it means eDP was off >> until now and driver should bypass this delay to save some times to >> speed up eDP power on sequence. >> >> Cc: Shankar Uma <uma.shankar@intel.com> >> Cc: Jani Nikula <jani.nikula@linux.intel.com> >> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> >> Signed-off-by: Lee Shawn C <shawn.c.lee@intel.com> >> --- >> drivers/gpu/drm/i915/display/intel_pps.c | 13 +++++++++++-- >> 1 file changed, 11 insertions(+), 2 deletions(-) >> >> diff --git a/drivers/gpu/drm/i915/display/intel_pps.c >> b/drivers/gpu/drm/i915/display/intel_pps.c >> index 21944f5bf3a8..f3485a4fbfd0 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. > >But someone else may have turned it off just before we were handed control, we have no idea. The pessimistic estimate is the safe one. Thanks! It looks to me this situation should not be happpened. Right? > >Also I don't understand what this has to do with the story in the comit message. In that story you say eDP was already on, so the power cycle delay is not relevant at all. /me confused. > I will modify commit messages and send patch v2 later. Best regards, Shawn >> 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(&i915->drm, "Wait for panel power cycle\n"); >> >> /* take the difference of current time and panel power off time @@ >> -1098,9 +1105,11 @@ bool intel_pps_have_panel_power_or_vdd(struct >> intel_dp *intel_dp) >> >> static void pps_init_timestamps(struct intel_dp *intel_dp) { >> + unsigned long tmp_jiffies = jiffies; >> + >> 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_power_on = tmp_jiffies; >> + intel_dp->pps.last_backlight_off = tmp_jiffies; >> } >> >> static void >> -- >> 2.17.1 > >-- >Ville Syrjälä >Intel >
On Mon, Oct 24, 2022 at 01:47:56PM +0000, Lee, Shawn C wrote: > > On Monday, October 24, 2022 9:17 PM, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote: > >On Mon, Oct 24, 2022 at 02:40:04PM +0800, Lee Shawn C wrote: > >> Driver always apply panel power off cycle delay before eDP enable. > >> If eDP display was enabled at pre-os stage, driver would always > >> trigger modeset to optimize cdclk setting after boot into kernel. > >> So last_power_on and last_backlight_off value will be updated. > >> > >> We can check last_power_on and last_backlight_off before insert panel > >> power cycle delay. If these values are the same, it means eDP was off > >> until now and driver should bypass this delay to save some times to > >> speed up eDP power on sequence. > >> > >> Cc: Shankar Uma <uma.shankar@intel.com> > >> Cc: Jani Nikula <jani.nikula@linux.intel.com> > >> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> > >> Signed-off-by: Lee Shawn C <shawn.c.lee@intel.com> > >> --- > >> drivers/gpu/drm/i915/display/intel_pps.c | 13 +++++++++++-- > >> 1 file changed, 11 insertions(+), 2 deletions(-) > >> > >> diff --git a/drivers/gpu/drm/i915/display/intel_pps.c > >> b/drivers/gpu/drm/i915/display/intel_pps.c > >> index 21944f5bf3a8..f3485a4fbfd0 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. > > > >But someone else may have turned it off just before we were handed control, we have no idea. The pessimistic estimate is the safe one. > > Thanks! It looks to me this situation should not be happpened. Right? What situation? We have no idea when the panel was powered off, so we have to make a pessimistic estimate. > > > > >Also I don't understand what this has to do with the story in the comit message. In that story you say eDP was already on, so the power cycle delay is not relevant at all. /me confused. > > > > I will modify commit messages and send patch v2 later. > > Best regards, > Shawn > > >> 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(&i915->drm, "Wait for panel power cycle\n"); > >> > >> /* take the difference of current time and panel power off time @@ > >> -1098,9 +1105,11 @@ bool intel_pps_have_panel_power_or_vdd(struct > >> intel_dp *intel_dp) > >> > >> static void pps_init_timestamps(struct intel_dp *intel_dp) { > >> + unsigned long tmp_jiffies = jiffies; > >> + > >> 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_power_on = tmp_jiffies; > >> + intel_dp->pps.last_backlight_off = tmp_jiffies; > >> } > >> > >> static void > >> -- > >> 2.17.1 > > > >-- > >Ville Syrjälä > >Intel > >
diff --git a/drivers/gpu/drm/i915/display/intel_pps.c b/drivers/gpu/drm/i915/display/intel_pps.c index 21944f5bf3a8..f3485a4fbfd0 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(&i915->drm, "Wait for panel power cycle\n"); /* take the difference of current time and panel power off time @@ -1098,9 +1105,11 @@ bool intel_pps_have_panel_power_or_vdd(struct intel_dp *intel_dp) static void pps_init_timestamps(struct intel_dp *intel_dp) { + unsigned long tmp_jiffies = jiffies; + 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_power_on = tmp_jiffies; + intel_dp->pps.last_backlight_off = tmp_jiffies; } static void
Driver always apply panel power off cycle delay before eDP enable. If eDP display was enabled at pre-os stage, driver would always trigger modeset to optimize cdclk setting after boot into kernel. So last_power_on and last_backlight_off value will be updated. We can check last_power_on and last_backlight_off before insert panel power cycle delay. If these values are the same, it means eDP was off until now and driver should bypass this delay to save some times to speed up eDP power on sequence. Cc: Shankar Uma <uma.shankar@intel.com> Cc: Jani Nikula <jani.nikula@linux.intel.com> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com> Signed-off-by: Lee Shawn C <shawn.c.lee@intel.com> --- drivers/gpu/drm/i915/display/intel_pps.c | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-)