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

[Andres Salomon]

On Sun, 23 Sep 2007 22:19:58 -0400
Bernardo Innocenti <bernie at codewiz.org> wrote:

> This patch merges the fb_powerup and fb_powerdown hooks in a single
> operation fb_power with an additional "state" parameter ranging
> from 0 (running) to 3 (poweroff).
> 
> 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.



>  }
> diff --git a/include/linux/fb.h b/include/linux/fb.h
> index 5a82c83..0acbd27 100644
> --- a/include/linux/fb.h
> +++ b/include/linux/fb.h
> @@ -661,11 +661,8 @@ struct fb_ops {
>  	/* restore saved state */
>  	void (*fb_restore_state)(struct fb_info *info);
>  
> -	/* Shut down the graphics engine to save power */
> -	int (*fb_powerdown)(struct fb_info *info);
> -
> -	/* Power it back up */
> -	int (*fb_powerup)(struct fb_info *info);
> +	/* Change the power state of the graphics engine (0 = fully on, 3 = fully off */
> +	int (*fb_power)(struct fb_info *info, int state);
>  


Jordan and I discussed using fb_blank() rather than adding an additional API.
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.

Also, the fb_power() API really needs a stricter definition of power levels.


-- 
Andres Salomon <dilinger at queued.net>