| Message ID | 1470231378-10656-1-git-send-email-ville.syrjala@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On 08/03/2016 06:36 AM, ville.syrjala@linux.intel.com wrote: > From: Ville Syrjälä <ville.syrjala@linux.intel.com> > > Dell UP2414Q doesn't like it when we're driving it in MST mode, and then > reboot the machine. After reboot, the monitor is somehow confused and > refuses to do the payload allocation: > [drm:drm_dp_mst_allocate_vcpi] initing vcpi for 282 8 > [drm:drm_dp_dpcd_write_payload] status not set after read payload table status 0 > and thus we're left with a black screen until the monitor power is > toggled. > > It seems that if we shut down things gracefully prior to rebooting, the > monitor doesn't get confused like this. So let's try to shut down all > the displays gracefully on reboot. The downside is that we will > introduce the reboot notifier to all the modesetl locks. So if there's > a locking bug around, we might not be able to reboot the machine > gracefully. sysrq reboot will still work though, as it will not > call the notifiers. > > While we do this, we can eliminate the eDP reboot notifier, which is > there to shut down the panel power and make sure the firmware won't > violate the panel power cycle delay post-reboot. Since we're now > shutting eDP down properly, we can mostly just rip out the eDP notifier. > We still need to do the panel power cycle wait though, as that would > normally get postponed until the next modeset. So let's move that part > into intel_panel so that other display types will get the same treatment > as a bonus. > > The Dell UP2414Q can often get even more confused, and sometimes what > you have to do is: switch to another input on the monitor, toggle the > monitor power, switch the input back to the original setting. And > sometimes it seems you just have to yank the power cable entirely. I'm > not sure if this reboot notifier might avoid some of these other > failure modes as well, but I'm pretty sure it can't hurt at least. > While testing this change on SKL if 2 crtc's are enabled resume from suspend is 500ms longer than a single crtc resume. Are we calling the T12 msleep(500) for non eDP panels? During testing VDD to the panel, the T12 panel timeout requirement is being meet at >500ms during reboot and suspend/resume. > Cc: Clint Taylor <clinton.a.taylor@intel.com> > Cc: Paulo Zanoni <paulo.r.zanoni@intel.com> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com> > --- > drivers/gpu/drm/i915/i915_drv.h | 2 ++ > drivers/gpu/drm/i915/intel_display.c | 24 ++++++++++++++++++ > drivers/gpu/drm/i915/intel_dp.c | 49 ++---------------------------------- > drivers/gpu/drm/i915/intel_drv.h | 7 +++--- > drivers/gpu/drm/i915/intel_dsi.c | 3 ++- > drivers/gpu/drm/i915/intel_dvo.c | 2 +- > drivers/gpu/drm/i915/intel_lvds.c | 3 ++- > drivers/gpu/drm/i915/intel_panel.c | 29 ++++++++++++++++++++- > 8 files changed, 65 insertions(+), 54 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h > index 65ada5d2f88c..f8eb8c7de007 100644 > --- a/drivers/gpu/drm/i915/i915_drv.h > +++ b/drivers/gpu/drm/i915/i915_drv.h > @@ -1727,6 +1727,8 @@ struct intel_wm_config { > struct drm_i915_private { > struct drm_device drm; > > + struct notifier_block reboot_notifier; > + > struct kmem_cache *objects; > struct kmem_cache *vmas; > struct kmem_cache *requests; > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > index a8e8cc8dfae9..2192a21aa6c9 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -31,6 +31,8 @@ > #include <linux/kernel.h> > #include <linux/slab.h> > #include <linux/vgaarb.h> > +#include <linux/notifier.h> > +#include <linux/reboot.h> > #include <drm/drm_edid.h> > #include <drm/drmP.h> > #include "intel_drv.h" > @@ -15578,6 +15580,23 @@ fail: > drm_modeset_acquire_fini(&ctx); > } > > + > +static int intel_reboot_notifier(struct notifier_block *nb, > + unsigned long code, void *unused) > +{ > + struct drm_i915_private *dev_priv = > + container_of(nb, typeof(*dev_priv), reboot_notifier); > + > + if (code != SYS_RESTART) > + return 0; > + > + intel_display_suspend(&dev_priv->drm); > + Need to check to make sure there is a panel before calling intel_panel_reboot_notifier(). > + intel_panel_reboot_notifier(dev_priv); > + > + return 0; > +} > + > void intel_modeset_init(struct drm_device *dev) > { > struct drm_i915_private *dev_priv = to_i915(dev); > @@ -15706,6 +15725,9 @@ void intel_modeset_init(struct drm_device *dev) > * since the watermark calculation done here will use pstate->fb. > */ > sanitize_watermarks(dev); > + > + dev_priv->reboot_notifier.notifier_call = intel_reboot_notifier; > + register_reboot_notifier(&dev_priv->reboot_notifier); > } > > static void intel_enable_pipe_a(struct drm_device *dev) > @@ -16291,6 +16313,8 @@ void intel_modeset_cleanup(struct drm_device *dev) > { > struct drm_i915_private *dev_priv = to_i915(dev); > > + unregister_reboot_notifier(&dev_priv->reboot_notifier); > + > intel_disable_gt_powersave(dev_priv); > > /* > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c > index 001f74fc0ce5..d8d13a9e33e5 100644 > --- a/drivers/gpu/drm/i915/intel_dp.c > +++ b/drivers/gpu/drm/i915/intel_dp.c > @@ -28,8 +28,6 @@ > #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_atomic_helper.h> > #include <drm/drm_crtc.h> > @@ -631,42 +629,6 @@ _pp_stat_reg(struct intel_dp *intel_dp) > return regs.pp_stat; > } > > -/* 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 = to_i915(dev); > - > - if (!is_edp(intel_dp) || code != SYS_RESTART) > - return 0; > - > - pps_lock(intel_dp); > - > - if (IS_VALLEYVIEW(dev) || IS_CHERRYVIEW(dev)) { > - enum pipe pipe = vlv_power_sequencer_pipe(intel_dp); > - i915_reg_t pp_ctrl_reg, pp_div_reg; > - u32 pp_div; > - > - 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); > - } > - > - pps_unlock(intel_dp); > - > - return 0; > -} > - > static bool edp_have_panel_power(struct intel_dp *intel_dp) > { > struct drm_device *dev = intel_dp_to_dev(intel_dp); > @@ -4562,11 +4524,6 @@ void intel_dp_encoder_destroy(struct drm_encoder *encoder) > pps_lock(intel_dp); > edp_panel_vdd_off_sync(intel_dp); > pps_unlock(intel_dp); > - > - if (intel_dp->edp_notifier.notifier_call) { > - unregister_reboot_notifier(&intel_dp->edp_notifier); > - intel_dp->edp_notifier.notifier_call = NULL; > - } > } > > intel_dp_aux_fini(intel_dp); > @@ -5465,9 +5422,6 @@ static bool intel_edp_init_connector(struct intel_dp *intel_dp, > mutex_unlock(&dev->mode_config.mutex); > > if (IS_VALLEYVIEW(dev) || IS_CHERRYVIEW(dev)) { > - intel_dp->edp_notifier.notifier_call = edp_notify_handler; > - register_reboot_notifier(&intel_dp->edp_notifier); > - > /* > * Figure out the current pipe for the initial backlight setup. > * If the current pipe isn't valid, try the PPS pipe, and if that > @@ -5488,7 +5442,8 @@ static bool intel_edp_init_connector(struct intel_dp *intel_dp, > pipe_name(pipe)); > } > > - intel_panel_init(&intel_connector->panel, fixed_mode, downclock_mode); > + intel_panel_init(&intel_connector->panel, fixed_mode, downclock_mode, > + intel_dp->panel_power_cycle_delay); > intel_connector->panel.backlight.power = intel_edp_backlight_power; > intel_panel_setup_backlight(connector, pipe); > > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h > index 50cdc890d956..92fa66a02ca5 100644 > --- a/drivers/gpu/drm/i915/intel_drv.h > +++ b/drivers/gpu/drm/i915/intel_drv.h > @@ -226,6 +226,7 @@ struct intel_panel { > struct drm_display_mode *fixed_mode; > struct drm_display_mode *downclock_mode; > int fitting_mode; > + int reboot_power_cycle_delay; > > /* backlight */ > struct { > @@ -877,8 +878,6 @@ struct intel_dp { > unsigned long last_backlight_off; > ktime_t panel_power_off_time; > > - struct notifier_block edp_notifier; > - > /* > * Pipe whose power sequencer is currently locked into > * this port. Only relevant on VLV/CHV. > @@ -1521,8 +1520,10 @@ void intel_overlay_reset(struct drm_i915_private *dev_priv); > /* intel_panel.c */ > int intel_panel_init(struct intel_panel *panel, > struct drm_display_mode *fixed_mode, > - struct drm_display_mode *downclock_mode); > + struct drm_display_mode *downclock_mode, > + int reboot_power_cycle_delay); > void intel_panel_fini(struct intel_panel *panel); > +void intel_panel_reboot_notifier(struct drm_i915_private *dev_priv); > void intel_fixed_panel_mode(const struct drm_display_mode *fixed_mode, > struct drm_display_mode *adjusted_mode); > void intel_pch_panel_fitting(struct intel_crtc *crtc, > diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c > index de8e9fb51595..5b722ec23381 100644 > --- a/drivers/gpu/drm/i915/intel_dsi.c > +++ b/drivers/gpu/drm/i915/intel_dsi.c > @@ -1596,7 +1596,8 @@ void intel_dsi_init(struct drm_device *dev) > connector->display_info.width_mm = fixed_mode->width_mm; > connector->display_info.height_mm = fixed_mode->height_mm; > > - intel_panel_init(&intel_connector->panel, fixed_mode, NULL); > + intel_panel_init(&intel_connector->panel, fixed_mode, NULL, > + intel_dsi->panel_pwr_cycle_delay); > intel_panel_setup_backlight(connector, INVALID_PIPE); > > intel_dsi_add_properties(intel_connector); > diff --git a/drivers/gpu/drm/i915/intel_dvo.c b/drivers/gpu/drm/i915/intel_dvo.c > index 47bdf9dad0d3..c2c9c922590c 100644 > --- a/drivers/gpu/drm/i915/intel_dvo.c > +++ b/drivers/gpu/drm/i915/intel_dvo.c > @@ -547,7 +547,7 @@ void intel_dvo_init(struct drm_device *dev) > */ > intel_panel_init(&intel_connector->panel, > intel_dvo_get_current_mode(connector), > - NULL); > + NULL, 0); > intel_dvo->panel_wants_dither = true; > } > > diff --git a/drivers/gpu/drm/i915/intel_lvds.c b/drivers/gpu/drm/i915/intel_lvds.c > index 49550470483e..07d7ac2c1520 100644 > --- a/drivers/gpu/drm/i915/intel_lvds.c > +++ b/drivers/gpu/drm/i915/intel_lvds.c > @@ -1120,7 +1120,8 @@ void intel_lvds_init(struct drm_device *dev) > out: > mutex_unlock(&dev->mode_config.mutex); > > - intel_panel_init(&intel_connector->panel, fixed_mode, downclock_mode); > + /* FIXME fill in an accurate power cycle delay */ > + intel_panel_init(&intel_connector->panel, fixed_mode, downclock_mode, 0); > intel_panel_setup_backlight(connector, INVALID_PIPE); > > lvds_encoder->is_dual_link = compute_is_dual_link_lvds(lvds_encoder); > diff --git a/drivers/gpu/drm/i915/intel_panel.c b/drivers/gpu/drm/i915/intel_panel.c > index 96c65d77e886..fe57a743ad21 100644 > --- a/drivers/gpu/drm/i915/intel_panel.c > +++ b/drivers/gpu/drm/i915/intel_panel.c > @@ -1781,14 +1781,41 @@ intel_panel_init_backlight_funcs(struct intel_panel *panel) > } > } > > +/* > + * Make sure the power cycle delay is respected. The PPS > + * does supposedly initiate a power cycle on reset, but it > + * also resets the power cycle delay register value to the > + * default value, and hence may not wait long enough if the > + * firmware attempts to power up the panel immediately. > + * Also eg. DSI doesn't use the PPS. > + */ > +void intel_panel_reboot_notifier(struct drm_i915_private *dev_priv) > +{ > + struct drm_device *dev = &dev_priv->drm; > + struct intel_connector *connector; > + int reboot_power_cycle_delay = 0; > + > + for_each_intel_connector(dev, connector) { > + struct intel_panel *panel = &connector->panel; > + > + reboot_power_cycle_delay = max(reboot_power_cycle_delay, > + panel->reboot_power_cycle_delay); > + } > + > + if (reboot_power_cycle_delay) > + msleep(reboot_power_cycle_delay); > +} > + > int intel_panel_init(struct intel_panel *panel, > struct drm_display_mode *fixed_mode, > - struct drm_display_mode *downclock_mode) > + struct drm_display_mode *downclock_mode, > + int reboot_power_cycle_delay) > { > intel_panel_init_backlight_funcs(panel); > > panel->fixed_mode = fixed_mode; > panel->downclock_mode = downclock_mode; > + panel->reboot_power_cycle_delay = reboot_power_cycle_delay; > > return 0; > } >
On Wed, Aug 03, 2016 at 04:36:18PM +0300, ville.syrjala@linux.intel.com wrote: > From: Ville Syrjälä <ville.syrjala@linux.intel.com> > > Dell UP2414Q doesn't like it when we're driving it in MST mode, and then > reboot the machine. After reboot, the monitor is somehow confused and > refuses to do the payload allocation: > [drm:drm_dp_mst_allocate_vcpi] initing vcpi for 282 8 > [drm:drm_dp_dpcd_write_payload] status not set after read payload table status 0 > and thus we're left with a black screen until the monitor power is > toggled. > > It seems that if we shut down things gracefully prior to rebooting, the > monitor doesn't get confused like this. So let's try to shut down all > the displays gracefully on reboot. The downside is that we will > introduce the reboot notifier to all the modesetl locks. So if there's > a locking bug around, we might not be able to reboot the machine > gracefully. sysrq reboot will still work though, as it will not > call the notifiers. struct pci_driver has a ->shutdown hook which is currently not defined in i915_pci_driver. How about using that instead of reboot_notifier to avoid potential locking issues? Best regards, Lukas > > While we do this, we can eliminate the eDP reboot notifier, which is > there to shut down the panel power and make sure the firmware won't > violate the panel power cycle delay post-reboot. Since we're now > shutting eDP down properly, we can mostly just rip out the eDP notifier. > We still need to do the panel power cycle wait though, as that would > normally get postponed until the next modeset. So let's move that part > into intel_panel so that other display types will get the same treatment > as a bonus. > > The Dell UP2414Q can often get even more confused, and sometimes what > you have to do is: switch to another input on the monitor, toggle the > monitor power, switch the input back to the original setting. And > sometimes it seems you just have to yank the power cable entirely. I'm > not sure if this reboot notifier might avoid some of these other > failure modes as well, but I'm pretty sure it can't hurt at least. > > Cc: Clint Taylor <clinton.a.taylor@intel.com> > Cc: Paulo Zanoni <paulo.r.zanoni@intel.com> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com> > --- > drivers/gpu/drm/i915/i915_drv.h | 2 ++ > drivers/gpu/drm/i915/intel_display.c | 24 ++++++++++++++++++ > drivers/gpu/drm/i915/intel_dp.c | 49 ++---------------------------------- > drivers/gpu/drm/i915/intel_drv.h | 7 +++--- > drivers/gpu/drm/i915/intel_dsi.c | 3 ++- > drivers/gpu/drm/i915/intel_dvo.c | 2 +- > drivers/gpu/drm/i915/intel_lvds.c | 3 ++- > drivers/gpu/drm/i915/intel_panel.c | 29 ++++++++++++++++++++- > 8 files changed, 65 insertions(+), 54 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h > index 65ada5d2f88c..f8eb8c7de007 100644 > --- a/drivers/gpu/drm/i915/i915_drv.h > +++ b/drivers/gpu/drm/i915/i915_drv.h > @@ -1727,6 +1727,8 @@ struct intel_wm_config { > struct drm_i915_private { > struct drm_device drm; > > + struct notifier_block reboot_notifier; > + > struct kmem_cache *objects; > struct kmem_cache *vmas; > struct kmem_cache *requests; > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > index a8e8cc8dfae9..2192a21aa6c9 100644 > --- a/drivers/gpu/drm/i915/intel_display.c > +++ b/drivers/gpu/drm/i915/intel_display.c > @@ -31,6 +31,8 @@ > #include <linux/kernel.h> > #include <linux/slab.h> > #include <linux/vgaarb.h> > +#include <linux/notifier.h> > +#include <linux/reboot.h> > #include <drm/drm_edid.h> > #include <drm/drmP.h> > #include "intel_drv.h" > @@ -15578,6 +15580,23 @@ fail: > drm_modeset_acquire_fini(&ctx); > } > > + > +static int intel_reboot_notifier(struct notifier_block *nb, > + unsigned long code, void *unused) > +{ > + struct drm_i915_private *dev_priv = > + container_of(nb, typeof(*dev_priv), reboot_notifier); > + > + if (code != SYS_RESTART) > + return 0; > + > + intel_display_suspend(&dev_priv->drm); > + > + intel_panel_reboot_notifier(dev_priv); > + > + return 0; > +} > + > void intel_modeset_init(struct drm_device *dev) > { > struct drm_i915_private *dev_priv = to_i915(dev); > @@ -15706,6 +15725,9 @@ void intel_modeset_init(struct drm_device *dev) > * since the watermark calculation done here will use pstate->fb. > */ > sanitize_watermarks(dev); > + > + dev_priv->reboot_notifier.notifier_call = intel_reboot_notifier; > + register_reboot_notifier(&dev_priv->reboot_notifier); > } > > static void intel_enable_pipe_a(struct drm_device *dev) > @@ -16291,6 +16313,8 @@ void intel_modeset_cleanup(struct drm_device *dev) > { > struct drm_i915_private *dev_priv = to_i915(dev); > > + unregister_reboot_notifier(&dev_priv->reboot_notifier); > + > intel_disable_gt_powersave(dev_priv); > > /* > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c > index 001f74fc0ce5..d8d13a9e33e5 100644 > --- a/drivers/gpu/drm/i915/intel_dp.c > +++ b/drivers/gpu/drm/i915/intel_dp.c > @@ -28,8 +28,6 @@ > #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_atomic_helper.h> > #include <drm/drm_crtc.h> > @@ -631,42 +629,6 @@ _pp_stat_reg(struct intel_dp *intel_dp) > return regs.pp_stat; > } > > -/* 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 = to_i915(dev); > - > - if (!is_edp(intel_dp) || code != SYS_RESTART) > - return 0; > - > - pps_lock(intel_dp); > - > - if (IS_VALLEYVIEW(dev) || IS_CHERRYVIEW(dev)) { > - enum pipe pipe = vlv_power_sequencer_pipe(intel_dp); > - i915_reg_t pp_ctrl_reg, pp_div_reg; > - u32 pp_div; > - > - 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); > - } > - > - pps_unlock(intel_dp); > - > - return 0; > -} > - > static bool edp_have_panel_power(struct intel_dp *intel_dp) > { > struct drm_device *dev = intel_dp_to_dev(intel_dp); > @@ -4562,11 +4524,6 @@ void intel_dp_encoder_destroy(struct drm_encoder *encoder) > pps_lock(intel_dp); > edp_panel_vdd_off_sync(intel_dp); > pps_unlock(intel_dp); > - > - if (intel_dp->edp_notifier.notifier_call) { > - unregister_reboot_notifier(&intel_dp->edp_notifier); > - intel_dp->edp_notifier.notifier_call = NULL; > - } > } > > intel_dp_aux_fini(intel_dp); > @@ -5465,9 +5422,6 @@ static bool intel_edp_init_connector(struct intel_dp *intel_dp, > mutex_unlock(&dev->mode_config.mutex); > > if (IS_VALLEYVIEW(dev) || IS_CHERRYVIEW(dev)) { > - intel_dp->edp_notifier.notifier_call = edp_notify_handler; > - register_reboot_notifier(&intel_dp->edp_notifier); > - > /* > * Figure out the current pipe for the initial backlight setup. > * If the current pipe isn't valid, try the PPS pipe, and if that > @@ -5488,7 +5442,8 @@ static bool intel_edp_init_connector(struct intel_dp *intel_dp, > pipe_name(pipe)); > } > > - intel_panel_init(&intel_connector->panel, fixed_mode, downclock_mode); > + intel_panel_init(&intel_connector->panel, fixed_mode, downclock_mode, > + intel_dp->panel_power_cycle_delay); > intel_connector->panel.backlight.power = intel_edp_backlight_power; > intel_panel_setup_backlight(connector, pipe); > > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h > index 50cdc890d956..92fa66a02ca5 100644 > --- a/drivers/gpu/drm/i915/intel_drv.h > +++ b/drivers/gpu/drm/i915/intel_drv.h > @@ -226,6 +226,7 @@ struct intel_panel { > struct drm_display_mode *fixed_mode; > struct drm_display_mode *downclock_mode; > int fitting_mode; > + int reboot_power_cycle_delay; > > /* backlight */ > struct { > @@ -877,8 +878,6 @@ struct intel_dp { > unsigned long last_backlight_off; > ktime_t panel_power_off_time; > > - struct notifier_block edp_notifier; > - > /* > * Pipe whose power sequencer is currently locked into > * this port. Only relevant on VLV/CHV. > @@ -1521,8 +1520,10 @@ void intel_overlay_reset(struct drm_i915_private *dev_priv); > /* intel_panel.c */ > int intel_panel_init(struct intel_panel *panel, > struct drm_display_mode *fixed_mode, > - struct drm_display_mode *downclock_mode); > + struct drm_display_mode *downclock_mode, > + int reboot_power_cycle_delay); > void intel_panel_fini(struct intel_panel *panel); > +void intel_panel_reboot_notifier(struct drm_i915_private *dev_priv); > void intel_fixed_panel_mode(const struct drm_display_mode *fixed_mode, > struct drm_display_mode *adjusted_mode); > void intel_pch_panel_fitting(struct intel_crtc *crtc, > diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c > index de8e9fb51595..5b722ec23381 100644 > --- a/drivers/gpu/drm/i915/intel_dsi.c > +++ b/drivers/gpu/drm/i915/intel_dsi.c > @@ -1596,7 +1596,8 @@ void intel_dsi_init(struct drm_device *dev) > connector->display_info.width_mm = fixed_mode->width_mm; > connector->display_info.height_mm = fixed_mode->height_mm; > > - intel_panel_init(&intel_connector->panel, fixed_mode, NULL); > + intel_panel_init(&intel_connector->panel, fixed_mode, NULL, > + intel_dsi->panel_pwr_cycle_delay); > intel_panel_setup_backlight(connector, INVALID_PIPE); > > intel_dsi_add_properties(intel_connector); > diff --git a/drivers/gpu/drm/i915/intel_dvo.c b/drivers/gpu/drm/i915/intel_dvo.c > index 47bdf9dad0d3..c2c9c922590c 100644 > --- a/drivers/gpu/drm/i915/intel_dvo.c > +++ b/drivers/gpu/drm/i915/intel_dvo.c > @@ -547,7 +547,7 @@ void intel_dvo_init(struct drm_device *dev) > */ > intel_panel_init(&intel_connector->panel, > intel_dvo_get_current_mode(connector), > - NULL); > + NULL, 0); > intel_dvo->panel_wants_dither = true; > } > > diff --git a/drivers/gpu/drm/i915/intel_lvds.c b/drivers/gpu/drm/i915/intel_lvds.c > index 49550470483e..07d7ac2c1520 100644 > --- a/drivers/gpu/drm/i915/intel_lvds.c > +++ b/drivers/gpu/drm/i915/intel_lvds.c > @@ -1120,7 +1120,8 @@ void intel_lvds_init(struct drm_device *dev) > out: > mutex_unlock(&dev->mode_config.mutex); > > - intel_panel_init(&intel_connector->panel, fixed_mode, downclock_mode); > + /* FIXME fill in an accurate power cycle delay */ > + intel_panel_init(&intel_connector->panel, fixed_mode, downclock_mode, 0); > intel_panel_setup_backlight(connector, INVALID_PIPE); > > lvds_encoder->is_dual_link = compute_is_dual_link_lvds(lvds_encoder); > diff --git a/drivers/gpu/drm/i915/intel_panel.c b/drivers/gpu/drm/i915/intel_panel.c > index 96c65d77e886..fe57a743ad21 100644 > --- a/drivers/gpu/drm/i915/intel_panel.c > +++ b/drivers/gpu/drm/i915/intel_panel.c > @@ -1781,14 +1781,41 @@ intel_panel_init_backlight_funcs(struct intel_panel *panel) > } > } > > +/* > + * Make sure the power cycle delay is respected. The PPS > + * does supposedly initiate a power cycle on reset, but it > + * also resets the power cycle delay register value to the > + * default value, and hence may not wait long enough if the > + * firmware attempts to power up the panel immediately. > + * Also eg. DSI doesn't use the PPS. > + */ > +void intel_panel_reboot_notifier(struct drm_i915_private *dev_priv) > +{ > + struct drm_device *dev = &dev_priv->drm; > + struct intel_connector *connector; > + int reboot_power_cycle_delay = 0; > + > + for_each_intel_connector(dev, connector) { > + struct intel_panel *panel = &connector->panel; > + > + reboot_power_cycle_delay = max(reboot_power_cycle_delay, > + panel->reboot_power_cycle_delay); > + } > + > + if (reboot_power_cycle_delay) > + msleep(reboot_power_cycle_delay); > +} > + > int intel_panel_init(struct intel_panel *panel, > struct drm_display_mode *fixed_mode, > - struct drm_display_mode *downclock_mode) > + struct drm_display_mode *downclock_mode, > + int reboot_power_cycle_delay) > { > intel_panel_init_backlight_funcs(panel); > > panel->fixed_mode = fixed_mode; > panel->downclock_mode = downclock_mode; > + panel->reboot_power_cycle_delay = reboot_power_cycle_delay; > > return 0; > } > -- > 2.7.4 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
On Sat, Aug 06, 2016 at 10:09:16AM +0200, Lukas Wunner wrote: > On Wed, Aug 03, 2016 at 04:36:18PM +0300, ville.syrjala@linux.intel.com wrote: > > From: Ville Syrjälä <ville.syrjala@linux.intel.com> > > > > Dell UP2414Q doesn't like it when we're driving it in MST mode, and then > > reboot the machine. After reboot, the monitor is somehow confused and > > refuses to do the payload allocation: > > [drm:drm_dp_mst_allocate_vcpi] initing vcpi for 282 8 > > [drm:drm_dp_dpcd_write_payload] status not set after read payload table status 0 > > and thus we're left with a black screen until the monitor power is > > toggled. > > > > It seems that if we shut down things gracefully prior to rebooting, the > > monitor doesn't get confused like this. So let's try to shut down all > > the displays gracefully on reboot. The downside is that we will > > introduce the reboot notifier to all the modesetl locks. So if there's > > a locking bug around, we might not be able to reboot the machine > > gracefully. sysrq reboot will still work though, as it will not > > call the notifiers. > > struct pci_driver has a ->shutdown hook which is currently not defined > in i915_pci_driver. How about using that instead of reboot_notifier > to avoid potential locking issues? Interestingly that is also called prior to kexec. Doing a i915_gem_suspend at that point I think would stop some of the GPU faults we get across kexec. -Chris
On 06/08/16 09:29, Chris Wilson wrote: > On Sat, Aug 06, 2016 at 10:09:16AM +0200, Lukas Wunner wrote: >> On Wed, Aug 03, 2016 at 04:36:18PM +0300, ville.syrjala@linux.intel.com wrote: >>> From: Ville Syrjälä <ville.syrjala@linux.intel.com> >>> >>> Dell UP2414Q doesn't like it when we're driving it in MST mode, and then >>> reboot the machine. After reboot, the monitor is somehow confused and >>> refuses to do the payload allocation: >>> [drm:drm_dp_mst_allocate_vcpi] initing vcpi for 282 8 >>> [drm:drm_dp_dpcd_write_payload] status not set after read payload table status 0 >>> and thus we're left with a black screen until the monitor power is >>> toggled. >>> >>> It seems that if we shut down things gracefully prior to rebooting, the >>> monitor doesn't get confused like this. So let's try to shut down all >>> the displays gracefully on reboot. The downside is that we will >>> introduce the reboot notifier to all the modesetl locks. So if there's >>> a locking bug around, we might not be able to reboot the machine >>> gracefully. sysrq reboot will still work though, as it will not >>> call the notifiers. >> >> struct pci_driver has a ->shutdown hook which is currently not defined >> in i915_pci_driver. How about using that instead of reboot_notifier >> to avoid potential locking issues? > > Interestingly that is also called prior to kexec. Doing a > i915_gem_suspend at that point I think would stop some of the GPU faults > we get across kexec. > -Chris Agreed; we've had issues with things such as the GuC being left running across kexec, which causes firmware load to fail cos it's write-once! Doing a clean suspend before kexec would certainly help here. .Dave.
On Mon, Aug 08, 2016 at 12:47:09PM +0100, Dave Gordon wrote: > On 06/08/16 09:29, Chris Wilson wrote: > > On Sat, Aug 06, 2016 at 10:09:16AM +0200, Lukas Wunner wrote: > >> On Wed, Aug 03, 2016 at 04:36:18PM +0300, ville.syrjala@linux.intel.com wrote: > >>> From: Ville Syrjälä <ville.syrjala@linux.intel.com> > >>> > >>> Dell UP2414Q doesn't like it when we're driving it in MST mode, and then > >>> reboot the machine. After reboot, the monitor is somehow confused and > >>> refuses to do the payload allocation: > >>> [drm:drm_dp_mst_allocate_vcpi] initing vcpi for 282 8 > >>> [drm:drm_dp_dpcd_write_payload] status not set after read payload table status 0 > >>> and thus we're left with a black screen until the monitor power is > >>> toggled. > >>> > >>> It seems that if we shut down things gracefully prior to rebooting, the > >>> monitor doesn't get confused like this. So let's try to shut down all > >>> the displays gracefully on reboot. The downside is that we will > >>> introduce the reboot notifier to all the modesetl locks. So if there's > >>> a locking bug around, we might not be able to reboot the machine > >>> gracefully. sysrq reboot will still work though, as it will not > >>> call the notifiers. > >> > >> struct pci_driver has a ->shutdown hook which is currently not defined > >> in i915_pci_driver. How about using that instead of reboot_notifier > >> to avoid potential locking issues? > > > > Interestingly that is also called prior to kexec. Doing a > > i915_gem_suspend at that point I think would stop some of the GPU faults > > we get across kexec. > > -Chris > > Agreed; we've had issues with things such as the GuC being left running > across kexec, which causes firmware load to fail cos it's write-once! > Doing a clean suspend before kexec would certainly help here. I also realized that even my display suspend is somewhat insufficient as I didn't consider hpds, ->detect() etc. potentially turning on vdd again. I thought I might have to start reworking the hpd disable to not depend on intel_runtime_pm_disable_interrupts(), but if we're going to want a full blown suspend anyway I might not bother.
On Fri, Aug 05, 2016 at 03:45:19PM -0700, Clint Taylor wrote: > On 08/03/2016 06:36 AM, ville.syrjala@linux.intel.com wrote: > > From: Ville Syrjälä <ville.syrjala@linux.intel.com> > > > > Dell UP2414Q doesn't like it when we're driving it in MST mode, and then > > reboot the machine. After reboot, the monitor is somehow confused and > > refuses to do the payload allocation: > > [drm:drm_dp_mst_allocate_vcpi] initing vcpi for 282 8 > > [drm:drm_dp_dpcd_write_payload] status not set after read payload table status 0 > > and thus we're left with a black screen until the monitor power is > > toggled. > > > > It seems that if we shut down things gracefully prior to rebooting, the > > monitor doesn't get confused like this. So let's try to shut down all > > the displays gracefully on reboot. The downside is that we will > > introduce the reboot notifier to all the modesetl locks. So if there's > > a locking bug around, we might not be able to reboot the machine > > gracefully. sysrq reboot will still work though, as it will not > > call the notifiers. > > > > While we do this, we can eliminate the eDP reboot notifier, which is > > there to shut down the panel power and make sure the firmware won't > > violate the panel power cycle delay post-reboot. Since we're now > > shutting eDP down properly, we can mostly just rip out the eDP notifier. > > We still need to do the panel power cycle wait though, as that would > > normally get postponed until the next modeset. So let's move that part > > into intel_panel so that other display types will get the same treatment > > as a bonus. > > > > The Dell UP2414Q can often get even more confused, and sometimes what > > you have to do is: switch to another input on the monitor, toggle the > > monitor power, switch the input back to the original setting. And > > sometimes it seems you just have to yank the power cable entirely. I'm > > not sure if this reboot notifier might avoid some of these other > > failure modes as well, but I'm pretty sure it can't hurt at least. > > > > While testing this change on SKL if 2 crtc's are enabled resume from > suspend is 500ms longer than a single crtc resume. Are we calling the > T12 msleep(500) for non eDP panels? Seems unlikely, unless we misdetect DP as eDP. 500ms sure sounds a lot for non-eDP though. > > During testing VDD to the panel, the T12 panel timeout requirement is > being meet at >500ms during reboot and suspend/resume. > > > Cc: Clint Taylor <clinton.a.taylor@intel.com> > > Cc: Paulo Zanoni <paulo.r.zanoni@intel.com> > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com> > > --- > > drivers/gpu/drm/i915/i915_drv.h | 2 ++ > > drivers/gpu/drm/i915/intel_display.c | 24 ++++++++++++++++++ > > drivers/gpu/drm/i915/intel_dp.c | 49 ++---------------------------------- > > drivers/gpu/drm/i915/intel_drv.h | 7 +++--- > > drivers/gpu/drm/i915/intel_dsi.c | 3 ++- > > drivers/gpu/drm/i915/intel_dvo.c | 2 +- > > drivers/gpu/drm/i915/intel_lvds.c | 3 ++- > > drivers/gpu/drm/i915/intel_panel.c | 29 ++++++++++++++++++++- > > 8 files changed, 65 insertions(+), 54 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h > > index 65ada5d2f88c..f8eb8c7de007 100644 > > --- a/drivers/gpu/drm/i915/i915_drv.h > > +++ b/drivers/gpu/drm/i915/i915_drv.h > > @@ -1727,6 +1727,8 @@ struct intel_wm_config { > > struct drm_i915_private { > > struct drm_device drm; > > > > + struct notifier_block reboot_notifier; > > + > > struct kmem_cache *objects; > > struct kmem_cache *vmas; > > struct kmem_cache *requests; > > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c > > index a8e8cc8dfae9..2192a21aa6c9 100644 > > --- a/drivers/gpu/drm/i915/intel_display.c > > +++ b/drivers/gpu/drm/i915/intel_display.c > > @@ -31,6 +31,8 @@ > > #include <linux/kernel.h> > > #include <linux/slab.h> > > #include <linux/vgaarb.h> > > +#include <linux/notifier.h> > > +#include <linux/reboot.h> > > #include <drm/drm_edid.h> > > #include <drm/drmP.h> > > #include "intel_drv.h" > > @@ -15578,6 +15580,23 @@ fail: > > drm_modeset_acquire_fini(&ctx); > > } > > > > + > > +static int intel_reboot_notifier(struct notifier_block *nb, > > + unsigned long code, void *unused) > > +{ > > + struct drm_i915_private *dev_priv = > > + container_of(nb, typeof(*dev_priv), reboot_notifier); > > + > > + if (code != SYS_RESTART) > > + return 0; > > + > > + intel_display_suspend(&dev_priv->drm); > > + > > Need to check to make sure there is a panel before calling > intel_panel_reboot_notifier(). > > > + intel_panel_reboot_notifier(dev_priv); > > + > > + return 0; > > +} > > + > > void intel_modeset_init(struct drm_device *dev) > > { > > struct drm_i915_private *dev_priv = to_i915(dev); > > @@ -15706,6 +15725,9 @@ void intel_modeset_init(struct drm_device *dev) > > * since the watermark calculation done here will use pstate->fb. > > */ > > sanitize_watermarks(dev); > > + > > + dev_priv->reboot_notifier.notifier_call = intel_reboot_notifier; > > + register_reboot_notifier(&dev_priv->reboot_notifier); > > } > > > > static void intel_enable_pipe_a(struct drm_device *dev) > > @@ -16291,6 +16313,8 @@ void intel_modeset_cleanup(struct drm_device *dev) > > { > > struct drm_i915_private *dev_priv = to_i915(dev); > > > > + unregister_reboot_notifier(&dev_priv->reboot_notifier); > > + > > intel_disable_gt_powersave(dev_priv); > > > > /* > > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c > > index 001f74fc0ce5..d8d13a9e33e5 100644 > > --- a/drivers/gpu/drm/i915/intel_dp.c > > +++ b/drivers/gpu/drm/i915/intel_dp.c > > @@ -28,8 +28,6 @@ > > #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_atomic_helper.h> > > #include <drm/drm_crtc.h> > > @@ -631,42 +629,6 @@ _pp_stat_reg(struct intel_dp *intel_dp) > > return regs.pp_stat; > > } > > > > -/* 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 = to_i915(dev); > > - > > - if (!is_edp(intel_dp) || code != SYS_RESTART) > > - return 0; > > - > > - pps_lock(intel_dp); > > - > > - if (IS_VALLEYVIEW(dev) || IS_CHERRYVIEW(dev)) { > > - enum pipe pipe = vlv_power_sequencer_pipe(intel_dp); > > - i915_reg_t pp_ctrl_reg, pp_div_reg; > > - u32 pp_div; > > - > > - 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); > > - } > > - > > - pps_unlock(intel_dp); > > - > > - return 0; > > -} > > - > > static bool edp_have_panel_power(struct intel_dp *intel_dp) > > { > > struct drm_device *dev = intel_dp_to_dev(intel_dp); > > @@ -4562,11 +4524,6 @@ void intel_dp_encoder_destroy(struct drm_encoder *encoder) > > pps_lock(intel_dp); > > edp_panel_vdd_off_sync(intel_dp); > > pps_unlock(intel_dp); > > - > > - if (intel_dp->edp_notifier.notifier_call) { > > - unregister_reboot_notifier(&intel_dp->edp_notifier); > > - intel_dp->edp_notifier.notifier_call = NULL; > > - } > > } > > > > intel_dp_aux_fini(intel_dp); > > @@ -5465,9 +5422,6 @@ static bool intel_edp_init_connector(struct intel_dp *intel_dp, > > mutex_unlock(&dev->mode_config.mutex); > > > > if (IS_VALLEYVIEW(dev) || IS_CHERRYVIEW(dev)) { > > - intel_dp->edp_notifier.notifier_call = edp_notify_handler; > > - register_reboot_notifier(&intel_dp->edp_notifier); > > - > > /* > > * Figure out the current pipe for the initial backlight setup. > > * If the current pipe isn't valid, try the PPS pipe, and if that > > @@ -5488,7 +5442,8 @@ static bool intel_edp_init_connector(struct intel_dp *intel_dp, > > pipe_name(pipe)); > > } > > > > - intel_panel_init(&intel_connector->panel, fixed_mode, downclock_mode); > > + intel_panel_init(&intel_connector->panel, fixed_mode, downclock_mode, > > + intel_dp->panel_power_cycle_delay); > > intel_connector->panel.backlight.power = intel_edp_backlight_power; > > intel_panel_setup_backlight(connector, pipe); > > > > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h > > index 50cdc890d956..92fa66a02ca5 100644 > > --- a/drivers/gpu/drm/i915/intel_drv.h > > +++ b/drivers/gpu/drm/i915/intel_drv.h > > @@ -226,6 +226,7 @@ struct intel_panel { > > struct drm_display_mode *fixed_mode; > > struct drm_display_mode *downclock_mode; > > int fitting_mode; > > + int reboot_power_cycle_delay; > > > > /* backlight */ > > struct { > > @@ -877,8 +878,6 @@ struct intel_dp { > > unsigned long last_backlight_off; > > ktime_t panel_power_off_time; > > > > - struct notifier_block edp_notifier; > > - > > /* > > * Pipe whose power sequencer is currently locked into > > * this port. Only relevant on VLV/CHV. > > @@ -1521,8 +1520,10 @@ void intel_overlay_reset(struct drm_i915_private *dev_priv); > > /* intel_panel.c */ > > int intel_panel_init(struct intel_panel *panel, > > struct drm_display_mode *fixed_mode, > > - struct drm_display_mode *downclock_mode); > > + struct drm_display_mode *downclock_mode, > > + int reboot_power_cycle_delay); > > void intel_panel_fini(struct intel_panel *panel); > > +void intel_panel_reboot_notifier(struct drm_i915_private *dev_priv); > > void intel_fixed_panel_mode(const struct drm_display_mode *fixed_mode, > > struct drm_display_mode *adjusted_mode); > > void intel_pch_panel_fitting(struct intel_crtc *crtc, > > diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c > > index de8e9fb51595..5b722ec23381 100644 > > --- a/drivers/gpu/drm/i915/intel_dsi.c > > +++ b/drivers/gpu/drm/i915/intel_dsi.c > > @@ -1596,7 +1596,8 @@ void intel_dsi_init(struct drm_device *dev) > > connector->display_info.width_mm = fixed_mode->width_mm; > > connector->display_info.height_mm = fixed_mode->height_mm; > > > > - intel_panel_init(&intel_connector->panel, fixed_mode, NULL); > > + intel_panel_init(&intel_connector->panel, fixed_mode, NULL, > > + intel_dsi->panel_pwr_cycle_delay); > > intel_panel_setup_backlight(connector, INVALID_PIPE); > > > > intel_dsi_add_properties(intel_connector); > > diff --git a/drivers/gpu/drm/i915/intel_dvo.c b/drivers/gpu/drm/i915/intel_dvo.c > > index 47bdf9dad0d3..c2c9c922590c 100644 > > --- a/drivers/gpu/drm/i915/intel_dvo.c > > +++ b/drivers/gpu/drm/i915/intel_dvo.c > > @@ -547,7 +547,7 @@ void intel_dvo_init(struct drm_device *dev) > > */ > > intel_panel_init(&intel_connector->panel, > > intel_dvo_get_current_mode(connector), > > - NULL); > > + NULL, 0); > > intel_dvo->panel_wants_dither = true; > > } > > > > diff --git a/drivers/gpu/drm/i915/intel_lvds.c b/drivers/gpu/drm/i915/intel_lvds.c > > index 49550470483e..07d7ac2c1520 100644 > > --- a/drivers/gpu/drm/i915/intel_lvds.c > > +++ b/drivers/gpu/drm/i915/intel_lvds.c > > @@ -1120,7 +1120,8 @@ void intel_lvds_init(struct drm_device *dev) > > out: > > mutex_unlock(&dev->mode_config.mutex); > > > > - intel_panel_init(&intel_connector->panel, fixed_mode, downclock_mode); > > + /* FIXME fill in an accurate power cycle delay */ > > + intel_panel_init(&intel_connector->panel, fixed_mode, downclock_mode, 0); > > intel_panel_setup_backlight(connector, INVALID_PIPE); > > > > lvds_encoder->is_dual_link = compute_is_dual_link_lvds(lvds_encoder); > > diff --git a/drivers/gpu/drm/i915/intel_panel.c b/drivers/gpu/drm/i915/intel_panel.c > > index 96c65d77e886..fe57a743ad21 100644 > > --- a/drivers/gpu/drm/i915/intel_panel.c > > +++ b/drivers/gpu/drm/i915/intel_panel.c > > @@ -1781,14 +1781,41 @@ intel_panel_init_backlight_funcs(struct intel_panel *panel) > > } > > } > > > > +/* > > + * Make sure the power cycle delay is respected. The PPS > > + * does supposedly initiate a power cycle on reset, but it > > + * also resets the power cycle delay register value to the > > + * default value, and hence may not wait long enough if the > > + * firmware attempts to power up the panel immediately. > > + * Also eg. DSI doesn't use the PPS. > > + */ > > +void intel_panel_reboot_notifier(struct drm_i915_private *dev_priv) > > +{ > > + struct drm_device *dev = &dev_priv->drm; > > + struct intel_connector *connector; > > + int reboot_power_cycle_delay = 0; > > + > > + for_each_intel_connector(dev, connector) { > > + struct intel_panel *panel = &connector->panel; > > + > > + reboot_power_cycle_delay = max(reboot_power_cycle_delay, > > + panel->reboot_power_cycle_delay); > > + } > > + > > + if (reboot_power_cycle_delay) > > + msleep(reboot_power_cycle_delay); > > +} > > + > > int intel_panel_init(struct intel_panel *panel, > > struct drm_display_mode *fixed_mode, > > - struct drm_display_mode *downclock_mode) > > + struct drm_display_mode *downclock_mode, > > + int reboot_power_cycle_delay) > > { > > intel_panel_init_backlight_funcs(panel); > > > > panel->fixed_mode = fixed_mode; > > panel->downclock_mode = downclock_mode; > > + panel->reboot_power_cycle_delay = reboot_power_cycle_delay; > > > > return 0; > > } > >
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 65ada5d2f88c..f8eb8c7de007 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -1727,6 +1727,8 @@ struct intel_wm_config { struct drm_i915_private { struct drm_device drm; + struct notifier_block reboot_notifier; + struct kmem_cache *objects; struct kmem_cache *vmas; struct kmem_cache *requests; diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index a8e8cc8dfae9..2192a21aa6c9 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -31,6 +31,8 @@ #include <linux/kernel.h> #include <linux/slab.h> #include <linux/vgaarb.h> +#include <linux/notifier.h> +#include <linux/reboot.h> #include <drm/drm_edid.h> #include <drm/drmP.h> #include "intel_drv.h" @@ -15578,6 +15580,23 @@ fail: drm_modeset_acquire_fini(&ctx); } + +static int intel_reboot_notifier(struct notifier_block *nb, + unsigned long code, void *unused) +{ + struct drm_i915_private *dev_priv = + container_of(nb, typeof(*dev_priv), reboot_notifier); + + if (code != SYS_RESTART) + return 0; + + intel_display_suspend(&dev_priv->drm); + + intel_panel_reboot_notifier(dev_priv); + + return 0; +} + void intel_modeset_init(struct drm_device *dev) { struct drm_i915_private *dev_priv = to_i915(dev); @@ -15706,6 +15725,9 @@ void intel_modeset_init(struct drm_device *dev) * since the watermark calculation done here will use pstate->fb. */ sanitize_watermarks(dev); + + dev_priv->reboot_notifier.notifier_call = intel_reboot_notifier; + register_reboot_notifier(&dev_priv->reboot_notifier); } static void intel_enable_pipe_a(struct drm_device *dev) @@ -16291,6 +16313,8 @@ void intel_modeset_cleanup(struct drm_device *dev) { struct drm_i915_private *dev_priv = to_i915(dev); + unregister_reboot_notifier(&dev_priv->reboot_notifier); + intel_disable_gt_powersave(dev_priv); /* diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c index 001f74fc0ce5..d8d13a9e33e5 100644 --- a/drivers/gpu/drm/i915/intel_dp.c +++ b/drivers/gpu/drm/i915/intel_dp.c @@ -28,8 +28,6 @@ #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_atomic_helper.h> #include <drm/drm_crtc.h> @@ -631,42 +629,6 @@ _pp_stat_reg(struct intel_dp *intel_dp) return regs.pp_stat; } -/* 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 = to_i915(dev); - - if (!is_edp(intel_dp) || code != SYS_RESTART) - return 0; - - pps_lock(intel_dp); - - if (IS_VALLEYVIEW(dev) || IS_CHERRYVIEW(dev)) { - enum pipe pipe = vlv_power_sequencer_pipe(intel_dp); - i915_reg_t pp_ctrl_reg, pp_div_reg; - u32 pp_div; - - 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); - } - - pps_unlock(intel_dp); - - return 0; -} - static bool edp_have_panel_power(struct intel_dp *intel_dp) { struct drm_device *dev = intel_dp_to_dev(intel_dp); @@ -4562,11 +4524,6 @@ void intel_dp_encoder_destroy(struct drm_encoder *encoder) pps_lock(intel_dp); edp_panel_vdd_off_sync(intel_dp); pps_unlock(intel_dp); - - if (intel_dp->edp_notifier.notifier_call) { - unregister_reboot_notifier(&intel_dp->edp_notifier); - intel_dp->edp_notifier.notifier_call = NULL; - } } intel_dp_aux_fini(intel_dp); @@ -5465,9 +5422,6 @@ static bool intel_edp_init_connector(struct intel_dp *intel_dp, mutex_unlock(&dev->mode_config.mutex); if (IS_VALLEYVIEW(dev) || IS_CHERRYVIEW(dev)) { - intel_dp->edp_notifier.notifier_call = edp_notify_handler; - register_reboot_notifier(&intel_dp->edp_notifier); - /* * Figure out the current pipe for the initial backlight setup. * If the current pipe isn't valid, try the PPS pipe, and if that @@ -5488,7 +5442,8 @@ static bool intel_edp_init_connector(struct intel_dp *intel_dp, pipe_name(pipe)); } - intel_panel_init(&intel_connector->panel, fixed_mode, downclock_mode); + intel_panel_init(&intel_connector->panel, fixed_mode, downclock_mode, + intel_dp->panel_power_cycle_delay); intel_connector->panel.backlight.power = intel_edp_backlight_power; intel_panel_setup_backlight(connector, pipe); diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h index 50cdc890d956..92fa66a02ca5 100644 --- a/drivers/gpu/drm/i915/intel_drv.h +++ b/drivers/gpu/drm/i915/intel_drv.h @@ -226,6 +226,7 @@ struct intel_panel { struct drm_display_mode *fixed_mode; struct drm_display_mode *downclock_mode; int fitting_mode; + int reboot_power_cycle_delay; /* backlight */ struct { @@ -877,8 +878,6 @@ struct intel_dp { unsigned long last_backlight_off; ktime_t panel_power_off_time; - struct notifier_block edp_notifier; - /* * Pipe whose power sequencer is currently locked into * this port. Only relevant on VLV/CHV. @@ -1521,8 +1520,10 @@ void intel_overlay_reset(struct drm_i915_private *dev_priv); /* intel_panel.c */ int intel_panel_init(struct intel_panel *panel, struct drm_display_mode *fixed_mode, - struct drm_display_mode *downclock_mode); + struct drm_display_mode *downclock_mode, + int reboot_power_cycle_delay); void intel_panel_fini(struct intel_panel *panel); +void intel_panel_reboot_notifier(struct drm_i915_private *dev_priv); void intel_fixed_panel_mode(const struct drm_display_mode *fixed_mode, struct drm_display_mode *adjusted_mode); void intel_pch_panel_fitting(struct intel_crtc *crtc, diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c index de8e9fb51595..5b722ec23381 100644 --- a/drivers/gpu/drm/i915/intel_dsi.c +++ b/drivers/gpu/drm/i915/intel_dsi.c @@ -1596,7 +1596,8 @@ void intel_dsi_init(struct drm_device *dev) connector->display_info.width_mm = fixed_mode->width_mm; connector->display_info.height_mm = fixed_mode->height_mm; - intel_panel_init(&intel_connector->panel, fixed_mode, NULL); + intel_panel_init(&intel_connector->panel, fixed_mode, NULL, + intel_dsi->panel_pwr_cycle_delay); intel_panel_setup_backlight(connector, INVALID_PIPE); intel_dsi_add_properties(intel_connector); diff --git a/drivers/gpu/drm/i915/intel_dvo.c b/drivers/gpu/drm/i915/intel_dvo.c index 47bdf9dad0d3..c2c9c922590c 100644 --- a/drivers/gpu/drm/i915/intel_dvo.c +++ b/drivers/gpu/drm/i915/intel_dvo.c @@ -547,7 +547,7 @@ void intel_dvo_init(struct drm_device *dev) */ intel_panel_init(&intel_connector->panel, intel_dvo_get_current_mode(connector), - NULL); + NULL, 0); intel_dvo->panel_wants_dither = true; } diff --git a/drivers/gpu/drm/i915/intel_lvds.c b/drivers/gpu/drm/i915/intel_lvds.c index 49550470483e..07d7ac2c1520 100644 --- a/drivers/gpu/drm/i915/intel_lvds.c +++ b/drivers/gpu/drm/i915/intel_lvds.c @@ -1120,7 +1120,8 @@ void intel_lvds_init(struct drm_device *dev) out: mutex_unlock(&dev->mode_config.mutex); - intel_panel_init(&intel_connector->panel, fixed_mode, downclock_mode); + /* FIXME fill in an accurate power cycle delay */ + intel_panel_init(&intel_connector->panel, fixed_mode, downclock_mode, 0); intel_panel_setup_backlight(connector, INVALID_PIPE); lvds_encoder->is_dual_link = compute_is_dual_link_lvds(lvds_encoder); diff --git a/drivers/gpu/drm/i915/intel_panel.c b/drivers/gpu/drm/i915/intel_panel.c index 96c65d77e886..fe57a743ad21 100644 --- a/drivers/gpu/drm/i915/intel_panel.c +++ b/drivers/gpu/drm/i915/intel_panel.c @@ -1781,14 +1781,41 @@ intel_panel_init_backlight_funcs(struct intel_panel *panel) } } +/* + * Make sure the power cycle delay is respected. The PPS + * does supposedly initiate a power cycle on reset, but it + * also resets the power cycle delay register value to the + * default value, and hence may not wait long enough if the + * firmware attempts to power up the panel immediately. + * Also eg. DSI doesn't use the PPS. + */ +void intel_panel_reboot_notifier(struct drm_i915_private *dev_priv) +{ + struct drm_device *dev = &dev_priv->drm; + struct intel_connector *connector; + int reboot_power_cycle_delay = 0; + + for_each_intel_connector(dev, connector) { + struct intel_panel *panel = &connector->panel; + + reboot_power_cycle_delay = max(reboot_power_cycle_delay, + panel->reboot_power_cycle_delay); + } + + if (reboot_power_cycle_delay) + msleep(reboot_power_cycle_delay); +} + int intel_panel_init(struct intel_panel *panel, struct drm_display_mode *fixed_mode, - struct drm_display_mode *downclock_mode) + struct drm_display_mode *downclock_mode, + int reboot_power_cycle_delay) { intel_panel_init_backlight_funcs(panel); panel->fixed_mode = fixed_mode; panel->downclock_mode = downclock_mode; + panel->reboot_power_cycle_delay = reboot_power_cycle_delay; return 0; }