Re: Fbdev and PM patch.

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