| Message ID | 20180717174216.22252-1-ville.syrjala@linux.intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Tue, Jul 17, 2018 at 08:42:14PM +0300, Ville Syrjala wrote: > From: Ville Syrjälä <ville.syrjala@linux.intel.com> > > We broke the LVDS notifier resume thing in (presumably) commit > e2c8b8701e2d ("drm/i915: Use atomic helpers for suspend, v2.") as > we no longer duplicate the current state in the LVDS notifier and > thus we never resume it properly either. > > Instead of trying to fix it again let's just kill off the lid > notifier entirely. None of the machines tested thus far have > apparently needed it. Originally the lid notifier was added to > work around cases where the VBIOS was clobbering some of the > hardware state behind the driver's back, mostly on Thinkpads. > We now have a few report of Thinkpads working just fine without > the notifier. So maybe it was misdiagnosed originally, or > something else has changed (ACPI video stuff perhaps?). > > If we do end up finding a machine where the VBIOS is still causing > problems I would suggest that we first try setting various bits in > the VBIOS scratch registers. There are several to choose from that > may instruct the VBIOS to steer clear. > > With the notifier gone we'll also stop looking at the panel status > in ->detect(). > > Cc: stable@vger.kernel.org > Cc: Wolfgang Draxinger <wdraxinger.maillist@draxit.de> > Cc: Vito Caputo <vcaputo@pengaru.com> > Cc: kitsunyan <kitsunyan@airmail.cc> > Cc: Joonas Saarinen <jza@saunalahti.fi> > Tested-by: Vito Caputo <vcaputo@pengaru.com> # Thinkapd X61s > Tested-by: kitsunyan <kitsunyan@airmail.cc> # ThinkPad X200 > Tested-by: Joonas Saarinen <jza@saunalahti.fi> # Fujitsu Siemens U9210 > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=105902 > References: https://lists.freedesktop.org/archives/intel-gfx/2018-June/169315.html > References: https://bugs.freedesktop.org/show_bug.cgi?id=21230 > Fixes: e2c8b8701e2d ("drm/i915: Use atomic helpers for suspend, v2.") > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com> > --- > drivers/gpu/drm/i915/i915_drv.c | 10 --- > drivers/gpu/drm/i915/i915_drv.h | 2 - > drivers/gpu/drm/i915/intel_lvds.c | 136 +------------------------------------- > 3 files changed, 2 insertions(+), 146 deletions(-) > > diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c > index 3834bd758a2e..f8cfd16be534 100644 > --- a/drivers/gpu/drm/i915/i915_drv.c > +++ b/drivers/gpu/drm/i915/i915_drv.c > @@ -900,7 +900,6 @@ static int i915_driver_init_early(struct drm_i915_private *dev_priv, > spin_lock_init(&dev_priv->uncore.lock); > > mutex_init(&dev_priv->sb_lock); > - mutex_init(&dev_priv->modeset_restore_lock); > mutex_init(&dev_priv->av_mutex); > mutex_init(&dev_priv->wm.wm_mutex); > mutex_init(&dev_priv->pps_mutex); > @@ -1570,11 +1569,6 @@ static int i915_drm_suspend(struct drm_device *dev) > struct pci_dev *pdev = dev_priv->drm.pdev; > pci_power_t opregion_target_state; > > - /* ignore lid events during suspend */ > - mutex_lock(&dev_priv->modeset_restore_lock); > - dev_priv->modeset_restore = MODESET_SUSPENDED; > - mutex_unlock(&dev_priv->modeset_restore_lock); > - > disable_rpm_wakeref_asserts(dev_priv); > > /* We do a lot of poking in a lot of registers, make sure they work > @@ -1770,10 +1764,6 @@ static int i915_drm_resume(struct drm_device *dev) > > intel_fbdev_set_suspend(dev, FBINFO_STATE_RUNNING, false); > > - mutex_lock(&dev_priv->modeset_restore_lock); > - dev_priv->modeset_restore = MODESET_DONE; > - mutex_unlock(&dev_priv->modeset_restore_lock); > - > intel_opregion_notify_adapter(dev_priv, PCI_D0); > > enable_rpm_wakeref_asserts(dev_priv); > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h > index 4fb937399440..1b0af905b74c 100644 > --- a/drivers/gpu/drm/i915/i915_drv.h > +++ b/drivers/gpu/drm/i915/i915_drv.h - enum modeset_restore { - MODESET_ON_LID_OPEN, - MODESET_DONE, - MODESET_SUSPENDED, - }; with that: Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com> > @@ -1730,8 +1730,6 @@ struct drm_i915_private { > > unsigned long quirks; > > - enum modeset_restore modeset_restore; > - struct mutex modeset_restore_lock; > struct drm_atomic_state *modeset_restore_state; > struct drm_modeset_acquire_ctx reset_ctx; > > diff --git a/drivers/gpu/drm/i915/intel_lvds.c b/drivers/gpu/drm/i915/intel_lvds.c > index ca55b0a82ba6..f9f3b0885ba5 100644 > --- a/drivers/gpu/drm/i915/intel_lvds.c > +++ b/drivers/gpu/drm/i915/intel_lvds.c > @@ -44,8 +44,6 @@ > /* Private structure for the integrated LVDS support */ > struct intel_lvds_connector { > struct intel_connector base; > - > - struct notifier_block lid_notifier; > }; > > struct intel_lvds_pps { > @@ -452,26 +450,9 @@ static bool intel_lvds_compute_config(struct intel_encoder *intel_encoder, > return true; > } > > -/* > - * Detect the LVDS connection. > - * > - * Since LVDS doesn't have hotlug, we use the lid as a proxy. Open means > - * connected and closed means disconnected. We also send hotplug events as > - * needed, using lid status notification from the input layer. > - */ > static enum drm_connector_status > intel_lvds_detect(struct drm_connector *connector, bool force) > { > - struct drm_i915_private *dev_priv = to_i915(connector->dev); > - enum drm_connector_status status; > - > - DRM_DEBUG_KMS("[CONNECTOR:%d:%s]\n", > - connector->base.id, connector->name); > - > - status = intel_panel_detect(dev_priv); > - if (status != connector_status_unknown) > - return status; > - > return connector_status_connected; > } > > @@ -496,117 +477,6 @@ static int intel_lvds_get_modes(struct drm_connector *connector) > return 1; > } > > -static int intel_no_modeset_on_lid_dmi_callback(const struct dmi_system_id *id) > -{ > - DRM_INFO("Skipping forced modeset for %s\n", id->ident); > - return 1; > -} > - > -/* The GPU hangs up on these systems if modeset is performed on LID open */ > -static const struct dmi_system_id intel_no_modeset_on_lid[] = { > - { > - .callback = intel_no_modeset_on_lid_dmi_callback, > - .ident = "Toshiba Tecra A11", > - .matches = { > - DMI_MATCH(DMI_SYS_VENDOR, "TOSHIBA"), > - DMI_MATCH(DMI_PRODUCT_NAME, "TECRA A11"), > - }, > - }, > - > - { } /* terminating entry */ > -}; > - > -/* > - * Lid events. Note the use of 'modeset': > - * - we set it to MODESET_ON_LID_OPEN on lid close, > - * and set it to MODESET_DONE on open > - * - we use it as a "only once" bit (ie we ignore > - * duplicate events where it was already properly set) > - * - the suspend/resume paths will set it to > - * MODESET_SUSPENDED and ignore the lid open event, > - * because they restore the mode ("lid open"). > - */ > -static int intel_lid_notify(struct notifier_block *nb, unsigned long val, > - void *unused) > -{ > - struct intel_lvds_connector *lvds_connector = > - container_of(nb, struct intel_lvds_connector, lid_notifier); > - struct drm_connector *connector = &lvds_connector->base.base; > - struct drm_device *dev = connector->dev; > - struct drm_i915_private *dev_priv = to_i915(dev); > - > - if (dev->switch_power_state != DRM_SWITCH_POWER_ON) > - return NOTIFY_OK; > - > - mutex_lock(&dev_priv->modeset_restore_lock); > - if (dev_priv->modeset_restore == MODESET_SUSPENDED) > - goto exit; > - /* > - * check and update the status of LVDS connector after receiving > - * the LID nofication event. > - */ > - connector->status = connector->funcs->detect(connector, false); > - > - /* Don't force modeset on machines where it causes a GPU lockup */ > - if (dmi_check_system(intel_no_modeset_on_lid)) > - goto exit; > - if (!acpi_lid_open()) { > - /* do modeset on next lid open event */ > - dev_priv->modeset_restore = MODESET_ON_LID_OPEN; > - goto exit; > - } > - > - if (dev_priv->modeset_restore == MODESET_DONE) > - goto exit; > - > - /* > - * Some old platform's BIOS love to wreak havoc while the lid is closed. > - * We try to detect this here and undo any damage. The split for PCH > - * platforms is rather conservative and a bit arbitrary expect that on > - * those platforms VGA disabling requires actual legacy VGA I/O access, > - * and as part of the cleanup in the hw state restore we also redisable > - * the vga plane. > - */ > - if (!HAS_PCH_SPLIT(dev_priv)) > - intel_display_resume(dev); > - > - dev_priv->modeset_restore = MODESET_DONE; > - > -exit: > - mutex_unlock(&dev_priv->modeset_restore_lock); > - return NOTIFY_OK; > -} > - > -static int > -intel_lvds_connector_register(struct drm_connector *connector) > -{ > - struct intel_lvds_connector *lvds = to_lvds_connector(connector); > - int ret; > - > - ret = intel_connector_register(connector); > - if (ret) > - return ret; > - > - lvds->lid_notifier.notifier_call = intel_lid_notify; > - if (acpi_lid_notifier_register(&lvds->lid_notifier)) { > - DRM_DEBUG_KMS("lid notifier registration failed\n"); > - lvds->lid_notifier.notifier_call = NULL; > - } > - > - return 0; > -} > - > -static void > -intel_lvds_connector_unregister(struct drm_connector *connector) > -{ > - struct intel_lvds_connector *lvds = to_lvds_connector(connector); > - > - if (lvds->lid_notifier.notifier_call) > - acpi_lid_notifier_unregister(&lvds->lid_notifier); > - > - intel_connector_unregister(connector); > -} > - > /** > * intel_lvds_destroy - unregister and free LVDS structures > * @connector: connector to free > @@ -639,8 +509,8 @@ static const struct drm_connector_funcs intel_lvds_connector_funcs = { > .fill_modes = drm_helper_probe_single_connector_modes, > .atomic_get_property = intel_digital_connector_atomic_get_property, > .atomic_set_property = intel_digital_connector_atomic_set_property, > - .late_register = intel_lvds_connector_register, > - .early_unregister = intel_lvds_connector_unregister, > + .late_register = intel_connector_register, > + .early_unregister = intel_connector_unregister, > .destroy = intel_lvds_destroy, > .atomic_destroy_state = drm_atomic_helper_connector_destroy_state, > .atomic_duplicate_state = intel_digital_connector_duplicate_state, > @@ -1114,8 +984,6 @@ void intel_lvds_init(struct drm_i915_private *dev_priv) > * 2) check for VBT data > * 3) check to see if LVDS is already on > * if none of the above, no panel > - * 4) make sure lid is open > - * if closed, act like it's not there for now > */ > > /* > -- > 2.16.4 > > _______________________________________________ > Intel-gfx mailing list > Intel-gfx@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c index 3834bd758a2e..f8cfd16be534 100644 --- a/drivers/gpu/drm/i915/i915_drv.c +++ b/drivers/gpu/drm/i915/i915_drv.c @@ -900,7 +900,6 @@ static int i915_driver_init_early(struct drm_i915_private *dev_priv, spin_lock_init(&dev_priv->uncore.lock); mutex_init(&dev_priv->sb_lock); - mutex_init(&dev_priv->modeset_restore_lock); mutex_init(&dev_priv->av_mutex); mutex_init(&dev_priv->wm.wm_mutex); mutex_init(&dev_priv->pps_mutex); @@ -1570,11 +1569,6 @@ static int i915_drm_suspend(struct drm_device *dev) struct pci_dev *pdev = dev_priv->drm.pdev; pci_power_t opregion_target_state; - /* ignore lid events during suspend */ - mutex_lock(&dev_priv->modeset_restore_lock); - dev_priv->modeset_restore = MODESET_SUSPENDED; - mutex_unlock(&dev_priv->modeset_restore_lock); - disable_rpm_wakeref_asserts(dev_priv); /* We do a lot of poking in a lot of registers, make sure they work @@ -1770,10 +1764,6 @@ static int i915_drm_resume(struct drm_device *dev) intel_fbdev_set_suspend(dev, FBINFO_STATE_RUNNING, false); - mutex_lock(&dev_priv->modeset_restore_lock); - dev_priv->modeset_restore = MODESET_DONE; - mutex_unlock(&dev_priv->modeset_restore_lock); - intel_opregion_notify_adapter(dev_priv, PCI_D0); enable_rpm_wakeref_asserts(dev_priv); diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 4fb937399440..1b0af905b74c 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -1730,8 +1730,6 @@ struct drm_i915_private { unsigned long quirks; - enum modeset_restore modeset_restore; - struct mutex modeset_restore_lock; struct drm_atomic_state *modeset_restore_state; struct drm_modeset_acquire_ctx reset_ctx; diff --git a/drivers/gpu/drm/i915/intel_lvds.c b/drivers/gpu/drm/i915/intel_lvds.c index ca55b0a82ba6..f9f3b0885ba5 100644 --- a/drivers/gpu/drm/i915/intel_lvds.c +++ b/drivers/gpu/drm/i915/intel_lvds.c @@ -44,8 +44,6 @@ /* Private structure for the integrated LVDS support */ struct intel_lvds_connector { struct intel_connector base; - - struct notifier_block lid_notifier; }; struct intel_lvds_pps { @@ -452,26 +450,9 @@ static bool intel_lvds_compute_config(struct intel_encoder *intel_encoder, return true; } -/* - * Detect the LVDS connection. - * - * Since LVDS doesn't have hotlug, we use the lid as a proxy. Open means - * connected and closed means disconnected. We also send hotplug events as - * needed, using lid status notification from the input layer. - */ static enum drm_connector_status intel_lvds_detect(struct drm_connector *connector, bool force) { - struct drm_i915_private *dev_priv = to_i915(connector->dev); - enum drm_connector_status status; - - DRM_DEBUG_KMS("[CONNECTOR:%d:%s]\n", - connector->base.id, connector->name); - - status = intel_panel_detect(dev_priv); - if (status != connector_status_unknown) - return status; - return connector_status_connected; } @@ -496,117 +477,6 @@ static int intel_lvds_get_modes(struct drm_connector *connector) return 1; } -static int intel_no_modeset_on_lid_dmi_callback(const struct dmi_system_id *id) -{ - DRM_INFO("Skipping forced modeset for %s\n", id->ident); - return 1; -} - -/* The GPU hangs up on these systems if modeset is performed on LID open */ -static const struct dmi_system_id intel_no_modeset_on_lid[] = { - { - .callback = intel_no_modeset_on_lid_dmi_callback, - .ident = "Toshiba Tecra A11", - .matches = { - DMI_MATCH(DMI_SYS_VENDOR, "TOSHIBA"), - DMI_MATCH(DMI_PRODUCT_NAME, "TECRA A11"), - }, - }, - - { } /* terminating entry */ -}; - -/* - * Lid events. Note the use of 'modeset': - * - we set it to MODESET_ON_LID_OPEN on lid close, - * and set it to MODESET_DONE on open - * - we use it as a "only once" bit (ie we ignore - * duplicate events where it was already properly set) - * - the suspend/resume paths will set it to - * MODESET_SUSPENDED and ignore the lid open event, - * because they restore the mode ("lid open"). - */ -static int intel_lid_notify(struct notifier_block *nb, unsigned long val, - void *unused) -{ - struct intel_lvds_connector *lvds_connector = - container_of(nb, struct intel_lvds_connector, lid_notifier); - struct drm_connector *connector = &lvds_connector->base.base; - struct drm_device *dev = connector->dev; - struct drm_i915_private *dev_priv = to_i915(dev); - - if (dev->switch_power_state != DRM_SWITCH_POWER_ON) - return NOTIFY_OK; - - mutex_lock(&dev_priv->modeset_restore_lock); - if (dev_priv->modeset_restore == MODESET_SUSPENDED) - goto exit; - /* - * check and update the status of LVDS connector after receiving - * the LID nofication event. - */ - connector->status = connector->funcs->detect(connector, false); - - /* Don't force modeset on machines where it causes a GPU lockup */ - if (dmi_check_system(intel_no_modeset_on_lid)) - goto exit; - if (!acpi_lid_open()) { - /* do modeset on next lid open event */ - dev_priv->modeset_restore = MODESET_ON_LID_OPEN; - goto exit; - } - - if (dev_priv->modeset_restore == MODESET_DONE) - goto exit; - - /* - * Some old platform's BIOS love to wreak havoc while the lid is closed. - * We try to detect this here and undo any damage. The split for PCH - * platforms is rather conservative and a bit arbitrary expect that on - * those platforms VGA disabling requires actual legacy VGA I/O access, - * and as part of the cleanup in the hw state restore we also redisable - * the vga plane. - */ - if (!HAS_PCH_SPLIT(dev_priv)) - intel_display_resume(dev); - - dev_priv->modeset_restore = MODESET_DONE; - -exit: - mutex_unlock(&dev_priv->modeset_restore_lock); - return NOTIFY_OK; -} - -static int -intel_lvds_connector_register(struct drm_connector *connector) -{ - struct intel_lvds_connector *lvds = to_lvds_connector(connector); - int ret; - - ret = intel_connector_register(connector); - if (ret) - return ret; - - lvds->lid_notifier.notifier_call = intel_lid_notify; - if (acpi_lid_notifier_register(&lvds->lid_notifier)) { - DRM_DEBUG_KMS("lid notifier registration failed\n"); - lvds->lid_notifier.notifier_call = NULL; - } - - return 0; -} - -static void -intel_lvds_connector_unregister(struct drm_connector *connector) -{ - struct intel_lvds_connector *lvds = to_lvds_connector(connector); - - if (lvds->lid_notifier.notifier_call) - acpi_lid_notifier_unregister(&lvds->lid_notifier); - - intel_connector_unregister(connector); -} - /** * intel_lvds_destroy - unregister and free LVDS structures * @connector: connector to free @@ -639,8 +509,8 @@ static const struct drm_connector_funcs intel_lvds_connector_funcs = { .fill_modes = drm_helper_probe_single_connector_modes, .atomic_get_property = intel_digital_connector_atomic_get_property, .atomic_set_property = intel_digital_connector_atomic_set_property, - .late_register = intel_lvds_connector_register, - .early_unregister = intel_lvds_connector_unregister, + .late_register = intel_connector_register, + .early_unregister = intel_connector_unregister, .destroy = intel_lvds_destroy, .atomic_destroy_state = drm_atomic_helper_connector_destroy_state, .atomic_duplicate_state = intel_digital_connector_duplicate_state, @@ -1114,8 +984,6 @@ void intel_lvds_init(struct drm_i915_private *dev_priv) * 2) check for VBT data * 3) check to see if LVDS is already on * if none of the above, no panel - * 4) make sure lid is open - * if closed, act like it's not there for now */ /*