[PATCH 1/3] drm/i915: Nuke the LVDS lid notifier

[PATCH 1/3] drm/i915: Nuke the LVDS lid notifier

[Ville Syrjala]

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
@@ -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

[PATCH 1/3] drm/i915: Nuke the LVDS lid notifier

[Ville Syrjala]

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
@@ -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

[PATCH 2/3] drm/i915: Assume eDP is always connected

[Ville Syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

We never registered any kind of lid notifier for eDP, so looking at the
lid status is pretty much bonkers. Let's just consider eDP always
connected instead.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_dp.c | 11 ++---------
 1 file changed, 2 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index b45b08420c1f..61657b8b109c 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -4532,14 +4532,7 @@ intel_dp_detect_dpcd(struct intel_dp *intel_dp)
 static enum drm_connector_status
 edp_detect(struct intel_dp *intel_dp)
 {
-	struct drm_i915_private *dev_priv = to_i915(intel_dp_to_dev(intel_dp));
-	enum drm_connector_status status;
-
-	status = intel_panel_detect(dev_priv);
-	if (status == connector_status_unknown)
-		status = connector_status_connected;
-
-	return status;
+	return connector_status_connected;
 }
 
 static bool ibx_digital_port_connected(struct intel_encoder *encoder)
@@ -4802,7 +4795,7 @@ intel_dp_long_pulse(struct intel_connector *connector)
 
 	intel_display_power_get(dev_priv, intel_dp->aux_power_domain);
 
-	/* Can't disconnect eDP, but you can close the lid... */
+	/* Can't disconnect eDP */
 	if (intel_dp_is_edp(intel_dp))
 		status = edp_detect(intel_dp);
 	else if (intel_digital_port_connected(&dp_to_dig_port(intel_dp)->base))
-- 
2.16.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 3/3] drm/i915: Remove intel_panel_detect()

[Ville Syrjala]

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

With neither LVDS or eDP no longer using intel_panel_detect() we can
kill it, and the accompanying modparam.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_params.c |  4 ----
 drivers/gpu/drm/i915/i915_params.h |  1 -
 drivers/gpu/drm/i915/intel_drv.h   |  1 -
 drivers/gpu/drm/i915/intel_panel.c | 20 --------------------
 4 files changed, 26 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_params.c b/drivers/gpu/drm/i915/i915_params.c
index 817576701ed7..295e981e4a39 100644
--- a/drivers/gpu/drm/i915/i915_params.c
+++ b/drivers/gpu/drm/i915/i915_params.c
@@ -44,10 +44,6 @@ i915_param_named(modeset, int, 0400,
 	"Use kernel modesetting [KMS] (0=disable, "
 	"1=on, -1=force vga console preference [default])");
 
-i915_param_named_unsafe(panel_ignore_lid, int, 0600,
-	"Override lid status (0=autodetect, 1=autodetect disabled [default], "
-	"-1=force lid closed, -2=force lid open)");
-
 i915_param_named_unsafe(enable_dc, int, 0400,
 	"Enable power-saving display C-states. "
 	"(-1=auto [default]; 0=disable; 1=up to DC5; 2=up to DC6)");
diff --git a/drivers/gpu/drm/i915/i915_params.h b/drivers/gpu/drm/i915/i915_params.h
index aebe0469ddaa..6c4d4a21474b 100644
--- a/drivers/gpu/drm/i915/i915_params.h
+++ b/drivers/gpu/drm/i915/i915_params.h
@@ -36,7 +36,6 @@ struct drm_printer;
 #define I915_PARAMS_FOR_EACH(param) \
 	param(char *, vbt_firmware, NULL) \
 	param(int, modeset, -1) \
-	param(int, panel_ignore_lid, 1) \
 	param(int, lvds_channel_mode, 0) \
 	param(int, panel_use_ssc, -1) \
 	param(int, vbt_sdvo_panel_type, -1) \
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 794d7b05a8b0..94ec8fa89edf 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1892,7 +1892,6 @@ void intel_panel_enable_backlight(const struct intel_crtc_state *crtc_state,
 				  const struct drm_connector_state *conn_state);
 void intel_panel_disable_backlight(const struct drm_connector_state *old_conn_state);
 void intel_panel_destroy_backlight(struct drm_connector *connector);
