lkml.org 
[lkml]   [2016]   [Dec]   [20]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH v1 07/12] scsi: ufs: add option to change default UFS power management level
On 2016-12-19 10:38, Rob Herring wrote:
> On Tue, Dec 13, 2016 at 2:16 PM, Subhash Jadavani
> <subhashj@codeaurora.org> wrote:
>> On 2016-12-13 12:04, Rob Herring wrote:
>>>
>>> On Mon, Dec 12, 2016 at 04:54:20PM -0800, Subhash Jadavani wrote:
>>>>
>>>> UFS device and link can be put in multiple different low power modes
>>>> hence
>>>> UFS driver supports multiple different low power modes. By default
>>>> UFS
>>>> driver selects the default (optimal) low power mode (which gives
>>>> moderate
>>>> power savings and have relatively less enter and exit latencies) but
>>>> we might have to tune this default power mode for different chipset
>>>> platforms to meet the low power requirements/goals. Hence this patch
>>>> adds option to change default UFS low power mode (level).
>>>>
>>>> Reviewed-by: Yaniv Gardi <ygardi@codeaurora.org>
>>>> Signed-off-by: Subhash Jadavani <subhashj@codeaurora.org>
>>>> ---
>>>> .../devicetree/bindings/ufs/ufshcd-pltfrm.txt | 10 ++++++
>>>> drivers/scsi/ufs/ufshcd-pltfrm.c | 14 ++++++++
>>>> drivers/scsi/ufs/ufshcd.c | 39
>>>> ++++++++++++++++++++++
>>>> drivers/scsi/ufs/ufshcd.h | 4 +--
>>>> 4 files changed, 65 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/Documentation/devicetree/bindings/ufs/ufshcd-pltfrm.txt
>>>> b/Documentation/devicetree/bindings/ufs/ufshcd-pltfrm.txt
>>>> index a99ed55..c3836c5 100644
>>>> --- a/Documentation/devicetree/bindings/ufs/ufshcd-pltfrm.txt
>>>> +++ b/Documentation/devicetree/bindings/ufs/ufshcd-pltfrm.txt
>>>> @@ -41,6 +41,14 @@ Optional properties:
>>>> -lanes-per-direction : number of lanes available per direction -
>>>> either 1 or 2.
>>>> Note that it is assume same number of
>>>> lanes is
>>>> used both
>>>> directions at once. If not specified,
>>>> default
>>>> is 2 lanes per direction.
>>>> +- rpm-level : UFS Runtime power management level.
>>>> Following
>>>> PM levels are supported:
>>>> + 0 - Both UFS device and Link in active
>>>> state
>>>> (Highest power consumption)
>>>> + 1 - UFS device in active state but Link in
>>>> Hibern8 state
>>>> + 2 - UFS device in Sleep state but Link in
>>>> active state
>>>> + 3 - UFS device in Sleep state and Link in
>>>> hibern8 state (default PM level)
>>>> + 4 - UFS device in Power-down state and
>>>> Link in
>>>> Hibern8 state
>>>> + 5 - UFS device in Power-down state and
>>>> Link in
>>>> OFF state (Lowest power consumption)
>>>> +- spm-level : UFS System power management level. Allowed
>>>> PM
>>>> levels are same as rpm-level.
>>>
>>>
>>> This looks like you are putting policy for Linux into DT.
>>>
>>> What I would expect to see here is disabling of states that don't
>>> work
>>> due to some h/w limitation. Otherwise, it is a user decision for what
>>> modes to go into. Also, I think link and device states should be
>>> separate.
>>
>>
>> Yes, generally default level (3) is good enough (and recommended) for
>> all
>> platforms and most likely user is only expected to change this if they
>> see
>> issues (most H/W) on their platform or they want even more aggressive
>> power
>> state (level-4 or level-5) and ready to take the performance hit
>> associated
>> with resume latencies.
>
> What latencies can be tolerated is going to depend on the application
> and could vary while running, so putting in DT doesn't make sense. I
> would break down settings like this:
>
> broken h/w -> DT
> user tuning/config -> sysfs
> sensible defaults -> driver

Make sense.
we already have #2 and #3 in place, will rework this patch so we have a
way to specify what is broken in h/w.

>
>> Also, I think it is better to keep Link and device states tied, one
>> reason
>> is that we can't keep device in sleep/active state when Link is in OFF
>> state.
>
> The driver can tie the states to together if needed. Just document
> what's broken in DT and let the driver make decisions.

Yes, agreed. will rework this patch so we have a way to specify what is
broken in h/w and separate the device and link states (something like
broken-hibern8, broken-sleep etc.)
Thanks for the suggestions.

>
> Rob

--
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project

\
 
 \ /
  Last update: 2016-12-20 20:37    [W:0.061 / U:0.104 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site