diff mbox

drm/i915/vlv: T12 eDP panel timing enforcement during reboot

Message ID 1404763306-1692-1-git-send-email-clinton.a.taylor@intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

Taylor, Clinton A July 7, 2014, 8:01 p.m. UTC
From: Clint Taylor <clinton.a.taylor@intel.com>

The panel power sequencer on vlv doesn't appear to accept changes to its
T12 power down duration during warm reboots. This change forces a delay
for warm reboots to the T12 panel timing as defined in the VBT table for
the connected panel.

Ver2: removed redundant pr_crit(), commented magic value for pp_div_reg

Ver3: moved SYS_RESTART check earlier, new name for pp_div.

Ver4: Minor issue changes

Ver5: Move registration of reboot notifier to edp_connector_init,
      Added warning comment to handler about lack of PM notification.

Signed-off-by: Clint Taylor <clinton.a.taylor@intel.com>
---
 drivers/gpu/drm/i915/intel_dp.c  |   42 ++++++++++++++++++++++++++++++++++++++
 drivers/gpu/drm/i915/intel_drv.h |    2 ++
 2 files changed, 44 insertions(+)

Comments

Paulo Zanoni July 8, 2014, 2:06 p.m. UTC | #1
2014-07-07 17:01 GMT-03:00  <clinton.a.taylor@intel.com>:
> From: Clint Taylor <clinton.a.taylor@intel.com>
>
> The panel power sequencer on vlv doesn't appear to accept changes to its
> T12 power down duration during warm reboots. This change forces a delay
> for warm reboots to the T12 panel timing as defined in the VBT table for
> the connected panel.
>
> Ver2: removed redundant pr_crit(), commented magic value for pp_div_reg
>
> Ver3: moved SYS_RESTART check earlier, new name for pp_div.
>
> Ver4: Minor issue changes
>
> Ver5: Move registration of reboot notifier to edp_connector_init,
>       Added warning comment to handler about lack of PM notification.

Not everything I pointed was implemented, but at least the ghost eDP
bug was fixed, so the current patch is already an improvement:
Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>