-enum drm_connector_status intel_panel_detect(struct drm_i915_private *dev_priv);
 extern struct drm_display_mode *intel_find_panel_downclock(
 				struct drm_i915_private *dev_priv,
 				struct drm_display_mode *fixed_mode,
diff --git a/drivers/gpu/drm/i915/intel_panel.c b/drivers/gpu/drm/i915/intel_panel.c
index 14b827ec5427..4a9f139e7b73 100644
--- a/drivers/gpu/drm/i915/intel_panel.c
+++ b/drivers/gpu/drm/i915/intel_panel.c
@@ -375,26 +375,6 @@ void intel_gmch_panel_fitting(struct intel_crtc *intel_crtc,
 	pipe_config->gmch_pfit.lvds_border_bits = border;
 }
 
-enum drm_connector_status
-intel_panel_detect(struct drm_i915_private *dev_priv)
-{
-	/* Assume that the BIOS does not lie through the OpRegion... */
-	if (!i915_modparams.panel_ignore_lid && dev_priv->opregion.lid_state) {
-		return *dev_priv->opregion.lid_state & 0x1 ?
-			connector_status_connected :
-			connector_status_disconnected;
-	}
-
-	switch (i915_modparams.panel_ignore_lid) {
-	case -2:
-		return connector_status_connected;
-	case -1:
-		return connector_status_disconnected;
-	default:
-		return connector_status_unknown;
-	}
-}
-
 /**
  * scale - scale values from one range to another
  * @source_val: value in range [@source_min..@source_max]
-- 
2.16.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✗ Fi.CI.CHECKPATCH: warning for series starting with [1/3] drm/i915: Nuke the LVDS lid notifier

[Patchwork]

== Series Details ==

Series: series starting with [1/3] drm/i915: Nuke the LVDS lid notifier
URL   : https://patchwork.freedesktop.org/series/46732/
State : warning

== Summary ==

$ dim checkpatch origin/drm-tip
4b2a45e6cc96 drm/i915: Nuke the LVDS lid notifier
-:10: ERROR:GIT_COMMIT_ID: Please use git commit description style 'commit <12+ chars of sha1> ("<title line>")' - ie: 'commit e2c8b8701e2d ("drm/i915: Use atomic helpers for suspend, v2.")'
#10: 
e2c8b8701e2d ("drm/i915: Use atomic helpers for suspend, v2.") as

-:40: WARNING:COMMIT_LOG_LONG_LINE: Possible unwrapped commit description (prefer a maximum 75 chars per line)
#40: 
References: https://lists.freedesktop.org/archives/intel-gfx/2018-June/169315.html

total: 1 errors, 1 warnings, 0 checks, 205 lines checked
a809908f38fb drm/i915: Assume eDP is always connected
65af14159255 drm/i915: Remove intel_panel_detect()

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✗ Fi.CI.SPARSE: warning for series starting with [1/3] drm/i915: Nuke the LVDS lid notifier

[Patchwork]

== Series Details ==

Series: series starting with [1/3] drm/i915: Nuke the LVDS lid notifier
URL   : https://patchwork.freedesktop.org/series/46732/
State : warning

== Summary ==

$ dim sparse origin/drm-tip
Commit: drm/i915: Nuke the LVDS lid notifier
-drivers/gpu/drm/i915/selftests/../i915_drv.h:3653:16: warning: expression using sizeof(void)
+drivers/gpu/drm/i915/selftests/../i915_drv.h:3651:16: warning: expression using sizeof(void)

Commit: drm/i915: Assume eDP is always connected
Okay!

Commit: drm/i915: Remove intel_panel_detect()
Okay!

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✓ Fi.CI.BAT: success for series starting with [1/3] drm/i915: Nuke the LVDS lid notifier

[Patchwork]

== Series Details ==

Series: series starting with [1/3] drm/i915: Nuke the LVDS lid notifier
URL   : https://patchwork.freedesktop.org/series/46732/
State : success

== Summary ==

= CI Bug Log - changes from CI_DRM_4502 -> Patchwork_9696 =

== Summary - SUCCESS ==

  No regressions found.

  External URL: https://patchwork.freedesktop.org/api/1.0/series/46732/revisions/1/mbox/

== Possible new issues ==

  Here are the unknown changes that may have been introduced in Patchwork_9696:

  === IGT changes ===

    ==== Possible regressions ====

    igt@drv_selftest@live_workarounds:
      {fi-cfl-8109u}:     PASS -> DMESG-FAIL

    
== Known issues ==

  Here are the changes found in Patchwork_9696 that come from known issues:

  === IGT changes ===

    ==== Issues hit ====

    igt@drv_selftest@live_hangcheck:
      fi-skl-guc:         PASS -> DMESG-FAIL (fdo#107174)

    igt@kms_flip@basic-flip-vs-dpms:
      fi-skl-6700hq:      PASS -> DMESG-WARN (fdo#105998)

    igt@kms_frontbuffer_tracking@basic:
      fi-hsw-peppy:       PASS -> DMESG-FAIL (fdo#106103, fdo#102614)

    igt@kms_pipe_crc_basic@suspend-read-crc-pipe-c:
      fi-bxt-dsi:         PASS -> INCOMPLETE (fdo#103927)

    
    ==== Possible fixes ====

    igt@prime_vgem@basic-fence-flip:
      fi-ilk-650:         FAIL (fdo#104008) -> PASS

    
  {name}: This element is suppressed. This means it is ignored when computing
          the status of the difference (SUCCESS, WARNING, or FAILURE).

  fdo#102614 https://bugs.freedesktop.org/show_bug.cgi?id=102614
  fdo#103927 https://bugs.freedesktop.org/show_bug.cgi?id=103927
  fdo#104008 https://bugs.freedesktop.org/show_bug.cgi?id=104008
  fdo#105998 https://bugs.freedesktop.org/show_bug.cgi?id=105998
  fdo#106103 https://bugs.freedesktop.org/show_bug.cgi?id=106103
  fdo#107174 https://bugs.freedesktop.org/show_bug.cgi?id=107174


== Participating hosts (46 -> 41) ==

  Missing    (5): fi-ctg-p8600 fi-ilk-m540 fi-byt-squawks fi-bsw-cyan fi-hsw-4200u 


== Build changes ==

    * Linux: CI_DRM_4502 -> Patchwork_9696

  CI_DRM_4502: 1783eeea9b08ffb39b8c618c9427fd2aac65acf9 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4562: 8781fd89a63eabed9359d02b50583cca67ff3673 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_9696: 65af1415925527264eefbeb33fa9a25e06d35094 @ git://anongit.freedesktop.org/gfx-ci/linux


== Linux commits ==

65af14159255 drm/i915: Remove intel_panel_detect()
a809908f38fb drm/i915: Assume eDP is always connected
4b2a45e6cc96 drm/i915: Nuke the LVDS lid notifier

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_9696/issues.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [PATCH 1/3] drm/i915: Nuke the LVDS lid notifier

[Rodrigo Vivi]

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

Re: [Intel-gfx] [PATCH 1/3] drm/i915: Nuke the LVDS lid notifier

[Rodrigo Vivi]

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

Re: [PATCH 2/3] drm/i915: Assume eDP is always connected

[Rodrigo Vivi]

On Tue, Jul 17, 2018 at 08:42:15PM +0300, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> We never registered any kind of lid notifier for eDP, so looking at the
> lid status is pretty much bonkers. Let's just consider eDP always
> connected instead.

hm....
I've seen in the past, specially on rvps, so many times that
we detect edp or assume edp is there and it is not...
that generates a flood of aux timeout reads...

I never investigate that, but I wonder if this change here
couldn't make that case even more common... 

> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/intel_dp.c | 11 ++---------
>  1 file changed, 2 insertions(+), 9 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index b45b08420c1f..61657b8b109c 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -4532,14 +4532,7 @@ intel_dp_detect_dpcd(struct intel_dp *intel_dp)
>  static enum drm_connector_status
>  edp_detect(struct intel_dp *intel_dp)
>  {
> -	struct drm_i915_private *dev_priv = to_i915(intel_dp_to_dev(intel_dp));
> -	enum drm_connector_status status;
> -
> -	status = intel_panel_detect(dev_priv);
> -	if (status == connector_status_unknown)
> -		status = connector_status_connected;
> -
> -	return status;
> +	return connector_status_connected;
>  }
>  
>  static bool ibx_digital_port_connected(struct intel_encoder *encoder)
> @@ -4802,7 +4795,7 @@ intel_dp_long_pulse(struct intel_connector *connector)
>  
>  	intel_display_power_get(dev_priv, intel_dp->aux_power_domain);
>  
> -	/* Can't disconnect eDP, but you can close the lid... */
> +	/* Can't disconnect eDP */
>  	if (intel_dp_is_edp(intel_dp))
>  		status = edp_detect(intel_dp);
>  	else if (intel_digital_port_connected(&dp_to_dig_port(intel_dp)->base))
> -- 
> 2.16.4
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 3/3] drm/i915: Remove intel_panel_detect()

