On Mon, 21 November 2011 Timur Tabi wrote:

Bruno Prémont wrote:

quoted

Wouldn't it even make sense to move some more of the initialization of fb_info
out of fb registration into this new init funtion? (I'm thinking about initializing
mutexes and the like)

I was thinking the opposite.  See my reply to Florian's message.

Well, in there you don't mention a reason to put it in
register_framebuffer() rather than collect initialization into
a new function for exactly that task.

quoted

This way fb_info could be used before being registered. Registration would then
be reduced to makeing the framebuffer visible to userspace and listed in
registered_fb[].

This way framebuffer_alloc() would be no more that kzalloc(), framebuffer_init()
would setup all "non-zero" fields of fb_info (including setup of all mutexes, one
of which is currently being done by framebuffer_alloc() and the rest by
do_register_framebuffer()!).

I think the problem is that it's unclear what the difference is between
framebuffer_alloc() and register_framebuffer().  The problem is that
register_framebuffer() also initializes the fb_info structure.  So some
initialization is done in framebuffer_alloc(), some is done in register_framebuffer(),
and some is done by the driver.  Not only that, but drivers that don't call
framebuffer_alloc() can't really know what needs to be initialized before
calling register_framebuffer().

Why is fb_info->lock initialized in register_framebuffer(), but
fb_info->bl_curve_mutex is initialized in framebuffer_alloc()?
 They're both mutexes.

Exactly, thats why I would prefer to see all that initialization moved
out of registrer_framebuffer() into a init_framebuffer().

For simplicity any driver that uses framebuffer_alloc() should not need
to additionally call init_framebuffer() - framebuffer_alloc() should
call it just before returning.
All those drivers that don't call framebuffer_alloc() would then have to
call init_framebuffer().

I think register_framebuffer() is a bit too late to do the initialisation
of mutexes and other class state because driver cannot use the same code
for HW setup before registration and after registration as at least the lock
is in undefined state.


In pseudo-code my though would be:


// driver using their own memory allocation

struct driver_data {
   ...
   struct fb_info fb;
   ...
}

int driver_init() {
   struct driver_data *data;
   ...
   memset(&data->fb, 0, sizeof(data->fb));
   init_framebuffer(&data->fb);
   // fill in driver's settings for fb
   ...
   register_framebuffer(&data->fb);
   ...
}

// driver using alloc_framebuffer()

struct driver_data {
   ...
   struct fb_info *fb;
   ...
}

int driver_init() {
   struct driver_data *data;
   ...
   data->fb = alloc_framebuffer(0); // this implicitly calls init_framebuffer();
   // fill in driver's settings for fb
   ...
   register_framebuffer(&data->fb);
   ...
}




Bruno