drm/i915: Nuke the LVDS lid notifier
authorVille Syrjälä <ville.syrjala@linux.intel.com>
Tue, 17 Jul 2018 17:42:14 +0000 (20:42 +0300)
committerVille Syrjälä <ville.syrjala@linux.intel.com>
Thu, 19 Jul 2018 15:39:31 +0000 (18:39 +0300)
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().

v2: Nuke enum modeset_restore (Rodrigo)

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>
Link: https://patchwork.freedesktop.org/patch/msgid/20180717174216.22252-1-ville.syrjala@linux.intel.com
Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>
drivers/gpu/drm/i915/i915_drv.c
drivers/gpu/drm/i915/i915_drv.h
drivers/gpu/drm/i915/intel_lvds.c

index 337b1aad5212b01057389f2a767666151960d38f..343e79a44abd18d541740dc2601523f9b117f63d 100644 (file)
@@ -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);
index 08d4303abb1468639cc9b19e09874aa18f74f973..995656f51b57454e72a5365adac6e6650e688019 100644 (file)
@@ -1002,12 +1002,6 @@ struct i915_gem_mm {
 
 #define I915_ENGINE_WEDGED_TIMEOUT  (60 * HZ)  /* Reset but no recovery? */
 
-enum modeset_restore {
-       MODESET_ON_LID_OPEN,
-       MODESET_DONE,
-       MODESET_SUSPENDED,
-};
-
 #define DP_AUX_A 0x40
 #define DP_AUX_B 0x10
 #define DP_AUX_C 0x20
@@ -1730,8 +1724,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;
 
index bb06744d28a432c1d481d24debecbd51fd21ff8b..a354041192574e0721a2803e4dc54c8fb6ded7ff 100644 (file)
@@ -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
         */
 
        /*