[Rodrigo Vivi]

On Tue, Jul 17, 2018 at 08:42:16PM +0300, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> With neither LVDS or eDP no longer using intel_panel_detect() we can
> kill it, and the accompanying modparam.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

supposing that patch 2/3 goes through,

Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>



> ---
>  drivers/gpu/drm/i915/i915_params.c |  4 ----
>  drivers/gpu/drm/i915/i915_params.h |  1 -
>  drivers/gpu/drm/i915/intel_drv.h   |  1 -
>  drivers/gpu/drm/i915/intel_panel.c | 20 --------------------
>  4 files changed, 26 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_params.c b/drivers/gpu/drm/i915/i915_params.c
> index 817576701ed7..295e981e4a39 100644
> --- a/drivers/gpu/drm/i915/i915_params.c
> +++ b/drivers/gpu/drm/i915/i915_params.c
> @@ -44,10 +44,6 @@ i915_param_named(modeset, int, 0400,
>  	"Use kernel modesetting [KMS] (0=disable, "
>  	"1=on, -1=force vga console preference [default])");
>  
> -i915_param_named_unsafe(panel_ignore_lid, int, 0600,
> -	"Override lid status (0=autodetect, 1=autodetect disabled [default], "
> -	"-1=force lid closed, -2=force lid open)");
> -
>  i915_param_named_unsafe(enable_dc, int, 0400,
>  	"Enable power-saving display C-states. "
>  	"(-1=auto [default]; 0=disable; 1=up to DC5; 2=up to DC6)");
> diff --git a/drivers/gpu/drm/i915/i915_params.h b/drivers/gpu/drm/i915/i915_params.h
> index aebe0469ddaa..6c4d4a21474b 100644
> --- a/drivers/gpu/drm/i915/i915_params.h
> +++ b/drivers/gpu/drm/i915/i915_params.h
> @@ -36,7 +36,6 @@ struct drm_printer;
>  #define I915_PARAMS_FOR_EACH(param) \
>  	param(char *, vbt_firmware, NULL) \
>  	param(int, modeset, -1) \
> -	param(int, panel_ignore_lid, 1) \
>  	param(int, lvds_channel_mode, 0) \
>  	param(int, panel_use_ssc, -1) \
>  	param(int, vbt_sdvo_panel_type, -1) \
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 794d7b05a8b0..94ec8fa89edf 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -1892,7 +1892,6 @@ void intel_panel_enable_backlight(const struct intel_crtc_state *crtc_state,
>  				  const struct drm_connector_state *conn_state);
>  void intel_panel_disable_backlight(const struct drm_connector_state *old_conn_state);
>  void intel_panel_destroy_backlight(struct drm_connector *connector);
> -enum drm_connector_status intel_panel_detect(struct drm_i915_private *dev_priv);
>  extern struct drm_display_mode *intel_find_panel_downclock(
>  				struct drm_i915_private *dev_priv,
>  				struct drm_display_mode *fixed_mode,
> diff --git a/drivers/gpu/drm/i915/intel_panel.c b/drivers/gpu/drm/i915/intel_panel.c
> index 14b827ec5427..4a9f139e7b73 100644
> --- a/drivers/gpu/drm/i915/intel_panel.c
> +++ b/drivers/gpu/drm/i915/intel_panel.c
> @@ -375,26 +375,6 @@ void intel_gmch_panel_fitting(struct intel_crtc *intel_crtc,
>  	pipe_config->gmch_pfit.lvds_border_bits = border;
>  }
>  
> -enum drm_connector_status
> -intel_panel_detect(struct drm_i915_private *dev_priv)
> -{
> -	/* Assume that the BIOS does not lie through the OpRegion... */
> -	if (!i915_modparams.panel_ignore_lid && dev_priv->opregion.lid_state) {
> -		return *dev_priv->opregion.lid_state & 0x1 ?
> -			connector_status_connected :
> -			connector_status_disconnected;
> -	}
> -
> -	switch (i915_modparams.panel_ignore_lid) {
> -	case -2:
> -		return connector_status_connected;
> -	case -1:
> -		return connector_status_disconnected;
> -	default:
> -		return connector_status_unknown;
> -	}
> -}
> -
>  /**
>   * scale - scale values from one range to another
>   * @source_val: value in range [@source_min..@source_max]
> -- 
> 2.16.4
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✓ Fi.CI.IGT: success for series starting with [1/3] drm/i915: Nuke the LVDS lid notifier

[Patchwork]

== Series Details ==

Series: series starting with [1/3] drm/i915: Nuke the LVDS lid notifier
URL   : https://patchwork.freedesktop.org/series/46732/
State : success

== Summary ==

= CI Bug Log - changes from CI_DRM_4502_full -> Patchwork_9696_full =

== Summary - WARNING ==

  Minor unknown changes coming with Patchwork_9696_full need to be verified
  manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in Patchwork_9696_full, please notify your bug team to allow them
  to document this new failure mode, which will reduce false positives in CI.

  

== Possible new issues ==

  Here are the unknown changes that may have been introduced in Patchwork_9696_full:

  === IGT changes ===

    ==== Warnings ====

    igt@gem_exec_schedule@deep-vebox:
      shard-kbl:          PASS -> SKIP +1

    igt@gem_mocs_settings@mocs-rc6-vebox:
      shard-kbl:          SKIP -> PASS

    igt@kms_frontbuffer_tracking@fbc-2p-scndscrn-indfb-pgflip-blt:
      shard-hsw:          SKIP -> PASS

    
== Known issues ==

  Here are the changes found in Patchwork_9696_full that come from known issues:

  === IGT changes ===

    ==== Issues hit ====

    igt@gem_eio@in-flight-10ms:
      shard-glk:          PASS -> FAIL (fdo#105957)

    
    ==== Possible fixes ====

    igt@drv_suspend@shrink:
      shard-snb:          FAIL (fdo#106886) -> PASS

    igt@kms_flip@2x-plain-flip-ts-check:
      shard-glk:          FAIL (fdo#100368) -> PASS

    igt@kms_frontbuffer_tracking@fbc-suspend:
      shard-kbl:          INCOMPLETE (fdo#103665, fdo#105959) -> PASS

    
  fdo#100368 https://bugs.freedesktop.org/show_bug.cgi?id=100368
  fdo#103665 https://bugs.freedesktop.org/show_bug.cgi?id=103665
  fdo#105957 https://bugs.freedesktop.org/show_bug.cgi?id=105957
  fdo#105959 https://bugs.freedesktop.org/show_bug.cgi?id=105959
  fdo#106886 https://bugs.freedesktop.org/show_bug.cgi?id=106886


== Participating hosts (5 -> 5) ==

  No changes in participating hosts


== Build changes ==

    * Linux: CI_DRM_4502 -> Patchwork_9696

  CI_DRM_4502: 1783eeea9b08ffb39b8c618c9427fd2aac65acf9 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGT_4562: 8781fd89a63eabed9359d02b50583cca67ff3673 @ git://anongit.freedesktop.org/xorg/app/intel-gpu-tools
  Patchwork_9696: 65af1415925527264eefbeb33fa9a25e06d35094 @ git://anongit.freedesktop.org/gfx-ci/linux
  piglit_4509: fdc5a4ca11124ab8413c7988896eec4c97336694 @ git://anongit.freedesktop.org/piglit

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_9696/shards.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 2/3] drm/i915: Assume eDP is always connected

[Ville Syrjälä]

On Tue, Jul 17, 2018 at 01:00:25PM -0700, Rodrigo Vivi wrote:
> On Tue, Jul 17, 2018 at 08:42:15PM +0300, Ville Syrjala wrote:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > 
> > We never registered any kind of lid notifier for eDP, so looking at the
> > lid status is pretty much bonkers. Let's just consider eDP always
> > connected instead.
> 
> hm....
> I've seen in the past, specially on rvps, so many times that
> we detect edp or assume edp is there and it is not...
> that generates a flood of aux timeout reads...
> 
> I never investigate that, but I wonder if this change here
> couldn't make that case even more common... 

I don't know if RVPs would have any of the lid stuff implemented.
I somewhat doubt it.

If dpcd reads fail we won't register edp so any timeouts and whatnot
should only be logged during driver load.

> 
> > 
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> >  drivers/gpu/drm/i915/intel_dp.c | 11 ++---------
> >  1 file changed, 2 insertions(+), 9 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> > index b45b08420c1f..61657b8b109c 100644
> > --- a/drivers/gpu/drm/i915/intel_dp.c
> > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > @@ -4532,14 +4532,7 @@ intel_dp_detect_dpcd(struct intel_dp *intel_dp)
> >  static enum drm_connector_status
> >  edp_detect(struct intel_dp *intel_dp)
> >  {
> > -	struct drm_i915_private *dev_priv = to_i915(intel_dp_to_dev(intel_dp));
> > -	enum drm_connector_status status;
> > -
> > -	status = intel_panel_detect(dev_priv);
> > -	if (status == connector_status_unknown)
> > -		status = connector_status_connected;
> > -
> > -	return status;
> > +	return connector_status_connected;
> >  }
> >  
> >  static bool ibx_digital_port_connected(struct intel_encoder *encoder)
> > @@ -4802,7 +4795,7 @@ intel_dp_long_pulse(struct intel_connector *connector)
> >  
> >  	intel_display_power_get(dev_priv, intel_dp->aux_power_domain);
> >  
> > -	/* Can't disconnect eDP, but you can close the lid... */
> > +	/* Can't disconnect eDP */
> >  	if (intel_dp_is_edp(intel_dp))
> >  		status = edp_detect(intel_dp);
> >  	else if (intel_digital_port_connected(&dp_to_dig_port(intel_dp)->base))
> > -- 
> > 2.16.4
> > 
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 2/3] drm/i915: Assume eDP is always connected

