fbdev cursor management

fbdev cursor management

[Andrew Morton]

Guys,

I've been taking a look at the cursor flashing code,
from the point of view of how it's affected by the
recent enabling of interrupts across the console code.

Pretty much all of the cursor-blink stuff is racy,
and has always been racy on SMP.  Enabling the
interrupts has made it racy on UP as well.

It all happens in timer handlers and interrupt handlers,
with no protection against mainline code accessing the
hardware simultaneously.  As far as I can see, the
races will be harmless - the worst they can do is to
leave some snailtrails on the screen, which will soon
scroll away. So unless someone feels strongly about it,
or can pick a problem which I've missed I'd propose that
we leave it as it is for now.

vgacon looks OK.


Some things which I'd propose for future work:

- Collapse all the various per-driver cursor flashing
  routines into a single place - manage the timer from
  drivers/video/fbcon.c and from there, call into the
  driver layer if requested.

- The cursor flash code should do the actual flash stuff
  from within process context, not interrupt context.  This
  way, it can do acquire_console_sem() to serialise
  everything properly.

  The only way we have of doing this at present is to call
  schedule_task() from within the timer handler.  This works
  fine, but it complicates the device close and module unload
  problem somewhat.  del_timer_sync + flush_scheduled_tasks
  will be needed in the right places.

  There are lots of places in the kernel which would benefit
  from a process-context timer callback mechanism - ethernet
  drivers in particular.  So this is a piece of infrastructure
  which we should develop for 2.5.  It'll be just fine for
  use in das blinkencursor code.

- With rivafb, I note that fbcon_vbl_handler() is being
  called at 50 Hz, and it doesn't actually *do* anything.
  All the work is being done by riva_cursor_timer_handler().
  Seems a little inefficient?

- riva_cursor_timer_handler() is being called at 100Hz,
  which is also more costly than it needs to be.  Also,
  it does this:

	rinfo->cursor->timer->expires = jiffies + (HZ / 100);

  so if the system has HZ < 100, the machine locks
  up - run_timer_list() never returns.  Minor point, but
  it's another argument in favour of using, say, HZ/10.

-

Re: fbdev cursor management

[James Simmons]

>Guys,
>
>I've been taking a look at the cursor flashing code,
>from the point of view of how it's affected by the
>recent enabling of interrupts across the console code.

I see the problems. I just pursposed a new cursor api soe we can have a
clean fbdev to fbcon abstraction. Also we really need to move the curent
cursor code in fbcon.c:fbcon_startup. They really belong in their
respected drivers.

>It all happens in timer handlers and interrupt handlers,
>with no protection against mainline code accessing the
>hardware simultaneously.

Many drivers use spinlocks to manage access.

>- Collapse all the various per-driver cursor flashing
>  routines into a single place - manage the timer from
>  drivers/video/fbcon.c and from there, call into the
>  driver layer if requested.

fbcon_cursor :-)

> The only way we have of doing this at present is to call
> schedule_task() from within the timer handler.  This works
> fine, but it complicates the device close and module unload
> problem somewhat.  del_timer_sync + flush_scheduled_tasks
> will be needed in the right places.

Yuck!!! I see this also a problem for VBL dependent drivers. Some
drivers to require you sync with the VBL to do things like change
the color map. Worst some of these drivers require you poll a bit
to see if the VBL occured.

[snip] rivafb problems. These will be fixed now they are pointed out.


MS: (n) 1. A debilitating and surprisingly widespread affliction that
renders the sufferer barely able to perform the simplest task. 2. A disease.

James Simmons  [jsimmons@linux-fbdev.org]               ____/|
fbdev/console/gfx developer                             \ o.O|
http://www.linux-fbdev.org                               =(_)=
http://linuxgfx.sourceforge.net                            U
http://linuxconsole.sourceforge.net