Hi there,
Well, I did not modify the content of the patch. It can be 'git am' on
ubuntu kernel without conflict.
This patch is directly from git://git.linaro.org/amitarora/powertop.git
commit cc1aaff10ee9046b835441d295fda498f68cb1ac
Author: Arjan van de Ven <[email protected]>
Date: Mon Jul 12 17:25:49 2010 -0400
alpm kernel patch
Based on my understanding, powertop only uses it to collect harddisk's
status. so if it is risky to merge to current ubuntu kernel, we may wait a
better version of this from Arjan.
Yong
On Wed, Sep 15, 2010 at 6:33 AM, Leann Ogasawara <
[email protected]> wrote:
> On Tue, 2010-09-14 at 10:37 -0600, Tim Gardner wrote:
> > On 09/13/2010 12:31 AM, [email protected] wrote:
> > > From: Arjan van de Ven<[email protected]>
> > >
> > > PowerTOP wants to be able to show the user how effective the ALPM link
> > > power management is for the user. ALPM is worth around 0.5W on a quiet
> > > link; PowerTOP wants to be able to find cases where the "quiet link"
> isn't
> > > actually quiet.
> > >
> > > This patch adds state accounting functionality to the AHCI driver for
> > > PowerTOP to use.
> > > The parts of the patch are
> > > 1) the sysfs logic of exposing the stats for each state in sysfs
> > > 2) the basic accounting logic that gets update on link change
> interrupts
> > > (or when the user accesses the info from sysfs)
> > > 3) a "accounting enable" flag; in order to get the accounting to work,
> > > the driver needs to get phyrdy interrupts on link status changes.
> > > Normally and currently this is disabled by the driver when ALPM is
> > > on (to reduce overhead); when PowerTOP is running this will need
> > > to be on to get usable statistics... hence the sysfs tunable.
> > >
> > > The PowerTOP output currently looks like this:
> > >
> > > Recent SATA AHCI link activity statistics
> > > Active Partial Slumber Device name
> > > 0.5% 99.5% 0.0% host0
> > >
> > > (work to resolve "host0" to a more human readable name is in progress)
> > >
> > > Signed-off-by: Arjan van de Ven<[email protected]>
> > > Signed-off-by: Shen Yong<[email protected]>
> > > ---
> > > drivers/ata/ahci.h | 15 ++++
> > > drivers/ata/libahci.c | 190
> ++++++++++++++++++++++++++++++++++++++++++++++++-
> > > 2 files changed, 203 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/drivers/ata/ahci.h b/drivers/ata/ahci.h
> > > index 7113c57..6a3a291 100644
> > > --- a/drivers/ata/ahci.h
> > > +++ b/drivers/ata/ahci.h
> > > @@ -261,6 +261,13 @@ struct ahci_em_priv {
> > > unsigned long led_state;
> > > };
> > >
> > > +enum ahci_port_states {
> > > + AHCI_PORT_NOLINK = 0,
> > > + AHCI_PORT_ACTIVE = 1,
> > > + AHCI_PORT_PARTIAL = 2,
> > > + AHCI_PORT_SLUMBER = 3
> > > +};
> > > +
> > > struct ahci_port_priv {
> > > struct ata_link *active_link;
> > > struct ahci_cmd_hdr *cmd_slot;
> > > @@ -279,6 +286,14 @@ struct ahci_port_priv {
> > > int fbs_last_dev; /* save FBS.DEV of last FIS
> */
> > > /* enclosure management info per PM slot */
> > > struct ahci_em_priv em_priv[EM_MAX_SLOTS];
> > > +
> > > + /* ALPM accounting state and stats */
> > > + unsigned int accounting_active:1;
> > > + u64 active_jiffies;
> > > + u64 partial_jiffies;
> > > + u64 slumber_jiffies;
> > > + int previous_state;
> > > + int previous_jiffies;
> > > };
> > >
> > > struct ahci_host_priv {
> > > diff --git a/drivers/ata/libahci.c b/drivers/ata/libahci.c
> > > index 81e772a..c3250ee 100644
> > > --- a/drivers/ata/libahci.c
> > > +++ b/drivers/ata/libahci.c
> > > @@ -59,6 +59,20 @@ MODULE_PARM_DESC(ignore_sss, "Ignore staggered
> spinup flag (0=don't ignore, 1=ig
> > > static int ahci_enable_alpm(struct ata_port *ap,
> > > enum link_pm policy);
> > > static void ahci_disable_alpm(struct ata_port *ap);
> > > +static ssize_t ahci_alpm_show_active(struct device *dev,
> > > + struct device_attribute *attr, char
> *buf);
> > > +static ssize_t ahci_alpm_show_slumber(struct device *dev,
> > > + struct device_attribute *attr, char
> *buf);
> > > +static ssize_t ahci_alpm_show_partial(struct device *dev,
> > > + struct device_attribute *attr, char
> *buf);
> > > +
> > > +static ssize_t ahci_alpm_show_accounting(struct device *dev,
> > > + struct device_attribute *attr, char
> *buf);
> > > +
> > > +static ssize_t ahci_alpm_set_accounting(struct device *dev,
> > > + struct device_attribute *attr,
> > > + const char *buf, size_t count);
> > > +
> > > static ssize_t ahci_led_show(struct ata_port *ap, char *buf);
> > > static ssize_t ahci_led_store(struct ata_port *ap, const char *buf,
> > > size_t size);
> > > @@ -118,6 +132,12 @@ static DEVICE_ATTR(ahci_host_caps, S_IRUGO,
> ahci_show_host_caps, NULL);
> > > static DEVICE_ATTR(ahci_host_cap2, S_IRUGO, ahci_show_host_cap2,
> NULL);
> > > static DEVICE_ATTR(ahci_host_version, S_IRUGO,
> ahci_show_host_version, NULL);
> > > static DEVICE_ATTR(ahci_port_cmd, S_IRUGO, ahci_show_port_cmd, NULL);
> > > +static DEVICE_ATTR(ahci_alpm_active, S_IRUGO, ahci_alpm_show_active,
> NULL);
> > > +static DEVICE_ATTR(ahci_alpm_partial, S_IRUGO, ahci_alpm_show_partial,
> NULL);
> > > +static DEVICE_ATTR(ahci_alpm_slumber, S_IRUGO, ahci_alpm_show_slumber,
> NULL);
> > > +static DEVICE_ATTR(ahci_alpm_accounting, S_IRUGO | S_IWUSR,
> > > + ahci_alpm_show_accounting, ahci_alpm_set_accounting);
> > > +
> > > static DEVICE_ATTR(em_buffer, S_IWUSR | S_IRUGO,
> > > ahci_read_em_buffer, ahci_store_em_buffer);
> > >
> > > @@ -129,6 +149,10 @@ static struct device_attribute *ahci_shost_attrs[]
> = {
> > > &dev_attr_ahci_host_cap2,
> > > &dev_attr_ahci_host_version,
> > > &dev_attr_ahci_port_cmd,
> > > + &dev_attr_ahci_alpm_active,
> > > + &dev_attr_ahci_alpm_partial,
> > > + &dev_attr_ahci_alpm_slumber,
> > > + &dev_attr_ahci_alpm_accounting,
> > > &dev_attr_em_buffer,
> > > NULL
> > > };
> > > @@ -734,9 +758,14 @@ static int ahci_enable_alpm(struct ata_port *ap,
> > > * getting woken up due to spurious phy ready interrupts
> > > * TBD - Hot plug should be done via polling now, is
> > > * that even supported?
> > > + *
> > > + * However, when accounting_active is set, we do want
> > > + * the interrupts for accounting purposes.
> > > */
> > > - pp->intr_mask&= ~PORT_IRQ_PHYRDY;
> > > - writel(pp->intr_mask, port_mmio + PORT_IRQ_MASK);
> > > + if (!pp->accounting_active) {
> > > + pp->intr_mask&= ~PORT_IRQ_PHYRDY;
> > > + writel(pp->intr_mask, port_mmio + PORT_IRQ_MASK);
> > > + }
> > >
> > > /*
> > > * Set a flag to indicate that we should ignore all PhyRdy
> > > @@ -1645,6 +1674,162 @@ static void ahci_error_intr(struct ata_port
> *ap, u32 irq_stat)
> > > ata_port_abort(ap);
> > > }
> > >
> > > +static int get_current_alpm_state(struct ata_port *ap)
> > > +{
> > > + u32 status = 0;
> > > +
> > > + ahci_scr_read(&ap->link, SCR_STATUS,&status);
> > > +
> > > + /* link status is in bits 11-8 */
> > > + status = status>> 8;
> > > + status = status& 0x7;
> > > +
> > > + if (status == 6)
> > > + return AHCI_PORT_SLUMBER;
> > > + if (status == 2)
> > > + return AHCI_PORT_PARTIAL;
> > > + if (status == 1)
> > > + return AHCI_PORT_ACTIVE;
> > > + return AHCI_PORT_NOLINK;
> > > +}
> > > +
> > > +static void account_alpm_stats(struct ata_port *ap)
> > > +{
> > > + struct ahci_port_priv *pp;
> > > +
> > > + int new_state;
> > > + u64 new_jiffies, jiffies_delta;
> > > +
> > > + if (ap == NULL)
> > > + return;
> > > + pp = ap->private_data;
> > > +
> > > + if (!pp) return;
> > > +
> > > + new_state = get_current_alpm_state(ap);
> > > + new_jiffies = jiffies;
> > > +
> > > + jiffies_delta = new_jiffies - pp->previous_jiffies;
> > > +
> > > + switch (pp->previous_state) {
> > > + case AHCI_PORT_NOLINK:
> > > + pp->active_jiffies = 0;
> > > + pp->partial_jiffies = 0;
> > > + pp->slumber_jiffies = 0;
> > > + break;
> > > + case AHCI_PORT_ACTIVE:
> > > + pp->active_jiffies += jiffies_delta;
> > > + break;
> > > + case AHCI_PORT_PARTIAL:
> > > + pp->partial_jiffies += jiffies_delta;
> > > + break;
> > > + case AHCI_PORT_SLUMBER:
> > > + pp->slumber_jiffies += jiffies_delta;
> > > + break;
> > > + default:
> > > + break;
> > > + }
> > > + pp->previous_state = new_state;
> > > + pp->previous_jiffies = new_jiffies;
> > > +}
> > > +
> > > +static ssize_t ahci_alpm_show_active(struct device *dev,
> > > + struct device_attribute *attr, char
> *buf)
> > > +{
> > > + struct Scsi_Host *shost = class_to_shost(dev);
> > > + struct ata_port *ap = ata_shost_to_port(shost);
> > > + struct ahci_port_priv *pp;
> > > +
> > > + if (!ap || ata_port_is_dummy(ap))
> > > + return -EINVAL;
> > > + pp = ap->private_data;
> > > + account_alpm_stats(ap);
> > > +
> > > + return sprintf(buf, "%u\n", jiffies_to_msecs(pp->active_jiffies));
> > > +}
> > > +
> > > +static ssize_t ahci_alpm_show_partial(struct device *dev,
> > > + struct device_attribute *attr, char
> *buf)
> > > +{
> > > + struct Scsi_Host *shost = class_to_shost(dev);
> > > + struct ata_port *ap = ata_shost_to_port(shost);
> > > + struct ahci_port_priv *pp;
> > > +
> > > + if (!ap || ata_port_is_dummy(ap))
> > > + return -EINVAL;
> > > +
> > > + pp = ap->private_data;
> > > + account_alpm_stats(ap);
> > > +
> > > + return sprintf(buf, "%u\n", jiffies_to_msecs(pp->partial_jiffies));
> > > +}
> > > +
> > > +static ssize_t ahci_alpm_show_slumber(struct device *dev,
> > > + struct device_attribute *attr, char
> *buf)
> > > +{
> > > + struct Scsi_Host *shost = class_to_shost(dev);
> > > + struct ata_port *ap = ata_shost_to_port(shost);
> > > + struct ahci_port_priv *pp;
> > > +
> > > + if (!ap || ata_port_is_dummy(ap))
> > > + return -EINVAL;
> > > +
> > > + pp = ap->private_data;
> > > +
> > > + account_alpm_stats(ap);
> > > +
> > > + return sprintf(buf, "%u\n", jiffies_to_msecs(pp->slumber_jiffies));
> > > +}
> > > +
> > > +static ssize_t ahci_alpm_show_accounting(struct device *dev,
> > > + struct device_attribute *attr, char *buf)
> > > +{
> > > + struct Scsi_Host *shost = class_to_shost(dev);
> > > + struct ata_port *ap = ata_shost_to_port(shost);
> > > + struct ahci_port_priv *pp;
> > > +
> > > + if (!ap || ata_port_is_dummy(ap))
> > > + return -EINVAL;
> > > +
> > > + pp = ap->private_data;
> > > +
> > > + return sprintf(buf, "%u\n", pp->accounting_active);
> > > +}
> > > +
> > > +static ssize_t ahci_alpm_set_accounting(struct device *dev,
> > > + struct device_attribute *attr,
> > > + const char *buf, size_t count)
> > > +{
> > > + unsigned long flags;
> > > + struct Scsi_Host *shost = class_to_shost(dev);
> > > + struct ata_port *ap = ata_shost_to_port(shost);
> > > + struct ahci_port_priv *pp;
> > > + void __iomem *port_mmio;
> > > +
> > > + if (!ap || ata_port_is_dummy(ap))
> > > + return 1;
> > > +
> > > + pp = ap->private_data;
> > > + port_mmio = ahci_port_base(ap);
> > > +
> > > + if (!pp)
> > > + return 1;
> > > + if (buf[0] == '0')
> > > + pp->accounting_active = 0;
> > > + if (buf[0] == '1')
> > > + pp->accounting_active = 1;
> > > +
> > > + /* we need to enable the PHYRDY interrupt when we want accounting
> */
> > > + if (pp->accounting_active) {
> > > + spin_lock_irqsave(ap->lock, flags);
> > > + pp->intr_mask |= PORT_IRQ_PHYRDY;
> > > + writel(pp->intr_mask, port_mmio + PORT_IRQ_MASK);
> > > + spin_unlock_irqrestore(ap->lock, flags);
> > > + }
> > > + return count;
> > > +}
> > > +
> > > +
> > > static void ahci_port_intr(struct ata_port *ap)
> > > {
> > > void __iomem *port_mmio = ahci_port_base(ap);
> > > @@ -1670,6 +1855,7 @@ static void ahci_port_intr(struct ata_port *ap)
> > > if ((hpriv->flags& AHCI_HFLAG_NO_HOTPLUG)&&
> > > (status& PORT_IRQ_PHYRDY)) {
> > > status&= ~PORT_IRQ_PHYRDY;
> > > + account_alpm_stats(ap);
> > > ahci_scr_write(&ap->link, SCR_ERROR, ((1<< 16) | (1<<
> 18)));
> > > }
> > >
> >
> > Yong Shen - is this patch faithful to the version released by Arjan?
>
> This looks like a modified verison of the patch which Arjan submitted
> upstream a while back, ie Nov 2009:
>
> http://thread.gmane.org/gmane.linux.ide/43337
>
> Has there been any recent follow up to get this accepted upstream?
>
> > Since its not upstream I can't really tell. Otherwise this patch looks
> > OK since accounting is not on by default. Note that statistics gathering
> > within the PHY interrupt handler could have an impact on throughput.
> > This is definitely a SAUCE patch since its not upstream, and is likely
> > to suffer some edits before acceptance.
> >
> > Does Maverick have a version of powertop that uses any of these sysfs
> > entries?
> >
> > Acked-by: Tim Gardner <[email protected]>
>
> Applied to Maverick linux master but marked as SAUCE.
>
> Thanks,
> Leann
>
>
_______________________________________________
linaro-dev mailing list
[email protected]
http://lists.linaro.org/mailman/listinfo/linaro-dev