[Rodrigo Vivi]

On Wed, Jul 18, 2018 at 01:03:59PM +0300, Ville Syrjälä wrote:
> On Tue, Jul 17, 2018 at 01:00:25PM -0700, Rodrigo Vivi wrote:
> > On Tue, Jul 17, 2018 at 08:42:15PM +0300, Ville Syrjala wrote:
> > > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > 
> > > We never registered any kind of lid notifier for eDP, so looking at the
> > > lid status is pretty much bonkers. Let's just consider eDP always
> > > connected instead.
> > 
> > hm....
> > I've seen in the past, specially on rvps, so many times that
> > we detect edp or assume edp is there and it is not...
> > that generates a flood of aux timeout reads...
> > 
> > I never investigate that, but I wonder if this change here
> > couldn't make that case even more common... 
> 
> I don't know if RVPs would have any of the lid stuff implemented.
> I somewhat doubt it.

I think they do... and with some hard toggles buttons,

but anyways I believe we might safe,
otherwise our CI would have complained about it right?

> 
> If dpcd reads fail we won't register edp so any timeouts and whatnot
> should only be logged during driver load.

yeap, but I'm afraid that without any kind of check we would
start seeing those aux failures everywhere... even on regular desktop
units where we naver saw that kind of failure would start to having logs
poluted.

