Re: [Linux-fbdev-devel] Comments on fbgen.c and fbcon-accel.c

[James Simmons]

> I have a few observations on fbgen and fbcon-accel.

Don't mix fbgen with fbcon-accel. The new gen_* stuff in fbgen.c is meant
to replace the old fbgen_* stuff. That is why the below doesn't work.
 
> 1.  fbcon_accel_clear_margins may not work correctly with fbgen since
> fbcon_accel will use the xoffset and yoffset values from info->var.  
> 
>     void fbcon_accel_clear_margins(struct vc_data *vc, struct display
>     *p,
>     			       int bottom_only)
>     {
>     	<<<snip>>> 
>     
>     	if (bh) {
>     		region.dx = info->var.xoffset;
>                     region.dy = info->var.yoffset + bs;
>                     region.width  = rs;
>                     region.height = bh;
>     		info->fbops->fb_fillrect(info, &region);
>     	}
>     }
> 
> However fbgen_pan_display updates the xoffset and yoffset in
> fb_display[con].var. So margins don't get cleared if the driver supports
> y-panning or y-wrapping.
> 
>     int fbgen_pan_display(struct fb_var_screeninfo *var, int con,
>     		      struct fb_info *info)
>     {
>     	<<< snip >>>
>     
>         if (con == info->currcon) {
>     	if (fbhw->pan_display) {
>     	    if ((err = fbhw->pan_display(var, info2)))
>     		return err;
>     	} else
>     	    return -EINVAL;
>         }
>         fb_display[con].var.xoffset = var->xoffset;
>         fb_display[con].var.yoffset = var->yoffset;
>     
>     	<<< snip >>>
>     }

> 2. Also, fbgen_switch basically just do an fbgen_do_set_var()
> (decode_var(), followed by set_par()).  This is okay most times, but
> it's probably better if fbgen_switch also does an encode_fix() since
> fbcon's drawing functions also rely on fix->line_length.

Most likely that is also broken. I haven't thought about it since I plan
to make all the old fbgen_* functions go away.

> If an fb_fix_screeninfo is not updated, display corruption occurs when
> switching to another display with a different pixelformat.

Correct. That is why I require info->fix to be updated when set_par is
called.