Re: [PATCH] Logitech G13 driver (fixed cc list --- ignore others)

[Rick L. Vinyard, Jr.]

Miguel Ojeda wrote:
> On Tue, Jan 5, 2010 at 1:14 AM, Jaya Kumar <jayakumar.lkml@xxxxxxxxx>
> wrote:
>> On Tue, Jan 5, 2010 at 6:48 AM, Pavel Machek <pavel@xxxxxx> wrote:
>>>
>>> Ok, but I guess you should cc auxdisplay people in future.
>>
>> Hi Pavel,
>>
>> I just looked at the drivers/auxdisplay directory and got a bit
>> confused. The reason I got confused is because auxdisplay is actually
>> an fbdev driver but it is outside of the drivers/video directory. It
>> looks like there has only been 1 commit and that was for the Samsung
>> KS0108 controller. It also sort of uses platform support but the way
>> it is abstracted is odd to my thinking. The controller is ks0108, so
>> in my mind that would be the code that would be platform independent,
>> and then that would use a board specific IO driver to push data (eg:
>> parport or gpio or usb). I think in the long term it would probably
>> make sense to write a cleaner approach with a drivers/video/ks0108.c
>> which is cleanly platform independent (and back within fbdev proper)
>> and then a board specific driver in the appropriate location that
>> handles the IO.
>
> I wrote long ago the driver(s) and people that reviewed it thought it
> was better to keep it outside. I think that if someone else is going
> to need ks0108, then I agree: we should write a independent driver.
>
> It should not be hard, it is an easy controller to play with and the
> code is already there. I would try to do it; however, I am not sure if
> I would be the most appropriate person to code such generic driver, as
> I know almost nothing about all drivers/video/* stuff and the ways of
> making it truly generic for future video/ users. Still, I will help
> gladly.
>

When I started to look at writing the G13 framebuffer the first code I
looked at was the cfag12864b, and started off trying to adapt it.

However, as I was digging through the video/* directory looking for
something (I forget now what) I came across the hecubafb and patterned the
G13 after it instead.

In moving between the two, the biggest difference was that I was able to
strip out alot of the workqueue code you had since all that was provided
by defio. Otherwise, the general structure was almost identical.

In particular, what would change is the lower half of cfag12864b.c and you
would be able to eliminate almost everything from the /* Update work */
and below comment with the exception of cfag12864b_update().

cfag12864b_update() would become almost analogous to the g13_fb_update() I
have in the G13 driver which is triggered by the deferred_io member of the
fb_deferred_io structure.

You would have something like:

/* Callback from deferred IO workqueue */
static void cfag12864b_deferred_io(struct fb_info *info, struct list_head
*pagelist)
{
	cfag12864b_update(info->par);
}

static struct fb_deferred_io cfag12864b_defio = {
	.delay = HZ / CFAG12864B_UPDATE_RATE_DEFAULT,
	.deferred_io = cfag12864b_deferred_io,
};


The other major change is that you could eliminate the periodic memcmp()
to see if the buffer has change since the deferred_io is only going to
trigger on a page write fault.

But, that isn't a major change in the code... only in performance.

---

Rick


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