Re: [Linux-fbdev-devel] latest patch for atyfb, even more patches

[Antonino A. D. <ad...@ho...>]

On Sunday 17 October 2004 05:47, Alexander Kern wrote:
> Am Freitag, 15. Oktober 2004 21:25 schrieb Alexander Kern:
> > Am Dienstag, 12. Oktober 2004 14:42 schrieb Antonino A. Daplas:
>
> Hi,  here is more patches to go
>
> Our cursor API is a bit understandable, 80 % of drivers given up, and using
> soft_cursor (see a comment in ffb.c#61).
>
> Three drivers made an error and use cursor->enable insted of
> info->cursor.enable.
>
> [PATCH atyfb] fix hw cursor breakage
>
> [PATCH g364fb] fix hw cursor breakage
>
> [PATCH imsttfb] fix hw cursor breakage
>
> And tdfxfb is in my mind, broken as a hell.
> cut from tdfxfb_cursor function
>
> #ifdef TDFX_HARDWARE_CURSOR <-- not defined at all

Yep, tdfxfb_cursor() doesn't get compiled as it's seriously broken.

> static int tdfxfb_cursor(struct fb_info *info, struct fb_cursor *cursor)
> {
> ......
> 	if (info->cursor.enable) {    < Hey it's right
> ......
> 	/* Turn the cursor on */
> 	cursor->enable = 1;
> 	info->cursor = *cursor;    < Outch!
> }
> #endif
>

Yep, the cursor API, IMHO is very confusing, some fields are taken from
info->cursor (ie, mask, enable, rop), the rest from the passed fb_cursor
structure. There are also a lot of short-circuits in the cursor code path.

I've cleaned all this up in my tree so all drivers will use fields in the
passed fb_cursor structure _only_. Also, drivers need not store the changed
fields in info->cursor, fbcon will do that for you.  The end result is that
drivers don't have to look at info->cursor at all.  

Driver changes should be minimal, all references to info->cursor will just
be changed to cursor.  And things like this can be removed:

if (cursor->set & FB_CUR_BLAH)
	info->cursor.blah = cursor->blah; <-- this line can be removed

I'll document all of this in skeletenfb.c.

Other changes: cleaned up the cursor code path.

I'll submit the cursor API patches (including my attempt to modify
drivers) when 2.6.9 comes out.

Tony