linux-scsi.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Tejun Heo <htejun@gmail.com>
To: Arjan van de Ven <arjan@infradead.org>
Cc: Jeff Garzik <jeff@garzik.org>,
	Kristen Carlson Accardi <kristen.c.accardi@intel.com>,
	James.Bottomley@steeleye.com, linux-scsi@vger.kernel.org,
	akpm@linux-foundation.org, linux-kernel@vger.kernel.org,
	linux-ide@vger.kernel.org, edwintorok@gmail.com, axboe@kernel.dk
Subject: Re: [patch 2/4] Expose Power Management Policy option to users
Date: Tue, 31 Jul 2007 23:45:25 +0900	[thread overview]
Message-ID: <46AF4B05.4010700@gmail.com> (raw)
In-Reply-To: <1185891382.2750.8.camel@laptopd505.fenrus.org>

Arjan van de Ven wrote:
> On Tue, 2007-07-31 at 15:27 +0900, Tejun Heo wrote:
>> Jeff Garzik wrote:
>>> Any chance the SCSI peeps could ACK this, and then let me include it in
>>> the ALPM patchset in the libata tree?
>> ATA link PS is pretty complex with HIPM, DIPM and AHCI ALPM.  I'm not
>> sure whether this three level knob would be sufficient. 
> 
> adding more levels later is easy.

Dunno whether they would be linear levels and putting HIPM, DIPM, ALPM
selection into SCSI sysfs knob doesn't look so appealing.

>>  It might be
>> good enough if we're gonna develop extensive in-kernel black/white list
>> specifying which method works on which combination but my gut tells me
>> that it's best left to userland (probably in the form of per-notebook PS
>> profile).
> 
> either sucks. AHCI ALPM ought to work if it's supported; it's what other
> operating systems also use... 
>> Adding to the fun, there are quite a few broken devices out there which
>> act weirdly when link PS actions are taken.
> 
> do you have any specific examples that act funny with the patch in
> question here? (the patch tries to be careful, previous patches weren't
> always so please test this patch before claiming the concept as a whole
> is broken)

They were hardware problems.  I don't think any amount of proper
implementation can fix them.  I have one DVD RAM somewhere in my pile of
hardware which locks up solidly if any link PS mode is used and had a
report of a HDD which had problem with link PS.  Can't remember the
details tho.  Also, IIRC one of my wendies spins down on SLUMBER.

>> Also, I generally don't think AHCI ALPM is a good idea.  It doesn't have
>> 'cool down' period before entering PS state
> 
> that's a chipset implementation decision.... not part of the
> spec/technology per se.

That's actually something AHCI spec specifically states.  From section
8.3.1.3.

When PxCMD.ALPE is set to ‘1’, if the HBA recognizes that there are no
commands to process, the HBA shall initiate a transition to Partial or
Slumber interface power management state based upon the setting of
PxCMD.ASP. The HBA recognizes no commands to transmit as either:

    • PxSACT is set to 0h, and the HBA updates PxCI from a non-zero
value to 0h.
    • PxCI is set to 0h, and a Set Device Bits FIS is received that
updates PxSACT from a non-zero value to 0h.

Have no idea why it's specified this way.  Adding 100ms or 1s cool down
time wouldn't burn noticeably more power.  Maybe AHCI expects the host
driver to queue commands even for non-NCQ drives but libata currently
doesn't do that.

Anyways, I don't really think this attribute belongs to SCSI sysfs
hierarchy.  There currently isn't any alternative but sysfs is part of
userland visible interface and putting something into SCSI sysfs
hierarchy just because libata doesn't have one doesn't look like a good
idea.

sysfs isn't far from being detached from kobject and driver model.  I
think it would be best to wait a bit and build proper libata sysfs
hierarchy which won't have to be changed later when libata departs from
SCSI.  Well, it isn't really a good way but IMHO it's better than
sticking ATA power saving node into SCSI sysfs hierarchy.

Thanks.

-- 
tejun

  reply	other threads:[~2007-07-31 14:45 UTC|newest]

Thread overview: 43+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <20070705194909.337398431@intel.com>
2007-07-05 20:05 ` [patch 1/4] Store interrupt value Kristen Carlson Accardi
2007-08-01  8:18   ` Tejun Heo
2007-08-01 14:01   ` Jeff Garzik
2007-08-01 21:18     ` Kristen Carlson Accardi
2007-07-05 20:05 ` [patch 2/4] Expose Power Management Policy option to users Kristen Carlson Accardi
2007-07-09 19:36   ` Pavel Machek
2007-07-11 16:51     ` Kristen Carlson Accardi
2007-07-30 16:32   ` Jeff Garzik
2007-07-31  6:27     ` Tejun Heo
2007-07-31 14:16       ` Arjan van de Ven
2007-07-31 14:45         ` Tejun Heo [this message]
2007-07-31 16:15           ` Arjan van de Ven
2007-07-31 18:29             ` Tejun Heo
2007-08-01  9:23               ` Tejun Heo
2007-08-01 16:31                 ` Kristen Carlson Accardi
2007-08-01 21:16                 ` Kristen Carlson Accardi
2007-08-09 16:10                   ` Kristen Carlson Accardi
2007-07-31 16:18           ` Kristen Carlson Accardi
2007-07-31 17:48             ` Tejun Heo
2007-07-31 20:24               ` Kristen Carlson Accardi
2007-08-01  3:20                 ` Tejun Heo
2007-07-31 14:58         ` Tejun Heo
2007-07-31 14:18       ` James Bottomley
2007-08-01 16:53         ` Matthew Wilcox
2007-08-01 17:06           ` James Bottomley
2007-07-31 16:30       ` Kristen Carlson Accardi
2007-07-31 18:02         ` Tejun Heo
2007-07-31 19:58           ` Kristen Carlson Accardi
2007-08-01  3:24             ` Tejun Heo
2007-08-01 15:52               ` Kristen Carlson Accardi
2007-08-01 21:07               ` Kristen Carlson Accardi
2007-07-05 20:05 ` [patch 3/4] Enable link power management for ata drivers Kristen Carlson Accardi
2007-07-05 22:33   ` Andrew Morton
2007-07-05 22:37     ` Andrew Morton
2007-07-06  0:01       ` Jeff Garzik
2007-07-06  0:02       ` Jeff Garzik
2007-07-06  0:17         ` Andrew Morton
2007-07-06  0:00     ` Jeff Garzik
2007-08-01  8:27   ` Tejun Heo
2007-08-01  9:45     ` edwintorok
2007-08-01 21:11     ` Kristen Carlson Accardi
2007-08-02  5:27       ` Tejun Heo
2007-07-05 20:05 ` [patch 4/4] Enable Aggressive Link Power management for AHCI controllers Kristen Carlson Accardi

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=46AF4B05.4010700@gmail.com \
    --to=htejun@gmail.com \
    --cc=James.Bottomley@steeleye.com \
    --cc=akpm@linux-foundation.org \
    --cc=arjan@infradead.org \
    --cc=axboe@kernel.dk \
    --cc=edwintorok@gmail.com \
    --cc=jeff@garzik.org \
    --cc=kristen.c.accardi@intel.com \
    --cc=linux-ide@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-scsi@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).