2.5 offb breakage

2.5 offb breakage

[Benjamin Herrenschmidt]

Hi !

I've investigated a bit why offb dies on boot with current 2.5.

It appears that dispsw is never set. You removed the code setting
it for the various bit depth. It seems the gen_set_disp() call
is supposed to do that instead.

However, gen_set_disp() only does it for var.bits_per_pixel == 24,
not for the other bit depths. In my case, offb boots at 8bpp, thus
dispsw is never set, causing fbcon_setup to die when trying to
call set_font(). That is without the accel in config. If I enable
accel support, then it dies later, in cfbimgblt, apparently due
to cfb_imageblit() beeing called with a NULL argument.

What is the correct fix ? Changing back offb to set dispsw or 
changing gen_set_disp() to set dispsw for all bit depth ?

Also, when code in fbcon_accel.c is calling things like
cfb_imageblit, it uses the "fb_info" field of structure
struct display. Where is this supposed to be initialized ?
It doesn't seem it's done in the drivers themselves (surely
not in offb).

Regards,
Ben.





-------------------------------------------------------
This sf.net email is sponsored by: Dice - The leading online job board
for high-tech professionals. Search and apply for tech jobs today!
http://seeker.dice.com/seeker.epl?rel_code=31

Re: 2.5 offb breakage

[James Simmons]

Sorry I have been away for some time. I'm looking for work :-(

> What is the correct fix ? Changing back offb to set dispsw or
> changing gen_set_disp() to set dispsw for all bit depth ?

I found what the problem was. The soft accel files cfb*.c where not being
linked into off.c. Try this patch and tell me if everything works. BTW you
need to have CONFIG_FBCON_ACCEL set. Eventually all the fbcon-cfb* stuff
will go away.

> Also, when code in fbcon_accel.c is calling things like
> cfb_imageblit, it uses the "fb_info" field of structure
> struct display. Where is this supposed to be initialized ?
> It doesn't seem it's done in the drivers themselves (surely
> not in offb).

The upper fbcon code sets that. See fbcon_init in fbcon.c.

   . ---
   |o_o |
   |:_/ |   Give Micro$oft the Bird!!!!
  //   \ \  Use Linux!!!!
 (|     | )
 /'\_   _/`\
 \___)=(___/





-------------------------------------------------------
This sf.net email is sponsored by: Dice - The leading online job board
for high-tech professionals. Search and apply for tech jobs today!
http://seeker.dice.com/seeker.epl?rel_code=31

Re: Re: 2.5 offb breakage

[Benjamin Herrenschmidt]

>
>Sorry I have been away for some time. I'm looking for work :-(
>
>> What is the correct fix ? Changing back offb to set dispsw or
>> changing gen_set_disp() to set dispsw for all bit depth ?
>
>I found what the problem was. The soft accel files cfb*.c where not being
>linked into off.c. Try this patch and tell me if everything works. BTW you
>need to have CONFIG_FBCON_ACCEL set. Eventually all the fbcon-cfb* stuff
>will go away.

I have fixed that for a long time or it wouldn't link anyway...

So if I don't have CONFIG_FBCON_ACCEL, it dies in fbcon_setup because
dispsw is not set, if I have CONFIG_FBCON_ACCEL set, it dies in cfb_imageblt,
I think it's display->fb_info that is null, but I have to dbl check.

Why would you kill the fbcon-cfg stuffs ? That would mean making
CONFIG_FBCON_ACCEL not to be an option then. Or simple framebuffers like
offb wouldn't work.

>> Also, when code in fbcon_accel.c is calling things like
>> cfb_imageblit, it uses the "fb_info" field of structure
>> struct display. Where is this supposed to be initialized ?
>> It doesn't seem it's done in the drivers themselves (surely
>> not in offb).
>
>The upper fbcon code sets that. See fbcon_init in fbcon.c.




-------------------------------------------------------
This sf.net email is sponsored by: Dice - The leading online job board
for high-tech professionals. Search and apply for tech jobs today!
http://seeker.dice.com/seeker.epl?rel_code=31

Re: Re: 2.5 offb breakage

[James Simmons]

> I have fixed that for a long time or it wouldn't link anyway...

I have teh fix to push to linus soon.

> So if I don't have CONFIG_FBCON_ACCEL, it dies in fbcon_setup because
> dispsw is not set,

Correct. I should have it set to fbcon_dummy instead. Added fix to BK
repository.

> if I have CONFIG_FBCON_ACCEL set, it dies in cfb_imageblt,
> I think it's display->fb_info that is null, but I have to dbl check.

I think it might be a memset issue. Try this patch.

