[Intel-gfx] [PATCH 1/3] drm/i915: Nuke the LVDS lid notifier
Rodrigo Vivi
rodrigo.vivi at intel.com
Tue Jul 17 19:57:51 UTC 2018
On Tue, Jul 17, 2018 at 08:42:14PM +0300, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala at 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 at vger.kernel.org
> Cc: Wolfgang Draxinger <wdraxinger.maillist at draxit.de>
> Cc: Vito Caputo <vcaputo at pengaru.com>
> Cc: kitsunyan <kitsunyan at airmail.cc>
> Cc: Joonas Saarinen <jza at saunalahti.fi>
> Tested-by: Vito Caputo <vcaputo at pengaru.com> # Thinkapd X61s
> Tested-by: kitsunyan <kitsunyan at airmail.cc> # ThinkPad X200
> Tested-by: Joonas Saarinen <jza at 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 at 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 at 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 at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
More information about the Intel-gfx
mailing list