Re: [Linux-fbdev-devel] [BK FBDEV] A few more updates.

James Simmons
2003-03-26T05:23:43Z

> 1. The following lines are missing from softcursor.c, but is present in
> fb_show_logo() where it shoudn't be:
>
> atomic_dec(&info->pixmap.count);
> smp_mb__after_atomic_dec();
>
> In both cases, the reference counting will be incorrect which can cause
> irregular cursor flashing and unnecessary "syncing". Only functions
> that call fb_get_buffer_offset() need to decrement the reference count
> afterwards.

Fixed. Applied. I missed it when I was applying your patches by hand. My
tree was way to different to accept your patches.

> 2. I applied your workqueue patch and changed locking from spinlocks to
> semaphores in fb_get_buffer_offset().

Cool. Needed really bad. Applied.

> 3. BTW, there are too many kmalloc's/kfree's in accel_cursor() and
> softcursor(). Personally, I would rather have 2 64-byte buffers for the
> mask and the data in the info->cursor structure than allocating/freeing
> memory each time the cursor flashes. However, if you prefer doing it
> this way, the patch also includes changes so kmallocs are only done when
> necessary. Still, accel_cursor() has unnecessary work being done, such
> as always creating the mask bitmap, when a simple flag to monitor cursor
> shape changes could prevent all this.

I agree. The problem is the upper layer of the console system is to brain
dead. Its either erase the cursor or redraw it again. There is no way to
just say cursor just moved. There is a CM_MOVE but the upper layer doesn't
even use it :-( If you look at vgacon and friends you will see they
recreate the cursor every time the cursor blinks. Yes even vgacon.c does
this. It is stupid and brain dead but that is the way the upper layers of
the console work. The correct solution would be to use actually use
CM_MOVE in the upper layers.

> 5. softcursor should not concern itself with memory bookkeeping, and
> must be able to function with just the parameter passed to it in order
> to keep it as simple as possible. These tasks are moved to
> accel_cursor.

We do if we make a ioctl for cursors. I'm trying to avoid reprogramming
the hardware over and over again if the properties of the cursor don't
change. The idea is similar to passing in var and comparing it to the var
in struct fb_info.

> 6. The patch should also fix the "irregularly dashed" cursor.

Okay.

> PS: What happened to the logo? I just get a white square in the uppper left corner.

Strange. It should work. I reworked the logo code anyways. I did this
because if we have more than one framebuffer in the system, say vesafb and
vga16fb. We need to select the 16 color logo and a 224 color logo. The
right logo needs to be draw on the right screen.