--- /usr/src/linus-2.5/drivers/video/offb.c	Tue Jul 30 22:55:24 2002
+++ offb.c	Wed Jul 31 10:43:33 2002
@@ -82,17 +82,15 @@
 			 unsigned long address, struct device_node *dp);

 static struct fb_ops offb_ops = {
-	owner:		THIS_MODULE,
-	fb_get_fix:	gen_get_fix,
-	fb_get_var:	gen_get_var,
-	fb_set_var:	gen_set_var,
-	fb_get_cmap:	gen_get_cmap,
-	fb_set_cmap:	gen_set_cmap,
-	fb_setcolreg:	offb_setcolreg,
-	fb_blank:	offb_blank,
-	fb_fillrect:	cfb_fillrect,
-	fb_copyarea:	cfb_copyarea,
-	fb_imageblit:	cfb_imageblit,
+	.owner		= THIS_MODULE,
+	.fb_set_var	= gen_set_var,
+	.fb_get_cmap	= gen_get_cmap,
+	.fb_set_cmap	= gen_set_cmap,
+	.fb_setcolreg	= offb_setcolreg,
+	.fb_blank	= offb_blank,
+	.fb_fillrect	= cfb_fillrect,
+	.fb_copyarea	= cfb_copyarea,
+	.fb_imageblit	= cfb_imageblit,
 };

     /*
@@ -399,7 +397,7 @@
 	struct fb_fix_screeninfo *fix;
 	struct fb_var_screeninfo *var;
 	struct fb_info *info;
-	int i;
+	int size, i;

 	if (!request_mem_region(res_start, res_size, "offb"))
 		return;
@@ -414,14 +412,15 @@
 		return;
 	}

-	info =
-	    kmalloc(sizeof(struct fb_info) + sizeof(struct display) +
-		    sizeof(u32) * 17, GFP_ATOMIC);
+	size = sizeof(struct fb_info) + sizeof(struct display) + sizeof(u32) * 17;
+
+	info = kmalloc(size, GFP_ATOMIC);
+
 	if (info == 0) {
 		release_mem_region(res_start, res_size);
 		return;
 	}
-	memset(info, 0, sizeof(*info));
+	memset(info, 0, size);

 	fix = &info->fix;
 	var = &info->var;

> Why would you kill the fbcon-cfg stuffs ? That would mean making
> CONFIG_FBCON_ACCEL not to be an option then. Or simple framebuffers like
> offb wouldn't work.

The goal is to eventually move fbcon_accel into the upper framebuffer
console layer. What we want to do is move all the console code out of
every fbdev driver into fbcon.c. This will allow fbdev to exist without
fbcon. This will be allow for smaller kernels which is needed by resource
constrant handheld devices.

The problem is that over the years the limitations of the console system
have been handled on the drivers side. Now I reworking the upper layers so
the low end drivers can be really really simple. You can see the
difference with the tdfx driver or example. It had  60% code size
reduction. 60% !!!!!




-------------------------------------------------------
This sf.net email is sponsored by: Dice - The leading online job board
for high-tech professionals. Search and apply for tech jobs today!
http://seeker.dice.com/seeker.epl?rel_code=31

Re: Re: 2.5 offb breakage

[Benjamin Herrenschmidt]

>-	info =
>-	    kmalloc(sizeof(struct fb_info) + sizeof(struct display) +
>-		    sizeof(u32) * 17, GFP_ATOMIC);
>+	size = sizeof(struct fb_info) + sizeof(struct display) + sizeof(u32) * 17;
>+
>+	info = kmalloc(size, GFP_ATOMIC);

Ok, I fixed that in my tree as well, but didn't yet test with
CONFIG_FBCON_ACCEL _and_ that fix. I'll do that now.

Ben.




-------------------------------------------------------
This sf.net email is sponsored by: Dice - The leading online job board
for high-tech professionals. Search and apply for tech jobs today!
http://seeker.dice.com/seeker.epl?rel_code=31

Re: Re: 2.5 offb breakage

[Benjamin Herrenschmidt]

Ok, now it's really weird.

If I fix the memset, I get my old crash. However, it seems that
it's r4 that is NULL in cfb_imageblit, which would mean the
"image" paremeter is NULL. The stack shows the caller beeing
fbcon_accel_putcs, so that make no sense as image is on the
stack...

I'll have to investigate more.

Ben.




-------------------------------------------------------
This sf.net email is sponsored by: Dice - The leading online job board
for high-tech professionals. Search and apply for tech jobs today!
http://seeker.dice.com/seeker.epl?rel_code=31

Re: 2.5 offb breakage

[James Simmons]

Oops. Here is the patch. Let me know if it works.

--- /usr/src/linus-2.5/drivers/video/Makefile	Tue Jul 30 22:55:21 2002
+++ Makefile	Wed Jul 31 10:08:02 2002
@@ -60,7 +60,7 @@
 obj-$(CONFIG_FB_3DFX)             += tdfxfb.o
 obj-$(CONFIG_FB_MAC)              += macfb.o macmodes.o cfbfillrect.o cfbcopyarea.o cfbimgblt.o
 obj-$(CONFIG_FB_HP300)            += hpfb.o cfbfillrect.o cfbimgblt.o
-obj-$(CONFIG_FB_OF)               += offb.o
+obj-$(CONFIG_FB_OF)               += offb.o cfbfillrect.o cfbimgblit.o cfbcopyarea.o
 obj-$(CONFIG_FB_IMSTT)            += imsttfb.o
 obj-$(CONFIG_FB_RETINAZ3)         += retz3fb.o
 obj-$(CONFIG_FB_CLGEN)            += clgenfb.o



-------------------------------------------------------
This sf.net email is sponsored by: Dice - The leading online job board
for high-tech professionals. Search and apply for tech jobs today!
http://seeker.dice.com/seeker.epl?rel_code=31