* [PATCH] powermac: defer work in backlight key press
@ 2006-07-04 1:39 Aristeu Sergio Rozanski Filho
2006-07-04 8:06 ` Johannes Berg
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Aristeu Sergio Rozanski Filho @ 2006-07-04 1:39 UTC (permalink / raw)
To: linuxppc-dev
powermac: defer work in backlight key press
pmac_backlight_key() is called under interrupt context, can't use mutexes or
semaphores, so defer the backlight level for later, as it's not critical
Signed-off-by: Aristeu S. Rozanski F. <aris@valeta.org>
Index: ppc-2.6/arch/powerpc/platforms/powermac/backlight.c
===================================================================
--- ppc-2.6.orig/arch/powerpc/platforms/powermac/backlight.c 2006-07-03 21:00:44.000000000 -0300
+++ ppc-2.6/arch/powerpc/platforms/powermac/backlight.c 2006-07-03 21:38:12.000000000 -0300
@@ -15,6 +15,10 @@
#define OLD_BACKLIGHT_MAX 15
+static void pmac_backlight_key_worker(void *data);
+static DECLARE_WORK(pmac_backlight_key_work, pmac_backlight_key_worker, NULL);
+static int pmac_backlight_key_queued;
+
/* Protect the pmac_backlight variable */
DEFINE_MUTEX(pmac_backlight_mutex);
@@ -71,7 +75,7 @@
return level;
}
-static void pmac_backlight_key(int direction)
+static void pmac_backlight_key_worker(void *data)
{
mutex_lock(&pmac_backlight_mutex);
if (pmac_backlight) {
@@ -82,7 +86,8 @@
props = pmac_backlight->props;
brightness = props->brightness +
- ((direction?-1:1) * (props->max_brightness / 15));
+ ((pmac_backlight_key_queued?-1:1) *
+ (props->max_brightness / 15));
if (brightness < 0)
brightness = 0;
@@ -97,6 +102,14 @@
mutex_unlock(&pmac_backlight_mutex);
}
+static void pmac_backlight_key(int direction)
+{
+ /* we can receive multiple interrupts here, but the scheduled work
+ * will run only once, with the last value */
+ pmac_backlight_key_queued = direction;
+ schedule_work(&pmac_backlight_key_work);
+}
+
void pmac_backlight_key_up()
{
pmac_backlight_key(0);
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] powermac: defer work in backlight key press 2006-07-04 1:39 [PATCH] powermac: defer work in backlight key press Aristeu Sergio Rozanski Filho @ 2006-07-04 8:06 ` Johannes Berg 2006-07-04 11:03 ` Aristeu Sergio Rozanski Filho 2006-07-08 14:35 ` Michael Hanselmann 2006-07-08 15:58 ` [PATCH] powermac: defer work in backlight key press (and export fixes) Michael Hanselmann 2 siblings, 1 reply; 6+ messages in thread From: Johannes Berg @ 2006-07-04 8:06 UTC (permalink / raw) To: Aristeu Sergio Rozanski Filho; +Cc: linuxppc-dev [-- Attachment #1: Type: text/plain, Size: 377 bytes --] > > + /* we can receive multiple interrupts here, but the scheduled work > + * will run only once, with the last value */ > + pmac_backlight_key_queued = direction; Shouldn't you be doing something like using the whole pmac_backlight_key_queued in the workqueue, then setting it to 0 there and using += here? That way all keypresses will be adhered to. johannes [-- Attachment #2: This is a digitally signed message part --] [-- Type: application/pgp-signature, Size: 828 bytes --] ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] powermac: defer work in backlight key press 2006-07-04 8:06 ` Johannes Berg @ 2006-07-04 11:03 ` Aristeu Sergio Rozanski Filho 2006-07-04 11:16 ` Johannes Berg 0 siblings, 1 reply; 6+ messages in thread From: Aristeu Sergio Rozanski Filho @ 2006-07-04 11:03 UTC (permalink / raw) To: Johannes Berg; +Cc: linuxppc-dev > > + /* we can receive multiple interrupts here, but the scheduled work > > + * will run only once, with the last value */ > > + pmac_backlight_key_queued = direction; > > Shouldn't you be doing something like using the whole > pmac_backlight_key_queued in the workqueue, then setting it to 0 there > and using += here? That way all keypresses will be adhered to. today pmac_backlight_key() is called either with 0 (to reduce backlight level) or 1 (to increase). I found that actually happening two keystrokes between interrupt being serviced and actually running that work queue would be so damn small that the extra code wouldn't be worth. but if you think it's worth for, I can fix it, no problem. -- Aristeu ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] powermac: defer work in backlight key press 2006-07-04 11:03 ` Aristeu Sergio Rozanski Filho @ 2006-07-04 11:16 ` Johannes Berg 0 siblings, 0 replies; 6+ messages in thread From: Johannes Berg @ 2006-07-04 11:16 UTC (permalink / raw) To: Aristeu Sergio Rozanski Filho; +Cc: linuxppc-dev [-- Attachment #1: Type: text/plain, Size: 621 bytes --] On Tue, 2006-07-04 at 08:03 -0300, Aristeu Sergio Rozanski Filho wrote: > today pmac_backlight_key() is called either with 0 (to reduce backlight > level) or 1 (to increase). Oh, it looked on first look like it was -1 or 1, I see it now. > I found that actually happening two > keystrokes between interrupt being serviced and actually running that > work queue would be so damn small that the extra code wouldn't be worth. > but if you think it's worth for, I can fix it, no problem. Yeah, you're right. Unless that thing has some fast repeat if you hold keys... Not sure what I was thinking. johannes [-- Attachment #2: This is a digitally signed message part --] [-- Type: application/pgp-signature, Size: 828 bytes --] ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] powermac: defer work in backlight key press 2006-07-04 1:39 [PATCH] powermac: defer work in backlight key press Aristeu Sergio Rozanski Filho 2006-07-04 8:06 ` Johannes Berg @ 2006-07-08 14:35 ` Michael Hanselmann 2006-07-08 15:58 ` [PATCH] powermac: defer work in backlight key press (and export fixes) Michael Hanselmann 2 siblings, 0 replies; 6+ messages in thread From: Michael Hanselmann @ 2006-07-08 14:35 UTC (permalink / raw) To: Aristeu Sergio Rozanski Filho; +Cc: linuxppc-dev On Mon, Jul 03, 2006 at 10:39:23PM -0300, Aristeu Sergio Rozanski Filho wrote: > powermac: defer work in backlight key press > pmac_backlight_key() is called under interrupt context, can't use mutexes or > semaphores, so defer the backlight level for later, as it's not critical Nack, needs spinlock around this part: > + pmac_backlight_key_queued = direction; > + schedule_work(&pmac_backlight_key_work); I'll submit another patch with more fixes soon. Greets, Michael ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] powermac: defer work in backlight key press (and export fixes) 2006-07-04 1:39 [PATCH] powermac: defer work in backlight key press Aristeu Sergio Rozanski Filho 2006-07-04 8:06 ` Johannes Berg 2006-07-08 14:35 ` Michael Hanselmann @ 2006-07-08 15:58 ` Michael Hanselmann 2 siblings, 0 replies; 6+ messages in thread From: Michael Hanselmann @ 2006-07-08 15:58 UTC (permalink / raw) To: akpm; +Cc: linuxppc-dev, johannes, aris, linux-kernel pmac_backlight_key() is called under interrupt context, and therefore can't use mutexes or semaphores, so defer the backlight level for later, as it's not critical (code by Aristeu S. Rozanski F. <aris@valeta.org>). Also, it fixes exports and Kconfig depdencies. Signed-off-by: Michael Hanselmann <linux-kernel@hansmi.ch> Acked-by: Aristeu S. Rozanski F. <aris@valeta.org> --- diff -Nrup --exclude-from linux-exclude-from linux-2.6.18-rc1.orig/arch/powerpc/platforms/powermac/backlight.c linux-2.6.18-rc1/arch/powerpc/platforms/powermac/backlight.c --- linux-2.6.18-rc1.orig/arch/powerpc/platforms/powermac/backlight.c 2006-07-08 12:27:01.000000000 +0200 +++ linux-2.6.18-rc1/arch/powerpc/platforms/powermac/backlight.c 2006-07-08 17:30:23.000000000 +0200 @@ -15,6 +15,15 @@ #define OLD_BACKLIGHT_MAX 15 +static void pmac_backlight_key_worker(void *data); +static DECLARE_WORK(pmac_backlight_key_work, pmac_backlight_key_worker, NULL); + +/* Although this variable is used in interrupt context, it makes no sense to + * protect it. No user is able to produce enough key events per second and + * notice the errors that might happen. + */ +static int pmac_backlight_key_queued; + /* Protect the pmac_backlight variable */ DEFINE_MUTEX(pmac_backlight_mutex); @@ -71,7 +80,7 @@ int pmac_backlight_curve_lookup(struct f return level; } -static void pmac_backlight_key(int direction) +static void pmac_backlight_key_worker(void *data) { mutex_lock(&pmac_backlight_mutex); if (pmac_backlight) { @@ -82,7 +91,8 @@ static void pmac_backlight_key(int direc props = pmac_backlight->props; brightness = props->brightness + - ((direction?-1:1) * (props->max_brightness / 15)); + ((pmac_backlight_key_queued?-1:1) * + (props->max_brightness / 15)); if (brightness < 0) brightness = 0; @@ -97,14 +107,13 @@ static void pmac_backlight_key(int direc mutex_unlock(&pmac_backlight_mutex); } -void pmac_backlight_key_up() +void pmac_backlight_key(int direction) { - pmac_backlight_key(0); -} - -void pmac_backlight_key_down() -{ - pmac_backlight_key(1); + /* we can receive multiple interrupts here, but the scheduled work + * will run only once, with the last value + */ + pmac_backlight_key_queued = direction; + schedule_work(&pmac_backlight_key_work); } int pmac_backlight_set_legacy_brightness(int brightness) @@ -157,3 +166,7 @@ int pmac_backlight_get_legacy_brightness return result; } + +EXPORT_SYMBOL_GPL(pmac_backlight); +EXPORT_SYMBOL_GPL(pmac_backlight_mutex); +EXPORT_SYMBOL_GPL(pmac_has_backlight_type); diff -Nrup --exclude-from linux-exclude-from linux-2.6.18-rc1.orig/drivers/macintosh/Kconfig linux-2.6.18-rc1/drivers/macintosh/Kconfig --- linux-2.6.18-rc1.orig/drivers/macintosh/Kconfig 2006-07-08 12:27:10.000000000 +0200 +++ linux-2.6.18-rc1/drivers/macintosh/Kconfig 2006-07-08 15:28:36.000000000 +0200 @@ -113,7 +113,10 @@ config PMAC_MEDIABAY config PMAC_BACKLIGHT bool "Backlight control for LCD screens" - depends on ADB_PMU && (BROKEN || !PPC64) + depends on ADB_PMU && FB = y && (BROKEN || !PPC64) + select FB_BACKLIGHT + select BACKLIGHT_CLASS_DEVICE + select BACKLIGHT_LCD_SUPPORT help Say Y here to enable Macintosh specific extensions of the generic backlight code. With this enabled, the brightness keys on older diff -Nrup --exclude-from linux-exclude-from linux-2.6.18-rc1.orig/include/asm-powerpc/backlight.h linux-2.6.18-rc1/include/asm-powerpc/backlight.h --- linux-2.6.18-rc1.orig/include/asm-powerpc/backlight.h 2006-07-08 12:27:21.000000000 +0200 +++ linux-2.6.18-rc1/include/asm-powerpc/backlight.h 2006-07-08 17:14:43.000000000 +0200 @@ -16,13 +16,16 @@ extern struct backlight_device *pmac_backlight; extern struct mutex pmac_backlight_mutex; -extern void pmac_backlight_calc_curve(struct fb_info*); extern int pmac_backlight_curve_lookup(struct fb_info *info, int value); extern int pmac_has_backlight_type(const char *type); -extern void pmac_backlight_key_up(void); -extern void pmac_backlight_key_down(void); +extern void pmac_backlight_key(int direction); + +#define pmac_backlight_key_up() \ + do { pmac_backlight_key(0); } while(0); +#define pmac_backlight_key_down() \ + do { pmac_backlight_key(1); } while(0); extern int pmac_backlight_set_legacy_brightness(int brightness); extern int pmac_backlight_get_legacy_brightness(void); ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2006-07-08 15:58 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2006-07-04 1:39 [PATCH] powermac: defer work in backlight key press Aristeu Sergio Rozanski Filho 2006-07-04 8:06 ` Johannes Berg 2006-07-04 11:03 ` Aristeu Sergio Rozanski Filho 2006-07-04 11:16 ` Johannes Berg 2006-07-08 14:35 ` Michael Hanselmann 2006-07-08 15:58 ` [PATCH] powermac: defer work in backlight key press (and export fixes) Michael Hanselmann
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.