All of lore.kernel.org
 help / color / mirror / Atom feed
* Save/Restore (PM) support in DSS2
@ 2010-03-22  5:13 Hiremath, Vaibhav
  2010-03-23 17:50 ` Kevin Hilman
  2010-03-24  7:55 ` Tomi Valkeinen
  0 siblings, 2 replies; 3+ messages in thread
From: Hiremath, Vaibhav @ 2010-03-22  5:13 UTC (permalink / raw)
  To: Tomi Valkeinen; +Cc: linux-omap

Hi Tomi and others,

As part of our internal release I found that save and restore support doesn't work as expected (I have fixed it for our release and full PM support works fine for me), and below is my analysis/observation and fix for the same - 

Observation - 
-----------

The save and restore of DSS registers happen inside function dss_clk_enable and dss_clk_disable. The save context gets executed as per expectation when actually required, i.e. when clock usage count goes to 0.

The issue is with restore functionality, the restore is blocked by various conditions - 

void dss_clk_enable(enum dss_clock clks)
{
        bool check_ctx = core.num_clks_enabled == 0;

        dss_clk_enable_no_ctx(clks);

        if (check_ctx && cpu_is_omap34xx() && dss_need_ctx_restore())
                restore_all_ctx();
}

The check for clock usage count is mandatory here (which was missing earlier) but I am more concerned about other of two, cpu_is_omap34xx() & dss_need_ctx_restore(). 

Why this is tied only to omap34xx? And 
Frankly I am quite not sure whether I understood use of dss_need_ctx_restore()? What are we trying to do here with function dss_need_ctx_restore()? It internally calls dss_get_ctx_id() which internally calls pdata->get_last_off_on_transaction_id().

Are we providing control to platform file when to restore? If yes, then what could be the trigger for that decision?

Currently none of OMAP board files defines this function, so it is always going to return 0. That means, without this function (and returns true) restore won't happen at all.

If I understand correctly, irrespective of which platform you are running on and whether you are hitting off/idle/retention state or not, you must save and restore states when your module interface clock usage count goes to zero. I may be missing something here.

So the function should be some thing like -


if (check_ctx || dss_need_ctx_restore())
    restore_all_ctx();

Although, I am not quite comfortable with function dss_need_ctx_restore(), so may not required at all.


Thanks,
Vaibhav Hiremath

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: Save/Restore (PM) support in DSS2
  2010-03-22  5:13 Save/Restore (PM) support in DSS2 Hiremath, Vaibhav
@ 2010-03-23 17:50 ` Kevin Hilman
  2010-03-24  7:55 ` Tomi Valkeinen
  1 sibling, 0 replies; 3+ messages in thread
From: Kevin Hilman @ 2010-03-23 17:50 UTC (permalink / raw)
  To: Hiremath, Vaibhav; +Cc: Tomi Valkeinen, linux-omap

"Hiremath, Vaibhav" <hvaibhav@ti.com> writes:

> Hi Tomi and others,
>
> As part of our internal release I found that save and restore support doesn't work as expected (I have fixed it for our release and full PM support works fine for me), and below is my analysis/observation and fix for the same - 
>
> Observation - 
> -----------
>
> The save and restore of DSS registers happen inside function dss_clk_enable and dss_clk_disable. The save context gets executed as per expectation when actually required, i.e. when clock usage count goes to 0.
>
> The issue is with restore functionality, the restore is blocked by various conditions - 
>
> void dss_clk_enable(enum dss_clock clks)
> {
>         bool check_ctx = core.num_clks_enabled == 0;
>
>         dss_clk_enable_no_ctx(clks);
>
>         if (check_ctx && cpu_is_omap34xx() && dss_need_ctx_restore())
>                 restore_all_ctx();
> }
>
> The check for clock usage count is mandatory here (which was missing earlier) but I am more concerned about other of two, cpu_is_omap34xx() & dss_need_ctx_restore(). 
>
> Why this is tied only to omap34xx? And 
> Frankly I am quite not sure whether I understood use of dss_need_ctx_restore()? What are we trying to do here with function dss_need_ctx_restore()? It internally calls dss_get_ctx_id() which internally calls pdata->get_last_off_on_transaction_id().
>
> Are we providing control to platform file when to restore? If yes, then what could be the trigger for that decision?
>
> Currently none of OMAP board files defines this function, so it is always going to return 0. That means, without this function (and returns true) restore won't happen at all.
>
> If I understand correctly, irrespective of which platform you are running on and whether you are hitting off/idle/retention state or not, you must save and restore states when your module interface clock usage count goes to zero. I may be missing something here.