>
> Signed-off-by: Clint Taylor <clinton.a.taylor@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_dp.c  |   42 ++++++++++++++++++++++++++++++++++++++
>  drivers/gpu/drm/i915/intel_drv.h |    2 ++
>  2 files changed, 44 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index b5ec489..7204dee 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -28,6 +28,8 @@
>  #include <linux/i2c.h>
>  #include <linux/slab.h>
>  #include <linux/export.h>
> +#include <linux/notifier.h>
> +#include <linux/reboot.h>
>  #include <drm/drmP.h>
>  #include <drm/drm_crtc.h>
>  #include <drm/drm_crtc_helper.h>
> @@ -336,6 +338,37 @@ static u32 _pp_stat_reg(struct intel_dp *intel_dp)
>                 return VLV_PIPE_PP_STATUS(vlv_power_sequencer_pipe(intel_dp));
>  }
>
> +/* Reboot notifier handler to shutdown panel power to guarantee T12 timing
> +   This function only applicable when panel PM state is not to be tracked */
> +static int edp_notify_handler(struct notifier_block *this, unsigned long code,
> +                             void *unused)
> +{
> +       struct intel_dp *intel_dp = container_of(this, typeof(* intel_dp),
> +                                                edp_notifier);
> +       struct drm_device *dev = intel_dp_to_dev(intel_dp);
> +       struct drm_i915_private *dev_priv = dev->dev_private;
> +       u32 pp_div;
> +       u32 pp_ctrl_reg, pp_div_reg;
> +       enum pipe pipe = vlv_power_sequencer_pipe(intel_dp);
> +
> +       if (!is_edp(intel_dp) || code != SYS_RESTART)
> +               return 0;
> +
> +       if (IS_VALLEYVIEW(dev)) {
> +               pp_ctrl_reg = VLV_PIPE_PP_CONTROL(pipe);
> +               pp_div_reg  = VLV_PIPE_PP_DIVISOR(pipe);
> +               pp_div = I915_READ(pp_div_reg);
> +               pp_div &= PP_REFERENCE_DIVIDER_MASK;
> +
> +               /* 0x1F write to PP_DIV_REG sets max cycle delay */
> +               I915_WRITE(pp_div_reg, pp_div | 0x1F);
> +               I915_WRITE(pp_ctrl_reg, PANEL_UNLOCK_REGS | PANEL_POWER_OFF);
> +               msleep(intel_dp->panel_power_cycle_delay);
> +       }
> +
> +       return 0;
> +}
> +
>  static bool edp_have_panel_power(struct intel_dp *intel_dp)
>  {
>         struct drm_device *dev = intel_dp_to_dev(intel_dp);
> @@ -3785,6 +3818,10 @@ void intel_dp_encoder_destroy(struct drm_encoder *encoder)
>                 drm_modeset_lock(&dev->mode_config.connection_mutex, NULL);
>                 edp_panel_vdd_off_sync(intel_dp);
>                 drm_modeset_unlock(&dev->mode_config.connection_mutex);
> +               if (intel_dp->edp_notifier.notifier_call) {
> +                       unregister_reboot_notifier(&intel_dp->edp_notifier);
> +                       intel_dp->edp_notifier.notifier_call = NULL;
> +               }
>         }
>         kfree(intel_dig_port);
>  }
> @@ -4262,6 +4299,11 @@ static bool intel_edp_init_connector(struct intel_dp *intel_dp,
>         }
>         mutex_unlock(&dev->mode_config.mutex);
>
> +       if (IS_VALLEYVIEW(dev)) {
> +               intel_dp->edp_notifier.notifier_call = edp_notify_handler;
> +               register_reboot_notifier(&intel_dp->edp_notifier);
> +       }
> +
>         intel_panel_init(&intel_connector->panel, fixed_mode, downclock_mode);
>         intel_panel_setup_backlight(connector);
>
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 5f7c7bd..87d1715 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -541,6 +541,8 @@ struct intel_dp {
>         unsigned long last_power_cycle;
>         unsigned long last_power_on;
>         unsigned long last_backlight_off;
> +       struct notifier_block edp_notifier;
> +
>         bool use_tps3;
>         struct intel_connector *attached_connector;
>
> --
> 1.7.9.5
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Daniel Vetter July 8, 2014, 2:55 p.m. UTC | #2
On Tue, Jul 08, 2014 at 11:06:00AM -0300, Paulo Zanoni wrote:
> 2014-07-07 17:01 GMT-03:00  <clinton.a.taylor@intel.com>:
> > From: Clint Taylor <clinton.a.taylor@intel.com>
> >
> > The panel power sequencer on vlv doesn't appear to accept changes to its
> > T12 power down duration during warm reboots. This change forces a delay
> > for warm reboots to the T12 panel timing as defined in the VBT table for
> > the connected panel.
> >
> > Ver2: removed redundant pr_crit(), commented magic value for pp_div_reg
> >
> > Ver3: moved SYS_RESTART check earlier, new name for pp_div.
> >
> > Ver4: Minor issue changes
> >
> > Ver5: Move registration of reboot notifier to edp_connector_init,
> >       Added warning comment to handler about lack of PM notification.
> 
> Not everything I pointed was implemented, but at least the ghost eDP
> bug was fixed, so the current patch is already an improvement:
> Reviewed-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
Picked up for -fixes, thanks for the patch.
-Daniel

> 
> >
> > Signed-off-by: Clint Taylor <clinton.a.taylor@intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_dp.c  |   42 ++++++++++++++++++++++++++++++++++++++
> >  drivers/gpu/drm/i915/intel_drv.h |    2 ++
> >  2 files changed, 44 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> > index b5ec489..7204dee 100644
> > --- a/drivers/gpu/drm/i915/intel_dp.c
> > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > @@ -28,6 +28,8 @@
> >  #include <linux/i2c.h>
> >  #include <linux/slab.h>
> >  #include <linux/export.h>
> > +#include <linux/notifier.h>
> > +#include <linux/reboot.h>
> >  #include <drm/drmP.h>
> >  #include <drm/drm_crtc.h>
> >  #include <drm/drm_crtc_helper.h>
> > @@ -336,6 +338,37 @@ static u32 _pp_stat_reg(struct intel_dp *intel_dp)
> >                 return VLV_PIPE_PP_STATUS(vlv_power_sequencer_pipe(intel_dp));
> >  }
> >
> > +/* Reboot notifier handler to shutdown panel power to guarantee T12 timing
> > +   This function only applicable when panel PM state is not to be tracked */
> > +static int edp_notify_handler(struct notifier_block *this, unsigned long code,
> > +                             void *unused)
> > +{
> > +       struct intel_dp *intel_dp = container_of(this, typeof(* intel_dp),
> > +                                                edp_notifier);
> > +       struct drm_device *dev = intel_dp_to_dev(intel_dp);
> > +       struct drm_i915_private *dev_priv = dev->dev_private;
> > +       u32 pp_div;
> > +       u32 pp_ctrl_reg, pp_div_reg;
> > +       enum pipe pipe = vlv_power_sequencer_pipe(intel_dp);
> > +
> > +       if (!is_edp(intel_dp) || code != SYS_RESTART)
> > +               return 0;
> > +
> > +       if (IS_VALLEYVIEW(dev)) {
> > +               pp_ctrl_reg = VLV_PIPE_PP_CONTROL(pipe);
> > +               pp_div_reg  = VLV_PIPE_PP_DIVISOR(pipe);
> > +               pp_div = I915_READ(pp_div_reg);
> > +               pp_div &= PP_REFERENCE_DIVIDER_MASK;
> > +
> > +               /* 0x1F write to PP_DIV_REG sets max cycle delay */
> > +               I915_WRITE(pp_div_reg, pp_div | 0x1F);
> > +               I915_WRITE(pp_ctrl_reg, PANEL_UNLOCK_REGS | PANEL_POWER_OFF);
> > +               msleep(intel_dp->panel_power_cycle_delay);
> > +       }
> > +
> > +       return 0;
> > +}
> > +
> >  static bool edp_have_panel_power(struct intel_dp *intel_dp)
> >  {
> >         struct drm_device *dev = intel_dp_to_dev(intel_dp);
> > @@ -3785,6 +3818,10 @@ void intel_dp_encoder_destroy(struct drm_encoder *encoder)
> >                 drm_modeset_lock(&dev->mode_config.connection_mutex, NULL);
> >                 edp_panel_vdd_off_sync(intel_dp);
> >                 drm_modeset_unlock(&dev->mode_config.connection_mutex);
> > +               if (intel_dp->edp_notifier.notifier_call) {
> > +                       unregister_reboot_notifier(&intel_dp->edp_notifier);
> > +                       intel_dp->edp_notifier.notifier_call = NULL;
> > +               }
> >         }
> >         kfree(intel_dig_port);
> >  }
> > @@ -4262,6 +4299,11 @@ static bool intel_edp_init_connector(struct intel_dp *intel_dp,
> >         }
> >         mutex_unlock(&dev->mode_config.mutex);
> >
> > +       if (IS_VALLEYVIEW(dev)) {
> > +               intel_dp->edp_notifier.notifier_call = edp_notify_handler;
> > +               register_reboot_notifier(&intel_dp->edp_notifier);
> > +       }
> > +
> >         intel_panel_init(&intel_connector->panel, fixed_mode, downclock_mode);
> >         intel_panel_setup_backlight(connector);
> >
> > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> > index 5f7c7bd..87d1715 100644
> > --- a/drivers/gpu/drm/i915/intel_drv.h
> > +++ b/drivers/gpu/drm/i915/intel_drv.h
> > @@ -541,6 +541,8 @@ struct intel_dp {
> >         unsigned long last_power_cycle;
> >         unsigned long last_power_on;
> >         unsigned long last_backlight_off;
> > +       struct notifier_block edp_notifier;
> > +
> >         bool use_tps3;
> >         struct intel_connector *attached_connector;
> >
> > --
> > 1.7.9.5
> >
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> 
> 
> -- 
> Paulo Zanoni
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
diff mbox

Patch

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index b5ec489..7204dee 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -28,6 +28,8 @@ 
 #include <linux/i2c.h>
 #include <linux/slab.h>
 #include <linux/export.h>
+#include <linux/notifier.h>
+#include <linux/reboot.h>
 #include <drm/drmP.h>
 #include <drm/drm_crtc.h>
 #include <drm/drm_crtc_helper.h>
@@ -336,6 +338,37 @@  static u32 _pp_stat_reg(struct intel_dp *intel_dp)
 		return VLV_PIPE_PP_STATUS(vlv_power_sequencer_pipe(intel_dp));
 }
 
