On Fri, Dec 06, 2013 at 05:32:43PM -0200, Paulo Zanoni wrote:
> From: Paulo Zanoni <paulo.r.zanoni@xxxxxxxxx>
>
> The eDP spec defines some points where after you do action A, you have
> to wait some time before action B. The thing is that in our driver
> action B does not happen exactly after action A, but we still use
> msleep() calls directly. What this patch does is that we record the
> timestamp of when action A happened, then, just before action B, we
> look at how much time has passed and only sleep the remaining amount
> needed.
>
> With this change, I am able to save about 5-20ms (out of the total
> 200ms) of the backlight_off delay and completely skip the 1ms
> backlight_on delay. The 600ms vdd_off delay doesn't happen during
> normal usage anymore due to a previous patch.
>
> v2: - Rename ironlake_wait_jiffies_delay to intel_wait_until_after and
> move it to intel_display.c
> - Fix the msleep call: diff is in jiffies
> v3: - Use "tmp_jiffies" so we don't need to worry about the value of
> "jiffies" advancing while we're doing the math.
>
> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@xxxxxxxxx>
> ---
> drivers/gpu/drm/i915/intel_display.c | 18 ++++++++++++++++++
> drivers/gpu/drm/i915/intel_dp.c | 26 +++++++++++++++++++++++---
> drivers/gpu/drm/i915/intel_drv.h | 4 ++++
> 3 files changed, 45 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 3c59b67..0c238dd 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -806,6 +806,24 @@ void intel_wait_for_vblank(struct drm_device *dev, int pipe)
> DRM_DEBUG_KMS("vblank wait timed out\n");
> }
>
> +/* If you need to wait X ms between events A and B, but event B doesn't happen
> + * exactly after event A, you record the timestamp (jiffies) of when event A
> + * happened, then just before event B you call intel_wait_until_after and pass
> + * the timestamp as the first argument, and X as the second argument. */
> +void intel_wait_until_after(unsigned long timestamp, int to_wait_ms)
> +{
> + unsigned long target = timestamp + msecs_to_jiffies(to_wait_ms);
msec_to_jiffies_timeout is what we want here. Also I nowadays prefer
multiline comments with the /* and */ on their own line for more
consistency ...
-Daniel
> + unsigned long diff;
> + /* Don't re-read the value of "jiffies" every time since it may change
> + * behind our back and break the math. */
> + unsigned long tmp_jiffies = jiffies;
> +
> + if (time_after(target, tmp_jiffies)) {
> + diff = (long)target - (long)tmp_jiffies;
> + msleep(jiffies_to_msecs(diff));
This will add one more jiffy again, so for optimal results we need to use
something else here I think.
I've merged the previous 3 patches and signed up Jesse to review the last
one.
-Daniel
> + }
> +}
> +
> static bool pipe_dsl_stopped(struct drm_device *dev, enum pipe pipe)
> {
> struct drm_i915_private *dev_priv = dev->dev_private;
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index a2aace2..79f7ec2 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -1056,9 +1056,26 @@ static void ironlake_wait_panel_off(struct intel_dp *intel_dp)
> static void ironlake_wait_panel_power_cycle(struct intel_dp *intel_dp)
> {
> DRM_DEBUG_KMS("Wait for panel power cycle\n");
> +
> + /* When we disable the VDD override bit last we have to do the manual
> + * wait. */
> + intel_wait_until_after(intel_dp->last_power_cycle,
> + intel_dp->panel_power_cycle_delay);
> +
> ironlake_wait_panel_status(intel_dp, IDLE_CYCLE_MASK, IDLE_CYCLE_VALUE);
> }
>
> +static void ironlake_wait_backlight_on(struct intel_dp *intel_dp)
> +{
> + intel_wait_until_after(intel_dp->last_power_on,
> + intel_dp->backlight_on_delay);
> +}
> +
> +static void ironlake_edp_wait_backlight_off(struct intel_dp *intel_dp)
> +{
> + intel_wait_until_after(intel_dp->last_backlight_off,
> + intel_dp->backlight_off_delay);
> +}
>
> /* Read the current pp_control value, unlocking the register if it
> * is locked
> @@ -1144,7 +1161,7 @@ static void ironlake_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)
> - msleep(intel_dp->panel_power_cycle_delay);
> + intel_dp->last_power_cycle = jiffies;
> }
> }
>
> @@ -1217,6 +1234,7 @@ void ironlake_edp_panel_on(struct intel_dp *intel_dp)
> POSTING_READ(pp_ctrl_reg);
>
> ironlake_wait_panel_on(intel_dp);
> + intel_dp->last_power_on = jiffies;
>
> if (IS_GEN5(dev)) {
> pp |= PANEL_POWER_RESET; /* restore panel reset bit */
> @@ -1237,6 +1255,8 @@ void ironlake_edp_panel_off(struct intel_dp *intel_dp)
>
> DRM_DEBUG_KMS("Turn eDP power off\n");
>
> + ironlake_edp_wait_backlight_off(intel_dp);
> +
> pp = ironlake_get_pp_control(intel_dp);
> /* We need to switch off panel power _and_ force vdd, for otherwise some
> * panels get very unhappy and cease to work. */
> @@ -1268,7 +1288,7 @@ void ironlake_edp_backlight_on(struct intel_dp *intel_dp)
> * link. So delay a bit to make sure the image is solid before
> * allowing it to appear.
> */
> - msleep(intel_dp->backlight_on_delay);
> + ironlake_wait_backlight_on(intel_dp);
> pp = ironlake_get_pp_control(intel_dp);
> pp |= EDP_BLC_ENABLE;
>
> @@ -1300,7 +1320,7 @@ void ironlake_edp_backlight_off(struct intel_dp *intel_dp)
>
> I915_WRITE(pp_ctrl_reg, pp);
> POSTING_READ(pp_ctrl_reg);
> - msleep(intel_dp->backlight_off_delay);
> + intel_dp->last_backlight_off = jiffies;
> }
>
> static void ironlake_edp_pll_on(struct intel_dp *intel_dp)
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index ea62673..eda8ee8 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -484,6 +484,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;
> bool psr_setup_done;
> struct intel_connector *attached_connector;
> };
> @@ -707,6 +710,7 @@ void hsw_enable_ips(struct intel_crtc *crtc);
> void hsw_disable_ips(struct intel_crtc *crtc);
> void intel_display_set_init_power(struct drm_device *dev, bool enable);
> int valleyview_get_vco(struct drm_i915_private *dev_priv);
> +void intel_wait_until_after(unsigned long timestamp, int to_wait_ms);
>
> /* intel_dp.c */
> void intel_dp_init(struct drm_device *dev, int output_reg, enum port port);
> --
> 1.8.3.1
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
