From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Rick L. Vinyard, Jr." Subject: Re: [PATCH] Logitech G13 driver (fixed cc list --- ignore others) Date: Thu, 14 Jan 2010 16:03:23 -0700 Message-ID: <8f404284c29a6e7736de49ede9a44a2c.squirrel@intranet.cs.nmsu.edu> References: <200912142122.nBELMW7d001243@mustang.cs.nmsu.edu> <20091216103423.GE1377@ucw.cz> <7f9100aac5f9b06ec78efff25c7a5a71.squirrel@intranet.cs.nmsu.edu> <20100104224817.GA21695@elf.ucw.cz> <45a44e481001041614i35ceef84q5f12a068e2f0b97b@mail.gmail.com> <044387d146c2f91cb7f593736fcce28f.squirrel@intranet.cs.nmsu.edu> Mime-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Transfer-Encoding: QUOTED-PRINTABLE Return-path: Received: from mail.cs.nmsu.edu ([128.123.64.3]:63858 "EHLO mail.cs.nmsu.edu" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754655Ab0ANXDe (ORCPT ); Thu, 14 Jan 2010 18:03:34 -0500 In-Reply-To: Sender: linux-input-owner@vger.kernel.org List-Id: linux-input@vger.kernel.org To: Miguel Ojeda Cc: Jaya Kumar , Pavel Machek , Jiri Kosina , linux-kernel@vger.kernel.org, krzysztof.h1@wp.pl, Andrew Morton , linux-usb@vger.kernel.org, Oliver Neukum , linux-input@vger.kernel.org Miguel Ojeda wrote: > On Thu, Jan 14, 2010 at 10:48 PM, Rick L. Vinyard, Jr. > wrote: >> Miguel Ojeda wrote: >>> On Tue, Jan 5, 2010 at 1:14 AM, Jaya Kumar >>> wrote: >>>> On Tue, Jan 5, 2010 at 6:48 AM, Pavel Machek 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 actua= lly >>>> 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 Samsu= ng >>>> KS0108 controller. It also sort of uses platform support but the w= ay >>>> 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 independe= nt, >>>> and then that would use a board specific IO driver to push data (e= g: >>>> parport or gpio or usb). I think in the long term it would probabl= y >>>> make sense to write a cleaner approach with a drivers/video/ks0108= =2Ec >>>> which is cleanly platform independent (and back within fbdev prope= r) >>>> 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 goin= g >>> 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 th= e >>> 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. >> > > I hope it was useful, at least at first. : ) > >> However, as I was digging through the video/* directory looking for >> something (I forget now what) I came across the hecubafb and pattern= ed >> the >> G13 after it instead. >> >> In moving between the two, the biggest difference was that I was abl= e to >> strip out alot of the workqueue code you had since all that was prov= ided >> by defio. Otherwise, the general structure was almost identical. >> >> In particular, what would change is the lower half of cfag12864b.c a= nd >> 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_upda= te() >> 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) >> { >> =A0 =A0 =A0 =A0cfag12864b_update(info->par); >> } >> >> static struct fb_deferred_io cfag12864b_defio =3D { >> =A0 =A0 =A0 =A0.delay =3D HZ / CFAG12864B_UPDATE_RATE_DEFAULT, >> =A0 =A0 =A0 =A0.deferred_io =3D cfag12864b_deferred_io, >> }; >> > > Thank you for the analysis of cfag12864b. See below. > >> >> The other major change is that you could eliminate the periodic memc= mp() >> to see if the buffer has change since the deferred_io is only going = to >> trigger on a page write fault. > > Yeah, I admit the memcmp() is pretty ugly knowing about deferred_io, > which I did not. It is strange that anyone pointed it out long before= , > is it new? Are there any known drawbacks? > Not sure how old it is... I don't know of any drawbacks. >> >> But, that isn't a major change in the code... only in performance. >> > > So less code and greater performance. That sounds like a winning deal= ! > > About ks0108, have you got any thoughts on how to write a generic > driver? Do you need something special about ks0108? I only needed raw > output operations so I just implemented that. Also, cfag12864b uses > two ks0108 controllers and I suppose other LCD's use many more, so > there are many points that may need a "research". > Actually, I don't need the ks0108 code. Way back when Alan Cox suggeste= d taking a framebuffer approach for the G13, Pavel suggested looking at t= he auxdisplay code. But, the LCD in the G13 is really a USB device that ships the image out= as an interrupt message with the framebuffer image as the payload. So, in essence, the callback in the G13 is really a usbhid_submit_message() af= ter some other work to massage the bits from an xbm format to a format specific to the Logitech game panel. --- Rick -- To unsubscribe from this list: send the line "unsubscribe linux-input" = in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html