Re: [PATCH 2/2] i7300_idle driver v1.55

[Randy Dunlap <randy.dunlap@oracle.com>]

On Thu, 16 Oct 2008 18:08:01 -0400 (EDT) Len Brown wrote:

> From: Andy Henroid <andrew.d.henroid@intel.com>
> 
> The Intel 7300 Memory Controller supports dynamic throttling of memory which can
> be used to save power when system is idle. This driver does the memory
> throttling when all CPUs are idle on such a system.
> 
> Refer to "Intel 7300 Memory Controller Hub (MCH)" datasheet
> for the config space description.
> 
> Signed-off-by: Andy Henroid <andrew.d.henroid@intel.com>
> Signed-off-by: Len Brown <len.brown@intel.com>
> Signed-off-by: Venkatesh Pallipadi <venkatesh.pallipadi@intel.com>
> ---
>  MAINTAINERS               |    6 +
>  arch/x86/Kconfig          |    2 +
>  drivers/Makefile          |    1 +
>  drivers/dma/ioat_dma.c    |    3 +
>  drivers/idle/Kconfig      |   16 +
>  drivers/idle/Makefile     |    2 +
>  drivers/idle/i7300_idle.c |  674 +++++++++++++++++++++++++++++++++++++++++++++

Why not just use drivers/power/ instead of a new sub-directory?


>  include/asm-x86/idle.h    |    1 +
>  include/linux/pci_ids.h   |    1 +
>  9 files changed, 706 insertions(+), 0 deletions(-)
>  create mode 100644 drivers/idle/Kconfig
>  create mode 100644 drivers/idle/Makefile
>  create mode 100644 drivers/idle/i7300_idle.c
> 
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 8dae455..43f71b0 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -2078,6 +2078,12 @@ L:	linux-ide@vger.kernel.org
>  L:	linux-scsi@vger.kernel.org
>  S:	Orphan
>  
> +IDLE-I7300
> +P:	Andy Henroid
> +M:	andrew.d.henroid@intel.com

welcome back.

> +L:	linux-pm@lists.linux-foundation.org
> +S:	Supported
> +
>  IEEE 1394 SUBSYSTEM (drivers/ieee1394)
>  P:	Ben Collins
>  M:	ben.collins@ubuntu.com

> diff --git a/drivers/idle/i7300_idle.c b/drivers/idle/i7300_idle.c
> new file mode 100644
> index 0000000..59d1bbc
> --- /dev/null
> +++ b/drivers/idle/i7300_idle.c
> @@ -0,0 +1,674 @@
> +/*
> + * (C) Copyright 2008 Intel Corporation
> + * Authors:
> + * Andy Henroid <andrew.d.henroid@intel.com>
> + * Venkatesh Pallipadi <venkatesh.pallipadi@intel.com>
> + */
> +
> +
> +/*
> + * Value to set THRTLOW to when initiating throttling
> + *  0 = No throttling
> + *  1 = Throttle when > 4 activations per eval window (Maximum throttling)
> + *  2 = Throttle when > 8 activations
> + *  168 = Throttle when > 168 activations (Minimum throttling)

Why do some numbers mean > their value (i.e., 168) but others don't?
Is it a linear value or logarithmic or what?


> + */
> +#define MAX_THRTLWLIMIT		168
> +static uint i7300_idle_thrtlowlm = 1;
> +module_param_named(thrtlwlimit, i7300_idle_thrtlowlm, uint, 0644);
> +MODULE_PARM_DESC(thrtlwlimit,
> +		"Value for THRTLOWLM activation field "
> +		"(0 = disable throttle, 1 = Max throttle, 168 = Min throttle)");
> +

> +/* Stop I/O AT memory copy */
> +static void i7300_idle_ioat_stop(void)
> +{
> +	int i;
> +	u8 sts;
> +
> +	for (i = 0; i < 5; i++) {
> +		writeb(IOAT_CHANCMD_RESET,
> +			ioat_chanbase + IOAT1_CHANCMD_OFFSET);
> +
> +		udelay(10);
> +
> +		sts = readq(ioat_chanbase + IOAT1_CHANSTS_OFFSET) &
> +			IOAT_CHANSTS_DMA_TRANSFER_STATUS;

readq() returns 64 bits.  You only want to LS-byte of it here?

> +		if (sts != IOAT_CHANSTS_DMA_TRANSFER_STATUS_ACTIVE)
> +			break;
> +
> +	}
> +
> +	if (i == 5)
> +		dprintk("failed to suspend+reset I/O AT after 5 retries\n");
> +
> +}


> +struct debugfs_file_info {
> +	void *ptr;
> +	char name[32];
> +	struct dentry *file;
> +} debugfs_file_list[] = {
> +				{&total_starts, "total_starts", NULL},
> +				{&total_us, "total_us", NULL},
> +#ifdef DEBUG
> +				{&past_skip, "past_skip", NULL},
> +#endif
> +				{NULL, "", NULL}
> +			};

What is debugfs used for?  any docs for it?

Does this build/work if CONFIG_DEBUG_FS=n?


> +static int __init i7300_idle_init(void)
> +{
> +	spin_lock_init(&i7300_idle_lock);
> +	cpus_clear(idle_cpumask);
> +	total_us = 0;
> +
> +	if (i7300_idle_platform_probe())
> +		return -ENODEV;
> +
> +	if (i7300_idle_thrt_save())
> +		return -ENODEV;
> +
> +	if (i7300_idle_ioat_init())
> +		return -ENODEV;
> +
> +	debugfs_dir = debugfs_create_dir("i7300_idle", NULL);
> +	if (debugfs_dir) {
> +		int i = 0;
> +
> +		while (debugfs_file_list[i].ptr != NULL) {
> +			debugfs_file_list[i].file = debugfs_create_file(
> +					debugfs_file_list[i].name,
> +					S_IRUSR,
> +					debugfs_dir,
> +					debugfs_file_list[i].ptr,
> +					&idle_fops);
> +			i++;
> +		}
> +	}
> +
> +	idle_notifier_register(&i7300_idle_nb);
> +
> +	printk(KERN_INFO "i7300_idle: loaded v%s\n", I7300_IDLE_DRIVER_VERSION);
> +	return 0;
> +}


---
~Randy