On Mon, Mar 24, 2008 at 05:43:15PM -0400, Peter Jones wrote:
This mail contains a patch which merges the IMAC mode code into the
efifb
driver and removes the imacfb driver entirely. There are also a couple of
minor bug fixes. Any comments before I start bothering the upstream
maintainers with it?
<snip>
+ switch (model) {
+ case M_I17:
+ width = 1440;
+ height = 900;
+ linelength = 1472 * 4;
+ base = 0x80010000;
+ break;
+ case M_I20:
+ width = 1680;
+ height = 1050;
+ linelength = 1728 * 4;
+ base = 0x80010000;
+ break;
+ case M_MINI:
+ width = 1024;
+ height = 768;
+ linelength = 2048 * 4;
+ base = 0x80000000;
+ break;
+ case M_MACBOOK:
+ width = 1280;
+ height = 800;
+ linelength = 2048 * 4;
+ base = 0x80000000;
+ break;
+ }
Oh wow. This is... ultragroady. Couldn't we do something slightly
cleaner, like instead of setting the dmi private data to a int flag, set
it to a pointer to a struct efifb_params or something? This has the
potential to become a switch-of-doom...
Might be nice to put the Intel Mac specific code in its own source file
too?
+
+ if (!screen_info.lfb_depth)
+ screen_info.lfb_depth = 32;
+ if (!screen_info.pages)
+ screen_info.pages = 1;
+ if (base && !screen_info.lfb_base)
+ screen_info.lfb_base = base;
+
+ if (manual_width)
+ width = manual_width;
+ if (width && !screen_info.lfb_width)
+ screen_info.lfb_width = width;
+ if (manual_height)
+ height = manual_height;
+ if (height && !screen_info.lfb_height)
+ screen_info.lfb_height = height;
+
+ /* just assume they're all unset if any are */
+ if (!screen_info.blue_size) {
+ screen_info.blue_size = 8;
+ screen_info.blue_pos = 0;
+ screen_info.green_size = 8;
+ screen_info.green_pos = 8;
+ screen_info.red_size = 8;
+ screen_info.red_pos = 16;
+ screen_info.rsvd_size = 8;
+ screen_info.rsvd_pos = 24;
+ }
+
+ if (linelength && !screen_info.lfb_linelength)
+ screen_info.lfb_linelength = linelength;
And possibly slurp this out of line into a seperate setup_fb_from_param
function or something?
It looks kind of groady for something that isn't the "common case"... I
hope.
Awesome patch though, always good to see more "-" than "+" :)
cheers, Kyle