Re: [PATCH 2.6.19-rc1 full] drivers: add LCD support

[Franck Bui-Huu <vagabon.xyz@gmail.com>]

Miguel Ojeda wrote:
> Refresh rate is fixed in this driver (the user can change it to other
> value at Kconfig or at loading time as a module parameter).
> 
> An application should not refresh images so fast (the LCD controller
> can handle it, but the readability is pretty bad). Having a refresh
> rate like 10 Hz for example, the application can be sure all images
> are displayed. Anyway, an animation of 10 Hz wouldn't be fine at this
> kind of LCDs, so it is pointless which the refresh rate of the driver
> is, as it is not useful to display images as fast as the driver
> refresh the LCD.
> 

An application might want to display quickly a set of images, not for
doing animations but rather displaying 'fake' greyscale images.

> Well, mmaping is the best option, as it is the easiest and the
> fastest. Any use will be better using mmaping than doing synchrone
> write(). Yes, many uses just need a write() call, but other uses would
> need mmap.
> 

That's true for devices which have a frame buffer. But it's not your
case. You _emulate_ this behaviour and I can see big drawbacks to
this design:

  - You need to keep synchrone your buffer and the display every
    100ms. It makes your CPU busy even if nothing has been written in
    LCD for a while. Futhermore this kind of display is likely to run
    on embedded system where CPU speed can be less than 100MHz.

  - All accesses to the device depend on the previous behaviour whereas
    write(), read() syscall could be synchrone and easier to use for
    fast writing of image sets. Actually the refresh stuff is really
    needed only if you mmap the device. And it seems not really used
    for now since it was broken on your last patch.

  - Your driver _is_ a frame buffer driver by design, I think you
    don't need to seperate cfa12864cb.c and cfa12864cfb.c

So ok you code is very small with this design but IMHO it is not the
most polyvalent if you want to address all needs. Why not starting the
refresh stuff only if you mmap the device and keep the rest synch ?


> $ cat /sys/module/cfag12864b/parameters/cfag12864b_rate
> 
> Yes, a more generic option would be better. Do the fbdevices have some
> standard way to retrieve such kind of info (like the bits per pixel,
> width, height...)? If not, which would be a good to retrieve the
> refresh rate?

maybe fb_ioctl() can return that. Or your driver can have its own
ioctl() method which could return such specific info.

		Franck
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/