Re: Fbdev and PM patch.

[Benjamin Herrenschmidt <benh@kernel.crashing.org>]

On Fri, 2003-09-12 at 20:08, James Simmons wrote:
> Hi!
> 
>     I looked at your PM patch and I have a few ideas. The first thing I 
> did was create a dummy struct fb_ops instead of dummy functions for each 
> function for fb_ops. Plus you only had dummy functions for the basic 
> functions. So in the style of dummycon.c I created a dummy struct fb_ops.
>     I like to seperate out the client management code out of fbmem.c into 
> a seperate file. Especially with the device handling stuff it is quite 
> ugly looking. Here is what I suggest:

I kept the fb_ops on purpose. If you look at my implementation in
radeonfb, you'll see for example that the check_var/set_par fully
works during sleep, except that the HW isn't touched, but on wakeup,
the mode that was set during sleep will be applied properly.

If you put dummy fb_ops, you'll either let var be changed without
control from the driver, or make any set_var fail, which is a policy
decision I didn't want to take (but acceptable).

>     struct fb_client_ops {
> 	struct module *owner;
> 	void (*state_manager)(struct fb_info *info, void *data, u32 state);
>     }
> 
> The idea is instead of a bunch of functions we have one function for 
> state management. In struct fb_info we keep track of the state flag to see
> what the states are. We have something like this:
> 
> int
> fb_change_state(struct fb_info *info, u32 state)
> {
> 	if (info->state & state)
> 		return;
> 
> 	Your fb_client_call_##name code except it is now only one 
> 	function.
> }			
> 
> What do you think?

Well... The mode change notification isn't a state per-se and would
benefit from beeing a separate function. For the Power Management
callbacks, I don't mind either way

Ben.




-------------------------------------------------------
This sf.net email is sponsored by:ThinkGeek
Welcome to geek heaven.
http://thinkgeek.com/sf