+/* Reboot notifier handler to shutdown panel power to guarantee T12 timing
+   This function only applicable when panel PM state is not to be tracked */
+static int edp_notify_handler(struct notifier_block *this, unsigned long code,
+			      void *unused)
+{
+	struct intel_dp *intel_dp = container_of(this, typeof(* intel_dp),
+						 edp_notifier);
+	struct drm_device *dev = intel_dp_to_dev(intel_dp);
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	u32 pp_div;
+	u32 pp_ctrl_reg, pp_div_reg;
+	enum pipe pipe = vlv_power_sequencer_pipe(intel_dp);
+
+	if (!is_edp(intel_dp) || code != SYS_RESTART)
+		return 0;
+
+	if (IS_VALLEYVIEW(dev)) {
+		pp_ctrl_reg = VLV_PIPE_PP_CONTROL(pipe);
+		pp_div_reg  = VLV_PIPE_PP_DIVISOR(pipe);
+		pp_div = I915_READ(pp_div_reg);
+		pp_div &= PP_REFERENCE_DIVIDER_MASK;
+
+		/* 0x1F write to PP_DIV_REG sets max cycle delay */
+		I915_WRITE(pp_div_reg, pp_div | 0x1F);
+		I915_WRITE(pp_ctrl_reg, PANEL_UNLOCK_REGS | PANEL_POWER_OFF);
+		msleep(intel_dp->panel_power_cycle_delay);
+	}
+
+	return 0;
+}
+
 static bool edp_have_panel_power(struct intel_dp *intel_dp)
 {
 	struct drm_device *dev = intel_dp_to_dev(intel_dp);
@@ -3785,6 +3818,10 @@  void intel_dp_encoder_destroy(struct drm_encoder *encoder)
 		drm_modeset_lock(&dev->mode_config.connection_mutex, NULL);
 		edp_panel_vdd_off_sync(intel_dp);
 		drm_modeset_unlock(&dev->mode_config.connection_mutex);
+		if (intel_dp->edp_notifier.notifier_call) {
+			unregister_reboot_notifier(&intel_dp->edp_notifier);
+			intel_dp->edp_notifier.notifier_call = NULL;
+		}
 	}
 	kfree(intel_dig_port);
 }
@@ -4262,6 +4299,11 @@  static bool intel_edp_init_connector(struct intel_dp *intel_dp,
 	}
 	mutex_unlock(&dev->mode_config.mutex);
 
+	if (IS_VALLEYVIEW(dev)) {
+		intel_dp->edp_notifier.notifier_call = edp_notify_handler;
+		register_reboot_notifier(&intel_dp->edp_notifier);
+	}
+
 	intel_panel_init(&intel_connector->panel, fixed_mode, downclock_mode);
 	intel_panel_setup_backlight(connector);
 
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 5f7c7bd..87d1715 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -541,6 +541,8 @@  struct intel_dp {
 	unsigned long last_power_cycle;
 	unsigned long last_power_on;
 	unsigned long last_backlight_off;
+	struct notifier_block edp_notifier;
+
 	bool use_tps3;
 	struct intel_connector *attached_connector;