Prev: imxfb: add flag to avoid disabling clk multiple times
Next: [PATCH 0/3 v2] msm: Add gpiolib devices to MSM7x30 and QSD8x50.
From: Marek Vasut on 19 Jun 2010 12:00 Dne St 3. března 2010 06:13:12 Pavel Machek napsal(a): > Hi! > > > >> +/* > > >> + * This is a default logo to be loaded upon driver initialization > > >> + * replacing the default Logitech G13 image loaded on device > > >> + * initialization. This is to provide the user a cue that the > > >> + * Linux driver is loaded and ready. > > >> + * > > >> + * This logo contains the text G13 in the center with two penguins > > >> + * on each side of the text. The penguins are a 33x40 rendition of > > >> + * the default framebuffer 80x80 monochrome image scaled down and > > >> + * cleaned up to retain something that looks a little better than > > >> + * a simple scaling. > > >> + * > > >> + * This logo is a simple xbm image created in GIMP and exported. > > >> + * To view the image copy the following two #defines plus the > > >> + * g13_lcd_bits to an ASCII text file and save with extension > > >> + * .xbm, then open with GIMP or any other graphical editor > > >> + * such as eog that recognizes the .xbm format. > > >> + */ > > > > > > ... > > > > > >> +static unsigned char g13_lcd_bits[] = { > > >> + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, > > >> 0x00, > > > > > > I think you'll agree above is not an elegant solution. > > > > I could make it a config option. > > You get about 10 complains already, but you are not willing to solve > it. No, config option does not help. > > If you want to, make it separate patch so we can reject it separately. > > > >> +static DEVICE_ATTR(rgb, 0666, g13_rgb_show, g13_rgb_store); > > > > > > Reading above code, it looks like this new userspace sysfs attribute > > > is for backlight control. Could that be better exposed using the > > > existing backlight class? > > > > I looked at the backlight class and it didn't seem to be a very good fit. > > > > /* This structure defines all the properties of a backlight */ > > struct backlight_properties { > > > > /* Current User requested brightness (0 - max_brightness) */ > > int brightness; > > /* Maximal value for brightness (read-only) */ > > int max_brightness; > > /* Current FB Power mode (0: full on, 1..3: power saving > > > > modes; 4: full off), see FB_BLANK_XXX */ > > > > int power; > > /* FB Blanking active? (values as for power) */ > > /* Due to be removed, please use (state & BL_CORE_FBBLANK) */ > > int fb_blank; > > /* Flags used to signal drivers of state changes */ > > /* Upper 4 bits are reserved for driver internal use */ > > unsigned int state; > > > > }; > > > > The g13's backlight doesn't support any of these properties. All you can > > set is an rgb value on the g13; no power modes, no brightness, etc. I > > could add a brightness attribute by scaling the rgb values, but I would > > prefer to do that from userspace rather than the driver since it's not > > supported in the device itself. > > > > And, we'd still have to have the rgb attribute since the backlight class > > doesn't have color. > > Or you could pretend you have 3 different backlights? ....which is > actually what you have...? > Pavel Any updates on this? btw. it'd be good if we could use the g13fb on G15 ... would that be possible? Cheers! -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: Jiri Kosina on 21 Jun 2010 08:50 On Sat, 19 Jun 2010, Marek Vasut wrote: > > > >> +/* > > > >> + * This is a default logo to be loaded upon driver initialization > > > >> + * replacing the default Logitech G13 image loaded on device > > > >> + * initialization. This is to provide the user a cue that the > > > >> + * Linux driver is loaded and ready. > > > >> + * > > > >> + * This logo contains the text G13 in the center with two penguins > > > >> + * on each side of the text. The penguins are a 33x40 rendition of > > > >> + * the default framebuffer 80x80 monochrome image scaled down and > > > >> + * cleaned up to retain something that looks a little better than > > > >> + * a simple scaling. > > > >> + * > > > >> + * This logo is a simple xbm image created in GIMP and exported. > > > >> + * To view the image copy the following two #defines plus the > > > >> + * g13_lcd_bits to an ASCII text file and save with extension > > > >> + * .xbm, then open with GIMP or any other graphical editor > > > >> + * such as eog that recognizes the .xbm format. > > > >> + */ > > > > > > > > ... > > > > > > > >> +static unsigned char g13_lcd_bits[] = { > > > >> + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, > > > >> 0x00, > > > > > > > > I think you'll agree above is not an elegant solution. > > > > > > I could make it a config option. > > > > You get about 10 complains already, but you are not willing to solve > > it. No, config option does not help. > > > > If you want to, make it separate patch so we can reject it separately. > > > > > >> +static DEVICE_ATTR(rgb, 0666, g13_rgb_show, g13_rgb_store); > > > > > > > > Reading above code, it looks like this new userspace sysfs attribute > > > > is for backlight control. Could that be better exposed using the > > > > existing backlight class? > > > > > > I looked at the backlight class and it didn't seem to be a very good fit. > > > > > > /* This structure defines all the properties of a backlight */ > > > struct backlight_properties { > > > > > > /* Current User requested brightness (0 - max_brightness) */ > > > int brightness; > > > /* Maximal value for brightness (read-only) */ > > > int max_brightness; > > > /* Current FB Power mode (0: full on, 1..3: power saving > > > > > > modes; 4: full off), see FB_BLANK_XXX */ > > > > > > int power; > > > /* FB Blanking active? (values as for power) */ > > > /* Due to be removed, please use (state & BL_CORE_FBBLANK) */ > > > int fb_blank; > > > /* Flags used to signal drivers of state changes */ > > > /* Upper 4 bits are reserved for driver internal use */ > > > unsigned int state; > > > > > > }; > > > > > > The g13's backlight doesn't support any of these properties. All you can > > > set is an rgb value on the g13; no power modes, no brightness, etc. I > > > could add a brightness attribute by scaling the rgb values, but I would > > > prefer to do that from userspace rather than the driver since it's not > > > supported in the device itself. > > > > > > And, we'd still have to have the rgb attribute since the backlight class > > > doesn't have color. > > > > Or you could pretend you have 3 different backlights? ....which is > > actually what you have...? > > Pavel > > Any updates on this? > > btw. it'd be good if we could use the g13fb on G15 ... would that be possible? > Cheers! Rick, please correct me if I am wrong, but as far as I remember, you have gathered quite some feedback and were about to incorporate them and re-submit the driver, which hasn't happened yet. Is that correct? Or have I lost track completely? Thanks, -- Jiri Kosina SUSE Labs, Novell Inc. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: Rick L. Vinyard, Jr. on 22 Jun 2010 15:10 Jiri Kosina wrote: > On Sat, 19 Jun 2010, Marek Vasut wrote: > >> > > >> +/* >> > > >> + * This is a default logo to be loaded upon driver >> initialization >> > > >> + * replacing the default Logitech G13 image loaded on device >> > > >> + * initialization. This is to provide the user a cue that the >> > > >> + * Linux driver is loaded and ready. >> > > >> + * >> > > >> + * This logo contains the text G13 in the center with two >> penguins >> > > >> + * on each side of the text. The penguins are a 33x40 rendition >> of >> > > >> + * the default framebuffer 80x80 monochrome image scaled down >> and >> > > >> + * cleaned up to retain something that looks a little better >> than >> > > >> + * a simple scaling. >> > > >> + * >> > > >> + * This logo is a simple xbm image created in GIMP and exported. >> > > >> + * To view the image copy the following two #defines plus the >> > > >> + * g13_lcd_bits to an ASCII text file and save with extension >> > > >> + * .xbm, then open with GIMP or any other graphical editor >> > > >> + * such as eog that recognizes the .xbm format. >> > > >> + */ >> > > > >> > > > ... >> > > > >> > > >> +static unsigned char g13_lcd_bits[] = { >> > > >> + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, >> 0x00, >> > > >> 0x00, >> > > > >> > > > I think you'll agree above is not an elegant solution. >> > > >> > > I could make it a config option. >> > >> > You get about 10 complains already, but you are not willing to solve >> > it. No, config option does not help. >> > >> > If you want to, make it separate patch so we can reject it separately. >> > >> > > >> +static DEVICE_ATTR(rgb, 0666, g13_rgb_show, g13_rgb_store); >> > > > >> > > > Reading above code, it looks like this new userspace sysfs >> attribute >> > > > is for backlight control. Could that be better exposed using the >> > > > existing backlight class? >> > > >> > > I looked at the backlight class and it didn't seem to be a very good >> fit. >> > > >> > > /* This structure defines all the properties of a backlight */ >> > > struct backlight_properties { >> > > >> > > /* Current User requested brightness (0 - max_brightness) */ >> > > int brightness; >> > > /* Maximal value for brightness (read-only) */ >> > > int max_brightness; >> > > /* Current FB Power mode (0: full on, 1..3: power saving >> > > >> > > modes; 4: full off), see FB_BLANK_XXX */ >> > > >> > > int power; >> > > /* FB Blanking active? (values as for power) */ >> > > /* Due to be removed, please use (state & BL_CORE_FBBLANK) */ >> > > int fb_blank; >> > > /* Flags used to signal drivers of state changes */ >> > > /* Upper 4 bits are reserved for driver internal use */ >> > > unsigned int state; >> > > >> > > }; >> > > >> > > The g13's backlight doesn't support any of these properties. All you >> can >> > > set is an rgb value on the g13; no power modes, no brightness, etc. >> I >> > > could add a brightness attribute by scaling the rgb values, but I >> would >> > > prefer to do that from userspace rather than the driver since it's >> not >> > > supported in the device itself. >> > > >> > > And, we'd still have to have the rgb attribute since the backlight >> class >> > > doesn't have color. >> > >> > Or you could pretend you have 3 different backlights? ....which is >> > actually what you have...? >> > Pavel >> >> Any updates on this? >> >> btw. it'd be good if we could use the g13fb on G15 ... would that be >> possible? >> Cheers! > > Rick, please correct me if I am wrong, but as far as I remember, you have > gathered quite some feedback and were about to incorporate them and > re-submit the driver, which hasn't happened yet. Is that correct? Or have > I lost track completely? > I've been waiting to resubmit until I get some of the userspace libraries and utilities a little more mature. There were some things I was exposing to sysfs that I didn't absolutely need, and some others that I needed to add. The cairo and papyrus libraries are (preliminarily) fully working. Once I make sure I have everything the daemon will need I'll resubmit the driver, and once that is done I was planning on submitting the libs/utils to Fedora. For now, I've been dropping Fedora 12/13 patches and kernel SRPMS here: http://miskatonic.cs.nmsu.edu/g13/ And the userspace libraries/utilities are at: http://g13.sourceforge.net As for the g13fb on the G15... I would imagine it could be done, but I don't have a G15 so I can't say for sure. If I had to guess I think you'll have to take the core functionality of the keyboard driver and extend it with the fb code in the _same_ driver if the keyboard and display use a similar design to the G13. The G13 uses the same endpoint for both keyboard input and display messages, and since a USB endpoint can't be shared across drivers both will have to reside in the same driver. -- Rick -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: Bruno Prémont on 23 Jun 2010 06:30 Hi Rick, Please check how your driver behaves if the framebuffer is still in use (open and/or mmapped) by some userspace process when you unplug the keyboard. Probably you will get hit be the same complaints from kernel I was seeing with PicoLCD driver (see my patch http://lkml.org/lkml/2010/5/30/15 that is expected to fix it on PicoLCD side, the refcounting part of the patch) Bruno On Tue, 22 June 2010 "Rick L. Vinyard, Jr." <rvinyard(a)cs.nmsu.edu> wrote: > I've been waiting to resubmit until I get some of the userspace libraries > and utilities a little more mature. There were some things I was exposing > to sysfs that I didn't absolutely need, and some others that I needed to > add. > > The cairo and papyrus libraries are (preliminarily) fully working. Once I > make sure I have everything the daemon will need I'll resubmit the driver, > and once that is done I was planning on submitting the libs/utils to > Fedora. > > For now, I've been dropping Fedora 12/13 patches and kernel SRPMS here: > http://miskatonic.cs.nmsu.edu/g13/ > > And the userspace libraries/utilities are at: > http://g13.sourceforge.net > > As for the g13fb on the G15... I would imagine it could be done, but I > don't have a G15 so I can't say for sure. > > If I had to guess I think you'll have to take the core functionality of > the keyboard driver and extend it with the fb code in the _same_ driver if > the keyboard and display use a similar design to the G13. > > The G13 uses the same endpoint for both keyboard input and display > messages, and since a USB endpoint can't be shared across drivers both > will have to reside in the same driver. > > -- > > Rick > -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
From: Jiri Kosina on 23 Jun 2010 06:40
On Wed, 23 Jun 2010, Bruno Pr�mont wrote: > Please check how your driver behaves if the framebuffer is still in use > (open and/or mmapped) by some userspace process when you unplug the > keyboard. > > Probably you will get hit be the same complaints from kernel I was seeing > with PicoLCD driver (see my patch http://lkml.org/lkml/2010/5/30/15 that > is expected to fix it on PicoLCD side, the refcounting part of the patch) BTW I'd like to have this fixed, but I haven't seen any Acks from framebuffer people, which I'd really like to have before taking it through my tree. Thanks, -- Jiri Kosina SUSE Labs, Novell Inc. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo(a)vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/ |