But if CI is happy and you checked that this is not poluting the logs
of platforms without eDP then the patch looks correct:

Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>

> 
> > 
> > > 
> > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > ---
> > >  drivers/gpu/drm/i915/intel_dp.c | 11 ++---------
> > >  1 file changed, 2 insertions(+), 9 deletions(-)
> > > 
> > > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> > > index b45b08420c1f..61657b8b109c 100644
> > > --- a/drivers/gpu/drm/i915/intel_dp.c
> > > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > > @@ -4532,14 +4532,7 @@ intel_dp_detect_dpcd(struct intel_dp *intel_dp)
> > >  static enum drm_connector_status
> > >  edp_detect(struct intel_dp *intel_dp)
> > >  {
> > > -	struct drm_i915_private *dev_priv = to_i915(intel_dp_to_dev(intel_dp));
> > > -	enum drm_connector_status status;
> > > -
> > > -	status = intel_panel_detect(dev_priv);
> > > -	if (status == connector_status_unknown)
> > > -		status = connector_status_connected;
> > > -
> > > -	return status;
> > > +	return connector_status_connected;
> > >  }
> > >  
> > >  static bool ibx_digital_port_connected(struct intel_encoder *encoder)
> > > @@ -4802,7 +4795,7 @@ intel_dp_long_pulse(struct intel_connector *connector)
> > >  
> > >  	intel_display_power_get(dev_priv, intel_dp->aux_power_domain);
> > >  
> > > -	/* Can't disconnect eDP, but you can close the lid... */
> > > +	/* Can't disconnect eDP */
> > >  	if (intel_dp_is_edp(intel_dp))
> > >  		status = edp_detect(intel_dp);
> > >  	else if (intel_digital_port_connected(&dp_to_dig_port(intel_dp)->base))
> > > -- 
> > > 2.16.4
> > > 
> > > _______________________________________________
> > > Intel-gfx mailing list
> > > Intel-gfx@lists.freedesktop.org
> > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> -- 
> Ville Syrjälä
> Intel
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 2/3] drm/i915: Assume eDP is always connected