You only need restore if the powerdomain has gone to off mode (or
OSWR, which is not yet in l-o.)  For retention or inactive, no context
save/restore is needed.

That pdata pointer was intended to hook up to the OMAP PM layer
funciton of the same name, which uses the has the smarts to determine
if context was actually lost.

Kevin

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: Save/Restore (PM) support in DSS2
  2010-03-22  5:13 Save/Restore (PM) support in DSS2 Hiremath, Vaibhav
  2010-03-23 17:50 ` Kevin Hilman
@ 2010-03-24  7:55 ` Tomi Valkeinen
  1 sibling, 0 replies; 3+ messages in thread
From: Tomi Valkeinen @ 2010-03-24  7:55 UTC (permalink / raw)
  To: ext Hiremath, Vaibhav; +Cc: linux-omap

Hi,

On Mon, 2010-03-22 at 06:13 +0100, ext Hiremath, Vaibhav wrote:
> Hi Tomi and others,
> 
> As part of our internal release I found that save and restore support doesn't work as expected (I have fixed it for our release and full PM support works fine for me), and below is my analysis/observation and fix for the same - 
> 
> Observation - 
> -----------
> 
> The save and restore of DSS registers happen inside function dss_clk_enable and dss_clk_disable. The save context gets executed as per expectation when actually required, i.e. when clock usage count goes to 0.
> 
> The issue is with restore functionality, the restore is blocked by various conditions - 
> 
> void dss_clk_enable(enum dss_clock clks)
> {
>         bool check_ctx = core.num_clks_enabled == 0;
> 
>         dss_clk_enable_no_ctx(clks);
> 
>         if (check_ctx && cpu_is_omap34xx() && dss_need_ctx_restore())
>                 restore_all_ctx();
> }
> 
> The check for clock usage count is mandatory here (which was missing earlier) but I am more concerned about other of two, cpu_is_omap34xx() & dss_need_ctx_restore(). 
> 
> Why this is tied only to omap34xx? And 

That's to make to code not to be run on OMAP2 boards. I don't think
there's similar PM on OMAP2s. If there is, the DSS PM code probably
needs some work to support it.

> Frankly I am quite not sure whether I understood use of dss_need_ctx_restore()? What are we trying to do here with function dss_need_ctx_restore()? It internally calls dss_get_ctx_id() which internally calls pdata->get_last_off_on_transaction_id().
> 
> Are we providing control to platform file when to restore? If yes, then what could be the trigger for that decision?
> 
> Currently none of OMAP board files defines this function, so it is always going to return 0. That means, without this function (and returns true) restore won't happen at all.
> 
> If I understand correctly, irrespective of which platform you are running on and whether you are hitting off/idle/retention state or not, you must save and restore states when your module interface clock usage count goes to zero. I may be missing something here.

I think Kevin already answered to all these. So basically you just set
the get_last_off_on_transaction_id to point to the PM's function, and it
will tell if context has been lost. 

 Tomi



^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2010-03-24  7:55 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-03-22  5:13 Save/Restore (PM) support in DSS2 Hiremath, Vaibhav
2010-03-23 17:50 ` Kevin Hilman
2010-03-24  7:55 ` Tomi Valkeinen

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.