From mboxrd@z Thu Jan 1 00:00:00 1970 From: Miguel Ojeda Subject: Re: [PATCH] Logitech G13 driver (fixed cc list --- ignore others) Date: Fri, 15 Jan 2010 00:34:02 +0100 Message-ID: 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> <8f404284c29a6e7736de49ede9a44a2c.squirrel@intranet.cs.nmsu.edu> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: QUOTED-PRINTABLE Return-path: In-Reply-To: <8f404284c29a6e7736de49ede9a44a2c.squirrel-2xSMGd46i5akveL4JqN78fZ8FUJU4vz8@public.gmane.org> Sender: linux-usb-owner-u79uwXL29TY76Z2rM5mHXA@public.gmane.org To: "Rick L. Vinyard, Jr." Cc: Jaya Kumar , Pavel Machek , Jiri Kosina , linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, krzysztof.h1-5tc4TXWwyLM@public.gmane.org, Andrew Morton , linux-usb-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Oliver Neukum , linux-input-u79uwXL29TY76Z2rM5mHXA@public.gmane.org List-Id: linux-input@vger.kernel.org On Fri, Jan 15, 2010 at 12:03 AM, Rick L. Vinyard, Jr. wrote: > 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 actu= ally >>>>> 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 Sams= ung >>>>> 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 independ= ent, >>>>> 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 probab= ly >>>>> make sense to write a cleaner approach with a drivers/video/ks010= 8.c >>>>> which is cleanly platform independent (and back within fbdev prop= er) >>>>> 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 goi= ng >>>> to need ks0108, then I agree: we should write a independent driver= =2E >>>> >>>> It should not be hard, it is an easy controller to play with and t= he >>>> code is already there. I would try to do it; however, I am not sur= e 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 hel= p >>>> gladly. >>>> >>> >>> When I started to look at writing the G13 framebuffer the first cod= e 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 patter= ned >>> the >>> G13 after it instead. >>> >>> In moving between the two, the biggest difference was that I was ab= le to >>> strip out alot of the workqueue code you had since all that was pro= vided >>> 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 wor= k */ >>> and below comment with the exception of cfag12864b_update(). >>> >>> cfag12864b_update() would become almost analogous to the g13_fb_upd= ate() >>> 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 mem= cmp() >>> 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 befor= e, >> 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 dea= l! >> >> About ks0108, have you got any thoughts on how to write a generic >> driver? Do you need something special about ks0108? I only needed ra= w >> 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 sugges= ted > taking a framebuffer approach for the G13, Pavel suggested looking at= the > auxdisplay code. > > But, the LCD in the G13 is really a USB device that ships the image o= ut as > an interrupt message with the framebuffer image as the payload. So, i= n > essence, the callback in the G13 is really a usbhid_submit_message() = after > some other work to massage the bits from an xbm format to a format > specific to the Logitech game panel. > Oh, excuse me, I started reading from your first message in this thread after a search for "ks0108" on my inbox and I thought Jaya was referring to your code. Miguel Ojeda > --- > > Rick > > > -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org More majordomo info at http://vger.kernel.org/majordomo-info.html