From: Hans de Goede <hdegoede@redhat.com>
To: "Daniel Vetter" <daniel.vetter@intel.com>,
"Jani Nikula" <jani.nikula@linux.intel.com>,
"Ville Syrjälä" <ville.syrjala@linux.intel.com>,
"Jarkko Nikula" <jarkko.nikula@linux.intel.com>,
"Wolfram Sang" <wsa@the-dreams.de>,
"Andy Shevchenko" <andriy.shevchenko@linux.intel.com>,
"Thomas Gleixner" <tglx@linutronix.de>,
"H . Peter Anvin" <hpa@zytor.com>
Cc: Takashi Iwai <tiwai@suse.de>,
"russianneuromancer @ ya . ru" <russianneuromancer@ya.ru>,
intel-gfx <intel-gfx@lists.freedesktop.org>,
Hans de Goede <hdegoede@redhat.com>,
linux-i2c@vger.kernel.org,
Mika Westerberg <mika.westerberg@linux.intel.com>
Subject: [PATCH v4 12/12] drm/i915: Listen for PMIC bus access notifications
Date: Fri, 24 Feb 2017 10:29:12 +0100 [thread overview]
Message-ID: <20170224092912.10685-13-hdegoede@redhat.com> (raw)
In-Reply-To: <20170224092912.10685-1-hdegoede@redhat.com>
Listen for PMIC bus access notifications and get FORCEWAKE_ALL while
the bus is accessed to avoid needing to do any forcewakes, which need
PMIC bus access, while the PMIC bus is busy:
This fixes errors like these showing up in dmesg, usually followed
by a gfx or system freeze:
[drm:fw_domains_get [i915]] *ERROR* render: timed out waiting for forcewake ack request.
[drm:fw_domains_get [i915]] *MEDIA* render: timed out waiting for forcewake ack request.
i2c_designware 808622C1:06: punit semaphore timed out, resetting
i2c_designware 808622C1:06: PUNIT SEM: 2
i2c_designware 808622C1:06: couldn't acquire bus ownership
Downside of this approach is that it causes wakeups whenever the PMIC
bus is accessed. Unfortunately we cannot simply wait for the PMIC bus
to go idle when we hit a race, as forcewakes may be done from interrupt
handlers where we cannot sleep to wait for the i2c PMIC bus access to
finish.
Note that the notifications and thus the wakeups will only happen on
baytrail / cherrytrail devices using PMICs with a shared i2c bus for
P-Unit and host PMIC access (i2c busses with a _SEM method in their
APCI node), e.g. an axp288 PMIC.
I plan to write some patches for drivers accessing the PMIC bus to
limit their bus accesses to a bare minimum (e.g. cache registers, do not
update battery level more often then 4 times a minute), to limit the
amount of wakeups.
BugLink: https://bugzilla.kernel.org/show_bug.cgi?id=155241
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
Tested-by: tagorereddy <tagore.chandan@gmail.com>
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Acked-by: Jani Nikula <jani.nikula@intel.com>
---
Changes in v2:
-Spelling: P-Unit, PMIC
Changes in v3:
-Improve comment explaining why we call intel_uncore_forcewake_get(ALL) on
MBI_PMIC_BUS_ACCESS_BEGIN notification
---
drivers/gpu/drm/i915/Kconfig | 1 +
drivers/gpu/drm/i915/i915_drv.h | 1 +
drivers/gpu/drm/i915/intel_uncore.c | 39 +++++++++++++++++++++++++++++++++++++
3 files changed, 41 insertions(+)
diff --git a/drivers/gpu/drm/i915/Kconfig b/drivers/gpu/drm/i915/Kconfig
index 1ae0bb9..a5cd5da 100644
--- a/drivers/gpu/drm/i915/Kconfig
+++ b/drivers/gpu/drm/i915/Kconfig
@@ -20,6 +20,7 @@ config DRM_I915
select ACPI_VIDEO if ACPI
select ACPI_BUTTON if ACPI
select SYNC_FILE
+ select IOSF_MBI
help
Choose this option if you have a system that has "Intel Graphics
Media Accelerator" or "HD Graphics" integrated graphics,
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index ca699de..e4af9b7 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -749,6 +749,7 @@ struct intel_uncore {
const struct intel_forcewake_range *fw_domains_table;
unsigned int fw_domains_table_entries;
+ struct notifier_block pmic_bus_access_nb;
struct intel_uncore_funcs funcs;
unsigned fifo_count;
diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c
index 699a107..7d965e2 100644
--- a/drivers/gpu/drm/i915/intel_uncore.c
+++ b/drivers/gpu/drm/i915/intel_uncore.c
@@ -25,6 +25,7 @@
#include "intel_drv.h"
#include "i915_vgpu.h"
+#include <asm/iosf_mbi.h>
#include <linux/pm_runtime.h>
#define FORCEWAKE_ACK_TIMEOUT_MS 50
@@ -436,12 +437,16 @@ static void __intel_uncore_early_sanitize(struct drm_i915_private *dev_priv,
void intel_uncore_suspend(struct drm_i915_private *dev_priv)
{
+ iosf_mbi_unregister_pmic_bus_access_notifier(
+ &dev_priv->uncore.pmic_bus_access_nb);
intel_uncore_forcewake_reset(dev_priv, false);
}
void intel_uncore_resume_early(struct drm_i915_private *dev_priv)
{
__intel_uncore_early_sanitize(dev_priv, true);
+ iosf_mbi_register_pmic_bus_access_notifier(
+ &dev_priv->uncore.pmic_bus_access_nb);
i915_check_and_clear_faults(dev_priv);
}
@@ -1285,6 +1290,32 @@ static void intel_uncore_fw_domains_init(struct drm_i915_private *dev_priv)
dev_priv->uncore.fw_domains_table_entries = ARRAY_SIZE((d)); \
}
+static int i915_pmic_bus_access_notifier(struct notifier_block *nb,
+ unsigned long action, void *data)
+{
+ struct drm_i915_private *dev_priv = container_of(nb,
+ struct drm_i915_private, uncore.pmic_bus_access_nb);
+
+ switch (action) {
+ case MBI_PMIC_BUS_ACCESS_BEGIN:
+ /*
+ * forcewake all now to make sure that we don't need to do a
+ * forcewake later which on systems where this notifier gets
+ * called requires the punit to access to the shared pmic i2c
+ * bus, which will be busy after this notification, leading to:
+ * "render: timed out waiting for forcewake ack request."
+ * errors.
+ */
+ intel_uncore_forcewake_get(dev_priv, FORCEWAKE_ALL);
+ break;
+ case MBI_PMIC_BUS_ACCESS_END:
+ intel_uncore_forcewake_put(dev_priv, FORCEWAKE_ALL);
+ break;
+ }
+
+ return NOTIFY_OK;
+}
+
void intel_uncore_init(struct drm_i915_private *dev_priv)
{
i915_check_vgpu(dev_priv);
@@ -1294,6 +1325,8 @@ void intel_uncore_init(struct drm_i915_private *dev_priv)
__intel_uncore_early_sanitize(dev_priv, false);
dev_priv->uncore.unclaimed_mmio_check = 1;
+ dev_priv->uncore.pmic_bus_access_nb.notifier_call =
+ i915_pmic_bus_access_notifier;
switch (INTEL_INFO(dev_priv)->gen) {
default:
@@ -1344,6 +1377,9 @@ void intel_uncore_init(struct drm_i915_private *dev_priv)
break;
}
+ iosf_mbi_register_pmic_bus_access_notifier(
+ &dev_priv->uncore.pmic_bus_access_nb);
+
i915_check_and_clear_faults(dev_priv);
}
#undef ASSIGN_WRITE_MMIO_VFUNCS
@@ -1351,6 +1387,9 @@ void intel_uncore_init(struct drm_i915_private *dev_priv)
void intel_uncore_fini(struct drm_i915_private *dev_priv)
{
+ iosf_mbi_unregister_pmic_bus_access_notifier(
+ &dev_priv->uncore.pmic_bus_access_nb);
+
/* Paranoia: make sure we have disabled everything before we exit. */
intel_uncore_sanitize(dev_priv);
intel_uncore_forcewake_reset(dev_priv, false);
--
2.9.3
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
prev parent reply other threads:[~2017-02-24 9:29 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-02-24 9:29 [PATCH v4 00/12] coordinate cht i2c-pmic and i915-punit accesses Hans de Goede
2017-02-24 9:29 ` [PATCH v4 01/12] x86/platform/intel/iosf_mbi: Add a mutex for P-Unit access Hans de Goede
2017-02-24 9:29 ` [PATCH v4 02/12] x86/platform/intel/iosf_mbi: Add a PMIC bus access notifier Hans de Goede
2017-02-24 9:29 ` [PATCH v4 03/12] i2c: designware: Rename accessor_flags to flags Hans de Goede
2017-02-24 9:29 ` [PATCH v4 04/12] i2c: designware-baytrail: Pass dw_i2c_dev into helper functions Hans de Goede
2017-02-24 9:29 ` [PATCH v4 05/12] i2c: designware-baytrail: Only check iosf_mbi_available() for shared hosts Hans de Goede
2017-02-24 9:29 ` [PATCH v4 06/12] i2c: designware-baytrail: Disallow the CPU to enter C6 or C7 while holding the punit semaphore Hans de Goede
2017-02-24 9:29 ` [PATCH v4 07/12] i2c: designware-baytrail: Fix race when resetting the semaphore Hans de Goede
2017-02-24 9:29 ` [PATCH v4 08/12] i2c: designware-baytrail: Add support for cherrytrail Hans de Goede
2017-02-24 9:29 ` [PATCH v4 09/12] i2c: designware-baytrail: Acquire P-Unit access on bus acquire Hans de Goede
2017-02-24 9:29 ` [PATCH v4 10/12] i2c: designware-baytrail: Call pmic_bus_access_notifier_chain Hans de Goede
2017-02-24 9:29 ` [PATCH v4 11/12] drm/i915: Add intel_uncore_suspend / resume functions Hans de Goede
2017-02-24 9:29 ` Hans de Goede [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20170224092912.10685-13-hdegoede@redhat.com \
--to=hdegoede@redhat.com \
--cc=andriy.shevchenko@linux.intel.com \
--cc=daniel.vetter@intel.com \
--cc=hpa@zytor.com \
--cc=intel-gfx@lists.freedesktop.org \
--cc=jani.nikula@linux.intel.com \
--cc=jarkko.nikula@linux.intel.com \
--cc=linux-i2c@vger.kernel.org \
--cc=mika.westerberg@linux.intel.com \
--cc=russianneuromancer@ya.ru \
--cc=tglx@linutronix.de \
--cc=tiwai@suse.de \
--cc=ville.syrjala@linux.intel.com \
--cc=wsa@the-dreams.de \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).