[Ville Syrjälä]

On Wed, Jul 18, 2018 at 02:00:42PM -0700, Rodrigo Vivi wrote:
> On Wed, Jul 18, 2018 at 01:03:59PM +0300, Ville Syrjälä wrote:
> > On Tue, Jul 17, 2018 at 01:00:25PM -0700, Rodrigo Vivi wrote:
> > > On Tue, Jul 17, 2018 at 08:42:15PM +0300, Ville Syrjala wrote:
> > > > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > > 
> > > > We never registered any kind of lid notifier for eDP, so looking at the
> > > > lid status is pretty much bonkers. Let's just consider eDP always
> > > > connected instead.
> > > 
> > > hm....
> > > I've seen in the past, specially on rvps, so many times that
> > > we detect edp or assume edp is there and it is not...
> > > that generates a flood of aux timeout reads...
> > > 
> > > I never investigate that, but I wonder if this change here
> > > couldn't make that case even more common... 
> > 
> > I don't know if RVPs would have any of the lid stuff implemented.
> > I somewhat doubt it.
> 
> I think they do... and with some hard toggles buttons,

Oh right, some do. I think I have one SKL with those switches on the
front. Don't think I've seen those buttons on any Atom RVPs though.

> 
> but anyways I believe we might safe,
> otherwise our CI would have complained about it right?

