v3
Fixed biggest blunder
consecutive read output
#cat /sys/kernel/debug/punit_atom# cat dev_power_state
PUNIT NORTH COMPLEX DEVICES :
GFX RENDER : D0i3
GFX MEDIA : D0i3
DISPLAY : D0i3
VED : D0i3
ISP : D0i3
MIO : D0
#/sys/kernel/debug/punit_atom# cat dev_power_state
PUNIT NORTH COMPLEX DEVICES :
GFX RENDER : D0i3
GFX MEDIA : D0i3
DISPLAY : D0i3
VED : D0i3
ISP : D0i3
MIO : D0
#cat /sys/kernel/debug/punit_atom# cat dev_power_state
PUNIT NORTH COMPLEX DEVICES :
GFX RENDER : D0i3
GFX MEDIA : D0i3
DISPLAY : D0i3
VED : D0i3
ISP : D0i3
MIO : D0
# cat /sys/kernel/debug/punit_atom# cat dev_power_state
PUNIT NORTH COMPLEX DEVICES :
GFX RENDER : D0i3
GFX MEDIA : D0i3
DISPLAY : D0i3
VED : D0i3
ISP : D0i3
MIO : D0
v2
Addressed Ingo Molnar's comments
- Fix commit message
- Added punit explanation
- Formatting comments
- Moved to arch/x86/platform/intel_mid
- changed the debugfs file name
v1
Based on review comments
- Changed to tristate instead of bool
- Moved config to kconfig.debug
- Added debug in module name
- Returning -ENXIO on debugfs file create error
v0:
Base version
Srinivas Pandruvada (1):
x86: punit_atom: punit device state debug driver
arch/x86/Kconfig.debug | 9 ++
arch/x86/platform/intel-mid/Makefile | 2 +
arch/x86/platform/intel-mid/punit_atom_debug.c | 186 +++++++++++++++++++++++++
3 files changed, 197 insertions(+)
create mode 100644 arch/x86/platform/intel-mid/punit_atom_debug.c
--
1.9.1
The patch adds a debug driver, which dumps the power states
of all the North complex (NC) devices. This debug interface is
useful to figure out the devices, which blocks the S0ix
transitions on the platform. This is extremely useful during
enabling PM on customer platforms and derivatives.
This submission is based on the submission from
Kumar P, Mahesh <mahesh.kumar.p.intel.com>.
https://lkml.org/lkml/2014/11/5/367
The changes done on top of the above submission:
- Dropped changes to config for PMC_ATOM, as PMC_ATOM
is not just a debug driver as suggested by the change. It has
additional power off interface also.
- Instead of just using nc ("North complex") use punit_..
similar to south complex PMC.
- Removed pmc_config structure, as we don't need to predefine
number of register, we want to dump. This way new register
can be added without changing NC_NUM_DEVICES.
- prefixed function with punit_
- The debugfs directory will be punit_atom, which is NC equivalent
of pmc_atom, which we already exposed by pmc_atom driver.
- Added explanation for register and shift defines
- Will not create debugfs if the cpuid doesn't match
- Formatting changes to match compliance to coding convention
- Address review comments
Signed-off-by: Srinivas Pandruvada <[email protected]>
---
arch/x86/Kconfig.debug | 9 ++
arch/x86/platform/intel-mid/Makefile | 2 +
arch/x86/platform/intel-mid/punit_atom_debug.c | 186 +++++++++++++++++++++++++
3 files changed, 197 insertions(+)
create mode 100644 arch/x86/platform/intel-mid/punit_atom_debug.c
diff --git a/arch/x86/Kconfig.debug b/arch/x86/Kconfig.debug
index 20028da..4f42c3a 100644
--- a/arch/x86/Kconfig.debug
+++ b/arch/x86/Kconfig.debug
@@ -336,4 +336,13 @@ config X86_DEBUG_STATIC_CPU_HAS
If unsure, say N.
+config PUNIT_ATOM_DEBUG
+ tristate "ATOM Punit debug driver"
+ depends on DEBUG_FS
+ select IOSF_MBI
+ ---help---
+ This is a debug driver, which gets the power states
+ of all Punit North Complex devices. The power states of
+ each device is exposed as part of the debugfs interface.
+
endmenu
diff --git a/arch/x86/platform/intel-mid/Makefile b/arch/x86/platform/intel-mid/Makefile
index 0a8ee70..4763952 100644
--- a/arch/x86/platform/intel-mid/Makefile
+++ b/arch/x86/platform/intel-mid/Makefile
@@ -5,3 +5,5 @@ obj-$(CONFIG_EARLY_PRINTK_INTEL_MID) += early_printk_intel_mid.o
ifdef CONFIG_X86_INTEL_MID
obj-$(CONFIG_SFI) += sfi.o device_libs/
endif
+
+obj-$(CONFIG_PUNIT_ATOM_DEBUG) += punit_atom_debug.o
diff --git a/arch/x86/platform/intel-mid/punit_atom_debug.c b/arch/x86/platform/intel-mid/punit_atom_debug.c
new file mode 100644
index 0000000..3b41e0a
--- /dev/null
+++ b/arch/x86/platform/intel-mid/punit_atom_debug.c
@@ -0,0 +1,186 @@
+/*
+ * Intel SOC Punit device state debug driver
+ * Punit controls power management for North Complex devices (Graphics
+ * blocks, Image Signal Processing, video processing, display, DSP etc.)
+ *
+ * Copyright (c) 2015, Intel Corporation.
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for
+ * more details.
+ *
+ */
+
+#include <linux/module.h>
+#include <linux/init.h>
+#include <linux/device.h>
+#include <linux/debugfs.h>
+#include <linux/seq_file.h>
+#include <linux/io.h>
+#include <asm/cpu_device_id.h>
+#include <asm/iosf_mbi.h>
+
+/* Side band Interface port */
+#define PUNIT_PORT 0x04
+/* Power gate status reg */
+#define PWRGT_STATUS 0x61
+/* Subsystem config/status Video processor */
+#define VED_SS_PM0 0x32
+/* Subsystem config/status ISP (Image Signal Processor) */
+#define ISP_SS_PM0 0x39
+/* Subsystem config/status Input/output controller */
+#define MIO_SS_PM 0x3B
+/* Shift bits for getting status for video, isp and i/o */
+#define SSS_SHIFT 24
+/* Shift bits for getting status for graphics rendering */
+#define RENDER_POS 0
+/* Shift bits for getting status for media control */
+#define MEDIA_POS 2
+/* Shift bits for getting status for Valley View/Baytrail display */
+#define VLV_DISPLAY_POS 6
+/* Subsystem config/status DSP for Cherry Trail SOC */
+#define CHT_DSP_SSS 0x36
+/* Shift bits for getting status for DSP */
+#define CHT_DSP_SSS_POS 16
+
+struct punit_device {
+ char *name;
+ int reg;
+ int sss_pos;
+};
+
+static struct punit_device *punit_device;
+
+static const struct punit_device punit_device_byt[] = {
+ { "GFX RENDER", PWRGT_STATUS, RENDER_POS },
+ { "GFX MEDIA", PWRGT_STATUS, MEDIA_POS },
+ { "DISPLAY", PWRGT_STATUS, VLV_DISPLAY_POS },
+ { "VED", VED_SS_PM0, SSS_SHIFT },
+ { "ISP", ISP_SS_PM0, SSS_SHIFT },
+ { "MIO", MIO_SS_PM, SSS_SHIFT },
+ { NULL }
+};
+
+static const struct punit_device punit_device_cht[] = {
+ { "GFX RENDER", PWRGT_STATUS, RENDER_POS },
+ { "GFX MEDIA", PWRGT_STATUS, MEDIA_POS },
+ { "DSP", CHT_DSP_SSS, CHT_DSP_SSS_POS },
+ { "VED", VED_SS_PM0, SSS_SHIFT },
+ { "ISP", ISP_SS_PM0, SSS_SHIFT },
+ { "MIO", MIO_SS_PM, SSS_SHIFT },
+ { NULL }
+};
+
+static const char * const dstates[] = {"D0", "D0i1", "D0i2", "D0i3"};
+
+static int punit_dev_state_show(struct seq_file *seq_file, void *unused)
+{
+ u32 punit_pwr_status;
+ struct punit_device *punit_devp = punit_device;
+ int index;
+ int status;
+
+ seq_puts(seq_file, "\n\nPUNIT NORTH COMPLEX DEVICES :\n");
+ while (punit_devp->name) {
+ status = iosf_mbi_read(PUNIT_PORT, BT_MBI_PMC_READ,
+ punit_devp->reg,
+ &punit_pwr_status);
+ if (status)
+ seq_printf(seq_file, "%9s : Read Failed\n",
+ punit_devp->name);
+ else {
+ index = (punit_pwr_status >> punit_devp->sss_pos) & 3;
+ seq_printf(seq_file, "%9s : %s\n", punit_devp->name,
+ dstates[index]);
+ }
+ punit_devp++;
+ }
+
+ return 0;
+}
+
+static int punit_dev_state_open(struct inode *inode, struct file *file)
+{
+ return single_open(file, punit_dev_state_show, inode->i_private);
+}
+
+static const struct file_operations punit_dev_state_ops = {
+ .open = punit_dev_state_open,
+ .read = seq_read,
+ .llseek = seq_lseek,
+ .release = single_release,
+};
+
+static struct dentry *punit_dbg_file;
+
+static int punit_dbgfs_register(void)
+{
+ static struct dentry *dev_state;
+
+ punit_dbg_file = debugfs_create_dir("punit_atom", NULL);
+ if (!punit_dbg_file)
+ return -ENXIO;
+
+ dev_state = debugfs_create_file("dev_power_state", S_IFREG | S_IRUGO,
+ punit_dbg_file, NULL,
+ &punit_dev_state_ops);
+ if (!dev_state) {
+ pr_err("punit_dev_state register failed\n");
+ return -ENXIO;
+ }
+
+ return 0;
+}
+
+static void punit_dbgfs_unregister(void)
+{
+ debugfs_remove_recursive(punit_dbg_file);
+}
+
+#define ICPU(model, drv_data) \
+ { X86_VENDOR_INTEL, 6, model, X86_FEATURE_MWAIT,\
+ (kernel_ulong_t)&drv_data }
+
+static const struct x86_cpu_id intel_punit_cpu_ids[] = {
+ ICPU(0x4c, punit_device_cht),
+ ICPU(0x37, punit_device_byt),
+ {}
+};
+
+MODULE_DEVICE_TABLE(x86cpu, intel_punit_cpu_ids);
+
+static int __init punit_atom_debug_init(void)
+{
+ const struct x86_cpu_id *id;
+ int ret;
+
+ id = x86_match_cpu(intel_punit_cpu_ids);
+ if (!id)
+ return -ENODEV;
+
+ punit_device = (struct punit_device *)id->driver_data;
+
+ ret = punit_dbgfs_register();
+ if (ret < 0)
+ return ret;
+
+ return 0;
+}
+
+static void __exit punit_atom_debug_exit(void)
+{
+ punit_dbgfs_unregister();
+}
+
+module_init(punit_atom_debug_init);
+module_exit(punit_atom_debug_exit);
+
+MODULE_AUTHOR("Kumar P, Mahesh <[email protected]>");
+MODULE_AUTHOR("Srinivas Pandruvada <[email protected]>");
+MODULE_DESCRIPTION("Driver for Punit devices states debugging");
+MODULE_LICENSE("GPL v2");
--
1.9.1
On Mon, 2015-05-04 at 13:49 -0700, Srinivas Pandruvada wrote:
> v3
> Fixed biggest blunder
>
> consecutive read output
root@VALLEYVIEW-C0-PLATFORM:/sys/kernel/debug/punit_atom# cat
dev_power_state
PUNIT NORTH COMPLEX DEVICES :
GFX RENDER : D0i3
GFX MEDIA : D0i3
DISPLAY : D0i3
VED : D0i3
ISP : D0i3
MIO : D0
root@VALLEYVIEW-C0-PLATFORM:/sys/kernel/debug/punit_atom# cat
dev_power_state
PUNIT NORTH COMPLEX DEVICES :
GFX RENDER : D0i3
GFX MEDIA : D0i3
DISPLAY : D0i3
VED : D0i3
ISP : D0i3
MIO : D0
root@VALLEYVIEW-C0-PLATFORM:/sys/kernel/debug/punit_atom# cat
dev_power_state
PUNIT NORTH COMPLEX DEVICES :
GFX RENDER : D0i3
GFX MEDIA : D0i3
DISPLAY : D0i3
VED : D0i3
ISP : D0i3
MIO : D0
root@VALLEYVIEW-C0-PLATFORM:/sys/kernel/debug/punit_atom# cat
dev_power_state
PUNIT NORTH COMPLEX DEVICES :
GFX RENDER : D0i3
GFX MEDIA : D0i3
DISPLAY : D0i3
VED : D0i3
ISP : D0i3
MIO : D0
root@VALLEYVIEW-C0-PLATFORM:/sys/kernel/debug/punit_atom# cat
dev_power_state
PUNIT NORTH COMPLEX DEVICES :
GFX RENDER : D0i3
GFX MEDIA : D0i3
DISPLAY : D0i3
VED : D0i3
ISP : D0i3
MIO : D0
root@VALLEYVIEW-C0-PLATFORM:/sys/kernel/debug/punit_atom# cat
dev_power_state
PUNIT NORTH COMPLEX DEVICES :
GFX RENDER : D0i3
GFX MEDIA : D0i3
DISPLAY : D0i3
VED : D0i3
ISP : D0i3
MIO : D0
root@VALLEYVIEW-C0-PLATFORM:/sys/kernel/debug/punit_atom# cat
dev_power_state
PUNIT NORTH COMPLEX DEVICES :
GFX RENDER : D0i3
GFX MEDIA : D0i3
DISPLAY : D0i3
VED : D0i3
ISP : D0i3
MIO : D0
root@VALLEYVIEW-C0-PLATFORM:/sys/kernel/debug/punit_atom# cat
dev_power_state
PUNIT NORTH COMPLEX DEVICES :
GFX RENDER : D0i3
GFX MEDIA : D0i3
DISPLAY : D0i3
VED : D0i3
ISP : D0i3
MIO : D0
root@VALLEYVIEW-C0-PLATFORM:/sys/kernel/debug/punit_atom# cat
dev_power_state
PUNIT NORTH COMPLEX DEVICES :
GFX RENDER : D0i3
GFX MEDIA : D0i3
DISPLAY : D0i3
VED : D0i3
ISP : D0i3
MIO : D0
>
>
> v2
> Addressed Ingo Molnar's comments
> - Fix commit message
> - Added punit explanation
> - Formatting comments
> - Moved to arch/x86/platform/intel_mid
> - changed the debugfs file name
>
> v1
> Based on review comments
> - Changed to tristate instead of bool
> - Moved config to kconfig.debug
> - Added debug in module name
> - Returning -ENXIO on debugfs file create error
>
> v0:
> Base version
>
>
> Srinivas Pandruvada (1):
> x86: punit_atom: punit device state debug driver
>
> arch/x86/Kconfig.debug | 9 ++
> arch/x86/platform/intel-mid/Makefile | 2 +
> arch/x86/platform/intel-mid/punit_atom_debug.c | 186 +++++++++++++++++++++++++
> 3 files changed, 197 insertions(+)
> create mode 100644 arch/x86/platform/intel-mid/punit_atom_debug.c
>
* Srinivas Pandruvada <[email protected]> wrote:
> The patch adds a debug driver, which dumps the power states of all
> the North complex (NC) devices. This debug interface is useful to
> figure out the devices, which blocks the S0ix transitions on the
> platform. This is extremely useful during enabling PM on customer
> platforms and derivatives.
Looks mostly good. Small nits:
> +config PUNIT_ATOM_DEBUG
> + tristate "ATOM Punit debug driver"
> + depends on DEBUG_FS
> + select IOSF_MBI
I suspect you could select DEBUG_FS as well? Half of the drivers seem
to do that.
> + ---help---
> + This is a debug driver, which gets the power states
> + of all Punit North Complex devices. The power states of
> + each device is exposed as part of the debugfs interface.
Might as well mention the path of the file? To keep people from
guessing and so.
> +static int punit_dev_state_show(struct seq_file *seq_file, void *unused)
> +{
> + u32 punit_pwr_status;
> + struct punit_device *punit_devp = punit_device;
You could stick stick 'punit_device' into s->private? You do that by
passing it to debugfs_create_file(). That way you could avoid the
fugly static allocation of 'punit_device' and its global setting in
punit_atom_debug_init().
> + int index;
> + int status;
> +
> + seq_puts(seq_file, "\n\nPUNIT NORTH COMPLEX DEVICES :\n");
> + while (punit_devp->name) {
> + status = iosf_mbi_read(PUNIT_PORT, BT_MBI_PMC_READ,
> + punit_devp->reg,
> + &punit_pwr_status);
> + if (status)
> + seq_printf(seq_file, "%9s : Read Failed\n",
> + punit_devp->name);
> + else {
> + index = (punit_pwr_status >> punit_devp->sss_pos) & 3;
> + seq_printf(seq_file, "%9s : %s\n", punit_devp->name,
> + dstates[index]);
> + }
We only use symmetric curly braces in the kernel.
> +#define ICPU(model, drv_data) \
> + { X86_VENDOR_INTEL, 6, model, X86_FEATURE_MWAIT,\
> + (kernel_ulong_t)&drv_data }
> +
> +static const struct x86_cpu_id intel_punit_cpu_ids[] = {
> + ICPU(0x4c, punit_device_cht),
> + ICPU(0x37, punit_device_byt),
> + {}
> +};
So should the models be listed in increasing order?
Also, I'd use decimal, as we do for models typically. Also, might as
well mention which Intel Atom models those are: 22nm Atom "Silvermont"
and 14nm Atom "Airmont", right?
Thanks,
Ingo