[PATCH] GEODE: decouple sleep/resume from powerdown/powerup (take 2)

[Andres Salomon]

On Tue, 25 Sep 2007 02:18:09 -0400
Bernardo Innocenti <bernie at codewiz.org> wrote:

> Andres Salomon wrote:
> 
> >> diff --git a/drivers/video/geode/lxfb_ops.c b/drivers/video/geode/lxfb_ops.c
> >> index b2ecb4d..b380238 100644
> >> --- a/drivers/video/geode/lxfb_ops.c
> >> +++ b/drivers/video/geode/lxfb_ops.c
> >> @@ -829,33 +829,42 @@ static void lx_restore_regs(struct fb_info *info, struct geoderegs *regs)
> > [...]
> >> -int lx_shutdown(struct fb_info *info)
> >> +int lx_power(struct fb_info *info, int state)
> >>  {
> >>  	struct lxfb_par *par = info->par;
> >>  
> >> -	if (lx_power_on == 0)
> >> -		return 0;
> >> -
> >> -	writel(DC_UNLOCK_CODE, par->dc_regs + DC_UNLOCK);
> >> -	lx_save_regs(info, &saved_regs);
> >> -	lx_graphics_disable(info);
> >> +	if (state < 0 || state > 3)
> >> +		return -EINVAL;
> >>  
> >> -	lx_power_on = 0;
> >> -	return 0;
> >> -}
> >> +	/* Going into power save */
> >> +	if (state >= 2 && lx_power_state < 2)
> >> +		lx_graphics_disable(info);
> >>  
> >> -int lx_powerup(struct fb_info *info)
> >> -{
> >> -	struct lxfb_par *par = info->par;
> >> +	/* Powering down */
> >> +	if (state >= 3 && lx_power_state < 3) {
> >> +		writel(DC_UNLOCK_CODE, par->dc_regs + DC_UNLOCK);
> >> +		lx_save_regs(info, &saved_regs);
> >> +	}
> >>  
> >> -	if (lx_power_on == 1)
> >> -		return 0;
> >> +	/* Restoring from power save */
> >> +	if (state < 3 && lx_power_state >= 3) {
> >> +		lx_restore_regs(info, &saved_regs);
> >> +		writel(0, par->dc_regs + DC_UNLOCK);
> >> +	}
> >>  
> >> -	lx_restore_regs(info, &saved_regs);
> >> -	writel(0, par->dc_regs + DC_UNLOCK);
> >> +	/* Powering up */
> >> +	if (state < 2 && lx_power_state >= 2)
> >> +		lx_graphics_enable(info);
> >>  
> >> -	lx_power_on = 1;
> >> +	lx_power_state = state;
> >>  	return 0;
> > 
> > 
> > Jordan claims that we don't want to be calling lx_{save,restore}_regs
> > for state 2...  I'll let him explain.
> 
> We aren't doing that already.
> 
> We save registers only on the transition from 2 to 3 and we restore them
> when we go back from 3 to 2.
> 
> On the GX, we *save* them always and restore them only on powerup.
> Saving the registers is needed because, later on, we peek a few values
> from that structure for powering up the video unit.
> 


Er, brain fart.  We don't want to be calling lx_graphics_{en,dis}able when
transitioning to state 2; that shuts down too many things.  That's what
I meant to say.


> 
> > Jordan and I discussed using fb_blank() rather than adding an additional API.
> 
> Was it on IRC or email?  Could you please forward it to me?
> 

IRC.

> 
> > Basically, my feeling is that fb_blank is already doing (or should be doing)
> > what we want to be doing (disabling misc hardware bits that are not needed
> > when the video processor is no longer pulling images from the GPU).  So, we
> > might as well just be calling fb_blank from the dcon's sleep/freeze functions.
> > Jordan prefers the fb_power* stuff.  We can argue about it later, with
> > upstream.
> 
> Using the fb_blank() interface seems like a very good idea to me.  The different
> kinds of blanking can be mapped to DCON freeze, video off, and complete poweroff.
> 
> This rework will take some time.  Could we push it after Trial3?  For now, I see
> this patch as a quick bugfix only.  And the change brings the fb_power() API
> somewhat *closer* to what would be needed for switching to fb_
> 
> 
> > Also, the fb_power() API really needs a stricter definition of power levels.
> 
> I looked into pci_power_t and it looks close to what we need:
> 
>  #define PCI_D0          ((pci_power_t __force) 0)
>  #define PCI_D1          ((pci_power_t __force) 1)
>  #define PCI_D2          ((pci_power_t __force) 2)
>  #define PCI_D3hot       ((pci_power_t __force) 3)
>  #define PCI_D3cold      ((pci_power_t __force) 4)
>  #define PCI_UNKNOWN     ((pci_power_t __force) 5)
>  #define PCI_POWER_ERROR ((pci_power_t __force) -1)
> 
> But this change will have to be undone later if we move to fb_blank().
> To save time, couldn't we just elide it?
> 
> -- 
>  \___/
>  |___|  Bernardo Innocenti - http://www.codewiz.org/
>   \___\ One Laptop Per Child - http://www.laptop.org/
> _______________________________________________
> Devel mailing list
> Devel at lists.laptop.org
> http://lists.laptop.org/listinfo/devel


-- 
Andres Salomon <dilinger at laptop.org>