Not sure. Hmm. Actually none of this should matter for any machine that
doesn't actually have eDP. We don't check this stuff when we initialize
the encoder/connector so we'd still get the same amount of DPCD fail
as before.

> 
> > 
> > If dpcd reads fail we won't register edp so any timeouts and whatnot
> > should only be logged during driver load.
> 
> yeap, but I'm afraid that without any kind of check we would
> start seeing those aux failures everywhere... even on regular desktop
> units where we naver saw that kind of failure would start to having logs
> poluted.
> 
> But if CI is happy and you checked that this is not poluting the logs
> of platforms without eDP then the patch looks correct:
> 
> Reviewed-by: Rodrigo Vivi <rodrigo.vivi@intel.com>

Thanks.

> 
> > 
> > > 
> > > > 
> > > > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > > > ---
> > > >  drivers/gpu/drm/i915/intel_dp.c | 11 ++---------
> > > >  1 file changed, 2 insertions(+), 9 deletions(-)
> > > > 
> > > > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> > > > index b45b08420c1f..61657b8b109c 100644
> > > > --- a/drivers/gpu/drm/i915/intel_dp.c
> > > > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > > > @@ -4532,14 +4532,7 @@ intel_dp_detect_dpcd(struct intel_dp *intel_dp)
> > > >  static enum drm_connector_status
> > > >  edp_detect(struct intel_dp *intel_dp)
> > > >  {
> > > > -	struct drm_i915_private *dev_priv = to_i915(intel_dp_to_dev(intel_dp));
> > > > -	enum drm_connector_status status;
> > > > -
> > > > -	status = intel_panel_detect(dev_priv);
> > > > -	if (status == connector_status_unknown)
> > > > -		status = connector_status_connected;
> > > > -
> > > > -	return status;
> > > > +	return connector_status_connected;
> > > >  }
> > > >  
> > > >  static bool ibx_digital_port_connected(struct intel_encoder *encoder)
> > > > @@ -4802,7 +4795,7 @@ intel_dp_long_pulse(struct intel_connector *connector)
> > > >  
> > > >  	intel_display_power_get(dev_priv, intel_dp->aux_power_domain);
> > > >  
> > > > -	/* Can't disconnect eDP, but you can close the lid... */
> > > > +	/* Can't disconnect eDP */
> > > >  	if (intel_dp_is_edp(intel_dp))
> > > >  		status = edp_detect(intel_dp);
> > > >  	else if (intel_digital_port_connected(&dp_to_dig_port(intel_dp)->base))
> > > > -- 
> > > > 2.16.4
> > > > 
> > > > _______________________________________________
> > > > Intel-gfx mailing list
> > > > Intel-gfx@lists.freedesktop.org
> > > > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> > 
> > -- 
> > Ville Syrjälä
> > Intel
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx