All of lore.kernel.org
 help / color / mirror / Atom feed
From: Kevin Hilman <khilman@ti.com>
To: Paul Walmsley <paul@pwsan.com>
Cc: linux-omap@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
	Tero Kristo <tero.kristo@nokia.com>
Subject: Re: [PATCH 1/2] OMAP3: cpuidle: prevent CORE power domain from going to RET or OFF when DSS is on
Date: Thu, 03 Mar 2011 15:24:48 -0800	[thread overview]
Message-ID: <87pqq7pz27.fsf@ti.com> (raw)
In-Reply-To: <20110301224918.32613.95176.stgit@twilight.localdomain> (Paul Walmsley's message of "Tue, 01 Mar 2011 15:49:24 -0700")

Paul Walmsley <paul@pwsan.com> writes:

> From: Tero Kristo <tero.kristo@nokia.com>
>
> Prevent the CORE power domain from entering RETENTION or OFF when DSS
> is on.  Otherwise, the display FIFO(s) may underflow due to the time
> needed for the CORE to wake back up, causing tearing and unnecessary
> interrupts.
>
> Signed-off-by: Tero Kristo <tero.kristo@nokia.com>
> [paul@pwsan.com: wrote commit message]
> Signed-off-by: Paul Walmsley <paul@pwsan.com>

This isn't quite ready for merge, and needs a little more testing with
current DSS driver and mainline.

> ---
>  arch/arm/mach-omap2/cpuidle34xx.c |   10 ++++++++--
>  1 files changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/arch/arm/mach-omap2/cpuidle34xx.c b/arch/arm/mach-omap2/cpuidle34xx.c
> index 0335cd8..d1b7789 100644
> --- a/arch/arm/mach-omap2/cpuidle34xx.c
> +++ b/arch/arm/mach-omap2/cpuidle34xx.c
> @@ -9,8 +9,9 @@
>   * Copyright (C) 2007 Texas Instruments, Inc.
>   * Karthik Dasu <karthik-dp@ti.com>
>   *
> - * Copyright (C) 2006 Nokia Corporation
> + * Copyright (C) 2006, 2011 Nokia Corporation
>   * Tony Lindgren <tony@atomide.com>
> + * Tero Kristo <tero.kristo@nokia.com>
>   *
>   * Copyright (C) 2005 Texas Instruments, Inc.
>   * Richard Woodruff <r-woodruff2@ti.com>
> @@ -268,6 +269,12 @@ static int omap3_enter_idle_bm(struct cpuidle_device *dev,
>  		goto select_state;
>  	}
>  
> +	/* If DSS is active, prevent CORE RET/OFF */
> +	dss_state = pwrdm_read_pwrst(dss_pd);
> +	if (dss_state == PWRDM_POWER_ON &&
> +	    core_next_state != PWRDM_POWER_ON)
> +		core_next_state = PWRDM_POWER_INACTIVE;
> +

Due to sleepdeps/autodeps, when this code runs, DSS powerdomain is
always on.  The result is that CORE is always set to INACTIVE.

A side effect of this problem is exposing a known issue with PER wakeups
(UART3, GPIO).  Since CORE never goes to retention/off, IO-pad
wakeups are never enabled, so PER wakeups do not work.

I think we need a different way of checking for DSS activity.

Sounds like another usecase for idle notifiers.

Kevin

WARNING: multiple messages have this Message-ID (diff)
From: khilman@ti.com (Kevin Hilman)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH 1/2] OMAP3: cpuidle: prevent CORE power domain from going to RET or OFF when DSS is on
Date: Thu, 03 Mar 2011 15:24:48 -0800	[thread overview]
Message-ID: <87pqq7pz27.fsf@ti.com> (raw)
In-Reply-To: <20110301224918.32613.95176.stgit@twilight.localdomain> (Paul Walmsley's message of "Tue, 01 Mar 2011 15:49:24 -0700")

Paul Walmsley <paul@pwsan.com> writes:

> From: Tero Kristo <tero.kristo@nokia.com>
>
> Prevent the CORE power domain from entering RETENTION or OFF when DSS
> is on.  Otherwise, the display FIFO(s) may underflow due to the time
> needed for the CORE to wake back up, causing tearing and unnecessary
> interrupts.
>
> Signed-off-by: Tero Kristo <tero.kristo@nokia.com>
> [paul at pwsan.com: wrote commit message]
> Signed-off-by: Paul Walmsley <paul@pwsan.com>

This isn't quite ready for merge, and needs a little more testing with
current DSS driver and mainline.

> ---
>  arch/arm/mach-omap2/cpuidle34xx.c |   10 ++++++++--
>  1 files changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/arch/arm/mach-omap2/cpuidle34xx.c b/arch/arm/mach-omap2/cpuidle34xx.c
> index 0335cd8..d1b7789 100644
> --- a/arch/arm/mach-omap2/cpuidle34xx.c
> +++ b/arch/arm/mach-omap2/cpuidle34xx.c
> @@ -9,8 +9,9 @@
>   * Copyright (C) 2007 Texas Instruments, Inc.
>   * Karthik Dasu <karthik-dp@ti.com>
>   *
> - * Copyright (C) 2006 Nokia Corporation
> + * Copyright (C) 2006, 2011 Nokia Corporation
>   * Tony Lindgren <tony@atomide.com>
> + * Tero Kristo <tero.kristo@nokia.com>
>   *
>   * Copyright (C) 2005 Texas Instruments, Inc.
>   * Richard Woodruff <r-woodruff2@ti.com>
> @@ -268,6 +269,12 @@ static int omap3_enter_idle_bm(struct cpuidle_device *dev,
>  		goto select_state;
>  	}
>  
> +	/* If DSS is active, prevent CORE RET/OFF */
> +	dss_state = pwrdm_read_pwrst(dss_pd);
> +	if (dss_state == PWRDM_POWER_ON &&
> +	    core_next_state != PWRDM_POWER_ON)
> +		core_next_state = PWRDM_POWER_INACTIVE;
> +

Due to sleepdeps/autodeps, when this code runs, DSS powerdomain is
always on.  The result is that CORE is always set to INACTIVE.

A side effect of this problem is exposing a known issue with PER wakeups
(UART3, GPIO).  Since CORE never goes to retention/off, IO-pad
wakeups are never enabled, so PER wakeups do not work.

I think we need a different way of checking for DSS activity.

Sounds like another usecase for idle notifiers.

Kevin

  reply	other threads:[~2011-03-03 23:24 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-03-01 22:49 [PATCH 0/2] OMAP3: cpuidle: prevent CORE pwrdm from entering low-power state when DSS active Paul Walmsley
2011-03-01 22:49 ` Paul Walmsley
2011-03-01 22:49 ` [PATCH 1/2] OMAP3: cpuidle: prevent CORE power domain from going to RET or OFF when DSS is on Paul Walmsley
2011-03-01 22:49   ` Paul Walmsley
2011-03-03 23:24   ` Kevin Hilman [this message]
2011-03-03 23:24     ` Kevin Hilman
2011-03-04  8:14     ` Tero.Kristo
2011-03-04  8:14       ` Tero.Kristo at nokia.com
2011-03-04 16:56       ` Kevin Hilman
2011-03-04 16:56         ` Kevin Hilman
2011-03-07 12:02         ` Tero.Kristo
2011-03-07 12:02           ` Tero.Kristo at nokia.com
2011-03-07 19:06           ` Kevin Hilman
2011-03-07 19:06             ` Kevin Hilman
2011-03-10 10:17           ` Paul Walmsley
2011-03-10 10:17             ` Paul Walmsley
2011-03-09  0:04       ` Paul Walmsley
2011-03-09  0:04         ` Paul Walmsley
2011-03-01 22:49 ` [PATCH 2/2] OMAP3: cpuidle: add more details to the DSS-related CORE power domain state restriction Paul Walmsley
2011-03-01 22:49   ` Paul Walmsley
2011-03-02  8:18 ` [PATCH 0/2] OMAP3: cpuidle: prevent CORE pwrdm from entering low-power state when DSS active Tero.Kristo
2011-03-02  8:18   ` Tero.Kristo at nokia.com

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=87pqq7pz27.fsf@ti.com \
    --to=khilman@ti.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-omap@vger.kernel.org \
    --cc=paul@pwsan.com \
    --cc=tero.kristo@nokia.com \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.