[PATCH] Merge the IMAC mode code with efifb and remove imacfb entirely.

[Kyle McMartin]

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