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: Thu, 14 Jan 2010 23:34:21 +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> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: QUOTED-PRINTABLE Return-path: In-Reply-To: <044387d146c2f91cb7f593736fcce28f.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 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 actual= ly >>> an fbdev driver but it is outside of the drivers/video directory. I= t >>> looks like there has only been 1 commit and that was for the Samsun= g >>> KS0108 controller. It also sort of uses platform support but the wa= y >>> it is abstracted is odd to my thinking. The controller is ks0108, s= o >>> in my mind that would be the code that would be platform independen= t, >>> 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 i= t >> 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 o= f >> 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 patterne= d 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 provi= ded > by defio. Otherwise, the general structure was almost identical. > > In particular, what would change is the lower half of cfag12864b.c an= d 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_updat= e() I > have in the G13 driver which is triggered by the deferred_io member o= f 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 memcm= p() > to see if the buffer has change since the deferred_io is only going t= o > 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? > > 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". 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