* [RFC 0/6] Haswell runtime PM support + D3
@ 2013-10-22 19:30 Paulo Zanoni
2013-10-22 19:30 ` [RFC 1/6] drm/i915: add initial Runtime PM functions Paulo Zanoni
` (7 more replies)
0 siblings, 8 replies; 25+ messages in thread
From: Paulo Zanoni @ 2013-10-22 19:30 UTC (permalink / raw)
To: intel-gfx; +Cc: Paulo Zanoni
From: Paulo Zanoni <paulo.r.zanoni@intel.com>
Hi
This RFC series adds runtime PM support on Haswell. The current implementation
puts the device in the PCI D3cold state when we decide to sleep. It uses the
same refcount+timeout idea from the PC8 code, but now through the Kernel runtime
PM infrastructure.
I saw Jesse and Imre are still playing with the power wells on Baytrail, so I
thought maybe they would take some time to do the whole D3 + runtime_pm thing,
so I decided to ressurrect some code I had, test it and send anyway.
The basic idea of this series is that it adds some functions that should be
reused when we add support for other platforms, but OTOH it completely relies on
the already-implemented PC8 support for saving+restoring everything on Haswell.
So even though the code is ready for Haswell, it's not ready for !Haswell.
So we'll only reach the zero refcount after we enable PC8, and we'll increment
the refcount just before disabling PC8. I thought we would need more register
save-restore code, but it seems the current save-restore code from PC8 is enough
to keep everything going. All I needed was to add some more put/get calls to
keep the device awake when needed.
Now here's the reason why this is still an RFC: I just added put/get calls to
the places I spotted problems while doing my basic tests. I basically tested
runtime PM while running Gnome and Xfce, and I also tested sysfs and debugfs. I
still didn't write the full test suite I've been promising to write: this will
be my next step. OTOH, the runtime PM support is disabled by default, so merging
these patches shouldn't really hurt.
But our driver has so many entry points that I just don't think we'll ever be
confident that we always wake up from D3 when needed, and I don't think we'll be
able to write a single test case that tests everything. So I fear this feature
will be just like the "unclaimed register" checking code: we'll be stuck on a
break-bisect-fix loop forever. And considering most people won't ever run the
runtime PM code, we may stay long periods of time without noticing the broken
state.
When we add support for BYT and other platforms we will have to rearrange the
PC8 code so that we move some things out of it to some common PM infrastructure
(i.e, dev_priv->pm). The first things I can think of are the code that counts
the current number of used CRTCs and the code that handles pc8.gpu_idle.
The interfaces for configuring the runtime PM support are the standard runtime
PM interfaces. Go to /sys/bus/pci/devices/0000\:00\:02.0/power/ and have some
fun. I suggest you to "echo 5 > autosuspend_delay_ms" and boot with
"i915.pc8_timeout=5" if you really want to test things.
Another thing which we could discuss is if we really want to keep PC8 and D3 as
separate features. Today they're separate features, where D3 is a "deeper" step
that can be disabled while PC8 is still enabled. This creates the problem that
we need to separately test/validate PC8-only and PC8+D3 (and possibly D3-only in
the future, depending on how we rearrange code). Since I can't really think of a
case where we would want PC8 but not D3, maybe we should kill the "PC8 without
D3" option.
Opinions? Bikesheds? Does this really looks like it can be resued on BYT? Jesse?
Imre?
Thanks,
Paulo
Paulo Zanoni (6):
drm/i915: add initial Runtime PM functions
drm/i915: do adapter power state notification at runtime PM
drm/i915: add enable_runtime_pm option
drm/i915: add runtime put/get calls at the basic places
drm/i915: add some runtime PM get/put calls
drm/i915: add runtime PM support on Haswell
drivers/gpu/drm/i915/i915_debugfs.c | 46 ++++++++++++++++++++++++--
drivers/gpu/drm/i915/i915_dma.c | 38 +++++++++++++++++++++
drivers/gpu/drm/i915/i915_drv.c | 48 +++++++++++++++++++++++++++
drivers/gpu/drm/i915/i915_drv.h | 8 +++++
drivers/gpu/drm/i915/i915_gem.c | 53 +++++++++++++++++++-----------
drivers/gpu/drm/i915/i915_gem_execbuffer.c | 6 ++++
drivers/gpu/drm/i915/i915_irq.c | 6 ++++
drivers/gpu/drm/i915/i915_sysfs.c | 14 ++++++--
drivers/gpu/drm/i915/intel_display.c | 4 +++
drivers/gpu/drm/i915/intel_dp.c | 11 +++++--
drivers/gpu/drm/i915/intel_drv.h | 2 ++
drivers/gpu/drm/i915/intel_panel.c | 9 ++++-
drivers/gpu/drm/i915/intel_pm.c | 26 +++++++++++++++
drivers/gpu/drm/i915/intel_uncore.c | 13 ++++++++
14 files changed, 257 insertions(+), 27 deletions(-)
--
1.8.3.1
^ permalink raw reply [flat|nested] 25+ messages in thread
* [RFC 1/6] drm/i915: add initial Runtime PM functions
2013-10-22 19:30 [RFC 0/6] Haswell runtime PM support + D3 Paulo Zanoni
@ 2013-10-22 19:30 ` Paulo Zanoni
2013-10-28 13:21 ` Imre Deak
2013-10-22 19:30 ` [RFC 2/6] drm/i915: do adapter power state notification at runtime PM Paulo Zanoni
` (6 subsequent siblings)
7 siblings, 1 reply; 25+ messages in thread
From: Paulo Zanoni @ 2013-10-22 19:30 UTC (permalink / raw)
To: intel-gfx; +Cc: Paulo Zanoni
From: Paulo Zanoni <paulo.r.zanoni@intel.com>
This patch adds the initial infrastructure to allow a Runtime PM
implementation that sets the device to its D3 state. The patch just
adds the necessary callbacks and the initial infrastructure.
We still don't have any platform that actually uses this
infrastructure, we still don't call get/put in all the places we need
to, and we don't have any function to save/restore the state of the
registers. This is not a problem since no platform uses the code added
by this patch. We have a few people simultaneously working on runtime
PM, so this initial code could help everybody make their plans.
Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
drivers/gpu/drm/i915/i915_dma.c | 38 +++++++++++++++++++++++++++++++++
drivers/gpu/drm/i915/i915_drv.c | 42 +++++++++++++++++++++++++++++++++++++
drivers/gpu/drm/i915/i915_drv.h | 7 +++++++
drivers/gpu/drm/i915/intel_drv.h | 2 ++
drivers/gpu/drm/i915/intel_pm.c | 26 +++++++++++++++++++++++
drivers/gpu/drm/i915/intel_uncore.c | 9 ++++++++
6 files changed, 124 insertions(+)
diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
index fd848ef..6aa044e 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -42,6 +42,8 @@
#include <linux/vga_switcheroo.h>
#include <linux/slab.h>
#include <acpi/video.h>
+#include <linux/pm.h>
+#include <linux/pm_runtime.h>
#define LP_RING(d) (&((struct drm_i915_private *)(d))->ring[RCS])
@@ -1449,6 +1451,38 @@ static void i915_dump_device_info(struct drm_i915_private *dev_priv)
#undef SEP_COMMA
}
+static void i915_init_runtime_pm(struct drm_i915_private *dev_priv)
+{
+ struct drm_device *dev = dev_priv->dev;
+ struct device *device = &dev->pdev->dev;
+
+ dev_priv->pm.suspended = false;
+
+ if (!HAS_RUNTIME_PM(dev))
+ return;
+
+ pm_runtime_set_active(device);
+ pm_runtime_enable(device);
+
+ pm_runtime_set_autosuspend_delay(device, 10000); /* 10s */
+ pm_runtime_mark_last_busy(device);
+ pm_runtime_use_autosuspend(device);
+ pm_runtime_allow(device);
+}
+
+static void i915_fini_runtime_pm(struct drm_i915_private *dev_priv)
+{
+ struct drm_device *dev = dev_priv->dev;
+ struct device *device = &dev->pdev->dev;
+
+ if (!HAS_RUNTIME_PM(dev))
+ return;
+
+ /* Make sure we're not suspended first. */
+ pm_runtime_get_sync(device);
+ pm_runtime_disable(device);
+}
+
/**
* i915_driver_load - setup chip and create an initial config
* @dev: DRM device
@@ -1664,6 +1698,8 @@ int i915_driver_load(struct drm_device *dev, unsigned long flags)
if (IS_GEN5(dev))
intel_gpu_ips_init(dev_priv);
+ i915_init_runtime_pm(dev_priv);
+
return 0;
out_power_well:
@@ -1704,6 +1740,8 @@ int i915_driver_unload(struct drm_device *dev)
struct drm_i915_private *dev_priv = dev->dev_private;
int ret;
+ i915_fini_runtime_pm(dev_priv);
+
intel_gpu_ips_teardown();
if (HAS_POWER_WELL(dev)) {
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 85ae0dc..08fc7ea 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -469,6 +469,8 @@ static int i915_drm_freeze(struct drm_device *dev)
struct drm_i915_private *dev_priv = dev->dev_private;
struct drm_crtc *crtc;
+ intel_runtime_pm_get(dev_priv);
+
/* ignore lid events during suspend */
mutex_lock(&dev_priv->modeset_restore_lock);
dev_priv->modeset_restore = MODESET_SUSPENDED;
@@ -653,6 +655,8 @@ static int __i915_drm_thaw(struct drm_device *dev, bool restore_gtt_mappings)
mutex_lock(&dev_priv->modeset_restore_lock);
dev_priv->modeset_restore = MODESET_DONE;
mutex_unlock(&dev_priv->modeset_restore_lock);
+
+ intel_runtime_pm_put(dev_priv);
return error;
}
@@ -880,6 +884,42 @@ static int i915_pm_poweroff(struct device *dev)
return i915_drm_freeze(drm_dev);
}
+static int i915_runtime_suspend(struct device *device)
+{
+ struct pci_dev *pdev = to_pci_dev(device);
+ struct drm_device *dev = pci_get_drvdata(pdev);
+ struct drm_i915_private *dev_priv = dev->dev_private;
+
+ WARN_ON(!HAS_RUNTIME_PM(dev));
+
+ DRM_DEBUG_KMS("Suspending device\n");
+
+ dev_priv->pm.suspended = true;
+
+ pci_save_state(pdev);
+ pci_set_power_state(pdev, PCI_D3cold);
+
+ return 0;
+}
+
+static int i915_runtime_resume(struct device *device)
+{
+ struct pci_dev *pdev = to_pci_dev(device);
+ struct drm_device *dev = pci_get_drvdata(pdev);
+ struct drm_i915_private *dev_priv = dev->dev_private;
+
+ WARN_ON(!HAS_RUNTIME_PM(dev));
+
+ DRM_DEBUG_KMS("Resuming device\n");
+
+ pci_set_power_state(pdev, PCI_D0);
+ pci_restore_state(pdev);
+
+ dev_priv->pm.suspended = false;
+
+ return 0;
+}
+
static const struct dev_pm_ops i915_pm_ops = {
.suspend = i915_pm_suspend,
.resume = i915_pm_resume,
@@ -887,6 +927,8 @@ static const struct dev_pm_ops i915_pm_ops = {
.thaw = i915_pm_thaw,
.poweroff = i915_pm_poweroff,
.restore = i915_pm_resume,
+ .runtime_suspend = i915_runtime_suspend,
+ .runtime_resume = i915_runtime_resume,
};
static const struct vm_operations_struct i915_gem_vm_ops = {
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 80957ca..74f2b5d 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1230,6 +1230,10 @@ struct i915_package_c8 {
} regsave;
};
+struct i915_runtime_pm {
+ bool suspended;
+};
+
enum intel_pipe_crc_source {
INTEL_PIPE_CRC_SOURCE_NONE,
INTEL_PIPE_CRC_SOURCE_PLANE1,
@@ -1462,6 +1466,8 @@ typedef struct drm_i915_private {
struct i915_package_c8 pc8;
+ struct i915_runtime_pm pm;
+
/* Old dri1 support infrastructure, beware the dragons ya fools entering
* here! */
struct i915_dri1_state dri1;
@@ -1779,6 +1785,7 @@ struct drm_i915_file_private {
#define HAS_POWER_WELL(dev) (IS_HASWELL(dev))
#define HAS_FPGA_DBG_UNCLAIMED(dev) (INTEL_INFO(dev)->has_fpga_dbg)
#define HAS_PSR(dev) (IS_HASWELL(dev))
+#define HAS_RUNTIME_PM(dev) false
#define INTEL_PCH_DEVICE_ID_MASK 0xff00
#define INTEL_PCH_IBX_DEVICE_ID_TYPE 0x3b00
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index af1553c..80b5a80 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -840,6 +840,8 @@ void gen6_rps_idle(struct drm_i915_private *dev_priv);
void gen6_rps_boost(struct drm_i915_private *dev_priv);
void intel_aux_display_runtime_get(struct drm_i915_private *dev_priv);
void intel_aux_display_runtime_put(struct drm_i915_private *dev_priv);
+void intel_runtime_pm_get(struct drm_i915_private *dev_priv);
+void intel_runtime_pm_put(struct drm_i915_private *dev_priv);
void ilk_wm_get_hw_state(struct drm_device *dev);
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 3e140ab..a973f35 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -31,6 +31,7 @@
#include "../../../platform/x86/intel_ips.h"
#include <linux/module.h>
#include <drm/i915_powerwell.h>
+#include <linux/pm_runtime.h>
/**
* RC6 is a special power stage which allows the GPU to enter an very
@@ -5779,6 +5780,31 @@ void intel_aux_display_runtime_put(struct drm_i915_private *dev_priv)
hsw_enable_package_c8(dev_priv);
}
+void intel_runtime_pm_get(struct drm_i915_private *dev_priv)
+{
+ struct drm_device *dev = dev_priv->dev;
+ struct device *device = &dev->pdev->dev;
+
+ if (!HAS_RUNTIME_PM(dev))
+ return;
+
+ pm_runtime_mark_last_busy(device);
+ pm_runtime_get(device);
+ pm_runtime_barrier(device);
+}
+
+void intel_runtime_pm_put(struct drm_i915_private *dev_priv)
+{
+ struct drm_device *dev = dev_priv->dev;
+ struct device *device = &dev->pdev->dev;
+
+ if (!HAS_RUNTIME_PM(dev))
+ return;
+
+ pm_runtime_mark_last_busy(device);
+ pm_runtime_put_autosuspend(device);
+}
+
/* Set up chip specific power management-related functions */
void intel_init_pm(struct drm_device *dev)
{
diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c
index f6fae35..f5a2a6d 100644
--- a/drivers/gpu/drm/i915/intel_uncore.c
+++ b/drivers/gpu/drm/i915/intel_uncore.c
@@ -343,6 +343,13 @@ hsw_unclaimed_reg_check(struct drm_i915_private *dev_priv, u32 reg)
}
}
+static void
+assert_device_not_suspended(struct drm_i915_private *dev_priv)
+{
+ WARN(HAS_RUNTIME_PM(dev_priv->dev) && dev_priv->pm.suspended,
+ "Device suspended\n");
+}
+
#define REG_READ_HEADER(x) \
unsigned long irqflags; \
u##x val = 0; \
@@ -435,6 +442,7 @@ gen6_write##x(struct drm_i915_private *dev_priv, off_t reg, u##x val, bool trace
if (NEEDS_FORCE_WAKE((dev_priv), (reg))) { \
__fifo_ret = __gen6_gt_wait_for_fifo(dev_priv); \
} \
+ assert_device_not_suspended(dev_priv); \
__raw_i915_write##x(dev_priv, reg, val); \
if (unlikely(__fifo_ret)) { \
gen6_gt_check_fifodbg(dev_priv); \
@@ -450,6 +458,7 @@ hsw_write##x(struct drm_i915_private *dev_priv, off_t reg, u##x val, bool trace)
if (NEEDS_FORCE_WAKE((dev_priv), (reg))) { \
__fifo_ret = __gen6_gt_wait_for_fifo(dev_priv); \
} \
+ assert_device_not_suspended(dev_priv); \
hsw_unclaimed_reg_clear(dev_priv, reg); \
__raw_i915_write##x(dev_priv, reg, val); \
if (unlikely(__fifo_ret)) { \
--
1.8.3.1
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [RFC 2/6] drm/i915: do adapter power state notification at runtime PM
2013-10-22 19:30 [RFC 0/6] Haswell runtime PM support + D3 Paulo Zanoni
2013-10-22 19:30 ` [RFC 1/6] drm/i915: add initial Runtime PM functions Paulo Zanoni
@ 2013-10-22 19:30 ` Paulo Zanoni
2013-10-22 19:30 ` [RFC 3/6] drm/i915: add enable_runtime_pm option Paulo Zanoni
` (5 subsequent siblings)
7 siblings, 0 replies; 25+ messages in thread
From: Paulo Zanoni @ 2013-10-22 19:30 UTC (permalink / raw)
To: intel-gfx; +Cc: Jani Nikula, Paulo Zanoni
From: Paulo Zanoni <paulo.r.zanoni@intel.com>
Now that we are actually setting the device to the D3 state, we should
issue the notification.
Jani originally wrote a similar patch for PC8, but then we discovered
that we were not really changing the PCI D states when
enabling/disabling PC8, so we had to postpone his patch.
Cc: Jani Nikula <jani.nikula@intel.com>
Credits-to: Jani Nikula <jani.nikula@intel.com>
Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
drivers/gpu/drm/i915/i915_drv.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 08fc7ea..a999a3f 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -896,6 +896,7 @@ static int i915_runtime_suspend(struct device *device)
dev_priv->pm.suspended = true;
+ intel_opregion_notify_adapter(dev, PCI_D3cold);
pci_save_state(pdev);
pci_set_power_state(pdev, PCI_D3cold);
@@ -914,6 +915,7 @@ static int i915_runtime_resume(struct device *device)
pci_set_power_state(pdev, PCI_D0);
pci_restore_state(pdev);
+ intel_opregion_notify_adapter(dev, PCI_D0);
dev_priv->pm.suspended = false;
--
1.8.3.1
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [RFC 3/6] drm/i915: add enable_runtime_pm option
2013-10-22 19:30 [RFC 0/6] Haswell runtime PM support + D3 Paulo Zanoni
2013-10-22 19:30 ` [RFC 1/6] drm/i915: add initial Runtime PM functions Paulo Zanoni
2013-10-22 19:30 ` [RFC 2/6] drm/i915: do adapter power state notification at runtime PM Paulo Zanoni
@ 2013-10-22 19:30 ` Paulo Zanoni
2013-10-28 13:27 ` Imre Deak
2013-10-22 19:30 ` [RFC 4/6] drm/i915: add runtime put/get calls at the basic places Paulo Zanoni
` (4 subsequent siblings)
7 siblings, 1 reply; 25+ messages in thread
From: Paulo Zanoni @ 2013-10-22 19:30 UTC (permalink / raw)
To: intel-gfx; +Cc: Paulo Zanoni
From: Paulo Zanoni <paulo.r.zanoni@intel.com>
And leave it off by default. We have way too many driver entry points,
we can't assume this will work without regressions without tons of
testing first. This option allows people to test and fix the problems.
Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
drivers/gpu/drm/i915/i915_dma.c | 4 ++--
drivers/gpu/drm/i915/i915_drv.c | 8 ++++++--
drivers/gpu/drm/i915/i915_drv.h | 1 +
drivers/gpu/drm/i915/intel_pm.c | 4 ++--
4 files changed, 11 insertions(+), 6 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
index 6aa044e..dd4f424 100644
--- a/drivers/gpu/drm/i915/i915_dma.c
+++ b/drivers/gpu/drm/i915/i915_dma.c
@@ -1458,7 +1458,7 @@ static void i915_init_runtime_pm(struct drm_i915_private *dev_priv)
dev_priv->pm.suspended = false;
- if (!HAS_RUNTIME_PM(dev))
+ if (!HAS_RUNTIME_PM(dev) || !i915_enable_runtime_pm)
return;
pm_runtime_set_active(device);
@@ -1475,7 +1475,7 @@ static void i915_fini_runtime_pm(struct drm_i915_private *dev_priv)
struct drm_device *dev = dev_priv->dev;
struct device *device = &dev->pdev->dev;
- if (!HAS_RUNTIME_PM(dev))
+ if (!HAS_RUNTIME_PM(dev) || !i915_enable_runtime_pm)
return;
/* Make sure we're not suspended first. */
diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index a999a3f..c75b78f 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -154,6 +154,10 @@ module_param_named(prefault_disable, i915_prefault_disable, bool, 0600);
MODULE_PARM_DESC(prefault_disable,
"Disable page prefaulting for pread/pwrite/reloc (default:false). For developers only.");
+int i915_enable_runtime_pm __read_mostly = 0;
+module_param_named(enable_runtime_pm, i915_enable_runtime_pm, int, 0600);
+MODULE_PARM_DESC(enable_runtime_pm, "Enable runtime PM on supported platforms (default: disabled)");
+
static struct drm_driver driver;
extern int intel_agp_enabled;
@@ -890,7 +894,7 @@ static int i915_runtime_suspend(struct device *device)
struct drm_device *dev = pci_get_drvdata(pdev);
struct drm_i915_private *dev_priv = dev->dev_private;
- WARN_ON(!HAS_RUNTIME_PM(dev));
+ WARN_ON(!HAS_RUNTIME_PM(dev) || !i915_enable_runtime_pm);
DRM_DEBUG_KMS("Suspending device\n");
@@ -909,7 +913,7 @@ static int i915_runtime_resume(struct device *device)
struct drm_device *dev = pci_get_drvdata(pdev);
struct drm_i915_private *dev_priv = dev->dev_private;
- WARN_ON(!HAS_RUNTIME_PM(dev));
+ WARN_ON(!HAS_RUNTIME_PM(dev) || !i915_enable_runtime_pm);
DRM_DEBUG_KMS("Resuming device\n");
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 74f2b5d..73ebb9e 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1831,6 +1831,7 @@ extern bool i915_fastboot __read_mostly;
extern int i915_enable_pc8 __read_mostly;
extern int i915_pc8_timeout __read_mostly;
extern bool i915_prefault_disable __read_mostly;
+extern int i915_enable_runtime_pm __read_mostly;
extern int i915_suspend(struct drm_device *dev, pm_message_t state);
extern int i915_resume(struct drm_device *dev);
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index a973f35..617e934 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -5785,7 +5785,7 @@ void intel_runtime_pm_get(struct drm_i915_private *dev_priv)
struct drm_device *dev = dev_priv->dev;
struct device *device = &dev->pdev->dev;
- if (!HAS_RUNTIME_PM(dev))
+ if (!HAS_RUNTIME_PM(dev) || !i915_enable_runtime_pm)
return;
pm_runtime_mark_last_busy(device);
@@ -5798,7 +5798,7 @@ void intel_runtime_pm_put(struct drm_i915_private *dev_priv)
struct drm_device *dev = dev_priv->dev;
struct device *device = &dev->pdev->dev;
- if (!HAS_RUNTIME_PM(dev))
+ if (!HAS_RUNTIME_PM(dev) || !i915_enable_runtime_pm)
return;
pm_runtime_mark_last_busy(device);
--
1.8.3.1
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [RFC 4/6] drm/i915: add runtime put/get calls at the basic places
2013-10-22 19:30 [RFC 0/6] Haswell runtime PM support + D3 Paulo Zanoni
` (2 preceding siblings ...)
2013-10-22 19:30 ` [RFC 3/6] drm/i915: add enable_runtime_pm option Paulo Zanoni
@ 2013-10-22 19:30 ` Paulo Zanoni
2013-10-22 19:30 ` [RFC 5/6] drm/i915: add some runtime PM get/put calls Paulo Zanoni
` (3 subsequent siblings)
7 siblings, 0 replies; 25+ messages in thread
From: Paulo Zanoni @ 2013-10-22 19:30 UTC (permalink / raw)
To: intel-gfx; +Cc: Paulo Zanoni
From: Paulo Zanoni <paulo.r.zanoni@intel.com>
If I add code to enable runtime PM on my Haswell machine, start a
desktop environment, then enable runtime PM, these functions will
complain that they're trying to read/write registers while the
graphics card is suspended.
Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
drivers/gpu/drm/i915/i915_gem.c | 53 +++++++++++++++++++-----------
drivers/gpu/drm/i915/i915_gem_execbuffer.c | 6 ++++
drivers/gpu/drm/i915/i915_irq.c | 6 ++++
3 files changed, 46 insertions(+), 19 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
index e7b39d7..e7c2581 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -1377,36 +1377,38 @@ int i915_gem_fault(struct vm_area_struct *vma, struct vm_fault *vmf)
drm_i915_private_t *dev_priv = dev->dev_private;
pgoff_t page_offset;
unsigned long pfn;
- int ret = 0;
+ int rc = 0, ret;
bool write = !!(vmf->flags & FAULT_FLAG_WRITE);
+ intel_runtime_pm_get(dev_priv);
+
/* We don't use vmf->pgoff since that has the fake offset */
page_offset = ((unsigned long)vmf->virtual_address - vma->vm_start) >>
PAGE_SHIFT;
- ret = i915_mutex_lock_interruptible(dev);
- if (ret)
+ rc = i915_mutex_lock_interruptible(dev);
+ if (rc)
goto out;
trace_i915_gem_object_fault(obj, page_offset, true, write);
/* Access to snoopable pages through the GTT is incoherent. */
if (obj->cache_level != I915_CACHE_NONE && !HAS_LLC(dev)) {
- ret = -EINVAL;
+ rc = -EINVAL;
goto unlock;
}
/* Now bind it into the GTT if needed */
- ret = i915_gem_obj_ggtt_pin(obj, 0, true, false);
- if (ret)
+ rc = i915_gem_obj_ggtt_pin(obj, 0, true, false);
+ if (rc)
goto unlock;
- ret = i915_gem_object_set_to_gtt_domain(obj, write);
- if (ret)
+ rc = i915_gem_object_set_to_gtt_domain(obj, write);
+ if (rc)
goto unpin;
- ret = i915_gem_object_get_fence(obj);
- if (ret)
+ rc = i915_gem_object_get_fence(obj);
+ if (rc)
goto unpin;
obj->fault_mappable = true;
@@ -1416,19 +1418,21 @@ int i915_gem_fault(struct vm_area_struct *vma, struct vm_fault *vmf)
pfn += page_offset;
/* Finally, remap it using the new GTT offset */
- ret = vm_insert_pfn(vma, (unsigned long)vmf->virtual_address, pfn);
+ rc = vm_insert_pfn(vma, (unsigned long)vmf->virtual_address, pfn);
unpin:
i915_gem_object_unpin(obj);
unlock:
mutex_unlock(&dev->struct_mutex);
out:
- switch (ret) {
+ switch (rc) {
case -EIO:
/* If this -EIO is due to a gpu hang, give the reset code a
* chance to clean up the mess. Otherwise return the proper
* SIGBUS. */
- if (i915_terminally_wedged(&dev_priv->gpu_error))
- return VM_FAULT_SIGBUS;
+ if (i915_terminally_wedged(&dev_priv->gpu_error)) {
+ ret = VM_FAULT_SIGBUS;
+ break;
+ }
case -EAGAIN:
/*
* EAGAIN means the gpu is hung and we'll wait for the error
@@ -1443,15 +1447,22 @@ out:
* EBUSY is ok: this just means that another thread
* already did the job.
*/
- return VM_FAULT_NOPAGE;
+ ret = VM_FAULT_NOPAGE;
+ break;
case -ENOMEM:
- return VM_FAULT_OOM;
+ ret = VM_FAULT_OOM;
+ break;
case -ENOSPC:
- return VM_FAULT_SIGBUS;
+ ret = VM_FAULT_SIGBUS;
+ break;
default:
- WARN_ONCE(ret, "unhandled error in i915_gem_fault: %i\n", ret);
- return VM_FAULT_SIGBUS;
+ WARN_ONCE(rc, "unhandled error in i915_gem_fault: %i\n", rc);
+ ret = VM_FAULT_SIGBUS;
+ break;
}
+
+ intel_runtime_pm_put(dev_priv);
+ return ret;
}
/**
@@ -4164,6 +4175,8 @@ void i915_gem_free_object(struct drm_gem_object *gem_obj)
drm_i915_private_t *dev_priv = dev->dev_private;
struct i915_vma *vma, *next;
+ intel_runtime_pm_get(dev_priv);
+
trace_i915_gem_object_destroy(obj);
if (obj->phys_obj)
@@ -4208,6 +4221,8 @@ void i915_gem_free_object(struct drm_gem_object *gem_obj)
kfree(obj->bit_17);
i915_gem_object_free(obj);
+
+ intel_runtime_pm_put(dev_priv);
}
struct i915_vma *i915_gem_obj_to_vma(struct drm_i915_gem_object *obj,
diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
index 0ce0d47..0b2f200 100644
--- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c
+++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c
@@ -1062,6 +1062,8 @@ i915_gem_do_execbuffer(struct drm_device *dev, void *data,
}
}
+ intel_runtime_pm_get(dev_priv);
+
ret = i915_mutex_lock_interruptible(dev);
if (ret)
goto pre_mutex_err;
@@ -1197,6 +1199,10 @@ err:
pre_mutex_err:
kfree(cliprects);
+
+ /* intel_gpu_busy should also get a ref, so it will free when the device
+ * is really idle. */
+ intel_runtime_pm_put(dev_priv);
return ret;
}
diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 1a7dc77..1ded745 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -2242,6 +2242,12 @@ static void i915_hangcheck_elapsed(unsigned long data)
if (!i915_enable_hangcheck)
return;
+ /* Just postpone in case we're completely idle... */
+ if (dev_priv->pm.suspended) {
+ i915_queue_hangcheck(dev);
+ return;
+ }
+
for_each_ring(ring, dev_priv, i) {
u32 seqno, acthd;
bool busy = true;
--
1.8.3.1
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [RFC 5/6] drm/i915: add some runtime PM get/put calls
2013-10-22 19:30 [RFC 0/6] Haswell runtime PM support + D3 Paulo Zanoni
` (3 preceding siblings ...)
2013-10-22 19:30 ` [RFC 4/6] drm/i915: add runtime put/get calls at the basic places Paulo Zanoni
@ 2013-10-22 19:30 ` Paulo Zanoni
2013-10-22 19:30 ` [RFC 6/6] drm/i915: add runtime PM support on Haswell Paulo Zanoni
` (2 subsequent siblings)
7 siblings, 0 replies; 25+ messages in thread
From: Paulo Zanoni @ 2013-10-22 19:30 UTC (permalink / raw)
To: intel-gfx; +Cc: Paulo Zanoni
From: Paulo Zanoni <paulo.r.zanoni@intel.com>
These are needed when we cat the debugfs and sysfs files.
Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
drivers/gpu/drm/i915/i915_debugfs.c | 46 ++++++++++++++++++++++++++++++++++---
drivers/gpu/drm/i915/i915_sysfs.c | 14 +++++++++--
drivers/gpu/drm/i915/intel_dp.c | 11 +++++++--
drivers/gpu/drm/i915/intel_panel.c | 9 +++++++-
drivers/gpu/drm/i915/intel_uncore.c | 4 ++++
5 files changed, 76 insertions(+), 8 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index 5c45e9e..b83846f 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -564,10 +564,12 @@ static int i915_gem_seqno_info(struct seq_file *m, void *data)
ret = mutex_lock_interruptible(&dev->struct_mutex);
if (ret)
return ret;
+ intel_runtime_pm_get(dev_priv);
for_each_ring(ring, dev_priv, i)
i915_ring_seqno_info(m, ring);
+ intel_runtime_pm_put(dev_priv);
mutex_unlock(&dev->struct_mutex);
return 0;
@@ -585,6 +587,7 @@ static int i915_interrupt_info(struct seq_file *m, void *data)
ret = mutex_lock_interruptible(&dev->struct_mutex);
if (ret)
return ret;
+ intel_runtime_pm_get(dev_priv);
if (IS_VALLEYVIEW(dev)) {
seq_printf(m, "Display IER:\t%08x\n",
@@ -665,6 +668,7 @@ static int i915_interrupt_info(struct seq_file *m, void *data)
}
i915_ring_seqno_info(m, ring);
}
+ intel_runtime_pm_put(dev_priv);
mutex_unlock(&dev->struct_mutex);
return 0;
@@ -858,9 +862,11 @@ static int i915_rstdby_delays(struct seq_file *m, void *unused)
ret = mutex_lock_interruptible(&dev->struct_mutex);
if (ret)
return ret;
+ intel_runtime_pm_get(dev_priv);
crstanddelay = I915_READ16(CRSTANDVID);
+ intel_runtime_pm_put(dev_priv);
mutex_unlock(&dev->struct_mutex);
seq_printf(m, "w/ctx: %d, w/o ctx: %d\n", (crstanddelay >> 8) & 0x3f, (crstanddelay & 0x3f));
@@ -873,7 +879,9 @@ static int i915_cur_delayinfo(struct seq_file *m, void *unused)
struct drm_info_node *node = (struct drm_info_node *) m->private;
struct drm_device *dev = node->minor->dev;
drm_i915_private_t *dev_priv = dev->dev_private;
- int ret;
+ int ret = 0;
+
+ intel_runtime_pm_get(dev_priv);
flush_delayed_work(&dev_priv->rps.delayed_resume_work);
@@ -899,7 +907,7 @@ static int i915_cur_delayinfo(struct seq_file *m, void *unused)
/* RPSTAT1 is in the GT power well */
ret = mutex_lock_interruptible(&dev->struct_mutex);
if (ret)
- return ret;
+ goto out;
gen6_gt_force_wake_get(dev_priv);
@@ -988,7 +996,9 @@ static int i915_cur_delayinfo(struct seq_file *m, void *unused)
seq_puts(m, "no P-state info available\n");
}
- return 0;
+out:
+ intel_runtime_pm_put(dev_priv);
+ return ret;
}
static int i915_delayfreq_table(struct seq_file *m, void *unused)
@@ -1002,6 +1012,7 @@ static int i915_delayfreq_table(struct seq_file *m, void *unused)
ret = mutex_lock_interruptible(&dev->struct_mutex);
if (ret)
return ret;
+ intel_runtime_pm_get(dev_priv);
for (i = 0; i < 16; i++) {
delayfreq = I915_READ(PXVFREQ_BASE + i * 4);
@@ -1009,6 +1020,8 @@ static int i915_delayfreq_table(struct seq_file *m, void *unused)
(delayfreq & PXVFREQ_PX_MASK) >> PXVFREQ_PX_SHIFT);
}
+ intel_runtime_pm_put(dev_priv);
+
mutex_unlock(&dev->struct_mutex);
return 0;
@@ -1030,12 +1043,14 @@ static int i915_inttoext_table(struct seq_file *m, void *unused)
ret = mutex_lock_interruptible(&dev->struct_mutex);
if (ret)
return ret;
+ intel_runtime_pm_get(dev_priv);
for (i = 1; i <= 32; i++) {
inttoext = I915_READ(INTTOEXT_BASE_ILK + i * 4);
seq_printf(m, "INTTOEXT%02d: 0x%08x\n", i, inttoext);
}
+ intel_runtime_pm_put(dev_priv);
mutex_unlock(&dev->struct_mutex);
return 0;
@@ -1053,11 +1068,13 @@ static int ironlake_drpc_info(struct seq_file *m)
ret = mutex_lock_interruptible(&dev->struct_mutex);
if (ret)
return ret;
+ intel_runtime_pm_get(dev_priv);
rgvmodectl = I915_READ(MEMMODECTL);
rstdbyctl = I915_READ(RSTDBYCTL);
crstandvid = I915_READ16(CRSTANDVID);
+ intel_runtime_pm_put(dev_priv);
mutex_unlock(&dev->struct_mutex);
seq_printf(m, "HD boost: %s\n", (rgvmodectl & MEMMODE_BOOST_EN) ?
@@ -1121,6 +1138,7 @@ static int gen6_drpc_info(struct seq_file *m)
ret = mutex_lock_interruptible(&dev->struct_mutex);
if (ret)
return ret;
+ intel_runtime_pm_get(dev_priv);
spin_lock_irq(&dev_priv->uncore.lock);
forcewake_count = dev_priv->uncore.forcewake_count;
@@ -1146,6 +1164,8 @@ static int gen6_drpc_info(struct seq_file *m)
sandybridge_pcode_read(dev_priv, GEN6_PCODE_READ_RC6VIDS, &rc6vids);
mutex_unlock(&dev_priv->rps.hw_lock);
+ intel_runtime_pm_put(dev_priv);
+
seq_printf(m, "Video Turbo Mode: %s\n",
yesno(rpmodectl1 & GEN6_RP_MEDIA_TURBO));
seq_printf(m, "HW control enabled: %s\n",
@@ -1360,6 +1380,7 @@ static int i915_ring_freq_table(struct seq_file *m, void *unused)
ret = mutex_lock_interruptible(&dev_priv->rps.hw_lock);
if (ret)
return ret;
+ intel_runtime_pm_get(dev_priv);
seq_puts(m, "GPU freq (MHz)\tEffective CPU freq (MHz)\tEffective Ring freq (MHz)\n");
@@ -1376,6 +1397,7 @@ static int i915_ring_freq_table(struct seq_file *m, void *unused)
((ia_freq >> 8) & 0xff) * 100);
}
+ intel_runtime_pm_put(dev_priv);
mutex_unlock(&dev_priv->rps.hw_lock);
return 0;
@@ -1391,8 +1413,10 @@ static int i915_gfxec(struct seq_file *m, void *unused)
ret = mutex_lock_interruptible(&dev->struct_mutex);
if (ret)
return ret;
+ intel_runtime_pm_get(dev_priv);
seq_printf(m, "GFXEC: %ld\n", (unsigned long)I915_READ(0x112f4));
+ intel_runtime_pm_put(dev_priv);
mutex_unlock(&dev->struct_mutex);
@@ -1564,6 +1588,7 @@ static int i915_swizzle_info(struct seq_file *m, void *data)
ret = mutex_lock_interruptible(&dev->struct_mutex);
if (ret)
return ret;
+ intel_runtime_pm_get(dev_priv);
seq_printf(m, "bit6 swizzle for X-tiling = %s\n",
swizzle_string(dev_priv->mm.bit_6_swizzle_x));
@@ -1591,6 +1616,7 @@ static int i915_swizzle_info(struct seq_file *m, void *data)
seq_printf(m, "DISP_ARB_CTL = 0x%08x\n",
I915_READ(DISP_ARB_CTL));
}
+ intel_runtime_pm_put(dev_priv);
mutex_unlock(&dev->struct_mutex);
return 0;
@@ -1608,6 +1634,8 @@ static int i915_ppgtt_info(struct seq_file *m, void *data)
ret = mutex_lock_interruptible(&dev->struct_mutex);
if (ret)
return ret;
+ intel_runtime_pm_get(dev_priv);
+
if (INTEL_INFO(dev)->gen == 6)
seq_printf(m, "GFX_MODE: 0x%08x\n", I915_READ(GFX_MODE));
@@ -1626,6 +1654,8 @@ static int i915_ppgtt_info(struct seq_file *m, void *data)
seq_printf(m, "pd gtt offset: 0x%08x\n", ppgtt->pd_offset);
}
seq_printf(m, "ECOCHK: 0x%08x\n", I915_READ(GAM_ECOCHK));
+
+ intel_runtime_pm_put(dev_priv);
mutex_unlock(&dev->struct_mutex);
return 0;
@@ -1699,6 +1729,8 @@ static int i915_edp_psr_status(struct seq_file *m, void *data)
u32 psrperf = 0;
bool enabled = false;
+ intel_runtime_pm_get(dev_priv);
+
seq_printf(m, "Sink_Support: %s\n", yesno(dev_priv->psr.sink_support));
seq_printf(m, "Source_OK: %s\n", yesno(dev_priv->psr.source_ok));
@@ -1711,6 +1743,7 @@ static int i915_edp_psr_status(struct seq_file *m, void *data)
EDP_PSR_PERF_CNT_MASK;
seq_printf(m, "Performance_Counter: %u\n", psrperf);
+ intel_runtime_pm_put(dev_priv);
return 0;
}
@@ -2679,8 +2712,11 @@ i915_cache_sharing_get(void *data, u64 *val)
ret = mutex_lock_interruptible(&dev->struct_mutex);
if (ret)
return ret;
+ intel_runtime_pm_get(dev_priv);
snpcr = I915_READ(GEN6_MBCUNIT_SNPCR);
+
+ intel_runtime_pm_put(dev_priv);
mutex_unlock(&dev_priv->dev->struct_mutex);
*val = (snpcr & GEN6_MBC_SNPCR_MASK) >> GEN6_MBC_SNPCR_SHIFT;
@@ -2701,6 +2737,7 @@ i915_cache_sharing_set(void *data, u64 val)
if (val > 3)
return -EINVAL;
+ intel_runtime_pm_get(dev_priv);
DRM_DEBUG_DRIVER("Manually setting uncore sharing to %llu\n", val);
/* Update the cache sharing policy here as well */
@@ -2709,6 +2746,7 @@ i915_cache_sharing_set(void *data, u64 val)
snpcr |= (val << GEN6_MBC_SNPCR_SHIFT);
I915_WRITE(GEN6_MBCUNIT_SNPCR, snpcr);
+ intel_runtime_pm_put(dev_priv);
return 0;
}
@@ -2724,6 +2762,7 @@ static int i915_forcewake_open(struct inode *inode, struct file *file)
if (INTEL_INFO(dev)->gen < 6)
return 0;
+ intel_runtime_pm_get(dev_priv);
gen6_gt_force_wake_get(dev_priv);
return 0;
@@ -2738,6 +2777,7 @@ static int i915_forcewake_release(struct inode *inode, struct file *file)
return 0;
gen6_gt_force_wake_put(dev_priv);
+ intel_runtime_pm_put(dev_priv);
return 0;
}
diff --git a/drivers/gpu/drm/i915/i915_sysfs.c b/drivers/gpu/drm/i915/i915_sysfs.c
index bb31ff3..55290c1 100644
--- a/drivers/gpu/drm/i915/i915_sysfs.c
+++ b/drivers/gpu/drm/i915/i915_sysfs.c
@@ -40,10 +40,13 @@ static u32 calc_residency(struct drm_device *dev, const u32 reg)
struct drm_i915_private *dev_priv = dev->dev_private;
u64 raw_time; /* 32b value may overflow during fixed point math */
u64 units = 128ULL, div = 100000ULL, bias = 100ULL;
+ u32 ret;
if (!intel_enable_rc6(dev))
return 0;
+ intel_runtime_pm_get(dev_priv);
+
/* On VLV, residency time is in CZ units rather than 1.28us */
if (IS_VALLEYVIEW(dev)) {
u32 clkctl2;
@@ -52,7 +55,8 @@ static u32 calc_residency(struct drm_device *dev, const u32 reg)
CLK_CTL2_CZCOUNT_30NS_SHIFT;
if (!clkctl2) {
WARN(!clkctl2, "bogus CZ count value");
- return 0;
+ ret = 0;
+ goto out;
}
units = DIV_ROUND_UP_ULL(30ULL * bias, (u64)clkctl2);
if (I915_READ(VLV_COUNTER_CONTROL) & VLV_COUNT_RANGE_HIGH)
@@ -62,7 +66,11 @@ static u32 calc_residency(struct drm_device *dev, const u32 reg)
}
raw_time = I915_READ(reg) * units;
- return DIV_ROUND_UP_ULL(raw_time, div);
+ ret = DIV_ROUND_UP_ULL(raw_time, div);
+
+out:
+ intel_runtime_pm_put(dev_priv);
+ return ret;
}
static ssize_t
@@ -449,7 +457,9 @@ static ssize_t gt_rp_mhz_show(struct device *kdev, struct device_attribute *attr
ret = mutex_lock_interruptible(&dev->struct_mutex);
if (ret)
return ret;
+ intel_runtime_pm_get(dev_priv);
rp_state_cap = I915_READ(GEN6_RP_STATE_CAP);
+ intel_runtime_pm_put(dev_priv);
mutex_unlock(&dev->struct_mutex);
if (attr == &dev_attr_gt_RP0_freq_mhz) {
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 749c605..797ec11 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -3019,9 +3019,12 @@ intel_dp_detect(struct drm_connector *connector, bool force)
struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
struct intel_encoder *intel_encoder = &intel_dig_port->base;
struct drm_device *dev = connector->dev;
+ struct drm_i915_private *dev_priv = dev->dev_private;
enum drm_connector_status status;
struct edid *edid = NULL;
+ intel_runtime_pm_get(dev_priv);
+
DRM_DEBUG_KMS("[CONNECTOR:%d:%s]\n",
connector->base.id, drm_get_connector_name(connector));
@@ -3033,7 +3036,7 @@ intel_dp_detect(struct drm_connector *connector, bool force)
status = g4x_dp_detect(intel_dp);
if (status != connector_status_connected)
- return status;
+ goto out;
intel_dp_probe_oui(intel_dp);
@@ -3049,7 +3052,11 @@ intel_dp_detect(struct drm_connector *connector, bool force)
if (intel_encoder->type != INTEL_OUTPUT_EDP)
intel_encoder->type = INTEL_OUTPUT_DISPLAYPORT;
- return connector_status_connected;
+ status = connector_status_connected;
+
+out:
+ intel_runtime_pm_put(dev_priv);
+ return status;
}
static int intel_dp_get_modes(struct drm_connector *connector)
diff --git a/drivers/gpu/drm/i915/intel_panel.c b/drivers/gpu/drm/i915/intel_panel.c
index de15186..cb33ca0 100644
--- a/drivers/gpu/drm/i915/intel_panel.c
+++ b/drivers/gpu/drm/i915/intel_panel.c
@@ -692,7 +692,14 @@ static int intel_panel_update_status(struct backlight_device *bd)
static int intel_panel_get_brightness(struct backlight_device *bd)
{
struct drm_device *dev = bl_get_data(bd);
- return intel_panel_get_backlight(dev);
+ struct drm_i915_private *dev_priv = dev->dev_private;
+ int ret;
+
+ intel_runtime_pm_get(dev_priv);
+ ret = intel_panel_get_backlight(dev);
+ intel_runtime_pm_put(dev_priv);
+
+ return ret;
}
static const struct backlight_ops intel_panel_bl_ops = {
diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c
index f5a2a6d..6904ab2 100644
--- a/drivers/gpu/drm/i915/intel_uncore.c
+++ b/drivers/gpu/drm/i915/intel_uncore.c
@@ -285,6 +285,8 @@ void gen6_gt_force_wake_get(struct drm_i915_private *dev_priv)
if (!dev_priv->uncore.funcs.force_wake_get)
return;
+ intel_runtime_pm_get(dev_priv);
+
spin_lock_irqsave(&dev_priv->uncore.lock, irqflags);
if (dev_priv->uncore.forcewake_count++ == 0)
dev_priv->uncore.funcs.force_wake_get(dev_priv);
@@ -309,6 +311,8 @@ void gen6_gt_force_wake_put(struct drm_i915_private *dev_priv)
1);
}
spin_unlock_irqrestore(&dev_priv->uncore.lock, irqflags);
+
+ intel_runtime_pm_put(dev_priv);
}
/* We give fast paths for the really cool registers */
--
1.8.3.1
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [RFC 6/6] drm/i915: add runtime PM support on Haswell
2013-10-22 19:30 [RFC 0/6] Haswell runtime PM support + D3 Paulo Zanoni
` (4 preceding siblings ...)
2013-10-22 19:30 ` [RFC 5/6] drm/i915: add some runtime PM get/put calls Paulo Zanoni
@ 2013-10-22 19:30 ` Paulo Zanoni
2013-10-25 13:44 ` [RFC i-g-t] tests: add runtime_pm Paulo Zanoni
2013-10-28 13:09 ` [RFC 0/6] Haswell runtime PM support + D3 Imre Deak
7 siblings, 0 replies; 25+ messages in thread
From: Paulo Zanoni @ 2013-10-22 19:30 UTC (permalink / raw)
To: intel-gfx; +Cc: Paulo Zanoni
From: Paulo Zanoni <paulo.r.zanoni@intel.com>
The code to enable/disable PC8 already takes care of saving and
restoring all the registers we need to save/restore, so do a put()
call when we enable PC8 and a get() call when we disable it.
Ideally, in order to make it easier to add runtime PM support to other
platforms, we should move some things from the PC8 code to the runtime
PM code, but let's do this later, since we can make Haswell work right
now.
Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
drivers/gpu/drm/i915/i915_drv.h | 2 +-
drivers/gpu/drm/i915/intel_display.c | 4 ++++
2 files changed, 5 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 73ebb9e..31008c4 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -1785,7 +1785,7 @@ struct drm_i915_file_private {
#define HAS_POWER_WELL(dev) (IS_HASWELL(dev))
#define HAS_FPGA_DBG_UNCLAIMED(dev) (INTEL_INFO(dev)->has_fpga_dbg)
#define HAS_PSR(dev) (IS_HASWELL(dev))
-#define HAS_RUNTIME_PM(dev) false
+#define HAS_RUNTIME_PM(dev) (IS_HASWELL(dev))
#define INTEL_PCH_DEVICE_ID_MASK 0xff00
#define INTEL_PCH_IBX_DEVICE_ID_TYPE 0x3b00
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 3e79a2a..02d1067 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -6417,6 +6417,8 @@ void hsw_enable_pc8_work(struct work_struct *__work)
lpt_disable_clkout_dp(dev);
hsw_pc8_disable_interrupts(dev);
hsw_disable_lcpll(dev_priv, true, true);
+
+ intel_runtime_pm_put(dev_priv);
}
static void __hsw_enable_package_c8(struct drm_i915_private *dev_priv)
@@ -6452,6 +6454,8 @@ static void __hsw_disable_package_c8(struct drm_i915_private *dev_priv)
DRM_DEBUG_KMS("Disabling package C8+\n");
+ intel_runtime_pm_get(dev_priv);
+
hsw_restore_lcpll(dev_priv);
hsw_pc8_restore_interrupts(dev);
lpt_init_pch_refclk(dev);
--
1.8.3.1
^ permalink raw reply related [flat|nested] 25+ messages in thread
* [RFC i-g-t] tests: add runtime_pm
2013-10-22 19:30 [RFC 0/6] Haswell runtime PM support + D3 Paulo Zanoni
` (5 preceding siblings ...)
2013-10-22 19:30 ` [RFC 6/6] drm/i915: add runtime PM support on Haswell Paulo Zanoni
@ 2013-10-25 13:44 ` Paulo Zanoni
2013-10-27 13:37 ` Daniel Vetter
2013-10-28 13:09 ` [RFC 0/6] Haswell runtime PM support + D3 Imre Deak
7 siblings, 1 reply; 25+ messages in thread
From: Paulo Zanoni @ 2013-10-25 13:44 UTC (permalink / raw)
To: intel-gfx; +Cc: Paulo Zanoni
From: Paulo Zanoni <paulo.r.zanoni@intel.com>
This test is based on pc8.c. It copies most of the tests from pc8.c,
but it depends on runtime PM status changes (parsed from sysfs)
instead of PC8 residency changes (parsed from the MSR registers).
There's also a test that checks for PC8 residency.
For now, runtime PM and PC8 are different features, so having 2 test
suites makes sense. In the future we'll merge both, so we'll only get
PC8 when runtime PM is enabled, so we'll just kill pc8.c and keep
using runtime_pm.c.
Changes compared to pc8.c:
- We now look at the runtime PM status instead of PC8 residencies
- Added more GEM tests (mmap, pread, execbuf, stress tests)
- Added LPSP and non-LPSP tests
- Added tests fro sysfs and debugfs files
- Added a test specifically for PC8
Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
---
tests/.gitignore | 1 +
tests/Makefile.am | 1 +
tests/runtime_pm.c | 1262 ++++++++++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 1264 insertions(+)
create mode 100644 tests/runtime_pm.c
diff --git a/tests/.gitignore b/tests/.gitignore
index 1c97a04..cda6d09 100644
--- a/tests/.gitignore
+++ b/tests/.gitignore
@@ -105,6 +105,7 @@ prime_nv_pcopy
prime_nv_test
prime_self_import
prime_udl
+runtime_pm
sysfs_rc6_residency
sysfs_rps
testdisplay
diff --git a/tests/Makefile.am b/tests/Makefile.am
index f6ba368..234aaf5 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -59,6 +59,7 @@ TESTS_progs_M = \
$(NOUVEAU_TESTS_M) \
pc8 \
prime_self_import \
+ runtime_pm \
$(NULL)
TESTS_progs = \
diff --git a/tests/runtime_pm.c b/tests/runtime_pm.c
new file mode 100644
index 0000000..425e743
--- /dev/null
+++ b/tests/runtime_pm.c
@@ -0,0 +1,1262 @@
+/*
+ * Copyright © 2013 Intel Corporation
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ * The above copyright notice and this permission notice (including the next
+ * paragraph) shall be included in all copies or substantial portions of the
+ * Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
+ * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
+ * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
+ * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
+ * IN THE SOFTWARE.
+ *
+ * Authors:
+ * Paulo Zanoni <paulo.r.zanoni@intel.com>
+ *
+ */
+
+#include <string.h>
+#include <stdio.h>
+#include <stdint.h>
+#include <stdbool.h>
+#include <stdlib.h>
+
+#include <dirent.h>
+#include <fcntl.h>
+#include <unistd.h>
+#include <sys/ioctl.h>
+#include <sys/mman.h>
+#include <sys/types.h>
+#include <sys/stat.h>
+#include <linux/i2c.h>
+#include <linux/i2c-dev.h>
+
+#include "drmtest.h"
+#include "intel_gpu_tools.h"
+#include "igt_display.h"
+
+
+#define MSR_PC8_RES 0x630
+#define MSR_PC9_RES 0x631
+#define MSR_PC10_RES 0x632
+
+#define POWER_DIR "/sys/devices/pci0000:00/0000:00:02.0/power"
+
+#define MAX_CONNECTORS 32
+#define MAX_ENCODERS 32
+#define MAX_CRTCS 16
+
+
+enum runtime_pm_status {
+ RUNTIME_PM_STATUS_ACTIVE,
+ RUNTIME_PM_STATUS_SUSPENDED,
+ RUNTIME_PM_STATUS_SUSPENDING,
+ RUNTIME_PM_STATUS_UNKNOWN,
+};
+
+enum screen_type {
+ SCREEN_TYPE_LPSP,
+ SCREEN_TYPE_NON_LPSP,
+ SCREEN_TYPE_ANY,
+};
+
+/* Stuff used when creating FBs and mode setting. */
+struct mode_set_data {
+ drmModeResPtr res;
+ drmModeConnectorPtr connectors[MAX_CONNECTORS];
+ drmModePropertyBlobPtr edids[MAX_CONNECTORS];
+
+ uint32_t devid;
+};
+
+/* Stuff we query at different times so we can compare. */
+struct compare_data {
+ drmModeResPtr res;
+ drmModeEncoderPtr encoders[MAX_ENCODERS];
+ drmModeConnectorPtr connectors[MAX_CONNECTORS];
+ drmModeCrtcPtr crtcs[MAX_CRTCS];
+ drmModePropertyBlobPtr edids[MAX_CONNECTORS];
+};
+
+
+int drm_fd;
+int status_fd;
+int msr_fd = 0;
+struct mode_set_data ms_data;
+bool quick = false;
+
+
+static uint64_t get_residency(uint32_t type)
+{
+ int rc;
+ uint64_t ret;
+
+ rc = pread(msr_fd, &ret, sizeof(uint64_t), type);
+ igt_assert(rc == sizeof(ret));
+
+ return ret;
+}
+
+static bool pc8_plus_residency_changed(unsigned int timeout_sec)
+{
+ unsigned int i;
+ uint64_t res_pc8, res_pc9, res_pc10;
+ int to_sleep = 100 * 1000;
+
+ res_pc8 = get_residency(MSR_PC8_RES);
+ res_pc9 = get_residency(MSR_PC9_RES);
+ res_pc10 = get_residency(MSR_PC10_RES);
+
+ for (i = 0; i < timeout_sec * 1000 * 1000; i += to_sleep) {
+ if (res_pc8 != get_residency(MSR_PC8_RES) ||
+ res_pc9 != get_residency(MSR_PC9_RES) ||
+ res_pc10 != get_residency(MSR_PC10_RES)) {
+ return true;
+ }
+ usleep(to_sleep);
+ }
+
+ return false;
+}
+
+/* Checks not only if PC8+ is allowed, but also if we're reaching it.
+ * We call this when we expect this function to return quickly since PC8 is
+ * actually enabled, so the 30s timeout we use shouldn't matter. */
+static bool pc8_plus_enabled(void)
+{
+ return pc8_plus_residency_changed(30);
+}
+
+/* We call this when we expect PC8+ to be actually disabled, so we should not
+ * return until the 5s timeout expires. In other words: in the "happy case",
+ * every time we call this function, it will sleep for 5 seconds. */
+static bool pc8_plus_disabled(void)
+{
+ return !pc8_plus_residency_changed(5);
+}
+
+/* If the read fails, then the machine doesn't support PC8+ residencies. */
+static bool supports_pc8_plus_residencies(void)
+{
+ int rc;
+ uint64_t val;
+
+ rc = pread(msr_fd, &val, sizeof(uint64_t), MSR_PC8_RES);
+ if (rc != sizeof(val))
+ return false;
+ rc = pread(msr_fd, &val, sizeof(uint64_t), MSR_PC9_RES);
+ if (rc != sizeof(val))
+ return false;
+ rc = pread(msr_fd, &val, sizeof(uint64_t), MSR_PC10_RES);
+ if (rc != sizeof(val))
+ return false;
+
+ return true;
+}
+
+static enum runtime_pm_status get_runtime_pm_status(void)
+{
+ ssize_t n_read;
+ char buf[32];
+
+ lseek(status_fd, 0, SEEK_SET);
+ n_read = read(status_fd, buf, ARRAY_SIZE(buf));
+ igt_assert(n_read >= 0);
+ buf[n_read] = '\0';
+
+ if (strncmp(buf, "suspended\n", n_read) == 0)
+ return RUNTIME_PM_STATUS_SUSPENDED;
+ else if (strncmp(buf, "active\n", n_read) == 0)
+ return RUNTIME_PM_STATUS_ACTIVE;
+ else if (strncmp(buf, "suspending\n", n_read) == 0)
+ return RUNTIME_PM_STATUS_SUSPENDING;
+
+ igt_assert_f(false, "Unknown status %s\n", buf);
+ return RUNTIME_PM_STATUS_UNKNOWN;
+}
+
+static bool wait_for_status(enum runtime_pm_status status)
+{
+ int i;
+ int hundred_ms = 100 * 1000, ten_s = 10 * 1000 * 1000;
+
+ for (i = 0; i < ten_s; i += hundred_ms) {
+ if (get_runtime_pm_status() == status)
+ return true;
+
+ usleep(hundred_ms);
+ }
+
+ return false;
+}
+
+static bool wait_for_suspended(void)
+{
+ return wait_for_status(RUNTIME_PM_STATUS_SUSPENDED);
+}
+
+static bool wait_for_active(void)
+{
+ return wait_for_status(RUNTIME_PM_STATUS_ACTIVE);
+}
+
+static void disable_all_screens(struct mode_set_data *data)
+{
+ int i, rc;
+
+ for (i = 0; i < data->res->count_crtcs; i++) {
+ rc = drmModeSetCrtc(drm_fd, data->res->crtcs[i], -1, 0, 0,
+ NULL, 0, NULL);
+ igt_assert(rc == 0);
+ }
+}
+
+static uint32_t create_fb(struct mode_set_data *data, int width, int height)
+{
+ struct kmstest_fb fb;
+ cairo_t *cr;
+ uint32_t buffer_id;
+
+ buffer_id = kmstest_create_fb(drm_fd, width, height, 32, 24, false,
+ &fb);
+ cr = kmstest_get_cairo_ctx(drm_fd, &fb);
+ kmstest_paint_test_pattern(cr, width, height);
+ return buffer_id;
+}
+
+static bool enable_one_screen_with_type(struct mode_set_data *data,
+ enum screen_type type)
+{
+ uint32_t crtc_id = 0, buffer_id = 0, connector_id = 0;
+ drmModeModeInfoPtr mode = NULL;
+ int i, rc;
+
+ for (i = 0; i < data->res->count_connectors; i++) {
+ drmModeConnectorPtr c = data->connectors[i];
+
+ if (type == SCREEN_TYPE_LPSP &&
+ c->connector_type != DRM_MODE_CONNECTOR_eDP)
+ continue;
+
+ if (type == SCREEN_TYPE_NON_LPSP &&
+ c->connector_type == DRM_MODE_CONNECTOR_eDP)
+ continue;
+
+ if (c->connection == DRM_MODE_CONNECTED && c->count_modes) {
+ connector_id = c->connector_id;
+ mode = &c->modes[0];
+ break;
+ }
+ }
+
+ if (connector_id == 0)
+ return false;
+
+ crtc_id = data->res->crtcs[0];
+ buffer_id = create_fb(data, mode->hdisplay, mode->vdisplay);
+
+ igt_assert(crtc_id);
+ igt_assert(buffer_id);
+ igt_assert(connector_id);
+ igt_assert(mode);
+
+ rc = drmModeSetCrtc(drm_fd, crtc_id, buffer_id, 0, 0, &connector_id,
+ 1, mode);
+ igt_assert(rc == 0);
+
+ return true;
+}
+
+static void enable_one_screen(struct mode_set_data *data)
+{
+ igt_assert(enable_one_screen_with_type(data, SCREEN_TYPE_ANY));
+}
+
+/* We could check the checksum too, but just the header is probably enough. */
+static bool edid_is_valid(const unsigned char *edid)
+{
+ char edid_header[] = {
+ 0x0, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0x0,
+ };
+
+ return (memcmp(edid, edid_header, sizeof(edid_header)) == 0);
+}
+
+static int count_drm_valid_edids(struct mode_set_data *data)
+{
+ int i, ret = 0;
+
+ for (i = 0; i < data->res->count_connectors; i++)
+ if (data->edids[i] && edid_is_valid(data->edids[i]->data))
+ ret++;
+ return ret;
+}
+
+static bool i2c_edid_is_valid(int fd)
+{
+ int rc;
+ unsigned char edid[128] = {};
+ struct i2c_msg msgs[] = {
+ { /* Start at 0. */
+ .addr = 0x50,
+ .flags = 0,
+ .len = 1,
+ .buf = edid,
+ }, { /* Now read the EDID. */
+ .addr = 0x50,
+ .flags = I2C_M_RD,
+ .len = 128,
+ .buf = edid,
+ }
+ };
+ struct i2c_rdwr_ioctl_data msgset = {
+ .msgs = msgs,
+ .nmsgs = 2,
+ };
+
+ rc = ioctl(fd, I2C_RDWR, &msgset);
+ return (rc >= 0) ? edid_is_valid(edid) : false;
+}
+
+static int count_i2c_valid_edids(void)
+{
+ int fd, ret = 0;
+ DIR *dir;
+
+ struct dirent *dirent;
+ char full_name[32];
+
+ dir = opendir("/dev/");
+ igt_assert(dir);
+
+ while ((dirent = readdir(dir))) {
+ if (strncmp(dirent->d_name, "i2c-", 4) == 0) {
+ snprintf(full_name, 32, "/dev/%s", dirent->d_name);
+ fd = open(full_name, O_RDWR);
+ igt_assert(fd != -1);
+ if (i2c_edid_is_valid(fd))
+ ret++;
+ close(fd);
+ }
+ }
+
+ closedir(dir);
+
+ return ret;
+}
+
+static void test_i2c(struct mode_set_data *data)
+{
+ int i2c_edids = count_i2c_valid_edids();
+ int drm_edids = count_drm_valid_edids(data);
+
+ igt_assert(i2c_edids == drm_edids);
+}
+
+static void i2c_subtest_check_environment(void)
+{
+ int i2c_dev_files = 0;
+ DIR *dev_dir;
+ struct dirent *dirent;
+
+ /* Make sure the /dev/i2c-* files exist. */
+ dev_dir = opendir("/dev");
+ igt_assert(dev_dir);
+ while ((dirent = readdir(dev_dir))) {
+ if (strncmp(dirent->d_name, "i2c-", 4) == 0)
+ i2c_dev_files++;
+ }
+ closedir(dev_dir);
+ igt_require(i2c_dev_files);
+}
+
+
+#define COMPARE(d1, d2, data) igt_assert(d1->data == d2->data)
+#define COMPARE_ARRAY(d1, d2, size, data) do { \
+ for (i = 0; i < size; i++) \
+ igt_assert(d1->data[i] == d2->data[i]); \
+} while (0)
+
+static void assert_drm_resources_equal(struct compare_data *d1,
+ struct compare_data *d2)
+{
+ COMPARE(d1, d2, res->count_connectors);
+ COMPARE(d1, d2, res->count_encoders);
+ COMPARE(d1, d2, res->count_crtcs);
+ COMPARE(d1, d2, res->min_width);
+ COMPARE(d1, d2, res->max_width);
+ COMPARE(d1, d2, res->min_height);
+ COMPARE(d1, d2, res->max_height);
+}
+
+static void assert_modes_equal(drmModeModeInfoPtr m1, drmModeModeInfoPtr m2)
+{
+ COMPARE(m1, m2, clock);
+ COMPARE(m1, m2, hdisplay);
+ COMPARE(m1, m2, hsync_start);
+ COMPARE(m1, m2, hsync_end);
+ COMPARE(m1, m2, htotal);
+ COMPARE(m1, m2, hskew);
+ COMPARE(m1, m2, vdisplay);
+ COMPARE(m1, m2, vsync_start);
+ COMPARE(m1, m2, vsync_end);
+ COMPARE(m1, m2, vtotal);
+ COMPARE(m1, m2, vscan);
+ COMPARE(m1, m2, vrefresh);
+ COMPARE(m1, m2, flags);
+ COMPARE(m1, m2, type);
+ igt_assert(strcmp(m1->name, m2->name) == 0);
+}
+
+static void assert_drm_connectors_equal(drmModeConnectorPtr c1,
+ drmModeConnectorPtr c2)
+{
+ int i;
+
+ COMPARE(c1, c2, connector_id);
+ COMPARE(c1, c2, connector_type);
+ COMPARE(c1, c2, connector_type_id);
+ COMPARE(c1, c2, mmWidth);
+ COMPARE(c1, c2, mmHeight);
+ COMPARE(c1, c2, count_modes);
+ COMPARE(c1, c2, count_props);
+ COMPARE(c1, c2, count_encoders);
+ COMPARE_ARRAY(c1, c2, c1->count_props, props);
+ COMPARE_ARRAY(c1, c2, c1->count_encoders, encoders);
+
+ for (i = 0; i < c1->count_modes; i++)
+ assert_modes_equal(&c1->modes[0], &c2->modes[0]);
+}
+
+static void assert_drm_encoders_equal(drmModeEncoderPtr e1,
+ drmModeEncoderPtr e2)
+{
+ COMPARE(e1, e2, encoder_id);
+ COMPARE(e1, e2, encoder_type);
+ COMPARE(e1, e2, possible_crtcs);
+ COMPARE(e1, e2, possible_clones);
+}
+
+static void assert_drm_crtcs_equal(drmModeCrtcPtr c1, drmModeCrtcPtr c2)
+{
+ COMPARE(c1, c2, crtc_id);
+}
+
+static void assert_drm_edids_equal(drmModePropertyBlobPtr e1,
+ drmModePropertyBlobPtr e2)
+{
+ if (!e1 && !e2)
+ return;
+ igt_assert(e1 && e2);
+
+ COMPARE(e1, e2, id);
+ COMPARE(e1, e2, length);
+
+ igt_assert(memcmp(e1->data, e2->data, e1->length) == 0);
+}
+
+static void assert_drm_infos_equal(struct compare_data *d1,
+ struct compare_data *d2)
+{
+ int i;
+
+ assert_drm_resources_equal(d1, d2);
+
+ for (i = 0; i < d1->res->count_connectors; i++) {
+ assert_drm_connectors_equal(d1->connectors[i],
+ d2->connectors[i]);
+ assert_drm_edids_equal(d1->edids[i], d2->edids[i]);
+ }
+
+ for (i = 0; i < d1->res->count_encoders; i++)
+ assert_drm_encoders_equal(d1->encoders[i], d2->encoders[i]);
+
+ for (i = 0; i < d1->res->count_crtcs; i++)
+ assert_drm_crtcs_equal(d1->crtcs[i], d2->crtcs[i]);
+}
+
+static drmModePropertyBlobPtr get_connector_edid(drmModeConnectorPtr connector,
+ int index)
+{
+ unsigned int i;
+ drmModeObjectPropertiesPtr props;
+ drmModePropertyBlobPtr ret = NULL;
+
+ props = drmModeObjectGetProperties(drm_fd, connector->connector_id,
+ DRM_MODE_OBJECT_CONNECTOR);
+
+ for (i = 0; i < props->count_props; i++) {
+ drmModePropertyPtr prop = drmModeGetProperty(drm_fd,
+ props->props[i]);
+
+ if (strcmp(prop->name, "EDID") == 0) {
+ igt_assert(prop->flags & DRM_MODE_PROP_BLOB);
+ igt_assert(prop->count_blobs == 0);
+ ret = drmModeGetPropertyBlob(drm_fd,
+ props->prop_values[i]);
+ }
+
+ drmModeFreeProperty(prop);
+ }
+
+ drmModeFreeObjectProperties(props);
+ return ret;
+}
+
+
+static void get_drm_info(struct compare_data *data)
+{
+ int i;
+
+ data->res = drmModeGetResources(drm_fd);
+ igt_assert(data->res);
+
+ igt_assert(data->res->count_connectors <= MAX_CONNECTORS);
+ igt_assert(data->res->count_encoders <= MAX_ENCODERS);
+ igt_assert(data->res->count_crtcs <= MAX_CRTCS);
+
+ for (i = 0; i < data->res->count_connectors; i++) {
+ data->connectors[i] = drmModeGetConnector(drm_fd,
+ data->res->connectors[i]);
+ data->edids[i] = get_connector_edid(data->connectors[i], i);
+ }
+ for (i = 0; i < data->res->count_encoders; i++)
+ data->encoders[i] = drmModeGetEncoder(drm_fd,
+ data->res->encoders[i]);
+ for (i = 0; i < data->res->count_crtcs; i++)
+ data->crtcs[i] = drmModeGetCrtc(drm_fd, data->res->crtcs[i]);
+}
+
+static void free_drm_info(struct compare_data *data)
+{
+ int i;
+
+ for (i = 0; i < data->res->count_connectors; i++) {
+ drmModeFreeConnector(data->connectors[i]);
+ drmModeFreePropertyBlob(data->edids[i]);
+ }
+ for (i = 0; i < data->res->count_encoders; i++)
+ drmModeFreeEncoder(data->encoders[i]);
+ for (i = 0; i < data->res->count_crtcs; i++)
+ drmModeFreeCrtc(data->crtcs[i]);
+
+ drmModeFreeResources(data->res);
+}
+
+static void init_mode_set_data(struct mode_set_data *data)
+{
+ int i;
+
+ data->res = drmModeGetResources(drm_fd);
+ igt_assert(data->res);
+ igt_assert(data->res->count_connectors <= MAX_CONNECTORS);
+
+ for (i = 0; i < data->res->count_connectors; i++) {
+ data->connectors[i] = drmModeGetConnector(drm_fd,
+ data->res->connectors[i]);
+ data->edids[i] = get_connector_edid(data->connectors[i], i);
+ }
+
+ data->devid = intel_get_drm_devid(drm_fd);
+
+ do_or_die(igt_set_vt_graphics_mode());
+}
+
+static void fini_mode_set_data(struct mode_set_data *data)
+{
+ int i;
+
+ for (i = 0; i < data->res->count_connectors; i++) {
+ drmModeFreeConnector(data->connectors[i]);
+ drmModeFreePropertyBlob(data->edids[i]);
+ }
+ drmModeFreeResources(data->res);
+}
+
+static void pc8_subtest_check_environment(void)
+{
+ igt_require_f(IS_HASWELL(ms_data.devid),
+ "PC8+ feature only supported on Haswell.\n");
+
+ /* If this fails, you don't have the Kernel module loaded. */
+ msr_fd = open("/dev/cpu/0/msr", O_RDONLY);
+ igt_assert_f(msr_fd >= 0, "Can't open /dev/cpu/0/msr.\n");
+
+ /* Non-ULT machines don't support PC8+. */
+ igt_require_f(supports_pc8_plus_residencies(),
+ "Machine doesn't support PC8+ residencies.\n");
+}
+
+static void assert_runtime_pm_is_enabled(void)
+{
+ int fd;
+ ssize_t n_read;
+ char buf[6];
+ int rc;
+
+ fd = open(POWER_DIR "/control", O_RDONLY);
+ igt_assert_f(fd >= 0, "Can't open " POWER_DIR "/control\n");
+
+ n_read = read(fd, buf, ARRAY_SIZE(buf));
+ igt_assert(n_read >= 0);
+ igt_require_f(strncmp(buf, "auto\n", n_read) == 0,
+ "Runtime PM not enabled or supported on this platform\n");
+
+ rc = close(fd);
+ igt_assert(rc == 0);
+}
+
+/* We set 1ms autosuspend delay here so things start/stop fast and we don't
+ * waste time waiting for runtime PM to kick in. We don't care about restoring
+ * the original value at the end of the test. */
+static void set_autosuspend_delay(void)
+{
+ int fd;
+ ssize_t n_write;
+ int rc;
+
+ fd = open(POWER_DIR "/autosuspend_delay_ms", O_WRONLY);
+ igt_assert_f(fd >= 0,
+ "Can't open " POWER_DIR "/autosuspend_delay_ms\n");
+
+ n_write = write(fd, "1\n", 2);
+ igt_assert(n_write == 2);
+
+ rc = close(fd);
+ igt_assert(rc == 0);
+
+}
+
+static void setup_environment(void)
+{
+
+ drm_fd = drm_open_any();
+ igt_assert(drm_fd >= 0);
+
+ init_mode_set_data(&ms_data);
+
+ assert_runtime_pm_is_enabled(),
+
+ status_fd = open(POWER_DIR "/runtime_status", O_RDONLY);
+ igt_assert_f(status_fd >= 0,
+ "Can't open " POWER_DIR "/runtime_status\n");
+
+ set_autosuspend_delay();
+}
+
+static void teardown_environment(void)
+{
+ int rc;
+
+ fini_mode_set_data(&ms_data);
+
+ drmClose(drm_fd);
+
+ rc = close(status_fd);
+ igt_assert(rc == 0);
+
+ if (msr_fd) {
+ rc = close(msr_fd);
+ igt_assert(rc == 0);
+ }
+}
+
+/* Sanity test: make sure it suspends when all screens are disabled and it
+ * resumes when we enable a screen. */
+static void basic_subtest(void)
+{
+ disable_all_screens(&ms_data);
+ igt_assert_f(wait_for_suspended(),
+ "Graphics device is not suspending, please check its "
+ "runtime PM configuration.\n");
+
+ enable_one_screen(&ms_data);
+ igt_assert_f(wait_for_active(),
+ "Graphics device is still suspended even with a screen "
+ "enabled. Something is really wrong.\n");
+}
+
+/* Test if the DRM resources reported by the IOCTLs are still the same. This
+ * ensures we still see the monitors with the same eyes. We get the EDIDs and
+ * compare them, which ensures we use DP AUX or GMBUS depending on what's
+ * connected. */
+static void drm_resources_subtest(void)
+{
+ struct compare_data pre_suspend, during_suspend, post_suspend;
+
+ enable_one_screen(&ms_data);
+ igt_assert(wait_for_active());
+ get_drm_info(&pre_suspend);
+ igt_assert(wait_for_active());
+
+ disable_all_screens(&ms_data);
+ igt_assert(wait_for_suspended());
+ get_drm_info(&during_suspend);
+ igt_assert(wait_for_suspended());
+
+ enable_one_screen(&ms_data);
+ igt_assert(wait_for_active());
+ get_drm_info(&post_suspend);
+ igt_assert(wait_for_active());
+
+ assert_drm_infos_equal(&pre_suspend, &during_suspend);
+ assert_drm_infos_equal(&pre_suspend, &post_suspend);
+
+ free_drm_info(&pre_suspend);
+ free_drm_info(&during_suspend);
+ free_drm_info(&post_suspend);
+}
+
+/* Try to use raw I2C, which also needs interrupts. */
+static void i2c_subtest(void)
+{
+ i2c_subtest_check_environment();
+
+ enable_one_screen(&ms_data);
+ igt_assert(wait_for_active());
+
+ disable_all_screens(&ms_data);
+ igt_assert(wait_for_suspended());
+ test_i2c(&ms_data);
+ igt_assert(wait_for_suspended());
+
+ enable_one_screen(&ms_data);
+ igt_assert(wait_for_active());
+}
+
+/* The LPSP version tests eDP, which doesn't require the power well. The
+ * non-LPSP version will make sure we toggle the power well on/off every time.
+ * We SKIP in case we can't find the appropriate type of output connected, but
+ * that doesn't test as much as we could. */
+static void modeset_stress(bool lpsp)
+{
+ int i, max;
+ enum screen_type type;
+
+ max = quick ? 5 : 50;
+ type = lpsp ? SCREEN_TYPE_LPSP : SCREEN_TYPE_NON_LPSP;
+
+ for (i = 0; i < max; i++) {
+ disable_all_screens(&ms_data);
+ igt_assert(wait_for_suspended());
+
+ /* If we skip this line it's because the type of screen we want
+ * is not connected. */
+ igt_require(enable_one_screen_with_type(&ms_data, type));
+ igt_assert(wait_for_active());
+ }
+}
+
+/* Make sure we get to PC8 or deeper states when we're suspended. Notice that
+ * this actually depends on a properly configured machine, where the runtime PM
+ * polices for _all_ devices (not only graphics) are configured correctly. If
+ * you use "powertop" and switch everything to "good", there's a good chance
+ * you'll be fine here. */
+static void pc8_subtest(void)
+{
+ pc8_subtest_check_environment();
+
+ disable_all_screens(&ms_data);
+ igt_assert(wait_for_suspended());
+ igt_assert(pc8_plus_enabled());
+
+ enable_one_screen(&ms_data);
+ igt_assert(wait_for_active());
+ igt_assert(pc8_plus_disabled());
+}
+
+static void *mmap_gem_buf(uint32_t handle, int size)
+{
+ struct drm_i915_gem_mmap gem_mmap;
+
+ gem_mmap.handle = handle;
+ gem_mmap.offset = 0;
+ gem_mmap.size = size;
+ do_ioctl(drm_fd, DRM_IOCTL_I915_GEM_MMAP, &gem_mmap);
+ igt_assert(gem_mmap.addr_ptr);
+
+ return (void *) gem_mmap.addr_ptr;
+}
+
+/* Make sure even mmap still works when we're in D3. */
+static void gem_mmap_subtest(void)
+{
+ int i;
+ uint32_t handle;
+ int buf_size = 8192;
+ uint8_t *gem_buf;
+
+ /* Create, map and set data while the device is active. */
+ enable_one_screen(&ms_data);
+ igt_assert(wait_for_active());
+
+ handle = gem_create(drm_fd, buf_size);
+
+ gem_buf = mmap_gem_buf(handle, buf_size);
+
+ for (i = 0; i < buf_size; i++)
+ gem_buf[i] = i & 0xFF;
+
+ for (i = 0; i < buf_size; i++)
+ igt_assert(gem_buf[i] == (i & 0xFF));
+
+ /* Now suspend, read and modify. */
+ disable_all_screens(&ms_data);
+ igt_assert(wait_for_suspended());
+
+ for (i = 0; i < buf_size; i++)
+ igt_assert(gem_buf[i] == (i & 0xFF));
+ igt_assert(wait_for_suspended());
+
+ for (i = 0; i < buf_size; i++)
+ gem_buf[i] = (~i & 0xFF);
+ igt_assert(wait_for_suspended());
+
+ /* Now resume and see if it's still there. */
+ enable_one_screen(&ms_data);
+ igt_assert(wait_for_active());
+ for (i = 0; i < buf_size; i++)
+ igt_assert(gem_buf[i] == (~i & 0xFF));
+
+ munmap(gem_buf, buf_size);
+
+ gem_close(drm_fd, handle);
+}
+
+/* Make sure pread() and pwrite() still work when we're in D3. */
+static void gem_pread_subtest(void)
+{
+ int i;
+ uint32_t handle;
+ int buf_size = 8192;
+ uint8_t *cpu_buf, *read_buf;
+
+ cpu_buf = malloc(buf_size);
+ read_buf = malloc(buf_size);
+ igt_assert(cpu_buf);
+ igt_assert(read_buf);
+ memset(cpu_buf, 0, buf_size);
+ memset(read_buf, 0, buf_size);
+
+ /* Create and set data while the device is active. */
+ enable_one_screen(&ms_data);
+ igt_assert(wait_for_active());
+
+ handle = gem_create(drm_fd, buf_size);
+
+ for (i = 0; i < buf_size; i++)
+ cpu_buf[i] = i & 0xFF;
+
+ gem_write(drm_fd, handle, 0, cpu_buf, buf_size);
+
+ gem_read(drm_fd, handle, 0, read_buf, buf_size);
+
+ for (i = 0; i < buf_size; i++)
+ igt_assert(cpu_buf[i] == read_buf[i]);
+
+ /* Now suspend, read and modify. */
+ disable_all_screens(&ms_data);
+ igt_assert(wait_for_suspended());
+
+ memset(read_buf, 0, buf_size);
+ gem_read(drm_fd, handle, 0, read_buf, buf_size);
+
+ for (i = 0; i < buf_size; i++)
+ igt_assert(cpu_buf[i] == read_buf[i]);
+ igt_assert(wait_for_suspended());
+
+ for (i = 0; i < buf_size; i++)
+ cpu_buf[i] = (~i & 0xFF);
+ gem_write(drm_fd, handle, 0, cpu_buf, buf_size);
+ igt_assert(wait_for_suspended());
+
+ /* Now resume and see if it's still there. */
+ enable_one_screen(&ms_data);
+ igt_assert(wait_for_active());
+
+ memset(read_buf, 0, buf_size);
+ gem_read(drm_fd, handle, 0, read_buf, buf_size);
+
+ for (i = 0; i < buf_size; i++)
+ igt_assert(cpu_buf[i] == read_buf[i]);
+
+ gem_close(drm_fd, handle);
+
+ free(cpu_buf);
+ free(read_buf);
+}
+
+/* Paints a square of color $color, size $width x $height, at position $x x $y
+ * of $dst_handle, which contains pitch $pitch. */
+static void submit_blt_cmd(uint32_t dst_handle, uint32_t x, uint32_t y,
+ uint32_t width, uint32_t height, uint32_t pitch,
+ uint32_t color, uint32_t *presumed_dst_offset)
+{
+ int i, reloc_pos;
+ int bpp = 4;
+ uint32_t batch_handle;
+ int batch_size = 8 * sizeof(uint32_t);
+ uint32_t batch_buf[batch_size];
+ uint32_t offset_in_dst = (pitch * y) + (x * bpp);
+ struct drm_i915_gem_execbuffer2 execbuf = {};
+ struct drm_i915_gem_exec_object2 objs[2] = {{}, {}};
+ struct drm_i915_gem_relocation_entry relocs[1] = {{}};
+ struct drm_i915_gem_wait gem_wait;
+
+ i = 0;
+
+ batch_buf[i++] = COLOR_BLT_CMD | COLOR_BLT_WRITE_ALPHA |
+ COLOR_BLT_WRITE_RGB;
+ batch_buf[i++] = (3 << 24) | (0xF0 << 16) | pitch;
+ batch_buf[i++] = (height << 16) | width * bpp;
+ reloc_pos = i;
+ batch_buf[i++] = *presumed_dst_offset + offset_in_dst;
+ batch_buf[i++] = color;
+
+ batch_buf[i++] = MI_NOOP;
+ batch_buf[i++] = MI_BATCH_BUFFER_END;
+ batch_buf[i++] = MI_NOOP;
+
+ igt_assert(i * sizeof(uint32_t) == batch_size);
+
+ batch_handle = gem_create(drm_fd, batch_size);
+ gem_write(drm_fd, batch_handle, 0, batch_buf, batch_size);
+
+ relocs[0].target_handle = dst_handle;
+ relocs[0].delta = offset_in_dst;
+ relocs[0].offset = reloc_pos * sizeof(uint32_t);
+ relocs[0].presumed_offset = *presumed_dst_offset;
+ relocs[0].read_domains = 0;
+ relocs[0].write_domain = I915_GEM_DOMAIN_RENDER;
+
+ objs[0].handle = dst_handle;
+ objs[0].alignment = 64;
+
+ objs[1].handle = batch_handle;
+ objs[1].relocation_count = 1;
+ objs[1].relocs_ptr = (uint64_t) relocs;
+
+ execbuf.buffers_ptr = (uint64_t) objs;
+ execbuf.buffer_count = 2;
+ execbuf.batch_len = batch_size;
+ execbuf.flags = I915_EXEC_BLT;
+ i915_execbuffer2_set_context_id(execbuf, 0);
+
+ do_ioctl(drm_fd, DRM_IOCTL_I915_GEM_EXECBUFFER2, &execbuf);
+
+ *presumed_dst_offset = relocs[0].presumed_offset;
+
+ gem_wait.flags = 0;
+ gem_wait.timeout_ns = 10000000000LL; /* 10s */
+
+ gem_wait.bo_handle = batch_handle;
+ do_ioctl(drm_fd, DRM_IOCTL_I915_GEM_WAIT, &gem_wait);
+
+ gem_wait.bo_handle = dst_handle;
+ do_ioctl(drm_fd, DRM_IOCTL_I915_GEM_WAIT, &gem_wait);
+
+ gem_close(drm_fd, batch_handle);
+}
+
+/* Make sure we can submit a batch buffer and verify its result. */
+static void gem_execbuf_subtest(void)
+{
+ int x, y;
+ uint32_t handle;
+ int bpp = 4;
+ int pitch = 128 * bpp;
+ int dst_size = 128 * 128 * bpp; /* 128x128 square */
+ uint32_t *cpu_buf;
+ uint32_t presumed_offset = 0;
+ int sq_x = 5, sq_y = 10, sq_w = 15, sq_h = 20;
+ uint32_t color;
+
+ /* Create and set data while the device is active. */
+ enable_one_screen(&ms_data);
+ igt_assert(wait_for_active());
+
+ handle = gem_create(drm_fd, dst_size);
+
+ cpu_buf = malloc(dst_size);
+ igt_assert(cpu_buf);
+ memset(cpu_buf, 0, dst_size);
+ gem_write(drm_fd, handle, 0, cpu_buf, dst_size);
+
+ /* Now suspend and try it. */
+ disable_all_screens(&ms_data);
+ igt_assert(wait_for_suspended());
+
+ color = 0x12345678;
+ submit_blt_cmd(handle, sq_x, sq_y, sq_w, sq_h, pitch, color,
+ &presumed_offset);
+ igt_assert(wait_for_suspended());
+
+ gem_read(drm_fd, handle, 0, cpu_buf, dst_size);
+ igt_assert(wait_for_suspended());
+ for (y = 0; y < 128; y++) {
+ for (x = 0; x < 128; x++) {
+ uint32_t px = cpu_buf[y * 128 + x];
+
+ if (y >= sq_y && y < (sq_y + sq_h) &&
+ x >= sq_x && x < (sq_x + sq_w))
+ igt_assert(px == color);
+ else
+ igt_assert(px == 0);
+ }
+ }
+
+ /* Now resume and check for it again. */
+ enable_one_screen(&ms_data);
+ igt_assert(wait_for_active());
+
+ memset(cpu_buf, 0, dst_size);
+ gem_read(drm_fd, handle, 0, cpu_buf, dst_size);
+ for (y = 0; y < 128; y++) {
+ for (x = 0; x < 128; x++) {
+ uint32_t px = cpu_buf[y * 128 + x];
+
+ if (y >= sq_y && y < (sq_y + sq_h) &&
+ x >= sq_x && x < (sq_x + sq_w))
+ igt_assert(px == color);
+ else
+ igt_assert(px == 0);
+ }
+ }
+
+ /* Now we'll do the opposite: do the blt while active, then read while
+ * suspended. We use the same spot, but a different color. As a bonus,
+ * we're testing the presumed_offset from the previous command. */
+ color = 0x87654321;
+ submit_blt_cmd(handle, sq_x, sq_y, sq_w, sq_h, pitch, color,
+ &presumed_offset);
+
+ disable_all_screens(&ms_data);
+ igt_assert(wait_for_suspended());
+
+ memset(cpu_buf, 0, dst_size);
+ gem_read(drm_fd, handle, 0, cpu_buf, dst_size);
+ for (y = 0; y < 128; y++) {
+ for (x = 0; x < 128; x++) {
+ uint32_t px = cpu_buf[y * 128 + x];
+
+ if (y >= sq_y && y < (sq_y + sq_h) &&
+ x >= sq_x && x < (sq_x + sq_w))
+ igt_assert(px == color);
+ else
+ igt_assert(px == 0);
+ }
+ }
+
+ gem_close(drm_fd, handle);
+
+ free(cpu_buf);
+}
+
+/* Assuming execbuf already works, let's see what happens when we force many
+ * suspend/resume cycles with commands. */
+static void gem_execbuf_stress_subtest(void)
+{
+ int i;
+ int max = quick ? 5 : 50;
+ int batch_size = 4 * sizeof(uint32_t);
+ uint32_t batch_buf[batch_size];
+ uint32_t handle;
+ struct drm_i915_gem_execbuffer2 execbuf = {};
+ struct drm_i915_gem_exec_object2 objs[1] = {{}};
+
+ i = 0;
+ batch_buf[i++] = MI_NOOP;
+ batch_buf[i++] = MI_NOOP;
+ batch_buf[i++] = MI_BATCH_BUFFER_END;
+ batch_buf[i++] = MI_NOOP;
+ igt_assert(i * sizeof(uint32_t) == batch_size);
+
+ disable_all_screens(&ms_data);
+ igt_assert(wait_for_suspended());
+
+ handle = gem_create(drm_fd, batch_size);
+ gem_write(drm_fd, handle, 0, batch_buf, batch_size);
+
+ objs[0].handle = handle;
+
+ execbuf.buffers_ptr = (uint64_t) objs;
+ execbuf.buffer_count = 1;
+ execbuf.batch_len = batch_size;
+ execbuf.flags = I915_EXEC_RENDER;
+ i915_execbuffer2_set_context_id(execbuf, 0);
+
+ for (i = 0; i < max; i++) {
+ do_ioctl(drm_fd, DRM_IOCTL_I915_GEM_EXECBUFFER2, &execbuf);
+ igt_assert(wait_for_suspended());
+ }
+
+ gem_close(drm_fd, handle);
+}
+
+static void read_full_file(const char *name)
+{
+ int rc, fd;
+ char buf[128];
+
+ /* Make sure we're suspended! */
+ igt_assert(wait_for_suspended());
+
+ fd = open(name, O_RDONLY);
+ if (fd < 0)
+ return;
+
+ do {
+ rc = read(fd, buf, ARRAY_SIZE(buf));
+ } while (rc == ARRAY_SIZE(buf));
+
+ rc = close(fd);
+ igt_assert(rc == 0);
+}
+
+static void read_files_from_dir(const char *name, int level)
+{
+ DIR *dir;
+ struct dirent *dirent;
+ char *full_name;
+ int rc;
+
+ dir = opendir(name);
+ igt_assert(dir);
+
+ full_name = malloc(PATH_MAX);
+
+ igt_assert(level < 128);
+
+ while ((dirent = readdir(dir))) {
+ struct stat stat_buf;
+
+ if (strcmp(dirent->d_name, ".") == 0)
+ continue;
+ if (strcmp(dirent->d_name, "..") == 0)
+ continue;
+
+ snprintf(full_name, PATH_MAX, "%s/%s", name, dirent->d_name);
+
+ rc = lstat(full_name, &stat_buf);
+ igt_assert(rc == 0);
+
+ if (S_ISDIR(stat_buf.st_mode))
+ read_files_from_dir(full_name, level + 1);
+
+ if (S_ISREG(stat_buf.st_mode))
+ read_full_file(full_name);
+ }
+
+ free(full_name);
+ closedir(dir);
+}
+
+/* This test will probably pass, with a small chance of hanging the machine in
+ * case of bugs. Many of the bugs exercised by this patch just result in dmesg
+ * errors, so a "pass" here should be confirmed by a check on dmesg. */
+static void debugfs_read_subtest(void)
+{
+ const char *path = "/sys/kernel/debug/dri/0";
+ DIR *dir;
+
+ dir = opendir(path);
+ igt_require_f(dir, "Can't open the debugfs directory\n");
+ closedir(dir);
+
+ disable_all_screens(&ms_data);
+ igt_assert(wait_for_suspended());
+
+ read_files_from_dir(path, 0);
+}
+
+/* Read the comment on debugfs_read_subtest(). */
+static void sysfs_read_subtest(void)
+{
+ const char *path = "/sys/devices/pci0000:00/0000:00:02.0";
+ DIR *dir;
+
+ dir = opendir(path);
+ igt_require_f(dir, "Can't open the sysfs directory\n");
+ closedir(dir);
+
+ disable_all_screens(&ms_data);
+ igt_assert(wait_for_suspended());
+
+ read_files_from_dir(path, 0);
+}
+
+/* Make sure we don't suspend when we have the i915_forcewake_user file open. */
+static void debugfs_forcewake_user_subtest(void)
+{
+ int fd, rc;
+
+ igt_require(!(IS_GEN2(ms_data.devid) || IS_GEN3(ms_data.devid) ||
+ IS_GEN4(ms_data.devid) || IS_GEN5(ms_data.devid)));
+
+ disable_all_screens(&ms_data);
+ igt_assert(wait_for_suspended());
+
+ fd = open("/sys/kernel/debug/dri/0/i915_forcewake_user", O_RDONLY);
+ igt_require(fd);
+
+ igt_assert(!wait_for_suspended());
+
+ rc = close(fd);
+ igt_assert(rc == 0);
+
+ igt_assert(wait_for_suspended());
+}
+
+int main(int argc, char *argv[])
+{
+ /* The --quick option makes the stress tests not so stressful. */
+ if (argc > 1 && strcmp(argv[1], "--quick") == 0)
+ quick = true;
+
+ igt_subtest_init(argc, argv);
+
+ igt_fixture
+ setup_environment();
+
+ igt_subtest("basic")
+ basic_subtest();
+ igt_subtest("drm-resources")
+ drm_resources_subtest();
+ igt_subtest("i2c")
+ i2c_subtest();
+ igt_subtest("pc8")
+ pc8_subtest();
+ igt_subtest("gem-mmap")
+ gem_mmap_subtest();
+ igt_subtest("gem-pread")
+ gem_pread_subtest();
+ igt_subtest("gem-execbuf")
+ gem_execbuf_subtest();
+ igt_subtest("modeset-stress-lpsp")
+ modeset_stress(true);
+ igt_subtest("modeset-stress-non-lpsp")
+ modeset_stress(false);
+ igt_subtest("gem-execbuf-stress")
+ gem_execbuf_stress_subtest();
+ igt_subtest("debugfs-read")
+ debugfs_read_subtest();
+ igt_subtest("debugfs-forcewake-user")
+ debugfs_forcewake_user_subtest();
+ igt_subtest("sysfs-read")
+ sysfs_read_subtest();
+
+ igt_fixture
+ teardown_environment();
+
+ igt_exit();
+}
--
1.8.3.1
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply related [flat|nested] 25+ messages in thread
* Re: [RFC i-g-t] tests: add runtime_pm
2013-10-25 13:44 ` [RFC i-g-t] tests: add runtime_pm Paulo Zanoni
@ 2013-10-27 13:37 ` Daniel Vetter
2013-10-28 12:20 ` Paulo Zanoni
0 siblings, 1 reply; 25+ messages in thread
From: Daniel Vetter @ 2013-10-27 13:37 UTC (permalink / raw)
To: Paulo Zanoni; +Cc: intel-gfx, Paulo Zanoni
On Fri, Oct 25, 2013 at 11:44:05AM -0200, Paulo Zanoni wrote:
> From: Paulo Zanoni <paulo.r.zanoni@intel.com>
>
> This test is based on pc8.c. It copies most of the tests from pc8.c,
> but it depends on runtime PM status changes (parsed from sysfs)
> instead of PC8 residency changes (parsed from the MSR registers).
> There's also a test that checks for PC8 residency.
>
> For now, runtime PM and PC8 are different features, so having 2 test
> suites makes sense. In the future we'll merge both, so we'll only get
> PC8 when runtime PM is enabled, so we'll just kill pc8.c and keep
> using runtime_pm.c.
>
> Changes compared to pc8.c:
> - We now look at the runtime PM status instead of PC8 residencies
> - Added more GEM tests (mmap, pread, execbuf, stress tests)
> - Added LPSP and non-LPSP tests
> - Added tests fro sysfs and debugfs files
> - Added a test specifically for PC8
>
> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
Since the actual tests we're running are so similar I prefer if we merge
all the runtime pm tests in one file. It makes testcase maintaince (and
bufixing, that happens) much easier. I guess a struct per runtime pm
method (pc8, D3, ...) with a few vfuncs should get things going. The
overall test would loop over all the pm methods and try to set things up.
Then loop over all subtests and either skip them all (if that particular
runtime pm method isn't supported) or just run them.
We've had a few other case of massive copy&pasting in i-g-t and in the
past few months I've merged most of them back again.
-Daniel
> ---
> tests/.gitignore | 1 +
> tests/Makefile.am | 1 +
> tests/runtime_pm.c | 1262 ++++++++++++++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 1264 insertions(+)
> create mode 100644 tests/runtime_pm.c
>
> diff --git a/tests/.gitignore b/tests/.gitignore
> index 1c97a04..cda6d09 100644
> --- a/tests/.gitignore
> +++ b/tests/.gitignore
> @@ -105,6 +105,7 @@ prime_nv_pcopy
> prime_nv_test
> prime_self_import
> prime_udl
> +runtime_pm
> sysfs_rc6_residency
> sysfs_rps
> testdisplay
> diff --git a/tests/Makefile.am b/tests/Makefile.am
> index f6ba368..234aaf5 100644
> --- a/tests/Makefile.am
> +++ b/tests/Makefile.am
> @@ -59,6 +59,7 @@ TESTS_progs_M = \
> $(NOUVEAU_TESTS_M) \
> pc8 \
> prime_self_import \
> + runtime_pm \
> $(NULL)
>
> TESTS_progs = \
> diff --git a/tests/runtime_pm.c b/tests/runtime_pm.c
> new file mode 100644
> index 0000000..425e743
> --- /dev/null
> +++ b/tests/runtime_pm.c
> @@ -0,0 +1,1262 @@
> +/*
> + * Copyright © 2013 Intel Corporation
> + *
> + * Permission is hereby granted, free of charge, to any person obtaining a
> + * copy of this software and associated documentation files (the "Software"),
> + * to deal in the Software without restriction, including without limitation
> + * the rights to use, copy, modify, merge, publish, distribute, sublicense,
> + * and/or sell copies of the Software, and to permit persons to whom the
> + * Software is furnished to do so, subject to the following conditions:
> + *
> + * The above copyright notice and this permission notice (including the next
> + * paragraph) shall be included in all copies or substantial portions of the
> + * Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
> + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
> + * IN THE SOFTWARE.
> + *
> + * Authors:
> + * Paulo Zanoni <paulo.r.zanoni@intel.com>
> + *
> + */
> +
> +#include <string.h>
> +#include <stdio.h>
> +#include <stdint.h>
> +#include <stdbool.h>
> +#include <stdlib.h>
> +
> +#include <dirent.h>
> +#include <fcntl.h>
> +#include <unistd.h>
> +#include <sys/ioctl.h>
> +#include <sys/mman.h>
> +#include <sys/types.h>
> +#include <sys/stat.h>
> +#include <linux/i2c.h>
> +#include <linux/i2c-dev.h>
> +
> +#include "drmtest.h"
> +#include "intel_gpu_tools.h"
> +#include "igt_display.h"
> +
> +
> +#define MSR_PC8_RES 0x630
> +#define MSR_PC9_RES 0x631
> +#define MSR_PC10_RES 0x632
> +
> +#define POWER_DIR "/sys/devices/pci0000:00/0000:00:02.0/power"
> +
> +#define MAX_CONNECTORS 32
> +#define MAX_ENCODERS 32
> +#define MAX_CRTCS 16
> +
> +
> +enum runtime_pm_status {
> + RUNTIME_PM_STATUS_ACTIVE,
> + RUNTIME_PM_STATUS_SUSPENDED,
> + RUNTIME_PM_STATUS_SUSPENDING,
> + RUNTIME_PM_STATUS_UNKNOWN,
> +};
> +
> +enum screen_type {
> + SCREEN_TYPE_LPSP,
> + SCREEN_TYPE_NON_LPSP,
> + SCREEN_TYPE_ANY,
> +};
> +
> +/* Stuff used when creating FBs and mode setting. */
> +struct mode_set_data {
> + drmModeResPtr res;
> + drmModeConnectorPtr connectors[MAX_CONNECTORS];
> + drmModePropertyBlobPtr edids[MAX_CONNECTORS];
> +
> + uint32_t devid;
> +};
> +
> +/* Stuff we query at different times so we can compare. */
> +struct compare_data {
> + drmModeResPtr res;
> + drmModeEncoderPtr encoders[MAX_ENCODERS];
> + drmModeConnectorPtr connectors[MAX_CONNECTORS];
> + drmModeCrtcPtr crtcs[MAX_CRTCS];
> + drmModePropertyBlobPtr edids[MAX_CONNECTORS];
> +};
> +
> +
> +int drm_fd;
> +int status_fd;
> +int msr_fd = 0;
> +struct mode_set_data ms_data;
> +bool quick = false;
> +
> +
> +static uint64_t get_residency(uint32_t type)
> +{
> + int rc;
> + uint64_t ret;
> +
> + rc = pread(msr_fd, &ret, sizeof(uint64_t), type);
> + igt_assert(rc == sizeof(ret));
> +
> + return ret;
> +}
> +
> +static bool pc8_plus_residency_changed(unsigned int timeout_sec)
> +{
> + unsigned int i;
> + uint64_t res_pc8, res_pc9, res_pc10;
> + int to_sleep = 100 * 1000;
> +
> + res_pc8 = get_residency(MSR_PC8_RES);
> + res_pc9 = get_residency(MSR_PC9_RES);
> + res_pc10 = get_residency(MSR_PC10_RES);
> +
> + for (i = 0; i < timeout_sec * 1000 * 1000; i += to_sleep) {
> + if (res_pc8 != get_residency(MSR_PC8_RES) ||
> + res_pc9 != get_residency(MSR_PC9_RES) ||
> + res_pc10 != get_residency(MSR_PC10_RES)) {
> + return true;
> + }
> + usleep(to_sleep);
> + }
> +
> + return false;
> +}
> +
> +/* Checks not only if PC8+ is allowed, but also if we're reaching it.
> + * We call this when we expect this function to return quickly since PC8 is
> + * actually enabled, so the 30s timeout we use shouldn't matter. */
> +static bool pc8_plus_enabled(void)
> +{
> + return pc8_plus_residency_changed(30);
> +}
> +
> +/* We call this when we expect PC8+ to be actually disabled, so we should not
> + * return until the 5s timeout expires. In other words: in the "happy case",
> + * every time we call this function, it will sleep for 5 seconds. */
> +static bool pc8_plus_disabled(void)
> +{
> + return !pc8_plus_residency_changed(5);
> +}
> +
> +/* If the read fails, then the machine doesn't support PC8+ residencies. */
> +static bool supports_pc8_plus_residencies(void)
> +{
> + int rc;
> + uint64_t val;
> +
> + rc = pread(msr_fd, &val, sizeof(uint64_t), MSR_PC8_RES);
> + if (rc != sizeof(val))
> + return false;
> + rc = pread(msr_fd, &val, sizeof(uint64_t), MSR_PC9_RES);
> + if (rc != sizeof(val))
> + return false;
> + rc = pread(msr_fd, &val, sizeof(uint64_t), MSR_PC10_RES);
> + if (rc != sizeof(val))
> + return false;
> +
> + return true;
> +}
> +
> +static enum runtime_pm_status get_runtime_pm_status(void)
> +{
> + ssize_t n_read;
> + char buf[32];
> +
> + lseek(status_fd, 0, SEEK_SET);
> + n_read = read(status_fd, buf, ARRAY_SIZE(buf));
> + igt_assert(n_read >= 0);
> + buf[n_read] = '\0';
> +
> + if (strncmp(buf, "suspended\n", n_read) == 0)
> + return RUNTIME_PM_STATUS_SUSPENDED;
> + else if (strncmp(buf, "active\n", n_read) == 0)
> + return RUNTIME_PM_STATUS_ACTIVE;
> + else if (strncmp(buf, "suspending\n", n_read) == 0)
> + return RUNTIME_PM_STATUS_SUSPENDING;
> +
> + igt_assert_f(false, "Unknown status %s\n", buf);
> + return RUNTIME_PM_STATUS_UNKNOWN;
> +}
> +
> +static bool wait_for_status(enum runtime_pm_status status)
> +{
> + int i;
> + int hundred_ms = 100 * 1000, ten_s = 10 * 1000 * 1000;
> +
> + for (i = 0; i < ten_s; i += hundred_ms) {
> + if (get_runtime_pm_status() == status)
> + return true;
> +
> + usleep(hundred_ms);
> + }
> +
> + return false;
> +}
> +
> +static bool wait_for_suspended(void)
> +{
> + return wait_for_status(RUNTIME_PM_STATUS_SUSPENDED);
> +}
> +
> +static bool wait_for_active(void)
> +{
> + return wait_for_status(RUNTIME_PM_STATUS_ACTIVE);
> +}
> +
> +static void disable_all_screens(struct mode_set_data *data)
> +{
> + int i, rc;
> +
> + for (i = 0; i < data->res->count_crtcs; i++) {
> + rc = drmModeSetCrtc(drm_fd, data->res->crtcs[i], -1, 0, 0,
> + NULL, 0, NULL);
> + igt_assert(rc == 0);
> + }
> +}
> +
> +static uint32_t create_fb(struct mode_set_data *data, int width, int height)
> +{
> + struct kmstest_fb fb;
> + cairo_t *cr;
> + uint32_t buffer_id;
> +
> + buffer_id = kmstest_create_fb(drm_fd, width, height, 32, 24, false,
> + &fb);
> + cr = kmstest_get_cairo_ctx(drm_fd, &fb);
> + kmstest_paint_test_pattern(cr, width, height);
> + return buffer_id;
> +}
> +
> +static bool enable_one_screen_with_type(struct mode_set_data *data,
> + enum screen_type type)
> +{
> + uint32_t crtc_id = 0, buffer_id = 0, connector_id = 0;
> + drmModeModeInfoPtr mode = NULL;
> + int i, rc;
> +
> + for (i = 0; i < data->res->count_connectors; i++) {
> + drmModeConnectorPtr c = data->connectors[i];
> +
> + if (type == SCREEN_TYPE_LPSP &&
> + c->connector_type != DRM_MODE_CONNECTOR_eDP)
> + continue;
> +
> + if (type == SCREEN_TYPE_NON_LPSP &&
> + c->connector_type == DRM_MODE_CONNECTOR_eDP)
> + continue;
> +
> + if (c->connection == DRM_MODE_CONNECTED && c->count_modes) {
> + connector_id = c->connector_id;
> + mode = &c->modes[0];
> + break;
> + }
> + }
> +
> + if (connector_id == 0)
> + return false;
> +
> + crtc_id = data->res->crtcs[0];
> + buffer_id = create_fb(data, mode->hdisplay, mode->vdisplay);
> +
> + igt_assert(crtc_id);
> + igt_assert(buffer_id);
> + igt_assert(connector_id);
> + igt_assert(mode);
> +
> + rc = drmModeSetCrtc(drm_fd, crtc_id, buffer_id, 0, 0, &connector_id,
> + 1, mode);
> + igt_assert(rc == 0);
> +
> + return true;
> +}
> +
> +static void enable_one_screen(struct mode_set_data *data)
> +{
> + igt_assert(enable_one_screen_with_type(data, SCREEN_TYPE_ANY));
> +}
> +
> +/* We could check the checksum too, but just the header is probably enough. */
> +static bool edid_is_valid(const unsigned char *edid)
> +{
> + char edid_header[] = {
> + 0x0, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0x0,
> + };
> +
> + return (memcmp(edid, edid_header, sizeof(edid_header)) == 0);
> +}
> +
> +static int count_drm_valid_edids(struct mode_set_data *data)
> +{
> + int i, ret = 0;
> +
> + for (i = 0; i < data->res->count_connectors; i++)
> + if (data->edids[i] && edid_is_valid(data->edids[i]->data))
> + ret++;
> + return ret;
> +}
> +
> +static bool i2c_edid_is_valid(int fd)
> +{
> + int rc;
> + unsigned char edid[128] = {};
> + struct i2c_msg msgs[] = {
> + { /* Start at 0. */
> + .addr = 0x50,
> + .flags = 0,
> + .len = 1,
> + .buf = edid,
> + }, { /* Now read the EDID. */
> + .addr = 0x50,
> + .flags = I2C_M_RD,
> + .len = 128,
> + .buf = edid,
> + }
> + };
> + struct i2c_rdwr_ioctl_data msgset = {
> + .msgs = msgs,
> + .nmsgs = 2,
> + };
> +
> + rc = ioctl(fd, I2C_RDWR, &msgset);
> + return (rc >= 0) ? edid_is_valid(edid) : false;
> +}
> +
> +static int count_i2c_valid_edids(void)
> +{
> + int fd, ret = 0;
> + DIR *dir;
> +
> + struct dirent *dirent;
> + char full_name[32];
> +
> + dir = opendir("/dev/");
> + igt_assert(dir);
> +
> + while ((dirent = readdir(dir))) {
> + if (strncmp(dirent->d_name, "i2c-", 4) == 0) {
> + snprintf(full_name, 32, "/dev/%s", dirent->d_name);
> + fd = open(full_name, O_RDWR);
> + igt_assert(fd != -1);
> + if (i2c_edid_is_valid(fd))
> + ret++;
> + close(fd);
> + }
> + }
> +
> + closedir(dir);
> +
> + return ret;
> +}
> +
> +static void test_i2c(struct mode_set_data *data)
> +{
> + int i2c_edids = count_i2c_valid_edids();
> + int drm_edids = count_drm_valid_edids(data);
> +
> + igt_assert(i2c_edids == drm_edids);
> +}
> +
> +static void i2c_subtest_check_environment(void)
> +{
> + int i2c_dev_files = 0;
> + DIR *dev_dir;
> + struct dirent *dirent;
> +
> + /* Make sure the /dev/i2c-* files exist. */
> + dev_dir = opendir("/dev");
> + igt_assert(dev_dir);
> + while ((dirent = readdir(dev_dir))) {
> + if (strncmp(dirent->d_name, "i2c-", 4) == 0)
> + i2c_dev_files++;
> + }
> + closedir(dev_dir);
> + igt_require(i2c_dev_files);
> +}
> +
> +
> +#define COMPARE(d1, d2, data) igt_assert(d1->data == d2->data)
> +#define COMPARE_ARRAY(d1, d2, size, data) do { \
> + for (i = 0; i < size; i++) \
> + igt_assert(d1->data[i] == d2->data[i]); \
> +} while (0)
> +
> +static void assert_drm_resources_equal(struct compare_data *d1,
> + struct compare_data *d2)
> +{
> + COMPARE(d1, d2, res->count_connectors);
> + COMPARE(d1, d2, res->count_encoders);
> + COMPARE(d1, d2, res->count_crtcs);
> + COMPARE(d1, d2, res->min_width);
> + COMPARE(d1, d2, res->max_width);
> + COMPARE(d1, d2, res->min_height);
> + COMPARE(d1, d2, res->max_height);
> +}
> +
> +static void assert_modes_equal(drmModeModeInfoPtr m1, drmModeModeInfoPtr m2)
> +{
> + COMPARE(m1, m2, clock);
> + COMPARE(m1, m2, hdisplay);
> + COMPARE(m1, m2, hsync_start);
> + COMPARE(m1, m2, hsync_end);
> + COMPARE(m1, m2, htotal);
> + COMPARE(m1, m2, hskew);
> + COMPARE(m1, m2, vdisplay);
> + COMPARE(m1, m2, vsync_start);
> + COMPARE(m1, m2, vsync_end);
> + COMPARE(m1, m2, vtotal);
> + COMPARE(m1, m2, vscan);
> + COMPARE(m1, m2, vrefresh);
> + COMPARE(m1, m2, flags);
> + COMPARE(m1, m2, type);
> + igt_assert(strcmp(m1->name, m2->name) == 0);
> +}
> +
> +static void assert_drm_connectors_equal(drmModeConnectorPtr c1,
> + drmModeConnectorPtr c2)
> +{
> + int i;
> +
> + COMPARE(c1, c2, connector_id);
> + COMPARE(c1, c2, connector_type);
> + COMPARE(c1, c2, connector_type_id);
> + COMPARE(c1, c2, mmWidth);
> + COMPARE(c1, c2, mmHeight);
> + COMPARE(c1, c2, count_modes);
> + COMPARE(c1, c2, count_props);
> + COMPARE(c1, c2, count_encoders);
> + COMPARE_ARRAY(c1, c2, c1->count_props, props);
> + COMPARE_ARRAY(c1, c2, c1->count_encoders, encoders);
> +
> + for (i = 0; i < c1->count_modes; i++)
> + assert_modes_equal(&c1->modes[0], &c2->modes[0]);
> +}
> +
> +static void assert_drm_encoders_equal(drmModeEncoderPtr e1,
> + drmModeEncoderPtr e2)
> +{
> + COMPARE(e1, e2, encoder_id);
> + COMPARE(e1, e2, encoder_type);
> + COMPARE(e1, e2, possible_crtcs);
> + COMPARE(e1, e2, possible_clones);
> +}
> +
> +static void assert_drm_crtcs_equal(drmModeCrtcPtr c1, drmModeCrtcPtr c2)
> +{
> + COMPARE(c1, c2, crtc_id);
> +}
> +
> +static void assert_drm_edids_equal(drmModePropertyBlobPtr e1,
> + drmModePropertyBlobPtr e2)
> +{
> + if (!e1 && !e2)
> + return;
> + igt_assert(e1 && e2);
> +
> + COMPARE(e1, e2, id);
> + COMPARE(e1, e2, length);
> +
> + igt_assert(memcmp(e1->data, e2->data, e1->length) == 0);
> +}
> +
> +static void assert_drm_infos_equal(struct compare_data *d1,
> + struct compare_data *d2)
> +{
> + int i;
> +
> + assert_drm_resources_equal(d1, d2);
> +
> + for (i = 0; i < d1->res->count_connectors; i++) {
> + assert_drm_connectors_equal(d1->connectors[i],
> + d2->connectors[i]);
> + assert_drm_edids_equal(d1->edids[i], d2->edids[i]);
> + }
> +
> + for (i = 0; i < d1->res->count_encoders; i++)
> + assert_drm_encoders_equal(d1->encoders[i], d2->encoders[i]);
> +
> + for (i = 0; i < d1->res->count_crtcs; i++)
> + assert_drm_crtcs_equal(d1->crtcs[i], d2->crtcs[i]);
> +}
> +
> +static drmModePropertyBlobPtr get_connector_edid(drmModeConnectorPtr connector,
> + int index)
> +{
> + unsigned int i;
> + drmModeObjectPropertiesPtr props;
> + drmModePropertyBlobPtr ret = NULL;
> +
> + props = drmModeObjectGetProperties(drm_fd, connector->connector_id,
> + DRM_MODE_OBJECT_CONNECTOR);
> +
> + for (i = 0; i < props->count_props; i++) {
> + drmModePropertyPtr prop = drmModeGetProperty(drm_fd,
> + props->props[i]);
> +
> + if (strcmp(prop->name, "EDID") == 0) {
> + igt_assert(prop->flags & DRM_MODE_PROP_BLOB);
> + igt_assert(prop->count_blobs == 0);
> + ret = drmModeGetPropertyBlob(drm_fd,
> + props->prop_values[i]);
> + }
> +
> + drmModeFreeProperty(prop);
> + }
> +
> + drmModeFreeObjectProperties(props);
> + return ret;
> +}
> +
> +
> +static void get_drm_info(struct compare_data *data)
> +{
> + int i;
> +
> + data->res = drmModeGetResources(drm_fd);
> + igt_assert(data->res);
> +
> + igt_assert(data->res->count_connectors <= MAX_CONNECTORS);
> + igt_assert(data->res->count_encoders <= MAX_ENCODERS);
> + igt_assert(data->res->count_crtcs <= MAX_CRTCS);
> +
> + for (i = 0; i < data->res->count_connectors; i++) {
> + data->connectors[i] = drmModeGetConnector(drm_fd,
> + data->res->connectors[i]);
> + data->edids[i] = get_connector_edid(data->connectors[i], i);
> + }
> + for (i = 0; i < data->res->count_encoders; i++)
> + data->encoders[i] = drmModeGetEncoder(drm_fd,
> + data->res->encoders[i]);
> + for (i = 0; i < data->res->count_crtcs; i++)
> + data->crtcs[i] = drmModeGetCrtc(drm_fd, data->res->crtcs[i]);
> +}
> +
> +static void free_drm_info(struct compare_data *data)
> +{
> + int i;
> +
> + for (i = 0; i < data->res->count_connectors; i++) {
> + drmModeFreeConnector(data->connectors[i]);
> + drmModeFreePropertyBlob(data->edids[i]);
> + }
> + for (i = 0; i < data->res->count_encoders; i++)
> + drmModeFreeEncoder(data->encoders[i]);
> + for (i = 0; i < data->res->count_crtcs; i++)
> + drmModeFreeCrtc(data->crtcs[i]);
> +
> + drmModeFreeResources(data->res);
> +}
> +
> +static void init_mode_set_data(struct mode_set_data *data)
> +{
> + int i;
> +
> + data->res = drmModeGetResources(drm_fd);
> + igt_assert(data->res);
> + igt_assert(data->res->count_connectors <= MAX_CONNECTORS);
> +
> + for (i = 0; i < data->res->count_connectors; i++) {
> + data->connectors[i] = drmModeGetConnector(drm_fd,
> + data->res->connectors[i]);
> + data->edids[i] = get_connector_edid(data->connectors[i], i);
> + }
> +
> + data->devid = intel_get_drm_devid(drm_fd);
> +
> + do_or_die(igt_set_vt_graphics_mode());
> +}
> +
> +static void fini_mode_set_data(struct mode_set_data *data)
> +{
> + int i;
> +
> + for (i = 0; i < data->res->count_connectors; i++) {
> + drmModeFreeConnector(data->connectors[i]);
> + drmModeFreePropertyBlob(data->edids[i]);
> + }
> + drmModeFreeResources(data->res);
> +}
> +
> +static void pc8_subtest_check_environment(void)
> +{
> + igt_require_f(IS_HASWELL(ms_data.devid),
> + "PC8+ feature only supported on Haswell.\n");
> +
> + /* If this fails, you don't have the Kernel module loaded. */
> + msr_fd = open("/dev/cpu/0/msr", O_RDONLY);
> + igt_assert_f(msr_fd >= 0, "Can't open /dev/cpu/0/msr.\n");
> +
> + /* Non-ULT machines don't support PC8+. */
> + igt_require_f(supports_pc8_plus_residencies(),
> + "Machine doesn't support PC8+ residencies.\n");
> +}
> +
> +static void assert_runtime_pm_is_enabled(void)
> +{
> + int fd;
> + ssize_t n_read;
> + char buf[6];
> + int rc;
> +
> + fd = open(POWER_DIR "/control", O_RDONLY);
> + igt_assert_f(fd >= 0, "Can't open " POWER_DIR "/control\n");
> +
> + n_read = read(fd, buf, ARRAY_SIZE(buf));
> + igt_assert(n_read >= 0);
> + igt_require_f(strncmp(buf, "auto\n", n_read) == 0,
> + "Runtime PM not enabled or supported on this platform\n");
> +
> + rc = close(fd);
> + igt_assert(rc == 0);
> +}
> +
> +/* We set 1ms autosuspend delay here so things start/stop fast and we don't
> + * waste time waiting for runtime PM to kick in. We don't care about restoring
> + * the original value at the end of the test. */
> +static void set_autosuspend_delay(void)
> +{
> + int fd;
> + ssize_t n_write;
> + int rc;
> +
> + fd = open(POWER_DIR "/autosuspend_delay_ms", O_WRONLY);
> + igt_assert_f(fd >= 0,
> + "Can't open " POWER_DIR "/autosuspend_delay_ms\n");
> +
> + n_write = write(fd, "1\n", 2);
> + igt_assert(n_write == 2);
> +
> + rc = close(fd);
> + igt_assert(rc == 0);
> +
> +}
> +
> +static void setup_environment(void)
> +{
> +
> + drm_fd = drm_open_any();
> + igt_assert(drm_fd >= 0);
> +
> + init_mode_set_data(&ms_data);
> +
> + assert_runtime_pm_is_enabled(),
> +
> + status_fd = open(POWER_DIR "/runtime_status", O_RDONLY);
> + igt_assert_f(status_fd >= 0,
> + "Can't open " POWER_DIR "/runtime_status\n");
> +
> + set_autosuspend_delay();
> +}
> +
> +static void teardown_environment(void)
> +{
> + int rc;
> +
> + fini_mode_set_data(&ms_data);
> +
> + drmClose(drm_fd);
> +
> + rc = close(status_fd);
> + igt_assert(rc == 0);
> +
> + if (msr_fd) {
> + rc = close(msr_fd);
> + igt_assert(rc == 0);
> + }
> +}
> +
> +/* Sanity test: make sure it suspends when all screens are disabled and it
> + * resumes when we enable a screen. */
> +static void basic_subtest(void)
> +{
> + disable_all_screens(&ms_data);
> + igt_assert_f(wait_for_suspended(),
> + "Graphics device is not suspending, please check its "
> + "runtime PM configuration.\n");
> +
> + enable_one_screen(&ms_data);
> + igt_assert_f(wait_for_active(),
> + "Graphics device is still suspended even with a screen "
> + "enabled. Something is really wrong.\n");
> +}
> +
> +/* Test if the DRM resources reported by the IOCTLs are still the same. This
> + * ensures we still see the monitors with the same eyes. We get the EDIDs and
> + * compare them, which ensures we use DP AUX or GMBUS depending on what's
> + * connected. */
> +static void drm_resources_subtest(void)
> +{
> + struct compare_data pre_suspend, during_suspend, post_suspend;
> +
> + enable_one_screen(&ms_data);
> + igt_assert(wait_for_active());
> + get_drm_info(&pre_suspend);
> + igt_assert(wait_for_active());
> +
> + disable_all_screens(&ms_data);
> + igt_assert(wait_for_suspended());
> + get_drm_info(&during_suspend);
> + igt_assert(wait_for_suspended());
> +
> + enable_one_screen(&ms_data);
> + igt_assert(wait_for_active());
> + get_drm_info(&post_suspend);
> + igt_assert(wait_for_active());
> +
> + assert_drm_infos_equal(&pre_suspend, &during_suspend);
> + assert_drm_infos_equal(&pre_suspend, &post_suspend);
> +
> + free_drm_info(&pre_suspend);
> + free_drm_info(&during_suspend);
> + free_drm_info(&post_suspend);
> +}
> +
> +/* Try to use raw I2C, which also needs interrupts. */
> +static void i2c_subtest(void)
> +{
> + i2c_subtest_check_environment();
> +
> + enable_one_screen(&ms_data);
> + igt_assert(wait_for_active());
> +
> + disable_all_screens(&ms_data);
> + igt_assert(wait_for_suspended());
> + test_i2c(&ms_data);
> + igt_assert(wait_for_suspended());
> +
> + enable_one_screen(&ms_data);
> + igt_assert(wait_for_active());
> +}
> +
> +/* The LPSP version tests eDP, which doesn't require the power well. The
> + * non-LPSP version will make sure we toggle the power well on/off every time.
> + * We SKIP in case we can't find the appropriate type of output connected, but
> + * that doesn't test as much as we could. */
> +static void modeset_stress(bool lpsp)
> +{
> + int i, max;
> + enum screen_type type;
> +
> + max = quick ? 5 : 50;
> + type = lpsp ? SCREEN_TYPE_LPSP : SCREEN_TYPE_NON_LPSP;
> +
> + for (i = 0; i < max; i++) {
> + disable_all_screens(&ms_data);
> + igt_assert(wait_for_suspended());
> +
> + /* If we skip this line it's because the type of screen we want
> + * is not connected. */
> + igt_require(enable_one_screen_with_type(&ms_data, type));
> + igt_assert(wait_for_active());
> + }
> +}
> +
> +/* Make sure we get to PC8 or deeper states when we're suspended. Notice that
> + * this actually depends on a properly configured machine, where the runtime PM
> + * polices for _all_ devices (not only graphics) are configured correctly. If
> + * you use "powertop" and switch everything to "good", there's a good chance
> + * you'll be fine here. */
> +static void pc8_subtest(void)
> +{
> + pc8_subtest_check_environment();
> +
> + disable_all_screens(&ms_data);
> + igt_assert(wait_for_suspended());
> + igt_assert(pc8_plus_enabled());
> +
> + enable_one_screen(&ms_data);
> + igt_assert(wait_for_active());
> + igt_assert(pc8_plus_disabled());
> +}
> +
> +static void *mmap_gem_buf(uint32_t handle, int size)
> +{
> + struct drm_i915_gem_mmap gem_mmap;
> +
> + gem_mmap.handle = handle;
> + gem_mmap.offset = 0;
> + gem_mmap.size = size;
> + do_ioctl(drm_fd, DRM_IOCTL_I915_GEM_MMAP, &gem_mmap);
> + igt_assert(gem_mmap.addr_ptr);
> +
> + return (void *) gem_mmap.addr_ptr;
> +}
> +
> +/* Make sure even mmap still works when we're in D3. */
> +static void gem_mmap_subtest(void)
> +{
> + int i;
> + uint32_t handle;
> + int buf_size = 8192;
> + uint8_t *gem_buf;
> +
> + /* Create, map and set data while the device is active. */
> + enable_one_screen(&ms_data);
> + igt_assert(wait_for_active());
> +
> + handle = gem_create(drm_fd, buf_size);
> +
> + gem_buf = mmap_gem_buf(handle, buf_size);
> +
> + for (i = 0; i < buf_size; i++)
> + gem_buf[i] = i & 0xFF;
> +
> + for (i = 0; i < buf_size; i++)
> + igt_assert(gem_buf[i] == (i & 0xFF));
> +
> + /* Now suspend, read and modify. */
> + disable_all_screens(&ms_data);
> + igt_assert(wait_for_suspended());
> +
> + for (i = 0; i < buf_size; i++)
> + igt_assert(gem_buf[i] == (i & 0xFF));
> + igt_assert(wait_for_suspended());
> +
> + for (i = 0; i < buf_size; i++)
> + gem_buf[i] = (~i & 0xFF);
> + igt_assert(wait_for_suspended());
> +
> + /* Now resume and see if it's still there. */
> + enable_one_screen(&ms_data);
> + igt_assert(wait_for_active());
> + for (i = 0; i < buf_size; i++)
> + igt_assert(gem_buf[i] == (~i & 0xFF));
> +
> + munmap(gem_buf, buf_size);
> +
> + gem_close(drm_fd, handle);
> +}
> +
> +/* Make sure pread() and pwrite() still work when we're in D3. */
> +static void gem_pread_subtest(void)
> +{
> + int i;
> + uint32_t handle;
> + int buf_size = 8192;
> + uint8_t *cpu_buf, *read_buf;
> +
> + cpu_buf = malloc(buf_size);
> + read_buf = malloc(buf_size);
> + igt_assert(cpu_buf);
> + igt_assert(read_buf);
> + memset(cpu_buf, 0, buf_size);
> + memset(read_buf, 0, buf_size);
> +
> + /* Create and set data while the device is active. */
> + enable_one_screen(&ms_data);
> + igt_assert(wait_for_active());
> +
> + handle = gem_create(drm_fd, buf_size);
> +
> + for (i = 0; i < buf_size; i++)
> + cpu_buf[i] = i & 0xFF;
> +
> + gem_write(drm_fd, handle, 0, cpu_buf, buf_size);
> +
> + gem_read(drm_fd, handle, 0, read_buf, buf_size);
> +
> + for (i = 0; i < buf_size; i++)
> + igt_assert(cpu_buf[i] == read_buf[i]);
> +
> + /* Now suspend, read and modify. */
> + disable_all_screens(&ms_data);
> + igt_assert(wait_for_suspended());
> +
> + memset(read_buf, 0, buf_size);
> + gem_read(drm_fd, handle, 0, read_buf, buf_size);
> +
> + for (i = 0; i < buf_size; i++)
> + igt_assert(cpu_buf[i] == read_buf[i]);
> + igt_assert(wait_for_suspended());
> +
> + for (i = 0; i < buf_size; i++)
> + cpu_buf[i] = (~i & 0xFF);
> + gem_write(drm_fd, handle, 0, cpu_buf, buf_size);
> + igt_assert(wait_for_suspended());
> +
> + /* Now resume and see if it's still there. */
> + enable_one_screen(&ms_data);
> + igt_assert(wait_for_active());
> +
> + memset(read_buf, 0, buf_size);
> + gem_read(drm_fd, handle, 0, read_buf, buf_size);
> +
> + for (i = 0; i < buf_size; i++)
> + igt_assert(cpu_buf[i] == read_buf[i]);
> +
> + gem_close(drm_fd, handle);
> +
> + free(cpu_buf);
> + free(read_buf);
> +}
> +
> +/* Paints a square of color $color, size $width x $height, at position $x x $y
> + * of $dst_handle, which contains pitch $pitch. */
> +static void submit_blt_cmd(uint32_t dst_handle, uint32_t x, uint32_t y,
> + uint32_t width, uint32_t height, uint32_t pitch,
> + uint32_t color, uint32_t *presumed_dst_offset)
> +{
> + int i, reloc_pos;
> + int bpp = 4;
> + uint32_t batch_handle;
> + int batch_size = 8 * sizeof(uint32_t);
> + uint32_t batch_buf[batch_size];
> + uint32_t offset_in_dst = (pitch * y) + (x * bpp);
> + struct drm_i915_gem_execbuffer2 execbuf = {};
> + struct drm_i915_gem_exec_object2 objs[2] = {{}, {}};
> + struct drm_i915_gem_relocation_entry relocs[1] = {{}};
> + struct drm_i915_gem_wait gem_wait;
> +
> + i = 0;
> +
> + batch_buf[i++] = COLOR_BLT_CMD | COLOR_BLT_WRITE_ALPHA |
> + COLOR_BLT_WRITE_RGB;
> + batch_buf[i++] = (3 << 24) | (0xF0 << 16) | pitch;
> + batch_buf[i++] = (height << 16) | width * bpp;
> + reloc_pos = i;
> + batch_buf[i++] = *presumed_dst_offset + offset_in_dst;
> + batch_buf[i++] = color;
> +
> + batch_buf[i++] = MI_NOOP;
> + batch_buf[i++] = MI_BATCH_BUFFER_END;
> + batch_buf[i++] = MI_NOOP;
> +
> + igt_assert(i * sizeof(uint32_t) == batch_size);
> +
> + batch_handle = gem_create(drm_fd, batch_size);
> + gem_write(drm_fd, batch_handle, 0, batch_buf, batch_size);
> +
> + relocs[0].target_handle = dst_handle;
> + relocs[0].delta = offset_in_dst;
> + relocs[0].offset = reloc_pos * sizeof(uint32_t);
> + relocs[0].presumed_offset = *presumed_dst_offset;
> + relocs[0].read_domains = 0;
> + relocs[0].write_domain = I915_GEM_DOMAIN_RENDER;
> +
> + objs[0].handle = dst_handle;
> + objs[0].alignment = 64;
> +
> + objs[1].handle = batch_handle;
> + objs[1].relocation_count = 1;
> + objs[1].relocs_ptr = (uint64_t) relocs;
> +
> + execbuf.buffers_ptr = (uint64_t) objs;
> + execbuf.buffer_count = 2;
> + execbuf.batch_len = batch_size;
> + execbuf.flags = I915_EXEC_BLT;
> + i915_execbuffer2_set_context_id(execbuf, 0);
> +
> + do_ioctl(drm_fd, DRM_IOCTL_I915_GEM_EXECBUFFER2, &execbuf);
> +
> + *presumed_dst_offset = relocs[0].presumed_offset;
> +
> + gem_wait.flags = 0;
> + gem_wait.timeout_ns = 10000000000LL; /* 10s */
> +
> + gem_wait.bo_handle = batch_handle;
> + do_ioctl(drm_fd, DRM_IOCTL_I915_GEM_WAIT, &gem_wait);
> +
> + gem_wait.bo_handle = dst_handle;
> + do_ioctl(drm_fd, DRM_IOCTL_I915_GEM_WAIT, &gem_wait);
> +
> + gem_close(drm_fd, batch_handle);
> +}
> +
> +/* Make sure we can submit a batch buffer and verify its result. */
> +static void gem_execbuf_subtest(void)
> +{
> + int x, y;
> + uint32_t handle;
> + int bpp = 4;
> + int pitch = 128 * bpp;
> + int dst_size = 128 * 128 * bpp; /* 128x128 square */
> + uint32_t *cpu_buf;
> + uint32_t presumed_offset = 0;
> + int sq_x = 5, sq_y = 10, sq_w = 15, sq_h = 20;
> + uint32_t color;
> +
> + /* Create and set data while the device is active. */
> + enable_one_screen(&ms_data);
> + igt_assert(wait_for_active());
> +
> + handle = gem_create(drm_fd, dst_size);
> +
> + cpu_buf = malloc(dst_size);
> + igt_assert(cpu_buf);
> + memset(cpu_buf, 0, dst_size);
> + gem_write(drm_fd, handle, 0, cpu_buf, dst_size);
> +
> + /* Now suspend and try it. */
> + disable_all_screens(&ms_data);
> + igt_assert(wait_for_suspended());
> +
> + color = 0x12345678;
> + submit_blt_cmd(handle, sq_x, sq_y, sq_w, sq_h, pitch, color,
> + &presumed_offset);
> + igt_assert(wait_for_suspended());
> +
> + gem_read(drm_fd, handle, 0, cpu_buf, dst_size);
> + igt_assert(wait_for_suspended());
> + for (y = 0; y < 128; y++) {
> + for (x = 0; x < 128; x++) {
> + uint32_t px = cpu_buf[y * 128 + x];
> +
> + if (y >= sq_y && y < (sq_y + sq_h) &&
> + x >= sq_x && x < (sq_x + sq_w))
> + igt_assert(px == color);
> + else
> + igt_assert(px == 0);
> + }
> + }
> +
> + /* Now resume and check for it again. */
> + enable_one_screen(&ms_data);
> + igt_assert(wait_for_active());
> +
> + memset(cpu_buf, 0, dst_size);
> + gem_read(drm_fd, handle, 0, cpu_buf, dst_size);
> + for (y = 0; y < 128; y++) {
> + for (x = 0; x < 128; x++) {
> + uint32_t px = cpu_buf[y * 128 + x];
> +
> + if (y >= sq_y && y < (sq_y + sq_h) &&
> + x >= sq_x && x < (sq_x + sq_w))
> + igt_assert(px == color);
> + else
> + igt_assert(px == 0);
> + }
> + }
> +
> + /* Now we'll do the opposite: do the blt while active, then read while
> + * suspended. We use the same spot, but a different color. As a bonus,
> + * we're testing the presumed_offset from the previous command. */
> + color = 0x87654321;
> + submit_blt_cmd(handle, sq_x, sq_y, sq_w, sq_h, pitch, color,
> + &presumed_offset);
> +
> + disable_all_screens(&ms_data);
> + igt_assert(wait_for_suspended());
> +
> + memset(cpu_buf, 0, dst_size);
> + gem_read(drm_fd, handle, 0, cpu_buf, dst_size);
> + for (y = 0; y < 128; y++) {
> + for (x = 0; x < 128; x++) {
> + uint32_t px = cpu_buf[y * 128 + x];
> +
> + if (y >= sq_y && y < (sq_y + sq_h) &&
> + x >= sq_x && x < (sq_x + sq_w))
> + igt_assert(px == color);
> + else
> + igt_assert(px == 0);
> + }
> + }
> +
> + gem_close(drm_fd, handle);
> +
> + free(cpu_buf);
> +}
> +
> +/* Assuming execbuf already works, let's see what happens when we force many
> + * suspend/resume cycles with commands. */
> +static void gem_execbuf_stress_subtest(void)
> +{
> + int i;
> + int max = quick ? 5 : 50;
> + int batch_size = 4 * sizeof(uint32_t);
> + uint32_t batch_buf[batch_size];
> + uint32_t handle;
> + struct drm_i915_gem_execbuffer2 execbuf = {};
> + struct drm_i915_gem_exec_object2 objs[1] = {{}};
> +
> + i = 0;
> + batch_buf[i++] = MI_NOOP;
> + batch_buf[i++] = MI_NOOP;
> + batch_buf[i++] = MI_BATCH_BUFFER_END;
> + batch_buf[i++] = MI_NOOP;
> + igt_assert(i * sizeof(uint32_t) == batch_size);
> +
> + disable_all_screens(&ms_data);
> + igt_assert(wait_for_suspended());
> +
> + handle = gem_create(drm_fd, batch_size);
> + gem_write(drm_fd, handle, 0, batch_buf, batch_size);
> +
> + objs[0].handle = handle;
> +
> + execbuf.buffers_ptr = (uint64_t) objs;
> + execbuf.buffer_count = 1;
> + execbuf.batch_len = batch_size;
> + execbuf.flags = I915_EXEC_RENDER;
> + i915_execbuffer2_set_context_id(execbuf, 0);
> +
> + for (i = 0; i < max; i++) {
> + do_ioctl(drm_fd, DRM_IOCTL_I915_GEM_EXECBUFFER2, &execbuf);
> + igt_assert(wait_for_suspended());
> + }
> +
> + gem_close(drm_fd, handle);
> +}
> +
> +static void read_full_file(const char *name)
> +{
> + int rc, fd;
> + char buf[128];
> +
> + /* Make sure we're suspended! */
> + igt_assert(wait_for_suspended());
> +
> + fd = open(name, O_RDONLY);
> + if (fd < 0)
> + return;
> +
> + do {
> + rc = read(fd, buf, ARRAY_SIZE(buf));
> + } while (rc == ARRAY_SIZE(buf));
> +
> + rc = close(fd);
> + igt_assert(rc == 0);
> +}
> +
> +static void read_files_from_dir(const char *name, int level)
> +{
> + DIR *dir;
> + struct dirent *dirent;
> + char *full_name;
> + int rc;
> +
> + dir = opendir(name);
> + igt_assert(dir);
> +
> + full_name = malloc(PATH_MAX);
> +
> + igt_assert(level < 128);
> +
> + while ((dirent = readdir(dir))) {
> + struct stat stat_buf;
> +
> + if (strcmp(dirent->d_name, ".") == 0)
> + continue;
> + if (strcmp(dirent->d_name, "..") == 0)
> + continue;
> +
> + snprintf(full_name, PATH_MAX, "%s/%s", name, dirent->d_name);
> +
> + rc = lstat(full_name, &stat_buf);
> + igt_assert(rc == 0);
> +
> + if (S_ISDIR(stat_buf.st_mode))
> + read_files_from_dir(full_name, level + 1);
> +
> + if (S_ISREG(stat_buf.st_mode))
> + read_full_file(full_name);
> + }
> +
> + free(full_name);
> + closedir(dir);
> +}
> +
> +/* This test will probably pass, with a small chance of hanging the machine in
> + * case of bugs. Many of the bugs exercised by this patch just result in dmesg
> + * errors, so a "pass" here should be confirmed by a check on dmesg. */
> +static void debugfs_read_subtest(void)
> +{
> + const char *path = "/sys/kernel/debug/dri/0";
> + DIR *dir;
> +
> + dir = opendir(path);
> + igt_require_f(dir, "Can't open the debugfs directory\n");
> + closedir(dir);
> +
> + disable_all_screens(&ms_data);
> + igt_assert(wait_for_suspended());
> +
> + read_files_from_dir(path, 0);
> +}
> +
> +/* Read the comment on debugfs_read_subtest(). */
> +static void sysfs_read_subtest(void)
> +{
> + const char *path = "/sys/devices/pci0000:00/0000:00:02.0";
> + DIR *dir;
> +
> + dir = opendir(path);
> + igt_require_f(dir, "Can't open the sysfs directory\n");
> + closedir(dir);
> +
> + disable_all_screens(&ms_data);
> + igt_assert(wait_for_suspended());
> +
> + read_files_from_dir(path, 0);
> +}
> +
> +/* Make sure we don't suspend when we have the i915_forcewake_user file open. */
> +static void debugfs_forcewake_user_subtest(void)
> +{
> + int fd, rc;
> +
> + igt_require(!(IS_GEN2(ms_data.devid) || IS_GEN3(ms_data.devid) ||
> + IS_GEN4(ms_data.devid) || IS_GEN5(ms_data.devid)));
> +
> + disable_all_screens(&ms_data);
> + igt_assert(wait_for_suspended());
> +
> + fd = open("/sys/kernel/debug/dri/0/i915_forcewake_user", O_RDONLY);
> + igt_require(fd);
> +
> + igt_assert(!wait_for_suspended());
> +
> + rc = close(fd);
> + igt_assert(rc == 0);
> +
> + igt_assert(wait_for_suspended());
> +}
> +
> +int main(int argc, char *argv[])
> +{
> + /* The --quick option makes the stress tests not so stressful. */
> + if (argc > 1 && strcmp(argv[1], "--quick") == 0)
> + quick = true;
> +
> + igt_subtest_init(argc, argv);
> +
> + igt_fixture
> + setup_environment();
> +
> + igt_subtest("basic")
> + basic_subtest();
> + igt_subtest("drm-resources")
> + drm_resources_subtest();
> + igt_subtest("i2c")
> + i2c_subtest();
> + igt_subtest("pc8")
> + pc8_subtest();
> + igt_subtest("gem-mmap")
> + gem_mmap_subtest();
> + igt_subtest("gem-pread")
> + gem_pread_subtest();
> + igt_subtest("gem-execbuf")
> + gem_execbuf_subtest();
> + igt_subtest("modeset-stress-lpsp")
> + modeset_stress(true);
> + igt_subtest("modeset-stress-non-lpsp")
> + modeset_stress(false);
> + igt_subtest("gem-execbuf-stress")
> + gem_execbuf_stress_subtest();
> + igt_subtest("debugfs-read")
> + debugfs_read_subtest();
> + igt_subtest("debugfs-forcewake-user")
> + debugfs_forcewake_user_subtest();
> + igt_subtest("sysfs-read")
> + sysfs_read_subtest();
> +
> + igt_fixture
> + teardown_environment();
> +
> + igt_exit();
> +}
> --
> 1.8.3.1
>
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [RFC i-g-t] tests: add runtime_pm
2013-10-27 13:37 ` Daniel Vetter
@ 2013-10-28 12:20 ` Paulo Zanoni
2013-10-28 16:05 ` Daniel Vetter
2013-11-04 21:40 ` Ville Syrjälä
0 siblings, 2 replies; 25+ messages in thread
From: Paulo Zanoni @ 2013-10-28 12:20 UTC (permalink / raw)
To: Daniel Vetter; +Cc: Intel Graphics Development, Paulo Zanoni
2013/10/27 Daniel Vetter <daniel@ffwll.ch>:
> On Fri, Oct 25, 2013 at 11:44:05AM -0200, Paulo Zanoni wrote:
>> From: Paulo Zanoni <paulo.r.zanoni@intel.com>
>>
>> This test is based on pc8.c. It copies most of the tests from pc8.c,
>> but it depends on runtime PM status changes (parsed from sysfs)
>> instead of PC8 residency changes (parsed from the MSR registers).
>> There's also a test that checks for PC8 residency.
>>
>> For now, runtime PM and PC8 are different features, so having 2 test
>> suites makes sense. In the future we'll merge both, so we'll only get
>> PC8 when runtime PM is enabled, so we'll just kill pc8.c and keep
>> using runtime_pm.c.
>>
>> Changes compared to pc8.c:
>> - We now look at the runtime PM status instead of PC8 residencies
>> - Added more GEM tests (mmap, pread, execbuf, stress tests)
>> - Added LPSP and non-LPSP tests
>> - Added tests fro sysfs and debugfs files
>> - Added a test specifically for PC8
>>
>> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
>
> Since the actual tests we're running are so similar I prefer if we merge
> all the runtime pm tests in one file. It makes testcase maintaince (and
> bufixing, that happens) much easier. I guess a struct per runtime pm
> method (pc8, D3, ...) with a few vfuncs should get things going. The
> overall test would loop over all the pm methods and try to set things up.
> Then loop over all subtests and either skip them all (if that particular
> runtime pm method isn't supported) or just run them.
>
> We've had a few other case of massive copy&pasting in i-g-t and in the
> past few months I've merged most of them back again.
At this moment I'm really leaning towards merging PC8 and D3 into a
single feature, so it won't be possible to test them in separate
anymore. With this, we'd just have runtime_pm.c and we'd completely
kill pc8.c.
> -Daniel
>
>> ---
>> tests/.gitignore | 1 +
>> tests/Makefile.am | 1 +
>> tests/runtime_pm.c | 1262 ++++++++++++++++++++++++++++++++++++++++++++++++++++
>> 3 files changed, 1264 insertions(+)
>> create mode 100644 tests/runtime_pm.c
>>
>> diff --git a/tests/.gitignore b/tests/.gitignore
>> index 1c97a04..cda6d09 100644
>> --- a/tests/.gitignore
>> +++ b/tests/.gitignore
>> @@ -105,6 +105,7 @@ prime_nv_pcopy
>> prime_nv_test
>> prime_self_import
>> prime_udl
>> +runtime_pm
>> sysfs_rc6_residency
>> sysfs_rps
>> testdisplay
>> diff --git a/tests/Makefile.am b/tests/Makefile.am
>> index f6ba368..234aaf5 100644
>> --- a/tests/Makefile.am
>> +++ b/tests/Makefile.am
>> @@ -59,6 +59,7 @@ TESTS_progs_M = \
>> $(NOUVEAU_TESTS_M) \
>> pc8 \
>> prime_self_import \
>> + runtime_pm \
>> $(NULL)
>>
>> TESTS_progs = \
>> diff --git a/tests/runtime_pm.c b/tests/runtime_pm.c
>> new file mode 100644
>> index 0000000..425e743
>> --- /dev/null
>> +++ b/tests/runtime_pm.c
>> @@ -0,0 +1,1262 @@
>> +/*
>> + * Copyright © 2013 Intel Corporation
>> + *
>> + * Permission is hereby granted, free of charge, to any person obtaining a
>> + * copy of this software and associated documentation files (the "Software"),
>> + * to deal in the Software without restriction, including without limitation
>> + * the rights to use, copy, modify, merge, publish, distribute, sublicense,
>> + * and/or sell copies of the Software, and to permit persons to whom the
>> + * Software is furnished to do so, subject to the following conditions:
>> + *
>> + * The above copyright notice and this permission notice (including the next
>> + * paragraph) shall be included in all copies or substantial portions of the
>> + * Software.
>> + *
>> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
>> + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
>> + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL
>> + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
>> + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
>> + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS
>> + * IN THE SOFTWARE.
>> + *
>> + * Authors:
>> + * Paulo Zanoni <paulo.r.zanoni@intel.com>
>> + *
>> + */
>> +
>> +#include <string.h>
>> +#include <stdio.h>
>> +#include <stdint.h>
>> +#include <stdbool.h>
>> +#include <stdlib.h>
>> +
>> +#include <dirent.h>
>> +#include <fcntl.h>
>> +#include <unistd.h>
>> +#include <sys/ioctl.h>
>> +#include <sys/mman.h>
>> +#include <sys/types.h>
>> +#include <sys/stat.h>
>> +#include <linux/i2c.h>
>> +#include <linux/i2c-dev.h>
>> +
>> +#include "drmtest.h"
>> +#include "intel_gpu_tools.h"
>> +#include "igt_display.h"
>> +
>> +
>> +#define MSR_PC8_RES 0x630
>> +#define MSR_PC9_RES 0x631
>> +#define MSR_PC10_RES 0x632
>> +
>> +#define POWER_DIR "/sys/devices/pci0000:00/0000:00:02.0/power"
>> +
>> +#define MAX_CONNECTORS 32
>> +#define MAX_ENCODERS 32
>> +#define MAX_CRTCS 16
>> +
>> +
>> +enum runtime_pm_status {
>> + RUNTIME_PM_STATUS_ACTIVE,
>> + RUNTIME_PM_STATUS_SUSPENDED,
>> + RUNTIME_PM_STATUS_SUSPENDING,
>> + RUNTIME_PM_STATUS_UNKNOWN,
>> +};
>> +
>> +enum screen_type {
>> + SCREEN_TYPE_LPSP,
>> + SCREEN_TYPE_NON_LPSP,
>> + SCREEN_TYPE_ANY,
>> +};
>> +
>> +/* Stuff used when creating FBs and mode setting. */
>> +struct mode_set_data {
>> + drmModeResPtr res;
>> + drmModeConnectorPtr connectors[MAX_CONNECTORS];
>> + drmModePropertyBlobPtr edids[MAX_CONNECTORS];
>> +
>> + uint32_t devid;
>> +};
>> +
>> +/* Stuff we query at different times so we can compare. */
>> +struct compare_data {
>> + drmModeResPtr res;
>> + drmModeEncoderPtr encoders[MAX_ENCODERS];
>> + drmModeConnectorPtr connectors[MAX_CONNECTORS];
>> + drmModeCrtcPtr crtcs[MAX_CRTCS];
>> + drmModePropertyBlobPtr edids[MAX_CONNECTORS];
>> +};
>> +
>> +
>> +int drm_fd;
>> +int status_fd;
>> +int msr_fd = 0;
>> +struct mode_set_data ms_data;
>> +bool quick = false;
>> +
>> +
>> +static uint64_t get_residency(uint32_t type)
>> +{
>> + int rc;
>> + uint64_t ret;
>> +
>> + rc = pread(msr_fd, &ret, sizeof(uint64_t), type);
>> + igt_assert(rc == sizeof(ret));
>> +
>> + return ret;
>> +}
>> +
>> +static bool pc8_plus_residency_changed(unsigned int timeout_sec)
>> +{
>> + unsigned int i;
>> + uint64_t res_pc8, res_pc9, res_pc10;
>> + int to_sleep = 100 * 1000;
>> +
>> + res_pc8 = get_residency(MSR_PC8_RES);
>> + res_pc9 = get_residency(MSR_PC9_RES);
>> + res_pc10 = get_residency(MSR_PC10_RES);
>> +
>> + for (i = 0; i < timeout_sec * 1000 * 1000; i += to_sleep) {
>> + if (res_pc8 != get_residency(MSR_PC8_RES) ||
>> + res_pc9 != get_residency(MSR_PC9_RES) ||
>> + res_pc10 != get_residency(MSR_PC10_RES)) {
>> + return true;
>> + }
>> + usleep(to_sleep);
>> + }
>> +
>> + return false;
>> +}
>> +
>> +/* Checks not only if PC8+ is allowed, but also if we're reaching it.
>> + * We call this when we expect this function to return quickly since PC8 is
>> + * actually enabled, so the 30s timeout we use shouldn't matter. */
>> +static bool pc8_plus_enabled(void)
>> +{
>> + return pc8_plus_residency_changed(30);
>> +}
>> +
>> +/* We call this when we expect PC8+ to be actually disabled, so we should not
>> + * return until the 5s timeout expires. In other words: in the "happy case",
>> + * every time we call this function, it will sleep for 5 seconds. */
>> +static bool pc8_plus_disabled(void)
>> +{
>> + return !pc8_plus_residency_changed(5);
>> +}
>> +
>> +/* If the read fails, then the machine doesn't support PC8+ residencies. */
>> +static bool supports_pc8_plus_residencies(void)
>> +{
>> + int rc;
>> + uint64_t val;
>> +
>> + rc = pread(msr_fd, &val, sizeof(uint64_t), MSR_PC8_RES);
>> + if (rc != sizeof(val))
>> + return false;
>> + rc = pread(msr_fd, &val, sizeof(uint64_t), MSR_PC9_RES);
>> + if (rc != sizeof(val))
>> + return false;
>> + rc = pread(msr_fd, &val, sizeof(uint64_t), MSR_PC10_RES);
>> + if (rc != sizeof(val))
>> + return false;
>> +
>> + return true;
>> +}
>> +
>> +static enum runtime_pm_status get_runtime_pm_status(void)
>> +{
>> + ssize_t n_read;
>> + char buf[32];
>> +
>> + lseek(status_fd, 0, SEEK_SET);
>> + n_read = read(status_fd, buf, ARRAY_SIZE(buf));
>> + igt_assert(n_read >= 0);
>> + buf[n_read] = '\0';
>> +
>> + if (strncmp(buf, "suspended\n", n_read) == 0)
>> + return RUNTIME_PM_STATUS_SUSPENDED;
>> + else if (strncmp(buf, "active\n", n_read) == 0)
>> + return RUNTIME_PM_STATUS_ACTIVE;
>> + else if (strncmp(buf, "suspending\n", n_read) == 0)
>> + return RUNTIME_PM_STATUS_SUSPENDING;
>> +
>> + igt_assert_f(false, "Unknown status %s\n", buf);
>> + return RUNTIME_PM_STATUS_UNKNOWN;
>> +}
>> +
>> +static bool wait_for_status(enum runtime_pm_status status)
>> +{
>> + int i;
>> + int hundred_ms = 100 * 1000, ten_s = 10 * 1000 * 1000;
>> +
>> + for (i = 0; i < ten_s; i += hundred_ms) {
>> + if (get_runtime_pm_status() == status)
>> + return true;
>> +
>> + usleep(hundred_ms);
>> + }
>> +
>> + return false;
>> +}
>> +
>> +static bool wait_for_suspended(void)
>> +{
>> + return wait_for_status(RUNTIME_PM_STATUS_SUSPENDED);
>> +}
>> +
>> +static bool wait_for_active(void)
>> +{
>> + return wait_for_status(RUNTIME_PM_STATUS_ACTIVE);
>> +}
>> +
>> +static void disable_all_screens(struct mode_set_data *data)
>> +{
>> + int i, rc;
>> +
>> + for (i = 0; i < data->res->count_crtcs; i++) {
>> + rc = drmModeSetCrtc(drm_fd, data->res->crtcs[i], -1, 0, 0,
>> + NULL, 0, NULL);
>> + igt_assert(rc == 0);
>> + }
>> +}
>> +
>> +static uint32_t create_fb(struct mode_set_data *data, int width, int height)
>> +{
>> + struct kmstest_fb fb;
>> + cairo_t *cr;
>> + uint32_t buffer_id;
>> +
>> + buffer_id = kmstest_create_fb(drm_fd, width, height, 32, 24, false,
>> + &fb);
>> + cr = kmstest_get_cairo_ctx(drm_fd, &fb);
>> + kmstest_paint_test_pattern(cr, width, height);
>> + return buffer_id;
>> +}
>> +
>> +static bool enable_one_screen_with_type(struct mode_set_data *data,
>> + enum screen_type type)
>> +{
>> + uint32_t crtc_id = 0, buffer_id = 0, connector_id = 0;
>> + drmModeModeInfoPtr mode = NULL;
>> + int i, rc;
>> +
>> + for (i = 0; i < data->res->count_connectors; i++) {
>> + drmModeConnectorPtr c = data->connectors[i];
>> +
>> + if (type == SCREEN_TYPE_LPSP &&
>> + c->connector_type != DRM_MODE_CONNECTOR_eDP)
>> + continue;
>> +
>> + if (type == SCREEN_TYPE_NON_LPSP &&
>> + c->connector_type == DRM_MODE_CONNECTOR_eDP)
>> + continue;
>> +
>> + if (c->connection == DRM_MODE_CONNECTED && c->count_modes) {
>> + connector_id = c->connector_id;
>> + mode = &c->modes[0];
>> + break;
>> + }
>> + }
>> +
>> + if (connector_id == 0)
>> + return false;
>> +
>> + crtc_id = data->res->crtcs[0];
>> + buffer_id = create_fb(data, mode->hdisplay, mode->vdisplay);
>> +
>> + igt_assert(crtc_id);
>> + igt_assert(buffer_id);
>> + igt_assert(connector_id);
>> + igt_assert(mode);
>> +
>> + rc = drmModeSetCrtc(drm_fd, crtc_id, buffer_id, 0, 0, &connector_id,
>> + 1, mode);
>> + igt_assert(rc == 0);
>> +
>> + return true;
>> +}
>> +
>> +static void enable_one_screen(struct mode_set_data *data)
>> +{
>> + igt_assert(enable_one_screen_with_type(data, SCREEN_TYPE_ANY));
>> +}
>> +
>> +/* We could check the checksum too, but just the header is probably enough. */
>> +static bool edid_is_valid(const unsigned char *edid)
>> +{
>> + char edid_header[] = {
>> + 0x0, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0x0,
>> + };
>> +
>> + return (memcmp(edid, edid_header, sizeof(edid_header)) == 0);
>> +}
>> +
>> +static int count_drm_valid_edids(struct mode_set_data *data)
>> +{
>> + int i, ret = 0;
>> +
>> + for (i = 0; i < data->res->count_connectors; i++)
>> + if (data->edids[i] && edid_is_valid(data->edids[i]->data))
>> + ret++;
>> + return ret;
>> +}
>> +
>> +static bool i2c_edid_is_valid(int fd)
>> +{
>> + int rc;
>> + unsigned char edid[128] = {};
>> + struct i2c_msg msgs[] = {
>> + { /* Start at 0. */
>> + .addr = 0x50,
>> + .flags = 0,
>> + .len = 1,
>> + .buf = edid,
>> + }, { /* Now read the EDID. */
>> + .addr = 0x50,
>> + .flags = I2C_M_RD,
>> + .len = 128,
>> + .buf = edid,
>> + }
>> + };
>> + struct i2c_rdwr_ioctl_data msgset = {
>> + .msgs = msgs,
>> + .nmsgs = 2,
>> + };
>> +
>> + rc = ioctl(fd, I2C_RDWR, &msgset);
>> + return (rc >= 0) ? edid_is_valid(edid) : false;
>> +}
>> +
>> +static int count_i2c_valid_edids(void)
>> +{
>> + int fd, ret = 0;
>> + DIR *dir;
>> +
>> + struct dirent *dirent;
>> + char full_name[32];
>> +
>> + dir = opendir("/dev/");
>> + igt_assert(dir);
>> +
>> + while ((dirent = readdir(dir))) {
>> + if (strncmp(dirent->d_name, "i2c-", 4) == 0) {
>> + snprintf(full_name, 32, "/dev/%s", dirent->d_name);
>> + fd = open(full_name, O_RDWR);
>> + igt_assert(fd != -1);
>> + if (i2c_edid_is_valid(fd))
>> + ret++;
>> + close(fd);
>> + }
>> + }
>> +
>> + closedir(dir);
>> +
>> + return ret;
>> +}
>> +
>> +static void test_i2c(struct mode_set_data *data)
>> +{
>> + int i2c_edids = count_i2c_valid_edids();
>> + int drm_edids = count_drm_valid_edids(data);
>> +
>> + igt_assert(i2c_edids == drm_edids);
>> +}
>> +
>> +static void i2c_subtest_check_environment(void)
>> +{
>> + int i2c_dev_files = 0;
>> + DIR *dev_dir;
>> + struct dirent *dirent;
>> +
>> + /* Make sure the /dev/i2c-* files exist. */
>> + dev_dir = opendir("/dev");
>> + igt_assert(dev_dir);
>> + while ((dirent = readdir(dev_dir))) {
>> + if (strncmp(dirent->d_name, "i2c-", 4) == 0)
>> + i2c_dev_files++;
>> + }
>> + closedir(dev_dir);
>> + igt_require(i2c_dev_files);
>> +}
>> +
>> +
>> +#define COMPARE(d1, d2, data) igt_assert(d1->data == d2->data)
>> +#define COMPARE_ARRAY(d1, d2, size, data) do { \
>> + for (i = 0; i < size; i++) \
>> + igt_assert(d1->data[i] == d2->data[i]); \
>> +} while (0)
>> +
>> +static void assert_drm_resources_equal(struct compare_data *d1,
>> + struct compare_data *d2)
>> +{
>> + COMPARE(d1, d2, res->count_connectors);
>> + COMPARE(d1, d2, res->count_encoders);
>> + COMPARE(d1, d2, res->count_crtcs);
>> + COMPARE(d1, d2, res->min_width);
>> + COMPARE(d1, d2, res->max_width);
>> + COMPARE(d1, d2, res->min_height);
>> + COMPARE(d1, d2, res->max_height);
>> +}
>> +
>> +static void assert_modes_equal(drmModeModeInfoPtr m1, drmModeModeInfoPtr m2)
>> +{
>> + COMPARE(m1, m2, clock);
>> + COMPARE(m1, m2, hdisplay);
>> + COMPARE(m1, m2, hsync_start);
>> + COMPARE(m1, m2, hsync_end);
>> + COMPARE(m1, m2, htotal);
>> + COMPARE(m1, m2, hskew);
>> + COMPARE(m1, m2, vdisplay);
>> + COMPARE(m1, m2, vsync_start);
>> + COMPARE(m1, m2, vsync_end);
>> + COMPARE(m1, m2, vtotal);
>> + COMPARE(m1, m2, vscan);
>> + COMPARE(m1, m2, vrefresh);
>> + COMPARE(m1, m2, flags);
>> + COMPARE(m1, m2, type);
>> + igt_assert(strcmp(m1->name, m2->name) == 0);
>> +}
>> +
>> +static void assert_drm_connectors_equal(drmModeConnectorPtr c1,
>> + drmModeConnectorPtr c2)
>> +{
>> + int i;
>> +
>> + COMPARE(c1, c2, connector_id);
>> + COMPARE(c1, c2, connector_type);
>> + COMPARE(c1, c2, connector_type_id);
>> + COMPARE(c1, c2, mmWidth);
>> + COMPARE(c1, c2, mmHeight);
>> + COMPARE(c1, c2, count_modes);
>> + COMPARE(c1, c2, count_props);
>> + COMPARE(c1, c2, count_encoders);
>> + COMPARE_ARRAY(c1, c2, c1->count_props, props);
>> + COMPARE_ARRAY(c1, c2, c1->count_encoders, encoders);
>> +
>> + for (i = 0; i < c1->count_modes; i++)
>> + assert_modes_equal(&c1->modes[0], &c2->modes[0]);
>> +}
>> +
>> +static void assert_drm_encoders_equal(drmModeEncoderPtr e1,
>> + drmModeEncoderPtr e2)
>> +{
>> + COMPARE(e1, e2, encoder_id);
>> + COMPARE(e1, e2, encoder_type);
>> + COMPARE(e1, e2, possible_crtcs);
>> + COMPARE(e1, e2, possible_clones);
>> +}
>> +
>> +static void assert_drm_crtcs_equal(drmModeCrtcPtr c1, drmModeCrtcPtr c2)
>> +{
>> + COMPARE(c1, c2, crtc_id);
>> +}
>> +
>> +static void assert_drm_edids_equal(drmModePropertyBlobPtr e1,
>> + drmModePropertyBlobPtr e2)
>> +{
>> + if (!e1 && !e2)
>> + return;
>> + igt_assert(e1 && e2);
>> +
>> + COMPARE(e1, e2, id);
>> + COMPARE(e1, e2, length);
>> +
>> + igt_assert(memcmp(e1->data, e2->data, e1->length) == 0);
>> +}
>> +
>> +static void assert_drm_infos_equal(struct compare_data *d1,
>> + struct compare_data *d2)
>> +{
>> + int i;
>> +
>> + assert_drm_resources_equal(d1, d2);
>> +
>> + for (i = 0; i < d1->res->count_connectors; i++) {
>> + assert_drm_connectors_equal(d1->connectors[i],
>> + d2->connectors[i]);
>> + assert_drm_edids_equal(d1->edids[i], d2->edids[i]);
>> + }
>> +
>> + for (i = 0; i < d1->res->count_encoders; i++)
>> + assert_drm_encoders_equal(d1->encoders[i], d2->encoders[i]);
>> +
>> + for (i = 0; i < d1->res->count_crtcs; i++)
>> + assert_drm_crtcs_equal(d1->crtcs[i], d2->crtcs[i]);
>> +}
>> +
>> +static drmModePropertyBlobPtr get_connector_edid(drmModeConnectorPtr connector,
>> + int index)
>> +{
>> + unsigned int i;
>> + drmModeObjectPropertiesPtr props;
>> + drmModePropertyBlobPtr ret = NULL;
>> +
>> + props = drmModeObjectGetProperties(drm_fd, connector->connector_id,
>> + DRM_MODE_OBJECT_CONNECTOR);
>> +
>> + for (i = 0; i < props->count_props; i++) {
>> + drmModePropertyPtr prop = drmModeGetProperty(drm_fd,
>> + props->props[i]);
>> +
>> + if (strcmp(prop->name, "EDID") == 0) {
>> + igt_assert(prop->flags & DRM_MODE_PROP_BLOB);
>> + igt_assert(prop->count_blobs == 0);
>> + ret = drmModeGetPropertyBlob(drm_fd,
>> + props->prop_values[i]);
>> + }
>> +
>> + drmModeFreeProperty(prop);
>> + }
>> +
>> + drmModeFreeObjectProperties(props);
>> + return ret;
>> +}
>> +
>> +
>> +static void get_drm_info(struct compare_data *data)
>> +{
>> + int i;
>> +
>> + data->res = drmModeGetResources(drm_fd);
>> + igt_assert(data->res);
>> +
>> + igt_assert(data->res->count_connectors <= MAX_CONNECTORS);
>> + igt_assert(data->res->count_encoders <= MAX_ENCODERS);
>> + igt_assert(data->res->count_crtcs <= MAX_CRTCS);
>> +
>> + for (i = 0; i < data->res->count_connectors; i++) {
>> + data->connectors[i] = drmModeGetConnector(drm_fd,
>> + data->res->connectors[i]);
>> + data->edids[i] = get_connector_edid(data->connectors[i], i);
>> + }
>> + for (i = 0; i < data->res->count_encoders; i++)
>> + data->encoders[i] = drmModeGetEncoder(drm_fd,
>> + data->res->encoders[i]);
>> + for (i = 0; i < data->res->count_crtcs; i++)
>> + data->crtcs[i] = drmModeGetCrtc(drm_fd, data->res->crtcs[i]);
>> +}
>> +
>> +static void free_drm_info(struct compare_data *data)
>> +{
>> + int i;
>> +
>> + for (i = 0; i < data->res->count_connectors; i++) {
>> + drmModeFreeConnector(data->connectors[i]);
>> + drmModeFreePropertyBlob(data->edids[i]);
>> + }
>> + for (i = 0; i < data->res->count_encoders; i++)
>> + drmModeFreeEncoder(data->encoders[i]);
>> + for (i = 0; i < data->res->count_crtcs; i++)
>> + drmModeFreeCrtc(data->crtcs[i]);
>> +
>> + drmModeFreeResources(data->res);
>> +}
>> +
>> +static void init_mode_set_data(struct mode_set_data *data)
>> +{
>> + int i;
>> +
>> + data->res = drmModeGetResources(drm_fd);
>> + igt_assert(data->res);
>> + igt_assert(data->res->count_connectors <= MAX_CONNECTORS);
>> +
>> + for (i = 0; i < data->res->count_connectors; i++) {
>> + data->connectors[i] = drmModeGetConnector(drm_fd,
>> + data->res->connectors[i]);
>> + data->edids[i] = get_connector_edid(data->connectors[i], i);
>> + }
>> +
>> + data->devid = intel_get_drm_devid(drm_fd);
>> +
>> + do_or_die(igt_set_vt_graphics_mode());
>> +}
>> +
>> +static void fini_mode_set_data(struct mode_set_data *data)
>> +{
>> + int i;
>> +
>> + for (i = 0; i < data->res->count_connectors; i++) {
>> + drmModeFreeConnector(data->connectors[i]);
>> + drmModeFreePropertyBlob(data->edids[i]);
>> + }
>> + drmModeFreeResources(data->res);
>> +}
>> +
>> +static void pc8_subtest_check_environment(void)
>> +{
>> + igt_require_f(IS_HASWELL(ms_data.devid),
>> + "PC8+ feature only supported on Haswell.\n");
>> +
>> + /* If this fails, you don't have the Kernel module loaded. */
>> + msr_fd = open("/dev/cpu/0/msr", O_RDONLY);
>> + igt_assert_f(msr_fd >= 0, "Can't open /dev/cpu/0/msr.\n");
>> +
>> + /* Non-ULT machines don't support PC8+. */
>> + igt_require_f(supports_pc8_plus_residencies(),
>> + "Machine doesn't support PC8+ residencies.\n");
>> +}
>> +
>> +static void assert_runtime_pm_is_enabled(void)
>> +{
>> + int fd;
>> + ssize_t n_read;
>> + char buf[6];
>> + int rc;
>> +
>> + fd = open(POWER_DIR "/control", O_RDONLY);
>> + igt_assert_f(fd >= 0, "Can't open " POWER_DIR "/control\n");
>> +
>> + n_read = read(fd, buf, ARRAY_SIZE(buf));
>> + igt_assert(n_read >= 0);
>> + igt_require_f(strncmp(buf, "auto\n", n_read) == 0,
>> + "Runtime PM not enabled or supported on this platform\n");
>> +
>> + rc = close(fd);
>> + igt_assert(rc == 0);
>> +}
>> +
>> +/* We set 1ms autosuspend delay here so things start/stop fast and we don't
>> + * waste time waiting for runtime PM to kick in. We don't care about restoring
>> + * the original value at the end of the test. */
>> +static void set_autosuspend_delay(void)
>> +{
>> + int fd;
>> + ssize_t n_write;
>> + int rc;
>> +
>> + fd = open(POWER_DIR "/autosuspend_delay_ms", O_WRONLY);
>> + igt_assert_f(fd >= 0,
>> + "Can't open " POWER_DIR "/autosuspend_delay_ms\n");
>> +
>> + n_write = write(fd, "1\n", 2);
>> + igt_assert(n_write == 2);
>> +
>> + rc = close(fd);
>> + igt_assert(rc == 0);
>> +
>> +}
>> +
>> +static void setup_environment(void)
>> +{
>> +
>> + drm_fd = drm_open_any();
>> + igt_assert(drm_fd >= 0);
>> +
>> + init_mode_set_data(&ms_data);
>> +
>> + assert_runtime_pm_is_enabled(),
>> +
>> + status_fd = open(POWER_DIR "/runtime_status", O_RDONLY);
>> + igt_assert_f(status_fd >= 0,
>> + "Can't open " POWER_DIR "/runtime_status\n");
>> +
>> + set_autosuspend_delay();
>> +}
>> +
>> +static void teardown_environment(void)
>> +{
>> + int rc;
>> +
>> + fini_mode_set_data(&ms_data);
>> +
>> + drmClose(drm_fd);
>> +
>> + rc = close(status_fd);
>> + igt_assert(rc == 0);
>> +
>> + if (msr_fd) {
>> + rc = close(msr_fd);
>> + igt_assert(rc == 0);
>> + }
>> +}
>> +
>> +/* Sanity test: make sure it suspends when all screens are disabled and it
>> + * resumes when we enable a screen. */
>> +static void basic_subtest(void)
>> +{
>> + disable_all_screens(&ms_data);
>> + igt_assert_f(wait_for_suspended(),
>> + "Graphics device is not suspending, please check its "
>> + "runtime PM configuration.\n");
>> +
>> + enable_one_screen(&ms_data);
>> + igt_assert_f(wait_for_active(),
>> + "Graphics device is still suspended even with a screen "
>> + "enabled. Something is really wrong.\n");
>> +}
>> +
>> +/* Test if the DRM resources reported by the IOCTLs are still the same. This
>> + * ensures we still see the monitors with the same eyes. We get the EDIDs and
>> + * compare them, which ensures we use DP AUX or GMBUS depending on what's
>> + * connected. */
>> +static void drm_resources_subtest(void)
>> +{
>> + struct compare_data pre_suspend, during_suspend, post_suspend;
>> +
>> + enable_one_screen(&ms_data);
>> + igt_assert(wait_for_active());
>> + get_drm_info(&pre_suspend);
>> + igt_assert(wait_for_active());
>> +
>> + disable_all_screens(&ms_data);
>> + igt_assert(wait_for_suspended());
>> + get_drm_info(&during_suspend);
>> + igt_assert(wait_for_suspended());
>> +
>> + enable_one_screen(&ms_data);
>> + igt_assert(wait_for_active());
>> + get_drm_info(&post_suspend);
>> + igt_assert(wait_for_active());
>> +
>> + assert_drm_infos_equal(&pre_suspend, &during_suspend);
>> + assert_drm_infos_equal(&pre_suspend, &post_suspend);
>> +
>> + free_drm_info(&pre_suspend);
>> + free_drm_info(&during_suspend);
>> + free_drm_info(&post_suspend);
>> +}
>> +
>> +/* Try to use raw I2C, which also needs interrupts. */
>> +static void i2c_subtest(void)
>> +{
>> + i2c_subtest_check_environment();
>> +
>> + enable_one_screen(&ms_data);
>> + igt_assert(wait_for_active());
>> +
>> + disable_all_screens(&ms_data);
>> + igt_assert(wait_for_suspended());
>> + test_i2c(&ms_data);
>> + igt_assert(wait_for_suspended());
>> +
>> + enable_one_screen(&ms_data);
>> + igt_assert(wait_for_active());
>> +}
>> +
>> +/* The LPSP version tests eDP, which doesn't require the power well. The
>> + * non-LPSP version will make sure we toggle the power well on/off every time.
>> + * We SKIP in case we can't find the appropriate type of output connected, but
>> + * that doesn't test as much as we could. */
>> +static void modeset_stress(bool lpsp)
>> +{
>> + int i, max;
>> + enum screen_type type;
>> +
>> + max = quick ? 5 : 50;
>> + type = lpsp ? SCREEN_TYPE_LPSP : SCREEN_TYPE_NON_LPSP;
>> +
>> + for (i = 0; i < max; i++) {
>> + disable_all_screens(&ms_data);
>> + igt_assert(wait_for_suspended());
>> +
>> + /* If we skip this line it's because the type of screen we want
>> + * is not connected. */
>> + igt_require(enable_one_screen_with_type(&ms_data, type));
>> + igt_assert(wait_for_active());
>> + }
>> +}
>> +
>> +/* Make sure we get to PC8 or deeper states when we're suspended. Notice that
>> + * this actually depends on a properly configured machine, where the runtime PM
>> + * polices for _all_ devices (not only graphics) are configured correctly. If
>> + * you use "powertop" and switch everything to "good", there's a good chance
>> + * you'll be fine here. */
>> +static void pc8_subtest(void)
>> +{
>> + pc8_subtest_check_environment();
>> +
>> + disable_all_screens(&ms_data);
>> + igt_assert(wait_for_suspended());
>> + igt_assert(pc8_plus_enabled());
>> +
>> + enable_one_screen(&ms_data);
>> + igt_assert(wait_for_active());
>> + igt_assert(pc8_plus_disabled());
>> +}
>> +
>> +static void *mmap_gem_buf(uint32_t handle, int size)
>> +{
>> + struct drm_i915_gem_mmap gem_mmap;
>> +
>> + gem_mmap.handle = handle;
>> + gem_mmap.offset = 0;
>> + gem_mmap.size = size;
>> + do_ioctl(drm_fd, DRM_IOCTL_I915_GEM_MMAP, &gem_mmap);
>> + igt_assert(gem_mmap.addr_ptr);
>> +
>> + return (void *) gem_mmap.addr_ptr;
>> +}
>> +
>> +/* Make sure even mmap still works when we're in D3. */
>> +static void gem_mmap_subtest(void)
>> +{
>> + int i;
>> + uint32_t handle;
>> + int buf_size = 8192;
>> + uint8_t *gem_buf;
>> +
>> + /* Create, map and set data while the device is active. */
>> + enable_one_screen(&ms_data);
>> + igt_assert(wait_for_active());
>> +
>> + handle = gem_create(drm_fd, buf_size);
>> +
>> + gem_buf = mmap_gem_buf(handle, buf_size);
>> +
>> + for (i = 0; i < buf_size; i++)
>> + gem_buf[i] = i & 0xFF;
>> +
>> + for (i = 0; i < buf_size; i++)
>> + igt_assert(gem_buf[i] == (i & 0xFF));
>> +
>> + /* Now suspend, read and modify. */
>> + disable_all_screens(&ms_data);
>> + igt_assert(wait_for_suspended());
>> +
>> + for (i = 0; i < buf_size; i++)
>> + igt_assert(gem_buf[i] == (i & 0xFF));
>> + igt_assert(wait_for_suspended());
>> +
>> + for (i = 0; i < buf_size; i++)
>> + gem_buf[i] = (~i & 0xFF);
>> + igt_assert(wait_for_suspended());
>> +
>> + /* Now resume and see if it's still there. */
>> + enable_one_screen(&ms_data);
>> + igt_assert(wait_for_active());
>> + for (i = 0; i < buf_size; i++)
>> + igt_assert(gem_buf[i] == (~i & 0xFF));
>> +
>> + munmap(gem_buf, buf_size);
>> +
>> + gem_close(drm_fd, handle);
>> +}
>> +
>> +/* Make sure pread() and pwrite() still work when we're in D3. */
>> +static void gem_pread_subtest(void)
>> +{
>> + int i;
>> + uint32_t handle;
>> + int buf_size = 8192;
>> + uint8_t *cpu_buf, *read_buf;
>> +
>> + cpu_buf = malloc(buf_size);
>> + read_buf = malloc(buf_size);
>> + igt_assert(cpu_buf);
>> + igt_assert(read_buf);
>> + memset(cpu_buf, 0, buf_size);
>> + memset(read_buf, 0, buf_size);
>> +
>> + /* Create and set data while the device is active. */
>> + enable_one_screen(&ms_data);
>> + igt_assert(wait_for_active());
>> +
>> + handle = gem_create(drm_fd, buf_size);
>> +
>> + for (i = 0; i < buf_size; i++)
>> + cpu_buf[i] = i & 0xFF;
>> +
>> + gem_write(drm_fd, handle, 0, cpu_buf, buf_size);
>> +
>> + gem_read(drm_fd, handle, 0, read_buf, buf_size);
>> +
>> + for (i = 0; i < buf_size; i++)
>> + igt_assert(cpu_buf[i] == read_buf[i]);
>> +
>> + /* Now suspend, read and modify. */
>> + disable_all_screens(&ms_data);
>> + igt_assert(wait_for_suspended());
>> +
>> + memset(read_buf, 0, buf_size);
>> + gem_read(drm_fd, handle, 0, read_buf, buf_size);
>> +
>> + for (i = 0; i < buf_size; i++)
>> + igt_assert(cpu_buf[i] == read_buf[i]);
>> + igt_assert(wait_for_suspended());
>> +
>> + for (i = 0; i < buf_size; i++)
>> + cpu_buf[i] = (~i & 0xFF);
>> + gem_write(drm_fd, handle, 0, cpu_buf, buf_size);
>> + igt_assert(wait_for_suspended());
>> +
>> + /* Now resume and see if it's still there. */
>> + enable_one_screen(&ms_data);
>> + igt_assert(wait_for_active());
>> +
>> + memset(read_buf, 0, buf_size);
>> + gem_read(drm_fd, handle, 0, read_buf, buf_size);
>> +
>> + for (i = 0; i < buf_size; i++)
>> + igt_assert(cpu_buf[i] == read_buf[i]);
>> +
>> + gem_close(drm_fd, handle);
>> +
>> + free(cpu_buf);
>> + free(read_buf);
>> +}
>> +
>> +/* Paints a square of color $color, size $width x $height, at position $x x $y
>> + * of $dst_handle, which contains pitch $pitch. */
>> +static void submit_blt_cmd(uint32_t dst_handle, uint32_t x, uint32_t y,
>> + uint32_t width, uint32_t height, uint32_t pitch,
>> + uint32_t color, uint32_t *presumed_dst_offset)
>> +{
>> + int i, reloc_pos;
>> + int bpp = 4;
>> + uint32_t batch_handle;
>> + int batch_size = 8 * sizeof(uint32_t);
>> + uint32_t batch_buf[batch_size];
>> + uint32_t offset_in_dst = (pitch * y) + (x * bpp);
>> + struct drm_i915_gem_execbuffer2 execbuf = {};
>> + struct drm_i915_gem_exec_object2 objs[2] = {{}, {}};
>> + struct drm_i915_gem_relocation_entry relocs[1] = {{}};
>> + struct drm_i915_gem_wait gem_wait;
>> +
>> + i = 0;
>> +
>> + batch_buf[i++] = COLOR_BLT_CMD | COLOR_BLT_WRITE_ALPHA |
>> + COLOR_BLT_WRITE_RGB;
>> + batch_buf[i++] = (3 << 24) | (0xF0 << 16) | pitch;
>> + batch_buf[i++] = (height << 16) | width * bpp;
>> + reloc_pos = i;
>> + batch_buf[i++] = *presumed_dst_offset + offset_in_dst;
>> + batch_buf[i++] = color;
>> +
>> + batch_buf[i++] = MI_NOOP;
>> + batch_buf[i++] = MI_BATCH_BUFFER_END;
>> + batch_buf[i++] = MI_NOOP;
>> +
>> + igt_assert(i * sizeof(uint32_t) == batch_size);
>> +
>> + batch_handle = gem_create(drm_fd, batch_size);
>> + gem_write(drm_fd, batch_handle, 0, batch_buf, batch_size);
>> +
>> + relocs[0].target_handle = dst_handle;
>> + relocs[0].delta = offset_in_dst;
>> + relocs[0].offset = reloc_pos * sizeof(uint32_t);
>> + relocs[0].presumed_offset = *presumed_dst_offset;
>> + relocs[0].read_domains = 0;
>> + relocs[0].write_domain = I915_GEM_DOMAIN_RENDER;
>> +
>> + objs[0].handle = dst_handle;
>> + objs[0].alignment = 64;
>> +
>> + objs[1].handle = batch_handle;
>> + objs[1].relocation_count = 1;
>> + objs[1].relocs_ptr = (uint64_t) relocs;
>> +
>> + execbuf.buffers_ptr = (uint64_t) objs;
>> + execbuf.buffer_count = 2;
>> + execbuf.batch_len = batch_size;
>> + execbuf.flags = I915_EXEC_BLT;
>> + i915_execbuffer2_set_context_id(execbuf, 0);
>> +
>> + do_ioctl(drm_fd, DRM_IOCTL_I915_GEM_EXECBUFFER2, &execbuf);
>> +
>> + *presumed_dst_offset = relocs[0].presumed_offset;
>> +
>> + gem_wait.flags = 0;
>> + gem_wait.timeout_ns = 10000000000LL; /* 10s */
>> +
>> + gem_wait.bo_handle = batch_handle;
>> + do_ioctl(drm_fd, DRM_IOCTL_I915_GEM_WAIT, &gem_wait);
>> +
>> + gem_wait.bo_handle = dst_handle;
>> + do_ioctl(drm_fd, DRM_IOCTL_I915_GEM_WAIT, &gem_wait);
>> +
>> + gem_close(drm_fd, batch_handle);
>> +}
>> +
>> +/* Make sure we can submit a batch buffer and verify its result. */
>> +static void gem_execbuf_subtest(void)
>> +{
>> + int x, y;
>> + uint32_t handle;
>> + int bpp = 4;
>> + int pitch = 128 * bpp;
>> + int dst_size = 128 * 128 * bpp; /* 128x128 square */
>> + uint32_t *cpu_buf;
>> + uint32_t presumed_offset = 0;
>> + int sq_x = 5, sq_y = 10, sq_w = 15, sq_h = 20;
>> + uint32_t color;
>> +
>> + /* Create and set data while the device is active. */
>> + enable_one_screen(&ms_data);
>> + igt_assert(wait_for_active());
>> +
>> + handle = gem_create(drm_fd, dst_size);
>> +
>> + cpu_buf = malloc(dst_size);
>> + igt_assert(cpu_buf);
>> + memset(cpu_buf, 0, dst_size);
>> + gem_write(drm_fd, handle, 0, cpu_buf, dst_size);
>> +
>> + /* Now suspend and try it. */
>> + disable_all_screens(&ms_data);
>> + igt_assert(wait_for_suspended());
>> +
>> + color = 0x12345678;
>> + submit_blt_cmd(handle, sq_x, sq_y, sq_w, sq_h, pitch, color,
>> + &presumed_offset);
>> + igt_assert(wait_for_suspended());
>> +
>> + gem_read(drm_fd, handle, 0, cpu_buf, dst_size);
>> + igt_assert(wait_for_suspended());
>> + for (y = 0; y < 128; y++) {
>> + for (x = 0; x < 128; x++) {
>> + uint32_t px = cpu_buf[y * 128 + x];
>> +
>> + if (y >= sq_y && y < (sq_y + sq_h) &&
>> + x >= sq_x && x < (sq_x + sq_w))
>> + igt_assert(px == color);
>> + else
>> + igt_assert(px == 0);
>> + }
>> + }
>> +
>> + /* Now resume and check for it again. */
>> + enable_one_screen(&ms_data);
>> + igt_assert(wait_for_active());
>> +
>> + memset(cpu_buf, 0, dst_size);
>> + gem_read(drm_fd, handle, 0, cpu_buf, dst_size);
>> + for (y = 0; y < 128; y++) {
>> + for (x = 0; x < 128; x++) {
>> + uint32_t px = cpu_buf[y * 128 + x];
>> +
>> + if (y >= sq_y && y < (sq_y + sq_h) &&
>> + x >= sq_x && x < (sq_x + sq_w))
>> + igt_assert(px == color);
>> + else
>> + igt_assert(px == 0);
>> + }
>> + }
>> +
>> + /* Now we'll do the opposite: do the blt while active, then read while
>> + * suspended. We use the same spot, but a different color. As a bonus,
>> + * we're testing the presumed_offset from the previous command. */
>> + color = 0x87654321;
>> + submit_blt_cmd(handle, sq_x, sq_y, sq_w, sq_h, pitch, color,
>> + &presumed_offset);
>> +
>> + disable_all_screens(&ms_data);
>> + igt_assert(wait_for_suspended());
>> +
>> + memset(cpu_buf, 0, dst_size);
>> + gem_read(drm_fd, handle, 0, cpu_buf, dst_size);
>> + for (y = 0; y < 128; y++) {
>> + for (x = 0; x < 128; x++) {
>> + uint32_t px = cpu_buf[y * 128 + x];
>> +
>> + if (y >= sq_y && y < (sq_y + sq_h) &&
>> + x >= sq_x && x < (sq_x + sq_w))
>> + igt_assert(px == color);
>> + else
>> + igt_assert(px == 0);
>> + }
>> + }
>> +
>> + gem_close(drm_fd, handle);
>> +
>> + free(cpu_buf);
>> +}
>> +
>> +/* Assuming execbuf already works, let's see what happens when we force many
>> + * suspend/resume cycles with commands. */
>> +static void gem_execbuf_stress_subtest(void)
>> +{
>> + int i;
>> + int max = quick ? 5 : 50;
>> + int batch_size = 4 * sizeof(uint32_t);
>> + uint32_t batch_buf[batch_size];
>> + uint32_t handle;
>> + struct drm_i915_gem_execbuffer2 execbuf = {};
>> + struct drm_i915_gem_exec_object2 objs[1] = {{}};
>> +
>> + i = 0;
>> + batch_buf[i++] = MI_NOOP;
>> + batch_buf[i++] = MI_NOOP;
>> + batch_buf[i++] = MI_BATCH_BUFFER_END;
>> + batch_buf[i++] = MI_NOOP;
>> + igt_assert(i * sizeof(uint32_t) == batch_size);
>> +
>> + disable_all_screens(&ms_data);
>> + igt_assert(wait_for_suspended());
>> +
>> + handle = gem_create(drm_fd, batch_size);
>> + gem_write(drm_fd, handle, 0, batch_buf, batch_size);
>> +
>> + objs[0].handle = handle;
>> +
>> + execbuf.buffers_ptr = (uint64_t) objs;
>> + execbuf.buffer_count = 1;
>> + execbuf.batch_len = batch_size;
>> + execbuf.flags = I915_EXEC_RENDER;
>> + i915_execbuffer2_set_context_id(execbuf, 0);
>> +
>> + for (i = 0; i < max; i++) {
>> + do_ioctl(drm_fd, DRM_IOCTL_I915_GEM_EXECBUFFER2, &execbuf);
>> + igt_assert(wait_for_suspended());
>> + }
>> +
>> + gem_close(drm_fd, handle);
>> +}
>> +
>> +static void read_full_file(const char *name)
>> +{
>> + int rc, fd;
>> + char buf[128];
>> +
>> + /* Make sure we're suspended! */
>> + igt_assert(wait_for_suspended());
>> +
>> + fd = open(name, O_RDONLY);
>> + if (fd < 0)
>> + return;
>> +
>> + do {
>> + rc = read(fd, buf, ARRAY_SIZE(buf));
>> + } while (rc == ARRAY_SIZE(buf));
>> +
>> + rc = close(fd);
>> + igt_assert(rc == 0);
>> +}
>> +
>> +static void read_files_from_dir(const char *name, int level)
>> +{
>> + DIR *dir;
>> + struct dirent *dirent;
>> + char *full_name;
>> + int rc;
>> +
>> + dir = opendir(name);
>> + igt_assert(dir);
>> +
>> + full_name = malloc(PATH_MAX);
>> +
>> + igt_assert(level < 128);
>> +
>> + while ((dirent = readdir(dir))) {
>> + struct stat stat_buf;
>> +
>> + if (strcmp(dirent->d_name, ".") == 0)
>> + continue;
>> + if (strcmp(dirent->d_name, "..") == 0)
>> + continue;
>> +
>> + snprintf(full_name, PATH_MAX, "%s/%s", name, dirent->d_name);
>> +
>> + rc = lstat(full_name, &stat_buf);
>> + igt_assert(rc == 0);
>> +
>> + if (S_ISDIR(stat_buf.st_mode))
>> + read_files_from_dir(full_name, level + 1);
>> +
>> + if (S_ISREG(stat_buf.st_mode))
>> + read_full_file(full_name);
>> + }
>> +
>> + free(full_name);
>> + closedir(dir);
>> +}
>> +
>> +/* This test will probably pass, with a small chance of hanging the machine in
>> + * case of bugs. Many of the bugs exercised by this patch just result in dmesg
>> + * errors, so a "pass" here should be confirmed by a check on dmesg. */
>> +static void debugfs_read_subtest(void)
>> +{
>> + const char *path = "/sys/kernel/debug/dri/0";
>> + DIR *dir;
>> +
>> + dir = opendir(path);
>> + igt_require_f(dir, "Can't open the debugfs directory\n");
>> + closedir(dir);
>> +
>> + disable_all_screens(&ms_data);
>> + igt_assert(wait_for_suspended());
>> +
>> + read_files_from_dir(path, 0);
>> +}
>> +
>> +/* Read the comment on debugfs_read_subtest(). */
>> +static void sysfs_read_subtest(void)
>> +{
>> + const char *path = "/sys/devices/pci0000:00/0000:00:02.0";
>> + DIR *dir;
>> +
>> + dir = opendir(path);
>> + igt_require_f(dir, "Can't open the sysfs directory\n");
>> + closedir(dir);
>> +
>> + disable_all_screens(&ms_data);
>> + igt_assert(wait_for_suspended());
>> +
>> + read_files_from_dir(path, 0);
>> +}
>> +
>> +/* Make sure we don't suspend when we have the i915_forcewake_user file open. */
>> +static void debugfs_forcewake_user_subtest(void)
>> +{
>> + int fd, rc;
>> +
>> + igt_require(!(IS_GEN2(ms_data.devid) || IS_GEN3(ms_data.devid) ||
>> + IS_GEN4(ms_data.devid) || IS_GEN5(ms_data.devid)));
>> +
>> + disable_all_screens(&ms_data);
>> + igt_assert(wait_for_suspended());
>> +
>> + fd = open("/sys/kernel/debug/dri/0/i915_forcewake_user", O_RDONLY);
>> + igt_require(fd);
>> +
>> + igt_assert(!wait_for_suspended());
>> +
>> + rc = close(fd);
>> + igt_assert(rc == 0);
>> +
>> + igt_assert(wait_for_suspended());
>> +}
>> +
>> +int main(int argc, char *argv[])
>> +{
>> + /* The --quick option makes the stress tests not so stressful. */
>> + if (argc > 1 && strcmp(argv[1], "--quick") == 0)
>> + quick = true;
>> +
>> + igt_subtest_init(argc, argv);
>> +
>> + igt_fixture
>> + setup_environment();
>> +
>> + igt_subtest("basic")
>> + basic_subtest();
>> + igt_subtest("drm-resources")
>> + drm_resources_subtest();
>> + igt_subtest("i2c")
>> + i2c_subtest();
>> + igt_subtest("pc8")
>> + pc8_subtest();
>> + igt_subtest("gem-mmap")
>> + gem_mmap_subtest();
>> + igt_subtest("gem-pread")
>> + gem_pread_subtest();
>> + igt_subtest("gem-execbuf")
>> + gem_execbuf_subtest();
>> + igt_subtest("modeset-stress-lpsp")
>> + modeset_stress(true);
>> + igt_subtest("modeset-stress-non-lpsp")
>> + modeset_stress(false);
>> + igt_subtest("gem-execbuf-stress")
>> + gem_execbuf_stress_subtest();
>> + igt_subtest("debugfs-read")
>> + debugfs_read_subtest();
>> + igt_subtest("debugfs-forcewake-user")
>> + debugfs_forcewake_user_subtest();
>> + igt_subtest("sysfs-read")
>> + sysfs_read_subtest();
>> +
>> + igt_fixture
>> + teardown_environment();
>> +
>> + igt_exit();
>> +}
>> --
>> 1.8.3.1
>>
>> _______________________________________________
>> Intel-gfx mailing list
>> Intel-gfx@lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
>
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> +41 (0) 79 365 57 48 - http://blog.ffwll.ch
--
Paulo Zanoni
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [RFC 0/6] Haswell runtime PM support + D3
2013-10-22 19:30 [RFC 0/6] Haswell runtime PM support + D3 Paulo Zanoni
` (6 preceding siblings ...)
2013-10-25 13:44 ` [RFC i-g-t] tests: add runtime_pm Paulo Zanoni
@ 2013-10-28 13:09 ` Imre Deak
2013-10-28 16:10 ` Daniel Vetter
7 siblings, 1 reply; 25+ messages in thread
From: Imre Deak @ 2013-10-28 13:09 UTC (permalink / raw)
To: Paulo Zanoni; +Cc: intel-gfx, Paulo Zanoni
[-- Attachment #1.1: Type: text/plain, Size: 5289 bytes --]
Hi,
On Tue, 2013-10-22 at 17:30 -0200, Paulo Zanoni wrote:
> From: Paulo Zanoni <paulo.r.zanoni@intel.com>
>
> Hi
>
> This RFC series adds runtime PM support on Haswell. The current implementation
> puts the device in the PCI D3cold state when we decide to sleep. It uses the
> same refcount+timeout idea from the PC8 code, but now through the Kernel runtime
> PM infrastructure.
>
> I saw Jesse and Imre are still playing with the power wells on Baytrail, so I
> thought maybe they would take some time to do the whole D3 + runtime_pm thing,
> so I decided to ressurrect some code I had, test it and send anyway.
>
> The basic idea of this series is that it adds some functions that should be
> reused when we add support for other platforms, but OTOH it completely relies on
> the already-implemented PC8 support for saving+restoring everything on Haswell.
> So even though the code is ready for Haswell, it's not ready for !Haswell.
>
> So we'll only reach the zero refcount after we enable PC8, and we'll increment
> the refcount just before disabling PC8. I thought we would need more register
> save-restore code, but it seems the current save-restore code from PC8 is enough
> to keep everything going. All I needed was to add some more put/get calls to
> keep the device awake when needed.
>
> Now here's the reason why this is still an RFC: I just added put/get calls to
> the places I spotted problems while doing my basic tests. I basically tested
> runtime PM while running Gnome and Xfce, and I also tested sysfs and debugfs. I
> still didn't write the full test suite I've been promising to write: this will
> be my next step. OTOH, the runtime PM support is disabled by default, so merging
> these patches shouldn't really hurt.
>
> But our driver has so many entry points that I just don't think we'll ever be
> confident that we always wake up from D3 when needed, and I don't think we'll be
> able to write a single test case that tests everything. So I fear this feature
> will be just like the "unclaimed register" checking code: we'll be stuck on a
> break-bisect-fix loop forever. And considering most people won't ever run the
> runtime PM code, we may stay long periods of time without noticing the broken
> state.
>
> When we add support for BYT and other platforms we will have to rearrange the
> PC8 code so that we move some things out of it to some common PM infrastructure
> (i.e, dev_priv->pm). The first things I can think of are the code that counts
> the current number of used CRTCs and the code that handles pc8.gpu_idle.
>
> The interfaces for configuring the runtime PM support are the standard runtime
> PM interfaces. Go to /sys/bus/pci/devices/0000\:00\:02.0/power/ and have some
> fun. I suggest you to "echo 5 > autosuspend_delay_ms" and boot with
> "i915.pc8_timeout=5" if you really want to test things.
>
> Another thing which we could discuss is if we really want to keep PC8 and D3 as
> separate features. Today they're separate features, where D3 is a "deeper" step
> that can be disabled while PC8 is still enabled. This creates the problem that
> we need to separately test/validate PC8-only and PC8+D3 (and possibly D3-only in
> the future, depending on how we rearrange code). Since I can't really think of a
> case where we would want PC8 but not D3, maybe we should kill the "PC8 without
> D3" option.
>
> Opinions? Bikesheds? Does this really looks like it can be resued on BYT? Jesse?
> Imre?
I read through the patchset, in general it looks good to me. I agree it
would make sense to merge already now at least the parts that would
enable others to sort out platform specific details like HW
state/restore for !HSW.
I'm also wondering where to place the get/put calls, here is my 2 cents
about this, please correct me where I'm wrong: With RPM we want to
control whether the whole device is in D0 or D3/D3_cold state, which is
the coarsest granularity. While in D0 we also need a finer granularity
control to gate as many clocks and power domains as we can (while others
remain active preventing D3/D3_cold). For this we enable/disable PC8/RC6
to let the HW do automatic gating, or we do this manually for example
through power domains control.
If this is correct we should try our best to only do fine grain control
at the highest level and do RPM get/put only as a consequence. For
example modeset code would only enable/disable whatever power domains
are needed for the current mode and the power domain framework would do
the RPM get/put. Your PC8 changes in your patchset are in accordance
with this: at the highest level we only enable/disable PC8 which will do
an RPM get/put if needed. Debugfs and other places where we need
register access: atm you do only an RPM get/put around these, but this
just ensures we are in D0, not that we have the needed power
domains/clocks enabled. I guess this works for HSW, but it may not work
on other platforms. One solution could be to do the same here as for
modeset, we just enable/disable clocks and power domains around these
places as needed and adjust the RPM refcount only as a consequence.
I also have a few nitpicks about the patches, sending those inlined.
--Imre
[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 490 bytes --]
[-- Attachment #2: Type: text/plain, Size: 159 bytes --]
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [RFC 1/6] drm/i915: add initial Runtime PM functions
2013-10-22 19:30 ` [RFC 1/6] drm/i915: add initial Runtime PM functions Paulo Zanoni
@ 2013-10-28 13:21 ` Imre Deak
2013-11-06 20:32 ` Paulo Zanoni
0 siblings, 1 reply; 25+ messages in thread
From: Imre Deak @ 2013-10-28 13:21 UTC (permalink / raw)
To: Paulo Zanoni; +Cc: intel-gfx, Paulo Zanoni
[-- Attachment #1.1: Type: text/plain, Size: 10696 bytes --]
On Tue, 2013-10-22 at 17:30 -0200, Paulo Zanoni wrote:
> From: Paulo Zanoni <paulo.r.zanoni@intel.com>
>
> This patch adds the initial infrastructure to allow a Runtime PM
> implementation that sets the device to its D3 state. The patch just
> adds the necessary callbacks and the initial infrastructure.
>
> We still don't have any platform that actually uses this
> infrastructure, we still don't call get/put in all the places we need
> to, and we don't have any function to save/restore the state of the
> registers. This is not a problem since no platform uses the code added
> by this patch. We have a few people simultaneously working on runtime
> PM, so this initial code could help everybody make their plans.
>
> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
> ---
> drivers/gpu/drm/i915/i915_dma.c | 38 +++++++++++++++++++++++++++++++++
> drivers/gpu/drm/i915/i915_drv.c | 42 +++++++++++++++++++++++++++++++++++++
> drivers/gpu/drm/i915/i915_drv.h | 7 +++++++
> drivers/gpu/drm/i915/intel_drv.h | 2 ++
> drivers/gpu/drm/i915/intel_pm.c | 26 +++++++++++++++++++++++
> drivers/gpu/drm/i915/intel_uncore.c | 9 ++++++++
> 6 files changed, 124 insertions(+)
>
> diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
> index fd848ef..6aa044e 100644
> --- a/drivers/gpu/drm/i915/i915_dma.c
> +++ b/drivers/gpu/drm/i915/i915_dma.c
> @@ -42,6 +42,8 @@
> #include <linux/vga_switcheroo.h>
> #include <linux/slab.h>
> #include <acpi/video.h>
> +#include <linux/pm.h>
> +#include <linux/pm_runtime.h>
>
> #define LP_RING(d) (&((struct drm_i915_private *)(d))->ring[RCS])
>
> @@ -1449,6 +1451,38 @@ static void i915_dump_device_info(struct drm_i915_private *dev_priv)
> #undef SEP_COMMA
> }
>
> +static void i915_init_runtime_pm(struct drm_i915_private *dev_priv)
> +{
> + struct drm_device *dev = dev_priv->dev;
> + struct device *device = &dev->pdev->dev;
> +
> + dev_priv->pm.suspended = false;
> +
> + if (!HAS_RUNTIME_PM(dev))
> + return;
> +
> + pm_runtime_set_active(device);
> + pm_runtime_enable(device);
> +
> + pm_runtime_set_autosuspend_delay(device, 10000); /* 10s */
> + pm_runtime_mark_last_busy(device);
> + pm_runtime_use_autosuspend(device);
> + pm_runtime_allow(device);
This shouldn't be needed as we get here already with an allowed state.
It's not a problem as it's just a nop here, but imo it's confusing that
we don't have the corresponding pm_runtime_forbid() in
i915_fini_runtime_pm().
> +}
> +
> +static void i915_fini_runtime_pm(struct drm_i915_private *dev_priv)
> +{
> + struct drm_device *dev = dev_priv->dev;
> + struct device *device = &dev->pdev->dev;
> +
> + if (!HAS_RUNTIME_PM(dev))
> + return;
> +
> + /* Make sure we're not suspended first. */
> + pm_runtime_get_sync(device);
> + pm_runtime_disable(device);
> +}
> +
Could we have the above functions in intel_pm.c?
> /**
> * i915_driver_load - setup chip and create an initial config
> * @dev: DRM device
> @@ -1664,6 +1698,8 @@ int i915_driver_load(struct drm_device *dev, unsigned long flags)
> if (IS_GEN5(dev))
> intel_gpu_ips_init(dev_priv);
>
> + i915_init_runtime_pm(dev_priv);
> +
> return 0;
>
> out_power_well:
> @@ -1704,6 +1740,8 @@ int i915_driver_unload(struct drm_device *dev)
> struct drm_i915_private *dev_priv = dev->dev_private;
> int ret;
>
> + i915_fini_runtime_pm(dev_priv);
> +
> intel_gpu_ips_teardown();
>
> if (HAS_POWER_WELL(dev)) {
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index 85ae0dc..08fc7ea 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -469,6 +469,8 @@ static int i915_drm_freeze(struct drm_device *dev)
> struct drm_i915_private *dev_priv = dev->dev_private;
> struct drm_crtc *crtc;
>
> + intel_runtime_pm_get(dev_priv);
> +
> /* ignore lid events during suspend */
> mutex_lock(&dev_priv->modeset_restore_lock);
> dev_priv->modeset_restore = MODESET_SUSPENDED;
> @@ -653,6 +655,8 @@ static int __i915_drm_thaw(struct drm_device *dev, bool restore_gtt_mappings)
> mutex_lock(&dev_priv->modeset_restore_lock);
> dev_priv->modeset_restore = MODESET_DONE;
> mutex_unlock(&dev_priv->modeset_restore_lock);
> +
> + intel_runtime_pm_put(dev_priv);
> return error;
> }
>
> @@ -880,6 +884,42 @@ static int i915_pm_poweroff(struct device *dev)
> return i915_drm_freeze(drm_dev);
> }
>
> +static int i915_runtime_suspend(struct device *device)
> +{
> + struct pci_dev *pdev = to_pci_dev(device);
> + struct drm_device *dev = pci_get_drvdata(pdev);
> + struct drm_i915_private *dev_priv = dev->dev_private;
> +
> + WARN_ON(!HAS_RUNTIME_PM(dev));
> +
> + DRM_DEBUG_KMS("Suspending device\n");
> +
> + dev_priv->pm.suspended = true;
> +
> + pci_save_state(pdev);
> + pci_set_power_state(pdev, PCI_D3cold);
> +
> + return 0;
> +}
> +
> +static int i915_runtime_resume(struct device *device)
> +{
> + struct pci_dev *pdev = to_pci_dev(device);
> + struct drm_device *dev = pci_get_drvdata(pdev);
> + struct drm_i915_private *dev_priv = dev->dev_private;
> +
> + WARN_ON(!HAS_RUNTIME_PM(dev));
> +
> + DRM_DEBUG_KMS("Resuming device\n");
> +
> + pci_set_power_state(pdev, PCI_D0);
> + pci_restore_state(pdev);
> +
> + dev_priv->pm.suspended = false;
> +
> + return 0;
> +}
> +
> static const struct dev_pm_ops i915_pm_ops = {
> .suspend = i915_pm_suspend,
> .resume = i915_pm_resume,
> @@ -887,6 +927,8 @@ static const struct dev_pm_ops i915_pm_ops = {
> .thaw = i915_pm_thaw,
> .poweroff = i915_pm_poweroff,
> .restore = i915_pm_resume,
> + .runtime_suspend = i915_runtime_suspend,
> + .runtime_resume = i915_runtime_resume,
> };
>
> static const struct vm_operations_struct i915_gem_vm_ops = {
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 80957ca..74f2b5d 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1230,6 +1230,10 @@ struct i915_package_c8 {
> } regsave;
> };
>
> +struct i915_runtime_pm {
> + bool suspended;
> +};
> +
> enum intel_pipe_crc_source {
> INTEL_PIPE_CRC_SOURCE_NONE,
> INTEL_PIPE_CRC_SOURCE_PLANE1,
> @@ -1462,6 +1466,8 @@ typedef struct drm_i915_private {
>
> struct i915_package_c8 pc8;
>
> + struct i915_runtime_pm pm;
> +
> /* Old dri1 support infrastructure, beware the dragons ya fools entering
> * here! */
> struct i915_dri1_state dri1;
> @@ -1779,6 +1785,7 @@ struct drm_i915_file_private {
> #define HAS_POWER_WELL(dev) (IS_HASWELL(dev))
> #define HAS_FPGA_DBG_UNCLAIMED(dev) (INTEL_INFO(dev)->has_fpga_dbg)
> #define HAS_PSR(dev) (IS_HASWELL(dev))
> +#define HAS_RUNTIME_PM(dev) false
>
> #define INTEL_PCH_DEVICE_ID_MASK 0xff00
> #define INTEL_PCH_IBX_DEVICE_ID_TYPE 0x3b00
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index af1553c..80b5a80 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -840,6 +840,8 @@ void gen6_rps_idle(struct drm_i915_private *dev_priv);
> void gen6_rps_boost(struct drm_i915_private *dev_priv);
> void intel_aux_display_runtime_get(struct drm_i915_private *dev_priv);
> void intel_aux_display_runtime_put(struct drm_i915_private *dev_priv);
> +void intel_runtime_pm_get(struct drm_i915_private *dev_priv);
> +void intel_runtime_pm_put(struct drm_i915_private *dev_priv);
> void ilk_wm_get_hw_state(struct drm_device *dev);
>
>
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 3e140ab..a973f35 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -31,6 +31,7 @@
> #include "../../../platform/x86/intel_ips.h"
> #include <linux/module.h>
> #include <drm/i915_powerwell.h>
> +#include <linux/pm_runtime.h>
>
> /**
> * RC6 is a special power stage which allows the GPU to enter an very
> @@ -5779,6 +5780,31 @@ void intel_aux_display_runtime_put(struct drm_i915_private *dev_priv)
> hsw_enable_package_c8(dev_priv);
> }
>
> +void intel_runtime_pm_get(struct drm_i915_private *dev_priv)
> +{
> + struct drm_device *dev = dev_priv->dev;
> + struct device *device = &dev->pdev->dev;
> +
> + if (!HAS_RUNTIME_PM(dev))
> + return;
> +
> + pm_runtime_mark_last_busy(device);
What's the point of calling this here? We call pm_runtime_get below,
which will prevent an auto suspend anyway and we call
pm_runtime_mark_last_busy() then intel_runtime_pm_put().
> + pm_runtime_get(device);
> + pm_runtime_barrier(device);
Why not using pm_runtime_get_sync() instead of the above 2 calls?
--Imre
> +}
> +
> +void intel_runtime_pm_put(struct drm_i915_private *dev_priv)
> +{
> + struct drm_device *dev = dev_priv->dev;
> + struct device *device = &dev->pdev->dev;
> +
> + if (!HAS_RUNTIME_PM(dev))
> + return;
> +
> + pm_runtime_mark_last_busy(device);
> + pm_runtime_put_autosuspend(device);
> +}
> +
> /* Set up chip specific power management-related functions */
> void intel_init_pm(struct drm_device *dev)
> {
> diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c
> index f6fae35..f5a2a6d 100644
> --- a/drivers/gpu/drm/i915/intel_uncore.c
> +++ b/drivers/gpu/drm/i915/intel_uncore.c
> @@ -343,6 +343,13 @@ hsw_unclaimed_reg_check(struct drm_i915_private *dev_priv, u32 reg)
> }
> }
>
> +static void
> +assert_device_not_suspended(struct drm_i915_private *dev_priv)
> +{
> + WARN(HAS_RUNTIME_PM(dev_priv->dev) && dev_priv->pm.suspended,
> + "Device suspended\n");
> +}
> +
> #define REG_READ_HEADER(x) \
> unsigned long irqflags; \
> u##x val = 0; \
> @@ -435,6 +442,7 @@ gen6_write##x(struct drm_i915_private *dev_priv, off_t reg, u##x val, bool trace
> if (NEEDS_FORCE_WAKE((dev_priv), (reg))) { \
> __fifo_ret = __gen6_gt_wait_for_fifo(dev_priv); \
> } \
> + assert_device_not_suspended(dev_priv); \
> __raw_i915_write##x(dev_priv, reg, val); \
> if (unlikely(__fifo_ret)) { \
> gen6_gt_check_fifodbg(dev_priv); \
> @@ -450,6 +458,7 @@ hsw_write##x(struct drm_i915_private *dev_priv, off_t reg, u##x val, bool trace)
> if (NEEDS_FORCE_WAKE((dev_priv), (reg))) { \
> __fifo_ret = __gen6_gt_wait_for_fifo(dev_priv); \
> } \
> + assert_device_not_suspended(dev_priv); \
> hsw_unclaimed_reg_clear(dev_priv, reg); \
> __raw_i915_write##x(dev_priv, reg, val); \
> if (unlikely(__fifo_ret)) { \
[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 490 bytes --]
[-- Attachment #2: Type: text/plain, Size: 159 bytes --]
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [RFC 3/6] drm/i915: add enable_runtime_pm option
2013-10-22 19:30 ` [RFC 3/6] drm/i915: add enable_runtime_pm option Paulo Zanoni
@ 2013-10-28 13:27 ` Imre Deak
2013-11-04 21:36 ` Ville Syrjälä
0 siblings, 1 reply; 25+ messages in thread
From: Imre Deak @ 2013-10-28 13:27 UTC (permalink / raw)
To: Paulo Zanoni; +Cc: intel-gfx, Paulo Zanoni
[-- Attachment #1.1: Type: text/plain, Size: 4582 bytes --]
On Tue, 2013-10-22 at 17:30 -0200, Paulo Zanoni wrote:
> From: Paulo Zanoni <paulo.r.zanoni@intel.com>
>
> And leave it off by default. We have way too many driver entry points,
> we can't assume this will work without regressions without tons of
> testing first. This option allows people to test and fix the problems.
>
> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
> ---
> drivers/gpu/drm/i915/i915_dma.c | 4 ++--
> drivers/gpu/drm/i915/i915_drv.c | 8 ++++++--
> drivers/gpu/drm/i915/i915_drv.h | 1 +
> drivers/gpu/drm/i915/intel_pm.c | 4 ++--
> 4 files changed, 11 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
> index 6aa044e..dd4f424 100644
> --- a/drivers/gpu/drm/i915/i915_dma.c
> +++ b/drivers/gpu/drm/i915/i915_dma.c
> @@ -1458,7 +1458,7 @@ static void i915_init_runtime_pm(struct drm_i915_private *dev_priv)
>
> dev_priv->pm.suspended = false;
>
> - if (!HAS_RUNTIME_PM(dev))
> + if (!HAS_RUNTIME_PM(dev) || !i915_enable_runtime_pm)
> return;
>
> pm_runtime_set_active(device);
> @@ -1475,7 +1475,7 @@ static void i915_fini_runtime_pm(struct drm_i915_private *dev_priv)
> struct drm_device *dev = dev_priv->dev;
> struct device *device = &dev->pdev->dev;
>
> - if (!HAS_RUNTIME_PM(dev))
> + if (!HAS_RUNTIME_PM(dev) || !i915_enable_runtime_pm)
> return;
>
> /* Make sure we're not suspended first. */
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index a999a3f..c75b78f 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -154,6 +154,10 @@ module_param_named(prefault_disable, i915_prefault_disable, bool, 0600);
> MODULE_PARM_DESC(prefault_disable,
> "Disable page prefaulting for pread/pwrite/reloc (default:false). For developers only.");
>
> +int i915_enable_runtime_pm __read_mostly = 0;
> +module_param_named(enable_runtime_pm, i915_enable_runtime_pm, int, 0600);
> +MODULE_PARM_DESC(enable_runtime_pm, "Enable runtime PM on supported platforms (default: disabled)");
> +
> static struct drm_driver driver;
> extern int intel_agp_enabled;
>
> @@ -890,7 +894,7 @@ static int i915_runtime_suspend(struct device *device)
> struct drm_device *dev = pci_get_drvdata(pdev);
> struct drm_i915_private *dev_priv = dev->dev_private;
>
> - WARN_ON(!HAS_RUNTIME_PM(dev));
> + WARN_ON(!HAS_RUNTIME_PM(dev) || !i915_enable_runtime_pm);
>
> DRM_DEBUG_KMS("Suspending device\n");
>
> @@ -909,7 +913,7 @@ static int i915_runtime_resume(struct device *device)
> struct drm_device *dev = pci_get_drvdata(pdev);
> struct drm_i915_private *dev_priv = dev->dev_private;
>
> - WARN_ON(!HAS_RUNTIME_PM(dev));
> + WARN_ON(!HAS_RUNTIME_PM(dev) || !i915_enable_runtime_pm);
This could a problem if someone disables runtime_pm through sysfs while
the device is suspended. One solution would be just to do a get/put in
the handler of i915_enable_runtime_pm and not check for it afterwards.
--Imre
>
> DRM_DEBUG_KMS("Resuming device\n");
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 74f2b5d..73ebb9e 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1831,6 +1831,7 @@ extern bool i915_fastboot __read_mostly;
> extern int i915_enable_pc8 __read_mostly;
> extern int i915_pc8_timeout __read_mostly;
> extern bool i915_prefault_disable __read_mostly;
> +extern int i915_enable_runtime_pm __read_mostly;
>
> extern int i915_suspend(struct drm_device *dev, pm_message_t state);
> extern int i915_resume(struct drm_device *dev);
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index a973f35..617e934 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -5785,7 +5785,7 @@ void intel_runtime_pm_get(struct drm_i915_private *dev_priv)
> struct drm_device *dev = dev_priv->dev;
> struct device *device = &dev->pdev->dev;
>
> - if (!HAS_RUNTIME_PM(dev))
> + if (!HAS_RUNTIME_PM(dev) || !i915_enable_runtime_pm)
> return;
>
> pm_runtime_mark_last_busy(device);
> @@ -5798,7 +5798,7 @@ void intel_runtime_pm_put(struct drm_i915_private *dev_priv)
> struct drm_device *dev = dev_priv->dev;
> struct device *device = &dev->pdev->dev;
>
> - if (!HAS_RUNTIME_PM(dev))
> + if (!HAS_RUNTIME_PM(dev) || !i915_enable_runtime_pm)
> return;
>
> pm_runtime_mark_last_busy(device);
[-- Attachment #1.2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 490 bytes --]
[-- Attachment #2: Type: text/plain, Size: 159 bytes --]
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [RFC i-g-t] tests: add runtime_pm
2013-10-28 12:20 ` Paulo Zanoni
@ 2013-10-28 16:05 ` Daniel Vetter
2013-11-04 21:40 ` Ville Syrjälä
1 sibling, 0 replies; 25+ messages in thread
From: Daniel Vetter @ 2013-10-28 16:05 UTC (permalink / raw)
To: Paulo Zanoni; +Cc: Intel Graphics Development, Paulo Zanoni
On Mon, Oct 28, 2013 at 10:20:56AM -0200, Paulo Zanoni wrote:
> 2013/10/27 Daniel Vetter <daniel@ffwll.ch>:
> > On Fri, Oct 25, 2013 at 11:44:05AM -0200, Paulo Zanoni wrote:
> >> From: Paulo Zanoni <paulo.r.zanoni@intel.com>
> >>
> >> This test is based on pc8.c. It copies most of the tests from pc8.c,
> >> but it depends on runtime PM status changes (parsed from sysfs)
> >> instead of PC8 residency changes (parsed from the MSR registers).
> >> There's also a test that checks for PC8 residency.
> >>
> >> For now, runtime PM and PC8 are different features, so having 2 test
> >> suites makes sense. In the future we'll merge both, so we'll only get
> >> PC8 when runtime PM is enabled, so we'll just kill pc8.c and keep
> >> using runtime_pm.c.
> >>
> >> Changes compared to pc8.c:
> >> - We now look at the runtime PM status instead of PC8 residencies
> >> - Added more GEM tests (mmap, pread, execbuf, stress tests)
> >> - Added LPSP and non-LPSP tests
> >> - Added tests fro sysfs and debugfs files
> >> - Added a test specifically for PC8
> >>
> >> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
> >
> > Since the actual tests we're running are so similar I prefer if we merge
> > all the runtime pm tests in one file. It makes testcase maintaince (and
> > bufixing, that happens) much easier. I guess a struct per runtime pm
> > method (pc8, D3, ...) with a few vfuncs should get things going. The
> > overall test would loop over all the pm methods and try to set things up.
> > Then loop over all subtests and either skip them all (if that particular
> > runtime pm method isn't supported) or just run them.
> >
> > We've had a few other case of massive copy&pasting in i-g-t and in the
> > past few months I've merged most of them back again.
>
> At this moment I'm really leaning towards merging PC8 and D3 into a
> single feature, so it won't be possible to test them in separate
> anymore. With this, we'd just have runtime_pm.c and we'd completely
> kill pc8.c.
Yeah, I guess for hsw this makes sense. But it looks like we'll have all
kinds of different runtime pm features with all possible cuts between the
different domains going forward. Hence why I think a bit a more
generalized runtime pm test framework might be useful.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [RFC 0/6] Haswell runtime PM support + D3
2013-10-28 13:09 ` [RFC 0/6] Haswell runtime PM support + D3 Imre Deak
@ 2013-10-28 16:10 ` Daniel Vetter
2013-10-28 20:02 ` Jesse Barnes
0 siblings, 1 reply; 25+ messages in thread
From: Daniel Vetter @ 2013-10-28 16:10 UTC (permalink / raw)
To: Imre Deak; +Cc: intel-gfx, Paulo Zanoni
On Mon, Oct 28, 2013 at 03:09:11PM +0200, Imre Deak wrote:
> Hi,
>
> On Tue, 2013-10-22 at 17:30 -0200, Paulo Zanoni wrote:
> > From: Paulo Zanoni <paulo.r.zanoni@intel.com>
> >
> > Hi
> >
> > This RFC series adds runtime PM support on Haswell. The current implementation
> > puts the device in the PCI D3cold state when we decide to sleep. It uses the
> > same refcount+timeout idea from the PC8 code, but now through the Kernel runtime
> > PM infrastructure.
> >
> > I saw Jesse and Imre are still playing with the power wells on Baytrail, so I
> > thought maybe they would take some time to do the whole D3 + runtime_pm thing,
> > so I decided to ressurrect some code I had, test it and send anyway.
> >
> > The basic idea of this series is that it adds some functions that should be
> > reused when we add support for other platforms, but OTOH it completely relies on
> > the already-implemented PC8 support for saving+restoring everything on Haswell.
> > So even though the code is ready for Haswell, it's not ready for !Haswell.
> >
> > So we'll only reach the zero refcount after we enable PC8, and we'll increment
> > the refcount just before disabling PC8. I thought we would need more register
> > save-restore code, but it seems the current save-restore code from PC8 is enough
> > to keep everything going. All I needed was to add some more put/get calls to
> > keep the device awake when needed.
> >
> > Now here's the reason why this is still an RFC: I just added put/get calls to
> > the places I spotted problems while doing my basic tests. I basically tested
> > runtime PM while running Gnome and Xfce, and I also tested sysfs and debugfs. I
> > still didn't write the full test suite I've been promising to write: this will
> > be my next step. OTOH, the runtime PM support is disabled by default, so merging
> > these patches shouldn't really hurt.
> >
> > But our driver has so many entry points that I just don't think we'll ever be
> > confident that we always wake up from D3 when needed, and I don't think we'll be
> > able to write a single test case that tests everything. So I fear this feature
> > will be just like the "unclaimed register" checking code: we'll be stuck on a
> > break-bisect-fix loop forever. And considering most people won't ever run the
> > runtime PM code, we may stay long periods of time without noticing the broken
> > state.
> >
> > When we add support for BYT and other platforms we will have to rearrange the
> > PC8 code so that we move some things out of it to some common PM infrastructure
> > (i.e, dev_priv->pm). The first things I can think of are the code that counts
> > the current number of used CRTCs and the code that handles pc8.gpu_idle.
> >
> > The interfaces for configuring the runtime PM support are the standard runtime
> > PM interfaces. Go to /sys/bus/pci/devices/0000\:00\:02.0/power/ and have some
> > fun. I suggest you to "echo 5 > autosuspend_delay_ms" and boot with
> > "i915.pc8_timeout=5" if you really want to test things.
> >
> > Another thing which we could discuss is if we really want to keep PC8 and D3 as
> > separate features. Today they're separate features, where D3 is a "deeper" step
> > that can be disabled while PC8 is still enabled. This creates the problem that
> > we need to separately test/validate PC8-only and PC8+D3 (and possibly D3-only in
> > the future, depending on how we rearrange code). Since I can't really think of a
> > case where we would want PC8 but not D3, maybe we should kill the "PC8 without
> > D3" option.
> >
> > Opinions? Bikesheds? Does this really looks like it can be resued on BYT? Jesse?
> > Imre?
>
> I read through the patchset, in general it looks good to me. I agree it
> would make sense to merge already now at least the parts that would
> enable others to sort out platform specific details like HW
> state/restore for !HSW.
>
> I'm also wondering where to place the get/put calls, here is my 2 cents
> about this, please correct me where I'm wrong: With RPM we want to
> control whether the whole device is in D0 or D3/D3_cold state, which is
> the coarsest granularity. While in D0 we also need a finer granularity
> control to gate as many clocks and power domains as we can (while others
> remain active preventing D3/D3_cold). For this we enable/disable PC8/RC6
> to let the HW do automatic gating, or we do this manually for example
> through power domains control.
>
> If this is correct we should try our best to only do fine grain control
> at the highest level and do RPM get/put only as a consequence. For
> example modeset code would only enable/disable whatever power domains
> are needed for the current mode and the power domain framework would do
> the RPM get/put. Your PC8 changes in your patchset are in accordance
> with this: at the highest level we only enable/disable PC8 which will do
> an RPM get/put if needed. Debugfs and other places where we need
> register access: atm you do only an RPM get/put around these, but this
> just ensures we are in D0, not that we have the needed power
> domains/clocks enabled. I guess this works for HSW, but it may not work
> on other platforms. One solution could be to do the same here as for
> modeset, we just enable/disable clocks and power domains around these
> places as needed and adjust the RPM refcount only as a consequence.
I haven't read through the patches, but overall we should only get/put the
most fine-grained power domain possible. If there's nesting going on (e.g.
nested power wells, or different runtime pm states like pc8/D3/...) then
the fine-grained power domains should in turn grab references for the next
level up. So if someone grabs the aux display references to do edid reads
(which on hsw disables pc8), this will also block D3. Same if we need one
of the display power wells, that should in turn prevent pc8 (again on
hsw).
I know that the current code isn't there at all. But long-term this should
be the direction imo, otherwise we'll have a giant mess between the
runtime pm handling and the get/put callsites.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [RFC 0/6] Haswell runtime PM support + D3
2013-10-28 16:10 ` Daniel Vetter
@ 2013-10-28 20:02 ` Jesse Barnes
0 siblings, 0 replies; 25+ messages in thread
From: Jesse Barnes @ 2013-10-28 20:02 UTC (permalink / raw)
To: Daniel Vetter; +Cc: intel-gfx, Paulo Zanoni
On Mon, 28 Oct 2013 17:10:02 +0100
Daniel Vetter <daniel@ffwll.ch> wrote:
> On Mon, Oct 28, 2013 at 03:09:11PM +0200, Imre Deak wrote:
> > Hi,
> >
> > On Tue, 2013-10-22 at 17:30 -0200, Paulo Zanoni wrote:
> > > From: Paulo Zanoni <paulo.r.zanoni@intel.com>
> > >
> > > Hi
> > >
> > > This RFC series adds runtime PM support on Haswell. The current implementation
> > > puts the device in the PCI D3cold state when we decide to sleep. It uses the
> > > same refcount+timeout idea from the PC8 code, but now through the Kernel runtime
> > > PM infrastructure.
> > >
> > > I saw Jesse and Imre are still playing with the power wells on Baytrail, so I
> > > thought maybe they would take some time to do the whole D3 + runtime_pm thing,
> > > so I decided to ressurrect some code I had, test it and send anyway.
> > >
> > > The basic idea of this series is that it adds some functions that should be
> > > reused when we add support for other platforms, but OTOH it completely relies on
> > > the already-implemented PC8 support for saving+restoring everything on Haswell.
> > > So even though the code is ready for Haswell, it's not ready for !Haswell.
> > >
> > > So we'll only reach the zero refcount after we enable PC8, and we'll increment
> > > the refcount just before disabling PC8. I thought we would need more register
> > > save-restore code, but it seems the current save-restore code from PC8 is enough
> > > to keep everything going. All I needed was to add some more put/get calls to
> > > keep the device awake when needed.
> > >
> > > Now here's the reason why this is still an RFC: I just added put/get calls to
> > > the places I spotted problems while doing my basic tests. I basically tested
> > > runtime PM while running Gnome and Xfce, and I also tested sysfs and debugfs. I
> > > still didn't write the full test suite I've been promising to write: this will
> > > be my next step. OTOH, the runtime PM support is disabled by default, so merging
> > > these patches shouldn't really hurt.
> > >
> > > But our driver has so many entry points that I just don't think we'll ever be
> > > confident that we always wake up from D3 when needed, and I don't think we'll be
> > > able to write a single test case that tests everything. So I fear this feature
> > > will be just like the "unclaimed register" checking code: we'll be stuck on a
> > > break-bisect-fix loop forever. And considering most people won't ever run the
> > > runtime PM code, we may stay long periods of time without noticing the broken
> > > state.
> > >
> > > When we add support for BYT and other platforms we will have to rearrange the
> > > PC8 code so that we move some things out of it to some common PM infrastructure
> > > (i.e, dev_priv->pm). The first things I can think of are the code that counts
> > > the current number of used CRTCs and the code that handles pc8.gpu_idle.
> > >
> > > The interfaces for configuring the runtime PM support are the standard runtime
> > > PM interfaces. Go to /sys/bus/pci/devices/0000\:00\:02.0/power/ and have some
> > > fun. I suggest you to "echo 5 > autosuspend_delay_ms" and boot with
> > > "i915.pc8_timeout=5" if you really want to test things.
> > >
> > > Another thing which we could discuss is if we really want to keep PC8 and D3 as
> > > separate features. Today they're separate features, where D3 is a "deeper" step
> > > that can be disabled while PC8 is still enabled. This creates the problem that
> > > we need to separately test/validate PC8-only and PC8+D3 (and possibly D3-only in
> > > the future, depending on how we rearrange code). Since I can't really think of a
> > > case where we would want PC8 but not D3, maybe we should kill the "PC8 without
> > > D3" option.
> > >
> > > Opinions? Bikesheds? Does this really looks like it can be resued on BYT? Jesse?
> > > Imre?
> >
> > I read through the patchset, in general it looks good to me. I agree it
> > would make sense to merge already now at least the parts that would
> > enable others to sort out platform specific details like HW
> > state/restore for !HSW.
> >
> > I'm also wondering where to place the get/put calls, here is my 2 cents
> > about this, please correct me where I'm wrong: With RPM we want to
> > control whether the whole device is in D0 or D3/D3_cold state, which is
> > the coarsest granularity. While in D0 we also need a finer granularity
> > control to gate as many clocks and power domains as we can (while others
> > remain active preventing D3/D3_cold). For this we enable/disable PC8/RC6
> > to let the HW do automatic gating, or we do this manually for example
> > through power domains control.
> >
> > If this is correct we should try our best to only do fine grain control
> > at the highest level and do RPM get/put only as a consequence. For
> > example modeset code would only enable/disable whatever power domains
> > are needed for the current mode and the power domain framework would do
> > the RPM get/put. Your PC8 changes in your patchset are in accordance
> > with this: at the highest level we only enable/disable PC8 which will do
> > an RPM get/put if needed. Debugfs and other places where we need
> > register access: atm you do only an RPM get/put around these, but this
> > just ensures we are in D0, not that we have the needed power
> > domains/clocks enabled. I guess this works for HSW, but it may not work
> > on other platforms. One solution could be to do the same here as for
> > modeset, we just enable/disable clocks and power domains around these
> > places as needed and adjust the RPM refcount only as a consequence.
>
> I haven't read through the patches, but overall we should only get/put the
> most fine-grained power domain possible. If there's nesting going on (e.g.
> nested power wells, or different runtime pm states like pc8/D3/...) then
> the fine-grained power domains should in turn grab references for the next
> level up. So if someone grabs the aux display references to do edid reads
> (which on hsw disables pc8), this will also block D3. Same if we need one
> of the display power wells, that should in turn prevent pc8 (again on
> hsw).
>
> I know that the current code isn't there at all. But long-term this should
> be the direction imo, otherwise we'll have a giant mess between the
> runtime pm handling and the get/put callsites.
Yep, agreed. Fortunately Paulo has tested this and identified where
the get/puts belong, so now it's mainly a matter of figuring out which
power well needs to be referenced, rather than referencing the whole
device.
On other platforms we may need to add more get/puts if we have
new/different power wells, but let's start with HSW and work from there.
--
Jesse Barnes, Intel Open Source Technology Center
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [RFC 3/6] drm/i915: add enable_runtime_pm option
2013-10-28 13:27 ` Imre Deak
@ 2013-11-04 21:36 ` Ville Syrjälä
2013-11-06 20:04 ` Paulo Zanoni
0 siblings, 1 reply; 25+ messages in thread
From: Ville Syrjälä @ 2013-11-04 21:36 UTC (permalink / raw)
To: Imre Deak; +Cc: intel-gfx, Paulo Zanoni
On Mon, Oct 28, 2013 at 03:27:39PM +0200, Imre Deak wrote:
> On Tue, 2013-10-22 at 17:30 -0200, Paulo Zanoni wrote:
> > From: Paulo Zanoni <paulo.r.zanoni@intel.com>
> >
> > And leave it off by default. We have way too many driver entry points,
> > we can't assume this will work without regressions without tons of
> > testing first. This option allows people to test and fix the problems.
> >
> > Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
> > ---
> > drivers/gpu/drm/i915/i915_dma.c | 4 ++--
> > drivers/gpu/drm/i915/i915_drv.c | 8 ++++++--
> > drivers/gpu/drm/i915/i915_drv.h | 1 +
> > drivers/gpu/drm/i915/intel_pm.c | 4 ++--
> > 4 files changed, 11 insertions(+), 6 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
> > index 6aa044e..dd4f424 100644
> > --- a/drivers/gpu/drm/i915/i915_dma.c
> > +++ b/drivers/gpu/drm/i915/i915_dma.c
> > @@ -1458,7 +1458,7 @@ static void i915_init_runtime_pm(struct drm_i915_private *dev_priv)
> >
> > dev_priv->pm.suspended = false;
> >
> > - if (!HAS_RUNTIME_PM(dev))
> > + if (!HAS_RUNTIME_PM(dev) || !i915_enable_runtime_pm)
> > return;
> >
> > pm_runtime_set_active(device);
> > @@ -1475,7 +1475,7 @@ static void i915_fini_runtime_pm(struct drm_i915_private *dev_priv)
> > struct drm_device *dev = dev_priv->dev;
> > struct device *device = &dev->pdev->dev;
> >
> > - if (!HAS_RUNTIME_PM(dev))
> > + if (!HAS_RUNTIME_PM(dev) || !i915_enable_runtime_pm)
> > return;
> >
> > /* Make sure we're not suspended first. */
> > diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> > index a999a3f..c75b78f 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.c
> > +++ b/drivers/gpu/drm/i915/i915_drv.c
> > @@ -154,6 +154,10 @@ module_param_named(prefault_disable, i915_prefault_disable, bool, 0600);
> > MODULE_PARM_DESC(prefault_disable,
> > "Disable page prefaulting for pread/pwrite/reloc (default:false). For developers only.");
> >
> > +int i915_enable_runtime_pm __read_mostly = 0;
> > +module_param_named(enable_runtime_pm, i915_enable_runtime_pm, int, 0600);
> > +MODULE_PARM_DESC(enable_runtime_pm, "Enable runtime PM on supported platforms (default: disabled)");
> > +
> > static struct drm_driver driver;
> > extern int intel_agp_enabled;
> >
> > @@ -890,7 +894,7 @@ static int i915_runtime_suspend(struct device *device)
> > struct drm_device *dev = pci_get_drvdata(pdev);
> > struct drm_i915_private *dev_priv = dev->dev_private;
> >
> > - WARN_ON(!HAS_RUNTIME_PM(dev));
> > + WARN_ON(!HAS_RUNTIME_PM(dev) || !i915_enable_runtime_pm);
> >
> > DRM_DEBUG_KMS("Suspending device\n");
> >
> > @@ -909,7 +913,7 @@ static int i915_runtime_resume(struct device *device)
> > struct drm_device *dev = pci_get_drvdata(pdev);
> > struct drm_i915_private *dev_priv = dev->dev_private;
> >
> > - WARN_ON(!HAS_RUNTIME_PM(dev));
> > + WARN_ON(!HAS_RUNTIME_PM(dev) || !i915_enable_runtime_pm);
>
> This could a problem if someone disables runtime_pm through sysfs while
> the device is suspended. One solution would be just to do a get/put in
> the handler of i915_enable_runtime_pm and not check for it afterwards.
There's already a standard interface for enabling/disabling runtime PM
under sysfs. Why do we want this custom one?
--
Ville Syrjälä
Intel OTC
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [RFC i-g-t] tests: add runtime_pm
2013-10-28 12:20 ` Paulo Zanoni
2013-10-28 16:05 ` Daniel Vetter
@ 2013-11-04 21:40 ` Ville Syrjälä
2013-11-08 18:19 ` Paulo Zanoni
1 sibling, 1 reply; 25+ messages in thread
From: Ville Syrjälä @ 2013-11-04 21:40 UTC (permalink / raw)
To: Paulo Zanoni; +Cc: Intel Graphics Development, Paulo Zanoni
On Mon, Oct 28, 2013 at 10:20:56AM -0200, Paulo Zanoni wrote:
> 2013/10/27 Daniel Vetter <daniel@ffwll.ch>:
> > On Fri, Oct 25, 2013 at 11:44:05AM -0200, Paulo Zanoni wrote:
> >> From: Paulo Zanoni <paulo.r.zanoni@intel.com>
> >>
> >> This test is based on pc8.c. It copies most of the tests from pc8.c,
> >> but it depends on runtime PM status changes (parsed from sysfs)
> >> instead of PC8 residency changes (parsed from the MSR registers).
> >> There's also a test that checks for PC8 residency.
> >>
> >> For now, runtime PM and PC8 are different features, so having 2 test
> >> suites makes sense. In the future we'll merge both, so we'll only get
> >> PC8 when runtime PM is enabled, so we'll just kill pc8.c and keep
> >> using runtime_pm.c.
> >>
> >> Changes compared to pc8.c:
> >> - We now look at the runtime PM status instead of PC8 residencies
> >> - Added more GEM tests (mmap, pread, execbuf, stress tests)
> >> - Added LPSP and non-LPSP tests
> >> - Added tests fro sysfs and debugfs files
> >> - Added a test specifically for PC8
> >>
> >> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
> >
> > Since the actual tests we're running are so similar I prefer if we merge
> > all the runtime pm tests in one file. It makes testcase maintaince (and
> > bufixing, that happens) much easier. I guess a struct per runtime pm
> > method (pc8, D3, ...) with a few vfuncs should get things going. The
> > overall test would loop over all the pm methods and try to set things up.
> > Then loop over all subtests and either skip them all (if that particular
> > runtime pm method isn't supported) or just run them.
> >
> > We've had a few other case of massive copy&pasting in i-g-t and in the
> > past few months I've merged most of them back again.
>
> At this moment I'm really leaning towards merging PC8 and D3 into a
> single feature, so it won't be possible to test them in separate
> anymore. With this, we'd just have runtime_pm.c and we'd completely
> kill pc8.c.
All this time I've been wondering whether PC8 offers anything
substantial over D3. I think we discussed this in some meeting, and
GTT maps were the only thing that came to my mind. But IIRC you
said that you weren't actually sure whether GTT maps still work in
PC8. Has that been verified or is it still an open question?
--
Ville Syrjälä
Intel OTC
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [RFC 3/6] drm/i915: add enable_runtime_pm option
2013-11-04 21:36 ` Ville Syrjälä
@ 2013-11-06 20:04 ` Paulo Zanoni
0 siblings, 0 replies; 25+ messages in thread
From: Paulo Zanoni @ 2013-11-06 20:04 UTC (permalink / raw)
To: Ville Syrjälä; +Cc: Intel Graphics Development, Paulo Zanoni
2013/11/4 Ville Syrjälä <ville.syrjala@linux.intel.com>:
> On Mon, Oct 28, 2013 at 03:27:39PM +0200, Imre Deak wrote:
>> On Tue, 2013-10-22 at 17:30 -0200, Paulo Zanoni wrote:
>> > From: Paulo Zanoni <paulo.r.zanoni@intel.com>
>> >
>> > And leave it off by default. We have way too many driver entry points,
>> > we can't assume this will work without regressions without tons of
>> > testing first. This option allows people to test and fix the problems.
>> >
>> > Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
>> > ---
>> > drivers/gpu/drm/i915/i915_dma.c | 4 ++--
>> > drivers/gpu/drm/i915/i915_drv.c | 8 ++++++--
>> > drivers/gpu/drm/i915/i915_drv.h | 1 +
>> > drivers/gpu/drm/i915/intel_pm.c | 4 ++--
>> > 4 files changed, 11 insertions(+), 6 deletions(-)
>> >
>> > diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
>> > index 6aa044e..dd4f424 100644
>> > --- a/drivers/gpu/drm/i915/i915_dma.c
>> > +++ b/drivers/gpu/drm/i915/i915_dma.c
>> > @@ -1458,7 +1458,7 @@ static void i915_init_runtime_pm(struct drm_i915_private *dev_priv)
>> >
>> > dev_priv->pm.suspended = false;
>> >
>> > - if (!HAS_RUNTIME_PM(dev))
>> > + if (!HAS_RUNTIME_PM(dev) || !i915_enable_runtime_pm)
>> > return;
>> >
>> > pm_runtime_set_active(device);
>> > @@ -1475,7 +1475,7 @@ static void i915_fini_runtime_pm(struct drm_i915_private *dev_priv)
>> > struct drm_device *dev = dev_priv->dev;
>> > struct device *device = &dev->pdev->dev;
>> >
>> > - if (!HAS_RUNTIME_PM(dev))
>> > + if (!HAS_RUNTIME_PM(dev) || !i915_enable_runtime_pm)
>> > return;
>> >
>> > /* Make sure we're not suspended first. */
>> > diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
>> > index a999a3f..c75b78f 100644
>> > --- a/drivers/gpu/drm/i915/i915_drv.c
>> > +++ b/drivers/gpu/drm/i915/i915_drv.c
>> > @@ -154,6 +154,10 @@ module_param_named(prefault_disable, i915_prefault_disable, bool, 0600);
>> > MODULE_PARM_DESC(prefault_disable,
>> > "Disable page prefaulting for pread/pwrite/reloc (default:false). For developers only.");
>> >
>> > +int i915_enable_runtime_pm __read_mostly = 0;
>> > +module_param_named(enable_runtime_pm, i915_enable_runtime_pm, int, 0600);
>> > +MODULE_PARM_DESC(enable_runtime_pm, "Enable runtime PM on supported platforms (default: disabled)");
>> > +
>> > static struct drm_driver driver;
>> > extern int intel_agp_enabled;
>> >
>> > @@ -890,7 +894,7 @@ static int i915_runtime_suspend(struct device *device)
>> > struct drm_device *dev = pci_get_drvdata(pdev);
>> > struct drm_i915_private *dev_priv = dev->dev_private;
>> >
>> > - WARN_ON(!HAS_RUNTIME_PM(dev));
>> > + WARN_ON(!HAS_RUNTIME_PM(dev) || !i915_enable_runtime_pm);
>> >
>> > DRM_DEBUG_KMS("Suspending device\n");
>> >
>> > @@ -909,7 +913,7 @@ static int i915_runtime_resume(struct device *device)
>> > struct drm_device *dev = pci_get_drvdata(pdev);
>> > struct drm_i915_private *dev_priv = dev->dev_private;
>> >
>> > - WARN_ON(!HAS_RUNTIME_PM(dev));
>> > + WARN_ON(!HAS_RUNTIME_PM(dev) || !i915_enable_runtime_pm);
>>
>> This could a problem if someone disables runtime_pm through sysfs while
>> the device is suspended. One solution would be just to do a get/put in
>> the handler of i915_enable_runtime_pm and not check for it afterwards.
>
> There's already a standard interface for enabling/disabling runtime PM
> under sysfs. Why do we want this custom one?
Mostly to avoid those people that run powertop and switch everything
to "good". But I don't really have a strong opinion here, I'm fine
with just discarding this patch.
>
> --
> Ville Syrjälä
> Intel OTC
--
Paulo Zanoni
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [RFC 1/6] drm/i915: add initial Runtime PM functions
2013-10-28 13:21 ` Imre Deak
@ 2013-11-06 20:32 ` Paulo Zanoni
2013-11-07 9:38 ` Imre Deak
0 siblings, 1 reply; 25+ messages in thread
From: Paulo Zanoni @ 2013-11-06 20:32 UTC (permalink / raw)
To: Imre Deak; +Cc: Intel Graphics Development, Paulo Zanoni
2013/10/28 Imre Deak <imre.deak@intel.com>:
> On Tue, 2013-10-22 at 17:30 -0200, Paulo Zanoni wrote:
>> From: Paulo Zanoni <paulo.r.zanoni@intel.com>
>>
>> This patch adds the initial infrastructure to allow a Runtime PM
>> implementation that sets the device to its D3 state. The patch just
>> adds the necessary callbacks and the initial infrastructure.
>>
>> We still don't have any platform that actually uses this
>> infrastructure, we still don't call get/put in all the places we need
>> to, and we don't have any function to save/restore the state of the
>> registers. This is not a problem since no platform uses the code added
>> by this patch. We have a few people simultaneously working on runtime
>> PM, so this initial code could help everybody make their plans.
>>
>> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
>> ---
>> drivers/gpu/drm/i915/i915_dma.c | 38 +++++++++++++++++++++++++++++++++
>> drivers/gpu/drm/i915/i915_drv.c | 42 +++++++++++++++++++++++++++++++++++++
>> drivers/gpu/drm/i915/i915_drv.h | 7 +++++++
>> drivers/gpu/drm/i915/intel_drv.h | 2 ++
>> drivers/gpu/drm/i915/intel_pm.c | 26 +++++++++++++++++++++++
>> drivers/gpu/drm/i915/intel_uncore.c | 9 ++++++++
>> 6 files changed, 124 insertions(+)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
>> index fd848ef..6aa044e 100644
>> --- a/drivers/gpu/drm/i915/i915_dma.c
>> +++ b/drivers/gpu/drm/i915/i915_dma.c
>> @@ -42,6 +42,8 @@
>> #include <linux/vga_switcheroo.h>
>> #include <linux/slab.h>
>> #include <acpi/video.h>
>> +#include <linux/pm.h>
>> +#include <linux/pm_runtime.h>
>>
>> #define LP_RING(d) (&((struct drm_i915_private *)(d))->ring[RCS])
>>
>> @@ -1449,6 +1451,38 @@ static void i915_dump_device_info(struct drm_i915_private *dev_priv)
>> #undef SEP_COMMA
>> }
>>
>> +static void i915_init_runtime_pm(struct drm_i915_private *dev_priv)
>> +{
>> + struct drm_device *dev = dev_priv->dev;
>> + struct device *device = &dev->pdev->dev;
>> +
>> + dev_priv->pm.suspended = false;
>> +
>> + if (!HAS_RUNTIME_PM(dev))
>> + return;
>> +
>> + pm_runtime_set_active(device);
>> + pm_runtime_enable(device);
>> +
>> + pm_runtime_set_autosuspend_delay(device, 10000); /* 10s */
>> + pm_runtime_mark_last_busy(device);
>> + pm_runtime_use_autosuspend(device);
>> + pm_runtime_allow(device);
>
> This shouldn't be needed as we get here already with an allowed state.
> It's not a problem as it's just a nop here, but imo it's confusing that
> we don't have the corresponding pm_runtime_forbid() in
> i915_fini_runtime_pm().
If we don't call this, when we boot the machine the "power/control"
sysfs file will be "on", which means runtime PM is disabled. We have
to manually "echo auto > control" to enable runtime PM then. But I
guess leaving runtime PM disabled by default might be what we want, so
I'll remove the call here.
>
>> +}
>> +
>> +static void i915_fini_runtime_pm(struct drm_i915_private *dev_priv)
>> +{
>> + struct drm_device *dev = dev_priv->dev;
>> + struct device *device = &dev->pdev->dev;
>> +
>> + if (!HAS_RUNTIME_PM(dev))
>> + return;
>> +
>> + /* Make sure we're not suspended first. */
>> + pm_runtime_get_sync(device);
>> + pm_runtime_disable(device);
>> +}
>> +
>
> Could we have the above functions in intel_pm.c?
Done.
>
>> /**
>> * i915_driver_load - setup chip and create an initial config
>> * @dev: DRM device
>> @@ -1664,6 +1698,8 @@ int i915_driver_load(struct drm_device *dev, unsigned long flags)
>> if (IS_GEN5(dev))
>> intel_gpu_ips_init(dev_priv);
>>
>> + i915_init_runtime_pm(dev_priv);
>> +
>> return 0;
>>
>> out_power_well:
>> @@ -1704,6 +1740,8 @@ int i915_driver_unload(struct drm_device *dev)
>> struct drm_i915_private *dev_priv = dev->dev_private;
>> int ret;
>>
>> + i915_fini_runtime_pm(dev_priv);
>> +
>> intel_gpu_ips_teardown();
>>
>> if (HAS_POWER_WELL(dev)) {
>> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
>> index 85ae0dc..08fc7ea 100644
>> --- a/drivers/gpu/drm/i915/i915_drv.c
>> +++ b/drivers/gpu/drm/i915/i915_drv.c
>> @@ -469,6 +469,8 @@ static int i915_drm_freeze(struct drm_device *dev)
>> struct drm_i915_private *dev_priv = dev->dev_private;
>> struct drm_crtc *crtc;
>>
>> + intel_runtime_pm_get(dev_priv);
>> +
>> /* ignore lid events during suspend */
>> mutex_lock(&dev_priv->modeset_restore_lock);
>> dev_priv->modeset_restore = MODESET_SUSPENDED;
>> @@ -653,6 +655,8 @@ static int __i915_drm_thaw(struct drm_device *dev, bool restore_gtt_mappings)
>> mutex_lock(&dev_priv->modeset_restore_lock);
>> dev_priv->modeset_restore = MODESET_DONE;
>> mutex_unlock(&dev_priv->modeset_restore_lock);
>> +
>> + intel_runtime_pm_put(dev_priv);
>> return error;
>> }
>>
>> @@ -880,6 +884,42 @@ static int i915_pm_poweroff(struct device *dev)
>> return i915_drm_freeze(drm_dev);
>> }
>>
>> +static int i915_runtime_suspend(struct device *device)
>> +{
>> + struct pci_dev *pdev = to_pci_dev(device);
>> + struct drm_device *dev = pci_get_drvdata(pdev);
>> + struct drm_i915_private *dev_priv = dev->dev_private;
>> +
>> + WARN_ON(!HAS_RUNTIME_PM(dev));
>> +
>> + DRM_DEBUG_KMS("Suspending device\n");
>> +
>> + dev_priv->pm.suspended = true;
>> +
>> + pci_save_state(pdev);
>> + pci_set_power_state(pdev, PCI_D3cold);
>> +
>> + return 0;
>> +}
>> +
>> +static int i915_runtime_resume(struct device *device)
>> +{
>> + struct pci_dev *pdev = to_pci_dev(device);
>> + struct drm_device *dev = pci_get_drvdata(pdev);
>> + struct drm_i915_private *dev_priv = dev->dev_private;
>> +
>> + WARN_ON(!HAS_RUNTIME_PM(dev));
>> +
>> + DRM_DEBUG_KMS("Resuming device\n");
>> +
>> + pci_set_power_state(pdev, PCI_D0);
>> + pci_restore_state(pdev);
>> +
>> + dev_priv->pm.suspended = false;
>> +
>> + return 0;
>> +}
>> +
>> static const struct dev_pm_ops i915_pm_ops = {
>> .suspend = i915_pm_suspend,
>> .resume = i915_pm_resume,
>> @@ -887,6 +927,8 @@ static const struct dev_pm_ops i915_pm_ops = {
>> .thaw = i915_pm_thaw,
>> .poweroff = i915_pm_poweroff,
>> .restore = i915_pm_resume,
>> + .runtime_suspend = i915_runtime_suspend,
>> + .runtime_resume = i915_runtime_resume,
>> };
>>
>> static const struct vm_operations_struct i915_gem_vm_ops = {
>> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
>> index 80957ca..74f2b5d 100644
>> --- a/drivers/gpu/drm/i915/i915_drv.h
>> +++ b/drivers/gpu/drm/i915/i915_drv.h
>> @@ -1230,6 +1230,10 @@ struct i915_package_c8 {
>> } regsave;
>> };
>>
>> +struct i915_runtime_pm {
>> + bool suspended;
>> +};
>> +
>> enum intel_pipe_crc_source {
>> INTEL_PIPE_CRC_SOURCE_NONE,
>> INTEL_PIPE_CRC_SOURCE_PLANE1,
>> @@ -1462,6 +1466,8 @@ typedef struct drm_i915_private {
>>
>> struct i915_package_c8 pc8;
>>
>> + struct i915_runtime_pm pm;
>> +
>> /* Old dri1 support infrastructure, beware the dragons ya fools entering
>> * here! */
>> struct i915_dri1_state dri1;
>> @@ -1779,6 +1785,7 @@ struct drm_i915_file_private {
>> #define HAS_POWER_WELL(dev) (IS_HASWELL(dev))
>> #define HAS_FPGA_DBG_UNCLAIMED(dev) (INTEL_INFO(dev)->has_fpga_dbg)
>> #define HAS_PSR(dev) (IS_HASWELL(dev))
>> +#define HAS_RUNTIME_PM(dev) false
>>
>> #define INTEL_PCH_DEVICE_ID_MASK 0xff00
>> #define INTEL_PCH_IBX_DEVICE_ID_TYPE 0x3b00
>> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
>> index af1553c..80b5a80 100644
>> --- a/drivers/gpu/drm/i915/intel_drv.h
>> +++ b/drivers/gpu/drm/i915/intel_drv.h
>> @@ -840,6 +840,8 @@ void gen6_rps_idle(struct drm_i915_private *dev_priv);
>> void gen6_rps_boost(struct drm_i915_private *dev_priv);
>> void intel_aux_display_runtime_get(struct drm_i915_private *dev_priv);
>> void intel_aux_display_runtime_put(struct drm_i915_private *dev_priv);
>> +void intel_runtime_pm_get(struct drm_i915_private *dev_priv);
>> +void intel_runtime_pm_put(struct drm_i915_private *dev_priv);
>> void ilk_wm_get_hw_state(struct drm_device *dev);
>>
>>
>> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
>> index 3e140ab..a973f35 100644
>> --- a/drivers/gpu/drm/i915/intel_pm.c
>> +++ b/drivers/gpu/drm/i915/intel_pm.c
>> @@ -31,6 +31,7 @@
>> #include "../../../platform/x86/intel_ips.h"
>> #include <linux/module.h>
>> #include <drm/i915_powerwell.h>
>> +#include <linux/pm_runtime.h>
>>
>> /**
>> * RC6 is a special power stage which allows the GPU to enter an very
>> @@ -5779,6 +5780,31 @@ void intel_aux_display_runtime_put(struct drm_i915_private *dev_priv)
>> hsw_enable_package_c8(dev_priv);
>> }
>>
>> +void intel_runtime_pm_get(struct drm_i915_private *dev_priv)
>> +{
>> + struct drm_device *dev = dev_priv->dev;
>> + struct device *device = &dev->pdev->dev;
>> +
>> + if (!HAS_RUNTIME_PM(dev))
>> + return;
>> +
>> + pm_runtime_mark_last_busy(device);
>
> What's the point of calling this here? We call pm_runtime_get below,
> which will prevent an auto suspend anyway and we call
> pm_runtime_mark_last_busy() then intel_runtime_pm_put().
>
>> + pm_runtime_get(device);
>> + pm_runtime_barrier(device);
>
> Why not using pm_runtime_get_sync() instead of the above 2 calls?
Done. Also added a WARN just to make sure it does what we want (I had
some problems with this while debugging).
Thanks for the review!
>
> --Imre
>
>> +}
>> +
>> +void intel_runtime_pm_put(struct drm_i915_private *dev_priv)
>> +{
>> + struct drm_device *dev = dev_priv->dev;
>> + struct device *device = &dev->pdev->dev;
>> +
>> + if (!HAS_RUNTIME_PM(dev))
>> + return;
>> +
>> + pm_runtime_mark_last_busy(device);
>> + pm_runtime_put_autosuspend(device);
>> +}
>> +
>> /* Set up chip specific power management-related functions */
>> void intel_init_pm(struct drm_device *dev)
>> {
>> diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c
>> index f6fae35..f5a2a6d 100644
>> --- a/drivers/gpu/drm/i915/intel_uncore.c
>> +++ b/drivers/gpu/drm/i915/intel_uncore.c
>> @@ -343,6 +343,13 @@ hsw_unclaimed_reg_check(struct drm_i915_private *dev_priv, u32 reg)
>> }
>> }
>>
>> +static void
>> +assert_device_not_suspended(struct drm_i915_private *dev_priv)
>> +{
>> + WARN(HAS_RUNTIME_PM(dev_priv->dev) && dev_priv->pm.suspended,
>> + "Device suspended\n");
>> +}
>> +
>> #define REG_READ_HEADER(x) \
>> unsigned long irqflags; \
>> u##x val = 0; \
>> @@ -435,6 +442,7 @@ gen6_write##x(struct drm_i915_private *dev_priv, off_t reg, u##x val, bool trace
>> if (NEEDS_FORCE_WAKE((dev_priv), (reg))) { \
>> __fifo_ret = __gen6_gt_wait_for_fifo(dev_priv); \
>> } \
>> + assert_device_not_suspended(dev_priv); \
>> __raw_i915_write##x(dev_priv, reg, val); \
>> if (unlikely(__fifo_ret)) { \
>> gen6_gt_check_fifodbg(dev_priv); \
>> @@ -450,6 +458,7 @@ hsw_write##x(struct drm_i915_private *dev_priv, off_t reg, u##x val, bool trace)
>> if (NEEDS_FORCE_WAKE((dev_priv), (reg))) { \
>> __fifo_ret = __gen6_gt_wait_for_fifo(dev_priv); \
>> } \
>> + assert_device_not_suspended(dev_priv); \
>> hsw_unclaimed_reg_clear(dev_priv, reg); \
>> __raw_i915_write##x(dev_priv, reg, val); \
>> if (unlikely(__fifo_ret)) { \
>
--
Paulo Zanoni
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [RFC 1/6] drm/i915: add initial Runtime PM functions
2013-11-06 20:32 ` Paulo Zanoni
@ 2013-11-07 9:38 ` Imre Deak
2013-11-07 11:36 ` Imre Deak
0 siblings, 1 reply; 25+ messages in thread
From: Imre Deak @ 2013-11-07 9:38 UTC (permalink / raw)
To: Paulo Zanoni; +Cc: Intel Graphics Development, Paulo Zanoni
On Wed, 2013-11-06 at 18:32 -0200, Paulo Zanoni wrote:
> 2013/10/28 Imre Deak <imre.deak@intel.com>:
> > On Tue, 2013-10-22 at 17:30 -0200, Paulo Zanoni wrote:
> >> From: Paulo Zanoni <paulo.r.zanoni@intel.com>
> >>
> >> This patch adds the initial infrastructure to allow a Runtime PM
> >> implementation that sets the device to its D3 state. The patch just
> >> adds the necessary callbacks and the initial infrastructure.
> >>
> >> We still don't have any platform that actually uses this
> >> infrastructure, we still don't call get/put in all the places we need
> >> to, and we don't have any function to save/restore the state of the
> >> registers. This is not a problem since no platform uses the code added
> >> by this patch. We have a few people simultaneously working on runtime
> >> PM, so this initial code could help everybody make their plans.
> >>
> >> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
> >> ---
> >> drivers/gpu/drm/i915/i915_dma.c | 38 +++++++++++++++++++++++++++++++++
> >> drivers/gpu/drm/i915/i915_drv.c | 42 +++++++++++++++++++++++++++++++++++++
> >> drivers/gpu/drm/i915/i915_drv.h | 7 +++++++
> >> drivers/gpu/drm/i915/intel_drv.h | 2 ++
> >> drivers/gpu/drm/i915/intel_pm.c | 26 +++++++++++++++++++++++
> >> drivers/gpu/drm/i915/intel_uncore.c | 9 ++++++++
> >> 6 files changed, 124 insertions(+)
> >>
> >> diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
> >> index fd848ef..6aa044e 100644
> >> --- a/drivers/gpu/drm/i915/i915_dma.c
> >> +++ b/drivers/gpu/drm/i915/i915_dma.c
> >> @@ -42,6 +42,8 @@
> >> #include <linux/vga_switcheroo.h>
> >> #include <linux/slab.h>
> >> #include <acpi/video.h>
> >> +#include <linux/pm.h>
> >> +#include <linux/pm_runtime.h>
> >>
> >> #define LP_RING(d) (&((struct drm_i915_private *)(d))->ring[RCS])
> >>
> >> @@ -1449,6 +1451,38 @@ static void i915_dump_device_info(struct drm_i915_private *dev_priv)
> >> #undef SEP_COMMA
> >> }
> >>
> >> +static void i915_init_runtime_pm(struct drm_i915_private *dev_priv)
> >> +{
> >> + struct drm_device *dev = dev_priv->dev;
> >> + struct device *device = &dev->pdev->dev;
> >> +
> >> + dev_priv->pm.suspended = false;
> >> +
> >> + if (!HAS_RUNTIME_PM(dev))
> >> + return;
> >> +
> >> + pm_runtime_set_active(device);
> >> + pm_runtime_enable(device);
> >> +
> >> + pm_runtime_set_autosuspend_delay(device, 10000); /* 10s */
> >> + pm_runtime_mark_last_busy(device);
> >> + pm_runtime_use_autosuspend(device);
> >> + pm_runtime_allow(device);
> >
> > This shouldn't be needed as we get here already with an allowed state.
> > It's not a problem as it's just a nop here, but imo it's confusing that
> > we don't have the corresponding pm_runtime_forbid() in
> > i915_fini_runtime_pm().
>
> If we don't call this, when we boot the machine the "power/control"
> sysfs file will be "on", which means runtime PM is disabled. We have
> to manually "echo auto > control" to enable runtime PM then. But I
> guess leaving runtime PM disabled by default might be what we want, so
> I'll remove the call here.
Right, I haven't noticed that pci_pm_init() does an explicit
pm_runtime_forbid(). Documentation/runtime_pm.txt says that drivers
should call pm_runtime_forbid() explicitly if they want to disable user
control. Imo the PCI subsystem doing this in the background is somewhat
deceiving for driver authors.
I noticed only now by looking at pci_pm_init() that the same goes for
pm_runtime_set_active(), pm_runtime_enable() above. Since these are
already called for you, atm you'll get an "unbalanced pm_runtime_enable"
message, though this doesn't cause any other problem. Again contrary to
what you'd expect reading runtime_pm.txt.
--Imre
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [RFC 1/6] drm/i915: add initial Runtime PM functions
2013-11-07 9:38 ` Imre Deak
@ 2013-11-07 11:36 ` Imre Deak
2013-11-07 18:13 ` Imre Deak
0 siblings, 1 reply; 25+ messages in thread
From: Imre Deak @ 2013-11-07 11:36 UTC (permalink / raw)
To: Paulo Zanoni; +Cc: Intel Graphics Development, Paulo Zanoni
On Thu, 2013-11-07 at 11:38 +0200, Imre Deak wrote:
> On Wed, 2013-11-06 at 18:32 -0200, Paulo Zanoni wrote:
> > 2013/10/28 Imre Deak <imre.deak@intel.com>:
> > > On Tue, 2013-10-22 at 17:30 -0200, Paulo Zanoni wrote:
> > >> From: Paulo Zanoni <paulo.r.zanoni@intel.com>
> > >>
> > >> This patch adds the initial infrastructure to allow a Runtime PM
> > >> implementation that sets the device to its D3 state. The patch just
> > >> adds the necessary callbacks and the initial infrastructure.
> > >>
> > >> We still don't have any platform that actually uses this
> > >> infrastructure, we still don't call get/put in all the places we need
> > >> to, and we don't have any function to save/restore the state of the
> > >> registers. This is not a problem since no platform uses the code added
> > >> by this patch. We have a few people simultaneously working on runtime
> > >> PM, so this initial code could help everybody make their plans.
> > >>
> > >> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
> > >> ---
> > >> drivers/gpu/drm/i915/i915_dma.c | 38 +++++++++++++++++++++++++++++++++
> > >> drivers/gpu/drm/i915/i915_drv.c | 42 +++++++++++++++++++++++++++++++++++++
> > >> drivers/gpu/drm/i915/i915_drv.h | 7 +++++++
> > >> drivers/gpu/drm/i915/intel_drv.h | 2 ++
> > >> drivers/gpu/drm/i915/intel_pm.c | 26 +++++++++++++++++++++++
> > >> drivers/gpu/drm/i915/intel_uncore.c | 9 ++++++++
> > >> 6 files changed, 124 insertions(+)
> > >>
> > >> diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
> > >> index fd848ef..6aa044e 100644
> > >> --- a/drivers/gpu/drm/i915/i915_dma.c
> > >> +++ b/drivers/gpu/drm/i915/i915_dma.c
> > >> @@ -42,6 +42,8 @@
> > >> #include <linux/vga_switcheroo.h>
> > >> #include <linux/slab.h>
> > >> #include <acpi/video.h>
> > >> +#include <linux/pm.h>
> > >> +#include <linux/pm_runtime.h>
> > >>
> > >> #define LP_RING(d) (&((struct drm_i915_private *)(d))->ring[RCS])
> > >>
> > >> @@ -1449,6 +1451,38 @@ static void i915_dump_device_info(struct drm_i915_private *dev_priv)
> > >> #undef SEP_COMMA
> > >> }
> > >>
> > >> +static void i915_init_runtime_pm(struct drm_i915_private *dev_priv)
> > >> +{
> > >> + struct drm_device *dev = dev_priv->dev;
> > >> + struct device *device = &dev->pdev->dev;
> > >> +
> > >> + dev_priv->pm.suspended = false;
> > >> +
> > >> + if (!HAS_RUNTIME_PM(dev))
> > >> + return;
> > >> +
> > >> + pm_runtime_set_active(device);
> > >> + pm_runtime_enable(device);
> > >> +
> > >> + pm_runtime_set_autosuspend_delay(device, 10000); /* 10s */
> > >> + pm_runtime_mark_last_busy(device);
> > >> + pm_runtime_use_autosuspend(device);
> > >> + pm_runtime_allow(device);
> > >
> > > This shouldn't be needed as we get here already with an allowed state.
> > > It's not a problem as it's just a nop here, but imo it's confusing that
> > > we don't have the corresponding pm_runtime_forbid() in
> > > i915_fini_runtime_pm().
> >
> > If we don't call this, when we boot the machine the "power/control"
> > sysfs file will be "on", which means runtime PM is disabled. We have
> > to manually "echo auto > control" to enable runtime PM then. But I
> > guess leaving runtime PM disabled by default might be what we want, so
> > I'll remove the call here.
>
> Right, I haven't noticed that pci_pm_init() does an explicit
> pm_runtime_forbid(). Documentation/runtime_pm.txt says that drivers
> should call pm_runtime_forbid() explicitly if they want to disable user
> control. Imo the PCI subsystem doing this in the background is somewhat
> deceiving for driver authors.
>
> I noticed only now by looking at pci_pm_init() that the same goes for
> pm_runtime_set_active(), pm_runtime_enable() above. Since these are
> already called for you, atm you'll get an "unbalanced pm_runtime_enable"
> message, though that doesn't cause any other problem. Again contrary to
> what you'd expect reading runtime_pm.txt.
Ok, Documentation/power/pci.txt explains the semantics on calling
pm_runtime_allow/forbid() for PCI devices, but still states incorrectly
that you need to call pm_runtime_enable().
So based on all these I think the correct init order is if you want to
leave auto suspend disabled:
pm_runtime_set_autosuspend_delay(device, 10000); /* 10s */
pm_runtime_mark_last_busy(device);
pm_runtime_use_autosuspend(device);
pm_runtime_put(device);
--Imre
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [RFC 1/6] drm/i915: add initial Runtime PM functions
2013-11-07 11:36 ` Imre Deak
@ 2013-11-07 18:13 ` Imre Deak
0 siblings, 0 replies; 25+ messages in thread
From: Imre Deak @ 2013-11-07 18:13 UTC (permalink / raw)
To: Paulo Zanoni; +Cc: Intel Graphics Development, Paulo Zanoni
On Thu, 2013-11-07 at 13:36 +0200, Imre Deak wrote:
> On Thu, 2013-11-07 at 11:38 +0200, Imre Deak wrote:
> > On Wed, 2013-11-06 at 18:32 -0200, Paulo Zanoni wrote:
> > > 2013/10/28 Imre Deak <imre.deak@intel.com>:
> > > > On Tue, 2013-10-22 at 17:30 -0200, Paulo Zanoni wrote:
> > > >> From: Paulo Zanoni <paulo.r.zanoni@intel.com>
> > > >>
> > > >> This patch adds the initial infrastructure to allow a Runtime PM
> > > >> implementation that sets the device to its D3 state. The patch just
> > > >> adds the necessary callbacks and the initial infrastructure.
> > > >>
> > > >> We still don't have any platform that actually uses this
> > > >> infrastructure, we still don't call get/put in all the places we need
> > > >> to, and we don't have any function to save/restore the state of the
> > > >> registers. This is not a problem since no platform uses the code added
> > > >> by this patch. We have a few people simultaneously working on runtime
> > > >> PM, so this initial code could help everybody make their plans.
> > > >>
> > > >> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
> > > >> ---
> > > >> drivers/gpu/drm/i915/i915_dma.c | 38 +++++++++++++++++++++++++++++++++
> > > >> drivers/gpu/drm/i915/i915_drv.c | 42 +++++++++++++++++++++++++++++++++++++
> > > >> drivers/gpu/drm/i915/i915_drv.h | 7 +++++++
> > > >> drivers/gpu/drm/i915/intel_drv.h | 2 ++
> > > >> drivers/gpu/drm/i915/intel_pm.c | 26 +++++++++++++++++++++++
> > > >> drivers/gpu/drm/i915/intel_uncore.c | 9 ++++++++
> > > >> 6 files changed, 124 insertions(+)
> > > >>
> > > >> diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c
> > > >> index fd848ef..6aa044e 100644
> > > >> --- a/drivers/gpu/drm/i915/i915_dma.c
> > > >> +++ b/drivers/gpu/drm/i915/i915_dma.c
> > > >> @@ -42,6 +42,8 @@
> > > >> #include <linux/vga_switcheroo.h>
> > > >> #include <linux/slab.h>
> > > >> #include <acpi/video.h>
> > > >> +#include <linux/pm.h>
> > > >> +#include <linux/pm_runtime.h>
> > > >>
> > > >> #define LP_RING(d) (&((struct drm_i915_private *)(d))->ring[RCS])
> > > >>
> > > >> @@ -1449,6 +1451,38 @@ static void i915_dump_device_info(struct drm_i915_private *dev_priv)
> > > >> #undef SEP_COMMA
> > > >> }
> > > >>
> > > >> +static void i915_init_runtime_pm(struct drm_i915_private *dev_priv)
> > > >> +{
> > > >> + struct drm_device *dev = dev_priv->dev;
> > > >> + struct device *device = &dev->pdev->dev;
> > > >> +
> > > >> + dev_priv->pm.suspended = false;
> > > >> +
> > > >> + if (!HAS_RUNTIME_PM(dev))
> > > >> + return;
> > > >> +
> > > >> + pm_runtime_set_active(device);
> > > >> + pm_runtime_enable(device);
> > > >> +
> > > >> + pm_runtime_set_autosuspend_delay(device, 10000); /* 10s */
> > > >> + pm_runtime_mark_last_busy(device);
> > > >> + pm_runtime_use_autosuspend(device);
> > > >> + pm_runtime_allow(device);
> > > >
> > > > This shouldn't be needed as we get here already with an allowed state.
> > > > It's not a problem as it's just a nop here, but imo it's confusing that
> > > > we don't have the corresponding pm_runtime_forbid() in
> > > > i915_fini_runtime_pm().
> > >
> > > If we don't call this, when we boot the machine the "power/control"
> > > sysfs file will be "on", which means runtime PM is disabled. We have
> > > to manually "echo auto > control" to enable runtime PM then. But I
> > > guess leaving runtime PM disabled by default might be what we want, so
> > > I'll remove the call here.
> >
> > Right, I haven't noticed that pci_pm_init() does an explicit
> > pm_runtime_forbid(). Documentation/runtime_pm.txt says that drivers
> > should call pm_runtime_forbid() explicitly if they want to disable user
> > control. Imo the PCI subsystem doing this in the background is somewhat
> > deceiving for driver authors.
> >
> > I noticed only now by looking at pci_pm_init() that the same goes for
> > pm_runtime_set_active(), pm_runtime_enable() above. Since these are
> > already called for you, atm you'll get an "unbalanced pm_runtime_enable"
> > message, though that doesn't cause any other problem. Again contrary to
> > what you'd expect reading runtime_pm.txt.
>
> Ok, Documentation/power/pci.txt explains the semantics on calling
> pm_runtime_allow/forbid() for PCI devices, but still states incorrectly
> that you need to call pm_runtime_enable().
>
> So based on all these I think the correct init order is if you want to
> leave auto suspend disabled:
>
> pm_runtime_set_autosuspend_delay(device, 10000); /* 10s */
> pm_runtime_mark_last_busy(device);
> pm_runtime_use_autosuspend(device);
> pm_runtime_put(device);
Err, pm_runtime_put() is not needed since we need to keep a reference
for pc8 (at least for HSW). Sorry for the noise.
--Imre
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [RFC i-g-t] tests: add runtime_pm
2013-11-04 21:40 ` Ville Syrjälä
@ 2013-11-08 18:19 ` Paulo Zanoni
2013-11-08 18:42 ` Daniel Vetter
0 siblings, 1 reply; 25+ messages in thread
From: Paulo Zanoni @ 2013-11-08 18:19 UTC (permalink / raw)
To: Ville Syrjälä; +Cc: Intel Graphics Development, Paulo Zanoni
2013/11/4 Ville Syrjälä <ville.syrjala@linux.intel.com>:
> On Mon, Oct 28, 2013 at 10:20:56AM -0200, Paulo Zanoni wrote:
>> 2013/10/27 Daniel Vetter <daniel@ffwll.ch>:
>> > On Fri, Oct 25, 2013 at 11:44:05AM -0200, Paulo Zanoni wrote:
>> >> From: Paulo Zanoni <paulo.r.zanoni@intel.com>
>> >>
>> >> This test is based on pc8.c. It copies most of the tests from pc8.c,
>> >> but it depends on runtime PM status changes (parsed from sysfs)
>> >> instead of PC8 residency changes (parsed from the MSR registers).
>> >> There's also a test that checks for PC8 residency.
>> >>
>> >> For now, runtime PM and PC8 are different features, so having 2 test
>> >> suites makes sense. In the future we'll merge both, so we'll only get
>> >> PC8 when runtime PM is enabled, so we'll just kill pc8.c and keep
>> >> using runtime_pm.c.
>> >>
>> >> Changes compared to pc8.c:
>> >> - We now look at the runtime PM status instead of PC8 residencies
>> >> - Added more GEM tests (mmap, pread, execbuf, stress tests)
>> >> - Added LPSP and non-LPSP tests
>> >> - Added tests fro sysfs and debugfs files
>> >> - Added a test specifically for PC8
>> >>
>> >> Signed-off-by: Paulo Zanoni <paulo.r.zanoni@intel.com>
>> >
>> > Since the actual tests we're running are so similar I prefer if we merge
>> > all the runtime pm tests in one file. It makes testcase maintaince (and
>> > bufixing, that happens) much easier. I guess a struct per runtime pm
>> > method (pc8, D3, ...) with a few vfuncs should get things going. The
>> > overall test would loop over all the pm methods and try to set things up.
>> > Then loop over all subtests and either skip them all (if that particular
>> > runtime pm method isn't supported) or just run them.
>> >
>> > We've had a few other case of massive copy&pasting in i-g-t and in the
>> > past few months I've merged most of them back again.
>>
>> At this moment I'm really leaning towards merging PC8 and D3 into a
>> single feature, so it won't be possible to test them in separate
>> anymore. With this, we'd just have runtime_pm.c and we'd completely
>> kill pc8.c.
>
> All this time I've been wondering whether PC8 offers anything
> substantial over D3.
It's more like "PC8 is a diet D3". I don't see why anyone would want
PC8 without D3.
> I think we discussed this in some meeting, and
> GTT maps were the only thing that came to my mind. But IIRC you
> said that you weren't actually sure whether GTT maps still work in
> PC8. Has that been verified or is it still an open question?
Daniel also asked for this test. This week I finally had some time to
go back to this. I just added a test for GTT mmaps and the test fails
while we're on D3 (didn't try PC8-without-D3 yet). Clever ideas on how
to fix the problem are always welcome :)
>
> --
> Ville Syrjälä
> Intel OTC
--
Paulo Zanoni
^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [RFC i-g-t] tests: add runtime_pm
2013-11-08 18:19 ` Paulo Zanoni
@ 2013-11-08 18:42 ` Daniel Vetter
0 siblings, 0 replies; 25+ messages in thread
From: Daniel Vetter @ 2013-11-08 18:42 UTC (permalink / raw)
To: Paulo Zanoni; +Cc: Intel Graphics Development, Paulo Zanoni
On Fri, Nov 8, 2013 at 7:19 PM, Paulo Zanoni <przanoni@gmail.com> wrote:
>> I think we discussed this in some meeting, and
>> GTT maps were the only thing that came to my mind. But IIRC you
>> said that you weren't actually sure whether GTT maps still work in
>> PC8. Has that been verified or is it still an open question?
>
> Daniel also asked for this test. This week I finally had some time to
> go back to this. I just added a test for GTT mmaps and the test fails
> while we're on D3 (didn't try PC8-without-D3 yet). Clever ideas on how
> to fix the problem are always welcome :)
You need to rip out all userspace mmaps when entering D3 (which is
fairly expensive to do and will hurt on wakeup when userspace renders
again) and then make sure the chip wakes up again when handling the
fault.
So in a way a gtt mmap holds a weak D3 reference - it doesn't prevent
D3 (we'll never enter it otherwise), but if we enter D3 we need to
remove them all.
The functions you're looking for is unmap_mapping_range on the entire
drm address_space (see how drm_vma_node_unmap uses it only for a given
gtt mmap). And then i915_gem_fault, where we need to wake-up the chip
again.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
^ permalink raw reply [flat|nested] 25+ messages in thread
end of thread, other threads:[~2013-11-08 21:41 UTC | newest]
Thread overview: 25+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-10-22 19:30 [RFC 0/6] Haswell runtime PM support + D3 Paulo Zanoni
2013-10-22 19:30 ` [RFC 1/6] drm/i915: add initial Runtime PM functions Paulo Zanoni
2013-10-28 13:21 ` Imre Deak
2013-11-06 20:32 ` Paulo Zanoni
2013-11-07 9:38 ` Imre Deak
2013-11-07 11:36 ` Imre Deak
2013-11-07 18:13 ` Imre Deak
2013-10-22 19:30 ` [RFC 2/6] drm/i915: do adapter power state notification at runtime PM Paulo Zanoni
2013-10-22 19:30 ` [RFC 3/6] drm/i915: add enable_runtime_pm option Paulo Zanoni
2013-10-28 13:27 ` Imre Deak
2013-11-04 21:36 ` Ville Syrjälä
2013-11-06 20:04 ` Paulo Zanoni
2013-10-22 19:30 ` [RFC 4/6] drm/i915: add runtime put/get calls at the basic places Paulo Zanoni
2013-10-22 19:30 ` [RFC 5/6] drm/i915: add some runtime PM get/put calls Paulo Zanoni
2013-10-22 19:30 ` [RFC 6/6] drm/i915: add runtime PM support on Haswell Paulo Zanoni
2013-10-25 13:44 ` [RFC i-g-t] tests: add runtime_pm Paulo Zanoni
2013-10-27 13:37 ` Daniel Vetter
2013-10-28 12:20 ` Paulo Zanoni
2013-10-28 16:05 ` Daniel Vetter
2013-11-04 21:40 ` Ville Syrjälä
2013-11-08 18:19 ` Paulo Zanoni
2013-11-08 18:42 ` Daniel Vetter
2013-10-28 13:09 ` [RFC 0/6] Haswell runtime PM support + D3 Imre Deak
2013-10-28 16:10 ` Daniel Vetter
2013-10-28 20:02 ` Jesse Barnes
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.